[FFmpeg-devel] [PATCH] rotate filter

[Michael Niedermayer]

On Wed, May 04, 2011 at 12:31:29PM +0200, Stefano Sabatini wrote:
> On date Wednesday 2011-05-04 02:53:57 +0200, Michael Niedermayer encoded:
> > On Wed, May 04, 2011 at 01:32:46AM +0200, Stefano Sabatini wrote:
> [...]
> > > Update with some minor fixes, and with regression tests added.
> > > 
> > > Bikeshed: I'm not sure if it is better to make the angle mandatory,
> > > and require thus:
> > > rotate=123
> > > 
> > > rather than make it an option, and thus settable via:
> > > rotate=a=123
> > 
> > pick whatever you prefer
> 
> Opted for making the rotate argument mandatory.
> 
> > > the first is slightly simpler (and compatible with the old rotate),
> > > but doesn't allow rotate with no arguments.
> > > 
> > > Michael please comment on the ipol >> issue, I can't find a way as you
> > > suggest.
> > [...]
> > > +/**
> > > + * Interpolate the color in src at position x and y using bilinear
> > > + * interpolation.
> > > + *
> > > + * @param dst_color put here the destination color
> > > + */
> > > +static uint8_t *ipol(uint8_t *dst_color,
> > > +                     const uint8_t *src, int src_linesize, int src_linestep,
> > > +                     int x, int y, int max_x, int max_y)
> > > +{
> > > +    int int_x = x>>16;
> > > +    int int_y = y>>16;
> > > +    int frac_x = x&0xFFFF;
> > > +    int frac_y = y&0xFFFF;
> > > +    int i;
> > > +    int int_x1 = FFMIN(int_x+1,max_x);
> > > +    int int_y1 = FFMIN(int_y+1,max_y);
> > > +
> > 
> > > +    for (i = 0; i < src_linestep; i++) {
> > > +        int s00 = src[src_linestep * int_x  + i + src_linesize * int_y ];
> > > +        int s01 = src[src_linestep * int_x1 + i + src_linesize * int_y ];
> > > +        int s10 = src[src_linestep * int_x  + i + src_linesize * int_y1];
> > > +        int s11 = src[src_linestep * int_x1 + i + src_linesize * int_y1];
> > > +        int s0 = (((1<<16) - frac_x)*s00 + frac_x*s01);
> > > +        int s1 = (((1<<16) - frac_x)*s10 + frac_x*s11);
> > > +
> > > +        dst_color[i] = ((int64_t)((1<<16) - frac_y)*s0 + (int64_t)frac_y*s1)>>32;
> > > +    }
> > 
> > frac_x >>= 6;
> > frac_y >>= 6;
> > for (i = 0; i < src_linestep; i++) {
> >     int s00 = src[src_linestep * int_x  + i + src_linesize * int_y ];
> >     int s01 = src[src_linestep * int_x1 + i + src_linesize * int_y ];
> >     int s10 = src[src_linestep * int_x  + i + src_linesize * int_y1];
> >     int s11 = src[src_linestep * int_x1 + i + src_linesize * int_y1];
> >     int s0 = (((1<<10) - frac_x)*s00 + frac_x*s01);
> >     int s1 = (((1<<10) - frac_x)*s10 + frac_x*s11);
> > 
> >     dst_color[i] = (((1<<10) - frac_y)*s0 + frac_y*s1)>>20;
> > }
> > 
> > you can also shift by just 4 instead of 6 and use unsigned values
> 
> Isn't this losing precision (and indeed producing a different output)?
> Which are the pros/cons?

64bit arithmetic is slow on hw that does not have native 64bit support
and with infinite precisse coeffs you will have +-0.5 errors
with 16bit ~0.502
with 12bit ~0.531

these worst case errors happen on black white transitions. in flatly
colored areas the difference is significantly smaller that is its all
pretty much 0.5 there

also mmx/sse optimized code would not use 64bit arithmetic so it would
produce different output when C is using 64bit arithmetic


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

The misfortune of the wise is better than the prosperity of the fool.
-- Epicurus
-------------- 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/20110504/2c48a070/attachment.asc>