Commit Graph

5 Commits

Author SHA1 Message Date
OmikronApex
d887c68014 docs: commit the audio-engine TLC findings + plan this branch implements
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-07-14 03:24:46 +02:00
OmikronApex
3e3f1f868c
fix(audio): never feed chain processors blocks larger than prepared size (#85)
Some checks are pending
Addon CI / addon (arm64, macos-14, mac) (push) Waiting to run
Addon CI / addon (x64, ubuntu-22.04, linux) (push) Waiting to run
Addon CI / addon (x64, windows-latest, win) (push) Waiting to run
Ship CI / CI (push) Waiting to run
* fix(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>
2026-07-08 22:44:26 +02:00
ChrisBeWithYou
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>
2026-06-29 00:49:25 +02:00
ChrisBeWithYou
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>
2026-06-27 13:16:03 +02:00
Byron Gamatos
bd603184d5 Clean release snapshot 2026-06-16 18:48:12 +02:00