mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-09-11 01:44:11 +00:00
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>
This commit is contained in:
co-authored by
Claude Opus 4.8
parent
06c68262a9
commit
92a78b4c9a
@@ -1577,6 +1577,16 @@ SourceChain* AudioEngine::getSource(int id)
|
||||
return (id == 0 || src.isActive()) ? &src : nullptr;
|
||||
}
|
||||
|
||||
void AudioEngine::setMlNoteDetectionEnabled(bool e)
|
||||
{
|
||||
// Fan to every source in the fixed pool (not just active ones) so a source
|
||||
// activated later inherits the current arm state instead of silently
|
||||
// staying dormant. Each MlNoteDetector::setEnabled is a cheap atomic + a
|
||||
// cold-state clear on a real transition.
|
||||
for (int i = 0; i < kMaxSources; ++i)
|
||||
sources[(size_t) i]->getMlNoteDetector().setEnabled(e);
|
||||
}
|
||||
|
||||
std::vector<AudioEngine::SourceInfo> AudioEngine::listSources() const
|
||||
{
|
||||
std::vector<SourceInfo> out;
|
||||
|
||||
@@ -30,6 +30,13 @@ public:
|
||||
PitchDetector& getPitchDetector() { return source0().getPitchDetector(); }
|
||||
MlNoteDetector& getMlNoteDetector() { return source0().getMlNoteDetector(); }
|
||||
|
||||
// Arm/suspend the ML note-detection pipeline across every source's detector.
|
||||
// Defaults off; the renderer (note_detect) calls this true only while a
|
||||
// consumer actually reads ML notes (native-frame detection / non-verifier
|
||||
// fallback) and false otherwise, so the default harmonic-comb verifier path
|
||||
// — and the always-on home tuner — never pay for ONNX inference. Main thread.
|
||||
void setMlNoteDetectionEnabled(bool e);
|
||||
|
||||
// Load the Basic Pitch ONNX model for the polyphonic ML detector. When a
|
||||
// model is loaded, getActiveDetection() / scoreChord() route through it;
|
||||
// otherwise they fall back to the YIN PitchDetector / ChordScorer.
|
||||
|
||||
@@ -141,6 +141,25 @@ struct MlNoteDetector::Impl
|
||||
// than an ML scorer that would return all-misses until the window fills.
|
||||
std::atomic<bool> hasPublished{ false };
|
||||
|
||||
// Master gate. The ML pipeline is the single most expensive thing in the
|
||||
// engine (~30 ms ONNX inference every hop), and on the default desktop path
|
||||
// note detection is scored by the harmonic-comb NoteVerifier — nothing reads
|
||||
// the ML detector. So it defaults OFF and the renderer arms it (via
|
||||
// setNoteDetectionEnabled) only when a consumer actually needs it
|
||||
// (native-frame detection / non-verifier fallback). When false the audio
|
||||
// thread stops feeding pushSamples and the inference thread runs no Run(),
|
||||
// so a silent home tuner — or an in-song verifier session — costs nothing.
|
||||
std::atomic<bool> enabled{ false };
|
||||
|
||||
// Requested by setEnabled(true): the INFERENCE THREAD clears the rolling
|
||||
// window/FIFO/snapshot on its next wake before the first inference, giving a
|
||||
// cold start on (re)arm instead of scoring against audio left over from a
|
||||
// previous arm. The reset runs on the thread (never the N-API thread) so it
|
||||
// can't race ingest(); pushSamples() no-ops while it's pending so the audio
|
||||
// thread isn't writing the FIFO mid-reset. This preserves the invariant that
|
||||
// clearAudioState() only runs when nothing else is touching the buffers.
|
||||
std::atomic<bool> resetPending{ false };
|
||||
|
||||
// Start / stop the background inference thread. prepare() and stop() use
|
||||
// these so the thread is never alive while clearAudioState() mutates the
|
||||
// FIFO / inQueue / circular buffer — otherwise a device stop→start cycle
|
||||
@@ -149,6 +168,19 @@ struct MlNoteDetector::Impl
|
||||
{
|
||||
thread = std::make_unique<MlInferenceThread>([this]()
|
||||
{
|
||||
// Cold-start reset requested by setEnabled(true) — runs here, on the
|
||||
// thread that owns the buffers. Keep resetPending set THROUGH the
|
||||
// reset (clear it only after) so pushSamples() stays gated off the
|
||||
// FIFO for the whole drain, then release it.
|
||||
if (resetPending.load(std::memory_order_acquire))
|
||||
{
|
||||
threadColdStart();
|
||||
resetPending.store(false, std::memory_order_release);
|
||||
}
|
||||
// Master gate: when no consumer wants ML, run nothing — no ingest,
|
||||
// no inference — so the thread sits idle instead of burning CPU.
|
||||
if (! enabled.load(std::memory_order_acquire))
|
||||
return;
|
||||
ingest();
|
||||
runInferenceIfDue();
|
||||
});
|
||||
@@ -169,7 +201,11 @@ struct MlNoteDetector::Impl
|
||||
}
|
||||
}
|
||||
|
||||
void clearAudioState()
|
||||
// Reset everything EXCEPT the FIFO. These fields are touched only by the
|
||||
// inference thread (resampler/inQueue/circ/window counters) or under
|
||||
// snapshotLock (the published snapshot), so this is safe to call either with
|
||||
// the thread stopped (prepare/stop) or on the thread itself (cold re-arm).
|
||||
void clearBuffersExceptFifo()
|
||||
{
|
||||
resampler.reset();
|
||||
inQueue.clear();
|
||||
@@ -177,7 +213,6 @@ struct MlNoteDetector::Impl
|
||||
circWrite = 0;
|
||||
totalResampled = 0;
|
||||
sinceInference = 0;
|
||||
fifo.reset();
|
||||
onsetTimeMs.fill(0.0);
|
||||
onsetSeq.fill(0);
|
||||
onsetConf.fill(0.0f);
|
||||
@@ -197,6 +232,27 @@ struct MlNoteDetector::Impl
|
||||
hasPublished.store(false, std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
// Full reset including fifo.reset(). fifo.reset() mutates BOTH FIFO indices
|
||||
// and is NOT safe against a concurrent producer, so this must only be called
|
||||
// with the audio callback quiescent: prepare()/stop() call it after
|
||||
// stopThread() joins the inference thread, and the device is not streaming.
|
||||
void clearAudioState()
|
||||
{
|
||||
fifo.reset();
|
||||
clearBuffersExceptFifo();
|
||||
}
|
||||
|
||||
// Cold re-arm on the inference thread. Cannot fifo.reset() here — the audio
|
||||
// thread may still be producing — so DRAIN the FIFO instead (advancing only
|
||||
// the read index, which is the consumer's own; safe SPSC against a
|
||||
// concurrent pushSamples). pushSamples() also gates on resetPending while
|
||||
// this runs, so in practice the producer is quiescent anyway.
|
||||
void threadColdStart()
|
||||
{
|
||||
fifo.finishedRead(fifo.getNumReady());
|
||||
clearBuffersExceptFifo();
|
||||
}
|
||||
|
||||
// Drain the FIFO, resample to 22050 Hz, append to the rolling window.
|
||||
void ingest()
|
||||
{
|
||||
@@ -371,10 +427,13 @@ bool MlNoteDetector::isAvailable() const
|
||||
|
||||
bool MlNoteDetector::isReady() const
|
||||
{
|
||||
// Available AND has published at least one inference snapshot — the
|
||||
// engine gates ML routing on this so the cold-start window after an
|
||||
// audio start/restart uses the YIN/ChordScorer fallback.
|
||||
// Available AND armed AND has published at least one inference snapshot.
|
||||
// The engine gates ML routing on this, so: the cold-start window after an
|
||||
// audio start/restart (or a re-arm) uses the YIN/ChordScorer fallback, and
|
||||
// a suspended detector (enabled=false) routes to YIN rather than serving the
|
||||
// stale snapshot left from its last arm.
|
||||
return isAvailable()
|
||||
&& impl->enabled.load(std::memory_order_acquire)
|
||||
&& impl->hasPublished.load(std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
@@ -452,9 +511,41 @@ void MlNoteDetector::stop()
|
||||
impl->clearAudioState();
|
||||
}
|
||||
|
||||
void MlNoteDetector::setEnabled(bool e)
|
||||
{
|
||||
if (impl->enabled.load(std::memory_order_relaxed) == e) return; // dedup; no spurious re-arm
|
||||
if (e)
|
||||
{
|
||||
// Arming. Drop readiness SYNCHRONOUSLY, BEFORE exposing enabled=true, so
|
||||
// no reader (isReady() → DetectNotes/getActiveDetection) can observe the
|
||||
// previous arm's snapshot during the cold-start window. isReady() loads
|
||||
// enabled (acquire) before hasPublished, so seeing enabled=true here
|
||||
// guarantees seeing hasPublished=false. Also request the thread-side
|
||||
// buffer cold start (drains the FIFO + clears the window on the thread).
|
||||
impl->hasPublished.store(false, std::memory_order_release);
|
||||
impl->resetPending.store(true, std::memory_order_release);
|
||||
}
|
||||
// Publish the new state last (release). Disarming needs no buffer work: the
|
||||
// thread sees !enabled on its next wake and idles, and isReady() gates on
|
||||
// enabled so a suspended detector serves the YIN fallback, not a stale snapshot.
|
||||
impl->enabled.store(e, std::memory_order_release);
|
||||
}
|
||||
|
||||
bool MlNoteDetector::isEnabled() const
|
||||
{
|
||||
return impl->enabled.load(std::memory_order_relaxed);
|
||||
}
|
||||
|
||||
void MlNoteDetector::pushSamples(const float* data, int numSamples)
|
||||
{
|
||||
if (numSamples <= 0) return;
|
||||
// Master gate (audio thread, relaxed atomic loads — no lock, no allocation):
|
||||
// don't feed the FIFO when no consumer wants ML, nor while a thread-side
|
||||
// cold-start reset is pending (the reset clears the FIFO — keep the audio
|
||||
// thread off it until that's done). The inference thread also early-returns,
|
||||
// so the whole pipeline is dormant until armed.
|
||||
if (! impl->enabled.load(std::memory_order_relaxed)
|
||||
|| impl->resetPending.load(std::memory_order_relaxed)) return;
|
||||
// Lock-free write; if the FIFO is full (inference stalled) the oldest
|
||||
// unread samples are simply not overwritten — we drop the newest instead,
|
||||
// which never blocks or allocates on the audio thread.
|
||||
@@ -563,6 +654,8 @@ bool MlNoteDetector::isReady() const { return false; }
|
||||
bool MlNoteDetector::loadModel(const juce::File&) { return false; }
|
||||
void MlNoteDetector::prepare(double, int) {}
|
||||
void MlNoteDetector::stop() {}
|
||||
void MlNoteDetector::setEnabled(bool) {}
|
||||
bool MlNoteDetector::isEnabled() const { return false; }
|
||||
void MlNoteDetector::pushSamples(const float*, int) {}
|
||||
std::vector<MlNoteDetector::ActiveNote> MlNoteDetector::getActiveNotes() const { return {}; }
|
||||
MlNoteDetector::ActiveNote MlNoteDetector::getDominantNote() const { return {}; }
|
||||
|
||||
@@ -47,6 +47,18 @@ public:
|
||||
void prepare(double sampleRate, int blockSize);
|
||||
void stop();
|
||||
|
||||
// Master gate. Defaults OFF: the ML pipeline only runs inference (and only
|
||||
// accepts pushSamples) while enabled. The renderer arms it via the
|
||||
// setNoteDetectionEnabled bridge when a consumer actually needs ML notes
|
||||
// (native-frame detection / non-verifier fallback); the default desktop path
|
||||
// scores with the harmonic-comb NoteVerifier and leaves this off, so a home
|
||||
// tuner — or an in-song verifier session — pays nothing for ML. Disabling
|
||||
// clears the rolling window + published snapshot (cold re-arm); the inference
|
||||
// thread stays alive but idle, so toggling needs no thread restart. Safe to
|
||||
// call from the N-API/main thread; the audio thread reads the flag lock-free.
|
||||
void setEnabled(bool e);
|
||||
bool isEnabled() const;
|
||||
|
||||
// Audio thread — lock-free, no allocation.
|
||||
void pushSamples(const float* data, int numSamples);
|
||||
|
||||
|
||||
@@ -616,6 +616,20 @@ static Napi::Value SetMonitorMute(const Napi::CallbackInfo& info)
|
||||
return info.Env().Undefined();
|
||||
}
|
||||
|
||||
// setNoteDetectionEnabled(bool) -> undefined. Arms/suspends the polyphonic ML
|
||||
// note-detection pipeline across all sources. The renderer (note_detect) calls
|
||||
// this true only while a consumer actually reads ML notes (native-frame
|
||||
// detection / non-verifier fallback) and false otherwise — the default
|
||||
// harmonic-comb verifier path and the always-on home tuner leave ML suspended,
|
||||
// so the engine runs no ONNX inference when nothing needs it.
|
||||
static Napi::Value SetNoteDetectionEnabled(const Napi::CallbackInfo& info)
|
||||
{
|
||||
auto liveEngine = snapshotEngine();
|
||||
if (liveEngine && info.Length() > 0)
|
||||
liveEngine->setMlNoteDetectionEnabled(info[0].As<Napi::Boolean>().Value());
|
||||
return info.Env().Undefined();
|
||||
}
|
||||
|
||||
static Napi::Value SetMonitorMuteSuppressed(const Napi::CallbackInfo& info)
|
||||
{
|
||||
// IsBoolean()-guarded so a mismatched renderer build / manual caller
|
||||
@@ -3259,6 +3273,7 @@ static Napi::Object InitModule(Napi::Env env, Napi::Object exports)
|
||||
exports.Set("getSampleRate", Napi::Function::New(env, GetSampleRate));
|
||||
exports.Set("loadNoteModel", Napi::Function::New(env, LoadNoteModel));
|
||||
exports.Set("isMlNoteDetection", Napi::Function::New(env, IsMlNoteDetection));
|
||||
exports.Set("setNoteDetectionEnabled", Napi::Function::New(env, SetNoteDetectionEnabled));
|
||||
exports.Set("detectNotes", Napi::Function::New(env, DetectNotes));
|
||||
|
||||
// VST scanning
|
||||
|
||||
Reference in New Issue
Block a user