n10v / id3v2

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

Default encoding issues #85

Open hut8 opened 12 months ago

hut8 commented 12 months ago

Let's say that I do this:

    tags.SetDefaultEncoding(id3v2.EncodingUTF16)
    tags.SetVersion(3)                           // id3v2.3.0 right?

I would expect that I'm using ID3v2.3.0, and that my default encoding is UTF-16 (with BOM). But it looks like tags.SetVersion(3) will overwrite my default choice:

https://github.com/n10v/id3v2/blob/e76f9ab76b5519c769d4239738467ce1db7b415f/v2/tag.go#L305-L315

https://github.com/n10v/id3v2/blob/e76f9ab76b5519c769d4239738467ce1db7b415f/v2/tag.go#L202-L208

So for now I'm just changing the order around. But I have another thought:

Why is the default for version 3 ISO-8859-1? It seems to me like it should be UTF16.

    if version == 4 {
        tag.SetDefaultEncoding(EncodingUTF8)
    } else {
        tag.SetDefaultEncoding(EncodingISO)
    }

I would like to advocate for replacing that choice with UTF-16 with a BOM. From the spec:

All Unicode strings use 16-bit unicode 2.0 (ISO/IEC 10646-1:1993, UCS-2). Unicode strings must begin with the Unicode BOM ($FF FE or $FE FF) to identify the byte order.

So it seems like both specs can handle that, and it can handle (almost) all characters. Using your library, my Garmin GPS displays mojibake because of this default when listening to Лигалайз or other Russian rappers.

Nitpick: UCS-2 is not UTF-16, but it should be close enough that nobody notices (maybe actually encode strings as UCS-2, or at least check that the length is exactly 2 bytes per character). Here's a really fun read on the subject: https://unascribed.com/b/2019-08-02-the-tragedy-of-ucs2.html