Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| pub(crate) fn record( | ||
| capture: &PipeWireStream, | ||
| session: &RecordingSession, | ||
| settings: &RecorderSettings, | ||
| events: &mpsc::Sender<RecorderEvent>, | ||
| commands: mpsc::Receiver<Command>, | ||
| stop: &watch::Receiver<bool>, | ||
| ) -> Result<()> { |
There was a problem hiding this comment.
🔍 Production capture path lacks hardware validation
Synthetic tests bypass compositor DMA-BUF export, PipeWire negotiation, VA imports, and hardware encoding. Validate real Intel and AMD sessions before release.
Was this helpful? React with 👍 or 👎 to provide feedback.
Greptile SummaryThis PR adds an initial Linux CLI recording backend built around the Wayland ScreenCast portal, PipeWire DMA-BUF transport, VA-API encoding, and GStreamer muxing. It also integrates Linux daemon lifecycle handling, relocatable packaging, documentation, and CI coverage.
Confidence Score: 4/5The PR should not merge until stopping a job before recorder startup completes yields a successful cancellation rather than a failed job. The coordinator exposes stop control before invoking the Linux recorder’s start method, while the recorder converts that reachable pre-start stop into an error that the coordinator records as failure; the remaining CI action-pinning concern is non-blocking hardening. Files Needing Attention: crates/linux/src/lib.rs, crates/daemon/src/coordinator.rs, .github/workflows/ci.yml
|
| Filename | Overview |
|---|---|
| crates/linux/src/lib.rs | Implements the Linux recorder worker and control lifecycle; pre-start cancellation is incorrectly surfaced as startup failure. |
| crates/linux/src/pipeline.rs | Builds the DMA-BUF-to-VA-API recording pipeline with audio, metrics, pause/resume, finalization, and synthetic tests. |
| crates/linux/src/portal.rs | Implements ScreenCast portal discovery, source selection, cancellation, PipeWire access, and bounded cleanup. |
| crates/daemon/src/coordinator.rs | Defers Recording status and duration accounting until the backend reports its first encoded frame. |
| crates/daemon/src/server.rs | Selects the platform runtime and gives Linux captures a bounded shutdown-finalization period. |
| crates/control/src/client.rs | Adds release-prefix-relative Linux daemon discovery and direct executable launching. |
| .github/workflows/ci.yml | Adds Linux build, portal, packaging, and smoke-test coverage, but executes newly referenced actions through mutable tags. |
| scripts/package-cli-linux.sh | Produces a relocatable Linux CLI and daemon archive with documentation and licensing. |
| scripts/test-cli-linux.py | Exercises daemon discovery, headless failures, cleanup, restart, and signal handling from the extracted archive. |
Sequence Diagram
sequenceDiagram
participant CLI as wrec CLI
participant D as Linux daemon
participant C as Coordinator
participant P as Desktop portal
participant PW as PipeWire
participant G as GStreamer/VA-API
CLI->>D: record display:0/window:0
D->>C: create Starting job
C->>P: create session and open picker
P-->>C: selected stream and PipeWire node
C->>PW: open remote FD
C->>G: start DMA-BUF pipeline
G-->>C: first encoded frame
C-->>CLI: job becomes Recording
CLI->>C: pause/resume/stop
C->>G: state change or EOS
G-->>C: finalized fragmented MOV
C->>P: close portal session
C-->>CLI: Completed or Failed
Reviews (1): Last reviewed commit: "feat(linux): add Wayland CLI recording w..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
shivamhwp
changed the title
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| let exists = self | ||
| .connection | ||
| .get_window_attributes(self.xid) | ||
| .ok() | ||
| .and_then(|cookie| cookie.reply().ok()) | ||
| .is_some(); | ||
| if !exists { | ||
| self.lost.store(true, Ordering::SeqCst); | ||
| } |
There was a problem hiding this comment.
🔴 Minimizing a window can kill the daemon
When a captured window becomes unmapped, SourceWatch::lost still treats it as available. get_window_attributes succeeds for unmapped windows, while ximagesrc can raise BadMatch when reading them. handle_error delegates that error to Xlib's fatal handler, terminating the daemon.
Learn more
An X11 window can remain allocated after it is minimized or unmapped. GetWindowAttributes then succeeds, but its reply reports MapState::UNMAPPED. The watch currently checks only whether a reply exists. record_attempt enables ximagesrc to read that same window; an XGetImage request against an unmapped window can produce BadMatch. handle_error does not claim BadMatch, so Xlib's default error handler can terminate the daemon.
Example: A user starts recording window 42 and then minimizes it. Window 42 remains valid, so the polling watch reports no loss. A subsequent image read fails with BadMatch, and the daemon exits instead of reporting the stopped recording.
Recommended fix: Check the attribute reply's map_state as well as its existence, and handle the Xlib error associated with an unmapped captured window without masking unrelated displays or windows. Validate the minimize/unmap path in the X11 integration test.
Was this helpful? React with 👍 or 👎 to provide feedback.
| self.lost.load(Ordering::SeqCst) | ||
| && self.damage == Some((error.request_code, error.error_code)) | ||
| } |
There was a problem hiding this comment.
🔴 Early damage errors can kill the daemon
If XDamage reports a source-loss error before polling detects closure, Source::claims rejects it while lost is false. handle_error then forwards the error to Xlib's fatal handler, terminating the daemon instead of failing the recording.
Learn more
The XDamage extension has its own error code and may report an invalid damage resource when a captured window disappears. The source's lost flag is set only after a matching BadWindow/BadDrawable or a successful polling check of the vanished XID. An XDamage error that arrives first does not pass this predicate, so handle_error passes it to the previous Xlib handler; Xlib's default handler terminates the process on unhandled X errors. Polling is performed later in run, so that ordering is possible during the normal bus wait.
Example: Window 42 disappears just after a successful attribute check. Before the next check, a damage request returns BadDamage for the now-invalid damage resource. The lost flag is false, so the daemon exits rather than reporting window 42 as lost.
Recommended fix: Track the damage resource associated with each captured window, or otherwise attribute relevant damage errors without requiring an earlier lost flag. Exercise closure immediately after startup and between polling checks.
Was this helpful? React with 👍 or 👎 to provide feedback.
| status = Path(f"/proc/{daemon_pid}/status").read_text() | ||
| idle_rss_mib = int(next(line.split()[1] for line in status.splitlines() if line.startswith("VmRSS:"))) / 1024 | ||
| assert idle_rss_mib < 256, f"4K encoder retained {idle_rss_mib:.1f} MiB after teardown" |
There was a problem hiding this comment.
🔍 4K memory check depends on daemon baseline
The 256 MiB limit includes unrelated daemon and plugin memory, so CI host differences can fail this test without encoder retention. Compare idle RSS against a measured pre-recording baseline as well as a fixed budget.
Was this helpful? React with 👍 or 👎 to provide feedback.
This branch was successfully deployed
1 participant
Linux could not record through the CLI. This adds Wayland portal/PipeWire and X11 capture, preferring VA-API or NVIDIA encoding and falling back to software. The preferred Wayland/VA path shares DMA-BUFs and converts on the GPU; other paths trade CPU usage for compatibility.
Includes a relocatable CLI/daemon package, source selection and cancellation, AAC audio, pause/resume, and CI that records a real X11 desktop through the extracted package. Hardware testing caught and fixed cached portal connections outliving their runtime, variable-rate negotiation failures, and PipeWire resume failures. Pause now drops input buffers before conversion/encoding and adjusts timestamp metadata while preserving GPU memory.
Validation: 143 workspace tests, the isolated portal test, Clippy, six packaged CLI checks, and three real X11 capture checks. On Corex (Ryzen 9 5900H / Radeon Cezanne, Ubuntu 26.04, GNOME Wayland), actual H.264 display and HEVC animated-window recordings used DMA-BUF + VA-API without fallback. Full video/audio decoding and timestamps passed, including system audio, pause/resume, and stopping while paused. Process teardown was also checked: a pending real portal picker exited in 0.2s after SIGTERM; native GStreamer hangs encountered during diagnosis exited at the 15s deadline, with both processes confirmed gone.
The short 1080p HEVC sample used about 4% of one CPU core and 129 MiB peak daemon RSS. It recorded about 31 fps with a requested 60 fps ceiling; this does not establish sustained 60 fps, 4K performance, or Mac efficiency parity. Intel/NVIDIA hardware and other compositor combinations remain unverified. The existing Rust 1.78 SQLite build failure also occurs on unchanged main; validation uses stable Rust.
HTML test report · Linux setup and limitations