diff --git a/core/agents/interfaces.go b/core/agents/interfaces.go index 19df91d02..1fe8c2a23 100644 --- a/core/agents/interfaces.go +++ b/core/agents/interfaces.go @@ -4,6 +4,7 @@ import ( "context" "errors" + "github.com/gohugoio/hashstructure" "github.com/navidrome/navidrome/model" ) @@ -39,11 +40,31 @@ type Song struct { ISRC string Artist string ArtistMBID string + Artists []Artist // optional full artist list; ArtistList normalizes against Artist/ArtistMBID Album string AlbumMBID string Duration uint32 // Duration in milliseconds, 0 means unknown } +// Equals reports strict whole-value equality, used to dedup identical input songs. It hashes +// rather than comparing with ==, which the Artists slice makes illegal. +func (s Song) Equals(other Song) bool { + h1, _ := hashstructure.Hash(s, nil) + h2, _ := hashstructure.Hash(other, nil) + return h1 == h2 +} + +// ArtistList normalizes the single/multi-artist representations so callers never branch on len(Artists). +func (s Song) ArtistList() []Artist { + if len(s.Artists) > 0 { + return s.Artists + } + if s.Artist != "" { + return []Artist{{Name: s.Artist, MBID: s.ArtistMBID}} + } + return nil +} + var ( ErrNotFound = errors.New("not found") ) diff --git a/core/agents/interfaces_test.go b/core/agents/interfaces_test.go new file mode 100644 index 000000000..46bd2f93b --- /dev/null +++ b/core/agents/interfaces_test.go @@ -0,0 +1,47 @@ +package agents + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Song.Equals", func() { + base := Song{ID: "1", Name: "S", Artist: "A", Artists: []Artist{{ID: "x", Name: "A"}}} + It("true for identical songs incl Artists", func() { + Expect(base.Equals(base)).To(BeTrue()) + }) + It("false when Artists differ", func() { + other := base + other.Artists = []Artist{{ID: "y", Name: "B"}} + Expect(base.Equals(other)).To(BeFalse()) + }) + It("false when a scalar differs", func() { + other := base + other.Name = "T" + Expect(base.Equals(other)).To(BeFalse()) + }) + It("true when both have empty Artists and equal scalars", func() { + a := Song{ID: "1", Name: "S", Artist: "A"} + Expect(a.Equals(a)).To(BeTrue()) + }) +}) + +var _ = Describe("Song.ArtistList", func() { + It("returns the Artists slice when present", func() { + s := Song{Artist: "Primary", ArtistMBID: "mbid-primary", Artists: []Artist{ + {ID: "id-drake", Name: "Drake", MBID: "mbid-drake"}, + {Name: "Future", MBID: "mbid-future"}, + }} + Expect(s.ArtistList()).To(Equal([]Artist{ + {ID: "id-drake", Name: "Drake", MBID: "mbid-drake"}, + {Name: "Future", MBID: "mbid-future"}, + })) + }) + It("falls back to the single Artist field with empty ID", func() { + s := Song{Artist: "Drake", ArtistMBID: "mbid-drake"} + Expect(s.ArtistList()).To(Equal([]Artist{{Name: "Drake", MBID: "mbid-drake"}})) + }) + It("returns empty when no artist is set", func() { + Expect(Song{}.ArtistList()).To(BeEmpty()) + }) +}) diff --git a/core/matcher/doc.go b/core/matcher/doc.go index cff866799..1024bc274 100644 --- a/core/matcher/doc.go +++ b/core/matcher/doc.go @@ -39,25 +39,40 @@ // 1. Title similarity (Jaro-Winkler score, 0.0–1.0) // 2. Duration proximity (closer duration scores higher; 1.0 when the agent // reports no duration) -// 3. Preferred-track flag (enabled by conf.Server.Matcher.PreferStarred; -// prioritizes tracks that are starred or rated >= 4) -// 4. Specificity level (0–5, based on metadata precision; higher is better) -// 5. Album similarity (Jaro-Winkler, as the final tiebreaker) +// 3. Specificity level (0–5, based on metadata precision; higher is better) +// 4. Artist overlap (how many of the song's artists the track credits; more +// shared artists is better) +// 5. Preferred-track flag (enabled by conf.Server.Matcher.PreferStarred; +// prioritizes tracks that are starred or rated >= 4, but only among +// candidates of equal specificity and overlap) +// 6. Album similarity (Jaro-Winkler, as the final tiebreaker) // // The specificity levels, from most to least specific, are: // -// Level 5: Title + Artist MBID + Album MBID -// Level 4: Title + Artist MBID + Album name (fuzzy) +// Level 5: Title + Artist identity + Album MBID +// Level 4: Title + Artist identity + Album name (fuzzy) // Level 3: Title + Artist name + Album name (fuzzy) -// Level 2: Title + Artist MBID +// Level 2: Title + Artist identity // Level 1: Title + Artist name // Level 0: Title only // +// "Artist identity" is a match on the artist's Navidrome ID (the strongest signal, +// when a source supplies one) or its MBID. A plain name match is the weaker fallback +// used for an artist with no identity match (e.g. a cover credited to a different +// artist of the same name). +// // The title phase always requires an agent artist to scope the library query, so // Level 0 does not mean "no artist": it applies when a candidate matches on title // but its own artist differs from the query's (e.g. a cover or a featured-artist // credit), leaving the title as the only shared field. // +// A song may carry several artists, and the title phase scopes candidate tracks by +// ANY of them: a track credited to at least one shared artist is considered. When a +// source supplies a Navidrome artist ID, that artist is matched directly, skipping +// name/MBID resolution. Among equally specific candidates, the one sharing more of +// the song's artists wins, so a track crediting every collaborator outranks one +// crediting only a single artist. +// // Each input song is scored independently, so two songs with the same title and // artist but different durations can resolve to different library tracks (each // matches the track closest to its own duration). @@ -97,12 +112,13 @@ // Result: studio for the first song, live for the second // // Preferred track — when conf.Server.Matcher.PreferStarred is enabled, a -// starred (or rating >= 4) track is preferred even over a more specific match, -// because the preferred flag outranks specificity: +// starred (or rating >= 4) track is preferred, but only when specificity and +// artist overlap are equal. A more specific match always wins regardless of the +// preferred flag: // // Agent returns: {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"} // Library has: // {ID: "exact", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"} // Level 3 // {ID: "starred", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Singles", Starred: true} // Level 1, starred -// Result: starred (the preferred flag outranks the better album match) +// Result: exact (specificity outranks the starred flag; preferred only breaks ties of equal identity) package matcher diff --git a/core/matcher/matcher.go b/core/matcher/matcher.go index 5322b649f..d472f4a95 100644 --- a/core/matcher/matcher.go +++ b/core/matcher/matcher.go @@ -209,11 +209,18 @@ func (m *Matcher) matchByISRC(ctx context.Context, songs []agents.Song, result m return nil } +// queryArtist is one of a song's artists. A non-empty id is matched directly, skipping name/MBID +// resolution; name is pre-sanitized (article-stripped). +type queryArtist struct { + id string + name string + mbid string +} + // songQuery represents a normalized query for matching a song to library tracks. type songQuery struct { title string - artist string - artistMBID string + artists []queryArtist album string albumMBID string durationMs uint32 @@ -224,11 +231,13 @@ type matchScore struct { titleSimilarity float64 durationProximity float64 preferredMatch bool - albumSimilarity float64 specificityLevel int + artistOverlap int + albumSimilarity float64 } // betterThan returns true if this score beats another. +// Identity signals (specificity, overlap) outrank the taste signal (preferred). func (s matchScore) betterThan(other matchScore) bool { if s.titleSimilarity != other.titleSimilarity { return s.titleSimilarity > other.titleSimilarity @@ -236,12 +245,15 @@ func (s matchScore) betterThan(other matchScore) bool { if s.durationProximity != other.durationProximity { return s.durationProximity > other.durationProximity } - if s.preferredMatch != other.preferredMatch { - return s.preferredMatch - } if s.specificityLevel != other.specificityLevel { return s.specificityLevel > other.specificityLevel } + if s.artistOverlap != other.artistOverlap { + return s.artistOverlap > other.artistOverlap + } + if s.preferredMatch != other.preferredMatch { + return s.preferredMatch + } return s.albumSimilarity > other.albumSimilarity } @@ -249,46 +261,57 @@ func (s matchScore) betterThan(other matchScore) bool { // when the same track is scored against multiple queries. The `mf` field is a pointer to avoid // copying the large MediaFile struct into each entry of the sanitized slice. type sanitizedTrack struct { - mf *model.MediaFile - title string - artist string - album string - artistMBID string // resolved from the artist table; mf.MbzArtistID is not populated on the bulk path + mf *model.MediaFile + title string + artist string + album string + artistIDs map[string]struct{} // query's owned artist IDs this track credits; an ID match is the strongest identity signal + artistMBIDs map[string]struct{} // MBIDs of those artists (artist table; mf.MbzArtistID is not populated on the bulk path) } -func newSanitizedTrack(mf *model.MediaFile, artistMBID string) sanitizedTrack { +func newSanitizedTrack(mf *model.MediaFile, artistIDs, artistMBIDs map[string]struct{}) sanitizedTrack { return sanitizedTrack{ - mf: mf, - title: str.SanitizeFieldForSorting(mf.Title), - artist: str.SanitizeFieldForSortingNoArticle(mf.Artist), - album: str.SanitizeFieldForSorting(mf.Album), - artistMBID: artistMBID, + mf: mf, + title: str.SanitizeFieldForSorting(mf.Title), + artist: str.SanitizeFieldForSortingNoArticle(mf.Artist), + album: str.SanitizeFieldForSorting(mf.Album), + artistIDs: artistIDs, + artistMBIDs: artistMBIDs, } } -// computeSpecificityLevel determines how well query metadata matches a track (0-5). -// The track's title, artist, and album fields must be pre-sanitized, and artistMBID -// must hold the resolved artist MBID. +// computeSpecificityLevel determines how well query metadata matches a track (0-5), taking the best +// level achievable across any of the query's artists. Fields must be pre-sanitized. +// +// A query artist counts as an identity match when the track credits its resolved Navidrome ID (the +// strongest signal, our own primary key) or its MBID; that identity then unlocks the album tiers. +// Name matching is the lowest fallback for an artist with no identity match (e.g. a cover credited +// to a different artist by the same name). func computeSpecificityLevel(q songQuery, t sanitizedTrack, albumThreshold float64) int { - if q.artistMBID != "" && q.albumMBID != "" && - t.artistMBID == q.artistMBID && t.mf.MbzAlbumID == q.albumMBID { - return 5 + best := 0 + albumOK := q.album != "" && similarityRatio(t.album, q.album) >= albumThreshold + for _, a := range q.artists { + _, idMember := t.artistIDs[a.id] + _, mbidMember := t.artistMBIDs[a.mbid] + identity := (a.id != "" && idMember) || (a.mbid != "" && mbidMember) + level := 0 + switch { + case identity && q.albumMBID != "" && t.mf.MbzAlbumID == q.albumMBID: + level = 5 + case identity && q.album != "" && albumOK: + level = 4 + case a.name != "" && q.album != "" && t.artist == a.name && albumOK: + level = 3 + case identity: + level = 2 + case a.name != "" && t.artist == a.name: + level = 1 + } + if level > best { + best = level + } } - if q.artistMBID != "" && q.album != "" && - t.artistMBID == q.artistMBID && similarityRatio(t.album, q.album) >= albumThreshold { - return 4 - } - if q.artist != "" && q.album != "" && - t.artist == q.artist && similarityRatio(t.album, q.album) >= albumThreshold { - return 3 - } - if q.artistMBID != "" && t.artistMBID == q.artistMBID { - return 2 - } - if q.artist != "" && t.artist == q.artist { - return 1 - } - return 0 + return best } // indexedQuery pairs a normalized songQuery with the index of the input song @@ -301,12 +324,12 @@ type indexedQuery struct { // matchByTitle fills result with fuzzy title+artist matches, skipping songs // already matched by a higher-priority loader. func (m *Matcher) matchByTitle(ctx context.Context, songs []agents.Song, result map[int]model.MediaFile) error { - byArtist := groupQueriesByArtist(songs, result) - if len(byArtist) == 0 { + queries := groupQueries(songs, result) + if len(queries) == 0 { return nil } - resolved, err := m.resolveArtists(ctx, byArtist) + resolved, err := m.resolveArtists(ctx, queries) if err != nil || len(resolved.allIDs) == 0 { return err } @@ -318,136 +341,191 @@ func (m *Matcher) matchByTitle(ctx context.Context, songs []agents.Song, result tracksByQuery := resolved.bucketTracks(tracks) threshold := float64(conf.Server.Matcher.FuzzyThreshold) / 100.0 - for artist, queries := range byArtist { - sanitized := tracksByQuery[artist] - // Each song is matched independently by index, so two songs with the same - // (title, artist) but different durations can resolve to different tracks. - for _, iq := range queries { - if mf, found := m.findBestMatch(iq.query, sanitized, threshold); found { - result[iq.index] = mf - } + for _, iq := range queries { + sanitized := tracksByQuery[iq.index] + if mf, found := m.findBestMatch(iq.query, sanitized, threshold); found { + result[iq.index] = mf } } return nil } -// groupQueriesByArtist buckets the still-unmatched title queries by sanitized artist name. -// Songs without an artist are skipped: title matching needs one to scope the library query. -func groupQueriesByArtist(songs []agents.Song, result map[int]model.MediaFile) map[string][]indexedQuery { - byArtist := map[string][]indexedQuery{} +// groupQueries builds one normalized title query per still-unmatched song, carrying its full +// artist set. An artist is usable if it carries a Navidrome ID or a non-empty sanitized name; +// songs with no usable artist are skipped (the title phase needs at least one to scope the query). +func groupQueries(songs []agents.Song, result map[int]model.MediaFile) []indexedQuery { + var queries []indexedQuery for i, s := range songs { if _, done := result[i]; done { continue } - artist := str.SanitizeFieldForSortingNoArticle(s.Artist) - if artist == "" { + var artists []queryArtist + for _, a := range s.ArtistList() { + name := str.SanitizeFieldForSortingNoArticle(a.Name) + if a.ID == "" && name == "" { + continue + } + artists = append(artists, queryArtist{id: a.ID, name: name, mbid: a.MBID}) + } + if len(artists) == 0 { continue } - byArtist[artist] = append(byArtist[artist], indexedQuery{index: i, query: songQuery{ + queries = append(queries, indexedQuery{index: i, query: songQuery{ title: str.SanitizeFieldForSorting(s.Name), - artist: artist, - artistMBID: s.ArtistMBID, + artists: artists, album: str.SanitizeFieldForSorting(s.Album), albumMBID: s.AlbumMBID, durationMs: s.Duration, }}) } - return byArtist + return queries } -// resolvedArtists holds the agent artists resolved to artist-table rows. Everything routes by -// stable artist ID, never by name, so MBID-resolved artists whose order name differs from the -// query name are not misrouted. +// resolvedArtists holds the agent artists resolved to artist-table rows, keyed by the query index +// that owns them. Routing is always by stable artist ID, never by name. type resolvedArtists struct { - byQuery map[string]map[string]struct{} // sanitized query name -> set of resolved artist IDs - mbid map[string]string // artist ID -> its MBID (the real one, from the artist table) - allIDs []string // every resolved artist ID, for the track lookup + byQuery map[int]map[string]struct{} // query index -> set of resolved artist IDs + mbid map[string]string // artist ID -> its MBID (from the artist table) + allIDs []string // every resolved artist ID, for the track lookup } -// resolveArtists resolves the queries' artists against the artist table (by sort name or -// agent-provided MBID) and records, for each query, which artist IDs it owns. -func (m *Matcher) resolveArtists(ctx context.Context, byArtist map[string][]indexedQuery) (resolvedArtists, error) { - names := make([]string, 0, len(byArtist)) - mbidToQueries := make(map[string][]string, len(byArtist)) // agent ArtistMBID -> query names that supplied it - for name, queries := range byArtist { - names = append(names, name) - for _, iq := range queries { - if iq.query.artistMBID != "" { - mbidToQueries[iq.query.artistMBID] = append(mbidToQueries[iq.query.artistMBID], name) +// resolveArtists resolves every artist of every query to artist-table rows. Artists that carry a +// Navidrome ID are owned directly (no name/MBID lookup). The remaining names/MBIDs are resolved in +// one batched query. Ownership is recorded per query index. +func (m *Matcher) resolveArtists(ctx context.Context, queries []indexedQuery) (resolvedArtists, error) { + res := resolvedArtists{ + byQuery: make(map[int]map[string]struct{}, len(queries)), + mbid: make(map[string]string), + } + allIDs := map[string]struct{}{} // de-dupe across fast-path + resolved + + // One pending entry per non-ID artist (carrying the query that owns it). ID-bearing artists + // take the fast-path and are owned directly. + type pendingArtist struct { + name, mbid string + query int + } + var pending []pendingArtist + for _, iq := range queries { + for _, a := range iq.query.artists { + if a.id != "" { + addToSet(res.byQuery, iq.index, a.id) // ID fast-path: own directly + allIDs[a.id] = struct{}{} + continue } + pending = append(pending, pendingArtist{name: a.name, mbid: a.mbid, query: iq.index}) + } + } + // query indices that supplied each order name / each MBID (skip the empty key — an artist may + // have only one of name/mbid). + nameToQueries := map[string][]int{} + mbidToQueries := map[string][]int{} + for _, p := range pending { + if p.name != "" { + nameToQueries[p.name] = append(nameToQueries[p.name], p.query) + } + if p.mbid != "" { + mbidToQueries[p.mbid] = append(mbidToQueries[p.mbid], p.query) } } - filter := squirrel.Or{squirrel.Eq{"order_artist_name": names}} + // Query the artist table for name/MBID artists AND for the fast-path IDs (so their MBIDs are + // available for specificity scoring). + var filter squirrel.Or + if len(nameToQueries) > 0 { + filter = append(filter, squirrel.Eq{"order_artist_name": slices.Collect(maps.Keys(nameToQueries))}) + } if len(mbidToQueries) > 0 { filter = append(filter, squirrel.Eq{"mbz_artist_id": slices.Collect(maps.Keys(mbidToQueries))}) } - artists, err := m.ds.Artist(ctx).GetAll(model.QueryOptions{Filters: filter}) - if err != nil { - return resolvedArtists{}, err + if len(allIDs) > 0 { + filter = append(filter, squirrel.Eq{"id": slices.Collect(maps.Keys(allIDs))}) } - - res := resolvedArtists{ - byQuery: make(map[string]map[string]struct{}, len(byArtist)), - mbid: make(map[string]string, len(artists)), - allIDs: make([]string, 0, len(artists)), - } - for _, a := range artists { - res.mbid[a.ID] = a.MbzArtistID - res.allIDs = append(res.allIDs, a.ID) - // An artist belongs to a query if its order name matches the query name, or if its MBID - // matches one a query supplied. The same MBID can come from several queries (agent aliases), - // so every one of them owns the artist. - res.own(a.OrderArtistName, a.ID) - if a.MbzArtistID != "" { - for _, name := range mbidToQueries[a.MbzArtistID] { - res.own(name, a.ID) + if len(filter) > 0 { + artists, err := m.ds.Artist(ctx).GetAll(model.QueryOptions{Filters: filter}) + if err != nil { + return resolvedArtists{}, err + } + for _, a := range artists { + res.mbid[a.ID] = a.MbzArtistID + allIDs[a.ID] = struct{}{} + for _, idx := range nameToQueries[a.OrderArtistName] { + addToSet(res.byQuery, idx, a.ID) + } + if a.MbzArtistID != "" { + for _, idx := range mbidToQueries[a.MbzArtistID] { + addToSet(res.byQuery, idx, a.ID) + } } } } + + res.allIDs = slices.Collect(maps.Keys(allIDs)) return res, nil } -// own records that the named query owns the given artist ID. A name that is not a query simply -// gets its own (unused) entry. -func (r resolvedArtists) own(name, artistID string) { - if r.byQuery[name] == nil { - r.byQuery[name] = map[string]struct{}{} +func addToSet(m map[int]map[string]struct{}, k int, v string) { + if m[k] == nil { + m[k] = map[string]struct{}{} } - r.byQuery[name][artistID] = struct{}{} + m[k][v] = struct{}{} } -// bucketTracks groups tracks by query name, at most once per query even when a track credits -// several of that query's artists, so the same track is not scored twice. The participants JSON -// on each track carries artist IDs but not their MBID, so the MBID comes from r.mbid instead. -func (r resolvedArtists) bucketTracks(tracks []model.MediaFile) map[string][]sanitizedTrack { - // Invert byQuery once so each participant maps straight to the queries that own it, instead of - // scanning every query per participant. - queriesByArtist := make(map[string][]string) - for name, ids := range r.byQuery { +// scoredTrack is a candidate track for a query; overlap is how many of the query's distinct +// artist IDs the track credits. +type scoredTrack struct { + sanitizedTrack + overlap int +} + +// queryAccum tallies, for one track against one query, the overlap count, the credited artists' +// owned IDs, and their MBIDs. +type queryAccum struct { + overlap int + ids map[string]struct{} + mbids map[string]struct{} +} + +func (r resolvedArtists) bucketTracks(tracks []model.MediaFile) map[int][]scoredTrack { + queriesByArtist := make(map[string][]int) + for idx, ids := range r.byQuery { for id := range ids { - queriesByArtist[id] = append(queriesByArtist[id], name) + queriesByArtist[id] = append(queriesByArtist[id], idx) } } - byQuery := make(map[string][]sanitizedTrack, len(r.byQuery)) - added := make(map[string]map[string]struct{}, len(r.byQuery)) // query name -> set of track IDs already bucketed + byQuery := make(map[int][]scoredTrack, len(r.byQuery)) for i := range tracks { + acc := map[int]*queryAccum{} + credited := map[string]struct{}{} for _, p := range tracks[i].Participants[model.RoleArtist] { - mbid, isResolved := r.mbid[p.ID] - if !isResolved { + if _, dup := credited[p.ID]; dup { continue } - for _, name := range queriesByArtist[p.ID] { - if added[name] == nil { - added[name] = map[string]struct{}{} - } - if _, dup := added[name][tracks[i].ID]; dup { - continue - } - added[name][tracks[i].ID] = struct{}{} - byQuery[name] = append(byQuery[name], newSanitizedTrack(&tracks[i], mbid)) + owners, owned := queriesByArtist[p.ID] + if !owned { + continue } + credited[p.ID] = struct{}{} + mbid := r.mbid[p.ID] // "" if not in the artist-table result + for _, idx := range owners { + a := acc[idx] + if a == nil { + a = &queryAccum{ids: map[string]struct{}{}, mbids: map[string]struct{}{}} + acc[idx] = a + } + a.overlap++ + a.ids[p.ID] = struct{}{} + if mbid != "" { + a.mbids[mbid] = struct{}{} + } + } + } + for idx, a := range acc { + byQuery[idx] = append(byQuery[idx], scoredTrack{ + sanitizedTrack: newSanitizedTrack(&tracks[i], a.ids, a.mbids), + overlap: a.overlap, + }) } } return byQuery @@ -488,35 +566,32 @@ func durationProximity(durationMs uint32, mediaFileDurationSec float32) float64 } // findBestMatch finds the best matching track using combined title/album similarity and specificity scoring. -func (m *Matcher) findBestMatch(q songQuery, sanitizedTracks []sanitizedTrack, threshold float64) (model.MediaFile, bool) { +func (m *Matcher) findBestMatch(q songQuery, candidates []scoredTrack, threshold float64) (model.MediaFile, bool) { var bestMatch model.MediaFile bestScore := matchScore{titleSimilarity: -1} found := false preferStarred := conf.Server.Matcher.PreferStarred - for _, t := range sanitizedTracks { - titleSim := similarityRatio(q.title, t.title) - + for _, c := range candidates { + titleSim := similarityRatio(q.title, c.title) if titleSim < threshold { continue } - var albumSim float64 if q.album != "" { - albumSim = similarityRatio(q.album, t.album) + albumSim = similarityRatio(q.album, c.album) } - score := matchScore{ titleSimilarity: titleSim, - durationProximity: durationProximity(q.durationMs, t.mf.Duration), - preferredMatch: preferStarred && isPreferredTrack(t.mf), + durationProximity: durationProximity(q.durationMs, c.mf.Duration), + preferredMatch: preferStarred && isPreferredTrack(c.mf), albumSimilarity: albumSim, - specificityLevel: computeSpecificityLevel(q, t, threshold), + specificityLevel: computeSpecificityLevel(q, c.sanitizedTrack, threshold), + artistOverlap: c.overlap, } - if score.betterThan(bestScore) { bestScore = score - bestMatch = *t.mf + bestMatch = *c.mf found = true } } @@ -544,7 +619,7 @@ func orderAndDedup(songs []agents.Song, matches map[int]model.MediaFile, count i continue } if prevIdx, alreadyAdded := addedBy[mf.ID]; alreadyAdded { - if s != songs[prevIdx] { + if !s.Equals(songs[prevIdx]) { continue } } else { diff --git a/core/matcher/matcher_internal_test.go b/core/matcher/matcher_internal_test.go index 62bccd844..b81fa9012 100644 --- a/core/matcher/matcher_internal_test.go +++ b/core/matcher/matcher_internal_test.go @@ -1,7 +1,9 @@ package matcher import ( + "github.com/navidrome/navidrome/core/agents" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -54,14 +56,200 @@ var _ = Describe("similarityRatio", func() { }) var _ = Describe("matcher internals", func() { - It("computeSpecificityLevel uses sanitizedTrack.artistMBID for artist-MBID levels", func() { + It("computeSpecificityLevel uses sanitizedTrack.artistMBIDs for artist-MBID levels", func() { q := songQuery{ - title: "song", - artistMBID: "artist-mbid-1", - albumMBID: "album-mbid-1", + title: "song", + artists: []queryArtist{{mbid: "artist-mbid-1"}}, + albumMBID: "album-mbid-1", } mf := model.MediaFile{Title: "Song", MbzAlbumID: "album-mbid-1"} // note: mf.MbzArtistID intentionally empty - t := newSanitizedTrack(&mf, "artist-mbid-1") // resolved MBID supplied here + t := newSanitizedTrack(&mf, nil, map[string]struct{}{"artist-mbid-1": {}}) Expect(computeSpecificityLevel(q, t, 0.85)).To(Equal(5)) }) + + It("computeSpecificityLevel maximizes the level over all query artists", func() { + // First artist does not match; second matches by name with a matching album → level 3. + q := songQuery{ + title: "song", + album: "violator", + artists: []queryArtist{ + {name: "no match"}, + {name: "depeche mode"}, + }, + } + mf := model.MediaFile{Title: "Song", Artist: "Depeche Mode", Album: "Violator"} + t := newSanitizedTrack(&mf, nil, nil) + Expect(computeSpecificityLevel(q, t, 0.85)).To(Equal(3)) + }) + + It("scores MBID specificity for any credited artist, not just the last", func() { + q := songQuery{ + title: "song", + artists: []queryArtist{ + {name: "drake", mbid: "mbz-drake"}, + {name: "future", mbid: "mbz-future"}, + }, + album: "wrong album", // force album mismatch so only MBID-level (2) is reachable, not 3+ + } + // Track credits BOTH MBIDs; with the old last-wins string this would only match one. + t := sanitizedTrack{ + mf: &model.MediaFile{}, + title: "song", + artist: "drake", + album: "some other album", + artistMBIDs: map[string]struct{}{"mbz-drake": {}, "mbz-future": {}}, + } + // Either artist's MBID matching yields level 2 (MBID, no album match). The point: it is + // reached via mbz-future too, which the old code would have dropped. + Expect(computeSpecificityLevel(q, t, 0.85)).To(Equal(2)) + }) + + It("treats an ID-only artist (no name/MBID) as an identity match, unlocking album tiers", func() { + // A plugin that supplies only a Navidrome artist ID: name and mbid are empty. The ID is the + // strongest identity signal, so the track's album still elevates specificity above 0. + q := songQuery{ + title: "song", + artists: []queryArtist{{id: "artist-1"}}, + album: "violator", + albumMBID: "album-mbid-1", + } + // Track credits the owned artist ID; no MBID anywhere (untagged library / ID-only plugin). + mf := model.MediaFile{Title: "Song", Album: "Violator", MbzAlbumID: "album-mbid-1"} + t := newSanitizedTrack(&mf, map[string]struct{}{"artist-1": {}}, nil) + // Album MBID matches → level 5 via the ID identity, where the old code scored 0. + Expect(computeSpecificityLevel(q, t, 0.85)).To(Equal(5)) + + // Same artist, album name matches but no album MBID → level 4 via the ID identity. + q.albumMBID = "" + mf2 := model.MediaFile{Title: "Song", Album: "Violator"} + t2 := newSanitizedTrack(&mf2, map[string]struct{}{"artist-1": {}}, nil) + Expect(computeSpecificityLevel(q, t2, 0.85)).To(Equal(4)) + }) }) + +var _ = Describe("groupQueries", func() { + It("builds one query per unmatched song carrying all artists", func() { + songs := []agents.Song{ + {Name: "Song A", Artists: []agents.Artist{{Name: "Drake"}, {Name: "Future"}}}, + } + queries := groupQueries(songs, map[int]model.MediaFile{}) + Expect(queries).To(HaveLen(1)) + Expect(queries[0].index).To(Equal(0)) + Expect(queries[0].query.title).To(Equal("song a")) + Expect(queries[0].query.artists).To(HaveLen(2)) + Expect(queries[0].query.artists[0].name).To(Equal("drake")) + Expect(queries[0].query.artists[1].name).To(Equal("future")) + }) + + It("strips leading articles from artist names", func() { + songs := []agents.Song{ + {Name: "Song A", Artist: "The Drake"}, + } + queries := groupQueries(songs, map[int]model.MediaFile{}) + Expect(queries).To(HaveLen(1)) + Expect(queries[0].query.artists).To(HaveLen(1)) + Expect(queries[0].query.artists[0].name).To(Equal("drake")) + }) + + It("keeps an artist that carries only an ID (empty name)", func() { + songs := []agents.Song{ + {Name: "Song A", Artists: []agents.Artist{{ID: "ar-x"}}}, + } + queries := groupQueries(songs, map[int]model.MediaFile{}) + Expect(queries).To(HaveLen(1)) + Expect(queries[0].query.artists).To(HaveLen(1)) + Expect(queries[0].query.artists[0].id).To(Equal("ar-x")) + Expect(queries[0].query.artists[0].name).To(Equal("")) + }) + + It("drops an artist with empty id and empty name but keeps usable ones", func() { + songs := []agents.Song{ + {Name: "Song A", Artists: []agents.Artist{{Name: ""}, {Name: "Future"}}}, + } + queries := groupQueries(songs, map[int]model.MediaFile{}) + Expect(queries).To(HaveLen(1)) + Expect(queries[0].query.artists).To(HaveLen(1)) + Expect(queries[0].query.artists[0].name).To(Equal("future")) + }) + + It("falls back to the single Artist field via ArtistList", func() { + songs := []agents.Song{ + {Name: "Song A", Artist: "Future", ArtistMBID: "mbid-1"}, + } + queries := groupQueries(songs, map[int]model.MediaFile{}) + Expect(queries).To(HaveLen(1)) + Expect(queries[0].query.artists).To(HaveLen(1)) + Expect(queries[0].query.artists[0].name).To(Equal("future")) + Expect(queries[0].query.artists[0].mbid).To(Equal("mbid-1")) + }) + + It("skips already-matched songs and songs with no usable artist", func() { + songs := []agents.Song{ + {Name: "Already Matched", Artist: "Drake"}, + {Name: "No Artist"}, + {Name: "Song C", Artist: "Future"}, + } + queries := groupQueries(songs, map[int]model.MediaFile{0: {ID: "done"}}) + Expect(queries).To(HaveLen(1)) + Expect(queries[0].index).To(Equal(2)) + Expect(queries[0].query.artists[0].name).To(Equal("future")) + }) +}) + +var _ = Describe("bucketTracks", func() { + It("scores tracks by how many of the query's artists they credit (overlap)", func() { + r := resolvedArtists{ + byQuery: map[int]map[string]struct{}{ + 0: {"ar-1": {}, "ar-2": {}}, + }, + mbid: map[string]string{}, + } + trackA := model.MediaFile{ID: "a", Title: "A", + Participants: artistParticipants( + model.Artist{ID: "ar-1", OrderArtistName: "one"}, + model.Artist{ID: "ar-2", OrderArtistName: "two"}, + ), + } + trackB := model.MediaFile{ID: "b", Title: "B", + Participants: artistParticipants(model.Artist{ID: "ar-1", OrderArtistName: "one"}), + } + byQuery := r.bucketTracks(model.MediaFiles{trackA, trackB}) + Expect(byQuery[0]).To(HaveLen(2)) + overlaps := map[string]int{} + for _, st := range byQuery[0] { + overlaps[st.mf.ID] = st.overlap + } + Expect(overlaps["a"]).To(Equal(2)) + Expect(overlaps["b"]).To(Equal(1)) + }) +}) + +var _ = Describe("resolveArtists ID fast-path", func() { + It("owns an artist supplied by ID without a name match", func() { + ctx := GinkgoT().Context() + artistRepo := tests.CreateMockArtistRepo() + artistRepo.SetData(model.Artists{ + {ID: "ar-x", Name: "Some Artist", OrderArtistName: "some artist", MbzArtistID: "mbz-x"}, + }) + ds := &tests.MockDataStore{MockedArtist: artistRepo} + m := New(ds) + + queries := []indexedQuery{ + {index: 0, query: songQuery{title: "song", artists: []queryArtist{{id: "ar-x"}}}}, + } + res, err := m.resolveArtists(ctx, queries) + Expect(err).ToNot(HaveOccurred()) + Expect(res.byQuery[0]).To(HaveKey("ar-x")) + Expect(res.allIDs).To(ContainElement("ar-x")) + Expect(res.mbid["ar-x"]).To(Equal("mbz-x")) + }) +}) + +// artistParticipants builds a Participants map crediting the given artists under RoleArtist. +func artistParticipants(artists ...model.Artist) model.Participants { + list := make(model.ParticipantList, len(artists)) + for i, a := range artists { + list[i] = model.Participant{Artist: a} + } + return model.Participants{model.RoleArtist: list} +} diff --git a/core/matcher/matcher_test.go b/core/matcher/matcher_test.go index 1fad0ebcc..1b3af3aa3 100644 --- a/core/matcher/matcher_test.go +++ b/core/matcher/matcher_test.go @@ -466,6 +466,103 @@ var _ = Describe("Matcher", func() { Expect(result[0].ID).To(Equal("track-1")) }) }) + + Context("multiple artists", func() { + It("prefers the track that shares more of the song's artists", func() { + conf.Server.Matcher.FuzzyThreshold = 85 + songs := []agents.Song{ + {Name: "Life Is Good", Artists: []agents.Artist{{Name: "Drake"}, {Name: "Future"}}}, + } + // Both candidates have display Artist "Drake", so they tie at specificity level 1 + // (name match). The deciding factor is artistOverlap: "both" credits Drake AND + // Future (overlap 2), "one" credits only Drake (overlap 1). + bothArtists := model.MediaFile{ + ID: "both", Title: "Life Is Good", Artist: "Drake", + Participants: artistParticipants( + model.Artist{ID: "drake", Name: "Drake", OrderArtistName: "drake"}, + model.Artist{ID: "future", Name: "Future", OrderArtistName: "future"}, + ), + } + oneArtist := model.MediaFile{ + ID: "one", Title: "Life Is Good", Artist: "Drake", + Participants: artistParticipants( + model.Artist{ID: "drake", Name: "Drake", OrderArtistName: "drake"}, + ), + } + allowTitlePhase(model.MediaFiles{oneArtist, bothArtists}) + + result, err := m.MatchSongs(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("both")) + }) + + It("matches a single-artist song against a track crediting several artists", func() { + conf.Server.Matcher.FuzzyThreshold = 85 + songs := []agents.Song{ + {Name: "Life Is Good", Artist: "Future"}, + } + track := model.MediaFile{ + ID: "multi", Title: "Life Is Good", Artist: "Future feat. Drake", + Participants: artistParticipants( + model.Artist{ID: "future", Name: "Future", OrderArtistName: "future"}, + model.Artist{ID: "drake", Name: "Drake", OrderArtistName: "drake"}, + ), + } + allowTitlePhase(model.MediaFiles{track}) + + result, err := m.MatchSongs(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("multi")) + }) + + It("matches by a directly-supplied Navidrome artist ID (fast-path)", func() { + conf.Server.Matcher.FuzzyThreshold = 85 + songs := []agents.Song{ + {Name: "Song A", Artists: []agents.Artist{{ID: "ar-x"}}}, + } + track := model.MediaFile{ + ID: "by-id", Title: "Song A", Artist: "Some Artist", + Participants: artistParticipants( + model.Artist{ID: "ar-x", Name: "Some Artist", OrderArtistName: "some artist"}, + ), + } + allowTitlePhase(model.MediaFiles{track}) + + result, err := m.MatchSongs(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("by-id")) + }) + + It("prefers a higher artist-overlap track over a starred lower-overlap track", func() { + conf.Server.Matcher.PreferStarred = true + conf.Server.Matcher.FuzzyThreshold = 85 + songs := []agents.Song{ + {Name: "Collab Hit", Artists: []agents.Artist{{Name: "Drake"}, {Name: "Future"}}}, + } + // Shares only Drake (overlap 1) but starred. + starredOne := model.MediaFile{ + ID: "starred-one", Title: "Collab Hit", + Annotations: model.Annotations{Starred: true}, + Participants: artistParticipants(model.Artist{ID: "id-drake", Name: "Drake", OrderArtistName: "drake"}), + } + // Shares both (overlap 2), not starred. + shareTwo := model.MediaFile{ + ID: "share-two", Title: "Collab Hit", + Participants: artistParticipants( + model.Artist{ID: "id-drake", Name: "Drake", OrderArtistName: "drake"}, + model.Artist{ID: "id-future", Name: "Future", OrderArtistName: "future"}, + ), + } + allowTitlePhase(model.MediaFiles{starredOne, shareTwo}) + result, err := m.MatchSongs(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("share-two")) // overlap outranks the starred flag + }) + }) }) Describe("MatchSongsIndexed", func() { @@ -848,7 +945,7 @@ var _ = Describe("Matcher", func() { Expect(result[0].ID).To(Equal("exact")) }) - It("prefers starred songs over better album match when enabled", func() { + It("prefers a more specific match over a starred track when PreferStarred is enabled", func() { conf.Server.Matcher.PreferStarred = true songs := []agents.Song{ {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, @@ -862,17 +959,14 @@ var _ = Describe("Matcher", func() { Annotations: model.Annotations{Starred: true}, Participants: artistParticipants(model.Artist{ID: "dm", Name: "Depeche Mode", OrderArtistName: "depeche mode"}), } - allowTitlePhase(model.MediaFiles{albumMatch, starredTrack}) - result, err := m.MatchSongs(ctx, songs, 5) - Expect(err).ToNot(HaveOccurred()) Expect(result).To(HaveLen(1)) - Expect(result[0].ID).To(Equal("starred")) + Expect(result[0].ID).To(Equal("album-match")) // specificity now outranks the starred flag }) - It("prefers 4-star songs over better album match when enabled", func() { + It("prefers a more specific match over a 4-star track when PreferStarred is enabled", func() { conf.Server.Matcher.PreferStarred = true songs := []agents.Song{ {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, @@ -886,14 +980,33 @@ var _ = Describe("Matcher", func() { Annotations: model.Annotations{Rating: 4}, Participants: artistParticipants(model.Artist{ID: "dm", Name: "Depeche Mode", OrderArtistName: "depeche mode"}), } - allowTitlePhase(model.MediaFiles{albumMatch, ratedTrack}) - result, err := m.MatchSongs(ctx, songs, 5) - Expect(err).ToNot(HaveOccurred()) Expect(result).To(HaveLen(1)) - Expect(result[0].ID).To(Equal("rated")) + Expect(result[0].ID).To(Equal("album-match")) // specificity now outranks the 4-star rating + }) + + It("prefers a starred track when specificity and overlap are equal", func() { + conf.Server.Matcher.PreferStarred = true + songs := []agents.Song{ + {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, + } + // Both credit the same single artist and the same album → equal specificity AND equal overlap. + plain := model.MediaFile{ + ID: "plain", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator", + Participants: artistParticipants(model.Artist{ID: "dm", Name: "Depeche Mode", OrderArtistName: "depeche mode"}), + } + starred := model.MediaFile{ + ID: "starred", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator", + Annotations: model.Annotations{Starred: true}, + Participants: artistParticipants(model.Artist{ID: "dm", Name: "Depeche Mode", OrderArtistName: "depeche mode"}), + } + allowTitlePhase(model.MediaFiles{plain, starred}) + result, err := m.MatchSongs(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("starred")) // preferred still wins the tie }) }) diff --git a/plugins/capabilities/metadata_agent.go b/plugins/capabilities/metadata_agent.go index 407f21ec5..935594abd 100644 --- a/plugins/capabilities/metadata_agent.go +++ b/plugins/capabilities/metadata_agent.go @@ -148,6 +148,8 @@ type SongRef struct { Artist string `json:"artist,omitempty"` // ArtistMBID is the MusicBrainz artist ID. ArtistMBID string `json:"artistMbid,omitempty"` + // Artists is the full artist list; when set, takes precedence over Artist/ArtistMBID for matching. + Artists []ArtistRef `json:"artists,omitempty"` // Album is the album name. Album string `json:"album,omitempty"` // AlbumMBID is the MusicBrainz release ID. diff --git a/plugins/capabilities/metadata_agent.yaml b/plugins/capabilities/metadata_agent.yaml index 4940a5056..c90ac1ce0 100644 --- a/plugins/capabilities/metadata_agent.yaml +++ b/plugins/capabilities/metadata_agent.yaml @@ -352,6 +352,11 @@ components: artistMbid: type: string description: ArtistMBID is the MusicBrainz artist ID. + artists: + type: array + description: Artists is the full artist list; when set, takes precedence over Artist/ArtistMBID for matching. + items: + $ref: '#/components/schemas/ArtistRef' album: type: string description: Album is the album name. diff --git a/plugins/capabilities/sonic_similarity.yaml b/plugins/capabilities/sonic_similarity.yaml index cba97d9b0..1d35e1e92 100644 --- a/plugins/capabilities/sonic_similarity.yaml +++ b/plugins/capabilities/sonic_similarity.yaml @@ -16,6 +16,20 @@ exports: contentType: application/json components: schemas: + ArtistRef: + description: ArtistRef is a reference to an artist with name and optional MBID. + properties: + id: + type: string + description: ID is the internal Navidrome artist ID (if known). + name: + type: string + description: Name is the artist name. + mbid: + type: string + description: MBID is the MusicBrainz ID for the artist. + required: + - name FindSonicPathRequest: properties: startSong: @@ -60,6 +74,11 @@ components: artistMbid: type: string description: ArtistMBID is the MusicBrainz artist ID. + artists: + type: array + description: Artists is the full artist list; when set, takes precedence over Artist/ArtistMBID for matching. + items: + $ref: '#/components/schemas/ArtistRef' album: type: string description: Album is the album name. diff --git a/plugins/cmd/ndpgen/internal/generator.go b/plugins/cmd/ndpgen/internal/generator.go index 705cd4d36..514da578b 100644 --- a/plugins/cmd/ndpgen/internal/generator.go +++ b/plugins/cmd/ndpgen/internal/generator.go @@ -568,9 +568,16 @@ func rustConstName(name string) string { } // skipSerializingFunc returns the appropriate skip_serializing_if function name. +// The check must match the rendered Rust type: pointers become Option, slices Vec, +// and maps HashMap, each with a different emptiness predicate. func skipSerializingFunc(goType string) string { - if strings.HasPrefix(goType, "*") || strings.HasPrefix(goType, "[]") || strings.HasPrefix(goType, "map[") { + switch { + case strings.HasPrefix(goType, "*"): return "Option::is_none" + case strings.HasPrefix(goType, "[]"): + return "Vec::is_empty" + case strings.HasPrefix(goType, "map["): + return "HashMap::is_empty" } switch goType { case "string": diff --git a/plugins/cmd/ndpgen/internal/generator_test.go b/plugins/cmd/ndpgen/internal/generator_test.go index 34c2c2886..052aa146c 100644 --- a/plugins/cmd/ndpgen/internal/generator_test.go +++ b/plugins/cmd/ndpgen/internal/generator_test.go @@ -1289,12 +1289,16 @@ type OnInitOutput struct { var _ = Describe("Rust Generation", func() { Describe("skipSerializingFunc", func() { - It("should return Option::is_none for pointer, slice, and map types", func() { + It("should return Option::is_none for pointer types", func() { Expect(skipSerializingFunc("*string")).To(Equal("Option::is_none")) Expect(skipSerializingFunc("*MyStruct")).To(Equal("Option::is_none")) - Expect(skipSerializingFunc("[]string")).To(Equal("Option::is_none")) - Expect(skipSerializingFunc("[]int32")).To(Equal("Option::is_none")) - Expect(skipSerializingFunc("map[string]int")).To(Equal("Option::is_none")) + }) + + It("should return the matching emptiness predicate for slice and map types", func() { + // The predicate must match the rendered Rust type: []T -> Vec, map[K]V -> HashMap. + Expect(skipSerializingFunc("[]string")).To(Equal("Vec::is_empty")) + Expect(skipSerializingFunc("[]int32")).To(Equal("Vec::is_empty")) + Expect(skipSerializingFunc("map[string]int")).To(Equal("HashMap::is_empty")) }) It("should return String::is_empty for string type", func() { diff --git a/plugins/metadata_agent.go b/plugins/metadata_agent.go index 52542300c..f3c26411c 100644 --- a/plugins/metadata_agent.go +++ b/plugins/metadata_agent.go @@ -229,6 +229,13 @@ func (a *MetadataAgent) GetSimilarSongsByArtist(ctx context.Context, id, name, m // songRefToAgentSong converts a single SongRef to agents.Song func songRefToAgentSong(s capabilities.SongRef) agents.Song { + var artists []agents.Artist + if len(s.Artists) > 0 { + artists = make([]agents.Artist, len(s.Artists)) + for i, a := range s.Artists { + artists[i] = agents.Artist{ID: a.ID, Name: a.Name, MBID: a.MBID} + } + } return agents.Song{ ID: s.ID, Name: s.Name, @@ -236,6 +243,7 @@ func songRefToAgentSong(s capabilities.SongRef) agents.Song { ISRC: s.ISRC, Artist: s.Artist, ArtistMBID: s.ArtistMBID, + Artists: artists, Album: s.Album, AlbumMBID: s.AlbumMBID, Duration: uint32(s.Duration * 1000), diff --git a/plugins/metadata_agent_test.go b/plugins/metadata_agent_test.go index 067ae80ca..be9d309ae 100644 --- a/plugins/metadata_agent_test.go +++ b/plugins/metadata_agent_test.go @@ -4,6 +4,7 @@ package plugins import ( "github.com/navidrome/navidrome/core/agents" + "github.com/navidrome/navidrome/plugins/capabilities" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -329,3 +330,24 @@ var _ = Describe("MetadataAgent partial implementation", Ordered, func() { Expect(err).To(MatchError(errNotImplemented)) }) }) + +var _ = Describe("songRefToAgentSong multi-artist", func() { + It("maps ArtistRef to agents.Artist", func() { + ref := capabilities.SongRef{Name: "Collab", Artist: "Drake", Artists: []capabilities.ArtistRef{ + {ID: "id-drake", Name: "Drake", MBID: "m-drake"}, + {Name: "Future", MBID: "m-future"}, + }} + got := songRefToAgentSong(ref) + Expect(got.Artists).To(Equal([]agents.Artist{ + {ID: "id-drake", Name: "Drake", MBID: "m-drake"}, + {Name: "Future", MBID: "m-future"}, + })) + }) + It("leaves Artists nil and keeps the single Artist when no Artists provided", func() { + ref := capabilities.SongRef{Name: "Solo", Artist: "Drake", ArtistMBID: "m-drake"} + got := songRefToAgentSong(ref) + Expect(got.Artists).To(BeNil()) + Expect(got.Artist).To(Equal("Drake")) + Expect(got.ArtistMBID).To(Equal("m-drake")) + }) +}) diff --git a/plugins/pdk/go/metadata/metadata.go b/plugins/pdk/go/metadata/metadata.go index 7cd63865b..cec6ce339 100644 --- a/plugins/pdk/go/metadata/metadata.go +++ b/plugins/pdk/go/metadata/metadata.go @@ -177,6 +177,8 @@ type SongRef struct { Artist string `json:"artist,omitempty"` // ArtistMBID is the MusicBrainz artist ID. ArtistMBID string `json:"artistMbid,omitempty"` + // Artists is the full artist list; when set, takes precedence over Artist/ArtistMBID for matching. + Artists []ArtistRef `json:"artists,omitempty"` // Album is the album name. Album string `json:"album,omitempty"` // AlbumMBID is the MusicBrainz release ID. diff --git a/plugins/pdk/go/metadata/metadata_stub.go b/plugins/pdk/go/metadata/metadata_stub.go index bdcd06fcb..b71a4902f 100644 --- a/plugins/pdk/go/metadata/metadata_stub.go +++ b/plugins/pdk/go/metadata/metadata_stub.go @@ -174,6 +174,8 @@ type SongRef struct { Artist string `json:"artist,omitempty"` // ArtistMBID is the MusicBrainz artist ID. ArtistMBID string `json:"artistMbid,omitempty"` + // Artists is the full artist list; when set, takes precedence over Artist/ArtistMBID for matching. + Artists []ArtistRef `json:"artists,omitempty"` // Album is the album name. Album string `json:"album,omitempty"` // AlbumMBID is the MusicBrainz release ID. diff --git a/plugins/pdk/go/sonicsimilarity/sonicsimilarity.go b/plugins/pdk/go/sonicsimilarity/sonicsimilarity.go index 7a2681f93..864d13311 100644 --- a/plugins/pdk/go/sonicsimilarity/sonicsimilarity.go +++ b/plugins/pdk/go/sonicsimilarity/sonicsimilarity.go @@ -11,6 +11,16 @@ import ( "github.com/navidrome/navidrome/plugins/pdk/go/pdk" ) +// ArtistRef is a reference to an artist with name and optional MBID. +type ArtistRef struct { + // ID is the internal Navidrome artist ID (if known). + ID string `json:"id,omitempty"` + // Name is the artist name. + Name string `json:"name"` + // MBID is the MusicBrainz ID for the artist. + MBID string `json:"mbid,omitempty"` +} + // FindSonicPathRequest represents the FindSonicPathRequest data structure. type FindSonicPathRequest struct { StartSong SongRef `json:"startSong"` @@ -38,6 +48,8 @@ type SongRef struct { Artist string `json:"artist,omitempty"` // ArtistMBID is the MusicBrainz artist ID. ArtistMBID string `json:"artistMbid,omitempty"` + // Artists is the full artist list; when set, takes precedence over Artist/ArtistMBID for matching. + Artists []ArtistRef `json:"artists,omitempty"` // Album is the album name. Album string `json:"album,omitempty"` // AlbumMBID is the MusicBrainz release ID. diff --git a/plugins/pdk/go/sonicsimilarity/sonicsimilarity_stub.go b/plugins/pdk/go/sonicsimilarity/sonicsimilarity_stub.go index 28d86301c..cf4cfc7ef 100644 --- a/plugins/pdk/go/sonicsimilarity/sonicsimilarity_stub.go +++ b/plugins/pdk/go/sonicsimilarity/sonicsimilarity_stub.go @@ -8,6 +8,16 @@ package sonicsimilarity +// ArtistRef is a reference to an artist with name and optional MBID. +type ArtistRef struct { + // ID is the internal Navidrome artist ID (if known). + ID string `json:"id,omitempty"` + // Name is the artist name. + Name string `json:"name"` + // MBID is the MusicBrainz ID for the artist. + MBID string `json:"mbid,omitempty"` +} + // FindSonicPathRequest represents the FindSonicPathRequest data structure. type FindSonicPathRequest struct { StartSong SongRef `json:"startSong"` @@ -35,6 +45,8 @@ type SongRef struct { Artist string `json:"artist,omitempty"` // ArtistMBID is the MusicBrainz artist ID. ArtistMBID string `json:"artistMbid,omitempty"` + // Artists is the full artist list; when set, takes precedence over Artist/ArtistMBID for matching. + Artists []ArtistRef `json:"artists,omitempty"` // Album is the album name. Album string `json:"album,omitempty"` // AlbumMBID is the MusicBrainz release ID. diff --git a/plugins/pdk/rust/nd-pdk-capabilities/src/metadata.rs b/plugins/pdk/rust/nd-pdk-capabilities/src/metadata.rs index 463e52c37..1a51ad9fd 100644 --- a/plugins/pdk/rust/nd-pdk-capabilities/src/metadata.rs +++ b/plugins/pdk/rust/nd-pdk-capabilities/src/metadata.rs @@ -251,6 +251,9 @@ pub struct SongRef { /// ArtistMBID is the MusicBrainz artist ID. #[serde(default, skip_serializing_if = "String::is_empty")] pub artist_mbid: String, + /// Artists is the full artist list; when set, takes precedence over Artist/ArtistMBID for matching. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub artists: Vec, /// Album is the album name. #[serde(default, skip_serializing_if = "String::is_empty")] pub album: String, diff --git a/plugins/pdk/rust/nd-pdk-capabilities/src/sonicsimilarity.rs b/plugins/pdk/rust/nd-pdk-capabilities/src/sonicsimilarity.rs index eb2868929..81b163e5f 100644 --- a/plugins/pdk/rust/nd-pdk-capabilities/src/sonicsimilarity.rs +++ b/plugins/pdk/rust/nd-pdk-capabilities/src/sonicsimilarity.rs @@ -18,6 +18,20 @@ fn is_zero_u64(value: &u64) -> bool { *value == 0 } fn is_zero_f32(value: &f32) -> bool { *value == 0.0 } #[allow(dead_code)] fn is_zero_f64(value: &f64) -> bool { *value == 0.0 } +/// ArtistRef is a reference to an artist with name and optional MBID. +#[derive(Debug, Clone, Default, Serialize, Deserialize)] +#[serde(rename_all = "camelCase")] +pub struct ArtistRef { + /// ID is the internal Navidrome artist ID (if known). + #[serde(default, skip_serializing_if = "String::is_empty")] + pub id: String, + /// Name is the artist name. + #[serde(default)] + pub name: String, + /// MBID is the MusicBrainz ID for the artist. + #[serde(default, skip_serializing_if = "String::is_empty")] + pub mbid: String, +} /// FindSonicPathRequest represents the FindSonicPathRequest data structure. #[derive(Debug, Clone, Default, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] @@ -60,6 +74,9 @@ pub struct SongRef { /// ArtistMBID is the MusicBrainz artist ID. #[serde(default, skip_serializing_if = "String::is_empty")] pub artist_mbid: String, + /// Artists is the full artist list; when set, takes precedence over Artist/ArtistMBID for matching. + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub artists: Vec, /// Album is the album name. #[serde(default, skip_serializing_if = "String::is_empty")] pub album: String, diff --git a/plugins/sonic_similarity_adapter.go b/plugins/sonic_similarity_adapter.go index ff399680e..e29684512 100644 --- a/plugins/sonic_similarity_adapter.go +++ b/plugins/sonic_similarity_adapter.go @@ -72,6 +72,9 @@ func mediaFileToSongRef(mf *model.MediaFile) capabilities.SongRef { AlbumMBID: mf.MbzAlbumID, Duration: mf.Duration, } + for _, p := range mf.Participants[model.RoleArtist] { + ref.Artists = append(ref.Artists, capabilities.ArtistRef{ID: p.ID, Name: p.Name, MBID: p.MbzArtistID}) + } if isrcs := mf.Tags.Values(model.TagISRC); len(isrcs) > 0 { ref.ISRC = isrcs[0] } diff --git a/plugins/sonic_similarity_adapter_test.go b/plugins/sonic_similarity_adapter_test.go index daea04761..1b8d4efa4 100644 --- a/plugins/sonic_similarity_adapter_test.go +++ b/plugins/sonic_similarity_adapter_test.go @@ -5,6 +5,7 @@ package plugins import ( "github.com/navidrome/navidrome/core/sonic" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/plugins/capabilities" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -108,3 +109,24 @@ var _ = Describe("SonicSimilarityPlugin error handling", Ordered, func() { Expect(err.Error()).To(ContainSubstring("simulated plugin error")) }) }) + +var _ = Describe("mediaFileToSongRef multi-artist", func() { + It("fills Artists (with IDs) from role=artist participants", func() { + mf := &model.MediaFile{ID: "x", Title: "Collab", Participants: model.Participants{ + model.RoleArtist: model.ParticipantList{ + {Artist: model.Artist{ID: "ar-drake", Name: "Drake", MbzArtistID: "m-drake"}}, + {Artist: model.Artist{ID: "ar-future", Name: "Future", MbzArtistID: "m-future"}}, + }, + }} + ref := mediaFileToSongRef(mf) + Expect(ref.Artists).To(Equal([]capabilities.ArtistRef{ + {ID: "ar-drake", Name: "Drake", MBID: "m-drake"}, + {ID: "ar-future", Name: "Future", MBID: "m-future"}, + })) + }) + It("leaves Artists nil when the track has no role=artist participants", func() { + mf := &model.MediaFile{ID: "x", Title: "Solo", Artist: "Drake"} + ref := mediaFileToSongRef(mf) + Expect(ref.Artists).To(BeNil()) + }) +})