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

Commit c4eef29a authored by Eric Laurent's avatar Eric Laurent Committed by Gerrit Code Review
Browse files

Merge "fix deadlock issues that arise when there are simultaneous effect...

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"
parents b447379e f90c7e0b
Loading
Loading
Loading
Loading
+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
+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;
+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
+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)
+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;
Loading