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

[Gyan]

Ping x2

On 20-12-2019 11:24 am, Gyan wrote:
> Ping for this and remaining patches in set.
>
> On 17-12-2019 02:55 pm, Gyan wrote:
>>
>>
>> On 17-12-2019 01:44 am, Michael Niedermayer wrote:
>>> 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
>>
>> More compact v2 patch attached.
>>
>> Which changes do you find messy? Once parse and eval are separated, 
>> the AVExpr and expr strings are backed up for restoration should the 
>> command fail.
>>
>> Gyan
>>
>> _______________________________________________
>> ffmpeg-devel mailing list
>> ffmpeg-devel at ffmpeg.org
>> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>>
>> To unsubscribe, visit link above, or email
>> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".