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

[Laurent Pinchart]

Hi Niklas,

Thank you for the patch.

On Thu, Mar 26, 2020 at 05:08:16PM +0100, Niklas Söderlund wrote:
> Their is no need to try and process cancelled frames.

s/Their/There/

> 
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> index f64807984519779b..f49b3a7f0945ac92 100644
> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp
> @@ -1011,6 +1011,9 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)
>  
>  void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  {
> +	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> +		return;
> +
>  	ASSERT(activeCamera_);
>  	RkISP1CameraData *data = cameraData(activeCamera_);
>  	Request *request = buffer->request();

For the capture buffer, you actually need to process it in order to
complete the request. You'll need a special case there, as you should
not wait for metadata in that case (we're stopping, metadata may never
arrive).

> @@ -1026,6 +1029,9 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)
>  
>  void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
>  {
> +	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> +		return;
> +
>  	ASSERT(activeCamera_);
>  	RkISP1CameraData *data = cameraData(activeCamera_);

For the params buffer, I think it should not take part in request
completion at all, so the logic in this function has to be reworked.

>  
> @@ -1037,6 +1043,9 @@ void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)
>  
>  void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)
>  {
> +	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
> +		return;
> +
>  	ASSERT(activeCamera_);
>  	RkISP1CameraData *data = cameraData(activeCamera_);
>  

-- 
Regards,

Laurent Pinchart