Accessing tee patches with holding the thread lock.
There is race that the tee patches can be updated from thread loop and
the tee patches can also be accessed when the client has some
operations. In that case, it will be safe to access tee patches with
holding the thread lock.
Bug: 286576245
Test: atest AudioPlaybackCaptureTest
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:7434e81670e459054a94fd1bb51a963547440b0e)
Merged-In: Ia704350ed5b3c914448ee609da33b867e3aa7626
Change-Id: Ia704350ed5b3c914448ee609da33b867e3aa7626
diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp
index 4811586..224c65b 100644
--- a/services/audioflinger/Tracks.cpp
+++ b/services/audioflinger/Tracks.cpp
@@ -895,12 +895,12 @@
audio_utils::lock_guard _l(thread->mutex());
auto* const playbackThread = thread->asIAfPlaybackThread().get();
wasActive = playbackThread->destroyTrack_l(this);
+ forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->destroy(); });
}
if (isExternalTrack() && !wasActive) {
AudioSystem::releaseOutput(mPortId);
}
}
- forEachTeePatchTrack([](auto patchTrack) { patchTrack->destroy(); });
}
void Track::appendDumpHeader(String8& result) const
@@ -1275,12 +1275,13 @@
buffer.mFrameCount = 1;
(void) mAudioTrackServerProxy->obtainBuffer(&buffer, true /*ackFlush*/);
}
+ if (status == NO_ERROR) {
+ forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->start(); });
+ }
} else {
status = BAD_VALUE;
}
if (status == NO_ERROR) {
- forEachTeePatchTrack([](auto patchTrack) { patchTrack->start(); });
-
// send format to AudioManager for playback activity monitoring
const sp<IAudioManager> audioManager =
thread->afThreadCallback()->getOrCreateAudioManager();
@@ -1331,8 +1332,8 @@
ALOGV("%s(%d): not stopping/stopped => stopping/stopped on thread %d",
__func__, mId, (int)mThreadIoHandle);
}
+ forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->stop(); });
}
- forEachTeePatchTrack([](auto patchTrack) { patchTrack->stop(); });
}
void Track::pause()
@@ -1367,9 +1368,9 @@
default:
break;
}
+ // Pausing the TeePatch to avoid a glitch on underrun, at the cost of buffered audio loss.
+ forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->pause(); });
}
- // Pausing the TeePatch to avoid a glitch on underrun, at the cost of buffered audio loss.
- forEachTeePatchTrack([](auto patchTrack) { patchTrack->pause(); });
}
void Track::flush()
@@ -1430,9 +1431,10 @@
// before mixer thread can run. This is important when offloading
// because the hardware buffer could hold a large amount of audio
playbackThread->broadcast_l();
+ // Flush the Tee to avoid on resume playing old data and glitching on the transition to
+ // new data
+ forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->flush(); });
}
- // Flush the Tee to avoid on resume playing old data and glitching on the transition to new data
- forEachTeePatchTrack([](auto patchTrack) { patchTrack->flush(); });
}
// must be called with thread lock held
@@ -1611,19 +1613,19 @@
*backInserter++ = metadata;
}
-void Track::updateTeePatches() {
+void Track::updateTeePatches_l() {
if (mTeePatchesToUpdate.has_value()) {
- forEachTeePatchTrack([](auto patchTrack) { patchTrack->destroy(); });
+ forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->destroy(); });
mTeePatches = mTeePatchesToUpdate.value();
if (mState == TrackBase::ACTIVE || mState == TrackBase::RESUMING ||
mState == TrackBase::STOPPING_1) {
- forEachTeePatchTrack([](auto patchTrack) { patchTrack->start(); });
+ forEachTeePatchTrack_l([](const auto& patchTrack) { patchTrack->start(); });
}
mTeePatchesToUpdate.reset();
}
}
-void Track::setTeePatchesToUpdate(TeePatches teePatchesToUpdate) {
+void Track::setTeePatchesToUpdate_l(TeePatches teePatchesToUpdate) {
ALOGW_IF(mTeePatchesToUpdate.has_value(),
"%s, existing tee patches to update will be ignored", __func__);
mTeePatchesToUpdate = std::move(teePatchesToUpdate);