Previously the current iterator bbt was moved to p->bbt().
From then on, no grid line is reached if the point p is not on
a bar and mod_bar != 0 or the point is not on an expected beat_div.
e.g. when using bbt += mod_bar, and a tempo-change is at 5|2|0.
iterations continues 6|2|0 7|2|0 is_bar() is always false
and no more grid-lines were added.
Rather than trying
bbt = round-up-to-next-grid-mod-div
and then finding the metric for that position, this
approach only does the latter using the already incremented
BBT position.
We cannot call TempoMetric::superclock_at (BBT_Time) if the BBT time is beyond
the range of the current TempoMetric. We must discover that *before* we make
that call, not as part of the test to see if we've exceeded the range.
There were many logical errors in the previous implementation. This one is
simpler to read, and appears to work much better.
It also allows the caller to specify the quarter-note subdivision to use when
generating the grid, rather than choosing only between some bar modulo or
quarter notes.
introducing a new time signature that uses non-quarter notes as the denominator
will move the beat position a given BBT time (since the middle "B" of BBT
refers to "beats" given by the denominator, not quarters).
This is never for inline references to parameters, only for starting parameter
documentation blocks. The "@p" command is for this, although unfortunately
Doxygen doesn't actually do anything with it and it's just an alias for code
text.
MIDI clock start at the next beat (round_up_to_beat), so
here we have to round the current tick, rather than fall
back to a tick that is not yet complete, as 14da117bc8 does.
Reproduced with the Session from #9027
Start loop at bar 40 with MClk generator enabled.
```
#3 in __GI___assert_fail (assertion=0x7fedd86c4fd5 "clk_pos >= pos", file=0x7fedd86c38b7 "../libs/temporal/tempo.cc", line=3336, function=0x7fedd86c4f60 "void Temporal::TempoMap::midi_clock_beat_at_or_after(Temporal::samplepos_t, Temporal::samplepos_t&, uint32_t&) const") at assert.c:101
#4 in Temporal::TempoMap::midi_clock_beat_at_or_after(long, long&, unsigned int&) const (this= 0x560187e92c00, pos=20691033, clk_pos=@0x7fedc02178b8: 20691032, clk_beat=@0x7fedc02178c4: 11472) at ../libs/temporal/tempo.cc:3336
#5 in ARDOUR::MidiClockTicker::tick(long, long, unsigned int, long) (this=0x56018eed6db0, start_sample=20691033, end_sample=20692057, n_samples=1024, pre_roll=0) at ../libs/ardour/ticker.cc:170
#6 in ARDOUR::Session::send_mclk_for_cycle(long, long, unsigned int, long) (this=0x56018a216340, start_sample=20691033, end_sample=20692057, n_samples=1024, pre_roll=0) at ../libs/ardour/session.cc:7495
#7 in ARDOUR::AudioEngine::process_callback(unsigned int) (this=0x5601881a4f20, nframes=1024) at ../libs/ardour/audioengine.cc:563
```
Found via `codespell -q 3 -S *.po,./.git,./share/patchfiles,./libs,./msvc_extra_headers,./share/web_surfaces,*.patch -L ba,buss,busses,discreet,doubleclick,hsi,ontop,ro,scrollin,seh,siz,sord,sur,te,trough,ue`
Also remove Meter:: versions of related methods, because they are not necessary. We
only need metrical information for operations like ::round_to_bar()
This expands significantly the maximum number of Beats that can be represented, which is a good
thing in itself. It slightly speeds up some Beats::operator methods, and slightly slows down
::get_beats() and ::get_ticks().
One minor change in an API user was required, and several tweaks to the unit tests due to the
macros being used by cppunit creating possible type confusion.
Units test pass
When TempoMap::copy_points() is called, the new points are intended to belong
to the (nascent) new map. But the copy constructor for the points leaves the
_map member of a Point unchanged, and so the new points reference the old
map (forever!). ::copy_points() must reset each Point to reference the new map.
Refactored the object that has the _map member, so that we could limit access
to its ::set_map() method to TempoMap.
Because a bartime point IS-A tempo point and IS-A meter point, we cannot just delete the tempo
point passed into core_add_tempo() if the new point replaces an existing one. Ditto for meter.
So, leave that logic up to the caller
Even if we start at time T, if the tempo point before T is ramped, we need to
adjust it's omega value based on the beat time of the following tempo point. If
we don't do this, then using that tempo to compute ::superclock_at() for
subsequent points will be incorrect.
Note: there may be an ordering issue here with Tempo/Meter points. We might
need to pass over all tempo points first, then process meters etc. Something
like that.
changes in tempo can be negative (decelerando/slowing down), and thus so can _omega. We cannot call log() or its
cousins on negative values, so in this scenario use an alternate expression for "t from b" in the tempo.pdf paper
omega can be computed from Beat or superclock duration. This gives rise to
different units for omega, and we must use the correct value in a given
context.
This commit also changes the way that the audio time omega is computed during
TempoMap::reset_starting_at()
Rampable only existed to provide exclusive access to ::set_end() for the
TempoMap. More idiomatic C++ but now that _type has also gone away, so has
::set_ramped() and it really was not worth keeping it around.
Ramped/Constant is really a function of start/end note_types_per_minute. Having
a separate member is really just caching it and leads to errors or risk thereof.
We were using a given tempo or meter point as part of the metric used when
recomputing its position. In fact, the metric should be only use the
tempo/meter immediately preceding the given point.
It is more useful to get the actual TempoPoint than just the Tempo
that we discover.
Aside: it would be awesome to understand how to use boost::intrusive to get the
next tempo point directly from the tempo hook.
This commit leaves two issues outstanding:
1. unclear/ugly semantics for drag operations that reset the GUI thread's tempo map to the writable copy
2. undo/redo for the tempo map
These will be addressed in future commits
std::upper_bound() was not the correct tool to find the existing point,
it should have been std::lower_bound(). For code consistency, this
change doesn't use either but like ::remove_tempo() and similar methods,
just iterates over the whole list
We do not want a value as large as the previous one, which limits the time
range that can be represented in 62 bits unnecessarily. The new value is
9 times smaller than the previous value, and loses only 384000 as a significant
factor.
This commit also switches to using an (inline) accessor for superclock_ticks_per_second,
making it possible in debug/testing phases to spot early/illegal uses of the value.
The old code used the instantaneous tempo at T0 to compute where the next
quarter note would be. This is incorrect, since the tempo is
changing (continuously, for now) during the time represented by that quarter
note. Instead, we need to add a quarter note (or technically, whatever the
tempo note type is) to get a new position in beats, then compute the superclock
time at that location (which will use our equations for tempo, including the
use of omega, the ramp factor).
These comments should correct an impression left in the commit message for
6e9e28343bc3695d that there may be some sort of problem with synchronization
of TempoMap changes. The actual problem is that TempoMap edits are done using
RCU, so the modifications are performaned using a copy of the map, but with
map elements taken from the pre-copy version.
The correct algorithm is to traverse the type-specific list of points,
find the point (if any) whose time matches that of the argument (because
we do not allow multiple points of the same type at the same time), and
then use that discovered point from the _points list.
This approach is required because the actual argument may no longer be
in the tempo map (due to a change made by another thread). The lack of
sync, however, needs investigation.
a negative beat position needs to be legal, so the assert was moved and modified. The only check
for a negative value is that the TempoPoint being used is at absolute zero.
This check might turn out to be wrong in the future, but for now we still require a tempo and
meter point at absolute zero
pbd/i18n.h MUST NEVER be included from header files and always be
the last include. This is because `_` is declared other headers
notably boost and some apple headers.
leading to issues like
../libs/pbd/gettext.h:58:27: error: expected unqualified-id before ‘const’
58 | # define gettext(Msgid) ((const char *) (Msgid))
