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

[Michael Niedermayer]

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 subframeno,
> > > +        int pitch_lag_min)
> > > +{
> > > +    // (4.1.3 of G.729 and 5.6.1 of AMR)
> > > +    if (!subframeno)
> > > +        return decode_lag3_1st_8_bits(ac_index);
> > > +    else
> > > +        return 3*pitch_lag_min + ac_index - 1;
> > > +}
> > 
> > It seems as if it would be simpler to do the if (!subframeno) check outside
> > after all its always doing the same, this would also avoid having to pass
> > subframeno as argument.
> 
> See patch.

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


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

and
quant_energy[0] = FFMAX(avg_gain / ma_pred_order, -10240) - 4096;
looks nicer to me


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


> +
> +    /* 3.9.1, 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;
> +}

> +
> +int16_t ff_acelp_decode_gain_code(
> +    int gain_corr_factor,
> +    const int16_t* fc_v,
> +    int mean_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, Equation 66 of G.729 */
> +    energy = sum_of_squares(fc_v, subframe_size, 0, 0);
> +
> +    /*
> +      mean_energy is in (6.13)
> +      energy=mean_energy-E
> +      E is calculated as following (3.9.1 Equation 66 of G.729)
> +
> +      E = 10 * log10(energy / (2^26 * subframe_size))
> +      = 10 * log2(energy / (2^26 * subframe_size)) / log2(10)
> +      = 10*log2(energy/2^26)/log2(10) - 10*log2(subframe_size)/log2(10)
> +      = [10/log2(10)] * log2(energy/2^26) - [10/log2(10)] * log2(subframe_size)
> +      = 24660 * log2(energy) - 24660 * log2(subframe_size) - 24660 * 26
> +
> +      24660 = 10/log2(10) in (2.13)
> +    */
> +    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?


> +
> +    energy <<= 10; // (7.13) -> (7.23)
> +    /* 3.9.1, Equation 69 of G.729*/
> +    for(i=0; i<ma_pred_order; i++)
> +        energy += quant_energy[i] * ma_prediction_coeff[i];
> +
> +    /* 3.9.1, Equation 71 of G.729*/
> +    /*
> +      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)
> +
> +    /*
> +      The following code will calculate energy*2^14 instead of energy*2^exp
> +      due to the recent change of the ineger part of energy_int.
> +      This is done to avoid overflow. Result fits into 16 bit.
> +    */
> +    exp = (energy >> 15);             // integer part (exponent)
> +    // Only fraction part of (0.15) and rounding

> +    energy = ((ff_exp2(energy & 0x7fff) + 16) >> 5) & 0x7fff;

is the second & 0x7fff needed?


> +
> +    // apply correction
> +    energy *= gain_corr_factor >> 1; // energy*2^14 in (3.12)
> +

> +    // energy*2^14 in (3.12) -> energy*2^exp in (14.1)
> +    if(exp > 25)
> +        energy <<= exp - 25;
> +    else
> +        energy >>= 25 - exp;

seperate function


[...]
-- 
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/20080509/26e38ce4/attachment.pgp>