[FFmpeg-devel] [PATCH] TwinVQ decoder
Vitor Sessak
vitor1001
Mon Jun 8 22:01:40 CEST 2009
Michael Niedermayer wrote:
> On Mon, May 18, 2009 at 06:32:55PM +0200, Vitor Sessak wrote:
>> Michael Niedermayer wrote:
>>> On Wed, May 06, 2009 at 05:09:50PM +0200, Vitor Sessak wrote:
>>>> Vitor Sessak wrote:
>>>>> Hi,
>>>>> I'm running out of ideas on how to improve this code, so I'm submitting
>>>>> it for review. Among the things I'm not 100% happy with is the windowing
>>>>> mess and the set_interleave_table_* code duplication. Any suggestion is
>>>>> welcome.
>>>>> About using dsputils->vector_fmul, it gets two parameters (one for
>>>>> input/output and other for the window), and I need one that gets
>>>>> three...
>>>>> Finally, it would be very practical to start the review by the tables
>>>>> since they are so big that every time I send this patch the message need
>>>>> moderator approval...
>>>> Ping?
>>>>
>>>> New version attached...
>>> [...]
[...]
>> /**
>> * Evaluate a single LPC amplitude spectrum envelope coefficient from the line
>> * spectrum pairs
>> *
>> * @param cos_lsp a vector of the cosinus of the LSP values
>> * @param cos_val cos(PI*i/N) where i is the index of the LPC amplitude
>> * @return the LPC value
>> *
>> * @todo reuse code from vorbis_dec.c: vorbis_floor0_decode
>> */
>> static float eval_lpc_spectrum(const float *cos_lsp, float cos_val, int size)
>> {
>> int i;
>> float a = .5;
>> float b = .5;
>>
>> for (i=0; i < size; i += 2) {
>> a *= 2*cos_val - cos_lsp[i ];
>> b *= 2*cos_val - cos_lsp[i+1];
>> }
>>
>> a *= a*(2+2*cos_val);
>> b *= b*(2-2*cos_val);
>>
>> return .5/(a+b);
>> }
>
> if iam not too tired the .5 and 2 are useless
It is, but it is like that to make it easier to see in what way it is
similar to vorbis_floor0_decode().
[...]
>> static void extend_pitch(int a1, const float *a2, float a3, float *a4,
>> TwinContext *tctx)
>> {
>> const ModeTab *mtab = tctx->mtab;
>> int v5;
>> int v33;
>> int v35;
>> int fcmax_i, fcmin_i;
>> int i, j;
>> int basf_step = (1 << mtab->basf_bit) - 1;
>> int isampf = tctx->avctx->sample_rate/1000;
>> int ibps = tctx->avctx->bit_rate/(1000 * tctx->avctx->channels);
>>
>
>> fcmin_i = (40*2*mtab->size + isampf/2)/isampf;
>>
>> fcmax_i = (40*6*2*mtab->size + isampf/2)/isampf;
>
> vertical align
>
>
>> v33 = fcmin_i + (a1*(fcmax_i - fcmin_i) + basf_step/2)/basf_step;
>>
>> if ((isampf == 22) && (ibps == 32))
>> v35 = ((v33 + 800LL)* mtab->pitch_cst * mtab->pit_cb_len * v33 +
>> 200LL * mtab->size*v33)/
>> (400LL * mtab->size * v33);
>
> looks like some things can be facored out here
Without making it yet more ugly?
> and, btw vXY are unacceptable variable names (this one must be fixed and
> all of them)
>
>
>> else
>> v35 = (mtab->pitch_cst * mtab->pit_cb_len * v33)/(400 * mtab->size);
>>
>> for (i=0; i < v35/2; i++)
>> a4[i] += a3 * a2[i];
>>
>> v5 = v35/2;
>>
>> for (i=0; i < mtab->pit_cb_len; i++) {
>> int v38 = very_broken_op(v33, i + 1);
>> for (j = - v35/2; j < (v35 + 1)/2 ; j++) {
>> a4[j+v38] += a3 * a2[v5];
>> ++v5;
>>
>> if (v5 >= mtab->pit_cb_len)
>> return;
>> }
>> }
>> }
>>
>
>> static void dec_gain(enum FrameType ftype, float *out, TwinContext *tctx)
>> {
>> const ModeTab *mtab = tctx->mtab;
>> int i, j;
>> int sub = mtab->fmode[ftype].sub;
>> float step = AMP_MAX / ((1 << GAIN_BITS) - 1);
>> float sub_step = SUB_AMP_MAX / ((1 << SUB_GAIN_BITS) - 1);
>>
>> if (ftype == FT_LONG) {
>> for (i=0; i < tctx->avctx->channels; i++)
>> out[i] = (1./(1<<10)) *
>> mulawinv(step * .5 + step * get_bits(&tctx->gb, GAIN_BITS),
>> AMP_MAX, MU);
>> } else {
>> for (i=0; i < tctx->avctx->channels; i++) {
>> float val = (1./(1<<20)) *
>> mulawinv(step *.5 + step * get_bits(&tctx->gb, GAIN_BITS),
>> AMP_MAX, MU);
>>
>> for (j=0; j < sub; j++) {
>> out[i*sub + j] =
>> val*mulawinv(sub_step*.5 +
>> sub_step* get_bits(&tctx->gb, SUB_GAIN_BITS),
>> SUB_AMP_MAX, MU);
>> }
>> }
>> }
>> }
>
> inefficient, please loose mulawinv() and the pow() it calls
I've tried a LUT, and it doesn't affect total decoding time in a
(easily) measurable way. I changed the pow() call to just an exp()
(without an addition log() if the compiler do his job properly).
>> static void check_lsp(int a1, float *a2, float a3)
>> {
>> int i;
>>
>> for (i=0; i < a1; i++)
>> if (a2[i] - a3 < a2[i-1]) {
>
> the variable names are unacceptable, a1 and a2 are not even the same type
>
>
>> float tmp =(a2[i-1] - a2[i] + a3)*.5;
>> a2[i-1] -= tmp;
>> a2[i] += tmp;
>> }
>> }
>>
>
>> static void check_lsp_sort(int a1, float *a2)
>> {
>> int i,j;
>>
>> for (i=0; i < a1; ++i)
>> for (j=0; j < a1; j++)
>> if (a2[j-1] > a2[j])
>> FFSWAP(float, a2[j],a2[j-1]);
>> }
>
> same, also doing a O(n^2) by default sort is unacceptable
> are the values mostly sorted already?
yes
> [...]
>> static void dec_bark_env(const uint8_t *in, int use_hist, int ch, float *out,
>> enum FrameType ftype, TwinContext *tctx)
>> {
>> const ModeTab *mtab = tctx->mtab;
>> int i,j;
>> float *hist = tctx->bark_hist[ftype][ch];
>> float val = ((const float []) {0.4, 0.35, 0.28})[ftype];
>> int bark_n_coef = mtab->fmode[ftype].bark_n_coef;
>> int fw_cb_len = mtab->fmode[ftype].bark_env_size / bark_n_coef;
>> int pos = 0;
>>
>> for (i = 0; i < fw_cb_len; i++)
>> for (j = 0; j < bark_n_coef; j++) {
>> int idx = j + i*bark_n_coef;
>> float tmp2 = mtab->fmode[ftype].bark_cb[fw_cb_len*in[j] + i];
>> float st = (use_hist) ?
>> (1. - val) * tmp2 + val*hist[idx] + 1. : tmp2 + 1.;
>>
>> hist[idx] = tmp2;
>> if (st < -1.) st = 1.;
>>
>> while (pos < (mtab->fmode[ftype].bark_tab)[idx])
>> out[pos++] = st;
>
> a memset_float() could come in handy, this one isnt the first case i
> see that would benefit from it
Here or in a shared file?
All comments I didn't answer were fixed.
-Vitor
-------------- next part --------------
A non-text attachment was scrubbed...
Name: twinvq.c
Type: text/x-csrc
Size: 34772 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090608/a9334fb2/attachment.c>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: twinvq_data.h
Type: text/x-chdr
Size: 12740 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090608/a9334fb2/attachment.h>
More information about the ffmpeg-devel
mailing list