From 9b0bfc606bc5978032702a259de8defd820e474d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Fri, 10 Apr 2026 19:29:20 -0400 Subject: [PATCH 01/18] fix(subsonic): always emit required `created` field on AlbumID3 (#5340) * fix(subsonic): always emit required `created` field on AlbumID3 Strict OpenSubsonic clients (e.g. Navic via dev.zt64.subsonic) reject search3/getAlbum/getAlbumList2 responses that omit the `created` field, which the spec marks as required. Navidrome was dropping it whenever the album's CreatedAt was zero. Root cause was threefold: 1. buildAlbumID3/childFromAlbum conditionally emitted `created`, so a zero CreatedAt became a missing JSON key. 2. ToAlbum's `older()` helper treated a zero BirthTime as the minimum, so a single track with missing filesystem birth time could poison the album aggregation. 3. phase_1_folders' CopyAttributes copied `created_at` from the previous album row unconditionally, propagating an already-zero value forward on every metadata-driven album ID change. Since sql_base_repository drops `created_at` on UPDATE, a poisoned row could never self-heal. Fixes: - Always emit `created`, falling back to UpdatedAt/ImportedAt when CreatedAt is zero. Adds albumCreatedAt() helper used by both buildAlbumID3 and childFromAlbum. - Guard `older()` against a zero second argument. - Skip the CopyAttributes call in phase_1_folders when the previous album's created_at is zero, so the freshly-computed value survives. - New migration backfills existing broken rows from media_file.birth_time (falling back to updated_at). Tested against a real DB: repaired 605/6922 affected rows, no side effects on healthy rows. Signed-off-by: Deluan * refactor(subsonic): return albumCreatedAt by value to avoid heap escape Returning *time.Time from albumCreatedAt caused Go escape analysis to move the entire model.Album parameter to the heap, since the returned pointer aliased a field of the value receiver. For hot endpoints like getAlbumList2 and search3, this meant one full-struct heap allocation per album result. Return time.Time by value and let callers wrap it with gg.P() to take the address locally. Only the small time.Time value escapes; the model.Album struct stays on the stack. Also corrects the doc comment to reflect the actual guarantee ("best-effort" rather than "non-zero"), matching the test case that exercises the all-zero fallback. --------- Signed-off-by: Deluan --- ...260410201914_fix_zero_album_created_at.sql | 22 +++++++++++++ model/mediafile.go | 3 ++ model/mediafile_test.go | 14 ++++++++ persistence/album_repository.go | 12 ++++++- persistence/album_repository_test.go | 26 +++++++++++++++ server/subsonic/helpers.go | 22 ++++++++++--- server/subsonic/helpers_test.go | 32 +++++++++++++++++++ 7 files changed, 126 insertions(+), 5 deletions(-) create mode 100644 db/migrations/20260410201914_fix_zero_album_created_at.sql diff --git a/db/migrations/20260410201914_fix_zero_album_created_at.sql b/db/migrations/20260410201914_fix_zero_album_created_at.sql new file mode 100644 index 000000000..ff47eb95f --- /dev/null +++ b/db/migrations/20260410201914_fix_zero_album_created_at.sql @@ -0,0 +1,22 @@ +-- +goose Up + +-- Backfill album.created_at for rows poisoned by early scanner versions or +-- propagated via CopyAttributes during metadata-driven ID changes. Prefer the +-- oldest valid birth_time from the album's media files, fall back to updated_at. +UPDATE album +SET created_at = COALESCE( + (SELECT MIN(birth_time) + FROM media_file + WHERE media_file.album_id = album.id + AND birth_time IS NOT NULL + AND birth_time != '' + AND birth_time NOT LIKE '0001-%'), + updated_at +) +WHERE created_at IS NULL + OR created_at = '' + OR created_at LIKE '0001-%'; + +-- +goose Down + +SELECT 1; diff --git a/model/mediafile.go b/model/mediafile.go index ec83b76fd..6be8402ae 100644 --- a/model/mediafile.go +++ b/model/mediafile.go @@ -361,6 +361,9 @@ func older(t1, t2 time.Time) time.Time { if t1.IsZero() { return t2 } + if t2.IsZero() { + return t1 + } if t1.After(t2) { return t2 } diff --git a/model/mediafile_test.go b/model/mediafile_test.go index 038ac93d5..8b0c13da2 100644 --- a/model/mediafile_test.go +++ b/model/mediafile_test.go @@ -119,6 +119,20 @@ var _ = Describe("MediaFiles", func() { Expect(a.MinYear).To(Equal(1999)) }) }) + Context("CreatedAt aggregation", func() { + It("ignores zero BirthTime values when computing the oldest", func() { + mfs = MediaFiles{ + {BirthTime: t("2022-12-19 08:30")}, + {BirthTime: time.Time{}}, + {BirthTime: t("2022-12-18 10:00")}, + } + Expect(mfs.ToAlbum().CreatedAt).To(Equal(t("2022-12-18 10:00"))) + }) + It("returns zero when all BirthTime values are zero", func() { + mfs = MediaFiles{{BirthTime: time.Time{}}, {BirthTime: time.Time{}}} + Expect(mfs.ToAlbum().CreatedAt).To(BeZero()) + }) + }) }) When("we have multiple songs with same dates", func() { BeforeEach(func() { diff --git a/persistence/album_repository.go b/persistence/album_repository.go index c51a5beb1..99ed10877 100644 --- a/persistence/album_repository.go +++ b/persistence/album_repository.go @@ -252,7 +252,17 @@ func (r *albumRepository) CopyAttributes(fromID, toID string, columns ...string) } to := make(map[string]any) for _, col := range columns { - to[col] = from[col] + v := from[col] + // created_at is aggregated from song birth_times and must never be + // overwritten with a zero/poisoned value, or it propagates forward on + // every metadata-driven album ID change. + if col == "created_at" && (!v.Valid || v.String == "" || strings.HasPrefix(v.String, "0001-")) { + continue + } + to[col] = v + } + if len(to) == 0 { + return nil } _, err = r.executeSQL(Update(r.tableName).SetMap(to).Where(Eq{"id": toID})) return err diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go index 2792cec97..a6270933f 100644 --- a/persistence/album_repository_test.go +++ b/persistence/album_repository_test.go @@ -41,6 +41,32 @@ var _ = Describe("AlbumRepository", func() { }) }) + Describe("CopyAttributes", func() { + var srcTime, dstTime time.Time + BeforeEach(func() { + srcTime = time.Date(2020, 1, 2, 3, 4, 5, 0, time.UTC) + dstTime = time.Date(2024, 6, 7, 8, 9, 10, 0, time.UTC) + Expect(albumRepo.Put(&model.Album{ID: "copy-src", Name: "src", LibraryID: 1, CreatedAt: srcTime})).To(Succeed()) + Expect(albumRepo.Put(&model.Album{ID: "copy-dst", Name: "dst", LibraryID: 1, CreatedAt: dstTime})).To(Succeed()) + Expect(albumRepo.Put(&model.Album{ID: "copy-zero", Name: "zero", LibraryID: 1})).To(Succeed()) + DeferCleanup(func() { + _, _ = albumRepo.executeSQL(squirrel.Delete("album").Where(squirrel.Eq{"id": []string{"copy-src", "copy-dst", "copy-zero"}})) + }) + }) + It("copies a valid created_at from source to destination", func() { + Expect(albumRepo.CopyAttributes("copy-src", "copy-dst", "created_at")).To(Succeed()) + got, err := albumRepo.Get("copy-dst") + Expect(err).ToNot(HaveOccurred()) + Expect(got.CreatedAt).To(BeTemporally("~", srcTime, time.Second)) + }) + It("leaves destination untouched when source created_at is zero", func() { + Expect(albumRepo.CopyAttributes("copy-zero", "copy-dst", "created_at")).To(Succeed()) + got, err := albumRepo.Get("copy-dst") + Expect(err).ToNot(HaveOccurred()) + Expect(got.CreatedAt).To(BeTemporally("~", dstTime, time.Second)) + }) + }) + Describe("GetAll", func() { var GetAll = func(opts ...model.QueryOptions) (model.Albums, error) { albums, err := albumRepo.GetAll(opts...) diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index e930aa630..ffa10898e 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -10,6 +10,7 @@ import ( "slices" "sort" "strings" + "time" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" @@ -17,6 +18,7 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/subsonic/responses" + . "github.com/navidrome/navidrome/utils/gg" "github.com/navidrome/navidrome/utils/number" "github.com/navidrome/navidrome/utils/req" "github.com/navidrome/navidrome/utils/slice" @@ -317,6 +319,20 @@ func sanitizeSlashes(target string) string { return strings.ReplaceAll(target, "/", "_") } +// albumCreatedAt returns a best-effort timestamp for the album's `created` +// field, which is required by the OpenSubsonic spec but may be zero on legacy +// DB rows. Falls back to UpdatedAt → ImportedAt; can still return zero if all +// three are unset. +func albumCreatedAt(al model.Album) time.Time { + if !al.CreatedAt.IsZero() { + return al.CreatedAt + } + if !al.UpdatedAt.IsZero() { + return al.UpdatedAt + } + return al.ImportedAt +} + func childFromAlbum(ctx context.Context, al model.Album) responses.Child { child := responses.Child{} child.Id = al.ID @@ -329,7 +345,7 @@ func childFromAlbum(ctx context.Context, al model.Album) responses.Child { child.Year = int32(cmp.Or(al.MaxOriginalYear, al.MaxYear)) child.Genre = al.Genre child.CoverArt = al.CoverArtID().String() - child.Created = &al.CreatedAt + child.Created = P(albumCreatedAt(al)) child.Parent = al.AlbumArtistID child.ArtistId = al.AlbumArtistID child.Duration = int32(al.Duration) @@ -421,9 +437,7 @@ func buildAlbumID3(ctx context.Context, album model.Album) responses.AlbumID3 { dir.PlayCount = album.PlayCount dir.Year = int32(cmp.Or(album.MaxOriginalYear, album.MaxYear)) dir.Genre = album.Genre - if !album.CreatedAt.IsZero() { - dir.Created = &album.CreatedAt - } + dir.Created = P(albumCreatedAt(album)) if album.Starred { dir.Starred = album.StarredAt } diff --git a/server/subsonic/helpers_test.go b/server/subsonic/helpers_test.go index 4eb756b98..abf6116f3 100644 --- a/server/subsonic/helpers_test.go +++ b/server/subsonic/helpers_test.go @@ -571,6 +571,38 @@ var _ = Describe("helpers", func() { }) }) + Describe("buildAlbumID3 Created field", func() { + It("uses CreatedAt when set", func() { + t := time.Date(2020, 1, 2, 3, 4, 5, 0, time.UTC) + al := model.Album{ID: "a1", Name: "A", CreatedAt: t} + dir := buildAlbumID3(ctx, al) + Expect(dir.Created).ToNot(BeNil()) + Expect(*dir.Created).To(Equal(t)) + }) + + It("falls back to UpdatedAt when CreatedAt is zero", func() { + updated := time.Date(2019, 5, 6, 7, 8, 9, 0, time.UTC) + al := model.Album{ID: "a2", Name: "A", UpdatedAt: updated} + dir := buildAlbumID3(ctx, al) + Expect(dir.Created).ToNot(BeNil()) + Expect(*dir.Created).To(Equal(updated)) + }) + + It("falls back to ImportedAt when CreatedAt and UpdatedAt are zero", func() { + imported := time.Date(2021, 8, 9, 10, 11, 12, 0, time.UTC) + al := model.Album{ID: "a3", Name: "A", ImportedAt: imported} + dir := buildAlbumID3(ctx, al) + Expect(dir.Created).ToNot(BeNil()) + Expect(*dir.Created).To(Equal(imported)) + }) + + It("never leaves Created nil even when all timestamps are zero", func() { + al := model.Album{ID: "a4", Name: "A"} + dir := buildAlbumID3(ctx, al) + Expect(dir.Created).ToNot(BeNil()) + }) + }) + Describe("EnableAverageRating config", func() { It("excludes averageRating when disabled", func() { conf.Server.Subsonic.EnableAverageRating = false From ab2f1b45de4fd11fa674168b2945040ef0bd0a2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Fri, 10 Apr 2026 21:59:49 -0400 Subject: [PATCH 02/18] perf: reduce hot-path heap escapes from value-param pointer aliasing (#5342) * perf(subsonic): keep album/mediafile params on stack in response helpers Two helpers were forcing their entire value parameter onto the heap via pointer-to-field aliasing, adding one full-struct heap allocation per response item on hot Subsonic endpoints (search3, getAlbumList2, etc.). - childFromMediaFile assigned &mf.BirthTime to the returned Child, pulling the whole ~1KB model.MediaFile to the heap on every call. - buildDiscSubtitles passed &a.UpdatedAt to NewArtworkID inside a loop, pulling the whole model.Album to the heap on every album with discs. Both now copy the time.Time to a stack-local and use gg.P / &local so only the small time.Time escapes. Verified via go build -gcflags=-m=2: moved to heap: mf and moved to heap: a are gone at these sites. * perf(metadata): avoid per-track closure allocations in PID computation createGetPID was a factory that returned nested closures capturing mf model.MediaFile (~992 bytes) by reference. Since it is called three times per track during scans (trackPID, albumID, artistID), every track triggered the allocation of three closures plus a heap copy of the full MediaFile. Refactor the body into package-level functions (computePID, getPIDAttr) that take hash as an explicit parameter and the inner slice.Map callback to an indexed for loop, removing the closure-capture of mf entirely. trackPID/albumID/artistID now call computePID directly. The tiny createGetPID wrapper was kept only for tests; move the closure-building into the test file so production has no dead API. Verified via go build -gcflags=-m=2 on model/metadata: no "moved to heap: mf" anywhere in persistent_ids.go, and the callers in map_mediafile.go / map_participants.go no longer heap-promote their MediaFile argument. --- model/metadata/persistent_ids.go | 115 +++++++++++++------------- model/metadata/persistent_ids_test.go | 11 +-- server/subsonic/helpers.go | 7 +- 3 files changed, 67 insertions(+), 66 deletions(-) diff --git a/model/metadata/persistent_ids.go b/model/metadata/persistent_ids.go index 70dfe0532..db315dc6b 100644 --- a/model/metadata/persistent_ids.go +++ b/model/metadata/persistent_ids.go @@ -12,88 +12,85 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/id" "github.com/navidrome/navidrome/utils" - "github.com/navidrome/navidrome/utils/slice" "github.com/navidrome/navidrome/utils/str" ) type hashFunc = func(...string) string -// createGetPID returns a function that calculates the persistent ID for a given spec, getting the referenced values from the metadata -// The spec is a pipe-separated list of fields, where each field is a comma-separated list of attributes -// Attributes can be either tags or some processed values like folder, albumid, albumartistid, etc. -// For each field, it gets all its attributes values and concatenates them, then hashes the result. -// If a field is empty, it is skipped and the function looks for the next field. -type getPIDFunc = func(mf model.MediaFile, md Metadata, spec string, prependLibId bool) string - -func createGetPID(hash hashFunc) getPIDFunc { - var getPID getPIDFunc - getAttr := func(mf model.MediaFile, md Metadata, attr string, prependLibId bool, spec string) string { - attr = strings.TrimSpace(strings.ToLower(attr)) - switch attr { - case "albumid": - if spec == conf.Server.PID.Album { - log.Error("Recursive PID definition detected, ignoring `albumid`", "spec", spec) - return "" +// computePID calculates the persistent ID for a given spec. The spec is a +// pipe-separated list of fields, where each field is a comma-separated list of +// attributes. Attributes can be either tags or processed values like folder, +// albumid, albumartistid, etc. For each field, it gets all its attribute values +// and concatenates them, then hashes the result. If a field is empty, it is +// skipped and the function looks for the next field. +// +// Taking hash as a parameter (instead of closing over it in a factory) keeps +// mf on the stack: closing over mf would force the whole ~1KB MediaFile to the +// heap on every call. +func computePID(mf model.MediaFile, md Metadata, spec string, prependLibId bool, hash hashFunc) string { + switch spec { + case "track_legacy": + return legacyTrackID(mf, prependLibId) + case "album_legacy": + return legacyAlbumID(mf, md, prependLibId) + } + pid := "" + fields := strings.SplitSeq(spec, "|") + for field := range fields { + attributes := strings.Split(field, ",") + values := make([]string, len(attributes)) + hasValue := false + for i, attr := range attributes { + v := getPIDAttr(mf, md, attr, prependLibId, spec, hash) + if v != "" { + hasValue = true } - return getPID(mf, md, conf.Server.PID.Album, prependLibId) - case "folder": - return filepath.Dir(mf.Path) - case "albumartistid": - return hash(str.Clear(strings.ToLower(mf.AlbumArtist))) - case "title": - return mf.Title - case "album": - return str.Clear(strings.ToLower(md.String(model.TagAlbum))) + values[i] = v + } + if hasValue { + pid += strings.Join(values, "\\") + break } - return md.String(model.TagName(attr)) } - getPID = func(mf model.MediaFile, md Metadata, spec string, prependLibId bool) string { - pid := "" - fields := strings.SplitSeq(spec, "|") - for field := range fields { - attributes := strings.Split(field, ",") - hasValue := false - values := slice.Map(attributes, func(attr string) string { - v := getAttr(mf, md, attr, prependLibId, spec) - if v != "" { - hasValue = true - } - return v - }) - if hasValue { - pid += strings.Join(values, "\\") - break - } - } - if prependLibId { - pid = fmt.Sprintf("%d\\%s", mf.LibraryID, pid) - } - return hash(pid) + if prependLibId { + pid = fmt.Sprintf("%d\\%s", mf.LibraryID, pid) } + return hash(pid) +} - return func(mf model.MediaFile, md Metadata, spec string, prependLibId bool) string { - switch spec { - case "track_legacy": - return legacyTrackID(mf, prependLibId) - case "album_legacy": - return legacyAlbumID(mf, md, prependLibId) +func getPIDAttr(mf model.MediaFile, md Metadata, attr string, prependLibId bool, spec string, hash hashFunc) string { + attr = strings.TrimSpace(strings.ToLower(attr)) + switch attr { + case "albumid": + if spec == conf.Server.PID.Album { + log.Error("Recursive PID definition detected, ignoring `albumid`", "spec", spec) + return "" } - return getPID(mf, md, spec, prependLibId) + return computePID(mf, md, conf.Server.PID.Album, prependLibId, hash) + case "folder": + return filepath.Dir(mf.Path) + case "albumartistid": + return hash(str.Clear(strings.ToLower(mf.AlbumArtist))) + case "title": + return mf.Title + case "album": + return str.Clear(strings.ToLower(md.String(model.TagAlbum))) } + return md.String(model.TagName(attr)) } func (md Metadata) trackPID(mf model.MediaFile) string { - return createGetPID(id.NewHash)(mf, md, conf.Server.PID.Track, true) + return computePID(mf, md, conf.Server.PID.Track, true, id.NewHash) } func (md Metadata) albumID(mf model.MediaFile, pidConf string) string { - return createGetPID(id.NewHash)(mf, md, pidConf, true) + return computePID(mf, md, pidConf, true, id.NewHash) } // BFR Must be configurable? func (md Metadata) artistID(name string) string { mf := model.MediaFile{AlbumArtist: name} - return createGetPID(id.NewHash)(mf, md, "albumartistid", false) + return computePID(mf, md, "albumartistid", false, id.NewHash) } func (md Metadata) mapTrackTitle() string { diff --git a/model/metadata/persistent_ids_test.go b/model/metadata/persistent_ids_test.go index 9f1dacbd4..47f5ca63f 100644 --- a/model/metadata/persistent_ids_test.go +++ b/model/metadata/persistent_ids_test.go @@ -12,15 +12,16 @@ import ( var _ = Describe("getPID", func() { var ( - md Metadata - mf model.MediaFile - sum hashFunc - getPID getPIDFunc + md Metadata + mf model.MediaFile + sum hashFunc ) + getPID := func(mf model.MediaFile, md Metadata, spec string, prependLibId bool) string { + return computePID(mf, md, spec, prependLibId, sum) + } BeforeEach(func() { sum = func(s ...string) string { return "(" + strings.Join(s, ",") + ")" } - getPID = createGetPID(sum) }) Context("attributes are tags", func() { diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index ffa10898e..74d57ade4 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -217,7 +217,7 @@ func childFromMediaFile(ctx context.Context, mf model.MediaFile) responses.Child child.Path = fakePath(mf) } child.DiscNumber = int32(mf.DiscNumber) - child.Created = &mf.BirthTime + child.Created = P(mf.BirthTime) child.AlbumId = mf.AlbumID child.ArtistId = mf.ArtistID child.Type = "music" @@ -407,9 +407,12 @@ func buildDiscSubtitles(a model.Album) []responses.DiscTitle { return nil } var discTitles []responses.DiscTitle + // Hoist UpdatedAt to a single stack-local so &updatedAt doesn't force the + // whole model.Album parameter onto the heap. + updatedAt := a.UpdatedAt for num, title := range a.Discs { artID := model.NewArtworkID(model.KindDiscArtwork, - model.DiscArtworkID(a.ID, num), &a.UpdatedAt) + model.DiscArtworkID(a.ID, num), &updatedAt) discTitles = append(discTitles, responses.DiscTitle{ Disc: int32(num), Title: title, From 1f3a7efa759c464455af789f9937938dec402038 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 11 Apr 2026 21:14:52 -0400 Subject: [PATCH 03/18] fix(backup): surface real SQLite error when backup step fails The error-check ordering after backupOp.Step(-1) checked !done before err, which masked the underlying SQLite error (e.g. SQLITE_BUSY, I/O errors) with a generic "backup not done with step -1" message. On failure, Step returns done=false together with a non-nil err, so the !done branch short-circuited before the real error was ever reported. Swap the checks so the SQLite error is returned first, making failing backups actually diagnosable. Refs https://github.com/navidrome/navidrome/issues/5305#issuecomment-4230470593 --- db/backup.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/db/backup.go b/db/backup.go index 8b0f18b1b..a34255d7e 100644 --- a/db/backup.go +++ b/db/backup.go @@ -81,12 +81,12 @@ func backupOrRestore(ctx context.Context, isBackup bool, path string) error { // Caution: -1 means that sqlite will hold a read lock until the operation finishes // This will lock out other writes that could happen at the same time done, err := backupOp.Step(-1) - if !done { - return fmt.Errorf("backup not done with step -1") - } if err != nil { return fmt.Errorf("error during backup step: %w", err) } + if !done { + return fmt.Errorf("backup not done with step -1") + } err = backupOp.Finish() if err != nil { From de6475bb497bfbda4f1dc945efe203963839b2d1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sat, 11 Apr 2026 21:19:57 -0400 Subject: [PATCH 04/18] fix(artwork): allow shared disc art from unnumbered filenames in single-folder albums (#5344) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * test(artwork): expect shared disc art for unnumbered filenames in single-folder albums * fix(artwork): match unnumbered disc art for every disc in single-folder albums * test(artwork): verify shared disc art resolves for every disc number * test(artwork): regression guard for numbered disc filter with mixed filenames * test(artwork): verify DiscArtPriority order decides numbered vs shared disc art * test(artwork): strengthen regression guard to exercise both disc art branches * refactor(artwork): simplify disc art matching and drop redundant comments - Lowercase the pattern and filename once in fromExternalFile and pass lowered values into extractDiscNumber, eliminating the duplicate strings.ToLower calls inside that helper. - Drop narrating comments in reader_disc.go and reader_disc_test.go that duplicated information already conveyed by nearby code or doc comments. * fix(artwork): prefer numbered disc art over shared fallback within a pattern Review feedback: with files [disc.jpg, disc1.jpg, disc2.jpg] in a single folder, the previous single-folder fall-through returned the first match in imgFiles order. Because compareImageFiles sorts 'disc' before 'disc1' and 'disc2', disc.jpg would mask the per-disc numbered files for every disc, regressing the behavior from before the shared-disc-art change. Within a single pattern the loop now records the first viable unnumbered candidate as a fallback and keeps scanning for a numbered match equal to the target disc. Numbered matches still win immediately; the shared file is only returned when no numbered match for the target disc exists. Also drops the redundant strings.ToLower(pattern) at the top of fromExternalFile; fromDiscArtPriority already lowercases the whole priority string before splitting, so the function contract is now 'pattern must be lowercase' (documented on the function). * refactor(artwork): trim disc art matching comments and table-drive tests Doc comment on fromExternalFile is trimmed to the one non-obvious contract (caller must pre-lowercase the pattern) plus the headline behavior; the bulleted restatement of the branch logic went away. Two inline comments that narrated what the code already shows are also gone. Hoisting a `hasWildcard := strings.ContainsRune(pattern, '*')` check out of the loop avoids per-iteration extractDiscNumber calls for literal patterns (e.g. `shellac.png`) and lets the loop break as soon as a viable fallback is found, since literal patterns can never be beaten by a numbered match. Wildcard patterns keep the original scan-to-end-for-numbered-match behavior. The two regression tests added in the previous commit were structurally identical apart from discNumber/expected, so they are collapsed into a DescribeTable with two entries — matching the existing table style used for extractDiscNumber tests in the same file. * fix(artwork): support '?' and '[...]' wildcards in disc art patterns filepath.Match understands three glob metacharacters ('*', '?', '[') but extractDiscNumber only looked for '*'. A pattern like 'disc?.jpg' or 'cd[12].jpg' would therefore be treated as unnumbered, and every disc of a multi-disc album would resolve to the same (first-sorted) file instead of the per-disc numbered art. extractDiscNumber now finds the literal prefix of the pattern by scanning for the first '*', '?', or '[' (via strings.IndexAny), strips it from the filename, and parses the leading digits that follow. The standalone filepath.Match check is dropped; HasPrefix plus the leading-digits requirement is enough to reject non-matches, and the caller already verifies the glob match before calling. fromExternalFile's literal-pattern optimization is widened correspondingly: a pattern is treated as literal only when it contains none of '*', '?', '['. Any wildcard form now keeps the scan-to-end behavior so a numbered match can beat a fallback. Adds table entries for both the extractDiscNumber parser and the fromExternalFile higher-level behavior, covering '?' and '[...]' patterns as well as a literal-pattern baseline. * refactor(artwork): tidy extractDiscNumber after glob-wildcard support - Name the '*?[' charset as globMetaChars, used by both extractDiscNumber and fromExternalFile so the two call sites can't drift. - Trim the extractDiscNumber doc comment: keep the non-obvious caller contract, drop the algorithm narration. - Replace the byte-slice digit accumulator with a direct filename slice fed to strconv.Atoi. - Rename the four new non-'*' wildcard Entry descriptions so they read like the existing extractDiscNumber table ('pattern, target → expected') instead of the ambiguous 'disc 1' shorthand. * fix(artwork): retry remaining fallbacks when the first one fails to open Review feedback: the previous shape remembered only the first unnumbered candidate and fell through to a generic error if os.Open failed on it, even though other matching unnumbered files in imgFiles could have succeeded. The pre-PR code was more resilient because it looped and continued on open failure. fromExternalFile now collects every viable unnumbered candidate into a slice during the scan, then tries them in order after the loop, mirroring the pre-PR retry-on-open-failure behavior. Numbered matches still return immediately on first success and skip the candidate list entirely — an open failure on a numbered match means no other file has that number anyway. Also: - globMetaChars doc comment now notes that '\' escape is intentionally excluded (filepath.Match supports it but treating it as a metachar here would misalign extractDiscNumber's literal-prefix extraction with no benefit for realistic config patterns). - The 'cover.jpg doesn't match disc*.*' Entry in the extractDiscNumber table is renamed to 'cover.jpg with disc*.* (no prefix match)' to reflect that the test now exercises the HasPrefix defensive guard, not the removed internal filepath.Match check. Regression test added: a single-folder album with a deleted first candidate file resolves to the second candidate. * fix(artwork): scan all literal-pattern matches so fallback retry works Review feedback: the 'break on first literal match' optimization assumed only one file in imgFiles could match a literal basename, but filepath.Match compares basenames only — multiple folders can contribute files with the same basename, and the fallback-list retry in 5d79f751c is defeated if the loop breaks after recording just the first one. Removing the break makes literal and wildcard patterns follow the same scan-to-end path, preserving the retry-on-open-failure resilience regained in 5d79f751c. The efficiency cost is negligible — imgFiles is 5-20 entries per album and this is a cache-miss path. --- core/artwork/reader_disc.go | 102 ++++++++-------- core/artwork/reader_disc_test.go | 194 +++++++++++++++++++++++++++++-- 2 files changed, 233 insertions(+), 63 deletions(-) diff --git a/core/artwork/reader_disc.go b/core/artwork/reader_disc.go index 7548f76d2..5a7a8a65e 100644 --- a/core/artwork/reader_disc.go +++ b/core/artwork/reader_disc.go @@ -168,47 +168,38 @@ func (d *discArtworkReader) fromDiscSubtitle(ctx context.Context, subtitle strin } } -// extractDiscNumber extracts a disc number from a filename based on a glob pattern. -// It finds the portion of the filename that the wildcard matched and parses leading -// digits as the disc number. Returns (0, false) if the pattern doesn't match or -// no leading digits are found in the wildcard portion. +// globMetaChars holds the substitution metacharacters understood by +// filepath.Match. The '\' escape character is intentionally excluded: +// disc art patterns come from user config and never include escaped +// metachars in practice, and treating '\' as a metachar would misalign +// the literal-prefix extraction in extractDiscNumber. +const globMetaChars = "*?[" + +// extractDiscNumber parses the disc number from a filename matched by a +// filepath.Match-style glob pattern. +// +// Both pattern and filename must already be lowercased by the caller, which +// is also expected to have verified that filepath.Match(pattern, filename) +// is true before calling this function. func extractDiscNumber(pattern, filename string) (int, bool) { - filename = strings.ToLower(filename) - pattern = strings.ToLower(pattern) - - matched, err := filepath.Match(pattern, filename) - if err != nil || !matched { + metaIdx := strings.IndexAny(pattern, globMetaChars) + if metaIdx < 0 { return 0, false } - - // Find the prefix before the first '*' in the pattern - starIdx := strings.IndexByte(pattern, '*') - if starIdx < 0 { - return 0, false - } - prefix := pattern[:starIdx] - - // Strip the prefix from the filename to get the wildcard-matched portion + prefix := pattern[:metaIdx] if !strings.HasPrefix(filename, prefix) { return 0, false } - remainder := filename[len(prefix):] - // Extract leading ASCII digits from the remainder - var digits []byte - for _, r := range remainder { - if r >= '0' && r <= '9' { - digits = append(digits, byte(r)) - } else { - break - } + start := len(prefix) + end := start + for end < len(filename) && filename[end] >= '0' && filename[end] <= '9' { + end++ } - - if len(digits) == 0 { + if end == start { return 0, false } - - num, err := strconv.Atoi(string(digits)) + num, err := strconv.Atoi(filename[start:end]) if err != nil { return 0, false } @@ -216,20 +207,16 @@ func extractDiscNumber(pattern, filename string) (int, bool) { } // fromExternalFile returns a sourceFunc that matches image files against a glob -// pattern with disc-number-aware filtering. -// -// Matching rules: -// - If a disc number can be extracted from the filename, the file matches only if -// the number equals the target disc number. -// - If no number is found and this is a multi-folder album, the file matches if -// it's in a folder containing tracks for this disc. -// - If no number is found and this is a single-folder album, the file is skipped -// (ambiguous). +// pattern. A numbered filename whose number equals the target disc wins over +// any unnumbered candidate; callers must pass a lowercase pattern. func (d *discArtworkReader) fromExternalFile(ctx context.Context, pattern string) sourceFunc { + isLiteral := !strings.ContainsAny(pattern, globMetaChars) return func() (io.ReadCloser, string, error) { + var fallbacks []string for _, file := range d.imgFiles { _, name := filepath.Split(file) - match, err := filepath.Match(pattern, strings.ToLower(name)) + name = strings.ToLower(name) + match, err := filepath.Match(pattern, name) if err != nil { log.Warn(ctx, "Error matching disc art file to pattern", "pattern", pattern, "file", file) continue @@ -238,24 +225,27 @@ func (d *discArtworkReader) fromExternalFile(ctx context.Context, pattern string continue } - // Try to extract disc number from filename - num, hasNum := extractDiscNumber(pattern, name) - if hasNum { - // File has a disc number — must match target disc - if num != d.discNumber { - continue + if !isLiteral { + if num, hasNum := extractDiscNumber(pattern, name); hasNum { + if num != d.discNumber { + continue + } + f, err := os.Open(file) + if err != nil { + log.Warn(ctx, "Could not open disc art file", "file", file, err) + continue + } + return f, file, nil } - } else if d.isMultiFolder { - // No number, multi-folder: match by folder association - dir := filepath.Dir(file) - if !d.discFolders[dir] { - continue - } - } else { - // No number, single-folder: ambiguous, skip - continue } + if d.isMultiFolder && !d.discFolders[filepath.Dir(file)] { + continue + } + fallbacks = append(fallbacks, file) + } + + for _, file := range fallbacks { f, err := os.Open(file) if err != nil { log.Warn(ctx, "Could not open disc art file", "file", file, err) diff --git a/core/artwork/reader_disc_test.go b/core/artwork/reader_disc_test.go index f8193e24e..7b633342f 100644 --- a/core/artwork/reader_disc_test.go +++ b/core/artwork/reader_disc_test.go @@ -42,11 +42,24 @@ var _ = Describe("Disc Artwork Reader", func() { // Case insensitive (filename already lowered by caller) Entry("Disc1.jpg lowered", "disc*.*", "disc1.jpg", 1, true), - // Pattern doesn't match - Entry("cover.jpg doesn't match disc*.*", "disc*.*", "cover.jpg", 0, false), + // HasPrefix guard: filename doesn't share the pattern's literal prefix + Entry("cover.jpg with disc*.* (no prefix match)", "disc*.*", "cover.jpg", 0, false), // Pattern with no wildcard before dot Entry("front1.jpg with front*.*", "front*.*", "front1.jpg", 1, true), + + // '?' single-char wildcard + Entry("disc?.jpg with disc1.jpg", "disc?.jpg", "disc1.jpg", 1, true), + Entry("disc?.jpg with disc2.jpg", "disc?.jpg", "disc2.jpg", 2, true), + Entry("cd??.jpg with cd07.jpg", "cd??.jpg", "cd07.jpg", 7, true), + + // '[...]' character class wildcard + Entry("cd[12].jpg with cd1.jpg", "cd[12].jpg", "cd1.jpg", 1, true), + Entry("cd[12].jpg with cd2.jpg", "cd[12].jpg", "cd2.jpg", 2, true), + Entry("disc[0-9].jpg with disc5.jpg", "disc[0-9].jpg", "disc5.jpg", 5, true), + + // Literal pattern (no wildcard) returns false + Entry("shellac.png literal", "shellac.png", "shellac.png", 0, false), ) }) @@ -85,19 +98,186 @@ var _ = Describe("Disc Artwork Reader", func() { Expect(path).To(Equal(f1)) }) - It("skips file without number in single-folder album", func() { - f1 := createFile("album/disc.jpg") + It("matches file without number in single-folder album (shared disc art)", func() { + f1 := createFile("album/cover.png") reader := &discArtworkReader{ discNumber: 1, imgFiles: []string{f1}, discFolders: map[string]bool{filepath.Join(tmpDir, "album"): true}, } - sf := reader.fromExternalFile(ctx, "disc*.*") - r, _, _ := sf() - Expect(r).To(BeNil()) + sf := reader.fromExternalFile(ctx, "cover.*") + r, path, err := sf() + Expect(err).ToNot(HaveOccurred()) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(f1)) }) + It("returns shared disc art for every disc number in single-folder album", func() { + f1 := createFile("album/shellac.png") + makeReader := func(discNum int) *discArtworkReader { + return &discArtworkReader{ + discNumber: discNum, + imgFiles: []string{f1}, + discFolders: map[string]bool{filepath.Join(tmpDir, "album"): true}, + } + } + + for _, disc := range []int{1, 2, 5} { + sf := makeReader(disc).fromExternalFile(ctx, "shellac.png") + r, path, err := sf() + Expect(err).ToNot(HaveOccurred(), "disc %d", disc) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(f1), "disc %d", disc) + } + }) + + It("numbered and unnumbered patterns both resolve against the same reader", func() { + f1 := createFile("album/cover.png") + f2 := createFile("album/disc1.jpg") + f3 := createFile("album/disc2.jpg") + reader := &discArtworkReader{ + discNumber: 2, + imgFiles: []string{f1, f2, f3}, + discFolders: map[string]bool{filepath.Join(tmpDir, "album"): true}, + } + + sf := reader.fromExternalFile(ctx, "disc*.*") + r, path, err := sf() + Expect(err).ToNot(HaveOccurred()) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(f3)) + + sf = reader.fromExternalFile(ctx, "cover.*") + r, path, err = sf() + Expect(err).ToNot(HaveOccurred()) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(f1)) + }) + + It("respects DiscArtPriority order when both numbered and unnumbered patterns match", func() { + f1 := createFile("album/cover.png") + f2 := createFile("album/disc1.jpg") + reader := &discArtworkReader{ + discNumber: 1, + imgFiles: []string{f1, f2}, + discFolders: map[string]bool{filepath.Join(tmpDir, "album"): true}, + } + + ff := reader.fromDiscArtPriority(ctx, nil, "disc*.*, cover.*") + Expect(ff).To(HaveLen(2)) + r, path, err := ff[0]() + Expect(err).ToNot(HaveOccurred()) + Expect(path).To(Equal(f2)) + r.Close() + + ff = reader.fromDiscArtPriority(ctx, nil, "cover.*, disc*.*") + Expect(ff).To(HaveLen(2)) + r, path, err = ff[0]() + Expect(err).ToNot(HaveOccurred()) + Expect(path).To(Equal(f1)) + r.Close() + }) + + DescribeTable("numbered match wins over shared fallback within a pattern", + func(discNumber, expectedIdx int) { + files := []string{ + createFile("album/disc.jpg"), + createFile("album/disc1.jpg"), + createFile("album/disc2.jpg"), + } + reader := &discArtworkReader{ + discNumber: discNumber, + imgFiles: files, + discFolders: map[string]bool{filepath.Join(tmpDir, "album"): true}, + } + + sf := reader.fromExternalFile(ctx, "disc*.*") + r, path, err := sf() + Expect(err).ToNot(HaveOccurred()) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(files[expectedIdx])) + }, + Entry("disc 2 picks disc2.jpg over the shared disc.jpg", 2, 2), + Entry("disc 3 falls back to disc.jpg when no numbered match exists", 3, 0), + ) + + It("tries the next fallback candidate when the first one cannot be opened", func() { + f1 := createFile("album/cover.jpg") + f2 := createFile("album/cover.png") + // Remove f1 so os.Open will fail on it; f2 should still win. + Expect(os.Remove(f1)).To(Succeed()) + reader := &discArtworkReader{ + discNumber: 1, + imgFiles: []string{f1, f2}, + discFolders: map[string]bool{filepath.Join(tmpDir, "album"): true}, + } + + sf := reader.fromExternalFile(ctx, "cover.*") + r, path, err := sf() + Expect(err).ToNot(HaveOccurred()) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(f2)) + }) + + It("keeps scanning literal-pattern matches so fallback retry still works", func() { + // Guards against an 'early break on first literal match' optimization. + // Multiple imgFiles entries can share a basename (symlinks, case-variant + // duplicates on case-sensitive filesystems). If the loop breaks after + // recording just the first, the fallback retry cannot recover when + // that first file is unreadable. + f1 := createFile("album/stale/cover.png") + f2 := createFile("album/cover.png") + Expect(os.Remove(f1)).To(Succeed()) + reader := &discArtworkReader{ + discNumber: 1, + imgFiles: []string{f1, f2}, + discFolders: map[string]bool{ + filepath.Join(tmpDir, "album"): true, + filepath.Join(tmpDir, "album/stale"): true, + }, + isMultiFolder: true, + } + + sf := reader.fromExternalFile(ctx, "cover.png") + r, path, err := sf() + Expect(err).ToNot(HaveOccurred()) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(f2)) + }) + + DescribeTable("filters by disc number for non-'*' wildcard patterns", + func(pattern string, discNumber, expectedIdx int) { + files := []string{ + createFile("album/disc1.jpg"), + createFile("album/disc2.jpg"), + } + reader := &discArtworkReader{ + discNumber: discNumber, + imgFiles: files, + discFolders: map[string]bool{filepath.Join(tmpDir, "album"): true}, + } + + sf := reader.fromExternalFile(ctx, pattern) + r, path, err := sf() + Expect(err).ToNot(HaveOccurred()) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(files[expectedIdx])) + }, + Entry("disc?.jpg, target disc 1 → disc1.jpg", "disc?.jpg", 1, 0), + Entry("disc?.jpg, target disc 2 → disc2.jpg", "disc?.jpg", 2, 1), + Entry("disc[0-9].jpg, target disc 1 → disc1.jpg", "disc[0-9].jpg", 1, 0), + Entry("disc[0-9].jpg, target disc 2 → disc2.jpg", "disc[0-9].jpg", 2, 1), + ) + It("matches file without number in multi-folder album by folder", func() { f1 := createFile("album/cd1/disc.jpg") f2 := createFile("album/cd2/disc.jpg") From 27209ed26a87dcc0eebd6dfd64fad94b3c3a073f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sat, 11 Apr 2026 23:15:07 -0400 Subject: [PATCH 05/18] fix(transcoding): clamp target channels to codec limit (#5336) (#5345) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(transcoding): clamp target channels to codec limit (#5336) When transcoding a multi-channel source (e.g. 6-channel FLAC) to MP3, the decider passed the source channel count through to ffmpeg unchanged. The default MP3 command path then emitted `-ac 6`, and the template path injected `-ac 6` after the template's own `-ac 2`, causing ffmpeg to honor the last occurrence and fail with exit code 234 since libmp3lame only supports up to 2 channels. Introduce `codecMaxChannels()` in core/stream/codec.go (mp3→2, opus→8), mirroring the existing `codecMaxSampleRate` pattern, and apply the clamp in `computeTranscodedStream` right after the sample-rate clamps. Also fix a pre-existing ordering bug where the profile's MaxAudioChannels check compared against src.Channels rather than ts.Channels, which would have let a looser profile setting raise the codec-clamped value back up. Comparing against the already-clamped ts.Channels makes profile limits strictly narrowing, which matches how the sample-rate block already behaves. The ffmpeg buildTemplateArgs comment is refreshed to point at the new upstream clamp, since the flags it injects are now always codec-safe. Adds unit tests for codecMaxChannels and four decider scenarios covering the literal issue repro (6-ch FLAC→MP3 clamps to 2), a stricter profile limit winning over the codec clamp, a looser profile limit leaving the codec clamp intact, and a codec with no hard limit (AAC) passing 6 channels through. * test(e2e): pin codec channel clamp at the Subsonic API surface (#5336) Add a 6-channel FLAC fixture to the e2e test suite and use it to assert the codec channel clamp end-to-end on both Subsonic streaming endpoints: - getTranscodeDecision (mp3OnlyClient, no MaxAudioChannels in profile): expects TranscodeStream.AudioChannels == 2 for the 6-channel source. This exercises the new codecMaxChannels() helper through the OpenSubsonic decision endpoint, with no profile-level channel limit masking the bug. - /rest/stream (legacy): requests format=mp3 against the multichannel fixture and asserts streamerSpy.LastRequest.Channels == 2, confirming the clamp propagates through ResolveRequest into the stream.Request that the streamer receives. The fixture is metadata-only (channels: 6 plumbed via the existing storagetest.File helper) — no real audio bytes required, since the e2e suite uses a spy streamer rather than invoking ffmpeg. Bumps the empty-query search3 song count expectation from 13 to 14 to account for the new fixture. * test(decider): clarify codec-clamp comment terminology Distinguish "transcoding profile MaxAudioChannels" (Profile.MaxAudioChannels field) from "LimitationAudioChannels" (CodecProfile rule constant). The regression test bypasses the former, not the latter. --- core/ffmpeg/ffmpeg.go | 5 +- core/stream/codec.go | 13 ++++++ core/stream/codec_test.go | 22 +++++++++ core/stream/decider.go | 9 +++- core/stream/decider_test.go | 67 +++++++++++++++++++++++++++ server/e2e/e2e_suite_test.go | 4 ++ server/e2e/subsonic_searching_test.go | 2 +- server/e2e/subsonic_stream_test.go | 14 +++++- server/e2e/subsonic_transcode_test.go | 29 +++++++++--- 9 files changed, 151 insertions(+), 14 deletions(-) diff --git a/core/ffmpeg/ffmpeg.go b/core/ffmpeg/ffmpeg.go index c034ca7d0..5e6dcd115 100644 --- a/core/ffmpeg/ffmpeg.go +++ b/core/ffmpeg/ffmpeg.go @@ -412,8 +412,9 @@ func buildDynamicArgs(opts TranscodeOptions) []string { // buildTemplateArgs handles user-customized command templates, with dynamic injection // of sample rate, channels, and bit depth when requested by the transcode decision. -// Note: these flags are injected unconditionally when non-zero, even if the template -// already includes them. FFmpeg uses the last occurrence of duplicate flags. +// Values in opts have already been clamped to codec limits upstream (see +// core/stream/codec.go codecMax* helpers), so injecting them unconditionally is safe — +// ffmpeg honors the last occurrence of a duplicate flag. func buildTemplateArgs(opts TranscodeOptions) []string { args := createFFmpegCommand(opts.Command, opts.FilePath, opts.BitRate, opts.Offset) diff --git a/core/stream/codec.go b/core/stream/codec.go index 88d1ae45d..28bff75c4 100644 --- a/core/stream/codec.go +++ b/core/stream/codec.go @@ -75,3 +75,16 @@ func codecMaxSampleRate(codec string) int { } return 0 } + +// codecMaxChannels returns the hard maximum number of audio channels a codec +// supports. Returns 0 if the codec has no hard limit (or is unknown), in which +// case the source/profile constraints applied upstream are authoritative. +func codecMaxChannels(codec string) int { + switch strings.ToLower(codec) { + case "mp3": + return 2 + case "opus": + return 8 + } + return 0 +} diff --git a/core/stream/codec_test.go b/core/stream/codec_test.go index 4c76b3ecd..97e15bdb5 100644 --- a/core/stream/codec_test.go +++ b/core/stream/codec_test.go @@ -66,4 +66,26 @@ var _ = Describe("Codec", func() { Expect(normalizeProbeCodec("DSD_LSBF_PLANAR")).To(Equal("dsd")) }) }) + + Describe("codecMaxChannels", func() { + It("returns 2 for mp3", func() { + Expect(codecMaxChannels("mp3")).To(Equal(2)) + }) + + It("returns 8 for opus", func() { + Expect(codecMaxChannels("opus")).To(Equal(8)) + }) + + It("is case-insensitive", func() { + Expect(codecMaxChannels("MP3")).To(Equal(2)) + Expect(codecMaxChannels("Opus")).To(Equal(8)) + }) + + It("returns 0 for codecs with no hard limit", func() { + Expect(codecMaxChannels("aac")).To(Equal(0)) + Expect(codecMaxChannels("flac")).To(Equal(0)) + Expect(codecMaxChannels("vorbis")).To(Equal(0)) + Expect(codecMaxChannels("")).To(Equal(0)) + }) + }) }) diff --git a/core/stream/decider.go b/core/stream/decider.go index 713c779fe..cde12f0f3 100644 --- a/core/stream/decider.go +++ b/core/stream/decider.go @@ -294,14 +294,19 @@ func (s *deciderService) computeTranscodedStream(ctx context.Context, src *Detai if maxRate := codecMaxSampleRate(ts.Codec); maxRate > 0 && ts.SampleRate > maxRate { ts.SampleRate = maxRate } + if maxCh := codecMaxChannels(ts.Codec); maxCh > 0 && ts.Channels > maxCh { + ts.Channels = maxCh + } // Determine target bitrate (all in kbps) if ok := s.computeBitrate(ctx, src, targetFormat, targetIsLossless, clientInfo, ts); !ok { return nil, "" } - // Apply MaxAudioChannels from the transcoding profile - if profile.MaxAudioChannels > 0 && src.Channels > profile.MaxAudioChannels { + // Apply MaxAudioChannels from the transcoding profile. Compare against the + // already-clamped ts.Channels (not src.Channels) so the codec hard limit + // applied above is never raised by a looser profile setting. + if profile.MaxAudioChannels > 0 && ts.Channels > profile.MaxAudioChannels { ts.Channels = profile.MaxAudioChannels } diff --git a/core/stream/decider_test.go b/core/stream/decider_test.go index c776cbdc3..8b58f3323 100644 --- a/core/stream/decider_test.go +++ b/core/stream/decider_test.go @@ -770,6 +770,73 @@ var _ = Describe("Decider", func() { }) }) + Context("Codec channel limits", func() { + It("clamps 6-channel FLAC to 2 channels when transcoding to MP3", func() { + // Regression test for #5336: ffmpeg's mp3 encoder rejects >2 channels. + // The decider must clamp to the codec's hard limit even when no + // transcoding profile MaxAudioChannels is configured. + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: 16}) + ci := &ClientInfo{ + MaxTranscodingAudioBitrate: 320, + TranscodingProfiles: []Profile{ + {Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP}, + }, + } + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanTranscode).To(BeTrue()) + Expect(decision.TargetFormat).To(Equal("mp3")) + Expect(decision.TranscodeStream.Channels).To(Equal(2)) + Expect(decision.TargetChannels).To(Equal(2)) + }) + + It("honors a stricter profile MaxAudioChannels over the codec clamp", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: 16}) + ci := &ClientInfo{ + MaxTranscodingAudioBitrate: 320, + TranscodingProfiles: []Profile{ + {Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP, MaxAudioChannels: 1}, + }, + } + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanTranscode).To(BeTrue()) + Expect(decision.TranscodeStream.Channels).To(Equal(1)) + Expect(decision.TargetChannels).To(Equal(1)) + }) + + It("applies the codec clamp when the profile limit is looser", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: 16}) + ci := &ClientInfo{ + MaxTranscodingAudioBitrate: 320, + TranscodingProfiles: []Profile{ + {Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP, MaxAudioChannels: 4}, + }, + } + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanTranscode).To(BeTrue()) + Expect(decision.TranscodeStream.Channels).To(Equal(2)) + Expect(decision.TargetChannels).To(Equal(2)) + }) + + It("passes channels through unchanged for codecs with no hard limit", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: 16}) + ci := &ClientInfo{ + MaxTranscodingAudioBitrate: 320, + TranscodingProfiles: []Profile{ + {Container: "m4a", AudioCodec: "aac", Protocol: ProtocolHTTP}, + }, + } + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanTranscode).To(BeTrue()) + Expect(decision.TargetFormat).To(Equal("aac")) + Expect(decision.TranscodeStream.Channels).To(Equal(6)) + Expect(decision.TargetChannels).To(Equal(6)) + }) + }) + Context("Probe-based lossless detection", func() { It("uses probe codec name for lossless detection", func() { // WavPack files: ffprobe reports codec as "wavpack", suffix is ".wv" diff --git a/server/e2e/e2e_suite_test.go b/server/e2e/e2e_suite_test.go index 03fa9bbef..5b3500f7a 100644 --- a/server/e2e/e2e_suite_test.go +++ b/server/e2e/e2e_suite_test.go @@ -172,6 +172,10 @@ func buildTestFS() storagetest.FakeFS { "title": "TC MKA Opus", "track": 6, "suffix": "mka", "codec": "opus", "bitrate": 128, "samplerate": 48000, "bitdepth": 0, "channels": 2, "duration": int64(220), }), + "Test/Transcode Formats/07 - TC FLAC Multichannel.flac": file(tcBase, _t{ + "title": "TC FLAC Multichannel", "track": 7, "suffix": "flac", + "bitrate": 4500, "samplerate": 48000, "bitdepth": 24, "channels": 6, "duration": int64(180), + }), // _empty folder (directory with no audio) "_empty/.keep": &fstest.MapFile{Data: []byte{}, ModTime: time.Now()}, diff --git a/server/e2e/subsonic_searching_test.go b/server/e2e/subsonic_searching_test.go index 7f6aaf57a..e348bc6b9 100644 --- a/server/e2e/subsonic_searching_test.go +++ b/server/e2e/subsonic_searching_test.go @@ -117,7 +117,7 @@ var _ = Describe("Search Endpoints", func() { Expect(resp.SearchResult3).ToNot(BeNil()) Expect(resp.SearchResult3.Artist).To(HaveLen(6)) Expect(resp.SearchResult3.Album).To(HaveLen(7)) - Expect(resp.SearchResult3.Song).To(HaveLen(13)) + Expect(resp.SearchResult3.Song).To(HaveLen(14)) }) It("finds across all entity types simultaneously", func() { diff --git a/server/e2e/subsonic_stream_test.go b/server/e2e/subsonic_stream_test.go index 6a11c1740..281524636 100644 --- a/server/e2e/subsonic_stream_test.go +++ b/server/e2e/subsonic_stream_test.go @@ -13,8 +13,9 @@ import ( var _ = Describe("stream.view (legacy streaming)", Ordered, func() { var ( - mp3TrackID string // Come Together (mp3, 320kbps) - flacTrackID string // TC FLAC Standard (flac, 900kbps) + mp3TrackID string // Come Together (mp3, 320kbps) + flacTrackID string // TC FLAC Standard (flac, 900kbps) + flacMultichTrackID string // TC FLAC Multichannel (flac, 6ch) ) BeforeAll(func() { @@ -30,6 +31,8 @@ var _ = Describe("stream.view (legacy streaming)", Ordered, func() { Expect(mp3TrackID).ToNot(BeEmpty()) flacTrackID = byTitle["TC FLAC Standard"] Expect(flacTrackID).ToNot(BeEmpty()) + flacMultichTrackID = byTitle["TC FLAC Multichannel"] + Expect(flacMultichTrackID).ToNot(BeEmpty()) }) Describe("raw / direct play", func() { @@ -101,6 +104,13 @@ var _ = Describe("stream.view (legacy streaming)", Ordered, func() { Expect(streamerSpy.LastRequest.Format).To(Equal("mp3")) Expect(streamerSpy.LastRequest.BitRate).To(Equal(128)) }) + + It("clamps multichannel FLAC to 2 channels when transcoding to mp3 (#5336)", func() { + w := doRawReq("stream", "id", flacMultichTrackID, "format", "mp3", "maxBitRate", "256") + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(streamerSpy.LastRequest.Format).To(Equal("mp3")) + Expect(streamerSpy.LastRequest.Channels).To(Equal(2)) + }) }) Describe("downsampling with maxBitRate only", func() { diff --git a/server/e2e/subsonic_transcode_test.go b/server/e2e/subsonic_transcode_test.go index f134448df..6041cd013 100644 --- a/server/e2e/subsonic_transcode_test.go +++ b/server/e2e/subsonic_transcode_test.go @@ -114,13 +114,14 @@ const ( var _ = Describe("Transcode Endpoints", Ordered, func() { // Track IDs resolved in BeforeAll var ( - mp3TrackID string // Come Together (mp3, 320kbps) - flacTrackID string // TC FLAC Standard (flac, 900kbps) - flacHiResTrackID string // TC FLAC HiRes (flac, 3000kbps) - alacTrackID string // TC ALAC Track (m4a, alac) - dsdTrackID string // TC DSD Track (dsf, dsd) - opusTrackID string // TC Opus Track (opus, 128kbps) - mkaOpusTrackID string // TC MKA Opus (mka, opus via codec tag) + mp3TrackID string // Come Together (mp3, 320kbps) + flacTrackID string // TC FLAC Standard (flac, 900kbps) + flacHiResTrackID string // TC FLAC HiRes (flac, 3000kbps) + flacMultichTrackID string // TC FLAC Multichannel (flac, 6ch) + alacTrackID string // TC ALAC Track (m4a, alac) + dsdTrackID string // TC DSD Track (dsf, dsd) + opusTrackID string // TC Opus Track (opus, 128kbps) + mkaOpusTrackID string // TC MKA Opus (mka, opus via codec tag) ) BeforeAll(func() { @@ -140,6 +141,7 @@ var _ = Describe("Transcode Endpoints", Ordered, func() { mp3TrackID = ensureGetTrackID("Come Together") flacTrackID = ensureGetTrackID("TC FLAC Standard") flacHiResTrackID = ensureGetTrackID("TC FLAC HiRes") + flacMultichTrackID = ensureGetTrackID("TC FLAC Multichannel") alacTrackID = ensureGetTrackID("TC ALAC Track") dsdTrackID = ensureGetTrackID("TC DSD Track") opusTrackID = ensureGetTrackID("TC Opus Track") @@ -353,6 +355,19 @@ var _ = Describe("Transcode Endpoints", Ordered, func() { // maxTranscodingAudioBitrate is 192000 bps = 192 kbps → response in bps Expect(resp.TranscodeDecision.TranscodeStream.AudioBitrate).To(Equal(int32(192000))) }) + + It("clamps multichannel FLAC to 2 channels when transcoding to MP3 (#5336)", func() { + // mp3OnlyClient has no MaxAudioChannels set, so this exercises the + // codec-intrinsic clamp in core/stream/codec.go (codecMaxChannels). + resp := doPostReq("getTranscodeDecision", mp3OnlyClient, "mediaId", flacMultichTrackID, "mediaType", "song") + Expect(resp.Status).To(Equal(responses.StatusOK)) + Expect(resp.TranscodeDecision).ToNot(BeNil()) + Expect(resp.TranscodeDecision.CanTranscode).To(BeTrue()) + Expect(resp.TranscodeDecision.SourceStream.AudioChannels).To(Equal(int32(6))) + Expect(resp.TranscodeDecision.TranscodeStream).ToNot(BeNil()) + Expect(resp.TranscodeDecision.TranscodeStream.Codec).To(Equal("mp3")) + Expect(resp.TranscodeDecision.TranscodeStream.AudioChannels).To(Equal(int32(2))) + }) }) Describe("response structure", func() { From 501c6eaf8fe8458df4fdd6959a71e4415d1aa22e Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 11 Apr 2026 23:23:04 -0400 Subject: [PATCH 06/18] refactor(ffmpeg): consolidate dynamic audio flag injection into a single function Signed-off-by: Deluan --- core/ffmpeg/ffmpeg.go | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/core/ffmpeg/ffmpeg.go b/core/ffmpeg/ffmpeg.go index 5e6dcd115..abeda5c9e 100644 --- a/core/ffmpeg/ffmpeg.go +++ b/core/ffmpeg/ffmpeg.go @@ -387,18 +387,7 @@ func buildDynamicArgs(opts TranscodeOptions) []string { if opts.BitRate > 0 { args = append(args, "-b:a", strconv.Itoa(opts.BitRate)+"k") } - if opts.SampleRate > 0 { - args = append(args, "-ar", strconv.Itoa(opts.SampleRate)) - } - if opts.Channels > 0 { - args = append(args, "-ac", strconv.Itoa(opts.Channels)) - } - // Only pass -sample_fmt for lossless output formats where bit depth matters. - // Lossy codecs (mp3, aac, opus) handle sample format conversion internally, - // and passing interleaved formats like "s16" causes silent failures. - if opts.BitDepth >= 16 && isLosslessOutputFormat(opts.Format) { - args = append(args, "-sample_fmt", bitDepthToSampleFmt(opts.BitDepth)) - } + args = injectDynamicAudioFlags(args, opts) args = append(args, "-v", "0") @@ -417,8 +406,14 @@ func buildDynamicArgs(opts TranscodeOptions) []string { // ffmpeg honors the last occurrence of a duplicate flag. func buildTemplateArgs(opts TranscodeOptions) []string { args := createFFmpegCommand(opts.Command, opts.FilePath, opts.BitRate, opts.Offset) + return injectDynamicAudioFlags(args, opts) +} - // Dynamically inject -ar, -ac, and -sample_fmt before the output target +// injectDynamicAudioFlags appends -ar, -ac, and -sample_fmt flags based on opts. +// Only passes -sample_fmt for lossless output formats where bit depth matters: +// lossy codecs (mp3, aac, opus) handle sample format conversion internally, and +// passing interleaved formats like "s16" causes silent failures. +func injectDynamicAudioFlags(args []string, opts TranscodeOptions) []string { if opts.SampleRate > 0 { args = injectBeforeOutput(args, "-ar", strconv.Itoa(opts.SampleRate)) } From 85e9982b434f27604f01817f45de006cddd18376 Mon Sep 17 00:00:00 2001 From: Jorge Pardo Pardo <78924065+J0R6IT0@users.noreply.github.com> Date: Sun, 12 Apr 2026 16:27:58 +0200 Subject: [PATCH 07/18] feat(plugins): add path to Scrobbler and Lyrics plugin TrackInfo (#5339) * feat: add Path to TrackInfo struct * refactor: improve naming to follow the rest of the code * test: add tests * fix: actually check for filesystem permission * refactor: remove library logic from specific plugins * refactor: move hasFilesystemPermission to a Manifest method * test(plugins): add unit tests for hasLibraryFilesystemAccess method Signed-off-by: Deluan * refactor(plugins): remove hasFilesystemPerm field and use manifest for filesystem permission checks Signed-off-by: Deluan * refactor(plugins): streamline library filesystem access checks in lyrics and scrobbler adapters Signed-off-by: Deluan --------- Signed-off-by: Deluan Co-authored-by: Deluan --- plugins/capabilities/lyrics.yaml | 5 +++ plugins/capabilities/scrobbler.go | 3 ++ plugins/capabilities/scrobbler.yaml | 5 +++ plugins/lyrics_adapter.go | 2 +- plugins/manager_loader.go | 3 +- plugins/manager_plugin.go | 28 +++++++++++++++ plugins/manager_plugin_test.go | 34 +++++++++++++++++++ plugins/manifest.go | 7 ++++ plugins/pdk/go/lyrics/lyrics.go | 3 ++ plugins/pdk/go/lyrics/lyrics_stub.go | 3 ++ plugins/pdk/go/scrobbler/scrobbler.go | 3 ++ plugins/pdk/go/scrobbler/scrobbler_stub.go | 3 ++ .../rust/nd-pdk-capabilities/src/lyrics.rs | 4 +++ .../rust/nd-pdk-capabilities/src/scrobbler.rs | 4 +++ plugins/scrobbler_adapter.go | 16 ++++++--- plugins/scrobbler_adapter_test.go | 34 +++++++++++++++++++ 16 files changed, 150 insertions(+), 7 deletions(-) create mode 100644 plugins/manager_plugin_test.go diff --git a/plugins/capabilities/lyrics.yaml b/plugins/capabilities/lyrics.yaml index e4f88476c..4ac907559 100644 --- a/plugins/capabilities/lyrics.yaml +++ b/plugins/capabilities/lyrics.yaml @@ -102,6 +102,11 @@ components: mbzReleaseTrackId: type: string description: MBZReleaseTrackID is the MusicBrainz release track ID. + path: + type: string + description: |- + Path is the full path to the track file, relative to the library root. + Only included if the plugin has library permission with filesystem access for the track's library. required: - id - title diff --git a/plugins/capabilities/scrobbler.go b/plugins/capabilities/scrobbler.go index 8091efe50..34cf60015 100644 --- a/plugins/capabilities/scrobbler.go +++ b/plugins/capabilities/scrobbler.go @@ -68,6 +68,9 @@ type TrackInfo struct { MBZReleaseGroupID string `json:"mbzReleaseGroupId,omitempty"` // MBZReleaseTrackID is the MusicBrainz release track ID. MBZReleaseTrackID string `json:"mbzReleaseTrackId,omitempty"` + // Path is the full path to the track file, relative to the library root. + // Only included if the plugin has library permission with filesystem access for the track's library. + Path string `json:"path,omitempty"` } // NowPlayingRequest is the request for now playing notification. diff --git a/plugins/capabilities/scrobbler.yaml b/plugins/capabilities/scrobbler.yaml index 5de351a5f..f62da1745 100644 --- a/plugins/capabilities/scrobbler.yaml +++ b/plugins/capabilities/scrobbler.yaml @@ -128,6 +128,11 @@ components: mbzReleaseTrackId: type: string description: MBZReleaseTrackID is the MusicBrainz release track ID. + path: + type: string + description: |- + Path is the full path to the track file, relative to the library root. + Only included if the plugin has library permission with filesystem access for the track's library. required: - id - title diff --git a/plugins/lyrics_adapter.go b/plugins/lyrics_adapter.go index aa9930664..43ebc0e4b 100644 --- a/plugins/lyrics_adapter.go +++ b/plugins/lyrics_adapter.go @@ -31,7 +31,7 @@ type LyricsPlugin struct { // using model.ToLyrics. func (l *LyricsPlugin) GetLyrics(ctx context.Context, mf *model.MediaFile) (model.LyricList, error) { req := capabilities.GetLyricsRequest{ - Track: mediaFileToTrackInfo(mf), + Track: mediaFileToTrackInfo(l.plugin, mf), } resp, err := callPluginFunction[capabilities.GetLyricsRequest, capabilities.GetLyricsResponse]( ctx, l.plugin, FuncLyricsGetLyrics, req, diff --git a/plugins/manager_loader.go b/plugins/manager_loader.go index 59f48453f..ccda9e4cb 100644 --- a/plugins/manager_loader.go +++ b/plugins/manager_loader.go @@ -301,7 +301,7 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error { } // Configure filesystem access for library permission - if pkg.Manifest.Permissions != nil && pkg.Manifest.Permissions.Library != nil && pkg.Manifest.Permissions.Library.Filesystem { + if pkg.Manifest.HasLibraryFilesystemPermission() { adminCtx := adminContext(ctx) libraries, err := m.ds.Library(adminCtx).GetAll() if err != nil { @@ -384,6 +384,7 @@ func (m *Manager) loadPluginWithConfig(p *model.Plugin) error { metrics: m.metrics, allowedUserIDs: allowedUsers, allUsers: p.AllUsers, + libraries: newLibraryAccess(allowedLibraries, p.AllLibraries), } m.mu.Unlock() diff --git a/plugins/manager_plugin.go b/plugins/manager_plugin.go index 08c0073b6..1d4a8c301 100644 --- a/plugins/manager_plugin.go +++ b/plugins/manager_plugin.go @@ -21,6 +21,7 @@ type plugin struct { metrics PluginMetricsRecorder allowedUserIDs []string // User IDs this plugin can access (from DB configuration) allUsers bool // If true, plugin can access all users + libraries libraryAccess } // instance creates a new plugin instance for the given context. @@ -47,3 +48,30 @@ func (p *plugin) Close() error { } return errors.Join(errs...) } + +func (p *plugin) hasLibraryFilesystemAccess(libID int) bool { + return p.manifest.HasLibraryFilesystemPermission() && p.libraries.contains(libID) +} + +// libraryAccess captures the set of libraries a plugin is permitted to see, +// precomputed at load time for O(1) lookup. +type libraryAccess struct { + allLibraries bool + libraryIDSet map[int]struct{} +} + +func newLibraryAccess(allowedLibraryIDs []int, allLibraries bool) libraryAccess { + set := make(map[int]struct{}, len(allowedLibraryIDs)) + for _, id := range allowedLibraryIDs { + set[id] = struct{}{} + } + return libraryAccess{allLibraries: allLibraries, libraryIDSet: set} +} + +func (a libraryAccess) contains(libID int) bool { + if a.allLibraries { + return true + } + _, ok := a.libraryIDSet[libID] + return ok +} diff --git a/plugins/manager_plugin_test.go b/plugins/manager_plugin_test.go new file mode 100644 index 000000000..513b8cb8e --- /dev/null +++ b/plugins/manager_plugin_test.go @@ -0,0 +1,34 @@ +package plugins + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("plugin", func() { + Describe("hasLibraryFilesystemAccess", func() { + fsManifest := &Manifest{ + Permissions: &Permissions{ + Library: &LibraryPermission{Filesystem: true}, + }, + } + + It("returns false when the manifest does not grant filesystem permission", func() { + p := &plugin{manifest: &Manifest{}, libraries: newLibraryAccess(nil, true)} + Expect(p.hasLibraryFilesystemAccess(1)).To(BeFalse()) + }) + + It("returns true for any library when allLibraries is set", func() { + p := &plugin{manifest: fsManifest, libraries: newLibraryAccess(nil, true)} + Expect(p.hasLibraryFilesystemAccess(1)).To(BeTrue()) + Expect(p.hasLibraryFilesystemAccess(42)).To(BeTrue()) + }) + + It("returns true only for libraries in the allowed list", func() { + p := &plugin{manifest: fsManifest, libraries: newLibraryAccess([]int{1, 3}, false)} + Expect(p.hasLibraryFilesystemAccess(1)).To(BeTrue()) + Expect(p.hasLibraryFilesystemAccess(3)).To(BeTrue()) + Expect(p.hasLibraryFilesystemAccess(2)).To(BeFalse()) + }) + }) +}) diff --git a/plugins/manifest.go b/plugins/manifest.go index 375e73e7f..7484718e3 100644 --- a/plugins/manifest.go +++ b/plugins/manifest.go @@ -86,3 +86,10 @@ func ValidateWithCapabilities(m *Manifest, capabilities []Capability) error { func (m *Manifest) HasExperimentalThreads() bool { return m.Experimental != nil && m.Experimental.Threads != nil } + +// HasLibraryFilesystemPermission checks if the manifest grants filesystem permission for libraries. +func (m *Manifest) HasLibraryFilesystemPermission() bool { + return m.Permissions != nil && + m.Permissions.Library != nil && + m.Permissions.Library.Filesystem +} diff --git a/plugins/pdk/go/lyrics/lyrics.go b/plugins/pdk/go/lyrics/lyrics.go index 4f5aa6302..188371fee 100644 --- a/plugins/pdk/go/lyrics/lyrics.go +++ b/plugins/pdk/go/lyrics/lyrics.go @@ -68,6 +68,9 @@ type TrackInfo struct { MBZReleaseGroupID string `json:"mbzReleaseGroupId,omitempty"` // MBZReleaseTrackID is the MusicBrainz release track ID. MBZReleaseTrackID string `json:"mbzReleaseTrackId,omitempty"` + // Path is the full path to the track file, relative to the library root. + // Only included if the plugin has library permission with filesystem access for the track's library. + Path string `json:"path,omitempty"` } // Lyrics requires all methods to be implemented. diff --git a/plugins/pdk/go/lyrics/lyrics_stub.go b/plugins/pdk/go/lyrics/lyrics_stub.go index 1fdf184e5..91eec4997 100644 --- a/plugins/pdk/go/lyrics/lyrics_stub.go +++ b/plugins/pdk/go/lyrics/lyrics_stub.go @@ -65,6 +65,9 @@ type TrackInfo struct { MBZReleaseGroupID string `json:"mbzReleaseGroupId,omitempty"` // MBZReleaseTrackID is the MusicBrainz release track ID. MBZReleaseTrackID string `json:"mbzReleaseTrackId,omitempty"` + // Path is the full path to the track file, relative to the library root. + // Only included if the plugin has library permission with filesystem access for the track's library. + Path string `json:"path,omitempty"` } // Lyrics requires all methods to be implemented. diff --git a/plugins/pdk/go/scrobbler/scrobbler.go b/plugins/pdk/go/scrobbler/scrobbler.go index c694f59d8..e16bfed4b 100644 --- a/plugins/pdk/go/scrobbler/scrobbler.go +++ b/plugins/pdk/go/scrobbler/scrobbler.go @@ -92,6 +92,9 @@ type TrackInfo struct { MBZReleaseGroupID string `json:"mbzReleaseGroupId,omitempty"` // MBZReleaseTrackID is the MusicBrainz release track ID. MBZReleaseTrackID string `json:"mbzReleaseTrackId,omitempty"` + // Path is the full path to the track file, relative to the library root. + // Only included if the plugin has library permission with filesystem access for the track's library. + Path string `json:"path,omitempty"` } // Scrobbler requires all methods to be implemented. diff --git a/plugins/pdk/go/scrobbler/scrobbler_stub.go b/plugins/pdk/go/scrobbler/scrobbler_stub.go index 6d4afd818..86a71af03 100644 --- a/plugins/pdk/go/scrobbler/scrobbler_stub.go +++ b/plugins/pdk/go/scrobbler/scrobbler_stub.go @@ -89,6 +89,9 @@ type TrackInfo struct { MBZReleaseGroupID string `json:"mbzReleaseGroupId,omitempty"` // MBZReleaseTrackID is the MusicBrainz release track ID. MBZReleaseTrackID string `json:"mbzReleaseTrackId,omitempty"` + // Path is the full path to the track file, relative to the library root. + // Only included if the plugin has library permission with filesystem access for the track's library. + Path string `json:"path,omitempty"` } // Scrobbler requires all methods to be implemented. diff --git a/plugins/pdk/rust/nd-pdk-capabilities/src/lyrics.rs b/plugins/pdk/rust/nd-pdk-capabilities/src/lyrics.rs index 16882abae..fcfe553f8 100644 --- a/plugins/pdk/rust/nd-pdk-capabilities/src/lyrics.rs +++ b/plugins/pdk/rust/nd-pdk-capabilities/src/lyrics.rs @@ -102,6 +102,10 @@ pub struct TrackInfo { /// MBZReleaseTrackID is the MusicBrainz release track ID. #[serde(default, skip_serializing_if = "String::is_empty")] pub mbz_release_track_id: String, + /// Path is the full path to the track file, relative to the library root. + /// Only included if the plugin has library permission with filesystem access for the track's library. + #[serde(default, skip_serializing_if = "String::is_empty")] + pub path: String, } /// Error represents an error from a capability method. diff --git a/plugins/pdk/rust/nd-pdk-capabilities/src/scrobbler.rs b/plugins/pdk/rust/nd-pdk-capabilities/src/scrobbler.rs index 2572712d1..dd42e6803 100644 --- a/plugins/pdk/rust/nd-pdk-capabilities/src/scrobbler.rs +++ b/plugins/pdk/rust/nd-pdk-capabilities/src/scrobbler.rs @@ -122,6 +122,10 @@ pub struct TrackInfo { /// MBZReleaseTrackID is the MusicBrainz release track ID. #[serde(default, skip_serializing_if = "String::is_empty")] pub mbz_release_track_id: String, + /// Path is the full path to the track file, relative to the library root. + /// Only included if the plugin has library permission with filesystem access for the track's library. + #[serde(default, skip_serializing_if = "String::is_empty")] + pub path: String, } /// Error represents an error from a capability method. diff --git a/plugins/scrobbler_adapter.go b/plugins/scrobbler_adapter.go index 874c6603a..4f7cd4661 100644 --- a/plugins/scrobbler_adapter.go +++ b/plugins/scrobbler_adapter.go @@ -80,7 +80,7 @@ func (s *ScrobblerPlugin) NowPlaying(ctx context.Context, userId string, track * username := getUsernameFromContext(ctx) input := capabilities.NowPlayingRequest{ Username: username, - Track: mediaFileToTrackInfo(track), + Track: mediaFileToTrackInfo(s.plugin, track), Position: int32(position), } @@ -93,7 +93,7 @@ func (s *ScrobblerPlugin) Scrobble(ctx context.Context, userId string, sc scrobb username := getUsernameFromContext(ctx) input := capabilities.ScrobbleRequest{ Username: username, - Track: mediaFileToTrackInfo(&sc.MediaFile), + Track: mediaFileToTrackInfo(s.plugin, &sc.MediaFile), Timestamp: sc.TimeStamp.Unix(), } @@ -109,9 +109,11 @@ func getUsernameFromContext(ctx context.Context) string { return "" } -// mediaFileToTrackInfo converts a model.MediaFile to capabilities.TrackInfo -func mediaFileToTrackInfo(mf *model.MediaFile) capabilities.TrackInfo { - return capabilities.TrackInfo{ +// mediaFileToTrackInfo converts a model.MediaFile to capabilities.TrackInfo. +// Path is populated only when the plugin is allowed filesystem access to the +// track's library. +func mediaFileToTrackInfo(p *plugin, mf *model.MediaFile) capabilities.TrackInfo { + ti := capabilities.TrackInfo{ ID: mf.ID, Title: mf.Title, Album: mf.Album, @@ -127,6 +129,10 @@ func mediaFileToTrackInfo(mf *model.MediaFile) capabilities.TrackInfo { MBZReleaseGroupID: mf.MbzReleaseGroupID, MBZReleaseTrackID: mf.MbzReleaseTrackID, } + if p.hasLibraryFilesystemAccess(mf.LibraryID) { + ti.Path = mf.Path + } + return ti } // participantsToArtistRefs converts a ParticipantList to a slice of ArtistRef diff --git a/plugins/scrobbler_adapter_test.go b/plugins/scrobbler_adapter_test.go index ab8dc6f88..0ee229022 100644 --- a/plugins/scrobbler_adapter_test.go +++ b/plugins/scrobbler_adapter_test.go @@ -240,6 +240,40 @@ var _ = Describe("ScrobblerPlugin", Ordered, func() { Expect(names).ToNot(ContainElement("test-metadata-agent")) }) }) + + Describe("mediaFileToTrackInfo", func() { + var track *model.MediaFile + + BeforeEach(func() { + track = &model.MediaFile{ + ID: "track-1", + Title: "Test Song", + Path: "/music/test.flac", + LibraryID: 1, + } + }) + + fsManifest := &Manifest{ + Permissions: &Permissions{ + Library: &LibraryPermission{Filesystem: true}, + }, + } + + It("includes Path when the plugin has filesystem access to the track's library", func() { + p := &plugin{manifest: fsManifest, libraries: newLibraryAccess([]int{1}, false)} + Expect(mediaFileToTrackInfo(p, track).Path).To(Equal("/music/test.flac")) + }) + + It("omits Path when the plugin lacks filesystem permission", func() { + p := &plugin{manifest: &Manifest{}, libraries: newLibraryAccess([]int{1}, false)} + Expect(mediaFileToTrackInfo(p, track).Path).To(BeEmpty()) + }) + + It("omits Path when the track's library is not in the allowed set", func() { + p := &plugin{manifest: fsManifest, libraries: newLibraryAccess([]int{2}, false)} + Expect(mediaFileToTrackInfo(p, track).Path).To(BeEmpty()) + }) + }) }) var _ = Describe("mapScrobblerError", func() { From c49e5855b9d6651510cbf6d018f9027ae8793da4 Mon Sep 17 00:00:00 2001 From: m8tec <38794725+m8tec@users.noreply.github.com> Date: Sun, 12 Apr 2026 17:16:00 +0200 Subject: [PATCH 08/18] feat(artwork): make max image upload size configurable (#5335) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(config): make max image upload size configurable Let max image upload size be set from config or environment instead of a fixed 10 MB cap. The upload handler still falls back to 10 MB when MaxImageUploadSize is not set. Signed-off-by: M8te <38794725+m8tec@users.noreply.github.com> * feat(config): support human-readable MaxImageUploadSize values Max image upload size can now be configured as a readable string like 10MB or 1GB instead of raw bytes. The config load validates it at startup, and the upload handler parses it before applying request limits (10MB fallback if it fails). + MaxImageUploadSize as human-readable string + removed redundant max(1, ...) to address code review + cap memory usage of ParseMultipartForm to 10MB (address code review) Signed-off-by: M8te <38794725+m8tec@users.noreply.github.com> * refactor(config): consolidate MaxImageUploadSize default and add tests Move the "10MB" default constant to consts.DefaultMaxImageUploadSize so both the viper default and the runtime fallback share a single source of truth. Improve the validator error message with fmt.Errorf wrapping to match the project convention (e.g. validatePurgeMissingOption). Add unit tests for validateMaxImageUploadSize (valid/invalid inputs) and maxImageUploadSize (configured, empty, invalid, raw bytes). Compute maxImageSize once at handler creation rather than per request. --------- Signed-off-by: M8te <38794725+m8tec@users.noreply.github.com> Co-authored-by: Deluan Quintão --- conf/configuration.go | 13 ++++++++++ conf/configuration_test.go | 31 ++++++++++++++++++++++++ conf/export_test.go | 2 ++ consts/consts.go | 3 ++- server/nativeapi/image_upload.go | 13 ++++++++-- server/nativeapi/image_upload_test.go | 34 +++++++++++++++++++++++++++ 6 files changed, 93 insertions(+), 3 deletions(-) create mode 100644 server/nativeapi/image_upload_test.go diff --git a/conf/configuration.go b/conf/configuration.go index 58239884a..24d116e66 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -12,6 +12,7 @@ import ( "time" "github.com/bmatcuk/doublestar/v4" + "github.com/dustin/go-humanize" "github.com/go-viper/encoding/ini" "github.com/kr/pretty" "github.com/navidrome/navidrome/consts" @@ -80,6 +81,7 @@ type configOptions struct { EnableStarRating bool EnableUserEditing bool EnableArtworkUpload bool + MaxImageUploadSize string EnableSharing bool ShareURL string DefaultShareExpiration time.Duration @@ -360,6 +362,7 @@ func Load(noConfigDump bool) { validateBackupSchedule, validatePlaylistsPath, validatePurgeMissingOption, + validateMaxImageUploadSize, validateURL("ExtAuth.LogoutURL", Server.ExtAuth.LogoutURL), ) if err != nil { @@ -584,6 +587,15 @@ func validatePurgeMissingOption() error { return nil } +func validateMaxImageUploadSize() error { + if _, err := humanize.ParseBytes(Server.MaxImageUploadSize); err != nil { + err = fmt.Errorf("invalid MaxImageUploadSize %q: use values like '10MB', '1GB', or raw bytes like '10485760': %w", Server.MaxImageUploadSize, err) + log.Error(err.Error()) + return err + } + return nil +} + func validateScanSchedule() error { if Server.Scanner.Schedule == "0" || Server.Scanner.Schedule == "" { Server.Scanner.Schedule = "" @@ -742,6 +754,7 @@ func setViperDefaults() { viper.SetDefault("enablecoveranimation", true) viper.SetDefault("enablenowplaying", true) viper.SetDefault("enableartworkupload", true) + viper.SetDefault("maximageuploadsize", consts.DefaultMaxImageUploadSize) viper.SetDefault("enablesharing", false) viper.SetDefault("shareurl", "") viper.SetDefault("defaultshareexpiration", 8760*time.Hour) diff --git a/conf/configuration_test.go b/conf/configuration_test.go index eb2176e83..121b1902c 100644 --- a/conf/configuration_test.go +++ b/conf/configuration_test.go @@ -219,6 +219,37 @@ var _ = Describe("Configuration", func() { }) + Describe("ValidateMaxImageUploadSize", func() { + BeforeEach(func() { + viper.Reset() + conf.SetViperDefaults() + viper.SetDefault("datafolder", GinkgoT().TempDir()) + viper.SetDefault("loglevel", "error") + conf.ResetConf() + }) + + DescribeTable("accepts valid size values", + func(input string) { + conf.Server.MaxImageUploadSize = input + Expect(conf.ValidateMaxImageUploadSize()).To(Succeed()) + }, + Entry("megabytes", "10MB"), + Entry("gigabytes", "1GB"), + Entry("raw bytes", "10485760"), + Entry("mebibytes", "10MiB"), + Entry("lower case", "50mb"), + ) + + DescribeTable("rejects invalid size values", + func(input string) { + conf.Server.MaxImageUploadSize = input + Expect(conf.ValidateMaxImageUploadSize()).To(MatchError(ContainSubstring("invalid MaxImageUploadSize"))) + }, + Entry("garbage string", "not-a-size"), + Entry("negative-looking", "-10MB"), + ) + }) + DescribeTable("should load configuration from", func(format string) { filename := filepath.Join("testdata", "cfg."+format) diff --git a/conf/export_test.go b/conf/export_test.go index 051f9bb65..85755aa12 100644 --- a/conf/export_test.go +++ b/conf/export_test.go @@ -14,6 +14,8 @@ var NormalizeSearchBackend = normalizeSearchBackend var ToPascalCase = toPascalCase +var ValidateMaxImageUploadSize = validateMaxImageUploadSize + func SetLogFatal(f func(...any)) func() { old := logFatal logFatal = f diff --git a/consts/consts.go b/consts/consts.go index ff5dedc2b..3db0b831a 100644 --- a/consts/consts.go +++ b/consts/consts.go @@ -85,7 +85,8 @@ const ( ) const ( - DefaultUICoverArtSize = 300 + DefaultUICoverArtSize = 300 + DefaultMaxImageUploadSize = "10MB" ) // Prometheus options diff --git a/server/nativeapi/image_upload.go b/server/nativeapi/image_upload.go index 1f55e3851..5e2d29876 100644 --- a/server/nativeapi/image_upload.go +++ b/server/nativeapi/image_upload.go @@ -13,14 +13,22 @@ import ( "path/filepath" "strings" + "github.com/dustin/go-humanize" "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/request" _ "golang.org/x/image/webp" ) -const maxImageSize = 10 << 20 // 10MB +func maxImageUploadSize() int64 { + if size, err := humanize.ParseBytes(conf.Server.MaxImageUploadSize); err == nil && size > 0 { + return int64(size) + } + size, _ := humanize.ParseBytes(consts.DefaultMaxImageUploadSize) + return int64(size) +} func checkImageUploadPermission(w http.ResponseWriter, r *http.Request) bool { user, _ := request.UserFrom(r.Context()) @@ -32,13 +40,14 @@ func checkImageUploadPermission(w http.ResponseWriter, r *http.Request) bool { } func handleImageUpload(saveFn func(ctx context.Context, reader io.Reader, ext string) error) http.HandlerFunc { + maxImageSize := maxImageUploadSize() return func(w http.ResponseWriter, r *http.Request) { ctx := r.Context() if !checkImageUploadPermission(w, r) { return } r.Body = http.MaxBytesReader(w, r.Body, maxImageSize) - if err := r.ParseMultipartForm(maxImageSize / 2); err != nil { + if err := r.ParseMultipartForm(min(maxImageSize, 10<<20)); err != nil { log.Error(ctx, "Error parsing multipart form", err) http.Error(w, "file too large or invalid form", http.StatusBadRequest) return diff --git a/server/nativeapi/image_upload_test.go b/server/nativeapi/image_upload_test.go new file mode 100644 index 000000000..291912e67 --- /dev/null +++ b/server/nativeapi/image_upload_test.go @@ -0,0 +1,34 @@ +package nativeapi + +import ( + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("maxImageUploadSize", func() { + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + }) + + It("returns the configured size when valid", func() { + conf.Server.MaxImageUploadSize = "20MB" + Expect(maxImageUploadSize()).To(Equal(int64(20_000_000))) + }) + + It("returns the default size when config is empty", func() { + conf.Server.MaxImageUploadSize = "" + Expect(maxImageUploadSize()).To(Equal(int64(10_000_000))) + }) + + It("returns the default size when config is invalid", func() { + conf.Server.MaxImageUploadSize = "not-a-size" + Expect(maxImageUploadSize()).To(Equal(int64(10_000_000))) + }) + + It("parses raw byte values", func() { + conf.Server.MaxImageUploadSize = "52428800" + Expect(maxImageUploadSize()).To(Equal(int64(52_428_800))) + }) +}) From 1988a4162e51026eadc53d3fbaa27365d3cbcdec Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 12 Apr 2026 12:18:13 -0400 Subject: [PATCH 09/18] refactor(configuration): improve error handling in configuration validation Signed-off-by: Deluan --- conf/configuration.go | 26 ++++++++------------------ 1 file changed, 8 insertions(+), 18 deletions(-) diff --git a/conf/configuration.go b/conf/configuration.go index 24d116e66..a8b0e4c8a 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -366,7 +366,7 @@ func Load(noConfigDump bool) { validateURL("ExtAuth.LogoutURL", Server.ExtAuth.LogoutURL), ) if err != nil { - os.Exit(1) + logFatal(err) } Server.Search.Backend = normalizeSearchBackend(Server.Search.Backend) @@ -552,8 +552,7 @@ func validatePlaylistsPath() error { for path := range strings.SplitSeq(Server.PlaylistsPath, string(filepath.ListSeparator)) { _, err := doublestar.Match(path, "") if err != nil { - log.Error("Invalid PlaylistsPath", "path", path, err) - return err + return fmt.Errorf("invalid PlaylistsPath %q: %w", path, err) } } return nil @@ -580,7 +579,6 @@ func validatePurgeMissingOption() error { valid := slices.Contains(allowedValues, Server.Scanner.PurgeMissing) if !valid { err := fmt.Errorf("invalid Scanner.PurgeMissing value: '%s'. Must be one of: %v", Server.Scanner.PurgeMissing, allowedValues) - log.Error(err.Error()) Server.Scanner.PurgeMissing = consts.PurgeMissingNever return err } @@ -589,9 +587,7 @@ func validatePurgeMissingOption() error { func validateMaxImageUploadSize() error { if _, err := humanize.ParseBytes(Server.MaxImageUploadSize); err != nil { - err = fmt.Errorf("invalid MaxImageUploadSize %q: use values like '10MB', '1GB', or raw bytes like '10485760': %w", Server.MaxImageUploadSize, err) - log.Error(err.Error()) - return err + return fmt.Errorf("invalid MaxImageUploadSize %q: use values like '10MB', '1GB', or raw bytes like '10485760': %w", Server.MaxImageUploadSize, err) } return nil } @@ -619,9 +615,9 @@ func validateBackupSchedule() error { func validateSchedule(schedule, field string) (string, error) { _, err := scheduler.ParseCrontab(schedule) if err != nil { - log.Error(fmt.Sprintf("Invalid %s. Please read format spec at https://pkg.go.dev/github.com/robfig/cron#hdr-CRON_Expression_Format", field), "schedule", schedule, err) + return schedule, fmt.Errorf("invalid %s %q (see https://pkg.go.dev/github.com/robfig/cron#hdr-CRON_Expression_Format): %w", field, schedule, err) } - return schedule, err + return schedule, nil } // validateURL checks if the provided URL is valid and has either http or https scheme. @@ -633,19 +629,13 @@ func validateURL(optionName, optionURL string) func() error { } u, err := url.Parse(optionURL) if err != nil { - log.Error(fmt.Sprintf("Invalid %s: it could not be parsed", optionName), "url", optionURL, "err", err) - return err + return fmt.Errorf("invalid %s %q: %w", optionName, optionURL, err) } if u.Scheme != "http" && u.Scheme != "https" { - err := fmt.Errorf("invalid scheme for %s: '%s'. Only 'http' and 'https' are allowed", optionName, u.Scheme) - log.Error(err.Error()) - return err + return fmt.Errorf("invalid scheme for %s: '%s'. Only 'http' and 'https' are allowed", optionName, u.Scheme) } - // Require an absolute URL with a non-empty host and no opaque component. if u.Host == "" || u.Opaque != "" { - err := fmt.Errorf("invalid %s: '%s'. A full http(s) URL with a non-empty host is required", optionName, optionURL) - log.Error(err.Error()) - return err + return fmt.Errorf("invalid %s: '%s'. A full http(s) URL with a non-empty host is required", optionName, optionURL) } return nil } From 9dfd9ac8497caf7c1ec0310fb652b905c351e0be Mon Sep 17 00:00:00 2001 From: Alexander Makeenkov Date: Sun, 12 Apr 2026 20:17:09 +0300 Subject: [PATCH 10/18] fix(ui): update Russian translations and add missing gain keys (#5329) * feat(i18n): add album and track gain translation strings * chore(i18n): update Russian translations --------- Co-authored-by: Alexander Makeenkov Signed-off-by: Deluan --- resources/i18n/ru.json | 282 +++++++++++++++++++++-------------------- ui/src/i18n/en.json | 2 + 2 files changed, 144 insertions(+), 140 deletions(-) diff --git a/resources/i18n/ru.json b/resources/i18n/ru.json index 78e7cfa26..1a7adcc4a 100644 --- a/resources/i18n/ru.json +++ b/resources/i18n/ru.json @@ -1,5 +1,5 @@ { - "languageName": "Pусский", + "languageName": "Русский", "resources": { "song": { "name": "Трек |||| Треки |||| Треков", @@ -7,19 +7,19 @@ "albumArtist": "Исполнитель альбома", "duration": "Длительность", "trackNumber": "#", - "playCount": "Проигрывания", + "playCount": "Прослушивания", "title": "Название трека", - "artist": "Исполнитель", + "artist": "Артист", "album": "Альбом", "path": "Путь", "genre": "Жанр", "compilation": "Сборник", "year": "Год", "size": "Размер", - "updatedAt": "Обновлен", + "updatedAt": "Обновлено", "bitRate": "Битрейт", "discSubtitle": "Название диска", - "starred": "Избранные", + "starred": "Избранное", "comment": "Комментарий", "rating": "Рейтинг", "quality": "Качество", @@ -35,10 +35,12 @@ "rawTags": "Исходные теги", "bitDepth": "Битовая глубина (Bit)", "sampleRate": "Частота дискретизации (Hz)", + "albumGain": "Усиление альбома", + "trackGain": "Усиление трека", "missing": "Поле отсутствует", "libraryName": "Библиотека", "composer": "Композитор", - "disc": "" + "disc": "Диск %{discNumber}" }, "actions": { "addToQueue": "В очередь", @@ -53,18 +55,18 @@ } }, "album": { - "name": "Альбом |||| Альбомы", + "name": "Альбом |||| Альбомы |||| Альбомов", "fields": { "albumArtist": "Исполнитель альбома", - "artist": "Исполнитель", + "artist": "Артист", "duration": "Длительность", - "songCount": "Треков", - "playCount": "Проигрывания", + "songCount": "Трек |||| Треки |||| Треков", + "playCount": "Прослушивания", "name": "Название альбома", "genre": "Жанр", "compilation": "Сборник", "year": "Год", - "updatedAt": "Обновлен", + "updatedAt": "Обновлено", "comment": "Комментарий", "rating": "Рейтинг", "createdAt": "Дата добавления", @@ -99,17 +101,17 @@ "recentlyAdded": "Новые", "recentlyPlayed": "Проигранные", "mostPlayed": "Популярные", - "starred": "Избранные", + "starred": "Избранное", "topRated": "Лучшие" } }, "artist": { - "name": "Исполнитель |||| Исполнители", + "name": "Артист |||| Артисты |||| Артистов", "fields": { "name": "Название исполнителя", "albumCount": "Количество альбомов", "songCount": "Количество треков", - "playCount": "Проигрывания", + "playCount": "Прослушивания", "rating": "Рейтинг", "genre": "Жанр", "size": "Размер", @@ -117,29 +119,29 @@ "missing": "Поле отсутствует" }, "roles": { - "albumartist": "Исполнитель альбома |||| Исполнители альбома", - "artist": "Исполнитель |||| Исполнители", - "composer": "Композитор |||| Композиторы", - "conductor": "Дирижёр |||| Дирижёры", - "lyricist": "Автор текста |||| Авторы текста", - "arranger": "Аранжировщик |||| Аранжировщики", - "producer": "Продюсер |||| Продюсеры", - "director": "Режиссёр |||| Режиссёры", - "engineer": "Инженер |||| Инженеры", - "mixer": "Звукоинженер |||| Звукоинженеры", - "remixer": "Ремиксер |||| Ремиксеры", - "djmixer": "DJ-миксер |||| DJ-миксеры", - "performer": "Исполнитель |||| Исполнители", - "maincredit": "Исполнитель альбома или Исполнитель |||| Исполнители альбома или Исполнители" + "albumartist": "Исполнитель альбома |||| Исполнители альбома |||| Исполнителей альбома", + "artist": "Артист |||| Артисты |||| Артистов", + "composer": "Композитор |||| Композиторы |||| Композиторов", + "conductor": "Дирижёр |||| Дирижёры |||| Дирижёров", + "lyricist": "Автор текста |||| Авторы текста |||| Авторов текста", + "arranger": "Аранжировщик |||| Аранжировщики |||| Аранжировщиков", + "producer": "Продюсер |||| Продюсеры |||| Продюсеров", + "director": "Режиссёр |||| Режиссёры |||| Режиссёров", + "engineer": "Инженер |||| Инженеры |||| Инженеров", + "mixer": "Звукоинженер |||| Звукоинженеры |||| Звукоинженеров", + "remixer": "Ремиксер |||| Ремиксеры |||| Ремиксеров", + "djmixer": "DJ-миксер |||| DJ-миксеры |||| DJ-миксеров", + "performer": "Исполнитель |||| Исполнители |||| Исполнителей", + "maincredit": "Исполнитель альбома или артист |||| Исполнители альбома или артисты |||| Исполнителей альбома или артистов" }, "actions": { - "shuffle": "Смешать", + "shuffle": "Перемешать", "radio": "Радио", "topSongs": "Топовые треки" } }, "user": { - "name": "Пользователь |||| Пользователи", + "name": "Пользователь |||| Пользователи |||| Пользователей", "fields": { "userName": "Имя пользователя", "isAdmin": "Администратор", @@ -175,9 +177,9 @@ } }, "player": { - "name": "Плеер |||| Плееры", + "name": "Плеер |||| Плееры |||| Плееров", "fields": { - "name": "Имя", + "name": "Название", "transcodingId": "Транскодирование", "maxBitRate": "Макс. битрейт", "client": "Клиент", @@ -188,7 +190,7 @@ } }, "transcoding": { - "name": "Транскодирование |||| Транскодирование", + "name": "Транскодирование |||| Транскодирование |||| Транскодирований", "fields": { "name": "Название", "targetFormat": "Целевой формат", @@ -197,15 +199,15 @@ } }, "playlist": { - "name": "Плейлист |||| Плейлисты", + "name": "Плейлист |||| Плейлисты |||| Плейлистов", "fields": { - "name": "Название трека", + "name": "Название", "duration": "Длительность", "ownerName": "Владелец", "public": "Публичный", - "updatedAt": "Обновлен", + "updatedAt": "Обновлено", "createdAt": "Создан", - "songCount": "Треков", + "songCount": "Трек |||| Трека |||| Треков", "comment": "Комментарий", "sync": "Автоимпорт", "path": "Импортировать из" @@ -218,7 +220,7 @@ "makePrivate": "Сделать личным", "saveQueue": "Сохранить очередь в плейлист", "searchOrCreate": "Поиск плейлистов или введите текст для создания новых...", - "pressEnterToCreate": "Нажмите Enter, чтобы создать новый список воспроизведения", + "pressEnterToCreate": "Нажмите Enter, чтобы создать новый плейлист", "removeFromSelection": "Удалить из списка выделенных" }, "message": { @@ -229,9 +231,9 @@ } }, "radio": { - "name": "Радио |||| Радио", + "name": "Радио |||| Радио |||| Радио", "fields": { - "name": "Имя", + "name": "Название", "streamUrl": "Ссылка на поток", "homePageUrl": "Домашняя страница", "updatedAt": "Обновлено", @@ -242,7 +244,7 @@ } }, "share": { - "name": "Общий доступ |||| Общий доступ", + "name": "Общий доступ |||| Общий доступ |||| Общий доступ", "fields": { "username": "Поделился", "url": "Ссылка", @@ -253,15 +255,15 @@ "visitCount": "Количество посещений", "format": "Формат", "maxBitRate": "Макс. битрейт", - "updatedAt": "Обновлено в", + "updatedAt": "Обновлено", "createdAt": "Создано", "downloadable": "Разрешить загрузку?" } }, "missing": { - "name": "Файл отсутствует |||| Файлы отсутствуют", + "name": "Отсутствующий файл |||| Отсутствующие файлы |||| Отсутствующих файлов", "fields": { - "path": "Место расположения", + "path": "Путь", "size": "Размер", "updatedAt": "Исчез", "libraryName": "Библиотека" @@ -276,21 +278,21 @@ "empty": "Нет отсутствующих файлов" }, "library": { - "name": "Библиотека |||| Библиотеки", + "name": "Библиотека |||| Библиотеки |||| Библиотек", "fields": { - "name": "Имя", + "name": "Название", "path": "Путь", "remotePath": "Удаленный путь", "lastScanAt": "Последнее сканирование", "songCount": "Треки", "albumCount": "Альбомы", - "artistCount": "Исполнители", + "artistCount": "Артисты", "totalSongs": "Треки", "totalAlbums": "Альбомы", - "totalArtists": "Исполнители", + "totalArtists": "Артисты", "totalFolders": "Папки", - "totalFiles": "Файлов", - "totalMissingFiles": "Пропавших файлов", + "totalFiles": "Файлы", + "totalMissingFiles": "Отсутствующие файлы", "totalSize": "Общий размер", "totalDuration": "Длительность", "defaultNewUsers": "По умолчанию для новых пользователей", @@ -319,7 +321,7 @@ "scanError": "Ошибка при запуске сканирования. Проверьте логи" }, "validation": { - "nameRequired": "Имя библиотеки обязательно", + "nameRequired": "Название библиотеки обязательно", "pathRequired": "Путь к библиотеке обязателен", "pathNotDirectory": "Путь к библиотеке должен быть директорией", "pathNotFound": "Путь к библиотеке не найден", @@ -333,14 +335,14 @@ } }, "plugin": { - "name": "Плагин |||| Плагины", + "name": "Плагин |||| Плагины |||| Плагинов", "fields": { "id": "ID", - "name": "Имя", + "name": "Название", "description": "Описание", "version": "Версия", "author": "Автор", - "website": "Вебсайт", + "website": "Веб-сайт", "permissions": "Разрешения", "enabled": "Включено", "status": "Статус", @@ -348,26 +350,26 @@ "lastError": "Ошибка", "hasError": "Ошибка", "updatedAt": "Обновлено", - "createdAt": "Установленный", + "createdAt": "Дата установки", "configKey": "Ключ", "configValue": "Значение", "allUsers": "Разрешить всем пользователям", "selectedUsers": "Выбранные пользователи", "allLibraries": "Разрешить доступ ко всем библиотекам", - "selectedLibraries": "Избранные библиотеки", - "allowWriteAccess": "" + "selectedLibraries": "Выбранные библиотеки", + "allowWriteAccess": "Разрешить запись" }, "sections": { "status": "Статус", "info": "Информация о плагине", "configuration": "Конфигурация", "manifest": "Манифест", - "usersPermission": "Разрешение пользователей", - "libraryPermission": "Разрешение на использование библиотеки" + "usersPermission": "Права доступа пользователей", + "libraryPermission": "Права доступа к библиотекам" }, "status": { "enabled": "Включено", - "disabled": "Отключить" + "disabled": "Отключено" }, "actions": { "enable": "Включить", @@ -401,7 +403,7 @@ "requiredHosts": "Необходимые хосты", "configValidationError": "Проверка конфигурации завершилась неудачей:", "schemaRenderError": "Не удалось отобразить форму конфигурации. Возможно, схема плагина недействительна.", - "allowWriteAccessHelp": "" + "allowWriteAccessHelp": "Разрешить плагину изменять файлы в вашей библиотеке" }, "placeholders": { "configKey": "ключ", @@ -412,9 +414,9 @@ "ra": { "auth": { "welcome1": "Спасибо за установку Navidrome!", - "welcome2": "Для начала, создайте аккаунт Администратора", - "confirmPassword": "Подтвердить Пароль", - "buttonCreateAdmin": "Создать аккаунт Администратора", + "welcome2": "Для начала создайте аккаунт администратора", + "confirmPassword": "Подтвердите пароль", + "buttonCreateAdmin": "Создать аккаунт администратора", "auth_check_error": "Пожалуйста, авторизуйтесь для продолжения работы", "user_menu": "Профиль", "username": "Имя пользователя", @@ -428,14 +430,14 @@ "invalidChars": "Пожалуйста, используйте только буквы и цифры", "passwordDoesNotMatch": "Пароли не совпадают", "required": "Обязательно для заполнения", - "minLength": "Минимальное кол-во символов %{min}", - "maxLength": "Максимальное кол-во символов %{max}", - "minValue": "Минимальное значение %{min}", - "maxValue": "Значение может быть %{max} или меньше", - "number": "Должно быть цифрой", + "minLength": "Минимальное количество символов: %{min}", + "maxLength": "Максимальное количество символов: %{max}", + "minValue": "Минимальное значение: %{min}", + "maxValue": "Максимальное значение: %{max}", + "number": "Должно быть числом", "email": "Некорректный Email", "oneOf": "Должно быть одним из: %{options}", - "regex": "Должно быть в формате (regexp): %{pattern}", + "regex": "Должно соответствовать формату: %{pattern}", "unique": "Должно быть уникальным", "url": "Должен быть действительный URL" }, @@ -443,7 +445,7 @@ "add_filter": "Фильтр", "add": "Добавить", "back": "Назад", - "bulk_actions": "1 выбран |||| %{smart_count} выбрано |||| %{smart_count} выбрано", + "bulk_actions": "1 выбран |||| %{smart_count} выбраны |||| %{smart_count} выбрано", "cancel": "Отмена", "clear_input_value": "Очистить", "clone": "Дублировать", @@ -461,13 +463,13 @@ "show": "Просмотр", "sort": "Сортировать", "undo": "Отменить", - "expand": "Расширить", + "expand": "Развернуть", "close": "Закрыть", "open_menu": "Открыть меню", "close_menu": "Закрыть меню", - "unselect": "Отменить выделение", + "unselect": "Снять выделение", "skip": "Пропустить", - "bulk_actions_mobile": "1 |||| %{smart_count}", + "bulk_actions_mobile": "1 |||| %{smart_count} |||| %{smart_count}", "share": "Поделиться", "download": "Скачать" }, @@ -481,7 +483,7 @@ "edit": "%{name} #%{id}", "error": "Что-то пошло не так", "list": "%{name}", - "loading": "Загрузка", + "loading": "Загрузка...", "not_found": "Не найдено", "show": "%{name} #%{id}", "empty": "Нет %{name}.", @@ -493,13 +495,13 @@ "upload_single": "Перетащите файл для загрузки или щёлкните для выбора." }, "image": { - "upload_several": "Перетащите картинки для загрузки или щёлкните для выбора.", - "upload_single": "Перетащите картинку для загрузки или щёлкните для выбора." + "upload_several": "Перетащите изображения для загрузки или щёлкните для выбора.", + "upload_single": "Перетащите изображение для загрузки или щёлкните для выбора." }, "references": { "all_missing": "Связанных данных не найдено.", - "many_missing": "Некоторые из связанных данных не доступны", - "single_missing": "Связанный объект не доступен" + "many_missing": "Некоторые из связанных данных недоступны", + "single_missing": "Связанный объект недоступен" }, "password": { "toggle_visible": "Скрыть пароль", @@ -507,45 +509,45 @@ } }, "message": { - "about": "Справка", + "about": "О программе", "are_you_sure": "Вы уверены?", - "bulk_delete_content": "Вы уверены, что хотите удалить %{name}? |||| Вы уверены, что хотите удалить объекты, кол-вом %{smart_count} ? |||| Вы уверены, что хотите удалить объекты, кол-вом %{smart_count} ?", - "bulk_delete_title": "Удалить %{name} |||| Удалить %{smart_count} %{name} |||| Удалить %{smart_count} %{name}", - "delete_content": "Вы уверены что хотите удалить этот объект", + "bulk_delete_content": "Вы уверены, что хотите удалить %{name}? |||| Удалить %{smart_count} объекта? |||| Удалить %{smart_count} объектов?", + "bulk_delete_title": "Удалить %{name} |||| Удалить %{smart_count} объекта |||| Удалить %{smart_count} объектов", + "delete_content": "Вы уверены, что хотите удалить этот объект?", "delete_title": "Удалить %{name} #%{id}", - "details": "Описание", - "error": "При выполнении запроса возникла ошибка, и он не может быть завершен", - "invalid_form": "Форма заполнена неверно, проверьте, пожалуйста, ошибки", - "loading": "Идет загрузка, пожалуйста, немного подождите", + "details": "Подробности", + "error": "При выполнении запроса возникла ошибка", + "invalid_form": "Форма заполнена неверно, проверьте ошибки", + "loading": "Загрузка, пожалуйста, подождите...", "no": "Нет", - "not_found": "Либо вы ввели неправильный URL, либо перешли по некорректной ссылке.", + "not_found": "Страница не найдена. Возможно, вы ввели неправильный URL.", "yes": "Да", - "unsaved_changes": "Некоторые из ваших изменений не сохранены. Продолжить без сохранения?" + "unsaved_changes": "Есть несохраненные изменения. Продолжить без сохранения?" }, "navigation": { "no_results": "Результатов не найдено", - "no_more_results": "Страница %{page} выходит за пределы нумерации, попробуйте предыдущую", - "page_out_of_boundaries": "Страница %{page} выходит за пределы нумерации", + "no_more_results": "Страница %{page} выходит за пределы, попробуйте предыдущую", + "page_out_of_boundaries": "Страница %{page} выходит за пределы", "page_out_from_end": "Невозможно переместиться дальше последней страницы", "page_out_from_begin": "Номер страницы не может быть меньше 1", "page_range_info": "%{offsetBegin}-%{offsetEnd} из %{total}", "page_rows_per_page": "Строк на странице:", - "next": "Следующая", - "prev": "Предыдущая", - "skip_nav": "Перейти к содержанию" + "next": "Вперед", + "prev": "Назад", + "skip_nav": "Перейти к основному контенту" }, "notification": { - "updated": "Элемент обновлен |||| %{smart_count} обновлено |||| %{smart_count} обновлено", + "updated": "Элемент обновлен |||| %{smart_count} элемента обновлены |||| %{smart_count} элементов обновлено", "created": "Элемент создан", - "deleted": "Элемент удален |||| %{smart_count} удалено |||| %{smart_count} удалено", - "bad_item": "Неправильный элемент", + "deleted": "Элемент удален |||| %{smart_count} элемента удалены |||| %{smart_count} элементов удалено", + "bad_item": "Некорректный элемент", "item_doesnt_exist": "Элемент не существует", "http_error": "Ошибка сервера", - "data_provider_error": "Ошибка dataProvider, проверьте консоль", - "i18n_error": "Не удалось загрузить перевод для указанного языка", + "data_provider_error": "Ошибка поставщика данных, проверьте консоль", + "i18n_error": "Не удалось загрузить перевод", "canceled": "Операция отменена", - "logged_out": "Ваша сессия завершена, попробуйте переподключиться/войти снова", - "new_version": "Доступна новая версия! Пожалуйста, обновите это окно." + "logged_out": "Сессия завершена, пожалуйста, войдите снова", + "new_version": "Доступна новая версия! Пожалуйста, обновите страницу." }, "toggleFieldsMenu": { "columnsToDisplay": "Отображение столбцов", @@ -556,42 +558,42 @@ }, "message": { "note": "ПРИМЕЧАНИЕ", - "transcodingDisabled": "Изменение настроек транскодирования через веб интерфейс, отключено по соображениям безопасности. Если вы хотите изменить или добавить опции транскодирования, перезапустите сервер с опцией конфигурации %{config}.", - "transcodingEnabled": "Navidrome работает с настройками %{config}, позволяющими запускать команды с настройками транскодирования через веб интерфейс. В целях безопасности, мы рекомендуем отключить эту возможность.", - "songsAddedToPlaylist": "Один трек добавлен в плейлист |||| %{smart_count} треков добавлено в плейлист", + "transcodingDisabled": "Изменение настроек транскодирования через веб-интерфейс отключено по соображениям безопасности. Если вы хотите изменить или добавить опции транскодирования, перезапустите сервер с опцией конфигурации %{config}.", + "transcodingEnabled": "Navidrome работает с настройками %{config}, позволяющими запускать команды транскодирования через веб-интерфейс. В целях безопасности мы рекомендуем отключить эту возможность.", + "songsAddedToPlaylist": "Добавлен 1 трек |||| Добавлены %{smart_count} трека |||| Добавлено %{smart_count} треков", "noPlaylistsAvailable": "Недоступно", "delete_user_title": "Удалить пользователя '%{name}'", - "delete_user_content": "Вы уверены, что вы хотите удалить пользователя и все его данные (включая плейлисты и настройки)?", + "delete_user_content": "Вы уверены, что хотите удалить пользователя и все его данные (включая плейлисты и настройки)?", "notifications_blocked": "Вы заблокировали уведомления для этой страницы в настройках вашего браузера", "notifications_not_available": "Ваш браузер не поддерживает всплывающие уведомления", "lastfmLinkSuccess": "Соединение с Last.fm установлено, скробблинг включен", - "lastfmLinkFailure": "Last.fm не может быть подключен", - "lastfmUnlinkSuccess": "Соединение с Last.fm удалено, скробблинг отключен", - "lastfmUnlinkFailure": "Соединение с Last.fm не может быть удалено", + "lastfmLinkFailure": "Не удалось подключиться к Last.fm", + "lastfmUnlinkSuccess": "Соединение с Last.fm разорвано, скробблинг отключен", + "lastfmUnlinkFailure": "Не удалось разорвать соединение с Last.fm", "openIn": { "lastfm": "Показать на Last.fm", "musicbrainz": "Показать на MusicBrainz" }, "lastfmLink": "Подробнее...", "listenBrainzLinkSuccess": "ListenBrainz скробблинг успешно подключен для пользователя: %{user}", - "listenBrainzLinkFailure": "ListenBrainz не может быть связан:", + "listenBrainzLinkFailure": "Не удалось подключить ListenBrainz:", "listenBrainzUnlinkSuccess": "ListenBrainz скробблинг отключен", - "listenBrainzUnlinkFailure": "ListenBrainz не удалось отключить", + "listenBrainzUnlinkFailure": "Не удалось отключить ListenBrainz", "downloadOriginalFormat": "Скачать в оригинальном формате", "shareOriginalFormat": "Поделиться в оригинальном формате", "shareDialogTitle": "Поделиться %{resource} '%{name}'", - "shareBatchDialogTitle": "Поделиться 1 %{resource} |||| Поделиться %{smart_count} %{resource}", + "shareBatchDialogTitle": "Поделиться 1 %{resource} |||| Поделиться %{smart_count} %{resource} |||| Поделиться %{smart_count} %{resource}", "shareSuccess": "URL скопирован в буфер обмена: %{url}", - "shareFailure": "Ошибка копирования URL-адреса %{url} в буфер обмена", + "shareFailure": "Ошибка копирования URL %{url} в буфер обмена", "downloadDialogTitle": "Скачать %{resource} '%{name}' (%{size})", "shareCopyToClipboard": "Копировать в буфер обмена: Ctrl+C, Enter", "remove_missing_title": "Удалить отсутствующие файлы?", "remove_missing_content": "Вы уверены, что хотите удалить выбранные отсутствующие файлы из базы данных? Это навсегда удалит все ссылки на них, включая данные о прослушиваниях и рейтингах.", - "remove_all_missing_title": "Удалите все отсутствующие файлы", - "remove_all_missing_content": "Вы уверены, что хотите удалить все отсутствующие файлы из базы данных? Это навсегда удалит все упоминания о них, включая количество игр и рейтинг.", + "remove_all_missing_title": "Удалить все отсутствующие файлы", + "remove_all_missing_content": "Вы уверены, что хотите удалить все отсутствующие файлы из базы данных? Это навсегда удалит все упоминания о них, включая количество прослушиваний и рейтинг.", "noSimilarSongsFound": "Похожих треков не найдено", "noTopSongsFound": "Лучших треков не найдено", - "startingInstantMix": "Загрузка быстрого микса" + "startingInstantMix": "Загрузка быстрого микса..." }, "menu": { "library": "Библиотека", @@ -599,7 +601,7 @@ "version": "Версия", "theme": "Тема", "personal": { - "name": "Личные", + "name": "Личное", "options": { "theme": "Тема", "language": "Язык", @@ -607,10 +609,10 @@ "desktop_notifications": "Уведомления на рабочем столе", "lastfmScrobbling": "Скробблинг Last.fm", "listenBrainzScrobbling": "Скробблинг ListenBrainz", - "replaygain": "ReplayGain режим", - "preAmp": "ReplayGain предусилитель (dB)", + "replaygain": "Режим ReplayGain", + "preAmp": "Предусилитель ReplayGain (дБ)", "gain": { - "none": "Отключить", + "none": "Отключено", "album": "Использовать усиление альбома", "track": "Использовать усиление трека" }, @@ -620,16 +622,16 @@ "albumList": "Альбомы", "about": "О программе", "playlists": "Плейлисты", - "sharedPlaylists": "Поделиться плейлистом", + "sharedPlaylists": "Общие плейлисты", "librarySelector": { "allLibraries": "Все библиотеки (%{count})", - "multipleLibraries": "%{selected} из %{total} Библиотеки", + "multipleLibraries": "%{selected} из %{total} библиотек |||| %{selected} из %{total} библиотек |||| %{selected} из %{total} библиотек", "selectLibraries": "Выбор библиотек", "none": "Отсутствует" } }, "player": { - "playListsText": "Очередь Воспроизведения", + "playListsText": "Очередь воспроизведения", "openText": "Открыть", "closeText": "Закрыть", "notContentText": "Нет музыки", @@ -643,19 +645,19 @@ "toggleMiniModeText": "Свернуть", "destroyText": "Выключить", "downloadText": "Скачать", - "removeAudioListsText": "Удалить список воспроизведения", + "removeAudioListsText": "Очистить очередь", "clickToDeleteText": "Нажмите для удаления %{name}", - "emptyLyricText": "Без текста", + "emptyLyricText": "Текст песни отсутствует", "playModeText": { "order": "По порядку", "orderLoop": "Повторять", - "singleLoop": "Повторить один раз", + "singleLoop": "Повторять один трек", "shufflePlay": "Перемешать" } }, "about": { "links": { - "homepage": "Главная", + "homepage": "Сайт проекта", "source": "Исходный код", "featureRequests": "Предложения", "lastInsightsCollection": "Последний сбор данных", @@ -665,51 +667,51 @@ } }, "tabs": { - "about": "О нас", + "about": "О программе", "config": "Конфигурация" }, "config": { - "configName": "Имя конфигурации", - "environmentVariable": "Переменная среды", + "configName": "Параметр", + "environmentVariable": "Переменная окружения", "currentValue": "Текущее значение", "configurationFile": "Файл конфигурации", - "exportToml": "Экспорт конфигурации (TOML)", - "exportSuccess": "Конфигурация экспортирована в буфер обмена в формате TOML", + "exportToml": "Экспорт в TOML", + "exportSuccess": "Конфигурация скопирована в буфер обмена в формате TOML", "exportFailed": "Не удалось скопировать конфигурацию", "devFlagsHeader": "Флаги разработки (могут быть изменены/удалены)", "devFlagsComment": "Это экспериментальные настройки, которые могут быть удалены в будущих версиях.", - "downloadToml": "Скачать конфигурацию (TOML)" + "downloadToml": "Скачать TOML" } }, "activity": { - "title": "Действия", + "title": "Активность", "totalScanned": "Всего просканировано папок", "quickScan": "Быстрое сканирование", "fullScan": "Полное сканирование", "serverUptime": "Время работы сервера", - "serverDown": "Оффлайн", + "serverDown": "Офлайн", "scanType": "Тип", - "status": "Ошибка сканирования", + "status": "Статус", "elapsedTime": "Прошедшее время", - "selectiveScan": "Избирательный" + "selectiveScan": "Избирательное" }, "help": { "title": "Горячие клавиши Navidrome", "hotkeys": { "show_help": "Показать справку", - "toggle_menu": "Показать / скрыть боковое меню", - "toggle_play": "Играть / Пауза", + "toggle_menu": "Показать/скрыть боковое меню", + "toggle_play": "Играть/Пауза", "prev_song": "Предыдущий трек", "next_song": "Следующий трек", "vol_up": "Увеличить громкость", "vol_down": "Уменьшить громкость", - "toggle_love": "Добавить / удалить песню из избранного", + "toggle_love": "Добавить/удалить из избранного", "current_song": "Перейти к текущему треку" } }, "nowPlaying": { "title": "Сейчас играет", "empty": "Ничего не играет", - "minutesAgo": "%{smart_count} минут назад |||| %{smart_count} минут назад" + "minutesAgo": "%{smart_count} минуту назад |||| %{smart_count} минуты назад |||| %{smart_count} минут назад" } } diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index 6c6592178..74fb23ab9 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -22,6 +22,8 @@ "bitRate": "Bit rate", "bitDepth": "Bit depth", "sampleRate": "Sample rate", + "albumGain": "Album gain", + "trackGain": "Track gain", "channels": "Channels", "disc": "Disc %{discNumber}", "discSubtitle": "Disc Subtitle", From aa84e645ba5be6d1f3d50b38c6e03558803e6627 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 12 Apr 2026 13:22:56 -0400 Subject: [PATCH 11/18] fix(ui): add albumGain and trackGain translations in Brazilian Portuguese Signed-off-by: Deluan --- resources/i18n/pt-br.json | 2 ++ 1 file changed, 2 insertions(+) diff --git a/resources/i18n/pt-br.json b/resources/i18n/pt-br.json index 2e4f517a9..d9f29f5d4 100644 --- a/resources/i18n/pt-br.json +++ b/resources/i18n/pt-br.json @@ -35,6 +35,8 @@ "rawTags": "Tags originais", "bitDepth": "Profundidade de bits", "sampleRate": "Taxa de amostragem", + "albumGain": "Ganho do álbum", + "trackGain": "Ganho da faixa", "missing": "Ausente", "libraryName": "Biblioteca", "composer": "Compositor", From 52e47b896a3f2b9a7e3683b01758baae51374f46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sun, 12 Apr 2026 16:47:22 -0400 Subject: [PATCH 12/18] refactor: extract song-to-library matcher to core/matcher package (#5348) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor: extract matchSongsToLibrary to core/matcher package Move the song-to-library matching algorithm from core/external into its own core/matcher package. The Matcher struct exposes a single public method MatchSongsToLibrary that implements a multi-phase matching algorithm (ID > MBID > ISRC > fuzzy title+artist). Includes pre-sanitization optimization for the fuzzy matching loop. No behavioral changes — the algorithm is identical to the version in core/external/provider_matching.go. * refactor: inject matcher.Matcher via Wire instead of creating it inline Add *matcher.Matcher as a dependency of external.NewProvider, wired via Google Wire. Update all provider test files to pass matcher.New(ds). This eliminates tight coupling so future consumers can reuse the matcher without depending on the external package. * refactor: remove old provider_matching files Delete core/external/provider_matching.go and its tests. All matching logic now lives in core/matcher/. * test(matcher): restore test coverage lost in extraction Port back 23 specs that existed in the old provider_matching_test.go but were dropped during the extraction. Covers specificity levels, fuzzy matching thresholds, fuzzy album matching, duration matching, and deduplication edge cases. * test(matcher): extract matchFieldInAnd/matchFieldInEq helpers The four inline mock.MatchedBy closures in setupAllPhaseExpectations all followed the same squirrel.And -> squirrel.Eq -> field-name-check pattern. Extract into two small helpers to reduce duplication and make the setup functions read as a concise list of phase expectations. * refactor(matcher): address PR #5348 review feedback - sanitizedTrack now holds *model.MediaFile instead of a value copy. Since MediaFile is a large struct (~74 fields), this avoids the per-track copy into sanitized[] and a second copy when findBestMatch assigns the winner. loadTracksByTitleAndArtist updated to iterate by index and pass &tracks[i]. - loadTracksByISRC now sorts results (starred desc, rating desc, year asc, compilation asc) so that when multiple library tracks share an ISRC the most relevant one is picked deterministically, matching the sort order already used by loadTracksByTitleAndArtist. - Restored the four worked examples (MBID Priority, ISRC Priority, Specificity Ranking, Fuzzy Title Matching) in the MatchSongsToLibrary godoc that were dropped during the extraction. - matcher_test.go: tests now enforce expectations via AssertExpectations in a DeferCleanup. The old setupAllPhaseExpectations helper was replaced with per-phase helpers (expectIDPhase/expectMBIDPhase/expectISRCPhase + allowOtherPhases) so each test deterministically verifies which matching phases fire. This surfaced (and fixes) a latent issue copilot flagged: the old .Once() expectations were not actually asserted, so tests would silently pass even when phases short-circuited unexpectedly. --- cmd/wire_gen.go | 16 +- core/external/provider.go | 10 +- core/external/provider_albumimage_test.go | 3 +- core/external/provider_artistimage_test.go | 3 +- core/external/provider_matching_test.go | 762 ----------------- core/external/provider_similarsongs_test.go | 3 +- core/external/provider_topsongs_test.go | 3 +- .../external/provider_updatealbuminfo_test.go | 3 +- .../provider_updateartistinfo_test.go | 3 +- .../matcher.go} | 208 ++--- .../matcher_internal_test.go} | 6 +- core/matcher/matcher_suite_test.go | 17 + core/matcher/matcher_test.go | 807 ++++++++++++++++++ core/wire_providers.go | 2 + 14 files changed, 947 insertions(+), 899 deletions(-) delete mode 100644 core/external/provider_matching_test.go rename core/{external/provider_matching.go => matcher/matcher.go} (57%) rename core/{external/provider_matching_internal_test.go => matcher/matcher_internal_test.go} (89%) create mode 100644 core/matcher/matcher_suite_test.go create mode 100644 core/matcher/matcher_test.go diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index 5b9fd648f..b25b4c100 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -17,6 +17,7 @@ import ( "github.com/navidrome/navidrome/core/external" "github.com/navidrome/navidrome/core/ffmpeg" "github.com/navidrome/navidrome/core/lyrics" + "github.com/navidrome/navidrome/core/matcher" "github.com/navidrome/navidrome/core/metrics" "github.com/navidrome/navidrome/core/playback" "github.com/navidrome/navidrome/core/playlists" @@ -72,7 +73,8 @@ func CreateNativeAPIRouter(ctx context.Context) *nativeapi.Router { metricsMetrics := metrics.GetPrometheusInstance(dataStore) manager := plugins.GetManager(dataStore, broker, metricsMetrics) agentsAgents := agents.GetAgents(dataStore, manager) - provider := external.NewProvider(dataStore, agentsAgents) + matcherMatcher := matcher.New(dataStore) + provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher) artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, provider) cacheWarmer := artwork.NewCacheWarmer(artworkArtwork, fileCache) modelScanner := scanner.New(ctx, dataStore, cacheWarmer, broker, playlistsPlaylists, metricsMetrics) @@ -93,7 +95,8 @@ func CreateSubsonicAPIRouter(ctx context.Context) *subsonic.Router { metricsMetrics := metrics.GetPrometheusInstance(dataStore) manager := plugins.GetManager(dataStore, broker, metricsMetrics) agentsAgents := agents.GetAgents(dataStore, manager) - provider := external.NewProvider(dataStore, agentsAgents) + matcherMatcher := matcher.New(dataStore) + provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher) artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, provider) transcodingCache := stream.GetTranscodingCache() mediaStreamer := stream.NewMediaStreamer(dataStore, fFmpeg, transcodingCache) @@ -121,7 +124,8 @@ func CreatePublicRouter() *public.Router { metricsMetrics := metrics.GetPrometheusInstance(dataStore) manager := plugins.GetManager(dataStore, broker, metricsMetrics) agentsAgents := agents.GetAgents(dataStore, manager) - provider := external.NewProvider(dataStore, agentsAgents) + matcherMatcher := matcher.New(dataStore) + provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher) artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, provider) transcodingCache := stream.GetTranscodingCache() mediaStreamer := stream.NewMediaStreamer(dataStore, fFmpeg, transcodingCache) @@ -168,7 +172,8 @@ func CreateScanner(ctx context.Context) model.Scanner { metricsMetrics := metrics.GetPrometheusInstance(dataStore) manager := plugins.GetManager(dataStore, broker, metricsMetrics) agentsAgents := agents.GetAgents(dataStore, manager) - provider := external.NewProvider(dataStore, agentsAgents) + matcherMatcher := matcher.New(dataStore) + provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher) artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, provider) cacheWarmer := artwork.NewCacheWarmer(artworkArtwork, fileCache) imageUploadService := core.NewImageUploadService() @@ -186,7 +191,8 @@ func CreateScanWatcher(ctx context.Context) scanner.Watcher { metricsMetrics := metrics.GetPrometheusInstance(dataStore) manager := plugins.GetManager(dataStore, broker, metricsMetrics) agentsAgents := agents.GetAgents(dataStore, manager) - provider := external.NewProvider(dataStore, agentsAgents) + matcherMatcher := matcher.New(dataStore) + provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher) artworkArtwork := artwork.NewArtwork(dataStore, fileCache, fFmpeg, provider) cacheWarmer := artwork.NewCacheWarmer(artworkArtwork, fileCache) imageUploadService := core.NewImageUploadService() diff --git a/core/external/provider.go b/core/external/provider.go index 40ca34069..7e8aaba1c 100644 --- a/core/external/provider.go +++ b/core/external/provider.go @@ -12,6 +12,7 @@ import ( "github.com/Masterminds/squirrel" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/core/agents" + "github.com/navidrome/navidrome/core/matcher" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/utils" @@ -41,6 +42,7 @@ type Provider interface { type provider struct { ds model.DataStore ag Agents + matcher *matcher.Matcher artistQueue refreshQueue[auxArtist] albumQueue refreshQueue[auxAlbum] } @@ -85,8 +87,8 @@ type Agents interface { agents.SimilarSongsByArtistRetriever } -func NewProvider(ds model.DataStore, agents Agents) Provider { - e := &provider{ds: ds, ag: agents} +func NewProvider(ds model.DataStore, agents Agents, m *matcher.Matcher) Provider { + e := &provider{ds: ds, ag: agents, matcher: m} e.artistQueue = newRefreshQueue(context.TODO(), e.populateArtistInfo) e.albumQueue = newRefreshQueue(context.TODO(), e.populateAlbumInfo) return e @@ -300,7 +302,7 @@ func (e *provider) SimilarSongs(ctx context.Context, id string, count int) (mode } if err == nil && len(songs) > 0 { - return e.matchSongsToLibrary(ctx, songs, count) + return e.matcher.MatchSongsToLibrary(ctx, songs, count) } // Fallback to existing similar artists + top songs algorithm @@ -479,7 +481,7 @@ func (e *provider) getMatchingTopSongs(ctx context.Context, agent agents.ArtistT } } - mfs, err := e.matchSongsToLibrary(ctx, songs, count) + mfs, err := e.matcher.MatchSongsToLibrary(ctx, songs, count) if err != nil { return nil, err } diff --git a/core/external/provider_albumimage_test.go b/core/external/provider_albumimage_test.go index 8a81b4f4d..e801b7cce 100644 --- a/core/external/provider_albumimage_test.go +++ b/core/external/provider_albumimage_test.go @@ -9,6 +9,7 @@ import ( "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" . "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/core/matcher" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" @@ -43,7 +44,7 @@ var _ = Describe("Provider - AlbumImage", func() { mockAlbumAgent = newMockAlbumInfoAgent() agentsCombined := &mockAgents{albumInfoAgent: mockAlbumAgent} - provider = NewProvider(ds, agentsCombined) + provider = NewProvider(ds, agentsCombined, matcher.New(ds)) // Default mocks // Mocks for GetEntityByID sequence (initial failed lookups) diff --git a/core/external/provider_artistimage_test.go b/core/external/provider_artistimage_test.go index 529289ed3..37d3fd81a 100644 --- a/core/external/provider_artistimage_test.go +++ b/core/external/provider_artistimage_test.go @@ -11,6 +11,7 @@ import ( "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" . "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/core/matcher" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" @@ -51,7 +52,7 @@ var _ = Describe("Provider - ArtistImage", func() { imageAgent: mockImageAgent, } - provider = NewProvider(ds, agentsCombined) + provider = NewProvider(ds, agentsCombined, matcher.New(ds)) // Default mocks for successful Get calls mockArtistRepo.On("Get", "artist-1").Return(&model.Artist{ID: "artist-1", Name: "Artist One"}, nil).Maybe() diff --git a/core/external/provider_matching_test.go b/core/external/provider_matching_test.go deleted file mode 100644 index b3624ef3a..000000000 --- a/core/external/provider_matching_test.go +++ /dev/null @@ -1,762 +0,0 @@ -package external_test - -import ( - "context" - - "github.com/Masterminds/squirrel" - "github.com/navidrome/navidrome/conf" - "github.com/navidrome/navidrome/conf/configtest" - "github.com/navidrome/navidrome/core/agents" - . "github.com/navidrome/navidrome/core/external" - "github.com/navidrome/navidrome/model" - "github.com/navidrome/navidrome/tests" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" - "github.com/stretchr/testify/mock" -) - -var _ = Describe("Provider - Song Matching", func() { - var ds model.DataStore - var provider Provider - var agentsCombined *mockAgents - var artistRepo *mockArtistRepo - var mediaFileRepo *mockMediaFileRepo - var albumRepo *mockAlbumRepo - var ctx context.Context - - BeforeEach(func() { - ctx = GinkgoT().Context() - - artistRepo = newMockArtistRepo() - mediaFileRepo = newMockMediaFileRepo() - albumRepo = newMockAlbumRepo() - - ds = &tests.MockDataStore{ - MockedArtist: artistRepo, - MockedMediaFile: mediaFileRepo, - MockedAlbum: albumRepo, - } - - agentsCombined = &mockAgents{} - 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 - - BeforeEach(func() { - DeferCleanup(configtest.SetupConfig()) - // Disable fuzzy matching for these tests to avoid unexpected GetAll calls - conf.Server.SimilarSongsMatchThreshold = 100 - - track = model.MediaFile{ID: "track-1", Title: "Test Track", Artist: "Test Artist", MbzRecordingID: ""} - - // 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() - }) - - setupExpectations := func(returnedSongs []agents.Song, idMatches, mbidMatches, artistTracks model.MediaFiles) { - agentsCombined.On("GetSimilarSongsByTrack", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything). - Return(returnedSongs, nil).Once() - - // loadTracksByID - mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - _, ok := opt.Filters.(squirrel.Eq) - return ok - })).Return(idMatches, nil).Once() - - // loadTracksByMBID - mediaFileRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - and, ok := opt.Filters.(squirrel.And) - if !ok || len(and) < 1 { - return false - } - eq, hasEq := and[0].(squirrel.Eq) - if !hasEq { - return false - } - _, hasMBID := eq["mbz_recording_id"] - return hasMBID - })).Return(mbidMatches, nil).Once() - - // loadTracksByTitleAndArtist - now 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() - } - - Context("when agent returns artist and album metadata", func() { - It("matches by title + artist MBID + album MBID (highest priority)", func() { - // Song in library with all MBIDs - correctMatch := model.MediaFile{ - ID: "correct-match", Title: "Similar Song", Artist: "Depeche Mode", Album: "Violator", - MbzArtistID: "artist-mbid-123", MbzAlbumID: "album-mbid-456", - } - // Another song with same title but different MBIDs (should NOT match) - wrongMatch := model.MediaFile{ - ID: "wrong-match", Title: "Similar Song", Artist: "Depeche Mode", Album: "Some Other Album", - MbzArtistID: "artist-mbid-123", MbzAlbumID: "different-album-mbid", - } - returnedSongs := []agents.Song{ - {Name: "Similar Song", Artist: "Depeche Mode", ArtistMBID: "artist-mbid-123", Album: "Violator", AlbumMBID: "album-mbid-456"}, - } - - setupExpectations(returnedSongs, model.MediaFiles{}, model.MediaFiles{}, model.MediaFiles{wrongMatch, 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-match")) - }) - - It("matches by title + artist name + album name when MBIDs unavailable", func() { - // Song in library without MBIDs but with matching artist/album names - correctMatch := model.MediaFile{ - ID: "correct-match", Title: "Similar Song", Artist: "depeche mode", Album: "violator", - } - // Another song with same title but different artist (should NOT match) - wrongMatch := model.MediaFile{ - ID: "wrong-match", Title: "Similar Song", Artist: "Other Artist", Album: "Other Album", - } - - returnedSongs := []agents.Song{ - {Name: "Similar Song", Artist: "Depeche Mode", Album: "Violator"}, // No MBIDs - } - - setupExpectations(returnedSongs, model.MediaFiles{}, model.MediaFiles{}, model.MediaFiles{wrongMatch, 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-match")) - }) - - It("matches by title + artist only when album info unavailable", func() { - // Song in library with matching artist - correctMatch := model.MediaFile{ - ID: "correct-match", Title: "Similar Song", Artist: "depeche mode", Album: "Some Album", - } - // Another song with same title but different artist - wrongMatch := model.MediaFile{ - ID: "wrong-match", Title: "Similar Song", Artist: "Other Artist", Album: "Other Album", - } - returnedSongs := []agents.Song{ - {Name: "Similar Song", Artist: "Depeche Mode"}, // No album info - } - - setupExpectations(returnedSongs, model.MediaFiles{}, model.MediaFiles{}, model.MediaFiles{wrongMatch, 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-match")) - }) - - It("does not match songs without artist info", func() { - // Songs without artist info cannot be matched since we query by artist - returnedSongs := []agents.Song{ - {Name: "Similar Song"}, // No artist/album info at all - } - - // No artist to query, so no GetAll calls for title matching - setupExpectations(returnedSongs, model.MediaFiles{}, model.MediaFiles{}, model.MediaFiles{}) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(BeEmpty()) - }) - }) - - Context("when matching multiple songs with the same title but different artists", func() { - It("returns distinct matches for each artist's version (covers scenario)", func() { - // Multiple covers of the same song by different artists - cover1 := model.MediaFile{ - ID: "cover-1", Title: "Yesterday", Artist: "The Beatles", Album: "Help!", - } - cover2 := model.MediaFile{ - ID: "cover-2", Title: "Yesterday", Artist: "Ray Charles", Album: "Greatest Hits", - } - cover3 := model.MediaFile{ - ID: "cover-3", Title: "Yesterday", Artist: "Frank Sinatra", Album: "My Way", - } - - returnedSongs := []agents.Song{ - {Name: "Yesterday", Artist: "The Beatles", Album: "Help!"}, - {Name: "Yesterday", Artist: "Ray Charles", Album: "Greatest Hits"}, - {Name: "Yesterday", Artist: "Frank Sinatra", Album: "My Way"}, - } - - setupExpectations(returnedSongs, model.MediaFiles{}, model.MediaFiles{}, model.MediaFiles{cover1, cover2, cover3}) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - // All three covers should be returned, not just the first one - Expect(songs).To(HaveLen(3)) - // Verify all three different versions are included - ids := []string{songs[0].ID, songs[1].ID, songs[2].ID} - Expect(ids).To(ContainElements("cover-1", "cover-2", "cover-3")) - }) - }) - - Context("when matching multiple songs with different precision levels", func() { - It("prefers more precise matches for each song", func() { - // Library has multiple versions of same song - preciseMatch := model.MediaFile{ - ID: "precise", Title: "Song A", Artist: "Artist One", Album: "Album One", - MbzArtistID: "mbid-1", MbzAlbumID: "album-mbid-1", - } - lessAccurateMatch := model.MediaFile{ - ID: "less-accurate", Title: "Song A", Artist: "Artist One", Album: "Compilation", - MbzArtistID: "mbid-1", - } - artistTwoMatch := model.MediaFile{ - ID: "artist-two", Title: "Song B", Artist: "Artist Two", - } - - returnedSongs := []agents.Song{ - {Name: "Song A", Artist: "Artist One", ArtistMBID: "mbid-1", Album: "Album One", AlbumMBID: "album-mbid-1"}, - {Name: "Song B", Artist: "Artist Two"}, // Different artist - } - - setupExpectations(returnedSongs, model.MediaFiles{}, model.MediaFiles{}, model.MediaFiles{lessAccurateMatch, preciseMatch, artistTwoMatch}) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(2)) - // First song should be the precise match (has all MBIDs) - Expect(songs[0].ID).To(Equal("precise")) - // Second song matches by title + artist - Expect(songs[1].ID).To(Equal("artist-two")) - }) - }) - }) - - Describe("Fuzzy matching fallback", func() { - var track model.MediaFile - - BeforeEach(func() { - DeferCleanup(configtest.SetupConfig()) - 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("with default threshold (85%)", func() { - It("matches songs with remastered suffix", func() { - conf.Server.SimilarSongsMatchThreshold = 85 - - // Agent returns "Paranoid Android" but library has "Paranoid Android - Remastered" - returnedSongs := []agents.Song{ - {Name: "Paranoid Android", Artist: "Radiohead"}, - } - // Artist catalog has the remastered version (fuzzy match will find it) - artistTracks := model.MediaFiles{ - {ID: "remastered", Title: "Paranoid Android - Remastered", Artist: "Radiohead"}, - } - - setupSimilarSongsExpectations(returnedSongs, artistTracks) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(1)) - Expect(songs[0].ID).To(Equal("remastered")) - }) - - It("matches songs with live suffix", func() { - conf.Server.SimilarSongsMatchThreshold = 85 - - returnedSongs := []agents.Song{ - {Name: "Bohemian Rhapsody", Artist: "Queen"}, - } - artistTracks := model.MediaFiles{ - {ID: "live", Title: "Bohemian Rhapsody (Live)", Artist: "Queen"}, - } - - setupSimilarSongsExpectations(returnedSongs, artistTracks) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(1)) - Expect(songs[0].ID).To(Equal("live")) - }) - - It("does not match completely different songs", func() { - conf.Server.SimilarSongsMatchThreshold = 85 - - returnedSongs := []agents.Song{ - {Name: "Yesterday", Artist: "The Beatles"}, - } - // Artist catalog has completely different songs - artistTracks := model.MediaFiles{ - {ID: "different", Title: "Tomorrow Never Knows", Artist: "The Beatles"}, - {ID: "different2", Title: "Here Comes The Sun", Artist: "The Beatles"}, - } - - setupSimilarSongsExpectations(returnedSongs, artistTracks) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(BeEmpty()) - }) - }) - - Context("with threshold set to 100 (exact match only)", func() { - It("only matches exact titles", func() { - conf.Server.SimilarSongsMatchThreshold = 100 - - returnedSongs := []agents.Song{ - {Name: "Paranoid Android", Artist: "Radiohead"}, - } - // Artist catalog has only remastered version - no exact match - artistTracks := model.MediaFiles{ - {ID: "remastered", Title: "Paranoid Android - Remastered", Artist: "Radiohead"}, - } - - setupSimilarSongsExpectations(returnedSongs, artistTracks) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(BeEmpty()) - }) - }) - - Context("with lower threshold (75%)", func() { - It("matches more aggressively", func() { - conf.Server.SimilarSongsMatchThreshold = 75 - - returnedSongs := []agents.Song{ - {Name: "Song", Artist: "Artist"}, - } - artistTracks := model.MediaFiles{ - {ID: "extended", Title: "Song (Extended Mix)", Artist: "Artist"}, - } - - setupSimilarSongsExpectations(returnedSongs, artistTracks) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(1)) - Expect(songs[0].ID).To(Equal("extended")) - }) - }) - - Context("with fuzzy album matching", func() { - It("matches album with (Remaster) suffix", func() { - conf.Server.SimilarSongsMatchThreshold = 85 - - // Agent returns "A Night at the Opera" but library has remastered version - returnedSongs := []agents.Song{ - {Name: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera"}, - } - // Library has same album with remaster suffix - correctMatch := model.MediaFile{ - ID: "correct", Title: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera (2011 Remaster)", - } - wrongMatch := model.MediaFile{ - ID: "wrong", Title: "Bohemian Rhapsody", Artist: "Queen", Album: "Greatest Hits", - } - - setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{wrongMatch, correctMatch}) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(1)) - // Should prefer the fuzzy album match (Level 3) over title+artist only (Level 1) - Expect(songs[0].ID).To(Equal("correct")) - }) - - It("matches album with (Deluxe Edition) suffix", func() { - conf.Server.SimilarSongsMatchThreshold = 85 - - returnedSongs := []agents.Song{ - {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, - } - correctMatch := model.MediaFile{ - ID: "correct", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator (Deluxe Edition)", - } - wrongMatch := model.MediaFile{ - ID: "wrong", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "101", - } - - setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{wrongMatch, 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("prefers exact album match over fuzzy album match", func() { - conf.Server.SimilarSongsMatchThreshold = 85 - - returnedSongs := []agents.Song{ - {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, - } - exactMatch := model.MediaFile{ - ID: "exact", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator", - } - fuzzyMatch := model.MediaFile{ - ID: "fuzzy", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator (Deluxe Edition)", - } - - setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{fuzzyMatch, exactMatch}) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(1)) - // Both have same title similarity (1.0), so should prefer exact album match (higher specificity via higher album similarity) - Expect(songs[0].ID).To(Equal("exact")) - }) - }) - }) - - Describe("Duration matching", 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 tracks with close duration", 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.5 seconds (close to target) - closeDuration := model.MediaFile{ - ID: "close-duration", Title: "Similar Song", Artist: "Test Artist", Duration: 182.5, - } - - setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{closeDuration}) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(1)) - Expect(songs[0].ID).To(Equal("close-duration")) - }) - - It("prefers closer duration over farther duration", func() { - // Agent returns song with duration 180000ms (180 seconds) - returnedSongs := []agents.Song{ - {Name: "Similar Song", Artist: "Test Artist", Duration: 180000}, - } - // Library has one close, one far - closeDuration := model.MediaFile{ - ID: "close", Title: "Similar Song", Artist: "Test Artist", Duration: 181.0, - } - farDuration := model.MediaFile{ - ID: "far", Title: "Similar Song", Artist: "Test Artist", Duration: 190.0, - } - - setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{farDuration, closeDuration}) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(1)) - Expect(songs[0].ID).To(Equal("close")) - }) - - It("still matches when no tracks have matching duration", 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()) - // Duration mismatch doesn't exclude the track; it's just scored lower - Expect(songs).To(HaveLen(1)) - Expect(songs[0].ID).To(Equal("different")) - }) - - It("prefers title match over duration match when titles differ", 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 (wins on title similarity) - 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()) - // Title similarity is the top priority, so the correct title wins despite duration mismatch - 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 close duration", func() { - // 30-second song with 1-second difference - 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")) - }) - }) - }) - - Describe("Deduplication of mismatched songs", func() { - var track model.MediaFile - - BeforeEach(func() { - DeferCleanup(configtest.SetupConfig()) - conf.Server.SimilarSongsMatchThreshold = 85 // Allow fuzzy matching - - 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() - }) - - It("removes duplicates when different input songs match the same library track", func() { - // Agent returns two different versions that will both fuzzy-match to the same library track - returnedSongs := []agents.Song{ - {Name: "Bohemian Rhapsody (Live)", Artist: "Queen"}, - {Name: "Bohemian Rhapsody (Original Mix)", Artist: "Queen"}, - } - // Library only has one version - libraryTrack := model.MediaFile{ - ID: "br-live", Title: "Bohemian Rhapsody (Live)", Artist: "Queen", - } - - setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{libraryTrack}) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - // Should only return one track, not two duplicates - Expect(songs).To(HaveLen(1)) - Expect(songs[0].ID).To(Equal("br-live")) - }) - - It("preserves duplicates when identical input songs match the same library track", func() { - // Agent returns the exact same song twice (intentional repetition) - returnedSongs := []agents.Song{ - {Name: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera"}, - {Name: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera"}, - } - // Library has matching track - libraryTrack := model.MediaFile{ - ID: "br", Title: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera", - } - - setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{libraryTrack}) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - // Should return two tracks since input songs were identical - Expect(songs).To(HaveLen(2)) - Expect(songs[0].ID).To(Equal("br")) - Expect(songs[1].ID).To(Equal("br")) - }) - - It("handles mixed scenario with both identical and different input songs", func() { - // Agent returns: Song A, Song B (different from A), Song A again (same as first) - // All three match to the same library track - returnedSongs := []agents.Song{ - {Name: "Yesterday", Artist: "The Beatles", Album: "Help!"}, - {Name: "Yesterday (Remastered)", Artist: "The Beatles", Album: "1"}, // Different version - {Name: "Yesterday", Artist: "The Beatles", Album: "Help!"}, // Same as first - {Name: "Yesterday (Anthology)", Artist: "The Beatles", Album: "Anthology"}, // Another different version - } - // Library only has one version - libraryTrack := model.MediaFile{ - ID: "yesterday", Title: "Yesterday", Artist: "The Beatles", Album: "Help!", - } - - setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{libraryTrack}) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - // Should return 2 tracks: - // 1. First "Yesterday" (original) - // 2. Third "Yesterday" (same as first, so kept) - // Skip: Second "Yesterday (Remastered)" (different input, same library track) - // Skip: Fourth "Yesterday (Anthology)" (different input, same library track) - Expect(songs).To(HaveLen(2)) - Expect(songs[0].ID).To(Equal("yesterday")) - Expect(songs[1].ID).To(Equal("yesterday")) - }) - - It("does not deduplicate songs that match different library tracks", func() { - // Agent returns different songs that match different library tracks - returnedSongs := []agents.Song{ - {Name: "Song A", Artist: "Artist"}, - {Name: "Song B", Artist: "Artist"}, - {Name: "Song C", Artist: "Artist"}, - } - // Library has all three songs - trackA := model.MediaFile{ID: "track-a", Title: "Song A", Artist: "Artist"} - trackB := model.MediaFile{ID: "track-b", Title: "Song B", Artist: "Artist"} - trackC := model.MediaFile{ID: "track-c", Title: "Song C", Artist: "Artist"} - - setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{trackA, trackB, trackC}) - - songs, err := provider.SimilarSongs(ctx, "track-1", 5) - - Expect(err).ToNot(HaveOccurred()) - // All three should be returned since they match different library tracks - Expect(songs).To(HaveLen(3)) - Expect(songs[0].ID).To(Equal("track-a")) - Expect(songs[1].ID).To(Equal("track-b")) - Expect(songs[2].ID).To(Equal("track-c")) - }) - - It("respects count limit after deduplication", func() { - // Agent returns 4 songs: 2 unique + 2 that would create duplicates - returnedSongs := []agents.Song{ - {Name: "Song A", Artist: "Artist"}, - {Name: "Song A (Live)", Artist: "Artist"}, // Different, matches same track - {Name: "Song B", Artist: "Artist"}, - {Name: "Song B (Remix)", Artist: "Artist"}, // Different, matches same track - } - trackA := model.MediaFile{ID: "track-a", Title: "Song A", Artist: "Artist"} - trackB := model.MediaFile{ID: "track-b", Title: "Song B", Artist: "Artist"} - - setupSimilarSongsExpectations(returnedSongs, model.MediaFiles{trackA, trackB}) - - // Request only 2 songs - songs, err := provider.SimilarSongs(ctx, "track-1", 2) - - Expect(err).ToNot(HaveOccurred()) - // Should return exactly 2: Song A and Song B (skipping duplicates) - Expect(songs).To(HaveLen(2)) - Expect(songs[0].ID).To(Equal("track-a")) - Expect(songs[1].ID).To(Equal("track-b")) - }) - }) -}) diff --git a/core/external/provider_similarsongs_test.go b/core/external/provider_similarsongs_test.go index 1491d394e..c9a1a64ef 100644 --- a/core/external/provider_similarsongs_test.go +++ b/core/external/provider_similarsongs_test.go @@ -7,6 +7,7 @@ import ( "github.com/Masterminds/squirrel" "github.com/navidrome/navidrome/core/agents" . "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/core/matcher" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" @@ -48,7 +49,7 @@ var _ = Describe("Provider - SimilarSongs", func() { similarAgent: mockSimilarAgent, } - provider = NewProvider(ds, agentsCombined) + provider = NewProvider(ds, agentsCombined, matcher.New(ds)) }) Describe("dispatch by entity type", func() { diff --git a/core/external/provider_topsongs_test.go b/core/external/provider_topsongs_test.go index b73c8ab3e..4bd0e5959 100644 --- a/core/external/provider_topsongs_test.go +++ b/core/external/provider_topsongs_test.go @@ -10,6 +10,7 @@ import ( "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" . "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/core/matcher" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" @@ -44,7 +45,7 @@ var _ = Describe("Provider - TopSongs", func() { ag = new(mockAgents) - p = NewProvider(ds, ag) + p = NewProvider(ds, ag, matcher.New(ds)) }) It("returns top songs for a known artist", func() { diff --git a/core/external/provider_updatealbuminfo_test.go b/core/external/provider_updatealbuminfo_test.go index 5f5d41a87..3dd8a587a 100644 --- a/core/external/provider_updatealbuminfo_test.go +++ b/core/external/provider_updatealbuminfo_test.go @@ -8,6 +8,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/core/agents" "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/core/matcher" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" @@ -34,7 +35,7 @@ var _ = Describe("Provider - UpdateAlbumInfo", func() { ctx = GinkgoT().Context() ds = new(tests.MockDataStore) ag = new(mockAgents) - p = external.NewProvider(ds, ag) + p = external.NewProvider(ds, ag, matcher.New(ds)) mockAlbumRepo = ds.Album(ctx).(*tests.MockAlbumRepo) conf.Server.DevAlbumInfoTimeToLive = 1 * time.Hour }) diff --git a/core/external/provider_updateartistinfo_test.go b/core/external/provider_updateartistinfo_test.go index 0c489eadd..e309ece6e 100644 --- a/core/external/provider_updateartistinfo_test.go +++ b/core/external/provider_updateartistinfo_test.go @@ -9,6 +9,7 @@ import ( "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" "github.com/navidrome/navidrome/core/external" + "github.com/navidrome/navidrome/core/matcher" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/tests" @@ -37,7 +38,7 @@ var _ = Describe("Provider - UpdateArtistInfo", func() { ctx = GinkgoT().Context() ds = new(tests.MockDataStore) ag = new(mockAgents) - p = external.NewProvider(ds, ag) + p = external.NewProvider(ds, ag, matcher.New(ds)) mockArtistRepo = ds.Artist(ctx).(*tests.MockArtistRepo) }) diff --git a/core/external/provider_matching.go b/core/matcher/matcher.go similarity index 57% rename from core/external/provider_matching.go rename to core/matcher/matcher.go index 74ad56d42..40d4dc160 100644 --- a/core/external/provider_matching.go +++ b/core/matcher/matcher.go @@ -1,4 +1,4 @@ -package external +package matcher import ( "context" @@ -13,7 +13,17 @@ import ( "github.com/xrash/smetrics" ) -// matchSongsToLibrary matches agent song results to local library tracks using a multi-phase +// Matcher matches agent song results to local library tracks. +type Matcher struct { + ds model.DataStore +} + +// New creates a new Matcher with the given DataStore. +func New(ds model.DataStore) *Matcher { + return &Matcher{ds: ds} +} + +// MatchSongsToLibrary matches agent song results to local library tracks using a multi-phase // matching algorithm that prioritizes accuracy over recall. // // # Algorithm Overview @@ -95,36 +105,34 @@ import ( // // Returns up to 'count' MediaFiles from the library that best match the input songs, // preserving the original order from the agent. Songs that cannot be matched are skipped. -func (e *provider) matchSongsToLibrary(ctx context.Context, songs []agents.Song, count int) (model.MediaFiles, error) { - idMatches, err := e.loadTracksByID(ctx, songs) +func (m *Matcher) MatchSongsToLibrary(ctx context.Context, songs []agents.Song, count int) (model.MediaFiles, error) { + idMatches, err := m.loadTracksByID(ctx, songs) if err != nil { return nil, fmt.Errorf("failed to load tracks by ID: %w", err) } - mbidMatches, err := e.loadTracksByMBID(ctx, songs, idMatches) + mbidMatches, err := m.loadTracksByMBID(ctx, songs, idMatches) if err != nil { return nil, fmt.Errorf("failed to load tracks by MBID: %w", err) } - isrcMatches, err := e.loadTracksByISRC(ctx, songs, idMatches, mbidMatches) + isrcMatches, err := m.loadTracksByISRC(ctx, songs, idMatches, mbidMatches) if err != nil { return nil, fmt.Errorf("failed to load tracks by ISRC: %w", err) } - titleMatches, err := e.loadTracksByTitleAndArtist(ctx, songs, idMatches, mbidMatches, isrcMatches) + titleMatches, err := m.loadTracksByTitleAndArtist(ctx, songs, idMatches, mbidMatches, isrcMatches) if err != nil { return nil, fmt.Errorf("failed to load tracks by title: %w", err) } - return e.selectBestMatchingSongs(songs, idMatches, mbidMatches, isrcMatches, titleMatches, count), nil + return m.selectBestMatchingSongs(songs, idMatches, mbidMatches, isrcMatches, titleMatches, count), nil } // songMatchedIn checks if a song has already been matched in any of the provided match maps. -// It checks the song's ID, MBID, and ISRC fields against the corresponding map keys. func songMatchedIn(s agents.Song, priorMatches ...map[string]model.MediaFile) bool { _, found := lookupByIdentifiers(s, priorMatches...) return found } // lookupByIdentifiers searches for a song's identifiers (ID, MBID, ISRC) in the provided maps. -// Returns the first matching MediaFile found and true, or an empty MediaFile and false if no match. func lookupByIdentifiers(s agents.Song, maps ...map[string]model.MediaFile) (model.MediaFile, bool) { keys := []string{s.ID, s.MBID, s.ISRC} for _, m := range maps { @@ -140,10 +148,7 @@ func lookupByIdentifiers(s agents.Song, maps ...map[string]model.MediaFile) (mod } // loadTracksByID fetches MediaFiles from the library using direct ID matching. -// It extracts all non-empty ID fields from the input songs and performs a single -// batch query to the database. Returns a map keyed by MediaFile ID for O(1) lookup. -// Only non-missing files are returned. -func (e *provider) loadTracksByID(ctx context.Context, songs []agents.Song) (map[string]model.MediaFile, error) { +func (m *Matcher) loadTracksByID(ctx context.Context, songs []agents.Song) (map[string]model.MediaFile, error) { var ids []string for _, s := range songs { if s.ID != "" { @@ -154,7 +159,7 @@ func (e *provider) loadTracksByID(ctx context.Context, songs []agents.Song) (map if len(ids) == 0 { return matches, nil } - res, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{ + res, err := m.ds.MediaFile(ctx).GetAll(model.QueryOptions{ Filters: squirrel.And{ squirrel.Eq{"media_file.id": ids}, squirrel.Eq{"missing": false}, @@ -172,10 +177,7 @@ func (e *provider) loadTracksByID(ctx context.Context, songs []agents.Song) (map } // loadTracksByMBID fetches MediaFiles from the library using MusicBrainz Recording IDs. -// It extracts all non-empty MBID fields from the input songs and performs a single -// batch query against the mbz_recording_id column. Returns a map keyed by MBID for -// O(1) lookup. Only non-missing files are returned. -func (e *provider) loadTracksByMBID(ctx context.Context, songs []agents.Song, priorMatches ...map[string]model.MediaFile) (map[string]model.MediaFile, error) { +func (m *Matcher) loadTracksByMBID(ctx context.Context, songs []agents.Song, priorMatches ...map[string]model.MediaFile) (map[string]model.MediaFile, error) { var mbids []string for _, s := range songs { if s.MBID != "" && !songMatchedIn(s, priorMatches...) { @@ -186,7 +188,7 @@ func (e *provider) loadTracksByMBID(ctx context.Context, songs []agents.Song, pr if len(mbids) == 0 { return matches, nil } - res, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{ + res, err := m.ds.MediaFile(ctx).GetAll(model.QueryOptions{ Filters: squirrel.And{ squirrel.Eq{"mbz_recording_id": mbids}, squirrel.Eq{"missing": false}, @@ -205,11 +207,8 @@ func (e *provider) loadTracksByMBID(ctx context.Context, songs []agents.Song, pr return matches, nil } -// loadTracksByISRC fetches MediaFiles from the library using ISRC (International Standard -// Recording Code) matching. It extracts all non-empty ISRC fields from the input songs and -// queries the tags JSON column for matching ISRC values. Returns a map keyed by ISRC for -// O(1) lookup. Only non-missing files are returned. -func (e *provider) loadTracksByISRC(ctx context.Context, songs []agents.Song, priorMatches ...map[string]model.MediaFile) (map[string]model.MediaFile, error) { +// loadTracksByISRC fetches MediaFiles from the library using ISRC matching. +func (m *Matcher) loadTracksByISRC(ctx context.Context, songs []agents.Song, priorMatches ...map[string]model.MediaFile) (map[string]model.MediaFile, error) { var isrcs []string for _, s := range songs { if s.ISRC != "" && !songMatchedIn(s, priorMatches...) { @@ -220,8 +219,9 @@ func (e *provider) loadTracksByISRC(ctx context.Context, songs []agents.Song, pr if len(isrcs) == 0 { return matches, nil } - res, err := e.ds.MediaFile(ctx).GetAllByTags(model.TagISRC, isrcs, model.QueryOptions{ + res, err := m.ds.MediaFile(ctx).GetAllByTags(model.TagISRC, isrcs, model.QueryOptions{ Filters: squirrel.Eq{"missing": false}, + Sort: "starred desc, rating desc, year asc, compilation asc", }) if err != nil { return matches, err @@ -237,27 +237,24 @@ func (e *provider) loadTracksByISRC(ctx context.Context, songs []agents.Song, pr } // songQuery represents a normalized query for matching a song to library tracks. -// All string fields are sanitized (lowercased, diacritics removed) for comparison. -// This struct is used internally by loadTracksByTitleAndArtist to group queries by artist. type songQuery struct { - title string // Sanitized song title - artist string // Sanitized artist name (without articles like "The") - 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) + title string + artist string + artistMBID string + album string + albumMBID string + durationMs uint32 } -// matchScore combines title/album similarity with metadata specificity for ranking matches +// matchScore combines title/album similarity with metadata specificity for ranking matches. type matchScore struct { - titleSimilarity float64 // 0.0-1.0 (Jaro-Winkler) - durationProximity float64 // 0.0-1.0 (closer duration = higher, 1.0 if unknown) - albumSimilarity float64 // 0.0-1.0 (Jaro-Winkler), used as tiebreaker - specificityLevel int // 0-5 (higher = more specific metadata match) + titleSimilarity float64 + durationProximity float64 + albumSimilarity float64 + specificityLevel int } // betterThan returns true if this score beats another. -// Comparison order: title similarity > duration proximity > specificity level > album similarity func (s matchScore) betterThan(other matchScore) bool { if s.titleSimilarity != other.titleSimilarity { return s.titleSimilarity > other.titleSimilarity @@ -271,58 +268,62 @@ func (s matchScore) betterThan(other matchScore) bool { return s.albumSimilarity > other.albumSimilarity } -// computeSpecificityLevel determines how well query metadata matches a track (0-5). -// Higher values indicate more specific matches (MBIDs > names > title only). -// Uses fuzzy matching for album names with the same threshold as title matching. -func computeSpecificityLevel(q songQuery, mf model.MediaFile, albumThreshold float64) int { - title := str.SanitizeFieldForSorting(mf.Title) - artist := str.SanitizeFieldForSortingNoArticle(mf.Artist) - album := str.SanitizeFieldForSorting(mf.Album) +// sanitizedTrack holds pre-sanitized fields for a media file, avoiding redundant sanitization +// when the same track is scored against multiple queries in the inner loop. The `mf` field +// is a pointer to avoid copying the large MediaFile struct into each entry of the per-artist +// sanitized slice. +type sanitizedTrack struct { + mf *model.MediaFile + title string + artist string + album string +} - // Level 5: Title + Artist MBID + Album MBID (most specific) +func newSanitizedTrack(mf *model.MediaFile) sanitizedTrack { + return sanitizedTrack{ + mf: mf, + title: str.SanitizeFieldForSorting(mf.Title), + artist: str.SanitizeFieldForSortingNoArticle(mf.Artist), + album: str.SanitizeFieldForSorting(mf.Album), + } +} + +// computeSpecificityLevel determines how well query metadata matches a track (0-5). +// The track's title, artist, and album fields must be pre-sanitized. +func computeSpecificityLevel(q songQuery, t sanitizedTrack, albumThreshold float64) int { if q.artistMBID != "" && q.albumMBID != "" && - mf.MbzArtistID == q.artistMBID && mf.MbzAlbumID == q.albumMBID { + t.mf.MbzArtistID == q.artistMBID && t.mf.MbzAlbumID == q.albumMBID { return 5 } - // Level 4: Title + Artist MBID + Album name (fuzzy) if q.artistMBID != "" && q.album != "" && - mf.MbzArtistID == q.artistMBID && similarityRatio(album, q.album) >= albumThreshold { + t.mf.MbzArtistID == q.artistMBID && similarityRatio(t.album, q.album) >= albumThreshold { return 4 } - // Level 3: Title + Artist name + Album name (fuzzy) if q.artist != "" && q.album != "" && - artist == q.artist && similarityRatio(album, q.album) >= albumThreshold { + t.artist == q.artist && similarityRatio(t.album, q.album) >= albumThreshold { return 3 } - // Level 2: Title + Artist MBID - if q.artistMBID != "" && mf.MbzArtistID == q.artistMBID { + if q.artistMBID != "" && t.mf.MbzArtistID == q.artistMBID { return 2 } - // Level 1: Title + Artist name - if q.artist != "" && artist == q.artist { + if q.artist != "" && t.artist == q.artist { return 1 } - // Level 0: Title only match (but for fuzzy, title matched via similarity) - // Check if at least the title matches exactly - if title == q.title { + if t.title == q.title { return 0 } - return -1 // No exact title match, but could still be a fuzzy match + return -1 } // loadTracksByTitleAndArtist loads tracks matching by title with optional artist/album filtering. -// Uses a unified scoring approach that combines title similarity (Jaro-Winkler) with -// metadata specificity (MBIDs, album names) for both exact and fuzzy matches. -// Returns a map keyed by "title|artist" for compatibility with selectBestMatchingSongs. -func (e *provider) loadTracksByTitleAndArtist(ctx context.Context, songs []agents.Song, priorMatches ...map[string]model.MediaFile) (map[string]model.MediaFile, error) { - queries := e.buildTitleQueries(songs, priorMatches...) +func (m *Matcher) loadTracksByTitleAndArtist(ctx context.Context, songs []agents.Song, priorMatches ...map[string]model.MediaFile) (map[string]model.MediaFile, error) { + queries := m.buildTitleQueries(songs, priorMatches...) if len(queries) == 0 { return map[string]model.MediaFile{}, nil } threshold := float64(conf.Server.SimilarSongsMatchThreshold) / 100.0 - // Group queries by artist for efficient DB access byArtist := map[string][]songQuery{} for _, q := range queries { if q.artist != "" { @@ -332,8 +333,7 @@ func (e *provider) loadTracksByTitleAndArtist(ctx context.Context, songs []agent matches := map[string]model.MediaFile{} for artist, artistQueries := range byArtist { - // Single DB query per artist - get all their tracks - tracks, err := e.ds.MediaFile(ctx).GetAll(model.QueryOptions{ + tracks, err := m.ds.MediaFile(ctx).GetAll(model.QueryOptions{ Filters: squirrel.And{ squirrel.Eq{"order_artist_name": artist}, squirrel.Eq{"missing": false}, @@ -344,9 +344,13 @@ func (e *provider) loadTracksByTitleAndArtist(ctx context.Context, songs []agent continue } - // Find best match for each query using unified scoring + sanitized := make([]sanitizedTrack, len(tracks)) + for i := range tracks { + sanitized[i] = newSanitizedTrack(&tracks[i]) + } + for _, q := range artistQueries { - if mf, found := e.findBestMatch(q, tracks, threshold); found { + if mf, found := m.findBestMatch(q, sanitized, threshold); found { key := q.title + "|" + q.artist if _, exists := matches[key]; !exists { matches[key] = mf @@ -357,13 +361,11 @@ func (e *provider) loadTracksByTitleAndArtist(ctx context.Context, songs []agent return matches, nil } -// durationProximity returns a score from 0.0 to 1.0 indicating how close -// the track's duration is to the target. A perfect match returns 1.0, and the -// score decreases as the difference grows (using 1 / (1 + diff)). Returns 1.0 -// if durationMs is 0 (unknown), so duration does not influence scoring. +// durationProximity returns a score from 0.0 to 1.0 indicating how close the track's duration +// is to the target. Returns 1.0 if durationMs is 0 (unknown). func durationProximity(durationMs uint32, mediaFileDurationSec float32) float64 { - if durationMs <= 0 { - return 1.0 // Unknown duration — don't penalise + if durationMs == 0 { + return 1.0 } durationSec := float64(durationMs) / 1000.0 diff := math.Abs(durationSec - float64(mediaFileDurationSec)) @@ -371,41 +373,33 @@ func durationProximity(durationMs uint32, mediaFileDurationSec float32) float64 } // findBestMatch finds the best matching track using combined title/album similarity and specificity scoring. -// A track must meet the threshold for title similarity, then the best match is chosen by: -// 1. Highest title similarity -// 2. Duration proximity (closer duration = higher score, 1.0 if unknown) -// 3. Highest specificity level -// 4. Highest album similarity (as final tiebreaker) -func (e *provider) findBestMatch(q songQuery, tracks model.MediaFiles, threshold float64) (model.MediaFile, bool) { +func (m *Matcher) findBestMatch(q songQuery, sanitizedTracks []sanitizedTrack, threshold float64) (model.MediaFile, bool) { var bestMatch model.MediaFile bestScore := matchScore{titleSimilarity: -1} found := false - for _, mf := range tracks { - trackTitle := str.SanitizeFieldForSorting(mf.Title) - titleSim := similarityRatio(q.title, trackTitle) + for _, t := range sanitizedTracks { + titleSim := similarityRatio(q.title, t.title) if titleSim < threshold { continue } - // Compute album similarity for tiebreaking (0.0 if no album in query) var albumSim float64 if q.album != "" { - trackAlbum := str.SanitizeFieldForSorting(mf.Album) - albumSim = similarityRatio(q.album, trackAlbum) + albumSim = similarityRatio(q.album, t.album) } score := matchScore{ titleSimilarity: titleSim, - durationProximity: durationProximity(q.durationMs, mf.Duration), + durationProximity: durationProximity(q.durationMs, t.mf.Duration), albumSimilarity: albumSim, - specificityLevel: computeSpecificityLevel(q, mf, threshold), + specificityLevel: computeSpecificityLevel(q, t, threshold), } if score.betterThan(bestScore) { bestScore = score - bestMatch = mf + bestMatch = *t.mf found = true } } @@ -413,9 +407,7 @@ func (e *provider) findBestMatch(q songQuery, tracks model.MediaFiles, threshold } // buildTitleQueries converts agent songs into normalized songQuery structs for title+artist matching. -// It skips songs that have already been matched in prior phases (by ID, MBID, or ISRC) and sanitizes -// all string fields for consistent comparison (lowercase, diacritics removed, articles stripped from artist names). -func (e *provider) buildTitleQueries(songs []agents.Song, priorMatches ...map[string]model.MediaFile) []songQuery { +func (m *Matcher) buildTitleQueries(songs []agents.Song, priorMatches ...map[string]model.MediaFile) []songQuery { var queries []songQuery for _, s := range songs { if songMatchedIn(s, priorMatches...) { @@ -434,18 +426,9 @@ func (e *provider) buildTitleQueries(songs []agents.Song, priorMatches ...map[st } // selectBestMatchingSongs assembles the final result by mapping input songs to their best matching -// library tracks. It iterates through the input songs in order and selects the first available match -// using priority order: ID > MBID > ISRC > title+artist. -// -// The function also handles deduplication: when multiple different input songs would match the same -// library track (e.g., "Song (Live)" and "Song (Remastered)" both matching "Song (Live)" in the library), -// only the first match is kept. However, if the same input song appears multiple times (intentional -// repetition), duplicates are preserved in the output. -// -// Returns up to 'count' MediaFiles, preserving the input order. Songs that cannot be matched are skipped. -func (e *provider) selectBestMatchingSongs(songs []agents.Song, byID, byMBID, byISRC, byTitleArtist map[string]model.MediaFile, count int) model.MediaFiles { +// library tracks using priority order: ID > MBID > ISRC > title+artist. +func (m *Matcher) selectBestMatchingSongs(songs []agents.Song, byID, byMBID, byISRC, byTitleArtist map[string]model.MediaFile, count int) model.MediaFiles { mfs := make(model.MediaFiles, 0, len(songs)) - // Track MediaFile.ID -> input song that added it, for deduplication addedBy := make(map[string]agents.Song, len(songs)) for _, t := range songs { @@ -458,11 +441,9 @@ func (e *provider) selectBestMatchingSongs(songs []agents.Song, byID, byMBID, by continue } - // Check for duplicate library track if prevSong, alreadyAdded := addedBy[mf.ID]; alreadyAdded { - // Only add duplicate if input songs are identical if t != prevSong { - continue // Different input songs → skip mismatch-induced duplicate + continue } } else { addedBy[mf.ID] = t @@ -473,14 +454,11 @@ func (e *provider) selectBestMatchingSongs(songs []agents.Song, byID, byMBID, by return mfs } -// findMatchingTrack looks up a song in the match maps using priority order: ID > MBID > ISRC > title+artist. -// Returns the matched MediaFile and true if found, or an empty MediaFile and false if no match exists. +// findMatchingTrack looks up a song in the match maps using priority order. func findMatchingTrack(t agents.Song, byID, byMBID, byISRC, byTitleArtist map[string]model.MediaFile) (model.MediaFile, bool) { - // Try identifier-based matches first (ID, MBID, ISRC) if mf, found := lookupByIdentifiers(t, byID, byMBID, byISRC); found { return mf, true } - // Fall back to title+artist fuzzy match key := str.SanitizeFieldForSorting(t.Name) + "|" + str.SanitizeFieldForSortingNoArticle(t.Artist) if mf, ok := byTitleArtist[key]; ok { return mf, true @@ -489,9 +467,6 @@ func findMatchingTrack(t agents.Song, byID, byMBID, byISRC, byTitleArtist map[st } // similarityRatio calculates the similarity between two strings using Jaro-Winkler algorithm. -// Returns a value between 0.0 (completely different) and 1.0 (identical). -// Jaro-Winkler is well-suited for matching song titles because it gives higher scores -// when strings share a common prefix (e.g., "Song Title" vs "Song Title - Remastered"). func similarityRatio(a, b string) float64 { if a == b { return 1.0 @@ -499,6 +474,5 @@ func similarityRatio(a, b string) float64 { if len(a) == 0 || len(b) == 0 { return 0.0 } - // JaroWinkler params: boostThreshold=0.7, prefixSize=4 return smetrics.JaroWinkler(a, b, 0.7, 4) } diff --git a/core/external/provider_matching_internal_test.go b/core/matcher/matcher_internal_test.go similarity index 89% rename from core/external/provider_matching_internal_test.go rename to core/matcher/matcher_internal_test.go index 5b9ccea3b..f111364c1 100644 --- a/core/external/provider_matching_internal_test.go +++ b/core/matcher/matcher_internal_test.go @@ -1,4 +1,4 @@ -package external +package matcher import ( . "github.com/onsi/ginkgo/v2" @@ -16,25 +16,21 @@ var _ = Describe("similarityRatio", func() { }) It("returns high similarity for remastered suffix", func() { - // Jaro-Winkler gives ~0.92 for this case ratio := similarityRatio("paranoid android", "paranoid android remastered") Expect(ratio).To(BeNumerically(">=", 0.85)) }) It("returns high similarity for suffix additions like (Live)", func() { - // Jaro-Winkler gives ~0.96 for this case ratio := similarityRatio("bohemian rhapsody", "bohemian rhapsody live") Expect(ratio).To(BeNumerically(">=", 0.90)) }) It("returns high similarity for 'yesterday' variants (common prefix)", func() { - // Jaro-Winkler gives ~0.90 because of common prefix ratio := similarityRatio("yesterday", "yesterday once more") Expect(ratio).To(BeNumerically(">=", 0.85)) }) It("returns low similarity for same suffix", func() { - // Jaro-Winkler gives ~0.70 for this case ratio := similarityRatio("postman (live)", "taxman (live)") Expect(ratio).To(BeNumerically("<", 0.85)) }) diff --git a/core/matcher/matcher_suite_test.go b/core/matcher/matcher_suite_test.go new file mode 100644 index 000000000..44877a3c8 --- /dev/null +++ b/core/matcher/matcher_suite_test.go @@ -0,0 +1,17 @@ +package matcher_test + +import ( + "testing" + + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +func TestMatcher(t *testing.T) { + tests.Init(t, false) + log.SetLevel(log.LevelFatal) + RegisterFailHandler(Fail) + RunSpecs(t, "Matcher Suite") +} diff --git a/core/matcher/matcher_test.go b/core/matcher/matcher_test.go new file mode 100644 index 000000000..b1f59b258 --- /dev/null +++ b/core/matcher/matcher_test.go @@ -0,0 +1,807 @@ +package matcher_test + +import ( + "context" + "errors" + + "github.com/Masterminds/squirrel" + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/core/agents" + "github.com/navidrome/navidrome/core/matcher" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/stretchr/testify/mock" +) + +var _ = Describe("Matcher", func() { + var ds model.DataStore + var mediaFileRepo *mockMediaFileRepo + var ctx context.Context + var m *matcher.Matcher + + BeforeEach(func() { + ctx = GinkgoT().Context() + DeferCleanup(configtest.SetupConfig()) + mediaFileRepo = newMockMediaFileRepo() + DeferCleanup(func() { + mediaFileRepo.AssertExpectations(GinkgoT()) + }) + ds = &tests.MockDataStore{ + MockedMediaFile: mediaFileRepo, + } + m = matcher.New(ds) + }) + + // Per-phase expectation helpers. Each `expect*Phase` registers a .Once() expectation + // that will fail the suite via AssertExpectations if the phase is NOT called. Tests + // use these to deterministically verify which matching phases fire. Phases that may + // or may not fire should use the `allow*Phase` variants instead, which register + // .Maybe() fallbacks. + expectIDPhase := func(matches model.MediaFiles) { + mediaFileRepo.On("GetAll", mock.MatchedBy(matchFieldInAnd("media_file.id"))). + Return(matches, nil).Once() + } + expectMBIDPhase := func(matches model.MediaFiles) { + mediaFileRepo.On("GetAll", mock.MatchedBy(matchFieldInAnd("mbz_recording_id"))). + Return(matches, nil).Once() + } + expectISRCPhase := func(matches model.MediaFiles) { + mediaFileRepo.On("GetAll", mock.MatchedBy(matchFieldInEq("missing"))). + Return(matches, nil).Once() + } + + // allowOtherPhases installs .Maybe() catch-alls so phases that short-circuit (return + // early without hitting the DB) don't cause test failures for unexpected calls. Call + // this after expect*Phase for the phases the test actually wants to verify. + allowOtherPhases := func() { + mediaFileRepo.On("GetAll", mock.MatchedBy(matchFieldInAnd("media_file.id"))). + Return(model.MediaFiles{}, nil).Maybe() + mediaFileRepo.On("GetAll", mock.MatchedBy(matchFieldInAnd("mbz_recording_id"))). + Return(model.MediaFiles{}, nil).Maybe() + mediaFileRepo.On("GetAll", mock.MatchedBy(matchFieldInEq("missing"))). + Return(model.MediaFiles{}, nil).Maybe() + mediaFileRepo.On("GetAll", mock.MatchedBy(matchFieldInAnd("order_artist_name"))). + Return(model.MediaFiles{}, nil).Maybe() + } + + // setupTitleOnlyExpectations is a convenience for fuzzy-match tests that only exercise + // the title+artist phase. The title phase uses .Maybe() because it may short-circuit + // when no songs have an artist. + setupTitleOnlyExpectations := func(artistTracks model.MediaFiles) { + mediaFileRepo.On("GetAll", mock.MatchedBy(matchFieldInAnd("order_artist_name"))). + Return(artistTracks, nil).Maybe() + } + + Describe("MatchSongsToLibrary", func() { + Context("matching by direct ID", func() { + It("matches songs with an ID field to MediaFiles by ID", func() { + conf.Server.SimilarSongsMatchThreshold = 100 + songs := []agents.Song{ + {ID: "track-1", Name: "Some Song", Artist: "Some Artist"}, + } + idMatch := model.MediaFile{ + ID: "track-1", Title: "Some Song", Artist: "Some Artist", + } + expectIDPhase(model.MediaFiles{idMatch}) + allowOtherPhases() + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("track-1")) + }) + }) + + Context("matching by MBID", func() { + It("matches songs with MBID to tracks with matching mbz_recording_id", func() { + conf.Server.SimilarSongsMatchThreshold = 100 + songs := []agents.Song{ + {Name: "Paranoid Android", MBID: "abc-123", Artist: "Radiohead"}, + } + mbidMatch := model.MediaFile{ + ID: "track-mbid", Title: "Paranoid Android", Artist: "Radiohead", + MbzRecordingID: "abc-123", + } + expectMBIDPhase(model.MediaFiles{mbidMatch}) + allowOtherPhases() + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("track-mbid")) + }) + }) + + Context("matching by ISRC", func() { + It("matches songs with ISRC to tracks with matching ISRC tag", func() { + conf.Server.SimilarSongsMatchThreshold = 100 + songs := []agents.Song{ + {Name: "Paranoid Android", ISRC: "GBAYE0000351", Artist: "Radiohead"}, + } + isrcMatch := model.MediaFile{ + ID: "track-isrc", Title: "Paranoid Android", Artist: "Radiohead", + Tags: model.Tags{model.TagISRC: []string{"GBAYE0000351"}}, + } + expectISRCPhase(model.MediaFiles{isrcMatch}) + allowOtherPhases() + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("track-isrc")) + }) + }) + + Context("fuzzy title+artist matching", func() { + It("matches songs by title and artist name", func() { + conf.Server.SimilarSongsMatchThreshold = 100 + songs := []agents.Song{ + {Name: "Enjoy the Silence", Artist: "Depeche Mode"}, + } + titleMatch := model.MediaFile{ + ID: "track-title", Title: "Enjoy the Silence", Artist: "Depeche Mode", + } + setupTitleOnlyExpectations(model.MediaFiles{titleMatch}) + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("track-title")) + }) + + It("matches songs with fuzzy title similarity", func() { + conf.Server.SimilarSongsMatchThreshold = 85 + songs := []agents.Song{ + {Name: "Bohemian Rhapsody", Artist: "Queen"}, + } + fuzzyMatch := model.MediaFile{ + ID: "track-fuzzy", Title: "Bohemian Rhapsody (Live)", Artist: "Queen", + } + setupTitleOnlyExpectations(model.MediaFiles{fuzzyMatch}) + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("track-fuzzy")) + }) + + It("does not match completely different titles", func() { + conf.Server.SimilarSongsMatchThreshold = 85 + songs := []agents.Song{ + {Name: "Yesterday", Artist: "The Beatles"}, + } + differentTracks := model.MediaFiles{ + {ID: "different", Title: "Tomorrow Never Knows", Artist: "The Beatles"}, + } + setupTitleOnlyExpectations(differentTracks) + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeEmpty()) + }) + }) + + Context("deduplication", func() { + It("removes duplicates when different input songs match the same library track", func() { + conf.Server.SimilarSongsMatchThreshold = 85 + songs := []agents.Song{ + {Name: "Bohemian Rhapsody (Live)", Artist: "Queen"}, + {Name: "Bohemian Rhapsody (Original Mix)", Artist: "Queen"}, + } + libraryTrack := model.MediaFile{ + ID: "br-live", Title: "Bohemian Rhapsody (Live)", Artist: "Queen", + } + setupTitleOnlyExpectations(model.MediaFiles{libraryTrack}) + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("br-live")) + }) + + It("preserves duplicates when identical input songs match the same library track", func() { + conf.Server.SimilarSongsMatchThreshold = 85 + songs := []agents.Song{ + {Name: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera"}, + {Name: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera"}, + } + libraryTrack := model.MediaFile{ + ID: "br", Title: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera", + } + setupTitleOnlyExpectations(model.MediaFiles{libraryTrack}) + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(2)) + Expect(result[0].ID).To(Equal("br")) + Expect(result[1].ID).To(Equal("br")) + }) + }) + + Context("priority ordering", func() { + It("prefers ID match over MBID match", func() { + conf.Server.SimilarSongsMatchThreshold = 100 + // Song has both ID and MBID set. The matcher should resolve via ID + // and short-circuit the MBID phase entirely, so no MBID fetch should + // occur even though an mbz_recording_id exists in the input. + songs := []agents.Song{ + {ID: "track-id", Name: "Song", MBID: "mbid-1", Artist: "Artist"}, + } + idMatch := model.MediaFile{ + ID: "track-id", Title: "Song", Artist: "Artist", + } + expectIDPhase(model.MediaFiles{idMatch}) + allowOtherPhases() + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("track-id")) + }) + }) + + Context("count limit", func() { + It("returns at most 'count' results", func() { + conf.Server.SimilarSongsMatchThreshold = 100 + songs := []agents.Song{ + {Name: "Song A", Artist: "Artist"}, + {Name: "Song B", Artist: "Artist"}, + {Name: "Song C", Artist: "Artist"}, + } + tracks := model.MediaFiles{ + {ID: "a", Title: "Song A", Artist: "Artist"}, + {ID: "b", Title: "Song B", Artist: "Artist"}, + {ID: "c", Title: "Song C", Artist: "Artist"}, + } + setupTitleOnlyExpectations(tracks) + result, err := m.MatchSongsToLibrary(ctx, songs, 2) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(2)) + }) + }) + + Context("empty input", func() { + It("returns empty results for no songs", func() { + result, err := m.MatchSongsToLibrary(ctx, []agents.Song{}, 5) + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeEmpty()) + }) + }) + }) + + Describe("specificity level matching", func() { + BeforeEach(func() { + conf.Server.SimilarSongsMatchThreshold = 100 + }) + + It("matches by title + artist MBID + album MBID (highest priority)", func() { + correctMatch := model.MediaFile{ + ID: "correct-match", Title: "Similar Song", Artist: "Depeche Mode", Album: "Violator", + MbzArtistID: "artist-mbid-123", MbzAlbumID: "album-mbid-456", + } + wrongMatch := model.MediaFile{ + ID: "wrong-match", Title: "Similar Song", Artist: "Depeche Mode", Album: "Some Other Album", + MbzArtistID: "artist-mbid-123", MbzAlbumID: "different-album-mbid", + } + songs := []agents.Song{ + {Name: "Similar Song", Artist: "Depeche Mode", ArtistMBID: "artist-mbid-123", Album: "Violator", AlbumMBID: "album-mbid-456"}, + } + + setupTitleOnlyExpectations(model.MediaFiles{wrongMatch, correctMatch}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("correct-match")) + }) + + It("matches by title + artist name + album name when MBIDs unavailable", func() { + correctMatch := model.MediaFile{ + ID: "correct-match", Title: "Similar Song", Artist: "depeche mode", Album: "violator", + } + wrongMatch := model.MediaFile{ + ID: "wrong-match", Title: "Similar Song", Artist: "Other Artist", Album: "Other Album", + } + songs := []agents.Song{ + {Name: "Similar Song", Artist: "Depeche Mode", Album: "Violator"}, + } + + setupTitleOnlyExpectations(model.MediaFiles{wrongMatch, correctMatch}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("correct-match")) + }) + + It("matches by title + artist only when album info unavailable", func() { + correctMatch := model.MediaFile{ + ID: "correct-match", Title: "Similar Song", Artist: "depeche mode", Album: "Some Album", + } + wrongMatch := model.MediaFile{ + ID: "wrong-match", Title: "Similar Song", Artist: "Other Artist", Album: "Other Album", + } + songs := []agents.Song{ + {Name: "Similar Song", Artist: "Depeche Mode"}, + } + + setupTitleOnlyExpectations(model.MediaFiles{wrongMatch, correctMatch}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("correct-match")) + }) + + It("does not match songs without artist info", func() { + songs := []agents.Song{ + {Name: "Similar Song"}, + } + + setupTitleOnlyExpectations(model.MediaFiles{}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeEmpty()) + }) + + It("returns distinct matches for each artist's version (covers scenario)", func() { + cover1 := model.MediaFile{ID: "cover-1", Title: "Yesterday", Artist: "The Beatles", Album: "Help!"} + cover2 := model.MediaFile{ID: "cover-2", Title: "Yesterday", Artist: "Ray Charles", Album: "Greatest Hits"} + cover3 := model.MediaFile{ID: "cover-3", Title: "Yesterday", Artist: "Frank Sinatra", Album: "My Way"} + + songs := []agents.Song{ + {Name: "Yesterday", Artist: "The Beatles", Album: "Help!"}, + {Name: "Yesterday", Artist: "Ray Charles", Album: "Greatest Hits"}, + {Name: "Yesterday", Artist: "Frank Sinatra", Album: "My Way"}, + } + + setupTitleOnlyExpectations(model.MediaFiles{cover1, cover2, cover3}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(3)) + ids := []string{result[0].ID, result[1].ID, result[2].ID} + Expect(ids).To(ContainElements("cover-1", "cover-2", "cover-3")) + }) + + It("prefers more precise matches for each song", func() { + preciseMatch := model.MediaFile{ + ID: "precise", Title: "Song A", Artist: "Artist One", Album: "Album One", + MbzArtistID: "mbid-1", MbzAlbumID: "album-mbid-1", + } + lessAccurateMatch := model.MediaFile{ + ID: "less-accurate", Title: "Song A", Artist: "Artist One", Album: "Compilation", + MbzArtistID: "mbid-1", + } + artistTwoMatch := model.MediaFile{ + ID: "artist-two", Title: "Song B", Artist: "Artist Two", + } + + songs := []agents.Song{ + {Name: "Song A", Artist: "Artist One", ArtistMBID: "mbid-1", Album: "Album One", AlbumMBID: "album-mbid-1"}, + {Name: "Song B", Artist: "Artist Two"}, + } + + setupTitleOnlyExpectations(model.MediaFiles{lessAccurateMatch, preciseMatch, artistTwoMatch}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(2)) + Expect(result[0].ID).To(Equal("precise")) + Expect(result[1].ID).To(Equal("artist-two")) + }) + }) + + Describe("fuzzy matching thresholds", func() { + Context("with default threshold (85%)", func() { + It("matches songs with remastered suffix", func() { + conf.Server.SimilarSongsMatchThreshold = 85 + + songs := []agents.Song{ + {Name: "Paranoid Android", Artist: "Radiohead"}, + } + artistTracks := model.MediaFiles{ + {ID: "remastered", Title: "Paranoid Android - Remastered", Artist: "Radiohead"}, + } + + setupTitleOnlyExpectations(artistTracks) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("remastered")) + }) + + It("matches songs with live suffix", func() { + conf.Server.SimilarSongsMatchThreshold = 85 + + songs := []agents.Song{ + {Name: "Bohemian Rhapsody", Artist: "Queen"}, + } + artistTracks := model.MediaFiles{ + {ID: "live", Title: "Bohemian Rhapsody (Live)", Artist: "Queen"}, + } + + setupTitleOnlyExpectations(artistTracks) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("live")) + }) + }) + + Context("with threshold set to 100 (exact match only)", func() { + It("only matches exact titles", func() { + conf.Server.SimilarSongsMatchThreshold = 100 + + songs := []agents.Song{ + {Name: "Paranoid Android", Artist: "Radiohead"}, + } + artistTracks := model.MediaFiles{ + {ID: "remastered", Title: "Paranoid Android - Remastered", Artist: "Radiohead"}, + } + + setupTitleOnlyExpectations(artistTracks) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(BeEmpty()) + }) + }) + + Context("with lower threshold (75%)", func() { + It("matches more aggressively", func() { + conf.Server.SimilarSongsMatchThreshold = 75 + + songs := []agents.Song{ + {Name: "Song", Artist: "Artist"}, + } + artistTracks := model.MediaFiles{ + {ID: "extended", Title: "Song (Extended Mix)", Artist: "Artist"}, + } + + setupTitleOnlyExpectations(artistTracks) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("extended")) + }) + }) + }) + + Describe("fuzzy album matching", func() { + BeforeEach(func() { + conf.Server.SimilarSongsMatchThreshold = 85 + }) + + It("matches album with (Remaster) suffix", func() { + songs := []agents.Song{ + {Name: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera"}, + } + correctMatch := model.MediaFile{ + ID: "correct", Title: "Bohemian Rhapsody", Artist: "Queen", Album: "A Night at the Opera (2011 Remaster)", + } + wrongMatch := model.MediaFile{ + ID: "wrong", Title: "Bohemian Rhapsody", Artist: "Queen", Album: "Greatest Hits", + } + + setupTitleOnlyExpectations(model.MediaFiles{wrongMatch, correctMatch}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("correct")) + }) + + It("matches album with (Deluxe Edition) suffix", func() { + songs := []agents.Song{ + {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, + } + correctMatch := model.MediaFile{ + ID: "correct", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator (Deluxe Edition)", + } + wrongMatch := model.MediaFile{ + ID: "wrong", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "101", + } + + setupTitleOnlyExpectations(model.MediaFiles{wrongMatch, correctMatch}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("correct")) + }) + + It("prefers exact album match over fuzzy album match", func() { + songs := []agents.Song{ + {Name: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator"}, + } + exactMatch := model.MediaFile{ + ID: "exact", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator", + } + fuzzyMatch := model.MediaFile{ + ID: "fuzzy", Title: "Enjoy the Silence", Artist: "Depeche Mode", Album: "Violator (Deluxe Edition)", + } + + setupTitleOnlyExpectations(model.MediaFiles{fuzzyMatch, exactMatch}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("exact")) + }) + }) + + Describe("duration matching", func() { + BeforeEach(func() { + conf.Server.SimilarSongsMatchThreshold = 100 + }) + + It("prefers tracks with matching duration", func() { + songs := []agents.Song{ + {Name: "Similar Song", Artist: "Test Artist", Duration: 180000}, + } + 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, + } + + setupTitleOnlyExpectations(model.MediaFiles{wrongDuration, correctMatch}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("correct")) + }) + + It("matches tracks with close duration", func() { + songs := []agents.Song{ + {Name: "Similar Song", Artist: "Test Artist", Duration: 180000}, + } + closeDuration := model.MediaFile{ + ID: "close-duration", Title: "Similar Song", Artist: "Test Artist", Duration: 182.5, + } + + setupTitleOnlyExpectations(model.MediaFiles{closeDuration}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("close-duration")) + }) + + It("prefers closer duration over farther duration", func() { + songs := []agents.Song{ + {Name: "Similar Song", Artist: "Test Artist", Duration: 180000}, + } + closeDuration := model.MediaFile{ + ID: "close", Title: "Similar Song", Artist: "Test Artist", Duration: 181.0, + } + farDuration := model.MediaFile{ + ID: "far", Title: "Similar Song", Artist: "Test Artist", Duration: 190.0, + } + + setupTitleOnlyExpectations(model.MediaFiles{farDuration, closeDuration}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("close")) + }) + + It("still matches when no tracks have matching duration", func() { + songs := []agents.Song{ + {Name: "Similar Song", Artist: "Test Artist", Duration: 180000}, + } + differentDuration := model.MediaFile{ + ID: "different", Title: "Similar Song", Artist: "Test Artist", Duration: 300.0, + } + + setupTitleOnlyExpectations(model.MediaFiles{differentDuration}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("different")) + }) + + It("prefers title match over duration match when titles differ", func() { + songs := []agents.Song{ + {Name: "Similar Song", Artist: "Test Artist", Duration: 180000}, + } + 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, + } + + setupTitleOnlyExpectations(model.MediaFiles{differentTitle, correctTitle}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("correct-title")) + }) + + It("matches without duration filtering when agent duration is 0", func() { + songs := []agents.Song{ + {Name: "Similar Song", Artist: "Test Artist", Duration: 0}, + } + anyTrack := model.MediaFile{ + ID: "any", Title: "Similar Song", Artist: "Test Artist", Duration: 999.0, + } + + setupTitleOnlyExpectations(model.MediaFiles{anyTrack}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("any")) + }) + + It("handles very short songs with close duration", func() { + songs := []agents.Song{ + {Name: "Short Song", Artist: "Test Artist", Duration: 30000}, + } + shortTrack := model.MediaFile{ + ID: "short", Title: "Short Song", Artist: "Test Artist", Duration: 31.0, + } + + setupTitleOnlyExpectations(model.MediaFiles{shortTrack}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(1)) + Expect(result[0].ID).To(Equal("short")) + }) + }) + + Describe("deduplication edge cases", func() { + BeforeEach(func() { + conf.Server.SimilarSongsMatchThreshold = 85 + }) + + It("handles mixed scenario with both identical and different input songs", func() { + songs := []agents.Song{ + {Name: "Yesterday", Artist: "The Beatles", Album: "Help!"}, + {Name: "Yesterday (Remastered)", Artist: "The Beatles", Album: "1"}, + {Name: "Yesterday", Artist: "The Beatles", Album: "Help!"}, + {Name: "Yesterday (Anthology)", Artist: "The Beatles", Album: "Anthology"}, + } + libraryTrack := model.MediaFile{ + ID: "yesterday", Title: "Yesterday", Artist: "The Beatles", Album: "Help!", + } + + setupTitleOnlyExpectations(model.MediaFiles{libraryTrack}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(2)) + Expect(result[0].ID).To(Equal("yesterday")) + Expect(result[1].ID).To(Equal("yesterday")) + }) + + It("does not deduplicate songs that match different library tracks", func() { + songs := []agents.Song{ + {Name: "Song A", Artist: "Artist"}, + {Name: "Song B", Artist: "Artist"}, + {Name: "Song C", Artist: "Artist"}, + } + trackA := model.MediaFile{ID: "track-a", Title: "Song A", Artist: "Artist"} + trackB := model.MediaFile{ID: "track-b", Title: "Song B", Artist: "Artist"} + trackC := model.MediaFile{ID: "track-c", Title: "Song C", Artist: "Artist"} + + setupTitleOnlyExpectations(model.MediaFiles{trackA, trackB, trackC}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 5) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(3)) + Expect(result[0].ID).To(Equal("track-a")) + Expect(result[1].ID).To(Equal("track-b")) + Expect(result[2].ID).To(Equal("track-c")) + }) + + It("respects count limit after deduplication", func() { + songs := []agents.Song{ + {Name: "Song A", Artist: "Artist"}, + {Name: "Song A (Live)", Artist: "Artist"}, + {Name: "Song B", Artist: "Artist"}, + {Name: "Song B (Remix)", Artist: "Artist"}, + } + trackA := model.MediaFile{ID: "track-a", Title: "Song A", Artist: "Artist"} + trackB := model.MediaFile{ID: "track-b", Title: "Song B", Artist: "Artist"} + + setupTitleOnlyExpectations(model.MediaFiles{trackA, trackB}) + + result, err := m.MatchSongsToLibrary(ctx, songs, 2) + + Expect(err).ToNot(HaveOccurred()) + Expect(result).To(HaveLen(2)) + Expect(result[0].ID).To(Equal("track-a")) + Expect(result[1].ID).To(Equal("track-b")) + }) + }) +}) + +type mockMediaFileRepo struct { + mock.Mock + model.MediaFileRepository +} + +func newMockMediaFileRepo() *mockMediaFileRepo { + return &mockMediaFileRepo{} +} + +func (m *mockMediaFileRepo) GetAll(options ...model.QueryOptions) (model.MediaFiles, error) { + argsSlice := make([]any, len(options)) + for i, v := range options { + argsSlice[i] = v + } + args := m.Called(argsSlice...) + if args.Get(0) == nil { + return nil, args.Error(1) + } + return args.Get(0).(model.MediaFiles), args.Error(1) +} + +func (m *mockMediaFileRepo) GetAllByTags(_ model.TagName, _ []string, options ...model.QueryOptions) (model.MediaFiles, error) { + return m.GetAll(options...) +} + +func (m *mockMediaFileRepo) SetError(hasError bool) { + if hasError { + m.On("GetAll", mock.Anything).Return(nil, errors.New("mock repo error")) + } +} + +// matchFieldInAnd returns a matcher that checks whether QueryOptions.Filters is a +// squirrel.And whose first element is a squirrel.Eq containing the given field name. +func matchFieldInAnd(fieldName string) func(opt model.QueryOptions) bool { + return 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 + } + _, hasField := eq[fieldName] + return hasField + } +} + +// matchFieldInEq returns a matcher that checks whether QueryOptions.Filters is a +// squirrel.Eq containing the given field name. +func matchFieldInEq(fieldName string) func(opt model.QueryOptions) bool { + return func(opt model.QueryOptions) bool { + eq, ok := opt.Filters.(squirrel.Eq) + if !ok { + return false + } + _, hasField := eq[fieldName] + return hasField + } +} diff --git a/core/wire_providers.go b/core/wire_providers.go index 276d9556a..a2fffa34f 100644 --- a/core/wire_providers.go +++ b/core/wire_providers.go @@ -6,6 +6,7 @@ import ( "github.com/navidrome/navidrome/core/external" "github.com/navidrome/navidrome/core/ffmpeg" "github.com/navidrome/navidrome/core/lyrics" + "github.com/navidrome/navidrome/core/matcher" "github.com/navidrome/navidrome/core/metrics" "github.com/navidrome/navidrome/core/playback" "github.com/navidrome/navidrome/core/playlists" @@ -28,6 +29,7 @@ var Set = wire.NewSet( stream.NewTranscodeDecider, agents.GetAgents, external.NewProvider, + matcher.New, wire.Bind(new(external.Agents), new(*agents.Agents)), ffmpeg.New, scrobbler.GetPlayTracker, From 0a6b5519cc9d32f57dad4b4163d988260a33a791 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sun, 12 Apr 2026 21:52:29 -0400 Subject: [PATCH 13/18] refactor(scanner): remove C++ taglib adapter (#5349) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(build): remove CPP taglib adapter Remove the CGO-based TagLib adapter (adapters/taglib/) and all cross-taglib build infrastructure. The WASM-based go-taglib adapter (adapters/gotaglib/) is now the sole metadata extractor. - Delete adapters/taglib/ (CPP/CGO wrapper) - Delete .github/actions/download-taglib/ - Remove CROSS_TAGLIB_VERSION, CGO_CFLAGS_ALLOW, and all taglib-related references from Dockerfile, Makefile, CI pipeline, and devcontainer * fix(scanner): gracefully fallback to default extractor instead of crashing Replace log.Fatal with a graceful fallback when the configured scanner extractor is not found. Instead of terminating the process, the code now warns and falls back to the default taglib extractor using the existing consts.DefaultScannerExtractor constant. A fatal log is retained only for the case where the default extractor itself is not registered, which indicates a broken build. * test(scanner): cover default extractor fallback and suppress redundant warn Address review feedback on the extractor fallback in newLocalStorage: - Only log the "using default" warning when the configured extractor differs from the default, so a broken build (default extractor itself missing) logs only the fatal — not a misleading "falling back" warn followed immediately by the fatal. - Add a unit test that registers a mock under consts.DefaultScannerExtractor, sets the configured extractor to an unknown name, and asserts the local storage is constructed using the default extractor's constructor. --- .devcontainer/Dockerfile | 12 - .devcontainer/devcontainer.json | 3 +- .github/actions/download-taglib/action.yml | 23 -- .github/workflows/pipeline.yml | 18 +- Dockerfile | 25 +- Makefile | 5 - adapters/taglib/end_to_end_test.go | 274 ------------------- adapters/taglib/get_filename.go | 9 - adapters/taglib/get_filename_win.go | 96 ------- adapters/taglib/taglib.go | 178 ------------ adapters/taglib/taglib_suite_test.go | 17 -- adapters/taglib/taglib_test.go | 295 -------------------- adapters/taglib/taglib_wrapper.cpp | 299 --------------------- adapters/taglib/taglib_wrapper.go | 157 ----------- adapters/taglib/taglib_wrapper.h | 24 -- cmd/root.go | 1 - cmd/wire_gen.go | 1 - core/storage/local/local.go | 9 +- core/storage/local/local_test.go | 26 +- 19 files changed, 32 insertions(+), 1440 deletions(-) delete mode 100644 .github/actions/download-taglib/action.yml delete mode 100644 adapters/taglib/end_to_end_test.go delete mode 100644 adapters/taglib/get_filename.go delete mode 100644 adapters/taglib/get_filename_win.go delete mode 100644 adapters/taglib/taglib.go delete mode 100644 adapters/taglib/taglib_suite_test.go delete mode 100644 adapters/taglib/taglib_test.go delete mode 100644 adapters/taglib/taglib_wrapper.cpp delete mode 100644 adapters/taglib/taglib_wrapper.go delete mode 100644 adapters/taglib/taglib_wrapper.h diff --git a/.devcontainer/Dockerfile b/.devcontainer/Dockerfile index c7ccbf9fa..b2aa76450 100644 --- a/.devcontainer/Dockerfile +++ b/.devcontainer/Dockerfile @@ -13,17 +13,5 @@ RUN if [ "${INSTALL_NODE}" = "true" ]; then su vscode -c "source /usr/local/shar RUN apt-get update && export DEBIAN_FRONTEND=noninteractive \ && apt-get -y install --no-install-recommends ffmpeg -# Install TagLib from cross-taglib releases -ARG CROSS_TAGLIB_VERSION="2.2.0-1" -ARG TARGETARCH -RUN DOWNLOAD_ARCH="linux-${TARGETARCH}" \ - && wget -q "https://github.com/navidrome/cross-taglib/releases/download/v${CROSS_TAGLIB_VERSION}/taglib-${DOWNLOAD_ARCH}.tar.gz" -O /tmp/cross-taglib.tar.gz \ - && tar -xzf /tmp/cross-taglib.tar.gz -C /usr --strip-components=1 \ - && mv /usr/include/taglib/* /usr/include/ \ - && rmdir /usr/include/taglib \ - && rm /tmp/cross-taglib.tar.gz /usr/provenance.json - -ENV CGO_CFLAGS_ALLOW="--define-prefix" - # [Optional] Uncomment this line to install global node packages. # RUN su vscode -c "source /usr/local/share/nvm/nvm.sh && npm install -g " 2>&1 diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 81398a3ce..311090b91 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -7,8 +7,7 @@ "VARIANT": "1.25", // Options "INSTALL_NODE": "true", - "NODE_VERSION": "v24", - "CROSS_TAGLIB_VERSION": "2.2.0-1" + "NODE_VERSION": "v24" } }, "workspaceMount": "", diff --git a/.github/actions/download-taglib/action.yml b/.github/actions/download-taglib/action.yml deleted file mode 100644 index ea6de8783..000000000 --- a/.github/actions/download-taglib/action.yml +++ /dev/null @@ -1,23 +0,0 @@ -name: 'Download TagLib' -description: 'Downloads and extracts the TagLib library, adding it to PKG_CONFIG_PATH' -inputs: - version: - description: 'Version of TagLib to download' - required: true - platform: - description: 'Platform to download TagLib for' - default: 'linux-amd64' -runs: - using: 'composite' - steps: - - name: Download TagLib - shell: bash - run: | - mkdir -p /tmp/taglib - cd /tmp - FILE=taglib-${{ inputs.platform }}.tar.gz - wget https://github.com/navidrome/cross-taglib/releases/download/v${{ inputs.version }}/${FILE} - tar -xzf ${FILE} -C taglib - PKG_CONFIG_PREFIX=/tmp/taglib - echo "PKG_CONFIG_PREFIX=${PKG_CONFIG_PREFIX}" >> $GITHUB_ENV - echo "PKG_CONFIG_PATH=${PKG_CONFIG_PATH}:${PKG_CONFIG_PREFIX}/lib/pkgconfig" >> $GITHUB_ENV diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index 2529aaf36..e939f1d13 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -14,8 +14,6 @@ concurrency: cancel-in-progress: true env: - CROSS_TAGLIB_VERSION: "2.2.0-1" - CGO_CFLAGS_ALLOW: "--define-prefix" IS_RELEASE: ${{ startsWith(github.ref, 'refs/tags/') && 'true' || 'false' }} jobs: @@ -66,11 +64,6 @@ jobs: steps: - uses: actions/checkout@v6 - - name: Download TagLib - uses: ./.github/actions/download-taglib - with: - version: ${{ env.CROSS_TAGLIB_VERSION }} - - name: golangci-lint uses: golangci/golangci-lint-action@v9 with: @@ -106,18 +99,11 @@ jobs: - name: Check out code into the Go module directory uses: actions/checkout@v6 - - name: Download TagLib - uses: ./.github/actions/download-taglib - with: - version: ${{ env.CROSS_TAGLIB_VERSION }} - - name: Download dependencies run: go mod download - name: Test - run: | - pkg-config --define-prefix --cflags --libs taglib # for debugging - go test -shuffle=on -tags netgo,sqlite_fts5 -race ./... -v + run: go test -shuffle=on -tags netgo,sqlite_fts5 -race ./... -v - name: Test ndpgen run: | @@ -232,7 +218,6 @@ jobs: build-args: | GIT_SHA=${{ env.GIT_SHA }} GIT_TAG=${{ env.GIT_TAG }} - CROSS_TAGLIB_VERSION=${{ env.CROSS_TAGLIB_VERSION }} - name: Upload Binaries uses: actions/upload-artifact@v7 @@ -253,7 +238,6 @@ jobs: build-args: | GIT_SHA=${{ env.GIT_SHA }} GIT_TAG=${{ env.GIT_TAG }} - CROSS_TAGLIB_VERSION=${{ env.CROSS_TAGLIB_VERSION }} outputs: | type=image,name=${{ steps.docker.outputs.hub_repository }},push-by-digest=true,name-canonical=true,push=${{ steps.docker.outputs.hub_enabled }} type=image,name=ghcr.io/${{ github.repository }},push-by-digest=true,name-canonical=true,push=true diff --git a/Dockerfile b/Dockerfile index b32c1df56..f6ea14ff3 100644 --- a/Dockerfile +++ b/Dockerfile @@ -24,26 +24,6 @@ RUN cd /out && \ FROM scratch AS xx COPY --from=xx-build /out/ /usr/bin/ -######################################################################################################################## -### Get TagLib -FROM --platform=$BUILDPLATFORM public.ecr.aws/docker/library/alpine:3.20 AS taglib-build -ARG TARGETPLATFORM -ARG CROSS_TAGLIB_VERSION=2.2.0-1 -ENV CROSS_TAGLIB_RELEASES_URL=https://github.com/navidrome/cross-taglib/releases/download/v${CROSS_TAGLIB_VERSION}/ - -# wget in busybox can't follow redirects -RUN < - -const size_t SIZEOF_WCHAR_T = sizeof(wchar_t); - -void gowchar_set (wchar_t *arr, int pos, wchar_t val) -{ - arr[pos] = val; -} - -wchar_t gowchar_get (wchar_t *arr, int pos) -{ - return arr[pos]; -} -*/ -import "C" - -import ( - "fmt" - "unicode/utf16" - "unicode/utf8" -) - -var SIZEOF_WCHAR_T C.size_t = C.size_t(C.SIZEOF_WCHAR_T) - -func getFilename(s string) *C.wchar_t { - wstr, _ := StringToWcharT(s) - return wstr -} - -func StringToWcharT(s string) (*C.wchar_t, C.size_t) { - switch SIZEOF_WCHAR_T { - case 2: - return stringToWchar2(s) // Windows - case 4: - return stringToWchar4(s) // Unix - default: - panic(fmt.Sprintf("Invalid sizeof(wchar_t) = %v", SIZEOF_WCHAR_T)) - } - panic("?!!") -} - -// Windows -func stringToWchar2(s string) (*C.wchar_t, C.size_t) { - var slen int - s1 := s - for len(s1) > 0 { - r, size := utf8.DecodeRuneInString(s1) - if er, _ := utf16.EncodeRune(r); er == '\uFFFD' { - slen += 1 - } else { - slen += 2 - } - s1 = s1[size:] - } - slen++ // \0 - res := C.malloc(C.size_t(slen) * SIZEOF_WCHAR_T) - var i int - for len(s) > 0 { - r, size := utf8.DecodeRuneInString(s) - if r1, r2 := utf16.EncodeRune(r); r1 != '\uFFFD' { - C.gowchar_set((*C.wchar_t)(res), C.int(i), C.wchar_t(r1)) - i++ - C.gowchar_set((*C.wchar_t)(res), C.int(i), C.wchar_t(r2)) - i++ - } else { - C.gowchar_set((*C.wchar_t)(res), C.int(i), C.wchar_t(r)) - i++ - } - s = s[size:] - } - C.gowchar_set((*C.wchar_t)(res), C.int(slen-1), C.wchar_t(0)) // \0 - return (*C.wchar_t)(res), C.size_t(slen) -} - -// Unix -func stringToWchar4(s string) (*C.wchar_t, C.size_t) { - slen := utf8.RuneCountInString(s) - slen++ // \0 - res := C.malloc(C.size_t(slen) * SIZEOF_WCHAR_T) - var i int - for len(s) > 0 { - r, size := utf8.DecodeRuneInString(s) - C.gowchar_set((*C.wchar_t)(res), C.int(i), C.wchar_t(r)) - s = s[size:] - i++ - } - C.gowchar_set((*C.wchar_t)(res), C.int(slen-1), C.wchar_t(0)) // \0 - return (*C.wchar_t)(res), C.size_t(slen) -} diff --git a/adapters/taglib/taglib.go b/adapters/taglib/taglib.go deleted file mode 100644 index ac299ea2b..000000000 --- a/adapters/taglib/taglib.go +++ /dev/null @@ -1,178 +0,0 @@ -package taglib - -import ( - "io/fs" - "path/filepath" - "strconv" - "strings" - "time" - - "github.com/navidrome/navidrome/conf" - "github.com/navidrome/navidrome/core/storage/local" - "github.com/navidrome/navidrome/log" - "github.com/navidrome/navidrome/model/metadata" -) - -type extractor struct { - baseDir string -} - -func (e extractor) Parse(files ...string) (map[string]metadata.Info, error) { - results := make(map[string]metadata.Info) - for _, path := range files { - props, err := e.extractMetadata(path) - if err != nil { - continue - } - results[path] = *props - } - return results, nil -} - -func (e extractor) Version() string { - return Version() -} - -func (e extractor) extractMetadata(filePath string) (*metadata.Info, error) { - fullPath := filepath.Join(e.baseDir, filePath) - tags, err := Read(fullPath) - if err != nil { - log.Warn("extractor: Error reading metadata from file. Skipping", "filePath", fullPath, err) - return nil, err - } - - // Parse audio properties - ap := metadata.AudioProperties{} - ap.BitRate = parseProp(tags, "__bitrate") - ap.Channels = parseProp(tags, "__channels") - ap.SampleRate = parseProp(tags, "__samplerate") - ap.BitDepth = parseProp(tags, "__bitspersample") - length := parseProp(tags, "__lengthinmilliseconds") - ap.Duration = (time.Millisecond * time.Duration(length)).Round(time.Millisecond * 10) - - // Extract basic tags - parseBasicTag(tags, "__title", "title") - parseBasicTag(tags, "__artist", "artist") - parseBasicTag(tags, "__album", "album") - parseBasicTag(tags, "__comment", "comment") - parseBasicTag(tags, "__genre", "genre") - parseBasicTag(tags, "__year", "year") - parseBasicTag(tags, "__track", "tracknumber") - - // Parse track/disc totals - parseTuple := func(prop string) { - tagName := prop + "number" - tagTotal := prop + "total" - if value, ok := tags[tagName]; ok && len(value) > 0 { - parts := strings.Split(value[0], "/") - tags[tagName] = []string{parts[0]} - if len(parts) == 2 { - tags[tagTotal] = []string{parts[1]} - } - } - } - parseTuple("track") - parseTuple("disc") - - // Adjust some ID3 tags - parseLyrics(tags) - parseTIPL(tags) - delete(tags, "tmcl") // TMCL is already parsed by TagLib - - return &metadata.Info{ - Tags: tags, - AudioProperties: ap, - HasPicture: tags["has_picture"] != nil && len(tags["has_picture"]) > 0 && tags["has_picture"][0] == "true", - }, nil -} - -// parseLyrics make sure lyrics tags have language -func parseLyrics(tags map[string][]string) { - lyrics := tags["lyrics"] - if len(lyrics) > 0 { - tags["lyrics:xxx"] = lyrics - delete(tags, "lyrics") - } -} - -// These are the only roles we support, based on Picard's tag map: -// https://picard-docs.musicbrainz.org/downloads/MusicBrainz_Picard_Tag_Map.html -var tiplMapping = map[string]string{ - "arranger": "arranger", - "engineer": "engineer", - "producer": "producer", - "mix": "mixer", - "DJ-mix": "djmixer", -} - -// parseProp parses a property from the tags map and sets it to the target integer. -// It also deletes the property from the tags map after parsing. -func parseProp(tags map[string][]string, prop string) int { - if value, ok := tags[prop]; ok && len(value) > 0 { - v, _ := strconv.Atoi(value[0]) - delete(tags, prop) - return v - } - return 0 -} - -// parseBasicTag checks if a basic tag (like __title, __artist, etc.) exists in the tags map. -// If it does, it moves the value to a more appropriate tag name (like title, artist, etc.), -// and deletes the basic tag from the map. If the target tag already exists, it ignores the basic tag. -func parseBasicTag(tags map[string][]string, basicName string, tagName string) { - basicValue := tags[basicName] - if len(basicValue) == 0 { - return - } - delete(tags, basicName) - if len(tags[tagName]) == 0 { - tags[tagName] = basicValue - } -} - -// parseTIPL parses the ID3v2.4 TIPL frame string, which is received from TagLib in the format: -// -// "arranger Andrew Powell engineer Chris Blair engineer Pat Stapley producer Eric Woolfson". -// -// and breaks it down into a map of roles and names, e.g.: -// -// {"arranger": ["Andrew Powell"], "engineer": ["Chris Blair", "Pat Stapley"], "producer": ["Eric Woolfson"]}. -func parseTIPL(tags map[string][]string) { - tipl := tags["tipl"] - if len(tipl) == 0 { - return - } - - addRole := func(currentRole string, currentValue []string) { - if currentRole != "" && len(currentValue) > 0 { - role := tiplMapping[currentRole] - tags[role] = append(tags[role], strings.Join(currentValue, " ")) - } - } - - var currentRole string - var currentValue []string - for _, part := range strings.Split(tipl[0], " ") { - if _, ok := tiplMapping[part]; ok { - addRole(currentRole, currentValue) - currentRole = part - currentValue = nil - continue - } - currentValue = append(currentValue, part) - } - addRole(currentRole, currentValue) - delete(tags, "tipl") -} - -var _ local.Extractor = (*extractor)(nil) - -func init() { - local.RegisterExtractor("legacy-taglib", func(_ fs.FS, baseDir string) local.Extractor { - // ignores fs, as taglib extractor only works with local files - return &extractor{baseDir} - }) - conf.AddHook(func() { - log.Debug("TagLib version", "version", Version()) - }) -} diff --git a/adapters/taglib/taglib_suite_test.go b/adapters/taglib/taglib_suite_test.go deleted file mode 100644 index 2b26612cf..000000000 --- a/adapters/taglib/taglib_suite_test.go +++ /dev/null @@ -1,17 +0,0 @@ -package taglib - -import ( - "testing" - - "github.com/navidrome/navidrome/log" - "github.com/navidrome/navidrome/tests" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -func TestTagLib(t *testing.T) { - tests.Init(t, true) - log.SetLevel(log.LevelFatal) - RegisterFailHandler(Fail) - RunSpecs(t, "TagLib Suite") -} diff --git a/adapters/taglib/taglib_test.go b/adapters/taglib/taglib_test.go deleted file mode 100644 index f524f77ec..000000000 --- a/adapters/taglib/taglib_test.go +++ /dev/null @@ -1,295 +0,0 @@ -package taglib - -import ( - "io/fs" - "os" - "strings" - - "github.com/navidrome/navidrome/utils" - . "github.com/onsi/ginkgo/v2" - . "github.com/onsi/gomega" -) - -var _ = Describe("Extractor", func() { - var e *extractor - - BeforeEach(func() { - e = &extractor{} - }) - - Describe("Parse", func() { - It("correctly parses metadata from all files in folder", func() { - mds, err := e.Parse( - "tests/fixtures/test.mp3", - "tests/fixtures/test.ogg", - ) - Expect(err).NotTo(HaveOccurred()) - Expect(mds).To(HaveLen(2)) - - // Test MP3 - m := mds["tests/fixtures/test.mp3"] - Expect(m.Tags).To(HaveKeyWithValue("title", []string{"Song"})) - Expect(m.Tags).To(HaveKeyWithValue("album", []string{"Album"})) - Expect(m.Tags).To(HaveKeyWithValue("artist", []string{"Artist"})) - Expect(m.Tags).To(HaveKeyWithValue("albumartist", []string{"Album Artist"})) - - Expect(m.HasPicture).To(BeTrue()) - Expect(m.AudioProperties.Duration.String()).To(Equal("1.02s")) - Expect(m.AudioProperties.BitRate).To(Equal(192)) - Expect(m.AudioProperties.Channels).To(Equal(2)) - Expect(m.AudioProperties.SampleRate).To(Equal(44100)) - - Expect(m.Tags).To(Or( - HaveKeyWithValue("compilation", []string{"1"}), - HaveKeyWithValue("tcmp", []string{"1"})), - ) - Expect(m.Tags).To(HaveKeyWithValue("genre", []string{"Rock"})) - Expect(m.Tags).To(HaveKeyWithValue("date", []string{"2014-05-21"})) - Expect(m.Tags).To(HaveKeyWithValue("originaldate", []string{"1996-11-21"})) - Expect(m.Tags).To(HaveKeyWithValue("releasedate", []string{"2020-12-31"})) - Expect(m.Tags).To(HaveKeyWithValue("discnumber", []string{"1"})) - Expect(m.Tags).To(HaveKeyWithValue("disctotal", []string{"2"})) - Expect(m.Tags).To(HaveKeyWithValue("comment", []string{"Comment1\nComment2"})) - Expect(m.Tags).To(HaveKeyWithValue("bpm", []string{"123"})) - Expect(m.Tags).To(HaveKeyWithValue("replaygain_album_gain", []string{"+3.21518 dB"})) - Expect(m.Tags).To(HaveKeyWithValue("replaygain_album_peak", []string{"0.9125"})) - Expect(m.Tags).To(HaveKeyWithValue("replaygain_track_gain", []string{"-1.48 dB"})) - Expect(m.Tags).To(HaveKeyWithValue("replaygain_track_peak", []string{"0.4512"})) - - Expect(m.Tags).To(HaveKeyWithValue("tracknumber", []string{"2"})) - Expect(m.Tags).To(HaveKeyWithValue("tracktotal", []string{"10"})) - - Expect(m.Tags).ToNot(HaveKey("lyrics")) - Expect(m.Tags).To(Or(HaveKeyWithValue("lyrics:eng", []string{ - "[00:00.00]This is\n[00:02.50]English SYLT\n", - "[00:00.00]This is\n[00:02.50]English", - }), HaveKeyWithValue("lyrics:eng", []string{ - "[00:00.00]This is\n[00:02.50]English", - "[00:00.00]This is\n[00:02.50]English SYLT\n", - }))) - Expect(m.Tags).To(Or(HaveKeyWithValue("lyrics:xxx", []string{ - "[00:00.00]This is\n[00:02.50]unspecified SYLT\n", - "[00:00.00]This is\n[00:02.50]unspecified", - }), HaveKeyWithValue("lyrics:xxx", []string{ - "[00:00.00]This is\n[00:02.50]unspecified", - "[00:00.00]This is\n[00:02.50]unspecified SYLT\n", - }))) - - // Test OGG - m = mds["tests/fixtures/test.ogg"] - Expect(err).To(BeNil()) - Expect(m.Tags).To(HaveKeyWithValue("fbpm", []string{"141.7"})) - - // TagLib 1.12 returns 18, previous versions return 39. - // See https://github.com/taglib/taglib/commit/2f238921824741b2cfe6fbfbfc9701d9827ab06b - Expect(m.AudioProperties.BitRate).To(BeElementOf(18, 19, 39, 40, 43, 49)) - Expect(m.AudioProperties.Channels).To(BeElementOf(2)) - Expect(m.AudioProperties.SampleRate).To(BeElementOf(8000)) - Expect(m.HasPicture).To(BeTrue()) - }) - - DescribeTable("Format-Specific tests", - func(file, duration string, channels, samplerate, bitdepth int, albumGain, albumPeak, trackGain, trackPeak string, id3Lyrics bool, image bool) { - file = "tests/fixtures/" + file - mds, err := e.Parse(file) - Expect(err).NotTo(HaveOccurred()) - Expect(mds).To(HaveLen(1)) - - m := mds[file] - - Expect(m.HasPicture).To(Equal(image)) - Expect(m.AudioProperties.Duration.String()).To(Equal(duration)) - Expect(m.AudioProperties.Channels).To(Equal(channels)) - Expect(m.AudioProperties.SampleRate).To(Equal(samplerate)) - Expect(m.AudioProperties.BitDepth).To(Equal(bitdepth)) - - Expect(m.Tags).To(Or( - HaveKeyWithValue("replaygain_album_gain", []string{albumGain}), - HaveKeyWithValue("----:com.apple.itunes:replaygain_album_gain", []string{albumGain}), - )) - - Expect(m.Tags).To(Or( - HaveKeyWithValue("replaygain_album_peak", []string{albumPeak}), - HaveKeyWithValue("----:com.apple.itunes:replaygain_album_peak", []string{albumPeak}), - )) - Expect(m.Tags).To(Or( - HaveKeyWithValue("replaygain_track_gain", []string{trackGain}), - HaveKeyWithValue("----:com.apple.itunes:replaygain_track_gain", []string{trackGain}), - )) - Expect(m.Tags).To(Or( - HaveKeyWithValue("replaygain_track_peak", []string{trackPeak}), - HaveKeyWithValue("----:com.apple.itunes:replaygain_track_peak", []string{trackPeak}), - )) - - Expect(m.Tags).To(HaveKeyWithValue("title", []string{"Title"})) - Expect(m.Tags).To(HaveKeyWithValue("album", []string{"Album"})) - Expect(m.Tags).To(HaveKeyWithValue("artist", []string{"Artist"})) - Expect(m.Tags).To(HaveKeyWithValue("albumartist", []string{"Album Artist"})) - Expect(m.Tags).To(HaveKeyWithValue("genre", []string{"Rock"})) - Expect(m.Tags).To(HaveKeyWithValue("date", []string{"2014"})) - - Expect(m.Tags).To(HaveKeyWithValue("bpm", []string{"123"})) - Expect(m.Tags).To(Or( - HaveKeyWithValue("tracknumber", []string{"3"}), - HaveKeyWithValue("tracknumber", []string{"3/10"}), - )) - if !strings.HasSuffix(file, "test.wma") { - // TODO Not sure why this is not working for WMA - Expect(m.Tags).To(HaveKeyWithValue("tracktotal", []string{"10"})) - } - Expect(m.Tags).To(Or( - HaveKeyWithValue("discnumber", []string{"1"}), - HaveKeyWithValue("discnumber", []string{"1/2"}), - )) - Expect(m.Tags).To(HaveKeyWithValue("disctotal", []string{"2"})) - - // WMA does not have a "compilation" tag, but "wm/iscompilation" - Expect(m.Tags).To(Or( - HaveKeyWithValue("compilation", []string{"1"}), - HaveKeyWithValue("wm/iscompilation", []string{"1"})), - ) - - if id3Lyrics { - Expect(m.Tags).To(HaveKeyWithValue("lyrics:eng", []string{ - "[00:00.00]This is\n[00:02.50]English", - })) - Expect(m.Tags).To(HaveKeyWithValue("lyrics:xxx", []string{ - "[00:00.00]This is\n[00:02.50]unspecified", - })) - } else { - Expect(m.Tags).To(HaveKeyWithValue("lyrics:xxx", []string{ - "[00:00.00]This is\n[00:02.50]unspecified", - "[00:00.00]This is\n[00:02.50]English", - })) - } - - Expect(m.Tags).To(HaveKeyWithValue("comment", []string{"Comment1\nComment2"})) - }, - - // ffmpeg -f lavfi -i "sine=frequency=1200:duration=1" test.flac - Entry("correctly parses flac tags", "test.flac", "1s", 1, 44100, 16, "+4.06 dB", "0.12496948", "+4.06 dB", "0.12496948", false, true), - - Entry("correctly parses m4a (aac) gain tags", "01 Invisible (RED) Edit Version.m4a", "1.04s", 2, 44100, 16, "0.37", "0.48", "0.37", "0.48", false, true), - Entry("correctly parses m4a (aac) gain tags (uppercase)", "test.m4a", "1.04s", 2, 44100, 16, "0.37", "0.48", "0.37", "0.48", false, true), - Entry("correctly parses ogg (vorbis) tags", "test.ogg", "1.04s", 2, 8000, 0, "+7.64 dB", "0.11772506", "+7.64 dB", "0.11772506", false, true), - - // ffmpeg -f lavfi -i "sine=frequency=900:duration=1" test.wma - // Weird note: for the tag parsing to work, the lyrics are actually stored in the reverse order - Entry("correctly parses wma/asf tags", "test.wma", "1.02s", 1, 44100, 16, "3.27 dB", "0.132914", "3.27 dB", "0.132914", false, true), - - // ffmpeg -f lavfi -i "sine=frequency=800:duration=1" test.wv - Entry("correctly parses wv (wavpak) tags", "test.wv", "1s", 1, 44100, 16, "3.43 dB", "0.125061", "3.43 dB", "0.125061", false, true), - - // ffmpeg -f lavfi -i "sine=frequency=1000:duration=1" test.wav - Entry("correctly parses wav tags", "test.wav", "1s", 1, 44100, 16, "3.06 dB", "0.125056", "3.06 dB", "0.125056", true, true), - - // ffmpeg -f lavfi -i "sine=frequency=1400:duration=1" test.aiff - Entry("correctly parses aiff tags", "test.aiff", "1s", 1, 44100, 16, "2.00 dB", "0.124972", "2.00 dB", "0.124972", true, true), - ) - - // Skip these tests when running as root - Context("Access Forbidden", func() { - var accessForbiddenFile string - var RegularUserContext = XContext - var isRegularUser = os.Getuid() != 0 - if isRegularUser { - RegularUserContext = Context - } - - // Only run permission tests if we are not root - RegularUserContext("when run without root privileges", func() { - BeforeEach(func() { - accessForbiddenFile = utils.TempFileName("access_forbidden-", ".mp3") - - f, err := os.OpenFile(accessForbiddenFile, os.O_WRONLY|os.O_CREATE, 0222) - Expect(err).ToNot(HaveOccurred()) - - DeferCleanup(func() { - Expect(f.Close()).To(Succeed()) - Expect(os.Remove(accessForbiddenFile)).To(Succeed()) - }) - }) - - It("correctly handle unreadable file due to insufficient read permission", func() { - _, err := e.extractMetadata(accessForbiddenFile) - Expect(err).To(MatchError(os.ErrPermission)) - }) - - It("skips the file if it cannot be read", func() { - files := []string{ - "tests/fixtures/test.mp3", - "tests/fixtures/test.ogg", - accessForbiddenFile, - } - mds, err := e.Parse(files...) - Expect(err).NotTo(HaveOccurred()) - Expect(mds).To(HaveLen(2)) - Expect(mds).ToNot(HaveKey(accessForbiddenFile)) - }) - }) - }) - - }) - - Describe("Error Checking", func() { - It("returns a generic ErrPath if file does not exist", func() { - testFilePath := "tests/fixtures/NON_EXISTENT.ogg" - _, err := e.extractMetadata(testFilePath) - Expect(err).To(MatchError(fs.ErrNotExist)) - }) - It("does not throw a SIGSEGV error when reading a file with an invalid frame", func() { - // File has an empty TDAT frame - md, err := e.extractMetadata("tests/fixtures/invalid-files/test-invalid-frame.mp3") - Expect(err).ToNot(HaveOccurred()) - Expect(md.Tags).To(HaveKeyWithValue("albumartist", []string{"Elvis Presley"})) - }) - }) - - Describe("parseTIPL", func() { - var tags map[string][]string - - BeforeEach(func() { - tags = make(map[string][]string) - }) - - Context("when the TIPL string is populated", func() { - It("correctly parses roles and names", func() { - tags["tipl"] = []string{"arranger Andrew Powell DJ-mix François Kevorkian DJ-mix Jane Doe engineer Chris Blair"} - parseTIPL(tags) - Expect(tags["arranger"]).To(ConsistOf("Andrew Powell")) - Expect(tags["engineer"]).To(ConsistOf("Chris Blair")) - Expect(tags["djmixer"]).To(ConsistOf("François Kevorkian", "Jane Doe")) - }) - - It("handles multiple names for a single role", func() { - tags["tipl"] = []string{"engineer Pat Stapley producer Eric Woolfson engineer Chris Blair"} - parseTIPL(tags) - Expect(tags["producer"]).To(ConsistOf("Eric Woolfson")) - Expect(tags["engineer"]).To(ConsistOf("Pat Stapley", "Chris Blair")) - }) - - It("discards roles without names", func() { - tags["tipl"] = []string{"engineer Pat Stapley producer engineer Chris Blair"} - parseTIPL(tags) - Expect(tags).ToNot(HaveKey("producer")) - Expect(tags["engineer"]).To(ConsistOf("Pat Stapley", "Chris Blair")) - }) - }) - - Context("when the TIPL string is empty", func() { - It("does nothing", func() { - tags["tipl"] = []string{""} - parseTIPL(tags) - Expect(tags).To(BeEmpty()) - }) - }) - - Context("when the TIPL is not present", func() { - It("does nothing", func() { - parseTIPL(tags) - Expect(tags).To(BeEmpty()) - }) - }) - }) - -}) diff --git a/adapters/taglib/taglib_wrapper.cpp b/adapters/taglib/taglib_wrapper.cpp deleted file mode 100644 index 2985e8f18..000000000 --- a/adapters/taglib/taglib_wrapper.cpp +++ /dev/null @@ -1,299 +0,0 @@ -#include -#include - -#define TAGLIB_STATIC -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include "taglib_wrapper.h" - -char has_cover(const TagLib::FileRef f); - -static char TAGLIB_VERSION[16]; - -char* taglib_version() { - snprintf((char *)TAGLIB_VERSION, 16, "%d.%d.%d", TAGLIB_MAJOR_VERSION, TAGLIB_MINOR_VERSION, TAGLIB_PATCH_VERSION); - return (char *)TAGLIB_VERSION; -} - -int taglib_read(const FILENAME_CHAR_T *filename, unsigned long id) { - TagLib::FileRef f(filename, true, TagLib::AudioProperties::Fast); - - if (f.isNull()) { - return TAGLIB_ERR_PARSE; - } - - if (!f.audioProperties()) { - return TAGLIB_ERR_AUDIO_PROPS; - } - - // Add audio properties to the tags - const TagLib::AudioProperties *props(f.audioProperties()); - goPutInt(id, (char *)"__lengthinmilliseconds", props->lengthInMilliseconds()); - goPutInt(id, (char *)"__bitrate", props->bitrate()); - goPutInt(id, (char *)"__channels", props->channels()); - goPutInt(id, (char *)"__samplerate", props->sampleRate()); - - // Extract bits per sample for supported formats - int bitsPerSample = 0; - if (const auto* apeProperties{ dynamic_cast(props) }) - bitsPerSample = apeProperties->bitsPerSample(); - else if (const auto* asfProperties{ dynamic_cast(props) }) - bitsPerSample = asfProperties->bitsPerSample(); - else if (const auto* flacProperties{ dynamic_cast(props) }) - bitsPerSample = flacProperties->bitsPerSample(); - else if (const auto* mp4Properties{ dynamic_cast(props) }) - bitsPerSample = mp4Properties->bitsPerSample(); - else if (const auto* wavePackProperties{ dynamic_cast(props) }) - bitsPerSample = wavePackProperties->bitsPerSample(); - else if (const auto* aiffProperties{ dynamic_cast(props) }) - bitsPerSample = aiffProperties->bitsPerSample(); - else if (const auto* wavProperties{ dynamic_cast(props) }) - bitsPerSample = wavProperties->bitsPerSample(); - else if (const auto* dsfProperties{ dynamic_cast(props) }) - bitsPerSample = dsfProperties->bitsPerSample(); - - if (bitsPerSample > 0) { - goPutInt(id, (char *)"__bitspersample", bitsPerSample); - } - - // Send all properties to the Go map - TagLib::PropertyMap tags = f.file()->properties(); - - // Make sure at least the basic properties are extracted - TagLib::Tag *basic = f.file()->tag(); - if (!basic->isEmpty()) { - if (!basic->title().isEmpty()) { - tags.insert("__title", basic->title()); - } - if (!basic->artist().isEmpty()) { - tags.insert("__artist", basic->artist()); - } - if (!basic->album().isEmpty()) { - tags.insert("__album", basic->album()); - } - if (!basic->comment().isEmpty()) { - tags.insert("__comment", basic->comment()); - } - if (!basic->genre().isEmpty()) { - tags.insert("__genre", basic->genre()); - } - if (basic->year() > 0) { - tags.insert("__year", TagLib::String::number(basic->year())); - } - if (basic->track() > 0) { - tags.insert("__track", TagLib::String::number(basic->track())); - } - } - - TagLib::ID3v2::Tag *id3Tags = NULL; - - // Get some extended/non-standard ID3-only tags (ex: iTunes extended frames) - TagLib::MPEG::File *mp3File(dynamic_cast(f.file())); - if (mp3File != NULL) { - id3Tags = mp3File->ID3v2Tag(); - } - - if (id3Tags == NULL) { - TagLib::RIFF::WAV::File *wavFile(dynamic_cast(f.file())); - if (wavFile != NULL && wavFile->hasID3v2Tag()) { - id3Tags = wavFile->ID3v2Tag(); - } - } - - if (id3Tags == NULL) { - TagLib::RIFF::AIFF::File *aiffFile(dynamic_cast(f.file())); - if (aiffFile && aiffFile->hasID3v2Tag()) { - id3Tags = aiffFile->tag(); - } - } - - // Yes, it is possible to have ID3v2 tags in FLAC. However, that can cause problems - // with many players, so they will not be parsed - - if (id3Tags != NULL) { - const auto &frames = id3Tags->frameListMap(); - - for (const auto &kv: frames) { - if (kv.first == "USLT") { - for (const auto &tag: kv.second) { - TagLib::ID3v2::UnsynchronizedLyricsFrame *frame = dynamic_cast(tag); - if (frame == NULL) continue; - - tags.erase("LYRICS"); - - const auto bv = frame->language(); - char language[4] = {'x', 'x', 'x', '\0'}; - if (bv.size() == 3) { - strncpy(language, bv.data(), 3); - } - - char *val = const_cast(frame->text().toCString(true)); - - goPutLyrics(id, language, val); - } - } else if (kv.first == "SYLT") { - for (const auto &tag: kv.second) { - TagLib::ID3v2::SynchronizedLyricsFrame *frame = dynamic_cast(tag); - if (frame == NULL) continue; - - const auto bv = frame->language(); - char language[4] = {'x', 'x', 'x', '\0'}; - if (bv.size() == 3) { - strncpy(language, bv.data(), 3); - } - - const auto format = frame->timestampFormat(); - if (format == TagLib::ID3v2::SynchronizedLyricsFrame::AbsoluteMilliseconds) { - - for (const auto &line: frame->synchedText()) { - char *text = const_cast(line.text.toCString(true)); - goPutLyricLine(id, language, text, line.time); - } - } else if (format == TagLib::ID3v2::SynchronizedLyricsFrame::AbsoluteMpegFrames) { - const int sampleRate = props->sampleRate(); - - if (sampleRate != 0) { - for (const auto &line: frame->synchedText()) { - const int timeInMs = (line.time * 1000) / sampleRate; - char *text = const_cast(line.text.toCString(true)); - goPutLyricLine(id, language, text, timeInMs); - } - } - } - } - } else if (kv.first == "TIPL"){ - if (!kv.second.isEmpty()) { - tags.insert(kv.first, kv.second.front()->toString()); - } - } - } - } - - // M4A may have some iTunes specific tags not captured by the PropertyMap interface - TagLib::MP4::File *m4afile(dynamic_cast(f.file())); - if (m4afile != NULL) { - const auto itemListMap = m4afile->tag()->itemMap(); - for (const auto item: itemListMap) { - char *key = const_cast(item.first.toCString(true)); - for (const auto value: item.second.toStringList()) { - char *val = const_cast(value.toCString(true)); - goPutM4AStr(id, key, val); - } - } - } - - // WMA/ASF files may have additional tags not captured by the PropertyMap interface - TagLib::ASF::File *asfFile(dynamic_cast(f.file())); - if (asfFile != NULL) { - const TagLib::ASF::Tag *asfTags{asfFile->tag()}; - const auto itemListMap = asfTags->attributeListMap(); - for (const auto item : itemListMap) { - char *key = const_cast(item.first.toCString(true)); - - for (auto j = item.second.begin(); - j != item.second.end(); ++j) { - - char *val = const_cast(j->toString().toCString(true)); - goPutStr(id, key, val); - } - } - } - - // Send all collected tags to the Go map - for (TagLib::PropertyMap::ConstIterator i = tags.begin(); i != tags.end(); - ++i) { - char *key = const_cast(i->first.toCString(true)); - for (TagLib::StringList::ConstIterator j = i->second.begin(); - j != i->second.end(); ++j) { - char *val = const_cast((*j).toCString(true)); - goPutStr(id, key, val); - } - } - - // Cover art has to be handled separately - if (has_cover(f)) { - goPutStr(id, (char *)"has_picture", (char *)"true"); - } - - return 0; -} - -// Detect if the file has cover art. Returns 1 if the file has cover art, 0 otherwise. -char has_cover(const TagLib::FileRef f) { - char hasCover = 0; - // ----- MP3 - if (TagLib::MPEG::File * mp3File{dynamic_cast(f.file())}) { - if (mp3File->ID3v2Tag()) { - const auto &frameListMap{mp3File->ID3v2Tag()->frameListMap()}; - hasCover = !frameListMap["APIC"].isEmpty(); - } - } - // ----- FLAC - else if (TagLib::FLAC::File * flacFile{dynamic_cast(f.file())}) { - hasCover = !flacFile->pictureList().isEmpty(); - } - // ----- MP4 - else if (TagLib::MP4::File * mp4File{dynamic_cast(f.file())}) { - auto &coverItem{mp4File->tag()->itemMap()["covr"]}; - TagLib::MP4::CoverArtList coverArtList{coverItem.toCoverArtList()}; - hasCover = !coverArtList.isEmpty(); - } - // ----- Ogg - else if (TagLib::Ogg::Vorbis::File * vorbisFile{dynamic_cast(f.file())}) { - hasCover = !vorbisFile->tag()->pictureList().isEmpty(); - } - // ----- Opus - else if (TagLib::Ogg::Opus::File * opusFile{dynamic_cast(f.file())}) { - hasCover = !opusFile->tag()->pictureList().isEmpty(); - } - // ----- WAV - else if (TagLib::RIFF::WAV::File * wavFile{ dynamic_cast(f.file()) }) { - if (wavFile->hasID3v2Tag()) { - const auto& frameListMap{ wavFile->ID3v2Tag()->frameListMap() }; - hasCover = !frameListMap["APIC"].isEmpty(); - } - } - // ----- AIFF - else if (TagLib::RIFF::AIFF::File * aiffFile{ dynamic_cast(f.file())}) { - if (aiffFile->hasID3v2Tag()) { - const auto& frameListMap{ aiffFile->tag()->frameListMap() }; - hasCover = !frameListMap["APIC"].isEmpty(); - } - } - // ----- WMA - else if (TagLib::ASF::File * asfFile{dynamic_cast(f.file())}) { - const TagLib::ASF::Tag *tag{ asfFile->tag() }; - hasCover = tag && tag->attributeListMap().contains("WM/Picture"); - } - // ----- DSF - else if (TagLib::DSF::File * dsffile{ dynamic_cast(f.file())}) { - const TagLib::ID3v2::Tag *tag { dsffile->tag() }; - hasCover = tag && !tag->frameListMap()["APIC"].isEmpty(); - } - // ----- WAVPAK (APE tag) - else if (TagLib::WavPack::File * wvFile{dynamic_cast(f.file())}) { - if (wvFile->hasAPETag()) { - // This is the particular string that Picard uses - hasCover = !wvFile->APETag()->itemListMap()["COVER ART (FRONT)"].isEmpty(); - } - } - - return hasCover; -} diff --git a/adapters/taglib/taglib_wrapper.go b/adapters/taglib/taglib_wrapper.go deleted file mode 100644 index 4a979920a..000000000 --- a/adapters/taglib/taglib_wrapper.go +++ /dev/null @@ -1,157 +0,0 @@ -package taglib - -/* -#cgo !windows pkg-config: --define-prefix taglib -#cgo windows pkg-config: taglib -#cgo illumos LDFLAGS: -lstdc++ -lsendfile -#cgo linux darwin CXXFLAGS: -std=c++11 -#cgo darwin LDFLAGS: -L/opt/homebrew/opt/taglib/lib -#include -#include -#include -#include "taglib_wrapper.h" -*/ -import "C" -import ( - "encoding/json" - "fmt" - "os" - "runtime/debug" - "strconv" - "strings" - "sync" - "sync/atomic" - "unsafe" - - "github.com/navidrome/navidrome/log" -) - -const iTunesKeyPrefix = "----:com.apple.itunes:" - -func Version() string { - return C.GoString(C.taglib_version()) -} - -func Read(filename string) (tags map[string][]string, err error) { - // Do not crash on failures in the C code/library - debug.SetPanicOnFault(true) - defer func() { - if r := recover(); r != nil { - log.Error("extractor: recovered from panic when reading tags", "file", filename, "error", r) - err = fmt.Errorf("extractor: recovered from panic: %s", r) - } - }() - - fp := getFilename(filename) - defer C.free(unsafe.Pointer(fp)) - id, m, release := newMap() - defer release() - - log.Trace("extractor: reading tags", "filename", filename, "map_id", id) - res := C.taglib_read(fp, C.ulong(id)) - switch res { - case C.TAGLIB_ERR_PARSE: - // Check additional case whether the file is unreadable due to permission - file, fileErr := os.OpenFile(filename, os.O_RDONLY, 0600) - defer file.Close() - - if os.IsPermission(fileErr) { - return nil, fmt.Errorf("navidrome does not have permission: %w", fileErr) - } else if fileErr != nil { - return nil, fmt.Errorf("cannot parse file media file: %w", fileErr) - } else { - return nil, fmt.Errorf("cannot parse file media file") - } - case C.TAGLIB_ERR_AUDIO_PROPS: - return nil, fmt.Errorf("can't get audio properties from file") - } - if log.IsGreaterOrEqualTo(log.LevelDebug) { - j, _ := json.Marshal(m) - log.Trace("extractor: read tags", "tags", string(j), "filename", filename, "id", id) - } else { - log.Trace("extractor: read tags", "tags", m, "filename", filename, "id", id) - } - - return m, nil -} - -type tagMap map[string][]string - -var allMaps sync.Map -var mapsNextID atomic.Uint32 - -func newMap() (uint32, tagMap, func()) { - id := mapsNextID.Add(1) - - m := tagMap{} - allMaps.Store(id, m) - - return id, m, func() { - allMaps.Delete(id) - } -} - -func doPutTag(id C.ulong, key string, val *C.char) { - if key == "" { - return - } - - r, _ := allMaps.Load(uint32(id)) - m := r.(tagMap) - k := strings.ToLower(key) - v := strings.TrimSpace(C.GoString(val)) - m[k] = append(m[k], v) -} - -//export goPutM4AStr -func goPutM4AStr(id C.ulong, key *C.char, val *C.char) { - k := C.GoString(key) - - // Special for M4A, do not catch keys that have no actual name - k = strings.TrimPrefix(k, iTunesKeyPrefix) - doPutTag(id, k, val) -} - -//export goPutStr -func goPutStr(id C.ulong, key *C.char, val *C.char) { - doPutTag(id, C.GoString(key), val) -} - -//export goPutInt -func goPutInt(id C.ulong, key *C.char, val C.int) { - valStr := strconv.Itoa(int(val)) - vp := C.CString(valStr) - defer C.free(unsafe.Pointer(vp)) - goPutStr(id, key, vp) -} - -//export goPutLyrics -func goPutLyrics(id C.ulong, lang *C.char, val *C.char) { - doPutTag(id, "lyrics:"+C.GoString(lang), val) -} - -//export goPutLyricLine -func goPutLyricLine(id C.ulong, lang *C.char, text *C.char, time C.int) { - language := C.GoString(lang) - line := C.GoString(text) - timeGo := int64(time) - - ms := timeGo % 1000 - timeGo /= 1000 - sec := timeGo % 60 - timeGo /= 60 - minimum := timeGo % 60 - formattedLine := fmt.Sprintf("[%02d:%02d.%02d]%s\n", minimum, sec, ms/10, line) - - key := "lyrics:" + language - - r, _ := allMaps.Load(uint32(id)) - m := r.(tagMap) - k := strings.ToLower(key) - existing, ok := m[k] - if ok { - existing[0] += formattedLine - } else { - m[k] = []string{formattedLine} - } -} diff --git a/adapters/taglib/taglib_wrapper.h b/adapters/taglib/taglib_wrapper.h deleted file mode 100644 index c93f4c14a..000000000 --- a/adapters/taglib/taglib_wrapper.h +++ /dev/null @@ -1,24 +0,0 @@ -#define TAGLIB_ERR_PARSE -1 -#define TAGLIB_ERR_AUDIO_PROPS -2 - -#ifdef __cplusplus -extern "C" { -#endif - -#ifdef WIN32 -#define FILENAME_CHAR_T wchar_t -#else -#define FILENAME_CHAR_T char -#endif - -extern void goPutM4AStr(unsigned long id, char *key, char *val); -extern void goPutStr(unsigned long id, char *key, char *val); -extern void goPutInt(unsigned long id, char *key, int val); -extern void goPutLyrics(unsigned long id, char *lang, char *val); -extern void goPutLyricLine(unsigned long id, char *lang, char *text, int time); -int taglib_read(const FILENAME_CHAR_T *filename, unsigned long id); -char* taglib_version(); - -#ifdef __cplusplus -} -#endif diff --git a/cmd/root.go b/cmd/root.go index 5fdb591ff..08773176a 100644 --- a/cmd/root.go +++ b/cmd/root.go @@ -27,7 +27,6 @@ import ( _ "github.com/navidrome/navidrome/adapters/gotaglib" _ "github.com/navidrome/navidrome/adapters/lastfm" _ "github.com/navidrome/navidrome/adapters/listenbrainz" - _ "github.com/navidrome/navidrome/adapters/taglib" ) var ( diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index b25b4c100..f66df2e75 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -40,7 +40,6 @@ import ( _ "github.com/navidrome/navidrome/adapters/gotaglib" _ "github.com/navidrome/navidrome/adapters/lastfm" _ "github.com/navidrome/navidrome/adapters/listenbrainz" - _ "github.com/navidrome/navidrome/adapters/taglib" ) // Injectors from wire_injectors.go: diff --git a/core/storage/local/local.go b/core/storage/local/local.go index cd60c9ef1..5384581e0 100644 --- a/core/storage/local/local.go +++ b/core/storage/local/local.go @@ -11,6 +11,7 @@ import ( "github.com/djherbis/times" "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/core/storage" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model/metadata" @@ -28,7 +29,13 @@ type localStorage struct { func newLocalStorage(u url.URL) storage.Storage { newExtractor, ok := extractors[conf.Server.Scanner.Extractor] if !ok || newExtractor == nil { - log.Fatal("Extractor not found", "path", conf.Server.Scanner.Extractor) + if conf.Server.Scanner.Extractor != consts.DefaultScannerExtractor { + log.Warn("Extractor not found, using default", "extractor", conf.Server.Scanner.Extractor, "default", consts.DefaultScannerExtractor) + } + newExtractor = extractors[consts.DefaultScannerExtractor] + if newExtractor == nil { + log.Fatal("Default extractor not registered", "extractor", consts.DefaultScannerExtractor) + } } isWindowsPath := filepath.VolumeName(u.Host) != "" if u.Scheme == storage.LocalSchemaID && isWindowsPath { diff --git a/core/storage/local/local_test.go b/core/storage/local/local_test.go index 3ed01bbc4..b977ef4a5 100644 --- a/core/storage/local/local_test.go +++ b/core/storage/local/local_test.go @@ -10,6 +10,7 @@ import ( "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/core/storage" "github.com/navidrome/navidrome/model/metadata" . "github.com/onsi/ginkgo/v2" @@ -135,16 +136,31 @@ var _ = Describe("LocalStorage", func() { }) }) - Context("with invalid extractor", func() { - It("should handle extractor validation correctly", func() { - // Note: The actual implementation uses log.Fatal which exits the process, - // so we test the normal path where extractors exist + Context("when the configured extractor is not registered", func() { + var defaultExtractor *mockTestExtractor + + BeforeEach(func() { + defaultExtractor = &mockTestExtractor{results: make(map[string]metadata.Info)} + RegisterExtractor(consts.DefaultScannerExtractor, func(fs.FS, string) Extractor { + return defaultExtractor + }) + DeferCleanup(func() { + lock.Lock() + delete(extractors, consts.DefaultScannerExtractor) + lock.Unlock() + }) + }) + + It("falls back to the default extractor instead of crashing", func() { + conf.Server.Scanner.Extractor = "nonexistent-extractor" u, err := url.Parse("file://" + tempDir) Expect(err).ToNot(HaveOccurred()) storage := newLocalStorage(*u) - Expect(storage).ToNot(BeNil()) + ls, ok := storage.(*localStorage) + Expect(ok).To(BeTrue()) + Expect(ls.extractor).To(BeIdenticalTo(defaultExtractor)) }) }) }) From e53e60d39da2a6cd014a0253e64d3f03da872369 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Mon, 13 Apr 2026 13:30:05 -0400 Subject: [PATCH 14/18] feat(artwork): enable native libwebp encoding in Docker image (#5350) * feat(docker): add musl build stage for native libwebp support Add a new build-alpine stage using Alpine/musl with xx cross-compilation, producing a dynamically-linked musl binary for the Docker image. The runtime image now installs libwebp, libwebpdemux, and libwebpmux and creates .so symlinks so gen2brain/webp can detect native libwebp via purego/dlopen at startup and use it automatically. The existing Debian/glibc 'build' stage is kept for standalone binary distribution (darwin, windows, and glibc linux binaries); the Docker image now ships the musl build from build-alpine instead. * fix(docker): use dynamic symlinks for libwebp libraries Avoid hardcoding SONAME versions (.so.7, .so.2, .so.3) which break on Alpine version bumps. Also fix misleading comment: the musl build is dynamic (required for purego dlopen), not static. * feat(docker): enable WebP encoding in Docker environment Signed-off-by: Deluan * fix(docker): pin build-alpine stage to Go 1.25 to match base stage Align the new build-alpine stage with the existing glibc 'base' stage, both pinned to Go 1.25. Bumping build-alpine independently would create a version skew between the Docker image binary and the standalone binaries, which should be avoided unless there is a specific reason. * fix(docker): harden build-alpine stage (musl pin, -latomic, dynamic-link check) Address review feedback on the build-alpine stage: - Pin Go builder to golang:1.25-alpine3.20 so the musl version used at build time matches the alpine:3.20 runtime image, eliminating any potential musl ABI skew between builder and runtime. - Add -extldflags '-latomic' so SQLite's 64-bit atomics resolve when cross-compiling for 32-bit arm targets (arm/v6, arm/v7). - Add a build-time check that the produced binary is dynamically linked (using 'file' from Alpine), failing the build if it is not. A fully-static binary cannot dlopen libwebp and would silently fall back to the WASM encoder, defeating the whole point of this stage. * fix(docker): revert to unpinned golang:1.25-alpine builder The golang:1.25-alpine3.20 tag suggested during review does not exist on public.ecr.aws (only 3.21, 3.22, 3.23, and unpinned 'alpine' are published). Revert to the unpinned 'golang:1.25-alpine' tag so the Docker build can resolve the base image. This means the builder's Alpine version can drift relative to the alpine:3.20 runtime, but in practice musl's backward compatibility covers this for Navidrome's small dlopen surface (a few libwebp symbols, no direct libc calls from the dlopen path). If a skew ever manifests, we can pin both builder and runtime to the same specific Alpine release in a follow-up. --------- Signed-off-by: Deluan --- Dockerfile | 55 +++++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 50 insertions(+), 5 deletions(-) diff --git a/Dockerfile b/Dockerfile index f6ea14ff3..66243f84c 100644 --- a/Dockerfile +++ b/Dockerfile @@ -42,7 +42,46 @@ FROM scratch AS ui-bundle COPY --from=ui /build /build ######################################################################################################################## -### Build Navidrome binary +### Build Navidrome binary for Docker image (dynamic musl, enables native libwebp via dlopen) +FROM --platform=$BUILDPLATFORM public.ecr.aws/docker/library/golang:1.25-alpine AS build-alpine +COPY --from=xx / / + +ARG TARGETPLATFORM + +RUN apk add --no-cache clang lld file git +RUN xx-apk add --no-cache gcc musl-dev zlib-dev +RUN xx-verify --setup + +WORKDIR /workspace + +RUN --mount=type=bind,source=. \ + --mount=type=cache,target=/root/.cache \ + --mount=type=cache,target=/go/pkg/mod \ + go mod download + +ARG GIT_SHA +ARG GIT_TAG + +RUN --mount=type=bind,source=. \ + --mount=from=ui,source=/build,target=./ui/build,ro \ + --mount=type=cache,target=/root/.cache \ + --mount=type=cache,target=/go/pkg/mod </dev/null | head -1) && \ + [ -n "$target" ] && ln -sf "$target" /usr/lib/$lib.so; \ + done -# Copy navidrome binary -COPY --from=build /out/navidrome /app/ +# Copy navidrome binary (musl build for Docker, enables native libwebp) +COPY --from=build-alpine /out/navidrome /app/ VOLUME ["/data", "/music"] ENV ND_MUSICFOLDER=/music ENV ND_DATAFOLDER=/data ENV ND_CONFIGFILE=/data/navidrome.toml ENV ND_PORT=4533 +ENV ND_ENABLEWEBPENCODING=true RUN touch /.nddockerenv EXPOSE ${ND_PORT} From 02c9fc3359fe07e96b5e9054008e2def156e660b Mon Sep 17 00:00:00 2001 From: Deluan Date: Mon, 13 Apr 2026 20:32:42 -0400 Subject: [PATCH 15/18] chore(deps): update go-sqlite3 and other dependencies to latest versions Signed-off-by: Deluan --- go.mod | 24 ++++++++++++------------ go.sum | 48 ++++++++++++++++++++++++------------------------ 2 files changed, 36 insertions(+), 36 deletions(-) diff --git a/go.mod b/go.mod index 4f4ad0461..ebac8064f 100644 --- a/go.mod +++ b/go.mod @@ -36,7 +36,7 @@ require ( github.com/kardianos/service v1.2.4 github.com/kr/pretty v0.3.1 github.com/lestrrat-go/jwx/v3 v3.0.13 - github.com/mattn/go-sqlite3 v1.14.38 + github.com/mattn/go-sqlite3 v1.14.42 github.com/microcosm-cc/bluemonday v1.0.27 github.com/mileusna/useragent v1.3.5 github.com/onsi/ginkgo/v2 v2.28.1 @@ -58,12 +58,12 @@ require ( github.com/xrash/smetrics v0.0.0-20250705151800-55b8f293f342 go.senan.xyz/taglib v0.11.1 go.uber.org/goleak v1.3.0 - golang.org/x/image v0.38.0 - golang.org/x/net v0.52.0 + golang.org/x/image v0.39.0 + golang.org/x/net v0.53.0 golang.org/x/sync v0.20.0 - golang.org/x/sys v0.42.0 - golang.org/x/term v0.41.0 - golang.org/x/text v0.35.0 + golang.org/x/sys v0.43.0 + golang.org/x/term v0.42.0 + golang.org/x/text v0.36.0 golang.org/x/time v0.15.0 gopkg.in/yaml.v3 v3.0.1 ) @@ -89,7 +89,7 @@ require ( github.com/goccy/go-json v0.10.6 // indirect github.com/goccy/go-yaml v1.19.2 // indirect github.com/google/go-cmp v0.7.0 // indirect - github.com/google/pprof v0.0.0-20260302011040-a15ffb7f9dcc // indirect + github.com/google/pprof v0.0.0-20260402051712-545e8a4df936 // indirect github.com/google/subcommands v1.2.0 // indirect github.com/gorilla/css v1.0.1 // indirect github.com/hashicorp/errwrap v1.1.0 // indirect @@ -101,7 +101,7 @@ require ( github.com/lann/builder v0.0.0-20180802200727-47ae307949d0 // indirect github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 // indirect github.com/lestrrat-go/blackmagic v1.0.4 // indirect - github.com/lestrrat-go/dsig v1.0.0 // indirect + github.com/lestrrat-go/dsig v1.3.0 // indirect github.com/lestrrat-go/dsig-secp256k1 v1.0.0 // indirect github.com/lestrrat-go/httpcc v1.0.1 // indirect github.com/lestrrat-go/httprc/v3 v3.0.5 // indirect @@ -134,10 +134,10 @@ require ( go.uber.org/multierr v1.11.0 // indirect go.yaml.in/yaml/v2 v2.4.3 // indirect go.yaml.in/yaml/v3 v3.0.4 // indirect - golang.org/x/crypto v0.49.0 // indirect - golang.org/x/mod v0.34.0 // indirect - golang.org/x/telemetry v0.0.0-20260311193753-579e4da9a98c // indirect - golang.org/x/tools v0.43.0 // indirect + golang.org/x/crypto v0.50.0 // indirect + golang.org/x/mod v0.35.0 // indirect + golang.org/x/telemetry v0.0.0-20260409153401-be6f6cb8b1fa // indirect + golang.org/x/tools v0.44.0 // indirect google.golang.org/protobuf v1.36.11 // indirect gopkg.in/ini.v1 v1.67.1 // indirect gopkg.in/natefinch/npipe.v2 v2.0.0-20160621034901-c1b8fa8bdcce // indirect diff --git a/go.sum b/go.sum index 5a0761f15..29b979413 100644 --- a/go.sum +++ b/go.sum @@ -108,8 +108,8 @@ github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/go-pipeline v0.0.0-20230411140531-6cbedfc1d3fc h1:hd+uUVsB1vdxohPneMrhGH2YfQuH5hRIK9u4/XCeUtw= github.com/google/go-pipeline v0.0.0-20230411140531-6cbedfc1d3fc/go.mod h1:SL66SJVysrh7YbDCP9tH30b8a9o/N2HeiQNUm85EKhc= -github.com/google/pprof v0.0.0-20260302011040-a15ffb7f9dcc h1:VBbFa1lDYWEeV5FZKUiYKYT0VxCp9twUmmaq9eb8sXw= -github.com/google/pprof v0.0.0-20260302011040-a15ffb7f9dcc/go.mod h1:MxpfABSjhmINe3F1It9d+8exIHFvUqtLIRCdOGNXqiI= +github.com/google/pprof v0.0.0-20260402051712-545e8a4df936 h1:EwtI+Al+DeppwYX2oXJCETMO23COyaKGP6fHVpkpWpg= +github.com/google/pprof v0.0.0-20260402051712-545e8a4df936/go.mod h1:MxpfABSjhmINe3F1It9d+8exIHFvUqtLIRCdOGNXqiI= github.com/google/subcommands v1.2.0 h1:vWQspBTo2nEqTUFita5/KeEWlUL8kQObDFbub/EN9oE= github.com/google/subcommands v1.2.0/go.mod h1:ZjhPrFU+Olkh9WazFPsl27BQ4UPiG37m3yTrtFlrHVk= github.com/google/uuid v1.6.0 h1:NIvaJDMOsjHA8n1jAhLSgzrAzy1Hgr+hNrb57e+94F0= @@ -161,8 +161,8 @@ github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0 h1:P6pPBnrTSX3DEVR4fDembhR github.com/lann/ps v0.0.0-20150810152359-62de8c46ede0/go.mod h1:vmVJ0l/dxyfGW6FmdpVm2joNMFikkuWg0EoCKLGUMNw= github.com/lestrrat-go/blackmagic v1.0.4 h1:IwQibdnf8l2KoO+qC3uT4OaTWsW7tuRQXy9TRN9QanA= github.com/lestrrat-go/blackmagic v1.0.4/go.mod h1:6AWFyKNNj0zEXQYfTMPfZrAXUWUfTIZ5ECEUEJaijtw= -github.com/lestrrat-go/dsig v1.0.0 h1:OE09s2r9Z81kxzJYRn07TFM9XA4akrUdoMwr0L8xj38= -github.com/lestrrat-go/dsig v1.0.0/go.mod h1:dEgoOYYEJvW6XGbLasr8TFcAxoWrKlbQvmJgCR0qkDo= +github.com/lestrrat-go/dsig v1.3.0 h1:phjMOCXvYzhuIgn7Voe2rex8z166vGfxRxmqM25P9/Q= +github.com/lestrrat-go/dsig v1.3.0/go.mod h1:RD2eOaidyPvpc7IJQoO3Qq52RWdy8ZcJs8lrOnoa1Kc= github.com/lestrrat-go/dsig-secp256k1 v1.0.0 h1:JpDe4Aybfl0soBvoVwjqDbp+9S1Y2OM7gcrVVMFPOzY= github.com/lestrrat-go/dsig-secp256k1 v1.0.0/go.mod h1:CxUgAhssb8FToqbL8NjSPoGQlnO4w3LG1P0qPWQm/NU= github.com/lestrrat-go/httpcc v1.0.1 h1:ydWCStUeJLkpYyjLDHihupbn2tYmZ7m22BGkcvZZrIE= @@ -177,8 +177,8 @@ github.com/maruel/natural v1.3.0 h1:VsmCsBmEyrR46RomtgHs5hbKADGRVtliHTyCOLFBpsg= github.com/maruel/natural v1.3.0/go.mod h1:v+Rfd79xlw1AgVBjbO0BEQmptqb5HvL/k9GRHB7ZKEg= github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= -github.com/mattn/go-sqlite3 v1.14.38 h1:tDUzL85kMvOrvpCt8P64SbGgVFtJB11GPi2AdmITgb4= -github.com/mattn/go-sqlite3 v1.14.38/go.mod h1:Uh1q+B4BYcTPb+yiD3kU8Ct7aC0hY9fxUwlHK0RXw+Y= +github.com/mattn/go-sqlite3 v1.14.42 h1:MigqEP4ZmHw3aIdIT7T+9TLa90Z6smwcthx+Azv4Cgo= +github.com/mattn/go-sqlite3 v1.14.42/go.mod h1:pjEuOr8IwzLJP2MfGeTb0A35jauH+C2kbHKBr7yXKVQ= github.com/mfridman/interpolate v0.0.2 h1:pnuTK7MQIxxFz1Gr+rjSIx9u7qVjf5VOoM/u6BbAxPY= github.com/mfridman/interpolate v0.0.2/go.mod h1:p+7uk6oE07mpE/Ik1b8EckO0O4ZXiGAfshKBWLUM9Xg= github.com/mfridman/tparse v0.18.0 h1:wh6dzOKaIwkUGyKgOntDW4liXSo37qg5AXbIhkMV3vE= @@ -319,19 +319,19 @@ golang.org/x/crypto v0.13.0/go.mod h1:y6Z2r+Rw4iayiXXAIxJIDAJ1zMW4yaTpebo8fPOliY golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= golang.org/x/crypto v0.23.0/go.mod h1:CKFgDieR+mRhux2Lsu27y0fO304Db0wZe70UKqHu0v8= golang.org/x/crypto v0.31.0/go.mod h1:kDsLvtWBEx7MV9tJOj9bnXsPbxwJQ6csT/x4KIN4Ssk= -golang.org/x/crypto v0.49.0 h1:+Ng2ULVvLHnJ/ZFEq4KdcDd/cfjrrjjNSXNzxg0Y4U4= -golang.org/x/crypto v0.49.0/go.mod h1:ErX4dUh2UM+CFYiXZRTcMpEcN8b/1gxEuv3nODoYtCA= +golang.org/x/crypto v0.50.0 h1:zO47/JPrL6vsNkINmLoo/PH1gcxpls50DNogFvB5ZGI= +golang.org/x/crypto v0.50.0/go.mod h1:3muZ7vA7PBCE6xgPX7nkzzjiUq87kRItoJQM1Yo8S+Q= golang.org/x/exp v0.0.0-20260218203240-3dfff04db8fa h1:Zt3DZoOFFYkKhDT3v7Lm9FDMEV06GpzjG2jrqW+QTE0= golang.org/x/exp v0.0.0-20260218203240-3dfff04db8fa/go.mod h1:K79w1Vqn7PoiZn+TkNpx3BUWUQksGO3JcVX6qIjytmA= -golang.org/x/image v0.38.0 h1:5l+q+Y9JDC7mBOMjo4/aPhMDcxEptsX+Tt3GgRQRPuE= -golang.org/x/image v0.38.0/go.mod h1:/3f6vaXC+6CEanU4KJxbcUZyEePbyKbaLoDOe4ehFYY= +golang.org/x/image v0.39.0 h1:skVYidAEVKgn8lZ602XO75asgXBgLj9G/FE3RbuPFww= +golang.org/x/image v0.39.0/go.mod h1:sIbmppfU+xFLPIG0FoVUTvyBMmgng1/XAMhQ2ft0hpA= golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4= golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.12.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs= golang.org/x/mod v0.15.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/mod v0.17.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= -golang.org/x/mod v0.34.0 h1:xIHgNUUnW6sYkcM5Jleh05DvLOtwc6RitGHbDk4akRI= -golang.org/x/mod v0.34.0/go.mod h1:ykgH52iCZe79kzLLMhyCUzhMci+nQj+0XkbXpNYtVjY= +golang.org/x/mod v0.35.0 h1:Ww1D637e6Pg+Zb2KrWfHQUnH2dQRLBQyAtpr/haaJeM= +golang.org/x/mod v0.35.0/go.mod h1:+GwiRhIInF8wPm+4AoT6L0FA1QWAad3OMdTRx4tFYlU= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= golang.org/x/net v0.0.0-20190603091049-60506f45cf65/go.mod h1:HSz+uSET+XFnRR8LxR5pz3Of3rY3CfYBVs4xY44aLks= golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= @@ -343,8 +343,8 @@ golang.org/x/net v0.15.0/go.mod h1:idbUs1IY1+zTqbi8yxTbhexhEEk5ur9LInksu6HrEpk= golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM= golang.org/x/net v0.33.0/go.mod h1:HXLR5J+9DxmrqMwG9qjGCxZ+zKXxBru04zlTvWlWuN4= -golang.org/x/net v0.52.0 h1:He/TN1l0e4mmR3QqHMT2Xab3Aj3L9qjbhRm78/6jrW0= -golang.org/x/net v0.52.0/go.mod h1:R1MAz7uMZxVMualyPXb+VaqGSa3LIaUqk0eEt3w36Sw= +golang.org/x/net v0.53.0 h1:d+qAbo5L0orcWAr0a9JweQpjXF19LMXJE8Ey7hwOdUA= +golang.org/x/net v0.53.0/go.mod h1:JvMuJH7rrdiCfbeHoo3fCQU24Lf5JJwT9W3sJFulfgs= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.1.0/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= @@ -369,11 +369,11 @@ golang.org/x/sys v0.12.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/sys v0.28.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/sys v0.42.0 h1:omrd2nAlyT5ESRdCLYdm3+fMfNFE/+Rf4bDIQImRJeo= -golang.org/x/sys v0.42.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/sys v0.43.0 h1:Rlag2XtaFTxp19wS8MXlJwTvoh8ArU6ezoyFsMyCTNI= +golang.org/x/sys v0.43.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= golang.org/x/telemetry v0.0.0-20240228155512-f48c80bd79b2/go.mod h1:TeRTkGYfJXctD9OcfyVLyj2J3IxLnKwHJR8f4D8a3YE= -golang.org/x/telemetry v0.0.0-20260311193753-579e4da9a98c h1:6a8FdnNk6bTXBjR4AGKFgUKuo+7GnR3FX5L7CbveeZc= -golang.org/x/telemetry v0.0.0-20260311193753-579e4da9a98c/go.mod h1:TpUTTEp9frx7rTdLpC9gFG9kdI7zVLFTFFlqaH2Cncw= +golang.org/x/telemetry v0.0.0-20260409153401-be6f6cb8b1fa h1:efT73AJZfAAUV7SOip6pWGkwJDzIGiKBZGVzHYa+ve4= +golang.org/x/telemetry v0.0.0-20260409153401-be6f6cb8b1fa/go.mod h1:kHjTxDEnAu6/Nl9lDkzjWpR+bmKfxeiRuSDlsMb70gE= golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo= golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8= golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k= @@ -382,8 +382,8 @@ golang.org/x/term v0.12.0/go.mod h1:owVbMEjm3cBLCHdkQu9b1opXd4ETQWc3BhuQGKgXgvU= golang.org/x/term v0.17.0/go.mod h1:lLRBjIVuehSbZlaOtGMbcMncT+aqLLLmKrsjNrUguwk= golang.org/x/term v0.20.0/go.mod h1:8UkIAJTvZgivsXaD6/pH6U9ecQzZ45awqEOzuCvwpFY= golang.org/x/term v0.27.0/go.mod h1:iMsnZpn0cago0GOrHO2+Y7u7JPn5AylBrcoWkElMTSM= -golang.org/x/term v0.41.0 h1:QCgPso/Q3RTJx2Th4bDLqML4W6iJiaXFq2/ftQF13YU= -golang.org/x/term v0.41.0/go.mod h1:3pfBgksrReYfZ5lvYM0kSO0LIkAl4Yl2bXOkKP7Ec2A= +golang.org/x/term v0.42.0 h1:UiKe+zDFmJobeJ5ggPwOshJIVt6/Ft0rcfrXZDLWAWY= +golang.org/x/term v0.42.0/go.mod h1:Dq/D+snpsbazcBG5+F9Q1n2rXV8Ma+71xEjTRufARgY= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= golang.org/x/text v0.3.2/go.mod h1:bEr9sfX3Q8Zfm5fL9x+3itogRgK3+ptLWKqgva+5dAk= golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ= @@ -394,8 +394,8 @@ golang.org/x/text v0.13.0/go.mod h1:TvPlkZtksWOMsz7fbANvkp4WM8x/WCo/om8BMLbz+aE= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.15.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ= -golang.org/x/text v0.35.0 h1:JOVx6vVDFokkpaq1AEptVzLTpDe9KGpj5tR4/X+ybL8= -golang.org/x/text v0.35.0/go.mod h1:khi/HExzZJ2pGnjenulevKNX1W67CUy0AsXcNubPGCA= +golang.org/x/text v0.36.0 h1:JfKh3XmcRPqZPKevfXVpI1wXPTqbkE5f7JA92a55Yxg= +golang.org/x/text v0.36.0/go.mod h1:NIdBknypM8iqVmPiuco0Dh6P5Jcdk8lJL0CUebqK164= golang.org/x/time v0.15.0 h1:bbrp8t3bGUeFOx08pvsMYRTCVSMk89u4tKbNOZbp88U= golang.org/x/time v0.15.0/go.mod h1:Y4YMaQmXwGQZoFaVFk4YpCt4FLQMYKZe9oeV/f4MSno= golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= @@ -405,8 +405,8 @@ golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc golang.org/x/tools v0.6.0/go.mod h1:Xwgl3UAJ/d3gWutnCtw505GrjyAbvKui8lOU390QaIU= golang.org/x/tools v0.13.0/go.mod h1:HvlwmtVNQAhOuCjW7xxvovg8wbNq7LwfXh/k7wXUl58= golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d/go.mod h1:aiJjzUbINMkxbQROHiO6hDPo2LHcIPhhQsa9DLh0yGk= -golang.org/x/tools v0.43.0 h1:12BdW9CeB3Z+J/I/wj34VMl8X+fEXBxVR90JeMX5E7s= -golang.org/x/tools v0.43.0/go.mod h1:uHkMso649BX2cZK6+RpuIPXS3ho2hZo4FVwfoy1vIk0= +golang.org/x/tools v0.44.0 h1:UP4ajHPIcuMjT1GqzDWRlalUEoY+uzoZKnhOjbIPD2c= +golang.org/x/tools v0.44.0/go.mod h1:KA0AfVErSdxRZIsOVipbv3rQhVXTnlU6UhKxHd1seDI= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= google.golang.org/appengine v1.6.5/go.mod h1:8WjMMxjGQR8xUklV/ARdw2HLXBOI7O7uCIDZVag1xfc= google.golang.org/protobuf v1.36.11 h1:fV6ZwhNocDyBLK0dj+fg8ektcVegBBuEolpbTQyBNVE= From e86d3266c41a341bd2349693d42c414298fecb3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Tue, 14 Apr 2026 19:19:42 -0400 Subject: [PATCH 17/18] Add context7.json with URL and public key --- context7.json | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 context7.json diff --git a/context7.json b/context7.json new file mode 100644 index 000000000..343873063 --- /dev/null +++ b/context7.json @@ -0,0 +1,4 @@ +{ + "url": "https://context7.com/navidrome/navidrome", + "public_key": "pk_WqzhKScNKWQ84J4n0oG0J" +} From 155e293f4d57bcb38e36ad62dbf204029e720838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Tue, 14 Apr 2026 19:31:01 -0400 Subject: [PATCH 18/18] chore(deps): upgrade Go to 1.26 (#5361) Bump the main module, Dockerfile build stages, and devcontainer to Go 1.26.0. Plugin sub-modules under plugins/ remain on go 1.25 intentionally (independent modules, untouched in this change). Also add an explicit actions/setup-go@v6 step (with go-version-file: go.mod) to the go-lint and go jobs in the CI pipeline. This matches the golangci-lint-action v4+ requirement that setup-go run before the linter, and pins the runner Go version to go.mod so CI does not depend on the ubuntu-latest tools cache picking up Go 1.26. --- .devcontainer/devcontainer.json | 2 +- .github/workflows/pipeline.yml | 8 ++++++++ Dockerfile | 4 ++-- go.mod | 2 +- 4 files changed, 12 insertions(+), 4 deletions(-) diff --git a/.devcontainer/devcontainer.json b/.devcontainer/devcontainer.json index 311090b91..c9e4ba2bf 100644 --- a/.devcontainer/devcontainer.json +++ b/.devcontainer/devcontainer.json @@ -4,7 +4,7 @@ "dockerfile": "Dockerfile", "args": { // Update the VARIANT arg to pick a version of Go: 1, 1.15, 1.14 - "VARIANT": "1.25", + "VARIANT": "1.26", // Options "INSTALL_NODE": "true", "NODE_VERSION": "v24" diff --git a/.github/workflows/pipeline.yml b/.github/workflows/pipeline.yml index e939f1d13..6ebb579e8 100644 --- a/.github/workflows/pipeline.yml +++ b/.github/workflows/pipeline.yml @@ -64,6 +64,10 @@ jobs: steps: - uses: actions/checkout@v6 + - uses: actions/setup-go@v6 + with: + go-version-file: go.mod + - name: golangci-lint uses: golangci/golangci-lint-action@v9 with: @@ -99,6 +103,10 @@ jobs: - name: Check out code into the Go module directory uses: actions/checkout@v6 + - uses: actions/setup-go@v6 + with: + go-version-file: go.mod + - name: Download dependencies run: go mod download diff --git a/Dockerfile b/Dockerfile index 66243f84c..105656afb 100644 --- a/Dockerfile +++ b/Dockerfile @@ -43,7 +43,7 @@ COPY --from=ui /build /build ######################################################################################################################## ### Build Navidrome binary for Docker image (dynamic musl, enables native libwebp via dlopen) -FROM --platform=$BUILDPLATFORM public.ecr.aws/docker/library/golang:1.25-alpine AS build-alpine +FROM --platform=$BUILDPLATFORM public.ecr.aws/docker/library/golang:1.26-alpine AS build-alpine COPY --from=xx / / ARG TARGETPLATFORM @@ -82,7 +82,7 @@ EOT ######################################################################################################################## ### Build Navidrome binary for standalone distribution (static glibc, cross-compiled) -FROM --platform=$BUILDPLATFORM public.ecr.aws/docker/library/golang:1.25-trixie AS base +FROM --platform=$BUILDPLATFORM public.ecr.aws/docker/library/golang:1.26-trixie AS base RUN apt-get update && apt-get install -y clang lld COPY --from=xx / / WORKDIR /workspace diff --git a/go.mod b/go.mod index ebac8064f..b7dbb9eeb 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/navidrome/navidrome -go 1.25.0 +go 1.26.0 // Fork to implement raw tags support replace go.senan.xyz/taglib => github.com/deluan/go-taglib v0.0.0-20260407173416-cf47afbaa67a