diff --git a/internal/library/genre.go b/internal/library/genre.go index 2c903f13..7981e026 100644 --- a/internal/library/genre.go +++ b/internal/library/genre.go @@ -32,11 +32,25 @@ func extractGenres(meta tag.Metadata, rs io.ReadSeeker) (genres []string, fellBa // No frame at all is the common case for untagged files, and // dhowden/tag will have nothing either — not worth flagging. fellBack = meta.Genre() != "" - default: - // Vorbis comments (FLAC/OGG/Opus) and MP4 atoms don't go through - // dhowden's welding path, so its value is already a faithful read of - // the primary genre. Multi-value handling for those containers is a - // separate, unproven concern — see #2500. + case tag.VORBIS, tag.MP4: + // dhowden/tag keeps only the last of a repeated field here (#2500): + // see vorbisgenre.go and mp4genre.go. + var values []string + var err error + switch { + case meta.Format() == tag.MP4: + values, err = readMP4GenreValues(rs) + case meta.FileType() == tag.OGG: + values, err = readOggGenreValues(rs) + default: + values, err = readFLACGenreValues(rs) + } + if err == nil && len(values) > 0 { + return normaliseGenres(values), false + } + // err == nil with no values: the file declares no genre of its own + // (or, for MP4, only the numeric gnre atom, which dhowden resolves). + fellBack = err != nil && meta.Genre() != "" } return normaliseGenres([]string{meta.Genre()}), fellBack } diff --git a/internal/library/mp4genre.go b/internal/library/mp4genre.go new file mode 100644 index 00000000..d2096e12 --- /dev/null +++ b/internal/library/mp4genre.go @@ -0,0 +1,168 @@ +package library + +import ( + "encoding/binary" + "errors" + "io" + "strings" +) + +// MP4 has the same gap as Vorbis comments (#2500). A tag writer stores several +// genres as several "data" atoms inside one ©gen atom (mutagen and Picard do), +// or occasionally as repeated ©gen atoms. dhowden/tag keeps atoms in a map keyed +// by name (mp4.go: m.data[name] = data), so one value survives either way. +// +// Path to the values: moov > udta > meta > ilst > ©gen > data. A file that +// carries only the numeric "gnre" atom has no ©gen; this reader returns +// (nil, nil) for it, and extractGenres falls back to dhowden/tag, which resolves +// gnre to a name. + +// maxMP4GenreAtomSize caps how much of one ©gen atom is buffered. Genre text is +// bytes; the cap only stops a corrupt size from making the scanner allocate. +const maxMP4GenreAtomSize = 1 << 20 + +var errMalformedAtom = errors.New("library: malformed MP4 atom") + +type mp4Atom struct { + kind string + body, limit int64 // body start and end offsets in the file +} + +// readMP4Atom reads the atom header at pos, which must lie within [pos, limit). +func readMP4Atom(rs io.ReadSeeker, pos, limit int64) (mp4Atom, error) { + if _, err := rs.Seek(pos, io.SeekStart); err != nil { + return mp4Atom{}, err + } + var hdr [8]byte + if _, err := io.ReadFull(rs, hdr[:]); err != nil { + return mp4Atom{}, errMalformedAtom + } + size := int64(binary.BigEndian.Uint32(hdr[0:4])) + hdrLen := int64(8) + switch size { + case 0: // runs to the end of the enclosing atom + size = limit - pos + case 1: // 64-bit size follows the type + var large [8]byte + if _, err := io.ReadFull(rs, large[:]); err != nil { + return mp4Atom{}, errMalformedAtom + } + size = int64(binary.BigEndian.Uint64(large[:])) + hdrLen = 16 + } + if size < hdrLen || size > limit-pos { + return mp4Atom{}, errMalformedAtom + } + return mp4Atom{kind: string(hdr[4:8]), body: pos + hdrLen, limit: pos + size}, nil +} + +// findMP4Child returns the first child of the given kind in [start, end). +func findMP4Child(rs io.ReadSeeker, start, end int64, kind string) (mp4Atom, bool, error) { + for pos := start; pos+8 <= end; { + a, err := readMP4Atom(rs, pos, end) + if err != nil { + return mp4Atom{}, false, err + } + if a.kind == kind { + return a, true, nil + } + pos = a.limit + } + return mp4Atom{}, false, nil +} + +// readMP4GenreValues returns every ©gen value, in file order. rs is seeked +// as needed, so it is safe to call after dhowden/tag has consumed the reader. +func readMP4GenreValues(rs io.ReadSeeker) ([]string, error) { + end, err := rs.Seek(0, io.SeekEnd) + if err != nil { + return nil, err + } + // moov may sit after mdat; walking headers and seeking past bodies finds + // it either way without reading the audio. + cur := mp4Atom{body: 0, limit: end} + for _, kind := range []string{"moov", "udta", "meta"} { + next, found, err := findMP4Child(rs, cur.body, cur.limit, kind) + if err != nil { + return nil, errNoGenreFrame + } + if !found { + return nil, nil // no iTunes metadata at all + } + cur = next + } + // meta is a "full box" with four bytes of version and flags before its + // children in the ISO layout. QuickTime-style files omit them, so the + // first child (hdlr) starts straight away. Look before skipping. + if _, err := rs.Seek(cur.body, io.SeekStart); err != nil { + return nil, errNoGenreFrame + } + var peek [8]byte + if _, err := io.ReadFull(rs, peek[:]); err != nil { + return nil, errNoGenreFrame + } + if string(peek[4:8]) != "hdlr" { + cur.body += 4 + } + ilst, found, err := findMP4Child(rs, cur.body, cur.limit, "ilst") + if err != nil { + return nil, errNoGenreFrame + } + if !found { + return nil, nil + } + + var out []string + for pos := ilst.body; pos+8 <= ilst.limit; { + a, err := readMP4Atom(rs, pos, ilst.limit) + if err != nil { + return nil, errNoGenreFrame + } + pos = a.limit + if a.kind != "\xa9gen" { + continue + } + n := a.limit - a.body + if n > maxMP4GenreAtomSize { + return nil, errNoGenreFrame + } + if _, err := rs.Seek(a.body, io.SeekStart); err != nil { + return nil, errNoGenreFrame + } + body := make([]byte, n) + if _, err := io.ReadFull(rs, body); err != nil { + return nil, errNoGenreFrame + } + values, ok := mp4DataValues(body) + if !ok { + return nil, errNoGenreFrame + } + out = append(out, values...) + } + return out, nil +} + +// mp4DataValues reads the "data" atoms in a ©gen body. Each is: size, "data", +// a 4-byte type indicator (1 = UTF-8 text), a 4-byte locale, then the value. +// Atoms of other types carry no text and are skipped. +func mp4DataValues(b []byte) ([]string, bool) { + var out []string + for len(b) >= 8 { + size := binary.BigEndian.Uint32(b[0:4]) + if size < 8 || uint64(size) > uint64(len(b)) { + return nil, false + } + atom := b[:size] + b = b[size:] + if string(atom[4:8]) != "data" || len(atom) < 16 { + continue + } + if binary.BigEndian.Uint32(atom[8:12])&0x00FFFFFF != 1 { + continue + } + if v := strings.TrimSpace(string(atom[16:])); v != "" { + out = append(out, v) + } + } + return out, true +} diff --git a/internal/library/scanner.go b/internal/library/scanner.go index e737aab5..4fcd73c9 100644 --- a/internal/library/scanner.go +++ b/internal/library/scanner.go @@ -60,7 +60,11 @@ var audioExtensions = map[string]bool{ // (#5241). Lidarr names albums by release group, not by the release id // albums.mbid holds, so re-acquisition and request completion need it for // the albums already in the library, not only for files added from now on. -const tagReadVersion int16 = 3 +// 4: every value of a repeated genre field is read for FLAC, Ogg Vorbis, Opus +// and MP4 (#2500). dhowden/tag kept only the last, and 3,658 of the +// operator's 4,092 FLACs declare more than one, so most of their genres +// were never stored. +const tagReadVersion int16 = 4 type Stats struct { Scanned int `json:"scanned"` diff --git a/internal/library/vorbisgenre.go b/internal/library/vorbisgenre.go new file mode 100644 index 00000000..fa3027ff --- /dev/null +++ b/internal/library/vorbisgenre.go @@ -0,0 +1,214 @@ +package library + +import ( + "bufio" + "bytes" + "encoding/binary" + "errors" + "io" + "strings" +) + +// Why this file exists: Vorbis comments (FLAC, Ogg Vorbis, Opus) say a field +// has several values by repeating it: +// +// GENRE=Boom Bap +// GENRE=Downtempo +// +// dhowden/tag stores comments in a map keyed by field name +// (vorbis.go: m.c[strings.ToLower(k)] = v), so each repeat overwrites the last +// and only the final value survives. On the operator's library that was 3,658 +// of 4,092 FLAC files: Kupla's Life Forms declares eight genres and was stored +// as "Instrumental Hip Hop" alone (#2500). Nothing looked corrupt; the other +// seven were simply never seen by browse or the taste profile. +// +// The comment block is read here directly, as the ID3v2 genre frame is in +// id3v2genre.go. Every other field still comes from dhowden/tag. + +// maxVorbisCommentSize caps a comment block or packet. Comments are kilobytes; +// an embedded METADATA_BLOCK_PICTURE can take them to a few megabytes. The cap +// only stops a corrupt length from making the scanner allocate wildly. +const maxVorbisCommentSize = 16 << 20 + +var errMalformedComment = errors.New("library: malformed Vorbis comment") + +// readFLACGenreValues returns every GENRE value in a FLAC file's comment block. +// (nil, nil) means the file is readable and declares no genre. rs is seeked to +// the start, so it is safe to call after dhowden/tag has consumed the reader. +func readFLACGenreValues(rs io.ReadSeeker) ([]string, error) { + if _, err := rs.Seek(0, io.SeekStart); err != nil { + return nil, err + } + var magic [4]byte + if _, err := io.ReadFull(rs, magic[:]); err != nil { + return nil, errNoGenreFrame + } + // Some taggers prepend an ID3v2 tag to a FLAC. The FLAC stream starts after it. + if string(magic[:3]) == "ID3" { + var rest [6]byte + if _, err := io.ReadFull(rs, rest[:]); err != nil { + return nil, errNoGenreFrame + } + size := syncsafeInt(rest[2:6]) + if size < 0 { + return nil, errNoGenreFrame + } + skip := int64(10 + size) + if rest[1]&0x10 != 0 { + skip += 10 // 2.4 footer + } + if _, err := rs.Seek(skip, io.SeekStart); err != nil { + return nil, errNoGenreFrame + } + if _, err := io.ReadFull(rs, magic[:]); err != nil { + return nil, errNoGenreFrame + } + } + if string(magic[:]) != "fLaC" { + return nil, errNoGenreFrame + } + + for { + var hdr [4]byte + if _, err := io.ReadFull(rs, hdr[:]); err != nil { + return nil, errNoGenreFrame + } + last := hdr[0]&0x80 != 0 + blockType := hdr[0] & 0x7F + size := int64(hdr[1])<<16 | int64(hdr[2])<<8 | int64(hdr[3]) + switch blockType { + case 4: // VORBIS_COMMENT + body := make([]byte, size) + if _, err := io.ReadFull(rs, body); err != nil { + return nil, errNoGenreFrame + } + return vorbisCommentGenres(body) + case 127: // invalid by spec; nothing after it can be trusted + return nil, errNoGenreFrame + } + if last { + return nil, nil // no comment block at all + } + if _, err := rs.Seek(size, io.SeekCurrent); err != nil { + return nil, errNoGenreFrame + } + } +} + +// readOggGenreValues returns every GENRE value in an Ogg Vorbis or Opus file. +// The comments are the stream's second packet, which can span several pages +// when it carries cover art, so pages are reassembled until it is complete. +// Only the first logical stream is read; pages of any other are skipped. +func readOggGenreValues(rs io.ReadSeeker) ([]string, error) { + if _, err := rs.Seek(0, io.SeekStart); err != nil { + return nil, err + } + br := bufio.NewReader(rs) + var ( + serial uint32 + haveFirst bool + packet []byte + packetIdx int + ) + for { + var hdr [27]byte + if _, err := io.ReadFull(br, hdr[:]); err != nil { + return nil, errNoGenreFrame + } + if string(hdr[0:4]) != "OggS" { + return nil, errNoGenreFrame + } + pageSerial := binary.LittleEndian.Uint32(hdr[14:18]) + segments := make([]byte, hdr[26]) + if _, err := io.ReadFull(br, segments); err != nil { + return nil, errNoGenreFrame + } + if !haveFirst { + serial, haveFirst = pageSerial, true + } + if pageSerial != serial { + var n int64 + for _, l := range segments { + n += int64(l) + } + if _, err := io.CopyN(io.Discard, br, n); err != nil { + return nil, errNoGenreFrame + } + continue + } + for _, l := range segments { + if len(packet)+int(l) > maxVorbisCommentSize { + return nil, errNoGenreFrame + } + seg := make([]byte, l) + if _, err := io.ReadFull(br, seg); err != nil { + return nil, errNoGenreFrame + } + packet = append(packet, seg...) + if l == 255 { + continue // the packet carries on in the next segment + } + if packetIdx == 1 { + return oggCommentGenres(packet) + } + packetIdx++ + packet = packet[:0] + } + } +} + +// oggCommentGenres strips the codec's comment-packet prefix. Ogg FLAC and Speex +// are rare enough to leave to dhowden/tag. +func oggCommentGenres(packet []byte) ([]string, error) { + switch { + case bytes.HasPrefix(packet, []byte("\x03vorbis")): + return vorbisCommentGenres(packet[7:]) + case bytes.HasPrefix(packet, []byte("OpusTags")): + return vorbisCommentGenres(packet[8:]) + } + return nil, errNoGenreFrame +} + +// vorbisCommentGenres reads a comment block (vendor string, then a count of +// length-prefixed "KEY=value" entries, little-endian lengths throughout) and +// returns the GENRE values in file order. Field names are case-insensitive by +// spec. Anything after the last entry, such as Vorbis's framing bit, is ignored. +func vorbisCommentGenres(b []byte) ([]string, error) { + next := func() ([]byte, bool) { + if len(b) < 4 { + return nil, false + } + n := binary.LittleEndian.Uint32(b) + if uint64(n) > uint64(len(b)-4) { + return nil, false + } + field := b[4 : 4+n] + b = b[4+n:] + return field, true + } + if _, ok := next(); !ok { // vendor string + return nil, errMalformedComment + } + if len(b) < 4 { + return nil, errMalformedComment + } + count := binary.LittleEndian.Uint32(b) + b = b[4:] + var out []string + // Each entry costs at least four bytes, so a corrupt count runs out of + // input long before it runs up a loop. + for i := uint32(0); i < count; i++ { + field, ok := next() + if !ok { + return nil, errMalformedComment + } + key, value, found := strings.Cut(string(field), "=") + if !found || !strings.EqualFold(key, "GENRE") { + continue + } + if value = strings.TrimSpace(value); value != "" { + out = append(out, value) + } + } + return out, nil +} diff --git a/internal/library/vorbisgenre_test.go b/internal/library/vorbisgenre_test.go new file mode 100644 index 00000000..1fe0f0be --- /dev/null +++ b/internal/library/vorbisgenre_test.go @@ -0,0 +1,288 @@ +package library + +import ( + "bytes" + "encoding/binary" + "strings" + "testing" + + "github.com/dhowden/tag" +) + +// vorbisComment builds a comment block: vendor, count, then each "KEY=value". +func vorbisComment(fields ...string) []byte { + var b bytes.Buffer + vendor := "minstrel-test" + _ = binary.Write(&b, binary.LittleEndian, uint32(len(vendor))) + b.WriteString(vendor) + _ = binary.Write(&b, binary.LittleEndian, uint32(len(fields))) + for _, f := range fields { + _ = binary.Write(&b, binary.LittleEndian, uint32(len(f))) + b.WriteString(f) + } + return b.Bytes() +} + +// buildFLAC assembles a STREAMINFO block and a comment block, optionally +// behind an ID3v2 tag the way some taggers write them. +func buildFLAC(withID3 bool, fields ...string) []byte { + var b bytes.Buffer + if withID3 { + pad := make([]byte, 20) + b.WriteString("ID3") + b.Write([]byte{4, 0, 0}) + b.Write(synchsafeBytes(len(pad))) + b.Write(pad) + } + b.WriteString("fLaC") + b.Write([]byte{0x00, 0, 0, 34}) // STREAMINFO, not last + b.Write(make([]byte, 34)) + vc := vorbisComment(fields...) + b.Write([]byte{0x84, byte(len(vc) >> 16), byte(len(vc) >> 8), byte(len(vc))}) // last, type 4 + b.Write(vc) + return b.Bytes() +} + +// oggCRC is the Ogg page checksum: CRC-32, polynomial 0x04c11db7, no +// reflection, zero initial value. dhowden/tag verifies it, so the end-to-end +// tests need real pages. +func oggCRC(p []byte) uint32 { + var crc uint32 + for _, c := range p { + crc ^= uint32(c) << 24 + for i := 0; i < 8; i++ { + if crc&0x80000000 != 0 { + crc = crc<<1 ^ 0x04c11db7 + } else { + crc <<= 1 + } + } + } + return crc +} + +// buildOgg lays packets out as Ogg pages for one logical stream, splitting a +// packet across pages when it needs more than 255 lacing values. +func buildOgg(serial uint32, packets ...[]byte) []byte { + var out bytes.Buffer + var seq uint32 + for _, p := range packets { + var lacing []byte + for n := len(p); ; n -= 255 { + if n >= 255 { + lacing = append(lacing, 255) + continue + } + lacing = append(lacing, byte(n)) + break + } + continued := false + for off := 0; len(lacing) > 0; { + segs := lacing + if len(segs) > 255 { + segs = segs[:255] + } + lacing = lacing[len(segs):] + n := 0 + for _, s := range segs { + n += int(s) + } + var page bytes.Buffer + page.WriteString("OggS") + page.WriteByte(0) + flags := byte(0) + if continued { + flags |= 0x01 + } + if seq == 0 { + flags |= 0x02 + } + page.WriteByte(flags) + page.Write(make([]byte, 8)) // granule position + _ = binary.Write(&page, binary.LittleEndian, serial) + _ = binary.Write(&page, binary.LittleEndian, seq) + page.Write(make([]byte, 4)) // checksum, filled below + page.WriteByte(byte(len(segs))) + page.Write(segs) + page.Write(p[off : off+n]) + raw := page.Bytes() + binary.LittleEndian.PutUint32(raw[22:26], oggCRC(raw)) + out.Write(raw) + off += n + seq++ + continued = true + } + } + return out.Bytes() +} + +func vorbisIdent() []byte { return append([]byte("\x01vorbis"), make([]byte, 23)...) } + +// mp4Box wraps children in an atom of the given kind. +func mp4Box(kind string, children ...[]byte) []byte { + body := bytes.Join(children, nil) + b := make([]byte, 8, 8+len(body)) + binary.BigEndian.PutUint32(b, uint32(8+len(body))) + copy(b[4:], kind) + return append(b, body...) +} + +func mp4Text(value string) []byte { + return mp4Box("data", []byte{0, 0, 0, 1, 0, 0, 0, 0}, []byte(value)) +} + +// buildMP4 places the ©gen atoms under moov/udta/meta/ilst. isoMeta adds the +// four version/flags bytes ISO files carry before meta's children. +func buildMP4(isoMeta bool, genAtoms ...[]byte) []byte { + ilst := mp4Box("ilst", append([][]byte{mp4Box("\xa9nam", mp4Text("A Song"))}, genAtoms...)...) + hdlr := mp4Box("hdlr", make([]byte, 25)) + var meta []byte + if isoMeta { + meta = mp4Box("meta", []byte{0, 0, 0, 0}, hdlr, ilst) + } else { + meta = mp4Box("meta", hdlr, ilst) + } + ftyp := mp4Box("ftyp", []byte("M4A \x00\x00\x00\x00M4A mp42isom")) + mdat := mp4Box("mdat", make([]byte, 64)) + // moov after mdat, as many encoders write it. + return bytes.Join([][]byte{ftyp, mdat, mp4Box("moov", mp4Box("udta", meta))}, nil) +} + +var lifeForms = []string{ + "Boom Bap", "Downtempo", "Hip Hop", "Instrumental", + "Lo-Fi", "Lo-Fi Hip Hop", "Chillwave", "Instrumental Hip Hop", +} + +func genreFields(values ...string) []string { + out := make([]string, len(values)) + for i, v := range values { + out[i] = "GENRE=" + v + } + return out +} + +// The #2500 regression, shaped like the operator's Kupla - Life Forms files: +// eight GENRE fields, of which dhowden/tag keeps the last. +func TestExtractGenres_FLACRepeatedFields(t *testing.T) { + data := buildFLAC(false, append([]string{"TITLE=Eons"}, genreFields(lifeForms...)...)...) + rs := bytes.NewReader(data) + meta, err := tag.ReadFrom(rs) + if err != nil { + t.Fatalf("tag.ReadFrom: %v", err) + } + // Confirm the upstream behaviour this fix exists for is still present. + if got := meta.Genre(); got != "Instrumental Hip Hop" { + t.Logf("note: dhowden/tag no longer keeps only the last GENRE (got %q)", got) + } + genres, fellBack := extractGenres(meta, rs) + if fellBack { + t.Error("fellBack = true, want false") + } + if !equalStrings(genres, lifeForms) { + t.Errorf("genres = %q, want %q", genres, lifeForms) + } +} + +func TestReadFLACGenreValues(t *testing.T) { + tests := []struct { + name string + data []byte + want []string + err bool + }{ + {"field names are case-insensitive", buildFLAC(false, "genre=Shoegaze", "Genre=Dream Pop"), []string{"Shoegaze", "Dream Pop"}, false}, + {"behind an ID3v2 tag", buildFLAC(true, "GENRE=Jazz", "GENRE=Bop"), []string{"Jazz", "Bop"}, false}, + {"no genre is not an error", buildFLAC(false, "TITLE=x"), nil, false}, + {"empty values dropped", buildFLAC(false, "GENRE= ", "GENRE=Rock"), []string{"Rock"}, false}, + {"a key that only starts with GENRE", buildFLAC(false, "GENRES=Rock", "GENRE=Jazz"), []string{"Jazz"}, false}, + {"not a FLAC", []byte("RIFF....WAVEfmt "), nil, true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := readFLACGenreValues(bytes.NewReader(tc.data)) + if (err != nil) != tc.err { + t.Fatalf("err = %v, want error: %v", err, tc.err) + } + if !equalStrings(got, tc.want) { + t.Errorf("got %q, want %q", got, tc.want) + } + }) + } +} + +func TestReadFLACGenreValues_TruncatedCommentIsAnError(t *testing.T) { + data := buildFLAC(false, "GENRE=Jazz", "GENRE=Bop") + if _, err := readFLACGenreValues(bytes.NewReader(data[:len(data)-2])); err == nil { + t.Error("err = nil for a cut-off comment block") + } +} + +func TestExtractGenres_OggVorbisRepeatedFields(t *testing.T) { + comment := append([]byte("\x03vorbis"), vorbisComment(genreFields("Shoegaze", "Dream Pop")...)...) + comment = append(comment, 1) // framing bit + rs := bytes.NewReader(buildOgg(7, vorbisIdent(), comment)) + meta, err := tag.ReadFrom(rs) + if err != nil { + t.Fatalf("tag.ReadFrom: %v", err) + } + genres, fellBack := extractGenres(meta, rs) + if fellBack || !equalStrings(genres, []string{"Shoegaze", "Dream Pop"}) { + t.Errorf("genres = %q (fellBack %v), want [Shoegaze Dream Pop]", genres, fellBack) + } +} + +func TestReadOggGenreValues_Opus(t *testing.T) { + head := append([]byte("OpusHead"), make([]byte, 11)...) + tags := append([]byte("OpusTags"), vorbisComment(genreFields("Ambient", "Drone")...)...) + got, err := readOggGenreValues(bytes.NewReader(buildOgg(3, head, tags))) + if err != nil || !equalStrings(got, []string{"Ambient", "Drone"}) { + t.Errorf("got %q, %v; want [Ambient Drone]", got, err) + } +} + +// Cover art makes the comment packet span pages. A reader that stopped at the +// first page boundary would read a truncated block. +func TestReadOggGenreValues_PacketSpansPages(t *testing.T) { + fields := append([]string{"METADATA_BLOCK_PICTURE=" + strings.Repeat("A", 70000)}, genreFields("Jazz", "Bop")...) + comment := append([]byte("\x03vorbis"), vorbisComment(fields...)...) + got, err := readOggGenreValues(bytes.NewReader(buildOgg(9, vorbisIdent(), comment))) + if err != nil || !equalStrings(got, []string{"Jazz", "Bop"}) { + t.Errorf("got %q, %v; want [Jazz Bop]", got, err) + } +} + +func TestReadMP4GenreValues(t *testing.T) { + tests := []struct { + name string + data []byte + want []string + }{ + {"several data atoms in one ©gen", buildMP4(true, mp4Box("\xa9gen", mp4Text("Hip Hop"), mp4Text("Lo-Fi"))), []string{"Hip Hop", "Lo-Fi"}}, + {"repeated ©gen atoms", buildMP4(true, mp4Box("\xa9gen", mp4Text("Hip Hop")), mp4Box("\xa9gen", mp4Text("Lo-Fi"))), []string{"Hip Hop", "Lo-Fi"}}, + {"QuickTime meta without version bytes", buildMP4(false, mp4Box("\xa9gen", mp4Text("Jazz"), mp4Text("Bop"))), []string{"Jazz", "Bop"}}, + {"no ©gen is not an error", buildMP4(true), nil}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got, err := readMP4GenreValues(bytes.NewReader(tc.data)) + if err != nil { + t.Fatalf("err = %v", err) + } + if !equalStrings(got, tc.want) { + t.Errorf("got %q, want %q", got, tc.want) + } + }) + } +} + +func TestExtractGenres_MP4MultiValue(t *testing.T) { + rs := bytes.NewReader(buildMP4(true, mp4Box("\xa9gen", mp4Text("Hip Hop"), mp4Text("Lo-Fi")))) + meta, err := tag.ReadFrom(rs) + if err != nil { + t.Fatalf("tag.ReadFrom: %v", err) + } + genres, fellBack := extractGenres(meta, rs) + if fellBack || !equalStrings(genres, []string{"Hip Hop", "Lo-Fi"}) { + t.Errorf("genres = %q (fellBack %v), want [Hip Hop Lo-Fi]", genres, fellBack) + } +}