[{"id":15920,"web_url":"https://patchwork.libcamera.org/comment/15920/","msgid":"","date":"2021-03-26T02:36:42","subject":"Re: [libcamera-devel] [PATCH v3 08/11] libcamera: camera: Extend\n\twith a Stopping state","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nThank you for the patch.\n\nOn Thu, Mar 25, 2021 at 01:42:28PM +0000, Kieran Bingham wrote:\n> When the camera is being stop()ped, active requests will complete. These\n> may trigger an application to re-queue those requests to the camera but\n> that is not permitted.\n\nMaybe \", and is an error in the application.\" to be clear ?\n\n> \n> Extend the camera state to include a stopping state which is entered as\n> soon as a call to stop() is made. At this point, any request queued will\n> be rejected with a warning, while any pending requests are either\n> successfully completed or cancelled.\n\nShould it be an error instead of a warning ?\n\n> When the pipeline handler has finished stopping, the camera state will\n> transition to the CameraConfigured state where it can begin to accept\n> requests again, and be restarted.\n> \n> Signed-off-by: Kieran Bingham \n> ---\n> src/libcamera/camera.cpp | 18 ++++++++++++++++--\n> 1 file changed, 16 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> index 336ab8695ab3..7f7956ba732f 100644\n> --- a/src/libcamera/camera.cpp\n> +++ b/src/libcamera/camera.cpp\n> @@ -339,6 +339,7 @@ public:\n> \t\tCameraAvailable,\n> \t\tCameraAcquired,\n> \t\tCameraConfigured,\n> +\t\tCameraStopping,\n> \t\tCameraRunning,\n> \t};\n> \n> @@ -382,6 +383,7 @@ static const char *const camera_state_names[] = {\n> \t\"Available\",\n> \t\"Acquired\",\n> \t\"Configured\",\n> +\t\"Stopping\",\n> \t\"Running\",\n> };\n> \n> @@ -492,6 +494,7 @@ void Camera::Private::setState(State state)\n> * node [shape = doublecircle ]; Available;\n> * node [shape = circle ]; Acquired;\n> * node [shape = circle ]; Configured;\n> + * node [shape = circle ]; Stopping;\n> * node [shape = circle ]; Running;\n> *\n> * Available -> Available [label = \"release()\"];\n> @@ -504,7 +507,8 @@ void Camera::Private::setState(State state)\n> * Configured -> Configured [label = \"configure(), createRequest()\"];\n> * Configured -> Running [label = \"start()\"];\n> *\n> - * Running -> Configured [label = \"stop()\"];\n> + * Running -> Stopping [label = \"stop()\"];\n> + * Stopping -> Configured;\n> * Running -> Running [label = \"createRequest(), queueRequest()\"];\n> * }\n> * \\enddot\n> @@ -524,6 +528,12 @@ void Camera::Private::setState(State state)\n> * release() the camera and to get back to the Available state or start()\n> * it to progress to the Running state.\n> *\n> + * \\subsubsection Stopping\n> + * The camera has been asked to stop. Pending reqeusts are being completed or\n\ns/reqeusts/requests/\n\nReviewed-by: Laurent Pinchart \n\n> + * cancelled, and no new requests are permitted to be queued. The camera will\n> + * transition to the Configured state when all queued requests have been\n> + * returned to the application.\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 is stopped and moved\n> @@ -1071,6 +1081,8 @@ int Camera::stop()\n> \n> \tLOG(Camera, Debug) << \"Stopping capture\";\n> \n> +\td->setState(Private::CameraStopping);\n> +\n> \td->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking,\n> \t\t\t this);\n> \n> @@ -1091,7 +1103,9 @@ void Camera::requestComplete(Request *request)\n> \tPrivate *const d = LIBCAMERA_D_PTR();\n> \n> \t/* Disconnected cameras are still able to complete requests. */\n> -\tint ret = d->isAccessAllowed(__FUNCTION__, Private::CameraRunning);\n> +\tint ret = d->isAccessAllowed(__FUNCTION__,\n> +\t\t\t\t Private::CameraStopping,\n> +\t\t\t\t Private::CameraRunning);\n> \tif (ret < 0 && ret != -ENODEV)\n> \t\tLOG(Camera, Fatal) << \"Trying to complete a request when stopped\";\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 00350C32EA\n\tfor ;\n\tFri, 26 Mar 2021 02:37:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F853603FE;\n\tFri, 26 Mar 2021 03:37:27 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BB8F4602E3\n\tfor ;\n\tFri, 26 Mar 2021 03:37:25 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1CBFA443;\n\tFri, 26 Mar 2021 03:37:25 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"cK9KRtGU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616726245;\n\tbh=RJwQChAZu2RBulxH0FOYWJISYB8KaMU1jjZ+7Koqqqk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cK9KRtGUMP7cBBA86goffx2XnBP/BOyin/WU6iRzF8bAUf3UXoY0Tzq6oKXzB5PYY\n\tMCSbUTalzKw95lMVxmf+aUWGQ5eVnUVC5eom/dycVen7aA2OBaTCF/uoOnvapFeJjF\n\tMt5z/Nm7CYgEdhR3PYsbD3Lh9D/HHevoKN99uQ6I=","Date":"Fri, 26 Mar 2021 04:36:42 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210325134231.1400051-1-kieran.bingham@ideasonboard.com>\n\t<20210325134231.1400051-11-kieran.bingham@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210325134231.1400051-11-kieran.bingham@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 08/11] libcamera: camera: Extend\n\twith a Stopping state","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15952,"web_url":"https://patchwork.libcamera.org/comment/15952/","msgid":"<4f48d43b-633d-891e-78d3-4eed9b9fe49b@ideasonboard.com>","date":"2021-03-26T15:48:07","subject":"Re: [libcamera-devel] [PATCH v3 08/11] libcamera: camera: Extend\n\twith a Stopping state","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 26/03/2021 02:36, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> Thank you for the patch.\n> \n> On Thu, Mar 25, 2021 at 01:42:28PM +0000, Kieran Bingham wrote:\n>> When the camera is being stop()ped, active requests will complete. These\n>> may trigger an application to re-queue those requests to the camera but\n>> that is not permitted.\n> \n> Maybe \", and is an error in the application.\" to be clear ?\n\nYes,\n\n> \n>>\n>> Extend the camera state to include a stopping state which is entered as\n>> soon as a call to stop() is made. At this point, any request queued will\n>> be rejected with a warning, while any pending requests are either\n>> successfully completed or cancelled.\n> \n> Should it be an error instead of a warning ?\n\nThe warning will come from the isAccessAllowed log level.\n\nI don't want to duplicate an error message, but it would probably be\nconsidered an error.\n\nWe could try to pass in a log level to the isAccessAllowed, but I think\nwe're overloading those operators enough already ...\n\nPerhaps we could actually raise the log from isAccessAllowed further\nfrom Debug to Warning, to all the way to Error?\n\nIf an operation is made in the wrong state - it's an error isn't it ?\n\n\n> \n>> When the pipeline handler has finished stopping, the camera state will\n>> transition to the CameraConfigured state where it can begin to accept\n>> requests again, and be restarted.\n>>\n>> Signed-off-by: Kieran Bingham \n>> ---\n>> src/libcamera/camera.cpp | 18 ++++++++++++++++--\n>> 1 file changed, 16 insertions(+), 2 deletions(-)\n>>\n>> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n>> index 336ab8695ab3..7f7956ba732f 100644\n>> --- a/src/libcamera/camera.cpp\n>> +++ b/src/libcamera/camera.cpp\n>> @@ -339,6 +339,7 @@ public:\n>> \t\tCameraAvailable,\n>> \t\tCameraAcquired,\n>> \t\tCameraConfigured,\n>> +\t\tCameraStopping,\n>> \t\tCameraRunning,\n>> \t};\n>> \n>> @@ -382,6 +383,7 @@ static const char *const camera_state_names[] = {\n>> \t\"Available\",\n>> \t\"Acquired\",\n>> \t\"Configured\",\n>> +\t\"Stopping\",\n>> \t\"Running\",\n>> };\n>> \n>> @@ -492,6 +494,7 @@ void Camera::Private::setState(State state)\n>> * node [shape = doublecircle ]; Available;\n>> * node [shape = circle ]; Acquired;\n>> * node [shape = circle ]; Configured;\n>> + * node [shape = circle ]; Stopping;\n>> * node [shape = circle ]; Running;\n>> *\n>> * Available -> Available [label = \"release()\"];\n>> @@ -504,7 +507,8 @@ void Camera::Private::setState(State state)\n>> * Configured -> Configured [label = \"configure(), createRequest()\"];\n>> * Configured -> Running [label = \"start()\"];\n>> *\n>> - * Running -> Configured [label = \"stop()\"];\n>> + * Running -> Stopping [label = \"stop()\"];\n>> + * Stopping -> Configured;\n>> * Running -> Running [label = \"createRequest(), queueRequest()\"];\n>> * }\n>> * \\enddot\n>> @@ -524,6 +528,12 @@ void Camera::Private::setState(State state)\n>> * release() the camera and to get back to the Available state or start()\n>> * it to progress to the Running state.\n>> *\n>> + * \\subsubsection Stopping\n>> + * The camera has been asked to stop. Pending reqeusts are being completed or\n> \n> s/reqeusts/requests/\n\nFixed, thanks.\n\n\n> \n> Reviewed-by: Laurent Pinchart \n> \n>> + * cancelled, and no new requests are permitted to be queued. The camera will\n>> + * transition to the Configured state when all queued requests have been\n>> + * returned to the application.\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 is stopped and moved\n>> @@ -1071,6 +1081,8 @@ int Camera::stop()\n>> \n>> \tLOG(Camera, Debug) << \"Stopping capture\";\n>> \n>> +\td->setState(Private::CameraStopping);\n>> +\n>> \td->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking,\n>> \t\t\t this);\n>> \n>> @@ -1091,7 +1103,9 @@ void Camera::requestComplete(Request *request)\n>> \tPrivate *const d = LIBCAMERA_D_PTR();\n>> \n>> \t/* Disconnected cameras are still able to complete requests. */\n>> -\tint ret = d->isAccessAllowed(__FUNCTION__, Private::CameraRunning);\n>> +\tint ret = d->isAccessAllowed(__FUNCTION__,\n>> +\t\t\t\t Private::CameraStopping,\n>> +\t\t\t\t Private::CameraRunning);\n>> \tif (ret < 0 && ret != -ENODEV)\n>> \t\tLOG(Camera, Fatal) << \"Trying to complete a request when stopped\";\n>> \n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id DDCFEC32EA\n\tfor ;\n\tFri, 26 Mar 2021 15:48:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3A96868D58;\n\tFri, 26 Mar 2021 16:48:11 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 96F47602D7\n\tfor ;\n\tFri, 26 Mar 2021 16:48:10 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0BEE2443;\n\tFri, 26 Mar 2021 16:48:10 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"PhBeP0od\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616773690;\n\tbh=xJQBxEzAzMHWPaJW68P0TihG1kGBebGeU4uVJGOOtHE=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=PhBeP0odVV9QphN8wnm9jSsVdVxrt6Wxbkzr+9zYyNntvRHb5qnK0+tNgBlVnoLqX\n\tvCR0P8daNxVUgrWO0GCtmopxceBc9WGccNqTGF7dW4BwGRBrTKUfDrkP+/9niB3oBm\n\taLFh2ZB18tvyNxpZ+KI2ewFX+dl9L/BXNnyfscbI=","To":"Laurent Pinchart ","References":"<20210325134231.1400051-1-kieran.bingham@ideasonboard.com>\n\t<20210325134231.1400051-11-kieran.bingham@ideasonboard.com>\n\t","From":"Kieran Bingham ","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<4f48d43b-633d-891e-78d3-4eed9b9fe49b@ideasonboard.com>","Date":"Fri, 26 Mar 2021 15:48:07 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 08/11] libcamera: camera: Extend\n\twith a Stopping state","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15955,"web_url":"https://patchwork.libcamera.org/comment/15955/","msgid":"","date":"2021-03-26T15:55:33","subject":"Re: [libcamera-devel] [PATCH v3 08/11] libcamera: camera: Extend\n\twith a Stopping state","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Fri, Mar 26, 2021 at 03:48:07PM +0000, Kieran Bingham wrote:\n> On 26/03/2021 02:36, Laurent Pinchart wrote:\n> > On Thu, Mar 25, 2021 at 01:42:28PM +0000, Kieran Bingham wrote:\n> >> When the camera is being stop()ped, active requests will complete. These\n> >> may trigger an application to re-queue those requests to the camera but\n> >> that is not permitted.\n> > \n> > Maybe \", and is an error in the application.\" to be clear ?\n> \n> Yes,\n> \n> >> Extend the camera state to include a stopping state which is entered as\n> >> soon as a call to stop() is made. At this point, any request queued will\n> >> be rejected with a warning, while any pending requests are either\n> >> successfully completed or cancelled.\n> > \n> > Should it be an error instead of a warning ?\n> \n> The warning will come from the isAccessAllowed log level.\n> \n> I don't want to duplicate an error message, but it would probably be\n> considered an error.\n> \n> We could try to pass in a log level to the isAccessAllowed, but I think\n> we're overloading those operators enough already ...\n> \n> Perhaps we could actually raise the log from isAccessAllowed further\n> from Debug to Warning, to all the way to Error?\n> \n> If an operation is made in the wrong state - it's an error isn't it ?\n\nYes, I think so.\n\n> >> When the pipeline handler has finished stopping, the camera state will\n> >> transition to the CameraConfigured state where it can begin to accept\n> >> requests again, and be restarted.\n> >>\n> >> Signed-off-by: Kieran Bingham \n> >> ---\n> >> src/libcamera/camera.cpp | 18 ++++++++++++++++--\n> >> 1 file changed, 16 insertions(+), 2 deletions(-)\n> >>\n> >> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp\n> >> index 336ab8695ab3..7f7956ba732f 100644\n> >> --- a/src/libcamera/camera.cpp\n> >> +++ b/src/libcamera/camera.cpp\n> >> @@ -339,6 +339,7 @@ public:\n> >> \t\tCameraAvailable,\n> >> \t\tCameraAcquired,\n> >> \t\tCameraConfigured,\n> >> +\t\tCameraStopping,\n> >> \t\tCameraRunning,\n> >> \t};\n> >> \n> >> @@ -382,6 +383,7 @@ static const char *const camera_state_names[] = {\n> >> \t\"Available\",\n> >> \t\"Acquired\",\n> >> \t\"Configured\",\n> >> +\t\"Stopping\",\n> >> \t\"Running\",\n> >> };\n> >> \n> >> @@ -492,6 +494,7 @@ void Camera::Private::setState(State state)\n> >> * node [shape = doublecircle ]; Available;\n> >> * node [shape = circle ]; Acquired;\n> >> * node [shape = circle ]; Configured;\n> >> + * node [shape = circle ]; Stopping;\n> >> * node [shape = circle ]; Running;\n> >> *\n> >> * Available -> Available [label = \"release()\"];\n> >> @@ -504,7 +507,8 @@ void Camera::Private::setState(State state)\n> >> * Configured -> Configured [label = \"configure(), createRequest()\"];\n> >> * Configured -> Running [label = \"start()\"];\n> >> *\n> >> - * Running -> Configured [label = \"stop()\"];\n> >> + * Running -> Stopping [label = \"stop()\"];\n> >> + * Stopping -> Configured;\n> >> * Running -> Running [label = \"createRequest(), queueRequest()\"];\n> >> * }\n> >> * \\enddot\n> >> @@ -524,6 +528,12 @@ void Camera::Private::setState(State state)\n> >> * release() the camera and to get back to the Available state or start()\n> >> * it to progress to the Running state.\n> >> *\n> >> + * \\subsubsection Stopping\n> >> + * The camera has been asked to stop. Pending reqeusts are being completed or\n> > \n> > s/reqeusts/requests/\n> \n> Fixed, thanks.\n> \n> > Reviewed-by: Laurent Pinchart \n> > \n> >> + * cancelled, and no new requests are permitted to be queued. The camera will\n> >> + * transition to the Configured state when all queued requests have been\n> >> + * returned to the application.\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 is stopped and moved\n> >> @@ -1071,6 +1081,8 @@ int Camera::stop()\n> >> \n> >> \tLOG(Camera, Debug) << \"Stopping capture\";\n> >> \n> >> +\td->setState(Private::CameraStopping);\n> >> +\n> >> \td->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking,\n> >> \t\t\t this);\n> >> \n> >> @@ -1091,7 +1103,9 @@ void Camera::requestComplete(Request *request)\n> >> \tPrivate *const d = LIBCAMERA_D_PTR();\n> >> \n> >> \t/* Disconnected cameras are still able to complete requests. */\n> >> -\tint ret = d->isAccessAllowed(__FUNCTION__, Private::CameraRunning);\n> >> +\tint ret = d->isAccessAllowed(__FUNCTION__,\n> >> +\t\t\t\t Private::CameraStopping,\n> >> +\t\t\t\t Private::CameraRunning);\n> >> \tif (ret < 0 && ret != -ENODEV)\n> >> \t\tLOG(Camera, Fatal) << \"Trying to complete a request when stopped\";\n> >>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 3CE80C32EA\n\tfor ;\n\tFri, 26 Mar 2021 15:56:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EF52968D6F;\n\tFri, 26 Mar 2021 16:56:18 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E0EDC68D58\n\tfor ;\n\tFri, 26 Mar 2021 16:56:17 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4AEA1443;\n\tFri, 26 Mar 2021 16:56:17 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"JDlyxe0I\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1616774177;\n\tbh=+iXdTLwrxsAQ8/MBexzDr7/BkyWW/jRDhB5a58SaEUc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JDlyxe0ImNt8Ws18XTL/HHBjIq0Rj41YllClsjcPXfEa2uUQJIqZb+8zCzj33b18R\n\tEGR/kady5/oWcbGysj77DgAlYjAkMxyJA40QXQ+humBlZU0h4go2eaXy6Uoqrrg30p\n\tO5DjFk4aAKCK3jTs7Zq77JU+B5Pvt+lkd4Dhty2w=","Date":"Fri, 26 Mar 2021 17:55:33 +0200","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210325134231.1400051-1-kieran.bingham@ideasonboard.com>\n\t<20210325134231.1400051-11-kieran.bingham@ideasonboard.com>\n\t\n\t<4f48d43b-633d-891e-78d3-4eed9b9fe49b@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<4f48d43b-633d-891e-78d3-4eed9b9fe49b@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 08/11] libcamera: camera: Extend\n\twith a Stopping state","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]