fix(mp3): livelock on misdetected frame headers; skip ID3v2 tag before decode - #24
fix(mp3): livelock on misdetected frame headers; skip ID3v2 tag before decode#240015 wants to merge 1 commit into
Conversation
…e decode MP3Decode() does not consume input when it rejects a frame header (ERR_MP3_INVALID_FRAMEHEADER), but decode_mp3() left read_ptr pointing at the misdetected sync word. MP3FindSyncWord() then kept returning the same position, so the function retried the identical bytes forever: the audio task busy-looped at 100% CPU, starved IDLE, and the task watchdog fired. Files with large embedded album art trigger this easily because JPEG data is full of 0xFF sequences that look like sync words. Fix by advancing read_ptr one byte past the rejected sync word, the same approach decode_mp3_io() already uses. Also position the stream after the ID3v2 tag in is_mp3()/is_mp3_io() (the tag size is already read there), so decoding starts at the first audio frame instead of trawling megabytes of cover art for sync words. This removes the "MP3 sync word not found, dropping N bytes" spam and makes playback start faster on tagged files.
|
This looks very similar to #24 , is this approach better than that one in terms of skipping ID3v2 tags? @Lee-Stone? |
It looks like we're running into a similar problem. Maybe you, the author, have a better solution? |
|
@Lee-Stone Well, I believe that this PR fixes a different bug than the tag-skip overlap suggests — the actual livelock in decode_mp3() (fp path never advancing read_ptr on ERR_MP3_INVALID_FRAMEHEADER) plus tag-skipping for both decode paths (fp and stream io). That's independent of #23, so I'd suggest merging this one first. |
|
@chmorgan Regarding the id3v2 tag issue, It seems to have already been perfectly revised by @Lee-Stone |
|
@0015 I've pushed an id3v2 fix to main, if this fix still applies please rebase and repush. |
3 participants
MP3Decode() does not consume input when it rejects a frame header (ERR_MP3_INVALID_FRAMEHEADER), but decode_mp3() left read_ptr pointing at the misdetected sync word. MP3FindSyncWord() then kept returning the same position, so the function retried the identical bytes forever: the audio task busy-looped at 100% CPU, starved IDLE, and the task watchdog fired. Files with large embedded album art trigger this easily because JPEG data is full of 0xFF sequences that look like sync words.
Fix by advancing read_ptr one byte past the rejected sync word, the same approach decode_mp3_io() already uses.
Also position the stream after the ID3v2 tag in is_mp3()/is_mp3_io() (the tag size is already read there), so decoding starts at the first audio frame instead of trawling megabytes of cover art for sync words. This removes the "MP3 sync word not found, dropping N bytes" spam and makes playback start faster on tagged files.