{"id":652,"url":"https://patchwork.libcamera.org/api/1.1/patches/652/?format=json","web_url":"https://patchwork.libcamera.org/patch/652/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20190228021626.15062-4-niklas.soderlund@ragnatech.se>","date":"2019-02-28T02:16:25","name":"[libcamera-devel,v2,3/4] libcamera: camera: add state machine to control access from applications","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"7d5b0115a61d4cac0ab43c0bd0e598338895837a","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/1.1/people/5/?format=json","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/652/mbox/","series":[{"id":195,"url":"https://patchwork.libcamera.org/api/1.1/series/195/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=195","date":"2019-02-28T02:16:23","name":"libcamera: improve validation of camera operations","version":2,"mbox":"https://patchwork.libcamera.org/series/195/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/652/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/652/checks/","tags":{},"headers":{"Return-Path":"","Received":["from bin-mail-out-06.binero.net (bin-mail-out-06.binero.net\n\t[195.74.38.229])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7FDE6610B2\n\tfor ;\n\tThu, 28 Feb 2019 03:16:39 +0100 (CET)","from bismarck.berto.se (unknown [89.233.230.99])\n\tby bin-vsp-out-02.atm.binero.net (Halon) with ESMTPA\n\tid df931342-3afe-11e9-985a-005056917f90;\n\tThu, 28 Feb 2019 03:16:36 +0100 (CET)"],"X-Halon-ID":"df931342-3afe-11e9-985a-005056917f90","Authorized-sender":"niklas@soderlund.pp.se","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","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v2 3/4] libcamera: camera: add state\n\tmachine to control access from applications","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Thu, 28 Feb 2019 02:16:39 -0000"},"content":"There is a need to better control the order of operations an application\nperforms on a camera for it to function correctly. Add a basic state\nmachine to ensure applications perform operations on the camera in good\norder.\n\nInternal to the Camera states are added; Available, Acquired,\nConfigured, Prepared and Running. Each state represents a higher state\nof configuration of the camera ultimately leading to the highest state\nwhere the camera is capturing frames. Each state supports a subset of\noperations the application may perform.\n\nSigned-off-by: Niklas Söderlund \n---\n include/libcamera/camera.h | 18 ++-\n src/libcamera/camera.cpp | 230 ++++++++++++++++++++++++++++++-------\n 2 files changed, 203 insertions(+), 45 deletions(-)","diff":"diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\nindex 9c8ae01ed5c607f1..e5212cf05d221279 100644\n--- a/include/libcamera/camera.h\n+++ b/include/libcamera/camera.h\n@@ -39,7 +39,7 @@ public:\n \tSignal disconnected;\n \n \tint acquire();\n-\tvoid release();\n+\tint release();\n \n \tconst std::set &streams() const;\n \tstd::map\n@@ -47,7 +47,7 @@ public:\n \tint configureStreams(std::map &config);\n \n \tint allocateBuffers();\n-\tvoid freeBuffers();\n+\tint freeBuffers();\n \n \tRequest *createRequest();\n \tint queueRequest(Request *request);\n@@ -56,20 +56,30 @@ public:\n \tint stop();\n \n private:\n+\tenum State {\n+\t\tCameraAvailable,\n+\t\tCameraAcquired,\n+\t\tCameraConfigured,\n+\t\tCameraPrepared,\n+\t\tCameraRunning,\n+\t};\n+\n \tCamera(PipelineHandler *pipe, const std::string &name);\n \t~Camera();\n \n+\tbool stateBetween(State low, State high) const;\n+\tbool stateIs(State state) const;\n+\n \tfriend class PipelineHandler;\n \tvoid disconnect();\n-\tint exclusiveAccess();\n \n \tstd::shared_ptr pipe_;\n \tstd::string name_;\n \tstd::set streams_;\n \tstd::set activeStreams_;\n \n-\tbool acquired_;\n \tbool disconnected_;\n+\tState state_;\n };\n \n } /* namespace libcamera */\ndiff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\nindex 84b97b5c2ce94ecf..4f0833300b9b7ffc 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -52,6 +52,78 @@ LOG_DECLARE_CATEGORY(Camera)\n * created with the create() function which returns a shared pointer. The\n * Camera constructors and destructor are private, to prevent instances from\n * being constructed and destroyed manually.\n+ *\n+ * \\section camera_operation Operating the camera\n+ *\n+ * An application needs to preform a sequence of operations on a camera before\n+ * it is ready to process requests. The camera needs to be acquired, configured\n+ * and resources allocated or import to prepare it for start. Once started the\n+ * camera can process requests until it's stopped. When an application is done\n+ * with a camera all resources allocated to process requests needs to be freed\n+ * and the camera released.\n+ *\n+ * An application may start and stop a camera multiple times as long as it's\n+ * not released. The camera may also be reconfigured provided that all\n+ * resources allocated are freed prior to the reconfiguration.\n+ *\n+ * \\subsection Camera States\n+ *\n+ * To help manage the sequence of operation need to control the camera a set\n+ * of states are defined. Each state describes which operations may be preformed\n+ * on the camera.\n+ *\n+ * \\dot\n+ * digraph camera_state_machine {\n+ * node [shape = doublecircle ]; Available;\n+ * node [shape = circle ]; Acquired;\n+ * node [shape = circle ]; Configured;\n+ * node [shape = circle ]; Prepared;\n+ * node [shape = circle ]; Running;\n+ *\n+ * Available -> Available [label = \"release(), streams(), streamConfiguration()\"];\n+ * Available -> Acquired [label = \"acquire()\"];\n+ *\n+ * Acquired -> Available [label = \"release()\"];\n+ * Acquired -> Acquired [label = \"streams(), streamConfiguration()\"];\n+ * Acquired -> Configured [label = \"configureStreams()\"];\n+ *\n+ * Configured -> Available [label = \"release()\"];\n+ * Configured -> Configured [label = \"streams(), streamConfiguration(), configureStreams()\"];\n+ * Configured -> Prepared [label = \"allocateBuffers()\"];\n+ *\n+ * Prepared -> Configured [label = \"freeBuffers()\"];\n+ * Prepared -> Prepared [label = \"streams(), streamConfiguration(), createRequest()\"];\n+ * Prepared -> Running [label = \"start()\"];\n+ *\n+ * Running -> Prepared [label = \"stop()\"];\n+ * Running -> Running [label = \"streams(), streamConfiguration(), createRequest(), queueRequest()\"];\n+ * }\n+ * \\enddot\n+ *\n+ * \\subsubsection Available\n+ * The base state of a camera, an application can inspect the properties of the\n+ * camera to determine if it wishes to use it. If an application wishes to use\n+ * a camera it should acquire it to proceed to the acquired state.\n+ *\n+ * \\subsubsection Acquired\n+ * In the acquired state an application have exclusive access to the camera and\n+ * may modify the camera's parameters to configure it and proceed to the\n+ * configured state.\n+ *\n+ * \\subsubsection Configured\n+ * The camera is configured and ready for the application to prepare it with\n+ * resources. The camera may be reconfigured multiple times until resources\n+ * are provided and the state progressed to provided.\n+ *\n+ * \\subsubsection Prepared\n+ * Camera have been configured and provided with resource and is ready to be\n+ * started. The application may free the camera's resources to get back to the\n+ * configured state or start it to progress to the running state.\n+ *\n+ * \\subsubsection Running\n+ * The camera is running and ready to process requests queued by the\n+ * application. The camera remains in this state until it's stopped and moved\n+ * to the prepared state.\n */\n \n /**\n@@ -116,17 +188,42 @@ const std::string &Camera::name() const\n */\n \n Camera::Camera(PipelineHandler *pipe, const std::string &name)\n-\t: pipe_(pipe->shared_from_this()), name_(name), acquired_(false),\n-\t disconnected_(false)\n+\t: pipe_(pipe->shared_from_this()), name_(name), disconnected_(false),\n+\t state_(CameraAvailable)\n {\n }\n \n Camera::~Camera()\n {\n-\tif (acquired_)\n+\tif (!stateIs(CameraAvailable))\n \t\tLOG(Camera, Error) << \"Removing camera while still in use\";\n }\n \n+bool Camera::stateBetween(State low, State high) const\n+{\n+\tstatic const std::string stateNames[] = {\n+\t\t\"Available\",\n+\t\t\"Acquired\",\n+\t\t\"Configured\",\n+\t\t\"Prepared\",\n+\t\t\"Running\",\n+\t};\n+\n+\tif (state_ >= low && state_ <= high)\n+\t\treturn true;\n+\n+\tLOG(Camera, Debug) << \"Camera in \" << stateNames[state_]\n+\t\t\t << \" state trying operation requiring state between \"\n+\t\t\t << stateNames[low] << \" and \" << stateNames[high];\n+\n+\treturn false;\n+}\n+\n+bool Camera::stateIs(State state) const\n+{\n+\treturn stateBetween(state, state);\n+}\n+\n /**\n * \\brief Notify camera disconnection\n *\n@@ -140,6 +237,17 @@ void Camera::disconnect()\n {\n \tLOG(Camera, Debug) << \"Disconnecting camera \" << name_;\n \n+\t/*\n+\t * If the camera was running when the hardware was removed force the\n+\t * state to prepared to allow applications to call freeBuffers() and\n+\t * release() before deleting the camera.\n+\t *\n+\t * \\todo: Update comment when importing buffers as well as allocating\n+\t * them are supported.\n+\t */\n+\tif (state_ == CameraRunning)\n+\t\tstate_ = CameraPrepared;\n+\n \tdisconnected_ = true;\n \tdisconnected.emit(this);\n }\n@@ -158,13 +266,19 @@ void Camera::disconnect()\n * \\todo Implement exclusive access across processes.\n *\n * \\return 0 on success or a negative error code otherwise\n+ * \\retval -ENODEV The camera is no longer connected to the hardware\n+ * \\retval -EBUSY The camera is not free and can't be acquired by the caller\n */\n int Camera::acquire()\n {\n-\tif (acquired_)\n+\tif (disconnected_)\n+\t\treturn -ENODEV;\n+\n+\tif (!stateIs(CameraAvailable))\n \t\treturn -EBUSY;\n \n-\tacquired_ = true;\n+\tstate_ = CameraAcquired;\n+\n \treturn 0;\n }\n \n@@ -173,10 +287,18 @@ int Camera::acquire()\n *\n * Releasing the camera device allows other users to acquire exclusive access\n * with the acquire() function.\n+ *\n+ * \\return 0 on success or a negative error code otherwise\n+ * \\retval -EBUSY The camera is running and can't be released\n */\n-void Camera::release()\n+int Camera::release()\n {\n-\tacquired_ = false;\n+\tif (!stateBetween(CameraAvailable, CameraConfigured))\n+\t\treturn -EBUSY;\n+\n+\tstate_ = CameraAvailable;\n+\n+\treturn 0;\n }\n \n /**\n@@ -236,17 +358,19 @@ Camera::streamConfiguration(std::set &streams)\n * to calling this function, otherwise an -EACCES error will be returned.\n *\n * \\return 0 on success or a negative error code otherwise\n- * \\retval -ENODEV The camera is not connected to any hardware\n- * \\retval -EACCES The user has not acquired exclusive access to the camera\n+ * \\retval -ENODEV The camera is no longer connected to the hardware\n+ * \\retval -EACCES The camera is not in a state where it can be configured\n * \\retval -EINVAL The configuration is not valid\n */\n int Camera::configureStreams(std::map &config)\n {\n \tint ret;\n \n-\tret = exclusiveAccess();\n-\tif (ret)\n-\t\treturn ret;\n+\tif (disconnected_)\n+\t\treturn -ENODEV;\n+\n+\tif (!stateBetween(CameraAvailable, CameraConfigured))\n+\t\treturn -EACCES;\n \n \tif (!config.size()) {\n \t\tLOG(Camera, Error)\n@@ -273,20 +397,25 @@ int Camera::configureStreams(std::map &config)\n \t\tstream->bufferPool().createBuffers(cfg.bufferCount);\n \t}\n \n+\tstate_ = CameraConfigured;\n+\n \treturn 0;\n }\n \n /**\n * \\brief Allocate buffers for all configured streams\n * \\return 0 on success or a negative error code otherwise\n+ * \\retval -ENODEV The camera is no longer connected to the hardware\n+ * \\retval -EACCES The camera is not in a state where buffers can be allocated\n+ * \\retval -EINVAL The configuration is not valid\n */\n int Camera::allocateBuffers()\n {\n-\tint ret;\n+\tif (disconnected_)\n+\t\treturn -ENODEV;\n \n-\tret = exclusiveAccess();\n-\tif (ret)\n-\t\treturn ret;\n+\tif (!stateIs(CameraConfigured))\n+\t\treturn -EACCES;\n \n \tif (activeStreams_.empty()) {\n \t\tLOG(Camera, Error)\n@@ -295,7 +424,7 @@ int Camera::allocateBuffers()\n \t}\n \n \tfor (Stream *stream : activeStreams_) {\n-\t\tret = pipe_->allocateBuffers(this, stream);\n+\t\tint ret = pipe_->allocateBuffers(this, stream);\n \t\tif (ret) {\n \t\t\tLOG(Camera, Error) << \"Failed to allocate buffers\";\n \t\t\tfreeBuffers();\n@@ -303,14 +432,22 @@ int Camera::allocateBuffers()\n \t\t}\n \t}\n \n+\tstate_ = CameraPrepared;\n+\n \treturn 0;\n }\n \n /**\n * \\brief Release all buffers from allocated pools in each stream\n+ *\n+ * \\return 0 on success or a negative error code otherwise\n+ * \\retval -EACCES The camera is not in a state where buffers can be freed\n */\n-void Camera::freeBuffers()\n+int Camera::freeBuffers()\n {\n+\tif (!stateIs(CameraPrepared))\n+\t\treturn -EACCES;\n+\n \tfor (Stream *stream : activeStreams_) {\n \t\tif (!stream->bufferPool().count())\n \t\t\tcontinue;\n@@ -318,6 +455,10 @@ void Camera::freeBuffers()\n \t\tpipe_->freeBuffers(this, stream);\n \t\tstream->bufferPool().destroyBuffers();\n \t}\n+\n+\tstate_ = CameraConfigured;\n+\n+\treturn 0;\n }\n \n /**\n@@ -333,7 +474,7 @@ void Camera::freeBuffers()\n */\n Request *Camera::createRequest()\n {\n-\tif (exclusiveAccess())\n+\tif (disconnected_ || !stateBetween(CameraPrepared, CameraRunning))\n \t\treturn nullptr;\n \n \treturn new Request(this);\n@@ -351,16 +492,18 @@ Request *Camera::createRequest()\n * automatically after it completes.\n *\n * \\return 0 on success or a negative error code otherwise\n+ * \\retval -ENODEV The camera is no longer connected to the hardware\n+ * \\retval -EACCES The camera is not running so requests can't be queued\n */\n int Camera::queueRequest(Request *request)\n {\n-\tint ret;\n+\tif (disconnected_)\n+\t\treturn -ENODEV;\n \n-\tret = exclusiveAccess();\n-\tif (ret)\n-\t\treturn ret;\n+\tif (!stateIs(CameraRunning))\n+\t\treturn -EACCES;\n \n-\tret = request->prepare();\n+\tint ret = request->prepare();\n \tif (ret) {\n \t\tLOG(Camera, Error) << \"Failed to prepare request\";\n \t\treturn ret;\n@@ -377,16 +520,26 @@ int Camera::queueRequest(Request *request)\n * until the capture session is terminated with \\a stop().\n *\n * \\return 0 on success or a negative error code otherwise\n+ * \\retval -ENODEV The camera is no longer connected to the hardware\n+ * \\retval -EACCES The camera is not in a state where it can be started\n */\n int Camera::start()\n {\n-\tint ret = exclusiveAccess();\n-\tif (ret)\n-\t\treturn ret;\n+\tif (disconnected_)\n+\t\treturn -ENODEV;\n+\n+\tif (!stateIs(CameraPrepared))\n+\t\treturn -EACCES;\n \n \tLOG(Camera, Debug) << \"Starting capture\";\n \n-\treturn pipe_->start(this);\n+\tint ret = pipe_->start(this);\n+\tif (ret)\n+\t\treturn ret;\n+\n+\tstate_ = CameraRunning;\n+\n+\treturn 0;\n }\n \n /**\n@@ -396,27 +549,22 @@ int Camera::start()\n * requests are cancelled and complete synchronously in an error state.\n *\n * \\return 0 on success or a negative error code otherwise\n+ * \\retval -ENODEV The camera is no longer connected to the hardware\n+ * \\retval -EACCES The camera is not running so can't be stopped\n */\n int Camera::stop()\n {\n-\tint ret = exclusiveAccess();\n-\tif (ret)\n-\t\treturn ret;\n+\tif (disconnected_)\n+\t\treturn -ENODEV;\n+\n+\tif (!stateIs(CameraRunning))\n+\t\treturn -EACCES;\n \n \tLOG(Camera, Debug) << \"Stopping capture\";\n \n \tpipe_->stop(this);\n \n-\treturn 0;\n-}\n-\n-int Camera::exclusiveAccess()\n-{\n-\tif (disconnected_)\n-\t\treturn -ENODEV;\n-\n-\tif (!acquired_)\n-\t\treturn -EACCES;\n+\tstate_ = CameraPrepared;\n \n \treturn 0;\n }\n","prefixes":["libcamera-devel","v2","3/4"]}