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

Message ID 20190403150735.27580-7-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: ipu3: Multiple streams support
Related show

Commit Message

Jacopo Mondi April 3, 2019, 3:07 p.m. UTC
Add support for queue request on the main and secondary outputs of the
ImgU.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/pipeline/ipu3/ipu3.cpp | 26 ++++++++++++++------------
 1 file changed, 14 insertions(+), 12 deletions(-)

Comments

Niklas Söderlund April 5, 2019, 11:39 a.m. UTC | #1
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@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@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

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);
+		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);