mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-09-11 05:04:10 +00:00
633ace20522d729e3eaab7019470672710c670f6
19
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 byrongamatosandClaude Opus 4.8
|
633ace2052 |
fix(audio): don't let "Default" wipe the capability input selection
Review of #112: syncSelectedInputSource() treated an empty device name as "nameless device — invalidate rather than guess" and called removeItem() on the persisted selection. But the input dropdown's first option is literally `<option value="">Default</option>`, so "" is the ordinary "use the OS default" choice, not a nameless device. init()'s auto-apply calls this on every startup, so a user sitting on Default had their capability selection deleted at each launch — and that selection is made in a DIFFERENT ui (the input_setup / tuner picker), so this silently discarded a device they explicitly chose. Because audioInputOpenHandler deliberately refuses to guess a device, a cleared selection leaves plugins with no input at all: the same dead-guitar symptom the PR set out to fix. Confirmed against the pre-fix build — a sync with the Default value emits removeItem on the stored key. "" now means "no opinion": leave the selection to the picker that owns it. Also extract inputSourceNameKey() as the single place a named input's key is built. Registration and selection were formatting the same template independently and had already drifted on the nameless branch (registration falls back to a positional key; sync emitted none). Tests pin the format, the Default behaviour, and a round-trip through the open handler's own parser regex. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
OmikronApexandClaude Fable 5
|
d791552261 |
fix(audio): skip redundant device rebind when opening the selected input
audioInputOpenHandler unconditionally called setDevice, so opening the selected input (tuner/note_detect on song start) restarted the engine even when the requested device was already bound — an audible dropout and the 'Audio paused unexpectedly' seen in tester logs. Now the handler reads the engine's actual binding (isAudioRunning + getCurrentDevice) and returns the bound identity without touching the device when input name and type already match; any read failure falls through to the normal rebind path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
OmikronApexandClaude Fable 5
|
854a53e8f7 |
fix(audio): sync capability input selection when the user applies a device
A stale feedBack.audioInput.selectedLogicalSourceKey (persisted by the audio-session capability) survived device changes made in the audio settings screen. The next plugin to open the 'selected input' (tuner / note_detect on song start) re-applied the stale device via audioInputOpenHandler, clobbering the engine's input mid-session — seen in the field as guitar input going dead when starting a song (vorrin logs: ASIO M-Audio input replaced by 'Microphone (WO Mic Device)' ~0.5s after playSong, input level 0.0005). Fix: after a successful device apply (Apply button and init auto- apply), select the matching source in the audio-session capability so its in-memory selection and persisted key follow the user's choice; fall back to rewriting/removing the localStorage key directly when the capability or source isn't available yet. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
byrongamatos
|
e29312f446 |
fix(renderer): roll back the mute-suppression latch when the IPC fails
CodeRabbit caught a real bug in the previous commit's fix. The latch mirrors the NATIVE refcount, but it was flipped before the invoke resolved: a rejected release left it reading "released" while the engine still held the suppression, so every later release short-circuited and monitor mute stayed suppressed for good — the same stuck-suppression bug the latch exists to prevent, just one level up. The latch now only stays flipped if the call actually landed, and rolls back otherwise (guarded so a newer call can't be clobbered by a stale rejection). A downlevel addon with no arbiter leaves the latch untouched instead of recording a hold it never acquired. Pins the whole contract with a vm-extracted unit test on the real screen.js function: unpaired acquires hold at most one native suppression, cycles stay balanced across 25 song loads, a rejected release retries, and both the downlevel and sync-throw paths are clean. Fails 3/5 against the original branch (the refcount leak) and 2/5 against the pre-rollback version. |
||
|
byrongamatos
|
a332c35c9b |
fix(audio): close the PR #107 review findings
Seven fixes on top of the audio-engine TLC branch, each with the gate that catches its regression. Blocking: - Monitor-mute suppression leaked its refcount. setMonitorMuteSuppressed() became a refcounted acquire/release, but screen.js's callers are deliberately unpaired: resolveChainRebuildGuard() leaves the suppression on when a rebuild yields an empty chain, and returns early without releasing while a provider route is still resolving. Harmless against the old latched bool, a permanent +1 each against a refcount — after a failed tone rebuild the count never returned to zero and monitor mute was silently dead for the rest of the session. The renderer now holds at most one suppression. - Slot ids are monotonic HANDLES (nextSlotId, never reset by clear()), not bounded indices, so argSlotId's 4096 ceiling meant that once a session created its 4096th processor EVERY guarded binding — setBypass, setParameter, remove/moveProcessor, open/closePluginEditor — silently no-opped for the rest of the run. Ceiling removed (same for SetMultiBypass's hardcoded 4096); unknown ids are still rejected by SignalChain::findSlotIndex. - clearChain / removeProcessor / moveProcessor took chainMutationMutex with a blocking lock_guard on the N-API thread — Electron's main thread, and on macOS also the JUCE message thread. LoadPreset/LoadVST hold that mutex across an unbounded plugin init (done->wait() has no timeout by design), so a slow plugin froze the whole main process, every IPC channel with it. They now queue on a libuv worker via queueChainMutation() and resolve a promise; the bridge awaits them so callers still observe the mutation applied. Also: - getChainState() dereferenced raw ProcessorSlot* returned by getAllSlots() after the lock was dropped — a concurrent clear() frees them under the reader. Replaced with SignalChain::getSlotSummaries(), which copies under the lock. getAllSlots() is gone (it had one caller). - The device-settings migration removed the localStorage copy even when the file-store save failed or was unavailable, losing the user's settings. - SetSlotState and GetParameters kept the raw Int32Value() path: IsNumber() is true for NaN, so setSlotState(NaN) wrote onto slot 0 — the same coercion class the rest of the branch fixed. - RendererBus flushed to the LIVE writeIndex, so a disable→re-enable with no pull in between discarded the freshly pushed audio along with the stale tail. It now snapshots the flush target at disable time. - LoadPreset's rebuild barrier is now released by a scope guard, so a throw between arming it and Queue() can't block editor opens forever. Gates: new renderer-bus case (fails on the old flush), new slot-id-handle case (fails on the old ceiling). ctest 9/9, npm test 79 pass / 0 fail, chain-mutation storm green, addon export contract unchanged. |
||
|
OmikronApexandClaude Fable 5
|
f0340ed425 |
fix(audio-engine): single persistence store for device settings (TLC Part II §4)
The device config was persisted in TWO stores — the main process's file-backed settings AND localStorage['slopsmith-audio-device'] — merged on load by newest-savedAt. A main-side migration/reset left stale localStorage that could win the timestamp race and resurrect wiped settings, and a device re-save from either path re-persisted mute flags captured at that moment, interleaving with the (now-arbitrated) runtime mute writers. The file store is now the only write target. localStorage is treated as a one-time migration source: a strictly-newer browser copy is imported into the file store, then the key is deleted either way — after the first load the file is the single source of truth. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> |
||
|
|
dd7ad9b786 |
feat(update): add nightly Velopack update channel (#80)
Adds `nightly` as a selectable auto-update channel (Windows + macOS). Client: - update-manager.ts: add 'nightly' to UpdateChannel (veloChannel already yields win-x64-nightly / osx-arm64-nightly, so no logic change) - main.ts: allow 'nightly' in the runtime IPC channel guard - preload.ts: add 'nightly' to the preload-local UpdateChannel union - screen.js / settings.html: add the Nightly option + helper text CI (nightly.yml): - derive <pkg>-nightly.<UTC date> version in the setup job - setup-dotnet (pinned from .build-config.json) so the vpk CLI has net8, matching build.yml - vpk pack win-x64-nightly / osx-arm64-nightly (mac signed + notarized, mirroring build.yml's signed/unsigned fallback) - publish a rolling `nightly` GitHub Release (prerelease=false, latest=false) that the in-app updater reads for the nightly channel - concurrency guard so an overlapping dispatch can't race the rolling release Co-authored-by: Claude Opus 4.8 (1M context) <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> |
||
|
|
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> |
||
|
|
0eabbceb73 |
feat(audio): own-rig opt-in — gate saved tone-chain restore on use_amp_sims (#46) (#50)
* feat(audio): own-rig opt-in — gate saved tone-chain restore on use_amp_sims Second half of #46 (desktop side), paired with the core onboarding PR. The amp-sim/tone chain auto-restores from localStorage on every launch, so once a user has loaded a tone they get a processed monitor forever — an idle high-gain amp is a constant distorted buzz, and the dry-only monitor mute can't kill it (the full monitor kill from #47 can, but only on demand). This makes monitoring "own-rig first": at app init, read the core `use_amp_sims` preference (set during onboarding / the new toggle) and only auto-restore the saved signal chain when the user opted IN. Default OFF — a missing key or any read failure is treated as opt-out, so a flaky/late backend can never resurrect the buzz. With no chain loaded, the existing default-on dry mute keeps the monitor silent. - screen.js: aeUseAmpSims() reads /api/settings; init gates loadDefaultPreset + saved-chain restore behind it. Extracted the restore loop into aeRestoreSavedChain() (shared by init and the live opt-in toggle). - screen.html/js: new "Use in-app amp sims" checkbox in Audio settings, persisted to /api/settings (shared with onboarding). Reflects the saved value on load; turning it ON loads the saved chain immediately (no restart). Stacked on #47 (monitor kill). node --check clean. NOT built/run here — the renderer change needs a desktop build + a tester check: with a saved tone and amp sims OFF, launch is silent (no buzz); toggling ON loads the tone live; the onboarding choice carries through. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(audio): make the amp-sim toggle apply live (Codex review on #50) Addresses Codex findings on the "Use in-app amp sims" checkbox: - P2: turning it OFF now clears the live engine chain so monitoring actually goes silent this session (with no processors, the default-on dry mute silences the bus) — previously OFF persisted the pref but left the amp running, so the checkbox lied and the buzz persisted until restart. The saved chain in localStorage is left intact (we don't call saveChainState) so re-enabling restores the same tone. - P2: ON no longer stacks a duplicate chain — when there's no default preset (so loadDefaultPreset returns without clearing) we clearChain() before aeRestoreSavedChain() instead of appending onto the current chain. - P3: the /api/settings POST now warns on a non-ok HTTP status. node --check clean. Still needs a desktop build + tester pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> * fix(audio): render empty chain directly on amp-sim OFF (avoid getChainState-after-clearChain JUCE crash) Codex re-review P2: the OFF path called refreshChain() right after clearChain(), which getChainStates the native engine — a sequence the codebase documents can crash some JUCE bridges (clearChainForNewSong). Render the empty-chain placeholder directly instead, mirroring that safe pattern. localStorage is still preserved so re-enabling restores the tone. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.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> |
||
|
|
e7b7a08f44 |
audio: tag input device labels with driver type (ASIO/WASAPI) on Windows (#44)
The setup wizard's audio-input picker showed one entry per device with no way to tell ASIO from WASAPI/DirectSound — testers asked to see the driver type "to really figure out their setup." The renderer registers one source per (driver type x device) but put the type only in the logicalSourceKey, not the label, so every variant shared an identical name. Core's input_setup then de-dupes by label and collapsed them to one — silently pinning whichever variant sorted first (often not the low-latency ASIO one). Append the driver type to the source label (and the redaction pseudonyms) so the variants read "Focusrite (ASIO)" vs "Focusrite (Windows Audio)" and the label de-dupe stops collapsing them. Gated on more than one driver type actually exposing inputs, so macOS (Core Audio only) shows no redundant suffix. Renderer-only; verified by syntax check. The node test suite covers src/main config logic, not this path; ASIO/WASAPI enumeration is verified on a Windows desktop build with a real interface. Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF Co-authored-by: ChrisBeWithYou <chris@rifflarr.local> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> |
||
|
|
59e1c1cb0e |
fix(preload): expose desktop bridge as window.feedBackDesktop (#40)
* fix(preload): expose desktop bridge as window.feedBackDesktop The core feedback app reads window.feedBackDesktop, but the desktop preload exposed the bridge as window.slopsmithDesktop. On the desktop build window.feedBackDesktop was therefore undefined: the DLC-folder Browse button stayed hidden in both the first-run wizard (#v3-ob-songdir-browse) and Settings (#btn-pick-dlc), and the rest of the bridge silently fell back to browser mode. Finish the rebrand: rename the exposed global slopsmithDesktop -> feedBackDesktop, plus the internal api object, the renderer + plugin-manager consumers, the private __feedBackDesktopAudioHooks scratch namespace, and the comments/migration doc. No compatibility alias — the ecosystem moves to the new name (TARGET-CURRENT). Plugins that still read window.slopsmithDesktop are renamed in their own PRs; nothing ships until the next desktop build bundles them together, so there is no broken shipped artifact. Fixes the "Select DLC Songs Folder — No Browse" report (wizard + Settings, Mac + Windows). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QbexxfTt8q2tAn436MqGWF * fix(preload): also expose bridge under legacy slopsmithDesktop name Keep plugins/community code built against the pre-rename bridge working after the rename. Same isMainFrame gating. See got-feedback/feedBack-desktop#41. 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> |
||
|
|
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>
|
||
|
|
b6515a0585 |
fix(audio): real device names are not labelSafe pseudonyms (#33)
Codex review of #32: the audio-session sanitizer (_safeInputLabel) returns `labelPseudonym` UN-redacted (it's assumed already safe), so putting a raw OS device name there leaks PII (e.g. "Byron's AirPods") to diagnostics/consumers regardless of labelSafe. Put the real name in `label` instead — the field the sanitizer can redact for suspicious/PII-looking names — and mark labelSafe only for the generic "Desktop input N" fallback. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> |
||
|
|
f18b0a51fd |
fix(audio): surface real audio input device names (not "Desktop input N") (#32)
registerAudioSessionInputSources() discarded the real device name (the
forEach arg was `_deviceName`) and labelled every audio-input source with a
generic `Desktop input ${index+1}` pseudonym — so the audio-input picker
showed "Desktop input 1/2/…" instead of the actual device names. The real
names are already available: AudioEngine reads JUCE getDeviceNames(true),
NodeAddon exposes them as typeInfo.inputs, and the renderer already uses
them for setDevice(). Use that name as the source label, falling back to the
generic form only when it's empty.
(Re-applies a fix that previously lived on the dead byrongamatos/slopsmith-
desktop branch and never reached got-feedback/main.)
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
||
|
Byron Gamatos
|
bd603184d5 | Clean release snapshot |