mirror of
https://github.com/storytold/filmcraft.git
synced 2026-10-24 10:10:05 -06:00
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 <noreply@anthropic.com> --------- Co-authored-by: Brandon Thomas <brandon@storyteller.ai> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5.5
Brandon Thomas
parent
68cfb0b52d
commit
980c388325
@@ -123,6 +123,20 @@ pub trait VideoSamples: Sync {
|
||||
fn make_decoder(&self) -> crate::Result<Box<dyn VideoDecoder>>;
|
||||
}
|
||||
|
||||
/// 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;
|
||||
|
||||
@@ -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> {
|
||||
usize::try_from(t).ok().and_then(|t| self.v.order.get(t).copied())
|
||||
|
||||
@@ -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> {
|
||||
usize::try_from(t).ok().and_then(|t| self.src.vorder.get(t).copied())
|
||||
|
||||
Reference in New Issue
Block a user