GNOME Bugzilla – Bug 612313
qtdemux: Post AAC profile/level in caps
Last modified: 2010-10-05 18:52:10 UTC
This patch looks at the ESDS atom for the AAC profile, and if present, export it as a tag.
Created attachment 155660 [details] [review] patch to expose AAC profile as a tag
Created attachment 155745 [details] [review] qtdemux: Post AAC profile in caps Post the profile in caps instead of a tag.
Created attachment 156565 [details] [review] Updated patch Updated patch with a pointer to bug #612312 for notes on how to use the profile string.
Created attachment 160002 [details] [review] qtdemux: Export AAC level in caps This exports the AAC level in caps for use as metadata and (eventually) for more fine-grained selection of decoders at caps-negotiation time.
Created attachment 160003 [details] [review] qtdemux: Export AAC profile in caps This exports the profile for AAC streams in caps, for use as metadata and (eventually) finer-grained decoder-selection.
Comment on attachment 156565 [details] [review] Updated patch The profile/level guessing code has been moved to pbutils in bug #617314, so we can avoid code duplication. Attached patches to use the utility functions.
Created attachment 162337 [details] [review] qtdemux: Export AAC profile and level in caps This exports the AAC profile and level in caps for use as metadata and (eventually) for more fine-grained selection of decoders at caps-negotiation time.
Updated (and simpler) patch to use setter functions. Since the changes are quite small, I've merged the two patches.
Ok, I've pushed this now: commit 845a3d6c3d1b013c2450f0d09a0c1e583e7ab4cf Author: Arun Raghavan <arun.raghavan@collabora.co.uk> Date: Fri Apr 30 14:06:27 2010 +0530 qtdemux: Export AAC profile and level in caps This exports the AAC profile and level in caps for use as metadata and (eventually) for more fine-grained selection of decoders at caps-negotiation time. (Doesn't work for HE-AAC yet though.) https://bugzilla.gnome.org/show_bug.cgi?id=612313 along with an unrelated fix for the channel override in this code section. Main question I have is this: why is the _set_level_and_profile() call within the if (codec_data_len == 2) block? Shouldn't it be outside that block, with the codec utils function handling this correctly? It seems to me that the caller of the codec utils functions shouldn't have to worry about the HE-AAC thing - if we have to worry about that, then maybe the codec utils API isn't quite right (not passed enough information or somesuch).
Created attachment 171679 [details] [review] qtdemux: AAC codec_data can be > 2 bytes long This fixes the assumption that DecoderSpecificInfo must be 2 bytes long for AAC files. The specification allows HE-AAC to be explicitly signalled in a backward compatible way. This is done by means of an additional information after the regular AAC header. It is expected that decoders that can play AAC but not HE-AAC will parse the header normally and ignore extended bits, much as they do for the HEAAC specific payload in the actual stream.
Tim: thanks for pushing all these patches! You're right - the codec utils should handle the HE-AAC bits when they get added. This may not always be possible (for implicitly signalled HE-AAC, you know that it is HE-AAC when you see the first SBR payload in the actual stream - no extra data is provided in headers). However, for the cases where it is explicitly signalled, we should be handle it in the codec utils function. I've attached a patch that removes the assumption about the header length and lets the codec utils function decide how to extract the profile/level.
I took the liberty to change your patch a little, I don't think it does what you meant to do (we want to keep the data_len == 2 check for the part where we override the stream parameters): commit 4a244e0d552f01d17b04de2cfcfb270ecd848521 Author: Arun Raghavan <arun.raghavan@collabora.co.uk> Date: Tue Oct 5 19:40:50 2010 +0100 qtdemux: AAC codec_data can be > 2 bytes long This fixes the assumption that DecoderSpecificInfo must be 2 bytes long for AAC files. The specification allows HE-AAC to be explicitly signalled in a backward compatible way. This is done by means of an additional information after the regular AAC header. It is expected that decoders that can play AAC but not HE-AAC will parse the header normally and ignore extended bits, much as they do for the HE-AAC specific payload in the actual stream. https://bugzilla.gnome.org/show_bug.cgi?id=612313 Now we just need to fix up the codec utils a little for HE-AAC. I'll file a new bug for that.