[libcamera-devel] [PATCH 8/8] libcamera: camera: make sure camera is configured before starting

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Tue, Feb 26, 2019 at 03:18:57AM +0100, Niklas Söderlund wrote:
> If the camera is not configured there is little use to start it. Add a
> check to make sure the camera is configured before it's started.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/camera.cpp | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index ba8638009992170f..cbc34599d25e5ed5 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -423,12 +423,19 @@ int Camera::queueRequest(Request *request)
>   *
>   * \return 0 on success or a negative error code
>   * \retval -EACCES The camera is not in a state where it can be started.
> + * \retval -EINVAL The camera is not configured.
>   */
>  int Camera::start()
>  {
>  	if (!stateIs(Acquired))
>  		return -EACCES;
>  
> +	if (activeStreams_.empty()) {
> +		LOG(Camera, Error)
> +			<< "Can't start camera without configuration";
> +		return -EINVAL;
> +	}

So this essentially creates a Configured state, without naming it. I
think the implementation may be simpler if you added an explicit state,
but there's no urgency, as the check is in place here.

I however wonder if -EINVAL is the right error code, given that you use
-EACCESS to denote state-related errors.

> +
>  	LOG(Camera, Debug) << "Starting capture";
>  
>  	int ret = pipe_->start(this);

-- 
Regards,

Laurent Pinchart