[FFmpeg-devel] [PATCH] ac3_parser type punning fix

[Baptiste Coudurier]

On Oct 19, 2008, at 2:31 PM, Michael Niedermayer <michaelni at gmx.at>
wrote:
> On Sun, Oct 19, 2008 at 12:26:31PM -0700, Baptiste Coudurier wrote:
>> Hi guys,
>>
>> Michael Niedermayer wrote:
>>> On Sun, Oct 19, 2008 at 05:23:52PM +0100, M?ns Rullg?rd wrote:
>>>> Michael Niedermayer <michaelni at gmx.at> writes:
> [...]
>>>> Looking at the code, I see one significant difference between
>>>> aac_sync() and ac3_sync().  Whereas aac_sync() only calls inlined
>>>> get_bits() functions (meaning gcc can know exactly what accesses  
>>>> are
>>>> happening), ac3_sync() calls another function to do the parsing.   
>>>> In
>>>> light of this, I agree it's unlikely that gcc is able to exploit  
>>>> any
>>>> aliasing tricks.  The disassembly above also suggests that  
>>>> something
>>>> more sinister is afoot here.
>>>>
>>>> The change to aac_sync() fixed a real aliasing violation, and I
>>>> verified the assembler before and after the change.  This seems  
>>>> to be
>>>> a bit different.
>>>
>>
>> FYI, Mans and I talked about this issue some time ago, and then I  
>> filled
>> a roundup issue regarding this (raw aac playback is broken)
>
> i cant find that issue, do you maybe know its number or a search  
> term that
> works?

Yes, it is issue 527, my memory fails a bit, I only confirmed the issue.

>
>>
>> Even at r15647 raw aac is still borken because the parser is still  
>> broken.
>
> i dont know about the parser but raw aac is broken because
> the aac decoder does
>    if (avccontext->extradata_size <= 0 ||
>        decode_audio_specific_config(ac, avccontext->extradata,  
> avccontext->extradata_size))
>        return -1;
>
> and nothing in raw.c nor in the parser sets extradata, nor is there a
> AVCodecParser.split() that would cause extradata to be set.
> Thats also why aac in mpeg-ts does not work ...

Well It is broken since some time now ... before aac decoder, and
still broken with faad which supports both bitstream formats.

Btw, I guess these (aac, ac3) parsers should be updated when state64 
will be added, needed by the dnxhd parser which I will commit soon (like 
tomorrow).

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Smartjog USA Inc.                                http://www.smartjog.com
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA