aaudio: call stop() from the callback
Stop the stream when the callback returns AAUDIO_CALLBACK_RESULT_STOP.
Refactor start/pause/stop code to allow different paths for
stop from the app or from the callback.
Fix error handling in joinThread().
Simplify code path through PlayerBase.
Bug: 120932593
Bug: 121158739
Test: test_return_stop
Test: test_return_stop -i
Test: test_return_stop -n
Test: test_return_stop -i -n
Test: atest CtsNativeMediaAAudioTestCases
Change-Id: I85134211348b26077693d9a71bf1331dad60da38
diff --git a/media/libaaudio/src/client/AudioStreamInternal.cpp b/media/libaaudio/src/client/AudioStreamInternal.cpp
index fffcda0..9c682b7 100644
--- a/media/libaaudio/src/client/AudioStreamInternal.cpp
+++ b/media/libaaudio/src/client/AudioStreamInternal.cpp
@@ -349,8 +349,7 @@
}
}
-aaudio_result_t AudioStreamInternal::requestStop()
-{
+aaudio_result_t AudioStreamInternal::requestStop() {
aaudio_result_t result = stopCallback();
if (result != AAUDIO_OK) {
return result;
diff --git a/media/libaaudio/src/client/AudioStreamInternalCapture.cpp b/media/libaaudio/src/client/AudioStreamInternalCapture.cpp
index 58ef7b1..7dcb620 100644
--- a/media/libaaudio/src/client/AudioStreamInternalCapture.cpp
+++ b/media/libaaudio/src/client/AudioStreamInternalCapture.cpp
@@ -259,6 +259,7 @@
if (callbackResult == AAUDIO_CALLBACK_RESULT_STOP) {
ALOGD("%s(): callback returned AAUDIO_CALLBACK_RESULT_STOP", __func__);
+ result = systemStopFromCallback();
break;
}
}
diff --git a/media/libaaudio/src/client/AudioStreamInternalPlay.cpp b/media/libaaudio/src/client/AudioStreamInternalPlay.cpp
index 9af47b2..0884fca 100644
--- a/media/libaaudio/src/client/AudioStreamInternalPlay.cpp
+++ b/media/libaaudio/src/client/AudioStreamInternalPlay.cpp
@@ -294,6 +294,7 @@
}
} else if (callbackResult == AAUDIO_CALLBACK_RESULT_STOP) {
ALOGD("%s(): callback returned AAUDIO_CALLBACK_RESULT_STOP", __func__);
+ result = systemStopFromCallback();
break;
}
}
diff --git a/media/libaaudio/src/core/AAudioAudio.cpp b/media/libaaudio/src/core/AAudioAudio.cpp
index 2fb3986..0d71efc 100644
--- a/media/libaaudio/src/core/AAudioAudio.cpp
+++ b/media/libaaudio/src/core/AAudioAudio.cpp
@@ -316,7 +316,7 @@
{
AudioStream *audioStream = convertAAudioStreamToAudioStream(stream);
ALOGD("%s(%p) called", __func__, stream);
- return audioStream->systemStop();
+ return audioStream->systemStopFromApp();
}
AAUDIO_API aaudio_result_t AAudioStream_waitForStateChange(AAudioStream* stream,
diff --git a/media/libaaudio/src/core/AudioStream.cpp b/media/libaaudio/src/core/AudioStream.cpp
index 391af29..e39a075 100644
--- a/media/libaaudio/src/core/AudioStream.cpp
+++ b/media/libaaudio/src/core/AudioStream.cpp
@@ -119,21 +119,29 @@
return AAUDIO_OK;
}
-aaudio_result_t AudioStream::safeStart() {
+aaudio_result_t AudioStream::systemStart() {
std::lock_guard<std::mutex> lock(mStreamLock);
+
if (collidesWithCallback()) {
ALOGE("%s cannot be called from a callback!", __func__);
return AAUDIO_ERROR_INVALID_STATE;
}
- return requestStart();
+
+ aaudio_result_t result = requestStart();
+ if (result == AAUDIO_OK) {
+ // We only call this for logging in "dumpsys audio". So ignore return code.
+ (void) mPlayerBase->start();
+ }
+ return result;
}
-aaudio_result_t AudioStream::safePause() {
+aaudio_result_t AudioStream::systemPause() {
+ std::lock_guard<std::mutex> lock(mStreamLock);
+
if (!isPauseSupported()) {
return AAUDIO_ERROR_UNIMPLEMENTED;
}
- std::lock_guard<std::mutex> lock(mStreamLock);
if (collidesWithCallback()) {
ALOGE("%s cannot be called from a callback!", __func__);
return AAUDIO_ERROR_INVALID_STATE;
@@ -169,7 +177,12 @@
return AAUDIO_ERROR_INVALID_STATE;
}
- return requestPause();
+ aaudio_result_t result = requestPause();
+ if (result == AAUDIO_OK) {
+ // We only call this for logging in "dumpsys audio". So ignore return code.
+ (void) mPlayerBase->pause();
+ }
+ return result;
}
aaudio_result_t AudioStream::safeFlush() {
@@ -192,12 +205,31 @@
return requestFlush();
}
-aaudio_result_t AudioStream::safeStop() {
+aaudio_result_t AudioStream::systemStopFromCallback() {
+ std::lock_guard<std::mutex> lock(mStreamLock);
+ aaudio_result_t result = safeStop();
+ if (result == AAUDIO_OK) {
+ // We only call this for logging in "dumpsys audio". So ignore return code.
+ (void) mPlayerBase->stop();
+ }
+ return result;
+}
+
+aaudio_result_t AudioStream::systemStopFromApp() {
std::lock_guard<std::mutex> lock(mStreamLock);
if (collidesWithCallback()) {
- ALOGE("stream cannot be stopped from a callback!");
+ ALOGE("stream cannot be stopped by calling from a callback!");
return AAUDIO_ERROR_INVALID_STATE;
}
+ aaudio_result_t result = safeStop();
+ if (result == AAUDIO_OK) {
+ // We only call this for logging in "dumpsys audio". So ignore return code.
+ (void) mPlayerBase->stop();
+ }
+ return result;
+}
+
+aaudio_result_t AudioStream::safeStop() {
switch (getState()) {
// Proceed with stopping.
@@ -224,7 +256,7 @@
case AAUDIO_STREAM_STATE_CLOSING:
case AAUDIO_STREAM_STATE_CLOSED:
default:
- ALOGW("requestStop() stream not running, state = %s",
+ ALOGW("%s() stream not running, state = %s", __func__,
AAudio_convertStreamStateToText(getState()));
return AAUDIO_ERROR_INVALID_STATE;
}
@@ -349,21 +381,33 @@
}
}
-aaudio_result_t AudioStream::joinThread(void** returnArg, int64_t timeoutNanoseconds)
+aaudio_result_t AudioStream::joinThread(void** returnArg, int64_t timeoutNanoseconds __unused)
{
if (!mHasThread) {
ALOGE("joinThread() - but has no thread");
return AAUDIO_ERROR_INVALID_STATE;
}
+ aaudio_result_t result = AAUDIO_OK;
+ // If the callback is stopping the stream because the app passed back STOP
+ // then we don't need to join(). The thread is already about to exit.
+ if (pthread_self() != mThread) {
+ // Called from an app thread. Not the callback.
#if 0
- // TODO implement equivalent of pthread_timedjoin_np()
- struct timespec abstime;
- int err = pthread_timedjoin_np(mThread, returnArg, &abstime);
+ // TODO implement equivalent of pthread_timedjoin_np()
+ struct timespec abstime;
+ int err = pthread_timedjoin_np(mThread, returnArg, &abstime);
#else
- int err = pthread_join(mThread, returnArg);
+ int err = pthread_join(mThread, returnArg);
#endif
+ if (err) {
+ ALOGE("%s() pthread_join() returns err = %d", __func__, err);
+ result = AAudioConvert_androidToAAudioResult(-err);
+ }
+ }
+ // This must be set false so that the callback thread can be created
+ // when the stream is restarted.
mHasThread = false;
- return err ? AAudioConvert_androidToAAudioResult(-errno) : mThreadRegistrationResult;
+ return (result != AAUDIO_OK) ? result : mThreadRegistrationResult;
}
aaudio_data_callback_result_t AudioStream::maybeCallDataCallback(void *audioData,
diff --git a/media/libaaudio/src/core/AudioStream.h b/media/libaaudio/src/core/AudioStream.h
index 60200b2..46951f5 100644
--- a/media/libaaudio/src/core/AudioStream.h
+++ b/media/libaaudio/src/core/AudioStream.h
@@ -51,21 +51,6 @@
virtual ~AudioStream();
- /**
- * Lock a mutex and make sure we are not calling from a callback function.
- * @return result of requestStart();
- */
- aaudio_result_t safeStart();
-
- aaudio_result_t safePause();
-
- aaudio_result_t safeFlush();
-
- aaudio_result_t safeStop();
-
- aaudio_result_t safeClose();
-
- // =========== Begin ABSTRACT methods ===========================
protected:
/* Asynchronous requests.
@@ -74,7 +59,7 @@
virtual aaudio_result_t requestStart() = 0;
/**
- * Check the state to see if Pause if currently legal.
+ * Check the state to see if Pause is currently legal.
*
* @param result pointer to return code
* @return true if OK to continue, if false then return result
@@ -356,33 +341,28 @@
mPlayerBase->unregisterWithAudioManager();
}
- // Pass start request through PlayerBase for tracking.
- aaudio_result_t systemStart() {
- mPlayerBase->start();
- // Pass aaudio_result_t around the PlayerBase interface, which uses status__t.
- return mPlayerBase->getResult();
- }
+ aaudio_result_t systemStart();
- // Pass pause request through PlayerBase for tracking.
- aaudio_result_t systemPause() {
- mPlayerBase->pause();
- return mPlayerBase->getResult();
- }
+ aaudio_result_t systemPause();
- // Pass stop request through PlayerBase for tracking.
- aaudio_result_t systemStop() {
- mPlayerBase->stop();
- return mPlayerBase->getResult();
- }
+ aaudio_result_t safeFlush();
+
+ /**
+ * This is called when an app calls AAudioStream_requestStop();
+ * It prevents calls from a callback.
+ */
+ aaudio_result_t systemStopFromApp();
+
+ /**
+ * This is called internally when an app callback returns AAUDIO_CALLBACK_RESULT_STOP.
+ */
+ aaudio_result_t systemStopFromCallback();
+
+ aaudio_result_t safeClose();
protected:
- // PlayerBase allows the system to control the stream.
- // Calling through PlayerBase->start() notifies the AudioManager of the player state.
- // The AudioManager also can start/stop a stream by calling mPlayerBase->playerStart().
- // systemStart() ==> mPlayerBase->start() mPlayerBase->playerStart() ==> requestStart()
- // \ /
- // ------ AudioManager -------
+ // PlayerBase allows the system to control the stream volume.
class MyPlayerBase : public android::PlayerBase {
public:
explicit MyPlayerBase(AudioStream *parent);
@@ -406,20 +386,19 @@
void clearParentReference() { mParent = nullptr; }
+ // Just a stub. The ability to start audio through PlayerBase is being deprecated.
android::status_t playerStart() override {
- // mParent should NOT be null. So go ahead and crash if it is.
- mResult = mParent->safeStart();
- return AAudioConvert_aaudioToAndroidStatus(mResult);
+ return android::NO_ERROR;
}
+ // Just a stub. The ability to pause audio through PlayerBase is being deprecated.
android::status_t playerPause() override {
- mResult = mParent->safePause();
- return AAudioConvert_aaudioToAndroidStatus(mResult);
+ return android::NO_ERROR;
}
+ // Just a stub. The ability to stop audio through PlayerBase is being deprecated.
android::status_t playerStop() override {
- mResult = mParent->safeStop();
- return AAudioConvert_aaudioToAndroidStatus(mResult);
+ return android::NO_ERROR;
}
android::status_t playerSetVolume() override {
@@ -548,6 +527,8 @@
private:
+ aaudio_result_t safeStop();
+
std::mutex mStreamLock;
const android::sp<MyPlayerBase> mPlayerBase;
diff --git a/media/libaaudio/src/legacy/AudioStreamLegacy.cpp b/media/libaaudio/src/legacy/AudioStreamLegacy.cpp
index a6b9f5d..2edab58 100644
--- a/media/libaaudio/src/legacy/AudioStreamLegacy.cpp
+++ b/media/libaaudio/src/legacy/AudioStreamLegacy.cpp
@@ -78,8 +78,9 @@
void AudioStreamLegacy::processCallbackCommon(aaudio_callback_operation_t opcode, void *info) {
aaudio_data_callback_result_t callbackResult;
- // This illegal size can be used to tell AudioFlinger to stop calling us.
- // This takes advantage of AudioFlinger killing the stream.
+ // This illegal size can be used to tell AudioRecord or AudioTrack to stop calling us.
+ // This takes advantage of them killing the stream when they see a size out of range.
+ // That is an undocumented behavior.
// TODO add to API in AudioRecord and AudioTrack
const size_t SIZE_STOP_CALLBACKS = SIZE_MAX;
@@ -95,7 +96,7 @@
ALOGW("processCallbackCommon() data, stream disconnected");
audioBuffer->size = SIZE_STOP_CALLBACKS;
} else if (!mCallbackEnabled.load()) {
- ALOGW("processCallbackCommon() stopping because callback disabled");
+ ALOGW("processCallbackCommon() no data because callback disabled");
audioBuffer->size = SIZE_STOP_CALLBACKS;
} else {
if (audioBuffer->frameCount == 0) {
@@ -115,10 +116,16 @@
}
if (callbackResult == AAUDIO_CALLBACK_RESULT_CONTINUE) {
audioBuffer->size = audioBuffer->frameCount * getBytesPerDeviceFrame();
- } else { // STOP or invalid result
- ALOGW("%s() callback requested stop, fake an error", __func__);
- audioBuffer->size = SIZE_STOP_CALLBACKS;
- // Disable the callback just in case AudioFlinger keeps trying to call us.
+ } else {
+ if (callbackResult == AAUDIO_CALLBACK_RESULT_STOP) {
+ ALOGD("%s() callback returned AAUDIO_CALLBACK_RESULT_STOP", __func__);
+ } else {
+ ALOGW("%s() callback returned invalid result = %d",
+ __func__, callbackResult);
+ }
+ audioBuffer->size = 0;
+ systemStopFromCallback();
+ // Disable the callback just in case the system keeps trying to call us.
mCallbackEnabled.store(false);
}
diff --git a/media/libaaudio/src/legacy/AudioStreamTrack.cpp b/media/libaaudio/src/legacy/AudioStreamTrack.cpp
index 1ac2558..c995e99 100644
--- a/media/libaaudio/src/legacy/AudioStreamTrack.cpp
+++ b/media/libaaudio/src/legacy/AudioStreamTrack.cpp
@@ -288,7 +288,7 @@
aaudio_result_t AudioStreamTrack::requestPause() {
if (mAudioTrack.get() == nullptr) {
- ALOGE("requestPause() no AudioTrack");
+ ALOGE("%s() no AudioTrack", __func__);
return AAUDIO_ERROR_INVALID_STATE;
}
@@ -304,7 +304,7 @@
aaudio_result_t AudioStreamTrack::requestFlush() {
if (mAudioTrack.get() == nullptr) {
- ALOGE("requestFlush() no AudioTrack");
+ ALOGE("%s() no AudioTrack", __func__);
return AAUDIO_ERROR_INVALID_STATE;
}
@@ -318,7 +318,7 @@
aaudio_result_t AudioStreamTrack::requestStop() {
if (mAudioTrack.get() == nullptr) {
- ALOGE("requestStop() no AudioTrack");
+ ALOGE("%s() no AudioTrack", __func__);
return AAUDIO_ERROR_INVALID_STATE;
}