mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-09-10 23:54:10 +00:00
9d0963d6d53c77d73464e46df008f6430424c869
25
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 OmikronApexandClaude Fable 5
|
9d0963d6d5 |
feat(audio): getLatencyBreakdown — one owner for every latency term
Deep-read §5: latency had three unreconciled truths — getLatencyMs' static half-capacity ring guess (42.7 ms), the verifier's input-delta-only offset, and the renderer bus adding prime+fill+resample that no figure surfaced. New engine API + export: per-term breakdown (deviceBufferMs, input/output driver latency, MEASURED split-ring residency, monitor total) plus the renderer-bus song-audio delay (measured bus fill) as its own term. On the user's split exclusive setup the measured ring sits at ~10 ms — the legacy figure overstated monitor latency by ~33 ms (102.7 reported vs ~70 real). getLatencyMs is unchanged for compatibility; UI adoption of the breakdown is renderer follow-up. Snapshots regenerated (104 exports). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
OmikronApexandClaude Fable 5
|
88f881dd1e |
fix(audio): refcounted monitor-mute arbiter (TLC Part II §2)
The old single monitorMuted atomic had five writers fighting last-writer-wins: the settings checkbox, startup restore, the executor's preload read-force-restore, releaseRoute's unconditional setMonitorMute(true) (which clobbered the user's persisted preference), and the renderer's song-load suppression (un-refcounted — overlapping windows un-suppressed each other early). Native arbiter on SourceChain: userMonitorMute (the preference — checkbox + restore only), refcounted monitorMuteHolds (force-mute overrides), and refcounted suppressions (setMonitorMuteSuppressed keeps its bool surface; true=acquire, false=release, clamped at 0). Effective dry-mute = (holds || pref) && chain empty && no suppression — the suppressed-beats-muted precedence is unchanged. New exports: acquire/releaseMonitorMuteHold, getMonitorMuteState (diag); snapshots regenerated. Executor rewrite: acquires a suppression (dry-during-load, the default) or a hold, and releases exactly what it acquired via a single-fire closure that runs UNCONDITIONALLY (each load owns its acquisition — the stale-snapshot race against a mid-hold user toggle is structurally gone). releaseRoute no longer touches mute state at all. The ownership test now pins: preference API never called, acquire/release balanced. Renderer callers are unchanged: the checkbox writes the preference as before, and the song-load suppression sites now compose instead of racing. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
OmikronApexandClaude Fable 5
|
7b72907503 |
fix(audio-effects): executor detects foreign chain writes via chainGeneration
The JS half of the phase-7a serializer (TLC Part II §1, executor-state
hazard): the executor's stageSlots map (stageId → native slotId) is built at
load time, but any direct loadPreset/clearChain from the audio_engine bundle
or rig_builder's legacy path silently invalidated it — subsequent
setStageBypass/setStageParameter/activateSegment flipped bypass/params on
the WRONG slots or returned no-target with nothing detecting the divergence.
Now: the route records the chainGeneration its load returned; every stage
operation compares it against getChainGeneration() first and reports a
stale-route no-target ('re-load the plan', with expected/current generations)
instead of mutating someone else's chain. loadChainPlan also verifies the
generation didn't move between its loadPreset and the getChainState slot
mapping, rolling back if a foreign write landed in that window. Old addons
without the counter degrade gracefully (checks no-op).
Pinned by a new executor test: fresh route flows, foreign bump → all three
stage ops refuse without touching native slots.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
||
|
OmikronApexandClaude Fable 5
|
dd40b2f227 |
fix(audio): renderer-bus flush flag + reconfigure reads user intent (phase 8)
Two deep-read fixes now homed in their phase-1/2 units:
RendererBus (§4): setEnabled(false) no longer writes readIndex from the
control thread — the ring's designated consumer-side writer is pull(). The
drop-on-disable is now a flushRequested atomic the consumer honors at its
next pull, closing the last SPSC-discipline hole (a concurrent pull
mid-drain could overwrite the control thread's store and replay a stale
tail after re-enable). New unit test pins flush-then-fresh-audio.
setAudioDevices (§3): the restart decision reads state.userWantsAudio
(intent, written only by start/stopAudio) instead of the racy device-state
flag that transient audioDeviceStopped() fires clear — a reconfigure landing
inside a transient-stop window no longer leaves the engine configured but
stopped ('no audio until Start/Apply is pressed again').
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
||
|
OmikronApexandClaude Fable 5
|
db337eaf29 |
fix(audio): serialize chain mutations + chainGeneration (phase 7a)
The single highest-value fix of the TLC pass (deep-read §1): one native chain-mutation mutex (addon/ChainOps) held across the FULL Execute() of every chain worker (LoadPreset/LoadVST/LoadNAM/LoadIR/ReplaceIR) and the synchronous mutators (clearChain/removeProcessor/moveProcessor). Two overlapping loadPreset calls can no longer interleave clear()/addProcessor() into a merged-garbage chain — the plugin-vs-plugin fight becomes last-writer-wins. chainGeneration (monotonic, bumped under the mutex) is returned in loadPreset results and exposed as getChainGeneration (new export, snapshot regenerated), so the audio-effects executor can detect a foreign write invalidated its stageSlots map and re-sync instead of flipping bypass/params on wrong slots — the prerequisite for the single-chain-owner ownership track. Also rides here: LoadPresetWorker's slot-state restore goes through setSlotState() instead of const_cast (deep-read §9). The phase-0 storm test flips from expected-fail to a hard gate: 50 iterations of concurrent loadPreset now always end with exactly one caller's chain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
OmikronApexandClaude Fable 5
|
d4e0bfc272 |
refactor(audio): extract AddonContext + NapiHelpers, guard raw N-API args (phase 6)
AddonContext (src/audio/addon/): engine/vstHost lifetime + snapshot rule, the JUCE message thread with the macOS no-pump fork quarantined into ONE file, the shutdown latch (exposed as isShuttingDown), doShutdown with a UI teardown hook (NodeAddon points it at the editor-window nuke, #56), and the pending-async-load registry. NodeAddon keeps using-declarations so the binding bodies are unchanged. Also fixes SetBackingSpeed's bare `engine` dereference — the one binding that dodged the file's own snapshot rule. NapiHelpers (typed extractors argInt/argSlotId/argFiniteFloat/argBool/ argMidiChannel/argMidiByte) + rewrites of the unguarded bindings — the deep-read §2 fix, done once: SetParameter/SetBypass/RemoveProcessor/ MoveProcessor/SetMultiBypass/SendMidiToSlot/SetGain plus the St-1 routing quartet (SetPan/SetPostGain/SetBranch/SetBranchSrc). NaN slot ids no longer coerce to slot 0; MIDI channel/program are range-checked before JUCE. New gate: tests/napi-arg-fuzz.test.js — table-driven garbage (NaN/Inf/ negative/string/missing/object) against the real addon; chain state must be byte-identical after the storm and a valid call must still apply. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
OmikronApexandClaude Fable 5
|
6ace5a209a |
refactor(audio): extract DeviceSetup + shared rate-match helpers (phase 4)
Moves probeDeviceOptionsDual, applyDuplexSetup, applySplitSetup, and
teardownSplitMode verbatim into src/audio/engine/DeviceSetup.{h,cpp}. The
component holds references to the two device managers + EngineState and owns
no lifetime; engine-owned collaborators (monitor chain, split output ring +
counters, output callback registration) are passed by reference per call.
setAudioDevices stays on the facade as the orchestrator. The public
DeviceOptions/DeviceConfig/DeviceConfigResult shapes move to the slopsmith
namespace with using-aliases on AudioEngine, so the NodeAddon spelling is
unchanged.
Lands the deep-read §7 dedupe structurally: the <=0.5 rate tolerance,
midpoint-rounding fail-closed candidate, and empty-name→first-enumerated
resolution now exist once (RateMatch.h — JUCE-free + unit-tested boundary
cases — and DeviceSetup::resolveDeviceName/rateSupportedBy) instead of three
hand-synced copies.
Full device-matrix validation (WASAPI shared/exclusive, ASIO, dual-type
split) rides the next tester build per the plan's phase-4 gate.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
||
|
OmikronApexandClaude Fable 5
|
70f3316094 |
refactor(audio): extract RendererBus (phase 2)
Moves the WebAudio→engine bus — ring, producer-side linear resampler, prefill gate, fill clamp, metrics — verbatim into src/audio/engine/RendererBus.h. AudioEngine keeps thin facades (setRendererBus/pushRendererAudio/pullRendererBus/getRendererBusMetrics) so the NodeAddon surface is unchanged. JUCE-free: pull() takes raw channel pointers, which is what lets tests/engine_units drive the resampler continuity, prime/underflow/clamp, and metrics cases without a device. The control-thread readIndex write on disable (deep-read §4) is preserved verbatim and marked; its flush-flag fix lands as the phase-8 commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
OmikronApexandClaude Fable 5
|
eb40b87dea |
refactor(audio): extract EngineState with intent/state split (phase 1)
Moves the shared run-state atomics (currentSampleRate, block sizes, duplexMode, run flags) into slopsmith::EngineState (src/audio/engine/) so later extracted units take EngineState& and stay unit-testable without JUCE devices. AudioEngine binds the members back by reference under their historical names — zero call-site churn, behavior-identical. The old audioRunning conflated user intent with device state (deep-read §3/§6); it is now state.deviceRunning (same semantics, isAudioRunning compat pinned) plus a new state.userWantsAudio written only by startAudio/stopAudio. Nothing reads the intent flag yet — phase 8 flips setAudioDevices' restart decision onto it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
OmikronApexandClaude Fable 5
|
eeb83cbdbc |
refactor(audio): extract PackedStereoRing — one SPSC ring template (phase 1)
Replaces the four hand-maintained copies of the packed-LR SPSC design (split-mode output ring, per-InputDeviceSlot rings, stream-sink ring, renderer-bus ring) with slopsmith::PackedStereoRing<NFrames> (src/audio/engine/PackedStereoRing.h). The template owns the storage, power-of-two/lock-free asserts, pack/unpack, producer publish, reset, the w<r resync, and the lapped catch-up; per-site consumer policy (pull-vs- consume skew, renderer prime/fill-clamp) stays verbatim at the call sites. Pure code move per the TLC plan — no behavior change; the renderer bus's control-thread readIndex write on disable (deep-read §4) is deliberately preserved and gets its flush-flag fix in phase 2. Unit-tested in tests/engine_units/packed_stereo_ring_test.cpp: threaded tear-freedom under lapping (2M frames), drop-oldest catch-up, index-reset resync, pull-vs-consume skew. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
OmikronApexandClaude Fable 5
|
3e449c0318 |
test(audio): quarantined chain-mutation storm test (expected-fail)
Documents TLC deep-read §1: concurrent loadPreset workers interleave clear()/addProcessor() and merge both presets — reproduces first iteration ([storm-ir-1-0, storm-ir-2-0, storm-ir-2-1]). Quarantined behind CHAIN_STORM=1; flips to a hard gate when ChainOps lands the serializer (plan phase 7). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
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> |
||
|
OmikronApexandClaude Fable 5
|
bbb3b58db8 |
test(contracts): Phase 0.a contract snapshots for audio surface
Snapshot the three public surfaces the audio-engine decomposition must not change (docs/audio-engine-tlc.md Part IV §4): addon export table, audio-bridge IPC channels, preload audio/audioEffects API keys. contract-check.test.js diffs regenerated surfaces against the committed snapshots. result-shapes.json (golden result key/type shapes) is deferred until the engine_units harness can run the addon against a null device. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
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 |