From fafbc479db0671fa27b94ec115dd3439623f47f5 Mon Sep 17 00:00:00 2001 From: Yin-Chia Yeh Date: Wed, 26 Jul 2017 16:53:20 -0700 Subject: [PATCH] Camera: fix various VTS issues Also fix wrong return values for processCaptureRequest in default wrapper. Test: running camera VTS Bug: 64041692 Change-Id: I397390af7c85a776713f6287bef1c4d11c721c9a Merged-In: I397390af7c85a776713f6287bef1c4d11c721c9a --- .../3.2/default/CameraDeviceSession.cpp | 16 +++++++++- .../device/3.2/default/CameraDeviceSession.h | 1 + .../VtsHalCameraProviderV2_4TargetTest.cpp | 29 ++++++++++++++----- 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/camera/device/3.2/default/CameraDeviceSession.cpp b/camera/device/3.2/default/CameraDeviceSession.cpp index f33adf8d1a..fcd134f45e 100644 --- a/camera/device/3.2/default/CameraDeviceSession.cpp +++ b/camera/device/3.2/default/CameraDeviceSession.cpp @@ -923,6 +923,7 @@ Return CameraDeviceSession::configureStreams( status = Status::INTERNAL_ERROR; } else { convertToHidl(stream_list, &outStreams); + mFirstRequest = true; } _hidl_cb(status, outStreams); @@ -1022,7 +1023,13 @@ Status CameraDeviceSession::processOneCaptureRequest(const CaptureRequest& reque if (!converted) { ALOGE("%s: capture request settings metadata is corrupt!", __FUNCTION__); - return Status::INTERNAL_ERROR; + return Status::ILLEGAL_ARGUMENT; + } + + if (mFirstRequest && halRequest.settings == nullptr) { + ALOGE("%s: capture request settings must not be null for first request!", + __FUNCTION__); + return Status::ILLEGAL_ARGUMENT; } hidl_vec allBufPtrs; @@ -1031,6 +1038,12 @@ Status CameraDeviceSession::processOneCaptureRequest(const CaptureRequest& reque request.inputBuffer.bufferId != 0); size_t numOutputBufs = request.outputBuffers.size(); size_t numBufs = numOutputBufs + (hasInputBuf ? 1 : 0); + + if (numOutputBufs == 0) { + ALOGE("%s: capture request must have at least one output buffer!", __FUNCTION__); + return Status::ILLEGAL_ARGUMENT; + } + status = importRequest(request, allBufPtrs, allFences); if (status != Status::OK) { return status; @@ -1102,6 +1115,7 @@ Status CameraDeviceSession::processOneCaptureRequest(const CaptureRequest& reque return Status::INTERNAL_ERROR; } + mFirstRequest = false; return Status::OK; } diff --git a/camera/device/3.2/default/CameraDeviceSession.h b/camera/device/3.2/default/CameraDeviceSession.h index fb3fc02a0f..2fe189fde1 100644 --- a/camera/device/3.2/default/CameraDeviceSession.h +++ b/camera/device/3.2/default/CameraDeviceSession.h @@ -148,6 +148,7 @@ private: static HandleImporter sHandleImporter; bool mInitFail; + bool mFirstRequest = false; common::V1_0::helper::CameraMetadata mDeviceInfo; diff --git a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp index e078614b7b..da782587ad 100644 --- a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp +++ b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp @@ -132,22 +132,34 @@ namespace { const char *kHAL1_0 = "1.0"; bool matchDeviceName(const hidl_string& deviceName, - const hidl_string &providerType, std::smatch& sm) { + const hidl_string &providerType, + std::string* deviceVersion, + std::string* cameraId) { ::android::String8 pattern; pattern.appendFormat(kDeviceNameRE, providerType.c_str()); std::regex e(pattern.string()); std::string deviceNameStd(deviceName.c_str()); - return std::regex_match(deviceNameStd, sm, e); + std::smatch sm; + if (std::regex_match(deviceNameStd, sm, e)) { + if (deviceVersion != nullptr) { + *deviceVersion = sm[1]; + } + if (cameraId != nullptr) { + *cameraId = sm[2]; + } + return true; + } + return false; } int getCameraDeviceVersion(const hidl_string& deviceName, const hidl_string &providerType) { - std::smatch sm; - bool match = matchDeviceName(deviceName, providerType, sm); + std::string version; + bool match = matchDeviceName(deviceName, providerType, &version, nullptr); if (!match) { return -1; } - std::string version = sm[1].str(); + if (version.compare(kHAL3_2) == 0) { // maybe switched to 3.4 or define the hidl version enumlater return CAMERA_DEVICE_API_VERSION_3_2; @@ -2978,6 +2990,9 @@ TEST_F(CameraHidlTest, processCaptureRequestPreview) { //Empty settings should be supported after the first call //for repeating requests. request.settings.setToExternal(nullptr, 0, true); + // The buffer has been registered to HAL by bufferId, so per + // API contract we should send a null handle for this buffer + request.outputBuffers[0].buffer = nullptr; mInflightMap.clear(); inflightReq = {1, false, supportsPartialResults, partialResultCount, resultQueue}; @@ -3074,7 +3089,7 @@ TEST_F(CameraHidlTest, processCaptureRequestInvalidSinglePreview) { numRequestProcessed = n; }); ASSERT_TRUE(ret.isOk()); - ASSERT_EQ(Status::INTERNAL_ERROR, status); + ASSERT_EQ(Status::ILLEGAL_ARGUMENT, status); ASSERT_EQ(numRequestProcessed, 0u); ret = session->close(); @@ -3135,7 +3150,7 @@ TEST_F(CameraHidlTest, processCaptureRequestInvalidBuffer) { numRequestProcessed = n; }); ASSERT_TRUE(ret.isOk()); - ASSERT_EQ(Status::INTERNAL_ERROR, status); + ASSERT_EQ(Status::ILLEGAL_ARGUMENT, status); ASSERT_EQ(numRequestProcessed, 0u); ret = session->close();