aaudio: cleaned up PlayerBase implementation
Simplify registration. Reduce use of virtual methods.
This CL, plus some CLs in AudioManager, should fix
crashes involving RefBase::decStrong
Bug: 65450109
Test: CTS or "write_sine_callback -m2 -pl"
Change-Id: Ie1decd20f8d44bdefb0faf92e6b74de4f72e86bd
diff --git a/media/libaaudio/src/client/AudioStreamInternalPlay.h b/media/libaaudio/src/client/AudioStreamInternalPlay.h
index 98783de..d5c1b1e 100644
--- a/media/libaaudio/src/client/AudioStreamInternalPlay.h
+++ b/media/libaaudio/src/client/AudioStreamInternalPlay.h
@@ -50,9 +50,6 @@
return AAUDIO_DIRECTION_OUTPUT;
}
- // Only register client side streams.
- bool needsSystemRegistration() override { return !mInService; }
-
protected:
aaudio_result_t requestPauseInternal();
diff --git a/media/libaaudio/src/core/AAudioAudio.cpp b/media/libaaudio/src/core/AAudioAudio.cpp
index 63569ad..58966e0 100644
--- a/media/libaaudio/src/core/AAudioAudio.cpp
+++ b/media/libaaudio/src/core/AAudioAudio.cpp
@@ -219,7 +219,7 @@
ALOGD("AAudioStreamBuilder_openStream() returns %d = %s for (%p) ----------------",
result, AAudio_convertResultToText(result), audioStream);
if (result == AAUDIO_OK) {
- audioStream->systemRegister();
+ audioStream->registerPlayerBase();
*streamPtr = (AAudioStream*) audioStream;
} else {
*streamPtr = nullptr;
@@ -243,7 +243,7 @@
ALOGD("AAudioStream_close(%p)", stream);
if (audioStream != nullptr) {
audioStream->close();
- audioStream->systemUnRegister();
+ audioStream->unregisterPlayerBase();
delete audioStream;
return AAUDIO_OK;
}
diff --git a/media/libaaudio/src/core/AudioStream.cpp b/media/libaaudio/src/core/AudioStream.cpp
index 35a1c6a..8dcc37a 100644
--- a/media/libaaudio/src/core/AudioStream.cpp
+++ b/media/libaaudio/src/core/AudioStream.cpp
@@ -45,6 +45,8 @@
|| getState() == AAUDIO_STREAM_STATE_DISCONNECTED),
"aaudio stream still in use, state = %s",
AAudio_convertStreamStateToText(getState()));
+
+ mPlayerBase->clearParentReference(); // remove reference to this AudioStream
}
static const char *AudioStream_convertSharingModeToShortText(aaudio_sharing_mode_t sharingMode) {
@@ -197,3 +199,38 @@
return err ? AAudioConvert_androidToAAudioResult(-errno) : mThreadRegistrationResult;
}
+
+#if AAUDIO_USE_VOLUME_SHAPER
+android::media::VolumeShaper::Status AudioStream::applyVolumeShaper(
+ const android::media::VolumeShaper::Configuration& configuration __unused,
+ const android::media::VolumeShaper::Operation& operation __unused) {
+ ALOGW("applyVolumeShaper() is not supported");
+ return android::media::VolumeShaper::Status::ok();
+}
+#endif
+
+AudioStream::MyPlayerBase::MyPlayerBase(AudioStream *parent) : mParent(parent) {
+}
+
+AudioStream::MyPlayerBase::~MyPlayerBase() {
+ ALOGV("MyPlayerBase::~MyPlayerBase(%p) deleted", this);
+}
+
+void AudioStream::MyPlayerBase::registerWithAudioManager() {
+ if (!mRegistered) {
+ init(android::PLAYER_TYPE_AAUDIO, AUDIO_USAGE_MEDIA);
+ mRegistered = true;
+ }
+}
+
+void AudioStream::MyPlayerBase::unregisterWithAudioManager() {
+ if (mRegistered) {
+ baseDestroy();
+ mRegistered = false;
+ }
+}
+
+
+void AudioStream::MyPlayerBase::destroy() {
+ unregisterWithAudioManager();
+}
diff --git a/media/libaaudio/src/core/AudioStream.h b/media/libaaudio/src/core/AudioStream.h
index 5800fa2..34202d2 100644
--- a/media/libaaudio/src/core/AudioStream.h
+++ b/media/libaaudio/src/core/AudioStream.h
@@ -256,29 +256,29 @@
virtual android::status_t doSetVolume() { return android::NO_ERROR; }
#if AAUDIO_USE_VOLUME_SHAPER
- virtual android::binder::Status applyVolumeShaper(
+ virtual ::android::binder::Status applyVolumeShaper(
const ::android::media::VolumeShaper::Configuration& configuration __unused,
- const ::android::media::VolumeShaper::Operation& operation __unused) {
- ALOGW("applyVolumeShaper() is not supported");
- return android::binder::Status::ok();
- }
+ const ::android::media::VolumeShaper::Operation& operation __unused);
#endif
- // Should this object be registered with the AudioManager?
- virtual bool needsSystemRegistration() { return false; }
-
- // Register this stream's PlayerBase with the AudioManager
- void systemRegister() {
- if (needsSystemRegistration()) {
+ /**
+ * Register this stream's PlayerBase with the AudioManager if needed.
+ * Only register output streams.
+ * This should only be called for client streams and not for streams
+ * that run in the service.
+ */
+ void registerPlayerBase() {
+ if (getDirection() == AAUDIO_DIRECTION_OUTPUT) {
mPlayerBase->registerWithAudioManager();
}
}
- // UnRegister this stream's PlayerBase with the AudioManager
- void systemUnRegister() {
- if (needsSystemRegistration()) {
- mPlayerBase->destroy();
- }
+ /**
+ * Unregister this stream's PlayerBase with the AudioManager.
+ * This will only unregister if already registered.
+ */
+ void unregisterPlayerBase() {
+ mPlayerBase->unregisterWithAudioManager();
}
// Pass start request through PlayerBase for tracking.
@@ -308,43 +308,53 @@
// ------ AudioManager -------
class MyPlayerBase : public android::PlayerBase {
public:
- MyPlayerBase(AudioStream *parent) : mParent(parent) {}
- virtual ~MyPlayerBase() {}
+ explicit MyPlayerBase(AudioStream *parent);
- void registerWithAudioManager() {
- init(android::PLAYER_TYPE_AAUDIO, AUDIO_USAGE_MEDIA);
- }
+ virtual ~MyPlayerBase();
- void destroy() override {
- // FIXME what else should this do? close()? disconnect()?
- baseDestroy();
- }
+ /**
+ * Register for volume changes and remote control.
+ */
+ void registerWithAudioManager();
- virtual android::status_t playerStart() {
+ /**
+ * UnRegister.
+ */
+ void unregisterWithAudioManager();
+
+ /**
+ * Just calls unregisterWithAudioManager().
+ */
+ void destroy() override;
+
+ void clearParentReference() { mParent = nullptr; }
+
+ android::status_t playerStart() override {
+ // mParent should NOT be null. So go ahead and crash if it is.
mResult = mParent->requestStart();
return AAudioConvert_aaudioToAndroidStatus(mResult);
}
- virtual android::status_t playerPause() {
+ android::status_t playerPause() override {
mResult = mParent->requestPause();
return AAudioConvert_aaudioToAndroidStatus(mResult);
}
- virtual android::status_t playerStop() {
+ android::status_t playerStop() override {
mResult = mParent->requestStop();
return AAudioConvert_aaudioToAndroidStatus(mResult);
}
- virtual android::status_t playerSetVolume() {
+ android::status_t playerSetVolume() override {
// No pan and only left volume is taken into account from IPLayer interface
mParent->setDuckAndMuteVolume(mVolumeMultiplierL /* * mPanMultiplierL */);
return android::NO_ERROR;
}
#if AAUDIO_USE_VOLUME_SHAPER
- android::binder::Status applyVolumeShaper(
- const android::media::VolumeShaper::Configuration& configuration,
- const android::media::VolumeShaper::Operation& operation) {
+ ::android::binder::Status applyVolumeShaper(
+ const ::android::media::VolumeShaper::Configuration& configuration,
+ const ::android::media::VolumeShaper::Operation& operation) {
return mParent->applyVolumeShaper(configuration, operation);
}
#endif
@@ -353,12 +363,12 @@
return mResult;
}
- AudioStream *mParent;
+ private:
+ AudioStream *mParent;
aaudio_result_t mResult = AAUDIO_OK;
+ bool mRegistered = false;
};
-
-
/**
* This should not be called after the open() call.
*/
diff --git a/media/libaaudio/src/legacy/AudioStreamTrack.cpp b/media/libaaudio/src/legacy/AudioStreamTrack.cpp
index 5e4446f..0e9aaef 100644
--- a/media/libaaudio/src/legacy/AudioStreamTrack.cpp
+++ b/media/libaaudio/src/legacy/AudioStreamTrack.cpp
@@ -478,6 +478,9 @@
}
#if AAUDIO_USE_VOLUME_SHAPER
+
+using namespace android::media::VolumeShaper;
+
binder::Status AudioStreamTrack::applyVolumeShaper(
const VolumeShaper::Configuration& configuration,
const VolumeShaper::Operation& operation) {
@@ -487,7 +490,7 @@
if (mAudioTrack.get() != nullptr) {
ALOGD("applyVolumeShaper() from IPlayer");
- VolumeShaper::Status status = mAudioTrack->applyVolumeShaper(spConfiguration, spOperation);
+ binder::Status status = mAudioTrack->applyVolumeShaper(spConfiguration, spOperation);
if (status < 0) { // a non-negative value is the volume shaper id.
ALOGE("applyVolumeShaper() failed with status %d", status);
}
diff --git a/media/libaaudio/src/legacy/AudioStreamTrack.h b/media/libaaudio/src/legacy/AudioStreamTrack.h
index dbcb94e..a871db4 100644
--- a/media/libaaudio/src/legacy/AudioStreamTrack.h
+++ b/media/libaaudio/src/legacy/AudioStreamTrack.h
@@ -77,8 +77,6 @@
return incrementFramesWritten(frames);
}
- bool needsSystemRegistration() override { return true; }
-
android::status_t doSetVolume() override;
#if AAUDIO_USE_VOLUME_SHAPER
