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

[Gyan]

On 24-12-2019 04:50 am, Michael Niedermayer wrote:
> On Tue, Dec 17, 2019 at 02:55:06PM +0530, Gyan wrote:
> [...]
>> @@ -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;
>> +    }
>> +
>> +        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);
>> +                goto revert;
>> +            }
>> +        }
>> +
>> +        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);
>> +                goto revert;
>> +            }
>> +        }
> Duplicate code

init_dict() is not called during command processing since we don't reset 
any other parameter except for one of width or height. Do you want me to 
reinit all parameters after a command?

>> @@ -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);
> Missing error handling

Will do.

Gyan