Commit Graph

41 Commits

Author SHA1 Message Date
Matthew Harris Glover
2a4396b7b7
Linux AppImage self-update on the nightly channel (#119)
Some checks failed
Ship CI / CI (push) Has been cancelled
Addon CI / addon (arm64, macos-14, mac) (push) Has been cancelled
Addon CI / addon (x64, ubuntu-22.04, linux) (push) Has been cancelled
Addon CI / addon (x64, windows-latest, win) (push) Has been cancelled
* feat(update): Linux AppImage self-update on the nightly channel

Adds a self-update engine for the Linux AppImage build. There's no
Velopack pipeline for Linux (Windows/macOS use it, Linux doesn't), so
this is a small, purpose-built GitHub-releases checker instead:

- On the nightly channel, compares the commit baked into the running
  build (dist/main/build-info.json, written at build time) against the
  published nightly's target_commitish. A mismatch means the running
  build is behind, so it's offered as an update — this sidesteps the
  fact that the AppImage's filename and app.getVersion() never change
  between nightly builds, so semver comparison can't detect a new one.
- The check returns immediately and the ~1.5GB download runs in the
  background with live progress (a new update:progress IPC event), so
  the UI never blocks or freezes waiting on it.
- The download streams straight to disk (no buffering the whole file in
  memory) and is swapped in with an atomic rename next to the running
  AppImage. The stale-generation check (a channel switch or new check
  invalidating an in-flight download) runs before that swap, and a
  failed or superseded download always cleans up its temp file.
- Applying the update spawns the (already-swapped-in) AppImage as a
  detached process and waits for a real 'spawn' confirmation before
  quitting this one, rather than assuming success — child_process.spawn
  can fail asynchronously, and quitting on an unconfirmed relaunch could
  leave the user with nothing running.
- The pure idle/staged/download decision is split into
  linux-update-decision.ts with a small truth-table test, and every
  main-process decision point (and the equivalent renderer-side
  actions, in the companion feedBack PR) is traced through a new
  update:diag IPC event that lands in the app's existing "Export
  Diagnostics" console-capture bundle — this is how the handful of real
  bugs below were actually root-caused, from real device captures
  rather than guesswork.

Also removes a forgotten, dead second implementation of the
update-channel UI (src/renderer/screen.js's
setupUpdateChannelControls() + its markup in settings.html), left over
from before this work discovered the real, visible System-tab update
UI lives in the feedBack repo. It was still wired up in the
audio_engine plugin's own settings panel and silently called
setChannel() with a stale channel value every time that panel
rendered — invisibly corrupting the real UI's state. This was the
actual root cause of several rounds of flaky, hard-to-reproduce
on-device behavior (a stuck "unsupported" warning, downloads starting
without an explicit check, etc.) chased down via the diagnostic
tracing above; once found, no other logic needed to change.

Dev tooling only, not used by CI: forces --platform linux/amd64 in the
local Docker build wrapper (the Linux target is x86_64-only end to
end — needed on Apple Silicon, where Rosetta chokes on a foreign-arch
binary inside an otherwise-native container) and adds a SLOPSMITH_REPO
override so a contributor without push access to the core repo can
bundle a fork branch for a local test build.

Verified end-to-end on a Steam Deck across many build/deploy rounds:
fresh launch, channel selection, check, background download with live
progress, atomic swap, and relaunch onto the new build — confirmed via
a real Export Diagnostics capture showing a clean, fully-accounted-for
trace with zero orphaned state transitions.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(update): fail safe when nightly release isn't pinned to a commit SHA

GitHub sets a release's target_commitish to whatever it was published
against — a 40-char SHA only if pinned, otherwise a branch name like
"main". The Linux update decision compares it SHA-vs-SHA, so a branch
name would never match the baked SHA and would re-download the ~1.5GB
AppImage on every check forever, never reaching idle. Add isCommitSha()
(pure, unit-tested) and have checkNowLinux() surface an error instead of
entering that loop.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Signed-off-by: Byron Gamatos <xasiklas@gmail.com>

---------

Signed-off-by: Byron Gamatos <xasiklas@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: Byron Gamatos <xasiklas@gmail.com>
2026-07-19 12:15:55 +02:00
OmikronApex
9e5ccdbd90
Merge pull request #117 from vo90/agent/library-path-live-refresh
Some checks are pending
Addon CI / addon (arm64, macos-14, mac) (push) Waiting to run
Addon CI / addon (x64, ubuntu-22.04, linux) (push) Waiting to run
Addon CI / addon (x64, windows-latest, win) (push) Waiting to run
Ship CI / CI (push) Waiting to run
fix(library): apply saved path without restart
2026-07-17 23:36:26 +02:00
OmikronApex
6eeec8e2a6 fix(library): keep the resolved env fallback on failure statuses
The backend has no built-in library default — _get_dlc_dir() returns None
when DLC_DIR is unset and config.json is unusable — so deleting DLC_DIR on
invalid-config/write-failed stranded the user with an empty library and
only a console warn (corrupt config.json, or a saved dlc_dir pointing at
an unplugged drive, previously still produced a working library via the
env fallback).

invalid-config and write-failed now export the resolved fallback as
DLC_DIR, restoring pre-#117 behaviour exactly and only where config.json
cannot own the path. Config-owned dynamic refresh is unchanged for the
configured/bootstrapped paths.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-17 23:16:55 +02:00
OmikronApex
16686389fd fix(audio): gate strict buffer-size verification to ASIO; review cleanups
Post-open verification previously failed closed on any buffer-size delta on
every backend. The wedge this guards against (a driver accepting a request
without changing its buffer, then blocking the next in-place request) is
ASIO behaviour; ALSA rounds requests to period constraints and CoreAudio
can clamp, and both previously worked by storing the driver-adjusted
actuals. Keep exact buffer equality for ASIO only; other backends log and
accept the adjusted size. Rate and channel-mask verification stay strict
everywhere.

Also from review: print the real `options.compatible` in the live-probe log
instead of a hard-coded 1, and document that the live-endpoint reuse in
probeDual is safe only under runDeviceLifecycleOp's message-thread
serialisation (PR #113).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-17 22:20:03 +02:00
Viktor Olausson
aa388b87d5 fix(library): validate saved library paths 2026-07-17 18:42:43 +02:00
Viktor Olausson
2dfa414e64 fix(library): validate DLC_DIR overrides 2026-07-17 16:24:39 +02:00
Viktor Olausson
9d40432700 fix(library): apply saved path without restart 2026-07-17 16:04:28 +02:00
Viktor Olausson
a558873c43 fix(audio): reuse live ASIO device capabilities 2026-07-17 15:18:33 +02:00
OmikronApex
cd93159751 fix(audio): clear duplexMode on duplex setup failure; review cleanups
Address PR #114 review:
- failClosed now stores duplexMode=false so a failed reconfigure cannot
  leave the engine reporting duplex-active on a closed device.
- Drop the dead BigInteger initializers in the verify block.
- Move the applyDuplex locate-guard ahead of the source slice in the
  lifecycle test so marker drift fails with a clear message.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-17 14:03:56 +02:00
Viktor Olausson
78a3979a3a test(audio): keep rate-match checks active in release 2026-07-17 13:22:48 +02:00
Viktor Olausson
f1c2ebc28d fix(audio): close Windows ASIO before reconfiguration 2026-07-17 12:25:12 +02:00
byrongamatos
633ace2052 fix(audio): don't let "Default" wipe the capability input selection
Review of #112: syncSelectedInputSource() treated an empty device name as
"nameless device — invalidate rather than guess" and called removeItem()
on the persisted selection. But the input dropdown's first option is
literally `<option value="">Default</option>`, so "" is the ordinary
"use the OS default" choice, not a nameless device.

init()'s auto-apply calls this on every startup, so a user sitting on
Default had their capability selection deleted at each launch — and that
selection is made in a DIFFERENT ui (the input_setup / tuner picker), so
this silently discarded a device they explicitly chose. Because
audioInputOpenHandler deliberately refuses to guess a device, a cleared
selection leaves plugins with no input at all: the same dead-guitar
symptom the PR set out to fix. Confirmed against the pre-fix build — a
sync with the Default value emits removeItem on the stored key.

"" now means "no opinion": leave the selection to the picker that owns it.

Also extract inputSourceNameKey() as the single place a named input's key
is built. Registration and selection were formatting the same template
independently and had already drifted on the nameless branch (registration
falls back to a positional key; sync emitted none). Tests pin the format,
the Default behaviour, and a round-trip through the open handler's own
parser regex.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-16 14:59:39 +02:00
byrongamatos
e29312f446 fix(renderer): roll back the mute-suppression latch when the IPC fails
CodeRabbit caught a real bug in the previous commit's fix. The latch mirrors
the NATIVE refcount, but it was flipped before the invoke resolved: a rejected
release left it reading "released" while the engine still held the
suppression, so every later release short-circuited and monitor mute stayed
suppressed for good — the same stuck-suppression bug the latch exists to
prevent, just one level up.

The latch now only stays flipped if the call actually landed, and rolls back
otherwise (guarded so a newer call can't be clobbered by a stale rejection). A
downlevel addon with no arbiter leaves the latch untouched instead of
recording a hold it never acquired.

Pins the whole contract with a vm-extracted unit test on the real screen.js
function: unpaired acquires hold at most one native suppression, cycles stay
balanced across 25 song loads, a rejected release retries, and both the
downlevel and sync-throw paths are clean. Fails 3/5 against the original
branch (the refcount leak) and 2/5 against the pre-rollback version.
2026-07-14 14:52:34 +02:00
byrongamatos
a332c35c9b fix(audio): close the PR #107 review findings
Seven fixes on top of the audio-engine TLC branch, each with the gate that
catches its regression.

Blocking:

- Monitor-mute suppression leaked its refcount. setMonitorMuteSuppressed()
  became a refcounted acquire/release, but screen.js's callers are
  deliberately unpaired: resolveChainRebuildGuard() leaves the suppression on
  when a rebuild yields an empty chain, and returns early without releasing
  while a provider route is still resolving. Harmless against the old latched
  bool, a permanent +1 each against a refcount — after a failed tone rebuild
  the count never returned to zero and monitor mute was silently dead for the
  rest of the session. The renderer now holds at most one suppression.

- Slot ids are monotonic HANDLES (nextSlotId, never reset by clear()), not
  bounded indices, so argSlotId's 4096 ceiling meant that once a session
  created its 4096th processor EVERY guarded binding — setBypass,
  setParameter, remove/moveProcessor, open/closePluginEditor — silently
  no-opped for the rest of the run. Ceiling removed (same for
  SetMultiBypass's hardcoded 4096); unknown ids are still rejected by
  SignalChain::findSlotIndex.

- clearChain / removeProcessor / moveProcessor took chainMutationMutex with a
  blocking lock_guard on the N-API thread — Electron's main thread, and on
  macOS also the JUCE message thread. LoadPreset/LoadVST hold that mutex
  across an unbounded plugin init (done->wait() has no timeout by design), so
  a slow plugin froze the whole main process, every IPC channel with it. They
  now queue on a libuv worker via queueChainMutation() and resolve a promise;
  the bridge awaits them so callers still observe the mutation applied.

Also:

- getChainState() dereferenced raw ProcessorSlot* returned by getAllSlots()
  after the lock was dropped — a concurrent clear() frees them under the
  reader. Replaced with SignalChain::getSlotSummaries(), which copies under
  the lock. getAllSlots() is gone (it had one caller).
- The device-settings migration removed the localStorage copy even when the
  file-store save failed or was unavailable, losing the user's settings.
- SetSlotState and GetParameters kept the raw Int32Value() path: IsNumber()
  is true for NaN, so setSlotState(NaN) wrote onto slot 0 — the same
  coercion class the rest of the branch fixed.
- RendererBus flushed to the LIVE writeIndex, so a disable→re-enable with no
  pull in between discarded the freshly pushed audio along with the stale
  tail. It now snapshots the flush target at disable time.
- LoadPreset's rebuild barrier is now released by a scope guard, so a throw
  between arming it and Queue() can't block editor opens forever.

Gates: new renderer-bus case (fails on the old flush), new slot-id-handle
case (fails on the old ceiling). ctest 9/9, npm test 79 pass / 0 fail,
chain-mutation storm green, addon export contract unchanged.
2026-07-14 14:29:03 +02:00
OmikronApex
ea8c6a9ccd fix(audio): address PR #107 review — close serializer gaps, editor lifetime races, dispatch failures
All 8 CodeRabbit findings verified against the code and fixed:

- ChainOps: macOS LoadVST routes its addProcessor through chainMutationMutex
  (macOS is a first-class platform; deadlock-safe — a worker holding the
  mutex never waits on the Node/main thread there). All four single-slot
  workers (LoadVST/NAM/IR/ReplaceIR) now bump chainGeneration so the
  executor's foreign-write detection sees direct loads, not just presets.
- Rebuild barrier (beginChainRebuild/endChainRebuild): LoadPreset and
  ClearChain arm it before editor teardown; OpenPluginEditor refuses to
  open while a teardown+clear/rebuild is pending (#56 window between
  closeAllPluginEditorWindows returning and the worker taking the mutex).
- EditorWindows: all slot/processor resolution in editor lambdas runs under
  a try_lock of chainMutationMutex (try_lock, never blocking — workers
  holding the mutex block-wait on the message thread). Sandbox promotion
  bumps chainGeneration. editorWindows map is now message-thread-only
  (duplicate-window check and close-erase moved into the queued lambdas).
  Null slot->processor recheck after a faulted promotion capture.
- closeAllPluginEditorWindows returns false on refused post / 15s timeout;
  ClearChain skips the clear and LoadPreset resolves {success:false}
  instead of freeing processors under a live editor.
- AddonContext: dispatchOnMessageThread reports refused-post/timeout;
  doShutdown leaves the message thread running when teardown didn't
  complete instead of unloading mid-destruction.
- RendererBus::push rejects NaN/Inf/non-positive rates and a step that
  underflows to zero; new testRejectsUnusableRates unit case.

Verified: addon builds clean, all 78 JS tests pass (storm, contracts,
executor, N-API fuzz), all 5 engine_units native tests pass.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 11:56:08 +02:00
OmikronApex
48d7e68a91 test: fix two Windows-environment-dependent failures (suite now fully green)
Both tests pre-dated this branch and failed only on Windows checkouts — the
product code was correct in both cases:

- audio-effects-executor 'preload exposes the trusted surface': asserted a
  byte-exact two-line bridge snippet with \n, which never matches a
  core.autocrlf (CRLF) working tree. Line endings are now normalized before
  the includes checks.

- config-paths 'SAFETY: ... ONLY in optInExtras': rebuilt the expected ML
  cache paths with host-native path.join, producing backslash paths that
  never equal the forward-slash simulated envs — failing the mlCaches
  equality and, worse, making the protected-root child checks vacuously
  pass on Windows (a silent coverage gap in the safety assertions). The
  test now uses the envs' resolved torchHome/hfHome fields, exactly what
  production returns, with '/' as the child separator.

npm test: 78/78 passing (1 quarantined storm gate, green under CHAIN_STORM=1).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 03:18:33 +02:00
OmikronApex
9d0963d6d5 feat(audio): getLatencyBreakdown — one owner for every latency term
Deep-read §5: latency had three unreconciled truths — getLatencyMs' static
half-capacity ring guess (42.7 ms), the verifier's input-delta-only offset,
and the renderer bus adding prime+fill+resample that no figure surfaced.

New engine API + export: per-term breakdown (deviceBufferMs, input/output
driver latency, MEASURED split-ring residency, monitor total) plus the
renderer-bus song-audio delay (measured bus fill) as its own term. On the
user's split exclusive setup the measured ring sits at ~10 ms — the legacy
figure overstated monitor latency by ~33 ms (102.7 reported vs ~70 real).

getLatencyMs is unchanged for compatibility; UI adoption of the breakdown
is renderer follow-up. Snapshots regenerated (104 exports).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 03:05:19 +02:00
OmikronApex
88f881dd1e fix(audio): refcounted monitor-mute arbiter (TLC Part II §2)
The old single monitorMuted atomic had five writers fighting
last-writer-wins: the settings checkbox, startup restore, the executor's
preload read-force-restore, releaseRoute's unconditional setMonitorMute(true)
(which clobbered the user's persisted preference), and the renderer's
song-load suppression (un-refcounted — overlapping windows un-suppressed
each other early).

Native arbiter on SourceChain: userMonitorMute (the preference — checkbox +
restore only), refcounted monitorMuteHolds (force-mute overrides), and
refcounted suppressions (setMonitorMuteSuppressed keeps its bool surface;
true=acquire, false=release, clamped at 0). Effective dry-mute =
(holds || pref) && chain empty && no suppression — the suppressed-beats-muted
precedence is unchanged. New exports: acquire/releaseMonitorMuteHold,
getMonitorMuteState (diag); snapshots regenerated.

Executor rewrite: acquires a suppression (dry-during-load, the default) or a
hold, and releases exactly what it acquired via a single-fire closure that
runs UNCONDITIONALLY (each load owns its acquisition — the stale-snapshot
race against a mid-hold user toggle is structurally gone). releaseRoute no
longer touches mute state at all. The ownership test now pins: preference
API never called, acquire/release balanced.

Renderer callers are unchanged: the checkbox writes the preference as
before, and the song-load suppression sites now compose instead of racing.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 03:01:09 +02:00
OmikronApex
7b72907503 fix(audio-effects): executor detects foreign chain writes via chainGeneration
The JS half of the phase-7a serializer (TLC Part II §1, executor-state
hazard): the executor's stageSlots map (stageId → native slotId) is built at
load time, but any direct loadPreset/clearChain from the audio_engine bundle
or rig_builder's legacy path silently invalidated it — subsequent
setStageBypass/setStageParameter/activateSegment flipped bypass/params on
the WRONG slots or returned no-target with nothing detecting the divergence.

Now: the route records the chainGeneration its load returned; every stage
operation compares it against getChainGeneration() first and reports a
stale-route no-target ('re-load the plan', with expected/current generations)
instead of mutating someone else's chain. loadChainPlan also verifies the
generation didn't move between its loadPreset and the getChainState slot
mapping, rolling back if a foreign write landed in that window. Old addons
without the counter degrade gracefully (checks no-op).

Pinned by a new executor test: fresh route flows, foreign bump → all three
stage ops refuse without touching native slots.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 02:19:42 +02:00
OmikronApex
dd40b2f227 fix(audio): renderer-bus flush flag + reconfigure reads user intent (phase 8)
Two deep-read fixes now homed in their phase-1/2 units:

RendererBus (§4): setEnabled(false) no longer writes readIndex from the
control thread — the ring's designated consumer-side writer is pull(). The
drop-on-disable is now a flushRequested atomic the consumer honors at its
next pull, closing the last SPSC-discipline hole (a concurrent pull
mid-drain could overwrite the control thread's store and replay a stale
tail after re-enable). New unit test pins flush-then-fresh-audio.

setAudioDevices (§3): the restart decision reads state.userWantsAudio
(intent, written only by start/stopAudio) instead of the racy device-state
flag that transient audioDeviceStopped() fires clear — a reconfigure landing
inside a transient-stop window no longer leaves the engine configured but
stopped ('no audio until Start/Apply is pressed again').

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 01:47:32 +02:00
OmikronApex
db337eaf29 fix(audio): serialize chain mutations + chainGeneration (phase 7a)
The single highest-value fix of the TLC pass (deep-read §1): one native
chain-mutation mutex (addon/ChainOps) held across the FULL Execute() of
every chain worker (LoadPreset/LoadVST/LoadNAM/LoadIR/ReplaceIR) and the
synchronous mutators (clearChain/removeProcessor/moveProcessor). Two
overlapping loadPreset calls can no longer interleave clear()/addProcessor()
into a merged-garbage chain — the plugin-vs-plugin fight becomes
last-writer-wins.

chainGeneration (monotonic, bumped under the mutex) is returned in loadPreset
results and exposed as getChainGeneration (new export, snapshot regenerated),
so the audio-effects executor can detect a foreign write invalidated its
stageSlots map and re-sync instead of flipping bypass/params on wrong slots —
the prerequisite for the single-chain-owner ownership track.

Also rides here: LoadPresetWorker's slot-state restore goes through
setSlotState() instead of const_cast (deep-read §9).

The phase-0 storm test flips from expected-fail to a hard gate: 50 iterations
of concurrent loadPreset now always end with exactly one caller's chain.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 01:45:01 +02:00
OmikronApex
d4e0bfc272 refactor(audio): extract AddonContext + NapiHelpers, guard raw N-API args (phase 6)
AddonContext (src/audio/addon/): engine/vstHost lifetime + snapshot rule,
the JUCE message thread with the macOS no-pump fork quarantined into ONE
file, the shutdown latch (exposed as isShuttingDown), doShutdown with a UI
teardown hook (NodeAddon points it at the editor-window nuke, #56), and the
pending-async-load registry. NodeAddon keeps using-declarations so the
binding bodies are unchanged. Also fixes SetBackingSpeed's bare `engine`
dereference — the one binding that dodged the file's own snapshot rule.

NapiHelpers (typed extractors argInt/argSlotId/argFiniteFloat/argBool/
argMidiChannel/argMidiByte) + rewrites of the unguarded bindings — the
deep-read §2 fix, done once: SetParameter/SetBypass/RemoveProcessor/
MoveProcessor/SetMultiBypass/SendMidiToSlot/SetGain plus the St-1 routing
quartet (SetPan/SetPostGain/SetBranch/SetBranchSrc). NaN slot ids no longer
coerce to slot 0; MIDI channel/program are range-checked before JUCE.

New gate: tests/napi-arg-fuzz.test.js — table-driven garbage (NaN/Inf/
negative/string/missing/object) against the real addon; chain state must be
byte-identical after the storm and a valid call must still apply.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 01:41:07 +02:00
OmikronApex
6ace5a209a refactor(audio): extract DeviceSetup + shared rate-match helpers (phase 4)
Moves probeDeviceOptionsDual, applyDuplexSetup, applySplitSetup, and
teardownSplitMode verbatim into src/audio/engine/DeviceSetup.{h,cpp}. The
component holds references to the two device managers + EngineState and owns
no lifetime; engine-owned collaborators (monitor chain, split output ring +
counters, output callback registration) are passed by reference per call.
setAudioDevices stays on the facade as the orchestrator. The public
DeviceOptions/DeviceConfig/DeviceConfigResult shapes move to the slopsmith
namespace with using-aliases on AudioEngine, so the NodeAddon spelling is
unchanged.

Lands the deep-read §7 dedupe structurally: the <=0.5 rate tolerance,
midpoint-rounding fail-closed candidate, and empty-name→first-enumerated
resolution now exist once (RateMatch.h — JUCE-free + unit-tested boundary
cases — and DeviceSetup::resolveDeviceName/rateSupportedBy) instead of three
hand-synced copies.

Full device-matrix validation (WASAPI shared/exclusive, ASIO, dual-type
split) rides the next tester build per the plan's phase-4 gate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 00:27:51 +02:00
OmikronApex
70f3316094 refactor(audio): extract RendererBus (phase 2)
Moves the WebAudio→engine bus — ring, producer-side linear resampler,
prefill gate, fill clamp, metrics — verbatim into
src/audio/engine/RendererBus.h. AudioEngine keeps thin facades
(setRendererBus/pushRendererAudio/pullRendererBus/getRendererBusMetrics) so
the NodeAddon surface is unchanged. JUCE-free: pull() takes raw channel
pointers, which is what lets tests/engine_units drive the resampler
continuity, prime/underflow/clamp, and metrics cases without a device.

The control-thread readIndex write on disable (deep-read §4) is preserved
verbatim and marked; its flush-flag fix lands as the phase-8 commit.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-13 23:46:55 +02:00
OmikronApex
eb40b87dea refactor(audio): extract EngineState with intent/state split (phase 1)
Moves the shared run-state atomics (currentSampleRate, block sizes,
duplexMode, run flags) into slopsmith::EngineState (src/audio/engine/) so
later extracted units take EngineState& and stay unit-testable without JUCE
devices. AudioEngine binds the members back by reference under their
historical names — zero call-site churn, behavior-identical.

The old audioRunning conflated user intent with device state (deep-read
§3/§6); it is now state.deviceRunning (same semantics, isAudioRunning compat
pinned) plus a new state.userWantsAudio written only by startAudio/stopAudio.
Nothing reads the intent flag yet — phase 8 flips setAudioDevices' restart
decision onto it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-13 23:42:29 +02:00
OmikronApex
eeb83cbdbc refactor(audio): extract PackedStereoRing — one SPSC ring template (phase 1)
Replaces the four hand-maintained copies of the packed-LR SPSC design
(split-mode output ring, per-InputDeviceSlot rings, stream-sink ring,
renderer-bus ring) with slopsmith::PackedStereoRing<NFrames>
(src/audio/engine/PackedStereoRing.h). The template owns the storage,
power-of-two/lock-free asserts, pack/unpack, producer publish, reset, the
w<r resync, and the lapped catch-up; per-site consumer policy (pull-vs-
consume skew, renderer prime/fill-clamp) stays verbatim at the call sites.

Pure code move per the TLC plan — no behavior change; the renderer bus's
control-thread readIndex write on disable (deep-read §4) is deliberately
preserved and gets its flush-flag fix in phase 2.

Unit-tested in tests/engine_units/packed_stereo_ring_test.cpp: threaded
tear-freedom under lapping (2M frames), drop-oldest catch-up, index-reset
resync, pull-vs-consume skew.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-13 23:38:56 +02:00
OmikronApex
3e449c0318 test(audio): quarantined chain-mutation storm test (expected-fail)
Documents TLC deep-read §1: concurrent loadPreset workers interleave
clear()/addProcessor() and merge both presets — reproduces first iteration
([storm-ir-1-0, storm-ir-2-0, storm-ir-2-1]). Quarantined behind
CHAIN_STORM=1; flips to a hard gate when ChainOps lands the serializer
(plan phase 7).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-13 23:28:34 +02:00
OmikronApex
3c8dd62ecb fix(audio): sanitize input/chain/output/backing gains at the engine setters
NaN/Inf from any JS caller (audio:setGain does no validation) previously
reached the gain atomics raw; a NaN master gain multiplies the whole device
output to NaN and poisons the peak meters (TLC deep-read §2). Clamp at the
four setters — the single choke point covering the legacy facade, the
source-indexed API, and the audio-effects executor.

Bounds 0..32 match the executor's clampGain (Phase 0.b compat pin); stream/
renderer-bus keep their historical 0..8 via the same JUCE-free helper, now
testable in the new tests/engine_units target (Phase 0.c harness).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-13 23:28:34 +02:00
OmikronApex
bbb3b58db8 test(contracts): Phase 0.a contract snapshots for audio surface
Snapshot the three public surfaces the audio-engine decomposition must not
change (docs/audio-engine-tlc.md Part IV §4): addon export table, audio-bridge
IPC channels, preload audio/audioEffects API keys. contract-check.test.js
diffs regenerated surfaces against the committed snapshots.

result-shapes.json (golden result key/type shapes) is deferred until the
engine_units harness can run the addon against a null device.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-13 23:21:21 +02:00
topkoa
030559dd38 fix(panes): only the main window may drive the tray; clamp fallback defaults
1. pane:sync was accepted from ANY renderer with the preload bridge. Pane
   windows are same-origin top-level frames, so preload's isMainFrame gate hands
   them the bridge too — which means a pane window (or any allowed pop-up) could
   send pane:sync and overwrite the tray's registry, most simply by pushing an
   empty list and emptying the menu.

   Exactly one renderer owns the pane registry. It is now accepted from that one
   only: event.sender must be the main window's webContents.

2. sanitizeWindowBounds returned sizing.defaultWidth/Height unclamped. The min
   clamp only runs when `saved` parses, so a caller whose defaults undercut its
   own minimums would get a window below the floor on precisely the paths where
   nothing is saved — first launch, or a corrupt config — and a correctly sized
   one everywhere else.

   That is the worst shape a bug can have: invisible in the common case, and
   visible only to a new user. The fallback is clamped to the floor now, with a
   test.

window-bounds: 14/14.
Signed-off-by: topkoa <topkoa@gmail.com>
2026-07-12 21:18:05 -04:00
topkoa
313d1e5d5b fix(panes): persist alwaysOnTop, flush geometry on quit, test the sizing override
Five more from review.

1. alwaysOnTop was RESTORED but never SAVED. A dead read: the only way to turn
   it on was to hand-edit the config file. There is now one snapshot() that
   decides what "remembered" means, used by the debounced save, the flush on
   close, and the flush on quit — so the three can never disagree about it again.

2. GEOMETRY WAS LOST ON QUIT. closeAllPanes() calls destroy(), and destroy()
   does not fire 'close' — so the flush wired to that event never ran on the one
   path every user takes. Combined with the 400ms debounce: move a pane, quit two
   seconds later, and its position was gone. Every pane is flushed before its
   window is destroyed.

3. The tray-icon comment claimed a 16px PNG; build:ts copies the 32px one. Also
   spelled out what the macOS consequence actually is (a colour icon rather than
   one that adapts to light/dark menu bars), rather than gesturing at it.

4. sanitizeWindowBounds' new `sizing` parameter had no tests — and it is the
   whole reason the function was touched. Without it a 380x560 pane restored
   through the MAIN window's 800x600 floor is silently inflated to three times
   the size the plugin asked for. Four tests now pin it: a small pane is not
   inflated, the min clamp uses the override, corrupt input falls back to the
   override's defaults, and — the one that protects everyone else — omitting
   `sizing` leaves the main window's behaviour byte-for-byte unchanged.

window-bounds tests: 13/13.

Signed-off-by: topkoa <topkoa@gmail.com>
2026-07-12 21:12:04 -04:00
gionnibgud
6349ed4c5f
feat(window): persist main window size/position across launches (#97)
Some checks are pending
Addon CI / addon (arm64, macos-14, mac) (push) Waiting to run
Addon CI / addon (x64, ubuntu-22.04, linux) (push) Waiting to run
Addon CI / addon (x64, windows-latest, win) (push) Waiting to run
Ship CI / CI (push) Waiting to run
* feat(window): persist main window size/position across launches

The main window always opened at a fixed 1400x900, forcing a manual
resize every session. Save the window geometry (normal bounds +
maximized flag) to the existing desktop prefs store on close, and
restore it in createWindow.

Saved bounds are validated by a pure sanitizer against the current
display layout before use, so stale state degrades safely instead of
producing an off-screen or absurd window:
- garbage/partial config -> 1400x900 centered defaults
- size clamped between the 800x600 window minimums and the largest
  display's workArea
- position kept only when the window overlaps a display by at least
  100x50 px (unplugged monitor / resolution change -> re-center);
  negative multi-monitor coordinates remain valid
- maximized sessions save getNormalBounds() and re-maximize on
  restore; fullscreen deliberately restores windowed

No new dependency; reuses get/setDesktopConfig (atomic write,
fail-soft) in soundfont-manager.ts. The store file is already in the
reset-app-settings delete-set, so a config reset also resets bounds.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: gionnibgud <gionnibgud@gmail.com>

* fix(window): don't crash shutdown if bounds persistence write fails

The close listener called setDesktopConfig synchronously with no error
handling; a disk-full or permissions failure during the write would throw
unhandled inside the close handler, risking a shutdown crash. Wrap the
write in try/catch and log a warning instead. Flagged by CodeRabbit on PR #97.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Signed-off-by: gionnibgud <gionnibgud@gmail.com>

---------

Signed-off-by: gionnibgud <gionnibgud@gmail.com>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-11 15:41:47 +02:00
OmikronApex
91c4e0037f
test: pin JUCE WASAPI exclusive device-type name (#90)
The shared player bundle (feedBack#824) detects exclusive-style output
by string-matching getCurrentDevice().outputType. The name is hardcoded
in vendored JUCE; a JUCE upgrade renaming it would silently disable the
feedpak-under-exclusive routing. Fail the build instead.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-09 22:18:04 +02:00
Jorge Fritis
f785fb9ab1
fix(renderer): keep Rig Builder's tone out of the user's manual VST chain (#73)
* fix(renderer): keep Rig Builder's tone out of the user's manual VST chain

Rig Builder's chain preloader is always on, so it loads its whole tone (amp /
pedals / racks / master pre-post / RB Final Leveler) into the SHARED engine
chain. The Audio menu's 'Save Current Chain' and auto-persist captured the LIVE
engine via getChainState()/savePreset(), baking those stages into the user's
manual chain — so a user who built their own VST chain saw it sprout a full Rig
Builder rig they never added.

Add aeIsRigBuilderStage() (path under /rig_builder/, 'RB Final Leveler', rs_gear
__rb*, or slot master_pre/post) + aeStripRigBuilderFromNativePreset(), and apply
them at save (items + native blob), the app-init restore loop, the preset-load
path (with an empty-guard), and refreshChain (display filter) so the manual
chain only ever holds the user's own processors.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(renderer): skip Rig Builder stages in the tone-switch preload paths too

Codex review: legacy polluted presets were only sanitized in
replaceChainWithPresetBlob(), but the tone-switch preloads load directly
from raw preset.items + nativePreset.chain (loadPresetItemsWithState in
IIFE 1 and the deliberately-inline copy in IIFE 2). Skip Rig Builder
stages by index in both loops — index-skips keep the items/nativeChain
alignment for the remaining pairs — and expose the detector as
window._aeIsRigBuilderStage for IIFE 2.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

* fix(renderer): load fully-polluted presets as empty instead of falling back

Codex review round 2: the never-empty guard restored the ORIGINAL
polluted blob whenever stripping emptied the chain — but a preset that
empties completely was 100% Rig Builder's tone, exactly the case the
sanitizer exists for. Load the stripped (empty) chain and warn; empty-
chain presets are a supported shape.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Jafz2001 <ignacio.fritis@mundotelecomunicaciones.cl>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
2026-07-03 15:59:34 +02:00
Byron Gamatos
e22981405c
fix(audio): stop signal-chain duplication on renderer re-evaluation (#71)
Testers on 0.3.0-alpha.1 reported the signal chain duplicating (every
VST/NAM/IR exactly twice) with blown-out gain after leaving the Audio
menu, plus VST edit windows closing and the Edit button going dead.

Root cause: the native JUCE chain lives in the Electron main process and
survives renderer reloads and screen.js re-evaluations (host re-hydration
after a backend restart), but init() unconditionally restored the
localStorage-saved chain by APPENDING — aeRestoreSavedChain never clears.
The #50 review added a clear-before-restore in the amp-sims toggle
handler only; the identical hazard at init() remained. Since the saved
chain mirrors the live chain, every init re-run produced an exact 2x
duplicate (two amp stages in series = the blown-out gain).

Fixes:
- init(): probe getChainState() first and skip ALL auto-load (default
  preset and saved-chain restore) when the engine already has a live
  chain. Also covers splitscreen pop-out windows re-running init.
- saveChainStateFromChain(): never persist a Rig-Builder-owned chain
  (identified by its _rb_unit_impulse / RB Final Leveler plumbing
  stages). Rig Builder reloads its default tone off-screen on its own
  schedule, so it is routinely the ambient live chain; snapshotting it
  made the saved chain resurrect Rig Builder's tone on restore — the
  exact processor set in the tester screenshot.
- aeRestoreSavedChain(): drop Rig Builder plumbing stages from legacy
  polluted saves and rewrite the cleaned list (self-healing).
- _aeOpenEditor(): a false return means the baked-in slot id went stale
  (chain rebuilt while the list was on screen); refresh the chain list
  instead of silently doing nothing.
- Install-once guard (hookState) on the arrangement:changed/song:ready
  reapply listeners — they stacked one pair per re-evaluation, running N
  racing clear+load sequences per song load after a re-eval.

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
2026-07-03 14:44:55 +02:00
Jorge Fritis
c8415113c8
audio: per-slot postGain for parallel-branch loudness leveling (#58)
Adds an optional per-slot output gain (ProcessorSlot.postGain, default 1.0 = no-op) applied in SignalChain::runSlot after processBlock + pan, plumbed through setPostGain / IPC / preload / loadPreset / getChainState (mirroring setPan). Gives each parallel branch its own loudness trim.

Review hardening (multi-angle + Codex): savePreset() now serializes postGain (it was read back but never written → save/load dropped it); setPostGain() rejects non-finite input (NaN would poison the buffer); new signalchain_postgain_test.cpp asserts gain scaling, NaN rejection, and serialization.

Verified locally: build:audio clean; signalchain_postgain_test 5/5 pass. (Org CI runners failed to start — infra/billing, unrelated to the change; changes are platform-neutral C++.)

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-02 12:50:47 +02:00
Byron Gamatos
92a78b4c9a
perf(audio): gate ML note-detection pipeline (default OFF, arm on demand) (#51)
* fix(audio-input): stable name-based input identity + fail-loud open + bound read-back

Replace the positional-index logicalSourceKey with a name-encoded one so a
named device survives reorder/hotplug; resolve by name and fail loud instead of
silently opening the default mic; read back and return the actually-bound device.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* perf(audio): gate the ML note-detection pipeline behind a master enable

The Basic-Pitch ONNX detector is the most expensive thing in the engine
(~30 ms inference every hop) and on the default desktop path nothing reads
it: note detection is scored by the harmonic-comb NoteVerifier, and the
always-on home tuner runs its own YIN over raw frames. Yet the pipeline ran
unconditionally from construction, pinning a core on an idle home screen.

Add a master gate so ML only runs when a consumer actually needs it:

- MlNoteDetector: std::atomic<bool> enabled{false}. pushSamples() early-
  returns on the audio thread (lock-free relaxed load, no feed) and
  runInferenceIfDue() early-returns on the inference thread (no Run()), so
  the whole pipeline is dormant until armed. setEnabled(false) clears the
  rolling window + published snapshot (clearAudioState resets hasPublished),
  so a re-arm starts cold and serves the YIN fallback until the first fresh
  inference. The inference thread stays alive but idle — toggling needs no
  thread restart. isEnabled() for symmetry; no-op stubs in the ONNX-off build.
- AudioEngine::setMlNoteDetectionEnabled(bool) fans to every source's detector
  (whole pool, so a later-activated source inherits the arm state).
- NodeAddon setNoteDetectionEnabled + audio-bridge ipc + preload, all typeof/
  try-guarded so a downlevel addon ignores it (fail-safe to current behaviour).

The renderer (note_detect) arms this true only while it will read ML notes
(native-frame detection / non-verifier fallback) and false otherwise — a
follow-up renderer change. Default OFF means the shipped verifier path and
the home tuner pay nothing for ML.

Verified: native addon builds clean (ONNX path); the standalone mlnd_test
detects the full C-major triad when armed (3/3); ml-note-detection +
multi-source JS suites pass (16/16). mlnotedetector/test.cpp arms the
detector after prepare() to match the new default.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(audio): make the ML gate reset race-free (thread-owned cold start)

The first cut cleared the rolling window/FIFO from setEnabled() on the N-API
thread while the inference thread was still alive — a data race on the buffers.
Move the reset onto the thread that owns them, and fix two follow-on issues
Codex flagged:

- fifo.reset() TOCTOU: resetting the FIFO on the inference thread can still race
  an in-flight pushSamples() that passed the resetPending gate just before it was
  set (the >=8 ms callback gap is not a guarantee). Fix: the thread-side cold
  start DRAINS the FIFO (fifo.finishedRead(getNumReady()) — advances only the
  consumer's read index, safe SPSC) instead of fifo.reset(). clearAudioState()
  (with the real reset) is kept for the prepare()/stop() paths where the thread
  is already joined. resetPending stays set through the drain so pushSamples()
  is gated off the FIFO the whole time, then is released.
- stale readiness on re-arm: setEnabled(true) exposed enabled=true immediately
  while hasPublished stayed true from the previous arm, so isReady() briefly
  served the old snapshot. Fix: drop hasPublished synchronously BEFORE storing
  enabled=true (release/acquire ordering: isReady() loads enabled before
  hasPublished, so seeing enabled=true guarantees seeing hasPublished=false).

Other gate mechanics: the enabled-gate is at the top of the inference callback
(disabled ⇒ no ingest, no inference), pushSamples() no-ops when !enabled or
resetPending, and isReady() gates on enabled so a suspended detector serves the
YIN fallback rather than a stale snapshot.

mlnotedetector/test.cpp asserts both directions: fed the chord region while
DISABLED, the detector publishes nothing and never becomes ready; armed, it
still detects the full C-major triad (3/3). Addon rebuilds clean; tsc clean;
ml-note-detection + multi-source JS suites pass (16/16).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-29 23:12:31 +02:00
Byron Gamatos
5188aab938
feat(config): real config reset/repair + migration framework (drop manual-delete) (#38)
Eliminates the fragile "delete the config folder before upgrading" tester
instruction, which was wrong-by-OS because the userData folder name was
derived inconsistently per platform (fee[dB]ack on macOS, slopsmith-desktop
on Linux/Windows).

A. Deterministic paths + migration framework
- Pin the userData name on every OS via app.setName('feedback-desktop') +
  build.extraMetadata.name; brand (productName 'fee[dB]ack') unchanged.
- One-time userData migration copies a legacy folder into the new one so
  upgraded users don't start fresh (atomic copy-then-rename, fail-soft).
  Runs before the single-instance lock / crashReporter, which would otherwise
  create userData and defeat the "new dir doesn't exist" gate.
- config-migrations.ts: versioned, ordered, idempotent, fail-soft migration
  runner stamped in CONFIG_DIR/config_version.json; logs the active CONFIG_DIR
  at startup (closes the Linux ~/.local/share/slopsmith shared-config gap).

B. In-app "Reset / repair configuration" (Settings panel)
- Granular options: reset app settings & caches, clear plugin state & cached
  Python deps, and full reset with default-OFF opt-ins for installed plugins /
  song library / ML caches.
- config-paths.ts is the single source of truth for per-OS path enumeration;
  the song library, installed plugins and ML caches are structurally confined
  to optInExtras and never wiped by the safe/full categories.
- Reset stops the backend, deletes immediate paths, includes SQLite WAL/SHM
  sidecars + the migration stamp on full reset, and defers Chromium/Crashpad
  state to next launch (consumed before any window reopens it). ML caches honor
  TORCH_HOME/HF_HOME. Empty selection is a no-op (backend left running).
- SECURITY: destructive resets require a native main-process confirmation
  dialog — the renderer bridge is reachable by plugin scripts, so a
  renderer-only confirm is not a sufficient gate.

Tests: node:test suites for path enumeration (per-OS + library/plugins
preserved), migration idempotency/fail-soft, reset delete pipeline guarantees,
userData migration, and deferred-deletion schedule/consume. `npm test` green
(adds a test script). codex review --base origin/main clean.

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-26 22:13:24 +02:00
Byron Gamatos
7f8975641e
fix(sandbox): force PolyChrome DSP plugins out-of-process (in-process WndProc DEP crash) (#34)
* fix(sandbox): force PolyChrome DSP plugins out-of-process (in-process WndProc DEP crash)

A tester crash dump (feedback.exe 0.3.0, Windows) showed an execute-DEP access
violation (0xC0000005) killing the app while McRocklin Suite.vst3 (PolyChrome
DSP) was loaded IN-PROCESS:

  Rax == Rip == McRocklin Suite.vst3 + 0x1D57050   (non-executable module data)
  caller [Rsp] = USER32.dll+0xEF5C
  WndProc(hwnd=0x51CCA, msg=0x1C WM_ACTIVATEAPP, wParam=1, lParam=0x1838)
  crash thread = the addon's background JUCE MessageManager thread (unnamed;
  start frame slopsmith_audio.node), NOT Electron's CrBrowserMain.

PolyChrome creates a top-level window during in-process init on JUCE's
*background* message thread. Its WndProc lands in non-executable memory there, so
when Windows broadcasts WM_ACTIVATEAPP the OS message pump executes it → DEP AV.
The plugin assumes a real host main UI thread (STA/main); the sandbox child
provides exactly that, so routing it out-of-process both isolates the fault and
gives the plugin the environment it needs.

Crucially this crash cannot be caught by the SignalChain in-process fault guard:
it arrives asynchronously via USER32→WndProc with NO host frame on the stack, so
guarding prepareToPlay/processBlock (or even instantiation) never sees it. Under
the current in-process-by-default policy (#24) the only fix is to not host these
plugins in-process. Graphene (same vendor) was already pre-seeded; this extends
the pre-seed to the whole PolyChrome vendor via a path-fragment match so McRocklin
Suite and any other PolyChrome product route to the sandbox too.

- Add kDefaultNeedsSandboxPathFragments (vendor/path match) + the loop in
  shouldSandbox; seed it with "PolyChrome".
- Refresh the stale kDefaultNeedsSandboxFilenames comment (it still claimed
  sandbox-by-default; #24 made the list authoritative again).
- e2e_test: add testShouldSandboxRouting() — pure shouldSandbox assertions
  (PolyChrome→sandbox, clean VST3→in-process, non-VST3→in-process).

Verified: audio addon builds clean; sandbox_e2e_test green (16/16, routing
assertions included).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* review: tighten PolyChrome match + dedupe path + harden routing test

Local high-effort review of the pre-seed fix surfaced four items; addressed:

- False-positive risk (no in-process fallback exists here — loadVstSandboxAware
  hard-fails a force-sandboxed load that can't spawn the child): narrow the
  fragment from the bare brand word "PolyChrome" to the vendor install folder
  "PolyChrome DSP", so an unrelated path (e.g. a username "polychrome") no
  longer forces the sandbox. Still matches McRocklin Suite + Graphene, which
  ship under Common Files/VST3/PolyChrome DSP/.
- Dedupe: getFullPathName() was computed twice (blocklist `canonical` +
  vendor `fullPath`); hoist one `fullPath` above the mutex block and reuse it.
- Test isolation: assert McRocklin Suite (NOT in the filename pre-seed) on both
  Windows- and POSIX-style paths so the case can only pass via the new vendor
  match; drop the redundant Graphene-in-folder line (Graphene already routes via
  the filename list).
- Test specificity + exit-code masking: add a negative proving a bare
  "polychrome" path is NOT sandboxed (guards the tightening), and surface
  routing CHECK failures on the no-args path (return 1, not the usage code 2)
  so a regression isn't masked on a manual/argless run.

Addon builds clean; sandbox_e2e_test 17/17 green (routing included).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-24 11:03:02 +02:00
Jorge Fritis
7671385ba8
Audio: song loudness normalization, in-process VSTs (perf), and stereo routing (#24)
* audio: flush denormals in the RT path + normalize the backing track

Two realtime-audio fixes (engine only — no change to amp/effect DSP):

1. Denormal flush (FTZ/DAZ). The signal path is full of IIR state (NAM, cab
   IRs, VST amp/EQ/comp chains); after each note that state decays toward zero
   and lands in the denormal range, where each float op is 10-100x slower. That
   produced sporadic CPU spikes -> buffer underruns heard as random "scratches"
   plus frame stutter (worse with larger buffers, independent of song/tone).
   Add a scoped juce::ScopedNoDenormals at the three RT entry points:
     - AudioEngine::audioDeviceIOCallbackWithContext (whole callback)
     - SignalChain::process (the plugin chain)
     - the sandbox worker's plugin processBlock in src/vst-host/main.cpp
       (VST3s run OUT-OF-PROCESS, so the host-side FTZ doesn't reach them)
   Denormals are sub -300 dBFS, so this is inaudible — CPU only, no tone change.

2. Backing-track loudness normalizer (BackingLeveler.h). Brings each song's
   backing to a consistent -12 LUFS so songs don't jump in level, applied in
   renderBackingBlockLocked BEFORE the mixer's backing-volume fader (so the
   fader still attenuates). Short-term BS.1770 K-weighted AGC (slow, no pumping)
   + a -1 dBFS brickwall limiter. RT-safe (no allocation in process()).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* audio: extend denormal flush to the split-output path + reuse chain MidiBuffer

Opt-1 low-risk RT tidy-ups (no DSP/tone change):
- ScopedNoDenormals in audioOutputCallback (the split-mode output clock that
  renders the backing track + phase-vocoder + leveler) — the primary callback's
  scope doesn't reach this separate output thread, leaving an IIR/decay path
  unprotected (a remaining source of the periodic "scratches").
- SignalChain::process reuses one juce::MidiBuffer across slots instead of
  copy-constructing it per slot per block (avoids RT-thread allocation).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* audio: per-slot pan + parallel branch routing (St-1 stereo, engine side)

Adds pan-only stereo to the signal chain so the node editor can place one amp
left and another right, pan effects, and let stereo plugins pass true L/R.

ProcessorSlot gains two fields:
  - pan    : -1..+1 constant-power, applied to that slot's output (0 = no-op)
  - branch : 0 = trunk (serial), >=1 = a parallel branch id

SignalChain::process keeps a bit-identical serial fast path when no slot has a
branch. When branches exist it runs the trunk-pre slots in place, snapshots that
as the split source, processes each branch on its own pre-allocated scratch
buffer, pans it, sums the branches into a merge bus, then runs any trunk-post
slots on the merged signal. Scratch is sized in prepare() (never on the RT
thread); falls back to serial for a non-stereo / oversized block.

The dual-mono amp output + post-amp pan is what yields "amp A left, amp B right"
without touching NAM or amp DSP. Preset schema emits pan/branch only when
non-default (mono presets unchanged); N-API gains setPan/setBranch and
getChainState/loadPreset round-trip them.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* audio: per-branch source channel (St-2) — feed a split L/R into separate branches

Extends the parallel-branch model so a stereo-out gear (e.g. a stereo delay) can
send its L output to one branch and its R to another. ProcessorSlot gains
branchSrc (0 = both, 1 = L, 2 = R); when seeding a branch from the split source,
L-only / R-only mono-izes that channel into the branch. Read from any slot in the
branch. N-API setBranchSrc + getChainState/preset round-trip it. Default 0 keeps
existing routing identical.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* audio-bridge: expose setPan/setBranch/setBranchSrc to the renderer

The engine N-API gained the stereo routing setters (setPan/setBranch/
setBranchSrc) but the main-process IPC handlers + the preload bridge didn't
forward them, so window.slopsmithDesktop.audio.setPan was undefined and the
node editor's stereo controls no-op'd. Wire all three through audio:setPan /
setBranch / setBranchSrc.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* audio: run scanned VSTs in-process + forward params + cut RT stalls

Big CPU/latency win for chains with VST plugins, plus the missing parameter
path. The out-of-process sandbox exists to crash-isolate the SCAN of unknown
plugins; a plugin only reaches a chain after it scanned cleanly, so paying the
per-block IPC cost (N serial round-trips, memcpy, poll waits) for every block
of playback was pure overhead.

- shouldSandbox(): default VST3 playback to IN-PROCESS. The runtime crash
  blocklist + launch sentinel still route a faulting plugin back through the
  sandbox on its next load, so it self-heals; only genuinely crash-prone gear
  keeps paying for isolation. Eliminates the IPC round-trips + the per-load
  subprocess spawn that caused the load-time "scratches".
- SignalChain::clear(): detach slots under a brief lock, destroy them OFF the
  lock. Sandbox teardown is slow; doing it under `lock` starved the RT
  ScopedTryLock and dropped audio blocks on every chain reload.
- AudioChannel::popBlock(): bounded busy-spin on the write index before the
  blocking poll() — a fast plugin's output lands within microseconds, so we
  skip the syscall + doorbell wakeup latency; a slow plugin falls through to the
  efficient wait (correctness + heavy-chain cost unchanged).
- SandboxedProcessor::setSandboxedParameter() + SignalChain::setParameter()
  route param changes to a sandboxed plugin over the control pipe (kSetParameter)
  — the JUCE getParameters() proxy layer isn't wired, so without this a
  sandboxed plugin's knobs/preset never reached it and it played at defaults.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* audio: PR #24 review follow-ups — POSIX fault guard + routing/spin/leveler fixes

Follow-up fixes from review of PR #24.

== POSIX in-process plugin fault guard (the main one) ==
PR #24 makes scanned VSTs run in-process by default. invokePlugin()'s catch(...)
only catches a plugin fault on Windows (where /EHa maps the SEH access violation
to a C++ exception); on macOS/Linux a plugin SIGSEGV during playback took down
the whole app, breaking the fail-soft-audio + cross-platform guarantees.

Add a POSIX fault guard in SignalChain.cpp: install chained SIGSEGV/SIGBUS/
SIGFPE/SIGILL handlers; while a guarded plugin call is live on the current
thread (thread-local, initial-exec TLS so the handler stays async-signal-safe),
siglongjmp() back into invokePlugin() and take the SAME blocklist+leak+survive
path as Windows. Faults outside a guarded call chain to the previously-installed
handler (V8/ASan/default), so real crashes and sanitizers are never masked. The
guard's armed flag is restored on EVERY exit from the guarded region — normal
return, signal-fault longjmp, and a normal C++ exception from the plugin — so a
thread is never left armed with a stale landing pad. Known limit: stack-overflow
faults aren't reliably caught (no sigaltstack on JUCE audio threads).

Comments in SandboxFactory_shared.cpp updated to match the kept in-process
default (the stale 'every VST3 sandboxes' / 'diagnostic tagging only' notes).

== Smaller correctness/quality fixes ==
- SignalChain parallel path: a branch==0 (trunk) slot interleaved inside the
  branch region was run by none of the loops -> silently dropped. Detect the
  region first and fall back to a serial chain (jassertfalse in debug) so no
  slot is lost if the node-editor contiguity invariant breaks.
- AudioChannel pop busy-spin: add a cpuRelax() (_mm_pause / arm yield) hint.
- BackingLeveler: reset AGC/limiter state on loadBackingTrack so a new song
  doesn't inherit the previous track's gain follower and briefly mis-level.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test: integration test for the in-process plugin fault guard

Drives deliberately-faulting in-process AudioProcessors through a real
SignalChain::process() and asserts the host survives, the processor is released,
and it's added to the crash blocklist (shouldSandbox() then routes it
out-of-process). Covers BOTH fault kinds: a hardware SIGSEGV (POSIX guard /
Windows SEH) and a normal C++ exception (the path that must leave the guard
disarmed). End-to-end counterpart to the standalone mechanism check — exercises
the actual invokePlugin() guard.

Lives in the POSIX-only sandbox e2e harness (already links juce_audio_processors
+ the full sandbox set). Leak detection is disabled for the target because the
guard leaks the faulting processor by design.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Jafz2001 <ignacio.fritis@mundotelecomunicaciones.cl>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: Byron Gamatos <xasiklas@gmail.com>
2026-06-19 23:08:05 +02:00
Byron Gamatos
bd603184d5 Clean release snapshot 2026-06-16 18:48:12 +02:00