[libcamera-devel] [PATCH v6 4/6] libcamera: ipu3: Queue requests for multiple streams

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Thu, Apr 18, 2019 at 06:46:36PM +0200, Jacopo Mondi wrote:
> Add support for queueing requests for multiple streams in the IPU3
> pipeline handler class. The output video node should be queued with
> buffers even if not part of the requested streams.

Is this still the case ?

> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 23 +++++++++--------------
>  1 file changed, 9 insertions(+), 14 deletions(-)
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index d14ba47e1c2c..8353272642bd 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -545,25 +545,20 @@ void PipelineHandlerIPU3::stop(Camera *camera)
>  
>  int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)
>  {
> -	IPU3CameraData *data = cameraData(camera);
> -	V4L2Device *output = data->imgu_->output_.dev;
> -	IPU3Stream *stream = &data->outStream_;
> +	int ret = 0;
>  
> -	/* 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 (auto it : request->buffers()) {
> +		IPU3Stream *stream = static_cast<IPU3Stream *>(it.first);
> +		Buffer *buffer = it.second;
>  
> -	int ret = output->queueBuffer(buffer);
> -	if (ret < 0)
> -		return ret;
> +		int status = stream->device_->dev->queueBuffer(buffer);
> +		if (status < 0)
> +			ret = status;

I'd rather s/ret/error/ and s/status/ret/ as ret is a very common name
for temporary return values. Or possibly just swap ret and status.

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

> +	}
>  
>  	PipelineHandler::queueRequest(camera, request);
>  
> -	return 0;
> +	return ret;
>  }
>  
>  bool PipelineHandlerIPU3::match(DeviceEnumerator *enumerator)

-- 
Regards,

Laurent Pinchart