Repository navigation
Conversation
…cale Instant and studio fragmented M4S paths stage D3D readback ahead of encode, run pixel conversion through AsyncConverterPool when needed, scale mismatched capture sizes on the D3D11 video processor before readback, and align Windows instant max capture size with encode resolution to avoid redundant swscale.
|
Someone is attempting to deploy a commit to the Cap Software Inc Team on Vercel. A member of the Team first needs to authorize it. |
| use tokio_util::{future::FutureExt as _, sync::CancellationToken}; | ||
| use tracing::*; | ||
|
|
||
| mod gpu_scale; |
There was a problem hiding this comment.
Windows cannot find the module
mod gpu_scale; looks for screen_capture/windows/gpu_scale.rs or screen_capture/windows/gpu_scale/mod.rs, but the new file is at screen_capture/gpu_scale.rs. Every Windows build fails with a missing-module error. Move the file under windows/, declare it in the parent module, or add an explicit path.
| mod gpu_scale; | |
| #[path = "gpu_scale.rs"] | |
| mod gpu_scale; |
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/sources/screen_capture/windows.rs
Line: 32
Comment:
**Windows cannot find the module**
`mod gpu_scale;` looks for `screen_capture/windows/gpu_scale.rs` or `screen_capture/windows/gpu_scale/mod.rs`, but the new file is at `screen_capture/gpu_scale.rs`. Every Windows build fails with a missing-module error. Move the file under `windows/`, declare it in the parent module, or add an explicit path.
```suggestion
#[path = "gpu_scale.rs"]
mod gpu_scale;
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| PastFrames: 0, | ||
| FutureFrames: 0, | ||
| ppPastSurfaces: ptr::null_mut(), | ||
| pInputSurface: std::mem::transmute_copy(&input_view), |
There was a problem hiding this comment.
Unsafe call blocks compilation
scale_texture calls std::mem::transmute_copy outside an unsafe block. Once the module path is fixed, this causes another Windows compile error. Wrap the call in an explicit unsafe block or construct pInputSurface without the transmute.
| pInputSurface: std::mem::transmute_copy(&input_view), | |
| pInputSurface: unsafe { std::mem::transmute_copy(&input_view) }, |
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/sources/screen_capture/gpu_scale.rs
Line: 158
Comment:
**Unsafe call blocks compilation**
`scale_texture` calls `std::mem::transmute_copy` outside an `unsafe` block. Once the module path is fixed, this causes another Windows compile error. Wrap the call in an explicit `unsafe` block or construct `pInputSurface` without the transmute.
```suggestion
pInputSurface: unsafe { std::mem::transmute_copy(&input_view) },
```
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| while let Some(converted) = pool.try_recv() { | ||
| let timestamp = Duration::from_micros(converted.frame.pts().unwrap_or(0).max(0) as u64); | ||
| let encode_start = std::time::Instant::now(); | ||
| match encoder.lock() { | ||
| Ok(mut enc) => { | ||
| if let Err(error) = enc.queue_preconverted_frame(converted.frame, timestamp) { |
There was a problem hiding this comment.
Recorded frames can go backward
The pool uses two to four independent workers, so a delayed worker can return an older frame after a newer one. drain_converted encodes results immediately and ignores converted.sequence. update_pts moves backward timestamps forward rather than restoring frame order, so recordings can show older screen content after newer content with incorrect timing. Preserve capture order, or discard stale results, in both this drain and the final drain.
Knowledge Base Used: Media encoding and muxing
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/output_pipeline/win_fragmented_m4s.rs
Line: 1138-1143
Comment:
**Recorded frames can go backward**
The pool uses two to four independent workers, so a delayed worker can return an older frame after a newer one. `drain_converted` encodes results immediately and ignores `converted.sequence`. `update_pts` moves backward timestamps forward rather than restoring frame order, so recordings can show older screen content after newer content with incorrect timing. Preserve capture order, or discard stale results, in both this drain and the final drain.
**Knowledge Base Used:** [Media encoding and muxing](https://app.greptile.com/cap/-/custom-context/knowledge-base/capsoftware/cap/-/docs/media-encoding-and-muxing.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| AsyncConverterPool::from_config(conversion_config, pool_config) | ||
| .map_err(|error| anyhow!("Failed to create Windows M4S converter pool: {error}"))?, |
There was a problem hiding this comment.
Software recordings change colors
The external conversion path drops the encoder's BT.709 color setup. With libx264, the required output is YUV420P, which the D3D11 converter does not support. The pool therefore falls back to SwscaleConverter, which uses swscale's default BT.601 coefficients, while H.264 still labels the video BT.709. This shifts recorded colors compared with the previous path. Apply the same BT.709 limited-range setup to external RGB-to-YUV conversion.
Knowledge Base Used: Media encoding and muxing
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/output_pipeline/win_fragmented_m4s.rs
Line: 1107-1108
Comment:
**Software recordings change colors**
The external conversion path drops the encoder's BT.709 color setup. With `libx264`, the required output is `YUV420P`, which the D3D11 converter does not support. The pool therefore falls back to `SwscaleConverter`, which uses swscale's default BT.601 coefficients, while H.264 still labels the video BT.709. This shifts recorded colors compared with the previous path. Apply the same BT.709 limited-range setup to external RGB-to-YUV conversion.
**Knowledge Base Used:** [Media encoding and muxing](https://app.greptile.com/cap/-/custom-context/knowledge-base/capsoftware/cap/-/docs/media-encoding-and-muxing.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| self.target_width, | ||
| self.target_height, | ||
| ); | ||
| self.gpu_scaler_source = self.gpu_scaler.as_ref().map(|_| (src_width, src_height)); |
There was a problem hiding this comment.
When try_new fails, gpu_scaler_source stays None. Every scaled frame then repeats device checks and processor creation before taking the CPU fallback. This adds repeated work on unsupported devices, especially now that capped Instant captures can need scaling throughout the recording. Cache the attempted source size even when creation fails, and retry only after the size or device changes.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/sources/screen_capture/windows.rs
Line: 231
Comment:
**Failed GPU setup repeats**
When `try_new` fails, `gpu_scaler_source` stays `None`. Every scaled frame then repeats device checks and processor creation before taking the CPU fallback. This adds repeated work on unsupported devices, especially now that capped Instant captures can need scaling throughout the recording. Cache the attempted source size even when creation fails, and retry only after the size or device changes.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Point the nested gpu_scale module at the sibling source file and wrap transmute_copy in an unsafe block for Rust 2024.
Preserve capture sequence through the async converter pool, apply BT.709 limited-range coefficients for RGB swscale fallbacks, and cache failed GPU scaler setup per source size.
|
@greptile-apps please re-review. Addressed the open P1/P2 items in the latest push on this branch. |
| pending_converted.insert(converted.sequence, converted); | ||
| while let Some(converted) = pending_converted.remove(&next_encode_sequence) { | ||
| encode_converted(converted)?; | ||
| } |
There was a problem hiding this comment.
Recording stops after one frame
next_encode_sequence never advances, so the default external-conversion path encodes only sequence 0. Every later converted frame stays in pending_converted until shutdown, which also checks only sequence 0. Recordings lose nearly all video while memory keeps growing.
Advance the sequence after encoding and handle missing results without waiting forever: AsyncConverterPool can drop submitted frames when its queues fill.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/output_pipeline/win_fragmented_m4s.rs
Line: 1168-1171
Comment:
**Recording stops after one frame**
`next_encode_sequence` never advances, so the default external-conversion path encodes only sequence 0. Every later converted frame stays in `pending_converted` until shutdown, which also checks only sequence 0. Recordings lose nearly all video while memory keeps growing.
Advance the sequence after encoding and handle missing results without waiting forever: `AsyncConverterPool` can drop submitted frames when its queues fill.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.4e93812 to
e077503
Compare
…fter runtime failure
|
@greptile-apps review |
…rness Hyprland 0.57 dropped hyprland.conf and only reads hyprland.lua, so the harness's monitor mode, session export and waybar were ignored and the output came up at vkms's preferred 1024x768. The screenshot scenario then reported screenshot_size=1024x768 instead of yes. Write the same settings as Lua and use the Lua form of the focus dispatcher.
…mera job The ubuntu-24.04 runner's 7.0.0-1017-azure kernel has no linux-modules-extra package, so apt-get failed before the camera test ran. That kernel ships videodev in its base modules (the desktop workflow's X11 camera step loads v4l2loopback there with only the headers), so install the extra modules only when videodev is missing and the package exists, install the headers v4l2loopback-dkms builds against, and fail clearly when videodev still cannot load.
…set bitrate At 80 kbps the paravirtualised VideoToolbox HEVC encoder on the CI macOS VMs occasionally left a blurred camera code cell at margin 0.17, below the reader's 0.18 threshold. 138,240 bps is the Web export preset (0.08 bpp), still below the Social preset the verify exports use.
|
@greptile-apps review |
|
@greptile-apps review |
Summary
prefer_external_conversionand feed frames throughAsyncConverterPool(D3D11 video processor when available) withqueue_preconverted_frame_to, overlapping GPU/CPU conversion with readback and libx264/NVENC work.VideoInfomatches the encode resolution instead of capturing native 4K and scaling twice (capture scaler + encoder swscale).VideoProcessorBltfor dimension mismatches and read back only at the target resolution; fall back to the existing CPU swscale path when the GPU scaler cannot be created.Not in this PR (follow-up)
WindowsMuxerinto fragmented M4S (zero-copy texture encode for studio/instant segments).Test plan
cargo check -p cap-recording(CI).

Confidence Score: 5/5
The PR appears safe to merge, with all previous findings addressed and no new actionable issues found.Summary
This PR overlaps Windows frame readback, conversion, and encoding, and adds GPU scaling with a CPU fallback.
Reviews (5) · Last reviewed commit: "fix(ci): walk GIF blocks when measuring ..." · Reviewed by Greptile