[libcamera-devel] [PATCH 2/3] libcamera: ipu3: Do not re-queue failed buffers

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Mon, Jul 15, 2019 at 07:59:34AM +0200, Jacopo Mondi wrote:
> When a video device is stopped all the buffers there queued are
> released and their state is set to failure.

Isn't the state set to BufferCancelled, not BufferError ?

> Currently, on buffer completion, failed buffers are blindly re-queued to
> the ImgU input or CIO2 output devices, preventing them to be re-started
> succesfully in future capture sessions.
> 
> Fix that by inspecting the buffers status and skip re-queueing if
> they're reported as failing. For the ImgU output buffers this is not
> required, as failed buffes should be anyhow delivered to applications in

s/buffes/buffers/

> order to report their failure.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 5204487684c2..11bf3af66ae6 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -916,6 +916,9 @@ int PipelineHandlerIPU3::registerCameras()
>   */
>  void IPU3CameraData::imguInputBufferReady(Buffer *buffer)
>  {
> +	if (buffer->status() != Buffer::BufferSuccess)
> +		return;
> +
>  	cio2_.output_->queueBuffer(buffer);
>  }
>  
> @@ -946,6 +949,9 @@ void IPU3CameraData::imguOutputBufferReady(Buffer *buffer)
>   */
>  void IPU3CameraData::cio2BufferReady(Buffer *buffer)
>  {
> +	if (buffer->status() != Buffer::BufferSuccess)
> +		return;
> +

We may need to differentiate between Buffer::BufferCancelled and
Buffer::BufferError. The former clearly needs to return immediately,
while the latter should probably retry one way or another, at least a
finite number of times. Should we already test ==
Buffer::BufferCancelled, and ignore BufferError for now ?

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

>  	imgu_->input_->queueBuffer(buffer);
>  }

-- 
Regards,

Laurent Pinchart