fix time parsing and frame boundary edge cases

crates/time/README.md

@@ -35,7 +35,7 @@ assert_eq!(audio_sample, one_sec);
 Frame rates are represented as exact fractions (`num / den`):
 
 ```rust
-use craft_time::FrameRate;
+use craft_time::{FrameRate, Tick};
 
 // Standard NTSC and PAL rates
 let ntsc_24 = FrameRate::FPS_23_976; // 24000 / 1001

crates/time/src/lib.rs

@@ -561,7 +561,7 @@ pub fn format_time(t: Tick, rate: FrameRate, drop_frame: bool, display: TimeDisp
             let abs_ticks = t.0.unsigned_abs();
             let total_secs = abs_ticks / (TICKS_PER_SECOND as u64);
             let rem_ticks = abs_ticks % (TICKS_PER_SECOND as u64);
-            let rem_samples = Tick(rem_ticks as i64).to_units_floor(sample_rate);
+            let rem_samples = Tick(rem_ticks as i64).to_units_floor(sample_rate.max(0));
             let h = total_secs / 3600;
             let m = (total_secs / 60) % 60;
             let s = total_secs % 60;
@@ -625,11 +625,22 @@ fn parse_timecode_with_negative_policy(
     if s.is_empty() {
         return Err(ParseError("empty timecode".into()));
     }
+    if s.len() > 80 {
+        return Err(ParseError("timecode is too long".into()));
+    }
     let (sign_char, body) = match s.as_bytes()[0] {
         b'+' => (Some('+'), &s[1..]),
         b'-' => (Some('-'), &s[1..]),
         _ => (None, s),
     };
+    // Four fields is the complete timecode grammar. Check limits before allocating split vectors.
+    let separators = body.bytes().filter(|b| matches!(b, b':' | b';' | b'.' | b',')).count();
+    if separators > 3 {
+        return Err(ParseError("too many timecode fields".into()));
+    }
+    if separators == 0 && body.len() > if sign_char.is_some() { 19 } else { 8 } {
+        return Err(ParseError("timecode is too long".into()));
+    }
     let parts: Vec<&str> = body.split([':', ';', '.', ',']).collect();
     let nums: Vec<i64> = if parts.len() == 1 {
         let digits = parts[0];
@@ -824,6 +835,15 @@ mod tests {
         }
     }
 
+    #[test]
+    fn timecode_parser_rejects_oversized_inputs_before_field_collection() {
+        let too_many_fields = ":".repeat(100_000);
+        assert!(parse_timecode(&too_many_fields, FrameRate::FPS_30, false, 0).is_err());
+        let compact = "1".repeat(9);
+        assert!(parse_timecode(&compact, FrameRate::FPS_30, false, 0).is_err());
+        assert!(parse_timecode_relative("+12345678901234567890", FrameRate::FPS_30, false, 0).is_err());
+    }
+
     #[test]
     fn test_broadcast_frame_rates() {
         for r in FrameRate::COMMON {
@@ -858,6 +878,7 @@ mod tests {
         assert_eq!(format_time(neg_complex, r, false, TimeDisplay::AudioSamples, 48_000), "-01:01:01:00000");
 
         let zero = Tick::ZERO;
+        assert_eq!(format_time(Tick(2), r, false, TimeDisplay::AudioSamples, -1), "00:00:00:00000");
         assert_eq!(format_time(zero, r, false, TimeDisplay::AudioSamples, 48_000), "00:00:00:00000");
     }
 

crates/time/src/soundcraft/format.rs

@@ -76,7 +76,7 @@ pub fn format_length(start: Samples, len: Samples, fmt: TimeFormat, sr: SampleRa
             let t0 = map.samples_to_ticks(start, sr);
             let t1 = map.samples_to_ticks(start.saturating_add(len), sr);
             let m = map.meter_at_tick(t0);
-            let d = (t1 - t0).max(0);
+            let d = t1.saturating_sub(t0).max(0);
             let bars = d / m.ticks_per_bar().max(1);
             let rem = d % m.ticks_per_bar().max(1);
             format!("{bars}| {}| {:03}", rem / m.ticks_per_beat().max(1), rem % m.ticks_per_beat().max(1))

crates/time/src/soundcraft/grid.rs

@@ -430,7 +430,7 @@ impl GridValue {
                 // always four quarter notes and therefore keeps a continuous tick origin.
                 let bb = map.bar_beat_at_tick(t);
                 let bar_tick = map.bar_start_tick(bb.bar);
-                let origin = if value == NoteValue::Whole { 0 } else { bar_tick };
+                let origin = if value == NoteValue::Whole || (value == NoteValue::Bar && dotted) { 0 } else { bar_tick };
                 let step = Self::note_ticks(value, dotted, triplet, map, origin);
                 let k = (t - origin).div_euclid(step);
                 map.tick_to_samples(origin + k * step, sr)
@@ -547,7 +547,14 @@ impl GridValue {
         }
         let step = self.step_samples(line, sr, map, rate);
         let candidate = self.line_at_or_before(line.saturating_add(step), sr, map, rate);
-        if candidate > line { candidate } else { line.saturating_add(step) }
+        let next = if candidate > line { candidate } else { line.saturating_add(step) };
+        if matches!(self, GridValue::Note { value: NoteValue::Bar, .. }) {
+            return next;
+        }
+        let tick = map.samples_to_ticks(line, sr);
+        let bar = map.bar_beat_at_tick(tick).bar;
+        let next_bar = map.tick_to_samples(map.bar_start_tick(bar.saturating_add(1)), sr);
+        if next_bar > line { next.min(next_bar) } else { next }
     }
 
     /// All grid lines within `[start, end)`, capped at `max` lines.
@@ -681,6 +688,27 @@ mod tests {
         assert_eq!(note(NoteValue::Half).lines(0, 150_000, sr, &map, rate, 10), [0, 48_000, 72_000, 120_000, 144_000]);
     }
 
+    #[test]
+    fn dotted_bar_grid_keeps_its_spacing_across_bars() {
+        let (sr, map, rate) = (SampleRate::HZ_48000, TempoMap::default(), FrameRate::Fps25);
+        let grid = GridValue::Note { value: NoteValue::Bar, dotted: true, triplet: false };
+        assert_eq!(grid.line_at_or_before(143_999, sr, &map, rate), 0);
+        assert_eq!(grid.line_at_or_before(144_000, sr, &map, rate), 144_000);
+        assert_eq!(grid.next_line(0, sr, &map, rate), 144_000);
+    }
+
+    #[test]
+    fn dotted_note_does_not_skip_a_meter_change_boundary() {
+        let sr = SampleRate::HZ_48000;
+        let mut map = TempoMap::default();
+        map.set_meter(TICKS_PER_QUARTER * 4, 2, 4).unwrap();
+        let rate = FrameRate::Fps25;
+        let grid = GridValue::Note { value: NoteValue::Half, dotted: true, triplet: false };
+        assert_eq!(grid.next_line(72_000, sr, &map, rate), 96_000);
+        assert!(grid.lines(72_000, 120_000, sr, &map, rate, 10).contains(&96_000));
+        assert_eq!(grid.snap(90_000, sr, &map, rate), 96_000);
+    }
+
     #[test]
     fn seconds_grid_lines_land_on_whole_seconds() {
         let (sr, map, rate) = (SampleRate::HZ_44100, TempoMap::default(), FrameRate::Fps30);
@@ -745,7 +773,8 @@ mod tests {
                 let frame = GridValue::drop_second_frame(seconds, rate);
                 let line = rate.samples_at_frame(frame, sr);
                 assert_eq!(grid.line_at_or_before(line, sr, &map, rate), line, "{rate:?} second {seconds}");
-                assert_eq!(grid.line_at_or_before(-line, sr, &map, rate), -line, "{rate:?} second -{seconds}");
+                let negative_line = rate.samples_at_frame(-frame, sr);
+                assert_eq!(grid.line_at_or_before(negative_line, sr, &map, rate), negative_line, "{rate:?} second -{seconds}");
                 for offset in -2..=rate.nominal() + 2 {
                     let position = rate.samples_at_frame(frame.saturating_add(offset), sr);
                     let before = grid.line_at_or_before(position, sr, &map, rate);
@@ -761,8 +790,9 @@ mod tests {
             let boundary = rate.samples_at_frame(minute_frame, sr);
             assert_eq!(grid.line_at_or_before(boundary, sr, &map, rate), boundary, "{rate:?} positive minute boundary");
             assert_eq!(grid.snap(boundary, sr, &map, rate), boundary, "{rate:?} positive minute snap");
-            assert_eq!(grid.line_at_or_before(-boundary, sr, &map, rate), -boundary, "{rate:?} negative minute boundary");
-            assert_eq!(grid.snap(-boundary, sr, &map, rate), -boundary, "{rate:?} negative minute snap");
+            let negative_boundary = rate.samples_at_frame(-minute_frame, sr);
+            assert_eq!(grid.line_at_or_before(negative_boundary, sr, &map, rate), negative_boundary, "{rate:?} negative minute boundary");
+            assert_eq!(grid.snap(negative_boundary, sr, &map, rate), negative_boundary, "{rate:?} negative minute snap");
         }
     }
 

crates/time/src/soundcraft/mod.rs

@@ -38,8 +38,16 @@ pub enum TimeError {
 
 /// Sample rates supported by sessions (Pro Tools offers 44.1 k – 192 k; we accept 8 k – 768 k).
 #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash, serde::Serialize, serde::Deserialize)]
+#[serde(try_from = "u32")]
 pub struct SampleRate(u32);
 
+impl TryFrom<u32> for SampleRate {
+    type Error = TimeError;
+    fn try_from(hz: u32) -> Result<Self, Self::Error> {
+        Self::new(hz)
+    }
+}
+
 impl SampleRate {
     pub const HZ_44100: SampleRate = SampleRate(44_100);
     pub const HZ_48000: SampleRate = SampleRate(48_000);
@@ -107,7 +115,7 @@ impl Range {
         s >= self.start && s < self.end
     }
     pub fn overlaps(&self, o: &Range) -> bool {
-        self.start < o.end && o.start < self.end
+        !self.is_empty() && !o.is_empty() && self.start < o.end && o.start < self.end
     }
     pub fn intersect(&self, o: &Range) -> Option<Range> {
         let s = self.start.max(o.start);
@@ -123,6 +131,24 @@ impl Range {
 mod tests {
     use super::*;
 
+    #[test]
+    fn sample_rate_serde_validates_nonzero_bounds() {
+        assert!(serde_json::from_str::<SampleRate>("0").is_err());
+        assert!(serde_json::from_str::<SampleRate>("7999").is_err());
+        let rate = SampleRate::new(48_000).unwrap();
+        assert_eq!(serde_json::to_string(&rate).unwrap(), "48000");
+        assert_eq!(serde_json::from_str::<SampleRate>("48000").unwrap(), rate);
+    }
+
+    #[test]
+    fn empty_ranges_never_overlap() {
+        let point = Range::point(5);
+        let span = Range::new(0, 10);
+        assert!(!point.overlaps(&span));
+        assert!(!span.overlaps(&point));
+        assert!(Range::new(0, 5).overlaps(&span));
+    }
+
     #[test]
     fn sample_rate_bounds() {
         assert!(SampleRate::new(48_000).is_ok());

crates/time/src/soundcraft/timecode.rs

@@ -108,9 +108,12 @@ impl FrameRate {
         if n == 0 {
             return 0;
         }
-        // Round half away from zero so negative frames mirror positive ones.
-        let mag = (i128::from(f.unsigned_abs()) * i128::from(sr.hz()) * i128::from(d) + i128::from(n) / 2) / i128::from(n);
-        let v = if f < 0 { -mag } else { mag };
+        // Use the first integer sample whose floored frame lookup is `f`; nearest rounding
+        // can put fractional-rate frame starts just before their own lookup interval.
+        let numerator = i128::from(f) * i128::from(sr.hz()) * i128::from(d);
+        let denominator = i128::from(n);
+        let floor = numerator.div_euclid(denominator);
+        let v = floor + i128::from(numerator.rem_euclid(denominator) != 0);
         i64::try_from(v).unwrap_or(if v < 0 { i64::MIN } else { i64::MAX })
     }
 
@@ -194,6 +197,12 @@ impl Timecode {
             Some(r) => (true, r),
             None => (false, t),
         };
+        if t.len() > 80 {
+            return Err(TimeError::Parse("timecode is too long".into(), "timecode"));
+        }
+        if t.bytes().filter(|b| matches!(b, b':' | b';' | b'.')).count() > 3 {
+            return Err(TimeError::Parse(text.to_string(), "timecode"));
+        }
         let parts: Vec<&str> = t.split([':', ';', '.']).collect();
         if parts.is_empty() || parts.len() > 4 {
             return Err(TimeError::Parse(text.to_string(), "timecode"));
@@ -232,15 +241,16 @@ mod tests {
     }
 
     #[test]
-    fn negative_timecode_rounds_symmetrically() {
+    fn fractional_frame_starts_match_floored_lookup_on_both_sides_of_zero() {
         let sr = SampleRate::HZ_48000;
         let tc = Timecode::parse("-00:00:01:00").unwrap();
         assert_eq!(tc.to_samples(FrameRate::Fps25, sr), -48_000);
         for r in [FrameRate::Fps25, FrameRate::Fps29_97, FrameRate::Fps29_97Drop, FrameRate::Fps23_976] {
-            for f in [1i64, 7, 25, 1799, 107_892] {
-                assert_eq!(r.samples_at_frame(-f, sr), -r.samples_at_frame(f, sr), "{r:?} frame {f}");
+            for f in [-107_892i64, -25, -7, -1, 0, 1, 2, 7, 25, 1799, 107_892] {
+                assert_eq!(r.frames_at(r.samples_at_frame(f, sr), sr), f, "{r:?} frame {f}");
             }
         }
+        assert_eq!(FrameRate::Fps29_97.frames_at(FrameRate::Fps29_97.samples_at_frame(2, sr), sr), 2);
     }
 
     #[test]
@@ -299,6 +309,7 @@ mod tests {
         assert_eq!((tc.seconds, tc.frames), (3, 4));
         assert!(Timecode::parse("a:b").is_err());
         assert!(Timecode::parse("1:2:3:4:5").is_err());
+        assert!(Timecode::parse(&format!("{}:", "1".repeat(100_000))).is_err());
     }
 
     #[test]