[FFmpeg-devel] r9017 breaks WMA decoding on Intel Macs

[Guillaume Poirier]

Hi,

Michael Niedermayer wrote:
> On Tue, May 29, 2007 at 03:13:16PM +0200, Guillaume Poirier wrote:
>> Le 29 mai 07 ? 11:58, Guillaume POIRIER a ?crit :
>>> On 5/29/07, Zuxy Meng <zuxy.meng at gmail.com> wrote:
>>>> 2007/5/29, Guillaume Poirier <gpoirier at mplayerhq.hu>:
>>>>> Le 28 mai 07 ? 01:26, Trent Piepho a ?crit :
>>>>>
>>>>>> On Mon, 28 May 2007, Guillaume POIRIER wrote:
>>>>>>> On 5/28/07, Trent Piepho <xyzzy at speakeasy.org> wrote:
>>>>>> I'm attaching a patch showing the right way to do.
>>>>> Ok, I get the idea. Inline asm is pretty much black magic to me.
>>>>>
>>>>> Your patch doesn't work, as it doesn't even assemble, but attached
>>>>> corrected patch does fix the problem.
>>>>>
>>>>> Zuxy, Michael, what do you think about it?
>>>>> Note that I did try to putting all asm blocks of fft_sse.c as "asm
>>>>> volatile" but that didn't fix the problem alone.
>>>> It looks good to me, but is the "asm" -> "asm volatile" part
>>>> absolutely necessary?
>>> Don't know. This sure can be tested. I'd like to point out that every
>>> ASM block in lavc is declared as "asm volatile", except in fft_sse.c.
>>> Why is that? Why would you want them to be left as non-volatile?
>>>
>>> Won't GCC try to outsmart us sooner or later and break your beautiful
>>> little ASM blocks? ;-)
>> Ok, I just tested it: adding "volatile" keyword doesn't help fixing  
>> the bug, so I removed it, hence attached patch.
> 
> id like to understand precissely what caused the problem and how before
> some patch is applied

I agree completely.

> 
> someonw should use gcc -S and gdb to look at what gets
> misscompiled/missassembled exactly

I'll try to see what's wrong. In the meantime, I'd like to point out
the message (which I had failed to see before) that I'm getting when
compiling vanilla sources:


cc -I../libswscale -I../libavcodec  -DHAVE_AV_CONFIG_H
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_ISOC9X_SOURCE -I.. -I..
-I../libavutil -Wdeclaration-after-statement -I. -I.. -I../libavutil
-O4 -march=pentium-m -mtune=pentium-m -pipe -ffast-math
-fomit-frame-pointer -mdynamic-no-pic -falign-loops=16 -DSYS_DARWIN
-DCONFIG_DARWIN -shared-libgcc -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE
-I/Users/guillaume/Prgm/x264 -I/usr/local/include
-I/opt/local/include/freetype2 -I/opt/local/include  -c -o
i386/fft_sse.o i386/fft_sse.c
In file included from ../libavcodec/dsputil.h:33,
                 from i386/fft_sse.c:21:
../libavcodec/avcodec.h:2501: warning: 'ImgReSampleContext' is deprecated
../libavcodec/avcodec.h:2507: warning: 'ImgReSampleContext' is deprecated
{standard input}:190:Missing operand value assumed absolute 0.
{standard input}:191:Missing operand value assumed absolute 0.
{standard input}:194:Missing operand value assumed absolute 0.
{standard input}:195:Missing operand value assumed absolute 0.
{standard input}:347:Missing operand value assumed absolute 0.
{standard input}:349:Missing operand value assumed absolute 0.
{standard input}:353:Missing operand value assumed absolute 0.
{standard input}:373:Missing operand value assumed absolute 0.


The patched version doesn't have these message:

cc -I../libswscale -I../libavcodec  -DHAVE_AV_CONFIG_H
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -D_ISOC9X_SOURCE -I.. -I..
-I../libavutil -Wdeclaration-after-statement -I. -I.. -I../libavutil
-O4 -march=pentium-m -mtune=pentium-m -pipe -ffast-math
-fomit-frame-pointer -mdynamic-no-pic -falign-loops=16 -DSYS_DARWIN
-DCONFIG_DARWIN -shared-libgcc -D_LARGEFILE_SOURCE
-D_FILE_OFFSET_BITS=64 -D_LARGEFILE64_SOURCE
-I/Users/guillaume/Prgm/x264 -I/usr/local/include
-I/opt/local/include/freetype2 -I/opt/local/include  -c -o
i386/fft_sse.o i386/fft_sse.c
In file included from ../libavcodec/dsputil.h:33,
                 from i386/fft_sse.c:21:
../libavcodec/avcodec.h:2501: warning: 'ImgReSampleContext' is deprecated
../libavcodec/avcodec.h:2507: warning: 'ImgReSampleContext' is deprecated


I'm attaching both version (the one with -vanilla postfix is the one
from trunk, that is getting misscompiled).

I'll have a look at it tonight, but in the meantime, if someone wants
to look at it....

Guillaume
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: fft_sse-vanilla.S
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070529/ebf93ef0/attachment.txt>
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: fftsse.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070529/ebf93ef0/attachment.asc>