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

[Marton Balint]

On Sun, 26 Jan 2025, Gyan Doshi wrote:

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

Ok, "strip_fps" is fine by me.

Thanks,
Marton