fluid_sequencer_set_time_scale causes timing problems

[imhcyx]

I'm using FluidSynth's sequencer and recently got a problem. I need to change the time scale several times and then send note events. I have noticed fluid_sequencer_set_time_scale limits scale<=1000, but I need to work on 1280 ticks/s. As a workaround, I reduced both the scale and the delays between events to one half, that is scale=640. So the situation is first I set the time scale to 960, then change it to 640, and finally play several notes. However, the timing is not correct and some of the notes seem to be eaten! I've no idea how this could happen.

Here is the code which can reproduce the problem. Using #define REMOVE_SCALE_LIMIT 0 will demostrate the problem. If the limit in fluid_sequencer_set_time_scale is removed and #define REMOVE_SCALE_LIMIT 1 is used, there will be no such problem.
play.zip
I've tested it on MSYS2 MinGW-w64 and Ubuntu 16.04 x86_64 using the 1.1.6 release and the latest commit on GitHub.

[derselbst]

I have noticed fluid_sequencer_set_time_scale limits scale<=1000

Yes, the smallest unit fluidsynth is able to render are 64 audio frames. Given a samplerate of 48kHz this makes about 1ms.

but I need to work on 1280 ticks/s. As a workaround, I reduced both the scale and the delays between events to one half, that is scale=640.

The logic behind that seems to be correct. To avoid misunderstandings: Even if you would change the scale to 1280 (or to 640 and halve the delays), wont give you any higher resolution (although scale being double in fluid_sequencer_set_time_scale might suggest so). E.g if you want an event to happen at

1/1280 = 0.5/640 = 0.78ms

it will actually happen at 0ms, because being casted to int (yeah, it should probably happen at 1ms to reduce the error...).

So as I understand this concept, using fluid_sequencer_set_time_scale is only a comfortable way that prevents you from having to recalculate event ticks, when sending them to the seq. Since you cant set scale to 1280, you have to fiddle with event timings anyway, so using fluid_sequencer_set_time_scale might be a questionable approach here.

However, the timing is not correct and some of the notes seem to be eaten!

From my understanding, timing problems might occur, because of double to int casts internally. Whether they're hearable or not probably depends a lot on the scale used (smaller scales make timing troubles more likely, right?). Also I dont see what could make notes to be eaten. But apparently this is the case, so this seems to be a bug.

[imhcyx]

I looked into this issue a little further and found _fluid_seq_queue_send_queued_events causes the bug.

unsigned int nowTicks = fluid_sequencer_get_tick(seq);

First it gets the current tick into nowTicks. Then it handles the events. When the timer event is handled and seq_callback is called, it calls fluid_sequencer_set_time_scale, which affects scale and queue0StartTime. Therefore the current tick changes but nowTicks is not updated. Then it does the whole mess.

while (cellNb <= (int)(nowTicks - seq->queue0StartTime)) {

If the time scale is decreased by fluid_sequencer_set_time_scale, (nowTicks - seq->queue0StartTime) will be bigger than it should be, which results in bigger prevCellNb. Then the following several notes will become "late" events (as described in _fluid_seq_queue_insert_entry) and be sent immediately, so the timing is wrong and the notes are overlapped as if some are eaten.

[derselbst]

Oh, nice spot, seems to be comprehensible. I however wont be able to have a closer look at this for the next days. Feel free to make a PR (maybe we should re-scale nowTicks after every call to _fluid_seq_queue_send_cell_events?).

[imhcyx]

Yes, I think doing nowTicks = fluid_sequencer_get_tick(seq); after every call to _fluid_seq_queue_send_cell_events is enough. I'll make the PR later.