From 664217f3f79e391522a2dc462d5ad3e12b016b4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sun, 5 Apr 2026 20:31:11 -0400 Subject: [PATCH] fix(transcoding): play WAV files directly in browsers instead of transcoding (#5309) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --------- Signed-off-by: Deluan --- core/stream/decider.go | 19 ++++++++--- core/stream/decider_test.go | 63 ++++++++++++++++++++++++++++++++----- core/stream/types.go | 13 ++++++++ 3 files changed, 84 insertions(+), 11 deletions(-) diff --git a/core/stream/decider.go b/core/stream/decider.go index 5cca0cb0f..6c1f06a06 100644 --- a/core/stream/decider.go +++ b/core/stream/decider.go @@ -195,6 +195,17 @@ func parseProbeData(data string) (*ffmpeg.AudioProbeResult, error) { return &result, nil } +// matchesPCMWAVBridge bridges Navidrome's internal "pcm" codec name with the +// "wav" codec name that browsers use to advertise audio/wav support. The match +// is scoped to WAV-container sources so AIFF files (which also normalize to +// codec "pcm" but use a different container) cannot false-match a codec-only +// ["wav"] profile. +func matchesPCMWAVBridge(src *Details, profile *DirectPlayProfile) bool { + return strings.EqualFold(src.Codec, "pcm") && + strings.EqualFold(src.Container, "wav") && + containsIgnoreCase(profile.AudioCodecs, "wav") +} + // checkDirectPlayProfile returns "" if the profile matches (direct play OK), // or a typed reason string if it doesn't match. func (s *deciderService) checkDirectPlayProfile(src *Details, profile *DirectPlayProfile, clientInfo *ClientInfo) string { @@ -205,17 +216,17 @@ func (s *deciderService) checkDirectPlayProfile(src *Details, profile *DirectPla // Check container if len(profile.Containers) > 0 && !matchesContainer(src.Container, profile.Containers) { - return "container not supported" + return fmt.Sprintf("container '%s' not supported by profile %s", src.Container, profile) } // Check codec - if len(profile.AudioCodecs) > 0 && !matchesCodec(src.Codec, profile.AudioCodecs) { - return "audio codec not supported" + if len(profile.AudioCodecs) > 0 && !matchesCodec(src.Codec, profile.AudioCodecs) && !matchesPCMWAVBridge(src, profile) { + return fmt.Sprintf("audio codec '%s' not supported by profile %s", src.Codec, profile) } // Check channels if profile.MaxAudioChannels > 0 && src.Channels > profile.MaxAudioChannels { - return "audio channels not supported" + return fmt.Sprintf("audio channels %d not supported by profile %s (max %d)", src.Channels, profile, profile.MaxAudioChannels) } // Check codec-specific limitations diff --git a/core/stream/decider_test.go b/core/stream/decider_test.go index 42ebd84f1..9eaa00990 100644 --- a/core/stream/decider_test.go +++ b/core/stream/decider_test.go @@ -76,7 +76,10 @@ var _ = Describe("Decider", func() { decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) Expect(err).ToNot(HaveOccurred()) Expect(decision.CanDirectPlay).To(BeFalse()) - Expect(decision.TranscodeReasons).To(ContainElement("container not supported")) + Expect(decision.TranscodeReasons).To(ContainElement(And( + ContainSubstring("container 'flac' not supported"), + ContainSubstring("[mp3]"), + ))) }) It("rejects direct play when codec doesn't match", func() { @@ -89,7 +92,10 @@ var _ = Describe("Decider", func() { decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) Expect(err).ToNot(HaveOccurred()) Expect(decision.CanDirectPlay).To(BeFalse()) - Expect(decision.TranscodeReasons).To(ContainElement("audio codec not supported")) + Expect(decision.TranscodeReasons).To(ContainElement(And( + ContainSubstring("audio codec 'alac' not supported"), + ContainSubstring("[m4a/aac]"), + ))) }) It("rejects direct play when channels exceed limit", func() { @@ -102,7 +108,44 @@ var _ = Describe("Decider", func() { decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) Expect(err).ToNot(HaveOccurred()) Expect(decision.CanDirectPlay).To(BeFalse()) - Expect(decision.TranscodeReasons).To(ContainElement("audio channels not supported")) + Expect(decision.TranscodeReasons).To(ContainElement(And( + ContainSubstring("audio channels 6 not supported"), + ContainSubstring("[flac]"), + ContainSubstring("(max 2)"), + ))) + }) + + It("accepts WAV source against a wav codec profile (pcm->wav bridge)", func() { + // ffprobe normalizes PCM variants (pcm_s16le etc) to codec "pcm", but + // browsers advertise WAV support as audioCodecs:["wav"] via audio/wav MIME. + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "wav", Codec: "pcm", BitRate: 1411, Channels: 2}) + ci := &ClientInfo{ + DirectPlayProfiles: []DirectPlayProfile{ + {Containers: []string{"wav"}, AudioCodecs: []string{"wav"}, Protocols: []string{ProtocolHTTP}}, + }, + } + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanDirectPlay).To(BeTrue()) + }) + + It("does not accept AIFF (pcm in non-wav container) against a wav codec profile", func() { + // AIFF files also normalize to codec="pcm" but use container="aiff". + // Without the container guard they would falsely match a codec-only + // ["wav"] profile and be direct-played as if they were WAV. + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "aiff", Codec: "pcm", BitRate: 1411, Channels: 2}) + ci := &ClientInfo{ + DirectPlayProfiles: []DirectPlayProfile{ + {AudioCodecs: []string{"wav"}, Protocols: []string{ProtocolHTTP}}, + }, + TranscodingProfiles: []Profile{ + {Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP}, + }, + } + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanDirectPlay).To(BeFalse()) + Expect(decision.TranscodeReasons).To(ContainElement(ContainSubstring("audio codec 'pcm'"))) }) It("handles container aliases (aac -> m4a)", func() { @@ -216,7 +259,10 @@ var _ = Describe("Decider", func() { Expect(decision.CanTranscode).To(BeTrue()) Expect(decision.TargetFormat).To(Equal("mp3")) Expect(decision.TargetBitrate).To(Equal(256)) // kbps - Expect(decision.TranscodeReasons).To(ContainElement("container not supported")) + Expect(decision.TranscodeReasons).To(ContainElement(And( + ContainSubstring("container 'flac' not supported"), + ContainSubstring("[mp3]"), + ))) }) It("rejects lossy to lossless transcoding", func() { @@ -901,9 +947,12 @@ var _ = Describe("Decider", func() { Expect(err).ToNot(HaveOccurred()) Expect(decision.CanDirectPlay).To(BeFalse()) Expect(decision.TranscodeReasons).To(HaveLen(3)) - Expect(decision.TranscodeReasons[0]).To(Equal("container not supported")) - Expect(decision.TranscodeReasons[1]).To(Equal("container not supported")) - Expect(decision.TranscodeReasons[2]).To(Equal("container not supported")) + Expect(decision.TranscodeReasons[0]).To(ContainSubstring("container 'ogg' not supported")) + Expect(decision.TranscodeReasons[0]).To(ContainSubstring("[flac]")) + Expect(decision.TranscodeReasons[1]).To(ContainSubstring("container 'ogg' not supported")) + Expect(decision.TranscodeReasons[1]).To(ContainSubstring("[mp3/mp3]")) + Expect(decision.TranscodeReasons[2]).To(ContainSubstring("container 'ogg' not supported")) + Expect(decision.TranscodeReasons[2]).To(ContainSubstring("[m4a,mp4/aac]")) }) }) diff --git a/core/stream/types.go b/core/stream/types.go index 0cb4ac47d..bd8ce292c 100644 --- a/core/stream/types.go +++ b/core/stream/types.go @@ -2,6 +2,7 @@ package stream import ( "errors" + "strings" "time" ) @@ -47,6 +48,18 @@ type DirectPlayProfile struct { MaxAudioChannels int } +func (p DirectPlayProfile) String() string { + containers := strings.Join(p.Containers, ",") + if containers == "" { + containers = "*" + } + codecs := strings.Join(p.AudioCodecs, ",") + if codecs == "" { + return "[" + containers + "]" + } + return "[" + containers + "/" + codecs + "]" +} + // Profile describes a transcoding target the client supports type Profile struct { Container string