[FFmpeg-devel] [PATCH] avpacket: RFC for ABI bump additions

[James Almer]

On 1/26/2021 1:17 PM, Anton Khirnov wrote:
> Quoting James Almer (2021-01-26 17:07:28)
>> On 1/26/2021 1:02 PM, Anton Khirnov wrote:
>>> Quoting Lynne (2021-01-24 18:34:36)
>>>>>>
>>>>>> Some devs (JEEB) wanted reception timestamps and original, overflowed timestamps for MPEG-TS.
>>>>>> I'd be willing to add a reception timestamp as long as we add an additional time_base field
>>>>>> and make it independent of the packet's pts field, since those timestamps are usually on
>>>>>> highly precise system clock time bases, and reducing their precision would be undesirable.
>>>>>>
>>>>>
>>>>> This sounds like side data to me.
>>>>>
>>>>>>
>>>>>> I'm not sure about overflowed original timestamps or how they would help.
>>>>>> Perhaps we should introduce an AVBufferRef *internal_ref like with AVFrame to
>>>>>> store such single library-only data?
>>>>>>
>>>>>
>>>>> I don't like treating the libraries specially, so -1 for me.
>>>>> internal_ref should not exist either.
>>>>>
>>>>
>>>> Yeah, tbh it's a single-library only field in avframes and is mainly used
>>>> for hardware API weirdness reasons.
>>>
>>> It's not even necessary, that code can work without it.
>>>
>>>> Also another suggestion: should we add a few dozen bytes of padding at
>>>> the end of the avpacket structure so if anything were to happen
>>>> we wouldn't be block on waiting for a bump to add a field?
>>>
>>> Ugly, but I guess acceptable? Have to be pretty careful what you add
>>> though.
>>
>> Alternatively, we could follow through with the idea of making its size
>> stop being part of the ABI.
>> It's a big change since AVPacket is kinda advertised as one of the few
>> structs with ABI-tied size, and it wont happen until at least two years
>> from now. But now that the old deprecated decode/encode API is being
>> removed and non reference counted packets will no longer be a thing,
>> it's probably a good time to do it.
> 
> We could start by adding a field to AVPacket that would be set to a
> magic value by av_packet_alloc().
> Then have e.g. AVCodecContext/AVFormatContext warn when they see a
> packet without this magic value.

I don't like much the idea of adding a public field just to emit a 
deprecation warning. And adding something like an internal pointer 
(Alongside a struct like AVPacketInternal) or an analog to 
AVFrame.private_ref would be an allocation overhead per packet, at least 
during the deprecation period (assuming it's not kept in place for other 
uses after that).