[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 at 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