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) + } + } +}