Repository navigation
Let the candidate switch microphones mid-interview - #426
Conversation
57ec696 to
26bdc3b
Compare
jserv
left a comment
There was a problem hiding this comment.
Check CONTRIBUTING.md carefully and run make indent.
26bdc3b to
562dceb
Compare
562dceb to
a50bed6
Compare
a50bed6 to
8bf697d
Compare
a446df2 to
e5dc048
Compare
e5dc048 to
9453697
Compare
08adfa1 to
980d5b8
Compare
980d5b8 to
19a397b
Compare
ea07c3a to
1fea9dc
Compare
jserv
left a comment
There was a problem hiding this comment.
This changes what the web pages show, so the pull request description must include screenshots of the result (before and after where something changed), with any personal data removed.
| track.enabled = micEnabled(); | ||
| if (!micEnabled()) await matchMute(published, false); | ||
| await published.replaceTrack(track, { userProvidedTrack: true }); |
There was a problem hiding this comment.
track.enabled is read from micEnabled() here and not again until the commit below, while the new input is reachable from neither the page stream nor the Mic button. A candidate who presses Mic while replaceTrack is pending gets setLocalAudioEnabled(false) on the old track and LiveKit's setTrackMuted on whichever track it still holds, but the replacement stays enabled and is sent until the commit disables it. That breaks the "never live while muted" guarantee this change sets out to give. Keep the pending track on the switch (for example a setEnabled(enabled) the button calls, which also updates the in-flight track) so a mute during the swap reaches it.
1fea9dc to
7c8be47
Compare
| if (!microphoneLive()) return; | ||
| const held = state.localUserStream?.getAudioTracks?.()[0] ?? null; | ||
| if (held && held.readyState !== "ended") return; | ||
| void microphoneSwitch().switchTo("", "lost", held); |
There was a problem hiding this comment.
After a chosen switch strands (the working input could not be handed back to the sender), the stream has no audio track. The next devicechange of any kind, even a headset output appearing, then lands here and moves the interview to the default with reason=lost. The trail records a loss for an input that already recorded MICROPHONE_STOPPED with reason=switch_failed, and the candidate is told their microphone disconnected. Meanwhile docs/integrity-evidence.md says this case waits for the candidate to pick again. Either remember in onFailed which reason stranded the stream and retry with that, or retry only after a loss and leave the chosen case to the select.
jserv
left a comment
There was a problem hiding this comment.
This changes what the web pages show, so the pull request description must include screenshots of the result (before and after where something changed), with any personal data removed.
7c8be47 to
e64b426
Compare
e64b426 to
088256d
Compare
jserv
left a comment
There was a problem hiding this comment.
Rebase latest main branch and explain how the CI result changes.
The gates (browser) failure here is Strict delayed judge language restoration respects end in tests/browser/strict-selection.test.js. This branch did not break it: the test was flaky on main itself. After End it waited for the ending overlay, but its room has no interviewer, so the offline summary replaced that overlay a moment later and a loaded runner missed the window. #461 (merge b0059b2) fixed the test by pausing a fake clock before End, and main passes the browser lane again at that commit. This branch is based on a commit before b0059b2, so it still runs the old test. After rebasing, reply here with the new CI result, and if anything still fails, say whether it comes from this branch or also fails on main.
| nodes.micInputSelect.addEventListener("change", () => { | ||
| void microphoneSwitch().switchTo( | ||
| nodes.micInputSelect.value, | ||
| "chosen", | ||
| null, | ||
| soleGroupOf(nodes.micInputSelect), | ||
| ); | ||
| }); |
There was a problem hiding this comment.
On Windows and Linux, Chromium changes a closed select's value on each Up or Down key and fires change every time, so a keyboard user moving from the first input to the third also opens, sends and records the one in between: two swaps and two MICROPHONE_SWITCHED events with reason=chosen for one decision, since the chain queues every pick and runs each in turn. Let a queued chosen pick that a later pick has superseded return "unchanged" before it opens anything, for example by keeping the last requested deviceId in switchTo and comparing it at the top of run.
ColtenOuO
left a comment
There was a problem hiding this comment.
Using AI to generate PR descriptions is totally fine, but please at least review what you're pasting. For instance, the first line is completely redundant.
jserv
left a comment
There was a problem hiding this comment.
Rebase onto the latest main branch, fix any regressions detected by GitHub Actions, and address the review comments.
088256d to
f21cbf5
Compare
@gino1203, do you understand the issue @ColtenOuO raised? Carefully review this pull request’s description and any AI-generated code. You MUST respond thoughtfully. Don’t just act as an operator of AI tools. Think for yourself and use your own judgment. |
f21cbf5 to
b108bda
Compare
| // publication's flag inside the replace, so the flag is held up across | ||
| // the swap and the new input is never live before the Mic state says. | ||
| track.enabled = micEnabled(); | ||
| await holdMute(published); |
There was a problem hiding this comment.
holdMute goes through LiveKit's mute(), which disables the track still in localUserStream (the old input) until liftMute re-enables it. The 1s state poll in startIntegrityTrackStateMonitor and the heartbeat read track.enabled from that stream, so a poll that lands inside the replaceTrack window, or inside a hand-back that rolls the switch back, records a MICROPHONE_STATE_CHANGED with live/off (severity warning) and microphone=live/off in the heartbeat, for a candidate whose Mic button still reads on. That is a false entry in the evidence trail, and it is written by the swap, not by the candidate. Consider a flag on state that the poll and integrityHeartbeatDetail consult while a swap holds the mute, and have them report the Mic button state (state.micEnabled) for that stretch instead of the raw enabled.
Once the interview starts, nothing can move it off the microphone the preflight opened, so a headset plugged in afterwards goes unused while the laptop microphone keeps hearing Jim, and the only way out is to end the interview and start over. Choosing a device before the interview is sysprog21#206; this covers the interview itself. The switch reuses the one capture path and replaces the published track rather than calling LiveKit's switchActiveDevice, which would reopen the device inside the vendored client and leave the stream the integrity checks read holding a stopped track. Because that trail exists to say whether the microphone is still the one the interview started with, a switch records MICROPHONE_SWITCHED, which the agent now accepts.
b108bda to
066d209
Compare
|
Thank @gino1203 for contributing! |
Once the interview started, the page had no way to move off the microphone the preflight opened, so a headset plugged in afterwards went unused while the laptop microphone kept hearing Jim through the speakers, and the only fix was to end the interview and start over. A select beside the Mic button now lists the inputs once the room is joined, and picking one moves the published audio to it without reconnecting. Chrome's default and Windows' communications aliases are left out of the list, as in #411's preflight picker, so picking the alias of the input in use cannot reopen it. Every real input keeps its own row, including several on one audio interface that share a groupId, and the select stays hidden unless there are at least two rows. When the input in use matches no row, such as a system default with a group of its own, the select names it instead of reading as if no microphone were open. The note under the controls is announced from a status region that is always present, since one hidden until filled is skipped by screen readers, and when unplugging leaves one input the select hands focus to the Mic button before it hides.
This covers the interview after it starts and nothing before it. Choosing devices in the preflight is #206, which #411 added; this change leaves the preflight alone and stores no device preference. Rebased onto #411, its selectMicrophone in web/devices.js now builds its constraints with the same helper the switch opens its input with, both capture through the one getUserMedia call, and the header of its web/audio-input.js names web/mic-switch.js as the mid-interview counterpart.
The new input is opened through the existing request path in web/devices.js, so the getUserMedia allowlist in tests/browser/meet-tab-audio.test.js still finds exactly one capture site, and it is handed to the published track with replaceTrack(track, { userProvidedTrack: true }). LiveKit's switchActiveDevice was the shorter route, but it reopens the device inside the vendored client and leaves the page's own stream, which the integrity heartbeat, the track-state poll and the turn ring read, holding the stopped track. A switch that fails hands the sender back to the input still working; when nothing can be sent, the page says Jim cannot hear the candidate, keeps the select open, and retries when a device is plugged in.
A switch records a MICROPHONE_SWITCHED integrity event (reason=chosen or reason=lost, then the device label) and no MICROPHONE_STOPPED. sanitize_integrity_event lists the new type in the same change, because the agent drops an unlisted type before it checks the chain, so every later event is refused as out of sequence. The wire fixture gains one generated row at the end, so the earlier hashes are unchanged and browser_generated_integrity_events_all_verify_in_the_agent fails if the type is dropped from the allowlist. A failed switch that has to drop an input that was still working records MICROPHONE_STOPPED with reason=switch_failed, since stopping it fires no ended. docs/integrity-evidence.md describes the event and both failure cases. While a switch holds LiveKit's mute flag, LiveKit has disabled the input still in the stream, so for that stretch the heartbeat and the 1s state poll report the Mic button's state rather than the track's, and a candidate who never pressed Mic is not recorded as muted.
The Mic button still mutes the track only, and also reaches the input a switch is still handing to the sender, which is in neither the stream nor the publication yet. The publication's own mute flag is a server-side mute: the SFU stops forwarding the track, so the agent would receive no frames rather than the silence that ends a spoken turn. The switch therefore raises that flag only across replaceTrack, since LiveKit enables a track it is handed from it, and does so whatever the Mic state, because the candidate can press Mic while the replace is pending; it lowers it afterwards with the Mic state read at that point, including when LiveKit raised it itself after a loss. LiveKit enables the track and announces unmuted in one synchronous step when the flag comes down, and a listener on that event disables a muted candidate's input again before anything else runs, so it is never live.
Validation: tests/browser/mic-switch.test.js drives the switch against fake tracks and a fake LiveKit publication and drives the page's device listing through loadInterview; tests/browser/devices.test.js covers the exact-device and default requests. Each guarded behavior was broken in turn and a test failed. node scripts/gen-wire-fixtures.mjs --check, rustfmt --check and Prettier pass. cargo test, clippy, ESLint and the Playwright lanes run in CI only, since this checkout lacks the vendored files; the browser tests that fail locally for that reason fail identically on a clean main. The screenshots below come from that flow in Chromium with its fake media devices (--use-fake-device-for-media-stream) and a LiveKit room that connects nowhere, the same setup tests/browser/restore-starter.test.js uses; the device names are Chromium's own. Not checked: a real browser with two physical microphones.
Screenshots of the interview controls after joining. Before, on main:
After, on joining, with the input in use named in the select:

After picking another input:

After picking an input that refuses to open:
