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/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index 180c150..98b6c27 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -3823,7 +3823,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/IAfTrack.h b/services/audioflinger/IAfTrack.h
index cf30ded..d21cb16 100644
--- a/services/audioflinger/IAfTrack.h
+++ b/services/audioflinger/IAfTrack.h
@@ -339,10 +339,10 @@
virtual sp<os::ExternalVibration> getExternalVibration() const = 0;
// This function should be called with holding thread lock.
- virtual void updateTeePatches() = 0;
+ virtual void updateTeePatches_l() = 0;
// Argument teePatchesToUpdate is by value, use std::move to optimize.
- virtual void setTeePatchesToUpdate(TeePatches teePatchesToUpdate) = 0;
+ virtual void setTeePatchesToUpdate_l(TeePatches teePatchesToUpdate) = 0;
static bool checkServerLatencySupported(audio_format_t format, audio_output_flags_t flags) {
return audio_is_linear_pcm(format) && (flags & AUDIO_OUTPUT_FLAG_HW_AV_SYNC) == 0;
diff --git a/services/audioflinger/PlaybackTracks.h b/services/audioflinger/PlaybackTracks.h
index 15e85f9..ae60ed0 100644
--- a/services/audioflinger/PlaybackTracks.h
+++ b/services/audioflinger/PlaybackTracks.h
@@ -193,8 +193,8 @@
sp<os::ExternalVibration> getExternalVibration() const final { return mExternalVibration; }
// This function should be called with holding thread lock.
- void updateTeePatches() final;
- void setTeePatchesToUpdate(TeePatches teePatchesToUpdate) final;
+ void updateTeePatches_l() final;
+ void setTeePatchesToUpdate_l(TeePatches teePatchesToUpdate) final;
void tallyUnderrunFrames(size_t frames) final {
if (isOut()) { // we expect this from output tracks only
@@ -349,8 +349,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 9a1fe1f..a314e60 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -4178,7 +4178,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 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);