[FFmpeg-devel] [PATCH v2] avfilter/scale: refactor common code for scaling height/width expressions
Mark Thompson
sw at jkqxz.net
Thu Feb 2 01:36:11 EET 2017
On 01/02/17 23:12, Aman Gupta wrote:
> From: Aman Gupta <aman at tmm1.net>
>
> Implements support for height/width expressions in vf_scale_vaapi,
> by refactoring common code into a new libavfilter/scale.c
Some more minor points:
> ---
> libavfilter/Makefile | 8 +--
> libavfilter/scale.c | 152 +++++++++++++++++++++++++++++++++++++++++++
> libavfilter/scale.h | 28 ++++++++
> libavfilter/vf_scale.c | 109 +++----------------------------
> libavfilter/vf_scale_npp.c | 94 +++-----------------------
> libavfilter/vf_scale_vaapi.c | 19 ++++--
> 6 files changed, 217 insertions(+), 193 deletions(-)
> create mode 100644 libavfilter/scale.c
> create mode 100644 libavfilter/scale.h
>
> ...
> +int ff_scale_eval_dimensions(void *log_ctx,
> + const char *w_expr, const char *h_expr,
> + AVFilterLink *inlink, AVFilterLink *outlink,
> + int *ret_w, int *ret_h)
> +{
> + const AVPixFmtDescriptor *desc = av_pix_fmt_desc_get(inlink->format);
> + const AVPixFmtDescriptor *out_desc = av_pix_fmt_desc_get(outlink->format);
> + const char *expr;
> + int w, h;
> + int factor_w, factor_h;
> + int eval_w, eval_h;
> + int ret;
> + double var_values[VARS_NB], res;
> +
> + var_values[VAR_PI] = M_PI;
> + var_values[VAR_PHI] = M_PHI;
> + var_values[VAR_E] = M_E;
> + var_values[VAR_IN_W] = var_values[VAR_IW] = inlink->w;
> + var_values[VAR_IN_H] = var_values[VAR_IH] = inlink->h;
> + var_values[VAR_OUT_W] = var_values[VAR_OW] = NAN;
> + var_values[VAR_OUT_H] = var_values[VAR_OH] = NAN;
> + var_values[VAR_A] = (double) inlink->w / inlink->h;
> + var_values[VAR_SAR] = inlink->sample_aspect_ratio.num ?
> + (double) inlink->sample_aspect_ratio.num / inlink->sample_aspect_ratio.den : 1;
> + var_values[VAR_DAR] = var_values[VAR_A] * var_values[VAR_SAR];
> + var_values[VAR_HSUB] = 1 << desc->log2_chroma_w;
> + var_values[VAR_VSUB] = 1 << desc->log2_chroma_h;
> + var_values[VAR_OHSUB] = 1 << out_desc->log2_chroma_w;
> + var_values[VAR_OVSUB] = 1 << out_desc->log2_chroma_h;
> +
> + /* evaluate width and height */
> + av_expr_parse_and_eval(&res, (expr = w_expr),
> + var_names, var_values,
> + NULL, NULL, NULL, NULL, NULL, 0, log_ctx);
> + eval_w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
> +
> + if ((ret = av_expr_parse_and_eval(&res, (expr = h_expr),
> + 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] = res;
> + /* evaluate again the width, as it may depend on the output height */
> + if ((ret = av_expr_parse_and_eval(&res, (expr = w_expr),
> + var_names, var_values,
> + NULL, NULL, NULL, NULL, NULL, 0, log_ctx)) < 0)
> + goto fail;
> + eval_w = res;
> +
> + w = eval_w;
> + h = eval_h;
> +
> + /* Check if it is requested that the result has to be divisible by a some
> + * factor (w or h = -n with n being the factor). */
> + factor_w = 1;
> + factor_h = 1;
> + if (w < -1) {
> + factor_w = -w;
> + }
> + if (h < -1) {
> + factor_h = -h;
> + }
> +
> + if (w < 0 && h < 0)
> + eval_w = eval_h = 0;
> +
> + if (!(w = eval_w))
> + w = inlink->w;
> + if (!(h = eval_h))
> + h = inlink->h;
> +
> + /* Make sure that the result is divisible by the factor we determined
> + * earlier. If no factor was set, it is nothing will happen as the default
> + * factor is 1 */
> + if (w < 0)
> + w = av_rescale(h, inlink->w, inlink->h * factor_w) * factor_w;
> + if (h < 0)
> + h = av_rescale(w, inlink->h, inlink->w * factor_h) * factor_h;
> +
> + *ret_w = w;
> + *ret_h = h;
> +
> + return 0;
> +
> +fail:
> + av_log(NULL, AV_LOG_ERROR,
Having passed the logging context, you should use it here.
> + "Error when evaluating the expression '%s'.\n"
> + "Maybe the expression for out_w:'%s' or for out_h:'%s' is self-referencing.\n",
> + expr, w_expr, h_expr);
> + return ret;
> +}
> ...
> @@ -359,64 +332,19 @@ static int nppscale_config_props(AVFilterLink *outlink)
> {
> AVFilterContext *ctx = outlink->src;
> AVFilterLink *inlink = outlink->src->inputs[0];
> - NPPScaleContext *s = ctx->priv;
> - int64_t w, h;
> - double var_values[VARS_NB], res;
> - char *expr;
> + NPPScaleContext *s = ctx->priv;
> + int w, h;
> int ret;
>
> - var_values[VAR_PI] = M_PI;
> - var_values[VAR_PHI] = M_PHI;
> - var_values[VAR_E] = M_E;
> - var_values[VAR_IN_W] = var_values[VAR_IW] = inlink->w;
> - var_values[VAR_IN_H] = var_values[VAR_IH] = inlink->h;
> - var_values[VAR_OUT_W] = var_values[VAR_OW] = NAN;
> - var_values[VAR_OUT_H] = var_values[VAR_OH] = NAN;
> - var_values[VAR_A] = (double) inlink->w / inlink->h;
> - var_values[VAR_SAR] = inlink->sample_aspect_ratio.num ?
> - (double) inlink->sample_aspect_ratio.num / inlink->sample_aspect_ratio.den : 1;
> - var_values[VAR_DAR] = var_values[VAR_A] * var_values[VAR_SAR];
> -
> - /* evaluate width and height */
> - av_expr_parse_and_eval(&res, (expr = s->w_expr),
> - var_names, var_values,
> - NULL, NULL, NULL, NULL, NULL, 0, ctx);
> - s->w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
> - if ((ret = av_expr_parse_and_eval(&res, (expr = s->h_expr),
> - var_names, var_values,
> - NULL, NULL, NULL, NULL, NULL, 0, ctx)) < 0)
> - goto fail;
> - s->h = var_values[VAR_OUT_H] = var_values[VAR_OH] = res;
> - /* evaluate again the width, as it may depend on the output height */
> - if ((ret = av_expr_parse_and_eval(&res, (expr = s->w_expr),
> - var_names, var_values,
> - NULL, NULL, NULL, NULL, NULL, 0, ctx)) < 0)
> + if ((ret = ff_scale_eval_dimensions(s,
> + s->w_expr, s->h_expr,
> + inlink, outlink,
> + &w, &h)) < 0)
> goto fail;
> - s->w = res;
> -
> - w = s->w;
> - h = s->h;
>
> - /* sanity check params */
> - if (w < -1 || h < -1) {
> - av_log(ctx, AV_LOG_ERROR, "Size values less than -1 are not acceptable.\n");
> - return AVERROR(EINVAL);
> - }
> - if (w == -1 && h == -1)
> - s->w = s->h = 0;
> -
> - if (!(w = s->w))
> - w = inlink->w;
> - if (!(h = s->h))
> - h = inlink->h;
> - if (w == -1)
> - w = av_rescale(h, inlink->w, inlink->h);
> - if (h == -1)
> - h = av_rescale(w, inlink->h, inlink->w);
> -
> - if (w > INT_MAX || h > INT_MAX ||
> - (h * inlink->w) > INT_MAX ||
> - (w * inlink->h) > INT_MAX)
> + if ((int64_t)w > INT_MAX || (int64_t)h > INT_MAX ||
They're already ints, so they can't be bigger than INT_MAX after the cast - just drop this part of the test. (I'm assuming the expression evaluation code protects against integer overflow.)
> + ((int64_t)h * inlink->w) > INT_MAX ||
> + ((int64_t)w * inlink->h) > INT_MAX)
I was only meaning these ones, as you've done.
> av_log(ctx, AV_LOG_ERROR, "Rescaled value for width or height is too big.\n");
>
> outlink->w = w;
> @@ -439,8 +367,6 @@ static int nppscale_config_props(AVFilterLink *outlink)
> return 0;
>
> fail:
> - av_log(NULL, AV_LOG_ERROR,
> - "Error when evaluating the expression '%s'\n", expr);
> return ret;
> }
>
> diff --git a/libavfilter/vf_scale_vaapi.c b/libavfilter/vf_scale_vaapi.c
> index d1cb246..2fc8faf 100644
> --- a/libavfilter/vf_scale_vaapi.c
> +++ b/libavfilter/vf_scale_vaapi.c
> @@ -31,6 +31,7 @@
> #include "avfilter.h"
> #include "formats.h"
> #include "internal.h"
> +#include "scale.h"
>
> typedef struct ScaleVAAPIContext {
> const AVClass *class;
> @@ -50,9 +51,12 @@ typedef struct ScaleVAAPIContext {
>
> char *output_format_string;
> enum AVPixelFormat output_format;
> - int output_width;
> - int output_height;
>
> + char *w_expr; ///< width expression string
> + char *h_expr; ///< height expression string
Remove the doxygen markers as well?
> +
> + int output_width; // computed width
> + int output_height; // computed height
> } ScaleVAAPIContext;
>
>
> @@ -110,6 +114,7 @@ static int scale_vaapi_config_input(AVFilterLink *inlink)
>
> static int scale_vaapi_config_output(AVFilterLink *outlink)
> {
> + AVFilterLink *inlink = outlink->src->inputs[0];
> AVFilterContext *avctx = outlink->src;
> ScaleVAAPIContext *ctx = avctx->priv;
> AVVAAPIHWConfig *hwconfig = NULL;
> @@ -162,6 +167,12 @@ static int scale_vaapi_config_output(AVFilterLink *outlink)
> }
> }
>
> + if ((err = ff_scale_eval_dimensions(ctx,
> + ctx->w_expr, ctx->h_expr,
> + inlink, outlink,
> + &ctx->output_width, &ctx->output_height)) < 0)
> + goto fail;
> +
> if (ctx->output_width < constraints->min_width ||
> ctx->output_height < constraints->min_height ||
> ctx->output_width > constraints->max_width ||
> @@ -414,9 +425,9 @@ static av_cold void scale_vaapi_uninit(AVFilterContext *avctx)
> #define FLAGS (AV_OPT_FLAG_FILTERING_PARAM|AV_OPT_FLAG_VIDEO_PARAM)
> static const AVOption scale_vaapi_options[] = {
> { "w", "Output video width",
> - OFFSET(output_width), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, .flags = FLAGS },
> + OFFSET(w_expr), AV_OPT_TYPE_STRING, {.str = "iw"}, .flags = FLAGS },
> { "h", "Output video height",
> - OFFSET(output_height), AV_OPT_TYPE_INT, { .i64 = 0 }, 0, INT_MAX, .flags = FLAGS },
> + OFFSET(h_expr), AV_OPT_TYPE_STRING, {.str = "ih"}, .flags = FLAGS },
> { "format", "Output video format (software format of hardware frames)",
> OFFSET(output_format_string), AV_OPT_TYPE_STRING, .flags = FLAGS },
> { NULL },
>
More information about the ffmpeg-devel
mailing list