Repository navigation
Conversation
|
@shubhook is attempting to deploy a commit to the Cap Software Inc Team on Vercel. A member of the Team first needs to authorize it. |
|
Thanks for the PR. Greptile currently has this at 4/5 (reviewed before your latest commit), and we only merge PRs once Greptile scores the current head 5/5. Please comment This also overlaps #2501, which adds MIT-SHM capture to the same |
…ction Port the x11-capture-benchmark example and scripts/bench-x11.sh, which time full X11Grabber::grab calls per backend, verify identical pixels, support --json, and fail if the SHM grabber falls back to GetImage. Add X11Grabber::with_shm so callers can force the GetImage backend.
They are pub only so the x11-capture-benchmark example and the x11_shm_capture integration tests can use them directly.
9e052a4 to
9771355
Compare
|
@greptileai please re-review the latest head (9771355). Rebased onto main, moved the x11_shm_capture CI step into rust-tests, and addressed both earlier findings. |
|
Thanks @richiemcilroy. I rebased onto main. The CI step moved into I also asked Greptile for a re-review. I checked PR #2501 and here's how this PR differs from that:
If you'd rather land #2501 because it's smaller, I would love to send the integration tests, benchmark script and fallback fix as a follow-up on top of it. Either works for me. |
|
Hey @richiemcilroy, just bumping this one. CI still needs approval for the fork whenever you get a sec. |
Summary
1080p recording on X11 goes from ~41 fps to ~59 fps at a 60 fps target, measured under Xvfb on a 2-vCPU aarch64 VM. Each individual frame grab is 3.3–4.9× faster, measured from 720p to 4K.
Today, Cap's X11 capture calls
GetImagefor every frame, which copies the whole frame through the X socket. This PR usesShmGetImagewith a reused SysV shared-memory segment instead, so the X server writes frames straight into memory Cap can read. PlainGetImagestays as an automatic fallback.Wayland, macOS and Windows aren't affected. All new code is behind
cfg(target_os = "linux").How it works
GetImage:CAP_X11_DISABLE_SHMis set to1,true,yesoronGetImage. The grabber only switches toGetImagefor good, with a single warning, if the retry succeeds. If both fail, the problem is the window or region rather than SHM, so SHM stays on and the error is returned.unsafecode is inscreen_capture/linux_shm.rs, with a SAFETY comment on each block. The segment is marked for removal (IPC_RMID) as soon as the X server attaches it, so the kernel frees it even if Cap crashes. On drop, Cap detaches it from the server, waits for the server to confirm, then unmaps it locally.Changes
crates/recording/src/sources/screen_capture/linux_shm.rs(new): the SysV segment lifecycle, the MIT-SHM version check, and the stride and size calculations.crates/recording/src/sources/screen_capture/linux.rs:X11Grabbernow has aGrabBackendenum withShmandGetImagevariants.GetImagecode moved, unchanged, into a helper.X11GrabberandX11InputConfigwent frompub(crate)to#[doc(hidden)] pub. That's the narrowest visibility that lets the benchmark example and the integration tests (both separate crates) call the real grabber. They aren't meant as a supported API.with_shm(config, allow_shm), so callers can forceGetImage, anduses_shm().crates/recording/Cargo.toml: enables x11rb'sshmfeature.libcwas already a dependency, andCargo.lockis unchanged.crates/recording/tests/x11_shm_capture.rs(new): integration tests, each against its own Xvfb.crates/recording/examples/x11-capture-benchmark.rsandscripts/bench-x11.sh(new): a benchmark of realX11Grabber::grab()calls on each backend.Benchmark
Xvfb on aarch64 (OCI Ampere)
Neoverse-N1 with 2 vCPUs, X.Org Xvfb 21.1.11 with MIT-SHM 1.2. Each sample is one full
X11Grabber::grab(), including the copy into a new ffmpeg BGRZ frame. Each backend gets 30 warmup grabs and 300 timed grabs.This is a small shared VM, so absolute times are higher than on desktop hardware. GetImage timings were noisy on the 2-vCPU VM: its p95 and p99 varied a lot between runs, and its 4K mean ranged from about 99 ms to 132 ms. SHM timings stayed steady. Compare the speedup ratios, not the absolute times.
End-to-end recording
recording-benchmark --mode screen --duration 15under a 1920x1080 Xvfb, with a 60 fps target, two runs per backend. "Before" isCAP_X11_DISABLE_SHM=1, which runs the originalGetImagecode. Frame counts are from the capture thread'ssent=… dropped=…log line.Nothing is dropped with
GetImage. Each grab just takes too long, so fewer frames get captured: a 60 fps target over 15 s would be about 900 frames. User CPU was about the same: 22.1–23.3 s forGetImageand 23.3–23.7 s for SHM. SHM spends that time encoding about 45% more frames.How to test
Benchmark: run
scripts/bench-x11.sh. It builds the example in release mode and runs it underxvfb-run -s "-screen 0 3840x2160x24"at 720p, 1080p, 1440p and 4K. Add--jsonfor machine-readable output. It exits with an error if:GetImage'sFFmpeg needs to be available first, e.g. through
bun run cap-setup.Real session: record on an X11 desktop. The logs should show
X11 capture using MIT-SHM.Opt-out: record again with
CAP_X11_DISABLE_SHM=1. The logs should showX11 MIT-SHM capture disabled by CAP_X11_DISABLE_SHM, and the recording should work the same apart from slower grabs.Testing
cargo fmt --all --checkcargo clippy -p cap-recording --lib --tests --example x11-capture-benchmark --no-deps -- -D warningscargo test -p cap-recording --lib screen_capture: 57 passed. These include stride and size calculations, and checks that dropping a segment frees it.cargo test -p cap-recording --test x11_shm_capture -- --include-ignored: 8 passed. Each test starts its own Xvfb and checks one of:GetImagereturn identical pixels for a known patternCAP_X11_DISABLE_SHMopt-out worksGetImageGetImageRuntime fallback: there is no committed test for switching from SHM to
GetImagemid-run. I checked it by hand with a throwaway test that detached the segment on the server side and then grabbed 50 frames. Exactly one warning was logged, and every later frame usedGetImage.Pre-existing on main, unrelated: an aarch64-only clippy lint in
cap-utils(disk_space.rs:186, which is why--no-depsis used above) and thereal-device-test-runnerexample not building on Linux.Not measured, and follow-ups
scripts/bench-x11.sh, or I'll add them once I have hardware.AttachFd)
Confidence Score: 5/5
The PR appears safe to merge; no actionable issues remain.Summary
Adds MIT-SHM capture for Linux X11, with
GetImageas the fallback.The previous and current heads have different merge bases with the target commit, so this review covered the full PR diff. No new actionable issues were found.
Reviews (2) · Last reviewed commit: "chore(recording): trim narrating comment..." · Reviewed by Greptile