[libcamera-devel] [PATCH] cam: Fail capture if to few Requests asked for

[Niklas Söderlund]

Hi Kieran and Jacopo,

Thanks for your feedback.

On 2021-02-08 10:20:07 +0000, Kieran Bingham wrote:
> Hi Niklas,
> 
> Also s/to/too/ in $SUBJECT
> 
> On 08/02/2021 10:14, Jacopo Mondi wrote:
> > Hi Niklas,
> > 
> > On Mon, Feb 08, 2021 at 10:44:16AM +0100, Niklas Söderlund wrote:
> >> Running cam with the --capture=N option only queues N Requests to the
> >> camera. If N is less then the number of requests needed by the camera to
> >> operate cam may deadlock waiting for the camera to complete requests
> >> while the camera waits for the application to give it more buffers.
> >>
> >> Fix this by adding a check in cam to no attempt a capture session if to
> >> few requests are requested.
> 
> and here s/to/too/
> 
> >>
> >> Reported-by: Sebastian Fricke <sebastian.fricke at posteo.net>
> >> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> >> ---
> >>  src/cam/capture.cpp | 7 +++++++
> >>  1 file changed, 7 insertions(+)
> >>
> >> diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
> >> index 7b55fc6770225e2a..18aa97c7cdb55dcc 100644
> >> --- a/src/cam/capture.cpp
> >> +++ b/src/cam/capture.cpp
> >> @@ -90,6 +90,13 @@ int Capture::capture(FrameBufferAllocator *allocator)
> >>  		nbuffers = std::min(nbuffers, allocated);
> >>  	}
> >>
> >> +	if (captureLimit_ && captureLimit_ < nbuffers) {
> >> +		std::cerr << "Camera requiers at least " << nbuffers
> > 
> > s/requiers/requires
> > 
> >> +			  << " reqests to function, " << captureLimit_
> > 
> > s/reqests/requests
> > 
> >> +			  << " requested" << std::endl;
> > 
> > I would make it shorter
> >                         << "Camera requires " << nbuffers << " requests, "
> >                         << "but only " << captureLimit_ << " have been provided";
> > 
> > Which is not that shorter :/
> > 
> > Up to you
> > 
> > Spelling fixes apart
> > Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> 
> Should we increase the capture lmit to the minimum? Or do you want to
> make sure cam fails if too few are requested?

I would prefers to fail as one could do

    $ cam --camera foo --capture=2 --file=frame-#.bin

And expect 2 files to be written to disk. If we auto increment it to the 
minimum 2 or more files may be written to disk which might not be what 
the user wants.

> 
> Either way,
> 
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> 
> 
> > 
> > Thanks
> >   j
> > 
> >> +		return -EINVAL;
> >> +	}
> >> +
> >>  	/*
> >>  	 * TODO: make cam tool smarter to support still capture by for
> >>  	 * example pushing a button. For now run all streams all the time.
> >> --
> >> 2.30.0
> >>
> >> _______________________________________________
> >> libcamera-devel mailing list
> >> libcamera-devel at lists.libcamera.org
> >> https://lists.libcamera.org/listinfo/libcamera-devel
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
> > 
> 
> -- 
> Regards
> --
> Kieran

-- 
Regards,
Niklas Söderlund