rename Plugin::ParameterChanged to ParameterChangedExternally to reflect its intent...
authorPaul Davis <paul@linuxaudiosystems.com>
Tue, 20 Oct 2015 14:12:21 +0000 (10:12 -0400)
committerPaul Davis <paul@linuxaudiosystems.com>
Tue, 20 Oct 2015 14:23:49 +0000 (10:23 -0400)
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.

libs/ardour/ardour/plugin.h
libs/ardour/ardour/plugin_insert.h
libs/ardour/audio_unit.cc
libs/ardour/plugin.cc
libs/ardour/plugin_insert.cc
libs/ardour/session_vst.cc
libs/ardour/source.cc
libs/ardour/vst_plugin.cc

index 2554a6816c01a5e6fa94f6623d6152c29a73ff3c..7bef40ab88814dc28601a4e4b931bd8d45578e3b 100644 (file)
@@ -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.
index 8788778a3a5a8f0177b618e034b22483d72835fa..d4d9adb54dc6a8fc2b521b5b82d2de6268400888 100644 (file)
@@ -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);
 
index 600a6fecbbb6815df66f257dcc3767ca7a1266e9..045669b29bcb25a540e0604a1ed3fb7cd406a286 100644 (file)
@@ -924,7 +924,7 @@ AUPlugin::set_parameter (uint32_t which, float val)
        theEvent.mArgument.mParameter.mElement = d.element;
 
        DEBUG_TRACE (DEBUG::AudioUnits, "notify about parameter change\n");
-       AUEventListenerNotify (NULL, NULL, &theEvent);
+       AUEventListenerNotify (NULL, this, &theEvent);
 
        Plugin::set_parameter (which, val);
 }
@@ -3078,7 +3078,7 @@ AUPlugin::_parameter_change_listener (void* arg, void* src, const AudioUnitEvent
 }
 
 void
-AUPlugin::parameter_change_listener (void* /*arg*/, void* /*src*/, const AudioUnitEvent* event, UInt64 /*host_time*/, Float32 new_value)
+AUPlugin::parameter_change_listener (void* /*arg*/, void* src, const AudioUnitEvent* event, UInt64 /*host_time*/, Float32 new_value)
 {
         ParameterMap::iterator i;
 
@@ -3094,7 +3094,12 @@ AUPlugin::parameter_change_listener (void* /*arg*/, void* /*src*/, const AudioUn
                 EndTouch (i->second);
                 break;
         case kAudioUnitEvent_ParameterValueChange:
-                ParameterChanged (i->second, new_value);
+               if (src != this) {
+                       std::cerr << "something changed " << i->second << " to " << new_value << std::endl;
+                       ParameterChangedExternally (i->second, new_value);
+               } else {
+                       std::cerr << "plugin changed " << i->second << " ignore it\n";
+               }
                 break;
         default:
                 break;
index f14c56798b65e4cb404c8cf6b8162037baa90fdf..d68502713c840caa2c63df731c5ed2302d53d91d 100644 (file)
@@ -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
index 98ff9ab4f0983040e1fc1eb0d49a42788dfab612..ec6d86d71a76c66094fdba48db8e72065af82033 100644 (file)
@@ -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
@@ -506,41 +533,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)
 {
@@ -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));
        }
index 9bf28473317e556166a68888988586d418144e29..009a4acc40a6e5a2c21fd8eddf3b8f745d589f70 100644 (file)
@@ -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;
 
index 29093035c2c62d4c81607cd8dab94e493dbed1b3..aaa50ff2973aaaa79039011f7c3e043c14dc247a 100644 (file)
@@ -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;
        }
index 08db7dec5e56fa18d533a366a13dd6f9377e51bf..1614b1d8fe394cc6a6ee62a6c9e9737a91347aec 100644 (file)
@@ -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
 {