Commit graph

6 commits

Author SHA1 Message Date
Deluan Quintão
664217f3f7 fix(transcoding): play WAV files directly in browsers instead of transcoding (#5309)
* fix: allow WAV direct play by aliasing pcm and wav codecs

WAV files were being transcoded to FLAC even when the browser declared
native WAV support. The backend normalizes ffprobe's pcm_s16le (and
similar PCM variants) to the internal codec name "pcm", while browsers
advertise WAV support as audioCodecs:["wav"] in their client profile.
The direct-play codec check compared these literally and rejected the
match with "audio codec not supported", forcing a needless FLAC
transcode.

Added {"pcm", "wav"} to codecAliasGroups so the matcher treats them
as equivalent. The container check runs first, so AIFF files (which also
normalize to codec "pcm" but use container "aiff") cannot
accidentally match a WAV direct-play profile.

* feat: include profile details in direct-play rejection reasons

The transcodeReason array returned by getTranscodeDecision previously
contained one generic string per failed DirectPlayProfile (e.g., five
copies of "container not supported"), making it hard to correlate a
reason with the profile that rejected the stream.

Each rejection reason now embeds the offending source value (in single
quotes) along with a compact representation of the full profile that
rejected it, rendered as [container/codec]. For example, clients with
two distinct ogg-container profiles (opus and vorbis) produced two
identical rejection strings; they now read "container 'wav' not
supported by profile [ogg/opus]" and "container 'wav' not supported
by profile [ogg/vorbis]", making each entry in the transcodeReason
array unique and self-describing.

A small describeProfile helper renders profiles as [container/codec]
(or [container] when no codec is constrained).

* refactor(stream): address code review — narrow pcm/wav match, tighten tests

Responds to reviewer feedback on the initial PR:

- Replace the symmetric pcm↔wav codec alias with a contextual
  isPCMInWAVMatch check in checkDirectPlayProfile. The alias
  unconditionally equated the two names in matchesCodec, which would
  let AIFF sources (also normalized to codec "pcm") falsely satisfy
  a codec-only ["wav"] direct-play profile that omitted containers.
  The new check additionally requires src.Container == "wav" before
  bridging the names, closing the false-positive path.

- Tighten the rejection-reason test assertions to verify the new
  formatted output (source value + profile descriptor) instead of
  just matching loose substrings like "container", preventing
  unrelated rejections from satisfying the expectations.

- Add coverage for the WAV→wav-codec acceptance path and for the
  AIFF-in-wav-codec-profile rejection path to pin down the contract
  of isPCMInWAVMatch.

* refactor(codec): rename isPCMInWAVMatch to matchesPCMWAVBridge for clarity

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-04-05 20:37:26 -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
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 Quintão
d8bc41fbb1
fix: use ADTS for AAC transcoding, temporarily exclude AAC from transcode decisions (#5167)
* fix: use ADTS format for AAC transcoding to avoid silent output on ffmpeg 8.0+

The fragmented MP4 muxer (`-f ipod -movflags frag_keyframe+empty_moov`)
produces corrupt/silent audio when ffmpeg pipes to stdout, confirmed on
ffmpeg 8.0+. The moof atom offset values are zeroed out in pipe mode,
causing AAC decoder errors. Switch to `-f adts` (raw AAC framing) which
works reliably via pipe and is widely supported by clients including
UPnP/Sonos devices.

* fix: exclude AAC from transcode decision, as it is not working for Sonos.

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>
2026-03-11 09:26:32 -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