From 27209ed26a87dcc0eebd6dfd64fad94b3c3a073f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Sat, 11 Apr 2026 23:15:07 -0400 Subject: [PATCH] fix(transcoding): clamp target channels to codec limit (#5336) (#5345) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(transcoding): clamp target channels to codec limit (#5336) When transcoding a multi-channel source (e.g. 6-channel FLAC) to MP3, the decider passed the source channel count through to ffmpeg unchanged. The default MP3 command path then emitted `-ac 6`, and the template path injected `-ac 6` after the template's own `-ac 2`, causing ffmpeg to honor the last occurrence and fail with exit code 234 since libmp3lame only supports up to 2 channels. Introduce `codecMaxChannels()` in core/stream/codec.go (mp3→2, opus→8), mirroring the existing `codecMaxSampleRate` pattern, and apply the clamp in `computeTranscodedStream` right after the sample-rate clamps. Also fix a pre-existing ordering bug where the profile's MaxAudioChannels check compared against src.Channels rather than ts.Channels, which would have let a looser profile setting raise the codec-clamped value back up. Comparing against the already-clamped ts.Channels makes profile limits strictly narrowing, which matches how the sample-rate block already behaves. The ffmpeg buildTemplateArgs comment is refreshed to point at the new upstream clamp, since the flags it injects are now always codec-safe. Adds unit tests for codecMaxChannels and four decider scenarios covering the literal issue repro (6-ch FLAC→MP3 clamps to 2), a stricter profile limit winning over the codec clamp, a looser profile limit leaving the codec clamp intact, and a codec with no hard limit (AAC) passing 6 channels through. * test(e2e): pin codec channel clamp at the Subsonic API surface (#5336) Add a 6-channel FLAC fixture to the e2e test suite and use it to assert the codec channel clamp end-to-end on both Subsonic streaming endpoints: - getTranscodeDecision (mp3OnlyClient, no MaxAudioChannels in profile): expects TranscodeStream.AudioChannels == 2 for the 6-channel source. This exercises the new codecMaxChannels() helper through the OpenSubsonic decision endpoint, with no profile-level channel limit masking the bug. - /rest/stream (legacy): requests format=mp3 against the multichannel fixture and asserts streamerSpy.LastRequest.Channels == 2, confirming the clamp propagates through ResolveRequest into the stream.Request that the streamer receives. The fixture is metadata-only (channels: 6 plumbed via the existing storagetest.File helper) — no real audio bytes required, since the e2e suite uses a spy streamer rather than invoking ffmpeg. Bumps the empty-query search3 song count expectation from 13 to 14 to account for the new fixture. * test(decider): clarify codec-clamp comment terminology Distinguish "transcoding profile MaxAudioChannels" (Profile.MaxAudioChannels field) from "LimitationAudioChannels" (CodecProfile rule constant). The regression test bypasses the former, not the latter. --- core/ffmpeg/ffmpeg.go | 5 +- core/stream/codec.go | 13 ++++++ core/stream/codec_test.go | 22 +++++++++ core/stream/decider.go | 9 +++- core/stream/decider_test.go | 67 +++++++++++++++++++++++++++ server/e2e/e2e_suite_test.go | 4 ++ server/e2e/subsonic_searching_test.go | 2 +- server/e2e/subsonic_stream_test.go | 14 +++++- server/e2e/subsonic_transcode_test.go | 29 +++++++++--- 9 files changed, 151 insertions(+), 14 deletions(-) diff --git a/core/ffmpeg/ffmpeg.go b/core/ffmpeg/ffmpeg.go index c034ca7d0..5e6dcd115 100644 --- a/core/ffmpeg/ffmpeg.go +++ b/core/ffmpeg/ffmpeg.go @@ -412,8 +412,9 @@ func buildDynamicArgs(opts TranscodeOptions) []string { // buildTemplateArgs handles user-customized command templates, with dynamic injection // of sample rate, channels, and bit depth when requested by the transcode decision. -// Note: these flags are injected unconditionally when non-zero, even if the template -// already includes them. FFmpeg uses the last occurrence of duplicate flags. +// Values in opts have already been clamped to codec limits upstream (see +// core/stream/codec.go codecMax* helpers), so injecting them unconditionally is safe — +// ffmpeg honors the last occurrence of a duplicate flag. func buildTemplateArgs(opts TranscodeOptions) []string { args := createFFmpegCommand(opts.Command, opts.FilePath, opts.BitRate, opts.Offset) diff --git a/core/stream/codec.go b/core/stream/codec.go index 88d1ae45d..28bff75c4 100644 --- a/core/stream/codec.go +++ b/core/stream/codec.go @@ -75,3 +75,16 @@ func codecMaxSampleRate(codec string) int { } return 0 } + +// codecMaxChannels returns the hard maximum number of audio channels a codec +// supports. Returns 0 if the codec has no hard limit (or is unknown), in which +// case the source/profile constraints applied upstream are authoritative. +func codecMaxChannels(codec string) int { + switch strings.ToLower(codec) { + case "mp3": + return 2 + case "opus": + return 8 + } + return 0 +} diff --git a/core/stream/codec_test.go b/core/stream/codec_test.go index 4c76b3ecd..97e15bdb5 100644 --- a/core/stream/codec_test.go +++ b/core/stream/codec_test.go @@ -66,4 +66,26 @@ var _ = Describe("Codec", func() { Expect(normalizeProbeCodec("DSD_LSBF_PLANAR")).To(Equal("dsd")) }) }) + + Describe("codecMaxChannels", func() { + It("returns 2 for mp3", func() { + Expect(codecMaxChannels("mp3")).To(Equal(2)) + }) + + It("returns 8 for opus", func() { + Expect(codecMaxChannels("opus")).To(Equal(8)) + }) + + It("is case-insensitive", func() { + Expect(codecMaxChannels("MP3")).To(Equal(2)) + Expect(codecMaxChannels("Opus")).To(Equal(8)) + }) + + It("returns 0 for codecs with no hard limit", func() { + Expect(codecMaxChannels("aac")).To(Equal(0)) + Expect(codecMaxChannels("flac")).To(Equal(0)) + Expect(codecMaxChannels("vorbis")).To(Equal(0)) + Expect(codecMaxChannels("")).To(Equal(0)) + }) + }) }) diff --git a/core/stream/decider.go b/core/stream/decider.go index 713c779fe..cde12f0f3 100644 --- a/core/stream/decider.go +++ b/core/stream/decider.go @@ -294,14 +294,19 @@ func (s *deciderService) computeTranscodedStream(ctx context.Context, src *Detai if maxRate := codecMaxSampleRate(ts.Codec); maxRate > 0 && ts.SampleRate > maxRate { ts.SampleRate = maxRate } + if maxCh := codecMaxChannels(ts.Codec); maxCh > 0 && ts.Channels > maxCh { + ts.Channels = maxCh + } // Determine target bitrate (all in kbps) if ok := s.computeBitrate(ctx, src, targetFormat, targetIsLossless, clientInfo, ts); !ok { return nil, "" } - // Apply MaxAudioChannels from the transcoding profile - if profile.MaxAudioChannels > 0 && src.Channels > profile.MaxAudioChannels { + // Apply MaxAudioChannels from the transcoding profile. Compare against the + // already-clamped ts.Channels (not src.Channels) so the codec hard limit + // applied above is never raised by a looser profile setting. + if profile.MaxAudioChannels > 0 && ts.Channels > profile.MaxAudioChannels { ts.Channels = profile.MaxAudioChannels } diff --git a/core/stream/decider_test.go b/core/stream/decider_test.go index c776cbdc3..8b58f3323 100644 --- a/core/stream/decider_test.go +++ b/core/stream/decider_test.go @@ -770,6 +770,73 @@ var _ = Describe("Decider", func() { }) }) + Context("Codec channel limits", func() { + It("clamps 6-channel FLAC to 2 channels when transcoding to MP3", func() { + // Regression test for #5336: ffmpeg's mp3 encoder rejects >2 channels. + // The decider must clamp to the codec's hard limit even when no + // transcoding profile MaxAudioChannels is configured. + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: 16}) + ci := &ClientInfo{ + MaxTranscodingAudioBitrate: 320, + TranscodingProfiles: []Profile{ + {Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP}, + }, + } + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanTranscode).To(BeTrue()) + Expect(decision.TargetFormat).To(Equal("mp3")) + Expect(decision.TranscodeStream.Channels).To(Equal(2)) + Expect(decision.TargetChannels).To(Equal(2)) + }) + + It("honors a stricter profile MaxAudioChannels over the codec clamp", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: 16}) + ci := &ClientInfo{ + MaxTranscodingAudioBitrate: 320, + TranscodingProfiles: []Profile{ + {Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP, MaxAudioChannels: 1}, + }, + } + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanTranscode).To(BeTrue()) + Expect(decision.TranscodeStream.Channels).To(Equal(1)) + Expect(decision.TargetChannels).To(Equal(1)) + }) + + It("applies the codec clamp when the profile limit is looser", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: 16}) + ci := &ClientInfo{ + MaxTranscodingAudioBitrate: 320, + TranscodingProfiles: []Profile{ + {Container: "mp3", AudioCodec: "mp3", Protocol: ProtocolHTTP, MaxAudioChannels: 4}, + }, + } + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanTranscode).To(BeTrue()) + Expect(decision.TranscodeStream.Channels).To(Equal(2)) + Expect(decision.TargetChannels).To(Equal(2)) + }) + + It("passes channels through unchanged for codecs with no hard limit", func() { + mf := withProbe(&model.MediaFile{ID: "1", Suffix: "flac", Codec: "FLAC", BitRate: 1000, Channels: 6, SampleRate: 44100, BitDepth: 16}) + ci := &ClientInfo{ + MaxTranscodingAudioBitrate: 320, + TranscodingProfiles: []Profile{ + {Container: "m4a", AudioCodec: "aac", Protocol: ProtocolHTTP}, + }, + } + decision, err := svc.MakeDecision(ctx, mf, ci, TranscodeOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(decision.CanTranscode).To(BeTrue()) + Expect(decision.TargetFormat).To(Equal("aac")) + Expect(decision.TranscodeStream.Channels).To(Equal(6)) + Expect(decision.TargetChannels).To(Equal(6)) + }) + }) + Context("Probe-based lossless detection", func() { It("uses probe codec name for lossless detection", func() { // WavPack files: ffprobe reports codec as "wavpack", suffix is ".wv" diff --git a/server/e2e/e2e_suite_test.go b/server/e2e/e2e_suite_test.go index 03fa9bbef..5b3500f7a 100644 --- a/server/e2e/e2e_suite_test.go +++ b/server/e2e/e2e_suite_test.go @@ -172,6 +172,10 @@ func buildTestFS() storagetest.FakeFS { "title": "TC MKA Opus", "track": 6, "suffix": "mka", "codec": "opus", "bitrate": 128, "samplerate": 48000, "bitdepth": 0, "channels": 2, "duration": int64(220), }), + "Test/Transcode Formats/07 - TC FLAC Multichannel.flac": file(tcBase, _t{ + "title": "TC FLAC Multichannel", "track": 7, "suffix": "flac", + "bitrate": 4500, "samplerate": 48000, "bitdepth": 24, "channels": 6, "duration": int64(180), + }), // _empty folder (directory with no audio) "_empty/.keep": &fstest.MapFile{Data: []byte{}, ModTime: time.Now()}, diff --git a/server/e2e/subsonic_searching_test.go b/server/e2e/subsonic_searching_test.go index 7f6aaf57a..e348bc6b9 100644 --- a/server/e2e/subsonic_searching_test.go +++ b/server/e2e/subsonic_searching_test.go @@ -117,7 +117,7 @@ var _ = Describe("Search Endpoints", func() { Expect(resp.SearchResult3).ToNot(BeNil()) Expect(resp.SearchResult3.Artist).To(HaveLen(6)) Expect(resp.SearchResult3.Album).To(HaveLen(7)) - Expect(resp.SearchResult3.Song).To(HaveLen(13)) + Expect(resp.SearchResult3.Song).To(HaveLen(14)) }) It("finds across all entity types simultaneously", func() { diff --git a/server/e2e/subsonic_stream_test.go b/server/e2e/subsonic_stream_test.go index 6a11c1740..281524636 100644 --- a/server/e2e/subsonic_stream_test.go +++ b/server/e2e/subsonic_stream_test.go @@ -13,8 +13,9 @@ import ( var _ = Describe("stream.view (legacy streaming)", Ordered, func() { var ( - mp3TrackID string // Come Together (mp3, 320kbps) - flacTrackID string // TC FLAC Standard (flac, 900kbps) + mp3TrackID string // Come Together (mp3, 320kbps) + flacTrackID string // TC FLAC Standard (flac, 900kbps) + flacMultichTrackID string // TC FLAC Multichannel (flac, 6ch) ) BeforeAll(func() { @@ -30,6 +31,8 @@ var _ = Describe("stream.view (legacy streaming)", Ordered, func() { Expect(mp3TrackID).ToNot(BeEmpty()) flacTrackID = byTitle["TC FLAC Standard"] Expect(flacTrackID).ToNot(BeEmpty()) + flacMultichTrackID = byTitle["TC FLAC Multichannel"] + Expect(flacMultichTrackID).ToNot(BeEmpty()) }) Describe("raw / direct play", func() { @@ -101,6 +104,13 @@ var _ = Describe("stream.view (legacy streaming)", Ordered, func() { Expect(streamerSpy.LastRequest.Format).To(Equal("mp3")) Expect(streamerSpy.LastRequest.BitRate).To(Equal(128)) }) + + It("clamps multichannel FLAC to 2 channels when transcoding to mp3 (#5336)", func() { + w := doRawReq("stream", "id", flacMultichTrackID, "format", "mp3", "maxBitRate", "256") + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(streamerSpy.LastRequest.Format).To(Equal("mp3")) + Expect(streamerSpy.LastRequest.Channels).To(Equal(2)) + }) }) Describe("downsampling with maxBitRate only", func() { diff --git a/server/e2e/subsonic_transcode_test.go b/server/e2e/subsonic_transcode_test.go index f134448df..6041cd013 100644 --- a/server/e2e/subsonic_transcode_test.go +++ b/server/e2e/subsonic_transcode_test.go @@ -114,13 +114,14 @@ const ( var _ = Describe("Transcode Endpoints", Ordered, func() { // Track IDs resolved in BeforeAll var ( - mp3TrackID string // Come Together (mp3, 320kbps) - flacTrackID string // TC FLAC Standard (flac, 900kbps) - flacHiResTrackID string // TC FLAC HiRes (flac, 3000kbps) - alacTrackID string // TC ALAC Track (m4a, alac) - dsdTrackID string // TC DSD Track (dsf, dsd) - opusTrackID string // TC Opus Track (opus, 128kbps) - mkaOpusTrackID string // TC MKA Opus (mka, opus via codec tag) + mp3TrackID string // Come Together (mp3, 320kbps) + flacTrackID string // TC FLAC Standard (flac, 900kbps) + flacHiResTrackID string // TC FLAC HiRes (flac, 3000kbps) + flacMultichTrackID string // TC FLAC Multichannel (flac, 6ch) + alacTrackID string // TC ALAC Track (m4a, alac) + dsdTrackID string // TC DSD Track (dsf, dsd) + opusTrackID string // TC Opus Track (opus, 128kbps) + mkaOpusTrackID string // TC MKA Opus (mka, opus via codec tag) ) BeforeAll(func() { @@ -140,6 +141,7 @@ var _ = Describe("Transcode Endpoints", Ordered, func() { mp3TrackID = ensureGetTrackID("Come Together") flacTrackID = ensureGetTrackID("TC FLAC Standard") flacHiResTrackID = ensureGetTrackID("TC FLAC HiRes") + flacMultichTrackID = ensureGetTrackID("TC FLAC Multichannel") alacTrackID = ensureGetTrackID("TC ALAC Track") dsdTrackID = ensureGetTrackID("TC DSD Track") opusTrackID = ensureGetTrackID("TC Opus Track") @@ -353,6 +355,19 @@ var _ = Describe("Transcode Endpoints", Ordered, func() { // maxTranscodingAudioBitrate is 192000 bps = 192 kbps → response in bps Expect(resp.TranscodeDecision.TranscodeStream.AudioBitrate).To(Equal(int32(192000))) }) + + It("clamps multichannel FLAC to 2 channels when transcoding to MP3 (#5336)", func() { + // mp3OnlyClient has no MaxAudioChannels set, so this exercises the + // codec-intrinsic clamp in core/stream/codec.go (codecMaxChannels). + resp := doPostReq("getTranscodeDecision", mp3OnlyClient, "mediaId", flacMultichTrackID, "mediaType", "song") + Expect(resp.Status).To(Equal(responses.StatusOK)) + Expect(resp.TranscodeDecision).ToNot(BeNil()) + Expect(resp.TranscodeDecision.CanTranscode).To(BeTrue()) + Expect(resp.TranscodeDecision.SourceStream.AudioChannels).To(Equal(int32(6))) + Expect(resp.TranscodeDecision.TranscodeStream).ToNot(BeNil()) + Expect(resp.TranscodeDecision.TranscodeStream.Codec).To(Equal("mp3")) + Expect(resp.TranscodeDecision.TranscodeStream.AudioChannels).To(Equal(int32(2))) + }) }) Describe("response structure", func() {