mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-09-10 23:04:09 +00:00
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>
This commit is contained in:
co-authored by
Claude Fable 5
parent
eeb83cbdbc
commit
eb40b87dea
@@ -1188,6 +1188,10 @@ void AudioEngine::teardownSplitMode()
|
||||
|
||||
void AudioEngine::startAudio()
|
||||
{
|
||||
// Intent flag first — even if the device open below fails or a transient
|
||||
// stop races in, "the user wants audio" survives (read by phase 8's
|
||||
// setAudioDevices restart fix; see EngineState.h).
|
||||
state.userWantsAudio.store(true, std::memory_order_relaxed);
|
||||
if (audioRunning.load(std::memory_order_relaxed))
|
||||
{
|
||||
fprintf(stderr, "[AudioEngine] startAudio: already running\n");
|
||||
@@ -1244,6 +1248,7 @@ void AudioEngine::startAudio()
|
||||
|
||||
void AudioEngine::stopAudio()
|
||||
{
|
||||
state.userWantsAudio.store(false, std::memory_order_relaxed);
|
||||
if (slopsmith_vst_trace::isEnabled())
|
||||
fprintf(stderr, "[diag] stopAudio: audioRunning=%d inputCbReg=%d outputCbReg=%d\n",
|
||||
(int) audioRunning.load(std::memory_order_relaxed),
|
||||
|
||||
+13
-18
@@ -2,6 +2,7 @@
|
||||
#include "SourceChain.h"
|
||||
#include "GainSanitize.h"
|
||||
#include "engine/PackedStereoRing.h"
|
||||
#include "engine/EngineState.h"
|
||||
#include "BackingLeveler.h"
|
||||
#include "signalsmith-stretch.h"
|
||||
#include <juce_audio_devices/juce_audio_devices.h>
|
||||
@@ -444,7 +445,12 @@ private:
|
||||
// with an SPSC ring between them.
|
||||
juce::AudioDeviceManager inputDeviceManager;
|
||||
juce::AudioDeviceManager outputDeviceManager;
|
||||
std::atomic<bool> duplexMode{true};
|
||||
|
||||
// Shared run-state atomics (TLC phase 1) — the members below are
|
||||
// reference aliases under their historical names so call sites are
|
||||
// untouched; extracted units take `state` (EngineState&) directly.
|
||||
slopsmith::EngineState state;
|
||||
std::atomic<bool>& duplexMode = state.duplexMode;
|
||||
|
||||
// Per-input capture+detect+monitor chains. A FIXED pool, all constructed up
|
||||
// front, so adding/removing a source never reassigns a pointer the audio
|
||||
@@ -538,23 +544,12 @@ private:
|
||||
std::atomic<bool> backingSpeedChangePending{false};
|
||||
juce::CriticalSection backingLock;
|
||||
|
||||
// Toggled from startAudio()/stopAudio() (main / device-management
|
||||
// threads) and read from isAudioRunning() on the JS thread via the
|
||||
// audio-bridge dispatch loop. Plain bool would be a data race;
|
||||
// relaxed-atomic is well-defined and compiles to a plain MOV.
|
||||
std::atomic<bool> audioRunning{false};
|
||||
// Sample rate is written from the JUCE device callbacks (audio
|
||||
// thread / device-management thread) and read from arbitrary
|
||||
// callers including the JS thread via getCurrentSampleRate(),
|
||||
// so a plain double would be a C++ data race. std::atomic<double>
|
||||
// is well-defined and lock-free on the platforms we ship; the
|
||||
// hot reads use relaxed since the consumer just wants the latest
|
||||
// observable value, not a synchronization point.
|
||||
std::atomic<double> currentSampleRate{48000.0};
|
||||
// Split mode allows different input vs output block sizes; the ring absorbs
|
||||
// the asymmetry. DSP prepares against input; backing resampler against output.
|
||||
std::atomic<int> inputBlockSize{256};
|
||||
std::atomic<int> outputBlockSize{256};
|
||||
// audioRunning keeps its historical DEVICE-STATE semantics (isAudioRunning
|
||||
// compat pin); the intent half is state.userWantsAudio — see EngineState.h.
|
||||
std::atomic<bool>& audioRunning = state.deviceRunning;
|
||||
std::atomic<double>& currentSampleRate = state.currentSampleRate;
|
||||
std::atomic<int>& inputBlockSize = state.inputBlockSize;
|
||||
std::atomic<int>& outputBlockSize = state.outputBlockSize;
|
||||
|
||||
// The per-input lock-free SPSC rings (pre-gate getInputFrame ring + post-gate
|
||||
// getRawAudioFrame ring), the YIN/ML detectors, and the zero-output capture
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
#pragma once
|
||||
|
||||
// EngineState — the audio engine's shared run-state atomics (TLC plan
|
||||
// phase 1 / §2.8). Every extracted engine unit takes an EngineState& instead
|
||||
// of reaching back into AudioEngine, which is what keeps them unit-testable
|
||||
// without JUCE devices. AudioEngine itself binds these members by reference
|
||||
// under their historical names, so existing call sites are untouched.
|
||||
//
|
||||
// The deliberate fix homed here (deep-read §3/§6): the old single
|
||||
// `audioRunning` flag conflated USER INTENT ("the user pressed Start") with
|
||||
// DEVICE STATE ("a device callback is live") — audioDeviceStopped() clears it
|
||||
// on transient stops (WASAPI exclusive opens routinely fire one mid-start),
|
||||
// so setAudioDevices' restart decision read a racy answer. The two are now
|
||||
// separate atomics:
|
||||
//
|
||||
// userWantsAudio — intent. Written ONLY by startAudio()/stopAudio().
|
||||
// deviceRunning — state. Written by startAudio()/stopAudio() AND the
|
||||
// device callbacks (aboutToStart/stopped), i.e. the exact
|
||||
// semantics the old audioRunning had. isAudioRunning()
|
||||
// keeps reporting THIS one (Phase 0.b compat pin).
|
||||
//
|
||||
// Until the phase-8 fix, nothing reads userWantsAudio — writing it here first
|
||||
// keeps that later commit a one-line read-side change in setAudioDevices.
|
||||
|
||||
#include <atomic>
|
||||
|
||||
namespace slopsmith {
|
||||
|
||||
struct EngineState
|
||||
{
|
||||
// Sample rate is written from the JUCE device callbacks (audio thread /
|
||||
// device-management thread) and read from arbitrary callers including the
|
||||
// JS thread, so a plain double would be a C++ data race. atomic<double>
|
||||
// is lock-free on the platforms we ship; hot reads use relaxed since the
|
||||
// consumer just wants the latest observable value, not a sync point.
|
||||
std::atomic<double> currentSampleRate{48000.0};
|
||||
// Split mode allows different input vs output block sizes; the ring
|
||||
// absorbs the asymmetry. DSP prepares against input; backing resampler
|
||||
// against output.
|
||||
std::atomic<int> inputBlockSize{256};
|
||||
std::atomic<int> outputBlockSize{256};
|
||||
// Duplex: one device manager owns both directions. Split: input-only +
|
||||
// output-only managers with an SPSC ring between them.
|
||||
std::atomic<bool> duplexMode{true};
|
||||
|
||||
// Intent: the user asked for audio to run. start/stopAudio only.
|
||||
std::atomic<bool> userWantsAudio{false};
|
||||
// State: toggled from startAudio()/stopAudio() (main/device-management
|
||||
// threads) and the device callbacks, read from isAudioRunning() on the JS
|
||||
// thread. Plain bool would be a data race; relaxed-atomic compiles to a
|
||||
// plain MOV.
|
||||
std::atomic<bool> deviceRunning{false};
|
||||
};
|
||||
|
||||
} // namespace slopsmith
|
||||
@@ -12,3 +12,7 @@ target_compile_features(packed_stereo_ring_test PRIVATE cxx_std_20)
|
||||
find_package(Threads REQUIRED)
|
||||
target_link_libraries(packed_stereo_ring_test PRIVATE Threads::Threads)
|
||||
add_test(NAME packed_stereo_ring COMMAND packed_stereo_ring_test)
|
||||
|
||||
add_executable(engine_state_test engine_state_test.cpp)
|
||||
target_compile_features(engine_state_test PRIVATE cxx_std_17)
|
||||
add_test(NAME engine_state COMMAND engine_state_test)
|
||||
|
||||
@@ -0,0 +1,53 @@
|
||||
// Phase 1 unit test for EngineState (docs/audio-engine-tlc.md §5): the
|
||||
// intent/state transition table. Mirrors how AudioEngine drives the two
|
||||
// flags — startAudio/stopAudio write BOTH (intent + state), the device
|
||||
// callbacks write deviceRunning ONLY — and pins the Phase 0.b compat
|
||||
// decision that isAudioRunning() reports DEVICE STATE: a transient device
|
||||
// stop flips it false even though the user never pressed Stop.
|
||||
|
||||
#include "../../src/audio/engine/EngineState.h"
|
||||
|
||||
#include <cassert>
|
||||
#include <cstdio>
|
||||
|
||||
using slopsmith::EngineState;
|
||||
|
||||
// The write sets, as AudioEngine performs them.
|
||||
static void userStart(EngineState& s) { s.userWantsAudio.store(true); s.deviceRunning.store(true); }
|
||||
static void userStop(EngineState& s) { s.userWantsAudio.store(false); s.deviceRunning.store(false); }
|
||||
static void deviceAboutToStart(EngineState& s) { s.deviceRunning.store(true); }
|
||||
static void deviceStopped(EngineState& s) { s.deviceRunning.store(false); }
|
||||
// isAudioRunning() facade == deviceRunning (compat pin).
|
||||
static bool isAudioRunning(const EngineState& s) { return s.deviceRunning.load(); }
|
||||
|
||||
int main()
|
||||
{
|
||||
EngineState s;
|
||||
assert(!s.userWantsAudio.load() && !isAudioRunning(s));
|
||||
|
||||
// User starts audio.
|
||||
userStart(s);
|
||||
assert(s.userWantsAudio.load() && isAudioRunning(s));
|
||||
|
||||
// Transient device stop (WASAPI exclusive mid-start hiccup): device state
|
||||
// drops, intent survives — this is the split that fixes deep-read §3.
|
||||
deviceStopped(s);
|
||||
assert(s.userWantsAudio.load() && "transient stop must not erase user intent");
|
||||
assert(!isAudioRunning(s) && "compat pin: isAudioRunning reports device state");
|
||||
|
||||
// JUCE auto-restart brings the device back without user action.
|
||||
deviceAboutToStart(s);
|
||||
assert(s.userWantsAudio.load() && isAudioRunning(s));
|
||||
|
||||
// Explicit user stop clears both.
|
||||
userStop(s);
|
||||
assert(!s.userWantsAudio.load() && !isAudioRunning(s));
|
||||
|
||||
// A stray device start (auto-restart after user stop) must not fabricate
|
||||
// intent: device state true, intent still false.
|
||||
deviceAboutToStart(s);
|
||||
assert(!s.userWantsAudio.load() && isAudioRunning(s));
|
||||
|
||||
std::puts("engine_state: all transitions passed");
|
||||
return 0;
|
||||
}
|
||||
Reference in New Issue
Block a user