Fix oddities when replacing VST-presets.
authorRobin Gareus <robin@gareus.org>
Thu, 19 Jan 2017 23:33:44 +0000 (00:33 +0100)
committerRobin Gareus <robin@gareus.org>
Thu, 19 Jan 2017 23:50:14 +0000 (00:50 +0100)
VST used the count of available of presets as URI:
 - add 2 presets (1,2)
 - remove first, add another one -> two presets with same URI (2,2)

PluginInfo::get_presets() simply lists all (name only) in a vector.
Plugin::find_presets() uses the URI in a map (unique by URI).

..various ensuing bugs: eg. Plugin::remove_preset() looked up by name,
but didn't check for NULL.

libs/ardour/ardour/plugin.h
libs/ardour/luaproc.cc
libs/ardour/plugin.cc
libs/ardour/vst_plugin.cc

index 374dca82c298f8cfd74531b7a46199d9b05b029f..56d19a0cb4f583e08c425f427433d62a3c77d92c 100644 (file)
@@ -358,8 +358,6 @@ private:
        /** Fill _presets with our presets */
        virtual void find_presets () = 0;
 
-       void update_presets (std::string src_unique_id, Plugin* src );
-
        /** Add state to an existing XMLNode */
        virtual void add_state (XMLNode *) const = 0;
 
index 436f60bfc47ac1bf969518bb12bfa8ae6c5def16..4bc2c00c8ba2e836533fadcddedb544ce4616d06 100644 (file)
@@ -1141,6 +1141,9 @@ LuaProc::do_save_preset (std::string name) {
                return "";
        }
 
+       // prevent dups -- just in case
+       t->root()->remove_nodes_and_delete (X_("label"), name);
+
        std::string uri (preset_name_to_uri (name));
 
        XMLNode* p = new XMLNode (X_("Preset"));
index d21ecfe632f4d2849f7451d0a2ac9cf168ab98d4..35495488dd3ef432022abca0df226b6f34017f49 100644 (file)
@@ -102,7 +102,6 @@ Plugin::Plugin (AudioEngine& e, Session& s)
        , _parameter_changed_since_last_preset (false)
 {
        _pending_stop_events.ensure_buffers (DataType::MIDI, 1, 4096);
-       PresetsChanged.connect_same_thread (_preset_connection, boost::bind (&Plugin::update_presets, this, _1 ,_2));
 }
 
 Plugin::Plugin (const Plugin& other)
@@ -117,7 +116,6 @@ Plugin::Plugin (const Plugin& other)
        , _parameter_changed_since_last_preset (false)
 {
        _pending_stop_events.ensure_buffers (DataType::MIDI, 1, 4096);
-       PresetsChanged.connect_same_thread (_preset_connection, boost::bind (&Plugin::update_presets, this, _1 ,_2));
 }
 
 Plugin::~Plugin ()
@@ -128,18 +126,23 @@ void
 Plugin::remove_preset (string name)
 {
        Plugin::PresetRecord const * p = preset_by_label (name);
+       if (!p) {
+               PBD::error << _("Trying to remove nonexistent preset.") << endmsg;
+               return;
+       }
        if (!p->user) {
                PBD::error << _("Cannot remove plugin factory preset.") << endmsg;
                return;
        }
 
        do_remove_preset (name);
-       _presets.erase (preset_by_label (name)->uri);
+       _presets.erase (p->uri);
 
        _last_preset.uri = "";
        _parameter_changed_since_last_preset = false;
-       PresetRemoved (); /* EMIT SIGNAL */
+       _have_presets = false;
        PresetsChanged (unique_id(), this); /* EMIT SIGNAL */
+       PresetRemoved (); /* EMIT SIGNAL */
 }
 
 /** @return PresetRecord with empty URI on failure */
@@ -155,8 +158,9 @@ Plugin::save_preset (string name)
 
        if (!uri.empty()) {
                _presets.insert (make_pair (uri, PresetRecord (uri, name)));
-               PresetAdded (); /* EMIT SIGNAL */
+               _have_presets = false;
                PresetsChanged (unique_id(), this); /* EMIT SIGNAL */
+               PresetAdded (); /* EMIT SIGNAL */
        }
 
        return PresetRecord (uri, name);
@@ -393,18 +397,6 @@ Plugin::resolve_midi ()
        _have_pending_stop_events = true;
 }
 
-void
-Plugin::update_presets (std::string src_unique_id, Plugin* src )
-{
-       if (src == this || unique_id() != src_unique_id) {
-               return;
-       }
-       _have_presets = false;
-       // TODO check if a preset was added/removed and emit the proper signal
-       // so far no subscriber distinguishes between PresetAdded and PresetRemoved
-       PresetAdded();
-}
-
 vector<Plugin::PresetRecord>
 Plugin::get_presets ()
 {
index a3dca8b3fa2e3d1cfe2e6ba58b899e44b2cfb17e..9ebc80fd83b7e364e081f397c88ed48ae658b75e 100644 (file)
@@ -456,6 +456,8 @@ VSTPlugin::load_user_preset (PresetRecord r)
        return false;
 }
 
+#include "sha1.c"
+
 string
 VSTPlugin::do_save_preset (string name)
 {
@@ -464,9 +466,23 @@ VSTPlugin::do_save_preset (string name)
                return "";
        }
 
+       // prevent dups -- just in case
+       t->root()->remove_nodes_and_delete (X_("label"), name);
+
        XMLNode* p = 0;
-       /* XXX: use of _presets.size() + 1 for the unique ID here is dubious at best */
-       string const uri = string_compose (X_("VST:%1:%2"), unique_id (), _presets.size() + 1);
+
+       char tmp[32];
+       snprintf (tmp, 31, "%d", _presets.size() + 1);
+       tmp[31] = 0;
+
+       char hash[41];
+       Sha1Digest s;
+       sha1_init (&s);
+       sha1_write (&s, (const uint8_t *) name.c_str(), name.size ());
+       sha1_write (&s, (const uint8_t *) tmp, strlen(tmp));
+       sha1_result_hash (&s, hash);
+
+       string const uri = string_compose (X_("VST:%1:x%2"), unique_id (), hash);
 
        if (_plugin->flags & 32 /* effFlagsProgramsChunks */) {