[FFmpeg-devel] [PATCH] avfilter/scale: allow dynamic output via expressions
Michael Niedermayer
michael at niedermayer.cc
Fri Nov 15 00:31:08 EET 2019
On Thu, Nov 14, 2019 at 11:12:23PM +0530, Gyan wrote:
>
>
> On 14-11-2019 01:12 am, Michael Niedermayer wrote:
> >Moving and changing code at the same time makes it hard to see th difference.
> >Idealy all code moves should be seperate from changes to the code.
> >
> >also more generally, spliting this patch up would simpify review
>
> Split into two. First patch mostly moves code and keeps existing
> functionality. 2nd patch introduces new features and requires the new eval
> function.
>
> Thanks,
> Gyan
> Makefile | 4 -
> scale.c | 72 +---------------------
> vf_scale.c | 192 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 196 insertions(+), 72 deletions(-)
> 77579fdbd7add3be08bada5ee401df41f60ea236 v2-0001-avfilter-scale-shift-ff_scale_eval_dimensions-inl.patch
> From 359f538703865c8ebeda16b5d1846d2cf1cf9c4d Mon Sep 17 00:00:00 2001
> From: Gyan Doshi <ffmpeg at gyani.pro>
> Date: Thu, 14 Nov 2019 21:08:32 +0530
> Subject: [PATCH v2 1/2] avfilter/scale: shift ff_scale_eval_dimensions inline
>
> This is a perfunctory change in preparation of adding
> direct animation support to scale and scale2ref filters
> ---
> libavfilter/Makefile | 4 +-
> libavfilter/scale.c | 72 +---------------
> libavfilter/vf_scale.c | 192 ++++++++++++++++++++++++++++++++++++++++-
> 3 files changed, 196 insertions(+), 72 deletions(-)
>
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index fce930360d..f1f6994574 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -358,12 +358,12 @@ OBJS-$(CONFIG_ROBERTS_OPENCL_FILTER) += vf_convolution_opencl.o opencl.o
> opencl/convolution.o
> OBJS-$(CONFIG_ROTATE_FILTER) += vf_rotate.o
> OBJS-$(CONFIG_SAB_FILTER) += vf_sab.o
> -OBJS-$(CONFIG_SCALE_FILTER) += vf_scale.o scale.o
> +OBJS-$(CONFIG_SCALE_FILTER) += vf_scale.o
> OBJS-$(CONFIG_SCALE_CUDA_FILTER) += vf_scale_cuda.o vf_scale_cuda.ptx.o
> OBJS-$(CONFIG_SCALE_NPP_FILTER) += vf_scale_npp.o scale.o
> OBJS-$(CONFIG_SCALE_QSV_FILTER) += vf_scale_qsv.o
> OBJS-$(CONFIG_SCALE_VAAPI_FILTER) += vf_scale_vaapi.o scale.o vaapi_vpp.o
> -OBJS-$(CONFIG_SCALE2REF_FILTER) += vf_scale.o scale.o
> +OBJS-$(CONFIG_SCALE2REF_FILTER) += vf_scale.o
> OBJS-$(CONFIG_SCROLL_FILTER) += vf_scroll.o
> OBJS-$(CONFIG_SELECT_FILTER) += f_select.o
> OBJS-$(CONFIG_SELECTIVECOLOR_FILTER) += vf_selectivecolor.o
> diff --git a/libavfilter/scale.c b/libavfilter/scale.c
> index eaee95fac6..668aa04622 100644
> --- a/libavfilter/scale.c
> +++ b/libavfilter/scale.c
> @@ -60,49 +60,6 @@ enum var_name {
> VARS_NB
> };
>
> -/**
> - * This must be kept in sync with var_names so that it is always a
> - * complete list of var_names with the scale2ref specific names
> - * appended. scale2ref values must appear in the order they appear
> - * in the var_name_scale2ref enum but also be below all of the
> - * non-scale2ref specific values.
> - */
> -static const char *const var_names_scale2ref[] = {
> - "PI",
> - "PHI",
> - "E",
> - "in_w", "iw",
> - "in_h", "ih",
> - "out_w", "ow",
> - "out_h", "oh",
> - "a",
> - "sar",
> - "dar",
> - "hsub",
> - "vsub",
> - "ohsub",
> - "ovsub",
> - "main_w",
> - "main_h",
> - "main_a",
> - "main_sar",
> - "main_dar", "mdar",
> - "main_hsub",
> - "main_vsub",
> - NULL
> -};
> -
> -enum var_name_scale2ref {
> - VAR_S2R_MAIN_W,
> - VAR_S2R_MAIN_H,
> - VAR_S2R_MAIN_A,
> - VAR_S2R_MAIN_SAR,
> - VAR_S2R_MAIN_DAR, VAR_S2R_MDAR,
> - VAR_S2R_MAIN_HSUB,
> - VAR_S2R_MAIN_VSUB,
> - VARS_S2R_NB
> -};
> -
> int ff_scale_eval_dimensions(void *log_ctx,
> const char *w_expr, const char *h_expr,
> AVFilterLink *inlink, AVFilterLink *outlink,
> @@ -115,16 +72,7 @@ int ff_scale_eval_dimensions(void *log_ctx,
> int factor_w, factor_h;
> int eval_w, eval_h;
> int ret;
> - const char scale2ref = outlink->src->nb_inputs == 2 && outlink->src->inputs[1] == inlink;
> - double var_values[VARS_NB + VARS_S2R_NB], res;
> - const AVPixFmtDescriptor *main_desc;
> - const AVFilterLink *main_link;
> - const char *const *names = scale2ref ? var_names_scale2ref : var_names;
> -
> - if (scale2ref) {
> - main_link = outlink->src->inputs[0];
> - main_desc = av_pix_fmt_desc_get(main_link->format);
> - }
> + double var_values[VARS_NB], res;
>
> var_values[VAR_PI] = M_PI;
> var_values[VAR_PHI] = M_PHI;
> @@ -142,32 +90,20 @@ int ff_scale_eval_dimensions(void *log_ctx,
> var_values[VAR_OHSUB] = 1 << out_desc->log2_chroma_w;
> var_values[VAR_OVSUB] = 1 << out_desc->log2_chroma_h;
>
> - if (scale2ref) {
> - var_values[VARS_NB + VAR_S2R_MAIN_W] = main_link->w;
> - var_values[VARS_NB + VAR_S2R_MAIN_H] = main_link->h;
> - var_values[VARS_NB + VAR_S2R_MAIN_A] = (double) main_link->w / main_link->h;
> - var_values[VARS_NB + VAR_S2R_MAIN_SAR] = main_link->sample_aspect_ratio.num ?
> - (double) main_link->sample_aspect_ratio.num / main_link->sample_aspect_ratio.den : 1;
> - var_values[VARS_NB + VAR_S2R_MAIN_DAR] = var_values[VARS_NB + VAR_S2R_MDAR] =
> - var_values[VARS_NB + VAR_S2R_MAIN_A] * var_values[VARS_NB + VAR_S2R_MAIN_SAR];
> - var_values[VARS_NB + VAR_S2R_MAIN_HSUB] = 1 << main_desc->log2_chroma_w;
> - var_values[VARS_NB + VAR_S2R_MAIN_VSUB] = 1 << main_desc->log2_chroma_h;
> - }
> -
> /* evaluate width and height */
> av_expr_parse_and_eval(&res, (expr = w_expr),
> - names, var_values,
> + var_names, var_values,
> NULL, NULL, NULL, NULL, NULL, 0, log_ctx);
> eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ? inlink->w : (int) res;
>
> if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr),
> - names, var_values,
> + var_names, var_values,
> NULL, NULL, NULL, NULL, NULL, 0, log_ctx)) < 0)
> goto fail;
> eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = (int) res == 0 ? inlink->h : (int) res;
> /* evaluate again the width, as it may depend on the output height */
> if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr),
> - names, var_values,
> + var_names, var_values,
> NULL, NULL, NULL, NULL, NULL, 0, log_ctx)) < 0)
> goto fail;
> eval_w = (int) res == 0 ? inlink->w : (int) res;
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index 41ddec7661..e7d52faccc 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -29,9 +29,9 @@
> #include "avfilter.h"
> #include "formats.h"
> #include "internal.h"
> -#include "scale.h"
> #include "video.h"
> #include "libavutil/avstring.h"
> +#include "libavutil/eval.h"
> #include "libavutil/internal.h"
> #include "libavutil/mathematics.h"
> #include "libavutil/opt.h"
> @@ -41,6 +41,85 @@
> #include "libavutil/avassert.h"
> #include "libswscale/swscale.h"
>
> +static const char *const var_names[] = {
> + "PI",
> + "PHI",
> + "E",
> + "in_w", "iw",
> + "in_h", "ih",
> + "out_w", "ow",
> + "out_h", "oh",
> + "a",
> + "sar",
> + "dar",
> + "hsub",
> + "vsub",
> + "ohsub",
> + "ovsub",
> + NULL
> +};
> +
> +enum var_name {
> + VAR_PI,
> + VAR_PHI,
> + VAR_E,
> + VAR_IN_W, VAR_IW,
> + VAR_IN_H, VAR_IH,
> + VAR_OUT_W, VAR_OW,
> + VAR_OUT_H, VAR_OH,
> + VAR_A,
> + VAR_SAR,
> + VAR_DAR,
> + VAR_HSUB,
> + VAR_VSUB,
> + VAR_OHSUB,
> + VAR_OVSUB,
> + VARS_NB
[...]
> +static int scale_eval_dimensions(void *log_ctx,
> + const char *w_expr, const char *h_expr,
> + AVFilterLink *inlink, AVFilterLink *outlink,
> + int *ret_w, int *ret_h)
> +{
> + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
> + const AVPixFmtDescriptor *out_desc = av_pix_fmt_desc_get(outlink->format);
> + const char *expr;
> + int w, h;
> + int factor_w, factor_h;
> + int eval_w, eval_h;
> + int ret;
> + const char scale2ref = outlink->src->nb_inputs == 2 && outlink->src->inputs[1] == inlink;
> + double var_values[VARS_NB + VARS_S2R_NB], res;
> + const AVPixFmtDescriptor *main_desc;
> + const AVFilterLink *main_link;
> + const char *const *names = scale2ref ? var_names_scale2ref : var_names;
> +
> + if (scale2ref) {
> + main_link = outlink->src->inputs[0];
> + main_desc = av_pix_fmt_desc_get(main_link->format);
> + }
> +
> + var_values[VAR_PI] = M_PI;
> + var_values[VAR_PHI] = M_PHI;
> + var_values[VAR_E] = M_E;
> + var_values[VAR_IN_W] = var_values[VAR_IW] = inlink->w;
> + var_values[VAR_IN_H] = var_values[VAR_IH] = inlink->h;
> + var_values[VAR_OUT_W] = var_values[VAR_OW] = NAN;
> + var_values[VAR_OUT_H] = var_values[VAR_OH] = NAN;
> + var_values[VAR_A] = (double) inlink->w / inlink->h;
> + var_values[VAR_SAR] = inlink->sample_aspect_ratio.num ?
> + (double) inlink->sample_aspect_ratio.num / inlink->sample_aspect_ratio.den : 1;
> + var_values[VAR_DAR] = var_values[VAR_A] * var_values[VAR_SAR];
> + var_values[VAR_HSUB] = 1 << desc->log2_chroma_w;
> + var_values[VAR_VSUB] = 1 << desc->log2_chroma_h;
> + var_values[VAR_OHSUB] = 1 << out_desc->log2_chroma_w;
> + var_values[VAR_OVSUB] = 1 << out_desc->log2_chroma_h;
> +
> + if (scale2ref) {
> + var_values[VARS_NB + VAR_S2R_MAIN_W] = main_link->w;
> + var_values[VARS_NB + VAR_S2R_MAIN_H] = main_link->h;
> + var_values[VARS_NB + VAR_S2R_MAIN_A] = (double) main_link->w / main_link->h;
> + var_values[VARS_NB + VAR_S2R_MAIN_SAR] = main_link->sample_aspect_ratio.num ?
> + (double) main_link->sample_aspect_ratio.num / main_link->sample_aspect_ratio.den : 1;
> + var_values[VARS_NB + VAR_S2R_MAIN_DAR] = var_values[VARS_NB + VAR_S2R_MDAR] =
> + var_values[VARS_NB + VAR_S2R_MAIN_A] * var_values[VARS_NB + VAR_S2R_MAIN_SAR];
> + var_values[VARS_NB + VAR_S2R_MAIN_HSUB] = 1 << main_desc->log2_chroma_w;
> + var_values[VARS_NB + VAR_S2R_MAIN_VSUB] = 1 << main_desc->log2_chroma_h;
> + }
> +
> + /* evaluate width and height */
> + av_expr_parse_and_eval(&res, (expr = w_expr),
> + names, var_values,
> + NULL, NULL, NULL, NULL, NULL, 0, log_ctx);
> + eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ? inlink->w : (int) res;
> +
> + if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr),
> + names, var_values,
> + NULL, NULL, NULL, NULL, NULL, 0, log_ctx)) < 0)
> + goto fail;
> + eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = (int) res == 0 ? inlink->h : (int) res;
> + /* evaluate again the width, as it may depend on the output height */
> + if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr),
> + names, var_values,
> + NULL, NULL, NULL, NULL, NULL, 0, log_ctx)) < 0)
> + goto fail;
> + eval_w = (int) res == 0 ? inlink->w : (int) res;
> +
> + w = eval_w;
> + h = eval_h;
> +
> + /* Check if it is requested that the result has to be divisible by a some
> + * factor (w or h = -n with n being the factor). */
> + factor_w = 1;
> + factor_h = 1;
> + if (w < -1) {
> + factor_w = -w;
> + }
> + if (h < -1) {
> + factor_h = -h;
> + }
> +
> + if (w < 0 && h < 0) {
> + w = inlink->w;
> + h = inlink->h;
> + }
> +
> + /* Make sure that the result is divisible by the factor we determined
> + * earlier. If no factor was set, it is nothing will happen as the default
> + * factor is 1 */
> + if (w < 0)
> + w = av_rescale(h, inlink->w, inlink->h * factor_w) * factor_w;
> + if (h < 0)
> + h = av_rescale(w, inlink->h, inlink->w * factor_h) * factor_h;
> +
> + *ret_w = w;
> + *ret_h = h;
> +
> + return 0;
> +
> +fail:
> + av_log(log_ctx, AV_LOG_ERROR,
> + "Error when evaluating the expression '%s'.\n"
> + "Maybe the expression for out_w:'%s' or for out_h:'%s' is self-referencing.\n",
> + expr, w_expr, h_expr);
> + return ret;
> +}
duplicated code
it would be more ideal if no code duplication is required
> +
> static int config_props(AVFilterLink *outlink)
> {
> AVFilterContext *ctx = outlink->src;
> @@ -231,7 +419,7 @@ static int config_props(AVFilterLink *outlink)
> int w, h;
> int ret;
>
> - if ((ret = ff_scale_eval_dimensions(ctx,
> + if ((ret = scale_eval_dimensions(ctx,
> scale->w_expr, scale->h_expr,
> inlink, outlink,
> &w, &h)) < 0)
> --
> 2.24.0
> vf_scale.c | 466 ++++++++++++++++++++++++++++++++++++++++++-------------------
> 1 file changed, 323 insertions(+), 143 deletions(-)
> 942c421f637c3bde95f12b7c32b9c674e310d9c0 v2-0002-avfilter-scale-allow-dynamic-output-via-expressio.patch
> From 001a16725336b657a9b88c82ebca046bfa16341d Mon Sep 17 00:00:00 2001
> From: Gyan Doshi <ffmpeg at gyani.pro>
> Date: Thu, 14 Nov 2019 23:03:39 +0530
> Subject: [PATCH v2 2/2] avfilter/scale: allow dynamic output via expressions
>
> At present, dynamic output can be had from scale and scale2ref filters
> by sending commands or varying the resolution of the input (in scale) or
> reference stream (in scale2ref). This is unwieldy when an interpolated
> or otherwise animated output is desired.
>
> This commit introduces standard temporal variables that can be used in
> width and height expressions, so dynamic output can be configured at
> initialization or via commands.
>
> Such varying output ought to be sent to filters which can accommodate
> them, such as the overlay filter.
> ---
> libavfilter/vf_scale.c | 466 ++++++++++++++++++++++++++++-------------
> 1 file changed, 323 insertions(+), 143 deletions(-)
>
> diff --git a/libavfilter/vf_scale.c b/libavfilter/vf_scale.c
> index e7d52faccc..cbacd34827 100644
> --- a/libavfilter/vf_scale.c
> +++ b/libavfilter/vf_scale.c
> @@ -42,9 +42,6 @@
> #include "libswscale/swscale.h"
>
> static const char *const var_names[] = {
> - "PI",
> - "PHI",
> - "E",
> "in_w", "iw",
> "in_h", "ih",
> "out_w", "ow",
> @@ -56,13 +53,23 @@ static const char *const var_names[] = {
> "vsub",
> "ohsub",
> "ovsub",
> + "n",
> + "t",
> + "pos",
> + "main_w",
> + "main_h",
> + "main_a",
> + "main_sar",
> + "main_dar", "mdar",
> + "main_hsub",
> + "main_vsub",
> + "main_n",
> + "main_t",
> + "main_pos",
> NULL
> };
>
> enum var_name {
> - VAR_PI,
> - VAR_PHI,
> - VAR_E,
> VAR_IN_W, VAR_IW,
> VAR_IN_H, VAR_IH,
> VAR_OUT_W, VAR_OW,
> @@ -74,42 +81,9 @@ enum var_name {
> VAR_VSUB,
> VAR_OHSUB,
> VAR_OVSUB,
> - VARS_NB
> -};
> -
> -/**
> - * This must be kept in sync with var_names so that it is always a
> - * complete list of var_names with the scale2ref specific names
> - * appended. scale2ref values must appear in the order they appear
> - * in the var_name_scale2ref enum but also be below all of the
> - * non-scale2ref specific values.
> - */
> -static const char *const var_names_scale2ref[] = {
> - "PI",
> - "PHI",
> - "E",
> - "in_w", "iw",
> - "in_h", "ih",
> - "out_w", "ow",
> - "out_h", "oh",
> - "a",
> - "sar",
> - "dar",
> - "hsub",
> - "vsub",
> - "ohsub",
> - "ovsub",
> - "main_w",
> - "main_h",
> - "main_a",
> - "main_sar",
> - "main_dar", "mdar",
> - "main_hsub",
> - "main_vsub",
> - NULL
> -};
> -
> -enum var_name_scale2ref {
> + VAR_N,
> + VAR_T,
> + VAR_POS,
> VAR_S2R_MAIN_W,
> VAR_S2R_MAIN_H,
> VAR_S2R_MAIN_A,
> @@ -117,7 +91,10 @@ enum var_name_scale2ref {
> VAR_S2R_MAIN_DAR, VAR_S2R_MDAR,
> VAR_S2R_MAIN_HSUB,
> VAR_S2R_MAIN_VSUB,
> - VARS_S2R_NB
> + VAR_S2R_MAIN_N,
> + VAR_S2R_MAIN_T,
> + VAR_S2R_MAIN_POS,
> + VARS_NB
> };
>
> enum EvalMode {
> @@ -151,6 +128,10 @@ typedef struct ScaleContext {
>
> char *w_expr; ///< width expression string
> char *h_expr; ///< height expression string
> + AVExpr *w_pexpr;
> + AVExpr *h_pexpr;
> + double var_values[VARS_NB];
> +
> char *flags_str;
>
> char *in_color_matrix;
> @@ -175,6 +156,8 @@ typedef struct ScaleContext {
>
> AVFilter ff_vf_scale2ref;
>
> +static int check_exprs(AVFilterContext *ctx);
The function could be put here avoiding the need for the forward declaration
> +
> static av_cold int init_dict(AVFilterContext *ctx, AVDictionary **opts)
> {
> ScaleContext *scale = ctx->priv;
> @@ -186,8 +169,10 @@ static av_cold int init_dict(AVFilterContext *ctx, AVDictionary **opts)
> return AVERROR(EINVAL);
> }
>
> - if (scale->w_expr && !scale->h_expr)
> + if (scale->w_expr && !scale->h_expr) {
> + av_log(ctx, AV_LOG_INFO, "No height expression found. Treating width expression as video size.\n");
> FFSWAP(char *, scale->w_expr, scale->size_str);
> + }
>
> if (scale->size_str) {
> char buf[32];
> @@ -206,6 +191,26 @@ static av_cold int init_dict(AVFilterContext *ctx, AVDictionary **opts)
> if (!scale->h_expr)
> av_opt_set(scale, "h", "ih", 0);
>
> + ret = av_expr_parse(&scale->w_pexpr, scale->w_expr,
> + var_names,
> + NULL, NULL, NULL, NULL, 0, ctx);
> + if (ret < 0) {
> + av_log(ctx, AV_LOG_ERROR, "Cannot parse width expression: '%s'\n", scale->w_expr);
> + return ret;
> + }
> +
> + ret = av_expr_parse(&scale->h_pexpr, scale->h_expr,
> + var_names,
> + NULL, NULL, NULL, NULL, 0, ctx);
> + if (ret < 0) {
> + av_log(ctx, AV_LOG_ERROR, "Cannot parse height expression: '%s'\n", scale->h_expr);
> + return ret;
> + }
> +
> + ret = check_exprs(ctx);
> + if (ret < 0)
> + return ret;
> +
> av_log(ctx, AV_LOG_VERBOSE, "w:%s h:%s flags:'%s' interl:%d\n",
> scale->w_expr, scale->h_expr, (char *)av_x_if_null(scale->flags_str, ""), scale->interlaced);
>
> @@ -228,6 +233,9 @@ static av_cold int init_dict(AVFilterContext *ctx, AVDictionary **opts)
> static av_cold void uninit(AVFilterContext *ctx)
> {
> ScaleContext *scale = ctx->priv;
> + av_expr_free(scale->w_pexpr);
> + av_expr_free(scale->h_pexpr);
> + scale->w_pexpr = scale->h_pexpr = NULL;
> sws_freeContext(scale->sws);
> sws_freeContext(scale->isws[0]);
> sws_freeContext(scale->isws[1]);
> @@ -273,6 +281,45 @@ static int query_formats(AVFilterContext *ctx)
> return 0;
> }
>
> +static int check_exprs(AVFilterContext *ctx)
> +{
> + ScaleContext *scale = ctx->priv;
> +
> + if (av_expr_count_var(scale->w_pexpr, VAR_OUT_W, VAR_OW+1)) {
> + av_log(ctx, AV_LOG_ERROR, "Width expression cannot be self-referencing: '%s'.\n", scale->w_expr);
> + return AVERROR(EINVAL);
> + }
This is not very readable because it does not list the variables it counts
instead it lists the first counted and the first not counted relying on
the ordering in the enum.
> +
> + if (av_expr_count_var(scale->h_pexpr, VAR_OUT_H, VAR_OH+1)) {
looking at how this is used here
a simpler and cleaner API might be to pass an empty array to
av_expr_count_var()
and on return have a count for each variable in it.
var_use_count[VAR_OUT_W] is much clearer what it refers to. The reader doesnt need
to know the ordering of elements in the enum
> + av_log(ctx, AV_LOG_ERROR, "Height expression cannot be self-referencing: '%s'.\n", scale->h_expr);
> + return AVERROR(EINVAL);
> + }
> +
> + if (av_expr_count_var(scale->w_pexpr, VAR_OUT_H, VAR_OH+1) &&
> + av_expr_count_var(scale->h_pexpr, VAR_OUT_W, VAR_OW+1)) {
> + av_log(ctx, AV_LOG_ERROR, "Circular expressions invalid for width '%s' and height '%s'.\n", scale->w_expr, scale->h_expr);
> + return AVERROR(EINVAL);
> + }
> +
> + if (ctx->filter != &ff_vf_scale2ref &&
> + (av_expr_count_var(scale->w_pexpr, VAR_S2R_MAIN_W, VARS_NB) ||
> + av_expr_count_var(scale->h_pexpr, VAR_S2R_MAIN_W, VARS_NB))) {
> + av_log(ctx, AV_LOG_ERROR, "Expressions with scale2ref variables are not valid in scale filter.\n");
> + return AVERROR(EINVAL);
> + }
> +
> + if (scale->eval_mode == EVAL_MODE_INIT &&
> + (av_expr_count_var(scale->w_pexpr, VAR_N, VAR_POS+1) ||
> + av_expr_count_var(scale->w_pexpr, VAR_S2R_MAIN_N, VAR_S2R_MAIN_POS+1) ||
> + av_expr_count_var(scale->h_pexpr, VAR_N, VAR_POS+1) ||
> + av_expr_count_var(scale->h_pexpr, VAR_S2R_MAIN_N, VAR_S2R_MAIN_POS+1))) {
> + av_log(ctx, AV_LOG_ERROR, "Expressions with frame variables 'n', 't', 'pos' are not valid in init eval_mode.\n");
> + return AVERROR(EINVAL);
> + }
> +
> + return 0;
> +}
Addition of expression checking should be independant of other features
and so could be in its own patch
> +
> static const int *parse_yuv_type(const char *s, enum AVColorSpace colorspace)
> {
> if (!s)
> @@ -297,74 +344,78 @@ static const int *parse_yuv_type(const char *s, enum AVColorSpace colorspace)
> return sws_getCoefficients(colorspace);
> }
>
> -static int scale_eval_dimensions(void *log_ctx,
> - const char *w_expr, const char *h_expr,
> - AVFilterLink *inlink, AVFilterLink *outlink,
> - int *ret_w, int *ret_h)
> +static int scale_eval_dimensions(AVFilterContext *ctx)
> {
> + ScaleContext *scale = ctx->priv;
> + const char scale2ref = ctx->filter == &ff_vf_scale2ref;
> + const AVFilterLink *inlink = scale2ref ? ctx->inputs[1] : ctx->inputs[0];
> + const AVFilterLink *outlink = ctx->outputs[0];
> const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
> const AVPixFmtDescriptor *out_desc = av_pix_fmt_desc_get(outlink->format);
> - const char *expr;
> + char *expr;
> int w, h;
> int factor_w, factor_h;
> int eval_w, eval_h;
> int ret;
> - const char scale2ref = outlink->src->nb_inputs == 2 && outlink->src->inputs[1] == inlink;
> - double var_values[VARS_NB + VARS_S2R_NB], res;
> +
> + double res;
> const AVPixFmtDescriptor *main_desc;
> const AVFilterLink *main_link;
> - const char *const *names = scale2ref ? var_names_scale2ref : var_names;
>
> if (scale2ref) {
> - main_link = outlink->src->inputs[0];
> + main_link = ctx->inputs[0];
> main_desc = av_pix_fmt_desc_get(main_link->format);
> }
>
> - var_values[VAR_PI] = M_PI;
> - var_values[VAR_PHI] = M_PHI;
> - var_values[VAR_E] = M_E;
> - var_values[VAR_IN_W] = var_values[VAR_IW] = inlink->w;
> - var_values[VAR_IN_H] = var_values[VAR_IH] = inlink->h;
> - var_values[VAR_OUT_W] = var_values[VAR_OW] = NAN;
> - var_values[VAR_OUT_H] = var_values[VAR_OH] = NAN;
> - var_values[VAR_A] = (double) inlink->w / inlink->h;
> - var_values[VAR_SAR] = inlink->sample_aspect_ratio.num ?
> + scale->var_values[VAR_IN_W] = scale->var_values[VAR_IW] = inlink->w;
> + scale->var_values[VAR_IN_H] = scale->var_values[VAR_IH] = inlink->h;
> + scale->var_values[VAR_OUT_W] = scale->var_values[VAR_OW] = NAN;
> + scale->var_values[VAR_OUT_H] = scale->var_values[VAR_OH] = NAN;
> + scale->var_values[VAR_A] = (double) inlink->w / inlink->h;
> + scale->var_values[VAR_SAR] = inlink->sample_aspect_ratio.num ?
> (double) inlink->sample_aspect_ratio.num / inlink->sample_aspect_ratio.den : 1;
> - var_values[VAR_DAR] = var_values[VAR_A] * var_values[VAR_SAR];
> - var_values[VAR_HSUB] = 1 << desc->log2_chroma_w;
> - var_values[VAR_VSUB] = 1 << desc->log2_chroma_h;
> - var_values[VAR_OHSUB] = 1 << out_desc->log2_chroma_w;
> - var_values[VAR_OVSUB] = 1 << out_desc->log2_chroma_h;
> + scale->var_values[VAR_DAR] = scale->var_values[VAR_A] * scale->var_values[VAR_SAR];
> + scale->var_values[VAR_HSUB] = 1 << desc->log2_chroma_w;
> + scale->var_values[VAR_VSUB] = 1 << desc->log2_chroma_h;
> + scale->var_values[VAR_OHSUB] = 1 << out_desc->log2_chroma_w;
> + scale->var_values[VAR_OVSUB] = 1 << out_desc->log2_chroma_h;
>
> if (scale2ref) {
> - var_values[VARS_NB + VAR_S2R_MAIN_W] = main_link->w;
> - var_values[VARS_NB + VAR_S2R_MAIN_H] = main_link->h;
> - var_values[VARS_NB + VAR_S2R_MAIN_A] = (double) main_link->w / main_link->h;
> - var_values[VARS_NB + VAR_S2R_MAIN_SAR] = main_link->sample_aspect_ratio.num ?
> + scale->var_values[VAR_S2R_MAIN_W] = main_link->w;
> + scale->var_values[VAR_S2R_MAIN_H] = main_link->h;
> + scale->var_values[VAR_S2R_MAIN_A] = (double) main_link->w / main_link->h;
> + scale->var_values[VAR_S2R_MAIN_SAR] = main_link->sample_aspect_ratio.num ?
> (double) main_link->sample_aspect_ratio.num / main_link->sample_aspect_ratio.den : 1;
> - var_values[VARS_NB + VAR_S2R_MAIN_DAR] = var_values[VARS_NB + VAR_S2R_MDAR] =
> - var_values[VARS_NB + VAR_S2R_MAIN_A] * var_values[VARS_NB + VAR_S2R_MAIN_SAR];
> - var_values[VARS_NB + VAR_S2R_MAIN_HSUB] = 1 << main_desc->log2_chroma_w;
> - var_values[VARS_NB + VAR_S2R_MAIN_VSUB] = 1 << main_desc->log2_chroma_h;
> + scale->var_values[VAR_S2R_MAIN_DAR] = scale->var_values[VAR_S2R_MDAR] =
> + scale->var_values[VAR_S2R_MAIN_A] * scale->var_values[VAR_S2R_MAIN_SAR];
> + scale->var_values[VAR_S2R_MAIN_HSUB] = 1 << main_desc->log2_chroma_w;
> + scale->var_values[VAR_S2R_MAIN_VSUB] = 1 << main_desc->log2_chroma_h;
> }
>
> - /* evaluate width and height */
> - av_expr_parse_and_eval(&res, (expr = w_expr),
> - names, var_values,
> - NULL, NULL, NULL, NULL, NULL, 0, log_ctx);
> - eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = (int) res == 0 ? inlink->w : (int) res;
> + if (!av_expr_count_var(scale->w_pexpr, VAR_OUT_H, VAR_OH+1)) {
> + res = av_expr_eval(scale->w_pexpr, scale->var_values, NULL);
> + if (isnan(res)) {
> + expr = scale->w_expr;
> + goto fail;
> + }
> + eval_w = scale->var_values[VAR_OUT_W] = scale->var_values[VAR_OW] = (int) res == 0 ? inlink->w : (int) res;
> + }
>
> - if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr),
> - names, var_values,
> - NULL, NULL, NULL, NULL, NULL, 0, log_ctx)) < 0)
> + res = av_expr_eval(scale->h_pexpr, scale->var_values, NULL);
> + if (isnan(res)) {
> + expr = scale->h_expr;
> goto fail;
> - eval_h = var_values[VAR_OUT_H] = var_values[VAR_OH] = (int) res == 0 ? inlink->h : (int) res;
> - /* evaluate again the width, as it may depend on the output height */
> - if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr),
> - names, var_values,
> - NULL, NULL, NULL, NULL, NULL, 0, log_ctx)) < 0)
> - goto fail;
> - eval_w = (int) res == 0 ? inlink->w : (int) res;
> + }
> + eval_h = scale->var_values[VAR_OUT_H] = scale->var_values[VAR_OH] = (int) res == 0 ? inlink->h : (int) res;
> +
> + if (av_expr_count_var(scale->w_pexpr, VAR_OUT_H, VAR_OH+1)) {
> + res = av_expr_eval(scale->w_pexpr, scale->var_values, NULL);
> + if (isnan(res)) {
> + expr = scale->w_expr;
> + goto fail;
> + }
> + eval_w = scale->var_values[VAR_OUT_W] = scale->var_values[VAR_OW] = (int) res == 0 ? inlink->w : (int) res;
> + }
>
> w = eval_w;
> h = eval_h;
I assume its intended to do this with more filters
If we now consider a filter which has more than 2 output variables
maybe width, height, xpos, ypos as in crop.
thats 4 outputs and thats 24 orderings to evaluate them. The method
used above would not work in that case
[...]
> @@ -548,9 +598,6 @@ static int config_props(AVFilterLink *outlink)
> outlink->sample_aspect_ratio.num, outlink->sample_aspect_ratio.den,
> scale->flags);
> return 0;
> -
> -fail:
> - return ret;
this is unrelated to the patchset
> }
>
> static int config_props_ref(AVFilterLink *outlink)
> @@ -600,30 +647,68 @@ static int scale_slice(AVFilterLink *link, AVFrame *out_buf, AVFrame *cur_pic, s
> out,out_stride);
> }
>
> +#define TS2T(ts, tb) ((ts) == AV_NOPTS_VALUE ? NAN : (double)(ts) * av_q2d(tb))
> +
> static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out)
> {
> - ScaleContext *scale = link->dst->priv;
> + AVFilterContext *ctx = link->dst;
> + ScaleContext *scale = ctx->priv;
> AVFilterLink *outlink = link->dst->outputs[0];
> AVFrame *out;
> const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(link->format);
> char buf[32];
> int in_range;
> + int frame_changed;
>
> *frame_out = NULL;
> if (in->colorspace == AVCOL_SPC_YCGCO)
> av_log(link->dst, AV_LOG_WARNING, "Detected unsupported YCgCo colorspace.\n");
>
> - if ( 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) {
> +
> + 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;
> +
> + if (scale->eval_mode == EVAL_MODE_FRAME || frame_changed) {
> int ret;
This change can be factored out in a seperate factorization patch
thx
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
If you think the mosad wants you dead since a long time then you are either
wrong or dead since a long time.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20191114/306717ed/attachment.sig>
More information about the ffmpeg-devel
mailing list