Voice: client-side capture, relay and spatial mixing (M2) - #245
Merged
Conversation
Completes the voice feature: nothing had produced or consumed a frame until now. Clients encode Opus and send to the server, which relays without decoding; receiving clients decode one stream per speaker and mix with distance attenuation and stereo panning. Vendors miniaudio 0.11.25 (pinned by commit, device I/O only) and adds CaptureDevice/PlaybackDevice, IVoiceSink, and VoiceClient. The relay session opens on connect, since the encoder channel is keyed on our own GUID and the relay target is the server -- neither exists before then. Registered via CoreModules alongside VoiceServer. Voice is unconditional; a client with no microphone degrades to listen-only rather than opting out. Speaker positions come from the replicated entity set in the client Instance, mirroring how the server's voice router gets them, so proximity voice is positioned correctly for any mod out of the box. A project supplies only the listener transform, which is the one part that depends on its camera. The Instance also suppresses transmission whenever its own chat box has the caret or a web view holds focus, so no mod can accidentally broadcast a player typing. Threading: the device thread touches only wait-free rings, per-slot atomic ids and a triple-buffered world snapshot. It never allocates, locks, or calls MafiaNet. Triple buffering rather than double, so the writer's next target is never a buffer the reader picked up one callback ago. Playback is jitter buffered. RakVoice flushes on a 50ms throttle while the device consumes every 20ms, so frames arrive in bursts and playing the first one that lands leaves the buffer riding empty; every hiccup then splices silence into the middle of a word, which sounds like distortion rather than a gap. Speakers now prime to 60ms before being heard and re-prime after an underrun, and the buffer is capped so late-packet bursts and capture/playback clock drift cannot push voice steadily further behind. Bitrate is 40kbps: Opus' reference for a mono stream is 64kbps and 24kbps was audibly band-limited. Bounds are split across two layers. kMaxAudibleTalkers caps the mix fan-in via a fixed slot array, evicting the most distant talker for a nearer one with hysteresis so speakers at similar range cannot trade the last slot every tick. Decode cannot be bounded from here at all -- RakVoice decodes in OnReceive before application code sees a frame -- so kMaxDecodedTalkers goes to RakVoice::SetMaxDecodedSpeakers, added in MafiaNet for this and picked up by the pin bump. It is deliberately larger than the mix cap: were they equal, the codec's recency-based choice would decide who is audible and the distance-based mixer would have nothing left to choose between. Moves final-stage limiting into mixer.cpp as LimitStereoBuffer, the counterpart to MixFrameInto's deliberate lack of clamping, and covers it with five tests (187 total, was 182). No SetNoiseFilter: RNNoise needs 480-sample frames and voice runs at 960. No SetLoopbackMode: it is a no-op in relay mode, since OnRelayVoiceData drops frames whose origin is our own GUID.
WalkthroughAdds a RakVoice client voice subsystem with miniaudio capture/playback, spatialized speaker mixing, framework lifecycle integration, updated voice configuration, and stereo limiter tests. ChangesClient voice subsystem
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmake/MafiaNetPin.cmake`:
- Line 27: Update the MafiaNet FetchContent configuration in
vendors/CMakeLists.txt to disable GIT_SHALLOW when using the MAFIANET_PIN commit
hash, while preserving the existing pinned revision and other fetch settings.
In `@code/framework/src/integrations/client/instance.cpp`:
- Around line 549-557: Update Instance::UpdateNetworking() so SetSpeakerPosition
uses the same MafiaNet::ToPeerGuid-derived key as PumpSpeakers(), rather than
casting entity->ownerGUID directly. Preserve the existing unassigned-owner guard
and ensure speaker admission and placement lookups share one identifier
representation.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 60420e62-99e2-40e8-af93-b8d2c8d86f10
📒 Files selected for processing (19)
cmake/MafiaNetPin.cmakecode/framework/CMakeLists.txtcode/framework/src/core_modules.hcode/framework/src/integrations/client/instance.cppcode/framework/src/integrations/client/instance.hcode/framework/src/voice/client/audio_device.cppcode/framework/src/voice/client/audio_device.hcode/framework/src/voice/client/i_voice_sink.hcode/framework/src/voice/client/mixer.cppcode/framework/src/voice/client/mixer.hcode/framework/src/voice/client/voice_client.cppcode/framework/src/voice/client/voice_client.hcode/framework/src/voice/voice_config.hcode/tests/modules/voice_mixer_ut.hvendors/CMakeLists.txtvendors/miniaudio/CMakeLists.txtvendors/miniaudio/LICENSEvendors/miniaudio/miniaudio.cvendors/miniaudio/miniaudio.h
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Completes proximity voice chat. The server half landed in #243, but nothing had ever produced or consumed a frame — this is the half that makes the feature exist.
Clients encode Opus and send to the server, which relays the payload without decoding it. Receiving clients decode one stream per speaker and mix with distance attenuation and constant-power stereo panning.
What's here
vendors/miniaudio/— miniaudio 0.11.25, pinned by commit, trimmed to device I/O (no decoders, resource manager, node graph or engine).voice/client/audio_device.{h,cpp}—CaptureDevice(s16/mono/48k →SpscRing) andPlaybackDevice(f32/stereo/48k, pulls a render callback).voice/client/i_voice_sink.h—Submit/ReleaseSpeaker, so a project can render voice through its own audio engine without touching transport, codec or scripting.voice/client/voice_client.{h,cpp}— the relay session, push-to-talk, speaker admission, and the built-in mixing sink.mixer.cpp—LimitStereoBuffer, the counterpart toMixFrameInto's deliberate lack of clamping, with five new tests.The relay session opens on connect, not at init: the encoder channel is keyed on our own GUID and the relay target is the server, and neither exists earlier.
Generic by default
Speaker positions come from the replicated entity set in the client
Instance, mirroring how the server's voice router already gets them. Proximity voice is therefore positioned correctly for any mod out of the box — a project supplies only the listener transform, which is the one part that depends on its camera.The
Instancealso suppresses transmission whenever its own chat box has the caret or a web view holds focus, so no mod can accidentally broadcast a player typing.Voice is unconditional — no
InstanceOptionsopt-out. A client with no microphone degrades to listen-only rather than disabling the feature.Threading
The device thread touches only wait-free rings, per-slot atomic ids and a triple-buffered world snapshot. It never allocates, locks, or calls MafiaNet. Triple buffering rather than double, so the writer's next target is never a buffer the reader picked up one callback ago.
Two-layer bound
kMaxAudibleTalkers(6) caps the mix fan-in via a fixed slot array, evicting the most distant talker for a nearer one with hysteresis so speakers at similar range can't trade the last slot every tick.Decode cannot be bounded from the Framework at all — RakVoice decodes in
OnReceive, insideRakPeer::Receive, before application code sees a frame. SokMaxDecodedTalkers(12) goes toRakVoice::SetMaxDecodedSpeakers, added in MafiaNet for this. It is deliberately larger than the mix cap: were they equal, the codec's recency-based choice would decide who is audible and the distance-based mixer would have nothing left to choose between.This bumps
cmake/MafiaNetPin.cmake, whichbump_version.shclassifies as MAJOR.That is conservative here: the relay wire format and the message-id enum are untouched, the new MafiaNet API is additive and off by default, and old/new peers are genuinely compatible. The pin is a proxy for "the wire may have moved" and the tooling can't tell the difference. Consider overriding the classification rather than spending a major on an additive API.
Depends on MafiaNet
caf9469a(already onmaster).Testing
Verified on a real two-client session (two instances, one server): capture → Opus encode → relay → decode → per-speaker mix → playback, with clean session open/close on both peers and zero warnings or errors. Devices negotiate 48 kHz natively, so no resampling.
Playback was initially mushy. Root cause was a missing jitter buffer: RakVoice flushes on a 50 ms throttle while the device consumes every 20 ms, so frames arrive in bursts and playing the first one that lands left the buffer riding empty — every hiccup spliced silence mid-word, which sounds like distortion rather than a gap. Speakers now prime to 60 ms and re-prime after an underrun, and the buffer is capped so late-packet bursts and capture/playback clock drift can't push voice steadily further behind. Bitrate also went 24 → 40 kbps (Opus' reference for a mono stream is 64). Confirmed clean by ear afterwards.
FrameworkTests: 187 passing, 0 failed (was 182; +5 for the limiter).FrameworkClient64-bit andM2OClient32-bit both build clean.check_style.shshows zero new violations againstdevelop.Not yet exercised, all needing more than two co-located players: distance falloff and the 25-unit cutoff, left/right panning, two simultaneous talkers, slot eviction (7+ speakers) and the new decode cap (13+).
Follow-up
Opus inband FEC is off (libopus default) and RakVoice exposes no ctl for it. Irrelevant on a LAN, but it will matter over the internet on
UnreliableSequenced. That's another MafiaNet change plus a pin bump, so it's deliberately not bundled here.