feat: one recording cut to several songs' lengths is named by AcoustID; the copy in its slot stays, the rest go, and the release is set aside (#6169)
release / govulncheck (push) Successful in 14s
release / web (push) Successful in 1m21s
release / go (push) Successful in 1m31s
release / android (push) Successful in 5m1s
release / integration (push) Successful in 5m1s
release / Build signed APK (releases and dev) (push) Successful in 5m9s
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

Savant's Mortals held one recording as 16 of its 50 tracks, each trimmed to
its own slot's MusicBrainz length, so the lengths could not say which slot
was its own and every group waited for the operator. When a one-album
mismatch is the same audio (identical, or fingerprints within 0.02) and
every copy fits its own slot, the resolver asks what the recording is: the
copies' own AcoustID lookups, or a fresh lookup of one copy (five a pass).
The track list names each slot's recording; the copy in that slot is kept
and the others removed, or every copy when the recording is in none of
their slots or AcoustID knows none. AcoustID unreachable or answers in
several copies' slots: the group waits.

Whenever wrong-slot copies are removed, the download the files were
imported from is set aside, so the album fetched again is not the same
files. Albums imported before Minstrel downloaded anything have none.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
2026-10-20 10:33:55 -04:00
co-authored by Claude Opus 5.5
parent e3eddab80f
commit 18d31e9cdd
10 changed files with 520 additions and 58 deletions
+3 -1
View File
@@ -367,7 +367,9 @@ func run() error {
// catalogue for the track list of an album whose wrong slot it must
// decide, a few a pass. See internal/library/duplicate_resolve.go.
go library.NewDuplicateResolveWorker(pool, logger.With("component", "duplicate_resolve"),
cfg.Storage.DataDir, fpSettings, cat.RefreshReleaseGroup).Run(ctx)
cfg.Storage.DataDir, fpSettings, library.ResolveSources{
TrackLists: cat.RefreshReleaseGroup, Identify: acoustIDLookup.IdentifyRecordings,
}).Run(ctx)
// Missing-file re-acquisition (milestone #290, M502 #5520). Wants back
// albums whose files have been gone longer than the grace window; the
+32 -19
View File
@@ -296,6 +296,7 @@ func (q *Queries) ListDuplicateCandidates(ctx context.Context, arg ListDuplicate
const listDuplicateGroupsForResolve = `-- name: ListDuplicateGroupsForResolve :many
SELECT g.id AS group_id,
g.tier,
g.worst_bit_error_rate,
g.class AS prior_class,
t.id AS track_id,
t.album_id,
@@ -315,37 +316,43 @@ SELECT g.id AS group_id,
AND t.track_number IS NOT NULL
AND o.disc_number IS NOT DISTINCT FROM t.disc_number
AND o.track_number = t.track_number)::boolean AS position_clash,
COALESCE(NULLIF(al.release_group_mbid, ''), lam.release_group_mbid, '')::text AS release_group_mbid
COALESCE(NULLIF(al.release_group_mbid, ''), lam.release_group_mbid, '')::text AS release_group_mbid,
l.state AS acoustid_state,
l.recording_mbid AS acoustid_recording
FROM duplicate_groups g
JOIN duplicate_group_members m ON m.group_id = g.id
JOIN tracks t ON t.id = m.track_id
JOIN artists ON artists.id = t.artist_id
JOIN albums al ON al.id = t.album_id
LEFT JOIN library_album_matches lam ON lam.album_id = t.album_id AND lam.outcome = 'listed'
LEFT JOIN track_acoustid_lookups l ON l.track_id = t.id
WHERE g.status = 'pending'
AND t.missing_since IS NULL
ORDER BY g.id, t.id
`
type ListDuplicateGroupsForResolveRow struct {
GroupID pgtype.UUID
Tier string
PriorClass *string
TrackID pgtype.UUID
AlbumID pgtype.UUID
Title string
SourceVerdict *string
ArtistName string
FilePath string
FileFormat string
FileSize int64
DurationMs int32
AddedAt pgtype.Timestamptz
DiscNumber *int32
TrackNumber *int32
HasMbid bool
PositionClash bool
ReleaseGroupMbid string
GroupID pgtype.UUID
Tier string
WorstBitErrorRate *float32
PriorClass *string
TrackID pgtype.UUID
AlbumID pgtype.UUID
Title string
SourceVerdict *string
ArtistName string
FilePath string
FileFormat string
FileSize int64
DurationMs int32
AddedAt pgtype.Timestamptz
DiscNumber *int32
TrackNumber *int32
HasMbid bool
PositionClash bool
ReleaseGroupMbid string
AcoustidState *string
AcoustidRecording *string
}
// Every pending group's present copies, for the resolver (M498 #5435): what it
@@ -357,6 +364,9 @@ type ListDuplicateGroupsForResolveRow struct {
// prior_class is the class the last pass gave the group, NULL for a group no
// pass has seen. release_group_mbid is the album's MusicBrainz group, from its
// tags or the library listing's match; ” when neither names one.
// acoustid_state and acoustid_recording are what AcoustID said of the file,
// when it was looked up (#6169: which song one recording cut to several slots
// is).
func (q *Queries) ListDuplicateGroupsForResolve(ctx context.Context) ([]ListDuplicateGroupsForResolveRow, error) {
rows, err := q.db.Query(ctx, listDuplicateGroupsForResolve)
if err != nil {
@@ -369,6 +379,7 @@ func (q *Queries) ListDuplicateGroupsForResolve(ctx context.Context) ([]ListDupl
if err := rows.Scan(
&i.GroupID,
&i.Tier,
&i.WorstBitErrorRate,
&i.PriorClass,
&i.TrackID,
&i.AlbumID,
@@ -385,6 +396,8 @@ func (q *Queries) ListDuplicateGroupsForResolve(ctx context.Context) ([]ListDupl
&i.HasMbid,
&i.PositionClash,
&i.ReleaseGroupMbid,
&i.AcoustidState,
&i.AcoustidRecording,
); err != nil {
return nil, err
}
+44
View File
@@ -348,6 +348,50 @@ func (q *Queries) ListGrabs(ctx context.Context, arg ListGrabsParams) ([]ListGra
return items, nil
}
const listImportedGrabsHolding = `-- name: ListImportedGrabsHolding :many
SELECT id, info_hash, name, want_id
FROM grabs
WHERE import_state = 'imported'
AND info_hash <> ''
AND destination <> ''
AND starts_with($1::text, rtrim(destination, '/') || '/')
`
type ListImportedGrabsHoldingRow struct {
ID pgtype.UUID
InfoHash string
Name string
WantID pgtype.UUID
}
// The downloads Minstrel imported into the folder holding path: the releases a
// file there came from, to block when the files proved not to be the album
// (#6169). A file imported before Minstrel downloaded anything has none.
func (q *Queries) ListImportedGrabsHolding(ctx context.Context, path string) ([]ListImportedGrabsHoldingRow, error) {
rows, err := q.db.Query(ctx, listImportedGrabsHolding, path)
if err != nil {
return nil, err
}
defer rows.Close()
var items []ListImportedGrabsHoldingRow
for rows.Next() {
var i ListImportedGrabsHoldingRow
if err := rows.Scan(
&i.ID,
&i.InfoHash,
&i.Name,
&i.WantID,
); err != nil {
return nil, err
}
items = append(items, i)
}
if err := rows.Err(); err != nil {
return nil, err
}
return items, nil
}
const listLiveGrabs = `-- name: ListLiveGrabs :many
SELECT g.id, g.want_id, g.info_hash, g.name, g.key, g.indexer, g.source, g.client, g.save_path, g.quality, g.origin, g.forced, g.grabbed_by, g.grabbed_at, g.ended_at, g.end_reason, g.end_detail, g.progress, g.import_state, g.import_reason, g.import_detail, g.import_forced, g.import_attempts, g.import_updated_at, g.imported_at, g.imported_files, g.destination, g.seeding, g.ratio, g.seed_released_at, w.release_group_mbid, w.artist_mbid, w.artist_name, w.title AS want_title,
+8 -1
View File
@@ -195,8 +195,12 @@ UPDATE duplicate_groups
-- prior_class is the class the last pass gave the group, NULL for a group no
-- pass has seen. release_group_mbid is the album's MusicBrainz group, from its
-- tags or the library listing's match; '' when neither names one.
-- acoustid_state and acoustid_recording are what AcoustID said of the file,
-- when it was looked up (#6169: which song one recording cut to several slots
-- is).
SELECT g.id AS group_id,
g.tier,
g.worst_bit_error_rate,
g.class AS prior_class,
t.id AS track_id,
t.album_id,
@@ -216,13 +220,16 @@ SELECT g.id AS group_id,
AND t.track_number IS NOT NULL
AND o.disc_number IS NOT DISTINCT FROM t.disc_number
AND o.track_number = t.track_number)::boolean AS position_clash,
COALESCE(NULLIF(al.release_group_mbid, ''), lam.release_group_mbid, '')::text AS release_group_mbid
COALESCE(NULLIF(al.release_group_mbid, ''), lam.release_group_mbid, '')::text AS release_group_mbid,
l.state AS acoustid_state,
l.recording_mbid AS acoustid_recording
FROM duplicate_groups g
JOIN duplicate_group_members m ON m.group_id = g.id
JOIN tracks t ON t.id = m.track_id
JOIN artists ON artists.id = t.artist_id
JOIN albums al ON al.id = t.album_id
LEFT JOIN library_album_matches lam ON lam.album_id = t.album_id AND lam.outcome = 'listed'
LEFT JOIN track_acoustid_lookups l ON l.track_id = t.id
WHERE g.status = 'pending'
AND t.missing_since IS NULL
ORDER BY g.id, t.id;
+11
View File
@@ -163,3 +163,14 @@ UPDATE wants
-- A track whose file was replaced in place is no longer the rip it was.
UPDATE tracks SET source_verdict = NULL, source_verdict_at = NULL
WHERE id = $1 AND source_verdict = 'suspect';
-- name: ListImportedGrabsHolding :many
-- The downloads Minstrel imported into the folder holding path: the releases a
-- file there came from, to block when the files proved not to be the album
-- (#6169). A file imported before Minstrel downloaded anything has none.
SELECT id, info_hash, name, want_id
FROM grabs
WHERE import_state = 'imported'
AND info_hash <> ''
AND destination <> ''
AND starts_with(sqlc.arg(path)::text, rtrim(destination, '/') || '/');
+40
View File
@@ -345,6 +345,46 @@ func (w *AcoustIDLookupWorker) lookupTrack(
return w.store(ctx, row, chooseRecording(cands, cfg.MinScore, row.Title, row.DurationMs)), nil
}
// ErrIdentifyUnavailable says AcoustID cannot be asked now: the lookup is off
// or has no key, the key was refused, or AcoustID could not be reached. The
// caller waits and asks again on a later pass.
var ErrIdentifyUnavailable = errors.New("acoustid: cannot be asked now")
// IdentifyRecordings asks AcoustID which recordings a file may be, for the
// duplicate resolver (#6169): every candidate at or above the minimum score,
// best first. None is an answer: AcoustID does not know the audio. Nothing is
// stored; the file's own lookup is the worker's.
func (w *AcoustIDLookupWorker) IdentifyRecordings(ctx context.Context, path string) ([]string, error) {
if w == nil {
return nil, ErrIdentifyUnavailable
}
cfg := w.settings.Get()
if !cfg.Ready() {
return nil, ErrIdentifyUnavailable
}
fp, err := w.fingerprint(ctx, path)
if err != nil {
if isInconclusive(err) {
return nil, fmt.Errorf("%w: %w", ErrIdentifyUnavailable, err)
}
return nil, fmt.Errorf("fingerprint: %w", err)
}
cands, err := w.client.Lookup(ctx, cfg.APIKey, fp.fingerprint, fp.durationSec)
switch {
case errors.Is(err, acoustid.ErrInvalidKey), errors.Is(err, acoustid.ErrUnavailable):
return nil, fmt.Errorf("%w: %w", ErrIdentifyUnavailable, err)
case err != nil:
return nil, err
}
var ids []string
for _, c := range cands {
if float32(c.Score) >= cfg.MinScore {
ids = append(ids, c.ID)
}
}
return ids, nil
}
// store records a decision and applies it to tracks.mbid in one transaction,
// so the gauge never shows a match whose id was not written. A failed write
// stores nothing and the track is tried again.
@@ -113,3 +113,51 @@ func TestWrongSlots(t *testing.T) {
}
}
}
// One recording cut to each of several slots' lengths (Savant's Mortals): the
// lengths cannot place it, and what AcoustID calls it does (#6169).
func TestTrimmedCopies(t *testing.T) {
track := func(title string, ms int32, rec string) dbq.MbReleaseTrack {
return dbq.MbReleaseTrack{Title: title, LengthMs: ms, RecordingMbid: rec}
}
copies := []slotCopy{
{Title: "Tormentor", Artist: "The Wombats", DurationMs: 220000},
{Title: "Record Shop", Artist: "The Wombats", DurationMs: 181000},
}
album := []dbq.MbReleaseTrack{
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 {
name string
tracks []dbq.MbReleaseTrack
recordings []string
keep int
losers []int
ok bool
}{
{"AcoustID names Tormentor", 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},
{"AcoustID knows none", album, nil, -1, []int{0, 1}, true},
{"the recording is another song on the album", album, []string{"rec-ice"}, -1, []int{0, 1}, true},
{"candidates in both copies' slots", album, []string{"rec-tormentor", "rec-record-shop"}, -1, nil, false},
{"the track list names no recording for a slot", []dbq.MbReleaseTrack{
track("Tormentor", 221000, "rec-tormentor"), track("Record Shop", 180000, ""),
}, []string{"rec-tormentor"}, -1, nil, false},
}
for _, tc := range cases {
keep, losers, ok := trimmedCopies(copies, tc.tracks, tc.recordings)
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",
tc.name, keep, losers, ok, tc.keep, tc.losers, tc.ok)
}
}
}
+40 -23
View File
@@ -34,8 +34,9 @@ import (
// - Two versions of a song on one album that sound alike are not duplicates:
// the group is dismissed (duplicate_settle.go).
// - One file filling several of an album's track slots: MusicBrainz's track
// lengths say which slot it belongs to, and the copies in the other slots
// are removed so the album is fetched again (duplicate_settle.go).
// lengths say which slot it belongs to, or AcoustID which song it is when
// each copy was cut to its own slot's length, and the copies in the other
// slots are removed so the album is fetched again (duplicate_settle.go).
// - Anything else waits for the operator.
const (
@@ -65,11 +66,14 @@ type DuplicateResolveResult struct {
// resolveGroup is one pending group and its present copies.
type resolveGroup struct {
id pgtype.UUID
tier string
members []dbq.ListDuplicateGroupsForResolveRow
class DuplicateClass
note string
id pgtype.UUID
tier string
// worstBER is how far apart the least alike copies' fingerprints are, for
// an acoustic group; nil for an exact one.
worstBER *float32
members []dbq.ListDuplicateGroupsForResolveRow
class DuplicateClass
note string
}
// isRip says whether a copy is a video rip: held back as one, or named like
@@ -116,20 +120,33 @@ func mergePlan(g *resolveGroup) (ok bool, survivor int) {
// MusicBrainz into the catalogue (catalogue.Service.RefreshReleaseGroup).
type TrackListFetcher func(ctx context.Context, releaseGroupMBID string) error
// RecordingIdentifier asks AcoustID which recordings a file may be
// (AcoustIDLookupWorker.IdentifyRecordings).
type RecordingIdentifier func(ctx context.Context, path string) ([]string, error)
// ResolveSources are what a pass may ask beyond the library, a few times per
// pass each. Either may be nil, and the pass decides from what is held.
type ResolveSources struct {
// TrackLists reads the track list of an album whose wrong slot the pass
// must decide and the catalogue has never read (trackListFetchesPerPass).
TrackLists TrackListFetcher
// Identify names the recording cut to several slots' lengths, when no
// copy's own lookup has (identifiesPerPass).
Identify RecordingIdentifier
}
// ResolveDuplicates runs one pass. act is the operator's auto-resolve setting;
// with it off, the pass only classifies. It reads only track lists already in
// the catalogue; ResolveDuplicatesFetching may also fetch missing ones.
// with it off, the pass only classifies. It decides from what the library and
// catalogue hold; ResolveDuplicatesWith may also ask MusicBrainz and AcoustID.
func ResolveDuplicates(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, act bool,
) (DuplicateResolveResult, error) {
return ResolveDuplicatesFetching(ctx, pool, logger, dataDir, act, nil)
return ResolveDuplicatesWith(ctx, pool, logger, dataDir, act, ResolveSources{})
}
// ResolveDuplicatesFetching is ResolveDuplicates that asks MusicBrainz, through
// fetch, for the track list of an album whose wrong slot it must decide and
// the catalogue has never read: a few per pass (trackListFetchesPerPass).
func ResolveDuplicatesFetching(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, act bool, fetch TrackListFetcher,
// ResolveDuplicatesWith is ResolveDuplicates that may ask src.
func ResolveDuplicatesWith(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, act bool, src ResolveSources,
) (DuplicateResolveResult, error) {
if logger == nil {
logger = slog.Default()
@@ -196,7 +213,7 @@ func ResolveDuplicatesFetching(
}
res.Dismissed = dismissDistinct(ctx, pool, logger, groups, merged)
wrong, names := settleWrongSlots(ctx, pool, logger, groups, merged, fetch)
wrong, names := settleWrongSlots(ctx, pool, logger, groups, merged, src)
res.WrongFiles = wrong
mergedNames = append(mergedNames, names...)
@@ -281,7 +298,7 @@ func foldResolveGroups(rows []dbq.ListDuplicateGroupsForResolveRow) []*resolveGr
var groups []*resolveGroup
for _, r := range rows {
if n := len(groups); n == 0 || groups[n-1].id != r.GroupID {
groups = append(groups, &resolveGroup{id: r.GroupID, tier: r.Tier})
groups = append(groups, &resolveGroup{id: r.GroupID, tier: r.Tier, worstBER: r.WorstBitErrorRate})
}
g := groups[len(groups)-1]
g.members = append(g.members, r)
@@ -389,17 +406,17 @@ type DuplicateResolveWorker struct {
logger *slog.Logger
dataDir string
settings *FingerprintSettingsService
fetch TrackListFetcher
src ResolveSources
tick time.Duration
}
// NewDuplicateResolveWorker builds a worker with the production cadence. fetch
// reads a missing track list from MusicBrainz; nil reads only what is held.
// NewDuplicateResolveWorker builds a worker with the production cadence. src
// is what a pass may ask beyond the library.
func NewDuplicateResolveWorker(
pool *pgxpool.Pool, logger *slog.Logger, dataDir string, settings *FingerprintSettingsService, fetch TrackListFetcher,
pool *pgxpool.Pool, logger *slog.Logger, dataDir string, settings *FingerprintSettingsService, src ResolveSources,
) *DuplicateResolveWorker {
return &DuplicateResolveWorker{
pool: pool, logger: logger, dataDir: dataDir, settings: settings, fetch: fetch, tick: duplicateResolveTick,
pool: pool, logger: logger, dataDir: dataDir, settings: settings, src: src, tick: duplicateResolveTick,
}
}
@@ -436,7 +453,7 @@ func (w *DuplicateResolveWorker) tickOnce(ctx context.Context) {
}
return
}
res, err := ResolveDuplicatesFetching(ctx, w.pool, w.logger, w.dataDir, w.settings.Get().AutoResolve, w.fetch)
res, err := ResolveDuplicatesWith(ctx, w.pool, w.logger, w.dataDir, w.settings.Get().AutoResolve, w.src)
if err != nil && ctx.Err() == nil {
w.logger.Warn("duplicate resolve: pass failed", "err", err)
}
+103 -1
View File
@@ -436,6 +436,108 @@ func TestResolveDuplicates_RemovesEveryCopyWhenNoSlotIsTheirs_Integration(t *tes
}
}
// seedTrimmedAlbum is newOneAlbumFixture for one recording cut to both its
// slots' lengths: identical audio, as Tormentor and as Record Shop, each as
// long as its own slot's song, and the track list naming each slot's
// recording. The files came from a download Minstrel imported.
func seedTrimmedAlbum(t *testing.T) oneAlbumFixture {
t.Helper()
f := newOneAlbumFixture(t, "exact", "Tormentor", "Record Shop")
ctx := context.Background()
seedTrackList(t, f.pool, f.album, map[string]int{"Tormentor": 221000, "Record Shop": 219000})
for _, stmt := range []struct {
sql string
args []any
}{
{`UPDATE mb_release_tracks SET recording_mbid = 'rec-' || lower(replace(title, ' ', '-'))`, nil},
{`INSERT INTO grabs (info_hash, name, key, client, save_path, origin, import_state, destination)
VALUES ('abc123', 'The Wombats - Void [WEB]', 'k', 'qbit', '/dl', 'schedule', 'imported', $1)`,
[]any{filepath.Dir(f.aPath)}},
} {
if _, err := f.pool.Exec(ctx, stmt.sql, stmt.args...); err != nil {
t.Fatalf("exec %q: %v", stmt.sql, err)
}
}
return f
}
func releaseBlocked(t *testing.T, pool *pgxpool.Pool, hash string) bool {
t.Helper()
var n int
if err := pool.QueryRow(context.Background(),
`SELECT count(*) FROM release_blocks WHERE info_hash = $1 AND reason = 'files'`, hash).Scan(&n); err != nil {
t.Fatal(err)
}
return n == 1
}
// AcoustID matched one copy to Tormentor's recording: that copy stays, Record
// Shop's goes, and the download they came from is set aside (#6169).
func TestResolveDuplicates_KeepsTheCopyAcoustIDNames_Integration(t *testing.T) {
f := seedTrimmedAlbum(t)
if _, err := f.pool.Exec(context.Background(),
`INSERT INTO track_acoustid_lookups (track_id, state, candidates, recording_mbid) VALUES ($1, 'matched', 1, 'rec-tormentor')`,
f.a.ID); err != nil {
t.Fatal(err)
}
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 {
t.Errorf("AcoustID asked %d times; the copy's own lookup already answered", asked)
}
if res.WrongFiles != 1 || !exists(f.aPath) || exists(f.bPath) {
t.Fatalf("result %+v, tormentor exists %v, record shop exists %v; want only Record Shop's copy removed",
res, exists(f.aPath), exists(f.bPath))
}
if !releaseBlocked(t, f.pool, "abc123") {
t.Error("the download the files came from was not set aside, so the album would be fetched from it again")
}
}
// AcoustID knows the recording as no song of the album: every copy goes. With
// AcoustID unreachable, nothing is decided and the group waits.
func TestResolveDuplicates_TrimmedCopiesAcoustIDCannotName_Integration(t *testing.T) {
t.Run("no match", func(t *testing.T) {
f := seedTrimmedAlbum(t)
asked := 0
res, err := ResolveDuplicatesWith(context.Background(), f.pool, nil, "", true, ResolveSources{
Identify: func(context.Context, string) ([]string, error) {
asked++
return []string{"rec-elsewhere"}, nil
},
})
if err != nil {
t.Fatal(err)
}
if asked != 1 || res.WrongFiles != 2 || exists(f.aPath) || exists(f.bPath) {
t.Fatalf("asked %d, result %+v; want one lookup and both copies removed", asked, res)
}
if status, _, _, _ := (resolveFixture{pool: f.pool, groupID: f.groupID}).group(t); status != "merged" {
t.Errorf("group = %s, want settled", status)
}
})
t.Run("unreachable", func(t *testing.T) {
f := seedTrimmedAlbum(t)
res, err := ResolveDuplicatesWith(context.Background(), f.pool, nil, "", true, ResolveSources{
Identify: func(context.Context, string) ([]string, error) { return nil, ErrIdentifyUnavailable },
})
if err != nil {
t.Fatal(err)
}
if res.WrongFiles != 0 || !exists(f.aPath) || !exists(f.bPath) || releaseBlocked(t, f.pool, "abc123") {
t.Fatalf("result %+v; want nothing removed or set aside while AcoustID cannot answer", res)
}
if status, _, _, _ := (resolveFixture{pool: f.pool, groupID: f.groupID}).group(t); status != "pending" {
t.Errorf("group = %s, want pending", status)
}
})
}
// An album whose track list the catalogue never read: the resolver asks for
// it, once, and decides with what comes back.
func TestResolveDuplicates_FetchesAMissingTrackList_Integration(t *testing.T) {
@@ -454,7 +556,7 @@ func TestResolveDuplicates_FetchesAMissingTrackList_Integration(t *testing.T) {
seedTrackList(t, f.pool, f.album, map[string]int{"Void": 220500, "Reach": 281000})
return nil
}
res, err := ResolveDuplicatesFetching(context.Background(), f.pool, nil, "", true, fetch)
res, err := ResolveDuplicatesWith(context.Background(), f.pool, nil, "", true, ResolveSources{TrackLists: fetch})
if err != nil {
t.Fatal(err)
}
+191 -13
View File
@@ -6,11 +6,13 @@ import (
"fmt"
"log/slog"
"os"
"slices"
"github.com/jackc/pgx/v5"
"github.com/jackc/pgx/v5/pgtype"
"github.com/jackc/pgx/v5/pgxpool"
"git.fabledsword.com/bvandeusen/minstrel/internal/acquire"
"git.fabledsword.com/bvandeusen/minstrel/internal/audit"
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
"git.fabledsword.com/bvandeusen/minstrel/internal/musicbrainz"
@@ -32,7 +34,15 @@ import (
// 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.
// When every slot's song is clearly not the file, every copy goes.
// Anything less certain stays with the operator.
// - trimmed: one recording cut to each of several slots' lengths (Savant's
// Mortals held one as 16 of its 50 tracks), so every slot fits and the
// lengths cannot say. AcoustID names the recording and the track list
// names each slot's: the copy in that recording's slot is kept and the
// rest go, or every copy when the recording is in none of their slots.
// 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.
const (
// trackListFetchesPerPass is how many albums' track lists a pass may ask
@@ -44,6 +54,14 @@ const (
// title must be for the file to be another song's.
slotFit = 3000
slotOther = 10000
// identifiesPerPass is how many groups a pass may ask AcoustID about. The
// client keeps to AcoustID's rate; this keeps a pass short.
identifiesPerPass = 5
// sameAudioBER is the worst bit error rate at which an acoustic group's
// copies are one recording cut differently rather than takes that sound
// alike: the groups measured as one recording trimmed to several slots
// read 0.000, and two takes of one song read far above this (#6169).
sameAudioBER = 0.02
)
// slotCopy is what deciding a wrong slot needs to know about one copy.
@@ -149,19 +167,22 @@ func dismissDistinct(
}
// settleWrongSlots removes the copies that sit in another song's slot, for
// every one-album mismatch the track list decides. It returns how many copies
// went and the songs whose slots were emptied, and adds the groups it settled
// to settled.
// every one-album mismatch the track list, or AcoustID with it, decides. It
// returns how many copies went and the songs whose slots were emptied, and
// adds the groups it settled to settled.
func settleWrongSlots(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, groups []*resolveGroup, settled map[pgtype.UUID]bool,
fetch TrackListFetcher,
src ResolveSources,
) (int, []string) {
q := dbq.New(pool)
tracksByGroup := map[string][]dbq.MbReleaseTrack{}
fetches := trackListFetchesPerPass
if fetch == nil {
fetches, identifies := trackListFetchesPerPass, identifiesPerPass
if src.TrackLists == nil {
fetches = 0
}
if src.Identify == nil {
identifies = 0
}
removed := 0
var names []string
for _, g := range groups {
@@ -181,7 +202,7 @@ func settleWrongSlots(
}
if len(tracks) == 0 && fetches > 0 && neverRead(ctx, q, rg) {
fetches--
if err := fetch(ctx, rg); err != nil {
if err := src.TrackLists(ctx, rg); err != nil {
logger.Info("duplicate resolve: fetching the album's track list failed", "release_group", rg, "err", err)
if errors.Is(err, musicbrainz.ErrUnreachable) {
fetches = 0
@@ -198,10 +219,20 @@ func settleWrongSlots(
copies[i] = slotCopy{Title: m.Title, Artist: m.ArtistName, DurationMs: m.DurationMs}
}
keep, losers, ok := wrongSlots(copies, tracks)
if !ok && sameAudio(g) && trimmedToSlots(copies, tracks) {
recordings, known := groupRecordings(ctx, logger, g, src.Identify, &identifies)
if !known {
continue
}
keep, losers, ok = trimmedCopies(copies, tracks, recordings)
}
if !ok {
continue
}
n, err := removeWrongSlots(ctx, pool, logger, g, keep, losers)
// Set aside before the files go: a pass that dies between the two
// still does not fetch the same files again.
blocked := blockReleasesHolding(ctx, pool, logger, g)
n, err := removeWrongSlots(ctx, pool, logger, g, keep, losers, blocked)
removed += n
if err != nil {
logger.Warn("duplicate resolve: removing wrong-slot copies failed", "group_id", syncpkg.FormatUUID(g.id), "err", err)
@@ -215,6 +246,149 @@ func settleWrongSlots(
return removed, names
}
// sameAudio reports whether a group's copies are one recording, cut however:
// identical audio, or fingerprints all but identical (sameAudioBER).
func sameAudio(g *resolveGroup) bool {
return g.tier == "exact" || (g.worstBER != nil && *g.worstBER <= sameAudioBER)
}
// trimmedToSlots reports whether every copy is as long as its own slot's song
// and they are filed as more than one song: one recording cut to several
// slots' lengths, which the lengths alone cannot place.
func trimmedToSlots(copies []slotCopy, tracks []dbq.MbReleaseTrack) bool {
keys := map[string]bool{}
for _, c := range copies {
key := MatchTitleKey(c.Title, c.Artist)
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
// wrongSlots: neither kept nor lost here.
func trimmedCopies(copies []slotCopy, tracks []dbq.MbReleaseTrack, recordings []string) (keep int, losers []int, ok bool) {
keys := make([]string, len(copies))
theirs := map[string]bool{}
for i, c := range copies {
keys[i] = MatchTitleKey(c.Title, c.Artist)
named := false
for _, t := range tracks {
if t.RecordingMbid == "" || MatchTitleKey(t.Title, c.Artist) != keys[i] {
continue
}
named = true
if slices.Contains(recordings, t.RecordingMbid) {
theirs[keys[i]] = true
}
}
if !named {
return -1, nil, false
}
}
if len(theirs) > 1 {
return -1, nil, false
}
keep = -1
for i, key := range keys {
switch {
case theirs[key] && keep < 0:
keep = i
case theirs[key]:
default:
losers = append(losers, i)
}
}
return keep, losers, true
}
// groupRecordings is what AcoustID says a group's recording may be: the
// recordings the copies' own lookups matched, none 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 for a later pass.
func groupRecordings(
ctx context.Context, logger *slog.Logger, g *resolveGroup, identify RecordingIdentifier, left *int,
) (recordings []string, known bool) {
noMatch := 0
for _, m := range g.members {
switch {
case m.AcoustidState == nil:
case *m.AcoustidState == lookupMatched && m.AcoustidRecording != nil:
if !slices.Contains(recordings, *m.AcoustidRecording) {
recordings = append(recordings, *m.AcoustidRecording)
}
case *m.AcoustidState == lookupNoMatch:
noMatch++
}
}
if len(recordings) > 0 || noMatch == len(g.members) {
return recordings, true
}
if identify == nil || *left <= 0 {
return nil, false
}
*left--
ids, err := identify(ctx, g.members[0].FilePath)
if err != nil {
if errors.Is(err, ErrIdentifyUnavailable) {
*left = 0
}
logger.Info("duplicate resolve: asking AcoustID which song a recording is failed",
"group_id", syncpkg.FormatUUID(g.id), "err", err)
return nil, false
}
return ids, true
}
// blockReleasesHolding sets aside the downloads a group's files were imported
// from, and returns their names. An album imported before Minstrel downloaded
// anything has none.
func blockReleasesHolding(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, g *resolveGroup) []string {
q := dbq.New(pool)
seen := map[string]bool{}
var names []string
for _, m := range g.members {
grabs, err := q.ListImportedGrabsHolding(ctx, m.FilePath)
if err != nil {
logger.Warn("duplicate resolve: finding the download a file came from failed", "path", m.FilePath, "err", err)
continue
}
for _, gr := range grabs {
if seen[gr.InfoHash] {
continue
}
seen[gr.InfoHash] = true
if err := acquire.NewMemory(pool).Block(ctx, dbq.BlockReleaseParams{
InfoHash: gr.InfoHash, Name: gr.Name, Reason: acquire.BlockFiles,
Detail: "one recording fills several of the album's tracks.", WantID: gr.WantID,
}); err != nil {
logger.Warn("duplicate resolve: setting the release aside failed", "release", gr.Name, "err", err)
continue
}
names = append(names, gr.Name)
}
}
return names
}
// neverRead reports whether the catalogue has never read a release group's
// releases. A group read and found empty is not asked about again: the
// catalogue's own refresh revisits it.
@@ -227,11 +401,12 @@ func neverRead(ctx context.Context, q *dbq.Queries, rg string) bool {
// the same transaction that closes the group. The row is kept, with its likes
// and plays: it is the slot, and a good copy fetched again takes it over. Its
// recording MBID is dropped, being the wrong file's, so missing-pair folding
// does not fold the slot into the copy that was kept. A
// file that cannot be removed stops the group; the ones already gone are still
// marked, and the group waits for the next pass.
// does not fold the slot into the copy that was kept. blocked names the
// releases set aside, for the record. A file that cannot be removed stops the
// group; the ones already gone are still marked, and the group waits for the
// next pass.
func removeWrongSlots(
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, g *resolveGroup, keep int, losers []int,
ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, g *resolveGroup, keep int, losers []int, blocked []string,
) (int, error) {
var gone []pgtype.UUID
var removedFiles []map[string]string
@@ -278,6 +453,9 @@ func removeWrongSlots(
"artist": g.members[0].ArtistName,
"removed": removedFiles,
}
if len(blocked) > 0 {
details["blocked"] = blocked
}
keptPath := ""
if keep >= 0 {
kept := g.members[keep]