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

[Lynne]

Jan 24, 2021, 17:05 by anton at khirnov.net:

> Quoting Lynne (2021-01-23 20:10:46)
>
>> This is an RFC about the upcoming additions to the AVPacket structure
>> (whose size is still part of the ABI, so we need to plan for any changes).
>>
>> The current RFC patch adds 3 fields:
>>     - "void *opaque;" for the user to use as they wish, same as AVFrame.opaque
>>     - "void *opaque_ref;" for more permanent and propagating user data, same as AVFrame.opaque_ref
>>
>
> Generally in favor, but:
> - do we really need both?
> - ownership semantics should be specified more clearly, so there is no
>  ambiguity over who is allowed to set it. Something like:
>  whoever owns (i.e. is responsible for unreffing) the packet ref also
>  owns opaque_ref
>

I think the void * is useful for API users. They can set it to a local context
(such as their own muxing context), while the opaque_ref can be used for
more global info (internal metadata, reception timestamp for the source
so latency can be measured and such).
As for ownership sematics, the description copied verbatim from
libavutil/frame.h. To keep sync, I think that description should be expanded
first.



>>     - "AVRational time_base;" which will be set to indicate the time base of the packet's timestamps
>>
>
> In favor - should reduce confusion and existing users can still rescale
> timestamps manually before passing them to lavf. But then it should also be added to AVFrame.
>

Thought so too, will send a patch for AVFrame.time_base.



> And compatibility will be tricky - e.g. encoders setting this field will
> break muxing in lavf (since current users all rescale manually).
>

We discussed this in IRC, the conclusion was to add an optional
flag to every major context (bsfc, avc, lavf, lavfi*) which will
set the time_base value of produced avframes and avpacket,
and if it's present, then we can automagically rescale the timestamps
in cosumers/producers.
The flag will be deprecated once the bump happens. If the user
wishes to rescale themselves, they will still be able to do that
once the flag is removed.



>>
>> 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.


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?