[libcamera-devel] [PATCH] cam: capture: Stop stream when queueRequest fails
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Jun 25 21:56:43 CEST 2019
Hi Helen,
On 25/06/2019 20:51, Kieran Bingham wrote:
> 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.
This works and passes the tests (and manual testing of cam) so I've
pushed it to master.
Thank you.
Regards,
Kieran
>> 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;
>> }
>> }
>>
>
More information about the libcamera-devel
mailing list