feat(plugins): add similar songs retrieval functions and improve duration consistency (#4933)

* feat: add duration filtering for similar songs matching Signed-off-by: Deluan <deluan@navidrome.org> * test: refactor expectations for similar songs in provider matching tests Signed-off-by: Deluan <deluan@navidrome.org> * feat(plugins): add functions to retrieve similar songs by track, album, and artist Signed-off-by: Deluan <deluan@navidrome.org> * fix(plugins): support uint32 in ndpgen Signed-off-by: Deluan <deluan@navidrome.org> * fix(plugins): update duration field to use seconds as float instead of milliseconds as uint32 Signed-off-by: Deluan <deluan@navidrome.org> * fix: add helper functions for Rust's skip_serializing_if with numeric types Signed-off-by: Deluan <deluan@navidrome.org> * feat(provider): enhance track matching logic to fallback to title match when duration-filtered tracks fail --------- Signed-off-by: Deluan <deluan@navidrome.org>
2026-01-26 18:28:41 -05:00
parent 4d4740b83b
commit fda35dd8ce
20 changed files with 1147 additions and 70 deletions
@@ -40,6 +40,7 @@ type Song struct {
 	ArtistMBID string
 	Album      string
 	AlbumMBID  string
+	Duration   uint32 // Duration in milliseconds, 0 means unknown
 }

 var (
@@ -3,6 +3,7 @@ package external
 import (
 	"context"
 	"fmt"
+	"math"

 	"github.com/Masterminds/squirrel"
 	"github.com/navidrome/navidrome/conf"
@@ -12,6 +13,11 @@ import (
 	"github.com/xrash/smetrics"
 )

+// durationToleranceSec is the maximum allowed difference in seconds when
+// matching tracks by duration. A tolerance of 3 seconds accounts for minor
+// encoding differences between sources.
+const durationToleranceSec = 3
+
 // matchSongsToLibrary matches agent song results to local library tracks using a multi-phase
 // matching algorithm that prioritizes accuracy over recall.
 //
@@ -175,6 +181,7 @@ type songQuery struct {
 	artistMBID string // MusicBrainz Artist ID (optional, for higher specificity matching)
 	album      string // Sanitized album name (optional, for specificity scoring)
 	albumMBID  string // MusicBrainz Album ID (optional, for highest specificity matching)
+	durationMs uint32 // Duration in milliseconds (0 means unknown, skip duration filtering)
 }

 // matchScore combines title/album similarity with metadata specificity for ranking matches
@@ -282,12 +289,50 @@ func (e *provider) loadTracksByTitleAndArtist(ctx context.Context, songs []agent
 	return matches, nil
 }

+// durationMatches checks if a track's duration is within tolerance of the target duration.
+// Returns true if durationMs is 0 (unknown) or if the difference is within durationToleranceSec.
+func durationMatches(durationMs uint32, mediaFileDurationSec float32) bool {
+	if durationMs <= 0 {
+		return true // Unknown duration matches anything
+	}
+	durationSec := float64(durationMs) / 1000.0
+	diff := math.Abs(durationSec - float64(mediaFileDurationSec))
+	return diff <= durationToleranceSec
+}
+
 // findBestMatch finds the best matching track using combined title/album similarity and specificity scoring.
+// When duration is known (durationMs > 0), it acts as a top-priority filter:
+// - First, only tracks with matching duration (±3 seconds) are considered
+// - If no title match is found among duration-filtered tracks, falls back to matching all tracks
 // A track must meet the threshold for title similarity, then the best match is chosen by:
 // 1. Highest title similarity
 // 2. Highest specificity level
 // 3. Highest album similarity (as final tiebreaker)
 func (e *provider) findBestMatch(q songQuery, tracks model.MediaFiles, threshold float64) (model.MediaFile, bool) {
+	// If duration is known, try to find matches among duration-filtered tracks first
+	if q.durationMs > 0 {
+		var durationFiltered model.MediaFiles
+		for _, mf := range tracks {
+			if durationMatches(q.durationMs, mf.Duration) {
+				durationFiltered = append(durationFiltered, mf)
+			}
+		}
+		// If we have duration-filtered candidates, try matching those first
+		if len(durationFiltered) > 0 {
+			if mf, found := findBestMatchInTracks(q, durationFiltered, threshold); found {
+				return mf, true
+			}
+		}
+		// Fall through to try all tracks if no duration-filtered match found
+	}
+
+	return findBestMatchInTracks(q, tracks, threshold)
+}
+
+// findBestMatchInTracks performs the core matching logic on a set of tracks.
+// It finds the track with the best combined score based on title similarity,
+// specificity level, and album similarity.
+func findBestMatchInTracks(q songQuery, tracks model.MediaFiles, threshold float64) (model.MediaFile, bool) {
 	var bestMatch model.MediaFile
 	bestScore := matchScore{titleSimilarity: -1}
 	found := false
@@ -338,6 +383,7 @@ func (e *provider) buildTitleQueries(songs []agents.Song, idMatches, mbidMatches
 			artistMBID: s.ArtistMBID,
 			album:      str.SanitizeFieldForSorting(s.Album),
 			albumMBID:  s.AlbumMBID,
+			durationMs: s.Duration,
 		})
 	}
 	return queries
@@ -41,6 +41,26 @@ var _ = Describe("Provider - Song Matching", func() {
 		provider = NewProvider(ds, agentsCombined)
 	})

+	// Shared helper for tests that only need artist track queries (no ID/MBID matching)
+	setupSimilarSongsExpectations := func(returnedSongs []agents.Song, artistTracks model.MediaFiles) {
+		agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
+			Return(returnedSongs, nil).Once()
+
+		// loadTracksByTitleAndArtist - queries by artist name
+		mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
+			and, ok := opt.Filters.(squirrel.And)
+			if !ok || len(and) < 2 {
+				return false
+			}
+			eq, hasEq := and[0].(squirrel.Eq)
+			if !hasEq {
+				return false
+			}
+			_, hasArtist := eq["order_artist_name"]
+			return hasArtist
+		})).Return(artistTracks, nil).Maybe()
+	}
+
 	Describe("matchSongsToLibrary priority matching", func() {
 		var track model.MediaFile

@@ -261,26 +281,6 @@ var _ = Describe("Provider - Song Matching", func() {
 			mediaFileRepo.On("Get", "track-1").Return(&track, nil).Once()
 		})

-		setupFuzzyExpectations := func(returnedSongs []agents.Song, artistTracks model.MediaFiles) {
-			agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything).
-				Return(returnedSongs, nil).Once()
-
-			// loadTracksByTitleAndArtist now queries by artist in a single pass
-			// Note: loadTracksByID and loadTracksByMBID return early when no IDs/MBIDs
-			mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool {
-				and, ok := opt.Filters.(squirrel.And)
-				if !ok || len(and) < 2 {
-					return false
-				}
-				eq, hasEq := and[0].(squirrel.Eq)
-				if !hasEq {
-					return false
-				}
-				_, hasArtist := eq["order_artist_name"]
-				return hasArtist
-			})).Return(artistTracks, nil).Maybe()
-		}
-
 		Context("with default threshold (85%)", func() {
 			It("matches songs with remastered suffix", func() {
 				conf.Server.SimilarSongsMatchThreshold = 85
@@ -294,7 +294,7 @@ var _ = Describe("Provider - Song Matching", func() {
 					{ID: "remastered", Title: "Paranoid Android - Remastered", Artist: "Radiohead"},
 				}

-				setupFuzzyExpectations(returnedSongs, artistTracks)
+				setupSimilarSongsExpectations(returnedSongs, artistTracks)

 				songs, err := provider.SimilarSongs(ctx, "track-1", 5)

@@ -313,7 +313,7 @@ var _ = Describe("Provider - Song Matching", func() {
 					{ID: "live", Title: "Bohemian Rhapsody (Live)", Artist: "Queen"},
 				}

-				setupFuzzyExpectations(returnedSongs, artistTracks)
+				setupSimilarSongsExpectations(returnedSongs, artistTracks)

 				songs, err := provider.SimilarSongs(ctx, "track-1", 5)

@@ -334,7 +334,7 @@ var _ = Describe("Provider - Song Matching", func() {
 					{ID: "different2", Title: "Here Comes The Sun", Artist: "The Beatles"},
 				}

-				setupFuzzyExpectations(returnedSongs, artistTracks)
+				setupSimilarSongsExpectations(returnedSongs, artistTracks)

 				songs, err := provider.SimilarSongs(ctx, "track-1", 5)

@@ -355,7 +355,7 @@ var _ = Describe("Provider - Song Matching", func() {
 					{ID: "remastered", Title: "Paranoid Android - Remastered", Artist: "Radiohead"},
 				}

-				setupFuzzyExpectations(returnedSongs, artistTracks)
+				setupSimilarSongsExpectations(returnedSongs, artistTracks)

 				songs, err := provider.SimilarSongs(ctx, "track-1", 5)

@@ -375,7 +375,7 @@ var _ = Describe("Provider - Song Matching", func() {
 					{ID: "extended", Title: "Song (Extended Mix)", Artist: "Artist"},
 				}

-				setupFuzzyExpectations(returnedSongs, artistTracks)
+				setupSimilarSongsExpectations(returnedSongs, artistTracks)

 				songs, err := provider.SimilarSongs(ctx, "track-1", 5)

@@ -401,7 +401,7 @@ var _ = Describe("Provider - Song Matching", func() {
 					ID: "wrong", Title: "Bohemian Rhapsody", Artist: "Queen", Album: "Greatest Hits",
 				}

-				setupFuzzyExpectations(returnedSongs, model.MediaFiles{wrongMatch, correctMatch})
+				setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{wrongMatch, correctMatch})

 				songs, err := provider.SimilarSongs(ctx, "track-1", 5)

@@ -424,7 +424,7 @@ var _ = Describe("Provider - Song Matching", func() {
 					ID: "wrong", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "101",
 				}

-				setupFuzzyExpectations(returnedSongs, model.MediaFiles{wrongMatch, correctMatch})
+				setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{wrongMatch, correctMatch})

 				songs, err := provider.SimilarSongs(ctx, "track-1", 5)

@@ -446,7 +446,7 @@ var _ = Describe("Provider - Song Matching", func() {
 					ID: "fuzzy", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator (Deluxe Edition)",
 				}

-				setupFuzzyExpectations(returnedSongs, model.MediaFiles{fuzzyMatch, exactMatch})
+				setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{fuzzyMatch, exactMatch})

 				songs, err := provider.SimilarSongs(ctx, "track-1", 5)

@@ -457,4 +457,171 @@ var _ = Describe("Provider - Song Matching", func() {
 			})
 		})
 	})
+
+	Describe("Duration filtering", func() {
+		var track model.MediaFile
+
+		BeforeEach(func() {
+			DeferCleanup(configtest.SetupConfig())
+			conf.Server.SimilarSongsMatchThreshold = 100 // Exact title match for predictable tests
+
+			track = model.MediaFile{ID: "track-1", Title: "Test Track", Artist: "Test Artist"}
+
+			// Setup for GetEntityByID to return the track
+			artistRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Once()
+			albumRepo.On("Get", "track-1").Return(nil, model.ErrNotFound).Once()
+			mediaFileRepo.On("Get", "track-1").Return(&track, nil).Once()
+		})
+
+		Context("when agent provides duration", func() {
+			It("prefers tracks with matching duration", func() {
+				// Agent returns song with duration 180000ms (180 seconds)
+				returnedSongs := []agents.Song{
+					{Name: "Similar Song", Artist: "Test Artist", Duration: 180000},
+				}
+				// Library has two versions: one matching duration, one not
+				correctMatch := model.MediaFile{
+					ID: "correct", Title: "Similar Song", Artist: "Test Artist", Duration: 180.0,
+				}
+				wrongDuration := model.MediaFile{
+					ID: "wrong", Title: "Similar Song", Artist: "Test Artist", Duration: 240.0,
+				}
+
+				setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{wrongDuration, correctMatch})
+
+				songs, err := provider.SimilarSongs(ctx, "track-1", 5)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(songs).To(HaveLen(1))
+				Expect(songs[0].ID).To(Equal("correct"))
+			})
+
+			It("matches within 3-second tolerance", func() {
+				// Agent returns song with duration 180000ms (180 seconds)
+				returnedSongs := []agents.Song{
+					{Name: "Similar Song", Artist: "Test Artist", Duration: 180000},
+				}
+				// Library has track with 182 seconds (within tolerance)
+				withinTolerance := model.MediaFile{
+					ID: "within-tolerance", Title: "Similar Song", Artist: "Test Artist", Duration: 182.5,
+				}
+
+				setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{withinTolerance})
+
+				songs, err := provider.SimilarSongs(ctx, "track-1", 5)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(songs).To(HaveLen(1))
+				Expect(songs[0].ID).To(Equal("within-tolerance"))
+			})
+
+			It("excludes tracks outside 3-second tolerance when other matches exist", func() {
+				// Agent returns song with duration 180000ms (180 seconds)
+				returnedSongs := []agents.Song{
+					{Name: "Similar Song", Artist: "Test Artist", Duration: 180000},
+				}
+				// Library has one within tolerance, one outside
+				withinTolerance := model.MediaFile{
+					ID: "within", Title: "Similar Song", Artist: "Test Artist", Duration: 181.0,
+				}
+				outsideTolerance := model.MediaFile{
+					ID: "outside", Title: "Similar Song", Artist: "Test Artist", Duration: 190.0,
+				}
+
+				setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{outsideTolerance, withinTolerance})
+
+				songs, err := provider.SimilarSongs(ctx, "track-1", 5)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(songs).To(HaveLen(1))
+				Expect(songs[0].ID).To(Equal("within"))
+			})
+
+			It("falls back to normal matching when no duration matches", func() {
+				// Agent returns song with duration 180000ms
+				returnedSongs := []agents.Song{
+					{Name: "Similar Song", Artist: "Test Artist", Duration: 180000},
+				}
+				// Library only has tracks with very different duration
+				differentDuration := model.MediaFile{
+					ID: "different", Title: "Similar Song", Artist: "Test Artist", Duration: 300.0,
+				}
+
+				setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{differentDuration})
+
+				songs, err := provider.SimilarSongs(ctx, "track-1", 5)
+
+				Expect(err).ToNot(HaveOccurred())
+				// Should fall back and return the track despite duration mismatch
+				Expect(songs).To(HaveLen(1))
+				Expect(songs[0].ID).To(Equal("different"))
+			})
+
+			It("falls back to title match when duration-filtered tracks fail title threshold", func() {
+				// Agent returns "Similar Song" with duration 180000ms
+				returnedSongs := []agents.Song{
+					{Name: "Similar Song", Artist: "Test Artist", Duration: 180000},
+				}
+				// Library has:
+				// - differentTitle: matches duration but has different title (won't pass title threshold)
+				// - correctTitle: doesn't match duration but has correct title (should be found via fallback)
+				differentTitle := model.MediaFile{
+					ID: "wrong-title", Title: "Different Song", Artist: "Test Artist", Duration: 180.0,
+				}
+				correctTitle := model.MediaFile{
+					ID: "correct-title", Title: "Similar Song", Artist: "Test Artist", Duration: 300.0,
+				}
+
+				setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{differentTitle, correctTitle})
+
+				songs, err := provider.SimilarSongs(ctx, "track-1", 5)
+
+				Expect(err).ToNot(HaveOccurred())
+				// Should fall back to all tracks and find the title match
+				Expect(songs).To(HaveLen(1))
+				Expect(songs[0].ID).To(Equal("correct-title"))
+			})
+		})
+
+		Context("when agent does not provide duration", func() {
+			It("matches without duration filtering (duration=0)", func() {
+				// Agent returns song without duration
+				returnedSongs := []agents.Song{
+					{Name: "Similar Song", Artist: "Test Artist", Duration: 0},
+				}
+				// Library tracks with various durations should all be candidates
+				anyTrack := model.MediaFile{
+					ID: "any", Title: "Similar Song", Artist: "Test Artist", Duration: 999.0,
+				}
+
+				setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{anyTrack})
+
+				songs, err := provider.SimilarSongs(ctx, "track-1", 5)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(songs).To(HaveLen(1))
+				Expect(songs[0].ID).To(Equal("any"))
+			})
+		})
+
+		Context("edge cases", func() {
+			It("handles very short songs with duration tolerance", func() {
+				// 30-second song with 1-second difference (within 3-second tolerance)
+				returnedSongs := []agents.Song{
+					{Name: "Short Song", Artist: "Test Artist", Duration: 30000},
+				}
+				shortTrack := model.MediaFile{
+					ID: "short", Title: "Short Song", Artist: "Test Artist", Duration: 31.0,
+				}
+
+				setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{shortTrack})
+
+				songs, err := provider.SimilarSongs(ctx, "track-1", 5)
+
+				Expect(err).ToNot(HaveOccurred())
+				Expect(songs).To(HaveLen(1))
+				Expect(songs[0].ID).To(Equal("short"))
+			})
+		})
+	})
 })