[FFmpeg-devel] [PATCH] frame: add a time_base field

[Lynne]

9 Sept 2021, 14:53 by george at nsup.org:

> Lynne (12021-09-09):
>
>> It's not a maintenance nightmare, it's a single optional field. Please don't
>>
>
> Then please answer this simple question: How do you guarantee that this
> new field will always be correctly kept updated?
>

Because all of our codecs pass their frames through a wrapper function before
they get to the user. So, we just set the field there, add a FATE test, and now
they're guaranteed to be correctly kept updated.


>> reject my ideas and needs outright, I'm not the only API user who would
>> benefit from this. Browsers have had issues with sandboxing for ages,
>> and they'd love to have all state be maintained in the frames. But since
>> that's unreasonable, I think just having the timebase used for the timestamps
>> makes a large difference to usability.
>>
>
> Software design 101: if you need some information along with an object,
> you define a structure to store that object and that information
> together.
>

So why are timestamps part of frames at all then?


> In other words:
>
> typedef struct My_frame {
>  AVFrame *frame;
>  AVRationl time_base;
>  whatever else I need;
> } My_frame;
>
>> What direction? It's completely reasonable to have timebases for the
>> currently-isolated timestamps in AVFrames in the same way it was
>> reasonable for packets.
>>
>
> Isolated timestamps make no sense.
>

They're not isolated, they're part of the frame. And to make any sense
of them, you need a timebase.


>> But they do exist in a void, I've been using them within a void with
>> only AVFrames serving as links between components. It's really
>> neat. Only avcodecparameters breaks that.
>>
>
> No, they do not exist in a void: not existing in the void is the
> difference between a frame and just an image.
>
>> You say you don't understand this patch, but on the other hand,
>>
>
> I have not said I do not understand the patch. I have asked that you (1)
> clarify the semantic and (2) specify what problem it is trying to solve
> so we can see if there is a simpler solution.
>

I did clarify the semantics. In technicolor. The message I sent earlier
reads like a spec. I don't mind sending a new RFC patch which includes
said clarifications in the comment section of the field.


> If the problem is that people want to use AVFrame in isolation, then
> there is a simpler solution: we answer that it is not how it is meant to
> work.
>
> I will add: if fftools/*.c could work without time_base in AVFrame, then
> any application can.
>
>> I don't understand your reasoning at all. It's not an over-arching
>> API design change, it's a single optional field that API users
>> were already including in their opaque context anyway.
>>
>
> And something extra we have to maintain and keep updated.
>

It's a single optional field! The way the comment is worded now,
we don't even have any obligation to update it.

I still don't understand your reasoning, other than "I don't like it.".