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

[Michael Niedermayer]

On Sun, May 18, 2008 at 01:39:42AM +0700, Vladimir Voroshilov wrote:
> 2008/5/18 Michael Niedermayer <michaelni at gmx.at>:
> > On Sat, May 17, 2008 at 10:25:05PM +0700, Vladimir Voroshilov wrote:
> >> 2008/5/14 Michael Niedermayer <michaelni at gmx.at>:
> >> > On Wed, May 14, 2008 at 12:08:01AM +0700, Vladimir Voroshilov wrote:
> >> >> 2008/5/12 Michael Niedermayer <michaelni at gmx.at>:
> >> >> > On Sun, May 11, 2008 at 09:46:04PM +0700, Vladimir Voroshilov wrote:
> >> >> >  > 2008/5/9 Vladimir Voroshilov <voroshil at gmail.com>:
> >>
> >> [...]
> >>
> >> >> >  > > Hm. I hope, this version is better..
> >> >> >
> >> >> >  there are still 2 seperate functions and now there are even wraper functions
> >> >> >  this is not better IMHO.
> >> >> >  see my example above, is there a problem with it?
> >> >>
> >> >> Already implemented yours routine.
> >> >> Now i should remove those wrappers,
> >> >> make common routine non-static and
> >> >> use it in main loop, right?
> >> >
> >> > yes
> >> >
> >> >
> >> >>
> >> >> >  > +/**
> >> >> >  > + * low-pass FIR (Finite Impulse Response) filter coefficients
> >> >> >  > + *
> >> >> >  > + *   A similar filter is named b30 in G.729.
> >> >> >  > + *
> >>
> >> [...]
> >>
> >> >> >  Maybe the generic interpolation should be in a seperate file.
> >> >>
> >> >> Move it back to acelp_filters together with corresponding lookup tables?
> >> >
> >> > yes, it seems o fit better there
> >>
> >> Attached patch is version with  removed interpolation filter.
> > [...]
> >
> >> +void ff_acelp_4pulses_13_21_bits(
> >> +        int16_t* fc_v,
> >> +        int fc_index,
> >> +        int pulses_signs,
> >> +        int bits)
> >> +{
> >> +    int mask = (1 << bits) - 1;
> >> +    int i, index;
> >> +
> >> +    for(i=0; i<3; i++)
> >> +    {
> >> +        index = i + 5 * (fc_index & mask);
> >> +        fc_v[ index ] += (pulses_signs & 1) ? 8191 : -8192; // +/-1 in (2.13)
> >> +
> >> +        fc_index >>= bits;
> >> +        pulses_signs >>= 1;
> >> +    }
> >> +
> >> +    index = 3 + (fc_index & 1) + 5 * ((fc_index>>1) & mask);
> >> +    fc_v[ index ] += (pulses_signs & 1) ? 8191 : -8192;
> >> +}
> >> +
> >> +void ff_acelp_fc_2pulses_9bits_g729d(
> >> +        int16_t* fc_v,
> >> +        int fc_index,
> >> +        int pulses_signs)
> >> +{
> >> +    int index;
> >> +
> >> +    index = ((5 * gray_decode[fc_index & 15]) >> 1) + 1;
> >> +    fc_v[ index ] += (pulses_signs & 1) ? 8191 : -8192; // +/-1 in (2.13)
> >> +
> >> +    pulses_signs >>= 1;
> >> +    fc_index >>= 4;
> >> +
> >> +    index = fc_2pulses_9bits_track2[gray_decode[fc_index & 31]];
> >> +    fc_v[ index ] += (pulses_signs & 1) ? 8191 : -8192; // +/-1 in (2.13)
> >> +}
> >
> > duplicate
> >
> > My suggestion for the third time:
> >
> >> >  > >>     for(i=0; i<pulse_count; i++){
> >> >  > >>         index = i + tab[fc_index & mask];
> >> >  > >>         fc_v[ index ] += (pulses_signs & 1) ? 8191 : -8192;
> >> >  > >>
> >> >  > >>         fc_index >>= bits;
> >> >  > >>         pulses_signs >>= 1;
> >> >  > >>     }
> >> >  > >>
> >> >  > >>     index = tab2[fc_index];
> >> >  > >>     fc_v[ index ] += pulses_signs ? 8191 : -8192;
> >
> > Or is there some reason why you do not use this? So far you didnt say
> > why not IIRC.
> 
> Did you see that second one is using gray decoding when first is not?
> Or you mean restoring "5*i" tables, one with gray coding and one
> without (you asked
> me to remove such tables some time ago)
> and pass them (as long as fc_2pulses_9bits_track2 table) via parameters?

yes, that was the idea, i think that the 50 bytes or so of tables extra
are better than the near duplicated function.
And sorry for the back and forward, it wasnt clear to me that the silly
5*i table would allow the code to be sigificantly simplified.

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

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- 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/20080517/cb476f28/attachment.pgp>