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

[Vladimir Voroshilov]

2008/5/18 Michael Niedermayer <michaelni at gmx.at>:
> On Sun, May 18, 2008 at 08:25:54AM +0700, Vladimir Voroshilov wrote:
>> 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>:
>> >>
> [...]
>> >> +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
>
> quant_energy[0] = FFMAX(avg_gain >> log2_ma_pred_order, -10240) - 4096; // -10 and -4 in (5.10)
>
> ?

This will require either additional parameter (ma_pred_order,
log2_ma_pred_order - duplication ?) or explicit call to av_log2.
Neither of the cases looks good for me.

>
>
>>
>> >> +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)
>
> hmm log(0) ...

Suprise! :P

>
>>
>> Do you agree with adding above check in main loop for G.729D only and
>> removing av_clip_int16 from the routine ?
>
> If its needed iam fine with the check here or in the main loop.

It is better there, because is not used by G.729 but only by G.729D.


>>
>> >> +}
>> >> +
>> >> +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);" ?
>
> gone because *5439 undoes it
> 24660*5439 ~ 2^27
>
>
>> this expression can't be evaluated with any constants/tables rescaling, imho.
>
> scale ma_prediction_coeff[] and mr_energy by 2^C / 24660

Here is result.
P.S. i've also changed lag decoding routines to return 3*lag instead of 3*lag+1,
since ff_acelp_interpolate now requires rounding to lowest and
(lag_3x+1)/3 with lag_3x/3 in main code looks better for me (due to
obvious rounding) than  lag3x/3 and (lag_3x-1)/3 accordingly.

-- 
Regards,
Vladimir Voroshilov mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 02_acelp_lag58.diff
Type: text/x-diff
Size: 13992 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080524/89603880/attachment.diff>