lots more fidgety work on automation. sort of works now, but undo/redo needs attention

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

gtk2_ardour/automation_time_axis.cc

@@ -565,6 +565,15 @@ AutomationTimeAxisView::add_automation_event (GdkEvent* event, framepos_t when,
 		return;
 	}
 
+	boost::shared_ptr<AutomationList> list = _line->the_list ();
+	
+	if (list->in_write_pass()) {
+		/* do not allow the GUI to add automation events during an
+		   automation write pass.
+		*/
+		return;
+	}
+
 	double x = 0;
 
 	_canvas_display->w2i (x, y);
@@ -577,7 +586,6 @@ AutomationTimeAxisView::add_automation_event (GdkEvent* event, framepos_t when,
 
 	_line->view_to_model_coord (x, y);
 
-	boost::shared_ptr<AutomationList> list = _line->the_list ();
 
 	_editor.snap_to_with_modifier (when, event);
 

libs/ardour/ardour/automation_watch.h

@@ -51,7 +51,9 @@ class AutomationWatch : public sigc::trackable, public ARDOUR::SessionHandlePtr,
     bool                    _run_thread;
     AutomationWatches        automation_watches;
     Glib::Mutex              automation_watch_lock;
+    PBD::ScopedConnection    transport_connection;
 
+    void transport_state_change ();
     void remove_weak_automation_watch (boost::weak_ptr<ARDOUR::AutomationControl>);
     void thread ();
 };

libs/ardour/automation_control.cc

@@ -60,7 +60,6 @@ void
 AutomationControl::set_value (double value)
 {
 	bool to_list = _list && ((AutomationList*)_list.get())->automation_write();
-	bool erase_since_last = _session.transport_rolling();
 
         if (to_list && parameter().toggled()) {
 
@@ -68,15 +67,14 @@ AutomationControl::set_value (double value)
                 // interpolation works right
 
 
-                _list->add (get_double(), _session.transport_frame()-1, erase_since_last);
+                _list->add (get_double(), _session.transport_frame()-1);
         }
 
-	Control::set_double (value, _session.transport_frame(), to_list, erase_since_last);
+	Control::set_double (value, _session.transport_frame(), to_list);
 
 	Changed(); /* EMIT SIGNAL */
 }
 
-
 void
 AutomationControl::set_list (boost::shared_ptr<Evoral::ControlList> list)
 {
@@ -91,17 +89,24 @@ AutomationControl::set_automation_state (AutoState as)
 
 		cerr << name() << " setting automation state to " << enum_2_string (as) << endl;
 
+		alist()->set_automation_state (as);
+
 		if (as == Write) {
 			AutomationWatch::instance().add_automation_watch (shared_from_this());
 		} else if (as == Touch) {
 			if (!touching()) {
 				AutomationWatch::instance().remove_automation_watch (shared_from_this());
+			} else {
+				/* this seems unlikely, but the combination of
+				 * a control surface and the mouse could make
+				 * it possible to put the control into Touch
+				 * mode *while* touching it.
+				 */
+				AutomationWatch::instance().add_automation_watch (shared_from_this());
 			}
 		} else {
 			AutomationWatch::instance().remove_automation_watch (shared_from_this());
 		}
-		
-		alist()->set_automation_state (as);
 	}
 }
 

libs/ardour/automation_watch.cc

@@ -68,6 +68,16 @@ AutomationWatch::add_automation_watch (boost::shared_ptr<AutomationControl> ac)
 	DEBUG_TRACE (DEBUG::Automation, string_compose ("now watching control %1 for automation\n", ac->name()));
 	automation_watches.push_back (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
+	 * on, rather than waiting for the transport to start.
+	 */
+
+	if (_session && _session->transport_rolling() && ac->alist()->automation_write()) {
+		DEBUG_TRACE (DEBUG::Automation, string_compose ("\ttransport is rolling @ %1, so enter write pass\n", _session->transport_speed()));
+		ac->list()->set_in_write_pass (true);
+	}
+
 	/* we can't store shared_ptr<Destructible> in connections because it
 	 * creates reference cycles. we don't need to make the weak_ptr<>
 	 * explicit here, but it helps to remind us what is going on.
@@ -95,6 +105,7 @@ AutomationWatch::remove_automation_watch (boost::shared_ptr<AutomationControl> a
 	Glib::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);
+	ac->list()->set_in_write_pass (false);
 }
 
 gint
@@ -110,7 +121,9 @@ AutomationWatch::timer ()
 		framepos_t time = _session->audible_frame ();
 
 		for (AutomationWatches::iterator aw = automation_watches.begin(); aw != automation_watches.end(); ++aw) {
-			(*aw)->list()->add (time, (*aw)->user_double(), true);
+			if ((*aw)->alist()->automation_write()) {
+				(*aw)->list()->add (time, (*aw)->user_double());
+			}
 		}
 	}
 
@@ -129,6 +142,8 @@ AutomationWatch::thread ()
 void
 AutomationWatch::set_session (Session* s)
 {
+	transport_connection.disconnect ();
+
 	if (_thread) {
 		_run_thread = false;
 		_thread->join ();
@@ -142,5 +157,31 @@ AutomationWatch::set_session (Session* s)
 		_thread = Glib::Thread::create (boost::bind (&AutomationWatch::thread, this),
 						500000, true, true, Glib::THREAD_PRIORITY_NORMAL);
 		
+		_session->TransportStateChange.connect_same_thread (transport_connection, boost::bind (&AutomationWatch::transport_state_change, this));
+	}
+}
+
+void
+AutomationWatch::transport_state_change ()
+{
+	if (!_session) {
+		return;
+	}
+
+	bool rolling = _session->transport_rolling();
+
+	{
+		Glib::Mutex::Lock lm (automation_watch_lock);
+
+		for (AutomationWatches::iterator aw = automation_watches.begin(); aw != automation_watches.end(); ++aw) {
+			DEBUG_TRACE (DEBUG::Automation, string_compose ("%1: transport state changed, speed %2, in write pass ? %3 writing ? %4\n", 
+									(*aw)->name(), _session->transport_speed(), rolling,
+									(*aw)->alist()->automation_write()));
+			if (rolling && (*aw)->alist()->automation_write()) {
+				(*aw)->list()->set_in_write_pass (true);
+			} else {
+				(*aw)->list()->set_in_write_pass (false);
+			}
+		}
 	}
 }

libs/ardour/midi_source.cc

@@ -300,8 +300,6 @@ MidiSource::mark_write_starting_now ()
 
 	set_timeline_position (_session.transport_frame ());
 	_last_write_end = _session.transport_frame ();
-	cerr << name() << " last write set to " << _last_write_end << endl;
-
 }
 
 void
@@ -388,7 +386,7 @@ MidiSource::session_saved()
 	*/
 
 	if (_model && _model->edited()) {
-
+		
 		// if the model is edited, write its contents into
 		// the current source file (overwiting previous contents.
 

libs/ardour/session_transport.cc

@@ -638,6 +638,7 @@ Session::non_realtime_stop (bool abort, int on_entry, bool& finished)
 	}
 
 	PositionChanged (_transport_frame); /* EMIT SIGNAL */
+	DEBUG_TRACE (DEBUG::Transport, string_compose ("send TSC with speed = %1\n", _transport_speed));
 	TransportStateChange (); /* EMIT SIGNAL */
 
 	/* and start it up again if relevant */
@@ -766,6 +767,7 @@ Session::set_play_loop (bool yn)
 		unset_play_loop ();
 	}
 
+	DEBUG_TRACE (DEBUG::Transport, string_compose ("send TSC2 with speed = %1\n", _transport_speed));
 	TransportStateChange ();
 }
 void
@@ -1117,6 +1119,7 @@ Session::set_transport_speed (double speed, bool abort, bool clear_state, bool a
 			_butler->schedule_transport_work ();
 		}
 
+		DEBUG_TRACE (DEBUG::Transport, string_compose ("send TSC3 with speed = %1\n", _transport_speed));
 		TransportStateChange (); /* EMIT SIGNAL */
 	}
 }
@@ -1238,6 +1241,7 @@ Session::start_transport ()
 		}
 	}
 
+	DEBUG_TRACE (DEBUG::Transport, string_compose ("send TSC4 with speed = %1\n", _transport_speed));
 	TransportStateChange (); /* EMIT SIGNAL */
 }
 
@@ -1486,6 +1490,7 @@ Session::set_play_range (list<AudioRange>& range, bool leave_rolling)
 	ev = new SessionEvent (SessionEvent::LocateRoll, SessionEvent::Add, SessionEvent::Immediate, range.front().start, 0.0f, false);
 	merge_event (ev);
 
+	DEBUG_TRACE (DEBUG::Transport, string_compose ("send TSC5 with speed = %1\n", _transport_speed));
 	TransportStateChange ();
 }
 
@@ -1528,6 +1533,7 @@ Session::engine_halted ()
 	non_realtime_stop (false, 0, ignored);
 	transport_sub_state = 0;
 
+	DEBUG_TRACE (DEBUG::Transport, string_compose ("send TSC6 with speed = %1\n", _transport_speed));
 	TransportStateChange (); /* EMIT SIGNAL */
 }
 

libs/ardour/smf_source.cc

@@ -281,7 +281,7 @@ SMFSource::write_unlocked (MidiRingBuffer<framepos_t>& source, framepos_t positi
 		append_event_unlocked_frames(ev, position);
 	}
 
-	Evoral::SMF::flush();
+	Evoral::SMF::flush ();
 	free (buf);
 
 	return duration;
@@ -338,9 +338,9 @@ SMFSource::append_event_unlocked_frames (const Evoral::Event<framepos_t>& ev, fr
 		return;
 	}
 
-	/* printf("SMFSource: %s - append_event_unlocked_frames ID = %d time = %u, size = %u, data = ",
+	// printf("SMFSource: %s - append_event_unlocked_frames ID = %d time = %u, size = %u, data = ",
-               name().c_str(), ev.id(), ev.time(), ev.size());
+	// name().c_str(), ev.id(), ev.time(), ev.size());
-	   for (size_t i=0; i < ev.size(); ++i) printf("%X ", ev.buffer()[i]); printf("\n");*/
+	// for (size_t i=0; i < ev.size(); ++i) printf("%X ", ev.buffer()[i]); printf("\n");
 
 	if (ev.time() < _last_ev_time_frames) {
 		cerr << "SMFSource: Warning: Skipping event with non-monotonic time" << endl;
@@ -431,6 +431,7 @@ SMFSource::mark_midi_streaming_write_completed (Evoral::Sequence<Evoral::Musical
 	MidiSource::mark_midi_streaming_write_completed (stuck_notes_option, when);
 
 	if (!writable()) {
+		warning << string_compose ("attempt to write to unwritable SMF file %1", _path) << endmsg;
 		return;
 	}
 

libs/evoral/evoral/Control.hpp

@@ -44,7 +44,7 @@ public:
 	Control(const Parameter& parameter, boost::shared_ptr<ControlList>);
 	virtual ~Control() {}
 
-        virtual void   set_double (double val, double frame=0, bool to_list=false, bool erase_since_last=false);
+        virtual void   set_double (double val, double frame=0, bool to_list=false);
 	virtual double get_double (bool from_list=false, double frame=0) const;
 
 	/** Get the latest user-set value

libs/evoral/evoral/ControlList.hpp

@@ -124,7 +124,7 @@ public:
 
 	virtual bool clamp_value (double& /*when*/, double& /*value*/) const { return true; }
 
-        void add (double when, double value, bool erase_since_last_add = false);
+        virtual void add (double when, double value);
 	void fast_simple_add (double when, double value);
 
 	void erase_range (double start, double end);
@@ -245,6 +245,8 @@ public:
 	virtual bool touch_enabled() const { return false; }
         void start_write_pass (double time);
 	void write_pass_finished (double when);
+        void set_in_write_pass (bool);
+        bool in_write_pass () const;
 
 	/** Emitted when mark_dirty() is called on this object */
 	mutable PBD::Signal0<void> Dirty;
@@ -291,10 +293,11 @@ protected:
         static double _thinning_factor;
 
   private:
-    iterator   insert_iterator;
+    iterator   most_recent_insert_iterator;
     double     insert_position;
     bool       new_write_pass;
     bool       did_write_during_pass;
+    bool       _in_write_pass;
     void unlocked_invalidate_insert_iterator ();
 };
 

libs/evoral/src/Control.cpp

@@ -46,12 +46,12 @@ Control::get_double (bool from_list, double frame) const
 
 
 void
-Control::set_double (double value, double frame, bool to_list, bool erase_since_last)
+Control::set_double (double value, double frame, bool to_list)
 {
 	_user_value = value;
 
 	if (to_list) {
-		_list->add (frame, value, erase_since_last);
+		_list->add (frame, value);
 	}
 }
 

libs/evoral/src/ControlList.cpp

@@ -67,9 +67,10 @@ ControlList::ControlList (const Parameter& id)
 	_search_cache.first = _events.end();
 	_sort_pending = false;
 	new_write_pass = true;
+	_in_write_pass = false;
 	did_write_during_pass = false;
 	insert_position = -1;
-	insert_iterator = _events.end();
+	most_recent_insert_iterator = _events.end();
 }
 
 ControlList::ControlList (const ControlList& other)
@@ -86,9 +87,10 @@ ControlList::ControlList (const ControlList& other)
 	_search_cache.first = _events.end();
 	_sort_pending = false;
 	new_write_pass = true;
+	_in_write_pass = false;
 	did_write_during_pass = false;
 	insert_position = -1;
-	insert_iterator = _events.end();
+	most_recent_insert_iterator = _events.end();
 
 	copy_events (other);
 
@@ -118,9 +120,10 @@ ControlList::ControlList (const ControlList& other, double start, double end)
 	}
 
 	new_write_pass = false;
+	_in_write_pass = false;
 	did_write_during_pass = false;
 	insert_position = -1;
-	insert_iterator = _events.end();
+	most_recent_insert_iterator = _events.end();
 
 	mark_dirty ();
 }
@@ -241,16 +244,6 @@ ControlList::_x_scale (double factor)
 	mark_dirty ();
 }
 
-void
-ControlList::write_pass_finished (double when)
-{
-	if (did_write_during_pass) {
-		thin ();
-	}
-	new_write_pass = true;
-	did_write_during_pass = false;
-}
-
 struct ControlEventTimeComparator {
 	bool operator() (ControlEvent* a, ControlEvent* b) {
 		return a->when < b->when;
@@ -534,7 +527,7 @@ ControlList::invalidate_insert_iterator ()
 void
 ControlList::unlocked_invalidate_insert_iterator ()
 {
-	insert_iterator = _events.end();
+	most_recent_insert_iterator = _events.end();
 }
 
 void
@@ -545,7 +538,7 @@ ControlList::start_write_pass (double 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
 	   of where it should be in a "lazy" way - deferring it until
 	   we actually add the first point (which may never happen).
@@ -555,7 +548,30 @@ ControlList::start_write_pass (double when)
 }
 
 void
-ControlList::add (double when, double value, bool erase_since_last_add)
+ControlList::write_pass_finished (double when)
+{
+	if (did_write_during_pass) {
+		thin ();
+		did_write_during_pass = false;
+	}
+	new_write_pass = true;
+	_in_write_pass = false;
+}
+
+void
+ControlList::set_in_write_pass (bool yn)
+{
+	_in_write_pass = yn;
+}
+
+bool
+ControlList::in_write_pass () const
+{
+	return _in_write_pass;
+}
+
+void
+ControlList::add (double when, double value)
 {
 	/* this is for making changes from some kind of user interface or
 	   control surface (GUI, MIDI, OSC etc)
@@ -565,7 +581,8 @@ ControlList::add (double when, double value, bool erase_since_last_add)
 		return;
 	}
 
-	DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 add %2 at %3 w/erase = %4\n", this, value, when, erase_since_last_add));
+	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::Mutex::Lock lm (_lock);
@@ -584,7 +601,7 @@ ControlList::add (double when, double value, bool erase_since_last_add)
 			}
 		}
 
-		if (new_write_pass) {
+		if (_in_write_pass && new_write_pass) {
 
 			DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 new write pass, insert pos = %2\n", this, insert_position));
 			
@@ -600,47 +617,49 @@ ControlList::add (double when, double value, bool erase_since_last_add)
 			 */
 			
 			ControlEvent cp (insert_position, 0.0);
-			insert_iterator = lower_bound (_events.begin(), _events.end(), &cp, time_comparator);
+			most_recent_insert_iterator = lower_bound (_events.begin(), _events.end(), &cp, time_comparator);
 			DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 looked up insert iterator for new write pass\n", this));
 
 			double eval_value = unlocked_eval (insert_position);
 			
-			if (insert_iterator == _events.end()) {
+			if (most_recent_insert_iterator == _events.end()) {
+
 				DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 insert iterator at end, adding eval-value there %2\n", this, eval_value));
 				_events.push_back (new ControlEvent (insert_position, eval_value));
 				/* leave insert iterator at the end */
 
-			} else if ((*insert_iterator)->when == when) {
+			} else if ((*most_recent_insert_iterator)->when == when) {
 
 				DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 insert iterator at existing point, setting eval-value there %2\n", this, eval_value));
 
-				/* insert_iterator points to a control event
+				/* most_recent_insert_iterator points to a control event
 				   already at the insert position, so there is
 				   nothing to do.
 
 				   ... except ... 
 
-				   advance insert_iterator so that the "real"
+				   advance most_recent_insert_iterator so that the "real"
 				   insert occurs in the right place, since it 
 				   points to the control event just inserted.
 				 */
 
-				++insert_iterator;
+				++most_recent_insert_iterator;
 			} else {
 
 				/* insert a new control event at the right spot
 				 */
 
-				DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 insert eval-value %2 at iterator\n", this, eval_value));
+				DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 insert eval-value %2 just before iterator @ %3\n", 
+										 this, eval_value, (*most_recent_insert_iterator)->when));
 
-				insert_iterator = _events.insert (insert_iterator, new ControlEvent (insert_position, eval_value));
+				most_recent_insert_iterator = _events.insert (most_recent_insert_iterator, new ControlEvent (insert_position, eval_value));
 
-				/* advance insert_iterator so that the "real"
+				/* advance most_recent_insert_iterator so that the "real"
 				 * insert occurs in the right place, since it 
 				 * points to the control event just inserted.
 				 */
 
-				++insert_iterator;
+				++most_recent_insert_iterator;
 			}
 
 			/* don't do this again till the next write pass */
@@ -648,10 +667,18 @@ ControlList::add (double when, double value, bool erase_since_last_add)
 			new_write_pass = false;
 			did_write_during_pass = true;
 
-		} else if (insert_iterator == _events.end() || when > (*insert_iterator)->when) {
+		} 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, (insert_iterator == _events.end())));
+									 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.
@@ -664,19 +691,23 @@ ControlList::add (double when, double value, bool erase_since_last_add)
 			if (_events.back()->when == when) {
 
 				/* we need to modify the final point, so 
-				   make insert_iterator point to it.
+				   make most_recent_insert_iterator point to it.
 				*/
 
 				DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 modify final value\n", this));
 				
-				insert_iterator = _events.end();
+				most_recent_insert_iterator = _events.end();
-				--insert_iterator;
+				--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 (erase_since_last_add) {
+				if (_in_write_pass) {
 					/* remove the final point, because
 					   we're adding one beyond it.
 					*/
@@ -686,26 +717,34 @@ ControlList::add (double when, double value, bool erase_since_last_add)
 				
 				/* leaving this here will force an append */
 
-				insert_iterator = _events.end();
+				most_recent_insert_iterator = _events.end();
 
 			} else {
 
 				DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 erase %2 from existing iterator (@end ? %3\n", 
-										 this, erase_since_last_add,
+										 this, _in_write_pass,
-										 (insert_iterator == _events.end())));
+										 (most_recent_insert_iterator == _events.end())));
 
-				while (insert_iterator != _events.end()) {
+				if (_in_write_pass) {
-					if ((*insert_iterator)->when < when) {
+					while (most_recent_insert_iterator != _events.end()) {
-						if (erase_since_last_add) {
+						if ((*most_recent_insert_iterator)->when < when) {
-							DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 erase existing @ %2\n", this, (*insert_iterator)));
+							if (_in_write_pass) {
-							delete *insert_iterator;
+								DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 erase existing @ %2\n", this, (*most_recent_insert_iterator)));
-							insert_iterator = _events.erase (insert_iterator);
+								delete *most_recent_insert_iterator;
-							continue;
+								most_recent_insert_iterator = _events.erase (most_recent_insert_iterator);
+								continue;
+							}
+						} else if ((*most_recent_insert_iterator)->when >= when) {
+							break;
 						}
-					} else if ((*insert_iterator)->when >= when) {
+						++most_recent_insert_iterator;
-						break;
 					}
-					++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);
 				}
 			}
 		} 
@@ -713,30 +752,90 @@ ControlList::add (double when, double value, bool erase_since_last_add)
 		/* OK, now we're really ready to add a new point
 		 */
 
-		if (insert_iterator == _events.end()) {
+		if (most_recent_insert_iterator == _events.end()) {
 			DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 appending new point at end\n", this));
-			_events.push_back (new ControlEvent (when, value));
+			
-			/* leave insert_iterator as it was: at the end */
+			bool done = false;
 
-		} else if ((*insert_iterator)->when == when) {
+			/* check if would just be adding to a straight line,
+			 * and don't add another point if so
+			 */
+
+			if (!_events.empty()) { // avoid O(N) _events.size() here
+				if (_events.back()->value == value) {
+					EventList::iterator b = _events.end();
+					--b; // last point, which we know exists
+					if (b != _events.begin()) { // step back again, which may not be possible
+						--b; // next-to-last-point
+						if ((*b)->value == value) {
+							/* straight line - just move the last
+							 * point to the new time
+							 */
+							_events.back()->when = when;
+							done = true;
+						}
+					}
+				}
+			}
+
+			if (!done) {
+				_events.push_back (new ControlEvent (when, value));
+			}
+
+			if (!_in_write_pass) {
+				most_recent_insert_iterator = _events.end();
+				--most_recent_insert_iterator;
+			}
+
+		} 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.
 			 */
-			(*insert_iterator)->value = value;
-			/* insert iterator now points past the control event we just
-			 * modified. the next insert needs to be after this,
-			 * so.. 
-			 */
-			++insert_iterator;
-		} else {
-			DEBUG_TRACE (DEBUG::ControlList, string_compose ("@%1 insert new point at %2 at iterator at %3\n", this, when, (*insert_iterator)->when));
-			_events.insert (insert_iterator, new ControlEvent (when, value));
-			/* leave insert iterator where it was, since it points
-			 * to the next control event AFTER the one we just inserted.
-			 */
-		}
 
+			(*most_recent_insert_iterator)->value = 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));
+			
+			bool done;
+			
+			/* check if would just be adding to a straight line,
+			 * and don't add another point if so
+			 */
+
+			if (most_recent_insert_iterator != _events.begin()) {
+				EventList::iterator b = most_recent_insert_iterator;
+				--b; // prior point, which we know exists
+				if ((*b)->value == value) { // same value as the point we plan to insert
+					if (b != _events.begin()) { // step back again, which may not be possible
+						EventList::iterator bb = b;
+						--bb; // next-to-prior-point
+						if ((*bb)->value == value) {
+							/* straight line - just move the prior
+							 * point to the new time
+							 */
+							(*b)->when = when;
+							
+							if (!_in_write_pass) {
+								most_recent_insert_iterator = b;
+							}
+
+							done = true;
+						}
+					}
+				}
+			}
+			
+			if (!done) {
+				EventList::iterator x = _events.insert (most_recent_insert_iterator, new ControlEvent (when, value));
+
+				if (!_in_write_pass) {
+					most_recent_insert_iterator = x;
+				}
+			}
+		}
 
 		mark_dirty ();
 	}
@@ -749,7 +848,7 @@ ControlList::erase (iterator i)
 {
 	{
 		Glib::Mutex::Lock lm (_lock);
-		if (insert_iterator == i) {
+		if (most_recent_insert_iterator == i) {
 			unlocked_invalidate_insert_iterator ();
 		}
 		_events.erase (i);
@@ -784,7 +883,7 @@ ControlList::erase (double when, double value)
 
 		if (i != end ()) {
 			_events.erase (i);
-			if (insert_iterator == i) {
+			if (most_recent_insert_iterator == i) {
 				unlocked_invalidate_insert_iterator ();
 			}
 		}