ID3v2: Fix double unsynchronisation of frames when the tag header flag is set#682
Open
attilagyorffy wants to merge 1 commit into
Open
Conversation
…g is set When an ID3v2.4 tag sets the header unsynchronisation flag (0x80) and its frames also carry the per-frame unsynchronisation flag (0x02), the tag-level stream and the frame reader each de-unsynchronised the frame content, stripping a `00` after every `0xFF` twice. Text frames survived, but binary APIC data came back corrupt. Skip the frame-level unsynchronisation pass when the tag-level stream is already active, so frame content is decoded exactly once. Fixes Serial-ATA#678 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
attilagyorffy
force-pushed
the
fix/id3v2-double-unsynchronisation
branch
from
July 21, 2026 12:50
7288926 to
bdfc357
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #678.
The bug
When an ID3v2.4 tag sets the header unsynchronisation flag (
0x80) and its frames also carry the per-frame unsynchronisation flag (0x02), lofty de-unsynchronises the frame content twice: once for the tag-level stream inparse_id3v2, and again for the frame flag in the frame reader. Each pass strips a00after every0xFF, so an encodedFF 00 00collapses toFF.Text frames mostly survive this, but
APICpictures are binary and real JPEGs are full ofFF 00pairs, so covers come back corrupt and no longer decode. This is the shape Bandcamp's MP3 tagger writes.The fix
Skip the frame-level unsynchronisation pass when the tag-level stream is already active, so frame content is decoded exactly once. The tag header's unsynchronisation flag is threaded down to the frame reader; the existing compression/encryption/plain arms then read the already-decoded bytes. Nonconforming writers that unsynchronise the whole tag without setting per-frame flags keep working, since that path is unchanged.
Test
Added
unsynchronised_apic_is_decoded_once, which builds the exact tag shape from #678 and checks theAPICbytes survive the round-trip. It fails onmain(FF 00 59decodes toFF 59) and passes with the fix. The rest of the suite is unaffected.Thanks to @zealsprince for the clear report and reproducer.