From patchwork Sat Oct 23 10:32:56 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14287 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 B81F5BDB1C for ; Sat, 23 Oct 2021 10:33:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6F22164875; Sat, 23 Oct 2021 12:33:15 +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="d1jheNU4"; 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 3C44364872 for ; Sat, 23 Oct 2021 12:33:12 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.213]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 30326547; Sat, 23 Oct 2021 12:33:10 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1634985191; bh=AlJReVeln4u9zXt1oUQuYkFTPfsRC8SFWObfcKIUMzE=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=d1jheNU4xt7Llm1yoqxeaUbnHw1T7z2v0E/QS3gwJQE67QxltVy8pQF8cF7ToVWV7 1+5mJ6djOhUQOSiQkwwZQq+iYHHDzIrUt5ozhj6Ew/73u7UK7wFsObyJs+U1/pD5SE l0fubRAvJFA4Sg6DKWRfN5Cuh2V9RHsqXmht1c8Y= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Sat, 23 Oct 2021 16:02:56 +0530 Message-Id: <20211023103302.152769-2-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 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 | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index f44a2717..ca25c4db 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -163,8 +163,7 @@ int CameraStream::process(const FrameBuffer &source, dest.fence = -1; } - if (!postProcessor_) - return 0; + ASSERT(type_ != Type::Direct); /* * \todo Buffer mapping and processing should be moved to a From patchwork Sat Oct 23 10:32:57 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14288 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 D240BBDB1C for ; Sat, 23 Oct 2021 10:33:16 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 02ED864873; Sat, 23 Oct 2021 12:33:16 +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="XoUvQbhJ"; 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 DF60260239 for ; Sat, 23 Oct 2021 12:33:13 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.213]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E3F5B547; Sat, 23 Oct 2021 12:33:12 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1634985193; bh=l/NM6MLuXFNLgFropUfBbVQLha4gxOv5E4sKnwI20g8=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=XoUvQbhJCzS+zQ16hV/kX2UoxLkhQ7Y7ZkdsyEMqYvpNbyiMFUtNrwQjuXNaAwkWR qmXdYivYE1lOwCanWLCubpRbi+Jz0G8FQow9wkTXTAoidFnxjsyU9LNN25EODiOhOV jmLdofkogMCGS6Vdx1RtHVcB+6AoNn/dLYS5RFVw= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Sat, 23 Oct 2021 16:02:57 +0530 Message-Id: <20211023103302.152769-3-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 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_ will be nullptr in PostProcessorJpeg::process() 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 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) From patchwork Sat Oct 23 10:32:59 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14290 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 9887CC324E 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 41E9A6486D; Sat, 23 Oct 2021 12:33:19 +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="mTyuLp41"; dkim-atps=neutral Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A895A6486D for ; Sat, 23 Oct 2021 12:33:17 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.213]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id A4F31547; Sat, 23 Oct 2021 12:33:16 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1634985197; bh=kVFhHCou/b4t8bk+YY6DnphhftoWalA7rIIgwju4VqQ=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=mTyuLp41p/AfUT6srWvyhaPC1NlPF2lWxrXEzxNzAv8VO2HE4PXtiH0KuzhRp3Rj9 BOOHok+M82exmsjHrcvB8MwdKb4a/LGVk2Bgre7eaiuHQtHBLaDsEYP22GAtzY0nQK XGCv4zDip7LwIvgG8EjlSRAwNjHU1rco17qU7BKk= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Sat, 23 Oct 2021 16:02:59 +0530 Message-Id: <20211023103302.152769-5-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 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 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 | 11 ++++++----- src/android/camera_request.cpp | 3 ++- src/android/camera_request.h | 5 +++++ src/android/camera_stream.cpp | 14 ++++++++------ src/android/camera_stream.h | 3 +-- 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, 37 insertions(+), 33 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index f4d4fb09..2a98a2e6 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; } @@ -1134,14 +1135,14 @@ void CameraDevice::requestComplete(Request *request) continue; } - int ret = stream->process(*src, buffer, descriptor); + int ret = stream->process(*src, 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..fd927167 100644 --- a/src/android/camera_request.cpp +++ b/src/android/camera_request.cpp @@ -36,7 +36,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 904886da..c4bc5d6e 100644 --- a/src/android/camera_request.h +++ b/src/android/camera_request.h @@ -17,6 +17,7 @@ #include +#include "camera_buffer.h" #include "camera_metadata.h" #include "camera_worker.h" @@ -36,6 +37,10 @@ public: std::unique_ptr frameBuffer; int fence; Status status; + libcamera::FrameBuffer *internalBuffer; + std::unique_ptr destBuffer; + const libcamera::FrameBuffer *srcBuffer; + Camera3RequestDescriptor *request; }; Camera3RequestDescriptor(libcamera::Camera *camera, diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index ca25c4db..0e268cdf 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -147,8 +147,7 @@ int CameraStream::waitFence(int fence) } int CameraStream::process(const FrameBuffer &source, - Camera3RequestDescriptor::StreamBuffer &dest, - Camera3RequestDescriptor *request) + Camera3RequestDescriptor::StreamBuffer &dest) { /* Handle waiting on fences on the destination buffer. */ if (dest.fence != -1) { @@ -170,14 +169,17 @@ 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()) { + dest.destBuffer = std::make_unique( + *dest.camera3Buffer, output.pixelFormat, output.size, + PROT_READ | PROT_WRITE); + if (!dest.destBuffer->isValid()) { LOG(HAL, Error) << "Failed to create destination buffer"; return -EINVAL; } - return postProcessor_->process(source, &destBuffer, request); + dest.srcBuffer = &source; + + return postProcessor_->process(&dest); } FrameBuffer *CameraStream::getBuffer() diff --git a/src/android/camera_stream.h b/src/android/camera_stream.h index f242336e..e9c320b1 100644 --- a/src/android/camera_stream.h +++ b/src/android/camera_stream.h @@ -122,8 +122,7 @@ public: int configure(); int process(const libcamera::FrameBuffer &source, - Camera3RequestDescriptor::StreamBuffer &dest, - Camera3RequestDescriptor *request); + Camera3RequestDescriptor::StreamBuffer &dest); 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..da71f113 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->destBuffer.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..eeb8f1f4 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->destBuffer.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 Sat Oct 23 10:33:00 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14291 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 82BC6BDB1C for ; Sat, 23 Oct 2021 10:33:21 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3B1B464872; Sat, 23 Oct 2021 12:33:21 +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="N95KbetL"; 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 B50D764870 for ; Sat, 23 Oct 2021 12:33:19 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.213]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8AE73547; Sat, 23 Oct 2021 12:33:18 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1634985199; bh=fUoLBab6iCL7Atuu2/jNcWYcAX7Nwx1kRd8tGN6qetI=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=N95KbetL8u6rg4MvtXMIken41/dJuDdukSUCNfaDK9Ki7iWFhFsA70t4zs7PLJ/nf Dr8eGZs7UJT5lVy9wQe4DgyzGcad0rKSLnRjT98fqDkdtGjTaiEFV7lm8GR2k5LBQJ jRN/DjKpWWz0H0c9lQO5LpmKlXGH+hnwJuREQWmU= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Sat, 23 Oct 2021 16:03:00 +0530 Message-Id: <20211023103302.152769-6-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 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 emit 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 streamProcessMutex_ 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 --- src/android/camera_device.cpp | 95 ++++++++++++++++++------ 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, 114 insertions(+), 25 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 2a98a2e6..3114def0 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -926,6 +926,8 @@ 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 +957,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,43 +1123,46 @@ void CameraDevice::requestComplete(Request *request) } /* Handle post-processing. */ - bool hasPostProcessingErrors = false; - for (auto &buffer : descriptor->buffers_) { - CameraStream *stream = buffer.stream; - - if (stream->type() == CameraStream::Type::Direct) - continue; + bool needPostProcessing = false; + /* + * \todo Protect the loop below with streamProcessMutex_ when post + * processor runs asynchronously. + */ + auto iter = descriptor->pendingStreamsToProcess_.begin(); + while (descriptor->pendingStreamsToProcess_.size() > 0) { + CameraStream *stream = iter->first; + Camera3RequestDescriptor::StreamBuffer *buffer = iter->second; + needPostProcessing = true; 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); - hasPostProcessingErrors = true; + setBufferStatus(*buffer, Camera3RequestDescriptor::Status::Error); + iter = descriptor->pendingStreamsToProcess_.erase(iter); continue; } - int ret = stream->process(*src, buffer); + ++iter; + int ret = stream->process(*src, *buffer); + if (ret) { + setBufferStatus(*buffer, Camera3RequestDescriptor::Status::Error); + descriptor->pendingStreamsToProcess_.erase(stream); + } + } + if (needPostProcessing) { /* - * If the framebuffer is internal to CameraStream return it back - * now that we're done processing it. + * \todo We will require to check if we failed to queue + * post-processing requests when we migrate to post-processor + * running asynchronously. + * + * if (descriptor->pendingStreamsToProcess_.size() == 0) + * completeDescriptor(descriptor); */ - if (buffer.internalBuffer) - stream->putBuffer(buffer.internalBuffer); - if (ret) { - buffer.status = Camera3RequestDescriptor::Status::Error; - hasPostProcessingErrors = true; - notifyError(descriptor->frameNumber_, stream->camera3Stream(), - CAMERA3_MSG_ERROR_BUFFER); - } + return; } - if (hasPostProcessingErrors) - descriptor->status_ = Camera3RequestDescriptor::Status::Error; - completeDescriptor(descriptor); } @@ -1210,6 +1218,45 @@ void CameraDevice::sendCaptureResults() } } +void CameraDevice::setBufferStatus(Camera3RequestDescriptor::StreamBuffer &streamBuffer, + Camera3RequestDescriptor::Status 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); + + 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) +{ + Camera3RequestDescriptor *request = streamBuffer->request; + MutexLocker locker(request->streamsProcessMutex_); + + setBufferStatus(*streamBuffer, status); + request->pendingStreamsToProcess_.erase(streamBuffer->stream); + + if (request->pendingStreamsToProcess_.size() > 0) + return; + + locker.unlock(); + + completeDescriptor(streamBuffer->request); +} + 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 c4bc5d6e..cc2b7035 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 0e268cdf..4e275cde 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 da71f113..cbbe7128 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 eeb8f1f4..8e77bf57 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->destBuffer.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 Sat Oct 23 10:33:01 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14292 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 DFB3ABDB1C for ; Sat, 23 Oct 2021 10:33:23 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9670C64871; Sat, 23 Oct 2021 12:33:23 +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="vQntXnz2"; 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 E69DD64870 for ; Sat, 23 Oct 2021 12:33:21 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.213]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 98D5D547; Sat, 23 Oct 2021 12:33:20 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1634985201; bh=9EcROADsZ0dnUpkC4C3yBGBKmTVRHri3V27iz+omevo=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=vQntXnz2BZeB64gtbyYyLMXxpS/fhgk9UK70ac6cySMfUCMwdmaPhJZXTAPiI2OUw BZPiNM+Cdxk1vS3WQccHvrnpIH0Kl5Z716mYb7OZlkAuJPUoHHN5KAGTfFA8L1zi3z xhrx4IqbokHFpTvEtULg9hRC/eUd+OUOiqS10zMc= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Sat, 23 Oct 2021 16:03:01 +0530 Message-Id: <20211023103302.152769-7-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 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 4e275cde..45d0607d 100644 --- a/src/android/camera_stream.cpp +++ b/src/android/camera_stream.cpp @@ -194,7 +194,9 @@ int CameraStream::process(const FrameBuffer &source, dest.srcBuffer = &source; - return postProcessor_->process(&dest); + postProcessor_->process(&dest); + + return 0; } FrameBuffer *CameraStream::getBuffer() diff --git a/src/android/jpeg/post_processor_jpeg.cpp b/src/android/jpeg/post_processor_jpeg.cpp index cbbe7128..9d7523cb 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 8e77bf57..1ac7995a 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->destBuffer.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 Sat Oct 23 10:33:02 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Umang Jain X-Patchwork-Id: 14293 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 4A8A2BDB1C for ; Sat, 23 Oct 2021 10:33:26 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 05D2264872; Sat, 23 Oct 2021 12:33:26 +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="IuOuLXkc"; 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 1F1DD60239 for ; Sat, 23 Oct 2021 12:33:24 +0200 (CEST) Received: from perceval.ideasonboard.com (unknown [103.251.226.213]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D0398547; Sat, 23 Oct 2021 12:33:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1634985203; bh=25zdqmzY5/GaMN86SNuiqtF6L56NB0UJK7k6TU006ZM=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=IuOuLXkcU7isq3lHnbMAANhI5y87jqZYXSb5OTMyPBLWCa33ITgtpN5N136XAGRtP avBRRul7ShpZvsfSbEEgn45q133N7sx7VOOzifGXvGNddAf0Wby1Dz6skQpABjfQEc CEVIJvb14EHNEVAEuBjPAUccQCcA9OYAhOzSHI/I= From: Umang Jain To: libcamera-devel@lists.libcamera.org Date: Sat, 23 Oct 2021 16:03:02 +0530 Message-Id: <20211023103302.152769-8-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 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 post processing streams loop in CameraDevice::requestComplete() is tweaked a bit to better suit asynchronous needs of post processing. Also, the entire 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. 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 | 49 +++++------------ src/android/camera_stream.cpp | 101 ++++++++++++++++++++++++++++++++-- src/android/camera_stream.h | 38 ++++++++++++- 3 files changed, 148 insertions(+), 40 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 3114def0..dc39467b 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -1026,29 +1026,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,12 +1103,16 @@ void CameraDevice::requestComplete(Request *request) /* Handle post-processing. */ bool needPostProcessing = false; + 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 (descriptor->pendingStreamsToProcess_.size() > 0) { + while (iter != descriptor->pendingStreamsToProcess_.end()) { CameraStream *stream = iter->first; Camera3RequestDescriptor::StreamBuffer *buffer = iter->second; needPostProcessing = true; @@ -1151,18 +1134,16 @@ void CameraDevice::requestComplete(Request *request) } if (needPostProcessing) { - /* - * \todo We will require to check if we failed to queue - * post-processing requests when we migrate to post-processor - * running asynchronously. - * - * if (descriptor->pendingStreamsToProcess_.size() == 0) - * completeDescriptor(descriptor); - */ + if (descriptor->pendingStreamsToProcess_.size() == 0) { + locker.unlock(); + completeDescriptor(descriptor); + } return; } + locker.unlock(); + completeDescriptor(descriptor); } diff --git a/src/android/camera_stream.cpp b/src/android/camera_stream.cpp index 45d0607d..68b916e9 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) { @@ -179,10 +182,6 @@ int CameraStream::process(const FrameBuffer &source, ASSERT(type_ != Type::Direct); - /* - * \todo Buffer mapping and processing should be moved to a - * separate thread. - */ const StreamConfiguration &output = configuration(); dest.destBuffer = std::make_unique( *dest.camera3Buffer, output.pixelFormat, output.size, @@ -194,11 +193,19 @@ int CameraStream::process(const FrameBuffer &source, dest.srcBuffer = &source; - postProcessor_->process(&dest); + worker_->queueRequest(&dest); return 0; } +void CameraStream::flush() +{ + if (!postProcessor_) + return; + + worker_->flush(); +} + FrameBuffer *CameraStream::getBuffer() { if (!allocator_) @@ -226,3 +233,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 e9c320b1..7c6e887c 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,9 +24,9 @@ #include #include "camera_request.h" +#include "post_processor.h" class CameraDevice; -class PostProcessor; class CameraStream { @@ -125,8 +129,38 @@ public: Camera3RequestDescriptor::StreamBuffer &dest); 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_; @@ -143,6 +177,8 @@ private: */ std::unique_ptr mutex_; std::unique_ptr postProcessor_; + + std::unique_ptr worker_; }; #endif /* __ANDROID_CAMERA_STREAM__ */