redesign VCA control over gain (and theoretically other scalar controls)

master(s) value now just scales the control's own value; a trivial
bit of math at assign/deassign ensures that values do not change
during add/remove master operations

5 files changed, 70 insertions, 135 deletions

diff --git a/libs/ardour/ardour/gain_control.h b/libs/ardour/ardour/gain_control.h
index 53f429b88a..431342dab3 100644
--- a/libs/ardour/ardour/gain_control.h
+++ b/libs/ardour/ardour/gain_control.h
@@ -49,9 +49,6 @@ class LIBARDOUR_API GainControl : public SlavableAutomationControl {
 	double range_db;
 
 	void inc_gain (gain_t);
-
-  private:
-	void recompute_masters_ratios (double val);
 };
 
 } /* namespace */
diff --git a/libs/ardour/ardour/slavable_automation_control.h b/libs/ardour/ardour/slavable_automation_control.h
index 857a8956e1..f1a78891e4 100644
--- a/libs/ardour/ardour/slavable_automation_control.h
+++ b/libs/ardour/ardour/slavable_automation_control.h
@@ -70,7 +70,7 @@ class LIBARDOUR_API SlavableAutomationControl : public AutomationControl
           public:
 		MasterRecord (boost::shared_ptr<AutomationControl> gc, double r)
 			: _master (gc)
-			, _ratio (r)
+			, _yn (false)
 		{}
 
 		boost::shared_ptr<AutomationControl> master() const { return _master; }
@@ -83,23 +83,12 @@ class LIBARDOUR_API SlavableAutomationControl : public AutomationControl
 		bool yn() const { return _yn; }
 		void set_yn (bool yn) { _yn = yn; }
 
-		/* for non-boolean/non-toggled controls, we store a ratio that
-		 * connects the value of the master with the value of this
-		 * slave. See comments in the source for more details on how
-		 * this is computed and used.
-		 */
-
-		double ratio () const { return _ratio; }
-		void reset_ratio (double r) { _ratio = r; }
-
 		PBD::ScopedConnection connection;
 
          private:
 		boost::shared_ptr<AutomationControl> _master;
-		union {
-			double _ratio;
-			bool   _yn;
-		};
+		/* holds most recently seen master value for boolean/toggle controls */
+		bool   _yn;
 	};
 
 	mutable Glib::Threads::RWLock master_lock;
@@ -109,11 +98,10 @@ class LIBARDOUR_API SlavableAutomationControl : public AutomationControl
 
 	void   master_going_away (boost::weak_ptr<AutomationControl>);
 	double get_value_locked() const;
-	void   actually_set_value (double val, PBD::Controllable::GroupControlDisposition group_override);
+	void   actually_set_value (double value, PBD::Controllable::GroupControlDisposition);
 	void   update_boolean_masters_records (boost::shared_ptr<AutomationControl>);
 
 	virtual void   master_changed (bool from_self, GroupControlDisposition gcd, boost::shared_ptr<AutomationControl>);
-	virtual void   recompute_masters_ratios (double val) { /* do nothing by default */}
 	virtual double get_masters_value_locked () const;
 	virtual void   pre_remove_master (boost::shared_ptr<AutomationControl>) {}
 	virtual void   post_add_master (boost::shared_ptr<AutomationControl>) {}
diff --git a/libs/ardour/automation_control.cc b/libs/ardour/automation_control.cc
index c090bca364..0a29e7e662 100644
--- a/libs/ardour/automation_control.cc
+++ b/libs/ardour/automation_control.cc
@@ -169,9 +169,9 @@ AutomationControl::actually_set_value (double value, PBD::Controllable::GroupCon
 	Control::set_double (value, pos, to_list);
 
 	if (old_value != value) {
-		// AutomationType at = (AutomationType) _parameter.type();
-		// std::cerr << "++++ Changed (" << enum_2_string (at) << ", " << enum_2_string (gcd) << ") = " << value
-		// << " (was " << old_value << ") @ " << this << std::endl;
+		//AutomationType at = (AutomationType) _parameter.type();
+		//std::cerr << "++++ Changed (" << enum_2_string (at) << ", " << enum_2_string (gcd) << ") = " << value
+		//<< " (was " << old_value << ") @ " << this << std::endl;
 
 		Changed (true, gcd);
 		_session.set_dirty ();
diff --git a/libs/ardour/gain_control.cc b/libs/ardour/gain_control.cc
index 21e1ba5f85..e6154495a4 100644
--- a/libs/ardour/gain_control.cc
+++ b/libs/ardour/gain_control.cc
@@ -100,50 +100,3 @@ GainControl::inc_gain (gain_t factor)
 	}
 }
 
-void
-GainControl::recompute_masters_ratios (double val)
-{
-	/* Master WRITE lock must be held */
-
-	/* V' is the new gain value for this
-
-	   Mv(n) is the return value of ::get_value() for the n-th master
-	   Mr(n) is the return value of ::ratio() for the n-th master record
-
-	   the slave should return V' on the next call to ::get_value().
-
-	   but the value is determined by the masters, so we know:
-
-	   V' = (Mv(1) * Mr(1)) * (Mv(2) * Mr(2)) * ... * (Mv(n) * Mr(n))
-
-	   hence:
-
-	   Mr(1) * Mr(2) * ... * (Mr(n) = V' / (Mv(1) * Mv(2) * ... * Mv(n))
-
-	   if we make all ratios equal (i.e. each master contributes the same
-	   fraction of its own gain level to make the final slave gain), then we
-	   have:
-
-	   pow (Mr(n), n) = V' / (Mv(1) * Mv(2) * ... * Mv(n))
-
-	   which gives
-
-	   Mr(n) = pow ((V' / (Mv(1) * Mv(2) * ... * Mv(n))), 1/n)
-
-	   Mr(n) is the new ratio number for the slaves
-	*/
-
-	const double nmasters = _masters.size();
-	double masters_total_gain_coefficient = 1.0;
-
-	for (Masters::iterator mr = _masters.begin(); mr != _masters.end(); ++mr) {
-		masters_total_gain_coefficient *= mr->second.master()->get_value();
-	}
-
-	const double new_universal_ratio = pow ((val / masters_total_gain_coefficient), (1.0/nmasters));
-
-	for (Masters::iterator mr = _masters.begin(); mr != _masters.end(); ++mr) {
-		mr->second.reset_ratio (new_universal_ratio);
-	}
-}
-
diff --git a/libs/ardour/slavable_automation_control.cc b/libs/ardour/slavable_automation_control.cc
index b76f91b13b..9acf7444ad 100644
--- a/libs/ardour/slavable_automation_control.cc
+++ b/libs/ardour/slavable_automation_control.cc
@@ -23,6 +23,7 @@
 #include "pbd/error.h"
 #include "pbd/i18n.h"
 
+#include "ardour/audioengine.h"
 #include "ardour/slavable_automation_control.h"
 #include "ardour/session.h"
 
@@ -52,7 +53,6 @@ SlavableAutomationControl::~SlavableAutomationControl ()
 double
 SlavableAutomationControl::get_masters_value_locked () const
 {
-	double v = _desc.normal;
 
 	if (_desc.toggled) {
 		for (Masters::const_iterator mr = _masters.begin(); mr != _masters.end(); ++mr) {
@@ -61,14 +61,16 @@ SlavableAutomationControl::get_masters_value_locked () const
 			}
 		}
 		return _desc.lower;
-	}
+	} else {
 
-	for (Masters::const_iterator mr = _masters.begin(); mr != _masters.end(); ++mr) {
-		/* get current master value, scale by our current ratio with that master */
-		v *= mr->second.master()->get_value () * mr->second.ratio();
-	}
+		double v = 1.0; /* the masters function as a scaling factor */
 
-	return min ((double) _desc.upper, v);
+		for (Masters::const_iterator mr = _masters.begin(); mr != _masters.end(); ++mr) {
+			v *= mr->second.master()->get_value ();
+		}
+
+		return v;
+	}
 }
 
 double
@@ -90,7 +92,23 @@ SlavableAutomationControl::get_value_locked() const
 		}
 	}
 
-	return get_masters_value_locked ();
+	return Control::get_double() * get_masters_value_locked ();
+}
+
+void
+SlavableAutomationControl::actually_set_value (double value, PBD::Controllable::GroupControlDisposition gcd)
+{
+	{
+		Glib::Threads::RWLock::WriterLock lm (master_lock);
+
+		if (!_masters.empty()) {
+			/* need to scale given value by current master's scaling */
+			value /= get_masters_value_locked();
+		}
+	}
+
+	/* this will call Control::set_double() and emit Changed signals as appropriate */
+	AutomationControl::actually_set_value (value, gcd);
 }
 
 /** Get the current effective `user' value based on automation state */
@@ -103,39 +121,17 @@ SlavableAutomationControl::get_value() const
 	if (!from_list) {
 		return get_value_locked ();
 	} else {
-		return get_masters_value_locked () * Control::get_double (from_list, _session.transport_frame());
+		return Control::get_double (true, _session.transport_frame()) * get_masters_value_locked();
 	}
 }
 
 void
-SlavableAutomationControl::actually_set_value (double val, Controllable::GroupControlDisposition group_override)
-{
-	val = std::max (std::min (val, (double)_desc.upper), (double)_desc.lower);
-
-	{
-		Glib::Threads::RWLock::WriterLock lm (master_lock);
-
-		if (!_masters.empty()) {
-			recompute_masters_ratios (val);
-		}
-	}
-
-	/* this sets the Evoral::Control::_user_value for us, which will
-	   be retrieved by AutomationControl::get_value ()
-	*/
-	AutomationControl::actually_set_value (val, group_override);
-}
-
-void
 SlavableAutomationControl::add_master (boost::shared_ptr<AutomationControl> m, bool loading)
 {
 	std::pair<Masters::iterator,bool> res;
 
 	{
 		Glib::Threads::RWLock::WriterLock lm (master_lock);
-		const double current_value = get_value_locked ();
-
-		/* ratio will be recomputed below */
 
 		pair<PBD::ID,MasterRecord> newpair (m->id(), MasterRecord (m, 1.0));
 		res = _masters.insert (newpair);
@@ -143,7 +139,20 @@ SlavableAutomationControl::add_master (boost::shared_ptr<AutomationControl> m, b
 		if (res.second) {
 
 			if (!loading) {
-				recompute_masters_ratios (current_value);
+
+				if (!_desc.toggled) {
+					const double master_value = m->get_value();
+
+					if (master_value == 0.0) {
+						actually_set_value (0.0, Controllable::NoGroup);
+					} else {
+						/* scale control's own value by
+						   amount that the master will
+						   contribute.
+						*/
+						AutomationControl::set_double ((Control::get_double() / master_value), Controllable::NoGroup);
+					}
+				}
 			}
 
 			/* note that we bind @param m as a weak_ptr<AutomationControl>, thus
@@ -254,22 +263,34 @@ SlavableAutomationControl::master_going_away (boost::weak_ptr<AutomationControl>
 void
 SlavableAutomationControl::remove_master (boost::shared_ptr<AutomationControl> m)
 {
-	double current_value;
-	double new_value;
-	bool masters_left;
 	Masters::size_type erased = 0;
 
 	pre_remove_master (m);
 
 	{
 		Glib::Threads::RWLock::WriterLock lm (master_lock);
-		current_value = get_value_locked ();
-		erased = _masters.erase (m->id());
-		if (erased && !_session.deletion_in_progress()) {
-			recompute_masters_ratios (current_value);
+
+		if (!_masters.erase (m->id())) {
+			return;
+		}
+
+		if (!_session.deletion_in_progress()) {
+
+			const double master_value = m->get_value ();
+
+			if (master_value == 0.0) {
+				/* slave would have been set to 0.0 as well,
+				   so just leave it there, and the user can
+				   bring it back up. this fits with the
+				   "removing a VCA does not change the level" rule.
+				*/
+			} else {
+				/* bump up the control's own value by the level
+				   of the master that is being removed.
+				*/
+				AutomationControl::set_double (AutomationControl::get_double() * master_value, Controllable::NoGroup);
+			}
 		}
-		masters_left = _masters.size ();
-		new_value = get_value_locked ();
 	}
 
 	if (_session.deletion_in_progress()) {
@@ -281,15 +302,6 @@ SlavableAutomationControl::remove_master (boost::shared_ptr<AutomationControl> m
 		MasterStatusChange (); /* EMIT SIGNAL */
 	}
 
-	if (new_value != current_value) {
-		if (masters_left == 0) {
-			/* no masters left, make sure we keep the same value
-			   that we had before.
-			*/
-			actually_set_value (current_value, Controllable::UseGroup);
-		}
-	}
-
 	/* no need to update boolean masters records, since the MR will have
 	 * been removed already.
 	 */
@@ -375,19 +387,6 @@ SlavableAutomationControl::use_saved_master_ratios ()
 
 	} else {
 
-		XMLProperty const * prop = _masters_node->property (X_("ratio"));
-
-		if (prop) {
-
-			gain_t ratio;
-			sscanf (prop->value().c_str(), "%g", &ratio);
-
-			for (Masters::iterator mr = _masters.begin(); mr != _masters.end(); ++mr) {
-				mr->second.reset_ratio (ratio);
-			}
-		} else {
-			PBD::error << string_compose (_("programming error: %1"), X_("missing ratio information for control slave"))<< endmsg;
-		}
 	}
 
 	delete _masters_node;
@@ -419,9 +418,7 @@ SlavableAutomationControl::get_state ()
 					masters_node->add_child_nocopy (*mnode);
 				}
 			} else {
-				XMLNode* masters_node = new XMLNode (X_("masters"));
-				/* ratio is the same for all masters, so just store one */
-				masters_node->add_property (X_("ratio"), PBD::to_string (_masters.begin()->second.ratio(), std::dec));
+
 			}
 
 			node.add_child_nocopy (*masters_node);