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];
@@ -36,11 +36,21 @@ static void apply_command(command_t cmd) {
       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;
+      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);
     }

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 };
+          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 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
@@ -28,28 +28,28 @@ static void *pipe_thread_func(void *arg) {
   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 (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 };
+      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 };
+      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 };
+      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 };
+      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 };
+      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 };
+      command_t cmd = {.type = CMD_UNBIND, .channel = -1, .data = 0};
       queue_push(&cmd_queue, cmd);
     }
     /* ignore unknown lines */

src/queue.c

@@ -25,6 +25,7 @@ bool queue_pop(spsc_queue_t *q, command_t *cmd) {
   if (t == h)
     return false; /* queue empty */
   *cmd = q->buffer[t];
-    atomic_store_explicit(&q->tail, (t + 1) % QUEUE_CAPACITY, memory_order_release);
+  atomic_store_explicit(&q->tail, (t + 1) % QUEUE_CAPACITY,
+                        memory_order_release);
   return true;
 }