From c4555f2b4cf97880e72142ad6ca09012afac3a26 Mon Sep 17 00:00:00 2001 From: songsong Date: Sat, 24 Oct 2026 14:48:33 +0800 Subject: [PATCH 1/5] fix(export): reject unreadable camera clips and empty output --- crates/compositor/src/timeline_walk.rs | 24 +++- crates/compositor/tests/export_timing.rs | 110 ++++++++++++++++++ .../architecture/export-pipeline.md | 12 ++ 3 files changed, 141 insertions(+), 5 deletions(-) diff --git a/crates/compositor/src/timeline_walk.rs b/crates/compositor/src/timeline_walk.rs index 5556e577b7..53465edc7d 100644 --- a/crates/compositor/src/timeline_walk.rs +++ b/crates/compositor/src/timeline_walk.rs @@ -22,7 +22,7 @@ use crate::frame_geometry::webcam_is_real; use crate::pipeline::{ClipSource, Decoder}; use crate::regions::{speed_segments_for_window, SpeedSegment}; use crate::scene::Scene; -use anyhow::Result; +use anyhow::{bail, Context, Result}; use std::collections::HashMap; /// Ce que le décodeur sait de la PROCHAINE frame, sans l'adopter. @@ -158,7 +158,11 @@ pub(crate) unsafe fn walk_composited_timeline( on_frame: &mut dyn FnMut(u64) -> Result<()>, on_clip_end: &mut dyn FnMut(usize, f64, u64, &[SpeedSegment]) -> Result<()>, ) -> Result { - walk_timeline(clips, gpu, comp, cfg, out_fps, scene, screen_decs, webcam_decs, None, on_frame, on_clip_end) + let frames = walk_timeline(clips, gpu, comp, cfg, out_fps, scene, screen_decs, webcam_decs, None, on_frame, on_clip_end)?; + if frames == 0 { + bail!("No video frames were exported. Check that the clips contain readable video before exporting again."); + } + Ok(frames) } /// Une frame de sortie relue par `render_frames`. @@ -350,6 +354,12 @@ unsafe fn walk_timeline( // par le même offset que le seek (`webcam_time = screen_time - offset`). let webcam_available_screen_end = webcam_available_duration.map(|duration| duration + clip.webcam_offset_sec); + if has_camera && webcam_available_screen_end.is_some_and(|end| end <= clip.source_start_sec) { + bail!( + "Cannot export clip {}: the camera video ends before this clip starts. Record the camera again or remove its track before exporting.", + clip_index + 1, + ); + } let mut source_end_sec = clip.source_end_sec; if let Some(duration) = screen_available_duration { source_end_sec = source_end_sec.min(duration); @@ -392,13 +402,17 @@ unsafe fn walk_timeline( // un seul seek keyframe, puis chaque décodeur avance selon son propre pts jusqu'aux // temps source demandés par les spans de vitesse. if sdec.seek_to(clip.source_start_sec)?.is_null() { - continue; // clip vide / au-delà de la source + bail!("Cannot export clip {}: the screen video has no readable frame at the clip's start.", clip_index + 1); } if wdec - .seek_to((clip.source_start_sec - clip.webcam_offset_sec).max(0.0))? + .seek_to((clip.source_start_sec - clip.webcam_offset_sec).max(0.0)) + .with_context(|| format!("Cannot export clip {}: could not read the camera video. Record the camera again or remove its track before exporting.", clip_index + 1))? .is_null() { - continue; + bail!( + "Cannot export clip {}: the camera video has no readable frame at the clip's start. Record the camera again or remove its track before exporting.", + clip_index + 1, + ); } if cursor_enabled { diff --git a/crates/compositor/tests/export_timing.rs b/crates/compositor/tests/export_timing.rs index cffda44694..e9e564664e 100644 --- a/crates/compositor/tests/export_timing.rs +++ b/crates/compositor/tests/export_timing.rs @@ -161,6 +161,116 @@ fn mp4_export_frame_count_follows_output_fps() { assert_eq!(probed, 120, "muxed file disagrees with the reported count"); } +/// A camera that ended before its requested offset must fail the whole export, +/// even after a good clip. Otherwise an empty or incomplete MP4 replaces the destination. +#[test] +fn mp4_export_rejects_an_unreadable_camera_without_publishing() { + let Some(dir) = media_dir() else { + eprintln!("skipped: set OPENSCREEN_TEST_MEDIA"); + return; + }; + let out_dir = dir.join("unreadable_camera_export"); + std::fs::create_dir_all(&out_dir).expect("output dir"); + let camera = out_dir.join("camera.webm"); + openscreen_compositor::remux::remux_to_seekable_matroska( + &dir.join("webcam_gray.mp4").to_string_lossy(), + &camera.to_string_lossy(), + ) + .expect("camera with no stream duration, like a browser recording"); + let gpu = Gpu::create(false).expect("gpu"); + let params = ExportParams { + width: 640, + height: 360, + fps: Some(30), + codec: ExportCodec::H264, + bit_rate: None, + }; + let comp = Compositor::new_sized(&gpu, 640, 360).expect("compositor"); + for unknown_duration in [false, true] { + for after_valid_clip in [false, true] { + let mut bad = whole_clip(&dir); + if unknown_duration { + bad.webcam = camera.to_string_lossy().into_owned(); + } + bad.webcam_offset_sec = -(SOURCE_SEC + 1.0); + 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 error = pipeline::run_composited_multi( + &clips, + &out.to_string_lossy(), + &gpu, + &comp, + &Cfg::c8(), + ¶ms, + &mut |_| {}, + ) + .err() + .expect("an unreadable camera must not silently drop a clip"); + let message = format!("{error:#}"); + assert!( + message.contains("camera"), + "missing camera diagnosis: {message}" + ); + assert!( + message.contains(&format!("clip {}", clips.len())), + "missing clip number: {message}" + ); + assert_eq!(std::fs::read(&out).unwrap(), b"previous export"); + assert_eq!( + std::fs::read_dir(&out_dir).unwrap().count(), + 2, + "partial output left behind" + ); + } + } + std::fs::remove_dir_all(out_dir).expect("remove test outputs"); +} + +#[test] +fn mp4_export_rejects_a_timeline_that_produces_no_frames() { + let Some(dir) = media_dir() else { + eprintln!("skipped: set OPENSCREEN_TEST_MEDIA"); + return; + }; + let gpu = Gpu::create(false).expect("gpu"); + let params = ExportParams { + width: 640, + height: 360, + fps: Some(30), + codec: ExportCodec::H264, + bit_rate: None, + }; + let comp = Compositor::new_sized(&gpu, 640, 360).expect("compositor"); + let mut clip = whole_clip(&dir); + clip.webcam.clear(); + clip.source_start_sec = SOURCE_SEC + 1.0; + clip.source_end_sec = SOURCE_SEC + 2.0; + let out = dir.join("no_frames.mp4"); + std::fs::write(&out, b"previous export").expect("seed destination"); + let error = pipeline::run_composited_multi( + &[clip], + &out.to_string_lossy(), + &gpu, + &comp, + &Cfg::c8(), + ¶ms, + &mut |_| {}, + ) + .err() + .expect("zero video frames must not be a successful export"); + assert!( + format!("{error:#}").contains("No video frames"), + "unexpected error: {error:#}" + ); + assert_eq!(std::fs::read(&out).unwrap(), b"previous export"); + std::fs::remove_file(out).expect("remove test output"); +} + /// Cancelling an MP4 mid-render stops the walk at the next frame, reports `ExportCancelled`, /// and publishes nothing: the destination keeps what it held and no partial is left beside /// it. On Windows that last check also proves the muxer closed its file, since an open diff --git a/technical-documentation/architecture/export-pipeline.md b/technical-documentation/architecture/export-pipeline.md index b1c99de54e..636d185a68 100644 --- a/technical-documentation/architecture/export-pipeline.md +++ b/technical-documentation/architecture/export-pipeline.md @@ -138,6 +138,18 @@ and **one** encoder + muxer pair: Audio jobs already in flight are joined, so a cancel settles once they finish. +- **An unreadable camera must not silently remove a clip.** If the camera ends + before the clip starts, or cannot produce a frame at 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 + 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, + never a successful empty file. These errors discard the staged output and + preserve an existing destination, including when earlier clips rendered + successfully. Frame inspection (`renderFrames`) may still return an empty + selection when the requested frame indices are outside the timeline. + - **Imported audio tracks** (voiceover / BGM / SFX, issue #350) are mixed on top of the assembled programme by `audio.rs::mix_external_tracks`, between `assemble_concatenated_pcm` and `finish_audio`. Each track's From 0ea00920de2e3467a1d069bff5ef465ca6d2f615 Mon Sep 17 00:00:00 2001 From: songsong Date: Sat, 24 Oct 2026 17:22:58 +0800 Subject: [PATCH 2/5] fix(export): bound clip-start recovery to normal video tails --- crates/compositor/src/timeline_walk.rs | 36 ++-- crates/compositor/tests/export_timing.rs | 155 ++++++++++++++++++ .../architecture/export-pipeline.md | 11 +- .../testing/manual-e2e-checklist.md | 1 + 4 files changed, 189 insertions(+), 14 deletions(-) diff --git a/crates/compositor/src/timeline_walk.rs b/crates/compositor/src/timeline_walk.rs index 53465edc7d..28e8518ada 100644 --- a/crates/compositor/src/timeline_walk.rs +++ b/crates/compositor/src/timeline_walk.rs @@ -257,6 +257,17 @@ unsafe fn catch_up(dec: &mut Decoder, target: f64, clip_start: f64) -> Result<() } } +/// Normal recordings can have audio up to roughly half a second longer than +/// video. Hold the last frame only in that margin: an unbounded fallback would +/// hide a genuinely truncated camera recording. +const CLIP_START_TAIL_TOLERANCE_SEC: f64 = 0.5; + +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) +} + /// `walk_composited_timeline`, qui ne compose que les frames de sortie `only` (triées) quand il y /// en a : les autres avancent le compteur sans décoder ni composer, et la marche s'arrête après /// la dernière. Un flux sans pts fiable avance d'une frame par frame de sortie : sauter le rend @@ -354,7 +365,7 @@ unsafe fn walk_timeline( // par le même offset que le seek (`webcam_time = screen_time - offset`). let webcam_available_screen_end = webcam_available_duration.map(|duration| duration + clip.webcam_offset_sec); - if has_camera && webcam_available_screen_end.is_some_and(|end| end <= clip.source_start_sec) { + if has_camera && webcam_available_screen_end.is_some_and(|end| end + CLIP_START_TAIL_TOLERANCE_SEC < clip.source_start_sec) { bail!( "Cannot export clip {}: the camera video ends before this clip starts. Record the camera again or remove its track before exporting.", clip_index + 1, @@ -401,18 +412,21 @@ unsafe fn walk_timeline( // un seul seek keyframe, puis chaque décodeur avance selon son propre pts jusqu'aux // temps source demandés par les spans de vitesse. - if sdec.seek_to(clip.source_start_sec)?.is_null() { + if !seek_clip_start(sdec, clip.source_start_sec)? { bail!("Cannot export clip {}: the screen video has no readable frame at the clip's start.", clip_index + 1); } - if wdec - .seek_to((clip.source_start_sec - clip.webcam_offset_sec).max(0.0)) - .with_context(|| format!("Cannot export clip {}: could not read the camera video. Record the camera again or remove its track before exporting.", clip_index + 1))? - .is_null() - { - bail!( - "Cannot export clip {}: the camera video has no readable frame at the clip's start. Record the camera again or remove its track before exporting.", - clip_index + 1, - ); + let webcam_start = if has_camera { + (clip.source_start_sec - clip.webcam_offset_sec).max(0.0) + } else { + clip.source_start_sec + }; + let webcam_error = if has_camera { + format!("Cannot export clip {}: the camera video has no readable frame at the clip's start. Record the camera again or remove its track before exporting.", clip_index + 1) + } else { + format!("Cannot export clip {}: the screen video has no readable frame at the clip's start.", clip_index + 1) + }; + if !seek_clip_start(wdec, webcam_start).with_context(|| webcam_error.clone())? { + bail!(webcam_error); } if cursor_enabled { diff --git a/crates/compositor/tests/export_timing.rs b/crates/compositor/tests/export_timing.rs index e9e564664e..8581ebe2cd 100644 --- a/crates/compositor/tests/export_timing.rs +++ b/crates/compositor/tests/export_timing.rs @@ -271,6 +271,161 @@ fn mp4_export_rejects_a_timeline_that_produces_no_frames() { std::fs::remove_file(out).expect("remove test output"); } +/// Entering a clip just past its last frame is normal when audio outlasts video. +/// Exercise screen-only, real-camera and unknown-duration camera sources, including +/// frame inspection, which shares the export walk. +#[test] +fn mp4_export_accepts_tail_clips_and_frame_inspection() { + let Some(dir) = media_dir() else { + eprintln!("skipped: set OPENSCREEN_TEST_MEDIA"); + return; + }; + let out_dir = dir.join("tail_clip_export"); + std::fs::create_dir_all(&out_dir).expect("output dir"); + let camera = out_dir.join("camera.mkv"); + openscreen_compositor::remux::remux_to_seekable_matroska( + &dir.join("webcam_gray.mp4").to_string_lossy(), + &camera.to_string_lossy(), + ) + .expect("unknown-duration camera"); + 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 source in 0..5 { + let mut tail = whole_clip(&dir); + if source == 0 || source == 4 { + tail.webcam.clear(); + } else if source == 2 { + tail.webcam = camera.to_string_lossy().into_owned(); + } + // Last video pts is 4 - 1/60; the declared stream end is 4 seconds. + tail.source_start_sec = if source >= 3 { + SOURCE_SEC + 0.25 + } else { + SOURCE_SEC - 0.01 + }; + tail.source_end_sec = tail.source_start_sec + 0.01; + let expected_tail_frames = if source >= 3 { 0 } else { 1 }; + let clips = [whole_clip(&dir), tail]; + let frames = unsafe { + openscreen_compositor::timeline_walk::render_frames( + &clips, + &gpu, + &comp, + &Cfg::c8(), + 30, + &[120], + ) + } + .expect("inspect tail frame"); + assert_eq!( + frames.len(), + expected_tail_frames, + "source {source}: tail inspection" + ); + if let Some(frame) = frames.first() { + assert_eq!(frame.index, 120); + } + let out = out_dir.join("out.mp4"); + let stats = pipeline::run_composited_multi( + &clips, + &out.to_string_lossy(), + &gpu, + &comp, + &Cfg::c8(), + ¶ms, + &mut |_| {}, + ) + .expect("tail must not fail the complete export"); + assert_eq!( + stats.frames, + 120 + expected_tail_frames as u64, + "source {source}: tail export" + ); + std::fs::remove_file(out).expect("remove output"); + } + std::fs::remove_dir_all(out_dir).expect("remove test outputs"); +} + +/// Unknown stream duration must not turn last-frame recovery into an unbounded +/// hold. A real gap still fails, and the screen stand-in must not blame a camera. +#[test] +fn mp4_export_bounds_tail_recovery_and_reports_the_correct_source() { + let Some(dir) = media_dir() else { + eprintln!("skipped: set OPENSCREEN_TEST_MEDIA"); + return; + }; + let out_dir = dir.join("bounded_tail_export"); + std::fs::create_dir_all(&out_dir).expect("output dir"); + let source = out_dir.join("source.mkv"); + openscreen_compositor::remux::remux_to_seekable_matroska( + &dir.join("screen_colors.mp4").to_string_lossy(), + &source.to_string_lossy(), + ) + .expect("unknown-duration source"); + 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 has_camera in [false, true] { + for gap in [0.25, 0.6] { + let mut clip = whole_clip(&dir); + if has_camera { + clip.webcam = source.to_string_lossy().into_owned(); + clip.webcam_offset_sec = -(SOURCE_SEC + gap); + clip.source_end_sec = 0.1; + } else { + clip.screen = source.to_string_lossy().into_owned(); + clip.webcam.clear(); + clip.source_start_sec = SOURCE_SEC + gap; + clip.source_end_sec = clip.source_start_sec + 0.1; + } + let out = out_dir.join("out.mp4"); + std::fs::write(&out, b"previous export").expect("seed output"); + let result = pipeline::run_composited_multi( + &[clip], + &out.to_string_lossy(), + &gpu, + &comp, + &Cfg::c8(), + ¶ms, + &mut |_| {}, + ); + if gap < 0.5 { + assert_eq!(result.expect("normal tail hold").frames, 3); + } else { + let message = format!("{:#}", result.err().expect("real gap must fail")); + assert!( + message.contains(if has_camera { "camera" } else { "screen" }), + "{message}" + ); + if !has_camera { + assert!(!message.contains("camera"), "{message}"); + } + assert_eq!(std::fs::read(&out).unwrap(), b"previous export"); + } + std::fs::remove_file(out).expect("remove output"); + assert_eq!( + std::fs::read_dir(&out_dir).unwrap().count(), + 1, + "partial output left behind" + ); + } + } + std::fs::remove_dir_all(out_dir).expect("remove test outputs"); +} + /// Cancelling an MP4 mid-render stops the walk at the next frame, reports `ExportCancelled`, /// and publishes nothing: the destination keeps what it held and no partial is left beside /// it. On Windows that last check also proves the muxer closed its file, since an open diff --git a/technical-documentation/architecture/export-pipeline.md b/technical-documentation/architecture/export-pipeline.md index 636d185a68..6149c5d461 100644 --- a/technical-documentation/architecture/export-pipeline.md +++ b/technical-documentation/architecture/export-pipeline.md @@ -139,8 +139,8 @@ and **one** encoder + muxer pair: finish. - **An unreadable camera must not silently remove a clip.** If the camera ends - before the clip starts, or cannot produce a frame at its requested start - offset, the shared MP4/GIF timeline walk fails with the clip number and a + 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 declared container duration outlasts their actual video frames. The user can re-record the camera or remove its track explicitly; the exporter does not @@ -148,7 +148,12 @@ and **one** encoder + muxer pair: never a successful empty file. These errors discard the staged output and preserve an existing destination, including when earlier clips rendered successfully. Frame inspection (`renderFrames`) may still return an empty - selection when the requested frame indices are outside the timeline. + selection when the requested frame indices are outside the timeline. Both + export and frame inspection hold the last video frame when entering a clip + within the 0.5-second tail margin (audio can outlast video). Clips starting + beyond their declared end within this margin retain the existing skip + behavior; an entirely empty export still fails. Screen-only clips report + screen errors, including errors from the stand-in decoder. - **Imported audio tracks** (voiceover / BGM / SFX, issue #350) are mixed on top of the assembled programme by `audio.rs::mix_external_tracks`, diff --git a/technical-documentation/testing/manual-e2e-checklist.md b/technical-documentation/testing/manual-e2e-checklist.md index 340df14803..b2220ec307 100644 --- a/technical-documentation/testing/manual-e2e-checklist.md +++ b/technical-documentation/testing/manual-e2e-checklist.md @@ -913,6 +913,7 @@ 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 | dev `fix/reject-empty-exports`, follow-up to PR #1377; rebuilt native addon | macOS 15.7.1, Apple Silicon | Partial — launch/editor only; GUI export unverified | Computer-use opened the isolated test profile, permissions onboarding and editor. The native file picker repeatedly cleared the synthetic project path on confirmation, so the project was not loaded and no GUI export is claimed. Native synthetic export/inspection tests separately cover screen/camera tails, unknown stream duration, real gaps, zero frames and destination preservation (7 passed; the unchanged macOS frame-count probe stub causes 1 existing failure). Recording, Presenter Overlay compatibility, other editor features, Windows/Linux 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. | | 2026-09-12 | dev `fix/643-gif-export-cancel`, based on `fbe461e9`; rebuilt compositor addon SHA256 `128c89fe65a459b3f17721d610ea9a78e0d3e980e9eb0b5fb2b0e5d018619e15` | Windows | Partial — export cancellation passed | Computer-use OS input, real Electron and native save dialog, imported synthetic 8 s source. GIF 15 FPS / Medium (720p) / loop on: observed 55/120 frames, clicked Cancel while rendering (pending snapshot 104/120), returned to the same options; no final or partial output remained. Retried in that dialog to the same path and received Saved to; resulting GIF is 1280×720, 120 frames, 8,200,202 bytes, fully decodes with ffmpeg (exit 0). Automated real-addon tests separately pass existing-destination preservation, GIF retry, ordinary GIF and MP4 export. Desktop adapter required screenshot-based coordinates and Ctrl+O to reach the editor; HUD reachability is not claimed. An unrelated process error window partly obscured the retry success screenshot; file verification independently passed. Skipped: recording, audio/mic, webcam, tray/HUD, effect/caption comparisons, second viewer, macOS/Linux and release promotion. Task-local profile and evidence retained; test app stopped. | | 2026-07-31 | dev build, `claude/e2e-tests-v1-8-0-474894` (e9578f09) | macOS 26.5, M1 | Partial — 1 defect | Ran launch/HUD, media, modifier anchoring, and export. **Defect: a dangling asset blanks the preview.** Modifier anchoring across a reorder verified in preview and in the exported frames. macOS export produced 1280×720 h264 + AAC at ~2× realtime. Chat sections skipped: no AI provider configured. HUD drag not runnable under computer-use (drop point is the desktop). | From ea32d893015e76ed907a869bccfbb8da7d250f60 Mon Sep 17 00:00:00 2001 From: songsong Date: Sat, 24 Oct 2026 19:57:59 +0800 Subject: [PATCH 3/5] docs: record desktop export follow-up and timing limits --- technical-documentation/testing/manual-e2e-checklist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/technical-documentation/testing/manual-e2e-checklist.md b/technical-documentation/testing/manual-e2e-checklist.md index b2220ec307..c6bf2102ef 100644 --- a/technical-documentation/testing/manual-e2e-checklist.md +++ b/technical-documentation/testing/manual-e2e-checklist.md @@ -913,7 +913,7 @@ 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 | dev `fix/reject-empty-exports`, follow-up to PR #1377; rebuilt native addon | macOS 15.7.1, Apple Silicon | Partial — launch/editor only; GUI export unverified | Computer-use opened the isolated test profile, permissions onboarding and editor. The native file picker repeatedly cleared the synthetic project path on confirmation, so the project was not loaded and no GUI export is claimed. Native synthetic export/inspection tests separately cover screen/camera tails, unknown stream duration, real gaps, zero frames and destination preservation (7 passed; the unchanged macOS frame-count probe stub causes 1 existing failure). Recording, Presenter Overlay compatibility, other editor features, Windows/Linux and release validation 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. | | 2026-09-12 | dev `fix/643-gif-export-cancel`, based on `fbe461e9`; rebuilt compositor addon SHA256 `128c89fe65a459b3f17721d610ea9a78e0d3e980e9eb0b5fb2b0e5d018619e15` | Windows | Partial — export cancellation passed | Computer-use OS input, real Electron and native save dialog, imported synthetic 8 s source. GIF 15 FPS / Medium (720p) / loop on: observed 55/120 frames, clicked Cancel while rendering (pending snapshot 104/120), returned to the same options; no final or partial output remained. Retried in that dialog to the same path and received Saved to; resulting GIF is 1280×720, 120 frames, 8,200,202 bytes, fully decodes with ffmpeg (exit 0). Automated real-addon tests separately pass existing-destination preservation, GIF retry, ordinary GIF and MP4 export. Desktop adapter required screenshot-based coordinates and Ctrl+O to reach the editor; HUD reachability is not claimed. An unrelated process error window partly obscured the retry success screenshot; file verification independently passed. Skipped: recording, audio/mic, webcam, tray/HUD, effect/caption comparisons, second viewer, macOS/Linux and release promotion. Task-local profile and evidence retained; test app stopped. | | 2026-07-31 | dev build, `claude/e2e-tests-v1-8-0-474894` (e9578f09) | macOS 26.5, M1 | Partial — 1 defect | Ran launch/HUD, media, modifier anchoring, and export. **Defect: a dangling asset blanks the preview.** Modifier anchoring across a reorder verified in preview and in the exported frames. macOS export produced 1280×720 h264 + AAC at ~2× realtime. Chat sections skipped: no AI provider configured. HUD drag not runnable under computer-use (drop point is the desktop). | From 0049ed2bae713d8d6556bb45cd4f3a1a1728619f Mon Sep 17 00:00:00 2001 From: songsong Date: Sat, 24 Oct 2026 20:19:41 +0800 Subject: [PATCH 4/5] fix(export): validate staged MP4 before replacing destination --- crates/compositor/src/export_control.rs | 170 ++++++++++++++++++ crates/compositor/src/pipeline_linux.rs | 4 +- crates/compositor/src/pipeline_macos.rs | 6 +- crates/compositor/src/pipeline_windows.rs | 6 +- crates/compositor/src/timeline_walk.rs | 5 +- crates/compositor/tests/export_timing.rs | 98 ++++++++++ .../architecture/export-pipeline.md | 9 +- .../testing/manual-e2e-checklist.md | 1 + 8 files changed, 289 insertions(+), 10 deletions(-) diff --git a/crates/compositor/src/export_control.rs b/crates/compositor/src/export_control.rs index 8542c55eab..4be3502652 100644 --- a/crates/compositor/src/export_control.rs +++ b/crates/compositor/src/export_control.rs @@ -106,6 +106,144 @@ pub(crate) fn with_staged_output( result } +/// Render and close an MP4, then verify a displayable decoded frame before +/// publishing it. A positive encoder submission count alone is insufficient: +/// a zero-duration MP4 sample can be flagged discard by the demuxer. +pub(crate) fn with_validated_mp4_output( + target: &Path, + control: &ExportControl, + render: impl FnOnce(File, &Path) -> Result, +) -> Result { + with_staged_output(target, control, |file, staged| { + let stats = render(file, staged)?; + validate_video_frame(staged, control).context("validating exported video")?; + Ok(stats) + }) +} + +/// Own every FFmpeg allocation so validation closes the file before publication +/// or cleanup, including on cancellation and partially initialized error paths. +#[derive(Default)] +struct VideoValidation { + input: *mut crate::ffi::AVFormatContext, + decoder: *mut crate::ffi::AVCodecContext, + packet: *mut crate::ffi::AVPacket, + frame: *mut crate::ffi::AVFrame, +} + +impl Drop for VideoValidation { + fn drop(&mut self) { + unsafe { + crate::ffi::av_frame_free(&mut self.frame); + crate::ffi::av_packet_free(&mut self.packet); + crate::ffi::avcodec_free_context(&mut self.decoder); + if !self.input.is_null() { + crate::ffi::avformat_close_input(&mut self.input); + } + } + } +} + +/// Decode only until the first displayable frame. Decode discard-marked preroll +/// packets to preserve reference state, but never accept discard-marked frames. +/// 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<()> { + use crate::ffi::*; + use std::{ffi::CString, ptr}; + + control.check()?; + let path = CString::new(path.to_string_lossy().as_bytes())?; + let mut video = VideoValidation::default(); + unsafe { + averr( + avformat_open_input( + &mut video.input, + path.as_ptr(), + ptr::null(), + ptr::null_mut(), + ), + "open exported video", + )?; + averr( + avformat_find_stream_info(video.input, ptr::null_mut()), + "read exported video streams", + )?; + let index = av_find_best_stream( + video.input, + AVMediaType::AVMEDIA_TYPE_VIDEO, + -1, + -1, + ptr::null_mut(), + 0, + ); + averr(index, "find exported video stream")?; + let stream = sn_fmt_stream(video.input, index); + let params = (*stream).codecpar; + let codec = avcodec_find_decoder((*params).codec_id); + if codec.is_null() { + bail!("No decoder is available for the exported video."); + } + video.decoder = avcodec_alloc_context3(codec); + video.packet = av_packet_alloc(); + video.frame = av_frame_alloc(); + if video.decoder.is_null() || video.packet.is_null() || video.frame.is_null() { + bail!("Could not allocate exported video validation buffers."); + } + averr( + avcodec_parameters_to_context(video.decoder, params), + "configure exported video decoder", + )?; + averr( + avcodec_open2(video.decoder, codec, ptr::null_mut()), + "open exported video decoder", + )?; + let mut draining = false; + loop { + control.check()?; + let received = avcodec_receive_frame(video.decoder, video.frame); + if received >= 0 { + let displayable = (*video.frame).flags & AV_FRAME_FLAG_DISCARD as i32 == 0 + && (*video.frame).width > 0 + && (*video.frame).height > 0; + av_frame_unref(video.frame); + if displayable { + return Ok(()); + } + continue; + } + if received == AVERROR_EOF || (draining && received == AVERROR_EAGAIN) { + bail!("The exported video has no decodable displayable frames. The destination has not been replaced."); + } + averr( + if received == AVERROR_EAGAIN { + 0 + } else { + received + }, + "decode exported video frame", + )?; + let read = av_read_frame(video.input, video.packet); + if read == AVERROR_EOF { + averr( + avcodec_send_packet(video.decoder, ptr::null()), + "flush exported video decoder", + )?; + draining = true; + } else { + averr(read, "read exported video packet")?; + let sent = if (*video.packet).stream_index == index { + avcodec_send_packet(video.decoder, video.packet) + } else { + 0 + }; + av_packet_unref(video.packet); + averr(sent, "send exported video packet")?; + } + } + } +} + #[cfg(test)] mod tests { use super::*; @@ -129,6 +267,38 @@ mod tests { fn drop(&mut self) { let _ = fs::remove_dir_all(&self.0); } } + /// An invalid completed file must not replace the destination or leave a partial. + #[test] + fn invalid_mp4_is_rejected_before_publication() { + let dir = TestDir::new(); + let target = dir.0.join("out.mp4"); + fs::write(&target, b"previous export").unwrap(); + let result = with_validated_mp4_output(&target, &ExportControl::default(), |mut file, _| { + file.write_all(b"not an MP4")?; + Ok(1) + }); + assert!(format!("{:#}", result.unwrap_err()).contains("validating exported video")); + assert_eq!(fs::read(&target).unwrap(), b"previous export"); + dir.assert_only("out.mp4"); + } + + /// Cancellation after rendering wins before validation and publication. + #[test] + fn cancellation_before_mp4_validation_preserves_destination() { + let dir = TestDir::new(); + let target = dir.0.join("out.mp4"); + fs::write(&target, b"previous export").unwrap(); + let control = ExportControl::default(); + let result = with_validated_mp4_output(&target, &control, |mut file, _| { + file.write_all(b"not an MP4")?; + assert!(control.cancel()); + Ok(1) + }); + assert!(result.unwrap_err().is::()); + assert_eq!(fs::read(&target).unwrap(), b"previous export"); + dir.assert_only("out.mp4"); + } + #[test] fn cancellation_before_rendering_never_opens_output() { let dir = TestDir::new(); diff --git a/crates/compositor/src/pipeline_linux.rs b/crates/compositor/src/pipeline_linux.rs index e0fd1c2620..15c8d2b152 100644 --- a/crates/compositor/src/pipeline_linux.rs +++ b/crates/compositor/src/pipeline_linux.rs @@ -35,7 +35,7 @@ use crate::audio::{ use crate::audio_jobs::{decode_and_stretch_clip_audio, ClipAudioJobs}; use crate::config::Cfg; use crate::d3d::Gpu; -use crate::export_control::{with_staged_output, ExportControl}; +use crate::export_control::{with_validated_mp4_output, ExportControl}; use crate::ffi::AVFrame; use crate::linux_decode::SwDecoder; use crate::timeline_walk::NextFrameTime; @@ -872,7 +872,7 @@ pub fn run_composited_multi_cancellable( progress: &mut dyn FnMut(u64), control: &ExportControl, ) -> Result { - with_staged_output(Path::new(out), control, |file, staged| { + with_validated_mp4_output(Path::new(out), control, |file, staged| { drop(file); // ffmpeg le rouvre par son nom run_multi_inner(clips, &staged.to_string_lossy(), gpu, comp, cfg, params, progress, control) }) diff --git a/crates/compositor/src/pipeline_macos.rs b/crates/compositor/src/pipeline_macos.rs index 638b664f48..5cebfa1ed4 100644 --- a/crates/compositor/src/pipeline_macos.rs +++ b/crates/compositor/src/pipeline_macos.rs @@ -37,7 +37,7 @@ use crate::audio::{ use crate::audio_jobs::{decode_and_stretch_clip_audio, ClipAudioJobs}; use crate::compositor::Compositor; use crate::d3d::Gpu; -use crate::export_control::{with_staged_output, ExportControl}; +use crate::export_control::{with_validated_mp4_output, ExportControl}; use crate::timeline_walk::NextFrameTime; use anyhow::{anyhow, bail, Result}; use std::ffi::{c_void, CString}; @@ -1185,7 +1185,7 @@ pub fn run_composited_multi_cancellable( progress: &mut dyn FnMut(u64), control: &ExportControl, ) -> Result { - with_staged_output(Path::new(out), control, |file, staged| { + with_validated_mp4_output(Path::new(out), control, |file, staged| { drop(file); // ffmpeg le rouvre par son nom run_multi_inner(clips, &staged.to_string_lossy(), gpu, comp, cfg, params, progress, control) }) @@ -1391,7 +1391,7 @@ fn run_multi_inner( "write_trailer", )?; // Fichier fermé et contexte libéré par `_output` en fin de portée, avant que - // `with_staged_output` ne renomme le MP4. + // `with_validated_mp4_output` ne renomme le MP4. } let wall_s = t0.elapsed().as_secs_f64(); diff --git a/crates/compositor/src/pipeline_windows.rs b/crates/compositor/src/pipeline_windows.rs index 28bcc4faa7..a697326b5c 100644 --- a/crates/compositor/src/pipeline_windows.rs +++ b/crates/compositor/src/pipeline_windows.rs @@ -13,7 +13,7 @@ use crate::config::Cfg; use crate::cpu_frames::CpuFrames; use crate::cursor::CursorTrack; use crate::d3d::{Backend, Gpu}; -use crate::export_control::{with_staged_output, ExportControl}; +use crate::export_control::{with_validated_mp4_output, ExportControl}; use crate::ffi::*; use crate::regions::{speed_segments_for_window, SpeedSegment}; use crate::scene::Scene; @@ -1343,7 +1343,7 @@ pub fn run_composited_multi_cancellable( progress: &mut dyn FnMut(u64), control: &ExportControl, ) -> Result { - with_staged_output(Path::new(out), control, |file, staged| { + with_validated_mp4_output(Path::new(out), control, |file, staged| { drop(file); // ffmpeg le rouvre par son nom unsafe { run_multi_inner(clips, &staged.to_string_lossy(), gpu, comp, cfg, params, progress, control) @@ -1875,7 +1875,7 @@ unsafe fn run_multi_inner( let wall_s = t0.elapsed().as_secs_f64(); // teardown : les gardes et les Drop (décodeurs, encodeurs) en fin de portée — fichier fermé - // avant que `with_staged_output` ne le renomme. + // avant que `with_validated_mp4_output` ne le renomme. let fps = frames as f64 / wall_s; Ok(Stats { frames, wall_s, fps, video_duration_s: frames as f64 / out_fps as f64 }) } diff --git a/crates/compositor/src/timeline_walk.rs b/crates/compositor/src/timeline_walk.rs index 28e8518ada..59c00150eb 100644 --- a/crates/compositor/src/timeline_walk.rs +++ b/crates/compositor/src/timeline_walk.rs @@ -262,6 +262,7 @@ 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. unsafe fn seek_clip_start(decoder: &mut Decoder, target: f64) -> Result { let frame = decoder.seek_to_or_last(target)?; Ok(!frame.is_null() @@ -412,7 +413,9 @@ unsafe fn walk_timeline( // un seul seek keyframe, puis chaque décodeur avance selon son propre pts jusqu'aux // temps source demandés par les spans de vitesse. - if !seek_clip_start(sdec, clip.source_start_sec)? { + if !seek_clip_start(sdec, clip.source_start_sec) + .with_context(|| format!("Cannot export clip {}: could not read the screen video.", clip_index + 1))? + { bail!("Cannot export clip {}: the screen video has no readable frame at the clip's start.", clip_index + 1); } let webcam_start = if has_camera { diff --git a/crates/compositor/tests/export_timing.rs b/crates/compositor/tests/export_timing.rs index 8581ebe2cd..ba917d2027 100644 --- a/crates/compositor/tests/export_timing.rs +++ b/crates/compositor/tests/export_timing.rs @@ -231,6 +231,7 @@ fn mp4_export_rejects_an_unreadable_camera_without_publishing() { std::fs::remove_dir_all(out_dir).expect("remove test outputs"); } +/// An entirely out-of-range timeline cannot publish an empty replacement. #[test] fn mp4_export_rejects_a_timeline_that_produces_no_frames() { let Some(dir) = media_dir() else { @@ -426,6 +427,103 @@ fn mp4_export_bounds_tail_recovery_and_reports_the_correct_source() { std::fs::remove_dir_all(out_dir).expect("remove test outputs"); } +/// The staged decode check must accept short exports in both supported MP4 codecs. +#[cfg(target_os = "macos")] +#[test] +fn mp4_export_validation_accepts_h264_and_hevc() { + let Some(dir) = media_dir() else { + eprintln!("skipped: set OPENSCREEN_TEST_MEDIA"); + return; + }; + let out_dir = dir.join(format!("validated_codecs-{}", 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"); + for codec in [ExportCodec::H264, ExportCodec::H265] { + let params = ExportParams { + width: 640, height: 360, fps: Some(30), codec, bit_rate: None, + }; + let mut clip = whole_clip(&dir); + clip.source_end_sec = 0.2; + let out = out_dir.join("out.mp4"); + std::fs::write(&out, b"previous export").expect("seed destination"); + let stats = pipeline::run_composited_multi( + &[clip], &out.to_string_lossy(), &gpu, &comp, &Cfg::c8(), ¶ms, &mut |_| {}, + ).expect("valid encoded video must pass validation"); + assert_eq!(stats.frames, 6); + assert_ne!(std::fs::read(&out).unwrap(), b"previous export"); + std::fs::remove_file(out).expect("remove output"); + assert_eq!(std::fs::read_dir(&out_dir).unwrap().count(), 0); + } + std::fs::remove_dir(out_dir).expect("remove output dir"); +} + +/// VideoToolbox can mux one tail sample with zero duration and mark it discard. +/// A positive composed-frame count must not let that file replace a good export. +#[cfg(target_os = "macos")] +#[test] +fn mp4_export_rejects_an_undecodable_single_tail_frame_without_publishing() { + let Some(dir) = media_dir() else { + eprintln!("skipped: set OPENSCREEN_TEST_MEDIA"); + return; + }; + let out_dir = dir.join(format!("undecodable_single_tail-{}", std::process::id())); + std::fs::create_dir_all(&out_dir).expect("output dir"); + let out = out_dir.join("out.mp4"); + 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, + }; + let mut tail = whole_clip(&dir); + tail.webcam.clear(); + tail.source_start_sec = SOURCE_SEC - 0.01; + for existing_destination in [false, true] { + if existing_destination { + std::fs::write(&out, b"previous export").expect("seed destination"); + } + let result = pipeline::run_composited_multi( + &[ClipSource { + screen: tail.screen.clone(), + webcam: String::new(), + source_start_sec: tail.source_start_sec, + source_end_sec: tail.source_end_sec, + webcam_offset_sec: 0.0, + has_audio: false, + }], + &out.to_string_lossy(), + &gpu, + &comp, + &Cfg::c8(), + ¶ms, + &mut |_| {}, + ); + let error = result + .err() + .expect("undecodable output must not be published"); + assert!( + format!("{error:#}").contains("validating exported video"), + "{error:#}" + ); + if existing_destination { + assert_eq!(std::fs::read(&out).unwrap(), b"previous export"); + std::fs::remove_file(&out).expect("remove destination"); + } else { + assert!(!out.exists()); + } + assert_eq!( + std::fs::read_dir(&out_dir).unwrap().count(), + 0, + "partial output left behind" + ); + } + std::fs::remove_dir(out_dir).expect("remove output dir"); +} + /// Cancelling an MP4 mid-render stops the walk at the next frame, reports `ExportCancelled`, /// and publishes nothing: the destination keeps what it held and no partial is left beside /// it. On Windows that last check also proves the muxer closed its file, since an open diff --git a/technical-documentation/architecture/export-pipeline.md b/technical-documentation/architecture/export-pipeline.md index 6149c5d461..79bdcb80d3 100644 --- a/technical-documentation/architecture/export-pipeline.md +++ b/technical-documentation/architecture/export-pipeline.md @@ -153,7 +153,14 @@ and **one** encoder + muxer pair: within the 0.5-second tail margin (audio can outlast video). Clips starting beyond their declared end within this margin retain the existing skip behavior; an entirely empty export still fails. Screen-only clips report - screen errors, including errors from the stand-in decoder. + screen errors, including errors from the stand-in decoder. MP4 export then + closes and reopens the staged file and software-decodes until the first + displayable video frame before replacing the destination. Discard-marked + preroll is decoded to maintain reference state, but discard-marked output + frames do not satisfy validation. A single muxed sample or positive render + count is not sufficient. Validation errors or cancellation remove the staged + file and preserve the destination. This is a first-frame check, not a + full-file integrity scan; encoder timing for single-frame files is not repaired. - **Imported audio tracks** (voiceover / BGM / SFX, issue #350) are mixed on top of the assembled programme by `audio.rs::mix_external_tracks`, diff --git a/technical-documentation/testing/manual-e2e-checklist.md b/technical-documentation/testing/manual-e2e-checklist.md index c6bf2102ef..f09cf468b8 100644 --- a/technical-documentation/testing/manual-e2e-checklist.md +++ b/technical-documentation/testing/manual-e2e-checklist.md @@ -913,6 +913,7 @@ 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 | 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. | | 2026-09-12 | dev `fix/643-gif-export-cancel`, based on `fbe461e9`; rebuilt compositor addon SHA256 `128c89fe65a459b3f17721d610ea9a78e0d3e980e9eb0b5fb2b0e5d018619e15` | Windows | Partial — export cancellation passed | Computer-use OS input, real Electron and native save dialog, imported synthetic 8 s source. GIF 15 FPS / Medium (720p) / loop on: observed 55/120 frames, clicked Cancel while rendering (pending snapshot 104/120), returned to the same options; no final or partial output remained. Retried in that dialog to the same path and received Saved to; resulting GIF is 1280×720, 120 frames, 8,200,202 bytes, fully decodes with ffmpeg (exit 0). Automated real-addon tests separately pass existing-destination preservation, GIF retry, ordinary GIF and MP4 export. Desktop adapter required screenshot-based coordinates and Ctrl+O to reach the editor; HUD reachability is not claimed. An unrelated process error window partly obscured the retry success screenshot; file verification independently passed. Skipped: recording, audio/mic, webcam, tray/HUD, effect/caption comparisons, second viewer, macOS/Linux and release promotion. Task-local profile and evidence retained; test app stopped. | From aa4a30868488217956f84198fdd8f8a3b21200c6 Mon Sep 17 00:00:00 2001 From: songsong Date: Sat, 24 Oct 2026 20:37:06 +0800 Subject: [PATCH 5/5] fix(export): stop frame inspection before unrequested clips --- crates/compositor/src/export_control.rs | 1 + crates/compositor/src/pipeline_linux.rs | 2 + crates/compositor/src/pipeline_macos.rs | 2 + crates/compositor/src/pipeline_windows.rs | 2 + crates/compositor/src/timeline_walk.rs | 6 ++ crates/compositor/tests/export_timing.rs | 57 +++++++++++++++++++ .../architecture/export-pipeline.md | 4 +- 7 files changed, 73 insertions(+), 1 deletion(-) diff --git a/crates/compositor/src/export_control.rs b/crates/compositor/src/export_control.rs index 4be3502652..d289dc55cd 100644 --- a/crates/compositor/src/export_control.rs +++ b/crates/compositor/src/export_control.rs @@ -132,6 +132,7 @@ struct VideoValidation { } impl Drop for VideoValidation { + /// Release decoder buffers and close the input before rename or cleanup. fn drop(&mut self) { unsafe { crate::ffi::av_frame_free(&mut self.frame); diff --git a/crates/compositor/src/pipeline_linux.rs b/crates/compositor/src/pipeline_linux.rs index 15c8d2b152..288c2fc48a 100644 --- a/crates/compositor/src/pipeline_linux.rs +++ b/crates/compositor/src/pipeline_linux.rs @@ -878,6 +878,8 @@ pub fn run_composited_multi_cancellable( }) } +/// Encode and close the staged MP4. The caller must validate a displayable +/// decoded frame before publishing it; returned stats count composed frames. fn run_multi_inner( clips: &[ClipSource], out: &str, diff --git a/crates/compositor/src/pipeline_macos.rs b/crates/compositor/src/pipeline_macos.rs index 5cebfa1ed4..b407d7966b 100644 --- a/crates/compositor/src/pipeline_macos.rs +++ b/crates/compositor/src/pipeline_macos.rs @@ -1191,6 +1191,8 @@ pub fn run_composited_multi_cancellable( }) } +/// Encode and close the staged MP4. The caller must validate a displayable +/// decoded frame before publishing it; returned stats count composed frames. fn run_multi_inner( clips: &[ClipSource], out: &str, diff --git a/crates/compositor/src/pipeline_windows.rs b/crates/compositor/src/pipeline_windows.rs index a697326b5c..39ad180fab 100644 --- a/crates/compositor/src/pipeline_windows.rs +++ b/crates/compositor/src/pipeline_windows.rs @@ -1680,6 +1680,8 @@ impl Default for ExportParams { } +/// Encode and close the staged MP4. The caller must validate a displayable +/// decoded frame before publishing it; returned stats count composed frames. unsafe fn run_multi_inner( clips: &[ClipSource], out: &str, diff --git a/crates/compositor/src/timeline_walk.rs b/crates/compositor/src/timeline_walk.rs index 59c00150eb..3ad2456a30 100644 --- a/crates/compositor/src/timeline_walk.rs +++ b/crates/compositor/src/timeline_walk.rs @@ -310,6 +310,12 @@ unsafe fn walk_timeline( comp.set_segmentation_deterministic(true); '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) { + break 'clips; + } + } // Le preset de layout est GLOBAL (un seul panneau pour toute la timeline) mais la // caméra est PAR CLIP : un projet mélange sans problème un enregistrement avec webcam // et un import qui n'en a pas. Le preset ne doit donc s'appliquer qu'aux clips qui ont diff --git a/crates/compositor/tests/export_timing.rs b/crates/compositor/tests/export_timing.rs index ba917d2027..7bc5d0e879 100644 --- a/crates/compositor/tests/export_timing.rs +++ b/crates/compositor/tests/export_timing.rs @@ -427,6 +427,63 @@ fn mp4_export_bounds_tail_recovery_and_reports_the_correct_source() { std::fs::remove_dir_all(out_dir).expect("remove test outputs"); } +/// 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] +fn frame_inspection_stops_before_an_unrequested_unreadable_clip() { + let Some(dir) = media_dir() else { + eprintln!("skipped: set OPENSCREEN_TEST_MEDIA"); + return; + }; + let gpu = Gpu::create(false).expect("gpu"); + let comp = Compositor::new_sized(&gpu, 640, 360).expect("compositor"); + let mut bad = whole_clip(&dir); + bad.webcam_offset_sec = -(SOURCE_SEC + 1.0); + let clips = [whole_clip(&dir), bad]; + let frames = unsafe { + openscreen_compositor::timeline_walk::render_frames( + &clips, + &gpu, + &comp, + &Cfg::c8(), + 30, + &[119], + ) + } + .expect("the last frame of the valid clip must not inspect the next camera"); + assert_eq!(frames.len(), 1); + assert_eq!(frames[0].index, 119); + let empty = unsafe { + openscreen_compositor::timeline_walk::render_frames( + &clips[1..], + &gpu, + &comp, + &Cfg::c8(), + 30, + &[], + ) + } + .expect("an empty selection must not prepare any clip"); + assert!(empty.is_empty()); + let error = unsafe { + openscreen_compositor::timeline_walk::render_frames( + &clips, + &gpu, + &comp, + &Cfg::c8(), + 30, + &[120], + ) + } + .err() + .expect("a requested unreadable clip must still fail"); + let message = format!("{error:#}"); + assert!( + message.contains("clip 2") && message.contains("camera"), + "{message}" + ); +} + /// The staged decode check must accept short exports in both supported MP4 codecs. #[cfg(target_os = "macos")] #[test] diff --git a/technical-documentation/architecture/export-pipeline.md b/technical-documentation/architecture/export-pipeline.md index 79bdcb80d3..37bdda4967 100644 --- a/technical-documentation/architecture/export-pipeline.md +++ b/technical-documentation/architecture/export-pipeline.md @@ -148,7 +148,9 @@ and **one** encoder + muxer pair: never a successful empty file. These errors discard the staged output and preserve an existing destination, including when earlier clips rendered successfully. Frame inspection (`renderFrames`) may still return an empty - selection when the requested frame indices are outside the timeline. Both + selection when the requested frame indices are outside the timeline. It stops + before opening subsequent clips once its requested frame range is exhausted, + so an unrelated later camera cannot fail an already completed selection. Both export and frame inspection hold the last video frame when entering a clip within the 0.5-second tail margin (audio can outlast video). Clips starting beyond their declared end within this margin retain the existing skip