Skip to content

Commit

Permalink
Use shared_ptr in FilterAudioStream
Browse files Browse the repository at this point in the history
This is used when Oboe does sample rate conversion. Or format or channel count conversion.

It used to use a raw pointer, which could cause crashes.

Fixes #2035
  • Loading branch information
philburk committed Jun 12, 2024
1 parent 192eaaa commit abdc015
Show file tree
Hide file tree
Showing 5 changed files with 32 additions and 16 deletions.
2 changes: 1 addition & 1 deletion include/oboe/AudioStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ class AudioStream : public AudioStreamBase {
*/
explicit AudioStream(const AudioStreamBuilder &builder);

virtual ~AudioStream() = default;
virtual ~AudioStream();

/**
* Open a stream based on the current settings.
Expand Down
24 changes: 18 additions & 6 deletions src/aaudio/AudioStreamAAudio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -60,9 +60,13 @@ static aaudio_data_callback_result_t oboe_aaudio_data_callback_proc(
// This runs in its own thread.
// Only one of these threads will be launched from internalErrorCallback().
// It calls app error callbacks from a static function in case the stream gets deleted.
static void oboe_aaudio_error_thread_proc(AudioStreamAAudio *oboeStream,
static void oboe_aaudio_error_thread_proc_common(AudioStreamAAudio *oboeStream,
Result error) {
LOGD("%s(,%d) - entering >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>", __func__, error);
#if 0
LOGE("%s() sleep for 5 seconds", __func__);
usleep(5*1000*1000);
LOGD("%s() - woke up -------------------------", __func__);
#endif
AudioStreamErrorCallback *errorCallback = oboeStream->getErrorCallback();
if (errorCallback == nullptr) return; // should be impossible
bool isErrorHandled = errorCallback->onError(oboeStream, error);
Expand All @@ -74,16 +78,24 @@ static void oboe_aaudio_error_thread_proc(AudioStreamAAudio *oboeStream,
// Warning, oboeStream may get deleted by this callback.
errorCallback->onErrorAfterClose(oboeStream, error);
}
}

// Callback thread for raw pointers.
static void oboe_aaudio_error_thread_proc(AudioStreamAAudio *oboeStream,
Result error) {
LOGD("%s(,%d) - entering >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>", __func__, error);
oboe_aaudio_error_thread_proc_common(oboeStream, error);
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()
// Callback thread for shared pointers.
static void oboe_aaudio_error_thread_proc_shared(std::shared_ptr<AudioStream> sharedStream,
Result error) {
LOGD("%s(,%d) - entering >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>", __func__, error);
// Hold the shared pointer while we use the raw pointer.
AudioStreamAAudio *oboeStream = reinterpret_cast<AudioStreamAAudio*>(sharedStream.get());
oboe_aaudio_error_thread_proc(oboeStream, error);
oboe_aaudio_error_thread_proc_common(oboeStream, error);
LOGD("%s() - exiting <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<", __func__);
}

namespace oboe {
Expand Down
6 changes: 6 additions & 0 deletions src/common/AudioStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,12 @@ namespace oboe {
*/
AudioStream::AudioStream(const AudioStreamBuilder &builder)
: AudioStreamBase(builder) {
LOGD("Constructor for AudioStream at %p", this);
}

AudioStream::~AudioStream() {
// This is to help debug use after free bugs.
LOGD("Destructor for AudioStream at %p", this);
}

Result AudioStream::close() {
Expand Down
6 changes: 4 additions & 2 deletions src/common/AudioStreamBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ Result AudioStreamBuilder::openStreamInternal(AudioStream **streamPP) {
// Do we need to make a child stream and convert.
if (conversionNeeded) {
AudioStream *tempStream;
result = childBuilder.openStream(&tempStream);
result = childBuilder.openStreamInternal(&tempStream);
if (result != Result::OK) {
return result;
}
Expand All @@ -144,7 +144,9 @@ Result AudioStreamBuilder::openStreamInternal(AudioStream **streamPP) {

// Use childStream in a FilterAudioStream.
LOGI("%s() create a FilterAudioStream for data conversion.", __func__);
FilterAudioStream *filterStream = new FilterAudioStream(parentBuilder, tempStream);
std::shared_ptr<AudioStream> childStream(tempStream);
FilterAudioStream *filterStream = new FilterAudioStream(parentBuilder, childStream);
childStream->setWeakThis(childStream);
result = filterStream->configureFlowGraph();
if (result != Result::OK) {
filterStream->close();
Expand Down
10 changes: 3 additions & 7 deletions src/common/FilterAudioStream.h
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ class FilterAudioStream : public AudioStream, AudioStreamCallback {
*
* @param builder containing all the stream's attributes
*/
FilterAudioStream(const AudioStreamBuilder &builder, AudioStream *childStream)
FilterAudioStream(const AudioStreamBuilder &builder, std::shared_ptr<AudioStream> childStream)
: AudioStream(builder)
, mChildStream(childStream) {
, mChildStream(childStream) {
// Intercept the callback if used.
if (builder.isErrorCallbackSpecified()) {
mErrorCallback = mChildStream->swapErrorCallback(this);
Expand All @@ -66,10 +66,6 @@ class FilterAudioStream : public AudioStream, AudioStreamCallback {

virtual ~FilterAudioStream() = default;

AudioStream *getChildStream() const {
return mChildStream.get();
}

Result configureFlowGraph();

// Close child and parent.
Expand Down Expand Up @@ -216,7 +212,7 @@ class FilterAudioStream : public AudioStream, AudioStreamCallback {

private:

std::unique_ptr<AudioStream> mChildStream; // this stream wraps the child stream
std::shared_ptr<AudioStream> mChildStream; // this stream wraps the child stream
std::unique_ptr<DataConversionFlowGraph> mFlowGraph; // for converting data
std::unique_ptr<uint8_t[]> mBlockingBuffer; // temp buffer for write()
double mRateScaler = 1.0; // ratio parent/child sample rates
Expand Down

0 comments on commit abdc015

Please sign in to comment.