* refactor: extract matchSongsToLibrary to core/matcher package
Move the song-to-library matching algorithm from core/external into its own
core/matcher package. The Matcher struct exposes a single public method
MatchSongsToLibrary that implements a multi-phase matching algorithm
(ID > MBID > ISRC > fuzzy title+artist). Includes pre-sanitization
optimization for the fuzzy matching loop.
No behavioral changes — the algorithm is identical to the version in
core/external/provider_matching.go.
* refactor: inject matcher.Matcher via Wire instead of creating it inline
Add *matcher.Matcher as a dependency of external.NewProvider, wired via
Google Wire. Update all provider test files to pass matcher.New(ds).
This eliminates tight coupling so future consumers can reuse the matcher
without depending on the external package.
* refactor: remove old provider_matching files
Delete core/external/provider_matching.go and its tests. All matching
logic now lives in core/matcher/.
* test(matcher): restore test coverage lost in extraction
Port back 23 specs that existed in the old provider_matching_test.go
but were dropped during the extraction. Covers specificity levels,
fuzzy matching thresholds, fuzzy album matching, duration matching,
and deduplication edge cases.
* test(matcher): extract matchFieldInAnd/matchFieldInEq helpers
The four inline mock.MatchedBy closures in setupAllPhaseExpectations
all followed the same squirrel.And -> squirrel.Eq -> field-name-check
pattern. Extract into two small helpers to reduce duplication and
make the setup functions read as a concise list of phase expectations.
* refactor(matcher): address PR #5348 review feedback
- sanitizedTrack now holds *model.MediaFile instead of a value copy. Since
MediaFile is a large struct (~74 fields), this avoids the per-track copy
into sanitized[] and a second copy when findBestMatch assigns the winner.
loadTracksByTitleAndArtist updated to iterate by index and pass &tracks[i].
- loadTracksByISRC now sorts results (starred desc, rating desc, year asc,
compilation asc) so that when multiple library tracks share an ISRC the
most relevant one is picked deterministically, matching the sort order
already used by loadTracksByTitleAndArtist.
- Restored the four worked examples (MBID Priority, ISRC Priority,
Specificity Ranking, Fuzzy Title Matching) in the MatchSongsToLibrary
godoc that were dropped during the extraction.
- matcher_test.go: tests now enforce expectations via AssertExpectations in
a DeferCleanup. The old setupAllPhaseExpectations helper was replaced
with per-phase helpers (expectIDPhase/expectMBIDPhase/expectISRCPhase +
allowOtherPhases) so each test deterministically verifies which matching
phases fire. This surfaced (and fixes) a latent issue copilot flagged:
the old .Once() expectations were not actually asserted, so tests would
silently pass even when phases short-circuited unexpectedly.
* 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>
* fix(transcoding): clamp target channels to codec limit (#5336)
When transcoding a multi-channel source (e.g. 6-channel FLAC) to MP3, the
decider passed the source channel count through to ffmpeg unchanged. The
default MP3 command path then emitted `-ac 6`, and the template path injected
`-ac 6` after the template's own `-ac 2`, causing ffmpeg to honor the last
occurrence and fail with exit code 234 since libmp3lame only supports up to
2 channels.
Introduce `codecMaxChannels()` in core/stream/codec.go (mp3→2, opus→8),
mirroring the existing `codecMaxSampleRate` pattern, and apply the clamp in
`computeTranscodedStream` right after the sample-rate clamps. Also fix a
pre-existing ordering bug where the profile's MaxAudioChannels check compared
against src.Channels rather than ts.Channels, which would have let a looser
profile setting raise the codec-clamped value back up. Comparing against the
already-clamped ts.Channels makes profile limits strictly narrowing, which
matches how the sample-rate block already behaves.
The ffmpeg buildTemplateArgs comment is refreshed to point at the new upstream
clamp, since the flags it injects are now always codec-safe.
Adds unit tests for codecMaxChannels and four decider scenarios covering the
literal issue repro (6-ch FLAC→MP3 clamps to 2), a stricter profile limit
winning over the codec clamp, a looser profile limit leaving the codec clamp
intact, and a codec with no hard limit (AAC) passing 6 channels through.
* test(e2e): pin codec channel clamp at the Subsonic API surface (#5336)
Add a 6-channel FLAC fixture to the e2e test suite and use it to assert the
codec channel clamp end-to-end on both Subsonic streaming endpoints:
- getTranscodeDecision (mp3OnlyClient, no MaxAudioChannels in profile):
expects TranscodeStream.AudioChannels == 2 for the 6-channel source. This
exercises the new codecMaxChannels() helper through the OpenSubsonic
decision endpoint, with no profile-level channel limit masking the bug.
- /rest/stream (legacy): requests format=mp3 against the multichannel
fixture and asserts streamerSpy.LastRequest.Channels == 2, confirming
the clamp propagates through ResolveRequest into the stream.Request that
the streamer receives.
The fixture is metadata-only (channels: 6 plumbed via the existing
storagetest.File helper) — no real audio bytes required, since the e2e
suite uses a spy streamer rather than invoking ffmpeg. Bumps the empty-query
search3 song count expectation from 13 to 14 to account for the new fixture.
* test(decider): clarify codec-clamp comment terminology
Distinguish "transcoding profile MaxAudioChannels" (Profile.MaxAudioChannels
field) from "LimitationAudioChannels" (CodecProfile rule constant). The
regression test bypasses the former, not the latter.
* test(artwork): expect shared disc art for unnumbered filenames in single-folder albums
* fix(artwork): match unnumbered disc art for every disc in single-folder albums
* test(artwork): verify shared disc art resolves for every disc number
* test(artwork): regression guard for numbered disc filter with mixed filenames
* test(artwork): verify DiscArtPriority order decides numbered vs shared disc art
* test(artwork): strengthen regression guard to exercise both disc art branches
* refactor(artwork): simplify disc art matching and drop redundant comments
- Lowercase the pattern and filename once in fromExternalFile and pass
lowered values into extractDiscNumber, eliminating the duplicate
strings.ToLower calls inside that helper.
- Drop narrating comments in reader_disc.go and reader_disc_test.go that
duplicated information already conveyed by nearby code or doc comments.
* fix(artwork): prefer numbered disc art over shared fallback within a pattern
Review feedback: with files [disc.jpg, disc1.jpg, disc2.jpg] in a single
folder, the previous single-folder fall-through returned the first match
in imgFiles order. Because compareImageFiles sorts 'disc' before 'disc1'
and 'disc2', disc.jpg would mask the per-disc numbered files for every
disc, regressing the behavior from before the shared-disc-art change.
Within a single pattern the loop now records the first viable unnumbered
candidate as a fallback and keeps scanning for a numbered match equal to
the target disc. Numbered matches still win immediately; the shared file
is only returned when no numbered match for the target disc exists.
Also drops the redundant strings.ToLower(pattern) at the top of
fromExternalFile; fromDiscArtPriority already lowercases the whole
priority string before splitting, so the function contract is now
'pattern must be lowercase' (documented on the function).
* refactor(artwork): trim disc art matching comments and table-drive tests
Doc comment on fromExternalFile is trimmed to the one non-obvious
contract (caller must pre-lowercase the pattern) plus the headline
behavior; the bulleted restatement of the branch logic went away.
Two inline comments that narrated what the code already shows are
also gone.
Hoisting a `hasWildcard := strings.ContainsRune(pattern, '*')` check
out of the loop avoids per-iteration extractDiscNumber calls for
literal patterns (e.g. `shellac.png`) and lets the loop break as
soon as a viable fallback is found, since literal patterns can never
be beaten by a numbered match. Wildcard patterns keep the original
scan-to-end-for-numbered-match behavior.
The two regression tests added in the previous commit were
structurally identical apart from discNumber/expected, so they are
collapsed into a DescribeTable with two entries — matching the
existing table style used for extractDiscNumber tests in the same
file.
* fix(artwork): support '?' and '[...]' wildcards in disc art patterns
filepath.Match understands three glob metacharacters ('*', '?', '[')
but extractDiscNumber only looked for '*'. A pattern like 'disc?.jpg'
or 'cd[12].jpg' would therefore be treated as unnumbered, and every
disc of a multi-disc album would resolve to the same (first-sorted)
file instead of the per-disc numbered art.
extractDiscNumber now finds the literal prefix of the pattern by
scanning for the first '*', '?', or '[' (via strings.IndexAny),
strips it from the filename, and parses the leading digits that
follow. The standalone filepath.Match check is dropped; HasPrefix
plus the leading-digits requirement is enough to reject non-matches,
and the caller already verifies the glob match before calling.
fromExternalFile's literal-pattern optimization is widened
correspondingly: a pattern is treated as literal only when it
contains none of '*', '?', '['. Any wildcard form now keeps the
scan-to-end behavior so a numbered match can beat a fallback.
Adds table entries for both the extractDiscNumber parser and the
fromExternalFile higher-level behavior, covering '?' and '[...]'
patterns as well as a literal-pattern baseline.
* refactor(artwork): tidy extractDiscNumber after glob-wildcard support
- Name the '*?[' charset as globMetaChars, used by both extractDiscNumber
and fromExternalFile so the two call sites can't drift.
- Trim the extractDiscNumber doc comment: keep the non-obvious caller
contract, drop the algorithm narration.
- Replace the byte-slice digit accumulator with a direct filename slice
fed to strconv.Atoi.
- Rename the four new non-'*' wildcard Entry descriptions so they read
like the existing extractDiscNumber table ('pattern, target → expected')
instead of the ambiguous 'disc 1' shorthand.
* fix(artwork): retry remaining fallbacks when the first one fails to open
Review feedback: the previous shape remembered only the first unnumbered
candidate and fell through to a generic error if os.Open failed on it,
even though other matching unnumbered files in imgFiles could have
succeeded. The pre-PR code was more resilient because it looped and
continued on open failure.
fromExternalFile now collects every viable unnumbered candidate into a
slice during the scan, then tries them in order after the loop, mirroring
the pre-PR retry-on-open-failure behavior. Numbered matches still return
immediately on first success and skip the candidate list entirely — an
open failure on a numbered match means no other file has that number
anyway.
Also:
- globMetaChars doc comment now notes that '\' escape is intentionally
excluded (filepath.Match supports it but treating it as a metachar here
would misalign extractDiscNumber's literal-prefix extraction with no
benefit for realistic config patterns).
- The 'cover.jpg doesn't match disc*.*' Entry in the extractDiscNumber
table is renamed to 'cover.jpg with disc*.* (no prefix match)' to
reflect that the test now exercises the HasPrefix defensive guard,
not the removed internal filepath.Match check.
Regression test added: a single-folder album with a deleted first
candidate file resolves to the second candidate.
* fix(artwork): scan all literal-pattern matches so fallback retry works
Review feedback: the 'break on first literal match' optimization
assumed only one file in imgFiles could match a literal basename,
but filepath.Match compares basenames only — multiple folders can
contribute files with the same basename, and the fallback-list retry
in 5d79f751c is defeated if the loop breaks after recording just
the first one.
Removing the break makes literal and wildcard patterns follow the
same scan-to-end path, preserving the retry-on-open-failure
resilience regained in 5d79f751c. The efficiency cost is negligible
— imgFiles is 5-20 entries per album and this is a cache-miss path.
The error-check ordering after backupOp.Step(-1) checked !done before
err, which masked the underlying SQLite error (e.g. SQLITE_BUSY, I/O
errors) with a generic "backup not done with step -1" message. On
failure, Step returns done=false together with a non-nil err, so the
!done branch short-circuited before the real error was ever reported.
Swap the checks so the SQLite error is returned first, making failing
backups actually diagnosable.
Refs https://github.com/navidrome/navidrome/issues/5305#issuecomment-4230470593
* perf(subsonic): keep album/mediafile params on stack in response helpers
Two helpers were forcing their entire value parameter onto the heap via
pointer-to-field aliasing, adding one full-struct heap allocation per
response item on hot Subsonic endpoints (search3, getAlbumList2, etc.).
- childFromMediaFile assigned &mf.BirthTime to the returned Child,
pulling the whole ~1KB model.MediaFile to the heap on every call.
- buildDiscSubtitles passed &a.UpdatedAt to NewArtworkID inside a loop,
pulling the whole model.Album to the heap on every album with discs.
Both now copy the time.Time to a stack-local and use gg.P / &local so
only the small time.Time escapes. Verified via go build -gcflags=-m=2:
moved to heap: mf and moved to heap: a are gone at these sites.
* perf(metadata): avoid per-track closure allocations in PID computation
createGetPID was a factory that returned nested closures capturing
mf model.MediaFile (~992 bytes) by reference. Since it is called three
times per track during scans (trackPID, albumID, artistID), every track
triggered the allocation of three closures plus a heap copy of the full
MediaFile.
Refactor the body into package-level functions (computePID, getPIDAttr)
that take hash as an explicit parameter and the inner slice.Map callback
to an indexed for loop, removing the closure-capture of mf entirely.
trackPID/albumID/artistID now call computePID directly.
The tiny createGetPID wrapper was kept only for tests; move the
closure-building into the test file so production has no dead API.
Verified via go build -gcflags=-m=2 on model/metadata: no
"moved to heap: mf" anywhere in persistent_ids.go, and the callers in
map_mediafile.go / map_participants.go no longer heap-promote their
MediaFile argument.
* fix(subsonic): always emit required `created` field on AlbumID3
Strict OpenSubsonic clients (e.g. Navic via dev.zt64.subsonic) reject
search3/getAlbum/getAlbumList2 responses that omit the `created` field,
which the spec marks as required. Navidrome was dropping it whenever
the album's CreatedAt was zero.
Root cause was threefold:
1. buildAlbumID3/childFromAlbum conditionally emitted `created`, so a
zero CreatedAt became a missing JSON key.
2. ToAlbum's `older()` helper treated a zero BirthTime as the minimum,
so a single track with missing filesystem birth time could poison
the album aggregation.
3. phase_1_folders' CopyAttributes copied `created_at` from the previous
album row unconditionally, propagating an already-zero value forward
on every metadata-driven album ID change. Since sql_base_repository
drops `created_at` on UPDATE, a poisoned row could never self-heal.
Fixes:
- Always emit `created`, falling back to UpdatedAt/ImportedAt when
CreatedAt is zero. Adds albumCreatedAt() helper used by both
buildAlbumID3 and childFromAlbum.
- Guard `older()` against a zero second argument.
- Skip the CopyAttributes call in phase_1_folders when the previous
album's created_at is zero, so the freshly-computed value survives.
- New migration backfills existing broken rows from media_file.birth_time
(falling back to updated_at).
Tested against a real DB: repaired 605/6922 affected rows, no side
effects on healthy rows.
Signed-off-by: Deluan <deluan@navidrome.org>
* refactor(subsonic): return albumCreatedAt by value to avoid heap escape
Returning *time.Time from albumCreatedAt caused Go escape analysis to
move the entire model.Album parameter to the heap, since the returned
pointer aliased a field of the value receiver. For hot endpoints like
getAlbumList2 and search3, this meant one full-struct heap allocation
per album result.
Return time.Time by value and let callers wrap it with gg.P() to take
the address locally. Only the small time.Time value escapes; the
model.Album struct stays on the stack. Also corrects the doc comment
to reflect the actual guarantee ("best-effort" rather than "non-zero"),
matching the test case that exercises the all-zero fallback.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(msi): include ffprobe executable in MSI build
Signed-off-by: Deluan <deluan@navidrome.org>
* feat(ffmpeg): add IsProbeAvailable() to FFmpeg interface
Add runtime check for ffprobe binary availability with cached result
and startup logging. When ffprobe is missing, logs a warning at startup.
* feat(stream): guard MakeDecision behind ffprobe availability
When ffprobe is not available, MakeDecision returns a decision with
ErrorReason set and both CanDirectPlay and CanTranscode false, instead
of failing with an opaque exec error.
* feat(subsonic): only advertise transcoding extension when ffprobe is available
The OpenSubsonic transcoding extension is now conditionally included
based on ffprobe availability, so clients know not to call
getTranscodeDecision when ffprobe is missing.
* refactor(ffmpeg): move ffprobe startup warning to initial_setup
Move the ffprobe availability warning from the lazy IsProbeAvailable()
check to checkFFmpegInstallation() in server/initial_setup.go, alongside
the existing ffmpeg warning. This ensures the warning appears at startup
rather than on first endpoint call.
* fix(e2e): set noopFFmpeg.IsProbeAvailable to true
The e2e tests use pre-populated probe data and don't need a real ffprobe
binary. Setting IsProbeAvailable to true allows the transcode decision
logic to proceed normally in e2e tests.
* fix(stream): only guard on ffprobe when probing is needed
Move the IsProbeAvailable() guard inside the SkipProbe check so that
legacy stream requests (which pass SkipProbe: true) are not blocked
when ffprobe is missing. The guard only applies when probing is
actually required (i.e., getTranscodeDecision endpoint).
* refactor(stream): fall back to tag metadata when ffprobe is unavailable
Instead of blocking getTranscodeDecision when ffprobe is missing,
fall back to tag-based metadata (same behavior as /rest/stream).
The transcoding extension is always advertised. A startup warning
still alerts admins when ffprobe is not found.
* fix(stream): downgrade ffprobe-unavailable log to Debug
Avoids log spam when clients call getTranscodeDecision repeatedly
without ffprobe installed. The startup warning in initial_setup.go
already alerts admins at Warn level.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
The cleanupLoop goroutine could execute cleanupExpired against a closed
database because Close() did not wait for the goroutine to exit before
calling db.Close(). This caused 'sql: database is closed' errors during
plugin unload or shutdown.
Close() now cancels the cleanup goroutine's context and waits for it to
finish via a sync.WaitGroup before running the final cleanup and closing
the database.
Signed-off-by: Deluan <deluan@navidrome.org>
Replace log.Fatal with a graceful fallback when the configured scanner
extractor is not found. Instead of terminating the process, the code now
warns and falls back to the default taglib extractor using the existing
consts.DefaultScannerExtractor constant. A fatal log is retained only for
the case where the default extractor itself is not registered, which
indicates a broken build.
The Squiddies Glass theme applies a CSS color filter to all images inside
table cells (MuiTableCell '& img'), which was intended for small playback
indicator icons. This inadvertently also applied to disc cover art
thumbnails in multi-disc album views, turning them into solid color
blocks. Adding 'filter: none !important' to the discCoverArt style
ensures cover art images are always displayed correctly regardless of
the active theme.
Signed-off-by: Deluan <deluan@navidrome.org>
Avoid hardcoding SONAME versions (.so.7, .so.2, .so.3) which break
on Alpine version bumps. Also fix misleading comment: the musl build
is dynamic (required for purego dlopen), not static.
Remove the CGO-based TagLib adapter (adapters/taglib/) and all
cross-taglib build infrastructure. The WASM-based go-taglib adapter
(adapters/gotaglib/) is now the sole metadata extractor.
Add a musl-based build stage (build-alpine) to the Dockerfile using
xx for cross-compilation. This produces a dynamically-linked musl
binary for the Docker image, which allows purego to dlopen native
libwebp at runtime. The glibc build stage is kept for standalone
binary distribution.
The Docker image now bundles libwebp for native WebP encoding,
with automatic fallback to the built-in WASM encoder if unavailable.
* fix: allow WAV direct play by aliasing pcm and wav codecs
WAV files were being transcoded to FLAC even when the browser declared
native WAV support. The backend normalizes ffprobe's pcm_s16le (and
similar PCM variants) to the internal codec name "pcm", while browsers
advertise WAV support as audioCodecs:["wav"] in their client profile.
The direct-play codec check compared these literally and rejected the
match with "audio codec not supported", forcing a needless FLAC
transcode.
Added {"pcm", "wav"} to codecAliasGroups so the matcher treats them
as equivalent. The container check runs first, so AIFF files (which also
normalize to codec "pcm" but use container "aiff") cannot
accidentally match a WAV direct-play profile.
* feat: include profile details in direct-play rejection reasons
The transcodeReason array returned by getTranscodeDecision previously
contained one generic string per failed DirectPlayProfile (e.g., five
copies of "container not supported"), making it hard to correlate a
reason with the profile that rejected the stream.
Each rejection reason now embeds the offending source value (in single
quotes) along with a compact representation of the full profile that
rejected it, rendered as [container/codec]. For example, clients with
two distinct ogg-container profiles (opus and vorbis) produced two
identical rejection strings; they now read "container 'wav' not
supported by profile [ogg/opus]" and "container 'wav' not supported
by profile [ogg/vorbis]", making each entry in the transcodeReason
array unique and self-describing.
A small describeProfile helper renders profiles as [container/codec]
(or [container] when no codec is constrained).
* refactor(stream): address code review — narrow pcm/wav match, tighten tests
Responds to reviewer feedback on the initial PR:
- Replace the symmetric pcm↔wav codec alias with a contextual
isPCMInWAVMatch check in checkDirectPlayProfile. The alias
unconditionally equated the two names in matchesCodec, which would
let AIFF sources (also normalized to codec "pcm") falsely satisfy
a codec-only ["wav"] direct-play profile that omitted containers.
The new check additionally requires src.Container == "wav" before
bridging the names, closing the false-positive path.
- Tighten the rejection-reason test assertions to verify the new
formatted output (source value + profile descriptor) instead of
just matching loose substrings like "container", preventing
unrelated rejections from satisfying the expectations.
- Add coverage for the WAV→wav-codec acceptance path and for the
AIFF-in-wav-codec-profile rejection path to pin down the contract
of isPCMInWAVMatch.
* refactor(codec): rename isPCMInWAVMatch to matchesPCMWAVBridge for clarity
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(db): resolve schema inconsistencies in library_artist and scrobble_buffer tables
* fix(db): address PR comments around speed of the migration
* fix(db): simplify schema inconsistencies migration
Remove ineffective PRAGMA foreign_keys and cache_size statements, which are
no-ops inside goose's wrapping transaction. Drop the down migration body
(Navidrome does not run down migrations) and document the intent. Rename
the file to refresh the timestamp after rebase.
---------
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
PublicURL() copied only the scheme and host from conf.Server.ShareURL,
silently dropping any path component. This broke OpenGraph image URLs
(and other share links) when ShareURL was configured with a path prefix
like https://example.com/navi — generated URLs pointed to /share/img/...
at the root instead of /navi/share/img/...
Now the ShareURL path is prepended to the resource path, with trailing
slashes trimmed. When ShareURL has no path, behavior is unchanged.
* fix(ui): regenerate package-lock.json to have integrity fields
* chore(deps): update esbuild and related packages to version 0.27.7
Signed-off-by: Deluan <deluan@navidrome.org>
* chore(lint): exclude node_modules from golangci-lint
Prevents lint errors from Go files inside npm packages under
ui/node_modules from being picked up by golangci-lint.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
Co-authored-by: Deluan Quintão <deluan@navidrome.org>
* 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 3a213ef03e.
* fix(artwork): avoid data race in cache warmer config access
Capture UICoverArtSize at construction time instead of reading from
conf.Server on each doCacheImage call. The background goroutine could
race with test config cleanup, causing intermittent race detector
failures in CI.
* fix(configuration): clamp UICoverArtSize to be within 200 and 1200
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(artwork): preserve album cache key compatibility with v0.60.3
Restored the v0.60.3 hash input order for album artwork cache keys
(Agents + CoverArtPriority) so that existing caches remain valid on
upgrade when EnableExternalServices is true. Also ensures
CoverArtPriority is always part of the hash even when external services
are disabled, fixing a v0.60.3 bug where changing CoverArtPriority had
no effect on cache invalidation.
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: default EnableWebPEncoding to false and reduce artwork parallelism
Changed EnableWebPEncoding default to false so that upgrading users get
the same JPEG/PNG encoding behavior as v0.60.3 out of the box, avoiding
the WebP WASM overhead until native libwebp is available. Users can
opt in to WebP by setting EnableWebPEncoding=true. Also reduced the
default DevArtworkMaxRequests to half the CPU count (min 2) to lower
resource pressure during artwork processing.
* fix(configuration): update DefaultUICoverArtSize to 300
Signed-off-by: Deluan <deluan@navidrome.org>
* fix(Makefile): append EXTRA_BUILD_TAGS to GO_BUILD_TAGS
Signed-off-by: Deluan <deluan@navidrome.org>
---------
Signed-off-by: Deluan <deluan@navidrome.org>
buildOSPlaylist was the only OpenSubsonic builder function missing the
LegacyClients guard, causing attributes like `validUntil` and `readonly`
to appear in playlist XML responses for legacy clients like DSub2000.
This caused a crash when DSub2000 tried to parse evaluated smart
playlists containing the `validUntil` attribute.
The coverArt field was returning the raw uploaded image filename instead
of the standard ra-{id} artwork ID format. This caused getCoverArt to
fail when clients passed the coverArt value directly. Now uses
CoverArtID().String() consistent with how albums, artists, and playlists
return their coverArt values. Fixes#5293.
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>
* fix(external): refresh stale artist image URLs on expiry
ArtistImage() was serving cached image URLs from the database
indefinitely, ignoring ExternalInfoUpdatedAt. When users changed agent
configuration (e.g. disabling Deezer), old URLs persisted because only
the UpdateArtistInfo code path checked the TTL.
Now ArtistImage() checks the expiry and enqueues a background refresh
when the cached info is stale, matching the pattern used by
refreshArtistInfo(). The stale URL is still returned immediately to
avoid blocking clients.
Fixes#5266
* test: add expired artist image info test with log assertion
Verify that ArtistImage() enqueues a background refresh when cached
info is expired, by capturing log output and checking for the expected
debug message. Also asserts the stale URL is returned immediately
without calling the agent.
Signed-off-by: Deluan <deluan@navidrome.org>
* fix: only enqueue refresh when returning a stale cached URL
Move the expiry check to the else branch so we only enqueue a
background refresh when a cached image URL exists and is being
returned. This avoids doubling external API calls when the URL is
empty (synchronous fetch) but ExternalInfoUpdatedAt is old.
---------
Signed-off-by: Deluan <deluan@navidrome.org>
ffmpeg.ExtractImage returns a pipe-based reader immediately, before ffmpeg
finishes processing. When the audio file has no embedded image stream (e.g.
a plain MP3), ffmpeg exits with an error that closes the pipe asynchronously.
The selectImageReader function saw the non-nil reader as a success and
returned it instead of falling through to the next source in the chain
(album art). This caused getCoverArt to return an error response for tracks
on albums where the disc artwork reader was invoked but no embedded art
existed.
Fixed by reading one byte from the pipe to validate the stream delivers
data before returning it. If the read fails, the reader is closed and nil
is returned, allowing the fallback chain to continue to album artwork.
Closes#5265
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>
* 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
Replaced single Read assertion with Eventually loop to drain buffered
pipe data after context cancellation. The previous test assumed the first
Read after cancel() would fail, but ffmpeg may have already written data
into the pipe buffer before being killed, causing the Read to succeed
from buffered content.
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.
