Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit af1aa00e authored by Eric Laurent's avatar Eric Laurent Committed by Android Git Automerger
Browse files

am c4eef29a: Merge "fix deadlock issues that arise when there are simultaneous... 

am c4eef29a: Merge "fix deadlock issues that arise when there are simultaneous effect control interface calls to proxy and to non sub-effect wrappers(eg., bundlewrapper) from audioflinger Also, return NO_ERROR when CMD_OFFLOAD succeeds"

* commit 'c4eef29a':
  fix deadlock issues that arise when there are simultaneous effect control interface calls to proxy and to non sub-effect wrappers(eg., bundlewrapper) from audioflinger Also, return NO_ERROR when CMD_OFFLOAD succeeds
parents 1158e2f9 c4eef29a

include/media/EffectsFactoryApi.h

+0 −24
Original line number Diff line number Diff line
@@ -171,30 +171,6 @@ int EffectGetDescriptor(const effect_uuid_t *pEffectUuid, effect_descriptor_t *p 
////////////////////////////////////////////////////////////////////////////////

int EffectIsNullUuid(const effect_uuid_t *pEffectUuid);



////////////////////////////////////////////////////////////////////////////////

//

//    Function:       EffectGetSubEffects

//

//    Description:    Returns the descriptors of the sub effects of the effect

//                    whose uuid is pointed to by first argument.

//

//    Input:

//          pEffectUuid:    pointer to the effect uuid.

//          size:           size of the buffer pointed by pDescriptor.

//

//    Input/Output:

//          pDescriptor:    address where to return the sub effect descriptors.

//

//    Output:

//        returned value:    0          successful operation.

//                          -ENODEV     factory failed to initialize

//                          -EINVAL     invalid pEffectUuid or pDescriptor

//                          -ENOENT     no effect with this uuid found

//        *pDescriptor:     updated with the sub effect descriptors.

//

////////////////////////////////////////////////////////////////////////////////

int EffectGetSubEffects(const effect_uuid_t *pEffectUuid, effect_descriptor_t *pDescriptors, size_t size);



#if __cplusplus

}  // extern "C"

#endif

media/libeffects/factory/EffectsFactory.c

+6 −14
Original line number Diff line number Diff line
@@ -368,27 +368,21 @@ int EffectRelease(effect_handle_t handle) 
    }

    if (e1 == NULL) {

        ret = -ENOENT;

        pthread_mutex_unlock(&gLibLock);

        goto exit;

    }



    // release effect in library

    if (fx->lib == NULL) {

        ALOGW("EffectRelease() fx %p library already unloaded", handle);

        pthread_mutex_unlock(&gLibLock);

    } else {

        pthread_mutex_lock(&fx->lib->lock);

        // Releasing the gLibLock here as the list access is over as the

        // effect is removed from the list.

        // If the gLibLock is not released, we will have a deadlock situation

        // since we call the sub effect release inside the EffectRelease of Proxy

        pthread_mutex_unlock(&gLibLock);

        fx->lib->desc->release_effect(fx->subItfe);

        pthread_mutex_unlock(&fx->lib->lock);

    }

    free(fx);



exit:

    pthread_mutex_unlock(&gLibLock);

    return ret;

}
@@ -404,8 +398,8 @@ int EffectIsNullUuid(const effect_uuid_t *uuid) 
// is pointed by the first argument. It searches the gSubEffectList for the

// matching uuid and then copies the corresponding sub effect descriptors

// to the inout param

int EffectGetSubEffects(const effect_uuid_t *uuid,

                        effect_descriptor_t *pDescriptors, size_t size)

int EffectGetSubEffects(const effect_uuid_t *uuid, sub_effect_entry_t **pSube,

                        size_t size)

{

   ALOGV("EffectGetSubEffects() UUID: %08X-%04X-%04X-%04X-%02X%02X%02X%02X%02X"

          "%02X\n",uuid->timeLow, uuid->timeMid, uuid->timeHiAndVersion,
@@ -413,8 +407,7 @@ int EffectGetSubEffects(const effect_uuid_t *uuid, 
          uuid->node[3],uuid->node[4],uuid->node[5]);



   // Check if the size of the desc buffer is large enough for 2 subeffects

   if ((uuid == NULL) || (pDescriptors == NULL) ||

       (size < 2*sizeof(effect_descriptor_t))) {

   if ((uuid == NULL) || (pSube == NULL) || (size < 2)) {

       ALOGW("NULL pointer or insufficient memory. Cannot query subeffects");

       return -EINVAL;

   }
@@ -432,11 +425,10 @@ int EffectGetSubEffects(const effect_uuid_t *uuid, 
           list_elem_t *subefx = e->sub_elem;

           while (subefx != NULL) {

               subeffect = (sub_effect_entry_t*)subefx->object;

               d = (effect_descriptor_t*)(subeffect->object);

               pDescriptors[count++] = *d;

               pSube[count++] = subeffect;

               subefx = subefx->next;

           }

           ALOGV("EffectGetSubEffects end - copied the sub effect descriptors");

           ALOGV("EffectGetSubEffects end - copied the sub effect structures");

           return count;

       }

       e = e->next;

media/libeffects/factory/EffectsFactory.h

+27 −1
Original line number Diff line number Diff line
@@ -20,7 +20,7 @@ 
#include <cutils/log.h>

#include <pthread.h>

#include <dirent.h>

#include <media/EffectsFactoryApi.h>

#include <hardware/audio_effect.h>



#if __cplusplus

extern "C" {
@@ -66,6 +66,32 @@ typedef struct sub_effect_entry_s { 
    void *object;

} sub_effect_entry_t;





////////////////////////////////////////////////////////////////////////////////

//

//    Function:       EffectGetSubEffects

//

//    Description:    Returns the descriptors of the sub effects of the effect

//                    whose uuid is pointed to by first argument.

//

//    Input:

//          pEffectUuid:    pointer to the effect uuid.

//          size:           max number of sub_effect_entry_t * in pSube.

//

//    Input/Output:

//          pSube:          address where to return the sub effect structures.

//    Output:

//        returned value:    0          successful operation.

//                          -ENODEV     factory failed to initialize

//                          -EINVAL     invalid pEffectUuid or pDescriptor

//                          -ENOENT     no effect with this uuid found

//        *pDescriptor:     updated with the sub effect descriptors.

//

////////////////////////////////////////////////////////////////////////////////

int EffectGetSubEffects(const effect_uuid_t *pEffectUuid,

                        sub_effect_entry_t **pSube,

                        size_t size);



#if __cplusplus

}  // extern "C"

#endif

media/libeffects/proxy/Android.mk

+2 −1
Original line number Diff line number Diff line
@@ -28,7 +28,8 @@ LOCAL_SHARED_LIBRARIES := liblog libcutils libutils libdl libeffects 


LOCAL_C_INCLUDES := \

        system/media/audio_effects/include \

        bionic/libc/include

        bionic/libc/include \

        frameworks/av/media/libeffects/factory



include $(BUILD_SHARED_LIBRARY)

media/libeffects/proxy/EffectProxy.cpp

+49 −15
Original line number Diff line number Diff line
@@ -56,6 +56,8 @@ int EffectProxyCreate(const effect_uuid_t *uuid, 
                           effect_handle_t  *pHandle) {



    effect_descriptor_t* desc;

    audio_effect_library_t** aeli;

    sub_effect_entry_t** sube;

    EffectContext* pContext;

    if (pHandle == NULL || uuid == NULL) {

        ALOGE("EffectProxyCreate() called with NULL pointer");
@@ -74,31 +76,52 @@ int EffectProxyCreate(const effect_uuid_t *uuid, 


    // Get the HW and SW sub effect descriptors from the effects factory

    desc = new effect_descriptor_t[SUB_FX_COUNT];

    aeli = new audio_effect_library_t*[SUB_FX_COUNT];

    sube = new sub_effect_entry_t*[SUB_FX_COUNT];

    pContext->sube = new sub_effect_entry_t*[SUB_FX_COUNT];

    pContext->desc = new effect_descriptor_t[SUB_FX_COUNT];

    int retValue = EffectGetSubEffects(uuid, desc,

                                sizeof(effect_descriptor_t) * SUB_FX_COUNT);

    pContext->aeli = new audio_effect_library_t*[SUB_FX_COUNT];

    int retValue = EffectGetSubEffects(uuid, sube, SUB_FX_COUNT);

    // EffectGetSubEffects returns the number of sub-effects copied.

    if (retValue != SUB_FX_COUNT) {

       ALOGE("EffectCreate() could not get the sub effects");

       delete desc;

       delete pContext->desc;

       delete[] sube;

       delete[] desc;

       delete[] aeli;

       delete[] pContext->sube;

       delete[] pContext->desc;

       delete[] pContext->aeli;

       return -EINVAL;

    }

    // Check which is the HW descriptor and copy the descriptors

    // to the Context desc array

    // Also check if there is only one HW and one SW descriptor.

    // HW descriptor alone has the HW_TUNNEL flag.

    desc[0] = *(effect_descriptor_t*)(sube[0])->object;

    desc[1] = *(effect_descriptor_t*)(sube[1])->object;

    aeli[0] = sube[0]->lib->desc;

    aeli[1] = sube[1]->lib->desc;

    if ((desc[0].flags & EFFECT_FLAG_HW_ACC_TUNNEL) &&

       !(desc[1].flags & EFFECT_FLAG_HW_ACC_TUNNEL)) {

        pContext->sube[SUB_FX_OFFLOAD] = sube[0];

        pContext->desc[SUB_FX_OFFLOAD] = desc[0];

        pContext->aeli[SUB_FX_OFFLOAD] = aeli[0];

        pContext->sube[SUB_FX_HOST] = sube[1];

        pContext->desc[SUB_FX_HOST] = desc[1];

        pContext->aeli[SUB_FX_HOST] = aeli[1];

    }

    else if ((desc[1].flags & EFFECT_FLAG_HW_ACC_TUNNEL) &&

             !(desc[0].flags & EFFECT_FLAG_HW_ACC_TUNNEL)) {

        pContext->sube[SUB_FX_HOST] = sube[0];

        pContext->desc[SUB_FX_HOST] = desc[0];

        pContext->aeli[SUB_FX_HOST] = aeli[0];

        pContext->sube[SUB_FX_OFFLOAD] = sube[1];

        pContext->desc[SUB_FX_OFFLOAD] = desc[1];

        pContext->aeli[SUB_FX_OFFLOAD] = aeli[1];

    }

    delete desc;

    delete[] desc;

    delete[] aeli;

    delete[] sube;

#if (LOG_NDEBUG == 0)

    effect_uuid_t uuid_print = pContext->desc[SUB_FX_HOST].uuid;

    ALOGV("EffectCreate() UUID of HOST: %08X-%04X-%04X-%04X-%02X%02X%02X%02X"
@@ -128,13 +151,15 @@ int EffectProxyRelease(effect_handle_t handle) { 
        return -EINVAL;

    }

    ALOGV("EffectRelease");

    delete pContext->desc;

    delete[] pContext->desc;

    free(pContext->replyData);



    if (pContext->eHandle[SUB_FX_HOST])

       EffectRelease(pContext->eHandle[SUB_FX_HOST]);

       pContext->aeli[SUB_FX_HOST]->release_effect(pContext->eHandle[SUB_FX_HOST]);

    if (pContext->eHandle[SUB_FX_OFFLOAD])

       EffectRelease(pContext->eHandle[SUB_FX_OFFLOAD]);

       pContext->aeli[SUB_FX_OFFLOAD]->release_effect(pContext->eHandle[SUB_FX_OFFLOAD]);

    delete[] pContext->aeli;

    delete[] pContext->sube;

    delete pContext;

    pContext = NULL;

    return 0;
@@ -187,7 +212,8 @@ int Effect_command(effect_handle_t self, 
    }

    if (pContext->eHandle[SUB_FX_HOST] == NULL) {

        ALOGV("Effect_command() Calling HOST EffectCreate");

        status = EffectCreate(&pContext->desc[SUB_FX_HOST].uuid,

        status = pContext->aeli[SUB_FX_HOST]->create_effect(

                              &pContext->desc[SUB_FX_HOST].uuid,

                              pContext->sessionId, pContext->ioId,

                              &(pContext->eHandle[SUB_FX_HOST]));

        if (status != NO_ERROR || (pContext->eHandle[SUB_FX_HOST] == NULL)) {
@@ -197,11 +223,13 @@ int Effect_command(effect_handle_t self, 
    }

    if (pContext->eHandle[SUB_FX_OFFLOAD] == NULL) {

        ALOGV("Effect_command() Calling OFFLOAD EffectCreate");

        status = EffectCreate(&pContext->desc[SUB_FX_OFFLOAD].uuid,

        status = pContext->aeli[SUB_FX_OFFLOAD]->create_effect(

                              &pContext->desc[SUB_FX_OFFLOAD].uuid,

                              pContext->sessionId, pContext->ioId,

                              &(pContext->eHandle[SUB_FX_OFFLOAD]));

        if (status != NO_ERROR || (pContext->eHandle[SUB_FX_OFFLOAD] == NULL)) {

            ALOGV("Effect_command() Error creating HW effect");

            pContext->eHandle[SUB_FX_OFFLOAD] = NULL;

            // Do not return error here as SW effect is created

            // Return error if the CMD_OFFLOAD sends the index as OFFLOAD

        }
@@ -233,11 +261,17 @@ int Effect_command(effect_handle_t self, 
        // Update the DSP wrapper with the new ioHandle.

        // Pass the OFFLOAD command to the wrapper.

        // The DSP wrapper needs to handle this CMD

        if (pContext->eHandle[SUB_FX_OFFLOAD])

            status = (*pContext->eHandle[SUB_FX_OFFLOAD])->command(

        if (pContext->eHandle[SUB_FX_OFFLOAD]) {

            ALOGV("Effect_command: Calling OFFLOAD command");

            return (*pContext->eHandle[SUB_FX_OFFLOAD])->command(

                           pContext->eHandle[SUB_FX_OFFLOAD], cmdCode, cmdSize,

                           pCmdData, replySize, pReplyData);

        return status;

        }

        *(int*)pReplyData = NO_ERROR;

        ALOGV("Effect_command OFFLOAD return 0, replyData %d",

                                                *(int*)pReplyData);



        return NO_ERROR;

    }



    int index = pContext->index;