Add AutomationControl::set_value_unchecked() and AutomationControl::writable() and use them.

Classes derived from AutomationControl now check ::writable() in their ::set_value() methods to ensure that they
do not attempt to overwrite data sent to them while automation playback is underway.

14 files changed, 107 insertions, 2 deletions

diff --git a/libs/ardour/amp.cc b/libs/ardour/amp.cc
index c62bb7ff69..8b55b8ca8d 100644
--- a/libs/ardour/amp.cc
+++ b/libs/ardour/amp.cc
@@ -403,6 +403,14 @@ Amp::set_state (const XMLNode& node, int version)
 void
 Amp::GainControl::set_value (double val)
 {
+	if (writable()) {
+		set_value_unchecked (val);
+	}
+}
+
+void
+Amp::GainControl::set_value_unchecked (double val)
+{
 	AutomationControl::set_value (std::max (std::min (val, (double)_desc.upper), (double)_desc.lower));
 	_amp->session().set_dirty ();
 }
diff --git a/libs/ardour/ardour/amp.h b/libs/ardour/ardour/amp.h
index 62f273e477..7cde853155 100644
--- a/libs/ardour/ardour/amp.h
+++ b/libs/ardour/ardour/amp.h
@@ -91,6 +91,7 @@ public:
 		}
 
 		void set_value (double val);
+		void set_value_unchecked (double);
 
 		double internal_to_interface (double) const;
 		double interface_to_internal (double) const;
diff --git a/libs/ardour/ardour/automation_control.h b/libs/ardour/ardour/automation_control.h
index c634e3474a..9190b71677 100644
--- a/libs/ardour/ardour/automation_control.h
+++ b/libs/ardour/ardour/automation_control.h
@@ -81,9 +81,21 @@ public:
 	void start_touch(double when);
 	void stop_touch(bool mark, double when);
 
+	/* inherited from PBD::Controllable.
+	 * Derived classes MUST call ::writable() to verify
+	 * that writing to the parameter is legal at that time.
+	 */
 	void set_value (double);
 	double get_value () const;
 
+	/* automation related value setting */
+	virtual bool writable () const;
+	/* Call to ::set_value() with no test for writable() because
+	 * this is only used by automation playback. We would like
+	 * to make it pure virtual 
+	 */
+	virtual void set_value_unchecked (double val) {}
+
 	double lower()   const { return _desc.lower; }
 	double upper()   const { return _desc.upper; }
 	double normal()  const { return _desc.normal; }
diff --git a/libs/ardour/ardour/midi_track.h b/libs/ardour/ardour/midi_track.h
index c48f384ce9..80bb743b10 100644
--- a/libs/ardour/ardour/midi_track.h
+++ b/libs/ardour/ardour/midi_track.h
@@ -90,6 +90,8 @@ public:
 		{}
 
 		void set_value (double val);
+		void set_value_unchecked (double);
+		bool writable() const { return true; }
 
 		MidiTrack* _route;
 	};
diff --git a/libs/ardour/ardour/pan_controllable.h b/libs/ardour/ardour/pan_controllable.h
index 63ee9d3b8c..ff00d8119f 100644
--- a/libs/ardour/ardour/pan_controllable.h
+++ b/libs/ardour/ardour/pan_controllable.h
@@ -48,6 +48,7 @@ public:
 
 	double lower () const;
 	void set_value (double);
+	void set_value_unchecked (double);
 
 private:
 	Pannable* owner;
diff --git a/libs/ardour/ardour/plugin_insert.h b/libs/ardour/ardour/plugin_insert.h
index d4d9adb54d..a37c9cae68 100644
--- a/libs/ardour/ardour/plugin_insert.h
+++ b/libs/ardour/ardour/plugin_insert.h
@@ -95,6 +95,7 @@ class LIBARDOUR_API PluginInsert : public Processor
 		               boost::shared_ptr<AutomationList> list=boost::shared_ptr<AutomationList>());
 
 		void set_value (double val);
+		void set_value_unchecked (double);
 		double get_value (void) const;
 		void catch_up_with_external_value (double val);
 		XMLNode& get_state();
@@ -113,6 +114,7 @@ class LIBARDOUR_API PluginInsert : public Processor
 
 		void set_value (const Variant& val);
 		void set_value (double val);
+		void set_value_unchecked (double);
 		double get_value (void) const;
 		XMLNode& get_state();
 
diff --git a/libs/ardour/ardour/route.h b/libs/ardour/ardour/route.h
index 001accb2e3..da4f0f809e 100644
--- a/libs/ardour/ardour/route.h
+++ b/libs/ardour/ardour/route.h
@@ -389,6 +389,7 @@ class LIBARDOUR_API Route : public SessionObject, public Automatable, public Rou
 	public:
 		SoloControllable (std::string name, boost::shared_ptr<Route>);
 		void set_value (double);
+		void set_value_unchecked (double);
 		double get_value () const;
 
 	private:
@@ -399,6 +400,7 @@ class LIBARDOUR_API Route : public SessionObject, public Automatable, public Rou
 	public:
 		MuteControllable (std::string name, boost::shared_ptr<Route>);
 		void set_value (double);
+		void set_value_unchecked (double);
 		double get_value () const;
 
 		/* Pretend to change value, but do not affect actual route mute. */
diff --git a/libs/ardour/ardour/track.h b/libs/ardour/ardour/track.h
index 31ae261ea7..fa7b1f30bb 100644
--- a/libs/ardour/ardour/track.h
+++ b/libs/ardour/ardour/track.h
@@ -207,6 +207,7 @@ class LIBARDOUR_API Track : public Route, public PublicDiskstream
 		RecEnableControl (boost::shared_ptr<Track> t);
 
 		void set_value (double);
+		void set_value_unchecked (double);
 		double get_value (void) const;
 
 		boost::weak_ptr<Track> track;
diff --git a/libs/ardour/automation_control.cc b/libs/ardour/automation_control.cc
index 0e2355e708..671cd6e977 100644
--- a/libs/ardour/automation_control.cc
+++ b/libs/ardour/automation_control.cc
@@ -25,6 +25,7 @@
 #include "ardour/session.h"
 
 #include "pbd/memento_command.h"
+#include "pbd/stacktrace.h"
 
 #include "i18n.h"
 
@@ -48,6 +49,16 @@ AutomationControl::~AutomationControl ()
 {
 }
 
+bool
+AutomationControl::writable() const
+{
+	boost::shared_ptr<AutomationList> al = alist();
+	if (al) {
+		return al->automation_state() != Play;
+	}
+	return true;
+}
+
 /** Get the current effective `user' value based on automation state */
 double
 AutomationControl::get_value() const
diff --git a/libs/ardour/midi_track.cc b/libs/ardour/midi_track.cc
index f3bc2c4f79..c49a2ef7f1 100644
--- a/libs/ardour/midi_track.cc
+++ b/libs/ardour/midi_track.cc
@@ -717,6 +717,14 @@ MidiTrack::set_parameter_automation_state (Evoral::Parameter param, AutoState st
 void
 MidiTrack::MidiControl::set_value(double val)
 {
+	if (writable()) {
+		set_value_unchecked (val);
+	}
+}
+
+void
+MidiTrack::MidiControl::set_value_unchecked(double val)
+{
 	const Evoral::Parameter &parameter = _list ? _list->parameter() : Control::parameter();
 	const Evoral::ParameterDescriptor &desc = EventTypeMap::instance().descriptor(parameter);
 
diff --git a/libs/ardour/pan_controllable.cc b/libs/ardour/pan_controllable.cc
index ecffa17440..71c8a9f62f 100644
--- a/libs/ardour/pan_controllable.cc
+++ b/libs/ardour/pan_controllable.cc
@@ -37,7 +37,15 @@ PanControllable::lower () const
 void
 PanControllable::set_value (double v)
 {
-        boost::shared_ptr<Panner> p = owner->panner();
+	if (writable()) {
+		set_value_unchecked (v);
+	}
+}
+
+void
+PanControllable::set_value_unchecked (double v)
+{
+	boost::shared_ptr<Panner> p = owner->panner();
 
         if (!p) {
                 /* no panner: just do it */
diff --git a/libs/ardour/plugin_insert.cc b/libs/ardour/plugin_insert.cc
index ec6d86d71a..34745b7cab 100644
--- a/libs/ardour/plugin_insert.cc
+++ b/libs/ardour/plugin_insert.cc
@@ -405,7 +405,16 @@ PluginInsert::connect_and_run (BufferSet& bufs, pframes_t nframes, framecnt_t of
 				const float val = c->list()->rt_safe_eval (now, valid);
 
 				if (valid) {
-					c->set_value(val);
+					/* This is the ONLY place where we are
+					 *  allowed to call
+					 *  AutomationControl::set_value_unchecked(). We
+					 *  know that the control is in
+					 *  automation playback mode, so no
+					 *  check on writable() is required
+					 *  (which must be done in AutomationControl::set_value()
+					 *
+					 */
+					c->set_value_unchecked(val);
 				}
 
 			}
@@ -1295,6 +1304,14 @@ PluginInsert::PluginControl::PluginControl (PluginInsert*                     p,
 void
 PluginInsert::PluginControl::set_value (double user_val)
 {
+	if (writable()) {
+		set_value_unchecked (user_val);
+	}
+}
+
+void
+PluginInsert::PluginControl::set_value_unchecked (double user_val)
+{
 	/* FIXME: probably should be taking out some lock here.. */
 
 	for (Plugins::iterator i = _plugin->_plugins.begin(); i != _plugin->_plugins.end(); ++i) {
@@ -1360,6 +1377,14 @@ PluginInsert::PluginPropertyControl::PluginPropertyControl (PluginInsert*
 void
 PluginInsert::PluginPropertyControl::set_value (double user_val)
 {
+	if (writable()) {
+		set_value_unchecked (user_val);
+	}
+}
+
+void
+PluginInsert::PluginPropertyControl::set_value_unchecked (double user_val)
+{
 	/* Old numeric set_value(), coerce to appropriate datatype if possible.
 	   This is lossy, but better than nothing until Ardour's automation system
 	   can handle various datatypes all the way down. */
diff --git a/libs/ardour/route.cc b/libs/ardour/route.cc
index 60f4908768..56d1f69ebe 100644
--- a/libs/ardour/route.cc
+++ b/libs/ardour/route.cc
@@ -3848,6 +3848,14 @@ Route::SoloControllable::SoloControllable (std::string name, boost::shared_ptr<R
 void
 Route::SoloControllable::set_value (double val)
 {
+	if (writable()) {
+		set_value_unchecked (val);
+	}
+}
+
+void
+Route::SoloControllable::set_value_unchecked (double val)
+{
 	const bool bval = ((val >= 0.5) ? true : false);
 
 	boost::shared_ptr<RouteList> rl (new RouteList);
@@ -3921,6 +3929,14 @@ Route::MuteControllable::set_superficial_value(bool muted)
 void
 Route::MuteControllable::set_value (double val)
 {
+	if (writable()) {
+		set_value_unchecked (val);
+	}
+}
+
+void
+Route::MuteControllable::set_value_unchecked (double val)
+{
 	const bool bval = ((val >= 0.5) ? true : false);
 
 	boost::shared_ptr<Route> r = _route.lock ();
diff --git a/libs/ardour/track.cc b/libs/ardour/track.cc
index e99d07f047..c4c82808bd 100644
--- a/libs/ardour/track.cc
+++ b/libs/ardour/track.cc
@@ -192,6 +192,14 @@ Track::RecEnableControl::RecEnableControl (boost::shared_ptr<Track> t)
 void
 Track::RecEnableControl::set_value (double val)
 {
+	if (writable()) {
+		set_value_unchecked (val);
+	}
+}
+
+void
+Track::RecEnableControl::set_value_unchecked (double val)
+{
 	boost::shared_ptr<Track> t = track.lock ();
 	if (!t) {
 		return;