On 28.01.2025 10:45, Will Godfrey wrote:
On Tue, 28 Jan 2025 06:44:43 +0100
ichthyo <p...@ichthyostega.de> wrote:
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.
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 :(
Lol, seriously!?
That might explain how I ended up in those "wiped-out-session"
situations earlier. Perhaps I had enabled program change and it really
did affect every LV2 instance, even pre-saved ones. TBH I don't remember
the exact details, since I've been very careful *not* to enable it for
years already.
In any case, I agree with the conclusions (patch looks sensible too!),
and I'm glad we finally have a way forward. This was a tough discussion! 😄
--
Kristian
_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
