Skip to content

Commit 8a6b8df

Browse files
author
Datanoise
committed
relay: fix stale-burst replay and false Ogg page alignment
Several related bugs in the audio path together caused two user-visible symptoms: new listeners hearing old audio when the source was idle, and glitched / shuffled audio on otherwise-healthy streams. - Stream.Subscribe was bursting from Buffer.Head minus burst-size unconditionally, so when a producer had been silent for a while a new listener received seconds of stale buffered audio. Add a FreshHead watermark plus a LastDataReceived > 2s freshness gate, and have all producer entry points (icecast SOURCE, relay pull, WebRTC source, transcoder, AutoDJ MP3/Opus encoders) call a new Stream.BeginSession() that advances FreshHead and wipes stale Ogg state at the start of each session. - Stream.Broadcast tracked any byte sequence matching 'OggS' as a page boundary, including false positives inside Opus packet payloads. Validate the Ogg version byte (must be 0) before recording an offset; same validation in the new CircularBuffer.AlignToOggPage replacing the unsafe free function in relay/ogg.go. - Transcoder restarts re-initialised the Opus encoder with a fresh serial number but left the old PageOffsets / OggHead behind, so listeners could be aligned to bytes from a previous Ogg serial. BeginSession now resets that state. - OggHead / OggHeaderOffset and Buffer.Head / Buffer.Data were read and written without locking from multiple goroutines. Add Stream.SetOggHead / GetOggHead (defensive copy under lock) and CircularBuffer.SnapshotHead / AlignToOggPage (under the buffer's own lock) and route all callers through them. - Subscribe's page-alignment fallback initialised bestAlign to LastPageOffset and could end up walking the listener backwards past the requested start. Search for the smallest tracked offset >= start and otherwise fall back to Head (never backwards).
1 parent db4fd8f commit 8a6b8df

8 files changed

Lines changed: 242 additions & 85 deletions

File tree

relay/buffer.go

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@ import (
99
"sync"
1010
)
1111

12+
// oggMagic is the 4-byte capture pattern at the start of every Ogg page header.
13+
var oggMagic = [4]byte{'O', 'g', 'g', 'S'}
14+
1215
// CircularBuffer is a thread-safe fixed-size ring buffer for stream data.
1316
//
1417
// This buffer implementation is optimized for audio streaming scenarios where
@@ -128,3 +131,82 @@ func (cb *CircularBuffer) ReadAt(start int64, p []byte) (int, int64, bool) {
128131
actual := copy(p, cb.Data[pos:pos+n])
129132
return actual, start + int64(actual), skipped
130133
}
134+
135+
// SnapshotHead returns the current write position under the buffer lock.
136+
// Callers that need a consistent Head value without copying data should use
137+
// this rather than reading cb.Head directly, which races with Write.
138+
func (cb *CircularBuffer) SnapshotHead() int64 {
139+
cb.mu.RLock()
140+
defer cb.mu.RUnlock()
141+
return cb.Head
142+
}
143+
144+
// AlignToOggPage scans the buffer for the next valid Ogg page header at or
145+
// after the absolute offset 'start' and returns its offset. If no valid page
146+
// is found within the buffer window, it returns the current Head (i.e. "start
147+
// from now").
148+
//
149+
// A valid header here means the 4-byte "OggS" capture pattern followed by a
150+
// version byte of 0. This rejects the false positives that occur when the
151+
// byte sequence 0x4F 0x67 0x67 0x53 appears inside Opus packet payloads and
152+
// would otherwise misalign listeners mid-packet.
153+
//
154+
// The whole scan runs under the buffer's read lock so the data isn't being
155+
// overwritten beneath us while we search.
156+
func (cb *CircularBuffer) AlignToOggPage(start int64) int64 {
157+
cb.mu.RLock()
158+
defer cb.mu.RUnlock()
159+
160+
head := cb.Head
161+
size := cb.Size
162+
163+
// Clamp to the valid buffer window.
164+
if start < head-size {
165+
start = head - size
166+
}
167+
if start < 0 {
168+
start = 0
169+
}
170+
// Need at least the 4-byte magic plus the version byte.
171+
if start >= head-5 {
172+
return head
173+
}
174+
175+
for i := start; i < head-5; {
176+
pos := i % size
177+
// Scan in segments that don't cross the physical buffer boundary.
178+
end := size
179+
if i+size > head {
180+
end = pos + (head - i)
181+
}
182+
if end > size {
183+
end = size
184+
}
185+
186+
segment := cb.Data[pos:end]
187+
// Find "OggS" within the segment. Don't include the last 4 bytes
188+
// because we need to peek at the version byte that follows.
189+
searchEnd := len(segment) - 4
190+
if searchEnd < 0 {
191+
searchEnd = 0
192+
}
193+
for j := 0; j < searchEnd; j++ {
194+
if segment[j] == oggMagic[0] &&
195+
segment[j+1] == oggMagic[1] &&
196+
segment[j+2] == oggMagic[2] &&
197+
segment[j+3] == oggMagic[3] &&
198+
segment[j+4] == 0 { // Ogg version byte
199+
return i + int64(j)
200+
}
201+
}
202+
203+
// Step forward, overlapping by 4 bytes so an "OggS" straddling the
204+
// segment boundary still gets caught next iteration.
205+
advance := int64(len(segment)) - 4
206+
if advance <= 0 {
207+
break
208+
}
209+
i += advance
210+
}
211+
return head
212+
}

relay/client.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -151,6 +151,10 @@ func (rm *RelayManager) performPull(ctx context.Context, inst *RelayInstance) {
151151

152152
stream := rm.relay.GetOrCreateStream(inst.Mount)
153153
stream.SourceIP = "relay-pull"
154+
// Each (re)connect is a new producer session — wipe any stale audio /
155+
// Ogg state left over from a previous pull so reconnecting listeners
156+
// don't get bursted with content from before the disconnect.
157+
stream.BeginSession()
154158

155159
// Metadata
156160
name := resp.Header.Get("Ice-Name")

relay/io.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,10 @@ func (r *StreamReader) Read(p []byte) (int, error) {
111111

112112
// Handle Ogg synchronization if enabled
113113
if skipped && r.oggSync && r.stream != nil {
114-
r.stream.mu.RLock()
115-
r.offset = FindNextPageBoundary(r.stream.Buffer.Data, r.stream.Buffer.Size, r.stream.Buffer.Head, next)
116-
r.stream.mu.RUnlock()
114+
// AlignToOggPage scans under the buffer's own read lock and
115+
// validates the Ogg version byte, so we can't drift into a
116+
// false positive inside a payload.
117+
r.offset = r.buffer.AlignToOggPage(next)
117118
continue // Retry read at aligned offset
118119
}
119120

relay/ogg.go

Lines changed: 0 additions & 36 deletions
This file was deleted.

relay/stream.go

Lines changed: 117 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,12 @@ type Stream struct {
7676
PageOffsets []int64 // Circular list of last ~100 page starts
7777
PageIndex int // Index for managing PageOffsets circular list
7878

79+
// FreshHead marks the oldest buffer offset that belongs to the current
80+
// playback session. Subscribe() will never hand out a start offset older
81+
// than this, which prevents bursting stale audio from a prior source.
82+
// Updated by BeginSession() whenever a new source/encoder takes over.
83+
FreshHead int64
84+
7985
// Core streaming infrastructure
8086
Buffer *CircularBuffer // Audio data buffer (typically 2MB)
8187
listeners map[string]chan struct{} // Signal channels for connected listeners
@@ -146,6 +152,58 @@ func (s *Stream) DisconnectListeners() {
146152
}
147153
}
148154

155+
// BeginSession marks the start of a new producer session on this stream
156+
// (new source client, new transcoder run, new AutoDJ encoder, ...). It:
157+
//
158+
// - advances FreshHead so Subscribe never bursts audio from before this point,
159+
// - clears stale Ogg page tracking (different Ogg serial / different format
160+
// would otherwise leave PageOffsets pointing at unrelated bytes),
161+
// - clears OggHead until the new producer captures fresh headers.
162+
//
163+
// Producers should call this once, before they start writing audio to the
164+
// stream's buffer.
165+
func (s *Stream) BeginSession() {
166+
s.mu.Lock()
167+
defer s.mu.Unlock()
168+
169+
head := s.Buffer.SnapshotHead()
170+
s.FreshHead = head
171+
s.OggHeaderOffset = head
172+
s.OggHead = nil
173+
s.LastPageOffset = 0
174+
s.PageIndex = 0
175+
for i := range s.PageOffsets {
176+
s.PageOffsets[i] = 0
177+
}
178+
}
179+
180+
// SetOggHead stores the captured Ogg ID/Tags headers and the buffer offset
181+
// at which the first audio page starts. Stored under lock so concurrent
182+
// listener subscriptions see a consistent snapshot (no torn slice).
183+
func (s *Stream) SetOggHead(headers []byte, offset int64) {
184+
cp := make([]byte, len(headers))
185+
copy(cp, headers)
186+
187+
s.mu.Lock()
188+
s.OggHead = cp
189+
s.OggHeaderOffset = offset
190+
s.mu.Unlock()
191+
}
192+
193+
// GetOggHead returns a defensive copy of the stored Ogg headers, or nil if
194+
// none have been captured yet. The copy means callers can safely write the
195+
// bytes to a network connection without racing the next SetOggHead.
196+
func (s *Stream) GetOggHead() []byte {
197+
s.mu.RLock()
198+
defer s.mu.RUnlock()
199+
if s.OggHead == nil {
200+
return nil
201+
}
202+
cp := make([]byte, len(s.OggHead))
203+
copy(cp, s.OggHead)
204+
return cp
205+
}
206+
149207
// Broadcast sends data to all listeners via the shared circular buffer.
150208
//
151209
// This is the core method that distributes audio data from sources to all connected
@@ -188,11 +246,20 @@ func (s *Stream) Broadcast(data []byte, relay *Relay) {
188246
atomic.AddInt64(&relay.BytesIn, int64(len(data)))
189247
atomic.AddInt64(&s.BytesIn, int64(len(data)))
190248

191-
// Track Ogg Page boundaries for alignment
192-
// This enables new Opus listeners to start at proper page boundaries
249+
// Track Ogg page boundaries for listener alignment.
250+
//
251+
// We MUST validate the byte that follows the "OggS" magic: the four
252+
// bytes 0x4F 0x67 0x67 0x53 also occur inside Opus packet payloads, and
253+
// without validation those false positives end up in PageOffsets and
254+
// cause new listeners to start mid-packet (audible as glitched/shuffled
255+
// audio). Byte index 4 of a real Ogg page header is the version, which
256+
// is always 0.
257+
//
258+
// If the magic lands at the very tail of `data` (no version byte yet),
259+
// we skip it — the next Broadcast call will see the full header.
193260
if s.IsOggStream {
194-
for i := 0; i <= len(data)-4; i++ {
195-
if data[i] == 'O' && data[i+1] == 'g' && data[i+2] == 'g' && data[i+3] == 'S' {
261+
for i := 0; i <= len(data)-5; i++ {
262+
if data[i] == 'O' && data[i+1] == 'g' && data[i+2] == 'g' && data[i+3] == 'S' && data[i+4] == 0 {
196263
offset := s.Buffer.Head + int64(i)
197264
s.LastPageOffset = offset
198265
s.PageOffsets[s.PageIndex%len(s.PageOffsets)] = offset
@@ -265,53 +332,77 @@ func (s *Stream) Subscribe(id string, burstSize int) (int64, chan struct{}) {
265332
ch := make(chan struct{}, 1)
266333
s.listeners[id] = ch
267334

268-
// Start at current head minus burst size for instant playback
269-
// This gives the listener immediate audio data instead of waiting for new data
270-
start := s.Buffer.Head - int64(burstSize)
335+
head := s.Buffer.SnapshotHead()
336+
337+
// If the source has been silent for a while, the buffer holds stale
338+
// audio from a previous session. Don't burst it — start at "now" and
339+
// the listener will get fresh data as soon as the source resumes.
340+
// 2s is long enough to span normal between-frame gaps but short enough
341+
// that pauses between songs/sources don't replay old audio.
342+
if !s.LastDataReceived.IsZero() && time.Since(s.LastDataReceived) > 2*time.Second {
343+
return head, ch
344+
}
345+
346+
// Start at current head minus burst size for instant playback.
347+
start := head - int64(burstSize)
271348
if start < 0 {
272349
start = 0
273350
}
351+
// Never reach back past the current session's first byte.
352+
if start < s.FreshHead {
353+
start = s.FreshHead
354+
}
274355

275-
// For Ogg/Opus, align to the oldest known page boundary within the valid buffer range
276-
// This is crucial for proper Opus decoding - listeners MUST start at page boundaries
356+
// For Ogg/Opus, align to the oldest known page boundary within the valid
357+
// buffer range. Listeners MUST start at a page boundary or the decoder
358+
// will produce garbage.
277359
if s.IsOggStream {
278-
validStart := s.Buffer.Head - s.Buffer.Size
360+
validStart := head - s.Buffer.Size
361+
if validStart < s.FreshHead {
362+
validStart = s.FreshHead
363+
}
279364
if validStart < 0 {
280365
validStart = 0
281366
}
282367

283-
// If we have an OggHead persistent storage, we want to start reading
284-
// from the Buffer AFTER the initial headers to avoid duplicates.
368+
// The Ogg ID/Tags headers live before OggHeaderOffset; we prepend
369+
// them separately, so the buffer read must start at or after them.
285370
if s.OggHeaderOffset > start {
286371
start = s.OggHeaderOffset
287372
}
288-
289373
if start < validStart {
290374
start = validStart
291375
}
292376

293-
// Find the best page boundary that is >= start and still valid
294-
bestAlign := s.LastPageOffset
295-
found := false
377+
// Find the smallest tracked page offset that is >= start. This is
378+
// the oldest valid alignment we can hand the listener that still
379+
// honours their burst request.
380+
const noAlign = int64(-1)
381+
bestAlign := noAlign
296382
for _, po := range s.PageOffsets {
297-
// Find the smallest PO that is >= start AND is still valid
298-
if po >= start && po >= validStart && po < bestAlign {
299-
bestAlign = po
300-
found = true
383+
if po >= start && po >= validStart && po < head {
384+
if bestAlign == noAlign || po < bestAlign {
385+
bestAlign = po
386+
}
301387
}
302388
}
303-
if found {
304-
start = bestAlign
305-
} else if bestAlign >= validStart && bestAlign > 0 {
389+
if bestAlign != noAlign {
306390
start = bestAlign
391+
} else if s.LastPageOffset >= start && s.LastPageOffset < head {
392+
// Tracked offsets all wrapped out of the window, but the most
393+
// recent one is still valid and at/after our start — use it.
394+
start = s.LastPageOffset
307395
} else {
308-
start = s.Buffer.Head // Fallback to now if nothing valid found
396+
// No known page boundary at or after `start` — start from head
397+
// and the StreamReader's Ogg sync will re-align once new pages
398+
// arrive. Never walk the listener backwards.
399+
start = head
309400
}
310401
}
311402

312403
// Ensure we don't go back further than the buffer allows
313-
if s.Buffer.Head-start > s.Buffer.Size {
314-
start = s.Buffer.Head - s.Buffer.Size
404+
if head-start > s.Buffer.Size {
405+
start = head - s.Buffer.Size
315406
}
316407

317408
return start, ch

relay/transcode.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,9 @@ func EncodeMP3(ctx context.Context, relay *Relay, output *Stream, decoder io.Rea
169169
if sampleRate <= 0 {
170170
sampleRate = 44100 // Fallback
171171
}
172+
// Fresh producer session: stops listeners that join during this run from
173+
// being bursted with audio buffered by a previous source.
174+
output.BeginSession()
172175
// Shine MP3 initialization
173176
encoder := shine.NewEncoder(sampleRate, 2)
174177

@@ -233,6 +236,11 @@ func EncodeOpus(ctx context.Context, relay *Relay, output *Stream, decoder io.Re
233236
enc.SetBitrate(bitrate * 1000)
234237
}
235238

239+
// New Opus session: fresh serial number, fresh headers. Wipe any Ogg
240+
// state left over from a prior producer so subscribers don't get aligned
241+
// to page offsets that now point at bytes from a different stream.
242+
output.BeginSession()
243+
236244
// Ogg encapsulation
237245
writer := &streamWriter{stream: output, relay: relay, stats: stats, capture: true}
238246
serial := uint32(time.Now().UnixNano())
@@ -254,9 +262,9 @@ func EncodeOpus(ctx context.Context, relay *Relay, output *Stream, decoder io.Re
254262
pw.WritePacket(tagsPacket, 0, false, false)
255263
pw.Flush()
256264

257-
// Store for mid-stream listeners
258-
output.OggHead = writer.headerBuf.Bytes()
259-
output.OggHeaderOffset = output.Buffer.Head
265+
// Publish captured headers under lock so concurrent listener
266+
// subscriptions see a consistent (headers, offset) pair.
267+
output.SetOggHead(writer.headerBuf.Bytes(), output.Buffer.SnapshotHead())
260268
writer.capture = false // Stop capturing headers
261269

262270
pcmBuf := make([]byte, frameSize*channels*2)

0 commit comments

Comments
 (0)