audioflinger: fix effect volume control delay

Fix delay between effect enable and forced volume update on
offload threads.

Bug: 30458082
Change-Id: I4222ccde8c6d0bb834c525d3746bd668654f50f3
diff --git a/services/audioflinger/Effects.cpp b/services/audioflinger/Effects.cpp
index e6c8abc..4cde70d 100644
--- a/services/audioflinger/Effects.cpp
+++ b/services/audioflinger/Effects.cpp
@@ -216,9 +216,10 @@
     return mHandles.size();
 }
 
-void AudioFlinger::EffectModule::updateState() {
+bool AudioFlinger::EffectModule::updateState() {
     Mutex::Autolock _l(mLock);
 
+    bool started = false;
     switch (mState) {
     case RESTART:
         reset_l();
@@ -233,6 +234,7 @@
         }
         if (start_l() == NO_ERROR) {
             mState = ACTIVE;
+            started = true;
         } else {
             mState = IDLE;
         }
@@ -256,6 +258,8 @@
     default: //IDLE , ACTIVE, DESTROYED
         break;
     }
+
+    return started;
 }
 
 void AudioFlinger::EffectModule::process()
@@ -462,10 +466,22 @@
     }
 }
 
+// start() must be called with PlaybackThread::mLock or EffectChain::mLock held
 status_t AudioFlinger::EffectModule::start()
 {
-    Mutex::Autolock _l(mLock);
-    return start_l();
+    sp<EffectChain> chain;
+    status_t status;
+    {
+        Mutex::Autolock _l(mLock);
+        status = start_l();
+        if (status == NO_ERROR) {
+            chain = mChain.promote();
+        }
+    }
+    if (chain != 0) {
+        chain->resetVolume_l();
+    }
+    return status;
 }
 
 status_t AudioFlinger::EffectModule::start_l()
@@ -489,10 +505,6 @@
     }
     if (status == 0) {
         addEffectToHal_l();
-        sp<EffectChain> chain = mChain.promote();
-        if (chain != 0) {
-            chain->forceVolume();
-        }
     }
     return status;
 }
@@ -1349,7 +1361,7 @@
                                         audio_session_t sessionId)
     : mThread(thread), mSessionId(sessionId), mActiveTrackCnt(0), mTrackCnt(0), mTailBufferCount(0),
       mOwnInBuffer(false), mVolumeCtrlIdx(-1), mLeftVolume(UINT_MAX), mRightVolume(UINT_MAX),
-      mNewLeftVolume(UINT_MAX), mNewRightVolume(UINT_MAX), mForceVolume(false)
+      mNewLeftVolume(UINT_MAX), mNewRightVolume(UINT_MAX)
 {
     mStrategy = AudioSystem::getStrategyForStream(AUDIO_STREAM_MUSIC);
     if (thread == NULL) {
@@ -1471,8 +1483,12 @@
             mEffects[i]->process();
         }
     }
+    bool doResetVolume = false;
     for (size_t i = 0; i < size; i++) {
-        mEffects[i]->updateState();
+        doResetVolume = mEffects[i]->updateState() || doResetVolume;
+    }
+    if (doResetVolume) {
+        resetVolume_l();
     }
 }
 
@@ -1653,8 +1669,8 @@
     }
 }
 
-// setVolume_l() must be called with PlaybackThread::mLock held
-bool AudioFlinger::EffectChain::setVolume_l(uint32_t *left, uint32_t *right)
+// setVolume_l() must be called with PlaybackThread::mLock or EffectChain::mLock held
+bool AudioFlinger::EffectChain::setVolume_l(uint32_t *left, uint32_t *right, bool force)
 {
     uint32_t newLeft = *left;
     uint32_t newRight = *right;
@@ -1672,7 +1688,7 @@
         }
     }
 
-    if (!isVolumeForced() && ctrlIdx == mVolumeCtrlIdx &&
+    if (!force && ctrlIdx == mVolumeCtrlIdx &&
             *left == mLeftVolume && *right == mRightVolume) {
         if (hasControl) {
             *left = mNewLeftVolume;
@@ -1714,6 +1730,14 @@
     return hasControl;
 }
 
+// resetVolume_l() must be called with PlaybackThread::mLock or EffectChain::mLock held
+void AudioFlinger::EffectChain::resetVolume_l()
+{
+    uint32_t left = mLeftVolume;
+    uint32_t right = mRightVolume;
+    (void)setVolume_l(&left, &right, true);
+}
+
 void AudioFlinger::EffectChain::syncHalEffectsState()
 {
     Mutex::Autolock _l(mLock);
diff --git a/services/audioflinger/Effects.h b/services/audioflinger/Effects.h
index 3b62652..322c06a 100644
--- a/services/audioflinger/Effects.h
+++ b/services/audioflinger/Effects.h
@@ -60,7 +60,7 @@
 
     int         id() const { return mId; }
     void process();
-    void updateState();
+    bool updateState();
     status_t command(uint32_t cmdCode,
                      uint32_t cmdSize,
                      void *pCmdData,
@@ -277,7 +277,8 @@
     sp<EffectModule> getEffectFromId_l(int id);
     sp<EffectModule> getEffectFromType_l(const effect_uuid_t *type);
     // FIXME use float to improve the dynamic range
-    bool setVolume_l(uint32_t *left, uint32_t *right);
+    bool setVolume_l(uint32_t *left, uint32_t *right, bool force = false);
+    void resetVolume_l();
     void setDevice_l(audio_devices_t device);
     void setMode_l(audio_mode_t mode);
     void setAudioSource_l(audio_source_t source);
@@ -323,13 +324,6 @@
     // At least one non offloadable effect in the chain is enabled
     bool isNonOffloadableEnabled();
 
-    // use release_cas because we don't care about the observed value, just want to make sure the
-    // new value is observable.
-    void forceVolume() { android_atomic_release_cas(false, true, &mForceVolume); }
-    // use acquire_cas because we are interested in the observed value and
-    // we are the only observers.
-    bool isVolumeForced() { return (android_atomic_acquire_cas(true, false, &mForceVolume) == 0); }
-
     void syncHalEffectsState();
 
     bool hasSoftwareEffect() const;
@@ -393,5 +387,4 @@
              // timeLow fields among effect type UUIDs.
              // Updated by updateSuspendedSessions_l() only.
              KeyedVector< int, sp<SuspendedEffectDesc> > mSuspendedEffects;
-    volatile int32_t mForceVolume; // force next volume command because a new effect was enabled
 };