[libcamera-devel] [PATCH v4 10/12] libcamera: ipu3: Connect viewfinder's BufferReady signal

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2019-04-09 21:25:46 +0200, Jacopo Mondi wrote:
> Connect the viewfinder buffer ready signal to the IPU3CameraData slot
> that complets the buffer first, and if not waiting for other buffers
> completes the request as well.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 43 ++++++++++++++++++++++++----
>  1 file changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index bb8d4ce644ca..75ffdc56d157 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -182,7 +182,7 @@ private:
>  		{
>  		}
>  
> -		void imguOutputBufferReady(Buffer *buffer);
> +		void imguCaptureBufferReady(Buffer *buffer);
>  		void imguInputBufferReady(Buffer *buffer);
>  		void cio2BufferReady(Buffer *buffer);
>  
> @@ -722,7 +722,9 @@ int PipelineHandlerIPU3::registerCameras()
>  		data->imgu_->input_->bufferReady.connect(data.get(),
>  					&IPU3CameraData::imguInputBufferReady);
>  		data->imgu_->output_.dev->bufferReady.connect(data.get(),
> -					&IPU3CameraData::imguOutputBufferReady);
> +					&IPU3CameraData::imguCaptureBufferReady);
> +		data->imgu_->viewfinder_.dev->bufferReady.connect(data.get(),
> +					&IPU3CameraData::imguCaptureBufferReady);
>  
>  
>  		/* Initialize and register the Camera and its streams. */
> @@ -769,12 +771,41 @@ void PipelineHandlerIPU3::IPU3CameraData::imguInputBufferReady(Buffer *buffer)
>   *
>   * Buffers completed from the ImgU output are directed to the application.
>   */
> -void PipelineHandlerIPU3::IPU3CameraData::imguOutputBufferReady(Buffer *buffer)
> +void PipelineHandlerIPU3::IPU3CameraData::imguCaptureBufferReady(Buffer *buffer)
>  {
> -	Request *request = queuedRequests_.front();
> +	Request *request = buffer->request();
> +	if (!request)
> +		/*
> +		 * Completed buffers not part of a request are ignored
> +		 * (they most probably come from the output stream
> +		 * internal pool)
> +		 */
> +		return;
> +
> +	if (!pipe_->completeBuffer(camera_, request, buffer))
> +		/* Request not completed yet, return here. */
> +		return;
> +
> +	/*
> +	 * Complete requests in queuing order: if some other request is
> +	 * pending, post-pone completion.
> +	 */
> +	Request *front = queuedRequests_.front();
> +	if (front != request)
> +		return;

I'm not sure, but i think this needs to be moved to the framework 
somehow. It will be slight variations of how this will be implemented in 
different pipeline handlers if we leave it to each pipeline handler to 
implement ordered completion of requests.

Best case it should happen transparent to the pipeline handlers, but 
that might be a tall order. How about implementing a 
PipelineHandler::completeReady() helper which implementations can call 
which would take care of dequeuing requests in a ordered fashion? Or if 
you have a nice idea for how to skip the helper and move to something 
transparent that would be even better.

>  
> -	pipe_->completeBuffer(camera_, request, buffer);
> -	pipe_->completeRequest(camera_, request);
> +	/*
> +	 * Complete the current request, and all the other pending ones,
> +	 * in queuing order.
> +	 */
> +	while (1) {
> +		if (front->empty())
> +			pipe_->completeRequest(camera_, front);
> +		else
> +			break;
> +
> +		front = queuedRequests_.front();
> +	}
>  }
>  
>  /**
> -- 
> 2.21.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund