From patchwork Thu Feb 28 02:16:25 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Niklas_S=C3=B6derlund?= X-Patchwork-Id: 652 Return-Path: Received: from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net [195.74.38.229]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7FDE6610B2 for ; Thu, 28 Feb 2019 03:16:39 +0100 (CET) X-Halon-ID: df931342-3afe-11e9-985a-005056917f90 Authorized-sender: niklas@soderlund.pp.se Received: from bismarck.berto.se (unknown [89.233.230.99]) by bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA id df931342-3afe-11e9-985a-005056917f90; Thu, 28 Feb 2019 03:16:36 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Thu, 28 Feb 2019 03:16:25 +0100 Message-Id: <20190228021626.15062-4-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190228021626.15062-1-niklas.soderlund@ragnatech.se> References: <20190228021626.15062-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 3/4] libcamera: camera: add state machine to control access from applications X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 28 Feb 2019 02:16:39 -0000 There is a need to better control the order of operations an application performs on a camera for it to function correctly. Add a basic state machine to ensure applications perform operations on the camera in good order. Internal to the Camera states are added; Available, Acquired, Configured, Prepared and Running. Each state represents a higher state of configuration of the camera ultimately leading to the highest state where the camera is capturing frames. Each state supports a subset of operations the application may perform. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- include/libcamera/camera.h | 18 ++- src/libcamera/camera.cpp | 230 ++++++++++++++++++++++++++++++------- 2 files changed, 203 insertions(+), 45 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index 9c8ae01ed5c607f1..e5212cf05d221279 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -39,7 +39,7 @@ public: Signal disconnected; int acquire(); - void release(); + int release(); const std::set &streams() const; std::map @@ -47,7 +47,7 @@ public: int configureStreams(std::map &config); int allocateBuffers(); - void freeBuffers(); + int freeBuffers(); Request *createRequest(); int queueRequest(Request *request); @@ -56,20 +56,30 @@ public: int stop(); private: + enum State { + CameraAvailable, + CameraAcquired, + CameraConfigured, + CameraPrepared, + CameraRunning, + }; + Camera(PipelineHandler *pipe, const std::string &name); ~Camera(); + bool stateBetween(State low, State high) const; + bool stateIs(State state) const; + friend class PipelineHandler; void disconnect(); - int exclusiveAccess(); std::shared_ptr pipe_; std::string name_; std::set streams_; std::set activeStreams_; - bool acquired_; bool disconnected_; + State state_; }; } /* namespace libcamera */ diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index 84b97b5c2ce94ecf..4f0833300b9b7ffc 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -52,6 +52,78 @@ LOG_DECLARE_CATEGORY(Camera) * created with the create() function which returns a shared pointer. The * Camera constructors and destructor are private, to prevent instances from * being constructed and destroyed manually. + * + * \section camera_operation Operating the camera + * + * An application needs to preform a sequence of operations on a camera before + * it is ready to process requests. The camera needs to be acquired, configured + * and resources allocated or import to prepare it for start. Once started the + * camera can process requests until it's stopped. When an application is done + * with a camera all resources allocated to process requests needs to be freed + * and the camera released. + * + * An application may start and stop a camera multiple times as long as it's + * not released. The camera may also be reconfigured provided that all + * resources allocated are freed prior to the reconfiguration. + * + * \subsection Camera States + * + * To help manage the sequence of operation need to control the camera a set + * of states are defined. Each state describes which operations may be preformed + * on the camera. + * + * \dot + * digraph camera_state_machine { + * node [shape = doublecircle ]; Available; + * node [shape = circle ]; Acquired; + * node [shape = circle ]; Configured; + * node [shape = circle ]; Prepared; + * node [shape = circle ]; Running; + * + * Available -> Available [label = "release(), streams(), streamConfiguration()"]; + * Available -> Acquired [label = "acquire()"]; + * + * Acquired -> Available [label = "release()"]; + * Acquired -> Acquired [label = "streams(), streamConfiguration()"]; + * Acquired -> Configured [label = "configureStreams()"]; + * + * Configured -> Available [label = "release()"]; + * Configured -> Configured [label = "streams(), streamConfiguration(), configureStreams()"]; + * Configured -> Prepared [label = "allocateBuffers()"]; + * + * Prepared -> Configured [label = "freeBuffers()"]; + * Prepared -> Prepared [label = "streams(), streamConfiguration(), createRequest()"]; + * Prepared -> Running [label = "start()"]; + * + * Running -> Prepared [label = "stop()"]; + * Running -> Running [label = "streams(), streamConfiguration(), createRequest(), queueRequest()"]; + * } + * \enddot + * + * \subsubsection Available + * The base state of a camera, an application can inspect the properties of the + * camera to determine if it wishes to use it. If an application wishes to use + * a camera it should acquire it to proceed to the acquired state. + * + * \subsubsection Acquired + * In the acquired state an application have exclusive access to the camera and + * may modify the camera's parameters to configure it and proceed to the + * configured state. + * + * \subsubsection Configured + * The camera is configured and ready for the application to prepare it with + * resources. The camera may be reconfigured multiple times until resources + * are provided and the state progressed to provided. + * + * \subsubsection Prepared + * Camera have been configured and provided with resource and is ready to be + * started. The application may free the camera's resources to get back to the + * configured state or start it to progress to the running state. + * + * \subsubsection Running + * The camera is running and ready to process requests queued by the + * application. The camera remains in this state until it's stopped and moved + * to the prepared state. */ /** @@ -116,17 +188,42 @@ const std::string &Camera::name() const */ Camera::Camera(PipelineHandler *pipe, const std::string &name) - : pipe_(pipe->shared_from_this()), name_(name), acquired_(false), - disconnected_(false) + : pipe_(pipe->shared_from_this()), name_(name), disconnected_(false), + state_(CameraAvailable) { } Camera::~Camera() { - if (acquired_) + if (!stateIs(CameraAvailable)) LOG(Camera, Error) << "Removing camera while still in use"; } +bool Camera::stateBetween(State low, State high) const +{ + static const std::string stateNames[] = { + "Available", + "Acquired", + "Configured", + "Prepared", + "Running", + }; + + if (state_ >= low && state_ <= high) + return true; + + LOG(Camera, Debug) << "Camera in " << stateNames[state_] + << " state trying operation requiring state between " + << stateNames[low] << " and " << stateNames[high]; + + return false; +} + +bool Camera::stateIs(State state) const +{ + return stateBetween(state, state); +} + /** * \brief Notify camera disconnection * @@ -140,6 +237,17 @@ void Camera::disconnect() { LOG(Camera, Debug) << "Disconnecting camera " << name_; + /* + * If the camera was running when the hardware was removed force the + * state to prepared to allow applications to call freeBuffers() and + * release() before deleting the camera. + * + * \todo: Update comment when importing buffers as well as allocating + * them are supported. + */ + if (state_ == CameraRunning) + state_ = CameraPrepared; + disconnected_ = true; disconnected.emit(this); } @@ -158,13 +266,19 @@ void Camera::disconnect() * \todo Implement exclusive access across processes. * * \return 0 on success or a negative error code otherwise + * \retval -ENODEV The camera is no longer connected to the hardware + * \retval -EBUSY The camera is not free and can't be acquired by the caller */ int Camera::acquire() { - if (acquired_) + if (disconnected_) + return -ENODEV; + + if (!stateIs(CameraAvailable)) return -EBUSY; - acquired_ = true; + state_ = CameraAcquired; + return 0; } @@ -173,10 +287,18 @@ int Camera::acquire() * * Releasing the camera device allows other users to acquire exclusive access * with the acquire() function. + * + * \return 0 on success or a negative error code otherwise + * \retval -EBUSY The camera is running and can't be released */ -void Camera::release() +int Camera::release() { - acquired_ = false; + if (!stateBetween(CameraAvailable, CameraConfigured)) + return -EBUSY; + + state_ = CameraAvailable; + + return 0; } /** @@ -236,17 +358,19 @@ Camera::streamConfiguration(std::set &streams) * to calling this function, otherwise an -EACCES error will be returned. * * \return 0 on success or a negative error code otherwise - * \retval -ENODEV The camera is not connected to any hardware - * \retval -EACCES The user has not acquired exclusive access to the camera + * \retval -ENODEV The camera is no longer connected to the hardware + * \retval -EACCES The camera is not in a state where it can be configured * \retval -EINVAL The configuration is not valid */ int Camera::configureStreams(std::map &config) { int ret; - ret = exclusiveAccess(); - if (ret) - return ret; + if (disconnected_) + return -ENODEV; + + if (!stateBetween(CameraAvailable, CameraConfigured)) + return -EACCES; if (!config.size()) { LOG(Camera, Error) @@ -273,20 +397,25 @@ int Camera::configureStreams(std::map &config) stream->bufferPool().createBuffers(cfg.bufferCount); } + state_ = CameraConfigured; + return 0; } /** * \brief Allocate buffers for all configured streams * \return 0 on success or a negative error code otherwise + * \retval -ENODEV The camera is no longer connected to the hardware + * \retval -EACCES The camera is not in a state where buffers can be allocated + * \retval -EINVAL The configuration is not valid */ int Camera::allocateBuffers() { - int ret; + if (disconnected_) + return -ENODEV; - ret = exclusiveAccess(); - if (ret) - return ret; + if (!stateIs(CameraConfigured)) + return -EACCES; if (activeStreams_.empty()) { LOG(Camera, Error) @@ -295,7 +424,7 @@ int Camera::allocateBuffers() } for (Stream *stream : activeStreams_) { - ret = pipe_->allocateBuffers(this, stream); + int ret = pipe_->allocateBuffers(this, stream); if (ret) { LOG(Camera, Error) << "Failed to allocate buffers"; freeBuffers(); @@ -303,14 +432,22 @@ int Camera::allocateBuffers() } } + state_ = CameraPrepared; + return 0; } /** * \brief Release all buffers from allocated pools in each stream + * + * \return 0 on success or a negative error code otherwise + * \retval -EACCES The camera is not in a state where buffers can be freed */ -void Camera::freeBuffers() +int Camera::freeBuffers() { + if (!stateIs(CameraPrepared)) + return -EACCES; + for (Stream *stream : activeStreams_) { if (!stream->bufferPool().count()) continue; @@ -318,6 +455,10 @@ void Camera::freeBuffers() pipe_->freeBuffers(this, stream); stream->bufferPool().destroyBuffers(); } + + state_ = CameraConfigured; + + return 0; } /** @@ -333,7 +474,7 @@ void Camera::freeBuffers() */ Request *Camera::createRequest() { - if (exclusiveAccess()) + if (disconnected_ || !stateBetween(CameraPrepared, CameraRunning)) return nullptr; return new Request(this); @@ -351,16 +492,18 @@ Request *Camera::createRequest() * automatically after it completes. * * \return 0 on success or a negative error code otherwise + * \retval -ENODEV The camera is no longer connected to the hardware + * \retval -EACCES The camera is not running so requests can't be queued */ int Camera::queueRequest(Request *request) { - int ret; + if (disconnected_) + return -ENODEV; - ret = exclusiveAccess(); - if (ret) - return ret; + if (!stateIs(CameraRunning)) + return -EACCES; - ret = request->prepare(); + int ret = request->prepare(); if (ret) { LOG(Camera, Error) << "Failed to prepare request"; return ret; @@ -377,16 +520,26 @@ int Camera::queueRequest(Request *request) * until the capture session is terminated with \a stop(). * * \return 0 on success or a negative error code otherwise + * \retval -ENODEV The camera is no longer connected to the hardware + * \retval -EACCES The camera is not in a state where it can be started */ int Camera::start() { - int ret = exclusiveAccess(); - if (ret) - return ret; + if (disconnected_) + return -ENODEV; + + if (!stateIs(CameraPrepared)) + return -EACCES; LOG(Camera, Debug) << "Starting capture"; - return pipe_->start(this); + int ret = pipe_->start(this); + if (ret) + return ret; + + state_ = CameraRunning; + + return 0; } /** @@ -396,27 +549,22 @@ int Camera::start() * requests are cancelled and complete synchronously in an error state. * * \return 0 on success or a negative error code otherwise + * \retval -ENODEV The camera is no longer connected to the hardware + * \retval -EACCES The camera is not running so can't be stopped */ int Camera::stop() { - int ret = exclusiveAccess(); - if (ret) - return ret; + if (disconnected_) + return -ENODEV; + + if (!stateIs(CameraRunning)) + return -EACCES; LOG(Camera, Debug) << "Stopping capture"; pipe_->stop(this); - return 0; -} - -int Camera::exclusiveAccess() -{ - if (disconnected_) - return -ENODEV; - - if (!acquired_) - return -EACCES; + state_ = CameraPrepared; return 0; }