search

Serial-ATA / lofty-rs

Audio metadata library
Apache License 2.0
179 stars 34 forks source link

id3v2: Do not replace non-empty frames with duplicate, empty frames #351

Closed uklotzde closed 4 months ago

uklotzde commented 4 months ago

I have found an MP3 file with 2 comment frames. The first frame contains the actual value and the second frame is empty.

Quod Libet and mediainfo only show the first comment, while exiftool and lofty pick the second, empty comment. Many applications like Mixxx also only read the first non-empty frame and ignore any subsequent, empty frames. Even though TagLib provides access to all of them.

This fix tries to avoid loss of information and behaves like most other applications would handle those conflicts.

Serial-ATA commented 4 months ago

So the tag has 2 comments with the same language and description but one of them is empty? That's strange. Do you know where these files are coming from? Just to document why such a hack is necessary.

uklotzde commented 4 months ago

So the tag has 2 comments with the same language and description but one of them is empty? That's strange. Do you know where these files are coming from? Just to document why such a hack is necessary.

No idea how this tag was created. But duplicate frames could always occur, even if they are not permitted. The on-disk-format allows this and we have to handle it somehow.

Currently, only the last frame was preserved, regardless of any preceding frames. But many applications will probably only read the first frame and ignore the rest. Picking the first non-empty frame is probably a good strategy.

Serial-ATA commented 4 months ago

Makes sense, thanks!

uklotzde commented 4 months ago

MusicBrainz Picard seems to add duplicate and empty comment frames to MP3 files. Maybe not always and only under certain cirumstances.

Serial-ATA commented 4 months ago

Is that a known issue? For an app as big as Picard something like this should be looked into.

uklotzde commented 4 months ago

I didn't check the details, but this issue might be related: https://tickets.metabrainz.org/browse/PICARD-2468

The files that were affected were rejected by Lofty with BadFrameLength. Removing all the tags added by MusicBrainz Picard a long time ago fixed that issue. But trying to re-add them again in MusicBrainz Picard (current version) then generated those duplicate comment tags (~4 empty + 2 with the actual comment).

Serial-ATA commented 4 months ago

Does your file have a comment with language="XXX"? Just tried out that issue reproducer and Picard cannot handle those comments correctly at all.

uklotzde commented 4 months ago

Does your file have a comment with language="XXX"? Just tried out that issue reproducer and Picard cannot handle those comments correctly at all.

I didn't check. After fixing those files (using Quod Libet) that Lofty rejected I try to avoid MB Picard.

Serial-ATA commented 4 months ago

I went ahead and fixed that issue in Picard and now they're thinking of ways to better handle comments overall. Hopefully this issue will be a thing of the past.