fix: AcoustID decides any one-album mismatch the lengths cannot, not only copies each cut to its slot (#6169)
release / govulncheck (push) Successful in 13s
release / web (push) Successful in 1m21s
release / go (push) Successful in 1m37s
release / integration (push) Successful in 5m3s
release / android (push) Successful in 5m6s
release / Build signed APK (releases and dev) (push) Successful in 5m14s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 15s
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 13s
release / web (push) Successful in 1m21s
release / go (push) Successful in 1m37s
release / integration (push) Successful in 5m3s
release / android (push) Successful in 5m6s
release / Build signed APK (releases and dev) (push) Successful in 5m14s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 15s
release / Verify release artifacts (tag releases only) (push) Skipped
The first pass on the operator's library held 16 groups. In several, every copy's own lookup names one recording (Wombats Record Shop/Tormentor, Alchemist (2)/Prelude, Monday Chill 8/9, 404sessions), but one copy sits 3-10s from its slot, so the every-copy-fits precondition kept AcoustID out. Now: copies whose lookups all agree are that recording, whatever their fingerprints read; copies of the same audio are asked about as before; and the track list places the recording in one copy's slot, or none. Copies filed as one song are not settled here. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -114,9 +114,9 @@ func TestWrongSlots(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// One recording cut to each of several slots' lengths (Savant's Mortals): the
|
// The lengths cannot place a recording filed as several songs (Savant's
|
||||||
// lengths cannot place it, and what AcoustID calls it does (#6169).
|
// Mortals, cut to each slot's length); what AcoustID calls it can (#6169).
|
||||||
func TestTrimmedCopies(t *testing.T) {
|
func TestNamedCopies(t *testing.T) {
|
||||||
track := func(title string, ms int32, rec string) dbq.MbReleaseTrack {
|
track := func(title string, ms int32, rec string) dbq.MbReleaseTrack {
|
||||||
return dbq.MbReleaseTrack{Title: title, LengthMs: ms, RecordingMbid: rec}
|
return dbq.MbReleaseTrack{Title: title, LengthMs: ms, RecordingMbid: rec}
|
||||||
}
|
}
|
||||||
@@ -127,34 +127,27 @@ func TestTrimmedCopies(t *testing.T) {
|
|||||||
album := []dbq.MbReleaseTrack{
|
album := []dbq.MbReleaseTrack{
|
||||||
track("Tormentor", 221000, "rec-tormentor"), track("Record Shop", 180000, "rec-record-shop"), track("Ice Cream", 200000, "rec-ice"),
|
track("Tormentor", 221000, "rec-tormentor"), track("Record Shop", 180000, "rec-record-shop"), track("Ice Cream", 200000, "rec-ice"),
|
||||||
}
|
}
|
||||||
if !trimmedToSlots(copies, album) {
|
|
||||||
t.Fatal("each copy fits its own slot: trimmedToSlots should hold the group")
|
|
||||||
}
|
|
||||||
if trimmedToSlots(copies[:1], album) {
|
|
||||||
t.Error("one title is not several songs' slots")
|
|
||||||
}
|
|
||||||
if trimmedToSlots([]slotCopy{copies[0], {Title: "Record Shop", Artist: "The Wombats", DurationMs: 220000}}, album) {
|
|
||||||
t.Error("a copy that does not fit its slot is wrongSlots' to decide")
|
|
||||||
}
|
|
||||||
cases := []struct {
|
cases := []struct {
|
||||||
name string
|
name string
|
||||||
|
copies []slotCopy
|
||||||
tracks []dbq.MbReleaseTrack
|
tracks []dbq.MbReleaseTrack
|
||||||
recordings []string
|
recordings []string
|
||||||
keep int
|
keep int
|
||||||
losers []int
|
losers []int
|
||||||
ok bool
|
ok bool
|
||||||
}{
|
}{
|
||||||
{"AcoustID names Tormentor", album, []string{"rec-tormentor"}, 0, []int{1}, true},
|
{"AcoustID names Tormentor", copies, album, []string{"rec-tormentor"}, 0, []int{1}, true},
|
||||||
{"one of several candidates is on the album", album, []string{"rec-elsewhere", "rec-record-shop"}, 1, []int{0}, true},
|
{"one of several candidates is on the album", copies, album, []string{"rec-elsewhere", "rec-record-shop"}, 1, []int{0}, true},
|
||||||
{"AcoustID knows none", album, nil, -1, []int{0, 1}, true},
|
{"AcoustID knows none", copies, album, nil, -1, []int{0, 1}, true},
|
||||||
{"the recording is another song on the album", album, []string{"rec-ice"}, -1, []int{0, 1}, true},
|
{"the recording is another song on the album", copies, album, []string{"rec-ice"}, -1, []int{0, 1}, true},
|
||||||
{"candidates in both copies' slots", album, []string{"rec-tormentor", "rec-record-shop"}, -1, nil, false},
|
{"candidates in both copies' slots", copies, album, []string{"rec-tormentor", "rec-record-shop"}, -1, nil, false},
|
||||||
{"the track list names no recording for a slot", []dbq.MbReleaseTrack{
|
{"the track list names no recording for a slot", copies, []dbq.MbReleaseTrack{
|
||||||
track("Tormentor", 221000, "rec-tormentor"), track("Record Shop", 180000, ""),
|
track("Tormentor", 221000, "rec-tormentor"), track("Record Shop", 180000, ""),
|
||||||
}, []string{"rec-tormentor"}, -1, nil, false},
|
}, []string{"rec-tormentor"}, -1, nil, false},
|
||||||
|
{"every copy filed as the one song named", []slotCopy{copies[0], copies[0]}, album, []string{"rec-tormentor"}, -1, nil, false},
|
||||||
}
|
}
|
||||||
for _, tc := range cases {
|
for _, tc := range cases {
|
||||||
keep, losers, ok := trimmedCopies(copies, tc.tracks, tc.recordings)
|
keep, losers, ok := namedCopies(tc.copies, tc.tracks, tc.recordings)
|
||||||
if keep != tc.keep || ok != tc.ok || fmt.Sprint(losers) != fmt.Sprint(tc.losers) {
|
if keep != tc.keep || ok != tc.ok || fmt.Sprint(losers) != fmt.Sprint(tc.losers) {
|
||||||
t.Errorf("%s: got keep=%d losers=%v ok=%v, want keep=%d losers=%v ok=%v",
|
t.Errorf("%s: got keep=%d losers=%v ok=%v, want keep=%d losers=%v ok=%v",
|
||||||
tc.name, keep, losers, ok, tc.keep, tc.losers, tc.ok)
|
tc.name, keep, losers, ok, tc.keep, tc.losers, tc.ok)
|
||||||
|
|||||||
@@ -499,6 +499,56 @@ func TestResolveDuplicates_KeepsTheCopyAcoustIDNames_Integration(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Copies whose own lookups all name Record Shop's recording are that
|
||||||
|
// recording, though their fingerprints differ a little and one is six seconds
|
||||||
|
// from Tormentor's length, too near for the lengths to decide (the operator's
|
||||||
|
// Wombats and Monday Chill groups, 2026-10-20). With no lookups, copies that
|
||||||
|
// far apart are not asked about.
|
||||||
|
func TestResolveDuplicates_AgreeingLookupsDecideAnAcousticGroup_Integration(t *testing.T) {
|
||||||
|
setup := func(t *testing.T) oneAlbumFixture {
|
||||||
|
f := seedTrimmedAlbum(t)
|
||||||
|
for _, stmt := range []string{
|
||||||
|
`UPDATE duplicate_groups SET tier = 'acoustic', worst_bit_error_rate = 0.04`,
|
||||||
|
`UPDATE mb_release_tracks SET length_ms = 226000 WHERE title = 'Tormentor'`,
|
||||||
|
} {
|
||||||
|
if _, err := f.pool.Exec(context.Background(), stmt); err != nil {
|
||||||
|
t.Fatalf("exec %q: %v", stmt, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return f
|
||||||
|
}
|
||||||
|
t.Run("agreeing", func(t *testing.T) {
|
||||||
|
f := setup(t)
|
||||||
|
if _, err := f.pool.Exec(context.Background(),
|
||||||
|
`INSERT INTO track_acoustid_lookups (track_id, state, candidates, recording_mbid)
|
||||||
|
VALUES ($1, 'matched', 1, 'rec-record-shop'), ($2, 'matched', 1, 'rec-record-shop')`,
|
||||||
|
f.a.ID, f.b.ID); err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
res, err := ResolveDuplicates(context.Background(), f.pool, nil, "", true)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if res.WrongFiles != 1 || exists(f.aPath) || !exists(f.bPath) {
|
||||||
|
t.Fatalf("result %+v, tormentor exists %v, record shop exists %v; want only Tormentor's copy removed",
|
||||||
|
res, exists(f.aPath), exists(f.bPath))
|
||||||
|
}
|
||||||
|
})
|
||||||
|
t.Run("unknown", func(t *testing.T) {
|
||||||
|
f := setup(t)
|
||||||
|
asked := 0
|
||||||
|
res, err := ResolveDuplicatesWith(context.Background(), f.pool, nil, "", true, ResolveSources{
|
||||||
|
Identify: func(context.Context, string) ([]string, error) { asked++; return nil, nil },
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatal(err)
|
||||||
|
}
|
||||||
|
if asked != 0 || res.WrongFiles != 0 || !exists(f.aPath) || !exists(f.bPath) {
|
||||||
|
t.Fatalf("asked %d, result %+v; want nothing asked or removed for copies that may be two takes", asked, res)
|
||||||
|
}
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
// AcoustID knows the recording as no song of the album: every copy goes. With
|
// AcoustID knows the recording as no song of the album: every copy goes. With
|
||||||
// AcoustID unreachable, nothing is decided and the group waits.
|
// AcoustID unreachable, nothing is decided and the group waits.
|
||||||
func TestResolveDuplicates_TrimmedCopiesAcoustIDCannotName_Integration(t *testing.T) {
|
func TestResolveDuplicates_TrimmedCopiesAcoustIDCannotName_Integration(t *testing.T) {
|
||||||
|
|||||||
@@ -34,13 +34,16 @@ import (
|
|||||||
// in the other slots are removed and their tracks marked missing, so the
|
// in the other slots are removed and their tracks marked missing, so the
|
||||||
// reacquisition sweeper wants the album back and a good copy fills them.
|
// reacquisition sweeper wants the album back and a good copy fills them.
|
||||||
// When every slot's song is clearly not the file, every copy goes.
|
// When every slot's song is clearly not the file, every copy goes.
|
||||||
// - trimmed: one recording cut to each of several slots' lengths (Savant's
|
// - named: the lengths cannot say (Savant's Mortals held one recording as
|
||||||
// Mortals held one as 16 of its 50 tracks), so every slot fits and the
|
// 16 of its 50 tracks, each cut to its own slot's length), but AcoustID
|
||||||
// lengths cannot say. AcoustID names the recording and the track list
|
// can. When every copy's lookup names one recording, or the copies are
|
||||||
// names each slot's: the copy in that recording's slot is kept and the
|
// the same audio and a lookup names any, the track list says whose slot
|
||||||
// rest go, or every copy when the recording is in none of their slots.
|
// that recording is: that copy is kept and the rest go, or every copy
|
||||||
// The release the files came from is set aside, so the album fetched
|
// when it is in none of their slots. Same audio that AcoustID does not
|
||||||
// again is not the same files (#6169).
|
// know at all is no copy's song, and every copy goes.
|
||||||
|
//
|
||||||
|
// Whenever copies go, the release the files came from is set aside, so the
|
||||||
|
// album fetched again is not the same files (#6169).
|
||||||
//
|
//
|
||||||
// Anything less certain stays with the operator.
|
// Anything less certain stays with the operator.
|
||||||
|
|
||||||
@@ -219,12 +222,15 @@ func settleWrongSlots(
|
|||||||
copies[i] = slotCopy{Title: m.Title, Artist: m.ArtistName, DurationMs: m.DurationMs}
|
copies[i] = slotCopy{Title: m.Title, Artist: m.ArtistName, DurationMs: m.DurationMs}
|
||||||
}
|
}
|
||||||
keep, losers, ok := wrongSlots(copies, tracks)
|
keep, losers, ok := wrongSlots(copies, tracks)
|
||||||
if !ok && sameAudio(g) && trimmedToSlots(copies, tracks) {
|
if !ok {
|
||||||
recordings, known := groupRecordings(ctx, logger, g, src.Identify, &identifies)
|
recordings, known := groupRecordings(ctx, logger, g, src.Identify, &identifies)
|
||||||
if !known {
|
if !known {
|
||||||
continue
|
continue
|
||||||
}
|
}
|
||||||
keep, losers, ok = trimmedCopies(copies, tracks, recordings)
|
if keep, losers, ok = namedCopies(copies, tracks, recordings); !ok {
|
||||||
|
logger.Info("duplicate resolve: AcoustID's answer does not place the recording in one slot; left for review",
|
||||||
|
"group_id", syncpkg.FormatUUID(g.id), "recordings", recordings)
|
||||||
|
}
|
||||||
}
|
}
|
||||||
if !ok {
|
if !ok {
|
||||||
continue
|
continue
|
||||||
@@ -252,39 +258,17 @@ func sameAudio(g *resolveGroup) bool {
|
|||||||
return g.tier == "exact" || (g.worstBER != nil && *g.worstBER <= sameAudioBER)
|
return g.tier == "exact" || (g.worstBER != nil && *g.worstBER <= sameAudioBER)
|
||||||
}
|
}
|
||||||
|
|
||||||
// trimmedToSlots reports whether every copy is as long as its own slot's song
|
// namedCopies decides a one-album mismatch the lengths could not, from the
|
||||||
// and they are filed as more than one song: one recording cut to several
|
// recordings AcoustID says it may be. The track list names each slot's
|
||||||
// slots' lengths, which the lengths alone cannot place.
|
// recording: the copy in a slot of one of them is kept and the others are
|
||||||
func trimmedToSlots(copies []slotCopy, tracks []dbq.MbReleaseTrack) bool {
|
// losers. When none of the copies' slots is one of them, or AcoustID knows
|
||||||
keys := map[string]bool{}
|
// none (recordings empty), every copy is another song's and all go. ok is
|
||||||
for _, c := range copies {
|
// false when they fall in more than one copy's slot, the track list names no
|
||||||
key := MatchTitleKey(c.Title, c.Artist)
|
// recording for a copy's slot, or the copies are all filed as one song.
|
||||||
fits := false
|
|
||||||
for _, t := range tracks {
|
|
||||||
if t.LengthMs > 0 && MatchTitleKey(t.Title, c.Artist) == key && absMs(t.LengthMs-c.DurationMs) <= slotFit {
|
|
||||||
fits = true
|
|
||||||
break
|
|
||||||
}
|
|
||||||
}
|
|
||||||
if !fits {
|
|
||||||
return false
|
|
||||||
}
|
|
||||||
keys[key] = true
|
|
||||||
}
|
|
||||||
return len(keys) > 1
|
|
||||||
}
|
|
||||||
|
|
||||||
// trimmedCopies decides a group trimmedToSlots holds, from the recordings
|
|
||||||
// AcoustID says it may be. The track list names each slot's recording: the
|
|
||||||
// copy in a slot of one of them is kept and the others are losers. When none
|
|
||||||
// of the copies' slots is one of them, or AcoustID knows none (recordings
|
|
||||||
// empty), every copy is another song's and all go. ok is false when they fall
|
|
||||||
// in more than one copy's slot, or the track list names no recording for a
|
|
||||||
// copy's slot.
|
|
||||||
//
|
//
|
||||||
// Copies sharing the kept copy's title are one song held twice, as in
|
// Copies sharing the kept copy's title are one song held twice, as in
|
||||||
// wrongSlots: neither kept nor lost here.
|
// wrongSlots: neither kept nor lost here.
|
||||||
func trimmedCopies(copies []slotCopy, tracks []dbq.MbReleaseTrack, recordings []string) (keep int, losers []int, ok bool) {
|
func namedCopies(copies []slotCopy, tracks []dbq.MbReleaseTrack, recordings []string) (keep int, losers []int, ok bool) {
|
||||||
keys := make([]string, len(copies))
|
keys := make([]string, len(copies))
|
||||||
theirs := map[string]bool{}
|
theirs := map[string]bool{}
|
||||||
for i, c := range copies {
|
for i, c := range copies {
|
||||||
@@ -316,22 +300,28 @@ func trimmedCopies(copies []slotCopy, tracks []dbq.MbReleaseTrack, recordings []
|
|||||||
losers = append(losers, i)
|
losers = append(losers, i)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if len(losers) == 0 {
|
||||||
|
return -1, nil, false
|
||||||
|
}
|
||||||
return keep, losers, true
|
return keep, losers, true
|
||||||
}
|
}
|
||||||
|
|
||||||
// groupRecordings is what AcoustID says a group's recording may be: the
|
// groupRecordings is what AcoustID says a group's recording may be. Copies
|
||||||
// recordings the copies' own lookups matched, none when every copy's lookup
|
// whose own lookups all name one recording are that recording, however their
|
||||||
// found no match, or else a lookup of one copy now, while the pass has
|
// fingerprints compare. Otherwise only copies that are the same audio
|
||||||
// lookups left. known is false when there is no answer yet, and the group
|
// (sameAudio) are asked about: the recordings any copy's lookup matched, none
|
||||||
// waits for a later pass.
|
// when every copy's lookup found no match, or else a lookup of one copy now,
|
||||||
|
// while the pass has lookups left. known is false when there is no answer
|
||||||
|
// yet, and the group waits.
|
||||||
func groupRecordings(
|
func groupRecordings(
|
||||||
ctx context.Context, logger *slog.Logger, g *resolveGroup, identify RecordingIdentifier, left *int,
|
ctx context.Context, logger *slog.Logger, g *resolveGroup, identify RecordingIdentifier, left *int,
|
||||||
) (recordings []string, known bool) {
|
) (recordings []string, known bool) {
|
||||||
noMatch := 0
|
matched, noMatch := 0, 0
|
||||||
for _, m := range g.members {
|
for _, m := range g.members {
|
||||||
switch {
|
switch {
|
||||||
case m.AcoustidState == nil:
|
case m.AcoustidState == nil:
|
||||||
case *m.AcoustidState == lookupMatched && m.AcoustidRecording != nil:
|
case *m.AcoustidState == lookupMatched && m.AcoustidRecording != nil:
|
||||||
|
matched++
|
||||||
if !slices.Contains(recordings, *m.AcoustidRecording) {
|
if !slices.Contains(recordings, *m.AcoustidRecording) {
|
||||||
recordings = append(recordings, *m.AcoustidRecording)
|
recordings = append(recordings, *m.AcoustidRecording)
|
||||||
}
|
}
|
||||||
@@ -339,6 +329,12 @@ func groupRecordings(
|
|||||||
noMatch++
|
noMatch++
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
if matched == len(g.members) && len(recordings) == 1 {
|
||||||
|
return recordings, true
|
||||||
|
}
|
||||||
|
if !sameAudio(g) {
|
||||||
|
return nil, false
|
||||||
|
}
|
||||||
if len(recordings) > 0 || noMatch == len(g.members) {
|
if len(recordings) > 0 || noMatch == len(g.members) {
|
||||||
return recordings, true
|
return recordings, true
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user