From patchwork Thu May 13 09:22:39 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 12263 X-Patchwork-Delegate: jacopo@jmondi.org 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 AE15DC31EE for ; Thu, 13 May 2021 09:22:09 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 23B1968922; Thu, 13 May 2021 11:22:09 +0200 (CEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 610676891A for ; Thu, 13 May 2021 11:22:07 +0200 (CEST) Received: from uno.LocalDomain (93-61-96-190.ip145.fastwebnet.it [93.61.96.190]) (Authenticated sender: jacopo@jmondi.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id BB0B8240003; Thu, 13 May 2021 09:22:06 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Thu, 13 May 2021 11:22:39 +0200 Message-Id: <20210513092246.42847-2-jacopo@jmondi.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210513092246.42847-1-jacopo@jmondi.org> References: <20210513092246.42847-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 1/8] android: Rework request completion notification 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" The current implementation of CameraDevice::requestComplete() which handles event notification and calls the framework capture result callback does not handle error notification precisely enough. In detail: - Error notification is an asynchronous callback that has to be notified to the framework as soon as an error condition is detected, and it independent from the process_capture_result() callback - Error notification requires the HAL to report the precise error cause, by specifying the correct CAMERA3_MSG_ERROR_* error code. The current implementation only notifies errors of type CAMERA3_MSG_ERROR_REQUEST at the end of the procedure, before the callback invocation. Rework the procedure to: - Notify CAMERA3_MSG_ERROR_DEVICE and perform library tear-down in case a Fatal error is detected - Notify CAMERA3_MSG_ERROR_REQUEST if the libcamera::Request::status is different than RequestCompleted and immediately call process_capture_result() with all buffers in error state. - Notify the shutter event as soon as possible - Notify CAMERA3_MSG_ERROR_RESULT in case the metadata cannot be generated correctly and call process_capture_result() with the right buffer state regardless of metadata availability. - Notify CAMERA3_MSG_ERROR_BUFFER for buffers whose post-processing failed While at it, return the CameraStream buffer by calling cameraStream->putBuffer() regardless of the post-processing result. No regression detected when running CTS in LIMITED mode. Signed-off-by: Jacopo Mondi Reviewed-by: Hirokazu Honda --- src/android/camera_device.cpp | 138 +++++++++++++++++++++------------- src/android/camera_device.h | 3 +- 2 files changed, 86 insertions(+), 55 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 7d4d0febf361..383baa60f918 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -2027,17 +2027,20 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques void CameraDevice::requestComplete(Request *request) { - camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; - std::unique_ptr resultMetadata; - decltype(descriptors_)::node_type node; { std::scoped_lock lock(mutex_); auto it = descriptors_.find(request->cookie()); if (it == descriptors_.end()) { + /* + * \todo Clarify if the Camera has to be closed on + * ERROR_DEVICE and possibly demote the Fatal to simple + * Error. + */ + notifyError(0, nullptr, CAMERA3_MSG_ERROR_DEVICE); LOG(HAL, Fatal) << "Unknown request: " << request->cookie(); - status = CAMERA3_BUFFER_STATUS_ERROR; + return; } @@ -2045,16 +2048,72 @@ void CameraDevice::requestComplete(Request *request) } Camera3RequestDescriptor &descriptor = node.mapped(); + /* + * Prepare the capture result for the Android camera stack. + * + * The buffer status is set to OK and later changed to ERROR if + * post-processing/compression fails. + */ + camera3_capture_result_t captureResult = {}; + captureResult.frame_number = descriptor.frameNumber_; + captureResult.num_output_buffers = descriptor.buffers_.size(); + for (camera3_stream_buffer_t &buffer : descriptor.buffers_) { + buffer.acquire_fence = -1; + buffer.release_fence = -1; + buffer.status = CAMERA3_BUFFER_STATUS_OK; + } + captureResult.output_buffers = descriptor.buffers_.data(); + captureResult.partial_result = 1; + + /* + * If the Request has failed, abort the request by notifying the error + * and complete the request with all buffers in error state. + */ if (request->status() != Request::RequestComplete) { - LOG(HAL, Error) << "Request not successfully completed: " + LOG(HAL, Error) << "Request " << request->cookie() + << " not successfully completed: " << request->status(); - status = CAMERA3_BUFFER_STATUS_ERROR; + + notifyError(descriptor.frameNumber_, + descriptor.buffers_[0].stream, + CAMERA3_MSG_ERROR_REQUEST); + + captureResult.partial_result = 0; + for (camera3_stream_buffer_t &buffer : descriptor.buffers_) + buffer.status = CAMERA3_BUFFER_STATUS_ERROR; + callbacks_->process_capture_result(callbacks_, &captureResult); + + return; } + /* + * Notify shutter as soon as we have verified we have a valid request. + * + * \todo The shutter event notification should be sent to the framework + * as soon as possible, earlier than request completion time. + */ + uint64_t sensorTimestamp = static_cast(request->metadata() + .get(controls::SensorTimestamp)); + notifyShutter(descriptor.frameNumber_, sensorTimestamp); + LOG(HAL, Debug) << "Request " << request->cookie() << " completed with " << descriptor.buffers_.size() << " streams"; - resultMetadata = getResultMetadata(descriptor); + /* + * Generate the metadata associated with the captured buffers. + * + * Notify if the metadata generation has failed, but continue processing + * buffers and return an empty metadata pack. + */ + std::unique_ptr resultMetadata = getResultMetadata(descriptor); + if (!resultMetadata) { + notifyError(descriptor.frameNumber_, descriptor.buffers_[0].stream, + CAMERA3_MSG_ERROR_RESULT); + + /* The camera framework expects an empy metadata pack on error. */ + resultMetadata = std::make_unique(0, 0); + } + captureResult.result = resultMetadata->get(); /* Handle any JPEG compression. */ for (camera3_stream_buffer_t &buffer : descriptor.buffers_) { @@ -2067,56 +2126,27 @@ void CameraDevice::requestComplete(Request *request) FrameBuffer *src = request->findBuffer(cameraStream->stream()); if (!src) { LOG(HAL, Error) << "Failed to find a source stream buffer"; + buffer.status = CAMERA3_BUFFER_STATUS_ERROR; + notifyError(descriptor.frameNumber_, buffer.stream, + CAMERA3_MSG_ERROR_BUFFER); continue; } - int ret = cameraStream->process(*src, - *buffer.buffer, + int ret = cameraStream->process(*src, *buffer.buffer, descriptor.settings_, resultMetadata.get()); - if (ret) { - status = CAMERA3_BUFFER_STATUS_ERROR; - continue; - } - /* * Return the FrameBuffer to the CameraStream now that we're * done processing it. */ if (cameraStream->type() == CameraStream::Type::Internal) cameraStream->putBuffer(src); - } - /* Prepare to call back the Android camera stack. */ - camera3_capture_result_t captureResult = {}; - captureResult.frame_number = descriptor.frameNumber_; - captureResult.num_output_buffers = descriptor.buffers_.size(); - for (camera3_stream_buffer_t &buffer : descriptor.buffers_) { - buffer.acquire_fence = -1; - buffer.release_fence = -1; - buffer.status = status; - } - captureResult.output_buffers = descriptor.buffers_.data(); - - if (status == CAMERA3_BUFFER_STATUS_OK) { - uint64_t timestamp = - static_cast(request->metadata() - .get(controls::SensorTimestamp)); - notifyShutter(descriptor.frameNumber_, timestamp); - - captureResult.partial_result = 1; - captureResult.result = resultMetadata->get(); - } - - if (status == CAMERA3_BUFFER_STATUS_ERROR || !captureResult.result) { - /* \todo Improve error handling. In case we notify an error - * because the metadata generation fails, a shutter event has - * already been notified for this frame number before the error - * is here signalled. Make sure the error path plays well with - * the camera stack state machine. - */ - notifyError(descriptor.frameNumber_, - descriptor.buffers_[0].stream); + if (ret) { + buffer.status = CAMERA3_BUFFER_STATUS_ERROR; + notifyError(descriptor.frameNumber_, buffer.stream, + CAMERA3_MSG_ERROR_BUFFER); + } } callbacks_->process_capture_result(callbacks_, &captureResult); @@ -2138,23 +2168,23 @@ void CameraDevice::notifyShutter(uint32_t frameNumber, uint64_t timestamp) callbacks_->notify(callbacks_, ¬ify); } -void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream) +void CameraDevice::notifyError(uint32_t frameNumber, camera3_stream_t *stream, + camera3_error_msg_code code) { camera3_notify_msg_t notify = {}; - /* - * \todo Report and identify the stream number or configuration to - * clarify the stream that failed. - */ - LOG(HAL, Error) << "Error occurred on frame " << frameNumber << " (" - << toPixelFormat(stream->format).toString() << ")"; - notify.type = CAMERA3_MSG_ERROR; notify.message.error.error_stream = stream; notify.message.error.frame_number = frameNumber; - notify.message.error.error_code = CAMERA3_MSG_ERROR_REQUEST; + notify.message.error.error_code = code; callbacks_->notify(callbacks_, ¬ify); + + if (stream) + LOG(HAL, Error) << "Error occurred on frame " << frameNumber << " (" + << toPixelFormat(stream->format).toString() << ")"; + else + LOG(HAL, Error) << "Fatal error occurred on device"; } /* diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 23457e47767a..8d5da8bc59e1 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -101,7 +101,8 @@ private: std::tuple calculateStaticMetadataSize(); libcamera::FrameBuffer *createFrameBuffer(const buffer_handle_t camera3buffer); void notifyShutter(uint32_t frameNumber, uint64_t timestamp); - void notifyError(uint32_t frameNumber, camera3_stream_t *stream); + void notifyError(uint32_t frameNumber, camera3_stream_t *stream, + camera3_error_msg_code code); std::unique_ptr requestTemplatePreview(); std::unique_ptr requestTemplateVideo(); libcamera::PixelFormat toPixelFormat(int format) const; From patchwork Thu May 13 09:22:40 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 12264 X-Patchwork-Delegate: jacopo@jmondi.org 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 2952FC31EE for ; Thu, 13 May 2021 09:22:11 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E81B96891D; Thu, 13 May 2021 11:22:10 +0200 (CEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 1AF406891C for ; Thu, 13 May 2021 11:22:08 +0200 (CEST) Received: from uno.LocalDomain (93-61-96-190.ip145.fastwebnet.it [93.61.96.190]) (Authenticated sender: jacopo@jmondi.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 8EB88240006; Thu, 13 May 2021 09:22:07 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Thu, 13 May 2021 11:22:40 +0200 Message-Id: <20210513092246.42847-3-jacopo@jmondi.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210513092246.42847-1-jacopo@jmondi.org> References: <20210513092246.42847-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 2/8] libcamera: request: Add Request::cancel() 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" Add a cancel() function to the Request class that allows to forcefully complete the request and its associated buffers in error state. Only pending requests can be forcefully cancelled. Enforce that by asserting the request state to be RequestPending. Signed-off-by: Jacopo Mondi Reviewed-by: Hirokazu Honda --- include/libcamera/request.h | 1 + src/libcamera/request.cpp | 30 ++++++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/include/libcamera/request.h b/include/libcamera/request.h index 4cf5ff3f7d3b..5596901ddd8e 100644 --- a/include/libcamera/request.h +++ b/include/libcamera/request.h @@ -65,6 +65,7 @@ private: friend class PipelineHandler; void complete(); + void cancel(); bool completeBuffer(FrameBuffer *buffer); diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp index ce2dd7b17f10..fc5e25199112 100644 --- a/src/libcamera/request.cpp +++ b/src/libcamera/request.cpp @@ -292,6 +292,36 @@ void Request::complete() LIBCAMERA_TRACEPOINT(request_complete, this); } +/** + * \brief Cancel a queued request + * + * Mark the request and its associated buffers as cancelled and complete it. + * + * Set the status of each buffer in the request to the frame cancelled state and + * remove them from the pending buffer queue before completing the request with + * error. + */ +void Request::cancel() +{ + LIBCAMERA_TRACEPOINT(request_cancel, this); + + ASSERT(status_ == RequestPending); + + /* + * We can't simply loop and call completeBuffer() as erase() invalidates + * pointers and iterators, so we have to manually cancel the buffer and + * erase it from the pending buffers list. + */ + for (auto buffer = pending_.begin(); buffer != pending_.end();) { + (*buffer)->cancel(); + (*buffer)->setRequest(nullptr); + buffer = pending_.erase(buffer); + } + + cancelled_ = true; + complete(); +} + /** * \brief Complete a buffer for the request * \param[in] buffer The buffer that has completed From patchwork Thu May 13 09:22:41 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 12265 X-Patchwork-Delegate: jacopo@jmondi.org 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 4F0FAC31EE for ; Thu, 13 May 2021 09:22:12 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1733D6891F; Thu, 13 May 2021 11:22:12 +0200 (CEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9D69F68919 for ; Thu, 13 May 2021 11:22:08 +0200 (CEST) Received: from uno.LocalDomain (93-61-96-190.ip145.fastwebnet.it [93.61.96.190]) (Authenticated sender: jacopo@jmondi.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 29AA1240006; Thu, 13 May 2021 09:22:07 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Thu, 13 May 2021 11:22:41 +0200 Message-Id: <20210513092246.42847-4-jacopo@jmondi.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210513092246.42847-1-jacopo@jmondi.org> References: <20210513092246.42847-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 3/8] libcamera: pipeline_handler: Notify Request queueing failure 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" Capture requests are queued by the PipelineHandler base class to each pipeline handler implementation using the virtual queueRequestDevice() function. However, if the pipeline handler fails to queue the request to the hardware, the request gets silently deleted from the list of queued ones, without notifying application of the error. Allowing applications to know if a Request has failed to queue by emitting the Camera::bufferCompleted and Camera::requestCompleted signals allows them to maintain a state consistent with the one internal to the library. To do so, if queuing a request to the device fails, cancel the request and emit the completion signal to report the failure to applications. Signed-off-by: Jacopo Mondi --- src/libcamera/pipeline_handler.cpp | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index f41b7a7b3308..260a52018a4d 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -391,6 +391,8 @@ bool PipelineHandler::hasPendingRequests(const Camera *camera) const * This method queues a capture request to the pipeline handler for processing. * The request is first added to the internal list of queued requests, and * then passed to the pipeline handler with a call to queueRequestDevice(). + * If the pipeline handler fails in queuing the request to the hardware the + * request is immediately completed with error. * * Keeping track of queued requests ensures automatic completion of all requests * when the pipeline handler is stopped with stop(). Request completion shall be @@ -409,8 +411,19 @@ void PipelineHandler::queueRequest(Request *request) request->sequence_ = data->requestSequence_++; int ret = queueRequestDevice(camera, request); - if (ret) + if (ret) { + /* + * Cancel the request and notify completion of its buffers in + * error state. Then complete the cancelled request and remove + * it from the queued requests list. + */ + request->cancel(); + for (auto bufferMap : request->buffers()) + camera->bufferCompleted.emit(request, bufferMap.second); + + camera->requestComplete(request); data->queuedRequests_.remove(request); + } } /** From patchwork Thu May 13 09:22:42 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 12266 X-Patchwork-Delegate: jacopo@jmondi.org 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 1F85CC31EE for ; Thu, 13 May 2021 09:22:13 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id E3D3C68923; Thu, 13 May 2021 11:22:12 +0200 (CEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8D0236891D for ; Thu, 13 May 2021 11:22:09 +0200 (CEST) Received: from uno.LocalDomain (93-61-96-190.ip145.fastwebnet.it [93.61.96.190]) (Authenticated sender: jacopo@jmondi.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id C5DD1240006; Thu, 13 May 2021 09:22:08 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Thu, 13 May 2021 11:22:42 +0200 Message-Id: <20210513092246.42847-5-jacopo@jmondi.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210513092246.42847-1-jacopo@jmondi.org> References: <20210513092246.42847-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 4/8] android: camera_device: Replace running_ with CameraState 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" The CameraDevice class maintains the camera state in the 'running_' boolean flag to check if the camera has to be started at the first received process_capture_request() call which happens after the camera had been stopped. So far this was correct, as the operations that change the camera could only start or stop the camera, so a simple boolean flag was enough. To prepare to handle the flush() operation that will introduce a new 'flushing' state, replace the simple plain boolean flag with an enumeration of values that define the CameraState. Signed-off-by: Jacopo Mondi Reviewed-by: Niklas Söderlund Reviewed-by: Hirokazu Honda --- src/android/camera_device.cpp | 10 +++++----- src/android/camera_device.h | 8 +++++++- 2 files changed, 12 insertions(+), 6 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 383baa60f918..82dd7ce0ee99 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -402,7 +402,7 @@ CameraDevice::Camera3RequestDescriptor::Camera3RequestDescriptor( */ CameraDevice::CameraDevice(unsigned int id, std::shared_ptr camera) - : id_(id), running_(false), camera_(std::move(camera)), + : id_(id), state_(CameraStopped), camera_(std::move(camera)), facing_(CAMERA_FACING_FRONT), orientation_(0) { camera_->requestCompleted.connect(this, &CameraDevice::requestComplete); @@ -758,14 +758,14 @@ void CameraDevice::close() void CameraDevice::stop() { - if (!running_) + if (state_ == CameraStopped) return; worker_.stop(); camera_->stop(); descriptors_.clear(); - running_ = false; + state_ = CameraStopped; } void CameraDevice::setCallbacks(const camera3_callback_ops_t *callbacks) @@ -1915,7 +1915,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques return -EINVAL; /* Start the camera if that's the first request we handle. */ - if (!running_) { + if (state_ == CameraStopped) { worker_.start(); int ret = camera_->start(); @@ -1924,7 +1924,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques return ret; } - running_ = true; + state_ = CameraRunning; } /* diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 8d5da8bc59e1..30b364decaab 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -88,6 +88,11 @@ private: int androidFormat; }; + enum State { + CameraStopped, + CameraRunning, + }; + void stop(); int initializeStreamConfigurations(); @@ -115,7 +120,8 @@ private: CameraWorker worker_; - bool running_; + State state_; + std::shared_ptr camera_; std::unique_ptr config_; From patchwork Thu May 13 09:22:43 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 12267 X-Patchwork-Delegate: jacopo@jmondi.org 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 472F5C31EE for ; Thu, 13 May 2021 09:22:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1586968921; Thu, 13 May 2021 11:22:15 +0200 (CEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8B10368919 for ; Thu, 13 May 2021 11:22:10 +0200 (CEST) Received: from uno.LocalDomain (93-61-96-190.ip145.fastwebnet.it [93.61.96.190]) (Authenticated sender: jacopo@jmondi.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id C12E3240007; Thu, 13 May 2021 09:22:09 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Thu, 13 May 2021 11:22:43 +0200 Message-Id: <20210513092246.42847-6-jacopo@jmondi.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210513092246.42847-1-jacopo@jmondi.org> References: <20210513092246.42847-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 5/8] android: Replace scoped_lock<> with libcamera::MutexLocker 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" The CameraDevice class uses std::scoped_lock<> to guard access to the class' descriptors_ member. std::scoped_lock<> provides a set of features that guarantees safety when locking multiple mutexes in a critical section, while for single locks happening in a scoped block it does not provides benefits compared to the simplest std::unique_lock<> which libcamera provides the MutexLocker type for. Replace usage of std::scoped_lock<> with libcamera::MutexLocker to make the implementation consistent with the rest of the code base. Signed-off-by: Jacopo Mondi Reviewed-by: Niklas Söderlund Reviewed-by: Hirokazu Honda --- src/android/camera_device.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 82dd7ce0ee99..8a4543f079eb 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -23,6 +23,7 @@ #include "libcamera/internal/formats.h" #include "libcamera/internal/log.h" +#include "libcamera/internal/thread.h" #include "libcamera/internal/utils.h" #include "system/graphics.h" @@ -2018,7 +2019,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques worker_.queueRequest(descriptor.request_.get()); { - std::scoped_lock lock(mutex_); + MutexLocker lock(mutex_); descriptors_[descriptor.request_->cookie()] = std::move(descriptor); } @@ -2029,7 +2030,7 @@ void CameraDevice::requestComplete(Request *request) { decltype(descriptors_)::node_type node; { - std::scoped_lock lock(mutex_); + MutexLocker lock(mutex_); auto it = descriptors_.find(request->cookie()); if (it == descriptors_.end()) { /* From patchwork Thu May 13 09:22:44 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 12268 X-Patchwork-Delegate: jacopo@jmondi.org 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 81C47C31EF for ; Thu, 13 May 2021 09:22:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 4426A6892E; Thu, 13 May 2021 11:22:15 +0200 (CEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 547C26891A for ; Thu, 13 May 2021 11:22:11 +0200 (CEST) Received: from uno.LocalDomain (93-61-96-190.ip145.fastwebnet.it [93.61.96.190]) (Authenticated sender: jacopo@jmondi.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id B0B92240008; Thu, 13 May 2021 09:22:10 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Thu, 13 May 2021 11:22:44 +0200 Message-Id: <20210513092246.42847-7-jacopo@jmondi.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210513092246.42847-1-jacopo@jmondi.org> References: <20210513092246.42847-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 6/8] android: Guard access to the camera state 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" Guard access to the camera state and the start/stop sequences with a mutex. Currently only stop() and the first call to processCaptureRequest() start and stop the camera, and they're not meant to race with each other. With the introduction of flush() the camera can be stopped concurrently to a processCaptureRequest() call, hence access to the camera state will need to be protected. Prepare for that by guarding the existing paths with a mutex. Signed-off-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- src/android/camera_device.cpp | 23 ++++++++++++++--------- src/android/camera_device.h | 1 + 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 8a4543f079eb..c6cd0b6e8be7 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -759,6 +759,7 @@ void CameraDevice::close() void CameraDevice::stop() { + MutexLocker cameraLock(cameraMutex_); if (state_ == CameraStopped) return; @@ -1915,17 +1916,21 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques if (!isValidRequest(camera3Request)) return -EINVAL; - /* Start the camera if that's the first request we handle. */ - if (state_ == CameraStopped) { - worker_.start(); + { + MutexLocker cameraLock(cameraMutex_); - int ret = camera_->start(); - if (ret) { - LOG(HAL, Error) << "Failed to start camera"; - return ret; - } + /* Start the camera if that's the first request we handle. */ + if (state_ == CameraStopped) { + worker_.start(); - state_ = CameraRunning; + int ret = camera_->start(); + if (ret) { + LOG(HAL, Error) << "Failed to start camera"; + return ret; + } + + state_ = CameraRunning; + } } /* diff --git a/src/android/camera_device.h b/src/android/camera_device.h index 30b364decaab..f263fdae472a 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -120,6 +120,7 @@ private: CameraWorker worker_; + libcamera::Mutex cameraMutex_; /* Protects access to the camera state. */ State state_; std::shared_ptr camera_; From patchwork Thu May 13 09:22:45 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 12269 X-Patchwork-Delegate: jacopo@jmondi.org 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 F3F96C31EE for ; Thu, 13 May 2021 09:22:15 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6F31A68930; Thu, 13 May 2021 11:22:15 +0200 (CEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2175C68922 for ; Thu, 13 May 2021 11:22:12 +0200 (CEST) Received: from uno.LocalDomain (93-61-96-190.ip145.fastwebnet.it [93.61.96.190]) (Authenticated sender: jacopo@jmondi.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 7E9E2240006; Thu, 13 May 2021 09:22:11 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Thu, 13 May 2021 11:22:45 +0200 Message-Id: <20210513092246.42847-8-jacopo@jmondi.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210513092246.42847-1-jacopo@jmondi.org> References: <20210513092246.42847-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 7/8] android: Rename CameraDevice::mutex_ 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" With the introduction of an additional mutex class member, the name of the existing one is too generic. Rename CameraDevice::mutex_ in CameraDevice::requestsMutex_ and use the libcamera provided libcamera::Mutex type to align the style with the rest of the code base. Signed-off-by: Jacopo Mondi Reviewed-by: Niklas Söderlund Reviewed-by: Hirokazu Honda --- src/android/camera_device.cpp | 4 ++-- src/android/camera_device.h | 3 ++- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index c6cd0b6e8be7..7f8c9bd7832d 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -2024,7 +2024,7 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques worker_.queueRequest(descriptor.request_.get()); { - MutexLocker lock(mutex_); + MutexLocker requestsLock(requestsMutex_); descriptors_[descriptor.request_->cookie()] = std::move(descriptor); } @@ -2035,7 +2035,7 @@ void CameraDevice::requestComplete(Request *request) { decltype(descriptors_)::node_type node; { - MutexLocker lock(mutex_); + MutexLocker requestsLock(requestsMutex_); auto it = descriptors_.find(request->cookie()); if (it == descriptors_.end()) { /* diff --git a/src/android/camera_device.h b/src/android/camera_device.h index f263fdae472a..ed992ae56d5d 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -24,6 +24,7 @@ #include "libcamera/internal/buffer.h" #include "libcamera/internal/log.h" #include "libcamera/internal/message.h" +#include "libcamera/internal/thread.h" #include "camera_metadata.h" #include "camera_stream.h" @@ -134,7 +135,7 @@ private: std::map formatsMap_; std::vector streams_; - std::mutex mutex_; /* Protect descriptors_ */ + libcamera::Mutex requestsMutex_; /* Protects descriptors_. */ std::map descriptors_; std::string maker_; From patchwork Thu May 13 09:22:46 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 12270 X-Patchwork-Delegate: jacopo@jmondi.org 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 404C4C31EF for ; Thu, 13 May 2021 09:22:16 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 9A97968933; Thu, 13 May 2021 11:22:15 +0200 (CEST) Received: from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net [217.70.183.193]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B253168921 for ; Thu, 13 May 2021 11:22:12 +0200 (CEST) Received: from uno.LocalDomain (93-61-96-190.ip145.fastwebnet.it [93.61.96.190]) (Authenticated sender: jacopo@jmondi.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 47ECA240007; Thu, 13 May 2021 09:22:12 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Thu, 13 May 2021 11:22:46 +0200 Message-Id: <20210513092246.42847-9-jacopo@jmondi.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210513092246.42847-1-jacopo@jmondi.org> References: <20210513092246.42847-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 8/8] android: Implement flush() camera operation 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" Implement the flush() camera operation in the CameraDevice class and make it available to the camera framework by implementing the operation wrapper in camera_ops.cpp. The flush() implementation stops the Camera and the worker thread and waits for all in-flight requests to be returned. Stopping the Camera forces all Requests already queued to be returned immediately in error state. As flush() has to wait until all of them have been returned, make it wait on a newly introduced condition variable which is notified by the request completion handler when the queue of pending requests has been exhausted. As flush() can race with processCaptureRequest() protect the requests queueing by introducing a new CameraState::CameraFlushing state that processCaptureRequest() inspects before queuing the Request to the Camera. If flush() has been called while processCaptureRequest() was executing, return the current Request immediately in error state. Signed-off-by: Jacopo Mondi --- src/android/camera_device.cpp | 91 +++++++++++++++++++++++++++++++++-- src/android/camera_device.h | 9 +++- src/android/camera_ops.cpp | 8 ++- 3 files changed, 102 insertions(+), 6 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 7f8c9bd7832d..6d6730a50ec7 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -750,16 +750,64 @@ int CameraDevice::open(const hw_module_t *hardwareModule) void CameraDevice::close() { - streams_.clear(); + MutexLocker cameraLock(cameraMutex_); + if (state_ == CameraFlushing) { + MutexLocker flushLock(flushMutex_); + flushed_.wait(flushLock, [&] { return state_ != CameraStopped; }); + camera_->release(); + return; + } + + streams_.clear(); stop(); camera_->release(); } +/* + * Flush is similar to stop() but sets the camera state to 'flushing' and wait + * until all the in-flight requests have been returned before setting the + * camera state to stopped. + * + * Once flushing is done it unlocks concurrent camera close() calls or new + * camera configurations. + */ +void CameraDevice::flush() +{ + { + MutexLocker cameraLock(cameraMutex_); + + if (state_ != CameraRunning) + return; + + worker_.stop(); + camera_->stop(); + state_ = CameraFlushing; + } + + /* + * Now wait for all the in-flight requests to be completed before + * continuing. Stopping the Camera guarantees that all in-flight + * requests are completed in error state. + */ + { + MutexLocker requestsLock(requestsMutex_); + flushing_.wait(requestsLock, [&] { return descriptors_.empty(); }); + } + + /* + * Unlock close() or configureStreams() that might be waiting for + * flush to be completed. + */ + MutexLocker flushLocker(flushMutex_); + state_ = CameraStopped; + flushed_.notify_one(); +} + +/* Calls to stop() must be protected by cameraMutex_ being held by the caller. */ void CameraDevice::stop() { - MutexLocker cameraLock(cameraMutex_); if (state_ == CameraStopped) return; @@ -1652,8 +1700,20 @@ PixelFormat CameraDevice::toPixelFormat(int format) const */ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list) { - /* Before any configuration attempt, stop the camera. */ - stop(); + { + /* + * If a flush is in progress, wait for it to complete and to + * stop the camera, otherwise before any new configuration + * attempt we have to stop the camera explictely. + */ + MutexLocker cameraLock(cameraMutex_); + if (state_ == CameraFlushing) { + MutexLocker flushLock(flushMutex_); + flushed_.wait(flushLock, [&] { return state_ != CameraStopped; }); + } else { + stop(); + } + } if (stream_list->num_streams == 0) { LOG(HAL, Error) << "No streams in configuration"; @@ -2021,6 +2081,25 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques if (ret) return ret; + /* + * Just before queuing the request, make sure flush() has not + * been called after this function has been executed. In that + * case, immediately return the request with errors. + */ + MutexLocker cameraLock(cameraMutex_); + if (state_ == CameraFlushing || state_ == CameraStopped) { + for (camera3_stream_buffer_t &buffer : descriptor.buffers_) { + buffer.status = CAMERA3_BUFFER_STATUS_ERROR; + buffer.release_fence = buffer.acquire_fence; + } + + notifyError(descriptor.frameNumber_, + descriptor.buffers_[0].stream, + CAMERA3_MSG_ERROR_REQUEST); + + return 0; + } + worker_.queueRequest(descriptor.request_.get()); { @@ -2050,6 +2129,10 @@ void CameraDevice::requestComplete(Request *request) return; } + /* Release flush if all the pending requests have been completed. */ + if (descriptors_.empty()) + flushing_.notify_one(); + node = descriptors_.extract(it); } Camera3RequestDescriptor &descriptor = node.mapped(); diff --git a/src/android/camera_device.h b/src/android/camera_device.h index ed992ae56d5d..9c55cc2674b7 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -7,6 +7,7 @@ #ifndef __ANDROID_CAMERA_DEVICE_H__ #define __ANDROID_CAMERA_DEVICE_H__ +#include #include #include #include @@ -42,6 +43,7 @@ public: int open(const hw_module_t *hardwareModule); void close(); + void flush(); unsigned int id() const { return id_; } camera3_device_t *camera3Device() { return &camera3Device_; } @@ -92,6 +94,7 @@ private: enum State { CameraStopped, CameraRunning, + CameraFlushing, }; void stop(); @@ -124,6 +127,9 @@ private: libcamera::Mutex cameraMutex_; /* Protects access to the camera state. */ State state_; + libcamera::Mutex flushMutex_; /* Protects the flushed_ condition variable. */ + std::condition_variable flushed_; + std::shared_ptr camera_; std::unique_ptr config_; @@ -135,8 +141,9 @@ private: std::map formatsMap_; std::vector streams_; - libcamera::Mutex requestsMutex_; /* Protects descriptors_. */ + libcamera::Mutex requestsMutex_; /* Protects descriptors_ and flushing_. */ std::map descriptors_; + std::condition_variable flushing_; std::string maker_; std::string model_; diff --git a/src/android/camera_ops.cpp b/src/android/camera_ops.cpp index 696e80436821..8a3cfa175ff5 100644 --- a/src/android/camera_ops.cpp +++ b/src/android/camera_ops.cpp @@ -66,8 +66,14 @@ static void hal_dev_dump([[maybe_unused]] const struct camera3_device *dev, { } -static int hal_dev_flush([[maybe_unused]] const struct camera3_device *dev) +static int hal_dev_flush(const struct camera3_device *dev) { + if (!dev) + return -EINVAL; + + CameraDevice *camera = reinterpret_cast(dev->priv); + camera->flush(); + return 0; }