[libcamera-devel] [PATCH v3 08/11] libcamera: camera: Extend with a Stopping state

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thu, Mar 25, 2021 at 01:42:28PM +0000, Kieran Bingham wrote:
> When the camera is being stop()ped, active requests will complete. These
> may trigger an application to re-queue those requests to the camera but
> that is not permitted.

Maybe ", and is an error in the application." to be clear ?

> 
> Extend the camera state to include a stopping state which is entered as
> soon as a call to stop() is made. At this point, any request queued will
> be rejected with a warning, while any pending requests are either
> successfully completed or cancelled.

Should it be an error instead of a warning ?

> When the pipeline handler has finished stopping, the camera state will
> transition to the CameraConfigured state where it can begin to accept
> requests again, and be restarted.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  src/libcamera/camera.cpp | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 336ab8695ab3..7f7956ba732f 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -339,6 +339,7 @@ public:
>  		CameraAvailable,
>  		CameraAcquired,
>  		CameraConfigured,
> +		CameraStopping,
>  		CameraRunning,
>  	};
>  
> @@ -382,6 +383,7 @@ static const char *const camera_state_names[] = {
>  	"Available",
>  	"Acquired",
>  	"Configured",
> +	"Stopping",
>  	"Running",
>  };
>  
> @@ -492,6 +494,7 @@ void Camera::Private::setState(State state)
>   *   node [shape = doublecircle ]; Available;
>   *   node [shape = circle ]; Acquired;
>   *   node [shape = circle ]; Configured;
> + *   node [shape = circle ]; Stopping;
>   *   node [shape = circle ]; Running;
>   *
>   *   Available -> Available [label = "release()"];
> @@ -504,7 +507,8 @@ void Camera::Private::setState(State state)
>   *   Configured -> Configured [label = "configure(), createRequest()"];
>   *   Configured -> Running [label = "start()"];
>   *
> - *   Running -> Configured [label = "stop()"];
> + *   Running -> Stopping [label = "stop()"];
> + *   Stopping -> Configured;
>   *   Running -> Running [label = "createRequest(), queueRequest()"];
>   * }
>   * \enddot
> @@ -524,6 +528,12 @@ void Camera::Private::setState(State state)
>   * release() the camera and to get back to the Available state or start()
>   * it to progress to the Running state.
>   *
> + * \subsubsection Stopping
> + * The camera has been asked to stop. Pending reqeusts are being completed or

s/reqeusts/requests/

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> + * cancelled, and no new requests are permitted to be queued. The camera will
> + * transition to the Configured state when all queued requests have been
> + * returned to the application.
> + *
>   * \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
> @@ -1071,6 +1081,8 @@ int Camera::stop()
>  
>  	LOG(Camera, Debug) << "Stopping capture";
>  
> +	d->setState(Private::CameraStopping);
> +
>  	d->pipe_->invokeMethod(&PipelineHandler::stop, ConnectionTypeBlocking,
>  			       this);
>  
> @@ -1091,7 +1103,9 @@ void Camera::requestComplete(Request *request)
>  	Private *const d = LIBCAMERA_D_PTR();
>  
>  	/* Disconnected cameras are still able to complete requests. */
> -	int ret = d->isAccessAllowed(__FUNCTION__, Private::CameraRunning);
> +	int ret = d->isAccessAllowed(__FUNCTION__,
> +				     Private::CameraStopping,
> +				     Private::CameraRunning);
>  	if (ret < 0 && ret != -ENODEV)
>  		LOG(Camera, Fatal) << "Trying to complete a request when stopped";
>  

-- 
Regards,

Laurent Pinchart