[libcamera-devel,v5,02/11] libcamera: request: Make Request class Extensible
diff mbox series

Message ID 20211210205239.354901-3-jacopo@jmondi.org
State Superseded
Headers show
Series
  • libcamera: Add fence
Related show

Commit Message

Jacopo Mondi Dec. 10, 2021, 8:52 p.m. UTC
From: Laurent Pinchart <laurent.pinchart@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 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        |   1 +
 include/libcamera/internal/request.h          |  49 ++++
 .../libcamera/internal/tracepoints/request.tp |  13 +-
 include/libcamera/request.h                   |  19 +-
 src/libcamera/pipeline_handler.cpp            |  15 +-
 src/libcamera/request.cpp                     | 230 ++++++++++++------
 6 files changed, 219 insertions(+), 108 deletions(-)
 create mode 100644 include/libcamera/internal/request.h

Patch
diff mbox series

diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
index a96bbb95544d..c9e055d4fe7b 100644
--- a/include/libcamera/internal/meson.build
+++ b/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',
diff --git a/include/libcamera/internal/request.h b/include/libcamera/internal/request.h
new file mode 100644
index 000000000000..1340ffa2a683
--- /dev/null
+++ b/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__ */
diff --git a/include/libcamera/internal/tracepoints/request.tp b/include/libcamera/internal/tracepoints/request.tp
index 37d8c46f4d96..f039ffd4f88d 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,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(int, status, req->status())
+		ctf_integer(uint64_t, cookie, req->_o<libcamera::Request>()->cookie())
+		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)
 	)
diff --git a/include/libcamera/request.h b/include/libcamera/request.h
index f434335b243a..8c78970d88ab 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..692202473360 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,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),
-	  status_(RequestPending), cancelled_(false)
+	: Extensible(std::make_unique<Private>(camera)),
+	  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
  */
-
-/**
- * \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 +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_] << ":"
-	   << pending_.size() << "/" << bufferMap_.size() << ":"
+	ss << "Request(" << sequence() << ":" << statuses[status_] << ":"
+	   << _d()->pending_.size() << "/" << bufferMap_.size() << ":"
 	   << cookie_ << ")";
 
 	return ss.str();