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

Message ID 20190704225334.26170-6-jacopo@jmondi.org
State Superseded
Headers show
Series
  • Add support for external bufferes
Related show

Commit Message

Jacopo Mondi July 4, 2019, 10:53 p.m. UTC
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@jmondi.org>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 include/libcamera/request.h |  4 ++--
 src/libcamera/camera.cpp    |  2 +-
 src/libcamera/request.cpp   | 16 ++++++++--------
 3 files changed, 11 insertions(+), 11 deletions(-)

Comments

Kieran Bingham July 8, 2019, 9:44 a.m. UTC | #1
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@jmondi.org>
> Reviewed-by: Niklas Söderlund <niklas.soderlund@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);
>

Patch

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