[libcamera-devel,3/3] libcamera: pipeline: rkisp1: Allow requests to be cancelled
diff mbox series

Message ID 20210406182335.85847-4-nfraprado@collabora.com
State Accepted
Commit e51926f07bfe26db6ae68b72bc64f1523968742d
Headers show
Series
  • libcamera: pipeline: rkisp1: Fix issues exposed by lc-compliance
Related show

Commit Message

Nícolas F. R. A. Prado April 6, 2021, 6:23 p.m. UTC
Previously when a frame got cancelled, the frameInfo flags
metadataProcessed and paramDequeued wouldn't get set, meaning that the
request wasn't able to be completed and cancelled.

Make sure that these flags are set when the frame gets cancelled so that
the request can be cancelled.

This issue happened while running lc-compliance.

Suggested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

Comments

Niklas Söderlund April 14, 2021, 2:08 p.m. UTC | #1
Hi Nícolas,

Thanks for your work.

On 2021-04-06 15:23:35 -0300, Nícolas F. R. A. Prado wrote:
> Previously when a frame got cancelled, the frameInfo flags
> metadataProcessed and paramDequeued wouldn't get set, meaning that the
> request wasn't able to be completed and cancelled.
> 
> Make sure that these flags are set when the frame gets cancelled so that
> the request can be cancelled.
> 
> This issue happened while running lc-compliance.
> 
> Suggested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 3c8d9dfe6f87..549f4a4e61a8 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1073,9 +1073,6 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  
>  void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
>  {
> -	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> -		return;
> -
>  	ASSERT(activeCamera_);
>  	RkISP1CameraData *data = cameraData(activeCamera_);
>  
> @@ -1087,9 +1084,6 @@ void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
>  
>  void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>  {
> -	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> -		return;
> -
>  	ASSERT(activeCamera_);
>  	RkISP1CameraData *data = cameraData(activeCamera_);
>  
> @@ -1097,6 +1091,12 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>  	if (!info)
>  		return;
>  
> +	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
> +		info->metadataProcessed = true;
> +		tryCompleteRequest(info->request);
> +		return;
> +	}
> +
>  	if (data->frame_ <= buffer->metadata().sequence)
>  		data->frame_ = buffer->metadata().sequence + 1;
>  
> -- 
> 2.31.1
>
Kieran Bingham April 14, 2021, 2:12 p.m. UTC | #2
On 06/04/2021 19:23, Nícolas F. R. A. Prado wrote:
> Previously when a frame got cancelled, the frameInfo flags
> metadataProcessed and paramDequeued wouldn't get set, meaning that the
> request wasn't able to be completed and cancelled.
> 
> Make sure that these flags are set when the frame gets cancelled so that
> the request can be cancelled.
> 
> This issue happened while running lc-compliance.
> 
> Suggested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>

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


> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 ++++++------
>  1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index 3c8d9dfe6f87..549f4a4e61a8 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1073,9 +1073,6 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  
>  void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
>  {
> -	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> -		return;
> -
>  	ASSERT(activeCamera_);
>  	RkISP1CameraData *data = cameraData(activeCamera_);
>  
> @@ -1087,9 +1084,6 @@ void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
>  
>  void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>  {
> -	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> -		return;
> -
>  	ASSERT(activeCamera_);
>  	RkISP1CameraData *data = cameraData(activeCamera_);
>  
> @@ -1097,6 +1091,12 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>  	if (!info)
>  		return;
>  
> +	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
> +		info->metadataProcessed = true;
> +		tryCompleteRequest(info->request);
> +		return;
> +	}
> +
>  	if (data->frame_ <= buffer->metadata().sequence)
>  		data->frame_ = buffer->metadata().sequence + 1;
>  
>

Patch
diff mbox series

diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
index 3c8d9dfe6f87..549f4a4e61a8 100644
--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
@@ -1073,9 +1073,6 @@  void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
 
 void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
 {
-	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
-		return;
-
 	ASSERT(activeCamera_);
 	RkISP1CameraData *data = cameraData(activeCamera_);
 
@@ -1087,9 +1084,6 @@  void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
 
 void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
 {
-	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
-		return;
-
 	ASSERT(activeCamera_);
 	RkISP1CameraData *data = cameraData(activeCamera_);
 
@@ -1097,6 +1091,12 @@  void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
 	if (!info)
 		return;
 
+	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
+		info->metadataProcessed = true;
+		tryCompleteRequest(info->request);
+		return;
+	}
+
 	if (data->frame_ <= buffer->metadata().sequence)
 		data->frame_ = buffer->metadata().sequence + 1;