Repository navigation
Conversation
|
@Addin is attempting to deploy a commit to the Cap Software Inc Team on Vercel. A member of the Team first needs to authorize it. |
4c8420c to
a463d72
Compare
|
Addressed the two current-head Greptile findings:
The other three comments were left on the pre-rebase stacked head and reference the Windows readback commits ( |
|
@greptileai review |
|
@greptileai review |
|
Hi @richiemcilroy can I have a review on this, or should this be closed? |
|
Thanks! Can you resolve the conflicts with main? Then we'll take a look. |
|
@greptileai review |
|
@richiemcilroy This is ready, review is 5/5. |
|
Thanks! Can you rebase on main to clear the conflicts? Then we'll take a look. |
2f4b8bf to
9a8e002
Compare
|
Rebased onto current Conflict notes: combined main's MJPEG scaled-IDCT work with the frame-conversion/stats refactor in @greptileai review |
Wrap Media Foundation buffers and CVPixelBuffer planes as refcounted AVFrames instead of allocating and copying each frame. Keep native Media Foundation buffers through the feed, run MF and DirectShow sample callbacks on bounded worker queues, and reuse an IOSurface-backed CVPixelBufferPool for VideoToolbox output. Add conversion counters and a synthetic 1080p frame-copy benchmark.
Use the same Media Foundation lock type for every consumer of a capture buffer, including the hardware upload and preview copy paths, and cache the last converted frame for paced repeats. Revert the macOS camera-ffmpeg zero-copy wrapper so preview and ffmpeg consumers copy instead of holding CVPixelBuffer locks for an AVFrame lifetime. Fix the VideoToolbox pool attribute compile error, share DirectShow media types across the callback worker, initialize that worker as MTA, and expose callback drop counters. Restore NV21 to NV12 conversion in as_ffmpeg and log conversion statistics periodically.
Wrap each MF buffer in a single AVBufferRef with stack plane metadata, cutting the zero-copy path from 0.002 to 0.001 ms/frame. Replace the per-frame Arc<Mutex<IMFMediaBuffer>> with a direct COM-backed CameraBuffer. Cache the D3D11 upload texture and immediate context and use UpdateSubresource instead of CreateTexture2D per frame: 0.733 vs 0.956 ms/frame for 1080p NV12. Upload straight into the hardware encoder's video-processor input texture to skip the extra CopyResource, while still running VideoProcessorBlt for conversion and scaling. Add camera-upload and zero-copy/preview-copy benchmarks.
Gate the Windows-only CameraBuffer import in feeds/camera.rs; the unconditional import broke macOS and Linux builds of cap-recording. Extend camera-upload-bench with the direct encoder-input path and a cached-upload-plus-CopyResource comparison. At 1080p NV12 the direct path is 1.053 ms/frame versus 1.359 ms/frame for the two-step path; cached and uncached uploads are 0.700 and 0.912 ms/frame.
Infer the stride of contiguous camera buffers from the buffer length so padded DirectShow/MF memory buffers keep the correct chroma offset; add a padded NV12 regression test. Use cv::PixelFormat::to_cf_number() for the VideoToolbox pool attributes to match the other pool builders and keep the macOS converter building.
9a8e002 to
a269dee
Compare
|
The GPUI Desktop build jobs were failing because the separate Verified with |
Summary
This branch reduces per-frame CPU/GPU work in the camera capture path and fixes capture/format issues found while profiling.
Windows / Media Foundation
AVBufferRefwith stack plane metadata instead of oneVec/Arcper plane.Arc<Mutex<IMFMediaBuffer>>with a direct COM-backedCameraBuffer; every consumer uses a consistent 2D/contiguous lock policy.UpdateSubresourceinstead of creating a texture per frame.CopyResource;VideoProcessorBltstill performs conversion/scaling.CameraBufferimport gate.macOS / VideoToolbox
CVPixelBufferPoolfor VideoToolbox output.cv::PixelFormat::to_cf_number()for the pool attributes.CVPixelBufferRefalias so macOS Clippy stays warning-free.Benchmarks
Added
frame-copy-benchandcamera-upload-bench.1080p NV12, release, median of 3 runs:
Direct encoder-input upload versus the old two-step path: 1.359 → 0.884 ms/frame, 35.0% lower latency, 1.54× speedup. Upload timings vary by roughly ±20% run-to-run on this host; the table uses medians, with the best cached upload at 0.700 ms and best direct upload at 0.818 ms.
Test plan
cargo clippy --workspace --exclude cap-desktop --all-features --locked -- -D warnings(Windows/MSVC) ΓÇö passed.cargo fmt --all --checkΓÇö passed.scripts/test-recording-reliability.pyandscripts/test-capbench.pyΓÇö passed (18 and 17 tests, with expected skips).CAP_EXPORT_FORCE_SOFTWARE_ENCODER=1; the local NVIDIA NVENC path fails 3 segment-count tests, which is hardware-encoder-specific and not caused by this branch.Notes
CameraBufferimport is nowcfg(windows)-gated; macOS CI will validate the build.--all-targetsclippy (not the CI clippy command) still reports two pre-existing test-only lints incap-exportandcap-recording; neither is touched by this branch.

Confidence Score: 5/5
The reviewed changes appear safe to merge; no new actionable issue was established.Summary
This PR reduces camera-frame copies and reuses upload resources.
Reviews (5) · Last reviewed commit: "fix(camera): remove unused VideoToolbox ..." · Reviewed by Greptile