refactor(library): move detection matches on the audio hash, not size and duration (M400 #3914)
test-go / test (push) Successful in 1m9s
test-go / integration (push) Successful in 3m45s
release / Build signed APK (releases and dev) (push) Successful in 4m52s
release / Build + push container image (push) Successful in 14s
release / Verify release artifacts (tag releases only) (push) Skipped

A file that comes back renamed or moved keeps its track row, and with
it its likes and play history, by being matched to the missing row it
replaces (#2528). Untagged files were matched on (file_size,
duration_ms), which was never a fingerprint. It could pair two
unrelated files that happened to share a byte count and a duration,
and it missed a file retagged in place, whose size changes. The only
defence was requiring a unique match and otherwise giving up.

Now there is a real identity. FindMissingTrackByAudioHash matches a
missing track by the SHA-256 of its encoded audio (track_fingerprints,
#3906). That survives a rename, a move and a retag, and only an
identical recording can match it. adoptMovedTrack takes the new file's
hash, which the scan already computes before adoption. The size and
duration query and fallback are removed outright, with no second path
(rule 22).

Unchanged:
- MBID first: it identifies the recording and survives a re-encode
  that even the hash does not
- a unique match is still required
- an absent hash is never looked up, so unhashable files cannot pair
  with each other

The test fake answers the hash lookup only for the hash it holds, so
the tests can tell adoption by identity apart from adoption by
coincidence. That includes the case the old pair got wrong: different
audio of equal size and duration is not adopted.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
This commit is contained in:
2026-09-11 17:31:42 -04:00
co-authored by Claude Opus 5
parent 11ef044ef6
commit c8bf9dc929
6 changed files with 143 additions and 136 deletions
+16 -19
View File
@@ -148,39 +148,36 @@ func (q *Queries) DeleteTrack(ctx context.Context, id pgtype.UUID) (DeleteTrackR
return i, err return i, err
} }
const findMissingTrackByFingerprint = `-- name: FindMissingTrackByFingerprint :many const findMissingTrackByAudioHash = `-- name: FindMissingTrackByAudioHash :many
SELECT id, file_path FROM tracks SELECT t.id, t.file_path
WHERE missing_since IS NOT NULL FROM tracks t
AND file_size = $1 JOIN track_fingerprints f ON f.track_id = t.id
AND duration_ms = $2 WHERE t.missing_since IS NOT NULL
AND f.audio_stream_sha256 = $1
LIMIT 2 LIMIT 2
` `
type FindMissingTrackByFingerprintParams struct { type FindMissingTrackByAudioHashRow struct {
FileSize int64
DurationMs int32
}
type FindMissingTrackByFingerprintRow struct {
ID pgtype.UUID ID pgtype.UUID
FilePath string FilePath string
} }
// Move detection fallback for files with no MBID (#2528). Exact byte size AND // Move detection fallback for files with no MBID (#2528, #3914). The audio stream
// exact decoded duration is a strong pair: a plain move or rename preserves // hash identifies the encoded audio itself, so it survives a rename, a move and a
// both, while a re-encode changes at least one — and a re-encode genuinely is a // retag — anything short of a re-encode. It replaced (file_size, duration_ms),
// different file, so failing to match there is correct rather than a gap. // which could pair two unrelated files that happened to share a byte count and a
// duration, and missed a file retagged in place, whose size changes.
// //
// Same missing-only constraint and same LIMIT 2 rationale as the MBID variant. // Same missing-only constraint and same LIMIT 2 rationale as the MBID variant.
func (q *Queries) FindMissingTrackByFingerprint(ctx context.Context, arg FindMissingTrackByFingerprintParams) ([]FindMissingTrackByFingerprintRow, error) { func (q *Queries) FindMissingTrackByAudioHash(ctx context.Context, audioStreamSha256 []byte) ([]FindMissingTrackByAudioHashRow, error) {
rows, err := q.db.Query(ctx, findMissingTrackByFingerprint, arg.FileSize, arg.DurationMs) rows, err := q.db.Query(ctx, findMissingTrackByAudioHash, audioStreamSha256)
if err != nil { if err != nil {
return nil, err return nil, err
} }
defer rows.Close() defer rows.Close()
var items []FindMissingTrackByFingerprintRow var items []FindMissingTrackByAudioHashRow
for rows.Next() { for rows.Next() {
var i FindMissingTrackByFingerprintRow var i FindMissingTrackByAudioHashRow
if err := rows.Scan(&i.ID, &i.FilePath); err != nil { if err := rows.Scan(&i.ID, &i.FilePath); err != nil {
return nil, err return nil, err
} }
+11 -9
View File
@@ -155,17 +155,19 @@ SELECT id, file_path FROM tracks
AND mbid = sqlc.arg(mbid)::text AND mbid = sqlc.arg(mbid)::text
LIMIT 2; LIMIT 2;
-- name: FindMissingTrackByFingerprint :many -- name: FindMissingTrackByAudioHash :many
-- Move detection fallback for files with no MBID (#2528). Exact byte size AND -- Move detection fallback for files with no MBID (#2528, #3914). The audio stream
-- exact decoded duration is a strong pair: a plain move or rename preserves -- hash identifies the encoded audio itself, so it survives a rename, a move and a
-- both, while a re-encode changes at least one — and a re-encode genuinely is a -- retag — anything short of a re-encode. It replaced (file_size, duration_ms),
-- different file, so failing to match there is correct rather than a gap. -- which could pair two unrelated files that happened to share a byte count and a
-- duration, and missed a file retagged in place, whose size changes.
-- --
-- Same missing-only constraint and same LIMIT 2 rationale as the MBID variant. -- Same missing-only constraint and same LIMIT 2 rationale as the MBID variant.
SELECT id, file_path FROM tracks SELECT t.id, t.file_path
WHERE missing_since IS NOT NULL FROM tracks t
AND file_size = sqlc.arg(file_size) JOIN track_fingerprints f ON f.track_id = t.id
AND duration_ms = sqlc.arg(duration_ms) WHERE t.missing_since IS NOT NULL
AND f.audio_stream_sha256 = sqlc.arg(audio_stream_sha256)
LIMIT 2; LIMIT 2;
-- name: AdoptTrackPath :execrows -- name: AdoptTrackPath :execrows
+13 -15
View File
@@ -41,7 +41,7 @@ import (
// match/ambiguity logic can be tested against a fake. // match/ambiguity logic can be tested against a fake.
type trackAdopter interface { type trackAdopter interface {
FindMissingTrackByMbid(ctx context.Context, mbid string) ([]dbq.FindMissingTrackByMbidRow, error) FindMissingTrackByMbid(ctx context.Context, mbid string) ([]dbq.FindMissingTrackByMbidRow, error)
FindMissingTrackByFingerprint(ctx context.Context, arg dbq.FindMissingTrackByFingerprintParams) ([]dbq.FindMissingTrackByFingerprintRow, error) FindMissingTrackByAudioHash(ctx context.Context, audioStreamSha256 []byte) ([]dbq.FindMissingTrackByAudioHashRow, error)
AdoptTrackPath(ctx context.Context, arg dbq.AdoptTrackPathParams) (int64, error) AdoptTrackPath(ctx context.Context, arg dbq.AdoptTrackPathParams) (int64, error)
} }
@@ -53,10 +53,10 @@ type trackAdopter interface {
// pre-#2528 behaviour. // pre-#2528 behaviour.
func (s *Scanner) adoptMovedTrack( func (s *Scanner) adoptMovedTrack(
ctx context.Context, q trackAdopter, newPath string, ctx context.Context, q trackAdopter, newPath string,
fileSize int64, durationMs int32, recordingMBID string, audioHash []byte, recordingMBID string,
) bool { ) bool {
// MBID first. It identifies the recording rather than the bytes, so it // MBID first. It identifies the recording rather than the bytes, so it
// survives a re-encode that the fingerprint cannot. // survives a re-encode that the audio hash cannot.
if recordingMBID != "" { if recordingMBID != "" {
rows, err := q.FindMissingTrackByMbid(ctx, recordingMBID) rows, err := q.FindMissingTrackByMbid(ctx, recordingMBID)
if err != nil { if err != nil {
@@ -67,19 +67,17 @@ func (s *Scanner) adoptMovedTrack(
} }
} }
// Fingerprint fallback for untagged files. Both components must be real: // Audio-hash fallback for untagged files (#3914): the encoded audio itself,
// duration_ms is 0 when ffprobe failed, and matching 0 against 0 would pair // which a rename, a move or a retag leaves unchanged. Absent when the hash
// up unrelated broken files. // could not be taken, and then nothing is matched — an empty hash must never
if fileSize > 0 && durationMs > 0 { // be looked up, or every unhashable file would pair with every other.
rows, err := q.FindMissingTrackByFingerprint(ctx, dbq.FindMissingTrackByFingerprintParams{ if len(audioHash) > 0 {
FileSize: fileSize, rows, err := q.FindMissingTrackByAudioHash(ctx, audioHash)
DurationMs: durationMs,
})
if err != nil { if err != nil {
s.logger.Warn("library scan: move lookup by fingerprint failed", s.logger.Warn("library scan: move lookup by audio hash failed",
"path", newPath, "err", err) "path", newPath, "err", err)
} else if c, ok := s.uniqueMatch(rowsFromFingerprint(rows), newPath, "fingerprint"); ok { } else if c, ok := s.uniqueMatch(rowsFromAudioHash(rows), newPath, "audio_hash"); ok {
return s.adopt(ctx, q, c, newPath, "fingerprint") return s.adopt(ctx, q, c, newPath, "audio_hash")
} }
} }
@@ -100,7 +98,7 @@ func rowsFromMbid(rows []dbq.FindMissingTrackByMbidRow) []candidate {
return out return out
} }
func rowsFromFingerprint(rows []dbq.FindMissingTrackByFingerprintRow) []candidate { func rowsFromAudioHash(rows []dbq.FindMissingTrackByAudioHashRow) []candidate {
out := make([]candidate, 0, len(rows)) out := make([]candidate, 0, len(rows))
for _, r := range rows { for _, r := range rows {
out = append(out, candidate{id: r.ID, filePath: r.FilePath}) out = append(out, candidate{id: r.ID, filePath: r.FilePath})
+96 -86
View File
@@ -1,6 +1,7 @@
package library package library
import ( import (
"bytes"
"context" "context"
"errors" "errors"
"testing" "testing"
@@ -10,18 +11,21 @@ import (
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
) )
// fakeAdopter answers the audio-hash lookup only for the hash it holds, the way
// the real query does. A fake that returned its rows for any hash could not tell
// adoption by identity apart from adoption by coincidence.
type fakeAdopter struct { type fakeAdopter struct {
byMbid []dbq.FindMissingTrackByMbidRow byMbid []dbq.FindMissingTrackByMbidRow
byFingerprint []dbq.FindMissingTrackByFingerprintRow hash []byte
byHash []dbq.FindMissingTrackByAudioHashRow
mbidErr error
hashErr error
adoptErr error
adoptRows int64
mbidErr error mbidQueried []string
fingerprintErr error hashQueried [][]byte
adoptErr error adopted []dbq.AdoptTrackPathParams
adoptRows int64
mbidQueried []string
fingerprintQueried []dbq.FindMissingTrackByFingerprintParams
adopted []dbq.AdoptTrackPathParams
} }
func (f *fakeAdopter) FindMissingTrackByMbid(_ context.Context, mbid string) ([]dbq.FindMissingTrackByMbidRow, error) { func (f *fakeAdopter) FindMissingTrackByMbid(_ context.Context, mbid string) ([]dbq.FindMissingTrackByMbidRow, error) {
@@ -29,11 +33,17 @@ func (f *fakeAdopter) FindMissingTrackByMbid(_ context.Context, mbid string) ([]
return f.byMbid, f.mbidErr return f.byMbid, f.mbidErr
} }
func (f *fakeAdopter) FindMissingTrackByFingerprint( func (f *fakeAdopter) FindMissingTrackByAudioHash(
_ context.Context, arg dbq.FindMissingTrackByFingerprintParams, _ context.Context, audioStreamSha256 []byte,
) ([]dbq.FindMissingTrackByFingerprintRow, error) { ) ([]dbq.FindMissingTrackByAudioHashRow, error) {
f.fingerprintQueried = append(f.fingerprintQueried, arg) f.hashQueried = append(f.hashQueried, audioStreamSha256)
return f.byFingerprint, f.fingerprintErr if f.hashErr != nil {
return nil, f.hashErr
}
if !bytes.Equal(audioStreamSha256, f.hash) {
return nil, nil
}
return f.byHash, nil
} }
func (f *fakeAdopter) AdoptTrackPath(_ context.Context, arg dbq.AdoptTrackPathParams) (int64, error) { func (f *fakeAdopter) AdoptTrackPath(_ context.Context, arg dbq.AdoptTrackPathParams) (int64, error) {
@@ -51,10 +61,12 @@ func mbidRow(n byte, path string) dbq.FindMissingTrackByMbidRow {
return dbq.FindMissingTrackByMbidRow{ID: testUUID(n), FilePath: path} return dbq.FindMissingTrackByMbidRow{ID: testUUID(n), FilePath: path}
} }
func fpRow(n byte, path string) dbq.FindMissingTrackByFingerprintRow { func hashRow(n byte, path string) dbq.FindMissingTrackByAudioHashRow {
return dbq.FindMissingTrackByFingerprintRow{ID: testUUID(n), FilePath: path} return dbq.FindMissingTrackByAudioHashRow{ID: testUUID(n), FilePath: path}
} }
func audioHash(b byte) []byte { return bytes.Repeat([]byte{b}, 32) }
const ( const (
oldPath = "/music/Linkin Park/Minutes to Midnight/02 - Bleed It Out.mp3" oldPath = "/music/Linkin Park/Minutes to Midnight/02 - Bleed It Out.mp3"
newPath = "/music/Linkin Park/Minutes to Midnight/04 - Bleed It Out.mp3" newPath = "/music/Linkin Park/Minutes to Midnight/04 - Bleed It Out.mp3"
@@ -64,47 +76,53 @@ func TestAdoptMovedTrack_MatchesByMbid(t *testing.T) {
s := testScanner(t) s := testScanner(t)
q := &fakeAdopter{byMbid: []dbq.FindMissingTrackByMbidRow{mbidRow(7, oldPath)}, adoptRows: 1} q := &fakeAdopter{byMbid: []dbq.FindMissingTrackByMbidRow{mbidRow(7, oldPath)}, adoptRows: 1}
if !s.adoptMovedTrack(context.Background(), q, newPath, 5_000_000, 200_000, "rec-mbid") { if !s.adoptMovedTrack(context.Background(), q, newPath, audioHash(1), "rec-mbid") {
t.Fatal("expected the moved track to be adopted") t.Fatal("expected the moved track to be adopted")
} }
if len(q.adopted) != 1 { if len(q.adopted) != 1 || q.adopted[0].ID != testUUID(7) || q.adopted[0].FilePath != newPath {
t.Fatalf("adopted %d rows, want 1", len(q.adopted)) t.Fatalf("adopted = %+v, want row 7 at %q", q.adopted, newPath)
}
if q.adopted[0].ID != testUUID(7) {
t.Errorf("adopted the wrong row: %v", q.adopted[0].ID)
}
if q.adopted[0].FilePath != newPath {
t.Errorf("adopted FilePath = %q, want %q", q.adopted[0].FilePath, newPath)
} }
// MBID matched, so the weaker signal should not have been consulted. // MBID matched, so the weaker signal should not have been consulted.
if len(q.fingerprintQueried) != 0 { if len(q.hashQueried) != 0 {
t.Errorf("queried the fingerprint despite an MBID match") t.Errorf("queried the audio hash despite an MBID match")
} }
} }
func TestAdoptMovedTrack_FallsBackToFingerprint(t *testing.T) { func TestAdoptMovedTrack_FallsBackToAudioHash(t *testing.T) {
s := testScanner(t) s := testScanner(t)
q := &fakeAdopter{byFingerprint: []dbq.FindMissingTrackByFingerprintRow{fpRow(3, oldPath)}, adoptRows: 1} q := &fakeAdopter{hash: audioHash(3), byHash: []dbq.FindMissingTrackByAudioHashRow{hashRow(3, oldPath)}, adoptRows: 1}
// No MBID: an untagged file, which is exactly what the fallback is for. // No MBID: an untagged file, which is exactly what the fallback is for.
if !s.adoptMovedTrack(context.Background(), q, newPath, 4_200_000, 187_000, "") { if !s.adoptMovedTrack(context.Background(), q, newPath, audioHash(3), "") {
t.Fatal("expected adoption via fingerprint") t.Fatal("expected adoption via the audio hash")
} }
if len(q.mbidQueried) != 0 { if len(q.mbidQueried) != 0 {
t.Errorf("queried by MBID with no MBID available") t.Errorf("queried by MBID with no MBID available")
} }
if len(q.fingerprintQueried) != 1 { if len(q.hashQueried) != 1 || !bytes.Equal(q.hashQueried[0], audioHash(3)) {
t.Fatalf("fingerprint queried %d times, want 1", len(q.fingerprintQueried)) t.Fatalf("hash queried = %x, want exactly the file's hash", q.hashQueried)
}
got := q.fingerprintQueried[0]
if got.FileSize != 4_200_000 || got.DurationMs != 187_000 {
t.Errorf("fingerprint = %+v, want size 4200000 duration 187000", got)
} }
if len(q.adopted) != 1 || q.adopted[0].ID != testUUID(3) { if len(q.adopted) != 1 || q.adopted[0].ID != testUUID(3) {
t.Errorf("adopted = %+v, want row 3", q.adopted) t.Errorf("adopted = %+v, want row 3", q.adopted)
} }
} }
// The case the old (file_size, duration_ms) pair got wrong: an unrelated file
// that happens to share a size and a duration with a missing track. Size and
// duration are no longer inputs at all; only the audio itself can match, and a
// different recording has a different hash.
func TestAdoptMovedTrack_DifferentAudioIsNotAdopted(t *testing.T) {
s := testScanner(t)
q := &fakeAdopter{hash: audioHash(0xAA), byHash: []dbq.FindMissingTrackByAudioHashRow{hashRow(1, oldPath)}, adoptRows: 1}
if s.adoptMovedTrack(context.Background(), q, newPath, audioHash(0xBB), "") {
t.Fatal("adopted a missing track whose audio differs")
}
if len(q.adopted) != 0 {
t.Errorf("adopted = %+v, want none", q.adopted)
}
}
// Two missing rows carrying the same recording MBID means real duplicates. // Two missing rows carrying the same recording MBID means real duplicates.
// Adopting one arbitrarily would attach this file's future history to a coin // Adopting one arbitrarily would attach this file's future history to a coin
// flip, so it must insert fresh instead. // flip, so it must insert fresh instead.
@@ -115,7 +133,7 @@ func TestAdoptMovedTrack_RefusesAmbiguousMbidMatch(t *testing.T) {
mbidRow(2, "/music/b.mp3"), mbidRow(2, "/music/b.mp3"),
}, adoptRows: 1} }, adoptRows: 1}
if s.adoptMovedTrack(context.Background(), q, newPath, 0, 0, "rec-mbid") { if s.adoptMovedTrack(context.Background(), q, newPath, nil, "rec-mbid") {
t.Fatal("expected refusal on an ambiguous MBID match") t.Fatal("expected refusal on an ambiguous MBID match")
} }
if len(q.adopted) != 0 { if len(q.adopted) != 0 {
@@ -123,66 +141,57 @@ func TestAdoptMovedTrack_RefusesAmbiguousMbidMatch(t *testing.T) {
} }
} }
// An ambiguous MBID may still be resolvable by the fingerprint, which is a // An ambiguous MBID may still be resolvable by the audio hash, which is a
// narrower signal — so falling through is allowed to succeed. // narrower signal — so falling through is allowed to succeed.
func TestAdoptMovedTrack_AmbiguousMbidFallsThroughToFingerprint(t *testing.T) { func TestAdoptMovedTrack_AmbiguousMbidFallsThroughToAudioHash(t *testing.T) {
s := testScanner(t) s := testScanner(t)
q := &fakeAdopter{ q := &fakeAdopter{
byMbid: []dbq.FindMissingTrackByMbidRow{ byMbid: []dbq.FindMissingTrackByMbidRow{
mbidRow(1, "/music/a.mp3"), mbidRow(1, "/music/a.mp3"),
mbidRow(2, "/music/b.mp3"), mbidRow(2, "/music/b.mp3"),
}, },
byFingerprint: []dbq.FindMissingTrackByFingerprintRow{fpRow(2, "/music/b.mp3")}, hash: audioHash(2),
adoptRows: 1, byHash: []dbq.FindMissingTrackByAudioHashRow{hashRow(2, "/music/b.mp3")},
adoptRows: 1,
} }
if !s.adoptMovedTrack(context.Background(), q, newPath, 1000, 2000, "rec-mbid") { if !s.adoptMovedTrack(context.Background(), q, newPath, audioHash(2), "rec-mbid") {
t.Fatal("expected the fingerprint to disambiguate") t.Fatal("expected the audio hash to disambiguate")
} }
if len(q.adopted) != 1 || q.adopted[0].ID != testUUID(2) { if len(q.adopted) != 1 || q.adopted[0].ID != testUUID(2) {
t.Errorf("adopted = %+v, want row 2", q.adopted) t.Errorf("adopted = %+v, want row 2", q.adopted)
} }
} }
func TestAdoptMovedTrack_RefusesAmbiguousFingerprintMatch(t *testing.T) { // Two missing tracks with identical audio are duplicates of each other; a new
// file matching both cannot be assigned to either.
func TestAdoptMovedTrack_RefusesAmbiguousAudioHashMatch(t *testing.T) {
s := testScanner(t) s := testScanner(t)
q := &fakeAdopter{byFingerprint: []dbq.FindMissingTrackByFingerprintRow{ q := &fakeAdopter{hash: audioHash(5), byHash: []dbq.FindMissingTrackByAudioHashRow{
fpRow(1, "/music/a.mp3"), hashRow(1, "/music/a.mp3"),
fpRow(2, "/music/b.mp3"), hashRow(2, "/music/b.mp3"),
}, adoptRows: 1} }, adoptRows: 1}
if s.adoptMovedTrack(context.Background(), q, newPath, 1000, 2000, "") { if s.adoptMovedTrack(context.Background(), q, newPath, audioHash(5), "") {
t.Fatal("expected refusal on an ambiguous fingerprint match") t.Fatal("expected refusal on an ambiguous audio-hash match")
} }
if len(q.adopted) != 0 { if len(q.adopted) != 0 {
t.Errorf("adopted despite ambiguity: %+v", q.adopted) t.Errorf("adopted despite ambiguity: %+v", q.adopted)
} }
} }
// duration_ms is 0 when ffprobe failed. Matching 0 against 0 would pair up // No hash means it could not be taken. An empty hash must never be looked up:
// unrelated broken files, so the fingerprint must not be attempted. // every unhashable file would pair with every other.
func TestAdoptMovedTrack_SkipsFingerprintWithoutRealValues(t *testing.T) { func TestAdoptMovedTrack_SkipsAudioHashWhenAbsent(t *testing.T) {
tests := []struct { for name, hash := range map[string][]byte{"nil": nil, "empty": {}} {
name string t.Run(name, func(t *testing.T) {
size int64
duration int32
}{
{"no duration", 1000, 0},
{"no size", 0, 2000},
{"neither", 0, 0},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
s := testScanner(t) s := testScanner(t)
q := &fakeAdopter{ q := &fakeAdopter{hash: hash, byHash: []dbq.FindMissingTrackByAudioHashRow{hashRow(1, oldPath)}, adoptRows: 1}
byFingerprint: []dbq.FindMissingTrackByFingerprintRow{fpRow(1, oldPath)}, if s.adoptMovedTrack(context.Background(), q, newPath, hash, "") {
adoptRows: 1, t.Error("adopted without an audio hash")
} }
if s.adoptMovedTrack(context.Background(), q, newPath, tc.size, tc.duration, "") { if len(q.hashQueried) != 0 {
t.Error("adopted on an unusable fingerprint") t.Error("looked up an absent audio hash")
}
if len(q.fingerprintQueried) != 0 {
t.Error("queried the fingerprint with unusable values")
} }
}) })
} }
@@ -192,7 +201,7 @@ func TestAdoptMovedTrack_NoCandidates(t *testing.T) {
s := testScanner(t) s := testScanner(t)
q := &fakeAdopter{adoptRows: 1} q := &fakeAdopter{adoptRows: 1}
if s.adoptMovedTrack(context.Background(), q, newPath, 1000, 2000, "rec-mbid") { if s.adoptMovedTrack(context.Background(), q, newPath, audioHash(9), "rec-mbid") {
t.Fatal("expected no adoption when nothing matches") t.Fatal("expected no adoption when nothing matches")
} }
if len(q.adopted) != 0 { if len(q.adopted) != 0 {
@@ -209,7 +218,7 @@ func TestAdoptMovedTrack_LostRaceReportsNotAdopted(t *testing.T) {
adoptRows: 0, adoptRows: 0,
} }
if s.adoptMovedTrack(context.Background(), q, newPath, 1000, 2000, "rec-mbid") { if s.adoptMovedTrack(context.Background(), q, newPath, audioHash(5), "rec-mbid") {
t.Fatal("expected not-adopted when the update matched no rows") t.Fatal("expected not-adopted when the update matched no rows")
} }
} }
@@ -223,7 +232,7 @@ func TestAdoptMovedTrack_ToleratesQueryErrors(t *testing.T) {
q *fakeAdopter q *fakeAdopter
}{ }{
{"mbid lookup fails", &fakeAdopter{mbidErr: sentinel}}, {"mbid lookup fails", &fakeAdopter{mbidErr: sentinel}},
{"fingerprint lookup fails", &fakeAdopter{fingerprintErr: sentinel}}, {"audio hash lookup fails", &fakeAdopter{hashErr: sentinel}},
{"adopt fails", &fakeAdopter{ {"adopt fails", &fakeAdopter{
byMbid: []dbq.FindMissingTrackByMbidRow{mbidRow(1, oldPath)}, byMbid: []dbq.FindMissingTrackByMbidRow{mbidRow(1, oldPath)},
adoptErr: sentinel, adoptErr: sentinel,
@@ -232,24 +241,25 @@ func TestAdoptMovedTrack_ToleratesQueryErrors(t *testing.T) {
for _, tc := range tests { for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) { t.Run(tc.name, func(t *testing.T) {
s := testScanner(t) s := testScanner(t)
if s.adoptMovedTrack(context.Background(), tc.q, newPath, 1000, 2000, "rec-mbid") { if s.adoptMovedTrack(context.Background(), tc.q, newPath, audioHash(1), "rec-mbid") {
t.Error("reported adoption despite a query error") t.Error("reported adoption despite a query error")
} }
}) })
} }
} }
// A failed MBID lookup must not stop the fingerprint from being tried. // A failed MBID lookup must not stop the audio hash from being tried.
func TestAdoptMovedTrack_MbidErrorStillTriesFingerprint(t *testing.T) { func TestAdoptMovedTrack_MbidErrorStillTriesAudioHash(t *testing.T) {
s := testScanner(t) s := testScanner(t)
q := &fakeAdopter{ q := &fakeAdopter{
mbidErr: errors.New("db hiccup"), mbidErr: errors.New("db hiccup"),
byFingerprint: []dbq.FindMissingTrackByFingerprintRow{fpRow(9, oldPath)}, hash: audioHash(9),
adoptRows: 1, byHash: []dbq.FindMissingTrackByAudioHashRow{hashRow(9, oldPath)},
adoptRows: 1,
} }
if !s.adoptMovedTrack(context.Background(), q, newPath, 1000, 2000, "rec-mbid") { if !s.adoptMovedTrack(context.Background(), q, newPath, audioHash(9), "rec-mbid") {
t.Fatal("expected the fingerprint to be tried after an MBID lookup error") t.Fatal("expected the audio hash to be tried after an MBID lookup error")
} }
if len(q.adopted) != 1 || q.adopted[0].ID != testUUID(9) { if len(q.adopted) != 1 || q.adopted[0].ID != testUUID(9) {
t.Errorf("adopted = %+v, want row 9", q.adopted) t.Errorf("adopted = %+v, want row 9", q.adopted)
@@ -280,9 +290,9 @@ func TestRowConverters(t *testing.T) {
if len(got) != 2 || got[0].id != testUUID(1) || got[1].filePath != "/b" { if len(got) != 2 || got[0].id != testUUID(1) || got[1].filePath != "/b" {
t.Errorf("rowsFromMbid = %+v", got) t.Errorf("rowsFromMbid = %+v", got)
} }
got = rowsFromFingerprint([]dbq.FindMissingTrackByFingerprintRow{fpRow(3, "/c")}) got = rowsFromAudioHash([]dbq.FindMissingTrackByAudioHashRow{hashRow(3, "/c")})
if len(got) != 1 || got[0].id != testUUID(3) || got[0].filePath != "/c" { if len(got) != 1 || got[0].id != testUUID(3) || got[0].filePath != "/c" {
t.Errorf("rowsFromFingerprint = %+v", got) t.Errorf("rowsFromAudioHash = %+v", got)
} }
} }
+4 -4
View File
@@ -325,11 +325,11 @@ func (s *Scanner) scanFile(
// file_path and updates THAT row: same track id, likes and play history // file_path and updates THAT row: same track id, likes and play history
// intact. Without this, renumbering an album forks every track on it. // intact. Without this, renumbering an album forks every track on it.
// //
// Runs here rather than earlier because the fingerprint needs the probed // Runs after fingerprinting because, for a file with no MBID, adoption matches
// duration, and only for genuinely unknown paths — a known path is already // on its audio hash (#3914), and only for genuinely unknown paths — a known
// the row we're going to update. // path is already the row we're going to update.
if !knownTrack { if !knownTrack {
if s.adoptMovedTrack(ctx, q, path, info.Size(), durationMs, recordingMBID) { if s.adoptMovedTrack(ctx, q, path, fp.streamSHA256, recordingMBID) {
// Count it as an update: the row existed, and reporting it as Added // Count it as an update: the row existed, and reporting it as Added
// would overstate library growth on every reorganisation. // would overstate library growth on every reorganisation.
knownTrack = true knownTrack = true
+3 -3
View File
@@ -211,9 +211,9 @@ func writeTestMP3(t *testing.T, path string, frames map[string]string) {
// playlist memberships travel with it — rather than forking into a marked ghost // playlist memberships travel with it — rather than forking into a marked ghost
// plus a fresh zero-history row. // plus a fresh zero-history row.
// //
// Uses the MBID path. The synthetic MP3s here carry no real audio, so ffprobe // Uses the MBID path. The synthetic MP3s here carry no real audio, so their audio
// yields duration 0 and the size+duration fingerprint is deliberately unusable — // stream hash cannot be relied on — which is why the recording MBID is the signal
// which is why the recording MBID is the signal under test. // under test. The audio-hash path is covered by moved_test.go.
// //
// Eight tracks with one rename keeps the marked fraction at 12.5%, under // Eight tracks with one rename keeps the marked fraction at 12.5%, under
// missingMarkMaxFraction. That is load-bearing: if the rename exceeded the cap, // missingMarkMaxFraction. That is load-bearing: if the rename exceeded the cap,