Freeform pad/knob panels + Piano audio engine (Wave C/D)#1
Conversation
Adds a polyphonic piano plugin with a producer/consumer audio engine: - SFZ/SF2 instrument loader; ships with 3 built-in synth instruments (sine-piano, electric-piano, organ) generated by generate_builtin.py - Lock-free SPSC ring buffer (float32 stereo) decouples the audio callback from synthesis work — callback is zero-allocation, just reads into outdata - Producer thread owns voices and FX chain, receives note_on/note_off/ reconfigure/set_fx_param/stop_all through a queue — no race with MIDI - 7-effect stereo FX chain, in-place on (N, 2): volume -> filter -> pitch -> chorus -> delay -> reverb -> pan; pan actually panorams now - WASAPI shared low-latency output with fallback to default host API - Pre-computed fade-out window, voice stealing before append (prefers releasing voices), underrun counter exposed via status() - Thread-safe reconfigure applied between blocks via threading.Event Tests: 7 ring_buffer + 8 audio_engine, sounddevice mocked. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Replaces the fixed padBank-A/B and knobBank-A/B slots with a freeform
panel model. Any number of pad/knob panels can be opened, each with its
own bank+preset+title+active state.
Backend (mapper.py, backend/core.py, backend/app.py):
- Mapper._panel_presets (instanceId -> preset name) and _active_panels
((type, bank) -> instanceId) replace the hard-coded routing tables.
- Dispatch resolves through lookup_pad_for_active(note) /
lookup_knob_for_active_bank(cc) — the active panel's preset determines
the action. Bank is inferred from the physical note (16-23 = A,
24-31 = B) and cached in _active_pad_bank_mem on every pad press.
- core.create_panel/update_panel/delete_panel/activate_panel mutate
settings + mapper state atomically under self._lock and broadcast
panel.created/updated/deleted/activated events.
- Exclusivity: activating a panel auto-deactivates any other panel on
the same (type, bank). update_panel also clears the old slot and
re-activates on the new bank when bank changes on an active panel.
- Legacy panel_presets.padBank-A/B and knobBank-A/B are migrated on
first boot into 4 starter freeform panels (pad A active, pad B
inactive, knob A active, knob B inactive); ui_layout is reset.
- REST: GET/POST/PATCH/DELETE /api/panels, POST /api/panels/{id}/activate.
- Audio settings endpoints: GET /api/audio/devices, PUT /api/settings/
piano_audio — applied through piano_plugin.reconfigure() without restart.
- Fixes an IndentationError on /api/knobs/catalog.
Frontend:
- New generic PadPanel.tsx / KnobPanel.tsx driven by panel.bank and
panel.preset from the store. PianoPanel.tsx wires the piano plugin UI.
- PanelHeader.tsx provides the A/B bank selector, preset dropdown, and
the engineering-style activate button (monospace LED indicator; solid
green ACTIVE / outline INACTIVE).
- PresetBar now renders hierarchical submenus: Controls > Add Pad/Knob,
Plugins, Settings, Logs. Submenu flies out to the left (menu is right-
anchored) with a 150 ms close debounce so you can navigate diagonally.
- Inactive panels stay fully editable; they just don't receive MIDI.
- dockview registers generic padPanel/knobPanel components with unique
instanceIds; layout persists via settings + localStorage fallback.
- SettingsPanel gets an Audio (Piano) section: sample rate, block size,
max polyphony, latency mode, output device, master volume.
- WebSocketProvider handles panel.* events and propagates active_panels
+ active_knob_presets on handshake.
Tests: 14 panel-model regression tests in test_core_panels.py; existing
mapper/core tests updated to the new dispatch path. Total 103/103 pass.
- README: new hero screenshot (docs/screenshots/01-default-layout.png), Web UI Tour section covering freeform panels and submenu navigation, Piano plugin in the catalog - docs/web-ui.md: replaces the old "2 банка × 4 пада" panel list with the freeform pad/knob model (bank+preset per panel, exclusivity rule, engineering activate button) and adds an Audio engine section with the producer/consumer architecture, ring buffer, WASAPI, and FX chain - docs/plugins.md: new Piano section covering the engine and FX chain - CHANGELOG: [2.2.0] entry for Wave C/D - docs/screenshots/: 4 PNGs (default layout, Controls submenu, Plugins submenu, multiple pad panels with ACTIVE/INACTIVE LED states) and the helpers used to capture/crop them Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
myfunc
left a comment
There was a problem hiding this comment.
Ревью: Freeform панели + Piano audio engine
Большой, хорошо структурированный PR. Архитектура producer/consumer + SPSC RingBuffer правильная и читается аккуратно, стерео-FX chain тоже на своём месте. Ниже — выводы по блокам.
Общее впечатление
- Audio engine: идея разделить real-time callback и mixing-producer через ring buffer — в точку. Zero-allocation в коллбэке действительно соблюдён. Но продюсер крутится на 1 ms busy-sleep и аллоцирует в pitch/reverb — это основной источник jitter'а, уже явно помечен в Follow-ups.
- Freeform panels: миграция с
bankA/bankB/knobBank-A/Bна dynamic instance IDs и правило exclusivity(type, bank)сделаны аккуратно. Есть несколько мест, где лок держится непоследовательно — оставил inline-комментарии. - Тесты: +37 тестов (ring_buffer, audio_engine, freeform panels) — хороший охват регрессий. Smoke-import
backend.appв workflow — правильная инвестиция.
Что хотелось бы закрыть до merge
audio_engine._apply_reconfigureтеряет пользовательские FX-параметры при пересозданииFXChain— snapshot/restore черезget_state()/set_param.- Deadlock-ish на shutdown —
_CmdShutdownне ACK'ает pending_CmdReconfigure→reconfigure(...)блокируется на 5 сек и отдаёт ложное "timeout". - Race на
_pitch_cache—dictменяется из MIDI/HTTP/reconfigure тредов без лока; compoundget→__setitem__. - Рассинхрон блокировки в
update_panel— часть записей вsettingsидёт внеself._lock, параллельные PATCH могут затирать друг друга.
Можно и после merge, но желательно
- Package-relative импорты в
plugins/piano/*— иначеring_buffer,fx_engineи т.п. занимают top-level namespace процесса. - Pre-allocated scratch в
PitchиReverballpass (основной kandidat на GC-jitter). - Убрать 1 ms busy-sleep в продюсере —
threading.Event, сигналимый из коллбэка. sendNoteOn(bank)— либо реально использовать bank на бэкенде, либо убрать из payload.- Мелочи:
replace('#', '#'), обращения кmapper._active_panels/_panel_presets, case-insensitive сравнение имен пресетов вdelete_preset/rename_preset.
Case-insensitive сравнение пресетов (не смог прикрепить inline к точной строке)
В backend/core.py:delete_preset и rename_preset сравнение идёт через panel_data.get("preset", "").lower() == name.lower(), хотя get_preset_index_by_name и остальной код работают case-sensitive. Получается: удалишь "Live" — панель с preset: "live" перепишется на remaining_name, хотя по строгому сравнению не должна. Стоит выровнять к одной политике (или нормализация имени в Mapper, или убрать .lower()).
Ничего критически блокирующего, но первые четыре пункта рекомендую добить перед merge — остальное можно тикетами в follow-up.
Generated by Claude Code
| sd = None # type: ignore | ||
|
|
||
| from ring_buffer import RingBuffer | ||
| from fx_engine import FXChain |
There was a problem hiding this comment.
Абсолютные импорты без namespace'а плагина.
from ring_buffer import RingBuffer и from fx_engine import FXChain работают только благодаря PluginManager.load_plugin, который временно вставляет plugin_path в sys.path. Побочные эффекты:
- Модули
ring_buffer,fx_engine,audio_engine,sfz_parser,sf2_loader,fxрегистрируются вsys.modulesкак top-level — имена сразу становятся заняты для всего процесса. Если другой плагин (или сторонняя библиотека) захочет использовать такое же имя, будет коллизия. importlib.reload(piano_plugin)перезагружает только точку входа;audio_engine/ring_bufferостаются старыми — неочевидное поведение приtoggle plugin.- После
sys.path.remove(d)вmanager.pyлюбые lazy-импорты внутри пакета сломаются (здесь повезло — всё загружается eagerly).
Лучше перевести внутренние модули плагина на package-relative импорты: from .ring_buffer import RingBuffer, from .fx_engine import FXChain, from .fx import .... Тогда plugin.toml entry будет вида plugins.piano.piano_plugin:PianoPlugin, а менеджер просто положит plugins_root в path (это у него уже есть через from plugins.base import Plugin).
Generated by Claude Code
| # FX chain keeps sample-rate-dependent delay line sizes — rebuild. | ||
| self._fx_chain = FXChain(sample_rate=new_sr) | ||
| self._mix_buf = np.zeros((new_bs, 2), dtype=np.float32) | ||
| self._fade_out_window = self._make_fade_window(int(new_sr * 0.005)) | ||
| # Reset ring buffer — old stale audio must not play at new rate. | ||
| self._ring = RingBuffer( | ||
| capacity_frames=max(new_bs * 4 + 1, 4096), | ||
| channels=2, | ||
| ) | ||
| self._voices.clear() | ||
|
|
||
| ok = self._open_stream() | ||
| if not ok: | ||
| err = self._stream_error or "stream open failed" | ||
| # Rollback stream config. | ||
| self._stream_cfg = old | ||
| self._fx_chain = FXChain(sample_rate=old.sample_rate) | ||
| self._mix_buf = np.zeros((old.block_size, 2), dtype=np.float32) | ||
| self._fade_out_window = self._make_fade_window(int(old.sample_rate * 0.005)) | ||
| self._ring = RingBuffer( | ||
| capacity_frames=max(old.block_size * 4 + 1, 4096), | ||
| channels=2, | ||
| ) | ||
| self._state = old_state | ||
| self._open_stream() | ||
| return False, err |
There was a problem hiding this comment.
Rollback reconfigure тихо теряет все пользовательские FX‑параметры.
Здесь self._fx_chain = FXChain(sample_rate=new_sr) — полностью новый объект с дефолтными mix/decay/cutoff/pan/etc. Если _open_stream() провалится и откат тоже создаст FXChain(sample_rate=old.sample_rate) — всё, что пользователь настроил через set_fx_param, молча исчезнет даже на успешном пути (когда sample_rate/block_size/latency не менялись, FX сохраняются за счёт if not stream_changed: return True).
Предложения:
- Перед пересозданием снять snapshot параметров через
self._fx_chain.get_state()и восстановить черезset_paramпослеFXChain(...). - Либо добавить
FXChain.clone_state_to(other)/ переинициализировать delay-line-буферы in-place без смены объекта — это избежит аллокаций в продюсере. - Rollback также не восстанавливает
self._stream_error(строка 310 оставит текст от последней неудачной попытки), и при неудаче отката второй вызов_open_stream()может снова упасть — в этом случае_stream_errorперезапишется, но_state = old_stateуже выполнено, а_stream_cfg = oldтоже. Т.е. возможно состояние "engine думает, что работает на старой конфигурации, но стрим закрыт" без явного сигнала наружу.
Generated by Claude Code
| def _producer_loop(self) -> None: | ||
| """Main mixing loop. Owns all voices / FX / state mutation.""" | ||
| # Producer-owned scratch buffers, resized if block_size changes. | ||
| while not self._stop_event.is_set(): | ||
| self._drain_commands(block_for_idle=True) | ||
| if self._stop_event.is_set(): | ||
| break | ||
|
|
||
| # Produce as many blocks as fit into the ring buffer. | ||
| produced_any = False | ||
| while self._ring.available_write() >= self._stream_cfg.block_size: | ||
| # Drain non-blocking so note_on can interleave with mixing. | ||
| self._drain_commands(block_for_idle=False) | ||
| if self._stop_event.is_set(): | ||
| break | ||
| self._produce_block() | ||
| produced_any = True | ||
|
|
||
| if not produced_any: | ||
| # Ring full — sleep briefly until callback consumes. | ||
| time.sleep(0.001) |
There was a problem hiding this comment.
Продюсер крутит 1 ms sleep-loop, когда кольцо полное.
_producer_loop просыпается каждые ~1 ms, даже если коллбэк забирает блок раз в ~20 ms. На block_size=2048 @ 44.1 kHz получается ~50 ложных пробуждений на каждое реальное — бесполезная работа для GIL (мешает соседним тредам: MIDI, WebSocket).
Более аккуратно: завести threading.Event "ring_drained", сигналить из _audio_callback (один атомарный set() из real-time треда допустим — sounddevice это разрешает), и в продюсере ждать его с таймаутом, эквивалентным block_size/sample_rate. Альтернатива — threading.Condition + notify() из коллбэка, но это дороже из-за acquire локов.
Бонус: при такой конструкции можно снять _drain_commands(block_for_idle=True) на входе — все команды всё равно будут обработаны между блоками.
Generated by Claude Code
| n_in = buf.shape[0] | ||
| n_out = int(n_in / ratio) | ||
| if n_out < 1: | ||
| return | ||
|
|
||
| indices = np.linspace(0, n_in - 1, n_out) | ||
| base = np.arange(n_in) | ||
|
|
||
| for ch in range(buf.shape[1]): | ||
| shifted = np.interp(indices, base, buf[:, ch]).astype(np.float32) | ||
| if len(shifted) < n_in: | ||
| buf[:len(shifted), ch] = shifted | ||
| buf[len(shifted):, ch] = 0.0 | ||
| else: | ||
| buf[:, ch] = shifted[:n_in] |
There was a problem hiding this comment.
Аллокации на каждый блок в продюсере.
np.linspace(0, n_in-1, n_out), np.arange(n_in), np.interp(...).astype(np.float32) и ещё shifted как новый ndarray — 4 аллокации на канал, т.е. 8 на блок. При block_size=256 / sample_rate=44100 это ~172 блока/с → ~1400 мелких аллокаций/с. Это основной кандидат на GC-jitter, упомянутый в Follow-ups PR. Аналогичная проблема в pan.py (mono = buf[:,0] + buf[:,1] создаёт новый array, если вызывать до пре-аллокации).
Можно держать pre-allocated scratch в Pitch.__init__(...) и ре-аллоцировать только при смене block_size (такой хук уже есть в AudioEngine._apply_reconfigure, но FXChain создаётся заново без проброса block_size). Вариант — проставлять scratch через FXChain.resize_block(n) вместе с self._mix_buf.
Также: abs(shift - 0.5) < 0.005 — безопасный байпас, но ранний return прячется за параметром, который может меняться между блоками без лока (OK под GIL, но если перехватить _shift в локальную переменную один раз — результат детерминирован, сейчас shift = self._shift выше по коду как раз так и делает — хорошо).
Generated by Claude Code
| cache_key = (origin_id, semitones) | ||
| cached = self._pitch_cache.get(cache_key) | ||
| if cached is not None: | ||
| data = cached | ||
| else: | ||
| resampled = _resample_pitch(data, semitones) | ||
| if resampled is None: | ||
| continue | ||
| data = resampled | ||
| if len(self._pitch_cache) < _PITCH_CACHE_MAX: | ||
| self._pitch_cache[cache_key] = data |
There was a problem hiding this comment.
_pitch_cache меняется без синхронизации из нескольких тредов.
_note_on вызывается из: MIDI-поток (event loop в backend/core.py:_handle_event), HTTP-хэндлер /api/piano/note, а reconfigure очищает кэш из треда, обслуживающего настройки. Паттерн if cached: ... else: compute; cache[k] = data — classic compound race: возможна:
- двойная вставка (небольшая потеря CPU);
RuntimeError: dictionary changed size during iteration, если параллельно где-то пройдёт iteration (пока такого места нет, но легко добавить);- чтение стейла при смене
sample_rate, еслиreconfigureвыполнил_pitch_cache.clear()раньше, чем текущий_note_onуспел закоммитить свой результат (получится кэш старого SR на новом инструменте).
Достаточно threading.Lock + критсекция вокруг .get / []= / .clear(). Это явно упомянуто в Follow-ups PR, но поскольку reconfigure именно через это и может сделать неприятно — стоило бы закрыть сразу.
Generated by Claude Code
| # Legacy mirror so existing fallbacks keep working | ||
| for (panel_type, bank), instance_id in self.mapper._active_panels.items(): | ||
| preset_name = self.mapper._panel_presets.get(instance_id) | ||
| if not preset_name: | ||
| continue | ||
| if panel_type == "pad": | ||
| routing_key = "bankA" if bank == "A" else "bankB" | ||
| self.mapper.set_midi_routing(routing_key, preset_name) | ||
| elif panel_type == "knob": | ||
| legacy = "knobBank-A" if bank == "A" else "knobBank-B" | ||
| self.mapper.set_knob_routing(legacy, preset_name) | ||
|
|
There was a problem hiding this comment.
Обращение к приватным атрибутам Mapper.
self.mapper._active_panels и self.mapper._panel_presets — приватные. Позже они могут быть рефакторены (например, в единую PanelRoutingState), и этот код тихо сломается. Плюс сейчас эти dict'ы читаются без каких-либо блокировок Mapper. Раз уж в mapper уже есть set_active_panel / register_panel_preset — логично добавить симметричный публичный iter_active_panels() / get_panel_preset(instance_id) и использовать только их.
Generated by Claude Code
| function sendNoteOn(note: number, velocity: number, bank: 'A' | 'B') { | ||
| fetch('/api/piano/note', { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ note, velocity, bank }), | ||
| }).catch(e => console.error('[PianoPanel] note on failed:', e)) |
There was a problem hiding this comment.
bank отправляется на сервер, но бэкенд его игнорирует.
В backend/app.py хэндлер /api/piano/note извлекает только note и velocity, а piano._note_on(note, velocity) в piano_plugin.py вообще не принимает bank. Получается мёртвое поле в payload — если UI полагается на bank switch для отображения, со стороны плагина ничего не меняется (инструмент один на весь Piano).
Либо удалить параметр (и сузить сигнатуру sendNoteOn), либо протащить bank в плагин и использовать его как преффикс (bank A/B → смещение октавы / другой инструмент). Текущее состояние вводит в заблуждение будущих мейнтейнеров.
Generated by Claude Code
| onTouchEnd={() => onNoteOff(key.note)} | ||
| > | ||
| <span style={styles.keyLabel(true, pressed)}> | ||
| {key.name.replace('#', '#')} |
There was a problem hiding this comment.
| import time as _time | ||
| base = int(_time.time() * 1000) | ||
| pad_a_id = f"padPanel-{base + 1}" | ||
| pad_b_id = f"padPanel-{base + 2}" | ||
| knob_a_id = f"knobPanel-{base + 3}" | ||
| knob_b_id = f"knobPanel-{base + 4}" |
There was a problem hiding this comment.
Генерация ID через int(time.time()*1000)+index ненадёжна.
Функция _migrate_panel_presets крутится один раз на пользователя — баг скорее теоретический, но в create_panel тоже формируется ID вида f"{panel_type}Panel-{int(_time.time() * 1000)}-{uuid.uuid4().hex[:6]}" (с uuid) — нормально. А здесь base + 1..4 может совпасть с уже созданным через create_panel ID в параллельной сессии/рестарте.
Поставить uuid.uuid4().hex[:8] тоже сюда — и консистентно с остальным кодом, и без риска коллизий.
Generated by Claude Code
| def _process_allpass_inplace( | ||
| signal: np.ndarray, buf: np.ndarray, idx: int, delay: int, g: float | ||
| ) -> int: | ||
| n = len(signal) | ||
| pos = 0 | ||
| while pos < n: | ||
| chunk = min(n - pos, delay - idx) | ||
| buf_slice = buf[idx:idx + chunk].copy() | ||
| inp_slice = signal[pos:pos + chunk].copy() |
There was a problem hiding this comment.
_process_allpass_inplace делает .copy() внутри цикла.
Два .copy() на каждый chunk × число allpass-секций × 2 канала = 8 аллокаций на блок только в ревербе. На маленьких block_size заметно скажется. Часто all-pass пишут через tmp-скаляр в цикле (медленно в Python, но тогда numba / C-ext) или через пару np.subtract(buf_slice, inp_slice * g, out=...) с одним pre-allocated buffer на инстанс.
Не блокер, но в паре с pitch.py основной контрибьютор к GC jitter под нагрузкой.
Generated by Claude Code
Applying the 4 "must close before merge" findings and 3 easy wins from the PR #1 review. Added in this branch (PR #2) per user request since PR #1 is the parent — GitHub shows cumulative diff. Must-close - `audio_engine._apply_reconfigure` preserved FX params: `FXChain.apply_state(state)` added; the engine snapshots via `get_state()` before rebuilding the chain and restores both on the success path and the rollback path. `_stream_error` is now also cleared on successful rollback so stale errors don't bleed through. - Shutdown deadlock on pending reconfigure: `_handle_command(_CmdShutdown)` now drains the remaining queue, and any `_CmdReconfigure` gets `{ok: False, error: "engine shutting down"}` + `done.set()` before the producer exits. No more 5 s ghost timeouts. - `_pitch_cache` thread safety: introduced `self._pitch_cache_lock` in `PianoPlugin.__init__`, guards every `.get` / `[]=` / `.clear()` access (MIDI thread, HTTP thread, reconfigure thread). - `update_panel` lock asymmetry: full critical section (settings read/write, mapper routing, legacy mirror, snapshot) now runs under `self._lock`. `plugin_manager.on_mode_changed`, `notify_preset_changed`, and `event_bus.publish` still fire outside the lock to avoid reentrancy into plugin code. Easy wins - Removed `.lower()` from preset-name comparisons in `delete_preset` / `rename_preset` — matches the case-sensitive semantics used everywhere else. - `_migrate_panel_presets` now generates panel IDs with `uuid.uuid4().hex[:8]` suffix (consistent with `create_panel`), eliminating the collision risk from `int(time()*1000)+index`. - Added public `Mapper.iter_active_panels()` returning a defensive copy; `core.py` no longer reaches into `self.mapper._active_panels` / `_panel_presets` directly. Not in this commit (intentional) - Package-relative imports in `plugins/piano/*`, pre-allocated scratch in `Pitch`/`Reverb`, `threading.Event`-signaled producer loop — bigger refactors, tracked as follow-ups in PR #1. - Frontend `.replace('#', '#')` concern — current code already renders bare `{key.name}`; no change needed. Tests (+5, 119 -> 124, all green) - `test_reconfigure_preserves_fx_state`: sets non-default reverb/volume params, reconfigures to 48 kHz, asserts params preserved. - `test_shutdown_acks_pending_reconfigure`: two queued reconfigures during shutdown both get `ok=False, error='engine shutting down'`, no 5 s hang. - `test_pitch_cache_thread_safe`: 4 threads interleave put/get/clear, no `RuntimeError`. - `test_update_panel_holds_lock_for_full_critical_section`: lock-acquire counter around `settings.put` asserts zero out-of-lock writes during `update_panel(bank=, preset=)`. - `test_mapper_iter_active_panels_is_copy`: mutating the returned dict does not leak into live state. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Спасибо за ревью, все 4 must-close пункта закрыл в PR #2 (stacked на этой ветке), плюс 3 easy wins. Поскольку PR #2 уже висел на этой ветке, сделал фикс-коммитом там — GitHub покажет всё в кумулятивном диффе. Коммит: 6082fcf. Must-close
Easy wins
Новые тесты (+5, 119 → 124):
Отложил в follow-ups (крупнее, в отдельный PR):
pytest 124/124, |
Summary
Large feature wave covering the UI redesign and a new Piano plugin. Three commits, each self-contained:
feat(piano)— SFZ/SF2 polyphonic synth with a lock-free producer/consumer audio engine (zero-allocation callback, SPSC ring buffer, WASAPI low-latency, 7-effect stereo FX chain).feat(ui)— Replaces fixed `padBank-A/B` / `knobBank-A/B` slots with a freeform panel model. Any number of pad/knob panels, each with its own bank+preset, exclusivity per `(type, bank)`, engineering-style activate button, hierarchical submenu in the toolbar, Audio (Piano) section in Settings.docs— README hero screenshot + Web UI Tour, updated `docs/web-ui.md` and `docs/plugins.md`, CHANGELOG entry, 4 screenshots under `docs/screenshots/`.Screenshots
Highlights
Freeform panels
Audio engine (Piano)
Settings → Audio (Piano)
Sample rate, block size, max polyphony, latency mode, output device, master volume — applied through `piano_plugin.reconfigure()` without restart.
Bug fixes surfaced en route
Test plan
Follow-ups (not in this PR)
🤖 Generated with Claude Code