Update WritePass logic + AutomationList Undo

Fixes various issues when changing AutomationState while rolling.

4 files changed, 71 insertions, 34 deletions

diff --git a/libs/ardour/ardour/automation_list.h b/libs/ardour/ardour/automation_list.h
index 65268d503a..46643dc6fa 100644
--- a/libs/ardour/ardour/automation_list.h
+++ b/libs/ardour/ardour/automation_list.h
@@ -110,8 +110,6 @@ public:
 
 	XMLNode& get_state ();
 	int set_state (const XMLNode &, int version);
-	XMLNode& state (bool full);
-	XMLNode& serialize_events ();
 
 	Command* memento_command (XMLNode* before, XMLNode* after);
 
@@ -119,6 +117,7 @@ public:
 
 	XMLNode* before () { XMLNode* rv = _before; _before = 0; return rv; }
 	void clear_history ();
+	void snapshot_history (bool need_lock);
 
 	ControlList::InterpolationStyle default_interpolation () const;
 
@@ -126,11 +125,16 @@ private:
 	void create_curve_if_necessary ();
 	int deserialize_events (const XMLNode&);
 
+	XMLNode& state (bool full, bool need_lock);
+	XMLNode& serialize_events (bool need_lock);
+
 	void maybe_signal_changed ();
 
 	AutoState    _state;
 	gint         _touching;
 
+	PBD::ScopedConnection _writepass_connection;
+
 	bool operator== (const AutomationList&) const { /* not called */ abort(); return false; }
 	XMLNode* _before; //used for undo of touch start/stop pairs.
 
diff --git a/libs/ardour/automation_list.cc b/libs/ardour/automation_list.cc
index 74181d5514..d92e65288f 100644
--- a/libs/ardour/automation_list.cc
+++ b/libs/ardour/automation_list.cc
@@ -161,6 +161,8 @@ AutomationList::create_curve_if_necessary()
 	default:
 		break;
 	}
+
+	WritePassStarted.connect_same_thread (_writepass_connection, boost::bind (&AutomationList::snapshot_history, this, false));
 }
 
 AutomationList&
@@ -193,16 +195,14 @@ AutomationList::maybe_signal_changed ()
 void
 AutomationList::set_automation_state (AutoState s)
 {
-	if (s != _state) {
-		_state = s;
-		delete _before;
-		if (s == Write && _desc.toggled) {
-			_before = &get_state ();
-		} else {
-			_before = 0;
-		}
-		automation_state_changed (s); /* EMIT SIGNAL */
+	if (s == _state) {
+		return;
+	}
+	_state = s;
+	if (s == Write && _desc.toggled) {
+		snapshot_history (true);
 	}
+	automation_state_changed (s); /* EMIT SIGNAL */
 }
 
 Evoral::ControlList::InterpolationStyle
@@ -231,12 +231,7 @@ AutomationList::default_interpolation () const
 void
 AutomationList::start_write_pass (double when)
 {
-	delete _before;
-	if (in_new_write_pass ()) {
-		_before = &get_state ();
-	} else {
-		_before = 0;
-	}
+	snapshot_history (true);
 	ControlList::start_write_pass (when);
 }
 
@@ -249,9 +244,9 @@ AutomationList::write_pass_finished (double when, double thinning_factor)
 void
 AutomationList::start_touch (double when)
 {
-        if (_state == Touch) {
+	if (_state == Touch) {
 		start_write_pass (when);
-        }
+	}
 
 	g_atomic_int_set (&_touching, 1);
 }
@@ -282,6 +277,17 @@ AutomationList::clear_history ()
 }
 
 void
+AutomationList::snapshot_history (bool need_lock)
+{
+	if (!in_new_write_pass ()) {
+		return;
+	}
+	delete _before;
+	_before = &state (true, need_lock);
+}
+
+
+void
 AutomationList::thaw ()
 {
 	ControlList::thaw();
@@ -326,11 +332,11 @@ AutomationList::memento_command (XMLNode* before, XMLNode* after)
 XMLNode&
 AutomationList::get_state ()
 {
-	return state (true);
+	return state (true, true);
 }
 
 XMLNode&
-AutomationList::state (bool full)
+AutomationList::state (bool full, bool need_lock)
 {
 	XMLNode* root = new XMLNode (X_("AutomationList"));
 
@@ -372,18 +378,22 @@ AutomationList::state (bool full)
 	}
 
 	if (!_events.empty()) {
-		root->add_child_nocopy (serialize_events());
+		root->add_child_nocopy (serialize_events (need_lock));
 	}
 
 	return *root;
 }
 
 XMLNode&
-AutomationList::serialize_events ()
+AutomationList::serialize_events (bool need_lock)
 {
 	XMLNode* node = new XMLNode (X_("events"));
 	stringstream str;
 
+	Glib::Threads::RWLock::ReaderLock lm (Evoral::ControlList::_lock, Glib::Threads::NOT_LOCK);
+	if (need_lock) {
+		lm.acquire ();
+	}
 	for (iterator xx = _events.begin(); xx != _events.end(); ++xx) {
 		str << PBD::to_string ((*xx)->when);
 		str << ' ';
diff --git a/libs/evoral/evoral/ControlList.hpp b/libs/evoral/evoral/ControlList.hpp
index 4aececf40d..3d70fbdda4 100644
--- a/libs/evoral/evoral/ControlList.hpp
+++ b/libs/evoral/evoral/ControlList.hpp
@@ -307,6 +307,7 @@ public:
 	bool in_write_pass () const;
 	bool in_new_write_pass () { return new_write_pass; }
 
+	PBD::Signal0<void> WritePassStarted;
 	/** Emitted when mark_dirty() is called on this object */
 	mutable PBD::Signal0<void> Dirty;
 	/** Emitted when our interpolation style changes */
@@ -359,6 +360,8 @@ private:
 	void unlocked_remove_duplicates ();
 	void unlocked_invalidate_insert_iterator ();
 	void add_guard_point (double when, double offset);
+
+	bool is_sorted () const;
 };
 
 } // namespace Evoral
diff --git a/libs/evoral/src/ControlList.cpp b/libs/evoral/src/ControlList.cpp
index 0bba1dba11..801d8562c4 100644
--- a/libs/evoral/src/ControlList.cpp
+++ b/libs/evoral/src/ControlList.cpp
@@ -119,7 +119,7 @@ ControlList::ControlList (const ControlList& other, double start, double end)
 		copy_events (*(section.get()));
 	}
 
-	new_write_pass = false;
+	new_write_pass = true;
 	_in_write_pass = false;
 	did_write_during_pass = false;
 	insert_position = -1;
@@ -346,6 +346,8 @@ ControlList::thin (double thinning_factor)
 		return;
 	}
 
+	assert (is_sorted ());
+
 	bool changed = false;
 
 	{
@@ -457,8 +459,6 @@ ControlList::start_write_pass (double when)
 
 	DEBUG_TRACE (DEBUG::ControlList, string_compose ("%1: setup write pass @ %2\n", this, when));
 
-	new_write_pass = true;
-	did_write_during_pass = false;
 	insert_position = when;
 
 	/* leave the insert iterator invalid, so that we will do the lookup
@@ -519,12 +519,20 @@ ControlList::add_guard_point (double when, double offset)
 		}
 	}
 
+	/* don't do this again till the next write pass,
+	 * unless we're not in a write-pass (transport stopped)
+	 */
+	if (_in_write_pass && new_write_pass) {
+		WritePassStarted (); /* EMIT SIGNAL w/WriteLock */
+		new_write_pass = false;
+	}
+
 	when += offset;
 
 	ControlEvent cp (when, 0.0);
 	most_recent_insert_iterator = lower_bound (_events.begin(), _events.end(), &cp, time_comparator);
 
-	double eval_value = unlocked_eval (insert_position);
+	double eval_value = unlocked_eval (when);
 
 	if (most_recent_insert_iterator == _events.end()) {
 
@@ -564,13 +572,6 @@ ControlList::add_guard_point (double when, double offset)
 
 		++most_recent_insert_iterator;
 	}
-
-	/* don't do this again till the next write pass,
-	 * unless we're not in a write-pass (transport stopped)
-	 */
-	if (_in_write_pass) {
-		new_write_pass = false;
-	}
 }
 
 bool
@@ -745,6 +746,7 @@ ControlList::add (double when, double value, bool with_guards, bool with_initial
 				const ControlEvent cp (when, 0.0);
 				most_recent_insert_iterator = lower_bound (_events.begin(), _events.end(), &cp, time_comparator);
 			}
+			WritePassStarted (); /* EMIT SIGNAL w/WriteLock */
 			new_write_pass = false;
 
 		} else if (_in_write_pass &&
@@ -1983,6 +1985,24 @@ ControlList::operator!= (ControlList const & other) const
 		);
 }
 
+bool
+ControlList::is_sorted () const
+{
+	Glib::Threads::RWLock::ReaderLock lm (_lock);
+	if (_events.size () == 0) {
+		return true;
+	}
+	const_iterator i = _events.begin();
+	const_iterator n = i;
+	while (++n != _events.end ()) {
+		if (event_time_less_than(*n,*i)) {
+			return false;
+		}
+		++i;
+	}
+	return true;
+}
+
 void
 ControlList::dump (ostream& o)
 {