[email protected] wrote: > Quoting Rui Nuno Capela <[email protected]>: >> [email protected] wrote: >>> >>> I notice that QSynth makes use of fluid_synth_get_channel_preset(). >>> There have been a number of thread safety issues with FluidSynth, many >>> of which have been fixed in 1.1.0. The issue with that particular >>> function, is that presets are allocated when they are assigned to a >>> channel and freed when another preset is assigned (a program change for >>> example). This means that currently there are no guarantees that the >>> preset returned by fluid_synth_get_channel_preset() will be valid, in a >>> multi-thread environment. >>> >>> I'd like to resolve this issue. The ideal solution would be to add >>> reference counting to presets, but this would require an additional API >>> function (fluid_synth_ref_channel_preset() and fluid_preset_unref() for >>> example). Another option would be to add a function to query the >>> current preset info, without having to retrieve the preset, something >>> like: >>> fluid_synth_channel_preset_info (synth, chan, &name, &bank, &program); >>> >>> Trying to get fluid_synth_get_channel_preset() to be thread safe, isn't >>> so straightforward though, since there is no way for FluidSynth to know >>> how long the calling application will access the preset. Its documented >>> as a "low level access" function, so I was considering just making it a >>> function restricted to synthesis context (from a note-on method or in >>> the audio driver) and providing other methods of safely retrieving the >>> preset info. >>> >>> By the way, this issue is nothing new. Older versions of FluidSynth >>> could potentially crash QSynth if a program change occurred at the right >>> moment. >>> >>> Any ideas or preferences? >>> >> >> a variation of what you suggested would be fine, >> >> fluid_preset_info_t info; >> fluid_synth_get_channel_preset_info(synth, chan, &info); >> >> seems to be a solution. the returned new struct fluid_preset_info_t >> would have bank, num, name and sfont as public api accessible members. >> >> then we could throw fluid_synth_get_channel_preset() into the deep low >> deprecation abyss :) >> >> cheers >> -- >> rncbc aka Rui Nuno Capela >> [email protected] >> > > > Sounds good. How about adding a size field to, so that there can be > potential future extensions to fluid_preset_info_t. Example: > fluid_preset_info_t info; > fluid_synth_get_channel_preset_info(synth, chan, &info, sizeof (info)); >
omg, that additional size argument just raises from the grave all too many windows api'esque nightmares. i'll pretend i did not read that and close my eyes if you really think it's necessary :)) one possible and alternative solution is about having the fluid_preset_info_t as an opaque struct, accessible trough additional api functions (or macros): const char *fluid_preset_info_get_name(&info); int fluid_preset_info_get_bank(&info); int fluid_preset_info_get_num(&info); fluid_sfont_t *fluid_preset_info_get_sfont(&info); any additional fields would have corresponding accessors in a possible future. cheers -- rncbc aka Rui Nuno Capela [email protected] _______________________________________________ fluid-dev mailing list [email protected] http://lists.nongnu.org/mailman/listinfo/fluid-dev
