Merge pull request #821 from google/deleterace
Add openSharedStream() to prevent deletion while executing onError callbacks
diff --git a/apps/OboeTester/app/build.gradle b/apps/OboeTester/app/build.gradle
index a57de16..9784049 100644
--- a/apps/OboeTester/app/build.gradle
+++ b/apps/OboeTester/app/build.gradle
@@ -7,8 +7,8 @@
minSdkVersion 23
targetSdkVersion 28
// Also update the version in the AndroidManifest.xml file.
- versionCode 31
- versionName "1.5.23"
+ versionCode 32
+ versionName "1.5.24"
testInstrumentationRunner "android.support.test.runner.AndroidJUnitRunner"
externalNativeBuild {
cmake {
diff --git a/apps/OboeTester/app/src/main/AndroidManifest.xml b/apps/OboeTester/app/src/main/AndroidManifest.xml
index d52d321..e0cc3d1 100644
--- a/apps/OboeTester/app/src/main/AndroidManifest.xml
+++ b/apps/OboeTester/app/src/main/AndroidManifest.xml
@@ -1,8 +1,8 @@
<?xml version="1.0" encoding="utf-8"?>
<manifest xmlns:android="http://schemas.android.com/apk/res/android"
package="com.google.sample.oboe.manualtest"
- android:versionCode="31"
- android:versionName="1.5.23">
+ android:versionCode="32"
+ android:versionName="1.5.24">
<!-- versionCode and versionName also have to be updated in build.gradle -->
<uses-feature android:name="android.hardware.microphone" android:required="true" />
diff --git a/apps/OboeTester/app/src/main/cpp/NativeAudioContext.cpp b/apps/OboeTester/app/src/main/cpp/NativeAudioContext.cpp
index 5a145d4..ee98132 100644
--- a/apps/OboeTester/app/src/main/cpp/NativeAudioContext.cpp
+++ b/apps/OboeTester/app/src/main/cpp/NativeAudioContext.cpp
@@ -59,7 +59,7 @@
oboe::AudioStream * ActivityContext::getOutputStream() {
for (auto entry : mOboeStreams) {
- oboe::AudioStream *oboeStream = entry.second;
+ oboe::AudioStream *oboeStream = entry.second.get();
if (oboeStream->getDirection() == oboe::Direction::Output) {
return oboeStream;
}
@@ -69,7 +69,7 @@
oboe::AudioStream * ActivityContext::getInputStream() {
for (auto entry : mOboeStreams) {
- oboe::AudioStream *oboeStream = entry.second;
+ oboe::AudioStream *oboeStream = entry.second.get();
if (oboeStream != nullptr) {
if (oboeStream->getDirection() == oboe::Direction::Input) {
return oboeStream;
@@ -80,6 +80,7 @@
}
void ActivityContext::freeStreamIndex(int32_t streamIndex) {
+ mOboeStreams[streamIndex].reset();
mOboeStreams.erase(streamIndex);
}
@@ -88,13 +89,14 @@
}
void ActivityContext::close(int32_t streamIndex) {
+ LOGD("ActivityContext::%s() called for stream %d ", __func__, streamIndex);
stopBlockingIOThread();
oboe::AudioStream *oboeStream = getStream(streamIndex);
+ LOGD("ActivityContext::%s() close stream %p ", __func__, oboeStream);
if (oboeStream != nullptr) {
oboeStream->close();
- freeStreamIndex(streamIndex);
LOGD("ActivityContext::%s() delete stream %d ", __func__, streamIndex);
- delete oboeStream;
+ freeStreamIndex(streamIndex);
}
}
@@ -110,7 +112,7 @@
oboe::Result result = oboe::Result::OK;
stopBlockingIOThread();
for (auto entry : mOboeStreams) {
- oboe::AudioStream *oboeStream = entry.second;
+ oboe::AudioStream *oboeStream = entry.second.get();
result = oboeStream->requestPause();
printScheduler();
}
@@ -123,8 +125,8 @@
LOGD("ActivityContext::stopAllStreams() called");
for (auto entry : mOboeStreams) {
LOGD("ActivityContext::stopAllStreams() handle = %d, stream %p",
- entry.first, entry.second);
- oboe::AudioStream *oboeStream = entry.second;
+ entry.first, entry.second.get());
+ oboe::AudioStream *oboeStream = entry.second.get();
result = oboeStream->requestStop();
printScheduler();
}
@@ -204,18 +206,16 @@
AAudioExtensions::getInstance().setMMapEnabled(isMMap);
// Open a stream based on the builder settings.
- oboe::AudioStream *oboeStream = nullptr;
- oboe::Result result = builder.openStream(&oboeStream);
+ std::shared_ptr<oboe::AudioStream> oboeStream;
+ Result result = builder.openStream(oboeStream);
LOGD("ActivityContext::open() builder.openStream() returned %d, oboeStream = %p",
- result, oboeStream);
+ result, oboeStream.get());
AAudioExtensions::getInstance().setMMapEnabled(oldMMapEnabled);
- if (result != oboe::Result::OK) {
- delete oboeStream;
- oboeStream = nullptr;
+ if (result != Result::OK) {
freeStreamIndex(streamIndex);
streamIndex = -1;
} else {
- mOboeStreams[streamIndex] = oboeStream;
+ mOboeStreams[streamIndex] = oboeStream; // save shared_ptr
mChannelCount = oboeStream->getChannelCount(); // FIXME store per stream
mFramesPerBurst = oboeStream->getFramesPerBurst();
@@ -223,17 +223,15 @@
createRecording();
- finishOpen(isInput, oboeStream);
+ finishOpen(isInput, oboeStream.get());
}
-
if (!mUseCallback) {
int numSamples = getFramesPerBlock() * mChannelCount;
dataBuffer = std::make_unique<float[]>(numSamples);
}
-
- return ((int)result < 0) ? (int)result : streamIndex;
+ return (result != Result::OK) ? (int)result : streamIndex;
}
oboe::Result ActivityContext::start() {
@@ -606,6 +604,7 @@
// =================================================================== ActivityTestDisconnect
void ActivityTestDisconnect::close(int32_t streamIndex) {
+ LOGD("ActivityTestDisconnect::%s() called for stream %d ", __func__, streamIndex);
ActivityContext::close(streamIndex);
mSinkFloat.reset();
}
diff --git a/apps/OboeTester/app/src/main/cpp/NativeAudioContext.h b/apps/OboeTester/app/src/main/cpp/NativeAudioContext.h
index c6965f1..b86d50a 100644
--- a/apps/OboeTester/app/src/main/cpp/NativeAudioContext.h
+++ b/apps/OboeTester/app/src/main/cpp/NativeAudioContext.h
@@ -194,7 +194,7 @@
oboe::AudioStream *getStream(int32_t streamIndex) {
auto it = mOboeStreams.find(streamIndex);
if (it != mOboeStreams.end()) {
- return it->second;
+ return it->second.get();
} else {
return nullptr;
}
@@ -329,7 +329,7 @@
std::unique_ptr<MultiChannelRecording> mRecording{};
int32_t mNextStreamHandle = 0;
- std::unordered_map<int32_t, oboe::AudioStream *> mOboeStreams;
+ std::unordered_map<int32_t, std::shared_ptr<oboe::AudioStream>> mOboeStreams;
int32_t mFramesPerBurst = 0; // TODO per stream
int32_t mChannelCount = 0; // TODO per stream
int32_t mSampleRate = 0; // TODO per stream
diff --git a/apps/OboeTester/app/src/main/cpp/OboeStreamCallbackProxy.cpp b/apps/OboeTester/app/src/main/cpp/OboeStreamCallbackProxy.cpp
index ab8f45b..ec25935 100644
--- a/apps/OboeTester/app/src/main/cpp/OboeStreamCallbackProxy.cpp
+++ b/apps/OboeTester/app/src/main/cpp/OboeStreamCallbackProxy.cpp
@@ -85,6 +85,8 @@
if (mCallback != nullptr) {
mCallback->onErrorBeforeClose(audioStream, error);
}
+ // usleep(2000 * 1000); // FIXME - sleep to provoke a race condition
+ LOGD("OboeStreamCallbackProxy::%s(%p, %d) returning after sleep", __func__, audioStream, error);
}
void OboeStreamCallbackProxy::onErrorAfterClose(oboe::AudioStream *audioStream, oboe::Result error) {
diff --git a/include/oboe/AudioStream.h b/include/oboe/AudioStream.h
index d9b2046..faee05b 100644
--- a/include/oboe/AudioStream.h
+++ b/include/oboe/AudioStream.h
@@ -42,6 +42,7 @@
* Base class for Oboe C++ audio stream.
*/
class AudioStream : public AudioStreamBase {
+ friend class AudioStreamBuilder; // allow access to setWeakThis() and lockWeakThis()
public:
AudioStream() {}
@@ -479,6 +480,23 @@
mDataCallbackEnabled = enabled;
}
+ /*
+ * Set a weak_ptr to this stream from the shared_ptr so that we can
+ * later use a shared_ptr in the error callback.
+ */
+ void setWeakThis(std::shared_ptr<oboe::AudioStream> &sharedStream) {
+ mWeakThis = sharedStream;
+ }
+
+ /*
+ * Make a shared_ptr that will prevent this stream from being deleted.
+ */
+ std::shared_ptr<oboe::AudioStream> lockWeakThis() {
+ return mWeakThis.lock();
+ }
+
+ std::weak_ptr<AudioStream> mWeakThis; // weak pointer to this object
+
/**
* Number of frames which have been written into the stream
*
@@ -497,17 +515,17 @@
std::mutex mLock; // for synchronizing start/stop/close
+
private:
int mPreviousScheduler = -1;
std::atomic<bool> mDataCallbackEnabled{false};
std::atomic<bool> mErrorCallbackCalled{false};
-
};
/**
- * This struct is a stateless functor which closes a audiostream prior to its deletion.
+ * This struct is a stateless functor which closes an AudioStream prior to its deletion.
* This means it can be used to safely delete a smart pointer referring to an open stream.
*/
struct StreamDeleterFunctor {
diff --git a/include/oboe/AudioStreamBase.h b/include/oboe/AudioStreamBase.h
index fd327e6..4a6237d 100644
--- a/include/oboe/AudioStreamBase.h
+++ b/include/oboe/AudioStreamBase.h
@@ -27,6 +27,7 @@
* Base class containing parameters for audio streams and builders.
**/
class AudioStreamBase {
+
public:
AudioStreamBase() {}
diff --git a/include/oboe/AudioStreamBuilder.h b/include/oboe/AudioStreamBuilder.h
index b0f7de8..a1ea0a8 100644
--- a/include/oboe/AudioStreamBuilder.h
+++ b/include/oboe/AudioStreamBuilder.h
@@ -19,12 +19,14 @@
#include "oboe/Definitions.h"
#include "oboe/AudioStreamBase.h"
+#include "ResultWithValue.h"
namespace oboe {
// This depends on AudioStream, so we use forward declaration, it will close and delete the stream
struct StreamDeleterFunctor;
using ManagedStream = std::unique_ptr<AudioStream, StreamDeleterFunctor>;
+
/**
* Factory class for an audio Stream.
*/
@@ -382,11 +384,23 @@
*
* The caller owns the pointer to the AudioStream object.
*
+ * @deprecated Use openStream(std::shared_ptr<oboe::AudioStream> &stream) instead.
* @param stream pointer to a variable to receive the stream address
* @return OBOE_OK if successful or a negative error code
*/
Result openStream(AudioStream **stream);
+ /**
+ * Create and open a stream object based on the current settings.
+ *
+ * The caller shares the pointer to the AudioStream object.
+ * The shared_ptr is used internally by Oboe to prevent the stream from being
+ * deleted while it is being used by callbacks.
+ *
+ * @param stream reference to a shared_ptr to receive the stream address
+ * @return OBOE_OK if successful or a negative error code
+ */
+ Result openStream(std::shared_ptr<oboe::AudioStream> &stream);
/**
* Create and open a ManagedStream object based on the current builder state.
diff --git a/include/oboe/Version.h b/include/oboe/Version.h
index 69d7f46..5aed55b 100644
--- a/include/oboe/Version.h
+++ b/include/oboe/Version.h
@@ -34,10 +34,10 @@
#define OBOE_VERSION_MAJOR 1
// Type: 8-bit unsigned int. Min value: 0 Max value: 255. See below for description.
-#define OBOE_VERSION_MINOR 3
+#define OBOE_VERSION_MINOR 4
// Type: 16-bit unsigned int. Min value: 0 Max value: 65535. See below for description.
-#define OBOE_VERSION_PATCH 3
+#define OBOE_VERSION_PATCH 0
#define OBOE_STRINGIFY(x) #x
#define OBOE_TOSTRING(x) OBOE_STRINGIFY(x)
diff --git a/src/aaudio/AudioStreamAAudio.cpp b/src/aaudio/AudioStreamAAudio.cpp
index b19ec91..383e9f6 100644
--- a/src/aaudio/AudioStreamAAudio.cpp
+++ b/src/aaudio/AudioStreamAAudio.cpp
@@ -74,6 +74,17 @@
LOGD("%s() - exiting <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<", __func__);
}
+// This runs in its own thread.
+// Only one of these threads will be launched from internalErrorCallback().
+// Prevents deletion of the stream if the app is using AudioStreamBuilder::openSharedStream()
+static void oboe_aaudio_error_thread_proc_shared(std::shared_ptr<AudioStream> sharedStream,
+ Result error) {
+ LOGD("%s() - entering >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>", __func__);
+ AudioStreamAAudio *oboeStream = reinterpret_cast<AudioStreamAAudio*>(sharedStream.get());
+ oboe_aaudio_error_thread_proc(oboeStream, error);
+ LOGD("%s() - exiting <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<", __func__);
+}
+
namespace oboe {
/*
@@ -101,12 +112,21 @@
void *userData,
aaudio_result_t error) {
AudioStreamAAudio *oboeStream = reinterpret_cast<AudioStreamAAudio*>(userData);
+
+ // Prevents deletion of the stream if the app is using AudioStreamBuilder::openSharedStream()
+ std::shared_ptr<AudioStream> sharedStream = oboeStream->lockWeakThis();
+
// These checks should be enough because we assume that the stream close()
// will join() any active callback threads and will not allow new callbacks.
if (oboeStream->wasErrorCallbackCalled()) { // block extra error callbacks
LOGE("%s() multiple error callbacks called!", __func__);
} else if (stream != oboeStream->getUnderlyingStream()) {
LOGD("%s() stream already closed", __func__); // can happen if there are bugs
+ } else if (sharedStream) {
+ // Handle error on a separate thread using shared pointer.
+ std::thread t(oboe_aaudio_error_thread_proc_shared, sharedStream,
+ static_cast<Result>(error));
+ t.detach();
} else {
// Handle error on a separate thread.
std::thread t(oboe_aaudio_error_thread_proc, oboeStream,
diff --git a/src/common/AudioStreamBuilder.cpp b/src/common/AudioStreamBuilder.cpp
index 9d44625..a94e54e 100644
--- a/src/common/AudioStreamBuilder.cpp
+++ b/src/common/AudioStreamBuilder.cpp
@@ -186,4 +186,16 @@
return result;
}
-} // namespace oboe
\ No newline at end of file
+Result AudioStreamBuilder::openStream(std::shared_ptr<AudioStream> &sharedStream) {
+ sharedStream.reset();
+ AudioStream *streamptr;
+ auto result = openStream(&streamptr);
+ if (result == Result::OK) {
+ sharedStream.reset(streamptr);
+ // Save a weak_ptr in the stream for use with callbacks.
+ streamptr->setWeakThis(sharedStream);
+ }
+ return result;
+}
+
+} // namespace oboe