From patchwork Sat Oct 23 10:32:58 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14289 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 314DDBDB1C for ; Sat, 23 Oct 2021 10:33:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E304B60129; Sat, 23 Oct 2021 12:33:18 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="eb9mVE5C"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D6F0D6486D for ; Sat, 23 Oct 2021 12:33:15 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.213]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D77CC547; Sat, 23 Oct 2021 12:33:14 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1634985195; bh=G06W9ZadYI3FT7xONrL5GjiZvjR7RdUAUJDMOf/S70A=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=eb9mVE5C8UxZOhhRmccxotpoehOq54nGAciz5Lfbe6xPFCn63wmzpx9micPcRyatw wiL44BgIKXpVqRjBShQKR1BgH+zZrCI9TeqiRMWzwOrbu9LVcSkKYZMxgti5RlwuSv 3Db7n5mxKU1ZetZMqJElBkgPKT4hDp276+8+USgU= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Sat, 23 Oct 2021 16:02:58 +0530 Message-Id: <20211023103302.152769-4-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211023103302.152769-1-umang.jain@ideasonboard.com> References: <20211023103302.152769-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v6 3/7] android: camera_device: Refactor descriptor status and sendCaptureResults() X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Currently, we use Camera3RequestDescriptor::Status to determine: - When the descriptor has been completely processed by HAL - Whether any errors were encountered, during its processing Both of these are essential to know whether the descriptor is eligible to call process_capture_results() through sendCaptureResults(). When a status(Success/Error) is set on the descriptor, it is ready to be sent back via sendCaptureResults(). However, this might lead to undesired results especially when sendCaptureResults() runs in a different thread (for e.g. stream's post-processor async completion slot). This patch decouples the descriptor status (Success/Error) from the descriptor's completion status (pending or complete). The advantage of this is we can set the completion status when the descriptor has been processed fully by the layer and we can set the error status on the descriptor wherever an error is encountered, throughout the lifetime descriptor in the HAL layer. While at it, introduce a wrapper completeDescriptor() around sendCaptureResults(). completeDescriptor() as the name suggests will mark the descriptor as complete, so it is ready to be sent back. The locking mechanism is moved from sendCaptureResults() to this wrapper since the intention is to use completeDescriptor() in place of existing sendCaptureResults() calls. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart Reviewed-by: Hirokazu Honda --- src/android/camera_device.cpp | 33 ++++++++++++++++++--------------- src/android/camera_device.h | 1 + src/android/camera_request.cpp | 2 +- src/android/camera_request.h | 7 ++++--- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 806b4090..f4d4fb09 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -982,13 +982,13 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques MutexLocker stateLock(stateMutex_); if (state_ == State::Flushing) { - abortRequest(descriptor.get()); + Camera3RequestDescriptor *rawDescriptor = descriptor.get(); + abortRequest(rawDescriptor); { MutexLocker descriptorsLock(descriptorsMutex_); descriptors_.push(std::move(descriptor)); } - - sendCaptureResults(); + completeDescriptor(rawDescriptor); return 0; } @@ -1079,7 +1079,7 @@ void CameraDevice::requestComplete(Request *request) << request->status(); abortRequest(descriptor); - sendCaptureResults(); + completeDescriptor(descriptor); return; } @@ -1117,6 +1117,7 @@ void CameraDevice::requestComplete(Request *request) } /* Handle post-processing. */ + bool hasPostProcessingErrors = false; for (auto &buffer : descriptor->buffers_) { CameraStream *stream = buffer.stream; @@ -1129,6 +1130,7 @@ void CameraDevice::requestComplete(Request *request) buffer.status = Camera3RequestDescriptor::Status::Error; notifyError(descriptor->frameNumber_, stream->camera3Stream(), CAMERA3_MSG_ERROR_BUFFER); + hasPostProcessingErrors = true; continue; } @@ -1143,29 +1145,32 @@ void CameraDevice::requestComplete(Request *request) if (ret) { buffer.status = Camera3RequestDescriptor::Status::Error; + hasPostProcessingErrors = true; notifyError(descriptor->frameNumber_, stream->camera3Stream(), CAMERA3_MSG_ERROR_BUFFER); } } - descriptor->status_ = Camera3RequestDescriptor::Status::Success; + if (hasPostProcessingErrors) + descriptor->status_ = Camera3RequestDescriptor::Status::Error; + + completeDescriptor(descriptor); +} + +void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor) +{ + MutexLocker lock(descriptorsMutex_); + descriptor->completeDescriptor(); + sendCaptureResults(); } void CameraDevice::sendCaptureResults() { - MutexLocker lock(descriptorsMutex_); while (!descriptors_.empty() && !descriptors_.front()->isPending()) { auto descriptor = std::move(descriptors_.front()); descriptors_.pop(); - /* - * \todo Releasing and re-acquiring the critical section for - * every request completion (grain-locking) might have an - * impact on performance which should be measured. - */ - lock.unlock(); - camera3_capture_result_t captureResult = {}; captureResult.frame_number = descriptor->frameNumber_; @@ -1201,8 +1206,6 @@ void CameraDevice::sendCaptureResults() captureResult.partial_result = 1; callbacks_->process_capture_result(callbacks_, &captureResult); - - lock.lock(); } } diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 863cf414..e544f2bd 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -93,6 +93,7 @@ private: void notifyError(uint32_t frameNumber, camera3_stream_t *stream, camera3_error_msg_code code) const; int processControls(Camera3RequestDescriptor *descriptor); + void completeDescriptor(Camera3RequestDescriptor *descriptor); void sendCaptureResults(); std::unique_ptr getResultMetadata( const Camera3RequestDescriptor &descriptor) const; diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp index faa85ada..16cf266f 100644 --- a/src/android/camera_request.cpp +++ b/src/android/camera_request.cpp @@ -36,7 +36,7 @@ Camera3RequestDescriptor::Camera3RequestDescriptor( static_cast(buffer.stream->priv); buffers_.push_back({ stream, buffer.buffer, nullptr, - buffer.acquire_fence, Status::Pending }); + buffer.acquire_fence, Status::Success }); } /* Clone the controls associated with the camera3 request. */ diff --git a/src/android/camera_request.h b/src/android/camera_request.h index 05dabf89..904886da 100644 --- a/src/android/camera_request.h +++ b/src/android/camera_request.h @@ -26,7 +26,6 @@ class Camera3RequestDescriptor { public: enum class Status { - Pending, Success, Error, }; @@ -43,7 +42,8 @@ public: const camera3_capture_request_t *camera3Request); ~Camera3RequestDescriptor(); - bool isPending() const { return status_ == Status::Pending; } + bool isPending() const { return !complete_; } + void completeDescriptor() { complete_ = true; } uint32_t frameNumber_ = 0; @@ -53,7 +53,8 @@ public: std::unique_ptr request_; std::unique_ptr resultMetadata_; - Status status_ = Status::Pending; + bool complete_ = false; + Status status_ = Status::Success; private: LIBCAMERA_DISABLE_COPY(Camera3RequestDescriptor)