From patchwork Fri May 21 15:42:27 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jacopo Mondi X-Patchwork-Id: 12358 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 D2E9AC31FF for ; Fri, 21 May 2021 15:41:58 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 8E87D68920; Fri, 21 May 2021 17:41:58 +0200 (CEST) Received: from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net [217.70.183.201]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 449616891B for ; Fri, 21 May 2021 17:41:53 +0200 (CEST) Received: (Authenticated sender: jacopo@jmondi.org) by relay8-d.mail.gandi.net (Postfix) with ESMTPSA id B66911BF215; Fri, 21 May 2021 15:41:52 +0000 (UTC) From: Jacopo Mondi To: libcamera-devel@lists.libcamera.org Date: Fri, 21 May 2021 17:42:27 +0200 Message-Id: <20210521154227.60186-9-jacopo@jmondi.org> X-Mailer: git-send-email 2.31.1 In-Reply-To: <20210521154227.60186-1-jacopo@jmondi.org> References: <20210521154227.60186-1-jacopo@jmondi.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 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. Protect potentially concurrent calls to close() and configureStreams() by inspecting the CameraState, and force a wait for any flush() call to complete before proceeding. Signed-off-by: Jacopo Mondi Reviewed-by: Hirokazu Honda --- src/android/camera_device.cpp | 90 +++++++++++++++++++++++++++++++++-- src/android/camera_device.h | 9 +++- src/android/camera_ops.cpp | 8 +++- 3 files changed, 100 insertions(+), 7 deletions(-) diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 3fce14035718..899afaa49439 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -750,16 +750,65 @@ int CameraDevice::open(const hw_module_t *hardwareModule) void CameraDevice::close() { - streams_.clear(); + MutexLocker cameraLock(cameraMutex_); + if (state_ == CameraFlushing) { + flushed_.wait(cameraLock, [&] { return state_ != CameraStopped; }); + camera_->release(); + return; + } + + streams_.clear(); stop(); camera_->release(); } -void CameraDevice::stop() +/* + * 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 calls to camera close() and + * configureStreams(). + */ +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(); }); + } + + /* + * Set state to stopped and unlock close() or configureStreams() that + * might be waiting for flush to be completed. + */ MutexLocker cameraLock(cameraMutex_); + state_ = CameraStopped; + flushed_.notify_one(); +} + +/* Calls to stop() must be protected by cameraMutex_ being held by the caller. */ +void CameraDevice::stop() +{ + ASSERT(state_ != CameraFlushing); + if (state_ == CameraStopped) return; @@ -1581,8 +1630,18 @@ 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) + flushed_.wait(cameraLock, [&] { return state_ != CameraStopped; }); + else + stop(); + } if (stream_list->num_streams == 0) { LOG(HAL, Error) << "No streams in configuration"; @@ -1950,6 +2009,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()); { @@ -1979,6 +2057,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 7cf8e8370387..e1b3bf7d30f2 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(); @@ -120,8 +123,9 @@ private: CameraWorker worker_; - libcamera::Mutex cameraMutex_; /* Protects access to the camera state. */ + libcamera::Mutex cameraMutex_; /* Protects the camera state and flushed_. */ State state_; + std::condition_variable flushed_; std::shared_ptr camera_; std::unique_ptr config_; @@ -134,8 +138,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; }