[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 8:49 AM, James Almer wrote:
> 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".

Err, error code would be for av_frame_side_data_clone(). 
av_frame_side_data_new() can either return an entry or NULL, which the 
user will consider a failure.

> 
>>      * 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?