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

Michael Niedermayer michaelni
Mon Sep 13 18:23:11 CEST 2010


On Sun, Sep 12, 2010 at 08:30:11PM +0200, Stefano Sabatini wrote:
> On date Sunday 2010-09-12 14:23:24 +0200, Michael Niedermayer encoded:
> > On Sun, Sep 12, 2010 at 01:51:23AM +0200, Stefano Sabatini wrote:
> > > On date Saturday 2010-09-11 19:05:46 +0200, Michael Niedermayer encoded:
> > > > On Sat, Sep 11, 2010 at 11:35:58AM +0200, Stefano Sabatini wrote:
> > > > > On date Friday 2010-09-10 23:13:45 +0200, Michael Niedermayer encoded:
> > > > > > On Fri, Sep 10, 2010 at 06:33:36PM +0200, Stefano Sabatini wrote:
> > > > > > [...]
> > > > > > > > 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.
> > > > > > 
> > > > > > no its not, you dont clip the double before converting to int
> > > > > > and if iam not mistaken C doesnt gurantee not representable types for that
> > > > > > not just crashing your program
> > > > > 
> > > > > Check attached and tell me if you like it.
> > > > [...]
> > > > > +# trembling effect
> > > > > +crop='10+10*sin(n/10):20+20*sin(n/5):w-2*x:h-2*y
> > > > > +
> > > > > +# erratic camera effect depending on timestamp
> > > > > +crop='20+20*sin(3*t):10+10*sin(2*t):w-2*x:h-2*y'
> > > > > +
> > > > > +# set x depending on the value of y
> > > > > +crop='y:10+10*sin(n/10):w-2*x:h-2*y'
> > > > > + at end example
> > > > 
> > > > if x and y change per frame but w/h cannot then this looks odd
> > > 
> > > If you prefer we can use x0/y0 in place of x/y for the w and h
> > > expressions.
> > 
> > for filters like above this makes as much sense as x/y
> > what you mean (but dont conciously realize) is the maximum x/y that the
> > x/y expressions can generate for any input.
> > sadly we dont know these and its not easy to calculate for a computer
> 
> We could let the user specify w/h *before* x/y, and then use w/W/h/H
> in x/y expressions.
> 
> This looks more useful than allowing the user to specify w/h in
> function of the initial x/y values.

yes


>  
> > > > [...]
> > > > >  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[X  ]   = NAN;
> > > > > +    crop->var_values[Y  ]   = NAN;
> > > > > +    crop->var_values[W  ]   = ctx->inputs[0]->w;
> > > > > +    crop->var_values[H  ]   = ctx->inputs[0]->h;
> > > > > +    crop->var_values[N  ]   = 0;
> > > > >  
> > > > >      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_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);
> > > > 
> > > > this could trigger for unknown pos/ts
> > > 
> > > ???
> > 
> > NAN in (unknown ts/pos), NAN out
> 
> I can set var_values[T] = NAN if pts == NAN.
> 
> The we still need to define a behavior in case the computed value for
> x/y is NAN or out-bound.
> The currently implemented behavior:
> 
> NAN       => use the last valid value
> out-bound => approximate the value to the nearest in-bound value
> 
> Is it OK to keep this behavior?

for the moment of course, we might one day rethink this though


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

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- 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/20100913/cbaea161/attachment.pgp>



More information about the ffmpeg-devel mailing list