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

From: "Ronald S. Bultje" <rsbultje@gmail.com>
To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
Subject: Re: [FFmpeg-devel] [PATCH v3] avfilter/setpts: add option to preserve framerate
Date: Thu, 23 Jan 2025 10:59:55 -0500
Message-ID: <CAEEMt2k6aKt-ROt7DyWSNhocUq4F-GxK=Xh0vL+dhBwEoWVR5w@mail.gmail.com> (raw)
In-Reply-To: <20250123070837.1819-1-ffmpeg@gyani.pro>

Hi,

On Thu, Jan 23, 2025 at 2:09 AM Gyan Doshi <ffmpeg@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.
>
> +@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?

Ronald
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".