From 9d1844a5edcce0cc8dbf15ce2a6635b2b4cbea89 Mon Sep 17 00:00:00 2001 From: Francesco Bonacci Date: Tue, 26 May 2026 17:41:20 +0200 Subject: [PATCH 1/3] fix(cua-driver-rs)(recording): app_state.json + click.png-for-element_index + ffmpeg TCC fast-fail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three regressions introduced by the cross-platform Rust recording refactor (#1718, since the rename to cua-driver-core). User reproduced all three live on macOS Calculator (5+3) — zero app_state.json files, zero click.png files (all 5 turns used element_index), and a 0-byte recording.mp4 with `finalized: false` while ffmpeg silently blocked forever on a TCC prompt the daemon couldn't surface. 1. **app_state.json never written.** `recording::write_turn` had no AX snapshot path. Added `AX_SNAPSHOT_FN` callback mirroring the existing `SCREENSHOT_FN`/`CLICK_MARKER_FN` pattern, wired to `platform_macos::recording_hooks::app_state_json_for` (walks the AX tree fresh for the target pid + window_id and emits the same shape `get_window_state` returns minus the screenshot fields) and the same on Windows via UIA. Linux is intentionally a no-op — ATSPI has no cheap whole-tree snapshot and the file is omitted rather than faked. 2. **click.png never written for element_index clicks.** The check at recording.rs:335 only set `click_point` when `x` AND `y` were present in args, so AX-indexed clicks (the dominant path on Calculator AX buttons — all 5 user-reproed turns) fell through and click.png was never produced. Added `ELEMENT_BOUNDS_FN` resolving `element_index` → window-local screenshot-pixel coords via the live AX cache (macOS: `element_screen_center` minus window origin, multiplied by Retina scale derived from PNG width / logical width; Windows: cached center minus window origin, no scale). The element cache is shared with the recording-hook layer at `register_all` startup so the callback is wired without a registry detour. 3. **recording.mp4 never finalized on macOS.** ffmpeg+avfoundation needs its own per-binary Screen Recording grant; TCC doesn't propagate from the parent process. When spawned by a daemon there's no UI thread to display the consent dialog, so ffmpeg blocks forever on `[ScreenCaptureKit] requesting consent...`. Two fast-fail probes added to `VideoRecorder::start`: (a) `try_wait` polling for the first 1500 ms catches immediate exits with stderr tail, (b) on macOS, after another ~500 ms, if the output file is still 0 bytes we treat it as a TCC hang, kill the subprocess, and return an actionable error pointing the user at Privacy & Security → Screen & System Audio Recording with the resolved ffmpeg path. The error ends up in `session.json` `video.error` AND the `start_recording` response text so callers can't miss it. Deferred (separate follow-up issue): replace ffmpeg+avfoundation on macOS with a native ScreenCaptureKit binding so video works zero-config. Multi-week native-binding work, out of scope for a regression fix. Docs: RECORDING.md updated with the per-turn `app_state.json` shape + when it's omitted (Linux), the click.png element_index path, and the macOS ffmpeg TCC requirement with the planned migration. The `start_recording` MCP tool description picks up the same TCC note. Co-Authored-By: Claude Opus 4.7 --- .../rust/Skills/cua-driver/RECORDING.md | 42 ++++++++---- .../crates/cua-driver-core/src/recording.rs | 48 +++++++++++++- .../cua-driver-core/src/recording_tools.rs | 22 +++++-- .../rust/crates/cua-driver-core/src/video.rs | 61 +++++++++++++++++ .../rust/crates/cua-driver/src/main.rs | 30 +++++++++ .../rust/crates/platform-macos/src/lib.rs | 2 + .../platform-macos/src/recording_hooks.rs | 66 +++++++++++++++++++ .../crates/platform-macos/src/tools/mod.rs | 3 + .../rust/crates/platform-windows/src/lib.rs | 1 + .../platform-windows/src/recording_hooks.rs | 56 ++++++++++++++++ .../platform-windows/src/tools/impl_.rs | 3 + 11 files changed, 315 insertions(+), 19 deletions(-) create mode 100644 libs/cua-driver/rust/crates/platform-macos/src/recording_hooks.rs create mode 100644 libs/cua-driver/rust/crates/platform-windows/src/recording_hooks.rs diff --git a/libs/cua-driver/rust/Skills/cua-driver/RECORDING.md b/libs/cua-driver/rust/Skills/cua-driver/RECORDING.md index b3c88adf38..803ee17626 100644 --- a/libs/cua-driver/rust/Skills/cua-driver/RECORDING.md +++ b/libs/cua-driver/rust/Skills/cua-driver/RECORDING.md @@ -27,6 +27,18 @@ you don't want video. Requires ffmpeg on PATH; when missing, the per- turn capture continues without video and `last_error` carries the install hint. +**macOS gotcha — ffmpeg needs its own Screen Recording grant.** TCC on +macOS is per-binary, not per-process-tree. Even when cua-driver has +Screen Recording permission, the ffmpeg subprocess does not inherit +that grant — and when run from a daemon there's no UI thread to +surface the consent prompt, so ffmpeg blocks forever on the request. +The recorder fast-fails this case after ~2 s, kills the subprocess, +and surfaces an actionable error. Fix: add your ffmpeg binary +(`/opt/homebrew/bin/ffmpeg` for Homebrew on Apple Silicon) to System +Settings → Privacy & Security → Screen & System Audio Recording, then +restart cua-driver. A future PR will replace ffmpeg+avfoundation with +a native ScreenCaptureKit binding so video works zero-config on macOS. + ## Start / stop Two equivalent surfaces: the `start_recording` / `stop_recording` MCP @@ -59,22 +71,26 @@ daemon restart resets to disabled. Each action writes to `turn-NNNNN/` (five-digit zero-padded counter): -- `app_state.json` — post-action AX snapshot for the target pid, same - shape `get_window_state` returns (tree_markdown, element_count, - turn_id, etc.) minus the screenshot fields. The recorder resolves a - frontmost window internally (visible + on-current-Space preferred, - max-area fallback) since individual action tools carry a - window_id but the recorder has no caller-supplied anchor. -- `screenshot.png` — post-action capture of the same window the - recorder just snapshotted. Omitted when the pid has no visible - window. +- `app_state.json` — post-action AX/UIA snapshot for the target + `(pid, window_id)` carrying the same `tree_markdown` + + `element_count` shape `get_window_state` returns (minus the + screenshot fields — those live in `screenshot.png`). On macOS the + recorder resolves a frontmost window internally when the action's + args don't carry one; on Windows it uses the first window of the + target pid. **Omitted on Linux** — ATSPI doesn't expose a cheap + whole-tree snapshot, and the file is left out rather than faked. +- `screenshot.png` — post-action capture of the target window. + Omitted when the pid has no visible window. - `action.json` — the tool name, full input arguments, result summary, pid, click point (when applicable), ISO-8601 timestamp. -- `click.png` — only for click-family actions (`click`, +- `click.png` — for click-family actions (`click`, `double_click`, `right_click`): a copy of `screenshot.png` with a red dot drawn at - the click point (screen-absolute point → window-local pixels via - the screenshot's `scale_factor`). Absent for other tools and for - clicks whose point falls outside the captured window. + the click point. **Both addressing modes are covered:** explicit + `x, y` clicks use the supplied coordinates directly, and + `element_index`-addressed clicks resolve to the element's center + via the live AX/UIA cache, then convert to window-local screenshot + pixels. Absent for non-click tools and for clicks whose resolved + point falls outside the captured window. ## When to use it diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/recording.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/recording.rs index 1df064acb9..cf236283b3 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/recording.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/recording.rs @@ -49,6 +49,35 @@ pub fn set_click_marker_fn(f: impl Fn(&[u8], f64, f64) -> Option> + Send let _ = CLICK_MARKER_FN.set(Box::new(f)); } +// ── Platform AX-snapshot callback ──────────────────────────────────────────── +// +// Takes (window_id, pid) and returns JSON bytes for `app_state.json` (the +// post-action AX/UIA snapshot), or None if no snapshot is available on this +// platform. + +type AxSnapshotFnBox = Box, Option) -> Option> + Send + Sync>; +static AX_SNAPSHOT_FN: OnceLock = OnceLock::new(); + +/// Register the platform-specific AX/UIA snapshot callback. Call once at startup. +pub fn set_ax_snapshot_fn(f: impl Fn(Option, Option) -> Option> + Send + Sync + 'static) { + let _ = AX_SNAPSHOT_FN.set(Box::new(f)); +} + +// ── Platform element-bounds callback ───────────────────────────────────────── +// +// Resolves an element_index to its center point in window-local screenshot +// pixels (the same coordinate space as the existing `(cx, cy)` arg to +// `CLICK_MARKER_FN`). Used so click.png is also written on element-indexed +// clicks, not just pixel-addressed ones. + +type ElementBoundsFnBox = Box Option<(f64, f64)> + Send + Sync>; +static ELEMENT_BOUNDS_FN: OnceLock = OnceLock::new(); + +/// Register the platform-specific element-bounds resolver. Args: (window_id, pid, element_index). +pub fn set_element_bounds_fn(f: impl Fn(u64, i64, u32) -> Option<(f64, f64)> + Send + Sync + 'static) { + let _ = ELEMENT_BOUNDS_FN.set(Box::new(f)); +} + /// Persistent recording session state (singleton per process). pub struct RecordingSession { inner: Mutex, @@ -330,14 +359,21 @@ fn write_turn( // Extract window_id and pid from args for screenshot capture. let window_id = args.opt_u64("window_id"); let pid = args.opt_i64("pid"); + let element_index = args.opt_u64("element_index"); - // Extract click point for click-family tools. + // Extract click point for click-family tools. Falls back to the + // platform element_index → window-local-pixels resolver when the call + // used `element_index` instead of explicit `x, y`, so click.png is + // written for AX-indexed clicks too. let click_point: Option<(f64, f64)> = if matches!( tool_name, "click" | "double_click" | "right_click" ) { match (args.opt_f64("x"), args.opt_f64("y")) { (Some(x), Some(y)) => Some((x, y)), - _ => None, + _ => match (window_id, pid, element_index, ELEMENT_BOUNDS_FN.get()) { + (Some(wid), Some(p), Some(idx), Some(f)) => f(wid, p, idx as u32), + _ => None, + }, } } else { None @@ -356,6 +392,14 @@ fn write_turn( } write_json_atomic(&turn_dir.join("action.json"), &payload)?; + // Post-action AX/UIA snapshot — omitted on platforms that don't expose + // a cheap snapshot helper (today: Linux ATSPI). + if let Some(ax_fn) = AX_SNAPSHOT_FN.get() { + if let Some(json_bytes) = ax_fn(window_id, pid) { + let _ = std::fs::write(turn_dir.join("app_state.json"), &json_bytes); + } + } + // Capture screenshot if a callback is registered. if let Some(screenshot_fn) = SCREENSHOT_FN.get() { if let Some(png_bytes) = screenshot_fn(window_id, pid) { diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/recording_tools.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/recording_tools.rs index 07b9b3c229..b31be4e809 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/recording_tools.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/recording_tools.rs @@ -75,6 +75,14 @@ impl Tool for StartRecordingTool { brew install ffmpeg / apt install ffmpeg); when ffmpeg is missing the \ per-turn capture (screenshots + action.json) still runs and the \ session's `last_error` field carries the ffmpeg-not-found message.\n\n\ + **macOS extra requirement:** the ffmpeg binary itself needs Screen \ + Recording permission (System Settings → Privacy & Security → Screen & \ + System Audio Recording → add /opt/homebrew/bin/ffmpeg or equivalent). \ + TCC is per-binary on macOS — cua-driver having Screen Recording is NOT \ + sufficient for the ffmpeg subprocess. If the grant is missing, video \ + start fast-fails after ~2 s and the error surfaces in the response; \ + per-turn JSON+screenshot capture continues. A future PR will replace \ + ffmpeg with a native ScreenCaptureKit binding to remove this gate.\n\n\ State persists for the life of the daemon / MCP session; a restart \ resets to disabled with no on-disk state. Call `stop_recording` to \ disable + finalize the mp4.".into(), @@ -114,11 +122,17 @@ impl Tool for StartRecordingTool { match self.session.start(output_dir.as_deref().unwrap(), record_video) { Ok(()) => { let state = self.session.current_state(); + // When the caller asked for video and it failed (e.g. macOS + // ffmpeg TCC prompt deadlock), surface the actual error + // prominently — the per-turn capture still runs, but the + // caller deserves to know the mp4 won't materialize. + let video_failed = record_video && !state.video_active; let video_note = if record_video && state.video_active { - " (video → recording.mp4)" - } else if record_video && !state.video_active { - " (video requested but ffmpeg not available — see last_error)" - } else { "" }; + " (video → recording.mp4)".to_string() + } else if video_failed { + let err = state.last_error.clone().unwrap_or_else(|| "unknown".into()); + format!("\n\n⚠️ Video capture failed (per-turn JSON+screenshot still running):\n{err}") + } else { String::new() }; let msg = format!("✅ Recording started -> {}{}", state.output_dir.as_deref().unwrap_or("?"), video_note); diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/video.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/video.rs index 9d9ab226dc..7b3f86c676 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/video.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/video.rs @@ -129,6 +129,67 @@ impl VideoRecorder { }) }); + // Fast-fail probe. ffmpeg failures we care about: + // 1. immediate exit (bad input device, missing codec) — surface stderr + // 2. macOS: silent hang waiting on a TCC Screen Recording prompt that + // can't be displayed (subprocess of a daemon) — detected via + // no-frame-progress after 2 s. + let probe_deadline = Instant::now() + Duration::from_millis(1500); + loop { + match child.try_wait() { + Ok(Some(status)) => { + let tail = stderr_thread + .map(|h| h.join().unwrap_or_default()) + .unwrap_or_default(); + let tail_str = String::from_utf8_lossy(&tail); + let _ = child.kill(); + anyhow::bail!( + "ffmpeg exited immediately ({status}). stderr tail:\n{tail_str}" + ); + } + Ok(None) => { + if Instant::now() >= probe_deadline { + break; + } + std::thread::sleep(Duration::from_millis(100)); + } + Err(e) => anyhow::bail!("ffmpeg try_wait failed: {e}"), + } + } + + // macOS-specific: ffmpeg + avfoundation Screen Recording permission + // is per-binary, NOT inherited from the parent process. When a + // daemon spawns ffmpeg the OS can't surface the consent dialog and + // the subprocess blocks forever. Detect via no-output-file-grew + // 2 s in, kill the child, return an actionable error. + #[cfg(target_os = "macos")] + { + std::thread::sleep(Duration::from_millis(500)); + let progressed = std::fs::metadata(&output_path) + .map(|m| m.len() > 0) + .unwrap_or(false); + if !progressed { + let _ = child.kill(); + let _ = child.wait(); + let tail = stderr_thread + .map(|h| h.join().unwrap_or_default()) + .unwrap_or_default(); + let tail_str = String::from_utf8_lossy(&tail); + anyhow::bail!( + "ffmpeg appears to be blocked on the macOS Screen Recording TCC \ + prompt. Open System Settings → Privacy & Security → Screen & \ + System Audio Recording and grant access to your ffmpeg binary \ + (e.g. /opt/homebrew/bin/ffmpeg), then restart cua-driver. Note: \ + cua-driver itself having Screen Recording permission is NOT \ + sufficient — the ffmpeg subprocess needs its own grant. A \ + follow-up PR will replace ffmpeg with ScreenCaptureKit on macOS \ + to remove this requirement. ffmpeg path: {ffmpeg_path}. stderr \ + tail:\n{tail_str}", + ffmpeg_path = ffmpeg.display() + ); + } + } + Ok(VideoRecorder { child, output_path, diff --git a/libs/cua-driver/rust/crates/cua-driver/src/main.rs b/libs/cua-driver/rust/crates/cua-driver/src/main.rs index 887291efd9..2402ee9cb5 100644 --- a/libs/cua-driver/rust/crates/cua-driver/src/main.rs +++ b/libs/cua-driver/rust/crates/cua-driver/src/main.rs @@ -122,6 +122,12 @@ fn main() { cua_driver_core::recording::set_click_marker_fn(|png_bytes, cx, cy| { platform_macos::capture::crosshair_png_bytes(png_bytes, cx, cy).ok() }); + cua_driver_core::recording::set_ax_snapshot_fn(|window_id, pid| { + platform_macos::recording_hooks::app_state_json_for(window_id, pid) + }); + cua_driver_core::recording::set_element_bounds_fn(|wid, pid, idx| { + platform_macos::recording_hooks::element_window_local_xy(wid, pid, idx) + }); let reg = Arc::new(platform_macos::register_tools()); reg.init_self_weak(); cli::run_call(reg, &tool, json_args, screenshot_out_file, socket); @@ -163,6 +169,12 @@ fn main() { cua_driver_core::recording::set_click_marker_fn(|png_bytes, cx, cy| { platform_macos::capture::crosshair_png_bytes(png_bytes, cx, cy).ok() }); + cua_driver_core::recording::set_ax_snapshot_fn(|window_id, pid| { + platform_macos::recording_hooks::app_state_json_for(window_id, pid) + }); + cua_driver_core::recording::set_element_bounds_fn(|wid, pid, idx| { + platform_macos::recording_hooks::element_window_local_xy(wid, pid, idx) + }); let reg = Arc::new(platform_macos::register_tools()); reg.init_self_weak(); let sp = socket.unwrap_or_else(serve::default_socket_path); @@ -287,6 +299,12 @@ fn main() { cua_driver_core::recording::set_click_marker_fn(|png_bytes, cx, cy| { platform_macos::capture::crosshair_png_bytes(png_bytes, cx, cy).ok() }); + cua_driver_core::recording::set_ax_snapshot_fn(|window_id, pid| { + platform_macos::recording_hooks::app_state_json_for(window_id, pid) + }); + cua_driver_core::recording::set_element_bounds_fn(|wid, pid, idx| { + platform_macos::recording_hooks::element_window_local_xy(wid, pid, idx) + }); std::thread::Builder::new() .name("cua-mcp".into()) @@ -522,6 +540,12 @@ fn build_registry(cursor_cfg: cursor_overlay::CursorConfig) -> cua_driver_core:: cua_driver_core::recording::set_click_marker_fn(|png_bytes, cx, cy| { platform_windows::capture::crosshair_png_bytes(png_bytes, cx, cy).ok() }); + cua_driver_core::recording::set_ax_snapshot_fn(|window_id, pid| { + platform_windows::recording_hooks::app_state_json_for(window_id, pid) + }); + cua_driver_core::recording::set_element_bounds_fn(|wid, pid, idx| { + platform_windows::recording_hooks::element_window_local_xy(wid, pid, idx) + }); platform_windows::register_tools_with_cursor(cursor_cfg, compat) } #[cfg(target_os = "linux")] @@ -575,6 +599,12 @@ fn build_registry_no_cursor() -> cua_driver_core::tool::ToolRegistry { cua_driver_core::recording::set_click_marker_fn(|png_bytes, cx, cy| { platform_windows::capture::crosshair_png_bytes(png_bytes, cx, cy).ok() }); + cua_driver_core::recording::set_ax_snapshot_fn(|window_id, pid| { + platform_windows::recording_hooks::app_state_json_for(window_id, pid) + }); + cua_driver_core::recording::set_element_bounds_fn(|wid, pid, idx| { + platform_windows::recording_hooks::element_window_local_xy(wid, pid, idx) + }); platform_windows::register_tools_with_cursor(cursor_overlay::CursorConfig { enabled: false, ..Default::default() }, compat) } #[cfg(target_os = "linux")] diff --git a/libs/cua-driver/rust/crates/platform-macos/src/lib.rs b/libs/cua-driver/rust/crates/platform-macos/src/lib.rs index 634141c270..f6afcc35c5 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/lib.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/lib.rs @@ -30,6 +30,8 @@ pub mod focus_guard; pub mod window_change_detector; #[cfg(target_os = "macos")] pub mod tools; +#[cfg(target_os = "macos")] +pub mod recording_hooks; use cua_driver_core::tool::ToolRegistry; diff --git a/libs/cua-driver/rust/crates/platform-macos/src/recording_hooks.rs b/libs/cua-driver/rust/crates/platform-macos/src/recording_hooks.rs new file mode 100644 index 0000000000..ca5ca86bcc --- /dev/null +++ b/libs/cua-driver/rust/crates/platform-macos/src/recording_hooks.rs @@ -0,0 +1,66 @@ +//! Recording callbacks exposed to `cua_driver_core::recording`. +//! +//! Two hooks live here: +//! - `app_state_json_for` — produces `app_state.json` bytes for a turn folder. +//! - `element_window_local_xy` — resolves `element_index` to a click point in +//! window-local screenshot-pixel coordinates so `click.png` is also written +//! on AX-indexed clicks (not just pixel-addressed ones). +//! +//! The element-bounds resolver needs the per-(pid, window_id) element cache, +//! which lives in `ToolState`. `tools::register_all` shares the active cache +//! here via `set_element_cache` at startup. + +use std::sync::{Arc, OnceLock}; + +use crate::ax::cache::ElementCache; +use crate::ax::bindings::{element_screen_center, AXUIElementRef}; + +static ELEMENT_CACHE: OnceLock> = OnceLock::new(); + +pub fn set_element_cache(cache: Arc) { + let _ = ELEMENT_CACHE.set(cache); +} + +/// Build `app_state.json` bytes for the turn folder. Walks the AX tree for +/// (pid, window_id) and emits the same shape `get_window_state` returns +/// (minus screenshot fields — those live in `screenshot.png`). +pub fn app_state_json_for(window_id: Option, pid: Option) -> Option> { + let pid = pid? as i32; + let resolved_wid = match window_id { + Some(w) => w as u32, + None => crate::windows::resolve_main_window_id(pid).ok()?, + }; + let result = crate::ax::tree::walk_tree(pid, Some(resolved_wid), None); + let element_count = result.nodes.iter().filter(|n| n.element_index.is_some()).count(); + let payload = serde_json::json!({ + "pid": pid, + "window_id": resolved_wid, + "element_count": element_count, + "tree_markdown": result.tree_markdown, + }); + serde_json::to_vec_pretty(&payload).ok() +} + +/// Resolve `element_index` to window-local screenshot-pixel coords for +/// (pid, window_id). `element_screen_center` returns SCREEN points; convert +/// by subtracting the window's screen origin and multiplying by the +/// screenshot's pixels-per-point scale. +pub fn element_window_local_xy(window_id: u64, pid: i64, element_index: u32) -> Option<(f64, f64)> { + let cache = ELEMENT_CACHE.get()?; + let ptr = cache.get_element_ptr(pid as i32, window_id as u32, element_index as usize)?; + let (sx, sy) = unsafe { element_screen_center(ptr as AXUIElementRef)? }; + + let bounds = crate::windows::window_bounds_by_id(window_id as u32)?; + // Probe the captured PNG's width to derive the Retina scale — the + // screenshot is in physical pixels, the window bounds are in points. + let scale = if let Ok(png) = crate::capture::screenshot_window_bytes(window_id as u32) { + if png.len() >= 24 { + let pw = u32::from_be_bytes([png[16], png[17], png[18], png[19]]) as f64; + if bounds.width > 0.0 && pw > bounds.width { pw / bounds.width } else { 1.0 } + } else { 1.0 } + } else { 1.0 }; + + let wx = (sx - bounds.x) * scale; + let wy = (sy - bounds.y) * scale; + Some((wx, wy)) +} diff --git a/libs/cua-driver/rust/crates/platform-macos/src/tools/mod.rs b/libs/cua-driver/rust/crates/platform-macos/src/tools/mod.rs index bc8fc4ca1d..baa5731f67 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/tools/mod.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/tools/mod.rs @@ -209,6 +209,9 @@ impl Default for ToolState { /// note telling the caller to use pixel-addressed tools. pub fn register_all(registry: &mut ToolRegistry, compat: bool) { let state = Arc::new(ToolState::default()); + // Share the element cache with the recording-hook layer so it can + // resolve element_index → window-local screenshot coords for click.png. + crate::recording_hooks::set_element_cache(state.element_cache.clone()); registry.register(Box::new(list_apps::ListAppsTool)); registry.register(Box::new(list_windows::ListWindowsTool)); diff --git a/libs/cua-driver/rust/crates/platform-windows/src/lib.rs b/libs/cua-driver/rust/crates/platform-windows/src/lib.rs index 7ad55df239..74130c6778 100644 --- a/libs/cua-driver/rust/crates/platform-windows/src/lib.rs +++ b/libs/cua-driver/rust/crates/platform-windows/src/lib.rs @@ -14,6 +14,7 @@ use cua_driver_core::tool::ToolRegistry; pub mod tools; pub mod overlay; pub mod diagnostics; +pub mod recording_hooks; #[cfg(target_os = "windows")] pub mod win32; diff --git a/libs/cua-driver/rust/crates/platform-windows/src/recording_hooks.rs b/libs/cua-driver/rust/crates/platform-windows/src/recording_hooks.rs new file mode 100644 index 0000000000..2b738d860c --- /dev/null +++ b/libs/cua-driver/rust/crates/platform-windows/src/recording_hooks.rs @@ -0,0 +1,56 @@ +//! Recording callbacks exposed to `cua_driver_core::recording`. +//! +//! Two hooks: +//! - `app_state_json_for` — produces `app_state.json` bytes for a turn folder. +//! - `element_window_local_xy` — resolves `element_index` to a click point in +//! window-local screenshot-pixel coordinates so `click.png` is also written +//! on UIA/MSAA-indexed clicks (not just pixel-addressed ones). + +#[cfg(target_os = "windows")] +use std::sync::{Arc, OnceLock}; + +#[cfg(target_os = "windows")] +use crate::uia::ElementCache; + +#[cfg(target_os = "windows")] +static ELEMENT_CACHE: OnceLock> = OnceLock::new(); + +#[cfg(target_os = "windows")] +pub fn set_element_cache(cache: Arc) { + let _ = ELEMENT_CACHE.set(cache); +} + +#[cfg(target_os = "windows")] +pub fn app_state_json_for(window_id: Option, pid: Option) -> Option> { + let pid = pid? as u32; + let hwnd = match window_id { + Some(w) => w, + None => crate::win32::list_windows(Some(pid)).first().map(|w| w.hwnd)?, + }; + let result = crate::uia::walk_tree(hwnd, None); + let element_count = result.nodes.iter().filter(|n| n.element_index.is_some()).count(); + let payload = serde_json::json!({ + "pid": pid, + "window_id": hwnd, + "element_count": element_count, + "tree_markdown": result.tree_markdown, + }); + serde_json::to_vec_pretty(&payload).ok() +} + +#[cfg(target_os = "windows")] +pub fn element_window_local_xy(window_id: u64, pid: i64, element_index: u32) -> Option<(f64, f64)> { + let cache = ELEMENT_CACHE.get()?; + let (sx, sy) = cache.get_element_center(pid as u32, window_id, element_index as usize)?; + // The cached center is in SCREEN coords. Convert to window-local pixel + // coords by subtracting the window's screen origin (GetWindowRect-equivalent + // in WindowInfo). Windows captures at logical pixels so no scale factor. + let wins = crate::win32::list_windows(Some(pid as u32)); + let win = wins.iter().find(|w| w.hwnd == window_id)?; + Some(((sx - win.x) as f64, (sy - win.y) as f64)) +} + +#[cfg(not(target_os = "windows"))] +pub fn app_state_json_for(_window_id: Option, _pid: Option) -> Option> { None } +#[cfg(not(target_os = "windows"))] +pub fn element_window_local_xy(_window_id: u64, _pid: i64, _element_index: u32) -> Option<(f64, f64)> { None } diff --git a/libs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rs b/libs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rs index 0350bae0b8..63b96ec3a6 100644 --- a/libs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rs +++ b/libs/cua-driver/rust/crates/platform-windows/src/tools/impl_.rs @@ -5117,6 +5117,9 @@ impl Tool for DebugWindowInfoTool { pub fn build_registry(compat: bool) -> ToolRegistry { let state = ToolState::new(); + // Share the element cache with the recording-hook layer so it can + // resolve element_index → window-local screenshot coords for click.png. + crate::recording_hooks::set_element_cache(state.element_cache.clone()); let mut r = ToolRegistry::new(); r.register(Box::new(ListAppsTool)); r.register(Box::new(ListWindowsTool)); From eb4bf5e6f0cf272936d38783919f827b9b4036cf Mon Sep 17 00:00:00 2001 From: Francesco Bonacci Date: Tue, 26 May 2026 21:47:45 +0200 Subject: [PATCH 2/3] feat(cua-driver-rs)(recording): native ScreenCaptureKit video on macOS, ffmpeg only on Win/Linux MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the macOS ffmpeg+avfoundation subprocess pipeline with an in-process SCStream + SCRecordingOutput. ScreenCaptureKit runs in the cua-driver process, so it inherits the daemon's Screen Recording TCC grant — no separate subprocess prompt, no fast-fail hang heuristic. video.rs now exposes `VideoBackend` + `VideoBackendFactory` traits; each platform crate registers its concrete factory at startup (`set_video_backend_factory`), mirroring how `SCREENSHOT_FN` / `AX_SNAPSHOT_FN` are wired. macOS registers `SckitVideoBackendFactory` (platform-macos/src/video_sckit.rs); Windows + Linux register `FfmpegVideoBackendFactory` (cua-driver-core/src/video_ffmpeg.rs, carrying the existing stderr-tail fast-fail). Requires macOS 15.0+ (SCRecordingOutput introduced in macOS 15). Co-Authored-By: Claude Opus 4.7 --- libs/cua-driver/rust/Cargo.lock | 55 +++ .../rust/Skills/cua-driver/RECORDING.md | 34 +- .../rust/crates/cua-driver-core/src/lib.rs | 1 + .../crates/cua-driver-core/src/recording.rs | 11 +- .../cua-driver-core/src/recording_loader.rs | 2 +- .../cua-driver-core/src/recording_render.rs | 2 +- .../cua-driver-core/src/recording_tools.rs | 26 +- .../rust/crates/cua-driver-core/src/video.rs | 413 ++---------------- .../cua-driver-core/src/video_ffmpeg.rs | 290 ++++++++++++ .../rust/crates/cua-driver/build.rs | 29 ++ .../rust/crates/cua-driver/src/main.rs | 21 + .../rust/crates/platform-macos/Cargo.toml | 6 + .../rust/crates/platform-macos/src/lib.rs | 2 + .../crates/platform-macos/src/video_sckit.rs | 149 +++++++ 14 files changed, 632 insertions(+), 409 deletions(-) create mode 100644 libs/cua-driver/rust/crates/cua-driver-core/src/video_ffmpeg.rs create mode 100644 libs/cua-driver/rust/crates/cua-driver/build.rs create mode 100644 libs/cua-driver/rust/crates/platform-macos/src/video_sckit.rs diff --git a/libs/cua-driver/rust/Cargo.lock b/libs/cua-driver/rust/Cargo.lock index 517a173a72..e092bbaa41 100644 --- a/libs/cua-driver/rust/Cargo.lock +++ b/libs/cua-driver/rust/Cargo.lock @@ -23,6 +23,25 @@ version = "1.0.102" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "7f202df86484c868dbad7eaa557ef785d5c66295e41b460ef922eca0723b842c" +[[package]] +name = "apple-cf" +version = "0.9.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e7fd680cd0f3f02ee717b2014b26d18985c57b784384ba47213fcf8791e8c250" +dependencies = [ + "doom-fish-utils", +] + +[[package]] +name = "apple-metal" +version = "0.8.7" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "770ed47ba948122fc9ce0bc10a433cf9fabcf1c2ed6d44db5b7ad0d70ffede15" +dependencies = [ + "doom-fish-utils", + "libc", +] + [[package]] name = "arrayref" version = "0.3.9" @@ -230,6 +249,21 @@ dependencies = [ "cfg-if", ] +[[package]] +name = "crossbeam-queue" +version = "0.3.12" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0f58bbc28f91df819d0aa2a2c00cd19754769c2fad90579b3592b1c9ba7a3115" +dependencies = [ + "crossbeam-utils", +] + +[[package]] +name = "crossbeam-utils" +version = "0.8.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "d0a5c400df2834b80a4c3327b3aad3a4c4cd4de0629063962b03235697506a28" + [[package]] name = "crypto-common" version = "0.1.7" @@ -364,6 +398,16 @@ dependencies = [ "litrs", ] +[[package]] +name = "doom-fish-utils" +version = "0.3.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "a2f014d56e0b8ef916deeaa003fb711eb52879fae2701d86cc73953367b805ac" +dependencies = [ + "crossbeam-queue", + "futures-util", +] + [[package]] name = "embed-manifest" version = "1.5.0" @@ -1192,6 +1236,7 @@ dependencies = [ "objc2-app-kit", "objc2-foundation", "objc2-quartz-core", + "screencapturekit", "serde", "serde_json", "thiserror", @@ -1497,6 +1542,16 @@ version = "1.2.0" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "94143f37725109f92c262ed2cf5e59bce7498c01bcc1502d7b9afe439a4e9f49" +[[package]] +name = "screencapturekit" +version = "6.0.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "74afe91a6a3f8859ff08339535891819f5438e7cb284c1b74cc8866eb80956bc" +dependencies = [ + "apple-cf", + "apple-metal", +] + [[package]] name = "security-framework" version = "3.7.0" diff --git a/libs/cua-driver/rust/Skills/cua-driver/RECORDING.md b/libs/cua-driver/rust/Skills/cua-driver/RECORDING.md index 803ee17626..8482fecdee 100644 --- a/libs/cua-driver/rust/Skills/cua-driver/RECORDING.md +++ b/libs/cua-driver/rust/Skills/cua-driver/RECORDING.md @@ -20,24 +20,22 @@ permission probes, agent-cursor getters / setters, and the recording controls themselves) are not recorded. **Video on by default.** `start_recording` also captures the main -display to `/recording.mp4` (H.264 / yuv420p / 30 fps) via -an ffmpeg subprocess for the lifetime of the session. The mp4 is -finalized on `stop_recording`. Opt out with `record_video: false` when -you don't want video. Requires ffmpeg on PATH; when missing, the per- -turn capture continues without video and `last_error` carries the -install hint. - -**macOS gotcha — ffmpeg needs its own Screen Recording grant.** TCC on -macOS is per-binary, not per-process-tree. Even when cua-driver has -Screen Recording permission, the ffmpeg subprocess does not inherit -that grant — and when run from a daemon there's no UI thread to -surface the consent prompt, so ffmpeg blocks forever on the request. -The recorder fast-fails this case after ~2 s, kills the subprocess, -and surfaces an actionable error. Fix: add your ffmpeg binary -(`/opt/homebrew/bin/ffmpeg` for Homebrew on Apple Silicon) to System -Settings → Privacy & Security → Screen & System Audio Recording, then -restart cua-driver. A future PR will replace ffmpeg+avfoundation with -a native ScreenCaptureKit binding so video works zero-config on macOS. +display to `/recording.mp4` (H.264 / 30 fps) for the +lifetime of the session. The mp4 is finalized on `stop_recording`. Opt +out with `record_video: false` when you don't want video. + +**macOS — native ScreenCaptureKit, zero-config.** On macOS the +recorder uses an in-process `SCStream` + `SCRecordingOutput`, so it +inherits cua-driver's own Screen Recording grant — no separate +subprocess prompt, no fast-fail, no second TCC dance. Requires macOS +15.0+ (SCRecordingOutput introduced in macOS 15). No ffmpeg needed. + +**Windows / Linux — ffmpeg subprocess.** Outside macOS the recorder +shells to ffmpeg with `gdigrab` (Windows) or `x11grab` (Linux). The +binary needs to be on PATH (`winget install Gyan.FFmpeg` / +`apt install ffmpeg`); when missing, the per-turn capture continues +without video and `last_error` carries the install hint. ffmpeg +startup failures fast-fail with a stderr tail in the error. ## Start / stop diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/lib.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/lib.rs index 8915a8a9c3..e0b7789b6f 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/lib.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/lib.rs @@ -26,5 +26,6 @@ pub mod text_sanitize; pub mod tool; pub mod tool_args; pub mod video; +pub mod video_ffmpeg; pub use recording::RecordingSession; diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/recording.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/recording.rs index cf236283b3..11b816b444 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/recording.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/recording.rs @@ -18,7 +18,7 @@ use std::time::Instant; use serde_json::Value; use crate::cursor_sampler::CursorSampler; -use crate::video::{VideoMetadata, VideoRecorder}; +use crate::video::{self, VideoBackend, VideoMetadata}; // ── Platform screenshot callback ───────────────────────────────────────────── // @@ -92,9 +92,10 @@ struct RecordingInner { /// matches the action-timeline anchor in `action.json`. session_monotonic_start: Option, last_error: Option, - /// Live ffmpeg subprocess when video capture is active. Recreated - /// per session. - video: Option, + /// Live video backend when capture is active. Recreated per + /// session. The concrete type is platform-determined (SCKit on + /// macOS, ffmpeg subprocess elsewhere). + video: Option>, /// Recorded after `stop()` until the next start — exposed in /// `current_state()` so callers can read the finalized video info /// after stopping. @@ -173,7 +174,7 @@ impl RecordingSession { let mut video_error: Option = None; if record_video { let path = dir.join("recording.mp4"); - match VideoRecorder::start(&path) { + match video::start_video(&path) { Ok(rec) => { inner.video = Some(rec); video_present = true; diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/recording_loader.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/recording_loader.rs index 16e04d31ee..59ada94f73 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/recording_loader.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/recording_loader.rs @@ -125,7 +125,7 @@ fn load_session_metadata( /// absolute last resort. fn probe_video_dimensions(video_path: &Path) -> Option<(u32, u32)> { use std::process::Command; - let ffprobe = crate::video::find_ffprobe()?; + let ffprobe = crate::video_ffmpeg::find_ffprobe()?; let out = Command::new(ffprobe) .args(["-v", "error", "-select_streams", "v:0", diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/recording_render.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/recording_render.rs index a12707e250..325b84cace 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/recording_render.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/recording_render.rs @@ -36,7 +36,7 @@ use std::process::{Command, Stdio}; use crate::recording_loader::{load, LoadError}; use crate::recording_zoom::{generate_zoom_regions, ZoomRegion}; -use crate::video::{find_ffmpeg, find_ffprobe}; +use crate::video_ffmpeg::{find_ffmpeg, find_ffprobe}; /// Default zoom magnification. const DEFAULT_ZOOM_SCALE: f64 = 2.0; diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/recording_tools.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/recording_tools.rs index b31be4e809..cd2e02912b 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/recording_tools.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/recording_tools.rs @@ -69,20 +69,18 @@ impl Tool for StartRecordingTool { Turn folders are named `turn-00001/`, `turn-00002/`, etc. Turn \ numbering restarts at 1 each time recording is (re-)started.\n\n\ **Video is on by default** — the main display is captured to \ - `/recording.mp4` (H.264 / yuv420p / 30 fps) via an ffmpeg \ - subprocess for the lifetime of the session. Pass `record_video: false` \ - to opt out. Requires ffmpeg on PATH (winget install Gyan.FFmpeg / \ - brew install ffmpeg / apt install ffmpeg); when ffmpeg is missing the \ - per-turn capture (screenshots + action.json) still runs and the \ - session's `last_error` field carries the ffmpeg-not-found message.\n\n\ - **macOS extra requirement:** the ffmpeg binary itself needs Screen \ - Recording permission (System Settings → Privacy & Security → Screen & \ - System Audio Recording → add /opt/homebrew/bin/ffmpeg or equivalent). \ - TCC is per-binary on macOS — cua-driver having Screen Recording is NOT \ - sufficient for the ffmpeg subprocess. If the grant is missing, video \ - start fast-fails after ~2 s and the error surfaces in the response; \ - per-turn JSON+screenshot capture continues. A future PR will replace \ - ffmpeg with a native ScreenCaptureKit binding to remove this gate.\n\n\ + `/recording.mp4` (H.264 / 30 fps) for the lifetime of \ + the session. Pass `record_video: false` to opt out.\n\n\ + **macOS uses native ScreenCaptureKit** (in-process SCStream + \ + SCRecordingOutput) so video inherits cua-driver's own Screen \ + Recording grant — no extra TCC prompt, no ffmpeg subprocess. \ + Requires macOS 15.0+.\n\n\ + **Windows + Linux use an ffmpeg subprocess** (`gdigrab` / \ + `x11grab` + libx264). Requires ffmpeg on PATH (winget install \ + Gyan.FFmpeg / apt install ffmpeg); when ffmpeg is missing or \ + fails on startup the per-turn capture (screenshots + \ + action.json) still runs and the session's `last_error` field \ + carries the diagnostic.\n\n\ State persists for the life of the daemon / MCP session; a restart \ resets to disabled with no on-disk state. Call `stop_recording` to \ disable + finalize the mp4.".into(), diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/video.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/video.rs index 7b3f86c676..f7eddc53d9 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/video.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/video.rs @@ -1,392 +1,65 @@ -//! Cross-platform screen-video capture via ffmpeg subprocess. +//! Cross-platform video-capture abstraction. //! -//! Spawned by `RecordingSession` when video recording is requested. Each -//! platform uses ffmpeg with a platform-appropriate input device: +//! The recording session calls into a single `VideoBackend` trait; the +//! concrete implementation is selected at process startup by the +//! platform crate. Today: //! -//! - **Windows:** `gdigrab` (GDI screen-grab of the main display) -//! - **macOS:** `avfoundation` (input "1" = main display, no audio) -//! - **Linux:** `x11grab` (DISPLAY env var, defaults to `:0.0`) +//! - **macOS:** native ScreenCaptureKit via `platform_macos::video_sckit` +//! (no extra TCC grant — inherits cua-driver's own Screen Recording +//! permission, no subprocess). +//! - **Windows + Linux:** ffmpeg subprocess via `video_ffmpeg` +//! (`gdigrab` / `x11grab` input + libx264 encode). //! -//! Encoder is `libx264 -preset ultrafast -pix_fmt yuv420p` everywhere — -//! produces a broadly playable MP4 at low CPU. Default framerate 30 fps. +//! The factory is registered with `set_video_backend_factory` from each +//! platform's `main.rs` startup block, mirroring how `SCREENSHOT_FN` / +//! `AX_SNAPSHOT_FN` are wired in `recording.rs`. //! -//! Lifecycle: -//! 1. `VideoRecorder::start(path)` spawns ffmpeg writing to `path`. -//! 2. Caller stays alive while recording. -//! 3. `recorder.stop()` sends ffmpeg `q` on stdin (clean shutdown that -//! finalizes the mp4 moov atom) and waits for it to exit. On Windows -//! `q\n` works the same way — ffmpeg's stdin handler reads any -//! keypress and triggers `do_exit()`. -//! -//! When ffmpeg isn't on PATH or fails to start, `start()` returns a -//! structured error that the recording session surfaces to the MCP / -//! CLI caller. - -use std::io::Write; -use std::path::PathBuf; -use std::process::{Child, Command, Stdio}; -use std::time::{Duration, Instant}; +//! `VideoMetadata` is the shape `RecordingSession` stamps into +//! `session.json` after `stop()` — kept identical to the prior concrete +//! `VideoRecorder::stop` return so the on-disk schema is unchanged. -/// One active video-capture process. -pub struct VideoRecorder { - child: Child, - output_path: PathBuf, - started_at: Instant, - /// Drains ffmpeg's stderr so its pipe doesn't fill up and block the - /// encoder. The collected tail is read in `stop()` for diagnostics - /// when ffmpeg exits non-zero. - stderr_thread: Option>>, -} +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; -/// Metadata returned by `VideoRecorder::stop()`; mirrors the Swift impl's -/// `FinalMetadata` so `session.json` carries the same shape across both -/// platforms. +/// Finalized metadata returned by `VideoBackend::stop`. Mirrors the +/// Swift impl's `FinalMetadata` so `session.json` carries the same +/// shape across all backends. #[derive(Debug, Clone)] pub struct VideoMetadata { pub path: PathBuf, /// Wall-clock duration the recorder was active. pub duration_ms: u64, - /// Whether ffmpeg exited cleanly (so the mp4 is playable). + /// Whether the backend finalized the mp4 cleanly (playable file). pub finalized: bool, } -impl VideoRecorder { - /// Spawn ffmpeg writing the main display to `output_path`. - pub fn start(output_path: impl Into) -> anyhow::Result { - let output_path = output_path.into(); - - // Resolve ffmpeg location. We probe `ffmpeg` (PATH lookup) and emit - // a clear, actionable error when missing — ffmpeg is the one - // runtime dep video recording carries, and a vague "command failed" - // would just push debugging cost onto callers. - let ffmpeg = match find_ffmpeg() { - Some(p) => p, - None => anyhow::bail!( - "ffmpeg not found on PATH. Install with: \ - winget install Gyan.FFmpeg (Windows), \ - brew install ffmpeg (macOS), \ - or apt install ffmpeg (Linux)." - ), - }; - - // Make sure the parent directory exists. - if let Some(parent) = output_path.parent() { - std::fs::create_dir_all(parent).ok(); - } - - // Platform-specific input device flags. Encoder + container flags - // are shared. - let mut cmd = Command::new(&ffmpeg); - cmd.arg("-y") // overwrite existing output - .arg("-loglevel").arg("error"); // we don't want ffmpeg's progress on stderr - - platform_input_args(&mut cmd); - - // yuv420p (the broad-compat pixel format we encode to) requires - // BOTH width and height to be even. Most desktops aren't (taskbar - // ate one row on this Win11 host: 1512×949). Pad by 1 px on the - // bottom/right when needed so libx264 accepts the frame. Padding - // beats cropping — keeps the full display in frame. - cmd.arg("-vf").arg("pad=ceil(iw/2)*2:ceil(ih/2)*2"); - - cmd.arg("-c:v").arg("libx264") - .arg("-preset").arg("ultrafast") - .arg("-pix_fmt").arg("yuv420p") - .arg("-movflags").arg("+faststart") - // Force a keyframe every 30 frames (1s @ 30fps) so a clean - // stop has a recent IDR — keeps the final mp4 from being - // unplayable if the encoder hadn't emitted a keyframe yet. - .arg("-g").arg("30") - .arg(&output_path); - - // We need stdin to send `q\n` for a clean shutdown that - // finalizes the moov atom. Stderr is captured for diagnostics on - // an unexpected exit; stdout is silenced (ffmpeg writes nothing - // useful to stdout in this mode). - cmd.stdin(Stdio::piped()) - .stdout(Stdio::null()) - .stderr(Stdio::piped()); - - let mut child = cmd.spawn().map_err(|e| { - anyhow::anyhow!("Failed to spawn ffmpeg ({}): {e}", ffmpeg.display()) - })?; - - // Spawn a thread that drains stderr into a buffer. ffmpeg can be - // verbose even with `-loglevel error` (filter setup warnings, - // codec init notes), and a full stderr pipe blocks the encoder. - // We keep the tail for diagnostics on a non-zero exit. - let stderr_thread = child.stderr.take().map(|mut stderr| { - std::thread::spawn(move || -> Vec { - use std::io::Read; - let mut buf = Vec::with_capacity(4096); - let _ = stderr.read_to_end(&mut buf); - // Cap at last 4 KB so we don't unboundedly grow. - let len = buf.len(); - if len > 4096 { - buf.drain(..len - 4096); - } - buf - }) - }); - - // Fast-fail probe. ffmpeg failures we care about: - // 1. immediate exit (bad input device, missing codec) — surface stderr - // 2. macOS: silent hang waiting on a TCC Screen Recording prompt that - // can't be displayed (subprocess of a daemon) — detected via - // no-frame-progress after 2 s. - let probe_deadline = Instant::now() + Duration::from_millis(1500); - loop { - match child.try_wait() { - Ok(Some(status)) => { - let tail = stderr_thread - .map(|h| h.join().unwrap_or_default()) - .unwrap_or_default(); - let tail_str = String::from_utf8_lossy(&tail); - let _ = child.kill(); - anyhow::bail!( - "ffmpeg exited immediately ({status}). stderr tail:\n{tail_str}" - ); - } - Ok(None) => { - if Instant::now() >= probe_deadline { - break; - } - std::thread::sleep(Duration::from_millis(100)); - } - Err(e) => anyhow::bail!("ffmpeg try_wait failed: {e}"), - } - } - - // macOS-specific: ffmpeg + avfoundation Screen Recording permission - // is per-binary, NOT inherited from the parent process. When a - // daemon spawns ffmpeg the OS can't surface the consent dialog and - // the subprocess blocks forever. Detect via no-output-file-grew - // 2 s in, kill the child, return an actionable error. - #[cfg(target_os = "macos")] - { - std::thread::sleep(Duration::from_millis(500)); - let progressed = std::fs::metadata(&output_path) - .map(|m| m.len() > 0) - .unwrap_or(false); - if !progressed { - let _ = child.kill(); - let _ = child.wait(); - let tail = stderr_thread - .map(|h| h.join().unwrap_or_default()) - .unwrap_or_default(); - let tail_str = String::from_utf8_lossy(&tail); - anyhow::bail!( - "ffmpeg appears to be blocked on the macOS Screen Recording TCC \ - prompt. Open System Settings → Privacy & Security → Screen & \ - System Audio Recording and grant access to your ffmpeg binary \ - (e.g. /opt/homebrew/bin/ffmpeg), then restart cua-driver. Note: \ - cua-driver itself having Screen Recording permission is NOT \ - sufficient — the ffmpeg subprocess needs its own grant. A \ - follow-up PR will replace ffmpeg with ScreenCaptureKit on macOS \ - to remove this requirement. ffmpeg path: {ffmpeg_path}. stderr \ - tail:\n{tail_str}", - ffmpeg_path = ffmpeg.display() - ); - } - } - - Ok(VideoRecorder { - child, - output_path, - started_at: Instant::now(), - stderr_thread, - }) - } - - /// Gracefully terminate ffmpeg. Sends `q\n` on stdin (ffmpeg's clean - /// shutdown trigger) and waits up to ~3 s for it to exit. Falls back - /// to `kill()` if the polite path stalls — that leaves the mp4 - /// non-finalized (no moov atom), which we report as `finalized: - /// false` so the caller can decide what to do with it. - pub fn stop(mut self) -> anyhow::Result { - let elapsed = self.started_at.elapsed(); - let finalized; - - // Send the quit signal. If the stdin pipe is already dropped - // (ffmpeg crashed early), proceed to the wait/kill path. - if let Some(mut stdin) = self.child.stdin.take() { - let _ = stdin.write_all(b"q\n"); - let _ = stdin.flush(); - // Drop closes the pipe — ffmpeg's input loop sees EOF and - // exits cleanly. - } - - // Poll for clean exit up to ~3 s. - let deadline = Instant::now() + Duration::from_millis(3000); - loop { - match self.child.try_wait()? { - Some(status) => { - finalized = status.success(); - break; - } - None => { - if Instant::now() > deadline { - // Polite shutdown stalled — force kill. - let _ = self.child.kill(); - let _ = self.child.wait(); - finalized = false; - break; - } - std::thread::sleep(Duration::from_millis(80)); - } - } - } - - // If ffmpeg didn't finalize cleanly, surface the tail of stderr - // to the trace log so failures don't disappear into silence. - if !finalized { - if let Some(handle) = self.stderr_thread.take() { - if let Ok(buf) = handle.join() { - let tail = String::from_utf8_lossy(&buf); - tracing::warn!(target: "recording", - "ffmpeg did not finalize cleanly. Last stderr tail:\n{tail}"); - } - } - } else { - // Still join the thread so the OS handle gets cleaned up. - if let Some(handle) = self.stderr_thread.take() { - let _ = handle.join(); - } - } - - Ok(VideoMetadata { - path: self.output_path, - duration_ms: elapsed.as_millis() as u64, - finalized, - }) - } +/// One active capture session. Owned by `RecordingSession` for the +/// session's lifetime; `stop()` consumes it and finalizes the file. +pub trait VideoBackend: Send { + fn stop(self: Box) -> anyhow::Result; } -/// Locate `ffprobe` using the same PATH + package-manager fallback the -/// `find_ffmpeg` lookup uses. ffprobe ships next to ffmpeg in every -/// build I've seen, so we just transform the resolved ffmpeg path. -pub fn find_ffprobe() -> Option { - let ffmpeg = find_ffmpeg()?; - // PATH lookup form returns just "ffmpeg" — assume "ffprobe" is on - // the same PATH. - if ffmpeg.parent().map(|p| p.as_os_str().is_empty()).unwrap_or(true) { - return Some(PathBuf::from("ffprobe")); - } - // File-path form: swap the filename. - let mut p = ffmpeg.clone(); - p.set_file_name(if cfg!(target_os = "windows") { "ffprobe.exe" } else { "ffprobe" }); - if p.exists() { Some(p) } else { None } +/// Spawns a fresh `VideoBackend` writing to `output_path`. Registered +/// once at startup via `set_video_backend_factory`. +pub trait VideoBackendFactory: Send + Sync { + fn start(&self, output_path: &Path) -> anyhow::Result>; } -/// Locate the ffmpeg binary. Tries `ffmpeg` (PATH lookup) first, then a -/// few well-known package-manager install paths so a freshly winget / -/// brew / apt-installed ffmpeg works without a shell restart. -pub(crate) fn find_ffmpeg() -> Option { - // 1. On PATH via the OS resolver. - if Command::new("ffmpeg").arg("-version") - .stdout(Stdio::null()).stderr(Stdio::null()) - .status().map(|s| s.success()).unwrap_or(false) - { - return Some(PathBuf::from("ffmpeg")); - } +static VIDEO_BACKEND_FACTORY: OnceLock> = OnceLock::new(); - // 2. Well-known install locations. - #[cfg(target_os = "windows")] - { - // winget Gyan.FFmpeg drops a versioned dir under WinGet/Packages. - if let Ok(local_appdata) = std::env::var("LOCALAPPDATA") { - let pkg_root = PathBuf::from(local_appdata) - .join("Microsoft/WinGet/Packages"); - if let Ok(entries) = std::fs::read_dir(&pkg_root) { - for e in entries.flatten() { - let name = e.file_name(); - if name.to_string_lossy().starts_with("Gyan.FFmpeg") { - if let Ok(sub_entries) = std::fs::read_dir(e.path()) { - for sub in sub_entries.flatten() { - let cand = sub.path().join("bin").join("ffmpeg.exe"); - if cand.exists() { - return Some(cand); - } - } - } - } - } - } - } - // Also check chocolatey / scoop default locations. - for p in &[ - "C:/ProgramData/chocolatey/bin/ffmpeg.exe", - "C:/tools/ffmpeg/bin/ffmpeg.exe", - ] { - let pb = PathBuf::from(p); - if pb.exists() { return Some(pb); } - } - } - - #[cfg(target_os = "macos")] - { - for p in &[ - "/opt/homebrew/bin/ffmpeg", - "/usr/local/bin/ffmpeg", - "/usr/bin/ffmpeg", - ] { - let pb = PathBuf::from(p); - if pb.exists() { return Some(pb); } - } - } - - #[cfg(target_os = "linux")] - { - for p in &[ - "/usr/bin/ffmpeg", - "/usr/local/bin/ffmpeg", - "/snap/bin/ffmpeg", - ] { - let pb = PathBuf::from(p); - if pb.exists() { return Some(pb); } - } - } - - None +/// Register the platform's video backend. Idempotent — subsequent calls +/// are silently ignored, matching the other recording-callback setters. +pub fn set_video_backend_factory(factory: Box) { + let _ = VIDEO_BACKEND_FACTORY.set(factory); } -/// Append the platform-appropriate `-framerate N -f -i ` -/// flags to an in-progress ffmpeg command. -fn platform_input_args(cmd: &mut Command) { - let framerate = "30"; - - #[cfg(target_os = "windows")] - { - // gdigrab is the bundled GDI screen-grab device on Windows. `desktop` - // captures the entire virtual desktop; switch to `title=` - // for per-window capture (deferred — main display is the Swift - // parity behavior). - cmd.arg("-f").arg("gdigrab") - .arg("-framerate").arg(framerate) - .arg("-draw_mouse").arg("1") // include the OS cursor - .arg("-i").arg("desktop"); - } - - #[cfg(target_os = "macos")] - { - // avfoundation: "1" = main display, ":" = no audio input. The - // exact index of the main display can vary; the Swift impl - // selected it programmatically through SCShareableContent. ffmpeg - // accepts `default` as an alias when present (newer macOS), with - // "1" as the historical convention. - cmd.arg("-f").arg("avfoundation") - .arg("-framerate").arg(framerate) - .arg("-pix_fmt").arg("uyvy422") // avfoundation's native fmt - .arg("-i").arg("1:"); - } - - #[cfg(target_os = "linux")] - { - // x11grab uses the DISPLAY env var. Wayland users need a - // different backend (pipewire / wf-recorder) — left as a TODO. - let display = std::env::var("DISPLAY").unwrap_or_else(|_| ":0.0".into()); - cmd.arg("-f").arg("x11grab") - .arg("-framerate").arg(framerate) - .arg("-i").arg(display); - } +/// Start a video capture using the registered backend. Returns an error +/// when no backend has been registered for this platform (treated by +/// `RecordingSession` as "video failed to start" — the per-turn pipeline +/// keeps running). +pub fn start_video(output_path: &Path) -> anyhow::Result> { + let factory = VIDEO_BACKEND_FACTORY + .get() + .ok_or_else(|| anyhow::anyhow!("no video backend registered for this platform"))?; + factory.start(output_path) } diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/video_ffmpeg.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/video_ffmpeg.rs new file mode 100644 index 0000000000..dc4e80f225 --- /dev/null +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/video_ffmpeg.rs @@ -0,0 +1,290 @@ +//! ffmpeg-subprocess video backend (Windows + Linux). +//! +//! macOS uses native ScreenCaptureKit in `platform_macos::video_sckit` +//! — that backend doesn't need ffmpeg and doesn't carry the TCC +//! per-binary subprocess gotcha. This file is the cross-platform +//! fallback for OSes where we ship a subprocess pipeline instead. +//! +//! Inputs per OS: +//! - **Windows:** `gdigrab` (full virtual desktop) +//! - **Linux:** `x11grab` (`$DISPLAY` env var, defaults to `:0.0`) +//! +//! Encoder is `libx264 -preset ultrafast -pix_fmt yuv420p` everywhere. +//! Lifecycle: spawn → caller stays alive → send `q\n` on stdin for a +//! clean shutdown that finalizes the moov atom (force-kill on stall). + +use std::io::Write; +use std::path::{Path, PathBuf}; +use std::process::{Child, Command, Stdio}; +use std::time::{Duration, Instant}; + +use crate::video::{VideoBackend, VideoBackendFactory, VideoMetadata}; + +pub struct FfmpegVideoBackendFactory; + +impl VideoBackendFactory for FfmpegVideoBackendFactory { + fn start(&self, output_path: &Path) -> anyhow::Result> { + FfmpegVideoBackend::start(output_path).map(|b| Box::new(b) as Box) + } +} + +pub struct FfmpegVideoBackend { + child: Child, + output_path: PathBuf, + started_at: Instant, + /// Drains ffmpeg's stderr so its pipe doesn't fill up and block the + /// encoder. The collected tail is read in `stop()` for diagnostics + /// when ffmpeg exits non-zero. + stderr_thread: Option>>, +} + +impl FfmpegVideoBackend { + fn start(output_path: &Path) -> anyhow::Result { + let output_path = output_path.to_path_buf(); + + let ffmpeg = match find_ffmpeg() { + Some(p) => p, + None => anyhow::bail!( + "ffmpeg not found on PATH. Install with: \ + winget install Gyan.FFmpeg (Windows) or \ + apt install ffmpeg (Linux)." + ), + }; + + if let Some(parent) = output_path.parent() { + std::fs::create_dir_all(parent).ok(); + } + + let mut cmd = Command::new(&ffmpeg); + cmd.arg("-y") + .arg("-loglevel").arg("error"); + + platform_input_args(&mut cmd); + + // yuv420p needs even dimensions; pad rather than crop so the full + // display stays in frame on odd resolutions (e.g. 1512×949 Win11). + cmd.arg("-vf").arg("pad=ceil(iw/2)*2:ceil(ih/2)*2"); + + cmd.arg("-c:v").arg("libx264") + .arg("-preset").arg("ultrafast") + .arg("-pix_fmt").arg("yuv420p") + .arg("-movflags").arg("+faststart") + .arg("-g").arg("30") + .arg(&output_path); + + cmd.stdin(Stdio::piped()) + .stdout(Stdio::null()) + .stderr(Stdio::piped()); + + let mut child = cmd.spawn().map_err(|e| { + anyhow::anyhow!("Failed to spawn ffmpeg ({}): {e}", ffmpeg.display()) + })?; + + let stderr_thread = child.stderr.take().map(|mut stderr| { + std::thread::spawn(move || -> Vec { + use std::io::Read; + let mut buf = Vec::with_capacity(4096); + let _ = stderr.read_to_end(&mut buf); + let len = buf.len(); + if len > 4096 { + buf.drain(..len - 4096); + } + buf + }) + }); + + // Fast-fail probe — surface stderr immediately when ffmpeg dies + // on startup (bad input device, missing codec). Without this the + // recording session would record a useless empty mp4. + let probe_deadline = Instant::now() + Duration::from_millis(1500); + loop { + match child.try_wait() { + Ok(Some(status)) => { + let tail = stderr_thread + .map(|h| h.join().unwrap_or_default()) + .unwrap_or_default(); + let tail_str = String::from_utf8_lossy(&tail); + let _ = child.kill(); + anyhow::bail!( + "ffmpeg exited immediately ({status}). stderr tail:\n{tail_str}" + ); + } + Ok(None) => { + if Instant::now() >= probe_deadline { + break; + } + std::thread::sleep(Duration::from_millis(100)); + } + Err(e) => anyhow::bail!("ffmpeg try_wait failed: {e}"), + } + } + + Ok(FfmpegVideoBackend { + child, + output_path, + started_at: Instant::now(), + stderr_thread, + }) + } +} + +impl VideoBackend for FfmpegVideoBackend { + fn stop(mut self: Box) -> anyhow::Result { + let elapsed = self.started_at.elapsed(); + let finalized; + + if let Some(mut stdin) = self.child.stdin.take() { + let _ = stdin.write_all(b"q\n"); + let _ = stdin.flush(); + } + + let deadline = Instant::now() + Duration::from_millis(3000); + loop { + match self.child.try_wait()? { + Some(status) => { + finalized = status.success(); + break; + } + None => { + if Instant::now() > deadline { + // Polite shutdown stalled — force kill. mp4 will lack + // a moov atom and won't be playable; `finalized: + // false` tells the caller. + let _ = self.child.kill(); + let _ = self.child.wait(); + finalized = false; + break; + } + std::thread::sleep(Duration::from_millis(80)); + } + } + } + + if !finalized { + if let Some(handle) = self.stderr_thread.take() { + if let Ok(buf) = handle.join() { + let tail = String::from_utf8_lossy(&buf); + tracing::warn!(target: "recording", + "ffmpeg did not finalize cleanly. Last stderr tail:\n{tail}"); + } + } + } else if let Some(handle) = self.stderr_thread.take() { + let _ = handle.join(); + } + + Ok(VideoMetadata { + path: self.output_path, + duration_ms: elapsed.as_millis() as u64, + finalized, + }) + } +} + +/// Locate `ffprobe` next to the resolved `ffmpeg`. Used by the recording +/// renderer to probe mp4 duration. ffprobe ships next to ffmpeg in every +/// distro I've seen. +pub fn find_ffprobe() -> Option { + let ffmpeg = find_ffmpeg()?; + if ffmpeg.parent().map(|p| p.as_os_str().is_empty()).unwrap_or(true) { + return Some(PathBuf::from("ffprobe")); + } + let mut p = ffmpeg.clone(); + p.set_file_name(if cfg!(target_os = "windows") { "ffprobe.exe" } else { "ffprobe" }); + if p.exists() { Some(p) } else { None } +} + +pub(crate) fn find_ffmpeg() -> Option { + if Command::new("ffmpeg").arg("-version") + .stdout(Stdio::null()).stderr(Stdio::null()) + .status().map(|s| s.success()).unwrap_or(false) + { + return Some(PathBuf::from("ffmpeg")); + } + + #[cfg(target_os = "windows")] + { + if let Ok(local_appdata) = std::env::var("LOCALAPPDATA") { + let pkg_root = PathBuf::from(local_appdata) + .join("Microsoft/WinGet/Packages"); + if let Ok(entries) = std::fs::read_dir(&pkg_root) { + for e in entries.flatten() { + let name = e.file_name(); + if name.to_string_lossy().starts_with("Gyan.FFmpeg") { + if let Ok(sub_entries) = std::fs::read_dir(e.path()) { + for sub in sub_entries.flatten() { + let cand = sub.path().join("bin").join("ffmpeg.exe"); + if cand.exists() { + return Some(cand); + } + } + } + } + } + } + } + for p in &[ + "C:/ProgramData/chocolatey/bin/ffmpeg.exe", + "C:/tools/ffmpeg/bin/ffmpeg.exe", + ] { + let pb = PathBuf::from(p); + if pb.exists() { return Some(pb); } + } + } + + #[cfg(target_os = "linux")] + { + for p in &[ + "/usr/bin/ffmpeg", + "/usr/local/bin/ffmpeg", + "/snap/bin/ffmpeg", + ] { + let pb = PathBuf::from(p); + if pb.exists() { return Some(pb); } + } + } + + #[cfg(target_os = "macos")] + { + // ffmpeg backend isn't wired on macOS, but recording_render still + // calls find_ffprobe to inspect existing mp4s. Keep the macOS + // probe so that path keeps working when ffmpeg is installed. + for p in &[ + "/opt/homebrew/bin/ffmpeg", + "/usr/local/bin/ffmpeg", + "/usr/bin/ffmpeg", + ] { + let pb = PathBuf::from(p); + if pb.exists() { return Some(pb); } + } + } + + None +} + +fn platform_input_args(cmd: &mut Command) { + let framerate = "30"; + + #[cfg(target_os = "windows")] + { + cmd.arg("-f").arg("gdigrab") + .arg("-framerate").arg(framerate) + .arg("-draw_mouse").arg("1") + .arg("-i").arg("desktop"); + } + + #[cfg(target_os = "linux")] + { + let display = std::env::var("DISPLAY").unwrap_or_else(|_| ":0.0".into()); + cmd.arg("-f").arg("x11grab") + .arg("-framerate").arg(framerate) + .arg("-i").arg(display); + } + + // macOS not wired here — the macOS factory is `SckitVideoBackendFactory` + // in platform-macos and the ffmpeg backend is never registered. + #[cfg(target_os = "macos")] + { + let _ = framerate; + let _ = cmd; + } +} diff --git a/libs/cua-driver/rust/crates/cua-driver/build.rs b/libs/cua-driver/rust/crates/cua-driver/build.rs new file mode 100644 index 0000000000..387b7c3f56 --- /dev/null +++ b/libs/cua-driver/rust/crates/cua-driver/build.rs @@ -0,0 +1,29 @@ +// Bake Swift runtime rpaths into the cua-driver binary on macOS. +// +// The `screencapturekit` dep ships a small Swift-bridge shim that links +// against the Swift Concurrency runtime (`@rpath/libswift_Concurrency.dylib` +// and friends). Its own build.rs emits `cargo:rustc-link-arg=-Wl,-rpath,…` +// directives, but those only flow through to the binary linker when the +// emitting crate is the final binary crate — for transitive deps Cargo +// silently drops them. So we re-emit the same rpaths from here. +// +// No-op on Windows / Linux — those builds don't pull in a Swift runtime. + +fn main() { + if std::env::var("CARGO_CFG_TARGET_OS").as_deref() != Ok("macos") { + return; + } + println!("cargo:rustc-link-arg=-Wl,-rpath,/usr/lib/swift"); + + if let Ok(out) = std::process::Command::new("xcode-select").arg("-p").output() { + if out.status.success() { + let xcode_path = String::from_utf8_lossy(&out.stdout).trim().to_string(); + for sub in [ + "Toolchains/XcodeDefault.xctoolchain/usr/lib/swift/macosx", + "Toolchains/XcodeDefault.xctoolchain/usr/lib/swift-5.5/macosx", + ] { + println!("cargo:rustc-link-arg=-Wl,-rpath,{xcode_path}/{sub}"); + } + } + } +} diff --git a/libs/cua-driver/rust/crates/cua-driver/src/main.rs b/libs/cua-driver/rust/crates/cua-driver/src/main.rs index 2402ee9cb5..88437a7aae 100644 --- a/libs/cua-driver/rust/crates/cua-driver/src/main.rs +++ b/libs/cua-driver/rust/crates/cua-driver/src/main.rs @@ -128,6 +128,9 @@ fn main() { cua_driver_core::recording::set_element_bounds_fn(|wid, pid, idx| { platform_macos::recording_hooks::element_window_local_xy(wid, pid, idx) }); + cua_driver_core::video::set_video_backend_factory( + Box::new(platform_macos::video_sckit::SckitVideoBackendFactory), + ); let reg = Arc::new(platform_macos::register_tools()); reg.init_self_weak(); cli::run_call(reg, &tool, json_args, screenshot_out_file, socket); @@ -175,6 +178,9 @@ fn main() { cua_driver_core::recording::set_element_bounds_fn(|wid, pid, idx| { platform_macos::recording_hooks::element_window_local_xy(wid, pid, idx) }); + cua_driver_core::video::set_video_backend_factory( + Box::new(platform_macos::video_sckit::SckitVideoBackendFactory), + ); let reg = Arc::new(platform_macos::register_tools()); reg.init_self_weak(); let sp = socket.unwrap_or_else(serve::default_socket_path); @@ -305,6 +311,9 @@ fn main() { cua_driver_core::recording::set_element_bounds_fn(|wid, pid, idx| { platform_macos::recording_hooks::element_window_local_xy(wid, pid, idx) }); + cua_driver_core::video::set_video_backend_factory( + Box::new(platform_macos::video_sckit::SckitVideoBackendFactory), + ); std::thread::Builder::new() .name("cua-mcp".into()) @@ -546,6 +555,9 @@ fn build_registry(cursor_cfg: cursor_overlay::CursorConfig) -> cua_driver_core:: cua_driver_core::recording::set_element_bounds_fn(|wid, pid, idx| { platform_windows::recording_hooks::element_window_local_xy(wid, pid, idx) }); + cua_driver_core::video::set_video_backend_factory( + Box::new(cua_driver_core::video_ffmpeg::FfmpegVideoBackendFactory), + ); platform_windows::register_tools_with_cursor(cursor_cfg, compat) } #[cfg(target_os = "linux")] @@ -565,6 +577,9 @@ fn build_registry(cursor_cfg: cursor_overlay::CursorConfig) -> cua_driver_core:: cua_driver_core::recording::set_click_marker_fn(|png_bytes, cx, cy| { platform_linux::capture::crosshair_png_bytes(png_bytes, cx, cy).ok() }); + cua_driver_core::video::set_video_backend_factory( + Box::new(cua_driver_core::video_ffmpeg::FfmpegVideoBackendFactory), + ); platform_linux::register_tools_with_cursor(cursor_cfg, compat) } #[cfg(not(any(target_os = "windows", target_os = "linux")))] @@ -605,6 +620,9 @@ fn build_registry_no_cursor() -> cua_driver_core::tool::ToolRegistry { cua_driver_core::recording::set_element_bounds_fn(|wid, pid, idx| { platform_windows::recording_hooks::element_window_local_xy(wid, pid, idx) }); + cua_driver_core::video::set_video_backend_factory( + Box::new(cua_driver_core::video_ffmpeg::FfmpegVideoBackendFactory), + ); platform_windows::register_tools_with_cursor(cursor_overlay::CursorConfig { enabled: false, ..Default::default() }, compat) } #[cfg(target_os = "linux")] @@ -624,6 +642,9 @@ fn build_registry_no_cursor() -> cua_driver_core::tool::ToolRegistry { cua_driver_core::recording::set_click_marker_fn(|png_bytes, cx, cy| { platform_linux::capture::crosshair_png_bytes(png_bytes, cx, cy).ok() }); + cua_driver_core::video::set_video_backend_factory( + Box::new(cua_driver_core::video_ffmpeg::FfmpegVideoBackendFactory), + ); platform_linux::register_tools_with_cursor(cursor_overlay::CursorConfig { enabled: false, ..Default::default() }, compat) } #[cfg(not(any(target_os = "windows", target_os = "linux")))] diff --git a/libs/cua-driver/rust/crates/platform-macos/Cargo.toml b/libs/cua-driver/rust/crates/platform-macos/Cargo.toml index 3f53e31d1d..0f46e869c1 100644 --- a/libs/cua-driver/rust/crates/platform-macos/Cargo.toml +++ b/libs/cua-driver/rust/crates/platform-macos/Cargo.toml @@ -43,6 +43,12 @@ objc2-app-kit = { version = "0.2", features = [ # QuartzCore for CALayer objc2-quartz-core = { version = "0.2", features = ["CALayer"] } +# Native ScreenCaptureKit bindings — used by `video_sckit` to replace the +# ffmpeg subprocess on macOS. `macos_15_0` enables the `SCRecordingOutput` +# convenience that finalises an mp4 in-process, removing the per-binary +# Screen Recording TCC prompt that subprocess capture would otherwise trip. +screencapturekit = { version = "6", features = ["macos_15_0"] } + # tiny-skia for cursor rendering tiny-skia = { version = "0.11", default-features = false, features = ["std"] } diff --git a/libs/cua-driver/rust/crates/platform-macos/src/lib.rs b/libs/cua-driver/rust/crates/platform-macos/src/lib.rs index f6afcc35c5..b1ee523510 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/lib.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/lib.rs @@ -32,6 +32,8 @@ pub mod window_change_detector; pub mod tools; #[cfg(target_os = "macos")] pub mod recording_hooks; +#[cfg(target_os = "macos")] +pub mod video_sckit; use cua_driver_core::tool::ToolRegistry; diff --git a/libs/cua-driver/rust/crates/platform-macos/src/video_sckit.rs b/libs/cua-driver/rust/crates/platform-macos/src/video_sckit.rs new file mode 100644 index 0000000000..c8e394ef1e --- /dev/null +++ b/libs/cua-driver/rust/crates/platform-macos/src/video_sckit.rs @@ -0,0 +1,149 @@ +//! Native ScreenCaptureKit video backend (macOS). +//! +//! Replaces the ffmpeg subprocess pipeline with an in-process SCStream + +//! SCRecordingOutput. The key win is TCC: ScreenCaptureKit runs in the +//! same process as cua-driver, so it inherits the daemon's Screen +//! Recording grant. No per-binary subprocess gotcha, no second prompt, +//! no fast-fail-on-hang heuristic. +//! +//! Requires macOS 15.0+ (SCRecordingOutput introduced in macOS 15). The +//! Swift impl this is modelled on lives at +//! `libs/cua-driver/swift/Sources/CuaDriverCore/Recording/VideoRecorder.swift`, +//! though that version composes SCStream + AVAssetWriter manually so it +//! also runs on macOS 14. We use SCRecordingOutput here because the +//! Rust binding doesn't expose AVAssetWriter and macOS 15 is already +//! widespread enough that requiring it is acceptable for the Rust port. +//! +//! Lifecycle: +//! 1. `start(path)` resolves the main display, builds a 30fps full-display +//! SCStream config + SCRecordingOutput pointing at the mp4 path, +//! attaches the recording output, calls `start_capture()`. +//! 2. Caller stays alive while recording. +//! 3. `stop()` calls `stop_capture()` (which finalises the mp4 moov +//! atom) and returns the elapsed-time metadata. + +use std::path::Path; +use std::time::Instant; + +use cua_driver_core::video::{VideoBackend, VideoBackendFactory, VideoMetadata}; + +use screencapturekit::prelude::{ + SCContentFilter, SCShareableContent, SCStream, SCStreamConfiguration, +}; +use screencapturekit::recording_output::{ + SCRecordingOutput, SCRecordingOutputCodec, SCRecordingOutputConfiguration, + SCRecordingOutputFileType, +}; + +pub struct SckitVideoBackendFactory; + +impl VideoBackendFactory for SckitVideoBackendFactory { + fn start(&self, output_path: &Path) -> anyhow::Result> { + SckitVideoBackend::start(output_path).map(|b| Box::new(b) as Box) + } +} + +pub struct SckitVideoBackend { + stream: SCStream, + // SCStream's add_recording_output is non-owning — Apple's API requires + // the SCRecordingOutput stay alive for the stream's lifetime, so we + // keep it parked here. Dropping it before stop_capture aborts the + // encode mid-file. + _recording: SCRecordingOutput, + output_path: std::path::PathBuf, + started_at: Instant, +} + +impl SckitVideoBackend { + fn start(output_path: &Path) -> anyhow::Result { + if let Some(parent) = output_path.parent() { + std::fs::create_dir_all(parent).ok(); + } + // SCRecordingOutput appends-or-fails on an existing file; match the + // Swift impl by clearing any stale recording.mp4 from a prior run. + let _ = std::fs::remove_file(output_path); + + let content = SCShareableContent::get() + .map_err(|e| anyhow::anyhow!("SCShareableContent::get failed: {e}"))?; + let displays = content.displays(); + let display = displays + .into_iter() + .next() + .ok_or_else(|| anyhow::anyhow!("no displays available for ScreenCaptureKit"))?; + + let filter = SCContentFilter::create() + .with_display(&display) + .with_excluding_windows(&[]) + .build(); + + // Match the Swift recorder's pixel resolution + 30fps target. The + // display's reported width/height are in pixels (already + // backing-scale-multiplied) on SCDisplay, so passing them through + // gives a native-resolution capture. + let pixel_width = display.width(); + let pixel_height = display.height(); + let frame_interval = screencapturekit::cm::CMTime::new(1, 30); + let config = SCStreamConfiguration::new() + .with_width(pixel_width) + .with_height(pixel_height) + .with_minimum_frame_interval(&frame_interval) + .with_shows_cursor(true); + + let rec_config = SCRecordingOutputConfiguration::new() + .with_output_url(output_path) + .with_video_codec(SCRecordingOutputCodec::H264) + .with_output_file_type(SCRecordingOutputFileType::MP4); + + let recording = SCRecordingOutput::new(&rec_config).ok_or_else(|| { + anyhow::anyhow!( + "SCRecordingOutput::new returned nil — macOS 15.0+ is required for \ + native ScreenCaptureKit video; older macOS needs to use the ffmpeg \ + backend (currently disabled on macOS)." + ) + })?; + + let stream = SCStream::new(&filter, &config); + stream + .add_recording_output(&recording) + .map_err(|e| anyhow::anyhow!("SCStream::add_recording_output failed: {e}"))?; + stream + .start_capture() + .map_err(|e| anyhow::anyhow!("SCStream::start_capture failed: {e}"))?; + + tracing::info!( + target: "recording", + path = %output_path.display(), + width = pixel_width, + height = pixel_height, + "sckit video capture started" + ); + + Ok(Self { + stream, + _recording: recording, + output_path: output_path.to_path_buf(), + started_at: Instant::now(), + }) + } +} + +impl VideoBackend for SckitVideoBackend { + fn stop(self: Box) -> anyhow::Result { + let elapsed = self.started_at.elapsed(); + // SCStream::stop_capture finalises the mp4 moov atom synchronously + // on the recording output before returning. Errors here mean the + // file may be unplayable — surface as `finalized: false`. + let finalized = self.stream.stop_capture().is_ok(); + if !finalized { + tracing::warn!( + target: "recording", + "SCStream::stop_capture failed; recording.mp4 may be incomplete" + ); + } + Ok(VideoMetadata { + path: self.output_path, + duration_ms: elapsed.as_millis() as u64, + finalized, + }) + } +} From 1cfdd2b72b84fbc03560e475202e7c3a7f473f6b Mon Sep 17 00:00:00 2001 From: Francesco Bonacci Date: Tue, 26 May 2026 23:26:56 +0200 Subject: [PATCH 3/3] fix(cua-driver-rs)(recording): address CodeRabbit findings on PR #1720 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 7 CodeRabbit nits across the recording refactor: 1. recording_tools.rs — `record_video` schema description now says macOS uses ScreenCaptureKit, only Win/Linux need ffmpeg (was contradicting the description text just above) 2. recording.rs — `element_index` u64→u32 narrowing now uses `u32::try_from(...).ok().and_then(...)` instead of silent `as` cast 3. video_ffmpeg.rs — `create_dir_all` error now propagates with context instead of being swallowed 4. platform-macos/recording_hooks.rs — i64→i32 and u64→u32 conversions for pid + window_id now use checked `try_from` 5. video_sckit.rs — fs::create_dir_all error propagates; fs::remove_file ignores only NotFound, propagates other IO errors 6. platform-windows/recording_hooks.rs — same checked conversions for pid + window_id casts 7. RECORDING.md — header no longer claims "macOS-only" since recording now ships cross-platform via SCKit/ffmpeg Build: `cargo build --release -p cua-driver` green on macOS; `cargo check -p platform-windows --target=x86_64-pc-windows-msvc` green. --- .../rust/Skills/cua-driver/RECORDING.md | 10 ++++------ .../crates/cua-driver-core/src/recording.rs | 4 +++- .../cua-driver-core/src/recording_tools.rs | 4 +++- .../crates/cua-driver-core/src/video_ffmpeg.rs | 7 ++++++- .../platform-macos/src/recording_hooks.rs | 12 +++++++----- .../crates/platform-macos/src/video_sckit.rs | 18 ++++++++++++++++-- .../platform-windows/src/recording_hooks.rs | 7 ++++--- 7 files changed, 43 insertions(+), 19 deletions(-) diff --git a/libs/cua-driver/rust/Skills/cua-driver/RECORDING.md b/libs/cua-driver/rust/Skills/cua-driver/RECORDING.md index 8482fecdee..a7b2e55a98 100644 --- a/libs/cua-driver/rust/Skills/cua-driver/RECORDING.md +++ b/libs/cua-driver/rust/Skills/cua-driver/RECORDING.md @@ -1,11 +1,9 @@ # Recording & replaying trajectories -> **Platform: macOS-only today.** Trajectory recording / replay is -> currently implemented on the macOS backend only. On Windows, `cua-driver -> recording {start,stop,status}` is registered but returns "Recording is -> currently macOS-only". On Linux (BETA): not supported. See `WINDOWS.md` -> / `LINUX.md` for capture-state alternatives via `screenshot` and -> `get_window_state`. +> **Cross-platform.** Recording is available on macOS (native +> ScreenCaptureKit), Windows (ffmpeg + `gdigrab`), and Linux (ffmpeg + +> `x11grab`). Replay is cross-platform as long as the recorded artifacts +> are present. Session-scoped capture of action sequences + pre/post state, suitable for demos, regression diffs, and training data. Invoked only when the diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/recording.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/recording.rs index 11b816b444..423ad3ba5b 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/recording.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/recording.rs @@ -372,7 +372,9 @@ fn write_turn( match (args.opt_f64("x"), args.opt_f64("y")) { (Some(x), Some(y)) => Some((x, y)), _ => match (window_id, pid, element_index, ELEMENT_BOUNDS_FN.get()) { - (Some(wid), Some(p), Some(idx), Some(f)) => f(wid, p, idx as u32), + (Some(wid), Some(p), Some(idx), Some(f)) => { + u32::try_from(idx).ok().and_then(|idx32| f(wid, p, idx32)) + } _ => None, }, } diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/recording_tools.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/recording_tools.rs index cd2e02912b..e4fbc9b16e 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/recording_tools.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/recording_tools.rs @@ -97,7 +97,9 @@ impl Tool for StartRecordingTool { "type": "boolean", "description": "Capture the main display to /recording.mp4. \ Default: true. Set to false to record only the per-turn \ - screenshots + JSON. Requires ffmpeg on PATH." + screenshots + JSON. On macOS this uses native \ + ScreenCaptureKit (no extra TCC prompt, macOS 15.0+); on \ + Windows + Linux it requires ffmpeg on PATH." } }, "additionalProperties": false diff --git a/libs/cua-driver/rust/crates/cua-driver-core/src/video_ffmpeg.rs b/libs/cua-driver/rust/crates/cua-driver-core/src/video_ffmpeg.rs index dc4e80f225..4ab065d3db 100644 --- a/libs/cua-driver/rust/crates/cua-driver-core/src/video_ffmpeg.rs +++ b/libs/cua-driver/rust/crates/cua-driver-core/src/video_ffmpeg.rs @@ -52,7 +52,12 @@ impl FfmpegVideoBackend { }; if let Some(parent) = output_path.parent() { - std::fs::create_dir_all(parent).ok(); + std::fs::create_dir_all(parent).map_err(|e| { + anyhow::anyhow!( + "failed to create recording output directory {}: {e}", + parent.display() + ) + })?; } let mut cmd = Command::new(&ffmpeg); diff --git a/libs/cua-driver/rust/crates/platform-macos/src/recording_hooks.rs b/libs/cua-driver/rust/crates/platform-macos/src/recording_hooks.rs index ca5ca86bcc..ab79efa434 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/recording_hooks.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/recording_hooks.rs @@ -25,9 +25,9 @@ pub fn set_element_cache(cache: Arc) { /// (pid, window_id) and emits the same shape `get_window_state` returns /// (minus screenshot fields — those live in `screenshot.png`). pub fn app_state_json_for(window_id: Option, pid: Option) -> Option> { - let pid = pid? as i32; + let pid = i32::try_from(pid?).ok()?; let resolved_wid = match window_id { - Some(w) => w as u32, + Some(w) => u32::try_from(w).ok()?, None => crate::windows::resolve_main_window_id(pid).ok()?, }; let result = crate::ax::tree::walk_tree(pid, Some(resolved_wid), None); @@ -47,13 +47,15 @@ pub fn app_state_json_for(window_id: Option, pid: Option) -> Option Option<(f64, f64)> { let cache = ELEMENT_CACHE.get()?; - let ptr = cache.get_element_ptr(pid as i32, window_id as u32, element_index as usize)?; + let pid_i32 = i32::try_from(pid).ok()?; + let window_id_u32 = u32::try_from(window_id).ok()?; + let ptr = cache.get_element_ptr(pid_i32, window_id_u32, element_index as usize)?; let (sx, sy) = unsafe { element_screen_center(ptr as AXUIElementRef)? }; - let bounds = crate::windows::window_bounds_by_id(window_id as u32)?; + let bounds = crate::windows::window_bounds_by_id(window_id_u32)?; // Probe the captured PNG's width to derive the Retina scale — the // screenshot is in physical pixels, the window bounds are in points. - let scale = if let Ok(png) = crate::capture::screenshot_window_bytes(window_id as u32) { + let scale = if let Ok(png) = crate::capture::screenshot_window_bytes(window_id_u32) { if png.len() >= 24 { let pw = u32::from_be_bytes([png[16], png[17], png[18], png[19]]) as f64; if bounds.width > 0.0 && pw > bounds.width { pw / bounds.width } else { 1.0 } diff --git a/libs/cua-driver/rust/crates/platform-macos/src/video_sckit.rs b/libs/cua-driver/rust/crates/platform-macos/src/video_sckit.rs index c8e394ef1e..ce3841ec99 100644 --- a/libs/cua-driver/rust/crates/platform-macos/src/video_sckit.rs +++ b/libs/cua-driver/rust/crates/platform-macos/src/video_sckit.rs @@ -57,11 +57,25 @@ pub struct SckitVideoBackend { impl SckitVideoBackend { fn start(output_path: &Path) -> anyhow::Result { if let Some(parent) = output_path.parent() { - std::fs::create_dir_all(parent).ok(); + std::fs::create_dir_all(parent).map_err(|e| { + anyhow::anyhow!( + "failed to create recording output directory {}: {e}", + parent.display() + ) + })?; } // SCRecordingOutput appends-or-fails on an existing file; match the // Swift impl by clearing any stale recording.mp4 from a prior run. - let _ = std::fs::remove_file(output_path); + match std::fs::remove_file(output_path) { + Ok(()) => {} + Err(e) if e.kind() == std::io::ErrorKind::NotFound => {} + Err(e) => { + anyhow::bail!( + "failed to remove stale recording file {}: {e}", + output_path.display() + ); + } + } let content = SCShareableContent::get() .map_err(|e| anyhow::anyhow!("SCShareableContent::get failed: {e}"))?; diff --git a/libs/cua-driver/rust/crates/platform-windows/src/recording_hooks.rs b/libs/cua-driver/rust/crates/platform-windows/src/recording_hooks.rs index 2b738d860c..f587d311b8 100644 --- a/libs/cua-driver/rust/crates/platform-windows/src/recording_hooks.rs +++ b/libs/cua-driver/rust/crates/platform-windows/src/recording_hooks.rs @@ -22,7 +22,7 @@ pub fn set_element_cache(cache: Arc) { #[cfg(target_os = "windows")] pub fn app_state_json_for(window_id: Option, pid: Option) -> Option> { - let pid = pid? as u32; + let pid = u32::try_from(pid?).ok()?; let hwnd = match window_id { Some(w) => w, None => crate::win32::list_windows(Some(pid)).first().map(|w| w.hwnd)?, @@ -41,11 +41,12 @@ pub fn app_state_json_for(window_id: Option, pid: Option) -> Option Option<(f64, f64)> { let cache = ELEMENT_CACHE.get()?; - let (sx, sy) = cache.get_element_center(pid as u32, window_id, element_index as usize)?; + let pid_u32 = u32::try_from(pid).ok()?; + let (sx, sy) = cache.get_element_center(pid_u32, window_id, element_index as usize)?; // The cached center is in SCREEN coords. Convert to window-local pixel // coords by subtracting the window's screen origin (GetWindowRect-equivalent // in WindowInfo). Windows captures at logical pixels so no scale factor. - let wins = crate::win32::list_windows(Some(pid as u32)); + let wins = crate::win32::list_windows(Some(pid_u32)); let win = wins.iter().find(|w| w.hwnd == window_id)?; Some(((sx - win.x) as f64, (sy - win.y) as f64)) }