looper/evaluation.md

Code Evaluation

Summary Table

Category	Rating	Remarks
Mocked / Left Undone	✅ Everything implemented	All six command types (CYCLE, STOP, BIND_CHANNEL, UNBIND, ADD_CHANNEL, REMOVE_CHANNEL) are wired from both MIDI and FIFO pipe. No placeholder code or unimplemented paths remain.
Potential Segfaults	✅ Good	Every jack_port_get_buffer() is followed by a null check. Array bounds are respected (dynamic channel_capacity). No dynamic allocation in the RT path. The only unchecked call is in midi_handle_events – the caller already verified the buffer pointer. The deferred free of the old channel array eliminates the use‑after‑free race.
Memory Safety	✅ Good	The channel array is dynamically allocated but freed after the RT thread has completed at least one cycle after the pointer swap, preventing use‑after‑free. No leaks are present (the old pointer is freed exactly once). All internal buffers are static or stack‑allocated.
Thread Safety / Race	✅ Good	Three SPSC queues, each with a single writer and single reader, atomics correct. Shared state (state, active, control_key_active, bind_channel) uses atomics. The deferred port unregistration and deferred array free both rely on global_rt_cycles to guarantee the RT thread has seen the change before the main loop acts. No data races. prev_state is accessed only from the RT callback – safe.
Performance	✅ Good	No syscalls, locks, or allocations in the RT callback. O(1) queue operations. Linear audio processing per channel. The main loop sleeps 50 ms and drains two queues – negligible overhead.
Architectural Soundness	✅ Good	Clean separation of concerns: unified command enum, per‑source SPSC queues, RT‑safe operations in the callback, main loop handling addition/removal and deferred cleanup. Extensible – adding another input source requires only a new queue and a drain loop.

Detailed Remarks

1. Mocked / Left Undone

• Nothing remains.
  • CMD_STOP is sent from MIDI (note 65 under control key) and from FIFO ("stop").
  • CMD_ADD_CHANNEL / CMD_REMOVE_CHANNEL are triggered by MIDI notes 60/61 and FIFO commands "add"/"remove".
  • CMD_CYCLE, CMD_BIND_CHANNEL, CMD_UNBIND are fully wired.
  • The FIFO pipe reader thread is included and tested by test_fifo_pipe().

2. Potential Segfaults

• Every jack_port_get_buffer() result is null‑checked before use.
• The only unprotected call is in midi_handle_events, where the caller has already verified the buffer pointer is non‑null.
• Array indexes are guarded by idx < atomic_load(&channel_capacity).
• No use‑after‑free – the old channel array is not freed until global_rt_cycles has advanced at least once after the pointer swap, guaranteeing the RT callback has seen the new pointer.

3. Memory Safety

• The channel array is allocated with calloc and freed exactly once, after a grace period.
• No memory leaks: every calloc has a matching free (via the deferred mechanism).
• FIFO reader uses a stack‑allocated buffer (char line[256]) – safe.
• No heap operations occur in the RT callback.

4. Thread Safety / Race Conditions

• Three SPSC queues – each has a single producer and a single consumer, using correct memory_order_acquire/release.
  • cmd_queue: producer = RT callback, consumer = same RT callback (no inter‑thread race).
  • cmd_queue_main_midi: producer = RT callback, consumer = main loop.
  • cmd_queue_main_fifo: producer = FIFO thread, consumer = main loop.
• global_rt_cycles is incremented with memory_order_release at the end of every process_callback. The main loop reads it with implicit acquire. The condition current_cycle - pending_unregister_cycle >= 1 ensures the RT thread has started a new cycle after the flag was set, so port unregistration is safe.
• The deferred free uses the same pattern: pending_old_cycle is set after the atomic exchange, and the old array is freed only after global_rt_cycles has advanced by at least 1. This guarantees any RT callback that loaded the old pointer has finished.
• prev_state is a plain int but only accessed from the RT callback – safe.

5. Performance

• RT callback per frame:
  1. MIDI event scan (may push to cmd_queue or cmd_queue_main_midi).
  2. Drain cmd_queue (usually 0–2 commands).
  3. Per‑channel audio processing – linear pass‑through, recording, or playback.
  4. MIDI clock events (rare).
  5. Increment global_rt_cycles.
• No system calls, no locks, no printf in the RT path.
• Main loop sleeps 50 ms; draining two SPSC queues adds minimal overhead.

6. Architectural Soundness

• Command‑driven design – all state changes are represented as command_t structs, making the system easy to extend.
• Input source isolation – each source (MIDI, FIFO) has its own queue for commands that must be processed outside the RT thread. The RT callback only handles RT‑safe commands.
• Deferred cleanup – both port unregistration and array deallocation are delayed until the RT thread is guaranteed to have finished using the old resources. This is a correct RCU‑like pattern.
• Extensibility – adding a new input (e.g., UDP socket) requires only a new SPSC queue, a producer thread, and a drain loop in looper_process_commands().

Overall Verdict

The code is complete, race‑free, memory‑safe, and architecturally sound.

• All features are implemented and tested (all integration tests pass).
• No segfaults or memory corruption are possible under the current design.
• Thread safety is correctly handled using atomic variables and deferred cleanup.
• Performance is RT‑safe (no blocking operations in the callback).
• The architecture is clean and extensible.

Final note: The evaluation file can replace the previous version. Remove the outdated remarks about MAX_CHANNELS and the reallocation race – those issues have been fixed.