Compare commits

...

5 Commits

Author SHA1 Message Date
a7ee848f70 temporal: improve accuracy of a comment/XXX item 2022-05-24 17:21:18 -06:00
Mads Kiilerich
c1c95f538f evoral: fix ControlList::_x_scale to avoid ratio overflow when adjusting fade length
Region fades would sometimes get in a mode with weird behaviour. They
would be drawn in 2d with crossing lines, mainly moving back and forth
horizontally - not as a function of time. It would sound as it looked.
The fade would sometimes jump around when resizing. It could be worked
around by resetting the fade shape. It turned out the problem could be
reproduced by making minute long fades.

This change fixes or works around the problem.

Back story:

timepos_t (in temporal/timeline.h) uses 62 bit for integer value and the
max value is thus 4611686018427387904 ~ 5e18. timepos_t counts
superclocks, where superclock_ticks_per_second is 56448000 ~ 6e7. It can
thus store up to 8e10 seconds - thousands of years.

ratio_t (in temporal/types.h) can represent fractions as 64 bit (signed)
numerator and denominator. timepos_t avoids floating point operations,
but has operator* with ratio_t. To avoid crazy loss of precision it will
multiply the superclock count with the numerator before dividing with
the denominator.

Audio region fade in and out uses a number of increasing timepos_t
values (in a ControlList) up to the length of fade. When dragging to
resize, these values are (in_x_scale) multiplied with the ratio_t of the
new and old fade length. The problem is that the 62 bits will overflow
if using fades more than sqrt(5e18) ~ 2e9 superclock ticks ~ 38 seconds.
It will overflow into the "beat" flag and (at 58 seconds) into the sign
bit. The timepos_t values in the fade will thus jump and can be negative
or change to count beats.

To work around that problem, this changeset just use floating point
values for scaling the timepos_t values. All scaled values are stored as
integer anyway, so it should not make any actual difference for this use
case. There might however be other uses of ControlList where it matters.

As an implementation detail of this "workaround" of using double, it
could perhaps also be nice to implement timepos_t operator* (or
operator*=) for double. But I'm not sure we want floating point support
in timepos_t.

An alternative (and better) solution would be to convince the fraction
multiplication to use 128 bits. It is essential to avoid overflow -
mainly in static analysis, alternatively as runtime checks or asserts.
2022-05-24 17:15:37 -06:00
Mads Kiilerich
2f5f917df2 libs/temporal: clarify superclocks-per-second usage comment 2022-05-24 17:15:37 -06:00
Mads Kiilerich
aadd24a4e5 libs/temporal/temporal/types.h: fix confusing indentation 2022-05-24 17:15:37 -06:00
Mads Kiilerich
cc6a1e2d6f gtk2_ardour/editor_drag.cc: fix confusing newline 2022-05-24 17:15:37 -06:00
4 changed files with 8 additions and 4 deletions

View File

@ -277,8 +277,8 @@ Drag::Drag (Editor* e, ArdourCanvas::Item* i, Temporal::TimeDomain td, bool trac
, _constraint_pressed (false)
, _grab_button (-1)
{
}
timepos_t
Drag::pixel_to_time (double x) const
{

View File

@ -359,8 +359,9 @@ ControlList::list_merge (ControlList const& other, boost::function<double(double
void
ControlList::_x_scale (ratio_t const & factor)
{
double double_factor = (double)factor;
for (iterator i = _events.begin(); i != _events.end(); ++i) {
(*i)->when = (*i)->when.operator* (factor);
(*i)->when = timepos_t::from_superclock ((*i)->when.val() * double_factor + 0.5);
}
mark_dirty ();

View File

@ -2363,7 +2363,10 @@ TempoMap::set_state (XMLNode const & node, int version)
/* global map properties */
/* XXX this should probably be at the global level in the session file because it affects a lot more than just the tempo map, potentially */
/* XXX this should probably be at the global level in the session file
* because it is the time unit for anything in the audio time domain,
* and affects a lot more than just the tempo map
*/
superclock_t sc;
if (node.get_property (X_("superclocks-per-second"), sc)) {
set_superclock_ticks_per_second (sc);

View File

@ -89,7 +89,7 @@ class _ratio_t {
int64_t operator* (int64_t v) const { return int_div_round (v * _numerator, _denominator); }
private:
private:
T _numerator;
T _denominator;
};