n10v / id3v2

🎵 ID3 decoding and encoding library for Go
https://pkg.go.dev/github.com/bogem/id3v2/v2
MIT License
334 stars 50 forks source link

Improving approach for APIC frame conflict solving. #65

Closed tillt closed 2 years ago

tillt commented 2 years ago

A file containing multiple APIC frames will lose all but the last one when parsing.

iTunes appears to be perfectly capable of parsing files with multiple pieces of artwork (APIC) attached. When parsing such file with id3v2, we see only a single APIC coming out of the "AllFrames()" factory. When then saving we did effectively delete all those additional APIC frames.

The AddFrame implementation of sequence is rather confusing to me. It tries to find out if a key already exists within the sequence and if so, does not add the frame but replaces the existing frame in the sequence. What is the point of a sequence if we only ever add a single element?

Anyway, by patching AddFrame as follows, we get proper amounts of pictures out of the parser;

func (s *sequence) AddFrame(f Framer) {
    s.frames = append(s.frames, f)
}

Not sure what I am breaking here though....

tillt commented 2 years ago

After reading a bit more through the code, I am beginning to understand what I am breaking here. Not a good approach assuming that indeed the "description" is supposed to be unique and thus feasible for hashing a picture in id3v2.

Turns out I did also not observe iTunes' behaviour correctly. When fed with the file/s I am testing with, iTunes also reduces the file to a single image - iTunes uses the one advertising itself as a "COVER_FRONT" - thus ignoring the "Description" but using the "Picture type" for additional hashing. My test file has 5 "OTHER" and one "COVER_FRONT".

Example:

-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
Time: 00:01 MPEG1, Layer III    [ 320 kb/s @ 44100 Hz - Joint stereo ]
-------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
ID3 v2.3:
title: Tests
artist: STRANGE Things
album: TesTiNG GARBAGE FILES
album artist: TESTING GARBAGE FILES
composer: TESTING GARBAGE FILES
track:
Comment: [Description: ] [Lang: eng]
TESTING GARBAGE FILES
FRONT_COVER Image: [Size: 53596 bytes] [Type: image/jpeg]
Description:

OTHER Image: [Size: 44912 bytes] [Type: image/jpeg]
Description:

OTHER Image: [Size: 56502 bytes] [Type: image/jpeg]
Description:

OTHER Image: [Size: 61351 bytes] [Type: image/jpeg]
Description:

OTHER Image: [Size: 30346 bytes] [Type: image/jpeg]
Description:

OTHER Image: [Size: 5588 bytes] [Type: image/jpeg]
Description:
tillt commented 2 years ago

Proposing https://github.com/bogem/id3v2/pull/66 - @bogem let me know if that was acceptable.

n10v commented 2 years ago

Fixed in #66. Thank you, @tillt!