[FFmpeg-devel] [PATCH] QCELP decoder
Diego Biurrun
diego
Thu Oct 9 23:59:55 CEST 2008
On Sun, Oct 05, 2008 at 09:49:59AM -0700, Kenan Gillet wrote:
> thank you for your reviewing. Here is a round3 of he decoder.
Build system part is OK, documentation and changelog updates are
missing.
> --- libavcodec/qcelpdata.h (revision 0)
> +++ libavcodec/qcelpdata.h (revision 0)
> @@ -0,0 +1,398 @@
> + * lspv[10] 60 61 62 63 64 65 66 67 68 69 (LSP for RATE_OCTAVE, LSPV for other rate)
I think you need to say 'rateS' here.
> + * What follows are the reference frame slices. Each tuple will be mapped
> + * to a QCELPBitmap showing the location of each bit in the input with respect
> + * to a transmission code in the 'universal frame'.
nit: needlessly long line
> +/**
> + * LSP Vector quantization tables in x*10000 form
vector
> --- libavcodec/qcelpdec.c (revision 0)
> +++ libavcodec/qcelpdec.c (revision 0)
> @@ -0,0 +1,812 @@
> + uint8_t lspv[10]; /*!< LSP for RATE_OCTAVE, LSPV for other rate */
rateS, see above
> +/**
> +* Verify the samplerate and the number of channels.
> +* Initialize the speech codec to the specs.
spec or specs?
And I think you mean "according to" instead of "to".
> +static int qcelp_decode_init(AVCodecContext *avctx) {
> + QCELPContext *q = avctx->priv_data;
> + int i;
I think there is no need to align the variable names here.
> + for (i = 0; i < 10; i++) {
> + q->prev_lspf[i] = (i + 1) / 11.;
> + }
useless {}
> +/**
> + * Decodes the 10 quantized LSP frequencies from the LSPV/LSP
> + * transmission codes of any framerate and check for badly
> + * received packets.
nit: This can be shortened to two lines.
> + } 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;
> + }
more pointless {}
There are more cases below. In FFmpeg generally the form without {} is
preferred.
> +static int decode_gain_and_index(QCELPContext *q, float *gain) {
> + int i, g1[16];
> + float ga[16], gain_memory, smooth_coef;
strange alignment
> + for (i = 0; i < 16; i++) {
> + if (q->framerate == RATE_HALF && i>=4) break;
I dislike putting the statement right after the if instead of on a new
line, it makes the code harder to read.
> + * Computes energy of the subframeno-ith subvector. These values are
the energy
> + * @param vector vector from which to measure the subframe's energy
vector to measure the subframe energy of
There should be better ways to say this.
> + * @param q if not null, apply harcoded coefficient infinite impulse response filter
harDcoded
> + * @param in vector to control gain off
of
> + * @param out gain-controled output vector
controlled
> + * @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]) {
The order of the last two parameters is swapped wrt the function signature.
> + float hamm_tmp;
hamming?
> + * @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) {
ppf_vector?
> + * @param cos
?
> +void interpolate_lpc(QCELPContext *q, const float *curr_lspf, float *lpc, const int subframe_num) {
Sometimes you break long function declarations, sometimes you do not.
> +/**
> + * formant synthesis filter
> + *
> + * TIA/EIA/IS-733 2.4.3.1 (NOOOOT)
> + *
> + * @param vector in/out vector of the formant filter
> + */
> +static void do_formant(float *vector, const float *lpc, float *memory) {
missing parameter documentation
> + av_log(avctx, AV_LOG_WARNING,
> + "Framerate byte is missing, guessing the framerate from packet size.\n");
from the
> + * @param cnd_vector array where to put the generated scaled codebook vector
array for the generated scaled codebook vector
> + * @return 0 on successful, -1 if the frame must be erased
on success
> + warn_insufficient_frame_quality(avctx, "cannot initiliaze pitch filter.");
initialize
Quickly running your files through a spellchecker will likely find more
of these..
Diego
More information about the ffmpeg-devel
mailing list