likely fixes for most remaining issues with data in automation/control lists, but more testing needed

git-svn-id: svn://localhost/ardour2/branches/3.0@13497 d708f5d6-7413-0410-9779-e7cbd77b26cf

3 files changed, 54 insertions, 91 deletions

diff --git a/libs/ardour/ardour/automation_watch.h b/libs/ardour/ardour/automation_watch.h
index 6fd5a0eb65..f3e343468d 100644
--- a/libs/ardour/ardour/automation_watch.h
+++ b/libs/ardour/ardour/automation_watch.h
@@ -17,7 +17,7 @@
 
 */
 
-#include <list>
+#include <set>
 #include <boost/shared_ptr.hpp>
 #include <glibmm/threads.h>
 #include <sigc++/signal.h>
@@ -41,7 +41,7 @@ class AutomationWatch : public sigc::trackable, public ARDOUR::SessionHandlePtr,
     gint timer ();
 
   private:
-    typedef std::list<boost::shared_ptr<ARDOUR::AutomationControl> > AutomationWatches;
+    typedef std::set<boost::shared_ptr<ARDOUR::AutomationControl> > AutomationWatches;
 
     AutomationWatch ();
     ~AutomationWatch();
@@ -50,7 +50,7 @@ class AutomationWatch : public sigc::trackable, public ARDOUR::SessionHandlePtr,
     Glib::Threads::Thread*  _thread;
     bool                    _run_thread;
     AutomationWatches        automation_watches;
-    Glib::Threads::Mutex              automation_watch_lock;
+    Glib::Threads::Mutex     automation_watch_lock;
     PBD::ScopedConnection    transport_connection;
 
     void transport_state_change ();
diff --git a/libs/ardour/automation_watch.cc b/libs/ardour/automation_watch.cc
index d3138ee336..7f67bb2c03 100644
--- a/libs/ardour/automation_watch.cc
+++ b/libs/ardour/automation_watch.cc
@@ -28,8 +28,6 @@
 
 using namespace ARDOUR;
 using namespace PBD;
-using std::cerr;
-using std::endl;
 
 AutomationWatch* AutomationWatch::_instance = 0;
 
@@ -65,8 +63,8 @@ void
 AutomationWatch::add_automation_watch (boost::shared_ptr<AutomationControl> ac)
 {
 	Glib::Threads::Mutex::Lock lm (automation_watch_lock);
-	DEBUG_TRACE (DEBUG::Automation, string_compose ("now watching control %1 for automation\n", ac->name()));
-	automation_watches.push_back (ac);
+	DEBUG_TRACE (DEBUG::Automation, string_compose ("now watching control %1 for automation, astate = %2\n", ac->name(), enum_2_string (ac->automation_state())));
+	automation_watches.insert (ac);
 
 	/* if an automation control is added here while the transport is
 	 * rolling, make sure that it knows that there is a write pass going
@@ -104,7 +102,7 @@ AutomationWatch::remove_automation_watch (boost::shared_ptr<AutomationControl> a
 {
 	Glib::Threads::Mutex::Lock lm (automation_watch_lock);
 	DEBUG_TRACE (DEBUG::Automation, string_compose ("remove control %1 from automation watch\n", ac->name()));
-	automation_watches.remove (ac);
+	automation_watches.erase (ac);
 	ac->list()->set_in_write_pass (false);
 }
 
@@ -117,7 +115,7 @@ AutomationWatch::timer ()
 
 	{
 		Glib::Threads::Mutex::Lock lm (automation_watch_lock);
-		
+
 		framepos_t time = _session->audible_frame ();
 
 		for (AutomationWatches::iterator aw = automation_watches.begin(); aw != automation_watches.end(); ++aw) {
diff --git a/libs/evoral/src/ControlList.cpp b/libs/evoral/src/ControlList.cpp
index d2ef1fe246..31811e2fc9 100644
--- a/libs/evoral/src/ControlList.cpp
+++ b/libs/evoral/src/ControlList.cpp
@@ -390,7 +390,6 @@ ControlList::add (double when, double value)
 
 	DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 add %2 at %3 w/erase = %4 at end ? %5\n", 
 							 this, value, when, _in_write_pass, (most_recent_insert_iterator == _events.end())));
-
 	{
 		Glib::Threads::Mutex::Lock lm (_lock);
 		ControlEvent cp (when, 0.0f);
@@ -416,7 +415,7 @@ ControlList::add (double when, double value)
 			 * write pass.
 			 *
 			 * We need to add a new point at insert_position
-			 * corresponding the value there. 
+			 * corresponding to the (existing, implicit) value there. 
 			 */
 
 			/* the insert_iterator is not set, figure out where
@@ -475,84 +474,31 @@ ControlList::add (double when, double value)
 			did_write_during_pass = true;
 
 		} else if (most_recent_insert_iterator == _events.end() || when > (*most_recent_insert_iterator)->when) {
-
-			/* this is NOT the first point to be added after the
-			   start of a write pass, and we have a bit of work to
-			   do figuring out where to add the new point, as well
-			   as potentially erasing existing data between the
-			   most recently added point and wherever this one
-			   will end up.
-			*/
-				
-			DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 need to discover insert iterator (@end ? %2)\n", 
-									 this, (most_recent_insert_iterator == _events.end())));
-
-			/* this means that we either *know* we want to insert
-			 * at the end, or that we don't know where to insert.
-			 * 
-			 * so ... lets perform some quick checks before we
-			 * go doing binary search to figure out where to
-			 * insert.
-			 */
-
-			if (_events.back()->when == when) {
-
-				/* we need to modify the final point, so 
-				   make most_recent_insert_iterator point to it.
-				*/
-
-				DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 modify final value\n", this));
-				
-				most_recent_insert_iterator = _events.end();
-				--most_recent_insert_iterator;
-
-			} else if (_events.back()->when < when) {
-
-				/* the new point is beyond the end of the
-				 * current list
-				 */
-
-				DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 plan to append to list\n", this));
-
-				if (_in_write_pass) {
-					/* remove the final point, because
-					   we're adding one beyond it.
-					*/
-					delete _events.back();
-					_events.pop_back();
-				}
-				
-				/* leaving this here will force an append */
-
-				most_recent_insert_iterator = _events.end();
-
-			} else {
-
-				DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 erase %2 from existing iterator (@end ? %3\n", 
-										 this, _in_write_pass,
-										 (most_recent_insert_iterator == _events.end())));
-
-				if (_in_write_pass) {
-					while (most_recent_insert_iterator != _events.end()) {
-						if ((*most_recent_insert_iterator)->when < when) {
-							if (_in_write_pass) {
-								DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 erase existing @ %2\n", this, (*most_recent_insert_iterator)));
-								delete *most_recent_insert_iterator;
-								most_recent_insert_iterator = _events.erase (most_recent_insert_iterator);
-								continue;
-							}
-						} else if ((*most_recent_insert_iterator)->when >= when) {
-							break;
+			
+			DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 %2 erase from existing iterator (@end ? %3)\n", 
+									 this, (_in_write_pass  ? "DO" : "DON'T"),
+									 (most_recent_insert_iterator == _events.end())));
+			
+			if (_in_write_pass) {
+				while (most_recent_insert_iterator != _events.end()) {
+					if ((*most_recent_insert_iterator)->when < when) {
+						if (_in_write_pass) {
+							DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 erase existing @ %2\n", this, (*most_recent_insert_iterator)));
+							delete *most_recent_insert_iterator;
+							most_recent_insert_iterator = _events.erase (most_recent_insert_iterator);
+							continue;
 						}
-						++most_recent_insert_iterator;
+					} else if ((*most_recent_insert_iterator)->when >= when) {
+						break;
 					}
-				} else {
-					
-					/* not in a write pass: figure out the iterator we should insert in front of */
-
-					ControlEvent cp (when, 0.0f);
-					most_recent_insert_iterator = lower_bound (_events.begin(), _events.end(), &cp, time_comparator);
+					++most_recent_insert_iterator;
 				}
+			} else {
+				
+				/* not in a write pass: figure out the iterator we should insert in front of */
+				
+				ControlEvent cp (when, 0.0f);
+				most_recent_insert_iterator = lower_bound (_events.begin(), _events.end(), &cp, time_comparator);
 			}
 		} 
 		
@@ -581,6 +527,7 @@ ControlList::add (double when, double value)
 							 */
 							_events.back()->when = when;
 							done = true;
+							DEBUG_TRACE (DEBUG::ControlList, string_compose ("final value of %1 moved to %2\n", value, when));
 						}
 					}
 				}
@@ -588,6 +535,7 @@ ControlList::add (double when, double value)
 
 			if (!done) {
 				_events.push_back (new ControlEvent (when, value));
+				DEBUG_TRACE (DEBUG::ControlList, string_compose ("\tactually appended, size now %1\n", _events.size()));
 			}
 
 			if (!_in_write_pass) {
@@ -596,13 +544,28 @@ ControlList::add (double when, double value)
 			}
 
 		} else if ((*most_recent_insert_iterator)->when == when) {
-			DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 reset existing point to new value %2\n", this, value));
 
-			/* only one point allowed per time point, so just
-			 * reset the value here.
-			 */
+			if ((*most_recent_insert_iterator)->value != value) {
+				DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 reset existing point to new value %2\n", this, value));
+
+				/* only one point allowed per time point, so just
+				 * reset the value here.
+				 */
+				
+				(*most_recent_insert_iterator)->value = value;
 
-			(*most_recent_insert_iterator)->value = value;
+				/* if we modified the final value, then its as
+				 * if we inserted a new point as far as the
+				 * next addition, so make sure we know that.
+				 */
+
+				if (_in_write_pass && _events.back()->when == when) {
+					most_recent_insert_iterator = _events.end();
+				}
+
+			} else {
+				DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 same time %2, same value value %3\n", this, when, value));
+			}
 
 		} else {
 			DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 insert new point at %2 at iterator at %3\n", this, when, (*most_recent_insert_iterator)->when));
@@ -630,6 +593,7 @@ ControlList::add (double when, double value)
 								most_recent_insert_iterator = b;
 							}
 
+							DEBUG_TRACE (DEBUG::ControlList, string_compose ("final value of %1 moved to %2\n", value, when));
 							done = true;
 						}
 					}
@@ -638,6 +602,7 @@ ControlList::add (double when, double value)
 			
 			if (!done) {
 				EventList::iterator x = _events.insert (most_recent_insert_iterator, new ControlEvent (when, value));
+				DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 inserted new value before MRI, size now %2\n", this, _events.size()));
 
 				if (!_in_write_pass) {
 					most_recent_insert_iterator = x;