Clean up and hopefully fix handling of logarithmic plugin parameters (fixes #3769).

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

11 files changed, 102 insertions, 104 deletions

diff --git a/gtk2_ardour/automation_controller.cc b/gtk2_ardour/automation_controller.cc
index 5c00d5a779..75239f7f80 100644
--- a/gtk2_ardour/automation_controller.cc
+++ b/gtk2_ardour/automation_controller.cc
@@ -94,20 +94,20 @@ AutomationController::get_label (double& xpos)
 void
 AutomationController::display_effective_value()
 {
-	float value = _controllable->get_value();
+	double const ui_value = _controllable->user_to_ui (_controllable->get_value());
 
-	if (_adjustment->get_value() != value) {
+	if (_adjustment->get_value() != ui_value) {
 		_ignore_change = true;
-		_adjustment->set_value (value);
+		_adjustment->set_value (ui_value);
 		_ignore_change = false;
 	}
 }
 
 void
-AutomationController::value_adjusted()
+AutomationController::value_adjusted ()
 {
 	if (!_ignore_change) {
-		_controllable->set_value(_adjustment->get_value());
+		_controllable->set_value (_controllable->ui_to_user (_adjustment->get_value()));
 	}
 }
 
diff --git a/gtk2_ardour/generic_pluginui.cc b/gtk2_ardour/generic_pluginui.cc
index 9204912a36..fdf0d62482 100644
--- a/gtk2_ardour/generic_pluginui.cc
+++ b/gtk2_ardour/generic_pluginui.cc
@@ -494,19 +494,10 @@ GenericPluginUI::build_control_ui (guint32 port_index, boost::shared_ptr<Automat
 		*/
 
                 Adjustment* adj = control_ui->controller->adjustment();
+		boost::shared_ptr<PluginInsert::PluginControl> pc = boost::dynamic_pointer_cast<PluginInsert::PluginControl> (control_ui->control);
 
-		adj->set_lower (desc.lower);
-		adj->set_upper (desc.upper);
-
-		control_ui->logarithmic = desc.logarithmic;
-
-		if (control_ui->logarithmic) {
-			if (adj->get_lower() == 0.0) {
-				adj->set_lower (adj->get_upper()/10000);
-			}
-			adj->set_upper (log(adj->get_upper()));
-			adj->set_lower (log(adj->get_lower()));
-		}
+		adj->set_lower (pc->user_to_ui (desc.lower));
+		adj->set_upper (pc->user_to_ui (desc.upper));
 
 		adj->set_step_increment (desc.step);
 		adj->set_page_increment (desc.largestep);
@@ -522,23 +513,14 @@ GenericPluginUI::build_control_ui (guint32 port_index, boost::shared_ptr<Automat
 			control_ui->controller->set_name (X_("PluginSlider"));
 			control_ui->controller->set_style (BarController::LeftToRight);
 			control_ui->controller->set_use_parent (true);
-                        control_ui->controller->set_logarithmic (control_ui->logarithmic);
+                        control_ui->controller->set_logarithmic (desc.logarithmic);
 
 			control_ui->controller->StartGesture.connect (sigc::bind (sigc::mem_fun(*this, &GenericPluginUI::start_touch), control_ui));
 			control_ui->controller->StopGesture.connect (sigc::bind (sigc::mem_fun(*this, &GenericPluginUI::stop_touch), control_ui));
 
 		}
 
-		if (control_ui->logarithmic) {
-                        double val = plugin->get_parameter (port_index);
-                        if (isnan (val) || val <= 0.0) {
-                                adj->set_value (0.0);
-                        } else {
-                                adj->set_value (log(val));
-                        }
-		} else{
-			adj->set_value(plugin->get_parameter(port_index));
-		}
+		adj->set_value (pc->plugin_to_ui (plugin->get_parameter (port_index)));
 
 		/* XXX memory leak: SliderController not destroyed by ControlUI
 		   destructor, and manage() reports object hierarchy
diff --git a/gtk2_ardour/plugin_ui.h b/gtk2_ardour/plugin_ui.h
index cdb29bcd82..31a00ad9b3 100644
--- a/gtk2_ardour/plugin_ui.h
+++ b/gtk2_ardour/plugin_ui.h
@@ -213,7 +213,6 @@ class GenericPluginUI : public PlugUIBase, public Gtk::VBox
 		boost::shared_ptr<AutomationController> controller;
 		Gtkmm2ext::ClickBox*                    clickbox;
 		Gtk::Label                              label;
-		bool                                    logarithmic;
 		bool                                    update_pending;
 		char                                    ignore_change;
 		Gtk::Button                             automate_button;
diff --git a/libs/ardour/ardour/automation_control.h b/libs/ardour/ardour/automation_control.h
index 006e74346f..d786f28ab6 100644
--- a/libs/ardour/ardour/automation_control.h
+++ b/libs/ardour/ardour/automation_control.h
@@ -74,21 +74,26 @@ public:
 		return ((ARDOUR::AutomationList*)_list.get())->stop_touch(mark, when);
 	}
 
-	/** Set the value and do the right thing based on automation state
-	 * (e.g. record if necessary, etc.)
-	 */
-	void set_value(double val);
-
-	/** Get the current effective value based on automation state.
-	 */
-	double get_value() const;
+	void set_value (double);
+	double get_value () const;
 
         double lower() const { return parameter().min(); }
         double upper() const { return parameter().max(); }
 
         const ARDOUR::Session& session() const { return _session; }
 
+	/** Convert user values to UI values.  See pbd/controllable.h */
+	virtual double user_to_ui (double val) const {
+		return val;
+	}
+
+	/** Convert UI values to user values.  See pbd/controllable.h */
+	virtual double ui_to_user (double val) const {
+		return val;
+	}
+	
 protected:
+	
 	ARDOUR::Session& _session;
 };
 
diff --git a/libs/ardour/ardour/plugin.h b/libs/ardour/ardour/plugin.h
index 00ed6bdc94..f2af0360fc 100644
--- a/libs/ardour/ardour/plugin.h
+++ b/libs/ardour/ardour/plugin.h
@@ -90,8 +90,8 @@ class Plugin : public PBD::StatefulDestructible, public Latent
 		bool logarithmic;
 		bool sr_dependent;
 		std::string label;
-		float lower;
-		float upper;
+		float lower; ///< if this is a frequency, it will be in Hz (not a fraction of the sample rate)
+		float upper; ///< if this is a frequency, it will be in Hz (not a fraction of the sample rate)
 		float step;
 		float smallstep;
 		float largestep;
diff --git a/libs/ardour/ardour/plugin_insert.h b/libs/ardour/ardour/plugin_insert.h
index ea7a081c22..45a0478584 100644
--- a/libs/ardour/ardour/plugin_insert.h
+++ b/libs/ardour/ardour/plugin_insert.h
@@ -85,13 +85,16 @@ class PluginInsert : public Processor
 		double get_value (void) const;
                 XMLNode& get_state();
 
-		bool logarithmic () const {
-			return _logarithmic;
-		}
+		double user_to_ui (double) const;
+		double ui_to_user (double) const;
+		double plugin_to_ui (double) const;
                 
 	private:
+		double user_to_plugin (double) const;
+		
 		PluginInsert* _plugin;
 		bool _logarithmic;
+		bool _sr_dependent;
 		bool _toggled;
 	};
 
@@ -117,8 +120,6 @@ class PluginInsert : public Processor
 		return _splitting;
 	}
 
-        std::string value_as_string (boost::shared_ptr<AutomationControl>) const;
-
 	PBD::Signal2<void,BufferSet*, BufferSet*> AnalysisDataGathered;
 	/** Emitted when the return value of splitting () has changed */
 	PBD::Signal0<void> SplittingChanged;
diff --git a/libs/ardour/automation_control.cc b/libs/ardour/automation_control.cc
index a1611337fd..25a18949ee 100644
--- a/libs/ardour/automation_control.cc
+++ b/libs/ardour/automation_control.cc
@@ -40,6 +40,7 @@ AutomationControl::AutomationControl(
 {
 }
 
+/** Get the current effective `user' value based on automation state */
 double
 AutomationControl::get_value() const
 {
@@ -47,6 +48,10 @@ AutomationControl::get_value() const
 	return Control::get_double (from_list, _session.transport_frame());
 }
 
+/** Set the value and do the right thing based on automation state
+ *  (e.g. record if necessary, etc.)
+ *  @param value `user' value
+ */
 void
 AutomationControl::set_value(double value)
 {
diff --git a/libs/ardour/ladspa_plugin.cc b/libs/ardour/ladspa_plugin.cc
index 69feed9481..1ea158d7cd 100644
--- a/libs/ardour/ladspa_plugin.cc
+++ b/libs/ardour/ladspa_plugin.cc
@@ -178,32 +178,17 @@ LadspaPlugin::default_value (uint32_t port)
 		}
 
 		else if (LADSPA_IS_HINT_DEFAULT_LOW(prh[port].HintDescriptor)) {
-			if (LADSPA_IS_HINT_LOGARITHMIC(prh[port].HintDescriptor)) {
-				ret = exp(log(prh[port].LowerBound) * 0.75f + log(prh[port].UpperBound) * 0.25f);
-			}
-			else {
-				ret = prh[port].LowerBound * 0.75f + prh[port].UpperBound * 0.25f;
-			}
+			ret = prh[port].LowerBound * 0.75f + prh[port].UpperBound * 0.25f;
 			bounds_given = true;
 			sr_scaling = true;
 		}
 		else if (LADSPA_IS_HINT_DEFAULT_MIDDLE(prh[port].HintDescriptor)) {
-			if (LADSPA_IS_HINT_LOGARITHMIC(prh[port].HintDescriptor)) {
-				ret = exp(log(prh[port].LowerBound) * 0.5f + log(prh[port].UpperBound) * 0.5f);
-			}
-			else {
-				ret = prh[port].LowerBound * 0.5f + prh[port].UpperBound * 0.5f;
-			}
+			ret = prh[port].LowerBound * 0.5f + prh[port].UpperBound * 0.5f;
 			bounds_given = true;
 			sr_scaling = true;
 		}
 		else if (LADSPA_IS_HINT_DEFAULT_HIGH(prh[port].HintDescriptor)) {
-			if (LADSPA_IS_HINT_LOGARITHMIC(prh[port].HintDescriptor)) {
-				ret = exp(log(prh[port].LowerBound) * 0.25f + log(prh[port].UpperBound) * 0.75f);
-			}
-			else {
-				ret = prh[port].LowerBound * 0.25f + prh[port].UpperBound * 0.75f;
-			}
+			ret = prh[port].LowerBound * 0.25f + prh[port].UpperBound * 0.75f;
 			bounds_given = true;
 			sr_scaling = true;
 		}
@@ -314,6 +299,7 @@ LadspaPlugin::set_parameter (uint32_t which, float val)
 	Plugin::set_parameter (which, val);
 }
 
+/** @return `plugin' value */
 float
 LadspaPlugin::get_parameter (uint32_t which) const
 {
diff --git a/libs/ardour/plugin.cc b/libs/ardour/plugin.cc
index 75c471d1a4..35e57ab63a 100644
--- a/libs/ardour/plugin.cc
+++ b/libs/ardour/plugin.cc
@@ -277,6 +277,7 @@ Plugin::load_preset (PresetRecord r)
 	return true;
 }
 
+/** @param val `plugin' value */
 void
 Plugin::set_parameter (uint32_t which, float val)
 {
diff --git a/libs/ardour/plugin_insert.cc b/libs/ardour/plugin_insert.cc
index a690cce5cc..43df8cc29d 100644
--- a/libs/ardour/plugin_insert.cc
+++ b/libs/ardour/plugin_insert.cc
@@ -1030,49 +1030,73 @@ PluginInsert::PluginControl::PluginControl (PluginInsert* p, const Evoral::Param
 	Plugin::ParameterDescriptor desc;
 	p->plugin(0)->get_parameter_descriptor (param.id(), desc);
 	_logarithmic = desc.logarithmic;
+	_sr_dependent = desc.sr_dependent;
 	_toggled = desc.toggled;
 }
 
+/** @param val `user' value */
 void
-PluginInsert::PluginControl::set_value (double val)
+PluginInsert::PluginControl::set_value (double user_val)
 {
 	/* FIXME: probably should be taking out some lock here.. */
 
-	if (_toggled) {
-		if (val > 0.5) {
-			val = 1.0;
-		} else {
-			val = 0.0;
-		}
-	} else {
+	double const plugin_val = user_to_plugin (user_val);
 
-		/*const float range = _list->get_max_y() - _list->get_min_y();
-		const float lower = _list->get_min_y();
+	for (Plugins::iterator i = _plugin->_plugins.begin(); i != _plugin->_plugins.end(); ++i) {
+		(*i)->set_parameter (_list->parameter().id(), plugin_val);
+	}
 
-		if (!_logarithmic) {
-			val = lower + (range * val);
-		} else {
-			float log_lower = 0.0f;
-			if (lower > 0.0f) {
-				log_lower = log(lower);
-			}
+	boost::shared_ptr<Plugin> iasp = _plugin->_impulseAnalysisPlugin.lock();
+	if (iasp) {
+		iasp->set_parameter (_list->parameter().id(), plugin_val);
+	}
 
-			val = exp(log_lower + log(range) * val);
-		}*/
+	AutomationControl::set_value (user_val);
+}
 
+double
+PluginInsert::PluginControl::user_to_plugin (double val) const
+{
+	if (_sr_dependent) {
+		val /= _session.frame_rate ();
 	}
+	
+	return val;
+}
 
-	for (Plugins::iterator i = _plugin->_plugins.begin();
-			i != _plugin->_plugins.end(); ++i) {
-		(*i)->set_parameter (_list->parameter().id(), val);
+double
+PluginInsert::PluginControl::user_to_ui (double val) const
+{
+	if (_logarithmic) {
+		if (val > 0) {
+			val = log (val);
+		} else {
+			val = 0;
+		}
 	}
 
-	boost::shared_ptr<Plugin> iasp = _plugin->_impulseAnalysisPlugin.lock();
-	if (iasp) {
-		iasp->set_parameter (_list->parameter().id(), val);
+	return val;
+}
+
+double
+PluginInsert::PluginControl::ui_to_user (double val) const
+{
+	if (_logarithmic) {
+		val = exp (val);
+	}
+
+	return val;
+}
+
+/** Convert plugin values to UI values.  See pbd/controllable.h */
+double
+PluginInsert::PluginControl::plugin_to_ui (double val) const
+{
+	if (_sr_dependent) {
+		val = val * _session.frame_rate ();
 	}
 
-	AutomationControl::set_value(val);
+	return user_to_ui (val);
 }
 
 XMLNode&
@@ -1164,19 +1188,3 @@ PluginInsert::set_splitting (bool s)
 	_splitting = s;
 	SplittingChanged (); /* EMIT SIGNAL */
 }
-
-string
-PluginInsert::value_as_string (boost::shared_ptr<AutomationControl> ac) const
-{
-	boost::shared_ptr<PluginControl> pc = boost::dynamic_pointer_cast<PluginControl> (ac);
-	assert (pc);
-
-	stringstream s;
-	if (pc->logarithmic ()) {
-		s << exp (pc->get_value ());
-	} else {
-		s << pc->get_value ();
-	}
-
-	return s.str ();
-}
diff --git a/libs/pbd/pbd/controllable.h b/libs/pbd/pbd/controllable.h
index d0769ffd01..864a68b1a8 100644
--- a/libs/pbd/pbd/controllable.h
+++ b/libs/pbd/pbd/controllable.h
@@ -45,7 +45,18 @@ class Controllable : public PBD::StatefulDestructible {
 	Controllable (const std::string& name, Flag f = Flag (0));
 	virtual ~Controllable() { Destroyed (this); }
 
+	/* We express Controllable values in one of three ways:
+	 * 1. `user' --- as presented to the user (e.g. dB, Hz etc.)
+	 * 2. `UI' --- as used in some cases for the internal representation
+	 *    of the UI.  This may be the same as `user', or may be something
+	 *    like the natural log of frequency in order that sliders operate
+	 *    in a logarithmic fashion.
+	 * 3. `plugin' --- as passed to a plugin.
+	 */
+
+	/** Set `user' value */
 	virtual void set_value (double) = 0;
+	/** @return `user' value */
 	virtual double get_value (void) const = 0;
 
 	PBD::Signal0<void> LearningFinished;