diff --git a/gumble/gumbleopenal/stream.go b/gumble/gumbleopenal/stream.go index b0f2968..166da4b 100644 --- a/gumble/gumbleopenal/stream.go +++ b/gumble/gumbleopenal/stream.go @@ -53,8 +53,32 @@ const recorderOutgoingSource uint32 = ^uint32(0) const ( maxBufferSize = 11520 // Max frame size (2880) * bytes per stereo sample (4) jitterMaxPackets = 50 + // Mumble destroys and recreates AudioInput when the sender switches audio + // devices, which restarts its frame numbering at zero. The destructor + // sends no terminator, so a sender that never unkeys leaves us expecting a + // frame number the new stream will not reach for hours: every packet looks + // permanently late and gets discarded. Detect that and resync. + // + // Two conditions must hold together. A sustained run of late packets + // distinguishes a restarted stream from a clump of reordered packets, + // which is bounded and then recovers on its own. The backwards jump must + // also be too large to be network reordering; a smaller jump needs no + // intervention because the restarted stream climbs back past the stale + // expectation within jitterResyncJump frames anyway. + jitterLateResync = 5 + // Frame numbers are Mumble timestamps in 10 ms units, so this is 1 second + // — far beyond any real reordering window. + jitterResyncJump = 100 ) +// jitterShouldResync reports whether the sender restarted its frame numbering +// rather than merely delivering a few packets out of order. lateRun is the +// number of consecutive late packets and backJump is how far the current +// packet sits below the expected sequence. +func jitterShouldResync(lateRun int, backJump int64) bool { + return lateRun >= jitterLateResync && backJump >= jitterResyncJump +} + // jitterPlaybackReady holds the requested initial playout delay only once. // Requiring the delay on every packet drains and refills the renderer in bursts. func jitterPlaybackReady(started bool, buffered, target time.Duration) bool { @@ -495,6 +519,7 @@ func (s *Stream) OnAudioStream(e *gumble.AudioStreamEvent) { var jitterDuration time.Duration var jitterNextSeq int64 var jitterInit, jitterStarted bool + var jitterLateRun int var jitterDrainLogCounter, jitterAnomalyLogCounter int resetJitter := func() { jitterBuf = nil @@ -502,6 +527,7 @@ func (s *Stream) OnAudioStream(e *gumble.AudioStreamEvent) { jitterNextSeq = 0 jitterInit = false jitterStarted = false + jitterLateRun = 0 } // insertSorted inserts a packet into the jitter buffer sorted @@ -595,6 +621,17 @@ func (s *Stream) OnAudioStream(e *gumble.AudioStreamEvent) { if pkt == nil { if len(jitterBuf) > 0 { if jitterBuf[0].Sequence < jitterNextSeq { + jitterLateRun++ + if jitterShouldResync(jitterLateRun, jitterNextSeq-jitterBuf[0].Sequence) { + // The sender restarted its frame numbering + // mid-burst. Follow it instead of discarding + // every remaining packet until it unkeys. + log.Debug("jitter: sequence restart for %s, resyncing from %d to %d", + e.User.Name, jitterNextSeq, jitterBuf[0].Sequence) + jitterNextSeq = jitterBuf[0].Sequence + jitterLateRun = 0 + continue + } // Late or duplicate: discard so it doesn't // permanently block the drain loop. jitterAnomalyLogCounter++ @@ -622,6 +659,7 @@ func (s *Stream) OnAudioStream(e *gumble.AudioStreamEvent) { } break } + jitterLateRun = 0 jitterDrainLogCounter++ if jitterDrainLogCounter <= 3 || jitterDrainLogCounter%1000 == 0 { log.Debug("jitter: draining seq=%d for %s (buf=%d emptyBufs=%d)", diff --git a/gumble/gumbleopenal/stream_regression_test.go b/gumble/gumbleopenal/stream_regression_test.go index f22be87..e78f076 100644 --- a/gumble/gumbleopenal/stream_regression_test.go +++ b/gumble/gumbleopenal/stream_regression_test.go @@ -62,6 +62,25 @@ func TestJitterPlaybackDelayAppliesOnlyAtStartup(t *testing.T) { } } +func TestJitterResyncsAfterSenderRestartsSequence(t *testing.T) { + // Mumble restarts frame numbering at zero when the sender switches audio + // devices mid-burst, and sends no terminator to announce it. + if !jitterShouldResync(jitterLateResync, 52724) { + t.Fatal("jitter did not resync after the sender restarted its frame numbering") + } + if jitterShouldResync(jitterLateResync-1, 52724) { + t.Fatal("jitter resynced before the late run was conclusive") + } + // A clump of reordered packets is bounded and recovers on its own; it must + // not drag the expected sequence backwards. + if jitterShouldResync(jitterLateResync, jitterResyncJump-1) { + t.Fatal("jitter resynced on a backwards jump small enough to be reordering") + } + if jitterShouldResync(1, 52724) { + t.Fatal("jitter resynced on a single late packet") + } +} + func TestAudioPacketDurationUsesStereoFrameCount(t *testing.T) { packet := &gumble.AudioPacket{AudioBuffer: make(gumble.AudioBuffer, 2*gumble.AudioDefaultFrameSize)} if got := audioPacketDuration(packet); got != 10*time.Millisecond {