[libcamera-devel] [PATCH v4 12/31] libcamera: ipu3: Connect CIO2 and ImgU bufferReady signals

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Mar 20, 2019 at 05:30:36PM +0100, Jacopo Mondi wrote:
> Connect the CIO2 output bufferRead signal to a slot that simply
> queue the received buffer to ImgU for processing, and connect the ImgU
> main output bufferReady signal to the cameraData slot that notifies
> to applications that a new image buffer is available.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 58 +++++++++++++++++++++++++---
>  1 file changed, 53 insertions(+), 5 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index 8410e1f4b4a6..2623b2fe65f1 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -124,7 +124,9 @@ private:
>  		{
>  		}
>  
> -		void bufferReady(Buffer *buffer);
> +		void imguOutputBufferReady(Buffer *buffer);
> +		void imguInputBufferReady(Buffer *buffer);
> +		void cio2BufferReady(Buffer *buffer);
>  
>  		CIO2Device cio2;
>  		ImgUDevice *imgu;
> @@ -398,6 +400,21 @@ int PipelineHandlerIPU3::start(Camera *camera)
>  	IPU3CameraData *data = cameraData(camera);
>  	int ret;
>  
> +	/*
> +	 * Connect video devices' 'bufferReady' signals to their slot to
> +	 * implement the image processing pipeline.
> +	 *
> +	 * Frames produced by the CIO2 unit are shared with the associated
> +	 * ImgU input where they get processed and returned through the ImgU
> +	 * main and secondary outputs.
> +	 */
> +	data->cio2.output->bufferReady.connect(data,
> +					&IPU3CameraData::cio2BufferReady);
> +	data->imgu->input->bufferReady.connect(data,
> +					&IPU3CameraData::imguInputBufferReady);
> +	data->imgu->output->bufferReady.connect(data,
> +					&IPU3CameraData::imguOutputBufferReady);
> +

This should be done when registering cameras, not at start() time,
otherwise you'll have multiple connections when restarting cameras. This
call for a camera stream restart test.

>  	/*
>  	 * Enqueue all available buffers to the CIO2 unit to start frame
>  	 * capture. Start ImgU video devices and queue buffers to the output
> @@ -986,9 +1003,6 @@ void PipelineHandlerIPU3::registerCameras()
>  								cameraName,
>  								streams);
>  
> -		cio2->output->bufferReady.connect(data.get(),
> -						  &IPU3CameraData::bufferReady);
> -
>  		registerCamera(std::move(camera), std::move(data));
>  
>  		LOG(IPU3, Info)
> @@ -1000,7 +1014,29 @@ void PipelineHandlerIPU3::registerCameras()
>  	}
>  }
>  
> -void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)
> +/* ----------------------------------------------------------------------------

You're allowed one more dash ;-)

> + * Buffer Ready slots
> + */
> +
> +/**
> + * \brief ImgU input BufferReady slot

How about "Handle buffers completion at the ImgU input" ? Same for the
other functions below.

> + * \param buffer The completed buffer
> + *
> + * Buffer completed from the ImgU input are immediately queued back to the

s/Buffer/Buffers/

Same below.

> + * CIO2 unit to continue frame capture.
> + */
> +void PipelineHandlerIPU3::IPU3CameraData::imguInputBufferReady(Buffer *buffer)
> +{
> +	cio2.output->queueBuffer(buffer);
> +}
> +
> +/**
> + * \brief ImgU output BufferReady slot
> + * \param buffer The completed buffer
> + *
> + * Buffer completed from the ImgU output are directed to the applications.

s/applications/application/

> + */
> +void PipelineHandlerIPU3::IPU3CameraData::imguOutputBufferReady(Buffer *buffer)
>  {
>  	Request *request = queuedRequests_.front();
>  
> @@ -1008,6 +1044,18 @@ void PipelineHandlerIPU3::IPU3CameraData::bufferReady(Buffer *buffer)
>  	pipe_->completeRequest(camera_, request);
>  }
>  
> +/**
> + * \brief CIO2 BufferReady slot
> + * \param buffer The completed buffer
> + *
> + * Buffer completed from the CIO2 are immediately queued to the ImgU unit
> + * for further processing.
> + */
> +void PipelineHandlerIPU3::IPU3CameraData::cio2BufferReady(Buffer *buffer)
> +{
> +	imgu->input->queueBuffer(buffer);

Now this is where the fun will begin. What happens if the application
gets slow and has no request queued ? I think you'll need to requeue the
buffer to the cio2 instead of the imgu input in that case.

> +}
> +
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3);
>  
>  } /* namespace libcamera */

-- 
Regards,

Laurent Pinchart