[libcamera-devel] [PATCH] cam: capture: Stop stream when queueRequest fails

[Kieran Bingham]

Hi Helen,

Thank you for the patch, and welcome to libcamera !

On 25/06/2019 17:23, Helen Koike wrote:
> queueRequest is called after starting the stream.

Very minor, We normally put '()' to denote functions so it would be good
to put queueRequest()

I can add those while applying.


> If it fails, the stream should be stopped, otherwise it can get a
> "Device or resource busy" error, due to VIDIOC_REQBUFS ioctls being
> called after VIDIOC_STREAMON without VIDIOC_STREAMOFF in-between.

Sounds like a good find. I hope this means that libcamera is starting to
be useful already.

Out of curiosity, what platform are you running this on? RK3399 perhaps?


> Signed-off-by: Helen Koike <helen.koike at collabora.com>
> ---
>  src/cam/capture.cpp | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> index e455612..6b842d7 100644
> --- a/src/cam/capture.cpp
> +++ b/src/cam/capture.cpp
> @@ -117,6 +117,7 @@ int Capture::capture(EventLoop *loop)
>  		ret = camera_->queueRequest(request);
>  		if (ret < 0) {
>  			std::cerr << "Can't queue request" << std::endl;
> +			camera_->stop();

This looks good to me,

It seems clear that this is an unbalanced start()/stop() code path in
the cam utility. I started to wonder while reviewing this if we should
have a call somewhere that stops the camera on an error, but I think
that goes beyond the scope of this patch and handling errors with magic
will be some extra policy :-)

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


>  			return ret;
>  		}
>  	}
> 

-- 
Regards
--
Kieran