[libcamera-devel] [PATCH v4 16/31] libcamera: request: Store the streams in the request

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Wed, Mar 20, 2019 at 05:30:40PM +0100, Jacopo Mondi wrote:
> Store the streams which the request applies to and provide an accessor
> method to return them in a set.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  include/libcamera/request.h |  2 ++
>  src/libcamera/request.cpp   | 16 ++++++++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index 0dbd425115e8..1bf90de2c6f9 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -34,6 +34,7 @@ public:
>  
>  	int setBuffers(const std::map<Stream *, Buffer *> &streamMap);
>  	Buffer *findBuffer(Stream *stream) const;
> +	const std::set<Stream *> &streams() const { return streams_; }
>  
>  	Status status() const { return status_; }
>  
> @@ -49,6 +50,7 @@ private:
>  	Camera *camera_;
>  	std::map<Stream *, Buffer *> bufferMap_;
>  	std::unordered_set<Buffer *> pending_;
> +	std::set<Stream *> streams_;

This duplicates the keys stored in bufferMap_. One option to avoid this
would be to implement the streams() function as

std::set<Stream *> &Request::streams() const
{
	std::set<Stream *> streams;
	for (auto const &it: bufferMap_)
		streams.insert(it.first);
	return streams;
}

This comes at an additional cost at runtime, whether this is preferable
or not depends on the usage pattern, how many times do you expect this
function to be called per request ?

Depending on how the caller(s) use the returned set, an std::vector
could also be more efficient (slower lookup but faster push_back).

>  
>  	Status status_;
>  };
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index e0e77e972411..22c516208ede 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -51,6 +51,13 @@ Request::Request(Camera *camera)
>  {
>  }
>  
> +/**
> + * \fn Request::streams()
> + * \brief Retrieve the set of streams contained in the request
> + *
> + * \return The set of streams contained in the request
> + */
> +
>  /**
>   * \brief Set the streams to capture with associated buffers
>   * \param[in] streamMap The map of streams to buffers
> @@ -65,6 +72,10 @@ int Request::setBuffers(const std::map<Stream *, Buffer *> &streamMap)
>  	}
>  
>  	bufferMap_ = streamMap;
> +
> +	for (const auto &map : streamMap)
> +		streams_.insert(map.first);
> +
>  	return 0;
>  }
>  
> @@ -77,6 +88,11 @@ int Request::setBuffers(const std::map<Stream *, Buffer *> &streamMap)
>   * map.
>   */
>  
> +/**
> + * \var Request::streams_
> + * \brief Set of streams in this request
> + */
> +
>  /**
>   * \brief Return the buffer associated with a stream
>   * \param[in] stream The stream the buffer is associated to

-- 
Regards,

Laurent Pinchart