[FFmpeg-devel] [PATCH 4/5] Add the G723.1 demuxer and decoder

[Vitor Sessak]

Hi, some more comments:

On 03/17/2011 11:56 PM, banan at ludd.ltu.se wrote:

> +/**
> + * Bitexact implementation of 2ab scaled by 1/2^16.
> + *
> + * @param a 32 bit multiplicand
> + * @param b 16 bit multiplier
> + */
> +#define MULL2(a, b) \
> +        ((((a)>>  16) * (b)<<  1) + (((a)&  0xffff) * (b)>>  15))

I suppose the decoder is not bitexact to the reference if you just use 
MULL(a,b, 15), no?

> +/**
> + * Generate a train of dirac functions with period as pitch lag.
> + */
> +static void gen_dirac_train(int16_t *buf, int pitch_lag)
> +{
> +    int16_t vector[SUBFRAME_LEN];
> +    int i, j;
> +
> +    memcpy(vector, buf, SUBFRAME_LEN * sizeof(int16_t));
> +    for (i = pitch_lag; i < SUBFRAME_LEN; i += pitch_lag) {
> +        for (j = 0; j < SUBFRAME_LEN - i; j++)
> +            buf[i + j] += vector[j];
> +    }
> +}

The memcpy bothers me, but I see no obvious way of getting rid of it.

> +/**
> + * Generate adaptive codebook excitation.
> + */
> +static void gen_acb_excitation(int16_t *vector, int16_t *prev_excitation,
> +                               int pitch_lag, G723_1_Subframe subfrm,
> +                               Rate cur_rate)
> +{
> +    int16_t residual[SUBFRAME_LEN + PITCH_ORDER - 1];
> +    const int16_t *cb_ptr;
> +    int lag = pitch_lag + subfrm.ad_cb_lag - 1;
> +
> +    int i;
> +    int64_t sum;
> +
> +    get_residual(residual, prev_excitation, lag);
> +
> +    /* Select quantization table */
> +    if (cur_rate == Rate6k3 && pitch_lag < SUBFRAME_LEN - 2) {
> +        cb_ptr = adaptive_cb_gain85;
> +    } else
> +        cb_ptr = adaptive_cb_gain170;
> +
> +    /* Calculate adaptive vector */
> +    cb_ptr += subfrm.ad_cb_gain * 20;
> +    for (i = 0; i < SUBFRAME_LEN; i++) {
> +        sum = ff_dot_product(residual + i, cb_ptr, PITCH_ORDER, 1);
> +        vector[i] = av_clipl_int32((sum << 1) + (1 << 15)) >> 16;
> +    }

I might be missing something, but I see a lot of "<< 1" and ">> 1" in 
the code. While I understand why in fixed point math we usually shift by 
15 or 16, what is the reason for the shifts by one?

-Vitor