Patch Detail
Show a patch.
GET /api/1.1/patches/626/?format=api
{ "id": 626, "url": "https://patchwork.libcamera.org/api/1.1/patches/626/?format=api", "web_url": "https://patchwork.libcamera.org/patch/626/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/1.1/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20190226021857.28255-6-niklas.soderlund@ragnatech.se>", "date": "2019-02-26T02:18:54", "name": "[libcamera-devel,5/8] libcamera: camera: add state machine to control access from applications", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "fad09be62dba434566e371428638b792d5aa9529", "submitter": { "id": 5, "url": "https://patchwork.libcamera.org/api/1.1/people/5/?format=api", "name": "Niklas Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/626/mbox/", "series": [ { "id": 192, "url": "https://patchwork.libcamera.org/api/1.1/series/192/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=192", "date": "2019-02-26T02:18:50", "name": "libcamera: improve validation of camera operations", "version": 1, "mbox": "https://patchwork.libcamera.org/series/192/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/626/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/626/checks/", "tags": {}, "headers": { "Return-Path": "<niklas.soderlund@ragnatech.se>", "Received": [ "from bin-mail-out-05.binero.net (bin-mail-out-05.binero.net\n\t[195.74.38.228])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 02E08601E2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 26 Feb 2019 03:19:24 +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 ecc5dd6a-396c-11e9-985a-005056917f90;\n\tTue, 26 Feb 2019 03:19:22 +0100 (CET)" ], "X-Halon-ID": "ecc5dd6a-396c-11e9-985a-005056917f90", "Authorized-sender": "niklas@soderlund.pp.se", "From": "=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>", "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", "Content-Type": "text/plain; charset=UTF-8", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH 5/8] libcamera: camera: add state machine\n\tto control access from applications", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.23", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "X-List-Received-Date": "Tue, 26 Feb 2019 02:19:25 -0000" }, "content": "There is a need to better control the order of operations an application\nperform 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 four states are added; Disconnected, Free,\nAcquired and Running. Each state represents a higher state of\nconfiguration of the camera ultimately leading to the highest state\nwhere the camera is capturing frames. Each state supports a subset of\noperations the application can perform.\n\n* Disconnected\nIs the lowest state a camera can be in. It indicates that the camera\nhave been disconnected from the system and the only operations an\napplication shall do at this point is clean up and release the camera so\nit can be removed from libcamera as well.\n\n* Free\nIs the base state of a camera, an application can inspect the properties\nof the camera to determine if it wish to use it or not. If an\napplication wish to use a camera it should acquire it to proceed to the\nnext state.\n\n* Acquired\nWhen an application have acquired a camera it have exclusive access to\nit and can modify the cameras parameters to prepare it for capturing.\nOnce the application is done configure the camera it may be started to\nprogress to the running state. Once the camera is started it can not be\nreconfigured until it's stopped.\n\n* Running\nIn this state the camera is running and able to process requests queued\nto it by the application. Once an application finish capturing the\ncamera shall be stopped to change the state to acquired where it can be\nreconfigured or released.\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n---\n include/libcamera/camera.h | 14 ++++-\n src/libcamera/camera.cpp | 110 +++++++++++++++++++++++--------------\n 2 files changed, 80 insertions(+), 44 deletions(-)", "diff": "diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h\nindex bf70255a6a5ea364..8c8545b074e8ae13 100644\n--- a/include/libcamera/camera.h\n+++ b/include/libcamera/camera.h\n@@ -55,20 +55,28 @@ public:\n \tint stop();\n \n private:\n+\tenum State {\n+\t\tDisconnected,\n+\t\tFree,\n+\t\tAcquired,\n+\t\tRunning,\n+\t};\n+\n \tCamera(PipelineHandler *pipe, const std::string &name);\n \t~Camera();\n \n+\tbool stateIs(State state) const;\n+\tbool stateIsAtleast(State state) const;\n+\n \tfriend class PipelineHandler;\n \tvoid disconnect();\n-\tint exclusiveAccess();\n \n \tstd::shared_ptr<PipelineHandler> pipe_;\n \tstd::string name_;\n \tstd::vector<Stream *> streams_;\n \tstd::vector<Stream *> 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 d4258fe3c7551af3..c50b14bbd904fc1c 100644\n--- a/src/libcamera/camera.cpp\n+++ b/src/libcamera/camera.cpp\n@@ -116,17 +116,47 @@ 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), state_(Free)\n {\n }\n \n Camera::~Camera()\n {\n-\tif (acquired_)\n+\tif (state_ > Free)\n \t\tLOG(Camera, Error) << \"Removing camera while still in use\";\n }\n \n+static const std::string stateNames[] = {\n+\t\"Disconnected\",\n+\t\"Free\",\n+\t\"Acquired\",\n+\t\"Running\",\n+};\n+\n+bool Camera::stateIs(State state) const\n+{\n+\tif (state_ == state)\n+\t\treturn true;\n+\n+\tLOG(Camera, Error) << \"Camera in \" << stateNames[state_]\n+\t\t\t << \" state trying operation requiering \"\n+\t\t\t << stateNames[state] << \" state\";\n+\n+\treturn false;\n+}\n+\n+bool Camera::stateIsAtleast(State state) const\n+{\n+\tif (state_ >= state)\n+\t\treturn true;\n+\n+\tLOG(Camera, Error) << \"Camera in \" << stateNames[state_]\n+\t\t\t << \" state trying operation requiering at least \"\n+\t\t\t << stateNames[state] << \" state\";\n+\n+\treturn false;\n+}\n+\n /**\n * \\brief Notify camera disconnection\n *\n@@ -140,7 +170,7 @@ void Camera::disconnect()\n {\n \tLOG(Camera, Debug) << \"Disconnecting camera \" << name_;\n \n-\tdisconnected_ = true;\n+\tstate_ = Disconnected;\n \tdisconnected.emit(this);\n }\n \n@@ -162,10 +192,11 @@ void Camera::disconnect()\n */\n int Camera::acquire()\n {\n-\tif (acquired_)\n+\tif (!stateIs(Free))\n \t\treturn -EBUSY;\n \n-\tacquired_ = true;\n+\tstate_ = Acquired;\n+\n \treturn 0;\n }\n \n@@ -177,7 +208,10 @@ int Camera::acquire()\n */\n void Camera::release()\n {\n-\tacquired_ = false;\n+\tif (!stateIs(Acquired))\n+\t\treturn;\n+\n+\tstate_ = Free;\n }\n \n /**\n@@ -191,6 +225,9 @@ void Camera::release()\n */\n const std::vector<Stream *> &Camera::streams() const\n {\n+\tif (!stateIsAtleast(Free))\n+\t\tstd::vector<Stream *>{};\n+\n \treturn streams_;\n }\n \n@@ -213,7 +250,7 @@ const std::vector<Stream *> &Camera::streams() const\n std::map<Stream *, StreamConfiguration>\n Camera::streamConfiguration(std::vector<Stream *> &streams)\n {\n-\tif (disconnected_ || !streams.size())\n+\tif (!stateIsAtleast(Free) || !streams.size())\n \t\treturn std::map<Stream *, StreamConfiguration>{};\n \n \treturn pipe_->streamConfiguration(this, streams);\n@@ -244,9 +281,8 @@ int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)\n {\n \tint ret;\n \n-\tret = exclusiveAccess();\n-\tif (ret)\n-\t\treturn ret;\n+\tif (!stateIs(Acquired))\n+\t\treturn -EACCES;\n \n \tif (!config.size()) {\n \t\tLOG(Camera, Error)\n@@ -284,11 +320,8 @@ int Camera::configureStreams(std::map<Stream *, StreamConfiguration> &config)\n */\n int Camera::allocateBuffers()\n {\n-\tint ret;\n-\n-\tret = exclusiveAccess();\n-\tif (ret)\n-\t\treturn ret;\n+\tif (!stateIs(Acquired))\n+\t\treturn -EACCES;\n \n \tif (activeStreams_.empty()) {\n \t\tLOG(Camera, Error)\n@@ -297,7 +330,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@@ -313,6 +346,9 @@ int Camera::allocateBuffers()\n */\n void Camera::freeBuffers()\n {\n+\tif (!stateIs(Acquired))\n+\t\treturn;\n+\n \tfor (Stream *stream : activeStreams_) {\n \t\tif (!stream->bufferPool().count())\n \t\t\tcontinue;\n@@ -336,7 +372,7 @@ void Camera::freeBuffers()\n */\n Request *Camera::createRequest()\n {\n-\tif (exclusiveAccess())\n+\tif (!stateIsAtleast(Acquired))\n \t\treturn nullptr;\n \n \treturn new Request(this);\n@@ -358,13 +394,10 @@ Request *Camera::createRequest()\n */\n int Camera::queueRequest(Request *request)\n {\n-\tint ret;\n+\tif (!stateIs(Running))\n+\t\treturn -EACCES;\n \n-\tret = exclusiveAccess();\n-\tif (ret)\n-\t\treturn ret;\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@@ -385,13 +418,18 @@ int Camera::queueRequest(Request *request)\n */\n int Camera::start()\n {\n-\tint ret = exclusiveAccess();\n-\tif (ret)\n-\t\treturn ret;\n+\tif (!stateIs(Acquired))\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_ = Running;\n+\n+\treturn 0;\n }\n \n /**\n@@ -405,24 +443,14 @@ int Camera::start()\n */\n int Camera::stop()\n {\n-\tint ret = exclusiveAccess();\n-\tif (ret)\n-\t\treturn ret;\n+\tif (!stateIs(Running))\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_ = Acquired;\n \n \treturn 0;\n }\n", "prefixes": [ "libcamera-devel", "5/8" ] }