| Message ID | f1de2d24-5b13-0259-fb17-281fb91fdf2f@gyani.pro |
|---|---|
| State | New |
| Headers | show |
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@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@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
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@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@ffmpeg.org >> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> >> To unsubscribe, visit link above, or email >> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe". > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
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 > @@ -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 [...]
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
On 24-12-2019 11:39 am, Gyan wrote: > > > 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? Ping.
Hi Michael, On 26-12-2019 02:42 pm, Gyan wrote: > > > On 24-12-2019 11:39 am, Gyan wrote: >> >> >> 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? > > Ping. If you can suggest how you want me to proceed, that would be helpful. Gyan
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..93d3da807e 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,22 +746,88 @@ 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); + 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; + } + } + if ((ret = config_props(outlink)) < 0) { - scale->w = old_w; - scale->h = old_h; + av_log(ctx, AV_LOG_ERROR, "Command failed. Continuing with existing parameters.\n"); + goto revert; + } + + 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); + return AVERROR(ENOSYS); + + return 0; + +revert: + 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; + } return ret; }