mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-08 02:17:25 +02:00
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 <deluan@navidrome.org> * fix(ui): clean up comments in PlayerEdit component Signed-off-by: Deluan <deluan@navidrome.org> * 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 <deluan@navidrome.org>
This commit is contained in:
parent
2c90685bc2
commit
c4c70519b5
11 changed files with 364 additions and 44 deletions
|
|
@ -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)))
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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 {
|
||||
|
|
|
|||
|
|
@ -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() {
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue