From 8d5f3e31c914e29129f50fe9830d71adf52ab5cf Mon Sep 17 00:00:00 2001 From: Andreas Huber Date: Mon, 12 Aug 2013 09:19:45 -0700 Subject: [PATCH] Manage jclass objects (and most jobjects) in jni code using ScopedLocalRef for safer refcounting. Previously jclass objects were not DeleteLocalRef'ed at all, leading us to exhaust the local ref pool quickly in certain circumstances. This change also makes sure we properly serialize int64_t entries when converting from AMessage to HashMap and boosts thread priority for java-instantiated MediaCodecs slightly from NORMAL to FOREGROUND. Change-Id: I4ebdd8a5ca6b3442698c9f86fcc31af8c199aaf5 --- media/jni/android_media_MediaCodec.cpp | 80 ++++++++++++--------- media/jni/android_media_Utils.cpp | 97 +++++++++++++++----------- 2 files changed, 101 insertions(+), 76 deletions(-) diff --git a/media/jni/android_media_MediaCodec.cpp b/media/jni/android_media_MediaCodec.cpp index 8689e192890e2..ae1db8776b9f2 100644 --- a/media/jni/android_media_MediaCodec.cpp +++ b/media/jni/android_media_MediaCodec.cpp @@ -38,6 +38,8 @@ #include #include +#include + #include namespace android { @@ -86,7 +88,7 @@ JMediaCodec::JMediaCodec( mLooper->start( false, // runOnCallingThread false, // canCallJava - PRIORITY_DEFAULT); + PRIORITY_FOREGROUND); if (nameIsType) { mCodec = MediaCodec::CreateByType(mLooper, name, encoder); @@ -186,9 +188,10 @@ status_t JMediaCodec::dequeueOutputBuffer( return err; } - jclass clazz = env->FindClass("android/media/MediaCodec$BufferInfo"); + ScopedLocalRef clazz( + env, env->FindClass("android/media/MediaCodec$BufferInfo")); - jmethodID method = env->GetMethodID(clazz, "set", "(IIJI)V"); + jmethodID method = env->GetMethodID(clazz.get(), "set", "(IIJI)V"); env->CallVoidMethod(bufferInfo, method, offset, size, timeUs, flags); return OK; @@ -227,29 +230,33 @@ status_t JMediaCodec::getBuffers( return err; } - jclass byteBufferClass = env->FindClass("java/nio/ByteBuffer"); - CHECK(byteBufferClass != NULL); + ScopedLocalRef byteBufferClass( + env, env->FindClass("java/nio/ByteBuffer")); + + CHECK(byteBufferClass.get() != NULL); jmethodID orderID = env->GetMethodID( - byteBufferClass, + byteBufferClass.get(), "order", "(Ljava/nio/ByteOrder;)Ljava/nio/ByteBuffer;"); CHECK(orderID != NULL); - jclass byteOrderClass = env->FindClass("java/nio/ByteOrder"); - CHECK(byteOrderClass != NULL); + ScopedLocalRef byteOrderClass( + env, env->FindClass("java/nio/ByteOrder")); + + CHECK(byteOrderClass.get() != NULL); jmethodID nativeOrderID = env->GetStaticMethodID( - byteOrderClass, "nativeOrder", "()Ljava/nio/ByteOrder;"); + byteOrderClass.get(), "nativeOrder", "()Ljava/nio/ByteOrder;"); CHECK(nativeOrderID != NULL); jobject nativeByteOrderObj = - env->CallStaticObjectMethod(byteOrderClass, nativeOrderID); + env->CallStaticObjectMethod(byteOrderClass.get(), nativeOrderID); CHECK(nativeByteOrderObj != NULL); *bufArray = (jobjectArray)env->NewObjectArray( - buffers.size(), byteBufferClass, NULL); + buffers.size(), byteBufferClass.get(), NULL); if (*bufArray == NULL) { env->DeleteLocalRef(nativeByteOrderObj); return NO_MEMORY; @@ -338,11 +345,12 @@ static void android_media_MediaCodec_release(JNIEnv *env, jobject thiz) { } static void throwCryptoException(JNIEnv *env, status_t err, const char *msg) { - jclass clazz = env->FindClass("android/media/MediaCodec$CryptoException"); - CHECK(clazz != NULL); + ScopedLocalRef clazz( + env, env->FindClass("android/media/MediaCodec$CryptoException")); + CHECK(clazz.get() != NULL); jmethodID constructID = - env->GetMethodID(clazz, "", "(ILjava/lang/String;)V"); + env->GetMethodID(clazz.get(), "", "(ILjava/lang/String;)V"); CHECK(constructID != NULL); jstring msgObj = env->NewStringUTF(msg != NULL ? msg : "Unknown Error"); @@ -363,7 +371,7 @@ static void throwCryptoException(JNIEnv *env, status_t err, const char *msg) { } jthrowable exception = - (jthrowable)env->NewObject(clazz, constructID, err, msgObj); + (jthrowable)env->NewObject(clazz.get(), constructID, err, msgObj); env->Throw(exception); } @@ -848,51 +856,55 @@ static void android_media_MediaCodec_setVideoScalingMode( } static void android_media_MediaCodec_native_init(JNIEnv *env) { - jclass clazz = env->FindClass("android/media/MediaCodec"); - CHECK(clazz != NULL); + ScopedLocalRef clazz( + env, env->FindClass("android/media/MediaCodec")); + CHECK(clazz.get() != NULL); - gFields.context = env->GetFieldID(clazz, "mNativeContext", "I"); + gFields.context = env->GetFieldID(clazz.get(), "mNativeContext", "I"); CHECK(gFields.context != NULL); - clazz = env->FindClass("android/media/MediaCodec$CryptoInfo"); - CHECK(clazz != NULL); + clazz.reset(env->FindClass("android/media/MediaCodec$CryptoInfo")); + CHECK(clazz.get() != NULL); gFields.cryptoInfoNumSubSamplesID = - env->GetFieldID(clazz, "numSubSamples", "I"); + env->GetFieldID(clazz.get(), "numSubSamples", "I"); CHECK(gFields.cryptoInfoNumSubSamplesID != NULL); gFields.cryptoInfoNumBytesOfClearDataID = - env->GetFieldID(clazz, "numBytesOfClearData", "[I"); + env->GetFieldID(clazz.get(), "numBytesOfClearData", "[I"); CHECK(gFields.cryptoInfoNumBytesOfClearDataID != NULL); gFields.cryptoInfoNumBytesOfEncryptedDataID = - env->GetFieldID(clazz, "numBytesOfEncryptedData", "[I"); + env->GetFieldID(clazz.get(), "numBytesOfEncryptedData", "[I"); CHECK(gFields.cryptoInfoNumBytesOfEncryptedDataID != NULL); - gFields.cryptoInfoKeyID = env->GetFieldID(clazz, "key", "[B"); + gFields.cryptoInfoKeyID = env->GetFieldID(clazz.get(), "key", "[B"); CHECK(gFields.cryptoInfoKeyID != NULL); - gFields.cryptoInfoIVID = env->GetFieldID(clazz, "iv", "[B"); + gFields.cryptoInfoIVID = env->GetFieldID(clazz.get(), "iv", "[B"); CHECK(gFields.cryptoInfoIVID != NULL); - gFields.cryptoInfoModeID = env->GetFieldID(clazz, "mode", "I"); + gFields.cryptoInfoModeID = env->GetFieldID(clazz.get(), "mode", "I"); CHECK(gFields.cryptoInfoModeID != NULL); - clazz = env->FindClass("android/media/MediaCodec$CryptoException"); - CHECK(clazz != NULL); + clazz.reset(env->FindClass("android/media/MediaCodec$CryptoException")); + CHECK(clazz.get() != NULL); jfieldID field; - field = env->GetStaticFieldID(clazz, "ERROR_NO_KEY", "I"); + field = env->GetStaticFieldID(clazz.get(), "ERROR_NO_KEY", "I"); CHECK(field != NULL); - gCryptoErrorCodes.cryptoErrorNoKey = env->GetStaticIntField(clazz, field); + gCryptoErrorCodes.cryptoErrorNoKey = + env->GetStaticIntField(clazz.get(), field); - field = env->GetStaticFieldID(clazz, "ERROR_KEY_EXPIRED", "I"); + field = env->GetStaticFieldID(clazz.get(), "ERROR_KEY_EXPIRED", "I"); CHECK(field != NULL); - gCryptoErrorCodes.cryptoErrorKeyExpired = env->GetStaticIntField(clazz, field); + gCryptoErrorCodes.cryptoErrorKeyExpired = + env->GetStaticIntField(clazz.get(), field); - field = env->GetStaticFieldID(clazz, "ERROR_RESOURCE_BUSY", "I"); + field = env->GetStaticFieldID(clazz.get(), "ERROR_RESOURCE_BUSY", "I"); CHECK(field != NULL); - gCryptoErrorCodes.cryptoErrorResourceBusy = env->GetStaticIntField(clazz, field); + gCryptoErrorCodes.cryptoErrorResourceBusy = + env->GetStaticIntField(clazz.get(), field); } static void android_media_MediaCodec_native_setup( diff --git a/media/jni/android_media_Utils.cpp b/media/jni/android_media_Utils.cpp index e35ace38829eb..54c5e9bdd267b 100644 --- a/media/jni/android_media_Utils.cpp +++ b/media/jni/android_media_Utils.cpp @@ -24,6 +24,8 @@ #include #include +#include + namespace android { bool ConvertKeyValueArraysToKeyedVector( @@ -76,33 +78,35 @@ bool ConvertKeyValueArraysToKeyedVector( } static jobject makeIntegerObject(JNIEnv *env, int32_t value) { - jclass clazz = env->FindClass("java/lang/Integer"); - CHECK(clazz != NULL); + ScopedLocalRef clazz(env, env->FindClass("java/lang/Integer")); + CHECK(clazz.get() != NULL); - jmethodID integerConstructID = env->GetMethodID(clazz, "", "(I)V"); + jmethodID integerConstructID = + env->GetMethodID(clazz.get(), "", "(I)V"); CHECK(integerConstructID != NULL); - return env->NewObject(clazz, integerConstructID, value); + return env->NewObject(clazz.get(), integerConstructID, value); } static jobject makeLongObject(JNIEnv *env, int64_t value) { - jclass clazz = env->FindClass("java/lang/Long"); - CHECK(clazz != NULL); + ScopedLocalRef clazz(env, env->FindClass("java/lang/Long")); + CHECK(clazz.get() != NULL); - jmethodID longConstructID = env->GetMethodID(clazz, "", "(J)V"); + jmethodID longConstructID = env->GetMethodID(clazz.get(), "", "(J)V"); CHECK(longConstructID != NULL); - return env->NewObject(clazz, longConstructID, value); + return env->NewObject(clazz.get(), longConstructID, value); } static jobject makeFloatObject(JNIEnv *env, float value) { - jclass clazz = env->FindClass("java/lang/Float"); - CHECK(clazz != NULL); + ScopedLocalRef clazz(env, env->FindClass("java/lang/Float")); + CHECK(clazz.get() != NULL); - jmethodID floatConstructID = env->GetMethodID(clazz, "", "(F)V"); + jmethodID floatConstructID = + env->GetMethodID(clazz.get(), "", "(F)V"); CHECK(floatConstructID != NULL); - return env->NewObject(clazz, floatConstructID, value); + return env->NewObject(clazz.get(), floatConstructID, value); } static jobject makeByteBufferObject( @@ -110,15 +114,16 @@ static jobject makeByteBufferObject( jbyteArray byteArrayObj = env->NewByteArray(size); env->SetByteArrayRegion(byteArrayObj, 0, size, (const jbyte *)data); - jclass clazz = env->FindClass("java/nio/ByteBuffer"); - CHECK(clazz != NULL); + ScopedLocalRef clazz(env, env->FindClass("java/nio/ByteBuffer")); + CHECK(clazz.get() != NULL); jmethodID byteBufWrapID = - env->GetStaticMethodID(clazz, "wrap", "([B)Ljava/nio/ByteBuffer;"); + env->GetStaticMethodID( + clazz.get(), "wrap", "([B)Ljava/nio/ByteBuffer;"); CHECK(byteBufWrapID != NULL); jobject byteBufObj = env->CallStaticObjectMethod( - clazz, byteBufWrapID, byteArrayObj); + clazz.get(), byteBufWrapID, byteArrayObj); env->DeleteLocalRef(byteArrayObj); byteArrayObj = NULL; @@ -140,14 +145,15 @@ static void SetMapInt32( status_t ConvertMessageToMap( JNIEnv *env, const sp &msg, jobject *map) { - jclass hashMapClazz = env->FindClass("java/util/HashMap"); + ScopedLocalRef hashMapClazz( + env, env->FindClass("java/util/HashMap")); - if (hashMapClazz == NULL) { + if (hashMapClazz.get() == NULL) { return -EINVAL; } jmethodID hashMapConstructID = - env->GetMethodID(hashMapClazz, "", "()V"); + env->GetMethodID(hashMapClazz.get(), "", "()V"); if (hashMapConstructID == NULL) { return -EINVAL; @@ -155,7 +161,7 @@ status_t ConvertMessageToMap( jmethodID hashMapPutID = env->GetMethodID( - hashMapClazz, + hashMapClazz.get(), "put", "(Ljava/lang/Object;Ljava/lang/Object;)Ljava/lang/Object;"); @@ -163,7 +169,7 @@ status_t ConvertMessageToMap( return -EINVAL; } - jobject hashMap = env->NewObject(hashMapClazz, hashMapConstructID); + jobject hashMap = env->NewObject(hashMapClazz.get(), hashMapConstructID); for (size_t i = 0; i < msg->countEntries(); ++i) { AMessage::Type valueType; @@ -276,17 +282,16 @@ status_t ConvertMessageToMap( status_t ConvertKeyValueArraysToMessage( JNIEnv *env, jobjectArray keys, jobjectArray values, sp *out) { - jclass stringClass = env->FindClass("java/lang/String"); - CHECK(stringClass != NULL); - - jclass integerClass = env->FindClass("java/lang/Integer"); - CHECK(integerClass != NULL); - - jclass floatClass = env->FindClass("java/lang/Float"); - CHECK(floatClass != NULL); - - jclass byteBufClass = env->FindClass("java/nio/ByteBuffer"); - CHECK(byteBufClass != NULL); + ScopedLocalRef stringClass(env, env->FindClass("java/lang/String")); + CHECK(stringClass.get() != NULL); + ScopedLocalRef integerClass(env, env->FindClass("java/lang/Integer")); + CHECK(integerClass.get() != NULL); + ScopedLocalRef longClass(env, env->FindClass("java/lang/Long")); + CHECK(longClass.get() != NULL); + ScopedLocalRef floatClass(env, env->FindClass("java/lang/Float")); + CHECK(floatClass.get() != NULL); + ScopedLocalRef byteBufClass(env, env->FindClass("java/nio/ByteBuffer")); + CHECK(byteBufClass.get() != NULL); sp msg = new AMessage; @@ -309,7 +314,7 @@ status_t ConvertKeyValueArraysToMessage( for (jsize i = 0; i < numEntries; ++i) { jobject keyObj = env->GetObjectArrayElement(keys, i); - if (!env->IsInstanceOf(keyObj, stringClass)) { + if (!env->IsInstanceOf(keyObj, stringClass.get())) { return -EINVAL; } @@ -326,7 +331,7 @@ status_t ConvertKeyValueArraysToMessage( jobject valueObj = env->GetObjectArrayElement(values, i); - if (env->IsInstanceOf(valueObj, stringClass)) { + if (env->IsInstanceOf(valueObj, stringClass.get())) { const char *value = env->GetStringUTFChars((jstring)valueObj, NULL); if (value == NULL) { @@ -337,29 +342,37 @@ status_t ConvertKeyValueArraysToMessage( env->ReleaseStringUTFChars((jstring)valueObj, value); value = NULL; - } else if (env->IsInstanceOf(valueObj, integerClass)) { + } else if (env->IsInstanceOf(valueObj, integerClass.get())) { jmethodID intValueID = - env->GetMethodID(integerClass, "intValue", "()I"); + env->GetMethodID(integerClass.get(), "intValue", "()I"); CHECK(intValueID != NULL); jint value = env->CallIntMethod(valueObj, intValueID); msg->setInt32(key.c_str(), value); - } else if (env->IsInstanceOf(valueObj, floatClass)) { + } else if (env->IsInstanceOf(valueObj, longClass.get())) { + jmethodID longValueID = + env->GetMethodID(longClass.get(), "longValue", "()J"); + CHECK(longValueID != NULL); + + jlong value = env->CallLongMethod(valueObj, longValueID); + + msg->setInt64(key.c_str(), value); + } else if (env->IsInstanceOf(valueObj, floatClass.get())) { jmethodID floatValueID = - env->GetMethodID(floatClass, "floatValue", "()F"); + env->GetMethodID(floatClass.get(), "floatValue", "()F"); CHECK(floatValueID != NULL); jfloat value = env->CallFloatMethod(valueObj, floatValueID); msg->setFloat(key.c_str(), value); - } else if (env->IsInstanceOf(valueObj, byteBufClass)) { + } else if (env->IsInstanceOf(valueObj, byteBufClass.get())) { jmethodID positionID = - env->GetMethodID(byteBufClass, "position", "()I"); + env->GetMethodID(byteBufClass.get(), "position", "()I"); CHECK(positionID != NULL); jmethodID limitID = - env->GetMethodID(byteBufClass, "limit", "()I"); + env->GetMethodID(byteBufClass.get(), "limit", "()I"); CHECK(limitID != NULL); jint position = env->CallIntMethod(valueObj, positionID); @@ -375,7 +388,7 @@ status_t ConvertKeyValueArraysToMessage( buffer->size()); } else { jmethodID arrayID = - env->GetMethodID(byteBufClass, "array", "()[B"); + env->GetMethodID(byteBufClass.get(), "array", "()[B"); CHECK(arrayID != NULL); jbyteArray byteArray =