[libcamera-devel] [PATCH 3/3] libcamera: framebuffer: Make FrameBuffer class Extensible

[Laurent Pinchart]

Implement the D-Pointer design pattern in the FrameBuffer class to allow
changing internal data without affecting the public ABI.

Move the request_ field and the setRequest() function to the
FrameBuffer::Private class. This allows hiding the setRequest() function
from the public API, removing one todo item. More fields may be moved
later.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
 include/libcamera/framebuffer.h          |  8 +++---
 include/libcamera/internal/framebuffer.h | 13 +++++++++
 src/libcamera/framebuffer.cpp            | 34 +++++++++++++++---------
 src/libcamera/pipeline/ipu3/cio2.cpp     |  3 ++-
 src/libcamera/pipeline/ipu3/frames.cpp   |  5 ++--
 src/libcamera/request.cpp                |  7 ++---
 6 files changed, 47 insertions(+), 23 deletions(-)

diff --git a/include/libcamera/framebuffer.h b/include/libcamera/framebuffer.h
index baf22a466907..7265e816b036 100644
--- a/include/libcamera/framebuffer.h
+++ b/include/libcamera/framebuffer.h
@@ -35,8 +35,10 @@ struct FrameMetadata {
 	std::vector<Plane> planes;
 };
 
-class FrameBuffer final
+class FrameBuffer final : public Extensible
 {
+	LIBCAMERA_DECLARE_PRIVATE();
+
 public:
 	struct Plane {
 		FileDescriptor fd;
@@ -47,8 +49,7 @@ public:
 
 	const std::vector<Plane> &planes() const { return planes_; }
 
-	Request *request() const { return request_; }
-	void setRequest(Request *request) { request_ = request; }
+	Request *request() const;
 	const FrameMetadata &metadata() const { return metadata_; }
 
 	unsigned int cookie() const { return cookie_; }
@@ -63,7 +64,6 @@ private:
 
 	std::vector<Plane> planes_;
 
-	Request *request_;
 	FrameMetadata metadata_;
 
 	unsigned int cookie_;
diff --git a/include/libcamera/internal/framebuffer.h b/include/libcamera/internal/framebuffer.h
index 0c76fc62af1d..a11e895d9b88 100644
--- a/include/libcamera/internal/framebuffer.h
+++ b/include/libcamera/internal/framebuffer.h
@@ -47,6 +47,19 @@ public:
 	MappedFrameBuffer(const FrameBuffer *buffer, int flags);
 };
 
+class FrameBuffer::Private : public Extensible::Private
+{
+	LIBCAMERA_DECLARE_PUBLIC(FrameBuffer)
+
+public:
+	Private(FrameBuffer *buffer);
+
+	void setRequest(Request *request) { request_ = request; }
+
+private:
+	Request *request_;
+};
+
 } /* namespace libcamera */
 
 #endif /* __LIBCAMERA_INTERNAL_FRAMEBUFFER_H__ */
diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
index 4bde556c4013..5e13e281fb8c 100644
--- a/src/libcamera/framebuffer.cpp
+++ b/src/libcamera/framebuffer.cpp
@@ -100,6 +100,21 @@ LOG_DEFINE_CATEGORY(Buffer)
  * \brief Array of per-plane metadata
  */
 
+FrameBuffer::Private::Private(FrameBuffer *buffer)
+	: Extensible::Private(buffer), request_(nullptr)
+{
+}
+
+/**
+ * \fn FrameBuffer::Private::setRequest()
+ * \brief Set the request this buffer belongs to
+ * \param[in] request Request to set
+ *
+ * For buffers added to requests by applications, this method is called by
+ * Request::addBuffer() or Request::reuse(). For buffers internal to pipeline
+ * handlers, it is called by the pipeline handlers themselves.
+ */
+
 /**
  * \class FrameBuffer
  * \brief Frame buffer data and its associated dynamic metadata
@@ -161,7 +176,7 @@ LOG_DEFINE_CATEGORY(Buffer)
  * \param[in] cookie Cookie
  */
 FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie)
-	: planes_(planes), request_(nullptr), cookie_(cookie)
+	: Extensible(new Private(this)), planes_(planes), cookie_(cookie)
 {
 }
 
@@ -172,7 +187,6 @@ FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie)
  */
 
 /**
- * \fn FrameBuffer::request()
  * \brief Retrieve the request this buffer belongs to
  *
  * The intended callers of this method are buffer completion handlers that
@@ -185,18 +199,12 @@ FrameBuffer::FrameBuffer(const std::vector<Plane> &planes, unsigned int cookie)
  * \return The Request the FrameBuffer belongs to, or nullptr if the buffer is
  * not associated with a request
  */
+Request *FrameBuffer::request() const
+{
+	const Private *const d = LIBCAMERA_D_PTR();
 
-/**
- * \fn FrameBuffer::setRequest()
- * \brief Set the request this buffer belongs to
- * \param[in] request Request to set
- *
- * For buffers added to requests by applications, this method is called by
- * Request::addBuffer() or Request::reuse(). For buffers internal to pipeline
- * handlers, it is called by the pipeline handlers themselves.
- *
- * \todo Shall be hidden from applications with a d-pointer design.
- */
+	return d->request_;
+}
 
 /**
  * \fn FrameBuffer::metadata()
diff --git a/src/libcamera/pipeline/ipu3/cio2.cpp b/src/libcamera/pipeline/ipu3/cio2.cpp
index 1be2cbcd5cac..1bcd580e251c 100644
--- a/src/libcamera/pipeline/ipu3/cio2.cpp
+++ b/src/libcamera/pipeline/ipu3/cio2.cpp
@@ -14,6 +14,7 @@
 #include <libcamera/stream.h>
 
 #include "libcamera/internal/camera_sensor.h"
+#include "libcamera/internal/framebuffer.h"
 #include "libcamera/internal/media_device.h"
 #include "libcamera/internal/v4l2_subdevice.h"
 
@@ -278,7 +279,7 @@ FrameBuffer *CIO2Device::queueBuffer(Request *request, FrameBuffer *rawBuffer)
 
 		buffer = availableBuffers_.front();
 		availableBuffers_.pop();
-		buffer->setRequest(request);
+		buffer->_d()->setRequest(request);
 	}
 
 	int ret = output_->queueBuffer(buffer);
diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp
index ce5ccbf18e41..a4c3477cd9ef 100644
--- a/src/libcamera/pipeline/ipu3/frames.cpp
+++ b/src/libcamera/pipeline/ipu3/frames.cpp
@@ -10,6 +10,7 @@
 #include <libcamera/framebuffer.h>
 #include <libcamera/request.h>
 
+#include "libcamera/internal/framebuffer.h"
 #include "libcamera/internal/pipeline_handler.h"
 #include "libcamera/internal/v4l2_videodevice.h"
 
@@ -56,8 +57,8 @@ IPU3Frames::Info *IPU3Frames::create(Request *request)
 	FrameBuffer *paramBuffer = availableParamBuffers_.front();
 	FrameBuffer *statBuffer = availableStatBuffers_.front();
 
-	paramBuffer->setRequest(request);
-	statBuffer->setRequest(request);
+	paramBuffer->_d()->setRequest(request);
+	statBuffer->_d()->setRequest(request);
 
 	availableParamBuffers_.pop();
 	availableStatBuffers_.pop();
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index 5faf3c71ff02..c095c9f45b09 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -18,6 +18,7 @@
 #include <libcamera/stream.h>
 
 #include "libcamera/internal/camera_controls.h"
+#include "libcamera/internal/framebuffer.h"
 #include "libcamera/internal/tracepoints.h"
 
 /**
@@ -121,7 +122,7 @@ void Request::reuse(ReuseFlag flags)
 	if (flags & ReuseBuffers) {
 		for (auto pair : bufferMap_) {
 			FrameBuffer *buffer = pair.second;
-			buffer->setRequest(this);
+			buffer->_d()->setRequest(this);
 			pending_.insert(buffer);
 		}
 	} else {
@@ -191,7 +192,7 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer)
 		return -EEXIST;
 	}
 
-	buffer->setRequest(this);
+	buffer->_d()->setRequest(this);
 	pending_.insert(buffer);
 	bufferMap_[stream] = buffer;
 
@@ -336,7 +337,7 @@ bool Request::completeBuffer(FrameBuffer *buffer)
 	int ret = pending_.erase(buffer);
 	ASSERT(ret == 1);
 
-	buffer->setRequest(nullptr);
+	buffer->_d()->setRequest(nullptr);
 
 	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
 		cancelled_ = true;
-- 
Regards,

Laurent Pinchart