[FFmpeg-devel] PATCH Fix SSE core-dumps when decoding AAC on optimized ffmpeg with shared libraries compiled under Mingw (win32)

[Ramiro Polla]

Hi,

On Wed, Oct 22, 2008 at 4:55 PM, Art Clarke <aclarke at vlideshow.com> wrote:
> On Wed, Oct 22, 2008 at 11:33 AM, Ramiro Polla <ramiro.polla at gmail.com> wrote:
>> On Wed, Oct 22, 2008 at 4:27 PM, Art Clarke <aclarke at vlideshow.com> wrote:
>>> Hi folks,
>>>
>>> Attached is a patch that fixes a bug where ACC decoding fails with the
>>> new decoder.  To reproduce, build ffmpeg in SHARED library mode on
>>> Mingw with the latest runtime and win32 library, with all
>>> optimizations turned on, and then try to decode an aac file on a
>>> machine with the SSE2 instruction set available (how's that for
>>> esoteric).
>>>
>>> ffmpeg -i aacfile.aac -acodec pcm_mulaw -y out.wav
>>>
>>> The problem is that the GCC compiler on mingw doesn't correctly align
>>> uninitialized local variables in mingw.  This causes some of the SSE
>>> optimizations under the new AAC decoder to work on non-64-bit aligned
>>> local variables and badness occurs.
>>>
>>> The fix is to pass "-fno-common" to the GCC compiler under mingw if
>>> mmx2 is enabled.  This allows assembly that tries to align
>>> uninitialized local variables on 64 bit boundaries at the cost of not
>>> allowing you to define an uninitialized variable with extern linkage
>>> more than once (without specifying at all but one instances as extern
>>> -- something ffmpeg already enforces)*.
>>>
>>> Major major kudos to Gianluigi Tiesi for figuring this out with the
>>> GCC team.  He's been working on getting mplayer to compile on Mingw.
>>>
>>> See here for the details on the GCC bug and "-fno-common" workaround:
>>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=37216
>>
>> ./configure --extra-cflags="-fno-common"
>>
>> and you're done...
>>
>> Your patch will do more harm than good once gcc/binutils are fixed on MinGW.
>>
>> Still, thanks for Gianluigi Tiese for finding that out.
[...]
> Although I'm actually pretty sure the attached fix is safe

It probably is safe on the current GCCs. At the very worst it turns
off some optimization (no I don't know what -fno-common does). But if
the default is on, it means the GCC folks decided it was worth an
optimization to turn on by default. Turning it off unconditional of
GCC version doesn't seem like a good idea (besides does it exist in
gcc 2.95.3? it would break compilation otherwise).

> , I
> understand your concern.  Can I least convince you to accept the
> following patch to save folks the 3 days of pain we just went through?

That's up to the documentation maintainer to decide, but if we kept
track of every toolchain bug that is bad for FFmpeg, we'd have a very
long list... Besides how do we know when the bug is fixed to list with
a min/max not supported version?

I think it's best if we just boost the pagerank of this message =). Or
even better wait for confirmation on
https://roundup.mplayerhq.hu/roundup/ffmpeg/issue688 and rename it
(and then close it as invalid).

Ramiro Polla