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

Commit 9acfdcf2 authored by Treehugger Robot's avatar Treehugger Robot Committed by Gerrit Code Review
Browse files

Merge changes I8c36f5af,I800bc352 

* changes:
  AudioEffect: fix racy access to callback data
  Analyze native crash in AudioEffect via event listener
parents 61c3d559 9690a02f

media/jni/audioeffect/android_media_AudioEffect.cpp

+35 −15
Original line number Diff line number Diff line
@@ -14,8 +14,8 @@ 
 * limitations under the License.

 */





#include <stdio.h>

#include <unordered_set>



//#define LOG_NDEBUG 0

#define LOG_TAG "AudioEffects-JNI"
@@ -58,22 +58,15 @@ static fields_t fields; 
struct effect_callback_cookie {

    jclass      audioEffect_class;  // AudioEffect class

    jobject     audioEffect_ref;    // AudioEffect object instance

    bool        busy;

    Condition   cond;

};



// ----------------------------------------------------------------------------

class AudioEffectJniStorage {

    public:

        effect_callback_cookie mCallbackData;



    AudioEffectJniStorage() {

    }



    ~AudioEffectJniStorage() {

    }



struct AudioEffectJniStorage {

    effect_callback_cookie mCallbackData{};

};





jint AudioEffectJni::translateNativeErrorToJava(int code) {

    switch(code) {

    case NO_ERROR:
@@ -101,6 +94,7 @@ jint AudioEffectJni::translateNativeErrorToJava(int code) { 
}



static Mutex sLock;

static std::unordered_set<effect_callback_cookie*> sAudioEffectCallBackCookies;



// ----------------------------------------------------------------------------

static void effectCallback(int event, void* user, void *info) {
@@ -121,7 +115,13 @@ static void effectCallback(int event, void* user, void *info) { 
        ALOGW("effectCallback error user %p, env %p", user, env);

        return;

    }



    {

        Mutex::Autolock l(sLock);

        if (sAudioEffectCallBackCookies.count(callbackInfo) == 0) {

            return;

        }

        callbackInfo->busy = true;

    }

    ALOGV("effectCallback: callbackInfo %p, audioEffect_ref %p audioEffect_class %p",

            callbackInfo,

            callbackInfo->audioEffect_ref,
@@ -188,6 +188,11 @@ effectCallback_Exit: 
        env->ExceptionDescribe();

        env->ExceptionClear();

    }

    {

        Mutex::Autolock l(sLock);

        callbackInfo->busy = false;

        callbackInfo->cond.broadcast();

    }

}



// ----------------------------------------------------------------------------
@@ -396,6 +401,10 @@ android_media_AudioEffect_native_setup(JNIEnv *env, jobject thiz, jobject weak_t 
        setAudioEffect(env, thiz, lpAudioEffect);

    }



    {

        Mutex::Autolock l(sLock);

        sAudioEffectCallBackCookies.insert(&lpJniStorage->mCallbackData);

    }

    env->SetLongField(thiz, fields.fidJniData, (jlong)lpJniStorage);



    return (jint) AUDIOEFFECT_SUCCESS;
@@ -427,6 +436,7 @@ setup_failure: 




// ----------------------------------------------------------------------------

#define CALLBACK_COND_WAIT_TIMEOUT_MS 1000

static void android_media_AudioEffect_native_release(JNIEnv *env,  jobject thiz) {

    sp<AudioEffect> lpAudioEffect = setAudioEffect(env, thiz, 0);

    if (lpAudioEffect == 0) {
@@ -442,7 +452,17 @@ static void android_media_AudioEffect_native_release(JNIEnv *env, jobject thiz) 
    env->SetLongField(thiz, fields.fidJniData, 0);



    if (lpJniStorage) {

        ALOGV("deleting pJniStorage: %p\n", lpJniStorage);

        Mutex::Autolock l(sLock);

        effect_callback_cookie *lpCookie = &lpJniStorage->mCallbackData;

        ALOGV("deleting lpJniStorage: %p\n", lpJniStorage);

        sAudioEffectCallBackCookies.erase(lpCookie);

        while (lpCookie->busy) {

            if (lpCookie->cond.waitRelative(sLock,

                                            milliseconds(CALLBACK_COND_WAIT_TIMEOUT_MS)) !=

                                                    NO_ERROR) {

                break;

            }

        }

        env->DeleteGlobalRef(lpJniStorage->mCallbackData.audioEffect_class);

        env->DeleteGlobalRef(lpJniStorage->mCallbackData.audioEffect_ref);

        delete lpJniStorage;

media/jni/audioeffect/android_media_Visualizer.cpp

+74 −18
Original line number Diff line number Diff line
@@ -15,6 +15,7 @@ 
 */



#include <stdio.h>

#include <unordered_set>



//#define LOG_NDEBUG 0

#define LOG_TAG "visualizers-JNI"
@@ -63,6 +64,12 @@ struct visualizer_callback_cookie { 
    jclass      visualizer_class;  // Visualizer class

    jobject     visualizer_ref;    // Visualizer object instance



    // 'busy_count' and 'cond' together with 'sLock' are used to serialize

    // concurrent access to the callback cookie from 'setup'/'release'

    // and the callback.

    int         busy_count;

    Condition   cond;



    // Lazily allocated arrays used to hold callback data provided to java

    // applications.  These arrays are allocated during the first callback and

    // reallocated when the size of the callback data changes.  Allocating on
@@ -70,14 +77,12 @@ struct visualizer_callback_cookie { 
    // reference to the provided data (they need to make a copy if they want to

    // hold onto outside of the callback scope), but it avoids GC thrash caused

    // by constantly allocating and releasing arrays to hold callback data.

    // 'callback_data_lock' must never be held at the same time with 'sLock'.

    Mutex       callback_data_lock;

    jbyteArray  waveform_data;

    jbyteArray  fft_data;



    visualizer_callback_cookie() {

        waveform_data = NULL;

        fft_data = NULL;

    }

    // Assumes use of default initialization by the client.



    ~visualizer_callback_cookie() {

        cleanupBuffers();
@@ -102,15 +107,8 @@ struct visualizer_callback_cookie { 
 };



// ----------------------------------------------------------------------------

class VisualizerJniStorage {

    public:

        visualizer_callback_cookie mCallbackData;



    VisualizerJniStorage() {

    }



    ~VisualizerJniStorage() {

    }

struct VisualizerJniStorage {

    visualizer_callback_cookie mCallbackData{};

};
@@ -136,6 +134,7 @@ static jint translateError(int code) { 
}



static Mutex sLock;

static std::unordered_set<visualizer_callback_cookie*> sVisualizerCallBackCookies;



// ----------------------------------------------------------------------------

static void ensureArraySize(JNIEnv *env, jbyteArray *array, uint32_t size) {
@@ -173,11 +172,19 @@ static void captureCallback(void* user, 
        return;

    }



    {

        Mutex::Autolock l(sLock);

        if (sVisualizerCallBackCookies.count(callbackInfo) == 0) {

            return;

        }

        callbackInfo->busy_count++;

    }

    ALOGV("captureCallback: callbackInfo %p, visualizer_ref %p visualizer_class %p",

            callbackInfo,

            callbackInfo->visualizer_ref,

            callbackInfo->visualizer_class);



    {

    AutoMutex lock(&callbackInfo->callback_data_lock);



    if (waveformSize != 0 && waveform != NULL) {
@@ -219,11 +226,17 @@ static void captureCallback(void* user, 
                jArray);

        }

    }

    }  // callback_data_lock scope



    if (env->ExceptionCheck()) {

        env->ExceptionDescribe();

        env->ExceptionClear();

    }

    {

        Mutex::Autolock l(sLock);

        callbackInfo->busy_count--;

        callbackInfo->cond.broadcast();

    }

}



// ----------------------------------------------------------------------------
@@ -332,16 +345,41 @@ static void android_media_visualizer_effect_callback(int32_t event, 
                                                     void *info) {

    if ((event == AudioEffect::EVENT_ERROR) &&

        (*((status_t*)info) == DEAD_OBJECT)) {

        VisualizerJniStorage* lpJniStorage = (VisualizerJniStorage*)user;

        visualizer_callback_cookie* callbackInfo = &lpJniStorage->mCallbackData;

        visualizer_callback_cookie* callbackInfo =

                (visualizer_callback_cookie *)user;

        JNIEnv *env = AndroidRuntime::getJNIEnv();



        if (!user || !env) {

            ALOGW("effectCallback error user %p, env %p", user, env);

            return;

        }

        {

            Mutex::Autolock l(sLock);

            if (sVisualizerCallBackCookies.count(callbackInfo) == 0) {

                return;

            }

            callbackInfo->busy_count++;

        }

        ALOGV("effectCallback: callbackInfo %p, visualizer_ref %p visualizer_class %p",

            callbackInfo,

            callbackInfo->visualizer_ref,

            callbackInfo->visualizer_class);



        env->CallStaticVoidMethod(

            callbackInfo->visualizer_class,

            fields.midPostNativeEvent,

            callbackInfo->visualizer_ref,

            NATIVE_EVENT_SERVER_DIED,

            0, NULL);

        if (env->ExceptionCheck()) {

            env->ExceptionDescribe();

            env->ExceptionClear();

        }

        {

            Mutex::Autolock l(sLock);

            callbackInfo->busy_count--;

            callbackInfo->cond.broadcast();

        }

    }

}
@@ -389,7 +427,7 @@ android_media_visualizer_native_setup(JNIEnv *env, jobject thiz, jobject weak_th 
    }

    lpVisualizer->set(0,

                      android_media_visualizer_effect_callback,

                      lpJniStorage,

                      &lpJniStorage->mCallbackData,

                      (audio_session_t) sessionId);



    lStatus = translateError(lpVisualizer->initCheck());
@@ -410,6 +448,10 @@ android_media_visualizer_native_setup(JNIEnv *env, jobject thiz, jobject weak_th 


    setVisualizer(env, thiz, lpVisualizer);



    {

        Mutex::Autolock l(sLock);

        sVisualizerCallBackCookies.insert(&lpJniStorage->mCallbackData);

    }

    env->SetLongField(thiz, fields.fidJniData, (jlong)lpJniStorage);



    return VISUALIZER_SUCCESS;
@@ -432,13 +474,15 @@ setup_failure: 
}



// ----------------------------------------------------------------------------

#define CALLBACK_COND_WAIT_TIMEOUT_MS 1000

static void android_media_visualizer_native_release(JNIEnv *env,  jobject thiz) {

    { //limit scope so that lpVisualizer is deleted before JNI storage data.

    {

        sp<Visualizer> lpVisualizer = setVisualizer(env, thiz, 0);

        if (lpVisualizer == 0) {

            return;

        }

        lpVisualizer->release();

        // Visualizer can still can be held by AudioEffect::EffectClient

    }

    // delete the JNI data

    VisualizerJniStorage* lpJniStorage =
@@ -449,9 +493,22 @@ static void android_media_visualizer_native_release(JNIEnv *env, jobject thiz) 
    env->SetLongField(thiz, fields.fidJniData, 0);



    if (lpJniStorage) {

        {

        Mutex::Autolock l(sLock);

        visualizer_callback_cookie *lpCookie = &lpJniStorage->mCallbackData;

        ALOGV("deleting pJniStorage: %p\n", lpJniStorage);

        sVisualizerCallBackCookies.erase(lpCookie);

        while (lpCookie->busy_count > 0) {

            if (lpCookie->cond.waitRelative(sLock,

                                            milliseconds(CALLBACK_COND_WAIT_TIMEOUT_MS)) !=

                                                    NO_ERROR) {

                break;

            }

        }

        ALOG_ASSERT(lpCookie->busy_count == 0, "Unbalanced busy_count inc/dec");

        env->DeleteGlobalRef(lpJniStorage->mCallbackData.visualizer_class);

        env->DeleteGlobalRef(lpJniStorage->mCallbackData.visualizer_ref);

        }  // sLock scope

        delete lpJniStorage;

    }

}
@@ -707,4 +764,3 @@ int register_android_media_visualizer(JNIEnv *env) 
{

    return AndroidRuntime::registerNativeMethods(env, kClassPathName, gMethods, NELEM(gMethods));

}

media/tests/EffectsTest/AndroidManifest.xml

+4 −0
Original line number Diff line number Diff line
@@ -13,6 +13,10 @@ 
     See the License for the specific language governing permissions and

     limitations under the License.

-->

<!--

Make sure to enable access to the mic in settings and run:

adb shell am compat enable ALLOW_TEST_API_ACCESS com.android.effectstest

-->



<manifest xmlns:android="http://schemas.android.com/apk/res/android"

    package="com.android.effectstest">

media/tests/EffectsTest/res/layout/bassboosttest.xml

+5 −0
Original line number Diff line number Diff line
@@ -187,6 +187,11 @@ 
                 android:layout_height="wrap_content"

                 android:scaleType="fitXY"/>



            <Button android:id="@+id/hammer_on_release_bug"

                    android:layout_width="fill_parent" android:layout_height="wrap_content"

                    android:text="@string/hammer_on_release_bug_name">

            </Button>



        </LinearLayout>



    </ScrollView>

media/tests/EffectsTest/res/layout/visualizertest.xml

+5 −0
Original line number Diff line number Diff line
@@ -175,6 +175,11 @@ 


    </LinearLayout>



    <Button android:id="@+id/hammer_on_release_bug"

            android:layout_width="fill_parent" android:layout_height="wrap_content"

            android:text="@string/hammer_on_release_bug_name">

    </Button>



    <ImageView

         android:src="@android:drawable/divider_horizontal_dark"

         android:layout_width="fill_parent"