zdoom/fluidsynth
0
0
Fork 0
mirror of https://github.com/ZDoom/fluidsynth.git synced 2025-01-19 07:50:49 +00:00
Code Issues Projects Releases Packages Wiki Activity

Solve the sequencer client unregistering problem (#610)

Browse source 
Responsibility for calling fluid_sequencer_unregister_client() in case of FLUID_SEQ_UNREGISTERING events has been moved to fluid_sequencer_send_now(). In other words, a FLUID_SEQ_UNREGISTERING event now really unregisters the client, no matter how the client's callback function looks like.

Avoids leaking the sequencer clients if implementations do not unregister them explicitly.

Also fixes another memory leak if fluid_sequencer_register_fluidsynth() clients were unregistered with fluid_sequencer_unregister_client() rather than by sending an unregistering event.
This commit is contained in:
Tom M 2020-02-01 14:32:35 +01:00 committed by GitHub
parent 943ed37e54
commit af2342ac43
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 62 additions and 39 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
doc/fluidsynth-v20-devdoc.txt
View file

@ -70,6 +70,7 @@ What is FluidSynth?
\section NewIn2_1_1 What's new in 2.1.1?
- requirements for explicit sequencer client unregistering have been relaxed: delete_fluid_sequencer() now correctly frees any registered sequencer clients (clients can still be explicitly unregistered)
- using the sequencer with the system timer as timing source has been deprecated
\section NewIn2_1_0 What's new in 2.1.0?
@ -121,7 +122,7 @@ FluidSynths major version was bumped. The API was reworked, deprecated functions
- all public \c fluid_settings_* functions that return an integer which is not meant to be interpreted as bool consistently return either FLUID_OK or FLUID_FAILED
- fluid_settings_setstr() cannot be used to set integer (toggle) settings with "yes" or "no" values anymore. Use fluid_settings_setint() instead, for example: <br /><code>fluid_settings_setint(settings, "synth.reverb.active", 0)</code> instead of <code>fluid_settings_setstr(settings, "synth.reverb.active", "no")</code>
- explicit client unregistering is required for fluid_sequencer_register_client() and fluid_sequencer_register_fluidsynth()
- <span style="text-decoration:line-through;">explicit client unregistering is required for fluid_sequencer_register_client() and fluid_sequencer_register_fluidsynth()</span> (since fluidsynth 2.1.1 not required anymore, but still recommend)
- all public functions consistently receive signed integers for soundfont ids, bank and program numbers
- use unique device names for the "audio.portaudio.device" setting
- fluid_synth_process() received a new more flexible implementation, but now requires zeroed-out sample buffers

48
src/midi/fluid_seq.c
View file

@ -143,7 +143,7 @@ new_fluid_sequencer2(int use_system_timer)
/**
* Free a sequencer object.
* @note Registered sequencer clients may not be fully freed by this function. Explicitly unregister them with fluid_sequencer_unregister_client().
* @note Before fluidsynth 2.1.1 registered sequencer clients may not be fully freed by this function.
* @param seq Sequencer to delete
*/
void
@ -160,17 +160,6 @@ delete_fluid_sequencer(fluid_sequencer_t *seq)
_fluid_seq_queue_end(seq);
/* if (seq->clients) {
fluid_list_t *tmp = seq->clients;
while (tmp != NULL) {
fluid_sequencer_client_t *client = (fluid_sequencer_client_t*)tmp->data;
if (client->name) FLUID_FREE(client->name);
tmp = tmp->next;
}
delete_fluid_list(seq->clients);
seq->clients = NULL;
}*/
FLUID_FREE(seq);
}
@ -200,11 +189,10 @@ fluid_sequencer_get_use_system_timer(fluid_sequencer_t *seq)
* @param data User data to pass to the \a callback
* @return Unique sequencer ID or #FLUID_FAILED on error
*
* Clients can be sources or destinations of events. Sources don't need to
* Clients can be sources or destinations of events. Sources don't need to
* register a callback.
*
* @note The user must explicitly unregister any registered client with fluid_sequencer_unregister_client()
* before deleting the sequencer!
* @note Implementations are encouraged to explicitly unregister any registered client with fluid_sequencer_unregister_client() before deleting the sequencer.
*/
fluid_seq_id_t
fluid_sequencer_register_client(fluid_sequencer_t *seq, const char *name,
@ -253,9 +241,16 @@ void
fluid_sequencer_unregister_client(fluid_sequencer_t *seq, fluid_seq_id_t id)
{
fluid_list_t *tmp;
fluid_event_t evt;
unsigned int now = fluid_sequencer_get_tick(seq);
fluid_return_if_fail(seq != NULL);
fluid_event_clear(&evt);
fluid_event_unregistering(&evt);
fluid_event_set_dest(&evt, id);
fluid_event_set_time(&evt, now);
tmp = seq->clients;
while(tmp)
@ -264,12 +259,19 @@ fluid_sequencer_unregister_client(fluid_sequencer_t *seq, fluid_seq_id_t id)
if(client->id == id)
{
// client found, remove it from the list to avoid recursive call when calling callback
seq->clients = fluid_list_remove_link(seq->clients, tmp);
// call the callback (if any), to free underlying memory (e.g. seqbind structure)
if (client->callback != NULL)
{
(client->callback)(now, &evt, seq, client->data);
}
if(client->name)
{
FLUID_FREE(client->name);
}
seq->clients = fluid_list_remove_link(seq->clients, tmp);
delete1_fluid_list(tmp);
FLUID_FREE(client);
return;
@ -409,11 +411,17 @@ fluid_sequencer_send_now(fluid_sequencer_t *seq, fluid_event_t *evt)
if(dest->id == destID)
{
if(dest->callback)
if(fluid_event_get_type(evt) == FLUID_SEQ_UNREGISTERING)
{
(dest->callback)(fluid_sequencer_get_tick(seq), evt, seq, dest->data);
fluid_sequencer_unregister_client(seq, destID);
}
else
{
if(dest->callback)
{
(dest->callback)(fluid_sequencer_get_tick(seq), evt, seq, dest->data);
}
}
return;
}

12
src/midi/fluid_seqbind.c
View file

@ -75,9 +75,9 @@ delete_fluid_seqbind(fluid_seqbind_t *seqbind)
* Registers a synthesizer as a destination client of the given sequencer.
* The \a synth is registered with the name "fluidsynth".
*
* @warning Due to internal memory allocation, the user must explicitly unregister
* the client by sending a fluid_event_unregistering(). Otherwise the behaviour is
* undefined after either \p seq or \p synth is destroyed.
* @note Implementations are encouraged to explicitly unregister this client either by calling
* fluid_sequencer_unregister_client() or by sending an unregistering event to the sequencer. Before
* fluidsynth 2.1.1 this was mandatory to avoid memory leaks.
@code{.cpp}
fluid_seq_id_t seqid = fluid_sequencer_register_fluidsynth(seq, synth);
@ -117,6 +117,7 @@ fluid_sequencer_register_fluidsynth(fluid_sequencer_t *seq, fluid_synth_t *synth
FLUID_MEMSET(seqbind, 0, sizeof(*seqbind));
seqbind->client_id = -1;
seqbind->synth = synth;
seqbind->seq = seq;
@ -129,7 +130,7 @@ fluid_sequencer_register_fluidsynth(fluid_sequencer_t *seq, fluid_synth_t *synth
if(seqbind->sample_timer == NULL)
{
FLUID_LOG(FLUID_PANIC, "sequencer: Out of memory\n");
delete_fluid_seqbind(seqbind);
FLUID_FREE(seqbind);
return FLUID_FAILED;
}
}
@ -140,7 +141,8 @@ fluid_sequencer_register_fluidsynth(fluid_sequencer_t *seq, fluid_synth_t *synth
if(seqbind->client_id == FLUID_FAILED)
{
delete_fluid_seqbind(seqbind);
delete_fluid_sample_timer(seqbind->synth, seqbind->sample_timer);
FLUID_FREE(seqbind);
return FLUID_FAILED;
}

31
test/test_seq_event_queue_sort.c
View file

@ -8,8 +8,8 @@
static short order = 0;
void callback_stable_sort(unsigned int time, fluid_event_t *event, fluid_sequencer_t *seq, void *data)
{
static const enum fluid_seq_event_type expected_type_order[] =
{ FLUID_SEQ_SYSTEMRESET, FLUID_SEQ_UNREGISTERING, FLUID_SEQ_NOTEOFF, FLUID_SEQ_NOTEON };
static const int expected_type_order[] =
{ FLUID_SEQ_NOTEOFF, FLUID_SEQ_NOTEON, FLUID_SEQ_NOTEOFF, FLUID_SEQ_NOTEON, FLUID_SEQ_SYSTEMRESET, FLUID_SEQ_UNREGISTERING };
TEST_ASSERT(fluid_event_get_type(event) == expected_type_order[order++]);
}
@ -26,32 +26,38 @@ void test_order_same_tick(fluid_sequencer_t *seq, fluid_event_t *evt)
for(i = 1; i <= 2; i++)
{
fluid_event_system_reset(evt);
TEST_SUCCESS(fluid_sequencer_send_at(seq, evt, i, 1));
fluid_event_unregistering(evt);
TEST_SUCCESS(fluid_sequencer_send_at(seq, evt, i, 1));
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));
}
fluid_sequencer_process(seq, 1);
TEST_ASSERT(order == 4);
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));
order = 0;
fluid_sequencer_process(seq, 1);
TEST_ASSERT(order == 2);
fluid_sequencer_process(seq, 2);
TEST_ASSERT(order == 4);
TEST_ASSERT(order == 6);
fluid_sequencer_unregister_client(seq, seqid);
TEST_ASSERT(fluid_sequencer_count_clients(seq) == 0);
}
static unsigned int prev_time;
static unsigned int prev_time, done = FALSE;
void callback_correct_order(unsigned int time, fluid_event_t *event, fluid_sequencer_t *seq, void *data)
{
TEST_ASSERT(fluid_event_get_type(event) == FLUID_SEQ_CONTROLCHANGE);
if(done)
{
TEST_ASSERT(fluid_event_get_type(event) == FLUID_SEQ_UNREGISTERING);
}
else
{
TEST_ASSERT(fluid_event_get_type(event) == FLUID_SEQ_CONTROLCHANGE);
}
TEST_ASSERT(prev_time <= fluid_event_get_time(event) && fluid_event_get_time(event) <= prev_time + 1);
prev_time = fluid_event_get_time(event);
}
@ -91,6 +97,7 @@ void test_correct_order(fluid_sequencer_t *seq, fluid_event_t *evt)
fluid_sequencer_process(seq, i + offset);
TEST_ASSERT(prev_time == (i - 1) + offset);
done = TRUE;
fluid_sequencer_unregister_client(seq, seqid);
}

7
test/test_seq_scale.c
View file

@ -42,7 +42,8 @@ static const unsigned int expected_callback_times[] =
1560, 1560, 1680, 1680, 1800, 1800, 1920, 2700,
2820, 2820, 2940, 2940, 3060, 3060, 3180, 4275,
4395, 4395, 4515, 4515, 4635, 4635, 4755,
799, 919, 919, 1039, 1039, 1159, 1159, 1279
799, 919, 919, 1039, 1039, 1159, 1159, 1279,
15999
};
static void fake_synth_callback(unsigned int time, fluid_event_t *event, fluid_sequencer_t *seq, void *data)
@ -124,6 +125,10 @@ static void seq_callback(unsigned int time, fluid_event_t *event, fluid_sequence
sendnoteon(0, 47, 0, 480);
break;
case 5:
// this is the unregistering event, ignore
break;
default:
TEST_ASSERT(0);
}