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
diff --git a/camera/ndk/NdkCameraMetadata.cpp b/camera/ndk/NdkCameraMetadata.cpp
index 1fec3e1..7d3a53e 100644
--- a/camera/ndk/NdkCameraMetadata.cpp
+++ b/camera/ndk/NdkCameraMetadata.cpp
@@ -81,15 +81,16 @@
}
// 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 @@
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__ */
\ No newline at end of file
+#endif /* __ANDROID_VNDK__ */
diff --git a/camera/ndk/impl/ACameraMetadata.cpp b/camera/ndk/impl/ACameraMetadata.cpp
index 18c5d3c..1bbb9c2 100644
--- a/camera/ndk/impl/ACameraMetadata.cpp
+++ b/camera/ndk/impl/ACameraMetadata.cpp
@@ -28,33 +28,24 @@
* 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 @@
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;
diff --git a/camera/ndk/impl/ACameraMetadata.h b/camera/ndk/impl/ACameraMetadata.h
index a57b2ff..084a60b 100644
--- a/camera/ndk/impl/ACameraMetadata.h
+++ b/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 @@
// 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 @@
// 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;
diff --git a/camera/ndk/include/camera/NdkCameraMetadata.h b/camera/ndk/include/camera/NdkCameraMetadata.h
index ee75610..072bb02 100644
--- a/camera/ndk/include/camera/NdkCameraMetadata.h
+++ b/camera/ndk/include/camera/NdkCameraMetadata.h
@@ -274,10 +274,9 @@
* <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 @@
#endif /* _NDK_CAMERA_METADATA_H */
-/** @} */
\ No newline at end of file
+/** @} */