am 8e886be4: Merge "Modifications in audio effect engine state management." into gingerbread

Merge commit '8e886be413570fab6c623930a5775dd36a6d11e1' into gingerbread-plus-aosp

* commit '8e886be413570fab6c623930a5775dd36a6d11e1':
  Modifications in audio effect engine state management.

include/media/EffectApi.h

@@ -223,6 +223,11 @@ typedef struct audio_buffer_s audio_buffer_t;
 //          samples as specified in output buffer descriptor. If the buffer descriptor
 //          is not specified the function must use either the buffer or the
 //          buffer provider function installed by the EFFECT_CMD_CONFIGURE command.
+//          The effect framework will call the process() function after the EFFECT_CMD_ENABLE
+//          command is received and until the EFFECT_CMD_DISABLE is received. When the engine
+//          receives the EFFECT_CMD_DISABLE command it should turn off the effect gracefully
+//          and when done indicate that it is OK to stop calling the process() function by
+//          returning the -ENODATA status.
 //
 //    NOTE: the process() function implementation should be "real-time safe" that is
 //      it should not perform blocking calls: malloc/free, sleep, read/write/open/close,
@@ -239,6 +244,8 @@ typedef struct audio_buffer_s audio_buffer_t;
 //
 //    Output:
 //        returned value:    0 successful operation
+//                          -ENODATA the engine has finished the disable phase and the framework
+//                                  can stop calling process()
 //                          -EINVAL invalid interface handle or
 //                                  invalid input/output buffer description
 ////////////////////////////////////////////////////////////////////////////////

libs/audioflinger/AudioFlinger.cpp

@@ -17,8 +17,7 @@
 
 
 #define LOG_TAG "AudioFlinger"
-//
+//#define LOG_NDEBUG 0
-#define LOG_NDEBUG 0
 
 #include <math.h>
 #include <signal.h>
@@ -5085,15 +5084,53 @@ void AudioFlinger::EffectModule::disconnect(const wp<EffectHandle>& handle)
     }
 }
 
+void AudioFlinger::EffectModule::updateState() {
+    Mutex::Autolock _l(mLock);
+
+    switch (mState) {
+    case RESTART:
+        reset_l();
+        // FALL THROUGH
+
+    case STARTING:
+        // clear auxiliary effect input buffer for next accumulation
+        if ((mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_AUXILIARY) {
+            memset(mConfig.inputCfg.buffer.raw,
+                   0,
+                   mConfig.inputCfg.buffer.frameCount*sizeof(int32_t));
+        }
+        start_l();
+        mState = ACTIVE;
+        break;
+    case STOPPING:
+        stop_l();
+        mDisableWaitCnt = mMaxDisableWaitCnt;
+        mState = STOPPED;
+        break;
+    case STOPPED:
+        // mDisableWaitCnt is forced to 1 by process() when the engine indicates the end of the
+        // turn off sequence.
+        if (--mDisableWaitCnt == 0) {
+            reset_l();
+            mState = IDLE;
+        }
+        break;
+    default: //IDLE , ACTIVE
+        break;
+    }
+}
+
 void AudioFlinger::EffectModule::process()
 {
     Mutex::Autolock _l(mLock);
 
-    if (mEffectInterface == NULL || mConfig.inputCfg.buffer.raw == NULL || mConfig.outputCfg.buffer.raw == NULL) {
+    if (mEffectInterface == NULL ||
+            mConfig.inputCfg.buffer.raw == NULL ||
+            mConfig.outputCfg.buffer.raw == NULL) {
         return;
     }
 
-    if (mState != IDLE) {
+    if (mState == ACTIVE || mState == STOPPING || mState == STOPPED) {
         // 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,
@@ -5101,33 +5138,15 @@ void AudioFlinger::EffectModule::process()
                                         mConfig.inputCfg.buffer.frameCount);
         }
 
-        // TODO: handle effects with buffer provider
-        if (mState != ACTIVE) {
-            switch (mState) {
-            case RESET:
-                reset_l();
-                mState = STARTING;
-                // clear auxiliary effect input buffer for next accumulation
-                if ((mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_AUXILIARY) {
-                    memset(mConfig.inputCfg.buffer.raw, 0, mConfig.inputCfg.buffer.frameCount*sizeof(int32_t));
-                }
-                return;
-            case STARTING:
-                start_l();
-                mState = ACTIVE;
-                break;
-            case STOPPING:
-                mState = STOPPED;
-                break;
-            case STOPPED:
-                stop_l();
-                mState = IDLE;
-                return;
-            }
-        }
-
         // do the actual processing in the effect engine
-        (*mEffectInterface)->process(mEffectInterface, &mConfig.inputCfg.buffer, &mConfig.outputCfg.buffer);
+        int ret = (*mEffectInterface)->process(mEffectInterface,
+                                               &mConfig.inputCfg.buffer,
+                                               &mConfig.outputCfg.buffer);
+
+        // force transition to IDLE state when engine is ready
+        if (mState == STOPPED && ret == -ENODATA) {
+            mDisableWaitCnt = 1;
+        }
 
         // clear auxiliary effect input buffer for next accumulation
         if ((mDescriptor.flags & EFFECT_FLAG_TYPE_MASK) == EFFECT_FLAG_TYPE_AUXILIARY) {
@@ -5216,6 +5235,10 @@ status_t AudioFlinger::EffectModule::configure()
     if (status == 0) {
         status = cmdStatus;
     }
+
+    mMaxDisableWaitCnt = (MAX_DISABLE_TIME_MS * mConfig.outputCfg.samplingRate) /
+            (1000 * mConfig.outputCfg.buffer.frameCount);
+
     return status;
 }
 
@@ -5292,21 +5315,19 @@ status_t AudioFlinger::EffectModule::setEnabled(bool enabled)
         switch (mState) {
         // going from disabled to enabled
         case IDLE:
-            mState = RESET;
+            mState = STARTING;
+            break;
+        case STOPPED:
+            mState = RESTART;
             break;
         case STOPPING:
             mState = ACTIVE;
             break;
-        case STOPPED:
-            mState = STARTING;
-            break;
 
         // going from enabled to disabled
-        case RESET:
+        case RESTART:
-            mState = IDLE;
-            break;
         case STARTING:
-            mState = STOPPED;
+            mState = IDLE;
             break;
         case ACTIVE:
             mState = STOPPING;
@@ -5325,7 +5346,7 @@ status_t AudioFlinger::EffectModule::setEnabled(bool enabled)
 bool AudioFlinger::EffectModule::isEnabled()
 {
     switch (mState) {
-    case RESET:
+    case RESTART:
     case STARTING:
     case ACTIVE:
         return true;
@@ -5772,6 +5793,9 @@ void AudioFlinger::EffectChain::process_l()
     for (size_t i = 0; i < size; i++) {
         mEffects[i]->process();
     }
+    for (size_t i = 0; i < size; i++) {
+        mEffects[i]->updateState();
+    }
     // if no track is active, input buffer must be cleared here as the mixer process
     // will not do it
     if (mSessionId > 0 && activeTracks() == 0) {

libs/audioflinger/AudioFlinger.h

@@ -905,7 +905,7 @@ private:
 
         enum effect_state {
             IDLE,
-            RESET,
+            RESTART,
             STARTING,
             ACTIVE,
             STOPPING,
@@ -914,6 +914,7 @@ private:
 
         int         id() { return mId; }
         void process();
+        void updateState();
         status_t command(int cmdCode, int cmdSize, void *pCmdData, int *replySize, void *pReplyData);
 
         void reset_l();
@@ -948,6 +949,9 @@ private:
 
     protected:
 
+        // Maximum time allocated to effect engines to complete the turn off sequence
+        static const uint32_t MAX_DISABLE_TIME_MS = 10000;
+
         EffectModule(const EffectModule&);
         EffectModule& operator = (const EffectModule&);
 
@@ -973,6 +977,9 @@ private:
         status_t mStatus;               // initialization status
         uint32_t mState;                // current activation state (effect_state)
         Vector< wp<EffectHandle> > mHandles;    // list of client handles
+        uint32_t mMaxDisableWaitCnt;    // maximum grace period before forcing an effect off after
+                                        // sending disable command.
+        uint32_t mDisableWaitCnt;       // current process() calls count during disable period.
     };
 
     // The EffectHandle class implements the IEffect interface. It provides resources

media/libeffects/EffectEqualizer.cpp

@@ -30,6 +30,12 @@
 // effect_interface_t interface implementation for equalizer effect
 extern "C" const struct effect_interface_s gEqualizerInterface;
 
+enum equalizer_state_e {
+    EQUALIZER_STATE_UNINITIALIZED,
+    EQUALIZER_STATE_INITIALIZED,
+    EQUALIZER_STATE_ACTIVE,
+};
+
 namespace android {
 namespace {
 
@@ -100,6 +106,7 @@ struct EqualizerContext {
     effect_config_t config;
     FormatAdapter adapter;
     AudioEqualizer * pEqualizer;
+    uint32_t state;
 };
 
 //--- local function prototypes
@@ -151,6 +158,7 @@ extern "C" int EffectCreate(effect_uuid_t *uuid,
 
     pContext->itfe = &gEqualizerInterface;
     pContext->pEqualizer = NULL;
+    pContext->state = EQUALIZER_STATE_UNINITIALIZED;
 
     ret = Equalizer_init(pContext);
     if (ret < 0) {
@@ -160,6 +168,7 @@ extern "C" int EffectCreate(effect_uuid_t *uuid,
     }
 
     *pInterface = (effect_interface_t)pContext;
+    pContext->state = EQUALIZER_STATE_INITIALIZED;
 
     LOGV("EffectLibCreateEffect %p, size %d", pContext, AudioEqualizer::GetInstanceSize(kNumBands)+sizeof(EqualizerContext));
 
@@ -175,6 +184,7 @@ extern "C" int EffectRelease(effect_interface_t interface) {
         return -EINVAL;
     }
 
+    pContext->state = EQUALIZER_STATE_UNINITIALIZED;
     pContext->pEqualizer->free();
     delete pContext;
 
@@ -528,6 +538,13 @@ extern "C" int Equalizer_process(effect_interface_t self, audio_buffer_t *inBuff
         return -EINVAL;
     }
 
+    if (pContext->state == EQUALIZER_STATE_UNINITIALIZED) {
+        return -EINVAL;
+    }
+    if (pContext->state == EQUALIZER_STATE_INITIALIZED) {
+        return -ENODATA;
+    }
+
     pContext->adapter.process(inBuffer->raw, outBuffer->raw, outBuffer->frameCount);
 
     return 0;
@@ -539,7 +556,7 @@ extern "C" int Equalizer_command(effect_interface_t self, int cmdCode, int cmdSi
     android::EqualizerContext * pContext = (android::EqualizerContext *) self;
     int retsize;
 
-    if (pContext == NULL) {
+    if (pContext == NULL || pContext->state == EQUALIZER_STATE_UNINITIALIZED) {
         return -EINVAL;
     }
 
@@ -594,10 +611,25 @@ extern "C" int Equalizer_command(effect_interface_t self, int cmdCode, int cmdSi
                 p->data + p->psize);
         } break;
     case EFFECT_CMD_ENABLE:
+        if (pReplyData == NULL || *replySize != sizeof(int)) {
+            return -EINVAL;
+        }
+        if (pContext->state != EQUALIZER_STATE_INITIALIZED) {
+            return -ENOSYS;
+        }
+        pContext->state = EQUALIZER_STATE_ACTIVE;
+        LOGV("EFFECT_CMD_ENABLE() OK");
+        *(int *)pReplyData = 0;
+        break;
     case EFFECT_CMD_DISABLE:
         if (pReplyData == NULL || *replySize != sizeof(int)) {
             return -EINVAL;
         }
+        if (pContext->state != EQUALIZER_STATE_ACTIVE) {
+            return -ENOSYS;
+        }
+        pContext->state = EQUALIZER_STATE_INITIALIZED;
+        LOGV("EFFECT_CMD_DISABLE() OK");
         *(int *)pReplyData = 0;
         break;
     case EFFECT_CMD_SET_DEVICE:

media/libeffects/EffectReverb.c

@@ -15,8 +15,7 @@
  */
 
 #define LOG_TAG "EffectReverb"
-//
+//#define LOG_NDEBUG 0
-#define LOG_NDEBUG 0
 #include <cutils/log.h>
 #include <stdlib.h>
 #include <string.h>
@@ -143,6 +142,8 @@ int EffectCreate(effect_uuid_t *uuid,
 
     module->itfe = &gReverbInterface;
 
+    module->context.mState = REVERB_STATE_UNINITIALIZED;
+
     if (memcmp(&desc->type, SL_IID_PRESETREVERB, sizeof(effect_uuid_t)) == 0) {
         preset = 1;
     }
@@ -158,6 +159,8 @@ int EffectCreate(effect_uuid_t *uuid,
 
     *pInterface = (effect_interface_t) module;
 
+    module->context.mState = REVERB_STATE_INITIALIZED;
+
     LOGV("EffectLibCreateEffect %p ,size %d", module, sizeof(reverb_module_t));
 
     return 0;
@@ -171,6 +174,8 @@ int EffectRelease(effect_interface_t interface) {
         return -EINVAL;
     }
 
+    pRvbModule->context.mState = REVERB_STATE_UNINITIALIZED;
+
     free(pRvbModule);
     return 0;
 }
@@ -195,6 +200,13 @@ static int Reverb_Process(effect_interface_t self, audio_buffer_t *inBuffer, aud
 
     pReverb = (reverb_object_t*) &pRvbModule->context;
 
+    if (pReverb->mState == REVERB_STATE_UNINITIALIZED) {
+        return -EINVAL;
+    }
+    if (pReverb->mState == REVERB_STATE_INITIALIZED) {
+        return -ENODATA;
+    }
+
     //if bypassed or the preset forces the signal to be completely dry
     if (pReverb->m_bBypass != 0) {
         if (inBuffer->raw != outBuffer->raw) {
@@ -257,13 +269,15 @@ static int Reverb_Process(effect_interface_t self, audio_buffer_t *inBuffer, aud
     return 0;
 }
 
+
 static int Reverb_Command(effect_interface_t self, int cmdCode, int cmdSize,
         void *pCmdData, int *replySize, void *pReplyData) {
     reverb_module_t *pRvbModule = (reverb_module_t *) self;
     reverb_object_t *pReverb;
     int retsize;
 
-    if (pRvbModule == NULL) {
+    if (pRvbModule == NULL ||
+            pRvbModule->context.mState == REVERB_STATE_UNINITIALIZED) {
         return -EINVAL;
     }
 
@@ -277,6 +291,9 @@ static int Reverb_Command(effect_interface_t self, int cmdCode, int cmdSize,
             return -EINVAL;
         }
         *(int *) pReplyData = Reverb_Init(pRvbModule, pReverb->m_Aux, pReverb->m_Preset);
+        if (*(int *) pReplyData == 0) {
+            pRvbModule->context.mState = REVERB_STATE_INITIALIZED;
+        }
         break;
     case EFFECT_CMD_CONFIGURE:
         if (pCmdData == NULL || cmdSize != sizeof(effect_config_t)
@@ -315,10 +332,25 @@ static int Reverb_Command(effect_interface_t self, int cmdCode, int cmdSize,
                 cmd->vsize, cmd->data + sizeof(int32_t));
         break;
     case EFFECT_CMD_ENABLE:
+        if (pReplyData == NULL || *replySize != sizeof(int)) {
+            return -EINVAL;
+        }
+        if (pReverb->mState != REVERB_STATE_INITIALIZED) {
+            return -ENOSYS;
+        }
+        pReverb->mState = REVERB_STATE_ACTIVE;
+        LOGV("EFFECT_CMD_ENABLE() OK");
+        *(int *)pReplyData = 0;
+        break;
     case EFFECT_CMD_DISABLE:
         if (pReplyData == NULL || *replySize != sizeof(int)) {
             return -EINVAL;
         }
+        if (pReverb->mState != REVERB_STATE_ACTIVE) {
+            return -ENOSYS;
+        }
+        pReverb->mState = REVERB_STATE_INITIALIZED;
+        LOGV("EFFECT_CMD_DISABLE() OK");
         *(int *)pReplyData = 0;
         break;
     case EFFECT_CMD_SET_DEVICE:

media/libeffects/EffectReverb.h

@@ -114,6 +114,12 @@ int32_t nPanG1 = -1.0 for cos
 #define AP1_GAIN_RANGE (int)(22936-6553)
 
 
+enum reverb_state_e {
+    REVERB_STATE_UNINITIALIZED,
+    REVERB_STATE_INITIALIZED,
+    REVERB_STATE_ACTIVE,
+};
+
 /* parameters for each allpass */
 typedef struct
 {
@@ -279,6 +285,7 @@ typedef struct
     uint16_t            m_Aux;                // if TRUE, is connected as auxiliary effect
     uint16_t            m_Preset;             // if TRUE, expose preset revert interface
 
+    uint32_t            mState;
 } reverb_object_t;
 
 

media/libeffects/EffectVisualizer.cpp

@@ -231,7 +231,7 @@ extern "C" int Visualizer_process(
         return -EINVAL;
     }
     if (pContext->mState != VISUALIZER_STATE_ACTIVE) {
-        return -ENOSYS;
+        return -ENODATA;
     }
 
     if (inBuffer == NULL || inBuffer->raw == NULL ||