Repository navigation
Conversation
Use capture fps for non-fragmented studio muxing, reuse NV12 readback buffers, downscale camera preview on Win/Linux, deepen D3D capture pools, and feed UYVY camera frames through without swizzle copies.
|
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. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| } else { | ||
| original_data |
There was a problem hiding this comment.
upload_mf_buffer_to_texture now uploads raw UYVY422 bytes, but NativeCameraFrame::dxgi_format() still labels them DXGI_FORMAT_YUY2. These formats put brightness and color bytes in different positions. A Windows recording using a UYVY webcam and the Media Foundation encoder will save corrupted brightness and colors. Keep the byte conversion for this texture path, even when the FFmpeg path accepts UYVY directly.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/output_pipeline/win.rs
Line: 1749-1750
Comment:
**Camera colors are corrupted**
`upload_mf_buffer_to_texture` now uploads raw `UYVY422` bytes, but `NativeCameraFrame::dxgi_format()` still labels them `DXGI_FORMAT_YUY2`. These formats put brightness and color bytes in different positions. A Windows recording using a UYVY webcam and the Media Foundation encoder will save corrupted brightness and colors. Keep the byte conversion for this texture path, even when the FFmpeg path accepts UYVY directly.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| let stride = ffmpeg_frame.stride(0); | ||
| copy_plane( | ||
| final_data, | ||
| data, |
There was a problem hiding this comment.
camera_frame_to_ffmpeg now copies the original UYVY buffer, but its UYVY branch still uses width * 2 as the source stride. That was correct only after the removed conversion packed the rows. If a webcam adds padding between rows, each row after the first starts at the wrong byte, corrupting the recorded image. Use layout.primary_stride for the source stride and size check.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/output_pipeline/win.rs
Line: 1371
Comment:
**Padded camera rows are corrupted**
`camera_frame_to_ffmpeg` now copies the original UYVY buffer, but its UYVY branch still uses `width * 2` as the source stride. That was correct only after the removed conversion packed the rows. If a webcam adds padding between rows, each row after the first starts at the wrong byte, corrupting the recorded image. Use `layout.primary_stride` for the source stride and size check.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.|
|
||
| let mut ffmpeg_frame = ffmpeg::frame::Video::new(final_format, frame.width, frame.height); | ||
| let flip = frame.is_bottom_up && frame.pixel_format != cap_camera_windows::PixelFormat::UYVY422; | ||
| let mut ffmpeg_frame = ffmpeg::frame::Video::new(ffmpeg_format, frame.width, frame.height); |
There was a problem hiding this comment.
camera_frame_to_ffmpeg now returns UYVY422 frames, but WindowsFragmentedM4SCameraMuxer::normalize_video_config_for_frame still configures UYVY cameras as YUYV422. The encoder builds its scaler from that configured format. The AVX2 converter hides the mismatch for same-size frames, but resizing or running without AVX2 falls through to a scaler that expects the wrong input, causing conversion to fail. Update the fragmented camera configuration to match the returned frame format.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/recording/src/output_pipeline/win.rs
Line: 1299
Comment:
**Camera frames fail conversion**
`camera_frame_to_ffmpeg` now returns `UYVY422` frames, but `WindowsFragmentedM4SCameraMuxer::normalize_video_config_for_frame` still configures UYVY cameras as `YUYV422`. The encoder builds its scaler from that configured format. The AVX2 converter hides the mismatch for same-size frames, but resizing or running without AVX2 falls through to a scaler that expects the wrong input, causing conversion to fail. Update the fragmented camera configuration to match the returned frame format.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| let mut data = self | ||
| .nv12_byte_pool | ||
| .pop() | ||
| .unwrap_or_else(|| Vec::with_capacity(copy_len)); |
There was a problem hiding this comment.
nv12_byte_pool starts empty and is never replenished. Its only uses are construction and pop(), while each resulting buffer leaves through Arc::new(data). Every frame therefore still allocates a fresh byte buffer. Add a return path that waits until consumers release the buffer, or remove the unused pool.
Prompt To Fix With AI
This is a comment left during a code review.
Path: crates/video-decode/src/media_foundation.rs
Line: 780-783
Comment:
**Decoder buffers are not reused**
`nv12_byte_pool` starts empty and is never replenished. Its only uses are construction and `pop()`, while each resulting buffer leaves through `Arc::new(data)`. Every frame therefore still allocates a fresh byte buffer. Add a return path that waits until consumers release the buffer, or remove the unused pool.
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.| .fail("Requested camera blur is unavailable".to_string()); | ||
| } | ||
| let max_dims = blur_mode.map(|_| crate::camera_blur_portable::MAX_DIMS); | ||
| let max_dims = Some(crate::camera_blur_portable::MAX_DIMS); |
There was a problem hiding this comment.
This cap also reduces the camera image saved in Linux Instant recordings, not just the preview. The worker publishes the same capped image through recording.publish. A 1080p webcam now supplies at most 640×360 even with blur off. If the recording uses a larger camera overlay, it must enlarge that reduced image and loses detail. Keep the preview cap separate from recording, or size recording frames for the actual overlay.
Knowledge Base Used: GPUI desktop client
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/src/feeds.rs
Line: 565
Comment:
**Recorded camera image loses detail**
This cap also reduces the camera image saved in Linux Instant recordings, not just the preview. The worker publishes the same capped image through `recording.publish`. A 1080p webcam now supplies at most 640×360 even with blur off. If the recording uses a larger camera overlay, it must enlarge that reduced image and loses detail. Keep the preview cap separate from recording, or size recording frames for the actual overlay.
**Knowledge Base Used:** [GPUI desktop client](https://app.greptile.com/cap/-/custom-context/knowledge-base/capsoftware/cap/-/docs/gpui-desktop-client.md)
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.Re-swizzle UYVY to YUY2 for Media Foundation texture uploads while keeping FFmpeg on native UYVY with padded stride handling.
Cap preview downscale to blur bubble only, match fragmented camera muxer to UYVY422 frames, and drop the non-functional NV12 byte pool.
|
@greptile-apps please re-review. Addressed the open P1/P2 items in the latest push on this branch. |
clippy::uninit_vec (deny) rejects with_capacity + set_len in the Media Foundation NV12 readback, which failed the Windows clippy job in cap-video-decode before cap-recording was reached. Copy the mapped bytes with to_vec (allocation plus memcpy, no zero fill) and take map_staging back to &self now that the byte pool is gone.
31f68df to
7e6b3c3
Compare
|
@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.
…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
Test plan
cargo check -p cap-recording -p cap-video-decode(Windows CI).
The PR appears safe to merge; all previous findings are addressed and no new blocking issue was found.Summary
This PR reduces copying in Windows recording paths and removes the fixed 30 fps limit from regular Studio output.
Reviews (5) · Last reviewed commit: "fix(ci): walk GIF blocks when measuring ..." · Reviewed by Greptile