diff --git a/crates/compositor/src/export_control.rs b/crates/compositor/src/export_control.rs index d289dc55c..472fdad91 100644 --- a/crates/compositor/src/export_control.rs +++ b/crates/compositor/src/export_control.rs @@ -150,6 +150,21 @@ impl Drop for VideoValidation { /// This excludes zero-duration MP4 edit-list samples. Drain delayed frames /// at EOF, and remain cancellable while scanning interleaved audio packets. fn validate_video_frame(path: &Path, control: &ExportControl) -> Result<()> { + decode_video_frames(path, control, true).map(|_| ()) +} + +/// Count actual displayable video frames, including delayed decoder output. +/// Unlike container metadata or duration-based estimates, this also works for +/// streams with no declared frame count. Export publication uses first-only +/// mode separately, so it does not pay for this complete scan. +#[cfg(target_os = "macos")] +pub(crate) fn count_decoded_video_frames(path: &Path) -> Result { + decode_video_frames(path, &ExportControl::default(), false) +} + +/// Share decoder ownership and discard/preroll handling between a first-frame +/// publication check and the macOS probe's complete frame count. +fn decode_video_frames(path: &Path, control: &ExportControl, first_only: bool) -> Result { use crate::ffi::*; use std::{ffi::CString, ptr}; @@ -200,6 +215,7 @@ fn validate_video_frame(path: &Path, control: &ExportControl) -> Result<()> { "open exported video decoder", )?; let mut draining = false; + let mut frames = 0; loop { control.check()?; let received = avcodec_receive_frame(video.decoder, video.frame); @@ -209,11 +225,17 @@ fn validate_video_frame(path: &Path, control: &ExportControl) -> Result<()> { && (*video.frame).height > 0; av_frame_unref(video.frame); if displayable { - return Ok(()); + frames += 1; + if first_only { + return Ok(frames); + } } continue; } if received == AVERROR_EOF || (draining && received == AVERROR_EAGAIN) { + if frames > 0 { + return Ok(frames); + } bail!("The exported video has no decodable displayable frames. The destination has not been replaced."); } averr( diff --git a/crates/compositor/src/pipeline_macos.rs b/crates/compositor/src/pipeline_macos.rs index b407d7966..0b8ac986b 100644 --- a/crates/compositor/src/pipeline_macos.rs +++ b/crates/compositor/src/pipeline_macos.rs @@ -1434,10 +1434,10 @@ unsafe fn drain_encoder( } } -/// Compte le nombre de frames d'un fichier (utilisé pour la barre de progression). -/// Le comptage est purement ffmpeg-side, donc portable. -pub fn probe_frame_count(_path: &str) -> Result { - Err(anyhow!("pipeline_macos::probe_frame_count: non implémenté")) +/// Count displayable frames by software decoding, including delayed frames at +/// EOF. No container frame-count metadata or duration estimate is required. +pub fn probe_frame_count(path: &str) -> Result { + crate::export_control::count_decoded_video_frames(Path::new(path)) } // Marqueur pour préserver la signature `fn run_composited(_: &Compositor, ...)` diff --git a/crates/compositor/src/timeline_walk.rs b/crates/compositor/src/timeline_walk.rs index 3ad2456a3..e7829180c 100644 --- a/crates/compositor/src/timeline_walk.rs +++ b/crates/compositor/src/timeline_walk.rs @@ -262,11 +262,26 @@ unsafe fn catch_up(dec: &mut Decoder, target: f64, clip_start: f64) -> Result<() /// hide a genuinely truncated camera recording. const CLIP_START_TAIL_TOLERANCE_SEC: f64 = 0.5; -/// Seek a clip start, holding the last frame only within the normal tail margin. +/// Seek a clip start within the normal tail margin. A future frame across an +/// interior gap is invalid, but a request before the first frame of an offset +/// stream retains the existing first-frame hold. unsafe fn seek_clip_start(decoder: &mut Decoder, target: f64) -> Result { let frame = decoder.seek_to_or_last(target)?; - Ok(!frame.is_null() - && target - decoder.cur_time_sec() <= CLIP_START_TAIL_TOLERANCE_SEC + 1e-6) + if frame.is_null() { + return Ok(false); + } + let current = decoder.cur_time_sec(); + if (target - current).abs() <= CLIP_START_TAIL_TOLERANCE_SEC + 1e-6 { + return Ok(true); + } + if current <= target { + return Ok(false); + } + // Only a distant future result needs this extra seek. Distinguish a late + // stream origin from a missing interior interval using the actual first + // frame, without changing decoder timestamps or the normal seek path. + let first = decoder.seek_to(0.0)?; + Ok(!first.is_null() && decoder.cur_time_sec() > target) } /// `walk_composited_timeline`, qui ne compose que les frames de sortie `only` (triées) quand il y @@ -312,7 +327,7 @@ unsafe fn walk_timeline( 'clips: for (clip_index, clip) in clips.iter().enumerate() { // A completed inspection must not open or seek an unrelated later clip. if let Some(only) = only { - if only.last().map_or(true, |&last| frames > last) { + if only.last().is_none_or(|&last| frames > last) { break 'clips; } } @@ -464,7 +479,7 @@ unsafe fn walk_timeline( for segment_frame in 0..segment.frame_count { let target_source_time = segment.source_time(segment_frame, out_fps as f64); if let Some(only) = only { - if only.last().map_or(true, |&last| frames > last) { + if only.last().is_none_or(|&last| frames > last) { break 'clips; } if only.binary_search(&frames).is_err() { diff --git a/crates/compositor/tests/export_timing.rs b/crates/compositor/tests/export_timing.rs index 7bc5d0e87..1024ebd67 100644 --- a/crates/compositor/tests/export_timing.rs +++ b/crates/compositor/tests/export_timing.rs @@ -9,9 +9,10 @@ //! (red → green → blue → white). A correct export spans all four colours; a //! mis-advanced one stays red for its whole length. //! -//! Needs a D3D11 GPU and the generated media, so it is opt-in: set +//! Needs a native GPU backend and the generated media, so it is opt-in: set //! OPENSCREEN_TEST_MEDIA to a directory holding `screen_colors.mp4` and -//! `webcam_gray.mp4`. Without it every test here skips. +//! `webcam_gray.mp4`. Gap/origin tests also need the derived fixtures below. +//! Without OPENSCREEN_TEST_MEDIA every test here skips. //! //! Regenerate the media with the vendored ffmpeg: //! for c in red green blue white; do ffmpeg -f lavfi \ @@ -20,6 +21,13 @@ //! ffmpeg -f concat -safe 0 -i concat.txt -c copy screen_colors.mp4 //! ffmpeg -f lavfi -i "color=c=gray:size=320x240:duration=4:rate=60" \ //! -c:v libopenh264 -g 60 -pix_fmt yuv420p webcam_gray.mp4 +//! ffmpeg -i screen_colors.mp4 -vf "select='lt(t,1)+gte(t,3)'" \ +//! -fps_mode vfr -c:v libopenh264 -g 60 -pix_fmt yuv420p video_gap.mp4 +//! ffmpeg -i screen_colors.mp4 -c copy -output_ts_offset 1 offset_start.mp4 +//! ffmpeg -i video_gap.mp4 -c copy -output_ts_offset 1 offset_gap.mp4 +//! The gap fixture must retain timestamps: video frames stop at 0.983 s and +//! resume at 3.000 s. An equivalent H.264 encoder is sufficient if libopenh264 +//! is unavailable. // Pas sur Linux : `pipeline::probe_frame_count` n'existe que dans // `pipeline_windows` et `pipeline_macos`. Les fichiers de `tests/` sont compiles @@ -427,6 +435,169 @@ fn mp4_export_bounds_tail_recovery_and_reports_the_correct_source() { std::fs::remove_dir_all(out_dir).expect("remove test outputs"); } +/// A seek inside a multi-second hole must not show a frame from the future. +/// Exercise both source roles and preservation after an earlier valid clip. +#[test] +fn mp4_export_rejects_a_start_inside_a_multisecond_video_gap() { + let Some(dir) = media_dir() else { + eprintln!("skipped: set OPENSCREEN_TEST_MEDIA"); + return; + }; + let out_dir = dir.join(format!("future_frame_gap-{}", std::process::id())); + std::fs::create_dir_all(&out_dir).expect("output dir"); + let gpu = Gpu::create(false).expect("gpu"); + let comp = Compositor::new_sized(&gpu, 640, 360).expect("compositor"); + let params = ExportParams { + width: 640, + height: 360, + fps: Some(30), + codec: ExportCodec::H264, + bit_rate: None, + }; + for timestamp_offset in [0.0, 1.0] { + let gap = dir.join(if timestamp_offset == 0.0 { + "video_gap.mp4" + } else { + "offset_gap.mp4" + }); + assert!( + gap.exists(), + "generate gap fixtures using the fixture header recipe" + ); + for camera_gap in [false, true] { + for after_valid_clip in [false, true] { + let mut bad = whole_clip(&dir); + if camera_gap { + bad.webcam = gap.to_string_lossy().into_owned(); + } else { + bad.screen = gap.to_string_lossy().into_owned(); + } + bad.source_start_sec = 1.5 + timestamp_offset; + bad.source_end_sec = 3.5 + timestamp_offset; + let mut clips = Vec::new(); + if after_valid_clip { + clips.push(whole_clip(&dir)); + } + clips.push(bad); + let out = out_dir.join("out.mp4"); + std::fs::write(&out, b"previous export").expect("seed destination"); + let result = pipeline::run_composited_multi( + &clips, + &out.to_string_lossy(), + &gpu, + &comp, + &Cfg::c8(), + ¶ms, + &mut |_| {}, + ); + let error = result + .err() + .expect("a future frame cannot stand in for a missing start"); + let message = format!("{error:#}"); + assert!( + message.contains(if camera_gap { "camera" } else { "screen" }), + "{message}" + ); + assert!( + message.contains(&format!("clip {}", clips.len())), + "{message}" + ); + assert_eq!(std::fs::read(&out).unwrap(), b"previous export"); + assert_eq!(std::fs::read_dir(&out_dir).unwrap().count(), 1); + std::fs::remove_file(out).expect("remove destination"); + } + } + } + std::fs::remove_dir(out_dir).expect("remove output dir"); +} + +/// A non-zero first video timestamp is a valid stream origin, not an interior gap. +#[test] +fn mp4_export_accepts_a_nonzero_first_video_timestamp() { + let Some(dir) = media_dir() else { + eprintln!("skipped: set OPENSCREEN_TEST_MEDIA"); + return; + }; + let offset = dir.join("offset_start.mp4"); + assert!( + offset.exists(), + "generate offset_start.mp4 using the fixture header recipe" + ); + let out_dir = dir.join(format!("offset_start-{}", std::process::id())); + std::fs::create_dir_all(&out_dir).expect("output dir"); + let gpu = Gpu::create(false).expect("gpu"); + let comp = Compositor::new_sized(&gpu, 640, 360).expect("compositor"); + #[cfg(target_os = "macos")] + { + let mut decoder = + pipeline::Decoder::open_for_export(&offset.to_string_lossy(), &gpu).expect("decoder"); + unsafe { + assert!(!decoder.seek_to(0.0).expect("first frame").is_null()); + assert!( + (decoder.cur_time_sec() - 1.0).abs() < 0.01, + "fixture must have a non-zero first timestamp" + ); + } + } + let params = ExportParams { + width: 640, + height: 360, + fps: Some(30), + codec: ExportCodec::H264, + bit_rate: None, + }; + for camera_offset in [false, true] { + let mut clip = whole_clip(&dir); + if camera_offset { + clip.webcam = offset.to_string_lossy().into_owned(); + } else { + clip.screen = offset.to_string_lossy().into_owned(); + } + let out = out_dir.join("out.mp4"); + let frames = pipeline::run_composited_multi( + &[clip], + &out.to_string_lossy(), + &gpu, + &comp, + &Cfg::c8(), + ¶ms, + &mut |_| {}, + ) + .expect("a non-zero stream origin must export"); + assert_eq!(frames.frames, 120); + assert!(pipeline::probe_frame_count(&out.to_string_lossy()).expect("decode output") > 0); + std::fs::remove_file(out).expect("remove export"); + } + std::fs::remove_dir(out_dir).expect("remove output dir"); +} + +/// The macOS frame probe must count decoded video rather than require MP4 +/// frame-count metadata; Matroska recordings commonly omit that metadata. +#[cfg(target_os = "macos")] +#[test] +fn frame_probe_counts_video_without_declared_frame_count() { + let Some(dir) = media_dir() else { + eprintln!("skipped: set OPENSCREEN_TEST_MEDIA"); + return; + }; + let source = dir.join(format!("frame_probe-{}.mkv", std::process::id())); + openscreen_compositor::remux::remux_to_seekable_matroska( + &dir.join("screen_colors.mp4").to_string_lossy(), + &source.to_string_lossy(), + ) + .expect("remux synthetic video without a declared frame count"); + assert_eq!( + pipeline::probe_frame_count(&source.to_string_lossy()).expect("decode Matroska"), + 240 + ); + assert_eq!( + pipeline::probe_frame_count(&dir.join("screen_colors.mp4").to_string_lossy()) + .expect("decode MP4"), + 240 + ); + std::fs::remove_file(source).expect("remove remux"); +} + /// Inspecting a completed selection must not prepare an unrelated later clip, /// while a selection that reaches the bad camera must still report its error. #[test] diff --git a/technical-documentation/architecture/export-pipeline.md b/technical-documentation/architecture/export-pipeline.md index 37bdda496..eb69eff62 100644 --- a/technical-documentation/architecture/export-pipeline.md +++ b/technical-documentation/architecture/export-pipeline.md @@ -140,8 +140,12 @@ and **one** encoder + muxer pair: - **An unreadable camera must not silently remove a clip.** If the camera ends more than 0.5 seconds before the clip starts, or cannot produce a frame - within 0.5 seconds of its requested start offset, the shared MP4/GIF timeline walk fails with the clip number and a - camera-specific explanation. This also covers browser recordings whose + within 0.5 seconds of its requested start offset in either direction (including + a seek landing on a future frame across a gap), the shared MP4/GIF timeline walk fails with the clip number and a + camera-specific explanation. A request before a non-zero first-frame timestamp + still holds that first frame; this valid stream origin is distinguished from + an interior gap by seeking the actual first frame only on the distant-future + fallback path. Decoder timestamps and the normal seek path are unchanged. This also covers browser recordings whose declared container duration outlasts their actual video frames. The user can re-record the camera or remove its track explicitly; the exporter does not discard it automatically. A walk producing no video frames is an error, diff --git a/technical-documentation/testing/manual-e2e-checklist.md b/technical-documentation/testing/manual-e2e-checklist.md index f09cf468b..7327615c8 100644 --- a/technical-documentation/testing/manual-e2e-checklist.md +++ b/technical-documentation/testing/manual-e2e-checklist.md @@ -913,6 +913,8 @@ These checks are the human slice: an agent cannot click Apple's picker (see abov | Date | Build / tag | Platform | Pass/fail | Notes | |------|-------------|----------|-----------|-------| +| 2026-10-24 | PR #1379 offset-origin follow-up; rebuilt addon SHA256 `0dd01525a163d8889dd2aaa7088ea0c1e923956b14c430ff70537cba5c24e9d5` | macOS 15.7.1, Apple Silicon | Partial — offset source exported; interior gap still rejected | Computer-use opened a synthetic video whose first decoded timestamp is 1.000 s, with a clip starting at 0. MP4 1080p/30 displayed Saved; independent probing counted 120 frames / 3.967 s at 1752×1080 and full-file decode exited 0. In the same session, opened the interior-gap project and confirmed replacement of that MP4: export reported the screen-start error, destination SHA256 stayed `d45192c6c319006849fee014bc3882141bead0f8b0c43e7c895c51cefe1b7d7b`, and no partial remained. Native regressions cover screen/camera offset sources and interior gaps with both zero and non-zero stream origins. Recording, Presenter Overlay compatibility, unrelated editor features, Windows/Linux runtime and release packaging were not exercised. | +| 2026-10-24 | dev post-merge follow-up to PR #1377 on `79c3aa24`; rebuilt addon SHA256 `53df86189b359d41b63a76e3eaafed5e9bf0e299fe0e89f8fd7950aa12bad6f3` | macOS 15.7.1, Apple Silicon | Partial — interior gap rejected; destination preserved; normal retry passed | Computer-use opened a synthetic screen clip starting at 1.5 s inside a timestamp gap from 0.983 to 3.000 s, then confirmed replacement of an existing synthetic MP4. Export reported that clip 1 had no readable screen frame at its start. Destination SHA256 stayed `854b0911778da18c2a456d3eb227da749c0af2138ad52092180156ed138a6a48`; no partial remained. In the same session, a normal four-second clip plus 10 ms tail exported successfully at 1080p/30: independent ffprobe counted 120 frames / 4.000 s at 1752×1080, and full-file decode exited 0. Native tests also cover camera gaps and failure after a valid preceding clip. Recording, Presenter Overlay compatibility, unrelated editor features, Windows/Linux runtime and release packaging were not exercised. | | 2026-10-24 | dev CodeRabbit follow-up to PR #1377; rebuilt addon SHA256 `8fe1baab14e941a5ba6d73b5596a27190a75edcd8b49630b451868f829bdaf71` | macOS 15.7.1, Apple Silicon | Partial — undecodable output rejected; destination preserved; retry passed | Computer-use reopened the same synthetic tail-only project used in the previous run, selected an existing synthetic MP4, and confirmed the native replace prompt. Export now reports validation failure instead of Saved. Destination SHA256 remained `854b0911778da18c2a456d3eb227da749c0af2138ad52092180156ed138a6a48`; no partial output remained. Without restarting the app, loaded the four-second valid clip plus tail and retried MP4 1080p/30: Saved, native walk 121 frames, independent ffprobe 120 decoded frames / 4.000 s at 1752×1080, full-file decode exit 0. This closes the publication-safety gap observed in the previous row; single-frame encoder timing itself remains unchanged. Native tests also cover absent/existing destination, malformed output, cancellation and successful H264/HEVC export. Recording, Presenter Overlay, unrelated editor features, Windows/Linux runtime and release packaging were not exercised. | | 2026-10-24 | dev `fix/reject-empty-exports`, PR #1377 follow-up; rebuilt addon SHA256 `2ad4ce50da3e2e8dd2a6dc67017048d5ae515f5b0d7143d509d727c9d09c29da` | macOS 15.7.1, Apple Silicon | Partial — multiclip GUI export passed; single-frame muxing limit observed | Computer-use opened an isolated profile and a synthetic four-second clip followed by a 10 ms tail starting at 3.99 s. MP4 1080p/30 displayed Saved; native walk reported 121 frames, independent ffprobe decoded 120 frames / 4.000 s at 1752×1080, and full-file decode completed without errors. Tail alone produced one muxed video sample with zero video duration and no decoded video frames; this additional single-frame muxing case is unresolved and no baseline attribution is claimed. Initial native-path entry failed; selecting the prepared project in the file list worked. Native export/inspection tests separately: 7 passed, 1 existing macOS frame-count-probe stub failure. Recording, Presenter Overlay compatibility, other editor features, Windows/Linux runtime and release validation were not exercised. | | 2026-09-23 | dev `fix/725-hidden-cursor-auto-zoom`; rebuilt compositor addon SHA256 `a0bcd61e17084c010141049a9978121e45be2985449ff1618ab6a3145f8381f6` | Windows 11 Home 26200 | Partial — issue #725 slider slice passed | Human-driven real recording in the Electron dev build: moving a slider whose application cursor becomes hidden kept the reconstructed cursor hidden in the Studio recording and did not create a click-driven automatic zoom for that press. A visible control click still produced the normal automatic zoom. Sidecar inspection independently confirmed the hidden interval (`visible: false`) and that no zoom was centred on the press immediately preceding it. Recording, stop and editor open passed. Not covered: export, packaged build, audio/webcam, tray/HUD, other editor features, macOS or Linux. |