From 76df3bb7faae8e0fc2fa651ce65dea5c93c04f0b Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 24 Sep 2026 16:42:35 -0400 Subject: [PATCH 1/6] fix(stream): compute the estimated content length in kilobits of 1000 bits estimateContentLength=true announced Duration * bitRate / 8 * 1024 bytes, but a bitrate in kbps is 1000 bits per second, so the header overstated the body by 2.4%. A client that trusts it waits for bytes that never arrive: on a 285s track at 128kbps it asked for 4,669,440 bytes of a 4,561,876 byte body. Reported in #6170. --- core/stream/media_streamer.go | 2 +- core/stream/media_streamer_test.go | 12 ++++++++++++ 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/core/stream/media_streamer.go b/core/stream/media_streamer.go index 6db2f6338..01b67030a 100644 --- a/core/stream/media_streamer.go +++ b/core/stream/media_streamer.go @@ -147,7 +147,7 @@ func (s *Stream) ContentType() string { return mime.TypeByExtension("." + s.form func (s *Stream) Name() string { return s.mf.Title + "." + s.format } func (s *Stream) ModTime() time.Time { return s.mf.UpdatedAt } func (s *Stream) EstimatedContentLength() int { - return int(s.mf.Duration * float32(s.bitRate) / 8 * 1024) + return int(s.mf.Duration * float32(s.bitRate) / 8 * 1000) } // Serve writes the stream to the HTTP response. For seekable streams it uses http.ServeContent diff --git a/core/stream/media_streamer_test.go b/core/stream/media_streamer_test.go index e06599208..e74fb10d4 100644 --- a/core/stream/media_streamer_test.go +++ b/core/stream/media_streamer_test.go @@ -183,6 +183,18 @@ var _ = Describe("MediaStreamer", func() { _, err = io.ReadAll(resp.Body) Expect(err).To(HaveOccurred()) }) + + It("estimates the content length from a bitrate in kilobits of 1000 bits", func() { + hundredSeconds := *mf + hundredSeconds.Duration = 100 + s := stream.NewStream(&hundredSeconds, "mp3", 128, io.NopCloser(bytes.NewReader(nil))) + w := httptest.NewRecorder() + r := httptest.NewRequest(http.MethodGet, "/?estimateContentLength=true", nil) + + _, _ = s.Serve(ctx, w, r) + + Expect(w.Header().Get("Content-Length")).To(Equal("1600000")) + }) }) }) From c05fbd0edbb7719601d1759e2e2eafeebb68c7d2 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 24 Sep 2026 16:42:48 -0400 Subject: [PATCH 2/6] 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 +} From 8dee15501fa8ec59a64b3293cfaf3ffd8b64d180 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 24 Sep 2026 16:50:41 -0400 Subject: [PATCH 3/6] refactor(transcoding): repair piped headers behind one patcher The FLAC and mp3 repairs had grown into two copies of the same wrapper: the same lazy peek on the first Read, the same MultiReader hand-off, the same tolerance of a short read, and the same comment twice. They now share one headerPatcher, with a prefix function per container. Transcode no longer selects the patcher by opts.Format. That string is the target format of a transcoding profile, which users can name anything, so a row called 'mp3 320' running the default mp3 command got no repair at all, while a row whose command emits FLAC got one only because the FLAC arm was the default. The shared peek dispatches on the magic bytes instead, which both patchers already had to check, and leaves anything it does not recognize untouched. Also drops b2i in favour of gg.If, and lowers the ID3 bound to 64KB: ffmpeg cannot write an attached picture to a non-seekable output, so the tag ffmpeg pipes out stays small. --- core/ffmpeg/ffmpeg.go | 10 +--- core/ffmpeg/flac_streaminfo.go | 43 ++++------------- core/ffmpeg/flac_streaminfo_test.go | 6 +-- core/ffmpeg/mp3_xing.go | 74 +++++------------------------ core/ffmpeg/mp3_xing_test.go | 30 ++++++------ core/ffmpeg/piped_header.go | 61 ++++++++++++++++++++++++ 6 files changed, 102 insertions(+), 122 deletions(-) create mode 100644 core/ffmpeg/piped_header.go diff --git a/core/ffmpeg/ffmpeg.go b/core/ffmpeg/ffmpeg.go index c56ad1a69..fbc66688b 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 or mp3 header. + Duration float32 // seconds; 0 = unknown. Only used to repair a header ffmpeg piped out unfinished. } // AudioProbeResult contains authoritative audio stream properties from ffprobe. @@ -91,13 +91,7 @@ func (e *ffmpeg) Transcode(ctx context.Context, opts TranscodeOptions) (io.ReadC if err != nil { return nil, err } - duration := opts.Duration - float32(opts.Offset) - switch opts.Format { - case "mp3": - return patchMP3Duration(out, duration), nil - default: - return patchFLACDuration(out, duration), nil - } + return patchPipedHeader(out, opts.Duration-float32(opts.Offset)), nil } func (e *ffmpeg) ConvertAnimatedImage(ctx context.Context, reader io.Reader, maxSize int, quality int) (io.ReadCloser, error) { diff --git a/core/ffmpeg/flac_streaminfo.go b/core/ffmpeg/flac_streaminfo.go index 878c28718..f4e70f3a9 100644 --- a/core/ffmpeg/flac_streaminfo.go +++ b/core/ffmpeg/flac_streaminfo.go @@ -1,10 +1,7 @@ package ffmpeg import ( - "bytes" "encoding/binary" - "errors" - "io" "math" ) @@ -13,43 +10,21 @@ const ( flacMaxTotalSamples = 1<<36 - 1 ) -// patchFLACDuration fills in the STREAMINFO total_samples that ffmpeg leaves at 0 -// when writing to a pipe, since a decoder cannot seek a cached FLAC without it. -func patchFLACDuration(r io.ReadCloser, duration float32) io.ReadCloser { - if duration <= 0 { - return r +// flacPrefix fills in the STREAMINFO total_samples that ffmpeg leaves at 0 when +// writing to a pipe, since a decoder cannot seek a cached FLAC without it. +func (h *headerPatcher) flacPrefix(buf []byte) ([]byte, error) { + buf, err := h.fill(buf, flacPrefixLen) + if err != nil || len(buf) < flacPrefixLen { + return buf, err } - return &flacPatcher{ReadCloser: r, duration: duration} -} - -type flacPatcher 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 (f *flacPatcher) Read(p []byte) (int, error) { - if f.stream == nil { - prefix := make([]byte, flacPrefixLen) - n, err := io.ReadFull(f.ReadCloser, prefix) - if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) { - return 0, err - } - prefix = prefix[:n] - if err == nil { - setFLACTotalSamples(prefix, f.duration) - } - f.stream = io.MultiReader(bytes.NewReader(prefix), f.ReadCloser) - } - return f.stream.Read(p) + setFLACTotalSamples(buf, h.duration) + return buf, nil } // setFLACTotalSamples takes the rate from the header rather than the transcode // options, so a resampled (-ar) output still gets the right count. func setFLACTotalSamples(prefix []byte, duration float32) { - if string(prefix[:4]) != "fLaC" || prefix[4]&0x7F != 0 { + if prefix[4]&0x7F != 0 { // the first metadata block must be STREAMINFO return } // 20-bit rate | 3-bit channels | 5-bit depth | 36-bit total_samples diff --git a/core/ffmpeg/flac_streaminfo_test.go b/core/ffmpeg/flac_streaminfo_test.go index 6bf3503d7..741f74b53 100644 --- a/core/ffmpeg/flac_streaminfo_test.go +++ b/core/ffmpeg/flac_streaminfo_test.go @@ -31,7 +31,7 @@ var _ = Describe("patchFLACDuration", func() { } readAll := func(in []byte, duration float32) []byte { - out, err := io.ReadAll(patchFLACDuration(io.NopCloser(bytes.NewReader(in)), duration)) + out, err := io.ReadAll(patchPipedHeader(io.NopCloser(bytes.NewReader(in)), duration)) Expect(err).ToNot(HaveOccurred()) return out } @@ -118,14 +118,14 @@ var _ = Describe("patchFLACDuration", func() { }) It("propagates a read error from the underlying stream", func() { - _, err := io.ReadAll(patchFLACDuration(io.NopCloser(io.MultiReader( + _, err := io.ReadAll(patchPipedHeader(io.NopCloser(io.MultiReader( bytes.NewReader(pipedFLAC()[:10]), &errReader{})), 1.0)) Expect(err).To(MatchError("boom")) }) It("closes the underlying stream", func() { c := &closeSpy{Reader: bytes.NewReader(pipedFLAC())} - Expect(patchFLACDuration(c, 1.0).Close()).To(Succeed()) + Expect(patchPipedHeader(c, 1.0).Close()).To(Succeed()) Expect(c.closed).To(BeTrue()) }) }) diff --git a/core/ffmpeg/mp3_xing.go b/core/ffmpeg/mp3_xing.go index 125469605..fd2c8524b 100644 --- a/core/ffmpeg/mp3_xing.go +++ b/core/ffmpeg/mp3_xing.go @@ -1,52 +1,24 @@ package ffmpeg import ( - "bytes" "encoding/binary" - "errors" - "io" "math" "slices" + + "github.com/navidrome/navidrome/utils/gg" ) const ( mp3HeaderLen = 4 mp3ID3Len = 10 - mp3MaxPrefix = 1 << 20 // an ID3 tag carrying cover art still fits + mp3MaxPrefix = 64 << 10 // ffmpeg cannot write an attached picture to a pipe, so the tag stays small ) -// 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) +// mp3Prefix returns the head of the stream with a Xing frame inserted before the first +// audio frame, which ffmpeg omits on a pipe since it only knows the frame count once it +// can rewind. It returns the head unchanged when it cannot make sense of what ffmpeg wrote. +func (h *headerPatcher) mp3Prefix(buf []byte) ([]byte, error) { + buf, err := h.fill(buf, mp3ID3Len) if err != nil || len(buf) < mp3ID3Len { return buf, err } @@ -56,39 +28,26 @@ func (m *mp3Patcher) peek() ([]byte, error) { return buf, nil } } - if buf, err = m.fill(buf, start+mp3HeaderLen); err != nil || len(buf) < start+mp3HeaderLen { + if buf, err = h.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 { + if buf, err = h.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) + xing, ok := xingFrame(buf[start:], frame, h.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 @@ -102,7 +61,7 @@ type mp3Frame struct { samples int // per frame size int // bytes, including the header sideInfo int - tagOffset int // where a Xing tag would sit in this frame + tagOffset int // where a Xing tag sits in the incoming frame, which may carry a CRC } var ( @@ -125,7 +84,7 @@ func parseMP3Header(h []byte) (mp3Frame, bool) { } mpeg1 := version == 3 sampleRate := mp3SampleRates[version][(h[2]>>2)&0x03] - bitRate := 1000 * mp3BitRates[b2i(!mpeg1)][h[2]>>4] + bitRate := 1000 * mp3BitRates[gg.If(mpeg1, 0, 1)][h[2]>>4] if sampleRate == 0 || bitRate == 0 { return mp3Frame{}, false } @@ -173,10 +132,3 @@ func xingFrame(first []byte, f mp3Frame, duration float32) ([]byte, bool) { 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 index 7d66268c2..2a66454c8 100644 --- a/core/ffmpeg/mp3_xing_test.go +++ b/core/ffmpeg/mp3_xing_test.go @@ -34,26 +34,25 @@ var _ = Describe("patchMP3Duration", func() { }) readAll := func(in []byte, duration float32) []byte { - out, err := io.ReadAll(patchMP3Duration(io.NopCloser(bytes.NewReader(in)), duration)) + out, err := io.ReadAll(patchPipedHeader(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) { + // Reads the tag and frame count of the frame starting at frameStart. + readXing := func(b []byte, frameStart, tagOffset int) (string, uint32) { at := frameStart + tagOffset - return string(b[at : at+4]), - binary.BigEndian.Uint32(b[at+4:]), - binary.BigEndian.Uint32(b[at+8:]) + return string(b[at : 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) + tag, 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") + flags := binary.BigEndian.Uint32(out[fixtureID3Len+stereoTagOffset+4:]) + Expect(flags).To(Equal(uint32(1)), "only the frame count is present") }) It("leaves the ID3 tag and the audio frames untouched", func() { @@ -72,13 +71,13 @@ var _ = Describe("patchMP3Duration", func() { out := readAll(in, 1.0) - _, _, frames := readXing(out, start, stereoTagOffset) + _, 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) + _, frames := readXing(out, fixtureID3Len, stereoTagOffset) Expect(frames).To(Equal(uint32(38))) }) @@ -87,7 +86,7 @@ var _ = Describe("patchMP3Duration", func() { out := readAll(in, 1.0) - tag, _, frames := readXing(out, 0, monoTagOffset) + tag, frames := readXing(out, 0, monoTagOffset) Expect(tag).To(Equal("Xing")) Expect(frames).To(Equal(uint32(38))) }) @@ -97,7 +96,7 @@ var _ = Describe("patchMP3Duration", func() { out := readAll(in, 1.0) - tag, _, _ := readXing(out, 0, stereoTagOffset) + tag, _ := readXing(out, 0, stereoTagOffset) Expect(tag).To(Equal("Xing")) Expect(out[fixtureFrameLen:]).To(Equal(in)) }) @@ -142,16 +141,15 @@ var _ = Describe("patchMP3Duration", func() { }) It("propagates a read error from the underlying stream", func() { - r := patchMP3Duration(io.NopCloser(iotest.ErrReader(errors.New("ffmpeg died"))), 1.0) + r := patchPipedHeader(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. +// monoMP3 is one silent MPEG1 Layer III frame, 128kbps, 44100Hz, mono. func monoMP3() []byte { - b := make([]byte, 2*417) + b := make([]byte, 417) copy(b, []byte{0xFF, 0xFB, 0x90, 0xC0}) - copy(b[417:], []byte{0xFF, 0xFB, 0x90, 0xC0}) return b } diff --git a/core/ffmpeg/piped_header.go b/core/ffmpeg/piped_header.go new file mode 100644 index 000000000..71f9ee754 --- /dev/null +++ b/core/ffmpeg/piped_header.go @@ -0,0 +1,61 @@ +package ffmpeg + +import ( + "bytes" + "errors" + "io" +) + +// patchPipedHeader repairs the header ffmpeg cannot finish when its output is a pipe, +// since it only learns the sample or frame count once it can no longer rewind. +func patchPipedHeader(r io.ReadCloser, duration float32) io.ReadCloser { + if duration <= 0 { + return r + } + return &headerPatcher{ReadCloser: r, duration: duration} +} + +type headerPatcher 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 (h *headerPatcher) Read(p []byte) (int, error) { + if h.stream == nil { + prefix, err := h.peek() + if err != nil { + return 0, err + } + h.stream = io.MultiReader(bytes.NewReader(prefix), h.ReadCloser) + } + return h.stream.Read(p) +} + +// peek dispatches on the magic bytes rather than on the requested format, which is a +// free-text label on the transcoding profile and need not match what ffmpeg emits. +func (h *headerPatcher) peek() ([]byte, error) { + buf, err := h.fill(nil, 4) + if err != nil || len(buf) < 4 { + return buf, err + } + if string(buf) == "fLaC" { + return h.flacPrefix(buf) + } + return h.mp3Prefix(buf) +} + +// fill grows buf to n bytes, stopping short when the stream ends first. +func (h *headerPatcher) fill(buf []byte, n int) ([]byte, error) { + if len(buf) >= n { + return buf, nil + } + more := make([]byte, n-len(buf)) + read, err := io.ReadFull(h.ReadCloser, more) + if err != nil && !errors.Is(err, io.EOF) && !errors.Is(err, io.ErrUnexpectedEOF) { + return buf, err + } + return append(buf, more[:read]...), nil +} From 79fc6fed6039bfc6763f6225c1b66ac8247f3df0 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 24 Sep 2026 17:05:05 -0400 Subject: [PATCH 4/6] fix(stream): keep the headroom in the estimated content length 5686c352c changed the estimate from bitRate / 8 * 1024 to * 1000, reading the 1024 as a unit mistake. It was headroom: the body also carries the ID3 tag, the Xing frame and frame padding, so a nominal-size estimate is short, and net/http aborts a response that writes past its declared Content-Length. Measured on a live server, a 429s track at 64kbps was estimated at 3,439,840 bytes against a 3,441,569-byte body, and the stream was cut with 'http: wrote more than the declared Content-Length'. Restores the 1024, says why next to it, and turns the test into one that asserts the headroom rather than a formula. --- core/stream/media_streamer.go | 5 ++++- core/stream/media_streamer_test.go | 7 +++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/core/stream/media_streamer.go b/core/stream/media_streamer.go index 01b67030a..3efd67a81 100644 --- a/core/stream/media_streamer.go +++ b/core/stream/media_streamer.go @@ -146,8 +146,11 @@ func (s *Stream) Duration() float32 { return s.mf.Duration } func (s *Stream) ContentType() string { return mime.TypeByExtension("." + s.format) } func (s *Stream) Name() string { return s.mf.Title + "." + s.format } func (s *Stream) ModTime() time.Time { return s.mf.UpdatedAt } + +// EstimatedContentLength deliberately overshoots by 2.4%: the body also carries tags and +// padding, and net/http aborts a response that writes past its declared length. func (s *Stream) EstimatedContentLength() int { - return int(s.mf.Duration * float32(s.bitRate) / 8 * 1000) + return int(s.mf.Duration * float32(s.bitRate) / 8 * 1024) } // Serve writes the stream to the HTTP response. For seekable streams it uses http.ServeContent diff --git a/core/stream/media_streamer_test.go b/core/stream/media_streamer_test.go index e74fb10d4..d0b694613 100644 --- a/core/stream/media_streamer_test.go +++ b/core/stream/media_streamer_test.go @@ -8,6 +8,7 @@ import ( "net/http" "net/http/httptest" "os" + "strconv" "testing/iotest" "time" @@ -184,7 +185,7 @@ var _ = Describe("MediaStreamer", func() { Expect(err).To(HaveOccurred()) }) - It("estimates the content length from a bitrate in kilobits of 1000 bits", func() { + It("estimates a content length above the nominal size, so the body never outruns it", func() { hundredSeconds := *mf hundredSeconds.Duration = 100 s := stream.NewStream(&hundredSeconds, "mp3", 128, io.NopCloser(bytes.NewReader(nil))) @@ -193,7 +194,9 @@ var _ = Describe("MediaStreamer", func() { _, _ = s.Serve(ctx, w, r) - Expect(w.Header().Get("Content-Length")).To(Equal("1600000")) + length, err := strconv.Atoi(w.Header().Get("Content-Length")) + Expect(err).ToNot(HaveOccurred()) + Expect(length).To(BeNumerically(">", 100*128*1000/8)) }) }) }) From e918b1278b810fc175ebaf71ea75dd0f3e807d06 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 24 Sep 2026 17:08:33 -0400 Subject: [PATCH 5/6] fix(transcoding): tag the inserted mp3 frame as Info, not Xing The frame inserted into piped mp3 transcodes carried a Xing tag with only a frame count. Media3/ExoPlayer, which most Android clients use, builds a XingSeeker from a Xing tag and reports it seekable only when a seek table is present, so a cached or downloaded transcode became unseekable there. Without any frame it would have used constant-bitrate seeking. An Info tag takes Media3 down its constant-bitrate path, now with the declared duration. Info is also what ffmpeg and LAME write for a CBR file, which is what the default mp3 profile produces. Checked against the decoders on hand: ffprobe reports 20.010s either way, Firefox reports 19.998s at loadedmetadata either way, and decoded audio is unchanged. Apple's AudioToolbox ignores Info frames, including ffmpeg's own, so it sees the same stream it saw before this series. --- core/ffmpeg/ffmpeg_test.go | 4 ++-- core/ffmpeg/mp3_xing.go | 6 +++--- core/ffmpeg/mp3_xing_test.go | 9 +++++---- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/core/ffmpeg/ffmpeg_test.go b/core/ffmpeg/ffmpeg_test.go index 79c46176b..9b0c7f6d4 100644 --- a/core/ffmpeg/ffmpeg_test.go +++ b/core/ffmpeg/ffmpeg_test.go @@ -722,7 +722,7 @@ var _ = Describe("ffmpeg", func() { Expect(readTotalSamples(out)).To(Equal(uint64(2 * 44100))) }) - It("inserts a Xing frame on a piped mp3 transcode", func() { + It("inserts an Info 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", @@ -734,7 +734,7 @@ var _ = Describe("ffmpeg", func() { out, err := io.ReadAll(stream) Expect(err).ToNot(HaveOccurred()) - at := bytes.Index(out, []byte("Xing")) + at := bytes.Index(out, []byte("Info")) Expect(at).To(BeNumerically(">", 0)) Expect(binary.BigEndian.Uint32(out[at+8:])).To(Equal(uint32(38))) }) diff --git a/core/ffmpeg/mp3_xing.go b/core/ffmpeg/mp3_xing.go index fd2c8524b..dcfb08790 100644 --- a/core/ffmpeg/mp3_xing.go +++ b/core/ffmpeg/mp3_xing.go @@ -14,7 +14,7 @@ const ( 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 a Xing frame inserted before the first +// mp3Prefix returns the head of the stream with an Info frame inserted before the first // audio frame, which ffmpeg omits on a pipe since it only knows the frame count once it // can rewind. It returns the head unchanged when it cannot make sense of what ffmpeg wrote. func (h *headerPatcher) mp3Prefix(buf []byte) ([]byte, error) { @@ -116,7 +116,7 @@ func isXingFrame(frame []byte, tagOffset int) bool { return tag == "Xing" || tag == "Info" } -// xingFrame builds a silent frame declaring how many frames follow it. It reuses the +// 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) { frames := math.Round(float64(duration) * float64(f.sampleRate) / float64(f.samples)) @@ -127,7 +127,7 @@ func xingFrame(first []byte, f mp3Frame, duration float32) ([]byte, bool) { 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") + 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)) return frame, true diff --git a/core/ffmpeg/mp3_xing_test.go b/core/ffmpeg/mp3_xing_test.go index 2a66454c8..a5fca40b8 100644 --- a/core/ffmpeg/mp3_xing_test.go +++ b/core/ffmpeg/mp3_xing_test.go @@ -31,6 +31,7 @@ var _ = Describe("patchMP3Duration", func() { Expect(err).ToNot(HaveOccurred()) Expect(pipedMP3[fixtureID3Len : fixtureID3Len+2]).To(Equal([]byte{0xFF, 0xFB})) Expect(bytes.Contains(pipedMP3[:4096], []byte("Xing"))).To(BeFalse()) + Expect(bytes.Contains(pipedMP3[:4096], []byte("Info"))).To(BeFalse()) }) readAll := func(in []byte, duration float32) []byte { @@ -45,11 +46,11 @@ var _ = Describe("patchMP3Duration", func() { return string(b[at : at+4]), binary.BigEndian.Uint32(b[at+8:]) } - It("inserts a Xing frame declaring the duration in frames", func() { + It("inserts an Info frame declaring the duration in frames", func() { out := readAll(pipedMP3, 1.0) tag, frames := readXing(out, fixtureID3Len, stereoTagOffset) - Expect(tag).To(Equal("Xing")) + Expect(tag).To(Equal("Info")) Expect(frames).To(Equal(uint32(38)), "1s at 44100Hz is 38 frames of 1152 samples") flags := binary.BigEndian.Uint32(out[fixtureID3Len+stereoTagOffset+4:]) Expect(flags).To(Equal(uint32(1)), "only the frame count is present") @@ -87,7 +88,7 @@ var _ = Describe("patchMP3Duration", func() { out := readAll(in, 1.0) tag, frames := readXing(out, 0, monoTagOffset) - Expect(tag).To(Equal("Xing")) + Expect(tag).To(Equal("Info")) Expect(frames).To(Equal(uint32(38))) }) @@ -97,7 +98,7 @@ var _ = Describe("patchMP3Duration", func() { out := readAll(in, 1.0) tag, _ := readXing(out, 0, stereoTagOffset) - Expect(tag).To(Equal("Xing")) + Expect(tag).To(Equal("Info")) Expect(out[fixtureFrameLen:]).To(Equal(in)) }) From ec49ae476f3ae6f852a2eea6ecc82c3652a6c8e6 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 24 Sep 2026 17:13:44 -0400 Subject: [PATCH 6/6] 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:]