[FFmpeg-devel] [PATCH] Common ACELP code & G.729 [3/7] - vectors operations

[Vladimir Voroshilov]

Michael Niedermayer wrote: 
> On Fri, May 02, 2008 at 06:43:18PM +0700, Vladimir Voroshilov wrote:
> > This patch contains routines for operating with adaptive and algebraic vectors.
> > 
> > FIR filter was moved here.
> > It now does not use any "frac=-frac" or "if(frac<9)" and can be called
> > as foo(delay/6, delay%6).
> > This possibility was not obvious for me due to loop asymmetry,
> > introduced in reference code (and in specification too)
> > to avoid "out of bounds" error.
> 
> [...]
> > +static const uint8_t mul_5x[8] = {0, 5, 10, 15, 20, 25, 30, 35};
> 
> *5 should do :)

This was optimization attempt.
I'll remove all such tables.
If anyone want to optimize code,
benchmark it, fill free to do this after commit.

[...]

> > +static uint8_t gray_decode[32] =
> > +{
> > +    0,  1,  3,  2,  7,  6,  4,  5,
> > +   15, 14, 12, 13,  8,  9, 11, 10,
> > +   31, 30, 28, 29, 24, 25, 27, 26,
> > +   16, 17, 19, 18, 23, 22, 20, 21
> > +};
> 
> this is just x^(x>>1)

Wrong. 

4^(4>>1) = 4^2 = 6 != 7 = gray_decode[4]
5^(5>>1) = 5^2 = 7 != 6 = gray_decode[5]
and so on...

Perhaps it was intended to be so till G.729 guys touched it :)
After seeing fc_2pulses_9bits_track2 table i'm not surpised.


> of course only replace the table with above if its smaller/faster, same
> applies to the other suggested replacements for the tables
> 
> 
> [...]
> > +void ff_acelp_fc_4pulses_13bits(
> > +        int16_t* fc_v,
> > +        int fc_index,
> > +        int pulses_signs)
> > +{
> > +    int i;
> > +    int index;
> > +
> > +    for(i=0; i<3; i++)
> > +    {
> > +        index = i + mul_5x[fc_index & 7];
> > +        fc_v[ index ] += (pulses_signs & 1) ? 8191 : -8192; // +/-1 in (2.13)
> > +
> > +        fc_index >>= 3;
> > +        pulses_signs >>= 1;
> > +    }
> > +
> > +    index = 3 + (fc_index & 1) + mul_5x[(fc_index>>1) & 7];
> > +    fc_v[ index ] += (pulses_signs & 1) ? 8191 : -8192;
> > +}
> > +
> 

[...]

> > +void ff_acelp_fc_4pulses_21bits(
> > +        int16_t* fc_v,
> > +        int fc_index,
> > +        int pulses_signs)
> > +{
> > +    int i;
> > +    int index;
> > +
> > +    for(i=0; i<3; i++)
> > +    {
> > +        index = i + 5 *(fc_index & 15);
> > +        fc_v[ index ] += (pulses_signs & 1) ? 8191 : -8192; // +/-1 in (2.13)
> > +
> > +        fc_index >>= 4;
> > +        pulses_signs >>= 1;
> > +    }
> > +
> > +    index = 3 + (fc_index & 1);
> > +    fc_index >>= 1;
> > +    index += 5 * (fc_index & 15);
> > +    fc_v[ index ] += (pulses_signs & 1) ? 8191 : -8192;
> > +}
> > +
> 
> duplicate of ff_acelp_fc_4pulses_13bits()

Added static routine.
I want to keep those routines because i'm using pointers to them in
formats array.

> also the 
> fc_v[ index ] += (pulses_signs & 1) ? 8191 : -8192;
> stuff is duplicated all over the place

This line adds pulses to fixed vector.
Lines are the same just because sign bits are packed together each other (one per
index) and separately from index bits.
There are also different packing methods (sipr @16k, amr @10.2k)
I don't think implementing this 1 line of code via macro or inline
routine is a good idea.

 
-- 
Regards,
Vladimir Voroshilov mailto:voroshil at gmail.com
Omsk State University
JID: voroshil at jabber.ru
ICQ: 95587719
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 03_acelp_vectors41.diff
Type: text/x-diff
Size: 14281 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080503/d59c77df/attachment.diff>