#210 review: bound MXF reads by the file, keep legacy projects opening, allow 16K

- mxf: read_pcm bounds a request by the sample frames the file actually stores (plus 16M
  samples of zero padding) instead of a fixed 16,777,216-sample cap, so whole-clip reads of
  long stereo MXF (transcription) work again; PCM/AES3 chunk buffers are clamped to the source
  length instead of a 256 MiB cap; decode_pcm's output is bounded by its input, so its
  fixed cap goes (the AES3 >8-channel refusal stays; a request is also bounded by the source
  length divided by the channel count). read_sample keeps only the source-length bound.
- mxf: index_entries no longer fails open() past 1,048,576 entries (the sparse BTreeMap
  already bounds memory by real entries); long recordings open.
- project/format: new Sequence/Track/CaptionTrack::check_bounds() hold only the hostile-value
  bounds (settings, marks, overflowing time ranges); project load and export ranges use it, so
  projects that break only structural rules (overlap, zero duration) still open and export.
  check() = check_bounds() + the structural rules, as before.
- project: frame-size cap raised to 268,435,456 pixels, 32768 per side (16K, 16384x8192).
- engine: checked_u32_p accepts integer-valued floats (1920.0); renderable_sequence returns
  an EngineError (new Session::try_render_program_at, used by automation render) instead of
  eprintln.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
This commit is contained in:
Brandon ThomasandClaude Opus 5.5 committed 2026-10-07 20:52:53 -04:00
1 parent 07d85b0f02
commit de70468ab6
14 files changed
+188 -74

No files matched your search

+1 -1
View File
@@ -403,7 +403,7 @@ impl FilmcraftMcp {
let (w, h) =
g.active_sequence().map(|q| (q.settings.width, q.settings.height)).ok_or_else(|| AutomationError::Other("no sequence".into()))?;
let scale = (max as f32 / w.max(h) as f32).min(1.0);
let img = g.render_program_at(scale, t).ok_or_else(|| AutomationError::Other("no sequence".into()))?;
let img = g.try_render_program_at(scale, t).map_err(|e| AutomationError::Other(e.to_string()))?;
png_rgba(img.w as u32, img.h as u32, img.over_black_rgba8(), max)
})
.await
+6 -1
View File
@@ -165,9 +165,14 @@ pub(crate) fn u64_p(p: &Value, k: &str) -> Option<u64> {
}
/// Typed dimensions/counts must not truncate, wrap or silently accept negative/fractional values.
/// Integer-valued floats (`1920.0`, as JSON from many clients) are integers.
pub(crate) fn checked_u32_p(p: &Value, key: &str, cmd: &str) -> Result<Option<u32>> {
let Some(value) = p.get(key).filter(|v| !v.is_null()) else { return Ok(None) };
value.as_u64().and_then(|v| u32::try_from(v).ok()).map(Some).ok_or_else(|| bad(cmd, format!("`{key}` must be an unsigned 32-bit integer")))
let exact = value
.as_u64()
.and_then(|v| u32::try_from(v).ok())
.or_else(|| value.as_f64().filter(|f| f.is_finite() && f.fract() == 0.0 && (0.0..=f64::from(u32::MAX)).contains(f)).map(|f| f as u32));
exact.map(Some).ok_or_else(|| bad(cmd, format!("`{key}` must be an unsigned 32-bit integer")))
}
/// Parse a time from params: `time` (ticks), `frame`, `seconds` or `timecode`, with `prefix`.
+1
View File
@@ -61,6 +61,7 @@ fn export_integer_parameters_cannot_wrap_or_overflow() {
}
}
assert!(s.execute("export.resolve", json!({"bitrateKbps":u32::MAX})).is_ok());
assert!(s.execute("export.resolve", json!({"bitrateKbps":8000.0, "keyframeDistance":48.0})).is_ok(), "integer-valued floats are integers");
}
fn probe(path: &str) -> Option<Value> {
+1 -1
View File
@@ -265,7 +265,7 @@ pub fn sequence_param(s: &Session, p: &Value, cmd: &str) -> Result<ItemId> {
/// without a `range` mean `custom`.
pub fn range_param(s: &Session, project: &Project, seq: ItemId, v: Option<&Value>, p: &Value, cmd: &str) -> Result<Option<TimeRange>> {
let q = project.sequence(seq).ok_or(EngineError::NoSequence)?;
q.check().map_err(|e| bad(cmd, e))?;
q.check_bounds().map_err(|e| bad(cmd, e))?;
let fd = q.settings.frame_rate.frame_duration();
let whole = TimeRange::from_bounds(Tick::ZERO, q.duration().max(fd));
let mode = match v {
+18 -14
View File
@@ -923,7 +923,7 @@ impl Session {
/// Render the active sequence at the playhead in its working colour space (HDR values kept;
/// for scopes and analysis).
pub fn render_program_working(&self, scale: f32) -> Option<filmcraft_render::Image> {
let seq = self.renderable_sequence(scale)?;
let seq = self.renderable_sequence(scale).ok()?;
let provider = self.media.provider(self.project.clone(), self.services.clone());
let opts = filmcraft_render::RenderOptions { scale, working_output: true, ..Default::default() };
Some(filmcraft_render::render_sequence(&self.project, seq, self.playhead(), opts, &provider))
@@ -935,29 +935,33 @@ impl Session {
}
/// Render the active sequence at `t` (snapped to its frame, as the playhead would be) without
/// moving the playhead (CPU reference path).
/// moving the playhead (CPU reference path). `None` when there is no sequence or the frame
/// cannot be rendered at `scale`; [`Session::try_render_program_at`] says why.
pub fn render_program_at(&self, scale: f32, t: Tick) -> Option<filmcraft_render::Image> {
self.try_render_program_at(scale, t).ok()
}
/// [`Session::render_program_at`], with the reason when nothing can be rendered.
pub fn try_render_program_at(&self, scale: f32, t: Tick) -> Result<filmcraft_render::Image> {
let seq = self.renderable_sequence(scale)?;
let t = self.sequence_rate().snap(t.max(Tick::ZERO));
let provider = self.media.provider(self.project.clone(), self.services.clone());
let opts = filmcraft_render::RenderOptions { scale, captions: true, ..Default::default() };
Some(filmcraft_render::render_sequence(&self.project, seq, t, opts, &provider))
Ok(filmcraft_render::render_sequence(&self.project, seq, t, opts, &provider))
}
fn renderable_sequence(&self, scale: f32) -> Option<ItemId> {
let id = self.state.active_sequence?;
let seq = self.project.sequence(id)?;
/// The active sequence, if a frame of it at `scale` is within the image size limits.
fn renderable_sequence(&self, scale: f32) -> Result<ItemId> {
let id = self.state.active_sequence.ok_or(EngineError::NoSequence)?;
let seq = self.project.sequence(id).ok_or(EngineError::NoSequence)?;
if !scale.is_finite() || scale <= 0.0 {
eprintln!("filmcraft: render scale must be finite and positive");
return None;
return Err(EngineError::Other(format!("render scale {scale} must be finite and positive")));
}
let (w, h) = filmcraft_render::output_size(seq, scale);
let valid = u32::try_from(w).ok().zip(u32::try_from(h).ok()).is_some_and(|(w, h)| filmcraft_project::validate_frame_size(w, h).is_ok());
if !valid {
eprintln!("filmcraft: requested render frame exceeds the image size limits");
return None;
}
Some(id)
let size = u32::try_from(w).ok().zip(u32::try_from(h).ok());
let Some((w, h)) = size else { return Err(EngineError::Other("the requested render frame exceeds the image size limits".into())) };
filmcraft_project::validate_frame_size(w, h).map_err(|e| EngineError::Other(format!("cannot render a {w}x{h} frame: {e}")))?;
Ok(id)
}
}
+7 -1
View File
@@ -8,6 +8,7 @@ fn invalid_preview_scales_are_rejected_without_allocating() {
assert!(s.render_program(scale).is_none());
assert!(s.render_program_working(scale).is_none());
}
assert!(s.try_render_program_at(f32::NAN, Tick::ZERO).unwrap_err().to_string().contains("finite"), "the reason is reported");
assert_eq!(s.render_program(0.5).unwrap().w, 80);
}
use serde_json::json;
@@ -121,7 +122,8 @@ fn sequence_parameters_are_bounded_and_failed_changes_are_atomic() {
json!({"sampleRate":u64::MAX}),
json!({"width":-1}),
json!({"width":1.5}),
json!({"width":16384,"height":16384}),
json!({"width":32768,"height":16384}),
json!({"width":1920.25}),
json!({"fps":1001}),
json!({"sampleRate":384001}),
] {
@@ -136,6 +138,10 @@ fn sequence_parameters_are_bounded_and_failed_changes_are_atomic() {
}
s.execute("file.newSequence", json!({"width":7680,"height":4320,"video":0,"audio":0})).unwrap();
assert_eq!(s.active_sequence().unwrap().settings.width, 7680, "standard 8K remains supported");
s.execute("file.newSequence", json!({"width":15360.0,"height":8640.0,"sampleRate":48000.0,"video":1.0,"audio":0})).unwrap();
assert_eq!(s.active_sequence().unwrap().settings.width, 15360, "16K, and integer-valued floats, are accepted");
s.execute("sequence.settings", json!({"width":16384,"height":8192})).unwrap();
assert_eq!(s.active_sequence().unwrap().settings.height, 8192, "a 16384x8192 panorama is accepted");
}
#[test]
+1 -1
View File
@@ -1217,7 +1217,7 @@ fn h264_factory(format: Format, w: u32, h: u32, rate: FrameRate, s: &ExportSetti
/// The range to export (settings → In/Out → whole sequence).
pub fn export_range(project: &Project, seq: ItemId, settings: &ExportSettings) -> Result<TimeRange> {
let q = project.sequence(seq).ok_or(ExportError::NoSequence)?;
q.check().map_err(ExportError::Unsupported)?;
q.check_bounds().map_err(ExportError::Unsupported)?;
if let Some(r) = settings.range {
validate_range(r)?;
return Ok(r);
+4 -1
View File
@@ -148,11 +148,14 @@ fn wav_and_aiff_audio_only() {
#[test]
fn invalid_export_allocations_are_rejected_before_rendering() {
let (project, sequence, _) = matte([1.0, 0.0, 0.0, 1.0], 64, 36, None);
for size in [(0, 36), (64, 0), (u32::MAX, u32::MAX), (16384, 16384)] {
for size in [(0, 36), (64, 0), (u32::MAX, u32::MAX), (32768, 16384), (40000, 16)] {
let settings = ExportSettings { frame_size: Some(size), ..Default::default() };
assert!(settings.validate().is_err(), "{size:?}");
assert!(pipeline::Pipeline::new(project.clone(), sequence, &settings, false).is_err());
}
for size in [(15360, 8640), (16384, 8192)] {
assert!(ExportSettings { frame_size: Some(size), ..Default::default() }.validate().is_ok(), "{size:?} is a real output size");
}
for scale in [0.0, -1.0, f32::NAN, f32::INFINITY, 1e30] {
let settings = ExportSettings { scale, ..Default::default() };
assert!(pipeline::Pipeline::new(project.clone(), sequence, &settings, false).is_err(), "scale {scale}");
+32 -2
View File
@@ -206,10 +206,12 @@ pub fn decode(bytes: &[u8]) -> Result<Loaded, FormatError> {
validate_loaded(env.loaded(found))
}
/// Refuse only values that would overflow or exhaust memory later (`Sequence::check_bounds`);
/// structural rules such as overlaps are left to the editor, so projects that open today still open.
fn validate_loaded(loaded: Loaded) -> Result<Loaded, FormatError> {
for item in loaded.project.sequences() {
if let filmcraft_project::ItemKind::Sequence(sequence) = &item.kind {
sequence.check().map_err(|e| FormatError::Corrupt(format!("sequence `{}`: {e}", item.name)))?;
sequence.check_bounds().map_err(|e| FormatError::Corrupt(format!("sequence `{}`: {e}", item.name)))?;
}
}
Ok(loaded)
@@ -326,13 +328,41 @@ mod tests {
fn damaged_sequence_settings_are_rejected_before_rendering() {
let mut p = Project::new("Invalid sequence");
let id = p.new_sequence("Bad", filmcraft_project::SequenceSettings::default(), 1, 1, None);
for (width, height, sample_rate) in [(0, 90, 48000), (160, 0, 48000), (160, 90, 0), (16384, 16384, 48000)] {
for (width, height, sample_rate) in [(0, 90, 48000), (160, 0, 48000), (160, 90, 0), (65536, 90, 48000), (32768, 16384, 48000)] {
let settings = &mut p.sequence_mut(id).unwrap().settings;
(settings.width, settings.height, settings.sample_rate) = (width, height, sample_rate);
assert!(matches!(decode(&encode(&p, false)), Err(FormatError::Corrupt(_))));
}
}
/// #210 review: load-time validation refuses only overflowing values. A project that breaks a
/// structural rule an older build may have written (overlapping or zero-length clips) still
/// opens, as it did before; and 16K sequences are not "too large".
#[test]
fn structural_rule_breaks_still_open() {
use filmcraft_project::{ItemKind, SequenceSettings, TrackKind};
use filmcraft_time::{FrameRate, Tick, TimeRange};
let mut p = Project::new("Legacy");
let rate = FrameRate::FPS_24;
let layer = p.add_item(
"Layer",
filmcraft_project::Label::Iris,
ItemKind::AdjustmentLayer { width: 1920, height: 1080, rate, duration: rate.tick_of(48) },
None,
);
let settings = SequenceSettings { width: 15360, height: 8640, ..Default::default() };
let seq = p.new_sequence("Sequence 01", settings, 1, 1, None);
let range = TimeRange::new(Tick::ZERO, rate.tick_of(48));
let a = p.make_track_item(layer, TrackKind::Video, Tick::ZERO, range, rate).unwrap();
let b = p.make_track_item(layer, TrackKind::Video, rate.tick_of(24), range, rate).unwrap();
let mut c = p.make_track_item(layer, TrackKind::Video, rate.tick_of(200), range, rate).unwrap();
c.duration = Tick::ZERO;
p.sequence_mut(seq).unwrap().video_tracks[0].items = vec![a, b, c];
assert!(p.sequence(seq).unwrap().check().is_err(), "the sequence breaks the editor's invariants");
let loaded = decode(&encode(&p, false)).unwrap_or_else(|e| panic!("a project that opened before is refused: {e}"));
assert_eq!(loaded.project.sequence(seq).unwrap().video_tracks[0].items.len(), 3);
}
#[test]
fn roundtrip_current() {
let p = Project::new("Round Trip");
+3 -3
View File
@@ -217,10 +217,8 @@ pub fn decode_pcm(v: &[u8], format: SoundFormat, channels: usize, bits: u32, fra
if bps == 0 || bps > 4 || frame_bytes < bps * ch {
return Err(crate::Error::Unsupported(format!("{bits}-bit PCM with {frame_bytes}-byte frames")));
}
// `frame_bytes >= bps * ch`, so the output is bounded by the bytes actually supplied.
let n = v.len() / frame_bytes;
if n.checked_mul(ch).is_none_or(|n| n > 16_777_216) {
return Err(crate::Error::Invalid("PCM decode exceeds the sample buffer limit".into()));
}
let mut out = vec![Vec::with_capacity(n); ch];
let scale = 1.0 / (1u64 << (8 * bps - 1)) as f32;
for i in 0..n {
@@ -239,6 +237,8 @@ pub fn decode_pcm(v: &[u8], format: SoundFormat, channels: usize, bits: u32, fra
Ok(out)
}
SoundFormat::Aes3Element => {
// SMPTE 331M: an AES3 element carries at most eight channels (and an output buffer per
// declared channel would multiply a hostile header's allocation).
if ch > 8 {
return Err(crate::Error::Invalid("an AES3 element has at most eight channels".into()));
}
+27 -22
View File
@@ -296,6 +296,8 @@ struct WalkResult {
/// Values larger than this are never read while opening (metadata and index values are small).
const MAX_META_VALUE: u64 = 64 << 20;
/// Samples (frames x channels) of zero padding `read_pcm` will produce past the stored essence.
const PCM_PAD_FRAMES_LIMIT: usize = 1 << 24;
/// Buffered reads of KLV headers.
struct Window<'a, S: ByteSource + ?Sized> {
@@ -749,15 +751,11 @@ pub fn open(src: &(impl ByteSource + ?Sized)) -> Result<MxfFile> {
}
/// Index entries per stored edit unit for an index SID (all segments when `sid` is 0 or unmatched).
fn index_entries(index: &[IndexSegment], sid: u32) -> Result<(std::collections::BTreeMap<usize, IndexEntry>, u32)> {
fn index_entries(index: &[IndexSegment], sid: u32) -> (std::collections::BTreeMap<usize, IndexEntry>, u32) {
let any = index.iter().any(|s| s.index_sid == sid);
let segs: Vec<&IndexSegment> = index.iter().filter(|s| !any || s.index_sid == sid).collect();
// ST 377-1 segments use absolute edit-unit positions, which need not start at zero.
// Keep positions sparse so even hostile gaps allocate only for real entries.
segs.iter()
.try_fold(0usize, |n, s| n.checked_add(s.entries.len()))
.filter(|&n| n <= 1_048_576)
.ok_or_else(|| Error::Invalid("index exceeds the stored-entry limit".into()))?;
// Keep positions sparse so even hostile gaps allocate only for real (already parsed) entries.
let mut out = std::collections::BTreeMap::new();
let mut eubc = 0;
for s in segs {
@@ -770,7 +768,7 @@ fn index_entries(index: &[IndexSegment], sid: u32) -> Result<(std::collections::
out.insert(p, *e);
}
}
Ok((out, eubc))
(out, eubc)
}
fn build_track(
@@ -872,7 +870,7 @@ fn build_pictures(
index: &[IndexSegment],
warnings: &mut Vec<String>,
) -> Result<()> {
let (entries, eubc) = index_entries(index, t.index_sid)?;
let (entries, eubc) = index_entries(index, t.index_sid);
let expected = t.duration.map(|d| d.saturating_add(t.first_edit_unit()).max(0) as usize);
let clip = elements.len() == 1 && expected.is_none_or(|n| n > 1) && (eubc > 0 || entries.len() > 1);
let mut samples: Vec<Sample> = Vec::new();
@@ -994,8 +992,8 @@ impl MxfFile {
pub fn read_sample(&self, src: &(impl ByteSource + ?Sized), track: usize, i: usize) -> Result<Vec<u8>> {
let t = self.tracks.get(track).ok_or_else(|| Error::Invalid(format!("no track {track}")))?;
let s = t.samples.get(i).ok_or_else(|| Error::Invalid(format!("no sample {i}")))?;
if u64::from(s.size) > src.len().saturating_sub(s.offset) || s.size > 268_435_456 {
return Err(Error::Invalid("picture sample exceeds the source or frame buffer limit".into()));
if u64::from(s.size) > src.len().saturating_sub(s.offset) {
return Err(Error::Invalid("picture sample extends past the end of the source".into()));
}
let mut v = vec![0u8; s.size as usize];
src.read_at(s.offset, &mut v)?;
@@ -1008,8 +1006,16 @@ impl MxfFile {
let t = self.tracks.get(track).ok_or_else(|| Error::Invalid(format!("no track {track}")))?;
let si = t.sound.as_ref().ok_or_else(|| Error::Invalid("not a sound track".into()))?;
let ch = si.channels.max(1) as usize;
if ch > 256 || frames.checked_mul(ch).is_none_or(|n| n > 16_777_216) {
return Err(Error::Invalid("sound request exceeds the channel or sample buffer limit".into()));
if ch > 256 {
return Err(Error::Invalid("sound track declares more than 256 channels".into()));
}
// Bound the output by what the file can actually hold (every stored sample frame takes at
// least one byte per channel), plus slack for zero padding past the end: header-declared
// sizes cannot force a giant allocation, while any legitimately long read still works.
let backed = t.stored_sample_frames().saturating_sub(start).min(src.len() / ch as u64);
let allowed = usize::try_from(backed).unwrap_or(usize::MAX).saturating_add(PCM_PAD_FRAMES_LIMIT / ch);
if frames > allowed {
return Err(Error::Invalid("sound request extends far past the stored essence".into()));
}
if t.sound_format == SoundFormat::Pcm && (si.bits == 0 || si.bits > 32) {
return Err(Error::Unsupported("invalid PCM bit depth or block alignment".into()));
@@ -1034,19 +1040,18 @@ impl MxfFile {
.checked_mul(fb as u64)
.and_then(|n| c.offset.checked_add(n))
.ok_or_else(|| Error::Invalid("PCM chunk offset overflows".into()))?;
let bytes = ((b - a) as usize)
.checked_mul(fb)
.filter(|&n| n <= 268_435_456)
.ok_or_else(|| Error::Invalid("PCM chunk exceeds the buffer limit".into()))?;
let bytes = ((b - a) as usize).checked_mul(fb).ok_or_else(|| Error::Invalid("PCM chunk size overflows".into()))?;
// Never allocate past the end of the source (a truncated or lying element).
let in_file = usize::try_from(src.len().saturating_sub(off)).unwrap_or(usize::MAX);
let bytes = bytes.min(in_file / fb * fb);
let mut v = vec![0u8; bytes];
src.read_at(off, &mut v)?;
decode_pcm(&v, SoundFormat::Pcm, ch, si.bits, fb)?
}
SoundFormat::Aes3Element => {
if c.size > 268_435_456 {
return Err(Error::Invalid("AES3 chunk exceeds the buffer limit".into()));
}
let mut v = vec![0u8; c.size as usize];
let size = c.size.min(src.len().saturating_sub(c.offset));
let size = usize::try_from(size).map_err(|_| Error::Invalid("AES3 chunk exceeds the address space".into()))?;
let mut v = vec![0u8; size];
src.read_at(c.offset, &mut v)?;
let all = decode_pcm(&v, SoundFormat::Aes3Element, ch, si.bits, 0)?;
let s0 = (a - c.first_sample) as usize;
@@ -1076,7 +1081,7 @@ mod index_bounds_tests {
let entry = IndexEntry { flags: 0x80, stream_offset: 1234, ..Default::default() };
for start_position in [5, 49_999_999, i64::MAX] {
let segment = IndexSegment { start_position, entries: vec![entry, entry], ..Default::default() };
let (entries, _) = index_entries(&[segment], 0).unwrap();
let (entries, _) = index_entries(&[segment], 0);
let start = usize::try_from(start_position).unwrap();
assert_eq!(entries.get(&start), Some(&entry));
assert_eq!(entries.len(), if start_position == i64::MAX { 1 } else { 2 });
@@ -1086,7 +1091,7 @@ mod index_bounds_tests {
#[test]
fn negative_preroll_positions_do_not_drop_later_valid_entries() {
let segment = IndexSegment { start_position: -1, entries: vec![IndexEntry::default(); 3], ..Default::default() };
let (entries, _) = index_entries(&[segment], 0).unwrap();
let (entries, _) = index_entries(&[segment], 0);
assert_eq!(entries.keys().copied().collect::<Vec<_>>(), vec![0, 1]);
}
}
+23
View File
@@ -316,3 +316,26 @@ fn truncated_and_mutated_files_never_panic() {
}
}
}
/// #210 review: a whole-clip read of long stereo PCM (more samples than the old fixed
/// 16 777 216-sample request cap, about 175 s at 48 kHz) must work; only reads far past the stored
/// essence are refused.
#[test]
fn long_pcm_reads_backed_by_the_file_are_not_capped() {
let frames = 200 * 48_000;
let samples: Vec<i32> = (0..frames * 2).map(|k| (k % 60_000) as i32 - 30_000).collect();
let opts = OpAtomPcm { sample_rate: 48_000, bits: 16, channels: 2, edit_rate: None, ids: PackageIds::from_seed("long", "Long"), timecode: None };
let f = write_opatom_pcm(&opts, &samples).unwrap();
let m = open(&f).unwrap();
assert_eq!(m.tracks[0].stored_sample_frames(), frames as u64);
// a little past the end is zero padding, as before
let pcm = m.read_pcm(&f, 0, 0, frames + 10).unwrap();
assert_eq!((pcm.len(), pcm[0].len()), (2, frames + 10));
for k in [0, 1, frames / 2, frames - 1] {
assert_eq!(pcm[0][k], samples[2 * k] as f32 / 32768.0, "frame {k}");
assert_eq!(pcm[1][k], samples[2 * k + 1] as f32 / 32768.0, "frame {k}");
}
assert_eq!(pcm[0][frames..], [0.0; 10]);
// a request that is mostly not backed by data is refused before allocating
assert!(m.read_pcm(&f, 0, 0, frames + (1 << 24)).is_err());
}
+10 -3
View File
@@ -208,10 +208,8 @@ impl CaptionTrack {
}
/// Invariant check: captions sorted, positive duration, no overlaps.
pub fn check(&self) -> Result<(), String> {
self.check_bounds()?;
for c in &self.captions {
if !crate::bounded_time_range(c.start, c.duration) {
return Err(format!("{}: caption {:?} is outside supported time bounds", self.name, c.id));
}
if c.duration.0 <= 0 {
return Err(format!("{}: caption {:?} has non-positive duration", self.name, c.id));
}
@@ -223,6 +221,15 @@ impl CaptionTrack {
}
Ok(())
}
/// Only the time bounds that keep caption arithmetic from overflowing (see `Track::check_bounds`).
pub fn check_bounds(&self) -> Result<(), String> {
for c in &self.captions {
if !crate::bounded_time_range(c.start, c.duration) {
return Err(format!("{}: caption {:?} is outside supported time bounds", self.name, c.id));
}
}
Ok(())
}
}
/// Strip `<…>` tags and decode the common HTML/WebVTT entities.
+54 -24
View File
@@ -787,16 +787,7 @@ impl Track {
/// Invariant check: items do not overlap.
pub fn check(&self) -> Result<(), String> {
// Validate before calling end(), including on a single-item track.
for item in &self.items {
if !bounded_time_range(item.start, item.duration) {
return Err(format!("{}: item {:?} is outside supported time bounds", self.name, item.id));
}
}
for transition in &self.transitions {
if !bounded_time_range(transition.start, transition.duration) {
return Err(format!("{}: transition {:?} is outside supported time bounds", self.name, transition.id));
}
}
self.check_bounds()?;
for w in self.items.windows(2) {
if w[0].end() > w[1].start {
return Err(format!("{}: items {:?} and {:?} overlap", self.name, w[0].id, w[1].id));
@@ -809,6 +800,21 @@ impl Track {
}
Ok(())
}
/// Only the value bounds that keep timeline arithmetic from overflowing (no structural rules
/// such as overlap or positive duration); what a loaded project must satisfy.
pub fn check_bounds(&self) -> Result<(), String> {
for item in &self.items {
if !bounded_time_range(item.start, item.duration) {
return Err(format!("{}: item {:?} is outside supported time bounds", self.name, item.id));
}
}
for transition in &self.transitions {
if !bounded_time_range(transition.start, transition.duration) {
return Err(format!("{}: transition {:?} is outside supported time bounds", self.name, transition.id));
}
}
Ok(())
}
}
#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
@@ -858,10 +864,15 @@ impl Default for SequenceSettings {
}
}
/// Bound working-frame allocations while retaining standard 8K and wide panoramic frames.
/// Largest frame side, in pixels.
pub const MAX_FRAME_SIDE: u32 = 32_768;
/// Largest frame area, in pixels (16384 x 16384: 16K and 16384 x 8192 panoramas fit).
pub const MAX_FRAME_PIXELS: u64 = 268_435_456;
/// Bound working-frame allocations while retaining 16K and wide panoramic frames.
pub fn validate_frame_size(width: u32, height: u32) -> Result<(), String> {
if width == 0 || height == 0 || width > 16_384 || height > 16_384 || u64::from(width) * u64::from(height) > 67_108_864 {
return Err("frame size must be positive, at most 16384 pixels per side and 67108864 pixels total".into());
if width == 0 || height == 0 || width > MAX_FRAME_SIDE || height > MAX_FRAME_SIDE || u64::from(width) * u64::from(height) > MAX_FRAME_PIXELS {
return Err(format!("frame size must be positive, at most {MAX_FRAME_SIDE} pixels per side and {MAX_FRAME_PIXELS} pixels total"));
}
Ok(())
}
@@ -1017,12 +1028,7 @@ impl Sequence {
v
}
pub fn check(&self) -> Result<(), String> {
self.settings.validate()?;
if self.mark_in.into_iter().chain(self.mark_out).any(|t| t < Tick::MIN || t > Tick::MAX)
|| self.work_area.is_some_and(|r| !bounded_time_range(r.start, r.duration))
{
return Err("sequence marks or work area are outside supported time bounds".into());
}
self.check_bounds()?;
for t in self.all_tracks() {
t.check()?;
}
@@ -1031,13 +1037,33 @@ impl Sequence {
}
Ok(())
}
/// The hostile-value bounds alone (settings, marks and every time range), without the
/// structural invariants of [`Sequence::check`]: a project that breaks only those (e.g. an
/// overlap written by an older build) still opens, but nothing in it can overflow.
pub fn check_bounds(&self) -> Result<(), String> {
self.settings.validate()?;
if self.mark_in.into_iter().chain(self.mark_out).any(|t| t < Tick::MIN || t > Tick::MAX)
|| self.work_area.is_some_and(|r| !bounded_time_range(r.start, r.duration))
{
return Err("sequence marks or work area are outside supported time bounds".into());
}
for t in self.all_tracks() {
t.check_bounds()?;
}
for t in &self.caption_tracks {
t.check_bounds()?;
}
Ok(())
}
}
/// Bound persisted timeline arithmetic before any start + duration operation. Negative timeline
/// positions remain supported; empty ranges are valid metadata (track items/captions also require
/// a positive duration in their own invariant checks).
/// Bound persisted timeline arithmetic before any start + duration operation: start, duration and
/// end all stay within `Tick::MIN..=Tick::MAX`, leaving headroom for later edit math. Negative
/// positions remain supported; the sign of the duration is a structural rule (track items and
/// captions require a positive one in their own invariant checks), not a bound.
pub(crate) fn bounded_time_range(start: Tick, duration: Tick) -> bool {
start >= Tick::MIN && start <= Tick::MAX && duration.0 >= 0 && start.0.checked_add(duration.0).is_some_and(|end| end <= Tick::MAX.0)
let within = |t: i64| (Tick::MIN.0..=Tick::MAX.0).contains(&t);
within(start.0) && within(duration.0) && start.0.checked_add(duration.0).is_some_and(within)
}
/// Project-level settings.
@@ -1685,7 +1711,11 @@ mod tests {
let settings = SequenceSettings { frame_rate, ..Default::default() };
assert!(settings.validate().is_err());
}
assert!(SequenceSettings { width: 7680, height: 4320, ..Default::default() }.validate().is_ok());
for (width, height) in [(7680, 4320), (15360, 8640), (16384, 8192), (16384, 16384), (32768, 8192)] {
assert!(SequenceSettings { width, height, ..Default::default() }.validate().is_ok(), "{width}x{height}");
}
assert!(validate_frame_size(32768, 16384).is_err());
assert!(validate_frame_size(65536, 1).is_err());
}
#[test]