[FFmpeg-devel] [PATCH 1/7 v4] avutil/frame: add a flag to allow overwritting existing entries

[FFmpeg-devel] [PATCH 1/7 v4] avutil/frame: add a flag to allow overwritting existing entries

[James Almer]

Enable it only for side data types that don't allow more than one entry.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavutil/frame.c                 | 59 ++++++++++++++++++++++++++++---
 libavutil/frame.h                 | 27 +++++++++-----
 libavutil/tests/side_data_array.c | 52 +++++++++++++++------------
 tests/ref/fate/side_data_array    | 22 ++++++------
 4 files changed, 115 insertions(+), 45 deletions(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index ef1613c344..d9bd19b2aa 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -799,12 +799,34 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
                                         enum AVFrameSideDataType type,
                                         size_t size, unsigned int flags)
 {
-    AVBufferRef     *buf = av_buffer_alloc(size);
+    const AVSideDataDescriptor *desc = av_frame_side_data_desc(type);
+    AVBufferRef     *buf;
     AVFrameSideData *ret = NULL;
 
     if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
         remove_side_data(sd, nb_sd, type);
+    if (!desc || !(desc->props & AV_SIDE_DATA_PROP_MULTI)) {
+        for (int i = 0; i < *nb_sd; i++) {
+            AVFrameSideData *entry = ((*sd)[i]);
+            if (entry->type != type)
+                continue;
+            if (!(flags & AV_FRAME_SIDE_DATA_FLAG_REPLACE))
+                return NULL;
+
+            buf = av_buffer_alloc(size);
+            if (!buf)
+                return NULL;
+
+            av_buffer_unref(&entry->buf);
+            av_dict_free(&entry->metadata);
+            entry->buf  = buf;
+            entry->data = buf->data;
+            entry->size = buf->size;
+            return entry;
+        }
+    }
 
+    buf = av_buffer_alloc(size);
     ret = add_side_data_from_buf(sd, nb_sd, type, buf);
     if (!ret)
         av_buffer_unref(&buf);
@@ -815,6 +837,7 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
 int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
                              const AVFrameSideData *src, unsigned int flags)
 {
+    const AVSideDataDescriptor *desc;
     AVBufferRef     *buf    = NULL;
     AVFrameSideData *sd_dst = NULL;
     int              ret    = AVERROR_BUG;
@@ -822,13 +845,41 @@ int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
     if (!sd || !src || !nb_sd || (*nb_sd && !*sd))
         return AVERROR(EINVAL);
 
+    desc = av_frame_side_data_desc(src->type);
+    if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
+        remove_side_data(sd, nb_sd, src->type);
+    if (!desc || !(desc->props & AV_SIDE_DATA_PROP_MULTI)) {
+        for (int i = 0; i < *nb_sd; i++) {
+            AVFrameSideData *entry = ((*sd)[i]);
+            AVDictionary *dict = NULL;
+
+            if (entry->type != src->type)
+                continue;
+            if (!(flags & AV_FRAME_SIDE_DATA_FLAG_REPLACE))
+                return AVERROR(EEXIST);
+
+            ret = av_dict_copy(&dict, src->metadata, 0);
+            if (ret < 0)
+                return ret;
+
+            ret = av_buffer_replace(&entry->buf, src->buf);
+            if (ret < 0) {
+                av_dict_free(&dict);
+                return ret;
+            }
+
+            av_dict_free(&entry->metadata);
+            entry->metadata = dict;
+            entry->data     = src->data;
+            entry->size     = src->size;
+            return 0;
+        }
+    }
+
     buf = av_buffer_ref(src->buf);
     if (!buf)
         return AVERROR(ENOMEM);
 
-    if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
-        remove_side_data(sd, nb_sd, src->type);
-
     sd_dst = add_side_data_from_buf(sd, nb_sd, src->type, buf);
     if (!sd_dst) {
         av_buffer_unref(&buf);
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 3b6d746a16..2ea129888e 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -1040,7 +1040,14 @@ const AVSideDataDescriptor *av_frame_side_data_desc(enum AVFrameSideDataType typ
  */
 void av_frame_side_data_free(AVFrameSideData ***sd, int *nb_sd);
 
+/**
+ * Remove existing entries before adding new ones.
+ */
 #define AV_FRAME_SIDE_DATA_FLAG_UNIQUE (1 << 0)
+/**
+ * Don't add a new entry if another of the same type exists.
+ */
+#define AV_FRAME_SIDE_DATA_FLAG_REPLACE (1 << 1)
 
 /**
  * Add new side data entry to an array.
@@ -1053,10 +1060,12 @@ void av_frame_side_data_free(AVFrameSideData ***sd, int *nb_sd);
  * @param size  size of the side data
  * @param flags Some combination of AV_FRAME_SIDE_DATA_FLAG_* flags, or 0.
  *
- * @return newly added side data on success, NULL on error. In case of
- *         AV_FRAME_SIDE_DATA_FLAG_UNIQUE being set, entries of matching
- *         AVFrameSideDataType will be removed before the addition is
- *         attempted.
+ * @return newly added side data on success, NULL on error.
+ * @note In case of AV_FRAME_SIDE_DATA_FLAG_UNIQUE being set, entries of
+ *       matching AVFrameSideDataType will be removed before the addition
+ *       is attempted.
+ * @note In case of AV_FRAME_SIDE_DATA_FLAG_REPLACE being set, if an
+ *       entry of the same type already exists, it will be replaced instead.
  */
 AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
                                         enum AVFrameSideDataType type,
@@ -1074,10 +1083,12 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
  *              for the buffer.
  * @param flags Some combination of AV_FRAME_SIDE_DATA_FLAG_* flags, or 0.
  *
- * @return negative error code on failure, >=0 on success. In case of
- *         AV_FRAME_SIDE_DATA_FLAG_UNIQUE being set, entries of matching
- *         AVFrameSideDataType will be removed before the addition is
- *         attempted.
+ * @return negative error code on failure, >=0 on success.
+ * @note In case of AV_FRAME_SIDE_DATA_FLAG_UNIQUE being set, entries of
+ *       matching AVFrameSideDataType will be removed before the addition
+ *       is attempted.
+ * @note In case of AV_FRAME_SIDE_DATA_FLAG_REPLACE being set, if an
+ *       entry of the same type already exists, it will be replaced instead.
  */
 int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
                              const AVFrameSideData *src, unsigned int flags);
diff --git a/libavutil/tests/side_data_array.c b/libavutil/tests/side_data_array.c
index 793a62c009..633e9ee681 100644
--- a/libavutil/tests/side_data_array.c
+++ b/libavutil/tests/side_data_array.c
@@ -20,23 +20,22 @@
 
 #include <stdio.h>
 #include "libavutil/frame.c"
-#include "libavutil/mastering_display_metadata.h"
+#include "libavutil/internal.h"
 
-static void print_clls(const AVFrameSideData **sd, const int nb_sd)
+static void print_entries(const AVFrameSideData **sd, const int nb_sd)
 {
     for (int i = 0; i < nb_sd; i++) {
         const AVFrameSideData *entry = sd[i];
 
-        printf("sd %d, %s",
-               i, av_frame_side_data_name(entry->type));
+        printf("sd %d (size %"SIZE_SPECIFIER"), %s",
+               i, entry->size, av_frame_side_data_name(entry->type));
 
-        if (entry->type != AV_FRAME_DATA_CONTENT_LIGHT_LEVEL) {
+        if (entry->type != AV_FRAME_DATA_SEI_UNREGISTERED) {
             putchar('\n');
             continue;
         }
 
-        printf(": MaxCLL: %u\n",
-               ((AVContentLightMetadata *)entry->data)->MaxCLL);
+        printf(": %d\n", *(int32_t *)entry->data);
     }
 }
 
@@ -51,51 +50,60 @@ int main(void)
 
     av_assert0(
         av_frame_side_data_new(&set.sd, &set.nb_sd,
-                               AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT,
-                               0, 0));
+                               AV_FRAME_DATA_CONTENT_LIGHT_LEVEL,
+                               sizeof(int64_t), 0));
+    av_assert0(
+        av_frame_side_data_new(&set.sd, &set.nb_sd,
+                               AV_FRAME_DATA_CONTENT_LIGHT_LEVEL,
+                               sizeof(int32_t), AV_FRAME_SIDE_DATA_FLAG_REPLACE));
 
     // test entries in the middle
     for (int value = 1; value < 4; value++) {
         AVFrameSideData *sd = av_frame_side_data_new(
-            &set.sd, &set.nb_sd, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL,
-            sizeof(AVContentLightMetadata), 0);
+            &set.sd, &set.nb_sd, AV_FRAME_DATA_SEI_UNREGISTERED,
+            sizeof(int32_t), 0);
 
         av_assert0(sd);
 
-        ((AVContentLightMetadata *)sd->data)->MaxCLL = value;
+        *(int32_t *)sd->data = value;
     }
 
     av_assert0(
         av_frame_side_data_new(
-            &set.sd, &set.nb_sd, AV_FRAME_DATA_SPHERICAL, 0, 0));
+            &set.sd, &set.nb_sd, AV_FRAME_DATA_SPHERICAL,
+           sizeof(int64_t), 0));
+
+    av_assert0(
+        av_frame_side_data_new(
+            &set.sd, &set.nb_sd, AV_FRAME_DATA_SPHERICAL,
+            sizeof(int32_t), AV_FRAME_SIDE_DATA_FLAG_REPLACE));
 
     // test entries at the end
     for (int value = 1; value < 4; value++) {
         AVFrameSideData *sd = av_frame_side_data_new(
-            &set.sd, &set.nb_sd, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL,
-            sizeof(AVContentLightMetadata), 0);
+            &set.sd, &set.nb_sd, AV_FRAME_DATA_SEI_UNREGISTERED,
+            sizeof(int32_t), 0);
 
         av_assert0(sd);
 
-        ((AVContentLightMetadata *)sd->data)->MaxCLL = value + 3;
+        *(int32_t *)sd->data = value + 3;
     }
 
     puts("Initial addition results with duplicates:");
-    print_clls((const AVFrameSideData **)set.sd, set.nb_sd);
+    print_entries((const AVFrameSideData **)set.sd, set.nb_sd);
 
     {
         AVFrameSideData *sd = av_frame_side_data_new(
-            &set.sd, &set.nb_sd, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL,
-            sizeof(AVContentLightMetadata),
-            AV_FRAME_SIDE_DATA_FLAG_UNIQUE);
+            &set.sd, &set.nb_sd, AV_FRAME_DATA_SEI_UNREGISTERED,
+            sizeof(int32_t), AV_FRAME_SIDE_DATA_FLAG_UNIQUE);
 
         av_assert0(sd);
 
-        ((AVContentLightMetadata *)sd->data)->MaxCLL = 1337;
+        *(int32_t *)sd->data = 1337;
     }
 
     puts("\nFinal state after a single 'no-duplicates' addition:");
-    print_clls((const AVFrameSideData **)set.sd, set.nb_sd);
+    print_entries((const AVFrameSideData **)set.sd, set.nb_sd);
 
     av_frame_side_data_free(&set.sd, &set.nb_sd);
 
diff --git a/tests/ref/fate/side_data_array b/tests/ref/fate/side_data_array
index 7d8c684d8f..c1d77b0445 100644
--- a/tests/ref/fate/side_data_array
+++ b/tests/ref/fate/side_data_array
@@ -1,14 +1,14 @@
 Initial addition results with duplicates:
-sd 0, Ambient viewing environment
-sd 1, Content light level metadata: MaxCLL: 1
-sd 2, Content light level metadata: MaxCLL: 2
-sd 3, Content light level metadata: MaxCLL: 3
-sd 4, Spherical Mapping
-sd 5, Content light level metadata: MaxCLL: 4
-sd 6, Content light level metadata: MaxCLL: 5
-sd 7, Content light level metadata: MaxCLL: 6
+sd 0 (size 4), Content light level metadata
+sd 1 (size 4), H.26[45] User Data Unregistered SEI message: 1
+sd 2 (size 4), H.26[45] User Data Unregistered SEI message: 2
+sd 3 (size 4), H.26[45] User Data Unregistered SEI message: 3
+sd 4 (size 4), Spherical Mapping
+sd 5 (size 4), H.26[45] User Data Unregistered SEI message: 4
+sd 6 (size 4), H.26[45] User Data Unregistered SEI message: 5
+sd 7 (size 4), H.26[45] User Data Unregistered SEI message: 6
 
 Final state after a single 'no-duplicates' addition:
-sd 0, Ambient viewing environment
-sd 1, Spherical Mapping
-sd 2, Content light level metadata: MaxCLL: 1337
+sd 0 (size 4), Content light level metadata
+sd 1 (size 4), Spherical Mapping
+sd 2 (size 4), H.26[45] User Data Unregistered SEI message: 1337
-- 
2.44.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/7 v4] avutil/frame: add helper for adding side data w/ AVBufferRef to array

[James Almer]

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavutil/frame.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
 libavutil/frame.h | 34 ++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index d9bd19b2aa..a165e56a64 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -834,6 +834,59 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
     return ret;
 }
 
+AVFrameSideData *av_frame_side_data_add(AVFrameSideData ***sd, int *nb_sd,
+                                        enum AVFrameSideDataType type,
+                                        AVBufferRef **pbuf, unsigned int flags)
+{
+    const AVSideDataDescriptor *desc = av_frame_side_data_desc(type);
+    AVFrameSideData *sd_dst  = NULL;
+    AVBufferRef *buf;
+
+    if (!sd || !pbuf || !*pbuf || !nb_sd || (*nb_sd && !*sd))
+        return NULL;
+
+    if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
+        remove_side_data(sd, nb_sd, type);
+    if (!desc || !(desc->props & AV_SIDE_DATA_PROP_MULTI)) {
+        for (int i = 0; i < *nb_sd; i++) {
+            AVFrameSideData *entry = ((*sd)[i]);
+
+            if (entry->type != type)
+                continue;
+            if (!(flags & AV_FRAME_SIDE_DATA_FLAG_REPLACE))
+                return NULL;
+
+            buf = *pbuf;
+            if (flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF) {
+                int ret = av_buffer_replace(&entry->buf, buf);
+                if (ret < 0)
+                    return NULL;
+            } else
+                *pbuf = NULL;
+
+            av_dict_free(&entry->metadata);
+            entry->data = buf->data;
+            entry->size = buf->size;
+            return entry;
+        }
+    }
+
+    buf = (flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF) ?
+           av_buffer_ref(*pbuf) : *pbuf;
+
+    sd_dst = add_side_data_from_buf(sd, nb_sd, type, buf);
+    if (!sd_dst) {
+        if (flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF)
+            av_buffer_unref(&buf);
+        return NULL;
+    }
+
+    if (!(flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF))
+        *pbuf = NULL;
+
+    return sd_dst;
+}
+
 int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
                              const AVFrameSideData *src, unsigned int flags)
 {
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 2ea129888e..3e5d170a5b 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -1048,6 +1048,10 @@ void av_frame_side_data_free(AVFrameSideData ***sd, int *nb_sd);
  * Don't add a new entry if another of the same type exists.
  */
 #define AV_FRAME_SIDE_DATA_FLAG_REPLACE (1 << 1)
+/**
+ * Create a new reference instead of taking ownership of the passed in one.
+ */
+#define AV_FRAME_SIDE_DATA_FLAG_NEW_REF (1 << 2)
 
 /**
  * Add new side data entry to an array.
@@ -1066,11 +1070,40 @@ void av_frame_side_data_free(AVFrameSideData ***sd, int *nb_sd);
  *       is attempted.
  * @note In case of AV_FRAME_SIDE_DATA_FLAG_REPLACE being set, if an
  *       entry of the same type already exists, it will be replaced instead.
+ * @note AV_FRAME_SIDE_DATA_FLAG_NEW_REF has no effect in this function.
  */
 AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
                                         enum AVFrameSideDataType type,
                                         size_t size, unsigned int flags);
 
+/**
+ * Add a new side data entry to an array from an existing AVBufferRef.
+ *
+ * @param sd    pointer to array of side data to which to add another entry,
+ *              or to NULL in order to start a new array.
+ * @param nb_sd pointer to an integer containing the number of entries in
+ *              the array.
+ * @param type  type of the added side data
+ * @param buf   Pointer to AVBufferRef to add to the array. On success,
+ *              the function takes ownership of the AVBufferRef and *buf is
+ *              set to NULL, unless AV_FRAME_SIDE_DATA_FLAG_NEW_REF is set
+ *              in which case the ownership will remain with the caller.
+ * @param flags Some combination of AV_FRAME_SIDE_DATA_FLAG_* flags, or 0.
+ *
+ * @return newly added side data on success, NULL on error.
+ * @note In case of AV_FRAME_SIDE_DATA_FLAG_UNIQUE being set, entries of
+ *       matching AVFrameSideDataType will be removed before the addition
+ *       is attempted.
+ * @note In case of AV_FRAME_SIDE_DATA_FLAG_REPLACE being set, if an
+ *       entry of the same type already exists, it will be replaced instead.
+ * @note In case of AV_FRAME_SIDE_DATA_FLAG_NEW_REF being set, the ownership
+ *       of *buf will remain with the caller.
+ *
+ */
+AVFrameSideData *av_frame_side_data_add(AVFrameSideData ***sd, int *nb_sd,
+                                        enum AVFrameSideDataType type,
+                                        AVBufferRef **buf, unsigned int flags);
+
 /**
  * Add a new side data entry to an array based on existing side data, taking
  * a reference towards the contained AVBufferRef.
@@ -1089,6 +1122,7 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
  *       is attempted.
  * @note In case of AV_FRAME_SIDE_DATA_FLAG_REPLACE being set, if an
  *       entry of the same type already exists, it will be replaced instead.
+ * @note AV_FRAME_SIDE_DATA_FLAG_NEW_REF has no effect in this function.
  */
 int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
                              const AVFrameSideData *src, unsigned int flags);
-- 
2.44.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/7 v4] avutil/frame: add helper for adding side data w/ AVBufferRef to array

[Anton Khirnov]

Quoting James Almer (2024-03-28 04:12:05)
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavutil/frame.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
>  libavutil/frame.h | 34 ++++++++++++++++++++++++++++++
>  2 files changed, 87 insertions(+)
> 
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index d9bd19b2aa..a165e56a64 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -834,6 +834,59 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
>      return ret;
>  }
>  
> +AVFrameSideData *av_frame_side_data_add(AVFrameSideData ***sd, int *nb_sd,
> +                                        enum AVFrameSideDataType type,
> +                                        AVBufferRef **pbuf, unsigned int flags)
> +{
> +    const AVSideDataDescriptor *desc = av_frame_side_data_desc(type);
> +    AVFrameSideData *sd_dst  = NULL;
> +    AVBufferRef *buf;
> +
> +    if (!sd || !pbuf || !*pbuf || !nb_sd || (*nb_sd && !*sd))

Overzealous checks like these tend to hide bugs. Any of these conditions
being false means the caller is insane and we should crash.

> +        return NULL;
> +
> +    if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
> +        remove_side_data(sd, nb_sd, type);
> +    if (!desc || !(desc->props & AV_SIDE_DATA_PROP_MULTI)) {
> +        for (int i = 0; i < *nb_sd; i++) {
> +            AVFrameSideData *entry = ((*sd)[i]);
> +
> +            if (entry->type != type)
> +                continue;
> +            if (!(flags & AV_FRAME_SIDE_DATA_FLAG_REPLACE))
> +                return NULL;
> +
> +            buf = *pbuf;
> +            if (flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF) {
> +                int ret = av_buffer_replace(&entry->buf, buf);
> +                if (ret < 0)
> +                    return NULL;
> +            } else
> +                *pbuf = NULL;
> +
> +            av_dict_free(&entry->metadata);
> +            entry->data = buf->data;
> +            entry->size = buf->size;
> +            return entry;

This again looks like a minor variation of the  block you've added twice
already.

> +        }
> +    }
> +
> +    buf = (flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF) ?
> +           av_buffer_ref(*pbuf) : *pbuf;
> +
> +    sd_dst = add_side_data_from_buf(sd, nb_sd, type, buf);
> +    if (!sd_dst) {
> +        if (flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF)
> +            av_buffer_unref(&buf);
> +        return NULL;
> +    }
> +
> +    if (!(flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF))
> +        *pbuf = NULL;
> +
> +    return sd_dst;
> +}
> +
>  int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
>                               const AVFrameSideData *src, unsigned int flags)
>  {
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 2ea129888e..3e5d170a5b 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -1048,6 +1048,10 @@ void av_frame_side_data_free(AVFrameSideData ***sd, int *nb_sd);
>   * Don't add a new entry if another of the same type exists.
>   */
>  #define AV_FRAME_SIDE_DATA_FLAG_REPLACE (1 << 1)
> +/**
> + * Create a new reference instead of taking ownership of the passed in one.
> + */
> +#define AV_FRAME_SIDE_DATA_FLAG_NEW_REF (1 << 2)

Who needs this?

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/7 v4] avutil/frame: add helper for adding side data w/ AVBufferRef to array

[James Almer]

On 3/28/2024 8:27 AM, Anton Khirnov wrote:
> Quoting James Almer (2024-03-28 04:12:05)
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavutil/frame.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
>>   libavutil/frame.h | 34 ++++++++++++++++++++++++++++++
>>   2 files changed, 87 insertions(+)
>>
>> diff --git a/libavutil/frame.c b/libavutil/frame.c
>> index d9bd19b2aa..a165e56a64 100644
>> --- a/libavutil/frame.c
>> +++ b/libavutil/frame.c
>> @@ -834,6 +834,59 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
>>       return ret;
>>   }
>>   
>> +AVFrameSideData *av_frame_side_data_add(AVFrameSideData ***sd, int *nb_sd,
>> +                                        enum AVFrameSideDataType type,
>> +                                        AVBufferRef **pbuf, unsigned int flags)
>> +{
>> +    const AVSideDataDescriptor *desc = av_frame_side_data_desc(type);
>> +    AVFrameSideData *sd_dst  = NULL;
>> +    AVBufferRef *buf;
>> +
>> +    if (!sd || !pbuf || !*pbuf || !nb_sd || (*nb_sd && !*sd))
> 
> Overzealous checks like these tend to hide bugs. Any of these conditions
> being false means the caller is insane and we should crash.

I'll remove some, but others simplify the code below (like knowing 
beforehand that *pbuf is not NULL).

> 
>> +        return NULL;
>> +
>> +    if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
>> +        remove_side_data(sd, nb_sd, type);
>> +    if (!desc || !(desc->props & AV_SIDE_DATA_PROP_MULTI)) {
>> +        for (int i = 0; i < *nb_sd; i++) {
>> +            AVFrameSideData *entry = ((*sd)[i]);
>> +
>> +            if (entry->type != type)
>> +                continue;
>> +            if (!(flags & AV_FRAME_SIDE_DATA_FLAG_REPLACE))
>> +                return NULL;
>> +
>> +            buf = *pbuf;
>> +            if (flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF) {
>> +                int ret = av_buffer_replace(&entry->buf, buf);
>> +                if (ret < 0)
>> +                    return NULL;
>> +            } else
>> +                *pbuf = NULL;
>> +
>> +            av_dict_free(&entry->metadata);
>> +            entry->data = buf->data;
>> +            entry->size = buf->size;
>> +            return entry;
> 
> This again looks like a minor variation of the  block you've added twice
> already.
> 
>> +        }
>> +    }
>> +
>> +    buf = (flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF) ?
>> +           av_buffer_ref(*pbuf) : *pbuf;
>> +
>> +    sd_dst = add_side_data_from_buf(sd, nb_sd, type, buf);
>> +    if (!sd_dst) {
>> +        if (flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF)
>> +            av_buffer_unref(&buf);
>> +        return NULL;
>> +    }
>> +
>> +    if (!(flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF))
>> +        *pbuf = NULL;
>> +
>> +    return sd_dst;
>> +}
>> +
>>   int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
>>                                const AVFrameSideData *src, unsigned int flags)
>>   {
>> diff --git a/libavutil/frame.h b/libavutil/frame.h
>> index 2ea129888e..3e5d170a5b 100644
>> --- a/libavutil/frame.h
>> +++ b/libavutil/frame.h
>> @@ -1048,6 +1048,10 @@ void av_frame_side_data_free(AVFrameSideData ***sd, int *nb_sd);
>>    * Don't add a new entry if another of the same type exists.
>>    */
>>   #define AV_FRAME_SIDE_DATA_FLAG_REPLACE (1 << 1)
>> +/**
>> + * Create a new reference instead of taking ownership of the passed in one.
>> + */
>> +#define AV_FRAME_SIDE_DATA_FLAG_NEW_REF (1 << 2)
> 
> Who needs this?

Someone who wants to keep the reference around, like when attaching a 
buffer to several outputs (global to individual output frames).
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/7 v4] avutil/frame: add helper for adding side data w/ AVBufferRef to array

[Anton Khirnov]

Quoting James Almer (2024-03-28 12:49:08)
> On 3/28/2024 8:27 AM, Anton Khirnov wrote:
> > Quoting James Almer (2024-03-28 04:12:05)
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >>   libavutil/frame.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
> >>   libavutil/frame.h | 34 ++++++++++++++++++++++++++++++
> >>   2 files changed, 87 insertions(+)
> >>
> >> diff --git a/libavutil/frame.c b/libavutil/frame.c
> >> index d9bd19b2aa..a165e56a64 100644
> >> --- a/libavutil/frame.c
> >> +++ b/libavutil/frame.c
> >> @@ -834,6 +834,59 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
> >>       return ret;
> >>   }
> >>   
> >> +AVFrameSideData *av_frame_side_data_add(AVFrameSideData ***sd, int *nb_sd,
> >> +                                        enum AVFrameSideDataType type,
> >> +                                        AVBufferRef **pbuf, unsigned int flags)
> >> +{
> >> +    const AVSideDataDescriptor *desc = av_frame_side_data_desc(type);
> >> +    AVFrameSideData *sd_dst  = NULL;
> >> +    AVBufferRef *buf;
> >> +
> >> +    if (!sd || !pbuf || !*pbuf || !nb_sd || (*nb_sd && !*sd))
> > 
> > Overzealous checks like these tend to hide bugs. Any of these conditions
> > being false means the caller is insane and we should crash.
> 
> I'll remove some, but others simplify the code below (like knowing 
> beforehand that *pbuf is not NULL).

You can just assume them all to be true. Or use av_assert0().

> >> diff --git a/libavutil/frame.h b/libavutil/frame.h
> >> index 2ea129888e..3e5d170a5b 100644
> >> --- a/libavutil/frame.h
> >> +++ b/libavutil/frame.h
> >> @@ -1048,6 +1048,10 @@ void av_frame_side_data_free(AVFrameSideData ***sd, int *nb_sd);
> >>    * Don't add a new entry if another of the same type exists.
> >>    */
> >>   #define AV_FRAME_SIDE_DATA_FLAG_REPLACE (1 << 1)
> >> +/**
> >> + * Create a new reference instead of taking ownership of the passed in one.
> >> + */
> >> +#define AV_FRAME_SIDE_DATA_FLAG_NEW_REF (1 << 2)
> > 
> > Who needs this?
> 
> Someone who wants to keep the reference around, like when attaching a 
> buffer to several outputs (global to individual output frames).

Is that a common enough use case to warrant this flag? It complicates
the code quite substantially.

And if you're making some side data instance global, what is the point
of also attaching it to frames?

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/7 v4] avutil/frame: add helper for adding side data w/ AVBufferRef to array

[James Almer]

On 3/28/2024 9:19 AM, Anton Khirnov wrote:
> Quoting James Almer (2024-03-28 12:49:08)
>> On 3/28/2024 8:27 AM, Anton Khirnov wrote:
>>> Quoting James Almer (2024-03-28 04:12:05)
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>>    libavutil/frame.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++
>>>>    libavutil/frame.h | 34 ++++++++++++++++++++++++++++++
>>>>    2 files changed, 87 insertions(+)
>>>>
>>>> diff --git a/libavutil/frame.c b/libavutil/frame.c
>>>> index d9bd19b2aa..a165e56a64 100644
>>>> --- a/libavutil/frame.c
>>>> +++ b/libavutil/frame.c
>>>> @@ -834,6 +834,59 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
>>>>        return ret;
>>>>    }
>>>>    
>>>> +AVFrameSideData *av_frame_side_data_add(AVFrameSideData ***sd, int *nb_sd,
>>>> +                                        enum AVFrameSideDataType type,
>>>> +                                        AVBufferRef **pbuf, unsigned int flags)
>>>> +{
>>>> +    const AVSideDataDescriptor *desc = av_frame_side_data_desc(type);
>>>> +    AVFrameSideData *sd_dst  = NULL;
>>>> +    AVBufferRef *buf;
>>>> +
>>>> +    if (!sd || !pbuf || !*pbuf || !nb_sd || (*nb_sd && !*sd))
>>>
>>> Overzealous checks like these tend to hide bugs. Any of these conditions
>>> being false means the caller is insane and we should crash.
>>
>> I'll remove some, but others simplify the code below (like knowing
>> beforehand that *pbuf is not NULL).
> 
> You can just assume them all to be true. Or use av_assert0().
> 
>>>> diff --git a/libavutil/frame.h b/libavutil/frame.h
>>>> index 2ea129888e..3e5d170a5b 100644
>>>> --- a/libavutil/frame.h
>>>> +++ b/libavutil/frame.h
>>>> @@ -1048,6 +1048,10 @@ void av_frame_side_data_free(AVFrameSideData ***sd, int *nb_sd);
>>>>     * Don't add a new entry if another of the same type exists.
>>>>     */
>>>>    #define AV_FRAME_SIDE_DATA_FLAG_REPLACE (1 << 1)
>>>> +/**
>>>> + * Create a new reference instead of taking ownership of the passed in one.
>>>> + */
>>>> +#define AV_FRAME_SIDE_DATA_FLAG_NEW_REF (1 << 2)
>>>
>>> Who needs this?
>>
>> Someone who wants to keep the reference around, like when attaching a
>> buffer to several outputs (global to individual output frames).
> 
> Is that a common enough use case to warrant this flag? It complicates
> the code quite substantially.
> 
> And if you're making some side data instance global, what is the point
> of also attaching it to frames?

To pass it to API that does not handle global entries, like currently 
happens with filtergraphs.

I can remove the flag for now in any case. It can be added later trivially.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/7 v4] avutil/frame: use the same data pointer as the source entry when cloning side data

[James Almer]

src->data does not need to match src->buf->data

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavutil/frame.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index a165e56a64..d27998d1f4 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -744,13 +744,11 @@ AVBufferRef *av_frame_get_plane_buffer(const AVFrame *frame, int plane)
 static AVFrameSideData *add_side_data_from_buf(AVFrameSideData ***sd,
                                                int *nb_sd,
                                                enum AVFrameSideDataType type,
-                                               AVBufferRef *buf)
+                                               AVBufferRef *buf, uint8_t *data,
+                                               size_t size)
 {
     AVFrameSideData *ret, **tmp;
 
-    if (!buf)
-        return NULL;
-
     // *nb_sd + 1 needs to fit into an int and a size_t.
     if ((unsigned)*nb_sd >= FFMIN(INT_MAX, SIZE_MAX))
         return NULL;
@@ -765,8 +763,8 @@ static AVFrameSideData *add_side_data_from_buf(AVFrameSideData ***sd,
         return NULL;
 
     ret->buf = buf;
-    ret->data = ret->buf->data;
-    ret->size = buf->size;
+    ret->data = data;
+    ret->size = size;
     ret->type = type;
 
     (*sd)[(*nb_sd)++] = ret;
@@ -778,9 +776,13 @@ AVFrameSideData *av_frame_new_side_data_from_buf(AVFrame *frame,
                                                  enum AVFrameSideDataType type,
                                                  AVBufferRef *buf)
 {
+    if (!buf)
+        return NULL;
+
     return
         add_side_data_from_buf(
-            &frame->side_data, &frame->nb_side_data, type, buf);
+            &frame->side_data, &frame->nb_side_data, type, buf,
+            buf->data, buf->size);
 }
 
 AVFrameSideData *av_frame_new_side_data(AVFrame *frame,
@@ -827,7 +829,9 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
     }
 
     buf = av_buffer_alloc(size);
-    ret = add_side_data_from_buf(sd, nb_sd, type, buf);
+    if (!buf)
+        return NULL;
+    ret = add_side_data_from_buf(sd, nb_sd, type, buf, buf->data, size);
     if (!ret)
         av_buffer_unref(&buf);
 
@@ -873,8 +877,10 @@ AVFrameSideData *av_frame_side_data_add(AVFrameSideData ***sd, int *nb_sd,
 
     buf = (flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF) ?
            av_buffer_ref(*pbuf) : *pbuf;
+    if (!buf)
+        return NULL;
 
-    sd_dst = add_side_data_from_buf(sd, nb_sd, type, buf);
+    sd_dst = add_side_data_from_buf(sd, nb_sd, type, buf, buf->data, buf->size);
     if (!sd_dst) {
         if (flags & AV_FRAME_SIDE_DATA_FLAG_NEW_REF)
             av_buffer_unref(&buf);
@@ -933,7 +939,7 @@ int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
     if (!buf)
         return AVERROR(ENOMEM);
 
-    sd_dst = add_side_data_from_buf(sd, nb_sd, src->type, buf);
+    sd_dst = add_side_data_from_buf(sd, nb_sd, src->type, buf, src->data, src->size);
     if (!sd_dst) {
         av_buffer_unref(&buf);
         return AVERROR(ENOMEM);
-- 
2.44.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/7 v4] avutil/frame: use the same data pointer as the source entry when cloning side data

[Anton Khirnov]

Quoting James Almer (2024-03-28 04:12:06)
> src->data does not need to match src->buf->data

When does it not?

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/7 v4] avutil/frame: use the same data pointer as the source entry when cloning side data

[James Almer]

On 3/28/2024 8:29 AM, Anton Khirnov wrote:
> Quoting James Almer (2024-03-28 04:12:06)
>> src->data does not need to match src->buf->data
> 
> When does it not?

It always matches for side data allocated by us within the libraries, 
but those are not the only callers, so this function should honor the 
data and size values in the source.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 3/7 v4] avutil/frame: use the same data pointer as the source entry when cloning side data

[Anton Khirnov]

Quoting James Almer (2024-03-28 12:33:47)
> On 3/28/2024 8:29 AM, Anton Khirnov wrote:
> > Quoting James Almer (2024-03-28 04:12:06)
> >> src->data does not need to match src->buf->data
> > 
> > When does it not?
> 
> It always matches for side data allocated by us within the libraries, 
> but those are not the only callers, so this function should honor the 
> data and size values in the source.

Okay then

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 4/7 v4] avutil/frame: add helper to remove side data of a given type from an array

[James Almer]

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavutil/frame.c | 6 ++++++
 libavutil/frame.h | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/libavutil/frame.c b/libavutil/frame.c
index d27998d1f4..f54c1fa472 100644
--- a/libavutil/frame.c
+++ b/libavutil/frame.c
@@ -965,6 +965,12 @@ const AVFrameSideData *av_frame_side_data_get_c(const AVFrameSideData * const *s
     return NULL;
 }
 
+void av_frame_side_data_remove(AVFrameSideData ***sd, int *nb_sd,
+                               enum AVFrameSideDataType type)
+{
+    remove_side_data(sd, nb_sd, type);
+}
+
 AVFrameSideData *av_frame_get_side_data(const AVFrame *frame,
                                         enum AVFrameSideDataType type)
 {
diff --git a/libavutil/frame.h b/libavutil/frame.h
index 3e5d170a5b..da78aeea7f 100644
--- a/libavutil/frame.h
+++ b/libavutil/frame.h
@@ -1156,6 +1156,11 @@ const AVFrameSideData *av_frame_side_data_get(AVFrameSideData * const *sd,
                                     nb_sd, type);
 }
 
+/**
+ * Remove and free all side data instances of the given type from an array.
+ */
+void av_frame_side_data_remove(AVFrameSideData ***sd, int *nb_sd,
+                               enum AVFrameSideDataType type);
 /**
  * @}
  */
-- 
2.44.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 5/7 v4] avutil/mastering_display_metadata: add a new allocator function that returns a size

[James Almer]

av_mastering_display_metadata_alloc() is not useful in scenarios where you need to
know the runtime size of AVMasteringDisplayMetadata.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavutil/mastering_display_metadata.c | 13 +++++++++++++
 libavutil/mastering_display_metadata.h |  9 +++++++++
 2 files changed, 22 insertions(+)

diff --git a/libavutil/mastering_display_metadata.c b/libavutil/mastering_display_metadata.c
index 6069347617..ea41f13f9d 100644
--- a/libavutil/mastering_display_metadata.c
+++ b/libavutil/mastering_display_metadata.c
@@ -18,6 +18,7 @@
  * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
  */
 
+#include <stddef.h>
 #include <stdint.h>
 #include <string.h>
 
@@ -29,6 +30,18 @@ AVMasteringDisplayMetadata *av_mastering_display_metadata_alloc(void)
     return av_mallocz(sizeof(AVMasteringDisplayMetadata));
 }
 
+AVMasteringDisplayMetadata *av_mastering_display_metadata_alloc_size(size_t *size)
+{
+    AVMasteringDisplayMetadata *mastering = av_mallocz(sizeof(AVMasteringDisplayMetadata));
+    if (!mastering)
+        return NULL;
+
+    if (size)
+        *size = sizeof(*mastering);
+
+    return mastering;
+}
+
 AVMasteringDisplayMetadata *av_mastering_display_metadata_create_side_data(AVFrame *frame)
 {
     AVFrameSideData *side_data = av_frame_new_side_data(frame,
diff --git a/libavutil/mastering_display_metadata.h b/libavutil/mastering_display_metadata.h
index c23b07c3cd..52fcef9e37 100644
--- a/libavutil/mastering_display_metadata.h
+++ b/libavutil/mastering_display_metadata.h
@@ -77,6 +77,15 @@ typedef struct AVMasteringDisplayMetadata {
  */
 AVMasteringDisplayMetadata *av_mastering_display_metadata_alloc(void);
 
+/**
+ * Allocate an AVMasteringDisplayMetadata structure and set its fields to
+ * default values. The resulting struct can be freed using av_freep().
+ *
+ * @return An AVMasteringDisplayMetadata filled with default values or NULL
+ *         on failure.
+ */
+AVMasteringDisplayMetadata *av_mastering_display_metadata_alloc_size(size_t *size);
+
 /**
  * Allocate a complete AVMasteringDisplayMetadata and add it to the frame.
  *
-- 
2.44.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 6/7 v4] avcodec/decode: make the AVFrameSideData helper wrappers not depend on frames

[James Almer]

They will be useful to fill arrays stored in other structs.

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/av1dec.c     |  7 +--
 libavcodec/cri.c        |  3 +-
 libavcodec/decode.c     | 99 +++++++++++++++++++++++++++--------------
 libavcodec/decode.h     | 28 ++++++------
 libavcodec/dpx.c        |  3 +-
 libavcodec/h2645_sei.c  |  4 +-
 libavcodec/h264_slice.c |  3 +-
 libavcodec/hevcdec.c    |  6 ++-
 libavcodec/libdav1d.c   |  7 +--
 libavcodec/libjxldec.c  |  3 +-
 libavcodec/mjpegdec.c   |  3 +-
 libavcodec/mpeg12dec.c  | 11 +++--
 libavcodec/pngdec.c     |  8 ++--
 libavcodec/qsvdec.c     |  4 +-
 libavcodec/tiff.c       |  3 +-
 libavcodec/webp.c       |  3 +-
 16 files changed, 120 insertions(+), 75 deletions(-)

diff --git a/libavcodec/av1dec.c b/libavcodec/av1dec.c
index 32a795e758..54bedc27e1 100644
--- a/libavcodec/av1dec.c
+++ b/libavcodec/av1dec.c
@@ -964,7 +964,8 @@ static int export_itut_t35(AVCodecContext *avctx, AVFrame *frame,
             if (!ret)
                 break;
 
-            ret = ff_frame_new_side_data_from_buf(avctx, frame, AV_FRAME_DATA_A53_CC, &buf, NULL);
+            ret = ff_frame_new_side_data_from_buf(avctx, &frame->side_data, &frame->nb_side_data,
+                                                  AV_FRAME_DATA_A53_CC, &buf);
             if (ret < 0)
                 return ret;
 
@@ -1028,7 +1029,7 @@ static int export_metadata(AVCodecContext *avctx, AVFrame *frame)
     if (s->mdcv) {
         AVMasteringDisplayMetadata *mastering;
 
-        ret = ff_decode_mastering_display_new(avctx, frame, &mastering);
+        ret = ff_decode_mastering_display_new(avctx, &frame->side_data, &frame->nb_side_data, &mastering);
         if (ret < 0)
             return ret;
 
@@ -1051,7 +1052,7 @@ static int export_metadata(AVCodecContext *avctx, AVFrame *frame)
     if (s->cll) {
         AVContentLightMetadata *light;
 
-        ret = ff_decode_content_light_new(avctx, frame, &light);
+        ret = ff_decode_content_light_new(avctx, &frame->side_data, &frame->nb_side_data, &light);
         if (ret < 0)
             return ret;
 
diff --git a/libavcodec/cri.c b/libavcodec/cri.c
index 990e52ac99..94468e7515 100644
--- a/libavcodec/cri.c
+++ b/libavcodec/cri.c
@@ -398,7 +398,8 @@ skip:
     }
 
     if (hflip || vflip) {
-        ff_frame_new_side_data(avctx, p, AV_FRAME_DATA_DISPLAYMATRIX,
+        ff_frame_new_side_data(avctx, &p->side_data, &p->nb_side_data,
+                               AV_FRAME_DATA_DISPLAYMATRIX,
                                sizeof(int32_t) * 9, &rotation);
         if (rotation) {
             av_display_rotation_set((int32_t *)rotation->data, 0.f);
diff --git a/libavcodec/decode.c b/libavcodec/decode.c
index 34bcb7cc64..48048cd599 100644
--- a/libavcodec/decode.c
+++ b/libavcodec/decode.c
@@ -1793,93 +1793,124 @@ int ff_decode_preinit(AVCodecContext *avctx)
 }
 
 /**
- * Check side data preference and clear existing side data from frame
+ * Check side data preference and clear existing side data from sd/nb_sd
  * if needed.
  *
  * @retval 0 side data of this type can be added to frame
  * @retval 1 side data of this type should not be added to frame
  */
-static int side_data_pref(const AVCodecContext *avctx, AVFrame *frame,
-                          enum AVFrameSideDataType type)
+static int side_data_pref(const AVCodecContext *avctx, AVFrameSideData ***sd,
+                          int *nb_sd, enum AVFrameSideDataType type)
 {
     DecodeContext *dc = decode_ctx(avctx->internal);
 
     // Note: could be skipped for `type` without corresponding packet sd
-    if (av_frame_get_side_data(frame, type)) {
+    if (av_frame_side_data_get(*sd, *nb_sd, type)) {
         if (dc->side_data_pref_mask & (1ULL << type))
             return 1;
-        av_frame_remove_side_data(frame, type);
+        av_frame_side_data_remove(sd, nb_sd, type);
     }
 
     return 0;
 }
 
-
-int ff_frame_new_side_data(const AVCodecContext *avctx, AVFrame *frame,
-                           enum AVFrameSideDataType type, size_t size,
+int ff_frame_new_side_data(const AVCodecContext *avctx, AVFrameSideData ***sd,
+                           int *nb_sd, enum AVFrameSideDataType type, size_t size,
                            AVFrameSideData **psd)
 {
-    AVFrameSideData *sd;
+    AVFrameSideData *entry;
 
-    if (side_data_pref(avctx, frame, type)) {
+    if (side_data_pref(avctx, sd, nb_sd, type)) {
         if (psd)
             *psd = NULL;
         return 0;
     }
 
-    sd = av_frame_new_side_data(frame, type, size);
+    entry = av_frame_side_data_new(sd, nb_sd, type, size, 0);
     if (psd)
-        *psd = sd;
+        *psd = entry;
 
-    return sd ? 0 : AVERROR(ENOMEM);
+    return entry ? 0 : AVERROR(ENOMEM);
 }
 
-int ff_frame_new_side_data_from_buf(const AVCodecContext *avctx,
-                                    AVFrame *frame, enum AVFrameSideDataType type,
-                                    AVBufferRef **buf, AVFrameSideData **psd)
+int ff_frame_new_side_data_from_buf(const AVCodecContext *avctx, AVFrameSideData ***sd,
+                                    int *nb_sd, enum AVFrameSideDataType type,
+                                    AVBufferRef **buf)
 {
-    AVFrameSideData *sd = NULL;
     int ret = 0;
 
-    if (side_data_pref(avctx, frame, type))
+    if (side_data_pref(avctx, sd, nb_sd, type))
         goto finish;
 
-    sd = av_frame_new_side_data_from_buf(frame, type, *buf);
-    if (sd)
-        *buf = NULL;
-    else
+    if (!av_frame_side_data_add(sd, nb_sd, type, buf, 0))
         ret = AVERROR(ENOMEM);
 
 finish:
     av_buffer_unref(buf);
-    if (psd)
-        *psd = sd;
 
     return ret;
 }
 
-int ff_decode_mastering_display_new(const AVCodecContext *avctx, AVFrame *frame,
-                                    AVMasteringDisplayMetadata **mdm)
+int ff_decode_mastering_display_new(const AVCodecContext *avctx, AVFrameSideData ***sd,
+                                    int *nb_sd, AVMasteringDisplayMetadata **mdm)
 {
-    if (side_data_pref(avctx, frame, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA)) {
+    AVBufferRef *buf;
+    size_t size;
+
+    if (side_data_pref(avctx, sd, nb_sd, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA)) {
         *mdm = NULL;
         return 0;
     }
 
-    *mdm = av_mastering_display_metadata_create_side_data(frame);
-    return *mdm ? 0 : AVERROR(ENOMEM);
+    *mdm = av_mastering_display_metadata_alloc_size(&size);
+    if (!*mdm)
+        return AVERROR(ENOMEM);
+
+    buf = av_buffer_create((uint8_t *)*mdm, size, NULL, NULL, 0);
+    if (!buf) {
+        av_freep(mdm);
+        return AVERROR(ENOMEM);
+    }
+
+    if (!av_frame_side_data_add(sd, nb_sd, AV_FRAME_DATA_MASTERING_DISPLAY_METADATA,
+                                &buf, 0)) {
+        *mdm = NULL;
+        av_buffer_unref(&buf);
+        return AVERROR(ENOMEM);
+    }
+
+    return 0;
 }
 
-int ff_decode_content_light_new(const AVCodecContext *avctx, AVFrame *frame,
-                                AVContentLightMetadata **clm)
+int ff_decode_content_light_new(const AVCodecContext *avctx, AVFrameSideData ***sd,
+                                int *nb_sd, AVContentLightMetadata **clm)
 {
-    if (side_data_pref(avctx, frame, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL)) {
+    AVBufferRef *buf;
+    size_t size;
+
+    if (side_data_pref(avctx, sd, nb_sd, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL)) {
         *clm = NULL;
         return 0;
     }
 
-    *clm = av_content_light_metadata_create_side_data(frame);
-    return *clm ? 0 : AVERROR(ENOMEM);
+    *clm = av_content_light_metadata_alloc(&size);
+    if (!*clm)
+        return AVERROR(ENOMEM);
+
+    buf = av_buffer_create((uint8_t *)*clm, size, NULL, NULL, 0);
+    if (!buf) {
+        av_freep(clm);
+        return AVERROR(ENOMEM);
+    }
+
+    if (!av_frame_side_data_add(sd, nb_sd, AV_FRAME_DATA_CONTENT_LIGHT_LEVEL,
+                                &buf, 0)) {
+        *clm = NULL;
+        av_buffer_unref(&buf);
+        return AVERROR(ENOMEM);
+    }
+
+    return 0;
 }
 
 int ff_copy_palette(void *dst, const AVPacket *src, void *logctx)
diff --git a/libavcodec/decode.h b/libavcodec/decode.h
index 4ffbd9db8e..1026c9dae9 100644
--- a/libavcodec/decode.h
+++ b/libavcodec/decode.h
@@ -156,14 +156,14 @@ const AVPacketSideData *ff_get_coded_side_data(const AVCodecContext *avctx,
                                                enum AVPacketSideDataType type);
 
 /**
- * Wrapper around av_frame_new_side_data, which rejects side data overridden by
+ * Wrapper around av_frame_side_data_new, which rejects side data overridden by
  * the demuxer. Returns 0 on success, and a negative error code otherwise.
- * If successful and sd is not NULL, *sd may either contain a pointer to the new
+ * If successful and psd is not NULL, *psd may either contain a pointer to the new
  * side data, or NULL in case the side data was already present.
  */
-int ff_frame_new_side_data(const AVCodecContext *avctx, AVFrame *frame,
-                           enum AVFrameSideDataType type, size_t size,
-                           AVFrameSideData **sd);
+int ff_frame_new_side_data(const AVCodecContext *avctx, AVFrameSideData ***sd,
+                           int *nb_sd, enum AVFrameSideDataType type, size_t size,
+                           AVFrameSideData **psd);
 
 /**
  * Similar to `ff_frame_new_side_data`, but using an existing buffer ref.
@@ -171,29 +171,29 @@ int ff_frame_new_side_data(const AVCodecContext *avctx, AVFrame *frame,
  * *buf is ALWAYS consumed by this function and NULL written in its place, even
  * on failure.
  */
-int ff_frame_new_side_data_from_buf(const AVCodecContext *avctx,
-                                    AVFrame *frame, enum AVFrameSideDataType type,
-                                    AVBufferRef **buf, AVFrameSideData **sd);
+int ff_frame_new_side_data_from_buf(const AVCodecContext *avctx, AVFrameSideData ***sd,
+                                    int *nb_sd, enum AVFrameSideDataType type,
+                                    AVBufferRef **buf);
 
 struct AVMasteringDisplayMetadata;
 struct AVContentLightMetadata;
 
 /**
- * Wrapper around av_mastering_display_metadata_create_side_data(), which
+ * Similar to av_mastering_display_metadata_create_side_data(), but
  * rejects side data overridden by the demuxer. Returns 0 on success, and a
  * negative error code otherwise. If successful, *mdm may either be a pointer to
  * the new side data, or NULL in case the side data was already present.
  */
-int ff_decode_mastering_display_new(const AVCodecContext *avctx, AVFrame *frame,
-                                    struct AVMasteringDisplayMetadata **mdm);
+int ff_decode_mastering_display_new(const AVCodecContext *avctx, AVFrameSideData ***sd,
+                                    int *nb_sd, struct AVMasteringDisplayMetadata **mdm);
 
 /**
- * Wrapper around av_content_light_metadata_create_side_data(), which
+ * Similar to av_content_light_metadata_create_side_data(), but
  * rejects side data overridden by the demuxer. Returns 0 on success, and a
  * negative error code otherwise. If successful, *clm may either be a pointer to
  * the new side data, or NULL in case the side data was already present.
  */
-int ff_decode_content_light_new(const AVCodecContext *avctx, AVFrame *frame,
-                                struct AVContentLightMetadata **clm);
+int ff_decode_content_light_new(const AVCodecContext *avctx, AVFrameSideData ***sd,
+                                int *nb_sd, struct AVContentLightMetadata **clm);
 
 #endif /* AVCODEC_DECODE_H */
diff --git a/libavcodec/dpx.c b/libavcodec/dpx.c
index 80616d98a2..ff9115bf86 100644
--- a/libavcodec/dpx.c
+++ b/libavcodec/dpx.c
@@ -288,7 +288,8 @@ static int decode_frame(AVCodecContext *avctx, AVFrame *p,
 
         if (i != 0xFFFFFFFF) {
             AVFrameSideData *tcside;
-            ret = ff_frame_new_side_data(avctx, p, AV_FRAME_DATA_S12M_TIMECODE,
+            ret = ff_frame_new_side_data(avctx, &p->side_data, &p->nb_side_data,
+                                         AV_FRAME_DATA_S12M_TIMECODE,
                                          sizeof(uint32_t) * 4, &tcside);
             if (ret < 0)
                 return ret;
diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
index afc103b69c..a9f4e01de8 100644
--- a/libavcodec/h2645_sei.c
+++ b/libavcodec/h2645_sei.c
@@ -750,7 +750,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
         int i;
         AVMasteringDisplayMetadata *metadata;
 
-        ret = ff_decode_mastering_display_new(avctx, frame, &metadata);
+        ret = ff_decode_mastering_display_new(avctx, &frame->side_data, &frame->nb_side_data, &metadata);
         if (ret < 0)
             return ret;
 
@@ -793,7 +793,7 @@ FF_ENABLE_DEPRECATION_WARNINGS
     if (sei->content_light.present) {
         AVContentLightMetadata *metadata;
 
-        ret = ff_decode_content_light_new(avctx, frame, &metadata);
+        ret = ff_decode_content_light_new(avctx, &frame->side_data, &frame->nb_side_data, &metadata);
         if (ret < 0)
             return ret;
 
diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c
index e9a404e41b..6d9d2ddc14 100644
--- a/libavcodec/h264_slice.c
+++ b/libavcodec/h264_slice.c
@@ -1253,7 +1253,8 @@ static int h264_export_frame_props(H264Context *h)
         uint32_t *tc_sd;
         char tcbuf[AV_TIMECODE_STR_SIZE];
         AVFrameSideData *tcside;
-        ret = ff_frame_new_side_data(h->avctx, out, AV_FRAME_DATA_S12M_TIMECODE,
+        ret = ff_frame_new_side_data(h->avctx, &out->side_data, &out->nb_side_data,
+                                     AV_FRAME_DATA_S12M_TIMECODE,
                                      sizeof(uint32_t)*4, &tcside);
         if (ret < 0)
             return ret;
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index 575836e340..d744e6f598 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -2791,7 +2791,8 @@ static int set_side_data(HEVCContext *s)
         uint32_t *tc_sd;
         char tcbuf[AV_TIMECODE_STR_SIZE];
         AVFrameSideData *tcside;
-        ret = ff_frame_new_side_data(s->avctx, out, AV_FRAME_DATA_S12M_TIMECODE,
+        ret = ff_frame_new_side_data(s->avctx, &out->side_data, &out->nb_side_data,
+                                     AV_FRAME_DATA_S12M_TIMECODE,
                                      sizeof(uint32_t) * 4, &tcside);
         if (ret < 0)
             return ret;
@@ -2821,7 +2822,8 @@ static int set_side_data(HEVCContext *s)
         if (!info_ref)
             return AVERROR(ENOMEM);
 
-        ret = ff_frame_new_side_data_from_buf(s->avctx, out, AV_FRAME_DATA_DYNAMIC_HDR_PLUS, &info_ref, NULL);
+        ret = ff_frame_new_side_data_from_buf(s->avctx, &out->side_data, &out->nb_side_data,
+                                              AV_FRAME_DATA_DYNAMIC_HDR_PLUS, &info_ref);
         if (ret < 0)
             return ret;
     }
diff --git a/libavcodec/libdav1d.c b/libavcodec/libdav1d.c
index ddcd0708b4..465c56e757 100644
--- a/libavcodec/libdav1d.c
+++ b/libavcodec/libdav1d.c
@@ -471,7 +471,7 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
     if (p->mastering_display) {
         AVMasteringDisplayMetadata *mastering;
 
-        res = ff_decode_mastering_display_new(c, frame, &mastering);
+        res = ff_decode_mastering_display_new(c, &frame->side_data, &frame->nb_side_data, &mastering);
         if (res < 0)
             goto fail;
 
@@ -493,7 +493,7 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
     if (p->content_light) {
         AVContentLightMetadata *light;
 
-        res = ff_decode_content_light_new(c, frame, &light);
+        res = ff_decode_content_light_new(c, &frame->side_data, &frame->nb_side_data, &light);
         if (res < 0)
             goto fail;
 
@@ -528,7 +528,8 @@ static int libdav1d_receive_frame(AVCodecContext *c, AVFrame *frame)
                 if (!res)
                     break;
 
-                res = ff_frame_new_side_data_from_buf(c, frame, AV_FRAME_DATA_A53_CC, &buf, NULL);
+                res = ff_frame_new_side_data_from_buf(c, &frame->side_data, &frame->nb_side_data,
+                                                      AV_FRAME_DATA_A53_CC, &buf);
                 if (res < 0)
                     goto fail;
 
diff --git a/libavcodec/libjxldec.c b/libavcodec/libjxldec.c
index d57a27418f..6e95c79177 100644
--- a/libavcodec/libjxldec.c
+++ b/libavcodec/libjxldec.c
@@ -483,7 +483,8 @@ static int libjxl_receive_frame(AVCodecContext *avctx, AVFrame *frame)
             /* full image is one frame, even if animated */
             av_log(avctx, AV_LOG_DEBUG, "FULL_IMAGE event emitted\n");
             if (ctx->iccp) {
-                ret = ff_frame_new_side_data_from_buf(avctx, ctx->frame, AV_FRAME_DATA_ICC_PROFILE, &ctx->iccp, NULL);
+                ret = ff_frame_new_side_data_from_buf(avctx, &ctx->frame->side_data, &ctx->frame->nb_side_data,
+                                                      AV_FRAME_DATA_ICC_PROFILE, &ctx->iccp);
                 if (ret < 0)
                     return ret;
             }
diff --git a/libavcodec/mjpegdec.c b/libavcodec/mjpegdec.c
index c9409eac6c..89443dc4cd 100644
--- a/libavcodec/mjpegdec.c
+++ b/libavcodec/mjpegdec.c
@@ -2839,7 +2839,8 @@ the_end:
         for (i = 0; i < s->iccnum; i++)
             total_size += s->iccentries[i].length;
 
-        ret = ff_frame_new_side_data(avctx, frame, AV_FRAME_DATA_ICC_PROFILE, total_size, &sd);
+        ret = ff_frame_new_side_data(avctx, &frame->side_data, &frame->nb_side_data,
+                                     AV_FRAME_DATA_ICC_PROFILE, total_size, &sd);
         if (ret < 0) {
             av_log(avctx, AV_LOG_ERROR, "Could not allocate frame side data\n");
             return ret;
diff --git a/libavcodec/mpeg12dec.c b/libavcodec/mpeg12dec.c
index 4ad1eb6572..2665ee2f95 100644
--- a/libavcodec/mpeg12dec.c
+++ b/libavcodec/mpeg12dec.c
@@ -1314,7 +1314,8 @@ static int mpeg_field_start(MpegEncContext *s, const uint8_t *buf, int buf_size)
             }
         }
 
-        ret = ff_frame_new_side_data(s->avctx, s->current_picture_ptr->f,
+        ret = ff_frame_new_side_data(s->avctx, &s->current_picture_ptr->f->side_data,
+                                     &s->current_picture_ptr->f->nb_side_data,
                                      AV_FRAME_DATA_PANSCAN, sizeof(s1->pan_scan),
                                      &pan_scan);
         if (ret < 0)
@@ -1324,8 +1325,9 @@ static int mpeg_field_start(MpegEncContext *s, const uint8_t *buf, int buf_size)
 
         if (s1->a53_buf_ref) {
             ret = ff_frame_new_side_data_from_buf(
-                s->avctx, s->current_picture_ptr->f, AV_FRAME_DATA_A53_CC,
-                &s1->a53_buf_ref, NULL);
+                 s->avctx, &s->current_picture_ptr->f->side_data,
+                 &s->current_picture_ptr->f->nb_side_data,
+                 AV_FRAME_DATA_A53_CC, &s1->a53_buf_ref);
             if (ret < 0)
                 return ret;
         }
@@ -1341,7 +1343,8 @@ static int mpeg_field_start(MpegEncContext *s, const uint8_t *buf, int buf_size)
 
         if (s1->has_afd) {
             AVFrameSideData *sd;
-            ret = ff_frame_new_side_data(s->avctx, s->current_picture_ptr->f,
+            ret = ff_frame_new_side_data(s->avctx, &s->current_picture_ptr->f->side_data,
+                                         &s->current_picture_ptr->f->nb_side_data,
                                          AV_FRAME_DATA_AFD, 1, &sd);
             if (ret < 0)
                 return ret;
diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
index de50e6a5b6..68c25cb5bd 100644
--- a/libavcodec/pngdec.c
+++ b/libavcodec/pngdec.c
@@ -680,8 +680,8 @@ static int populate_avctx_color_fields(AVCodecContext *avctx, AVFrame *frame)
         }
     } else if (s->iccp_data) {
         AVFrameSideData *sd;
-        ret = ff_frame_new_side_data(avctx, frame, AV_FRAME_DATA_ICC_PROFILE,
-                                     s->iccp_data_len, &sd);
+        ret = ff_frame_new_side_data(avctx, &frame->side_data, &frame->nb_side_data,
+                                     AV_FRAME_DATA_ICC_PROFILE, s->iccp_data_len, &sd);
         if (ret < 0)
             return ret;
         if (sd) {
@@ -748,7 +748,7 @@ static int populate_avctx_color_fields(AVCodecContext *avctx, AVFrame *frame)
     if (s->have_clli) {
         AVContentLightMetadata *clli;
 
-        ret = ff_decode_content_light_new(avctx, frame, &clli);
+        ret = ff_decode_content_light_new(avctx, &frame->side_data, &frame->nb_side_data, &clli);
         if (ret < 0)
             return ret;
 
@@ -765,7 +765,7 @@ static int populate_avctx_color_fields(AVCodecContext *avctx, AVFrame *frame)
     if (s->have_mdvc) {
         AVMasteringDisplayMetadata *mdvc;
 
-        ret = ff_decode_mastering_display_new(avctx, frame, &mdvc);
+        ret = ff_decode_mastering_display_new(avctx, &frame->side_data, &frame->nb_side_data, &mdvc);
         if (ret < 0)
             return ret;
 
diff --git a/libavcodec/qsvdec.c b/libavcodec/qsvdec.c
index fd9267c6f4..de3ed3c357 100644
--- a/libavcodec/qsvdec.c
+++ b/libavcodec/qsvdec.c
@@ -661,7 +661,7 @@ static int qsv_export_hdr_side_data(AVCodecContext *avctx, mfxExtMasteringDispla
         const int luma_den = 10000;
         int i;
 
-        ret = ff_decode_mastering_display_new(avctx, frame, &mastering);
+        ret = ff_decode_mastering_display_new(avctx, &frame->side_data, &frame->nb_side_data, &mastering);
         if (ret < 0)
             return ret;
 
@@ -687,7 +687,7 @@ static int qsv_export_hdr_side_data(AVCodecContext *avctx, mfxExtMasteringDispla
     if (clli->InsertPayloadToggle) {
         AVContentLightMetadata *light;
 
-        ret = ff_decode_content_light_new(avctx, frame, &light);
+        ret = ff_decode_content_light_new(avctx, &frame->side_data, &frame->nb_side_data, &light);
         if (ret < 0)
             return ret;
 
diff --git a/libavcodec/tiff.c b/libavcodec/tiff.c
index 7ce1ab32f6..9cde2e60e1 100644
--- a/libavcodec/tiff.c
+++ b/libavcodec/tiff.c
@@ -1706,7 +1706,8 @@ static int tiff_decode_tag(TiffContext *s, AVFrame *frame)
         if (bytestream2_get_bytes_left(&gb_temp) < count)
             return AVERROR_INVALIDDATA;
 
-        ret = ff_frame_new_side_data(s->avctx, frame, AV_FRAME_DATA_ICC_PROFILE, count, &sd);
+        ret = ff_frame_new_side_data(s->avctx, &frame->side_data, &frame->nb_side_data,
+                                     AV_FRAME_DATA_ICC_PROFILE, count, &sd);
         if (ret < 0)
             return ret;
         if (sd)
diff --git a/libavcodec/webp.c b/libavcodec/webp.c
index 9308ea2b69..ec23533ef2 100644
--- a/libavcodec/webp.c
+++ b/libavcodec/webp.c
@@ -1501,7 +1501,8 @@ exif_end:
 
             s->has_iccp = 1;
 
-            ret = ff_frame_new_side_data(avctx, p, AV_FRAME_DATA_ICC_PROFILE, chunk_size, &sd);
+            ret = ff_frame_new_side_data(avctx, &p->side_data, &p->nb_side_data,
+                                         AV_FRAME_DATA_ICC_PROFILE, chunk_size, &sd);
             if (ret < 0)
                 return ret;
 
-- 
2.44.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 6/7 v4] avcodec/decode: make the AVFrameSideData helper wrappers not depend on frames

[Anton Khirnov]

Quoting James Almer (2024-03-28 04:12:09)
> They will be useful to fill arrays stored in other structs.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/av1dec.c     |  7 +--
>  libavcodec/cri.c        |  3 +-
>  libavcodec/decode.c     | 99 +++++++++++++++++++++++++++--------------
>  libavcodec/decode.h     | 28 ++++++------
>  libavcodec/dpx.c        |  3 +-
>  libavcodec/h2645_sei.c  |  4 +-
>  libavcodec/h264_slice.c |  3 +-
>  libavcodec/hevcdec.c    |  6 ++-
>  libavcodec/libdav1d.c   |  7 +--
>  libavcodec/libjxldec.c  |  3 +-
>  libavcodec/mjpegdec.c   |  3 +-
>  libavcodec/mpeg12dec.c  | 11 +++--
>  libavcodec/pngdec.c     |  8 ++--
>  libavcodec/qsvdec.c     |  4 +-
>  libavcodec/tiff.c       |  3 +-
>  libavcodec/webp.c       |  3 +-
>  16 files changed, 120 insertions(+), 75 deletions(-)

Extra churn in all the decoders, longer and harder to read lines.
Why not make ff_frame_new_side_data_from_buf() a wrapper for a new
function instead?

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 6/7 v4] avcodec/decode: make the AVFrameSideData helper wrappers not depend on frames

[James Almer]

On 3/28/2024 8:32 AM, Anton Khirnov wrote:
> Quoting James Almer (2024-03-28 04:12:09)
>> They will be useful to fill arrays stored in other structs.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavcodec/av1dec.c     |  7 +--
>>   libavcodec/cri.c        |  3 +-
>>   libavcodec/decode.c     | 99 +++++++++++++++++++++++++++--------------
>>   libavcodec/decode.h     | 28 ++++++------
>>   libavcodec/dpx.c        |  3 +-
>>   libavcodec/h2645_sei.c  |  4 +-
>>   libavcodec/h264_slice.c |  3 +-
>>   libavcodec/hevcdec.c    |  6 ++-
>>   libavcodec/libdav1d.c   |  7 +--
>>   libavcodec/libjxldec.c  |  3 +-
>>   libavcodec/mjpegdec.c   |  3 +-
>>   libavcodec/mpeg12dec.c  | 11 +++--
>>   libavcodec/pngdec.c     |  8 ++--
>>   libavcodec/qsvdec.c     |  4 +-
>>   libavcodec/tiff.c       |  3 +-
>>   libavcodec/webp.c       |  3 +-
>>   16 files changed, 120 insertions(+), 75 deletions(-)
> 
> Extra churn in all the decoders, longer and harder to read lines.
> Why not make ff_frame_new_side_data_from_buf() a wrapper for a new
> function instead?

Can you elaborate? I'm making all the decode.h side data wrappers take 
pointers to AVFrameSideData instead of AVFrames so they're not limited 
to the latter.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 6/7 v4] avcodec/decode: make the AVFrameSideData helper wrappers not depend on frames

[Anton Khirnov]

Quoting James Almer (2024-03-28 12:36:37)
> On 3/28/2024 8:32 AM, Anton Khirnov wrote:
> > Quoting James Almer (2024-03-28 04:12:09)
> >> They will be useful to fill arrays stored in other structs.
> >>
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >>   libavcodec/av1dec.c     |  7 +--
> >>   libavcodec/cri.c        |  3 +-
> >>   libavcodec/decode.c     | 99 +++++++++++++++++++++++++++--------------
> >>   libavcodec/decode.h     | 28 ++++++------
> >>   libavcodec/dpx.c        |  3 +-
> >>   libavcodec/h2645_sei.c  |  4 +-
> >>   libavcodec/h264_slice.c |  3 +-
> >>   libavcodec/hevcdec.c    |  6 ++-
> >>   libavcodec/libdav1d.c   |  7 +--
> >>   libavcodec/libjxldec.c  |  3 +-
> >>   libavcodec/mjpegdec.c   |  3 +-
> >>   libavcodec/mpeg12dec.c  | 11 +++--
> >>   libavcodec/pngdec.c     |  8 ++--
> >>   libavcodec/qsvdec.c     |  4 +-
> >>   libavcodec/tiff.c       |  3 +-
> >>   libavcodec/webp.c       |  3 +-
> >>   16 files changed, 120 insertions(+), 75 deletions(-)
> > 
> > Extra churn in all the decoders, longer and harder to read lines.
> > Why not make ff_frame_new_side_data_from_buf() a wrapper for a new
> > function instead?
> 
> Can you elaborate? I'm making all the decode.h side data wrappers take 
> pointers to AVFrameSideData instead of AVFrames so they're not limited 
> to the latter.

I mean:
* add new function(s) that work with AVFrameSideData;
* keep signatures for existing function the same, but change their
  implementation into a wrapper for the above

this way the decoders do not need to be changed.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 7/7 v4] avcodec/hevcdec: export global side data in AVCodecContext

[James Almer]

Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/avcodec.h       |   2 +-
 libavcodec/h2645_sei.c     | 215 +++++++++++++++++++++----------------
 libavcodec/h2645_sei.h     |   2 +
 libavcodec/hevcdec.c       |   4 +
 libavcodec/pthread_frame.c |  11 ++
 5 files changed, 141 insertions(+), 93 deletions(-)

diff --git a/libavcodec/avcodec.h b/libavcodec/avcodec.h
index 83dc487251..968009a192 100644
--- a/libavcodec/avcodec.h
+++ b/libavcodec/avcodec.h
@@ -2071,7 +2071,7 @@ typedef struct AVCodecContext {
      * - encoding: may be set by user before calling avcodec_open2() for
      *             encoder configuration. Afterwards owned and freed by the
      *             encoder.
-     * - decoding: unused
+     * - decoding: may be set by libavcodec in avcodec_open2().
      */
     AVFrameSideData  **decoded_side_data;
     int             nb_decoded_side_data;
diff --git a/libavcodec/h2645_sei.c b/libavcodec/h2645_sei.c
index a9f4e01de8..4b7ce78a38 100644
--- a/libavcodec/h2645_sei.c
+++ b/libavcodec/h2645_sei.c
@@ -529,6 +529,120 @@ static int is_frame_packing_type_valid(SEIFpaType type, enum AVCodecID codec_id)
                type >= SEI_FPA_TYPE_SIDE_BY_SIDE;
 }
 
+static int h2645_sei_to_side_data(AVCodecContext *avctx, H2645SEI *sei,
+                                  AVFrameSideData ***sd, int *nb_sd)
+{
+    int ret;
+
+    for (unsigned i = 0; i < sei->unregistered.nb_buf_ref; i++) {
+        H2645SEIUnregistered *unreg = &sei->unregistered;
+
+        if (unreg->buf_ref[i]) {
+            AVFrameSideData *entry = av_frame_side_data_add(sd, nb_sd,
+                    AV_FRAME_DATA_SEI_UNREGISTERED,
+                    &unreg->buf_ref[i], 0);
+            if (!entry)
+                av_buffer_unref(&unreg->buf_ref[i]);
+        }
+    }
+    sei->unregistered.nb_buf_ref = 0;
+
+    if (sei->ambient_viewing_environment.present) {
+        H2645SEIAmbientViewingEnvironment *env =
+            &sei->ambient_viewing_environment;
+        AVBufferRef *buf;
+        size_t size;
+
+        AVAmbientViewingEnvironment *dst_env =
+            av_ambient_viewing_environment_alloc(&size);
+        if (!dst_env)
+            return AVERROR(ENOMEM);
+
+        buf = av_buffer_create((uint8_t *)dst_env, size, NULL, NULL, 0);
+        if (!buf) {
+            av_free(dst_env);
+            return AVERROR(ENOMEM);
+        }
+
+        ret = ff_frame_new_side_data_from_buf(avctx, sd, nb_sd,
+                                              AV_FRAME_DATA_AMBIENT_VIEWING_ENVIRONMENT, &buf);
+
+        if (ret < 0)
+            return ret;
+
+        dst_env->ambient_illuminance = av_make_q(env->ambient_illuminance, 10000);
+        dst_env->ambient_light_x     = av_make_q(env->ambient_light_x,     50000);
+        dst_env->ambient_light_y     = av_make_q(env->ambient_light_y,     50000);
+    }
+
+    if (sei->mastering_display.present) {
+        // HEVC uses a g,b,r ordering, which we convert to a more natural r,g,b
+        const int mapping[3] = {2, 0, 1};
+        const int chroma_den = 50000;
+        const int luma_den = 10000;
+        int i;
+        AVMasteringDisplayMetadata *metadata;
+
+        ret = ff_decode_mastering_display_new(avctx, sd, nb_sd, &metadata);
+        if (ret < 0)
+            return ret;
+
+        if (metadata) {
+            for (i = 0; i < 3; i++) {
+                const int j = mapping[i];
+                metadata->display_primaries[i][0].num = sei->mastering_display.display_primaries[j][0];
+                metadata->display_primaries[i][0].den = chroma_den;
+                metadata->display_primaries[i][1].num = sei->mastering_display.display_primaries[j][1];
+                metadata->display_primaries[i][1].den = chroma_den;
+            }
+            metadata->white_point[0].num = sei->mastering_display.white_point[0];
+            metadata->white_point[0].den = chroma_den;
+            metadata->white_point[1].num = sei->mastering_display.white_point[1];
+            metadata->white_point[1].den = chroma_den;
+
+            metadata->max_luminance.num = sei->mastering_display.max_luminance;
+            metadata->max_luminance.den = luma_den;
+            metadata->min_luminance.num = sei->mastering_display.min_luminance;
+            metadata->min_luminance.den = luma_den;
+            metadata->has_luminance = 1;
+            metadata->has_primaries = 1;
+
+            av_log(avctx, AV_LOG_DEBUG, "Mastering Display Metadata:\n");
+            av_log(avctx, AV_LOG_DEBUG,
+                   "r(%5.4f,%5.4f) g(%5.4f,%5.4f) b(%5.4f %5.4f) wp(%5.4f, %5.4f)\n",
+                   av_q2d(metadata->display_primaries[0][0]),
+                   av_q2d(metadata->display_primaries[0][1]),
+                   av_q2d(metadata->display_primaries[1][0]),
+                   av_q2d(metadata->display_primaries[1][1]),
+                   av_q2d(metadata->display_primaries[2][0]),
+                   av_q2d(metadata->display_primaries[2][1]),
+                   av_q2d(metadata->white_point[0]), av_q2d(metadata->white_point[1]));
+            av_log(avctx, AV_LOG_DEBUG,
+                   "min_luminance=%f, max_luminance=%f\n",
+                   av_q2d(metadata->min_luminance), av_q2d(metadata->max_luminance));
+        }
+    }
+
+    if (sei->content_light.present) {
+        AVContentLightMetadata *metadata;
+
+        ret = ff_decode_content_light_new(avctx, sd, nb_sd, &metadata);
+        if (ret < 0)
+            return ret;
+
+        if (metadata) {
+            metadata->MaxCLL  = sei->content_light.max_content_light_level;
+            metadata->MaxFALL = sei->content_light.max_pic_average_light_level;
+
+            av_log(avctx, AV_LOG_DEBUG, "Content Light Level Metadata:\n");
+            av_log(avctx, AV_LOG_DEBUG, "MaxCLL=%d, MaxFALL=%d\n",
+                   metadata->MaxCLL, metadata->MaxFALL);
+        }
+    }
+
+    return 0;
+}
+
 int ff_h2645_sei_to_frame(AVFrame *frame, H2645SEI *sei,
                           enum AVCodecID codec_id,
                           AVCodecContext *avctx, const H2645VUI *vui,
@@ -625,19 +739,9 @@ int ff_h2645_sei_to_frame(AVFrame *frame, H2645SEI *sei,
             avctx->properties |= FF_CODEC_PROPERTY_CLOSED_CAPTIONS;
     }
 
-    for (unsigned i = 0; i < sei->unregistered.nb_buf_ref; i++) {
-        H2645SEIUnregistered *unreg = &sei->unregistered;
-
-        if (unreg->buf_ref[i]) {
-            AVFrameSideData *sd = av_frame_new_side_data_from_buf(frame,
-                    AV_FRAME_DATA_SEI_UNREGISTERED,
-                    unreg->buf_ref[i]);
-            if (!sd)
-                av_buffer_unref(&unreg->buf_ref[i]);
-            unreg->buf_ref[i] = NULL;
-        }
-    }
-    sei->unregistered.nb_buf_ref = 0;
+    ret = h2645_sei_to_side_data(avctx, sei, &frame->side_data, &frame->nb_side_data);
+    if (ret < 0)
+        return ret;
 
     if (sei->afd.present) {
         AVFrameSideData *sd = av_frame_new_side_data(frame, AV_FRAME_DATA_AFD,
@@ -728,88 +832,15 @@ FF_ENABLE_DEPRECATION_WARNINGS
         return ret;
 #endif
 
-    if (sei->ambient_viewing_environment.present) {
-        H2645SEIAmbientViewingEnvironment *env =
-            &sei->ambient_viewing_environment;
-
-        AVAmbientViewingEnvironment *dst_env =
-            av_ambient_viewing_environment_create_side_data(frame);
-        if (!dst_env)
-            return AVERROR(ENOMEM);
-
-        dst_env->ambient_illuminance = av_make_q(env->ambient_illuminance, 10000);
-        dst_env->ambient_light_x     = av_make_q(env->ambient_light_x,     50000);
-        dst_env->ambient_light_y     = av_make_q(env->ambient_light_y,     50000);
-    }
-
-    if (sei->mastering_display.present) {
-        // HEVC uses a g,b,r ordering, which we convert to a more natural r,g,b
-        const int mapping[3] = {2, 0, 1};
-        const int chroma_den = 50000;
-        const int luma_den = 10000;
-        int i;
-        AVMasteringDisplayMetadata *metadata;
-
-        ret = ff_decode_mastering_display_new(avctx, &frame->side_data, &frame->nb_side_data, &metadata);
-        if (ret < 0)
-            return ret;
-
-        if (metadata) {
-            for (i = 0; i < 3; i++) {
-                const int j = mapping[i];
-                metadata->display_primaries[i][0].num = sei->mastering_display.display_primaries[j][0];
-                metadata->display_primaries[i][0].den = chroma_den;
-                metadata->display_primaries[i][1].num = sei->mastering_display.display_primaries[j][1];
-                metadata->display_primaries[i][1].den = chroma_den;
-            }
-            metadata->white_point[0].num = sei->mastering_display.white_point[0];
-            metadata->white_point[0].den = chroma_den;
-            metadata->white_point[1].num = sei->mastering_display.white_point[1];
-            metadata->white_point[1].den = chroma_den;
-
-            metadata->max_luminance.num = sei->mastering_display.max_luminance;
-            metadata->max_luminance.den = luma_den;
-            metadata->min_luminance.num = sei->mastering_display.min_luminance;
-            metadata->min_luminance.den = luma_den;
-            metadata->has_luminance = 1;
-            metadata->has_primaries = 1;
-
-            av_log(avctx, AV_LOG_DEBUG, "Mastering Display Metadata:\n");
-            av_log(avctx, AV_LOG_DEBUG,
-                   "r(%5.4f,%5.4f) g(%5.4f,%5.4f) b(%5.4f %5.4f) wp(%5.4f, %5.4f)\n",
-                   av_q2d(metadata->display_primaries[0][0]),
-                   av_q2d(metadata->display_primaries[0][1]),
-                   av_q2d(metadata->display_primaries[1][0]),
-                   av_q2d(metadata->display_primaries[1][1]),
-                   av_q2d(metadata->display_primaries[2][0]),
-                   av_q2d(metadata->display_primaries[2][1]),
-                   av_q2d(metadata->white_point[0]), av_q2d(metadata->white_point[1]));
-            av_log(avctx, AV_LOG_DEBUG,
-                   "min_luminance=%f, max_luminance=%f\n",
-                   av_q2d(metadata->min_luminance), av_q2d(metadata->max_luminance));
-        }
-    }
-
-    if (sei->content_light.present) {
-        AVContentLightMetadata *metadata;
-
-        ret = ff_decode_content_light_new(avctx, &frame->side_data, &frame->nb_side_data, &metadata);
-        if (ret < 0)
-            return ret;
-
-        if (metadata) {
-            metadata->MaxCLL  = sei->content_light.max_content_light_level;
-            metadata->MaxFALL = sei->content_light.max_pic_average_light_level;
-
-            av_log(avctx, AV_LOG_DEBUG, "Content Light Level Metadata:\n");
-            av_log(avctx, AV_LOG_DEBUG, "MaxCLL=%d, MaxFALL=%d\n",
-                   metadata->MaxCLL, metadata->MaxFALL);
-        }
-    }
-
     return 0;
 }
 
+int ff_h2645_sei_to_context(AVCodecContext *avctx, H2645SEI *sei)
+{
+    return h2645_sei_to_side_data(avctx, sei, &avctx->decoded_side_data,
+                                  &avctx->nb_decoded_side_data);
+}
+
 void ff_h2645_sei_reset(H2645SEI *s)
 {
     av_buffer_unref(&s->a53_caption.buf_ref);
diff --git a/libavcodec/h2645_sei.h b/libavcodec/h2645_sei.h
index b9a6c7587b..488dbcad7e 100644
--- a/libavcodec/h2645_sei.h
+++ b/libavcodec/h2645_sei.h
@@ -168,4 +168,6 @@ int ff_h2645_sei_to_frame(AVFrame *frame, H2645SEI *sei,
                           unsigned bit_depth_luma, unsigned bit_depth_chroma,
                           int seed);
 
+int ff_h2645_sei_to_context(AVCodecContext *avctx, H2645SEI *sei);
+
 #endif /* AVCODEC_H2645_SEI_H */
diff --git a/libavcodec/hevcdec.c b/libavcodec/hevcdec.c
index d744e6f598..70d22e5976 100644
--- a/libavcodec/hevcdec.c
+++ b/libavcodec/hevcdec.c
@@ -3658,6 +3658,10 @@ static av_cold int hevc_decode_init(AVCodecContext *avctx)
             if (ret < 0) {
                 return ret;
             }
+
+            ret = ff_h2645_sei_to_context(avctx, &s->sei.common);
+            if (ret < 0)
+                return ret;
         }
 
         sd = ff_get_coded_side_data(avctx, AV_PKT_DATA_DOVI_CONF);
diff --git a/libavcodec/pthread_frame.c b/libavcodec/pthread_frame.c
index fd356bd190..a693e12670 100644
--- a/libavcodec/pthread_frame.c
+++ b/libavcodec/pthread_frame.c
@@ -334,6 +334,15 @@ FF_ENABLE_DEPRECATION_WARNINGS
     if (for_user) {
         if (codec->update_thread_context_for_user)
             err = codec->update_thread_context_for_user(dst, src);
+
+        av_frame_side_data_free(&dst->decoded_side_data, &dst->nb_decoded_side_data);
+        for (int i = 0; i < src->nb_decoded_side_data; i++) {
+            int ret = av_frame_side_data_clone(&dst->decoded_side_data,
+                                               &dst->nb_decoded_side_data,
+                                               src->decoded_side_data[i], 0);
+            if (ret < 0)
+                return ret;
+        }
     } else {
         const PerThreadContext *p_src = src->internal->thread_ctx;
         PerThreadContext       *p_dst = dst->internal->thread_ctx;
@@ -767,6 +776,8 @@ static av_cold int init_thread(PerThreadContext *p, int *threads_to_free,
     if (!copy)
         return AVERROR(ENOMEM);
     copy->priv_data = NULL;
+    copy->decoded_side_data = NULL;
+    copy->nb_decoded_side_data = 0;
 
     /* From now on, this PerThreadContext will be cleaned up by
      * ff_frame_thread_free in case of errors. */
-- 
2.44.0

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/7 v4] avutil/frame: add a flag to allow overwritting existing entries

[Anton Khirnov]

Quoting James Almer (2024-03-28 04:12:04)
> Enable it only for side data types that don't allow more than one entry.
> 
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavutil/frame.c                 | 59 ++++++++++++++++++++++++++++---
>  libavutil/frame.h                 | 27 +++++++++-----
>  libavutil/tests/side_data_array.c | 52 +++++++++++++++------------
>  tests/ref/fate/side_data_array    | 22 ++++++------
>  4 files changed, 115 insertions(+), 45 deletions(-)
> 
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index ef1613c344..d9bd19b2aa 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -799,12 +799,34 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
>                                          enum AVFrameSideDataType type,
>                                          size_t size, unsigned int flags)
>  {
> -    AVBufferRef     *buf = av_buffer_alloc(size);
> +    const AVSideDataDescriptor *desc = av_frame_side_data_desc(type);
> +    AVBufferRef     *buf;
>      AVFrameSideData *ret = NULL;
>  
>      if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
>          remove_side_data(sd, nb_sd, type);
> +    if (!desc || !(desc->props & AV_SIDE_DATA_PROP_MULTI)) {

desc == NULL means type is invalid

> +        for (int i = 0; i < *nb_sd; i++) {
> +            AVFrameSideData *entry = ((*sd)[i]);
> +            if (entry->type != type)
> +                continue;

Why are you not calling av_frame_side_data_get()?

> @@ -822,13 +845,41 @@ int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
>      if (!sd || !src || !nb_sd || (*nb_sd && !*sd))
>          return AVERROR(EINVAL);
>  
> +    desc = av_frame_side_data_desc(src->type);
> +    if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
> +        remove_side_data(sd, nb_sd, src->type);
> +    if (!desc || !(desc->props & AV_SIDE_DATA_PROP_MULTI)) {
> +        for (int i = 0; i < *nb_sd; i++) {
> +            AVFrameSideData *entry = ((*sd)[i]);
> +            AVDictionary *dict = NULL;
> +
> +            if (entry->type != src->type)
> +                continue;
> +            if (!(flags & AV_FRAME_SIDE_DATA_FLAG_REPLACE))
> +                return AVERROR(EEXIST);
> +
> +            ret = av_dict_copy(&dict, src->metadata, 0);
> +            if (ret < 0)
> +                return ret;
> +
> +            ret = av_buffer_replace(&entry->buf, src->buf);
> +            if (ret < 0) {
> +                av_dict_free(&dict);
> +                return ret;
> +            }
> +
> +            av_dict_free(&entry->metadata);
> +            entry->metadata = dict;
> +            entry->data     = src->data;
> +            entry->size     = src->size;
> +            return 0;
> +        }

This whole block looks very similar to the one you're adding to
av_frame_side_data_new().

> +    }
> +
>      buf = av_buffer_ref(src->buf);
>      if (!buf)
>          return AVERROR(ENOMEM);
>  
> -    if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
> -        remove_side_data(sd, nb_sd, src->type);
> -
>      sd_dst = add_side_data_from_buf(sd, nb_sd, src->type, buf);
>      if (!sd_dst) {
>          av_buffer_unref(&buf);
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 3b6d746a16..2ea129888e 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -1040,7 +1040,14 @@ const AVSideDataDescriptor *av_frame_side_data_desc(enum AVFrameSideDataType typ
>   */
>  void av_frame_side_data_free(AVFrameSideData ***sd, int *nb_sd);
>  
> +/**
> + * Remove existing entries before adding new ones.
> + */
>  #define AV_FRAME_SIDE_DATA_FLAG_UNIQUE (1 << 0)
> +/**
> + * Don't add a new entry if another of the same type exists.

This should mention that it only applies to MULTI side data types.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/7 v4] avutil/frame: add a flag to allow overwritting existing entries

[James Almer]

On 3/28/2024 8:23 AM, Anton Khirnov wrote:
> Quoting James Almer (2024-03-28 04:12:04)
>> Enable it only for side data types that don't allow more than one entry.
>>
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>   libavutil/frame.c                 | 59 ++++++++++++++++++++++++++++---
>>   libavutil/frame.h                 | 27 +++++++++-----
>>   libavutil/tests/side_data_array.c | 52 +++++++++++++++------------
>>   tests/ref/fate/side_data_array    | 22 ++++++------
>>   4 files changed, 115 insertions(+), 45 deletions(-)
>>
>> diff --git a/libavutil/frame.c b/libavutil/frame.c
>> index ef1613c344..d9bd19b2aa 100644
>> --- a/libavutil/frame.c
>> +++ b/libavutil/frame.c
>> @@ -799,12 +799,34 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
>>                                           enum AVFrameSideDataType type,
>>                                           size_t size, unsigned int flags)
>>   {
>> -    AVBufferRef     *buf = av_buffer_alloc(size);
>> +    const AVSideDataDescriptor *desc = av_frame_side_data_desc(type);
>> +    AVBufferRef     *buf;
>>       AVFrameSideData *ret = NULL;
>>   
>>       if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
>>           remove_side_data(sd, nb_sd, type);
>> +    if (!desc || !(desc->props & AV_SIDE_DATA_PROP_MULTI)) {
> 
> desc == NULL means type is invalid

This being a generic alloc function, it should IMO not be limited to 
currently defined types. And I chose to treat them as non multi, as 
that's the most common scenario.

> 
>> +        for (int i = 0; i < *nb_sd; i++) {
>> +            AVFrameSideData *entry = ((*sd)[i]);
>> +            if (entry->type != type)
>> +                continue;
> 
> Why are you not calling av_frame_side_data_get()?

An oversight i guess :p

> 
>> @@ -822,13 +845,41 @@ int av_frame_side_data_clone(AVFrameSideData ***sd, int *nb_sd,
>>       if (!sd || !src || !nb_sd || (*nb_sd && !*sd))
>>           return AVERROR(EINVAL);
>>   
>> +    desc = av_frame_side_data_desc(src->type);
>> +    if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
>> +        remove_side_data(sd, nb_sd, src->type);
>> +    if (!desc || !(desc->props & AV_SIDE_DATA_PROP_MULTI)) {
>> +        for (int i = 0; i < *nb_sd; i++) {
>> +            AVFrameSideData *entry = ((*sd)[i]);
>> +            AVDictionary *dict = NULL;
>> +
>> +            if (entry->type != src->type)
>> +                continue;
>> +            if (!(flags & AV_FRAME_SIDE_DATA_FLAG_REPLACE))
>> +                return AVERROR(EEXIST);
>> +
>> +            ret = av_dict_copy(&dict, src->metadata, 0);
>> +            if (ret < 0)
>> +                return ret;
>> +
>> +            ret = av_buffer_replace(&entry->buf, src->buf);
>> +            if (ret < 0) {
>> +                av_dict_free(&dict);
>> +                return ret;
>> +            }
>> +
>> +            av_dict_free(&entry->metadata);
>> +            entry->metadata = dict;
>> +            entry->data     = src->data;
>> +            entry->size     = src->size;
>> +            return 0;
>> +        }
> 
> This whole block looks very similar to the one you're adding to
> av_frame_side_data_new().

I can try to factorize it.

> 
>> +    }
>> +
>>       buf = av_buffer_ref(src->buf);
>>       if (!buf)
>>           return AVERROR(ENOMEM);
>>   
>> -    if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
>> -        remove_side_data(sd, nb_sd, src->type);
>> -
>>       sd_dst = add_side_data_from_buf(sd, nb_sd, src->type, buf);
>>       if (!sd_dst) {
>>           av_buffer_unref(&buf);
>> diff --git a/libavutil/frame.h b/libavutil/frame.h
>> index 3b6d746a16..2ea129888e 100644
>> --- a/libavutil/frame.h
>> +++ b/libavutil/frame.h
>> @@ -1040,7 +1040,14 @@ const AVSideDataDescriptor *av_frame_side_data_desc(enum AVFrameSideDataType typ
>>    */
>>   void av_frame_side_data_free(AVFrameSideData ***sd, int *nb_sd);
>>   
>> +/**
>> + * Remove existing entries before adding new ones.
>> + */
>>   #define AV_FRAME_SIDE_DATA_FLAG_UNIQUE (1 << 0)
>> +/**
>> + * Don't add a new entry if another of the same type exists.
> 
> This should mention that it only applies to MULTI side data types.

Ok.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/7 v4] avutil/frame: add a flag to allow overwritting existing entries

[Anton Khirnov]

Quoting James Almer (2024-03-28 12:41:40)
> On 3/28/2024 8:23 AM, Anton Khirnov wrote:
> > Quoting James Almer (2024-03-28 04:12:04)
> >> Enable it only for side data types that don't allow more than one entry.
> >>
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >>   libavutil/frame.c                 | 59 ++++++++++++++++++++++++++++---
> >>   libavutil/frame.h                 | 27 +++++++++-----
> >>   libavutil/tests/side_data_array.c | 52 +++++++++++++++------------
> >>   tests/ref/fate/side_data_array    | 22 ++++++------
> >>   4 files changed, 115 insertions(+), 45 deletions(-)
> >>
> >> diff --git a/libavutil/frame.c b/libavutil/frame.c
> >> index ef1613c344..d9bd19b2aa 100644
> >> --- a/libavutil/frame.c
> >> +++ b/libavutil/frame.c
> >> @@ -799,12 +799,34 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
> >>                                           enum AVFrameSideDataType type,
> >>                                           size_t size, unsigned int flags)
> >>   {
> >> -    AVBufferRef     *buf = av_buffer_alloc(size);
> >> +    const AVSideDataDescriptor *desc = av_frame_side_data_desc(type);
> >> +    AVBufferRef     *buf;
> >>       AVFrameSideData *ret = NULL;
> >>   
> >>       if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
> >>           remove_side_data(sd, nb_sd, type);
> >> +    if (!desc || !(desc->props & AV_SIDE_DATA_PROP_MULTI)) {
> > 
> > desc == NULL means type is invalid
> 
> This being a generic alloc function, it should IMO not be limited to 
> currently defined types. And I chose to treat them as non multi, as 
> that's the most common scenario.

Andreas argued in the thread adding descriptors that side data type
without a descriptor is invalid and should explode. I tend to agree.

-- 
Anton Khirnov
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/7 v4] avutil/frame: add a flag to allow overwritting existing entries

[James Almer]

On 3/28/2024 9:21 AM, Anton Khirnov wrote:
> Quoting James Almer (2024-03-28 12:41:40)
>> On 3/28/2024 8:23 AM, Anton Khirnov wrote:
>>> Quoting James Almer (2024-03-28 04:12:04)
>>>> Enable it only for side data types that don't allow more than one entry.
>>>>
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>>    libavutil/frame.c                 | 59 ++++++++++++++++++++++++++++---
>>>>    libavutil/frame.h                 | 27 +++++++++-----
>>>>    libavutil/tests/side_data_array.c | 52 +++++++++++++++------------
>>>>    tests/ref/fate/side_data_array    | 22 ++++++------
>>>>    4 files changed, 115 insertions(+), 45 deletions(-)
>>>>
>>>> diff --git a/libavutil/frame.c b/libavutil/frame.c
>>>> index ef1613c344..d9bd19b2aa 100644
>>>> --- a/libavutil/frame.c
>>>> +++ b/libavutil/frame.c
>>>> @@ -799,12 +799,34 @@ AVFrameSideData *av_frame_side_data_new(AVFrameSideData ***sd, int *nb_sd,
>>>>                                            enum AVFrameSideDataType type,
>>>>                                            size_t size, unsigned int flags)
>>>>    {
>>>> -    AVBufferRef     *buf = av_buffer_alloc(size);
>>>> +    const AVSideDataDescriptor *desc = av_frame_side_data_desc(type);
>>>> +    AVBufferRef     *buf;
>>>>        AVFrameSideData *ret = NULL;
>>>>    
>>>>        if (flags & AV_FRAME_SIDE_DATA_FLAG_UNIQUE)
>>>>            remove_side_data(sd, nb_sd, type);
>>>> +    if (!desc || !(desc->props & AV_SIDE_DATA_PROP_MULTI)) {
>>>
>>> desc == NULL means type is invalid
>>
>> This being a generic alloc function, it should IMO not be limited to
>> currently defined types. And I chose to treat them as non multi, as
>> that's the most common scenario.
> 
> Andreas argued in the thread adding descriptors that side data type
> without a descriptor is invalid and should explode. I tend to agree.

I agree with that when you're handling side data that comes from 
something like a filter or similar modules within our libraries, as 
that'd be a bug. But this is the public core API for side data, and i 
don't see a reason why we should fail if the caller does 
av_frame_side_data_new(&sd, &nd_sd, 1000, 1, 0);. We never did before, 
and doing it now feels wrong.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".