[FFmpeg-devel] [PATCH] QCELP decoder
Kenan Gillet
kenan.gillet
Sun Oct 5 18:17:37 CEST 2008
On Oct 4, 2008, at 4:38 AM, Vitor Sessak wrote:
> On Sat, Oct 4, 2008 at 12:48 AM, Kenan Gillet
> <kenan.gillet at gmail.com> wrote:
>> Hi,
>>
>> here is a round 2 of the patch based on feedback from Vitor and
>> Diego.
>> It includes:
>> - some spelling/grammar fixes,
>> - some cosmetics,
>> - changes to output float instead of int,
>> - bug fixes uncovered from the change of output,
>> - improvements to the pitch pre/synthesis filters.
>
> Thanks, some more comments:
>
>> +typedef struct {
>> + uint16_t x;
>> + uint16_t y;
>> +} qcelp_vector;
>> +
>> +/**
>> + * LSP Vector quantization tables in x*10000 form
>> + *
>> + * TIA/EIA/IS-733 tables 2.4.3.2.6.3-1 through 2.4.3.2.6.3-5
>> + */
>> +
>> +static const qcelp_vector qcelp_lspvq1[64] = {
>> +{ 327, 118},{ 919, 111},{ 427, 440},{1327, 185},
>
> It think it would be better to just have
>
> static const int16_t qcelp_lspvq1[64][2] = {
>
> This make it easier for someone who is not familiar with the code to
> understand it (one less struct to learn) without making it more
> complex.
>
done
> +static const double qcelp_rnd_fir_coefs[22] = {
>> + 0 , -1.344519e-1, 1.735384e-2, -6.905826e-2,
>> + 2.434368e-2, -8.210701e-2, 3.041388e-2, -9.251384e-2,
>> + 3.501983e-2, -9.918777e-2, 3.749518e-2, 8.985137e-1,
>> + 3.749518e-2, -9.918777e-2, 3.501983e-2, -9.251384e-2,
>> + 3.041388e-2, -8.210701e-2, 2.434368e-2, -6.905826e-2,
>> + 1.735384e-2, -1.344519e-1
>> +}; /*!< Start reading from [1]. */
>
> Can this be just a vector of floats or is the double preision really
> needed?
I think the double precision is needed to hold several values like
-9.251384E-2
coming from the specs, but it is borderline.
I tested the changes to float with tiny_psnr: it does not seem to
change the
ouput, because
- it is a rare case (frame is 1/4)
- the output is clipped
>
>
>> +static int apply_pitch_filters(QCELPContext *q, float *cdn_vector,
>> float *ppf_vector) {
>> + int i;
>> + uint8_t *pgain, *plag, *pfrac;
>> + float gain[4];
>> + int lag[4];
>> +
>> + if (q->rate == RATE_FULL ||
>> + q->rate == RATE_HALF ||
>> + (q->rate == I_F_Q && (q->prev_frame_rate == RATE_FULL ||
>> + q->prev_frame_rate == RATE_HALF))) {
>> +
>> + pfrac = q->data + QCELP_PFRAC0_POS;
>> +
>> + if (q->rate == RATE_FULL ||
>> + q->rate == RATE_HALF) {
>> + pgain = q->data + QCELP_PGAIN0_POS;
>> + plag = q->data + QCELP_PLAG0_POS;
>> +
>> + // Compute Gain & Lag for the whole frame
>> + for (i = 0; i < 4; i++) {
>> + gain[i] = plag[i] ? (pgain[i] + 1) / 4.0 : 0.0;
>> +
>> + lag[i] = plag[i] + 16;
>> +
>> + if (pfrac[i] && lag[i] >= 140) return -1;
>> + }
>> + memcpy(q->prev_pitch_lag, lag, sizeof(q-
>> >prev_pitch_lag));
>
> The lag[] buffer could be just a pointer, it is always the copy of
> something. Also this function does a lot of memcpy'ing, I wonder if
> they are all really needed.
>
> -Vitor
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
More information about the ffmpeg-devel
mailing list