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

[Michael Niedermayer]

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 at 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


[....]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- 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/20191216/d9eae1c5/attachment.sig>