[FFmpeg-devel] [PATCH 01/17] avcodec/avcodec: add side data to AVCodecContext

[James Almer]

On 9/5/2023 8:37 AM, Anton Khirnov wrote:
> Quoting James Almer (2023-09-05 13:26:22)
>> On 9/5/2023 8:07 AM, Anton Khirnov wrote:
>>> Quoting James Almer (2023-09-05 00:08:48)
>>>> This will allow the propagation of global side data within the AVCodecContext
>>>> instead of having to do it inside packets, and thus be available during init().
>>>> Global and frame specific side data will therefore be distinct.
>>>
>>> This commit message is misleading - there is already
>>> AVCodecContext.coded_side_data for exactly this purpose. And after the
>>> changes from the last iteration I see even less of a reason to replace
>>> it with a new field.
>>
>> I insist the new field in the form of a set is better, for the sake of
>> unified helpers that can be used in avctx, codecpar, avstream, and
>> potentially others in the future.
> 
> The big problem with this is that AVPacket is left as is. And since
> changing it would be a huge break for very little gain, we'll have
> different handling for packets and everything else for the foreseeable
> future.

Packets and frames have their own helpers, so there will be a 
distinction between those and side data set helpers initially used in 
structs meant for global side data. The API user doesn't need to access 
the fields directly.

> 
> I think you'd get almost the same benefits with downsides by making the
> helpers accept array+count as parameters. It's slightly less elegant,
> but not hugely so IMO.
> 
>> It will also be the packet counterpart of Jan's frame side data set
>> field. coded_side_data is currently used only to export CPB props, so
>> the amount of users is probably very small (Maybe only lavf, even). I
>> think the benefits in the long run outweigh the cons from the breakage
>> that would mean replacing coded_side_data.
>>
>> Also, my interpretation of coded is still that it refers to a coded
>> stream, much like we make a distinction between coded and raw for
>> bits_per_sample, and in decoding scenarios, side data entries would have
>> information that refer to the decoded raw stream (hdr, etc).
> 
> The intent was for it to be a direct counterpart to AVStream.side_data,

Since that one is being deprecated and removed, doing the same with its 
less used counterpart should not be that much of a problem.

> as is mentioned in the relevant commit message, so your interpretation
> is objectively wrong.

Fair, but i question your choice of name :p

> 
>> That said, I don't want to keep delaying this set much longer, so if
>> you're really against that change I'll try to remove it from the set and
>> keep the rest.
> 
> I'd appreciate more opinions on this, from whoever cares.

Ok, but like i mentioned, i really doubt there are many coded_side_data 
users out there, for being limited to only exporting CPB props.