Documentation

Co-authored-by: aider (deepseek/deepseek-reasoner) <aider@aider.chat>

docs/12-command-architecture.md

@@ -0,0 +1,65 @@
+# Command Architecture
+
+## Overview
+
+The looper uses a **lock‑free, single‑producer single‑consumer (SPSC)** command queue to communicate between the real‑time JACK audio thread and the main (non‑RT) thread.  
+There are two families of queues:
+
+- **`cmd_queue`** (RT‑safe) – used for commands that can be handled directly inside the process callback (`CMD_CYCLE`, `CMD_STOP`, `CMD_BIND_CHANNEL`, `CMD_UNBIND`).  
+  The producer is the MIDI handler (`midi_handle_events`) or the FIFO pipe reader (`pipe_thread_func`); the consumer is `process_callback`.
+
+- **`cmd_queue_main_midi`** / **`cmd_queue_main_fifo`** – used for commands that require memory allocation or JACK API calls (`CMD_ADD_CHANNEL`, `CMD_REMOVE_CHANNEL`).  
+  The producer is the MIDI handler (or FIFO reader), and the consumer is `looper_process_commands`, which runs in the main loop approximately every 50 ms.
+
+## Command Types
+
+The `command_t` struct (defined in `command.h`) contains:
+
+- `type` – one of the `cmd_type_t` enumerators.
+- `channel` – target channel index; `-1` means “current bind channel” for some commands.
+- `data` – extra parameter (e.g., bind channel number for `CMD_BIND_CHANNEL`).
+
+### RT‑safe Commands (pushed to `cmd_queue`)
+
+| Type               | Effect                                                              |
+|--------------------|---------------------------------------------------------------------|
+| `CMD_CYCLE`        | Toggle the state machine of the target channel (IDLE→RECORD→LOOPING→PAUSED→LOOPING…). |
+| `CMD_STOP`         | Force the target channel (or all channels, if `channel == -1`) to `STATE_IDLE`. |
+| `CMD_BIND_CHANNEL` | Set the global `bind_channel` index to `data`.                      |
+| `CMD_UNBIND`       | Reset `bind_channel` to 0.                                          |
+
+### Main‑thread Commands (pushed to `cmd_queue_main_midi` / `cmd_queue_main_fifo`)
+
+| Type                | Effect                                                              |
+|---------------------|---------------------------------------------------------------------|
+| `CMD_ADD_CHANNEL`   | Create a new dynamic channel (port registration).                   |
+| `CMD_REMOVE_CHANNEL`| Remove the highest‑numbered active dynamic channel (excluding channel 0). |
+
+## Command Flow
+
+1. **MIDI input** – `midi_handle_events` parses incoming note‑on events and decides which command to push.  
+   RT‑safe commands are pushed to `cmd_queue`; add/remove commands are pushed to `cmd_queue_main_midi`.
+
+2. **FIFO input** – `pipe_thread_func` reads lines from `/tmp/looper_cmd` and pushes the corresponding command.  
+   RT‑safe commands go to `cmd_queue`; add/remove go to `cmd_queue_main_fifo`.
+
+3. **Process callback** – `process_callback` is invoked by JACK for each audio cycle. It drains `cmd_queue` and applies each command via `apply_command`. This function modifies the channel state and bind index atomically.
+
+4. **Main loop** – `looper_process_commands` is called in the main loop (≈ every 50 ms). It drains `cmd_queue_main_midi` and `cmd_queue_main_fifo`, performing the necessary port registrations/unregistrations and calling `channel_add` / `channel_remove`.
+
+## Deferred Port Unregistration
+
+When a dynamic channel is removed, the RT thread first sets `active = 0`. The main thread waits until it has seen at least one full RT cycle pass (using `global_rt_cycles`) before calling `jack_port_unregister`. This prevents a race between the RT thread still holding a reference to the port buffer and the port being unregistered.
+
+## SPSC Queue Implementation
+
+The queue itself (defined in `queue.c`/`queue.h`) is a simple circular buffer with head and tail indices. It uses C11 atomic loads/stores with appropriate memory ordering (`memory_order_acquire`/`memory_order_release`) to guarantee visibility without locks. Capacity is fixed at `QUEUE_CAPACITY` (256 commands). Push/pop operations are O(1) and never block.
+
+## Thread Safety
+
+- The JACK process callback runs in an RT thread.
+- The MIDI handler runs inside the process callback (it is called from `process_callback`).
+- The FIFO reader lives in a separate POSIX thread.
+- The main thread runs the rest of the program.
+
+The two‑queue design ensures that memory‑allocating operations never happen inside the RT thread, while RT‑pertinent commands are processed with minimal latency.

src/looper.c

@@ -1,7 +1,9 @@
 // cppcheck-suppress missingIncludeSystem
 #include "looper.h"
 #include "channel.h"
+#include "command.h"
 #include "midi.h"
+#include "queue.h"
 #include <jack/jack.h>
 #include <jack/midiport.h>
 #include <math.h>
@@ -9,8 +11,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include "command.h"
-#include "queue.h"
 
 /* Global state (shared across files) */
 struct channel_t channels[MAX_CHANNELS];
@@ -30,38 +30,48 @@ static int pending_unregister_idx = -1;
 static int pending_unregister_cycle = 0;
 
 static void apply_command(command_t cmd) {
-    switch (cmd.type) {
-    case CMD_CYCLE:
-        if (cmd.channel >= 0 && cmd.channel < MAX_CHANNELS) {
-            int cur = atomic_load(&channels[cmd.channel].state);
-            int next;
-            switch (cur) {
-            case STATE_IDLE:    next = STATE_RECORD;  break;
-            case STATE_RECORD:  next = STATE_LOOPING; break;
-            case STATE_LOOPING: next = STATE_PAUSED;  break;
-            case STATE_PAUSED:  next = STATE_LOOPING; break;
-            default:            next = STATE_IDLE;    break;
-            }
-            atomic_store(&channels[cmd.channel].state, next);
-        }
+  switch (cmd.type) {
+  case CMD_CYCLE:
+    if (cmd.channel >= 0 && cmd.channel < MAX_CHANNELS) {
+      int cur = atomic_load(&channels[cmd.channel].state);
+      int next;
+      switch (cur) {
+      case STATE_IDLE:
+        next = STATE_RECORD;
         break;
-    case CMD_STOP:
-        if (cmd.channel >= 0 && cmd.channel < MAX_CHANNELS)
-            atomic_store(&channels[cmd.channel].state, STATE_IDLE);
-        else {
-            for (int i = 0; i < MAX_CHANNELS; i++)
-                atomic_store(&channels[i].state, STATE_IDLE);
-        }
+      case STATE_RECORD:
+        next = STATE_LOOPING;
         break;
-    case CMD_BIND_CHANNEL:
-        atomic_store(&bind_channel, cmd.data);
+      case STATE_LOOPING:
+        next = STATE_PAUSED;
         break;
-    case CMD_UNBIND:
-        atomic_store(&bind_channel, 0);
+      case STATE_PAUSED:
+        next = STATE_LOOPING;
         break;
-    default:
+      default:
+        next = STATE_IDLE;
         break;
+      }
+      atomic_store(&channels[cmd.channel].state, next);
     }
+    break;
+  case CMD_STOP:
+    if (cmd.channel >= 0 && cmd.channel < MAX_CHANNELS)
+      atomic_store(&channels[cmd.channel].state, STATE_IDLE);
+    else {
+      for (int i = 0; i < MAX_CHANNELS; i++)
+        atomic_store(&channels[i].state, STATE_IDLE);
+    }
+    break;
+  case CMD_BIND_CHANNEL:
+    atomic_store(&bind_channel, cmd.data);
+    break;
+  case CMD_UNBIND:
+    atomic_store(&bind_channel, 0);
+    break;
+  default:
+    break;
+  }
 }
 
 /* ----------------------------------------------------------------

src/midi.c

@@ -36,36 +36,34 @@ void midi_handle_events(void *port_buffer, jack_nframes_t nframes) {
         if (ck) {
           atomic_store(&control_key_active, 0);
           if (note < 16) {
-            command_t cmd = { .type = CMD_BIND_CHANNEL, .channel = -1, .data = note };
+            command_t cmd = {
+                .type = CMD_BIND_CHANNEL, .channel = -1, .data = note};
             queue_push(&cmd_queue, cmd);
           } else {
             switch (note) {
-            case 60:
-            {
-              command_t cmd = { .type = CMD_ADD_CHANNEL, .channel = -1, .data = 0 };
+            case 60: {
+              command_t cmd = {
+                  .type = CMD_ADD_CHANNEL, .channel = -1, .data = 0};
               queue_push(&cmd_queue_main_midi, cmd);
             } break;
-            case 61:
-            {
-              command_t cmd = { .type = CMD_REMOVE_CHANNEL, .channel = -1, .data = 0 };
+            case 61: {
+              command_t cmd = {
+                  .type = CMD_REMOVE_CHANNEL, .channel = -1, .data = 0};
               queue_push(&cmd_queue_main_midi, cmd);
             } break;
-            case 62:
-            {
+            case 62: {
               int bch = atomic_load(&bind_channel);
               if (bch >= 0 && bch < MAX_CHANNELS) {
-                command_t cmd = { .type = CMD_CYCLE, .channel = bch, .data = 0 };
+                command_t cmd = {.type = CMD_CYCLE, .channel = bch, .data = 0};
                 queue_push(&cmd_queue, cmd);
               }
             } break;
-            case 63:
-            {
-              command_t cmd = { .type = CMD_UNBIND, .channel = -1, .data = 0 };
+            case 63: {
+              command_t cmd = {.type = CMD_UNBIND, .channel = -1, .data = 0};
               queue_push(&cmd_queue, cmd);
             } break;
-            case 65:
-            {
-              command_t cmd = { .type = CMD_STOP, .channel = -1, .data = 0 };
+            case 65: {
+              command_t cmd = {.type = CMD_STOP, .channel = -1, .data = 0};
               queue_push(&cmd_queue, cmd);
             } break;
             default:
@@ -75,19 +73,17 @@ void midi_handle_events(void *port_buffer, jack_nframes_t nframes) {
         } else {
           /* direct mapping */
           switch (note) {
-          case 1:
-            {
-              command_t cmd = { .type = CMD_CYCLE, .channel = 0, .data = 0 };
-              queue_push(&cmd_queue, cmd);
-            } break;
-          case 60:
-          {
-            command_t cmd = { .type = CMD_ADD_CHANNEL, .channel = -1, .data = 0 };
+          case 1: {
+            command_t cmd = {.type = CMD_CYCLE, .channel = 0, .data = 0};
+            queue_push(&cmd_queue, cmd);
+          } break;
+          case 60: {
+            command_t cmd = {.type = CMD_ADD_CHANNEL, .channel = -1, .data = 0};
             queue_push(&cmd_queue_main_midi, cmd);
           } break;
-          case 61:
-          {
-            command_t cmd = { .type = CMD_REMOVE_CHANNEL, .channel = -1, .data = 0 };
+          case 61: {
+            command_t cmd = {
+                .type = CMD_REMOVE_CHANNEL, .channel = -1, .data = 0};
             queue_push(&cmd_queue_main_midi, cmd);
           } break;
           default:

src/pipe.c

@@ -1,14 +1,14 @@
 #include "pipe.h"
-#include "queue.h"
 #include "command.h"
+#include "queue.h"
+#include <errno.h>
+#include <fcntl.h>
+#include <pthread.h>
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <unistd.h>
-#include <pthread.h>
 #include <sys/stat.h>
-#include <fcntl.h>
-#include <errno.h>
+#include <unistd.h>
 
 #define FIFO_PATH "/tmp/looper_cmd"
 #define LINE_MAX 256
@@ -18,57 +18,57 @@ extern spsc_queue_t cmd_queue;
 extern spsc_queue_t cmd_queue_main_fifo;
 
 static void *pipe_thread_func(void *arg) {
-    (void)arg;
-    FILE *fifo = fopen(FIFO_PATH, "r");
-    if (!fifo) {
-        perror("fopen fifo");
-        return NULL;
-    }
-    char line[LINE_MAX];
-    while (fgets(line, sizeof(line), fifo)) {
-        /* strip newline */
-        size_t len = strlen(line);
-        if (len > 0 && line[len-1] == '\n')
-            line[len-1] = '\0';
-
-        if (strcmp(line, "add") == 0) {
-            command_t cmd = { .type = CMD_ADD_CHANNEL, .channel = -1, .data = 0 };
-            queue_push(&cmd_queue_main_fifo, cmd);
-        } else if (strcmp(line, "remove") == 0) {
-            command_t cmd = { .type = CMD_REMOVE_CHANNEL, .channel = -1, .data = 0 };
-            queue_push(&cmd_queue_main_fifo, cmd);
-        } else if (strncmp(line, "record ", 7) == 0) {
-            int ch = atoi(line + 7);
-            command_t cmd = { .type = CMD_CYCLE, .channel = ch, .data = 0 };
-            queue_push(&cmd_queue, cmd);
-        } else if (strcmp(line, "stop") == 0) {
-            command_t cmd = { .type = CMD_STOP, .channel = -1, .data = 0 };
-            queue_push(&cmd_queue, cmd);
-        } else if (strncmp(line, "bind ", 5) == 0) {
-            int ch = atoi(line + 5);
-            command_t cmd = { .type = CMD_BIND_CHANNEL, .channel = -1, .data = ch };
-            queue_push(&cmd_queue, cmd);
-        } else if (strcmp(line, "unbind") == 0) {
-            command_t cmd = { .type = CMD_UNBIND, .channel = -1, .data = 0 };
-            queue_push(&cmd_queue, cmd);
-        }
-        /* ignore unknown lines */
-    }
-    fclose(fifo);
+  (void)arg;
+  FILE *fifo = fopen(FIFO_PATH, "r");
+  if (!fifo) {
+    perror("fopen fifo");
     return NULL;
+  }
+  char line[LINE_MAX];
+  while (fgets(line, sizeof(line), fifo)) {
+    /* strip newline */
+    size_t len = strlen(line);
+    if (len > 0 && line[len - 1] == '\n')
+      line[len - 1] = '\0';
+
+    if (strcmp(line, "add") == 0) {
+      command_t cmd = {.type = CMD_ADD_CHANNEL, .channel = -1, .data = 0};
+      queue_push(&cmd_queue_main_fifo, cmd);
+    } else if (strcmp(line, "remove") == 0) {
+      command_t cmd = {.type = CMD_REMOVE_CHANNEL, .channel = -1, .data = 0};
+      queue_push(&cmd_queue_main_fifo, cmd);
+    } else if (strncmp(line, "record ", 7) == 0) {
+      int ch = atoi(line + 7);
+      command_t cmd = {.type = CMD_CYCLE, .channel = ch, .data = 0};
+      queue_push(&cmd_queue, cmd);
+    } else if (strcmp(line, "stop") == 0) {
+      command_t cmd = {.type = CMD_STOP, .channel = -1, .data = 0};
+      queue_push(&cmd_queue, cmd);
+    } else if (strncmp(line, "bind ", 5) == 0) {
+      int ch = atoi(line + 5);
+      command_t cmd = {.type = CMD_BIND_CHANNEL, .channel = -1, .data = ch};
+      queue_push(&cmd_queue, cmd);
+    } else if (strcmp(line, "unbind") == 0) {
+      command_t cmd = {.type = CMD_UNBIND, .channel = -1, .data = 0};
+      queue_push(&cmd_queue, cmd);
+    }
+    /* ignore unknown lines */
+  }
+  fclose(fifo);
+  return NULL;
 }
 
 int pipe_start_reader(void) {
-    /* create FIFO if it doesn't exist */
-    if (mkfifo(FIFO_PATH, 0666) != 0 && errno != EEXIST) {
-        perror("mkfifo");
-        return -1;
-    }
-    pthread_t tid;
-    if (pthread_create(&tid, NULL, pipe_thread_func, NULL) != 0) {
-        perror("pthread_create");
-        return -1;
-    }
-    pthread_detach(tid); /* we don't need to join */
-    return 0;
+  /* create FIFO if it doesn't exist */
+  if (mkfifo(FIFO_PATH, 0666) != 0 && errno != EEXIST) {
+    perror("mkfifo");
+    return -1;
+  }
+  pthread_t tid;
+  if (pthread_create(&tid, NULL, pipe_thread_func, NULL) != 0) {
+    perror("pthread_create");
+    return -1;
+  }
+  pthread_detach(tid); /* we don't need to join */
+  return 0;
 }

src/queue.c

@@ -3,28 +3,29 @@
 #include <stdbool.h>
 
 void queue_init(spsc_queue_t *q) {
-    /* nothing to allocate, just ensure head/tail start at 0 */
-    q->head = 0;
-    q->tail = 0;
+  /* nothing to allocate, just ensure head/tail start at 0 */
+  q->head = 0;
+  q->tail = 0;
 }
 
 bool queue_push(spsc_queue_t *q, command_t cmd) {
-    int h = atomic_load_explicit(&q->head, memory_order_relaxed);
-    int t = atomic_load_explicit(&q->tail, memory_order_acquire);
-    int next = (h + 1) % QUEUE_CAPACITY;
-    if (next == t)
-        return false;               /* queue full */
-    q->buffer[h] = cmd;
-    atomic_store_explicit(&q->head, next, memory_order_release);
-    return true;
+  int h = atomic_load_explicit(&q->head, memory_order_relaxed);
+  int t = atomic_load_explicit(&q->tail, memory_order_acquire);
+  int next = (h + 1) % QUEUE_CAPACITY;
+  if (next == t)
+    return false; /* queue full */
+  q->buffer[h] = cmd;
+  atomic_store_explicit(&q->head, next, memory_order_release);
+  return true;
 }
 
 bool queue_pop(spsc_queue_t *q, command_t *cmd) {
-    int t = atomic_load_explicit(&q->tail, memory_order_relaxed);
-    int h = atomic_load_explicit(&q->head, memory_order_acquire);
-    if (t == h)
-        return false;               /* queue empty */
-    *cmd = q->buffer[t];
-    atomic_store_explicit(&q->tail, (t + 1) % QUEUE_CAPACITY, memory_order_release);
-    return true;
+  int t = atomic_load_explicit(&q->tail, memory_order_relaxed);
+  int h = atomic_load_explicit(&q->head, memory_order_acquire);
+  if (t == h)
+    return false; /* queue empty */
+  *cmd = q->buffer[t];
+  atomic_store_explicit(&q->tail, (t + 1) % QUEUE_CAPACITY,
+                        memory_order_release);
+  return true;
 }