From b084036c87b57c61eb80a6a4d345b8bf9b0c4857 Mon Sep 17 00:00:00 2001 From: Robin Gareus Date: Fri, 20 Jan 2017 00:33:44 +0100 Subject: Fix oddities when replacing VST-presets. 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 | 2 -- libs/ardour/luaproc.cc | 3 +++ libs/ardour/plugin.cc | 26 +++++++++----------------- libs/ardour/vst_plugin.cc | 20 ++++++++++++++++++-- 4 files changed, 30 insertions(+), 21 deletions(-) (limited to 'libs') diff --git a/libs/ardour/ardour/plugin.h b/libs/ardour/ardour/plugin.h index 374dca82c2..56d19a0cb4 100644 --- a/libs/ardour/ardour/plugin.h +++ b/libs/ardour/ardour/plugin.h @@ -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; diff --git a/libs/ardour/luaproc.cc b/libs/ardour/luaproc.cc index 436f60bfc4..4bc2c00c8b 100644 --- a/libs/ardour/luaproc.cc +++ b/libs/ardour/luaproc.cc @@ -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")); diff --git a/libs/ardour/plugin.cc b/libs/ardour/plugin.cc index d21ecfe632..35495488dd 100644 --- a/libs/ardour/plugin.cc +++ b/libs/ardour/plugin.cc @@ -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::get_presets () { diff --git a/libs/ardour/vst_plugin.cc b/libs/ardour/vst_plugin.cc index a3dca8b3fa..9ebc80fd83 100644 --- a/libs/ardour/vst_plugin.cc +++ b/libs/ardour/vst_plugin.cc @@ -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 */) { -- cgit v1.2.3