Effects: Add atomic_wp<T>, update setThread.
atomic_wp<T> is used to allow concurrent read and write to the wp<>.
We use this to fix EffectCallback::setThread to resolve multithreaded
access to the wp<T>.
Note that setThread is used for EffectChain migration between
PlaybackThreads; we also need to refine higher level transactional
locking to ensure enable/disable of effect is done consistently
during migration.
Test: basic audio works
Test: atest media_synchronization_tests
Test: AudioEffectTest AudioPreProcessingTest BassBoostTest
Test: EnvReverbTest EqualizerTest LoudnessEnhancerTest
Test: PresetReverbTest VirtualizerTest VisualizerTest
Bug: 161341295
Change-Id: If80ee4373859c832d6fd10fec0385d69064f50c6
diff --git a/services/audioflinger/Effects.h b/services/audioflinger/Effects.h
index 89321f9..476330c 100644
--- a/services/audioflinger/Effects.h
+++ b/services/audioflinger/Effects.h
@@ -512,6 +512,12 @@
private:
+ // For transaction consistency, please consider holding the EffectChain lock before
+ // calling the EffectChain::EffectCallback methods, excepting
+ // createEffectHal and allocateHalBuffer.
+ //
+ // This prevents migration of the EffectChain to another PlaybackThread
+ // for the purposes of the EffectCallback.
class EffectCallback : public EffectCallbackInterface {
public:
// Note: ctors taking a weak pointer to their owner must not promote it
@@ -556,10 +562,8 @@
wp<EffectChain> chain() const override { return mChain; }
- wp<ThreadBase> thread() { return mThread; }
+ wp<ThreadBase> thread() const { return mThread.load(); }
- // TODO(b/161341295) secure this against concurrent access to mThread
- // by other callers.
void setThread(const wp<ThreadBase>& thread) {
mThread = thread;
sp<ThreadBase> p = thread.promote();
@@ -568,7 +572,7 @@
private:
const wp<EffectChain> mChain;
- wp<ThreadBase> mThread; // TODO(b/161341295) protect against concurrent access
+ mediautils::atomic_wp<ThreadBase> mThread;
wp<AudioFlinger> mAudioFlinger; // this could be const with some rearrangement.
};