[FFmpeg-devel] [PATCH] WMA Voice postfilter

[Ronald S. Bultje]

Hi,

On Sun, Apr 11, 2010 at 7:48 PM, Vitor Sessak <vitor1001 at gmail.com> wrote:
> Ronald S. Bultje wrote:
>> + ? ?if (optimal_gain <= dot) {
>> + ? ? ? ?dot = 0.5 / (0.5 + 0.3 * optimal_gain / dot); // 0.0625-1.0000
>
> This can be done with one less division:
>
> dot = (0.5 * dot) / (0.5 * dot + 0.3 * optimal_gain); // 0.0625-1.0000

Fixed as suggested by Michael,

>> +/**
>> + * Derive denoise filter coefficients from the LPCs.
>> + */
>> +static void calc_coeffs(WMAVoiceContext *s, float *lpcs,
>> + ? ? ? ? ? ? ? ? ? ? ? ?int fcb_type, float *coeffs, int remainder)
>
> I think this would be better named "calc_input_response()"

Fixed.

>> + ? ?/* Create frequency power spectrum of speech input (i.e. RDFT of
>> LPCs);
>> + ? ? * we shift each value to an offset +1 so we don't have to create
>> temp
>> + ? ? * values. */
>> + ? ?ff_rdft_calc(&s->rdft, lpcs);
>> +#define log_range(x, assign) do { \
>> + ? ? ? ?float tmp = log10f(assign); ?lpcs[x] = tmp; \
>> + ? ? ? ?max ? ? ? = FFMAX(max, tmp); min ? ? = FFMIN(min, tmp); \
>> + ? ?} while (0)
>> + ? ?for (n = 1; n < 0x40; n++)
>> + ? ? ? ?log_range(n + 1, lpcs[n * 2] ? ? * lpcs[n * 2] +
>> + ? ? ? ? ? ? ? ? ? ? ? ? lpcs[n * 2 + 1] * lpcs[n * 2 + 1]);
>> + ? ?log_range(0x41, ? ? ?lpcs[1] ? ? ? ? * lpcs[1]);
>> + ? ?log_range(1, ? ? ? ? lpcs[0] ? ? ? ? * lpcs[0]);
>> +#undef log_range
>
> I think that readability could be improved using a single temp value (float
> last_coef = lpcs[1]) and avoiding the shifts.

Done, same in another place further down.

>> + ? ? ? ?idx = (pow * gain_mul - 0.0295) * 70.570526123;
>> + ? ? ? ?if (idx > 0x7F) {
>> + ? ? ? ? ? ?coeffs[n] = wmavoice_energy_table[127] *
>> + ? ? ? ? ? ? ? ? ? ? ? ?powf(1.0331663, idx - 127);
>> + ? ? ? ?} else
>> + ? ? ? ? ? ?coeffs[n] = wmavoice_energy_table[FFMAX(0, idx)];
>
> I think a comment saying that 70.570526123 == 1./log10(1.0331663) would be
> useful (and the if() is just a failback for some value that didn't fit in
> the table).

Fixed.

>> + ? ?/* calculate the Hilbert transform of the gains, which we do (since
>> this
>> + ? ? * is a sinus input) by doing a phase shift (in theory,
>> H(sin())=cos()). */
>> + ? ?ff_dct_calc(&s->dct, lpcs);
>> + ? ?ff_dct_calc(&s->dst, lpcs);
>
> I'd say the point of doing the Hilbert transform (that should be said in
> some comment) is that
>
> Hilbert_Transform(RDFT(F)) == Laplace_Transform(F)
>
> which is what you need to do a Wiener filter (unless I'm missing something).
>
>> + ? ?/* Project values as coefficients - note how this shifts them back
>> + ? ? * from offset=1 to offset=0. */
>
> More importantly, before you calculated the norm of the coefficient indexes
> (which discards all phase data). Now, here, you are putting back ?a
> (different) phase...

I changed the comments here, I hope they're OK now. As said before,
this is a little bit dark-gray magic to me, so my comments may
sometimes be a bit off, but I try to get it straight with what I read
in research papers on the subject as I go.

>> + ? ?sq = (1.0 / 64.0) * sqrtf(1 / ff_dot_productf(coeffs, coeffs,
>> remainder));
>> + ? ?for (n = 0; n < remainder; n++)
>> + ? ? ? ?coeffs[n] *= sq;
>> + ? ?ff_rdft_calc(&s->rdft, coeffs);
>> +}
>
> This last RDFT does not really belong to this function. It really would make
> much more sense in the caller, since it will read like
>
> ff_rdft_calc(&s->rdft, synth_pf);
> ff_rdft_calc(&s->rdft, coeffs);
>
> for(i = 0; i < N ; i++)
> ? ? [complex multiplication synth_pf[i] *= coeffs[i]];
>
> ff_rdft_calc(&s->irdft, synth_pf);
>
> Which makes it clear that we are convolving synth_pf with coeffs.

Done.

>> + ? ? ? ?double i_lsps[MAX_LSPS];
>
> i_?

Interpolated. There's already a "lsps" variable here, hence this name.

New patch attached.

Ronald
-------------- next part --------------
A non-text attachment was scrubbed...
Name: wmavoice-apf.patch
Type: application/octet-stream
Size: 38234 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100419/d99f575c/attachment.obj>