[FFmpeg-devel] [PATCH 1/7 v4] avutil/frame: add a flag to allow overwritting existing entries
James Almer
jamrial at gmail.com
Thu Mar 28 13:41:40 EET 2024
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 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
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.
More information about the ffmpeg-devel
mailing list