From ad5e2eb968dbc9bf8eeaecbee516ed0ae0118a02 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Thu, 15 Oct 2026 04:54:42 +0900 Subject: [PATCH 01/10] fix(rendering): keep the newest pts holes when the decoder hole map is full --- crates/rendering/src/decoder/mod.rs | 44 ++++++++++++++++++++++++----- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/crates/rendering/src/decoder/mod.rs b/crates/rendering/src/decoder/mod.rs index 170507f3090..f3469668307 100644 --- a/crates/rendering/src/decoder/mod.rs +++ b/crates/rendering/src/decoder/mod.rs @@ -673,8 +673,9 @@ const DEFAULT_MAX_FALLBACK_DISTANCE: u32 = 90; /// Records a pts hole discovered from a decode-order vend jump (frames vend /// in pts order, so a jump means no samples exist in between). The map stays -/// bounded by dropping the narrowest hole — wide static-screen holds matter -/// most. +/// bounded by dropping the hole farthest from the one just recorded: requests +/// trail the reader, so the newest holes are the ones about to be asked for, +/// and a dropped hole is rediscovered whenever the reader passes it again. pub(super) fn record_pts_hole( holes: &mut std::collections::BTreeMap, start: u32, @@ -683,12 +684,14 @@ pub(super) fn record_pts_hole( const MAX_TRACKED_HOLES: usize = 64; holes.insert(start, end); if holes.len() > MAX_TRACKED_HOLES - && let Some(narrowest) = holes - .iter() - .min_by_key(|&(&s, &e)| e.saturating_sub(s)) - .map(|(&s, _)| s) + && let (Some(&first), Some(&last)) = (holes.keys().next(), holes.keys().next_back()) { - holes.remove(&narrowest); + let farthest = if start.abs_diff(first) >= start.abs_diff(last) { + first + } else { + last + }; + holes.remove(&farthest); } } @@ -1654,6 +1657,33 @@ pub async fn spawn_decoder( mod tests { use super::*; + #[test] + fn pts_hole_tracking_keeps_the_newest_holes_when_full() { + let mut holes = std::collections::BTreeMap::new(); + let mut frame = 0u32; + for i in 0..1000u32 { + let width = if i % 4 == 0 { 3 } else { 2 }; + record_pts_hole(&mut holes, frame, frame + width); + assert_eq!(holes.get(&frame), Some(&(frame + width))); + frame += width; + } + assert_eq!(holes.len(), 64); + assert!(holes.keys().all(|&start| start + 200 >= frame)); + } + + #[test] + fn pts_hole_tracking_drops_the_farthest_hole_after_a_backward_seek() { + let mut holes = std::collections::BTreeMap::new(); + for i in 0..64u32 { + record_pts_hole(&mut holes, 1000 + i * 2, 1000 + i * 2 + 2); + } + record_pts_hole(&mut holes, 10, 40); + assert_eq!(holes.len(), 64); + assert_eq!(holes.get(&10), Some(&40)); + assert!(holes.contains_key(&1000)); + assert!(!holes.contains_key(&(1000 + 63 * 2))); + } + #[test] fn decoded_frame_storage_identity_matches_clones() { let frame = DecodedFrame::new_nv12(vec![0; 6], 2, 2, 2, 2); From 8cafa9bfbe425cbc5cac7912fd235dee626eafd9 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Thu, 15 Oct 2026 04:54:43 +0900 Subject: [PATCH 02/10] fix(rendering): hold IOSurface sources until the GPU has read them --- crates/rendering/src/yuv_converter.rs | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/crates/rendering/src/yuv_converter.rs b/crates/rendering/src/yuv_converter.rs index 4bd420ddb2a..6d8210dc7fb 100644 --- a/crates/rendering/src/yuv_converter.rs +++ b/crates/rendering/src/yuv_converter.rs @@ -410,6 +410,18 @@ impl YuvConverterPipelines { } } +#[cfg(target_os = "macos")] +struct RetainedImageBuf { + _image_buf: cidre::arc::R, +} + +// SAFETY: the buffer is only retained and released, never read, and +// CoreVideo reference counting is thread-safe. +#[cfg(target_os = "macos")] +unsafe impl Send for RetainedImageBuf {} +#[cfg(target_os = "macos")] +unsafe impl Sync for RetainedImageBuf {} + pub struct YuvToRgbaConverter { pipelines: Arc, y_texture: wgpu::Texture, @@ -429,6 +441,8 @@ pub struct YuvToRgbaConverter { bind_group_cache: BindGroupCache, #[cfg(target_os = "macos")] iosurface_cache: Option, + #[cfg(target_os = "macos")] + iosurface_sources: [Option; 2], #[cfg(target_os = "windows")] d3d11_staging_texture: Option, #[cfg(target_os = "windows")] @@ -489,6 +503,8 @@ impl YuvToRgbaConverter { bind_group_cache: BindGroupCache::new(), #[cfg(target_os = "macos")] iosurface_cache: IOSurfaceTextureCache::new(), + #[cfg(target_os = "macos")] + iosurface_sources: [None, None], #[cfg(target_os = "windows")] d3d11_staging_texture: None, #[cfg(target_os = "windows")] @@ -1099,6 +1115,14 @@ impl YuvToRgbaConverter { compute_pass.dispatch_workgroups(width.div_ceil(8), height.div_ceil(8), 1); } + // The Metal textures alias the IOSurface without retaining the pixel + // buffer, and the decoder recycles released buffers for new pictures. + // Hold the source until this output slot is reused, by which point + // the caller has waited for the GPU work that sampled it. + self.iosurface_sources[self.current_output] = Some(RetainedImageBuf { + _image_buf: image_buf.retained(), + }); + Ok(self.current_output_view()) } From 905e0933def0f696ac71f2998639a4a1e4719491 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:11:30 +0900 Subject: [PATCH 03/10] fix(rendering): keep IOSurface sources through pipelined transition frames --- crates/rendering/src/yuv_converter.rs | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/crates/rendering/src/yuv_converter.rs b/crates/rendering/src/yuv_converter.rs index 6d8210dc7fb..6791c1cc513 100644 --- a/crates/rendering/src/yuv_converter.rs +++ b/crates/rendering/src/yuv_converter.rs @@ -410,6 +410,13 @@ impl YuvConverterPipelines { } } +/// Conversions whose GPU work can still be pending when a new one starts. The +/// renderer waits for the previous frame only after submitting the next one, +/// and a transition frame converts twice (outgoing, then incoming), so up to +/// three earlier sources may still be sampled. +#[cfg(target_os = "macos")] +const RETAINED_IOSURFACE_SOURCES: usize = 4; + #[cfg(target_os = "macos")] struct RetainedImageBuf { _image_buf: cidre::arc::R, @@ -442,7 +449,9 @@ pub struct YuvToRgbaConverter { #[cfg(target_os = "macos")] iosurface_cache: Option, #[cfg(target_os = "macos")] - iosurface_sources: [Option; 2], + iosurface_sources: [Option; RETAINED_IOSURFACE_SOURCES], + #[cfg(target_os = "macos")] + next_iosurface_source: usize, #[cfg(target_os = "windows")] d3d11_staging_texture: Option, #[cfg(target_os = "windows")] @@ -504,7 +513,9 @@ impl YuvToRgbaConverter { #[cfg(target_os = "macos")] iosurface_cache: IOSurfaceTextureCache::new(), #[cfg(target_os = "macos")] - iosurface_sources: [None, None], + iosurface_sources: Default::default(), + #[cfg(target_os = "macos")] + next_iosurface_source: 0, #[cfg(target_os = "windows")] d3d11_staging_texture: None, #[cfg(target_os = "windows")] @@ -1117,11 +1128,12 @@ impl YuvToRgbaConverter { // The Metal textures alias the IOSurface without retaining the pixel // buffer, and the decoder recycles released buffers for new pictures. - // Hold the source until this output slot is reused, by which point + // Hold the source until enough later conversions have started that // the caller has waited for the GPU work that sampled it. - self.iosurface_sources[self.current_output] = Some(RetainedImageBuf { + self.iosurface_sources[self.next_iosurface_source] = Some(RetainedImageBuf { _image_buf: image_buf.retained(), }); + self.next_iosurface_source = (self.next_iosurface_source + 1) % RETAINED_IOSURFACE_SOURCES; Ok(self.current_output_view()) } From 06d9541b8fb15c005fcca4640f95997cf8299486 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Thu, 15 Oct 2026 13:33:10 +0900 Subject: [PATCH 04/10] fix(rendering): keep every pts hole of a realistic recording in the decoder hole map --- crates/rendering/src/decoder/mod.rs | 43 +++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 11 deletions(-) diff --git a/crates/rendering/src/decoder/mod.rs b/crates/rendering/src/decoder/mod.rs index f3469668307..75bd57d1a45 100644 --- a/crates/rendering/src/decoder/mod.rs +++ b/crates/rendering/src/decoder/mod.rs @@ -671,17 +671,20 @@ pub fn pts_to_frame(pts: i64, time_base: Rational, fps: u32) -> u32 { pub const FRAME_CACHE_SIZE: usize = 90; const DEFAULT_MAX_FALLBACK_DISTANCE: u32 = 90; +/// Bounds the pts-hole map. Holes are facts about the file, and a pooled +/// reader parked past a hole never re-vends it, so the map keeps every hole a +/// realistic recording has and only evicts on pathological inputs. +const MAX_TRACKED_HOLES: usize = 1 << 14; + /// Records a pts hole discovered from a decode-order vend jump (frames vend -/// in pts order, so a jump means no samples exist in between). The map stays -/// bounded by dropping the hole farthest from the one just recorded: requests -/// trail the reader, so the newest holes are the ones about to be asked for, -/// and a dropped hole is rediscovered whenever the reader passes it again. +/// in pts order, so a jump means no samples exist in between). When the map +/// is full it drops the hole farthest from the one just recorded: requests +/// trail the reader, so the newest holes are the ones about to be asked for. pub(super) fn record_pts_hole( holes: &mut std::collections::BTreeMap, start: u32, end: u32, ) { - const MAX_TRACKED_HOLES: usize = 64; holes.insert(start, end); if holes.len() > MAX_TRACKED_HOLES && let (Some(&first), Some(&last)) = (holes.keys().next(), holes.keys().next_back()) @@ -1661,27 +1664,45 @@ mod tests { fn pts_hole_tracking_keeps_the_newest_holes_when_full() { let mut holes = std::collections::BTreeMap::new(); let mut frame = 0u32; - for i in 0..1000u32 { + for i in 0..(MAX_TRACKED_HOLES as u32 + 1000) { let width = if i % 4 == 0 { 3 } else { 2 }; record_pts_hole(&mut holes, frame, frame + width); assert_eq!(holes.get(&frame), Some(&(frame + width))); frame += width; } - assert_eq!(holes.len(), 64); - assert!(holes.keys().all(|&start| start + 200 >= frame)); + assert_eq!(holes.len(), MAX_TRACKED_HOLES); + assert!( + holes + .keys() + .all(|&start| start as usize + 3 * MAX_TRACKED_HOLES >= frame as usize) + ); } #[test] fn pts_hole_tracking_drops_the_farthest_hole_after_a_backward_seek() { let mut holes = std::collections::BTreeMap::new(); - for i in 0..64u32 { + let last = MAX_TRACKED_HOLES as u32 - 1; + for i in 0..=last { record_pts_hole(&mut holes, 1000 + i * 2, 1000 + i * 2 + 2); } record_pts_hole(&mut holes, 10, 40); - assert_eq!(holes.len(), 64); + assert_eq!(holes.len(), MAX_TRACKED_HOLES); assert_eq!(holes.get(&10), Some(&40)); assert!(holes.contains_key(&1000)); - assert!(!holes.contains_key(&(1000 + 63 * 2))); + assert!(!holes.contains_key(&(1000 + last * 2))); + } + + #[test] + fn pts_hole_tracking_keeps_an_old_static_hold_while_later_holes_arrive() { + let mut holes = std::collections::BTreeMap::new(); + record_pts_hole(&mut holes, 1797, 2402); + let mut frame = 2402u32; + for i in 0..2000u32 { + let width = 2 + i % 7; + record_pts_hole(&mut holes, frame, frame + width); + frame += width; + } + assert_eq!(holes.get(&1797), Some(&2402)); } #[test] From 780aa0013da4ffecfe9b1580be6e1d99b8a19601 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Thu, 15 Oct 2026 13:33:10 +0900 Subject: [PATCH 05/10] fix(rendering): reset a pooled reader that has already passed the requested frame --- crates/rendering/src/decoder/avassetreader.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/crates/rendering/src/decoder/avassetreader.rs b/crates/rendering/src/decoder/avassetreader.rs index af79c9560e2..65af662f284 100644 --- a/crates/rendering/src/decoder/avassetreader.rs +++ b/crates/rendering/src/decoder/avassetreader.rs @@ -537,7 +537,12 @@ impl AVAssetReaderDecoder { }) } - fn select_best_decoder(&mut self, requested_time: f32, is_scrubbing: bool) -> (usize, bool) { + fn select_best_decoder( + &mut self, + requested_frame: u32, + requested_time: f32, + is_scrubbing: bool, + ) -> (usize, bool) { let decoder_count = self.decoders.len(); let (best_id, _distance, needs_reset) = if is_scrubbing { self.pool_manager @@ -552,6 +557,16 @@ impl AVAssetReaderDecoder { }; let decoder_idx = best_id.min(decoder_count.saturating_sub(1)); + // Pool positions are capped to the newest request, so a reader that + // decoded ahead can look usable for a request it has already passed. + // It can never vend that frame again, and reading on would answer + // with a stale or later frame, so it has to seek back first. + let needs_reset = needs_reset + || self + .decoders + .get(decoder_idx) + .and_then(|decoder| decoder.prev_vended) + .is_some_and(|prev| prev >= requested_frame); let max_decoder_count = self .pool_manager @@ -890,7 +905,7 @@ impl AVAssetReaderDecoder { } let this = this.as_mut().expect("Video reader initialized above"); - let (decoder_idx, was_reset) = this.select_best_decoder(requested_time, is_scrubbing); + let (decoder_idx, was_reset) = this.select_best_decoder(requested_frame, requested_time, is_scrubbing); let cache_min = if was_reset { min_requested_frame.saturating_sub(FRAME_CACHE_SIZE as u32 * 2) From 930c036e057168f3a61dd3808abf29604e090b51 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Thu, 15 Oct 2026 13:51:01 +0900 Subject: [PATCH 06/10] fix(rendering): retry from a fresh seek when the frame before a pts gap is no longer held --- crates/rendering/src/decoder/avassetreader.rs | 39 ++++++++++++++++++- 1 file changed, 37 insertions(+), 2 deletions(-) diff --git a/crates/rendering/src/decoder/avassetreader.rs b/crates/rendering/src/decoder/avassetreader.rs index 65af662f284..357344ba1b1 100644 --- a/crates/rendering/src/decoder/avassetreader.rs +++ b/crates/rendering/src/decoder/avassetreader.rs @@ -542,6 +542,7 @@ impl AVAssetReaderDecoder { requested_frame: u32, requested_time: f32, is_scrubbing: bool, + must_reset: bool, ) -> (usize, bool) { let decoder_count = self.decoders.len(); let (best_id, _distance, needs_reset) = if is_scrubbing { @@ -562,6 +563,7 @@ impl AVAssetReaderDecoder { // It can never vend that frame again, and reading on would answer // with a stale or later frame, so it has to seek back first. let needs_reset = needs_reset + || must_reset || self .decoders .get(decoder_idx) @@ -709,6 +711,7 @@ impl AVAssetReaderDecoder { frame: u32, max_fallback_distance: u32, sender: oneshot::Sender, + retried: bool, } let mut deferred_requests = VecDeque::::new(); @@ -766,6 +769,7 @@ impl AVAssetReaderDecoder { frame, max_fallback_distance, sender, + retried: false, }); } } @@ -784,6 +788,7 @@ impl AVAssetReaderDecoder { frame, max_fallback_distance, sender, + retried: false, }); } } @@ -905,7 +910,12 @@ impl AVAssetReaderDecoder { } let this = this.as_mut().expect("Video reader initialized above"); - let (decoder_idx, was_reset) = this.select_best_decoder(requested_frame, requested_time, is_scrubbing); + let (decoder_idx, was_reset) = this.select_best_decoder( + requested_frame, + requested_time, + is_scrubbing, + pending_requests.iter().any(|request| request.retried), + ); let cache_min = if was_reset { min_requested_frame.saturating_sub(FRAME_CACHE_SIZE as u32 * 2) @@ -938,6 +948,7 @@ impl AVAssetReaderDecoder { // reader re-vends from the keyframe at-or-before the request, and // that frame is the true VFR hold for requests inside a pts hole. let mut hold_candidate: Option<(u32, R)> = None; + let mut retry_requests = Vec::new(); { let decoder = &mut this.decoders[decoder_idx]; @@ -965,7 +976,8 @@ impl AVAssetReaderDecoder { let current_frame = pts_to_frame(frame.pts().value, Rational::new(1, frame.pts().scale), fps); - if let Some(prev) = decoder.prev_vended + let vended_before = decoder.prev_vended; + if let Some(prev) = vended_before && current_frame > prev + 1 { record_pts_hole(&mut pts_holes, prev, current_frame); @@ -1080,6 +1092,25 @@ impl AVAssetReaderDecoder { .flatten() .max_by_key(|d| d.number); + // The reader vends every sample in pts order, so the + // frame it vended just before this one is the newest + // at or before the request. When that frame is no + // longer held anywhere, any other answer is stale or + // from the future: retry once from a fresh seek, which + // re-vends and keeps it. + let expected = + vended_before.filter(|&number| number <= req.frame); + if !req.retried + && expected.is_some() + && best_before.as_ref().map(|d| d.number) != expected + { + retry_requests.push(PendingRequest { + retried: true, + ..req + }); + continue; + } + if let Some(data) = best_before { gap_hold = Some(data.clone()); *last_sent_frame.borrow_mut() = Some(data.clone()); @@ -1120,6 +1151,10 @@ impl AVAssetReaderDecoder { decoder.is_done = true; } + for request in retry_requests.into_iter().rev() { + deferred_requests.push_front(request); + } + let last_frame_in_cache = cache.keys().max().copied().unwrap_or(0); if frames_iterated > 0 { this.decoders[decoder_idx] From a048147de613589324199a367d0d9b8217d0bf7a Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Thu, 15 Oct 2026 13:51:01 +0900 Subject: [PATCH 07/10] fix(rendering): keep each pooled reader's last vended frame cached --- crates/rendering/src/decoder/avassetreader.rs | 42 +++++++++++++++---- 1 file changed, 35 insertions(+), 7 deletions(-) diff --git a/crates/rendering/src/decoder/avassetreader.rs b/crates/rendering/src/decoder/avassetreader.rs index 357344ba1b1..c8762858596 100644 --- a/crates/rendering/src/decoder/avassetreader.rs +++ b/crates/rendering/src/decoder/avassetreader.rs @@ -928,9 +928,23 @@ impl AVAssetReaderDecoder { max_requested_frame + FRAME_CACHE_SIZE as u32 }; + // The last frame each other pooled reader vended is the answer for a + // request that lands in the gap after it, and the reader can never + // vend it again; keep it cached while requests alternate between + // readers. + let pinned_frames: Vec = this + .decoders + .iter() + .enumerate() + .filter(|&(index, _)| index != decoder_idx) + .filter_map(|(_, decoder)| decoder.prev_vended) + .collect(); + if was_reset { *last_sent_frame.borrow_mut() = None; - cache.retain(|&f, _| f >= cache_min && f <= cache_max); + cache.retain(|f, _| { + (*f >= cache_min && *f <= cache_max) || pinned_frames.contains(f) + }); cache_bytes = cache .values() .map(CachedFrame::byte_len) @@ -1029,14 +1043,28 @@ impl AVAssetReaderDecoder { ) { if let Some(last_active) = &last_active_frame { - let frame_to_remove = if requested_frame > *last_active { - *cache.keys().next().unwrap() + let from_oldest = if requested_frame > *last_active { + true } else if requested_frame < *last_active { - *cache.keys().next_back().unwrap() + false + } else { + current_frame > *cache.keys().next_back().unwrap() + }; + let unpinned = |frame: &&u32| { + Some(**frame) != vended_before + && !pinned_frames.contains(*frame) + }; + let frame_to_remove = if from_oldest { + *cache + .keys() + .find(unpinned) + .unwrap_or_else(|| cache.keys().next().unwrap()) } else { - let min = *cache.keys().min().unwrap(); - let max = *cache.keys().max().unwrap(); - if current_frame > max { min } else { max } + *cache + .keys() + .rev() + .find(unpinned) + .unwrap_or_else(|| cache.keys().next_back().unwrap()) }; if let Some(removed) = cache.remove(&frame_to_remove) { cache_bytes = cache_bytes.saturating_sub(removed.byte_len()); From 89482c6db94752be9cee0666fcf688b2bb9662ca Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Thu, 15 Oct 2026 13:59:15 +0900 Subject: [PATCH 08/10] refactor(rendering): name the pooled reader reset, answer and eviction rules and test them --- crates/rendering/src/decoder/avassetreader.rs | 111 ++++++++++++++---- 1 file changed, 88 insertions(+), 23 deletions(-) diff --git a/crates/rendering/src/decoder/avassetreader.rs b/crates/rendering/src/decoder/avassetreader.rs index c8762858596..e904aa8de47 100644 --- a/crates/rendering/src/decoder/avassetreader.rs +++ b/crates/rendering/src/decoder/avassetreader.rs @@ -567,8 +567,7 @@ impl AVAssetReaderDecoder { || self .decoders .get(decoder_idx) - .and_then(|decoder| decoder.prev_vended) - .is_some_and(|prev| prev >= requested_frame); + .is_some_and(|decoder| reader_passed_request(decoder.prev_vended, requested_frame)); let max_decoder_count = self .pool_manager @@ -1050,23 +1049,11 @@ impl AVAssetReaderDecoder { } else { current_frame > *cache.keys().next_back().unwrap() }; - let unpinned = |frame: &&u32| { - Some(**frame) != vended_before - && !pinned_frames.contains(*frame) - }; - let frame_to_remove = if from_oldest { - *cache - .keys() - .find(unpinned) - .unwrap_or_else(|| cache.keys().next().unwrap()) - } else { - *cache - .keys() - .rev() - .find(unpinned) - .unwrap_or_else(|| cache.keys().next_back().unwrap()) - }; - if let Some(removed) = cache.remove(&frame_to_remove) { + let victim = cache_eviction_victim(&cache, from_oldest, |frame| { + Some(frame) == vended_before || pinned_frames.contains(&frame) + }); + if let Some(removed) = victim.and_then(|frame| cache.remove(&frame)) + { cache_bytes = cache_bytes.saturating_sub(removed.byte_len()); } } else { @@ -1126,11 +1113,12 @@ impl AVAssetReaderDecoder { // longer held anywhere, any other answer is stale or // from the future: retry once from a fresh seek, which // re-vends and keeps it. - let expected = - vended_before.filter(|&number| number <= req.frame); if !req.retried - && expected.is_some() - && best_before.as_ref().map(|d| d.number) != expected + && answer_misses_previous_vend( + vended_before, + req.frame, + best_before.as_ref().map(|d| d.number), + ) { retry_requests.push(PendingRequest { retried: true, @@ -1356,6 +1344,45 @@ impl AVAssetReaderDecoder { } } +fn reader_passed_request(prev_vended: Option, requested_frame: u32) -> bool { + prev_vended.is_some_and(|prev| prev >= requested_frame) +} + +// A request the reader steps over is answered by the frame it vended just +// before, the newest one at or before the request. Any other answer is stale. +fn answer_misses_previous_vend( + vended_before: Option, + requested_frame: u32, + answer: Option, +) -> bool { + vended_before + .filter(|&number| number <= requested_frame) + .is_some_and(|expected| answer != Some(expected)) +} + +// Drops from the end the cache is trimmed from, skipping pinned frames unless +// every frame is pinned. +fn cache_eviction_victim( + cache: &BTreeMap, + from_oldest: bool, + is_pinned: impl Fn(u32) -> bool, +) -> Option { + if from_oldest { + cache + .keys() + .copied() + .find(|&frame| !is_pinned(frame)) + .or_else(|| cache.keys().next().copied()) + } else { + cache + .keys() + .rev() + .copied() + .find(|&frame| !is_pinned(frame)) + .or_else(|| cache.keys().next_back().copied()) + } +} + fn should_grow_decoder_pool( is_scrubbing: bool, needs_reset: bool, @@ -1435,6 +1462,44 @@ mod tests { )); } + #[test] + fn a_reader_that_vended_the_request_cannot_serve_it_again() { + assert!(!reader_passed_request(None, 10)); + assert!(!reader_passed_request(Some(9), 10)); + assert!(reader_passed_request(Some(10), 10)); + assert!(reader_passed_request(Some(57), 54)); + } + + #[test] + fn only_the_frame_vended_before_a_gap_answers_requests_inside_it() { + assert!(!answer_misses_previous_vend(Some(1054), 1055, Some(1054))); + assert!(answer_misses_previous_vend(Some(1054), 1055, Some(1052))); + assert!(answer_misses_previous_vend(Some(751), 752, Some(421))); + assert!(answer_misses_previous_vend(Some(751), 752, None)); + assert!(!answer_misses_previous_vend(None, 752, Some(421))); + assert!(!answer_misses_previous_vend(Some(760), 752, Some(751))); + } + + #[test] + fn cache_eviction_skips_pinned_frames() { + let cache: BTreeMap = [415, 419, 421, 749, 751] + .into_iter() + .map(|f| (f, ())) + .collect(); + assert_eq!(cache_eviction_victim(&cache, true, |_| false), Some(415)); + assert_eq!(cache_eviction_victim(&cache, false, |_| false), Some(751)); + assert_eq!( + cache_eviction_victim(&cache, false, |f| f == 751), + Some(749) + ); + assert_eq!(cache_eviction_victim(&cache, true, |f| f < 420), Some(421)); + assert_eq!(cache_eviction_victim(&cache, true, |_| true), Some(415)); + assert_eq!( + cache_eviction_victim(&BTreeMap::::new(), true, |_| false), + None + ); + } + #[test] fn decoder_pool_grows_only_for_unserved_non_sequential_access() { assert!(should_grow_decoder_pool(true, true, 1, 5)); From 0bf8d138c3a51406fa050ced94486bc9519708dd Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Thu, 15 Oct 2026 14:03:28 +0900 Subject: [PATCH 09/10] fix(rendering): skip the passed-reader seek for requests before the stream's first frame --- crates/rendering/src/decoder/avassetreader.rs | 38 ++++++++++--------- 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/crates/rendering/src/decoder/avassetreader.rs b/crates/rendering/src/decoder/avassetreader.rs index e904aa8de47..ede98cdd48a 100644 --- a/crates/rendering/src/decoder/avassetreader.rs +++ b/crates/rendering/src/decoder/avassetreader.rs @@ -539,10 +539,9 @@ impl AVAssetReaderDecoder { fn select_best_decoder( &mut self, - requested_frame: u32, requested_time: f32, is_scrubbing: bool, - must_reset: bool, + must_reset: impl Fn(&DecoderInstance) -> bool, ) -> (usize, bool) { let decoder_count = self.decoders.len(); let (best_id, _distance, needs_reset) = if is_scrubbing { @@ -558,16 +557,7 @@ impl AVAssetReaderDecoder { }; let decoder_idx = best_id.min(decoder_count.saturating_sub(1)); - // Pool positions are capped to the newest request, so a reader that - // decoded ahead can look usable for a request it has already passed. - // It can never vend that frame again, and reading on would answer - // with a stale or later frame, so it has to seek back first. - let needs_reset = needs_reset - || must_reset - || self - .decoders - .get(decoder_idx) - .is_some_and(|decoder| reader_passed_request(decoder.prev_vended, requested_frame)); + let needs_reset = needs_reset || self.decoders.get(decoder_idx).is_some_and(must_reset); let max_decoder_count = self .pool_manager @@ -909,12 +899,24 @@ impl AVAssetReaderDecoder { } let this = this.as_mut().expect("Video reader initialized above"); - let (decoder_idx, was_reset) = this.select_best_decoder( - requested_frame, - requested_time, - is_scrubbing, - pending_requests.iter().any(|request| request.retried), - ); + // Pool positions are capped to the newest request, so a reader that + // decoded ahead can look usable for a request it has already passed. + // It can never vend that frame again, and reading on would answer + // with a stale or later frame, so it has to seek back first. Before + // the stream's first frame nothing better exists, so a seek is wasted. + let retrying = pending_requests.iter().any(|request| request.retried); + let before_stream_start = keyframe_index + .as_ref() + .and_then(|index| index.keyframes().first()) + .is_some_and(|&(_, secs)| { + f64::from(requested_frame) < (secs * f64::from(fps)).round() + }); + let (decoder_idx, was_reset) = + this.select_best_decoder(requested_time, is_scrubbing, |decoder| { + retrying + || (!before_stream_start + && reader_passed_request(decoder.prev_vended, requested_frame)) + }); let cache_min = if was_reset { min_requested_frame.saturating_sub(FRAME_CACHE_SIZE as u32 * 2) From 027858c69fa0864249afa97c302d3edf5416f662 Mon Sep 17 00:00:00 2001 From: Richie McIlroy <33632126+richiemcilroy@users.noreply.github.com> Date: Thu, 15 Oct 2026 14:35:16 +0900 Subject: [PATCH 10/10] fix(rendering): answer requests before a track's first frame with that frame from the cache --- crates/rendering/src/decoder/avassetreader.rs | 32 +++++++++++++++---- 1 file changed, 25 insertions(+), 7 deletions(-) diff --git a/crates/rendering/src/decoder/avassetreader.rs b/crates/rendering/src/decoder/avassetreader.rs index ede98cdd48a..97ec15b3813 100644 --- a/crates/rendering/src/decoder/avassetreader.rs +++ b/crates/rendering/src/decoder/avassetreader.rs @@ -666,7 +666,7 @@ impl AVAssetReaderDecoder { } }; let mut this = None::; - let mut keyframe_index = None; + let mut keyframe_index: Option> = None; let init_result = DecoderInitResult { width, @@ -860,6 +860,28 @@ impl AVAssetReaderDecoder { } pending_requests = still_unfulfilled; + // A track can start after the recording does. Before its first + // frame, that frame is the answer; serving it from the cache keeps + // the reader from stepping ahead on every such request. + let stream_start_frame = keyframe_index + .as_ref() + .and_then(|index| index.keyframes().first()) + .map(|&(_, secs)| (secs * f64::from(fps)).round() as u32); + if let Some(start) = stream_start_frame + && let Some(first) = cache.get(&start).map(|cached| cached.data().clone()) + { + let mut after_start = Vec::with_capacity(pending_requests.len()); + for req in pending_requests.drain(..) { + if req.frame < start { + *last_sent_frame.borrow_mut() = Some(first.clone()); + let _ = req.sender.send(first.to_decoded_frame()); + } else { + after_start.push(req); + } + } + pending_requests = after_start; + } + if pending_requests.is_empty() { continue; } @@ -905,12 +927,8 @@ impl AVAssetReaderDecoder { // with a stale or later frame, so it has to seek back first. Before // the stream's first frame nothing better exists, so a seek is wasted. let retrying = pending_requests.iter().any(|request| request.retried); - let before_stream_start = keyframe_index - .as_ref() - .and_then(|index| index.keyframes().first()) - .is_some_and(|&(_, secs)| { - f64::from(requested_frame) < (secs * f64::from(fps)).round() - }); + let before_stream_start = + stream_start_frame.is_some_and(|start| requested_frame < start); let (decoder_idx, was_reset) = this.select_best_decoder(requested_time, is_scrubbing, |decoder| { retrying