[FFmpeg-devel] [PATCH] QCELP decoder
Kenan Gillet
kenan.gillet
Thu Oct 9 21:54:17 CEST 2008
Hi,
On Oct 8, 2008, at 5:46 PM, Michael Niedermayer wrote:
> On Sun, Oct 05, 2008 at 09:49:59AM -0700, Kenan Gillet wrote:
>> Hi,
>> thank you for your reviewing. Here is a round3 of he decoder.
>> - the parser has been dropped.
>> - the unpacking has been revamped.
>> - some code has been simplify & factorize.
>> - some useless memcpy has been removed.
>
> [...]
>
>> +#define QCELP_FULLRATE_CODEBOOK(i) qcelp_fullrate_ccodebook[i] /
>> 100.0
>
> * .001 is faster than / 100.0
done
> [...]
>> +/**
>> + * Decodes the 10 quantized LSP frequencies from the LSPV/LSP
>> + * transmission codes of any framerate and check for badly
>> + * received packets.
>> + *
>> + * TIA/EIA/IS-733 2.4.3.2.6.2-2, 2.4.8.7.3
>> + */
>> +static int decode_lspf(QCELPContext *q, float *lspf) {
>
> the return value should be documented
done
>> + int i;
>> + float predictor, tmp_lspf;
>> +
>> + if (q->framerate == RATE_OCTAVE) {
>> + q->octave_count++;
>> +
>> + for (i = 0; i < 10; i++) {
>> + lspf[i] = (i + 1) / 11.
>> + + (q->lspv[i] ? QCELP_LSP_SPREAD_FACTOR *
>> QCELP_LSP_OCTAVE_PREDICTOR
>> + : -QCELP_LSP_SPREAD_FACTOR *
>> QCELP_LSP_OCTAVE_PREDICTOR);
>> + }
>> +
>> + // Check the stability of the LSP frequencies.
>> + if (lspf[0] < QCELP_LSP_SPREAD_FACTOR)
>> + lspf[0] = QCELP_LSP_SPREAD_FACTOR;
>> + for (i = 1; i < 10; i++) {
>> + if (lspf[i] < lspf[i-1] + QCELP_LSP_SPREAD_FACTOR)
>> + lspf[i] = lspf[i-1] + QCELP_LSP_SPREAD_FACTOR;
>> + }
>> + if (lspf[9] > 1.0 - QCELP_LSP_SPREAD_FACTOR)
>> + lspf[9] = 1.0 - QCELP_LSP_SPREAD_FACTOR;
>> + for (i = 9; i > 0; i--) {
>> + if (lspf[i-1] > lspf[i] - QCELP_LSP_SPREAD_FACTOR)
>> + lspf[i-1] = lspf[i] - QCELP_LSP_SPREAD_FACTOR;
>> + }
>
> this stuff has to be fixed before the patch can be commited, and yes
> i would give you a hint how if i knew what is intended.
I am working on it. Unfortunately, I do not have any background in audio
filtering so I am trying to make sense of the specs:
http://www.3gpp2.org/Public_html/Specs/C.S0020-0with3Gcover.pdf
This particular part is on page 48, in section
'2.4.3.3.1 Converting the LSP Transmission Codes to LSP Frequencies'
any help would be appreciated :)
>> +
>> + // Low-pass filter the LSP frequencies
>> + if (q->octave_count < 10) {
>> + interpolate_lspf(lspf, 1 - 0.125, lspf, q->prev_lspf);
>> + } else {
>> + interpolate_lspf(lspf, 1 - 0.9, lspf, q->prev_lspf);
>> + }
>> +
>> + } else if (q->framerate == I_F_Q) {
>> + predictor = QCELP_LSP_OCTAVE_PREDICTOR * -
>> QCELP_LSP_SPREAD_FACTOR;
>> + if (q->erasure_count > 1) {
>> + predictor *= (q->erasure_count < 4 ? 0.9 : 0.7);
>> + }
>> + for (i = 0; i < 10; i++) {
>> + lspf[i] = (i + 1) / 11. + predictor;
>> + }
>> +
>> + // Low-pass filter the LSP frequencies
>> + interpolate_lspf(lspf, 1 - 0.875, lspf, q->prev_lspf);
>> + } else {
>> + q->octave_count = 0;
>> +
>> + tmp_lspf = 0.;
>
>> + for (i = 0; i < 10 ; i++) {
>> + lspf[i] =
>> + tmp_lspf += qcelp_lspvq[i / 2][q->lspv[i / 2]][i &
>> 1] / 10000.0;
>> + }
>
> i would write:
> for (i = 0; i < 5 ; i++) {
> lspf[2*i+0] = tmp_lspf += qcelp_lspvq[i][q->lspv[i]][0] * 0.0001;
> lspf[2*i+1] = tmp_lspf += qcelp_lspvq[i][q->lspv[i]][1] * 0.0001;
> }
> but its a little nitpicking ...
done
> [...]
>> +/**
>> + * Converts codebook transmission codes to GAIN and INDEX.
>> + *
>> + * TIA/EIA/IS-733 2.4.6.2
>> + */
>> +static int decode_gain_and_index(QCELPContext *q, float *gain) {
>> + int i, g1[16];
>> + float ga[16], gain_memory, smooth_coef;
>> +
>> + switch (q->framerate) {
>> + case RATE_FULL:
>> + case RATE_HALF:
>> + for (i = 0; i < 16; i++) {
>> + if (q->framerate == RATE_HALF && i>=4) break;
>> +
>> + g1[i] = 4 * q->cbgain[i];
>
>> + if (q->framerate == RATE_FULL && i > 0 && !((i+1) &
>> 3)) {
>
> the i>0 check is redundant
done
> [...]
>> + // Provide smoothing of the unvoiced excitation energy.
>> + gain[0] = ga[0];
>> + gain[1] = 0.6*ga[0]+0.4*ga[1];
>> + gain[2] = ga[1];
>> + gain[3] = 0.2*ga[1]+0.8*ga[2];
>> + gain[4] = 0.8*ga[2]+0.2*ga[3];
>> + gain[5] = ga[3];
>
>> + gain[7] = 0.4*ga[3]+0.6*ga[4];
>> + gain[7] = ga[4];
>
> you didnt mean 7 twice, did you?
no,definitely not :)
changed
> btw, is there some reference sw or reference bitstreams?
> if so patch acceptance requires the decoder to decode the bitstreams
> sufficiently correct (that is if some hard numbers are required for
> conformance then at least that well) ...
> errors like the above would be caught by such tests ...
I don't know of any but I'll do some more research.
> [...]
>> + cbseed = q->first16bits;
>> + for (i = 0; i < 160; i++) {
>> + cbseed = 521 * cbseed + 259;
>> + cdn_vector[i] = gain[i/20]
>> + * QCELP_SQRT1887 / 32768.0 *
>> (int16_t)cbseed;
>> + }
>
> division is a rather expensive operation ...
> 2 loops would avoid the /20 and (QCELP_SQRT1887 / 32768.0) would
> ensure that
> the constants are combined at compile time
done
> [...]
>> +/**
>> + * Apply generic gain control.
>> + *
>> + * @param q if not null, apply harcoded coefficient infinite
>> impulse response filter
>> + * @param in vector to control gain off
>> + * @param out gain-controled output vector
>> + *
>> + * TIA/EIA/IS-733 2.4.8.3-4/5, 2.4.8.6
>> + */
>> +static void apply_gain_ctrl(const float *in, float *out) {
>
> param q?
> i see no q ...
done
> [...]
>> +/**
>> + * Apply filter in pitch-subframe steps.
>> + *
>> + * @param memory a buffer for the previous state of the filter
>> + * @param gain array of gain per subframe, each element is between
>> 0.0 and 2.0
>> + * @param lag array of lag per subframe, each element is
>> + * between 16 and 143 if its corresponding pfrac is 0,
>> + * between 16 and 139 otherwise
>> + * @param pfrac array of boolean per subframe, 1 if the lag is
>> fractional, 0 otherwise
>> + * @param v_in the input vector of the filter
>> + * @param v_out the output vector of the filter
>> + */
>> +static void do_pitchfilter(float *memory,
>> + const float gain[4], const uint8_t
>> *lag, const uint8_t pfrac[4],
>> + float v_out[160], const float
>> v_in[160]) {
>> + int i, j, k, index;
>> + float hamm_tmp;
>> +
>> + memory += 143; // Lookup must be done starting at the end of
>> the buffer.
>> +
>> + for (i = 0; i < 4; i++) {
>> + if (gain[i] != 0.0) {
>> + index = 40 * i - lag[i];
>> + for (k = 0; k < 40; k++) {
>> + if (pfrac[i]) { // If is a fractional lag...
>> + hamm_tmp = 0.0;
>
>> + for (j = -4; j < 4; j++) {
>> + hamm_tmp += qcelp_hammsinc_table[j + 4]
>> + * (index + j < 0 ? memory[index
>> + j] : v_out [index + j]);
>> + }
>
> having a check in the innermost loop to switch between
> 2 arrays is not a good idea. the code should be changed so the
> data is in the same array if possible
what about:
for (j = -4; j < 4; j++) {
if (index + j >= 0)
break;
hamm_tmp += qcelp_hammsinc_table[j + 4] *
memory[index + j];
}
for (; j < 4; j++) {
hamm_tmp += qcelp_hammsinc_table[j + 4] *
v_out [index + j];
}
also qcelp_hammsinc_table is symetric and could be reduce to 4 elements,
but i am not sure if it worth the extra complexity.
>> + } else {
>> + hamm_tmp = index < 0 ? memory[index] :
>> v_out[index];
>> + }
>> + v_out[40 * i + k] = v_in[40 * i + k] + gain[i] *
>> hamm_tmp;
>> + index++;
>
>> + }
>> + }
>> + else {
>> + memcpy(v_out + 40 * i, v_in + 40 * i, 40 * sizeof(float));
>> + }
>
> funny indention
done
>> + }
>> + memcpy(memory - 143, v_out + 17, 143 * sizeof(float));
>> +}
>> +
>
>> +/**
>> + * Apply pitch synthetis filter and pitch pre-filter to the scaled
>> codebook vector.
>> + * TIA/EIA/IS-733 2.4.5.2
>> + *
>> + * @param q the context
>> + * @param cdn_vector the scaled codebook vector
>> + * @param ppf_vector array where the result of the filter will be
>> stored
>> + *
>> + * @return 0 if everything goes well, -1 if frame should be erased
>> + */
>> +static int apply_pitch_filters(QCELPContext *q, float *cdn_vector) {
>
> this function has 2 params, 3 are documented ...
removed
> [...]
>> +/**
>> + * formant synthesis filter
>> + *
>> + * TIA/EIA/IS-733 2.4.3.1 (NOOOOT)
>
> NOOOOT ?
>
don't know either, removed.
More information about the ffmpeg-devel
mailing list