Repository navigation
perf(recording): capture X11 frames through MIT-SHM - #2501
Closed
yashranaway wants to merge 2 commits into
Closed
yashranaway wants to merge 2 commits into
yashranaway wants to merge 2 commits into
Conversation
X11Grabber requested every frame with GetImage, so each frame streamed through the X socket into a reply buffer before being copied into the FFmpeg frame. It now keeps one MIT-SHM 1.2 segment the server writes into with ShmGetImage, grows it when a captured window grows, and falls back to GetImage when the server has no shared memory or a shared read fails. A display test compares both paths byte for byte for a screen and a resized window, and skips without DISPLAY.
Contributor
Author
|
@greptileai review |
Contributor
|
@yashranaway 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}
The capture test painted the root window of whatever DISPLAY pointed at, which on a developer machine is the live desktop. It and the benchmark now read CAP_TEST_X11_DISPLAY and skip without it. Also drops two comments that only restated what the code does.
Contributor
Author
|
@greptileai review |
Member
|
Thanks for this! #2551 takes the same MIT-SHM approach and has now merged: it retries a failed ShmGetImage with GetImage for that frame instead of turning SHM off for the session, detaches its segments on drop, and its tests run in CI. Your memfd-based segment creation (no leak on crash, works across Flatpak/container IPC boundaries) would be a great follow-up on top of #2551 if you're interested. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
on x11 every captured frame came back through the x socket as one big image reply. that's about 23ms of cpu per 1080p frame here, so recording couldn't hold 60fps at 1080p or 30fps at 1440p. now the grabber keeps one shared memory segment the x server writes each frame into, the same way ffmpeg's x11grab and obs do it.
remote x or a server without shared memory falls back to the old path, and so does a shared read that ever fails. under xvfb a 1080p frame drops from about 25ms to 3.5ms and 1440p from 55ms to 8ms.
The PR appears safe to merge, with no outstanding findings.Summary
The PR adds shared-memory X11 capture and keeps
GetImageas a fallback.Reviews (2) · Last reviewed commit: "test(recording): run X11 capture tests o..." · Reviewed by Greptile