4.9 KiB
4.9 KiB
Evaluation of Looper Codebase
Summary Table
| Category | Rating | Notes |
|---|---|---|
| Mocked / left‑doing | ⚠️ Moderate | No mock objects; real Carla dependency. Tests for Carla host are stubs. Integration test requires running JACK server. Script module test now passes 7/7 (empty macro bug fixed). |
| Potential segfaults | ✅ Low | exec_command validates channel bounds (ch < MAX_CHANNELS). note_actions is checked for NULL before use. strdup returns not checked but safe in practice. No out‑of‑bounds access identified. |
| Memory management | ✅ Good | note_actions strings freed via script_cleanup() called in tui_cleanup(). strdup allocations are freed on reload. All dynamically allocated audio buffers are freed. No leaks at exit. |
| Thread safety | ✅ Good | All shared scene/channel fields use C11 atomics. Logging mutex never held in audio thread. Save deactivation uses atomic active flag with two wait periods (500ms + 200ms) guaranteeing RT thread sees the change. FIFO writes are atomic (PIPE_BUF). No race conditions identified. |
| Performance | ✅ Fair | Audio path: memcpy, linear loops, no allocations – real‑time safe. Main loop sleeps 50 ms – fine. Save pauses audio for 500 ms (acceptable for a tool). Logging adds negligible mutex overhead outside RT thread. JACK callback time is deterministic. |
| Architectural soundness | ⚠️ Medium | Separation of engine and client via FIFO is clean. Orchestrator simple but effective. Three command queues still add unnecessary complexity (single queue would suffice). prev_state transition detection works. Carla integration remains tightly coupled to JACK client. Script/macro mechanism is extensible and well isolated. Logging design correctly avoids audio thread. |
Detailed Commentary
1. Mocked / Left‑doing
- Engine tests require a live JACK server – no mocking.
- Client tests for
carla_hostandpluginsuse stubs whenTESTINGis defined; real Carla library is still linked. Integration test also requires JACK. - Script tests pass 7/7 after the empty macro bug was fixed.
- No mock for MIDI – engine integration test uses actual MIDI events.
2. Potential Segfaults
- Channel bounds are now validated in
exec_command:if (ch < 0 || ch >= MAX_CHANNELS) ch = 0;. Safe. - NULL pointer dereference:
script_handle_notechecks for NULL before usingmacro.strdupfailure would setnote_actions[note]to NULL, then checked. Safe. - FIFO write errors silently ignored – no crash.
3. Memory Management
note_actions[]strings are freed on everyscript_loadwrite and on cleanup (script_cleanupcalled intui_cleanup). No leak.- Audio loop buffer (
loop_data_t) is embedded inscene_t– no heap allocation. - Save path uses
malloc/freecorrectly; freed after write. - Log file pointer is closed at exit.
4. Thread Safety
- All
scene_tfields accessed from both threads are atomic (state,prev_state,record_pos,loop_count,playback_pos). Correct. channel_t.active,channel_t.save_ringare atomic. Correct.- Save sequence: set
active=0, sleep 500 ms, copy buffer, setactive=1. The sleep guarantees the RT thread has seen the deactivation. No race. - Logging mutex acquired only outside audio thread – fine.
- FIFO writes from multiple threads are not serialized but
writeto a FIFO is atomic for writes ≤PIPE_BUF(4096 bytes). Our messages are smaller. Safe.
5. Performance
- Audio buffer processing uses simple loops with no function calls.
nframesis typically 64‑256 – fine. statetransitions checkprev_stateeach callback – cheap.- Save mechanism: 500 ms pause may cause one xrun. Acceptable for a prototype.
- Main loop sleep 50 ms ensures low CPU usage.
6. Architectural Soundness
- Good: Clear separation between engine (real‑time audio) and client (UI, plugin management). Communication via FIFO files.
- Weakness: Three command queues (
cmd_queue,cmd_queue_main_midi,cmd_queue_main_fifo) are redundant – all feedexec_command. Could consolidate. - Weakness: Carla integration tied to the engine’s JACK client. A separate Carla engine instance would be cleaner.
- Strength: Script/macro system is simple text‑based and extensible. The notes FIFO allows any external controller to inject note numbers.
- Strength: Logging non‑intrusive and never used in real‑time path.
Overall, the codebase is functional and stable. All previously identified critical issues (channel bounds, memory leak, empty macro) have been fixed. Recommendations for further improvement:
- Replace three command queues with a single queue.
- Use a double‑buffer for save to eliminate the 500 ms pause.
- Consider mock objects for engine tests to remove JACK dependency.
- Add more unit tests for edge cases.
Evaluation date: 18 May 2026