From c4c70519b5f3a2a6eaf01f5786754d899cda9bfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sun, 14 Jun 2026 16:52:01 -0400 Subject: [PATCH] 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 * fix(ui): clean up comments in PlayerEdit component Signed-off-by: Deluan * 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 --- core/stream/legacy_client.go | 20 +++++-- core/stream/legacy_client_test.go | 86 ++++++++++++++++++++++++--- core/stream/types.go | 19 ++++++ core/stream/types_test.go | 59 ++++++++++++++++++ resources/i18n/pt-br.json | 7 ++- server/e2e/subsonic_transcode_test.go | 46 +++++++------- server/subsonic/transcode.go | 10 ++++ server/subsonic/transcode_test.go | 71 ++++++++++++++++++++++ ui/src/i18n/en.json | 3 + ui/src/player/PlayerEdit.jsx | 33 +++++++--- ui/src/player/PlayerEdit.test.jsx | 54 +++++++++++++++++ 11 files changed, 364 insertions(+), 44 deletions(-) create mode 100644 core/stream/types_test.go create mode 100644 ui/src/player/PlayerEdit.test.jsx diff --git a/core/stream/legacy_client.go b/core/stream/legacy_client.go index 9dd6179a0..652e42eba 100644 --- a/core/stream/legacy_client.go +++ b/core/stream/legacy_client.go @@ -12,7 +12,7 @@ import ( // buildLegacyClientInfo translates legacy Subsonic stream/download parameters // into a ClientInfo for use with MakeDecision. -func buildLegacyClientInfo(mf *model.MediaFile, reqFormat string, reqBitRate int) *ClientInfo { +func buildLegacyClientInfo(mf *model.MediaFile, reqFormat string, reqBitRate int, playerMaxBitRate int) *ClientInfo { ci := &ClientInfo{Name: "legacy"} // Determine target format for transcoding @@ -22,6 +22,10 @@ func buildLegacyClientInfo(mf *model.MediaFile, reqFormat string, reqBitRate int targetFormat = reqFormat case reqBitRate > 0 && reqBitRate < mf.BitRate && conf.Server.DefaultDownsamplingFormat != "": targetFormat = conf.Server.DefaultDownsamplingFormat + case playerMaxBitRate > 0 && playerMaxBitRate < mf.BitRate && conf.Server.DefaultDownsamplingFormat != "": + // Server-side player MaxBitRate alone forces downsampling, even when the + // client sent no format/bitrate params (issue #5583, legacy /stream path). + targetFormat = conf.Server.DefaultDownsamplingFormat } if targetFormat != "" { @@ -63,15 +67,19 @@ func (s *deciderService) ResolveRequest(ctx context.Context, mf *model.MediaFile return req } - clientInfo := buildLegacyClientInfo(mf, reqFormat, reqBitRate) + playerMaxBitRate := 0 + if player, ok := request.PlayerFrom(ctx); ok { + playerMaxBitRate = player.MaxBitRate + } + + clientInfo := buildLegacyClientInfo(mf, reqFormat, reqBitRate, playerMaxBitRate) // Apply server-side player transcoding override before making the decision if trc, ok := request.TranscodingFrom(ctx); ok && trc.TargetFormat != "" { clientInfo = applyServerOverride(ctx, clientInfo, &trc) - } else if player, ok := request.PlayerFrom(ctx); ok && player.MaxBitRate > 0 { - if clientInfo.MaxAudioBitrate == 0 || player.MaxBitRate < clientInfo.MaxAudioBitrate { - modified := *clientInfo - modified.MaxAudioBitrate = player.MaxBitRate + } else if player, ok := request.PlayerFrom(ctx); ok { + modified := *clientInfo + if modified.CapBitrate(player.MaxBitRate) { clientInfo = &modified log.Debug(ctx, "Applied player MaxBitRate cap", "playerMaxBitRate", player.MaxBitRate, "client", clientInfo.Name) } diff --git a/core/stream/legacy_client_test.go b/core/stream/legacy_client_test.go index 2ddd74ce0..bc8405976 100644 --- a/core/stream/legacy_client_test.go +++ b/core/stream/legacy_client_test.go @@ -21,7 +21,7 @@ var _ = Describe("buildLegacyClientInfo", func() { }) It("sets transcoding profile for explicit format without bitrate", func() { - ci := buildLegacyClientInfo(mf, "mp3", 0) + ci := buildLegacyClientInfo(mf, "mp3", 0, 0) Expect(ci.Name).To(Equal("legacy")) Expect(ci.TranscodingProfiles).To(HaveLen(1)) @@ -34,7 +34,7 @@ var _ = Describe("buildLegacyClientInfo", func() { }) It("does not add direct play profile when explicit format differs from source (no bitrate)", func() { - ci := buildLegacyClientInfo(mf, "opus", 0) + ci := buildLegacyClientInfo(mf, "opus", 0, 0) Expect(ci.TranscodingProfiles).To(HaveLen(1)) Expect(ci.TranscodingProfiles[0].Container).To(Equal("opus")) @@ -42,7 +42,7 @@ var _ = Describe("buildLegacyClientInfo", func() { }) It("adds direct play profile when explicit format matches source format", func() { - ci := buildLegacyClientInfo(mf, "flac", 0) + ci := buildLegacyClientInfo(mf, "flac", 0, 0) Expect(ci.TranscodingProfiles).To(HaveLen(1)) Expect(ci.TranscodingProfiles[0].Container).To(Equal("flac")) @@ -52,7 +52,7 @@ var _ = Describe("buildLegacyClientInfo", func() { }) It("sets transcoding profile and bitrate for explicit format with bitrate", func() { - ci := buildLegacyClientInfo(mf, "mp3", 192) + ci := buildLegacyClientInfo(mf, "mp3", 192, 0) Expect(ci.TranscodingProfiles).To(HaveLen(1)) Expect(ci.TranscodingProfiles[0].Container).To(Equal("mp3")) @@ -63,7 +63,7 @@ var _ = Describe("buildLegacyClientInfo", func() { }) It("returns direct play profile when no format and no bitrate", func() { - ci := buildLegacyClientInfo(mf, "", 0) + ci := buildLegacyClientInfo(mf, "", 0, 0) Expect(ci.DirectPlayProfiles).To(HaveLen(1)) Expect(ci.DirectPlayProfiles[0].Containers).To(BeEmpty()) @@ -77,7 +77,7 @@ var _ = Describe("buildLegacyClientInfo", func() { DeferCleanup(configtest.SetupConfig()) conf.Server.DefaultDownsamplingFormat = "opus" - ci := buildLegacyClientInfo(mf, "", 128) + ci := buildLegacyClientInfo(mf, "", 128, 0) Expect(ci.TranscodingProfiles).To(HaveLen(1)) Expect(ci.TranscodingProfiles[0].Container).To(Equal("opus")) @@ -91,7 +91,7 @@ var _ = Describe("buildLegacyClientInfo", func() { }) It("returns direct play when bitrate >= source bitrate", func() { - ci := buildLegacyClientInfo(mf, "", 960) + ci := buildLegacyClientInfo(mf, "", 960, 0) Expect(ci.DirectPlayProfiles).To(HaveLen(1)) Expect(ci.DirectPlayProfiles[0].Containers).To(BeEmpty()) @@ -100,6 +100,51 @@ var _ = Describe("buildLegacyClientInfo", func() { Expect(ci.TranscodingProfiles).To(BeEmpty()) Expect(ci.MaxAudioBitrate).To(BeZero()) }) + + It("uses default downsampling format when player MaxBitRate is below source and no format/bitrate", func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.DefaultDownsamplingFormat = "opus" + + ci := buildLegacyClientInfo(mf, "", 0, 256) + + Expect(ci.TranscodingProfiles).To(HaveLen(1)) + Expect(ci.TranscodingProfiles[0].Container).To(Equal("opus")) + Expect(ci.TranscodingProfiles[0].AudioCodec).To(Equal("opus")) + Expect(ci.DirectPlayProfiles).To(HaveLen(1)) + Expect(ci.DirectPlayProfiles[0].Containers).To(Equal([]string{"flac"})) + }) + + It("does not downsample when player MaxBitRate is >= source bitrate", func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.DefaultDownsamplingFormat = "opus" + + ci := buildLegacyClientInfo(mf, "", 0, 960) + + Expect(ci.TranscodingProfiles).To(BeEmpty()) + Expect(ci.DirectPlayProfiles).To(HaveLen(1)) + Expect(ci.DirectPlayProfiles[0].Containers).To(BeEmpty()) + }) + + It("does not downsample when DefaultDownsamplingFormat is empty even with player cap", func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.DefaultDownsamplingFormat = "" + + ci := buildLegacyClientInfo(mf, "", 0, 256) + + Expect(ci.TranscodingProfiles).To(BeEmpty()) + Expect(ci.DirectPlayProfiles).To(HaveLen(1)) + Expect(ci.DirectPlayProfiles[0].Containers).To(BeEmpty()) + }) + + It("prefers explicit request format over player cap", func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.DefaultDownsamplingFormat = "opus" + + ci := buildLegacyClientInfo(mf, "mp3", 0, 256) + + Expect(ci.TranscodingProfiles).To(HaveLen(1)) + Expect(ci.TranscodingProfiles[0].Container).To(Equal("mp3")) + }) }) var _ = Describe("ResolveRequest", func() { @@ -289,6 +334,33 @@ var _ = Describe("ResolveRequest", func() { Expect(req.Format).To(Equal("raw")) }) + + It("downsamples using DefaultDownsamplingFormat when only player MaxBitRate is set (no format/bitrate)", func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.DefaultDownsamplingFormat = "opus" + + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) + playerCtx := request.WithPlayer(ctx, model.Player{MaxBitRate: 256}) + + decider := svc.(*deciderService) + req := decider.ResolveRequest(playerCtx, mf, "", 0, 0) + + Expect(req.Format).To(Equal("opus")) + Expect(req.BitRate).To(Equal(256)) + }) + + It("serves raw when only player MaxBitRate is set but no DefaultDownsamplingFormat", func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.DefaultDownsamplingFormat = "" + + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: new(16)}) + playerCtx := request.WithPlayer(ctx, model.Player{MaxBitRate: 256}) + + decider := svc.(*deciderService) + req := decider.ResolveRequest(playerCtx, mf, "", 0, 0) + + Expect(req.Format).To(Equal("raw")) + }) }) Context("fallback for unknown format", func() { diff --git a/core/stream/types.go b/core/stream/types.go index bd8ce292c..11642c11c 100644 --- a/core/stream/types.go +++ b/core/stream/types.go @@ -40,6 +40,25 @@ type ClientInfo struct { CodecProfiles []CodecProfile } +// CapBitrate lowers the client's declared audio bitrate limits to maxKbps, +// never raising them. A zero limit means "unlimited" and is set to maxKbps. +// Returns true if anything changed. No-op when maxKbps <= 0. +func (ci *ClientInfo) CapBitrate(maxKbps int) bool { + if maxKbps <= 0 { + return false + } + changed := false + if ci.MaxAudioBitrate == 0 || maxKbps < ci.MaxAudioBitrate { + ci.MaxAudioBitrate = maxKbps + changed = true + } + if ci.MaxTranscodingAudioBitrate == 0 || maxKbps < ci.MaxTranscodingAudioBitrate { + ci.MaxTranscodingAudioBitrate = maxKbps + changed = true + } + return changed +} + // DirectPlayProfile describes a format the client can play directly type DirectPlayProfile struct { Containers []string diff --git a/core/stream/types_test.go b/core/stream/types_test.go new file mode 100644 index 000000000..2d2a83d06 --- /dev/null +++ b/core/stream/types_test.go @@ -0,0 +1,59 @@ +package stream + +import ( + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("ClientInfo", func() { + Describe("CapBitrate", func() { + It("is a no-op when maxKbps is zero", func() { + ci := &ClientInfo{MaxAudioBitrate: 320, MaxTranscodingAudioBitrate: 320} + Expect(ci.CapBitrate(0)).To(BeFalse()) + Expect(ci.MaxAudioBitrate).To(Equal(320)) + Expect(ci.MaxTranscodingAudioBitrate).To(Equal(320)) + }) + + It("is a no-op when maxKbps is negative", func() { + ci := &ClientInfo{MaxAudioBitrate: 320, MaxTranscodingAudioBitrate: 320} + Expect(ci.CapBitrate(-1)).To(BeFalse()) + Expect(ci.MaxAudioBitrate).To(Equal(320)) + Expect(ci.MaxTranscodingAudioBitrate).To(Equal(320)) + }) + + It("sets both limits when both are zero (unlimited)", func() { + ci := &ClientInfo{} + Expect(ci.CapBitrate(256)).To(BeTrue()) + Expect(ci.MaxAudioBitrate).To(Equal(256)) + Expect(ci.MaxTranscodingAudioBitrate).To(Equal(256)) + }) + + It("lowers limits higher than maxKbps", func() { + ci := &ClientInfo{MaxAudioBitrate: 320, MaxTranscodingAudioBitrate: 500} + Expect(ci.CapBitrate(192)).To(BeTrue()) + Expect(ci.MaxAudioBitrate).To(Equal(192)) + Expect(ci.MaxTranscodingAudioBitrate).To(Equal(192)) + }) + + It("does not raise limits lower than maxKbps", func() { + ci := &ClientInfo{MaxAudioBitrate: 128, MaxTranscodingAudioBitrate: 96} + Expect(ci.CapBitrate(320)).To(BeFalse()) + Expect(ci.MaxAudioBitrate).To(Equal(128)) + Expect(ci.MaxTranscodingAudioBitrate).To(Equal(96)) + }) + + It("reports changed when only one limit is lowered", func() { + ci := &ClientInfo{MaxAudioBitrate: 320, MaxTranscodingAudioBitrate: 128} + Expect(ci.CapBitrate(192)).To(BeTrue()) + Expect(ci.MaxAudioBitrate).To(Equal(192)) + Expect(ci.MaxTranscodingAudioBitrate).To(Equal(128)) + }) + + It("caps only the zero (unlimited) limit", func() { + ci := &ClientInfo{MaxAudioBitrate: 128, MaxTranscodingAudioBitrate: 0} + Expect(ci.CapBitrate(192)).To(BeTrue()) + Expect(ci.MaxAudioBitrate).To(Equal(128)) + Expect(ci.MaxTranscodingAudioBitrate).To(Equal(192)) + }) + }) +}) diff --git a/resources/i18n/pt-br.json b/resources/i18n/pt-br.json index d9f29f5d4..bc4d149a1 100644 --- a/resources/i18n/pt-br.json +++ b/resources/i18n/pt-br.json @@ -4,7 +4,7 @@ "song": { "name": "Música |||| Músicas", "fields": { - "albumArtist": "Artista", + "albumArtist": "Artista do Álbum", "duration": "Duração", "trackNumber": "#", "playCount": "Execuções", @@ -57,7 +57,7 @@ "album": { "name": "Álbum |||| Álbuns", "fields": { - "albumArtist": "Artista", + "albumArtist": "Artista do Álbum", "artist": "Artista", "duration": "Duração", "songCount": "Músicas", @@ -187,6 +187,9 @@ "lastSeen": "Últ. acesso", "reportRealPath": "Use paths reais", "scrobbleEnabled": "Enviar scrobbles para serviços externos" + }, + "helperTexts": { + "transcodingId": "O player web ignora o formato de conversão e aplica apenas o limite de Bitrate máx." } }, "transcoding": { diff --git a/server/e2e/subsonic_transcode_test.go b/server/e2e/subsonic_transcode_test.go index ae3d6208c..c769fd2d4 100644 --- a/server/e2e/subsonic_transcode_test.go +++ b/server/e2e/subsonic_transcode_test.go @@ -396,30 +396,34 @@ var _ = Describe("Transcode Endpoints", Ordered, func() { }) }) - Describe("player MaxBitRate cap is ignored", func() { - It("allows direct play even when source bitrate exceeds player MaxBitRate", func() { + Describe("player MaxBitRate cap is enforced", func() { + It("forces transcode when source bitrate exceeds player MaxBitRate", func() { setPlayerMaxBitRate(320) // 320 kbps cap - // FLAC is 900kbps, player cap is 320, but getTranscodeDecision - // ignores server-side overrides — client profiles are used as-is + // FLAC is 900kbps. Player cap (320) < source → direct play is + // rejected and the file is transcoded down. resp := doPostReq("getTranscodeDecision", flacAndMp3Client, "mediaId", flacTrackID, "mediaType", "song") Expect(resp.Status).To(Equal(responses.StatusOK)) Expect(resp.TranscodeDecision).ToNot(BeNil()) - Expect(resp.TranscodeDecision.CanDirectPlay).To(BeTrue()) + Expect(resp.TranscodeDecision.CanDirectPlay).To(BeFalse()) + Expect(resp.TranscodeDecision.CanTranscode).To(BeTrue()) + Expect(resp.TranscodeDecision.TranscodeStream).ToNot(BeNil()) + // Target bitrate is capped at the player MaxBitRate (320kbps). + Expect(resp.TranscodeDecision.TranscodeStream.AudioBitrate).To(Equal(int32(320000))) }) - It("uses only client limit, not player MaxBitRate", func() { + It("uses the player cap when it is more restrictive than the client limit", func() { setPlayerMaxBitRate(192) // 192 kbps player cap - // Client caps at 320kbps (bitrateCapClient), player is more restrictive at 192 - // but getTranscodeDecision ignores player cap → client limit (320kbps) applies + // Client caps at 320kbps (bitrateCapClient); player is more + // restrictive at 192 → player cap wins. resp := doPostReq("getTranscodeDecision", bitrateCapClient, "mediaId", flacTrackID, "mediaType", "song") Expect(resp.Status).To(Equal(responses.StatusOK)) Expect(resp.TranscodeDecision).ToNot(BeNil()) Expect(resp.TranscodeDecision.CanTranscode).To(BeTrue()) Expect(resp.TranscodeDecision.TranscodeStream).ToNot(BeNil()) - // Only client limit (320kbps) applies → 320000 bps - Expect(resp.TranscodeDecision.TranscodeStream.AudioBitrate).To(Equal(int32(320000))) + // Player cap (192kbps) applies → 192000 bps. + Expect(resp.TranscodeDecision.TranscodeStream.AudioBitrate).To(Equal(int32(192000))) }) }) @@ -475,35 +479,33 @@ var _ = Describe("Transcode Endpoints", Ordered, func() { }) }) - Describe("player MaxBitRate is ignored by getTranscodeDecision", func() { - It("does not inject maxAudioBitrate from player cap", func() { + Describe("player MaxBitRate injected by getTranscodeDecision", func() { + It("injects the player cap as the transcode target when the client declares none", func() { setPlayerMaxBitRate(320) - // opusTranscodeClient has no client bitrate limits - // Player cap is 320, but getTranscodeDecision ignores it - // FLAC (900kbps) → can't direct play → transcode to opus using format default + // opusTranscodeClient has no client bitrate limits. The player + // cap (320) is injected, so FLAC (900kbps) → opus is capped at 320. resp := doPostReq("getTranscodeDecision", opusTranscodeClient, "mediaId", flacTrackID, "mediaType", "song") Expect(resp.Status).To(Equal(responses.StatusOK)) Expect(resp.TranscodeDecision).ToNot(BeNil()) Expect(resp.TranscodeDecision.CanTranscode).To(BeTrue()) Expect(resp.TranscodeDecision.TranscodeStream).ToNot(BeNil()) Expect(resp.TranscodeDecision.TranscodeStream.Codec).To(Equal("opus")) - // Bitrate should be opus format default (128kbps), not player cap (320kbps) - Expect(resp.TranscodeDecision.TranscodeStream.AudioBitrate).To(Equal(int32(128000))) + // Bitrate is the player cap (320kbps), not the opus format default. + Expect(resp.TranscodeDecision.TranscodeStream.AudioBitrate).To(Equal(int32(320000))) }) - It("uses only client maxTranscodingAudioBitrate, ignoring player cap", func() { + It("keeps the lower client maxTranscodingAudioBitrate over a higher player cap", func() { setPlayerMaxBitRate(320) - // maxTranscodeBitrateClient: maxTranscodingAudioBitrate=192000 (192kbps) - // Player cap is 320, but getTranscodeDecision ignores it - // Only client maxTranscodingAudioBitrate=192 applies + // maxTranscodeBitrateClient: maxTranscodingAudioBitrate=192000 (192kbps). + // Player cap (320) is higher → the lower client limit wins. resp := doPostReq("getTranscodeDecision", maxTranscodeBitrateClient, "mediaId", flacTrackID, "mediaType", "song") Expect(resp.Status).To(Equal(responses.StatusOK)) Expect(resp.TranscodeDecision).ToNot(BeNil()) Expect(resp.TranscodeDecision.CanTranscode).To(BeTrue()) Expect(resp.TranscodeDecision.TranscodeStream).ToNot(BeNil()) - // maxTranscodingAudioBitrate=192 → 192000 bps + // Client limit (192kbps) wins → 192000 bps. Expect(resp.TranscodeDecision.TranscodeStream.AudioBitrate).To(Equal(int32(192000))) }) }) diff --git a/server/subsonic/transcode.go b/server/subsonic/transcode.go index 578ad44fc..c95c25cb0 100644 --- a/server/subsonic/transcode.go +++ b/server/subsonic/transcode.go @@ -11,6 +11,7 @@ import ( "github.com/navidrome/navidrome/core/stream" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/server/subsonic/responses" "github.com/navidrome/navidrome/utils/req" ) @@ -278,6 +279,15 @@ func (api *Router) GetTranscodeDecision(w http.ResponseWriter, r *http.Request) return stream.IsAACCodec(p.Container) }) + // 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) + } + // Get media file mf, err := api.ds.MediaFile(ctx).Get(mediaID) if err != nil { diff --git a/server/subsonic/transcode_test.go b/server/subsonic/transcode_test.go index 29a883ac1..4a1017752 100644 --- a/server/subsonic/transcode_test.go +++ b/server/subsonic/transcode_test.go @@ -9,6 +9,7 @@ import ( "github.com/navidrome/navidrome/core/stream" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" @@ -234,6 +235,76 @@ var _ = Describe("Transcode endpoints", func() { Expect(resp.TranscodeDecision.TranscodeStream).ToNot(BeNil()) Expect(resp.TranscodeDecision.TranscodeStream.Container).To(Equal("mp3")) }) + + Describe("player MaxBitRate cap", func() { + withPlayer := func(r *http.Request, maxBitRate int) *http.Request { + ctx := request.WithPlayer(r.Context(), model.Player{Client: "NavidromeUI", MaxBitRate: maxBitRate}) + return r.WithContext(ctx) + } + + BeforeEach(func() { + mockMFRepo.SetData(model.MediaFiles{ + {ID: "song-1", Suffix: "flac", Codec: "FLAC", BitRate: 900, Channels: 2, SampleRate: 44100}, + }) + mockTD.decision = &stream.TranscodeDecision{MediaID: "song-1", CanDirectPlay: true} + mockTD.token = "token" + }) + + It("caps client MaxAudioBitrate at the player MaxBitRate when client declares none", func() { + body := `{"directPlayProfiles":[{"containers":["flac"],"protocols":["http"]}]}` + r := withPlayer(newJSONPostRequest("mediaId=song-1&mediaType=song", body), 320) + + _, err := router.GetTranscodeDecision(w, r) + + Expect(err).ToNot(HaveOccurred()) + Expect(mockTD.capturedClient).ToNot(BeNil()) + Expect(mockTD.capturedClient.MaxAudioBitrate).To(Equal(320)) + Expect(mockTD.capturedClient.MaxTranscodingAudioBitrate).To(Equal(320)) + }) + + It("does not raise a lower client-declared limit", func() { + // Client declares 192 kbps (192000 bps); player cap is 320 — client wins. + body := `{"maxAudioBitrate":192000,"directPlayProfiles":[{"containers":["flac"],"protocols":["http"]}]}` + r := withPlayer(newJSONPostRequest("mediaId=song-1&mediaType=song", body), 320) + + _, err := router.GetTranscodeDecision(w, r) + + Expect(err).ToNot(HaveOccurred()) + Expect(mockTD.capturedClient.MaxAudioBitrate).To(Equal(192)) + }) + + It("lowers a higher client-declared limit to the player cap", func() { + // Client declares 320 kbps (320000 bps); player cap is 192 — player wins. + body := `{"maxAudioBitrate":320000,"directPlayProfiles":[{"containers":["flac"],"protocols":["http"]}]}` + r := withPlayer(newJSONPostRequest("mediaId=song-1&mediaType=song", body), 192) + + _, err := router.GetTranscodeDecision(w, r) + + Expect(err).ToNot(HaveOccurred()) + Expect(mockTD.capturedClient.MaxAudioBitrate).To(Equal(192)) + Expect(mockTD.capturedClient.MaxTranscodingAudioBitrate).To(Equal(192)) + }) + + It("does nothing when no player is in context", func() { + body := `{"maxAudioBitrate":320000,"directPlayProfiles":[{"containers":["flac"],"protocols":["http"]}]}` + r := newJSONPostRequest("mediaId=song-1&mediaType=song", body) + + _, err := router.GetTranscodeDecision(w, r) + + Expect(err).ToNot(HaveOccurred()) + Expect(mockTD.capturedClient.MaxAudioBitrate).To(Equal(320)) + }) + + It("does nothing when player MaxBitRate is 0", func() { + body := `{"maxAudioBitrate":320000,"directPlayProfiles":[{"containers":["flac"],"protocols":["http"]}]}` + r := withPlayer(newJSONPostRequest("mediaId=song-1&mediaType=song", body), 0) + + _, err := router.GetTranscodeDecision(w, r) + + Expect(err).ToNot(HaveOccurred()) + Expect(mockTD.capturedClient.MaxAudioBitrate).To(Equal(320)) + }) + }) }) Describe("GetTranscodeStream", func() { diff --git a/ui/src/i18n/en.json b/ui/src/i18n/en.json index 74fb23ab9..595b20a0d 100644 --- a/ui/src/i18n/en.json +++ b/ui/src/i18n/en.json @@ -187,6 +187,9 @@ "lastSeen": "Last Seen At", "reportRealPath": "Report Real Path", "scrobbleEnabled": "Send Scrobbles to external services" + }, + "helperTexts": { + "transcodingId": "The web player ignores the transcoding format and only enforces the Max. Bit Rate limit." } }, "transcoding": { diff --git a/ui/src/player/PlayerEdit.jsx b/ui/src/player/PlayerEdit.jsx index 1826500bd..d09ed855f 100644 --- a/ui/src/player/PlayerEdit.jsx +++ b/ui/src/player/PlayerEdit.jsx @@ -8,6 +8,7 @@ import { SelectInput, ReferenceInput, useTranslate, + useRecordContext, } from 'react-admin' import { Title } from '../common' import config from '../config' @@ -19,17 +20,35 @@ const PlayerTitle = ({ record }) => { return } +export const TranscodingInput = (props) => { + const translate = useTranslate() + const record = useRecordContext(props) + const isWebPlayer = record?.client === 'NavidromeUI' + return ( + <ReferenceInput + {...props} + source="transcodingId" + reference="transcoding" + sort={{ field: 'name', order: 'ASC' }} + > + <SelectInput + source="name" + resettable + helperText={ + isWebPlayer + ? translate('resources.player.helperTexts.transcodingId') + : undefined + } + /> + </ReferenceInput> + ) +} + const PlayerEdit = (props) => ( <Edit title={<PlayerTitle />} {...props}> <SimpleForm variant={'outlined'}> <TextInput source="name" validate={[required()]} /> - <ReferenceInput - source="transcodingId" - reference="transcoding" - sort={{ field: 'name', order: 'ASC' }} - > - <SelectInput source="name" resettable /> - </ReferenceInput> + <TranscodingInput /> <SelectInput source="maxBitRate" resettable choices={BITRATE_CHOICES} /> <BooleanInput source="reportRealPath" fullWidth /> {(config.lastFMEnabled || config.listenBrainzEnabled) && ( diff --git a/ui/src/player/PlayerEdit.test.jsx b/ui/src/player/PlayerEdit.test.jsx new file mode 100644 index 000000000..2b8c862e2 --- /dev/null +++ b/ui/src/player/PlayerEdit.test.jsx @@ -0,0 +1,54 @@ +import * as React from 'react' +import { render, screen, cleanup } from '@testing-library/react' +import { describe, it, expect, afterEach, vi, beforeEach } from 'vitest' +import { useRecordContext } from 'react-admin' +import { TranscodingInput } from './PlayerEdit' + +vi.mock('react-admin', async () => { + const actual = await vi.importActual('react-admin') + return { + ...actual, + useRecordContext: vi.fn(), + // Mock useTranslate to return the key verbatim so assertions don't depend + // on ra-core's out-of-provider translation behavior. + useTranslate: () => (key) => key, + // Render the inputs as simple stand-ins so we can read their props. + ReferenceInput: ({ children, variant }) => ( + <div data-testid="reference-input" data-variant={variant || ''}> + {children} + </div> + ), + SelectInput: ({ helperText }) => ( + <div data-testid="select-input" data-helpertext={helperText || ''} /> + ), + } +}) + +describe('<TranscodingInput />', () => { + beforeEach(() => { + useRecordContext.mockReset() + }) + afterEach(cleanup) + + it('shows helper text for the NavidromeUI player', () => { + useRecordContext.mockReturnValue({ client: 'NavidromeUI' }) + render(<TranscodingInput />) + expect(screen.getByTestId('select-input').dataset.helpertext).toBe( + 'resources.player.helperTexts.transcodingId', + ) + }) + + it('shows no helper text for other clients', () => { + useRecordContext.mockReturnValue({ client: 'DSub' }) + render(<TranscodingInput />) + expect(screen.getByTestId('select-input').dataset.helpertext).toBe('') + }) + + it('forwards the form variant injected by SimpleForm to the input', () => { + useRecordContext.mockReturnValue({ client: 'DSub' }) + render(<TranscodingInput variant="outlined" />) + expect(screen.getByTestId('reference-input').dataset.variant).toBe( + 'outlined', + ) + }) +})