[FFmpeg-devel] [PATCH] QCELP decoder

[Kenan Gillet]

On Nov 12, 2008, at 11:57 AM, Michael Niedermayer wrote:

> On Wed, Nov 12, 2008 at 09:05:02AM -0800, Kenan Gillet wrote:
>>
>> On Nov 11, 2008, at 10:00 AM, Michael Niedermayer wrote:
>>
>>> On Sun, Nov 09, 2008 at 09:49:20PM -0800, Kenan Gillet wrote:
> [...]
>>>> +        const float *predictors = (q->prev_framerate !=
>>>> RATE_OCTAVE ||
>>>> +                                   q->prev_framerate != I_F_Q ? q-
>>>>> prev_lspf
>>>> +                                                              : q-
>>>>> predictor_lspf);
>>>
>>> hmmmmmmm
>>> hmm
>>>
>>> Which value is not unequal to either RATE_OCTAVE or I_F_Q ?
>>
>> q->prev_framerate : framerate of the previous frame
>> q->framerate: framerate of the current frame
>> :)
>
> there is no q->framerate in there :)
>
>
>>
>>
>> The predictor changes based on the rate of the previous frame.
>>
>>
>>>
>>>
>>> Are you testing the code in each patch iteration?
>>
>> I do check the output to wav of all the test files I have for every
>> commit in my svn.
>
> great, now i feel better, this is a good strategy
>
> [...]
>>>> +        break;
>>>> +    case RATE_OCTAVE:
>>>> +        cbseed = q->first16bits;
>>>> +        for (i = 0; i < 8; i++) {
>>>> +            tmp_gain = gain[i] * (QCELP_SQRT1887 / 32768.0);
>>>> +            for (j = 0; j < 20; j++) {
>>>> +                cbseed = 521 * cbseed + 259;
>>>> +                *cdn_vector++ = tmp_gain * (int16_t)cbseed;
>>>> +            }
>>>> +        }
>>>> +        break;
>>>> +    case I_F_Q:
>>>> +        cbseed = -44; // random codebook index
>>>> +        for (i = 0; i < 4; i++) {
>>>> +            tmp_gain = gain[i] * QCELP_RATE_FULL_CODEBOOK_RATIO;
>>>> +            for (j = 0; j < 40; j++)
>>>> +                *cdn_vector++ = tmp_gain *
>>>> qcelp_rate_full_codebook[cbseed++ & 127];
>>>> +        }
>>>> +        break;
>>>> +    }
>>>> +}
>>>> +
>>>
>>>> +/**
>>>> + * Apply generic gain control.
>>>> + *
>>>> + * @param v_out output vector
>>>> + * @param v_in gain-controlled vector
>>>> + * @param v_ref vector to control gain of
>>>> + *
>>>> + * TIA/EIA/IS-733 2.4.8.3-2/3/4/5, 2.4.8.6
>>>> + */
>>>> +static void apply_gain_ctrl(float *v_out,
>>>> +                            const float *v_ref,
>>>> +                            const float *v_in) {
>>>> +    int   i, j, len;
>>>> +    float scalefactor;
>>>> +
>>>> +    for (i = 0, j = 0; i < 4; i++) {
>>>> +        scalefactor = ff_dot_productf(v_in + j, v_in + j, 40);
>>>
>>>> +        if (scalefactor) {
>>>> +            scalefactor = sqrt(ff_dot_productf(v_ref + j, v_ref +
>>>> j, 40) / scalefactor);
>>>> +            for (len = j + 40; j < len; j++)
>>>> +                v_out[j] = scalefactor * v_in[j];
>>>> +        } else {
>>>> +            memset(v_out + j, 0,  40 * sizeof(float));
>>>> +            j += 40;
>>>> +        }
>>>
>>> assuming this is correct
>>
>> it is undefined in the specs and the reference code is setting
>> scalefactor
>>  only if both dot product are not zero but sill assign
>> v_out[j] = v_in[j] * scalefactor
>>  with the unassigned scalefactor :(
>
> interresting, i guess then a  if() av_log_missing_feature() would be
> a good idea, so we will get a sample that triggers this

 From my investigation, an initial SILENCE frame could trigger it,
and on this particular case, v_in & v_out would be 0 vectors. I don't
think another case could trigger it but i have not investigate enough
to be sure, maybe a very long succession of SILENCE might trigger it
too.

I am not sure that carrying the AVCodecContext into this function to add
an av_log_missing_feature would be worth it.

But again it is up to you and Reynaldo.

Kenan