fix(audio): stop the echo/feedback loop and keep voice ahead of video - #40
Merged
Conversation
Calls echoed for both parties and sounded choppy. There were four
separate causes, spread across every client.
The feedback loop was the big one. On desktop the host published the
*recording mixer's* output to viewers, and that mix contains every
viewer's own audio — so each viewer got their own voice returned to
them and the loop closed through the host. Viewers with a second peer
in the call also heard that peer twice: once relayed directly, once
inside the host mix. The host publishes its bare microphone now; the
mixer stays what it always should have been, a recording-only artifact.
RTMP broadcast keeps the full mix, which is correct — it is one-way to
an outside audience, so nothing comes back.
The host also heard every viewer twice. Each remote track was played by
an <audio> element in the WebRTC hook *and* routed to the speakers again
through the AudioContext, offset by the graph's buffer.
No client ever asked for echo cancellation — every microphone was a bare
getUserMedia({ audio: true }). They now share one set of constraints, so
desktop, web and mobile cannot drift into the state where one end
cancels and the other does not. Mobile gets a narrower set: on Android
every key is forwarded to createAudioSource as a *mandatory* native
constraint, so it carries only the flags libwebrtc actually recognises.
Finally, screen video was marked high priority in both the encoder queue
and on the wire while audio was left unranked, so a share allowed up to
8 Mbps won the bandwidth allocator and left the voice stream to stutter.
Audio now outranks video, and Opus runs with in-band FEC so a lost
packet is a duller syllable rather than a gap. Priorities are re-applied
once a connection is established, since some stacks report no encodings
on a sender until negotiation completes.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
ThreatCrush Security Scan159 finding(s) HIGH/CRITICAL: 5 | MEDIUM: 48 | LOW: 106
…and 109 more. Full results in the Security tab. Snippets are redacted; ThreatCrush never prints matched credential material. |
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
1 participant
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.
Calls echoed for both parties and sounded choppy. Four separate causes, spread across every client.
The feedback loop (desktop) — the big one
CapturePreview.tsxpublished the recording mixer's output to viewers. That mix contains every viewer's own audio, so each viewer got their own voice returned to them and the loop closed through the host. With two viewers, B also heard A twice — once relayed directly, once inside the host mix.The host now publishes its bare microphone. The mixer stays what it always should have been: a recording-only artifact. RTMP broadcast keeps the full mix, which is correct — it's one-way to an outside audience, so nothing comes back into the call.
The host heard everyone twice
Each remote track was played by an
<audio>element in the WebRTC hook and routed to the speakers again through the AudioContext (playback=true), offset by the graph's buffer. Sounds exactly like an echo.Nobody ever asked for echo cancellation
Every mic capture across desktop/web/mobile was a bare
getUserMedia({ audio: true }). They now share one constraint set from@pairux/shared-types, so the clients can't drift into the state where one end cancels and the other doesn't.Mobile gets a deliberately narrower set: on Android react-native-webrtc forwards every key to
createAudioSourceas a mandatory native constraint, so it carries only the flags libwebrtc actually recognises —sampleRate/channelCountaren't audio-source constraints there and would only widen the blast radius.Video was starving audio (the choppiness)
Screen video was marked
priority: 'high'andnetworkPriority: 'high'at up to 8 Mbps, while audio senders got no encoding params at all — so the bandwidth allocator served 4K screen video first. Audio now outranks video, and Opus runs with in-band FEC + DTX so a lost packet is a duller syllable rather than a gap.Priorities are re-applied once each connection reaches
connected, since some stacks report no encodings on a sender until negotiation completes, which would make theaddTrack-time call a silent no-op.Verification
tsc --noEmitclean on desktop, web, and mobilemain)Not covered — needs a device
Mobile has no audio-session setup at all (no InCallManager equivalent), so iOS/Android may not enter voice-communication mode. The constraint change is the part I could verify; wiring up an audio session needs a native dep and a prebuild, so I left it out rather than ship it blind. See the "remaining" note in the task thread.
🤖 Generated with Claude Code