Revert "Revert "Lock waitForStateChange()""
diff --git a/src/aaudio/AudioStreamAAudio.cpp b/src/aaudio/AudioStreamAAudio.cpp
index b8c4394..bfbf3ee 100644
--- a/src/aaudio/AudioStreamAAudio.cpp
+++ b/src/aaudio/AudioStreamAAudio.cpp
@@ -20,6 +20,7 @@
#include "aaudio/AAudioLoader.h"
#include "aaudio/AudioStreamAAudio.h"
+#include "common/AudioClock.h"
#include "common/OboeDebug.h"
#include "oboe/Utilities.h"
@@ -370,25 +371,69 @@
}
}
+
+// AAudioStream_waitForStateChange() can crash if it is waiting on a stream and that stream
+// is closed from another thread. We do not want to lock the stream for the duration of the call.
+// So we call AAudioStream_waitForStateChange() with a timeout of zero so that it will not block.
+// Then we can do our own sleep with the lock unlocked.
Result AudioStreamAAudio::waitForStateChange(StreamState currentState,
StreamState *nextState,
int64_t timeoutNanoseconds) {
- AAudioStream *stream = mAAudioStream.load();
- if (stream != nullptr) {
+ Result oboeResult = Result::ErrorTimeout;
+ int64_t durationNanos = 20 * kNanosPerMillisecond; // arbitrary
+ aaudio_stream_state_t currentAAudioState = static_cast<aaudio_stream_state_t>(currentState);
- aaudio_stream_state_t aaudioNextState;
- aaudio_result_t result = mLibLoader->stream_waitForStateChange(
- mAAudioStream,
- static_cast<aaudio_stream_state_t>(currentState),
- &aaudioNextState,
- timeoutNanoseconds);
- *nextState = static_cast<StreamState>(aaudioNextState);
- return static_cast<Result>(result);
- } else {
- *nextState = StreamState::Closed;
+ aaudio_stream_state_t aaudioNextState;
+ aaudio_result_t result = AAUDIO_OK;
+ int64_t timeLeftNanos = timeoutNanoseconds;
+
+ mLock.lock();
+ while (true) {
+ // Do we still have an AAudio stream? If not then stream must have been closed.
+ AAudioStream *stream = mAAudioStream.load();
+ if (stream == nullptr) {
+ if (nextState != nullptr) {
+ *nextState = StreamState::Closed;
+ }
+ oboeResult = Result::ErrorClosed;
+ break;
+ }
+
+ // Update and query state change with no blocking.
+ result = mLibLoader->stream_waitForStateChange(
+ mAAudioStream,
+ currentAAudioState,
+ &aaudioNextState,
+ 0); // timeout=0 for non-blocking
+ if (nextState != nullptr) {
+ *nextState = static_cast<StreamState>(aaudioNextState);
+ }
+ if (result != AAUDIO_OK) {
+ oboeResult = static_cast<Result>(result);
+ break;
+ }
+ if (currentAAudioState != aaudioNextState) { // state changed?
+ oboeResult = Result::OK;
+ break;
+ }
+
+ // Did we timeout or did user ask for non-blocking?
+ if (timeLeftNanos <= 0) {
+ break;
+ }
+
+ // No change yet so sleep.
+ mLock.unlock(); // Don't sleep while locked.
+ if (durationNanos > timeLeftNanos) {
+ durationNanos = timeLeftNanos; // last little bit
+ }
+ AudioClock::sleepForNanos(durationNanos);
+ timeLeftNanos -= durationNanos;
+ mLock.lock();
}
- return (currentState != *nextState) ? Result::OK : Result::ErrorTimeout;
+ mLock.unlock();
+ return oboeResult;
}
ResultWithValue<int32_t> AudioStreamAAudio::setBufferSizeInFrames(int32_t requestedFrames) {
diff --git a/src/common/AudioStream.cpp b/src/common/AudioStream.cpp
index 5dfafb1..63f3c44 100644
--- a/src/common/AudioStream.cpp
+++ b/src/common/AudioStream.cpp
@@ -75,9 +75,13 @@
StreamState endingState,
int64_t timeoutNanoseconds)
{
- StreamState state = getState();
- if (state == StreamState::Closed) {
- return Result::ErrorClosed;
+ StreamState state;
+ {
+ std::lock_guard<std::mutex> lock(mLock);
+ state = getState();
+ if (state == StreamState::Closed || state == StreamState::Disconnected) {
+ return Result::ErrorClosed;
+ }
}
StreamState nextState = state;
@@ -87,6 +91,7 @@
return result;
}
}
+
if (nextState != endingState) {
return Result::ErrorInvalidState;
} else {
@@ -98,6 +103,7 @@
{
Result result = requestStart();
if (result != Result::OK) return result;
+ if (timeoutNanoseconds <= 0) return result;
return waitForStateTransition(StreamState::Starting,
StreamState::Started, timeoutNanoseconds);
}
@@ -106,6 +112,7 @@
{
Result result = requestPause();
if (result != Result::OK) return result;
+ if (timeoutNanoseconds <= 0) return result;
return waitForStateTransition(StreamState::Pausing,
StreamState::Paused, timeoutNanoseconds);
}
@@ -114,6 +121,7 @@
{
Result result = requestFlush();
if (result != Result::OK) return result;
+ if (timeoutNanoseconds <= 0) return result;
return waitForStateTransition(StreamState::Flushing,
StreamState::Flushed, timeoutNanoseconds);
}
@@ -122,6 +130,7 @@
{
Result result = requestStop();
if (result != Result::OK) return result;
+ if (timeoutNanoseconds <= 0) return result;
return waitForStateTransition(StreamState::Stopping,
StreamState::Stopped, timeoutNanoseconds);
}
diff --git a/src/opensles/AudioInputStreamOpenSLES.cpp b/src/opensles/AudioInputStreamOpenSLES.cpp
index 7f9ace0..cfdb1bd 100644
--- a/src/opensles/AudioInputStreamOpenSLES.cpp
+++ b/src/opensles/AudioInputStreamOpenSLES.cpp
@@ -209,7 +209,7 @@
LOGD("AudioInputStreamOpenSLES::%s()", __func__);
mLock.lock();
Result result = Result::OK;
- if (mState == StreamState::Closed){
+ if (getState() == StreamState::Closed){
result = Result::ErrorClosed;
} else {
mLock.unlock(); // avoid recursive lock
diff --git a/src/opensles/AudioOutputStreamOpenSLES.cpp b/src/opensles/AudioOutputStreamOpenSLES.cpp
index 3c9e27b..2b785ba 100644
--- a/src/opensles/AudioOutputStreamOpenSLES.cpp
+++ b/src/opensles/AudioOutputStreamOpenSLES.cpp
@@ -240,7 +240,7 @@
Result AudioOutputStreamOpenSLES::close() {
mLock.lock();
Result result = Result::OK;
- if (mState == StreamState::Closed){
+ if (getState() == StreamState::Closed){
result = Result::ErrorClosed;
} else {
mLock.unlock(); // avoid recursive lock
diff --git a/src/opensles/AudioStreamOpenSLES.cpp b/src/opensles/AudioStreamOpenSLES.cpp
index 93ad431..20c02af 100644
--- a/src/opensles/AudioStreamOpenSLES.cpp
+++ b/src/opensles/AudioStreamOpenSLES.cpp
@@ -234,7 +234,7 @@
mSimpleBufferQueueInterface = nullptr;
EngineOpenSLES::getInstance().close();
- mState = StreamState::Closed;
+ setState(StreamState::Closed);
return Result::OK;
}
}
@@ -311,23 +311,30 @@
Result AudioStreamOpenSLES::waitForStateChange(StreamState currentState,
StreamState *nextState,
int64_t timeoutNanoseconds) {
- LOGD("AudioStreamOpenSLES::waitForStateChange()");
-
+ Result oboeResult = (timeoutNanoseconds <= 0) ? Result::OK : Result::ErrorTimeout;
int64_t durationNanos = 20 * kNanosPerMillisecond; // arbitrary
- StreamState state = getState();
- while (state == currentState && timeoutNanoseconds > 0){
+ while (true) {
+ const StreamState state = getState(); // this does not require a lock
+ if (nextState != nullptr) {
+ *nextState = state;
+ }
+ if (currentState != state) { // state changed?
+ oboeResult = Result::OK;
+ break;
+ }
+
+ // Did we timeout or did user ask for non-blocking?
+ if (timeoutNanoseconds <= 0) {
+ break;
+ }
+
if (durationNanos > timeoutNanoseconds){
durationNanos = timeoutNanoseconds;
}
AudioClock::sleepForNanos(durationNanos);
timeoutNanoseconds -= durationNanos;
-
- state = getState();
- }
- if (nextState != nullptr) {
- *nextState = state;
}
- return (state == currentState) ? Result::ErrorTimeout : Result::OK;
+ return oboeResult;
}
diff --git a/src/opensles/AudioStreamOpenSLES.h b/src/opensles/AudioStreamOpenSLES.h
index c49fd35..3d6996a 100644
--- a/src/opensles/AudioStreamOpenSLES.h
+++ b/src/opensles/AudioStreamOpenSLES.h
@@ -55,7 +55,7 @@
*
* @return state or a negative error.
*/
- StreamState getState() override { return mState; }
+ StreamState getState() override { return mState.load(); }
int32_t getFramesPerBurst() override;
@@ -101,7 +101,7 @@
* Use this instead of directly setting the internal state variable.
*/
void setState(StreamState state) {
- mState = state;
+ mState.store(state);
}
int64_t getFramesProcessedByServer() const;
@@ -110,11 +110,13 @@
SLObjectItf mObjectInterface = nullptr;
SLAndroidSimpleBufferQueueItf mSimpleBufferQueueInterface = nullptr;
- uint8_t *mCallbackBuffer = nullptr;
- int32_t mBytesPerCallback = oboe::kUnspecified;
- StreamState mState = StreamState::Uninitialized;
+ uint8_t *mCallbackBuffer = nullptr;
+ int32_t mBytesPerCallback = oboe::kUnspecified;
+ MonotonicCounter mPositionMillis; // for tracking OpenSL ES service position
- MonotonicCounter mPositionMillis; // for tracking OpenSL ES service position
+private:
+ std::atomic<StreamState> mState{StreamState::Uninitialized};
+
};
} // namespace oboe
