From patchwork Mon Oct 25 20:38:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14295 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 2426BBF415 for ; Mon, 25 Oct 2021 20:38:46 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id DA84C6487B; Mon, 25 Oct 2021 22:38:45 +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="OpuzZ8c6"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 62F586486E for ; Mon, 25 Oct 2021 22:38:44 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.211]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D52B7119F; Mon, 25 Oct 2021 22:38:42 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1635194324; bh=Rfs5vxK5XN2WVTefTRqQ59wwjvesdClvoK29ikEUPZ4=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=OpuzZ8c6erYemdSX7LwbITsmt6SaroiDoCjpQZtI73BXPdJmaHr+Yj0Lc8l1hPzr+ 2yRw7nY3gtp8ZdAI4NhQa307dNy9qW6CFn/UiGzJFExQmkqECKvm3h/s1vSF9tg4q6 aEWDe9lQr3xIPQjXuFUyOrRhCGZNs6fsMPnp+014= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Tue, 26 Oct 2021 02:08:27 +0530 Message-Id: <20211025203833.122460-2-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211025203833.122460-1-umang.jain@ideasonboard.com> References: <20211025203833.122460-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 1/7] android: camera_stream: Replace post-processor nullptr check 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" Instead of checking postProcessor for nullptr, replace this check with an assertion that checks if the camera stream's type is not Type::Direct. Since it makes no sense to call CameraStream::process() on a Type::Direct camera stream. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart Reviewed-by: Hirokazu Honda Reviewed-by: Kieran Bingham --- src/android/camera_stream.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index f44a2717..5d991fe5 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -150,6 +150,8 @@ int CameraStream::process(const FrameBuffer &source, Camera3RequestDescriptor::StreamBuffer &dest, Camera3RequestDescriptor *request) { + ASSERT(type_ != Type::Direct); + /* Handle waiting on fences on the destination buffer. */ if (dest.fence != -1) { int ret = waitFence(dest.fence); @@ -163,9 +165,6 @@ int CameraStream::process(const FrameBuffer &source, dest.fence = -1; } - if (!postProcessor_) - return 0; - /* * \todo Buffer mapping and processing should be moved to a * separate thread. From patchwork Mon Oct 25 20:38:28 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14296 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 9E306BF415 for ; Mon, 25 Oct 2021 20:38:48 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6147D6486E; Mon, 25 Oct 2021 22:38:48 +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="neXbSFkZ"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B8BFF60125 for ; Mon, 25 Oct 2021 22:38:46 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.211]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 34664E0A; Mon, 25 Oct 2021 22:38:44 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1635194326; bh=B9+YjgtLiWLIJqVqRmov1SYv0g0IK79oU0QLperGiWw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=neXbSFkZEGGmu9q6Wsl5POguWhUjYTFqGcmxqswz3c/RhAdMZJDC1DbNBXfJj0Fir GH4ZTjXc8hZ/+6AkQrsZjlnwF0Usn4q27MuIXlwy8HdhSWBb6rXti/YFIMe2UGonLs O2h2YWabY0jEs7FMLruzBHxEqY6Fn8it7bFYimh4= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Tue, 26 Oct 2021 02:08:28 +0530 Message-Id: <20211025203833.122460-3-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211025203833.122460-1-umang.jain@ideasonboard.com> References: <20211025203833.122460-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 2/7] android: post_processor_jpeg: Replace encoder_ nullptr check 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" Instead of simply returning if encoder_ is nullptr, fail hard via an assertion. It is quite unlikely that encoder_ could only be null as a result of a fatal bug in the code, so be loud about the failure. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart Reviewed-by: Hirokazu Honda Reviewed-by: Kieran Bingham --- src/android/jpeg/post_processor_jpeg.cpp | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index 699576ef..49483836 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -102,9 +102,7 @@ int PostProcessorJpeg::process(const FrameBuffer &source, CameraBuffer *destination, Camera3RequestDescriptor *request) { - if (!encoder_) - return 0; - + ASSERT(encoder_); ASSERT(destination->numPlanes() == 1); const CameraMetadata &requestMetadata = request->settings_; From patchwork Mon Oct 25 20:38:29 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14297 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 0ADF8BF415 for ; Mon, 25 Oct 2021 20:38:50 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id BC0816487B; Mon, 25 Oct 2021 22:38:49 +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="Gyyp9EOm"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id EDFD560125 for ; Mon, 25 Oct 2021 22:38:48 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.211]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 979F9E0A; Mon, 25 Oct 2021 22:38:47 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1635194328; bh=i3xgVjRQSzYJcumu/ZRrhup36MHLEmP+pOYX6h/PmB8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=Gyyp9EOmIZD+3oTgh7Fae8n9y4cFOL8kPIa1BkOmQzFoXd+r/9YwH4R/3ZmaNQ+wf 1R03hyt1fMv6EszDDxeMrHUBfsnr9Sx8xTUTI6vxtb0TYbSLfFbfTxjXuyquPIC8hk NrzmST37FY4QXwJzluXOyFIza07XOC2QJY4J8qK8= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Tue, 26 Oct 2021 02:08:29 +0530 Message-Id: <20211025203833.122460-4-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211025203833.122460-1-umang.jain@ideasonboard.com> References: <20211025203833.122460-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 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. Also make sure the sequence of abortRequest() call happens in the same order at all places i.e. after its added to the descriptors_ queue. Fix one of the abortRequest() call accordingly. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart Reviewed-by: Hirokazu Honda --- src/android/camera_device.cpp | 29 ++++++++++++++--------------- src/android/camera_device.h | 1 + src/android/camera_request.cpp | 2 +- src/android/camera_request.h | 6 +++--- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 806b4090..bf9a2e69 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(); { MutexLocker descriptorsLock(descriptorsMutex_); descriptors_.push(std::move(descriptor)); } - - sendCaptureResults(); + abortRequest(rawDescriptor); + completeDescriptor(rawDescriptor); return 0; } @@ -1079,7 +1079,7 @@ void CameraDevice::requestComplete(Request *request) << request->status(); abortRequest(descriptor); - sendCaptureResults(); + completeDescriptor(descriptor); return; } @@ -1129,6 +1129,7 @@ void CameraDevice::requestComplete(Request *request) buffer.status = Camera3RequestDescriptor::Status::Error; notifyError(descriptor->frameNumber_, stream->camera3Stream(), CAMERA3_MSG_ERROR_BUFFER); + descriptor->status_ = Camera3RequestDescriptor::Status::Error; continue; } @@ -1145,27 +1146,27 @@ void CameraDevice::requestComplete(Request *request) buffer.status = Camera3RequestDescriptor::Status::Error; notifyError(descriptor->frameNumber_, stream->camera3Stream(), CAMERA3_MSG_ERROR_BUFFER); + descriptor->status_ = Camera3RequestDescriptor::Status::Error; } } - descriptor->status_ = Camera3RequestDescriptor::Status::Success; + completeDescriptor(descriptor); +} + +void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor) +{ + MutexLocker lock(descriptorsMutex_); + descriptor->complete_ = true; + 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 +1202,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..4d80ef32 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,7 @@ public: const camera3_capture_request_t *camera3Request); ~Camera3RequestDescriptor(); - bool isPending() const { return status_ == Status::Pending; } + bool isPending() const { return !complete_; } uint32_t frameNumber_ = 0; @@ -53,7 +52,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) From patchwork Mon Oct 25 20:38:30 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14298 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 91B24BF415 for ; Mon, 25 Oct 2021 20:38:53 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 30E9E6487B; Mon, 25 Oct 2021 22:38:53 +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="SqAIO8KI"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 409B260125 for ; Mon, 25 Oct 2021 22:38:51 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.211]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E9C34E0A; Mon, 25 Oct 2021 22:38:49 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1635194331; bh=fo3QLtg0GBlyoGCezLj8ctOiprKx3MPFknrzCVPWKVM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=SqAIO8KIi5agCTE2HTz1Y1TwdBCUcOy8gZUfCfi1O5BFHn/5WGLiLD/cs2uj7ZouI OnUmHriI3xN9aR3RJBkqqtuku+3J8xvKyf/0W26BPPftecQIB7pAy20bU52mPQ8RZe /hG9sFHhSmkY+TzRxiXVDKBFrAHdJ3G7qq2toEpY= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Tue, 26 Oct 2021 02:08:30 +0530 Message-Id: <20211025203833.122460-5-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211025203833.122460-1-umang.jain@ideasonboard.com> References: <20211025203833.122460-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 4/7] android: post_processor: Consolidate contextual information 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" Save and provide the context for post-processor of a camera stream via Camera3RequestDescriptor::StreamBuffer. We extend the structure to include source and destination buffers for the post processor, along with CameraStream::Type::Internal buffer pointer (if any). In addition to that, a back pointer to Camera3RequestDescriptor is convienient to get access to overall descriptor (status, metadata settings etc.) Also, migrate CameraStream::process() and PostProcessor::process() signature to use Camera3RequestDescriptor::StreamBuffer only. This will be helpful when we move to async post-processing in subsequent commits. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart Reviewed-by: Hirokazu Honda --- src/android/camera_device.cpp | 13 ++++++++----- src/android/camera_request.cpp | 5 ++++- src/android/camera_request.h | 5 +++++ src/android/camera_stream.cpp | 23 +++++++++++------------ src/android/camera_stream.h | 4 +--- src/android/jpeg/post_processor_jpeg.cpp | 12 +++++++----- src/android/jpeg/post_processor_jpeg.h | 4 +--- src/android/post_processor.h | 7 ++----- src/android/yuv/post_processor_yuv.cpp | 7 ++++--- src/android/yuv/post_processor_yuv.h | 4 +--- 10 files changed, 44 insertions(+), 40 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index bf9a2e69..9155728a 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -953,6 +953,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques * once it has been processed. */ frameBuffer = cameraStream->getBuffer(); + buffer.internalBuffer = frameBuffer; LOG(HAL, Debug) << ss.str() << " (internal)"; break; } @@ -1133,14 +1134,16 @@ void CameraDevice::requestComplete(Request *request) continue; } - int ret = stream->process(*src, buffer, descriptor); + buffer.srcBuffer = src; + + int ret = stream->process(&buffer); /* - * Return the FrameBuffer to the CameraStream now that we're - * done processing it. + * If the framebuffer is internal to CameraStream return it back + * now that we're done processing it. */ - if (stream->type() == CameraStream::Type::Internal) - stream->putBuffer(src); + if (buffer.internalBuffer) + stream->putBuffer(buffer.internalBuffer); if (ret) { buffer.status = Camera3RequestDescriptor::Status::Error; diff --git a/src/android/camera_request.cpp b/src/android/camera_request.cpp index 16cf266f..5bac1b8f 100644 --- a/src/android/camera_request.cpp +++ b/src/android/camera_request.cpp @@ -9,6 +9,8 @@ #include +#include "camera_buffer.h" + using namespace libcamera; /* @@ -36,7 +38,8 @@ Camera3RequestDescriptor::Camera3RequestDescriptor( static_cast(buffer.stream->priv); buffers_.push_back({ stream, buffer.buffer, nullptr, - buffer.acquire_fence, Status::Success }); + buffer.acquire_fence, Status::Success, + nullptr, nullptr, nullptr, this }); } /* Clone the controls associated with the camera3 request. */ diff --git a/src/android/camera_request.h b/src/android/camera_request.h index 4d80ef32..c7fda00d 100644 --- a/src/android/camera_request.h +++ b/src/android/camera_request.h @@ -21,6 +21,7 @@ #include "camera_worker.h" class CameraStream; +class CameraBuffer; class Camera3RequestDescriptor { @@ -36,6 +37,10 @@ public: std::unique_ptr frameBuffer; int fence; Status status; + libcamera::FrameBuffer *internalBuffer; + const libcamera::FrameBuffer *srcBuffer; + std::unique_ptr dstBuffer; + Camera3RequestDescriptor *request; }; Camera3RequestDescriptor(libcamera::Camera *camera, diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index 5d991fe5..282b19b0 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -146,23 +146,21 @@ int CameraStream::waitFence(int fence) return -errno; } -int CameraStream::process(const FrameBuffer &source, - Camera3RequestDescriptor::StreamBuffer &dest, - Camera3RequestDescriptor *request) +int CameraStream::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) { ASSERT(type_ != Type::Direct); /* Handle waiting on fences on the destination buffer. */ - if (dest.fence != -1) { - int ret = waitFence(dest.fence); + if (streamBuffer->fence != -1) { + int ret = waitFence(streamBuffer->fence); if (ret < 0) { LOG(HAL, Error) << "Failed waiting for fence: " - << dest.fence << ": " << strerror(-ret); + << streamBuffer->fence << ": " << strerror(-ret); return ret; } - ::close(dest.fence); - dest.fence = -1; + ::close(streamBuffer->fence); + streamBuffer->fence = -1; } /* @@ -170,14 +168,15 @@ int CameraStream::process(const FrameBuffer &source, * separate thread. */ const StreamConfiguration &output = configuration(); - CameraBuffer destBuffer(*dest.camera3Buffer, output.pixelFormat, - output.size, PROT_READ | PROT_WRITE); - if (!destBuffer.isValid()) { + streamBuffer->dstBuffer = std::make_unique( + *streamBuffer->camera3Buffer, output.pixelFormat, output.size, + PROT_READ | PROT_WRITE); + if (!streamBuffer->dstBuffer->isValid()) { LOG(HAL, Error) << "Failed to create destination buffer"; return -EINVAL; } - return postProcessor_->process(source, &destBuffer, request); + return postProcessor_->process(streamBuffer); } FrameBuffer *CameraStream::getBuffer() diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h index f242336e..e74a9a3b 100644 --- a/src/android/camera_stream.h +++ b/src/android/camera_stream.h @@ -121,9 +121,7 @@ public: libcamera::Stream *stream() const; int configure(); - int process(const libcamera::FrameBuffer &source, - Camera3RequestDescriptor::StreamBuffer &dest, - Camera3RequestDescriptor *request); + int process(Camera3RequestDescriptor::StreamBuffer *streamBuffer); libcamera::FrameBuffer *getBuffer(); void putBuffer(libcamera::FrameBuffer *buffer); diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index 49483836..240e29f6 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -98,15 +98,17 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source, } } -int PostProcessorJpeg::process(const FrameBuffer &source, - CameraBuffer *destination, - Camera3RequestDescriptor *request) +int PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) { ASSERT(encoder_); + + const FrameBuffer &source = *streamBuffer->srcBuffer; + CameraBuffer *destination = streamBuffer->dstBuffer.get(); + ASSERT(destination->numPlanes() == 1); - const CameraMetadata &requestMetadata = request->settings_; - CameraMetadata *resultMetadata = request->resultMetadata_.get(); + const CameraMetadata &requestMetadata = streamBuffer->request->settings_; + CameraMetadata *resultMetadata = streamBuffer->request->resultMetadata_.get(); camera_metadata_ro_entry_t entry; int ret; diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h index 0184d77e..92385548 100644 --- a/src/android/jpeg/post_processor_jpeg.h +++ b/src/android/jpeg/post_processor_jpeg.h @@ -22,9 +22,7 @@ public: int configure(const libcamera::StreamConfiguration &incfg, const libcamera::StreamConfiguration &outcfg) override; - int process(const libcamera::FrameBuffer &source, - CameraBuffer *destination, - Camera3RequestDescriptor *request) override; + int process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) override; private: void generateThumbnail(const libcamera::FrameBuffer &source, diff --git a/src/android/post_processor.h b/src/android/post_processor.h index 27eaef88..128161c8 100644 --- a/src/android/post_processor.h +++ b/src/android/post_processor.h @@ -11,8 +11,7 @@ #include #include "camera_buffer.h" - -class Camera3RequestDescriptor; +#include "camera_request.h" class PostProcessor { @@ -21,9 +20,7 @@ public: virtual int configure(const libcamera::StreamConfiguration &inCfg, const libcamera::StreamConfiguration &outCfg) = 0; - virtual int process(const libcamera::FrameBuffer &source, - CameraBuffer *destination, - Camera3RequestDescriptor *request) = 0; + virtual int process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) = 0; }; #endif /* __ANDROID_POST_PROCESSOR_H__ */ diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp index 8110a1f1..70385ab3 100644 --- a/src/android/yuv/post_processor_yuv.cpp +++ b/src/android/yuv/post_processor_yuv.cpp @@ -49,10 +49,11 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg, return 0; } -int PostProcessorYuv::process(const FrameBuffer &source, - CameraBuffer *destination, - [[maybe_unused]] Camera3RequestDescriptor *request) +int PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) { + const FrameBuffer &source = *streamBuffer->srcBuffer; + CameraBuffer *destination = streamBuffer->dstBuffer.get(); + if (!isValidBuffers(source, *destination)) return -EINVAL; diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h index a4e0ff5d..5954e11b 100644 --- a/src/android/yuv/post_processor_yuv.h +++ b/src/android/yuv/post_processor_yuv.h @@ -18,9 +18,7 @@ public: int configure(const libcamera::StreamConfiguration &incfg, const libcamera::StreamConfiguration &outcfg) override; - int process(const libcamera::FrameBuffer &source, - CameraBuffer *destination, - Camera3RequestDescriptor *request) override; + int process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) override; private: bool isValidBuffers(const libcamera::FrameBuffer &source, From patchwork Mon Oct 25 20:38:31 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14299 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 80CDDBF415 for ; Mon, 25 Oct 2021 20:38:55 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4428964881; Mon, 25 Oct 2021 22:38:55 +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="LDO9NmYV"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E3EA36486E for ; Mon, 25 Oct 2021 22:38:52 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.211]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 095F6119F; Mon, 25 Oct 2021 22:38:51 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1635194332; bh=GsUusicoSbQ0ot/rMEyORmc+Xe4rOS9e+ed3WeVBwqs=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=LDO9NmYVpOZXL9O0SRfOml5d/B3NV6zKJJLcccesVw5BYv0cqhtTgnqzewHdPv0Jj ewV/saSbyeZD39EF530gdBSgRZWeyV2Kt8BDYdoOEU3GITnVLmlDqAX6YbS++JS8QN 9DOS5jRz9ZLakmktflS2A8ePMcHnq7IqA2F8BYpw= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Tue, 26 Oct 2021 02:08:31 +0530 Message-Id: <20211025203833.122460-6-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211025203833.122460-1-umang.jain@ideasonboard.com> References: <20211025203833.122460-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 5/7] android: Track and notify post processing of streams 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" Notify that the post processing for a request has been completed, via a signal. The signal is emitted with a context pointer along with status of the buffer. The function CameraDevice::streamProcessingComplete() will finally set the status on the request descriptor and complete the descriptor if all the streams requiring post processing are completed. If buffer status obtained is in error state, notify the status to the framework and set the overall error status on the descriptor via setBufferStatus(). We need to track the number of streams requiring post-processing per Camera3RequestDescriptor (i.e. per capture request). Introduce a std::map<> to track the post-processing of streams. The nodes are dropped from the map when a particular stream post processing is completed (or on error paths). A std::map is selected for tracking post-processing requests, since we will move post-processing to be asynchronous in subsequent commits. A vector or queue will not be suitable then as the sequential order of post-processing completion of various requests won't be guaranteed then. A streamsProcessMutex_ has been introduced here as well, which will be applicable to guard access to descriptor's pendingStreamsToProcess_ when post-processing is moved to be asynchronous in subsequent commits. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart Reviewed-by: Hirokazu Honda --- src/android/camera_device.cpp | 87 +++++++++++++++++------- src/android/camera_device.h | 4 ++ src/android/camera_request.h | 6 ++ src/android/camera_stream.cpp | 15 ++++ src/android/jpeg/post_processor_jpeg.cpp | 2 + src/android/post_processor.h | 9 +++ src/android/yuv/post_processor_yuv.cpp | 8 ++- 7 files changed, 106 insertions(+), 25 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 9155728a..3ded0f7e 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -926,6 +926,9 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques * Request. */ LOG(HAL, Debug) << ss.str() << " (mapped)"; + + descriptor->pendingStreamsToProcess_.insert( + { cameraStream, &buffer }); continue; case CameraStream::Type::Direct: @@ -955,6 +958,9 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques frameBuffer = cameraStream->getBuffer(); buffer.internalBuffer = frameBuffer; LOG(HAL, Debug) << ss.str() << " (internal)"; + + descriptor->pendingStreamsToProcess_.insert( + { cameraStream, &buffer }); break; } @@ -1118,42 +1124,42 @@ void CameraDevice::requestComplete(Request *request) } /* Handle post-processing. */ - for (auto &buffer : descriptor->buffers_) { - CameraStream *stream = buffer.stream; - - if (stream->type() == CameraStream::Type::Direct) - continue; + /* + * \todo Protect the loop below with streamProcessMutex_ when post + * processor runs asynchronously. + */ + auto iter = descriptor->pendingStreamsToProcess_.begin(); + while (iter != descriptor->pendingStreamsToProcess_.end()) { + CameraStream *stream = iter->first; + Camera3RequestDescriptor::StreamBuffer *buffer = iter->second; FrameBuffer *src = request->findBuffer(stream->stream()); if (!src) { LOG(HAL, Error) << "Failed to find a source stream buffer"; - buffer.status = Camera3RequestDescriptor::Status::Error; - notifyError(descriptor->frameNumber_, stream->camera3Stream(), - CAMERA3_MSG_ERROR_BUFFER); - descriptor->status_ = Camera3RequestDescriptor::Status::Error; + setBufferStatus(*buffer, Camera3RequestDescriptor::Status::Error); + iter = descriptor->pendingStreamsToProcess_.erase(iter); continue; } - buffer.srcBuffer = src; - - int ret = stream->process(&buffer); - - /* - * If the framebuffer is internal to CameraStream return it back - * now that we're done processing it. - */ - if (buffer.internalBuffer) - stream->putBuffer(buffer.internalBuffer); + buffer->srcBuffer = src; + ++iter; + int ret = stream->process(buffer); if (ret) { - buffer.status = Camera3RequestDescriptor::Status::Error; - notifyError(descriptor->frameNumber_, stream->camera3Stream(), - CAMERA3_MSG_ERROR_BUFFER); - descriptor->status_ = Camera3RequestDescriptor::Status::Error; + setBufferStatus(*buffer, Camera3RequestDescriptor::Status::Error); + descriptor->pendingStreamsToProcess_.erase(stream); + + /* + * If the framebuffer is internal to CameraStream return + * it back now that we're done processing it. + */ + if (buffer->internalBuffer) + stream->putBuffer(buffer->internalBuffer); } } - completeDescriptor(descriptor); + if (descriptor->pendingStreamsToProcess_.empty()) + completeDescriptor(descriptor); } void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor) @@ -1208,6 +1214,39 @@ void CameraDevice::sendCaptureResults() } } +void CameraDevice::setBufferStatus(Camera3RequestDescriptor::StreamBuffer &streamBuffer, + Camera3RequestDescriptor::Status status) +{ + streamBuffer.status = status; + if (status != Camera3RequestDescriptor::Status::Success) { + notifyError(streamBuffer.request->frameNumber_, + streamBuffer.stream->camera3Stream(), + CAMERA3_MSG_ERROR_BUFFER); + + /* Also set error status on entire request descriptor. */ + streamBuffer.request->status_ = + Camera3RequestDescriptor::Status::Error; + } +} + +void CameraDevice::streamProcessingComplete(Camera3RequestDescriptor::StreamBuffer *streamBuffer, + Camera3RequestDescriptor::Status status) +{ + setBufferStatus(*streamBuffer, status); + + /* + * If the framebuffer is internal to CameraStream return it back now + * that we're done processing it. + */ + if (streamBuffer->internalBuffer) + streamBuffer->stream->putBuffer(streamBuffer->internalBuffer); + + Camera3RequestDescriptor *request = streamBuffer->request; + MutexLocker locker(request->streamsProcessMutex_); + + request->pendingStreamsToProcess_.erase(streamBuffer->stream); +} + std::string CameraDevice::logPrefix() const { return "'" + camera_->id() + "'"; diff --git a/src/android/camera_device.h b/src/android/camera_device.h index e544f2bd..2a414020 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -66,6 +66,8 @@ public: int configureStreams(camera3_stream_configuration_t *stream_list); int processCaptureRequest(camera3_capture_request_t *request); void requestComplete(libcamera::Request *request); + void streamProcessingComplete(Camera3RequestDescriptor::StreamBuffer *bufferStream, + Camera3RequestDescriptor::Status status); protected: std::string logPrefix() const override; @@ -95,6 +97,8 @@ private: int processControls(Camera3RequestDescriptor *descriptor); void completeDescriptor(Camera3RequestDescriptor *descriptor); void sendCaptureResults(); + void setBufferStatus(Camera3RequestDescriptor::StreamBuffer &buffer, + Camera3RequestDescriptor::Status status); std::unique_ptr getResultMetadata( const Camera3RequestDescriptor &descriptor) const; diff --git a/src/android/camera_request.h b/src/android/camera_request.h index c7fda00d..c28f7942 100644 --- a/src/android/camera_request.h +++ b/src/android/camera_request.h @@ -7,7 +7,9 @@ #ifndef __ANDROID_CAMERA_REQUEST_H__ #define __ANDROID_CAMERA_REQUEST_H__ +#include #include +#include #include #include @@ -43,6 +45,10 @@ public: Camera3RequestDescriptor *request; }; + /* Keeps track of streams requiring post-processing. */ + std::map pendingStreamsToProcess_; + std::mutex streamsProcessMutex_; + Camera3RequestDescriptor(libcamera::Camera *camera, const camera3_capture_request_t *camera3Request); ~Camera3RequestDescriptor(); diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index 282b19b0..dac216d6 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -22,6 +22,7 @@ #include "camera_capabilities.h" #include "camera_device.h" #include "camera_metadata.h" +#include "post_processor.h" using namespace libcamera; @@ -97,6 +98,20 @@ int CameraStream::configure() int ret = postProcessor_->configure(configuration(), output); if (ret) return ret; + + postProcessor_->processComplete.connect( + this, [&](Camera3RequestDescriptor::StreamBuffer *streamBuffer, + PostProcessor::Status status) { + Camera3RequestDescriptor::Status bufferStatus; + + if (status == PostProcessor::Status::Success) + bufferStatus = Camera3RequestDescriptor::Status::Success; + else + bufferStatus = Camera3RequestDescriptor::Status::Error; + + cameraDevice_->streamProcessingComplete(streamBuffer, + bufferStatus); + }); } if (type_ == Type::Internal) { diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index 240e29f6..5e8c61fc 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -198,6 +198,7 @@ int PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuf exif.data(), quality); if (jpeg_size < 0) { LOG(JPEG, Error) << "Failed to encode stream image"; + processComplete.emit(streamBuffer, PostProcessor::Status::Error); return jpeg_size; } @@ -211,6 +212,7 @@ int PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuf /* Update the JPEG result Metadata. */ resultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size); + processComplete.emit(streamBuffer, PostProcessor::Status::Success); return 0; } diff --git a/src/android/post_processor.h b/src/android/post_processor.h index 128161c8..4ac74fcf 100644 --- a/src/android/post_processor.h +++ b/src/android/post_processor.h @@ -7,6 +7,8 @@ #ifndef __ANDROID_POST_PROCESSOR_H__ #define __ANDROID_POST_PROCESSOR_H__ +#include + #include #include @@ -16,11 +18,18 @@ class PostProcessor { public: + enum class Status { + Error, + Success + }; + virtual ~PostProcessor() = default; virtual int configure(const libcamera::StreamConfiguration &inCfg, const libcamera::StreamConfiguration &outCfg) = 0; virtual int process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) = 0; + + libcamera::Signal processComplete; }; #endif /* __ANDROID_POST_PROCESSOR_H__ */ diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp index 70385ab3..05c4f1cf 100644 --- a/src/android/yuv/post_processor_yuv.cpp +++ b/src/android/yuv/post_processor_yuv.cpp @@ -54,12 +54,15 @@ int PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuff const FrameBuffer &source = *streamBuffer->srcBuffer; CameraBuffer *destination = streamBuffer->dstBuffer.get(); - if (!isValidBuffers(source, *destination)) + if (!isValidBuffers(source, *destination)) { + processComplete.emit(streamBuffer, PostProcessor::Status::Error); return -EINVAL; + } const MappedFrameBuffer sourceMapped(&source, MappedFrameBuffer::MapFlag::Read); if (!sourceMapped.isValid()) { LOG(YUV, Error) << "Failed to mmap camera frame buffer"; + processComplete.emit(streamBuffer, PostProcessor::Status::Error); return -EINVAL; } @@ -77,9 +80,12 @@ int PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuff libyuv::FilterMode::kFilterBilinear); if (ret) { LOG(YUV, Error) << "Failed NV12 scaling: " << ret; + processComplete.emit(streamBuffer, PostProcessor::Status::Error); return -EINVAL; } + processComplete.emit(streamBuffer, PostProcessor::Status::Success); + return 0; } From patchwork Mon Oct 25 20:38:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14300 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 E088DBF415 for ; Mon, 25 Oct 2021 20:38:56 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9D20864874; Mon, 25 Oct 2021 22:38:56 +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="CYhR1tzn"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id F26906487E for ; Mon, 25 Oct 2021 22:38:54 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.211]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BB255119F; Mon, 25 Oct 2021 22:38:53 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1635194334; bh=3nkEn2L9FkJicy6yOyHHIRt2i804WchpsL4UOJkoPlI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=CYhR1tzn54g3zy+7Cuh+lCGMJmxZqHJZuMI8rNVAComel3J5AyfqbzTTx+D9DjkZ9 IGynDZYSD+Q31iOTbR1/8tMDOVt6ICZEhBSHhWQhFHKoHD7GC9Q5k7lKNbHIfDcWon eiP74LCBfpRpNy426dCIqqNTUPLG1/xXRiHWEmB0= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Tue, 26 Oct 2021 02:08:32 +0530 Message-Id: <20211025203833.122460-7-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211025203833.122460-1-umang.jain@ideasonboard.com> References: <20211025203833.122460-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 6/7] android: post_processor: Drop return value for process() 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" PostProcessor::process() is invoked by CameraStream class in case any post-processing is required for the camera stream. The failure or success is checked via the value returned by CameraStream::process(). Now that the post-processor notifies about the post-processing completion operation, we can drop the return value of PostProcessor::process(). The status of post-processing is passed to CameraDevice::streamProcessingComplete() by the PostProcessor::processComplete signal's slot. Signed-off-by: Umang Jain Reviewed-by: Laurent Pinchart --- src/android/camera_stream.cpp | 4 +++- src/android/jpeg/post_processor_jpeg.cpp | 6 ++---- src/android/jpeg/post_processor_jpeg.h | 2 +- src/android/post_processor.h | 2 +- src/android/yuv/post_processor_yuv.cpp | 10 ++++------ src/android/yuv/post_processor_yuv.h | 2 +- 6 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index dac216d6..fed99022 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -191,7 +191,9 @@ int CameraStream::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) return -EINVAL; } - return postProcessor_->process(streamBuffer); + postProcessor_->process(streamBuffer); + + return 0; } FrameBuffer *CameraStream::getBuffer() diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index 5e8c61fc..d72ebc3c 100644 --- a/src/android/jpeg/post_processor_jpeg.cpp +++ b/src/android/jpeg/post_processor_jpeg.cpp @@ -98,7 +98,7 @@ void PostProcessorJpeg::generateThumbnail(const FrameBuffer &source, } } -int PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) +void PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) { ASSERT(encoder_); @@ -199,7 +199,7 @@ int PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuf if (jpeg_size < 0) { LOG(JPEG, Error) << "Failed to encode stream image"; processComplete.emit(streamBuffer, PostProcessor::Status::Error); - return jpeg_size; + return; } /* Fill in the JPEG blob header. */ @@ -213,6 +213,4 @@ int PostProcessorJpeg::process(Camera3RequestDescriptor::StreamBuffer *streamBuf /* Update the JPEG result Metadata. */ resultMetadata->addEntry(ANDROID_JPEG_SIZE, jpeg_size); processComplete.emit(streamBuffer, PostProcessor::Status::Success); - - return 0; } diff --git a/src/android/jpeg/post_processor_jpeg.h b/src/android/jpeg/post_processor_jpeg.h index 92385548..43fcbe60 100644 --- a/src/android/jpeg/post_processor_jpeg.h +++ b/src/android/jpeg/post_processor_jpeg.h @@ -22,7 +22,7 @@ public: int configure(const libcamera::StreamConfiguration &incfg, const libcamera::StreamConfiguration &outcfg) override; - int process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) override; + void process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) override; private: void generateThumbnail(const libcamera::FrameBuffer &source, diff --git a/src/android/post_processor.h b/src/android/post_processor.h index 4ac74fcf..5ec71c93 100644 --- a/src/android/post_processor.h +++ b/src/android/post_processor.h @@ -27,7 +27,7 @@ public: virtual int configure(const libcamera::StreamConfiguration &inCfg, const libcamera::StreamConfiguration &outCfg) = 0; - virtual int process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) = 0; + virtual void process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) = 0; libcamera::Signal processComplete; }; diff --git a/src/android/yuv/post_processor_yuv.cpp b/src/android/yuv/post_processor_yuv.cpp index 05c4f1cf..9f883924 100644 --- a/src/android/yuv/post_processor_yuv.cpp +++ b/src/android/yuv/post_processor_yuv.cpp @@ -49,21 +49,21 @@ int PostProcessorYuv::configure(const StreamConfiguration &inCfg, return 0; } -int PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) +void PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) { const FrameBuffer &source = *streamBuffer->srcBuffer; CameraBuffer *destination = streamBuffer->dstBuffer.get(); if (!isValidBuffers(source, *destination)) { processComplete.emit(streamBuffer, PostProcessor::Status::Error); - return -EINVAL; + return; } const MappedFrameBuffer sourceMapped(&source, MappedFrameBuffer::MapFlag::Read); if (!sourceMapped.isValid()) { LOG(YUV, Error) << "Failed to mmap camera frame buffer"; processComplete.emit(streamBuffer, PostProcessor::Status::Error); - return -EINVAL; + return; } int ret = libyuv::NV12Scale(sourceMapped.planes()[0].data(), @@ -81,12 +81,10 @@ int PostProcessorYuv::process(Camera3RequestDescriptor::StreamBuffer *streamBuff if (ret) { LOG(YUV, Error) << "Failed NV12 scaling: " << ret; processComplete.emit(streamBuffer, PostProcessor::Status::Error); - return -EINVAL; + return; } processComplete.emit(streamBuffer, PostProcessor::Status::Success); - - return 0; } bool PostProcessorYuv::isValidBuffers(const FrameBuffer &source, diff --git a/src/android/yuv/post_processor_yuv.h b/src/android/yuv/post_processor_yuv.h index 5954e11b..39ec7994 100644 --- a/src/android/yuv/post_processor_yuv.h +++ b/src/android/yuv/post_processor_yuv.h @@ -18,7 +18,7 @@ public: int configure(const libcamera::StreamConfiguration &incfg, const libcamera::StreamConfiguration &outcfg) override; - int process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) override; + void process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) override; private: bool isValidBuffers(const libcamera::FrameBuffer &source, From patchwork Mon Oct 25 20:38:33 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14301 X-Patchwork-Delegate: umang.jain@ideasonboard.com 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 7CF3FBF415 for ; Mon, 25 Oct 2021 20:38:58 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3BD2A6487B; Mon, 25 Oct 2021 22:38:58 +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="vOhxa+jE"; 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 F1E4B60125 for ; Mon, 25 Oct 2021 22:38:56 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.211]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A8FE41203; Mon, 25 Oct 2021 22:38:55 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1635194336; bh=8Xjppdn2IRr5eqh64IndAlYkC5Gi/jQDD4lCPtnNBpE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vOhxa+jE6+kur6sb3NGP/mwk6UJ+AE1S5Lw2fF7aFVv/X+Dpeqfgnt1+kGlG0JYVr gqv2v7L4Bi88FNqJBijtu9mw6joFOg5/nVq8oaGQShWJt2kZfnTcKvzfwQ+h/ocHkP QnvBP7xk78tHZVj9yDW3kFnluQ6nTaEXIgWajcEU= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Tue, 26 Oct 2021 02:08:33 +0530 Message-Id: <20211025203833.122460-8-umang.jain@ideasonboard.com> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20211025203833.122460-1-umang.jain@ideasonboard.com> References: <20211025203833.122460-1-umang.jain@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v7 7/7] android: post_processor: Make post processing async 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" Introduce a dedicated worker class derived from libcamera::Thread. The worker class maintains a queue for post-processing requests and waits for a post-processing request to become available. It will process them as per FIFO before de-queuing it from the queue. The entire post-processing handling iteration is locked under streamProcessMutex_ which helps us to queue all the post-processing request at once, before any of the post-processing completion slot (streamProcessingComplete()) is allowed to run for post-processing requests completing in parallel. This helps us to manage both synchronous and asynchronous errors encountered during the entire post processing operation. Since a post-processing operation can even complete after CameraDevice::requestComplete() has returned, we need to check and complete the descriptor from streamProcessingComplete() running in the PostProcessorWorker's thread. This patch also implements a flush() for the PostProcessorWorker class which is responsible to purge post-processing requests queued up while a camera is stopping/flushing. It is hooked with CameraStream::flush(), which isn't used currently but will be used when we handle flush/stop scenarios in greater detail subsequently (in a different patchset). The libcamera request completion handler CameraDevice::requestComplete() assumes that the request that has just completed is at the front of the queue. Now that the post-processor runs asynchronously, this isn't true anymore, a request being post-processed will stay in the queue and a new libcamera request may complete. Remove that assumption, and use the request cookie to obtain the Camera3RequestDescriptor. Signed-off-by: Umang Jain Signed-off-by: Laurent Pinchart Reviewed-by: Hirokazu Honda --- src/android/camera_device.cpp | 44 ++++++--------- src/android/camera_stream.cpp | 101 ++++++++++++++++++++++++++++++++-- src/android/camera_stream.h | 37 +++++++++++++ 3 files changed, 151 insertions(+), 31 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 3ded0f7e..53ebe0ea 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -1027,29 +1027,8 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques void CameraDevice::requestComplete(Request *request) { - Camera3RequestDescriptor *descriptor; - { - MutexLocker descriptorsLock(descriptorsMutex_); - ASSERT(!descriptors_.empty()); - descriptor = descriptors_.front().get(); - } - - if (descriptor->request_->cookie() != request->cookie()) { - /* - * \todo Clarify if the Camera has to be closed on - * ERROR_DEVICE. - */ - LOG(HAL, Error) - << "Out-of-order completion for request " - << utils::hex(request->cookie()); - - MutexLocker descriptorsLock(descriptorsMutex_); - descriptors_.pop(); - - notifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE); - - return; - } + Camera3RequestDescriptor *descriptor = + reinterpret_cast(request->cookie()); /* * Prepare the capture result for the Android camera stack. @@ -1124,9 +1103,13 @@ void CameraDevice::requestComplete(Request *request) } /* Handle post-processing. */ + MutexLocker locker(descriptor->streamsProcessMutex_); + /* - * \todo Protect the loop below with streamProcessMutex_ when post - * processor runs asynchronously. + * Queue all the post-processing streams request at once. The completion + * slot streamProcessingComplete() can only execute when we are out + * this critical section. This helps to handle synchronous errors here + * itself. */ auto iter = descriptor->pendingStreamsToProcess_.begin(); while (iter != descriptor->pendingStreamsToProcess_.end()) { @@ -1158,8 +1141,10 @@ void CameraDevice::requestComplete(Request *request) } } - if (descriptor->pendingStreamsToProcess_.empty()) + if (descriptor->pendingStreamsToProcess_.empty()) { + locker.unlock(); completeDescriptor(descriptor); + } } void CameraDevice::completeDescriptor(Camera3RequestDescriptor *descriptor) @@ -1245,6 +1230,13 @@ void CameraDevice::streamProcessingComplete(Camera3RequestDescriptor::StreamBuff MutexLocker locker(request->streamsProcessMutex_); request->pendingStreamsToProcess_.erase(streamBuffer->stream); + + if (!request->pendingStreamsToProcess_.empty()) + return; + + locker.unlock(); + + completeDescriptor(streamBuffer->request); } std::string CameraDevice::logPrefix() const diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index fed99022..9023c13c 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -99,6 +99,7 @@ int CameraStream::configure() if (ret) return ret; + worker_ = std::make_unique(postProcessor_.get()); postProcessor_->processComplete.connect( this, [&](Camera3RequestDescriptor::StreamBuffer *streamBuffer, PostProcessor::Status status) { @@ -112,6 +113,8 @@ int CameraStream::configure() cameraDevice_->streamProcessingComplete(streamBuffer, bufferStatus); }); + + worker_->start(); } if (type_ == Type::Internal) { @@ -178,10 +181,6 @@ int CameraStream::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) streamBuffer->fence = -1; } - /* - * \todo Buffer mapping and processing should be moved to a - * separate thread. - */ const StreamConfiguration &output = configuration(); streamBuffer->dstBuffer = std::make_unique( *streamBuffer->camera3Buffer, output.pixelFormat, output.size, @@ -191,11 +190,19 @@ int CameraStream::process(Camera3RequestDescriptor::StreamBuffer *streamBuffer) return -EINVAL; } - postProcessor_->process(streamBuffer); + worker_->queueRequest(streamBuffer); return 0; } +void CameraStream::flush() +{ + if (!postProcessor_) + return; + + worker_->flush(); +} + FrameBuffer *CameraStream::getBuffer() { if (!allocator_) @@ -223,3 +230,87 @@ void CameraStream::putBuffer(FrameBuffer *buffer) buffers_.push_back(buffer); } + +CameraStream::PostProcessorWorker::PostProcessorWorker(PostProcessor *postProcessor) + : postProcessor_(postProcessor) +{ +} + +CameraStream::PostProcessorWorker::~PostProcessorWorker() +{ + { + libcamera::MutexLocker lock(mutex_); + state_ = State::Stopped; + } + + cv_.notify_one(); + wait(); +} + +void CameraStream::PostProcessorWorker::start() +{ + { + libcamera::MutexLocker lock(mutex_); + ASSERT(state_ != State::Running); + state_ = State::Running; + } + + Thread::start(); +} + +void CameraStream::PostProcessorWorker::queueRequest(Camera3RequestDescriptor::StreamBuffer *dest) +{ + { + MutexLocker lock(mutex_); + ASSERT(state_ == State::Running); + requests_.push(dest); + } + + cv_.notify_one(); +} + +void CameraStream::PostProcessorWorker::run() +{ + MutexLocker locker(mutex_); + + while (1) { + cv_.wait(locker, [&] { + return state_ != State::Running || !requests_.empty(); + }); + + if (state_ != State::Running) + break; + + Camera3RequestDescriptor::StreamBuffer *streamBuffer = requests_.front(); + requests_.pop(); + locker.unlock(); + + postProcessor_->process(streamBuffer); + + locker.lock(); + } + + if (state_ == State::Flushing) { + std::queue requests = + std::move(requests_); + locker.unlock(); + + while (!requests.empty()) { + postProcessor_->processComplete.emit( + requests.front(), PostProcessor::Status::Error); + requests.pop(); + } + + locker.lock(); + state_ = State::Stopped; + } +} + +void CameraStream::PostProcessorWorker::flush() +{ + libcamera::MutexLocker lock(mutex_); + state_ = State::Flushing; + lock.unlock(); + + cv_.notify_one(); +} diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h index e74a9a3b..1588938a 100644 --- a/src/android/camera_stream.h +++ b/src/android/camera_stream.h @@ -7,12 +7,16 @@ #ifndef __ANDROID_CAMERA_STREAM_H__ #define __ANDROID_CAMERA_STREAM_H__ +#include #include #include +#include #include #include +#include + #include #include #include @@ -20,6 +24,7 @@ #include #include "camera_request.h" +#include "post_processor.h" class CameraDevice; class PostProcessor; @@ -124,8 +129,38 @@ public: int process(Camera3RequestDescriptor::StreamBuffer *streamBuffer); libcamera::FrameBuffer *getBuffer(); void putBuffer(libcamera::FrameBuffer *buffer); + void flush(); private: + class PostProcessorWorker : public libcamera::Thread + { + public: + enum class State { + Stopped, + Running, + Flushing, + }; + + PostProcessorWorker(PostProcessor *postProcessor); + ~PostProcessorWorker(); + + void start(); + void queueRequest(Camera3RequestDescriptor::StreamBuffer *request); + void flush(); + + protected: + void run() override; + + private: + PostProcessor *postProcessor_; + + libcamera::Mutex mutex_; + std::condition_variable cv_; + + std::queue requests_; + State state_ = State::Stopped; + }; + int waitFence(int fence); CameraDevice *const cameraDevice_; @@ -142,6 +177,8 @@ private: */ std::unique_ptr mutex_; std::unique_ptr postProcessor_; + + std::unique_ptr worker_; }; #endif /* __ANDROID_CAMERA_STREAM__ */