[libcamera-devel] [PATCH v2 01/12] libcamera: request: Make Request class Extensible

Jacopo Mondi jacopo at jmondi.org
Sat Nov 20 12:13:02 CET 2021


From: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

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 allow 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 at ideasonboard.com>
[Move all internal fields to Request::Private and remove friend  declaration]
Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
---
 include/libcamera/internal/meson.build        |   1 +
 include/libcamera/internal/request.h          |  51 ++++
 .../libcamera/internal/tracepoints/request.tp |   9 +-
 include/libcamera/request.h                   |  19 +-
 src/libcamera/pipeline_handler.cpp            |  15 +-
 src/libcamera/request.cpp                     | 256 ++++++++++++------
 6 files changed, 245 insertions(+), 106 deletions(-)
 create mode 100644 include/libcamera/internal/request.h

diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
index 665fd6de4ed3..cae65b0604ff 100644
--- a/include/libcamera/internal/meson.build
+++ b/include/libcamera/internal/meson.build
@@ -34,6 +34,7 @@ libcamera_internal_headers = files([
     'pipeline_handler.h',
     'process.h',
     'pub_key.h',
+    'request.h',
     'source_paths.h',
     'sysfs.h',
     'v4l2_device.h',
diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h
new file mode 100644
index 000000000000..59bddde3a090
--- /dev/null
+++ b/include/libcamera/internal/request.h
@@ -0,0 +1,51 @@
+/* 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;
+
+	uint64_t cookie() const;
+	Request::Status status() const;
+
+	bool completeBuffer(FrameBuffer *buffer);
+	void complete();
+	void cancel();
+	void reuse();
+
+	uint32_t sequence_ = 0;
+
+private:
+	void _cancel();
+
+	Camera *camera_;
+	bool cancelled_;
+
+	std::unordered_set<FrameBuffer *> pending_;
+};
+
+} /* namespace libcamera */
+
+#endif /* __LIBCAMERA_INTERNAL_REQUEST_H__ */
diff --git a/include/libcamera/internal/tracepoints/request.tp b/include/libcamera/internal/tracepoints/request.tp
index 37d8c46f4d96..37cd2f8864ce 100644
--- a/include/libcamera/internal/tracepoints/request.tp
+++ b/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,7 +80,7 @@ TRACEPOINT_EVENT(
 	libcamera,
 	request_complete_buffer,
 	TP_ARGS(
-		libcamera::Request *, req,
+		libcamera::Request::Private *, req,
 		libcamera::FrameBuffer *, buf
 	),
 	TP_FIELDS(
diff --git a/include/libcamera/request.h b/include/libcamera/request.h
index d16904e6b679..f0c5163d987e 100644
--- a/include/libcamera/request.h
+++ b/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 */
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index f69c4f03b80f..67fdf1d8db01 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/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();
 
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index 17fefab7ad0e..90c436648405 100644
--- a/src/libcamera/request.cpp
+++ b/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,165 @@ 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()
+{
+	_cancel();
+}
+
+/**
+ * \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();
+}
+
+/**
+ * \copydoc Request::cookie()
+ *
+ * Used by the tracing framework
+ */
+uint64_t Request::Private::cookie() const
+{
+	return _o<Request>()->cookie();
+}
+
+/**
+ * \copydoc Request::status()
+ *
+ * Used by the tracing framework
+ */
+Request::Status Request::Private::status() const
+{
+	return _o<Request>()->status();
+}
+
+/**
+ * \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::_cancel()
+{
+	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);
+
+	_cancel();
+}
+
+/**
+ * \copydoc Request::reuse()
+ */
+void Request::Private::reuse()
+{
+	sequence_ = 0;
+	cancelled_ = false;
+	pending_.clear();
+}
+/**
+ * \var Request::Private::sequence_
+ * \brief The request sequence number
+ *
+ * \copydoc Request::sequence()
+ */
+
 /**
  * \enum Request::Status
  * Request completion status
@@ -75,8 +234,8 @@ LOG_DEFINE_CATEGORY(Request)
  * completely opaque to libcamera.
  */
 Request::Request(Camera *camera, uint64_t cookie)
-	: camera_(camera), sequence_(0), cookie_(cookie),
-	  status_(RequestPending), cancelled_(false)
+	: Extensible(std::make_unique<Private>(camera)),
+	  cookie_(cookie), status_(RequestPending)
 {
 	controls_ = new ControlList(controls::controls,
 				    camera->_d()->validator());
@@ -113,20 +272,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 +346,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 +385,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 +399,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 +424,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
  */
-
-/**
- * \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);
-	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)
+bool Request::hasPendingBuffers() const
 {
-	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();
+	return !_d()->pending_.empty();
 }
 
 /**
@@ -356,8 +450,8 @@ std::string Request::toString() const
 	static const char *statuses = "PCX";
 
 	/* Example Output: Request(55:P:1/2:6523524) */
-	ss << "Request(" << sequence_ << ":" << statuses[status_] << ":"
-	   << pending_.size() << "/" << bufferMap_.size() << ":"
+	ss << "Request(" << sequence() << ":" << statuses[status_] << ":"
+	   << _d()->pending_.size() << "/" << bufferMap_.size() << ":"
 	   << cookie_ << ")";
 
 	return ss.str();
-- 
2.33.1



More information about the libcamera-devel mailing list