From 5b0879ddaa6f36745d4737737e4e1c0942dad889 Mon Sep 17 00:00:00 2001 From: "Brandon McGinty (chatgpt)" Date: Mon, 10 Aug 2026 15:18:04 -0400 Subject: [PATCH] Fix UDP audio frame timestamp handling --- gumble/gumble/audio.go | 4 +- gumble/gumble/client.go | 6 ++- gumble/gumble/integration_audio_test.go | 5 ++- gumble/gumble/udp15.go | 54 +++++++++++++++++++------ gumble/gumble/udp15_test.go | 25 ++++++++++++ gumble/gumble/user.go | 13 +++--- 6 files changed, 84 insertions(+), 23 deletions(-) diff --git a/gumble/gumble/audio.go b/gumble/gumble/audio.go index 358225d..74e868d 100644 --- a/gumble/gumble/audio.go +++ b/gumble/gumble/audio.go @@ -86,8 +86,8 @@ type AudioPacket struct { Sender *User Target *VoiceTarget - // Sequence is the UDP audio packet sequence number, used by the - // jitter buffer to reorder packets. + // Sequence is the UDP audio frame timestamp, used by the jitter buffer to + // reorder packets. Sequence int64 AudioBuffer diff --git a/gumble/gumble/client.go b/gumble/gumble/client.go index b3064ed..98a6e92 100644 --- a/gumble/gumble/client.go +++ b/gumble/gumble/client.go @@ -232,11 +232,15 @@ func (c *Client) AudioOutgoing() chan<- AudioBuffer { ch := make(chan AudioBuffer) go func() { var seq int64 + frameStep := int64(c.Config.AudioFrameSize() / AudioDefaultFrameSize) + if frameStep < 1 { + frameStep = 1 + } previous := <-ch for p := range ch { previous.writeAudio(c, seq, false) previous = p - seq = (seq + 1) % math.MaxInt32 + seq = (seq + frameStep) % math.MaxInt32 } if previous != nil { previous.writeAudio(c, seq, true) diff --git a/gumble/gumble/integration_audio_test.go b/gumble/gumble/integration_audio_test.go index 676ade5..f958114 100644 --- a/gumble/gumble/integration_audio_test.go +++ b/gumble/gumble/integration_audio_test.go @@ -106,10 +106,11 @@ func testLocalMumbleAudioRoundTrip(t *testing.T, disableUDP bool, interval time. // Discard the first few decoder warm-up frames before measuring pitch. var packet *gumble.AudioPacket var previousSequence int64 + frameStep := int64(sendConfig.AudioFrameSize() / gumble.AudioDefaultFrameSize) for i := 0; i < 3; i++ { select { case packet = <-listener.packets: - if i > 0 && packet.Sequence != previousSequence+1 { + if i > 0 && packet.Sequence != previousSequence+frameStep { t.Fatalf("choppy relay: sequence gap %d -> %d", previousSequence, packet.Sequence) } previousSequence = packet.Sequence @@ -119,7 +120,7 @@ func testLocalMumbleAudioRoundTrip(t *testing.T, disableUDP bool, interval time. } select { case packet = <-listener.packets: - if packet.Sequence != previousSequence+1 { + if packet.Sequence != previousSequence+frameStep { t.Fatalf("choppy relay: sequence gap %d -> %d", previousSequence, packet.Sequence) } frequency, purity, peak := audioQuality(packet.AudioBuffer) diff --git a/gumble/gumble/udp15.go b/gumble/gumble/udp15.go index aa484b1..7433750 100644 --- a/gumble/gumble/udp15.go +++ b/gumble/gumble/udp15.go @@ -733,6 +733,8 @@ func (c *Client) dispatchOpus15(pktNum uint64, session uint32, frameNum int64, o if terminator && len(opusData) == 0 { decoder.Reset() + user.audioSequenceValid = false + user.audioFrameStep = 0 log.Info("UDP15 #%d: terminator for %s, decoder reset", pktNum, user.Name) return } @@ -791,14 +793,23 @@ func (c *Client) handleLegacyUDPVoice(pktNum uint64, data []byte) { // decodeAndDispatch decodes an Opus frame and dispatches PCM to audio listeners. func (c *Client) decodeAndDispatch(pktNum uint64, user *User, decoder AudioDecoder, frameNum int64, opusData []byte, context uint32, position *[3]float32, volumeAdjustment float32) { - // Detect sequence gaps. + // Frame numbers are timestamps in 10 ms units, not packet counters. For + // example, a standard 20 ms Opus packet advances its frame number by two. + // Only generate PLC for complete missing packets; treating every timestamp + // unit as a packet doubles playout and eventually exhausts OpenAL buffers. if user.audioSequenceValid { gap := frameNum - user.audioSequence - if gap > 1 && gap < 100 { - log.Info("UDP15 #%d: seq gap for %s: %d -> %d (loss=%d), generating PLC", - pktNum, user.Name, user.audioSequence, frameNum, gap-1) - for i := int64(1); i < gap; i++ { - c.dispatchPLC15(user, decoder) + frameStep := user.audioFrameStep + if frameStep < 1 { + frameStep = 1 + } + if gap > frameStep && gap < 100 { + if missing := missingAudioPackets(gap, frameStep); missing > 0 { + log.Info("UDP15 #%d: audio gap for %s: %d -> %d (loss=%d), generating PLC", + pktNum, user.Name, user.audioSequence, frameNum, missing) + for i := int64(1); i <= missing; i++ { + c.dispatchPLC15(user, decoder, user.audioSequence+i*frameStep) + } } } else if gap < 0 && gap > -100 { log.Info("UDP15 #%d: seq reorder for %s: %d -> %d, resetting decoder", @@ -809,8 +820,6 @@ func (c *Client) decodeAndDispatch(pktNum uint64, user *User, decoder AudioDecod return } } - user.audioSequence = frameNum - user.audioSequenceValid = true pcm, err := decoder.Decode(opusData, AudioMaximumFrameSize) if err != nil { @@ -820,6 +829,9 @@ func (c *Client) decodeAndDispatch(pktNum uint64, user *User, decoder AudioDecod } log.Debug("UDP15 #%d: Opus OK for %s, pcm_samples=%d", pktNum, user.Name, len(pcm)) + user.audioSequence = frameNum + user.audioSequenceValid = true + user.audioFrameStep = audioFrameStep(len(pcm)) event := AudioPacket{ Client: c, @@ -836,20 +848,38 @@ func (c *Client) decodeAndDispatch(pktNum uint64, user *User, decoder AudioDecod c.dispatchAudio(user, &event) } +// missingAudioPackets returns the number of whole packets absent from a +// timestamp gap. A non-integral gap cannot reliably identify a missing packet. +func missingAudioPackets(gap, frameStep int64) int64 { + if frameStep < 1 || gap <= frameStep || gap%frameStep != 0 { + return 0 + } + return gap/frameStep - 1 +} + +// audioFrameStep converts interleaved stereo PCM length to Mumble's 10 ms +// frame-number units. +func audioFrameStep(samples int) int64 { + frames := samples / AudioChannels + step := int64(frames / AudioDefaultFrameSize) + if step < 1 { + return 1 + } + return step +} + // dispatchPLC15 generates a Packet Loss Concealment frame for 1.5 UDP. -func (c *Client) dispatchPLC15(user *User, decoder AudioDecoder) { +func (c *Client) dispatchPLC15(user *User, decoder AudioDecoder, sequence int64) { pcm, err := decoder.Decode(nil, AudioMaximumFrameSize) if err != nil { decoder.Reset() return } - seq := user.audioSequence + 1 - user.audioSequence = seq event := AudioPacket{ Client: c, Sender: user, Target: &VoiceTarget{ID: 0}, - Sequence: seq, + Sequence: sequence, AudioBuffer: AudioBuffer(pcm), } c.dispatchAudio(user, &event) diff --git a/gumble/gumble/udp15_test.go b/gumble/gumble/udp15_test.go index 8b19642..186dc91 100644 --- a/gumble/gumble/udp15_test.go +++ b/gumble/gumble/udp15_test.go @@ -223,6 +223,31 @@ func TestUDPAudioProtobufReferenceVector(t *testing.T) { } } +func TestAudioFrameTimestampGaps(t *testing.T) { + tests := []struct { + name string + gap, step int64 + want int64 + }{ + {"consecutive 10 ms packets", 1, 1, 0}, + {"consecutive 20 ms packets", 2, 2, 0}, + {"one missing 20 ms packet", 4, 2, 1}, + {"two missing 20 ms packets", 6, 2, 2}, + {"non-integral timestamp gap", 3, 2, 0}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := missingAudioPackets(tt.gap, tt.step); got != tt.want { + t.Fatalf("missingAudioPackets(%d, %d) = %d, want %d", tt.gap, tt.step, got, tt.want) + } + }) + } + + if got := audioFrameStep(1920); got != 2 { + t.Fatalf("audioFrameStep(1920) = %d, want 2", got) + } +} + func TestUDPAudioProtobufIncomingFields(t *testing.T) { var packet bytes.Buffer writeVarint := func(v uint64) { diff --git a/gumble/gumble/user.go b/gumble/gumble/user.go index 756f96f..fd8c5f3 100644 --- a/gumble/gumble/user.go +++ b/gumble/gumble/user.go @@ -52,17 +52,18 @@ type User struct { client *Client decoder AudioDecoder - // audioSequence tracks the last UDP audio packet sequence number for - // this user, used to detect packet loss and reset the Opus decoder. + // audioSequence tracks the last UDP audio frame timestamp for this user, + // used to detect packet loss and reset the Opus decoder. audioSequence int64 audioSequenceValid bool + audioFrameStep int64 // audioMu protects audio-related fields accessed from both the // audio processing goroutine (OnAudioStream) and the UI goroutine. - audioMu sync.Mutex - audioSource *openal.Source - boost uint16 - volume float32 + audioMu sync.Mutex + audioSource *openal.Source + boost uint16 + volume float32 locallyMuted bool }