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

[Michael Niedermayer]

On Sun, May 11, 2008 at 09:42:26PM +0700, Vladimir Voroshilov wrote:
> 2008/5/10 Michael Niedermayer <michaelni at gmx.at>:
> > On Fri, May 09, 2008 at 01:31:02PM +0700, Vladimir Voroshilov wrote:
> >> Michael Niedermayer wrote:
> >> > On Fri, May 02, 2008 at 06:36:34PM +0700, Vladimir Voroshilov wrote:
> 
> [...]
> 
> >> +int ff_acelp_decode_lag3_2nd_8_4_bits(
> >> +        int ac_index,
> >> +        int pitch_lag_min)
> >> +{
> >> +    ac_index &= 0xf;
> >> +
> >> +    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;
> >> +}
> >
> > if ac_index is 4 bits as the doxy says, then the & 0xf should not be needed
> 
> ok
> 
> >> +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. Equation 95 of G.729 */
> >> +    for(i=ma_pred_order-1; i>0; i--)
> >> +    {
> >> +        avg_gain       += quant_energy[i-1];
> >> +        quant_energy[i] = quant_energy[i-1];
> >> +    }
> >> +av_log(NULL, AV_LOG_ERROR, "%d %d %d\n", ma_pred_order, (avg_gain>>2), (avg_gain/ma_pred_order));
> >
> >> +    quant_energy[0] = FFMAX((avg_gain / ma_pred_order) - 4096, -14336); // -14 in (5.10)
> >> +//    quant_energy[0] = FFMAX((avg_gain >> 2) - 4096, -14336); // -14 in (5.10)
> >
> > why the commented out line?
> 
> Bitexact code. In attached patch it is enclosed into #ifdef
> 
> > and
> > quant_energy[0] = FFMAX(avg_gain / ma_pred_order, -10240) - 4096;
> > looks nicer to me
> 
> ok
> 
> >> +void ff_acelp_update_past_gain(int16_t* quant_energy, int gain_corr_factor, int ma_pred_order)
> >> +{
> >> +    int i;
> >> +
> >
> >> +    // shift prediction energy vector
> >> +    for(i=ma_pred_order-1; i>0; i--)
> >> +        quant_energy[i] = quant_energy[i-1];
> >
> > duplicate of above
> 
> hm. I don't see any advantage of separating one memmove
> into standalone routine (perhaps i should convert those two line
>  into memove), because it can not reuse one of two those
> routines from another without reduntant calculations.
> 
> [...]
> 
> >> +    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;



[...]
> +    // 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


> +
> +    /* 3.9.1, Equation 72 */

of what? g729?

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

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- 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/20080511/cde7a303/attachment.pgp>