Skip to content

Commit f09670c

Browse files
songoao25EtienneLescot
authored andcommitted
fix(export): preserve first-frame hold for offset sources
1 parent 1f38edd commit f09670c

4 files changed

Lines changed: 136 additions & 50 deletions

File tree

‎crates/compositor/src/timeline_walk.rs‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -262,12 +262,26 @@ unsafe fn catch_up(dec: &mut Decoder, target: f64, clip_start: f64) -> Result<()
262262
/// hide a genuinely truncated camera recording.
263263
const CLIP_START_TAIL_TOLERANCE_SEC: f64 = 0.5;
264264

265-
/// Seek a clip start within the normal tail margin in either direction.
266-
/// A seek may return a later frame, so also reject large interior gaps.
265+
/// Seek a clip start within the normal tail margin. A future frame across an
266+
/// interior gap is invalid, but a request before the first frame of an offset
267+
/// stream retains the existing first-frame hold.
267268
unsafe fn seek_clip_start(decoder: &mut Decoder, target: f64) -> Result<bool> {
268269
let frame = decoder.seek_to_or_last(target)?;
269-
Ok(!frame.is_null()
270-
&& (target - decoder.cur_time_sec()).abs() <= CLIP_START_TAIL_TOLERANCE_SEC + 1e-6)
270+
if frame.is_null() {
271+
return Ok(false);
272+
}
273+
let current = decoder.cur_time_sec();
274+
if (target - current).abs() <= CLIP_START_TAIL_TOLERANCE_SEC + 1e-6 {
275+
return Ok(true);
276+
}
277+
if current <= target {
278+
return Ok(false);
279+
}
280+
// Only a distant future result needs this extra seek. Distinguish a late
281+
// stream origin from a missing interior interval using the actual first
282+
// frame, without changing decoder timestamps or the normal seek path.
283+
let first = decoder.seek_to(0.0)?;
284+
Ok(!first.is_null() && decoder.cur_time_sec() > target)
271285
}
272286

273287
/// `walk_composited_timeline`, qui ne compose que les frames de sortie `only` (triées) quand il y

‎crates/compositor/tests/export_timing.rs‎

Lines changed: 113 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@
99
//! (red → green → blue → white). A correct export spans all four colours; a
1010
//! mis-advanced one stays red for its whole length.
1111
//!
12-
//! Needs a D3D11 GPU and the generated media, so it is opt-in: set
12+
//! Needs a native GPU backend and the generated media, so it is opt-in: set
1313
//! OPENSCREEN_TEST_MEDIA to a directory holding `screen_colors.mp4` and
14-
//! `webcam_gray.mp4`. The interior-gap test also needs `video_gap.mp4`.
14+
//! `webcam_gray.mp4`. Gap/origin tests also need the derived fixtures below.
1515
//! Without OPENSCREEN_TEST_MEDIA every test here skips.
1616
//!
1717
//! Regenerate the media with the vendored ffmpeg:
@@ -23,6 +23,8 @@
2323
//! -c:v libopenh264 -g 60 -pix_fmt yuv420p webcam_gray.mp4
2424
//! ffmpeg -i screen_colors.mp4 -vf "select='lt(t,1)+gte(t,3)'" \
2525
//! -fps_mode vfr -c:v libopenh264 -g 60 -pix_fmt yuv420p video_gap.mp4
26+
//! ffmpeg -i screen_colors.mp4 -c copy -output_ts_offset 1 offset_start.mp4
27+
//! ffmpeg -i video_gap.mp4 -c copy -output_ts_offset 1 offset_gap.mp4
2628
//! The gap fixture must retain timestamps: video frames stop at 0.983 s and
2729
//! resume at 3.000 s. An equivalent H.264 encoder is sufficient if libopenh264
2830
//! is unavailable.
@@ -441,11 +443,6 @@ fn mp4_export_rejects_a_start_inside_a_multisecond_video_gap() {
441443
eprintln!("skipped: set OPENSCREEN_TEST_MEDIA");
442444
return;
443445
};
444-
let gap = dir.join("video_gap.mp4");
445-
assert!(
446-
gap.exists(),
447-
"generate video_gap.mp4 using the fixture header recipe"
448-
);
449446
let out_dir = dir.join(format!("future_frame_gap-{}", std::process::id()));
450447
std::fs::create_dir_all(&out_dir).expect("output dir");
451448
let gpu = Gpu::create(false).expect("gpu");
@@ -457,49 +454,120 @@ fn mp4_export_rejects_a_start_inside_a_multisecond_video_gap() {
457454
codec: ExportCodec::H264,
458455
bit_rate: None,
459456
};
460-
for camera_gap in [false, true] {
461-
for after_valid_clip in [false, true] {
462-
let mut bad = whole_clip(&dir);
463-
if camera_gap {
464-
bad.webcam = gap.to_string_lossy().into_owned();
465-
} else {
466-
bad.screen = gap.to_string_lossy().into_owned();
467-
}
468-
bad.source_start_sec = 1.5;
469-
bad.source_end_sec = 3.5;
470-
let mut clips = Vec::new();
471-
if after_valid_clip {
472-
clips.push(whole_clip(&dir));
457+
for timestamp_offset in [0.0, 1.0] {
458+
let gap = dir.join(if timestamp_offset == 0.0 {
459+
"video_gap.mp4"
460+
} else {
461+
"offset_gap.mp4"
462+
});
463+
assert!(
464+
gap.exists(),
465+
"generate gap fixtures using the fixture header recipe"
466+
);
467+
for camera_gap in [false, true] {
468+
for after_valid_clip in [false, true] {
469+
let mut bad = whole_clip(&dir);
470+
if camera_gap {
471+
bad.webcam = gap.to_string_lossy().into_owned();
472+
} else {
473+
bad.screen = gap.to_string_lossy().into_owned();
474+
}
475+
bad.source_start_sec = 1.5 + timestamp_offset;
476+
bad.source_end_sec = 3.5 + timestamp_offset;
477+
let mut clips = Vec::new();
478+
if after_valid_clip {
479+
clips.push(whole_clip(&dir));
480+
}
481+
clips.push(bad);
482+
let out = out_dir.join("out.mp4");
483+
std::fs::write(&out, b"previous export").expect("seed destination");
484+
let result = pipeline::run_composited_multi(
485+
&clips,
486+
&out.to_string_lossy(),
487+
&gpu,
488+
&comp,
489+
&Cfg::c8(),
490+
&params,
491+
&mut |_| {},
492+
);
493+
let error = result
494+
.err()
495+
.expect("a future frame cannot stand in for a missing start");
496+
let message = format!("{error:#}");
497+
assert!(
498+
message.contains(if camera_gap { "camera" } else { "screen" }),
499+
"{message}"
500+
);
501+
assert!(
502+
message.contains(&format!("clip {}", clips.len())),
503+
"{message}"
504+
);
505+
assert_eq!(std::fs::read(&out).unwrap(), b"previous export");
506+
assert_eq!(std::fs::read_dir(&out_dir).unwrap().count(), 1);
507+
std::fs::remove_file(out).expect("remove destination");
473508
}
474-
clips.push(bad);
475-
let out = out_dir.join("out.mp4");
476-
std::fs::write(&out, b"previous export").expect("seed destination");
477-
let result = pipeline::run_composited_multi(
478-
&clips,
479-
&out.to_string_lossy(),
480-
&gpu,
481-
&comp,
482-
&Cfg::c8(),
483-
&params,
484-
&mut |_| {},
485-
);
486-
let error = result
487-
.err()
488-
.expect("a future frame cannot stand in for a missing start");
489-
let message = format!("{error:#}");
490-
assert!(
491-
message.contains(if camera_gap { "camera" } else { "screen" }),
492-
"{message}"
493-
);
509+
}
510+
}
511+
std::fs::remove_dir(out_dir).expect("remove output dir");
512+
}
513+
514+
/// A non-zero first video timestamp is a valid stream origin, not an interior gap.
515+
#[test]
516+
fn mp4_export_accepts_a_nonzero_first_video_timestamp() {
517+
let Some(dir) = media_dir() else {
518+
eprintln!("skipped: set OPENSCREEN_TEST_MEDIA");
519+
return;
520+
};
521+
let offset = dir.join("offset_start.mp4");
522+
assert!(
523+
offset.exists(),
524+
"generate offset_start.mp4 using the fixture header recipe"
525+
);
526+
let out_dir = dir.join(format!("offset_start-{}", std::process::id()));
527+
std::fs::create_dir_all(&out_dir).expect("output dir");
528+
let gpu = Gpu::create(false).expect("gpu");
529+
let comp = Compositor::new_sized(&gpu, 640, 360).expect("compositor");
530+
#[cfg(target_os = "macos")]
531+
{
532+
let mut decoder =
533+
pipeline::Decoder::open_for_export(&offset.to_string_lossy(), &gpu).expect("decoder");
534+
unsafe {
535+
assert!(!decoder.seek_to(0.0).expect("first frame").is_null());
494536
assert!(
495-
message.contains(&format!("clip {}", clips.len())),
496-
"{message}"
537+
(decoder.cur_time_sec() - 1.0).abs() < 0.01,
538+
"fixture must have a non-zero first timestamp"
497539
);
498-
assert_eq!(std::fs::read(&out).unwrap(), b"previous export");
499-
assert_eq!(std::fs::read_dir(&out_dir).unwrap().count(), 1);
500-
std::fs::remove_file(out).expect("remove destination");
501540
}
502541
}
542+
let params = ExportParams {
543+
width: 640,
544+
height: 360,
545+
fps: Some(30),
546+
codec: ExportCodec::H264,
547+
bit_rate: None,
548+
};
549+
for camera_offset in [false, true] {
550+
let mut clip = whole_clip(&dir);
551+
if camera_offset {
552+
clip.webcam = offset.to_string_lossy().into_owned();
553+
} else {
554+
clip.screen = offset.to_string_lossy().into_owned();
555+
}
556+
let out = out_dir.join("out.mp4");
557+
let frames = pipeline::run_composited_multi(
558+
&[clip],
559+
&out.to_string_lossy(),
560+
&gpu,
561+
&comp,
562+
&Cfg::c8(),
563+
&params,
564+
&mut |_| {},
565+
)
566+
.expect("a non-zero stream origin must export");
567+
assert_eq!(frames.frames, 120);
568+
assert!(pipeline::probe_frame_count(&out.to_string_lossy()).expect("decode output") > 0);
569+
std::fs::remove_file(out).expect("remove export");
570+
}
503571
std::fs::remove_dir(out_dir).expect("remove output dir");
504572
}
505573

‎technical-documentation/architecture/export-pipeline.md‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,10 @@ and **one** encoder + muxer pair:
142142
more than 0.5 seconds before the clip starts, or cannot produce a frame
143143
within 0.5 seconds of its requested start offset in either direction (including
144144
a seek landing on a future frame across a gap), the shared MP4/GIF timeline walk fails with the clip number and a
145-
camera-specific explanation. This also covers browser recordings whose
145+
camera-specific explanation. A request before a non-zero first-frame timestamp
146+
still holds that first frame; this valid stream origin is distinguished from
147+
an interior gap by seeking the actual first frame only on the distant-future
148+
fallback path. Decoder timestamps and the normal seek path are unchanged. This also covers browser recordings whose
146149
declared container duration outlasts their actual video frames. The user can
147150
re-record the camera or remove its track explicitly; the exporter does not
148151
discard it automatically. A walk producing no video frames is an error,

‎technical-documentation/testing/manual-e2e-checklist.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -913,6 +913,7 @@ These checks are the human slice: an agent cannot click Apple's picker (see abov
913913

914914
| Date | Build / tag | Platform | Pass/fail | Notes |
915915
|------|-------------|----------|-----------|-------|
916+
| 2026-10-24 | PR #1379 offset-origin follow-up; rebuilt addon SHA256 `0dd01525a163d8889dd2aaa7088ea0c1e923956b14c430ff70537cba5c24e9d5` | macOS 15.7.1, Apple Silicon | Partial — offset source exported; interior gap still rejected | Computer-use opened a synthetic video whose first decoded timestamp is 1.000 s, with a clip starting at 0. MP4 1080p/30 displayed Saved; independent probing counted 120 frames / 3.967 s at 1752×1080 and full-file decode exited 0. In the same session, opened the interior-gap project and confirmed replacement of that MP4: export reported the screen-start error, destination SHA256 stayed `d45192c6c319006849fee014bc3882141bead0f8b0c43e7c895c51cefe1b7d7b`, and no partial remained. Native regressions cover screen/camera offset sources and interior gaps with both zero and non-zero stream origins. Recording, Presenter Overlay compatibility, unrelated editor features, Windows/Linux runtime and release packaging were not exercised. |
916917
| 2026-10-24 | dev post-merge follow-up to PR #1377 on `79c3aa24`; rebuilt addon SHA256 `53df86189b359d41b63a76e3eaafed5e9bf0e299fe0e89f8fd7950aa12bad6f3` | macOS 15.7.1, Apple Silicon | Partial — interior gap rejected; destination preserved; normal retry passed | Computer-use opened a synthetic screen clip starting at 1.5 s inside a timestamp gap from 0.983 to 3.000 s, then confirmed replacement of an existing synthetic MP4. Export reported that clip 1 had no readable screen frame at its start. Destination SHA256 stayed `854b0911778da18c2a456d3eb227da749c0af2138ad52092180156ed138a6a48`; no partial remained. In the same session, a normal four-second clip plus 10 ms tail exported successfully at 1080p/30: independent ffprobe counted 120 frames / 4.000 s at 1752×1080, and full-file decode exited 0. Native tests also cover camera gaps and failure after a valid preceding clip. Recording, Presenter Overlay compatibility, unrelated editor features, Windows/Linux runtime and release packaging were not exercised. |
917918
| 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. |
918919
| 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. |

0 commit comments

Comments
 (0)