fix: update Windows API imports for windows crate 0.60.0 compatibility #1431
Conversation
WalkthroughUpdates Direct3D11 API bindings to newer Windows SDK paths with improved texture creation, view mapping, and error handling. Refactors camera feed buffer handling and screenshot capture logic with enhanced memory safety. Adds explicit unsafe blocks and error propagation throughout Direct3D and GDI interop paths. Marks NativeCameraFrame as Send/Sync and adds Win32 Xps storage feature to dependencies. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
crates/frame-converter/src/d3d11.rs (1)
544-584: Redundant nestedunsafeblocks inside anunsafe fn.The
copy_frame_to_mappedfunction is already declaredunsafe, so the innerunsafeblocks on lines 551, 561, 573 are redundant. They don't cause issues but add unnecessary nesting.unsafe fn copy_frame_to_mapped(frame: &frame::Video, dst: *mut u8, dst_stride: usize) { let height = frame.height() as usize; let format = frame.format(); match format { Pixel::NV12 => { for y in 0..height { - unsafe { - ptr::copy_nonoverlapping( - frame.data(0).as_ptr().add(y * frame.stride(0)), - dst.add(y * dst_stride), - frame.width() as usize, - ); - } + ptr::copy_nonoverlapping( + frame.data(0).as_ptr().add(y * frame.stride(0)), + dst.add(y * dst_stride), + frame.width() as usize, + ); } // ... similar for other blocks } // ... } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
crates/frame-converter/src/d3d11.rs(14 hunks)crates/recording/Cargo.toml(1 hunks)crates/recording/src/feeds/camera.rs(1 hunks)crates/recording/src/output_pipeline/win.rs(7 hunks)crates/recording/src/screenshot.rs(11 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx,js,jsx,rs,py,sh}
📄 CodeRabbit inference engine (CLAUDE.md)
Never add any form of comments to code (single-line //, multi-line /* /, documentation ///, //!, /* */, or any other comment syntax). Code must be self-explanatory through naming, types, and structure.
Files:
crates/recording/src/output_pipeline/win.rscrates/recording/src/screenshot.rscrates/recording/src/feeds/camera.rscrates/frame-converter/src/d3d11.rs
**/*.rs
📄 CodeRabbit inference engine (CLAUDE.md)
Use Rust 2024 edition and run cargo fmt to format all Rust code with rustfmt
**/*.rs: Format all Rust code usingrustfmtand apply workspace clippy lints
Use snake_case for Rust module names and kebab-case for Rust crate names
Files:
crates/recording/src/output_pipeline/win.rscrates/recording/src/screenshot.rscrates/recording/src/feeds/camera.rscrates/frame-converter/src/d3d11.rs
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx,rs}: Use kebab-case for file names (e.g.,user-menu.tsx)
Never add comments to code in any language (no//,/* */,///,//!,#, etc.). Code must be self-explanatory through naming, types, and structure
Files:
crates/recording/src/output_pipeline/win.rscrates/recording/src/screenshot.rscrates/recording/src/feeds/camera.rscrates/frame-converter/src/d3d11.rs
🧠 Learnings (2)
📚 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/recording/src/output_pipeline/win.rscrates/recording/src/feeds/camera.rscrates/frame-converter/src/d3d11.rs
📚 Learning: 2025-10-17T05:58:22.586Z
Learnt from: Brendonovich
Repo: CapSoftware/Cap PR: 1219
File: crates/enc-avfoundation/src/mp4.rs:350-373
Timestamp: 2025-10-17T05:58:22.586Z
Learning: In crates/enc-avfoundation/src/mp4.rs, the `finish()` method intentionally skips video extension when `is_paused` is true. This is correct behavior because if recording is paused, the video should not be extended beyond the pause point—the pause is user-initiated, unlike the case where ScreenCaptureKit stops providing frames during static content.
Applied to files:
crates/recording/src/output_pipeline/win.rscrates/recording/src/feeds/camera.rs
🧬 Code graph analysis (2)
crates/recording/src/feeds/camera.rs (2)
crates/camera-windows/src/lib.rs (1)
bytes(188-199)crates/camera-mediafoundation/src/lib.rs (1)
bytes(468-483)
crates/frame-converter/src/d3d11.rs (2)
crates/frame-converter/src/lib.rs (1)
new(94-110)crates/scap-direct3d/src/lib.rs (1)
texture(429-431)
🔇 Additional comments (18)
crates/recording/Cargo.toml (1)
77-77: LGTM!The
Win32_Storage_Xpsfeature is correctly added to support the relocatedPrintWindowAPI in windows crate 0.60.0.crates/frame-converter/src/d3d11.rs (6)
4-8: LGTM!Import additions for
ManuallyDropand the restructured Windows API types are correct for windows crate 0.60.0 compatibility.
155-165: LGTM!The
HMODULE::default()correctly replaces the previousNoneparameter for the software module handle inD3D11CreateDevice.
245-283: LGTM!The texture creation calls correctly adapt to the new API by extracting the raw
u32value from the flag types (.0 as u32).
378-392: LGTM!The input view creation now uses an explicit
Optionout-parameter with a null check, improving robustness over the previous API.
420-432: LGTM!The
D3D11_VIDEO_PROCESSOR_STREAMinitialization correctly usesManuallyDrop::new(None)for the optionalpInputSurfaceRightfield to match the new API's expectations for optional COM pointers.
515-541: LGTM!The
create_texturefunction correctly adapts to the new API withu32fields for flags and adds explicit null checking for the created texture.crates/recording/src/feeds/camera.rs (1)
565-587: LGTM!The buffer handling refactor improves control flow by:
- Acquiring the lock in a scoped block that ensures it's released before subsequent operations
- Using a
buffer_readyflag to gate frame sending on successful buffer population- Only calling
SetCurrentLengthand sending the frame when the buffer is properly initializedcrates/recording/src/output_pipeline/win.rs (4)
569-571: LGTM!The closure is correctly changed to
mutbecause it captures and mutatesfirst_timestampandframe_countacross invocations. This fixes the borrow checker error mentioned in the PR description.
674-676: LGTM!The error message improvement provides clearer context when the video channel is closed.
410-412: Verify UYVY422 to DXGI_FORMAT_YUY2 mapping correctness.YUYV422 and UYVY422 have different byte orderings:
- YUYV: Y0 U0 Y1 V0
- UYVY: U0 Y0 V0 Y1
Mapping both to
DXGI_FORMAT_YUY2(which expects YUYV ordering) may produce incorrect colors for UYVY422 input.#!/bin/bash # Check if there's any color conversion or if UYVY is actually handled differently elsewhere rg -n "UYVY" --type rust -C 5
397-398: The Send/Sync implementation is appropriate given the Mutex wrapper, but verify the buffer was initialized for thread-safe use.IMFMediaBuffer is not internally synchronized—Microsoft documentation confirms that
Lock/Unlockdo not provide thread synchronization. However, theArc<Mutex<IMFMediaBuffer>>pattern correctly implements external synchronization, which is the recommended approach per Microsoft best practices.The
unsafe impl Send + Syncis sound because theMutexserializes all access to the buffer. Verify thatIMFMediaBufferinstances are created with a threading model compatible with multi-threaded access (typically free-threaded or properly marshaled), and that all buffer access occurs through the mutex-protected wrapper.crates/recording/src/screenshot.rs (6)
38-38: LGTM!The
PrintWindowandPRINT_WINDOW_FLAGSimports are correctly updated to use the new location inWin32::Storage::Xpsas required by windows crate 0.60.0.
344-350: LGTM!The HDC null checks correctly use
.0.is_null()which is the appropriate pattern for checking null handles in the windows crate.
360-360: LGTM!Setting
biCompressionto0is semantically equivalent toBI_RGB(which has value 0) and aligns with the simplified API.
371-381: LGTM!The
CreateDIBSectioncall correctly handles the newResultreturn type and performs proper null checks on both the bitmap handle and data pointer before proceeding.
483-494: LGTM!The
PrintWindowcall correctly usesPRINT_WINDOW_FLAGS(2)as a replacement forPW_RENDERFULLCONTENTwhich was relocated to a different module in windows crate 0.60.0. The value2corresponds toPW_RENDERFULLCONTENT.
614-614: LGTM!The simplified result handling using
res.ok()?.ok()?correctly unwraps the nestedResulttypes.
2 participants
This PR fixes Windows build failures in the
cap-recordingcrate that occurred after updating towindowscrate v0.60.0.Thewindowscrate v0.60.0 introduced breaking changes by reorganizing Win32 API bindings. Specifically, thePrintWindowfunction and its associated types were relocated fromWin32::Graphics::GditoWin32::Storage::Xps. Additionally, thePW_RENDERFULLCONTENTconstant was moved toWin32::UI::WindowsAndMessaging.These changes caused unresolved import errors when building the recording crate on Windows, preventing the desktop app from compiling. This PR updates the import paths and fixes a related closure mutability issue to restore Windows build compatibility.Changes
PrintWindowimports inscreenshot.rs: ThePrintWindowfunction andPRINT_WINDOW_FLAGStype moved fromWin32::Graphics::GditoWin32::Storage::Xpsin windows crate 0.60.0PW_RENDERFULLCONTENT: This constant moved toWin32::UI::WindowsAndMessaging, so replaced withPRINT_WINDOW_FLAGS(2)directly since it's only used onceoutput_pipeline/win.rs: Addedmuttoprocess_frameclosure to fix borrow checker errorWhy
The
windowscrate reorganized some Win32 API bindings in v0.60.0.PrintWindowwas moved from the Gdi module to the Xps module (it's defined inuser32.dllbut the crate relocated the binding). Without these changes, the recording crate fails to compile on Windows.Testing
cargo check -p cap-recordingpassescargo build -p cap-recordingcompletes successfullySummary by CodeRabbit
Bug Fixes
Chores
✏️ Tip: You can customize this high-level summary in your review settings.