From patchwork Tue Feb 26 02:18:54 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: 626 Return-Path: Received: from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net [195.74.38.228]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 02E08601E2 for ; Tue, 26 Feb 2019 03:19:24 +0100 (CET) X-Halon-ID: ecc5dd6a-396c-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 ecc5dd6a-396c-11e9-985a-005056917f90; Tue, 26 Feb 2019 03:19:22 +0100 (CET) From: =?utf-8?q?Niklas_S=C3=B6derlund?= To: libcamera-devel@lists.libcamera.org Date: Tue, 26 Feb 2019 03:18:54 +0100 Message-Id: <20190226021857.28255-6-niklas.soderlund@ragnatech.se> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20190226021857.28255-1-niklas.soderlund@ragnatech.se> References: <20190226021857.28255-1-niklas.soderlund@ragnatech.se> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 5/8] 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: Tue, 26 Feb 2019 02:19:25 -0000 There is a need to better control the order of operations an application perform 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 four states are added; Disconnected, Free, Acquired 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 can perform. * Disconnected Is the lowest state a camera can be in. It indicates that the camera have been disconnected from the system and the only operations an application shall do at this point is clean up and release the camera so it can be removed from libcamera as well. * Free Is the base state of a camera, an application can inspect the properties of the camera to determine if it wish to use it or not. If an application wish to use a camera it should acquire it to proceed to the next state. * Acquired When an application have acquired a camera it have exclusive access to it and can modify the cameras parameters to prepare it for capturing. Once the application is done configure the camera it may be started to progress to the running state. Once the camera is started it can not be reconfigured until it's stopped. * Running In this state the camera is running and able to process requests queued to it by the application. Once an application finish capturing the camera shall be stopped to change the state to acquired where it can be reconfigured or released. Signed-off-by: Niklas Söderlund --- include/libcamera/camera.h | 14 ++++- src/libcamera/camera.cpp | 110 +++++++++++++++++++++++-------------- 2 files changed, 80 insertions(+), 44 deletions(-) diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h index bf70255a6a5ea364..8c8545b074e8ae13 100644 --- a/include/libcamera/camera.h +++ b/include/libcamera/camera.h @@ -55,20 +55,28 @@ public: int stop(); private: + enum State { + Disconnected, + Free, + Acquired, + Running, + }; + Camera(PipelineHandler *pipe, const std::string &name); ~Camera(); + bool stateIs(State state) const; + bool stateIsAtleast(State state) const; + friend class PipelineHandler; void disconnect(); - int exclusiveAccess(); std::shared_ptr pipe_; std::string name_; std::vector streams_; std::vector activeStreams_; - bool acquired_; - bool disconnected_; + State state_; }; } /* namespace libcamera */ diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp index d4258fe3c7551af3..c50b14bbd904fc1c 100644 --- a/src/libcamera/camera.cpp +++ b/src/libcamera/camera.cpp @@ -116,17 +116,47 @@ 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), state_(Free) { } Camera::~Camera() { - if (acquired_) + if (state_ > Free) LOG(Camera, Error) << "Removing camera while still in use"; } +static const std::string stateNames[] = { + "Disconnected", + "Free", + "Acquired", + "Running", +}; + +bool Camera::stateIs(State state) const +{ + if (state_ == state) + return true; + + LOG(Camera, Error) << "Camera in " << stateNames[state_] + << " state trying operation requiering " + << stateNames[state] << " state"; + + return false; +} + +bool Camera::stateIsAtleast(State state) const +{ + if (state_ >= state) + return true; + + LOG(Camera, Error) << "Camera in " << stateNames[state_] + << " state trying operation requiering at least " + << stateNames[state] << " state"; + + return false; +} + /** * \brief Notify camera disconnection * @@ -140,7 +170,7 @@ void Camera::disconnect() { LOG(Camera, Debug) << "Disconnecting camera " << name_; - disconnected_ = true; + state_ = Disconnected; disconnected.emit(this); } @@ -162,10 +192,11 @@ void Camera::disconnect() */ int Camera::acquire() { - if (acquired_) + if (!stateIs(Free)) return -EBUSY; - acquired_ = true; + state_ = Acquired; + return 0; } @@ -177,7 +208,10 @@ int Camera::acquire() */ void Camera::release() { - acquired_ = false; + if (!stateIs(Acquired)) + return; + + state_ = Free; } /** @@ -191,6 +225,9 @@ void Camera::release() */ const std::vector &Camera::streams() const { + if (!stateIsAtleast(Free)) + std::vector{}; + return streams_; } @@ -213,7 +250,7 @@ const std::vector &Camera::streams() const std::map Camera::streamConfiguration(std::vector &streams) { - if (disconnected_ || !streams.size()) + if (!stateIsAtleast(Free) || !streams.size()) return std::map{}; return pipe_->streamConfiguration(this, streams); @@ -244,9 +281,8 @@ int Camera::configureStreams(std::map &config) { int ret; - ret = exclusiveAccess(); - if (ret) - return ret; + if (!stateIs(Acquired)) + return -EACCES; if (!config.size()) { LOG(Camera, Error) @@ -284,11 +320,8 @@ int Camera::configureStreams(std::map &config) */ int Camera::allocateBuffers() { - int ret; - - ret = exclusiveAccess(); - if (ret) - return ret; + if (!stateIs(Acquired)) + return -EACCES; if (activeStreams_.empty()) { LOG(Camera, Error) @@ -297,7 +330,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(); @@ -313,6 +346,9 @@ int Camera::allocateBuffers() */ void Camera::freeBuffers() { + if (!stateIs(Acquired)) + return; + for (Stream *stream : activeStreams_) { if (!stream->bufferPool().count()) continue; @@ -336,7 +372,7 @@ void Camera::freeBuffers() */ Request *Camera::createRequest() { - if (exclusiveAccess()) + if (!stateIsAtleast(Acquired)) return nullptr; return new Request(this); @@ -358,13 +394,10 @@ Request *Camera::createRequest() */ int Camera::queueRequest(Request *request) { - int ret; + if (!stateIs(Running)) + return -EACCES; - ret = exclusiveAccess(); - if (ret) - return ret; - - ret = request->prepare(); + int ret = request->prepare(); if (ret) { LOG(Camera, Error) << "Failed to prepare request"; return ret; @@ -385,13 +418,18 @@ int Camera::queueRequest(Request *request) */ int Camera::start() { - int ret = exclusiveAccess(); - if (ret) - return ret; + if (!stateIs(Acquired)) + return -EACCES; LOG(Camera, Debug) << "Starting capture"; - return pipe_->start(this); + int ret = pipe_->start(this); + if (ret) + return ret; + + state_ = Running; + + return 0; } /** @@ -405,24 +443,14 @@ int Camera::start() */ int Camera::stop() { - int ret = exclusiveAccess(); - if (ret) - return ret; + if (!stateIs(Running)) + return -EACCES; LOG(Camera, Debug) << "Stopping capture"; pipe_->stop(this); - return 0; -} - -int Camera::exclusiveAccess() -{ - if (disconnected_) - return -ENODEV; - - if (!acquired_) - return -EACCES; + state_ = Acquired; return 0; }