mirror of
https://github.com/navidrome/navidrome.git
synced 2026-11-01 16:07:47 +01:00
* 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.
This commit is contained in:
parent
de6475bb49
commit
27209ed26a
9 changed files with 151 additions and 14 deletions
|
|
@ -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)
|
||||
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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))
|
||||
})
|
||||
})
|
||||
})
|
||||
|
|
|
|||
|
|
@ -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
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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"
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue