got-feedBack/feedBack-desktop
1
0
Fork 0
mirror of https://github.com/got-feedBack/feedBack-desktop.git synced 2026-07-23 05:11:43 +00:00
Code Issues Actions 3 Packages Projects Releases Wiki Activity
50 Commits 27 Branches 2 Tags 6.6 MiB
b5dce9af4d
Commit Graph

5 Commits

Author SHA1 Message Date
Jorge Fritis
b5dce9af4d
fix(audio): read-ahead the backing track off the RT audio thread (#60) 
* fix(audio): read-ahead the backing track off the RT audio thread

The backing AudioTransportSource was set up with no read-ahead buffer and
no reader thread — setSource(src, 0, nullptr, rate) — so the realtime audio
callback decoded the backing file synchronously inside getNextAudioBlock on
every block while a song plays. Any disk seek or codec spike (worst on
compressed formats) then blew the block's realtime budget, producing
underruns heard as glitches / brief mutes.

Interpose a juce::TimeSliceThread with 32768 source frames (~0.68 s @ 48 kHz)
of look-ahead so decode happens off the audio thread. The thread is declared
before backingTransport (so it is destroyed after it — the transport's
BufferingAudioSource holds a pointer to it) and started once in the ctor.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* docs(audio): document the bounded BufferingAudioSource lock residual

Codex review: readBufferSection() holds callbackLock across a refill
chunk decode and the RT callback takes the same lock. Accepted — the
window is bounded (2048-frame chunks) and only hit mid-refill, vs. the
old guaranteed synchronous decode every block; note it in the comment so
nobody mistakes the transport stack for fully RT-safe.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

---------

Co-authored-by: Jafz2001 <ignacio.fritis@mundotelecomunicaciones.cl>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: byrongamatos <xasiklas@gmail.com>
 2026-07-03 16:16:27 +02:00
Byron Gamatos
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>
 2026-06-29 23:12:31 +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
Jorge Fritis
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>
 2026-06-19 23:08:05 +02:00
Byron Gamatos
bd603184d5 Clean release snapshot 2026-06-16 18:48:12 +02:00