On 29.01.25 09:59, Will Godfrey wrote:
Well!
That is a surprise. I had always thought it was included. Good catch.
Mind you, the whole of config is like a rabbit warren and pretty much
always has been :(
The only thing I would ask, is to leave the windows as they are for now.
I'm working through the idea of embedding them in the structure so they
can be managed from the CLI as well as directly in the GUI
(and any other input source we may later add).
Fully agreed. If we indeed expand this changed instance config logic also to the
windows, then it would be more or less a prerequisite that the state files for
LV2 also contain this window data, otherwise users would get a mix-up of the
window states from several plugin instances.
So it seems prudent to leave the old filename logic still intact for the
window files for the time being.
Regarding the state loading, I have tried to bring both variants
together and make them identical, functionality-wise. See the attached patch.
What do you think? do these changes seem reasonable?
Basically
- relocate the getalldata() / putalldata() into the config obeject
- then compare with the saveSessionData() / restoreSessionData()
- and extract common shared base operations to use for both cases
As an aside, I noticed the following
- there are two currently unused variables, which are possibly a leftover
from earlier versions of the state handling; they were set only in this
compound of state saving functions and were never read from anywhere else,
Config::stateChanged
SynthEngine::usingYoshiType
- there was a mismatch in the config loading logic, so that in the case when
there is a valid base config, but no instance config found, then no config
would be loaded at all. Obviously we want to load a base config if it is
available. This can easily be fixed by reordering two else-branches.
-- Hermann
PS: both the previous patch and the new patch can also be found in my github
repo https://github.com/Ichthyostega/yoshimi.git, at the tip of branch master.
From 4679b3163d1525d0c5b820bad60e7794a8ee02ca Mon Sep 17 00:00:00 2001
From: Ichthyostega <p...@ichthyostega.de>
Date: Thu, 30 Jan 2025 07:15:18 +0100
Subject: [PATCH] Config-LV2: use the same patch-state handling for LV2 and
standalone
The state saving for LV2 was introduced with
98d8b39f7b9749
It used its own implementation, residing in the SynthEngine;
With the later rework of config handling in general, there are now
two implementations for the same functionality, and in fact their
behaviour differs: the LV2 instance does not retrieve/populate the
instance config part; this can lead to sublte and possibly dangerous
errors when restoring a LV2 session on top of a different instance config
or even in a new setup with default values.
With this change, both use cases (state file and LV2 state) will now
invoke handlers in the Config object, both delegating to the same
core implementation sequence.
As an aside, this patch also fixes the following
- remove unused variables Config::stateChanged and SynthEngine::usingYoshiType
- fix the handling of missing config; one possible scenario is that a base config
is found, but no instance config file is present. In this case, the base file
should always be loaded, and only the instance file needs to be regenerated
---
src/LV2_Plugin/YoshimiLV2Plugin.cpp | 4 +-
src/Misc/Config.cpp | 118 ++++++++++++++++++----------
src/Misc/Config.h | 7 +-
src/Misc/Part.cpp | 1 -
src/Misc/SynthEngine.cpp | 41 ----------
src/Misc/SynthEngine.h | 4 -
6 files changed, 84 insertions(+), 91 deletions(-)
diff --git a/src/LV2_Plugin/YoshimiLV2Plugin.cpp b/src/LV2_Plugin/YoshimiLV2Plugin.cpp
index f73f91c2..fc890c10 100644
--- a/src/LV2_Plugin/YoshimiLV2Plugin.cpp
+++ b/src/LV2_Plugin/YoshimiLV2Plugin.cpp
@@ -542,7 +542,7 @@ LV2_State_Status YoshimiLV2Plugin::stateSave(LV2_State_Store_Function store, LV2
// suppress warnings - may use later
char *data = NULL;
- int sz = synth.getalldata(&data);
+ int sz = runtime().saveSessionData(&data);
store(handle, _yoshimi_state_id, data, sz, _atom_string_id, LV2_STATE_IS_POD | LV2_STATE_IS_PORTABLE);
free(data);
@@ -564,7 +564,7 @@ LV2_State_Status YoshimiLV2Plugin::stateRestore(LV2_State_Retrieve_Function retr
const char *data = (const char *)retrieve(handle, _yoshimi_state_id, &sz, &type, &new_flags);
if (sz > 0)
- synth.putalldata(data, sz);
+ runtime().restoreSessionData(data, sz);
return LV2_STATE_SUCCESS;
}
diff --git a/src/Misc/Config.cpp b/src/Misc/Config.cpp
index 80e41283..b7db8d2b 100644
--- a/src/Misc/Config.cpp
+++ b/src/Misc/Config.cpp
@@ -93,7 +93,6 @@ Config::Config(SynthEngine& synthInstance)
, build_ID{BUILD_NUMBER}
, lastXMLmajor{0}
, lastXMLminor{0}
- , stateChanged{false}
, oldConfig{false}
, runSynth{false} // will be set by Instance::startUp()
, finishedCLI{true}
@@ -436,14 +435,24 @@ bool Config::initFromPersistentConfig()
}
}
+ bool success{true};
if (!isRegularFile(baseConfig))
{
Log("Basic configuration " + baseConfig + " not found, will use default settings.");
defaultPresets();
saveMasterConfig(); // generates a pristine "yoshimi.config"
}
+ else
+ {
+ // load baseConfig (always from the primary file)
+ auto xml{std::make_unique<XMLwrapper>(synth, true)};
+ success = xml->loadXMLfile(baseConfig); // note: we want correct base values even in a secondary config instance
+ if (success)
+ success = extractBaseParameters(*xml);
+ else
+ Log("loadConfig load base failed");
+ }
- bool success{false};
if (!isRegularFile(configFile))
{
if (0 < synth.getUniqueId())
@@ -458,19 +467,9 @@ bool Config::initFromPersistentConfig()
saveInstanceConfig(); // generates a new "yoshimi-#.instance"
}
else
- {
- // load baseConfig (always from the primary file)
- auto xml{std::make_unique<XMLwrapper>(synth, true)};
- success = xml->loadXMLfile(baseConfig); // note: we want correct base values even in a secondary config instance
- if (success)
- success = extractBaseParameters(*xml);
- else
- Log("loadConfig load base failed");
- }
-
if (success)
{
- // the instance data
+ // load instance configuration values
auto xml{std::make_unique<XMLwrapper>(synth, true)};
success = xml->loadXMLfile(configFile);
if (success)
@@ -478,6 +477,7 @@ bool Config::initFromPersistentConfig()
else
Log("loadConfig load instance failed");
}
+
if (synth.getUniqueId() == 0 && sessionStage != _SYS_::type::RestoreConf)
{
int currentVersion = lastXMLmajor * 10 + lastXMLminor;
@@ -981,57 +981,93 @@ void Config::addConfigXML(XMLwrapper& xml)
}
-bool Config::saveSessionData(string savefile)
+/**
+ * Extract current instance config and complete patch state from the engine,
+ * encode it as XML and write to a _state file_
+ */
+bool Config::saveSessionData(string sessionfile)
{
- savefile = setExtension(savefile, EXTEN::state);
+ sessionfile = setExtension(sessionfile, EXTEN::state);
xmlType = TOPLEVEL::XML::State;
auto xml{std::make_unique<XMLwrapper>(synth, true)};
- bool success{false};
- addConfigXML(*xml);
- synth.add2XML(*xml);
- synth.midilearn.insertMidiListData(*xml);
- if (xml->saveXMLfile(savefile))
- {
- Log("Session data saved to " + savefile, _SYS_::LogNotSerious);
- success = true;
- }
- else
- Log("Failed to save session data to " + savefile, _SYS_::LogNotSerious);
+ capturePatchState(*xml);
+
+ bool success = xml->saveXMLfile(sessionfile);
+ if (success)
+ Log("Session data saved to " + sessionfile, _SYS_::LogNotSerious);
+ else
+ Log("Failed to save session data to " + sessionfile, _SYS_::LogNotSerious);
return success;
}
+/** Variation to extract config and patch state for LV2 */
+int Config::saveSessionData(char** dataBuffer)
+{
+ xmlType = TOPLEVEL::XML::State;
+ auto xml{std::make_unique<XMLwrapper>(synth, true)};
-bool Config::restoreSessionData(string sessionfile)
+ capturePatchState(*xml);
+
+ *dataBuffer = xml->getXMLdata();
+ return strlen(*dataBuffer) + 1;
+}
+
+void Config::capturePatchState(XMLwrapper& xml)
{
- bool success{false};
+ addConfigXML(xml);
+ synth.add2XML(xml);
+ synth.midilearn.insertMidiListData(xml);
+}
+
+/**
+ * Read configuration and patch state from XML state file
+ * and overwrite config and engine settings with these values.
+ */
+bool Config::restoreSessionData(string sessionfile)
+{
if (sessionfile.size() && !isRegularFile(sessionfile))
sessionfile = setExtension(sessionfile, EXTEN::state);
if (!sessionfile.size() || !isRegularFile(sessionfile))
- {
Log("Session file " + sessionfile + " not available", _SYS_::LogNotSerious);
- return false;
- }
- auto xml{std::make_unique<XMLwrapper>(synth, true)};
- if (!xml->loadXMLfile(sessionfile))
+ else
{
- Log("Failed to load xml file " + sessionfile, _SYS_::LogNotSerious);
- return false;
+ auto xml{std::make_unique<XMLwrapper>(synth, true)};
+ if (!xml->loadXMLfile(sessionfile))
+ Log("Failed to load xml file " + sessionfile, _SYS_::LogNotSerious);
+ else
+ return restorePatchState(*xml);
}
+ return false;
+}
+/** Variation to retrieve patch state and config from the LV2 host */
+bool Config::restoreSessionData(const char* dataBuffer, int size)
+{
+ (void)size; // currently unused
- success = extractConfigData(*xml);
+ while (isspace(*dataBuffer))
+ ++dataBuffer;
+ auto xml{std::make_unique<XMLwrapper>(synth, true)};
+ if (!xml->putXMLdata(dataBuffer))
+ Log("SynthEngine: putXMLdata failed");
+ else
+ return restorePatchState(*xml);
+
+ return false;
+}
+
+bool Config::restorePatchState(XMLwrapper& xml)
+{
+ bool success = extractConfigData(xml);
if (success)
{
- // mark as soon as anything changes
- this->stateChanged = true;
-
synth.defaults();
- success = synth.getfromXML(*xml);
+ success = synth.getfromXML(xml);
if (success)
synth.setAllPartMaps();
- bool oklearn = synth.midilearn.extractMidiListData(false, *xml);
+ bool oklearn = synth.midilearn.extractMidiListData(false, xml);
if (oklearn)
synth.midilearn.updateGui(MIDILEARN::control::hideGUI);
// handles possibly undefined window
diff --git a/src/Misc/Config.h b/src/Misc/Config.h
index 7224321d..32ae2e03 100644
--- a/src/Misc/Config.h
+++ b/src/Misc/Config.h
@@ -82,8 +82,10 @@ class Config
bool saveInstanceConfig();
void loadConfig();
bool updateConfig(int control, int value);
- bool saveSessionData(string savefile);
+ bool saveSessionData(string sessionfile);
+ int saveSessionData(char** dataBuffer);
bool restoreSessionData(string sessionfile);
+ bool restoreSessionData(const char* dataBuffer, int size);
bool restoreJsession();
void setJackSessionSave(int event_type, string const& session_file);
float getConfigLimits(CommandBlock*);
@@ -106,7 +108,6 @@ class Config
uint build_ID;
int lastXMLmajor;
int lastXMLminor;
- bool stateChanged;
bool oldConfig;
static bool showSplash;
@@ -272,6 +273,8 @@ class Config
bool initFromPersistentConfig();
bool extractBaseParameters(XMLwrapper& xml);
bool extractConfigData(XMLwrapper& xml);
+ void capturePatchState(XMLwrapper& xml);
+ bool restorePatchState(XMLwrapper& xml);
void addConfigXML(XMLwrapper& xml);
void saveJackSession();
diff --git a/src/Misc/Part.cpp b/src/Misc/Part.cpp
index b7ed70a2..dae212c4 100644
--- a/src/Misc/Part.cpp
+++ b/src/Misc/Part.cpp
@@ -1372,7 +1372,6 @@ void Part::add2XML(XMLwrapper& xml, bool subset)
bool Part::saveXML(string filename, bool yoshiFormat)
{
- synth->usingYoshiType = yoshiFormat;
synth->getRuntime().xmlType = TOPLEVEL::XML::Instrument;
auto xml{std::make_unique<XMLwrapper>(*synth, yoshiFormat)};
diff --git a/src/Misc/SynthEngine.cpp b/src/Misc/SynthEngine.cpp
index b27526fe..98d724c4 100644
--- a/src/Misc/SynthEngine.cpp
+++ b/src/Misc/SynthEngine.cpp
@@ -114,7 +114,6 @@ SynthEngine::SynthEngine(uint instanceID)
, legatoPart{0}
, masterMono{false}
, fileCompatible{true}
- , usingYoshiType{false}
// part[]
, fadeAll{0}
, fadeStep{0}
@@ -525,7 +524,6 @@ void SynthEngine::defaults()
}
masterMono = false;
fileCompatible = true;
- usingYoshiType = false;
// System Effects init
syseffnum = 0;
@@ -2598,7 +2596,6 @@ bool SynthEngine::loadStateAndUpdate(string const& filename)
{
interchange.undoRedoClear();
Runtime.sessionStage = _SYS_::type::InProgram;
- Runtime.stateChanged = true;
bool success = Runtime.restoreSessionData(filename);
if (!success)
defaults();
@@ -3027,52 +3024,14 @@ void SynthEngine::add2XML(XMLwrapper& xml)
xml.endbranch(); // MASTER
}
-/*
- * the following two functions are only used by LV2
- */
-
-int SynthEngine::getalldata(char **data) // to state from instance
-{
- bool oldFormat = usingYoshiType;
- usingYoshiType = true; // make sure everything is saved
- getRuntime().xmlType = TOPLEVEL::XML::State;
- auto xml{std::make_unique<XMLwrapper>(*this, true)};
- add2XML(*xml);
- midilearn.insertMidiListData(*xml);
- *data = xml->getXMLdata();
- usingYoshiType = oldFormat;
- return strlen(*data) + 1;
-}
-
-
-void SynthEngine::putalldata(const char *data, int size) // to instance from state
-{
- while (isspace(*data))
- ++data;
- int a = size; size = a; // suppress warning (may be used later)
- auto xml{std::make_unique<XMLwrapper>(*this, true)};
- if (!xml->putXMLdata(data))
- {
- Runtime.Log("SynthEngine: putXMLdata failed");
- return;
- }
- defaults();
- getfromXML(*xml);
- midilearn.extractMidiListData(false, *xml);
- setAllPartMaps(); // TODO this seems to be a duplicate - already done in defaults()
-}
-
bool SynthEngine::savePatchesXML(string filename)
{
- bool oldFormat = usingYoshiType;
- usingYoshiType = true; // make sure everything is saved
filename = setExtension(filename, EXTEN::patchset);
Runtime.xmlType = TOPLEVEL::XML::Patch;
auto xml{std::make_unique<XMLwrapper>(*this, true)};
add2XML(*xml);
bool succes = xml->saveXMLfile(filename);
- usingYoshiType = oldFormat;
return succes;
}
diff --git a/src/Misc/SynthEngine.h b/src/Misc/SynthEngine.h
index 40b31d48..c1389d3a 100644
--- a/src/Misc/SynthEngine.h
+++ b/src/Misc/SynthEngine.h
@@ -117,9 +117,6 @@ class SynthEngine
bool getfromXML(XMLwrapper& xml);
- int getalldata(char **data);
- void putalldata(const char *data, int size);
-
void NoteOn(uchar chan, uchar note, uchar velocity);
void NoteOff(uchar chan, uchar note);
int RunChannelSwitch(uchar chan, int value);
@@ -164,7 +161,6 @@ class SynthEngine
bool masterMono;
bool fileCompatible;
- bool usingYoshiType;
float getLimits(CommandBlock *getData);
float getVectorLimits(CommandBlock *getData);
--
2.20.1
_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
