[FFmpeg-devel] [PATCH 2/2] avfilter/vf_scale2ref: switch to FFFrameSync
Andreas Rheinhardt
andreas.rheinhardt at outlook.com
Thu Mar 14 01:47:52 EET 2024
Niklas Haas:
> From: Niklas Haas <git at 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
> - at 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]
More information about the ffmpeg-devel
mailing list