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 <jamr...@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-requ...@ffmpeg.org with subject "unsubscribe".

Reply via email to