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

[josh]

Quoting David Henningsson <address@hidden>:
> Fluid_preset_t contains a pointer to the soundfont, so all channels must
> be notified to delete their corresponding presets when a soundfont is
> unloaded.
> For the other short-term uses of fluid_sfont_t, we could let the synth
> mutex include protection of all soundfont instances.
>
> Either that or reference-counting - I don't know what's more messy.

I started looking at adding reference counting to fluid_sfont_t, but  
realized that the allocation of them is handled by the SoundFont  
loader.  So this means that we can't add fields to the structure,  
without breaking binary compatibility :(

It seems like the best would be to use the synth's mutex as you  
suggested.  I'll make the appropriate changes.

In the next FluidSynth version after 1.1.0, I'd like to look at adding  
a new SoundFont loader API.  We can keep the old one for backwards  
compatibility.  The new one would support 24 bit and/or floating point  
audio, sample streaming via a callback and a scheme where FluidSynth  
controls the size of the allocated structures for future expansion.

> > I just committed changes which queue up preset assignment events.  I
> > also combined the SoundFont ID, bank and program channel fields to a
> > single integer, as I spoke of before.  This allows for atomic
> > retrieval/assignment of these values.  From what I can tell, the
> > synthesis thread only uses the preset.  The SoundFont ID, bank and
> > program values are used with the various functions which ultimately
> > select the preset (program change, bank change, SoundFont select, etc).
> > The parameters can be treated independently also, so they don't
> > necessarily match that of the currently active preset.
>
> I had a quick look and spotted two things:
>
> 1) Since the program-change message is not queued, we must also move the
> handling of BANK_SELECT_MSB and BANK_SELECT_LSB from
> fluid_synth_cc_LOCAL to fluid_synth_cc to prevent reordering.


Good point.  I'll fix that.


> 2) This might be independently of your thread-safe changes, but doesn't
> the three functions that set the preset (fluid_synth_program_change,
> fluid_synth_program_select, fluid_synth_program_select2) look a bit too
> different? Can they all be right? And why don't they call each other
> (especially fluid_synth_program_select2 that should make an name to id
> conversion and then call fluid_synth_program_select)?


Yeah, they could be cleaned up a little.  fluid_synth_program_change  
is just the channel and program number and has some additional logic  
to fallback to other bank numbers if a valid preset is not found.  The  
other 2 functions have explicit specification of the bank, so that  
logic doesn't make sense in that case.  Whats really occurring is  
various ways of searching for a valid preset, so I think I'll just add  
additional functions which search for presets based on various  
criteria.  They could handle locking as appropriate as well.


> > The following API should be considered for addition to FluidSynth 1.1.0,
> > though perhaps some renaming is in order (I don't like just appending a
> > 2).  These functions were meant to be made public at some point, as the
> > comment suggests at the top of fluid_synth.c
> >
> > fluid_synth_program_select2
> > fluid_synth_get_sfont_by_name
> > fluid_synth_set_gen2
>
> I see no reason not to make them public, so go ahead. (and choose a name
> of your preference.)


fluid_synth_program_select_by_sfont_name?
fluid_synth_set_gen_full?


Josh