From 2aee8ca737399b773d2ea391e2ddd30b58448886 Mon Sep 17 00:00:00 2001 From: Jared Lunde Date: Thu, 24 Sep 2026 15:57:27 -0700 Subject: [PATCH 1/2] video: keep every screencast paint by default `Page.startScreencast` was started with `everyNthFrame: 2`. Chromium only emits a frame when the compositor paints, so on a page that changes once after recording starts there are exactly two paints, and the second one, the result, was the frame being skipped: the clip came back as a single frame of 100 ms. Seen four times in a row while an agent recorded a fixed page transition (a click, then the new page). A repro with three paints survived as two frames, which hid the problem for "before" clips. Every paint is now kept. A still page costs nothing however long the recording runs, an animating page runs at its own rate, and the existing frame cap bounds a long animation. Adds a live test: one DOM change between two pauses yields at least two frames and a clip that spans the pause. Co-Authored-By: Claude Fable 5.1 --- rust-native/tests/video_recording.rs | 46 ++++++++++++++++++++++++++++ src/video.rs | 16 ++++++---- 2 files changed, 56 insertions(+), 6 deletions(-) diff --git a/rust-native/tests/video_recording.rs b/rust-native/tests/video_recording.rs index 50b4cd4..b551c90 100644 --- a/rust-native/tests/video_recording.rs +++ b/rust-native/tests/video_recording.rs @@ -63,3 +63,49 @@ fn page_screencast_records_webm() { page.close(Default::default()).expect("close page"); browser.close().expect("close browser"); } + +/// A recording of one interaction has exactly two paints: the state before +/// and the result. Both must survive, or the clip is a single frame that shows +/// nothing (every_nth_frame 2 used to drop the second one). +#[test] +fn page_screencast_keeps_a_single_paint_after_start() { + let Some(browser) = launch() else { + return; + }; + let page = browser.new_page().expect("new page"); + page.goto( + "data:text/html,once

before

", + GotoOptions::default().wait_until("load").timeout(10_000.0), + ) + .expect("goto"); + let output = std::env::temp_dir().join(format!( + "rustwright-video-once-{}-{}.webm", + std::process::id(), + SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("time") + .as_nanos() + )); + page.start_video(output.to_string_lossy().as_ref(), VideoOptions::default()) + .expect("start video"); + std::thread::sleep(Duration::from_millis(800)); + page.evaluate( + "document.getElementById('ok').textContent = 'after'", + None, + ActionOptions::default(), + ) + .expect("one paint"); + std::thread::sleep(Duration::from_millis(800)); + let recording = page.stop_video().expect("stop video"); + assert!( + recording.frames >= 2, + "expected the frame before and the frame after the one paint, got {}", + recording.frames + ); + assert!( + recording.duration_ms >= 600, + "expected the clip to span the pause, got {} ms", + recording.duration_ms + ); + let _ = fs::remove_file(&output); +} diff --git a/src/video.rs b/src/video.rs index ab68219..3a9e1a2 100644 --- a/src/video.rs +++ b/src/video.rs @@ -23,10 +23,14 @@ pub const DEFAULT_VIDEO_QUALITY: u32 = 80; /// Longest captured edge in CSS pixels. Chromium scales the screencast so /// the longer side is at most this (800 keeps a 16:9 clip around 800×450). pub const DEFAULT_VIDEO_MAX_WIDTH: u32 = 800; -/// CDP `Page.startScreencast` has no fps. Chromium emits on compositor paint -/// (often ~60 Hz when the page is animating). `2` keeps every other paint, -/// which is ~30 fps from a 60 Hz source. -pub const DEFAULT_VIDEO_EVERY_NTH_FRAME: u32 = 2; +/// CDP `Page.startScreencast` has no fps. Chromium emits a frame only when +/// the compositor paints, so a still page costs nothing however long the +/// recording runs, and an animating page runs at its own rate (often ~60 Hz). +/// Keep every paint: skipping alternate frames made a recording of one +/// interaction (a click, then one paint for the result) come back as a single +/// frame, because the result's paint was the skipped one. The frame cap below +/// still bounds a long animation. +pub const DEFAULT_VIDEO_EVERY_NTH_FRAME: u32 = 1; pub const MAX_VIDEO_FRAMES: u32 = 3_600; pub const MAX_VIDEO_JOURNAL_BYTES: u64 = 256 * 1024 * 1024; const DEFAULT_FALLBACK_WIDTH: u32 = 1280; @@ -867,13 +871,13 @@ mod tests { } #[test] - fn video_options_default_to_800px_and_every_second_frame() { + fn video_options_default_to_800px_and_every_frame() { assert_eq!( VideoStartOptions::default(), VideoStartOptions { quality: DEFAULT_VIDEO_QUALITY, max_width: 800, - every_nth_frame: 2, + every_nth_frame: 1, } ); assert_eq!( From f874b66e6f824cabd6da63897853b28c9374b5b3 Mon Sep 17 00:00:00 2001 From: Jared Lunde Date: Thu, 24 Sep 2026 16:07:32 -0700 Subject: [PATCH 2/2] video: thin bursts by time and keep how a burst ended The count skip is gone for good; in its place the journal thins frames by time. A frame at least 33 ms after the last kept one is written at once. A frame inside that interval is the burst's newest and is held: written when the burst ends (the next frame is a full interval away, so this one was on screen long enough to see) or when the journal finishes, so the final state is never lost to thinning. Animated pages still land near 30 fps with the same file sizes, which is what everyNthFrame 2 was for, and a lone paint after a pause, the result of a click, is always kept. Four journal tests cover the lone paint, a 60 Hz burst, a burst whose last frame stayed on screen (blank, then content, then a five-second pause: the count skip showed blank for five seconds), and interval zero. Co-Authored-By: Claude Fable 5.1 --- src/video.rs | 162 ++++++++++++++++++++++++++++++++++++++++++++------- 1 file changed, 141 insertions(+), 21 deletions(-) diff --git a/src/video.rs b/src/video.rs index 3a9e1a2..e6f37ab 100644 --- a/src/video.rs +++ b/src/video.rs @@ -26,11 +26,18 @@ pub const DEFAULT_VIDEO_MAX_WIDTH: u32 = 800; /// CDP `Page.startScreencast` has no fps. Chromium emits a frame only when /// the compositor paints, so a still page costs nothing however long the /// recording runs, and an animating page runs at its own rate (often ~60 Hz). -/// Keep every paint: skipping alternate frames made a recording of one -/// interaction (a click, then one paint for the result) come back as a single -/// frame, because the result's paint was the skipped one. The frame cap below -/// still bounds a long animation. +/// Ask for every paint: skipping alternate frames here made a recording of +/// one interaction (a click, then one paint for the result) come back as a +/// single frame, because the result's paint was the skipped one. The rate is +/// capped by time instead, in [`FrameJournal`], which can tell a burst from +/// the only frame that matters. pub const DEFAULT_VIDEO_EVERY_NTH_FRAME: u32 = 1; +/// Frames closer together than this are a burst (an animation, a scroll): +/// the journal keeps one per interval and the last one of the burst, so the +/// clip stays near 30 fps at most while a frame that follows a pause, or ends +/// a burst, is always kept. Every WebM frame is a keyframe, so this is what +/// bounds file size and encode time for animated pages. +pub const DEFAULT_VIDEO_MIN_FRAME_INTERVAL_US: u64 = 1_000_000 / 30; pub const MAX_VIDEO_FRAMES: u32 = 3_600; pub const MAX_VIDEO_JOURNAL_BYTES: u64 = 256 * 1024 * 1024; const DEFAULT_FALLBACK_WIDTH: u32 = 1280; @@ -112,6 +119,12 @@ pub struct FrameJournal { last_ts_us: Option, width: u32, height: u32, + /// Bursts are thinned to one frame per this many microseconds. + min_interval_us: u64, + /// The newest frame that arrived inside the current interval. Written + /// when the burst ends (the next frame is a full interval away) or when the + /// journal finishes, so the final state is never lost to thinning. + pending: Option<(u64, Vec)>, } #[derive(Debug)] @@ -141,15 +154,45 @@ impl FrameJournal { last_ts_us: None, width: 0, height: 0, + min_interval_us: DEFAULT_VIDEO_MIN_FRAME_INTERVAL_US, + pending: None, }) } - /// Append one JPEG frame. Returns `false` when the journal is at a cap and + /// Thin bursts to one frame per `min_interval_us`; `0` keeps every frame. + pub fn with_min_frame_interval_us(mut self, min_interval_us: u64) -> Self { + self.min_interval_us = min_interval_us; + self + } + + /// Offer one JPEG frame. The first frame, and any frame at least an + /// interval after the last kept one, is written at once. A frame inside the + /// interval is held as the burst's newest and written when the burst ends + /// or the journal finishes. Returns `false` when the journal is at a cap and /// the frame was dropped (the caller should still ACK the screencast). pub fn push(&mut self, timestamp_us: u64, jpeg: &[u8]) -> RwResult { if jpeg.is_empty() { return Ok(true); } + let Some(last_kept) = self.last_ts_us else { + return self.write_frame(timestamp_us, jpeg); + }; + if timestamp_us.saturating_sub(last_kept) < self.min_interval_us { + self.pending = Some((timestamp_us, jpeg.to_vec())); + return Ok(true); + } + // The burst is over. Its last frame was on screen from its own + // timestamp until now; keep it when that was long enough to see, drop + // it when this frame replaced it within an interval. + if let Some((pending_ts, pending_jpeg)) = self.pending.take() { + if timestamp_us.saturating_sub(pending_ts) >= self.min_interval_us { + self.write_frame(pending_ts, &pending_jpeg)?; + } + } + self.write_frame(timestamp_us, jpeg) + } + + fn write_frame(&mut self, timestamp_us: u64, jpeg: &[u8]) -> RwResult { if self.frames >= MAX_VIDEO_FRAMES || self.bytes >= MAX_VIDEO_JOURNAL_BYTES { return Ok(false); } @@ -159,9 +202,8 @@ impl FrameJournal { if self.bytes.saturating_add(framed) > MAX_VIDEO_JOURNAL_BYTES { return Ok(false); } - let frame_len = u32::try_from(jpeg.len()).map_err(|_| { - RwError::Message("video journal frame exceeds 4 GiB".to_string()) - })?; + let frame_len = u32::try_from(jpeg.len()) + .map_err(|_| RwError::Message("video journal frame exceeds 4 GiB".to_string()))?; if self.width == 0 || self.height == 0 { if let Some((width, height)) = jpeg_dimensions(jpeg) { self.width = width; @@ -183,6 +225,9 @@ impl FrameJournal { } pub fn finish(mut self) -> RwResult { + if let Some((pending_ts, pending_jpeg)) = self.pending.take() { + self.write_frame(pending_ts, &pending_jpeg)?; + } self.writer .flush() .map_err(|error| RwError::Message(format!("video journal flush failed: {error}")))?; @@ -261,7 +306,10 @@ fn video_container(output: &Path) -> RwResult { } } -fn prepare_video_output(journal: &FinishedJournal, output: &Path) -> RwResult<(u32, u32, u64, u32)> { +fn prepare_video_output( + journal: &FinishedJournal, + output: &Path, +) -> RwResult<(u32, u32, u64, u32)> { if journal.frames == 0 { return Err(RwError::Message( "video recording captured no frames".to_string(), @@ -395,16 +443,14 @@ fn rgb_to_vp8_frame(rgb: &[u8], width: u32, height: u32) -> Vp8Frame { } fn webm_duration_ms(frames: &[(u64, Vec)], fps: u32) -> f64 { - let last = frames.last().map(|(timestamp_ms, _)| *timestamp_ms).unwrap_or(0) as f64; + let last = frames + .last() + .map(|(timestamp_ms, _)| *timestamp_ms) + .unwrap_or(0) as f64; last + 1000.0 / f64::from(fps.max(1)) } -fn mux_webm( - width: u32, - height: u32, - frames: &[(u64, Vec)], - fps: u32, -) -> RwResult> { +fn mux_webm(width: u32, height: u32, frames: &[(u64, Vec)], fps: u32) -> RwResult> { let mut ebml_body = Vec::new(); ebml_body.extend(ebml_elem(&[0x42, 0x86], &[1])?); ebml_body.extend(ebml_elem(&[0x42, 0xF7], &[1])?); @@ -416,7 +462,10 @@ fn mux_webm( let ebml = ebml_elem(&[0x1A, 0x45, 0xDF, 0xA3], &ebml_body)?; let mut info = Vec::new(); - info.extend(ebml_elem(&[0x2A, 0xD7, 0xB1], &1_000_000u64.to_be_bytes()[4..])?); + info.extend(ebml_elem( + &[0x2A, 0xD7, 0xB1], + &1_000_000u64.to_be_bytes()[4..], + )?); info.extend(ebml_elem(&[0x4D, 0x80], b"rustwright")?); info.extend(ebml_elem(&[0x57, 0x41], b"rustwright")?); // HTML5