Skip to content

Add support for demuxing xhe-aac files #435

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add support for demuxing xhe-aac files #435

wants to merge 1 commit into from

Conversation

padenot
Copy link
Contributor

@padenot padenot commented Jul 8, 2025

Necessary to fix BMO#1711882

@padenot padenot requested a review from kinetiknz July 8, 2025 17:30
Copy link
Collaborator

@kinetiknz kinetiknz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

// Only set codec type if it hasn't been set to a more specific type (e.g., XHEAAC)
if esds.audio_codec == CodecType::Unknown {
esds.audio_codec = match object_profile {
0x40 | 0x66 | 0x67 => CodecType::AAC,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be a tiny bit clearer to use:
0x40 if esds.audio_object_type == Some(42) => CodecType::XHEAAC,
to keep the initialization of audio_codec in a single location, but it doesn't matter too much.

@@ -5118,7 +5119,7 @@ fn read_ds_descriptor(
};

match audio_object_type {
1..=4 | 6 | 7 | 17 | 19..=23 => {
1..=4 | 6 | 7 | 17 | 19..=23 | 42 => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to work for now (similar to Chromium's AAC parsing) but it looks like xHE-AAC uses UsacConfig rather than GASpecificConfig (ffmpeg handles them separately), so it'd be worth adding a comment highlighting that audio_object_type == 42 could need different handling in the future to avoid tripping ourselves up if the GASpecificConfig parsing is ever extended or made stricter or we need specific fields from the UsacConfig.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants