Skip to content

Commit

Permalink
do not process more blocks than we can store internally
Browse files Browse the repository at this point in the history 
addresses #192
  • Loading branch information
@derselbst
derselbst committed Aug 18, 2017
1 parent 1344059 commit d313db5
Showing 1 changed file with 10 additions and 1 deletion.
11 changes: 10 additions & 1 deletion fluidsynth/src/synth/fluid_synth.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2942,7 +2942,16 @@ fluid_synth_render_blocks(fluid_synth_t* synth, int blockcount)
for (i=0; i < blockcount; i++) {
fluid_sample_timer_process(synth);
fluid_synth_add_ticks(synth, FLUID_BUFSIZE);
if (fluid_rvoice_eventhandler_dispatch_count(synth->eventhandler)) {

/* If events have been queued waiting for fluid_rvoice_eventhandler_dispatch_all()
* (should only happen with parallel render) OR we are about to process more
* blocks than we can store internally (being at risk having already dispatched many rvoices,
* because we may not be using parallel render), stop processing and go for rendering
*/
if (fluid_rvoice_eventhandler_dispatch_count(synth->eventhandler)
|| i+1 >= FLUID_MIXER_MAX_BUFFERS_DEFAULT-1 /* there actually is one 1 too many here,
but this seems to be the only way it works */
) {
// Something has happened, we can't process more
blockcount = i+1;
break;
Expand Down

4 comments on commit d313db5

@derselbst
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@diwic Could you perhaps have a look whether this fix makes sense at all or might there be trouble somewhere else? Is my understanding correct so far?

Background:
(assume: parallel render and threadsafe API disabled)

If the user wants to render a whole MIDI with one single call to fluid_synth_write*() numerous blocks would be requested from fluid_synth_render_blocks(). fluid_sample_timer_process() would dispatch all the voices immediately, i.e. not queuing them because of non parallel render. And it would do that for all blocks requested, since fluid_rvoice_eventhandler_dispatch_count() always stays 0. However we can only render a limited no. of blocks. If we continue to rendering we would hear a huge "bang" at the beginning and silence for the rest of the song. So in this loop we shouldn't render more blocks than we can actually store internally, should we?

Note: never experienced this with the fluidsynth executable, because it only requests 64 audio frames from the synth.

@diwic
Copy link
Contributor

@diwic diwic commented on d313db5 Aug 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm...so I think your fix is about in the right place. You can make it less hacky by

  1. Exposing a int fluid_rvoice_mixer_get_bufcount(mixer) { return mixer->buffers.buf_blocks }

  2. Above the loop, do

    maxblocks = fluid_rvoice_mixer_get_bufcount();
    if (blockcount > maxblocks) blockcount = maxblocks;

...now you can skip the condition inside the loop. Correct?

@diwic
Copy link
Contributor

@diwic diwic commented on d313db5 Aug 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Side note: Parallel rendering with more than one thread was mostly a failed experiment, at least for me. FluidSynth rendering is so efficient that only in a few cases was the overhead of starting/stopping threads so low that rendering actually was faster with more threads.

@derselbst
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, thanks!

Please # to comment.