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

[Gyan Doshi]

On 2025-01-27 05:42 pm, Ronald S. Bultje wrote:
> Hi,
>
> On Sun, Jan 26, 2025 at 2:32 PM Marton Balint <cus at passwd.hu> wrote:
>
>>
>> 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. Does anyone object to backporting this to 7.1, where xpsnr was
> introduced?

I don't, but aren't we due for 7.2 by end of Feb?

Also see my patch at 
https://patchwork.ffmpeg.org/project/ffmpeg/patch/20250127052801.596-1-ffmpeg@gyani.pro/

Regards,
Gyan