From 8a0be29c19ba7c1de4a0983b8dbcafb730b17fc0 Mon Sep 17 00:00:00 2001 From: Jayant Chowdhary Date: Wed, 8 Jan 2020 13:10:38 -0800 Subject: [PATCH] Add some move constructors and assignment operators to CameraMetadata. This avoids unnecessary copying of camera metadata which can get expensive in cases of large camera metadata blobs. Bug: 71727540 Test: GCA (sanity) Test: Add CallStack::logStack() in CameraMetadata's move asignment operator -> see that it gets called for every insertResultLocked. Change-Id: I6c75c7ce5267126916c865b028e5f7c7f50b763b Signed-off-by: Jayant Chowdhary --- camera/CameraMetadata.cpp | 9 +++++++++ camera/CaptureResult.cpp | 6 ++++++ camera/include/camera/CameraMetadata.h | 10 ++++++++++ camera/include/camera/CaptureResult.h | 2 ++ .../camera/libcameraservice/device3/Camera3Device.h | 2 +- .../libcameraservice/device3/Camera3OfflineSession.h | 2 +- .../libcameraservice/device3/Camera3OutputUtils.cpp | 4 ++-- .../libcameraservice/device3/Camera3OutputUtils.h | 2 +- 8 files changed, 32 insertions(+), 5 deletions(-) diff --git a/camera/CameraMetadata.cpp b/camera/CameraMetadata.cpp index 15c295e9e6..4745ee8ab9 100644 --- a/camera/CameraMetadata.cpp +++ b/camera/CameraMetadata.cpp @@ -50,6 +50,15 @@ CameraMetadata::CameraMetadata(const CameraMetadata &other) : mBuffer = clone_camera_metadata(other.mBuffer); } +CameraMetadata::CameraMetadata(CameraMetadata &&other) :mBuffer(NULL), mLocked(false) { + acquire(other); +} + +CameraMetadata &CameraMetadata::operator=(CameraMetadata &&other) { + acquire(other); + return *this; +} + CameraMetadata::CameraMetadata(camera_metadata_t *buffer) : mBuffer(NULL), mLocked(false) { acquire(buffer); diff --git a/camera/CaptureResult.cpp b/camera/CaptureResult.cpp index 1d8e8c42d0..9cbfdb064e 100644 --- a/camera/CaptureResult.cpp +++ b/camera/CaptureResult.cpp @@ -117,6 +117,12 @@ CaptureResult::CaptureResult() : mMetadata(), mResultExtras() { } +CaptureResult::CaptureResult(CaptureResult &&otherResult) { + mMetadata = std::move(otherResult.mMetadata); + mResultExtras = otherResult.mResultExtras; + mPhysicalMetadatas = std::move(otherResult.mPhysicalMetadatas); +} + CaptureResult::CaptureResult(const CaptureResult &otherResult) { mResultExtras = otherResult.mResultExtras; mMetadata = otherResult.mMetadata; diff --git a/camera/include/camera/CameraMetadata.h b/camera/include/camera/CameraMetadata.h index 844bb80302..9d1b5c7c04 100644 --- a/camera/include/camera/CameraMetadata.h +++ b/camera/include/camera/CameraMetadata.h @@ -40,6 +40,11 @@ class CameraMetadata: public Parcelable { * dataCapacity extra storage */ CameraMetadata(size_t entryCapacity, size_t dataCapacity = 10); + /** + * Move constructor, acquires other's metadata buffer + */ + CameraMetadata(CameraMetadata &&other); + ~CameraMetadata(); /** Takes ownership of passed-in buffer */ @@ -53,6 +58,11 @@ class CameraMetadata: public Parcelable { CameraMetadata &operator=(const CameraMetadata &other); CameraMetadata &operator=(const camera_metadata_t *buffer); + /** + * Move assignment operator, acquires other's metadata buffer + */ + CameraMetadata &operator=(CameraMetadata &&other); + /** * Get reference to the underlying metadata buffer. Ownership remains with * the CameraMetadata object, but non-const CameraMetadata methods will not diff --git a/camera/include/camera/CaptureResult.h b/camera/include/camera/CaptureResult.h index ef830b50b7..dc3d2823ec 100644 --- a/camera/include/camera/CaptureResult.h +++ b/camera/include/camera/CaptureResult.h @@ -135,6 +135,8 @@ struct CaptureResult : public virtual LightRefBase { CaptureResult(const CaptureResult& otherResult); + CaptureResult(CaptureResult &&captureResult); + status_t readFromParcel(android::Parcel* parcel); status_t writeToParcel(android::Parcel* parcel) const; }; diff --git a/services/camera/libcameraservice/device3/Camera3Device.h b/services/camera/libcameraservice/device3/Camera3Device.h index e13e45fbe2..50944d2820 100644 --- a/services/camera/libcameraservice/device3/Camera3Device.h +++ b/services/camera/libcameraservice/device3/Camera3Device.h @@ -1102,7 +1102,7 @@ class Camera3Device : uint32_t mNextReprocessShutterFrameNumber; // the minimal frame number of the next ZSL still capture shutter uint32_t mNextZslStillShutterFrameNumber; - List mResultQueue; + std::list mResultQueue; std::condition_variable mResultSignal; wp mListener; diff --git a/services/camera/libcameraservice/device3/Camera3OfflineSession.h b/services/camera/libcameraservice/device3/Camera3OfflineSession.h index 27043d2fad..208f70de73 100644 --- a/services/camera/libcameraservice/device3/Camera3OfflineSession.h +++ b/services/camera/libcameraservice/device3/Camera3OfflineSession.h @@ -212,7 +212,7 @@ class Camera3OfflineSession : const uint32_t mNumPartialResults; std::mutex mOutputLock; - List mResultQueue; + std::list mResultQueue; std::condition_variable mResultSignal; // the minimal frame number of the next non-reprocess result uint32_t mNextResultFrameNumber; diff --git a/services/camera/libcameraservice/device3/Camera3OutputUtils.cpp b/services/camera/libcameraservice/device3/Camera3OutputUtils.cpp index 39b7db4296..238356e89d 100644 --- a/services/camera/libcameraservice/device3/Camera3OutputUtils.cpp +++ b/services/camera/libcameraservice/device3/Camera3OutputUtils.cpp @@ -153,9 +153,9 @@ void insertResultLocked(CaptureOutputStates& states, CaptureResult *result, uint } // Valid result, insert into queue - List::iterator queuedResult = + std::list::iterator queuedResult = states.resultQueue.insert(states.resultQueue.end(), CaptureResult(*result)); - ALOGVV("%s: result requestId = %" PRId32 ", frameNumber = %" PRId64 + ALOGV("%s: result requestId = %" PRId32 ", frameNumber = %" PRId64 ", burstId = %" PRId32, __FUNCTION__, queuedResult->mResultExtras.requestId, queuedResult->mResultExtras.frameNumber, diff --git a/services/camera/libcameraservice/device3/Camera3OutputUtils.h b/services/camera/libcameraservice/device3/Camera3OutputUtils.h index 300df5b444..fbb47f8b8c 100644 --- a/services/camera/libcameraservice/device3/Camera3OutputUtils.h +++ b/services/camera/libcameraservice/device3/Camera3OutputUtils.h @@ -62,7 +62,7 @@ namespace camera3 { std::mutex& inflightLock; InFlightRequestMap& inflightMap; // end of inflightLock scope std::mutex& outputLock; - List& resultQueue; + std::list& resultQueue; std::condition_variable& resultSignal; uint32_t& nextShutterFrameNum; uint32_t& nextReprocShutterFrameNum;