[FFmpeg-devel] [PATCH] QCELP decoder

[Kenan Gillet]

On Oct 9, 2008, at 4:08 PM, Michael Niedermayer wrote:

> On Thu, Oct 09, 2008 at 12:54:17PM -0700, Kenan Gillet wrote:
>> Hi,
>> On Oct 8, 2008, at 5:46 PM, Michael Niedermayer wrote:
> [...]
>>>> +    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.
>>
>> I am working on it. Unfortunately, I do not have any background in  
>> audio
>> filtering so I am trying to make sense of the specs:
>> http://www.3gpp2.org/Public_html/Specs/C.S0020-0with3Gcover.pdf
>>
>> This particular part is on page 48, in section
>> '2.4.3.3.1 Converting the LSP Transmission Codes to LSP Frequencies'
>>
>> any help would be appreciated :)
>
> search for 0.02 in code/quantize.c of the referenc decoder

ok, thanks.

>
> [...]
>>> [...]
>>>> +/**
>>>> + * 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
>>
>> what about:
>>                     for (j = -4; j < 4; j++) {
>>                         if (index + j >= 0)
>>                             break;
>>                         hamm_tmp += qcelp_hammsinc_table[j + 4] *
>> memory[index + j];
>>                     }
>>                     for (; j < 4; j++) {
>>                         hamm_tmp += qcelp_hammsinc_table[j + 4] *
>> v_out [index + j];
>>                     }
>
> no, id like to see things in the same array or an explanation why this
> is a bad idea which may be of course.

if we combine the 2 arrays memory and v_out into an array
[memory elements, v_out elements]  it would simplify the code
for the case of fractional lag, but we would need to make an extra
memcpy to put it back into v_out; this extra memcpy would be
superfluous for the other cases


>
> [...]
>>> [...]
>>>> +/**
>>>> + * formant synthesis filter
>>>> + *
>>>> + * TIA/EIA/IS-733 2.4.3.1 (NOOOOT)
>>>
>>> NOOOOT ?
>>>
>>
>> don't know either, removed.
>
> ask reynaldo, i have a bad feeling about just removing comments that  
> are
> not understood at all

will do

Kenan