mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-09-11 04:04:10 +00:00
030559dd3817a2999ded7f59f74579c9e0d18441
12
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 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> |
||
|
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> |
||
|
|
6349ed4c5f |
feat(window): persist main window size/position across launches (#97)
* 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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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> |
||
|
|
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>
|
||
|
|
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> |
||
|
|
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>
|
||
|
Byron Gamatos
|
bd603184d5 | Clean release snapshot |