On 30.01.2025 07:34, ichthyo wrote:
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.
Excellent work! Everything looks good to me!
Now, I have to admit that I was slightly surprised and confused when the
Program Change wipeout *still* happened to me (under controlled
conditions, of course), and I had to investigate it. Through this I made
some new discoveries.
First off, it turns out that my host doesn't do "state update" followed
by "Program Change". It does it in the opposite order! This means that
when the Program Change happens, the correct config still hasn't been
loaded by the state yet, and thus the Program Change still happens. How
can we get the correct config then..? Bummer.
But hold on! If it does it in that order, it should *already* have
worked, even before your patch, because doing the state last means that
any Program Change that happened before that is irrelevant; everything
is reloaded from the state. Well, it turns out that there is an extra
element in play here, and what actually happens is this:
1. Load LV2-instance config (Program Change is enabled, on purpose).
2. Receives Program Change, which *schedules* program change.
3. Receives state update, which contains both Program-Change-disabled
and correct patch.
4. Program change is handled, and since Program Change is now disabled,
it should be blocked, but the check for Program Change Enabled happens
in point 2, not here, therefore it loads the Program anyway.
So it still doesn't work correctly!
There are two ways we could handle this:
1. Make the Program Change in point 2 synchronous, forcing it to finish
right there and then. Then the state will load after and everything will
be fine.
2. Move (or copy) the Program Change Enabled check to where the load
actually happens (point 4), instead of where it is scheduled (point 2).
I'm personally leaning towards option 1 because I also discovered that
delaying the loading is actually violating the LV2 Programs extension. I
quote: "The program change should take effect immediately at the start
of the next run() call". This is impossible to guarantee if the call is
asynchronous. The host is still free to call the function from a low
priority thread, so by making it synchronous we are making it the host's
responsibility to do so rather than ours.
If I don't hear any counter-arguments, I'll get going with option 1.
FTR, what I described above is a separate problem from the original, so
everything Hermann has done is still valuable, and should go in
immediately as far as I'm concerned.
--
Kristian
_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
