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
Change-Id: Ia704350ed5b3c914448ee609da33b867e3aa7626
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 325adfa..817d3a4 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -4000,7 +4000,7 @@
patchTrack->setPeerProxy(patchRecord, true /* holdReference */);
patchRecord->setPeerProxy(patchTrack, false /* holdReference */);
}
- track->setTeePatchesToUpdate(std::move(teePatches));
+ track->setTeePatchesToUpdate_l(std::move(teePatches));
}
sp<AudioFlinger::SyncEvent> AudioFlinger::createSyncEvent(AudioSystem::sync_event_t type,
diff --git a/services/audioflinger/PlaybackTracks.h b/services/audioflinger/PlaybackTracks.h
index 0e1a3c9..d485ccc 100644
--- a/services/audioflinger/PlaybackTracks.h
+++ b/services/audioflinger/PlaybackTracks.h
@@ -187,8 +187,8 @@
sp<os::ExternalVibration> getExternalVibration() const { return mExternalVibration; }
// This function should be called with holding thread lock.
- void updateTeePatches();
- void setTeePatchesToUpdate(TeePatches teePatchesToUpdate);
+ void updateTeePatches_l();
+ void setTeePatchesToUpdate_l(TeePatches teePatchesToUpdate);
void tallyUnderrunFrames(size_t frames) override {
if (isOut()) { // we expect this from output tracks only
@@ -335,8 +335,9 @@
private:
void interceptBuffer(const AudioBufferProvider::Buffer& buffer);
+ // Must hold thread lock to access tee patches
template <class F>
- void forEachTeePatchTrack(F f) {
+ void forEachTeePatchTrack_l(F f) {
for (auto& tp : mTeePatches) { f(tp.patchTrack); }
};
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 700bdd2..d86042f 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -4100,7 +4100,7 @@
setHalLatencyMode_l();
for (const auto &track : mActiveTracks ) {
- track->updateTeePatches();
+ track->updateTeePatches_l();
}
// signal actual start of output stream when the render position reported by the kernel
diff --git a/services/audioflinger/Tracks.cpp b/services/audioflinger/Tracks.cpp
index 7d2c4db..7f99599 100644
--- a/services/audioflinger/Tracks.cpp
+++ b/services/audioflinger/Tracks.cpp
@@ -779,12 +779,12 @@
Mutex::Autolock _l(thread->mLock);
PlaybackThread *playbackThread = (PlaybackThread *)thread.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 AudioFlinger::PlaybackThread::Track::appendDumpHeader(String8& result)
@@ -1157,12 +1157,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
sp<IAudioManager> audioManager = thread->mAudioFlinger->getOrCreateAudioManager();
if (audioManager && mPortId != AUDIO_PORT_HANDLE_NONE) {
@@ -1212,8 +1213,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 AudioFlinger::PlaybackThread::Track::pause()
@@ -1248,9 +1249,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 AudioFlinger::PlaybackThread::Track::flush()
@@ -1311,9 +1312,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
@@ -1491,19 +1493,19 @@
*backInserter++ = metadata;
}
-void AudioFlinger::PlaybackThread::Track::updateTeePatches() {
+void AudioFlinger::PlaybackThread::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 AudioFlinger::PlaybackThread::Track::setTeePatchesToUpdate(TeePatches teePatchesToUpdate) {
+void AudioFlinger::PlaybackThread::Track::setTeePatchesToUpdate_l(TeePatches teePatchesToUpdate) {
ALOGW_IF(mTeePatchesToUpdate.has_value(),
"%s, existing tee patches to update will be ignored", __func__);
mTeePatchesToUpdate = std::move(teePatchesToUpdate);