[FFmpeg-devel] [PATCH 1/6 v2] avutil/frame: add a flag to not create duplicate entries in a side data array

[James Almer]

On 3/27/2024 5:05 AM, Anton Khirnov wrote:
> Quoting James Almer (2024-03-25 21:05:57)
>> +/**
>> + * 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.
>> + */
>> +#define AV_FRAME_SIDE_DATA_FLAG_DONT_APPEND (1 << 1)
> 
> I don't really like this API, because it leaves too much work to the
> user.
> 
> With my descriptor set, we know when it makes sense to have duplicate
> side data. So the cases that can occur when adding side data of type T
> are:
> * T not present, call succeeds
> * T present, then
>      * T does not have a MULTI prop, then user decides whether to
>        replace or do nothing (or perhaps fail)

av_frame_side_data_new() returns an entry, so for non-MULTI types, it 
should return the existing one. There's no "do nothing" scenario, and i 
don't particularly like failing, more so now that 7.0 is branched so we 
can't stealthly change behavior and for example define EEXIST error code 
as "did nothing".

>      * T does have a MULTI prop, then user decides whether to replace,
>        add, or do nothing
> 
> I think the default behaviour for MULTI types should be adding, and the
> other two cases should be covered by the user explicitly.
> 
> Then we only need a single flag, which only applies to non-MULTI types,
> and chooses whether to replace or not.

I don't follow. How does a single flag let you choose between all these 
scenarios?