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

Vladimir Voroshilov voroshil
Tue Aug 26 20:00:38 CEST 2008


2008/8/27 Michael Niedermayer <michaelni at gmx.at>:
> On Tue, Aug 26, 2008 at 10:05:12PM +0700, Vladimir Voroshilov wrote:
>> 2008/8/26 Michael Niedermayer <michaelni at gmx.at>:
>> > On Tue, Aug 26, 2008 at 11:39:53AM +0700, Vladimir Voroshilov wrote:
>> >> 2008/8/26 Michael Niedermayer <michaelni at gmx.at>:
>> >> > On Mon, Aug 25, 2008 at 10:27:18PM +0700, Vladimir Voroshilov wrote:
>> >> >> 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
>> >> >
>> >> > the comment says *2, there is nothing in the code that does *2
>> >>
>> >> code should be read as (2 * f * lsp) >> 15 which is equal to  f * lsp >> 14
>> >> "2*" removed due to optimization.
>> >
>> > when 2* is removed from the code it also has to be removed from the comment
>>
>> I'll remove entire comment.
>>
>> But i'm in doubt now.
>> What is better:
>
>> 1. add (int) cast to MULL arguments in i386/mathops.h (as pengvado
>> suggest in IRC after discussing bug together with mru and
>> Dark_Shikari)
>
> ok

Please look into attached patch and tell what chunks are ok.
I used (and tested) only MULL macro but others (due to used "l"
suffix) can have the same bug.
Unfortunately, i'm not know asm enough good to be sure.

-- 
Regards,
Vladimir Voroshilov mailto:voroshil at gmail.com
JID: voroshil at gmail.com, voroshil at jabber.ru
ICQ: 95587719
-------------- next part --------------
A non-text attachment was scrubbed...
Name: mathops_int16_case_fix.diff
Type: text/x-diff
Size: 933 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080827/8de3885b/attachment.diff>



More information about the ffmpeg-devel mailing list