test: create scratch folders exclusively (#1334)

crates/clap-host/src/scan.rs

@@ -150,7 +150,9 @@ mod tests {
     fn tmp(name: &str) -> PathBuf {
         let d = std::env::temp_dir().join(format!("soundcraft-clap-scan-{name}-{}", std::process::id()));
         let _ = std::fs::remove_dir_all(&d);
-        std::fs::create_dir_all(&d).unwrap();
+        // Created exclusively: a folder still there after the cleanup (someone else's) would
+        // let a link inside it redirect the test's writes.
+        std::fs::create_dir(&d).unwrap();
         d
     }
 

crates/engine/src/autosave.rs

@@ -212,6 +212,9 @@ impl Autosave {
         let audio_dir = self.dir.join("Audio Files");
         let mut s = session.clone();
         let mut copies = BTreeMap::new();
+        // Copies made in this write, by the audio they hold: sources sharing one decoded buffer
+        // (a session may name one file under many ids) share one file.
+        let mut written: BTreeMap<*const SourceAudio, String> = BTreeMap::new();
         for src in &mut s.sources {
             let file = if Path::new(&src.path).is_absolute() { Some(PathBuf::from(&src.path)) } else { base.as_ref().map(|b| b.join(&src.path)) };
             let audio = s.pool.get(src.id);
@@ -224,6 +227,7 @@ impl Autosave {
             let Some(audio) = audio else { continue };
             let name = match self.copies.remove(&src.id) {
                 Some((copy, name)) if copy.upgrade().is_some_and(|a| Arc::ptr_eq(&a, audio)) => name,
+                _ if written.contains_key(&Arc::as_ptr(audio)) => written.get(&Arc::as_ptr(audio)).cloned().unwrap_or_default(),
                 _ => {
                     let name = format!("{} {}.wav", sanitize_name(&src.name), self.next);
                     self.next += 1;
@@ -235,13 +239,16 @@ impl Autosave {
             };
             src.path = audio_dir.join(&name).to_string_lossy().into_owned();
             src.unsaved = true;
+            written.insert(Arc::as_ptr(audio), name.clone());
             copies.insert(src.id, (Arc::downgrade(audio), name));
         }
         let text = s.to_json().map_err(|err| EngineError::Io(err.to_string()))?;
         write_atomically(&self.dir.join(ORIGIN), origin.unwrap_or_default().as_bytes())?;
         write_atomically(&self.dir.join(SESSION), text.as_bytes())?;
         for (_, name) in std::mem::replace(&mut self.copies, copies).into_values() {
-            let _ = std::fs::remove_file(audio_dir.join(name));
+            if !self.copies.values().any(|(_, kept)| *kept == name) {
+                let _ = std::fs::remove_file(audio_dir.join(name));
+            }
         }
         if let Some(r) = recording {
             let plan = serde_json::to_string(r).map_err(|err| EngineError::Io(err.to_string()))?;
@@ -507,6 +514,25 @@ mod tests {
         let _ = std::fs::remove_dir_all(&dir);
     }
 
+    #[test]
+    fn sources_sharing_one_buffer_share_one_copy() {
+        let dir = folder("autosave-aliases");
+        let mut e = Engine::default();
+        record(&mut e, 0.5);
+        // Nine more source ids over the same decoded audio, as a session file may declare.
+        let s = e.session_mut();
+        let (src, audio) = (s.sources[0].clone(), s.pool.get(s.sources[0].id).cloned().unwrap());
+        for _ in 0..9 {
+            let id = SourceId(s.alloc());
+            s.sources.push(soundcraft_model::Source { id, ..src.clone() });
+            s.pool.insert(id, audio.clone());
+        }
+        let mut a = Autosave::create(&dir.join("Autosave"), "Song").unwrap();
+        a.write(e.session(), None, None).unwrap();
+        assert_eq!(std::fs::read_dir(a.dir.join("Audio Files")).unwrap().count(), 1, "one file for the ten ids");
+        let _ = std::fs::remove_dir_all(&dir);
+    }
+
     #[test]
     fn a_take_recorded_after_undo_gets_its_own_copy() {
         let dir = folder("autosave-undo");

crates/engine/src/cmd.rs

@@ -347,7 +347,10 @@ pub fn clip_id(p: &Value, cmd: &str) -> Result<ClipId> {
 
 pub fn clip_ids_param(e: &Engine, p: &Value) -> Vec<ClipId> {
     if let Some(arr) = p.get("clips").and_then(Value::as_array) {
-        return arr.iter().filter_map(Value::as_u64).map(ClipId).collect();
+        // Each clip once, in the order given: a repeated id would otherwise be processed (and,
+        // for a render, re-rendered into another source) once per repeat.
+        let mut seen = std::collections::HashSet::new();
+        return arr.iter().filter_map(Value::as_u64).map(ClipId).filter(|id| seen.insert(*id)).collect();
     }
     if let Some(n) = p.get("clip").and_then(Value::as_u64) {
         return vec![ClipId(n)];

crates/engine/src/score.rs

@@ -418,7 +418,9 @@ fn svg_budgeted(parts: &[Part], setup: &ScoreSetup, budget: usize) -> Result<Str
     const STAFF_GAP: f64 = 70.0;
     const SYSTEM_GAP: f64 = 40.0;
     let n_measures = parts.iter().map(|p| p.measures.len()).max().unwrap_or(0);
-    let per = setup.bars_per_system.max(1) as usize;
+    // The layout range the setup offers (1 to 16): a caller's own value can't multiply the
+    // barlines drawn per system beyond it.
+    let per = setup.bars_per_system.clamp(1, 16) as usize;
     let systems = n_measures.div_ceil(per).max(1);
     let label_w = if setup.show_track_names { 70.0 } else { 0.0 };
     let staff_x0 = MARGIN + label_w + 34.0;
@@ -782,6 +784,14 @@ mod tests {
         assert!(m.items.iter().any(|i| i.pitches.is_empty()), "the discarded suffix became rests");
     }
 
+    #[test]
+    fn bars_per_system_is_held_to_the_supported_range() {
+        let setup = ScoreSetup { title: "t".into(), composer: String::new(), bars_per_system: u32::MAX, show_track_names: false };
+        let part = Part { name: "p".into(), bass: false, measures: vec![Measure { numerator: 4, denominator: 4, items: Vec::new() }] };
+        let out = svg_budgeted(std::slice::from_ref(&part), &setup, 1 << 20).unwrap();
+        assert_eq!(out.matches("stroke-width=\"1\"").count(), 16, "sixteen barlines, not billions");
+    }
+
     #[test]
     fn oversized_title_is_refused_by_the_header_check() {
         // An oversized part name/title must be caught by the header check even with no measures:

crates/time/src/tempo.rs

@@ -213,18 +213,52 @@ impl TempoMap {
         }
     }
 
-    /// Remove the meters after `tick`, returning each with the bar it starts.
+    /// Remove the meters after `tick`, returning each with the bar it starts: the bar
+    /// [`Self::bar_beat_at_tick`] gives its tick, counted in one pass over the map rather than one
+    /// walk per meter, so a session with many meter changes edits in linear time.
     fn take_meters_after(&mut self, tick: i64) -> Vec<(i64, MeterEvent)> {
-        let later: Vec<_> = self.meters.iter().filter(|e| e.tick > tick).map(|e| (self.bar_beat_at_tick(e.tick).bar, *e)).collect();
+        let mut later = Vec::new();
+        let (mut bar, mut seg_tick) = (1i64, 0i64);
+        for (i, m) in self.meters.iter().enumerate() {
+            let tpb = m.ticks_per_bar();
+            if m.tick > tick {
+                later.push((bar.saturating_add(m.tick.saturating_sub(seg_tick) / tpb), *m));
+            }
+            if let Some(n) = self.meters.get(i + 1) {
+                bar = bar.saturating_add(ceil_div(n.tick.saturating_sub(seg_tick), tpb));
+                seg_tick = n.tick;
+            }
+        }
         self.meters.retain(|e| e.tick <= tick);
         later
     }
 
+    /// The bar and tick of the last meter's segment, where [`Self::bar_start_tick`]'s walk ends.
+    fn last_segment(&self) -> (i64, i64) {
+        let (mut cur_bar, mut cur_tick) = (1i64, 0i64);
+        for (i, m) in self.meters.iter().enumerate() {
+            if let Some(n) = self.meters.get(i + 1) {
+                cur_bar = cur_bar.saturating_add(ceil_div(n.tick.saturating_sub(cur_tick), m.ticks_per_bar()));
+                cur_tick = n.tick;
+            }
+        }
+        (cur_bar, cur_tick)
+    }
+
     /// Put meters back at the start of their bars under the current earlier meters, in bar order.
+    /// Each lands at or past the last meter, so the walk to its bar resumes from there; a bar
+    /// before it (meters off their bar lines) takes the full walk.
     fn restore_meters_at_bars(&mut self, later: Vec<(i64, MeterEvent)>) {
+        let (mut cur_bar, mut cur_tick) = self.last_segment();
         for (bar, ev) in later {
-            let tick = self.bar_start_tick(bar);
+            let tick = if bar >= cur_bar {
+                let tpb = self.meters.last().map_or(MeterEvent { tick: 0, numerator: 4, denominator: 4 }, |m| *m).ticks_per_bar();
+                cur_tick.saturating_add(bar.saturating_sub(cur_bar).saturating_mul(tpb))
+            } else {
+                self.bar_start_tick(bar)
+            };
             self.put_meter(MeterEvent { tick, ..ev });
+            (cur_bar, cur_tick) = if bar >= cur_bar { (bar, tick) } else { self.last_segment() };
         }
     }
 
@@ -441,6 +475,36 @@ mod tests {
         assert_eq!(m.bar_beat_at_tick(m.tick_at_bar_beat(bb)), bb);
     }
 
+    #[test]
+    fn editing_an_early_meter_keeps_later_bars_and_scales_linearly() {
+        // 200 000 meter changes, one per bar, built directly: a walk per meter would take
+        // minutes to edit here, and even to build through `set_meter`.
+        let mut m = TempoMap::new(120.0, 4, 4).unwrap();
+        let mut tick = 0;
+        for bar in 1..200_000i64 {
+            let numerator = 3 + (bar % 2) as u32;
+            tick += m.meters.last().map_or(0, MeterEvent::ticks_per_bar);
+            m.meters.push(MeterEvent { tick, numerator, denominator: 4 });
+        }
+        let sample = [1usize, 1_000, 199_998];
+        let bars = |m: &TempoMap| sample.map(|i| m.bar_beat_at_tick(m.meters[i].tick).bar);
+        assert_eq!(bars(&m), [2, 1_001, 199_999]);
+        m.set_meter(0, 7, 8).unwrap();
+        assert_eq!(bars(&m), [2, 1_001, 199_999], "later meters keep their bar numbers");
+        assert_eq!(m.meters.len(), 200_000);
+        assert!(m.meters.windows(2).all(|w| w[0].tick < w[1].tick));
+        assert!(m.remove_meter(m.meters[1].tick));
+        assert_eq!([1usize, 999, 199_997].map(|i| m.bar_beat_at_tick(m.meters[i].tick).bar), [3, 1_001, 199_999]);
+        // Meters off their bar lines (counted as bars 2 and 4) land where they always did: on
+        // bar 2 of the new meter, then two bars of their own later.
+        let mut off = TempoMap::new(120.0, 4, 4).unwrap();
+        off.meters.push(MeterEvent { tick: 100, numerator: 3, denominator: 4 });
+        off.meters.push(MeterEvent { tick: 5_000, numerator: 5, denominator: 4 });
+        off.set_meter(0, 2, 4).unwrap();
+        let placed: Vec<(i64, u32)> = off.meters.iter().map(|e| (e.tick, e.numerator)).collect();
+        assert_eq!(placed, vec![(0, 2), (2 * TICKS_PER_QUARTER, 3), (8 * TICKS_PER_QUARTER, 5)]);
+    }
+
     #[test]
     fn eighth_note_meters() {
         let m = TempoMap::new(120.0, 6, 8).unwrap();

crates/video/src/h264/dpb.rs

@@ -72,6 +72,22 @@ impl Dpb {
         self.entries.retain(|e| e.needed_for_output || e.mark != RefMark::Unused);
     }
 
+    /// Make room for a picture: output one (C.4.5.3), or, when none waits for output and
+    /// references alone fill the DPB (marking that keeps more than it holds), drop the reference
+    /// decoded longest ago. False when the DPB is empty.
+    fn make_room(&mut self, out: &mut Vec<Output>) -> bool {
+        if self.bump(out) {
+            return true;
+        }
+        let Some(i) = self.entries.iter().enumerate().filter(|(_, e)| e.mark != RefMark::Unused).min_by_key(|(_, e)| e.frame.id).map(|(i, _)| i)
+        else {
+            return false;
+        };
+        self.entries[i].mark = RefMark::Unused;
+        self.prune();
+        true
+    }
+
     /// Output the picture with the smallest POC (C.4.5.3 "bumping"). Returns false if none waits.
     fn bump(&mut self, out: &mut Vec<Output>) -> bool {
         let Some(i) = self.entries.iter().enumerate().filter(|(_, e)| e.needed_for_output).min_by_key(|(_, e)| e.poc).map(|(i, _)| i) else {
@@ -134,21 +150,29 @@ impl Dpb {
         poc_state: &mut dyn FnMut(u32),
     ) {
         let mut unused = (prev_ref_frame_num + 1) % max_frame_num;
+        // Only the last `max_num_ref_frames` missing frames can still be references when the gap
+        // ends: the sliding window drops the earlier ones first, so building their pictures would
+        // be work (up to 65 535 frame copies from one slice header) thrown straight away. They
+        // advance the POC state only.
+        let gap = (frame_num + max_frame_num - unused) % max_frame_num;
+        let skip = gap.saturating_sub(u32::try_from(max_num_ref_frames.max(1)).unwrap_or(u32::MAX));
         let mut guard = 0;
         while unused != frame_num && guard < max_frame_num {
-            self.sliding_window(unused, max_frame_num, max_num_ref_frames);
-            let (frame, meta) = make_frame(unused);
             poc_state(unused);
-            self.entries.push(DpbEntry {
-                poc: frame.poc,
-                frame,
-                frame_num: unused,
-                long_term_frame_idx: 0,
-                mark: RefMark::Short,
-                needed_for_output: false,
-                non_existing: true,
-                meta,
-            });
+            if guard >= skip {
+                self.sliding_window(unused, max_frame_num, max_num_ref_frames);
+                let (frame, meta) = make_frame(unused);
+                self.entries.push(DpbEntry {
+                    poc: frame.poc,
+                    frame,
+                    frame_num: unused,
+                    long_term_frame_idx: 0,
+                    mark: RefMark::Short,
+                    needed_for_output: false,
+                    non_existing: true,
+                    meta,
+                });
+            }
             unused = (unused + 1) % max_frame_num;
             guard += 1;
         }
@@ -277,6 +301,11 @@ impl Dpb {
                         }
                     }
                 }
+                3 | 6 if self.max_long_term_frame_idx.is_none_or(|max| m.long_term_frame_idx > max) => {
+                    // A long-term index outside what MMCO 4 (or the IDR) allowed (8.2.5.4.3): a
+                    // conforming stream never sends one, and honouring it would let distinct
+                    // indices retain references past the DPB's capacity.
+                }
                 3 => {
                     let pn = i64::from(curr_pic_num) - (i64::from(m.difference_of_pic_nums_minus1) + 1);
                     for e in &mut self.entries {
@@ -384,13 +413,13 @@ impl Dpb {
                     out.push(Output { frame, poc, meta });
                     return;
                 }
-                if !self.bump(out) {
+                if !self.make_room(out) {
                     break;
                 }
             }
         } else {
             while self.entries.len() >= self.capacity {
-                if !self.bump(out) {
+                if !self.make_room(out) {
                     break;
                 }
             }
@@ -429,6 +458,98 @@ mod tests {
         Mmco { op, difference_of_pic_nums_minus1: diff, long_term_pic_num: 0, long_term_frame_idx: 0, max_long_term_frame_idx_plus1: 0 }
     }
 
+    fn frame(id: u32) -> (FrameRef, Arc<OutputMeta>) {
+        let meta = OutputMeta {
+            pts: 0,
+            key: false,
+            crop: (0, 0, 0, 0),
+            full_range: false,
+            colour_primaries: 0,
+            transfer_characteristics: 0,
+            matrix_coefficients: 0,
+            sar: (1, 1),
+            draft: false,
+        };
+        (Arc::new(crate::h264::picture::Frame::new(id, 0, 1, 1)), Arc::new(meta))
+    }
+
+    /// A P slice header of a reference picture, with nothing else set.
+    fn reference_slice() -> SliceHeader {
+        SliceHeader {
+            nal_ref_idc: 1,
+            nal_unit_type: 1,
+            idr: false,
+            first_mb_in_slice: 0,
+            slice_type_raw: 0,
+            slice_type: SliceType::P,
+            pps_id: 0,
+            colour_plane_id: 0,
+            frame_num: 0,
+            field_pic: false,
+            bottom_field: false,
+            idr_pic_id: 0,
+            pic_order_cnt_lsb: 0,
+            delta_pic_order_cnt_bottom: 0,
+            delta_pic_order_cnt: [0; 2],
+            redundant_pic_cnt: 0,
+            direct_spatial_mv_pred: false,
+            num_ref_idx_active: [1, 0],
+            ref_pic_list_mod: [Vec::new(), Vec::new()],
+            pred_weight_table: None,
+            no_output_of_prior_pics: false,
+            long_term_reference: false,
+            adaptive_ref_pic_marking: false,
+            mmcos: Vec::new(),
+            cabac_init_idc: 0,
+            slice_qp_delta: 0,
+            sp_for_switch: false,
+            slice_qs_delta: 0,
+            disable_deblocking_filter_idc: 0,
+            slice_alpha_c0_offset_div2: 0,
+            slice_beta_offset_div2: 0,
+            slice_group_change_cycle: 0,
+            header_bits: 0,
+        }
+    }
+
+    #[test]
+    fn a_frame_num_gap_builds_only_the_frames_that_stay_references() {
+        let mut dpb = Dpb::new();
+        let (mut built, mut poc_updates) = (Vec::new(), 0u32);
+        // frame_num 0 was the last reference; the next slice says 60_000: 59 999 missing frames.
+        dpb.fill_frame_num_gap(0, 60_000, 1 << 16, 4, &mut |n| (built.push(n), frame(n)).1, &mut |_| poc_updates += 1);
+        assert_eq!(poc_updates, 59_999, "every missing frame advances the POC state");
+        assert_eq!(built, vec![59_996, 59_997, 59_998, 59_999], "only the ones the sliding window would keep are built");
+        assert_eq!(dpb.entries.iter().map(|e| e.frame_num).collect::<Vec<_>>(), built);
+        // A gap shorter than the window builds every frame, as before.
+        let mut dpb = Dpb::new();
+        let mut built = Vec::new();
+        dpb.fill_frame_num_gap(7, 10, 16, 4, &mut |n| (built.push(n), frame(n)).1, &mut |_| {});
+        assert_eq!(built, vec![8, 9]);
+    }
+
+    #[test]
+    fn long_term_marking_cannot_grow_the_dpb_past_its_capacity() {
+        let mut dpb = Dpb::new();
+        dpb.capacity = 4;
+        let mut out = Vec::new();
+        let mut sh = SliceHeader { adaptive_ref_pic_marking: true, ..reference_slice() };
+        // Each picture takes a distinct long-term index, after MMCO 4 allowed 1000 of them.
+        for n in 0..40u32 {
+            sh.frame_num = n;
+            sh.mmcos = vec![Mmco { op: 4, max_long_term_frame_idx_plus1: 1000, ..mmco(0, 0) }, Mmco { op: 6, long_term_frame_idx: n, ..mmco(0, 0) }];
+            let (f, meta) = frame(n);
+            dpb.store_picture(&sh, f, n as i32, 1 << 16, 16, meta, &mut out);
+            assert!(dpb.entries.len() <= 4, "{} entries after picture {n}", dpb.entries.len());
+        }
+        // An index past what MMCO 4 allowed is ignored: the picture is a short-term reference.
+        sh.frame_num = 40;
+        sh.mmcos = vec![Mmco { op: 6, long_term_frame_idx: 5000, ..mmco(0, 0) }];
+        let (f, meta) = frame(40);
+        dpb.store_picture(&sh, f, 40, 1 << 16, 16, meta, &mut out);
+        assert_eq!(dpb.entries.iter().find(|e| e.frame_num == 40).map(|e| e.mark), Some(RefMark::Short));
+    }
+
     #[test]
     fn mmco_huge_difference_does_not_panic() {
         for op in [1, 3] {

crates/video/src/h264/params.rs

@@ -397,6 +397,10 @@ impl Sps {
         let pic_height_in_map_units = plus(r.read_ue()?, 1, "picture too large")?;
         ensure!(pic_width_in_mbs <= 1024 && pic_height_in_map_units <= 1024, "picture too large");
         let frame_mbs_only = r.read_flag()?;
+        // Picture state is allocated from these before any macroblock is read: the movie's pixel
+        // budget applies here too, so a 16 384-square SPS can't claim its planes.
+        let coded_mbs = u64::from(pic_width_in_mbs) * u64::from(pic_height_in_map_units) * if frame_mbs_only { 1 } else { 2 };
+        ensure!(coded_mbs * 256 <= crate::movie::MAX_PIXELS, "picture too large");
         let mb_adaptive_frame_field = if !frame_mbs_only { r.read_flag()? } else { false };
         let direct_8x8_inference = r.read_flag()?;
         let frame_crop = if r.read_flag()? { Some((r.read_ue()?, r.read_ue()?, r.read_ue()?, r.read_ue()?)) } else { None };
@@ -699,6 +703,37 @@ mod tests {
         assert_eq!(l, [10; 16]);
     }
 
+    #[test]
+    fn an_sps_past_the_pixel_budget_is_refused_before_allocation() {
+        // As `parse_minimal_baseline_sps`, with `mbs` × `mbs` macroblocks.
+        let sps = |mbs: u32, frame_mbs_only: bool| {
+            let mut w = BitWriter::new();
+            w.write_bits(66, 8);
+            w.write_bits(0xC0, 8);
+            w.write_bits(30, 8);
+            w.write_ue(0);
+            w.write_ue(0);
+            w.write_ue(2);
+            w.write_ue(1);
+            w.write_bit(false);
+            w.write_ue(mbs - 1);
+            w.write_ue(mbs - 1);
+            w.write_bit(frame_mbs_only);
+            if !frame_mbs_only {
+                w.write_bit(false); // mb_adaptive_frame_field
+            }
+            w.write_bit(true);
+            w.write_bit(false);
+            w.write_bit(false);
+            w.rbsp_trailing();
+            Sps::parse(&w.finish())
+        };
+        assert!(sps(512, true).is_ok(), "8192 × 8192 is the budget");
+        assert!(sps(513, true).is_err(), "one macroblock column more is past it");
+        assert!(sps(512, false).is_err(), "field pictures double the map units");
+        assert!(sps(362, false).is_ok());
+    }
+
     #[test]
     fn parse_minimal_baseline_sps() {
         // profile 66, level 30, sps_id 0, log2_max_frame_num_minus4 0, poc type 2, 1 ref, 11x9 MBs.

xtask/src/version.rs

@@ -197,8 +197,11 @@ mod tests {
 
     #[test]
     fn a_failed_update_restores_the_manifest() {
-        let root = std::env::temp_dir().join(format!("xtask-version-restore-{}", std::process::id()));
-        std::fs::create_dir_all(&root).unwrap();
+        // Created exclusively: a folder already there (planted, or left by a failed run) would be
+        // someone else's, and a link inside it could redirect the writes below.
+        let nanos = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map_or(0, |d| d.subsec_nanos());
+        let root = std::env::temp_dir().join(format!("xtask-version-restore-{}-{nanos}", std::process::id()));
+        std::fs::create_dir(&root).unwrap();
         let path = root.join("Cargo.toml");
         std::fs::write(&path, MANIFEST).unwrap();
         let err = set(&root, &path, MANIFEST, "2.0.0", || Err("cargo update failed".into())).unwrap_err();