mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-09-11 05:14:09 +00:00
3e449c03181dd051d0de129173102cdf9e4cf1c0
19
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 OmikronApexandClaude Fable 5
|
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> |
||
|
|
c71aa7c82f |
feat(audio): ASIO/exclusive — renderer-bus in streamer mix, loopback plumbing, cache clear (#98)
* feat(audio): renderer-bus in streamer mix + whole-app loopback plumbing Two tester-confirmed gaps under ASIO/exclusive output: 1. Streamer mix carried guitar only when the song rode the renderer bus: composeAndPushStreamMix mixed guitar + native backing, never the bus. The bus ring is single-consumer, so the consumer step is reworked from mixRendererBusInto (drain+add) to pullRendererBus (drain once into a fixed scratch); both output callbacks then share the pulled block between the device output and the stream submix (rides includeBacking — it IS song audio). 2. Previews/UI sounds bypass the per-surface feeder taps entirely and leak to the default WASAPI device (audible under ASIO, which doesn't silence that endpoint). New plumbing lets the static bundle capture ALL app audio: setDisplayMediaRequestHandler answers with this window's own frame as audio source (frame-scoped — no other apps' audio), plus audio:setPageMuted IPC + preload setPageMuted() as the local-silence fallback when suppressLocalAudioPlayback is unsupported. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(diag): engine health metrics on every [asio-diag] line Tester symptom: all audio dead after stopping a song with tones active. The snapshot showed routing state but not whether the engine was still producing. Append volatile fields (outside change-detection): in/out/ backing levels, bus fill, input overflows, output underflows, split-ring fill — outputLevel≈0 with running=true is the silent-engine signature. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(main): clear Chromium HTTP cache before first load Testers hop between portable builds sharing one userData dir; an older build's server sent no Cache-Control, so its cached /static/app.js outlived it and silently replaced the new build's renderer code (the fix14 'watcher never installed' log). One cheap clearCache() per launch makes stale-bundle states impossible even against old-server caches. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
31248617a5 |
feat(audio): renderer-audio bus — mix renderer WebAudio master into engine output (Phase 2) (#91)
* fix(audio): prefer same-backend duplex routing * fix(audio): centre mono input; limit duplex to same-endpoint devices Two issues found while testing the USB-guitar-cable path on Windows: 1. Centre a mono input. SourceChain::processBlock fell into the pass-through branch for a 1-channel input, filling only min(inputChannels, outputChannels) = 1 output channel and zeroing the rest, so a mono USB guitar cable played out of the left speaker only. A single-channel input is now broadcast across every output channel. 2. Only attempt the combined (duplex) device when input and output are the SAME physical endpoint. Two different endpoints of the same backend (USB cable in + separate speakers out) are independent hardware clocks; routing them through one duplex device was unstable across the app lifecycle (no audio until an explicit Apply, then distortion / dropouts / silent-in-song on navigation). Different endpoints now use the split path, whose ring bridges the two clocks. Same-endpoint duplex (one interface for in and out) keeps the low-latency win. Low latency for the two-device case is a follow-up that needs the device-lifecycle work (startup restore + reconfigure on navigation). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu * fix(audio): probe same-endpoint duplex the same way apply routes it Startup auto-apply (renderer init) fail-closes on probeDeviceOptionsDual's `compatible` verdict, but the probe still measured a COMBINED duplex device for any same-backend pair while setAudioDevices now opens split for different endpoints. That mismatch made the startup probe describe a config that isn't the one applied — surfacing as "no audio until I press Apply" for a USB cable + separate speakers. Gate the probe's duplex path on the same sameEndpointIntent (same type AND same device) the apply path uses, so a two-device pair is probed via the split path it will actually run on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu * fix(audio): never feed chain processors blocks larger than prepared size WASAPI shared mode can deliver oversized blocks right after a device start. The NAM core pre-allocates its conv ring/output buffers to the Reset() maxBufferSize and only asserts (release no-op) on larger blocks; one oversized block corrupts the conv ring state and garbles all subsequent audio until the next Reset() — the 'first start heavily distorted until tone reset / engine restart' bug. - NAMProcessor::processBlock: process in slices of at most the prepared block size. - SignalChain::process: slice oversized device blocks into prepared-size chunks before any slot (VST/NAM/IR) sees them. See docs/audio-distortion-first-start-investigation.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(audio): close stale-format race when processors are added mid-reconfigure addProcessor/replaceProcessor prepare the incoming processor off the audio lock on N-API worker threads. A concurrent device reconfigure's SignalChain::prepare() can't see that processor (not slotted yet), so a slot could go live prepared at a stale sample rate / block size and stay wrong until the next device restart — heard as pitch-shifted/garbled monitoring when a chain loads while the device is being (re)opened (widest window: WASAPI exclusive mode's slower open). Re-check the chain's current format under the lock at insert/swap time and re-prepare if it moved; log the transition to stderr so tester logs show when the race fired. prepare() now publishes the format under the lock so the check can't tear. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(effects): align executor plan schema with rebranded capability layer The rebrand left a three-way schema split: rig_builder sent the old 'slopsmith.audio_effects.chain_plan.v1', the renderer capability layer validated against the new 'feedBack.…' id (rejecting every plan), and this executor still expected the old one. Result: every song chain load fell back to legacy clearChain+loadPreset — a full multi-VST rebuild per currentSong poll cycle, heard as continuous distortion during playback (tester logs: 4-6 rebuilds/session, slot IDs into the 90s). Executor now uses the rebranded id and accepts the legacy one as an alias (matching the capability layer's new alias), so neither side of the handoff can break on old plugin bundles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(audio): guard input callback against double registration Tester main-process log showed two consecutive 'startAudio: duplex=0' lines: a transient audioDeviceStopped() (WASAPI exclusive opens fire one mid-start) cleared audioRunning while the input callback stayed attached, so the second startAudio() re-added it. JUCE then dispatched the input callback twice per block: DSP ran twice and each block was pushed into the split ring twice — every sample played twice (half speed, one octave down, garbled). stopAudio()'s single removeAudioCallback left the duplicate registration alive, wedging the engine (restart no longer helped) and keeping the exclusive-mode device open even after app close. Mirror the existing outputCallbackRegistered guard for the input side. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(audio): diagnostic instrumentation for tester repro builds Main-process stderr logging on every open lead, RT paths rate-limited (first-25 per anomaly + ~5s heartbeats per callback clock): - primary callback re-entrancy (duplicate registration detector) - oversized blocks on primary/output callbacks, SignalChain slicing, NAM chunking (pre-fix corruption trigger visibility) - ring fill + under/overflow counters (split-mode pacing) - device lifecycle: aboutToStart/stopped on both managers with sr/bs and callback-registration flags; startAudio guard-skip; stopAudio state - SourceChain.prepare format trace (stale-rate lead) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(audio): drop diagnostic heartbeats, keep anomaly detectors The 5s ring/format heartbeats served the distortion hunt and are noise now. Keep the cheap anomaly-only diagnostics (callback re-entrancy, oversized-block detectors, chain slicing/chunking, stale-format re-prepare, device lifecycle) — they log only on misbehavior and stay relevant for the exclusive-mode playback work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: keep investigation notes out of the PR (local working notes) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * test: pin JUCE WASAPI exclusive device-type name 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> * feat(audio): renderer-audio bus — mix renderer WebAudio master into engine output (Phase 2) SPSC packed-LR ring (64K frames) fed over IPC by the renderer, consumed by whichever output callback is live (duplex or split), mixed like a backing track before master gain. Producer-side linear resampling with cross-chunk continuity; drop-oldest on overflow. Prefill gate (~10.7 ms) and fill clamp (~85 ms → trim to prime target) added from fix12 tester spike data, which also confirmed clock stability (drift → 0, zero overflow over 8 min). Off by default — zero behavior change until the renderer enables it. Exposed as setRendererBus / pushRendererAudio (fire-and-forget IPC, ~100 msgs/s) / getRendererBusMetrics. Spike script included for tester go/no-go runs. Consumed by the feeder in feedBack#824's Phase 2 follow-up. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(audio): gate lifecycle [diag] logs behind SLOPSMITH_SANDBOX_DEBUG Review follow-up (#86): the six lifecycle diagnostics (stopAudio, audioDeviceAboutToStart/Stopped, audioOutputAboutToStart/Stopped, SourceChain::prepare) printed unconditionally while the PR body claimed they were verbose-gated. Gate them behind the existing slopsmith_vst_trace::isEnabled() runtime flag (SLOPSMITH_SANDBOX_DEBUG — already flipped by the app's debug-logging switch, so tester debug runs still capture them). The RT-path anomaly detectors (primary re-entry, oversized-block) keep their firstN/anomaly bounds unchanged, as reviewed. VSTTrace.h now defines NOMINMAX/WIN32_LEAN_AND_MEAN before windows.h so including it from engine TUs doesn't clobber std::min. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: ChrisBeWithYou <chris@rifflarr.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
2d0dd12abe |
fix(audio): input-callback double registration, stale-format race, effects schema alignment (#86)
* fix(audio): prefer same-backend duplex routing * fix(audio): centre mono input; limit duplex to same-endpoint devices Two issues found while testing the USB-guitar-cable path on Windows: 1. Centre a mono input. SourceChain::processBlock fell into the pass-through branch for a 1-channel input, filling only min(inputChannels, outputChannels) = 1 output channel and zeroing the rest, so a mono USB guitar cable played out of the left speaker only. A single-channel input is now broadcast across every output channel. 2. Only attempt the combined (duplex) device when input and output are the SAME physical endpoint. Two different endpoints of the same backend (USB cable in + separate speakers out) are independent hardware clocks; routing them through one duplex device was unstable across the app lifecycle (no audio until an explicit Apply, then distortion / dropouts / silent-in-song on navigation). Different endpoints now use the split path, whose ring bridges the two clocks. Same-endpoint duplex (one interface for in and out) keeps the low-latency win. Low latency for the two-device case is a follow-up that needs the device-lifecycle work (startup restore + reconfigure on navigation). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu * fix(audio): probe same-endpoint duplex the same way apply routes it Startup auto-apply (renderer init) fail-closes on probeDeviceOptionsDual's `compatible` verdict, but the probe still measured a COMBINED duplex device for any same-backend pair while setAudioDevices now opens split for different endpoints. That mismatch made the startup probe describe a config that isn't the one applied — surfacing as "no audio until I press Apply" for a USB cable + separate speakers. Gate the probe's duplex path on the same sameEndpointIntent (same type AND same device) the apply path uses, so a two-device pair is probed via the split path it will actually run on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu * fix(audio): never feed chain processors blocks larger than prepared size WASAPI shared mode can deliver oversized blocks right after a device start. The NAM core pre-allocates its conv ring/output buffers to the Reset() maxBufferSize and only asserts (release no-op) on larger blocks; one oversized block corrupts the conv ring state and garbles all subsequent audio until the next Reset() — the 'first start heavily distorted until tone reset / engine restart' bug. - NAMProcessor::processBlock: process in slices of at most the prepared block size. - SignalChain::process: slice oversized device blocks into prepared-size chunks before any slot (VST/NAM/IR) sees them. See docs/audio-distortion-first-start-investigation.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(audio): close stale-format race when processors are added mid-reconfigure addProcessor/replaceProcessor prepare the incoming processor off the audio lock on N-API worker threads. A concurrent device reconfigure's SignalChain::prepare() can't see that processor (not slotted yet), so a slot could go live prepared at a stale sample rate / block size and stay wrong until the next device restart — heard as pitch-shifted/garbled monitoring when a chain loads while the device is being (re)opened (widest window: WASAPI exclusive mode's slower open). Re-check the chain's current format under the lock at insert/swap time and re-prepare if it moved; log the transition to stderr so tester logs show when the race fired. prepare() now publishes the format under the lock so the check can't tear. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(effects): align executor plan schema with rebranded capability layer The rebrand left a three-way schema split: rig_builder sent the old 'slopsmith.audio_effects.chain_plan.v1', the renderer capability layer validated against the new 'feedBack.…' id (rejecting every plan), and this executor still expected the old one. Result: every song chain load fell back to legacy clearChain+loadPreset — a full multi-VST rebuild per currentSong poll cycle, heard as continuous distortion during playback (tester logs: 4-6 rebuilds/session, slot IDs into the 90s). Executor now uses the rebranded id and accepts the legacy one as an alias (matching the capability layer's new alias), so neither side of the handoff can break on old plugin bundles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(audio): guard input callback against double registration Tester main-process log showed two consecutive 'startAudio: duplex=0' lines: a transient audioDeviceStopped() (WASAPI exclusive opens fire one mid-start) cleared audioRunning while the input callback stayed attached, so the second startAudio() re-added it. JUCE then dispatched the input callback twice per block: DSP ran twice and each block was pushed into the split ring twice — every sample played twice (half speed, one octave down, garbled). stopAudio()'s single removeAudioCallback left the duplicate registration alive, wedging the engine (restart no longer helped) and keeping the exclusive-mode device open even after app close. Mirror the existing outputCallbackRegistered guard for the input side. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(audio): diagnostic instrumentation for tester repro builds Main-process stderr logging on every open lead, RT paths rate-limited (first-25 per anomaly + ~5s heartbeats per callback clock): - primary callback re-entrancy (duplicate registration detector) - oversized blocks on primary/output callbacks, SignalChain slicing, NAM chunking (pre-fix corruption trigger visibility) - ring fill + under/overflow counters (split-mode pacing) - device lifecycle: aboutToStart/stopped on both managers with sr/bs and callback-registration flags; startAudio guard-skip; stopAudio state - SourceChain.prepare format trace (stale-rate lead) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(audio): drop diagnostic heartbeats, keep anomaly detectors The 5s ring/format heartbeats served the distortion hunt and are noise now. Keep the cheap anomaly-only diagnostics (callback re-entrancy, oversized-block detectors, chain slicing/chunking, stale-format re-prepare, device lifecycle) — they log only on misbehavior and stay relevant for the exclusive-mode playback work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs: keep investigation notes out of the PR (local working notes) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * chore(audio): gate lifecycle [diag] logs behind SLOPSMITH_SANDBOX_DEBUG Review follow-up (#86): the six lifecycle diagnostics (stopAudio, audioDeviceAboutToStart/Stopped, audioOutputAboutToStart/Stopped, SourceChain::prepare) printed unconditionally while the PR body claimed they were verbose-gated. Gate them behind the existing slopsmith_vst_trace::isEnabled() runtime flag (SLOPSMITH_SANDBOX_DEBUG — already flipped by the app's debug-logging switch, so tester debug runs still capture them). The RT-path anomaly detectors (primary re-entry, oversized-block) keep their firstN/anomaly bounds unchanged, as reviewed. VSTTrace.h now defines NOMINMAX/WIN32_LEAN_AND_MEAN before windows.h so including it from engine TUs doesn't clobber std::min. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: ChrisBeWithYou <chris@rifflarr.local> Co-authored-by: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
b2be91808c |
fix(audio): accept standard base64 for IR/NAM slot state (was silently dropped) (#88)
MemoryBlock::fromBase64Encoding only parses JUCE's proprietary
"<size>.<alphabet>" format and returns false on standard RFC-4648 base64 —
which is what the Python-side plugins (rig_builder) emit for per-slot state.
That silent false meant LoadPresetWorker/SetSlotState never called setState()
for those slots, so IR stages lost their per-stage `gain` (cab loudness makeup,
amp-trim impulse compensation) on every chain load.
Add decodeStateBlob(): JUCE format first (engine-native saves unchanged), then
a standard-base64 fallback via juce::Base64 — gated to IR/NAM slots only, whose
processors take exactly the JSON these states carry ({"irPath","gain"} /
{"modelPath",...}). VST slots keep the JUCE-only decode: their plugin-emitted
blobs are metadata wrappers, not real setStateInformation() chunks.
Co-authored-by: Jafz2001 <ignacio.fritis@mundotelecomunicaciones.cl>
Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
|
||
|
|
3e3f1f868c |
fix(audio): never feed chain processors blocks larger than prepared size (#85)
* fix(audio): never feed chain processors blocks larger than prepared size WASAPI shared mode can deliver oversized blocks right after a device start. The NAM core pre-allocates its conv ring/output buffers to the Reset() maxBufferSize and only asserts (release no-op) on larger blocks; one oversized block corrupts the conv ring state and garbles all subsequent audio until the next Reset() — the 'first start heavily distorted until tone reset / engine restart' bug. - NAMProcessor::processBlock: process in slices of at most the prepared block size. - SignalChain::process: slice oversized device blocks into prepared-size chunks before any slot (VST/NAM/IR) sees them. See docs/audio-distortion-first-start-investigation.md. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(audio): prefer same-backend duplex routing * fix(audio): centre mono input; limit duplex to same-endpoint devices Two issues found while testing the USB-guitar-cable path on Windows: 1. Centre a mono input. SourceChain::processBlock fell into the pass-through branch for a 1-channel input, filling only min(inputChannels, outputChannels) = 1 output channel and zeroing the rest, so a mono USB guitar cable played out of the left speaker only. A single-channel input is now broadcast across every output channel. 2. Only attempt the combined (duplex) device when input and output are the SAME physical endpoint. Two different endpoints of the same backend (USB cable in + separate speakers out) are independent hardware clocks; routing them through one duplex device was unstable across the app lifecycle (no audio until an explicit Apply, then distortion / dropouts / silent-in-song on navigation). Different endpoints now use the split path, whose ring bridges the two clocks. Same-endpoint duplex (one interface for in and out) keeps the low-latency win. Low latency for the two-device case is a follow-up that needs the device-lifecycle work (startup restore + reconfigure on navigation). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu * fix(audio): probe same-endpoint duplex the same way apply routes it Startup auto-apply (renderer init) fail-closes on probeDeviceOptionsDual's `compatible` verdict, but the probe still measured a COMBINED duplex device for any same-backend pair while setAudioDevices now opens split for different endpoints. That mismatch made the startup probe describe a config that isn't the one applied — surfacing as "no audio until I press Apply" for a USB cable + separate speakers. Gate the probe's duplex path on the same sameEndpointIntent (same type AND same device) the apply path uses, so a two-device pair is probed via the split path it will actually run on. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JEoFeTPSnz4NpwwCG52hnu * fix(audio): close stale-format race when processors are added mid-reconfigure addProcessor/replaceProcessor prepare the incoming processor off the audio lock on N-API worker threads. A concurrent device reconfigure's SignalChain::prepare() can't see that processor (not slotted yet), so a slot could go live prepared at a stale sample rate / block size and stay wrong until the next device restart — heard as pitch-shifted/garbled monitoring when a chain loads while the device is being (re)opened (widest window: WASAPI exclusive mode's slower open). Re-check the chain's current format under the lock at insert/swap time and re-prepare if it moved; log the transition to stderr so tester logs show when the race fired. prepare() now publishes the format under the lock so the check can't tear. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: ChrisBeWithYou <chris@rifflarr.local> |
||
|
|
56e929da4e |
audio: replaceIR(slotId, path, gain) for in-place cab/IR swap (#83)
* audio: add replaceIR(slotId, path, gain) for in-place cab/IR swap Swap an existing convolution slot's IR without a full loadPreset, so the rest of the chain — the amp VST above all — is not torn down and rebuilt (that teardown is the ~1-2 s wait when changing cabs / mic position). Mirrors the existing loadIR worker but calls SignalChain::replaceProcessor(slotId, ...); optional gain updates the slot post-gain (the cab makeup). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * audio: expose replaceIR over the IPC bridge Wire the native replaceIR(slotId, path, gain) through audio-bridge (ipcMain handle) + preload, so renderers get feedBackDesktop.audio.replaceIR. Lets the rig-builder cab room swap a cab's IRs in place instead of a full loadPreset + param re-apply (that re-apply was the brief 'can't move the mic yet' lag). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * audio: replaceIR updates slot name/path so getChainState reflects the swap replaceProcessor deliberately preserves the target slot's name/path during the prepare/fault window (so a fault in prepareToPlay is blocklisted against the right plugin path). It kept them even on success, so after a successful replaceIR the slot's audio was the new IR but getChainState()/preset-save still reported the OLD IR name+path — a footgun for any consumer that persists a chain read back from getChainState(). Add optional newName/newPath to replaceProcessor, applied under the swap lock ONLY on success (empty = keep, so the sandbox-promotion caller is unchanged). ReplaceIRWorker passes "IR: <name>" + the new path, mirroring LoadIRWorker. Built (npm run build:audio) clean. 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: byrongamatos <xasiklas@gmail.com> |
||
|
|
b5dce9af4d |
fix(audio): read-ahead the backing track off the RT audio thread (#60)
* fix(audio): read-ahead the backing track off the RT audio thread The backing AudioTransportSource was set up with no read-ahead buffer and no reader thread — setSource(src, 0, nullptr, rate) — so the realtime audio callback decoded the backing file synchronously inside getNextAudioBlock on every block while a song plays. Any disk seek or codec spike (worst on compressed formats) then blew the block's realtime budget, producing underruns heard as glitches / brief mutes. Interpose a juce::TimeSliceThread with 32768 source frames (~0.68 s @ 48 kHz) of look-ahead so decode happens off the audio thread. The thread is declared before backingTransport (so it is destroyed after it — the transport's BufferingAudioSource holds a pointer to it) and started once in the ctor. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * docs(audio): document the bounded BufferingAudioSource lock residual Codex review: readBufferSection() holds callbackLock across a refill chunk decode and the RT callback takes the same lock. Accepted — the window is bounded (2048-frame chunks) and only hit mid-refill, vs. the old guaranteed synchronous decode every block; note it in the comment so nobody mistakes the transport stack for fully RT-safe. Co-Authored-By: Claude Fable 5 <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: byrongamatos <xasiklas@gmail.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> |
||
|
|
27e8f56ad8 |
fix(audio): promote in-process VST3 to sandbox on editor-open (Windows crash) (#54)
Windows-only crash fix: opening an in-process VST3 editor faults via WndProc on the background message thread. OpenPluginEditor now promotes the slot to the out-of-process sandbox (state transferred via get/setStateInformation) via the new SignalChain::replaceProcessor, and opens the editor there. Review hardening (multi-angle + Codex): state capture runs under the audio lock + SEH guard (SignalChain::captureVstStateForPromotion) so it can't race processBlock or fault the app; the transient sandbox pin is undone on promotion failure (isCrashedPlugin/removeCrashedPlugin) so a healthy plugin isn't stranded; replaceProcessor stages type/name/path for correct blocklist attribution; shared prepareForPlayback helper. CI green incl. addon (windows-latest). Editor-open crash repro on a real Windows host still recommended as follow-up. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
850d0926c7 |
fix(audio): close plugin editor windows before freeing their processors (pause UAF #56) (#57)
* fix(audio): close in-process plugin editor windows before freeing their processors (#56) A `PluginEditorWindow` owns an `AudioProcessorEditor` bound to its slot's processor, but nothing tore those windows down when the chain was freed. On pause the renderer clears/reloads the chain (`clearChain` / `loadPreset`), which destroys every slot processor — leaving any open editor pointing at freed memory. Its next timer/paint callback then jumps through a dangling pointer: the reported ACCESS_VIOLATION / DEP-execute at an unmapped address, seconds after pausing (thread stack thick with `RB Final Leveler.vst3` editor-window frames calling back into slopsmith_audio.node). Fix: destroy the in-process editor windows BEFORE the processors they reference, in all three teardown paths: - ClearChain (JS thread) — close editors, then clear(). - LoadPresetWorker::Execute (libuv worker) — close editors, then clear() before rebuilding the chain. - doShutdown — destroy editors first inside the existing message-thread lambda, before engine.reset(). editorWindows holds JUCE GUI objects, so teardown must happen on the message thread. `closeAllPluginEditorWindows()` marshals via `dispatchOnMessageThread` (post-and-wait) so the caller blocks until every editor is gone — guaranteeing editors die before their processors. Callers already on the message thread (doShutdown) use the inline `destroyAllPluginEditorWindowsOnMessageThread()` to avoid a post-and-wait-on-self deadlock. On Linux/Windows the JUCE message thread is a dedicated std::thread, so ClearChain (Node) and the worker never deadlock; on macOS dispatch runs inline and in-process editors don't exist (sandboxed). Native addon builds clean (Release). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * review: fix macOS worker-thread editor teardown; assert precondition; report post/timeout Codex [P2]: closeAllPluginEditorWindows() delegated to dispatchOnMessageThread(), which runs inline under JUCE_MAC — so LoadPresetWorker::Execute() (libuv worker) could destroy JUCE DocumentWindow/AudioProcessorEditor objects off the message thread on macOS. Now branch on the caller's actual thread: run inline only when already on the message thread (else deadlock), otherwise post via MessageManager::callAsync (drained by the JUCE thread on Linux/Windows and the Node-main libuv timer on macOS) and wait. Correct on all platforms. Copilot: report a refused post / 15s wait timeout via stderr instead of silently assuming teardown completed (the previous "guarantee" wording overstated it); add JUCE_ASSERT_MESSAGE_THREAD to the inline variant as a debug tripwire. Native addon builds clean (Release). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * review(codex): don't callAsync+wait on an unpumped macOS MessageManager Codex round 2 [P2]: my round-1 fix routed the off-message-thread teardown through MessageManager::callAsync + WaitableEvent::wait on ALL platforms. On macOS there is no separate message-thread pump (startJuceMessageThread's JUCE_MAC branch only creates the manager; there is no dispatch loop), so a callAsync+wait from LoadPresetWorker's libuv worker would stall the full 15s timeout and then proceed with the editor still alive — the very UAF this targets. Platform-split the off-thread path, matching loadVstSandboxAware()'s existing JUCE_MAC handling: - Already on the message thread → inline (doShutdown; ClearChain on macOS). - Linux/Windows off-thread → post to the dedicated JUCE message thread + wait (with refused-post / timeout reporting). - macOS off-thread → clear inline (the pre-existing macOS worker-thread limitation). editorWindows is empty on macOS in practice (in-process editors route to the sandbox child), and the editor/processor UAF this targets is Windows-specific, so no message-thread hop is needed there. Native addon builds clean (Release). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * review(codex): tear down editors from LoadPreset (main thread), not the worker Codex flagged a genuine dilemma in the previous approach: closing editor windows from LoadPresetWorker::Execute (a libuv worker) is unsafe either way on macOS — callAsync+wait stalls (no message-thread pump; the "libuv timer" the comment promises was never implemented) AND clearing inline destroys JUCE GUI objects off the message thread. Resolve it by not tearing down from the worker at all: LoadPreset() (the N-API entry, on the Node/main thread) now closes editors before queuing the AsyncWorker. That is safe on every platform — macOS: main thread IS the message thread (inline); Linux/Windows: post to the dedicated JUCE message thread and wait — and still guarantees editors die before Execute() frees the chain's processors. closeAllPluginEditorWindows() is consequently never called off a worker thread, so its macOS special-case is gone and it reduces to the uniform on-message-thread / post-and-wait form. Native addon builds clean (Release). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- 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> |
||
|
|
06c68262a9 |
Streamer mix outputs (PR1): one stream bus → a 2nd output device (#49)
* feat(audio): streamer mix outputs — one stream bus to a 2nd output device (PR1) Built-in routing so a streamer can send a separate mix (game ± their guitar tone) to a second output device for OBS/Discord capture, while still monitoring locally — no VoiceMeeter/Reaper. PR1 of the design in docs/streamer-mix-outputs.md. Architecture: this inverts the engine's proven Phase-2 multi-INPUT-device pattern to the output side. A new StreamSink = its own AudioDeviceManager + drain callback + packed drop-oldest SPSC ring (a mirror of InputDeviceSlot). The PRODUCER is the main output path (both the duplex callback and the split audioOutputCallback): it snapshots the guitar monitor mix BEFORE backing is added, then composes the stream submix (includeGuitar ? guitar : 0) + (includeBacking ? backing : 0) × gain and packs it into the sink ring. The CONSUMER (streamSinkCallback) drains the ring to the second device. Backing is rendered once on the master clock and fanned to the stream ring (never re-advances the transport / touches backingLock). Default off → zero behaviour change; the sink reopens across restarts (reopenDesiredStreamSink, mirroring reopenDesiredExtraInputs). Surface: NodeAddon setStreamOutputDevice/clearStreamOutput/setStreamBus/ setStreamBusGain/getStreamSinkLevel/isStreamOutputActive/getStreamUnderflowCount → audio:* IPC → preload → a new "Streaming & Extra Outputs" section on the Audio page (device picker, game/guitar toggles, gain, a meter mirroring what OBS/Discord receives; persisted to localStorage). v1 rejects a sample-rate-mismatched sink with a clear error (async SRC is PR3). Scope (PR1): ONE stream bus = game ± the guitar monitor mix. Per-source A/B mixes (re-amped DI vs wet as separate OBS tracks) and per-bus mute that lets a local monitor-kill (#47) NOT silence the stream are PR2 (see the doc). No virtual driver shipped — route to a spare output / virtual cable / Go-Live capture. NOT compiled or run on the author's box — this is native C++ (AudioEngine / NodeAddon) that needs a desktop build. Renderer JS verified with node --check; TS bridge/preload are additive (AudioModule is an index type so the calls typecheck). Draft pending a build + a tester pass (see the PR checklist). Refs got-feedback/feedBack-desktop#48 (tracking), #46/#47 (audio-engine family). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF * fix(audio): harden streamer-mix sink lifecycle/RT-safety (review on PR #49) Addresses the P0/P1/P2/P3 findings from the Codex + manual review. P0 — shutdown UAF: ~AudioEngine() never tore down the stream sink, and StreamSink declared `manager` before `callback`/`ring`, so the manager could be destroyed after the callback/ring it drives. stopAudio() now closes the sink (and the dtor calls stopAudio()), and `manager` is declared LAST so it destructs first even if a teardown path is missed. P1 — stopAudio() ignored the sink: the 2nd output device kept running and underflowing while "stopped". It now closes via closeStreamSinkDevice() (intent preserved → startAudio() reopens, like extra inputs). P1 — split-path producer buffers could realloc under a live callback: streamGuitarScratch/streamMixScratch are now sized to a fixed capacity (>= the ring) so a same/smaller-block device restart on either clock never reallocates them mid-use. P1 — split path read backingBuffer OUTSIDE backingLock (duplex held it): composeAndPushStreamMix in audioOutputCallback now runs inside the lock scope, so backingBuffer is read under the lock that guards its resize. P1 — live setStreamOutputDevice() broke the SPSC single-writer invariant: streamSinkAboutToStart() resets the ring while the producer might still be writing. It now clears `active` before reconfiguring so the producer stops, and only re-arms after a clean open. P1 — failed open left stale state: a shared `fail()` path now closes the device and drops the desired intent, so a deterministic failure (e.g. SR mismatch) isn't retried every start and never reports active with no device. The renderer keeps its own persisted choice. P2 — streamSinkStopped() was empty: now marks the sink inactive (and clears the meter) on an unplanned device loss, preserving intent. P2 — no ring-capacity guard on the duplex path: composeAndPushStreamMix skips (and counts) a block larger than the ring instead of wrapping. P2 — gain NaN/Inf + bridge bool coercion: native sanitizeStreamGain() (finite, clamped 0..8); the TS bridge requires real booleans (no Boolean("false")===true) and a finite gain. P3 — drop-oldest now counted via streamSink.overflowCount, exposed as getStreamOverflowCount() through the addon/bridge/preload (mirrors underflow) for drift diagnosis. Still NOT compiled here (needs a desktop build). TS typechecks clean (tsc --noEmit); renderer node --check clean. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(audio): make stream scratch fixed-capacity; document reconfig tail Follow-up to the review-fix commit, closing the two residual edge cases from the Codex re-review: - Producer scratch (streamGuitarScratch/streamMixScratch) is now sized to a FIXED capacity == the ring and never grown with the block size. Oversized blocks are already skipped by the capacity guard, so a fixed cap is sufficient and means the buffers allocate exactly once — they can never realloc under a live split-mode producer for ANY later/hotplug block size (previously a larger restart block could still realloc). - Reworded the setStreamOutputDevice() comment to stop overstating the active=false barrier: it prevents NEW producer pushes, but a block already in flight can finish one push before the (much slower) device reopen drives streamSinkAboutToStart's ring reset. Net worst case is one imperfect block on the stream bus (never the local monitor) during a manual device switch — atomic, no data race, no UAF. Documented as a known PR1 limitation. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(audio): count oversized stream blocks (capacity guard before scratch guard) Codex re-review nit: with the fixed-size scratch (== ring), an oversized block tripped the undersized-scratch guard first and was dropped without being counted. Check the ring-capacity guard FIRST so oversized duplex blocks are always counted as stream overflows; keep the scratch guard after it as cold-start/reconfig defense. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: ChrisBeWithYou <chris@rifflarr.local> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com> |
||
|
|
9facf78c98 |
Add "Disable input monitoring" (full monitor kill) for own-rig players (#47)
* audio: add "Disable input monitoring" (full monitor kill) for own-rig players The "Mute direct monitoring" control only mutes the DRY pass-through: by design it's bypassed when the signal chain has processors (SourceChain.cpp — `monitorMuted && !hasProcessors`). So with an amp sim loaded (the opt-out default), the processed signal still reaches the output and the mute is a no-op — you can't silence in-app monitoring, and an idle input through a high-gain amp sim is a constant distorted buzz. Add an additive, default-OFF "monitor kill" that silences the guitar bus unconditionally (dry AND processed), independent of the dry-mute and not subject to the song-load suppression guard. It runs after the chain so the pitch detector / metering still see real signal, and before the backing- track mix so playback is unaffected. Wired end to end: SourceChain (flag + processBlock gate) -> AudioEngine facade -> NodeAddon setMonitorKill (IsBoolean-guarded) -> audio:setMonitorKill -> preload setMonitorKill -> Audio settings "Disable input monitoring" checkbox (persisted/restored like monitorMute). Default off means existing amp-sim monitoring is byte-for-byte unchanged; fail-soft at every layer (IsBoolean guard / typeof guard / optional call) so a downlevel addon or renderer is a clean no-op. Addresses got-feedback/feedBack-desktop#46 (the monitor-kill half). The amp-sim/NAM opt-in onboarding remains a follow-up tracked there. NOTE: not compiled/run on the author's box — the native addon needs a desktop build. Logic mirrors the existing setMonitorMute path; verified by inspection + `node --check` on the renderer. Needs a build + tester check. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF * audio: fix monitor-kill persistence + make it global; clarify mute copy Addresses review findings on the "Disable input monitoring" PR. P1 (blocker): monitorKill never persisted across restart. Both normalizeDeviceSettings whitelists (renderer screen.js + main audio-bridge.ts) and the AudioDeviceSettings type only carried monitorMute, so the saved flag was stripped on every load before the restore block could read it. Carry monitorKill through both normalizers and the TS interface, mirroring monitorMute. P2: the kill is a global "play through my own rig" preference but AudioEngine::setMonitorKill only touched source0(), so additional active sources (multi-input) stayed audible while the UI claimed it silences "all in-app monitoring". Apply it to every pooled source; addSource never resets the flag, so later-activated sources inherit it. Pool pointers are fixed and these are atomic stores, so the control-thread iteration is race-free. P3: clarify the existing "Mute direct monitoring" helper text so the two controls aren't confused — it mutes the dry passthrough only and a loaded amp sim is still heard; point users to "Disable input monitoring". Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: ChrisBeWithYou <chris@rifflarr.local> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: byrongamatos <xasiklas@gmail.com> |
||
|
|
43809fc19c |
audio: raise backingVolume default 0.7 -> 0.8 (#42)
Bring the backing track up ~1.2 dB so the player tone (leveled to -15.5 LUFS by RBFinalLeveler) sits with the music instead of dominating it. Part of the tone-vs-backing balance pass. Co-authored-by: Jafz2001 <ignacio.fritis@mundotelecomunicaciones.cl> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
cc0aceb365 |
feat(sandbox): native last-chance crash attribution for in-process VST3 faults (#36)
* feat(sandbox): native last-chance crash attribution for in-process VST3 faults (#35) The vst-crash-guard sentinel only covers the windows it arms around an in-process load or editor-open. A plugin that creates a top-level window keeps it for its whole loaded lifetime, and the OS can dispatch to its WndProc at any time (e.g. WM_ACTIVATEAPP on an alt-tab). A fault there arrives via USER32→ WndProc with no host frame on the stack — outside every armed sentinel window and uncatchable by the SignalChain guard — so it's never attributed and the app crash-loops (diagnosed from dmp a06f48e1 / McRocklin Suite; see #35). Add a process-wide last-chance attributor (Windows): a SetUnhandledException filter, chained to the previously installed filter (Crashpad), that on a fatal fault whose faulting instruction lies inside a loaded .vst3 module stamps the existing crash sentinel with { plugin, op: "native-crash" } and then defers to the prior filter so the dump is still produced and the process dies normally. initVstCrashGuard() already promotes a leftover sentinel into the persistent blocklist, so the next launch routes the offender to the out-of-process sandbox. This makes the dead-man's-pedal cover ANY fatal in-process VST3 fault, not just the armed load/editor windows — generalizing beyond the per-vendor pre-seed. - src/audio/Sandbox/CrashAttribution.{h,cpp}: install/uninstall + the filter. SetUnhandledExceptionFilter (last-chance only) avoids first-chance false positives and per-exception I/O; the write is allocation-free (stack buffers + raw Win32). No-op on non-Windows (POSIX SignalChain guard covers the armed path; sandbox is Windows-only today). - NodeAddon: setVstCrashSentinelPath(path) binding arms it; uninstall on shutdown (the addon/filter code may be unloaded). - vst-crash-guard.ts: export getSentinelPath(); audio-bridge wires it after initVstCrashGuard(). Addon builds clean; tsc --noEmit clean; sandbox tests + e2e unaffected. The Windows filter path needs hands-on validation (confirm the sentinel is written and Crashpad still dumps under the target Electron/Crashpad version). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * review: fix bundle-path attribution + one-shot gate in crash attributor Local review of #36 found two correctness bugs: - Inert on bundle VST3s: GetModuleFileNameW returns the INNER DLL of a Windows VST3 bundle (Foo.vst3\Contents\x86_64-win\Foo.vst3), but the blocklist keys on the bundle dir (desc.fileOrIdentifier = …\Foo.vst3). The two never matched, so a native-written sentinel never routed the offender to the sandbox — defeating the fix for bundle plugins. Add truncateToVst3Bundle(): resolve the module path to its enclosing .vst3 component in place before writing (single-file .vst3 is unchanged). Replaces endsWithVst3IgnoreCase. - One-shot latch burned by the wrong exception: the g_writing.exchange gate wrapped the whole filter evaluation, so the FIRST unhandled exception to reach the filter — even a non-VST3 or concurrent benign one — permanently disabled attribution for the real plugin fault. Move the latch to gate only the write, after a CONFIRMED .vst3 fatal fault; it still serialises concurrent plugin faults and guards write re-entrancy. Also: stop zeroing g_sentinelPathW in uninstall (the g_installed acquire-gate already disarms the write path; zeroing was the only non-atomic mutation that could race a faulting thread during teardown), and note the address-based attribution is a heuristic. Addon builds clean; tsc clean. Windows filter path still needs hands-on validation (sentinel written for a bundle + single-file VST3; Crashpad still dumps). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- 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 |