[FFmpeg-devel] [PATCH 1/2] avfilter: mark scale2ref as supporting dynamic sizes

[FFmpeg-devel] [PATCH 1/2] avfilter: mark scale2ref as supporting dynamic sizes

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

Analogous to the "scale" filter, which this is practically identical
with.
---
 libavfilter/avfilter.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
index 831871de90b..dcad4d55292 100644
--- a/libavfilter/avfilter.c
+++ b/libavfilter/avfilter.c
@@ -1027,7 +1027,8 @@ int ff_filter_frame(AVFilterLink *link, AVFrame *frame)
             strcmp(link->dst->filter->name, "format") &&
             strcmp(link->dst->filter->name, "idet") &&
             strcmp(link->dst->filter->name, "null") &&
-            strcmp(link->dst->filter->name, "scale")) {
+            strcmp(link->dst->filter->name, "scale") &&
+            strcmp(link->dst->filter->name, "scale2ref")) {
             av_assert1(frame->format        == link->format);
             av_assert1(frame->width         == link->w);
             av_assert1(frame->height        == link->h);
-- 
2.44.0

_______________________________________________
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] [PATCH 2/2] avfilter/vf_scale2ref: switch to FFFrameSync

[Niklas Haas]

From: Niklas Haas <git@haasn.dev>

This filter's existing design has a number of issues:

- There is no guarantee whatsoever about the order in which frames are
  pushed onto the main and ref link, due to this being entirely
  dependent on the order in which downstream filters decide to request
  frames from their various inputs. As such, there is absolutely no
  synchronization for ref streams with dynamically changing resolutions
  (see e.g. fate/h264/reinit-*).

- For some (likely historical) reason, this filter outputs its ref
  stream as a second ref output, which is in principle completely
  unnecessary (complex filter graph users can just duplicate the input
  pin), but in practice only required to allow this filter to
  "eventually" see changes to the ref stream (see first point). In
  particular, this means that if the user uses the "wrong" pin, this
  filter may break completely.

- The default filter activation function is fundamentally incapable of
  handling filters with multiple inputs cleanly, because doing so
  requires both knowledge of how these inputs should be internally
  ordered, but also how to handle EOF conditions on either input (or
  downstream). Both of these are best left to the filter specific
  options. (See #10795 for the consequences of using the default
  activate with multiple inputs).

Switching this filter to framesync fixes all three points:

- ff_framesync_activate() correctly handles multiple inputs and EOF
  conditions (and is configurable with the framesync-specific options)
- framesync only supports a single output, so we can (indeed must) drop
  the redundant ref output stream

Update documentation, changelog and tests to correspond to the new usage
pattern.

Fixes: https://trac.ffmpeg.org/ticket/10795
---
 Changelog                                |   2 +
 doc/filters.texi                         |  10 +-
 libavfilter/vf_scale.c                   | 130 ++++++++++++-----------
 tests/filtergraphs/scale2ref_keep_aspect |   3 +-
 4 files changed, 76 insertions(+), 69 deletions(-)

diff --git a/Changelog b/Changelog
index 069b8274489..bacda2524ea 100644
--- a/Changelog
+++ b/Changelog
@@ -32,6 +32,8 @@ version <next>:
 - DVD-Video demuxer, powered by libdvdnav and libdvdread
 - ffprobe -show_stream_groups option
 - ffprobe (with -export_side_data film_grain) now prints film grain metadata
+- scale2ref now only has a single output stream, and supports the framesync
+  options
 
 
 version 6.1:
diff --git a/doc/filters.texi b/doc/filters.texi
index e0436a5755c..07e8136adb3 100644
--- a/doc/filters.texi
+++ b/doc/filters.texi
@@ -21555,9 +21555,9 @@ Deprecated, do not use.
 Scale (resize) the input video, based on a reference video.
 
 See the scale filter for available options, scale2ref supports the same but
-uses the reference video instead of the main input as basis. scale2ref also
-supports the following additional constants for the @option{w} and
-@option{h} options:
+uses the reference video instead of the main input as basis. This filter also
+supports the @ref{framesync} options. In addition, scale2ref also supports the
+following additional constants for the @option{w} and @option{h} options:
 
 @table @var
 @item main_w
@@ -21600,13 +21600,13 @@ Only available with @code{eval=frame}.
 @item
 Scale a subtitle stream (b) to match the main video (a) in size before overlaying
 @example
-'scale2ref[b][a];[a][b]overlay'
+'[b][a]scale2ref[sub];[a][sub]overlay'
 @end example
 
 @item
 Scale a logo to 1/10th the height of a video, while preserving its display aspect ratio.
 @example
-[logo-in][video-in]scale2ref=w=oh*mdar:h=ih/10[logo-out][video-out]
+[logo-in][video]scale2ref=w=oh*mdar:h=ih/10[logo-out]
 @end example
 @end itemize
 
diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
index fc3b5a91e60..d4173b63097 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -29,6 +29,7 @@
 
 #include "avfilter.h"
 #include "formats.h"
+#include "framesync.h"
 #include "internal.h"
 #include "scale_eval.h"
 #include "video.h"
@@ -114,6 +115,7 @@ typedef struct ScaleContext {
     struct SwsContext *isws[2]; ///< software scaler context for interlaced material
     // context used for forwarding options to sws
     struct SwsContext *sws_opts;
+    FFFrameSync fs; ///< for scale2ref
 
     /**
      * New dimensions. Special values are:
@@ -288,6 +290,9 @@ static av_cold int preinit(AVFilterContext *ctx)
     if (ret < 0)
         return ret;
 
+    if (ctx->filter == &ff_vf_scale2ref)
+        ff_framesync_preinit(&scale->fs);
+
     return 0;
 }
 
@@ -303,6 +308,8 @@ static const int sws_colorspaces[] = {
     -1
 };
 
+static int do_scale2ref(FFFrameSync *fs);
+
 static av_cold int init(AVFilterContext *ctx)
 {
     ScaleContext *scale = ctx->priv;
@@ -380,6 +387,7 @@ static av_cold int init(AVFilterContext *ctx)
     if (!threads)
         av_opt_set_int(scale->sws_opts, "threads", ff_filter_get_nb_threads(ctx), 0);
 
+    scale->fs.on_event = do_scale2ref;
     return 0;
 }
 
@@ -389,6 +397,7 @@ static av_cold void uninit(AVFilterContext *ctx)
     av_expr_free(scale->w_pexpr);
     av_expr_free(scale->h_pexpr);
     scale->w_pexpr = scale->h_pexpr = NULL;
+    ff_framesync_uninit(&scale->fs);
     sws_freeContext(scale->sws_opts);
     sws_freeContext(scale->sws);
     sws_freeContext(scale->isws[0]);
@@ -678,35 +687,16 @@ static int config_props(AVFilterLink *outlink)
            flags_val);
     av_freep(&flags_val);
 
-    return 0;
-
-fail:
-    return ret;
-}
-
-static int config_props_ref(AVFilterLink *outlink)
-{
-    AVFilterLink *inlink = outlink->src->inputs[1];
-
-    outlink->w = inlink->w;
-    outlink->h = inlink->h;
-    outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
-    outlink->time_base = inlink->time_base;
-    outlink->frame_rate = inlink->frame_rate;
-    outlink->colorspace = inlink->colorspace;
-    outlink->color_range = inlink->color_range;
+    if (ctx->filter != &ff_vf_scale2ref)
+        return 0;
 
-    return 0;
-}
+    if ((ret = ff_framesync_init_dualinput(&scale->fs, ctx)) < 0)
+        return ret;
 
-static int request_frame(AVFilterLink *outlink)
-{
-    return ff_request_frame(outlink->src->inputs[0]);
-}
+    return ff_framesync_configure(&scale->fs);
 
-static int request_frame_ref(AVFilterLink *outlink)
-{
-    return ff_request_frame(outlink->src->inputs[1]);
+fail:
+    return ret;
 }
 
 static void frame_offset(AVFrame *frame, int dir, int is_pal)
@@ -909,43 +899,49 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
     return ret;
 }
 
-static int filter_frame_ref(AVFilterLink *link, AVFrame *in)
+static int do_scale2ref(FFFrameSync *fs)
 {
-    ScaleContext *scale = link->dst->priv;
-    AVFilterLink *outlink = link->dst->outputs[1];
-    int frame_changed;
+    AVFilterContext *ctx = fs->parent;
+    ScaleContext *scale = ctx->priv;
+    AVFilterLink *reflink = ctx->inputs[1];
+    AVFilterLink *outlink = ctx->outputs[0];
+    AVFrame *main, *ref, *out;
+    int ret;
 
-    frame_changed = in->width  != link->w ||
-                    in->height != link->h ||
-                    in->format != link->format ||
-                    in->sample_aspect_ratio.den != link->sample_aspect_ratio.den ||
-                    in->sample_aspect_ratio.num != link->sample_aspect_ratio.num ||
-                    in->colorspace != link->colorspace ||
-                    in->color_range != link->color_range;
+    ret = ff_framesync_dualinput_get(fs, &main, &ref);
+    if (ret < 0)
+        return ret;
 
-    if (frame_changed) {
-        link->format = in->format;
-        link->w = in->width;
-        link->h = in->height;
-        link->sample_aspect_ratio.num = in->sample_aspect_ratio.num;
-        link->sample_aspect_ratio.den = in->sample_aspect_ratio.den;
-        link->colorspace = in->colorspace;
-        link->color_range = in->color_range;
+    if (ref) {
+        reflink->format = ref->format;
+        reflink->w = ref->width;
+        reflink->h = ref->height;
+        reflink->sample_aspect_ratio.num = ref->sample_aspect_ratio.num;
+        reflink->sample_aspect_ratio.den = ref->sample_aspect_ratio.den;
+        reflink->colorspace = ref->colorspace;
+        reflink->color_range = ref->color_range;
 
-        config_props_ref(outlink);
-    }
+        ret = config_props(outlink);
+        if (ret < 0)
+            return ret;
 
-    if (scale->eval_mode == EVAL_MODE_FRAME) {
-        scale->var_values[VAR_N] = link->frame_count_out;
-        scale->var_values[VAR_T] = TS2T(in->pts, link->time_base);
+        if (scale->eval_mode == EVAL_MODE_FRAME) {
+            scale->var_values[VAR_N] = reflink->frame_count_out;
+            scale->var_values[VAR_T] = TS2T(ref->pts, reflink->time_base);
 #if FF_API_FRAME_PKT
 FF_DISABLE_DEPRECATION_WARNINGS
-        scale->var_values[VAR_POS] = in->pkt_pos == -1 ? NAN : in->pkt_pos;
+            scale->var_values[VAR_POS] = ref->pkt_pos == -1 ? NAN : ref->pkt_pos;
 FF_ENABLE_DEPRECATION_WARNINGS
 #endif
+        }
+    }
+
+    ret = scale_frame(ctx->inputs[0], main, &out);
+    if (out) {
+        return ff_filter_frame(outlink, out);
     }
 
-    return ff_filter_frame(outlink, in);
+    return ret;
 }
 
 static int process_command(AVFilterContext *ctx, const char *cmd, const char *args,
@@ -973,9 +969,25 @@ static int process_command(AVFilterContext *ctx, const char *cmd, const char *ar
     return ret;
 }
 
+static int activate(AVFilterContext *ctx)
+{
+    ScaleContext *scale = ctx->priv;
+    return ff_framesync_activate(&scale->fs);
+}
+
 static const AVClass *child_class_iterate(void **iter)
 {
-    const AVClass *c = *iter ? NULL : sws_get_class();
+    void *tmp = NULL;
+    const AVClass *sws = sws_get_class();
+    const AVClass *fs = ff_framesync_child_class_iterate(tmp);
+    const AVClass *c;
+    if (!*iter) {
+        c = sws;
+    } else if (*iter == (void*)(uintptr_t)sws) {
+        c = fs;
+    } else {
+        c = NULL;
+    }
     *iter = (void*)(uintptr_t)c;
     return c;
 }
@@ -985,6 +997,8 @@ static void *child_next(void *obj, void *prev)
     ScaleContext *s = obj;
     if (!prev)
         return s->sws_opts;
+    if (prev == s->sws_opts)
+        return &s->fs;
     return NULL;
 }
 
@@ -1082,12 +1096,10 @@ static const AVFilterPad avfilter_vf_scale2ref_inputs[] = {
     {
         .name         = "default",
         .type         = AVMEDIA_TYPE_VIDEO,
-        .filter_frame = filter_frame,
     },
     {
         .name         = "ref",
         .type         = AVMEDIA_TYPE_VIDEO,
-        .filter_frame = filter_frame_ref,
     },
 };
 
@@ -1096,13 +1108,6 @@ static const AVFilterPad avfilter_vf_scale2ref_outputs[] = {
         .name         = "default",
         .type         = AVMEDIA_TYPE_VIDEO,
         .config_props = config_props,
-        .request_frame= request_frame,
-    },
-    {
-        .name         = "ref",
-        .type         = AVMEDIA_TYPE_VIDEO,
-        .config_props = config_props_ref,
-        .request_frame= request_frame_ref,
     },
 };
 
@@ -1117,5 +1122,6 @@ const AVFilter ff_vf_scale2ref = {
     FILTER_INPUTS(avfilter_vf_scale2ref_inputs),
     FILTER_OUTPUTS(avfilter_vf_scale2ref_outputs),
     FILTER_QUERY_FUNC(query_formats),
+    .activate        = activate,
     .process_command = process_command,
 };
diff --git a/tests/filtergraphs/scale2ref_keep_aspect b/tests/filtergraphs/scale2ref_keep_aspect
index f407460ec7c..d63968666a8 100644
--- a/tests/filtergraphs/scale2ref_keep_aspect
+++ b/tests/filtergraphs/scale2ref_keep_aspect
@@ -1,5 +1,4 @@
 sws_flags=+accurate_rnd+bitexact;
 testsrc=size=320x240 [main];
 testsrc=size=640x360 [ref];
-[main][ref] scale2ref=iw/4:ow/mdar [main][ref];
-[ref] nullsink
+[main][ref] scale2ref=iw/4:ow/mdar [main]
-- 
2.44.0

_______________________________________________
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 2/2] avfilter/vf_scale2ref: switch to FFFrameSync

[Niklas Haas]

On Wed, 13 Mar 2024 13:24:25 +0100 Niklas Haas <ffmpeg@haasn.xyz> wrote:
> -    frame_changed = in->width  != link->w ||
> -                    in->height != link->h ||
> -                    in->format != link->format ||
> -                    in->sample_aspect_ratio.den != link->sample_aspect_ratio.den ||
> -                    in->sample_aspect_ratio.num != link->sample_aspect_ratio.num ||
> -                    in->colorspace != link->colorspace ||
> -                    in->color_range != link->color_range;
> +    ret = ff_framesync_dualinput_get(fs, &main, &ref);
> +    if (ret < 0)
> +        return ret;
>  
> -    if (frame_changed) {
> -        link->format = in->format;
> -        link->w = in->width;
> -        link->h = in->height;
> -        link->sample_aspect_ratio.num = in->sample_aspect_ratio.num;
> -        link->sample_aspect_ratio.den = in->sample_aspect_ratio.den;
> -        link->colorspace = in->colorspace;
> -        link->color_range = in->color_range;
> +    if (ref) {
> +        reflink->format = ref->format;
> +        reflink->w = ref->width;
> +        reflink->h = ref->height;
> +        reflink->sample_aspect_ratio.num = ref->sample_aspect_ratio.num;
> +        reflink->sample_aspect_ratio.den = ref->sample_aspect_ratio.den;
> +        reflink->colorspace = ref->colorspace;
> +        reflink->color_range = ref->color_range;
>  
> -        config_props_ref(outlink);
> -    }
> +        ret = config_props(outlink);
> +        if (ret < 0)
> +            return ret;

This change unintentionally dropped the `frame_changed` check to avoid
reconfiguring the sws context unnecessarily. Fixed locally by
reintroducing the check.
_______________________________________________
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 1/2] avfilter: mark scale2ref as supporting dynamic sizes

[Gyan Doshi]

On 2024-03-13 05:54 pm, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
>
> Analogous to the "scale" filter, which this is practically identical
> with.
> ---
>   libavfilter/avfilter.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 831871de90b..dcad4d55292 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -1027,7 +1027,8 @@ int ff_filter_frame(AVFilterLink *link, AVFrame *frame)
>               strcmp(link->dst->filter->name, "format") &&
>               strcmp(link->dst->filter->name, "idet") &&
>               strcmp(link->dst->filter->name, "null") &&
> -            strcmp(link->dst->filter->name, "scale")) {
> +            strcmp(link->dst->filter->name, "scale") &&
> +            strcmp(link->dst->filter->name, "scale2ref")) {
>               av_assert1(frame->format        == link->format);
>               av_assert1(frame->width         == link->w);
>               av_assert1(frame->height        == link->h);

LGTM.

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 2/2] avfilter/vf_scale2ref: switch to FFFrameSync

[Gyan Doshi]

On 2024-03-13 05:54 pm, Niklas Haas wrote:
> From: Niklas Haas<git@haasn.dev>
>
> This filter's existing design has a number of issues:
>
> - There is no guarantee whatsoever about the order in which frames are
>    pushed onto the main and ref link, due to this being entirely
>    dependent on the order in which downstream filters decide to request
>    frames from their various inputs. As such, there is absolutely no
>    synchronization for ref streams with dynamically changing resolutions
>    (see e.g. fate/h264/reinit-*).

The only raison d'etre for scale2ref was to have some way to access 
another stream's parameters. Dynamic streams weren't really the focus.

> - For some (likely historical) reason, this filter outputs its ref
>    stream as a second ref output, which is in principle completely
>    unnecessary (complex filter graph users can just duplicate the input
>    pin), but in practice only required to allow this filter to
>    "eventually" see changes to the ref stream (see first point). In
>    particular, this means that if the user uses the "wrong" pin, this
>    filter may break completely.

This change is fine but you should note in the manual docs that this 
change has occurred (and when) as existing scripts will fail due to the 
surplus output pad.

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 2/2] avfilter/vf_scale2ref: switch to FFFrameSync

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 2485 bytes --]

On Wed, Mar 13, 2024 at 01:24:25PM +0100, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> This filter's existing design has a number of issues:
> 
> - There is no guarantee whatsoever about the order in which frames are
>   pushed onto the main and ref link, due to this being entirely
>   dependent on the order in which downstream filters decide to request
>   frames from their various inputs. As such, there is absolutely no
>   synchronization for ref streams with dynamically changing resolutions
>   (see e.g. fate/h264/reinit-*).
> 
> - For some (likely historical) reason, this filter outputs its ref
>   stream as a second ref output, which is in principle completely
>   unnecessary (complex filter graph users can just duplicate the input
>   pin), but in practice only required to allow this filter to
>   "eventually" see changes to the ref stream (see first point). In
>   particular, this means that if the user uses the "wrong" pin, this
>   filter may break completely.
> 
> - The default filter activation function is fundamentally incapable of
>   handling filters with multiple inputs cleanly, because doing so
>   requires both knowledge of how these inputs should be internally
>   ordered, but also how to handle EOF conditions on either input (or
>   downstream). Both of these are best left to the filter specific
>   options. (See #10795 for the consequences of using the default
>   activate with multiple inputs).
> 
> Switching this filter to framesync fixes all three points:
> 
> - ff_framesync_activate() correctly handles multiple inputs and EOF
>   conditions (and is configurable with the framesync-specific options)
> - framesync only supports a single output, so we can (indeed must) drop
>   the redundant ref output stream
> 
> Update documentation, changelog and tests to correspond to the new usage
> pattern.
> 
> Fixes: https://trac.ffmpeg.org/ticket/10795
> ---
>  Changelog                                |   2 +
>  doc/filters.texi                         |  10 +-
>  libavfilter/vf_scale.c                   | 130 ++++++++++++-----------
>  tests/filtergraphs/scale2ref_keep_aspect |   3 +-
>  4 files changed, 76 insertions(+), 69 deletions(-)

this causes
./ffplay --help
to segfault

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 2/2] avfilter/vf_scale2ref: switch to FFFrameSync

[James Almer]

On 3/13/2024 8:41 PM, Michael Niedermayer wrote:
> On Wed, Mar 13, 2024 at 01:24:25PM +0100, Niklas Haas wrote:
>> From: Niklas Haas <git@haasn.dev>
>>
>> This filter's existing design has a number of issues:
>>
>> - There is no guarantee whatsoever about the order in which frames are
>>    pushed onto the main and ref link, due to this being entirely
>>    dependent on the order in which downstream filters decide to request
>>    frames from their various inputs. As such, there is absolutely no
>>    synchronization for ref streams with dynamically changing resolutions
>>    (see e.g. fate/h264/reinit-*).
>>
>> - For some (likely historical) reason, this filter outputs its ref
>>    stream as a second ref output, which is in principle completely
>>    unnecessary (complex filter graph users can just duplicate the input
>>    pin), but in practice only required to allow this filter to
>>    "eventually" see changes to the ref stream (see first point). In
>>    particular, this means that if the user uses the "wrong" pin, this
>>    filter may break completely.
>>
>> - The default filter activation function is fundamentally incapable of
>>    handling filters with multiple inputs cleanly, because doing so
>>    requires both knowledge of how these inputs should be internally
>>    ordered, but also how to handle EOF conditions on either input (or
>>    downstream). Both of these are best left to the filter specific
>>    options. (See #10795 for the consequences of using the default
>>    activate with multiple inputs).
>>
>> Switching this filter to framesync fixes all three points:
>>
>> - ff_framesync_activate() correctly handles multiple inputs and EOF
>>    conditions (and is configurable with the framesync-specific options)
>> - framesync only supports a single output, so we can (indeed must) drop
>>    the redundant ref output stream
>>
>> Update documentation, changelog and tests to correspond to the new usage
>> pattern.
>>
>> Fixes: https://trac.ffmpeg.org/ticket/10795
>> ---
>>   Changelog                                |   2 +
>>   doc/filters.texi                         |  10 +-
>>   libavfilter/vf_scale.c                   | 130 ++++++++++++-----------
>>   tests/filtergraphs/scale2ref_keep_aspect |   3 +-
>>   4 files changed, 76 insertions(+), 69 deletions(-)
> 
> this causes
> ./ffplay --help
> to segfault

Unrelated to this crash, but why does that command line output every 
single option from every single filter? It's several thousand printed 
lines. Shouldn't that be the output of --longhelp or similar, and leave 
--help to print some basic non filter specific options?
_______________________________________________
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 2/2] avfilter/vf_scale2ref: switch to FFFrameSync

[Andreas Rheinhardt]

Niklas Haas:
> From: Niklas Haas <git@haasn.dev>
> 
> This filter's existing design has a number of issues:
> 
> - There is no guarantee whatsoever about the order in which frames are
>   pushed onto the main and ref link, due to this being entirely
>   dependent on the order in which downstream filters decide to request
>   frames from their various inputs. As such, there is absolutely no
>   synchronization for ref streams with dynamically changing resolutions
>   (see e.g. fate/h264/reinit-*).
> 
> - For some (likely historical) reason, this filter outputs its ref
>   stream as a second ref output, which is in principle completely
>   unnecessary (complex filter graph users can just duplicate the input
>   pin), but in practice only required to allow this filter to
>   "eventually" see changes to the ref stream (see first point). In
>   particular, this means that if the user uses the "wrong" pin, this
>   filter may break completely.
> 
> - The default filter activation function is fundamentally incapable of
>   handling filters with multiple inputs cleanly, because doing so
>   requires both knowledge of how these inputs should be internally
>   ordered, but also how to handle EOF conditions on either input (or
>   downstream). Both of these are best left to the filter specific
>   options. (See #10795 for the consequences of using the default
>   activate with multiple inputs).
> 
> Switching this filter to framesync fixes all three points:
> 
> - ff_framesync_activate() correctly handles multiple inputs and EOF
>   conditions (and is configurable with the framesync-specific options)
> - framesync only supports a single output, so we can (indeed must) drop
>   the redundant ref output stream
> 
> Update documentation, changelog and tests to correspond to the new usage
> pattern.
> 
> Fixes: https://trac.ffmpeg.org/ticket/10795
> ---
>  Changelog                                |   2 +
>  doc/filters.texi                         |  10 +-
>  libavfilter/vf_scale.c                   | 130 ++++++++++++-----------
>  tests/filtergraphs/scale2ref_keep_aspect |   3 +-
>  4 files changed, 76 insertions(+), 69 deletions(-)
> 
> diff --git a/Changelog b/Changelog
> index 069b8274489..bacda2524ea 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -32,6 +32,8 @@ version <next>:
>  - DVD-Video demuxer, powered by libdvdnav and libdvdread
>  - ffprobe -show_stream_groups option
>  - ffprobe (with -export_side_data film_grain) now prints film grain metadata
> +- scale2ref now only has a single output stream, and supports the framesync
> +  options
>  
>  
>  version 6.1:
> diff --git a/doc/filters.texi b/doc/filters.texi
> index e0436a5755c..07e8136adb3 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -21555,9 +21555,9 @@ Deprecated, do not use.
>  Scale (resize) the input video, based on a reference video.
>  
>  See the scale filter for available options, scale2ref supports the same but
> -uses the reference video instead of the main input as basis. scale2ref also
> -supports the following additional constants for the @option{w} and
> -@option{h} options:
> +uses the reference video instead of the main input as basis. This filter also
> +supports the @ref{framesync} options. In addition, scale2ref also supports the
> +following additional constants for the @option{w} and @option{h} options:
>  
>  @table @var
>  @item main_w
> @@ -21600,13 +21600,13 @@ Only available with @code{eval=frame}.
>  @item
>  Scale a subtitle stream (b) to match the main video (a) in size before overlaying
>  @example
> -'scale2ref[b][a];[a][b]overlay'
> +'[b][a]scale2ref[sub];[a][sub]overlay'
>  @end example
>  
>  @item
>  Scale a logo to 1/10th the height of a video, while preserving its display aspect ratio.
>  @example
> -[logo-in][video-in]scale2ref=w=oh*mdar:h=ih/10[logo-out][video-out]
> +[logo-in][video]scale2ref=w=oh*mdar:h=ih/10[logo-out]
>  @end example
>  @end itemize
>  
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index fc3b5a91e60..d4173b63097 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -29,6 +29,7 @@
>  
>  #include "avfilter.h"
>  #include "formats.h"
> +#include "framesync.h"
>  #include "internal.h"
>  #include "scale_eval.h"
>  #include "video.h"
> @@ -114,6 +115,7 @@ typedef struct ScaleContext {
>      struct SwsContext *isws[2]; ///< software scaler context for interlaced material
>      // context used for forwarding options to sws
>      struct SwsContext *sws_opts;
> +    FFFrameSync fs; ///< for scale2ref
>  
>      /**
>       * New dimensions. Special values are:
> @@ -288,6 +290,9 @@ static av_cold int preinit(AVFilterContext *ctx)
>      if (ret < 0)
>          return ret;
>  
> +    if (ctx->filter == &ff_vf_scale2ref)
> +        ff_framesync_preinit(&scale->fs);
> +
>      return 0;
>  }
>  
> @@ -303,6 +308,8 @@ static const int sws_colorspaces[] = {
>      -1
>  };
>  
> +static int do_scale2ref(FFFrameSync *fs);
> +
>  static av_cold int init(AVFilterContext *ctx)
>  {
>      ScaleContext *scale = ctx->priv;
> @@ -380,6 +387,7 @@ static av_cold int init(AVFilterContext *ctx)
>      if (!threads)
>          av_opt_set_int(scale->sws_opts, "threads", ff_filter_get_nb_threads(ctx), 0);
>  
> +    scale->fs.on_event = do_scale2ref;
>      return 0;
>  }
>  
> @@ -389,6 +397,7 @@ static av_cold void uninit(AVFilterContext *ctx)
>      av_expr_free(scale->w_pexpr);
>      av_expr_free(scale->h_pexpr);
>      scale->w_pexpr = scale->h_pexpr = NULL;
> +    ff_framesync_uninit(&scale->fs);
>      sws_freeContext(scale->sws_opts);
>      sws_freeContext(scale->sws);
>      sws_freeContext(scale->isws[0]);
> @@ -678,35 +687,16 @@ static int config_props(AVFilterLink *outlink)
>             flags_val);
>      av_freep(&flags_val);
>  
> -    return 0;
> -
> -fail:
> -    return ret;
> -}
> -
> -static int config_props_ref(AVFilterLink *outlink)
> -{
> -    AVFilterLink *inlink = outlink->src->inputs[1];
> -
> -    outlink->w = inlink->w;
> -    outlink->h = inlink->h;
> -    outlink->sample_aspect_ratio = inlink->sample_aspect_ratio;
> -    outlink->time_base = inlink->time_base;
> -    outlink->frame_rate = inlink->frame_rate;
> -    outlink->colorspace = inlink->colorspace;
> -    outlink->color_range = inlink->color_range;
> +    if (ctx->filter != &ff_vf_scale2ref)
> +        return 0;
>  
> -    return 0;
> -}
> +    if ((ret = ff_framesync_init_dualinput(&scale->fs, ctx)) < 0)
> +        return ret;
>  
> -static int request_frame(AVFilterLink *outlink)
> -{
> -    return ff_request_frame(outlink->src->inputs[0]);
> -}
> +    return ff_framesync_configure(&scale->fs);
>  
> -static int request_frame_ref(AVFilterLink *outlink)
> -{
> -    return ff_request_frame(outlink->src->inputs[1]);
> +fail:
> +    return ret;
>  }
>  
>  static void frame_offset(AVFrame *frame, int dir, int is_pal)
> @@ -909,43 +899,49 @@ static int filter_frame(AVFilterLink *link, AVFrame *in)
>      return ret;
>  }
>  
> -static int filter_frame_ref(AVFilterLink *link, AVFrame *in)
> +static int do_scale2ref(FFFrameSync *fs)
>  {
> -    ScaleContext *scale = link->dst->priv;
> -    AVFilterLink *outlink = link->dst->outputs[1];
> -    int frame_changed;
> +    AVFilterContext *ctx = fs->parent;
> +    ScaleContext *scale = ctx->priv;
> +    AVFilterLink *reflink = ctx->inputs[1];
> +    AVFilterLink *outlink = ctx->outputs[0];
> +    AVFrame *main, *ref, *out;
> +    int ret;
>  
> -    frame_changed = in->width  != link->w ||
> -                    in->height != link->h ||
> -                    in->format != link->format ||
> -                    in->sample_aspect_ratio.den != link->sample_aspect_ratio.den ||
> -                    in->sample_aspect_ratio.num != link->sample_aspect_ratio.num ||
> -                    in->colorspace != link->colorspace ||
> -                    in->color_range != link->color_range;
> +    ret = ff_framesync_dualinput_get(fs, &main, &ref);
> +    if (ret < 0)
> +        return ret;
>  
> -    if (frame_changed) {
> -        link->format = in->format;
> -        link->w = in->width;
> -        link->h = in->height;
> -        link->sample_aspect_ratio.num = in->sample_aspect_ratio.num;
> -        link->sample_aspect_ratio.den = in->sample_aspect_ratio.den;
> -        link->colorspace = in->colorspace;
> -        link->color_range = in->color_range;
> +    if (ref) {
> +        reflink->format = ref->format;
> +        reflink->w = ref->width;
> +        reflink->h = ref->height;
> +        reflink->sample_aspect_ratio.num = ref->sample_aspect_ratio.num;
> +        reflink->sample_aspect_ratio.den = ref->sample_aspect_ratio.den;
> +        reflink->colorspace = ref->colorspace;
> +        reflink->color_range = ref->color_range;
>  
> -        config_props_ref(outlink);
> -    }
> +        ret = config_props(outlink);
> +        if (ret < 0)
> +            return ret;
>  
> -    if (scale->eval_mode == EVAL_MODE_FRAME) {
> -        scale->var_values[VAR_N] = link->frame_count_out;
> -        scale->var_values[VAR_T] = TS2T(in->pts, link->time_base);
> +        if (scale->eval_mode == EVAL_MODE_FRAME) {
> +            scale->var_values[VAR_N] = reflink->frame_count_out;
> +            scale->var_values[VAR_T] = TS2T(ref->pts, reflink->time_base);
>  #if FF_API_FRAME_PKT
>  FF_DISABLE_DEPRECATION_WARNINGS
> -        scale->var_values[VAR_POS] = in->pkt_pos == -1 ? NAN : in->pkt_pos;
> +            scale->var_values[VAR_POS] = ref->pkt_pos == -1 ? NAN : ref->pkt_pos;
>  FF_ENABLE_DEPRECATION_WARNINGS
>  #endif
> +        }
> +    }
> +
> +    ret = scale_frame(ctx->inputs[0], main, &out);
> +    if (out) {
> +        return ff_filter_frame(outlink, out);
>      }
>  
> -    return ff_filter_frame(outlink, in);
> +    return ret;
>  }
>  
>  static int process_command(AVFilterContext *ctx, const char *cmd, const char *args,
> @@ -973,9 +969,25 @@ static int process_command(AVFilterContext *ctx, const char *cmd, const char *ar
>      return ret;
>  }
>  
> +static int activate(AVFilterContext *ctx)
> +{
> +    ScaleContext *scale = ctx->priv;
> +    return ff_framesync_activate(&scale->fs);
> +}
> +
>  static const AVClass *child_class_iterate(void **iter)
>  {
> -    const AVClass *c = *iter ? NULL : sws_get_class();
> +    void *tmp = NULL;
> +    const AVClass *sws = sws_get_class();
> +    const AVClass *fs = ff_framesync_child_class_iterate(tmp);

This should be &tmp. It is probably the reason for Michael's segfault.
Apart from that: It is easier if you simply used 0..2 for *iter (1==
returned sws_get_class, 2 returned ff_framesync_child_class_iterate).

> +    const AVClass *c;
> +    if (!*iter) {
> +        c = sws;
> +    } else if (*iter == (void*)(uintptr_t)sws) {
> +        c = fs;
> +    } else {
> +        c = NULL;
> +    }
>      *iter = (void*)(uintptr_t)c;
>      return c;
>  }
> @@ -985,6 +997,8 @@ static void *child_next(void *obj, void *prev)
>      ScaleContext *s = obj;
>      if (!prev)
>          return s->sws_opts;
> +    if (prev == s->sws_opts)
> +        return &s->fs;
>      return NULL;
>  }
>  
> @@ -1082,12 +1096,10 @@ static const AVFilterPad avfilter_vf_scale2ref_inputs[] = {
>      {
>          .name         = "default",
>          .type         = AVMEDIA_TYPE_VIDEO,
> -        .filter_frame = filter_frame,
>      },
>      {
>          .name         = "ref",
>          .type         = AVMEDIA_TYPE_VIDEO,
> -        .filter_frame = filter_frame_ref,
>      },
>  };
>  
> @@ -1096,13 +1108,6 @@ static const AVFilterPad avfilter_vf_scale2ref_outputs[] = {
>          .name         = "default",
>          .type         = AVMEDIA_TYPE_VIDEO,
>          .config_props = config_props,
> -        .request_frame= request_frame,
> -    },
> -    {
> -        .name         = "ref",
> -        .type         = AVMEDIA_TYPE_VIDEO,
> -        .config_props = config_props_ref,
> -        .request_frame= request_frame_ref,
>      },
>  };
>  
> @@ -1117,5 +1122,6 @@ const AVFilter ff_vf_scale2ref = {
>      FILTER_INPUTS(avfilter_vf_scale2ref_inputs),
>      FILTER_OUTPUTS(avfilter_vf_scale2ref_outputs),
>      FILTER_QUERY_FUNC(query_formats),
> +    .activate        = activate,
>      .process_command = process_command,
>  };
> diff --git a/tests/filtergraphs/scale2ref_keep_aspect b/tests/filtergraphs/scale2ref_keep_aspect
> index f407460ec7c..d63968666a8 100644
> --- a/tests/filtergraphs/scale2ref_keep_aspect
> +++ b/tests/filtergraphs/scale2ref_keep_aspect
> @@ -1,5 +1,4 @@
>  sws_flags=+accurate_rnd+bitexact;
>  testsrc=size=320x240 [main];
>  testsrc=size=640x360 [ref];
> -[main][ref] scale2ref=iw/4:ow/mdar [main][ref];
> -[ref] nullsink
> +[main][ref] scale2ref=iw/4:ow/mdar [main]

_______________________________________________
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 2/2] avfilter/vf_scale2ref: switch to FFFrameSync

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 3260 bytes --]

On Wed, Mar 13, 2024 at 08:43:58PM -0300, James Almer wrote:
> On 3/13/2024 8:41 PM, Michael Niedermayer wrote:
> > On Wed, Mar 13, 2024 at 01:24:25PM +0100, Niklas Haas wrote:
> > > From: Niklas Haas <git@haasn.dev>
> > > 
> > > This filter's existing design has a number of issues:
> > > 
> > > - There is no guarantee whatsoever about the order in which frames are
> > >    pushed onto the main and ref link, due to this being entirely
> > >    dependent on the order in which downstream filters decide to request
> > >    frames from their various inputs. As such, there is absolutely no
> > >    synchronization for ref streams with dynamically changing resolutions
> > >    (see e.g. fate/h264/reinit-*).
> > > 
> > > - For some (likely historical) reason, this filter outputs its ref
> > >    stream as a second ref output, which is in principle completely
> > >    unnecessary (complex filter graph users can just duplicate the input
> > >    pin), but in practice only required to allow this filter to
> > >    "eventually" see changes to the ref stream (see first point). In
> > >    particular, this means that if the user uses the "wrong" pin, this
> > >    filter may break completely.
> > > 
> > > - The default filter activation function is fundamentally incapable of
> > >    handling filters with multiple inputs cleanly, because doing so
> > >    requires both knowledge of how these inputs should be internally
> > >    ordered, but also how to handle EOF conditions on either input (or
> > >    downstream). Both of these are best left to the filter specific
> > >    options. (See #10795 for the consequences of using the default
> > >    activate with multiple inputs).
> > > 
> > > Switching this filter to framesync fixes all three points:
> > > 
> > > - ff_framesync_activate() correctly handles multiple inputs and EOF
> > >    conditions (and is configurable with the framesync-specific options)
> > > - framesync only supports a single output, so we can (indeed must) drop
> > >    the redundant ref output stream
> > > 
> > > Update documentation, changelog and tests to correspond to the new usage
> > > pattern.
> > > 
> > > Fixes: https://trac.ffmpeg.org/ticket/10795
> > > ---
> > >   Changelog                                |   2 +
> > >   doc/filters.texi                         |  10 +-
> > >   libavfilter/vf_scale.c                   | 130 ++++++++++++-----------
> > >   tests/filtergraphs/scale2ref_keep_aspect |   3 +-
> > >   4 files changed, 76 insertions(+), 69 deletions(-)
> > 
> > this causes
> > ./ffplay --help
> > to segfault
> 
> Unrelated to this crash, but why does that command line output every single
> option from every single filter? It's several thousand printed lines.
> Shouldn't that be the output of --longhelp or similar, and leave --help to
> print some basic non filter specific options?

I think the help handling should be consistent between the tools, iam not sure
why it is not currently

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 2/2] avfilter/vf_scale2ref: switch to FFFrameSync

[Niklas Haas]

> This should be &tmp. It is probably the reason for Michael's segfault.
> Apart from that: It is easier if you simply used 0..2 for *iter (1==
> returned sws_get_class, 2 returned ff_framesync_child_class_iterate).

Changed this function to:

static const AVClass *child_class_iterate(void **iter)
{
    void *tmp = NULL;
    switch ((uintptr_t)*iter) {
    case 0:
        *iter = (void*)(uintptr_t)1;
        return sws_get_class();
    case 1:
        *iter = (void*)(uintptr_t)2;
        return ff_framesync_child_class_iterate(&tmp);
    }

    return NULL;
}
_______________________________________________
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 1/2] avfilter: mark scale2ref as supporting dynamic sizes

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 1323 bytes --]

On Wed, Mar 13, 2024 at 01:24:24PM +0100, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> Analogous to the "scale" filter, which this is practically identical
> with.
> ---
>  libavfilter/avfilter.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/libavfilter/avfilter.c b/libavfilter/avfilter.c
> index 831871de90b..dcad4d55292 100644
> --- a/libavfilter/avfilter.c
> +++ b/libavfilter/avfilter.c
> @@ -1027,7 +1027,8 @@ int ff_filter_frame(AVFilterLink *link, AVFrame *frame)
>              strcmp(link->dst->filter->name, "format") &&
>              strcmp(link->dst->filter->name, "idet") &&
>              strcmp(link->dst->filter->name, "null") &&
> -            strcmp(link->dst->filter->name, "scale")) {
> +            strcmp(link->dst->filter->name, "scale") &&
> +            strcmp(link->dst->filter->name, "scale2ref")) {
>              av_assert1(frame->format        == link->format);
>              av_assert1(frame->width         == link->w);
>              av_assert1(frame->height        == link->h);

LGTM

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Freedom in capitalist society always remains about the same as it was in
ancient Greek republics: Freedom for slave owners. -- Vladimir Lenin

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 2/2] avfilter/vf_scale2ref: switch to FFFrameSync

[Andreas Rheinhardt]

Niklas Haas:
>> This should be &tmp. It is probably the reason for Michael's segfault.
>> Apart from that: It is easier if you simply used 0..2 for *iter (1==
>> returned sws_get_class, 2 returned ff_framesync_child_class_iterate).
> 
> Changed this function to:
> 
> static const AVClass *child_class_iterate(void **iter)
> {
>     void *tmp = NULL;
>     switch ((uintptr_t)*iter) {
>     case 0:
>         *iter = (void*)(uintptr_t)1;
>         return sws_get_class();
>     case 1:
>         *iter = (void*)(uintptr_t)2;
>         return ff_framesync_child_class_iterate(&tmp);
>     }
> 
>     return NULL;
> }

Better make framesync_class un-static and return that directly.
Also: It makes no sense for vf_scale's child_class_iterate and
child_next to return framesync-objects (even a framesync context whose
AVClass* has not been set!); but this is what your patch does.

- Andreas

_______________________________________________
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 2/2] avfilter/vf_scale2ref: switch to FFFrameSync

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 3125 bytes --]

On Wed, Mar 13, 2024 at 01:24:25PM +0100, Niklas Haas wrote:
> From: Niklas Haas <git@haasn.dev>
> 
> This filter's existing design has a number of issues:
> 
> - There is no guarantee whatsoever about the order in which frames are
>   pushed onto the main and ref link, due to this being entirely
>   dependent on the order in which downstream filters decide to request
>   frames from their various inputs. As such, there is absolutely no
>   synchronization for ref streams with dynamically changing resolutions
>   (see e.g. fate/h264/reinit-*).
> 
> - For some (likely historical) reason, this filter outputs its ref
>   stream as a second ref output, which is in principle completely
>   unnecessary (complex filter graph users can just duplicate the input
>   pin), but in practice only required to allow this filter to
>   "eventually" see changes to the ref stream (see first point). In
>   particular, this means that if the user uses the "wrong" pin, this
>   filter may break completely.
> 
> - The default filter activation function is fundamentally incapable of
>   handling filters with multiple inputs cleanly, because doing so
>   requires both knowledge of how these inputs should be internally
>   ordered, but also how to handle EOF conditions on either input (or
>   downstream). Both of these are best left to the filter specific
>   options. (See #10795 for the consequences of using the default
>   activate with multiple inputs).
> 
> Switching this filter to framesync fixes all three points:
> 
> - ff_framesync_activate() correctly handles multiple inputs and EOF
>   conditions (and is configurable with the framesync-specific options)
> - framesync only supports a single output, so we can (indeed must) drop
>   the redundant ref output stream
> 
> Update documentation, changelog and tests to correspond to the new usage
> pattern.
> 
> Fixes: https://trac.ffmpeg.org/ticket/10795
> ---
>  Changelog                                |   2 +
>  doc/filters.texi                         |  10 +-
>  libavfilter/vf_scale.c                   | 130 ++++++++++++-----------
>  tests/filtergraphs/scale2ref_keep_aspect |   3 +-
>  4 files changed, 76 insertions(+), 69 deletions(-)
> 
> diff --git a/Changelog b/Changelog
> index 069b8274489..bacda2524ea 100644
> --- a/Changelog
> +++ b/Changelog
> @@ -32,6 +32,8 @@ version <next>:
>  - DVD-Video demuxer, powered by libdvdnav and libdvdread
>  - ffprobe -show_stream_groups option
>  - ffprobe (with -export_side_data film_grain) now prints film grain metadata
> +- scale2ref now only has a single output stream, and supports the framesync
> +  options

is it neccessary to drop compatibility to the old syntax ?

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

If you drop bombs on a foreign country and kill a hundred thousand
innocent people, expect your government to call the consequence
"unprovoked inhuman terrorist attacks" and use it to justify dropping
more bombs and killing more people. The technology changed, the idea is old.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 2/2] avfilter/vf_scale2ref: switch to FFFrameSync

[Niklas Haas]

On Tue, 19 Mar 2024 22:55:56 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote:
> is it neccessary to drop compatibility to the old syntax ?

Only if we want to use framesync. If we don't care about synchronizing
the streams, then we could drop FS and just use a custom activate
function which forwards output status only to the *corresponding* input.

Maybe also with printing a warning if the refstream parameters do happen
to change, since at that point we know it wouldn't be in sync.
_______________________________________________
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 2/2] avfilter/vf_scale2ref: switch to FFFrameSync

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 784 bytes --]

On Wed, Mar 20, 2024 at 03:23:53PM +0100, Niklas Haas wrote:
> On Tue, 19 Mar 2024 22:55:56 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote:
> > is it neccessary to drop compatibility to the old syntax ?
> 
> Only if we want to use framesync. If we don't care about synchronizing
> the streams, then we could drop FS and just use a custom activate
> function which forwards output status only to the *corresponding* input.

if the full bugfix cant be done in the same filter then i guess we need a 2nd
filter, maybe scale1ref

[...]

thx

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The smallest minority on earth is the individual. Those who deny 
individual rights cannot claim to be defenders of minorities. - Ayn Rand

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

_______________________________________________
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 2/2] avfilter/vf_scale2ref: switch to FFFrameSync

[Niklas Haas]

On Wed, 20 Mar 2024 20:55:26 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote:
> On Wed, Mar 20, 2024 at 03:23:53PM +0100, Niklas Haas wrote:
> > On Tue, 19 Mar 2024 22:55:56 +0100 Michael Niedermayer <michael@niedermayer.cc> wrote:
> > > is it neccessary to drop compatibility to the old syntax ?
> > 
> > Only if we want to use framesync. If we don't care about synchronizing
> > the streams, then we could drop FS and just use a custom activate
> > function which forwards output status only to the *corresponding* input.
> 
> if the full bugfix cant be done in the same filter then i guess we need a 2nd
> filter, maybe scale1ref

I think we can do this by extending vf_scale itself to support taking
a second input, that way we don't need a second filter at all.

My proposal for this:
- Add new variables ref_w, ref_h (and related fields), possibly aliased
  to 'rw' and 'rh'.
- When `ref_*` is referenced in any expression, vf_scale will init()
  with a second input stream.
- Semantics will be equivalent to this patch.
- Fully deprecate scale2ref, and eventually remove it and the main_*
  series of variables.

That way, scale2ref just becomes `scale=w=rw:h=rh`, and also allows us
to simplify these filters a bit.
_______________________________________________
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".