From 9cd9a80baebbc3b584725c0f578f8168ebd82efb Mon Sep 17 00:00:00 2001 From: Aditya Garud <153842990+yashranaway@users.noreply.github.com> Date: Sat, 17 Oct 2026 17:29:58 +0000 Subject: [PATCH] fix(rendering): preserve late camera timestamps in FFmpeg decoder --- crates/rendering/src/decoder/ffmpeg.rs | 19 ++--- .../src/decoder/model/expectations.toml | 7 ++ crates/rendering/src/decoder/model/mod.rs | 75 ++++++++++++------- crates/verify/expectations.toml | 7 -- 4 files changed, 63 insertions(+), 45 deletions(-) diff --git a/crates/rendering/src/decoder/ffmpeg.rs b/crates/rendering/src/decoder/ffmpeg.rs index 1a06901233d..ec8e7a1345d 100644 --- a/crates/rendering/src/decoder/ffmpeg.rs +++ b/crates/rendering/src/decoder/ffmpeg.rs @@ -574,7 +574,6 @@ impl FfmpegDecoder { }; let time_base = this.decoder().time_base(); - let start_time = this.start_time(); let video_width = this.decoder().width(); let video_height = this.decoder().height(); let is_hw = this.is_hardware_accelerated(); @@ -626,7 +625,6 @@ impl FfmpegDecoder { }; let sw_time_base = sw_this.decoder().time_base(); - let sw_start_time = sw_this.start_time(); let mut sw_cache = BTreeMap::::new(); let mut sw_last_active_frame = None::; @@ -661,8 +659,7 @@ impl FfmpegDecoder { return; } if let Some(frame) = first_software_frame { - let current_frame = - pts_to_frame(frame.pts().unwrap_or(0) - sw_start_time, sw_time_base, fps); + let current_frame = pts_to_frame(frame.pts().unwrap_or(0), sw_time_base, fps); let mut cache_frame = CachedFrame::Raw { frame, number: current_frame, @@ -896,8 +893,7 @@ impl FfmpegDecoder { if let Ok(frame) = &frame && let Some(pts) = frame.pts() { - sw_reader_at = - Some(pts_to_frame(pts - sw_start_time, sw_time_base, fps)); + sw_reader_at = Some(pts_to_frame(pts, sw_time_base, fps)); } break; } @@ -913,7 +909,7 @@ impl FfmpegDecoder { sw_prev_vended = None; continue; }; - let current_frame = pts_to_frame(pts - sw_start_time, sw_time_base, fps); + let current_frame = pts_to_frame(pts, sw_time_base, fps); if let Some(prev) = sw_prev_vended && current_frame > prev + 1 @@ -1138,7 +1134,7 @@ impl FfmpegDecoder { return; } if let Some(frame) = first_frame_result { - let current_frame = pts_to_frame(frame.pts().unwrap_or(0) - start_time, time_base, fps); + let current_frame = pts_to_frame(frame.pts().unwrap_or(0), time_base, fps); let mut cache_frame = CachedFrame::Raw { frame, number: current_frame, @@ -1208,8 +1204,7 @@ impl FfmpegDecoder { match frames.next() { Some(Ok(frame)) => { if let Some(pts) = frame.pts() { - let current_frame = - pts_to_frame(pts - start_time, time_base, fps); + let current_frame = pts_to_frame(pts, time_base, fps); if let Some(prev) = prev_vended && current_frame > prev + 1 { @@ -1465,7 +1460,7 @@ impl FfmpegDecoder { if let Ok(frame) = &frame && let Some(pts) = frame.pts() { - reader_at = Some(pts_to_frame(pts - start_time, time_base, fps)); + reader_at = Some(pts_to_frame(pts, time_base, fps)); } break; } @@ -1481,7 +1476,7 @@ impl FfmpegDecoder { prev_vended = None; continue; }; - let current_frame = pts_to_frame(pts - start_time, time_base, fps); + let current_frame = pts_to_frame(pts, time_base, fps); if let Some(prev) = prev_vended && current_frame > prev + 1 diff --git a/crates/rendering/src/decoder/model/expectations.toml b/crates/rendering/src/decoder/model/expectations.toml index ac2eb15e2c7..2f1594266af 100644 --- a/crates/rendering/src/decoder/model/expectations.toml +++ b/crates/rendering/src/decoder/model/expectations.toml @@ -43,6 +43,13 @@ flaky = true reason = "Seeds 28, 53 and 59 fail in every run; which wrong answer comes first depends on the wall clock. Without a stall the first wrong answer is a future one at a seek (seed 28 request 61, seed 53 request 8, seed 59 request 9: the next sample, e.g. sample 68 for 67): pool position capping, the reader's pool position is capped to the newest request (avassetreader.rs update_decoder_position(decoder_idx, capped)) so a reader that already decoded past the sample is reused without a reset. When the test thread stalls for 250 ms or more, the decoder's idle timeout (IDLE_READER_TIMEOUT, avassetreader.rs) clears the cache and reader state, the later requests take another path and the first wrong answer is a stale one from the wrong-GOP seek (reader started on the keyframe time rounded to 10 ms). Measured on Apple silicon: future 3 of 3 per seed without a stall, stale for all three seeds with a 300 ms stall before every request, and for seed 53 with a single 300 ms stall before request 6 (a 240 ms stall keeps future). macos-15: future in 6 of 7 runs, stale in 1 (38743345280). With #2595's decoder changes seed 28 passes and seeds 53 and 59 keep only the stale (wrong-GOP) answer" link = "https://github.com/CapSoftware/Cap/pull/2595 https://github.com/CapSoftware/Cap/pull/2603" +[[xfail]] +case = "decoder-model/ffmpeg/late-start/random-seeks/playback" +class = "grid-rounding" +seeds = "0 6 9 11 16" +reason = "Same seek-to-request grid rounding as the zero-start FFmpeg cases below: a seek can land on the frame on screen at the exact request time even though its rounded grid label is after the request. Newly covered by enabling late-start streams for FFmpeg; no future or stale answers remain" +link = "" + [[xfail]] case = "decoder-model/ffmpeg/camera-fast/random-seeks/playback" class = "grid-rounding" diff --git a/crates/rendering/src/decoder/model/mod.rs b/crates/rendering/src/decoder/model/mod.rs index b29dac20e08..881b509268a 100644 --- a/crates/rendering/src/decoder/model/mod.rs +++ b/crates/rendering/src/decoder/model/mod.rs @@ -11,9 +11,9 @@ //! //! Reference rule (the decoder contract documented next to `pts_to_frame`): //! a sample is labelled `round(fps * pts_seconds)` on the decoder grid, where -//! FFmpeg measures pts from the stream start time and AVAssetReader from -//! zero. A request for time `t` asks for grid index `floor(t * fps)`. The -//! correct answer is the newest sample whose label is at or before that index +//! both decoders preserve the stream's presentation timeline. A request for +//! time `t` asks for grid index `floor(t * fps)`. The correct answer is the +//! newest sample whose label is at or before that index //! (the frame on screen at `t`); when several samples share that label any of //! them is accepted. A request before the first sample is answered with the //! first sample. @@ -184,20 +184,16 @@ fn label(pts: i64, time_base: (i32, i32), fps: u32) -> u32 { impl Oracle { fn new(script: &Script, decoder: Decoder, fps: u32) -> Self { - let origin = match decoder { - Decoder::Avf => 0, - Decoder::Ffmpeg => script.frames[0].pts, - }; Self { labels: script .frames .iter() - .map(|frame| label(frame.pts - origin, script.time_base, fps)) + .map(|frame| label(frame.pts, script.time_base, fps)) .collect(), secs: script .frames .iter() - .map(|frame| script.secs(frame.pts - origin)) + .map(|frame| script.secs(frame.pts)) .collect(), fps, decoder, @@ -447,7 +443,6 @@ struct Family { pattern: PatternKind, mode: Mode, seeds: usize, - avf_only: bool, } const fn family(source: SourceKind, pattern: PatternKind, mode: Mode, seeds: usize) -> Family { @@ -456,7 +451,6 @@ const fn family(source: SourceKind, pattern: PatternKind, mode: Mode, seeds: usi pattern, mode, seeds, - avf_only: false, } } @@ -494,20 +488,8 @@ const FAMILIES: &[Family] = { family(HalfGrid, BackIntoOldGap, Export, 60), family(Syn, Edlist, Playback, 8), family(Syn, Edlist, Export, 8), - Family { - source: LateStart, - pattern: Sequential, - mode: Playback, - seeds: 80, - avf_only: true, - }, - Family { - source: LateStart, - pattern: RandomSeeks, - mode: Playback, - seeds: 80, - avf_only: true, - }, + family(LateStart, Sequential, Playback, 80), + family(LateStart, RandomSeeks, Playback, 80), ] }; @@ -826,7 +808,6 @@ fn run_suite(decoder: Decoder) { let families: Vec<&Family> = FAMILIES .iter() - .filter(|family| decoder == Decoder::Avf || !family.avf_only) .filter(|family| { filter .as_ref() @@ -1117,3 +1098,45 @@ fn avassetreader_answers_match_the_reference_model() { fn ffmpeg_answers_match_the_reference_model() { run_suite(Decoder::Ffmpeg); } + +#[test] +fn ffmpeg_preserves_late_pts_across_playback_and_seeks() { + let script = Script { + time_base: (1, 30), + frames: (60..180) + .map(|pts| scripted::ScriptFrame { + pts, + keyframe: pts % 30 == 0, + }) + .collect(), + last_duration: 1, + }; + let runtime = runtime(); + for mode in [Mode::Export, Mode::Playback] { + let case = Case { + name: "late-pts".into(), + seed_index: 0, + seed: 0, + decoder: Decoder::Ffmpeg, + mode, + fps: 30, + max_fallback_distance: 2, + script: script.clone(), + requests: (0..180) + .chain([135, 15, 75, 165, 45, 105]) + .enumerate() + .map(|(position, frame)| Request { + time: (frame as f32 + 0.25) / 30.0, + initial: mode == Mode::Playback && (position == 0 || position >= 180), + }) + .collect(), + }; + let outcome = run_case(&case, &runtime, false); + assert!( + outcome.failures.is_empty(), + "{mode:?}: {:?}", + outcome.failures + ); + assert_eq!(outcome.requests, case.requests.len()); + } +} diff --git a/crates/verify/expectations.toml b/crates/verify/expectations.toml index 32dedfe7a42..a34f7322418 100644 --- a/crates/verify/expectations.toml +++ b/crates/verify/expectations.toml @@ -56,13 +56,6 @@ bound = { camera-frames = 1, delta = 1, frame = [2579, 2579] } reason = "AVAssetReader serves one camera frame 1 source frame stale after the timeline jumps backward (output frame 2579: source 1048 for 1049): the reader chosen after the jump had already vended past the request and its previous vend was no longer cached, so the nearest older cached frame answered. Every run in probe 38762089521 (42 of 42) and on hosted macOS, but it passed on blacksmith-6vcpu-macos-15 in push run 38846099909: whether the previous vend is still cached depends on the decoder's wall-clock idle timeout, so it is flaky there" link = "https://github.com/CapSoftware/Cap/pull/2638" -[[xfail]] -case = "export/camera-pts-late/full/mp430/ffmpeg" -class = "future" -bound = { camera-frames = 178, delta = 60, frame = [1, 178] } -reason = "NEW: FFmpeg decoder, same as the AVAssetReader case: a camera file whose pts start 2 s in runs ahead from the first output frame (178 of 179 frames, up to 60 source frames ahead, every platform)" -link = "" - # Decode deadline under machine starvation. These apply only to a case the # starvation sentinel saw stall (starved = true) and only with the signature # of the mechanism; the same symptom on a healthy machine is a failure.