From 43549d934ea08427d21401dabc0f3067fe2bf5bb Mon Sep 17 00:00:00 2001 From: Avichal Rakesh Date: Thu, 25 Jan 2024 16:08:52 -0800 Subject: [PATCH] ExternalCameraHAL: Skip importing buffer from capture request ExternalCameraHAL supports HAL buffer management which means cameraservice will not send it an output buffer along with the capture request, and the HAL has the freedom to request an output buffer when an output buffer is needed. As a remnant of migration from HIDL to AIDL, the ExternalCameraHAL still attempted to import buffers that the cameraservice has sent with the CaptureRequest. However, with HAL buffer manager enabled, this buffer is always null and results in the HAL failing to process the capture request. This CL removes the logic for importing output buffers when processing capture requests from the cameraservice, and lets the HAL call requestStreamBuffers when it needs an output buffer. Bug: 299182874 Test: VTS Tests now pass Change-Id: I00654836b7ae91a91a2afa4b149712977e07dcc5 --- .../default/ExternalCameraDeviceSession.cpp | 76 ++++--------------- .../default/ExternalCameraDeviceSession.h | 9 --- 2 files changed, 15 insertions(+), 70 deletions(-) diff --git a/camera/device/default/ExternalCameraDeviceSession.cpp b/camera/device/default/ExternalCameraDeviceSession.cpp index a16dd7fd83..479167cf62 100644 --- a/camera/device/default/ExternalCameraDeviceSession.cpp +++ b/camera/device/default/ExternalCameraDeviceSession.cpp @@ -538,6 +538,19 @@ Status ExternalCameraDeviceSession::processOneCaptureRequest(const CaptureReques return Status::INTERNAL_ERROR; } + if (request.outputBuffers.empty()) { + ALOGE("%s: No output buffers provided.", __FUNCTION__); + return Status::ILLEGAL_ARGUMENT; + } + + for (auto& outputBuf : request.outputBuffers) { + if (outputBuf.streamId == -1 || mStreamMap.find(outputBuf.streamId) == mStreamMap.end()) { + ALOGE("%s: Invalid streamId in CaptureRequest.outputBuffers: %d", __FUNCTION__, + outputBuf.streamId); + return Status::ILLEGAL_ARGUMENT; + } + } + const camera_metadata_t* rawSettings = nullptr; bool converted; CameraMetadata settingsFmq; // settings from FMQ @@ -572,8 +585,6 @@ Status ExternalCameraDeviceSession::processOneCaptureRequest(const CaptureReques return Status::ILLEGAL_ARGUMENT; } - std::vector allBufPtrs; - std::vector allFences; size_t numOutputBufs = request.outputBuffers.size(); if (numOutputBufs == 0) { @@ -629,11 +640,6 @@ Status ExternalCameraDeviceSession::processOneCaptureRequest(const CaptureReques } } - status = importRequestLocked(request, allBufPtrs, allFences); - if (status != Status::OK) { - return status; - } - nsecs_t shutterTs = 0; std::unique_ptr frameIn = dequeueV4l2FrameLocked(&shutterTs); if (frameIn == nullptr) { @@ -656,8 +662,8 @@ Status ExternalCameraDeviceSession::processOneCaptureRequest(const CaptureReques halBuf.height = stream.height; halBuf.format = stream.format; halBuf.usage = stream.usage; - halBuf.bufPtr = allBufPtrs[i]; - halBuf.acquireFence = allFences[i]; + halBuf.bufPtr = nullptr; // threadloop will request buffer from cameraservice + halBuf.acquireFence = 0; // threadloop will request fence from cameraservice halBuf.fenceTimeout = false; } { @@ -1351,58 +1357,6 @@ bool ExternalCameraDeviceSession::isSupported( return false; } -Status ExternalCameraDeviceSession::importRequestLocked(const CaptureRequest& request, - std::vector& allBufPtrs, - std::vector& allFences) { - return importRequestLockedImpl(request, allBufPtrs, allFences); -} - -Status ExternalCameraDeviceSession::importRequestLockedImpl( - const CaptureRequest& request, std::vector& allBufPtrs, - std::vector& allFences) { - size_t numOutputBufs = request.outputBuffers.size(); - size_t numBufs = numOutputBufs; - // Validate all I/O buffers - std::vector allBufs; - std::vector allBufIds; - allBufs.resize(numBufs); - allBufIds.resize(numBufs); - allBufPtrs.resize(numBufs); - allFences.resize(numBufs); - std::vector streamIds(numBufs); - - for (size_t i = 0; i < numOutputBufs; i++) { - allBufs[i] = ::android::makeFromAidl(request.outputBuffers[i].buffer); - allBufIds[i] = request.outputBuffers[i].bufferId; - allBufPtrs[i] = &allBufs[i]; - streamIds[i] = request.outputBuffers[i].streamId; - } - - { - Mutex::Autolock _l(mCbsLock); - for (size_t i = 0; i < numBufs; i++) { - Status st = importBufferLocked(streamIds[i], allBufIds[i], allBufs[i], &allBufPtrs[i]); - if (st != Status::OK) { - // Detailed error logs printed in importBuffer - return st; - } - } - } - - // All buffers are imported. Now validate output buffer acquire fences - for (size_t i = 0; i < numOutputBufs; i++) { - native_handle_t* h = ::android::makeFromAidl(request.outputBuffers[i].acquireFence); - if (!sHandleImporter.importFence(h, allFences[i])) { - ALOGE("%s: output buffer %zu acquire fence is invalid", __FUNCTION__, i); - cleanupInflightFences(allFences, i); - native_handle_delete(h); - return Status::INTERNAL_ERROR; - } - native_handle_delete(h); - } - return Status::OK; -} - Status ExternalCameraDeviceSession::importBuffer(int32_t streamId, uint64_t bufId, buffer_handle_t buf, /*out*/ buffer_handle_t** outBufPtr) { diff --git a/camera/device/default/ExternalCameraDeviceSession.h b/camera/device/default/ExternalCameraDeviceSession.h index 736bfd1528..795b5899b6 100644 --- a/camera/device/default/ExternalCameraDeviceSession.h +++ b/camera/device/default/ExternalCameraDeviceSession.h @@ -266,15 +266,6 @@ class ExternalCameraDeviceSession : public BnCameraDeviceSession, public OutputT const std::vector& supportedFormats, const ExternalCameraConfig& cfg); - // Validate and import request's output buffers and acquire fence - Status importRequestLocked(const CaptureRequest& request, - std::vector& allBufPtrs, - std::vector& allFences); - - Status importRequestLockedImpl(const CaptureRequest& request, - std::vector& allBufPtrs, - std::vector& allFences); - Status importBufferLocked(int32_t streamId, uint64_t bufId, buffer_handle_t buf, /*out*/ buffer_handle_t** outBufPtr); static void cleanupInflightFences(std::vector& allFences, size_t numFences);