Commit Graph

6 Commits

Author SHA1 Message Date
Skyler Lehmkuhl a5cdbfd0fd Type the time domains: no raw f64 in any time-carrying API
Three bugs in a row came from the same root: a time value crossing an API
boundary as a bare f64, with the caller and the callee disagreeing about
whether it meant seconds or beats. Recording landed at the wrong time,
MIDI clips grew too fast, and a 1-second split played back as half a
second. Each was "obviously" one domain at the call site and read as the
other on the far side. This makes the mismatch a compile error.

Backend API — every time-carrying f64 is gone:
- Commands: Seek/SetOffset/SetTrimStart/SetTrimEnd -> Seconds; MoveClip/
  ExtendClip/CreateMidiClip/AddMidiNote/AddLoadedMidiClip/
  UpdateMidiClipNotes/AddMidiClipSync and all four automation commands ->
  Beats; TrimClip -> TrimRange.
- Events/queries: PlaybackPosition, WaveformChunksReady's time range,
  AudioFileReady::duration, PoolFileInfo, get_playhead_seconds -> Seconds.
- Serialized: MidiClipData::duration and AutomationKeyframeData::time ->
  Beats. Both newtypes are #[serde(transparent)], so the .beam on-disk
  format is unchanged.
- Several controller methods ALREADY took Beats and unwrapped it to shove
  into the command — the newtype was being discarded at the very boundary
  it existed to protect.

TrimRange, for the domain-polymorphic case: a clip's content time is
SECONDS for sampled audio but BEATS for MIDI, so a single newtype can't
express it (there was even a comment in engine.rs saying so, and that
rationalization is what let the bug through). A domain-tagged enum can.
The engine rejects a range whose domain doesn't match the track, and the
range is built from the clip (clip.trim_range()) so callers can't pick
the wrong variant.

ContentTime, for the trim fields: ClipInstance::trim_start/trim_end are
content times, and were the last untyped f64 — the actual root of the
split bug. ContentTime is deliberately a DEAD END: no .to_seconds(), no
.to_beats(), no arithmetic with Seconds or Beats. Content times combine
freely with each other (same clip, same domain — safe), so the ~100
passthrough sites cost nothing; the only exit is resolving against the
clip that knows the domain (AudioClip::resolve_content_time /
Document::resolve_content_time / ClipDuration::same_domain). Mixing
domains no longer compiles.

Two more live bugs the types surfaced:
- ClipInstance::effective_duration_beats took a SECONDS clip duration and
  subtracted trim_start from it. For a TRIMMED MIDI clip that subtracted
  a beats offset from a seconds duration, so the clip's timeline length
  was wrong at any tempo but 60 BPM. Untrimmed clips happened to work,
  which is why it hid. It now takes a ClipDuration and resolves in the
  clip's own domain: beats content carries over directly (tempo-
  invariant), wall-clock content converts at the clip's position.
  Regression test asserts a clip trimmed to beats 2..6 is 4 beats long at
  60/90/120 BPM.
- Trim validation clamped a content-domain trim against a wall-clock gap.
  gap_to_content/content_to_secs now convert at the clip's position.

Also folds two more copies of the backend add-logic into
BackendContext::add_clip_instance (split and remove_clip_instances both
re-add clips), so the trim/duration conversions live in exactly one place
instead of four.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-14 09:31:04 -04:00
Skyler Lehmkuhl 64bf9bb431 Type the rest of the audio-controller boundary (no more bare-f64 wrapping)
Push Beats/Seconds through the remaining controller methods that took a bare
f64 and let the audio thread wrap it in a newtype, so the caller's domain is
now compiler-checked (the seam that hid the recording bug):

- seek -> Seconds
- set_trim_start/set_trim_end -> Seconds / Option<Seconds> (metatrack, always seconds)
- add_midi_note, add_loaded_midi_clip, update_midi_clip_notes -> Beats
- add_automation_point, remove_automation_point, automation_add_keyframe,
  automation_remove_keyframe -> Beats

Command enums stay raw f64 transport; only the public signatures + call sites
change. No behavior change — every caller already passed the right domain, this
just makes it enforced.

Two deliberate exceptions, documented in place:
- trim_clip stays f64: the TrimClip handler interprets it as Seconds for a
  sampled-audio clip but Beats for a MIDI clip, so no single newtype fits;
  callers pass the clip's own trim value, which matches its content domain.
- The piano-roll MIDI note model stays f64 internally (a beats-only subsystem
  with no seconds anywhere); it's converted to Beats at the update_midi_clip_notes
  boundary in UpdateMidiNotesAction, same as trim_start f64 -> Seconds at add_audio_clip.
2026-07-11 14:33:22 -04:00
Skyler Lehmkuhl b5766672ee Fix recording start position: pass beats, not seconds, to the backend
The reported bug (a second recording lands early and overlaps the first)
survived the timeline type refactor: start_recording/create_midi_clip/
start_midi_recording took f64 and wrapped Beats(x) internally, so the type
boundary stopped at the method and the timeline handed them *shared.playback_time
(seconds). At 120 BPM a 5s playhead (=10 beats) was recorded at beat 5 = 2.5s.

Type all three backend methods to take Beats so the caller must convert; the
timeline now converts the seconds playhead once (start_beats) and passes it to
every recording command and the placeholder clip. TUI debug caller updated.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-11 14:10:44 -04:00
Skyler Lehmkuhl f372a84313 Massive tempo refactor - make beats canonical time rep and allow them to be non constant 2026-04-02 10:26:01 -04:00
Skyler Lehmkuhl a1ad0b44b1 Add support for audio trimming and looping 2025-11-28 06:53:28 -05:00
Skyler Lehmkuhl 16f4a2a359 Add audio node graph editing 2025-10-25 03:29:54 -04:00