FullDuplexStream: deprecate raw pointers when setting streams (#2034)

google · Jun 25, 2024 · fae46d4 · fae46d4
1 parent 03242e9
commit fae46d4
Show file tree

Hide file tree

Showing 9 changed files with 84 additions and 67 deletions.
diff --git a/apps/OboeTester/app/src/main/cpp/NativeAudioContext.cpp b/apps/OboeTester/app/src/main/cpp/NativeAudioContext.cpp
@@ -248,7 +248,7 @@ int ActivityContext::open(jint nativeApi,
 
         createRecording();
 
-        finishOpen(isInput, oboeStream.get());
+        finishOpen(isInput, oboeStream);
     }
 
     if (!mUseCallback) {
@@ -652,11 +652,11 @@ void ActivityEcho::configureBuilder(bool isInput, oboe::AudioStreamBuilder &buil
     }
 }
 
-void ActivityEcho::finishOpen(bool isInput, oboe::AudioStream *oboeStream) {
+void ActivityEcho::finishOpen(bool isInput, std::shared_ptr<oboe::AudioStream> &oboeStream) {
     if (isInput) {
-        mFullDuplexEcho->setInputStream(oboeStream);
+        mFullDuplexEcho->setSharedInputStream(oboeStream);
     } else {
-        mFullDuplexEcho->setOutputStream(oboeStream);
+        mFullDuplexEcho->setSharedOutputStream(oboeStream);
     }
 }
 
@@ -674,12 +674,13 @@ void ActivityRoundTripLatency::configureBuilder(bool isInput, oboe::AudioStreamB
     }
 }
 
-void ActivityRoundTripLatency::finishOpen(bool isInput, AudioStream *oboeStream) {
+void ActivityRoundTripLatency::finishOpen(bool isInput, std::shared_ptr<oboe::AudioStream>
+        &oboeStream) {
     if (isInput) {
-        mFullDuplexLatency->setInputStream(oboeStream);
+        mFullDuplexLatency->setSharedInputStream(oboeStream);
         mFullDuplexLatency->setRecording(mRecording.get());
     } else {
-        mFullDuplexLatency->setOutputStream(oboeStream);
+        mFullDuplexLatency->setSharedOutputStream(oboeStream);
     }
 }
 
@@ -729,12 +730,12 @@ void ActivityGlitches::configureBuilder(bool isInput, oboe::AudioStreamBuilder &
     }
 }
 
-void ActivityGlitches::finishOpen(bool isInput, oboe::AudioStream *oboeStream) {
+void ActivityGlitches::finishOpen(bool isInput, std::shared_ptr<oboe::AudioStream> &oboeStream) {
     if (isInput) {
-        mFullDuplexGlitches->setInputStream(oboeStream);
+        mFullDuplexGlitches->setSharedInputStream(oboeStream);
         mFullDuplexGlitches->setRecording(mRecording.get());
     } else {
-        mFullDuplexGlitches->setOutputStream(oboeStream);
+        mFullDuplexGlitches->setSharedOutputStream(oboeStream);
     }
 }
 
@@ -752,12 +753,12 @@ void ActivityDataPath::configureBuilder(bool isInput, oboe::AudioStreamBuilder &
     }
 }
 
-void ActivityDataPath::finishOpen(bool isInput, oboe::AudioStream *oboeStream) {
+void ActivityDataPath::finishOpen(bool isInput, std::shared_ptr<oboe::AudioStream> &oboeStream) {
     if (isInput) {
-        mFullDuplexDataPath->setInputStream(oboeStream);
+        mFullDuplexDataPath->setSharedInputStream(oboeStream);
         mFullDuplexDataPath->setRecording(mRecording.get());
     } else {
-        mFullDuplexDataPath->setOutputStream(oboeStream);
+        mFullDuplexDataPath->setSharedOutputStream(oboeStream);
     }
 }
 

diff --git a/apps/OboeTester/app/src/main/cpp/NativeAudioContext.h b/apps/OboeTester/app/src/main/cpp/NativeAudioContext.h
@@ -311,7 +311,7 @@ class ActivityContext {
                                                              SECONDS_TO_RECORD * mSampleRate);
     }
 
-    virtual void finishOpen(bool isInput, oboe::AudioStream *oboeStream) {}
+    virtual void finishOpen(bool isInput, std::shared_ptr<oboe::AudioStream> &oboeStream) {}
 
     virtual oboe::Result startStreams() = 0;
 
@@ -544,7 +544,7 @@ class ActivityEcho : public ActivityFullDuplex {
     }
 
 protected:
-    void finishOpen(bool isInput, oboe::AudioStream *oboeStream) override;
+    void finishOpen(bool isInput, std::shared_ptr<oboe::AudioStream> &oboeStream) override;
 
 private:
     std::unique_ptr<FullDuplexEcho>   mFullDuplexEcho{};
@@ -616,7 +616,7 @@ class ActivityRoundTripLatency : public ActivityFullDuplex {
     jdouble measureTimestampLatency();
 
 protected:
-    void finishOpen(bool isInput, oboe::AudioStream *oboeStream) override;
+    void finishOpen(bool isInput, std::shared_ptr<oboe::AudioStream> &oboeStream) override;
 
 private:
     std::unique_ptr<FullDuplexAnalyzer>   mFullDuplexLatency{};
@@ -658,7 +658,7 @@ class ActivityGlitches : public ActivityFullDuplex {
     }
 
 protected:
-    void finishOpen(bool isInput, oboe::AudioStream *oboeStream) override;
+    void finishOpen(bool isInput, std::shared_ptr<oboe::AudioStream> &oboeStream) override;
 
 private:
     std::unique_ptr<FullDuplexAnalyzer>   mFullDuplexGlitches{};
@@ -700,7 +700,7 @@ class ActivityDataPath : public ActivityFullDuplex {
     }
 
 protected:
-    void finishOpen(bool isInput, oboe::AudioStream *oboeStream) override;
+    void finishOpen(bool isInput, std::shared_ptr<oboe::AudioStream> &oboeStream) override;
 
 private:
     std::unique_ptr<FullDuplexAnalyzer>   mFullDuplexDataPath{};

diff --git a/include/oboe/FullDuplexStream.h b/include/oboe/FullDuplexStream.h
@@ -53,39 +53,69 @@ class FullDuplexStream : public AudioStreamDataCallback {
     virtual ~FullDuplexStream() = default;
 
     /**
-     * Sets the input stream. Calling this is mandatory.
+     * Sets the input stream.
      *
+     * @deprecated Call setSharedInputStream(std::shared_ptr<AudioStream> &stream) instead.
      * @param stream the output stream
      */
     void setInputStream(AudioStream *stream) {
-        mInputStream = stream;
+        mRawInputStream = stream;
+    }
+
+    /**
+     * Sets the input stream. Calling this is mandatory.
+     *
+     * @param stream the output stream
+     */
+    void setSharedInputStream(std::shared_ptr<AudioStream> &stream) {
+        mSharedInputStream = stream;
     }
 
     /**
-     * Gets the input stream
+     * Gets the current input stream. This function tries to return the shared input stream if it
+     * is set before the raw input stream.
      *
-     * @return the input stream
+     * @return pointer to an output stream or nullptr.
      */
     AudioStream *getInputStream() {
-        return mInputStream;
+        if (mSharedInputStream) {
+            return mSharedInputStream.get();
+        } else {
+            return mRawInputStream;
+        }
     }
 
     /**
-     * Sets the output stream. Calling this is mandatory.
+     * Sets the output stream.
      *
+     * @deprecated Call setSharedOutputStream(std::shared_ptr<AudioStream> &stream) instead.
      * @param stream the output stream
      */
     void setOutputStream(AudioStream *stream) {
-        mOutputStream = stream;
+        mRawOutputStream = stream;
+    }
+
+    /**
+     * Sets the output stream. Calling this is mandatory.
+     *
+     * @param stream the output stream
+     */
+    void setSharedOutputStream(std::shared_ptr<AudioStream> &stream) {
+        mSharedOutputStream = stream;
     }
 
     /**
-     * Gets the output stream
+     * Gets the current output stream. This function tries to return the shared output stream if it
+     * is set before the raw output stream.
      *
-     * @return the output stream
+     * @return pointer to an output stream or nullptr.
      */
     AudioStream *getOutputStream() {
-        return mOutputStream;
+        if (mSharedOutputStream) {
+            return mSharedOutputStream.get();
+        } else {
+            return mRawOutputStream;
+        }
     }
 
     /**
@@ -123,10 +153,10 @@ class FullDuplexStream : public AudioStreamDataCallback {
         Result outputResult = Result::OK;
         Result inputResult = Result::OK;
         if (getOutputStream()) {
-            outputResult = mOutputStream->requestStop();
+            outputResult = getOutputStream()->requestStop();
         }
         if (getInputStream()) {
-            inputResult = mInputStream->requestStop();
+            inputResult = getOutputStream()->requestStop();
         }
         if (outputResult != Result::OK) {
             return outputResult;
@@ -312,8 +342,10 @@ class FullDuplexStream : public AudioStreamDataCallback {
     // Discard some callbacks so the input and output reach equilibrium.
     int32_t              mCountCallbacksToDiscard = kNumCallbacksToDiscard;
 
-    AudioStream   *mInputStream = nullptr;
-    AudioStream   *mOutputStream = nullptr;
+    AudioStream *mRawInputStream = nullptr;
+    AudioStream *mRawOutputStream = nullptr;
+    std::shared_ptr<AudioStream> mSharedInputStream;
+    std::shared_ptr<AudioStream> mSharedOutputStream;
 
     int32_t              mBufferSize = 0;
     std::unique_ptr<float[]> mInputBuffer;

diff --git a/samples/LiveEffect/src/main/cpp/LiveEffectEngine.cpp b/samples/LiveEffect/src/main/cpp/LiveEffectEngine.cpp
@@ -47,11 +47,9 @@ bool LiveEffectEngine::setEffectOn(bool isOn) {
         if (isOn) {
             success = openStreams() == oboe::Result::OK;
             if (success) {
-                mFullDuplexPass.start();
                 mIsEffectOn = isOn;
             }
         } else {
-            mFullDuplexPass.stop();
             closeStreams();
             mIsEffectOn = isOn;
        }
@@ -68,11 +66,10 @@ void LiveEffectEngine::closeStreams() {
     * which would cause the app to crash since the recording stream would be
     * null.
     */
+    mDuplexStream->stop();
     closeStream(mPlayStream);
-    mFullDuplexPass.setOutputStream(nullptr);
-
     closeStream(mRecordingStream);
-    mFullDuplexPass.setInputStream(nullptr);
+    mDuplexStream.reset();
 }
 
 oboe::Result  LiveEffectEngine::openStreams() {
@@ -102,8 +99,10 @@ oboe::Result  LiveEffectEngine::openStreams() {
     }
     warnIfNotLowLatency(mRecordingStream);
 
-    mFullDuplexPass.setInputStream(mRecordingStream.get());
-    mFullDuplexPass.setOutputStream(mPlayStream.get());
+    mDuplexStream = std::make_unique<FullDuplexPass>();
+    mDuplexStream->setSharedInputStream(mRecordingStream);
+    mDuplexStream->setSharedOutputStream(mPlayStream);
+    mDuplexStream->start();
     return result;
 }
 
@@ -208,7 +207,7 @@ void LiveEffectEngine::warnIfNotLowLatency(std::shared_ptr<oboe::AudioStream> &s
  */
 oboe::DataCallbackResult LiveEffectEngine::onAudioReady(
     oboe::AudioStream *oboeStream, void *audioData, int32_t numFrames) {
-    return mFullDuplexPass.onAudioReady(oboeStream, audioData, numFrames);
+    return mDuplexStream->onAudioReady(oboeStream, audioData, numFrames);
 }
 
 /**
@@ -236,19 +235,11 @@ void LiveEffectEngine::onErrorAfterClose(oboe::AudioStream *oboeStream,
          oboe::convertToText(oboeStream->getDirection()),
          oboe::convertToText(error));
 
-    // Stop the Full Duplex stream.
-    // Since the error callback occurs only for the output stream, close the input stream.
-    mFullDuplexPass.stop();
-    mFullDuplexPass.setOutputStream(nullptr);
-    closeStream(mRecordingStream);
-    mFullDuplexPass.setInputStream(nullptr);
+    closeStreams();
 
     // Restart the stream if the error is a disconnect.
     if (error == oboe::Result::ErrorDisconnected) {
         LOGI("Restarting AudioStream");
-        oboe::Result result = openStreams();
-        if (result == oboe::Result::OK) {
-            mFullDuplexPass.start();
-        }
+        openStreams();
     }
 }
diff --git a/samples/LiveEffect/src/main/cpp/LiveEffectEngine.h b/samples/LiveEffect/src/main/cpp/LiveEffectEngine.h
@@ -52,7 +52,6 @@ class LiveEffectEngine : public oboe::AudioStreamCallback {
     bool isAAudioRecommended(void);
 
 private:
-    FullDuplexPass    mFullDuplexPass;
     bool              mIsEffectOn = false;
     int32_t           mRecordingDeviceId = oboe::kUnspecified;
     int32_t           mPlaybackDeviceId = oboe::kUnspecified;
@@ -62,6 +61,7 @@ class LiveEffectEngine : public oboe::AudioStreamCallback {
     const int32_t     mInputChannelCount = oboe::ChannelCount::Stereo;
     const int32_t     mOutputChannelCount = oboe::ChannelCount::Stereo;
 
+    std::unique_ptr<FullDuplexPass> mDuplexStream;
     std::shared_ptr<oboe::AudioStream> mRecordingStream;
     std::shared_ptr<oboe::AudioStream> mPlayStream;
 

diff --git a/tests/testFullDuplexStream.cpp b/tests/testFullDuplexStream.cpp
@@ -55,7 +55,7 @@ class TestFullDuplexStream : public ::testing::Test,
         mOutputBuilder.setFormat(AudioFormat::Float);
         mOutputBuilder.setDataCallback(this);
 
-        Result r = mOutputBuilder.openStream(&mOutputStream);
+        Result r = mOutputBuilder.openStream(mOutputStream);
         ASSERT_EQ(r, Result::OK) << "Failed to open output stream " << convertToText(r);
 
         mInputBuilder.setDirection(Direction::Input);
@@ -68,11 +68,11 @@ class TestFullDuplexStream : public ::testing::Test,
         mInputBuilder.setBufferCapacityInFrames(mOutputStream->getBufferCapacityInFrames() * 2);
         mInputBuilder.setSampleRate(mOutputStream->getSampleRate());
 
-        r = mInputBuilder.openStream(&mInputStream);
+        r = mInputBuilder.openStream(mInputStream);
         ASSERT_EQ(r, Result::OK) << "Failed to open input stream " << convertToText(r);
 
-        setInputStream(mInputStream);
-        setOutputStream(mOutputStream);
+        setSharedInputStream(mInputStream);
+        setSharedOutputStream(mOutputStream);
     }
 
     void startStream() {
@@ -88,10 +88,8 @@ class TestFullDuplexStream : public ::testing::Test,
     void closeStream() {
         Result r = mOutputStream->close();
         ASSERT_EQ(r, Result::OK) << "Failed to close output stream " << convertToText(r);
-        setOutputStream(nullptr);
         r = mInputStream->close();
         ASSERT_EQ(r, Result::OK) << "Failed to close input stream " << convertToText(r);
-        setInputStream(nullptr);
     }
 
     void checkXRuns() {
@@ -107,8 +105,8 @@ class TestFullDuplexStream : public ::testing::Test,
 
     AudioStreamBuilder mInputBuilder;
     AudioStreamBuilder mOutputBuilder;
-    AudioStream *mInputStream = nullptr;
-    AudioStream *mOutputStream = nullptr;
+    std::shared_ptr<AudioStream> mInputStream;
+    std::shared_ptr<AudioStream> mOutputStream;
     std::atomic<int32_t> mCallbackCount{0};
     std::atomic<int32_t> mGoodCallbackCount{0};
 };

diff --git a/tests/testReturnStop.cpp b/tests/testReturnStop.cpp
@@ -58,9 +58,8 @@ class StreamReturnStop : public ::testing::Test,
 };
 
 void StreamReturnStop::TearDown() {
-    if (mStream != nullptr) {
+    if (mStream) {
         mStream->close();
-        mStream = nullptr;
     }
 }
 
@@ -78,7 +77,6 @@ TEST_P(StreamReturnStop, VerifyStreamReturnStop) {
     if (mBuilder.isAAudioRecommended()) {
         mBuilder.setAudioApi(audioApi);
     }
-    mStream = nullptr;
     Result r = mBuilder.openStream(mStream);
     ASSERT_EQ(r, Result::OK) << "Failed to open stream. " << convertToText(r);
 

diff --git a/tests/testStreamFramesProcessed.cpp b/tests/testStreamFramesProcessed.cpp
@@ -43,9 +43,8 @@ class StreamFramesProcessed : public ::testing::Test,
 };
 
 void StreamFramesProcessed::TearDown() {
-    if (mStream != nullptr) {
+    if (mStream) {
         mStream->close();
-        mStream = nullptr;
     }
 }
 
@@ -61,7 +60,6 @@ TEST_P(StreamFramesProcessed, VerifyFramesProcessed) {
             ->setPerformanceMode(PerformanceMode::LowLatency)
             ->setSharingMode(SharingMode::Exclusive)
             ->setDataCallback(callback);
-    mStream = nullptr;
     Result r = mBuilder.openStream(mStream);
     ASSERT_EQ(r, Result::OK) << "Failed to open stream." << convertToText(r);