Skip to content

Fix choppy voice at low FPS caused by per-frame decode cap - #5186

Open
QueryOfficial wants to merge 2 commits into
multitheftauto:masterfrom
QueryOfficial:fix/voice-decode-throttle
Open

Fix choppy voice at low FPS caused by per-frame decode cap#5186
QueryOfficial wants to merge 2 commits into
multitheftauto:masterfrom
QueryOfficial:fix/voice-decode-throttle

Conversation

@QueryOfficial

Copy link
Copy Markdown
Contributor

Summary

Replace the fixed per-game-frame voice decode cap introduced in #5175 with a real-time token bucket, and expose the burst ceiling as a new server setting max_voice_decode_burst.

CClientPlayerVoice::DecodeAndBuffer currently drops every packet past the 6th one received in a single game frame from the same speaker. This PR replaces that counter with credit that accrues at the voice frame rate (one 20 ms frame per 20 ms) and is spent per decode, capped at a configurable ceiling. Steady speech can never outrun real time, so it is never throttled regardless of the listener's frame rate; a flood still fills the ceiling and is then held to real time, so the CPU bound that #5175 introduced is preserved.

Changes:

  • CClientPlayerVoice: m_voiceFramesThisPulseTakeDecodeCredit() (real-time credit, burst ceiling). DoPulse no longer resets a counter.
  • CVoiceRecorder: stores the ceiling received from the server; new VOICE_FRAME_DURATION_MS and VOICE_DECODE_BURST_DEFAULT constants.
  • CMainConfig: new max_voice_decode_burst (6–200, default 25 = 500 ms of audio), in the SIntSetting table so it also works with setServerConfigSetting.
  • CPlayerJoinCompletePacket / CPacketHandler: the ceiling is sent in the join packet, gated by a new eBitStreamVersion::VoiceDecodeBurstLimit. Either side lacking the version falls back to the default, so old server + new client and new server + old client both keep working.
  • mtaserver.conf: documented the new setting next to the other max_voice_* entries.

Motivation

Since 9e28567 (#5175) landed, remote voice has become choppy for many players, most noticeably in crowded areas. #5175 was also backported to release/1.6.0 in 0ce13e2, so the bug is live in the 1.6 nightlies.

Speex emits one 20 ms frame per packet at every sample rate, so a speaker sends ~50 packets per second. The cap of 6 decodes per game frame is only safe while the listener renders at ≥ 50 FPS with perfectly even packet arrival. In practice:

  • A listener at 20–30 FPS (typical in a crowded RP scene) accumulates 2–3 packets per frame on average.
  • RakNet frequently delivers several voice packets in one batch, so a single frame can see 8–10 packets.

Everything past the 6th is silently discarded. Each dropped packet leaves a 20 ms hole; the BASS push stream has no jitter buffer, stalls on the hole, and fires onClientPlayerVoiceStop mid-sentence. Speech that used to arrive as one continuous stream now arrives as many 250–350 ms fragments.

Observed on a live server with a client-side counter on onClientPlayerVoiceStart/Stop per speaker:

Speaker A: 42 segments, 40 stalls, ~384 ms per segment
Speaker B: 69 segments, 66 stalls, ~311 ms per segment
Speaker C: 140 segments, 136 stalls, ~298 ms per segment
Local player (own voice, no network): 5 segments, 2 stalls, ~4741 ms per segment

The local player's own voice goes through the same DecodeAndBuffer path but arrives one frame at a time with no network batching, which is why it is the only stream that stays intact — it never reaches the cap. Everyone else does.

The DoS concern behind #5175 is legitimate; the problem is that a per-frame counter conflates "flood" with "listener is slower than the sender". A time-based budget separates the two: it bounds decode work per unit of real time rather than per rendered frame.

max_voice_decode_burst lets operators trade the two off explicitly. 25 (500 ms) is comfortably above any legitimate burst — anything longer than that has already stalled the stream, so decoding it late buys nothing — while still capping a flood at 25 decodes per frame per speaker (~1 ms of Speex work). The minimum of 6 reproduces current behaviour.

Test plan

Reproduce the bug first, on current master or a 1.6 nightly, so the fix has a baseline:

  1. Bind a client-side counter to onClientPlayerVoiceStart / onClientPlayerVoiceStop per speaker (segment count, and average time between start and stop).
  2. On the listener, fpslimit 25. Have 3–5 other players talk at once for ~30 seconds.
  3. Expected on unfixed builds: dozens of segments per speaker, ~250–400 ms each, stop events firing while the speaker is clearly still holding the key.

Then with this change:

  • New server + new client, fpslimit 25, 3–5 concurrent speakers. Segment count should be one per actual transmission; onClientPlayerVoiceStop should fire only on key release.
  • New server + new client, FPS uncapped. No regression against master — voice sounds identical to before Further hardening against voice-based DoS/lag effect on remote players #5175.
  • Old server (without this PR) + new client. Client should fall back to the default ceiling of 25 and voice should be continuous.
  • New server + old client (pre-Further hardening against voice-based DoS/lag effect on remote players #5175 and post-Further hardening against voice-based DoS/lag effect on remote players #5175). Join must succeed; the old client behaves exactly as it does today.
  • setServerConfigSetting("max_voice_decode_burst", 6), reconnect. Choppiness at low FPS should return, confirming the setting is live and 6 reproduces current behaviour. Set back to 25, reconnect: resolved. Note the value is applied at join, so a reconnect is required after changing it.
  • Flood check. A client sending voice well above real time (or a relay flood test) should still be capped: decodes per frame for that speaker must never exceed the configured ceiling, and CPU should stay bounded as it is on master.

For future regression checks: keep the per-speaker segment counter, cap FPS at 25, and confirm segment count stays at one per transmission.

Checklist

  • Your code should follow the coding guidelines.
  • Smaller pull requests are easier to review. If your pull request is beefy, your pull request should be reviewable commit-by-commit.

9e28567 (multitheftauto#5175) capped remote voice decoding at 6 frames per game frame
to bound CPU under relay floods. Speex emits one 20 ms frame per packet
at every sample rate, so a speaker produces 50 packets per second; any
listener rendering below 50 FPS, or receiving a coalesced burst, exceeds
the cap and the surplus packets are silently dropped. Each dropped
packet leaves a 20 ms hole, the BASS push stream stalls on it, and
onClientPlayerVoiceStop fires mid-sentence. In practice voice became
choppy for everyone in crowded areas, where FPS is lowest.

Credit now accrues in real time at the voice frame rate and is spent per
decode, capped at a burst ceiling. Legitimate speech never outruns real
time so it is never throttled, regardless of the listener's frame rate;
a flood still fills the ceiling and is then held to real time, so the
CPU bound remains.

The ceiling is exposed as max_voice_decode_burst (6-200, default 25 =
500 ms of audio) and sent to clients in the join packet behind
eBitStreamVersion::VoiceDecodeBurstLimit. Either side lacking the
version falls back to the default.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant