[FFmpeg-devel] [PATCH] Common fixed-point ACELP routines (1/3) - math

Michael Niedermayer michaelni
Tue Apr 22 19:49:02 CEST 2008


On Tue, Apr 22, 2008 at 11:53:10PM +0700, Vladimir Voroshilov wrote:
> 
> Michael Niedermayer wrote: 
> > On Tue, Apr 22, 2008 at 09:12:16AM +0700, Vladimir Voroshilov wrote:
> > > On Tue, Apr 22, 2008 at 6:05 AM, Michael Niedermayer <michaelni at gmx.at> wrote:
> 
> [...]
> 
> > > >  These functions have nothing to do with acelp. log,cos,
> > > >  exp2 (yes please name it exp2 not pow2) have been known before acelp.
> > > 
> > > ff_acelp_pow2_14_x renamed to ff_acelp_exp2.
> > > 
> > > Do you want to say that i have choosen wrong file to place them in (as
> > > long as their prefix) ?
> > > Please suggest proper place and prefix and i'll fix it immediately.
> > 
> > I cant think of a good name ATM, i guess we can always rename it later
> > after its in svn ...
> 
> ok
> 
> > 
> > 
> > [...]
> > > +
> > > +/**
> > > + * Cosine table: ff_acelp_base_cos[i] = (1<<15) * cos(i*PI/64)
> > > + */
> > 
> > > +static const int16_t ff_acelp_base_cos[64] =
> > 
> > Because its static, base_cos should be fine. Same for the other tables
> 
> Fixed
> 
> > [...]
> > > +/**
> > > + * \brief fixed-point implementation of cosine in [0; PI) domain
> > > + * \param arg fixed-point cosine argument, 0 <= arg < 0x4000
> > > + *
> > > + * \return value of (1<<15) * cos(arg * PI / (1<<14)), -0x8000 <= result <= 0x7fff
> > > + */
> > > +int16_t ff_acelp_cos(uint16_t arg)
> > > +{
> > 
> > > +    uint8_t offset= arg & 0xff;
> > 
> > the & 0xff is uneeded
> 
> ok
> 
> > > +    uint8_t ind = arg >> 8;
> > > +
> > > +    assert(arg < 0x4000);
> > > +
> > > +    return FFMAX(ff_acelp_base_cos[ind] + ((ff_acelp_slope_cos[ind] * offset) >> 12), -0x8000);
> > > +}
> > 
> > The FFMAX is new i think, why is it needed now?
> 
> While writing comments i've rechecked min/max return values and found possible
> overflow. It doesn't triggers on any vectors but is theoretically possible.
> 

> > Also:
> >     uint8_t offset= arg;
> >     uint8_t ind   = arg >> 8;
> > 
> >     return base_cos[ind] + (((base_cos[ind+1] - base_cos[ind]) * offset) >> 8);
> > 
> > is slightly more accurate and needs just half the tables (add a -32768
> > at the end of base_cos)
> 
> Small note: original code uses 19 bit per fractional part of slope,
> while this is uses only 15.
> Didn't compare with float though.

Ive checked it against cos() the 15 bits where more accurate (unless i had
a bug in my code), dont ask me why, but the tables seem to be generated
naivly, that is tab[x]= cos(x). Not linear least squares based ones.


> 
> > Sadly this is not bitexact.
> > The question here is, is it possible at all to maintain bitexactness with
> > common code? 
> > I assume the other acelp codecs use slightly different integer
> > implementations?
> 
> This is rhetorical question :) ?
> 
> I prefer to keep bitexact code at least till commit into FFmpeg tree.
> This makes developments much simple and safe in terms of breaking anything.
> Of course i can keep bitexact routines in local tree only and commit more
> precise math operations to tree.
> Thus if new code gives acceptable results i'll not have anything against it.

We could keep bitexact routines under #ifdef for each acelp variant if you
want. That way there are both well written fast+precisse ones and the
g729 bitexact ones. That surely could come in handy if we stumble across a
g729 stream which decodes with artifacts.


[...]

> int16_t ff_acelp_cos(uint16_t arg)

ff_cos()


> {
>     uint8_t offset= arg;
>     uint8_t ind = arg >> 8;
> 
>     assert(arg <= 0x3fff);
> 
>     return tab_cos[ind] + (offset * (tab_cos[ind+1] - tab_cos[ind]) >> 8);
> }
> 
> /**
>  * \brief fixed-point implementation of exp2(x) in [0; 1] domain
>  * \param power argument to exp2, 0 <= power <= 0x7fff
>  *
>  * \return value of (1<<14) * exp2(power / (1<<15)) rounded to nearest, 0x4000 <= result <= 0x7fff
>  */

> int ff_acelp_exp2(uint16_t power)

ff_exp2()


> {
>     unsigned int result= exp2a[power>>10] + 0x10000;
> 
>     assert(arg <= 0x7fff);
>  
>     result= (result<<4) + ((result*exp2b[(power>>5)&31])>>16);
>     result=  result     + ((result*exp2c[ power    &31])>>18);

>     return (result + 32) >> 6;

IMHO the rounding should be done outside exp2(), this is just senslessly
throwing bits away.

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

I wish the Xiph folks would stop pretending they've got something they
do not.  Somehow I fear this will remain a wish. -- M?ns Rullg?rd
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080422/cb93386f/attachment.pgp>



More information about the ffmpeg-devel mailing list