[FFmpeg-devel] [PATCH v3] avfilter/setpts: add option to preserve framerate

[Gyan Doshi]

On 2025-01-23 09:29 pm, Ronald S. Bultje wrote:
> Hi,
>
> On Thu, Jan 23, 2025 at 2:09 AM Gyan Doshi <ffmpeg at gyani.pro> wrote:
>
>> In f121d95, the outlink framerate was unconditionally unset.
>> This breaks/bloats outputs from CFR muxers unless the user explicitly
>> set 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
>> form `PTS+constant` which preserve the input framerate.
>>
>> Fixes #11428
>> ---
>> Corrected failing FATES
>>
>>   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..fc352ed34a 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 keep_fps (@emph{video only})
>> +Boolean option which determines if the original framerate is preserved.
>> +If set to false, be advised that a sane frame rate should be explicitly
>> +specified if output is sent to a constant frame rate muxer.
>> +Default is @code{true}.
>> +
>>   @end table
>>
>>   The expression is evaluated through the eval API and can contain the
>> following
>> diff --git a/libavfilter/setpts.c b/libavfilter/setpts.c
>> index 75d96247af..6609fe86aa 100644
>> --- a/libavfilter/setpts.c
>> +++ b/libavfilter/setpts.c
>> @@ -98,6 +98,7 @@ typedef struct SetPTSContext {
>>       const AVClass *class;
>>       char *expr_str;
>>       AVExpr *expr;
>> +    int keep_fps;
>>       double var_values[VAR_VARS_NB];
>>       enum AVMediaType type;
>>   } SetPTSContext;
>> @@ -153,8 +154,10 @@ static int config_input(AVFilterLink *inlink)
>>   static int config_output_video(AVFilterLink *outlink)
>>   {
>>       FilterLink *l = ff_filter_link(outlink);
>> +    SetPTSContext *s = outlink->src->priv;
>>
>> -    l->frame_rate = (AVRational){ 1, 0 };
>> +    if (!s->keep_fps)
>> +        l->frame_rate = (AVRational){ 1, 0 };
>>
>>       return 0;
>>   }
>> @@ -320,6 +323,7 @@ static int process_command(AVFilterContext *ctx, const
>> char *cmd, const char *ar
>>   #if CONFIG_SETPTS_FILTER
>>   static const AVOption setpts_options[] = {
>>       { "expr", "Expression determining the frame timestamp",
>> OFFSET(expr_str), AV_OPT_TYPE_STRING, { .str = "PTS" }, .flags = V|F|R },
>> +    { "keep_fps", "Preserve input framerate", OFFSET(keep_fps),
>> AV_OPT_TYPE_INT, { .i64 = 1 }, 0, 1, .flags = V|F },
>>       { NULL }
>>   };
>>   AVFILTER_DEFINE_CLASS(setpts);
>
> Thanks, that seems to do the right thing for me.
>
> Are there other situations where fps is unavailable? (I don't care about
> being exact or an average or whatever - just that xpsnr doesn't SIGFPE for
> regular ffmpeg commandline invocations.) Should we consider adding a
> div-by-zero protection in xpsnr?

Not a bad idea.  What would be the working fps for the filter in case 
fps.den is 0?

Other filters which unset framerate are the combination filters: 
{h,v,x}stack, concat, interleave

Regards,
Gyan