[FFmpeg-devel] [PATCH] QCELP decoder
Michael Niedermayer
michaelni
Thu Oct 9 02:46:11 CEST 2008
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
[...]
> +/**
> + * 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
> + 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.
> +
> + // 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 ...
[...]
> +/**
> + * 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
[...]
> + // 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?
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 ...
[...]
> + 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
[...]
> +/**
> + * 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 ...
[...]
> +/**
> + * 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
> + } 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
> + }
> + 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 ...
[...]
> +/**
> + * formant synthesis filter
> + *
> + * TIA/EIA/IS-733 2.4.3.1 (NOOOOT)
NOOOOT ?
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
The educated differ from the uneducated as much as the living from the
dead. -- Aristotle
-------------- 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/20081009/a51cb1fa/attachment.pgp>
More information about the ffmpeg-devel
mailing list