From ec49ae476f3ae6f852a2eea6ecc82c3652a6c8e6 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 24 Sep 2026 17:13:44 -0400 Subject: [PATCH] fix(transcoding): size the inserted mp3 frame to fit its tag The inserted Info frame reused the first audio frame's header, and so its size. A VBR encode at 22050Hz that starts on silence opens with an 8kbps frame of 26 bytes, while the tag needs 33, so building the frame panicked with an index out of range. Reproduced with ffmpeg -ar 22050 -q:a 9 on a quiet input. The frame now keeps the first frame's version, sample rate and channel mode but raises the bitrate until the frame is big enough for the tag, which is what ffmpeg's own mp3 muxer does. The frame count is unaffected, and decoded audio of the reproducing file is byte-identical after the patch. --- core/ffmpeg/mp3_xing.go | 47 +++++++++++++++++++++--------------- core/ffmpeg/mp3_xing_test.go | 14 +++++++++++ 2 files changed, 42 insertions(+), 19 deletions(-) diff --git a/core/ffmpeg/mp3_xing.go b/core/ffmpeg/mp3_xing.go index dcfb08790..a173cf671 100644 --- a/core/ffmpeg/mp3_xing.go +++ b/core/ffmpeg/mp3_xing.go @@ -9,9 +9,10 @@ import ( ) const ( - mp3HeaderLen = 4 - mp3ID3Len = 10 - mp3MaxPrefix = 64 << 10 // ffmpeg cannot write an attached picture to a pipe, so the tag stays small + mp3HeaderLen = 4 + mp3ID3Len = 10 + mp3InfoTagLen = 12 // tag, flags and frame count + mp3MaxPrefix = 64 << 10 // ffmpeg cannot write an attached picture to a pipe, so the tag stays small ) // mp3Prefix returns the head of the stream with an Info frame inserted before the first @@ -41,7 +42,7 @@ func (h *headerPatcher) mp3Prefix(buf []byte) ([]byte, error) { if isXingFrame(buf[start:], frame.tagOffset) { return buf, nil } - xing, ok := xingFrame(buf[start:], frame, h.duration) + xing, ok := xingFrame(buf[start:], h.duration) if !ok { return buf, nil } @@ -60,7 +61,6 @@ type mp3Frame struct { sampleRate int samples int // per frame size int // bytes, including the header - sideInfo int tagOffset int // where a Xing tag sits in the incoming frame, which may carry a CRC } @@ -89,19 +89,20 @@ func parseMP3Header(h []byte) (mp3Frame, bool) { return mp3Frame{}, false } f := mp3Frame{sampleRate: sampleRate, samples: 576} + var sideInfo int mono := (h[3]>>6)&0x03 == 3 switch { case mpeg1 && mono: - f.samples, f.sideInfo = 1152, 17 + f.samples, sideInfo = 1152, 17 case mpeg1: - f.samples, f.sideInfo = 1152, 32 + f.samples, sideInfo = 1152, 32 case mono: - f.sideInfo = 9 + sideInfo = 9 default: - f.sideInfo = 17 + sideInfo = 17 } f.size = f.samples/8*bitRate/sampleRate + int((h[2]>>1)&0x01) - f.tagOffset = mp3HeaderLen + f.sideInfo + f.tagOffset = mp3HeaderLen + sideInfo if h[1]&0x01 == 0 { // CRC follows the header f.tagOffset += 2 } @@ -116,19 +117,27 @@ func isXingFrame(frame []byte, tagOffset int) bool { return tag == "Xing" || tag == "Info" } -// xingFrame builds a silent Info frame declaring how many frames follow it. It reuses the -// first frame's header, minus its CRC, so the two describe the same stream. -func xingFrame(first []byte, f mp3Frame, duration float32) ([]byte, bool) { +// xingFrame builds a silent Info frame declaring how many frames follow it. Its header is +// the first frame's minus the CRC, with the bitrate raised until the frame fits the tag. +func xingFrame(first []byte, duration float32) ([]byte, bool) { + header := [mp3HeaderLen]byte(first) + header[1] |= 0x01 + f, ok := parseMP3Header(header[:]) + for ok && f.size < f.tagOffset+mp3InfoTagLen { + header[2] += 0x10 // next bitrate index; 15 is invalid, which ends the loop + f, ok = parseMP3Header(header[:]) + } + if !ok { + return nil, false + } frames := math.Round(float64(duration) * float64(f.sampleRate) / float64(f.samples)) if frames < 1 || frames > math.MaxUint32 { return nil, false } frame := make([]byte, f.size) - copy(frame, first[:mp3HeaderLen]) - frame[1] |= 0x01 // no CRC, so the tag follows the side info directly - at := mp3HeaderLen + f.sideInfo - copy(frame[at:], "Info") // a Xing tag without a seek table marks the stream unseekable to some decoders - binary.BigEndian.PutUint32(frame[at+4:], 1) // only the frame count is present - binary.BigEndian.PutUint32(frame[at+8:], uint32(frames)) + copy(frame, header[:]) + copy(frame[f.tagOffset:], "Info") // a Xing tag without a seek table marks the stream unseekable to some decoders + binary.BigEndian.PutUint32(frame[f.tagOffset+4:], 1) // only the frame count is present + binary.BigEndian.PutUint32(frame[f.tagOffset+8:], uint32(frames)) return frame, true } diff --git a/core/ffmpeg/mp3_xing_test.go b/core/ffmpeg/mp3_xing_test.go index a5fca40b8..d3bf8ff1a 100644 --- a/core/ffmpeg/mp3_xing_test.go +++ b/core/ffmpeg/mp3_xing_test.go @@ -92,6 +92,20 @@ var _ = Describe("patchMP3Duration", func() { Expect(frames).To(Equal(uint32(38))) }) + It("raises the bitrate of the inserted frame when the first frame is too small for the tag", func() { + // MPEG 2, 8kbps, 22050Hz, stereo: 26 bytes, as a VBR encode starting on silence emits. + in := make([]byte, 26) + copy(in, []byte{0xFF, 0xF3, 0x10, 0x00}) + + out := readAll(in, 1.0) + + Expect(out[2]>>4).To(Equal(byte(2)), "16kbps, the first bitrate whose frame fits the tag") + tag, frames := readXing(out, 0, 21) // 4 header + 17 side info + Expect(tag).To(Equal("Info")) + Expect(frames).To(Equal(uint32(38))) // 1s at 22050Hz is 38 frames of 576 samples + Expect(out[52:]).To(Equal(in)) // 52 bytes = frame size at 16kbps, 22050Hz + }) + It("inserts the frame at the start of a stream with no ID3 tag", func() { in := pipedMP3[fixtureID3Len:]