[fluid-dev] Re: FluidSynth event queue and thread safety changes

[josh]

Quoting David Henningsson <address@hidden>:

> address@hidden skrev:
> > Hey David and list,
> >
> > I finally got around to getting my event queue and thread safe related
> > FluidSynth changes cleaned up.  I merged with the latest trunk and
> > created a branch in SVN.  I did this because it is a lot of significant
> > changes to fluid_synth.c and fluid_chan.c and thought it would be the
> > easiest way to collaborate and track the changes.  I didn't want to
> > commit it just yet to the main branch, in case of serious issues.  We
> > should merge it soon though, to prevent too much divergence.
> >
> > The current code builds, but I have not thoroughly tested it.
>
> A short update: I've started to test the new code now and committed two
> bugfixes (actually three, but the last was my own ;-) ). I've also
> removed the sequencer from fluidsynth.c, now that the synth is thread
> safe in itself. To ease merging I suggest that we work on the event
> queue branch from now on and I think it'll be ready for becoming our
> "trunk" in a week or so.
>
> // David

Great!  I looked over your changes, good catch on those bugs :)

I'm reviewing the code in fluid_synth.c and fluid_chan.c currently to  
check for other threading issues.

Here is what I have so far:

Assignment/read of fluid_preset_t on MIDI channels is problematic.   
Preset selection (fluid_sfont_get_preset) should occur outside of the  
synthesis thread, since it contains malloc calls and may perform other  
types of locking depending on the SoundFont loader backend being used.  
 The returned preset from fluid_sfont_get_preset has been allocated  
and is owned by the caller, so it could be passed to the synthesis  
thread for assignment to a channel via the queue, at which point the  
synthesis thread (channel structure) would "own" the preset.

I think the preset field of the MIDI channel should be private to the  
synthesis thread.  There are 2 public functions which deal with  
fluid_preset_t instances: fluid_synth_get_channel_preset and  
fluid_synth_start.  I don't think it will be an issue to restrict  
these to synthesis thread use only (from within the noteon handler for  
example), since I don't see why else they would be used.

A selected preset on a channel has other associated information with  
it, including: SoundFont ID, MIDI Bank # and MIDI Program #.   
Currently they aren't atomic with respect to one another (so there is  
no way to guarantee that the retrieved values aren't actually out of  
sync, due to another thread switching the current instrument in the  
middle of retrieving the values).  An atomic integer could be used  
though to store all 3 fields.  7 bits for program #, 14 bits for bank  
# and the rest (11 bits, 2048 SoundFonts max) for SoundFont ID.   
Atomic operations could then be used for setting and retrieving this  
when a preset is assigned.

I'm going to remove a bunch of the documentation in the synth.h file,  
which is already covered in the C code.

Cheers!

Josh