libcamera: request: Make Request class Extensible

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

Move the internal fields that are not needed to implement the public
API to the Request::Private class already. This allows to remove
the friend class declaration for the PipelineHandler class, which can
now use the Request::Private API.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
[Move all internal fields to Request::Private and remove friend  declaration]
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

include/libcamera/internal/meson.build

@@ -35,6 +35,7 @@ libcamera_internal_headers = files([
     'pipeline_handler.h',
     'process.h',
     'pub_key.h',
+    'request.h',
     'source_paths.h',
     'sysfs.h',
     'v4l2_device.h',

include/libcamera/internal/request.h

@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2019, Google Inc.
+ *
+ * request.h - Request class private data
+ */
+#ifndef __LIBCAMERA_INTERNAL_REQUEST_H__
+#define __LIBCAMERA_INTERNAL_REQUEST_H__
+
+#include <memory>
+
+#include <libcamera/request.h>
+
+namespace libcamera {
+
+class Camera;
+class FrameBuffer;
+
+class Request::Private : public Extensible::Private
+{
+	LIBCAMERA_DECLARE_PUBLIC(Request)
+
+public:
+	Private(Camera *camera);
+	~Private();
+
+	Camera *camera() const { return camera_; }
+	bool hasPendingBuffers() const;
+
+	bool completeBuffer(FrameBuffer *buffer);
+	void complete();
+	void cancel();
+	void reuse();
+
+private:
+	friend class PipelineHandler;
+
+	void doCancelRequest();
+
+	Camera *camera_;
+	bool cancelled_;
+	uint32_t sequence_ = 0;
+
+	std::unordered_set<FrameBuffer *> pending_;
+};
+
+} /* namespace libcamera */
+
+#endif /* __LIBCAMERA_INTERNAL_REQUEST_H__ */

include/libcamera/internal/tracepoints/request.tp

@@ -5,8 +5,9 @@
  * request.tp - Tracepoints for the request object
  */
 
+#include <libcamera/internal/request.h>
+
 #include <libcamera/framebuffer.h>
-#include <libcamera/request.h>
 
 TRACEPOINT_EVENT_CLASS(
 	libcamera,
@@ -62,7 +63,7 @@ TRACEPOINT_EVENT_INSTANCE(
 	request,
 	request_complete,
 	TP_ARGS(
-		libcamera::Request *, req
+		libcamera::Request::Private *, req
 	)
 )
 
@@ -71,7 +72,7 @@ TRACEPOINT_EVENT_INSTANCE(
 	request,
 	request_cancel,
 	TP_ARGS(
-		libcamera::Request *, req
+		libcamera::Request::Private *, req
 	)
 )
 
@@ -79,13 +80,13 @@ TRACEPOINT_EVENT(
 	libcamera,
 	request_complete_buffer,
 	TP_ARGS(
-		libcamera::Request *, req,
+		libcamera::Request::Private *, req,
 		libcamera::FrameBuffer *, buf
 	),
 	TP_FIELDS(
 		ctf_integer_hex(uintptr_t, request, reinterpret_cast<uintptr_t>(req))
-		ctf_integer(uint64_t, cookie, req->cookie())
+		ctf_integer(uint64_t, cookie, req->_o<libcamera::Request>()->cookie())
-		ctf_integer(int, status, req->status())
+		ctf_integer(int, status, req->_o<libcamera::Request>()->status())
 		ctf_integer_hex(uintptr_t, buffer, reinterpret_cast<uintptr_t>(buf))
 		ctf_enum(libcamera, buffer_status, uint32_t, buf_status, buf->metadata().status)
 	)

include/libcamera/request.h

@@ -25,8 +25,10 @@ class CameraControlValidator;
 class FrameBuffer;
 class Stream;
 
-class Request
+class Request : public Extensible
 {
+	LIBCAMERA_DECLARE_PRIVATE()
+
 public:
 	enum Status {
 		RequestPending,
@@ -52,34 +54,23 @@ public:
 	int addBuffer(const Stream *stream, FrameBuffer *buffer);
 	FrameBuffer *findBuffer(const Stream *stream) const;
 
-	uint32_t sequence() const { return sequence_; }
+	uint32_t sequence() const;
 	uint64_t cookie() const { return cookie_; }
 	Status status() const { return status_; }
 
-	bool hasPendingBuffers() const { return !pending_.empty(); }
+	bool hasPendingBuffers() const;
 
 	std::string toString() const;
 
 private:
 	LIBCAMERA_DISABLE_COPY(Request)
 
-	friend class PipelineHandler;
-
-	void complete();
-	void cancel();
-
-	bool completeBuffer(FrameBuffer *buffer);
-
-	Camera *camera_;
 	ControlList *controls_;
 	ControlList *metadata_;
 	BufferMap bufferMap_;
-	std::unordered_set<FrameBuffer *> pending_;
 
-	uint32_t sequence_;
 	const uint64_t cookie_;
 	Status status_;
-	bool cancelled_;
 };
 
 } /* namespace libcamera */

src/libcamera/pipeline_handler.cpp

@@ -19,6 +19,7 @@
 #include "libcamera/internal/camera.h"
 #include "libcamera/internal/device_enumerator.h"
 #include "libcamera/internal/media_device.h"
+#include "libcamera/internal/request.h"
 #include "libcamera/internal/tracepoints.h"
 
 /**
@@ -311,15 +312,15 @@ void PipelineHandler::queueRequest(Request *request)
 {
 	LIBCAMERA_TRACEPOINT(request_queue, request);
 
-	Camera *camera = request->camera_;
+	Camera *camera = request->_d()->camera();
 	Camera::Private *data = camera->_d();
 	data->queuedRequests_.push_back(request);
 
-	request->sequence_ = data->requestSequence_++;
+	request->_d()->sequence_ = data->requestSequence_++;
 
 	int ret = queueRequestDevice(camera, request);
 	if (ret) {
-		request->cancel();
+		request->_d()->cancel();
 		completeRequest(request);
 	}
 }
@@ -360,9 +361,9 @@ void PipelineHandler::queueRequest(Request *request)
  */
 bool PipelineHandler::completeBuffer(Request *request, FrameBuffer *buffer)
 {
-	Camera *camera = request->camera_;
+	Camera *camera = request->_d()->camera();
 	camera->bufferCompleted.emit(request, buffer);
-	return request->completeBuffer(buffer);
+	return request->_d()->completeBuffer(buffer);
 }
 
 /**
@@ -381,9 +382,9 @@ bool PipelineHandler::completeBuffer(Request *request, FrameBuffer *buffer)
  */
 void PipelineHandler::completeRequest(Request *request)
 {
-	Camera *camera = request->camera_;
+	Camera *camera = request->_d()->camera();
 
-	request->complete();
+	request->_d()->complete();
 
 	Camera::Private *data = camera->_d();
 

src/libcamera/request.cpp

@@ -5,7 +5,7 @@
  * request.cpp - Capture request handling
  */
 
-#include <libcamera/request.h>
+#include "libcamera/internal/request.h"
 
 #include <map>
 #include <sstream>
@@ -23,7 +23,7 @@
 #include "libcamera/internal/tracepoints.h"
 
 /**
- * \file request.h
+ * \file libcamera/request.h
  * \brief Describes a frame capture request to be processed by a camera
  */
 
@@ -31,6 +31,139 @@ namespace libcamera {
 
 LOG_DEFINE_CATEGORY(Request)
 
+/**
+ * \class Request::Private
+ * \brief Request private data
+ *
+ * The Request::Private class stores all private data associated with a
+ * request. It implements the d-pointer design pattern to hide core
+ * Request data from the public API, and exposes utility functions to
+ * internal users of the request (namely the PipelineHandler class and its
+ * subclasses).
+ */
+
+/**
+ * \brief Create a Request::Private
+ * \param camera The Camera that creates the request
+ */
+Request::Private::Private(Camera *camera)
+	: camera_(camera), cancelled_(false)
+{
+}
+
+Request::Private::~Private()
+{
+	doCancelRequest();
+}
+
+/**
+ * \fn Request::Private::camera()
+ * \brief Retrieve the camera this request has been queued to
+ * \return The Camera this request has been queued to, or nullptr if the
+ * request hasn't been queued
+ */
+
+/**
+ * \brief Check if a request has buffers yet to be completed
+ *
+ * \return True if the request has buffers pending for completion, false
+ * otherwise
+ */
+bool Request::Private::hasPendingBuffers() const
+{
+	return !pending_.empty();
+}
+
+/**
+ * \brief Complete a buffer for the request
+ * \param[in] buffer The buffer that has completed
+ *
+ * A request tracks the status of all buffers it contains through a set of
+ * pending buffers. This function removes the \a buffer from the set to mark it
+ * as complete. All buffers associate with the request shall be marked as
+ * complete by calling this function once and once only before reporting the
+ * request as complete with the complete() function.
+ *
+ * \return True if all buffers contained in the request have completed, false
+ * otherwise
+ */
+bool Request::Private::completeBuffer(FrameBuffer *buffer)
+{
+	LIBCAMERA_TRACEPOINT(request_complete_buffer, this, buffer);
+
+	int ret = pending_.erase(buffer);
+	ASSERT(ret == 1);
+
+	buffer->_d()->setRequest(nullptr);
+
+	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
+		cancelled_ = true;
+
+	return !hasPendingBuffers();
+}
+
+/**
+ * \brief Complete a queued request
+ *
+ * Mark the request as complete by updating its status to RequestComplete,
+ * unless buffers have been cancelled in which case the status is set to
+ * RequestCancelled.
+ */
+void Request::Private::complete()
+{
+	Request *request = _o<Request>();
+
+	ASSERT(request->status() == RequestPending);
+	ASSERT(!hasPendingBuffers());
+
+	request->status_ = cancelled_ ? RequestCancelled : RequestComplete;
+
+	LOG(Request, Debug) << request->toString();
+
+	LIBCAMERA_TRACEPOINT(request_complete, this);
+}
+
+void Request::Private::doCancelRequest()
+{
+	Request *request = _o<Request>();
+
+	for (FrameBuffer *buffer : pending_) {
+		buffer->cancel();
+		camera_->bufferCompleted.emit(request, buffer);
+	}
+
+	cancelled_ = true;
+	pending_.clear();
+}
+
+/**
+ * \brief Cancel a queued request
+ *
+ * Mark the request and its associated buffers as cancelled and complete it.
+ *
+ * Set each pending buffer in error state and emit the buffer completion signal
+ * before completing the Request.
+ */
+void Request::Private::cancel()
+{
+	LIBCAMERA_TRACEPOINT(request_cancel, this);
+
+	Request *request = _o<Request>();
+	ASSERT(request->status() == RequestPending);
+
+	doCancelRequest();
+}
+
+/**
+ * \copydoc Request::reuse()
+ */
+void Request::Private::reuse()
+{
+	sequence_ = 0;
+	cancelled_ = false;
+	pending_.clear();
+}
+
 /**
  * \enum Request::Status
  * Request completion status
@@ -75,8 +208,8 @@ LOG_DEFINE_CATEGORY(Request)
  * completely opaque to libcamera.
  */
 Request::Request(Camera *camera, uint64_t cookie)
-	: camera_(camera), sequence_(0), cookie_(cookie),
+	: Extensible(std::make_unique<Private>(camera)),
-	  status_(RequestPending), cancelled_(false)
+	  cookie_(cookie), status_(RequestPending)
 {
 	controls_ = new ControlList(controls::controls,
 				    camera->_d()->validator());
@@ -113,20 +246,19 @@ void Request::reuse(ReuseFlag flags)
 {
 	LIBCAMERA_TRACEPOINT(request_reuse, this);
 
-	pending_.clear();
+	_d()->reuse();
+
 	if (flags & ReuseBuffers) {
 		for (auto pair : bufferMap_) {
 			FrameBuffer *buffer = pair.second;
 			buffer->_d()->setRequest(this);
-			pending_.insert(buffer);
+			_d()->pending_.insert(buffer);
 		}
 	} else {
 		bufferMap_.clear();
 	}
 
-	sequence_ = 0;
 	status_ = RequestPending;
-	cancelled_ = false;
 
 	controls_->clear();
 	metadata_->clear();
@@ -188,7 +320,7 @@ int Request::addBuffer(const Stream *stream, FrameBuffer *buffer)
 	}
 
 	buffer->_d()->setRequest(this);
-	pending_.insert(buffer);
+	_d()->pending_.insert(buffer);
 	bufferMap_[stream] = buffer;
 
 	return 0;
@@ -227,7 +359,6 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const
  */
 
 /**
- * \fn Request::sequence()
  * \brief Retrieve the sequence number for the request
  *
  * When requests are queued, they are given a sequential number to track the
@@ -242,6 +373,10 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const
  *
  * \return The request sequence number
  */
+uint32_t Request::sequence() const
+{
+	return _d()->sequence_;
+}
 
 /**
  * \fn Request::cookie()
@@ -263,81 +398,14 @@ FrameBuffer *Request::findBuffer(const Stream *stream) const
  */
 
 /**
- * \fn Request::hasPendingBuffers()
  * \brief Check if a request has buffers yet to be completed
  *
  * \return True if the request has buffers pending for completion, false
  * otherwise
  */
-
+bool Request::hasPendingBuffers() const
-/**
- * \brief Complete a queued request
- *
- * Mark the request as complete by updating its status to RequestComplete,
- * unless buffers have been cancelled in which case the status is set to
- * RequestCancelled.
- */
-void Request::complete()
 {
-	ASSERT(status_ == RequestPending);
+	return !_d()->pending_.empty();
-	ASSERT(!hasPendingBuffers());
-
-	status_ = cancelled_ ? RequestCancelled : RequestComplete;
-
-	LOG(Request, Debug) << toString();
-
-	LIBCAMERA_TRACEPOINT(request_complete, this);
-}
-
-/**
- * \brief Cancel a queued request
- *
- * Mark the request and its associated buffers as cancelled and complete it.
- *
- * Set each pending buffer in error state and emit the buffer completion signal
- * before completing the Request.
- */
-void Request::cancel()
-{
-	LIBCAMERA_TRACEPOINT(request_cancel, this);
-
-	ASSERT(status_ == RequestPending);
-
-	for (FrameBuffer *buffer : pending_) {
-		buffer->cancel();
-		camera_->bufferCompleted.emit(this, buffer);
-	}
-
-	pending_.clear();
-	cancelled_ = true;
-}
-
-/**
- * \brief Complete a buffer for the request
- * \param[in] buffer The buffer that has completed
- *
- * A request tracks the status of all buffers it contains through a set of
- * pending buffers. This function removes the \a buffer from the set to mark it
- * as complete. All buffers associate with the request shall be marked as
- * complete by calling this function once and once only before reporting the
- * request as complete with the complete() function.
- *
- * \return True if all buffers contained in the request have completed, false
- * otherwise
- */
-bool Request::completeBuffer(FrameBuffer *buffer)
-{
-	LIBCAMERA_TRACEPOINT(request_complete_buffer, this, buffer);
-
-	int ret = pending_.erase(buffer);
-	ASSERT(ret == 1);
-
-	buffer->_d()->setRequest(nullptr);
-
-	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
-		cancelled_ = true;
-
-	return !hasPendingBuffers();
 }
 
 /**
@@ -356,8 +424,8 @@ std::string Request::toString() const
 	static const char *statuses = "PCX";
 
 	/* Example Output: Request(55:P:1/2:6523524) */
-	ss << "Request(" << sequence_ << ":" << statuses[status_] << ":"
+	ss << "Request(" << sequence() << ":" << statuses[status_] << ":"
-	   << pending_.size() << "/" << bufferMap_.size() << ":"
+	   << _d()->pending_.size() << "/" << bufferMap_.size() << ":"
 	   << cookie_ << ")";
 
 	return ss.str();