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

Michael Niedermayer michaelni
Fri Sep 24 22:28:00 CEST 2010


On Fri, Sep 24, 2010 at 03:52:58PM +0200, Stefano Sabatini wrote:
> On date Wednesday 2010-09-22 22:59:50 +0200, Michael Niedermayer encoded:
> > On Tue, Sep 21, 2010 at 10:36:52AM +0200, Stefano Sabatini wrote:
> > > On date Friday 2010-09-17 12:09:04 +0200, Michael Niedermayer encoded:
> > > > On Wed, Sep 15, 2010 at 12:28:06AM +0200, Stefano Sabatini wrote:
> > > > [...]
> > > > > +static inline int normalize_double(int *n, double d)
> > > > > +{
> > > > > +    int ret = 0;
> > > > > +
> > > > > +    if (isnan(d)) {
> > > > > +        ret = AVERROR(EINVAL);
> > > > > +    } else if (d > INT_MAX || d < INT_MIN) {
> > > > > +        *n = d > INT_MAX ? INT_MAX : INT_MIN;
> > > > > +        ret = AVERROR(EINVAL);
> > > > > +    } else
> > > > 
> > > > > +        *n = d;
> > > > 
> > > > something that rounds to nearest should be used
> > > 
> > > round()
> > > 
> > > > > +
> > > > > +    return ret;
> > > > > +}
> > > > > +
> > > > >  static int config_input(AVFilterLink *link)
> > > > >  {
> > > > >      AVFilterContext *ctx = link->dst;
> > > > >      CropContext *crop = ctx->priv;
> > > > >      const AVPixFmtDescriptor *pix_desc = &av_pix_fmt_descriptors[link->format];
> > > > > +    int ret;
> > > > > +    const char *expr;
> > > > > +    double res;
> > > > > +
> > > > > +    crop->var_values[E]     = M_E;
> > > > > +    crop->var_values[PHI]   = M_PHI;
> > > > > +    crop->var_values[PI]    = M_PI;
> > > > > +    crop->var_values[IN_W]  = crop->var_values[IW] = ctx->inputs[0]->w;
> > > > > +    crop->var_values[IN_H]  = crop->var_values[IH] = ctx->inputs[0]->h;
> > > > > +    crop->var_values[X]     = NAN;
> > > > > +    crop->var_values[Y]     = NAN;
> > > > > +    crop->var_values[OUT_W] = crop->var_values[OW] = NAN;
> > > > > +    crop->var_values[OUT_H] = crop->var_values[OH] = NAN;
> > > > > +    crop->var_values[N]     = 0;
> > > > 
> > > > T/POS are random?
> > > 
> > > Fixed, set now to NAN.
> > > 
> > > > >      av_image_fill_max_pixsteps(crop->max_step, NULL, pix_desc);
> > > > >      crop->hsub = av_pix_fmt_descriptors[link->format].log2_chroma_w;
> > > > >      crop->vsub = av_pix_fmt_descriptors[link->format].log2_chroma_h;
> > > > >  
> > > > > -    if (crop->w == 0)
> > > > > -        crop->w = link->w - crop->x;
> > > > > -    if (crop->h == 0)
> > > > > -        crop->h = link->h - crop->y;
> > > > > +    if ((ret = av_parse_and_eval_expr(&res, (expr = crop->ow_expr),
> > > > > +                                      var_names, crop->var_values,
> > > > > +                                      NULL, NULL, NULL, NULL, NULL, 0, ctx)) < 0) goto fail_expr;
> > > > > +    crop->var_values[OUT_W] = crop->var_values[OW] = res;
> > > > > +    if ((ret = av_parse_and_eval_expr(&res, (expr = crop->oh_expr),
> > > > > +                                      var_names, crop->var_values,
> > > > > +                                      NULL, NULL, NULL, NULL, NULL, 0, ctx)) < 0) goto fail_expr;
> > > > > +    crop->var_values[OUT_H] = crop->var_values[OH] = res;
> > > > > +    /* evaluate again ow as it may depend on oh */
> > > > > +    if ((ret = av_parse_and_eval_expr(&res, (expr = crop->ow_expr),
> > > > > +                                      var_names, crop->var_values,
> > > > > +                                      NULL, NULL, NULL, NULL, NULL, 0, ctx)) < 0) goto fail_expr;
> > > > > +    crop->var_values[OUT_W] = crop->var_values[OW] = res;
> > > > > +    if (normalize_double(&crop->w, crop->var_values[OUT_W]) < 0 ||
> > > > > +        normalize_double(&crop->h, crop->var_values[OUT_H]) < 0) {
> > > > > +        av_log(ctx, AV_LOG_ERROR,
> > > > > +               "Too big value or invalid expression for out_w/ow or out_h/oh. "
> > > > > +               "Maybe the expression for out_w:'%s' or for out_h:'%s' is self-referencing.\n",
> > > > > +               crop->ow_expr, crop->oh_expr);
> > > > > +        return AVERROR(EINVAL);
> > > > > +    }
> > > > > +    crop->w &= ~((1 << crop->hsub) - 1);
> > > > > +    crop->h &= ~((1 << crop->vsub) - 1);
> > > > > +
> > > > > +    if ((ret = av_parse_expr(&crop->x_pexpr, crop->x_expr, var_names,
> > > > > +                             NULL, NULL, NULL, NULL, 0, ctx)) < 0 ||
> > > > > +        (ret = av_parse_expr(&crop->y_pexpr, crop->y_expr, var_names,
> > > > > +                             NULL, NULL, NULL, NULL, 0, ctx)) < 0)
> > > > > +        return AVERROR(EINVAL);
> > > > >  
> > > > > +    crop->var_values[X] = av_eval_expr(crop->x_pexpr, crop->var_values, NULL);
> > > > > +    crop->var_values[Y] = av_eval_expr(crop->y_pexpr, crop->var_values, NULL);
> > > > > +    /* evaluate again x as it may depend on y */
> > > > > +    crop->var_values[X] = av_eval_expr(crop->x_pexpr, crop->var_values, NULL);
> > > > > +    if (normalize_double(&crop->x, crop->var_values[X]) < 0 ||
> > > > > +        normalize_double(&crop->y, crop->var_values[Y]) < 0) {
> > > > > +        av_log(ctx, AV_LOG_ERROR,
> > > > > +               "Too big value or invalid expression for x or y. "
> > > > > +               "Maybe the expression for x:'%s' or for y:'%s' is self-referencing.\n",
> > > > > +               crop->x_expr, crop->y_expr);
> > > > > +        return AVERROR(EINVAL);
> > > > > +    }
> > > > 
> > > > i dont think that will work they can depend on N/T/POS which should be NAN for W/H
> > > 
> > > Changed the logic, now x/y is evaluated only in start_frame(), a
> > > default value is set in case the first evaluated x/y is not valid.
> > > This way the filter won't fail if the initial value of x/y is not
> > > invalid (and the last valid value is used).
> > > 
> > > Regards.
> > > -- 
> > > FFmpeg = Fascinating and Fostering MultiPurpose Extroverse Gnome
> > 
> > >  doc/ffmpeg-doc.texi       |    8 +-
> > >  doc/filters.texi          |  104 ++++++++++++++++++++++-----
> > >  ffmpeg.c                  |    4 -
> > >  libavfilter/vf_crop.c     |  176 +++++++++++++++++++++++++++++++++++++++++-----
> > >  tests/lavfi-regression.sh |   10 +-
> > >  5 files changed, 254 insertions(+), 48 deletions(-)
> > > 39c3adc9137408cd71d23329d7c7da16b8abada3  0001-Make-the-crop-filter-accept-parametric-expressions.patch
> > > From 9330a5223bfeeadd1ffbb1c8e6d39c6d95906812 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.
> > > 
> > > Also change the syntax of the crop filter, from:
> > > x:y:w:h
> > > to
> > > w:h:x:y
> > > 
> > > which looks more useful.
> > > ---
> > >  doc/ffmpeg-doc.texi       |    8 +-
> > >  doc/filters.texi          |  104 ++++++++++++++++++++++-----
> > >  ffmpeg.c                  |    4 +-
> > >  libavfilter/vf_crop.c     |  176 ++++++++++++++++++++++++++++++++++++++++-----
> > >  tests/lavfi-regression.sh |   10 ++--
> > >  5 files changed, 254 insertions(+), 48 deletions(-)
> > > 
> > > diff --git a/doc/ffmpeg-doc.texi b/doc/ffmpeg-doc.texi
> > > index d9c5c14..e43e87e 100644
> > > --- a/doc/ffmpeg-doc.texi
> > > +++ b/doc/ffmpeg-doc.texi
> > > @@ -226,13 +226,13 @@ The following abbreviations are recognized:
> > >  
> > >  @item -aspect @var{aspect}
> > >  Set aspect ratio (4:3, 16:9 or 1.3333, 1.7777).
> > > - at item -croptop @var{size} (deprecated - use -vf crop=x:y:width:height instead)
> > > + at item -croptop @var{size} (deprecated - use the crop filter instead)
> > >  Set top crop band size (in pixels).
> > > - at item -cropbottom @var{size} (deprecated - use -vf crop=x:y:width:height instead)
> > > + at item -cropbottom @var{size} (deprecated - use the crop filter instead)
> > >  Set bottom crop band size (in pixels).
> > > - at item -cropleft @var{size} (deprecated - use -vf crop=x:y:width:height instead)
> > > + at item -cropleft @var{size} (deprecated - use the crop filter instead)
> > >  Set left crop band size (in pixels).
> > > - at item -cropright @var{size} (deprecated - use -vf crop=x:y:width:height instead)
> > > + at item -cropright @var{size} (deprecated - use the crop filter instead)
> > >  Set right crop band size (in pixels).
> > >  @item -padtop @var{size}
> > >  @item -padbottom @var{size}
> > > diff --git a/doc/filters.texi b/doc/filters.texi
> > > index 262116c..209fa2a 100644
> > > --- a/doc/filters.texi
> > > +++ b/doc/filters.texi
> > > @@ -26,34 +26,102 @@ Below is a description of the currently available video filters.
> > >  
> > >  @section crop
> > >  
> > > -Crop the input video to @var{x}:@var{y}:@var{width}:@var{height}.
> > > +Crop the input video to @var{out_w}:@var{out_h}:@var{x}:@var{y}.
> > >  
> > > - 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 x, y
> > > +the computed values for @var{x} and @var{y}. They are evaluated for
> > > +each new frame.
> > > +
> > > + at item in_w, in_h
> > > +the input width and heigth
> > > +
> > > + at item iw, ih
> > > +same as @var{in_w} and @var{in_h}
> > > +
> > > + at item out_w, out_h
> > > +the output (cropped) width and heigth
> > > +
> > > + at item ow, oh
> > > +same as @var{out_w} and @var{out_h}
> > > +
> > > + at item n
> > > +the number of input frame, starting from 0
> > > +
> > > + at item pos
> > > +the position in the file of the input frame, NAN if unknown
> > > +
> > 
> > > + at item t
> > > +timestamp expressed in seconds, NAN if the input timestamp is invalid
> > 
> > is unknown
> > 
> > 
> > > +
> > > + at end table
> > >  
> > > -The @var{width} and @var{height} parameters specify the width and height
> > > -of the output (non-cropped) area.
> > > +The @var{out_w} and @var{out_h} parameters specify the expressions for
> > > +the width and height of the output (cropped) video. They are
> > > +evaluated just at the configuration of the filter.
> > >  
> > > -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.
> > > +The default value of @var{out_w} is "in_w", and the default value of
> > > + at var{out_h} is "in_h".
> > >
> > 
> > > -For example the parameters:
> > > +The expression for @var{out_w} may depend on the value of
> > > + at var{out_h}, and the expression for @var{out_h} may depend on
> > > + at var{out_h}, but they cannot depend on @var{y} and @var{y}, as
> > 
> > out_h may depend on out_h ?
> > 
> > 
> > > + at var{x} and @var{y} are evaluated are evaluated after @var{out_w} and
> > > + at var{out_h}.
> > 
> > are evaluated are evaluated
> > 
> > please review your own patch before submitting
> > thank you
> 
> Sorry for that, it should be fixed now.
> -- 
> FFmpeg = Fancy Fascinating MultiPurpose Evangelical Gymnast

>  Changelog                 |    1 
>  doc/ffmpeg-doc.texi       |    8 +-
>  doc/filters.texi          |  103 ++++++++++++++++++++++----
>  ffmpeg.c                  |    4 -
>  libavfilter/vf_crop.c     |  176 +++++++++++++++++++++++++++++++++++++++++-----
>  tests/lavfi-regression.sh |   10 +-
>  6 files changed, 254 insertions(+), 48 deletions(-)
> 770d4898e8f08ed2e1e88c639d5b391ab6780cda  0001-Make-the-crop-filter-accept-parametric-expressions.patch
> From e8a7ea8df0eb58a4edf1eab4627cce9f40bc2d79 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.
> 
> Also change the syntax of the crop filter, from:
> x:y:w:h
> to
> w:h:x:y

this should be a seperate patch and as its part of the reasons cited for a
"fork" id say it has high priority that this is commited


rest of the patch looks good too

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When the tyrant has disposed of foreign enemies by conquest or treaty, and
there is nothing more to fear from them, then he is always stirring up
some war or other, in order that the people may require a leader. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100924/aaa15d86/attachment.pgp>



More information about the ffmpeg-devel mailing list