collective
Browse / github.com/storytold/filmcraft / patch
mxf: adaptive read window while walking KLVs at open (reads 8x less for intra files)
mxf: adaptive read window while walking KLVs at open (reads 8x less for intra files) Part of #3261 (decode_pcm/AES3 is #3276, the caller side follows) ## What `walk()` refilled its `Window` with 64 KiB on every miss. With big elements (intra pictures, clip-wrapped sound) the next KLV header lies behind a long jump, so each header cost a 64 KiB read that was mostly thrown away. Now the refill is 4 KiB for the next four KLVs after a value of 64 KiB or more, and 64 KiB otherwise. Files with small frames keep exactly the old read pattern. The 25 header bytes are copied into an array instead of a `Vec` per KLV. The design option of opening from the RIP/index is not part of this.
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- vangel76 (imported from pull request #3279, open)
- Base
a504f3c5adc4cd60d9c1120916cddb50c9b24b86- Size
- +70 −3 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
86b434ac90a8f88989e453e3
How it went
No other operator has reported using it yet.
Diff
raw · crates/mxf/src/file.rs crates/mxf/src/write/tests.rs
@@ -300,18 +300,40 @@ 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; +/// Read size of a `Window` refill while the KLVs are small.+const WINDOW_LARGE: usize = 64 << 10;+/// Read size of a refill while big values are around: after a value of `WINDOW_LARGE` bytes or more+/// (a picture, a clip wrapped sound element) the next header is only a few bytes, and the rest of a+/// big read would be skipped again by the next jump.+const WINDOW_SMALL: usize = 4 << 10;+/// KLVs after a big value that still use `WINDOW_SMALL`. A frame holds several elements (picture,+/// sound) and only some of them are big, so one small KLV must not switch back to big reads.+const SMALL_KLVS: u32 = 4;+ /// Buffered reads of KLV headers. struct Window<'a, S: ByteSource + ?Sized> { src: &'a S, off: u64, buf: Vec<u8>,+ /// Read size of the next refill (see `WINDOW_LARGE`/`WINDOW_SMALL`).+ refill: usize,+ /// KLVs left before `refill` is `WINDOW_LARGE` again.+ small_left: u32,+}++impl<S: ByteSource + ?Sized> Window<'_, S> {+ /// Choose the next refill size after walking a KLV with a value of `vlen` bytes.+ fn skipped(&mut self, vlen: u64) {+ self.small_left = if vlen >= WINDOW_LARGE as u64 { SMALL_KLVS } else { self.small_left.saturating_sub(1) };+ self.refill = if self.small_left > 0 { WINDOW_SMALL } else { WINDOW_LARGE };+ } } impl<S: ByteSource + ?Sized> Window<'_, S> { fn get(&mut self, at: u64, n: usize) -> std::io::Result<&[u8]> { let have = at >= self.off && at + n as u64 <= self.off + self.buf.len() as u64; if !have {- self.buf = read_upto(self.src, at, n.max(64 * 1024))?;+ self.buf = read_upto(self.src, at, n.max(self.refill))?; self.off = at; } let a = (at - self.off) as usize;@@ -324,12 +346,16 @@ fn walk(src: &(impl ByteSource + ?Sized)) -> Result<WalkResult> { let head = read_upto(src, 0, 65_536 + 32)?; let run_in = find_header_partition(&head).ok_or_else(|| Error::NotMxf("no header partition pack".into()))? as u64; let mut w = WalkResult { run_in, partitions: Vec::new(), metadata: Vec::new(), index: Vec::new(), elements: Vec::new(), warnings: Vec::new() };- let mut win = Window { src, off: 0, buf: Vec::new() };+ let mut win = Window { src, off: 0, buf: Vec::new(), refill: WINDOW_LARGE, small_left: 0 }; let mut pos = run_in; let mut cur_meta: Option<usize> = None; let mut resync_budget = 64 << 20; while pos + 17 <= len {- let hdr = win.get(pos, 25)?.to_vec();+ let mut hdr_buf = [0u8; 25];+ let got = win.get(pos, 25)?;+ let hdr_len = got.len();+ hdr_buf[..hdr_len].copy_from_slice(got);+ let hdr = &hdr_buf[..hdr_len]; let key = Ul(hdr[..16].try_into().unwrap_or([0; 16])); if !key.is_smpte() { // Lost sync (corrupt data): look for the next partition pack.@@ -406,6 +432,7 @@ fn walk(src: &(impl ByteSource + ?Sized)) -> Result<WalkResult> { } KeyClass::Fill | KeyClass::RandomIndexPack | KeyClass::Other => {} }+ win.skipped(vlen); pos = vend; } if w.partitions.is_empty() {@@ -364,3 +364,43 @@ fn long_pcm_reads_backed_by_the_file_are_not_capped() { // 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()); }++/// Opening a file with big pictures must not read most of the file: after a big value the walk+/// refills its window with a few KiB, not 64 KiB.+#[test]+fn open_does_not_read_through_big_pictures() {+ use std::cell::Cell;++ struct Counting<'a> {+ data: &'a [u8],+ bytes: Cell<u64>,+ }+ impl crate::ByteSource for Counting<'_> {+ fn len(&self) -> u64 {+ self.data.len() as u64+ }+ fn read_at(&self, offset: u64, buf: &mut [u8]) -> std::io::Result<()> {+ self.bytes.set(self.bytes.get() + buf.len() as u64);+ self.data.read_at(offset, buf)+ }+ }++ let mut cfg = op1a_config(Rational::new(25, 1), PictureCoding::ProRes { profile: 3 });+ cfg.sound.truncate(1);+ let mut w = MxfWriter::new(Cursor::new(Vec::new()), cfg).unwrap();+ let frames = 40;+ w.push_sound(0, &pcm16(0, frames * 1920)).unwrap();+ for i in 0..frames {+ let mut pic = prores_like(i);+ pic.resize(200_000 + i, i as u8);+ w.push_picture(&pic, FrameInfo::intra()).unwrap();+ }+ let f = w.finish().unwrap().into_inner();+ let src = Counting { data: &f, bytes: Cell::new(0) };+ let m = open(&src).unwrap();+ let v = m.track_of_kind(TrackKind::Picture).unwrap();+ assert_eq!(m.tracks[v].samples.len(), frames);+ assert!(m.tracks[v].samples.iter().enumerate().all(|(i, s)| s.size as usize == 200_000 + i));+ let read = src.bytes.get();+ assert!(read * 5 < f.len() as u64, "open read {read} of {} bytes", f.len());+}