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 to use std::shared_ptr internally
- Switch ACameraMetadata to use std::shared_ptr internally
- Always copy data in the ACameraMetadata copy constructor

Test: New CTS tests pass, fail without this CL
Bug: 148972471
Change-Id: I40a0ccb8b40c7a89ee7d3a6f7bac7c9c88d709f1

camera/ndk/NdkCameraMetadata.cpp

@@ -81,15 +81,16 @@ bool InitJni(JNIEnv* env) {
 }
 
 // Given cameraMetadata, an instance of android.hardware.camera2.CameraMetadata, invokes
-// cameraMetadata.getNativeMetadataPtr() and returns it as a CameraMetadata*.
-CameraMetadata* CameraMetadata_getNativeMetadataPtr(JNIEnv* env, jobject cameraMetadata) {
+// cameraMetadata.getNativeMetadataPtr() and returns it as a std::shared_ptr<CameraMetadata>*.
+std::shared_ptr<CameraMetadata>* CameraMetadata_getNativeMetadataPtr(JNIEnv* env,
+        jobject cameraMetadata) {
     if (cameraMetadata == nullptr) {
         ALOGE("%s: Invalid Java CameraMetadata object.", __FUNCTION__);
         return nullptr;
     }
     jlong ret = env->CallLongMethod(cameraMetadata,
                                     android_hardware_camera2_CameraMetadata_getNativeMetadataPtr);
-    return reinterpret_cast<CameraMetadata *>(ret);
+    return reinterpret_cast<std::shared_ptr<CameraMetadata>* >(ret);
 }
 
 }  // namespace
@@ -179,10 +180,9 @@ ACameraMetadata* ACameraMetadata_fromCameraMetadata(JNIEnv* env, jobject cameraM
         return nullptr;
     }
 
-    CameraMetadata* src = CameraMetadata_getNativeMetadataPtr(env,
-                                                              cameraMetadata);
-    ACameraMetadata* output = new ACameraMetadata(src, type);
+    auto sharedData = CameraMetadata_getNativeMetadataPtr(env, cameraMetadata);
+    ACameraMetadata* output = new ACameraMetadata(*sharedData, type);
     output->incStrong(/*id=*/(void*) ACameraMetadata_fromCameraMetadata);
     return output;
 }
-#endif  /* __ANDROID_VNDK__ */
+#endif  /* __ANDROID_VNDK__ */

camera/ndk/impl/ACameraMetadata.cpp

@@ -28,33 +28,24 @@ using namespace android;
  * ACameraMetadata Implementation
  */
 ACameraMetadata::ACameraMetadata(camera_metadata_t* buffer, ACAMERA_METADATA_TYPE type) :
-        mData(new CameraMetadata(buffer)),
-        mOwnsData(true),
+        mData(std::make_shared<CameraMetadata>(buffer)),
         mType(type) {
     init();
 }
 
-ACameraMetadata::ACameraMetadata(CameraMetadata* cameraMetadata, ACAMERA_METADATA_TYPE type) :
+ACameraMetadata::ACameraMetadata(const std::shared_ptr<CameraMetadata>& cameraMetadata,
+        ACAMERA_METADATA_TYPE type) :
         mData(cameraMetadata),
-        mOwnsData(false),
         mType(type) {
     init();
 }
 
 ACameraMetadata::ACameraMetadata(const ACameraMetadata& other) :
-        mOwnsData(other.mOwnsData),
+        mData(std::make_shared<CameraMetadata>(*(other.mData))),
         mType(other.mType) {
-    if (other.mOwnsData) {
-        mData = new CameraMetadata(*(other.mData));
-    } else {
-        mData = other.mData;
-    }
 }
 
 ACameraMetadata::~ACameraMetadata() {
-    if (mOwnsData) {
-        delete mData;
-    }
 }
 
 void
@@ -373,7 +364,7 @@ ACameraMetadata::getConstEntry(uint32_t tag, ACameraMetadata_const_entry* entry)
 
     Mutex::Autolock _l(mLock);
 
-    camera_metadata_ro_entry rawEntry = static_cast<const CameraMetadata*>(mData)->find(tag);
+    camera_metadata_ro_entry rawEntry = static_cast<const CameraMetadata*>(mData.get())->find(tag);
     if (rawEntry.count == 0) {
         ALOGE("%s: cannot find metadata tag %d", __FUNCTION__, tag);
         return ACAMERA_ERROR_METADATA_NOT_FOUND;

camera/ndk/impl/ACameraMetadata.h

@@ -18,6 +18,7 @@
 
 #include <unordered_set>
 #include <vector>
+#include <memory>
 
 #include <sys/types.h>
 #include <utils/Mutex.h>
@@ -50,14 +51,13 @@ struct ACameraMetadata : public RefBase {
     // Constructs a ACameraMetadata that takes ownership of `buffer`.
     ACameraMetadata(camera_metadata_t* buffer, ACAMERA_METADATA_TYPE type);
 
-    // Constructs a ACameraMetadata that is a view of `cameraMetadata`.
-    // `cameraMetadata` will not be deleted by ~ACameraMetadata().
-    ACameraMetadata(CameraMetadata* cameraMetadata, ACAMERA_METADATA_TYPE type);
+    // Constructs a ACameraMetadata that shares its data with something else, like a Java object
+    ACameraMetadata(const std::shared_ptr<CameraMetadata>& cameraMetadata,
+            ACAMERA_METADATA_TYPE type);
 
     // Copy constructor.
     //
-    // If `other` owns its CameraMetadata, then makes a deep copy.
-    // Otherwise, the new instance is also a view of the same data.
+    // Always makes a deep copy.
     ACameraMetadata(const ACameraMetadata& other);
 
     ~ACameraMetadata();
@@ -125,9 +125,7 @@ struct ACameraMetadata : public RefBase {
     // Guard access of public APIs: get/update/getTags.
     mutable Mutex mLock;
 
-    CameraMetadata* mData;
-    // If true, has ownership of mData. Otherwise, mData is a view of an external instance.
-    bool mOwnsData;
+    std::shared_ptr<CameraMetadata> mData;
 
     mutable Vector<uint32_t> mTags; // Updated by `getTags()`, cleared by `update()`.
     const ACAMERA_METADATA_TYPE mType;

camera/ndk/include/camera/NdkCameraMetadata.h

@@ -274,10 +274,9 @@ bool ACameraMetadata_isLogicalMultiCamera(const ACameraMetadata* staticMetadata,
  * <p>The returned ACameraMetadata must be freed by the application by {@link ACameraMetadata_free}
  * after application is done using it.</p>
  *
- * <p>This function does not affect the lifetime of {@link cameraMetadata}. Attempting to use the
- * returned ACameraMetadata object after {@link cameraMetadata} has been garbage collected is
- * unsafe. To manage the lifetime beyond the current JNI function call, use
- * {@code env->NewGlobalRef()} and {@code env->DeleteGlobalRef()}.
+ * <p>The ACameraMetadata maintains a reference count to the underlying data, so
+ * it can be used independently of the Java object, and it remains valid even if
+ * the Java metadata is garbage collected.
  *
  * @param env the JNI environment.
  * @param cameraMetadata the source {@link android.hardware.camera2.CameraMetadata} from which the
@@ -297,4 +296,4 @@ __END_DECLS
 
 #endif /* _NDK_CAMERA_METADATA_H */
 
-/** @} */
+/** @} */