Skip to content
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

Writing ID3v2.2 tag writes album image as APIC frame rather than PIC #151

Open
joshuamaciak opened this issue Oct 11, 2018 · 0 comments
Open

Comments

@joshuamaciak
Copy link

joshuamaciak commented Oct 11, 2018

Hi, I've stumbled upon what I think is a bug in mp3agic.

While testing my own (very buggy) album art puller, I noticed what appears to be an issue with saving an mp3 with id3v2.2 frames. To reproduce, simply open an mp3, set an id3v2.2 tag, and save the mp3. As so:

Mp3File mp3 = new Mp3File("notags.mp3"); 
ID3v22Tag tag = new ID3v22Tag();
byte[] albumArt = Files.readAllBytes(new File("image.png").toPath());
tag.setAlbumImage(albumArt, "image/png");
mp3.setId3v2Tag(tag);
mp3.save("id3v22.mp3");

I've used the notags.mp3 & image.png from the test resources so it is more easily reproducible.

Opening up the id3v22.mp3 in a hex editor shows the following

49443302 00000000 0E744150 4943

Translated:

ID3 version 2 (as expected) ... APIC

According to the ID3v2 spec:

Attached picture   "PIC"
Frame size         $xx xx xx
Text encoding      $xx
Image format       $xx xx xx
Picture type       $xx
Description        <textstring> $00 (00)
Picture data       <binary data>

The APIC frame did not exist until ID3v2.3, so trying to parse the frame as a true PIC frame fails as the frame layout is slightly different.
I was able to trace this back to AbstractID3v2Tag.java in the packFrames(...) method where it packs the frame as APIC regardless of the ID3v2 version.

int newOffset = packSpecifiedFrames(bytes, offset, null, "APIC");
newOffset = packSpecifiedFrames(bytes, newOffset, "APIC", null);
return newOffset;

The way I'd fix it is maybe overriding the one of the 'pack' methods in each ID3v2#Tag class. This way we could pack the corresponding frames in the appropriate way in each version. At a glance, it seems like a quite few of the frames changed between 2.2 & 2.3, so this is pretty significant.

BTW, thanks a lot for this library. I use it quite a lot for an android app I'm developing.

EDIT: after working through this I saw in the README that mp3agic doesn't support writing id3v2.2 tags. However, I feel this is still an issue as the new mp3 saves successfully & still reports v2.2 tags in the outputted mp3.

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

No branches or pull requests

1 participant