diff options
author | Paul Davis <paul@linuxaudiosystems.com> | 2015-10-20 09:07:51 -0400 |
---|---|---|
committer | Paul Davis <paul@linuxaudiosystems.com> | 2015-10-20 09:07:58 -0400 |
commit | 336b2eb9a4a8634bae84a15e952d20335aa28c12 (patch) | |
tree | ab171341dd5ab7915ae77c937673ae586116ecb9 | |
parent | f1a6d7816d13d3eca5885494f711b88a8270c899 (diff) |
rename ParameterChanged signal in Plugin to ParameterChangedExternally to reflect its intent, and clean up the result.
The signal exists to notify listeners that something outside of the host's control (e.g. a plugin's own GUI for AU or VST)
has modified a plugin parameter. Previous code had strange feedback loops and ambiguous semantics.
Significant modification of LV2 GUI updating was required.
Still to be tested for feedback loop issues: AudioUnits
-rw-r--r-- | gtk2_ardour/lv2_plugin_ui.cc | 54 | ||||
-rw-r--r-- | gtk2_ardour/lv2_plugin_ui.h | 5 | ||||
-rw-r--r-- | gtk2_ardour/plugin_ui.cc | 8 | ||||
-rw-r--r-- | gtk2_ardour/plugin_ui.h | 1 | ||||
-rw-r--r-- | libs/ardour/ardour/plugin.h | 22 | ||||
-rw-r--r-- | libs/ardour/ardour/plugin_insert.h | 4 | ||||
-rw-r--r-- | libs/ardour/plugin.cc | 12 | ||||
-rw-r--r-- | libs/ardour/plugin_insert.cc | 105 | ||||
-rw-r--r-- | libs/ardour/session_vst.cc | 2 | ||||
-rw-r--r-- | libs/ardour/source.cc | 2 | ||||
-rw-r--r-- | libs/ardour/vst_plugin.cc | 6 |
11 files changed, 115 insertions, 106 deletions
diff --git a/gtk2_ardour/lv2_plugin_ui.cc b/gtk2_ardour/lv2_plugin_ui.cc index 141f437f4c..e2c446ef20 100644 --- a/gtk2_ardour/lv2_plugin_ui.cc +++ b/gtk2_ardour/lv2_plugin_ui.cc @@ -20,7 +20,9 @@ #include "ardour/lv2_plugin.h" #include "ardour/session.h" #include "pbd/error.h" +#include "pbd/stacktrace.h" +#include "gui_thread.h" #include "lv2_plugin_ui.h" #include "timers.h" @@ -53,6 +55,9 @@ LV2PluginUI::write_from_ui(void* controller, } boost::shared_ptr<AutomationControl> ac = me->_controllables[port_index]; + /* Cache our local copy of the last value received from the GUI */ + me->_values[port_index] = *(const float*) buffer; + /* Now update the control itself */ if (ac) { ac->set_value(*(const float*)buffer); } @@ -120,26 +125,18 @@ LV2PluginUI::on_external_ui_closed(void* controller) } void -LV2PluginUI::parameter_changed(uint32_t port_index, float val) +LV2PluginUI::control_changed (uint32_t port_index) { - PlugUIBase::parameter_changed(port_index, val); - - if (val != _values[port_index]) { - parameter_update(port_index, val); + /* Must run in GUI thread because we modify _updates with no lock */ + if (_lv2->get_parameter (port_index) != _values[port_index]) { + /* current plugin parameter does not match last value received + from GUI, so queue an update to push it to the GUI during + our regular timeout. + */ + _updates.insert (port_index); } } -void -LV2PluginUI::parameter_update(uint32_t port_index, float val) -{ - if (!_inst) { - return; - } - - suil_instance_port_event((SuilInstance*)_inst, port_index, 4, 0, &val); - _values[port_index] = val; -} - bool LV2PluginUI::start_updating(GdkEventAny*) { @@ -183,13 +180,14 @@ LV2PluginUI::output_update() } } - /* FIXME only works with control output ports (which is all we support now anyway) */ - uint32_t nports = _output_ports.size(); - for (uint32_t i = 0; i < nports; ++i) { - uint32_t index = _output_ports[i]; - parameter_changed(index, _lv2->get_parameter(index)); + if (_inst) { + for (Updates::iterator i = _updates.begin(); i != _updates.end(); ++i) { + float val = _lv2->get_parameter (*i); + /* push current value to the GUI */ + suil_instance_port_event ((SuilInstance*)_inst, (*i), 4, 0, &val); + } + _updates.clear (); } - } LV2PluginUI::LV2PluginUI(boost::shared_ptr<PluginInsert> pi, @@ -358,12 +356,14 @@ LV2PluginUI::lv2ui_instantiate(const std::string& title) bool ok; uint32_t port = _lv2->nth_parameter(i, ok); if (ok) { - _values[port] = _lv2->get_parameter(port); _controllables[port] = boost::dynamic_pointer_cast<ARDOUR::AutomationControl> ( insert->control(Evoral::Parameter(PluginAutomation, 0, port))); - if (_lv2->parameter_is_control(port) && _lv2->parameter_is_input(port)) { - parameter_update(port, _values[port]); + /* FIXME only works with control output ports (which is all we support now anyway) */ + if (_controllables[port] && _lv2->parameter_is_control(port) && _lv2->parameter_is_input(port)) { + _controllables[port]->Changed.connect (control_connections, invalidator (*this), boost::bind (&LV2PluginUI::control_changed, this, port), gui_context()); + /* queue for first update ("push") to GUI */ + _updates.insert (port); } } } @@ -401,9 +401,7 @@ LV2PluginUI::lv2ui_free() LV2PluginUI::~LV2PluginUI () { - if (_values) { - delete[] _values; - } + delete [] _values; _message_update_connection.disconnect(); _screen_update_connection.disconnect(); diff --git a/gtk2_ardour/lv2_plugin_ui.h b/gtk2_ardour/lv2_plugin_ui.h index 6a8acf9cf5..25ab4b9e30 100644 --- a/gtk2_ardour/lv2_plugin_ui.h +++ b/gtk2_ardour/lv2_plugin_ui.h @@ -26,6 +26,7 @@ #include <list> #include <map> +#include <set> #include <vector> #include <gtkmm/widget.h> @@ -64,7 +65,7 @@ class LV2PluginUI : public PlugUIBase, public Gtk::VBox private: - void parameter_changed (uint32_t, float); + void control_changed (uint32_t); typedef boost::shared_ptr<ARDOUR::AutomationControl> ControllableRef; @@ -85,6 +86,8 @@ class LV2PluginUI : public PlugUIBase, public Gtk::VBox LV2_Feature _parent_feature; Gtk::Window* _win_ptr; void* _inst; + typedef std::set<uint32_t> Updates; + Updates _updates; static void on_external_ui_closed(void* controller); diff --git a/gtk2_ardour/plugin_ui.cc b/gtk2_ardour/plugin_ui.cc index 37c01462b2..19dbee422f 100644 --- a/gtk2_ardour/plugin_ui.cc +++ b/gtk2_ardour/plugin_ui.cc @@ -484,7 +484,7 @@ PlugUIBase::PlugUIBase (boost::shared_ptr<PluginInsert> pi) plugin->PresetAdded.connect (*this, invalidator (*this), boost::bind (&PlugUIBase::preset_added_or_removed, this), gui_context ()); plugin->PresetRemoved.connect (*this, invalidator (*this), boost::bind (&PlugUIBase::preset_added_or_removed, this), gui_context ()); plugin->PresetLoaded.connect (*this, invalidator (*this), boost::bind (&PlugUIBase::update_preset, this), gui_context ()); - plugin->ParameterChanged.connect (*this, invalidator (*this), boost::bind (&PlugUIBase::parameter_changed, this, _1, _2), gui_context ()); + plugin->PresetDirty.connect (*this, invalidator (*this), boost::bind (&PlugUIBase::update_preset_modified, this), gui_context ()); insert->AutomationStateChanged.connect (*this, invalidator (*this), boost::bind (&PlugUIBase::automation_state_changed, this), gui_context()); @@ -814,12 +814,6 @@ PlugUIBase::update_preset_modified () } void -PlugUIBase::parameter_changed (uint32_t, float) -{ - update_preset_modified (); -} - -void PlugUIBase::preset_added_or_removed () { /* Update both the list and the currently-displayed preset */ diff --git a/gtk2_ardour/plugin_ui.h b/gtk2_ardour/plugin_ui.h index 41f4ef39f1..d2e5e21cca 100644 --- a/gtk2_ardour/plugin_ui.h +++ b/gtk2_ardour/plugin_ui.h @@ -170,7 +170,6 @@ class PlugUIBase : public virtual sigc::trackable, public PBD::ScopedConnectionL void processor_active_changed (boost::weak_ptr<ARDOUR::Processor> p); void plugin_going_away (); void automation_state_changed (); - virtual void parameter_changed (uint32_t, float); void preset_added_or_removed (); void update_preset_modified (); diff --git a/libs/ardour/ardour/plugin.h b/libs/ardour/ardour/plugin.h index 2554a6816c..7bef40ab88 100644 --- a/libs/ardour/ardour/plugin.h +++ b/libs/ardour/ardour/plugin.h @@ -208,10 +208,17 @@ class LIBARDOUR_API Plugin : public PBD::StatefulDestructible, public Latent /** Emitted when a preset has been loaded */ PBD::Signal0<void> PresetLoaded; + /** Emitted when a parameter is altered in a way that may have + * changed the settings with respect to any loaded preset. + */ + PBD::Signal0<void> PresetDirty; + virtual bool has_editor () const = 0; - /** Emitted when any parameter changes */ - PBD::Signal2<void, uint32_t, float> ParameterChanged; + /** Emitted when a parameter is altered by something outside of our + * control, most typically a Plugin GUI/editor + */ + PBD::Signal2<void, uint32_t, float> ParameterChangedExternally; virtual bool configure_io (ChanCount /*in*/, ChanCount /*out*/) { return true; } @@ -272,9 +279,18 @@ class LIBARDOUR_API Plugin : public PBD::StatefulDestructible, public Latent protected: friend class PluginInsert; + friend class Session; + /* Called when a parameter of the plugin is changed outside of this + * host's control (typical via a plugin's own GUI/editor) + */ + void parameter_changed_externally (uint32_t which, float val); + + /* should be overridden by plugin API specific derived types to + * actually implement changing the parameter. The derived type should + * call this after the change is made. + */ virtual void set_parameter (uint32_t which, float val); - virtual void set_parameter_automated (uint32_t which, float val); /** Do the actual saving of the current plugin settings to a preset of the provided name. * Should return a URI on success, or an empty string on failure. diff --git a/libs/ardour/ardour/plugin_insert.h b/libs/ardour/ardour/plugin_insert.h index 8788778a3a..d4d9adb54d 100644 --- a/libs/ardour/ardour/plugin_insert.h +++ b/libs/ardour/ardour/plugin_insert.h @@ -96,6 +96,7 @@ class LIBARDOUR_API PluginInsert : public Processor void set_value (double val); double get_value (void) const; + void catch_up_with_external_value (double val); XMLNode& get_state(); private: @@ -164,10 +165,9 @@ class LIBARDOUR_API PluginInsert : public Processor /* disallow copy construction */ PluginInsert (const PluginInsert&); - void parameter_changed (uint32_t, float); + void parameter_changed_externally (uint32_t, float); void set_parameter (Evoral::Parameter param, float val); - float get_parameter (Evoral::Parameter param); float default_parameter_value (const Evoral::Parameter& param); diff --git a/libs/ardour/plugin.cc b/libs/ardour/plugin.cc index f14c56798b..d68502713c 100644 --- a/libs/ardour/plugin.cc +++ b/libs/ardour/plugin.cc @@ -356,19 +356,21 @@ Plugin::clear_preset () PresetLoaded (); /* EMIT SIGNAL */ } -/** @param val `plugin' value */ void -Plugin::set_parameter (uint32_t which, float) +Plugin::set_parameter (uint32_t /* which */, float /* value */) { _parameter_changed_since_last_preset = true; _session.set_dirty (); - ParameterChanged (which, get_parameter (which)); /* EMIT SIGNAL */ + PresetDirty (); /* EMIT SIGNAL */ } void -Plugin::set_parameter_automated (uint32_t which, float val) +Plugin::parameter_changed_externally (uint32_t which, float /* value */) { - Plugin::set_parameter (which, val); + _parameter_changed_since_last_preset = true; + _session.set_dirty (); + ParameterChangedExternally (which, get_parameter (which)); /* EMIT SIGNAL */ + PresetDirty (); /* EMIT SIGNAL */ } int diff --git a/libs/ardour/plugin_insert.cc b/libs/ardour/plugin_insert.cc index 98ff9ab4f0..ec6d86d71a 100644 --- a/libs/ardour/plugin_insert.cc +++ b/libs/ardour/plugin_insert.cc @@ -264,26 +264,53 @@ PluginInsert::create_automatable_parameters () } } } - +/** Called when something outside of this host has modified a plugin + * parameter. Responsible for propagating the change to two places: + * + * 1) anything listening to the Control itself + * 2) any replicated plugins that make up this PluginInsert. + * + * The PluginInsert is connected to the ParameterChangedExternally signal for + * the first (primary) plugin, and here broadcasts that change to any others. + * + * XXX We should probably drop this whole replication idea (Paul, October 2015) + * since it isn't used by sensible plugin APIs (AU, LV2). + */ void -PluginInsert::parameter_changed (uint32_t which, float val) +PluginInsert::parameter_changed_externally (uint32_t which, float val) { boost::shared_ptr<AutomationControl> ac = automation_control (Evoral::Parameter (PluginAutomation, 0, which)); - if (ac) { - ac->set_value (val); + /* First propagation: alter the underlying value of the control, + * without telling the plugin(s) that own/use it to set it. + */ + + if (!ac) { + return; + } - Plugins::iterator i = _plugins.begin(); + boost::shared_ptr<PluginControl> pc = boost::dynamic_pointer_cast<PluginControl> (ac); - /* don't set the first plugin, just all the slaves */ + if (pc) { + pc->catch_up_with_external_value (val); + } - if (i != _plugins.end()) { - ++i; - for (; i != _plugins.end(); ++i) { - (*i)->set_parameter (which, val); - } - } - } + /* Second propagation: tell all plugins except the first to + update the value of this parameter. For sane plugin APIs, + there are no other plugins, so this is a no-op in those + cases. + */ + + Plugins::iterator i = _plugins.begin(); + + /* don't set the first plugin, just all the slaves */ + + if (i != _plugins.end()) { + ++i; + for (; i != _plugins.end(); ++i) { + (*i)->set_parameter (which, val); + } + } } int @@ -507,41 +534,6 @@ PluginInsert::run (BufferSet& bufs, framepos_t start_frame, framepos_t /*end_fra } void -PluginInsert::set_parameter (Evoral::Parameter param, float val) -{ - if (param.type() != PluginAutomation) { - return; - } - - /* the others will be set from the event triggered by this */ - - _plugins[0]->set_parameter (param.id(), val); - - boost::shared_ptr<AutomationControl> ac - = boost::dynamic_pointer_cast<AutomationControl>(control(param)); - - if (ac) { - ac->set_value(val); - } else { - warning << "set_parameter called for nonexistent parameter " - << EventTypeMap::instance().to_symbol(param) << endmsg; - } - - _session.set_dirty(); -} - -float -PluginInsert::get_parameter (Evoral::Parameter param) -{ - if (param.type() != PluginAutomation) { - return 0.0; - } else { - assert (!_plugins.empty ()); - return _plugins[0]->get_parameter (param.id()); - } -} - -void PluginInsert::automation_run (BufferSet& bufs, framepos_t start, pframes_t nframes) { Evoral::ControlEvent next_event (0, 0.0f); @@ -1317,6 +1309,12 @@ PluginInsert::PluginControl::set_value (double user_val) AutomationControl::set_value (user_val); } +void +PluginInsert::PluginControl::catch_up_with_external_value (double user_val) +{ + AutomationControl::set_value (user_val); +} + XMLNode& PluginInsert::PluginControl::get_state () { @@ -1333,8 +1331,13 @@ PluginInsert::PluginControl::get_state () double PluginInsert::PluginControl::get_value () const { - /* FIXME: probably should be taking out some lock here.. */ - return _plugin->get_parameter (_list->parameter()); + boost::shared_ptr<Plugin> plugin = _plugin->plugin (0); + + if (!plugin) { + return 0.0; + } + + return plugin->get_parameter (_list->parameter().id()); } PluginInsert::PluginPropertyControl::PluginPropertyControl (PluginInsert* p, @@ -1430,7 +1433,7 @@ PluginInsert::add_plugin (boost::shared_ptr<Plugin> plugin) /* first (and probably only) plugin instance - connect to relevant signals */ - plugin->ParameterChanged.connect_same_thread (*this, boost::bind (&PluginInsert::parameter_changed, this, _1, _2)); + plugin->ParameterChangedExternally.connect_same_thread (*this, boost::bind (&PluginInsert::parameter_changed_externally, this, _1, _2)); plugin->StartTouch.connect_same_thread (*this, boost::bind (&PluginInsert::start_touch, this, _1)); plugin->EndTouch.connect_same_thread (*this, boost::bind (&PluginInsert::end_touch, this, _1)); } diff --git a/libs/ardour/session_vst.cc b/libs/ardour/session_vst.cc index 9bf2847331..009a4acc40 100644 --- a/libs/ardour/session_vst.cc +++ b/libs/ardour/session_vst.cc @@ -86,7 +86,7 @@ intptr_t Session::vst_callback ( SHOW_CALLBACK ("audioMasterAutomate"); // index, value, returns 0 if (plug) { - plug->set_parameter_automated (index, opt); + plug->parameter_changed_externally (index, opt); } return 0; diff --git a/libs/ardour/source.cc b/libs/ardour/source.cc index 29093035c2..aaa50ff297 100644 --- a/libs/ardour/source.cc +++ b/libs/ardour/source.cc @@ -178,7 +178,7 @@ Source::set_been_analysed (bool yn) yn = false; } } - if (yn != _analysed); { + if (yn != _analysed) { Glib::Threads::Mutex::Lock lm (_analysis_lock); _analysed = yn; } diff --git a/libs/ardour/vst_plugin.cc b/libs/ardour/vst_plugin.cc index 08db7dec5e..1614b1d8fe 100644 --- a/libs/ardour/vst_plugin.cc +++ b/libs/ardour/vst_plugin.cc @@ -117,12 +117,6 @@ VSTPlugin::set_parameter (uint32_t which, float newval) } } -void -VSTPlugin::set_parameter_automated (uint32_t which, float newval) -{ - Plugin::set_parameter_automated (which, newval); -} - uint32_t VSTPlugin::nth_parameter (uint32_t n, bool& ok) const { |