[FFmpeg-devel] [PATCH v2] lavu/avframe: add a time_base field

[Lynne]

1 Dec 2021, 21:11 by dev at lynne.ee:

> 1 Dec 2021, 16:47 by anton at khirnov.net:
>
>> Quoting Lynne (2021-11-26 09:00:59)
>>
>>> 25 Nov 2021, 23:49 by cus at passwd.hu:
>>>
>>> >
>>> >
>>> > On Thu, 25 Nov 2021, Lynne wrote:
>>> >
>>> >> This adds a time_base field (currently unused), analogue to the
>>> >> AVPacket.time_base field.
>>> >> This allows for API clients to exchange AVFrames directly, without
>>> >> needing to plumb extra data from sources via side mechanisms.
>>> >>
>>> >
>>> > The objections raised before still stand I believe, and again, the main concern is that the API user won't know when to use the packet or frame timebase and when to use the frame/packet source timebase.
>>> >
>>> > You write this in the doxy:
>>> >
>>> >> Time base for the timestamps in this frame. May be 0, in which case the
>>> >> time_base from the frame source should be used.
>>> >>
>>> >
>>> > One could easily think - based on this text alone - that every user of avcodec_receive_frame should check if AVFrame->time_base is 0 to calculate real PTS...
>>> >
>>> > I'd be a lot more willing to accept this if you could document explicitly that AVFrame->time_base (and AVPacket->time_base) is not used by the API right now, and is similar to e.g. *opaque field. And later, if by using some magic flag or new API or whatever it turns out to make sense to actually use AVPacket/AVFrame time base, the doxy text can be extended accordingly.
>>> >
>>>
>>> So you'd like for the field to either be fully opaque or fully working (by always
>>> having the correct time_base value)?
>>> That sounds fair, description changed:
>>> "Time base for the timestamps in this frame. Currently unused by any API,
>>> users can set this and use it as an opaque field. In the future, this field may
>>> be set by the API for you, but its value will always be ignored."
>>>
>>
>> Always is a bit too strong, presumably we do want to start using that
>> field eventually.
>>
>> How about something like:
>> In the future, this field may be set on frames output by decoders or
>> filters, but its value will be by default ignored on input to encoders
>> or filters.
>>
>
> Yes, that's much better and allows us to extend it like avpacket's field.
> I've changed it locally. I think this has been on the ML long enough,
> so I'll push it in 2 days with the amended description so it makes it in 5.0.
> I'll also send a patch to change the description to the AVPacket's field
> to this soon.
>

Thanks everyone for the review, patch pushed along with APIchanges and minor bump.
I've sent a patch to add the same description for the AVPacket.time_base field,
and I'll write something up during the weekend to set them where possible and easy.