[FFmpeg-devel] [PATCH] ac3_parser type punning fix
Sun Oct 19 12:26:29 CEST 2008
Michael Niedermayer <michaelni at gmx.at> writes:
> On Sat, Oct 18, 2008 at 10:44:16PM -0700, David DeHaven wrote:
>> > Nothing major, I just applied the same fix that was applied to
>> > aac_parser.c recently...
>> > -DrD-
>> > <ac3_parser.patch>
>> I think there was confusion about the whole point of this patch...
>> On the four x86 based machine/OS combos I built and tested on (i686-pc-
>> mingw32, 2x i686-pc-linux, i686-apple-darwin), gcc with optimizations
>> at more than -O0 would produce machine code that would *only* byteswap
>> 32 of the 64 bits in the "state" variable passed to both aac_sync and
>> ac3_sync leading to complete and utter misinterpretation of the header
>> information. The result: at the *least* would be that it reported the
>> wrong information to stdout, at worst it would crash due to having the
>> wrong codec parameters. I can't honestly believe that noone else has
>> seen this problem, especially considering aac_sync has been already
>> fixed unless that was simply a "compiler warning" fix.
>> I can provide disassembly snippets if you need further convincing...
> I have no doubt that gcc generates wrong code i also have no doubt that
> the code breaks strict aliassing rules.
> The doubt i have is the relation, so to convince me first tell us if the
> code works with all identical but -fno-strict-aliasing added. If not
> the bug is elsewhere, and this patch is hiding it.
The C code is the same as in aac_parser, and I confirmed it was broken
myself. IIRC, it is only gcc 4.3 that optimises it "incorrectly".
Scare quotes because the C code is what is incorrect, breaking strict
aliasing rules. You accepted the same patch for the aac parser
without questions, so why the hesitation now?
mans at mansr.com
More information about the ffmpeg-devel