34 commits

Author	SHA1	Message	Date
Deluan Quintão	aa5aa731dc	refactor(lyrics): single ParseLyrics entry point + all-format plugin lyrics (#5632) ... * refactor(lyrics): read sidecar files via library storage FS Routes fromExternalFile reads through storage.For(mf.LibraryPath) instead of os.Open on AbsolutePath, fixing sidecar reads for non-local backends. UTF-16 LE/BE and BOM handling preserved via ioutils.UTF8Reader. * refactor(lyrics): address review feedback on sidecar FS read - Move blank local-storage import from sources.go into lyrics_suite_test.go (the test suite already imports the local package for RegisterExtractor, so local's init() runs; production binaries get the scheme via normal wiring) - Fix misleading comment: model.ParseLyrics → model.ParseLyricsFile - Replace what-comment with why-comment in BeforeSuite explaining the log.Fatal guard that requires the no-op extractor registration * test(lyrics): add subsonic e2e baseline for getLyrics endpoints Establishes a behavioral baseline for getLyricsBySongId (v2 structured) and getLyrics (legacy) before the lyrics parser refactor. Covers embedded formats (LRC synced, plain text, TTML) and sidecar formats (LRC, SRT, YAML), all isolated under a Lyrics/ fixture folder so the new fixtures do not perturb existing test behavior beyond fixture counts. Sidecar files are injected as raw &fstest.MapFile{Data: []byte(...)} entries; the scanner skips non-audio extensions (.lrc, .srt, .yaml) so they are invisible to scanning but reachable via the fake FS at request time through fromExternalFile/storage.For. Update album/artist/song counts in the album-list, multi-library, and search3 empty-query tests to reflect the six new tracks (1 new artist, 1 new album, 6 new songs). * test(lyrics): strengthen e2e lyrics baseline (lang assertions, rename helper) Rename the local helper `main` to `firstLyric` to avoid collision with the reserved-feeling built-in name. Add `Lang` assertions to both embedded and sidecar DescribeTable entries, locking the current observed values: "xxx" (ISO 639-2 "no language specified") for all embedded and LRC/SRT sidecars, and "eng" for the YAML sidecar (which explicitly sets `language: eng`). * feat(lyrics): detect Lyricsfile YAML in content-sniffing * feat(plugins): content-sniff plugin lyrics for all formats Replace model.ToLyrics (LRC/plain only) with model.ParseEmbedded so plugin responses are content-sniffed for TTML, SRT, YAML, LRC, and plain text. ParseEmbedded returns a LyricList, so the loop now flattens multiple tracks per response entry. The test-lyrics WASM plugin gains a "ttml" format mode (configured via pdk.GetConfig) that returns a minimal TTML document; rebuilt with the standard Go wasip1 toolchain (GOOS=wasip1 GOARCH=wasm). A new Ginkgo test asserts Synced==true and the exact cue value, which the old plain-text path could not produce. GetLyrics doc comment updated to reflect content-sniffing; a later task will retarget it to ParseLyrics once that function is introduced. * test(plugins): validate plugin lyrics auto-detect across all formats The test-lyrics WASM plugin now supports per-format modes via the "format" config key: ttml, srt, yaml, lrc, and plain, in addition to the existing default plain-text response. The plugin is rebuilt with the standard Go wasip1 compiler. lyrics_adapter_test.go gains a DescribeTable covering all five formats, asserting both Synced (the discriminator that proves correct format detection) and the exact line value. This validates the full auto-detect chain (TTML → SRT → YAML/Lyricsfile → LRC → plain) end-to-end through the real plugin → adapter → parser flow. * refactor(lyrics): consolidate parsers into model.ParseLyrics * refactor(lyrics): retarget legacy callers to model.ParseLyrics Pin suffix to ".lrc" to preserve byte-identical output for stored plain/LRC text that was previously handled by the now-removed ToLyrics. * test(lyrics): fix lyrics tests after parser consolidation - Rewrite the YAML-fallback test to assert the correct design: a non-Lyricsfile .yaml sidecar returns as plain text and shadows lower-priority sources (rather than falling through to .lrc). - Add LibraryPath + relative Path split to the three subsonic tests that read sidecar files via storage.For(), so they resolve against the correct fixtures directory. - Register a no-op extractor in api_suite_test.go BeforeSuite so newLocalStorage does not fatal when storage.For is called during sidecar-lyrics tests. * test(lyrics): add per-format ParseLyrics benchmarks Baseline measurements (count=2 runs) on M2: BenchmarkParseLyrics_LRC-8 5725 178796 ns/op 49.78 MB/s 427877 B/op 523 allocs/op BenchmarkParseLyrics_Plain-8 5425 230854 ns/op 32.44 MB/s 102508 B/op 16 allocs/op BenchmarkParseLyrics_EnhancedLRC-8 1942 605893 ns/op 17.66 MB/s 860678 B/op 4256 allocs/op BenchmarkParseLyrics_SRT-8 3249 373991 ns/op 25.91 MB/s 1113575 B/op 4407 allocs/op BenchmarkParseLyrics_TTML-8 1483 813027 ns/op 13.86 MB/s 2198052 B/op 8665 allocs/op BenchmarkParseLyrics_YAML-8 1700 678250 ns/op 13.01 MB/s 1235096 B/op 8288 allocs/op BenchmarkParseLyrics_SniffTTML-8 1525 776482 ns/op 14.51 MB/s 2225448 B/op 8681 allocs/op BenchmarkParseLyrics_SniffSRT-8 2528 451210 ns/op 21.48 MB/s 1157000 B/op 4422 allocs/op BenchmarkParseLyrics_SniffYAML-8 1333 827152 ns/op 10.67 MB/s 1337195 B/op 8718 allocs/op BenchmarkParseLyrics_SniffLRC-8 2820 413038 ns/op 21.55 MB/s 588934 B/op 1812 allocs/op BenchmarkParseLyrics_SniffPlain-8 2968 409091 ns/op 18.31 MB/s 254470 B/op 1491 allocs/op Content-sniff path overhead: 1.5–15% depending on format. * test(lyrics): use real public-domain fixtures for parser benchmarks Replace synthetic benchmark payloads with 'Auld Lang Syne' (Robert Burns, 1788, public domain) rendered into every supported format (LRC, plain, enhanced LRC, SRT, TTML, Lyricsfile YAML) so the numbers reflect realistic content. Same song across formats makes per-format cost comparable. Baseline (Apple M-series, -benchmem, real fixtures): LRC ~28 us/op 42 KB 147 allocs Plain ~23 us/op 18 KB 22 allocs EnhancedLRC ~37 us/op 51 KB 374 allocs SRT ~52 us/op 139 KB 581 allocs TTML ~119 us/op 276 KB 1227 allocs YAML ~142 us/op 193 KB 1732 allocs Sniff(LRC) ~47 us/op 57 KB 237 allocs Sniff(TTML) ~122 us/op 282 KB 1250 allocs Sniff(YAML) ~186 us/op 218 KB 1847 allocs Fixtures in tests/fixtures/lyrics/. * fix(lyrics): preserve [] (not null) for empty lyrics in backfill migration ParseLyrics returns nil for zero-line input (whitespace-only stored lyrics). json.Marshal(nil LyricList) produces null, violating the DB invariant that media_file.lyrics uses [] for empty lyrics, never null. Initialize to model.LyricList{} when ParseLyrics returns nil so the marshalled result is always []. * refactor(lyrics): unify parser dispatch and centralize empty-list invariant Apply thermo-nuclear review findings (behavior-preserving): - Replace the suffix switch + three single-use closure adapters (parseTTMLKnown/parseSRTKnown + inline YAML closure) with a bySuffix map of a single lyricParser(lang, contents) signature. Normalize parseTTMLWithDefaultLang/parseSRTWithLanguage to that (lang, contents) order so no adapter glue is needed. - Collapse the parallel sniffLyrics engine into one parseFirstMatch primitive shared by both the suffix and content-sniff paths (sniffOrder candidate list). TTML stays gated via parseTTMLIfDocument in sniff mode to avoid running the XML decoder on plain/LRC text. - Add LyricList.MarshalJSON so empty/nil always serializes to [] (the lyrics column invariant), in one canonical place. Delete the migration's nil-guard, which the marshaler now subsumes. Behavior verified unchanged: full suite + race + e2e green. * refactor(lyrics): single registry drives both suffix dispatch and sniff order Collapse the bySuffix map and sniffOrder slice into one ordered registry: slice order is the content-sniff probe order, each row's suffixes drive sidecar dispatch, and per-row bySuffix/byContent parsers preserve the gated-TTML-when-sniffing distinction. One source of truth, no duplicated parser references. * refactor(lyrics): self-skipping parsers collapse the format table to one column Move the TTML <tt>-document gate into parseTTMLWithDefaultLang itself (after the encoding fixup, so UTF-16-declared docs are still recognized): non-TTML content returns (nil, nil) to skip; a malformed <tt> document still errors. SRT and Lyricsfile YAML already self-skip. With every structured parser self-skipping, the format table drops to one {suffixes, parse} column named lyricFormats — no bySuffix/byContent split, no separate sniff-only TTML gate. Both the suffix and content-sniff paths share the same parser per format. * refactor(lyrics): strip BOM once at ParseLyrics entry for all paths Previously only the content-sniff path stripped the BOM; the suffix path relied on its callers (fromExternalFile via UTF8Reader) having already stripped it. That implicit contract was fragile — a caller passing raw BOM-prefixed bytes with a suffix would reach the parsers with the BOM intact (SanitizeText does not strip it). Strip once at entry so every path and parser sees clean bytes regardless of caller. No-op for already-stripped input. * refactor(lyrics): trim verbose comments to essential why * refactor(lyrics): move LRC parser to its own lyrics_lrc.go Extract parseLRC, the enhanced-LRC helpers (parseEnhancedLine, adjustGroup, stripEnhancedMarkers, shiftELRCCues), parseTime, and the LRC regexes from lyrics.go into lyrics_lrc.go, with the parseLRC tests in lyrics_lrc_test.go. This makes the layout symmetric — one file per format (lrc/srt/ttml/yaml) — and leaves lyrics.go holding only shared types and cue normalization. All moved symbols were already LRC-private; no behavior change. * refactor(lyrics): collapse ParseLyrics suffix/sniff branches into one loop Both modes differ only in which formats to try, so select candidates in a single loop (all formats when sniffing, the suffix's own otherwise) and run them through parseFirstMatch once. Drops the projected-slice make+index and the ContainsFunc closure; unmatched suffixes yield no candidates and fall to the plain-text floor, as before. * refactor(lyrics): apply simplify-review cleanups - stripBOM: bytes.TrimPrefix instead of []byte<->string round-trip (no alloc) - ParseLyrics: pre-size the candidates slice - move isTTMLDocument to lyrics_ttml.go beside its only caller (the dispatch layer should hold no per-format knowledge) * refactor(lyrics): simplify test descriptions for structured lyrics Signed-off-by: Deluan <deluan@navidrome.org> * refactor(lyrics): fold parseLyricsfile into lyricParser signature and rename file - parseLyricsfile now matches the lyricParser signature directly (reads via bytes.NewReader), removing the parseLyricsfileBytes adapter and the string(contents) copy; the lyricFormats table references it directly. - StructuredLyrics drops the vestigial LyricList{} init (json.Unmarshal overwrites; MarshalJSON owns the empty->[] invariant). - Rename lyricsfile.go -> lyrics_lyricsfile.go (and its test) to match the lyrics_<format>.go convention used by lrc/srt/ttml. * refactor(lyrics): move test-only parseTTML/parseSRT wrappers to test files These zero-arg wrappers (defaulting lang to "xxx") had no production callers after the consolidation — only the format tests used them. Move each beside its tests so the production files carry no test-only code. * build: exclude generated *_gen.go files from linting The plugin host *_gen.go files (ndpgen output) were tripping the whitespace linter despite carrying a generated marker. Exclude them by path so make lint and the pre-push hook pass on untouched generated code. * perf(lyrics): drop []byte/string round-trips in parsers Apply code-review feedback to remove avoidable allocations in the lyrics parsers. isTTMLDocument now takes []byte directly, so parseTTMLWithDefaultLang no longer copies its buffer into a string before the TTML probe. parseSRTBlock splits its block with strings.Split instead of converting to []byte and back per line. ParseLyrics hoists strings.ToLower(suffix) out of the format loop. No behavior change; the dropped len(scanner)==0 SRT guard was dead (strings.Split never returns an empty slice, and the existing len(lines)==0 check still covers empty input). Signed-off-by: Deluan <deluan@navidrome.org> * refactor(lyrics): colocate and unexport cue-normalization helpers Move the cue-normalization machinery out of lyrics.go into a dedicated lyrics_normalize.go (with lyrics_normalize_test.go), leaving lyrics.go to hold just the shared lyric types and their methods. lyrics.go was mixing the domain type/contract definitions with format-agnostic post-processing. Unexport normalizeLyrics, normalizeCueLines, and normalizeLineTiming: they have no callers outside the model package, so they should not be part of its public API. NormalizeCueEnds stays exported because the Subsonic enhanced-lyrics serializer (server/subsonic/lyrics.go) resolves cue ends per agent group while building the response; that is the only legitimate cross-package caller. Also includes a small no-op robustness tweak in parseLRC: len(times) == 0 instead of times == nil (equivalent here, more idiomatic). No behavior change. * test(lyrics): add direct coverage for NormalizeCueEnds NormalizeCueEnds is exported and carries the most intricate logic in the normalization cluster (fill-from-next, fill-from-fallback, both clamps, and the all-or-none clear), but was only exercised transitively. Add a focused spec covering each branch plus the empty-input and no-mutation guarantees, bringing the function to 100% coverage. * test(lyrics): cover legacy getLyrics across formats and sources Expand the legacy getLyrics e2e coverage from a single embedded-plain case to a table over all six fixtures: embedded LRC/plain/TTML and sidecar LRC/SRT/YAML. Each case asserts the v1 plain-text fallback contract — the structured lyric is flattened to LRC-style plain text with no timing markup leaking through (no LRC brackets, SRT arrows, or XML tags), regardless of the source format or whether it is embedded or a sidecar file. This pins the behavior that synced TTML/SRT/ YAML formats degrade gracefully to plain text on the legacy endpoint. * test(lyrics): cover songLyrics v1 vs v2 with word-level fixtures Correct and expand the e2e lyrics coverage to match the OpenSubsonic songLyrics extension contract: - v1 (getLyricsBySongId, no enhanced): line-level lyrics with no cueLine, kind, or agents — even for word-level formats (ELRC, Lyricsfile YAML). - v2 (getLyricsBySongId?enhanced=true): word-level cueLine surfaces for ELRC and YAML sources; kind="main" is set; a line-level source (SRT) still yields no cueLine even when enhanced. - legacy getLyrics (artist/title): the original Subsonic endpoint, flattening any format to plain text. A prior commit mislabeled this as the "v1 contract"; getLyrics predates OpenSubsonic and is unrelated to the extension versions. Drive these with the public-domain tests/fixtures/lyrics files (the same set the parser benchmarks use) so the e2e content stays in sync and actually carries the word-level timing needed to distinguish v1 from v2. The embedded "synced LRC" fixture is upgraded to ELRC (word-level); track counts are unchanged, so the rest of the suite is unaffected. * test(lyrics): parameterize v2 enhanced coverage across all formats Convert the v2 (enhanced) e2e block from three ad-hoc cases into a DescribeTable covering all six formats, matching the v1 and legacy tables. Each entry declares whether the source carries word-level timing: ELRC, TTML, and Lyricsfile YAML surface a cueLine; LRC, SRT, and plain text do not. All six get kind="main". Add word-level <span> timing to the first line of the auld-lang-syne.ttml fixture so TTML exercises the word-level cueLine path (the parser already supports <span begin/end>, but the fixture was line-level only). The first line now yields the same five word cues as the ELRC and YAML fixtures, keeping the table assertions uniform across formats. * fix(lyrics): honor caller language when Lyricsfile YAML omits it parseLyricsfile discarded the caller's language argument, so a Lyricsfile YAML parsed from an embedded tag or plugin response with no metadata.language was labeled "xxx" even when ParseLyrics was given a language. The SRT and TTML parsers already use the caller language as their default; fall back to it here too, preferring the document's own metadata.language when present. Also reword a misleading TTML comment: isTTMLDocument still runs an XML decode (it stops at the first element), so the skip avoids the full TTML parse, not the XML decoder entirely. * refactor(lyrics): consolidate lyrics parsing functions names Signed-off-by: Deluan <deluan@navidrome.org> * test(lyrics): drop test-only parse wrappers after parser rename Commit 48c0173e8 renamed the production parsers to parseTTML/parseSRT, which collided with the same-named test-only wrappers and broke the model test build (parseTTML/parseSRT redeclared). Remove the wrappers and call the production parsers directly with the placeholder language at each test site. * test(lyrics): complete the truncated enhanced-LRC fixture The auld-lang-syne.elrc fixture stopped after the first two stanzas (8 lyric lines) while every other format fixture carries the full 24-line song. Extend it to all 24 lines with per-word timing so it is a faithful enhanced-LRC sample and the EnhancedLRC parser benchmark runs on a workload comparable to the others. The first line's word timings are unchanged, so the e2e cueLine assertions still hold. --------- Signed-off-by: Deluan <deluan@navidrome.org>	2026-06-19 18:25:35 -04:00
Yuuta	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)	2026-06-19 12:00:58 -04:00
Deluan Quintão	08a027dbcc	fix(transcoding): honor player forced format on the WebUI transcode flow (#5613) ... * feat(stream): add ClientInfo.ForceFormat for browser-aware forced format Restricts the client to a forced transcoding format and suppresses direct play, but only when the client declares it supports that format. Part of #5583. * fix(transcoding): honor player forced format on getTranscodeDecision When the WebUI player has a forced transcoding format configured and the browser declares it can play that format, transcode to it (suppressing direct play). Fall back to normal negotiation with a warning when the format is unsupported. The MaxBitRate cap still applies on top. Fixes #5583. * test(e2e): cover player forced format on getTranscodeDecision Forced format honored when the client supports it, falls back to negotiation otherwise, and the MaxBitRate cap still applies on top. Part of #5583. * feat(ui): remove obsolete 'format ignored' helper text on player form The web player now honors the forced transcoding format, so the caveat added in #5611 no longer applies. Reverts the Transcoding field to a plain selector. Part of #5583.	2026-06-14 20:52:19 -04:00
Deluan Quintão	c4c70519b5	fix(transcoding): enforce server-side player MaxBitRate on /rest/stream (#5611) ... * fix(transcoding): enforce player MaxBitRate on getTranscodeDecision The Web UI streams via getTranscodeDecision, which (since #5473) ignored the server-side player config. Apply the player's MaxBitRate as a bitrate ceiling on the client's declared limits before MakeDecision, restoring per-player bitrate enforcement without reintroducing the forced-format override. Fixes #5583. * test(e2e): assert player MaxBitRate is enforced on getTranscodeDecision Invert the assertions added in #5473 that expected the player cap to be ignored; getTranscodeDecision now enforces it (issue #5583). * feat(ui): clarify web player ignores forced transcoding format Add helper text to the Transcoding field on the player edit form when the player is the NavidromeUI web client, since it enforces only the Max. Bit Rate, not the forced format. Part of issue #5583. * refactor(stream): extract ClientInfo.CapBitrate, share across transcode paths Move the player MaxBitRate ceiling logic into a canonical ClientInfo.CapBitrate method in core/stream, used by both getTranscodeDecision and the legacy ResolveRequest path. Removes handler-layer duplication and corrects a misleading comment that wrongly implied the legacy single-field cap was buggy. * fix(transcoding): downsample on legacy /stream when only player MaxBitRate is set A bare /stream or /download request from a player configured with a server-side MaxBitRate (but no forced format) was served raw, ignoring the cap. buildLegacyClientInfo now triggers DefaultDownsamplingFormat when the player MaxBitRate alone is below the source bitrate, matching the already-correct forced-format and request-bitrate paths. Part of #5583. * fix(ui): add Brazilian Portuguese translation for player transcoding helper text Translates the new resources.player.helperTexts.transcodingId key added for the web player transcoding-format clarification. Part of #5583. * fix(ui): restore Transcoding field styling and render helper text The TranscodingInput wrapper swallowed the variant SimpleForm injects into its direct children (field lost its outlined box) and put helperText on the ReferenceInput, which does not forward it to the input. Spread the form props onto ReferenceInput and move helperText to the SelectInput child so both the outlined styling and the helper text render. Part of #5583. * fix(i18n): update Brazilian Portuguese translation for album artist field Signed-off-by: Deluan <deluan@navidrome.org> * fix(ui): clean up comments in PlayerEdit component Signed-off-by: Deluan <deluan@navidrome.org> * test(ui): mock useTranslate in PlayerEdit test for determinism Avoid depending on ra-core's out-of-provider translation behavior, which can vary by version. Part of #5583. --------- Signed-off-by: Deluan <deluan@navidrome.org>	2026-06-14 16:52:01 -04:00
Deluan Quintão	da56df3160	feat(smartplaylist): extend isMissing/isPresent to bpm, bitDepth and many text fields (#5603) ... * feat(smartplaylist): support isMissing/isPresent on mbz_* and lyrics fields Mark the six mbz_* MusicBrainz ID columns and the lyrics column as Nullable in the criteria field map, then extend missingExpr to handle string columns where absence is encoded as NULL or empty string (plus '[]' for lyrics). The Numeric/Boolean path (ReplayGain) is preserved via an explicit type check. * refactor(model): make MediaFile BPM and BitDepth nullable pointers Convert BPM and BitDepth fields in model.MediaFile from int to *int so that 'tag absent' is distinguishable from zero. The metadata mapper now uses NullableFloat for BPM (nil when absent or zero/unparseable) and only sets BitDepth when the audio property is non-zero (lossy codecs report 0). All read sites use gg.V() for zero-fallback deref so Subsonic API output and transcoding behaviour are byte-identical to before. The persistence layer bridges the existing NOT NULL DB columns by coercing nil to 0 on write and 0 back to nil on read in PostMapArgs/PostScan; a later migration task will drop those constraints. Hash upgrade safety is verified by a new MediaFile.Hash describe block: nil *int hashes identically to the old int(0) default via ZeroNil+IgnoreZeroValue, so no files will be spuriously re-imported after this change. Extra files touched beyond the plan's list: core/stream/legacy_client_test.go (BitDepth in model.MediaFile literals), persistence/mediafile_repository.go (NOT NULL bridge). * test(model): pin pre-conversion golden hashes for BPM/BitDepth * feat(smartplaylist): support isMissing/isPresent on bpm and bitDepth * feat(db): make bpm and bit_depth columns nullable, backfill 0 to NULL Drop the NOT NULL constraint on media_file.bpm and bit_depth via a lossless migration that converts legacy 0-means-absent values to real NULL. Remove the temporary shim in PostScan/PostMapArgs that was bridging the old NOT NULL columns to the *int model fields. Add round-trip persistence tests asserting NULL storage for nil pointers and correct value round-trip for non-nil pointers. * test(e2e): verify isMissing/isPresent partition for nullable fields Add DescribeTable covering bpm, bitdepth, lyrics, and mbz_recording_id: for each field, isMissing + isPresent song counts must equal the total library count, proving the nullable-column SQL is exhaustive and correct. * test(e2e): seed bpm tag so isMissing/isPresent partition is non-trivial * fix(model): omit bitDepth from JSON when absent instead of emitting null * feat(smartplaylist): support isMissing/isPresent on more string fields Enable isMissing/isPresent operators for album, comment, catalognumber, discsubtitle, albumcomment, sorttitle, sortalbum, sortartist, sortalbumartist, and explicitstatus by marking them Nullable in fieldMap. * refactor(smartplaylist): unify missingExpr column logic into one flow Collapse the numeric/string fork in missingExpr into a single empties-driven loop (numeric/boolean fields simply have no empties), and replace the duplicated IsTag/IsRole guard with a three-way switch that expresses the dispatch model once. No SQL semantics change for string fields; numeric/boolean fields now emit a single-element Or/And which squirrel parenthesizes (e.g. `(col IS NULL)` instead of bare `col IS NULL`) — update the affected test expectations accordingly.	2026-06-13 13:15:20 -04:00
Deluan	1e7996f5d7	fix(share): enforce per-user ownership on share reads ... Share repository read methods (Get, GetAll, Read, ReadAll, Exists, Count, CountAll) did not apply an owner filter, so non-admin users saw shares belonging to other users. The write paths already enforced per-user ownership; this brings reads in line with them. Add an addRestriction()/ownerFilter() based scope to share reads, keeping admins and the headless public-share resolution path unrestricted. Route share and player Delete through a new base-repo deleteOwned() primitive that applies the ownership predicate in the DELETE's WHERE clause (atomic, no select-then- delete window) and classifies a zero-row result as permission-denied vs not-found, mirroring updateOwned. The addRestriction helper and the write-miss classifier are hoisted onto the base repository so player and share share one implementation. Also map rest.ErrPermissionDenied and rest.ErrNotFound in the Subsonic error handler so ownership/not-found failures from the rest-backed repositories return the proper Subsonic codes (50 / 70) instead of a generic error. Covered by unit tests (persistence, subsonic error mapping) and an end-to-end cross-user sharing isolation test.	2026-06-05 15:50:59 -04:00
Deluan Quintão	a84f092d00	fix(subsonic): require admin access for Subsonic management endpoints (#5510) ... * fix: require admin for radio mutations Subsonic internet radio station mutation endpoints are admin-only in the Subsonic and OpenSubsonic specs, but the router only required an authenticated player. Add a reusable Subsonic admin middleware and apply it to create, update, and delete radio routes while leaving the list endpoint available to authenticated users. Cover the middleware and router behavior with unit and e2e tests. * fix: streamline admin-only routes for internet radio station management Signed-off-by: Deluan <deluan@navidrome.org> * fix: use admin-only middleware for starting scans Signed-off-by: Deluan <deluan@navidrome.org> * test: align start scan authorization coverage StartScan authorization now lives in the shared Subsonic admin middleware instead of the handler. Remove the obsolete direct handler unit assertion so the package tests reflect the route-level guard covered by middleware and e2e tests. * fix: require admin for getUsers The Subsonic getUsers endpoint exposes user-list semantics and should use the same shared admin middleware as other admin-only management endpoints. Apply the route-level guard while leaving getUser unchanged, and update the multi-user e2e coverage to expect regular users to receive an authorization failure. * test: cover admin-only Subsonic access Add e2e coverage that admins can still call getUsers after the route-level guard and that regular authenticated users can still list internet radio stations. These cases capture the access boundaries raised during PR review. --------- Signed-off-by: Deluan <deluan@navidrome.org>	2026-05-19 14:23:38 -03:00
Deluan Quintão	b18dfb474a	fix(transcoding): don't apply server-side override on getTranscodeDecision (#5473) ... * fix(transcoding): don't apply server-side transcoding override on getTranscodeDecision The getTranscodeDecision endpoint was incorrectly applying server-side player transcoding overrides (forced format and MaxBitRate cap), which replaced the client's declared capabilities with synthetic profiles. This caused the endpoint to ignore what the client can actually play and return decisions for formats the client never requested (e.g. AAC when the client only supports FLAC/opus/mp3). The override is now gated behind an ApplyServerOverride flag in TranscodeOptions, which is only set by the legacy stream endpoint where this behavior is expected. Signed-off-by: Deluan <deluan@navidrome.org> * refactor: move server-side transcoding override to ResolveRequest Moved the server-side player transcoding override logic (forced format and MaxBitRate cap) from MakeDecision into ResolveRequest, where the legacy stream context is handled. This makes MakeDecision a pure function that only operates on the ClientInfo it receives, removing the ApplyServerOverride flag and all context-sniffing from the decision engine. Tests moved accordingly to legacy_client_test.go. * test(e2e): update transcode decision tests for server override removal Updated e2e tests to reflect that getTranscodeDecision no longer applies server-side player overrides (MaxBitRate cap and forced transcoding profile). The player MaxBitRate tests now verify the endpoint ignores the player cap and relies solely on client-declared capabilities. * test(e2e): assert opus default bitrate when player cap is ignored Added bitrate assertion to verify the player MaxBitRate cap is truly ignored: the target bitrate should be the opus format default (128kbps), not the player cap (320kbps). --------- Signed-off-by: Deluan <deluan@navidrome.org>	2026-05-06 10:03:24 -04:00
Deluan Quintão	13c48b38a0	fix(smartplaylists): coerce string booleans in smart playlist rules (#5450) ... * fix(criteria): coerce string booleans in smart playlist rules - #4826 When clients (e.g. Feishin) send boolean values as strings ("true"/"false") in smart playlist JSON rules, the SQL comparison fails because SQLite stores booleans as 0/1 integers. For example, `COALESCE(annotation.starred, false) = 'true'` never matches. This adds a `boolean` flag to mapped fields and coerces string values to native Go bools in `mapFields`, so squirrel generates correct SQL parameters. Signed-off-by: mango766 <mango766@users.noreply.github.com> Signed-off-by: easonysliu <easonysliu@tencent.com> * fix(criteria): implement boolean string coercion for smart playlist rules Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: mango766 <mango766@users.noreply.github.com> Signed-off-by: easonysliu <easonysliu@tencent.com> Signed-off-by: Deluan <deluan@navidrome.org> Co-authored-by: easonysliu <easonysliu@tencent.com>	2026-05-01 19:21:48 -04:00
Deluan Quintão	94eb6c522b	feat(subsonic): implement playbackReport OpenSubsonic extension (#5442) ... * feat(req): add Float64Or helper for parsing float query params * feat(scrobbler): extend NowPlayingInfo with state/position/rate fields * feat(scrobbler): implement ReportPlayback with state machine and auto-scrobble * feat(responses): add state/positionMs/playbackRate to NowPlayingEntry * feat(subsonic): add reportPlayback endpoint handler * feat(subsonic): include state/positionMs/playbackRate in getNowPlaying response * feat(subsonic): register playbackReport OpenSubsonic extension * test(e2e): add reportPlayback endpoint e2e tests * refactor(scrobbler): simplify ReportPlayback — extract helpers, remove duplication - Add state constants and exported ValidStates map - Extract remainingTTL() helper (was duplicated 3x) - Merge playing/paused switch cases into single branch - Use Get instead of GetWithParticipants for non-stopped states - Guard NowPlayingCount broadcast with count-change detection - Use cache entry for NowPlaying dispatch instead of extra DB query - Remove redundant Position field from NowPlayingInfo * refactor(scrobbler): skip DB query in playing/paused when playMap has entry * fix(play_tracker): handle errors when adding/updating NowPlayingInfo in cache Signed-off-by: Deluan <deluan@navidrome.org> * refactor(play_tracker): replace sort with slices.SortFunc for NowPlayingInfo Signed-off-by: Deluan <deluan@navidrome.org> * fix(play_tracker): check all ReportPlayback errors in tests Replace _ = with explicit error assertions to avoid masking failures in intermediate calls. Signed-off-by: Deluan <deluan@navidrome.org> * test(e2e): use real PlayTracker and assert getNowPlaying after reportPlayback Replace noopPlayTracker with a real PlayTracker backed by the E2E database. E2E tests now verify the full round-trip: reportPlayback creates/updates/removes entries visible via getNowPlaying, including state, positionMs, and playbackRate fields. Export NewPlayTracker constructor for use outside the scrobbler package. * fix(play_tracker): account for playback rate in TTL and detect track switches The remainingTTL function now divides remaining time by the playback rate, so cache entries expire correctly at non-1x speeds (e.g., 2x playback halves the TTL). Zero/negative rates default to 1.0. The playing/paused case now checks if the cached MediaFile ID matches the reported mediaId, falling back to a DB fetch when the client switches tracks without sending stopped/starting. Adds parameterized tests for remainingTTL covering rate variations and edge cases. * fix(subsonic): validate positionMs and playbackRate in reportPlayback Reject negative positionMs values and invalid playbackRate values (NaN, Inf, zero, negative) at the API boundary before they reach TTL and position estimation math. Returns clear error messages for each case. * feat(play_tracker): add ClientId and ClientName to ReportPlayback parameters Signed-off-by: Deluan <deluan@navidrome.org> * refactor(play_tracker): replace NowPlaying method with ReportPlayback calls Signed-off-by: Deluan <deluan@navidrome.org> * refactor(play_tracker_test): remove redundant TTL behavior tests and clean up mockPluginLoader Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org>	2026-04-30 23:04:05 -04:00
Deluan Quintão	259c1a9484	feat(subsonic): add sonicSimilarity extension as plugin capability (#5419) ... * feat(plugins): add sonicSimilarity capability types Defines the SonicSimilarity plugin capability interface with GetSonicSimilarTracks and FindSonicPath methods, along with their request/response types. * feat(sonic): add core sonic similarity service Implements the Sonic service with HasProvider, GetSonicSimilarTracks, and FindSonicPath, delegating to the PluginLoader and using the Matcher for index-preserving library resolution. * test(sonic): add sonic service unit tests Covers HasProvider, GetSonicSimilarTracks, and FindSonicPath with mock plugin loader and provider, verifying error propagation and successful match resolution via the library matcher. * feat(matcher): add MatchSongsToLibraryMap for index-preserving matching Adds a new method alongside MatchSongsToLibrary that returns a map[int]MediaFile keyed by input song index rather than a flat slice, enabling callers to correlate similarity scores back to the original position in the results. * fix(sonic): check provider availability before MediaFile lookup Avoids unnecessary DB call when no plugin is available, and ensures the correct error path is tested. * feat(plugins): add sonic similarity adapter Adds SonicSimilarityAdapter implementing sonic.Provider, bridging the plugin system to the core sonic service via Extism plugin functions. Reuses existing songRefsToAgentSongs helper for SongRef conversion. * feat(plugins): add LoadSonicSimilarity to plugin manager Adds Manager.LoadSonicSimilarity method following the pattern of LoadLyricsProvider, enabling the core sonic service to load a SonicSimilarityAdapter from a named plugin. * feat(subsonic): add sonicMatch response type Add SonicMatch struct with Entry and Similarity fields, and a SonicMatches slice to the Subsonic response struct. These types support the OpenSubsonic sonicSimilarity extension for returning similarity-scored track results. * feat(subsonic): add getSonicSimilarTracks and findSonicPath handlers Add two new Subsonic API handlers for the sonicSimilarity OpenSubsonic extension: GetSonicSimilarTracks returns similarity-scored tracks similar to a given song, and FindSonicPath returns a path of tracks connecting two songs. Both handlers delegate to the sonic core service and map results to SonicMatch response types. * feat(subsonic): advertise sonicSimilarity extension when plugin available Update GetOpenSubsonicExtensions to conditionally include the sonicSimilarity extension only when a sonic similarity plugin provider is available. The nil guard ensures backward compatibility with tests that pass nil for the sonic field. Also update the existing test to pass the new nil parameter. * feat(subsonic): wire sonic similarity service into router Add the sonic.Sonic service to the Router struct and New() constructor, register the getSonicSimilarTracks and findSonicPath routes, and wire sonic.New and its PluginLoader binding into the Wire dependency injection graph. Update all existing test call sites to pass the new nil parameter. Regenerate wire_gen.go. * fix(e2e): add sonic parameter to subsonic.New call in e2e tests * test(subsonic): add sonicSimilarity extension advertisement tests Restructures the GetOpenSubsonicExtensions test into two contexts: one verifying the baseline 5 extensions are returned when no sonic similarity plugin is configured, and one verifying that the sonicSimilarity extension is advertised (making 6 total) when a plugin loader reports an available provider. Adds a mockSonicPluginLoader to satisfy the sonic.PluginLoader interface without requiring a real plugin. * feat(subsonic): add nil guard and e2e tests for sonic similarity endpoints Handlers return ErrorDataNotFound when no sonic service is configured, preventing nil panics. E2e tests verify both endpoints return proper error responses when no plugin is available. * fix(subsonic): return HTTP 404 when no sonic similarity plugin available Endpoints are always registered but return 404 when no provider is available, rather than a subsonic error code 70. * refactor: clean up sonic similarity code after review Extract shared helpers to reduce duplication across the sonic similarity implementation: loadAllMatches in matcher consolidates the 4-phase matching pipeline, songRefToAgentSong eliminates per-iteration slice allocation in the adapter, sonicMatchResponse deduplicates response building in handlers, and a package-level constant replaces raw capability name strings in core/sonic. * fix empty response shapes Signed-off-by: Deluan <deluan@navidrome.org> * test(plugins): add testdata plugin and e2e tests for sonic similarity Add a test-sonic-similarity WASM plugin that implements both GetSonicSimilarTracks and FindSonicPath via the generated sonicsimilarity PDK. The plugin returns deterministic test data with decreasing similarity scores and supports error injection via config. Adapter tests verify the full round-trip through the WASM plugin including error handling. Also includes regenerated PDK code from make gen. * docs: update README to include new capabilities and usage examples for plugins Signed-off-by: Deluan <deluan@navidrome.org> * test(e2e): enhance sonic similarity tests with additional scenarios and mock provider Signed-off-by: Deluan <deluan@navidrome.org> * fix: address PR review feedback for sonic similarity Fix incorrect field names in README documentation ({from, to} → {startSong, endSong}) and remove unnecessary XML serialization test from e2e suite since OpenSubsonic endpoints only use JSON. * refactor: rename Matcher methods for conciseness Rename MatchSongsToLibrary to MatchSongs and MatchSongsToLibraryMap to MatchSongsIndexed. The Matcher receiver already establishes the "to library" context, making that suffix redundant, and "Indexed" better describes the intent (preserving input ordering) than "Map" which describes the data structure. * refactor: standardize variable naming for media files in sonic path methods Signed-off-by: Deluan <deluan@navidrome.org> * refactor: simplify plugin loading by introducing adapter constructors Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org>	2026-04-27 17:50:09 -04:00
Deluan Quintão	64c8d3f4c5	ci: run Go tests on Windows (#5380) ... * ci(windows): add skeleton go-windows job (compile-only smoke test) * ci(windows): fix comment to reference Task 7 not Task 6 * ci(windows): harden PATH visibility and set explicit bash shell * ci(windows): enable full go test suite and ndpgen check * test(gotaglib): skip Unix-only permission tests on Windows * test(lyrics): skip Windows-incompatible tests * test(utils): skip Windows-incompatible tests * test(mpv): skip Windows-incompatible playback tests Skip 3 subprocess-execution tests that rely on Unix-style mpv invocation; .bat output includes \r-terminated lines that break argument parsing (#TBD-mpv-windows). * test(storage): skip Windows-incompatible tests Skip relative-path test where filepath.Join uses backslash but the storage implementation returns a forward-slash URL path (#TBD-path-sep-storage). * test(storage/local): skip Windows-incompatible tests Skip 13 tests that fail because url.Parse("file://" + windowsPath) treats the drive letter colon as an invalid port; also skip the Windows drive-letter path test that exposes a backslash vs forward-slash normalisation bug (#TBD-path-sep-storage-local). * test(playlists): skip Windows-incompatible tests * test(model): skip Windows-incompatible tests * test(model/metadata): skip Windows-incompatible tests * test(core): skip Windows-incompatible tests AbsolutePath uses filepath.Join which produces OS-native path separators; skip the assertion test on Windows until the production code is fixed (#TBD-path-sep-core). * test(artwork): skip Windows-incompatible tests Artwork readers produce OS-native path separators on Windows while tests assert forward-slash paths; skip 11 affected tests pending a fix in production code (#TBD-path-sep-artwork). * test(persistence): skip Windows-incompatible tests Skip flaky timestamp comparison (#TBD-flake-persistence) and path-separator real-bugs (#TBD-path-sep-persistence) in FolderRepository.GetFolderUpdateInfo which uses filepath.Clean/os.PathSeparator converting stored forward-slash paths to backslashes on Windows. * test(scanner): skip Windows-incompatible tests Skip symlink tests (Unix-assumption), ndignore path-separator bugs (#TBD-path-sep-scanner) in processLibraryEvents/resolveFolderPath where filepath.Rel/filepath.Split return backslash paths incompatible with fs.FS forward-slash expectations, error message mismatch on Windows, and file format upgrade detection (#TBD-path-sep-scanner). * test(plugins): skip Windows-incompatible tests Add //go:build !windows tags to test files that reference the suite bootstrap (testManager, testdataDir, createTestManager) which is only compiled on non-Windows. Add a Windows-only suite stub that skips all specs via BeforeEach to prevent [build failed] on Windows CI. * test(server): skip Windows-incompatible tests Skip createUnixSocketFile tests that rely on Unix file permission bits (chmod/fchmod) which are not supported on Windows. * test(nativeapi): skip Windows-incompatible tests Skip the i18n JSON validation test that uses filepath.Join to build embedded-FS paths; filepath.Join produces backslashes on Windows which breaks fs.Open (embedded FS always uses forward slashes). * test(e2e): skip Windows-incompatible tests On Windows, SQLite holds file locks that prevent the Ginkgo TempDir DeferCleanup from deleting the DB file. Register an explicit db.Close DeferCleanup (LIFO before TempDir cleanup) on Windows so the file lock is released before the temp directory is removed. * test(windows): fix e2e AfterSuite and skip remaining scanner path test * test(scanner): skip another Windows path-sep test (#TBD-path-sep-scanner) * test(subsonic): skip timing-flaky test on Windows (#TBD-flake-time-resolution-subsonic) * test(scanner): skip 'detects file moved to different folder' on Windows * test(scanner): consolidate 'Library changes' Windows skips into BeforeEach * test(scanner): close DB before TempDir cleanup to fix Windows file lock * test(scanner): skip ScanFolders suite on Windows instead of closing shared DB * ci: retrigger for Windows soak run 2/3 * ci: retrigger for Windows soak run 3/3 * ci: retrigger for Windows soak run 3/3 (take 2) * test(scanner): skip Multi-Library suite on Windows (SQLite file lock) * ci(windows): promote go-windows to blocking status check * test(plugins): run platform-neutral specs on Windows, drop blanket Skip * test(windows): make tests cross-platform instead of skipping - subsonic: back-date submissionTime baseline by 1s so BeTemporally(">") passes under millisecond clock resolution - persistence: sleep briefly between Put calls so UpdatedAt is strictly after CreatedAt on low-resolution clocks - utils/files: close tempFile before os.Remove so the test works on Windows (where an open handle holds a file lock) - tests.TempFile: close the handle before returning; metadata tests no longer leak the open file into Ginkgo's TempDir cleanup Resolves Copilot review comments on #5380. * test(tests): add SkipOnWindows helper to reduce boilerplate Introduces tests.SkipOnWindows(reason) that wraps the 3-line runtime.GOOS guard pattern used in every Windows-skipped spec. * test(adapters): use tests.SkipOnWindows helper * test(core): use tests.SkipOnWindows helper * test(model): use tests.SkipOnWindows helper * test(persistence): use tests.SkipOnWindows helper * test(scanner): use tests.SkipOnWindows helper * test(server): use tests.SkipOnWindows helper * test(plugins): run pure-Go unit tests on Windows config_validation_test, manager_loader_test, and migrate_test have no WASM/exec dependencies and don't rely on the make-built test plugins from plugins_suite_test.go. Let them run on Windows too.	2026-04-19 13:16:47 -04:00
Deluan Quintão	27209ed26a	fix(transcoding): clamp target channels to codec limit (#5336) (#5345) ... * 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.	2026-04-11 23:15:07 -04:00
Deluan Quintão	36a7be9eaf	fix(transcoding): include ffprobe in MSI and fall back gracefully when absent (#5326) ... * 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>	2026-04-07 20:11:38 -04:00
Deluan Quintão	3f7226d253	fix(server): improve transcoding failure diagnostics and error responses (#5227) ... * fix(server): capture ffmpeg stderr and warn on empty transcoded output When ffmpeg fails during transcoding (e.g., missing codec like libopus), the error was silently discarded because stderr was sent to io.Discard and the HTTP response returned 200 OK with a 0-byte body. - Capture ffmpeg stderr in a bounded buffer (4KB) and include it in the error message when the process exits with a non-zero status code - Log a warning when transcoded output is 0 bytes, guiding users to check codec support and enable Trace logging for details - Remove log level guard so transcoding errors are always logged, not just at Debug level Signed-off-by: Deluan <deluan@navidrome.org> * fix(server): return proper error responses for empty transcoded output Instead of returning HTTP 200 with 0-byte body when transcoding fails, return a Subsonic error response (for stream/download/getTranscodeStream) or HTTP 500 (for public shared streams). This gives clients a clear signal that the request failed rather than a misleading empty success. Signed-off-by: Deluan <deluan@navidrome.org> * test(e2e): add tests for empty transcoded stream error responses Add E2E tests verifying that stream and download endpoints return Subsonic error responses when transcoding produces empty output. Extend spyStreamer with SimulateEmptyStream and SimulateError fields to support failure injection in tests. Signed-off-by: Deluan <deluan@navidrome.org> * refactor(server): extract stream serving logic into Stream.Serve method Extract the duplicated non-seekable stream serving logic (header setup, estimateContentLength, HEAD draining, io.Copy with error/empty detection) from server/subsonic/stream.go and server/public/handle_streams.go into a single Stream.Serve method on core/stream. Both callers now delegate to it, eliminating ~30 lines of near-identical code. * fix(server): return 200 with empty body for stream/download on empty transcoded output Don't return a Subsonic error response when transcoding produces empty output on stream/download endpoints — just log the error and return 200 with an empty body. The getTranscodeStream and public share endpoints still return HTTP 500 for empty output. Stream.Serve now returns (int64, error) so callers can check the byte count. --------- Signed-off-by: Deluan <deluan@navidrome.org>	2026-03-18 12:39:03 -04:00
Deluan Quintão	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>	2026-03-15 22:19:55 -04:00
Deluan Quintão	a50b2a1e72	feat(artwork): preserve animated image artwork during resize (#5184) ... * feat(artwork): preserve animated image artwork during resize Detect animated GIFs, WebPs, and APNGs via lightweight byte scanning and preserve their animation when serving resized artwork. Animated GIFs are converted to animated WebP via ffmpeg with optional downscaling; animated WebP/APNG are returned as-is since ffmpeg cannot re-encode them. Adds ConvertAnimatedImage to the FFmpeg interface for piping stdin data through ffmpeg with animated WebP output. * fix(artwork): address code review feedback for animated artwork Fix ReadCloser leak where ffmpeg pipe's Close was discarded by io.NopCloser wrapping — now preserves ReadCloser semantics when the resized reader already supports Close. Use uint64 for PNG chunk position to prevent potential overflow on 32-bit platforms. Add integration tests for the animation branching logic in resizeImage.	2026-03-13 18:11:12 -04:00
Kendall Garner	903e3f070f	fix(subsonic): always return required playqueue fields (#5172)	2026-03-12 08:29:37 -04:00
Deluan	5ecbe31a06	fix: implement fallback to DefaultDownsamplingFormat for unknown formats ... Signed-off-by: Deluan <deluan@navidrome.org>	2026-03-11 09:46:13 -04:00
Deluan	75e5bc4e81	refactor: rename spy to streamerSpy in e2e tests for clarity ... Renamed the spy variable to streamerSpy across all e2e test files so that its purpose is immediately clear without needing to look up the declaration.	2026-03-10 17:19:25 -04:00
Deluan	053a0fd6c0	fix: prevent raw file being returned when explicit transcode format is requested ... When a client requests transcoding with an explicit format (e.g., format=opus) but no maxBitRate, buildLegacyClientInfo was adding a direct play profile matching the source format. Since there was no bitrate constraint to block it, MakeDecision would match the source against the direct play profile and return the raw file instead of transcoding. This fix only adds the direct play profile when no explicit format was requested (bitrate-only downsampling) or when the requested format matches the source format (allowing direct play when no actual transcoding is needed).	2026-03-10 17:14:21 -04:00
Deluan Quintão	767744a301	refactor: rename core/transcode to core/stream, simplify MediaStreamer (#5166) ... * refactor: rename core/transcode directory to core/stream * refactor: update all imports from core/transcode to core/stream * refactor: rename exported symbols to fit core/stream package name * refactor: simplify MediaStreamer interface to single NewStream method Remove the two-method interface (NewStream + DoStream) in favor of a single NewStream(ctx, mf, req) method. Callers are now responsible for fetching the MediaFile before calling NewStream. This removes the implicit DB lookup from the streamer, making it a pure streaming concern. * refactor: update all callers from DoStream to NewStream * chore: update wire_gen.go and stale comment for core/stream rename * refactor: update wire command to handle GO_BUILD_TAGS correctly Signed-off-by: Deluan <deluan@navidrome.org> * fix: distinguish not-found from internal errors in public stream handler * refactor: remove unused ID field from stream.Request * refactor: simplify ResolveRequestFromToken to receive *model.MediaFile Move MediaFile fetching responsibility to callers, making the method focused on token validation and request resolution. Remove ErrMediaNotFound (no longer produced). Update GetTranscodeStream handler to fetch the media file before calling ResolveRequestFromToken. * refactor: extend tokenTTL from 12 to 48 hours Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org>	2026-03-09 22:22:58 -04:00
Deluan Quintão	d7c3a50f86	fix: player MaxBitRate cap, format-aware defaults, browser profile filtering (#5165) ... * feat(transcode): apply player MaxBitRate cap and use format-aware default bitrates Add player MaxBitRate cap to the transcode decider so server-side player bitrate limits are respected when making OpenSubsonic transcode decisions. The player cap is applied only when it is more restrictive than the client's maxAudioBitrate (or when the client has no limit). Also replace the hardcoded 256 kbps default with a format-aware lookup that checks the DB first (for user-customized values), then built-in defaults, and finally falls back to 256 kbps. For lossless→lossy transcoding, prefer maxTranscodingAudioBitrate over maxAudioBitrate when available. * test(e2e): add tests for player MaxBitRate cap and format-aware default bitrates Add e2e tests covering: - Player MaxBitRate forcing transcode when source exceeds cap - Player MaxBitRate having no effect when source is under cap - Client limit winning when more restrictive than player MaxBitRate - Player MaxBitRate winning when more restrictive than client limit - Player MaxBitRate=0 having no effect - Format-aware defaults: mp3 (192kbps), opus (128kbps) instead of hardcoded 256 - maxAudioBitrate fallback for lossless→lossy when no maxTranscodingAudioBitrate - maxTranscodingAudioBitrate taking priority over maxAudioBitrate - Combined player + client limits flowing correctly through decision→stream * feat(transcode): update transcoding profiles to add flac, filter by supported codecs, and ensure mp3 fallback Signed-off-by: Deluan <deluan@navidrome.org> * fix(db): ensure all default transcodings exist on upgrade Older installations that were seeded before aac/flac were added to DefaultTranscodings may be missing these entries. The previous migration only added flac; this one ensures all default transcodings are present without touching user-customized entries. * test: remove duplication Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org>	2026-03-09 16:47:34 -04:00
Deluan	7c5aa1fafa	test(e2e): add transcode endpoint e2e tests and clean up test helpers ... Add comprehensive e2e tests for getTranscodeDecision and getTranscodeStream endpoints covering direct play, transcoding, error handling, and round-trip token validation. Refactor buildPostReq to reuse buildReq for auth params, remove unused WAV/AAC test tracks, and consolidate duplicate test assertions.	2026-03-09 09:43:55 -04:00
Deluan Quintão	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>	2026-03-08 23:57:49 -04:00
Deluan Quintão	f03ca44a8e	feat(plugins): add lyrics provider plugin capability (#5126) ... * feat(plugins): add lyrics provider plugin capability Refactor the lyrics system from a static function to an interface-based service that supports WASM plugin providers. Plugins listed in the LyricsPriority config (alongside "embedded" and file extensions) are now resolved through the plugin system. Includes capability definition, Go/Rust PDK, adapter, Wire integration, and tests for plugin fallback behavior. * test(plugins): add lyrics capability integration test with test plugin * fix(plugins): default lyrics language to 'xxx' when plugin omits it Per the OpenSubsonic spec, the server must return 'und' or 'xxx' when the lyrics language is unknown. The lyrics plugin adapter was passing an empty string through when a plugin didn't provide a language value. This defaults the language to 'xxx', consistent with all other callers of model.ToLyrics() in the codebase. * refactor(plugins): rename lyrics import to improve clarity Signed-off-by: Deluan <deluan@navidrome.org> * refactor(lyrics): update TrackInfo description for clarity Signed-off-by: Deluan <deluan@navidrome.org> * fix(lyrics): enhance lyrics plugin handling and case sensitivity Signed-off-by: Deluan <deluan@navidrome.org> * fix(plugins): update payload type to string with byte format for task data Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org>	2026-03-03 15:48:39 -05:00
Deluan Quintão	b59eb32961	feat(subsonic): sort search3 results by relevance (#5086) ... * fix(subsonic): optimize search3 for high-cardinality FTS queries Use a two-phase query strategy for FTS5 searches to avoid the performance penalty of expensive LEFT JOINs (annotation, bookmark, library) on high-cardinality results like "the". Phase 1 runs a lightweight query (main table + FTS index only) to get sorted, paginated rowids. Phase 2 hydrates only those few rowids with the full JOINs, making them nearly free. For queries with complex ORDER BY expressions that reference joined tables (e.g. artist search sorted by play count), the optimization is skipped and the original single-query approach is used. * fix(search): update order by clauses to include 'rank' for FTS queries Signed-off-by: Deluan <deluan@navidrome.org> * fix(search): reintroduce 'rank' in Phase 2 ORDER BY for FTS queries Signed-off-by: Deluan <deluan@navidrome.org> * fix(search): remove 'rank' from ORDER BY in non-FTS queries and adjust two-phase query handling Signed-off-by: Deluan <deluan@navidrome.org> * fix(search): update FTS ranking to use bm25 weights and simplify ORDER BY qualification Signed-off-by: Deluan <deluan@navidrome.org> * fix(search): refine FTS query handling and improve comments for clarity Signed-off-by: Deluan <deluan@navidrome.org> * fix(search): refactor full-text search handling to streamline query strategy selection and improve LIKE fallback logic. Increase e2e coverage for search3 Signed-off-by: Deluan <deluan@navidrome.org> * refactor: enhance FTS column definitions and relevance weights Signed-off-by: Deluan <deluan@navidrome.org> * fix(search): refactor Search method signatures to remove offset and size parameters, streamline query handling Signed-off-by: Deluan <deluan@navidrome.org> * fix(search): allow single-character queries in search strategies and update related tests Signed-off-by: Deluan <deluan@navidrome.org> * fix(search): make FTS Phase 1 treat Max=0 as no limit, reorganize tests FTS Phase 1 unconditionally called Limit(uint64(options.Max)), which produced LIMIT 0 when Max was zero. This diverged from applyOptions where Max=0 means no limit. Now Phase 1 mirrors applyOptions: only add LIMIT/OFFSET when the value is positive. Also moved legacy backend integration tests from sql_search_fts_test.go to sql_search_like_test.go and added regression tests for the Max=0 behavior on both backends. * refactor: simplify callSearch function by removing variadic options and directly using QueryOptions Signed-off-by: Deluan <deluan@navidrome.org> * fix(search): implement ftsQueryDegraded function to detect significant content loss in FTS queries Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org>	2026-02-23 08:51:54 -05:00
Deluan	d02bf9a53d	test(e2e): add MusicBrainz ID tests for song and album searches ... Signed-off-by: Deluan <deluan@navidrome.org>	2026-02-22 00:32:14 -05:00
Deluan	ec75808153	fix(subsonic): handle empty quoted phrases in FTS5 query and search expression ... Signed-off-by: Deluan <deluan@navidrome.org>	2026-02-21 22:00:00 -05:00
Deluan Quintão	7ad2907719	refactor: move playlist business logic from repositories to service layer (#5027) ... * refactor: move playlist business logic from repositories to core.Playlists service Move authorization, permission checks, and orchestration logic from playlist repositories to the core.Playlists service, following the existing pattern used by core.Share and core.Library. Changes: - Expand core.Playlists interface with read, mutation, track management, and REST adapter methods - Add playlistRepositoryWrapper for REST Save/Update/Delete with permission checks (follows Share/Library pattern) - Simplify persistence/playlist_repository.go: remove isWritable(), auth checks from Delete()/Put()/updatePlaylist() - Simplify persistence/playlist_track_repository.go: remove isTracksEditable() and permission checks from Add/Delete/Reorder - Update Subsonic API handlers to route through service - Update Native API handlers to accept core.Playlists instead of model.DataStore * test: add coverage for playlist service methods and REST wrapper Add 30 new tests covering the service methods added during the playlist refactoring: - Delete: owner, admin, denied, not found - Create: new playlist, replace tracks, admin bypass, denied, not found - AddTracks: owner, admin, denied, smart playlist, not found - RemoveTracks: owner, smart playlist denied, non-owner denied - ReorderTrack: owner, smart playlist denied - NewRepository wrapper: Save (owner assignment, ID clearing), Update (owner, admin, denied, ownership change, not found), Delete (delegation with permission checks) Expand mockedPlaylistRepo with Get, Delete, Tracks, GetWithTracks, and rest.Persistable methods. Add mockedPlaylistTrackRepo for track operation verification. * fix: add authorization check to playlist Update method Added ownership verification to the Subsonic Update endpoint in the playlist service layer. The authorization check was present in the old repository code but was not carried over during the refactoring to the service layer, allowing any authenticated user to modify playlists they don't own via the Subsonic API. Also added corresponding tests for the Update method's permission logic. * refactor: improve playlist permission checks and error handling, add e2e tests Signed-off-by: Deluan <deluan@navidrome.org> * refactor: rename core.Playlists to playlists package and update references Signed-off-by: Deluan <deluan@navidrome.org> * refactor: rename playlists_internal_test.go to parse_m3u_test.go and update tests; add new parse_nsp.go and rest_adapter.go files Signed-off-by: Deluan <deluan@navidrome.org> * fix: block track mutations on smart playlists in Create and Update Create now rejects replacing tracks on smart playlists (pre-existing gap). Update now uses checkTracksEditable instead of checkWritable when track changes are requested, restoring the protection that was removed from the repository layer during the refactoring. Metadata-only updates on smart playlists remain allowed. * test: add smart playlist protection tests to ensure readonly behavior and mutation restrictions * refactor: optimize track removal and renumbering in playlists Signed-off-by: Deluan <deluan@navidrome.org> * refactor: implement track reordering in playlists with SQL updates Signed-off-by: Deluan <deluan@navidrome.org> * refactor: wrap track deletion and reordering in transactions for consistency Signed-off-by: Deluan <deluan@navidrome.org> * refactor: remove unused getTracks method from playlistTrackRepository Signed-off-by: Deluan <deluan@navidrome.org> * refactor: optimize playlist track renumbering with CTE-based UPDATE Replace the DELETE + re-INSERT renumbering strategy with a two-step UPDATE approach using a materialized CTE and ROW_NUMBER() window function. The previous approach (SELECT all IDs, DELETE all tracks, re-INSERT in chunks of 200) required 13 SQL operations for a 2000-track playlist. The new approach uses just 2 UPDATEs: first negating all IDs to clear the positive space, then assigning sequential positions via UPDATE...FROM with a CTE. This avoids the UNIQUE constraint violations that affected the original correlated subquery while reducing per-delete request time from ~110ms to ~12ms on a 2000-track playlist. Signed-off-by: Deluan <deluan@navidrome.org> * refactor: rename New function to NewPlaylists for clarity Signed-off-by: Deluan <deluan@navidrome.org> * refactor: update mock playlist repository and tests for consistency Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org>	2026-02-21 19:57:13 -05:00
Deluan Quintão	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>	2026-02-21 17:52:42 -05:00
Deluan	897de02a84	docs: documents how subsonic e2e tests are structured	2026-02-11 22:49:41 -05:00
Deluan	e05a7e230f	fix: prevent data race on conf.Server during cleanup in e2e tests ... Signed-off-by: Deluan <deluan@navidrome.org>	2026-02-10 11:25:17 -05:00
Deluan Quintão	8319905d2c	test(subsonic): add comprehensive e2e test suite for Subsonic API (#5003) ... * test(e2e): add comprehensive tests for Subsonic API endpoints Signed-off-by: Deluan <deluan@navidrome.org> * fix(e2e): improve database handling and snapshot restoration in tests Signed-off-by: Deluan <deluan@navidrome.org> * test(e2e): add tests for album sharing and user isolation scenarios Signed-off-by: Deluan <deluan@navidrome.org> * test(e2e): add tests for multi-library support and user access control Signed-off-by: Deluan <deluan@navidrome.org> * test(e2e): tests are fast, no need to skip on -short Signed-off-by: Deluan <deluan@navidrome.org> * address gemini comments Signed-off-by: Deluan <deluan@navidrome.org> * fix(tests): prevent MockDataStore from caching repos with stale context When RealDS is set, MockDataStore previously cached repository instances on first access, binding them to the initial caller's context. This meant repos created with an admin context would skip library filtering for all subsequent non-admin calls, silently masking access control bugs. Changed MockDataStore to delegate to RealDS on every call without caching, so each caller gets a fresh repo with the correct context. Removed the pre-warm calls in e2e setupTestDB that were working around the old caching behavior. * test(e2e): route subsonic tests through full HTTP middleware stack Replace direct router method calls with full HTTP round-trips via router.ServeHTTP(w, r) across all 15 e2e test files. Tests now exercise the complete chi middleware chain including postFormToQueryParams, checkRequiredParameters, authenticate, UpdateLastAccessMiddleware, getPlayer, and sendResponse/sendError serialization. New helpers (doReq, doReqWithUser, doRawReq, buildReq, parseJSONResponse) use plaintext password auth and JSON response format. Old helpers that injected context directly (newReq, newReqWithUser, newRawReq) are removed. Sharing tests now set conf.Server.EnableSharing before router creation to ensure sharing routes are registered. --------- Signed-off-by: Deluan <deluan@navidrome.org>	2026-02-09 08:24:37 -05:00