audio policy: fix HAL stream activity ref counting
commit f05fc906 did not take into account the specificities of duplicated
outputs causing a crash when accessing the profile (null for duplicated outputs)
and not counting activity resulting from duplicated output activity.
Also account for HW source activity when connected to an output mix.
Bug: 70319466
Test: CTS tests for AudioTrack and AudioRecord
Test: audio smoke tests
Change-Id: Id0364170794302e42212a90e1b40e28c1e7193fe
diff --git a/services/audiopolicy/common/managerdefinitions/include/AudioInputDescriptor.h b/services/audiopolicy/common/managerdefinitions/include/AudioInputDescriptor.h
index 65270b6..b25d6d4 100644
--- a/services/audiopolicy/common/managerdefinitions/include/AudioInputDescriptor.h
+++ b/services/audiopolicy/common/managerdefinitions/include/AudioInputDescriptor.h
@@ -79,6 +79,12 @@
audio_source_t source,
audio_input_flags_t flags,
audio_io_handle_t *input);
+ // Called when a stream is about to be started.
+ // Note: called after AudioSession::changeActiveCount(1)
+ status_t start();
+ // Called after a stream is stopped
+ // Note: called after AudioSession::changeActiveCount(-1)
+ void stop();
void close();
private:
diff --git a/services/audiopolicy/common/managerdefinitions/include/AudioOutputDescriptor.h b/services/audiopolicy/common/managerdefinitions/include/AudioOutputDescriptor.h
index f4cbdd9..2803ec1 100644
--- a/services/audiopolicy/common/managerdefinitions/include/AudioOutputDescriptor.h
+++ b/services/audiopolicy/common/managerdefinitions/include/AudioOutputDescriptor.h
@@ -126,6 +126,12 @@
audio_stream_type_t stream,
audio_output_flags_t flags,
audio_io_handle_t *output);
+ // Called when a stream is about to be started
+ // Note: called before changeRefCount(1);
+ status_t start();
+ // Called after a stream is stopped.
+ // Note: called after changeRefCount(-1);
+ void stop();
void close();
const sp<IOProfile> mProfile; // I/O profile this output derives from
diff --git a/services/audiopolicy/common/managerdefinitions/src/AudioInputDescriptor.cpp b/services/audiopolicy/common/managerdefinitions/src/AudioInputDescriptor.cpp
index 3642e3c..f589743 100644
--- a/services/audiopolicy/common/managerdefinitions/src/AudioInputDescriptor.cpp
+++ b/services/audiopolicy/common/managerdefinitions/src/AudioInputDescriptor.cpp
@@ -234,6 +234,27 @@
return status;
}
+status_t AudioInputDescriptor::start()
+{
+ if (getAudioSessionCount(true/*activeOnly*/) == 1) {
+ if (!mProfile->canStartNewIo()) {
+ ALOGI("%s mProfile->curActiveCount %d", __func__, mProfile->curActiveCount);
+ return INVALID_OPERATION;
+ }
+ mProfile->curActiveCount++;
+ }
+ return NO_ERROR;
+}
+
+void AudioInputDescriptor::stop()
+{
+ if (!isActive()) {
+ LOG_ALWAYS_FATAL_IF(mProfile->curActiveCount < 1,
+ "%s invalid profile active count %u",
+ __func__, mProfile->curActiveCount);
+ mProfile->curActiveCount--;
+ }
+}
void AudioInputDescriptor::close()
{
@@ -241,6 +262,9 @@
mClientInterface->closeInput(mIoHandle);
LOG_ALWAYS_FATAL_IF(mProfile->curOpenCount < 1, "%s profile open count %u",
__FUNCTION__, mProfile->curOpenCount);
+ // do not call stop() here as stop() is supposed to be called after
+ // AudioSession::changeActiveCount(-1) and we don't know how many sessions
+ // are still active at this time
if (isActive()) {
mProfile->curActiveCount--;
}
diff --git a/services/audiopolicy/common/managerdefinitions/src/AudioOutputDescriptor.cpp b/services/audiopolicy/common/managerdefinitions/src/AudioOutputDescriptor.cpp
index c667640..6e677e4 100644
--- a/services/audiopolicy/common/managerdefinitions/src/AudioOutputDescriptor.cpp
+++ b/services/audiopolicy/common/managerdefinitions/src/AudioOutputDescriptor.cpp
@@ -442,6 +442,44 @@
return status;
}
+status_t SwAudioOutputDescriptor::start()
+{
+ if (isDuplicated()) {
+ status_t status = mOutput1->start();
+ if (status != NO_ERROR) {
+ return status;
+ }
+ status = mOutput2->start();
+ if (status != NO_ERROR) {
+ mOutput1->stop();
+ return status;
+ }
+ return NO_ERROR;
+ }
+ if (!isActive()) {
+ if (!mProfile->canStartNewIo()) {
+ return INVALID_OPERATION;
+ }
+ mProfile->curActiveCount++;
+ }
+ return NO_ERROR;
+}
+
+void SwAudioOutputDescriptor::stop()
+{
+ if (isDuplicated()) {
+ mOutput1->stop();
+ mOutput2->stop();
+ return;
+ }
+
+ if (!isActive()) {
+ LOG_ALWAYS_FATAL_IF(mProfile->curActiveCount < 1,
+ "%s invalid profile active count %u",
+ __func__, mProfile->curActiveCount);
+ mProfile->curActiveCount--;
+ }
+}
void SwAudioOutputDescriptor::close()
{
@@ -454,6 +492,8 @@
LOG_ALWAYS_FATAL_IF(mProfile->curOpenCount < 1, "%s profile open count %u",
__FUNCTION__, mProfile->curOpenCount);
+ // do not call stop() here as stop() is supposed to be called after changeRefCount(-1)
+ // and we don't know how many streams are still active at this time
if (isActive()) {
mProfile->curActiveCount--;
}
@@ -462,7 +502,6 @@
}
}
-
// HwAudioOutputDescriptor implementation
HwAudioOutputDescriptor::HwAudioOutputDescriptor(const sp<AudioSourceDescriptor>& source,
AudioPolicyClientInterface *clientInterface)
diff --git a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
index 5298034..0e11c5c 100644
--- a/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
+++ b/services/audiopolicy/managerdefault/AudioPolicyManager.cpp
@@ -1072,11 +1072,9 @@
sp<SwAudioOutputDescriptor> outputDesc = mOutputs.valueAt(index);
- if (!outputDesc->isActive()) {
- if (!outputDesc->mProfile->canStartNewIo()) {
- return INVALID_OPERATION;
- }
- outputDesc->mProfile->curActiveCount++;
+ status_t status = outputDesc->start();
+ if (status != NO_ERROR) {
+ return status;
}
// Routing?
@@ -1102,16 +1100,11 @@
uint32_t delayMs = 0;
- status_t status = startSource(outputDesc, stream, newDevice, address, &delayMs);
+ status = startSource(outputDesc, stream, newDevice, address, &delayMs);
if (status != NO_ERROR) {
mOutputRoutes.decRouteActivity(session);
- if (!outputDesc->isActive()) {
- LOG_ALWAYS_FATAL_IF(outputDesc->mProfile->curActiveCount < 1,
- "%s invalid profile active count %u",
- __FUNCTION__, outputDesc->mProfile->curActiveCount);
- outputDesc->mProfile->curActiveCount--;
- }
+ outputDesc->stop();
return status;
}
// Automatically enable the remote submix input when output is started on a re routing mix
@@ -1302,11 +1295,8 @@
status_t status = stopSource(outputDesc, stream, forceDeviceUpdate);
- if (status == NO_ERROR && !outputDesc->isActive()) {
- LOG_ALWAYS_FATAL_IF(outputDesc->mProfile->curActiveCount < 1,
- "%s invalid profile active count %u",
- __FUNCTION__, outputDesc->mProfile->curActiveCount);
- outputDesc->mProfile->curActiveCount--;
+ if (status == NO_ERROR ) {
+ outputDesc->stop();
}
return status;
}
@@ -1929,14 +1919,14 @@
audio_devices_t device = getNewInputDevice(inputDesc);
setInputDevice(input, device, true /* force */);
- if (inputDesc->getAudioSessionCount(true/*activeOnly*/) == 1) {
- if (!inputDesc->mProfile->canStartNewIo()) {
- mInputRoutes.decRouteActivity(session);
- audioSession->changeActiveCount(-1);
- return INVALID_OPERATION;
- }
- inputDesc->mProfile->curActiveCount++;
+ status_t status = inputDesc->start();
+ if (status != NO_ERROR) {
+ mInputRoutes.decRouteActivity(session);
+ audioSession->changeActiveCount(-1);
+ return status;
+ }
+ if (inputDesc->getAudioSessionCount(true/*activeOnly*/) == 1) {
// if input maps to a dynamic policy with an activity listener, notify of state change
if ((inputDesc->mPolicyMix != NULL)
&& ((inputDesc->mPolicyMix->mCbFlags & AudioMix::kCbFlagNotifyActivity) != 0)) {
@@ -2002,15 +1992,10 @@
mInputRoutes.decRouteActivity(session);
if (audioSession->activeCount() == 0) {
-
+ inputDesc->stop();
if (inputDesc->isActive()) {
setInputDevice(input, getNewInputDevice(inputDesc), false /* force */);
} else {
- LOG_ALWAYS_FATAL_IF(inputDesc->mProfile->curActiveCount < 1,
- "%s invalid profile active count %u",
- __FUNCTION__, inputDesc->mProfile->curActiveCount);
- inputDesc->mProfile->curActiveCount--;
-
// if input maps to a dynamic policy with an activity listener, notify of state change
if ((inputDesc->mPolicyMix != NULL)
&& ((inputDesc->mPolicyMix->mCbFlags & AudioMix::kCbFlagNotifyActivity) != 0)) {
@@ -3303,6 +3288,11 @@
ALOGV("%s output for device %08x is duplicated", __FUNCTION__, sinkDevice);
return INVALID_OPERATION;
}
+ status_t status = outputDesc->start();
+ if (status != NO_ERROR) {
+ return status;
+ }
+
// create a special patch with no sink and two sources:
// - the second source indicates to PatchPanel through which output mix this patch should
// be connected as well as the stream type for volume control
@@ -3313,7 +3303,7 @@
srcDeviceDesc->toAudioPortConfig(&patch->sources[0], NULL);
outputDesc->toAudioPortConfig(&patch->sources[1], NULL);
patch->sources[1].ext.mix.usecase.stream = stream;
- status_t status = mpClientInterface->createAudioPatch(patch,
+ status = mpClientInterface->createAudioPatch(patch,
&afPatchHandle,
0);
ALOGV("%s patch panel returned %d patchHandle %d", __FUNCTION__,
@@ -3415,7 +3405,10 @@
audio_stream_type_t stream = streamTypefromAttributesInt(&sourceDesc->mAttributes);
sp<SwAudioOutputDescriptor> swOutputDesc = sourceDesc->mSwOutput.promote();
if (swOutputDesc != 0) {
- stopSource(swOutputDesc, stream, false);
+ status_t status = stopSource(swOutputDesc, stream, false);
+ if (status == NO_ERROR) {
+ swOutputDesc->stop();
+ }
mpClientInterface->releaseAudioPatch(patchDesc->mAfPatchHandle, 0);
} else {
sp<HwAudioOutputDescriptor> hwOutputDesc = sourceDesc->mHwOutput.promote();
@@ -4149,7 +4142,7 @@
if (dupOutputDesc->isDuplicated() &&
(dupOutputDesc->mOutput1 == outputDesc ||
dupOutputDesc->mOutput2 == outputDesc)) {
- sp<AudioOutputDescriptor> outputDesc2;
+ sp<SwAudioOutputDescriptor> outputDesc2;
if (dupOutputDesc->mOutput1 == outputDesc) {
outputDesc2 = dupOutputDesc->mOutput2;
} else {
@@ -4159,10 +4152,16 @@
// and as they were also referenced on the other output, the reference
// count for their stream type must be adjusted accordingly on
// the other output.
+ bool wasActive = outputDesc2->isActive();
for (int j = 0; j < AUDIO_STREAM_CNT; j++) {
int refCount = dupOutputDesc->mRefCount[j];
outputDesc2->changeRefCount((audio_stream_type_t)j,-refCount);
}
+ // stop() will be a no op if the output is still active but is needed in case all
+ // active streams refcounts where cleared above
+ if (wasActive) {
+ outputDesc2->stop();
+ }
audio_io_handle_t duplicatedOutput = mOutputs.keyAt(i);
ALOGV("closeOutput() closing also duplicated output %d", duplicatedOutput);