diff --git a/crates/time/README.md b/crates/time/README.md index 0f62fb3b1..9c85b0219 100644 --- a/crates/time/README.md +++ b/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 diff --git a/crates/time/src/lib.rs b/crates/time/src/lib.rs index 6cc780cf9..4a3af58df 100644 --- a/crates/time/src/lib.rs +++ b/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 = 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"); } diff --git a/crates/time/src/soundcraft/format.rs b/crates/time/src/soundcraft/format.rs index 11b588c3f..b16511006 100644 --- a/crates/time/src/soundcraft/format.rs +++ b/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)) diff --git a/crates/time/src/soundcraft/grid.rs b/crates/time/src/soundcraft/grid.rs index 5f32ca6d8..2d40a454b 100644 --- a/crates/time/src/soundcraft/grid.rs +++ b/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"); } } diff --git a/crates/time/src/soundcraft/mod.rs b/crates/time/src/soundcraft/mod.rs index bd4df3aa9..300909003 100644 --- a/crates/time/src/soundcraft/mod.rs +++ b/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 for SampleRate { + type Error = TimeError; + fn try_from(hz: u32) -> Result { + 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 { 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::("0").is_err()); + assert!(serde_json::from_str::("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::("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()); diff --git a/crates/time/src/soundcraft/timecode.rs b/crates/time/src/soundcraft/timecode.rs index 03eb1cc94..d09458e5c 100644 --- a/crates/time/src/soundcraft/timecode.rs +++ b/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]