mirror of
https://github.com/navidrome/navidrome.git
synced 2026-04-26 10:30:46 +00:00
fix(transcoding): play WAV files directly in browsers instead of transcoding (#5309)
Some checks are pending
Pipeline: Test, Lint, Build / Get version info (push) Waiting to run
Pipeline: Test, Lint, Build / Build (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Lint Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test JS code (push) Waiting to run
Pipeline: Test, Lint, Build / Lint i18n files (push) Waiting to run
Pipeline: Test, Lint, Build / Check Docker configuration (push) Waiting to run
Pipeline: Test, Lint, Build / Build-1 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-2 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-3 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-4 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-5 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-6 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-7 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-8 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-9 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-10 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to GHCR (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to Docker Hub (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Cleanup digest artifacts (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build Windows installers (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Package/Release (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Upload Linux PKG (push) Blocked by required conditions
Some checks are pending
Pipeline: Test, Lint, Build / Get version info (push) Waiting to run
Pipeline: Test, Lint, Build / Build (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Lint Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test Go code (push) Waiting to run
Pipeline: Test, Lint, Build / Test JS code (push) Waiting to run
Pipeline: Test, Lint, Build / Lint i18n files (push) Waiting to run
Pipeline: Test, Lint, Build / Check Docker configuration (push) Waiting to run
Pipeline: Test, Lint, Build / Build-1 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-2 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-3 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-4 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-5 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-6 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-7 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-8 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-9 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build-10 (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to GHCR (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Push to Docker Hub (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Cleanup digest artifacts (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Build Windows installers (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Package/Release (push) Blocked by required conditions
Pipeline: Test, Lint, Build / Upload Linux PKG (push) Blocked by required conditions
* 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>
This commit is contained in:
parent
991bd3ed21
commit
664217f3f7
3 changed files with 84 additions and 11 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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]"))
|
||||
})
|
||||
})
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue