Cap VBR frames by the buffer, not the rate

encoder.go

@@ -392,7 +392,7 @@ func (e *Encoder) EncodeFloat32(in []float32, out []byte) (int, error) {
 	if frameBytes <= 0 || frameBytes > maxOpusFrameSize {
 		return 0, fmt.Errorf("%w: %d", errInvalidFrameByteBudget, frameBytes)
 	}
-	if len(out) < frameBytes+1 {
+	if len(out) < frameBytes+tocHeaderBytes {
 		return 0, errOutBufferTooSmall
 	}
 	out[0] = byte(e.tocHeader())
@@ -401,7 +401,14 @@ func (e *Encoder) EncodeFloat32(in []float32, out []byte) (int, error) {
 	if err != nil {
 		return 0, err
 	}
-	n, err := e.celtEncoder.EncodeFrame(channels, out[1:frameBytes+1], frameBytes, startBand, endBand)
+	// VBR gets the whole buffer the caller supplied: a demanding frame may run
+	// past the nominal rate and the bit reservoir wins it back later. CBR is
+	// pinned to its share.
+	payload := out[tocHeaderBytes:]
+	if !e.vbr && len(payload) > frameBytes {
+		payload = payload[:frameBytes]
+	}
+	n, err := e.celtEncoder.EncodeFrame(channels, payload, frameBytes, startBand, endBand)
 	if err != nil {
 		return 0, err
 	}

encoder_test.go

@@ -943,3 +943,42 @@ func TestApplyStereoFadeCrossfadesOverOverlap(t *testing.T) {
 	assert.InDelta(t, 0, left[1], 1e-6, "overlap ends at the new width")
 	assert.InDelta(t, 0, left[2], 1e-6, "past the overlap the new width applies")
 }
+
+func TestVBRUsesBufferHeadroom(t *testing.T) {
+	// Unconstrained VBR may run a hard frame past the nominal rate when the
+	// caller left room, and the reservoir wins it back on the easy ones. Capping
+	// every frame at the rate is what kept pion from ever reaching its target.
+	const bitrate = 96000
+	enc, err := NewEncoder(WithChannels(2), WithBitrate(bitrate), WithVBR(true), WithConstrainedVBR(false))
+	require.NoError(t, err)
+
+	frameBudget := bitrate / 50 / 8
+	total, frames, largest := 0, 60, 0
+	phase := 0.0
+	for i := range frames {
+		pcm := make([]float32, encoderTestFrameSampleCount*2)
+		// Alternate quiet stretches with dense ones so the target has to move.
+		amp := float32(0.02)
+		if i%4 == 0 {
+			amp = 0.6
+		}
+		for j := range encoderTestFrameSampleCount {
+			v := amp * float32(math.Sin(phase)+math.Sin(phase*7.3)+math.Sin(phase*23.1))
+			pcm[2*j] = v
+			pcm[2*j+1] = -v
+			phase += 2 * math.Pi * 440 / 48000
+		}
+		packet := make([]byte, 1500)
+		n, encErr := enc.EncodeFloat32(pcm, packet)
+		require.NoError(t, encErr)
+		total += n
+		largest = max(largest, n)
+	}
+
+	// No ceiling assertion here on purpose: on a sustained hard signal there is
+	// nothing easy to win the bits back on, and the reference overshoots the
+	// nominal rate by the same margin. TestVBRTracksTargetBitrate covers the
+	// average on material that can actually be tracked.
+	assert.Greater(t, largest, frameBudget, "a demanding frame should be allowed past the nominal rate")
+	assert.Positive(t, total)
+}

internal/celt/celt.go

@@ -9,8 +9,10 @@ const (
 	// 48 kHz mode with 21 energy bands and 2.5 ms band-edge units.
 	sampleRate            = 48000
 	shortBlockSampleCount = 120
-	maxLM                 = 3
-	maxFrameSampleCount   = shortBlockSampleCount << maxLM
-	maxBands              = 21
-	hybridStartBand       = 17
+	// maxCELTFrameBytes is the largest Opus frame payload (RFC 6716 Section 3.4).
+	maxCELTFrameBytes   = 1275
+	maxLM               = 3
+	maxFrameSampleCount = shortBlockSampleCount << maxLM
+	maxBands            = 21
+	hybridStartBand     = 17
 )

internal/celt/encoder.go

@@ -566,17 +566,28 @@ func computeVBR(
 // and updates the bit reservoir/drift state that biases future frames.
 // tellFrac is e.rangeEncoder.TellFrac() at the point of the call. Mirrors
 // celt_encoder.c's VBR block around compute_vbr (~lines 2436-2530).
+// frameCeiling is how many bytes the frame may occupy. libopus caps a VBR frame
+// at the room the caller left (nbCompressedBytes), which is what lets it run
+// above the nominal rate on a hard frame; CBR stays pinned to its share.
+func (e *Encoder) frameCeiling(dst []byte, frameBytes int) int {
+	if !e.vbr {
+		return frameBytes
+	}
+
+	return min(len(dst), maxCELTFrameBytes)
+}
+
 func (e *Encoder) applyVBR(
 	frameBytes int, dr dynallocResult,
-	effectiveBytes, lm, channelCount, tellFrac int, tfEstimate float32,
+	maxBytes, lm, channelCount, tellFrac int, tfEstimate float32,
 ) int {
 	vbrRate := frameBytes << 6 // libopus vbr_rate, 1/8-bit units
 
 	if e.constrainedVBR {
 		// libopus allows any multiple of vbrRate as the bound; pion always
 		// uses 2x (vbr_bound == vbr_rate in celt_encoder.c).
 		maxAllowed := max(2, (2*vbrRate-int(e.vbrReservoir))>>6)
-		effectiveBytes = min(effectiveBytes, maxAllowed)
+		maxBytes = min(maxBytes, maxAllowed)
 	}
 
 	baseTarget := max(0, vbrRate-((40*channelCount+20)<<3))
@@ -591,8 +602,16 @@ func (e *Encoder) applyVBR(
 		baseTarget, dr.maxDepth, dr.totBoostBits, e.constrainedVBR, channelCount, lm, tfEstimate,
 	) + tellFrac
 
-	nbAvailableBytes := max(2, (rawTarget+(1<<5))>>6)
-	nbAvailableBytes = min(nbAvailableBytes, effectiveBytes)
+	// The frame still has to fit what has already been written plus the
+	// dynalloc boosts, or the range coder runs out of room (libopus
+	// min_allowed).
+	minAllowed := ((tellFrac + dr.totBoostBits + (1 << 6) - 1) >> 6) + 2
+
+	// The ceiling is how much room the caller left, not the nominal rate:
+	// unconstrained VBR is allowed to spend over the average on a hard frame
+	// and win it back later through the reservoir.
+	nbAvailableBytes := max(minAllowed, (rawTarget+(1<<5))>>6)
+	nbAvailableBytes = min(nbAvailableBytes, maxBytes)
 
 	e.updateVBRReservoir(vbrRate, rawTarget, nbAvailableBytes<<6)
 
@@ -716,6 +735,7 @@ func (e *Encoder) EncodeFrame(pcm [][]float32, dst []byte, frameBytes, startBand
 	// and tf decisions: spread_weight feeds spreadingDecision and importance
 	// feeds tfAnalysis (libopus runs dynalloc_analysis first).
 	effectiveBytes := frameBytes
+	maxBytes := e.frameCeiling(dst, frameBytes)
 	dr := dynallocAnalysis(
 		analysis.logBandAmp, e.prevLogBandAmp,
 		info.lm, info.startBand, info.endBand, info.channelCount,
@@ -756,7 +776,7 @@ func (e *Encoder) EncodeFrame(pcm [][]float32, dst []byte, frameBytes, startBand
 	tellFrac := int(e.rangeEncoder.TellFrac())
 
 	if e.vbr {
-		effectiveBytes = e.applyVBR(frameBytes, dr, effectiveBytes, info.lm, info.channelCount, tellFrac, tfEstimate)
+		effectiveBytes = e.applyVBR(frameBytes, dr, maxBytes, info.lm, info.channelCount, tellFrac, tfEstimate)
 	}
 	info.totalBits = uint(effectiveBytes) * 8
 	shapeBits := (int(info.totalBits) << bitResolution) - tellFrac - 1