From aa1d233c81fbd75fc7ae63e1f5df5554463ef8e0 Mon Sep 17 00:00:00 2001 From: Brandon McGinty Date: Thu, 20 Aug 2026 14:23:46 -0400 Subject: [PATCH] conceal lost audio and size the Opus encoder to the frame Fill sequence gaps with Opus packet loss concealment. A dropped packet previously left a hole in the stream. The decoder is now asked for a concealed frame for each missing sequence number, and is only reset when a decode actually fails or a talk burst ends. Mark the end of a talk burst with a terminator packet. Listeners need to drop their per-speaker ordering state before the sender starts numbering a new burst. Set the encoder bitrate from the packet budget and frame size. SetBitrateToMax ignored the server's bandwidth limit, so frames were encoded larger than the allowed data bytes and truncated. The result is clamped to Opus's 8 kbps floor, and the auto-bitrate listener now keeps at least ten bytes per frame so a low-bandwidth server cannot compute a budget too small to encode anything. Allocate the decode buffer for the sample count it is given. The decoder doubled the requested frame size for stereo and then returned a slice scaled by the same factor again. Hold the client read lock across encode and reset. File playback can replace or reset the stereo encoder while a voice frame is being encoded with it. Co-Authored-By: Claude Opus 5 --- gumble/gumble/audio.go | 37 +++++++++++++-------- gumble/gumble/handlers.go | 63 ++++++++++++++++++++++++++++++++++-- gumble/gumbleutil/bitrate.go | 8 ++++- gumble/opus/opus.go | 39 ++++++++++++++++------ gumble/opus/opus_test.go | 16 +++++++++ 5 files changed, 136 insertions(+), 27 deletions(-) create mode 100644 gumble/opus/opus_test.go diff --git a/gumble/gumble/audio.go b/gumble/gumble/audio.go index ceb10b2..0fdb224 100644 --- a/gumble/gumble/audio.go +++ b/gumble/gumble/audio.go @@ -53,30 +53,30 @@ type AudioStreamEvent struct { type AudioBuffer []int16 func (a AudioBuffer) writeAudio(client *Client, seq int64, final bool) error { - // Choose encoder based on whether buffer size indicates stereo or mono + // Encoding shares mutable codec state with server-configuration and file + // playback changes. Keep the client read lock through Encode and Reset so a + // stereo encoder cannot be replaced or reset while it is in use. + client.volatile.RLock() encoder := client.AudioEncoder - frameSize := client.Config.AudioFrameSize() - if len(a) == frameSize*AudioChannels && client.AudioEncoderStereo != nil { - encoder = client.AudioEncoderStereo - } else if client.IsStereoEncoderEnabled() && client.AudioEncoderStereo != nil { + if client.useStereoEncoder && client.AudioEncoderStereo != nil { encoder = client.AudioEncoderStereo } if encoder == nil { + client.volatile.RUnlock() return nil } - dataBytes := client.Config.AudioDataBytes - raw, err := encoder.Encode(a, len(a), dataBytes) + raw, err := encoder.Encode(a, len(a), client.Config.AudioDataBytes) if final { - defer encoder.Reset() + encoder.Reset() } - if err != nil { - return err - } - var targetID byte if target := client.VoiceTarget; target != nil { targetID = byte(target.ID) } + client.volatile.RUnlock() + if err != nil { + return err + } return client.Conn.WriteAudio(byte(4), targetID, seq, final, raw, nil, nil, nil) } @@ -86,8 +86,17 @@ type AudioPacket struct { Sender *User Target *VoiceTarget + // Sequence is the UDP audio frame timestamp, used by the jitter buffer to + // reorder packets. + Sequence int64 + AudioBuffer - HasPosition bool - X, Y, Z float32 + // Terminator marks the final packet in a talk burst. Audio listeners use + // it to discard ordering state before the sender starts a new burst. + Terminator bool + + HasPosition bool + X, Y, Z float32 + VolumeAdjustment float32 } diff --git a/gumble/gumble/handlers.go b/gumble/gumble/handlers.go index 81c6cc2..ceb2f95 100644 --- a/gumble/gumble/handlers.go +++ b/gumble/gumble/handlers.go @@ -11,6 +11,7 @@ import ( "git.stormux.org/storm/barnard/gumble/gumble/MumbleProto" "git.stormux.org/storm/barnard/gumble/gumble/varint" + "git.stormux.org/storm/barnard/log" "google.golang.org/protobuf/proto" ) @@ -113,13 +114,35 @@ func (c *Client) handleUDPTunnel(buffer []byte) error { } // Sequence - // TODO: use in jitter buffer - _, n = varint.Decode(buffer) + seq, n := varint.Decode(buffer) if n <= 0 { return errInvalidProtobuf } buffer = buffer[n:] + // Detect sequence gaps (packet loss). Use Opus Packet Loss + // Concealment to fill gaps rather than resetting the decoder, + // which would cause audible glitches. + if user.audioSequenceValid { + gap := seq - user.audioSequence + if gap > 1 && gap < 100 { + log.Info("audio seq gap for %s: %d -> %d (loss=%d), generating PLC", + user.Name, user.audioSequence, seq, gap-1) + for i := int64(1); i < gap; i++ { + c.dispatchPLC(user, audioTarget, decoder) + } + } else if gap < 0 && gap > -100 { + log.Info("audio seq reorder for %s: %d -> %d, resetting decoder", + user.Name, user.audioSequence, seq) + decoder.Reset() + } else if gap == 0 { + log.Info("audio seq duplicate for %s: seq=%d", user.Name, seq) + return nil + } + } + user.audioSequence = seq + user.audioSequenceValid = true + // Length length, n := varint.Decode(buffer) if n <= 0 { @@ -128,12 +151,17 @@ func (c *Client) handleUDPTunnel(buffer []byte) error { buffer = buffer[n:] // Opus audio packets set the 13th bit in the size field as the terminator. audioLength := int(length) &^ 0x2000 + isFinal := (length & 0x2000) != 0 if audioLength > len(buffer) { return errInvalidProtobuf } pcm, err := decoder.Decode(buffer[:audioLength], AudioMaximumFrameSize) if err != nil { + // Decode failure indicates corrupted decoder state; reset and drop. + log.Warn("handleUDPTunnel: Opus decode FAILED for %s seq=%d: %v", + user.Name, seq, err) + decoder.Reset() return err } @@ -143,6 +171,7 @@ func (c *Client) handleUDPTunnel(buffer []byte) error { Target: &VoiceTarget{ ID: uint32(audioTarget), }, + Sequence: seq, AudioBuffer: AudioBuffer(pcm), } @@ -157,9 +186,39 @@ func (c *Client) handleUDPTunnel(buffer []byte) error { } c.dispatchAudio(user, &event) + if isFinal { + decoder.Reset() + user.audioSequenceValid = false + c.dispatchAudio(user, &AudioPacket{Client: c, Sender: user, Terminator: true}) + } return nil } +// dispatchPLC generates a Packet Loss Concealment frame from the decoder +// and dispatches it to all audio listeners for the given user. +// seq is the expected sequence number for the concealed frame. +func (c *Client) dispatchPLC(user *User, audioTarget byte, decoder AudioDecoder) { + // Feed empty data to the decoder to trigger Opus PLC, which + // produces a concealed frame bridging the gap. + pcm, err := decoder.Decode(nil, AudioMaximumFrameSize) + if err != nil { + // If PLC fails, reset the decoder so the next real packet + // starts from a clean state. + decoder.Reset() + return + } + seq := user.audioSequence + 1 + user.audioSequence = seq + event := AudioPacket{ + Client: c, + Sender: user, + Target: &VoiceTarget{ID: uint32(audioTarget)}, + Sequence: seq, + AudioBuffer: AudioBuffer(pcm), + } + c.dispatchAudio(user, &event) +} + // dispatchAudio sends an audio packet to all registered audio listeners. func (c *Client) dispatchAudio(user *User, packet *AudioPacket) { listeners := &c.Config.AudioListeners diff --git a/gumble/gumbleutil/bitrate.go b/gumble/gumbleutil/bitrate.go index 7244562..0c465b3 100644 --- a/gumble/gumbleutil/bitrate.go +++ b/gumble/gumbleutil/bitrate.go @@ -9,10 +9,16 @@ import ( var autoBitrate = &Listener{ Connect: func(e *gumble.ConnectEvent) { if e.MaximumBitrate != nil { - const safety = 5 + const ( + safety = 5 + minBytes = 10 // minimum bytes per frame for usable Opus (8 kbps) + ) interval := e.Client.Config.AudioInterval dataBytes := (*e.MaximumBitrate / (8 * (int(time.Second/interval) + safety))) - 32 - 10 + if dataBytes < minBytes { + dataBytes = minBytes + } e.Client.Config.AudioDataBytes = dataBytes } }, diff --git a/gumble/opus/opus.go b/gumble/opus/opus.go index f0976e4..0eab4df 100644 --- a/gumble/opus/opus.go +++ b/gumble/opus/opus.go @@ -29,9 +29,9 @@ func (*generator) ID() int { func (*generator) NewEncoder() gumble.AudioEncoder { // Force mono for voice transmission e, _ := opus.NewEncoder(gumble.AudioSampleRate, VoiceChannels, opus.AppVoIP) - _ = e.SetBitrateToMax() return &Encoder{ - e, + Encoder: e, + channels: VoiceChannels, } } @@ -39,9 +39,9 @@ func (*generator) NewEncoder() gumble.AudioEncoder { func NewStereoEncoder() gumble.AudioEncoder { // Create stereo encoder for file playback e, _ := opus.NewEncoder(gumble.AudioSampleRate, gumble.AudioChannels, opus.AppAudio) - _ = e.SetBitrateToMax() return &Encoder{ - e, + Encoder: e, + channels: gumble.AudioChannels, } } @@ -58,13 +58,20 @@ func (*generator) NewDecoder() gumble.AudioDecoder { // encoder type Encoder struct { *opus.Encoder + channels int } func (*Encoder) ID() int { return ID } -func (e *Encoder) Encode(pcm []int16, _, maxDataBytes int) ([]byte, error) { +func (e *Encoder) Encode(pcm []int16, frameSamples, maxDataBytes int) ([]byte, error) { + bitrate := encoderBitrate(maxDataBytes, frameSamples, e.channels) + if bitrate < 8000 { + bitrate = 8000 // Opus minimum viable bitrate for voice + } + _ = e.Encoder.SetBitrate(bitrate) + buf := make([]byte, maxDataBytes) n, err := e.Encoder.Encode(pcm, buf) if err != nil { @@ -73,6 +80,14 @@ func (e *Encoder) Encode(pcm []int16, _, maxDataBytes int) ([]byte, error) { return buf[:n], nil } +// encoderBitrate converts a per-frame packet budget to bits per second. +func encoderBitrate(maxDataBytes, frameSamples, channels int) int { + if frameSamples <= 0 || channels <= 0 { + return 8000 + } + return maxDataBytes * 8 * gumble.AudioSampleRate * channels / frameSamples +} + func (e *Encoder) Reset() { _ = e.Encoder.Reset() } @@ -89,17 +104,21 @@ func (*Decoder) ID() int { } func (d *Decoder) Decode(data []byte, frameSize int) ([]int16, error) { - // Allocate buffer for stereo - frameSize is per channel - pcm := make([]int16, frameSize*gumble.AudioChannels) + // frameSize is the maximum number of PCM samples (all channels + // combined). The underlying Opus decoder output is interleaved + // stereo, so the buffer holds left+right pairs. + pcm := make([]int16, frameSize) - // Decode the data + // Decode the data. If data is nil/empty, the decoder performs + // Packet Loss Concealment and produces a concealed frame. n, err := d.Decoder.Decode(data, pcm) if err != nil { return []int16{}, err } - // Return the exact number of samples decoded - return pcm[:n*gumble.AudioChannels], nil + // n is the number of samples per channel; stereo interleaved + // output means total samples = n * channels. + return pcm[:n*d.channels], nil } func (d *Decoder) Reset() { diff --git a/gumble/opus/opus_test.go b/gumble/opus/opus_test.go new file mode 100644 index 0000000..0d3531f --- /dev/null +++ b/gumble/opus/opus_test.go @@ -0,0 +1,16 @@ +package opus + +import "testing" + +// Regression: the encoder always assumed ten millisecond frames, causing the +// bitrate for 20/40/60 ms packets to be 2/4/6 times their actual budget. +func TestEncoderBitrateUsesFrameDuration(t *testing.T) { + const budget = 100 + for _, frameSamples := range []int{480, 960, 1920, 2880} { + got := encoderBitrate(budget, frameSamples, 1) + want := budget * 8 * 48000 / frameSamples + if got != want { + t.Fatalf("%d samples: got %d, want %d", frameSamples, got, want) + } + } +}