Re: [fluid-dev] Thread safety long-term thoughts

[David Henningsson]

address@hidden wrote:
> Quoting David Henningsson <address@hidden>:
> > Btw; with the sample timers we also have the entire midi file player
> > (with its fopen calls etc) inside the audio thread. I'm guilty of that,
> > and it should be fixed by inserting a sequencer between the player and
> > the audio thread, at least in real-time use cases. Perhaps something
> > for 1.1.2.
>
> This scenario is less critical IMO, since the audio buffers could be 
> increased without issue (so what if the MIDI file plays 100ms later).  
> Good to know though.  We may want FluidSynth to be smarter about 
> selecting default buffer sizes in the future, depending on the use case.

If someone is playing along with the midi track, they can't have high 
latency. On the other hand, I think it is only between songs there is a 
problem, the rest should not be very time-consuming.

> > And instead of creating shadow variables, all variables should belong
> > the state machine, unless explicitly needed by the voices directly.
>
> For clarification:  Shadow variables were provided in only 2 cases for 
> the purpose of returning the most recent value assigned when querying 
> their values.  The shadow values are used when querying the value only.  
> The 2 cases are presets, since atomic operations can't be used in that 
> case (might be possible if there was preset reference counting) and 
> polyphony, since polyphony is accessed fairly often in the synthesis 
> thread and I thought it was better to shadow the value than have to 
> atomically read it every time and also deal with issues that might arise 
> if the value is changed while looping over voices.
>
> > In the case above, the current pitch bend controller current value
> > belongs to the state machine. The new value is sent to the audio
> > thread. Btw, if someone tries to read the current pitch bend controller
> > value just after having set it, will it work?
>
> Yes it will work.  As far as I know, all values which can be queried 
> should now work, as far as appearing immediately.  For all parameters, 
> except presets and polyphony, the value is set and accessed atomically 
> by all threads.  

If this includes the audio thread, don't we have a problem? If the value 
is being read by the audio thread after being atomically set by the MIDI 
thread(s), but before the corresponding event has arrived in the queue, 
things will be incosistent?

(Perhaps this is not such a big issue for the pitch bend, but there 
could be another events where this problem could hurt more?)

> When an event occurs, it assigns the new value to the 
> parameter and sends an update event to the synthesis thread.  The update 
> event does not contain the value, but just tells the synth thread to 
> update the parameter based on the latest value.  If an event is set from 
> within the synth thread, its pretty much the same, except no queuing of 
> the update occurs.
>
> > Perhaps an option to let the libfluidsynth user call a function
> > periodically if he wants to skip the additional thread.
> Well it isn't just for garbage collection now.  Its also being used to 
> handle program changes, which should happen ASAP.

Eh? I had a look at that code and it seems to screw up fast-render and 
embedded cases pretty bad, unless I'm missing something...?

> > Could we do something in create_audio_driver/delete_audio_driver to
> > register with the synth, and say that it is now operating with an audio
> > driver, and that means real-time operation? And if that is not the
> > case, handle events directly. Likewise, we could make the MIDI drivers
> > (and shell thread) register that we now have additional threads
> > referencing the state machine, so the state machine must be
> > multi-threaded.
>
> Yeah, I think something like that could be good, rather than trying to 
> auto detect it.  A simple API function like:
> void fluid_synth_multi_thread_enable(fluid_synth_t *synth, int enable);

We have two kinds of multi-threading, a) we have an audio thread or we 
don't, and b) we have either one, or more than one, thread accessing the 
state machine. I think we should separate those cases if we made such an 
API.

> And/Or, like I mentioned before, we could try to improve the 
> multi-thread use auto-detection.  

> I haven't fully looked into the 
> scenario, but if we made all the public API event functions 
> automatically enable multi-threading and then provide an alternative to 
> fluid_synth_handle_midi_event, say 
> fluid_synth_handle_midi_event_noqueue() that might suffice and somewhat 
> simplify things.

Don't like it, I prefer configuring the use case via 
fluid_synth_multi_thread_enable or something similar. If it is common 
opinion that we expose too much of the internals, this approach won't 
improve the situation.

> > (Note: potential screwup with libfluidsynth users creating their own
> > audio drivers, although I assume it was screwed up in 1.0.9 the same
> > way as well.)
>
> In regards to thread safety you mean?  As things are now, it just 
> assumes it is multi-threaded, which means slightly less efficiency.

I mean, if we were to change the assumption, we might have to look out 
for that problem.

Btw, ready to release 1.1.1 anytime soon?

// David