diff --git a/core/stream/decider_test.go b/core/stream/decider_test.go index 577207636..01fef1249 100644 --- a/core/stream/decider_test.go +++ b/core/stream/decider_test.go @@ -1144,6 +1144,82 @@ var _ = Describe("Decider", func() { }) }) + Context("Player-forced format", func() { + symfonium := func() *ClientInfo { + return &ClientInfo{ + Name: "Symfonium", + DirectPlayProfiles: []DirectPlayProfile{ + {Containers: []string{"mp3", "flac", "ogg"}, Protocols: []string{ProtocolHTTP}}, + }, + TranscodingProfiles: []Profile{ + {Container: "flac", AudioCodec: "flac", Protocol: ProtocolHTTP}, + {Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP}, + }, + } + } + + It("direct plays a flac source forced to flac", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1026, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) + ci := symfonium() + Expect(ci.ForceFormat("flac")).To(BeTrue()) + + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanDirectPlay).To(BeTrue()) + }) + + It("still transcodes a 24-bit flac when the client caps bit depth", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 4600, Channels: 2, SampleRate: 96000, BitDepth: new(24)}) + ci := symfonium() + ci.CodecProfiles = []CodecProfile{{ + Type: CodecProfileTypeAudio, Name: "flac", + Limitations: []Limitation{{Name: LimitationAudioBitdepth, Comparison: ComparisonLessThanEqual, Values: []string{"16"}, Required: true}}, + }} + Expect(ci.ForceFormat("flac")).To(BeTrue()) + + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanDirectPlay).To(BeFalse()) + Expect(decision.CanTranscode).To(BeTrue()) + Expect(decision.TranscodeStream.BitDepth).To(Equal(16)) + }) + + It("still transcodes a 320 mp3 forced to mp3 at a lower bitrate", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "mp3", Codec: "MP3", BitRate: 320, Channels: 2, SampleRate: 44100}) + ci := symfonium() + Expect(ci.ForceFormat("mp3")).To(BeTrue()) + ci.CapBitrate(192) + + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanDirectPlay).To(BeFalse()) + Expect(decision.CanTranscode).To(BeTrue()) + Expect(decision.TargetBitrate).To(Equal(192)) + }) + + It("direct plays a 128 mp3 forced to mp3 at a higher bitrate", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "mp3", Codec: "MP3", BitRate: 128, Channels: 2, SampleRate: 44100}) + ci := symfonium() + Expect(ci.ForceFormat("mp3")).To(BeTrue()) + ci.CapBitrate(192) + + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanDirectPlay).To(BeTrue()) + }) + + It("transcodes a flac source forced to mp3", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1026, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) + ci := symfonium() + Expect(ci.ForceFormat("mp3")).To(BeTrue()) + + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanDirectPlay).To(BeFalse()) + Expect(decision.CanTranscode).To(BeTrue()) + Expect(decision.TargetFormat).To(Equal("mp3")) + }) + }) }) Describe("ensureProbed", func() { diff --git a/core/stream/types.go b/core/stream/types.go index 19474dd91..300017c01 100644 --- a/core/stream/types.go +++ b/core/stream/types.go @@ -59,28 +59,38 @@ func (ci *ClientInfo) CapBitrate(maxKbps int) bool { return changed } -// ForceFormat narrows the client to transcoding to targetFormat and suppresses -// direct play, but only if the client already declares a profile for that -// format. All matching profiles are kept so negotiation can still pick among -// them (e.g. by protocol). Returns false (no-op) when targetFormat is empty or -// unsupported. +// ForceFormat narrows the client to transcoding to targetFormat, but only if the +// client already declares a profile for it. All matching profiles are kept so +// negotiation can still pick among them (e.g. by protocol). Direct play is rebuilt +// from those profiles rather than dropped, since declaring a transcoding profile +// for a format is proof the client can play it. Returns false when unsupported. func (ci *ClientInfo) ForceFormat(targetFormat string) bool { if targetFormat == "" { return false } var matched []Profile + var directPlay []DirectPlayProfile for i := range ci.TranscodingProfiles { + p := &ci.TranscodingProfiles[i] // matchesContainer is alias-aware, so a forced "oga" (legacy Opus // target_format) still matches a resolved "opus" profile. - if _, format := resolveTargetFormat(&ci.TranscodingProfiles[i]); matchesContainer(format, []string{targetFormat}) { - matched = append(matched, ci.TranscodingProfiles[i]) + container, format := resolveTargetFormat(p) + if !matchesContainer(format, []string{targetFormat}) { + continue } + matched = append(matched, *p) + directPlay = append(directPlay, DirectPlayProfile{ + Containers: []string{container}, + AudioCodecs: []string{format}, + Protocols: []string{ProtocolHTTP}, + MaxAudioChannels: p.MaxAudioChannels, + }) } if len(matched) == 0 { return false } ci.TranscodingProfiles = matched - ci.DirectPlayProfiles = nil + ci.DirectPlayProfiles = directPlay return true } diff --git a/core/stream/types_test.go b/core/stream/types_test.go index eff408362..88ad904a7 100644 --- a/core/stream/types_test.go +++ b/core/stream/types_test.go @@ -58,7 +58,7 @@ var _ = Describe("ClientInfo", func() { }) Describe("ForceFormat", func() { - It("restricts to the forced format and clears direct play when supported", func() { + It("restricts direct play to the forced format when supported", func() { ci := &ClientInfo{ DirectPlayProfiles: []DirectPlayProfile{{Containers: []string{"flac"}, AudioCodecs: []string{"flac"}}}, TranscodingProfiles: []Profile{ @@ -71,7 +71,35 @@ var _ = Describe("ClientInfo", func() { Expect(ok).To(BeTrue()) Expect(ci.TranscodingProfiles).To(HaveLen(1)) Expect(ci.TranscodingProfiles[0].AudioCodec).To(Equal("opus")) - Expect(ci.DirectPlayProfiles).To(BeEmpty()) + Expect(ci.DirectPlayProfiles).To(ConsistOf(DirectPlayProfile{ + Containers: []string{"ogg"}, AudioCodecs: []string{"opus"}, Protocols: []string{ProtocolHTTP}, + })) + }) + + It("keeps direct play for a source already in the forced format", func() { + ci := &ClientInfo{ + DirectPlayProfiles: []DirectPlayProfile{{Containers: []string{"flac"}, AudioCodecs: []string{"flac"}}}, + TranscodingProfiles: []Profile{ + {Container: "flac", AudioCodec: "flac", Protocol: ProtocolHTTP}, + {Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP}, + }, + } + ok := ci.ForceFormat("flac") + Expect(ok).To(BeTrue()) + Expect(ci.DirectPlayProfiles).To(ConsistOf(DirectPlayProfile{ + Containers: []string{"flac"}, AudioCodecs: []string{"flac"}, Protocols: []string{ProtocolHTTP}, + })) + }) + + It("carries the channel limit of the forced profile into direct play", func() { + ci := &ClientInfo{ + TranscodingProfiles: []Profile{ + {Container: "flac", AudioCodec: "flac", Protocol: ProtocolHTTP, MaxAudioChannels: 2}, + }, + } + Expect(ci.ForceFormat("flac")).To(BeTrue()) + Expect(ci.DirectPlayProfiles).To(HaveLen(1)) + Expect(ci.DirectPlayProfiles[0].MaxAudioChannels).To(Equal(2)) }) It("matches a container-only forced format (mp3)", func() { diff --git a/server/subsonic/transcode.go b/server/subsonic/transcode.go index 9eb2af160..d64bce605 100644 --- a/server/subsonic/transcode.go +++ b/server/subsonic/transcode.go @@ -280,12 +280,19 @@ func (api *Router) GetTranscodeDecision(w http.ResponseWriter, r *http.Request) return stream.IsAACCodec(p.Container) }) + player, hasPlayer := request.PlayerFrom(ctx) + // Honor the player's forced transcoding format, falling back to normal // negotiation when the client can't play it (issue #5583). + maxBitRate := 0 if trc, ok := request.TranscodingFrom(ctx); ok && trc.TargetFormat != "" { - if !clientInfo.ForceFormat(trc.TargetFormat) { + if clientInfo.ForceFormat(trc.TargetFormat) { + // DirectPlayProfile carries no bitrate, so this ceiling is the only + // thing keeping an over-bitrate source out of direct play. + maxBitRate = trc.DefaultBitRate + } else { clientName := clientInfo.Name - if player, ok := request.PlayerFrom(ctx); ok && player.Client != "" { + if hasPlayer && player.Client != "" { clientName = player.Client } log.Debug(ctx, "Player forced format not supported by client; falling back to negotiation", @@ -293,13 +300,13 @@ func (api *Router) GetTranscodeDecision(w http.ResponseWriter, r *http.Request) } } - // Apply the player's MaxBitRate as a ceiling on the client's declared - // limits (issue #5583). Both fields are capped because the client sends - // them independently here; capping only MaxAudioBitrate would let an - // independent MaxTranscodingAudioBitrate slip through computeBitrate. - if player, ok := request.PlayerFrom(ctx); ok && clientInfo.CapBitrate(player.MaxBitRate) { - log.Debug(ctx, "Applied player MaxBitRate cap to transcode decision", - "playerMaxBitRate", player.MaxBitRate, "client", clientInfo.Name) + // The player's own MaxBitRate outranks the forced-format default (issue #5583). + if hasPlayer && player.MaxBitRate > 0 { + maxBitRate = player.MaxBitRate + } + if clientInfo.CapBitrate(maxBitRate) { + log.Debug(ctx, "Applied bitrate ceiling to transcode decision", + "maxBitRate", maxBitRate, "client", clientInfo.Name) } // Get media file diff --git a/server/subsonic/transcode_test.go b/server/subsonic/transcode_test.go index 8d5cbb974..0f3c24832 100644 --- a/server/subsonic/transcode_test.go +++ b/server/subsonic/transcode_test.go @@ -369,7 +369,7 @@ var _ = Describe("Transcode endpoints", func() { mockTD.token = "token" }) - It("forces a supported format and clears direct play", func() { + It("forces a supported format and narrows direct play to it", func() { body := `{"directPlayProfiles":[{"containers":["flac"],"audioCodecs":["flac"],"protocols":["http"]}], "transcodingProfiles":[{"container":"ogg","audioCodec":"opus","protocol":"http"}, {"container":"mp3","audioCodec":"mp3","protocol":"http"}]}` @@ -380,7 +380,11 @@ var _ = Describe("Transcode endpoints", func() { Expect(err).ToNot(HaveOccurred()) Expect(mockTD.capturedClient.TranscodingProfiles).To(HaveLen(1)) Expect(mockTD.capturedClient.TranscodingProfiles[0].AudioCodec).To(Equal("opus")) - Expect(mockTD.capturedClient.DirectPlayProfiles).To(BeEmpty()) + Expect(mockTD.capturedClient.DirectPlayProfiles).To(ConsistOf(stream.DirectPlayProfile{ + Containers: []string{"ogg"}, + AudioCodecs: []string{"opus"}, + Protocols: []string{"http"}, + })) }) It("falls back to negotiation when the forced format is unsupported", func() { @@ -416,6 +420,43 @@ var _ = Describe("Transcode endpoints", func() { Expect(mockTD.capturedClient.MaxAudioBitrate).To(Equal(128)) Expect(mockTD.capturedClient.MaxTranscodingAudioBitrate).To(Equal(128)) }) + + withForcedBitRate := func(r *http.Request, format string, defaultBitRate, playerMaxBitRate int) *http.Request { + ctx := request.WithTranscoding(r.Context(), model.Transcoding{TargetFormat: format, DefaultBitRate: defaultBitRate}) + ctx = request.WithPlayer(ctx, model.Player{Client: "NavidromeUI", MaxBitRate: playerMaxBitRate}) + return r.WithContext(ctx) + } + + It("applies the transcoding default bitrate when the player sets no maxBitRate", func() { + body := `{"transcodingProfiles":[{"container":"mp3","audioCodec":"mp3","protocol":"http"}]}` + r := withForcedBitRate(newJSONPostRequest("mediaId=song-1&mediaType=song", body), "mp3", 192, 0) + + _, err := router.GetTranscodeDecision(w, r) + + Expect(err).ToNot(HaveOccurred()) + Expect(mockTD.capturedClient.MaxAudioBitrate).To(Equal(192)) + Expect(mockTD.capturedClient.MaxTranscodingAudioBitrate).To(Equal(192)) + }) + + It("prefers the player maxBitRate over the transcoding default bitrate", func() { + body := `{"transcodingProfiles":[{"container":"mp3","audioCodec":"mp3","protocol":"http"}]}` + r := withForcedBitRate(newJSONPostRequest("mediaId=song-1&mediaType=song", body), "mp3", 192, 320) + + _, err := router.GetTranscodeDecision(w, r) + + Expect(err).ToNot(HaveOccurred()) + Expect(mockTD.capturedClient.MaxAudioBitrate).To(Equal(320)) + }) + + It("ignores the transcoding default bitrate when the forced format is unsupported", func() { + body := `{"transcodingProfiles":[{"container":"mp3","audioCodec":"mp3","protocol":"http"}]}` + r := withForcedBitRate(newJSONPostRequest("mediaId=song-1&mediaType=song", body), "opus", 192, 0) + + _, err := router.GetTranscodeDecision(w, r) + + Expect(err).ToNot(HaveOccurred()) + Expect(mockTD.capturedClient.MaxAudioBitrate).To(BeZero()) + }) }) })