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

[Laurent Pinchart]

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 at ragnatech.se>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Signed-off-by: Jacopo Mondi <jacopo at 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

-- 
Regards,

Laurent Pinchart