We need a new, better sequencer!

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
14 messages Options
Reply | Threaded
Open this post in threaded view
|

We need a new, better sequencer!

fluid-dev mailing list
Current situation: I have a program[1], which reads standard MIDI files, processes it's events and finally adds all MIDI events to fluid_sequencer.

The current sequencer sucks:

1. It is slow.
2. A meaningful ordering for events with the same tick has not been considered.
3. Its implementation codebase is *huge and complicated*, implementing its own heap, maintaining five internal queues, etc.
4. Events that have been processed are deleted and gone.
5. The sequencer supports the system timer as alternative time source.
6. Time Scaling that only allows to make things worse.

1. Why is it slow?

My program supports MIDI track loops. It unrolls them by adding all MIDI events within the loop over and over again to the sequencer. Thus, the sequencer has to cope with many events, most of them will be dispatched in the far future. For such a scenario, there is a while() loop [2] where it tries to determine where to insert a new event, which turns out to be the bottleneck.

2. A meaningful ordering?

As per MIDI standard, events are ordered by tick count. If two events share the same tick count, the order is undefined. In such a case almost all MIDI players out there fall back to the order the events appear in the "stream" (e.g. file). IMO, this doesn't make sense. Let me remind you of an issue [3], where a progChange and a NoteOn happen at the same tick. Question: Which instrument will the NoteOn trigger? The new program or the old program?

To overcome this problem I'm suggesting to implement an ordering function which makes sure that NoteOn events are always last (within the same tick count). On the other hand FLUID_SEQ_UNREGISTERING events are always first within the same tick count. This is because if an unregistering happens at a given tick, it doesn't make any sense to keep posting events, for which the client won't have time to process them, because the client will be unregistered at the same tick anyway. For all other events the order is retained.

3. Simpler implementation?

I just had a quick try using C++ std::priority_queue. The results are very promising in terms of performance. Whereas the UI thread of my program previously was stuck in [2] for seconds, with the C++ implementation I'm unable to notice any delay.

4. Processed events are gone

Why does it matter? Because originally, fluid_sequencer_t along with its fluid_event_t were designed to replace fluid_player_t and fluid_midi_event_t. Meanwhile, the fluid_player supports seeking, thus events must not be deleted after they have been processed.
So, if we ever want to have a realistic chance to remove fluid_midi_event_t and replace it by fluid_event_t and its sequencer, this must be considered.

5. The system timer

I see no reason to retain support for the system timer. The only time source for dispatching events at the right time should be the sample timer. Whether this actually happens in real-time or not, depends on whoever calls the rendering functions of the synth, e.g. audio driver vs. fast-file-renderer.

6. Time Scale

Also, I see no reason to keep the time scaling. The default scale is 1 tick per milisecond. There is no way to get a higher resolution, you can only make the resolution worse. Why would you want to do this? (Perhaps in case of the system timer, to avoid that it fires a callback every milisecond. But this is not a reason anymore, see 5.)


To address those issues I see two ways to go:

1. Next to the current sequencer implementation, add a new C++ implementation for the sequencer and decide at cmake time whether to use the old pure C or the new C++ implementation.
2. Replace the current implementation of the sequencer with some heavy glib usage.

Any opinions or comments highly welcome.


Tom


[1] https://github.com/derselbst/ANMP
[2] https://github.com/FluidSynth/fluidsynth/blob/08848864d246bd0371b12e493e4e927baff77d08/src/midi/fluid_seq.c#L1030-L1042
[3] https://lists.nongnu.org/archive/html/fluid-dev/2017-05/msg00004.html




_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: We need a new, better sequencer!

Marcus Weseloh
Hi Tom,

generally I'm all in favour of cleaning up the sequencer code. Below are just some ideas and smaller concerns that came to my mind while reading your post:

Am Mo., 9. Dez. 2019 um 19:46 Uhr schrieb Tom M. via fluid-dev <[hidden email]>:
2. A meaningful ordering?

As per MIDI standard, events are ordered by tick count. If two events share the same tick count, the order is undefined. In such a case almost all MIDI players out there fall back to the order the events appear in the "stream" (e.g. file). IMO, this doesn't make sense. Let me remind you of an issue [3], where a progChange and a NoteOn happen at the same tick. Question: Which instrument will the NoteOn trigger? The new program or the old program?

Hm... I disagree. I think it makes perfect sense to fall back to the order of events in the stream. It's closer to the way real-time MIDI messages over a serial connection work. And if I understood the problem in [3] correctly, then the issue occurred because the program change events where in the wrong track. So the abc to midi converter was to blame, not the synth playing the MIDI file.

To overcome this problem I'm suggesting to implement an ordering function which makes sure that NoteOn events are always last (within the same tick count).

As you say: almost all MIDI players fall back to the event order, so why shoudl Fluidsynth be different? Why is "Note-On last" the better order? How do you know that that special (and seemingly Fluidsynth specific) ordering is what the author of the MIDI file wanted?

3. Simpler implementation?

I just had a quick try using C++ std::priority_queue. The results are very promising in terms of performance. Whereas the UI thread of my program previously was stuck in [2] for seconds, with the C++ implementation I'm unable to notice any delay.

Simpler and faster implementation sounds great, IMO! What trips me up though: if your program was really stuck in that loop for seconds, I'm wondering how many events you were actually adding to the sequencer... Millions? And if you really are adding that many events... could you explain why?
 
5. The system timer

I see no reason to retain support for the system timer. The only time source for dispatching events at the right time should be the sample timer. Whether this actually happens in real-time or not, depends on whoever calls the rendering functions of the synth, e.g. audio driver vs. fast-file-renderer.

Just thinking out loud and I'm not sure if this actually applies here... but isn't there a global timing event that MIDI devices can send to each other to synchronise playback position and playback speed? So if we would ever support that, wouldn't we need an adjustable timing source? But maybe that only applies to real-time MIDI playback, not events read from a file. Not sure...
 
6. Time Scale

Also, I see no reason to keep the time scaling. The default scale is 1 tick per milisecond. There is no way to get a higher resolution, you can only make the resolution worse. Why would you want to do this? (Perhaps in case of the system timer, to avoid that it fires a callback every milisecond. But this is not a reason anymore, see 5.)

Similar comment as above and again, just thinking out loud: wouldn't it be a great feature to allow slower or faster MIDI playback? 
 
To address those issues I see two ways to go:

1. Next to the current sequencer implementation, add a new C++ implementation for the sequencer and decide at cmake time whether to use the old pure C or the new C++ implementation.
2. Replace the current implementation of the sequencer with some heavy glib usage.

Any opinions or comments highly welcome.

I'm all for cleaning up the code base, but adding another implementation in C++ is not cleanup IMO... it adds more technological debt because we now have to keep two different implementations in sync. So personally I would vote to have only a single implementation and don't really care if it is C or C++. But I do wonder: what can C++ do that C can't? And especially: why can C++ do it faster?

Cheers
Marcus

_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: We need a new, better sequencer!

fluid-dev mailing list
Thanks for your thoughts, Marcus.

> > 2. A meaningful ordering?
> > To overcome this problem I'm suggesting to implement an ordering function which makes sure that NoteOn events are always last (within the same tick count).

> As you say: almost all MIDI players fall back to the event order, so why shoudl Fluidsynth be different?

I'm looking at it with my software engineer glasses: Whenever something has undefined behaviour, one can think about how to tweak and optimize things.

> Why is "Note-On last" the better order? How do you know that that special (and seemingly Fluidsynth specific) ordering is what the author of the MIDI file wanted?

When something happens at the same tick, I don't claim to know anything. My proposed solution is simply the best guess I can come up with in such a situation. Two examples:

a) NoteOn and ProgChange happen in that order at the same tick. Would it make sense to turn on the note using the old instrument, although we are just about to receive a new program at the same tick? I don't think so. The NoteOn, although first in the stream, should use the new program.

b) NoteOn at tick 0, after e.g. 100 ticks: NoteOn and NoteOff. All three note events happen in the given order on the same channel and trigger the same key. How would fluidsynth interpret them currently? The first note is triggered, it sounds for a while until the second NoteOn arrives. That second NoteOn kills overlapping notes, i.e. it kills the first note. But oops, just after that second note was turned on, it will be turned off right again, because we have a NoteOff at the same tick, which apparently was meant to turn off the first note though.

For case b) I have MIDI files that do such things. Ofc you are right Marcus, technically the MIDI converter program is to blame. I cannot really change the converter though. I would alternatively need to handle those cases in my program. I was just thinking of a possibly acceptable way for fluidsynth to handle such "corner-cases".


> > 3. Simpler implementation?
> What trips me up though: if your program was really stuck in that loop for seconds, I'm wondering how many events you were actually adding to the sequencer... Millions?

I'm takling about ten-thousands of events. Here are some measurements:

virtual void MidiWrapper::open(): Added 23976 events to the seq, took 12 ms
void FluidsynthWrapper::Render(float*, frame_t): Rendering took 897 ms

virtual void MidiWrapper::open(): Added 28395 events to the seq, took 17 ms
void FluidsynthWrapper::Render(float*, frame_t): Rendering took 1441 ms

virtual void MidiWrapper::open(): Added 84961 events to the seq, took 48 ms
void FluidsynthWrapper::Render(float*, frame_t): Rendering took 19709 ms

The problem is the very first rendering call (in my case: fluid_synth_process), when the sample-timers are called back until they ultimately reach the while loop I mentioned. You can see there seems to be an exponential increase. ATM, I just tested 4 different MIDI files. The first 3 files have a playduration of 4-6 minutes, the last one has 12 minutes playback. Ofc, this heavily relies on the timing order of the events, so making a generalized benchmark is not so straight forward to do.

Same test, but with std::priority_queue :

virtual void MidiWrapper::open(): Added 23976 events to the seq, took 9 ms
void FluidsynthWrapper::Render(float*, frame_t): Rendering took 4 ms

virtual void MidiWrapper::open(): Added 28395 events to the seq, took 16 ms
Warning: void FluidsynthWrapper::Render(float*, frame_t): Rendering took 3 ms

MidiWrapper::open(): Added 84961 events to the seq, took 39 ms
Warning: void FluidsynthWrapper::Render(float*, frame_t): Rendering took 4 ms


> > 5. The system timer
> isn't there a global timing event that MIDI devices can send to each other to synchronise playback position and playback speed?

You seem to refer to "System Real Time Messages". But I don't know how this could be integrated into fluidsynth.

> > 6. Time Scale
> Similar comment as above and again, just thinking out loud: wouldn't it be a great feature to allow slower or faster MIDI playback?

Indeed, supporting tempo changes would be a great and necessary feature for the seq, if we ever want to use it in the midi player. Ok, so better improve the time scaling rather than remove it.


> adding another implementation in C++ is not cleanup IMO... it adds more technological debt because we now have to keep two different implementations in sync.

Having two implementations would be temporarily. First add an "experimental" C++ impl., then deprecate the current C implementation while using the C++ impl. by default and finally remove the deprecated.

> But I do wonder: what can C++ do that C can't? And especially: why can C++ do it faster?

Simply that I'm more familiar how to do those things in C++ rather than with glib. And I'm not sure what Carlo, you, and the other embedded system guys said, if we fortify the glib dependency. I'm not sure how they think about a dependency to the C++ std lib either...


Tom




_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: We need a new, better sequencer!

Antoine Schmitt
Hello Marcus and Tom and all,

as the original implementor of the fluidsynth sequencer, and using my engineer's glasses ("make it simple"), here are my opinions and thoughts on these matters :

- C++ would a big dependency to add to fluidsynth.. I leave it to the community to decide whether or not to add it
- if the performance (speed) of adding a large number of events at once can be improved using a C++ implementation, it surely can also be improved using an optimization of the current C implementation.
- about re-ordering events happening on the same tick : it is always good to make the client responsible for its actions and not make decisions on its behalf. The "first added, first served" implementation seems the most straightforward to me, especially if most MIDI players do it also.
- the sequencer is also often used for real time usage (this was my usage), where events are added every pattern (1 second or so) regarding the upcoming pattern to be rendered. This means that the System Timer feature is actually used by clients of fluidsynth. If it is removed from fluidsynth, provisions should be made for clients to implement their own timer and callback to the sequencer rendering function.
- I have no opinion regarding the Time Scale. I guess that this can be used to optimize speed if time scale is known to be lower than the standard one. But maybe this is useless..

Hope this helps
Antoine



> Le 10 déc. 2019 à 11:03, Tom M. via fluid-dev <[hidden email]> a écrit :
>
> Thanks for your thoughts, Marcus.
>
>>> 2. A meaningful ordering?
>>> To overcome this problem I'm suggesting to implement an ordering function which makes sure that NoteOn events are always last (within the same tick count).
>
>> As you say: almost all MIDI players fall back to the event order, so why shoudl Fluidsynth be different?
>
> I'm looking at it with my software engineer glasses: Whenever something has undefined behaviour, one can think about how to tweak and optimize things.
>
>> Why is "Note-On last" the better order? How do you know that that special (and seemingly Fluidsynth specific) ordering is what the author of the MIDI file wanted?
>
> When something happens at the same tick, I don't claim to know anything. My proposed solution is simply the best guess I can come up with in such a situation. Two examples:
>
> a) NoteOn and ProgChange happen in that order at the same tick. Would it make sense to turn on the note using the old instrument, although we are just about to receive a new program at the same tick? I don't think so. The NoteOn, although first in the stream, should use the new program.
>
> b) NoteOn at tick 0, after e.g. 100 ticks: NoteOn and NoteOff. All three note events happen in the given order on the same channel and trigger the same key. How would fluidsynth interpret them currently? The first note is triggered, it sounds for a while until the second NoteOn arrives. That second NoteOn kills overlapping notes, i.e. it kills the first note. But oops, just after that second note was turned on, it will be turned off right again, because we have a NoteOff at the same tick, which apparently was meant to turn off the first note though.
>
> For case b) I have MIDI files that do such things. Ofc you are right Marcus, technically the MIDI converter program is to blame. I cannot really change the converter though. I would alternatively need to handle those cases in my program. I was just thinking of a possibly acceptable way for fluidsynth to handle such "corner-cases".
>
>
>>> 3. Simpler implementation?
>> What trips me up though: if your program was really stuck in that loop for seconds, I'm wondering how many events you were actually adding to the sequencer... Millions?
>
> I'm takling about ten-thousands of events. Here are some measurements:
>
> virtual void MidiWrapper::open(): Added 23976 events to the seq, took 12 ms
> void FluidsynthWrapper::Render(float*, frame_t): Rendering took 897 ms
>
> virtual void MidiWrapper::open(): Added 28395 events to the seq, took 17 ms
> void FluidsynthWrapper::Render(float*, frame_t): Rendering took 1441 ms
>
> virtual void MidiWrapper::open(): Added 84961 events to the seq, took 48 ms
> void FluidsynthWrapper::Render(float*, frame_t): Rendering took 19709 ms
>
> The problem is the very first rendering call (in my case: fluid_synth_process), when the sample-timers are called back until they ultimately reach the while loop I mentioned. You can see there seems to be an exponential increase. ATM, I just tested 4 different MIDI files. The first 3 files have a playduration of 4-6 minutes, the last one has 12 minutes playback. Ofc, this heavily relies on the timing order of the events, so making a generalized benchmark is not so straight forward to do.
>
> Same test, but with std::priority_queue :
>
> virtual void MidiWrapper::open(): Added 23976 events to the seq, took 9 ms
> void FluidsynthWrapper::Render(float*, frame_t): Rendering took 4 ms
>
> virtual void MidiWrapper::open(): Added 28395 events to the seq, took 16 ms
> Warning: void FluidsynthWrapper::Render(float*, frame_t): Rendering took 3 ms
>
> MidiWrapper::open(): Added 84961 events to the seq, took 39 ms
> Warning: void FluidsynthWrapper::Render(float*, frame_t): Rendering took 4 ms
>
>
>>> 5. The system timer
>> isn't there a global timing event that MIDI devices can send to each other to synchronise playback position and playback speed?
>
> You seem to refer to "System Real Time Messages". But I don't know how this could be integrated into fluidsynth.
>
>>> 6. Time Scale
>> Similar comment as above and again, just thinking out loud: wouldn't it be a great feature to allow slower or faster MIDI playback?
>
> Indeed, supporting tempo changes would be a great and necessary feature for the seq, if we ever want to use it in the midi player. Ok, so better improve the time scaling rather than remove it.
>
>
>> adding another implementation in C++ is not cleanup IMO... it adds more technological debt because we now have to keep two different implementations in sync.
>
> Having two implementations would be temporarily. First add an "experimental" C++ impl., then deprecate the current C implementation while using the C++ impl. by default and finally remove the deprecated.
>
>> But I do wonder: what can C++ do that C can't? And especially: why can C++ do it faster?
>
> Simply that I'm more familiar how to do those things in C++ rather than with glib. And I'm not sure what Carlo, you, and the other embedded system guys said, if we fortify the glib dependency. I'm not sure how they think about a dependency to the C++ std lib either...
>
>
> Tom
>
>
>
>
> _______________________________________________
> fluid-dev mailing list
> [hidden email]
> https://lists.nongnu.org/mailman/listinfo/fluid-dev


_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: We need a new, better sequencer!

fluid-dev mailing list

 

Antoine, I was hoping you are listening :)

 

It is not quite clear to me why you need the system timer for scheduling those patterns, could you pls. elaborate? I would assume that you add a FLUID_SEQ_TIMER event to the synth and once you get the callback via fluid_event_callback_t you are provided with a timestamp by the sequencer. The timestamp currently comes from fluid_sequencer_get_tick(). Why does it matter that you need fluid_curtime() rather than currentMs which is provided by the synth?

 

seq->useSystemTimer ? (int) fluid_curtime() : fluid_atomic_int_get(&seq->currentMs);

 

 

> then we open ourselves up to bug reports where people ask: why does this (broken) MIDI file sound different in Fluidsynth than in almost all other MIDI players?

 

Not quite, because for now this change would be limited to the sequencer only, the fluid_player will be unchanged. The sequencer is a fluidsynth specific component, so I do believe that we have the freedom to tweak things here and there. So far I can't see any drawbacks of the event ordering I've proposed. And if we ever decide to make the fluid_player use the sequencer, we can just assign an alternative ordering function internally, that restores the FIFO ordering.

 

 

> Why would we need a deprecation period? Are you intending to change the behaviour or API of the sequencer?

 

I don't think an API change will be necessary. The deprecation just came to my mind because of the possible transition from C to C++.

 

 

> it surely can also be improved using an optimization of the current C implementation

 

The problem seems to be the heavy usage of linked lists. Lists have a poor cache performance. One would need to rewrite it using a contiguous container...

 

 

> Couldn't we simply implement that kind of heap/queue in C?

 

... we could. But I'm not a friend of adding yet another custom heap/queue implementation to the code base, when we can simply use one from a library.

 

 

[For the record, I'm appending Marcus' last mail below, which he mistakenly only sent to me]

 

Tom

 

 

10. Dezember 2019, 18:26:35 CET:

> Hi again,

>

> Am Di., 10. Dez. 2019 um 11:03 Uhr schrieb Tom M. <[hidden email]>:

>

> > > Why is "Note-On last" the better order? How do you know that that

> > > special (and seemingly Fluidsynth specific) ordering is what the author of

> > > the MIDI file wanted?

> >

> > When something happens at the same tick, I don't claim to know anything.

> > My proposed solution is simply the best guess I can come up with in such a

> > situation. Two examples:

> >

> > a) NoteOn and ProgChange happen in that order at the same tick. Would it

> > make sense to turn on the note using the old instrument, although we are

> > just about to receive a new program at the same tick? I don't think so. The

> > NoteOn, although first in the stream, should use the new program.

> >

> > b) NoteOn at tick 0, after e.g. 100 ticks: NoteOn and NoteOff. All three

> > note events happen in the given order on the same channel and trigger the

> > same key. How would fluidsynth interpret them currently? The first note is

> > triggered, it sounds for a while until the second NoteOn arrives. That

> > second NoteOn kills overlapping notes, i.e. it kills the first note. But

> > oops, just after that second note was turned on, it will be turned off

> > right again, because we have a NoteOff at the same tick, which apparently

> > was meant to turn off the first note though.

> >

> > For case b) I have MIDI files that do such things. Ofc you are right

> > Marcus, technically the MIDI converter program is to blame. I cannot really

> > change the converter though. I would alternatively need to handle those

> > cases in my program. I was just thinking of a possibly acceptable way for

> > fluidsynth to handle such "corner-cases".

> >

>

> I understand where you are coming from and I share your pain. But the ugly

> truth is: MIDI events on the same tick don't have a defined order in the

> spec. The only real extra information that we have that might carry some

> extra information about the intention of the author of the file is the

> order of events in the stream. All other orderings are completely

> artificial and by definition non-standard. And as you mention that almost

> all MIDI players use the stream order in these cases, that comes close to a

> agreed solution by the community in my opinion. And we would have to have a

> real good reason not to go along with the established standard.

>

> If we implement our own ordering, then we open ourselves up to bug reports

> where people ask: why does this (broken) MIDI file sound different in

> Fluidsynth than in almost all other MIDI players? And responding with

> "because it's broken" doesn't really work, because it works in almost all

> other players.

>

>

> > > > 3. Simpler implementation?

> > > What trips me up though: if your program was really stuck in that loop

> > > for seconds, I'm wondering how many events you were actually adding to the

> > > sequencer... Millions?

> >

> > I'm takling about ten-thousands of events. Here are some measurements:

> >

>

> Wow, that's quite a difference, thanks for the info!

>

>

> > > > 5. The system timer

> > > isn't there a global timing event that MIDI devices can send to each

> > > other to synchronise playback position and playback speed?

> >

> > You seem to refer to "System Real Time Messages". But I don't know how

> > this could be integrated into fluidsynth.

> >

>

> On second thought, this is probably out of scope for Fluidsynth. And to be

> honest, I don't really understand why we have two timing sources.

>

>

> > > adding another implementation in C++ is not cleanup IMO... it adds more

> > > technological debt because we now have to keep two different

> > > implementations in sync.

> >

> > Having two implementations would be temporarily. First add an

> > "experimental" C++ impl., then deprecate the current C implementation while

> > using the C++ impl. by default and finally remove the deprecated.

> >

>

> Why would we need a deprecation period? Are you intending to change the

> behaviour or API of the sequencer?

>

> > > But I do wonder: what can C++ do that C can't? And especially: why can

> > > C++ do it faster?

> >

> > Simply that I'm more familiar how to do those things in C++ rather than

> > with glib. And I'm not sure what Carlo, you, and the other embedded system

> > guys said, if we fortify the glib dependency. I'm not sure how they think

> > about a dependency to the C++ std lib either...

> >

>

> I'm not worried about glib, but Carlo probably has quite a different

> opinion :-)

>

> If what I found online is correct, then std::priority_queue uses a max-heap

> structure to store the data, hence its speed. Couldn't we simply implement

> that kind of heap/queue in C? I'm sure there is tried and tested code

> around that we could simply adopt.

>

> Cheers,

> Marcus

>

 


_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: We need a new, better sequencer!

Marcus Weseloh
Am Mi., 11. Dez. 2019 um 14:25 Uhr schrieb Tom M. via fluid-dev <[hidden email]>:
> > then we open ourselves up to bug reports where people ask: why does this (broken) MIDI file sound different in Fluidsynth than in almost all other MIDI players?
> Not quite, because for now this change would be limited to the sequencer only, the fluid_player will be unchanged. The sequencer is a fluidsynth specific component, so I do believe that we have the freedom to tweak things here and there. So far I can't see any drawbacks of the event ordering I've proposed. And if we ever decide to make the fluid_player use the sequencer, we can just assign an alternative ordering function internally, that restores the FIFO ordering.

Ah, I wasn't aware that the player doesn't use the sequencer internally. Still, the custom ordering of events might be at least surprising to sequencer users. And if I understand corrently, then it's a change of existing behaviour. I guess I still haven't understood whose problem you are actually trying to solve. If it is a problem specific to your usage of the sequencer, couldn't you ensure your preferred ordering of simultaneous events in your client application?

> > Couldn't we simply implement that kind of heap/queue in C?
> ... we could. But I'm not a friend of adding yet another custom heap/queue implementation to the code base, when we can simply use one from a library.

One good reason might be because it's not "simply use one from a library", as it creates additional dependencies. Is the sequencer heap/queue used in other parts of the code? If not, then it wouldn't be yet another implementation, but simply a different one.

> [For the record, I'm appending Marcus' last mail below, which he mistakenly only sent to me]

Sorry about that. And thanks for posting it here!

Cheers
Marcus

_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: We need a new, better sequencer!

Marcus Weseloh
Am Mi., 11. Dez. 2019 um 19:14 Uhr schrieb Marcus Weseloh <[hidden email]>:
Ah, I wasn't aware that the player doesn't use the sequencer internally. Still, the custom ordering of events might be at least surprising to sequencer users. And if I understand corrently, then it's a change of existing behaviour. I guess I still haven't understood whose problem you are actually trying to solve. If it is a problem specific to your usage of the sequencer, couldn't you ensure your preferred ordering of simultaneous events in your client application?

Just one more thought... one drawback that I see with the ordering you propose: you currently have the option to choose a different order in your client application because we currently process events in the order they were added. If we change Fluidsynth to force a different order, sequencer users would not be in complete control anymore. Is the problem you are trying to solve big enough to justify that?

Cheers
Marcus

_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: We need a new, better sequencer!

fluid-dev mailing list

 

> one drawback that I see with the ordering you propose: you currently have the option to choose a different order in your client application because we currently process events in the order they were added.

 

Primarily, we process events according to timestamps. Here, we are talking about a corner-case, when events have the same timestamp. How fluidsynth treats those events with the same timestamp is currently not documented anywhere (AFAIK). It is only fluidsynth's current implementation that treats it FIFO. Implementations are subject to change. And because this corner-case hasn't been communicated to the user, I don't really consider it to be a change in behaviour. ATM, I only see the two advantages mentioned before. And I do believe that makes it "big" enough to justify the change.

 

> Still, the custom ordering of events might be at least surprising to sequencer users. And if I understand corrently, then it's a change of existing behaviour. I guess I still haven't understood whose problem you are actually trying to solve. If it is a problem specific to your usage of the sequencer, couldn't you ensure your preferred ordering of simultaneous events in your client application?

 

Right now, it seems to be only me experiencing this problem (... and Francesco Ariis, who I mentioned initially; just remove the multiple midi tracks by saving his hymn1.mid as SMF0). Admittedly, I am completely biased and selfish: Ofc, I could defer it to my client app. In this case, the MIDI parsing lib would manage its sorted event array internally. Then, my app would come along, sort/fix all events with the same tickvalue. And finally my app would pass everyting to the fluid_sequencer, which sorts the events again to manage its heap based structure. ...It's just redundant amount of work and I was thinking about how to turn this undefined behaviour in a well-defined one (at least for fluidsynth).

 

---

 

In the meantime, I did some real testing:

 

Attached is a (simplified) MIDI file illustrating the problem. It starts two different notes at the same time. After some delay it starts the same note pair again, after that but still at the same tick, it turns off the notes. After another delay, it starts the same two notes again and turns them off right at the same tick.

 

When playing it with fluidsynth you will only hear the first note pair sounding, ie. after 8 sec. there'll silence. When you import the MIDI in an editor like rosegarden, qtractor or MuSE, they correctly recognize that there are two note pairs, making the MIDI file sound for 16 sec. (The treatment of the zero length notepair at the end differs between the programs though... not of interest to us anyway). LMMS however, only recognized the first note pair.

 

For being absolutely sure, I went to the basement to fetch my old Emu Proteus FX hardware synth: It makes the first note pair sound as expected. The second and the third note pairs are so short that they are barely audible. Apparently both are treated as zero-duration notes. When changing the order of events (i.e. turn off the first note pair, then turn on the second pair, still both at the same tick) it works fine. This makes me believe that the FIFO ordering is a leftover from the 90s, where events were transmitted in real-time between different synths, without knowing what comes next. With fluidsynths sequencer however, we have the knowledge which event(s) are about to come next. So IMO, fluidsynth should handle events at the same tick in a smart(er) way.

 

 

In case somebody is interested in my progress with the C++ implementation, here's the current dev branch:

https://github.com/FluidSynth/fluidsynth/commits/new-seq2

 

And unit tests for the sequencer are ready:

https://github.com/FluidSynth/fluidsynth/pull/598

 

 

Tom

 


_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev

noteOnOffTest.mid (142 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: We need a new, better sequencer!

Marcus Weseloh
Am Do., 12. Dez. 2019 um 12:03 Uhr schrieb Tom M. via fluid-dev
<[hidden email]>:
> Here, we are talking about a corner-case, when events have the same timestamp. [...] Implementations are subject to change. And because this corner-case hasn't been communicated to the user, I don't really consider it to be a change in behaviour.

Ok, that sounds reasonable.

> I was thinking about how to turn this undefined behaviour in a well-defined one (at least for fluidsynth).

That also sounds good. I think we only differ in our idea of what
would be the best or most natural behaviour. :-)

> Attached is a (simplified) MIDI file illustrating the problem. It starts two different notes at the same time. After some delay it starts the same note pair again, after that but still at the same tick, it turns off the notes. After another delay, it starts the same two notes again and turns them off right at the same tick.
> When playing it with fluidsynth you will only hear the first note pair sounding, ie. after 8 sec. there'll silence. When you import the MIDI in an editor like rosegarden, qtractor or MuSE, they correctly recognize that there are two note pairs, making the MIDI file sound for 16 sec. (The treatment of the zero length notepair at the end differs between the programs though... not of interest to us anyway). LMMS however, only recognized the first note pair.

Thanks, very interesting, especially the MuSE, Rosegarden et al.
behaviour! Still... I don't understand how their behaviour is
"correct" and Fluidsynths behaviour isn't. It seems like we simply
have different "gut feelings" about what's right here. As I mainly
deal with real-time MIDI, the event order makes absolute sense to me.
Doing custom orderings in the synth just feels wrong. But this is such
a small change and in a part of Fluidsynth that I don't really use, so
let me just register my -0 for this change and then I'll shut my mouth
:-)

> In case somebody is interested in my progress with the C++ implementation, here's the current dev branch:
> https://github.com/FluidSynth/fluidsynth/commits/new-seq2

Looks nice and clean... but I don't speak C++ so I can't really
comment in any detail. And that is also a reason I'm -0 to -1 on
adding the C++ dependency and (more importantly) C++ code to
Fluidsynth.

Cheers
Marcus

_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: We need a new, better sequencer!

Antoine Schmitt
In reply to this post by fluid-dev mailing list
Hello,

well, it took me some time to delves into this 18 years old code, that has moreover changed quite a bit since.

You are right, the real time client can rely on the currentMs time of the sequencer. No need for the fluid_curtime().

Cheers
Antoine


Le 11 déc. 2019 à 11:57, Tom M. via fluid-dev <[hidden email]> a écrit :

 

Antoine, I was hoping you are listening :)

 

It is not quite clear to me why you need the system timer for scheduling those patterns, could you pls. elaborate? I would assume that you add a FLUID_SEQ_TIMER event to the synth and once you get the callback via fluid_event_callback_t you are provided with a timestamp by the sequencer. The timestamp currently comes from fluid_sequencer_get_tick(). Why does it matter that you need fluid_curtime() rather than currentMs which is provided by the synth?

 

seq->useSystemTimer ? (int) fluid_curtime() : fluid_atomic_int_get(&seq->currentMs);

 

 

> then we open ourselves up to bug reports where people ask: why does this (broken) MIDI file sound different in Fluidsynth than in almost all other MIDI players?

 

Not quite, because for now this change would be limited to the sequencer only, the fluid_player will be unchanged. The sequencer is a fluidsynth specific component, so I do believe that we have the freedom to tweak things here and there. So far I can't see any drawbacks of the event ordering I've proposed. And if we ever decide to make the fluid_player use the sequencer, we can just assign an alternative ordering function internally, that restores the FIFO ordering.

 

 

> Why would we need a deprecation period? Are you intending to change the behaviour or API of the sequencer?

 

I don't think an API change will be necessary. The deprecation just came to my mind because of the possible transition from C to C++.

 

 

> it surely can also be improved using an optimization of the current C implementation

 

The problem seems to be the heavy usage of linked lists. Lists have a poor cache performance. One would need to rewrite it using a contiguous container...

 

 

> Couldn't we simply implement that kind of heap/queue in C?

 

... we could. But I'm not a friend of adding yet another custom heap/queue implementation to the code base, when we can simply use one from a library.

 

 

[For the record, I'm appending Marcus' last mail below, which he mistakenly only sent to me]

 

Tom

 

 

10. Dezember 2019, 18:26:35 CET:
> Hi again,
>
> Am Di., 10. Dez. 2019 um 11:03 Uhr schrieb Tom M. <[hidden email]>:
>
> > > Why is "Note-On last" the better order? How do you know that that
> > > special (and seemingly Fluidsynth specific) ordering is what the author of
> > > the MIDI file wanted?
> >
> > When something happens at the same tick, I don't claim to know anything.
> > My proposed solution is simply the best guess I can come up with in such a
> > situation. Two examples:
> >
> > a) NoteOn and ProgChange happen in that order at the same tick. Would it
> > make sense to turn on the note using the old instrument, although we are
> > just about to receive a new program at the same tick? I don't think so. The
> > NoteOn, although first in the stream, should use the new program.
> >
> > b) NoteOn at tick 0, after e.g. 100 ticks: NoteOn and NoteOff. All three
> > note events happen in the given order on the same channel and trigger the
> > same key. How would fluidsynth interpret them currently? The first note is
> > triggered, it sounds for a while until the second NoteOn arrives. That
> > second NoteOn kills overlapping notes, i.e. it kills the first note. But
> > oops, just after that second note was turned on, it will be turned off
> > right again, because we have a NoteOff at the same tick, which apparently
> > was meant to turn off the first note though.
> >
> > For case b) I have MIDI files that do such things. Ofc you are right
> > Marcus, technically the MIDI converter program is to blame. I cannot really
> > change the converter though. I would alternatively need to handle those
> > cases in my program. I was just thinking of a possibly acceptable way for
> > fluidsynth to handle such "corner-cases".
> >
>
> I understand where you are coming from and I share your pain. But the ugly
> truth is: MIDI events on the same tick don't have a defined order in the
> spec. The only real extra information that we have that might carry some
> extra information about the intention of the author of the file is the
> order of events in the stream. All other orderings are completely
> artificial and by definition non-standard. And as you mention that almost
> all MIDI players use the stream order in these cases, that comes close to a
> agreed solution by the community in my opinion. And we would have to have a
> real good reason not to go along with the established standard.
>
> If we implement our own ordering, then we open ourselves up to bug reports
> where people ask: why does this (broken) MIDI file sound different in
> Fluidsynth than in almost all other MIDI players? And responding with
> "because it's broken" doesn't really work, because it works in almost all
> other players.
>
>
> > > > 3. Simpler implementation?
> > > What trips me up though: if your program was really stuck in that loop
> > > for seconds, I'm wondering how many events you were actually adding to the
> > > sequencer... Millions?
> >
> > I'm takling about ten-thousands of events. Here are some measurements:
> >
>
> Wow, that's quite a difference, thanks for the info!
>
>
> > > > 5. The system timer
> > > isn't there a global timing event that MIDI devices can send to each
> > > other to synchronise playback position and playback speed?
> >
> > You seem to refer to "System Real Time Messages". But I don't know how
> > this could be integrated into fluidsynth.
> >
>
> On second thought, this is probably out of scope for Fluidsynth. And to be
> honest, I don't really understand why we have two timing sources.
>
>
> > > adding another implementation in C++ is not cleanup IMO... it adds more
> > > technological debt because we now have to keep two different
> > > implementations in sync.
> >
> > Having two implementations would be temporarily. First add an
> > "experimental" C++ impl., then deprecate the current C implementation while
> > using the C++ impl. by default and finally remove the deprecated.
> >
>
> Why would we need a deprecation period? Are you intending to change the
> behaviour or API of the sequencer?
>
> > > But I do wonder: what can C++ do that C can't? And especially: why can
> > > C++ do it faster?
> >
> > Simply that I'm more familiar how to do those things in C++ rather than
> > with glib. And I'm not sure what Carlo, you, and the other embedded system
> > guys said, if we fortify the glib dependency. I'm not sure how they think
> > about a dependency to the C++ std lib either...
> >
>
> I'm not worried about glib, but Carlo probably has quite a different
> opinion :-)
>
> If what I found online is correct, then std::priority_queue uses a max-heap
> structure to store the data, hence its speed. Couldn't we simply implement
> that kind of heap/queue in C? I'm sure there is tried and tested code
> around that we could simply adopt.
>
> Cheers,
> Marcus
>

 

_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev


_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: We need a new, better sequencer!

Antoine Schmitt
In reply to this post by Marcus Weseloh
Yes, this was exactly my point of giving the responsibility to the client. If the FIFO behavior of the sequencer is documented, then the client can decide the order. If the sequencer does its own custom ordering, the client looses control.

Le 11 déc. 2019 à 19:27, Marcus Weseloh <[hidden email]> a écrit :

Am Mi., 11. Dez. 2019 um 19:14 Uhr schrieb Marcus Weseloh <[hidden email]>:
Ah, I wasn't aware that the player doesn't use the sequencer internally. Still, the custom ordering of events might be at least surprising to sequencer users. And if I understand corrently, then it's a change of existing behaviour. I guess I still haven't understood whose problem you are actually trying to solve. If it is a problem specific to your usage of the sequencer, couldn't you ensure your preferred ordering of simultaneous events in your client application?

Just one more thought... one drawback that I see with the ordering you propose: you currently have the option to choose a different order in your client application because we currently process events in the order they were added. If we change Fluidsynth to force a different order, sequencer users would not be in complete control anymore. Is the problem you are trying to solve big enough to justify that?

Cheers
Marcus
_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev


_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: We need a new, better sequencer!

fluid-dev mailing list

Thank you Marcus and Antoine for your opinions.

 

I've file a PR for deprecating the system timer: https://github.com/FluidSynth/fluidsynth/pull/599

 

Also, I had a look into glib. Looks like it doesn't provide a heap sort. I guess we would need a third party lib anyway, which is why I will continue to pursue shaping the C++ implementation. I'll call back when it's ready.

 

Tom

 


_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: We need a new, better sequencer!

fluid-dev mailing list
In reply to this post by Antoine Schmitt

FYI: A draft C++ implementation is now ready. Detailed information can be found on Github:

 

https://github.com/FluidSynth/fluidsynth/pull/604

 

I've already written a few unit tests for it, to make sure it stays backward compatible with the current implementation. I'll more intensively test it in the upcoming weeks/months by integrating it into my app. In any case, feedback is highly welcome!

 

Tom

 


_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev
Reply | Threaded
Open this post in threaded view
|

Re: We need a new, better sequencer!

fluid-dev mailing list
In reply to this post by Antoine Schmitt
FYI: A new implementation is now ready and proposed here:

https://github.com/FluidSynth/fluidsynth/pull/604#issuecomment-616091967

It's the C++ implementation we've talked about earlier, because it received quite positive feedback from the mailing list compared to the glib approach.

An open concern was the event ordering at the same tick I've proposed. With this new implementation, events that have the same tick are given the following order:

- #FLUID_SEQ_SYSTEMRESET events precede any other event type.
- #FLUID_SEQ_UNREGISTERING events succeed #FLUID_SEQ_SYSTEMRESET and precede other event type.
- #FLUID_SEQ_NOTEON and #FLUID_SEQ_NOTE events succeed any other event type.
- Otherwise the order is undefined.

I do not see how this ordering could break any existing application. Events are ordered by tick and it stays that way. On the contrary, it ensures that NoteOns are spawned with the correct configuration that has been set up at a given tick. Which is important, when you consider MIDI files that have their NoteOn and programChange scattered over two different tracks (https://lists.nongnu.org/archive/html/fluid-dev/2017-05/msg00004.html).

In addition, the sequencer now supports tempo changes, because you can change its scale to any arbitrary value.

And No, this PR will not affect fluidsynth's MIDI player behaviour (for the moment).

Feedback is welcome.

Tom



_______________________________________________
fluid-dev mailing list
[hidden email]
https://lists.nongnu.org/mailman/listinfo/fluid-dev