2019-12-12 10:42:34 +00:00
|
|
|
|
|
|
|
#include "test.h"
|
|
|
|
#include "fluidsynth.h" // use local fluidsynth header
|
|
|
|
#include "fluid_event.h"
|
Revise the sequencer's event queue (#604)
Proposing a new event queue for the sequencer, based on prior discussion:
https://lists.nongnu.org/archive/html/fluid-dev/2019-12/msg00001.html
With this change fluidsynth will require a C++98 compliant compiler.
Consider this as RFC, feedback is welcome.
The "pain-points" from the discussion:
#### 1. It is slow.
Done (thanks to heap sort), see runtime of `test_seq_event_queue_sort`.
#### 2. A meaningful ordering for events with the same tick has not been considered.
Done, see comments in `fluid_seq_queue.cpp`.
#### 3. Complicated implementation
Now uses one single event queue, which requires C++98. Implemented similarly to std::priority_queue by using heap sort.
The "queue" I use currently is of type `std::deque`. `deque` does not provide preallocation. `std::vector` does provide it. However, `std::deque` has the huge advantage that appending additional elements is cheap. For `std::vector` appending new elements would require to reallocate all the memory and copy it to the new array. So,
* either use `std::deque` with the risk that memory allocation may occur during `fluid_sequencer_send_at()`, or
* use `std::vector` with a preallocated pool of events and make `fluid_sequencer_send_at()` when the `vector` runs out of capacity.
Comments?
#### 4. Events that have been processed are deleted and gone.
After having thought about this more, this is the correct behavior. After events have been dispatched, they must be released to free underlying memory, see point 3. For the very rare case that a client (e.g. fluid_player) may require those events in the future, the client should be responsible for storing the events somewhere.
#### 5. The sequencer supports the system timer as alternative time source.
The conclusion from the mailing list was that the system timer can be removed. This has been done.
#### 6. Time Scaling
Time scaling can now be used for arbitrary tempo changes. The previous implementation was capable of that as well, however, the time-scale was limited to 1000. The only limitation for the scale is now >0, see `test_seq_scale`.
### Other Points
* `fluid_sequencer_remove_events()` turned out to be broken before, as it did not remove all events from the queue. This has been fixed, see `test_seq_event_queue_remove`.
* Consider the following code executed by `threadA`:
```c
fluid_sequencer_send_at(event0);
fluid_sequencer_set_time_scale(); // new scale
fluid_sequencer_send_at(event1);
```
The new scale will be definitely applied to `event1`. However, if another concurrently running `threadB` executes `fluid_sequencer_process()`, it was previously not clear, whether the new scale was also applied to event0. This depends on whether `event0` was still in the `preQueue`, and this depends on `event0.time` and the tick count that `fluid_sequencer_process()` is being called with. This has been changed. As of now, events are queued with their timestamp AS-IS. And only the latest call to `fluid_sequencer_set_time_scale()` is being considered during `fluid_sequencer_process()`. This makes the implementation very simple, i.e. no events need to be changed and the sequencer doesn't have to be locked down. On the other hand, it means that `fluid_sequencer_set_time_scale()` can only be used for tempo changes when called from the sequencer callback. In other words, if `threadA` executes the code above followed by `fluid_sequencer_process()`, `event0` and `event1` will be executed with the same tempo, which is the latest scale provided to the seq. Is this acceptable? The old implementation had the same limitation. And when looking through the internet, I only find users who call `fluid_sequencer_set_time_scale()` from the sequencer callback. Still, this is a point I'm raising for discussion.
2020-05-26 15:16:22 +00:00
|
|
|
#include "fluidsynth_priv.h"
|
2019-12-12 10:42:34 +00:00
|
|
|
|
|
|
|
#include <limits.h>
|
|
|
|
|
|
|
|
static short order = 0;
|
|
|
|
void callback_stable_sort(unsigned int time, fluid_event_t *event, fluid_sequencer_t *seq, void *data)
|
|
|
|
{
|
2020-02-01 13:32:35 +00:00
|
|
|
static const int expected_type_order[] =
|
Revise the sequencer's event queue (#604)
Proposing a new event queue for the sequencer, based on prior discussion:
https://lists.nongnu.org/archive/html/fluid-dev/2019-12/msg00001.html
With this change fluidsynth will require a C++98 compliant compiler.
Consider this as RFC, feedback is welcome.
The "pain-points" from the discussion:
#### 1. It is slow.
Done (thanks to heap sort), see runtime of `test_seq_event_queue_sort`.
#### 2. A meaningful ordering for events with the same tick has not been considered.
Done, see comments in `fluid_seq_queue.cpp`.
#### 3. Complicated implementation
Now uses one single event queue, which requires C++98. Implemented similarly to std::priority_queue by using heap sort.
The "queue" I use currently is of type `std::deque`. `deque` does not provide preallocation. `std::vector` does provide it. However, `std::deque` has the huge advantage that appending additional elements is cheap. For `std::vector` appending new elements would require to reallocate all the memory and copy it to the new array. So,
* either use `std::deque` with the risk that memory allocation may occur during `fluid_sequencer_send_at()`, or
* use `std::vector` with a preallocated pool of events and make `fluid_sequencer_send_at()` when the `vector` runs out of capacity.
Comments?
#### 4. Events that have been processed are deleted and gone.
After having thought about this more, this is the correct behavior. After events have been dispatched, they must be released to free underlying memory, see point 3. For the very rare case that a client (e.g. fluid_player) may require those events in the future, the client should be responsible for storing the events somewhere.
#### 5. The sequencer supports the system timer as alternative time source.
The conclusion from the mailing list was that the system timer can be removed. This has been done.
#### 6. Time Scaling
Time scaling can now be used for arbitrary tempo changes. The previous implementation was capable of that as well, however, the time-scale was limited to 1000. The only limitation for the scale is now >0, see `test_seq_scale`.
### Other Points
* `fluid_sequencer_remove_events()` turned out to be broken before, as it did not remove all events from the queue. This has been fixed, see `test_seq_event_queue_remove`.
* Consider the following code executed by `threadA`:
```c
fluid_sequencer_send_at(event0);
fluid_sequencer_set_time_scale(); // new scale
fluid_sequencer_send_at(event1);
```
The new scale will be definitely applied to `event1`. However, if another concurrently running `threadB` executes `fluid_sequencer_process()`, it was previously not clear, whether the new scale was also applied to event0. This depends on whether `event0` was still in the `preQueue`, and this depends on `event0.time` and the tick count that `fluid_sequencer_process()` is being called with. This has been changed. As of now, events are queued with their timestamp AS-IS. And only the latest call to `fluid_sequencer_set_time_scale()` is being considered during `fluid_sequencer_process()`. This makes the implementation very simple, i.e. no events need to be changed and the sequencer doesn't have to be locked down. On the other hand, it means that `fluid_sequencer_set_time_scale()` can only be used for tempo changes when called from the sequencer callback. In other words, if `threadA` executes the code above followed by `fluid_sequencer_process()`, `event0` and `event1` will be executed with the same tempo, which is the latest scale provided to the seq. Is this acceptable? The old implementation had the same limitation. And when looking through the internet, I only find users who call `fluid_sequencer_set_time_scale()` from the sequencer callback. Still, this is a point I'm raising for discussion.
2020-05-26 15:16:22 +00:00
|
|
|
{ FLUID_SEQ_NOTEOFF, FLUID_SEQ_NOTEON, FLUID_SEQ_SYSTEMRESET, FLUID_SEQ_UNREGISTERING
|
|
|
|
/* technically, FLUID_SEQ_NOTEOFF and FLUID_SEQ_NOTEON are to follow, but we've already unregisted */ };
|
2019-12-12 10:42:34 +00:00
|
|
|
|
|
|
|
TEST_ASSERT(fluid_event_get_type(event) == expected_type_order[order++]);
|
|
|
|
}
|
|
|
|
|
|
|
|
void test_order_same_tick(fluid_sequencer_t *seq, fluid_event_t *evt)
|
|
|
|
{
|
|
|
|
// silently creates a fluid_seqbind_t
|
|
|
|
int i, seqid = fluid_sequencer_register_client(seq, "test order at same tick", callback_stable_sort, NULL);
|
|
|
|
TEST_SUCCESS(seqid);
|
|
|
|
TEST_ASSERT(fluid_sequencer_count_clients(seq) == 1);
|
|
|
|
|
|
|
|
fluid_event_set_source(evt, -1);
|
|
|
|
fluid_event_set_dest(evt, seqid);
|
|
|
|
|
|
|
|
for(i = 1; i <= 2; i++)
|
|
|
|
{
|
|
|
|
fluid_event_noteoff(evt, 0, 64);
|
|
|
|
TEST_SUCCESS(fluid_sequencer_send_at(seq, evt, i, 1));
|
|
|
|
fluid_event_noteon(evt, 0, 64, 127);
|
|
|
|
TEST_SUCCESS(fluid_sequencer_send_at(seq, evt, i, 1));
|
|
|
|
}
|
|
|
|
|
2020-02-01 13:32:35 +00:00
|
|
|
fluid_event_system_reset(evt);
|
|
|
|
TEST_SUCCESS(fluid_sequencer_send_at(seq, evt, 2, 1));
|
|
|
|
fluid_event_unregistering(evt);
|
|
|
|
TEST_SUCCESS(fluid_sequencer_send_at(seq, evt, 2, 1));
|
2019-12-12 10:42:34 +00:00
|
|
|
|
2020-02-01 13:32:35 +00:00
|
|
|
fluid_sequencer_process(seq, 1);
|
|
|
|
TEST_ASSERT(order == 2);
|
2019-12-12 10:42:34 +00:00
|
|
|
|
|
|
|
fluid_sequencer_process(seq, 2);
|
Revise the sequencer's event queue (#604)
Proposing a new event queue for the sequencer, based on prior discussion:
https://lists.nongnu.org/archive/html/fluid-dev/2019-12/msg00001.html
With this change fluidsynth will require a C++98 compliant compiler.
Consider this as RFC, feedback is welcome.
The "pain-points" from the discussion:
#### 1. It is slow.
Done (thanks to heap sort), see runtime of `test_seq_event_queue_sort`.
#### 2. A meaningful ordering for events with the same tick has not been considered.
Done, see comments in `fluid_seq_queue.cpp`.
#### 3. Complicated implementation
Now uses one single event queue, which requires C++98. Implemented similarly to std::priority_queue by using heap sort.
The "queue" I use currently is of type `std::deque`. `deque` does not provide preallocation. `std::vector` does provide it. However, `std::deque` has the huge advantage that appending additional elements is cheap. For `std::vector` appending new elements would require to reallocate all the memory and copy it to the new array. So,
* either use `std::deque` with the risk that memory allocation may occur during `fluid_sequencer_send_at()`, or
* use `std::vector` with a preallocated pool of events and make `fluid_sequencer_send_at()` when the `vector` runs out of capacity.
Comments?
#### 4. Events that have been processed are deleted and gone.
After having thought about this more, this is the correct behavior. After events have been dispatched, they must be released to free underlying memory, see point 3. For the very rare case that a client (e.g. fluid_player) may require those events in the future, the client should be responsible for storing the events somewhere.
#### 5. The sequencer supports the system timer as alternative time source.
The conclusion from the mailing list was that the system timer can be removed. This has been done.
#### 6. Time Scaling
Time scaling can now be used for arbitrary tempo changes. The previous implementation was capable of that as well, however, the time-scale was limited to 1000. The only limitation for the scale is now >0, see `test_seq_scale`.
### Other Points
* `fluid_sequencer_remove_events()` turned out to be broken before, as it did not remove all events from the queue. This has been fixed, see `test_seq_event_queue_remove`.
* Consider the following code executed by `threadA`:
```c
fluid_sequencer_send_at(event0);
fluid_sequencer_set_time_scale(); // new scale
fluid_sequencer_send_at(event1);
```
The new scale will be definitely applied to `event1`. However, if another concurrently running `threadB` executes `fluid_sequencer_process()`, it was previously not clear, whether the new scale was also applied to event0. This depends on whether `event0` was still in the `preQueue`, and this depends on `event0.time` and the tick count that `fluid_sequencer_process()` is being called with. This has been changed. As of now, events are queued with their timestamp AS-IS. And only the latest call to `fluid_sequencer_set_time_scale()` is being considered during `fluid_sequencer_process()`. This makes the implementation very simple, i.e. no events need to be changed and the sequencer doesn't have to be locked down. On the other hand, it means that `fluid_sequencer_set_time_scale()` can only be used for tempo changes when called from the sequencer callback. In other words, if `threadA` executes the code above followed by `fluid_sequencer_process()`, `event0` and `event1` will be executed with the same tempo, which is the latest scale provided to the seq. Is this acceptable? The old implementation had the same limitation. And when looking through the internet, I only find users who call `fluid_sequencer_set_time_scale()` from the sequencer callback. Still, this is a point I'm raising for discussion.
2020-05-26 15:16:22 +00:00
|
|
|
TEST_ASSERT(order == 4);
|
2019-12-12 10:42:34 +00:00
|
|
|
|
|
|
|
fluid_sequencer_unregister_client(seq, seqid);
|
|
|
|
TEST_ASSERT(fluid_sequencer_count_clients(seq) == 0);
|
|
|
|
}
|
|
|
|
|
2020-02-01 13:32:35 +00:00
|
|
|
static unsigned int prev_time, done = FALSE;
|
2019-12-12 10:42:34 +00:00
|
|
|
void callback_correct_order(unsigned int time, fluid_event_t *event, fluid_sequencer_t *seq, void *data)
|
|
|
|
{
|
2020-02-01 13:32:35 +00:00
|
|
|
if(done)
|
|
|
|
{
|
|
|
|
TEST_ASSERT(fluid_event_get_type(event) == FLUID_SEQ_UNREGISTERING);
|
|
|
|
}
|
|
|
|
else
|
|
|
|
{
|
|
|
|
TEST_ASSERT(fluid_event_get_type(event) == FLUID_SEQ_CONTROLCHANGE);
|
|
|
|
}
|
2019-12-12 10:42:34 +00:00
|
|
|
TEST_ASSERT(prev_time <= fluid_event_get_time(event) && fluid_event_get_time(event) <= prev_time + 1);
|
|
|
|
prev_time = fluid_event_get_time(event);
|
|
|
|
}
|
|
|
|
|
|
|
|
void test_correct_order(fluid_sequencer_t *seq, fluid_event_t *evt)
|
|
|
|
{
|
|
|
|
// silently creates a fluid_seqbind_t
|
|
|
|
unsigned int i, offset;
|
|
|
|
int seqid = fluid_sequencer_register_client(seq, "correct order test", callback_correct_order, NULL);
|
|
|
|
TEST_SUCCESS(seqid);
|
|
|
|
|
|
|
|
fluid_event_set_source(evt, -1);
|
|
|
|
fluid_event_set_dest(evt, seqid);
|
|
|
|
fluid_event_control_change(evt, 0, 1, 127);
|
|
|
|
|
|
|
|
for(i = 0; i < 10000; i++)
|
|
|
|
{
|
|
|
|
TEST_SUCCESS(fluid_sequencer_send_at(seq, evt, 10000 - i, 0));
|
|
|
|
}
|
|
|
|
|
|
|
|
for(; i <= 10000 + 20000; i++)
|
|
|
|
{
|
|
|
|
TEST_SUCCESS(fluid_sequencer_send_at(seq, evt, i, 0));
|
|
|
|
}
|
|
|
|
|
|
|
|
for(; i < 80000; i++)
|
|
|
|
{
|
|
|
|
TEST_SUCCESS(fluid_sequencer_send_at(seq, evt, 80000 - (i - 10000 - 20000), 0));
|
|
|
|
}
|
|
|
|
|
|
|
|
for(; i < 200000; i++)
|
|
|
|
{
|
|
|
|
TEST_SUCCESS(fluid_sequencer_send_at(seq, evt, i, 0));
|
|
|
|
}
|
|
|
|
|
|
|
|
offset = prev_time = fluid_sequencer_get_tick(seq);
|
|
|
|
fluid_sequencer_process(seq, i + offset);
|
|
|
|
TEST_ASSERT(prev_time == (i - 1) + offset);
|
|
|
|
|
2020-02-01 13:32:35 +00:00
|
|
|
done = TRUE;
|
2019-12-12 10:42:34 +00:00
|
|
|
fluid_sequencer_unregister_client(seq, seqid);
|
|
|
|
}
|
|
|
|
|
|
|
|
// simple test to ensure that manually unregistering and deleting the internal fluid_seqbind_t works without crashing
|
|
|
|
int main(void)
|
|
|
|
{
|
|
|
|
fluid_event_t *evt;
|
|
|
|
fluid_sequencer_t *seq = new_fluid_sequencer2(0 /*i.e. use sample timer*/);
|
|
|
|
TEST_ASSERT(seq != NULL);
|
|
|
|
TEST_ASSERT(fluid_sequencer_get_use_system_timer(seq) == 0);
|
|
|
|
evt = new_fluid_event();
|
|
|
|
TEST_ASSERT(evt != NULL);
|
|
|
|
|
|
|
|
test_order_same_tick(seq, evt);
|
|
|
|
test_correct_order(seq, evt);
|
|
|
|
|
|
|
|
// client should be removed, deleting the seq should not free the struct again
|
|
|
|
delete_fluid_event(evt);
|
|
|
|
delete_fluid_sequencer(seq);
|
|
|
|
|
|
|
|
return EXIT_SUCCESS;
|
|
|
|
}
|