[libcamera-devel] [PATCH v3 6/8] libcamera: ipu3: Queue request for multiple streams

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2019-04-03 17:07:33 +0200, Jacopo Mondi wrote:
> Add support for queue request on the main and secondary outputs of the
> ImgU.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 26 ++++++++++++++------------
>  1 file changed, 14 insertions(+), 12 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index f7e75fac1dfe..8c67cf985d1e 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -572,20 +572,22 @@ void PipelineHandlerIPU3::stop(Camera *camera)
>  int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)
>  {
>  	IPU3CameraData *data = cameraData(camera);
> -	V4L2Device *output = data->imgu_->output_.dev;
> -	Stream *stream = &data->outStream_;
>  
> -	/* Queue a buffer to the ImgU output for capture. */
> -	Buffer *buffer = request->findBuffer(stream);
> -	if (!buffer) {
> -		LOG(IPU3, Error)
> -			<< "Attempt to queue request with invalid stream";
> -		return -ENOENT;
> -	}
> +	for (Stream *stream : request->streams()) {
> +		Buffer *buffer = request->findBuffer(stream);

As this is in a critical code path I'm not loving how this turned out.

First request->streams() iterates Request::bufferMap_ to build a list of 
Streams * which are then yet again used to lookup stuff in the same map.  
Would it not be nicer just to expose Request::bufferMap_ to the pipeline 
handlers?

> +		if (!buffer) {
> +			LOG(IPU3, Error) << "Attempt to queue invalid request";
> +			return -ENOENT;
> +		}
>  
> -	int ret = output->queueBuffer(buffer);
> -	if (ret < 0)
> -		return ret;
> +		V4L2Device *videoDevice = isOutput(data, stream)
> +					? data->imgu_->output_.dev
> +					: data->imgu_->viewfinder_.dev;
> +
> +		int ret = videoDevice->queueBuffer(buffer);
> +		if (ret)
> +			return ret;
> +	}
>  
>  	PipelineHandler::queueRequest(camera, request);
>  
> -- 
> 2.21.0
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund