diff --git a/backend/download/pathmatch.go b/backend/download/pathmatch.go index a9149e6..5348c4d 100644 --- a/backend/download/pathmatch.go +++ b/backend/download/pathmatch.go @@ -258,21 +258,72 @@ func AnnotateFiles(files []CandidateFile) []CandidateFile { // matchFiles aligns a candidate's audio files to the expected tracklist // and returns the per-file assignment plus the mean title similarity of -// the aligned pairs. +// the aligned pairs. alignFiles is the same alignment with the +// duration evidence as well. +func matchFiles( + files []CandidateFile, + expected []ExpectedTrack, +) ([]CandidateFile, float64) { + a := alignFiles(files, expected) + + return a.files, a.titleFit +} + +// alignment is what aligning a candidate to a tracklist found. +type alignment struct { + files []CandidateFile + + // titleFit is the mean title similarity over aligned pairs. + titleFit float64 + + // durationFit is the mean duration agreement over aligned pairs + // where both sides state a length, and timedPairs is how many such + // pairs there were. + durationFit float64 + timedPairs int + aligned int +} + +// durationAgreement scores how well a file's length matches the +// expected track's, in 0..1. Rips of the same master differ by a +// second or two of silence; a different edit, a live take or a +// truncated file differs by tens of seconds. +func durationAgreement(got, want int64) float64 { + const ( + exactMillis = 3_000 + wrongMillis = 30_000 + ) + + d := got - want + if d < 0 { + d = -d + } + + switch { + case d <= exactMillis: + return 1 + case d >= wrongMillis: + return 0 + default: + return 1 - float64(d-exactMillis)/float64(wrongMillis-exactMillis) + } +} + +// alignFiles aligns a candidate's audio files to the expected tracklist. // // Alignment is greedy by score rather than optimal: candidate folders // are small (a few dozen files at most) and the common cases — correct // track numbers, or clean "NN Title" names — are unambiguous, so the // extra machinery of Hungarian assignment buys nothing here. -func matchFiles( +func alignFiles( files []CandidateFile, expected []ExpectedTrack, -) ([]CandidateFile, float64) { +) alignment { annotated := make([]CandidateFile, len(files)) copy(annotated, files) if len(expected) == 0 { - return annotated, 0 + return alignment{files: annotated} } hints := make([]TrackHint, len(annotated)) @@ -285,8 +336,19 @@ func matchFiles( var ( total float64 matched int + + durTotal float64 + timed int ) + // timing adds a pair's duration evidence when both sides state one. + timing := func(f CandidateFile, e ExpectedTrack) { + if f.LengthMillis > 0 && e.LengthMillis > 0 { + durTotal += durationAgreement(f.LengthMillis, e.LengthMillis) + timed++ + } + } + // Pass 1: trust explicit track numbers when they are unique and in // range. A folder that numbers its files correctly is the strong // case, and title comparison only adds noise there. @@ -305,6 +367,8 @@ func matchFiles( total += autotag.TitleSimilarity(hints[i].Title, expected[idx].Title) matched++ + + timing(annotated[i], expected[idx]) } // Pass 2: title similarity for whatever is left. @@ -339,13 +403,26 @@ func matchFiles( total += bestSim matched++ + + timing(annotated[i], expected[bestIdx]) } if matched == 0 { - return annotated, 0 + return alignment{files: annotated} } - return annotated, total / float64(matched) + a := alignment{ + files: annotated, + titleFit: total / float64(matched), + timedPairs: timed, + aligned: matched, + } + + if timed > 0 { + a.durationFit = durTotal / float64(timed) + } + + return a } // indexForPosition finds the expected track at a disc/track position. diff --git a/backend/download/provider_slskd.go b/backend/download/provider_slskd.go index ae7f5f4..1286c72 100644 --- a/backend/download/provider_slskd.go +++ b/backend/download/provider_slskd.go @@ -9,6 +9,7 @@ import ( "os" "path" "path/filepath" + "regexp" "strings" "time" @@ -78,6 +79,9 @@ const ( // slskdHTTPTimeout bounds one API call. slskdHTTPTimeout = 20 * time.Second + // millisPerSecond converts slskd's whole-second file lengths. + millisPerSecond = 1000 + // slskdStallAfter is how long a grab may go without a byte arriving // before the peer is given up on. It is measured from enqueue, so // it covers a peer that queues us and never starts as well as one @@ -255,14 +259,13 @@ type slskdSearch struct { // slskdResponse is one peer's answer to a search. type slskdResponse struct { - Username string `json:"username"` - HasFreeUploadSlot bool `json:"hasFreeUploadSlot"` - QueueLength int `json:"queueLength"` - UploadSpeed int64 `json:"uploadSpeed"` - Files []slskdFile `json:"files"` - LockedFileCount int `json:"lockedFileCount"` - FileCount int `json:"fileCount"` - FreeUploadSlotFlag bool `json:"freeUploadSlots"` + Username string `json:"username"` + HasFreeUploadSlot bool `json:"hasFreeUploadSlot"` + QueueLength int `json:"queueLength"` + UploadSpeed int64 `json:"uploadSpeed"` + Files []slskdFile `json:"files"` + LockedFileCount int `json:"lockedFileCount"` + FileCount int `json:"fileCount"` } // slskdFile is one file a peer is offering. @@ -270,7 +273,9 @@ type slskdFile struct { Filename string `json:"filename"` Size int64 `json:"size"` BitRate int `json:"bitRate"` - Length int `json:"length"` + + // Length is the duration in whole seconds. + Length int `json:"length"` } // slskdTransfer is one download's state. @@ -302,24 +307,89 @@ func (t slskdTransfer) done() (finished, ok bool) { // per-folder candidates. A folder from one peer is the unit a user // actually wants: Soulseek has no album concept, but people organise // their shares by album directory. +// +// Up to two queries run at once — the request as written and a +// normalised form of it (see slskdQueries) — and their candidates are +// merged. They run concurrently rather than as a fallback because the +// manager gives a provider one search budget, and a Soulseek search +// spends most of it waiting for peers to answer; a second query after +// the first would not fit. func (s *slskd) Search(ctx context.Context, dl Download) ([]Candidate, error) { + queries := slskdQueries(dl) + if len(queries) == 0 { + return nil, nil + } + + type found struct { + candidates []Candidate + err error + } + + results := make(chan found, len(queries)) + + for _, q := range queries { + go func(q string) { + c, err := s.searchOnce(ctx, q, minFilesFor(dl)) + results <- found{candidates: c, err: err} + }(q) + } + + var ( + out []Candidate + seen = map[string]bool{} + firstErr error + answered int + ) + + for range queries { + r := <-results + if r.err != nil { + s.logger.Debug("slskd search failed", "error", r.err) + + if firstErr == nil { + firstErr = r.err + } + + continue + } + + answered++ + + // The same peer's folder turns up under both queries; the ID is + // peer and folder, so it is the same candidate. + for _, c := range r.candidates { + if seen[c.ID] { + continue + } + + seen[c.ID] = true + + out = append(out, c) + } + } + + if answered == 0 { + return nil, firstErr + } + + return out, nil +} + +// searchOnce runs one query to completion and returns its candidates. +func (s *slskd) searchOnce( + ctx context.Context, + text string, + minFiles int, +) ([]Candidate, error) { // slskd's search endpoint deserializes id as a .NET Guid server-side, // so it must be a dashed UUID — the app's own newID() (a plain hex // string, used for request/item IDs elsewhere) is rejected with an // HTTP 400 before any search happens. searchID := uuid.NewString() - body := map[string]any{ - "id": searchID, - "searchText": dl.SearchText(), - } - - if err := s.client.post(ctx, "/api/v0/searches", body, nil); err != nil { - return nil, err - } - - search, err := s.awaitSearch(ctx, searchID) - if err != nil { + if err := s.client.post( + ctx, "/api/v0/searches", s.searchRequest(searchID, text, minFiles), nil, + ); err != nil { return nil, err } @@ -331,7 +401,123 @@ func (s *slskd) Search(ctx context.Context, dl Download) ([]Candidate, error) { ) }() - return s.candidatesFrom(search, minFilesFor(dl)), nil + if err := s.awaitSearch(ctx, searchID); err != nil { + return nil, err + } + + responses, err := s.searchResponses(ctx, searchID) + if err != nil { + return nil, err + } + + return s.candidatesFrom(responses, minFiles), nil +} + +// searchRequest is the body that starts a search. +// +// Every option is stated rather than left to the daemon, because +// slskd's defaults are its own and not ours. Its search timeout in +// particular has to finish inside our wait: a search that slskd is still +// running when we stop polling is results we asked for and discarded. +// The response and file limits are raised well above what a popular +// album produces, and the peer filters let slskd drop answers this +// provider would only score down to nothing — a folder too small to be +// a candidate, a peer with a queue it will not reach today. +func (s *slskd) searchRequest(id, text string, minFiles int) map[string]any { + const ( + responseLimit = 500 + fileLimit = 20_000 + maximumPeerQueueLength = 100 + ) + + // A tenth of the wait is left for the last poll and the responses + // fetch. + timeout := s.searchWait - s.searchWait/10 + + return map[string]any{ + "id": id, + "searchText": text, + "searchTimeout": timeout.Milliseconds(), + "responseLimit": responseLimit, + "fileLimit": fileLimit, + "filterResponses": true, + "minimumResponseFileCount": minFiles, + "maximumPeerQueueLength": maximumPeerQueueLength, + } +} + +// slskdQueries is what is searched for a request: the request's own +// search text, and a normalised form of it when that differs. +// +// Soulseek matches every term against the file's full path, so each +// extra word is a filter, and some words filter wrongly: +// +// - edition qualifiers — "(Deluxe Edition)", "[2011 Remaster]" — are +// in the catalog's title and rarely in anyone's folder name; +// - punctuation splits a term oddly, and a term that starts with "-" +// is an *exclusion*, so an album called "-ism" searches for +// everything without it; +// - "Various Artists" is in no one's path for a compilation. +// +// A query the user typed is theirs and is searched exactly as written. +func slskdQueries(dl Download) []string { + primary := strings.TrimSpace(dl.SearchText()) + if primary == "" { + return nil + } + + out := []string{primary} + + if dl.Query != "" { + return out + } + + artist := dl.Artist + if isVariousArtists(artist) { + artist = "" + } + + normal := Download{ + Artist: normalizeSearchTerms(artist), + Album: normalizeSearchTerms(editionPattern.ReplaceAllString(dl.Album, " ")), + } + + if alt := strings.TrimSpace(normal.SearchText()); alt != "" && + !strings.EqualFold(alt, primary) { + out = append(out, alt) + } + + return out +} + +var ( + // editionPattern finds an edition qualifier: a bracketed group that + // names an edition, or a trailing " - 2011 Remaster". + editionPattern = regexp.MustCompile( + `(?i)\s*[(\[][^)\]]*\b(?:deluxe|edition|remaster(?:ed)?|expanded|` + + `anniversary|bonus|explicit|reissue|special|collector'?s?|` + + `version|mono|stereo)\b[^)\]]*[)\]]` + + `|\s+-\s+(?:\d{4}\s+)?remaster(?:ed)?\b.*$`, + ) + + // nonWordPattern is everything that is not a letter or a digit. + nonWordPattern = regexp.MustCompile(`[^\p{L}\p{N}]+`) +) + +// normalizeSearchTerms reduces text to plain words. +func normalizeSearchTerms(s string) string { + return strings.Join(strings.Fields(nonWordPattern.ReplaceAllString(s, " ")), " ") +} + +// isVariousArtists reports whether an artist credit is a compilation's +// placeholder rather than an artist. +func isVariousArtists(artist string) bool { + switch strings.ToLower(strings.TrimSpace(artist)) { + case "various artists", "various", "va": + return true + default: + return false + } } // minFilesFor is the fewest audio files a folder must offer to be a @@ -354,47 +540,83 @@ func minFilesFor(dl Download) int { // awaitSearch polls until the search completes or the budget runs out. // A timeout is not an error: partial Soulseek results are normal and // often good enough. -func (s *slskd) awaitSearch( - ctx context.Context, - searchID string, -) (slskdSearch, error) { +// +// The poll asks for the search's state only. It used to ask for every +// response on every one-second tick, which for a popular album is the +// same few thousand file entries serialised twenty times to be read +// once; searchResponses fetches them once at the end. +func (s *slskd) awaitSearch(ctx context.Context, searchID string) error { deadline := time.Now().Add(s.searchWait) - var last slskdSearch - for time.Now().Before(deadline) { select { case <-ctx.Done(): - return last, fmt.Errorf("%w: search cancelled", ErrSlskdTimeout) + return fmt.Errorf("%w: search cancelled", ErrSlskdTimeout) case <-time.After(s.searchPoll): } var search slskdSearch if err := s.client.get( - ctx, - "/api/v0/searches/"+searchID+"?includeResponses=true", - &search, + ctx, "/api/v0/searches/"+searchID, &search, ); err != nil { - return last, err + return err } - last = search - if search.IsComplete { - return search, nil + return nil } } - return last, nil + return nil +} + +// searchResponses fetches a search's responses once. +// +// `/searches/{id}/responses` is the endpoint for that; a daemon that +// does not answer it is asked the older way, with the search itself +// carrying its responses, so an older slskd degrades to the previous +// behaviour rather than to no results at all. +func (s *slskd) searchResponses( + ctx context.Context, + searchID string, +) ([]slskdResponse, error) { + var responses []slskdResponse + + err := s.client.get( + ctx, "/api/v0/searches/"+searchID+"/responses", &responses, + ) + if err == nil { + return responses, nil + } + + s.logger.Debug( + "slskd responses endpoint failed; asking with the search", + "error", err, + ) + + var search slskdSearch + + if err := s.client.get( + ctx, + "/api/v0/searches/"+searchID+"?includeResponses=true", + &search, + ); err != nil { + return nil, err + } + + return search.Responses, nil } // candidatesFrom groups a search's responses into candidates, dropping // folders with fewer than minFiles audio files. -func (s *slskd) candidatesFrom(search slskdSearch, minFiles int) []Candidate { - out := make([]Candidate, 0, len(search.Responses)) +func (s *slskd) candidatesFrom( + responses []slskdResponse, + minFiles int, +) []Candidate { + out := make([]Candidate, 0, len(responses)) - for _, resp := range search.Responses { + for _, resp := range responses { for folder, files := range groupByFolder(resp.Files) { audio := 0 @@ -414,6 +636,8 @@ func (s *slskd) candidatesFrom(search slskdSearch, minFiles int) []Candidate { Format: format, Bitrate: f.BitRate, IsAudio: isAudio, + + LengthMillis: int64(f.Length) * millisPerSecond, }) total += f.Size @@ -462,7 +686,7 @@ func groupByFolder(files []slskdFile) map[string][]slskdFile { func peerHealth(r slskdResponse) float64 { score := 0.35 - if r.HasFreeUploadSlot || r.FreeUploadSlotFlag { + if r.HasFreeUploadSlot { score += 0.4 } diff --git a/backend/download/provider_slskd_test.go b/backend/download/provider_slskd_test.go index de855a4..d06cec5 100644 --- a/backend/download/provider_slskd_test.go +++ b/backend/download/provider_slskd_test.go @@ -46,6 +46,15 @@ type slskdStub struct { // unauthorized makes every call return 401. unauthorized bool + + // searches records every search request body, and searchGets the + // request URI of every search GET. + searches []map[string]any + searchGets []string + + // noResponsesEndpoint makes /searches/{id}/responses 404, as an + // older daemon would. + noResponsesEndpoint bool } func newSlskdStub(t *testing.T) *slskdStub { @@ -67,6 +76,16 @@ func newSlskdStub(t *testing.T) *slskdStub { return } + var body map[string]any + + if err := json.NewDecoder(r.Body).Decode(&body); err != nil { + t.Errorf("decode search body: %v", err) + } + + s.mu.Lock() + s.searches = append(s.searches, body) + s.mu.Unlock() + w.WriteHeader(http.StatusCreated) }) @@ -83,8 +102,26 @@ func newSlskdStub(t *testing.T) *slskdStub { s.mu.Lock() responses := s.responses + noEndpoint := s.noResponsesEndpoint + s.searchGets = append(s.searchGets, r.URL.RequestURI()) s.mu.Unlock() + if strings.HasSuffix(r.URL.Path, "/responses") { + if noEndpoint { + w.WriteHeader(http.StatusNotFound) + + return + } + + writeJSON(t, w, responses) + + return + } + + if r.URL.Query().Get("includeResponses") != "true" { + responses = nil + } + writeJSON(t, w, slskdSearch{ ID: "search-1", IsComplete: true, diff --git a/backend/download/rank.go b/backend/download/rank.go index ff90f46..b50abb1 100644 --- a/backend/download/rank.go +++ b/backend/download/rank.go @@ -35,6 +35,15 @@ const ( weightArtistFit = 0.12 ) +// Match sub-weights when the candidate's durations are known. Duration +// takes its weight from title fit, the signal it corroborates: a title +// says which song a file claims to be, a length says whether it is that +// recording — the right edit, the whole file, not the live take. +const ( + timedWeightTitleFit = 0.25 + timedWeightDurationFit = 0.15 +) + // Quality sub-weights. Each set sums to 1.0. // // There are two of them because a stated preference changes what the @@ -319,13 +328,13 @@ func Score(dl Download, c Candidate, priority int, prefs AutoDownloadPrefs) Cand audio := c.AudioFiles() - matched, titleFit := matchFiles(audio, dl.Expected) + a := alignFiles(audio, dl.Expected) // Write the alignment back so the picker can show which file maps // to which track. - c.Files = mergeMatched(c.Files, matched) + c.Files = mergeMatched(c.Files, a.files) - c.Match = scoreMatch(dl, c, audio, titleFit) + c.Match = scoreMatch(dl, c, audio, a) c.Quality = scoreQuality( c, audio, priority, prefs, dl.runtimeMillis(), ) @@ -340,11 +349,16 @@ func scoreMatch( dl Download, c Candidate, audio []CandidateFile, - titleFit float64, + a alignment, ) MatchScore { m := MatchScore{ - Anchored: dl.Anchored(), - TitleFit: titleFit, + Anchored: dl.Anchored(), + TitleFit: a.titleFit, + DurationFit: a.durationFit, + + // Durations count once at least half the aligned pairs state + // one; a single timed pair would be a coin toss carrying 15%. + DurationKnown: a.timedPairs > 0 && a.timedPairs*2 >= a.aligned, } m.Completeness = completeness( @@ -369,9 +383,16 @@ func scoreMatch( // With no expected tracklist there is no title signal at all, so // redistribute its weight onto the album/artist evidence rather // than scoring every free-text result as half-wrong. - if len(dl.Expected) == 0 { + switch { + case len(dl.Expected) == 0: m.Overall = 0.55*m.AlbumFit + 0.45*m.ArtistFit - } else { + case m.DurationKnown: + m.Overall = timedWeightTitleFit*m.TitleFit + + timedWeightDurationFit*m.DurationFit + + weightCompleteness*m.Completeness + + weightAlbumFit*m.AlbumFit + + weightArtistFit*m.ArtistFit + default: m.Overall = weightTitleFit*m.TitleFit + weightCompleteness*m.Completeness + weightAlbumFit*m.AlbumFit + diff --git a/backend/download/search_recall_test.go b/backend/download/search_recall_test.go new file mode 100644 index 0000000..4973733 --- /dev/null +++ b/backend/download/search_recall_test.go @@ -0,0 +1,238 @@ +package download + +import ( + "context" + "strings" + "testing" +) + +// What Soulseek is asked, how, and what is kept from the answer (#271). + +func TestSlskdQueries(t *testing.T) { + t.Parallel() + + cases := []struct { + name string + dl Download + want []string + }{ + { + name: "a plain request is searched once", + dl: Download{Artist: "Radiohead", Album: "OK Computer"}, + want: []string{"Radiohead OK Computer"}, + }, + { + name: "an edition qualifier gets a second query without it", + dl: Download{Artist: "Radiohead", Album: "OK Computer (Collector's Edition)"}, + want: []string{ + "Radiohead OK Computer (Collector's Edition)", + "Radiohead OK Computer", + }, + }, + { + name: "a trailing remaster note", + dl: Download{Artist: "Pink Floyd", Album: "Animals - 2018 Remaster"}, + want: []string{ + "Pink Floyd Animals - 2018 Remaster", + "Pink Floyd Animals", + }, + }, + { + name: "a leading dash would be an exclusion", + dl: Download{Artist: "Mocky", Album: "-ism"}, + want: []string{"Mocky -ism", "Mocky ism"}, + }, + { + name: "a compilation is not searched by its placeholder artist", + dl: Download{Artist: "Various Artists", Album: "Pulp Fiction"}, + want: []string{"Various Artists Pulp Fiction", "Pulp Fiction"}, + }, + { + name: "what the user typed is searched as written", + dl: Download{Query: "ok computer (deluxe)", Album: "OK Computer (Deluxe)"}, + want: []string{"ok computer (deluxe)"}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + got := slskdQueries(tc.dl) + if strings.Join(got, "|") != strings.Join(tc.want, "|") { + t.Errorf("slskdQueries = %q, want %q", got, tc.want) + } + }) + } +} + +// Both queries run, the options are stated rather than left to the +// daemon's defaults, and a folder both queries found is one candidate. +func TestSlskdSearchRunsBothQueriesAndMerges(t *testing.T) { + t.Parallel() + + stub := newSlskdStub(t) + stub.responses = []slskdResponse{{ + Username: "peer", + Files: []slskdFile{ + {Filename: `\m\Radiohead - OK Computer\01 Airbag.flac`, Size: 1, Length: 284}, + {Filename: `\m\Radiohead - OK Computer\02 Paranoid Android.flac`, Size: 1, Length: 383}, + }, + }} + + s, _ := newStubSlskd(t, stub) + + got, err := s.Search(context.Background(), Download{ + ReleaseMBID: "rel", Artist: "Radiohead", Album: "OK Computer (Deluxe Edition)", + }) + if err != nil { + t.Fatalf("Search: %v", err) + } + + if len(got) != 1 { + t.Fatalf("got %d candidates, want the one folder once", len(got)) + } + + if got[0].Files[0].LengthMillis != 284_000 { + t.Errorf("length = %d ms, want 284000 from slskd's seconds", got[0].Files[0].LengthMillis) + } + + stub.mu.Lock() + searches := append([]map[string]any(nil), stub.searches...) + gets := append([]string(nil), stub.searchGets...) + stub.mu.Unlock() + + if len(searches) != 2 { + t.Fatalf("ran %d searches, want 2", len(searches)) + } + + for _, body := range searches { + for _, key := range []string{ + "searchTimeout", "responseLimit", "fileLimit", + "minimumResponseFileCount", "maximumPeerQueueLength", + } { + if _, ok := body[key]; !ok { + t.Errorf("search %q does not state %s", body["searchText"], key) + } + } + } + + // The responses are fetched once at the end, not with every poll. + for _, uri := range gets { + if strings.Contains(uri, "includeResponses") { + t.Errorf("poll %s asked for every response", uri) + } + } +} + +// A daemon without the responses endpoint still returns results. +func TestSlskdSearchFallsBackForAnOlderDaemon(t *testing.T) { + t.Parallel() + + stub := newSlskdStub(t) + stub.noResponsesEndpoint = true + stub.responses = []slskdResponse{{ + Username: "peer", + Files: []slskdFile{ + {Filename: `\m\Album\01 A.flac`, Size: 1}, + {Filename: `\m\Album\02 B.flac`, Size: 1}, + }, + }} + + s, _ := newStubSlskd(t, stub) + + got, err := s.Search(context.Background(), Download{Query: "album"}) + if err != nil { + t.Fatalf("Search: %v", err) + } + + if len(got) != 1 { + t.Errorf("got %d candidates, want 1 through the fallback", len(got)) + } +} + +func TestDurationAgreement(t *testing.T) { + t.Parallel() + + cases := []struct { + got, want int64 + score float64 + }{ + {300_000, 300_000, 1}, + {301_500, 300_000, 1}, // a second of silence + {300_000, 316_500, 0.5}, + {300_000, 345_000, 0}, // a different edit + } + + for _, tc := range cases { + if got := durationAgreement(tc.got, tc.want); got < tc.score-0.01 || got > tc.score+0.01 { + t.Errorf("durationAgreement(%d, %d) = %f, want %f", tc.got, tc.want, got, tc.score) + } + } +} + +// Two folders with the same track names are told apart by their +// lengths: one is the album, the other a live record of the same songs. +func TestDurationsSeparateTheRightRecording(t *testing.T) { + t.Parallel() + + dl := okComputer() + + timed := func(id string, lengths ...int64) Candidate { + c := candidateFor(id, allTitles(), ".flac", 30_000_000) + for i := range c.Files { + c.Files[i].LengthMillis = lengths[i] + } + + return c + } + + studio := timed("studio", trackMillis, trackMillis+1_000, trackMillis, trackMillis-500) + live := timed( + "live", + trackMillis+60_000, + trackMillis+75_000, + trackMillis+50_000, + trackMillis+90_000, + ) + + ranked := Rank(dl, []Candidate{live, studio}, nil, AutoDownloadPrefs{}) + + if ranked[0].ID != "studio" { + t.Fatalf("winner = %s, want the recording whose lengths match", ranked[0].ID) + } + + if !ranked[0].Match.DurationKnown || ranked[0].Match.DurationFit < 0.99 { + t.Errorf( + "studio duration fit = %f known=%v", + ranked[0].Match.DurationFit, + ranked[0].Match.DurationKnown, + ) + } + + if ranked[1].Match.DurationFit != 0 { + t.Errorf("live duration fit = %f, want 0", ranked[1].Match.DurationFit) + } +} + +// Without lengths the score is exactly what it was before durations +// were read, so a provider that reports none is not penalised. +func TestUnknownDurationsLeaveTheScoreAlone(t *testing.T) { + t.Parallel() + + dl := okComputer() + c := Score(dl, candidateFor("c", allTitles(), ".flac", 30_000_000), 50, AutoDownloadPrefs{}) + + if c.Match.DurationKnown { + t.Fatal("no file states a length, yet durations are known") + } + + want := weightTitleFit*c.Match.TitleFit + + weightCompleteness*c.Match.Completeness + + weightAlbumFit*c.Match.AlbumFit + + weightArtistFit*c.Match.ArtistFit + + if c.Match.Overall != want { + t.Errorf("match = %f, want the untimed formula's %f", c.Match.Overall, want) + } +} diff --git a/backend/download/types.go b/backend/download/types.go index d76c2ec..b753267 100644 --- a/backend/download/types.go +++ b/backend/download/types.go @@ -232,12 +232,17 @@ type Candidate struct { // results give paths and sizes but no tags, so Format and duration are // inferred from the path and size where possible. type CandidateFile struct { - Path string `json:"path"` - Size int64 `json:"size"` - Format Format `json:"format"` - Bitrate int `json:"bitrate,omitempty"` // kbps, 0 when unknown - IsAudio bool `json:"isAudio"` - MatchedTo int `json:"matchedTo,omitempty"` // expected track position + Path string `json:"path"` + Size int64 `json:"size"` + Format Format `json:"format"` + Bitrate int `json:"bitrate,omitempty"` // kbps, 0 when unknown + IsAudio bool `json:"isAudio"` + + // LengthMillis is the file's duration as the source reports it, or + // 0 when it does not. Soulseek reports it for most audio files. + LengthMillis int64 `json:"lengthMillis,omitempty"` + + MatchedTo int `json:"matchedTo,omitempty"` // expected track position } // Format is a normalized audio container/codec name. @@ -286,7 +291,14 @@ type MatchScore struct { TitleFit float64 `json:"titleFit"` // filenames vs expected titles ArtistFit float64 `json:"artistFit"` // path/origin vs expected artist AlbumFit float64 `json:"albumFit"` // folder name vs album title - Completeness float64 `json:"completeness"` // audio files vs expected count + Completeness float64 `json:"completeness"` // aligned tracks vs expected count + + // DurationFit is how well the aligned files' lengths agree with the + // expected tracks', and DurationKnown whether enough of them stated + // a length for that to count. When it does not, the score is the + // four text signals alone, exactly as before durations were read. + DurationFit float64 `json:"durationFit"` + DurationKnown bool `json:"durationKnown"` // Anchored records whether an MBID drove this score. Unanchored // matches are capped, because there is nothing to be right about. diff --git a/frontend/bindings/yellowjacket/backend/download/models.ts b/frontend/bindings/yellowjacket/backend/download/models.ts index fbe539a..0db4f82 100644 --- a/frontend/bindings/yellowjacket/backend/download/models.ts +++ b/frontend/bindings/yellowjacket/backend/download/models.ts @@ -121,6 +121,12 @@ export interface CandidateFile { "bitrate"?: number; "isAudio": boolean; + /** + * LengthMillis is the file's duration as the source reports it, or + * 0 when it does not. Soulseek reports it for most audio files. + */ + "lengthMillis"?: number; + /** * expected track position */ @@ -453,10 +459,19 @@ export interface MatchScore { "albumFit": number; /** - * audio files vs expected count + * aligned tracks vs expected count */ "completeness": number; + /** + * DurationFit is how well the aligned files' lengths agree with the + * expected tracks', and DurationKnown whether enough of them stated + * a length for that to count. When it does not, the score is the + * four text signals alone, exactly as before durations were read. + */ + "durationFit": number; + "durationKnown": boolean; + /** * Anchored records whether an MBID drove this score. Unanchored * matches are capped, because there is nothing to be right about.