[FFmpeg-devel] [PATCH 2/2] Add SIPR decoder for 5k0, 6k5 and 8k5 modes

Vitor Sessak vitor1001
Sat Dec 19 15:17:37 CET 2009


Michael Niedermayer wrote:
> On Fri, Dec 18, 2009 at 06:07:45PM +0100, Vitor Sessak wrote:
>> Diego Biurrun wrote:
>>> On Fri, Dec 18, 2009 at 05:52:13PM +0100, Vitor Sessak wrote:

[...]

>> Gave a try to a different formatting.
>>
>> Here is a new patch with these and a few other minor cosmetics fixed.
> 
> [...]
>> +typedef struct {
>> +    const char *mode_name;
>> +    uint16_t bits_per_frame;
> 
>> +    uint8_t subframe_size;
> 
> constant?

Not for 16k mode, but removed anyway.

>> +    uint8_t subframe_count;
>> +    uint8_t frames_per_packet;
> 
>> +    float treshpit;
> 
> treshpit ?

Indeed, bad name. Renamed.

>> +
>> +    /* bitstream parameters */
>> +    uint8_t number_of_fc_indexes;
>> +
>> +    /** size in bits of the i-th stage vector of quantizer */
>> +    uint8_t vq_indexes_bits[5];
>> +
>> +    /** size in bits of the adaptive-codebook index for every subframe */
>> +    uint8_t pitch_delay_bits[5];
>> +
>> +    uint8_t gp_index_bits;
>> +    uint8_t fc_index_bits[10]; ///< size in bits of the fixed codebook indexes
> 
>> +    uint8_t gc_index_bits; ///< size in bits of the gain codebook indexes
> 
> always 7 ?

Not for 16k.

> [...]
>> +static void dequant(float *out, const int *idx, const float *cbs[])
>> +{
>> +    int i;
>> +
>> +    int stride  = 2;
>> +    int num_vec = 5;
>> +
>> +    for (i = 0; i < num_vec; i++)
>> +        memcpy(out + stride*i, cbs[i] + stride*idx[i], stride*sizeof(float));
> 
> you dont need local vars for constants

They differ for 16k and this way of writing it minimize the patch to add 
it. I agree that normally this is not ok, but AFAIK only this, the 
previous change and fc_index_bits been constant are the only things 
affected by it, so I think it is worth it.

> [...]
>> +static void sipr_decode_lp(float *lsfnew, const float *lsfold, float *Az,
>> +                           int num_subfr, int filter_order)
>> +{
>> +    float *pAz = Az;
>> +    double lsfint[filter_order];
>> +    int i,j;
>> +    float t, t0 = 1.0 / num_subfr;
>> +
> 
>> +    t = t0/2;
> 
> *0.5 divide is slow and gcc not trustworthy
> 
> filter_order seems constant so why pass it into the func?

Agreed and fixed both (and elsewhere too).

>> +    for (i = 0; i < num_subfr; i++) {
>> +        for (j = 0; j < filter_order; j++)
>> +            lsfint[j] = lsfold[j] * (1 - t) + t * lsfnew[j];
>> +
>> +        lsp2lpc_sipr(lsfint, pAz, filter_order);
>> +        pAz += filter_order;
>> +        t += t0;
>> +    }
>> +}
>> +
>> +/**
>> + * Evaluate the adaptative impulse response
>> + */
> 
>> +static void eval_ir(const float *Az, int pitch_lag, float *freq,
>> +                    float treshpit, int length)
>> +{
>> +    float tmp1[L_SUBFR_SIPR+1], tmp2[LP_FILTER_ORDER+1];
>> +    int i;
>> +
>> +    tmp1[0] = 1.;
>> +    for (i = 0; i < LP_FILTER_ORDER; i++) {
>> +        tmp1[i+1] = Az[i] * ff_pow_0_55[i];
>> +        tmp2[i  ] = Az[i] * ff_pow_0_7 [i];
>> +    }
>> +    memset(tmp1 + 11, 0, 37*sizeof(float));
>> +
>> +    ff_celp_lp_synthesis_filterf(freq, tmp2, tmp1, L_SUBFR_SIPR,
>> +                                 LP_FILTER_ORDER);
>> +
>> +    pitch_sharpening(length, pitch_lag, treshpit, freq);
> [...]
>> +    for (i = 0; i < LP_FILTER_ORDER; i++) {
>> +        lpc_d[i] = lpc[i] * ff_pow_0_75[i];
>> +        lpc_n[i] = lpc[i] *    pow_0_5 [i];
>> +    };
>> +
>> +    memcpy(pole_out - LP_FILTER_ORDER, ctx->postfilter_mem,
>> +           LP_FILTER_ORDER*sizeof(float));
>> +
>> +    ff_celp_lp_synthesis_filterf(pole_out, lpc_d, samples, L_SUBFR_SIPR,
>> +                                 LP_FILTER_ORDER);
> 
> looks somewhat similar, dunno if it can be factorized ...

No good idea from me neither. I hate these similar, but not really 
easily mergeable, chunks...

> [...]
>> +static float lsf_cb5[32][2] = {
> 
> const
> 
> [...]
>> +static float pred[4] = {
> 
> const

Done everywhere.

> and i assume you checked that these tables are not duplicates from other
> similar codecs?

I checked AMR, which was the biggest source of "inspiration" for SIPR. 
Any other ideas?

New patch attached.

-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: add_sipr2_3.diff
Type: text/x-patch
Size: 39415 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091219/400010ea/attachment.bin>



More information about the ffmpeg-devel mailing list