Fix corrupted MP4 recordings on Windows

[PALConverteo]

Summary

• Fix a thread-safety issue in the native Windows capture encoder (wgc-capture) that causes corrupted/unplayable MP4 files
• The Direct3D11CaptureFramePool::CreateFreeThreaded frame pool dispatches FrameArrived callbacks on thread pool threads concurrently, but MFEncoder::writeFrame() accesses non-thread-safe resources (ID3D11DeviceContext, IMFSinkWriter, shared staging texture and NV12 buffer) without synchronization
• At 60fps, when frame processing takes longer than ~16.7ms (GPU map stall + CPU BGRA→NV12 conversion), callbacks overlap and corrupt the MP4 container
• Add a std::mutex to serialize writeFrame() and finalize() calls

Test plan

• Record a 30s–1min video on Windows using native capture
• Verify the resulting MP4 opens in VLC and other players
• Record multiple videos in succession to confirm consistency
• Test with system audio and microphone enabled (muxing path)

Fixes #96

Summary by CodeRabbit

• Bug Fixes
  • Improved thread safety in the encoder to prevent potential concurrent access issues during frame writing and finalization operations.

[coderabbitai]

Warning

.coderabbit.yaml has a parsing error

The CodeRabbit configuration file in this repository has a parsing error and default settings were used instead. Please fix the error(s) in the configuration file. You can initialize chat with CodeRabbit to get help with the configuration file.

💥 Parsing errors (1)

Validation error: Invalid regex pattern for base branch. Received: "*" at "reviews.auto_review.base_branches[0]"

⚙️ Configuration instructions

• Please see the configuration documentation for more information.
• You can also validate your configuration using the online YAML validator.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

📝 Walkthrough

Walkthrough

The MFEncoder class is enhanced with thread synchronization by introducing a private std::mutex member and protecting frame writing and finalization operations with std::lock_guard to prevent concurrent access to shared write operations.

Changes

Cohort / File(s)	Summary
Thread Synchronization Header electron/native/wgc-capture/src/mf_encoder.h	Added #include <mutex> and introduced private std::mutex writeMutex_ member for synchronization.
Thread Synchronization Implementation electron/native/wgc-capture/src/mf_encoder.cpp	Wrapped MFEncoder::writeFrame() and MFEncoder::finalize() methods with std::lock_guard to acquire writeMutex_, serializing access to frame-copy/mapping/conversion operations and sink writer finalization.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

---

🐰 A mutex stands guard with care,
Threads no longer interfere,
Locks protect each precious frame,
Safe and sound now is the game!
✨ Synchronization's gentle dance,
Gives our data one more chance! 🎬

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main fix: addressing corrupted MP4 recordings on Windows through thread-safety improvements to the capture encoder.
Description check	✅ Passed	The description provides comprehensive context including root cause analysis, solution overview, and a detailed test plan covering the linked issue requirements.
Linked Issues check	✅ Passed	The PR correctly addresses issue #96 by fixing the thread-safety bug causing corrupted MP4 files, which resolves the 'Failed to load video' error by ensuring recorded files are valid and playable.
Out of Scope Changes check	✅ Passed	All changes are scoped to the thread-safety fix: adding mutex synchronization to MFEncoder's writeFrame() and finalize() methods. No unrelated modifications are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

electron/native/wgc-capture/src/mf_encoder.cpp (1)

127-128: Lock initialize() with writeMutex_ to close a race window in lifecycle state initialization.

initialize() modifies initialized_, sinkWriter_, stagingTexture_, and nv12Buffer_ without acquiring writeMutex_, while writeFrame() and finalize() protect their checks with the same lock. Although the current call sequence (single init → many frames → single finalize) avoids collision in practice, the asymmetric locking leaves a window where writeFrame() could observe partially-initialized state if the threading model changes. Adding a lock guard at the start of initialize() ensures all state transitions are synchronized.

Proposed patch

 bool MFEncoder::initialize(const std::wstring& outputPath, int width, int height, int fps,
                            ID3D11Device* device, ID3D11DeviceContext* context) {
+    std::lock_guard<std::mutex> lock(writeMutex_);
     if (initialized_) return false;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@electron/native/wgc-capture/src/mf_encoder.cpp` around lines 127 - 128, The
initialize() method currently mutates initialized_, sinkWriter_,
stagingTexture_, and nv12Buffer_ without holding writeMutex_, leaving a race
against writeFrame() and finalize(); fix this by acquiring the same lock at the
start of initialize() (e.g., add a std::lock_guard<std::mutex>
lock(writeMutex_); as the first statement in initialize()) so all state changes
to initialized_, sinkWriter_, stagingTexture_, and nv12Buffer_ are synchronized
with writeFrame() and finalize().

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@electron/native/wgc-capture/src/mf_encoder.cpp`:
- Around line 127-128: The initialize() method currently mutates initialized_,
sinkWriter_, stagingTexture_, and nv12Buffer_ without holding writeMutex_,
leaving a race against writeFrame() and finalize(); fix this by acquiring the
same lock at the start of initialize() (e.g., add a std::lock_guard<std::mutex>
lock(writeMutex_); as the first statement in initialize()) so all state changes
to initialized_, sinkWriter_, stagingTexture_, and nv12Buffer_ are synchronized
with writeFrame() and finalize().

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 832a336a-3b26-4070-b95e-6b910f8f0dcb

📥 Commits

Reviewing files that changed from the base of the PR and between 6e69da0 and 906c476.

📒 Files selected for processing (2)

• electron/native/wgc-capture/src/mf_encoder.cpp
• electron/native/wgc-capture/src/mf_encoder.h