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

[Michael Niedermayer]

On Thu, May 22, 2008 at 05:50:28PM +0700, Vladimir Voroshilov wrote:
> 2008/5/22 Michael Niedermayer <michaelni at gmx.at>:
> > On Mon, May 19, 2008 at 08:26:27AM +0700, Vladimir Voroshilov wrote:
> >> 2008/5/19 Michael Niedermayer <michaelni at gmx.at>:
> >> > On Mon, May 19, 2008 at 12:47:16AM +0700, Vladimir Voroshilov wrote:
> >> >> 2008/5/18 Michael Niedermayer <michaelni at gmx.at>:
> >> >> > On Sun, May 18, 2008 at 01:23:20PM +0700, Vladimir Voroshilov wrote:
> >> >>
> >> >> [...]
> >> >>
> >> >> >> What about such uniform routine?
> >> >> >
> >> >> > I think its a mess.
> >> >> >
> >> >> > tab, tab2, pulse_count, bits should be arguments to the function not some
> >> >> > enum which sets them in a switch()
> >> >> >
> >> >>
> >> >> like this?
> >> > [...]
> >> >> +const uint8_t fc_2pulses_9bits_track2[32] =
> >> >> +{
> >> >> +   0,  2,   4,
> >> >> +   5,  7,   9,
> >> >> +   10, 12, 14,
> >> >> +   15, 17, 19,
> >> >> +   20, 22, 24,
> >> >> +   25, 27, 29,
> >> >> +   30, 32, 34,
> >> >> +   35, 37, 39,
> >> >> +   1,
> >> >> +   6,
> >> >> +   11,
> >> >> +   16,
> >> >> +   21,
> >> >> +   26,
> >> >> +   31,
> >> >> +   36
> >> >> +};
> >> >> +
> >> >> +const uint8_t fc_2pulses_9bits_track1[16] =
> >> >> +{
> >> >> +    1,  3,
> >> >> +    6,  8,
> >> >> +    11, 13,
> >> >> +    16, 18,
> >> >> +    21, 23,
> >> >> +    26, 28,
> >> >> +    31, 33,
> >> >> +    36, 38
> >> >> +};
> >> >> +
> >> >> +const uint8_t fc_4pulses_8bits_tracks_13[16] =
> >> >> +{
> >> >> +  0, 5, 10, 15, 20, 25, 30, 35, 40, 45, 50, 55, 60, 65, 70, 75,
> >> >> +};
> >> >> +
> >> >> +const uint8_t fc_4pulses_8bits_track_4[32] =
> >> >> +{
> >> >> +    3,  4,
> >> >> +    8,  9,
> >> >> +    13, 14,
> >> >> +    18, 19,
> >> >> +    23, 24,
> >> >> +    28, 29,
> >> >> +    33, 34,
> >> >> +    38, 39,
> >> >> +    43, 44,
> >> >> +    48, 49,
> >> >> +    53, 54,
> >> >> +    58, 59,
> >> >> +    63, 64,
> >> >> +    68, 69,
> >> >> +    73, 74,
> >> >> +    78, 79,
> >> >> +};
> >> >> +
> >> >> +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
> >> >> +};
> >> >
> >> > Are the tables which use gray_decode also used without it?
> >> > If no merging gray_decode into them would reduce complexity.
> >>
> >> Due to error in the second table, it is not used
> >> anywhere except G.729D, first can be reused in AMR @5.9k
> >> I'd applied gray coding to both and add _gray suffix.
> >>
> >> *_track1 (without gray coding) is kept as copy.
> >> Also kept gray_decoding table under #if 0
> >
> > [...]
> >
> >> +void ff_acelp_fc_enchance_harmonics(
> >> +        int16_t* fc_v,
> >> +        int pitch_delay,
> >> +        int16_t gain_pitch,
> >> +        int length)
> >> +{
> >> +    int i;
> >> +
> >> +    for(i=pitch_delay; i<length;i++)
> >> +        fc_v[i] += (fc_v[i - pitch_delay] * gain_pitch) >> 14;
> >> +}
> >> +
> >> +void ff_acelp_build_excitation(
> >> +        int16_t* exc,
> >> +        const int16_t *ac_v,
> >> +        const int16_t *fc_v,
> >> +        int16_t gp,
> >> +        int16_t gc,
> >> +        int subframe_size)
> >> +{
> >> +    int i;
> >> +
> >> +    // Clipping required here; breaks OVERFLOW test.
> >> +    for(i=0; i<subframe_size; i++)
> >> +        exc[i] = av_clip_int16((ac_v[i] * gp + fc_v[i] * gc + 0x2000) >> 14);
> >> +}
> >
> > duplicate, these 2 functions do the same, that is a wheighted sum of 2
> > vectors.
> >
> 
> agree.
> As i see
> 
> ff_acelp_fc_enchance_harmonics(fc_v, pitch_delay, gain_pitch, length)
> 
> is equal to
> 
> ff_acelp_build_excitation(fc_v+pitch_delay, fc_v+pitch_delay,
> fc,1<<14, gain_pitch, length-pitch_delay);
> 
> but what should i do with rounding ?
> 
> what about adding additional parameter (const summand) to
> ff_acelp_build_excitation
> I.e. something like:
> 
> void ff_acelp_build_excitation(
>         int16_t* exc,
>         const int16_t *ac_v,
>         const int16_t *fc_v,
>         int16_t gp,
>         int16_t gc,
>         int16_t summand,
>         int subframe_size)
> {
>     int i;
> 
>     // Clipping required here; breaks OVERFLOW test.
>     for(i=0; i<subframe_size; i++)
>         exc[i] = av_clip_int16((ac_v[i] * gp + fc_v[i] * gc + summand) >> 14);
> }
> 
> What about renaming ff_acelp_build_excitation like ff_acelp_weighted_vector_sum
> (and subframe_size -> length, ac_v -> vect_a, fc_v -> vect_b, gc ->
> weight_coeff_a, gp -> weight_coeff_b)?

ok


> 
> P.S. "summand" don't look like the best name too.

maybe offset or rounder are better?


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

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- 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/20080522/187174d5/attachment.pgp>