[libcamera-devel,3/3] cam: Drop frames once the capture limit is reached
diff mbox series

Message ID 20211012022321.27034-4-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • libcamera: Assorted fixes for the cam application
Related show

Commit Message

Laurent Pinchart Oct. 12, 2021, 2:23 a.m. UTC
The camera session keeps requeuing requests until the capture limit is
reached. This causes more request than the limit to complete, as there's
a queue of requests in flight. When capturing from multiple cameras
concurrently, this results in the captureDone signal being emitted for
every request completion after the limit is reached, instead of once per
camera session when reaching the limit.

Fix this by simply dropping any request that completes after the limit
is reached. We could instead avoid requeuing more requests than needed
to reach the limit, but that may cause request starvation in pipelines,
which are currently not handled consistently (or correctly).

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/cam/camera_session.cpp | 3 +++
 1 file changed, 3 insertions(+)

Comments

Kieran Bingham Oct. 12, 2021, 11:16 a.m. UTC | #1
Quoting Laurent Pinchart (2021-10-12 03:23:21)
> The camera session keeps requeuing requests until the capture limit is
> reached. This causes more request than the limit to complete, as there's
> a queue of requests in flight. When capturing from multiple cameras
> concurrently, this results in the captureDone signal being emitted for
> every request completion after the limit is reached, instead of once per
> camera session when reaching the limit.
> 
> Fix this by simply dropping any request that completes after the limit
> is reached. We could instead avoid requeuing more requests than needed
> to reach the limit, but that may cause request starvation in pipelines,
> which are currently not handled consistently (or correctly).
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/cam/camera_session.cpp | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
> index 605018278c5a..594cd51bb6a2 100644
> --- a/src/cam/camera_session.cpp
> +++ b/src/cam/camera_session.cpp
> @@ -348,6 +348,9 @@ void CameraSession::processRequest(Request *request)
>  {
>         const Request::BufferMap &buffers = request->buffers();
>  

I'd be tempted to put a comment here to explain that this is different
from, and required as well as the similar check down at the end of the
function.

But either way:


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

> +       if (captureLimit_ && captureCount_ >= captureLimit_)
> +               return;
> +
>         /*
>          * Compute the frame rate. The timestamp is arbitrarily retrieved from
>          * the first buffer, as all buffers should have matching timestamps.
> -- 
> Regards,
> 
> Laurent Pinchart
>

Patch
diff mbox series

diff --git a/src/cam/camera_session.cpp b/src/cam/camera_session.cpp
index 605018278c5a..594cd51bb6a2 100644
--- a/src/cam/camera_session.cpp
+++ b/src/cam/camera_session.cpp
@@ -348,6 +348,9 @@  void CameraSession::processRequest(Request *request)
 {
 	const Request::BufferMap &buffers = request->buffers();
 
+	if (captureLimit_ && captureCount_ >= captureLimit_)
+		return;
+
 	/*
 	 * Compute the frame rate. The timestamp is arbitrarily retrieved from
 	 * the first buffer, as all buffers should have matching timestamps.