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

Michael Niedermayer michaelni
Tue Dec 22 01:18:07 CET 2009


On Mon, Dec 21, 2009 at 03:43:02PM +0100, Vitor Sessak wrote:
> Michael Niedermayer wrote:
>> 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];

    paf=  pa[i]           *(1+isp[LP_FILTER_ORDER - 1]);
    qaf= (qa[i] - qa[i-2])*(1-isp[LP_FILTER_ORDER - 1]);
    Az[i-1]  = (paf + qaf)*0.5;
    Az[j-1]  = (paf - qaf)*0.5;

btw what value does isp[LP_FILTER_ORDER - 1] have?
its not 1 or -1 or something trivial ?

and above needs qa[-1]=0


>>> +    }
>>> +
>>> +    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 ...
>
>
> I've tried before and just gave a second try. If I change the code to not 
> take the -1 (I have to undo the last iteration), I have:
>
>> static void lsp2lpc_sipr(double *isp, float *Az)
>> {
>>     int lp_half_order = LP_FILTER_ORDER >> 1;
>>     double pa[lp_half_order+1], qa[lp_half_order+1];
>>     int i,j;
>>     ff_lsp2polyf(isp    , pa, lp_half_order);
>>     ff_lsp2polyf(isp + 1, qa, lp_half_order);
>>     qa[1] += 2 * isp[LP_FILTER_ORDER-1];
>>     for(j=2; j < lp_half_order; j++)
>>         qa[j] -= - 2 * qa[j-1] * isp[LP_FILTER_ORDER-1] + qa[j-2];
>>     for (i = lp_half_order-1; i > 1; i--)
>>         qa[i] -= qa[i-2];
>>     for (i = 0; i < lp_half_order; i++) {
>>         pa[i] *= (1 + isp[LP_FILTER_ORDER-1]);
>>         qa[i] *= (1 - isp[LP_FILTER_ORDER-1]);
>>     }
>>     for (i = 1, j = LP_FILTER_ORDER - 1; i < lp_half_order; i++, j--) {
>>         Az[i-1] = (pa[i] + qa[i]) * 0.5;
>>         Az[j-1] = (pa[i] - qa[i]) * 0.5;
>>     }
>>     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];
>> }
>
> From which I don't see any obvious simplification.

:(

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

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- 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/20091222/cebdf6a2/attachment.pgp>



More information about the ffmpeg-devel mailing list