Skip to content

Commit 68e7fe5

Browse files
committed
fix(wgc): make the GPU encode path opt-in until it earns the default
It has one confirmed success and one confirmed regression. On the machine in #252 it took display and window capture from a 13-second hang to a 105 ms stop. On the machine in #336 -- the same person who reported #327 -- it ended recording outright, where the CPU path had been producing video. Its fallbacks cover every check made inside initialize(). Nothing covers a failure that only shows up once frames are flowing, which is exactly what #336 is: the pipeline comes up, the sink writer is configured for NV12, and then a per-frame call fails with nowhere left to fall back to. Defaulting it on makes every user carry that risk so that the few who reproduce #252 might not have to, and #252 is not confirmed fixed by anyone. So: off, and OPENSCREEN_WGC_ENABLE_DXGI_INPUT=1 turns it on, which is what the people in #252 and #327 should be given to test with. Nothing is reverted. The path, its six review fixes, the writerMutex_ lock-order split, the stop breadcrumbs and the adapter reporting all stay -- they are either independent of this default or exist to diagnose the bug it was written for. The one line is the whole change, which is the point: the code was fine, the confidence behind switching it on for everyone was not. Earning the default back means one of two things: a failure mid-encode degrades to the CPU path instead of ending the recording, or someone on the affected hardware confirms it fixes #252.
1 parent be6e883 commit 68e7fe5

3 files changed

Lines changed: 23 additions & 16 deletions

File tree

electron/native/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ The current helper implementation supports display/window video capture, system
8585

8686
Encoder selection: by default the helper keeps the existing sink-writer path first. If that path fails while setting up H.264, it retries with the Microsoft software H.264 encoder (`mfh264enc.dll`). The key of this retry is registering that encoder locally in the helper process via `MFTRegisterLocalByCLSID`, which makes a software H.264 encoder available even when the machine's hardware encoders are missing or broken; hardware transforms are disabled for the retry only as a secondary guard so the sink writer prefers the locally registered software encoder, not as the fallback mechanism itself. Set `preferSoftwareEncoder: true` in the helper JSON, or set `OPENSCREEN_WGC_PREFER_SOFTWARE_ENCODER=true` before launching Electron, to force the software path from the first attempt.
8787

88-
Frame input path: the helper feeds the encoder from the GPU when it can. On that path it copies the WGC frame across a keyed-mutex bridge to a second D3D11 device, converts BGRA to NV12 with the D3D11 video processor, and submits an allocator-owned DXGI sample to the hardware H.264 encoder, so no frame ever passes through system memory. The alternative is the original path: a staging texture, `Map(D3D11_MAP_READ)`, and a row-by-row copy into an `IMFMediaBuffer` — which is where a driver stall costs a recording (issue #252). The GPU path is a preference, never a requirement: it is skipped outright for `preferSoftwareEncoder` and for inline webcam PiP (both need the frame in system memory), and it degrades to the CPU path on its own if the encoding device, the NV12 video processor, the shared bridge texture, the DXGI sample allocator, or the hardware sink writer is unavailable. Set `OPENSCREEN_WGC_DISABLE_DXGI_INPUT=1` to force the CPU path. Because the two paths land on different encoders and hardware MFTs default to constant bitrate, the GPU path asks for VBR through `ICodecAPI`; without it a static screen spends the full configured budget (measured 16.9 Mbps against 1.95 for the same desktop).
88+
Frame input path: the helper feeds the encoder from the GPU when it can. On that path it copies the WGC frame across a keyed-mutex bridge to a second D3D11 device, converts BGRA to NV12 with the D3D11 video processor, and submits an allocator-owned DXGI sample to the hardware H.264 encoder, so no frame ever passes through system memory. The alternative is the original path: a staging texture, `Map(D3D11_MAP_READ)`, and a row-by-row copy into an `IMFMediaBuffer` — which is where a driver stall costs a recording (issue #252). The GPU path is a preference, never a requirement: it is skipped outright for `preferSoftwareEncoder` and for inline webcam PiP (both need the frame in system memory), and it degrades to the CPU path on its own if the encoding device, the NV12 video processor, the shared bridge texture, the DXGI sample allocator, or the hardware sink writer is unavailable. The GPU path is OFF by default: it fixed #252 on the machine that reproduces it and broke recording outright in #336, and its fallbacks only cover failures during `initialize()`, not one that appears once frames are flowing. Set `OPENSCREEN_WGC_ENABLE_DXGI_INPUT=1` to turn it on. Because the two paths land on different encoders and hardware MFTs default to constant bitrate, the GPU path asks for VBR through `ICodecAPI`; without it a static screen spends the full configured budget (measured 16.9 Mbps against 1.95 for the same desktop).
8989

9090
The helper reports the outcome through the `encoder-selection` stdout event (`video` is `default`, `software-preferred`, or `software-fallback`; `videoInput` is `dxgi-nv12` or `cpu-rgb32`, and reports what the encoder settled on rather than what was asked for). On the GPU path the helper also prints one `[frame-drops] gpu_bridge_contended=<n>` line to stderr at stop: a frame the bridge was too busy to take is skipped rather than failing the recording, and a large count there is the first thing to look at in a report about missing frames. When the app sees `software-fallback` — the default encoder failed and the helper switched on its own — it shows a small dismissible notice in the recording HUD with a "Don't show again" option, because software encoding can raise CPU usage. An explicit `software-preferred` selection shows no notice, and the event stays available for diagnostics either way.
9191

electron/native/wgc-capture/src/main.cpp

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -710,23 +710,30 @@ int main(int argc, char* argv[]) {
710710
MFEncoderOptions encoderOptions{};
711711
encoderOptions.preferSoftwareEncoder = config.preferSoftwareEncoder;
712712
encoderOptions.injectDefaultSinkWriterFailureOnce = injectDefaultSinkWriterFailureOnce;
713-
// Keep the CPU path for software encoding and inline webcam PiP: both need
714-
// the frame in system memory, which is the one thing the DXGI path does not
715-
// produce. The env var is the escape hatch for a machine where the GPU path
716-
// misbehaves in a way the encoder's own probes do not catch -- a support
717-
// answer instead of a hotfix.
713+
// OFF by default. The GPU path exists to dodge a Map() that wedges inside
714+
// the display driver on the machine in #252, and it demonstrably fixed
715+
// display and window capture there. It also broke recording outright for
716+
// the reporter in #336, who had working video before it. Its fallbacks
717+
// cover every check made during initialize(); nothing covers a failure that
718+
// only appears once frames are flowing, which is what #336 is.
718719
//
719-
// config.webcamEnabled, not webcamActive: the latter is only set once the
720-
// webcam capture has started, which happens well after this. Reading it
721-
// here made the PiP condition dead code -- always false, so always
722-
// permitting the GPU path -- and an inline-PiP recording would have run on
723-
// DXGI and silently dropped the overlay, reporting success either way.
724-
// config.webcamEnabled is already cleared above when webcam init fails,
725-
// and writeSeparateWebcam is assigned there too, so both are final here.
720+
// So it is opt-in until a failure mid-encode degrades to the CPU path
721+
// instead of ending the recording, or until someone confirms it closes
722+
// #252. Neither has happened, and defaulting it on means every user carries
723+
// the risk so that the few who reproduce #252 might not have to.
724+
//
725+
// Set OPENSCREEN_WGC_ENABLE_DXGI_INPUT=1 to turn it on -- that is what the
726+
// people in #252 and #327 should be given to test with.
727+
//
728+
// The other two conditions are unchanged and still required: software
729+
// encoding and inline webcam PiP both need the frame in system memory,
730+
// which the DXGI path does not produce. config.webcamEnabled, not
731+
// webcamActive -- the latter is only set once webcam capture has started,
732+
// well after this.
726733
encoderOptions.useDxgiInput =
734+
readEnvInt("OPENSCREEN_WGC_ENABLE_DXGI_INPUT", 0) == 1 &&
727735
!config.preferSoftwareEncoder &&
728-
(!config.webcamEnabled || writeSeparateWebcam) &&
729-
readEnvInt("OPENSCREEN_WGC_DISABLE_DXGI_INPUT", 0) == 0;
736+
(!config.webcamEnabled || writeSeparateWebcam);
730737

731738
MFEncoder encoder;
732739
if (!encoder.initialize(

technical-documentation/architecture/recording.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,7 @@ Cursor samples are persisted as cursor telemetry rather than baked into editable
6969

7070
- A window with odd client dimensions can produce black video: H.264 encoding requires even dimensions (`electron/native/wgc-capture/src/wgc_session.cpp:38`).
7171
- The Windows helper's frame lock (`electron/native/wgc-capture/src/main.cpp`) is still held across blocking, uninterruptible D3D11 work: the WGC callback's `CopyResource`, and whatever the video writer does with the frame. A driver that stalls inside either one still costs the recording. What no longer happens is a hang: stop detection runs on `CaptureControl::stopMutex`, which no frame thread ever touches, and a shutdown watchdog force-exits the helper when a step overruns its budget, naming the step it died in. Each step gets `OPENSCREEN_WGC_STEP_BUDGET_MS` (8s by default) and that is the bound which normally fires; the whole shutdown is capped by `OPENSCREEN_WGC_STOP_BUDGET_MS` (50s by default), which the encoder-finalize step alone is allowed to spend in full because a long software-encoder finalize legitimately takes seconds (issue #34). Picking the D3D adapter that actually drives the captured monitor instead of adapter 0 is still outstanding.
72-
- The video writer has two ways to get a frame to the encoder, and which one runs is a per-machine outcome, not a setting. The GPU path (`videoInput: "dxgi-nv12"`) copies the frame across a keyed-mutex bridge to a second D3D11 device, converts BGRA to NV12 with the D3D11 video processor, and hands the hardware H.264 encoder a DXGI sample; it never touches system memory. The CPU path (`videoInput: "cpu-rgb32"`) is the original staging-texture `Map(D3D11_MAP_READ)` readback, and is what a `Map`/`Unmap` that never returns wedges (issue #252: Windows 10, WDDM 2.7, multi-adapter). The GPU path is the default and degrades to the CPU one on its own at every step — no hardware encoder, no NV12 video-processor output, no shared keyed-mutex texture, no DXGI sample allocator — so a machine it does not fit records exactly as it did before it existed. It is skipped outright for `preferSoftwareEncoder` and for inline webcam PiP, both of which need the frame in system memory, and `OPENSCREEN_WGC_DISABLE_DXGI_INPUT=1` forces it off. The two paths land on different encoders, so the GPU one asks for VBR explicitly through `ICodecAPI`: hardware MFTs default to constant bitrate and would spend the full configured budget on a static screen (measured 16.9 Mbps against 1.95 for the same desktop).
72+
- The video writer has two ways to get a frame to the encoder, and which one runs is a per-machine outcome, not a setting. The GPU path (`videoInput: "dxgi-nv12"`) copies the frame across a keyed-mutex bridge to a second D3D11 device, converts BGRA to NV12 with the D3D11 video processor, and hands the hardware H.264 encoder a DXGI sample; it never touches system memory. The CPU path (`videoInput: "cpu-rgb32"`) is the original staging-texture `Map(D3D11_MAP_READ)` readback, and is what a `Map`/`Unmap` that never returns wedges (issue #252: Windows 10, WDDM 2.7, multi-adapter). The GPU path is OFF by default (`OPENSCREEN_WGC_ENABLE_DXGI_INPUT=1` turns it on) and degrades to the CPU one on its own at every step — no hardware encoder, no NV12 video-processor output, no shared keyed-mutex texture, no DXGI sample allocator — so a machine it does not fit records exactly as it did before it existed. It is skipped outright for `preferSoftwareEncoder` and for inline webcam PiP, both of which need the frame in system memory, It is opt-in because its fallbacks cover every check made during initialization and none of a failure that only appears mid-encode, which is what cost the reporter in #336 their recording. The two paths land on different encoders, so the GPU one asks for VBR explicitly through `ICodecAPI`: hardware MFTs default to constant bitrate and would spend the full configured budget on a static screen (measured 16.9 Mbps against 1.95 for the same desktop).
7373
- Linux/Wayland can produce no usable frames on the `getDisplayMedia` fallback because Chromium initializes Vulkan against the Ozone Wayland backend. The PipeWire helper path is unaffected.
7474
- On Linux the compositor's source picker appears on every recording. That is deliberate — see "Why Linux sends no source identity" — but it is an interruption, and there is currently no way to reuse a previous choice without also making it impossible to change.
7575
- Holding a portal session across the countdown means the compositor's "screen is being shared" indicator is up before recording begins. That is honest — access really has been granted — but the user can click it to revoke, or close the window they picked. The helper's exit surfaces as a rejected `waitUntilSourceSelected`; the session is not yet subscribed to the portal's `Session::Closed` signal, so a revocation is reported as a failed start rather than a specific message.

0 commit comments

Comments
 (0)