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

Alex Converse alex.converse
Wed Jul 8 06:55:47 CEST 2009


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.

Regards,
Alex Converse



More information about the ffmpeg-devel mailing list