fix: window memory leak - memory increase while recording

[phuocithcmus]

Summary by CodeRabbit

• Refactor
  • Optimized memory management in video encoding processing to improve efficiency and stability.

[coderabbitai]

Walkthrough

Memory management optimization in H.264 video encoding MediaFoundation binding. The code now directly consumes samples from output buffers instead of cloning, reducing unnecessary allocations while maintaining identical control flow and error handling semantics.

Changes

Cohort / File(s)	Summary
Memory Management Refactor crates/enc-mediafoundation/src/video/h264.rs	Eliminated sample cloning in METransformHaveOutput handling by directly extracting pSample from the output buffer and consuming it in place; preserved error path when no sample is produced; added ownership clarification comment

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

• Specific areas requiring attention:
  • Verify that direct pSample consumption does not bypass required cleanup or reference counting in the MediaFoundation API
  • Confirm error path behavior remains equivalent when the output buffer contains no sample

Poem

🐰 A sample once cloned, now flows free,
Through buffers direct, no copy spree!
Memory dances lighter still,
H.264 hops with better will. 🎬

Pre-merge checks and finishing touches

❌ 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%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately reflects the main change: fixing a memory leak in the H.264 video encoder by optimizing METransformHaveOutput memory handling to prevent cloning samples.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fdec142 and 4a41f99.

📒 Files selected for processing (1)

• crates/enc-mediafoundation/src/video/h264.rs (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.rs

📄 CodeRabbit inference engine (AGENTS.md)

**/*.rs: Format Rust code using rustfmt and ensure all Rust code passes workspace-level clippy lints.
Rust modules should be named with snake_case, and crate directories should be in kebab-case.

Files:

• crates/enc-mediafoundation/src/video/h264.rs

crates/*/src/**/*

📄 CodeRabbit inference engine (AGENTS.md)

Rust crates should place tests within the src/ and/or a sibling tests/ directory for each crate inside crates/*.

Files:

• crates/enc-mediafoundation/src/video/h264.rs

🧠 Learnings (1)

📚 Learning: 2025-10-28T08:39:42.230Z

Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1305
File: crates/recording/src/output_pipeline/macos.rs:80-90
Timestamp: 2025-10-28T08:39:42.230Z
Learning: In `crates/recording/src/output_pipeline/macos.rs`, the `AVFoundationMp4Muxer` intentionally holds the `Mutex<MP4Encoder>` lock during retry attempts in `send_video_frame()` and `send_audio_frame()`. This blocking behavior is correct because frame processing must happen sequentially to prevent audio and video frames from being interleaved incorrectly in the encoder.

Applied to files:

• crates/enc-mediafoundation/src/video/h264.rs

🔇 Additional comments (1)

crates/enc-mediafoundation/src/video/h264.rs (1)

441-444: Excellent memory management fix!

Using take() instead of cloning the sample is the correct approach to prevent memory leaks. This transfers ownership from the output buffer and avoids incrementing COM reference counts unnecessarily, which directly addresses the streaming memory leak described in the PR.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Use a proper error HRESULT instead of 0.

HRESULT(0) represents S_OK (success), which is semantically incorrect for an error condition. Consider using a standard error code like E_FAIL or E_UNEXPECTED to properly indicate failure.

Apply this diff:

-                                return Err(
-                                    windows::core::Error::new(
-                                        windows::core::HRESULT(0),
-                                        "Too many consecutive empty samples"
-                                    )
-                                );
+                                return Err(windows::core::Error::new(
+                                    windows::Win32::Foundation::E_FAIL,
+                                    "Too many consecutive empty samples",
+                                ));

🤖 Prompt for AI Agents

In crates/enc-mediafoundation/src/video/h264.rs around lines 448 to 453, the
code constructs an error using windows::core::HRESULT(0) (which is S_OK).
Replace the zero HRESULT with a proper failure code (for example
windows::Win32::Foundation::E_FAIL or E_UNEXPECTED) when creating the
windows::core::Error; update imports if necessary so the chosen HRESULT constant
is available and use that constant when constructing the Error to correctly
indicate failure.

[richiemcilroy]

Thanks for this!!