NULL pointer deref if dynamic-sample-loading is enabled.

[derselbst]

FluidSynth version

2.1.2 (confirmed), most likely up to 2.0.0 (where dynamic-sample-loading was introduced)

Describe the bug

dsp_data is NULL, because voice->sample->data is NULL.

Thread 1 "anmp-qt" received signal SIGSEGV, Segmentation fault.
start_points[0] = fluid_rvoice_get_float_sample(dsp_data, dsp_data24, voice->start);
#0  0x00007ffff32ba7e9 in fluid_rvoice_dsp_interpolate_7th_order (voice=voice@entry=0xd82d68, dsp_buf=dsp_buf@entry=0x1b811c0, looping=1) at fluidsynth/src/rvoice/fluid_rvoice_dsp.c:417
#1  0x00007ffff32b8f89 in fluid_rvoice_write (voice=voice@entry=0xd82ab0, dsp_buf=dsp_buf@entry=0x1b811c0) at fluidsynth/src/rvoice/fluid_rvoice.c:450
#2  0x00007ffff32bc0d6 in fluid_mixer_buffers_render_one (blockcount=3, src_buf=0x1b811c0, dest_bufcount=64, dest_bufs=0x7fffffffba40, rvoice=<optimized out>, buffers=0xbbf358)
    at fluidsynth/src/rvoice/fluid_rvoice_mixer.c:434
#3  0x00007ffff32bc0d6 in fluid_render_loop_singlethread (mixer=mixer@entry=0xbbf350, blockcount=blockcount@entry=3) at fluidsynth/src/rvoice/fluid_rvoice_mixer.c:589
#4  0x00007ffff32bd120 in fluid_render_loop_multithread (current_blockcount=3, mixer=0xbbf350) at fluidsynth/src/rvoice/fluid_rvoice_mixer.c:1206
#5  0x00007ffff32bd120 in fluid_rvoice_mixer_render (mixer=0xbbf350, blockcount=3) at fluidsynth/src/rvoice/fluid_rvoice_mixer.c:1377
#6  0x00007ffff32c389c in fluid_synth_process_LOCAL (synth=0xb76a00, len=len@entry=2048, nfx=64, fx=0xac9e60, nout=<optimized out>, out=out@entry=0xd29320, block_render_func=0x7ffff32c0380 <fluid_synth_render_blocks>)
    at fluidsynth/src/synth/fluid_synth.c:3755
#7  0x00007ffff32c3c11 in fluid_synth_process (synth=<optimized out>, len=len@entry=2048, nfx=<optimized out>, fx=<optimized out>, nout=<optimized out>, out=out@entry=0xd29320)
    at fluidsynth/src/synth/fluid_synth.c:3672

Expected behavior

Should not crash.

Steps to reproduce

Suppose we want to start a voice with given preset, but this preset is not assigned to any MIDI channel currently:

fluid_settings_setint(settings, "synth.dynamic-sample-loading", 1);
...
sf_id = fluid_synth_sfload();
fluid_sfont_t* sfont = fluid_synth_get_sfont(this->synth, sf_id);
fluid_preset_t *preset_not_assigned_anywhere = fluid_sfont_get_preset(sfont, bnk, this->defaultProg);
fluid_synth_start(this->synth, voice_id, preset_not_assigned_anywhere, 0, chan, key, vel);
...
fluid_synth_process() // crash

When disabling dynamic sample loading, everything works fine.

[derselbst]

It is pretty trivial to add the preset notification to fluid_synth_start():

preset->sfont->refcount++;
fluid_preset_notify(preset, FLUID_PRESET_SELECTED, chan);

We need to unselect the preset in fluid_synth_stop(). But where to get the preset from?

[mawe42]

Sending a FLUID_PRESET_SELECTED signal for a preset that has not been selected seems wrong to me.

Is this a crash in an actual use-case? I mean is starting a voice for a preset that has not been assigned to a MIDI channel a feature we want to support with dynamic sample loading?

Maybe it would be easier to add a check in fluid_synth_start so that the null-deref can't happen and require the user to select the preset on a MIDI channel before starting the voice?

[derselbst]

Is this a crash in an actual use-case?

When notes are played on a channel that has no program assigned (yet), I would like to play these notes with a specific preset. This preset is (most likely) not used on any channel. And since fluid_synth_start() provides a soltuion under this circumstance, I do believe it's a valid use-case.

Whether we want to support this when dynamic sample loading is enabled is a different story. Ofc we could just error out in fluid_synth_start() if that preset is not selected. In this case I would need to create an extra audio channel for these unassigned notes. But since I render each MIDI channel to a separate audio channel, this would mean extra memory requirements. So, I would most likely simply turn off dynamic sample loading.

In order to support this use-case, one would need to add a fluid_preset_t* member into the struct fluid_voice_t.

[mawe42]

The segfault is obviously a bug which we should fix or guard against.

But the whole idea behind dynamic sample loading is to load the samples into memory when the preset has been selected on a channel and unload it again when it is unselected. Changing that to load and unload the samples when a voice for that preset has been started / stopped seems to be quite a large change to the underlying idea and quite frankly feels like a hack to me.

A cleaner solution might be to expose the loading / unloading methods the the user.

[derselbst]

But the whole idea behind dynamic sample loading is to load the samples into memory when the preset has been selected on a channel and unload it again when it is unselected. Changing that to load and unload the samples when a voice for that preset has been started / stopped seems to be quite a large change to the underlying idea and quite frankly feels like a hack to me.

Ok. I'll add a guard into fluid_synth_start() and update docs.

A cleaner solution might be to expose the loading / unloading methods the the user.

Oh no. No more complexity.

---

Btw, in case you missed it: PR 604 is ready.