From a343d98e8cc070c84123bfc76560cbb666a87f62 Mon Sep 17 00:00:00 2001 From: Bharatt Kukreja Date: Thu, 11 Jan 2024 21:19:41 +0000 Subject: [PATCH] Restrict HAL to only required keys for session chars Don't allow Camera HAL to send keys other than the required ones for session characteristics. Test: atest VtsAidlHalCameraProvider_TargetTest Bug: 314386872 Change-Id: I3808840e0d404b4a82c8bcfc6d51eab9b171b1e9 --- .../hardware/camera/device/ICameraDevice.aidl | 3 + .../VtsAidlHalCameraProvider_TargetTest.cpp | 10 +- camera/provider/aidl/vts/camera_aidl_test.cpp | 113 +++++++++++------- camera/provider/aidl/vts/camera_aidl_test.h | 3 +- 4 files changed, 84 insertions(+), 45 deletions(-) diff --git a/camera/device/aidl/android/hardware/camera/device/ICameraDevice.aidl b/camera/device/aidl/android/hardware/camera/device/ICameraDevice.aidl index 01009ad788..0129dea26f 100644 --- a/camera/device/aidl/android/hardware/camera/device/ICameraDevice.aidl +++ b/camera/device/aidl/android/hardware/camera/device/ICameraDevice.aidl @@ -446,6 +446,9 @@ interface ICameraDevice { * - ANDROID_CONTROL_ZOOM_RATIO_RANGE * - SCALER_AVAILABLE_MAX_DIGITAL_ZOOM * + * No other tags (other than vendor tags) should be set in the characteristics returned from + * the HAL. + * * A service specific error will be returned on the following conditions * INTERNAL_ERROR: * The camera device cannot be opened due to an internal diff --git a/camera/provider/aidl/vts/VtsAidlHalCameraProvider_TargetTest.cpp b/camera/provider/aidl/vts/VtsAidlHalCameraProvider_TargetTest.cpp index 9a5f248a6c..6e3ddc9094 100644 --- a/camera/provider/aidl/vts/VtsAidlHalCameraProvider_TargetTest.cpp +++ b/camera/provider/aidl/vts/VtsAidlHalCameraProvider_TargetTest.cpp @@ -330,10 +330,14 @@ TEST_P(CameraAidlTest, getSessionCharacteristics) { StreamConfiguration config; createStreamConfiguration(streams, StreamConfigurationMode::NORMAL_MODE, &config); - CameraMetadata chars; - ret = device->getSessionCharacteristics(config, &chars); + CameraMetadata camera_chars; + ret = device->getCameraCharacteristics(&camera_chars); ASSERT_TRUE(ret.isOk()); - verifySessionCharacteristics(chars); + + CameraMetadata session_chars; + ret = device->getSessionCharacteristics(config, &session_chars); + ASSERT_TRUE(ret.isOk()); + verifySessionCharacteristics(session_chars, camera_chars); } } else { ALOGI("getSessionCharacteristics: Test skipped.\n"); diff --git a/camera/provider/aidl/vts/camera_aidl_test.cpp b/camera/provider/aidl/vts/camera_aidl_test.cpp index d2a409e0cc..9e55434fb6 100644 --- a/camera/provider/aidl/vts/camera_aidl_test.cpp +++ b/camera/provider/aidl/vts/camera_aidl_test.cpp @@ -1931,52 +1931,83 @@ void CameraAidlTest::verifyStreamCombination(const std::shared_ptr(chars.metadata.data()); +void CameraAidlTest::verifySessionCharacteristics(const CameraMetadata& session_chars, + const CameraMetadata& camera_chars) { + if (!flags::feature_combination_query()) { + return; + } - size_t expectedSize = chars.metadata.size(); - int result = validate_camera_metadata_structure(metadata, &expectedSize); - ASSERT_TRUE((result == 0) || (result == CAMERA_METADATA_VALIDATION_SHIFTED)); - size_t entryCount = get_camera_metadata_entry_count(metadata); - ASSERT_GT(entryCount, 0u); + const camera_metadata_t* session_metadata = + reinterpret_cast(session_chars.metadata.data()); - camera_metadata_ro_entry entry; - int retcode = 0; - float maxDigitalZoom = 1.0; + const camera_metadata_t* camera_metadata = + reinterpret_cast(camera_chars.metadata.data()); - retcode = find_camera_metadata_ro_entry(metadata, ANDROID_SCALER_AVAILABLE_MAX_DIGITAL_ZOOM, - &entry); - // ANDROID_SCALER_AVAILABLE_MAX_DIGITAL_ZOOM should always be present. - if ((0 == retcode) && (entry.count == 1)) { - maxDigitalZoom = entry.data.f[0]; - } else { - ADD_FAILURE() << "Get camera scalerAvailableMaxDigitalZoom failed!"; + size_t expectedSize = session_chars.metadata.size(); + int result = validate_camera_metadata_structure(session_metadata, &expectedSize); + ASSERT_TRUE((result == 0) || (result == CAMERA_METADATA_VALIDATION_SHIFTED)); + size_t entryCount = get_camera_metadata_entry_count(session_metadata); + // There should be at least 1 characteristic present: + // SCALER_MAX_DIGITAL_ZOOM must always be available. + // ZOOM_RATIO_RANGE must be available if ZOOM_RATIO is supported. + ASSERT_TRUE(entryCount >= 1); + + camera_metadata_ro_entry entry; + int retcode = 0; + float maxDigitalZoom = 1.0; + + for (size_t i = 0; i < entryCount; i++) { + retcode = get_camera_metadata_ro_entry(session_metadata, i, &entry); + ASSERT_TRUE(retcode == 0); + + std::set allowed_tags = {ANDROID_SCALER_AVAILABLE_MAX_DIGITAL_ZOOM, + ANDROID_CONTROL_ZOOM_RATIO_RANGE}; + + if (contains(allowed_tags, entry.tag)) { + continue; } - retcode = find_camera_metadata_ro_entry(metadata, ANDROID_CONTROL_ZOOM_RATIO_RANGE, &entry); - bool hasZoomRatioRange = (0 == retcode && entry.count == 2); - if (!hasZoomRatioRange) { - return; - } - float minZoomRatio = entry.data.f[0]; - float maxZoomRatio = entry.data.f[1]; - constexpr float FLOATING_POINT_THRESHOLD = 0.00001f; - if (abs(maxDigitalZoom - maxZoomRatio) > FLOATING_POINT_THRESHOLD) { - ADD_FAILURE() << "Difference between maximum digital zoom " << maxDigitalZoom - << " and maximum zoom ratio " << maxZoomRatio - << " is greater than the threshold " << FLOATING_POINT_THRESHOLD << "!"; - } - if (minZoomRatio > maxZoomRatio) { - ADD_FAILURE() << "Maximum zoom ratio is less than minimum zoom ratio!"; - } - if (minZoomRatio > 1.0f) { - ADD_FAILURE() << "Minimum zoom ratio is more than 1.0!"; - } - if (maxZoomRatio < 1.0f) { - ADD_FAILURE() << "Maximum zoom ratio is less than 1.0!"; - } + // Other than the ones above, no tags should be allowed apart from vendor tags. + ASSERT_TRUE(entry.tag >= VENDOR_SECTION_START); + } + + retcode = find_camera_metadata_ro_entry(session_metadata, + ANDROID_SCALER_AVAILABLE_MAX_DIGITAL_ZOOM, &entry); + if ((0 == retcode) && (entry.count == 1)) { + maxDigitalZoom = entry.data.f[0]; + } else { + ADD_FAILURE() << "Get camera scalerAvailableMaxDigitalZoom failed!"; + } + + retcode = find_camera_metadata_ro_entry(camera_metadata, ANDROID_CONTROL_ZOOM_RATIO_RANGE, + &entry); + bool hasZoomRatioRange = (0 == retcode && entry.count == 2); + if (!hasZoomRatioRange) { + ALOGI("Skipping the rest of the test as ZOOM_RATIO_RANGE is not in camera characteristics"); + return; + } + + // Session characteristics must contain zoom_ratio_range if camera characteristics has it. + retcode = find_camera_metadata_ro_entry(session_metadata, ANDROID_CONTROL_ZOOM_RATIO_RANGE, + &entry); + ASSERT_TRUE(0 == retcode && entry.count == 2); + + float minZoomRatio = entry.data.f[0]; + float maxZoomRatio = entry.data.f[1]; + constexpr float FLOATING_POINT_THRESHOLD = 0.00001f; + if (abs(maxDigitalZoom - maxZoomRatio) > FLOATING_POINT_THRESHOLD) { + ADD_FAILURE() << "Difference between maximum digital zoom " << maxDigitalZoom + << " and maximum zoom ratio " << maxZoomRatio + << " is greater than the threshold " << FLOATING_POINT_THRESHOLD << "!"; + } + if (minZoomRatio > maxZoomRatio) { + ADD_FAILURE() << "Maximum zoom ratio is less than minimum zoom ratio!"; + } + if (minZoomRatio > 1.0f) { + ADD_FAILURE() << "Minimum zoom ratio is more than 1.0!"; + } + if (maxZoomRatio < 1.0f) { + ADD_FAILURE() << "Maximum zoom ratio is less than 1.0!"; } } diff --git a/camera/provider/aidl/vts/camera_aidl_test.h b/camera/provider/aidl/vts/camera_aidl_test.h index 507a6373ba..29c5894a30 100644 --- a/camera/provider/aidl/vts/camera_aidl_test.h +++ b/camera/provider/aidl/vts/camera_aidl_test.h @@ -290,7 +290,8 @@ class CameraAidlTest : public ::testing::TestWithParam { static void verifyStreamCombination(const std::shared_ptr& device, const StreamConfiguration& config, bool expectedStatus); - static void verifySessionCharacteristics(const CameraMetadata& chars); + static void verifySessionCharacteristics(const CameraMetadata& session_chars, + const CameraMetadata& camera_chars); static void verifyLogicalCameraResult(const camera_metadata_t* staticMetadata, const std::vector& resultMetadata);