[FFmpeg-devel] [PATCH] lavfi/overlay: add process_command callback

[Paul B Mahol]

On 2/19/13, Stefano Sabatini <stefasab at gmail.com> wrote:
> ---
>  doc/filters.texi         |   14 ++++++++++++
>  libavfilter/vf_overlay.c |   53
> ++++++++++++++++++++++++++++++----------------
>  2 files changed, 49 insertions(+), 18 deletions(-)
>
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 8ddbac4..f8279bd 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -3936,6 +3936,20 @@ the @var{movie} filter.
>  You can chain together more overlays but you should test the
>  efficiency of such approach.
>
> + at subsection Commands
> +
> +This filter supports the following command:
> + at table @option
> + at item x
> +Set the @option{x} option expression.
> +
> + at item y
> +Set the @option{y} option expression.
> +
> + at item enable
> +Set the @option{enable} option expression.
> + at end table
> +
>  @subsection Examples
>
>  @itemize
> diff --git a/libavfilter/vf_overlay.c b/libavfilter/vf_overlay.c
> index c9dedb5..b2d68ba 100644
> --- a/libavfilter/vf_overlay.c
> +++ b/libavfilter/vf_overlay.c
> @@ -168,6 +168,36 @@ static av_cold void uninit(AVFilterContext *ctx)
>      ff_bufqueue_discard_all(&over->queue_over);
>  }
>
> +static int configure_expr(AVExpr **pexpr, const char *expr, void *log_ctx)
> +{
> +    int ret;
> +
> +    if (*pexpr)
> +        av_expr_free(*pexpr);
> +    *pexpr = NULL;
> +    ret = av_expr_parse(pexpr, expr, var_names,
> +                        NULL, NULL, NULL, NULL, 0, log_ctx);
> +    if (ret < 0)
> +        av_log(NULL, AV_LOG_ERROR,

Why NULL? When there is log_ctx.


> +               "Error when evaluating the expression '%s'\n", expr);
> +    return ret;
> +}
> +
> +static int process_command(AVFilterContext *ctx, const char *cmd, const
> char *args,
> +                           char *res, int res_len, int flags)
> +{
> +    OverlayContext *over = ctx->priv;
> +
> +    if      (!strcmp(cmd, "x"))
> +        return configure_expr(&over->x_pexpr, args, ctx);
> +    else if (!strcmp(cmd, "y"))
> +        return configure_expr(&over->y_pexpr, args, ctx);
> +    else if (!strcmp(cmd, "enable"))
> +        return configure_expr(&over->enable_pexpr, args, ctx);
> +    else
> +        return AVERROR(ENOSYS);

Why ENOSYS? Isn't EINVAL better?

> +}
> +
>  static int query_formats(AVFilterContext *ctx)
>  {
>      OverlayContext *over = ctx->priv;
> @@ -250,7 +280,6 @@ static int config_input_overlay(AVFilterLink *inlink)
>  {
>      AVFilterContext *ctx  = inlink->dst;
>      OverlayContext  *over = inlink->dst->priv;
> -    char *expr;
>      int ret;
>      const AVPixFmtDescriptor *pix_desc =
> av_pix_fmt_desc_get(inlink->format);
>
> @@ -270,18 +299,10 @@ static int config_input_overlay(AVFilterLink *inlink)
>      over->var_values[VAR_T]     = NAN;
>      over->var_values[VAR_POS]   = NAN;
>
> -    expr = over->x_expr;
> -    if ((ret = av_expr_parse(&over->x_pexpr, expr, var_names,
> -                             NULL, NULL, NULL, NULL, 0, ctx)) < 0)
> -        goto fail;
> -    expr = over->y_expr;
> -    if ((ret = av_expr_parse(&over->y_pexpr, expr, var_names,
> -                             NULL, NULL, NULL, NULL, 0, ctx)) < 0)
> -        goto fail;
> -    expr = over->enable_expr;
> -    if ((ret = av_expr_parse(&over->enable_pexpr, expr, var_names,
> -                             NULL, NULL, NULL, NULL, 0, ctx)) < 0)
> -        goto fail;
> +    if ((ret = configure_expr(&over->x_pexpr, over->x_expr, ctx) < 0) ||
> +        (ret = configure_expr(&over->y_pexpr, over->y_expr, ctx) < 0) ||
> +        (ret = configure_expr(&over->enable_pexpr, over->enable_expr, ctx)
> < 0))
> +        return ret;
>
>      over->overlay_is_packed_rgb =
>          ff_fill_rgba_map(over->overlay_rgba_map, inlink->format) >= 0;
> @@ -294,11 +315,6 @@ static int config_input_overlay(AVFilterLink *inlink)
>             ctx->inputs[OVERLAY]->w, ctx->inputs[OVERLAY]->h,
>             av_get_pix_fmt_name(ctx->inputs[OVERLAY]->format));
>      return 0;
> -
> -fail:
> -    av_log(NULL, AV_LOG_ERROR,
> -           "Error when evaluating the expression '%s'\n", expr);
> -    return ret;
>  }
>
>  static int config_output(AVFilterLink *outlink)
> @@ -695,6 +711,7 @@ AVFilter avfilter_vf_overlay = {
>      .priv_size = sizeof(OverlayContext),
>
>      .query_formats = query_formats,
> +    .process_command = process_command,
>
>      .inputs    = avfilter_vf_overlay_inputs,
>      .outputs   = avfilter_vf_overlay_outputs,
> --
> 1.7.9.5

LGTM otherwise