From fdfec366921b4d3433f3e581c59bcbb6683eb13f Mon Sep 17 00:00:00 2001 From: Eino-Ville Talvala Date: Mon, 10 Feb 2020 13:52:14 -0800 Subject: [PATCH] Camera NDK: New lifecycle for ACameraMetadata_fromCameraMetadata Instead of requiring the user to call NewGlobalRef/DeleteGlobalRef for keeping the java object alive when creating an NDK view into it, reference count the real native data instead, so that there's no need to keep track of the Java object lifecycle. - Switch CameraMetadataNative use std::shared_ptr Test: New CTS tests pass, fail without this CL Bug: 148972471 Change-Id: I9a00fc1daa1beac35a15484960048facb90499a6 --- .../hardware/camera2/CameraMetadata.java | 2 +- .../camera2/impl/CameraMetadataNative.java | 2 +- ...ndroid_hardware_camera2_CameraMetadata.cpp | 41 +++++++++++-------- 3 files changed, 25 insertions(+), 20 deletions(-) diff --git a/core/java/android/hardware/camera2/CameraMetadata.java b/core/java/android/hardware/camera2/CameraMetadata.java index 9bef2e18db08a..8f3cb93ae13fc 100644 --- a/core/java/android/hardware/camera2/CameraMetadata.java +++ b/core/java/android/hardware/camera2/CameraMetadata.java @@ -95,7 +95,7 @@ public abstract class CameraMetadata { } /** - * Retrieves the native CameraMetadata* as a Java long. + * Retrieves the native std::shared_ptr* as a Java long. * Returns 0 if mNativeInstance is null. * * @hide diff --git a/core/java/android/hardware/camera2/impl/CameraMetadataNative.java b/core/java/android/hardware/camera2/impl/CameraMetadataNative.java index aefe66fe5dec7..df77f52bb0345 100644 --- a/core/java/android/hardware/camera2/impl/CameraMetadataNative.java +++ b/core/java/android/hardware/camera2/impl/CameraMetadataNative.java @@ -1680,7 +1680,7 @@ public class CameraMetadataNative implements Parcelable { } @UnsupportedAppUsage - private long mMetadataPtr; // native CameraMetadata* + private long mMetadataPtr; // native std::shared_ptr* private native long nativeAllocate(); private native long nativeAllocateCopy(CameraMetadataNative other) diff --git a/core/jni/android_hardware_camera2_CameraMetadata.cpp b/core/jni/android_hardware_camera2_CameraMetadata.cpp index fad5b0e9ca0fc..c523e2dc67376 100644 --- a/core/jni/android_hardware_camera2_CameraMetadata.cpp +++ b/core/jni/android_hardware_camera2_CameraMetadata.cpp @@ -89,13 +89,13 @@ status_t CameraMetadata_getNativeMetadata(JNIEnv* env, jobject thiz, ALOGE("%s: Invalid output metadata object.", __FUNCTION__); return BAD_VALUE; } - CameraMetadata* nativePtr = reinterpret_cast(env->GetLongField(thiz, - fields.metadata_ptr)); + auto nativePtr = reinterpret_cast *>( + env->GetLongField(thiz, fields.metadata_ptr)); if (nativePtr == NULL) { ALOGE("%s: Invalid native pointer in java metadata object.", __FUNCTION__); return BAD_VALUE; } - *metadata = *nativePtr; + *metadata = *(nativePtr->get()); return OK; } @@ -171,12 +171,15 @@ static jint CameraMetadata_setupGlobalVendorTagDescriptor(JNIEnv *env, jobject t // Less safe access to native pointer. Does NOT throw any Java exceptions if NULL. static CameraMetadata* CameraMetadata_getPointerNoThrow(JNIEnv *env, jobject thiz) { - - if (thiz == NULL) { - return NULL; + if (thiz == nullptr) { + return nullptr; } - - return reinterpret_cast(env->GetLongField(thiz, fields.metadata_ptr)); + auto metadata = reinterpret_cast *>( + env->GetLongField(thiz, fields.metadata_ptr)); + if (metadata == nullptr) { + return nullptr; + } + return metadata->get(); } // Safe access to native pointer from object. Throws if not possible to access. @@ -205,7 +208,7 @@ static CameraMetadata* CameraMetadata_getPointerThrow(JNIEnv *env, jobject thiz, static jlong CameraMetadata_allocate(JNIEnv *env, jobject thiz) { ALOGV("%s", __FUNCTION__); - return reinterpret_cast(new CameraMetadata()); + return reinterpret_cast(new std::shared_ptr(new CameraMetadata())); } static jlong CameraMetadata_allocateCopy(JNIEnv *env, jobject thiz, @@ -214,12 +217,12 @@ static jlong CameraMetadata_allocateCopy(JNIEnv *env, jobject thiz, CameraMetadata* otherMetadata = CameraMetadata_getPointerThrow(env, other, "other"); - // In case of exception, return if (otherMetadata == NULL) return NULL; - // Clone native metadata and return new pointer - return reinterpret_cast(new CameraMetadata(*otherMetadata)); + // Clone native metadata and return new pointer. + auto clonedMetadata = new CameraMetadata(*otherMetadata); + return reinterpret_cast(new std::shared_ptr(clonedMetadata)); } @@ -256,14 +259,16 @@ static jint CameraMetadata_getEntryCount(JNIEnv *env, jobject thiz) { static void CameraMetadata_close(JNIEnv *env, jobject thiz) { ALOGV("%s", __FUNCTION__); - CameraMetadata* metadata = CameraMetadata_getPointerNoThrow(env, thiz); - - if (metadata != NULL) { - delete metadata; - env->SetLongField(thiz, fields.metadata_ptr, 0); + if (thiz != nullptr) { + auto metadata = reinterpret_cast *>( + env->GetLongField(thiz, fields.metadata_ptr)); + if (metadata != nullptr) { + delete metadata; + env->SetLongField(thiz, fields.metadata_ptr, 0); + } } - LOG_ALWAYS_FATAL_IF(CameraMetadata_getPointerNoThrow(env, thiz) != NULL, + LOG_ALWAYS_FATAL_IF(CameraMetadata_getPointerNoThrow(env, thiz) != nullptr, "Expected the native ptr to be 0 after #close"); }