From 16686389fda677a23d0b514b3b8eef319f8a9508 Mon Sep 17 00:00:00 2001 From: OmikronApex Date: Fri, 17 Jul 2026 22:20:03 +0200 Subject: [PATCH] fix(audio): gate strict buffer-size verification to ASIO; review cleanups Post-open verification previously failed closed on any buffer-size delta on every backend. The wedge this guards against (a driver accepting a request without changing its buffer, then blocking the next in-place request) is ASIO behaviour; ALSA rounds requests to period constraints and CoreAudio can clamp, and both previously worked by storing the driver-adjusted actuals. Keep exact buffer equality for ASIO only; other backends log and accept the adjusted size. Rate and channel-mask verification stay strict everywhere. Also from review: print the real `options.compatible` in the live-probe log instead of a hard-coded 1, and document that the live-endpoint reuse in probeDual is safe only under runDeviceLifecycleOp's message-thread serialisation (PR #113). Co-Authored-By: Claude Fable 5 --- src/audio/engine/DeviceSetup.cpp | 30 +++++++++++++++++++++++++--- tests/device-setup-lifecycle.test.js | 11 ++++++++++ 2 files changed, 38 insertions(+), 3 deletions(-) diff --git a/src/audio/engine/DeviceSetup.cpp b/src/audio/engine/DeviceSetup.cpp index b806069..65e11b0 100644 --- a/src/audio/engine/DeviceSetup.cpp +++ b/src/audio/engine/DeviceSetup.cpp @@ -138,6 +138,11 @@ DeviceOptions DeviceSetup::probeDual(const juce::String& inputTypeName, // the live device's immutable capability lists instead. The // endpoint checks keep a newly selected device on the normal // temporary-probe path. + // + // Reading the live device here is safe only because probes and + // device mutations are serialised on the JUCE message thread + // (runDeviceLifecycleOp, PR #113) — a caller probing off-thread + // would race applyDuplex's close/reopen. auto* liveDevice = inMgr.getCurrentAudioDevice(); auto* liveType = inMgr.getCurrentDeviceTypeObject(); const auto liveSetup = inMgr.getAudioDeviceSetup(); @@ -160,11 +165,12 @@ DeviceOptions DeviceSetup::probeDual(const juce::String& inputTypeName, fprintf(stderr, "[AudioEngine] Probed live device options: " "inType='%s' outType='%s' in='%s' out='%s' " - "inputs=%d outputs=%d rates=%d buffers=%d compatible=1\n", + "inputs=%d outputs=%d rates=%d buffers=%d compatible=%d\n", options.inputType.toRawUTF8(), options.outputType.toRawUTF8(), options.input.toRawUTF8(), options.output.toRawUTF8(), options.inputChannels.size(), options.outputChannels.size(), - options.sampleRates.size(), options.bufferSizes.size()); + options.sampleRates.size(), options.bufferSizes.size(), + (int) options.compatible); return options; } @@ -477,8 +483,26 @@ juce::String DeviceSetup::applyDuplex(const juce::String& inputName, actualInputs.toString(2).toRawUTF8(), actualOutputs.toString(2).toRawUTF8()); + // Buffer-size strictness is ASIO-only. The Helix regression this + // gate exists for (success reported at the old buffer size, next + // request wedging the message thread) is an ASIO driver behaviour; + // ALSA rounds requests to period-size constraints and CoreAudio can + // clamp, and both previously worked by storing the driver-adjusted + // actuals. Failing those closed would turn a working driver-rounded + // 480-for-512 open into "no audio". Rate and channel-mask + // verification stay strict on every backend. + juce::String configuredTypeName; + if (auto* configuredType = inMgr.getCurrentDeviceTypeObject()) + configuredTypeName = configuredType->getTypeName(); + const bool strictBufferSize = (configuredTypeName == "ASIO"); + if (!strictBufferSize && bs != setup.bufferSize) + fprintf(stderr, "[AudioEngine] Duplex reconfigure phase=verify " + "accepting driver-adjusted buffer size %d (requested %d, type='%s')\n", + bs, setup.bufferSize, configuredTypeName.toRawUTF8()); + switch (validateOpenedDeviceFormat( - setup.sampleRate, setup.bufferSize, sr, bs, + setup.sampleRate, + strictBufferSize ? setup.bufferSize : bs, sr, bs, inputChannelsMatch, outputChannelsMatch)) { case DeviceFormatMismatch::sampleRate: diff --git a/tests/device-setup-lifecycle.test.js b/tests/device-setup-lifecycle.test.js index 6a3eb79..d1c6d68 100644 --- a/tests/device-setup-lifecycle.test.js +++ b/tests/device-setup-lifecycle.test.js @@ -68,6 +68,17 @@ test('duplex success is gated on actual rate, buffer, and active channel masks', assert.match(applyDuplex, /validateOpenedDeviceFormat\(/); }); +test('strict buffer-size verification is gated to ASIO; other backends accept driver-adjusted', () => { + // ALSA rounds to period constraints and CoreAudio can clamp; both must + // keep their pre-#114 store-the-actuals behaviour instead of failing + // closed on a working driver-rounded open. + assert.match(applyDuplex, + /const bool strictBufferSize = \(configuredTypeName == "ASIO"\);/); + assert.match(applyDuplex, + /strictBufferSize \? setup\.bufferSize : bs/); + assert.match(applyDuplex, /accepting driver-adjusted buffer size/); +}); + 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);