diff --git a/crates/compositor/src/export_control.rs b/crates/compositor/src/export_control.rs index 8542c55ea..d289dc55c 100644 --- a/crates/compositor/src/export_control.rs +++ b/crates/compositor/src/export_control.rs @@ -106,6 +106,145 @@ 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 { + /// Release decoder buffers and close the input before rename or cleanup. + 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 +268,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 e0fd1c262..288c2fc48 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,12 +872,14 @@ 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) }) } +/// 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 638b664f4..b407d7966 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,12 +1185,14 @@ 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) }) } +/// 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, @@ -1391,7 +1393,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 28bcc4faa..39ad180fa 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) @@ -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, @@ -1875,7 +1877,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 5556e577b..3ad2456a3 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`. @@ -253,6 +257,18 @@ 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; + +/// 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() + && 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 @@ -294,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 @@ -350,6 +372,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_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, + ); + } let mut source_end_sec = clip.source_end_sec; if let Some(duration) = screen_available_duration { source_end_sec = source_end_sec.min(duration); @@ -391,14 +419,23 @@ 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 - } - if wdec - .seek_to((clip.source_start_sec - clip.webcam_offset_sec).max(0.0))? - .is_null() + if !seek_clip_start(sdec, clip.source_start_sec) + .with_context(|| format!("Cannot export clip {}: could not read the screen video.", clip_index + 1))? { - continue; + 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 { + (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 cffda4469..7bc5d0e87 100644 --- a/crates/compositor/tests/export_timing.rs +++ b/crates/compositor/tests/export_timing.rs @@ -161,6 +161,426 @@ 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"); +} + +/// 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 { + 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"); +} + +/// 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"); +} + +/// 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] +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 b1c99de54..37bdda496 100644 --- a/technical-documentation/architecture/export-pipeline.md +++ b/technical-documentation/architecture/export-pipeline.md @@ -138,6 +138,32 @@ 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 + 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 + 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. 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 + behavior; an entirely empty export still fails. Screen-only clips report + 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`, between `assemble_concatenated_pcm` and `finish_audio`. Each track's diff --git a/technical-documentation/testing/manual-e2e-checklist.md b/technical-documentation/testing/manual-e2e-checklist.md index 340df1480..f09cf468b 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 | 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. | | 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). |