[FFmpeg-devel] [PATCH] Common ACELP code & G.729 [2/7] - pitch lag decoding

[Michael Niedermayer]

On Wed, May 14, 2008 at 12:12:26AM +0700, Vladimir Voroshilov wrote:
> 2008/5/12 Michael Niedermayer <michaelni at gmx.at>:
> >
> > On Sun, May 11, 2008 at 09:42:26PM +0700, Vladimir Voroshilov wrote:
> >  > 2008/5/10 Michael Niedermayer <michaelni at gmx.at>:
> 
> [...]
> 
> >  >
> >  > >> +    energy =  MULL(ff_log2(energy),       -24660);
> >  > >
> >  > >> +    energy += MULL(ff_log2(subframe_size), 24660);
> >  > >> +    energy += 0x9c888; // 24660 * 26
> >  > >> +    energy += mean_energy;
> >  > >> +
> >  > >> +#ifdef G729_BITEXACT
> >  > >> +    /*
> >  > >> +       Reference code uses a constant instead of the two previous lines.
> >  > >> +       That value (due to 2^6 rounding) differs by 2 from code above.
> >  > >> +       Subtracting 2 from energy makes the result be bit-equal with
> >  > >> +       the reference code.
> >  > >> +    */
> >  > >> +    energy -= 2;
> >  > >> +#endif
> >  > >
> >  > > wouldnt it be easier to just pass the sum of these as argument?
> >  >
> >  > ok. moved to g729dec.c with comment.
> >
> >  its still there:
> >
> >
> >  > +    energy += MULL(ff_log2(subframe_size), 24660);
> >  > +    energy += 0x9c888; // 24660 * 26
> >  > +    energy += mean_energy;
> >
> 
> I misunderstood you.
> I thought you speak about correction only.
> Moved as constant into formats structure.
> But now i'm not sure about parameter's name.
> 
> > > +    // shift prediction energy vector
> >  > +    for(i=ma_pred_order-1; i>0; i--)
> >  > +        quant_energy[i] = quant_energy[i-1];
> >
> >  the comment is redundant, its more than obvious from the code
> 
> Removed.
> 
> >  > +
> >  > +    /* 3.9.1, Equation 72 */
> >
> >  of what? g729?
> 
> Fixed.
[...]
> +int ff_acelp_decode_lag3_1st_8_bits(int ac_index)
> +{
> +    ac_index += 59;
> +    if(ac_index > 255)
> +        ac_index = 3 * ac_index - 512;
> +    return ac_index;
> +}
> +
> +int ff_acelp_decode_lag3_2nd_8_4_bits(
> +        int ac_index,
> +        int pitch_lag_min)
> +{
> +    if(ac_index < 4)
> +        return 3*(ac_index + pitch_lag_min) + 1;
> +    else if(ac_index < 12)
> +        return 3*pitch_lag_min + ac_index + 7;
> +    else
> +        return 3*(ac_index + pitch_lag_min) - 17;
> +}
> +
> +int ff_acelp_decode_lag3_2nd_8_56_bits(
> +        int ac_index,
> +        int pitch_lag_min)
> +{
> +        return 3*pitch_lag_min + ac_index - 1;
> +}
> +
> +int ff_acelp_decode_lag6_9_6_bits(int ac_index)
> +{
> +    if(ac_index < 463)
> +        return ac_index + 107;
> +    else
> +        return 6*(ac_index - 368) + 2;
> +}
> +int ff_acelp_decode_lag6_2nd_9_6_bits(
> +        int ac_index,
> +        int pitch_lag_min)
> +{
> +    return 6*pitch_lag_min + ac_index - 1;
> +}

considering the size of these functions they should be static and in a header
And i wonder why they are seperate and not one like

if(!subframe){
}else{
    if(bits==4){
    }else{
    }
}



> +
> +void ff_acelp_update_past_gain_erasure(int16_t *quant_energy, int ma_pred_order)
> +{
> +    int avg_gain=quant_energy[ma_pred_order-1]; // (5.10)
> +    int i;
> +
> +    /* 4.4.3 of G.729, Equation 95 */
> +    for(i=ma_pred_order-1; i>0; i--)
> +    {
> +        avg_gain       += quant_energy[i-1];
> +        quant_energy[i] = quant_energy[i-1];
> +    }
> +#ifdef G729_BITEXACT
> +    if(ma_pred_order == 4)
> +    quant_energy[0] = FFMAX(avg_gain >> 2, -10240) - 4096; // -10 and -4 in (5.10)
> +    else
> +#endif
> +    quant_energy[0] = FFMAX(avg_gain / ma_pred_order, -10240) - 4096; // -10 and -4 in (5.10)
> +}

Is this function used anywhere with a ma_pred_order != 4 ?


> +
> +void ff_acelp_update_past_gain(int16_t* quant_energy, int gain_corr_factor, int ma_pred_order)
> +{
> +    int i;
> +
> +    for(i=ma_pred_order-1; i>0; i--)
> +        quant_energy[i] = quant_energy[i-1];
> +
> +    /* 3.9.1 of G.729, Equation 72
> +      quant_energy[0] = 20*log10(gain_corr_factor) in (3.12)
> +      24660 = 10/log2(10) in (2.13) */
> +    quant_energy[0] = (24660 * av_clip_int16((ff_log2(gain_corr_factor) >> 2) - (13 << 13))) >> 15;

(6165 * av_clip_int16((ff_log2(gain_corr_factor) >> 2) - (13 << 13))) >> 13;

and i assume the av_clip_int16() is needed?


> +}
> +
> +int16_t ff_acelp_decode_gain_code(
> +    int gain_corr_factor,
> +    const int16_t* fc_v,
> +    int mr_energy,
> +    const int16_t* quant_energy,
> +    const int16_t* ma_prediction_coeff,
> +    int subframe_size,
> +    int ma_pred_order)
> +{
> +    int i;
> +    int energy;
> +    int exp;
> +
> +    /* 3.9.1 of G.729, Equation 66 */
> +    energy = sum_of_squares(fc_v, subframe_size, 0, 0);
> +

> +    /* 24660 = 10/log2(10) in (2.13) */
> +    energy =  MULL(ff_log2(energy), -24660);
> +    /* mr_energy = Em + 10log(N) + 10log(2^26) */
> +    energy += mr_energy;
> +
> +    energy <<= 10; // (7.13) -> (7.23)
> +    /* 3.9.1 of G.729, Equation 69 */
> +    for(i=0; i<ma_pred_order; i++)
> +        energy += quant_energy[i] * ma_prediction_coeff[i];
> +
> +    /* 3.9.1 of G.729, Equation 71
> +      energy = 10^(energy / 20) = 2^(3.3219 * energy / 20) = 2^ (0.166 * energy)
> +      5439 = 0.166 in (0.15) */
> +    energy = (5439 * (energy >> 15)) >> 8; // (0.15) = (0.15) * (7.23)

energy= mr_energy - (ff_log2(energy)<<10);

for(i=0; i<ma_pred_order; i++)
    energy += quant_energy[i] * ma_prediction_coeff[i];

energy >>= 10;

with constants and tables stuff rescaled as needed


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

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- 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/6fa76149/attachment.pgp>