[FFmpeg-devel] [PATCH] lavfi: port decimate libmpcodecs filter

[Stefano Sabatini]

On date Tuesday 2012-08-21 11:24:06 +0200, Nicolas George encoded:
> Le tridi 3 fructidor, an CCXX, Stefano Sabatini a écrit :
> > > Maybe store it as an integer? That would help making the filter bit-exact.
> > Please elaborate, also I'd like to keep compatibility with mp=decimate.
> 
> My suggestion would be to have "unsigned frac", with default value
> 1431655765 ((1<<32)/3) or 1417339208 ((1<<32)*0.33) and replace the
> following code 
> 
> > +    int t = (w/16)*(h/16)*decimate->frac;
> 
> by:
> 
> int t = av_rescale((w/16)*(h/16), decimate->frac, 0x100000000);
> 
> In other words: frac_as_a_float = frac_as_an_unsigned / (float)(1 << 32),
> but avoiding all floating-point arithmetic.

OK.
 
> Since (w/16)*(h/16) should be way smaller than 1<<32 (if av_image_check_size
> did its work, (w/16)*(h/16) < 1<<20), it will be accurate enough. And
> without floating-point arithmetic, we can be sure the computation is
> bit-exact across architectures and add FATE tests.
> 
> The /16 look very strange, though, as x and y are incremented by steps of 4.
> 
> > +        char c1, c2, c3, c4;
> > +        int n = sscanf(args, "%d%c%d%c%d%c%f%c",
> > +                       &decimate->max_drop_count, &c1,
> > +                       &decimate->hi, &c2, &decimate->lo, &c3,
> > +                       &decimate->frac, &c4);
> > +        if (n != 1 &&
> > +            (n != 3 || c1 != ':') &&
> > +            (n != 5 || c1 != ':' || c2 != ':') &&
> > +            (n != 7 || c1 != ':' || c2 != ':' || c3 != ':')) {
> > +            av_log(ctx, AV_LOG_ERROR,
> > +                   "Invalid syntax for argument '%s': "
> > +                   "must be in the form 'max:hi:lo:frac'\n", args);
> > +            return AVERROR(EINVAL);
> 

> This test look very clumsy, but I guess it comes from the original code.

Uhm no the original code was missing any validation check. The best
course would be to get your av_opt* changes and use them so we don't
need these clumsy checks all over the place.
 
> No other remarks.

BTW we discussed over IRC and came to the conclusion that we should
rely on DSPUtil so we don't duplicate code and optims which are
already implemented (for several archs) in lavc.
-- 
FFmpeg = Foolish and Faithless Multipurpose Purposeless Easy Gladiator