fix: player MaxBitRate cap, format-aware defaults, browser profile filtering (#5165)
* feat(transcode): apply player MaxBitRate cap and use format-aware default bitrates Add player MaxBitRate cap to the transcode decider so server-side player bitrate limits are respected when making OpenSubsonic transcode decisions. The player cap is applied only when it is more restrictive than the client's maxAudioBitrate (or when the client has no limit). Also replace the hardcoded 256 kbps default with a format-aware lookup that checks the DB first (for user-customized values), then built-in defaults, and finally falls back to 256 kbps. For lossless→lossy transcoding, prefer maxTranscodingAudioBitrate over maxAudioBitrate when available. * test(e2e): add tests for player MaxBitRate cap and format-aware default bitrates Add e2e tests covering: - Player MaxBitRate forcing transcode when source exceeds cap - Player MaxBitRate having no effect when source is under cap - Client limit winning when more restrictive than player MaxBitRate - Player MaxBitRate winning when more restrictive than client limit - Player MaxBitRate=0 having no effect - Format-aware defaults: mp3 (192kbps), opus (128kbps) instead of hardcoded 256 - maxAudioBitrate fallback for lossless→lossy when no maxTranscodingAudioBitrate - maxTranscodingAudioBitrate taking priority over maxAudioBitrate - Combined player + client limits flowing correctly through decision→stream * feat(transcode): update transcoding profiles to add flac, filter by supported codecs, and ensure mp3 fallback Signed-off-by: Deluan <deluan@navidrome.org> * fix(db): ensure all default transcodings exist on upgrade Older installations that were seeded before aac/flac were added to DefaultTranscodings may be missing these entries. The previous migration only added flac; this one ensures all default transcodings are present without touching user-customized entries. * test: remove duplication Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
@@ -14,7 +14,7 @@ import (
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
)
|
||||
|
||||
const defaultBitrate = 256 // kbps
|
||||
const fallbackBitrate = 256 // kbps
|
||||
|
||||
// Decider is the core service interface for making transcoding decisions
|
||||
type Decider interface {
|
||||
@@ -58,6 +58,13 @@ func (s *deciderService) MakeDecision(ctx context.Context, mf *model.MediaFile,
|
||||
// Check for server-side player transcoding override
|
||||
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
|
||||
clientInfo = &modified
|
||||
log.Debug(ctx, "Applied player MaxBitRate cap", "playerMaxBitRate", player.MaxBitRate, "client", clientInfo.Name)
|
||||
}
|
||||
}
|
||||
|
||||
log.Trace(ctx, "Making transcode decision", "mediaID", mf.ID, "container", src.Container,
|
||||
@@ -291,6 +298,21 @@ func (s *deciderService) computeTranscodedStream(ctx context.Context, src *Strea
|
||||
return ts, targetFormat
|
||||
}
|
||||
|
||||
// lookupDefaultBitrate returns the default bitrate for the given format.
|
||||
// It checks the DB first (for user-customized values), then falls back to
|
||||
// the built-in defaults, and finally to fallbackBitrate.
|
||||
func lookupDefaultBitrate(ctx context.Context, ds model.DataStore, format string) int {
|
||||
if t, err := ds.Transcoding(ctx).FindByFormat(format); err == nil && t.DefaultBitRate > 0 {
|
||||
return t.DefaultBitRate
|
||||
}
|
||||
for _, dt := range consts.DefaultTranscodings {
|
||||
if dt.TargetFormat == format && dt.DefaultBitRate > 0 {
|
||||
return dt.DefaultBitRate
|
||||
}
|
||||
}
|
||||
return fallbackBitrate
|
||||
}
|
||||
|
||||
// LookupTranscodeCommand returns the ffmpeg command for the given format.
|
||||
// It checks the DB first (for user-customized commands), then falls back to
|
||||
// the built-in default command. Returns "" if the format is unknown.
|
||||
@@ -341,8 +363,10 @@ func (s *deciderService) computeBitrate(ctx context.Context, src *StreamDetails,
|
||||
if !targetIsLossless {
|
||||
if clientInfo.MaxTranscodingAudioBitrate > 0 {
|
||||
ts.Bitrate = clientInfo.MaxTranscodingAudioBitrate
|
||||
} else if clientInfo.MaxAudioBitrate > 0 {
|
||||
ts.Bitrate = clientInfo.MaxAudioBitrate
|
||||
} else {
|
||||
ts.Bitrate = defaultBitrate
|
||||
ts.Bitrate = lookupDefaultBitrate(ctx, s.ds, targetFormat)
|
||||
}
|
||||
} else {
|
||||
if clientInfo.MaxAudioBitrate > 0 && src.Bitrate > clientInfo.MaxAudioBitrate {
|
||||
|
||||
@@ -229,7 +229,7 @@ var _ = Describe("Decider", func() {
|
||||
decision, err := svc.MakeDecision(ctx, mf, ci, DecisionOptions{})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(decision.CanTranscode).To(BeTrue())
|
||||
Expect(decision.TargetBitrate).To(Equal(defaultBitrate)) // 256 kbps
|
||||
Expect(decision.TargetBitrate).To(Equal(160)) // mp3 default from mock transcoding repo
|
||||
})
|
||||
|
||||
It("preserves lossy bitrate when under max", func() {
|
||||
@@ -993,8 +993,8 @@ var _ = Describe("Decider", func() {
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(decision.CanTranscode).To(BeTrue())
|
||||
Expect(decision.TargetFormat).To(Equal("mp3"))
|
||||
// With no cap, lossless→lossy uses defaultBitrate (256)
|
||||
Expect(decision.TargetBitrate).To(Equal(defaultBitrate))
|
||||
// With no cap, lossless→lossy uses format default bitrate (160 for mp3 from mock)
|
||||
Expect(decision.TargetBitrate).To(Equal(160))
|
||||
})
|
||||
|
||||
It("does not apply override when no transcoding is in context", func() {
|
||||
@@ -1012,6 +1012,97 @@ var _ = Describe("Decider", func() {
|
||||
})
|
||||
|
||||
})
|
||||
|
||||
Context("Player MaxBitRate cap", func() {
|
||||
It("applies player MaxBitRate cap when client has no limit", func() {
|
||||
mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16})
|
||||
ci := &ClientInfo{
|
||||
Name: "TestClient",
|
||||
DirectPlayProfiles: []DirectPlayProfile{
|
||||
{Containers: []string{"flac", "mp3"}, AudioCodecs: []string{"flac", "mp3"}, Protocols: []string{ProtocolHTTP}},
|
||||
},
|
||||
TranscodingProfiles: []Profile{
|
||||
{Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP},
|
||||
},
|
||||
}
|
||||
playerCtx := request.WithPlayer(ctx, model.Player{MaxBitRate: 320})
|
||||
|
||||
decision, err := svc.MakeDecision(playerCtx, mf, ci, DecisionOptions{})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
// Source bitrate 1000 > player cap 320, so direct play is not possible
|
||||
Expect(decision.CanDirectPlay).To(BeFalse())
|
||||
Expect(decision.CanTranscode).To(BeTrue())
|
||||
// Lossless→lossy should use MaxAudioBitrate (320) as target, not format default
|
||||
Expect(decision.TargetBitrate).To(Equal(320))
|
||||
})
|
||||
|
||||
It("uses client limit when it is more restrictive than player MaxBitRate", func() {
|
||||
mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16})
|
||||
ci := &ClientInfo{
|
||||
Name: "TestClient",
|
||||
MaxAudioBitrate: 256,
|
||||
MaxTranscodingAudioBitrate: 256,
|
||||
TranscodingProfiles: []Profile{
|
||||
{Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP},
|
||||
},
|
||||
}
|
||||
playerCtx := request.WithPlayer(ctx, model.Player{MaxBitRate: 500})
|
||||
|
||||
decision, err := svc.MakeDecision(playerCtx, mf, ci, DecisionOptions{})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(decision.CanTranscode).To(BeTrue())
|
||||
// Client limit 256 < player cap 500, so player cap doesn't apply; client limit wins
|
||||
Expect(decision.TargetBitrate).To(Equal(256))
|
||||
})
|
||||
|
||||
It("does not cap when player MaxBitRate is 0", func() {
|
||||
mf := withProbe(&model.MediaFile{ID: "1", Suffix: "mp3", Codec: "MP3", BitRate: 320, Channels: 2, SampleRate: 44100})
|
||||
ci := &ClientInfo{
|
||||
Name: "TestClient",
|
||||
DirectPlayProfiles: []DirectPlayProfile{
|
||||
{Containers: []string{"mp3"}, AudioCodecs: []string{"mp3"}, Protocols: []string{ProtocolHTTP}},
|
||||
},
|
||||
}
|
||||
playerCtx := request.WithPlayer(ctx, model.Player{MaxBitRate: 0})
|
||||
|
||||
decision, err := svc.MakeDecision(playerCtx, mf, ci, DecisionOptions{})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(decision.CanDirectPlay).To(BeTrue())
|
||||
})
|
||||
})
|
||||
|
||||
Context("Format-aware default bitrate", func() {
|
||||
It("uses opus default bitrate from DB", func() {
|
||||
mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 48000, BitDepth: 16})
|
||||
ci := &ClientInfo{
|
||||
TranscodingProfiles: []Profile{
|
||||
{Container: "opus", AudioCodec: "opus", Protocol: ProtocolHTTP},
|
||||
},
|
||||
}
|
||||
decision, err := svc.MakeDecision(ctx, mf, ci, DecisionOptions{})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(decision.CanTranscode).To(BeTrue())
|
||||
Expect(decision.TargetBitrate).To(Equal(96)) // opus default from mock
|
||||
})
|
||||
|
||||
It("uses aac default bitrate from DB", func() {
|
||||
mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 2, SampleRate: 44100, BitDepth: 16})
|
||||
ci := &ClientInfo{
|
||||
TranscodingProfiles: []Profile{
|
||||
{Container: "aac", AudioCodec: "aac", Protocol: ProtocolHTTP},
|
||||
},
|
||||
}
|
||||
decision, err := svc.MakeDecision(ctx, mf, ci, DecisionOptions{})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(decision.CanTranscode).To(BeTrue())
|
||||
Expect(decision.TargetBitrate).To(Equal(256)) // aac default from mock
|
||||
})
|
||||
|
||||
It("falls back to 256 for unknown format", func() {
|
||||
bitrate := lookupDefaultBitrate(ctx, ds, "xyz")
|
||||
Expect(bitrate).To(Equal(fallbackBitrate))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
Describe("ensureProbed", func() {
|
||||
|
||||
Reference in New Issue
Block a user