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"
diff --git a/include/media/EffectsFactoryApi.h b/include/media/EffectsFactoryApi.h
index b1143b9..b1ed7b0 100644
--- a/include/media/EffectsFactoryApi.h
+++ b/include/media/EffectsFactoryApi.h
@@ -171,30 +171,6 @@
////////////////////////////////////////////////////////////////////////////////
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
diff --git a/media/libeffects/factory/EffectsFactory.c b/media/libeffects/factory/EffectsFactory.c
index f8d6041..6d30d64 100644
--- a/media/libeffects/factory/EffectsFactory.c
+++ b/media/libeffects/factory/EffectsFactory.c
@@ -368,27 +368,21 @@
}
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 @@
// 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 @@
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 @@
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;
diff --git a/media/libeffects/factory/EffectsFactory.h b/media/libeffects/factory/EffectsFactory.h
index 147ff18..560b485 100644
--- a/media/libeffects/factory/EffectsFactory.h
+++ b/media/libeffects/factory/EffectsFactory.h
@@ -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 @@
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
diff --git a/media/libeffects/proxy/Android.mk b/media/libeffects/proxy/Android.mk
index d6d6c1e..b438796 100644
--- a/media/libeffects/proxy/Android.mk
+++ b/media/libeffects/proxy/Android.mk
@@ -28,7 +28,8 @@
LOCAL_C_INCLUDES := \
system/media/audio_effects/include \
- bionic/libc/include
+ bionic/libc/include \
+ frameworks/av/media/libeffects/factory
include $(BUILD_SHARED_LIBRARY)
diff --git a/media/libeffects/proxy/EffectProxy.cpp b/media/libeffects/proxy/EffectProxy.cpp
index e6faaaf..62d3fd3 100644
--- a/media/libeffects/proxy/EffectProxy.cpp
+++ b/media/libeffects/proxy/EffectProxy.cpp
@@ -56,6 +56,8 @@
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 @@
// 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 @@
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 @@
}
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 @@
}
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 @@
// 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(
- pContext->eHandle[SUB_FX_OFFLOAD], cmdCode, cmdSize,
- pCmdData, replySize, pReplyData);
- return status;
+ 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);
+ }
+ *(int*)pReplyData = NO_ERROR;
+ ALOGV("Effect_command OFFLOAD return 0, replyData %d",
+ *(int*)pReplyData);
+
+ return NO_ERROR;
}
int index = pContext->index;
diff --git a/media/libeffects/proxy/EffectProxy.h b/media/libeffects/proxy/EffectProxy.h
index acbe17e..046b93e 100644
--- a/media/libeffects/proxy/EffectProxy.h
+++ b/media/libeffects/proxy/EffectProxy.h
@@ -16,6 +16,8 @@
#include <hardware/audio.h>
#include <hardware/audio_effect.h>
+#include "EffectsFactory.h"
+
namespace android {
enum {
SUB_FX_HOST, // Index of HOST in the descriptor and handle arrays
@@ -62,7 +64,9 @@
struct EffectContext {
const struct effect_interface_s *common_itfe; // Holds the itfe of the Proxy
+ sub_effect_entry_t** sube; // Points to the sub effects
effect_descriptor_t* desc; // Points to the sub effect descriptors
+ audio_effect_library_t** aeli; // Points to the sub effect aeli
effect_handle_t eHandle[SUB_FX_COUNT]; // The effect handles of the sub effects
int index; // The index that is currently active - HOST or OFFLOAD
int32_t sessionId; // The sessiond in which the effect is created.