Skip to content

fix(projection): detect MP3 and AAC frames without an ID3 tag as audio - #297

Merged
donghaoren merged 2 commits into
apple:mainfrom
Manishram-ai:fix-mp3-aac-detection
Nov 2, 2026
Merged

donghaoren merged 2 commits into
apple:mainfrom
Manishram-ai:fix-mp3-aac-detection

Conversation

@Manishram-ai

Copy link
Copy Markdown
Contributor

_detect_mime_type only treats bytes as MP3 when they start with an ID3 tag or exactly FF FB, which is MPEG-1 Layer III without CRC. Many MP3s without an ID3 tag start differently: 16 to 24 kHz files (FF F3), 8 to 12 kHz files (FF E3) and files with CRC (FF FA). AAC in ADTS starts with FF F1. All of these fall through to the "image" default, so automatic modality detection treats audio columns as images.

This uses the same check as the frontend's detectAudioMimeType in packages/utils/src/media.ts: the 11-bit sync word, then the layer bits to tell AAC from MPEG audio. JPEG is still checked first, so FF D8 isn't affected.

Testing:

  • 5 new cases in tests/test_modality_detection.py fail on main and pass with the fix (54 passed).
  • I made a 22.05 kHz MP3 and an AAC file with ffmpeg. Both were detected as "image" before and "audio" after.
  • ruff format --check and ruff check are clean.

@donghaoren

Copy link
Copy Markdown
Collaborator

Thanks! Could you add a few more non-audio tests like this:

b"\xff\xd8\xff\xe0" + b"\x00" * 20,  # JPEG must win over the sync check
b"\xff\xff\xff" + b"\x00" * 20,  # 0xFF padding
b"\xff\xe1\x00" + b"\x00" * 20,  # not valid MPEG or ADTS
b"\xff",  # too short

These should all return "image".

… audio

0xFF padding and other binary data can start with the same sync bits as
an MPEG audio or ADTS frame. Also check the reserved version, layer,
bitrate and sample rate values, and the full 12-bit ADTS sync word.
@Manishram-ai

Copy link
Copy Markdown
Contributor Author

Thanks, added all four in f1083fc. Two of them (0xFF padding and FF E1 00) came back as audio, so the check now also looks at the rest of the frame header before calling it audio. The frontend check in media.ts has the same gap, and I'm happy to fix that in a separate PR.

@donghaoren
donghaoren enabled auto-merge (squash) November 2, 2026 18:48
@donghaoren
donghaoren merged commit ade2008 into apple:main Nov 2, 2026
8 checks passed
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