AudioFlinger: Synchronize removing client from output descriptor
Avoids race condition where APM::stopOutput is called after
APM::releaseOutput.
Test: audio sanity
Bug: 112067674
Change-Id: I244267d4a157078961589649b1e184206ee23248
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 13ca912..01c5ea2 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -2794,22 +2794,18 @@
void AudioFlinger::PlaybackThread::threadLoop_removeTracks(
const Vector< sp<Track> >& tracksToRemove)
{
- size_t count = tracksToRemove.size();
- if (count > 0) {
- for (size_t i = 0 ; i < count ; i++) {
- const sp<Track>& track = tracksToRemove.itemAt(i);
- if (track->isExternalTrack()) {
- AudioSystem::stopOutput(track->portId());
+ // Miscellaneous track cleanup when removed from the active list,
+ // called without Thread lock but synchronized with threadLoop processing.
#ifdef ADD_BATTERY_DATA
- // to track the speaker usage
- addBatteryData(IMediaPlayerService::kBatteryDataAudioFlingerStop);
-#endif
- if (track->isTerminated()) {
- AudioSystem::releaseOutput(track->portId());
- }
- }
+ for (const auto& track : tracksToRemove) {
+ if (track->isExternalTrack()) {
+ // to track the speaker usage
+ addBatteryData(IMediaPlayerService::kBatteryDataAudioFlingerStop);
}
}
+#else
+ (void)tracksToRemove; // suppress unused warning
+#endif
}
void AudioFlinger::PlaybackThread::checkSilentMode_l()
@@ -3710,24 +3706,28 @@
// removeTracks_l() must be called with ThreadBase::mLock held
void AudioFlinger::PlaybackThread::removeTracks_l(const Vector< sp<Track> >& tracksToRemove)
{
- size_t count = tracksToRemove.size();
- if (count > 0) {
- for (size_t i=0 ; i<count ; i++) {
- const sp<Track>& track = tracksToRemove.itemAt(i);
- mActiveTracks.remove(track);
- ALOGV("removeTracks_l removing track on session %d", track->sessionId());
- sp<EffectChain> chain = getEffectChain_l(track->sessionId());
- if (chain != 0) {
- ALOGV("stopping track on chain %p for session Id: %d", chain.get(),
- track->sessionId());
- chain->decActiveTrackCnt();
- }
+ for (const auto& track : tracksToRemove) {
+ mActiveTracks.remove(track);
+ ALOGV("%s(%d): removing track on session %d", __func__, track->id(), track->sessionId());
+ sp<EffectChain> chain = getEffectChain_l(track->sessionId());
+ if (chain != 0) {
+ ALOGV("%s(%d): stopping track on chain %p for session Id: %d",
+ __func__, track->id(), chain.get(), track->sessionId());
+ chain->decActiveTrackCnt();
+ }
+ // If an external client track, inform APM we're no longer active, and remove if needed.
+ // We do this under lock so that the state is consistent if the Track is destroyed.
+ if (track->isExternalTrack()) {
+ AudioSystem::stopOutput(track->portId());
if (track->isTerminated()) {
- removeTrack_l(track);
+ AudioSystem::releaseOutput(track->portId());
}
}
+ if (track->isTerminated()) {
+ // remove from our tracks vector
+ removeTrack_l(track);
+ }
}
-
}
status_t AudioFlinger::PlaybackThread::getTimestamp_l(AudioTimestamp& timestamp)
@@ -4115,12 +4115,6 @@
return latency;
}
-
-void AudioFlinger::MixerThread::threadLoop_removeTracks(const Vector< sp<Track> >& tracksToRemove)
-{
- PlaybackThread::threadLoop_removeTracks(tracksToRemove);
-}
-
ssize_t AudioFlinger::MixerThread::threadLoop_write()
{
// FIXME we should only do one push per cycle; confirm this is true
diff --git a/services/audioflinger/Threads.h b/services/audioflinger/Threads.h
index 88c6ff3..61f7baf 100644
--- a/services/audioflinger/Threads.h
+++ b/services/audioflinger/Threads.h
@@ -1137,7 +1137,6 @@
virtual void threadLoop_standby();
virtual void threadLoop_mix();
virtual void threadLoop_sleepTime();
- virtual void threadLoop_removeTracks(const Vector< sp<Track> >& tracksToRemove);
virtual uint32_t correctLatency_l(uint32_t latency) const;
virtual status_t createAudioPatch_l(const struct audio_patch *patch,
diff --git a/services/audiopolicy/service/AudioPolicyService.cpp b/services/audiopolicy/service/AudioPolicyService.cpp
index 8bca221..b1ed522 100644
--- a/services/audiopolicy/service/AudioPolicyService.cpp
+++ b/services/audiopolicy/service/AudioPolicyService.cpp
@@ -677,21 +677,27 @@
VolumeData *data = (VolumeData *)command->mParam.get();
ALOGV("AudioCommandThread() processing set volume stream %d, \
volume %f, output %d", data->mStream, data->mVolume, data->mIO);
+ mLock.unlock();
command->mStatus = AudioSystem::setStreamVolume(data->mStream,
data->mVolume,
data->mIO);
+ mLock.lock();
}break;
case SET_PARAMETERS: {
ParametersData *data = (ParametersData *)command->mParam.get();
ALOGV("AudioCommandThread() processing set parameters string %s, io %d",
data->mKeyValuePairs.string(), data->mIO);
+ mLock.unlock();
command->mStatus = AudioSystem::setParameters(data->mIO, data->mKeyValuePairs);
+ mLock.lock();
}break;
case SET_VOICE_VOLUME: {
VoiceVolumeData *data = (VoiceVolumeData *)command->mParam.get();
ALOGV("AudioCommandThread() processing set voice volume volume %f",
data->mVolume);
+ mLock.unlock();
command->mStatus = AudioSystem::setVoiceVolume(data->mVolume);
+ mLock.lock();
}break;
case STOP_OUTPUT: {
StopOutputData *data = (StopOutputData *)command->mParam.get();
@@ -724,7 +730,9 @@
if (af == 0) {
command->mStatus = PERMISSION_DENIED;
} else {
+ mLock.unlock();
command->mStatus = af->createAudioPatch(&data->mPatch, &data->mHandle);
+ mLock.lock();
}
} break;
case RELEASE_AUDIO_PATCH: {
@@ -734,7 +742,9 @@
if (af == 0) {
command->mStatus = PERMISSION_DENIED;
} else {
+ mLock.unlock();
command->mStatus = af->releaseAudioPatch(data->mHandle);
+ mLock.lock();
}
} break;
case UPDATE_AUDIOPORT_LIST: {
@@ -764,7 +774,9 @@
if (af == 0) {
command->mStatus = PERMISSION_DENIED;
} else {
+ mLock.unlock();
command->mStatus = af->setAudioPortConfig(&data->mConfig);
+ mLock.lock();
}
} break;
case DYN_POLICY_MIX_STATE_UPDATE: {