Files
minstrel/internal/library/vorbisgenre_test.go
T
bvandeusenandClaude Opus 5.5 37d3a5bcd3
release / govulncheck (push) Successful in 50s
release / web (push) Successful in 2m3s
release / go (push) Successful in 2m18s
release / integration (push) Successful in 5m42s
release / android (push) Successful in 6m30s
release / Build signed APK (releases and dev) (push) Successful in 6m0s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 1m20s
release / Verify release artifacts (tag releases only) (push) Skipped
fix(library): read every genre of FLAC, Ogg, Opus and MP4 files (#2500)
Vorbis comments repeat a field to give it several values (GENRE=Boom Bap,
GENRE=Downtempo, ...). dhowden/tag keeps comments in a map keyed by field
name, so each repeat overwrote the previous one and only the last genre
was stored. MP4 has the same gap: several data atoms in one ©gen atom, or
repeated ©gen atoms, collapse to one value.

On the operator's library 3,658 of 4,092 FLACs declare more than one
genre. Kupla's Life Forms carries eight and was stored as "Instrumental
Hip Hop" alone, so browse and the taste profile never saw the other seven.

- vorbisgenre.go reads the comment block directly: FLAC's metadata block
  (including FLACs behind an ID3v2 tag), and the comment packet of Ogg
  Vorbis and Opus, reassembled across pages when cover art makes it span
  several.
- mp4genre.go walks moov > udta > meta > ilst and returns every ©gen text
  value. It handles ISO and QuickTime meta layouts and a moov placed after
  mdat. A file with only the numeric gnre atom still falls back to
  dhowden, which resolves it.
- extractGenres routes VORBIS and MP4 through them, as #2499 did for
  ID3v2.
- tagReadVersion 3 -> 4, so the next scan re-reads the tags of files
  already indexed. Unchanged files keep their duration and fingerprint,
  so the pass costs tag reads only.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-07 23:40:56 -04:00

289 lines
9.2 KiB
Go

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)
}
}