mirror of
https://github.com/got-feedBack/feedBack-desktop.git
synced 2026-09-10 22:14:10 +00:00
fix(audio): renderer-bus flush flag + reconfigure reads user intent (phase 8)
Two deep-read fixes now homed in their phase-1/2 units:
RendererBus (§4): setEnabled(false) no longer writes readIndex from the
control thread — the ring's designated consumer-side writer is pull(). The
drop-on-disable is now a flushRequested atomic the consumer honors at its
next pull, closing the last SPSC-discipline hole (a concurrent pull
mid-drain could overwrite the control thread's store and replay a stale
tail after re-enable). New unit test pins flush-then-fresh-audio.
setAudioDevices (§3): the restart decision reads state.userWantsAudio
(intent, written only by start/stopAudio) instead of the racy device-state
flag that transient audioDeviceStopped() fires clear — a reconfigure landing
inside a transient-stop window no longer leaves the engine configured but
stopped ('no audio until Start/Apply is pressed again').
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Fable 5
parent
db337eaf29
commit
dd40b2f227
@@ -388,7 +388,15 @@ AudioEngine::DeviceConfigResult AudioEngine::setAudioDevices(const DeviceConfig&
|
||||
// stale output callback attached. stopAudio() is itself idempotent
|
||||
// (R9 fix — removeAudioCallback is a no-op when not registered), so
|
||||
// running it unconditionally is safe regardless of audioRunning.
|
||||
const bool wasRunning = audioRunning.load(std::memory_order_relaxed);
|
||||
// Read USER INTENT, not device state (deep-read §3 fix): audioRunning
|
||||
// (deviceRunning) is cleared by transient audioDeviceStopped() fires —
|
||||
// WASAPI exclusive opens routinely fire one mid-start — so a reconfigure
|
||||
// racing that window used to see false and leave the engine configured
|
||||
// but stopped ("no audio until Start/Apply is pressed again").
|
||||
// userWantsAudio is written only by start/stopAudio, so it answers the
|
||||
// question this restart decision actually asks. NOTE: stopAudio() below
|
||||
// clears the intent flag, hence the capture BEFORE it.
|
||||
const bool wasRunning = state.userWantsAudio.load(std::memory_order_relaxed);
|
||||
|
||||
// stopAudio() closes every extra input device but KEEPS its desiredDeviceName;
|
||||
// the startAudio() below re-opens them at the new config (so panels using a
|
||||
|
||||
@@ -42,12 +42,13 @@ public:
|
||||
if (was && !enabled)
|
||||
{
|
||||
// Drop buffered audio on disable so a later re-enable starts fresh
|
||||
// instead of playing a stale tail. Consumer tolerates the jump.
|
||||
// KNOWN ISSUE (deep-read §4, fixed in the follow-up commit): this
|
||||
// writes readIndex from the control thread while pull() is the
|
||||
// designated consumer-side writer.
|
||||
ring.readIndex.store(ring.writeIndex.load(std::memory_order_acquire),
|
||||
std::memory_order_release);
|
||||
// instead of playing a stale tail. The CONSUMER honors this flag at
|
||||
// its next pull (deep-read §4 fix): the old control-thread write to
|
||||
// readIndex violated the ring's own SPSC discipline — a concurrent
|
||||
// pull mid-drain could overwrite it with r + pull, replaying a
|
||||
// stale tail after re-enable, exactly what the drop was meant to
|
||||
// prevent. Only the consumer ever moves readIndex now.
|
||||
flushRequested.store(true, std::memory_order_release);
|
||||
primed.store(false, std::memory_order_relaxed);
|
||||
}
|
||||
}
|
||||
@@ -105,6 +106,11 @@ public:
|
||||
// call exactly once per output block.
|
||||
int pull(float* dl, float* dr, int numSamples)
|
||||
{
|
||||
// Consume a pending flush FIRST — even while disabled — so the tail
|
||||
// buffered before a disable is dropped by the ring's one legitimate
|
||||
// readIndex writer (this consumer), never by the control thread.
|
||||
if (flushRequested.exchange(false, std::memory_order_acq_rel))
|
||||
ring.commitRead(ring.writeIndex.load(std::memory_order_acquire));
|
||||
if (!busEnabled.load(std::memory_order_acquire)) return 0;
|
||||
const uint64_t w = ring.writeIndex.load(std::memory_order_acquire);
|
||||
uint64_t r = ring.readIndex.load(std::memory_order_relaxed);
|
||||
@@ -196,6 +202,9 @@ private:
|
||||
// Consumer-side prefill-gate state. Only the live output callback touches
|
||||
// it, but duplex/split hand-offs cross threads — atomic keeps that safe.
|
||||
std::atomic<bool> primed{false};
|
||||
// Set by setEnabled(false) on the control thread, consumed (exchange) by
|
||||
// pull() — the drop-on-disable request, honored by the single consumer.
|
||||
std::atomic<bool> flushRequested{false};
|
||||
// Producer-thread-only linear-resampler state (fractional read position
|
||||
// into the incoming chunk + the previous chunk's last frame for
|
||||
// interpolation continuity across pushes).
|
||||
|
||||
@@ -141,6 +141,30 @@ static void testGainApplied()
|
||||
assert(dl[0] == 2.0f && dr[0] == -2.0f);
|
||||
}
|
||||
|
||||
// Disable drops the buffered tail — via the consumer-honored flush flag
|
||||
// (deep-read §4 fix), so a re-enable never replays stale audio.
|
||||
static void testFlushOnDisable()
|
||||
{
|
||||
RendererBus bus;
|
||||
bus.setEnabled(true, 1.0f);
|
||||
const auto chunk = rampChunk(RendererBus::kPrimeFrames * 2, 5.0f, 0.0f);
|
||||
bus.push(chunk.data(), RendererBus::kPrimeFrames * 2, 48000.0, 48000.0);
|
||||
bus.setEnabled(false, 1.0f); // requests the flush; consumer performs it
|
||||
bus.setEnabled(true, 1.0f);
|
||||
std::vector<float> dl(64), dr(64);
|
||||
// First pull consumes the flush: the pre-disable tail is gone, so the bus
|
||||
// is empty and (re-)priming — nothing plays.
|
||||
assert(bus.pull(dl.data(), dr.data(), 64) == 0 && "stale tail must not replay");
|
||||
assert(bus.metrics().fillFrames == 0 && "flush must drop the buffered tail");
|
||||
// Fresh audio after the re-enable flows once primed.
|
||||
const auto fresh = rampChunk(RendererBus::kPrimeFrames + 65, 7.0f, 0.0f);
|
||||
bus.push(fresh.data(), RendererBus::kPrimeFrames + 65, 48000.0, 48000.0);
|
||||
assert(bus.pull(dl.data(), dr.data(), 64) == 64);
|
||||
// Frame 0 is the resampler's one-frame interpolation carry (by design);
|
||||
// everything after must be the fresh push, not the flushed 5.0 tail.
|
||||
assert(dl[1] == 7.0f && "post-re-enable audio must be the fresh push");
|
||||
}
|
||||
|
||||
int main()
|
||||
{
|
||||
testEqualRateBitExact();
|
||||
@@ -150,6 +174,7 @@ int main()
|
||||
testFillClampTrimsBacklog();
|
||||
testDisabledIsInert();
|
||||
testGainApplied();
|
||||
testFlushOnDisable();
|
||||
std::puts("renderer_bus: all cases passed");
|
||||
return 0;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user