[libcamera-devel] [PATCH 5/9] libcamera: request: Rename the Stream to Buffer map

[Kieran Bingham]

Hi Jacopo,

On 04/07/2019 23:53, Jacopo Mondi wrote:
> As we're about to add support for mapping application buffers to
> streams' ones, rename the existing bufferMap_ to just buffers_, which
> also matches the associated accessor operation name.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
>  include/libcamera/request.h |  4 ++--
>  src/libcamera/camera.cpp    |  2 +-
>  src/libcamera/request.cpp   | 16 ++++++++--------
>  3 files changed, 11 insertions(+), 11 deletions(-)
> 
> diff --git a/include/libcamera/request.h b/include/libcamera/request.h
> index a93468d7c8b7..70f6d7fa7eeb 100644
> --- a/include/libcamera/request.h
> +++ b/include/libcamera/request.h
> @@ -34,7 +34,7 @@ public:
>  	Request &operator=(const Request &) = delete;
>  
>  	ControlList &controls() { return controls_; }
> -	const std::map<Stream *, Buffer *> &buffers() const { return bufferMap_; }

I think it might be good to add a

  'using BufferMap = std::map<Stream *, Buffer *>;

into stream.h somewhere...

Then this would be

  const BufferMap &buffers() const { return buffers_; }

Or potentially StreamBufferMap might work too...

Is that something that could be done at the same time as this patch? Or
should that be separate?

Otherwise, I think especially as the accessor is called Buffers(),
renaming this makes sense.


> +	const std::map<Stream *, Buffer *> &buffers() const { return buffers_; }
>  	int setBuffers(const std::map<Stream *, Buffer *> &streamMap);
>  	Buffer *findBuffer(Stream *stream) const;
>  
> @@ -53,7 +53,7 @@ private:
>  
>  	Camera *camera_;
>  	ControlList controls_;
> -	std::map<Stream *, Buffer *> bufferMap_;
> +	std::map<Stream *, Buffer *> buffers_;
>  	std::unordered_set<Buffer *> pending_;
>  
>  	Status status_;
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index f3f01d040ecf..265755f1b9e3 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -895,7 +895,7 @@ int Camera::stop()
>   */
>  void Camera::requestComplete(Request *request)
>  {
> -	std::map<Stream *, Buffer *> buffers(std::move(request->bufferMap_));
> +	std::map<Stream *, Buffer *> buffers(std::move(request->buffers_));
>  	requestCompleted.emit(request, buffers);
>  	delete request;
>  }
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index f0b5985814bd..9ff0abbf119c 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -86,20 +86,20 @@ Request::Request(Camera *camera)
>   */
>  int Request::setBuffers(const std::map<Stream *, Buffer *> &streamMap)
>  {
> -	if (!bufferMap_.empty()) {
> +	if (!buffers_.empty()) {
>  		LOG(Request, Error) << "Buffers already set";
>  		return -EBUSY;
>  	}
>  
> -	bufferMap_ = streamMap;
> +	buffers_ = streamMap;
>  	return 0;
>  }
>  
>  /**
> - * \var Request::bufferMap_
> + * \var Request::buffers_
>   * \brief Mapping of streams to buffers for this request
>   *
> - * The bufferMap_ tracks the buffers associated with each stream. If a stream is
> + * The buffers_ tracks the buffers associated with each stream. If a stream is
>   * not utilised in this request there will be no buffer for that stream in the
>   * map.
>   */
> @@ -112,8 +112,8 @@ int Request::setBuffers(const std::map<Stream *, Buffer *> &streamMap)
>   */
>  Buffer *Request::findBuffer(Stream *stream) const
>  {
> -	auto it = bufferMap_.find(stream);
> -	if (it == bufferMap_.end())
> +	auto it = buffers_.find(stream);
> +	if (it == buffers_.end())
>  		return nullptr;
>  
>  	return it->second;
> @@ -150,12 +150,12 @@ Buffer *Request::findBuffer(Stream *stream) const
>   */
>  int Request::prepare()
>  {
> -	if (bufferMap_.empty()) {
> +	if (buffers_.empty()) {
>  		LOG(Request, Error) << "Invalid request due to missing buffers";
>  		return -EINVAL;
>  	}
>  
> -	for (auto const &pair : bufferMap_) {
> +	for (auto const &pair : buffers_) {
>  		Buffer *buffer = pair.second;
>  		buffer->setRequest(this);
>  		pending_.insert(buffer);
> 

-- 
Regards
--
Kieran