[FFmpeg-devel] [PATCH v3 07/12] avutil/frame: add helper for extending a set of side data

[Andreas Rheinhardt]

Jan Ekström:
> ---
>  libavutil/frame.c | 23 +++++++++++++++++++++++
>  libavutil/frame.h | 16 ++++++++++++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/libavutil/frame.c b/libavutil/frame.c
> index d8910a2120..04d56853f0 100644
> --- a/libavutil/frame.c
> +++ b/libavutil/frame.c
> @@ -880,6 +880,29 @@ AVFrameSideData *av_side_data_set_new_item(AVFrameSideDataSet *set,
>      return ret;
>  }
>  
> +int av_side_data_set_extend(AVFrameSideDataSet *dst,
> +                            const AVFrameSideDataSet src,
> +                            unsigned int allow_duplicates)
> +{
> +    for (int i = 0; i < src.nb_sd; i++) {
> +        const AVFrameSideData *sd_src = src.sd[i];
> +        AVFrameSideData *sd_dst =
> +            av_side_data_set_new_item(dst, sd_src->type,
> +                                      sd_src->size,
> +                                      allow_duplicates);
> +        if (!sd_dst) {
> +            av_side_data_set_uninit(dst);
> +            return AVERROR(ENOMEM);
> +        }
> +
> +        memcpy(sd_dst->data, sd_src->data, sd_src->size);

AVFrame side data is reference-counted.

> +
> +        av_dict_copy(&sd_dst->metadata, sd_src->metadata, 0);

Missing check.

> +    }
> +
> +    return 0;
> +}
> +
>  AVFrameSideData *av_side_data_set_get_item(const AVFrameSideDataSet set,
>                                             enum AVFrameSideDataType type)
>  {
> diff --git a/libavutil/frame.h b/libavutil/frame.h
> index 0cafc9c51f..2413000c9a 100644
> --- a/libavutil/frame.h
> +++ b/libavutil/frame.h
> @@ -1083,6 +1083,22 @@ AVFrameSideData *av_side_data_set_new_item(AVFrameSideDataSet *set,
>                                             size_t size,
>                                             unsigned int allow_duplicates);
>  
> +/**
> + * Add multiple side data entries to a set in one go.
> + *
> + * @param dst a set to which the side data should be added
> + * @param src a set from which the side data should be copied from

This needs to add something to ensure that dst and src refer to
different side-data (i.e. to disallow calls like
AVFrameSideDataSet set = { ... };

av_side_data_set_extend(&set, set, 0);)

> + * @param allow_duplicates an unsigned integer noting whether duplicates are
> + *                         allowed or not. If duplicates are not allowed, all
> + *                         entries of the same side data type are first removed
> + *                         and freed before the new entry is added.

Better use flags.

> + *
> + * @return negative error code on failure, >=0 on success.
> + */
> +int av_side_data_set_extend(AVFrameSideDataSet *dst,
> +                            const AVFrameSideDataSet src,
> +                            unsigned int allow_duplicates);

Do we really need this function? Are there enough potential users of it?

> +
>  /**
>   * Get a side data entry of a specific type from a set.
>   *