fix: under the album's own name, matching track titles are enough to match it; a refusal says which track differed and how (M502 #5569)
release / govulncheck (push) Successful in 15s
release / web (push) Successful in 1m19s
release / go (push) Successful in 1m33s
release / android (push) Successful in 4m54s
release / integration (push) Successful in 4m59s
release / Build signed APK (releases and dev) (push) Successful in 5m6s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 14s
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 15s
release / web (push) Successful in 1m19s
release / go (push) Successful in 1m33s
release / android (push) Successful in 4m54s
release / integration (push) Successful in 4m59s
release / Build signed APK (releases and dev) (push) Successful in 5m6s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 14s
release / Verify release artifacts (tag releases only) (push) Skipped
The first run on the library refused singles whose one track has the
album's title ("0 of 1 agree") and near-complete albums (11 of 14), because
every track's length had to fit as well. Lengths now only decide between
groups whose names merely overlap. The reason for a refusal names the first
track that differed, with both titles and lengths, so what is left can be
judged. Migration 0087 clears the refusals so those albums are matched
again.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,2 @@
|
|||||||
|
-- Nothing to undo: the refused albums are matched again either way.
|
||||||
|
SELECT 1;
|
||||||
@@ -0,0 +1,9 @@
|
|||||||
|
-- The first matching (#5569) required every track's length to fit as well
|
||||||
|
-- as its title, and refused singles and near-complete albums the library
|
||||||
|
-- plainly holds. Matching now takes the titles alone under the album's own
|
||||||
|
-- name: the albums it refused are matched again.
|
||||||
|
DELETE FROM library_album_matches WHERE outcome = 'unmatched';
|
||||||
|
|
||||||
|
UPDATE acquisition_settings
|
||||||
|
SET library_listed_at = NULL, library_unlisted = 0
|
||||||
|
WHERE library_listing_started_at IS NOT NULL;
|
||||||
@@ -252,10 +252,22 @@ func TestTrackAgreement(t *testing.T) {
|
|||||||
{"the right title at the wrong length", []libraryTrack{{Number: 2, Title: "Song", LengthMs: 400000}}, 0},
|
{"the right title at the wrong length", []libraryTrack{{Number: 2, Title: "Song", LengthMs: 400000}}, 0},
|
||||||
{"a track counted once", []libraryTrack{{Title: "Song", LengthMs: 200000}, {Title: "Song", LengthMs: 200000}}, 1},
|
{"a track counted once", []libraryTrack{{Title: "Song", LengthMs: 200000}, {Title: "Song", LengthMs: 200000}}, 1},
|
||||||
} {
|
} {
|
||||||
if got := trackAgreement(c.tracks, edition); got != c.want {
|
if got, _ := trackAgreement(c.tracks, edition, false); got != c.want {
|
||||||
t.Errorf("%s: %d agree, want %d", c.name, got, c.want)
|
t.Errorf("%s: %d agree, want %d", c.name, got, c.want)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// Under the album's own name the titles are enough; the reason a track
|
||||||
|
// did not agree names both sides.
|
||||||
|
edit := []libraryTrack{{Number: 2, Title: "Song (Radio Edit)", LengthMs: 150000}}
|
||||||
|
if n, miss := trackAgreement(edit, edition, false); n != 0 || miss != `"Song (Radio Edit)" runs 2:30 here; MusicBrainz's track 2 is "Song", 3:20` {
|
||||||
|
t.Errorf("by length: %d, %q", n, miss)
|
||||||
|
}
|
||||||
|
if n, _ := trackAgreement(edit, edition, true); n != 1 {
|
||||||
|
t.Errorf("by title alone: %d agree", n)
|
||||||
|
}
|
||||||
|
if _, miss := trackAgreement([]libraryTrack{{Title: "Other", LengthMs: 61000}}, edition, true); miss != `no track of its edition is like "Other" (1:01)` {
|
||||||
|
t.Errorf("unplaced: %q", miss)
|
||||||
|
}
|
||||||
if agrees(3, 4) || !agrees(4, 5) || agrees(0, 0) {
|
if agrees(3, 4) || !agrees(4, 5) || agrees(0, 0) {
|
||||||
t.Error("agrees: 80% of the album's tracks, and an album has tracks")
|
t.Error("agrees: 80% of the album's tracks, and an album has tracks")
|
||||||
}
|
}
|
||||||
|
|||||||
+59
-29
@@ -101,10 +101,14 @@ func tracksToMatch(rows []dbq.ListAlbumTracksToMatchRow) []libraryTrack {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// trackAgreement counts the album's tracks an edition agrees with: the same
|
// trackAgreement counts the album's tracks an edition agrees with: the same
|
||||||
// recording, or a length that fits and a title that agrees. A track is
|
// recording, or a title that agrees at a length that fits. When the group is
|
||||||
// paired by its disc and number first, then by title, so an album missing
|
// called just what the album is called, the titles alone are enough: the
|
||||||
// some tracks still pairs the ones it has.
|
// lengths are there to tell apart albums whose names only overlap, and a
|
||||||
func trackAgreement(tracks []libraryTrack, edition []editionTrack) int {
|
// rip's single may be the edit MusicBrainz lists at another length. A track
|
||||||
|
// is paired by its disc and number first, then by title, so an album missing
|
||||||
|
// some tracks still pairs the ones it has. miss says how the first track
|
||||||
|
// that did not agree differs, for the reason a match was refused.
|
||||||
|
func trackAgreement(tracks []libraryTrack, edition []editionTrack, titlesEnough bool) (n int, miss string) {
|
||||||
byPos := make(map[[2]int]int, len(edition))
|
byPos := make(map[[2]int]int, len(edition))
|
||||||
for i, e := range edition {
|
for i, e := range edition {
|
||||||
byPos[[2]int{max(e.Medium, 1), e.Position}] = i
|
byPos[[2]int{max(e.Medium, 1), e.Position}] = i
|
||||||
@@ -113,27 +117,47 @@ func trackAgreement(tracks []libraryTrack, edition []editionTrack) int {
|
|||||||
if t.Recording != "" && strings.EqualFold(t.Recording, e.Recording) {
|
if t.Recording != "" && strings.EqualFold(t.Recording, e.Recording) {
|
||||||
return true
|
return true
|
||||||
}
|
}
|
||||||
return release.LengthFits(t.LengthMs, e.LengthMs) && release.TitleAgrees(t.Title, e.Title)
|
return release.TitleAgrees(t.Title, e.Title) && (titlesEnough || release.LengthFits(t.LengthMs, e.LengthMs))
|
||||||
}
|
}
|
||||||
used := make([]bool, len(edition))
|
used := make([]bool, len(edition))
|
||||||
n := 0
|
|
||||||
for _, t := range tracks {
|
for _, t := range tracks {
|
||||||
|
at, placed := -1, false
|
||||||
if t.Number > 0 {
|
if t.Number > 0 {
|
||||||
if i, ok := byPos[[2]int{max(t.Disc, 1), t.Number}]; ok && !used[i] && agree(t, edition[i]) {
|
if i, ok := byPos[[2]int{max(t.Disc, 1), t.Number}]; ok {
|
||||||
used[i] = true
|
at = i
|
||||||
n++
|
placed = !used[i] && agree(t, edition[i])
|
||||||
continue
|
if placed {
|
||||||
|
used[i] = true
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
for i, e := range edition {
|
for i, e := range edition {
|
||||||
if !used[i] && agree(t, e) {
|
if !placed && !used[i] && agree(t, e) {
|
||||||
used[i] = true
|
used[i], placed = true, true
|
||||||
n++
|
|
||||||
break
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
switch {
|
||||||
|
case placed:
|
||||||
|
n++
|
||||||
|
case miss != "":
|
||||||
|
case at >= 0:
|
||||||
|
e := edition[at]
|
||||||
|
miss = fmt.Sprintf("%q runs %s here; MusicBrainz's track %d is %q, %s",
|
||||||
|
t.Title, minutes(t.LengthMs), e.Position, e.Title, minutes(e.LengthMs))
|
||||||
|
default:
|
||||||
|
miss = fmt.Sprintf("no track of its edition is like %q (%s)", t.Title, minutes(t.LengthMs))
|
||||||
|
}
|
||||||
}
|
}
|
||||||
return n
|
return n, miss
|
||||||
|
}
|
||||||
|
|
||||||
|
// minutes writes a length as m:ss, or "no length".
|
||||||
|
func minutes(ms int) string {
|
||||||
|
if ms <= 0 {
|
||||||
|
return "no length"
|
||||||
|
}
|
||||||
|
sec := (ms + 500) / 1000
|
||||||
|
return fmt.Sprintf("%d:%02d", sec/60, sec%60)
|
||||||
}
|
}
|
||||||
|
|
||||||
// agrees: enough of the album's tracks agree for the edition to be it.
|
// agrees: enough of the album's tracks agree for the edition to be it.
|
||||||
@@ -173,14 +197,14 @@ func (s *Service) matchAlbum(ctx context.Context, a dbq.ListAlbumsToMatchRow, tr
|
|||||||
// discography first, then a search.
|
// discography first, then a search.
|
||||||
tried := map[string]bool{}
|
tried := map[string]bool{}
|
||||||
var best albumMatch
|
var best albumMatch
|
||||||
bestN, bestEdition, considered := -1, 0, 0
|
bestN, bestEdition, considered, bestMiss := -1, 0, 0, ""
|
||||||
check := func(cands []candidate) error {
|
check := func(cands []candidate) error {
|
||||||
for _, c := range cands {
|
for _, c := range cands {
|
||||||
if tried[c.Group] || len(tried) >= matchCandidates {
|
if tried[c.Group] || len(tried) >= matchCandidates {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
tried[c.Group] = true
|
tried[c.Group] = true
|
||||||
n, size, err := s.agreementWith(ctx, c.Group, tracks)
|
n, size, miss, err := s.agreementWith(ctx, c.Group, tracks, sameTitle(c.Title, a.Title))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
@@ -189,7 +213,7 @@ func (s *Service) matchAlbum(ctx context.Context, a dbq.ListAlbumsToMatchRow, tr
|
|||||||
// one; then the edition nearest the album's length.
|
// one; then the edition nearest the album's length.
|
||||||
st, bt := sameTitle(c.Title, a.Title), sameTitle(best.Title, a.Title)
|
st, bt := sameTitle(c.Title, a.Title), sameTitle(best.Title, a.Title)
|
||||||
if n > bestN || (n == bestN && ((st && !bt) || (st == bt && abs(size-len(tracks)) < abs(bestEdition-len(tracks))))) {
|
if n > bestN || (n == bestN && ((st && !bt) || (st == bt && abs(size-len(tracks)) < abs(bestEdition-len(tracks))))) {
|
||||||
best, bestN, bestEdition = albumMatch{candidate: c}, n, size
|
best, bestN, bestEdition, bestMiss = albumMatch{candidate: c}, n, size, miss
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return nil
|
return nil
|
||||||
@@ -217,9 +241,15 @@ func (s *Service) matchAlbum(ctx context.Context, a dbq.ListAlbumsToMatchRow, tr
|
|||||||
if considered == 0 {
|
if considered == 0 {
|
||||||
return albumMatch{Detail: fmt.Sprintf("MusicBrainz has no album called %q by %s.", a.Title, a.ArtistName)}, nil
|
return albumMatch{Detail: fmt.Sprintf("MusicBrainz has no album called %q by %s.", a.Title, a.ArtistName)}, nil
|
||||||
}
|
}
|
||||||
return albumMatch{Detail: fmt.Sprintf(
|
detail := fmt.Sprintf("MusicBrainz has %d %s called %q by %s, but none has its tracks: the closest, %q, agrees on %d of its %d",
|
||||||
"MusicBrainz has %d %s called %q by %s, but none has its tracks: the closest, %q, agrees on %d of its %d.",
|
considered, plural(considered, "album", "albums"), a.Title, a.ArtistName, best.Title, max(bestN, 0), len(tracks))
|
||||||
considered, plural(considered, "album", "albums"), a.Title, a.ArtistName, best.Title, max(bestN, 0), len(tracks))}, nil
|
switch {
|
||||||
|
case bestN < 0:
|
||||||
|
detail += " (MusicBrainz lists no edition of it)"
|
||||||
|
case bestMiss != "":
|
||||||
|
detail += ": " + bestMiss
|
||||||
|
}
|
||||||
|
return albumMatch{Detail: detail + "."}, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// groupCandidate reads a group the tags name. A group MusicBrainz does not
|
// groupCandidate reads a group the tags name. A group MusicBrainz does not
|
||||||
@@ -296,15 +326,15 @@ func (s *Service) searchCandidates(ctx context.Context, a dbq.ListAlbumsToMatchR
|
|||||||
}
|
}
|
||||||
|
|
||||||
// agreementWith is how many of the album's tracks the group's best edition
|
// agreementWith is how many of the album's tracks the group's best edition
|
||||||
// agrees with, and that edition's length. A group MusicBrainz no longer has
|
// agrees with, that edition's length, and how it differs. A group
|
||||||
// agrees with none.
|
// MusicBrainz no longer has, or lists no edition of, agrees with none (-1).
|
||||||
func (s *Service) agreementWith(ctx context.Context, group string, tracks []libraryTrack) (n, size int, err error) {
|
func (s *Service) agreementWith(ctx context.Context, group string, tracks []libraryTrack, titlesEnough bool) (n, size int, miss string, err error) {
|
||||||
eds, err := s.cat.Releases(ctx, group)
|
eds, err := s.cat.Releases(ctx, group)
|
||||||
switch {
|
switch {
|
||||||
case errors.Is(err, catalogue.ErrNotFound):
|
case errors.Is(err, catalogue.ErrNotFound):
|
||||||
return -1, 0, nil
|
return -1, 0, "", nil
|
||||||
case err != nil:
|
case err != nil:
|
||||||
return 0, 0, fmt.Errorf("%w: %v", errMusicBrainzAway, err)
|
return 0, 0, "", fmt.Errorf("%w: %v", errMusicBrainzAway, err)
|
||||||
}
|
}
|
||||||
n = -1
|
n = -1
|
||||||
for _, e := range eds.Editions {
|
for _, e := range eds.Editions {
|
||||||
@@ -313,11 +343,11 @@ func (s *Service) agreementWith(ctx context.Context, group string, tracks []libr
|
|||||||
edition = append(edition, editionTrack{Medium: int(t.MediumPosition), Position: int(t.Position),
|
edition = append(edition, editionTrack{Medium: int(t.MediumPosition), Position: int(t.Position),
|
||||||
Title: t.Title, LengthMs: int(t.LengthMs), Recording: t.RecordingMbid})
|
Title: t.Title, LengthMs: int(t.LengthMs), Recording: t.RecordingMbid})
|
||||||
}
|
}
|
||||||
if got := trackAgreement(tracks, edition); got > n || (got == n && abs(len(edition)-len(tracks)) < abs(size-len(tracks))) {
|
if got, m := trackAgreement(tracks, edition, titlesEnough); got > n || (got == n && abs(len(edition)-len(tracks)) < abs(size-len(tracks))) {
|
||||||
n, size = got, len(edition)
|
n, size, miss = got, len(edition), m
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
return n, size, nil
|
return n, size, miss, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
// creditedArtist is the artist a want is filed under: the library's own
|
// creditedArtist is the artist a want is filed under: the library's own
|
||||||
|
|||||||
Reference in New Issue
Block a user