[libcamera-devel,v5,6/7] libcamera: request: Expose the Stream to Buffers map

Message ID 20190415231859.9747-7-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: Framework changes to prepare for multiple streams support
Related show

Commit Message

Jacopo Mondi April 15, 2019, 11:18 p.m. UTC
Add to the Request class a method to access the map to Stream to Buffer.

Prepare the IPU3 pipeline handler to support multiple streams by
retrieving the buffers to queue from the map exposed by this change, to
better show the purpose of the patch.

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/request.h          |  1 +
 src/libcamera/pipeline/ipu3/ipu3.cpp | 19 ++++++++-----------
 src/libcamera/request.cpp            | 10 ++++++++++
 3 files changed, 19 insertions(+), 11 deletions(-)

Comments

Laurent Pinchart April 16, 2019, 11:03 a.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Tue, Apr 16, 2019 at 01:18:58AM +0200, Jacopo Mondi wrote:
> Add to the Request class a method to access the map to Stream to Buffer.

s/to Stream/of Stream/

> 
> Prepare the IPU3 pipeline handler to support multiple streams by
> retrieving the buffers to queue from the map exposed by this change, to
> better show the purpose of the patch.
> 
> Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  include/libcamera/request.h          |  1 +
>  src/libcamera/pipeline/ipu3/ipu3.cpp | 19 ++++++++-----------
>  src/libcamera/request.cpp            | 10 ++++++++++
>  3 files changed, 19 insertions(+), 11 deletions(-)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 6f97aa177ae0..e9687f7e14d3 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -32,6 +32,7 @@ public:
>  	Request(const Request &) = delete;
>  	Request &operator=(const Request &) = delete;
>  
> +	const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }
>  	int setBuffers(const std::map<Stream *, Buffer *> &streamMap);
>  	Buffer *findBuffer(Stream *stream) const;
>  
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index f96e8763bce9..7d865fa329ea 100644
> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> @@ -409,19 +409,16 @@ int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)
>  {
>  	IPU3CameraData *data = cameraData(camera);
>  	V4L2Device *output = data->imgu_->output_.dev;
> -	Stream *stream = &data->stream_;
>  
> -	/* 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;
> -	}
> +	/* Queue buffers to the ImgU output for capture. */

s/output/outputs/

This change makes no sense though, as there's a single buffer per output
:-) I would squash it with an appropriate patch from the IPU3 rework.
The changes to request.h and request.cpp can stay in this patch.

> +	for (auto &it : request->buffers()) {
> +		Buffer *buffer = it.second;
>  
> -	int ret = output->queueBuffer(buffer);
> -	if (ret < 0)
> -		return ret;
> +		int ret = output->queueBuffer(buffer);
> +		if (ret < 0)
> +			return ret;
> +

Extra blank line.

> +	}
>  
>  	PipelineHandler::queueRequest(camera, request);
>  
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 33055c5ac533..7fa034e6c747 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -51,6 +51,16 @@ Request::Request(Camera *camera)
>  {
>  }
>  
> +/**
> + * \fn Request::buffers()
> + * \brief Retrieve the request's streams to buffer map

s/buffer/buffers/

> + *
> + * Return a reference to the map that associates each Stream part of the
> + * request to the Buffer the Stream output should be directed to.
> + *
> + * \return The map of Stream to Buffer
> + */
> +
>  /**
>   * \brief Set the streams to capture with associated buffers
>   * \param[in] streamMap The map of streams to buffers

Patch

diff --git a/include/libcamera/request.h b/include/libcamera/request.h
index 6f97aa177ae0..e9687f7e14d3 100644
--- a/include/libcamera/request.h
+++ b/include/libcamera/request.h
@@ -32,6 +32,7 @@  public:
 	Request(const Request &) = delete;
 	Request &operator=(const Request &) = delete;
 
+	const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }
 	int setBuffers(const std::map<Stream *, Buffer *> &streamMap);
 	Buffer *findBuffer(Stream *stream) const;
 
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index f96e8763bce9..7d865fa329ea 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -409,19 +409,16 @@  int PipelineHandlerIPU3::queueRequest(Camera *camera, Request *request)
 {
 	IPU3CameraData *data = cameraData(camera);
 	V4L2Device *output = data->imgu_->output_.dev;
-	Stream *stream = &data->stream_;
 
-	/* 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;
-	}
+	/* Queue buffers to the ImgU output for capture. */
+	for (auto &it : request->buffers()) {
+		Buffer *buffer = it.second;
 
-	int ret = output->queueBuffer(buffer);
-	if (ret < 0)
-		return ret;
+		int ret = output->queueBuffer(buffer);
+		if (ret < 0)
+			return ret;
+
+	}
 
 	PipelineHandler::queueRequest(camera, request);
 
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index 33055c5ac533..7fa034e6c747 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -51,6 +51,16 @@  Request::Request(Camera *camera)
 {
 }
 
+/**
+ * \fn Request::buffers()
+ * \brief Retrieve the request's streams to buffer map
+ *
+ * Return a reference to the map that associates each Stream part of the
+ * request to the Buffer the Stream output should be directed to.
+ *
+ * \return The map of Stream to Buffer
+ */
+
 /**
  * \brief Set the streams to capture with associated buffers
  * \param[in] streamMap The map of streams to buffers