From c20d3f5575cbb73adc8ad3cf6baac817ce8bd119 Mon Sep 17 00:00:00 2001
From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Date: Sun, 2 Oct 2022 02:21:16 +0300
Subject: libcamera: framebuffer: Move remaining private data to Private class

Private members of the FrameBuffer class are split between FrameBuffer
and FrameBuffer::Private. There was no real justification for this
split, and keeping some members private in the FrameBuffer class causes
multiple issues:

- Future modifications of the FrameBuffer class without breaking the ABI
  may be more difficult.
- Mutable access to members that should not be modified by applications
  require a friend statement, or going through the Private class.

Move all remaining private members to the Private class to address the
first issue, and add a Private::metadata() function to address the
second problem.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Tested-by: Naushir Patuck <naush@raspberrypi.com>
---
 src/libcamera/framebuffer.cpp | 58 ++++++++++++++++++++++++++++---------------
 1 file changed, 38 insertions(+), 20 deletions(-)

(limited to 'src/libcamera/framebuffer.cpp')

diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
index 7be18560..5a7f3c0b 100644
--- a/src/libcamera/framebuffer.cpp
+++ b/src/libcamera/framebuffer.cpp
@@ -114,9 +114,16 @@ LOG_DEFINE_CATEGORY(Buffer)
  * pipeline handlers.
  */
 
-FrameBuffer::Private::Private()
-	: request_(nullptr), isContiguous_(true)
+/**
+ * \brief Construct a FrameBuffer::Private instance
+ * \param[in] planes The frame memory planes
+ * \param[in] cookie Cookie
+ */
+FrameBuffer::Private::Private(const std::vector<Plane> &planes, uint64_t cookie)
+	: planes_(planes), cookie_(cookie), request_(nullptr),
+	  isContiguous_(true)
 {
+	metadata_.planes_.resize(planes_.size());
 }
 
 /**
@@ -194,6 +201,12 @@ FrameBuffer::Private::~Private()
  * indicate that the metadata is invalid.
  */
 
+/**
+ * \fn FrameBuffer::Private::metadata()
+ * \brief Retrieve the dynamic metadata
+ * \return Dynamic metadata for the frame contained in the buffer
+ */
+
 /**
  * \class FrameBuffer
  * \brief Frame buffer data and its associated dynamic metadata
@@ -291,29 +304,22 @@ ino_t fileDescriptorInode(const SharedFD &fd)
  * \param[in] cookie Cookie
  */
 FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie)
-	: FrameBuffer(std::make_unique<Private>(), planes, cookie)
+	: FrameBuffer(std::make_unique<Private>(planes, cookie))
 {
 }
 
 /**
- * \brief Construct a FrameBuffer with an extensible private class and an array
- * of planes
+ * \brief Construct a FrameBuffer with an extensible private class
  * \param[in] d The extensible private class
- * \param[in] planes The frame memory planes
- * \param[in] cookie Cookie
  */
-FrameBuffer::FrameBuffer(std::unique_ptr<Private> d,
-			 const std::vector<Plane> &planes,
-			 unsigned int cookie)
-	: Extensible(std::move(d)), planes_(planes), cookie_(cookie)
+FrameBuffer::FrameBuffer(std::unique_ptr<Private> d)
+	: Extensible(std::move(d))
 {
-	metadata_.planes_.resize(planes_.size());
-
 	unsigned int offset = 0;
 	bool isContiguous = true;
 	ino_t inode = 0;
 
-	for (const auto &plane : planes_) {
+	for (const auto &plane : _d()->planes_) {
 		ASSERT(plane.offset != Plane::kInvalidOffset);
 
 		if (plane.offset != offset) {
@@ -325,9 +331,9 @@ FrameBuffer::FrameBuffer(std::unique_ptr<Private> d,
 		 * Two different dmabuf file descriptors may still refer to the
 		 * same dmabuf instance. Check this using inodes.
 		 */
-		if (plane.fd != planes_[0].fd) {
+		if (plane.fd != _d()->planes_[0].fd) {
 			if (!inode)
-				inode = fileDescriptorInode(planes_[0].fd);
+				inode = fileDescriptorInode(_d()->planes_[0].fd);
 			if (fileDescriptorInode(plane.fd) != inode) {
 				isContiguous = false;
 				break;
@@ -344,10 +350,13 @@ FrameBuffer::FrameBuffer(std::unique_ptr<Private> d,
 }
 
 /**
- * \fn FrameBuffer::planes()
  * \brief Retrieve the static plane descriptors
  * \return Array of plane descriptors
  */
+const std::vector<FrameBuffer::Plane> &FrameBuffer::planes() const
+{
+	return _d()->planes_;
+}
 
 /**
  * \brief Retrieve the request this buffer belongs to
@@ -368,13 +377,15 @@ Request *FrameBuffer::request() const
 }
 
 /**
- * \fn FrameBuffer::metadata()
  * \brief Retrieve the dynamic metadata
  * \return Dynamic metadata for the frame contained in the buffer
  */
+const FrameMetadata &FrameBuffer::metadata() const
+{
+	return _d()->metadata_;
+}
 
 /**
- * \fn FrameBuffer::cookie()
  * \brief Retrieve the cookie
  *
  * The cookie belongs to the creator of the FrameBuffer, which controls its
@@ -384,9 +395,12 @@ Request *FrameBuffer::request() const
  *
  * \return The cookie
  */
+uint64_t FrameBuffer::cookie() const
+{
+	return _d()->cookie_;
+}
 
 /**
- * \fn FrameBuffer::setCookie()
  * \brief Set the cookie
  * \param[in] cookie Cookie to set
  *
@@ -395,6 +409,10 @@ Request *FrameBuffer::request() const
  * modify the cookie value of buffers they haven't created themselves. The
  * libcamera core never modifies the buffer cookie.
  */
+void FrameBuffer::setCookie(uint64_t cookie)
+{
+	_d()->cookie_ = cookie;
+}
 
 /**
  * \brief Extract the Fence associated with this Framebuffer
-- 
cgit v1.2.1