diff --git a/src/audio/engine/DeviceSetup.cpp b/src/audio/engine/DeviceSetup.cpp index 07160fd..72d5663 100644 --- a/src/audio/engine/DeviceSetup.cpp +++ b/src/audio/engine/DeviceSetup.cpp @@ -252,6 +252,27 @@ juce::String DeviceSetup::applyDuplex(const juce::String& inputName, setup.useDefaultInputChannels = inputName.isEmpty(); setup.useDefaultOutputChannels = outputName.isEmpty(); + // Every unsuccessful reconfiguration must leave the manager and the + // externally readable engine format in one truthful state: closed/zero. + // In particular, never keep a stale ASIO device or the previous 256-sample + // state alive after a failed request for 512. + auto failClosed = [&](const juce::String& error) -> juce::String { + fprintf(stderr, "[AudioEngine] Duplex reconfigure failed: %s; closing device\n", + error.toRawUTF8()); + try { inMgr.closeAudioDevice(); } + catch (...) { + fprintf(stderr, "[AudioEngine] Duplex failure cleanup: closeAudioDevice threw\n"); + } + state.currentSampleRate.store(0.0, std::memory_order_relaxed); + state.inputBlockSize.store(0, std::memory_order_relaxed); + state.outputBlockSize.store(0, std::memory_order_relaxed); + try { monitorChain.releaseMonitorChain(); } + catch (...) { + fprintf(stderr, "[AudioEngine] Duplex failure cleanup: monitor release threw\n"); + } + return error; + }; + // Channel masks must match too — high-numbered selectedInputChannel needs // the expanded mask that an older session may not have opened. if (auto* currentDevice = inMgr.getCurrentAudioDevice()) @@ -277,6 +298,7 @@ juce::String DeviceSetup::applyDuplex(const juce::String& inputName, && current.useDefaultOutputChannels == setup.useDefaultOutputChannels && current.inputChannels == expectedInputs && current.outputChannels == expectedOutputs + && currentDevice->isOpen() && state.duplexMode.load(std::memory_order_relaxed)) { fprintf(stderr, "[AudioEngine] Duplex device already configured with same settings, skipping\n"); @@ -293,29 +315,45 @@ juce::String DeviceSetup::applyDuplex(const juce::String& inputName, } } - // ALSA deadlocks on reconfigure unless we fully close first. WASAPI - // reconfigures in place and is much slower if closed. -#if JUCE_LINUX + // ALSA and Windows ASIO both need a full close before reconfiguration. + // The Helix driver was observed accepting a first request without changing + // its buffer, then wedging JUCE's message thread on the next in-place + // request. Close BEFORE the temporary channel probe too: constructing an + // ASIO device initialises the driver and briefly starts dummy buffers, so a + // probe must never overlap the live primary instance. WASAPI remains + // in-place because closing it is materially slower and this failure mode is + // specific to ASIO. juce::String currentTypeName; if (auto* currentType = inMgr.getCurrentDeviceTypeObject()) currentTypeName = currentType->getTypeName(); - if (inMgr.getCurrentAudioDevice() != nullptr) + + bool closeBeforeReconfigure = false; +#if JUCE_LINUX + closeBeforeReconfigure = true; +#elif JUCE_WINDOWS + closeBeforeReconfigure = (currentTypeName == "ASIO"); +#endif + + if (closeBeforeReconfigure && inMgr.getCurrentAudioDevice() != nullptr) { + fprintf(stderr, "[AudioEngine] Duplex reconfigure phase=close begin type='%s'\n", + currentTypeName.toRawUTF8()); try { inMgr.closeAudioDevice(); - fprintf(stderr, "[AudioEngine] Closed device for reconfiguration\n"); - if (currentTypeName.isNotEmpty()) - inMgr.setCurrentAudioDeviceType(currentTypeName, true); + // AudioDeviceManager::closeAudioDevice() preserves its current + // device type/setup. setAudioDeviceSetup() below sees a null + // device and creates a fresh instance of that same type. } catch (...) { - fprintf(stderr, "[AudioEngine] closeAudioDevice crashed, continuing\n"); + return failClosed("closeAudioDevice threw before reconfiguration"); } + fprintf(stderr, "[AudioEngine] Duplex reconfigure phase=close complete\n"); } -#endif int inputChannelCount = 0; int outputChannelCount = 0; if (auto* type = inMgr.getCurrentDeviceTypeObject()) { + fprintf(stderr, "[AudioEngine] Duplex reconfigure phase=probe begin\n"); try { if (auto probe = std::unique_ptr(type->createDevice(outputName, inputName))) @@ -332,6 +370,8 @@ juce::String DeviceSetup::applyDuplex(const juce::String& inputName, { fprintf(stderr, "[AudioEngine] Channel probe failed (unknown)\n"); } + fprintf(stderr, "[AudioEngine] Duplex reconfigure phase=probe complete inputs=%d outputs=%d\n", + inputChannelCount, outputChannelCount); } if (inputChannelCount <= 0) inputChannelCount = 2; if (outputChannelCount <= 0) outputChannelCount = 2; @@ -340,27 +380,93 @@ juce::String DeviceSetup::applyDuplex(const juce::String& inputName, setup.outputChannels.setRange(0, juce::jmin(outputChannelCount, 2), true); juce::String result; + fprintf(stderr, "[AudioEngine] Duplex reconfigure phase=open begin sr=%.0f bs=%d\n", + setup.sampleRate, setup.bufferSize); try { result = inMgr.setAudioDeviceSetup(setup, true); } catch (...) { - return "setAudioDeviceSetup threw"; + return failClosed("setAudioDeviceSetup threw"); } + fprintf(stderr, "[AudioEngine] Duplex reconfigure phase=open complete error='%s'\n", + result.toRawUTF8()); if (result.isNotEmpty()) { - fprintf(stderr, "[AudioEngine] Device setup error: %s\n", result.toRawUTF8()); - try { - result = inMgr.initialiseWithDefaultDevices(2, 2); - } catch (...) { - return "fallback initialiseWithDefaultDevices threw"; - } - if (result.isNotEmpty()) - return "device setup failed: " + result; + // A default-device fallback used to convert this failure into success, + // leaving only two channels active while the UI saved the requested + // ASIO device. Preserve the original error and stay closed instead. + return failClosed("device setup failed: " + result); } if (auto* configuredDevice = inMgr.getCurrentAudioDevice()) { + if (!configuredDevice->isOpen()) + return failClosed("device is not open after setup"); + const double sr = configuredDevice->getCurrentSampleRate(); const int bs = configuredDevice->getCurrentBufferSizeSamples(); + + // For explicitly named endpoints, "all inputs / first two outputs" is + // the requested contract. Rebuild those masks from the opened device's + // advertised channels so a failed pre-open probe cannot silently + // collapse an 8-input ASIO interface to the old two-channel fallback. + juce::BigInteger expectedInputs = setup.inputChannels; + if (!setup.useDefaultInputChannels) + { + expectedInputs.clear(); + expectedInputs.setRange( + 0, configuredDevice->getInputChannelNames().size(), true); + } + juce::BigInteger expectedOutputs = setup.outputChannels; + if (!setup.useDefaultOutputChannels) + { + expectedOutputs.clear(); + expectedOutputs.setRange( + 0, juce::jmin(configuredDevice->getOutputChannelNames().size(), 2), true); + } + + const auto actualInputs = configuredDevice->getActiveInputChannels(); + const auto actualOutputs = configuredDevice->getActiveOutputChannels(); + const bool inputChannelsMatch = + setup.useDefaultInputChannels || actualInputs == expectedInputs; + const bool outputChannelsMatch = + setup.useDefaultOutputChannels || actualOutputs == expectedOutputs; + + fprintf(stderr, + "[AudioEngine] Duplex reconfigure phase=verify requested(sr=%.0f bs=%d in=%s out=%s) " + "actual(sr=%.0f bs=%d in=%s out=%s)\n", + setup.sampleRate, setup.bufferSize, + expectedInputs.toString(2).toRawUTF8(), + expectedOutputs.toString(2).toRawUTF8(), + sr, bs, + actualInputs.toString(2).toRawUTF8(), + actualOutputs.toString(2).toRawUTF8()); + + switch (validateOpenedDeviceFormat( + setup.sampleRate, setup.bufferSize, sr, bs, + inputChannelsMatch, outputChannelsMatch)) + { + case DeviceFormatMismatch::sampleRate: + return failClosed( + "device opened at sample rate " + juce::String(sr) + + " (requested " + juce::String(setup.sampleRate) + ")"); + case DeviceFormatMismatch::bufferSize: + return failClosed( + "device opened at buffer size " + juce::String(bs) + + " (requested " + juce::String(setup.bufferSize) + ")"); + case DeviceFormatMismatch::inputChannels: + return failClosed( + "device opened with input channel mask " + + actualInputs.toString(2) + " (requested " + + expectedInputs.toString(2) + ")"); + case DeviceFormatMismatch::outputChannels: + return failClosed( + "device opened with output channel mask " + + actualOutputs.toString(2) + " (requested " + + expectedOutputs.toString(2) + ")"); + case DeviceFormatMismatch::none: + break; + } + state.currentSampleRate.store(sr, std::memory_order_relaxed); state.inputBlockSize.store(bs, std::memory_order_relaxed); state.outputBlockSize.store(bs, std::memory_order_relaxed); @@ -373,11 +479,7 @@ juce::String DeviceSetup::applyDuplex(const juce::String& inputName, monitorChain.prepareMonitorChain(sr, bs); return {}; } - state.currentSampleRate.store(0.0, std::memory_order_relaxed); - state.inputBlockSize.store(0, std::memory_order_relaxed); - state.outputBlockSize.store(0, std::memory_order_relaxed); - monitorChain.releaseMonitorChain(); - return "no current device after setup"; + return failClosed("no current device after setup"); } DeviceConfigResult DeviceSetup::applySplit(const DeviceConfig& config, diff --git a/src/audio/engine/RateMatch.h b/src/audio/engine/RateMatch.h index 7e11406..40595ed 100644 --- a/src/audio/engine/RateMatch.h +++ b/src/audio/engine/RateMatch.h @@ -30,4 +30,38 @@ inline bool nominalRateCandidate(double r, double r2, double& candidate) noexcep return ratesMatch(r, candidate) && ratesMatch(r2, candidate); } +// Post-open verification result shared by device setup. JUCE drivers may +// choose a "best" format when the exact request is unavailable; that is useful +// for generic callers, but this app must not report success for a different +// buffer/rate/channel mask than the user selected. Channel equality is +// computed by the JUCE-facing caller and passed as booleans so this decision +// table stays JUCE-free and unit-testable. +enum class DeviceFormatMismatch +{ + none, + sampleRate, + bufferSize, + inputChannels, + outputChannels, +}; + +inline DeviceFormatMismatch validateOpenedDeviceFormat( + double requestedSampleRate, + int requestedBufferSize, + double actualSampleRate, + int actualBufferSize, + bool inputChannelsMatch, + bool outputChannelsMatch) noexcept +{ + if (!ratesMatch(requestedSampleRate, actualSampleRate)) + return DeviceFormatMismatch::sampleRate; + if (requestedBufferSize != actualBufferSize) + return DeviceFormatMismatch::bufferSize; + if (!inputChannelsMatch) + return DeviceFormatMismatch::inputChannels; + if (!outputChannelsMatch) + return DeviceFormatMismatch::outputChannels; + return DeviceFormatMismatch::none; +} + } // namespace slopsmith diff --git a/tests/device-setup-lifecycle.test.js b/tests/device-setup-lifecycle.test.js new file mode 100644 index 0000000..410b054 --- /dev/null +++ b/tests/device-setup-lifecycle.test.js @@ -0,0 +1,58 @@ +'use strict'; + +// Source-level lifecycle contract for the hardware-dependent half of the ASIO +// repair. The pure format decision table is covered by rate_match_test.cpp; +// these assertions pin the ordering that cannot be exercised without loading +// a real Windows ASIO driver in CI. + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('node:fs'); +const path = require('node:path'); + +const source = fs.readFileSync( + path.join(__dirname, '..', 'src', 'audio', 'engine', 'DeviceSetup.cpp'), + 'utf8').replace(/\r\n/g, '\n'); + +const start = source.indexOf('juce::String DeviceSetup::applyDuplex'); +const end = source.indexOf('DeviceConfigResult DeviceSetup::applySplit', start); +const applyDuplex = source.slice(start, end); + +test('duplex setup closes Windows ASIO before constructing its channel probe', () => { + assert.ok(start >= 0 && end > start, 'could not locate applyDuplex'); + assert.match( + applyDuplex, + /#elif JUCE_WINDOWS\s+closeBeforeReconfigure = \(currentTypeName == "ASIO"\);/); + + const guardedClose = applyDuplex.indexOf( + 'if (closeBeforeReconfigure && inMgr.getCurrentAudioDevice() != nullptr)'); + const close = applyDuplex.indexOf('inMgr.closeAudioDevice();', guardedClose); + const probe = applyDuplex.indexOf('type->createDevice(outputName, inputName)'); + assert.ok(guardedClose >= 0 && close > guardedClose && probe > close, + 'the live ASIO device must be closed before a temporary probe is created'); +}); + +test('duplex setup never converts requested-device failure into default-device success', () => { + assert.doesNotMatch(applyDuplex, /initialiseWithDefaultDevices/); + assert.match(applyDuplex, /return failClosed\("device setup failed: " \+ result\);/); +}); + +test('duplex success is gated on actual rate, buffer, and active channel masks', () => { + assert.match(applyDuplex, /getCurrentSampleRate\(\)/); + assert.match(applyDuplex, /getCurrentBufferSizeSamples\(\)/); + assert.match(applyDuplex, /getActiveInputChannels\(\)/); + assert.match(applyDuplex, /getActiveOutputChannels\(\)/); + assert.match(applyDuplex, /validateOpenedDeviceFormat\(/); +}); + +test('duplex failures clear observable format state and release monitor resources', () => { + const cleanupStart = applyDuplex.indexOf('auto failClosed ='); + const cleanupEnd = applyDuplex.indexOf('// Channel masks must match too', cleanupStart); + const cleanup = applyDuplex.slice(cleanupStart, cleanupEnd); + + assert.match(cleanup, /inMgr\.closeAudioDevice\(\)/); + assert.match(cleanup, /currentSampleRate\.store\(0\.0/); + assert.match(cleanup, /inputBlockSize\.store\(0/); + assert.match(cleanup, /outputBlockSize\.store\(0/); + assert.match(cleanup, /monitorChain\.releaseMonitorChain\(\)/); +}); diff --git a/tests/engine_units/rate_match_test.cpp b/tests/engine_units/rate_match_test.cpp index e9c9293..869575d 100644 --- a/tests/engine_units/rate_match_test.cpp +++ b/tests/engine_units/rate_match_test.cpp @@ -10,6 +10,8 @@ using slopsmith::ratesMatch; using slopsmith::nominalRateCandidate; +using slopsmith::DeviceFormatMismatch; +using slopsmith::validateOpenedDeviceFormat; int main() { @@ -41,6 +43,20 @@ int main() // Non-matching pair → no candidate at all. assert(!nominalRateCandidate(44100.0, 48000.0, c)); + // A device setup is successful only when the driver accepted every + // requested format field. This pins the Helix regression where JUCE + // returned success for a 512 request while the driver remained at 256. + assert(validateOpenedDeviceFormat(48000.0, 512, 48000.0, 512, true, true) + == DeviceFormatMismatch::none); + assert(validateOpenedDeviceFormat(48000.0, 512, 44100.0, 512, true, true) + == DeviceFormatMismatch::sampleRate); + assert(validateOpenedDeviceFormat(48000.0, 512, 48000.0, 256, true, true) + == DeviceFormatMismatch::bufferSize); + assert(validateOpenedDeviceFormat(48000.0, 512, 48000.0, 512, false, true) + == DeviceFormatMismatch::inputChannels); + assert(validateOpenedDeviceFormat(48000.0, 512, 48000.0, 512, true, false) + == DeviceFormatMismatch::outputChannels); + std::puts("rate_match: all cases passed"); return 0; }