From 346c15d1c3b2cec1fec40cf1c0e5ad4499258ac2 Mon Sep 17 00:00:00 2001 From: Loic Coenen Date: Mon, 11 May 2026 22:14:33 +0000 Subject: [PATCH] fix: use persistent MIDI client and fix save_ring race condition Co-authored-by: aider (deepseek/deepseek-reasoner) --- src/channel.h | 2 +- src/looper.c | 15 ++++---- tests/integration.c | 85 ++++++++++++++++++++++++++------------------- 3 files changed, 59 insertions(+), 43 deletions(-) diff --git a/src/channel.h b/src/channel.h index a97e337..109c716 100644 --- a/src/channel.h +++ b/src/channel.h @@ -28,7 +28,7 @@ struct channel_t { jack_port_t *audio_in; jack_port_t *audio_out; - RingBuf *save_ring; + _Atomic RingBuf *save_ring; }; /* Globals declared in looper.c */ diff --git a/src/looper.c b/src/looper.c index 9b366b2..ec23be1 100644 --- a/src/looper.c +++ b/src/looper.c @@ -127,11 +127,12 @@ int process_callback(jack_nframes_t nframes, void *arg) { break; } - /* push loop output into save ring if saving */ - if (channels[c].save_ring != NULL) { + // push loop output into save ring if saving (atomic load) + RingBuf *r = atomic_load_explicit(&channels[c].save_ring, memory_order_acquire); + if (r != NULL) { if (state == STATE_LOOPING && channels[c].loop_count > 0) { float *outf = (float *)out; - ring_write(channels[c].save_ring, outf, nframes); + ring_write(r, outf, nframes); } } @@ -199,7 +200,7 @@ int looper_init(jack_client_t *client) { channels[0].loop_count = 0; channels[0].record_pos = 0; channels[0].playback_pos = 0; - channels[0].save_ring = NULL; + atomic_store_explicit(&channels[0].save_ring, NULL, memory_order_release); channels[0].audio_in = jack_port_register( client, "input", JACK_DEFAULT_AUDIO_TYPE, JackPortIsInput, 0); @@ -257,7 +258,9 @@ static void *writer_thread(void *arg) { ring_destroy(ring); free(ring); - ch->save_ring = NULL; + atomic_store_explicit(&ch->save_ring, NULL, memory_order_release); + ring_destroy(ring); + free(ring); return NULL; } @@ -326,7 +329,7 @@ void looper_process_commands(jack_client_t *client) { if (ring) { size_t sz = (size_t)channels[0].loop_count * 2; if (ring_init(ring, sz) == 0) { - channels[0].save_ring = ring; + atomic_store_explicit(&channels[0].save_ring, ring, memory_order_release); pthread_t th; pthread_create(&th, NULL, writer_thread, &channels[0]); pthread_detach(th); diff --git a/tests/integration.c b/tests/integration.c index f94853a..20f862c 100644 --- a/tests/integration.c +++ b/tests/integration.c @@ -225,50 +225,61 @@ static int test_audio_pass_through(void) { /* Helper: open a transient JACK client, send a MIDI note‑on, close */ +static jack_client_t *midi_persistent_client = NULL; +static jack_port_t *midi_persistent_port = NULL; + static int send_jack_note_on(const char *target_port, unsigned char note, unsigned char velocity) { + /* create persistent client on first call */ + if (!midi_persistent_client) { + jack_status_t st; + midi_persistent_client = jack_client_open("midi_inject_persistent", JackNoStartServer, &st); + if (!midi_persistent_client) return -1; + midi_persistent_port = jack_port_register(midi_persistent_client, "out", + JACK_DEFAULT_MIDI_TYPE, + JackPortIsOutput, 0); + if (!midi_persistent_port) { + jack_client_close(midi_persistent_client); + midi_persistent_client = NULL; + return -1; + } + char src[64]; + snprintf(src, sizeof(src), "midi_inject_persistent:out"); + if (jack_connect(midi_persistent_client, src, target_port) != 0) { + jack_client_close(midi_persistent_client); + midi_persistent_client = NULL; + midi_persistent_port = NULL; + return -1; + } + jack_set_process_callback(midi_persistent_client, midi_inject_process, NULL); + if (jack_activate(midi_persistent_client) != 0) { + jack_client_close(midi_persistent_client); + midi_persistent_client = NULL; + midi_persistent_port = NULL; + return -1; + } + } midi_inject_note = note; midi_inject_velocity = velocity; + midi_inject_pending = 1; - jack_status_t st; - midi_inject_client = jack_client_open("test_midi_inject", JackNoStartServer, &st); - if (!midi_inject_client) return -1; - - midi_inject_port = jack_port_register(midi_inject_client, "out", - JACK_DEFAULT_MIDI_TYPE, - JackPortIsOutput, 0); - if (!midi_inject_port) { - jack_client_close(midi_inject_client); - midi_inject_client = NULL; - return -1; - } - char src[64]; - snprintf(src, sizeof(src), "test_midi_inject:out"); - if (jack_connect(midi_inject_client, src, target_port) != 0) { - jack_client_close(midi_inject_client); - midi_inject_client = NULL; - midi_inject_port = NULL; - return -1; - } - midi_inject_pending = 1; /* signal before activation */ - jack_set_process_callback(midi_inject_client, midi_inject_process, NULL); - if (jack_activate(midi_inject_client) != 0) { - jack_client_close(midi_inject_client); - midi_inject_client = NULL; - midi_inject_port = NULL; - return -1; - } - /* wait for the process callback to clear the flag (event delivered) */ - for (int attempts = 0; attempts < 50; attempts++) { /* ~50 * 10ms = 500ms */ + /* wait for delivery (process callback clears the flag) */ + for (int attempts = 0; attempts < 100; attempts++) { safe_usleep(10000); if (!midi_inject_pending) break; } - jack_deactivate(midi_inject_client); - jack_client_close(midi_inject_client); - midi_inject_client = NULL; - midi_inject_port = NULL; return 0; } +/* must be called after all tests */ +static void close_persistent_midi(void) { + if (midi_persistent_client) { + jack_deactivate(midi_persistent_client); + jack_client_close(midi_persistent_client); + midi_persistent_client = NULL; + midi_persistent_port = NULL; + } +} + /* * Full loop recording test: * 1. start looper @@ -946,9 +957,9 @@ static int test_wav_load(void) { unlink("loop.wav"); return 1; } /* wait for the loop to be fully loaded and playing */ - safe_usleep(2000000); + safe_usleep(3000000); /* continue listening for the rest of the time */ - safe_usleep(4000000); /* total 6 seconds after activation */ + safe_usleep(6000000); /* total 9 seconds after activation */ jack_deactivate(client); jack_client_close(client); kill(pid, SIGTERM); waitpid(pid, NULL, 0); @@ -1142,6 +1153,8 @@ int main(void) { failures++; } + close_persistent_midi(); + if (failures > 0) { fprintf(stderr, "%d test(s) FAILED\n", failures); return 1;