diff --git a/CHANGELOG.md b/CHANGELOG.md index 88a221ce..aaf43d56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,4 +1,5 @@ ##### 4.1.7 (X Xxx 2026) +- fix: a device change that still fails now reports `SoLoudAudioDeviceFailedToStartCppException` instead of hanging. Thanks to @Colton127 - fix: `changeDevice()` now selects the system default device when called without an argument and reports device-change failures instead of silently succeeding. Thanks to @Colton127 #532 - fix: `init()` no longer blocks the UI thread while the audio device starts. On Android a slow or busy audio HAL could stall the platform thread long enough for the app to be reported as not responding; engine startup and teardown now run on a short-lived worker isolate. Thanks to @Colton127 #481 - added `deinitAsync()`, a non-blocking counterpart to `deinit()`. `deinit()` is unchanged and still supported, but it can stall the UI thread when it lands while `init()` is still starting the device — prefer `deinitAsync()` in new code. diff --git a/example/tests/tests/playback_devices.dart b/example/tests/tests/playback_devices.dart index f38955c7..9aaec447 100644 --- a/example/tests/tests/playback_devices.dart +++ b/example/tests/tests/playback_devices.dart @@ -5,6 +5,18 @@ import 'package:flutter_soloud/src/enums.dart'; import 'common.dart'; +/// Wall-clock budget for a single `changeDevice()` call. +/// +/// A swap closes one stream and opens another: tens of milliseconds in +/// practice, a few hundred on a slow emulator. What matters is the number this +/// excludes. Holding SoLoud's audio-thread mutex across the swap starves the +/// audio callback, and on AAudio's legacy (non-MMAP) path a stream only reports +/// STARTED once its first data callback has run — so `ma_device_start()` sits +/// in `AAudioStream_waitForStateChange()` for its full 5s timeout before it +/// fails. +/// Anything near that is the mutex regression, not a slow device. +const _changeDeviceBudget = Duration(seconds: 2); + /// Test playback device enumeration and switching. Future testPlaybackDevices() async { final strBuf = OutputBuffer(); @@ -38,22 +50,29 @@ Future testPlaybackDevices() async { // device count. // Note: this is also the first time this path is reachable on Web, where it // tears down and re-initializes the WebAudio device. - SoLoud.instance.changeDevice(); + final toDefault = _timeChangeDevice('Switching to the default device'); assert( await _isPlaybackUsable(sound), 'The engine should still play after switching to the default device', ); - strBuf.writeln('Switched to the default device, playback still usable'); + strBuf.writeln( + 'Switched to the default device in ${toDefault.inMilliseconds}ms, ' + 'playback still usable', + ); // Regression: an enumerated device still works. Only one device is required // because CI/desktop machines commonly expose a single one. - SoLoud.instance.changeDevice(newDevice: devices.first); + final toEnumerated = _timeChangeDevice( + 'Switching to device "${devices.first.name}"', + newDevice: devices.first, + ); assert( await _isPlaybackUsable(sound), 'The engine should still play after switching to an enumerated device', ); strBuf.writeln( - 'Switched to device "${devices.first.name}", playback still usable', + 'Switched to device "${devices.first.name}" in ' + '${toEnumerated.inMilliseconds}ms, playback still usable', ); // Invalid native selectors are rejected before the current device is @@ -77,6 +96,57 @@ Future testPlaybackDevices() async { ); strBuf.writeln('Invalid device IDs rejected without disrupting playback'); + // Swap repeatedly while the mixer is actually running. Nothing serializes the + // audio callback against the swap any more: `ma_device_uninit()` alone is + // responsible for quiescing the callback before its stream is closed, and + // `Soloud::mix()` takes the audio mutex itself. If that ordering were not + // enough, a live voice across back-to-back swaps is what would expose it. + final looped = SoLoud.instance.play(sound, looping: true); + assert( + SoLoud.instance.getIsValidVoiceHandle(looped), + 'The looping voice used for the swap stress test should start', + ); + var slowestSwap = Duration.zero; + for (var i = 0; i < 10; i++) { + final elapsed = _timeChangeDevice('Stress swap $i'); + if (elapsed > slowestSwap) slowestSwap = elapsed; + assert( + SoLoud.instance.getIsValidVoiceHandle(looped), + 'The looping voice should survive swap $i: a device change replaces the ' + 'output device, it does not touch voices', + ); + await delay(100); + } + assert( + SoLoud.instance.getActiveVoiceCount() > 0, + 'The engine should still be mixing after 10 device swaps', + ); + strBuf.writeln( + '10 back-to-back swaps under a live voice, slowest ' + '${slowestSwap.inMilliseconds}ms', + ); + + // Aim a swap at the deferred engine pause. `Player`'s pause scheduler stops + // the audio device from its own thread ~500ms (kPauseEngineDelayMs) after the + // last voice ends, so it is the one device operation an app can drive + // concurrently with a swap without any OS lifecycle event. Both act on the + // same `ma_device`, and mid-swap there is no device at all — a start or stop + // landing there is operating on a torn struct. + // + // Note this is a probe, not a proof: it can only make the collision likely, + // and on web there is no scheduler thread at all (the wasm build pauses + // inline). A green run is evidence, not a guarantee of correct locking. + await SoLoud.instance.stop(looped); + for (var i = 0; i < 5; i++) { + await delay(450); + _timeChangeDevice('Swap $i racing the deferred engine pause'); + assert( + await _isPlaybackUsable(sound), + 'The engine should still play after swap $i raced the engine pause', + ); + } + strBuf.writeln('5 swaps aimed at the deferred engine pause window survived'); + // On desktop platforms, we can test changing devices // On mobile and web, there's typically only the default device // Note: not all output devices can be heard. @@ -84,7 +154,7 @@ Future testPlaybackDevices() async { for (final device in devices) { strBuf.writeln('Testing device: ${device.name}'); debugPrint('Testing device: ${device.name}'); - SoLoud.instance.changeDevice(newDevice: device); + _timeChangeDevice('Switching to "${device.name}"', newDevice: device); await delay(3000); } @@ -134,6 +204,27 @@ Future testPlaybackDevices() async { return strBuf; } +/// Calls `changeDevice()` and reports how long the native call took, asserting +/// it stayed inside [_changeDeviceBudget]. +/// +/// This cannot catch a true deadlock: `changeDevice()` is a synchronous FFI +/// call on the UI isolate, so once the native side wedges there is no Dart code +/// left to time it out — the app just freezes (the Android ANR). What it does +/// catch is the multi-second stall that precedes that deadlock, which has the +/// same cause and is visible on every platform where the swap then recovers. +Duration _timeChangeDevice(String what, {PlaybackDevice? newDevice}) { + final stopwatch = Stopwatch()..start(); + SoLoud.instance.changeDevice(newDevice: newDevice); + stopwatch.stop(); + assert( + stopwatch.elapsed < _changeDeviceBudget, + '$what took ${stopwatch.elapsedMilliseconds}ms, over the ' + '${_changeDeviceBudget.inMilliseconds}ms budget. The audio callback is ' + 'being starved during the swap — see _changeDeviceBudget.', + ); + return stopwatch.elapsed; +} + /// Starts a voice and checks the engine handed back a usable handle, then /// stops it again. Used to confirm the output device survived a change. Future _isPlaybackUsable(AudioSource sound) async { diff --git a/src/soloud/src/backend/miniaudio/soloud_miniaudio.cpp b/src/soloud/src/backend/miniaudio/soloud_miniaudio.cpp index c6c373cb..b5865aad 100644 --- a/src/soloud/src/backend/miniaudio/soloud_miniaudio.cpp +++ b/src/soloud/src/backend/miniaudio/soloud_miniaudio.cpp @@ -114,8 +114,12 @@ namespace SoLoud static bool gDeviceInitialized = false; // Track if device is actually initialized static std::thread *gInitThread = nullptr; // Background thread for device init static std::mutex gInitMutex; // Protect device init state - // Serializes device start/stop operations (e.g. resume) against device - // teardown in soloud_miniaudio_deinit(). + // Serializes every operation that touches `gDevice`: pause, resume, the + // device swap in miniaudio_changeDevice_impl() and teardown in + // soloud_miniaudio_deinit(). Any two of these running at once act on a + // half-initialized or already-freed device. Note this is NOT SoLoud's + // audio-thread mutex and must not be confused with it: the data callback + // never takes this one, so holding it cannot starve the audio thread. static std::mutex gDeviceOpsMutex; // Configuration to store for deferred initialization @@ -315,6 +319,15 @@ namespace SoLoud // state and keeps MPRemoteCommandCenter routing intact. result soloud_miniaudio_pause(SoLoud::Soloud *aSoloud) { + // Take the same device-ops lock as resume()/deinit()/changeDevice(). + // Without it this is the one device operation that can still land on a + // `gDevice` another thread is swapping or tearing down — and it is the + // most likely one to do so, since Player's pause scheduler fires it + // from its own thread ~500ms after the last voice ends. + std::lock_guard deviceOpsLock(gDeviceOpsMutex); + if (!gDeviceInitialized) + return 0; // No device to pause. + if (ma_device_get_state(&gDevice) == ma_device_state_started) { #if defined(__EMSCRIPTEN__) || defined(__ANDROID__) @@ -625,17 +638,37 @@ namespace SoLoud if (soloud == nullptr) return UNKNOWN_ERROR; + // Serialize the whole swap against the other operations that act on + // `gDevice`: soloud_miniaudio_pause(), soloud_miniaudio_resume() and + // soloud_miniaudio_deinit(). Between the uninit and the init below + // there is no device at all, and a concurrent start/stop on that torn + // struct is the SIGABRT documented in soloud_miniaudio_resume(). + // This is reachable in ordinary use, not just on lifecycle events: + // Player's pause scheduler runs on its own thread and calls + // Soloud::pause() ~500ms after the last voice ends. + std::lock_guard deviceOpsLock(gDeviceOpsMutex); + // Stop the device before uninitializing to ensure clean shutdown if (ma_device_get_state(&gDevice) == ma_device_state_started) { ma_device_stop(&gDevice); } - // Lock the audio mutex to prevent race conditions during device change - soloud->lockAudioMutex_internal(); - + // SoLoud's audio-thread mutex is deliberately NOT held across the swap + // below, even though it guards the mixer, because it does not protect + // `gDevice`: the data callback reaches the engine through + // `pDevice->pUserData` and takes that mutex itself inside + // `Soloud::mix()`, and `ma_device_uninit()` already guarantees the + // callback has stopped before it returns. Holding it here only starves + // the audio thread, and on Android that is fatal: on AAudio's legacy + // (non-MMAP) path a stream reports STARTED only once its first data + // callback has run, so a held mutex makes that callback block, + // `ma_device_start()` time out after 5s, and the cleanup + // `ma_device_uninit()` then wait forever on the very callback the + // caller is blocking. That deadlock is the Android ANR. ma_device_uninit(&gDevice); gDeviceInitialized = false; + gDeviceStopped = true; ma_device_config deviceConfig = ma_device_config_init(ma_device_type_playback); deviceConfig.playback.pDeviceID = (ma_device_id *)pPlaybackInfos_id; @@ -679,8 +712,8 @@ namespace SoLoud #endif if (result != MA_SUCCESS) { + soloud_platform_log("miniaudio_changeDevice_impl: ma_device_init failed with error %d\n", result); gDeviceInitialized = false; - soloud->unlockAudioMutex_internal(); return UNKNOWN_ERROR; } @@ -692,11 +725,13 @@ namespace SoLoud soloud_platform_log("miniaudio_changeDevice_impl: ma_device_start failed with error %d\n", startResult); ma_device_uninit(&gDevice); gDeviceInitialized = false; - soloud->unlockAudioMutex_internal(); + // The device never reached a running state and is now gone, so + // don't leave `deinit()` polling for a "stopped" notification that + // can no longer arrive. + gDeviceStopped = true; return UNKNOWN_ERROR; } - soloud->unlockAudioMutex_internal(); return 0; } }; diff --git a/test/change_device_test.cpp b/test/change_device_test.cpp new file mode 100644 index 00000000..ece16dca --- /dev/null +++ b/test/change_device_test.cpp @@ -0,0 +1,217 @@ +// Standalone native regression tests for the output-device swap. +// +// miniaudio_changeDevice_impl() replaces the engine's ma_device in place: +// uninit the old one, init the new one, start it. Two things about that were +// wrong, and both are only reachable once a device change actually reaches the +// backend (before #532 it did not, so nothing exercised this). +// +// 1. The swap ran with SoLoud's audio-thread mutex held. That mutex does not +// protect gDevice at all -- the data callback reaches the engine through +// pDevice->pUserData and takes the mutex itself inside Soloud::mix() -- so +// holding it across the swap only starves the audio thread. On Android that +// is fatal: on AAudio's legacy (non-MMAP) path a stream reports STARTED only +// once its first data callback has run, so the blocked callback makes +// ma_device_start() time out after 5s, and the cleanup ma_device_uninit() +// then waits forever on that same blocked callback. Permanent freeze (ANR). +// +// 2. Nothing serialized the swap against the other operations on the same +// gDevice: soloud_miniaudio_pause(), soloud_miniaudio_resume() and +// soloud_miniaudio_deinit(). Between the uninit and the init there is no +// device at all, so a concurrent start/stop runs against a torn struct. +// This is not hypothetical or lifecycle-only: flutter_soloud's Player runs a +// pause scheduler on its own thread that calls Soloud::pause() ~500ms after +// the last voice ends. Without the device-ops lock, kConcurrentSwaps below +// segfaults or hangs; with it, it is stable. +// +// Build and run from the flutter_soloud repository root with: +// +// ./test/run_change_device_test.sh + +#include "soloud.h" +#include "soloud_audiosource.h" + +#include +#include +#include +#include +#include + +namespace { + +constexpr int kSerialSwaps = 30; +constexpr int kConcurrentThreads = 4; +constexpr int kSwapsPerThread = 25; + +// A single swap closes one stream and opens another: tens of milliseconds. +// The number that matters is the one this excludes -- the 5s AAudio +// state-transition timeout that the held audio mutex used to produce. +constexpr long long kSwapBudgetMs = 2000; + +int gFailures = 0; + +void check(bool condition, const char *what) +{ + if (condition) + { + std::printf(" ok: %s\n", what); + return; + } + std::printf(" FAILED: %s\n", what); + gFailures++; +} + +long long nowMs() +{ + return std::chrono::duration_cast( + std::chrono::steady_clock::now().time_since_epoch()) + .count(); +} + +// A never-ending silent source, so the mixer is actually running while the +// device underneath it is replaced. +class SilenceInstance : public SoLoud::AudioSourceInstance +{ +public: + unsigned int getAudio(float *aBuffer, unsigned int aSamplesToRead, + unsigned int aBufferSize) override + { + for (unsigned int c = 0; c < mChannels; c++) + for (unsigned int i = 0; i < aSamplesToRead; i++) + aBuffer[i + c * aBufferSize] = 0.0f; + return aSamplesToRead; + } + + bool hasEnded() override { return false; } +}; + +class Silence : public SoLoud::AudioSource +{ +public: + Silence() { mChannels = 2; } + + SoLoud::AudioSourceInstance *createInstance() override + { + return new SilenceInstance(); + } +}; + +// Swapping to the null device id asks miniaudio for the current OS default, +// which is what SoLoud.changeDevice() does when called without an argument. +SoLoud::result swapToDefault(SoLoud::Soloud &soloud) +{ + return soloud.miniaudio_changeDevice(nullptr); +} + +// A swap that never finishes cannot be caught from inside the process, so this +// pins down the stall that precedes the deadlock instead: with the audio mutex +// held across ma_device_start(), each swap costs the full AAudio timeout. +void testSwapsAreFastAndSurviveMixing(SoLoud::Soloud &soloud) +{ + std::printf("swaps under a live voice\n"); + + Silence silence; + const SoLoud::handle handle = soloud.play(silence); + check(soloud.isValidVoiceHandle(handle), "the test voice is playing"); + + long long worstMs = 0; + int errors = 0; + for (int i = 0; i < kSerialSwaps; i++) + { + const long long start = nowMs(); + if (swapToDefault(soloud) != SoLoud::SO_NO_ERROR) + errors++; + const long long elapsed = nowMs() - start; + if (elapsed > worstMs) + worstMs = elapsed; + std::this_thread::sleep_for(std::chrono::milliseconds(10)); + } + + std::printf(" %d swaps, slowest %lldms\n", kSerialSwaps, worstMs); + check(errors == 0, "every swap reported success"); + check(worstMs < kSwapBudgetMs, + "no swap blocked for the AAudio state-transition timeout"); + check(soloud.isValidVoiceHandle(handle), + "the voice survived: a device change replaces the output device, " + "it does not touch voices"); + + soloud.stop(handle); +} + +// The realistic collision is a swap against the pause scheduler's +// Soloud::pause() on its own thread. Racing swaps against each other reaches +// the same shared gDevice through the same lock, and is far easier to aim. +void testConcurrentDeviceOps(SoLoud::Soloud &soloud) +{ + std::printf("concurrent device operations\n"); + + Silence silence; + const SoLoud::handle handle = soloud.play(silence); + + std::atomic errors{0}; + std::vector threads; + threads.reserve(kConcurrentThreads); + for (int t = 0; t < kConcurrentThreads; t++) + { + threads.emplace_back([&soloud, &errors] + { + for (int i = 0; i < kSwapsPerThread; i++) + { + if (swapToDefault(soloud) != SoLoud::SO_NO_ERROR) + errors++; + std::this_thread::sleep_for(std::chrono::milliseconds(2)); + } }); + } + + // Pause/resume are the operations flutter_soloud itself drives concurrently + // with a swap, so run them against it too rather than only swap-vs-swap. + std::thread pauser([&soloud] + { + for (int i = 0; i < kSwapsPerThread; i++) + { + soloud.pause(); + soloud.resume(); + std::this_thread::sleep_for(std::chrono::milliseconds(3)); + } }); + + for (auto &thread : threads) + thread.join(); + pauser.join(); + + std::printf(" %d swaps across %d threads, interleaved with pause/resume\n", + kConcurrentThreads * kSwapsPerThread, kConcurrentThreads); + check(errors.load() == 0, "every concurrent swap reported success"); + check(soloud.isValidVoiceHandle(handle), + "the voice survived the concurrent swaps"); + + soloud.stop(handle); +} + +} // namespace + +int main() +{ + SoLoud::Soloud soloud; + const SoLoud::result init = soloud.init(SoLoud::Soloud::CLIP_ROUNDOFF, + SoLoud::Soloud::MINIAUDIO); + if (init != SoLoud::SO_NO_ERROR) + { + // No usable audio backend (headless CI, no /dev/snd). There is nothing + // to swap, and reporting a failure here would only be noise. + std::printf("SKIPPED: miniaudio could not open a device (error %d)\n", + init); + return 0; + } + + testSwapsAreFastAndSurviveMixing(soloud); + testConcurrentDeviceOps(soloud); + + soloud.deinit(); + + if (gFailures == 0) + { + std::printf("All change-device tests passed.\n"); + return 0; + } + std::printf("%d change-device test(s) failed.\n", gFailures); + return 1; +} diff --git a/test/run_change_device_test.sh b/test/run_change_device_test.sh new file mode 100755 index 00000000..2aff4e50 --- /dev/null +++ b/test/run_change_device_test.sh @@ -0,0 +1,28 @@ +#!/bin/bash +# Build and run the standalone native output-device swap regression tests. +# +# Unlike the other native tests here this one needs the miniaudio backend +# (that is where changeDevice lives) and therefore a real output device. On a +# machine without one the test reports SKIPPED and exits 0. + +set -e + +REPO_ROOT="$(cd "$(dirname "$0")/.." && pwd)" +cd "$REPO_ROOT" + +OUT="${TMPDIR:-/tmp}/change_device_test" + +c++ -std=c++17 -O2 -Wall -Wextra -pthread \ + -DWITH_MINIAUDIO \ + -DNO_XIPH_LIBS \ + -I src/soloud/include \ + -I src \ + -o "$OUT" \ + test/change_device_test.cpp \ + src/soloud_common.cpp \ + src/soloud/src/core/*.cpp \ + src/soloud/src/backend/miniaudio/soloud_miniaudio.cpp \ + src/mixeroutput/*.cpp \ + -ldl -lm + +"$OUT"