[FFmpeg-devel] [PATCH 2/2] Add SIPR decoder for 5k0, 6k5 and 8k5 modes
Vitor Sessak
vitor1001
Mon Dec 21 15:43:02 CET 2009
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];
>> + }
>> +
>> + 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.
>> +
>> +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)
ff_weighted_vector_sumf(), but it takes a vector of floats and not
doubles as output.
-Vitor
More information about the ffmpeg-devel
mailing list