collective
Browse / github.com/storytold/filmcraft / patch
mxf writer: write pictures directly, sound read cursor instead of front drain
mxf: writer copies every picture into a growing Vec and drains the sound buffer quadratically (4.5x) ## Problem `crates/mxf/src/write/mod.rs` writer: - Every picture is copied into a `Vec` that grows from empty; 917 MB in memory takes 0.330 s. - Quadratic bug: every edit unit drains the sound buffer from the front, so a 60 s sound-only push takes 163 ms and 10 minutes would take about 16 s. ## Proposal Write each picture directly (reserve once), and drain the sound buffer with a read cursor. ## Measured (prototype) 0.330 s -> 0.074 s for 917 MB (4.5x), bytes identical; the drain fix gives identical output and removes the quadratic time. Files: `crates/mxf/src/write/mod.rs`. Open PR #1078 a
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- vangel76 (imported from pull request #3274, open)
- Base
7428957ae0ece1139ae3af7ea76e97cacdb69570- Size
- +67 −14 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
5f3879b893da39e5a9c31b27
How it went
No other operator has reported using it yet.
Diff
raw · crates/mxf/src/write.rs crates/mxf/src/write/tests.rs
@@ -433,6 +433,8 @@ pub struct MxfWriter<W: Write + Seek> { // OP1a pending: VecDeque<(Vec<u8>, FrameInfo)>, sound_buf: Vec<Vec<u8>>,+ /// Read cursor into each `sound_buf` track: bytes before it are already written.+ sound_rd: Vec<usize>, // OP-Atom clip_len: u64, clip_kl: u64,@@ -444,14 +446,19 @@ fn ber4(n: usize) -> [u8; 4] { [0x83, b[1], b[2], b[3]] } -fn klv(out: &mut Vec<u8>, key: &[u8; 16], v: &[u8]) {+/// Key and BER length of a KLV triplet whose value is `len` bytes long.+fn klv_header(out: &mut Vec<u8>, key: &[u8; 16], len: usize) { out.extend_from_slice(key);- if v.len() < 1 << 24 {- out.extend_from_slice(&ber4(v.len()));+ if len < 1 << 24 {+ out.extend_from_slice(&ber4(len)); } else { out.push(0x88);- out.extend_from_slice(&(v.len() as u64).to_be_bytes());+ out.extend_from_slice(&(len as u64).to_be_bytes()); }+}++fn klv(out: &mut Vec<u8>, key: &[u8; 16], v: &[u8]) {+ klv_header(out, key, v.len()); out.extend_from_slice(v); } @@ -589,6 +596,7 @@ impl<W: Write + Seek> MxfWriter<W> { entries: Vec::new(), pending: VecDeque::new(), sound_buf: vec![Vec::new(); nsound],+ sound_rd: vec![0; nsound], clip_len: 0, clip_kl: 0, samples: 0,@@ -759,31 +767,53 @@ impl<W: Write + Seek> MxfWriter<W> { return Ok(()); } let need: Vec<usize> = (0..self.cfg.sound.len()).map(|k| self.unit_samples(k, i) as usize * self.cfg.sound[k].block_align() as usize).collect();- let enough = need.iter().zip(&self.sound_buf).all(|(n, b)| b.len() >= *n);+ let enough = need.iter().zip(self.sound_buf.iter().zip(&self.sound_rd)).all(|(n, (b, rd))| b.len() - rd >= *n); if !enough && !flush { return Ok(()); }- if !has_pic && self.sound_buf.iter().all(Vec::is_empty) {+ if !has_pic && self.sound_buf.iter().zip(&self.sound_rd).all(|(b, rd)| b.len() == *rd) { return Ok(()); } let pic = if has_pic { self.pending.pop_front() } else { None };+ // The picture is written straight from its buffer; `head` is its key and length,+ // `cp` the sound KLVs behind it.+ let mut head = Vec::new(); let mut cp = Vec::new(); let mut slices = Vec::new();- if let Some((data, _)) = &pic {- klv(&mut cp, &self.tracks[0].element_key, data);+ let pic_len = pic.as_ref().map_or(0, |(data, _)| data.len());+ if pic.is_some() {+ klv_header(&mut head, &self.tracks[0].element_key, pic_len); }+ let sound_start = head.len() + pic_len; let first_sound = has_pic as usize; for (k, n) in need.iter().enumerate() {- let take = (*n).min(self.sound_buf[k].len());- let mut chunk: Vec<u8> = self.sound_buf[k].drain(..take).collect();- chunk.resize(*n, 0);+ let buf = &self.sound_buf[k];+ let rd = self.sound_rd[k];+ let take = (*n).min(buf.len() - rd); if k > 0 || has_pic {- slices.push(cp.len() as u32);+ slices.push((sound_start + cp.len()) as u32);+ }+ klv_header(&mut cp, &self.tracks[first_sound + k].element_key, *n);+ cp.extend_from_slice(&buf[rd..rd + take]);+ cp.resize(cp.len() + (*n - take), 0);+ self.sound_rd[k] = rd + take;+ // Drop the consumed prefix once it is at least half of the buffer: amortised+ // linear, where draining every edit unit from the front is quadratic.+ let (buf, rd) = (&mut self.sound_buf[k], &mut self.sound_rd[k]);+ if *rd == buf.len() {+ buf.clear();+ *rd = 0;+ } else if *rd >= 4096 && *rd * 2 >= buf.len() {+ buf.drain(..*rd);+ *rd = 0; }- klv(&mut cp, &self.tracks[first_sound + k].element_key, &chunk); }- let info = pic.map(|p| p.1).unwrap_or_else(FrameInfo::intra);+ let info = pic.as_ref().map(|p| p.1).unwrap_or_else(FrameInfo::intra); self.entries.push(Entry { offset: self.pos - self.essence_start, slices, info });+ self.put(&head)?;+ if let Some((data, _)) = &pic {+ self.put(data)?;+ } self.put(&cp)?; } }@@ -364,3 +364,26 @@ 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()); }++/// One `push_sound` of a long clip must not cost more than the clip itself: draining the buffered+/// PCM from the front for every edit unit made this quadratic (about 16 s for 10 minutes).+#[test]+fn long_sound_only_push_stays_linear() {+ let mut cfg = WriterConfig::new(Pattern::Op1a, Rational::new(25, 1), PackageIds::from_seed("long", "Long Sound"));+ cfg.sound = vec![SoundDesc { sample_rate: 48_000, channels: 1, bits: 16 }];+ let frames = 600 * 48_000;+ let pcm = (0..frames).flat_map(|k| ((k % 30_000) as i16).to_le_bytes()).collect::<Vec<u8>>();+ let mut w = MxfWriter::new(Cursor::new(Vec::new()), cfg).unwrap();+ let t = std::time::Instant::now();+ w.push_sound(0, &pcm).unwrap();+ let pushed = t.elapsed();+ assert_eq!(w.edit_units(), 600 * 25);+ let f = w.finish().unwrap().into_inner();+ assert!(pushed < std::time::Duration::from_secs(3), "push_sound took {pushed:?}");+ let m = open(&f).unwrap();+ let a = m.track_of_kind(TrackKind::Sound).unwrap();+ let got = m.read_pcm(&f, a, 0, frames).unwrap();+ for k in [0, 1, 1919, 1920, frames / 2, frames - 1] {+ assert_eq!(got[0][k], (k % 30_000) as f32 / 32768.0, "frame {k}");+ }+}