From 8f38a1b5bd6858c41fc5109f939af3e54fa2ce86 Mon Sep 17 00:00:00 2001 From: Matthew Gregan Date: Wed, 21 Jan 2026 14:56:33 +1300 Subject: [PATCH 1/5] aaudio: Initialize get_{,input_}latency result when metrics unavailable. --- src/cubeb_aaudio.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cubeb_aaudio.cpp b/src/cubeb_aaudio.cpp index 8379328b..f5f15a25 100644 --- a/src/cubeb_aaudio.cpp +++ b/src/cubeb_aaudio.cpp @@ -1870,6 +1870,7 @@ aaudio_stream_get_latency(cubeb_stream * stm, uint32_t * latency) if (!stm->latency_metrics_available) { LOG("Not timing info yet (output)"); + *latency = 0; return CUBEB_OK; } @@ -1891,6 +1892,7 @@ aaudio_stream_get_input_latency(cubeb_stream * stm, uint32_t * latency) if (!stm->latency_metrics_available) { LOG("Not timing info yet (input)"); + *latency = 0; return CUBEB_OK; } From c8c2d1d36bc160e90ba1d1576f98d7c5c8fbe959 Mon Sep 17 00:00:00 2001 From: Matthew Gregan Date: Wed, 21 Jan 2026 14:34:14 +1300 Subject: [PATCH 2/5] aaudio: Remove racy in_data_callback check in shutdown_with_error. It's possible for the state_thread to run shutdown_with_error before the data callback has returned. stream_destroy ensures correct destruction anyway. --- src/cubeb_aaudio.cpp | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/src/cubeb_aaudio.cpp b/src/cubeb_aaudio.cpp index f5f15a25..5f331c0a 100644 --- a/src/cubeb_aaudio.cpp +++ b/src/cubeb_aaudio.cpp @@ -326,7 +326,6 @@ struct cubeb_stream { std::atomic latency_metrics_available{false}; std::atomic drain_target{-1}; std::atomic state{stream_state::INIT}; - std::atomic in_data_callback{false}; triple_buffer timing_info; AAudioStream * ostream{}; @@ -377,15 +376,6 @@ struct cubeb { struct cubeb_stream streams[MAX_STREAMS]; }; -struct AutoInCallback { - AutoInCallback(cubeb_stream * stm) : stm(stm) - { - stm->in_data_callback.store(true); - } - ~AutoInCallback() { stm->in_data_callback.store(false); } - cubeb_stream * stm; -}; - // Returns when aaudio_stream's state is equal to desired_state. // poll_frequency_ns is the duration that is slept in between asking for // state updates and getting the new state. @@ -452,7 +442,6 @@ shutdown_with_error(cubeb_stream * stm) } } - assert(!stm->in_data_callback.load()); stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_ERROR); stm->state.store(stream_state::SHUTDOWN); } @@ -892,7 +881,6 @@ aaudio_duplex_data_cb(AAudioStream * astream, void * user_data, void * audio_data, int32_t num_frames) { cubeb_stream * stm = (cubeb_stream *)user_data; - AutoInCallback aic(stm); assert(stm->ostream == astream); assert(stm->istream); assert(num_frames >= 0); @@ -993,7 +981,6 @@ aaudio_output_data_cb(AAudioStream * astream, void * user_data, void * audio_data, int32_t num_frames) { cubeb_stream * stm = (cubeb_stream *)user_data; - AutoInCallback aic(stm); assert(stm->ostream == astream); assert(!stm->istream); assert(num_frames >= 0); @@ -1047,7 +1034,6 @@ aaudio_input_data_cb(AAudioStream * astream, void * user_data, void * audio_data, int32_t num_frames) { cubeb_stream * stm = (cubeb_stream *)user_data; - AutoInCallback aic(stm); assert(stm->istream == astream); assert(!stm->ostream); assert(num_frames >= 0); From c4ccc9da1eae8b7ef4ae0a17171860d23d076aba Mon Sep 17 00:00:00 2001 From: Matthew Gregan Date: Wed, 21 Jan 2026 13:23:16 +1300 Subject: [PATCH 3/5] aaudio: Use requestStop for input streams rather than requestPause. Per https://developer.android.com/ndk/reference/group/audio#group___audio_1ga60d7800abe432ad1e3404243c8407e1c, requestPause is not supported for input streams and will return ERROR_UNIMPLEMENTED. Use requestStop instead and adjust state transition expectations. --- src/cubeb_aaudio.cpp | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/src/cubeb_aaudio.cpp b/src/cubeb_aaudio.cpp index 5f331c0a..90061c8f 100644 --- a/src/cubeb_aaudio.cpp +++ b/src/cubeb_aaudio.cpp @@ -616,16 +616,19 @@ update_state(cubeb_stream * stm) case stream_state::STOPPING: // If stream_stop happens while the stream is still starting, we may see // STARTING/STARTED, ignore these and handle STATE_STOPPED once we reach - // PAUSED. + // PAUSED (for output) or STOPPED (for input, which doesn't support + // pause). assert(!istate || istate == AAUDIO_STREAM_STATE_STARTING || istate == AAUDIO_STREAM_STATE_STARTED || - istate == AAUDIO_STREAM_STATE_PAUSING || - istate == AAUDIO_STREAM_STATE_PAUSED); + istate == AAUDIO_STREAM_STATE_STOPPING || + istate == AAUDIO_STREAM_STATE_STOPPED); assert(!ostate || ostate == AAUDIO_STREAM_STATE_STARTING || ostate == AAUDIO_STREAM_STATE_STARTED || ostate == AAUDIO_STREAM_STATE_PAUSING || ostate == AAUDIO_STREAM_STATE_PAUSED); - if ((!istate || istate == AAUDIO_STREAM_STATE_PAUSED) && + // Input streams use requestStop (goes to STOPPED), output uses + // requestPause (goes to PAUSED) + if ((!istate || istate == AAUDIO_STREAM_STATE_STOPPED) && (!ostate || ostate == AAUDIO_STREAM_STATE_PAUSED)) { stm->state_callback(stm, stm->user_ptr, CUBEB_STATE_STOPPED); new_state = stream_state::STOPPED; @@ -1730,9 +1733,11 @@ aaudio_stream_stop_locked(cubeb_stream * stm, lock_guard & lock) } if (stm->istream) { - res = WRAP(AAudioStream_requestPause)(stm->istream); + // AAudio input streams don't support pause - use stop instead. + // The stream will transition through STOPPING to STOPPED. + res = WRAP(AAudioStream_requestStop)(stm->istream); if (res != AAUDIO_OK) { - LOG("AAudioStream_requestPause (istream): %s", + LOG("AAudioStream_requestStop (istream): %s", WRAP(AAudio_convertResultToText)(res)); stm->state.store(stream_state::ERROR); return CUBEB_ERROR; From 8fd71f5132c1754a14bc55191b4a254899c557fd Mon Sep 17 00:00:00 2001 From: Matthew Gregan Date: Wed, 21 Jan 2026 22:17:09 +1300 Subject: [PATCH 4/5] aaudio: Wait for expected stream states in stream_{start,stop}. This makes stream_{start,stop} more predictable in the face of rapid start/stop cycles. --- src/cubeb_aaudio.cpp | 79 ++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 3 deletions(-) diff --git a/src/cubeb_aaudio.cpp b/src/cubeb_aaudio.cpp index 90061c8f..b27733ff 100644 --- a/src/cubeb_aaudio.cpp +++ b/src/cubeb_aaudio.cpp @@ -410,6 +410,25 @@ wait_for_state_change(AAudioStream * aaudio_stream, return CUBEB_OK; } +// Maps a transitional state to its corresponding stable state. +// Returns AAUDIO_STREAM_STATE_UNINITIALIZED if not transitional. +static aaudio_stream_state_t +get_stable_state(aaudio_stream_state_t state) +{ + switch (state) { + case AAUDIO_STREAM_STATE_STARTING: + return AAUDIO_STREAM_STATE_STARTED; + case AAUDIO_STREAM_STATE_PAUSING: + return AAUDIO_STREAM_STATE_PAUSED; + case AAUDIO_STREAM_STATE_STOPPING: + return AAUDIO_STREAM_STATE_STOPPED; + case AAUDIO_STREAM_STATE_FLUSHING: + return AAUDIO_STREAM_STATE_FLUSHED; + default: + return AAUDIO_STREAM_STATE_UNINITIALIZED; + } +} + // Only allowed from state thread, while mutex on stm is locked static void shutdown_with_error(cubeb_stream * stm) @@ -1577,9 +1596,15 @@ aaudio_stream_start_locked(cubeb_stream * stm, lock_guard & lock) { assert(stm && stm->in_use.load()); stream_state state = stm->state.load(); - int istate = stm->istream ? WRAP(AAudioStream_getState)(stm->istream) : 0; - int ostate = stm->ostream ? WRAP(AAudioStream_getState)(stm->ostream) : 0; - LOGV("STARTING stream %p: %d (%d %d)", (void *)stm, state, istate, ostate); + aaudio_stream_state_t istate = stm->istream + ? WRAP(AAudioStream_getState)(stm->istream) + : AAUDIO_STREAM_STATE_UNINITIALIZED; + aaudio_stream_state_t ostate = stm->ostream + ? WRAP(AAudioStream_getState)(stm->ostream) + : AAUDIO_STREAM_STATE_UNINITIALIZED; + LOG("STARTING stream %p: %d (in: %s out: %s)", (void *)stm, state, + WRAP(AAudio_convertStreamStateToText)(istate), + WRAP(AAudio_convertStreamStateToText)(ostate)); switch (state) { case stream_state::STARTED: @@ -1600,6 +1625,33 @@ aaudio_stream_start_locked(cubeb_stream * stm, lock_guard & lock) aaudio_result_t res; + // Wait for stream transitions to settle before starting. + int64_t poll_frequency_ns = 10 * NS_PER_S / 1000; + if (stm->ostream) { + ostate = WRAP(AAudioStream_getState)(stm->ostream); + aaudio_stream_state_t target = get_stable_state(ostate); + if (target != AAUDIO_STREAM_STATE_UNINITIALIZED) { + int rv = wait_for_state_change(stm->ostream, &target, poll_frequency_ns); + if (rv != CUBEB_OK) { + LOG("Failure waiting for ostream to reach stable state before start"); + stm->state.store(stream_state::ERROR); + return CUBEB_ERROR; + } + } + } + if (stm->istream) { + istate = WRAP(AAudioStream_getState)(stm->istream); + aaudio_stream_state_t target = get_stable_state(istate); + if (target != AAUDIO_STREAM_STATE_UNINITIALIZED) { + int rv = wait_for_state_change(stm->istream, &target, poll_frequency_ns); + if (rv != CUBEB_OK) { + LOG("Failure waiting for istream to reach stable state before start"); + stm->state.store(stream_state::ERROR); + return CUBEB_ERROR; + } + } + } + // Important to start istream before ostream. // As soon as we start ostream, the callbacks might be triggered an we // might read from istream (on duplex). If istream wasn't started yet @@ -1716,6 +1768,27 @@ aaudio_stream_stop_locked(cubeb_stream * stm, lock_guard & lock) aaudio_result_t res; + // Wait for stream transitions to settle before stopping. + int64_t poll_frequency_ns = 10 * NS_PER_S / 1000; // 10ms + if (stm->ostream && ostate == AAUDIO_STREAM_STATE_STARTING) { + aaudio_stream_state_t target = AAUDIO_STREAM_STATE_STARTED; + int rv = wait_for_state_change(stm->ostream, &target, poll_frequency_ns); + if (rv != CUBEB_OK) { + LOG("Failure waiting for ostream to finish starting before stop"); + stm->state.store(stream_state::ERROR); + return CUBEB_ERROR; + } + } + if (stm->istream && istate == AAUDIO_STREAM_STATE_STARTING) { + aaudio_stream_state_t target = AAUDIO_STREAM_STATE_STARTED; + int rv = wait_for_state_change(stm->istream, &target, poll_frequency_ns); + if (rv != CUBEB_OK) { + LOG("Failure waiting for istream to finish starting before stop"); + stm->state.store(stream_state::ERROR); + return CUBEB_ERROR; + } + } + // No callbacks are triggered anymore when requestPause returns. // That is important as we otherwise might read from a closed istream // for a duplex stream. From 0b86c4c97c6cf3ba71a631b4da90febf905ca460 Mon Sep 17 00:00:00 2001 From: Matthew Gregan Date: Wed, 21 Jan 2026 22:44:45 +1300 Subject: [PATCH 5/5] aaudio: Fix shutdown race with reinit threads. Ensure reinit threads can't outlive their associated logical stream, and don't try to perform a reinit on a stream once it has been reallocated. --- src/cubeb_aaudio.cpp | 76 +++++++++++++++++++++++++++++++++----------- 1 file changed, 58 insertions(+), 18 deletions(-) diff --git a/src/cubeb_aaudio.cpp b/src/cubeb_aaudio.cpp index b27733ff..301dff18 100644 --- a/src/cubeb_aaudio.cpp +++ b/src/cubeb_aaudio.cpp @@ -165,6 +165,12 @@ enum class stream_state { SHUTDOWN, }; +enum class slot_state { + FREE = 0, + IN_USE, + DESTROYING, +}; + struct AAudioTimingInfo { // The timestamp at which the audio engine last called the calback. uint64_t tstamp; @@ -322,7 +328,11 @@ struct cubeb_stream { cubeb * context{}; void * user_ptr{}; - std::atomic in_use{false}; + // slot_state is only written with the stream mutex held, but stream_init + // reads it outside the mutex as an optimization to avoid locking every + // stream when searching for a free slot. + std::atomic slot_state{slot_state::FREE}; + std::atomic pending_reinit{0}; std::atomic latency_metrics_available{false}; std::atomic drain_target{-1}; std::atomic state{stream_state::INIT}; @@ -372,7 +382,8 @@ struct cubeb { std::atomic waiting{false}; } state; - // streams[i].in_use signals whether a stream is used + // streams[i].slot_state signals whether a stream slot is free, in use, or + // being destroyed. struct cubeb_stream streams[MAX_STREAMS]; }; @@ -760,7 +771,7 @@ aaudio_destroy(cubeb * ctx) #ifndef NDEBUG // make sure all streams were destroyed for (auto & stream : ctx->streams) { - assert(!stream.in_use.load()); + assert(stream.slot_state.load() == slot_state::FREE); } #endif @@ -1108,8 +1119,27 @@ reinitialize_stream(cubeb_stream * stm) // thread. // In this situation, the lock is acquired for the entire duration of the // function, so that this reinitialization period is atomic. + + // Ensure only one reinit is pending at a time. + int expected = 0; + if (!stm->pending_reinit.compare_exchange_strong(expected, 1)) { + LOG("reinitialize_stream: reinit already pending, skipping"); + return; + } + std::thread([stm] { + struct PendingReinitGuard { + cubeb_stream * stm; + ~PendingReinitGuard() { stm->pending_reinit.store(0); } + } guard{stm}; + lock_guard lock(stm->mutex); + + if (stm->slot_state.load() != slot_state::IN_USE) { + LOG("reinitialize_stream: stream destroyed, cancelling"); + return; + } + stream_state state = stm->state.load(); bool was_playing = state == stream_state::STARTED || state == stream_state::STARTING || @@ -1130,8 +1160,6 @@ reinitialize_stream(cubeb_stream * stm) aaudio_stream_destroy_locked(stm, lock); err = aaudio_stream_init_impl(stm, lock); - assert(stm->in_use.load()); - // Set the new initial position. stm->pos_estimate.reinit(total_frames); @@ -1238,9 +1266,20 @@ realize_stream(AAudioStreamBuilder * sb, const cubeb_stream_params * params, static void aaudio_stream_destroy(cubeb_stream * stm) { - lock_guard lock(stm->mutex); - stm->in_use.store(false); - aaudio_stream_destroy_locked(stm, lock); + { + lock_guard lock(stm->mutex); + aaudio_stream_destroy_locked(stm, lock); + // Two-phase destroy, only mark as free once reinit threads exit. + stm->slot_state.store(slot_state::DESTROYING); + } + + // Wait for reinit threads to exit. + while (stm->pending_reinit.load() > 0) { + auto dur = std::chrono::milliseconds(5); + std::this_thread::sleep_for(dur); + } + + stm->slot_state.store(slot_state::FREE); } static void @@ -1512,19 +1551,20 @@ aaudio_stream_init(cubeb * ctx, cubeb_stream ** stream, unique_lock lock; for (auto & stream : ctx->streams) { // This check is only an optimization, we don't strictly need it - // since we check again after locking the mutex. - if (stream.in_use.load()) { + // since we check again after locking the mutex. It also skips + // DESTROYING slots, which is important to avoid racing with destroy. + if (stream.slot_state.load() != slot_state::FREE) { continue; } // if this fails, another thread initialized this stream - // between our check of in_use and this. + // between our check of slot_state and this. lock = unique_lock(stream.mutex, std::try_to_lock); if (!lock.owns_lock()) { continue; } - if (stream.in_use.load()) { + if (stream.slot_state.load() != slot_state::FREE) { lock = {}; continue; } @@ -1538,7 +1578,7 @@ aaudio_stream_init(cubeb * ctx, cubeb_stream ** stream, return CUBEB_ERROR; } - stm->in_use.store(true); + stm->slot_state.store(slot_state::IN_USE); stm->context = ctx; stm->user_ptr = user_ptr; stm->data_callback = data_callback; @@ -1594,7 +1634,7 @@ aaudio_stream_start(cubeb_stream * stm) static int aaudio_stream_start_locked(cubeb_stream * stm, lock_guard & lock) { - assert(stm && stm->in_use.load()); + assert(stm && stm->slot_state.load() == slot_state::IN_USE); stream_state state = stm->state.load(); aaudio_stream_state_t istate = stm->istream ? WRAP(AAudioStream_getState)(stm->istream) @@ -1728,7 +1768,7 @@ aaudio_stream_start_locked(cubeb_stream * stm, lock_guard & lock) static int aaudio_stream_stop(cubeb_stream * stm) { - assert(stm && stm->in_use.load()); + assert(stm && stm->slot_state.load() == slot_state::IN_USE); lock_guard lock(stm->mutex); return aaudio_stream_stop_locked(stm, lock); } @@ -1736,7 +1776,7 @@ aaudio_stream_stop(cubeb_stream * stm) static int aaudio_stream_stop_locked(cubeb_stream * stm, lock_guard & lock) { - assert(stm && stm->in_use.load()); + assert(stm && stm->slot_state.load() == slot_state::IN_USE); stream_state state = stm->state.load(); aaudio_stream_state_t istate = stm->istream @@ -1866,7 +1906,7 @@ aaudio_stream_stop_locked(cubeb_stream * stm, lock_guard & lock) static int aaudio_stream_get_position(cubeb_stream * stm, uint64_t * position) { - assert(stm && stm->in_use.load()); + assert(stm && stm->slot_state.load() == slot_state::IN_USE); lock_guard lock(stm->mutex); stream_state state = stm->state.load(); @@ -1971,7 +2011,7 @@ aaudio_stream_get_input_latency(cubeb_stream * stm, uint32_t * latency) static int aaudio_stream_set_volume(cubeb_stream * stm, float volume) { - assert(stm && stm->in_use.load() && stm->ostream); + assert(stm && stm->slot_state.load() == slot_state::IN_USE && stm->ostream); stm->volume.store(volume); return CUBEB_OK; }