Skip to content

Commit 417d541

Browse files
committed
fix(windows): finish #726 review cleanup
workerLoop() used the shared MMDeviceEnumerator without initializing COM on the worker thread (baseline lookups and device events could fail with CO_E_NOTINITIALIZED, and CoTaskMemFree has the same requirement); initialize MTA there and balance it with CoUninitialize on the single exit path, matching the WasapiRenderKeepAlive thread. recording.md's mic-only paragraph now says "before the keep-alive" so it no longer contradicts the mitigation that follows it.
1 parent 495c1bb commit 417d541

2 files changed

Lines changed: 14 additions & 2 deletions

File tree

‎electron/native/wgc-capture/src/wasapi_device_watcher.cpp‎

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,17 @@ void WasapiDeviceWatcher::stop() {
147147
}
148148

149149
void WasapiDeviceWatcher::workerLoop() {
150+
// GetDefaultAudioEndpoint/GetDevice/CoTaskMemFree are called from this thread,
151+
// which is otherwise never COM-initialized. Both this and the wmain thread are
152+
// MTA (see winrt::init_apartment in main.cpp), so no marshaling is needed --
153+
// this only satisfies the "calling thread must be initialized" requirement.
154+
const HRESULT comInit = CoInitializeEx(nullptr, COINIT_MULTITHREADED);
155+
if (FAILED(comInit)) {
156+
std::cerr << "WARNING: [device-watcher] CoInitializeEx(COINIT_MULTITHREADED) failed (hr=0x"
157+
<< std::hex << comInit << std::dec << ")" << std::endl;
158+
return;
159+
}
160+
150161
// This thread owns all name resolution and all event writes for this watcher,
151162
// so nothing here runs on an IMMNotificationClient callback thread.
152163
while (true) {
@@ -156,7 +167,7 @@ void WasapiDeviceWatcher::workerLoop() {
156167
queueCv_.wait(lock, [this] { return !queue_.empty() || workerStopRequested_; });
157168
if (queue_.empty()) {
158169
if (workerStopRequested_) {
159-
return;
170+
break;
160171
}
161172
continue;
162173
}
@@ -170,6 +181,7 @@ void WasapiDeviceWatcher::workerLoop() {
170181
writeDeviceEvent(event);
171182
}
172183
}
184+
CoUninitialize();
173185
}
174186

175187
void WasapiDeviceWatcher::enqueueBaseline(EDataFlow flow, const wchar_t* flowLabel) {

‎technical-documentation/architecture/recording.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ A session writes a screen video and a `.session.json` manifest. Windows normally
7272

7373
The Windows helper mixes system loopback and microphone into one track, and timestamps it from a running count of emitted frames. That count is advanced by a clock rather than by the arrival of samples: a chunk goes out every 10 ms for as long as the recording runs, filled from whichever source has data and with silence where neither does. Advancing it only when a queue held samples is what made a take that began in silence emit nothing at all — WASAPI loopback delivers no packets while nothing is playing — so the first sound landed at timestamp zero and the track came out shorter than the take. A working microphone concealed it by streaming continuously, which is why it appeared as a system-audio desync on a machine whose microphone had failed. `npm run test:wgc-audio-timeline:win` measures where a tone played at a known instant actually lands.
7474

75-
What a mic-only recording does to the render (output) endpoint is: nothing. System-audio capture is the only path that even opens it, and it reads rather than writes — which reads as idle to some wireless headsets' own firmware idle timers, and they power the set down mid-take (#724: a Corsair Void dropped reliably seven to ten minutes in, as a full disconnect the OS never observes; the USB dongle stays enumerated and the WASAPI endpoints stay ACTIVE throughout, so `IMMNotificationClient` sees nothing, and neither do the device's PnP properties). The helper therefore keeps the endpoint fed: when system audio is not being captured, it opens a second, ordinary shared-mode render stream on the default output device and writes a 19 kHz tone at 0.3% amplitude for as long as the take runs — above what the large majority of adults can hear, and real signal rather than packets flagged `AUDCLNT_BUFFERFLAGS_SILENT`, because the same hardware testing that found the timer also found that digital silence does not hold it off while an actual waveform does. The gate is not an optimization: loopback already fills the endpoint with real content when it runs, and anything this stream wrote beside it would be mixed straight into the recording's own system-audio track. It is a workaround, not a fix — the timer lives in the headset's firmware, below everything an application can observe or configure, and the per-vendor setting (iCUE for Corsair, equivalents elsewhere) is the only way to turn it off; "inaudible" is likewise per listener, since hearing range and driver harmonics vary. `OPENSCREEN_WGC_DISABLE_AUDIO_KEEPALIVE=1` turns the stream off. `OPENSCREEN_WGC_LOG_AUDIO_DEVICE_EVENTS=1` runs a diagnostic device watcher alongside the take, logging endpoint state transitions as JSON events to stderr — with the expectation, learned from #724, that a real firmware drop logs nothing at all, for exactly the reason above.
75+
Before the keep-alive below, a mic-only recording did nothing to the render (output) endpoint: system-audio capture was the only path that even opened it, and it reads rather than writes — which reads as idle to some wireless headsets' own firmware idle timers, and they power the set down mid-take (#724: a Corsair Void dropped reliably seven to ten minutes in, as a full disconnect the OS never observes; the USB dongle stays enumerated and the WASAPI endpoints stay ACTIVE throughout, so `IMMNotificationClient` sees nothing, and neither do the device's PnP properties). The helper therefore keeps the endpoint fed: when system audio is not being captured, it opens a second, ordinary shared-mode render stream on the default output device and writes a 19 kHz tone at 0.3% amplitude for as long as the take runs — above what the large majority of adults can hear, and real signal rather than packets flagged `AUDCLNT_BUFFERFLAGS_SILENT`, because the same hardware testing that found the timer also found that digital silence does not hold it off while an actual waveform does. The gate is not an optimization: loopback already fills the endpoint with real content when it runs, and anything this stream wrote beside it would be mixed straight into the recording's own system-audio track. It is a workaround, not a fix — the timer lives in the headset's firmware, below everything an application can observe or configure, and the per-vendor setting (iCUE for Corsair, equivalents elsewhere) is the only way to turn it off; "inaudible" is likewise per listener, since hearing range and driver harmonics vary. `OPENSCREEN_WGC_DISABLE_AUDIO_KEEPALIVE=1` turns the stream off. `OPENSCREEN_WGC_LOG_AUDIO_DEVICE_EVENTS=1` runs a diagnostic device watcher alongside the take, logging endpoint state transitions as JSON events to stderr — with the expectation, learned from #724, that a real firmware drop logs nothing at all, for exactly the reason above.
7676

7777
The macOS helper mixes the same two sources into one AAC track (`AudioTrackMixer`) and runs the same discipline, arrived at from the opposite direction. There the emit cursor is anchored to the writer session start and advanced by a monotonic clock read off `CMClockGetHostTimeClock` — the domain ScreenCaptureKit already timestamps its buffers in — with a 10 ms timer on the sample queue moving it while nothing is arriving to move it. The bug it replaces was the anchor, not the counter: `anchor` was set lazily on the first decoded buffer to `max(firstPresentationTime, sessionStart)`, so audio that first arrived four seconds in made frame zero of the track *be* four seconds in. The trailing half was worse still — `drain()` ran only from `ingest` and `finish`, and the helper never called `writer.endSession(atSourceTime:)` at all, so the file ended at the last sample anything happened to deliver. Both halves and any mid-take gap are now one mechanism: chunks go out for as long as the take runs, from whichever source covers them and from silence where none does, and a source is written off for a chunk only once it has gone 250 ms without delivering anything — a decision about when to stop waiting, which shifts nothing, and not a correction, which would. Measured from that source's own last delivery rather than from how far behind the clock its coverage sits, because those differ for a source that is alive but arriving late: a capture path delivering a fixed delay behind the audio it describes would be "behind" forever, so writing it off for that would emit silence and then drop its real samples, every chunk, for the whole take. A pause freezes the clock rather than resetting it, so what follows keeps the position it would have had. Whether ScreenCaptureKit's system-audio output is silence-gapped the way WASAPI loopback is has never been recorded anywhere, and it decides how much of a take the clock is carrying alone rather than how the mixer should work; the helper now reports it per take as `audio-timeline`, whose `undeliveredSeconds` is the whole track if the tap is gapped and near zero if it streams silence. That figure counts what each source handed over rather than what came out of the mixer — holes under two seconds are zero-filled into the source's own buffer, so a hole read off the mixed output would come back covered, which is the exact case it exists to catch — and `droppedSeconds` beside it is the cost side, counting audio that arrived too late to place and reading zero unless the grace is too short for that tap. `swift test` runs the mixer against a collecting sink and a hand-moved clock on every pull request (`swift-macos-helper` in ci.yml, the first Swift job this repository has had); `npm run test:sck-audio-timeline:mac` measures a real tone through a real device, comparing the audio track's length against the video track's in the same file.
7878

0 commit comments

Comments
 (0)