EffectChain setVolume with EffectChainmutex held
Bug: 315995877
Test: atest AudioTrackTest AudioRecordTest audioeffect_tests
Test: flush to Pixel and test audio functionality
Change-Id: I1389b6cefa1d38c364e24d01a038712e52834ff2
diff --git a/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp
index 3147433..a348b42 100644
--- a/services/audioflinger/Effects.cpp
+++ b/services/audioflinger/Effects.cpp
@@ -1356,8 +1356,7 @@
}
}
-status_t EffectModule::setVolume(uint32_t *left, uint32_t *right, bool controller)
-{
+status_t EffectModule::setVolume(uint32_t* left, uint32_t* right, bool controller) {
AutoLockReentrant _l(mutex(), mSetVolumeReentrantTid);
if (mStatus != NO_ERROR) {
return mStatus;
@@ -2574,9 +2573,14 @@
return false;
}
-// setVolume_l() must be called with IAfThreadBase::mutex() or EffectChain::mutex() held
-bool EffectChain::setVolume_l(uint32_t *left, uint32_t *right, bool force)
-{
+// setVolume() must be called without EffectChain::mutex()
+bool EffectChain::setVolume(uint32_t* left, uint32_t* right, bool force) {
+ audio_utils::lock_guard _l(mutex());
+ return setVolume_l(left, right, force);
+}
+
+// setVolume_l() must be called with EffectChain::mutex() held
+bool EffectChain::setVolume_l(uint32_t* left, uint32_t* right, bool force) {
uint32_t newLeft = *left;
uint32_t newRight = *right;
const size_t size = mEffects.size();
@@ -2651,7 +2655,7 @@
return volumeControlIndex.has_value();
}
-// resetVolume_l() must be called with IAfThreadBase::mutex() or EffectChain::mutex() held
+// resetVolume_l() must be called with EffectChain::mutex() held
void EffectChain::resetVolume_l()
{
if ((mLeftVolume != UINT_MAX) && (mRightVolume != UINT_MAX)) {
@@ -3293,7 +3297,7 @@
return true;
}
-void EffectChain::EffectCallback::resetVolume() {
+void EffectChain::EffectCallback::resetVolume() NO_THREAD_SAFETY_ANALYSIS {
sp<IAfEffectChain> c = chain().promote();
if (c == nullptr) {
return;
diff --git a/services/audioflinger/Effects.h b/services/audioflinger/Effects.h
index 8583d47..cbd62ea 100644
--- a/services/audioflinger/Effects.h
+++ b/services/audioflinger/Effects.h
@@ -401,9 +401,11 @@
public:
EffectChain(const sp<IAfThreadBase>& thread, audio_session_t sessionId);
- void process_l() final;
+ void process_l() final REQUIRES(audio_utils::EffectChain_Mutex);
- audio_utils::mutex& mutex() const final { return mMutex; }
+ audio_utils::mutex& mutex() const final RETURN_CAPABILITY(audio_utils::EffectChain_Mutex) {
+ return mMutex;
+ }
status_t createEffect_l(sp<IAfEffectModule>& effect,
effect_descriptor_t *desc,
@@ -423,8 +425,9 @@
std::vector<int> getEffectIds() const final;
// FIXME use float to improve the dynamic range
- bool setVolume_l(uint32_t *left, uint32_t *right, bool force = false) final;
- void resetVolume_l() final;
+ bool setVolume(uint32_t* left, uint32_t* right,
+ bool force = false) final EXCLUDES_EffectChain_Mutex;
+ void resetVolume_l() final REQUIRES(audio_utils::EffectChain_Mutex);
void setDevices_l(const AudioDeviceTypeAddrVector &devices) final;
void setInputDevice_l(const AudioDeviceTypeAddr &device) final;
void setMode_l(audio_mode_t mode) final;
@@ -521,7 +524,9 @@
void setThread(const sp<IAfThreadBase>& thread) final;
-private:
+ private:
+ bool setVolume_l(uint32_t* left, uint32_t* right, bool force = false)
+ REQUIRES(audio_utils::EffectChain_Mutex);
// For transaction consistency, please consider holding the EffectChain lock before
// calling the EffectChain::EffectCallback methods, excepting
diff --git a/services/audioflinger/IAfEffect.h b/services/audioflinger/IAfEffect.h
index 56076a3..d02fbb7 100644
--- a/services/audioflinger/IAfEffect.h
+++ b/services/audioflinger/IAfEffect.h
@@ -222,7 +222,8 @@
virtual void process_l() = 0;
- virtual audio_utils::mutex& mutex() const = 0;
+ virtual audio_utils::mutex& mutex() const
+ RETURN_CAPABILITY(android::audio_utils::EffectChain_Mutex) = 0;
virtual status_t createEffect_l(sp<IAfEffectModule>& effect,
effect_descriptor_t *desc,
@@ -241,8 +242,9 @@
virtual sp<IAfEffectModule> getEffectFromId_l(int id) const = 0;
virtual sp<IAfEffectModule> getEffectFromType_l(const effect_uuid_t *type) const = 0;
virtual std::vector<int> getEffectIds() const = 0;
- virtual bool setVolume_l(uint32_t *left, uint32_t *right, bool force = false) = 0;
- virtual void resetVolume_l() = 0;
+ virtual bool setVolume(uint32_t* left, uint32_t* right,
+ bool force = false) EXCLUDES_EffectChain_Mutex = 0;
+ virtual void resetVolume_l() REQUIRES(audio_utils::EffectChain_Mutex) = 0;
virtual void setDevices_l(const AudioDeviceTypeAddrVector &devices) = 0;
virtual void setInputDevice_l(const AudioDeviceTypeAddr &device) = 0;
virtual void setMode_l(audio_mode_t mode) = 0;
diff --git a/services/audioflinger/Threads.cpp b/services/audioflinger/Threads.cpp
index 244a262..ac8018b 100644
--- a/services/audioflinger/Threads.cpp
+++ b/services/audioflinger/Threads.cpp
@@ -1862,22 +1862,20 @@
}
}
-void ThreadBase::lockEffectChains_l(
- Vector<sp<IAfEffectChain>>& effectChains)
-NO_THREAD_SAFETY_ANALYSIS // calls EffectChain::lock()
+void ThreadBase::lockEffectChains_l(Vector<sp<IAfEffectChain>>& effectChains)
+ NO_THREAD_SAFETY_ANALYSIS // calls EffectChain::lock()
{
effectChains = mEffectChains;
- for (size_t i = 0; i < mEffectChains.size(); i++) {
- mEffectChains[i]->mutex().lock();
+ for (const auto& effectChain : effectChains) {
+ effectChain->mutex().lock();
}
}
-void ThreadBase::unlockEffectChains(
- const Vector<sp<IAfEffectChain>>& effectChains)
-NO_THREAD_SAFETY_ANALYSIS // calls EffectChain::unlock()
+void ThreadBase::unlockEffectChains(const Vector<sp<IAfEffectChain>>& effectChains)
+ NO_THREAD_SAFETY_ANALYSIS // calls EffectChain::unlock()
{
- for (size_t i = 0; i < effectChains.size(); i++) {
- effectChains[i]->mutex().unlock();
+ for (const auto& effectChain : effectChains) {
+ effectChain->mutex().unlock();
}
}
@@ -5501,7 +5499,7 @@
sp<IAfEffectChain> chain = getEffectChain_l(AUDIO_SESSION_OUTPUT_MIX);
if (chain != 0) {
uint32_t v = (uint32_t)(masterVolume * (1 << 24));
- chain->setVolume_l(&v, &v);
+ chain->setVolume(&v, &v);
masterVolume = (float)((v + (1 << 23)) >> 24);
chain.clear();
}
@@ -5836,7 +5834,7 @@
mixedTracks++;
- // track->mainBuffer() != mSinkBuffer or mMixerBuffer means
+ // track->mainBuffer() != mSinkBuffer and mMixerBuffer means
// there is an effect chain connected to the track
chain.clear();
if (track->mainBuffer() != mSinkBuffer &&
@@ -5940,7 +5938,7 @@
track->setFinalVolume(vrf, vlf);
// Delegate volume control to effect in track effect chain if needed
- if (chain != 0 && chain->setVolume_l(&vl, &vr)) {
+ if (chain != 0 && chain->setVolume(&vl, &vr)) {
// Do not ramp volume if volume is controlled by effect
param = AudioMixer::VOLUME;
// Update remaining floating point volume levels
@@ -6680,8 +6678,8 @@
// Convert volumes from float to 8.24
uint32_t vl = (uint32_t)(left * (1 << 24));
uint32_t vr = (uint32_t)(right * (1 << 24));
- // Direct/Offload effect chains set output volume in setVolume_l().
- (void)mEffectChains[0]->setVolume_l(&vl, &vr);
+ // Direct/Offload effect chains set output volume in setVolume().
+ (void)mEffectChains[0]->setVolume(&vl, &vr);
} else {
// otherwise we directly set the volume.
setVolumeForOutput_l(left, right);
@@ -11026,7 +11024,7 @@
// only one effect chain can be present on DirectOutputThread, so if
// there is one, the track is connected to it
if (!mEffectChains.isEmpty()) {
- mEffectChains[0]->setVolume_l(&vol, &vol);
+ mEffectChains[0]->setVolume(&vol, &vol);
volume = (float)vol / (1 << 24);
}
// Try to use HW volume control and fall back to SW control if not implemented
diff --git a/services/audioflinger/Threads.h b/services/audioflinger/Threads.h
index 8491e43..a8e4339 100644
--- a/services/audioflinger/Threads.h
+++ b/services/audioflinger/Threads.h
@@ -436,7 +436,8 @@
// ThreadBase mutex before processing the mixer and effects. This guarantees the
// integrity of the chains during the process.
// Also sets the parameter 'effectChains' to current value of mEffectChains.
- void lockEffectChains_l(Vector<sp<IAfEffectChain>>& effectChains) final REQUIRES(mutex());
+ void lockEffectChains_l(Vector<sp<IAfEffectChain>>& effectChains) final
+ REQUIRES(mutex()) EXCLUDES_EffectChain_Mutex;
// unlock effect chains after process
void unlockEffectChains(const Vector<sp<IAfEffectChain>>& effectChains) final;
// get a copy of mEffectChains vector