[FFmpeg-devel] [PATCH] Common ACELP code & G.729 [2/7] - pitch lag decoding
Michael Niedermayer
michaelni
Fri May 23 20:57:15 CEST 2008
On Sat, May 24, 2008 at 12:11:16AM +0700, Vladimir Voroshilov wrote:
> 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.
[...]
> +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 - 1;
> +}
can be simplified (to something like below assuming no typos ...)
int ff_acelp_decode_lag3_1st_8_bits(int ac_index)
{
ac_index += 58;
if(ac_index > 254)
ac_index = 3 * ac_index - 510;
return ac_index;
}
[...]
> +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 = -ff_log2(energy);
> + /* 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];
> +
> + energy >>= 11;
> +
> + /* The following code will calculate energy*2^14 instead of energy*2^exp
> + due to the recent change of the integer 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);
> +
> + /* 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) */
> + return bidir_sal(energy, exp - 25);
if id ignore all scaling and overflows then this is just:
energy= mr_energy;
for(i=0; i<ma_pred_order; i++)
energy += quant_energy[i] * ma_prediction_coeff[i];
return exp(energy) * gain_corr_factor / sum_of_squares(fc_v, subframe_size, 0, 0);
This makes me quite unhappy as there is a huge difference in complexity.
If we cant find a clean way with integers then i would suggest that this last
line is done using floats.
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
In a rich man's house there is no place to spit but his face.
-- Diogenes of Sinope
-------------- 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/20080523/4a8c8eb4/attachment.pgp>
More information about the ffmpeg-devel
mailing list