On 25.11.24 10:47, Kristian Amlie wrote:
However, I discovered that increasing the timeout on this line [1] solves
the problem. I have a very poor understanding of the GUI interfacing code,
so take my comments with a grain of salt, but shouldn't the timeout be much
longer for a message which is required to launch the software?
I'm thinking 10 or 20 seconds.
[1]
https://github.com/Yoshimi/yoshimi/blob/master/src/Interface/GuiDataExchange.cpp#L52
Hi Kristian and Will,
sorry for not responding earlier, just didn't look here fore some time.
Indeed, I am not surprised someone stumbles over this timeout, because it
represents a tricky trade-off. I am aware that this whole new GuiDataConnnect
system adds a new level of complexity; I tried to explain it in the mailinglist
and the documentation (and of course I pointed at that timeout) --
So first questions first, why did we start this attempt to build a new
connection system? Answer: because Yoshimi seems to work well, pragmatically
speaking, but the whole topic of communication between GUI and core is
problematic at a very deep level.
To say it bluntly, at bottom line, the whole idea of separate components that
communicate over a well defined interface was completely ignored when creating
the foundation. And this foundation was cast in concrete, figuratively speaking.
If you just really (analytically!) look at the code at the bottom level,
you will find dozens and dozens and dozens and dozens of cases where
some GUI control just directly grabs into the internal processing state
of the Synth engine. Just follow the usages of all those Synth* variables.
And so it is no wonder, that occasionally garbled data is a common encounter,
whenever you work more intensely with Yoshimi.
Numerous worthy attempts were made already to address this problem.
Unfortunately the state of affairs still falls short in parts:
Fortunately the majority of state changes is dispatched properly via
an atomic variable (in the ringbuffer) and actually the write is done
by the Synth engine in the Synth thread.
Somewhat more problematic are the background operations
(but skipping over those, as they are a topic on their own...)
And then there is the system of "Gui returns". This is quite elaborate,
but in fact only /hides/ the raw and uncoordinated reads to shared data.
The "get data" message is seemingly dispatched, but after passing through
numerous switch-case and dispatch functions, the actual data read happens
*synchronously* and *in the GUI thread*.
Simply put, it *must* be this way, because the returns are used from draw()
requests, and these are synchronous, meaning they need the result value
*here and now*. The way the GUI is written, it is totally unprepared for the
complexities of asynchronous data handover.
With the changes I made, I attempted to propose a new scheme: instead of
grabbing into internal data, let the GUI read data from "Data Transfer Objects",
which are dispatched asynchronously and placed into a buffer owned by the GUI.
This brings me to the relevant part for this issue at hand:
We are talking about serious amounts of data. Thus it can not be fed through
the ringbuffers, which are dead-on optimised since they need to cope with
midi messages. For that reason, I created a secondary system, the
GuiDataExchange. It has a *limited* number of *rather large* slots of fixed
size. As trigger, we place a special message into the toGUI ringbuffer,
to transport the Slot-#. The GUI then copies the DTO-Object out of this slot
into the internal data buffer, from where it will be used by subsequent draw()
processing.
Fine. But what happens if the GUI gets delayed and falls back with processing
its queue? Then later data deliveries will overwrite the storage slots of
earlier messages. As such this is unproblematic, since all deliveries are
done by the GUI thread itself, and it is presentation data after all, so
we are only interested in the newest iteration anyway.
But there is a catch: the ringbuffer has much more capacity than the slots,
so it can be filled up with (obsoleted) messages. If the backlog is long,
I saw that the GUI can be quite occupied just by shovelling obsolete data.
And what is the simplest solution to mitigate this problem? A Timeout!
The UI refresh is 33ms. One screen refresh at 60Hz is 16ms
Usually the GUI has no serious problems processing all drawing in < 10ms
Thus it is completely pointless to care for data updates that were sent
several UI cycles into the past.
So I'd say, up to this point everything is reasonable.
Than came the problems with the instance management, and to solve these,
I had also to change the start-up of LV2 and generally the way how a new
Synth-Engine instance is booted up.
And here is the problematic part: I /just used the new GuiDataExchange/
also to send an initial DTO to the GUI, which provides the connection
information required for all further data exchange.
We didn't have that problem in the old state. Previously, just the
SynthEngine* for the Instance was sent by a FLTK mesage. Now we need
to send an assortment of connection IDs, and these will become more
and more, /if we actually choose to follow up/ on the proposed path
and switch all significant data delivery to the new system
Right now, only the Effect parameters and the EQ curves are sent this way.
It is a prototype, out in the wild, to see how much we get burned!
Now you'll probably ask yourself, what options do we have?
I see several quite distinct avenues. You may see maybe further ones...
(1) play with the concrete time amount in the timeout, find a better tradeoff
(2) question the necessity of the timeout altogether; simply remove it and
process all messages, irrespective how old they are. However, if the
startup becomes delayed, chances are that later UI updates did overwrite
the slot with the bootstrap message. Then the UI will just remain stale.
(3) Investigate *why* the startup takes so long. It should not.
The bootstrap message is placed from SynthEngine::Init().
Kristian: are you encountering this problems from LV2 or from standalone?
- for standalone, the loading of Banks is between Init() and the launch
of the UI-Master (which picks up this message from the UI thread).
- for LV2 there should be no delay at all, since all happens from
InstanceManager::launchGui_forPlugin()
Loading banks involves IO. We always used to do it at this point in the
start-up sequence (I was careful not to change too much). Notably we do
that before starting the actual synth- and midi-threads (Client::start())
This way we do not need data synchronisation.
(4) Build a completely new and different communication channel for the
bootstrap message, don't use the GUI data exchange. Requirements...:
- either the DTO is generated before starting the synth / MusicClient,
but it is clear that the SynthEngine actually creates and owns the
individual GuiDataExchange::Connection<EffectDTO> objects (tag-IDs)
- or, when done afterwards, the Synth must generate the DTO, since
it is the Synth's data and other threads can not be sure to see the data.
- the UI thread must receive a message, where the hand-over is either
protected by a mutex, a semaphore, or an atomic flag or a promise/future
or the like, which allows the UI-Thread to get data visibility on the
contents of this DTO. After that, the GUI can (in the constructor of
MasterUI) dispatch that DTO basically the same way it does it now.
Folks, what I made here with the GuiDataExchange is a proposal. If we want
to follow-up that idea, we need to understand how it works, maybe improve it
or rewrite it or whatever. This implies that I for sure will not solve this
problem alone. Also it would not be wise if I make all the decisions and all
the adjustments and be the only one who understand how it works.
-- Hermann
PS: Relevant Code locations
GuiDataExchange::pushUpdates() <-- checks the timeout
InterChange::createGuiMaster() <-- also checks the timeout (could be skipped)
InstanceManager::Instance::startUp() <-- here we invoke SynthEngine::init()
InstanceManager::SynthGroom::dutyCycle() <---here we invoke createGuiMaster()
InstanceManager::launchGui_forPlugin() <---same for LV2
SynthEngine::publishGuiAnchor() <--- create the bootstrap DTO, ans pushes it
through GuiDataExchange (but could use
another communication channel as well)
_______________________________________________
Yoshimi-devel mailing list
Yoshimi-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/yoshimi-devel
