diff --git a/crates/media/src/ogg/mod.rs b/crates/media/src/ogg/mod.rs index fefca4c9c..387c3fc71 100644 --- a/crates/media/src/ogg/mod.rs +++ b/crates/media/src/ogg/mod.rs @@ -778,8 +778,15 @@ pub struct OpusTiming { pub total: i64, } +/// Largest granule position [`OpusTiming::of`] trusts: 2^62 samples at 48 kHz is about 3 million years. +pub const MAX_OPUS_GRANULE: i64 = 1 << 62; + impl OpusTiming { /// Timing of the Opus stream `s` (`pre_skip` from its `OpusHead`). + /// + /// A granule position outside `0..=MAX_OPUS_GRANULE` (a negative value other than -1, or an + /// absurdly large one) is treated like a missing granule: the page's packets continue from the + /// previous position instead of jumping to a nonsense one. pub fn of(stream: &Stream, pre_skip: u32) -> OpusTiming { // Granule positions come straight from the file, so the arithmetic saturates: a hostile // value gives wrong positions, never an overflow. @@ -789,10 +796,11 @@ impl OpusTiming { let mut i = 0; let mut cursor: Option = None; let mut trimmed_end: Option = None; + let granule_of = |k: usize| stream.packets.get(k).and_then(|p| p.granule).filter(|g| (0..=MAX_OPUS_GRANULE).contains(g)); while i < n { // packets completing on the same page: i..j, the last carries the page granule let mut j = i; - while j < n && stream.packets[j].granule.is_none() { + while j < n && granule_of(j).is_none() { j += 1; } if j == n { @@ -806,7 +814,7 @@ impl OpusTiming { cursor = Some(c); break; } - let Some(g) = stream.packets[j].granule else { break }; + let Some(g) = granule_of(j) else { break }; let sum = durations[i..=j].iter().fold(0i64, |a, &d| a.saturating_add(i64::from(d))); let eos = stream.packets[j].eos; let end = match cursor.map(|c| c.saturating_add(sum)) { diff --git a/crates/media/src/ogg/tests.rs b/crates/media/src/ogg/tests.rs index 4842c504c..f827648b1 100644 --- a/crates/media/src/ogg/tests.rs +++ b/crates/media/src/ogg/tests.rs @@ -91,6 +91,11 @@ fn pkt(i: usize, len: usize) -> Vec { /// Header pages, then pages of 3 packets each; the last page's granule trims `trim` samples. fn opus_file(npk: usize, pre_skip: u16, trim: i64) -> Vec { + opus_file_with(npk, pre_skip, |done, last| done as i64 * 960 - if last { trim } else { 0 }) +} + +/// Like `opus_file`, with the granule of each data page chosen by `granule(packets_done, is_last_page)`. +fn opus_file_with(npk: usize, pre_skip: u16, granule: impl Fn(usize, bool) -> i64) -> Vec { let mut f = Vec::new(); let head = opus_head(pre_skip); let (l, b) = lace(&[&head], false); @@ -103,10 +108,7 @@ fn opus_file(npk: usize, pre_skip: u16, trim: i64) -> Vec { for (seq, chunk) in (2..).zip(pkts.chunks(3)) { done += chunk.len(); let last = done == npk; - let mut g = done as i64 * 960; - if last { - g -= trim; - } + let g = granule(done, last); let refs: Vec<&[u8]> = chunk.iter().map(Vec::as_slice).collect(); let (l, b) = lace(&refs, false); page(&mut f, if last { 0x04 } else { 0 }, g, 9, seq, &l, &b); @@ -727,3 +729,55 @@ fn opus_timing_includes_packets_after_the_last_known_granule() { assert_eq!(timing.starts, vec![-312, 648, 1608, 2568, 3528, 4488]); assert_eq!(timing.total, 6 * 960 - 312); } + +/// The timing must be defined, bounded and panic-free for any granule a file can carry. +fn assert_sane_timing(t: &OpusTiming, npk: usize) { + assert_eq!(t.starts.len(), npk); + // treated as unknown granules: the packets simply follow one another from the start + for (k, s) in t.starts.iter().enumerate() { + assert_eq!(*s, k as i64 * 960 - 312, "packet {k}"); + } + // as for a truncated file with no usable granule, the total stays 0: it never exceeds what the packets add up to + assert!((0..=npk as i64 * 960).contains(&t.total), "total {}", t.total); +} + +#[test] +fn opus_timing_ignores_a_granule_near_i64_max() { + // panicked with "attempt to add with overflow" in debug builds, wrapped in release + let f = opus_file_with(10, 312, |done, _| i64::MAX - 10 + done as i64 / 3); + let o = open(&f).unwrap(); + assert_sane_timing(&OpusTiming::of(&o.streams[0], 312), 10); +} + +#[test] +fn opus_timing_ignores_a_granule_of_i64_min() { + // panicked with "attempt to subtract with overflow" in debug builds + for g in [i64::MIN, i64::MIN + 1, -2] { + let f = opus_file_with(10, 312, |_, _| g); + let o = open(&f).unwrap(); + assert_sane_timing(&OpusTiming::of(&o.streams[0], 312), 10); + } +} + +#[test] +fn opus_timing_with_a_bad_granule_on_one_page_recovers_on_the_next() { + // pages of 3 packets: the second page's granule is absurd, the others are right + let f = opus_file_with(9, 0, |done, _| if done == 6 { i64::MAX } else { done as i64 * 960 }); + let o = open(&f).unwrap(); + let t = OpusTiming::of(&o.streams[0], 0); + let expect: Vec = (0..9).map(|k| k * 960).collect(); + assert_eq!(t.starts, expect); + assert_eq!(t.total, 9 * 960); +} + +#[test] +fn largest_trusted_granule_with_a_huge_pre_skip_stays_in_range() { + let f = opus_file_with(6, 0, |done, _| MAX_OPUS_GRANULE - 6 * 960 + done as i64 * 960); + let o = open(&f).unwrap(); + let t = OpusTiming::of(&o.streams[0], u32::MAX); + // shift = base (the first raw start, 2^62 - 5760) + u32::MAX; every start is relative to it + let expect: Vec = (0..6).map(|k| k * 960 - u32::MAX as i64).collect(); + assert_eq!(t.starts, expect); + // the end (2^62) lies before the shift, so nothing is playable + assert_eq!(t.total, 0); +}