feat(teleop): hold to talk through the robot's speaker - #701
Open
DavidDobas wants to merge 7 commits into
Open
Conversation
Teleop gets a push-to-talk button beside the speak bar: hold it (or the spacebar) and the operator's microphone plays live out the robot's speaker. The voice rides the audio m-line the robot already negotiates for its own mic, in reverse — webrtcbin offers it sendrecv, so the browser answers sendrecv, keeps an empty sender, and replaceTrack()s the mic in on press. No renegotiation, no extra m-line, and no getUserMedia (so no permission prompt) until the button is first pressed. On the robot, webrtcbin's pad-added builds a per-peer playback branch gated by a valve, so nothing reaches the speaker unless someone is holding the button. Two loops this had to close: - The robot's speaker is within earshot of its own mic, so talking is half-duplex: while an operator holds talk, the robot stops sending them its mic and restores it on release. - The brain would otherwise transcribe the operator's voice as a human talking to it, so /talkback/is_playing joins /tts/is_playing in the input ducking path — tracked as a set of sources, since TTS ending mid-sentence must not un-duck the mic while the operator is still speaking. The agent page's hold-to-talk control moves to js/micControl.js and is shared by both pages; only the wording differs. Hidden in sim, which has no speaker to play anything out of. Not yet verified on hardware: no audio has moved end to end.
Contributor
Greptile SummaryThe PR adds browser-to-robot live push-to-talk over the existing WebRTC audio m-line and coordinates speaker playback with robot-microphone ducking.
Confidence Score: 3/5The PR is not yet safe to merge because buffered microphone audio can continue playing through the robot after the operator releases push-to-talk. The privacy gate is enforced before a buffered appsrc and speaker queue, while release only changes the gate state; audio already admitted downstream remains playable after the explicit release boundary. Files Needing Attention: ros2_ws/src/mars_bot/mars_cam/mars_cam/webrtc_encode.cpp and ros2_ws/src/mars_bot/mars_cam/mars_cam/webrtc_transport.cpp
|
| Filename | Overview |
|---|---|
| webapp/js/webrtcSession.js | Adds lazy, single-flight microphone acquisition and track replacement; the epoch and desired-state checks resolve the previously reported capture lifecycle races. |
| ros2_ws/src/mars_bot/mars_cam/mars_cam/webrtc_transport.cpp | Adds inbound talkback branch attachment and atomic gate updates; attachment races are addressed, but release does not clear downstream audio. |
| ros2_ws/src/mars_bot/mars_cam/mars_cam/webrtc_encode.cpp | Adds decoding and buffered speaker mixing, with already-admitted PCM able to play after the talk gate closes. |
| ros2_ws/src/brain/brain_client/brain_client/inputs/manager.py | Generalizes microphone ducking from a TTS flag to a set of independently active speaker sources. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart LR
BrowserMic[Browser microphone] --> WebRTC[WebRTC audio m-line]
WebRTC --> Decode[Per-peer Opus decode]
Decode --> Gate{Talk gate open?}
Gate -->|Yes| Appsrc[Buffered mixer appsrc]
Gate -->|No| Drop[Drop new PCM]
Appsrc --> Mixer[Shared audio mixer]
Mixer --> Queue[200 ms speaker queue]
Queue --> Speaker[Robot speaker]
Release[Operator releases talk] -->|Closes only admission gate| Gate
Reviews (4): Last reviewed commit: "feat(teleop): echo-cancelled full-duplex..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
A getUserMedia still pending across stop() — the operator left the page mid-permission-prompt — resolved into a live capture after teardown, and setTalk then re-latched talkRequested over the false stop() had just set. The mic epoch (bumped on every close) makes the late arrival stop its own tracks and report failure instead. On the robot, a talk release landing while the first inbound audio pad was being attached stored false and found no valve to close, and the branch then went into the pipeline with the valve open from the pre-add gate snapshot — the speaker stayed live until the next toggle. The pad callback now re-arms the valve from the gate after the branch is findable; set_peer_talk stores before it looks up, so whichever writer runs last carries the current value.
DavidDobas
force-pushed
the
feat/webrtc-talkback
branch
from
August 25, 2026 06:29
d3931a7 to
541272d
Compare
The previous race fixes were snapshots racing stores; both sides now derive state from one source inside one critical section, so there is no snapshot to go stale. Browser: setTalk records the desired state before its first await, and the press that resumes from getUserMedia commits only if it still matches — a release during acquisition previously no-oped (talkRequested was still false) and the resumed press then put the operator live with the button up. Acquisition is also single-flight: a press-release-press during the first getUserMedia joined nothing and started a second capture whose loser was never stopped. Robot: the gate grows a leaf mutex and owns the valve pointer; every drop write happens under the mutex and reads `open` inside the same section. The pad-added handler's load-then-write could interleave with set_peer_talk's store-then-close so the stale snapshot reopened a valve the release had just closed. The valve is built dropping and armed only in that critical section, so audio that raced the attach never leaks; the by-name pipeline lookup is gone with the snapshot.
Collaborator
Author
Docstrings that restated signatures, rationales said twice (hpp and cpp), and elaboration beyond the invariant. The race, thread, and wire contracts all stay.
* add shortcuts * fix(teleop): let / focus speech after clicking a control
* feat(teleop): echo-cancelled full-duplex talkback
Half-duplex exists because the robot's speaker is within earshot of its own
mic: without cancellation, sending the talker the robot's mic sends them
their own voice back. Cancel the echo instead and the duck can lift — the
operator keeps hearing the room while they speak.
The per-peer playback branches collapse into one speaker pipeline
(appsrc -> depay -> decode -> volume -> [webrtcechoprobe] -> alsasink),
built at startup and left PLAYING; peers attach a thin RTP tap that
forwards packets only while their talk gate is open. One playback path is
what AEC needs — a single probe of what the speaker actually plays — and
the mic pipeline gains a webrtcdsp paired with it (delay-agnostic, since
capture and playout sit on separate clocks and dmix hides the true playout
latency). Both elements ship in gstreamer1.0-plugins-bad, already a
dependency via webrtcbin.
Collapsing the branches also deletes the valve, and with it the class of
gate-snapshot races: the gate is now only ever read, per packet, on the
tap's streaming thread — there is no write-after-read to interleave.
enable_echo_cancel defaults to false until cancellation quality is
validated on hardware; the duck stays in place while it is off, so
behavior is unchanged by default. webrtcdsp hard-errors when its probe is
missing (verified live), so the speaker pipeline is built before the mic
pipeline and AEC is forced off whenever talkback is disabled.
Verified: builds clean; the exact speaker chain reaches PLAYING with the
probe, and probe + webrtcdsp pair and run in-process (gst-launch, on the
robot). Not verified: cancellation quality with real speaker/mic acoustics,
and talkback audio end to end — same standing caveat as the base branch.
Known limit: two peers talking at once interleave RTP into one
depayloader and glitch; one talker at a time is the supported shape (the
previous design mixed via dmix but could not host a single AEC probe).
* fix(teleop): mix talkers instead of sharing one depayloader
The single shared appsrc meant every talking peer's RTP landed in one
rtpopusdepay, so two operators talking at once interleaved two SSRCs into
one depayloader and glitched — and even a handoff between them crossed a
sequence-number discontinuity. Multi-operator talkback was the property
the client_id work bought; it should not be the price of echo cancellation.
AEC needs one SINK (a single reference of what was played), not one
decoder. So the speaker pipeline is now fronted by an audiomixer and each
peer decodes in its own transport pipeline, handing over PCM:
peer: webrtcbin -> queue -> rtpopusdepay -> opusdec plc=true
-> convert/resample -> caps -> appsink
speaker: audiomixer -> convert/resample -> volume -> [probe]
-> queue -> alsasink
A peer takes a mixer input for its whole life; an idle live input costs
nothing (the mixer aggregates on the clock and reads silence), so pressing
talk stays a pure atomic store with no pipeline surgery.
Two things fall out of decoding per peer: the jitterbuffer's GAP events
reach opusdec again, so plc=true conceals loss instead of the tap dropping
events on the floor; and the peer's appsrc is bounded + leaky like every
other appsrc in the node, so a stalled speaker drops rather than grows.
* fix(teleop): harden echo-cancelled talkback from review findings
- Tear down the speaker pipeline (and clear enable_echo_cancel_) when
talkback is disabled after it was built (no mic), instead of holding
ALSA open on a dead-end PLAYING pipeline for the node's lifetime.
- Recover the lifetime-scoped speaker pipeline from bus errors with a
throttled in-place NULL->PLAYING restart; nothing else rebuilds it.
- Name the echo probe and pair webrtcdsp with it explicitly rather than
relying on the process-global auto-name in a shared composable
container; pin 48k mono in front of the probe so its rate always
matches the capture's (webrtcdsp refuses a mismatched probe).
- Degrade to half-duplex when the webrtcdsp plugin is absent instead of
losing talkback to a parse failure; release half-built speaker
pipeline state when it fails to start.
- Dedupe the talk-duck rule (three inline spellings) into
peer_audio_active().
* fix(teleop): turn AEC on and align it via reported latency
Listening tests said the echo cancellation "wasn't working"; a measured
sweep on the robot says it literally wasn't. Playing speech-band noise
bursts out the speaker while recording the mic through the node's exact
gst topology (probe on playout, webrtcdsp on capture):
no AEC echo at -8.9 dBFS (baseline)
delay-agnostic=true, any echo at -9.9 dBFS (~1 dB ERLE)
delay-agnostic=false echo at -61 dBFS (~52 dB ERLE)
delay-agnostic mode - chosen on the assumption that dmix hides the true
playout latency, so the canceller must estimate alignment itself - never
aligns on this hardware, at any suppression level, with AGC on or off,
with the extended filter on or off. With reported latencies the canceller
converges on the first burst and the residual lands below the mic's own
noise floor. Verified against a node-faithful speaker branch (audiomixer
-> volume -> probe -> leaky queue -> unsynced alsasink) and an
echo-cancel=false control that brings the echo back, proving the silence
is cancellation rather than a dead playback path.
Suppression drops back from high to moderate: once the linear filter is
aligned the levels measure the same, and moderate keeps an interrupting
far-end voice alive during double-talk.
With the canceller actually working, enable_echo_cancel goes on in both
launch files - which also lifts the half-duplex talk duck, so an unmuted
operator keeps hearing the room.
* feat(teleop): replace push-to-talk with a Meet-style mute toggle
Holding a button to speak made two-way conversation a walkie-talkie
exercise. The mic control grows a "toggle" mode - click or a spacebar
tap flips mute/unmute - and teleop uses it; the agent page keeps hold
semantics. A hidden tab force-mutes so a forgotten tab never keeps a
live mic open in the robot's room.
While unmuted the mic streams continuously and receive stays open (the
robot's AEC keeps the operator's voice out of the return path; a robot
without AEC still ducks its outbound mic, degrading to deaf-while-
unmuted rather than echo). On top of that, the waveform meter doubles
as a voice-activity gate: while the operator speaks, local playback
ducks to 12% volume - low enough to bury any AEC residual, loud enough
that a shout or an interrupting voice still reads through - and
recovers 900ms after the last syllable, long enough for that
syllable's echo to finish the speaker->mic->return round trip.
Muting keeps the existing behavior: listen until three seconds of room
silence (or indefinitely with Always Listen).
---------
Co-authored-by: theo-michel <theo64200@gmail.com>
Comment on lines
+465
to
+471
| if (buffer && src && gate && (*gate)->load(std::memory_order_relaxed)) { | ||
| GstBuffer* out = gst_buffer_copy(buffer); | ||
| GST_BUFFER_PTS(out) = GST_CLOCK_TIME_NONE; | ||
| GST_BUFFER_DTS(out) = GST_CLOCK_TIME_NONE; | ||
| GstFlowReturn ret; | ||
| // "push-buffer" is transfer-none (see fan_out) — unref or leak one buffer per push. | ||
| g_signal_emit_by_name(src, "push-buffer", out, &ret); |
Contributor
There was a problem hiding this comment.
When the operator releases push-to-talk after PCM has entered the mixer appsrc or speaker queue, release only closes the admission gate, so the buffered microphone audio continues playing through the robot speaker until it drains—up to roughly 2.7 seconds of appsrc backlog plus the downstream queue under playback backpressure.
How this was verified: The release path only stores false in the gate, while admitted PCM remains in independently buffered downstream elements.
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.
Teleop gets a push-to-talk button beside the speak bar: hold it (or the spacebar) and your microphone plays live out the robot's speaker. Same control the agent page uses, different destination — that one becomes a transcript, this one becomes sound in the room.
How it works
The voice rides the audio m-line the robot already negotiates for its own mic, in reverse:
webrtcbinoffers that m-linesendrecv, so the browser answerssendrecvand keeps an empty sender, thenreplaceTrack()s the mic in on press. No renegotiation, no extra m-line, and nogetUserMedia— so no permission prompt — until the button is first pressed.pad-addedbuilds a per-peervalve → rtpopusdepay → opusdec → convert/resample → volume → queue → alsasinkbranch. The valve is the privacy gate: nothing reaches the speaker unless someone is holding the button.STARTcarriestalk,want_talk_refcounts it,/webrtc/active_streamsreports it per client.defaultis already dmix+softvol, so this shares the device with the brain's TTSaplayinstead of fighting it for exclusive access.Turning talkback on is a
START, not a reconnect — the same instant path the camera and mic toggles already take.Two feedback loops this had to close
/talkback/is_playingjoins/tts/is_playingin the input ducking path — tracked as a set of sources, not a flag, since TTS ending mid-sentence must not un-duck the mic while the operator is still speaking.Threading note for reviewers
pad-addedfires on webrtcbin's streaming thread, which must not takepeers_mutex_—~Peerrunsset_state(NULL)under that lock and joins this thread. The handler therefore takes nothing frompeers_: the pipeline comes from the pad's parent, and the talk state from ashared_ptr<atomic<bool>>tagged on the element, the same lock-free escapemedia_readyalready uses.Design notes
sendrecvfor sink-pad transceivers — verified against 1.20.3 before writing the branch. The code sets it explicitly and checks it stuck anyway, so a future version defaulting to sendonly disables talkback loudly instead of silently swallowing the operator's voice.AudioContextcreated outside a user gesture starts suspended, so a silent track would unreliably produce RTP while costing a permanently-negotiated send slot on every idle tab. Without it there is no RTP at all until the first press; the cost is a possible few-ms clip on the very first press of a session.config.simControls), like the existing mic toggle — the sim container has no speaker to play anything out of.webapp/js/micControl.jsand is now shared; only the wording differs. Agent behavior is unchanged.New parameters
enable_talkback(default true),audio_sink_element,audio_playback_device,talkback_volume. Talkback disables itself when there is no audio m-line or no sink element, which is why the sim launch (enable_audio: False) needs no change.Verification
brain_client: 274 tests pass, ruff check + format clean.a=sendrecvon the audio m-line, and the playback branch parses and ghost-pads correctly.Not yet verified on hardware — no audio has moved end to end. Before merge this wants
innate buildon a robot and a two-person check: hold talk and confirm the voice comes out the speaker, confirm TTS still plays concurrently through dmix, and measure clap-to-speaker latency.