From 31ec87f5555614a078648c9b971e254cf47f4fbf Mon Sep 17 00:00:00 2001 From: Seb1900 <1712315938@qq.com> Date: Sat, 8 Aug 2026 19:37:46 +0800 Subject: [PATCH 1/7] fix(wgc): add GPU DXGI path for Windows capture readback --- electron/native/wgc-capture/src/main.cpp | 27 +- .../native/wgc-capture/src/mf_encoder.cpp | 394 +++++++++++++++++- electron/native/wgc-capture/src/mf_encoder.h | 24 ++ .../native/wgc-capture/src/wgc_session.cpp | 9 +- 4 files changed, 441 insertions(+), 13 deletions(-) diff --git a/electron/native/wgc-capture/src/main.cpp b/electron/native/wgc-capture/src/main.cpp index 63cbcbba..b7878b26 100644 --- a/electron/native/wgc-capture/src/main.cpp +++ b/electron/native/wgc-capture/src/main.cpp @@ -607,6 +607,10 @@ int main(int argc, char* argv[]) { MFEncoderOptions encoderOptions{}; encoderOptions.preferSoftwareEncoder = config.preferSoftwareEncoder; encoderOptions.injectDefaultSinkWriterFailureOnce = injectDefaultSinkWriterFailureOnce; + // Keep the CPU path for software encoding and inline webcam PiP. The DXGI + // path is safe when the screen has no CPU-composited webcam frame. + encoderOptions.useDxgiInput = + !config.preferSoftwareEncoder && (!webcamActive || writeSeparateWebcam); MFEncoder encoder; if (!encoder.initialize( @@ -631,6 +635,7 @@ int main(int argc, char* argv[]) { if (writeSeparateWebcam) { MFEncoderOptions webcamEncoderOptions = encoderOptions; webcamEncoderOptions.injectDefaultSinkWriterFailureOnce = false; + webcamEncoderOptions.useDxgiInput = false; const int webcamPixels = std::max(1, webcamCapture.width()) * std::max(1, webcamCapture.height()); const int webcamBitrate = webcamPixels >= 1280 * 720 ? 8'000'000 : 4'000'000; if (!webcamEncoder.initialize( @@ -802,16 +807,18 @@ int main(int argc, char* argv[]) { std::this_thread::sleep_for(std::chrono::milliseconds(testStallReadbackMs)); } if (latestFrameTexture) { - // captureVideoSample performs the GPU readback - // (CopyResource/Map) from latestFrameTexture, which must - // stay serialized (via `mutex`) against the WGC - // frame-arrival callback above, which writes new data - // into the same texture on another thread. - hasVideoSample = encoder.captureVideoSample( - latestFrameTexture.Get(), - frameTimestampHns, - !writeSeparateWebcam && webcamFrame.data ? &webcamFrame : nullptr, - videoSample); + if (encoderOptions.useDxgiInput) { + hasVideoSample = encoder.captureDxgiSample( + latestFrameTexture.Get(), + frameTimestampHns, + videoSample); + } else { + hasVideoSample = encoder.captureVideoSample( + latestFrameTexture.Get(), + frameTimestampHns, + !writeSeparateWebcam && webcamFrame.data ? &webcamFrame : nullptr, + videoSample); + } if (!hasVideoSample) { encodeFailed = true; control.requestStop(); diff --git a/electron/native/wgc-capture/src/mf_encoder.cpp b/electron/native/wgc-capture/src/mf_encoder.cpp index 60f82e9f..383f4416 100644 --- a/electron/native/wgc-capture/src/mf_encoder.cpp +++ b/electron/native/wgc-capture/src/mf_encoder.cpp @@ -2,6 +2,8 @@ #include "audio_sample_utils.h" +#include +#include #include #include #include @@ -134,6 +136,7 @@ enum class SinkWriterCreateStage { SoftwareEncoderRegistration, CreateAttributes, DisableHardwareTransforms, + ConfigureDxgiManager, CreateSinkWriter, }; @@ -179,6 +182,7 @@ HRESULT ensureSoftwareH264EncoderRegisteredForProcess() { HRESULT createSinkWriterFromUrl( const std::wstring& outputPath, bool forceSoftwareEncoder, + IMFDXGIDeviceManager* dxgiDeviceManager, bool injectDefaultSinkWriterFailureOnce, bool& injectedDefaultSinkWriterFailure, Microsoft::WRL::ComPtr& sinkWriter, @@ -218,6 +222,27 @@ HRESULT createSinkWriterFromUrl( failedStage = SinkWriterCreateStage::DisableHardwareTransforms; return hr; } + } else if (dxgiDeviceManager != nullptr) { + HRESULT hr = MFCreateAttributes(&attributes, 3); + if (FAILED(hr)) { + std::cerr << "ERROR: MFCreateAttributes(DXGI sink writer) failed (hr=0x" + << std::hex << hr << std::dec << ")" << std::endl; + failedStage = SinkWriterCreateStage::CreateAttributes; + return hr; + } + hr = attributes->SetUINT32(MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS, TRUE); + if (FAILED(hr)) { + failedStage = SinkWriterCreateStage::ConfigureDxgiManager; + return hr; + } + hr = attributes->SetUnknown(MF_SINK_WRITER_D3D_MANAGER, dxgiDeviceManager); + if (FAILED(hr)) { + std::cerr << "ERROR: Set MF_SINK_WRITER_D3D_MANAGER failed (hr=0x" + << std::hex << hr << std::dec << ")" << std::endl; + failedStage = SinkWriterCreateStage::ConfigureDxgiManager; + return hr; + } + attributes->SetUINT32(MF_LOW_LATENCY, TRUE); } failedStage = SinkWriterCreateStage::CreateSinkWriter; @@ -347,12 +372,34 @@ bool MFEncoder::initialize( fps_ = std::max(1, fps); device_ = device; context_ = context; + captureDevice_ = device; + captureContext_ = context; + useDxgiInput_ = options.useDxgiInput; videoEncoderSelection_ = kVideoEncoderSelectionDefault; if (!succeeded(MFStartup(MF_VERSION), "MFStartup")) { return false; } + if (useDxgiInput_) { + if (!initializeDxgiEncodingDevice()) { + return false; + } + if (!succeeded( + MFCreateDXGIDeviceManager(&dxgiResetToken_, &dxgiDeviceManager_), + "MFCreateDXGIDeviceManager")) { + return false; + } + if (!succeeded( + dxgiDeviceManager_->ResetDevice(device_.Get(), dxgiResetToken_), + "IMFDXGIDeviceManager::ResetDevice")) { + return false; + } + if (!initializeVideoProcessor()) { + return false; + } + } + Microsoft::WRL::ComPtr outputType; if (!succeeded(MFCreateMediaType(&outputType), "MFCreateMediaType(output)")) { return false; @@ -370,13 +417,49 @@ bool MFEncoder::initialize( return false; } inputType->SetGUID(MF_MT_MAJOR_TYPE, MFMediaType_Video); - inputType->SetGUID(MF_MT_SUBTYPE, MFVideoFormat_RGB32); + inputType->SetGUID( + MF_MT_SUBTYPE, + useDxgiInput_ ? MFVideoFormat_NV12 : MFVideoFormat_RGB32); inputType->SetUINT32(MF_MT_INTERLACE_MODE, MFVideoInterlace_Progressive); - inputType->SetUINT32(MF_MT_DEFAULT_STRIDE, static_cast(width_ * 4)); + if (!useDxgiInput_) { + inputType->SetUINT32(MF_MT_DEFAULT_STRIDE, static_cast(width_ * 4)); + } setFrameSize(inputType.Get(), static_cast(width_), static_cast(height_)); setFrameRate(inputType.Get(), static_cast(fps_)); setPixelAspectRatio(inputType.Get()); + if (useDxgiInput_) { + if (!succeeded( + MFCreateVideoSampleAllocatorEx( + __uuidof(IMFVideoSampleAllocatorEx), + reinterpret_cast(videoSampleAllocator_.GetAddressOf())), + "MFCreateVideoSampleAllocatorEx")) { + return false; + } + if (!succeeded( + videoSampleAllocator_->SetDirectXManager(dxgiDeviceManager_.Get()), + "IMFVideoSampleAllocator::SetDirectXManager")) { + return false; + } + Microsoft::WRL::ComPtr allocatorAttributes; + if (!succeeded(MFCreateAttributes(&allocatorAttributes, 2), "MFCreateAttributes(allocator)")) { + return false; + } + allocatorAttributes->SetUINT32(MF_SA_D3D11_USAGE, D3D11_USAGE_DEFAULT); + allocatorAttributes->SetUINT32( + MF_SA_D3D11_BINDFLAGS, + D3D11_BIND_RENDER_TARGET | D3D11_BIND_SHADER_RESOURCE); + if (!succeeded( + videoSampleAllocator_->InitializeSampleAllocatorEx( + 4, + 30, + allocatorAttributes.Get(), + inputType.Get()), + "IMFVideoSampleAllocatorEx::InitializeSampleAllocatorEx")) { + return false; + } + } + bool injectedDefaultSinkWriterFailure = false; auto resetSinkWriterAttempt = [&]() { @@ -397,6 +480,7 @@ bool MFEncoder::initialize( const HRESULT sinkWriterHr = createSinkWriterFromUrl( outputPath, forceSoftwareEncoder, + forceSoftwareEncoder ? nullptr : dxgiDeviceManager_.Get(), options.injectDefaultSinkWriterFailureOnce, injectedDefaultSinkWriterFailure, sinkWriter_, @@ -439,6 +523,11 @@ bool MFEncoder::initialize( }; if (options.preferSoftwareEncoder) { + if (useDxgiInput_) { + std::cerr << "ERROR: DXGI input requires a hardware Media Foundation encoder" + << std::endl; + return false; + } return configureSinkWriterAttempt( true, kVideoEncoderSelectionSoftwarePreferred, @@ -449,6 +538,11 @@ bool MFEncoder::initialize( return true; } + if (useDxgiInput_) { + std::cerr << "ERROR: Hardware DXGI H.264 encoder setup failed" << std::endl; + return false; + } + std::cerr << "WARNING: Default Media Foundation H.264 encoder setup failed; " << "retrying with the Microsoft software H.264 encoder." @@ -603,6 +697,302 @@ bool MFEncoder::copyBgraFrameToBuffer(const BgraFrameView& frame, BYTE* destinat return true; } +bool MFEncoder::initializeDxgiEncodingDevice() { + Microsoft::WRL::ComPtr captureDxgiDevice; + if (!succeeded(captureDevice_.As(&captureDxgiDevice), "Query capture IDXGIDevice")) { + return false; + } + Microsoft::WRL::ComPtr adapter; + if (!succeeded(captureDxgiDevice->GetAdapter(&adapter), "Get capture DXGI adapter")) { + return false; + } + + const UINT flags = D3D11_CREATE_DEVICE_BGRA_SUPPORT | D3D11_CREATE_DEVICE_VIDEO_SUPPORT; + D3D_FEATURE_LEVEL featureLevels[] = { + D3D_FEATURE_LEVEL_11_1, + D3D_FEATURE_LEVEL_11_0, + D3D_FEATURE_LEVEL_10_1, + D3D_FEATURE_LEVEL_10_0, + }; + D3D_FEATURE_LEVEL featureLevel{}; + if (!succeeded( + D3D11CreateDevice( + adapter.Get(), + D3D_DRIVER_TYPE_UNKNOWN, + nullptr, + flags, + featureLevels, + ARRAYSIZE(featureLevels), + D3D11_SDK_VERSION, + &device_, + &featureLevel, + &context_), + "D3D11CreateDevice(encoder)")) { + return false; + } + + Microsoft::WRL::ComPtr multithread; + if (!succeeded(context_.As(&multithread), "Query encoder ID3D10Multithread")) { + return false; + } + multithread->SetMultithreadProtected(TRUE); + return true; +} + +bool MFEncoder::initializeVideoProcessor() { + if (!succeeded(device_.As(&videoDevice_), "Query ID3D11VideoDevice")) { + return false; + } + if (!succeeded(context_.As(&videoContext_), "Query ID3D11VideoContext")) { + return false; + } + + D3D11_VIDEO_PROCESSOR_CONTENT_DESC contentDesc{}; + contentDesc.InputFrameFormat = D3D11_VIDEO_FRAME_FORMAT_PROGRESSIVE; + contentDesc.InputFrameRate = {static_cast(fps_), 1}; + contentDesc.InputWidth = static_cast(width_); + contentDesc.InputHeight = static_cast(height_); + contentDesc.OutputFrameRate = {static_cast(fps_), 1}; + contentDesc.OutputWidth = static_cast(width_); + contentDesc.OutputHeight = static_cast(height_); + contentDesc.Usage = D3D11_VIDEO_USAGE_PLAYBACK_NORMAL; + + if (!succeeded( + videoDevice_->CreateVideoProcessorEnumerator( + &contentDesc, + &videoProcessorEnumerator_), + "CreateVideoProcessorEnumerator")) { + return false; + } + + UINT nv12Support = 0; + if (!succeeded( + videoProcessorEnumerator_->CheckVideoProcessorFormat( + DXGI_FORMAT_NV12, + &nv12Support), + "CheckVideoProcessorFormat(NV12)")) { + return false; + } + if ((nv12Support & D3D11_VIDEO_PROCESSOR_FORMAT_SUPPORT_OUTPUT) == 0) { + std::cerr << "ERROR: D3D11 video processor does not support NV12 output" << std::endl; + return false; + } + + return succeeded( + videoDevice_->CreateVideoProcessor( + videoProcessorEnumerator_.Get(), + 0, + &videoProcessor_), + "CreateVideoProcessor"); +} + +bool MFEncoder::convertBgraTextureToNv12( + ID3D11Texture2D* texture, + ID3D11Texture2D* outputTexture) { + + if (!captureBridgeTexture_) { + D3D11_TEXTURE2D_DESC bridgeDesc{}; + texture->GetDesc(&bridgeDesc); + bridgeDesc.BindFlags = D3D11_BIND_SHADER_RESOURCE | D3D11_BIND_RENDER_TARGET; + bridgeDesc.CPUAccessFlags = 0; + bridgeDesc.Usage = D3D11_USAGE_DEFAULT; + bridgeDesc.MiscFlags = D3D11_RESOURCE_MISC_SHARED_KEYEDMUTEX; + if (!succeeded( + captureDevice_->CreateTexture2D(&bridgeDesc, nullptr, &captureBridgeTexture_), + "CreateTexture2D(capture bridge)")) { + return false; + } + if (!succeeded(captureBridgeTexture_.As(&captureBridgeMutex_), "Query capture bridge mutex")) { + return false; + } + + Microsoft::WRL::ComPtr bridgeResource; + if (!succeeded(captureBridgeTexture_.As(&bridgeResource), "Query capture bridge resource")) { + return false; + } + HANDLE sharedHandle = nullptr; + if (!succeeded(bridgeResource->GetSharedHandle(&sharedHandle), "Get capture bridge handle")) { + return false; + } + if (!succeeded( + device_->OpenSharedResource( + sharedHandle, + __uuidof(ID3D11Texture2D), + reinterpret_cast(encoderBridgeTexture_.GetAddressOf())), + "Open encoder bridge texture")) { + return false; + } + if (!succeeded(encoderBridgeTexture_.As(&encoderBridgeMutex_), "Query encoder bridge mutex")) { + return false; + } + } + + if (!succeeded(captureBridgeMutex_->AcquireSync(0, 5000), "Acquire capture bridge")) { + return false; + } + captureContext_->CopyResource(captureBridgeTexture_.Get(), texture); + if (!succeeded(captureBridgeMutex_->ReleaseSync(1), "Release capture bridge")) { + return false; + } + if (!succeeded(encoderBridgeMutex_->AcquireSync(1, 5000), "Acquire encoder bridge")) { + return false; + } + const auto releaseEncoderBridge = [&]() { + return succeeded(encoderBridgeMutex_->ReleaseSync(0), "Release encoder bridge"); + }; + + D3D11_VIDEO_PROCESSOR_INPUT_VIEW_DESC inputViewDesc{}; + inputViewDesc.FourCC = 0; + inputViewDesc.ViewDimension = D3D11_VPIV_DIMENSION_TEXTURE2D; + inputViewDesc.Texture2D.MipSlice = 0; + inputViewDesc.Texture2D.ArraySlice = 0; + + Microsoft::WRL::ComPtr inputView; + if (!succeeded( + videoDevice_->CreateVideoProcessorInputView( + encoderBridgeTexture_.Get(), + videoProcessorEnumerator_.Get(), + &inputViewDesc, + &inputView), + "CreateVideoProcessorInputView")) { + releaseEncoderBridge(); + return false; + } + + D3D11_VIDEO_PROCESSOR_OUTPUT_VIEW_DESC outputViewDesc{}; + outputViewDesc.ViewDimension = D3D11_VPOV_DIMENSION_TEXTURE2D; + outputViewDesc.Texture2D.MipSlice = 0; + + Microsoft::WRL::ComPtr outputView; + if (!succeeded( + videoDevice_->CreateVideoProcessorOutputView( + outputTexture, + videoProcessorEnumerator_.Get(), + &outputViewDesc, + &outputView), + "CreateVideoProcessorOutputView")) { + releaseEncoderBridge(); + return false; + } + + const RECT sourceRect{0, 0, width_, height_}; + const RECT destinationRect{0, 0, width_, height_}; + videoContext_->VideoProcessorSetOutputTargetRect( + videoProcessor_.Get(), + TRUE, + &destinationRect); + videoContext_->VideoProcessorSetStreamFrameFormat( + videoProcessor_.Get(), + 0, + D3D11_VIDEO_FRAME_FORMAT_PROGRESSIVE); + videoContext_->VideoProcessorSetStreamSourceRect( + videoProcessor_.Get(), + 0, + TRUE, + &sourceRect); + videoContext_->VideoProcessorSetStreamDestRect( + videoProcessor_.Get(), + 0, + TRUE, + &destinationRect); + + D3D11_VIDEO_PROCESSOR_STREAM stream{}; + stream.Enable = TRUE; + stream.OutputIndex = 0; + stream.InputFrameOrField = 0; + stream.PastFrames = 0; + stream.FutureFrames = 0; + stream.pInputSurface = inputView.Get(); + + const bool converted = succeeded( + videoContext_->VideoProcessorBlt( + videoProcessor_.Get(), + outputView.Get(), + 0, + 1, + &stream), + "VideoProcessorBlt"); + const bool released = releaseEncoderBridge(); + return converted && released; +} + +bool MFEncoder::captureDxgiSample( + ID3D11Texture2D* texture, + int64_t timestampHns, + Microsoft::WRL::ComPtr& outSample) { + outSample.Reset(); + if (!texture) { + return false; + } + + D3D11_TEXTURE2D_DESC desc{}; + texture->GetDesc(&desc); + if (desc.Width != static_cast(width_) || + desc.Height != static_cast(height_) || + desc.Format != DXGI_FORMAT_B8G8R8A8_UNORM) { + std::cerr << "ERROR: Unexpected WGC DXGI texture format or dimensions" << std::endl; + return false; + } + + const int64_t sampleDuration = 10'000'000LL / fps_; + int64_t sampleTime = 0; + { + std::scoped_lock writerLock(writerMutex_); + if (!sinkWriter_ || finalized_) { + return false; + } + if (firstTimestampHns_ < 0) { + firstTimestampHns_ = timestampHns; + } + sampleTime = timestampHns - firstTimestampHns_; + if (sampleTime <= lastTimestampHns_) { + sampleTime = lastTimestampHns_ + sampleDuration; + } + lastTimestampHns_ = sampleTime; + } + + Microsoft::WRL::ComPtr sample; + if (!succeeded(videoSampleAllocator_->AllocateSample(&sample), "Allocate DXGI video sample")) { + return false; + } + + Microsoft::WRL::ComPtr buffer; + if (!succeeded(sample->GetBufferByIndex(0, &buffer), "Get DXGI video buffer")) { + return false; + } + + Microsoft::WRL::ComPtr dxgiBuffer; + if (!succeeded(buffer.As(&dxgiBuffer), "Query IMFDXGIBuffer")) { + return false; + } + Microsoft::WRL::ComPtr nv12Texture; + if (!succeeded( + dxgiBuffer->GetResource( + __uuidof(ID3D11Texture2D), + reinterpret_cast(nv12Texture.GetAddressOf())), + "IMFDXGIBuffer::GetResource")) { + return false; + } + if (!convertBgraTextureToNv12(texture, nv12Texture.Get())) { + return false; + } + + DWORD maximumLength = 0; + if (!succeeded(buffer->GetMaxLength(&maximumLength), "IMFMediaBuffer::GetMaxLength(DXGI)")) { + return false; + } + if (!succeeded( + buffer->SetCurrentLength(maximumLength), + "IMFMediaBuffer::SetCurrentLength(DXGI)")) { + return false; + } + + sample->SetSampleTime(sampleTime); + sample->SetSampleDuration(sampleDuration); + outSample = sample; + return true; +} + bool MFEncoder::captureVideoSample( ID3D11Texture2D* texture, int64_t timestampHns, diff --git a/electron/native/wgc-capture/src/mf_encoder.h b/electron/native/wgc-capture/src/mf_encoder.h index e5fbd74c..53995f90 100644 --- a/electron/native/wgc-capture/src/mf_encoder.h +++ b/electron/native/wgc-capture/src/mf_encoder.h @@ -29,6 +29,7 @@ struct AudioInputFormat { struct MFEncoderOptions { bool preferSoftwareEncoder = false; bool injectDefaultSinkWriterFailureOnce = false; + bool useDxgiInput = false; }; constexpr const char* kVideoEncoderSelectionDefault = "default"; @@ -67,6 +68,10 @@ class MFEncoder { int64_t timestampHns, const BgraFrameView* webcamFrame, Microsoft::WRL::ComPtr& outSample); + bool captureDxgiSample( + ID3D11Texture2D* texture, + int64_t timestampHns, + Microsoft::WRL::ComPtr& outSample); bool captureBgraSample( const BgraFrameView& frame, int64_t timestampHns, @@ -77,6 +82,11 @@ class MFEncoder { const char* videoEncoderSelection() const; private: + bool initializeDxgiEncodingDevice(); + bool initializeVideoProcessor(); + bool convertBgraTextureToNv12( + ID3D11Texture2D* texture, + ID3D11Texture2D* outputTexture); bool ensureStagingTexture(ID3D11Texture2D* texture); bool copyFrameToBuffer( ID3D11Texture2D* texture, @@ -89,7 +99,20 @@ class MFEncoder { Microsoft::WRL::ComPtr sinkWriter_; Microsoft::WRL::ComPtr device_; Microsoft::WRL::ComPtr context_; + Microsoft::WRL::ComPtr captureDevice_; + Microsoft::WRL::ComPtr captureContext_; + Microsoft::WRL::ComPtr captureBridgeTexture_; + Microsoft::WRL::ComPtr captureBridgeMutex_; + Microsoft::WRL::ComPtr encoderBridgeTexture_; + Microsoft::WRL::ComPtr encoderBridgeMutex_; Microsoft::WRL::ComPtr stagingTexture_; + Microsoft::WRL::ComPtr dxgiDeviceManager_; + Microsoft::WRL::ComPtr videoSampleAllocator_; + Microsoft::WRL::ComPtr videoDevice_; + Microsoft::WRL::ComPtr videoContext_; + Microsoft::WRL::ComPtr videoProcessorEnumerator_; + Microsoft::WRL::ComPtr videoProcessor_; + UINT dxgiResetToken_ = 0; std::mutex writerMutex_; DWORD videoStreamIndex_ = 0; DWORD audioStreamIndex_ = 0; @@ -100,5 +123,6 @@ class MFEncoder { int64_t firstTimestampHns_ = -1; int64_t lastTimestampHns_ = -1; bool finalized_ = false; + bool useDxgiInput_ = false; const char* videoEncoderSelection_ = kVideoEncoderSelectionDefault; }; diff --git a/electron/native/wgc-capture/src/wgc_session.cpp b/electron/native/wgc-capture/src/wgc_session.cpp index ccab0672..76649a99 100644 --- a/electron/native/wgc-capture/src/wgc_session.cpp +++ b/electron/native/wgc-capture/src/wgc_session.cpp @@ -1,6 +1,7 @@ #include "wgc_session.h" #include +#include #include #include #include @@ -63,7 +64,7 @@ WgcSession::~WgcSession() { } bool WgcSession::createD3DDevice() { - UINT flags = D3D11_CREATE_DEVICE_BGRA_SUPPORT; + UINT flags = D3D11_CREATE_DEVICE_BGRA_SUPPORT | D3D11_CREATE_DEVICE_VIDEO_SUPPORT; #if defined(_DEBUG) flags |= D3D11_CREATE_DEVICE_DEBUG; #endif @@ -109,6 +110,12 @@ bool WgcSession::createD3DDevice() { return false; } + Microsoft::WRL::ComPtr multithread; + if (!succeeded(d3dContext_.As(&multithread), "Query ID3D10Multithread")) { + return false; + } + multithread->SetMultithreadProtected(TRUE); + Microsoft::WRL::ComPtr dxgiDevice; if (!succeeded(d3dDevice_.As(&dxgiDevice), "Query IDXGIDevice")) { return false; From eefe3ad89db0ead241206d81949e8c1621dca01c Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Sat, 8 Aug 2026 12:41:24 +0200 Subject: [PATCH 2/7] fix(wgc): make the GPU encode path degrade instead of failing The DXGI path is the right shape for issue #252: it removes the Map/Unmap readback the reporter's driver wedges inside. What it must not do is become a requirement, because it fixes one machine and every other one still has to record. So every step of it now falls back rather than returning: the encoding device, the NV12 video processor, the shared bridge texture, the sample allocator and the hardware sink writer each drop the whole pipeline and retry the exact chain a machine without a GPU path would have taken. Without that, `useDxgiInput` being the default made the software H.264 fallback unreachable for every recording, and a machine with no hardware encoder went from recording in software to not recording at all. `releaseDxgiPipeline()` puts device_/context_ back on the capture device, because the CPU path's staging texture has to live where the WGC frames do. The choice is no longer knowable from the outside, so callers ask `usesDxgiInput()` and the `encoder-selection` event reports `videoInput`. The bridge acquire was a 5s wait taken on the video-writer thread while it holds the frame lock -- the lock issue #252 is about, measured against an 8s watchdog step budget. It is now a few frame intervals, and a timeout skips the frame instead of ending the recording; the timestamp is stamped after the conversion so a skipped frame no longer stretches the timeline. Frames lost that way are counted and reported once at stop. Measured on a working machine, GPU path against CPU path: - 16.9 Mbps against 1.95 for the same desktop, because the D3D manager switches the sink writer onto a hardware MFT and those default to CBR, spending the full 18 Mbps budget on a static screen. Asking for VBR through ICodecAPI brings it to 2.2. MF_LOW_LATENCY was measured and made no difference, so it is gone. - Colour matches: raw luma 13/222.5/239 against 13/224.3/242, mean rendered RGB 245,240,245 against 246,242,246. The video processor is told full-range BGRA in, studio BT.709 out, and the media types carry the matching tags -- untagged, the driver default is BT.601 and a player reads 1080p as BT.709. - Stop latency 107ms, 0 contended frames over repeated runs, software fallback and preferSoftwareEncoder still land on the CPU path. Co-authored-by: Seb1900 <1712315938@qq.com> --- electron/native/README.md | 4 +- electron/native/wgc-capture/src/main.cpp | 50 +- .../native/wgc-capture/src/mf_encoder.cpp | 434 ++++++++++++------ electron/native/wgc-capture/src/mf_encoder.h | 27 +- .../architecture/recording.md | 3 +- 5 files changed, 363 insertions(+), 155 deletions(-) diff --git a/electron/native/README.md b/electron/native/README.md index 5b38de88..7c9deeb0 100644 --- a/electron/native/README.md +++ b/electron/native/README.md @@ -85,7 +85,9 @@ The current helper implementation supports display/window video capture, system 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. -The helper reports the outcome through the `encoder-selection` stdout event (`video` is `default`, `software-preferred`, or `software-fallback`). 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. +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). + +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=` 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. Encoder diagnostic on final sink-writer failure: when the final `MFCreateSinkWriterFromURL` attempt fails, the helper logs the registered H.264 video encoder MFT count (via `MFTEnumEx`), the registered AAC encoder count when audio was requested, and the hex HRESULT. If no H.264 encoder is registered, it additionally emits the four-bullet actionable error (missing Media Feature Pack / GPU driver registration / empty `HKLM:\SOFTWARE\Microsoft\Windows Media Foundation\Transforms` / reboot). If an H.264 encoder IS registered but the sink writer still failed, it logs a hint pointing at invalid output path, missing MP4 mux, or GPU driver incompatibility. There is still no fail-fast pre-flight gate because `MFTEnumEx` and `MFCreateSinkWriterFromURL` can disagree about which H.264 encoders are available in non-interactive / Session 0 contexts. diff --git a/electron/native/wgc-capture/src/main.cpp b/electron/native/wgc-capture/src/main.cpp index b7878b26..376378f7 100644 --- a/electron/native/wgc-capture/src/main.cpp +++ b/electron/native/wgc-capture/src/main.cpp @@ -607,10 +607,15 @@ int main(int argc, char* argv[]) { MFEncoderOptions encoderOptions{}; encoderOptions.preferSoftwareEncoder = config.preferSoftwareEncoder; encoderOptions.injectDefaultSinkWriterFailureOnce = injectDefaultSinkWriterFailureOnce; - // Keep the CPU path for software encoding and inline webcam PiP. The DXGI - // path is safe when the screen has no CPU-composited webcam frame. + // Keep the CPU path for software encoding and inline webcam PiP: both need + // the frame in system memory, which is the one thing the DXGI path does not + // produce. The env var is the escape hatch for a machine where the GPU path + // misbehaves in a way the encoder's own probes do not catch -- a support + // answer instead of a hotfix. encoderOptions.useDxgiInput = - !config.preferSoftwareEncoder && (!webcamActive || writeSeparateWebcam); + !config.preferSoftwareEncoder && + (!webcamActive || writeSeparateWebcam) && + readEnvInt("OPENSCREEN_WGC_DISABLE_DXGI_INPUT", 0) == 0; MFEncoder encoder; if (!encoder.initialize( @@ -626,8 +631,14 @@ int main(int argc, char* argv[]) { std::cerr << "ERROR: Failed to initialize Media Foundation encoder" << std::endl; return 1; } + // `videoInput` reports what the encoder settled on, not what was asked for: + // it silently degrades to the CPU readback on any machine the GPU path does + // not fit, and a bug report that cannot tell the two apart is a bug report + // about the wrong path. + const bool usesDxgiInput = encoder.usesDxgiInput(); std::cout << "{\"event\":\"encoder-selection\",\"schemaVersion\":2,\"video\":\"" << encoder.videoEncoderSelection() + << "\",\"videoInput\":\"" << (usesDxgiInput ? "dxgi-nv12" : "cpu-rgb32") << "\",\"preferSoftwareEncoder\":" << (config.preferSoftwareEncoder ? "true" : "false") << "}" << std::endl; @@ -657,6 +668,10 @@ int main(int argc, char* argv[]) { CaptureControl control; std::atomic firstFrameWritten = false; std::atomic encodeFailed = false; + // Frames the GPU bridge was too busy to take. Reported at stop rather than + // per frame: a handful over a recording is normal contention, a stream of + // them is the next bug report, and neither is worth a log line each. + std::atomic contendedFrames = 0; Microsoft::WRL::ComPtr latestFrameTexture; int64_t latestFrameTimestampHns = 0; int64_t firstFrameTimestampHns = -1; @@ -807,24 +822,40 @@ int main(int argc, char* argv[]) { std::this_thread::sleep_for(std::chrono::milliseconds(testStallReadbackMs)); } if (latestFrameTexture) { - if (encoderOptions.useDxgiInput) { - hasVideoSample = encoder.captureDxgiSample( + // Both entry points do their GPU work on latestFrameTexture, + // which must stay serialized (via `mutex`) against the WGC + // frame-arrival callback above, which writes new data into + // the same texture on another thread. Which one is live is + // the encoder's answer, not this struct's request: it falls + // back to the CPU path on its own when the GPU path does + // not fit the machine. + bool captured = false; + if (usesDxgiInput) { + captured = encoder.captureDxgiSample( latestFrameTexture.Get(), frameTimestampHns, videoSample); } else { - hasVideoSample = encoder.captureVideoSample( + captured = encoder.captureVideoSample( latestFrameTexture.Get(), frameTimestampHns, !writeSeparateWebcam && webcamFrame.data ? &webcamFrame : nullptr, videoSample); } - if (!hasVideoSample) { + if (!captured) { encodeFailed = true; control.requestStop(); break; } - lastEncodedVideoTimestampHns = frameTimestampHns; + // The DXGI path returns success with no sample when the + // GPU bridge was momentarily busy. That costs one frame, + // which beats ending a recording that is otherwise fine. + hasVideoSample = videoSample != nullptr; + if (hasVideoSample) { + lastEncodedVideoTimestampHns = frameTimestampHns; + } else { + contendedFrames += 1; + } } } @@ -1131,6 +1162,9 @@ int main(int argc, char* argv[]) { beginStopStep("video-writer-join", stepBudgetMs); stopVideoWriter(); logStopStep("video-writer-join"); + if (usesDxgiInput) { + std::cerr << "[frame-drops] gpu_bridge_contended=" << contendedFrames.load() << std::endl; + } // No frame lock here, and the ordering above is what makes that safe rather // than incidental: stopVideoWriter() joined the only thread that calls into // the encoder's GPU readback, and audioMixer->stop() joined the only other diff --git a/electron/native/wgc-capture/src/mf_encoder.cpp b/electron/native/wgc-capture/src/mf_encoder.cpp index 383f4416..d880e366 100644 --- a/electron/native/wgc-capture/src/mf_encoder.cpp +++ b/electron/native/wgc-capture/src/mf_encoder.cpp @@ -2,8 +2,10 @@ #include "audio_sample_utils.h" +#include #include #include +#include #include #include #include @@ -242,7 +244,6 @@ HRESULT createSinkWriterFromUrl( failedStage = SinkWriterCreateStage::ConfigureDxgiManager; return hr; } - attributes->SetUINT32(MF_LOW_LATENCY, TRUE); } failedStage = SinkWriterCreateStage::CreateSinkWriter; @@ -357,6 +358,10 @@ const char* MFEncoder::videoEncoderSelection() const { return videoEncoderSelection_; } +bool MFEncoder::usesDxgiInput() const { + return useDxgiInput_; +} + bool MFEncoder::initialize( const std::wstring& outputPath, int width, @@ -374,30 +379,22 @@ bool MFEncoder::initialize( context_ = context; captureDevice_ = device; captureContext_ = context; - useDxgiInput_ = options.useDxgiInput; + // The injected failure exists to prove the software fallback still works. + // Leaving the GPU path on would make it prove something else: the DXGI + // attempt would eat the injection and the run would land on the plain CPU + // encoder, never reaching the software encoder the knob is aimed at. + useDxgiInput_ = options.useDxgiInput && !options.injectDefaultSinkWriterFailureOnce; videoEncoderSelection_ = kVideoEncoderSelectionDefault; if (!succeeded(MFStartup(MF_VERSION), "MFStartup")) { return false; } - if (useDxgiInput_) { - if (!initializeDxgiEncodingDevice()) { - return false; - } - if (!succeeded( - MFCreateDXGIDeviceManager(&dxgiResetToken_, &dxgiDeviceManager_), - "MFCreateDXGIDeviceManager")) { - return false; - } - if (!succeeded( - dxgiDeviceManager_->ResetDevice(device_.Get(), dxgiResetToken_), - "IMFDXGIDeviceManager::ResetDevice")) { - return false; - } - if (!initializeVideoProcessor()) { - return false; - } + if (useDxgiInput_ && !initializeDxgiPipeline()) { + std::cerr << "WARNING: The GPU DXGI encode path is unavailable on this machine; " + << "using the CPU readback path." << std::endl; + releaseDxgiPipeline(); + useDxgiInput_ = false; } Microsoft::WRL::ComPtr outputType; @@ -416,48 +413,57 @@ bool MFEncoder::initialize( if (!succeeded(MFCreateMediaType(&inputType), "MFCreateMediaType(input)")) { return false; } - inputType->SetGUID(MF_MT_MAJOR_TYPE, MFMediaType_Video); - inputType->SetGUID( - MF_MT_SUBTYPE, - useDxgiInput_ ? MFVideoFormat_NV12 : MFVideoFormat_RGB32); - inputType->SetUINT32(MF_MT_INTERLACE_MODE, MFVideoInterlace_Progressive); - if (!useDxgiInput_) { - inputType->SetUINT32(MF_MT_DEFAULT_STRIDE, static_cast(width_ * 4)); - } - setFrameSize(inputType.Get(), static_cast(width_), static_cast(height_)); - setFrameRate(inputType.Get(), static_cast(fps_)); - setPixelAspectRatio(inputType.Get()); - - if (useDxgiInput_) { - if (!succeeded( - MFCreateVideoSampleAllocatorEx( - __uuidof(IMFVideoSampleAllocatorEx), - reinterpret_cast(videoSampleAllocator_.GetAddressOf())), - "MFCreateVideoSampleAllocatorEx")) { - return false; + // Rebuilt rather than built once, because falling back to the CPU path + // after the sink writer has already refused the NV12 type has to leave a + // type the RGB32 path would have produced from scratch. Every attribute + // one mode sets is deleted by the other; nothing carries over. + auto configureVideoInputType = [&](bool dxgi) { + inputType->SetGUID(MF_MT_MAJOR_TYPE, MFMediaType_Video); + inputType->SetGUID(MF_MT_SUBTYPE, dxgi ? MFVideoFormat_NV12 : MFVideoFormat_RGB32); + inputType->SetUINT32(MF_MT_INTERLACE_MODE, MFVideoInterlace_Progressive); + if (dxgi) { + inputType->DeleteItem(MF_MT_DEFAULT_STRIDE); + // The video processor below converts full-range BGRA into + // studio-range BT.709, so say so. Left untagged, the encoder and + // the player each pick their own default (BT.601 is the common + // one) and the recording comes back with shifted colours the CPU + // path does not have. + inputType->SetUINT32(MF_MT_VIDEO_NOMINAL_RANGE, MFNominalRange_16_235); + inputType->SetUINT32(MF_MT_YUV_MATRIX, MFVideoTransferMatrix_BT709); + } else { + inputType->SetUINT32(MF_MT_DEFAULT_STRIDE, static_cast(width_ * 4)); + inputType->DeleteItem(MF_MT_VIDEO_NOMINAL_RANGE); + inputType->DeleteItem(MF_MT_YUV_MATRIX); } - if (!succeeded( - videoSampleAllocator_->SetDirectXManager(dxgiDeviceManager_.Get()), - "IMFVideoSampleAllocator::SetDirectXManager")) { - return false; - } - Microsoft::WRL::ComPtr allocatorAttributes; - if (!succeeded(MFCreateAttributes(&allocatorAttributes, 2), "MFCreateAttributes(allocator)")) { - return false; - } - allocatorAttributes->SetUINT32(MF_SA_D3D11_USAGE, D3D11_USAGE_DEFAULT); - allocatorAttributes->SetUINT32( - MF_SA_D3D11_BINDFLAGS, - D3D11_BIND_RENDER_TARGET | D3D11_BIND_SHADER_RESOURCE); - if (!succeeded( - videoSampleAllocator_->InitializeSampleAllocatorEx( - 4, - 30, - allocatorAttributes.Get(), - inputType.Get()), - "IMFVideoSampleAllocatorEx::InitializeSampleAllocatorEx")) { - return false; + setFrameSize(inputType.Get(), static_cast(width_), static_cast(height_)); + setFrameRate(inputType.Get(), static_cast(fps_)); + setPixelAspectRatio(inputType.Get()); + }; + + // Carried on the H.264 type as well so the MP4 sink writes the matching + // colour tags instead of leaving players to guess from the frame size. + auto configureOutputColorTags = [&](bool dxgi) { + if (dxgi) { + outputType->SetUINT32(MF_MT_VIDEO_NOMINAL_RANGE, MFNominalRange_16_235); + outputType->SetUINT32(MF_MT_YUV_MATRIX, MFVideoTransferMatrix_BT709); + } else { + outputType->DeleteItem(MF_MT_VIDEO_NOMINAL_RANGE); + outputType->DeleteItem(MF_MT_YUV_MATRIX); } + }; + + // The allocator is the last thing that can refuse the GPU path, and it can + // only be built once the NV12 type exists. Falling back here costs nothing + // but the type rewrite, because no sink writer has been created yet. + configureVideoInputType(useDxgiInput_); + configureOutputColorTags(useDxgiInput_); + if (useDxgiInput_ && !initializeSampleAllocator(inputType.Get())) { + std::cerr << "WARNING: The DXGI sample allocator is unavailable on this machine; " + << "using the CPU readback path." << std::endl; + releaseDxgiPipeline(); + useDxgiInput_ = false; + configureVideoInputType(false); + configureOutputColorTags(false); } bool injectedDefaultSinkWriterFailure = false; @@ -514,6 +520,9 @@ bool MFEncoder::initialize( "SetInputMediaType")) { return false; } + if (useDxgiInput_) { + applyHardwareRateControl(std::max(1, bitrate)); + } if (!succeeded(sinkWriter_->BeginWriting(), "BeginWriting")) { return false; } @@ -523,11 +532,6 @@ bool MFEncoder::initialize( }; if (options.preferSoftwareEncoder) { - if (useDxgiInput_) { - std::cerr << "ERROR: DXGI input requires a hardware Media Foundation encoder" - << std::endl; - return false; - } return configureSinkWriterAttempt( true, kVideoEncoderSelectionSoftwarePreferred, @@ -539,8 +543,22 @@ bool MFEncoder::initialize( } if (useDxgiInput_) { - std::cerr << "ERROR: Hardware DXGI H.264 encoder setup failed" << std::endl; - return false; + // The GPU path exists to dodge a CPU readback, not to be a requirement. + // Drop it and retry the exact chain a machine without it would have + // taken -- the software encoder cannot accept DXGI samples, so without + // this the fallback below would be unreachable for every recording that + // asked for the GPU path, which is all of them by default. + std::cerr + << "WARNING: Hardware DXGI H.264 encoder setup failed; " + << "retrying on the CPU readback path." + << std::endl; + releaseDxgiPipeline(); + useDxgiInput_ = false; + configureVideoInputType(false); + configureOutputColorTags(false); + if (configureSinkWriterAttempt(false, kVideoEncoderSelectionDefault, false)) { + return true; + } } std::cerr @@ -697,6 +715,38 @@ bool MFEncoder::copyBgraFrameToBuffer(const BgraFrameView& frame, BYTE* destinat return true; } +bool MFEncoder::initializeDxgiPipeline() { + return initializeDxgiEncodingDevice() && + succeeded( + MFCreateDXGIDeviceManager(&dxgiResetToken_, &dxgiDeviceManager_), + "MFCreateDXGIDeviceManager") && + succeeded( + dxgiDeviceManager_->ResetDevice(device_.Get(), dxgiResetToken_), + "IMFDXGIDeviceManager::ResetDevice") && + initializeVideoProcessor(); +} + +void MFEncoder::releaseDxgiPipeline() { + bridgeInputView_.Reset(); + encoderBridgeMutex_.Reset(); + encoderBridgeTexture_.Reset(); + captureBridgeMutex_.Reset(); + captureBridgeTexture_.Reset(); + videoProcessor_.Reset(); + videoProcessorEnumerator_.Reset(); + videoContext_.Reset(); + videoDevice_.Reset(); + videoSampleAllocator_.Reset(); + dxgiDeviceManager_.Reset(); + dxgiResetToken_ = 0; + // Put the encoder back on the capture device. initializeDxgiEncodingDevice + // overwrites device_/context_ with the second device it creates, and the + // CPU path's staging texture has to live on the same device the WGC frames + // do or its CopyResource silently does nothing. + device_ = captureDevice_; + context_ = captureContext_; +} + bool MFEncoder::initializeDxgiEncodingDevice() { Microsoft::WRL::ComPtr captureDxgiDevice; if (!succeeded(captureDevice_.As(&captureDxgiDevice), "Query capture IDXGIDevice")) { @@ -778,17 +828,115 @@ bool MFEncoder::initializeVideoProcessor() { return false; } + if (!succeeded( + videoDevice_->CreateVideoProcessor( + videoProcessorEnumerator_.Get(), + 0, + &videoProcessor_), + "CreateVideoProcessor")) { + return false; + } + + // Processor state, not per-blt arguments. Nothing below changes for the + // life of the recording -- the capture size is fixed at initialize() -- + // so setting it once keeps four driver round trips out of every frame. + const RECT frameRect{0, 0, width_, height_}; + videoContext_->VideoProcessorSetOutputTargetRect(videoProcessor_.Get(), TRUE, &frameRect); + videoContext_->VideoProcessorSetStreamFrameFormat( + videoProcessor_.Get(), + 0, + D3D11_VIDEO_FRAME_FORMAT_PROGRESSIVE); + videoContext_->VideoProcessorSetStreamSourceRect(videoProcessor_.Get(), 0, TRUE, &frameRect); + videoContext_->VideoProcessorSetStreamDestRect(videoProcessor_.Get(), 0, TRUE, &frameRect); + + // Screen pixels are full-range sRGB and H.264 in an MP4 is conventionally + // studio-range BT.709. Say both out loud: the driver's default is BT.601 + // limited, which a player then decodes as BT.709 at 1080p and above, and + // the recording comes back with visibly shifted colours. The matching tags + // go on the media types in initialize(). + D3D11_VIDEO_PROCESSOR_COLOR_SPACE inputColorSpace{}; + inputColorSpace.RGB_Range = 0; // full range, 0-255 + inputColorSpace.Nominal_Range = D3D11_VIDEO_PROCESSOR_NOMINAL_RANGE_0_255; + videoContext_->VideoProcessorSetStreamColorSpace(videoProcessor_.Get(), 0, &inputColorSpace); + + D3D11_VIDEO_PROCESSOR_COLOR_SPACE outputColorSpace{}; + outputColorSpace.YCbCr_Matrix = 1; // BT.709 + outputColorSpace.Nominal_Range = D3D11_VIDEO_PROCESSOR_NOMINAL_RANGE_16_235; + videoContext_->VideoProcessorSetOutputColorSpace(videoProcessor_.Get(), &outputColorSpace); + return true; +} + +void MFEncoder::applyHardwareRateControl(int bitrate) { + // The D3D manager switches the sink writer onto a hardware MFT, and those + // default to constant bitrate: a static desktop then spends the full + // configured budget doing nothing, 16.9 Mbps measured against the 1.95 the + // software encoder the CPU path lands on produced for the same screen. Same + // budget, opposite reading of it. Ask for VBR so the GPU path spends what + // the picture costs, which is what users have been getting all along. + // + // Best effort on purpose. An encoder that exposes neither knob still + // produces a valid recording, and a bitrate we could not pin down is not + // worth failing a capture over. + Microsoft::WRL::ComPtr codecApi; + if (FAILED(sinkWriter_->GetServiceForStream( + videoStreamIndex_, + GUID_NULL, + IID_PPV_ARGS(&codecApi)))) { + std::cerr << "WARNING: The hardware H.264 encoder exposes no ICodecAPI; " + << "its default bitrate applies." << std::endl; + return; + } + + VARIANT value{}; + value.vt = VT_UI4; + value.ulVal = eAVEncCommonRateControlMode_UnconstrainedVBR; + if (FAILED(codecApi->SetValue(&CODECAPI_AVEncCommonRateControlMode, &value))) { + std::cerr << "WARNING: Could not select VBR on the hardware H.264 encoder" << std::endl; + } + // Kept as the mean rather than lowered: the configured value is the budget + // for a busy screen, and under VBR a quiet one no longer has to spend it. + value.ulVal = static_cast(bitrate); + if (FAILED(codecApi->SetValue(&CODECAPI_AVEncCommonMeanBitRate, &value))) { + std::cerr << "WARNING: Could not set the hardware H.264 encoder bitrate" << std::endl; + } +} + +bool MFEncoder::initializeSampleAllocator(IMFMediaType* inputType) { + if (!succeeded( + MFCreateVideoSampleAllocatorEx( + __uuidof(IMFVideoSampleAllocatorEx), + reinterpret_cast(videoSampleAllocator_.GetAddressOf())), + "MFCreateVideoSampleAllocatorEx")) { + return false; + } + if (!succeeded( + videoSampleAllocator_->SetDirectXManager(dxgiDeviceManager_.Get()), + "IMFVideoSampleAllocator::SetDirectXManager")) { + return false; + } + Microsoft::WRL::ComPtr allocatorAttributes; + if (!succeeded(MFCreateAttributes(&allocatorAttributes, 2), "MFCreateAttributes(allocator)")) { + return false; + } + allocatorAttributes->SetUINT32(MF_SA_D3D11_USAGE, D3D11_USAGE_DEFAULT); + allocatorAttributes->SetUINT32( + MF_SA_D3D11_BINDFLAGS, + D3D11_BIND_RENDER_TARGET | D3D11_BIND_SHADER_RESOURCE); return succeeded( - videoDevice_->CreateVideoProcessor( - videoProcessorEnumerator_.Get(), - 0, - &videoProcessor_), - "CreateVideoProcessor"); + videoSampleAllocator_->InitializeSampleAllocatorEx(4, 30, allocatorAttributes.Get(), inputType), + "IMFVideoSampleAllocatorEx::InitializeSampleAllocatorEx"); } -bool MFEncoder::convertBgraTextureToNv12( +MFEncoder::Nv12ConvertResult MFEncoder::convertBgraTextureToNv12( ID3D11Texture2D* texture, ID3D11Texture2D* outputTexture) { + // Short on purpose. This runs on the video-writer thread while it holds + // main.cpp's frame lock, and that lock is what issue #252 was about: any + // multi-second wait taken under it is a multi-second wait the shutdown + // watchdog counts against its 8s step budget. A few frame intervals is + // long enough for a busy GPU and short enough that a stuck bridge costs a + // dropped frame instead of the recording. + const DWORD acquireTimeoutMs = static_cast(std::max(50, 4000 / fps_)); if (!captureBridgeTexture_) { D3D11_TEXTURE2D_DESC bridgeDesc{}; @@ -800,19 +948,19 @@ bool MFEncoder::convertBgraTextureToNv12( if (!succeeded( captureDevice_->CreateTexture2D(&bridgeDesc, nullptr, &captureBridgeTexture_), "CreateTexture2D(capture bridge)")) { - return false; + return Nv12ConvertResult::Failed; } if (!succeeded(captureBridgeTexture_.As(&captureBridgeMutex_), "Query capture bridge mutex")) { - return false; + return Nv12ConvertResult::Failed; } Microsoft::WRL::ComPtr bridgeResource; if (!succeeded(captureBridgeTexture_.As(&bridgeResource), "Query capture bridge resource")) { - return false; + return Nv12ConvertResult::Failed; } HANDLE sharedHandle = nullptr; if (!succeeded(bridgeResource->GetSharedHandle(&sharedHandle), "Get capture bridge handle")) { - return false; + return Nv12ConvertResult::Failed; } if (!succeeded( device_->OpenSharedResource( @@ -820,45 +968,54 @@ bool MFEncoder::convertBgraTextureToNv12( __uuidof(ID3D11Texture2D), reinterpret_cast(encoderBridgeTexture_.GetAddressOf())), "Open encoder bridge texture")) { - return false; + return Nv12ConvertResult::Failed; } if (!succeeded(encoderBridgeTexture_.As(&encoderBridgeMutex_), "Query encoder bridge mutex")) { - return false; + return Nv12ConvertResult::Failed; + } + + // The bridge is the only input this processor ever reads, so its view + // is built once here rather than per frame. Views describe a resource, + // they do not read it, so this needs no keyed-mutex ownership. + D3D11_VIDEO_PROCESSOR_INPUT_VIEW_DESC inputViewDesc{}; + inputViewDesc.FourCC = 0; + inputViewDesc.ViewDimension = D3D11_VPIV_DIMENSION_TEXTURE2D; + inputViewDesc.Texture2D.MipSlice = 0; + inputViewDesc.Texture2D.ArraySlice = 0; + if (!succeeded( + videoDevice_->CreateVideoProcessorInputView( + encoderBridgeTexture_.Get(), + videoProcessorEnumerator_.Get(), + &inputViewDesc, + &bridgeInputView_), + "CreateVideoProcessorInputView")) { + return Nv12ConvertResult::Failed; } } - if (!succeeded(captureBridgeMutex_->AcquireSync(0, 5000), "Acquire capture bridge")) { - return false; + // Key 0 is the capture side's, key 1 the encoder's. Timing out here leaves + // key 0 exactly where it was, so the next frame simply tries again; that + // is the whole reason this one is recoverable and the one below is not. + if (FAILED(captureBridgeMutex_->AcquireSync(0, acquireTimeoutMs))) { + return Nv12ConvertResult::Contended; } captureContext_->CopyResource(captureBridgeTexture_.Get(), texture); if (!succeeded(captureBridgeMutex_->ReleaseSync(1), "Release capture bridge")) { - return false; + return Nv12ConvertResult::Failed; } - if (!succeeded(encoderBridgeMutex_->AcquireSync(1, 5000), "Acquire encoder bridge")) { - return false; + // Key 1 was just handed over by this same thread and nothing else in the + // process can hold it, so a failure here means the bridge is broken rather + // than busy, and no later frame could recover it. + if (!succeeded(encoderBridgeMutex_->AcquireSync(1, acquireTimeoutMs), "Acquire encoder bridge")) { + return Nv12ConvertResult::Failed; } const auto releaseEncoderBridge = [&]() { return succeeded(encoderBridgeMutex_->ReleaseSync(0), "Release encoder bridge"); }; - D3D11_VIDEO_PROCESSOR_INPUT_VIEW_DESC inputViewDesc{}; - inputViewDesc.FourCC = 0; - inputViewDesc.ViewDimension = D3D11_VPIV_DIMENSION_TEXTURE2D; - inputViewDesc.Texture2D.MipSlice = 0; - inputViewDesc.Texture2D.ArraySlice = 0; - - Microsoft::WRL::ComPtr inputView; - if (!succeeded( - videoDevice_->CreateVideoProcessorInputView( - encoderBridgeTexture_.Get(), - videoProcessorEnumerator_.Get(), - &inputViewDesc, - &inputView), - "CreateVideoProcessorInputView")) { - releaseEncoderBridge(); - return false; - } - + // Recreated per frame because the allocator hands out a different texture + // from its pool each time. The input view and every processor setting are + // hoisted out; this one call is what is genuinely per-frame. D3D11_VIDEO_PROCESSOR_OUTPUT_VIEW_DESC outputViewDesc{}; outputViewDesc.ViewDimension = D3D11_VPOV_DIMENSION_TEXTURE2D; outputViewDesc.Texture2D.MipSlice = 0; @@ -872,37 +1029,16 @@ bool MFEncoder::convertBgraTextureToNv12( &outputView), "CreateVideoProcessorOutputView")) { releaseEncoderBridge(); - return false; + return Nv12ConvertResult::Failed; } - const RECT sourceRect{0, 0, width_, height_}; - const RECT destinationRect{0, 0, width_, height_}; - videoContext_->VideoProcessorSetOutputTargetRect( - videoProcessor_.Get(), - TRUE, - &destinationRect); - videoContext_->VideoProcessorSetStreamFrameFormat( - videoProcessor_.Get(), - 0, - D3D11_VIDEO_FRAME_FORMAT_PROGRESSIVE); - videoContext_->VideoProcessorSetStreamSourceRect( - videoProcessor_.Get(), - 0, - TRUE, - &sourceRect); - videoContext_->VideoProcessorSetStreamDestRect( - videoProcessor_.Get(), - 0, - TRUE, - &destinationRect); - D3D11_VIDEO_PROCESSOR_STREAM stream{}; stream.Enable = TRUE; stream.OutputIndex = 0; stream.InputFrameOrField = 0; stream.PastFrames = 0; stream.FutureFrames = 0; - stream.pInputSurface = inputView.Get(); + stream.pInputSurface = bridgeInputView_.Get(); const bool converted = succeeded( videoContext_->VideoProcessorBlt( @@ -913,7 +1049,7 @@ bool MFEncoder::convertBgraTextureToNv12( &stream), "VideoProcessorBlt"); const bool released = releaseEncoderBridge(); - return converted && released; + return converted && released ? Nv12ConvertResult::Ok : Nv12ConvertResult::Failed; } bool MFEncoder::captureDxgiSample( @@ -934,23 +1070,6 @@ bool MFEncoder::captureDxgiSample( return false; } - const int64_t sampleDuration = 10'000'000LL / fps_; - int64_t sampleTime = 0; - { - std::scoped_lock writerLock(writerMutex_); - if (!sinkWriter_ || finalized_) { - return false; - } - if (firstTimestampHns_ < 0) { - firstTimestampHns_ = timestampHns; - } - sampleTime = timestampHns - firstTimestampHns_; - if (sampleTime <= lastTimestampHns_) { - sampleTime = lastTimestampHns_ + sampleDuration; - } - lastTimestampHns_ = sampleTime; - } - Microsoft::WRL::ComPtr sample; if (!succeeded(videoSampleAllocator_->AllocateSample(&sample), "Allocate DXGI video sample")) { return false; @@ -973,8 +1092,35 @@ bool MFEncoder::captureDxgiSample( "IMFDXGIBuffer::GetResource")) { return false; } - if (!convertBgraTextureToNv12(texture, nv12Texture.Get())) { - return false; + switch (convertBgraTextureToNv12(texture, nv12Texture.Get())) { + case Nv12ConvertResult::Ok: + break; + case Nv12ConvertResult::Contended: + // No sample, no failure. Leaving outSample empty tells the caller + // to skip this pass. + return true; + case Nv12ConvertResult::Failed: + return false; + } + + // Stamped only once the frame exists. Doing this first, as the CPU path + // does, would let every dropped frame still advance lastTimestampHns_ and + // stretch the timeline by the frames that were never written. + const int64_t sampleDuration = 10'000'000LL / fps_; + int64_t sampleTime = 0; + { + std::scoped_lock writerLock(writerMutex_); + if (!sinkWriter_ || finalized_) { + return false; + } + if (firstTimestampHns_ < 0) { + firstTimestampHns_ = timestampHns; + } + sampleTime = timestampHns - firstTimestampHns_; + if (sampleTime <= lastTimestampHns_) { + sampleTime = lastTimestampHns_ + sampleDuration; + } + lastTimestampHns_ = sampleTime; } DWORD maximumLength = 0; diff --git a/electron/native/wgc-capture/src/mf_encoder.h b/electron/native/wgc-capture/src/mf_encoder.h index 53995f90..689c85cf 100644 --- a/electron/native/wgc-capture/src/mf_encoder.h +++ b/electron/native/wgc-capture/src/mf_encoder.h @@ -29,6 +29,11 @@ struct AudioInputFormat { struct MFEncoderOptions { bool preferSoftwareEncoder = false; bool injectDefaultSinkWriterFailureOnce = false; + // A request, never a requirement. Every step of the GPU path degrades to + // the CPU readback rather than failing the recording, so a machine without + // a hardware H.264 encoder, without NV12 video-processor output, or with a + // driver that refuses shared keyed-mutex textures records exactly as it did + // before the path existed. Ask usesDxgiInput() for what actually happened. bool useDxgiInput = false; }; @@ -80,11 +85,30 @@ class MFEncoder { bool writeAudio(const BYTE* data, DWORD byteCount, int64_t timestampHns, int64_t durationHns); bool finalize(); const char* videoEncoderSelection() const; + // Which video input path initialize() actually settled on, which is not + // necessarily the one that was asked for. Callers must read this rather + // than their own MFEncoderOptions to decide which capture entry point to + // call, or a machine that fell back would be fed DXGI samples the sink + // writer was never configured for. + bool usesDxgiInput() const; private: + // Contended is not Failed: the bridge is a two-key handshake and a missed + // acquire costs one frame, which is a better outcome than ending a + // recording that is otherwise healthy. + enum class Nv12ConvertResult { + Ok, + Contended, + Failed, + }; + + bool initializeDxgiPipeline(); + void releaseDxgiPipeline(); bool initializeDxgiEncodingDevice(); bool initializeVideoProcessor(); - bool convertBgraTextureToNv12( + bool initializeSampleAllocator(IMFMediaType* inputType); + void applyHardwareRateControl(int bitrate); + Nv12ConvertResult convertBgraTextureToNv12( ID3D11Texture2D* texture, ID3D11Texture2D* outputTexture); bool ensureStagingTexture(ID3D11Texture2D* texture); @@ -105,6 +129,7 @@ class MFEncoder { Microsoft::WRL::ComPtr captureBridgeMutex_; Microsoft::WRL::ComPtr encoderBridgeTexture_; Microsoft::WRL::ComPtr encoderBridgeMutex_; + Microsoft::WRL::ComPtr bridgeInputView_; Microsoft::WRL::ComPtr stagingTexture_; Microsoft::WRL::ComPtr dxgiDeviceManager_; Microsoft::WRL::ComPtr videoSampleAllocator_; diff --git a/technical-documentation/architecture/recording.md b/technical-documentation/architecture/recording.md index 01337f21..a56bf451 100644 --- a/technical-documentation/architecture/recording.md +++ b/technical-documentation/architecture/recording.md @@ -68,7 +68,8 @@ Cursor samples are persisted as cursor telemetry rather than baked into editable ## Known gaps - 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`). -- 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 the video writer's `Map(D3D11_MAP_READ)` readback in `mf_encoder.cpp`. 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). Getting the readback out of the lock, and picking the D3D adapter that actually drives the captured monitor instead of adapter 0, are the outstanding fixes (issue #252). +- 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. +- 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). - 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. - 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. - 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. From 976d44d4626f2d845f33da183fb32b84682139c8 Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Sat, 8 Aug 2026 18:32:15 +0200 Subject: [PATCH 3/7] fix(wgc): keep an encode off the frame lock, and name where it wedges #252's reporter confirmed the GPU path fixes display and window capture on the machine that reproduces it, and found two failures left: one consistent with system audio, one intermittent without it. Both report `wgc-quiesce drained=false` then `video-writer-join phase=abandoned`, which means the video writer is stuck while holding the frame lock and every WGC callback is queued behind it. The system-audio one is a lock-order defect, and it predates the GPU path. `writerMutex_` is held across IMFSinkWriter::WriteSample by both submitVideoSample and writeAudio -- a synchronous encode -- while the capture* entry points took that same mutex just to stamp a sample, from inside main.cpp's frame lock. So an audio write on the mixer thread stalls the video writer, the writer stalls the WGC callbacks, and stop finds nothing drainable. The sample clock moves to a `timestampMutex_` of its own, which no blocking call is ever held across, and the sinkWriter_/finalized_ check goes away with it: submitVideoSample already makes that check before writing, so a sample built for a writer that has gone is discarded one step later instead of costing a lock. The intermittent one is not diagnosable from here, so instrument it rather than guess. The encoder now keeps a breadcrumb of the call it is inside, and the shutdown watchdog prints it: `phase=abandoned encode_stage=bridge-copy` says which driver call wedged, where `encode_stage=idle` says the writer never got into the encoder at all. That is the same move that made #252 legible in the first place. Verified by forcing the failure shape locally with OPENSCREEN_WGC_TEST_STALL_READBACK_MS: wgc-quiesce drained=false at 5001ms, video-writer-join abandoned at 8021ms, exit 3, and the breadcrumb correctly reads `idle` for a stall that is outside the encoder. Display, window, system audio, the software fallback knob and the CPU kill switch all still pass. --- electron/native/wgc-capture/src/main.cpp | 6 +- .../native/wgc-capture/src/mf_encoder.cpp | 102 +++++++++--------- electron/native/wgc-capture/src/mf_encoder.h | 16 +++ 3 files changed, 73 insertions(+), 51 deletions(-) diff --git a/electron/native/wgc-capture/src/main.cpp b/electron/native/wgc-capture/src/main.cpp index 376378f7..bc96b150 100644 --- a/electron/native/wgc-capture/src/main.cpp +++ b/electron/native/wgc-capture/src/main.cpp @@ -1118,8 +1118,12 @@ int main(int argc, char* argv[]) { // sleep still be killed. if (stopElapsedMs() >= currentStepDeadlineMs.load() && !shutdownComplete.load()) { const char* step = currentStopStep.load(); + // The encoder stage is what turns "video-writer-join was + // abandoned" into something actionable: it names the call the + // writer thread is sitting in, instead of leaving the next + // report to guess the way issue #252 had to. std::cerr << "[stop-timing] step=" << step << " elapsed_ms=" << stopElapsedMs() - << " phase=abandoned" << std::endl; + << " phase=abandoned encode_stage=" << encoder.encodeStage() << std::endl; std::cout << "{\"event\":\"stop-timeout\",\"schemaVersion\":2,\"step\":\"" << step << "\"}" << std::endl; std::cout.flush(); diff --git a/electron/native/wgc-capture/src/mf_encoder.cpp b/electron/native/wgc-capture/src/mf_encoder.cpp index d880e366..046a63e2 100644 --- a/electron/native/wgc-capture/src/mf_encoder.cpp +++ b/electron/native/wgc-capture/src/mf_encoder.cpp @@ -362,6 +362,39 @@ bool MFEncoder::usesDxgiInput() const { return useDxgiInput_; } +const char* MFEncoder::encodeStage() const { + return encodeStage_.load(); +} + +int64_t MFEncoder::nextSampleTime(int64_t timestampHns, int64_t sampleDuration) { + // On `timestampMutex_` and not `writerMutex_`, deliberately. Every caller + // of this runs under main.cpp's frame lock, and `writerMutex_` is held + // across IMFSinkWriter::WriteSample by both submitVideoSample and + // writeAudio. Taking it here put a synchronous encode inside the frame + // lock: an audio WriteSample would stall the video writer, the video + // writer would stall every WGC callback waiting on that lock, and stop + // would find wgc-quiesce undrained and the writer unjoinable. That is the + // system-audio reproduction in the issue #252 follow-up. Nothing else + // touches these two fields, so they get a lock of their own that no + // blocking call is ever held across. + // + // No sinkWriter_/finalized_ check any more either: that check was the only + // other reason to be on writerMutex_, and submitVideoSample already makes + // it before writing. A sample built for a writer that has since gone away + // is discarded there, which costs one wasted buffer on a path that is + // shutting down anyway. + std::scoped_lock lock(timestampMutex_); + if (firstTimestampHns_ < 0) { + firstTimestampHns_ = timestampHns; + } + int64_t sampleTime = timestampHns - firstTimestampHns_; + if (sampleTime <= lastTimestampHns_) { + sampleTime = lastTimestampHns_ + sampleDuration; + } + lastTimestampHns_ = sampleTime; + return sampleTime; +} + bool MFEncoder::initialize( const std::wstring& outputPath, int width, @@ -996,13 +1029,18 @@ MFEncoder::Nv12ConvertResult MFEncoder::convertBgraTextureToNv12( // Key 0 is the capture side's, key 1 the encoder's. Timing out here leaves // key 0 exactly where it was, so the next frame simply tries again; that // is the whole reason this one is recoverable and the one below is not. + encodeStage_ = "bridge-acquire-capture"; if (FAILED(captureBridgeMutex_->AcquireSync(0, acquireTimeoutMs))) { + encodeStage_ = "idle"; return Nv12ConvertResult::Contended; } + encodeStage_ = "bridge-copy"; captureContext_->CopyResource(captureBridgeTexture_.Get(), texture); + encodeStage_ = "bridge-release-capture"; if (!succeeded(captureBridgeMutex_->ReleaseSync(1), "Release capture bridge")) { return Nv12ConvertResult::Failed; } + encodeStage_ = "bridge-acquire-encoder"; // Key 1 was just handed over by this same thread and nothing else in the // process can hold it, so a failure here means the bridge is broken rather // than busy, and no later frame could recover it. @@ -1021,6 +1059,7 @@ MFEncoder::Nv12ConvertResult MFEncoder::convertBgraTextureToNv12( outputViewDesc.Texture2D.MipSlice = 0; Microsoft::WRL::ComPtr outputView; + encodeStage_ = "output-view"; if (!succeeded( videoDevice_->CreateVideoProcessorOutputView( outputTexture, @@ -1040,6 +1079,7 @@ MFEncoder::Nv12ConvertResult MFEncoder::convertBgraTextureToNv12( stream.FutureFrames = 0; stream.pInputSurface = bridgeInputView_.Get(); + encodeStage_ = "video-processor-blt"; const bool converted = succeeded( videoContext_->VideoProcessorBlt( videoProcessor_.Get(), @@ -1048,7 +1088,9 @@ MFEncoder::Nv12ConvertResult MFEncoder::convertBgraTextureToNv12( 1, &stream), "VideoProcessorBlt"); + encodeStage_ = "bridge-release-encoder"; const bool released = releaseEncoderBridge(); + encodeStage_ = "idle"; return converted && released ? Nv12ConvertResult::Ok : Nv12ConvertResult::Failed; } @@ -1071,7 +1113,9 @@ bool MFEncoder::captureDxgiSample( } Microsoft::WRL::ComPtr sample; + encodeStage_ = "allocate-sample"; if (!succeeded(videoSampleAllocator_->AllocateSample(&sample), "Allocate DXGI video sample")) { + encodeStage_ = "idle"; return false; } @@ -1107,21 +1151,7 @@ bool MFEncoder::captureDxgiSample( // does, would let every dropped frame still advance lastTimestampHns_ and // stretch the timeline by the frames that were never written. const int64_t sampleDuration = 10'000'000LL / fps_; - int64_t sampleTime = 0; - { - std::scoped_lock writerLock(writerMutex_); - if (!sinkWriter_ || finalized_) { - return false; - } - if (firstTimestampHns_ < 0) { - firstTimestampHns_ = timestampHns; - } - sampleTime = timestampHns - firstTimestampHns_; - if (sampleTime <= lastTimestampHns_) { - sampleTime = lastTimestampHns_ + sampleDuration; - } - lastTimestampHns_ = sampleTime; - } + const int64_t sampleTime = nextSampleTime(timestampHns, sampleDuration); DWORD maximumLength = 0; if (!succeeded(buffer->GetMaxLength(&maximumLength), "IMFMediaBuffer::GetMaxLength(DXGI)")) { @@ -1147,23 +1177,7 @@ bool MFEncoder::captureVideoSample( outSample.Reset(); const int64_t sampleDuration = 10'000'000LL / fps_; - int64_t sampleTime = 0; - { - std::scoped_lock writerLock(writerMutex_); - if (!sinkWriter_ || finalized_) { - return false; - } - - if (firstTimestampHns_ < 0) { - firstTimestampHns_ = timestampHns; - } - - sampleTime = timestampHns - firstTimestampHns_; - if (sampleTime <= lastTimestampHns_) { - sampleTime = lastTimestampHns_ + sampleDuration; - } - lastTimestampHns_ = sampleTime; - } + const int64_t sampleTime = nextSampleTime(timestampHns, sampleDuration); // The GPU readback below (copyFrameToBuffer -> CopyResource/Map on // `texture`) is not internally synchronized here. Callers must hold their @@ -1210,23 +1224,7 @@ bool MFEncoder::captureBgraSample( outSample.Reset(); const int64_t sampleDuration = 10'000'000LL / fps_; - int64_t sampleTime = 0; - { - std::scoped_lock writerLock(writerMutex_); - if (!sinkWriter_ || finalized_) { - return false; - } - - if (firstTimestampHns_ < 0) { - firstTimestampHns_ = timestampHns; - } - - sampleTime = timestampHns - firstTimestampHns_; - if (sampleTime <= lastTimestampHns_) { - sampleTime = lastTimestampHns_ + sampleDuration; - } - lastTimestampHns_ = sampleTime; - } + const int64_t sampleTime = nextSampleTime(timestampHns, sampleDuration); Microsoft::WRL::ComPtr buffer; const DWORD frameBytes = static_cast(width_ * height_ * 4); @@ -1270,11 +1268,15 @@ bool MFEncoder::submitVideoSample(IMFSample* sample) { // encode synchronously on the calling thread. Callers must NOT hold any // lock shared with a thread that needs to make timely progress (e.g. a // stop-request check) across this call. + encodeStage_ = "write-sample"; std::scoped_lock writerLock(writerMutex_); if (!sinkWriter_ || finalized_) { + encodeStage_ = "idle"; return false; } - return succeeded(sinkWriter_->WriteSample(videoStreamIndex_, sample), "WriteSample"); + const bool written = succeeded(sinkWriter_->WriteSample(videoStreamIndex_, sample), "WriteSample"); + encodeStage_ = "idle"; + return written; } bool MFEncoder::writeAudio(const BYTE* data, DWORD byteCount, int64_t timestampHns, int64_t durationHns) { diff --git a/electron/native/wgc-capture/src/mf_encoder.h b/electron/native/wgc-capture/src/mf_encoder.h index 689c85cf..fd279700 100644 --- a/electron/native/wgc-capture/src/mf_encoder.h +++ b/electron/native/wgc-capture/src/mf_encoder.h @@ -7,6 +7,7 @@ #include #include +#include #include #include #include @@ -91,6 +92,8 @@ class MFEncoder { // call, or a machine that fell back would be fed DXGI samples the sink // writer was never configured for. bool usesDxgiInput() const; + // A breadcrumb, not state: safe to read from another thread at any time. + const char* encodeStage() const; private: // Contended is not Failed: the bridge is a two-key handshake and a missed @@ -108,6 +111,7 @@ class MFEncoder { bool initializeVideoProcessor(); bool initializeSampleAllocator(IMFMediaType* inputType); void applyHardwareRateControl(int bitrate); + int64_t nextSampleTime(int64_t timestampHns, int64_t sampleDuration); Nv12ConvertResult convertBgraTextureToNv12( ID3D11Texture2D* texture, ID3D11Texture2D* outputTexture); @@ -138,7 +142,19 @@ class MFEncoder { Microsoft::WRL::ComPtr videoProcessorEnumerator_; Microsoft::WRL::ComPtr videoProcessor_; UINT dxgiResetToken_ = 0; + // Guards the sink writer, and is held across IMFSinkWriter::WriteSample -- + // a synchronous encode. Only threads that can afford to wait out an encode + // may take it, which rules out anything holding a caller's frame lock. std::mutex writerMutex_; + // Guards the sample clock alone, so the capture* entry points (which do run + // under a caller's frame lock) never queue behind an encode. Splitting this + // out is what stops an audio WriteSample from wedging the video writer, and + // through it the WGC callbacks, at stop (issue #252 follow-up). + std::mutex timestampMutex_; + // Where the encoder is right now, for the shutdown watchdog to name when a + // step overruns. `video-writer-join phase=abandoned` says which thread is + // stuck; this says which call it is stuck in. + std::atomic encodeStage_{"idle"}; DWORD videoStreamIndex_ = 0; DWORD audioStreamIndex_ = 0; bool hasAudioStream_ = false; From 4d1a0cca0acba6c41989b6b589e52bc96901e06e Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Mon, 10 Aug 2026 18:03:40 +0200 Subject: [PATCH 4/7] fix(wgc): repair the four things the GPU path got wrong under review Four defects CodeRabbit surfaced on #305, each verified against the code before being touched. One of them is worse than it was reported to be. **AcquireSync was tested with the wrong predicate, in both directions.** IDXGIKeyedMutex::AcquireSync reports a timeout as WAIT_TIMEOUT (0x102), a positive HRESULT that passes both SUCCEEDED() and !FAILED(). The capture side tested FAILED(), so a timeout fell straight through to CopyResource without ever holding the key, and only hard errors -- device removed, E_FAIL, WAIT_ABANDONED -- were caught, then misreported as ordinary contention, which skips the frame and retries forever on a bridge that can never work again. The encoder side used succeeded(), so a timeout there read as acquired. Both now test WAIT_TIMEOUT by value. This also means the "Contended frames: 0" figure in the PR description was measured with a counter that could not count: a timeout never reached it. The number says nothing either way and is being remeasured. **Inline webcam PiP would have silently lost its overlay.** useDxgiInput read webcamActive, which is only set once webcam capture starts -- long after the encoder is configured. The condition was therefore dead: always false, always permitting the GPU path. A webcamEnabled recording with no separate output would have run on DXGI, which cannot compose the overlay, and reported success. It now reads config.webcamEnabled, which is final at that point. **The stop breadcrumb named the wrong call.** encodeStage_ was stamped before writerMutex_ was taken, so a video thread queued behind an audio write reported "write-sample" while it was in fact blocked on the lock -- precisely the case the watchdog exists to distinguish, since an audio write is the only other thing that takes that mutex. It is now stamped inside the lock, and writeAudio names its own write instead of being anonymous. Both stamps are serialized by the mutex they sit under. **finalize() left Media Foundation objects for the destructor.** videoSampleAllocator_ and dxgiDeviceManager_ outlived MFShutdown(), and captureDevice_/captureContext_ kept the WGC device alive past the point main.cpp believes session.stop() releases it. finalize() now calls releaseDxgiPipeline() and drops the capture device before MFShutdown(). Not addressed here: bridge-texture creation still happens on the first frame, so a driver that refuses shared keyed-mutex textures fails the recording rather than degrading, which contradicts what three documents claim. That one is a restructure with a real trade-off attached and is being decided separately. Compile-verified in CI only; no hardware smoke test on this machine. --- electron/native/wgc-capture/src/main.cpp | 10 +++- .../native/wgc-capture/src/mf_encoder.cpp | 57 +++++++++++++++++-- 2 files changed, 60 insertions(+), 7 deletions(-) diff --git a/electron/native/wgc-capture/src/main.cpp b/electron/native/wgc-capture/src/main.cpp index bc96b150..dcb57056 100644 --- a/electron/native/wgc-capture/src/main.cpp +++ b/electron/native/wgc-capture/src/main.cpp @@ -612,9 +612,17 @@ int main(int argc, char* argv[]) { // produce. The env var is the escape hatch for a machine where the GPU path // misbehaves in a way the encoder's own probes do not catch -- a support // answer instead of a hotfix. + // + // config.webcamEnabled, not webcamActive: the latter is only set once the + // webcam capture has started, which happens well after this. Reading it + // here made the PiP condition dead code -- always false, so always + // permitting the GPU path -- and an inline-PiP recording would have run on + // DXGI and silently dropped the overlay, reporting success either way. + // config.webcamEnabled is already cleared above when webcam init fails, + // and writeSeparateWebcam is assigned there too, so both are final here. encoderOptions.useDxgiInput = !config.preferSoftwareEncoder && - (!webcamActive || writeSeparateWebcam) && + (!config.webcamEnabled || writeSeparateWebcam) && readEnvInt("OPENSCREEN_WGC_DISABLE_DXGI_INPUT", 0) == 0; MFEncoder encoder; diff --git a/electron/native/wgc-capture/src/mf_encoder.cpp b/electron/native/wgc-capture/src/mf_encoder.cpp index 046a63e2..5abea175 100644 --- a/electron/native/wgc-capture/src/mf_encoder.cpp +++ b/electron/native/wgc-capture/src/mf_encoder.cpp @@ -1029,11 +1029,25 @@ MFEncoder::Nv12ConvertResult MFEncoder::convertBgraTextureToNv12( // Key 0 is the capture side's, key 1 the encoder's. Timing out here leaves // key 0 exactly where it was, so the next frame simply tries again; that // is the whole reason this one is recoverable and the one below is not. + // + // Tested by value, not with FAILED(): AcquireSync reports a timeout as + // WAIT_TIMEOUT (0x102), which is a *positive* HRESULT and therefore passes + // both SUCCEEDED() and !FAILED(). Testing FAILED() alone got this backwards + // in both directions -- a timeout fell through to the CopyResource below + // without ever holding the key, and a hard error (DXGI_ERROR_DEVICE_REMOVED, + // E_FAIL, WAIT_ABANDONED) was reported as ordinary contention, which skips + // the frame and retries forever on a bridge that can never work again. The + // recording then ends successfully with almost no frames in it. encodeStage_ = "bridge-acquire-capture"; - if (FAILED(captureBridgeMutex_->AcquireSync(0, acquireTimeoutMs))) { + const HRESULT captureAcquireHr = captureBridgeMutex_->AcquireSync(0, acquireTimeoutMs); + if (captureAcquireHr == static_cast(WAIT_TIMEOUT)) { encodeStage_ = "idle"; return Nv12ConvertResult::Contended; } + if (!succeeded(captureAcquireHr, "Acquire capture bridge")) { + encodeStage_ = "idle"; + return Nv12ConvertResult::Failed; + } encodeStage_ = "bridge-copy"; captureContext_->CopyResource(captureBridgeTexture_.Get(), texture); encodeStage_ = "bridge-release-capture"; @@ -1043,8 +1057,17 @@ MFEncoder::Nv12ConvertResult MFEncoder::convertBgraTextureToNv12( encodeStage_ = "bridge-acquire-encoder"; // Key 1 was just handed over by this same thread and nothing else in the // process can hold it, so a failure here means the bridge is broken rather - // than busy, and no later frame could recover it. - if (!succeeded(encoderBridgeMutex_->AcquireSync(1, acquireTimeoutMs), "Acquire encoder bridge")) { + // than busy, and no later frame could recover it. A timeout counts as broken + // for that same reason, and needs the same by-value test as above, since + // WAIT_TIMEOUT passes SUCCEEDED() and would otherwise be read as acquired. + const HRESULT encoderAcquireHr = encoderBridgeMutex_->AcquireSync(1, acquireTimeoutMs); + if (encoderAcquireHr == static_cast(WAIT_TIMEOUT)) { + std::cerr << "ERROR: Acquire encoder bridge timed out" << std::endl; + encodeStage_ = "idle"; + return Nv12ConvertResult::Failed; + } + if (!succeeded(encoderAcquireHr, "Acquire encoder bridge")) { + encodeStage_ = "idle"; return Nv12ConvertResult::Failed; } const auto releaseEncoderBridge = [&]() { @@ -1268,12 +1291,16 @@ bool MFEncoder::submitVideoSample(IMFSample* sample) { // encode synchronously on the calling thread. Callers must NOT hold any // lock shared with a thread that needs to make timely progress (e.g. a // stop-request check) across this call. - encodeStage_ = "write-sample"; + // Stamped after the lock, not before it. The breadcrumb is meant to name the + // call the writer is *inside*; setting it first made "write-sample" also mean + // "queued behind writeAudio, which is inside WriteSample" -- the one case the + // watchdog most needs to tell apart, since an audio write is the only other + // thing that takes this mutex. std::scoped_lock writerLock(writerMutex_); if (!sinkWriter_ || finalized_) { - encodeStage_ = "idle"; return false; } + encodeStage_ = "write-sample"; const bool written = succeeded(sinkWriter_->WriteSample(videoStreamIndex_, sample), "WriteSample"); encodeStage_ = "idle"; return written; @@ -1317,7 +1344,14 @@ bool MFEncoder::writeAudio(const BYTE* data, DWORD byteCount, int64_t timestampH sample->SetSampleTime(std::max(0, timestampHns)); sample->SetSampleDuration(durationHns); - return succeeded(sinkWriter_->WriteSample(audioStreamIndex_, sample.Get()), "WriteSample(audio)"); + // Named too, for the same reason the video write is: this is a synchronous + // encode holding writerMutex_, so it is a place the process can be stuck, + // and a watchdog report that only ever names video writes cannot say so. + encodeStage_ = "write-audio"; + const bool written = + succeeded(sinkWriter_->WriteSample(audioStreamIndex_, sample.Get()), "WriteSample(audio)"); + encodeStage_ = "idle"; + return written; } bool MFEncoder::finalize() { @@ -1333,6 +1367,17 @@ bool MFEncoder::finalize() { sinkWriter_.Reset(); } stagingTexture_.Reset(); + // Before MFShutdown(), not left to the destructor. Two of the objects + // releaseDxgiPipeline() drops -- videoSampleAllocator_ and + // dxgiDeviceManager_ -- are Media Foundation objects, and releasing those + // after MFShutdown() has run is not something the platform promises + // anything about. The rest matters to the caller rather than to MF: + // captureDevice_/captureContext_ hold the WGC D3D11 device, so leaving them + // set means session.stop() in main.cpp is no longer dropping the last + // reference to the device it thinks it owns. + releaseDxgiPipeline(); + captureContext_.Reset(); + captureDevice_.Reset(); context_.Reset(); device_.Reset(); MFShutdown(); From 09b267b68c32ce2f79810a0d25b9bed115f8fabb Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Mon, 10 Aug 2026 18:09:44 +0200 Subject: [PATCH 5/7] fix(wgc): build the bridge texture where a failure can still fall back The last of the six review findings, and the one that contradicted the title of this PR. Every other step of the GPU path drops the pipeline and retries the CPU chain when it fails. The shared keyed-mutex bridge did not: it was created lazily on the first frame, by which point initialize() had already configured the sink writer for NV12 and there was no chain left to retry. A driver that refuses D3D11_RESOURCE_MISC_SHARED_KEYEDMUTEX therefore failed the recording outright -- on exactly the class of machine that motivated #252 -- while three documents claimed it would record as if the path had never existed. Bridge creation moves into initializeDxgiPipeline(), where returning false already means "warn, releaseDxgiPipeline(), useDxgiInput_ = false". Those three statements are now true rather than aspirational, so none of them needed rewording. The descriptor is built from width_/height_ instead of a captured frame's, which is equivalent rather than a weakening: captureDxgiSample already rejects any texture whose dimensions or format differ from those same values before convertBgraTextureToNv12 is ever reached, so no frame that could disagree with the descriptor can arrive at the bridge. That guard is what makes pre-sizing safe; without it a mismatch would make CopyResource silently no-op and the recording would come out black instead of failing. convertBgraTextureToNv12 is now only the per-frame path, which is what its comment always claimed it was. Compile-verified in CI only; no hardware smoke test on this machine. --- .../native/wgc-capture/src/mf_encoder.cpp | 130 ++++++++++-------- electron/native/wgc-capture/src/mf_encoder.h | 1 + 2 files changed, 75 insertions(+), 56 deletions(-) diff --git a/electron/native/wgc-capture/src/mf_encoder.cpp b/electron/native/wgc-capture/src/mf_encoder.cpp index 5abea175..66c493ab 100644 --- a/electron/native/wgc-capture/src/mf_encoder.cpp +++ b/electron/native/wgc-capture/src/mf_encoder.cpp @@ -756,7 +756,80 @@ bool MFEncoder::initializeDxgiPipeline() { succeeded( dxgiDeviceManager_->ResetDevice(device_.Get(), dxgiResetToken_), "IMFDXGIDeviceManager::ResetDevice") && - initializeVideoProcessor(); + initializeVideoProcessor() && + initializeBridgeTexture(); +} + +// Built here rather than on the first frame, which is the whole point: this +// runs inside initialize(), where returning false drops the GPU pipeline and +// retries the exact chain a machine without one would have taken. A driver +// that refuses D3D11_RESOURCE_MISC_SHARED_KEYEDMUTEX -- and they exist, which +// is the entire premise of this path being optional -- therefore records on +// the CPU path instead of failing the recording on frame one, long after the +// sink writer has been configured for NV12 and no fallback is possible. +// +// The descriptor comes from width_/height_ rather than from a captured frame, +// which is exactly equivalent: captureDxgiSample rejects any texture whose +// dimensions or format do not match those same values before it ever reaches +// convertBgraTextureToNv12, so no frame that could disagree with this +// descriptor can reach the bridge. +bool MFEncoder::initializeBridgeTexture() { + D3D11_TEXTURE2D_DESC bridgeDesc{}; + bridgeDesc.Width = static_cast(width_); + bridgeDesc.Height = static_cast(height_); + bridgeDesc.MipLevels = 1; + bridgeDesc.ArraySize = 1; + bridgeDesc.Format = DXGI_FORMAT_B8G8R8A8_UNORM; + bridgeDesc.SampleDesc.Count = 1; + bridgeDesc.SampleDesc.Quality = 0; + bridgeDesc.BindFlags = D3D11_BIND_SHADER_RESOURCE | D3D11_BIND_RENDER_TARGET; + bridgeDesc.CPUAccessFlags = 0; + bridgeDesc.Usage = D3D11_USAGE_DEFAULT; + bridgeDesc.MiscFlags = D3D11_RESOURCE_MISC_SHARED_KEYEDMUTEX; + if (!succeeded( + captureDevice_->CreateTexture2D(&bridgeDesc, nullptr, &captureBridgeTexture_), + "CreateTexture2D(capture bridge)")) { + return false; + } + if (!succeeded(captureBridgeTexture_.As(&captureBridgeMutex_), "Query capture bridge mutex")) { + return false; + } + + Microsoft::WRL::ComPtr bridgeResource; + if (!succeeded(captureBridgeTexture_.As(&bridgeResource), "Query capture bridge resource")) { + return false; + } + HANDLE sharedHandle = nullptr; + if (!succeeded(bridgeResource->GetSharedHandle(&sharedHandle), "Get capture bridge handle")) { + return false; + } + if (!succeeded( + device_->OpenSharedResource( + sharedHandle, + __uuidof(ID3D11Texture2D), + reinterpret_cast(encoderBridgeTexture_.GetAddressOf())), + "Open encoder bridge texture")) { + return false; + } + if (!succeeded(encoderBridgeTexture_.As(&encoderBridgeMutex_), "Query encoder bridge mutex")) { + return false; + } + + // The bridge is the only input this processor ever reads, so its view is + // built once here rather than per frame. Views describe a resource, they do + // not read it, so this needs no keyed-mutex ownership. + D3D11_VIDEO_PROCESSOR_INPUT_VIEW_DESC inputViewDesc{}; + inputViewDesc.FourCC = 0; + inputViewDesc.ViewDimension = D3D11_VPIV_DIMENSION_TEXTURE2D; + inputViewDesc.Texture2D.MipSlice = 0; + inputViewDesc.Texture2D.ArraySlice = 0; + return succeeded( + videoDevice_->CreateVideoProcessorInputView( + encoderBridgeTexture_.Get(), + videoProcessorEnumerator_.Get(), + &inputViewDesc, + &bridgeInputView_), + "CreateVideoProcessorInputView"); } void MFEncoder::releaseDxgiPipeline() { @@ -971,61 +1044,6 @@ MFEncoder::Nv12ConvertResult MFEncoder::convertBgraTextureToNv12( // dropped frame instead of the recording. const DWORD acquireTimeoutMs = static_cast(std::max(50, 4000 / fps_)); - if (!captureBridgeTexture_) { - D3D11_TEXTURE2D_DESC bridgeDesc{}; - texture->GetDesc(&bridgeDesc); - bridgeDesc.BindFlags = D3D11_BIND_SHADER_RESOURCE | D3D11_BIND_RENDER_TARGET; - bridgeDesc.CPUAccessFlags = 0; - bridgeDesc.Usage = D3D11_USAGE_DEFAULT; - bridgeDesc.MiscFlags = D3D11_RESOURCE_MISC_SHARED_KEYEDMUTEX; - if (!succeeded( - captureDevice_->CreateTexture2D(&bridgeDesc, nullptr, &captureBridgeTexture_), - "CreateTexture2D(capture bridge)")) { - return Nv12ConvertResult::Failed; - } - if (!succeeded(captureBridgeTexture_.As(&captureBridgeMutex_), "Query capture bridge mutex")) { - return Nv12ConvertResult::Failed; - } - - Microsoft::WRL::ComPtr bridgeResource; - if (!succeeded(captureBridgeTexture_.As(&bridgeResource), "Query capture bridge resource")) { - return Nv12ConvertResult::Failed; - } - HANDLE sharedHandle = nullptr; - if (!succeeded(bridgeResource->GetSharedHandle(&sharedHandle), "Get capture bridge handle")) { - return Nv12ConvertResult::Failed; - } - if (!succeeded( - device_->OpenSharedResource( - sharedHandle, - __uuidof(ID3D11Texture2D), - reinterpret_cast(encoderBridgeTexture_.GetAddressOf())), - "Open encoder bridge texture")) { - return Nv12ConvertResult::Failed; - } - if (!succeeded(encoderBridgeTexture_.As(&encoderBridgeMutex_), "Query encoder bridge mutex")) { - return Nv12ConvertResult::Failed; - } - - // The bridge is the only input this processor ever reads, so its view - // is built once here rather than per frame. Views describe a resource, - // they do not read it, so this needs no keyed-mutex ownership. - D3D11_VIDEO_PROCESSOR_INPUT_VIEW_DESC inputViewDesc{}; - inputViewDesc.FourCC = 0; - inputViewDesc.ViewDimension = D3D11_VPIV_DIMENSION_TEXTURE2D; - inputViewDesc.Texture2D.MipSlice = 0; - inputViewDesc.Texture2D.ArraySlice = 0; - if (!succeeded( - videoDevice_->CreateVideoProcessorInputView( - encoderBridgeTexture_.Get(), - videoProcessorEnumerator_.Get(), - &inputViewDesc, - &bridgeInputView_), - "CreateVideoProcessorInputView")) { - return Nv12ConvertResult::Failed; - } - } - // Key 0 is the capture side's, key 1 the encoder's. Timing out here leaves // key 0 exactly where it was, so the next frame simply tries again; that // is the whole reason this one is recoverable and the one below is not. diff --git a/electron/native/wgc-capture/src/mf_encoder.h b/electron/native/wgc-capture/src/mf_encoder.h index fd279700..372e748b 100644 --- a/electron/native/wgc-capture/src/mf_encoder.h +++ b/electron/native/wgc-capture/src/mf_encoder.h @@ -109,6 +109,7 @@ class MFEncoder { void releaseDxgiPipeline(); bool initializeDxgiEncodingDevice(); bool initializeVideoProcessor(); + bool initializeBridgeTexture(); bool initializeSampleAllocator(IMFMediaType* inputType); void applyHardwareRateControl(int bitrate); int64_t nextSampleTime(int64_t timestampHns, int64_t sampleDuration); From ea44337d87b23b774005fcc6f7bdbd8b8c0843ab Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Mon, 10 Aug 2026 18:42:08 +0200 Subject: [PATCH 6/7] fix(wgc): give the audio write its own breadcrumb instead of the video's An adversarial review of 4d1a0cca caught this, and three independent passes landed on it separately: naming the audio write was right, putting it in encodeStage_ was not. encodeStage_ is a single slot. Almost all of the video thread's stages -- the whole DXGI bridge sequence in convertBgraTextureToNv12 and captureDxgiSample -- are set with no MFEncoder lock held; only submitVideoSample's stamp sits under writerMutex_. So the previous commit's claim that "both stamps are serialized by the mutex they sit under" was true of the two WriteSample stamps and of nothing else. The consequence ran backwards from the intent. writeAudio runs on the audio-mixer thread, which emits roughly every 10 ms and ends each call by storing "idle". A video thread wedged in bridge-copy therefore had its breadcrumb erased within milliseconds, and the watchdog line -- the single piece of evidence this instrumentation exists to produce -- would have printed encode_stage=idle for precisely the hang it was added to identify. Worse on the system-audio configuration than anywhere else, which is the configuration reproducing #252 most consistently. One slot per writing thread. encodeStage_ is single-writer again (the video thread), audioStage_ belongs to the mixer, and the abandoned-step line prints both. A report showing audio_stage=write-audio next to a video stage stuck on a bridge call now says the two are contending for writerMutex_, which is a thing no shared slot could have expressed. Verified at runtime on the previous commit: a 10-minute DXGI recording (16,177 frames, gpu_bridge_contended=0) and a 15-second one both stopped cleanly, so the surrounding path this touches is exercised, not just compiled. --- electron/native/wgc-capture/src/main.cpp | 9 ++++++++- electron/native/wgc-capture/src/mf_encoder.cpp | 17 +++++++++++++++-- electron/native/wgc-capture/src/mf_encoder.h | 8 ++++++++ 3 files changed, 31 insertions(+), 3 deletions(-) diff --git a/electron/native/wgc-capture/src/main.cpp b/electron/native/wgc-capture/src/main.cpp index dcb57056..fe2f8505 100644 --- a/electron/native/wgc-capture/src/main.cpp +++ b/electron/native/wgc-capture/src/main.cpp @@ -1130,8 +1130,15 @@ int main(int argc, char* argv[]) { // abandoned" into something actionable: it names the call the // writer thread is sitting in, instead of leaving the next // report to guess the way issue #252 had to. + // Both threads are named, because either can be the one that is + // stuck and each has its own slot: encode_stage is the video + // writer, audio_stage the mixer. A report showing audio_stage + // parked on write-audio while encode_stage sits at a bridge + // call says the two are fighting over writerMutex_, which no + // single-slot breadcrumb could ever have shown. std::cerr << "[stop-timing] step=" << step << " elapsed_ms=" << stopElapsedMs() - << " phase=abandoned encode_stage=" << encoder.encodeStage() << std::endl; + << " phase=abandoned encode_stage=" << encoder.encodeStage() + << " audio_stage=" << encoder.audioStage() << std::endl; std::cout << "{\"event\":\"stop-timeout\",\"schemaVersion\":2,\"step\":\"" << step << "\"}" << std::endl; std::cout.flush(); diff --git a/electron/native/wgc-capture/src/mf_encoder.cpp b/electron/native/wgc-capture/src/mf_encoder.cpp index 66c493ab..2a9317f4 100644 --- a/electron/native/wgc-capture/src/mf_encoder.cpp +++ b/electron/native/wgc-capture/src/mf_encoder.cpp @@ -366,6 +366,10 @@ const char* MFEncoder::encodeStage() const { return encodeStage_.load(); } +const char* MFEncoder::audioStage() const { + return audioStage_.load(); +} + int64_t MFEncoder::nextSampleTime(int64_t timestampHns, int64_t sampleDuration) { // On `timestampMutex_` and not `writerMutex_`, deliberately. Every caller // of this runs under main.cpp's frame lock, and `writerMutex_` is held @@ -1365,10 +1369,19 @@ bool MFEncoder::writeAudio(const BYTE* data, DWORD byteCount, int64_t timestampH // Named too, for the same reason the video write is: this is a synchronous // encode holding writerMutex_, so it is a place the process can be stuck, // and a watchdog report that only ever names video writes cannot say so. - encodeStage_ = "write-audio"; + // + // Its own slot, not encodeStage_. This runs on the audio-mixer thread, + // which emits roughly every 10 ms, while most of the video thread's stages + // (the whole DXGI bridge sequence) are set outside writerMutex_. Sharing + // one slot meant a video thread wedged in bridge-copy had its breadcrumb + // overwritten with "idle" within milliseconds, so the watchdog reported the + // absence of a stage instead of the call that hung -- the exact opposite of + // what the breadcrumb is for, on the exact configuration (#252 with system + // audio) it was added to diagnose. + audioStage_ = "write-audio"; const bool written = succeeded(sinkWriter_->WriteSample(audioStreamIndex_, sample.Get()), "WriteSample(audio)"); - encodeStage_ = "idle"; + audioStage_ = "idle"; return written; } diff --git a/electron/native/wgc-capture/src/mf_encoder.h b/electron/native/wgc-capture/src/mf_encoder.h index 372e748b..8ef6bea1 100644 --- a/electron/native/wgc-capture/src/mf_encoder.h +++ b/electron/native/wgc-capture/src/mf_encoder.h @@ -93,7 +93,14 @@ class MFEncoder { // writer was never configured for. bool usesDxgiInput() const; // A breadcrumb, not state: safe to read from another thread at any time. + // One slot per writing thread, deliberately. encodeStage() names what the + // video-writer thread is inside; audioStage() names what the audio-mixer + // thread is inside. A single shared slot cannot do both: most of the video + // stages are set outside writerMutex_, so an audio write landing every few + // milliseconds would overwrite a wedged video stage with "idle" and the + // watchdog would report the absence of the very call it is trying to name. const char* encodeStage() const; + const char* audioStage() const; private: // Contended is not Failed: the bridge is a two-key handshake and a missed @@ -156,6 +163,7 @@ class MFEncoder { // step overruns. `video-writer-join phase=abandoned` says which thread is // stuck; this says which call it is stuck in. std::atomic encodeStage_{"idle"}; + std::atomic audioStage_{"idle"}; DWORD videoStreamIndex_ = 0; DWORD audioStreamIndex_ = 0; bool hasAudioStream_ = false; From a084f6662773b6937e10eae2c48f68713c00f7b2 Mon Sep 17 00:00:00 2001 From: Etienne Lescot Date: Mon, 10 Aug 2026 18:55:11 +0200 Subject: [PATCH 7/7] fix(wgc): clear the breadcrumb by scope, not by remembering to Two remaining review findings, both about the same thing: a diagnostic is only worth having if it cannot lie. encodeStage_ was cleared by hand on each return, and the paths that forgot -- GetBufferByIndex, buffer.As, GetResource in captureDxgiSample; the bridge-release-capture and output-view returns in convertBgraTextureToNv12 -- left it naming a call the writer had already left. The watchdog would then report a stage the process was not in, which is worse than reporting nothing, because the next #252 report would be read as evidence. A StageGuard clears it on scope exit instead, in both functions, and the six manual resets it subsumes are gone. submitVideoSample keeps its pair: after the lock there is no early return between setting the stage and clearing it. Also logs the HRESULT when MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS(TRUE) fails. Its two neighbours already log theirs, and this one sits on the path a machine takes when the GPU pipeline is being set up -- exactly the machines whose logs are currently all we have to go on. Not doing the other half of that finding: the stage it reports is ConfigureDxgiManager rather than a value naming hardware transforms. Nothing reads the enum except the CreateSinkWriter comparison at mf_encoder.cpp:550, so a new enumerator would be ceremony. The log line carries the attribute name and the HRESULT, which is what a reader actually needs. --- .../native/wgc-capture/src/mf_encoder.cpp | 24 ++++++++++++++----- 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/electron/native/wgc-capture/src/mf_encoder.cpp b/electron/native/wgc-capture/src/mf_encoder.cpp index 2a9317f4..9fee5a7c 100644 --- a/electron/native/wgc-capture/src/mf_encoder.cpp +++ b/electron/native/wgc-capture/src/mf_encoder.cpp @@ -234,6 +234,8 @@ HRESULT createSinkWriterFromUrl( } hr = attributes->SetUINT32(MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS, TRUE); if (FAILED(hr)) { + std::cerr << "ERROR: Set MF_READWRITE_ENABLE_HARDWARE_TRANSFORMS(TRUE) failed (hr=0x" + << std::hex << hr << std::dec << ")" << std::endl; failedStage = SinkWriterCreateStage::ConfigureDxgiManager; return hr; } @@ -348,6 +350,18 @@ void compositeWebcam(BYTE* destination, int width, int height, const BgraFrameVi } } +// Clears a breadcrumb on every exit from the scope it guards, which the manual +// resets could not: each of these functions has a dozen failure returns, and +// every one that forgot to clear left the watchdog naming a call the writer had +// already left. Naming the wrong call is worse than naming none -- the whole +// point of the breadcrumb is that the next #252 report does not have to guess. +struct StageGuard { + std::atomic& stage; + ~StageGuard() { + stage = "idle"; + } +}; + } // namespace MFEncoder::~MFEncoder() { @@ -1047,6 +1061,7 @@ MFEncoder::Nv12ConvertResult MFEncoder::convertBgraTextureToNv12( // long enough for a busy GPU and short enough that a stuck bridge costs a // dropped frame instead of the recording. const DWORD acquireTimeoutMs = static_cast(std::max(50, 4000 / fps_)); + const StageGuard stageGuard{encodeStage_}; // Key 0 is the capture side's, key 1 the encoder's. Timing out here leaves // key 0 exactly where it was, so the next frame simply tries again; that @@ -1063,11 +1078,9 @@ MFEncoder::Nv12ConvertResult MFEncoder::convertBgraTextureToNv12( encodeStage_ = "bridge-acquire-capture"; const HRESULT captureAcquireHr = captureBridgeMutex_->AcquireSync(0, acquireTimeoutMs); if (captureAcquireHr == static_cast(WAIT_TIMEOUT)) { - encodeStage_ = "idle"; return Nv12ConvertResult::Contended; } if (!succeeded(captureAcquireHr, "Acquire capture bridge")) { - encodeStage_ = "idle"; return Nv12ConvertResult::Failed; } encodeStage_ = "bridge-copy"; @@ -1085,11 +1098,9 @@ MFEncoder::Nv12ConvertResult MFEncoder::convertBgraTextureToNv12( const HRESULT encoderAcquireHr = encoderBridgeMutex_->AcquireSync(1, acquireTimeoutMs); if (encoderAcquireHr == static_cast(WAIT_TIMEOUT)) { std::cerr << "ERROR: Acquire encoder bridge timed out" << std::endl; - encodeStage_ = "idle"; return Nv12ConvertResult::Failed; } if (!succeeded(encoderAcquireHr, "Acquire encoder bridge")) { - encodeStage_ = "idle"; return Nv12ConvertResult::Failed; } const auto releaseEncoderBridge = [&]() { @@ -1135,7 +1146,6 @@ MFEncoder::Nv12ConvertResult MFEncoder::convertBgraTextureToNv12( "VideoProcessorBlt"); encodeStage_ = "bridge-release-encoder"; const bool released = releaseEncoderBridge(); - encodeStage_ = "idle"; return converted && released ? Nv12ConvertResult::Ok : Nv12ConvertResult::Failed; } @@ -1156,11 +1166,13 @@ bool MFEncoder::captureDxgiSample( std::cerr << "ERROR: Unexpected WGC DXGI texture format or dimensions" << std::endl; return false; } + // Declared after the two early returns above, which run before any stage is + // set and so have nothing to clear. + const StageGuard stageGuard{encodeStage_}; Microsoft::WRL::ComPtr sample; encodeStage_ = "allocate-sample"; if (!succeeded(videoSampleAllocator_->AllocateSample(&sample), "Allocate DXGI video sample")) { - encodeStage_ = "idle"; return false; }