[FFmpeg-devel] [PATCH] WMA Voice decoder

[Ronald S. Bultje]

Hi,

same as Reimar - all comments taken care of unless otherwise stated:

On Sat, Jan 30, 2010 at 8:06 PM, Vitor Sessak <vitor1001 at gmail.com> wrote:
> Ronald S. Bultje wrote:
>> + ? ?idx = get_bits(gb, 7);
>> + ? ?fcb_gain = ff_wmavoice_gain_codebook_fcb[idx] *
>> + ? ? ? ? ? ? ? expf(ff_dot_productf(s->gain_pred_err, gain_coeff, 6) -
>> + ? ? ? ? ? ? ? ? ? ?5.2409161640);
>> + ? ?acb_gain = ff_wmavoice_gain_codebook_acb[idx];
>> + ? ?pred_err = logf(av_clipf(ff_wmavoice_gain_codebook_fcb[idx], 0.05,
>> 5.0));
>
> You can avoid the logf() if you replace the table by its logarithm and
>
> ? ?fcb_gain = expf(ff_dot_productf(s->gain_pred_err, gain_coeff, 6) -
> ? ? ? ? ? ? ? ? ? ?5.2409161640 + ff_wmavoice_gain_codebook_fcb[idx]);
> ? ?pred_err = av_clipf(ff_wmavoice_gain_codebook_fcb[idx], log(0.05),
> log(5.0)));

Oddly, I had the feeling this change made the code slower.

I still implemented it because I just don't believe that it makes it
slower, but still...

>> + ? ? ? ? ? ?//FIXME don't call this with len=1
>> + ? ? ? ? ? ?ff_acelp_interpolatef(&excitation[n], &excitation[n - pitch],
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?ff_wmavoice_ipol1_coeffs, 17,
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?(idx >= 0) | (abs(idx) << 1), 9, 1);
>
> I think you can at least call this with len=s->pitch_diff_sh16 or something
> like that (since pitch_sh8 seems to change only after a certain fixed number
> of iterations). Also it will not stay in the way for SIMD optimizations.

Uhm, I forgot this fixme. I'm gonna work on this tomorrow. I'm
re-attaching the patch for further review but this one is TODO and
will be fixed in the next iteration (I hope).

> Also, please also look at the doxygen warnings, there are quite a few that
> are really relevant.

Hm, ok, fixed that also.

Ronald