fix(subsonic): don't re-encode a source already in the player's forced format

When a player has a forced transcoding format, ClientInfo.ForceFormat cleared
DirectPlayProfiles unconditionally. A FLAC source on a player configured to
transcode to FLAC was therefore re-encoded to FLAC, wasting CPU and bandwidth
for no gain. Worse, the transcoder pipes ffmpeg output to stdout, so the
resulting FLAC has total_samples=0 and no seek table -- an offline copy of it
can never be seeked. Reported against getTranscodeDecision by the Symfonium
author.

ForceFormat now rebuilds DirectPlayProfiles from the matching transcoding
profiles instead of dropping them: a client declaring a transcoding profile for
a format is proof it can consume that format, so a source already in it is
served as-is. Container and codec come from resolveTargetFormat, so a legacy
"oga" target_format yields an ogg/opus profile, and the profile's
MaxAudioChannels is carried across.

DirectPlayProfile has no bitrate field, so restoring direct play needs a
ceiling to keep an over-bitrate source out of it. GetTranscodeDecision now
seeds that ceiling from the transcoding row's DefaultBitRate when a format was
successfully forced, with the player's own MaxBitRate still taking precedence.
This also closes a gap where the new endpoint ignored DefaultBitRate entirely:
an mp3 320 source on a player forced to mp3@192 was served at 320, while the
legacy /rest/stream path correctly gave 192.

Applied via CapBitrate, which only ever lowers, so a client declaring a
stricter limit keeps it. The legacy path (applyServerOverride) is untouched --
ForceFormat has no other callers.

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() {

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
 }
 

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() {

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

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())
+			})
 		})
 	})