fix: imported FLAC, Ogg and Opus files keep their track and disc numbers (#6171)
release / govulncheck (push) Successful in 14s
release / web (push) Successful in 1m21s
release / go (push) Successful in 1m29s
release / integration (push) Successful in 5m1s
release / android (push) Successful in 5m5s
release / Build signed APK (releases and dev) (push) Successful in 5m21s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 16s
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 14s
release / web (push) Successful in 1m21s
release / go (push) Successful in 1m29s
release / integration (push) Successful in 5m1s
release / android (push) Successful in 5m5s
release / Build signed APK (releases and dev) (push) Successful in 5m21s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 16s
release / Verify release artifacts (tag releases only) (push) Skipped
The importer wrote track=N/M for every format, which ffmpeg stores in a Vorbis comment as TRACKNUMBER=3/20; dhowden/tag reads a Vorbis TRACKNUMBER with a bare Atoi, so it came back 0. All 245 tracks imported since 2026-10-10 (Humanz, Plastic Beach, the Gorillaz super deluxe and twelve more albums) have no track or disc number, and their album pages have no order. - The importer writes a Vorbis track and disc number alone, with TRACKTOTAL and DISCTOTAL, as Picard does. ID3 and MP4 keep N/M. - ReadTrackAndDisc takes the number before a slash when the Vorbis reader returns 0, for the scanner and for the import's own check of a download. - tagReadVersion 6 re-reads the files already held (tag reads only). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -133,8 +133,7 @@ func readFile(ctx context.Context, root, path string) File {
|
|||||||
return f
|
return f
|
||||||
}
|
}
|
||||||
f.Title, f.Artist, f.AlbumArtist, f.Album = m.Title(), m.Artist(), m.AlbumArtist(), m.Album()
|
f.Title, f.Artist, f.AlbumArtist, f.Album = m.Title(), m.Artist(), m.AlbumArtist(), m.Album()
|
||||||
f.Track, _ = m.Track()
|
f.Track, f.Disc = library.ReadTrackAndDisc(m)
|
||||||
f.Disc, _ = m.Disc()
|
|
||||||
ids := library.ReadTagMBIDs(m)
|
ids := library.ReadTagMBIDs(m)
|
||||||
f.GroupMBID, f.ReleaseMBID, f.RecordingMBID = ids.ReleaseGroup, ids.Release, ids.Recording
|
f.GroupMBID, f.ReleaseMBID, f.RecordingMBID = ids.ReleaseGroup, ids.Release, ids.Recording
|
||||||
return f
|
return f
|
||||||
|
|||||||
@@ -38,16 +38,32 @@ type Tags struct {
|
|||||||
// ID3 takes Picard's TXXX descriptions, which ffmpeg writes for any key it
|
// ID3 takes Picard's TXXX descriptions, which ffmpeg writes for any key it
|
||||||
// has no frame for. MP4's freeform atoms ffmpeg cannot write, so an M4A
|
// has no frame for. MP4's freeform atoms ffmpeg cannot write, so an M4A
|
||||||
// gets the plain names only.
|
// gets the plain names only.
|
||||||
|
//
|
||||||
|
// A Vorbis track or disc number is the number alone, its count in
|
||||||
|
// TRACKTOTAL and DISCTOTAL, as Picard writes them: dhowden/tag reads
|
||||||
|
// TRACKNUMBER "3/20" as 0, and every album imported before #6171 lost its
|
||||||
|
// order that way. ID3 and MP4 take "3/20", which their readers parse.
|
||||||
func (t Tags) args(ext string) []string {
|
func (t Tags) args(ext string) []string {
|
||||||
|
vorbis := ext == ".flac" || ext == ".ogg" || ext == ".opus"
|
||||||
|
track, disc := ofN(t.Track, t.Tracks), ofN(t.Disc, t.Discs)
|
||||||
|
if vorbis {
|
||||||
|
track, disc = ofN(t.Track, 0), ofN(t.Disc, 0)
|
||||||
|
}
|
||||||
kv := [][2]string{
|
kv := [][2]string{
|
||||||
{"title", t.Title}, {"artist", t.Artist}, {"album", t.Album}, {"album_artist", t.AlbumArtist},
|
{"title", t.Title}, {"artist", t.Artist}, {"album", t.Album}, {"album_artist", t.AlbumArtist},
|
||||||
{"track", ofN(t.Track, t.Tracks)}, {"disc", ofN(t.Disc, t.Discs)},
|
{"track", track}, {"disc", disc},
|
||||||
}
|
}
|
||||||
if t.Year > 0 {
|
if t.Year > 0 {
|
||||||
kv = append(kv, [2]string{"date", strconv.Itoa(t.Year)})
|
kv = append(kv, [2]string{"date", strconv.Itoa(t.Year)})
|
||||||
}
|
}
|
||||||
switch ext {
|
switch ext {
|
||||||
case ".flac", ".ogg", ".opus":
|
case ".flac", ".ogg", ".opus":
|
||||||
|
if t.Track > 0 && t.Tracks > 0 {
|
||||||
|
kv = append(kv, [2]string{"TRACKTOTAL", strconv.Itoa(t.Tracks)})
|
||||||
|
}
|
||||||
|
if t.Disc > 0 && t.Discs > 0 {
|
||||||
|
kv = append(kv, [2]string{"DISCTOTAL", strconv.Itoa(t.Discs)})
|
||||||
|
}
|
||||||
kv = append(kv,
|
kv = append(kv,
|
||||||
[2]string{"MUSICBRAINZ_ALBUMID", t.ReleaseMBID},
|
[2]string{"MUSICBRAINZ_ALBUMID", t.ReleaseMBID},
|
||||||
[2]string{"MUSICBRAINZ_RELEASEGROUPID", t.GroupMBID},
|
[2]string{"MUSICBRAINZ_RELEASEGROUPID", t.GroupMBID},
|
||||||
|
|||||||
@@ -212,11 +212,21 @@ func TestTagsArgs(t *testing.T) {
|
|||||||
tg := Tags{Title: "Get Lucky", Artist: "Daft Punk", Track: 8, Tracks: 13, Disc: 1, Discs: 1, Year: 2013,
|
tg := Tags{Title: "Get Lucky", Artist: "Daft Punk", Track: 8, Tracks: 13, Disc: 1, Discs: 1, Year: 2013,
|
||||||
RecordingMBID: "rec-08", GroupMBID: "rg-ram"}
|
RecordingMBID: "rec-08", GroupMBID: "rg-ram"}
|
||||||
flac := strings.Join(tg.args(".flac"), " ")
|
flac := strings.Join(tg.args(".flac"), " ")
|
||||||
for _, s := range []string{"title=Get Lucky", "track=8/13", "date=2013", "MUSICBRAINZ_TRACKID=rec-08", "MUSICBRAINZ_RELEASEGROUPID=rg-ram"} {
|
for _, s := range []string{
|
||||||
|
"title=Get Lucky", "track=8 ", "TRACKTOTAL=13", "disc=1 ", "DISCTOTAL=1", "date=2013",
|
||||||
|
"MUSICBRAINZ_TRACKID=rec-08", "MUSICBRAINZ_RELEASEGROUPID=rg-ram",
|
||||||
|
} {
|
||||||
if !strings.Contains(flac, s) {
|
if !strings.Contains(flac, s) {
|
||||||
t.Errorf("flac args lack %q: %s", s, flac)
|
t.Errorf("flac args lack %q: %s", s, flac)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// dhowden/tag reads a Vorbis TRACKNUMBER of "8/13" as 0 (#6171).
|
||||||
|
if strings.Contains(flac, "/13") {
|
||||||
|
t.Errorf("flac track number carries its count: %s", flac)
|
||||||
|
}
|
||||||
|
if mp3 := strings.Join(tg.args(".mp3"), " "); !strings.Contains(mp3, "track=8/13") {
|
||||||
|
t.Errorf("mp3 track should be 8/13, which ID3 readers parse: %s", mp3)
|
||||||
|
}
|
||||||
if strings.Contains(flac, "MUSICBRAINZ_ALBUMID") {
|
if strings.Contains(flac, "MUSICBRAINZ_ALBUMID") {
|
||||||
t.Errorf("an empty release id was written: %s", flac)
|
t.Errorf("an empty release id was written: %s", flac)
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -242,3 +242,22 @@ func TestReadTagMBIDs_Recording(t *testing.T) {
|
|||||||
t.Errorf("release track only: recording = %q, want empty", got)
|
t.Errorf("release track only: recording = %q, want empty", got)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A Vorbis TRACKNUMBER of "3/20", as Minstrel's importer wrote it, is track 3
|
||||||
|
// (#6171): dhowden/tag reads it as 0.
|
||||||
|
func TestReadTrackAndDisc(t *testing.T) {
|
||||||
|
for _, tc := range []struct {
|
||||||
|
name string
|
||||||
|
m stubMeta
|
||||||
|
track, disc int
|
||||||
|
}{
|
||||||
|
{"vorbis with counts", stubMeta{format: tag.VORBIS, raw: map[string]interface{}{"tracknumber": "3/20", "discnumber": "2/2"}}, 3, 2},
|
||||||
|
{"vorbis bare", stubMeta{format: tag.VORBIS, raw: map[string]interface{}{"tracknumber": " 7 "}}, 7, 0},
|
||||||
|
{"vorbis junk", stubMeta{format: tag.VORBIS, raw: map[string]interface{}{"tracknumber": "A1", "discnumber": "-1"}}, 0, 0},
|
||||||
|
{"id3 is the library's own reading", stubMeta{format: tag.ID3v2_4, raw: map[string]interface{}{"tracknumber": "3/20"}}, 0, 0},
|
||||||
|
} {
|
||||||
|
if track, disc := ReadTrackAndDisc(tc.m); track != tc.track || disc != tc.disc {
|
||||||
|
t.Errorf("%s: got track %d disc %d, want %d and %d", tc.name, track, disc, tc.track, tc.disc)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -70,7 +70,11 @@ var audioExtensions = map[string]bool{
|
|||||||
// release-track name, so every Vorbis-tagged track came in without its
|
// release-track name, so every Vorbis-tagged track came in without its
|
||||||
// tag id and waited on an AcoustID lookup. A re-read stores the tag id,
|
// tag id and waited on an AcoustID lookup. A re-read stores the tag id,
|
||||||
// over a looked-up one (D4).
|
// over a looked-up one (D4).
|
||||||
const tagReadVersion int16 = 5
|
// 6: a Vorbis TRACKNUMBER or DISCNUMBER written as "3/20" is read (#6171).
|
||||||
|
// dhowden/tag reads it as 0, and Minstrel's own importer wrote it that
|
||||||
|
// way, so all 245 FLAC tracks it imported came in with no track or disc
|
||||||
|
// number and their albums lost their order.
|
||||||
|
const tagReadVersion int16 = 6
|
||||||
|
|
||||||
type Stats struct {
|
type Stats struct {
|
||||||
Scanned int `json:"scanned"`
|
Scanned int `json:"scanned"`
|
||||||
@@ -316,8 +320,7 @@ func (s *Scanner) scanFile(
|
|||||||
return pgtype.UUID{}, false, fmt.Errorf("album: %w", err)
|
return pgtype.UUID{}, false, fmt.Errorf("album: %w", err)
|
||||||
}
|
}
|
||||||
|
|
||||||
trackNum, _ := meta.Track()
|
trackNum, discNum := ReadTrackAndDisc(meta)
|
||||||
discNum, _ := meta.Disc()
|
|
||||||
|
|
||||||
// An unchanged file being re-read only to refresh tag-derived columns
|
// An unchanged file being re-read only to refresh tag-derived columns
|
||||||
// doesn't need another ffprobe: the stored duration is still accurate, and
|
// doesn't need another ffprobe: the stored duration is still accurate, and
|
||||||
|
|||||||
@@ -0,0 +1,40 @@
|
|||||||
|
package library
|
||||||
|
|
||||||
|
import (
|
||||||
|
"strconv"
|
||||||
|
"strings"
|
||||||
|
|
||||||
|
"github.com/dhowden/tag"
|
||||||
|
)
|
||||||
|
|
||||||
|
// ReadTrackAndDisc reads a file's track and disc numbers. dhowden/tag reads a
|
||||||
|
// Vorbis TRACKNUMBER or DISCNUMBER only as a bare number, but taggers also
|
||||||
|
// write "3/20" there (Minstrel's own importer did until #6171), and that
|
||||||
|
// reads as 0. For Vorbis the number before the slash is taken instead.
|
||||||
|
func ReadTrackAndDisc(m tag.Metadata) (track, disc int) {
|
||||||
|
track, _ = m.Track()
|
||||||
|
disc, _ = m.Disc()
|
||||||
|
if m.Format() != tag.VORBIS {
|
||||||
|
return track, disc
|
||||||
|
}
|
||||||
|
raw := m.Raw()
|
||||||
|
if track == 0 {
|
||||||
|
track = leadingNumber(raw["tracknumber"])
|
||||||
|
}
|
||||||
|
if disc == 0 {
|
||||||
|
disc = leadingNumber(raw["discnumber"])
|
||||||
|
}
|
||||||
|
return track, disc
|
||||||
|
}
|
||||||
|
|
||||||
|
// leadingNumber is the number in a tag value of "3" or "3/20"; 0 for
|
||||||
|
// anything else.
|
||||||
|
func leadingNumber(v any) int {
|
||||||
|
s, _ := v.(string)
|
||||||
|
before, _, _ := strings.Cut(s, "/")
|
||||||
|
n, err := strconv.Atoi(strings.TrimSpace(before))
|
||||||
|
if err != nil || n < 0 {
|
||||||
|
return 0
|
||||||
|
}
|
||||||
|
return n
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user