[libcamera-devel] [PATCH v7 7/8] libcamera: ipu3: Connect viewfinder's BufferReady signal

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Fri, Apr 19, 2019 at 12:18:38PM +0200, Jacopo Mondi wrote:
> The viewfinder and main output require identical logic for buffer and
> request completion. Connect the viewfinder bufferReady signal to the slot
> and handle requests for both main output and viewfinder there.
> 
> Update the slot logic to complete the request only when the last buffer
> has completed, and make sure to complete requests in the same order they
> have been queued to the pipeline handler.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 25 ++++++++++++++++++++++---
>  1 file changed, 22 insertions(+), 3 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 8fa59157532e..1964603c591f 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -747,6 +747,8 @@ int PipelineHandlerIPU3::registerCameras()
>  					&IPU3CameraData::imguInputBufferReady);
>  		data->imgu_->output_.dev->bufferReady.connect(data.get(),
>  					&IPU3CameraData::imguOutputBufferReady);
> +		data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(),
> +					&IPU3CameraData::imguOutputBufferReady);
>  
>  		/* Create and register the Camera instance. */
>  		std::string cameraName = cio2->sensor_->entity()->name() + " "
> @@ -792,10 +794,27 @@ void PipelineHandlerIPU3::IPU3CameraData::imguInputBufferReady(Buffer *buffer)
>   */
>  void PipelineHandlerIPU3::IPU3CameraData::imguOutputBufferReady(Buffer *buffer)
>  {
> -	Request *request = queuedRequests_.front();
> +	Request *request = buffer->request();
> +
> +	if (!pipe_->completeBuffer(camera_, request, buffer))
> +		/* Request not completed yet, return here. */
> +		return;
>  
> -	pipe_->completeBuffer(camera_, request, buffer);
> -	pipe_->completeRequest(camera_, request);
> +	/*
> +	 * Complete requests in queuing order: if some other request is
> +	 * pending, postpone completion.
> +	 */
> +	if (request != queuedRequests_.front())
> +		return;
> +

I think you can remove this check, the while loop below completes all
ready requests in queuing order and should be enough. With this fixed,

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

> +	/* Complete the pending requests in queuing order. */
> +	while (1) {
> +		request = queuedRequests_.front();
> +		if (request->hasPendingBuffers())
> +			break;
> +
> +		pipe_->completeRequest(camera_, request);
> +	}
>  }
>  
>  /**

-- 
Regards,

Laurent Pinchart