n10v / id3v2

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

does id3v2 work or doesn't id3v2 (commandline tool) nor iTunes get it right? #18

Closed mro closed 7 years ago

mro commented 7 years ago

I'm confused since I found my mp3-tagger https://github.com/mro/internet-radio-recorder/tree/master/src/enclosure-tag-cmd actually doesn't write tags readable by iTunes nor http://id3v2.sourceforge.net.

I made a test over at https://github.com/mro/id3v2/commit/40e73bb7bf5c7a42d384becc8ba70a8d0e4da120 and would be super happy to hear what you think about it.

Cheers and happy 🐇

n10v commented 7 years ago

Thank for a bug report! Actually I'm on my holidays (Easter in Germany) and I can't be with computer till monday (I'm writing from my iPhone). But I am testing every my commit as I use it by myself and it works for me. But are your tests passed? If not, please send me an output.

mro commented 7 years ago

No hurries.

Surprising is, that tmp.mp3 created by TestSetFull shows no trace of changed tags at all.

$ git log -1 --oneline
a5193f9 Make benchmarks more honest
$ time sh run.sh 

go version go1.8 darwin/amd64
GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/mro/gopath~"
GORACE=""
GOROOT="/usr/local/Cellar/go/1.8/libexec"
GOTOOLDIR="/usr/local/Cellar/go/1.8/libexec/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/j6/nrkx806n4pvg336kmncfncdh0000gn/T/go-build172985780=/tmp/go-build -gno-record-gcc-switches -fno-common"
CXX="clang++"
CGO_ENABLED="1"
PKG_CONFIG="pkg-config"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
github.com/stretchr/testify (download)
github.com/bogem/id3v2/bwpool
github.com/bogem/id3v2/util
github.com/bogem/id3v2/rdpool
github.com/bogem/id3v2
=== RUN   TestParseHeader
--- PASS: TestParseHeader (0.00s)
=== RUN   TestWriteTagHeader
--- PASS: TestWriteTagHeader (0.00s)
=== RUN   TestUnmodified
--- PASS: TestUnmodified (0.13s)
=== RUN   TestSetArtist
--- PASS: TestSetArtist (0.24s)
=== RUN   TestSetFull
--- PASS: TestSetFull (0.23s)
=== RUN   TestParseInvalidFrameSize
--- PASS: TestParseInvalidFrameSize (0.00s)
=== RUN   TestParse
--- PASS: TestParse (0.11s)
=== RUN   TestSequenceCacheUpdate
--- PASS: TestSequenceCacheUpdate (0.00s)
=== RUN   TestSequenceCommentFramesUniqueness
--- PASS: TestSequenceCommentFramesUniqueness (0.00s)
=== RUN   TestSequencePictureFramesUniqueness
--- PASS: TestSequencePictureFramesUniqueness (0.00s)
=== RUN   TestSequenceUSLFsUniqueness
--- PASS: TestSequenceUSLFsUniqueness (0.00s)
=== RUN   TestCountLenSize
--- PASS: TestCountLenSize (0.00s)
=== RUN   TestIntegrityOfMusicAtTheBeginning
--- PASS: TestIntegrityOfMusicAtTheBeginning (0.00s)
=== RUN   TestIntegrityOfMusicAtTheEnd
--- PASS: TestIntegrityOfMusicAtTheEnd (0.06s)
=== RUN   TestCheckPermissions
--- PASS: TestCheckPermissions (0.05s)
=== RUN   TestBlankID
--- PASS: TestBlankID (0.07s)
PASS
ok      github.com/bogem/id3v2  0.960s
?       github.com/bogem/id3v2/bwpool   [no test files]
?       github.com/bogem/id3v2/rdpool   [no test files]
=== RUN   TestReadTillDelim
--- PASS: TestReadTillDelim (0.00s)
=== RUN   TestReadTillZero
--- PASS: TestReadTillZero (0.00s)
=== RUN   TestNext
--- PASS: TestNext (0.00s)
=== RUN   TestReadTillDelimEOF
--- PASS: TestReadTillDelimEOF (0.00s)
=== RUN   TestReadTillDelims
--- PASS: TestReadTillDelims (0.00s)
=== RUN   TestParseSize
--- PASS: TestParseSize (0.00s)
=== RUN   TestFormSize
--- PASS: TestFormSize (0.00s)
PASS
ok      github.com/bogem/id3v2/util 0.006s
code: 0
id3v1 tag info for /Users/mro/gopath~/src/github.com/bogem/id3v2/testdata/tmp.mp3:
Title  : love letters drenched in gasol  Artist: suicide, part 2               
Album  :                                 Year: 2015, Genre: Unknown (255)
Comment:                                 Track: 0
/Users/mro/gopath~/src/github.com/bogem/id3v2/testdata/tmp.mp3: No ID3v2 tag

real    0m10.945s
user    0m2.606s
sys 0m1.047s
n10v commented 7 years ago

Thank you for waiting and happy easter! Actually I can't reproduce the problem. I've executed run.sh and iTunes can see the tag correctly: 2017-04-17 20 33 13 2017-04-17 20 34 31

id3v2 cli tool can't read id3v2.4 tags (id3v2 lib sets v2.4 tag by default), that's why it finds only id3v1 tag on the end of file (I don't know how it appeared there, because id3v2 lib can't physically write id3v1 tags). So if you add tag.SetVersion(3) in your code/tests right after opening the tag, it will work:

$ id3v2 -l /Users/albert/gopath~/src/github.com/bogem/id3v2/testdata/tmp.mp3
id3v1 tag info for /Users/albert/gopath~/src/github.com/bogem/id3v2/testdata/tmp.mp3:
Title  : love letters drenched in gasol  Artist: suicide, part 2
Album  :                                 Year: 2015, Genre: Unknown (255)
Comment:                                 Track: 0
id3v2 tag info for /Users/albert/gopath~/src/github.com/bogem/id3v2/testdata/tmp.mp3:
TCON (Content type): Radio (255)
WPUB (Official publisher webpage): https://soundcloud.com/suicidepart2
TIT2 (Title/songname/content description): 8 NACH 8 - DAS ENDE DER NACHT. Morgenmagazin mit Robert Schromm live aus dem Au�enstudio Bad Reichenhall.
TPE1 (Lead performer(s)/Soloist(s)): http://radiofabrik.at
TALB (Album/Movie/Show title): Album
TDRC ():  frame
COMM (Comments): (Short description)[eng]:
n10v commented 7 years ago

I deleted the id3v1 tag from testdata/test.mp3. Update id3v2 package and run tests one more time. The output will be more understandable.

mro commented 7 years ago

wow, just tested f8f96eee26ed and works like a charm! 🐳

mro commented 7 years ago

hngrr, I was too fast confirming the fix. I think https://github.com/mro/id3v2/commit/eeebfe5df3fb6c4f7d11ea0f8e4c17e13f90b538 shows some issues when using a bigger mp3 (see inside run.sh, line 5):

Am curious what you think.

n10v commented 7 years ago

I confirm, what artwork can't be read by any tag reader (including id3v2 lib), although it's successfully written in file. I don't know why it happens, but I will figure out soon. But I can't understand 1. and 3. points. COMM frame works as expect. And I also don't see any TENC in any mp3.

mro commented 7 years ago

oh, yes, sorry, you're right - the testcase provided doesn't leak COMM into lyrics but lyrics get lost altogether. Nor does it show the sticky TENC. So I'll have to re-investigate and provide a sample in case. May take a few days.

So the missing artwork and lyrics remain for now.

n10v commented 7 years ago

At first, I see, what you use "de" as language. You should choose a three-letters language code from ISO 639-2 code list ("ger" for german language). I should write it clearer in documentation

n10v commented 7 years ago

Temporary solution:

  1. Use three-letters language code as said in previous comment.
  2. Set tag version to 4 (tag.SetVersion(4))

iTunes will see the attached picture. Why it doesn't work with version 3, I will figure out later. And the size of mp3 or tag doesn't really matter. id3v2 library can work with any size. (Except if tag size is more 256MB. As said in id3v2 spec, the max possible size of id3v2 tag can be 256MB)

n10v commented 7 years ago

id3v2.3 doesn't support the UTF-8 encoding, that's why iTunes doesn't understand UTF-8 encoding symbol and doesn't read some frames. But how does it read other text frames, like artist, title and etc, although they also use UTF-8 encoding symbol?

mro commented 7 years ago

maybe the meaning of bytes isn't defined by id3 at all – so it may be either ascii or utf-8 or anything else as long as writer and consumer agree. But once the lib calculates lengths, it may be fooled by 2, 3 or even more bytes per utf-8 glyph. What may cause the leak I think to have seen once.

mro commented 7 years ago

http://id3.org/id3v2.4.0-changes?highlight=%28utf%29

mro commented 7 years ago

http://id3.org/iTunes?highlight=%28utf%29

n10v commented 7 years ago

But once the lib calculates lengths, it may be fooled by 2, 3 or even more bytes per utf-8 glyph.

No, it's not the reason of this problem. id3v2 lib correctly calculates length as len(text) and it counts bytes in text.

n10v commented 7 years ago

The problem is, what there is no good encoding converter for Golang. The only option is github.com/djimenez/iconv-go, but it just C bindings to iconv library. I could integrate it to id3v2 library and automatically convert encodings by parsing and writing, but I don't want to make library complex. If user needs other encoding, he/she should convert encodings manually.

mro commented 7 years ago

ic, doing the encoding isn't necessarily task for this lib here, maybe just point the requirements out in either the docs or the names (FooBarUTF16LE)?

And having v4 at hand for UTF8 is fine for me.

mro commented 7 years ago

Thanks a lot for this nice lib, your help and your kindness.

n10v commented 7 years ago

maybe just point the requirements out in either the docs or the names (FooBarUTF16LE)?

Actually I have no idea, how I should handle this issue now. So I just decided to wait for issue from users, who really needs other encodings.

You're always welcome! Thank you for your bug reports! I'm always glad to help with problems of id3v2 :)