[FFmpeg-devel] [PATCH] lavfi/crop: add support for x/y commands

[Michael Niedermayer]

On Wed, Jul 10, 2013 at 05:51:08PM +0200, Stefano Sabatini wrote:
> TODO: bump micro
> ---
>  doc/filters.texi      | 14 ++++++++++++++
>  libavfilter/vf_crop.c | 53 +++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 67 insertions(+)
> 
> diff --git a/doc/filters.texi b/doc/filters.texi
> index 92f8612..9e95f2c 100644
> --- a/doc/filters.texi
> +++ b/doc/filters.texi
> @@ -2503,6 +2503,20 @@ is approximated to the nearest valid value.
>  The expression for @var{x} may depend on @var{y}, and the expression
>  for @var{y} may depend on @var{x}.
>  
> + at subsection Commands
> +
> +This filter supports the following commands:
> + at table @option
> + at item x
> + at item y
> + at item H
> +Modify the x and/or the y position of the cropped area.
> +The command accepts the same syntax of the corresponding option.
> +
> +If the specified expression is not valid, it is kept at its current
> +value.
> + at end table
> +
>  @subsection Examples
>  
>  @itemize
> diff --git a/libavfilter/vf_crop.c b/libavfilter/vf_crop.c
> index 832e6cf..4c63107 100644
> --- a/libavfilter/vf_crop.c
> +++ b/libavfilter/vf_crop.c
> @@ -292,6 +292,58 @@ static int filter_frame(AVFilterLink *link, AVFrame *frame)
>      return ff_filter_frame(link->dst->outputs[0], frame);
>  }
>  
> +static int set_expr(AVExpr **pexpr_ptr, char **expr_ptr,
> +                    const char *expr, const char *option, void *log_ctx)
> +{
> +    int ret;
> +    AVExpr *new_pexpr;
> +    char *new_expr;
> +
> +    new_expr = av_strdup(expr);
> +    if (!new_expr)
> +        return AVERROR(ENOMEM);
> +    ret = av_expr_parse(&new_pexpr, expr, var_names,
> +                        NULL, NULL, NULL, NULL, 0, log_ctx);
> +    if (ret < 0) {
> +        av_log(log_ctx, AV_LOG_ERROR,
> +               "Error when evaluating the expression '%s' for %s\n",
> +               expr, option);
> +        av_free(new_expr);
> +        return ret;
> +    }
> +
> +    if (*pexpr_ptr)
> +        av_expr_free(*pexpr_ptr);
> +    *pexpr_ptr = new_pexpr;
> +    av_freep(expr_ptr);
> +    *expr_ptr = new_expr;
> +
> +    return 0;
> +}
> +
> +static int process_command(AVFilterContext *ctx, const char *cmd, const char *args,
> +                           char *res, int res_len, int flags)
> +{
> +    CropContext *crop = ctx->priv;
> +
> +#define SET_EXPR(expr, option)                                          \
> +    do {                                                                \
> +        int ret = set_expr(&crop->expr##_pexpr, &crop->expr##_expr,     \
> +                           args, option, ctx);                          \
> +        if (ret < 0)                                                    \
> +            return ret;                                                 \
> +    } while (0)
> +
> +    if (!strcmp(cmd, "x")) {
> +        SET_EXPR(x, "x");
> +    } else if (!strcmp(cmd, "y")) {
> +        SET_EXPR(y, "y");
> +    } else
> +        return AVERROR(ENOSYS);

the macro obfuscation can be avoided and a simple function and
pointers be used

also
the user might have new options like  "x=y,y=6" or "y=x,x=5" or
"x=4,y=x"
the order by which you parse this string and submit it affects the
results.

And why dont you flag settable AVOptions and use generic code to set
them through some command that is handled in the generic code ?

The expression parsing that already exists in filters could be
reused instead of duplicating it for the duplicated command processig
this also would avoid any order of evaluation issues as its the same
code either way no matter what did set the parameters
this also automatically avoids bugs with parameter validation and
rounding related to chroma subsamping as either way the same code
is used

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 2
"100% positive feedback" - "All either got their money back or didnt complain"
"Best seller ever, very honest" - "Seller refunded buyer after failed scam"
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130714/a153df97/attachment.asc>