[FFmpeg-devel] [PATCH] Common ACELP code & G.729 [4/7] - G.729 core
Vladimir Voroshilov
voroshil
Wed Sep 17 12:58:19 CEST 2008
2008/9/17 Michael Niedermayer <michaelni at gmx.at>:
> On Sun, Sep 07, 2008 at 07:26:46PM +0700, Vladimir Voroshilov wrote:
>> +static const G729_format_description formats[] =
>> +{
>> + {8000, 10, 160, 13, 0xf892c,},
>> +// Note: may not work
>> + {4400, 11, 176, 17, 0xf966b,},
>> +};
>
> Is the last parameter really best specified in hex?
No differences for me. fixed.
>> +/**
>> + * \brief Decode LSP coefficients from L0-L3 (3.2.4).
>> + * \param lsfq [out] (2.13) decoded LSP coefficients
>> + * \param lq_prev [in/out] (2.13) quantized LSF coefficients from previous frames
>> + * \param lsf_prev [in/out] (2.13) LSF coefficients from previous frame
>> + * \param ma_predictor switched MA predictor of LSP quantizer
>> + * \param vq_1st first stage vector of quantizer
>> + * \param vq_2nd_low second stage lower vector of LSP quantizer
>> + * \param vq_2nd_high second stage higher vector of LSP quantizer
>> + */
>> +static void g729_lsf_decode(
>> + int16_t* lsfq,
>> + int16_t lq_prev[MA_NP][10],
>> + int16_t* lsf_prev,
>> + int16_t ma_predictor,
>> + int16_t vq_1st,
>> + int16_t vq_2nd_low,
>> + int16_t vq_2nd_high)
>> +{
>> + int i,j,k;
>> + static const uint8_t min_distance[2]={10, 5}; //(2.13)
>
>> + int16_t lq[10]; //(2.13)
>
> what does "lq" stand for?
I've looked into spec more carefully.
In this routine variables are (as they called in spec):
lsfq[i] - quantized LSF coefficients for the current frame (??(m) in spec)
lq[i] - current quantizer output (l?(m) in spec)
lq_prev[i] - previous quantizer outputs (l?(m?k) in spec)
Hm.. looks like selected names for lq and lq_prev variables are wrong.
I sugges renaming (same for restore_from_previous routine):
lsf_prev -> "lsfq_prev"
lq -> "quantizer_output"
lq_prev -> "past_quantizer_outputs"
> [...]
>> + g729_lq_rotate(lq_prev, lq);
>> +
>> + ff_acelp_reorder_lsf(lsfq, LSFQ_DIFF_MIN, LSFQ_MIN, LSFQ_MAX, 10);
>> +}
>> +
[...]
>> + for(i=0; i<10; i++)
>> + {
>> + tmp = lsfq[i] << 15;
>> +
>> + for(k=0; k<MA_NP; k++)
>> + tmp -= lq_prev[k][i] * cb_ma_predictor[ma_predictor_prev][k][i];
>> +
>> + lq[i] = ((tmp >> 15) * cb_ma_predictor_sum_inv[ma_predictor_prev][i]) >> 12;
>
> the cb_ma_predictor_sum_inv table is unneeded division by cb_ma_predictor_sum
> can be used
cm_ma_predictor_sum_inv was added especially to avoid division.
> also the code looks a little odd, its subtracting here but adding above ...
You are talking about lsf_decode and lsf_restore_from previous routines, right?
In lsf_decode lsfq[i] is calculated from lq[i]:
lsfq[i] = lq[i] * ma_pred_sum[k] + sum[k=0..3] { lq_prev[i][k] * ma_pred[i][k] }
lsf_restore_from_previous lq[i] is calculated from lsfq[i]:
lq[i] = [ lsfq[i] - sum[k=0..3]{ lq_prev[i][k] * ma_pred[i][k] } ] /
ma_pred_sum[k]
For some reason this calculation is used instead of simple lq[i] = sum
[k=0..2]{lq_prev[i][k] } / 3
>> + }
>> +
>> + g729_lq_rotate(lq_prev, lq);
>> +}
>
> g729_lq_rotate() can be factorized out of these 2 functions that are only
> called by if/else
In this case lq[i] should be moved outside from g729_lsf_decode and
g729_lsf_restore_from_previous routines (while it is used only there)
to upper level g729_decode_frame routine
--
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