libcamera: framebuffer: Make FrameBuffer class Extensible

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@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

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_; }
+	Request *request() const;
-	void setRequest(Request *request) { request_ = request; }
 	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_;

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__ */

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,10 @@ 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
-/**
+{
- * \fn FrameBuffer::setRequest()
+	return _d()->request_;
- * \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.
- */
 
 /**
  * \fn FrameBuffer::metadata()

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);

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);
+	paramBuffer->_d()->setRequest(request);
-	statBuffer->setRequest(request);
+	statBuffer->_d()->setRequest(request);
 
 	availableParamBuffers_.pop();
 	availableStatBuffers_.pop();

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;