From 28eebbfc41f5114eae3a8d0234c1f1fcedffdf1a Mon Sep 17 00:00:00 2001 From: Yin-Chia Yeh Date: Thu, 30 Mar 2017 15:06:20 -0700 Subject: [PATCH] Camera: add interface to evict obsolete buffer caches Test: fix CTS ReprocessCaptureTest Bug: 34461678 Change-Id: Icde654b0c8423c31d7d39d180913ffa374e7de3c --- camera/device/3.2/ICameraDeviceSession.hal | 8 +++++- .../3.2/default/CameraDeviceSession.cpp | 26 ++++++++++++++++++- .../device/3.2/default/CameraDeviceSession.h | 6 ++++- camera/device/3.2/types.hal | 22 ++++++++++++++++ .../VtsHalCameraProviderV2_4TargetTest.cpp | 10 +++++++ 5 files changed, 69 insertions(+), 3 deletions(-) diff --git a/camera/device/3.2/ICameraDeviceSession.hal b/camera/device/3.2/ICameraDeviceSession.hal index e186c8daf9..731fc764aa 100644 --- a/camera/device/3.2/ICameraDeviceSession.hal +++ b/camera/device/3.2/ICameraDeviceSession.hal @@ -183,6 +183,11 @@ interface ICameraDeviceSession { * client, the HAL must process the requests in order of lowest index to * highest index. * + * The cachesToRemove argument contains a list of buffer caches (see + * StreamBuffer document for more information on buffer cache) to be removed + * by camera HAL. Camera HAL must remove these cache entries whether or not + * this method returns OK. + * * The actual request processing is asynchronous, with the results of * capture being returned by the HAL through the processCaptureResult() * call. This call requires the result metadata to be available, but output @@ -238,7 +243,8 @@ interface ICameraDeviceSession { * that HAL processed successfully before HAL runs into an error. * */ - processCaptureRequest(vec requests) + processCaptureRequest(vec requests, + vec cachesToRemove) generates (Status status, uint32_t numRequestProcessed); /** diff --git a/camera/device/3.2/default/CameraDeviceSession.cpp b/camera/device/3.2/default/CameraDeviceSession.cpp index fb1d1ff7c1..5b3024b238 100644 --- a/camera/device/3.2/default/CameraDeviceSession.cpp +++ b/camera/device/3.2/default/CameraDeviceSession.cpp @@ -678,8 +678,32 @@ void CameraDeviceSession::cleanupBuffersLocked(int id) { mCirculatingBuffers.erase(id); } +void CameraDeviceSession::updateBufferCaches(const hidl_vec& cachesToRemove) { + Mutex::Autolock _l(mInflightLock); + for (auto& cache : cachesToRemove) { + auto cbsIt = mCirculatingBuffers.find(cache.streamId); + if (cbsIt == mCirculatingBuffers.end()) { + // The stream could have been removed + continue; + } + CirculatingBuffers& cbs = cbsIt->second; + auto it = cbs.find(cache.bufferId); + if (it != cbs.end()) { + sHandleImporter.freeBuffer(it->second); + cbs.erase(it); + } else { + ALOGE("%s: stream %d buffer %" PRIu64 " is not cached", + __FUNCTION__, cache.streamId, cache.bufferId); + } + } +} + Return CameraDeviceSession::processCaptureRequest( - const hidl_vec& requests, processCaptureRequest_cb _hidl_cb) { + const hidl_vec& requests, + const hidl_vec& cachesToRemove, + processCaptureRequest_cb _hidl_cb) { + updateBufferCaches(cachesToRemove); + uint32_t numRequestProcessed = 0; Status s = Status::OK; for (size_t i = 0; i < requests.size(); i++, numRequestProcessed++) { diff --git a/camera/device/3.2/default/CameraDeviceSession.h b/camera/device/3.2/default/CameraDeviceSession.h index 8923c05bcf..781056eb3b 100644 --- a/camera/device/3.2/default/CameraDeviceSession.h +++ b/camera/device/3.2/default/CameraDeviceSession.h @@ -85,7 +85,9 @@ struct CameraDeviceSession : public ICameraDeviceSession, private camera3_callba Return configureStreams( const StreamConfiguration& requestedConfiguration, configureStreams_cb _hidl_cb) override; Return processCaptureRequest( - const hidl_vec& requests, processCaptureRequest_cb _hidl_cb) override; + const hidl_vec& requests, + const hidl_vec& cachesToRemove, + processCaptureRequest_cb _hidl_cb) override; Return flush() override; Return close() override; @@ -234,6 +236,8 @@ private: void cleanupBuffersLocked(int id); + void updateBufferCaches(const hidl_vec& cachesToRemove); + Status processOneCaptureRequest(const CaptureRequest& request); /** * Static callback forwarding methods from HAL to instance diff --git a/camera/device/3.2/types.hal b/camera/device/3.2/types.hal index fd755284b4..5ae7a18c39 100644 --- a/camera/device/3.2/types.hal +++ b/camera/device/3.2/types.hal @@ -958,3 +958,25 @@ struct CaptureResult { uint32_t partialResult; }; + +/** + * BufferCache: + * + * A list of cached bufferIds associated with a certain stream. + * Buffers are passed between camera service and camera HAL via bufferId except + * the first time a new buffer is being passed to HAL in CaptureRequest. Camera + * service and camera HAL therefore need to maintain a cached map of bufferId + * and corresponing native handle. + * + */ +struct BufferCache { + /** + * The ID of the stream this list is associated with. + */ + int32_t streamId; + + /** + * A cached buffer ID associated with streamId. + */ + uint64_t bufferId; +}; diff --git a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp index a79c9fafaa..47cc9f18aa 100644 --- a/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp +++ b/camera/provider/2.4/vts/functional/VtsHalCameraProviderV2_4TargetTest.cpp @@ -61,6 +61,7 @@ using ::android::hardware::camera::common::V1_0::TorchModeStatus; using ::android::hardware::camera::provider::V2_4::ICameraProvider; using ::android::hardware::camera::provider::V2_4::ICameraProviderCallback; using ::android::hardware::camera::device::V3_2::ICameraDevice; +using ::android::hardware::camera::device::V3_2::BufferCache; using ::android::hardware::camera::device::V3_2::CaptureRequest; using ::android::hardware::camera::device::V3_2::CaptureResult; using ::android::hardware::camera::device::V3_2::ICameraDeviceCallback; @@ -2481,8 +2482,10 @@ TEST_F(CameraHidlTest, processCaptureRequestPreview) { Status status = Status::INTERNAL_ERROR; uint32_t numRequestProcessed = 0; + hidl_vec cachesToRemove; Return returnStatus = session->processCaptureRequest( {request}, + cachesToRemove, [&status, &numRequestProcessed] (auto s, uint32_t n) { status = s; numRequestProcessed = n; @@ -2513,6 +2516,7 @@ TEST_F(CameraHidlTest, processCaptureRequestPreview) { returnStatus = session->processCaptureRequest( {request}, + cachesToRemove, [&status, &numRequestProcessed] (auto s, uint32_t n) { status = s; numRequestProcessed = n; @@ -2579,8 +2583,10 @@ TEST_F(CameraHidlTest, processCaptureRequestInvalidSinglePreview) { //Settings were not correctly initialized, we should fail here Status status = Status::OK; uint32_t numRequestProcessed = 0; + hidl_vec cachesToRemove; Return ret = session->processCaptureRequest( {request}, + cachesToRemove, [&status, &numRequestProcessed] (auto s, uint32_t n) { status = s; numRequestProcessed = n; @@ -2632,8 +2638,10 @@ TEST_F(CameraHidlTest, processCaptureRequestInvalidBuffer) { //Output buffers are missing, we should fail here Status status = Status::OK; uint32_t numRequestProcessed = 0; + hidl_vec cachesToRemove; ret = session->processCaptureRequest( {request}, + cachesToRemove, [&status, &numRequestProcessed] (auto s, uint32_t n) { status = s; numRequestProcessed = n; @@ -2701,8 +2709,10 @@ TEST_F(CameraHidlTest, flushPreviewRequest) { Status status = Status::INTERNAL_ERROR; uint32_t numRequestProcessed = 0; + hidl_vec cachesToRemove; ret = session->processCaptureRequest( {request}, + cachesToRemove, [&status, &numRequestProcessed] (auto s, uint32_t n) { status = s; numRequestProcessed = n;