[FFmpeg-devel] [PATCH 4/4] Make the crop filter accept parametric expressions.

[Stefano Sabatini]

On date Friday 2010-09-10 11:35:54 +0200, Michael Niedermayer encoded:
> On Fri, Sep 10, 2010 at 09:50:14AM +0200, Stefano Sabatini wrote:
[...]
> > Patch updated.
> > -- 
> > FFmpeg = Faithless and Free Mysterious Proud Eccentric Generator
> 
> >  doc/filters.texi      |   63 ++++++++++++++++++++++++------
> >  libavfilter/vf_crop.c |  105 +++++++++++++++++++++++++++++++++++++++++++++++---
> >  2 files changed, 151 insertions(+), 17 deletions(-)
> > abc8b6a67d12f3a518009ee944046c112ad57d89  0001-Make-the-crop-filter-accept-parametric-expressions.patch
> > From 81d82795d0d27de72982bc6f49961d7221906f29 Mon Sep 17 00:00:00 2001
> > From: Stefano Sabatini <stefano.sabatini-lala at poste.it>
> > Date: Thu, 9 Sep 2010 01:05:04 +0200
> > Subject: [PATCH] Make the crop filter accept parametric expressions.
> > 
> > ---
> >  doc/filters.texi      |   63 ++++++++++++++++++++++++------
> >  libavfilter/vf_crop.c |  105 ++++++++++++++++++++++++++++++++++++++++++++++--
> >  2 files changed, 151 insertions(+), 17 deletions(-)
> > 
> > diff --git a/doc/filters.texi b/doc/filters.texi
> > index 60ecf56..e0627dd 100644
> > --- a/doc/filters.texi
> > +++ b/doc/filters.texi
> > @@ -28,32 +28,71 @@ Below is a description of the currently available video filters.
> >  
> >  Crop the input video to @var{x}:@var{y}:@var{width}:@var{height}.
> >  
> > - at example
> > -./ffmpeg -i in.avi -vf "crop=0:0:0:240" out.avi
> > - at end example
> > +The parameters are expressions containing the following constants:
> >  
> > - at var{x} and @var{y} specify the position of the top-left corner of the
> > -output (non-cropped) area.
> > + at table @option
> > + at item E, PI, PHI
> > +the corresponding mathematical approximated values for e
> > +(euler number), pi (greek PI), PHI (golden ratio)
> >  
> > -The default value of @var{x} and @var{y} is 0.
> > + at item TB
> > +the input timebase used for the timestamps
> > +
> > + at item w, h
> > +the input width and heigth.
> >  
> > -The @var{width} and @var{height} parameters specify the width and height
> > -of the output (non-cropped) area.
> > + at item x, y
> > +the computed values for @var{x} and @var{y}.
> > +
> > + at item n
> > +the number of input frame, starting from 0
> > +
> 
> > + at item pos
> > +the position in the file of the input frame, -1 if unknown
> 
> should be NAN if unknown

Done.

> >  
> > -A value of 0 is interpreted as the maximum possible size contained in
> > -the area delimited by the top-left corner at position x:y.
> > + at item pts
> > +presentation time stamp of the input frame, expressed in TB units
> 
> time stamp should be in seconds, its unlikely that the user cares
> about the timebase or even knows what it is in the crop filter

Done.

> [...]
> > +#define PARSE_AND_EVAL(var, var_expr)                                                       \
> > +    if ((ret = av_parse_and_eval_expr(&res, (expr = var_expr), var_names, crop->var_values, \
> > +                                      NULL, NULL, NULL, NULL, NULL, 0, ctx)) < 0)           \
> > +        goto fail_expr;                                                                     \
> > +    var = res;
> > +
> > +    PARSE_AND_EVAL(crop->w, crop->w_expr);
> > +    PARSE_AND_EVAL(crop->h, crop->h_expr);
> 
> i dont think you need this macro here, and imho its just obfuscating what you
> do

Removed.

> >      av_log(link->dst, AV_LOG_INFO, "x:%d y:%d w:%d h:%d\n",
> >             crop->x, crop->y, crop->w, crop->h);
> > @@ -110,6 +175,11 @@ static int config_input(AVFilterLink *link)
> >      }
> >  
> >      return 0;
> > +
> > +fail_expr:
> > +    av_log(NULL, AV_LOG_ERROR,
> > +           "Error when evaluating the expression '%s'\n", expr);
> > +    return ret;
> >  }
> >  
> >  static int config_output(AVFilterLink *link)
> 
> > @@ -131,6 +201,22 @@ static void start_frame(AVFilterLink *link, AVFilterBufferRef *picref)
> >      picref->video->w = crop->w;
> >      picref->video->h = crop->h;
> >  
> > +    crop->var_values[PTS] = picref->pts;
> > +    crop->var_values[POS] = picref->pos;
> > +    crop->x = crop->var_values[X] = av_eval_expr(crop->x_pexpr, crop->var_values, NULL);
> > +    crop->y = crop->var_values[Y] = av_eval_expr(crop->y_pexpr, crop->var_values, NULL);
> 
> you must evaluate X after Y again so Y is available to X

Done.

> also i think the values should be cliped into sane integer range and maybe a
> NAN check is needed to

The clipping is already performed, and NAN check added.

Regards.
-- 
FFmpeg = Fantastic and Frightening Multimedia Ponderous Elected Ghost