[libcamera-devel,2/5] cam: Queue requests unconditionally
diff mbox series

Message ID 20220518171921.244168-3-jacopo@jmondi.org
State Superseded, archived
Headers show
Series
  • cam: Add support for capture scripts
Related show

Commit Message

Jacopo Mondi May 18, 2022, 5:19 p.m. UTC
The CaptureSession::queueRequest() function increments the queueCount_
counter, which tracks the number of requests queued to the camera.

The number of queued requests is currently compared with the capture
limit which should instead only be compared with the number of captured
frames, something which already happens in the
CameraSession::processRequest() function.

Remove the check and only compare the capture limit with the actual
number of captured frames. If more requests than the frame capture
limits end up being queued to the Camera, they will be released on
Camera::stop().

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/cam/camera_session.cpp | 3 ---
 1 file changed, 3 deletions(-)

Comments

Laurent Pinchart May 20, 2022, 10:16 a.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Wed, May 18, 2022 at 07:19:18PM +0200, Jacopo Mondi via libcamera-devel wrote:
> The CaptureSession::queueRequest() function increments the queueCount_
> counter, which tracks the number of requests queued to the camera.
> 
> The number of queued requests is currently compared with the capture
> limit which should instead only be compared with the number of captured
> frames, something which already happens in the
> CameraSession::processRequest() function.
> 
> Remove the check and only compare the capture limit with the actual
> number of captured frames. If more requests than the frame capture
> limits end up being queued to the Camera, they will be released on
> Camera::stop().

They will, but the point of this check is to capture exactly
captureLimit_ frames. See commit a3c75bba84ba ("cam: Only queue the
exact number of requests asked for"). I'd rather keep it if possible.

> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  src/cam/camera_session.cpp | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> index 71e6bd605139..186072817367 100644
> --- a/src/cam/camera_session.cpp
> +++ b/src/cam/camera_session.cpp
> @@ -327,9 +327,6 @@ int CameraSession::startCapture()
>  
>  int CameraSession::queueRequest(Request *request)
>  {
> -	if (captureLimit_ && queueCount_ >= captureLimit_)
> -		return 0;
> -
>  	queueCount_++;
>  
>  	return camera_->queueRequest(request);
Jacopo Mondi May 20, 2022, 4:02 p.m. UTC | #2
Hi Laurent,

On Fri, May 20, 2022 at 01:16:08PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Wed, May 18, 2022 at 07:19:18PM +0200, Jacopo Mondi via libcamera-devel wrote:
> > The CaptureSession::queueRequest() function increments the queueCount_
> > counter, which tracks the number of requests queued to the camera.
> >
> > The number of queued requests is currently compared with the capture
> > limit which should instead only be compared with the number of captured
> > frames, something which already happens in the
> > CameraSession::processRequest() function.
> >
> > Remove the check and only compare the capture limit with the actual
> > number of captured frames. If more requests than the frame capture
> > limits end up being queued to the Camera, they will be released on
> > Camera::stop().
>
> They will, but the point of this check is to capture exactly
> captureLimit_ frames. See commit a3c75bba84ba ("cam: Only queue the
> exact number of requests asked for"). I'd rather keep it if possible.
>

Fine, I think this was mostly a drive-by cleanup

> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  src/cam/camera_session.cpp | 3 ---
> >  1 file changed, 3 deletions(-)
> >
> > diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> > index 71e6bd605139..186072817367 100644
> > --- a/src/cam/camera_session.cpp
> > +++ b/src/cam/camera_session.cpp
> > @@ -327,9 +327,6 @@ int CameraSession::startCapture()
> >
> >  int CameraSession::queueRequest(Request *request)
> >  {
> > -	if (captureLimit_ && queueCount_ >= captureLimit_)
> > -		return 0;
> > -
> >  	queueCount_++;
> >
> >  	return camera_->queueRequest(request);
>
> --
> Regards,
>
> Laurent Pinchart

Patch
diff mbox series

diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
index 71e6bd605139..186072817367 100644
--- a/src/cam/camera_session.cpp
+++ b/src/cam/camera_session.cpp
@@ -327,9 +327,6 @@  int CameraSession::startCapture()
 
 int CameraSession::queueRequest(Request *request)
 {
-	if (captureLimit_ && queueCount_ >= captureLimit_)
-		return 0;
-
 	queueCount_++;
 
 	return camera_->queueRequest(request);