[FFmpeg-devel] [PATCH] QCELP decoder
Kenan Gillet
kenan.gillet
Fri Oct 17 00:38:21 CEST 2008
Hi,
On Oct 16, 2008, at 8:34 AM, Diego Biurrun wrote:
> On Tue, Oct 14, 2008 at 06:59:34PM -0700, Kenan Gillet wrote:
>>
>> Here is a summary of the round 4 patch:
>
> some more comments, mostly nits...
thanks for your review.
>> --- libavcodec/qcelpdata.h (revision 0)
>> +++ libavcodec/qcelpdata.h (revision 0)
>> +
>> +#ifndef AVCODEC_QCELPDATA_H
>> +#define AVCODEC_QCELPDATA_H
>> +
>> +static const uint16_t qcelp_bits_per_rate[6] = {
>
> You will need to #include at least stdint.h for this file to pass
> 'make
> checkheaders'. Please test your code with 'make checkheaders'.
done
>> + * YOU WILL NOT SEE ANY mention of a REFERENCE nor an UNIVERSAL
>> frame
>> + * in the specs, this is just some internal way of handling the
>
> Split this sentence in two at the comma.
this whole comment has been removed after cleanup asked
by Michael Niedermayer.
>
>> + * each bit in the input with respect to a transmission code in the
>
> nit: two spaces
same as above.
>> + {19,3},{19,2},{19,1},{19,0}
>> +};
>> +static const QCELPBitmap qcelp_rate_4thr_bitmap[] = {
>> +static const QCELPBitmap qcelp_rate_4thr_bitmap[] = {
>
> 4thr?
renamed qcelp_rate_4thr_bitmap to qcelp_rate_quarter_bitmap
renamed qcelp_rate_8thr_bitmap to qcelp_rate_octave_bitmap
> nit: You left a blank line between the other tables, do it here as
> well.
done
>> --- libavcodec/qcelpdec.c (revision 0)
>> +++ libavcodec/qcelpdec.c (revision 0)
>> + // Low-pass filter the LSP frequencies
>
> .
done
>> + // Check for badly received packets
>
> .
done
>> + 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[6] = 0.4*ga[3]+0.6*ga[4];
>> + gain[7] = ga[4];
>
> A few spaces between operators would make this look less cramped.
done space around +
>
>
>> + * The reason for this mistake seems to be the fact they forget to
>> mention you
>
> forgot
done
>> + * @param cnd_vector array for the generated scaled codebook vector
>> + */
>> +static void compute_svector(const QCELPContext *q, const float
>> *gain, float *cdn_vector) {
>
> cdn_vector or cnd_vector?
done
>> + * @param gain array of gain per subframe, each element is between
>> 0.0 and 2.0
>
> per-subframe gain array
done
>> + * @param lag array of lag per subframe, each element is
>
> per-subframe lag array
done
>> + * @param pfrac array of boolean per subframe, 1 if the lag is
>> fractional, 0 otherwise
>
> per-subframe boolean array
done
>> + * @param v_out the output vector of the filter
>
> filter output vector
done
>> + * @param v_in the input vector of the filter
>
> filter input vector
done
>> +/**
>> + * Apply pitch synthesis filter and pitch pre-filter to the scaled
>> codebook vector.
>
> I would say 'prefilter', same in other places.
done
>> + // Compute Gain & Lag for the whole frame
>
> .
done
> gain, lag
done
>> + memset(q->pfrac, 0, 4 *sizeof(*q->pfrac));
>> + memcpy(q->plag, q->prev_pitch_lag, 4 * sizeof(*q-
>> >plag));
>
> memset(q->pfrac, 0, 4 * sizeof(*q->pfrac));
> memcpy(q->plag, q->prev_pitch_lag, 4 * sizeof(*q->plag));
>
>> + * @param memory the previous state of the filter
>
> s/the//
done
>> + av_log(avctx, AV_LOG_ERROR, "frame #%d: unknown framerate,
>> unsupported size: %d\n",
>> + warn_insufficient_frame_quality(avctx, "framerate is 1/8
>> and first 16 bits are on.");
>> + warn_insufficient_frame_quality(avctx, "wrong data in
>> reserved frame area.");
>> + warn_insufficient_frame_quality(avctx, "sanity check of
>> the codebook gain failed.");
>> + warn_insufficient_frame_quality(avctx, "badly received
>> packets in frame.");
>> + warn_insufficient_frame_quality(avctx, "cannot initialize
>> pitch filter.");
>
> Please capitalize all these messages.
done
>
>
>> + // WIP Adaptive postfilter should be here
>
> adaptive
done
>> + .long_name = NULL_IF_CONFIG_SMALL("QCLEP / PureVoice"),
>
> QCELP
done
>> --- doc/general.texi (revision 15618)
>> +++ doc/general.texi (working copy)
>> @@ -388,6 +388,7 @@
>> @item Nellymoser ASAO @tab X @tab X
>> + at item QCELP / PureVoice @tab X @tab X
>
> Ummm, I don't think you have implemented an encoder as well, so just
> put
> an X in the decoder column.
done
Kenan
More information about the ffmpeg-devel
mailing list