[FFmpeg-devel] [PATCH v4] avfilter/setpts: add option to decide framerate handling

[Gyan Doshi]

On 2025-01-26 07:09 pm, Ronald S. Bultje wrote:
> Hi,
>
> On Sat, Jan 25, 2025 at 11:06 PM Gyan Doshi <ffmpeg at gyani.pro> wrote:
>
>>
>> On 2025-01-26 12:49 am, Marton Balint wrote:
>>>
>>> On Sat, 25 Jan 2025, Gyan Doshi wrote:
>>>
>>>> In f121d95, the outlink framerate was unconditionally unset.
>>>> This breaks/bloats outputs from CFR muxers unless the user explicitly
>>>> sets a sane framerate. And the most common invocation for setpts seen in
>>>> workflows, our docs and across the web is `PTS-STARTPTS` or others of
>>>> the
>>>> general form `PTS+constant` which preserves the input framerate.
>>>>
>>>> Fixes #11428
>>>> ---
>>>> v4: negated option sense and renamed to vfr
>>>>
>>>> doc/filters.texi          | 6 ++++++
>>>> libavfilter/setpts.c      | 6 +++++-
>>>> tests/fate/hevc.mak       | 2 +-
>>>> tests/fate/mov.mak        | 2 +-
>>>> tests/filtergraphs/setpts | 2 +-
>>>> 5 files changed, 14 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/doc/filters.texi b/doc/filters.texi
>>>> index b926b865ae..ea11d045ec 100644
>>>> --- a/doc/filters.texi
>>>> +++ b/doc/filters.texi
>>>> @@ -31478,6 +31478,12 @@ This filter accepts the following options:
>>>> @item expr
>>>> The expression which is evaluated for each frame to construct its
>>>> timestamp.
>>>>
>>>> + at item vfr (@emph{video only})
>>>> +Boolean option which determines if the original framerate metadata
>>>> is unset.
>>>> +If set to true, be advised that a sane frame rate should be explicitly
>>>> +specified if output is sent to a constant frame rate muxer.
>>> I propose a more understandable variant for the first sentence:
>>>
>>> Sets the filter output to variable frame rate by dropping the original
>>> constant framerate information if present. If set to true...
>> But that's not actually the case. This option does not make the output
>> VFR. If it did, this option would not be needed.
>> Once FR is unset, if the output goes to a CFR muxer and fps_mode is not
>> specified, ffmpeg will emit a  CFR stream using the time_base.
>> What the option does is a single narrow technical thing which has
>> implications depending on context. And that was the basis for the
>> original option name and description.
>>
>> 'strip_fps' and a corresponding description seems more accurate.
>
> (I'm the bug reporter.) I actually agree. Setting FPS doesn't mean CFR, it
> could just mean average frame rate or something. But please decide on
> something, we're currently stripping said metadata and there's no way to
> reinstate it except with lengthy hacks that we don't want people to start
> posting on stackoverflow - because then we'll never hear the end of it.

Ok, I'll change to strip_fps and push tomorrow (~18h) if there are no 
objections.

Regards,
Gyan