[FFmpeg-devel] [PATCH] Remove MULL macro from lsp.c

Vladimir Voroshilov voroshil
Mon Aug 25 17:27:18 CEST 2008


2008/8/25 Michael Niedermayer <michaelni at gmx.at>:
> On Sat, Aug 23, 2008 at 01:08:13AM +0700, Vladimir Voroshilov wrote:
>> Attached patch replaces MULL macro in lsp.c with multiplication and right shift.
>> Current code works wrong because one of macro's arguments is int16_t while asm
>> assumes both of them to be int.
>>
>> I'll also remove all occurences of MULL macro from my G.729 code.
>>
>> P.S. Another fix is leaving MULL there but explicitly cast the second
>> argument to (int).
>> I don't know which is better.
>
> I would say that the bug you are trying to fix is in MULL
> it should work when its arguments are not int.i

i hope, somebody fix it then.

>
> besides
>
> [...]
>>          f[i] = f[i-2];
>>          for(j=i; j>1; j--)
>> -            f[j] -= MULL(f[j-1], lsp[2*i-2]) - f[j-2]; // (3.22) * (0.15) * 2 -> (3.22)
>> +            f[j] -= (((int64_t)f[j-1] * lsp[2*i-2]) >> 14) - f[j-2]; // (3.22) * (0.15) * 2 -> (3.22)
>>
>
> please fix the comments

I didn't see any error in it, though. If it looks confusing on not
clear, i should prefer delete it.

> also please fix the comments in ff_acelp_high_pass_filter()
> they contradict the claim that cliping is needed. One of them has to
> be wrong.

hm... looks like i'm not able to clearly describe allowed range for hpf_f
Simple calculations shows unlimited domain for it.
Anyway those comments  are confusing even for me now :) thus i'd
prefer remove them too.

About clipping.
Input signal with repeating -16384,16383 values (sine signal with maximum
amplitude and sampling_rate/2 frequency) will cause int16_t overflow
in output even without rounding.

Should i add corresponding comment before clipping code ? Is above
phrase sufficient?

-- 
Regards,
Vladimir Voroshilov mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719




More information about the ffmpeg-devel mailing list