[libcamera-devel] [PATCH v2 1/4] cam: fix order camera is operated on

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Thu, Feb 28, 2019 at 03:16:23AM +0100, Niklas Söderlund wrote:
> Upcoming enforcing of order the camera shall be operate on is not
> compatible with the cam utility. Requests shall be queued after the
> camera is started, not before.

My comment on v1 hasn't been addressed. Could you at least add a \todo
in the state machine patch to remind us to document camera start timings
? I'm still not sure that preventing requests from being queued before
calling start() will not come back to bite us later, but for now I can't
think of a good alternative, so

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

(with the added todo in 3/4)

> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/cam/main.cpp | 18 +++++++++++-------
>  1 file changed, 11 insertions(+), 7 deletions(-)
> 
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 4c2df583fe8e99b7..8df8844c33a2d052 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -133,6 +133,7 @@ static int capture()
>  	int ret;
>  
>  	std::vector<Stream *> streams = camera->streams();
> +	std::vector<Request *> requests;
>  
>  	ret = configureStreams(camera.get(), streams);
>  	if (ret < 0) {
> @@ -169,21 +170,24 @@ static int capture()
>  			goto out;
>  		}
>  
> -		ret = camera->queueRequest(request);
> -		if (ret < 0) {
> -			std::cerr << "Can't queue request" << std::endl;
> -			goto out;
> -		}
> +		requests.push_back(request);
>  	}
>  
> -	std::cout << "Capture until user interrupts by SIGINT" << std::endl;
> -
>  	ret = camera->start();
>  	if (ret) {
>  		std::cout << "Failed to start capture" << std::endl;
>  		goto out;
>  	}
>  
> +	for (Request *request : requests) {
> +		ret = camera->queueRequest(request);
> +		if (ret < 0) {
> +			std::cerr << "Can't queue request" << std::endl;
> +			goto out;
> +		}
> +	}
> +
> +	std::cout << "Capture until user interrupts by SIGINT" << std::endl;
>  	ret = loop->exec();
>  
>  	ret = camera->stop();

-- 
Regards,

Laurent Pinchart