handle dropped packets and network delays
Add a per-user jitter buffer with restart resync. Incoming audio was rendered straight to OpenAL as packets arrived, so normal network jitter produced gaps and the renderer starved between frames. Buffer each user's decoded frames for a configurable playout delay (-jitter-buffer, default 40ms) and let the render thread pull from that buffer instead. Drop late packets, and use packet duration to set the expected frame number. A packet below the expected frame number is dropped rather than stalling delivery, and the expected frame is stepped by the packet's real duration instead of a fixed 10ms. After detecting a large backward jump in frame numbering, along with a sustained run of late packets, resync to the new frame numbering. When a sender switches audio devices, Mumble destroys and recreates their audio stream. This resets their frame number to zero without sending a terminator packet, which causes a potentially endless hang while we wait for a frame number that will not arrive for hours. Own the OpenAL context on one dedicated render thread. Contexts are current to an OS thread, so creating sources and buffers from whichever goroutine happened to be running could operate on no context at all. Every source and buffer is now created, filled, and deleted on that thread, and file playback renders through it too. Report device and capture failures with the device name. Startup errors said only that a device could not be opened, and a capture stall was reported as a microphone failure even though it is normal scheduler timing. Fatal microphone errors now exit instead of leaving a client that cannot transmit. Release queued buffers and deactivate the context on shutdown. Streams were torn down while buffers were still queued on a source, and the context was destroyed while still current. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
5ec82eb1fd
commit
6dcaa7f7a6
+10
-3
@@ -25,6 +25,9 @@ type Config struct {
|
||||
AudioInterval time.Duration
|
||||
// AudioDataBytes is the number of bytes that an audio frame can use.
|
||||
AudioDataBytes int
|
||||
// IncomingAudioBuffer is the amount of per-speaker audio retained before
|
||||
// playback starts, absorbing jitter in incoming UDP packet delivery.
|
||||
IncomingAudioBuffer time.Duration
|
||||
|
||||
// DisableUDP forces all audio to use the TCP tunnel instead of UDP.
|
||||
DisableUDP bool
|
||||
@@ -38,9 +41,10 @@ type Config struct {
|
||||
// NewConfig returns a new Config struct with default values set.
|
||||
func NewConfig() *Config {
|
||||
return &Config{
|
||||
Buffers: 8,
|
||||
AudioInterval: AudioDefaultInterval,
|
||||
AudioDataBytes: AudioDefaultDataBytes,
|
||||
Buffers: 8,
|
||||
AudioInterval: AudioDefaultInterval,
|
||||
AudioDataBytes: AudioDefaultDataBytes,
|
||||
IncomingAudioBuffer: 40 * time.Millisecond,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -54,6 +58,9 @@ func (c *Config) Validate() error {
|
||||
if c.AudioDataBytes <= 0 {
|
||||
return fmt.Errorf("gumble: AudioDataBytes must be positive")
|
||||
}
|
||||
if c.IncomingAudioBuffer < 0 {
|
||||
return fmt.Errorf("gumble: IncomingAudioBuffer must not be negative")
|
||||
}
|
||||
if c.Buffers <= 0 {
|
||||
return fmt.Errorf("gumble: Buffers must be positive")
|
||||
}
|
||||
|
||||
+715
-181
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,100 @@
|
||||
package gumbleopenal
|
||||
|
||||
import (
|
||||
"errors"
|
||||
"strings"
|
||||
"testing"
|
||||
"time"
|
||||
|
||||
"git.stormux.org/storm/barnard/gumble/go-openal/openal"
|
||||
"git.stormux.org/storm/barnard/gumble/gumble"
|
||||
)
|
||||
|
||||
// Regression: audio cleanup could send a final render command after Destroy
|
||||
// had closed renderCh, panicking instead of safely discarding that work.
|
||||
// Regression: StopSource returned before the capture worker ended, allowing
|
||||
// Destroy to close the device while that worker still used it.
|
||||
// Regression: OpenAL returned only a generic input/output error, hiding the
|
||||
// actual configured device that a user must correct.
|
||||
func TestDeviceOpenErrorsIncludeConfiguredDevice(t *testing.T) {
|
||||
input := openInputDeviceError("virtual_mic.monitor", openal.FormatMono16)
|
||||
if !errors.Is(input, ErrInputDevice) || !strings.Contains(input.Error(), "virtual_mic.monitor") {
|
||||
t.Fatalf("input error %q", input)
|
||||
}
|
||||
output := openOutputDeviceError("")
|
||||
if !errors.Is(output, ErrOutputDevice) || !strings.Contains(output.Error(), "default") {
|
||||
t.Fatalf("output error %q", output)
|
||||
}
|
||||
}
|
||||
|
||||
// Regression: later capture start failures also omitted the configured device.
|
||||
func TestStartSourceUnavailableDeviceIncludesName(t *testing.T) {
|
||||
s := &Stream{inputDeviceName: "virtual_mic.monitor"}
|
||||
err := s.StartSource(nil)
|
||||
if !errors.Is(err, ErrMic) || !strings.Contains(err.Error(), "virtual_mic.monitor") {
|
||||
t.Fatalf("start error %q", err)
|
||||
}
|
||||
}
|
||||
|
||||
func TestStopSourceWaitsForWorker(t *testing.T) {
|
||||
stop, done := make(chan bool), make(chan struct{})
|
||||
s := &Stream{sourceStop: stop, sourceDone: done}
|
||||
returned := make(chan struct{})
|
||||
go func() { _ = s.StopSource(); close(returned) }()
|
||||
select {
|
||||
case <-returned:
|
||||
t.Fatal("StopSource returned before worker")
|
||||
default:
|
||||
}
|
||||
close(done)
|
||||
<-returned
|
||||
}
|
||||
|
||||
func TestJitterPlaybackDelayAppliesOnlyAtStartup(t *testing.T) {
|
||||
if jitterPlaybackReady(false, 20*time.Millisecond, 40*time.Millisecond) {
|
||||
t.Fatal("jitter playback started before initial buffer filled")
|
||||
}
|
||||
if !jitterPlaybackReady(false, 40*time.Millisecond, 40*time.Millisecond) {
|
||||
t.Fatal("jitter playback did not start after initial buffer filled")
|
||||
}
|
||||
if !jitterPlaybackReady(true, 0, 40*time.Millisecond) {
|
||||
t.Fatal("jitter playback paused while refilling after startup")
|
||||
}
|
||||
}
|
||||
|
||||
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 {
|
||||
t.Fatalf("audioPacketDuration = %v, want 10ms", got)
|
||||
}
|
||||
}
|
||||
|
||||
func TestRenderRejectsWorkAfterShutdown(t *testing.T) {
|
||||
s := &Stream{renderClosed: true}
|
||||
called := false
|
||||
if s.render(func() { called = true }) {
|
||||
t.Fatal("closed renderer accepted work")
|
||||
}
|
||||
if called {
|
||||
t.Fatal("closed renderer executed work")
|
||||
}
|
||||
}
|
||||
@@ -14,6 +14,7 @@ import (
|
||||
"os/exec"
|
||||
"strings"
|
||||
"syscall"
|
||||
"time"
|
||||
|
||||
barnlog "git.stormux.org/storm/barnard/log"
|
||||
|
||||
@@ -114,6 +115,7 @@ func main() {
|
||||
serverSet := false
|
||||
usernameSet := false
|
||||
buffers := flag.Int("buffers", 16, "number of audio buffers to use")
|
||||
jitterBuffer := flag.Int("jitter-buffer", 40, "incoming per-user audio buffer in ms (0, 20, 40, or 60)")
|
||||
profile := flag.Bool("profile", false, "add http server to serve profiles")
|
||||
noiseSuppressionEnabled := flag.Bool("noise-suppression", false, "enable noise suppression for microphone input")
|
||||
tcpOnly := flag.Bool("tcp", false, "disable UDP, force audio through TCP tunnel")
|
||||
@@ -121,6 +123,10 @@ func main() {
|
||||
logFile := flag.String("logfile", "", "write logs to this file (logging is disabled when omitted)")
|
||||
|
||||
flag.Parse()
|
||||
selectedJitterBuffer, err := jitterBufferDuration(*jitterBuffer)
|
||||
if err != nil {
|
||||
handle_raw_error(err)
|
||||
}
|
||||
|
||||
// Set up logging
|
||||
var level barnlog.Level
|
||||
@@ -214,6 +220,7 @@ func main() {
|
||||
}
|
||||
b.Config.Buffers = *buffers
|
||||
b.Config.DisableUDP = *tcpOnly
|
||||
b.Config.IncomingAudioBuffer = selectedJitterBuffer
|
||||
|
||||
b.Hotkeys = b.UserConfig.GetHotkeys()
|
||||
b.UserConfig.SaveConfig()
|
||||
@@ -253,6 +260,18 @@ func main() {
|
||||
handle_error(&b)
|
||||
}
|
||||
|
||||
// jitterBufferDuration converts the requested incoming playout delay to a
|
||||
// supported duration. Zero starts playback without an initial safety buffer.
|
||||
func jitterBufferDuration(milliseconds int) (time.Duration, error) {
|
||||
interval := time.Duration(milliseconds) * time.Millisecond
|
||||
switch interval {
|
||||
case 0, 20 * time.Millisecond, 40 * time.Millisecond, 60 * time.Millisecond:
|
||||
return interval, nil
|
||||
default:
|
||||
return 0, fmt.Errorf("jitter buffer must be 0, 20, 40, or 60 ms, got %d", milliseconds)
|
||||
}
|
||||
}
|
||||
|
||||
func handle_raw_error(e error) {
|
||||
fmt.Fprintf(os.Stderr, "%s\n", e.Error())
|
||||
os.Exit(1)
|
||||
|
||||
Reference in New Issue
Block a user