Audio Effects: fix problems in volume control.

- Fixed click when re-enabling effect during the turn off phase:
make sure the effect states where effect is processed are the same
where volume control is delegated to effect.
- Fixed click when effect is deleted while still active: do not apply
volume ramp if an effect having volume control was just removed from the
effect chain.

Also fixed a crash when PCM dump is enabled in effect bundle wrapper.

Change-Id: Ib562f5cf75c69af75df0e862536262e2514493e4

3 files changed, 54 insertions, 15 deletions

diff --git a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp
index d3bb96e..7297811 100644
--- a/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp
+++ b/media/libeffects/lvm/wrapper/Bundle/EffectBundle.cpp
@@ -258,16 +258,24 @@ extern "C" int EffectCreate(effect_uuid_t       *uuid,
         pContext->pBundledContext->firstVolume              = LVM_TRUE;
 
         #ifdef LVM_PCM
-        pContext->pBundledContext->PcmInPtr  = NULL;
-        pContext->pBundledContext->PcmOutPtr = NULL;
 
-        pContext->pBundledContext->PcmInPtr  = fopen("/data/tmp/bundle_pcm_in.pcm", "w");
-        pContext->pBundledContext->PcmOutPtr = fopen("/data/tmp/bundle_pcm_out.pcm", "w");
+        char fileName[256];
+        snprintf(fileName, 256, "/data/tmp/bundle_%p_pcm_in.pcm", pContext->pBundledContext);
+        pContext->pBundledContext->PcmInPtr = fopen(fileName, "w");
+        if (pContext->pBundledContext->PcmInPtr == NULL) {
+            LOGV("cannot open %s", fileName);
+           return -EINVAL;
+        }
 
-        if((pContext->pBundledContext->PcmInPtr  == NULL)||
-           (pContext->pBundledContext->PcmOutPtr == NULL)){
+        snprintf(fileName, 256, "/data/tmp/bundle_%p_pcm_out.pcm", pContext->pBundledContext);
+        pContext->pBundledContext->PcmOutPtr = fopen(fileName, "w");
+        if (pContext->pBundledContext->PcmOutPtr == NULL) {
+            LOGV("cannot open %s", fileName);
+            fclose(pContext->pBundledContext->PcmInPtr);
+           pContext->pBundledContext->PcmInPtr = NULL;
            return -EINVAL;
         }
+
         #endif
 
         /* Saved strength is used to return the exact strength that was used in the set to the get
@@ -375,8 +383,14 @@ extern "C" int EffectRelease(effect_interface_t interface){
     (GlobalSessionMemory[pContext->pBundledContext->SessionNo].bVirtualizerInstantiated==LVM_FALSE))
     {
         #ifdef LVM_PCM
-        fclose(pContext->pBundledContext->PcmInPtr);
-        fclose(pContext->pBundledContext->PcmOutPtr);
+        if (pContext->pBundledContext->PcmInPtr != NULL) {
+            fclose(pContext->pBundledContext->PcmInPtr);
+            pContext->pBundledContext->PcmInPtr = NULL;
+        }
+        if (pContext->pBundledContext->PcmOutPtr != NULL) {
+            fclose(pContext->pBundledContext->PcmOutPtr);
+            pContext->pBundledContext->PcmOutPtr = NULL;
+        }
         #endif
 
         LvmSessionsActive--;
diff --git a/services/audioflinger/AudioFlinger.cpp b/services/audioflinger/AudioFlinger.cpp
index ff31470..1acdaaf 100644
--- a/services/audioflinger/AudioFlinger.cpp
+++ b/services/audioflinger/AudioFlinger.cpp
@@ -1781,6 +1781,14 @@ uint32_t AudioFlinger::MixerThread::prepareTracks_l(const SortedVector< wp<Track
                 if (chain != 0 && chain->setVolume_l(&vl, &vr)) {
                     // Do not ramp volume is volume is controlled by effect
                     param = AudioMixer::VOLUME;
+                    track->mHasVolumeController = true;
+                } else {
+                    // force no volume ramp when volume controller was just disabled or removed
+                    // from effect chain to avoid volume spike
+                    if (track->mHasVolumeController) {
+                        param = AudioMixer::VOLUME;
+                    }
+                    track->mHasVolumeController = false;
                 }
 
                 // Convert volumes from 8.24 to 4.12 format
@@ -2901,7 +2909,8 @@ AudioFlinger::PlaybackThread::Track::Track(
             const sp<IMemory>& sharedBuffer,
             int sessionId)
     :   TrackBase(thread, client, sampleRate, format, channelCount, frameCount, 0, sharedBuffer, sessionId),
-    mMute(false), mSharedBuffer(sharedBuffer), mName(-1), mMainBuffer(NULL), mAuxBuffer(NULL), mAuxEffectId(0)
+    mMute(false), mSharedBuffer(sharedBuffer), mName(-1), mMainBuffer(NULL), mAuxBuffer(NULL),
+    mAuxEffectId(0), mHasVolumeController(false)
 {
     if (mCblk != NULL) {
         sp<ThreadBase> baseThread = thread.promote();
@@ -5371,7 +5380,7 @@ void AudioFlinger::EffectModule::process()
         return;
     }
 
-    if (mState == ACTIVE || mState == STOPPING || mState == STOPPED || mState == RESTART) {
+    if (isProcessEnabled()) {
         // do 32 bit to 16 bit conversion for auxiliary effect input buffer
         if ((mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_AUXILIARY) {
             AudioMixer::ditherAndClamp(mConfig.inputCfg.buffer.s32,
@@ -5600,6 +5609,8 @@ status_t AudioFlinger::EffectModule::setEnabled(bool enabled)
 
         // going from enabled to disabled
         case RESTART:
+            mState = STOPPED;
+            break;
         case STARTING:
             mState = IDLE;
             break;
@@ -5632,6 +5643,21 @@ bool AudioFlinger::EffectModule::isEnabled()
     }
 }
 
+bool AudioFlinger::EffectModule::isProcessEnabled()
+{
+    switch (mState) {
+    case RESTART:
+    case ACTIVE:
+    case STOPPING:
+    case STOPPED:
+        return true;
+    case IDLE:
+    case STARTING:
+    default:
+        return false;
+    }
+}
+
 status_t AudioFlinger::EffectModule::setVolume(uint32_t *left, uint32_t *right, bool controller)
 {
     Mutex::Autolock _l(mLock);
@@ -5639,7 +5665,7 @@ status_t AudioFlinger::EffectModule::setVolume(uint32_t *left, uint32_t *right,
 
     // Send volume indication if EFFECT_FLAG_VOLUME_IND is set and read back altered volume
     // if controller flag is set (Note that controller == TRUE => EFFECT_FLAG_VOLUME_CTRL set)
-    if ((mState >= ACTIVE) &&
+    if (isProcessEnabled() &&
             ((mDescriptor.flags & EFFECT_FLAG_VOLUME_MASK) == EFFECT_FLAG_VOLUME_CTRL ||
             (mDescriptor.flags & EFFECT_FLAG_VOLUME_MASK) == EFFECT_FLAG_VOLUME_IND)) {
         status_t cmdStatus;
@@ -6288,7 +6314,7 @@ bool AudioFlinger::EffectChain::setVolume_l(uint32_t *left, uint32_t *right)
 
     // first update volume controller
     for (size_t i = size; i > 0; i--) {
-        if ((mEffects[i - 1]->state() >= EffectModule::ACTIVE) &&
+        if (mEffects[i - 1]->isProcessEnabled() &&
             (mEffects[i - 1]->desc().flags & EFFECT_FLAG_VOLUME_MASK) == EFFECT_FLAG_VOLUME_CTRL) {
             ctrlIdx = i - 1;
             hasControl = true;
@@ -6304,9 +6330,6 @@ bool AudioFlinger::EffectChain::setVolume_l(uint32_t *left, uint32_t *right)
         return hasControl;
     }
 
-    if (mVolumeCtrlIdx != -1) {
-        hasControl = true;
-    }
     mVolumeCtrlIdx = ctrlIdx;
     mLeftVolume = newLeft;
     mRightVolume = newRight;
diff --git a/services/audioflinger/AudioFlinger.h b/services/audioflinger/AudioFlinger.h
index 51881f0..5917632 100644
--- a/services/audioflinger/AudioFlinger.h
+++ b/services/audioflinger/AudioFlinger.h
@@ -514,6 +514,7 @@ private:
             int16_t             *mMainBuffer;
             int32_t             *mAuxBuffer;
             int                 mAuxEffectId;
+            bool                mHasVolumeController;
         };  // end of Track
 
 
@@ -965,6 +966,7 @@ private:
         }
         status_t    setEnabled(bool enabled);
         bool isEnabled();
+        bool isProcessEnabled();
 
         void        setInBuffer(int16_t *buffer) { mConfig.inputCfg.buffer.s16 = buffer; }
         int16_t     *inBuffer() { return mConfig.inputCfg.buffer.s16; }