[FFmpeg-devel] [PATCH] AAC: reject multiple channel configurations

Alex Converse alex.converse
Thu Jul 9 01:41:24 CEST 2009


On Wed, Jul 8, 2009 at 12:55 AM, Alex Converse<alex.converse at gmail.com> wrote:
> On Tue, Jul 7, 2009 at 6:16 PM, Robert Swain<robert.swain at gmail.com> wrote:
>> Hey,
>>
>> 2009/7/7 Alex Converse <alex.converse at gmail.com>:
>>> Invalid second channel configurations are one of the biggest causes of
>>> crashes in the AAC deocder in my experience. See issue 1254 for an
>>> example. If they are even legal is dubious at best. From 14496-3:2005:
>>
>> [...]
>>
>>> Even if they are legal we aren't reliably able to handle them at the
>>> moment so they should be disabled.
>>
>> Agreed. Thank you for the extensive and thorough argument, it's much
>> appreciated. :)
>>
>>> The attached patch does not break conformance.
>>
>> Which issues does it fix? It would be good to have this in the commit
>> message if possible.
>>
>
> Issue 1254 (as mentioned above :) )
>
>>> You may be curious why the patch does not just error out completely.
>>> It is clear from the spec that the redundant PCE must be parsed.
>>> Erroring would break concatenation of compatible ADTS files that
>>> contain a PCE.
>>
>> Patch OK, except I think the string in the av_log() should rather be
>> "Not evaluating a further program_config_element as this construct is
>> dubious at best." It need not necessarily be the second and I think
>
> OK
>
>> the reason is tied to the action. And I guess you'll reindent
>> afterwards. :)
>>
>
> Of course
>
> However, I went to test one more thing before applying and realized
> this breaks ADTS files with PCEs. (MPEG never should have allowed PCEs
> to be sent in band it's such a disaster.)
>
> For ADTS files with PCEs:
>
> Parser inits
> AAC Decoder inits
> -> no extradata, avccontext->channels = 0 --> no output configuration
> AAC Decoder decodes first frame
> ?-> AAC Decoder finds PCE in the frame
> ? ? -> output configures
> AAC Decoder closes
> ?-> output unconfigures
> AAC Decoder inits
> ?-> avccontext->channels > 0 --> output configures
> AAC Decoder decodes first frame
> ?-> AAC Decoder finds PCE in the frame
> ? ?-> Danger Will Robinson!
>
> Options:
> A: Only set the configured flag when a PCE is found not when the
> output is first configured. This removes the PCE detection benefit
> from all ADTS files without PCE (99% of ADTS files in the wild)
> B: avccontext->channels = 0 on aac_decode_close. SIGFPE
> C: Create a phony extradata format for the parser to signal that this
> file is channel configuration 0. Holden Caulfield doesn't like phonies
> and neither do I.
> D: Something awesome that I haven't thought of yet.
>

Option D: (attached)

--Alex
-------------- next part --------------
A non-text attachment was scrubbed...
Name: aac-ignore-second-pce.diff
Type: text/x-patch
Size: 2993 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090708/8f62de0a/attachment.bin>



More information about the ffmpeg-devel mailing list