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

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

[Gyan Doshi]

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.
 
+@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.
+Default is @code{false}.
+
 @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..9573e16b63 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 vfr;
     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->vfr)
+        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 },
+    { "vfr", "Preserve input framerate", OFFSET(vfr), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, .flags = V|F },
     { NULL }
 };
 AVFILTER_DEFINE_CLASS(setpts);
diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
index 9e6fd72618..e4b04f7f19 100644
--- a/tests/fate/hevc.mak
+++ b/tests/fate/hevc.mak
@@ -210,7 +210,7 @@ $(HEVC_TESTS_444_12BIT): SCALE_OPTS := -pix_fmt yuv444p12le -vf scale
 fate-hevc-conformance-%: CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bit $(SCALE_OPTS)
 $(HEVC_TESTS_422_10BIN): CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bin $(SCALE_OPTS)
 $(HEVC_TESTS_MULTIVIEW): CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bit \
-	-pix_fmt yuv420p -map "0:view:0" -map "0:view:1" -vf setpts=N
+	-pix_fmt yuv420p -map "0:view:0" -map "0:view:1" -vf setpts=N:vfr=1
 
 FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER) += $(HEVC_TESTS_8BIT) $(HEVC_TESTS_444_8BIT)
 FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER SCALE_FILTER) +=         \
diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
index ca13ebfd44..fd0aaa2416 100644
--- a/tests/fate/mov.mak
+++ b/tests/fate/mov.mak
@@ -225,7 +225,7 @@ fate-mov-pcm-remux: CMP = oneline
 fate-mov-pcm-remux: REF = e76115bc392d702da38f523216bba165
 
 FATE_MOV_FFMPEG-$(call TRANSCODE, RAWVIDEO, MOV, TESTSRC_FILTER SETPTS_FILTER) += fate-mov-vfr
-fate-mov-vfr: CMD = md5 -filter_complex testsrc=size=2x2:duration=1,setpts=N*N -c rawvideo -fflags +bitexact -f mov
+fate-mov-vfr: CMD = md5 -filter_complex testsrc=size=2x2:duration=1,setpts=N*N:vfr=1 -c rawvideo -fflags +bitexact -f mov
 fate-mov-vfr: CMP = oneline
 fate-mov-vfr: REF = 1558b4a9398d8635783c93f84eb5a60d
 
diff --git a/tests/filtergraphs/setpts b/tests/filtergraphs/setpts
index 79037d1b65..3e5da7446e 100644
--- a/tests/filtergraphs/setpts
+++ b/tests/filtergraphs/setpts
@@ -1,2 +1,2 @@
 settb=1/1000,
-setpts=1/(35*TB) * (N + 0.05 * sin(N*2*PI/25))
+setpts=1/(35*TB) * (N + 0.05 * sin(N*2*PI/25)):vfr=1
-- 
2.46.1

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

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

[Marton Balint]

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

> +Default is @code{false}.
> +
> @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..9573e16b63 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 vfr;
>     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->vfr)
> +        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 },
> +    { "vfr", "Preserve input framerate", OFFSET(vfr), AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, .flags = V|F },

"Output variabe frame rate"

Otherwise LGTM.

Thanks,
Marton


>     { NULL }
> };
> AVFILTER_DEFINE_CLASS(setpts);
> diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
> index 9e6fd72618..e4b04f7f19 100644
> --- a/tests/fate/hevc.mak
> +++ b/tests/fate/hevc.mak
> @@ -210,7 +210,7 @@ $(HEVC_TESTS_444_12BIT): SCALE_OPTS := -pix_fmt yuv444p12le -vf scale
> fate-hevc-conformance-%: CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bit $(SCALE_OPTS)
> $(HEVC_TESTS_422_10BIN): CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bin $(SCALE_OPTS)
> $(HEVC_TESTS_MULTIVIEW): CMD = framecrc -i $(TARGET_SAMPLES)/hevc-conformance/$(subst fate-hevc-conformance-,,$(@)).bit \
> -	-pix_fmt yuv420p -map "0:view:0" -map "0:view:1" -vf setpts=N
> +	-pix_fmt yuv420p -map "0:view:0" -map "0:view:1" -vf setpts=N:vfr=1
>
> FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER) += $(HEVC_TESTS_8BIT) $(HEVC_TESTS_444_8BIT)
> FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER SCALE_FILTER) +=         \
> diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
> index ca13ebfd44..fd0aaa2416 100644
> --- a/tests/fate/mov.mak
> +++ b/tests/fate/mov.mak
> @@ -225,7 +225,7 @@ fate-mov-pcm-remux: CMP = oneline
> fate-mov-pcm-remux: REF = e76115bc392d702da38f523216bba165
>
> FATE_MOV_FFMPEG-$(call TRANSCODE, RAWVIDEO, MOV, TESTSRC_FILTER SETPTS_FILTER) += fate-mov-vfr
> -fate-mov-vfr: CMD = md5 -filter_complex testsrc=size=2x2:duration=1,setpts=N*N -c rawvideo -fflags +bitexact -f mov
> +fate-mov-vfr: CMD = md5 -filter_complex testsrc=size=2x2:duration=1,setpts=N*N:vfr=1 -c rawvideo -fflags +bitexact -f mov
> fate-mov-vfr: CMP = oneline
> fate-mov-vfr: REF = 1558b4a9398d8635783c93f84eb5a60d
>
> diff --git a/tests/filtergraphs/setpts b/tests/filtergraphs/setpts
> index 79037d1b65..3e5da7446e 100644
> --- a/tests/filtergraphs/setpts
> +++ b/tests/filtergraphs/setpts
> @@ -1,2 +1,2 @@
> settb=1/1000,
> -setpts=1/(35*TB) * (N + 0.05 * sin(N*2*PI/25))
> +setpts=1/(35*TB) * (N + 0.05 * sin(N*2*PI/25)):vfr=1
> -- 
> 2.46.1
>
> _______________________________________________
> 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".
>
_______________________________________________
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".

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

[Gyan Doshi]

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

Regards,
Gyan


>
>> +Default is @code{false}.
>> +
>> @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..9573e16b63 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 vfr;
>>     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->vfr)
>> +        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 },
>> +    { "vfr", "Preserve input framerate", OFFSET(vfr), 
>> AV_OPT_TYPE_BOOL, { .i64 = 0 }, 0, 1, .flags = V|F },
>
> "Output variabe frame rate"
>
> Otherwise LGTM.
>
> Thanks,
> Marton
>
>
>>     { NULL }
>> };
>> AVFILTER_DEFINE_CLASS(setpts);
>> diff --git a/tests/fate/hevc.mak b/tests/fate/hevc.mak
>> index 9e6fd72618..e4b04f7f19 100644
>> --- a/tests/fate/hevc.mak
>> +++ b/tests/fate/hevc.mak
>> @@ -210,7 +210,7 @@ $(HEVC_TESTS_444_12BIT): SCALE_OPTS := -pix_fmt 
>> yuv444p12le -vf scale
>> fate-hevc-conformance-%: CMD = framecrc -i 
>> $(TARGET_SAMPLES)/hevc-conformance/$(subst 
>> fate-hevc-conformance-,,$(@)).bit $(SCALE_OPTS)
>> $(HEVC_TESTS_422_10BIN): CMD = framecrc -i 
>> $(TARGET_SAMPLES)/hevc-conformance/$(subst 
>> fate-hevc-conformance-,,$(@)).bin $(SCALE_OPTS)
>> $(HEVC_TESTS_MULTIVIEW): CMD = framecrc -i 
>> $(TARGET_SAMPLES)/hevc-conformance/$(subst 
>> fate-hevc-conformance-,,$(@)).bit \
>> -    -pix_fmt yuv420p -map "0:view:0" -map "0:view:1" -vf setpts=N
>> +    -pix_fmt yuv420p -map "0:view:0" -map "0:view:1" -vf setpts=N:vfr=1
>>
>> FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER) += 
>> $(HEVC_TESTS_8BIT) $(HEVC_TESTS_444_8BIT)
>> FATE_HEVC-$(call FRAMECRC, HEVC, HEVC, HEVC_PARSER SCALE_FILTER) 
>> +=         \
>> diff --git a/tests/fate/mov.mak b/tests/fate/mov.mak
>> index ca13ebfd44..fd0aaa2416 100644
>> --- a/tests/fate/mov.mak
>> +++ b/tests/fate/mov.mak
>> @@ -225,7 +225,7 @@ fate-mov-pcm-remux: CMP = oneline
>> fate-mov-pcm-remux: REF = e76115bc392d702da38f523216bba165
>>
>> FATE_MOV_FFMPEG-$(call TRANSCODE, RAWVIDEO, MOV, TESTSRC_FILTER 
>> SETPTS_FILTER) += fate-mov-vfr
>> -fate-mov-vfr: CMD = md5 -filter_complex 
>> testsrc=size=2x2:duration=1,setpts=N*N -c rawvideo -fflags +bitexact 
>> -f mov
>> +fate-mov-vfr: CMD = md5 -filter_complex 
>> testsrc=size=2x2:duration=1,setpts=N*N:vfr=1 -c rawvideo -fflags 
>> +bitexact -f mov
>> fate-mov-vfr: CMP = oneline
>> fate-mov-vfr: REF = 1558b4a9398d8635783c93f84eb5a60d
>>
>> diff --git a/tests/filtergraphs/setpts b/tests/filtergraphs/setpts
>> index 79037d1b65..3e5da7446e 100644
>> --- a/tests/filtergraphs/setpts
>> +++ b/tests/filtergraphs/setpts
>> @@ -1,2 +1,2 @@
>> settb=1/1000,
>> -setpts=1/(35*TB) * (N + 0.05 * sin(N*2*PI/25))
>> +setpts=1/(35*TB) * (N + 0.05 * sin(N*2*PI/25)):vfr=1
>> -- 
>> 2.46.1
>>
>> _______________________________________________
>> 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".
>>
> _______________________________________________
> 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".

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

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

[Ronald S. Bultje]

Hi,

On Sat, Jan 25, 2025 at 11:06 PM Gyan Doshi <ffmpeg@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.
> >>
> >> +@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.

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

Re: [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@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.
>>>>
>>>> +@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

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

Re: [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@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.
>>>>>
>>>>>  +@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
_______________________________________________
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".

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

[Ronald S. Bultje]

Hi,

On Sun, Jan 26, 2025 at 2:32 PM Marton Balint <cus@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@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.
> >>>>>
> >>>>>  +@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?

Thanks,
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".

Re: [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@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@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.
>>>>>>>
>>>>>>>   +@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

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