On 31/01/17 19:14, Aman Gupta wrote:
> From: Aman Gupta <a...@tmm1.net>
>
> Copied directly from vf_scale.c.
>
> Implements the same expression logic, however not all the variables were
> copied over.
> This patch was sufficient for my particular use-case
> `scale_vaapi=-2:min(ih\,720)`,
> but perhaps it makes sense to add support for the remaining variables
> and pull out shared code into a new vf_scale_common.c?
I would prefer the code fragment not to be copied again, yes.
(Implementing this and removing the duplication between scale, scale_npp,
scale_qsv and scale_vaapi has been on my to-maybe-do-at-some-point list for
quite a while, but I've never actually had a use-case for it to push me into
actually doing it :)
If you can't be bothered, then the patch looks mostly sensible to me (some
issues below, I think mainly coming from it being copied).
> ---
> libavfilter/vf_scale_vaapi.c | 98
> ++++++++++++++++++++++++++++++++++++++++++--
> 1 file changed, 95 insertions(+), 3 deletions(-)
>
> diff --git a/libavfilter/vf_scale_vaapi.c b/libavfilter/vf_scale_vaapi.c
> index d1cb246..0d1e1b3 100644
> --- a/libavfilter/vf_scale_vaapi.c
> +++ b/libavfilter/vf_scale_vaapi.c
> @@ -22,6 +22,7 @@
> #include <va/va_vpp.h>
>
> #include "libavutil/avassert.h"
> +#include "libavutil/eval.h"
> #include "libavutil/hwcontext.h"
> #include "libavutil/hwcontext_vaapi.h"
> #include "libavutil/mem.h"
> @@ -32,6 +33,22 @@
> #include "formats.h"
> #include "internal.h"
>
> +static const char *const var_names[] = {
> + "in_w", "iw",
> + "in_h", "ih",
> + "out_w", "ow",
> + "out_h", "oh",
> + NULL
> +};
> +
> +enum var_name {
> + VAR_IN_W, VAR_IW,
> + VAR_IN_H, VAR_IH,
> + VAR_OUT_W, VAR_OW,
> + VAR_OUT_H, VAR_OH,
> + VARS_NB
> +};
> +
> typedef struct ScaleVAAPIContext {
> const AVClass *class;
>
> @@ -50,9 +67,21 @@ typedef struct ScaleVAAPIContext {
>
> char *output_format_string;
> enum AVPixelFormat output_format;
> +
> + /**
> + * New dimensions. Special values are:
> + * 0 = original width/height
> + * -1 = keep original aspect
> + * -N = try to keep aspect but make sure it is divisible by N
> + */
> + int w, h;
Why do these exist in addition to output_width and output_height? They only
seem to be used as temporaries duplicating w and h in
scale_vaapi_config_output().
> +
> + char *w_expr; ///< width expression string
> + char *h_expr; ///< height expression string
> +
> + /* computed width/height */
> int output_width;
> int output_height;
> -
> } ScaleVAAPIContext;
>
>
> @@ -118,6 +147,14 @@ static int scale_vaapi_config_output(AVFilterLink
> *outlink)
> VAStatus vas;
> int err, i;
>
> + AVFilterLink *inlink = outlink->src->inputs[0];
> + ScaleVAAPIContext *scale = ctx;
Just use the ctx variable?
> + int64_t w, h;
> + double var_values[VARS_NB], res;
> + char *expr;
This variable is write-only.
> + int ret;
Just use err (because of the difference you aren't setting the correct error
return if one of the evaluation operations fails).
> + int factor_w, factor_h;
> +
> scale_vaapi_pipeline_uninit(ctx);
>
> ctx->device_ref = av_buffer_ref(ctx->input_frames->device_ref);
> @@ -162,6 +199,61 @@ static int scale_vaapi_config_output(AVFilterLink
> *outlink)
> }
> }
>
> + 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;
> +
> + /* evaluate width and height */
> + av_expr_parse_and_eval(&res, (expr = scale->w_expr),
> + var_names, var_values,
> + NULL, NULL, NULL, NULL, NULL, 0, ctx);
> + scale->w = var_values[VAR_OUT_W] = var_values[VAR_OW] = res;
> + if ((ret = av_expr_parse_and_eval(&res, (expr = scale->h_expr),
> + var_names, var_values,
> + NULL, NULL, NULL, NULL, NULL, 0, ctx))
> < 0)
> + goto fail;
> + scale->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 = scale->w_expr),
> + var_names, var_values,
> + NULL, NULL, NULL, NULL, NULL, 0, ctx))
> < 0)
> + goto fail;
> + scale->w = res;
> +
> + w = scale->w;
> + h = scale->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)
> + scale->w = scale->h = 0;
> +
> + if (!(w = scale->w))
> + w = inlink->w;
> + if (!(h = scale->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;
> +
> + ctx->output_width = w;
> + ctx->output_height = h;
> +
> if (ctx->output_width < constraints->min_width ||
> ctx->output_height < constraints->min_height ||
> ctx->output_width > constraints->max_width ||
> @@ -414,9 +506,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, .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, .flags = FLAGS },
Should these have default values? "iw", "ih", maybe. (It currently segfaults
in the evaluation if either of them are not set.)
> { "format", "Output video format (software format of hardware frames)",
> OFFSET(output_format_string), AV_OPT_TYPE_STRING, .flags = FLAGS },
> { NULL },
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
