[FFmpeg-devel] [PATCH] QCELP decoder

[Kenan Gillet]

On Oct 17, 2008, at 3:44 PM, Diego Biurrun wrote:

> On Fri, Oct 17, 2008 at 09:46:31AM -0700, Kenan Gillet wrote:
>>
>> round 6 summary:
>> - no code change from round 5
>> - replace svn cp by svn add to make the patch smaller
>> - rename qcelp_math.[ch] into celp_math.[ch]
>
> Maybe we can rename some of the acelp_*.[ch] files to celp_*.[ch]
>
> already if it helps you.

sure or i can just add the code in acelp_*.[ch].
Either way is fine with me

>
>
>> - the patch is now broken into:
>>
>>    - qcelp-round6-doc-glue.patch.txt
>>    doc and glue code
>
> This patch is OK.
>
>> Let me know if I can do anything to ease you reviewing effort.
>
> The small patches are helping, some of these can be committed before  
> the
> others, that should speed things up.
>
>> --- libavcodec/qcelpdata.h	(revision 0)
>> +++ libavcodec/qcelpdata.h	(revision 0)
>> @@ -0,0 +1,500 @@
>> +/**
>> + * Unpacking bitmap tables for RATE_FULL
>
> Are these the unpacked tables or the tables used to unpack?

tables used to unpack


>> --- libavcodec/qcelpdec.c	(revision 0)
>> +++ libavcodec/qcelpdec.c	(revision 0)
>> @@ -0,0 +1,662 @@
>> + * @param memory buffer for the previous state of the filter
>> + *        - must be able to contain 303 elements
>> + *        - the 143 fist elements are from the previous state
>
> fiRst

done


>> +    v_out = memory + 143; // output vector start at memory[143]
>
> startS

done


>> +                if (pfrac[i]) { // If is a fractional lag...
>
> If it is

done


>> +static int decode_scaled_codebook_vector(QCELPContext *q, float  
>> cdn_vector[160]) {
>> +    float gain[16];
>
> Align the function parameters.

done


>
>> --- libavcodec/qcelp_lsp.c	(revision 0)
>> +++ libavcodec/qcelp_lsp.c	(revision 0)
>> @@ -0,0 +1,98 @@
>> +/**
>> + * Linear convolution of two vectors v1 and [1, cos, 1]
>> + *
>> + * @param v_out the result of the convolution,
>> + *              needs to be able to hold v_in_len + 2 elements
>> + * @param v_in the input vector
>> + * @param cos
>
> Yes?

second element in the second vector of the convolution [1, cos, 1]
I suspect that the function will need some rework to be more generic  
though.

>> + * @param v_in_len the dimension of v_in assumed to be in   
>> {2,4,6,8,10}
>
> nit: two spaces

done


>> +/**
>> + * Reconstructs LPC coefficients from the line spectral pair  
>> frequencies.
>> + *
>> + * TIA/EIA/IS-733 2.4.3.3.5
>> + */
>> +void qcelp_lspf2lpc(const float *lspf, float *lpc) {
>> +    float pa[5],qa[5];
>> +    float v1[12], v2[12];
>> +    float beta = 0.9883;
>> +    int   i;
>> +
>> +    v1[0] = 0.5;
>> +    v1[1] = 0.5;
>> +    lsp2poly(pa, lspf, v1, v2, 0);
>> +
>> +    v1[0] =  0.5;
>> +    v1[1] = -0.5;
>> +    lsp2poly(qa, lspf, v1, v2, 1);
>> +
>> +    for (i = 0; i < 5; i++) {
>> +        lpc[i] = -(pa[i] + qa[i]) * beta;
>> +        beta *= 0.9883;
>> +    }
>> +    for (i = 5; i < 10; i++) {
>> +        lpc[i] = -(pa[9-i] - qa[9-i]) * beta;
>> +        beta *= 0.9883;
>> +    }
>> +}
>
>> Index: libavcodec/celp_math.c
>> ===================================================================
>> --- libavcodec/celp_math.c	(revision 0)
>> +++ libavcodec/celp_math.c	(revision 0)
>> @@ -0,0 +1,40 @@
>> +
>> +/**
>> + * returns the dot product.
>> + * @param a input data array
>> + * @param b input data array
>> + * @param length number of elements
>> + *
>> + * @return dot product = sum of elementwise products
>> + */
>> +float ff_dot_product(const float* a, const float* b, int length)
>
> I think the Doxygen documentation in the header file is enough.

done


>> --- libavcodec/celp_math.h	(revision 0)
>> +++ libavcodec/celp_math.h	(revision 0)
>> @@ -0,0 +1,36 @@
>> +
>> +#ifndef AVCODEC_QCELP_MATH_H
>> +#define AVCODEC_QCELP_MATH_H
>
> Use the filename as multiple inclusion guard.

done