Add beat-based Clip start/length/end APIs#15
Merged
Conversation
Adds setStart(BeatPosition,...), setLength(BeatDuration,...),
setEnd(BeatPosition,...), and the matching getStartBeats /
getEndBeats / getLengthBeats on the base Clip class, mirroring
the existing time-based setters. They delegate to the time
versions after converting against Edit::tempoSequence, so the
result is tempo-curve aware rather than the flat-tempo
seconds calculation callers do today.
setLength is implemented in terms of setEnd against the
absolute end beat (start + length, converted via the tempo
sequence), not as a duration-from-zero conversion, so it
behaves correctly across tempo changes inside the clip.
setOffset(BeatDuration) is intentionally not included: offset
semantics differ per clip type (MIDI offset is source-domain
beats, audio offset is source seconds scaled by stretching),
so a single base-class beat overload would have ambiguous
meaning. That can be added per-subclass later if needed.
Disambiguates one internal TE call site
(Edit::createBlankClip) where setStart({}) became ambiguous
between TimePosition and BeatPosition.
Stage 2 of #12.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Stage 2 of #12. Adds beat-based overloads on the base
Clipclass for clip placement, mirroring the transport beat APIs from #14.Setters:
void setStart(BeatPosition, bool preserveSync, bool keepLength)void setLength(BeatDuration, bool preserveSync)void setEnd(BeatPosition, bool preserveSync)Getters:
BeatPosition getStartBeats() constBeatPosition getEndBeats() constBeatDuration getLengthBeats() constAll implementations convert through
Edit::tempoSequenceand delegate to the existing time-based setters/storage. Internal storage stays asTimePosition/TimeDuration— this is the same wrapper-overload pattern as the transport change, centralising conversion inside TE instead of duplicating it at every caller.Why this is more than a thin wrapper
setLength(BeatDuration)is implemented assetEnd(startBeats + length)converted through the tempo sequence at the absolute end position, not as a flat-tempo duration calculation. That means it stays correct across tempo changes inside the clip — which the current MAGDA pattern (length_seconds = beats * 60 / bpm) does not.What's deliberately not included
setOffset(BeatDuration)is omitted. Offset semantics differ per clip type:speedRatioA single base-class beat overload would have ambiguous meaning. That can be added per-subclass later (most likely on
MidiClipfirst) once the MAGDA port forces the question.Internal change
One TE-internal call site disambiguated:
Edit::createBlankCliphadsetStart({}, false, true)which became ambiguous betweenTimePositionandBeatPosition. Changed toTimePosition().Test plan
make debugagainst MAGDA builds clean (verified locally)