From 1c834da07093c79d2a8fedb47836b78f00a31e2e Mon Sep 17 00:00:00 2001 From: Shuzhen Wang Date: Wed, 18 Dec 2019 11:17:03 -0800 Subject: [PATCH] Camera3: Remove mutex from ZoomRatioMapper ZoomRatioMapper's member variables are const after construction. As a result, we don't need mutex to protect updateCaptureRequest and updateCaptureResult calls. Also fixed missing zoom ratio mapping initialization for physical sub-camera. Test: testZoomRatio CTS Bug: 130025314 Change-Id: Ibe2ff7cffb13178dcf70fc9e2eb044184e58bcf2 --- .../common/CameraProviderManager.cpp | 7 +++++++ .../device3/Camera3Device.cpp | 14 ++----------- .../device3/ZoomRatioMapper.cpp | 18 +++++++--------- .../device3/ZoomRatioMapper.h | 21 ++++++------------- .../libcameraservice/tests/ZoomRatioTest.cpp | 19 +++++++++-------- 5 files changed, 33 insertions(+), 46 deletions(-) diff --git a/services/camera/libcameraservice/common/CameraProviderManager.cpp b/services/camera/libcameraservice/common/CameraProviderManager.cpp index 23f78842f7..42698c57b5 100644 --- a/services/camera/libcameraservice/common/CameraProviderManager.cpp +++ b/services/camera/libcameraservice/common/CameraProviderManager.cpp @@ -2112,6 +2112,13 @@ CameraProviderManager::ProviderInfo::DeviceInfo3::DeviceInfo3(const std::string& CameraProviderManager::statusToString(status), status); return; } + + res = camera3::ZoomRatioMapper::overrideZoomRatioTags( + &mPhysicalCameraCharacteristics[id], &mSupportNativeZoomRatio); + if (OK != res) { + ALOGE("%s: Unable to override zoomRatio related tags: %s (%d)", + __FUNCTION__, strerror(-res), res); + } } } diff --git a/services/camera/libcameraservice/device3/Camera3Device.cpp b/services/camera/libcameraservice/device3/Camera3Device.cpp index 1c5281d5eb..09b1f02623 100644 --- a/services/camera/libcameraservice/device3/Camera3Device.cpp +++ b/services/camera/libcameraservice/device3/Camera3Device.cpp @@ -161,14 +161,9 @@ status_t Camera3Device::initialize(sp manager, const Stri } } - res = mZoomRatioMappers[physicalId].initZoomRatioTags( + mZoomRatioMappers[physicalId] = ZoomRatioMapper( &mPhysicalDeviceInfoMap[physicalId], mSupportNativeZoomRatio, usePrecorrectArray); - if (res != OK) { - SET_ERR_L("Failed to initialize camera %s's zoomRatio tags: %s (%d)", - physicalId.c_str(), strerror(-res), res); - return res; - } } } @@ -353,13 +348,8 @@ status_t Camera3Device::initializeCommonLocked() { } } - res = mZoomRatioMappers[mId.c_str()].initZoomRatioTags(&mDeviceInfo, + mZoomRatioMappers[mId.c_str()] = ZoomRatioMapper(&mDeviceInfo, mSupportNativeZoomRatio, usePrecorrectArray); - if (res != OK) { - SET_ERR_L("Failed to initialize zoomRatio tags: %s (%d)", - strerror(-res), res); - return res; - } return OK; } diff --git a/services/camera/libcameraservice/device3/ZoomRatioMapper.cpp b/services/camera/libcameraservice/device3/ZoomRatioMapper.cpp index 7718819f4c..84da45a069 100644 --- a/services/camera/libcameraservice/device3/ZoomRatioMapper.cpp +++ b/services/camera/libcameraservice/device3/ZoomRatioMapper.cpp @@ -25,8 +25,6 @@ namespace android { namespace camera3 { -ZoomRatioMapper::ZoomRatioMapper() : mHalSupportsZoomRatio(false) { -} status_t ZoomRatioMapper::initZoomRatioInTemplate(CameraMetadata *request) { camera_metadata_entry_t entry; @@ -117,19 +115,17 @@ status_t ZoomRatioMapper::overrideZoomRatioTags( return OK; } -status_t ZoomRatioMapper::initZoomRatioTags(const CameraMetadata* deviceInfo, +ZoomRatioMapper::ZoomRatioMapper(const CameraMetadata* deviceInfo, bool supportNativeZoomRatio, bool usePrecorrectArray) { - std::lock_guard lock(mMutex); - camera_metadata_ro_entry_t entry; entry = deviceInfo->find(ANDROID_SENSOR_INFO_PRE_CORRECTION_ACTIVE_ARRAY_SIZE); - if (entry.count != 4) return BAD_VALUE; + if (entry.count != 4) return; int32_t arrayW = entry.data.i32[2]; int32_t arrayH = entry.data.i32[3]; entry = deviceInfo->find(ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE); - if (entry.count != 4) return BAD_VALUE; + if (entry.count != 4) return; int32_t activeW = entry.data.i32[2]; int32_t activeH = entry.data.i32[3]; @@ -144,11 +140,12 @@ status_t ZoomRatioMapper::initZoomRatioTags(const CameraMetadata* deviceInfo, ALOGV("%s: array size: %d x %d, mHalSupportsZoomRatio %d", __FUNCTION__, mArrayWidth, mArrayHeight, mHalSupportsZoomRatio); - return OK; + mIsValid = true; } status_t ZoomRatioMapper::updateCaptureRequest(CameraMetadata* request) { - std::lock_guard lock(mMutex); + if (!mIsValid) return INVALID_OPERATION; + status_t res = OK; bool zoomRatioIs1 = true; camera_metadata_entry_t entry; @@ -174,7 +171,8 @@ status_t ZoomRatioMapper::updateCaptureRequest(CameraMetadata* request) { } status_t ZoomRatioMapper::updateCaptureResult(CameraMetadata* result, bool requestedZoomRatioIs1) { - std::lock_guard lock(mMutex); + if (!mIsValid) return INVALID_OPERATION; + status_t res = OK; if (mHalSupportsZoomRatio && requestedZoomRatioIs1) { diff --git a/services/camera/libcameraservice/device3/ZoomRatioMapper.h b/services/camera/libcameraservice/device3/ZoomRatioMapper.h index 38efbfdf28..16b223b299 100644 --- a/services/camera/libcameraservice/device3/ZoomRatioMapper.h +++ b/services/camera/libcameraservice/device3/ZoomRatioMapper.h @@ -19,7 +19,6 @@ #include #include -#include #include "camera/CameraMetadata.h" #include "device3/CoordinateMapper.h" @@ -36,7 +35,9 @@ namespace camera3 { */ class ZoomRatioMapper : private CoordinateMapper { public: - ZoomRatioMapper(); + ZoomRatioMapper() = default; + ZoomRatioMapper(const CameraMetadata *deviceInfo, + bool supportNativeZoomRatio, bool usePrecorrectArray); ZoomRatioMapper(const ZoomRatioMapper& other) : mHalSupportsZoomRatio(other.mHalSupportsZoomRatio), mArrayWidth(other.mArrayWidth), mArrayHeight(other.mArrayHeight) {} @@ -52,16 +53,6 @@ class ZoomRatioMapper : private CoordinateMapper { static status_t overrideZoomRatioTags( CameraMetadata* deviceInfo, bool* supportNativeZoomRatio); - /** - * Initialize zoom ratio mapper with static metadata. - * - * Note: - * This function may modify the static metadata with zoomRatio related - * tags. - */ - status_t initZoomRatioTags(const CameraMetadata *deviceInfo, - bool supportNativeZoomRatio, bool usePrecorrectArray); - /** * Update capture request to handle both cropRegion and zoomRatio. */ @@ -82,16 +73,16 @@ class ZoomRatioMapper : private CoordinateMapper { void scaleCoordinates(int32_t* coordPairs, int coordCount, float scaleRatio, ClampMode clamp); + bool isValid() { return mIsValid; } private: + // const after construction bool mHalSupportsZoomRatio; - // active array / pre-correction array dimension int32_t mArrayWidth, mArrayHeight; - mutable std::mutex mMutex; + bool mIsValid = false; float deriveZoomRatio(const CameraMetadata* metadata); - void scaleRects(int32_t* rects, int rectCount, float scaleRatio); status_t separateZoomFromCropLocked(CameraMetadata* metadata, bool isResult); diff --git a/services/camera/libcameraservice/tests/ZoomRatioTest.cpp b/services/camera/libcameraservice/tests/ZoomRatioTest.cpp index 1bfa03faca..300da099a3 100644 --- a/services/camera/libcameraservice/tests/ZoomRatioTest.cpp +++ b/services/camera/libcameraservice/tests/ZoomRatioTest.cpp @@ -66,7 +66,8 @@ status_t setupTestMapper(ZoomRatioMapper *m, float maxDigitalZoom, return res; } - return m->initZoomRatioTags(&deviceInfo, hasZoomRatioRange, usePreCorrectArray); + *m = ZoomRatioMapper(&deviceInfo, hasZoomRatioRange, usePreCorrectArray); + return OK; } TEST(ZoomRatioTest, Initialization) { @@ -86,12 +87,12 @@ TEST(ZoomRatioTest, Initialization) { res = ZoomRatioMapper::overrideZoomRatioTags(&deviceInfo, &supportNativeZoomRatio); ASSERT_EQ(res, OK); ASSERT_EQ(supportNativeZoomRatio, false); - res = mapperNoZoomRange.initZoomRatioTags(&deviceInfo, + mapperNoZoomRange = ZoomRatioMapper(&deviceInfo, supportNativeZoomRatio, true/*usePreCorrectArray*/); - ASSERT_EQ(res, OK); - res = mapperNoZoomRange.initZoomRatioTags(&deviceInfo, + ASSERT_TRUE(mapperNoZoomRange.isValid()); + mapperNoZoomRange = ZoomRatioMapper(&deviceInfo, supportNativeZoomRatio, false/*usePreCorrectArray*/); - ASSERT_EQ(res, OK); + ASSERT_TRUE(mapperNoZoomRange.isValid()); entry = deviceInfo.find(ANDROID_CONTROL_ZOOM_RATIO_RANGE); ASSERT_EQ(entry.count, 2U); @@ -120,12 +121,12 @@ TEST(ZoomRatioTest, Initialization) { ASSERT_EQ(res, OK); ASSERT_EQ(supportNativeZoomRatio, true); ZoomRatioMapper mapperWithZoomRange; - res = mapperWithZoomRange.initZoomRatioTags(&deviceInfo, + mapperWithZoomRange = ZoomRatioMapper(&deviceInfo, supportNativeZoomRatio, true/*usePreCorrectArray*/); - ASSERT_EQ(res, OK); - res = mapperWithZoomRange.initZoomRatioTags(&deviceInfo, + ASSERT_TRUE(mapperWithZoomRange.isValid()); + mapperWithZoomRange = ZoomRatioMapper(&deviceInfo, supportNativeZoomRatio, false/*usePreCorrectArray*/); - ASSERT_EQ(res, OK); + ASSERT_TRUE(mapperWithZoomRange.isValid()); entry = deviceInfo.find(ANDROID_CONTROL_ZOOM_RATIO_RANGE); ASSERT_EQ(entry.count, 2U);