[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