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

Michael Niedermayer michaelni
Sun Dec 20 03:53:02 CET 2009


On Sat, Dec 19, 2009 at 03:17:37PM +0100, Vitor Sessak wrote:
> 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.

ok


>
>> [...]
>>> +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.
[...]

> +static void lsp2lpc_sipr(const double *isp, float *Az)
> +{
> +    int lp_half_order = LP_FILTER_ORDER >> 1;
> +    double pa[lp_half_order+1], qa[lp_half_order];
> +    int i,j;
> +
> +    ff_lsp2polyf(isp    , pa, lp_half_order    );
> +    ff_lsp2polyf(isp + 1, qa, lp_half_order - 1);
> +
> +    for (i = lp_half_order-1; i > 1; i--)
> +        qa[i] -= qa[i-2];
> +
> +    for (i = 1, j = LP_FILTER_ORDER - 1; i < lp_half_order; i++, j--) {
> +        float paf = (pa[i] + qa[i]) * 0.5;
> +        float qaf = (pa[i] - qa[i]) * 0.5;
> +
> +        Az[i-1]  = paf + qaf * isp[LP_FILTER_ORDER - 1];
> +        Az[j-1]  = qaf + paf * isp[LP_FILTER_ORDER - 1];
> +    }
> +
> +    Az[lp_half_order - 1] = (1.0 + isp[LP_FILTER_ORDER - 1]) *
> +        pa[lp_half_order] * 0.5;
> +
> +    Az[LP_FILTER_ORDER - 1] = isp[LP_FILTER_ORDER - 1];
> +}

have you checked if this can be simplified?
that is do you understand the difference from it to ff_acelp_lspd2lpc() ?
especially what effect the -1 has in ff_lsp2polyf() / that is if the
iteration less isnt just merged into the following code ...


> +
> +static void sipr_decode_lp(float *lsfnew, const float *lsfold, float *Az,
> +                           int num_subfr)
> +{
> +    float *pAz = Az;
> +    double lsfint[LP_FILTER_ORDER];
> +    int i,j;
> +    float t, t0 = 1.0 / num_subfr;
> +
> +    t = t0 * 0.5;
> +    for (i = 0; i < num_subfr; i++) {

> +        for (j = 0; j < LP_FILTER_ORDER; j++)
> +            lsfint[j] = lsfold[j] * (1 - t) + t * lsfnew[j];

dont we have some function for this somewhere? (no i dont know its just
i faintly remember something like this but it could have been integer too)


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Thouse who are best at talking, realize last or never when they are wrong.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091220/a555c905/attachment.pgp>



More information about the ffmpeg-devel mailing list