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