[libcamera-devel,v3,4/7] libcamera: pipeline: rkisp1: Do not try to process cancelled frames

3425 diff mbox series

There is no need to try and process cancelled frames, try to finish as
quickly as possible.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
* Changes since v2
- If the buffer intended for the application is canceled complete it and
  the request as to not lockup applications.
---
 src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Hi Niklas,

Thank you for the patch.

On Sat, Apr 11, 2020 at 02:19:08AM +0200, Niklas Söderlund wrote:
> There is no need to try and process cancelled frames, try to finish as
> quickly as possible.
> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
> * Changes since v2
> - If the buffer intended for the application is canceled complete it and
>   the request as to not lockup applications.
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index de90615edf217cca..daf84b5ac129efc8 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1015,6 +1015,12 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  	RkISP1CameraData *data = cameraData(activeCamera_);
>  	Request *request = buffer->request();
>  
> +	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
> +		completeBuffer(activeCamera_, request, buffer);
> +		completeRequest(activeCamera_, request);
> +		return;
> +	}
> +
>  	data->timeline_.bufferReady(buffer);
>  
>  	if (data->frame_ <= buffer->metadata().sequence)
> @@ -1026,6 +1032,9 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  
>  void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
>  {
> +	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> +		return;
> +
>  	ASSERT(activeCamera_);
>  	RkISP1CameraData *data = cameraData(activeCamera_);
>  
> @@ -1037,6 +1046,9 @@ void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
>  
>  void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>  {
> +	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> +		return;
> +

This looks fine, but will keep data in frameInfo_ after stop(). I think
you need to add a clear() operation to RkISP1Frames() and call it in
stop(). It can be done in a separate patch, so

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	ASSERT(activeCamera_);
>  	RkISP1CameraData *data = cameraData(activeCamera_);
>