[libcamera-devel] [PATCH v3 3/3] libcamera: camera: add state machine to control access from applications
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Feb 28 22:39:47 CET 2019
Hi Niklas,
Thank you for the patch.
On Thu, Feb 28, 2019 at 07:51:26PM +0100, Niklas Söderlund wrote:
> 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 <niklas.soderlund at ragnatech.se>
> ---
> include/libcamera/camera.h | 18 ++-
> src/libcamera/camera.cpp | 266 +++++++++++++++++++++++++++++++------
> 2 files changed, 238 insertions(+), 46 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<Camera *> disconnected;
>
> int acquire();
> - void release();
> + int release();
>
> const std::set<Stream *> &streams() const;
> std::map<Stream *, StreamConfiguration>
> @@ -47,7 +47,7 @@ public:
> int configureStreams(std::map<Stream *, StreamConfiguration> &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<PipelineHandler> pipe_;
> std::string name_;
> std::set<Stream *> streams_;
> std::set<Stream *> activeStreams_;
>
> - bool acquired_;
> bool disconnected_;
> + State state_;
> };
>
> } /* namespace libcamera */
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 84b97b5c2ce94ecf..0b06c0d838b356f3 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -11,6 +11,7 @@
>
> #include "log.h"
> #include "pipeline_handler.h"
> +#include "utils.h"
>
> /**
> * \file camera.h
> @@ -42,6 +43,9 @@ LOG_DECLARE_CATEGORY(Camera)
> * \class Camera
> * \brief Camera device
> *
> + * \todo Add documentation for camera start timings. What exactly does the
> + * camera expect the pipeline handler to do when start() is called?
> + *
> * The Camera class models a camera capable of producing one or more image
> * streams from a single image source. It provides the main interface to
> * configuring and controlling the device, and capturing image streams. It is
> @@ -52,6 +56,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 perform 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 imported to prepare the camera for capture. Once
> + * started the camera can process requests until it is stopped. When an
> + * application is done with a camera all resources allocated needs to be freed
s/needs/need/
> + * and the camera released.
> + *
> + * An application may start and stop a camera multiple times as long as it is
> + * 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 operations needed to control the camera a set
> + * of states are defined. Each state describes which operations may be performed
> + * on the camera. Operations not listed in the state diagram are allowed in all
> + * states.
> + *
> + * \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()"];
> + * Available -> Acquired [label = "acquire()"];
> + *
> + * Acquired -> Available [label = "release()"];
> + * Acquired -> Configured [label = "configureStreams()"];
> + *
> + * Configured -> Available [label = "release()"];
> + * Configured -> Configured [label = "configureStreams()"];
> + * Configured -> Prepared [label = "allocateBuffers()"];
> + *
> + * Prepared -> Configured [label = "freeBuffers()"];
> + * Prepared -> Prepared [label = "createRequest()"];
> + * Prepared -> Running [label = "start()"];
> + *
> + * Running -> Prepared [label = "stop()"];
> + * Running -> Running [label = "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 has 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 progresses to Prepared.
> + *
> + * \subsubsection Prepared
> + * The camera has been configured and provided with resources 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.
s/start/start()/
> + *
> + * \subsubsection Running
> + * The camera is running and ready to process requests queued by the
> + * application. The camera remains in this state until it is stopped and moved
> + * to the Prepared state.
> */
>
> /**
> @@ -116,17 +192,55 @@ 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";
> }
>
> +static const char *const camera_state_name[] = {
Maybe camera_state_names ?
> + "Available",
> + "Acquired",
> + "Configured",
> + "Prepared",
> + "Running",
> +};
> +
> +bool Camera::stateBetween(State low, State high) const
> +{
> + if (state_ >= low && state_ <= high)
> + return true;
> +
> + ASSERT(static_cast<unsigned int>(low) < ARRAY_SIZE(camera_state_name) &&
> + static_cast<unsigned int>(high) < ARRAY_SIZE(camera_state_name));
> +
> + LOG(Camera, Debug) << "Camera in " << camera_state_name[state_]
> + << " state trying operation requiring state between "
> + << camera_state_name[low] << " and "
> + << camera_state_name[high];
> +
> + return false;
> +}
> +
> +bool Camera::stateIs(State state) const
> +{
> + if (state_ == state)
> + return true;
> +
> + ASSERT(static_cast<unsigned int>(state) < ARRAY_SIZE(camera_state_name));
> +
> + LOG(Camera, Debug) << "Camera in " << camera_state_name[state_]
> + << " state trying operation requiring state "
> + << camera_state_name[state];
> +
> + return false;
> +}
> +
> /**
> * \brief Notify camera disconnection
> *
> @@ -135,11 +249,24 @@ Camera::~Camera()
> * instance notifies the application by emitting the #disconnected signal, and
> * ensures that all new calls to the application-facing Camera API return an
> * error immediately.
> + *
> + * \todo: Deal with pending requests if the camera is disconnected in a
> + * running state.
> + * \todo: Update comment about Running state when importing buffers as well as
> + * allocating them are supported.
No need for colons after any of the \todo tags.
> */
> 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.
> + */
> + if (state_ == CameraRunning)
> + state_ = CameraPrepared;
> +
> disconnected_ = true;
> disconnected.emit(this);
> }
> @@ -155,16 +282,24 @@ void Camera::disconnect()
> * Once exclusive access isn't needed anymore, the device should be released
> * with a call to the release() function.
> *
> + * This function effects the state of the camera, see \ref camera_operation.
s/effects/affects/ (through the whole file)
> + *
> * \todo Implement exclusive access across processes.
> *
> * \return 0 on success or a negative error code otherwise
> + * \retval -ENODEV The camera has been disconnected from the system
> + * \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 +308,20 @@ int Camera::acquire()
> *
> * Releasing the camera device allows other users to acquire exclusive access
> * with the acquire() function.
> + *
> + * This function effects the state of the camera, see \ref camera_operation.
> + *
> + * \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;
> }
>
> /**
> @@ -235,18 +380,22 @@ Camera::streamConfiguration(std::set<Stream *> &streams)
> * Exclusive access to the camera shall be ensured by a call to acquire() prior
> * to calling this function, otherwise an -EACCES error will be returned.
> *
> + * This function effects the state of the camera, see \ref camera_operation.
> + *
> * \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 has been disconnected from the system
> + * \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<Stream *, StreamConfiguration> &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 +422,28 @@ int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)
> stream->bufferPool().createBuffers(cfg.bufferCount);
> }
>
> + state_ = CameraConfigured;
> +
> return 0;
> }
>
> /**
> * \brief Allocate buffers for all configured streams
> + *
> + * This function effects the state of the camera, see \ref camera_operation.
> + *
> * \return 0 on success or a negative error code otherwise
> + * \retval -ENODEV The camera has been disconnected from the system
> + * \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 +452,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 +460,24 @@ int Camera::allocateBuffers()
> }
> }
>
> + state_ = CameraPrepared;
> +
> return 0;
> }
>
> /**
> * \brief Release all buffers from allocated pools in each stream
> + *
> + * This function effects the state of the camera, see \ref camera_operation.
> + *
> + * \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 +485,10 @@ void Camera::freeBuffers()
> pipe_->freeBuffers(this, stream);
> stream->bufferPool().destroyBuffers();
> }
> +
> + state_ = CameraConfigured;
> +
> + return 0;
> }
>
> /**
> @@ -333,7 +504,7 @@ void Camera::freeBuffers()
> */
> Request *Camera::createRequest()
Should you update the documentation of this function ?
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> {
> - if (exclusiveAccess())
> + if (disconnected_ || !stateBetween(CameraPrepared, CameraRunning))
> return nullptr;
>
> return new Request(this);
> @@ -351,16 +522,18 @@ Request *Camera::createRequest()
> * automatically after it completes.
> *
> * \return 0 on success or a negative error code otherwise
> + * \retval -ENODEV The camera has been disconnected from the system
> + * \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;
> @@ -376,17 +549,29 @@ int Camera::queueRequest(Request *request)
> * can queue requests to the camera to process and return to the application
> * until the capture session is terminated with \a stop().
> *
> + * This function effects the state of the camera, see \ref camera_operation.
> + *
> * \return 0 on success or a negative error code otherwise
> + * \retval -ENODEV The camera has been disconnected from the system
> + * \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;
> }
>
> /**
> @@ -395,30 +580,27 @@ int Camera::start()
> * This method stops capturing and processing requests immediately. All pending
> * requests are cancelled and complete synchronously in an error state.
> *
> + * This function effects the state of the camera, see \ref camera_operation.
> + *
> * \return 0 on success or a negative error code otherwise
> + * \retval -ENODEV The camera has been disconnected from the system
> + * \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";
>
> + state_ = CameraPrepared;
> +
> pipe_->stop(this);
>
> return 0;
> }
>
> -int Camera::exclusiveAccess()
> -{
> - if (disconnected_)
> - return -ENODEV;
> -
> - if (!acquired_)
> - return -EACCES;
> -
> - return 0;
> -}
> -
> } /* namespace libcamera */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list