From c05fbd0edbb7719601d1759e2e2eafeebb68c7d2 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 24 Sep 2026 16:42:48 -0400 Subject: [PATCH] fix(transcoding): declare the duration of a piped mp3 transcode The mp3 muxer writes the Xing frame that carries the frame count only after rewinding to the start of the stream, which it cannot do on a pipe (-f mp3 -). An uncached transcode therefore declares its length nowhere: no Xing frame, and no Content-Length unless the client asks for the estimate. A decoder has to guess the duration from the bytes it has so far, and an offline copy stays unseekable, the same defect #6105 fixed for FLAC. Transcode now inserts a Xing frame ahead of the first audio frame, carrying the frame count derived from the track duration. Only the frame count is declared: a piped encode cannot know its own byte size, and a wrong size is worse than an absent one. Decoded audio is unchanged, since decoders skip the tag frame. Measured on a 20s 128kbps transcode: ffprobe reports 20.010s instead of 20.036s, and Firefox reports 19.998s at loadedmetadata instead of 2.04s climbing over nine durationchange events. This does not make an uncached transcode resumable; that needs range support, which only the cache path has. Reported in #6170. --- core/ffmpeg/ffmpeg.go | 10 +- core/ffmpeg/ffmpeg_test.go | 19 ++++ core/ffmpeg/mp3_xing.go | 182 +++++++++++++++++++++++++++++++++++ core/ffmpeg/mp3_xing_test.go | 157 ++++++++++++++++++++++++++++++ 4 files changed, 366 insertions(+), 2 deletions(-) create mode 100644 core/ffmpeg/mp3_xing.go create mode 100644 core/ffmpeg/mp3_xing_test.go diff --git a/core/ffmpeg/ffmpeg.go b/core/ffmpeg/ffmpeg.go index cc38dd9de..c56ad1a69 100644 --- a/core/ffmpeg/ffmpeg.go +++ b/core/ffmpeg/ffmpeg.go @@ -32,7 +32,7 @@ type TranscodeOptions struct { Channels int // 0 = no constraint BitDepth int // 0 = no constraint; valid values: 16, 24, 32 Offset int // seconds - Duration float32 // seconds; 0 = unknown. Only used to repair a piped FLAC header. + Duration float32 // seconds; 0 = unknown. Only used to repair a piped FLAC or mp3 header. } // AudioProbeResult contains authoritative audio stream properties from ffprobe. @@ -91,7 +91,13 @@ func (e *ffmpeg) Transcode(ctx context.Context, opts TranscodeOptions) (io.ReadC if err != nil { return nil, err } - return patchFLACDuration(out, opts.Duration-float32(opts.Offset)), nil + duration := opts.Duration - float32(opts.Offset) + switch opts.Format { + case "mp3": + return patchMP3Duration(out, duration), nil + default: + return patchFLACDuration(out, duration), nil + } } func (e *ffmpeg) ConvertAnimatedImage(ctx context.Context, reader io.Reader, maxSize int, quality int) (io.ReadCloser, error) { diff --git a/core/ffmpeg/ffmpeg_test.go b/core/ffmpeg/ffmpeg_test.go index dbc8fa3c8..79c46176b 100644 --- a/core/ffmpeg/ffmpeg_test.go +++ b/core/ffmpeg/ffmpeg_test.go @@ -1,7 +1,9 @@ package ffmpeg import ( + "bytes" "context" + "encoding/binary" "errors" "io" "os" @@ -719,6 +721,23 @@ var _ = Describe("ffmpeg", func() { Expect(err).ToNot(HaveOccurred()) Expect(readTotalSamples(out)).To(Equal(uint64(2 * 44100))) }) + + It("inserts a Xing frame on a piped mp3 transcode", func() { + stream, err := ff.Transcode(GinkgoT().Context(), TranscodeOptions{ + Command: "ffmpeg -i %s -map 0:a:0 -v 0 -b:a 128k -f mp3 -", + Format: "mp3", + FilePath: "tests/fixtures/test.flac", + Duration: 1, // the fixture is exactly 1s at 44100Hz + }) + Expect(err).ToNot(HaveOccurred()) + defer stream.Close() + + out, err := io.ReadAll(stream) + Expect(err).ToNot(HaveOccurred()) + at := bytes.Index(out, []byte("Xing")) + Expect(at).To(BeNumerically(">", 0)) + Expect(binary.BigEndian.Uint32(out[at+8:])).To(Equal(uint32(38))) + }) }) Context("stderr capture", func() { diff --git a/core/ffmpeg/mp3_xing.go b/core/ffmpeg/mp3_xing.go new file mode 100644 index 000000000..125469605 --- /dev/null +++ b/core/ffmpeg/mp3_xing.go @@ -0,0 +1,182 @@ +package ffmpeg + +import ( + "bytes" + "encoding/binary" + "errors" + "io" + "math" + "slices" +) + +const ( + mp3HeaderLen = 4 + mp3ID3Len = 10 + mp3MaxPrefix = 1 << 20 // an ID3 tag carrying cover art still fits +) + +// patchMP3Duration prepends the Xing frame that ffmpeg omits when writing to a pipe, +// since it only knows the frame count once it can rewind to the first frame. +func patchMP3Duration(r io.ReadCloser, duration float32) io.ReadCloser { + if duration <= 0 { + return r + } + return &mp3Patcher{ReadCloser: r, duration: duration} +} + +type mp3Patcher struct { + io.ReadCloser + duration float32 + // Peeking here rather than in the constructor keeps Transcode from blocking + // until ffmpeg has emitted its first bytes. + stream io.Reader +} + +func (m *mp3Patcher) Read(p []byte) (int, error) { + if m.stream == nil { + prefix, err := m.peek() + if err != nil { + return 0, err + } + m.stream = io.MultiReader(bytes.NewReader(prefix), m.ReadCloser) + } + return m.stream.Read(p) +} + +// peek returns the head of the stream with a Xing frame inserted before the first +// audio frame, or unchanged when it cannot make sense of what ffmpeg wrote. +func (m *mp3Patcher) peek() ([]byte, error) { + buf, err := m.fill(nil, mp3ID3Len) + if err != nil || len(buf) < mp3ID3Len { + return buf, err + } + start := 0 + if string(buf[:3]) == "ID3" { + if start = id3TagLen(buf); start > mp3MaxPrefix { + return buf, nil + } + } + if buf, err = m.fill(buf, start+mp3HeaderLen); err != nil || len(buf) < start+mp3HeaderLen { + return buf, err + } + frame, ok := parseMP3Header(buf[start:]) + if !ok { + return buf, nil + } + if buf, err = m.fill(buf, start+frame.size); err != nil || len(buf) < start+frame.size { + return buf, err + } + if isXingFrame(buf[start:], frame.tagOffset) { + return buf, nil + } + xing, ok := xingFrame(buf[start:], frame, m.duration) + if !ok { + return buf, nil + } + return slices.Insert(buf, start, xing...), nil +} + +// fill grows buf to n bytes, stopping short when the stream ends first. +func (m *mp3Patcher) fill(buf []byte, n int) ([]byte, error) { + if len(buf) >= n { + return buf, nil + } + more := make([]byte, n-len(buf)) + read, err := io.ReadFull(m.ReadCloser, more) + if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) { + return buf, err + } + return append(buf, more[:read]...), nil +} + +func id3TagLen(header []byte) int { + size := int(header[6]&0x7F)<<21 | int(header[7]&0x7F)<<14 | int(header[8]&0x7F)<<7 | int(header[9]&0x7F) + if header[5]&0x10 != 0 { // footer present + size += mp3ID3Len + } + return mp3ID3Len + size +} + +type mp3Frame struct { + sampleRate int + samples int // per frame + size int // bytes, including the header + sideInfo int + tagOffset int // where a Xing tag would sit in this frame +} + +var ( + mp3BitRates = [2][16]int{ + {0, 32, 40, 48, 56, 64, 80, 96, 112, 128, 160, 192, 224, 256, 320, 0}, // MPEG 1 + {0, 8, 16, 24, 32, 40, 48, 56, 64, 80, 96, 112, 128, 144, 160, 0}, // MPEG 2 and 2.5 + } + mp3SampleRates = [4][4]int{ + {11025, 12000, 8000, 0}, // MPEG 2.5 + {}, // reserved + {22050, 24000, 16000, 0}, // MPEG 2 + {44100, 48000, 32000, 0}, // MPEG 1 + } +) + +func parseMP3Header(h []byte) (mp3Frame, bool) { + version, layer := (h[1]>>3)&0x03, (h[1]>>1)&0x03 + if h[0] != 0xFF || h[1]&0xE0 != 0xE0 || version == 1 || layer != 1 { + return mp3Frame{}, false // not the sync word of an MPEG Layer III frame + } + mpeg1 := version == 3 + sampleRate := mp3SampleRates[version][(h[2]>>2)&0x03] + bitRate := 1000 * mp3BitRates[b2i(!mpeg1)][h[2]>>4] + if sampleRate == 0 || bitRate == 0 { + return mp3Frame{}, false + } + f := mp3Frame{sampleRate: sampleRate, samples: 576} + mono := (h[3]>>6)&0x03 == 3 + switch { + case mpeg1 && mono: + f.samples, f.sideInfo = 1152, 17 + case mpeg1: + f.samples, f.sideInfo = 1152, 32 + case mono: + f.sideInfo = 9 + default: + f.sideInfo = 17 + } + f.size = f.samples/8*bitRate/sampleRate + int((h[2]>>1)&0x01) + f.tagOffset = mp3HeaderLen + f.sideInfo + if h[1]&0x01 == 0 { // CRC follows the header + f.tagOffset += 2 + } + return f, true +} + +func isXingFrame(frame []byte, tagOffset int) bool { + if len(frame) < tagOffset+4 { + return false + } + tag := string(frame[tagOffset : tagOffset+4]) + return tag == "Xing" || tag == "Info" +} + +// xingFrame builds a silent 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) { + 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:], "Xing") + binary.BigEndian.PutUint32(frame[at+4:], 1) // only the frame count is present + binary.BigEndian.PutUint32(frame[at+8:], uint32(frames)) + return frame, true +} + +func b2i(b bool) int { + if b { + return 1 + } + return 0 +} diff --git a/core/ffmpeg/mp3_xing_test.go b/core/ffmpeg/mp3_xing_test.go new file mode 100644 index 000000000..7d66268c2 --- /dev/null +++ b/core/ffmpeg/mp3_xing_test.go @@ -0,0 +1,157 @@ +package ffmpeg + +import ( + "bytes" + "encoding/binary" + "errors" + "io" + "os" + "testing/iotest" + + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +// Offsets of the Xing tag inside the first frame, for the two layouts the specs use. +const ( + stereoTagOffset = 36 // 4 header + 32 side info + monoTagOffset = 21 // 4 header + 17 side info + fixtureID3Len = 39581 // where the first frame of tests/fixtures/test.mp3 starts + fixtureFrameLen = 627 // its frame size, at 192kbps and 44100Hz +) + +var _ = Describe("patchMP3Duration", func() { + var pipedMP3 []byte + + // The fixture is an ffmpeg pipe's output in every way that matters here: ID3 tag, + // frames, no Xing. + BeforeEach(func() { + var err error + pipedMP3, err = os.ReadFile("tests/fixtures/test.mp3") + Expect(err).ToNot(HaveOccurred()) + Expect(pipedMP3[fixtureID3Len : fixtureID3Len+2]).To(Equal([]byte{0xFF, 0xFB})) + Expect(bytes.Contains(pipedMP3[:4096], []byte("Xing"))).To(BeFalse()) + }) + + readAll := func(in []byte, duration float32) []byte { + out, err := io.ReadAll(patchMP3Duration(io.NopCloser(bytes.NewReader(in)), duration)) + Expect(err).ToNot(HaveOccurred()) + return out + } + + // Reads the tag, flags and frame count of the frame starting at frameStart. + readXing := func(b []byte, frameStart, tagOffset int) (string, uint32, uint32) { + at := frameStart + tagOffset + return string(b[at : at+4]), + binary.BigEndian.Uint32(b[at+4:]), + binary.BigEndian.Uint32(b[at+8:]) + } + + It("inserts a Xing frame declaring the duration in frames", func() { + out := readAll(pipedMP3, 1.0) + + tag, flags, frames := readXing(out, fixtureID3Len, stereoTagOffset) + Expect(tag).To(Equal("Xing")) + Expect(flags).To(Equal(uint32(1)), "only the frame count is present") + Expect(frames).To(Equal(uint32(38)), "1s at 44100Hz is 38 frames of 1152 samples") + }) + + It("leaves the ID3 tag and the audio frames untouched", func() { + out := readAll(pipedMP3, 1.0) + + start := fixtureID3Len + Expect(out).To(HaveLen(len(pipedMP3) + fixtureFrameLen)) + Expect(out[:start]).To(Equal(pipedMP3[:start])) + Expect(out[start+fixtureFrameLen:]).To(Equal(pipedMP3[start:])) + }) + + It("takes the sample rate from the header, not from the source file", func() { + in := bytes.Clone(pipedMP3) + start := fixtureID3Len + in[start+2] = in[start+2]&^0x0C | 0x04 // sample rate index 1: 48000Hz + + out := readAll(in, 1.0) + + _, _, frames := readXing(out, start, stereoTagOffset) + Expect(frames).To(Equal(uint32(42))) // 48000/1152, rounded + }) + + It("rounds the frame count to the nearest frame", func() { + out := readAll(pipedMP3, 0.99) // 37.9 frames + _, _, frames := readXing(out, fixtureID3Len, stereoTagOffset) + Expect(frames).To(Equal(uint32(38))) + }) + + It("places the tag after the shorter side info of a mono stream", func() { + in := monoMP3() + + out := readAll(in, 1.0) + + tag, _, frames := readXing(out, 0, monoTagOffset) + Expect(tag).To(Equal("Xing")) + Expect(frames).To(Equal(uint32(38))) + }) + + It("inserts the frame at the start of a stream with no ID3 tag", func() { + in := pipedMP3[fixtureID3Len:] + + out := readAll(in, 1.0) + + tag, _, _ := readXing(out, 0, stereoTagOffset) + Expect(tag).To(Equal("Xing")) + Expect(out[fixtureFrameLen:]).To(Equal(in)) + }) + + It("leaves a stream that already declares its duration alone", func() { + patched := readAll(pipedMP3, 1.0) + Expect(readAll(patched, 99.0)).To(Equal(patched)) + }) + + It("passes through when the duration is zero or negative", func() { + Expect(readAll(pipedMP3, 0)).To(Equal(pipedMP3)) + Expect(readAll(pipedMP3, -5)).To(Equal(pipedMP3)) + }) + + It("passes through a stream that is not mp3", func() { + in := []byte("fLaC\x00\x00\x00\x22 and then some bytes that are not frames") + Expect(readAll(in, 1.0)).To(Equal(in)) + }) + + It("passes through a frame header with a reserved bitrate or sample rate", func() { + in := monoMP3() + in[2] |= 0xF0 // bitrate index 15 is invalid + Expect(readAll(in, 1.0)).To(Equal(in)) + + in = monoMP3() + in[2] |= 0x0C // sample rate index 3 is reserved + Expect(readAll(in, 1.0)).To(Equal(in)) + }) + + It("passes through a stream too short to hold a frame header", func() { + in := monoMP3()[:3] + Expect(readAll(in, 1.0)).To(Equal(in)) + }) + + It("passes through a stream whose ID3 tag never ends", func() { + in := bytes.Clone(pipedMP3[:200]) + Expect(readAll(in, 1.0)).To(Equal(in)) + }) + + It("passes through an empty stream", func() { + Expect(readAll(nil, 1.0)).To(BeEmpty()) + }) + + It("propagates a read error from the underlying stream", func() { + r := patchMP3Duration(io.NopCloser(iotest.ErrReader(errors.New("ffmpeg died"))), 1.0) + _, err := io.ReadAll(r) + Expect(err).To(MatchError(ContainSubstring("ffmpeg died"))) + }) +}) + +// monoMP3 is two silent MPEG1 Layer III frames, 128kbps, 44100Hz, mono. +func monoMP3() []byte { + b := make([]byte, 2*417) + copy(b, []byte{0xFF, 0xFB, 0x90, 0xC0}) + copy(b[417:], []byte{0xFF, 0xFB, 0x90, 0xC0}) + return b +}