From 14da117bc880ebcc8a805348a97acdae31b2753b Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Sat, 22 Oct 2022 02:10:05 +0200 Subject: [PATCH] Add explicit round/floor integer multiply/divide This fixes various rounding issues. Notably superclock to sample conversion must always round down when playing forward. `::process (start, end, speed = 1)` uses exclusive end. Processing begins at `start` and end ends just before `end`. Next cycle will begin with the current end. One example where this failed: - New session at 48kHz - Change tempo to 130 BPM - Enable snap to 1/8 note - Snap playhead to 1|3|0 - Enable Metronome - Play `assert (superclock_to_samples ((*i).sclock(), sample_rate()) < end);` end = 177231 samples == superclock 1042118280 A grid point is found at superclock 1042116920 (that is < 1042118280). However converting it back to samples rounded it to sample 177231 == end, while actual location is 1360 super-clock ticks before end. The metronome click has to be started this cycle, since the same position will not be found at the beginning of the next cycle, with start = 177232. Similarly a samplecnt_t t, converted to music-time and back must not be later than the given sample. ``` timepos_t tsc (t); assert (timepos_t::from_ticks (tsc.ticks ()).samples () <= t); ``` IOW. When playing forward, all super-clock time between 1|1|0 and 1|1|1 should round down to 1|1|0. "We have not yet reached the first tick". --- libs/pbd/pbd/integer_division.h | 36 ++++++++++++++++++++++++++++- libs/temporal/temporal/beats.h | 4 ++-- libs/temporal/temporal/superclock.h | 4 ++-- libs/temporal/temporal/tempo.h | 2 +- libs/temporal/timeline.cc | 8 +++---- 5 files changed, 44 insertions(+), 10 deletions(-) diff --git a/libs/pbd/pbd/integer_division.h b/libs/pbd/pbd/integer_division.h index e2fe056397..c8b05649ab 100644 --- a/libs/pbd/pbd/integer_division.h +++ b/libs/pbd/pbd/integer_division.h @@ -50,7 +50,7 @@ namespace PBD { */ inline -int64_t muldiv (int64_t v, int64_t n, int64_t d) +int64_t muldiv_round (int64_t v, int64_t n, int64_t d) { /* either n or d or both could be negative but for now we assume that only d could be (that is, n and d represent negative rational numbers of the @@ -92,6 +92,40 @@ int64_t muldiv (int64_t v, int64_t n, int64_t d) return(int64_t) (((_v * _n) + hd) / _d); #endif } + +inline +int64_t muldiv_floor (int64_t v, int64_t n, int64_t d) +{ +#ifndef COMPILER_INT128_SUPPORT + boost::multiprecision::int512_t bignum = v; + + bignum *= n; + bignum /= d; + + try { + + return bignum.convert_to (); + + } catch (...) { + fatal << "arithmetic overflow in timeline math\n" << endmsg; + /* NOTREACHED */ + return 0; + } + +#else + __int128 _n (n); + __int128 _d (d); + __int128 _v (v); + + /* this could overflow, but will not do so merely because we are + * multiplying two int64_t together and storing the result in an + * int64_t. Overflow will occur where (v*n)+hd > INT128_MAX (hard + * limit) or where v * n / d > INT64_T (i.e. n > d) + */ + + return(int64_t) ((_v * _n) / _d); +#endif +} } /* namespace */ #endif /* __libpbd_integer_division_h___ */ diff --git a/libs/temporal/temporal/beats.h b/libs/temporal/temporal/beats.h index b9395388d0..40541d974c 100644 --- a/libs/temporal/temporal/beats.h +++ b/libs/temporal/temporal/beats.h @@ -205,8 +205,8 @@ public: Beats operator*(int32_t factor) const {return ticks (_ticks * factor); } Beats operator/(int32_t factor) const { return ticks (_ticks / factor);} - Beats operator*(ratio_t const & factor) const {return ticks (PBD::muldiv (_ticks, factor.numerator(), factor.denominator())); } - Beats operator/(ratio_t const & factor) const {return ticks (PBD::muldiv (_ticks, factor.denominator(), factor.numerator())); } + Beats operator*(ratio_t const & factor) const {return ticks (PBD::muldiv_round (_ticks, factor.numerator(), factor.denominator())); } + Beats operator/(ratio_t const & factor) const {return ticks (PBD::muldiv_round (_ticks, factor.denominator(), factor.numerator())); } Beats operator% (Beats const & b) const { return Beats::ticks (_ticks % b.to_ticks());} diff --git a/libs/temporal/temporal/superclock.h b/libs/temporal/temporal/superclock.h index 68ea2e0885..e3394c915d 100644 --- a/libs/temporal/temporal/superclock.h +++ b/libs/temporal/temporal/superclock.h @@ -47,8 +47,8 @@ static inline superclock_t superclock_ticks_per_second() { if (!scts_set) { rais static inline superclock_t superclock_ticks_per_second() { return _superclock_ticks_per_second; } #endif -static inline superclock_t superclock_to_samples (superclock_t s, int sr) { return PBD::muldiv (s, sr, superclock_ticks_per_second()); } -static inline superclock_t samples_to_superclock (int64_t samples, int sr) { return PBD::muldiv (samples, superclock_ticks_per_second(), superclock_t (sr)); } +static inline superclock_t superclock_to_samples (superclock_t s, int sr) { return PBD::muldiv_floor (s, sr, superclock_ticks_per_second()); } +static inline superclock_t samples_to_superclock (int64_t samples, int sr) { return PBD::muldiv_round (samples, superclock_ticks_per_second(), superclock_t (sr)); } LIBTEMPORAL_API extern int most_recent_engine_sample_rate; diff --git a/libs/temporal/temporal/tempo.h b/libs/temporal/temporal/tempo.h index 090da7ec0c..69e1373935 100644 --- a/libs/temporal/temporal/tempo.h +++ b/libs/temporal/temporal/tempo.h @@ -245,7 +245,7 @@ class LIBTEMPORAL_API Tempo { static void superbeats_to_beats_ticks (int64_t sb, int32_t& b, int32_t& t) { b = sb / big_numerator; int64_t remain = sb - (b * big_numerator); - t = int_div_round ((Temporal::ticks_per_beat * remain), big_numerator); + t = PBD::muldiv_floor (Temporal::ticks_per_beat, remain, big_numerator); } bool active () const { return _active; } diff --git a/libs/temporal/timeline.cc b/libs/temporal/timeline.cc index 4910641101..808c911cee 100644 --- a/libs/temporal/timeline.cc +++ b/libs/temporal/timeline.cc @@ -201,9 +201,9 @@ timecnt_t timecnt_t::scale (ratio_t const & r) const { if (time_domain() == AudioTime) { - return timecnt_t::from_superclock (PBD::muldiv (_distance.val(), r.numerator(), r.denominator()), _position); + return timecnt_t::from_superclock (PBD::muldiv_round (_distance.val(), r.numerator(), r.denominator()), _position); } else { - return timecnt_t::from_ticks (PBD::muldiv (_distance.val(), r.numerator(), r.denominator()), _position); + return timecnt_t::from_ticks (PBD::muldiv_round (_distance.val(), r.numerator(), r.denominator()), _position); } } @@ -592,9 +592,9 @@ timepos_t timepos_t::scale (ratio_t const & n) const { if (time_domain() == AudioTime) { - return timepos_t::from_superclock (PBD::muldiv (val(), n.numerator(), n.denominator())); + return timepos_t::from_superclock (PBD::muldiv_round (val(), n.numerator(), n.denominator())); } else { - return timepos_t::from_ticks (PBD::muldiv (val(), n.numerator(), n.denominator())); + return timepos_t::from_ticks (PBD::muldiv_round (val(), n.numerator(), n.denominator())); } }