mirror of
https://github.com/navidrome/navidrome.git
synced 2026-07-30 16:56:22 -04:00
dependabot/github_actions/dot-github/workflows/actions/setup-node-7
383 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
53d54baef0 |
fix(jellyfin): stream collection responses to prevent OOM on large libraries (#5783)
* fix(jellyfin): stream /Items responses to prevent OOM on large libraries
Finamp's library sync issues an unbounded GET /Items?IncludeItemTypes=Audio
with Fields=MediaSources and no Limit. On a large library this built the whole
result set — every MediaFile and every BaseItemDto, each fat with MediaSources —
in memory, and json.Encoder then buffered the entire ~200MB response before
writing a byte. Measured on a 96k-track library, one such request peaked near
1.6GB RSS; in a memory-limited container the resulting slowness made clients
retry, stacking concurrent full-library builds until the process was OOM-killed.
Stream the song listing straight from a DB cursor (MediaFileRepository.GetCursor),
mapping and encoding one row at a time, so peak memory is bounded to about one
item regardless of library size. The cursor is opened lazily, when streaming
begins, so it doesn't hold a DB connection across the CountAll and ServerId
lookups that run first (ServerId can write on first use and would deadlock
against an open reader). Pagination still works — the cursor query carries
LIMIT/OFFSET/ORDER BY from StartIndex/Limit/SortBy — and TotalRecordCount stays
the full count. Other materialized responses stream their JSON too, avoiding the
encoder's buffer-the-whole-output cost.
Also bound artwork image concurrency with the same server.ThrottleBacklog that
Subsonic's getCoverArt uses, so a burst of image requests during sync can't
exhaust memory through concurrent decode/resize.
Verified against a copy of a 96k-track production database: byte-for-byte
identical response, peak RSS 1593MB -> 53MB, time-to-first-byte 8.1s -> 0.2s.
* fix(jellyfin): fail loudly on /Items streaming errors instead of a truncated 200
Addresses code review: a streamed /Items response commits HTTP 200 before an
error can occur, so failures were surfacing as misleadingly successful bodies.
- A cursor-open failure (e.g. a busy DB under connection pressure) is now
surfaced before the first byte is written: the cursor open is deferred into
the item source and run in writeItems after the ServerId lookup but before the
envelope, returning a clean 500 the client can retry — not a 200 with an empty
item list.
- A mid-stream (row-scan) error now aborts without closing the JSON envelope,
leaving the body malformed. A truncated-but-valid response would let a sync
client (Finamp) treat the short list as the whole library and prune local
tracks; malformed JSON forces the client's parser to fail and retry.
* refactor(jellyfin): stream every collection endpoint from a DB cursor
Streaming was applied only to the song listing, which left two write paths, two
ServerId stamping mechanisms and a listXxx return-type split ("songs is
special") that made the code hard to follow.
Add GetCursor to the album, artist, genre and playlist repositories, mirroring
MediaFileRepository.GetCursor: same select builder as GetAll, so each cursor
yields identical, fully-hydrated rows (hydration happens per-row in PostScan,
and toModels is only a deref loop). The playlist cursor keeps GetAll's
owner/public visibility filter. Each repository test asserts the cursor yields
exactly what GetAll returns, including Max/Offset.
With cursors everywhere, every listXxx returns itemsResult and every collection
streams through one writer:
- listAlbums, listArtists and listPlaylists now stream from their cursors;
search paths stay materialized (Search returns a slice).
- listGenres stays materialized: its total is the length of the full list and it
paginates in memory, so there is nothing for a cursor to page over.
- api.ok's QueryResult case delegates to writeItems, so the ~9 inline callers
funnel into the same writer; streamQueryResult and wrapResult are gone.
- /Items/Latest streams as a bare JSON array (its Jellyfin wire shape) via
writeItemsArray, which shares the item loop and stamping. That also closes the
same unbounded hole /Items had: limit=0 disabled its LIMIT.
- ServerId is now stamped in exactly one place, so api.ok only handles single,
non-collection payloads.
Verified against a copy of a 96k-track production database: every endpoint
byte-for-byte identical to master. Unbounded /Items peak RSS 1691MB -> 54MB;
time-to-first-byte 6.3s -> 0.19s, /Artists 1.14s -> 0.13s.
* refactor(jellyfin): route every response through api.ok
Handlers were split between api.ok and api.writeItems with no clear rule for
which to call. api.ok now accepts itemsResult too, so it is the single entry
point: callers hand it whatever they have and it routes collections (cursor
-backed or materialized) to the streaming writer. writeItems is reached only
through api.ok now. The one exception is /Items/Latest, which returns a bare
JSON array rather than a QueryResult envelope and so writes directly — noted in
both doc comments.
Also drop GenreRepository.GetCursor: listGenres derives its total from the
length of the full list and paginates in memory, so there is nothing for a
cursor to page over, leaving the method unused.
* fix(jellyfin): stream the unbounded multi-type /Items merge
The multi-type merge capped each per-type query at offset+limit only when a
Limit was given. Without one (Finamp's favorites screen sends multi-type), every
type ran an unbounded query and collect() drained each cursor into a slice — so
/Items?IncludeItemTypes=MusicAlbum,Audio with no Limit materialized every album
and every song, the same OOM class this branch set out to fix. The comment
claiming the set was "capped at offset+limit per type" was wrong for limit=0.
Without a limit the merged page is just each type's rows in order minus the
first offset, which is exactly what chaining the per-type cursors yields, so
stream that instead of merging in memory. The bounded path is unchanged: with a
Limit each type holds at most offset+limit rows, so merging and paginating
across the combined list is safe. Cursors are opened one at a time (each pins a
DB connection until drained), with the first opened eagerly so the usual failure
is still a clean error before any byte is written.
Measured on a 96k-track library, /Items?IncludeItemTypes=MusicAlbum,Audio with
no Limit (103,393 items): peak RSS 612MB -> 53MB, time-to-first-byte 4.9s ->
0.6s, response byte-for-byte identical.
* refactor(jellyfin): parse /Items params into a struct
The /Items dispatcher was a single 90-line block that parsed a dozen params and
then threaded them positionally through three layers — queryItemsOfType took 11
arguments, listSongs and listAlbums 9 each — so reading any one of them meant
decoding a long argument list.
Parse once into an itemsQuery and pass that instead. queryItems now reads as the
four things it actually does: parse, the id/playlists-folder/playlist-parent
special cases, single-type dispatch, multi-type merge — with the playlist-parent
and merge bodies moved to playlistTracks and mergeTypes. Every listXxx takes
(ctx, opts, q).
No behavior change: parsing, ordering and the entityParent rule are unchanged,
and /Artists still passes favOnly=false explicitly by building the subset of the
query it uses.
* docs(jellyfin): trim comments on the streaming path
Cut the comments back to the non-obvious why: the deferred cursor open (the
ServerId write would deadlock against an open reader), the deliberate malformed
JSON on a mid-stream error, why chained opens one cursor at a time, why
listGenres stays materialized, and why the cursor helpers take the underlying
func type. Dropped the rest — restatements of the code, doc comments that
repeated a test's own name, and the GetCursor interface comments that the
existing MediaFileRepository.GetCursor does without.
* refactor(persistence): fold the cursor wrappers into a generic wrapCursor
wrapAlbumCursor, wrapArtistCursor, wrapMediaFileCursor and wrapFolderCursor were
the same twelve lines four times over, differing only in the type name, and the
playlist cursor had its own inline copy of the loop.
Add one generic wrapCursor. It takes an extractor func rather than a method on
an interface: a type parameter can't reach an embedded field, and methods that
only satisfy a generic constraint are reported by the unused linter. The
extractor also lets both type parameters be inferred, so call sites need no
explicit instantiation. Each entity keeps its named wrapper as a one-liner,
since the model cursor types are defined types and need the conversion — and the
existing wrapper tests call them directly.
The nil-row error now names the model type via %T ("unexpected nil model.Album")
instead of a hand-written per-entity string; the three tests asserting that
message are updated.
* fix(jellyfin): bound concurrent collection streams
A streamed collection holds a DB cursor — and its pooled connection — for the whole client-paced
response, where the old materialize-then-write path released it as soon as the query finished. The
pool is shared with the scanner, Subsonic, the native API and the UI, so enough slow clients take
every connection and everything else blocks waiting for one. Measured against a copy of a 96k-track
production DB, 20 concurrent unbounded streams against a pool of 16 stalled a write for 16.2s (the
other 14 writes in the run took ~2ms — the signature of connection starvation, not lock contention).
Cap concurrent streams at half the pool. Excess requests queue rather than fail, so no client is
rejected: the same 20 streams now all complete and the worst write is 2.5ms.
- conf.MaxOpenConns() now owns the pool sizing (db calls it). It belongs in conf: the pool is a
tunable, db already imports conf, and putting it in db would force server/jellyfin to import db
just to size the cap against it. Expressing the cap as MaxOpenConns()/2 also keeps the two from
drifting apart.
- Uses chi's ThrottleBacklog, not server.ThrottleBacklog: the latter buffers the whole response to
release its token early, which is right for artwork but would undo the streaming. chi's panics on a
non-positive limit, so throttleStreams guards it — setting MaxConcurrentStreams=0 disables the cap
instead of crashing the server at startup.
* refactor(jellyfin): move throttleStreams to middlewares.go
It's a middleware, so it belongs beside normalizeQueryKeys, authenticate and
withPlayer rather than in api.go. Its tests move to middlewares_test.go with it,
keeping one test file per production file.
* fix(jellyfin): abandon the scan when the client stops reading
encodeItems discarded its write errors and relied on the final Flush to report
them, so once bufio's buffer filled and the flush failed, the loop still pulled
every remaining row through the cursor and serialized it for a client that was
gone. A test with a failing writer confirms it: all 20k items were drained.
That wastes CPU, and holds the cursor's pooled DB connection and a stream slot
(now a capped resource) for the length of a full scan nobody is reading. Check
the per-item writes so the first failure ends the scan; the fixed envelope
writes stay unchecked, since bufio latches for them anyway.
|
||
|
|
fe6ac2e577 |
feat(jellyfin): experimental Jellyfin Music API support (#5730)
* feat(sharing): enable sharing by default
Flip the EnableSharing default from false to true so new installations have the sharing feature available out of the box. Users can still disable it via the EnableSharing config option.
The native API only registers the /share route when sharing is enabled, so the nativeapi tests that build the router without wiring a share service now explicitly disable sharing in their setup to avoid registering a route backed by a nil service.
* feat(jellyfin): add config flag and URL path constant
Adds the disabled-by-default Server.Jellyfin config option (Enabled,
ServerName) and consts.URLPathJellyfinAPI, following the existing
LastFM/ListenBrainz patterns. Later tasks will use these to mount the
Jellyfin-compatible API router.
* fix(jellyfin): default Jellyfin ServerName to "Navidrome"
* feat(jellyfin): package skeleton with System handshake endpoints
Adds server/jellyfin: the Router (mirrors server/subsonic and
server/public), its Wire-friendly New(...) constructor, a chi routes()
table, and the ok() JSON response helper. Implements the unauthenticated
handshake surface Jellyfin clients probe first: GET /System/Info/Public,
GET+POST /System/Ping, and GET /QuickConnect/Enabled (quick connect is
unsupported, so it always reports disabled).
The public info payload's Id must be stable across restarts (Jellyfin
clients cache ServerId), so it reuses the get-or-create Property pattern
already used for InsightsID in core/metrics/insights.go: fetch
consts.JellyfinServerIDKey from the Property repository, generating and
persisting a new UUID on first read. A dedicated key (rather than the
existing InsightsID) keeps the anonymous telemetry identifier from being
exposed on this unauthenticated endpoint.
* test(jellyfin): cover ping and quickConnectEnabled handlers
Both were added alongside the System/Info/Public handshake endpoint but
had no direct test coverage.
* fix(jellyfin): memoize stable server Id and cover get-or-create path
* feat(jellyfin): wire and mount the router behind Jellyfin.Enabled
Add jellyfin.New to the shared Wire provider set and a
CreateJellyfinAPIRouter injector, then mount the router at /jellyfin
in startServer(), gated by conf.Server.Jellyfin.Enabled, mirroring the
existing LastFM/ListenBrainz router mounts.
* feat(jellyfin): add Jellyfin DTOs and model mappers
Adds BaseItemDto, QueryResult, UserItemDataDto, UserDto,
AuthenticationResult, MediaSourceInfo, PlaybackInfoResponse,
NameGuidPair and SessionInfo DTOs to server/jellyfin/dto, plus
model-to-DTO mappers for songs, albums, artists and genres that
later browse/stream/write endpoints will consume.
* test(jellyfin): cover GenreToBaseItem mapper
* feat(jellyfin): advertise Jellyfin-compatible version, brand ServerName with Navidrome version
* fix(jellyfin): map LastPlayedDate and omit zero track/disc numbers
* feat(jellyfin): authentication middleware and AuthenticateByName login
* fix(jellyfin): reject empty login password, wire ServerName, add negative auth tests
* refactor(jellyfin): use new(x) builtin instead of intPtr helper
* feat(jellyfin): user views and current-user endpoints
* feat(jellyfin): Items query engine, item detail, and latest
Adds the /Items universal query endpoint, dispatching by IncludeItemTypes
over albums/artists/songs/genres with ParentId, SearchTerm, Filters=IsFavorite,
SortBy/SortOrder and StartIndex/Limit support, plus GET /Items/{itemId} and
/Users/{userId}/Items/Latest. Reuses server/subsonic/filter builders (by
artist/album/starred) instead of hand-rolled squirrel filters, and resolves
sort keys per item type since each repository maps sort names to different
real/aliased columns.
Also adds the missing CountAll to tests.MockArtistRepo, exposed by this task
(the mock embedded a nil ArtistRepository for it and would panic on use).
* refactor(server): extract shared query filter builders to server/filter
Moves server/subsonic/filter to server/filter so server/jellyfin can use
the shared query-option builders without importing server/subsonic,
enforcing the rule that no API package imports another API's package.
* feat(jellyfin): full multi-library support with per-user access scoping
Replace the single hardcoded "music" UserView with one CollectionFolder
view per library the user can access, and scope every /Items browse
query (albums, songs, artists, /Latest) to those libraries via
server/filter's ApplyLibraryFilter/ApplyArtistLibraryFilter.
ParentId is now disambiguated: a numeric value the user has access to is
treated as a library scope (browsing a UserView), otherwise it falls
through as an entity id (artist/album), which safely matches nothing
rather than leaking another library's content. getItem now 404s when
fetching an album or song outside the user's accessible libraries;
artists are skipped (they can span multiple libraries) with a TODO.
Genres remain unscoped since they're global tags, not per-library
entities.
* test(jellyfin): cover admin library-access path and drop nil test contexts
* feat(jellyfin): Artists and Genres endpoints
Add /Artists, /Artists/AlbumArtists, /Genres and /MusicGenres. Artists
listing is library-scoped (defaulting to the user's accessible
libraries, narrowed by an accessible ParentId), delegating access
control to listArtists/ApplyArtistLibraryFilter. Genres are global and
unscoped, matching the ML decision already made for listGenres.
* refactor(jellyfin): share resolveLibraryScope between items and artists
* feat(jellyfin): item image endpoint via artwork service
* fix(jellyfin): let net/http sniff image Content-Type instead of forcing jpeg
* feat(jellyfin): audio streaming and PlaybackInfo with library access control
* feat(jellyfin): favorites and rating write-back with library access control
Adds POST/DELETE handlers for /Users/{userId}/FavoriteItems/{itemId} and
/Users/{userId}/Items/{itemId}/Rating. A shared resolveAnnotated helper
probes album/artist/media file (mirroring getItem's order) and 404s before
writing if the user lacks access to the album/song's library; artists are
exempt since they span multiple libraries. Ratings are halved coming in
(Jellyfin 0-10 -> Navidrome 0-5) to match the doubling in dto.UserData.
Also teaches MockMediaFileRepo and MockArtistRepo's SetStar/SetRating (and
MockAlbumRepo's, previously a no-op) to actually mutate the backing data so
write-back can be asserted in tests.
* feat(jellyfin): playback reporting and scrobbling
Add /Sessions/Playing[/Progress|/Stopped] and /Sessions/Capabilities[/Full]
handlers, backed by core/scrobbler.PlayTracker. A new withPlayer middleware
resolves/registers a model.Player from the Emby DeviceId header (used
directly as the stable player id, unlike Subsonic's cookie fallback) and
injects it into the request context for scrobbling.
The Stopped report calls ReportPlayback with IgnoreScrobble to end the
now-playing session without double-counting, then calls Submit as the
single source of the play-count increment and external scrobble.
* feat(jellyfin): playlist read and write-back
Adds POST /Playlists, GET/POST/DELETE /Playlists/{id}/Items. Tags each
playlist item with PlaylistItemId (the entry's position within the
playlist) so DELETE .../Items?EntryIds=... can remove a specific
occurrence by the id core/playlists.RemoveTracks actually expects,
rather than the song id used everywhere else.
* test(jellyfin): cover empty Ids/EntryIds in playlist add/remove
* feat(jellyfin): unknown-route logging, generic 500s, rating clamp, docs
Hardening pass ahead of real client testing: unmatched routes and
unsupported methods now return a logged, JSON 404 instead of chi's
default plain-text response, so a missing endpoint a client needs is
easy to spot in the logs. Internal errors (ffmpeg output, file paths,
etc.) no longer leak into 500 response bodies -- a shared
internalError helper logs the real error server-side and always
returns a generic message. Inbound Jellyfin ratings are clamped to
0-10 before being halved into Navidrome's 0-5 scale, and /System/Ping
now replies with a bare plain-text body as real Jellyfin servers do.
Adds a README with an enable/curl walkthrough and known limitations.
* docs(jellyfin): clarify public image endpoint and accessibleLibraryIDs comments
* fix(jellyfin): case-insensitive path routing for Jellyfin client compatibility
* refactor(server): extract case-insensitive path routing to a shared helper
* fix(jellyfin): return User Policy and Configuration so Finamp completes login
* fix(jellyfin): support Playlist type, multi-type Items queries, and PlayCount/DatePlayed sort
Real Finamp requests break against three /Items query engine bugs: Playlist
requests fell through to albums, multi-type IncludeItemTypes (e.g. Finamp's
favorites screen) only returned the first requested type, and comma-separated
SortBy lists (e.g. "DateCreated,SortName") were matched as one opaque string
so they always fell through to the repo default.
Also extends tests/mock_playlist_repo.go with SetData/GetAll so playlist
listing is testable like the other mock repos.
* feat(jellyfin): implement /socket WebSocket for real-time client sessions
* fix(jellyfin): resolve library-view ids in getItem so clients can load the library
* fix(jellyfin): serve direct file at /Items/{id}/File and accept ApiKey query param
* fix(jellyfin): resolve playlist ids in getItem
* fix(jellyfin): read lowercase ids/entryIds playlist params and add playlist-users endpoints
* fix(jellyfin): include MediaSources with Size/Bitrate on tracks so clients show download size
* fix(jellyfin): populate all required MediaSourceInfo bool/array fields to match Jellyfin
* fix(jellyfin): hex-encode item ids at the API boundary for Jellyfin client compatibility
Finamp (and presumably other clients) parses ids as radix-16, but Navidrome's
base62 nanoids aren't valid hex and crash its queue packing. Hex-encode every
id emitted by the Jellyfin API and decode every id received, keeping the
transform reversible and stateless at the boundary rather than touching any
model id.
* fix(jellyfin): populate MediaStreams with the audio stream so clients can size/transcode
* fix(jellyfin): support Ids batch-fetch in /Items so clients can fetch items by id
* fix(jellyfin): do not disable Jellyfin server when disabling external services
* fix(jellyfin): emit per-image ImageBlurHashes so clients de-dupe images and stop warning
* fix(jellyfin): support playlist cover upload/delete and display via item image endpoints
* feat(jellyfin): implement GET /Playlists/{id} for playlist visibility
Finamp's playlist edit screen calls GET /Playlists/{id} to read the
OpenAccess (public visibility) flag; we only had the sub-routes, so it
404'd and the edit screen failed to load. Return the Jellyfin PlaylistDto
shape (OpenAccess from Public, empty Shares, media item ids).
* fix(jellyfin): display playlist cover art
Uploaded playlist covers never showed in clients for two reasons: the
playlist BaseItemDto advertised no Primary ImageTag (so clients didn't
know to fetch a cover), and the public image endpoint resolved artwork
under the request's anonymous context, so a private playlist failed its
visibility filter and fell back to the placeholder. Advertise the Primary
image tag/blurhash on playlists, and resolve artwork under an elevated
context (as core/artwork's cache warmer does).
* fix(jellyfin): expand album/artist/playlist ids when building playlists
Jellyfin clients (Finamp) send container ids — an album, artist or
playlist — in a playlist's Ids list and expect the server to expand each
into its child tracks. core/playlists only understands media file ids, so
creating or adding with an album id silently produced an empty playlist.
Expand container ids to their tracks (in order) before create/add; bare
song ids still pass through. Adds filter.SongsByArtistID.
* fix(jellyfin): support playlist deletion via DELETE /Items/{id}
Finamp deletes a playlist with DELETE /Items/{id}, which we didn't route,
so deletion silently failed. Implement it via core/playlists.Delete (which
enforces ownership and removes the cover file). Only playlists are
deletable through this API; non-playlist ids return 404, non-owners 403.
* test(jellyfin): add e2e suite harness + smoke tests
* test(jellyfin): e2e for system, auth, routing, browsing, annotations
* test(jellyfin): e2e for playlists (CRUD, expansion, cover) and item images
* test(jellyfin): e2e for streaming, sessions, and multi-user access control
* fix(jellyfin): artist search 500 (library filter leaked into FTS query)
getArtists/listArtists reused the browse-path filters (notMissing +
ApplyArtistLibraryFilter) for the search path, but artist Search expects a
sole Eq{library_id} filter it can consume as a scope — artists have no
library_id column, so the compound/join filter leaked into the FTS query
and 500'd. Every Finamp artist search failed (the filter is applied even
unscoped, since admins resolve to all library ids). Build search filters
separately. Adds e2e search coverage.
* feat(jellyfin): implement POST /Playlists/{id} to update name, visibility, tracks
Finamp edits a playlist (make public, rename, reorder) via POST
/Playlists/{id}, which we didn't route, so every edit 404'd. Implement it:
Ids present -> replace track list (Create with existing id, preserving
name); otherwise update Name/IsPublic via core/playlists.Update. Adds a
shared playlistError helper (403/404/500) reused by deleteItem, plus e2e
coverage.
* docs(jellyfin): update README for playlists, images, id encoding, and e2e
* fix(jellyfin): default album track listing to track order
Browsing an album's tracks (Items?ParentId=<albumId>&IncludeItemTypes=Audio)
took only SongsByAlbum's filters and dropped its Sort, so tracks came back
in arbitrary order. Default opts.Sort to the album (disc+track) order when
browsing an album without an explicit SortBy — matching Subsonic's GetAlbum
and real Jellyfin. An explicit SortBy still wins.
* fix(jellyfin): filter items by AlbumArtistIds/ArtistIds
An artist's page in Finamp sends ParentId=<libraryId> (scoping) plus
AlbumArtistIds/ArtistIds/contributingArtistIds for the artist itself, but
queryItems only honored ParentId, so an artist's albums and tracks came
back unfiltered (every artist's content). Parse the artist-id params and
apply AlbumsByArtistID (albums) / SongsByArtistID (tracks). Adds e2e
coverage.
* fix(jellyfin): only count a play past the scrobble threshold
reportPlaybackStopped set IgnoreScrobble and force-submitted a play on
every Stopped report, so a briefly-played track (e.g. an immediate skip)
was marked played. Finamp sends Stopped on every track switch, so the
threshold must be applied server-side. Let ReportPlayback's StateStopped
logic decide (play + scrobble only past 50% of the track / 4-minute cap)
instead. Subsonic differs because there the client gates submission.
* fix(jellyfin): sort album tracks by track number when client sends IndexNumber SortBy
Finamp's album view requests SortBy=ParentIndexNumber,IndexNumber,SortName
(disc, track, name), but applySort didn't recognize ParentIndexNumber or
IndexNumber and fell through to SortName, sorting tracks alphabetically by
title. Map both keys to the album (disc+track) sort. The reversed-title
fixture lets the e2e tell track order from title order.
* fix(jellyfin): honor the isFavorite query param for favorites filtering
Finamp's artist 'Favourite tracks' widget requests favorites via the
standalone isFavorite=true query param, not Filters=IsFavorite, so the
filter was ignored and non-favorited tracks were returned. Detect both
forms. (The widget's reshuffling is Finamp's explicit SortBy=Random.)
* feat(jellyfin): emit DateCreated (Date Added) on items
BaseItemDto had no DateCreated, so clients showed 'No Date Added' and had
nothing to sort 'Recently Added' by. Emit it as ISO 8601 from each entity's
CreatedAt (the same field the recently_added sort uses) for songs, albums
and artists.
* fix(jellyfin): set ArtistItems/AlbumArtists on songs (Now Playing artist)
SongToBaseItem only set Artists (names) and AlbumArtist, not the structured
ArtistItems/AlbumArtists. Finamp's Now Playing screen reads ArtistItems and
shows 'Unknown Artist' when it's absent. Populate both (track artist and
album artist) as name+id pairs, mirroring AlbumToBaseItem.
* docs(jellyfin): note synthetic blurhash as a follow-up
Document that ImageBlurHashes are derived from the item id (a solid-color
placeholder), not computed from the cover art like real Jellyfin, and
outline what a proper implementation would take.
* docs(jellyfin): note WebSocket events and favourited playlists as follow-ups
* fix(jellyfin): filter the Artists page by role (album artist vs performer)
/Artists and /Artists/AlbumArtists both called the same role-agnostic
handler, so Navidrome's per-role artist entries (composers, arrangers,
performers) all showed as Album Artists, and the two tabs were identical.
Filter /Artists/AlbumArtists to RoleAlbumArtist and /Artists to RoleArtist
(and the MusicArtist browse to album artists), via filter.ArtistsByRole.
Verified live: Beatles Singles' album-artists dropped 138->5, composers
excluded, performers (Billy Preston) show only under /Artists.
* fix(jellyfin): sort playlists by name (missing Playlist sort mapping)
applySort had no sortColumnsByType entry for the Playlist type, so
SortBy=SortName was ignored and the Playlists screen showed them in the
repo's default order. Map SortName/Name -> name (as Subsonic does) and
DateCreated -> created_at. Verified live: case-insensitive alphabetical.
* fix(jellyfin): read query params case-insensitively (Jellify support)
Jellify (and the official Jellyfin TypeScript SDK) send query params in
camelCase (parentId, albumArtistIds, artistIds, includeItemTypes), where
Finamp sends PascalCase. Our handlers read fixed-case keys, so every
Jellify filter/sort/paging param was silently dropped:
- an artist's page listed albums and tracks from all artists
- opening an album listed every album instead of its tracks
Real Jellyfin binds query params case-insensitively (ASP.NET model
binding), so add a normalizeQueryKeys middleware that folds every query
key to lowercase once, and read params by their lowercase name. This
also removes the scattered dual-case hacks (Ids/ids, IsFavorite,
ContributingArtistIds, api_key/ApiKey, queryParam) that let this bug
class through.
Also infer the child type from an album parent: Jellify browses an album
with only parentId (no IncludeItemTypes), and Jellyfin infers Audio from
the parent; without it we fell back to listing all albums.
Verified live against Finamp+Jellify and with 4 new e2e specs replaying
Jellify's exact request shapes.
* fix(jellyfin): advertise LocalAddress in the public system info handshake
Jellify (and other @jellyfin/sdk clients) that connect by raw address fall
back to HTTP when TLS isn't available, then adopt the handshake's
LocalAddress as their server base URL. We never populated it, so Jellify's
SDK `api` object was undefined and sign-in crashed with "Cannot read
property 'configuration' of undefined" (getUserApi(api!) with an undefined
api). Over an HTTPS connection Jellify uses connectionType=hostname and
never reads LocalAddress, which is why it worked while an HTTPS proxy was
in front.
Populate LocalAddress from the request — scheme + host (honoring
X-Forwarded-*), plus the /jellyfin mount path — matching real Jellyfin,
which always sends it. Export server.ServerAddress for the resolution.
* fix(jellyfin): separate "Featured On" from an artist's own discography
Jellify's artist page fetches the discography via albumArtistIds and the
"Featured On" section via contributingArtistIds, relying on the server to
return disjoint sets. We collapsed albumArtistIds/artistIds/
contributingArtistIds into one AlbumsByArtistID filter, so an artist's own
albums appeared in both sections.
Add AlbumsByContributingArtistID — albums where the artist is a track
artist but NOT the album artist — matching Jellyfin's ContributingArtistIds
(in Artists, not in AlbumArtists), and route contributingArtistIds to it.
* fix(jellyfin): accept a bare Authorization token for audio streaming
Jellify's native player (react-native-nitro-player) authenticates the audio
stream by setting a bare Authorization header carrying the raw access token
({ AUTHORIZATION: api.accessToken }), not the "MediaBrowser ... Token=" scheme
parseEmbyAuth understands. tokenFromRequest didn't recognize it, so every
/Audio/{id}/stream request from the native player 401'd and no audio played
(the JS client still optimistically posted playback progress, masking it).
Accept a bare (or "Bearer <token>") Authorization header, as real Jellyfin
does. Verified live: the native player's exact request now streams 200.
* fix(jellyfin): embed a self-authenticating stream URL in PlaybackInfo
Jellify's native audio player (react-native-nitro-player / ExoPlayer) fetches
the stream without forwarding any auth — captured request headers were only
Connection/Icy-Metadata/Accept-Encoding/User-Agent, no Authorization and no
api_key — so every /Audio/{id}/stream request 401'd and nothing played (the
JS client still optimistically posted progress, masking it).
Real Jellyfin returns stream URLs with the token embedded; we returned none,
so Jellify fell back to a token-less DirectPlay URL. Populate
MediaSources[0].TranscodingUrl with /Audio/{id}/universal?api_key=<caller
token>, which Jellify's player uses verbatim. Direct-play clients (Finamp
builds its own /Items/{id}/File?ApiKey URL) ignore the field, so they're
unaffected.
* feat(jellyfin): serve per-item UserData (GET /UserItems/{id}/UserData)
Jellify fetches this per item to render played/favourite indicators; we 404'd
it. Resolve the item via the existing getItem resolver (which loads the
caller's annotations and enforces the same library-access gate) and return its
UserData, falling back to an empty-but-valid object for items without
annotations (e.g. playlists). Also registers the legacy
/Users/{userId}/Items/{itemId}/UserData spelling.
* refactor(jellyfin): drop unused bare-Authorization token support
Added mid-troubleshooting on the theory that Jellify's native player sends a
bare Authorization header, but the debug capture showed it sends no auth header
at all — the real fix was embedding api_key in PlaybackInfo's TranscodingUrl.
No client reaches this path (Jellify JS uses the MediaBrowser scheme, Finamp
uses X-Emby-Token, the native player uses the api_key URL), so remove it and
its tests per YAGNI. Effectively reverts
|
||
|
|
cc315dcc8c |
perf(db): keep query planner statistics trustworthy with full ANALYZE (#5740)
* perf(db): keep query planner statistics trustworthy with full ANALYZE PRAGMA optimize's internal ANALYZE runs with a limited analysis budget (~2000 rows) that writes wrong sqlite_stat1 entries for low-cardinality indexes: on a 96K-track library it claimed (missing, library_id) narrows to ~2000 rows when it matches the whole table. The planner then prefers that index over the sort index and falls back to a full-table temp B-tree sort per request, turning paginated song listings into multi-second queries (reproduced at 5.5s on real hardware; ~90x slower than with correct stats). Every index-creating migration re-triggered the poisoning via the post-migration optimize, and the daily optimizer could re-trigger it on large library changes. Setting analysis_limit on the connection does not help: optimize ignores it. Run a plain full ANALYZE instead: after migrations with schema changes, and in db.Optimize (daily schedule and scan-end). Stats are stored in the database file, so one connection suffices and the per-connection pool loop is gone. The Optimize call at shutdown is removed: stats are maintained at migration/scan/daily points, and an ANALYZE during shutdown only delays it and races container stop timeouts. * perf(db): drop startup PRAGMA optimize that re-poisons planner stats The startup PRAGMA optimize=0x10002 runs SQLite's budget-limited internal ANALYZE (bit 0x02), which writes truncated sqlite_stat1 rows for low-cardinality indexes -- the exact statistics-poisoning this PR set out to eliminate. Because DevOptimizeDB defaults to true, a restart with no pending migrations would re-poison the planner until the next scan or daily Optimize. Remove it: statistics are already refreshed with a full ANALYZE after schema-changing migrations (Init) and via Optimize at scan-end and on the daily schedule, so nothing on the startup path needs to touch them. Also clarify that Optimize is a no-op unless DevOptimizeDB is enabled. * chore(db): remove the DevOptimizeDB flag and skip Optimize on quick scans The flag only gated the optimize/ANALYZE maintenance calls and there is no reason to leave planner statistics unmaintained; the guards are gone along with the flag. The scan-end Optimize now runs only after full scans — quick scans barely move the statistics, and the daily schedule covers drift. * style(scanner): drop redundant comment in runOptimize * chore(persistence): drop the no-op PRAGMA optimize from ScanEnd Mask 0x10000 only selects candidate tables by size change; without the 0x02 action bit optimize does nothing (verified: sqlite_stat1 stays stale after a 100x table growth). The scan-end statistics refresh is db.Optimize's full ANALYZE, and the expression-collation-index concern the old comment guarded against no longer applies. * fix(scanner): run the post-scan ANALYZE in the server process With the external scanner (the default), the scan pipeline runs in a subprocess, so its ANALYZE was invisible to the server: SQLite loads sqlite_stat1 into the process's shared schema cache, and an ANALYZE from another process does not refresh it — verified with the production DSN that even brand-new pool connections keep planning with the old statistics until the server restarts. An in-process ANALYZE, by contrast, is immediately visible to every pooled connection through the same shared cache. Move the full-scan Optimize from the scanner pipeline to the scan controller, which always runs in the server process. * fix(scanner): honor promoted full scans in the optimize gate A quick scan resuming an interrupted full scan is promoted inside the scanner (possibly in a subprocess); mirror the promotion in the controller so the post-scan ANALYZE isn't skipped. * refactor: apply cleanup review findings - drop forceFullRescan's inline ANALYZE: Init already runs a full ANALYZE after any migration batch with schema changes, so upgrades including a full-rescan migration analyzed the whole DB twice - resumingFullScan uses a filtered CountAll instead of fetching and scanning all libraries - document why CallScan (CLI) deliberately skips the post-scan Optimize * perf(db): make planner analysis maintenance resilient Check analysis freshness every 30 minutes and refresh statistics when the last successful run is over 24 hours old or a scan marked them pending. Persist successful analysis state, retry skipped or failed maintenance, coordinate checks with scans, and cover standalone CLI full scans. * perf(db): avoid analyzing routine quick-scan changes Reserve pending analysis for full scans, unscanned libraries, and retry state. Incremental quick scans now rely on the 24-hour freshness window instead of triggering a full ANALYZE at the next maintenance check. * fix(scan): analyze resumed full scans in CLI * fix(db): back off failed analysis retries * feat(db): allow disabling scheduled analysis * test(db): remove redundant analysis coverage * refactor(db): split ANALYZE maintenance into optimize.go and dedupe call sites - move query-planner statistics code from db.go to its own optimize.go (and matching optimize_test.go) - log ANALYZE elapsed time inside Optimize/OptimizeIfNeeded instead of repeating the timing block at every call site - drop the LastDBAnalyzeAttemptAt write on success: it is only read while failures >= 1, and every failure rewrites it first - extract runPostScanAnalysis (cmd) and anyIncludedLibrary (scanner) helpers |
||
|
|
37e75c4354 |
feat(sharing): enable sharing by default (#5714)
Flip the EnableSharing default from false to true so new installations have the sharing feature available out of the box. Users can still disable it via the EnableSharing config option. The native API only registers the /share route when sharing is enabled, so the nativeapi tests that build the router without wiring a share service now explicitly disable sharing in their setup to avoid registering a route backed by a nil service. |
||
|
|
4cbba2ae49 |
feat(scanner): add ArtistSplitExceptions to protect artist names from splitting (#5701)
* refactor(scanner): make tag value splitting position-based Replaces the ZWSP substitution trick with index-based cutting, in preparation for artist split exceptions, which need match positions. * feat(scanner): protect whitelisted names in tag value splitting Separator matches inside word-bounded exception matches no longer split. Matching is case-insensitive and longest-first; boundaries are rune-aware. * feat(scanner): add Scanner.ArtistSplitExceptions config option * feat(scanner): honor artist split exceptions for participant tags Applies Scanner.ArtistSplitExceptions to artist, albumartist and role tag splitting. Generic tags (genre, mood, ...) are unaffected. * fix(scanner): apply split exceptions when per-tag Split overrides participant tags Per-tag Tags.<name>.Split makes the generic ingestion path split the tag before participant mapping runs, bypassing the whitelist. Attach the exceptions to participant tag mappings (including sort variants) in clean(). * feat(scanner): split performer names and honor split exceptions Performer pair values were never split; multiple names in one PERFORMER value stayed a single artist. Split them with the roles separators, using the same whitelist protection as other participant tags. * test(scanner): lock MBID ordering for split performer values * refactor(scanner): consolidate split-exception wiring and drop hot-path lock ArtistSplitExceptionsRx is called per tag mapping per scanned file across concurrent goroutines; replace the mutex+joined-key cache with an atomic pointer compared via slices.Equal. Route all participant call sites through WithParticipantExceptions and a shared splitParticipantValues helper. * refactor(scanner): unexport artistSplitExceptionsRx All external callers go through WithParticipantExceptions, so the accessor does not need to be part of the model package API. |
||
|
|
05105e91d9 |
feat(scanner): add Scanner.IgnoreDotFolders to allow indexing dot-prefixed folders (#5568)
* feat(scanner): add Scanner.IgnoreDotFolders to allow scanning dot folders
Adds a new Scanner.IgnoreDotFolders option (default true, preserving current
behavior) that, when disabled, lets the scanner traverse folders whose names
start with a dot, such as albums like ".Hack Sign Original Soundtrack".
Previously every dot-prefixed entry was skipped unconditionally before the
directory check, so such album folders were never indexed. The walk loop now
determines whether an entry is a directory first, then skips dot-prefixed files
always and dot-prefixed folders only when IgnoreDotFolders is enabled. Special
system directories are still ignored in all cases via the ignoredDirs blocklist,
which now also lists .git explicitly (it was previously caught only by the
generic dot-prefix rule). isDirIgnored is reduced to a pure blocklist check and
the name-only predicate is renamed from isEntryIgnored to isDotEntry.
* refactor(scanner): centralize entry ignore policy in isIgnoredEntry
Consolidates the directory-entry ignore decision into a single isIgnoredEntry
helper so the walk loop reads as pure dispatch (recurse into directories,
classify files) instead of interleaving ignore policy with traversal.
The dot-prefix rule and the ignoredDirs blocklist were previously checked in two
separate places inside loadDir's loop. They are now combined behind one helper
that takes the entry name and whether it is a directory. isDirIgnored remains a
standalone blocklist predicate because the file watcher (isIgnoredPath) calls it
directly. Adds focused unit tests for isIgnoredEntry covering both states of
Scanner.IgnoreDotFolders. No behavior change.
* fix(scanner): stop watcher from scanning ignored dot folders
A filesystem change inside a dot-prefixed folder (e.g. ".Hidden Album/track.mp3")
previously triggered a targeted scan of that folder, because isIgnoredPath let
all media files through and only checked the changed path's parent against the
ignore list (which never matched for nested paths due to the trailing separator).
With Scanner.IgnoreDotFolders enabled this caused the folder to be indexed even
though a full scan would skip it.
The watcher now ignores any change located inside an ignored directory via a new
isUnderIgnoredDir helper that reuses the same isIgnoredEntry policy as the scan
walk, and checks the entry itself with isIgnoredEntry instead of the parent dir.
This keeps the watcher and the scanner in agreement for both dot-folders (gated
by the flag) and the ignoredDirs blocklist. Adds direct table tests for
isIgnoredPath covering both states of the option.
* fix(scanner): exclude '.' from isDotEntry and ignore dot media files in watcher
Addresses code review feedback:
- isDotEntry now excludes the literal "." reference, matching its documentation.
Previously isDotEntry(".") returned true, which could mark a path component as
a dot-entry in the watcher.
- isIgnoredPath now ignores dot-prefixed media files (e.g. ".hidden.mp3") so the
watcher matches the scanner, which always skips dot files. Non-media leaves
still fall through to the directory-assumption check, so dot-folders continue
to follow Scanner.IgnoreDotFolders.
Adds unit tests for isDotEntry and watcher coverage for dot-prefixed media files.
* docs(scanner): clarify isDotEntry multi-dot exclusion and add test
Expand the isDotEntry comment to explain why names with two or more
leading dots (".."/"..foo"/"...Album") are not treated as hidden, which
surprised a reviewer testing dot-folder scanning. Add a "..foo" test case
to make the two-leading-dots behavior explicit.
Claude-Session: https://claude.ai/code/session_012STiDTyhZAdH8JNtdNe8L1
|
||
|
|
3a14faa033 |
feat(subsonic): add structured sidecar lyrics support with OpenSubsonic v2 karaoke cues and agent layers (#5076)
Expand backend lyrics support with richer sidecar formats and upgrade the OpenSubsonic songLyrics implementation to the version 2 structured karaoke contract, while preserving version 1 behavior by default. Sidecar formats and parsing: - Add a TTML parser (core/lyrics/ttml.go): clock time, offset time, bare decimal seconds, nested timing contexts, and token-level <span> timing for word/syllable karaoke. Parses Apple Music-style metadata tracks (translation and pronunciation/transliteration) and agent metadata into per-track agents[] plus per-cue-line agentId. Hydrates missing line timing from cue timing. - Add an SRT parser (core/lyrics/srt.go). - Add a LRCLIB Lyricsfile (.yaml/.yml) parser (model/lyricsfile.go): maps per-word lines[].words[] to cues with inclusive UTF-8 byte offsets and attributes overlapping lines to synthetic voice agents so parallel vocals split correctly in the enhanced response. - Extend LRC parsing for Enhanced LRC inline <mm:ss.xx> word-timing markers. - Add UTF-8 BOM and UTF-16 LE support for TTML/LRC sidecars. - Parse the above formats from embedded tags as well as sidecar files. Source resolution: - Default lyricspriority is now ".ttml,.yaml,.yml,.elrc,.lrc,.srt,.txt,embedded" so the new formats are discoverable without manual configuration. - Preserve configured source priority across duplicate media-file candidates instead of only checking the first DB match, so higher-priority sidecar lyrics on older duplicates can still win. - Raise the embedded-lyrics tag maxLength to 1 MB to fit word-timed TTML/Enhanced-LRC karaoke for a full song. OpenSubsonic songLyrics v2: - Advertise songLyrics versions [1, 2]. - With enhanced=true, getLyricsBySongId may return structuredLyrics.kind (main/translation/pronunciation), cueLine[] line-level karaoke groupings, cueLine.cue[] timed words/syllables with required UTF-8 byteStart/byteEnd, reusable structuredLyrics.agents[], and cueLine.agentId references. - Without enhanced=true, the response stays v1-compatible: no kind, no cueLine, no agents, no non-main tracks; the existing line[] payload is always populated so legacy clients keep working. Contract details: - cueLine is emitted only for synced lyrics with cue data. - Within a cueLine, cue.end is normalized all-or-none and overlaps are removed; overlaps across separate cueLines remain valid for parallel vocal layers. - Missing cue end-times are filled from the next cue or the parent line. - When cueLines share an index, the one whose agent has role "main" is first. - LyricCue.Value is serialized as XML chardata; cues with nil start are skipped rather than serialized as 0. Refactoring: - Move pure format parsers into model/ (lyrics.go, lyrics_ttml.go, lyrics_srt.go, lyrics_embedded.go, lyricsfile.go) and extract Subsonic response building into server/subsonic/lyrics.go. - Centralize lyric-kind constants and add Lyrics.EffectiveKind/IsMainKind. - Add gg.Clone helper. Spec references: https://github.com/opensubsonic/open-subsonic-api/discussions/213 https://github.com/opensubsonic/open-subsonic-api/pull/218 (songLyrics v2) https://github.com/opensubsonic/open-subsonic-api/pull/228 (cue byte offsets) |
||
|
|
fc9cdf39c8 |
fix(conf): make Dir a plain value type to prevent sync.Once corruption (#5543)
Dir embedded sync.Once directly and exposed a value-receiver GoString so
that pretty.Sprintf("%# v", Server) could render the path. That meant
every pretty-print copied the entire Dir along with its Once, and a
goroutine concurrently using the original (or any copy) for Path() could
hit a "sync: unlock of unlocked mutex" runtime fatal error. The failure
was reproduced deterministically on Windows CI when test-suite shuffle
ordering raced cache initialization (utils/cache/file_caches.go's
NewFileCache.func1 -> conf.CacheFolder.MustPath) against the
configuration-dump pretty.Sprintf in Load().
Drop the sync.Once entirely. Dir is now a plain {path, perm} value type,
and Path() calls os.MkdirAll on every invocation. MkdirAll is
idempotent, so repeated calls on an existing directory cost one stat
syscall — negligible for the few config paths read at startup and during
cache init.
This removes the entire class of bug:
- No Mutex, so copies (via reflection, pretty-print, etc.) are safe.
- No state pointer, so no nil-state defensive checks scattered across
methods, and no risk of two copies seeing different lifecycle state.
- go vet is happy with the value receivers — the //nolint:govet
suppression on GoString is gone.
Adds two regression tests in conf/dir_test.go:
- GoString renders Dir as a quoted path under pretty.Sprintf (and
does not leak the internal struct fields).
- Concurrent copy + Path() stress test, locking in the copy-safety
property in case the type ever grows non-trivial state again.
|
||
|
|
823d851b75 |
refactor(transcoding): rename EnableTranscodingCancellation to Transcoding.EnableCancellation (#5523)
Move the option into the nested Transcoding config group alongside the limit knobs it interacts with, so all transcoding-related settings live together. The old top-level name is still honored via the existing mapDeprecatedOption / logDeprecatedOptions plumbing, which forwards the value to the new key and logs a deprecation warning at startup. The old struct field is removed (the new field is the single source of truth); the deprecated default is removed so viper.IsSet correctly distinguishes "user set the legacy option" from "no one set it." |
||
|
|
945d0ba1e2 |
fix(transcoding): cap concurrent transcodes to prevent ffmpeg DoS (#5522)
* feat(transcoding): add MaxConcurrent and MaxConcurrentPerUser config Introduce Transcoding.MaxConcurrent (default NumCPU()*2) and Transcoding.MaxConcurrentPerUser (default 3) to support upcoming concurrency limits on the streaming pipeline. No behavior change yet. Refs #5246 * feat(transcoding): add TranscodeLimiter with global and per-user caps Introduce a non-blocking limiter that gates concurrent transcodes. Returns ErrTooManyTranscodes immediately when the cap is reached so callers can translate it into a 429 response, rather than queuing requests. The per-user reservation is taken first to avoid burning a global slot that would only be rolled back when the per-user cap rejects the caller. Release is idempotent so wrapping the transcoder reader's Close is safe. Refs #5246 * feat(transcoding): cap concurrent transcodes in media streamer Acquire a TranscodeLimiter slot before spawning ffmpeg in the transcoding cache's read function, and release it when the resulting reader is closed. Raw streams and cache hits bypass the limiter so a single saturating client cannot block ordinary playback. When the cap is reached, ErrTooManyTranscodes bubbles up through cache.Get, ready for the HTTP layer to translate into a 429 response. Refs #5246 * feat(transcoding): return HTTP 429 with Retry-After when transcode cap is hit Map stream.ErrTooManyTranscodes to HTTP 429 in both the Subsonic API (/stream, /download) and the public share endpoint, including a 5s Retry-After hint. The Subsonic response still carries a failed-status envelope so clients that ignore HTTP codes also see the failure. Refs #5246 * feat(transcoding): default MaxConcurrent to 0 (disabled) Ship the limiter opt-in so existing installations are not affected by a behavior change on upgrade. Users hitting the DoS reported in #5246 can enable it by setting Transcoding.MaxConcurrent to a positive value (NumCPU()*2 is a reasonable starting point). Refs #5246 * fix(transcoding): make global and per-user caps independent Previously the limiter short-circuited to a no-op whenever MaxConcurrent was zero, silently ignoring a configured MaxConcurrentPerUser. Treat each cap independently so an operator can throttle per-user without enforcing a global ceiling (or vice versa), and only fall back to the no-op limiter when both caps are disabled. * fix(archiver): abort archive download when the transcode limiter rejects The album/artist/playlist zip writers were silently producing zip entries with headers but no data when ms.NewStream returned ErrTooManyTranscodes, because the per-file error was discarded by `_ = a.addFileToZip(...)`. The client received HTTP 200 with a corrupt zip and no indication that the server was rate-limited. Now the zip loop bails out as soon as it sees ErrTooManyTranscodes, and the Download handler swallows the error (the response status and Content-Disposition are already flushed by the time the limit is hit, so no 429 can be sent). The truncated zip surfaces the problem to the client; operators see a clear "transcode cap reached" warning in the server logs. Refs #5246 * fix(transcoding): release limiter slot on client close, not ffmpeg EOF Previously the slot was wrapped around the ffmpeg source reader, so it was only released by the cache's background copyAndClose goroutine when ffmpeg finished producing the file — meaning a client that disconnected after a single byte still held the slot for the full transcode duration. Under MaxConcurrent=N this serialized fresh requests behind abandoned encodes for minutes. Hand the release function back from the cache producer via the streamJob struct and wire it into the consumer-side Stream.Close. The HTTP handler already runs `defer stream.Close()`, so disconnect now frees the slot immediately. Cache hits never enter the producer and still pay no slot, and singleflight waiters on the same key correctly inherit no release (only the original producer's job holds the slot). Refs #5246 * fix(transcoding): skip per-user cap for anonymous requests Public share viewers have no user in context, so userName(ctx) returned the literal string "UNKNOWN" and the limiter mapped every anonymous viewer to the same bucket. With MaxConcurrentPerUser=N, only N unrelated anonymous clients could stream a viral share at any time — the opposite of the fairness the per-user cap is meant to provide. Introduce a limiterKey(ctx) helper that returns "" for anonymous callers (userName(ctx) is unchanged for logs), and teach Acquire to skip the per-user reservation when the key is empty. The global cap is still enforced for anonymous traffic and remains the protection against runaway anonymous load. Refs #5246 * refactor(transcoding): tidy limiter struct and centralize Retry-After Per review feedback: - Drop the redundant maxConcurrent field on transcodeLimiter; the channel capacity already enforces the global cap and the field was only used inside the constructor. - Only allocate the perUser map when MaxConcurrentPerUser > 0. - Move the Retry-After value into core/stream as RetryAfterSeconds so the Subsonic API and public-share handlers cannot drift if the window is later tuned. * fix(transcoding): do not log limiter rejections as cache failures NewStream was emitting an error-level "Error accessing transcoding cache" log whenever cache.Get returned anything non-nil, including the limiter's ErrTooManyTranscodes — even though the producer had already logged the rejection at warn level. The result was double logging and a misleading "cache failure" classification that buries real cache problems. Skip the error log when the cause is ErrTooManyTranscodes; the warn line from the producer is the canonical signal. * fix(archiver): open stream before writing zip entry header Per review: addFileToZip previously called z.CreateHeader before NewStream, so when the limiter rejected a transcode the zip already contained a 0-byte entry for that track. Open the source first and only write the header once the read side is ready; rejections now skip the entry entirely. The truncation comment in handleArchiveErr was also misleading — z.Close finalises the central directory, so the client receives a well-formed zip containing only the tracks written before the rejection, not a "truncated" archive. Reword to match reality. * fix(transcoding): hold slot for ffmpeg lifetime, force cancellable ctx The previous release-on-consumer-close design let a client open many unique transcodes, disconnect immediately, and still spawn the configured cap's worth of ffmpeg processes — the cache writer goroutine continued draining ffmpeg to disk after the client disappeared, defeating the DoS protection the limiter is meant to provide. Move the release back onto the source reader so the slot is freed only when ffmpeg actually exits (either EOF or context cancellation). To keep disconnects from leaking slots for the full transcode duration, force the request context into ffmpeg whenever the limiter is enabled — so client disconnect cancels the process and frees the slot promptly. When the limiter is disabled, the legacy EnableTranscodingCancellation behavior is preserved unchanged. Reported by codex and Copilot reviewers on #5522. |
||
|
|
03ac02d964 |
refactor: more warnings clean up
Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
8f0b4930ff |
refactor(conf): replace eager dir creation with lazy Dir type (#5495)
* feat(conf): add Dir type with lazy directory creation Introduces the Dir type that wraps a directory path string and defers os.MkdirAll until the first call to Path() or MustPath(), using sync.Once to ensure the creation happens exactly once. Implements fmt.Stringer, encoding.TextMarshaler, and encoding.TextUnmarshaler for config integration. Includes Ginkgo/Gomega tests covering all methods and error paths. * refactor(conf): replace eager dir creation with lazy Dir type Change DataFolder, CacheFolder, Plugins.Folder, and Backup.Path from string to Dir. Remove all os.MkdirAll calls from Load() so directories are created lazily on first Path()/MustPath() call. Artwork folder creation was already handled at point-of-use in image_upload.go. Add SnapshotConfig() to conf package for safe test config save/restore that avoids copying sync.Once inside Dir fields. Fix copy-lock vet warning in nativeapi/config.go by marshalling pointer instead of value. * refactor(conf): migrate tests and db init to lazy Dir type Update all test files to use conf.NewDir() for Dir field assignments. Ensure DataFolder is created lazily when the database is first opened in db.Db(). Remove eager directory creation from conf.Load() tests. * fix(conf): address review findings for Dir type - Use os.ModePerm for DataFolder/CacheFolder (was 0700, should match original behavior). Add NewDirWithPerm for PluginsFolder (0700). - Use Path() instead of MustPath() in db.Prune() to avoid logFatal from background cron job. - Panic on marshal/unmarshal errors in SnapshotConfig (test helper). - Clean up redundant String()/MustPath() calls in plugin manager. - Remove dead code in dir_test.go. Signed-off-by: Deluan <deluan@navidrome.org> * fix(conf): add GoString to Dir for clean config dump output Implement fmt.GoStringer on Dir so pretty.Sprintf shows the path string instead of internal struct fields (sync.Once, perm, err). Also add TODO comment to configtest about removing the indirection. * fix(dir): improve error logging in MustPath method Signed-off-by: Deluan <deluan@navidrome.org> * refactor(tests): remove redundant tests for unwritable DataFolder and CacheFolder Signed-off-by: Deluan <deluan@navidrome.org> * fix(conf): address PR review feedback - Ensure Plugins.Folder always uses 0700, even when user-configured (previously only the derived default got restrictive permissions). - Create LogFile parent directory before opening, so LogFile paths inside a not-yet-created DataFolder work correctly. --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
5f0245ea84 |
fix(server): prevent artwork throttle token starvation on slow clients (#5472)
* fix(server): prevent artwork throttle token starvation on slow clients Replace Chi's ThrottleBacklog middleware for artwork endpoints with a custom RequestThrottle that releases processing tokens before writing the HTTP response. Previously, a slow or stalled client could hold a throttle token indefinitely during io.Copy, exhausting all 2-4 slots and blocking artwork requests for all users (reported after 15+ days uptime). The new approach buffers artwork into memory while holding the token, releases it immediately, then writes the buffered response. A 30-second per-request write deadline (SetWriteTimeout) prevents stalled writes from blocking indefinitely. Throttle exhaustion is now logged with context for operator visibility. * refactor(server): simplify throttle to middleware with same API as Chi Restructure RequestThrottle from a DI-injected type into a drop-in middleware function with the same signature as Chi's ThrottleBacklog. Handlers are reverted to their original simple form (no throttle awareness), and the middleware is applied at route definition time just like before. This eliminates the DI dependency, removes the artworkThrottle field from both Router structs, and consolidates SetWriteTimeout into the throttle file. When limit <= 0, the middleware returns a passthrough so callers don't need a guard. Signed-off-by: Deluan <deluan@navidrome.org> * feat(server): add opt-out flag for buffered artwork throttle Add DevArtworkThrottleBuffered config (default true) that controls whether the new buffered ThrottleBacklog middleware is used. When set to false, it falls back to Chi's original middleware, giving users a safety valve in case the buffered implementation causes issues. Signed-off-by: Deluan <deluan@navidrome.org> * test(server): clean up throttle tests for clarity and speed Consolidate duplicate router setup into runTwoRequests() and slowClientTest() helpers. Replace time.Sleep-based token holding with channel synchronization, reducing suite time from ~7s to ~1.5s. Remove redundant test, fix duplicate comment block, and add comment explaining why slowTestWriter can't embed httptest.ResponseRecorder. * fix: release artwork throttle tokens on panic Defer the buffered artwork throttle release inside the handler closure so tokens are returned even when a downstream handler panics before response flushing. Document that the middleware buffers full responses in memory and add a regression test covering recovery after a panic. * fix: align buffered throttle response behavior Keep only the first status code written to the buffered artwork throttle response writer so it matches net/http semantics. Strengthen the opt-out test to verify DevArtworkThrottleBuffered=false uses Chi's original slow-client behavior instead of only checking shared 429 handling. * refactor(server): remove setWriteTimeout from throttle middleware SetWriteDeadline only constrains the server's Write syscall, not how fast the client reads from the TCP buffer. For artwork-sized responses (up to ~500KB), the kernel accepts the entire write immediately even over real network interfaces due to TCP buffer auto-tuning. Verified by testing with a stalled client over both loopback and en0 — the deadline never triggers. The actual protection comes from buffering + early token release, which is already in place. --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
7e16b6acb5 |
feat(ui): replace UI scrobble with reportPlayback and redesign NowPlaying panel (#5448)
* feat(config): add UIPlaybackReportInterval setting * feat(server): expose playbackReportIntervalMs to UI config * feat(ui): add playbackReportIntervalMs config default * feat(ui): replace scrobble/nowPlaying with reportPlayback in subsonic API layer * feat(ui): replace scrobble logic with reportPlayback state machine in Player * refactor(ui): simplify Player heartbeat using useInterval hook - Replace manual setInterval/clearInterval with existing useInterval hook - Extract shared reportPlaybackUrl helper to deduplicate URL construction - Use ref for currentTrackId in beforeunload to stabilize effect deps - Have heartbeat read lastPositionMsRef instead of audioInstance.currentTime * feat(ui): redesign NowPlaying panel with Discord-style layout Show album art with play/pause overlay icon, track title, artist, album name, progress bar with position/duration, and username. * fix(ui): adjust NowPlaying panel height to fit 3 entries * fix(ui): send stopped on player close and tab close while paused - onBeforeDestroy now sends reportPlayback stopped before clearing queue - beforeunload sends stopped beacon regardless of pause state * feat(ui): animate NowPlaying progress bar with 1s client-side tick * fix(ui): account for playbackRate in NowPlaying progress interpolation * fix(ui): use timestamp-based interpolation for smooth NowPlaying progress Replace tick counter with fetchedAt timestamp so the progress bar advances smoothly without resetting on each server poll. * fix(ui): fix NowPlaying progress bar not animating Pass `now` (Date.now()) as a prop that changes every tick, so React.memo'd components actually re-render each second. * fix(ui): prevent progress bar reset on NowPlaying poll Set fetchedAt and now atomically on fetch so the elapsed offset starts at zero and the server's already-estimated positionMs is used as the base without a visible jump. * fix(ui): stamp entries with fetch time to prevent progress bar reset Embed _fetchedAt timestamp directly into each entry object so the position and its reference timestamp are always in the same state update, eliminating the React 17 multi-setState batching race. * fix(server): estimate position for starting state in GetNowPlaying GetNowPlaying was only estimating elapsed position for the "playing" state, returning raw positionMs=0 for "starting". Since the UI player sends "starting" once and then doesn't update until the 60s heartbeat, NowPlaying polls returned 0 for up to a minute, causing the progress bar to reset on every poll. * fix(ui): send playing immediately after starting to enable position estimation The server only estimates elapsed position for "playing" state in GetNowPlaying. The Player was sending "starting" once and then not updating until the 60s heartbeat, leaving the server state as "starting" with positionMs=0 for up to a minute. Now the Player follows up "starting" with an immediate "playing" call, transitioning the server state so position estimation works from the first poll. * fix(subsonic): fix getNowPlaying returning same playerId for all entries PlayerId was never incremented in the map callback, so every entry got playerId=1. This caused the UI to use duplicate React keys, mixing up rendered entries between players. Also use a stable composite key in the UI instead of the sequential playerId. * fix(ui): only send stopped beacon when tab is actually closing Move the reportPlaybackBeacon call from beforeunload to pagehide. beforeunload fires before the confirmation dialog, so cancelling the close would still send stopped. pagehide only fires when the page is actually being unloaded. * fix(ui): revert to beforeunload for stopped beacon pagehide does not fire reliably in Chrome when closing tabs. Use beforeunload instead — if the user cancels the close dialog, the heartbeat will re-register the NowPlaying entry on its next tick. * fix(ui): use synchronous XHR for stopped report on tab close Replace sendBeacon with synchronous XMLHttpRequest in beforeunload. This blocks the page from closing until the server acknowledges the stopped state, ensuring the NowPlaying entry is always removed. * fix(ui): fix confirmation dialog and use fetch keepalive for tab close - Move e.preventDefault() before the stopped report so the dialog always shows regardless of XHR errors - Use fetch with keepalive:true instead of sync XHR (more reliable, non-blocking, survives page teardown) - Fall back to sendBeacon if fetch throws * fix(ui): prevent heartbeat from re-adding entry after stopped on tab close Set a stoppedRef flag in beforeunload so the heartbeat interval skips sending playing reports after stopped has been sent. Without this, the heartbeat could re-register the NowPlaying entry after the stopped event removed it. * fix(ui): include client unique ID header in stopped report on tab close Root cause: reportPlaybackSync (fetch keepalive) did not include the X-ND-Client-Unique-Id header. Regular reportPlayback calls via httpClient include this header, and the server uses it as the playMap key. Without the header, the stopped call fell back to player.ID as the key, which didn't match the entry added with the UUID key. The playMap.Remove targeted the wrong key, so the entry persisted. Fix: export clientUniqueId from httpClient and include it as a header in the fetch keepalive request. * fix(ui): use pagehide for stopped report to avoid premature send beforeunload fires before the confirmation dialog, so the stopped event was sent even when the user cancelled closing. Move the stopped report to pagehide, which only fires when the page is actually being unloaded (after confirmation). * feat(server): broadcast NowPlaying SSE on every state change Previously, the SSE broadcast only fired when the NowPlaying count changed. Now it fires on every reportPlayback call (starting, playing, paused, stopped), so the NowPlaying panel gets instant updates for state transitions and position changes. The UI reducer stores a nowPlayingLastUpdate timestamp alongside the count, ensuring every SSE event triggers a re-fetch even when the count is unchanged (e.g., pause/resume). * fix(ui): clamp NowPlaying position to prevent negative time display * fix(ui): debounce NowPlaying fetches to prevent progress bar trembling During track changes, rapid SSE events (stopped, starting, playing) triggered multiple refetches within milliseconds, each resetting the interpolation base and causing the progress bar to oscillate. Skip fetches within 1 second of the previous fetch. * feat(ui): report playback position on seek Send a reportPlayback(playing) call when the user seeks/scrubs in the player, so the NowPlaying panel and server position stay in sync immediately instead of waiting for the next 60s heartbeat. * refactor: code review cleanup - Export clientUniqueIdHeader from httpClient, use in subsonic layer - Fix variable shadowing (now → fetchNow) in NowPlayingPanel fetchList - Fix onBeforeDestroy nested dep (read isRadio from ref instead) - Only broadcast SSE on state transitions, not heartbeat position updates - Only enqueue NowPlaying to external scrobblers on state transitions Signed-off-by: Deluan <deluan@navidrome.org> * fix(ui): use trailing-edge debounce for NowPlaying fetch Replace the leading-edge throttle (which fetched on the first event and blocked subsequent ones) with a trailing-edge debounce (300ms). During track transitions, the burst of events (stopped → starting → playing) now collapses into a single fetch after the burst settles, showing the new track immediately instead of briefly showing empty. * fix(ui): only show overlay on NowPlaying artwork when paused Signed-off-by: Deluan <deluan@navidrome.org> * refactor(ui): remove unnecessary sendBeacon fallback from reportPlaybackSync * refactor(ui): rename reportPlaybackSync to reportPlaybackKeepalive The function was never synchronous — it uses fetch with keepalive:true, which is fire-and-forget. The name now reflects the actual behavior. * style: format code with prettier * test: add tests for reportPlayback SSE broadcast and UI changes - play_tracker: verify SSE broadcast on every state transition and that broadcasts are skipped when EnableNowPlaying is false - activityReducer: verify nowPlayingLastUpdate timestamp is set - subsonic/index: verify reportPlayback URL construction Signed-off-by: Deluan <deluan@navidrome.org> * fix(ui): prevent NowPlaying from fetching every second when panel is open fetchList had unstable identity because it depended on doFetch (which depended on notify/dispatch). Each 1s setNow re-render recreated the callback chain, re-triggering the useEffect that calls fetchList. Use a ref for the fetch logic so fetchList has a stable identity with empty deps. * fix(ui): break fetch→dispatch→effect→fetch loop in NowPlaying panel The fetch dispatched nowPlayingCountUpdate on every result, which updated nowPlayingLastUpdate in Redux, which triggered the SSE effect to call fetchList again — creating a fetch loop. Fix: remove dispatch from fetch results. The badge count uses entries.length (from local state) when entries are loaded, falling back to Redux count (from SSE) when they aren't. SSE events remain the only trigger for nowPlayingLastUpdate, breaking the loop. * fix(ui): clear NowPlaying entries on panel close so badge uses SSE count * style: format code with prettier * fix: address code review feedback - Fix currentTime truthiness check to handle position 0 correctly - Report actual player state (playing/paused) on seek instead of always sending 'playing' - Remove idx from React key to avoid reorder issues - Add debounce timer cleanup on unmount - Keep entries on panel close so badge stays accurate from polling - Fix test description to match actual assertion * fix(ui): keep NowPlaying badge count accurate from polling Add a separate nowPlayingCountSync action that updates the Redux count without setting nowPlayingLastUpdate (which would trigger the SSE effect and cause a fetch loop). Polling results now sync the badge count via this action, so the badge stays accurate even when SSE is unavailable. * style: format code with prettier --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
44e63596a0 |
feat(server): add EnforceNonRootUser config option to exit early if started as root (#5373)
* feat(config): Add EnforceNonRootUser config option to exit early if started as root Signed-off-by: Aengus Walton <ventolin@gmail.com> * Move validateEnforceNonRootUser check to directly after parsing the config * Ensure the data directory hasn't been created in test --------- Signed-off-by: Aengus Walton <ventolin@gmail.com> Co-authored-by: Deluan Quintão <deluan@navidrome.org> |
||
|
|
3b7d3f4383 |
feat(matcher): add Matcher.PreferStarred option to bias fuzzy matcher toward starred/high-rated tracks (#5387)
* matcher: update godoc for matcher config scoring order * conf: log deprecated SimilarSongsMatchThreshold option * conf: enable matcher prefer-starred by default |
||
|
|
1988a4162e |
refactor(configuration): improve error handling in configuration validation
Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
c49e5855b9 |
feat(artwork): make max image upload size configurable (#5335)
* 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 <deluan@navidrome.org> |
||
|
|
c87db92cee |
fix(artwork): address WebP performance regression on low-power hardware (#5286)
* refactor(artwork): rename DevJpegCoverArt to EnableWebPEncoding
Replaced the internal DevJpegCoverArt flag with a user-facing
EnableWebPEncoding config option (defaults to true). When disabled, the
fallback encoding now preserves the original image format — PNG sources
stay PNG for non-square resizes, matching v0.60.3 behavior. The previous
implementation incorrectly re-encoded PNG sources as JPEG in non-square
mode. Also added EnableWebPEncoding to the insights data.
* feat: add configurable UICoverArtSize option
Converted the hardcoded UICoverArtSize constant (600px) into a
configurable option, allowing users to reduce the cover art size
requested by the UI to mitigate slow image encoding. The value is
served to the frontend via the app config and used by all components
that request cover art. Also simplified the cache warmer by removing
a single-iteration loop in favor of direct code.
* style: fix prettier formatting in subsonic test
* feat: log WebP encoder/decoder selection
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(artwork): address PR review feedback
- Add DevJpegCoverArt to logRemovedOptions so users with the old config
key get a clear warning instead of a silent ignore.
- Include EnableWebPEncoding in the resized artwork cache key to prevent
stale WebP responses after toggling the setting.
- Skip animated GIF to WebP conversion via ffmpeg when EnableWebPEncoding
is false, so the setting is consistent across all image types.
- Fix data race in cache warmer by reading UICoverArtSize at construction
time instead of per-image, avoiding concurrent access with config
cleanup in tests.
- Clarify cache warmer docstring to accurately describe caching behavior.
* Revert "fix(artwork): address PR review feedback"
This reverts commit
|
||
|
|
220019a9f1 |
fix: add missing viper defaults for mpvpath, artistimagefolder, and plugins.loglevel
Fix #5284 Several configOptions struct fields were missing corresponding viper.SetDefault entries, making them invisible to environment variable overrides and config file parsing. Added defaults for mpvpath (consistent with ffmpegpath), artistimagefolder, and plugins.loglevel. Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
049fc78177 |
refactor: extract logFatal helper for config error handling
Replace 14 repeated fmt.Fprintln(os.Stderr, "FATAL:", ...)/os.Exit(1) patterns with a single logFatal function. This reduces duplication and makes all fatal config paths testable via SetLogFatal. Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
2b041c02ad |
feat: accept ND_-prefixed env var names in config files (#5258)
* feat: add toPascalCase helper for config key display Adds a toPascalCase helper that converts dotted lowercase Viper config keys (e.g. 'scanner.schedule') to PascalCase (e.g. 'Scanner.Schedule') for use in user-facing warning messages. Includes export_test.go binding and a full Ginkgo DescribeTable test suite covering simple, dotted, multi-segment, already-capitalized, and empty-string cases. * feat: remap ND_-prefixed env var names found in config files Detect when users mistakenly use environment variable names (like ND_ADDRESS) in config files, remap them to canonical keys, and warn. Fatal error if both ND_ and canonical versions of the same key exist. Closes #5242 |
||
|
|
f33ca75378 |
refactor: rename EnableCoverArtUpload to EnableArtworkUpload
The config flag gates all image uploads (artists, radios, playlists), not just cover art. Rename it to accurately reflect its scope across the backend config, native API permission check, Subsonic CoverArtRole, serve_index JSON key, and frontend config. |
||
|
|
8a19fa9991 |
fix(server): require additional variable to enable systemd logging (#5222)
* fix(logging): require additional variable to enable systemd logging * use a better name |
||
|
|
5cd1fcb492 |
feat(scheduler): add crontab(5) random ~ syntax support (#5233)
* feat(scheduler): add CrontabSchedule with crontab(5) random ~ syntax Implement ParseCrontab() that extends robfig/cron with support for the crontab(5) random ~ operator (e.g., 0~30 * * * *). Random values are resolved fresh on each Next() call for load spreading. Supports A~B, ~B, A~, and bare ~ forms in all 6 fields (including seconds). Expressions without ~ delegate to robfig's standard parser with zero overhead. Integrates into scheduler.Add() and conf.validateSchedule() so that scanner.schedule and backup.schedule config values accept ~ syntax. * refactor(scheduler): resolve random ~ values once at parse time Change from per-Next() randomization to per-parse randomization, matching crontab(5) semantics. This prevents double-firing within the same period when random values land after the current time. ParseCrontab now resolves ~ fields to concrete values, substitutes them into the spec string, and delegates to robfig's parser. This eliminates CrontabSchedule, randomField, and resolveField entirely. * test(scheduler): replace WaitGroup with channel for job execution synchronization Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
ab8a58157a |
feat: add artist image uploads and image-folder artwork source (#5198)
* feat: add shared ImageUploadService for entity image management * feat: add UploadedImage field and methods to Artist model * feat: add uploaded_image column to artist table * feat: add ArtistImageFolder config option * refactor: wire ImageUploadService and delegate playlist file ops to it Wire ImageUploadService into the DI container and refactor the playlist service to delegate image file operations (SetImage/RemoveImage) to the shared ImageUploadService, removing duplicated file I/O logic. A local ImageUploadService interface is defined in core/playlists to avoid an import cycle between core and core/playlists. * feat: artist artwork reader checks uploaded image first * feat: add image-folder priority source for artist artwork * feat: cache key invalidation for image-folder and uploaded images * refactor: extract shared image upload HTTP helpers * feat: add artist image upload/delete API endpoints * refactor: playlist handlers use shared image upload helpers * feat: add shared ImageUploadOverlay component * feat: add i18n keys for artist image upload * feat: add image upload overlay to artist detail pages * refactor: playlist details uses shared ImageUploadOverlay component * fix: add gosec nolint directive for ParseMultipartForm * refactor: deduplicate image upload code and optimize dir scanning - Remove dead ImageFilename methods from Artist and Playlist models (production code uses core.imageFilename exclusively) - Extract shared uploadedImagePath helper in model/image.go - Extract findImageInArtistFolder to deduplicate dir-scanning logic between fromArtistImageFolder and getArtistImageFolderModTime - Fix fileInputRef in useCallback dependency array * fix: include artist UpdatedAt in artwork cache key Without this, uploading or deleting an artist image would not invalidate the cached artwork because the cache key was only based on album folder timestamps, not the artist's own UpdatedAt field. * feat: add Portuguese translations for artist image upload * refactor: use shared i18n keys for cover art upload messages Move cover art upload/remove translations from per-entity sections (artist, playlist) to a shared top-level "message" section, avoiding duplication across entity types and translation files. * refactor: move cover art i18n keys to shared message section for all languages * refactor: simplify image upload code and eliminate redundancies Extracted duplicate image loading/lightbox state logic from DesktopArtistDetails and MobileArtistDetails into a shared useArtistImageState hook. Moved entity type constants to the consts package and replaced raw string literals throughout model, core, and nativeapi packages. Exported model.UploadedImagePath and reused it in core/image_upload.go to consolidate path construction. Cached the ArtistImageFolder lookup result in artistReader to eliminate a redundant os.ReadDir call on every artwork request. Signed-off-by: Deluan <deluan@navidrome.org> * style: fix prettier formatting in ImageUploadOverlay * fix: address code review feedback on image upload error handling - RemoveImage now returns errors instead of swallowing them - Artist handlers distinguish not-found from other DB errors - Defer multipart temp file cleanup after parsing * fix: enforce hard request size limit with MaxBytesReader for image uploads Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
aa93911991 |
feat(server): add syslog priority prefixes for systemd-journald (#5192)
* fix: add syslog priority prefixes for systemd-journald When running under systemd, all log messages were assigned priority 3 (error) by journald because navidrome wrote plain text to stderr without syslog priority prefixes. Add a journalFormatter that wraps the existing logrus formatter and prepends <N> syslog priority prefixes (RFC 5424) to each log line. The formatter is automatically enabled when the JOURNAL_STREAM environment variable is set (indicating the process is managed by systemd). Priority mapping: - Fatal/Panic → <2>/<0> (crit/emerg) - Error → <3> (err) - Warn → <4> (warning) - Info → <6> (info) - Debug/Trace → <7> (debug) Fixes #5142 * test: refactor journalFormatter tests to use Ginkgo and DescribeTable Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> Co-authored-by: Deluan <deluan@navidrome.org> |
||
|
|
69e7d163fc |
remove built-in Spotify integration (#5197)
* refactor: remove built-in Spotify integration Remove the Spotify adapter and all related configuration, replacing the built-in integration with the plugin system. This deletes the adapters/spotify package, removes Spotify config options (ID/Secret), updates the default agents list from "deezer,lastfm,spotify" to "deezer,lastfm", and cleans up all references across configuration, metrics, logging, artwork caching, and documentation. Users with Spotify config options will now see a warning that the options are no longer available. * feat: add ListenBrainz to list of default agents Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
49a14d4583 |
feat(artwork): add per-disc cover art support (#5182)
* feat(artwork): add KindDiscArtwork and ParseDiscArtworkID Add new disc artwork kind with 'dc' prefix for per-disc cover art support. The composite ID format is albumID:discNumber, parsed by the new ParseDiscArtworkID helper. * feat(conf): add DiscArtPriority configuration option Default: 'disc*.*, cd*.*, embedded'. Controls how per-disc cover art is resolved, following the same pattern as CoverArtPriority and ArtistArtPriority. * feat(artwork): implement extractDiscNumber helper Extracts disc number from filenames based on glob patterns by parsing leading digits from the wildcard-matched portion. Used for matching disc-specific artwork files like disc1.jpg. * feat(artwork): implement fromDiscExternalFile source function Disc-aware variant of fromExternalFile that filters image files by disc number (extracted from filename) or folder association (for multi-folder albums). * feat(artwork): implement discArtworkReader Resolves disc artwork using DiscArtPriority config patterns. Supports glob patterns with disc number extraction, embedded images from first track, and falls back to album cover art. Handles both multi-folder and single-folder multi-disc albums. * feat(artwork): register disc artwork reader in dispatcher Add KindDiscArtwork case to getArtworkReader switch, routing disc artwork requests to the new discArtworkReader. * feat(subsonic): add CoverArt field to DiscTitle response Implements OpenSubsonic PR #220: optional cover art ID in DiscTitle responses for per-disc artwork support. * feat(subsonic): populate CoverArt in DiscTitle responses Each DiscTitle now includes a disc artwork ID (dc-albumID:discNum) that clients can use with getCoverArt to retrieve per-disc artwork. * style: fix file permission in test to satisfy gosec * feat(ui): add disc cover art display and lightbox functionality Signed-off-by: Deluan <deluan@navidrome.org> * refactor: simplify disc artwork code - Add DiscArtworkID constructor to encapsulate the "albumID:discNumber" format in one place - Convert fromDiscExternalFile to a method on discArtworkReader, reducing parameter count from 6 to 2 - Remove unused rootFolder field from discArtworkReader * style: fix prettier formatting in subsonic index * style(ui): move cursor style to makeStyles in SongDatagrid * feat(artwork): add discsubtitle option to DiscArtPriority Allow matching disc cover art by the disc's subtitle/name. When the "discsubtitle" keyword is in the priority list, image files whose stem matches the disc subtitle (case-insensitive) are used. This is useful for box sets with named discs (e.g., "The Blue Disc.jpg"). * feat(configuration): update discartpriority to include cover art options Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
4ddb0774ec |
perf(artwork): improve image serving performance with WebP encoding and optimized pipeline (#5181)
* test(artwork): add benchmark helpers for generating test images * test(artwork): add image decode benchmarks for JPEG/PNG at various sizes * test(artwork): add image resize benchmarks for Lanczos at various sizes * test(artwork): add image encode benchmarks for JPEG quality levels and PNG * test(artwork): add full resize pipeline benchmark (decode+resize+encode) * test(artwork): add tag extraction benchmark for embedded art * test(cache): add file cache benchmarks for read, write, and concurrent access * test(artwork): add E2E benchmarks for artwork.Get with cache on/off and concurrency * fix(test): use absolute path for tag extraction benchmark fixture * test(artwork): add resize alternatives benchmark comparing resamplers * perf(artwork): switch to CatmullRom resampler and JPEG for square images Replace imaging.Lanczos with imaging.CatmullRom for image resizing (30% faster, indistinguishable quality at thumbnail sizes). Stop forcing PNG encoding for square images when the source is JPEG — JPEG is smaller and faster to encode. Square images from JPEG sources went from 52ms to 10ms (80% improvement). Add sync.Pool for encode buffers to reduce GC pressure under concurrent load. * perf(artwork): increase cache warmer concurrency from 2 to 4 workers Resize is CPU-bound, so more workers improve throughput on multi-core systems. Doubled worker count to better utilize available cores during background cache warming. * perf(artwork): switch to xdraw.ApproxBiLinear and always encode as JPEG Replace disintegration/imaging with golang.org/x/image/draw for image resizing. This eliminates ~92K allocations per resize (from imaging's internal goroutine parallelism) down to ~20, reducing GC pressure under concurrent load. Always encode resized artwork as JPEG regardless of source format, since cover art doesn't need transparency. This is ~5x faster than PNG encode and produces much smaller output (e.g. 18KB JPEG vs 124KB PNG). * perf(artwork): skip external API call when artist image URL is cached ArtistImage() was always calling the external agent (Spotify/Last.fm) to get the image URL, even when the artist already had URLs stored in the database. This caused every artist image request to block on an external API call, creating severe serialization when loading artist grids (5-20 seconds for the first page). Now use the stored URL directly when available. Artists with no stored URL still fetch synchronously. Background refresh via UpdateArtistInfo handles TTL-based URL updates. * perf(artwork): increase getCoverArt throttle from NumCPU/3 to NumCPU The previous default of max(2, NumCPU/3) was too aggressive for artist images which are I/O-bound (downloading from external CDNs), not CPU-bound. On an 8-core machine this meant only 2 concurrent requests, causing a staircase pattern where 12 images took ~2.4s wall-clock. Bumping to max(4, NumCPU) cuts wall-clock time by ~50% for artist image grids while still preventing unbounded concurrency for CPU-bound resizes. * perf(artwork): encode resized images as WebP instead of JPEG Switch from JPEG to WebP encoding for resized artwork using gen2brain/webp (libwebp via WASM, no CGo). WebP produces ~74% smaller output at the same quality with only ~25% slower full-pipeline encode time (cached, so only paid once per artwork+size). Use NRGBA image type to preserve alpha channel in WebP output, and transparent padding for square canvas instead of black. Also removes the disintegration/imaging dependency entirely by replacing imaging.Fill in playlist tile generation with a custom fillCenter function using xdraw.ApproxBiLinear. * perf(artwork): switch from ApproxBiLinear to BiLinear scaling for improved image processing Signed-off-by: Deluan <deluan@navidrome.org> * refactor(configuration): rename CoverJpegQuality to CoverArtQuality and update references Signed-off-by: Deluan <deluan@navidrome.org> * feat(artwork): add DevJpegCoverArt option to control JPEG encoding for cover art Signed-off-by: Deluan <deluan@navidrome.org> * fix(artwork): remove redundant transparent fill and handle encode errors in resizeImage Removed a no-op draw.Draw call that filled the NRGBA canvas with transparent pixels — NewNRGBA already zero-initializes to fully transparent. Also added an early return on encode failure to avoid allocating and copying potentially corrupt buffer data before returning the error. * fix(configuration): reorder default agents (deezer is faster) Signed-off-by: Deluan <deluan@navidrome.org> * fix(test): resolve dogsled lint warning in tag extraction benchmark Use all return values from runtime.Caller instead of discarding three with blank identifiers, which triggered the dogsled linter. * fix(artwork): revert cache key format Signed-off-by: Deluan <deluan@navidrome.org> * fix(configuration): remove deprecated CoverJpegQuality field and update references to CoverArtQuality Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
ae1e0ddb11 |
feat(subsonic): implement OpenSubsonic Transcoding extension (#4990)
* feat(subsonic): implement transcode decision logic and codec handling for media files Signed-off-by: Deluan <deluan@navidrome.org> * fix(subsonic): update codec limitation structure and decision logic for improved clarity Signed-off-by: Deluan <deluan@navidrome.org> * fix(transcoding): update bitrate handling to use kilobits per second (kbps) across transcode decision logic Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): simplify container alias handling in matchesContainer function Signed-off-by: Deluan <deluan@navidrome.org> * fix(transcoding): enforce POST method for GetTranscodeDecision and handle non-POST requests Signed-off-by: Deluan <deluan@navidrome.org> * feat(transcoding): add enums for protocol, comparison operators, limitations, and codec profiles in transcode decision logic Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): streamline limitation checks and applyLimitation logic for improved readability and maintainability Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): replace strings.EqualFold with direct comparison for protocol and limitation checks Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): rename token methods to CreateTranscodeParams and ParseTranscodeParams for clarity Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): enhance logging for transcode decision process and client info conversion Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): rename TranscodeDecision to Decider and update related methods for clarity Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): enhance transcoding config lookup logic for audio codecs Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): enhance transcoding options with sample rate support and improve command handling Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): add bit depth support for audio transcoding and enhance related logic Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): enhance AAC command handling and support for audio channels in streaming Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): streamline transcoding logic by consolidating stream parameter handling and enhancing alias mapping Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcoding): update default command handling and add codec support for transcoding Signed-off-by: Deluan <deluan@navidrome.org> * fix: implement noopDecider for transcoding decision handling in tests Signed-off-by: Deluan <deluan@navidrome.org> * fix: address review findings for OpenSubsonic transcoding PR Fix multiple issues identified during code review of the transcoding extension: add missing return after error in shared stream handler preventing nil pointer panic, replace dead r.Body nil check with MaxBytesReader size limit, distinguish not-found from other DB errors, fix bpsToKbps integer truncation with rounding, add "pcm" to isLosslessFormat for consistency with model.IsLossless(), add sampleRate/bitDepth/channels to streaming log, fix outdated test comment, and add tests for conversion functions and GetTranscodeStream parameter passing. * feat(transcoding): add sourceUpdatedAt to decision and validate transcode parameters Signed-off-by: Deluan <deluan@navidrome.org> * fix: small issues Updated mock AAC transcoding command to use the new default (ipod with fragmented MP4) matching the migration, ensuring tests exercise the same buildDynamicArgs code path as production. Improved archiver test mock to match on the whole StreamRequest struct instead of decomposing fields, making it resilient to future field additions. Added named constants for JWT claim keys in the transcode token and wrapped ParseTranscodeParams errors with ErrTokenInvalid for consistency. Documented the IsLossless BitDepth fallback heuristic as temporary until Codec column is populated. Signed-off-by: Deluan <deluan@navidrome.org> * fix(transcoding): adapt transcode claims to struct-based auth.Claims Updated transcode token handling to use the struct-based auth.Claims introduced on master, replacing the previous map[string]any approach. Extended auth.Claims with transcoding-specific fields (MediaID, DirectPlay, UpdatedAt, Channels, SampleRate, BitDepth) and added float64 fallback in ClaimsFromToken for numeric claims that lose their Go type during JWT string serialization. Also added the missing lyrics parameter to all subsonic.New() calls in test files. * feat(model): add ProbeData field and UpdateProbeData repository method Add probe_data TEXT column to media_file for caching ffprobe results. Add UpdateProbeData to MediaFileRepository interface and implementations. Use hash:"ignore" tag so probe data doesn't affect MediaFile fingerprints. * feat(ffmpeg): add ProbeAudioStream for authoritative audio metadata Add ProbeAudioStream to FFmpeg interface, using ffprobe to extract codec, profile, bitrate, sample rate, bit depth, and channels. Parse bits_per_raw_sample as fallback for FLAC/ALAC bit depth. Normalize "unknown" profile to empty string. All parseProbeOutput tests use real ffprobe JSON from actual files. * feat(transcoding): integrate ffprobe into transcode decisions Add ensureProbed to probe media files on first transcode decision, caching results in probe_data. Build SourceStream from probe data with fallback to tag-based metadata. Refactor decision logic to pass StreamDetails instead of MediaFile, enabling codec profile limitations (e.g., audioProfile) to use probe data. Add normalizeProbeCodec to map ffprobe codec names (dsd_lsbf_planar, pcm_s16le) to internal names (dsd, pcm). NewDecider now accepts ffmpeg.FFmpeg; wire_gen.go regenerated. * feat(transcoding): add DevEnableMediaFileProbe config flag Add DevEnableMediaFileProbe (default true) to allow disabling ffprobe- based media file probing as a safety fallback. When disabled, the decider uses tag-based metadata from the scanner instead. * test(transcode): add ensureProbed unit tests Test probing when ProbeData is empty, skipping when already set, error propagation from ffprobe, and DevEnableMediaFileProbe flag. * refactor(ffmpeg): use command constant and select_streams for ProbeAudioStream Move ffprobe arguments to a probeAudioStreamCmd constant, following the same pattern as extractImageCmd and probeCmd. Add -select_streams a:0 to only probe the first audio stream, avoiding unnecessary parsing of video and artwork streams. Derive the ffprobe binary path safely using filepath.Dir/Base instead of replacing within the full path string. * refactor(transcode): decouple transcode token claims from auth.Claims Remove six transcode-specific fields (MediaID, DirectPlay, UpdatedAt, Channels, SampleRate, BitDepth) from auth.Claims, which is shared with session and share tokens. Transcode tokens are signed parameter-passing tokens, not authentication tokens, so coupling them to auth created misleading dependencies. The transcode package now owns its own JWT claim serialization via Decision.toClaimsMap() and paramsFromToken(), using generic auth.EncodeToken/DecodeAndVerifyToken wrappers that keep TokenAuth encapsulated. Wire format (JWT claim keys) is unchanged, so in-flight tokens remain compatible. Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcode): simplify code after review Extract getIntClaim helper to eliminate repeated int/int64/float64 JWT claim extraction pattern in paramsFromToken and ClaimsFromToken. Rewrite checkIntLimitation as a one-liner delegating to applyIntLimitation. Return probe result from ensureProbed to avoid redundant JSON round-trip. Extract toResponseStreamDetails helper and mediaTypeSong constant in the API layer, and use transcode.ProtocolHTTP constant instead of hardcoded string. Signed-off-by: Deluan <deluan@navidrome.org> * fix(ffmpeg): enhance bit_rate parsing logic for audio streams Signed-off-by: Deluan <deluan@navidrome.org> * fix(transcode): improve code review findings across transcode implementation - Fix parseProbeData to return nil on JSON unmarshal failure instead of a zero-valued struct, preventing silent degradation of source stream details - Use probe-resolved codec for lossless detection in buildSourceStream instead of the potentially stale scanner data - Remove MediaFile.IsLossless() (dead code) and consolidate lossless detection in isLosslessFormat(), using codec name only — bit depth is not reliable since lossy codecs like ADPCM report non-zero values - Add "wavpack" to lossless codec list (ffprobe codec_name for WavPack) - Guard bpsToKbps against negative input values - Fix misleading comment in buildTemplateArgs about conditional injection - Avoid leaking internal error details in Subsonic API responses - Add missing test for ErrNotFound branch in GetTranscodeDecision - Add TODO for hardcoded protocol in toResponseStreamDetails * refactor(transcode): streamline transcoding command lookup and format resolution Signed-off-by: Deluan <deluan@navidrome.org> * feat(transcode): implement server-side transcoding override for player formats Signed-off-by: Deluan <deluan@navidrome.org> * fix(transcode): honor bit depth and channel constraints in transcoding selection selectTranscodingOptions only checked sample rate when deciding whether same-format transcoding was needed, ignoring requested bit depth and channel reductions. This caused the streamer to return raw audio when the transcode decision requested downmix or bit-depth conversion. * refactor(transcode): unify streaming decision engine via MakeDecision Move transcoding decision-making out of mediaStreamer and into the subsonic Stream/Download handlers, using transcode.Decider.MakeDecision as the single decision engine. This eliminates selectTranscodingOptions and the mismatch between decision and streaming code paths (decision used LookupTranscodeCommand with built-in fallbacks, while streaming used FindByFormat which only checked the DB). - Add DecisionOptions with SkipProbe to MakeDecision so the legacy streaming path never calls ffprobe - Add buildLegacyClientInfo to translate legacy stream params (format, maxBitRate, DefaultDownsamplingFormat) into a synthetic ClientInfo - Add resolveStreamRequest on the subsonic Router to resolve legacy params into a fully specified StreamRequest via MakeDecision - Simplify DoStream to a dumb executor that receives pre-resolved params - Remove selectTranscodingOptions entirely Signed-off-by: Deluan <deluan@navidrome.org> * refactor(transcode): move MediaStreamer into core/transcode and unify StreamRequest Moved MediaStreamer, Stream, TranscodingCache and related types from core/media_streamer.go into core/transcode/, eliminating the duplicate StreamRequest type. The transcode.StreamRequest now carries all fields (ID, Format, BitRate, SampleRate, BitDepth, Channels, Offset) and ResolveStream returns a fully-populated value, removing manual field copying at every call site. Also moved buildLegacyClientInfo into the transcode package alongside ResolveStream, and unexported ParseTranscodeParams since it was only used internally by ValidateTranscodeParams. * refactor(transcode): rename Decider methods and unexport Params type Rename ResolveStream → ResolveRequest and ValidateTranscodeParams → ResolveRequestFromToken for clarity and consistency. The new ResolveRequestFromToken returns a StreamRequest directly (instead of the intermediate Params type), eliminating manual Params→StreamRequest conversion in callers. Unexport Params to params since it is now only used internally for JWT token parsing. * test(transcode): remove redundant tests and use constants Remove tests that duplicate coverage from integration-level tests (toClaimsMap, paramsFromToken round-trips, applyServerOverride direct call, duplicate 410 handler test). Replace raw "http" strings with ProtocolHTTP constant. Consolidate lossy -sample_fmt tests into DescribeTable. * refactor(transcode): split oversized files into focused modules Split transcode.go and transcode_test.go into focused files by concern: - decider.go: decision engine (MakeDecision, direct play/transcode evaluation, probe) - token.go: JWT token encode/decode (params, toClaimsMap, paramsFromToken, CreateTranscodeParams, ResolveRequestFromToken) - legacy_client.go: legacy Subsonic bridge (buildLegacyClientInfo, ResolveRequest) - codec_test.go: isLosslessFormat and normalizeProbeCodec tests - token_test.go: token round-trip and ResolveRequestFromToken tests Moved the Decider interface from types.go to decider.go to keep it near its implementation, and cleaned up types.go to contain only pure type definitions and constants. No public API changes. * refactor(transcode): reorder parameters in applyServerOverride function Signed-off-by: Deluan <deluan@navidrome.org> * test(e2e): add NewTestStream function and implement spyStreamer for testing Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
1ce561cc8e |
refactor(server): remove legacy embedded coverart logic
Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
435fb0b076 |
feat(server): add EnableCoverArtUpload config option
Allow administrators to disable playlist cover art upload/removal for non-admin users via the new EnableCoverArtUpload config option (default: true). - Guard uploadPlaylistImage and deletePlaylistImage endpoints (403 for non-admin when disabled) - Set CoverArtRole in Subsonic GetUser/GetUsers responses based on config and admin status - Pass config to frontend and conditionally hide upload/remove UI controls - Admins always retain upload capability regardless of setting |
||
|
|
acd69f6a4f |
feat(playlist): support #EXTALBUMARTURL directive and sidecar images (#5131)
* feat(playlist): add migration for playlist image field rename and external URL * refactor(playlist): rename ImageFile to UploadedImage and ArtworkPath to UploadedImagePath Rename playlist model fields and methods for clarity in preparation for adding external image URL and sidecar image support. Add the new ExternalImageURL field to the Playlist model. * feat(playlist): parse #EXTALBUMARTURL directive in M3U imports * feat(playlist): always sync ExternalImageURL on re-scan, preserve UploadedImage * feat(artwork): add sidecar image discovery and cache invalidation for playlists Add playlist sidecar image support to the artwork reader fallback chain. A sidecar image (e.g., MyPlaylist.jpg next to MyPlaylist.m3u) is discovered via case-insensitive base name matching using model.IsImageFile(). Cache invalidation uses max(playlist.UpdatedAt, imageFile.ModTime()) to bust stale artwork when sidecar or ExternalImageURL local files change. * feat(artwork): add external image URL source to playlist artwork reader Add fromPlaylistExternalImage source function that resolves playlist cover art from ExternalImageURL, supporting both HTTP(S) URLs (via the existing fromURL helper) and local file paths (via os.Open). Insert it in the Reader() fallback chain between sidecar and tiled cover. * refactor(artwork): simplify playlist artwork source functions Extract shared fromLocalFile helper, use url.Parse for scheme check, and collapse sidecar directory scan conditions. * test(artwork): remove redundant fromPlaylistSidecar tests These tests duplicated scenarios already covered by findPlaylistSidecarPath tests combined with fromLocalFile (tested via fromPlaylistExternalImage). After refactoring fromPlaylistSidecar to a one-liner composing those two functions, the wrapper tests add no value. * fix(playlist): address security review comments from PR #5131: - Use url.PathUnescape instead of url.QueryUnescape for file:// URLs so that '+' in filenames is preserved (not decoded as space). - Validate all local image paths (file://, absolute, relative) against known library boundaries via libraryMatcher, rejecting paths outside any configured library. - Harden #EXTALBUMARTURL against path traversal and SSRF by adding EnableM3UExternalAlbumArt config flag (default false, also disabled by EnableExternalServices=false) to gate HTTP(S) URL storage at parse time and fetching at read time (defense in depth). - Log a warning when os.ReadDir fails in findPlaylistSidecarPath for diagnosability. - Extract resolveLocalPath helper to simplify resolveImageURL. Signed-off-by: Deluan <deluan@navidrome.org> * feat(playlist): implement human-friendly filename generation for uploaded playlist cover images Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
d004f99f8f |
feat(playlist): add custom playlist cover art upload (#5110)
* feat(playlist): add custom playlist cover art upload - #406 Allow users to upload, view, and remove custom cover images for playlists. Custom images take priority over the auto-generated tiled artwork. Backend: - Add `image_path` column to playlist table (migration with proper rollback) - Add `SetImage`/`RemoveImage` methods to playlist service - Add `POST/DELETE /api/playlist/{id}/image` endpoints - Prioritize custom image in artwork reader pipeline - Clean up image files on playlist deletion - Use glob-based cleanup to prevent orphaned files across format changes - Reject uploads with undetermined image type (400) Frontend: - Hover overlay on playlist cover with upload (camera) and remove (trash) buttons - Lightbox for full-size cover art viewing - Cover art thumbnails in the playlist list view - Loading/error states and i18n strings Closes #406 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: adrbn <128328324+adrbn@users.noreply.github.com> * refactor: rename playlist image path migration file Signed-off-by: Deluan <deluan@navidrome.org> * fix(playlist): address review feedback for cover art upload - #406 - Use httpClient instead of raw fetch for image upload/remove - Revert glob cleanup to simple imagePath check - Add log.Error before all error HTTP responses - Add backend tests for SetImage and RemoveImage Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: adrbn <128328324+adrbn@users.noreply.github.com> * refactor(playlist): use Playlist.ArtworkPath() for image storage Migrate all playlist image path handling to use the new Playlist.ArtworkPath() method as the single source of truth. The DB now stores only the filename (e.g. "pls-1.jpg") instead of a relative path, and images are stored under {DataFolder}/artwork/playlist/ instead of {DataFolder}/playlist_images/. The artwork root directory is created at startup alongside DataFolder and CacheFolder. This also removes the conf dependency from reader_playlist.go since path resolution is now fully encapsulated in the model. Signed-off-by: Deluan <deluan@navidrome.org> * refactor(playlist): streamline artwork image selection logic Signed-off-by: Deluan <deluan@navidrome.org> * refactor: move translation keys, add pt-BR translations Signed-off-by: Deluan <deluan@navidrome.org> * refactor(playlist): rename image_path to image_file Rename the playlist cover art column and field from image_path/ImagePath to image_file/ImageFile across the migration, model, service, tests, and UI. The new name more accurately describes what the field stores (a filename, not a path) and aligns with the existing ImageFiles/IsImageFile naming conventions in the codebase. --------- Signed-off-by: adrbn <128328324+adrbn@users.noreply.github.com> Signed-off-by: Deluan <deluan@navidrome.org> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Deluan Quintão <deluan@navidrome.org> |
||
|
|
5bc2bbb70e |
feat(subsonic): append album version to names in Subsonic API (#5111)
* feat(subsonic): append album version to album names in Subsonic API responses Add AppendAlbumVersion config option (default: true) that appends the album version tag to album names in Subsonic API responses, similar to how AppendSubtitle works for track titles. This affects album names in childFromAlbum and buildAlbumID3 responses. Signed-off-by: Deluan <deluan@navidrome.org> * feat(subsonic): append album version to media file album names in Subsonic API Add FullAlbumName() to MediaFile that appends the album version tag, mirroring the Album.FullName() behavior. Use it in childFromMediaFile and fakePath to ensure media file responses also show the album version. Signed-off-by: Deluan <deluan@navidrome.org> * fix(subsonic): use len() check for album version tag to prevent panic on empty slice Use len(tags) > 0 instead of != nil to safely guard against empty slices when accessing the first element of the album version tag. Signed-off-by: Deluan <deluan@navidrome.org> * fix(subsonic): use FullName in buildAlbumDirectory and deduplicate FullName calls Apply album.FullName() in buildAlbumDirectory (getMusicDirectory) so album names are consistent across all Subsonic endpoints. Also compute al.FullName() once in childFromAlbum to avoid redundant calls. Signed-off-by: Deluan <deluan@navidrome.org> * fix: use len() check in MediaFile.FullTitle() to prevent panic on empty slice Apply the same safety improvement as FullAlbumName() and Album.FullName() for consistency. Signed-off-by: Deluan <deluan@navidrome.org> * test: add tests for Album.FullName, MediaFile.FullTitle, and MediaFile.FullAlbumName Cover all cases: config enabled/disabled, tag present, tag absent, and empty tag slice. Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
2bb13e5ff1 |
feat(server): add ExtAuth logout URL configuration (#5074)
* feat(server): add ExtAuth logout URL configuration (#4467) When external authentication (reverse proxy auth) is active, the Logout button is hidden because authentication is managed externally. Many external auth services (Authelia, Authentik, Keycloak) provide a logout URL that can terminate the session. Add `ExtAuth.LogoutURL` config option that, when set, shows the Logout button in the UI and redirects the user to the external auth provider's logout endpoint instead of the Navidrome login page. * feat(server): add validation for ExtAuth logout URL configuration * feat(server): refactor ExtAuth logout URL validation to a reusable function * fix(configuration): rename URL validation functions for consistency Signed-off-by: Deluan <deluan@navidrome.org> * fix(configuration): rename URL validation functions for consistency Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
54de0dbc52 |
feat(server): implement FTS5-based full-text search (#5079)
* build: add sqlite_fts5 build tag to enable FTS5 support
* feat: add SearchBackend config option (default: fts)
* feat: add buildFTS5Query for safe FTS5 query preprocessing
* feat: add FTS5 search backend with config toggle, refactor legacy search
- Add searchExprFunc type and getSearchExpr() for backend selection
- Rename fullTextExpr to legacySearchExpr
- Add ftsSearchExpr using FTS5 MATCH subquery
- Update fullTextFilter in sql_restful.go to use configured backend
* feat: add FTS5 migration with virtual tables, triggers, and search_participants
Creates FTS5 virtual tables for media_file, album, and artist with
unicode61 tokenizer and diacritic folding. Adds search_participants
column, populates from JSON, and sets up INSERT/UPDATE/DELETE triggers.
* feat: populate search_participants in PostMapArgs for FTS5 indexing
* test: add FTS5 search integration tests
* fix: exclude FTS5 virtual tables from e2e DB restore
The restoreDB function iterates all tables in sqlite_master and
runs DELETE + INSERT to reset state. FTS5 contentless virtual tables
cannot be directly deleted from. Since triggers handle FTS5 sync
automatically, simply skip tables matching *_fts and *_fts_* patterns.
* build: add compile-time guard for sqlite_fts5 build tag
Same pattern as netgo: compilation fails with a clear error if
the sqlite_fts5 build tag is missing.
* build: add sqlite_fts5 tag to reflex dev server config
* build: extract GO_BUILD_TAGS variable in Makefile to avoid duplication
* fix: strip leading * from FTS5 queries to prevent "unknown special query" error
* feat: auto-append prefix wildcard to FTS5 search tokens for broader matching
Every plain search token now gets a trailing * appended (e.g., "love" becomes
"love*"), so searching for "love" also matches "lovelace", "lovely", etc.
Quoted phrases are preserved as exact matches without wildcards. Results are
ordered alphabetically by name/title, so shorter exact matches naturally
appear first.
* fix: clarify comments about FTS5 operator neutralization
The comments said "strip" but the code lowercases operators to
neutralize them (FTS5 operators are case-sensitive). Updated comments
to accurately describe the behavior.
* fix: use fmt.Sprintf for FTS5 phrase placeholders
The previous encoding used rune('0'+index) which silently breaks with
10+ quoted phrases. Use fmt.Sprintf for arbitrary index support.
* fix: validate and normalize SearchBackend config option
Normalize the value to lowercase and fall back to "fts" with a log
warning for unrecognized values. This prevents silent misconfiguration
from typos like "FTS", "Legacy", or "fts5".
* refactor: improve documentation for build tags and FTS5 requirements
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: convert FTS5 query and search backend normalization tests to DescribeTable format
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: add sqlite_fts5 build tag to golangci configuration
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: add UISearchDebounceMs configuration option and update related components
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: fall back to legacy search when SearchFullString is enabled
FTS5 is token-based and cannot match substrings within words, so
getSearchExpr now returns legacySearchExpr when SearchFullString
is true, regardless of SearchBackend setting.
* fix: add sqlite_fts5 build tag to CI pipeline and Dockerfile
* fix: add WHEN clauses to FTS5 AFTER UPDATE triggers
Added WHEN clauses to the media_file_fts_au, album_fts_au, and
artist_fts_au triggers so they only fire when FTS-indexed columns
actually change. Previously, every row update (e.g., play count, rating,
starred status) triggered an unnecessary delete+insert cycle in the FTS
shadow tables. The WHEN clauses use IS NOT for NULL-safe comparison of
each indexed column, avoiding FTS index churn for non-indexed updates.
* feat: add SearchBackend configuration option to data and insights components
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: enhance input sanitization for FTS5 by stripping additional punctuation and special characters
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: add search_normalized column for punctuated name search (R.E.M., AC/DC)
Add index-time normalization and query-time single-letter collapsing to
fix FTS5 search for punctuated names. A new search_normalized column
stores concatenated forms of punctuated words (e.g., "R.E.M." → "REM",
"AC/DC" → "ACDC") and is indexed in FTS5 tables. At query time, runs of
consecutive single letters (from dot-stripping) are collapsed into OR
expressions like ("R E M" OR REM*) to match both the original tokens and
the normalized form. This enables searching by "R.E.M.", "REM", "AC/DC",
"ACDC", "A-ha", or "Aha" and finding the correct results.
* refactor: simplify isSingleUnicodeLetter to avoid []rune allocation
Use utf8.DecodeRuneInString to check for a single Unicode letter
instead of converting the entire string to a []rune slice.
* feat: define ftsSearchColumns for flexible FTS5 search column inclusion
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: update collapseSingleLetterRuns to return quoted phrases for abbreviations
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: implement extractPunctuatedWords to handle artist/album names with embedded punctuation
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: implement extractPunctuatedWords to handle artist/album names with embedded punctuation
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: punctuated word handling to improve processing of artist/album names
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: add CJK support for search queries with LIKE filters
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: enhance FTS5 search by adding album version support and CJK handling
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor: search configuration to use structured options
Signed-off-by: Deluan <deluan@navidrome.org>
* feat: enhance search functionality to support punctuation-only queries and update related tests
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
|
||
|
|
5fa8356b31 |
chore(deps): bump golangci-lint to v2.10.0 and suppress new gosec false positives
Bump golangci-lint from v2.9.0 to v2.10.0, which includes a newer gosec with additional taint-analysis rules (G117, G703, G704, G705) and a stricter G101 check. Added inline //nolint:gosec comments to suppress 21 false positives across 19 files: struct fields flagged as secrets (G117), w.Write calls flagged as XSS (G705), HTTP client calls flagged as SSRF (G704), os.Stat/os.ReadFile/os.Remove flagged as path traversal (G703), and a sort mapping flagged as hardcoded credentials (G101). Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
a704e86ac1 | refactor: run Go modernize (#5002) | ||
|
|
1e37e680d7 |
feat(agents): Add artist url and top and similar songs to ListenBrainz agent (#4934)
* feat(agents): Add artist url and top songs to ListenBrainz agent * add newline at end of file * respond to some feedback * add more tests, include more metadata in top songs * add duration to album info * add similar artists from labs * add similar artists and track radio * fix(client): replace sort with slices.SortFunc for deterministic ordering of recordings with same score Signed-off-by: Deluan <deluan@navidrome.org> * fix: typos Signed-off-by: Deluan <deluan@navidrome.org> * refactor: use struct literal initialization consistently Signed-off-by: Deluan <deluan@navidrome.org> * feat: configurable artist and track algorithms Signed-off-by: Deluan <deluan@navidrome.org> * test configuration changes --------- Signed-off-by: Deluan <deluan@navidrome.org> Co-authored-by: Deluan Quintão <deluan@navidrome.org> Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
2829cec0ce |
fix(subsonic): add SubMusic to default MinimalClients list
Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
7b523d6b61 |
feat(agents): support multiple languages for Last.fm and Deezer metadata (#4952)
* feat(lastfm): support multiple languages for album and artist info retrieval Signed-off-by: Deluan <deluan@navidrome.org> * fix(lastfm): improve content validation for album and artist descriptions Signed-off-by: Deluan <deluan@navidrome.org> * refactor(lastfm): remove single language test and clarify languages field in configuration Signed-off-by: Deluan <deluan@navidrome.org> * feat(deezer): support multiple languages for artist bio retrieval Signed-off-by: Deluan <deluan@navidrome.org> * refactor(lastfm): rename ignoredBiographies to ignoredContent for clarity Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
c9e58e3666 |
feat: enable plugins by default in configuration settings
Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
71f549afbf |
fix(configuration): ensure default PIDs are set for Album and Track
Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
772d1f359b |
feat: add similar songs functionality in agents, and Instant Mix (song-based) to UI (#4919)
* refactor: rename ArtistRadio to SimilarSongs for clarity and consistency Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement GetSimilarSongsByTrack and related functionality for song similarity retrieval Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance GetSimilarSongsByTrack to include artist and album details and update tests Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance song matching by implementing title and artist filtering in loadTracksByTitleAndArtist Signed-off-by: Deluan <deluan@navidrome.org> * test: add unit tests for song matching functionality in provider Signed-off-by: Deluan <deluan@navidrome.org> * refactor: extract song matching functionality into its own file Signed-off-by: Deluan <deluan@navidrome.org> * docs: clarify similarSongsFallback function description in provider.go Signed-off-by: Deluan <deluan@navidrome.org> * refactor: initialize result slice for songs with capacity based on response length Signed-off-by: Deluan <deluan@navidrome.org> * refactor: simplify agent method calls for retrieving images and similar songs Signed-off-by: Deluan <deluan@navidrome.org> * refactor: simplify agent method calls for retrieving images and similar songs Signed-off-by: Deluan <deluan@navidrome.org> * refactor: remove outdated comments in GetSimilarSongs methods Signed-off-by: Deluan <deluan@navidrome.org> * fix: use composite key for song matches to handle duplicates by title and artist Signed-off-by: Deluan <deluan@navidrome.org> * refactor: consolidate expectations setup for similar songs tests Signed-off-by: Deluan <deluan@navidrome.org> * feat: add instant mix action to song context menu and update translations Signed-off-by: Deluan <deluan@navidrome.org> * fix(provider): handle unknown entity types in GetSimilarSongs Signed-off-by: Deluan <deluan@navidrome.org> * refactor: move playSimilar action to playbackActions and streamline song processing Signed-off-by: Deluan <deluan@navidrome.org> * format Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance instant mix functionality with loading notification and shuffle option Signed-off-by: Deluan <deluan@navidrome.org> * feat: implement fuzzy matching for similar songs based on configurable threshold Signed-off-by: Deluan <deluan@navidrome.org> * refactor: implement track matching with multiple specificity levels Signed-off-by: Deluan <deluan@navidrome.org> * refactor: enhance track matching by implementing unified scoring with specificity levels Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance deezer top tracks result with album Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance track matching with fuzzy album similarity for improved scoring Signed-off-by: Deluan <deluan@navidrome.org> * docs: document multi-phase song matching algorithm with detailed scoring and prioritization Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
66474fc9f4 |
feat: add support for reading embedded images using taglib by default
Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
03120bac32 |
feat(subsonic): Add avgRating from subsonic spec (#4900)
* feat(subsonic): add averageRating to API responses Add averageRating attribute to Subsonic API responses for artists, albums, and songs. The average is calculated across all user ratings. * perf(db): add index for average rating queries Add composite index on (item_id, item_type, rating) to optimize the correlated subquery used for calculating average ratings. Signed-off-by: Terry Raimondo <terry.raimondo@gmail.com> * test: add tests for averageRating feature Add tests for: - Album.AverageRating calculation in persistence layer - MediaFile.AverageRating calculation in persistence layer - AverageRating mapping in subsonic response helpers Signed-off-by: Terry Raimondo <terry.raimondo@gmail.com> * test: improve averageRating rounding test with 3 users Add third test user to fixtures and update rounding test to use 3 ratings (5 + 4 + 4) / 3 = 4.33 for proper decimal rounding coverage. Signed-off-by: Terry Raimondo <terry.raimondo@gmail.com> * perf: store avg_rating on entity tables instead of using subquery - Add avg_rating column to album, media_file, and artist tables - Update SetRating() to recalculate and store average when ratings change - Read avg_rating directly from entity table in withAnnotation() - Remove old annotation index migration (no longer needed) This trades write-time computation for read-time performance by pre-computing the average rating instead of using a correlated subquery on every read. * feat: add Subsonic.EnableAverageRating config option (default true) Allow administrators to disable exposing averageRating in Subsonic API responses if they don't want to expose other users' rating data. The avg_rating column is still updated internally when users rate items, but the value is only included in API responses when this option is enabled. * address PR comments - Use structs:"avg_rating" with db:"avg_rating" tag instead of SQL alias - Remove avg_rating indexes (not needed) - Populate avg_rating columns from existing ratings in migration * Woops * rename avg_rating column to average_rating --------- Signed-off-by: Terry Raimondo <terry.raimondo@gmail.com> |
||
|
|
2de2484bca |
feat: add go-taglib pure Go metadata extractor (#4902)
* feat: implement go-taglib extractor Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance ID3v2 frame parsing for language-specific lyrics Signed-off-by: Deluan <deluan@navidrome.org> * feat: add support for reading iTunes-specific tags from M4A files Signed-off-by: Deluan <deluan@navidrome.org> * feat: expose BitDepth in AudioProperties struct Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance WMA tag parsing by adding support for ASF attributes Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance ID3v2 frame parsing for WAV and AIFF formats to support language codes Signed-off-by: Deluan <deluan@navidrome.org> * chore: usa a ignored go.work for local dependency management * feat: optimize metadata extraction by consolidating file reads and improving tag processing Signed-off-by: Deluan <deluan@navidrome.org> * remove comment Signed-off-by: Deluan <deluan@navidrome.org> * feat: improve language code extraction for lyrics tags in metadata processing Signed-off-by: Deluan <deluan@navidrome.org> * address PR comments Signed-off-by: Deluan <deluan@navidrome.org> * chore: remove outdated comments in gotaglib.go Signed-off-by: Deluan <deluan@navidrome.org> * feat: enhance extractor to utilize filesystem for file handling Signed-off-by: Deluan <deluan@navidrome.org> * chore: update go-taglib dependency version in go.mod and go.sum Signed-off-by: Deluan <deluan@navidrome.org> * feat: make new go-taglib extractor default Signed-off-by: Deluan <deluan@navidrome.org> * chore: formatting Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> |
||
|
|
9ab0c2dc67 |
feat: new "Subsonic Minimal Clients" configuration option (#4850)
* Add `.editorconfig` file Hints to users how to properly indent Go files (my setup was defaulting to 2 spaces). * Add Subsonic API minimal config option This will allow users to specify clients which can operate with or need the minimum required fields as per the [SubSonic API spec](https://subsonic.org/pages/api.jsp). * Return only required fields for Child Objects For a minimal client, only return the required fields for Child Objects. * Return only required fields for Playlist objects * refactor: simplify client list checks and improve playlist response handling Signed-off-by: Deluan <deluan@navidrome.org> * test: add unit tests for client list checks and playlist building logic Signed-off-by: Deluan <deluan@navidrome.org> * fix: revert Child.IsVideo and Playlist.Public fields from pointer to boolean, and add omitempty to XML tag Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> Co-authored-by: Deluan Quintão <deluan@navidrome.org> |