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

[Vladimir Voroshilov]

2008/5/23, Michael Niedermayer <michaelni at gmx.at>:
> On Thu, May 22, 2008 at 10:27:13PM +0700, Vladimir Voroshilov wrote:
>  > 2008/5/22, Michael Niedermayer <michaelni at gmx.at>:
>  > > On Thu, May 22, 2008 at 05:50:28PM +0700, Vladimir Voroshilov wrote:
>  >
>  > [...]
>  >
>  > >  > >> +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?
>  > >
>  >
>  > here is the patch.
>  > I have also added "shift" parameter.
>  > This will simplify reusing from g729 postfilter (since it
>  > uses vectors with different fixed-point base).
>
> [...]
>
> > +void ff_acelp_fc_pulse_per_track(
>
>
> id call it ff_acelp_decode_fixed_codebook_vector())
>

SIPR @16k and some of AMR modes use
fixed codebook with two pulses per one track (and thus
different coding scheme: one sign bit grouped together
with two indices instead of one sing bit and one index in
the routine above). It will be hard to merge two such
codings in one routine.
This is why i choosed such name (i was thinking about
future implementation of two pulses coding scheme in future).


>
>
>  > +        int16_t* fc_v,
>  > +        const uint8_t *tab1,
>  > +        const uint8_t *tab2,
>
>  > +        int fc_index,
>
>
> pulses_indices seems to fit better with pulses_signs
>  also i think it should be pulse_signs / indices not pulses

ok

>
>  except these patch ok

fine



-- 
Regards,
Vladimir Voroshilov     mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719