[FFmpeg-devel] [PATCH] QCELP decoder

[Diego Biurrun]

On Fri, Oct 03, 2008 at 03:48:52PM -0700, Kenan Gillet wrote:
> 
> here is a round 2 of the patch based on feedback from Vitor and Diego.
> 
> --- libavcodec/qcelpdata.h	(revision 0)
> +++ libavcodec/qcelpdata.h	(revision 0)
> @@ -0,0 +1,397 @@
> +/**
> + * position of the bitmapping data for each pkt type in
> + * the big REFERENCE FRAME array

If 'pkt' means packet, then please write just packet.

> --- libavcodec/qcelpdec.c	(revision 0)
> +++ libavcodec/qcelpdec.c	(revision 0)
> @@ -0,0 +1,867 @@
> +
> +    uint8_t           data[77];               /*!< Data from a _parsed_ frame */

lowercase

> +    float             prev_iirf_scalefactor;  /*!< Last gain scale factor from the previous
> +                                                   frame (TIA/EIA/IS-733 2.4.8.6-6) */

ditto

> +static void interpolate_lspf(float *interpolated_lspf, const float curr_weight,
> +                             const float *curr_lspf, const float *prev_lspf) {
> +    int   i;
> +
> +    for (i=0;i<10;i++)

That for expression is in dire need of whitespace.

> +* Verify the sample rate and the number of channels.

samplerate

> +                   "Unofficial samplerate %d, but supported by Quicktime.", avctx->sample_rate);

QuickTime

> +    switch (q->rate) {
> +        case RATE_FULL:

I think the preferred indentation style is to keep case on the same
level as switch.

> +            // Provide smoothing of the energy of the unvoiced excitation

Provide smoothing of the unvoiced excitation energy

> +/**
> + * Computes the scaled codebook vector Cdn From INDEX and GAIN
> + * For all rates.
> + *
> + * The specification misses some information here.

is missing / lacks

> + * @param subframeno Size 40 subframe number that should be measured

lowercase

> + * @param q if not null, apply harcoded coef infinite impulse response filter

coefficient

> +/**
> + * Apply pitch synthetis filter and pitch pre-filter to the scaled book vector.

book vector?

> + * @param ppf_vector an array where the result of the filter will be stored in.

s/an array/array/, s/stored in./stored/

> + * Reconstructs LPC coefficients from the line spectral pairs frequencies.

I think this should be 'pair'.

> +/**
> + * Interpolates LSP frequencies and computes LPC coefficients for a given rate & pitch subframe.

Please break unnecessarily long lines like this one.

> + * @param QCELPContext q the context

stray q?

> +/**
> + * Formant synthesis filter
> + *
> + * @param vector in/out vector of the formant filter

Settle for the capitalized or uncapitalized version of Formant.

> +/*
> + * Figure out and set it in QCELPContext frame rate by its buffer size and/or by looking at
> + * the first byte of its buffer if applicable.

What does the first 'it' refer to?  I find this explanation very
confusing.

Also, settle for a consistent spelling of framerate.

> + * @return 0 if success, negative error number otherwise.

on success

> +/**
> + * Decodes the scaled codebook vector Cdn.
> + *
> + * @param cnd_vector Array where to put the generated scaled codebook vector

array

> + * @return 0  if successful, -1 if the frame must be erased

on success

Diego