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

[Michael Niedermayer]

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


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Many that live deserve death. And some that die deserve life. Can you give
it to them? Then do not be too eager to deal out death in judgement. For
even the very wise cannot see all ends. -- Gandalf
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20080826/448c0900/attachment.pgp>