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

Vladimir Voroshilov voroshil
Sun May 18 03:25:54 CEST 2008


2008/5/18 Michael Niedermayer <michaelni at gmx.at>:
> 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>:
>>

[...]

>> +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

Agree

> And i wonder why they are seperate and not one like
>
> if(!subframe){
> }else{
>    if(bits==4){
>    }else{
>    }
> }
>

1. Because you asked me to remove subframe
parameter from those routines.
2. Because *_lag3_* and _*lag6_* calculate value with different
precision (1/3 vs 1/6)
3. Becase second subframe is calculated differently for 8_4 and 8_56.

>> +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 ?

in fixed-point SIPR 16k (i still keeping it synced with common routines)
with ma_pred_order == 2

>> +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?

It can be removed if gain_corr_factor will be >=512
This is not issue with G.729 since above condition is always true.
But for G.729D gain_corr_factor can be even zero
(while used log10(gain_corr_factor) formulas in spec shows that it should't)

Do you agree with adding above check in main loop for G.729D only and
removing av_clip_int16 from the routine ?

>> +}
>> +
>> +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);

where is "energy =  MULL(ff_log2(energy), -24660);" ?
this expression can't be evaluated with any constants/tables rescaling, imho.

>
> 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

i'll check this.


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




More information about the ffmpeg-devel mailing list