From c6a31250babd49354e5fd2284d255e5207a47c1c Mon Sep 17 00:00:00 2001
From: Paul Davis <paul@linuxaudiosystems.com>
Date: Fri, 28 Jan 2022 15:52:27 -0700
Subject: [PATCH] alter Source::_length from timecnt_t to timepos_t

THe length of a Source(File) is always measured from its start. In this sense,
the length is like a position on the timeline, which is a duration with an
implicit origin, or a Region start, also a duration with an implicit origin (in
that case the start of the Source). There is no good reason for using
a timecnt_t for this value, because the position component of a timecnt_t
(the origin for the duration) is implicit and always zero. So we make
this property into a timepos_t, and include a number of asserts() to check
for common possible coding errors related to the time domain
---
 gtk2_ardour/editor_audio_import.cc | 16 ++++++++------
 libs/ardour/ardour/audiosource.h   |  2 +-
 libs/ardour/ardour/region.h        |  3 +--
 libs/ardour/ardour/smf_source.h    |  2 +-
 libs/ardour/ardour/source.h        |  7 +++---
 libs/ardour/ardour/srcfilesource.h |  4 ++--
 libs/ardour/audiosource.cc         | 10 ++++++---
 libs/ardour/import.cc              |  2 +-
 libs/ardour/region.cc              | 35 ++++++++++--------------------
 libs/ardour/smf_source.cc          | 14 +++++++-----
 libs/ardour/sndfilesource.cc       |  4 +++-
 libs/ardour/source.cc              |  3 +--
 libs/temporal/temporal/timeline.h  |  3 +++
 13 files changed, 52 insertions(+), 53 deletions(-)

diff --git a/gtk2_ardour/editor_audio_import.cc b/gtk2_ardour/editor_audio_import.cc
index a9e266b66e..acb0b33846 100644
--- a/gtk2_ardour/editor_audio_import.cc
+++ b/gtk2_ardour/editor_audio_import.cc
@@ -925,18 +925,20 @@ Editor::add_sources (vector<string>            paths,
 			PropertyList plist;
 
 			/* Fudge region length to ensure it is non-zero; make it 1 beat at 120bpm
-			   for want of a better idea.  It can't be too small, otherwise if this
-			   is a MIDI region the conversion from samples -> beats -> samples will
-			   round it back down to 0 again.
+			   for want of a better idea.
 			*/
-			timecnt_t len = (*x)->length ();
+			timepos_t len = (*x)->length ();
 			cerr << "for " << (*x)->name() << " source length appears to be " << len << endl;
-			if (len == 0) {
-				len = timecnt_t (_session->sample_rate ()) / 2;
+			if (len.is_zero()) {
+				if ((*x)->type() == DataType::AUDIO) {
+					len = timepos_t (_session->sample_rate ()) / 2;
+				} else {
+					len = timepos_t (Beats (1, 0));
+				}
 				cerr << " reset to use " << len << endl;
 			}
 
-			plist.add (ARDOUR::Properties::start, timecnt_t ((*x)->type() == DataType::AUDIO ? Temporal::AudioTime : Temporal::BeatTime));
+			plist.add (ARDOUR::Properties::start, timepos_t ((*x)->type() == DataType::AUDIO ? Temporal::AudioTime : Temporal::BeatTime));
 			plist.add (ARDOUR::Properties::length, len);
 			plist.add (ARDOUR::Properties::name, region_name);
 			plist.add (ARDOUR::Properties::layer, 0);
diff --git a/libs/ardour/ardour/audiosource.h b/libs/ardour/ardour/audiosource.h
index ad849a0bad..021abfafb8 100644
--- a/libs/ardour/ardour/audiosource.h
+++ b/libs/ardour/ardour/audiosource.h
@@ -51,7 +51,7 @@ class LIBARDOUR_API AudioSource : virtual public Source, public ARDOUR::AudioRea
 	samplecnt_t readable_length_samples() const { return _length.samples(); }
 	virtual uint32_t n_channels()      const { return 1; }
 
-	void       update_length (timecnt_t const & cnt);
+	void       update_length (timepos_t const & dur);
 
 	virtual samplecnt_t available_peaks (double zoom) const;
 
diff --git a/libs/ardour/ardour/region.h b/libs/ardour/ardour/region.h
index 9e6de37a53..92373150a9 100644
--- a/libs/ardour/ardour/region.h
+++ b/libs/ardour/ardour/region.h
@@ -135,7 +135,7 @@ public:
 
 	void set_selected_for_solo(bool yn);
 
-	timecnt_t source_length (uint32_t n) const;
+	timepos_t source_length (uint32_t n) const;
 	uint32_t   max_source_level () const;
 
 	/* these two are valid ONLY during a StateChanged signal handler */
@@ -498,7 +498,6 @@ private:
 	void maybe_uncopy ();
 
 	bool verify_start (timepos_t const &);
-	bool verify_start_mutable (timecnt_t&);
 	bool verify_length (timecnt_t&);
 
 	virtual void recompute_at_start () = 0;
diff --git a/libs/ardour/ardour/smf_source.h b/libs/ardour/ardour/smf_source.h
index a380dfb8d8..a31006b554 100644
--- a/libs/ardour/ardour/smf_source.h
+++ b/libs/ardour/ardour/smf_source.h
@@ -56,7 +56,7 @@ public:
 	void append_event_beats (const Lock& lock, const Evoral::Event<Temporal::Beats>& ev);
 	void append_event_samples (const Lock& lock, const Evoral::Event<samplepos_t>& ev, samplepos_t source_start);
 
-	void update_length (timecnt_t const & cnt);
+	void update_length (timepos_t const & dur);
 
 	void mark_streaming_midi_write_started (const Lock& lock, NoteMode mode);
 	void mark_streaming_write_completed (const Lock& lock);
diff --git a/libs/ardour/ardour/source.h b/libs/ardour/ardour/source.h
index 7859f42ef1..0b6f19cf0b 100644
--- a/libs/ardour/ardour/source.h
+++ b/libs/ardour/ardour/source.h
@@ -75,11 +75,10 @@ public:
 	time_t timestamp() const { return _timestamp; }
 	void stamp (time_t when) { _timestamp = when; }
 
-	virtual timecnt_t length() const { return _length; }
-	samplecnt_t length_samples () const { return _length.samples(); };
+	virtual timepos_t length() const { return _length; }
 
 	virtual bool        empty () const;
-	virtual void        update_length (timecnt_t const & cnt) {}
+	virtual void        update_length (timepos_t const & dur) {}
 
 	void                 set_take_id (std::string id) { _take_id =id; }
 	const std::string&   take_id ()        const { return _take_id; }
@@ -161,7 +160,7 @@ public:
 	uint32_t            _level; /* how deeply nested is this source w.r.t a disk file */
 	std::string         _ancestor_name;
 	std::string        _captured_for;
-	timecnt_t           _length;
+	timepos_t          _length;
 	XrunPositions      _xruns;
 	CueMarkers         _cue_markers;
 
diff --git a/libs/ardour/ardour/srcfilesource.h b/libs/ardour/ardour/srcfilesource.h
index c70dc1b859..9df4327656 100644
--- a/libs/ardour/ardour/srcfilesource.h
+++ b/libs/ardour/ardour/srcfilesource.h
@@ -43,8 +43,8 @@ public:
 	float sample_rate () const { return _session.nominal_sample_rate(); }
 
 	timepos_t natural_position() const { return _source->natural_position() * _ratio;}
-	samplecnt_t readable_length_samples() const { return _source->length_samples () * _ratio; }
-	timecnt_t length () const { return timecnt_t ((samplecnt_t) (_source->length_samples () * _ratio)); }
+	samplecnt_t readable_length_samples() const { assert (_source->length().time_domain() == Temporal::AudioTime); return _source->length().samples () * _ratio; }
+	timepos_t length () const { assert (_source->length().time_domain() == Temporal::AudioTime); return timepos_t ((samplepos_t) (_source->length().samples () * _ratio)); }
 
 	bool can_be_analysed() const { return false; }
 	bool clamped_at_unity() const { return false; }
diff --git a/libs/ardour/audiosource.cc b/libs/ardour/audiosource.cc
index dfc517c5da..b57411ae07 100644
--- a/libs/ardour/audiosource.cc
+++ b/libs/ardour/audiosource.cc
@@ -150,10 +150,14 @@ AudioSource::set_state (const XMLNode& node, int /*version*/)
 }
 
 void
-AudioSource::update_length (timecnt_t const & len)
+AudioSource::update_length (timepos_t const & dur)
 {
-	if (len > _length) {
-		_length = len;
+	assert (_length.time_domain() == dur.time_domain());
+
+	/* audio files cannot get smaller via this mechanism */
+
+	if (dur > _length) {
+		_length = dur;
 	}
 }
 
diff --git a/libs/ardour/import.cc b/libs/ardour/import.cc
index 7d8e5ca3ef..f853e0cd42 100644
--- a/libs/ardour/import.cc
+++ b/libs/ardour/import.cc
@@ -471,7 +471,7 @@ write_midi_data_to_new_files (Evoral::SMF* source, ImportStatus& status,
 
 				/* extend the length of the region to the end of a bar */
 				const Temporal::Beats  length_beats = Temporal::Beats::ticks_at_rate(t, source->ppqn());
-				smfs->update_length (timecnt_t (length_beats.round_up_to_multiple(Temporal::Beats(pulses_per_bar,0)), timepos_t(Temporal::BeatTime)));
+				smfs->update_length (timepos_t (length_beats.round_up_to_multiple(Temporal::Beats(pulses_per_bar,0))));
 
 				smfs->mark_streaming_write_completed (source_lock);
 				smfs->load_model (source_lock, true);
diff --git a/libs/ardour/region.cc b/libs/ardour/region.cc
index 691b343b2c..0e20655c5d 100644
--- a/libs/ardour/region.cc
+++ b/libs/ardour/region.cc
@@ -1347,8 +1347,8 @@ Region::_set_state (const XMLNode& node, int version, PropertyChange& what_chang
 	 */
 
 	if (!_sources.empty() && _type == DataType::AUDIO) {
-		if ((length().time_domain() == Temporal::AudioTime) && (length() > _sources.front()->length())) {
-			_length = _sources.front()->length() - start();
+		if ((length().time_domain() == Temporal::AudioTime) && (length().distance() > _sources.front()->length())) {
+			_length = timecnt_t (start().distance (_sources.front()->length()), _length.val().position());
 		 }
 	}
 
@@ -1659,7 +1659,7 @@ Region::uses_source (boost::shared_ptr<const Source> source, bool shallow) const
 }
 
 
-timecnt_t
+timepos_t
 Region::source_length (uint32_t n) const
 {
 	assert (n < _sources.size());
@@ -1676,10 +1676,12 @@ Region::verify_length (timecnt_t& len)
 	timecnt_t maxlen;
 
 	for (uint32_t n = 0; n < _sources.size(); ++n) {
-		maxlen = max (maxlen, source_length(n) - _start);
+		/* this is computing the distance between _start and the end of the source */
+		timecnt_t max_possible_length = _start.val().distance (source_length(n));
+		maxlen = max (maxlen, max_possible_length);
 	}
 
-	len = min (len, maxlen);
+	len = timecnt_t (min (len, maxlen), len.position());
 
 	return true;
 }
@@ -1694,7 +1696,7 @@ Region::verify_start_and_length (timepos_t const & new_start, timecnt_t& new_len
 	timecnt_t maxlen;
 
 	for (uint32_t n = 0; n < _sources.size(); ++n) {
-		maxlen = max (maxlen, source_length(n) - new_start);
+		maxlen = max (maxlen, new_start.distance (source_length(n)));
 	}
 
 	new_length = min (new_length, maxlen);
@@ -1710,29 +1712,14 @@ Region::verify_start (timepos_t const & pos)
 	}
 
 	for (uint32_t n = 0; n < _sources.size(); ++n) {
-		if (pos > source_length(n) - _length) {
+		/* _start can't be before the start of the region as defined by its length */
+		if (pos > source_length(n).earlier (_length)) {
 			return false;
 		}
 	}
 	return true;
 }
 
-bool
-Region::verify_start_mutable (timecnt_t & new_start)
-{
-	if (source() && source()->length_mutable()) {
-		return true;
-	}
-
-	for (uint32_t n = 0; n < _sources.size(); ++n) {
-		if (new_start > source_length(n) - _length) {
-			new_start = source_length(n) - _length;
-		}
-	}
-
-	return true;
-}
-
 boost::shared_ptr<Region>
 Region::get_parent() const
 {
@@ -1977,7 +1964,7 @@ Region::latest_possible_sample () const
 		/* non-audio regions have a length that may vary based on their
 		 * position, so we have to pass it in the call.
 		 */
-		minlen = min (minlen, (*i)->length ());
+		minlen = min (minlen, timecnt_t ((*i)->length (), (*i)->natural_position()));
 	}
 
 	/* the latest possible last sample is determined by the current
diff --git a/libs/ardour/smf_source.cc b/libs/ardour/smf_source.cc
index ea74f71a83..8272329e88 100644
--- a/libs/ardour/smf_source.cc
+++ b/libs/ardour/smf_source.cc
@@ -395,9 +395,10 @@ SMFSource::write_unlocked (const Lock&                  lock,
 }
 
 void
-SMFSource::update_length (timecnt_t const & cnt)
+SMFSource::update_length (timepos_t const & dur)
 {
-	_length = cnt;
+	assert (!_length || (_length.time_domain() == dur.time_domain()));
+	_length = dur;
 }
 
 /** Append an event with a timestamp in beats */
@@ -445,7 +446,8 @@ SMFSource::append_event_beats (const Glib::Threads::Mutex::Lock&   lock,
 		_model->append (ev, event_id);
 	}
 
-	_length  = max (_length, timecnt_t (time));
+	assert (!_length || (_length.time_domain() == Temporal::BeatTime));
+	_length  = timepos_t (max (_length.beats(), time));
 
 	const Temporal::Beats delta_time_beats = time - _last_ev_time_beats;
 	const uint32_t      delta_time_ticks = delta_time_beats.to_ticks(ppqn());
@@ -498,7 +500,8 @@ SMFSource::append_event_samples (const Glib::Threads::Mutex::Lock& lock,
 		_model->append (beat_ev, event_id);
 	}
 
-	_length = max (_length, timecnt_t (ev_time_beats, timepos_t (position)));
+	assert (!_length || (_length.time_domain() == Temporal::BeatTime));
+	_length = timepos_t (max (_length.beats(), ev_time_beats));
 
 	/* a distance measure that starts at @param _last_ev_time_samples (audio time) and
 	   extends for ev.time() (audio time)
@@ -718,7 +721,8 @@ SMFSource::load_model (const Glib::Threads::Mutex::Lock& lock, bool force_reload
 				scratch_size = std::max(size, scratch_size);
 				size = scratch_size;
 
-				_length = max (_length, timecnt_t (event_time));
+				assert (!_length || (_length.time_domain() == Temporal::BeatTime));
+				_length = max (_length, timepos_t (event_time));
 			}
 
 			/* event ID's must immediately precede the event they are for */
diff --git a/libs/ardour/sndfilesource.cc b/libs/ardour/sndfilesource.cc
index f731ba8926..d13a13e5df 100644
--- a/libs/ardour/sndfilesource.cc
+++ b/libs/ardour/sndfilesource.cc
@@ -605,13 +605,15 @@ SndFileSource::nondestructive_write_unlocked (Sample *data, samplecnt_t cnt)
 		return 0;
 	}
 
+	assert (_length.time_domain() == Temporal::AudioTime);
 	samplepos_t sample_pos = _length.samples();
 
 	if (write_float (data, sample_pos, cnt) != cnt) {
 		return 0;
 	}
 
-	update_length (_length + timecnt_t (cnt, timepos_t (Temporal::AudioTime)));
+	assert (_length.time_domain() == Temporal::AudioTime);
+	update_length (timepos_t (_length.samples() + cnt));
 
 	if (_build_peakfiles) {
 		compute_and_write_peaks (data, sample_pos, cnt, true, true);
diff --git a/libs/ardour/source.cc b/libs/ardour/source.cc
index 2b00ada40a..098b3c3415 100644
--- a/libs/ardour/source.cc
+++ b/libs/ardour/source.cc
@@ -419,7 +419,6 @@ Source::set_natural_position (timepos_t const & pos)
 {
 	_natural_position = pos;
 	_have_natural_position = true;
-	_length.set_position (pos);
 }
 
 void
@@ -533,7 +532,7 @@ Source::clear_cue_markers ()
 bool
 Source::empty () const
 {
-	return _length == timecnt_t();
+	return _length == timepos_t ();
 }
 
 bool
diff --git a/libs/temporal/temporal/timeline.h b/libs/temporal/temporal/timeline.h
index 36272e5954..a2edeaae2b 100644
--- a/libs/temporal/temporal/timeline.h
+++ b/libs/temporal/temporal/timeline.h
@@ -74,6 +74,7 @@ class LIBTEMPORAL_API timepos_t : public int62_t  {
 	bool is_positive () const { return val() > 0; }
 	bool is_negative () const { return val() < 0; }
 	bool is_zero ()     const { return val() == 0; }
+	bool operator! ()   const { return val() == 0; }
 
 	Temporal::TimeDomain time_domain () const { if (flagged()) return Temporal::BeatTime; return Temporal::AudioTime; }
 	void set_time_domain (Temporal::TimeDomain);
@@ -84,6 +85,8 @@ class LIBTEMPORAL_API timepos_t : public int62_t  {
 	Beats        beats() const { if (is_beats()) return Beats::ticks (val()); return _beats (); }
 
 	timepos_t & operator= (timecnt_t const & t); /* will throw() if val is negative */
+	timepos_t & operator= (Beats const & b) { v.store (build (true, b.to_ticks())); return *this; }
+	timepos_t & operator= (samplepos_t const & s) { v.store (build (false, samples_to_superclock (s, TEMPORAL_SAMPLE_RATE))); return *this; }
 
 	timepos_t operator-() const { return timepos_t (int62_t::operator-()); }