From 980c3883253fb92cede914e4bbae3fccc2581d3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabriel=20Lu=C3=ADs=20da=20Silva?= <79342117+mercuryVM@users.noreply.github.com> Date: Fri, 23 Oct 2026 11:52:55 -0300 Subject: [PATCH] codecs: walk open-GOP leading pictures back in a loop, not by recursion (#2920) * codecs: walk open-GOP leading pictures back in a loop, not by recursion MxfVideo::sync_before and MpegVideo::sync_before recursed one GOP back per leading picture, so a crafted index whose every GOP leads its key frame recursed once per GOP: tens of thousands of frames for a small file, a plausible stack overflow in dev builds. Both now use gop::open_gop_sync, a loop over the same steps. It stops when the index's sync sample lies past the picture instead of looping, and reads pts and keys with get(). Fixes #2803 * gop: keep decode_start's doc comment on decode_start Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Brandon Thomas Co-authored-by: Claude Opus 5.5 --- crates/codecs/src/gop.rs | 27 +++++++++++++++++++++++++++ crates/codecs/src/mpeg.rs | 9 +++++++-- crates/codecs/src/mxf.rs | 5 +++-- 3 files changed, 37 insertions(+), 4 deletions(-) diff --git a/crates/codecs/src/gop.rs b/crates/codecs/src/gop.rs index 522b46b43..bcfd4f7ec 100644 --- a/crates/codecs/src/gop.rs +++ b/crates/codecs/src/gop.rs @@ -123,6 +123,20 @@ pub trait VideoSamples: Sync { fn make_decoder(&self) -> crate::Result>; } +/// A container's sync sample for `i` when leading pictures need the GOP before: `sync` is the +/// index's own sync sample, `leads(i, key)` says `i` is shown before `key`. One step per GOP, so a +/// loop: a crafted index can chain tens of thousands of GOPs (#2803). A `sync` past `i` stops the +/// walk instead of looping on it. +pub(crate) fn open_gop_sync(mut i: usize, sync: impl Fn(usize) -> usize, leads: impl Fn(usize, usize) -> bool) -> usize { + loop { + let key = sync(i); + if key == 0 || key > i || !leads(i, key) { + return key; + } + i = key - 1; + } +} + /// The sync sample to start decoding at for sample `i`: its sync sample, or an earlier one when /// `i` is a leading picture of an open GOP (stored after its random-access picture, shown before /// it, like the RASL pictures after every HEVC CRA of iPhone video). Those are predicted from the @@ -866,6 +880,19 @@ impl GopCache { #[cfg(test)] mod tests { + #[test] + fn open_gop_sync_walks_a_long_chain_of_leading_pictures_without_recursing() { + // Every GOP of 3 starts at a key whose leading picture is shown before it (#2803). + let gops = 200_000usize; + let sync = |i: usize| i - i % 3; + let leads = |i: usize, key: usize| i == key + 2; + assert_eq!(super::open_gop_sync(gops * 3 - 1, sync, leads), 0); + // A picture that is not a leading one keeps its own key. + assert_eq!(super::open_gop_sync(7, sync, |_, _| false), 6); + // An index whose sync sample lies past `i` ends the walk instead of looping. + assert_eq!(super::open_gop_sync(5, |_| 9, |_, _| true), 9); + } + use super::*; use crate::video::DecodedFrame; use std::collections::VecDeque; diff --git a/crates/codecs/src/mpeg.rs b/crates/codecs/src/mpeg.rs index 3ead983fe..040ca9092 100644 --- a/crates/codecs/src/mpeg.rs +++ b/crates/codecs/src/mpeg.rs @@ -769,10 +769,15 @@ impl VideoSamples for MpegVideo<'_> { } fn sync_before(&self, i: usize) -> usize { let i = i.min(self.v.keys.len().saturating_sub(1)); - let key = (0..=i).rev().find(|&k| self.v.keys[k]).unwrap_or(0); + let keys = &self.v.keys; + let pts = &self.v.pts; // a leading picture of an open GOP (shown before its random-access picture) needs the // previous GOP's pictures as references - if key > 0 && self.v.pts[i] < self.v.pts[key] { self.sync_before(key - 1) } else { key } + crate::gop::open_gop_sync( + i, + |i| (0..=i).rev().find(|&k| keys.get(k) == Some(&true)).unwrap_or(0), + |i, key| matches!((pts.get(i), pts.get(key)), (Some(a), Some(b)) if a < b), + ) } fn sample_at(&self, t: i64) -> Option { usize::try_from(t).ok().and_then(|t| self.v.order.get(t).copied()) diff --git a/crates/codecs/src/mxf.rs b/crates/codecs/src/mxf.rs index 1b230a8dc..cf8900a10 100644 --- a/crates/codecs/src/mxf.rs +++ b/crates/codecs/src/mxf.rs @@ -408,10 +408,11 @@ impl VideoSamples for MxfVideo<'_> { self.src.vpts[i] } fn sync_before(&self, i: usize) -> usize { - let key = self.src.file.tracks[self.track].sync_before(i); // a leading picture of an open GOP (stored after its random-access picture, shown // before it) is predicted from the previous GOP too - if key > 0 && i < self.src.vpts.len() && self.src.vpts[i] < self.src.vpts[key] { self.sync_before(key - 1) } else { key } + let track = &self.src.file.tracks[self.track]; + let pts = &self.src.vpts; + crate::gop::open_gop_sync(i, |i| track.sync_before(i), |i, key| matches!((pts.get(i), pts.get(key)), (Some(a), Some(b)) if a < b)) } fn sample_at(&self, t: i64) -> Option { usize::try_from(t).ok().and_then(|t| self.src.vorder.get(t).copied())