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

[josh]

Quoting David Henningsson <address@hidden>:
> address@hidden wrote:
> > 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.


True, hadn't really thought of that scenario.  Something to attend to  
at a later time.


> > 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?)


I don't think there is an issue with this.  Since the new value isn't  
actually passed through the queue, the queued event is essentially  
just an update request.  The latest value will always be the value  
assigned to the variable and the update event will ensure that the  
synthesis thread uses the latest value.  Events are processed at  
whatever interval the fluid_synth_one_block() function is called in  
relation to the MIDI events.  If more than one pitch bend event occurs  
within a given interval, the latest value will get used.


> > 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...?


Yeah, now that I think about it, it would.  In the case of fast  
render, program changes need to occur synchronously as part of the  
synthesis process, but in the case of realtime playback it needs to  
happen outside the synthesis thread.  Seems to fall under the single  
versus multi thread enable/disable.


> > 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.


I don't understand the difference between the two or what distinction  
should be made.  Can you clarify this a little and what this might  
look like as far as API?  It seems to me like there are really only 2  
distinctions that we care about, single threaded (audio synthesis and  
MIDI events occur synchronously and from the same thread) and  
multi-threaded where MIDI events may occur in the audio thread or in  
other threads.

In the case of multi-threads I wonder if there would be any use  
scenarios where synthesis would be running faster than realtime.  This  
would be a problem for the queuing of the program change event.


> > 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.


Sounds good to me.  So we can add an API function to set the mode.  I  
guess the default should be multi-threaded?  That should probably be  
the case to remain as backwards compatible as possible with older  
software which don't yet know about the new API.


> > > (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?


Yes, I want to get this released soon.  Need to take care of these  
issues though.

- Finalize API for getting channel info (sfont ID, bank, program and name)
- Add function to unset the preset on a channel
- Add API for setting threading mode

I'm going to defer the channel mute option to a future version, since  
I just want to get a nice usable FluidSynth which works with QSynth  
released.


> // David


Josh