diff mbox

[FFmpeg-devel,3/5] avfilter/scale: separate exprs parse and eval

Message ID 7801278e-697a-ae99-e050-1dc8f7b96a73@gyani.pro
State Superseded
Headers show

Commit Message

Gyan Doshi Dec. 15, 2019, 5:06 p.m. UTC 
3rd of 5 factorized patches; supersedes 
https://patchwork.ffmpeg.org/patch/16272/
From 00b54948b88ae60aa3ab7c158b98e55cb8b967d3 Mon Sep 17 00:00:00 2001
From: Gyan Doshi <ffmpeg@gyani.pro>
Date: Thu, 12 Dec 2019 22:54:31 +0530
Subject: [PATCH 3/5] avfilter/scale: separate exprs parse and eval

Will allow adding animation support.
---
 libavfilter/scale_eval.c |  69 +---------
 libavfilter/vf_scale.c   | 286 ++++++++++++++++++++++++++++++++++++---
 2 files changed, 271 insertions(+), 84 deletions(-)

Comments

Michael Niedermayer Dec. 16, 2019, 8:14 p.m. UTC | #1 
On Sun, Dec 15, 2019 at 10:36:58PM +0530, Gyan wrote:
> 3rd of 5 factorized patches; supersedes
> https://patchwork.ffmpeg.org/patch/16272/

>  scale_eval.c |   69 --------------
>  vf_scale.c   |  286 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----
>  2 files changed, 271 insertions(+), 84 deletions(-)
> 2a3ae4ce4e91893fddb020485e6936c82894eb81  0003-avfilter-scale-separate-exprs-parse-and-eval.patch
> From 00b54948b88ae60aa3ab7c158b98e55cb8b967d3 Mon Sep 17 00:00:00 2001
> From: Gyan Doshi <ffmpeg@gyani.pro>
> Date: Thu, 12 Dec 2019 22:54:31 +0530
> Subject: [PATCH 3/5] avfilter/scale: separate exprs parse and eval
> 
> Will allow adding animation support.

[...]
> @@ -566,19 +746,87 @@ static int process_command(AVFilterContext *ctx, const char *cmd, const char *ar
>                             char *res, int res_len, int flags)
>  {
>      ScaleContext *scale = ctx->priv;
> -    int ret;
> +    AVFilterLink *outlink = ctx->outputs[0];
> +    char *old_w_str, *old_h_str;
> +    AVExpr *old_w_pexpr, *old_h_pexpr;
> +    int ret, w = 0, h = 0;
> +    const char scale2ref = ctx->filter == &ff_vf_scale2ref;
> +    const char *const *names = scale2ref ? var_names_scale2ref : var_names;
> +
> +    w = !strcmp(cmd, "width")  || !strcmp(cmd, "w");
> +    h = !strcmp(cmd, "height")  || !strcmp(cmd, "h");
> +
> +    if (w || h) {
>  
> -    if (   !strcmp(cmd, "width")  || !strcmp(cmd, "w")
> -        || !strcmp(cmd, "height") || !strcmp(cmd, "h")) {
> +        if (w) {
> +            old_w_str = av_strdup(scale->w_expr);
> +            if (!old_w_str)
> +                return AVERROR(ENOMEM);
> +            old_w_pexpr = scale->w_pexpr;
> +            scale->w_pexpr = NULL;
> +        }
>  
> -        int old_w = scale->w;
> -        int old_h = scale->h;
> -        AVFilterLink *outlink = ctx->outputs[0];
> +        if (h) {
> +            old_h_str = av_strdup(scale->h_expr);
> +            if (!old_h_str)
> +                return AVERROR(ENOMEM);
> +            old_h_pexpr = scale->h_pexpr;
> +            scale->h_pexpr = NULL;
> +        }
>  
>          av_opt_set(scale, cmd, args, 0);
> +

> +        if (w) {
> +            ret = av_expr_parse(&scale->w_pexpr, scale->w_expr,
> +                            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);
> +                av_opt_set(scale, "w", old_w_str, 0);
> +                av_free(old_w_str);
> +                scale->w_pexpr = old_w_pexpr;
> +                return ret;
> +            }
> +        }
> +
> +        if (h) {
> +            ret = av_expr_parse(&scale->h_pexpr, scale->h_expr,
> +                            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);
> +                av_opt_set(scale, "h", old_h_str, 0);
> +                av_free(old_h_str);
> +                scale->h_pexpr = old_h_pexpr;
> +                return ret;
> +            }
> +        }
> +
>          if ((ret = config_props(outlink)) < 0) {
> -            scale->w = old_w;
> -            scale->h = old_h;
> +
> +            if (w) {
> +                av_opt_set(scale, "w", old_w_str, 0);
> +                av_free(old_w_str);
> +                av_expr_free(scale->w_pexpr);
> +                scale->w_pexpr = old_w_pexpr;
> +            }
> +            if (h) {
> +                av_opt_set(scale, "h", old_h_str, 0);
> +                av_free(old_h_str);
> +                av_expr_free(scale->h_pexpr);
> +                scale->h_pexpr = old_h_pexpr;
> +            }
> +            av_log(ctx, AV_LOG_ERROR, "Command failed. Continuing with existing parameters.\n");
> +            return ret;
> +        }

the cleanup code is duplicated

also if you can make the overall change this patch is making 
cleaner/clearer that would be welcome too. Its just a feeling
but this seems more messy than what i would expect from spliting
parse out

Thanks


[....]
diff mbox

Patch

diff --git a/libavfilter/scale_eval.c b/libavfilter/scale_eval.c
index 6c526a97af..dfec081e15 100644
--- a/libavfilter/scale_eval.c
+++ b/libavfilter/scale_eval.c
@@ -54,46 +54,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[] = {
-    "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,
@@ -104,16 +64,7 @@  int ff_scale_eval_dimensions(void *log_ctx,
     const char *expr;
     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_IN_W]  = var_values[VAR_IW] = inlink->w;
     var_values[VAR_IN_H]  = var_values[VAR_IH] = inlink->h;
@@ -128,32 +79,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 5a375fac5d..8eb74bf7dc 100644
--- a/libavfilter/vf_scale.c
+++ b/libavfilter/vf_scale.c
@@ -32,6 +32,7 @@ 
 #include "scale_eval.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 +42,76 @@ 
 #include "libavutil/avassert.h"
 #include "libswscale/swscale.h"
 
+static const char *const var_names[] = {
+    "in_w",   "iw",
+    "in_h",   "ih",
+    "out_w",  "ow",
+    "out_h",  "oh",
+    "a",
+    "sar",
+    "dar",
+    "hsub",
+    "vsub",
+    "ohsub",
+    "ovsub",
+    NULL
+};
+
+enum var_name {
+    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
+};
+
+/**
+ * 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[] = {
+    "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
+};
+
 enum EvalMode {
     EVAL_MODE_INIT,
     EVAL_MODE_FRAME,
@@ -72,6 +143,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 + VARS_S2R_NB];
+
     char *flags_str;
 
     char *in_color_matrix;
@@ -100,6 +175,8 @@  static av_cold int init_dict(AVFilterContext *ctx, AVDictionary **opts)
 {
     ScaleContext *scale = ctx->priv;
     int ret;
+    const char scale2ref = ctx->filter == &ff_vf_scale2ref;
+    const char *const *names = scale2ref ? var_names_scale2ref : var_names;
 
     if (scale->size_str && (scale->w_expr || scale->h_expr)) {
         av_log(ctx, AV_LOG_ERROR,
@@ -127,6 +204,22 @@  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,
+                            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,
+                            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;
+    }
+
     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);
 
@@ -149,6 +242,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]);
@@ -218,6 +314,81 @@  static const int *parse_yuv_type(const char *s, enum AVColorSpace colorspace)
     return sws_getCoefficients(colorspace);
 }
 
+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);
+    char *expr;
+    int eval_w, eval_h;
+    int ret;
+    double res;
+    const AVPixFmtDescriptor *main_desc;
+    const AVFilterLink *main_link;
+
+    if (scale2ref) {
+        main_link = ctx->inputs[0];
+        main_desc = av_pix_fmt_desc_get(main_link->format);
+    }
+
+    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;
+    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) {
+        scale->var_values[VARS_NB + VAR_S2R_MAIN_W] = main_link->w;
+        scale->var_values[VARS_NB + VAR_S2R_MAIN_H] = main_link->h;
+        scale->var_values[VARS_NB + VAR_S2R_MAIN_A] = (double) main_link->w / main_link->h;
+        scale->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;
+        scale->var_values[VARS_NB + VAR_S2R_MAIN_DAR] = scale->var_values[VARS_NB + VAR_S2R_MDAR] =
+            scale->var_values[VARS_NB + VAR_S2R_MAIN_A] * scale->var_values[VARS_NB + VAR_S2R_MAIN_SAR];
+        scale->var_values[VARS_NB + VAR_S2R_MAIN_HSUB] = 1 << main_desc->log2_chroma_w;
+        scale->var_values[VARS_NB + VAR_S2R_MAIN_VSUB] = 1 << main_desc->log2_chroma_h;
+    }
+
+    res = av_expr_eval(scale->w_pexpr, scale->var_values, NULL);
+    eval_w = scale->var_values[VAR_OUT_W] = scale->var_values[VAR_OW] = (int) res == 0 ? inlink->w : (int) res;
+
+    res = av_expr_eval(scale->h_pexpr, scale->var_values, NULL);
+    if (isnan(res)) {
+        expr = scale->h_expr;
+        ret = AVERROR(EINVAL);
+        goto fail;
+    }
+    eval_h = scale->var_values[VAR_OUT_H] = scale->var_values[VAR_OH] = (int) res == 0 ? inlink->h : (int) res;
+
+    res = av_expr_eval(scale->w_pexpr, scale->var_values, NULL);
+    if (isnan(res)) {
+        expr = scale->w_expr;
+        ret = AVERROR(EINVAL);
+        goto fail;
+    }
+    eval_w = scale->var_values[VAR_OUT_W] = scale->var_values[VAR_OW] = (int) res == 0 ? inlink->w : (int) res;
+
+    scale->w = eval_w;
+    scale->h = eval_h;
+
+    return 0;
+
+fail:
+    av_log(ctx, AV_LOG_ERROR,
+           "Error when evaluating the expression '%s'.\n", expr);
+    return ret;
+}
+
 static int config_props(AVFilterLink *outlink)
 {
     AVFilterContext *ctx = outlink->src;
@@ -228,26 +399,23 @@  static int config_props(AVFilterLink *outlink)
     enum AVPixelFormat outfmt = outlink->format;
     const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
     ScaleContext *scale = ctx->priv;
-    int w, h;
     int ret;
 
-    if ((ret = ff_scale_eval_dimensions(ctx,
-                                        scale->w_expr, scale->h_expr,
-                                        inlink, outlink,
-                                        &w, &h)) < 0)
+    if ((ret = scale_eval_dimensions(ctx)) < 0)
         goto fail;
 
-    ff_scale_adjust_dimensions(inlink, &w, &h,
+    ff_scale_adjust_dimensions(inlink, &scale->w, &scale->h,
                                scale->force_original_aspect_ratio,
                                scale->force_divisible_by);
 
-    if (w > INT_MAX || h > INT_MAX ||
-        (h * inlink->w) > INT_MAX  ||
-        (w * inlink->h) > INT_MAX)
+    if (scale->w > INT_MAX ||
+        scale->h > INT_MAX ||
+        (scale->h * inlink->w) > INT_MAX ||
+        (scale->w * inlink->h) > INT_MAX)
         av_log(ctx, AV_LOG_ERROR, "Rescaled value for width or height is too big.\n");
 
-    outlink->w = w;
-    outlink->h = h;
+    outlink->w = scale->w;
+    outlink->h = scale->h;
 
     /* TODO: make algorithm configurable */
 
@@ -421,6 +589,18 @@  static int scale_frame(AVFilterLink *link, AVFrame *in, AVFrame **frame_out)
             av_opt_set(scale, "w", buf, 0);
             snprintf(buf, sizeof(buf)-1, "%d", outlink->h);
             av_opt_set(scale, "h", buf, 0);
+
+            av_expr_free(scale->w_pexpr);
+            av_expr_free(scale->h_pexpr);
+            scale->w_pexpr = scale->h_pexpr = NULL;
+
+            av_expr_parse(&scale->w_pexpr, scale->w_expr,
+                          var_names,
+                          NULL, NULL, NULL, NULL, 0, ctx);
+
+            av_expr_parse(&scale->h_pexpr, scale->h_expr,
+                          var_names,
+                          NULL, NULL, NULL, NULL, 0, ctx);
         }
 
         link->dst->inputs[0]->format = in->format;
@@ -566,19 +746,87 @@  static int process_command(AVFilterContext *ctx, const char *cmd, const char *ar
                            char *res, int res_len, int flags)
 {
     ScaleContext *scale = ctx->priv;
-    int ret;
+    AVFilterLink *outlink = ctx->outputs[0];
+    char *old_w_str, *old_h_str;
+    AVExpr *old_w_pexpr, *old_h_pexpr;
+    int ret, w = 0, h = 0;
+    const char scale2ref = ctx->filter == &ff_vf_scale2ref;
+    const char *const *names = scale2ref ? var_names_scale2ref : var_names;
+
+    w = !strcmp(cmd, "width")  || !strcmp(cmd, "w");
+    h = !strcmp(cmd, "height")  || !strcmp(cmd, "h");
+
+    if (w || h) {
 
-    if (   !strcmp(cmd, "width")  || !strcmp(cmd, "w")
-        || !strcmp(cmd, "height") || !strcmp(cmd, "h")) {
+        if (w) {
+            old_w_str = av_strdup(scale->w_expr);
+            if (!old_w_str)
+                return AVERROR(ENOMEM);
+            old_w_pexpr = scale->w_pexpr;
+            scale->w_pexpr = NULL;
+        }
 
-        int old_w = scale->w;
-        int old_h = scale->h;
-        AVFilterLink *outlink = ctx->outputs[0];
+        if (h) {
+            old_h_str = av_strdup(scale->h_expr);
+            if (!old_h_str)
+                return AVERROR(ENOMEM);
+            old_h_pexpr = scale->h_pexpr;
+            scale->h_pexpr = NULL;
+        }
 
         av_opt_set(scale, cmd, args, 0);
+
+        if (w) {
+            ret = av_expr_parse(&scale->w_pexpr, scale->w_expr,
+                            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);
+                av_opt_set(scale, "w", old_w_str, 0);
+                av_free(old_w_str);
+                scale->w_pexpr = old_w_pexpr;
+                return ret;
+            }
+        }
+
+        if (h) {
+            ret = av_expr_parse(&scale->h_pexpr, scale->h_expr,
+                            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);
+                av_opt_set(scale, "h", old_h_str, 0);
+                av_free(old_h_str);
+                scale->h_pexpr = old_h_pexpr;
+                return ret;
+            }
+        }
+
         if ((ret = config_props(outlink)) < 0) {
-            scale->w = old_w;
-            scale->h = old_h;
+
+            if (w) {
+                av_opt_set(scale, "w", old_w_str, 0);
+                av_free(old_w_str);
+                av_expr_free(scale->w_pexpr);
+                scale->w_pexpr = old_w_pexpr;
+            }
+            if (h) {
+                av_opt_set(scale, "h", old_h_str, 0);
+                av_free(old_h_str);
+                av_expr_free(scale->h_pexpr);
+                scale->h_pexpr = old_h_pexpr;
+            }
+            av_log(ctx, AV_LOG_ERROR, "Command failed. Continuing with existing parameters.\n");
+            return ret;
+        }
+
+        if (w) {
+            av_free(old_w_str);
+            av_expr_free(old_w_pexpr);
+        }
+        if (h) {
+            av_free(old_h_str);
+            av_expr_free(old_h_pexpr);
         }
     } else
         ret = AVERROR(ENOSYS);