On 27.01.25 10:04, Kristian Amlie wrote:
So why don't we do this: Basically we save the configuration exactly as
Hermann describes, except instead of saving it to "instance-0", we save it
to "LV2-default" (or some other suitable name). And new LV2 instances also
pull their defaults from here.
On 27.01.25 18:53, ichthyo wrote:
This seems like a much better solution for me!
...
We'd have to extract the build-up of the file name, which is currently
done interwoven with the other config handling. But besides that, this
would be a really minimal change and would solve all the problems,
by addressing the root cause: essentially we are creating a new kind
of "instance", which is a LV2 instance, and separate from standalone
instance.


This morning I was doubtful if the above description is clear enough,
and -- moreover -- if it is really this simple.

Thus I tried with an experimental patch. See attached.


Findings:

- indeed it works and is basically simple to implement

- the bulk of the changes was to comb through the codebase and to
  find all those "other locations" where the config location was
  yet again built from scratch, instead of using the member fields
  of the Config object. Hopefully I haven't missed anything.

- deliberately I've left the *window* files allone, so they still
  use the old identification scheme

- one /relevant necessary change/ is in InstanceManager::Instance::startUp()
  There we need to set the Config::isLV2 flag earlier, since it is now
  already evaluated when bootstrapping the config, before we start the instance.


And now comes the Fun Fact:

The implementation of getalldata / putalldata is different than we
assumed in our discussion, insofar it *does not include the config*
ROFL!!!
It was so from the first versions when Andrew added state support for LV2.

Thus, the state handling in LV2 is not as it should be, because the
config settings for the plugin-instances are not captured. Rather, this
data is always loaded from the instance config in the user's setup.


Of course,the results of our discussion seem to imply that this should
be changed, and getalldata() / putalldata() should better delegate to
an implementation in the Config-object, like Config::saveSessionData()

I have not done that in the patch, but for a quick experiment
I added temporarily direct calls to the config save/load functions
into getalldata() / putalldata().
And then everyhing works as intended!

-- Hermann


PS: this is just meant as a proof-of concept or reference-point
for further discussion. This patch is also in my Github-Repo, master.
From a1854b2bbea23a91b9589231aa4f5e7fc8f08ec1 Mon Sep 17 00:00:00 2001
From: Ichthyostega <p...@ichthyostega.de>
Date: Tue, 28 Jan 2025 06:22:49 +0100
Subject: [PATCH] Config-LV2: keep instance config for LV2 apart from
 standalone instances

This is implemented by changing the way how instance-config files are located.
- for standalone : use the Synth-ID as key (as before)
- for LV2 : use a fixed string "LV2"

This implies that now a LV2-plugin instance does no longer _alias_ to some
standalone instance, which just happened to use the same Synth-ID.
Rather, all LV2-plugin instances now use a single common instance-config.

Note: usually the LV2 host pulls state data from each plug-in and
later stores this state-data internally, in the Host-Session.
This way, typically each plugin-instance has its own, private
patch-state.

But the base config and instance config is relevant for
default values when creating a new instance, and it can be
edited through the Config-UI in Yoshimi.


This changeset rearranges the code how the config-location is established,
and extracts it into a new helper function Config::buildConfigLocation()

The actual logic is only altered at a single point: when the instance-ID is generated.

The bulk of the further changes just eliminated directly generated config-file locations
to reuse this central helper function and the Config member fields instead.


Note: we need to set the Config::isLV2 flag earlier, because it is now
evaluated already when populating the config. We set this flag now
already from InstanceManager::Instance::startUp(pluginCreator),
i.e. before the Plugin-Instance itself is constructed.
---
 src/Interface/InterChange.cpp | 12 ++++------
 src/Misc/CmdOptions.cpp       |  2 +-
 src/Misc/Config.cpp           | 44 +++++++++++++++++++----------------
 src/Misc/Config.h             |  1 +
 src/Misc/FileMgrFuncs.h       |  3 +++
 src/Misc/InstanceManager.cpp  |  3 ++-
 src/Misc/SynthEngine.cpp      |  6 ++---
 src/Misc/XMLwrapper.cpp       |  2 +-
 src/UI/MasterUI.fl            |  6 ++---
 9 files changed, 41 insertions(+), 38 deletions(-)

diff --git a/src/Interface/InterChange.cpp b/src/Interface/InterChange.cpp
index b90bebd2..7da1d889 100644
--- a/src/Interface/InterChange.cpp
+++ b/src/Interface/InterChange.cpp
@@ -861,10 +861,8 @@ int InterChange::indirectMain(CommandBlock& cmd, uchar &newMsg, bool &guiTo, str
             if (synth.loadStateAndUpdate(text))
             {
                 text = setExtension(text, EXTEN::state);
-                string name = file::configDir() + string(YOSHIMI);
-                name += ("-" + to_string(synth.getUniqueId()));
-                name += ".state";
-                if ((text != name)) // never include default state
+                string defaultName = synth.getRuntime().defaultSession;
+                if ((text != defaultName)) // never include default state
                     synth.addHistory(text, TOPLEVEL::XML::State);
                 text = "ed " + text;
             }
@@ -878,10 +876,8 @@ int InterChange::indirectMain(CommandBlock& cmd, uchar &newMsg, bool &guiTo, str
             string filename = setExtension(text, EXTEN::state);
             if (synth.saveState(filename))
             {
-                string name = file::configDir() + string(YOSHIMI);
-                name += ("-" + to_string(synth.getUniqueId()));
-                name += ".state";
-                if ((text != name)) // never include default state
+                string defaultName = synth.getRuntime().defaultSession;
+                if ((text != defaultName)) // never include default state
                     synth.addHistory(filename, TOPLEVEL::XML::State);
                 text = "d " + text;
             }
diff --git a/src/Misc/CmdOptions.cpp b/src/Misc/CmdOptions.cpp
index 2ce309a5..6e15664d 100644
--- a/src/Misc/CmdOptions.cpp
+++ b/src/Misc/CmdOptions.cpp
@@ -43,7 +43,7 @@ namespace { // constants used in the implementation
         "Copyright 2012-2013 Jeremy Jongepier and others,\n"
         "Copyright 2014-2025 Will Godfrey and others";
 
-    string stateText = "load saved state, defaults to '$HOME/" + EXTEN::config + "/yoshimi/yoshimi-0.state'";
+    string stateText = "load saved state, defaults to '$HOME/" + EXTEN::config + "/yoshimi/yoshimi-0"+EXTEN::state+"'";
 
     const argp_option OPTION_SPEC[] = {
         {"alsa-audio",        'A',  "<device>", OPTION_ARG_OPTIONAL, "use alsa audio output", 0},
diff --git a/src/Misc/Config.cpp b/src/Misc/Config.cpp
index bb32385b..80e41283 100644
--- a/src/Misc/Config.cpp
+++ b/src/Misc/Config.cpp
@@ -359,6 +359,21 @@ void Config::loadConfig()
     }
 }
 
+
+void Config::buildConfigLocation()
+{
+    string location = file::configDir();
+    string instanceID = isLV2? file::LV2_INSTANCE                 // LV2-plugin uses a fixed key for instance config
+                             : asString(synth.getUniqueId());     // standalone-instances are keyed by Synth-ID
+
+    defaultStateName = location + "/" + YOSHIMI;
+    baseConfig       = location + "/" + YOSHIMI                    + EXTEN::config;
+    configFile       = location + "/" + YOSHIMI + "-" + instanceID + EXTEN::instance;
+    defaultSession   = location + "/" + YOSHIMI + "-" + instanceID + EXTEN::state;
+
+    presetDir = file::localDir() + "/presets";
+}
+
 bool Config::initFromPersistentConfig()
 {
     if (file::userHome() == "/tmp")
@@ -368,29 +383,18 @@ bool Config::initFromPersistentConfig()
         Log("Failed to create local yoshimi directory.");
         return false;
     }
-    string foundConfig = file::configDir();
-    defaultStateName = foundConfig + "/yoshimi";
-
     if (file::configDir().empty())
     {
         Log("Failed to create config directory '" + file::userHome() + "'");
         return false;
     }
-    string yoshimi = "/" + string(YOSHIMI);
-
-    baseConfig = foundConfig + yoshimi + string(EXTEN::config);
-
-    int currentInstance = synth.getUniqueId();
-    defaultSession = defaultStateName + "-" + asString(currentInstance) + EXTEN::state;
-    yoshimi += ("-" + asString(currentInstance));
 
-    configFile = foundConfig + yoshimi + EXTEN::instance;
+    buildConfigLocation();
 
-    if (currentInstance == 0 && sessionStage != _SYS_::type::RestoreConf)
+    if (synth.getUniqueId() == 0 && sessionStage != _SYS_::type::RestoreConf)
     {
         TextMsgBuffer::instance().init(); // sneaked it in here so it's early
 
-        presetDir = file::localDir() + "/presets";
         if (!isDirectory(presetDir))
         { // only ever want to do this once
             if (createDir(presetDir))
@@ -416,15 +420,16 @@ bool Config::initFromPersistentConfig()
 
         // conversion for old location
         string newInstance0 = configFile;
+        string oldAllConfig = defaultStateName + EXTEN::state;
         if (isRegularFile(baseConfig) && !isRegularFile(newInstance0), 0)
         {
             file::copyFile(baseConfig, newInstance0, 0);
             Log("Reorganising config files.");
-            if (isRegularFile(defaultStateName + EXTEN::state))
+            if (isRegularFile(oldAllConfig))
             {
                 if (!isRegularFile(defaultSession))
                 {
-                    renameFile(defaultStateName + EXTEN::state, defaultSession);
+                    renameFile(oldAllConfig, defaultSession);
                     Log("Moving default state file.");
                 }
             }
@@ -473,7 +478,7 @@ bool Config::initFromPersistentConfig()
         else
             Log("loadConfig load instance failed");
     }
-    if (currentInstance == 0 && sessionStage != _SYS_::type::RestoreConf)
+    if (synth.getUniqueId() == 0 && sessionStage != _SYS_::type::RestoreConf)
     {
         int currentVersion = lastXMLmajor * 10 + lastXMLminor;
         int storedVersion = MIN_CONFIG_MAJOR * 10 + MIN_CONFIG_MINOR;
@@ -509,7 +514,7 @@ bool Config::initFromPersistentConfig()
     if (success)
         loadPresetsList();
 
-    if (success && currentInstance == 0)
+    if (success && synth.getUniqueId() == 0)
     {
         // find user guide
         bool man_ok = false;
@@ -551,6 +556,7 @@ bool Config::updateConfig(int control, int value)
      * in the correct range as they otherwise couldn't have been created.
      */
 
+    buildConfigLocation();
     bool success{false};
     if (control <= CONFIG::control::XMLcompressionLevel)
     {// handling base config
@@ -610,10 +616,8 @@ bool Config::updateConfig(int control, int value)
     {// handling current session config
         const int offset = CONFIG::control::defaultStateStart;
         const int arraySize = CONFIG::control::historyLock - offset;
-        const string instance = asString(synth.getUniqueId());
 
         xmlType = TOPLEVEL::XML::Config;
-        string configFile = file::configDir() + "/yoshimi-" + instance + string(EXTEN::instance);
         int configData[arraySize]; // historyLock is handled elsewhere
         auto xml{std::make_unique<XMLwrapper>(synth, true)};
         success = xml->loadXMLfile(configFile);
@@ -714,7 +718,7 @@ bool Config::updateConfig(int control, int value)
         }
         else
         {
-            Log("loadConfig load instance config" + instance + " failed");
+            Log("loadConfig load instance config" + configFile + " failed");
         }
     }
     return success;
diff --git a/src/Misc/Config.h b/src/Misc/Config.h
index 75f47f99..7224321d 100644
--- a/src/Misc/Config.h
+++ b/src/Misc/Config.h
@@ -268,6 +268,7 @@ class Config
         pthread_t  findManual_Thread;
 
         void defaultPresets();
+        void buildConfigLocation();
         bool initFromPersistentConfig();
         bool extractBaseParameters(XMLwrapper& xml);
         bool extractConfigData(XMLwrapper& xml);
diff --git a/src/Misc/FileMgrFuncs.h b/src/Misc/FileMgrFuncs.h
index 41a012b9..a911b928 100644
--- a/src/Misc/FileMgrFuncs.h
+++ b/src/Misc/FileMgrFuncs.h
@@ -85,6 +85,9 @@ namespace file {
 using std::string;
 using std::stringstream;
 
+// Marker used for instance config when started as LV2 plugin
+const string LV2_INSTANCE = "LV2";
+
 // make a filename legal
 inline void make_legit_filename(string& fname)
 {
diff --git a/src/Misc/InstanceManager.cpp b/src/Misc/InstanceManager.cpp
index 769c16d9..579f9437 100644
--- a/src/Misc/InstanceManager.cpp
+++ b/src/Misc/InstanceManager.cpp
@@ -300,8 +300,9 @@ bool InstanceManager::Instance::startUp(PluginCreator pluginCreator)
 {
     cout << "\nStart-up Synth-Instance("<< getID() << ")..."<< endl;
     state = BOOTING;
-    runtime().loadConfig();
     bool isLV2 = bool(pluginCreator);
+    runtime().isLV2 = isLV2;
+    runtime().loadConfig();
     assert (not runtime().runSynth);
     if (isLV2)
     {
diff --git a/src/Misc/SynthEngine.cpp b/src/Misc/SynthEngine.cpp
index df3b7129..b27526fe 100644
--- a/src/Misc/SynthEngine.cpp
+++ b/src/Misc/SynthEngine.cpp
@@ -1992,8 +1992,8 @@ void SynthEngine::resetAll(bool andML)
     ClearNRPNs();
     if (Runtime.loadDefaultState)
     {
-        string filename = Runtime.defaultStateName + ("-" + to_string(this->getUniqueId()));
-        if (isRegularFile(filename + ".state"))
+        string filename = Runtime.defaultSession;
+        if (isRegularFile(filename))
         {
             Runtime.stateFile = filename;
             Runtime.restoreSessionData(Runtime.stateFile);
@@ -2773,7 +2773,7 @@ bool SynthEngine::loadHistory()
     string historyname = file::localDir()  + "/recent";
     if (!isRegularFile(historyname))
     {   // recover old version
-        historyname = file::configDir() + '/' + string(YOSHIMI) + ".history";
+        historyname = file::configDir() + '/' + YOSHIMI + ".history";
         if (!isRegularFile(historyname))
         {
             Runtime.Log("Missing recent history file");
diff --git a/src/Misc/XMLwrapper.cpp b/src/Misc/XMLwrapper.cpp
index 7fa2d9e3..051e3a8c 100644
--- a/src/Misc/XMLwrapper.cpp
+++ b/src/Misc/XMLwrapper.cpp
@@ -579,7 +579,7 @@ bool XMLwrapper::loadXMLfile(string const& filename)
     else
         synth.getRuntime().lastXMLminor = 0;
     string exten = findExtension(filename);
-    if (exten.length() != 4 && exten != ".state")
+    if (exten.length() != 4 && exten != EXTEN::state)
         return true; // we don't want config stuff
 
     if (synth.getRuntime().logXMLheaders)
diff --git a/src/UI/MasterUI.fl b/src/UI/MasterUI.fl
index 01ecf2c8..d6b94f38 100644
--- a/src/UI/MasterUI.fl
+++ b/src/UI/MasterUI.fl
@@ -719,10 +719,8 @@ class MasterUI {: {public GuiUpdates}
             break;
         case 29: // default
             {
-            string name = synth->getRuntime().defaultStateName;
-            name += ("-" + to_string(current_ID));
-            send_data(TOPLEVEL::action::lowPrio | TOPLEVEL::action::forceUpdate, MAIN::control::saveNamedState, 0, TOPLEVEL::type::Integer, TOPLEVEL::section::main, UNUSED, UNUSED, UNUSED, textMsgBuffer.push(name + ".state"));
-
+                string name = synth->getRuntime().defaultSession;
+                send_data(TOPLEVEL::action::lowPrio | TOPLEVEL::action::forceUpdate, MAIN::control::saveNamedState, 0, TOPLEVEL::type::Integer, TOPLEVEL::section::main, UNUSED, UNUSED, UNUSED, textMsgBuffer.push(name));
             }
             break;
         case 30: // recent
-- 
2.20.1

_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel

Reply via email to