[libcamera-devel] libcamera: request: Make Request class Extensible

14525 diff mbox series

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..ca954f06f497
--- /dev/null
+++ b/include/libcamera/internal/request.h
@@ -0,0 +1,36 @@ 
+/* 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 <libcamera/request.h>
+
+namespace libcamera {
+
+class Camera;
+class CameraControlValidator;
+class FrameBuffer;
+
+class Request::Private : public Extensible::Private
+{
+	LIBCAMERA_DECLARE_PUBLIC(Request)
+
+public:
+	Private(Camera *camera);
+	~Private();
+
+	Camera *camera() const { return camera_; }
+
+private:
+	Camera *camera_;
+	CameraControlValidator *validator_;
+	bool cancelled_;
+};
+
+} /* namespace libcamera */
+
+#endif /* __LIBCAMERA_INTERNAL_REQUEST_H__ */
diff --git a/include/libcamera/request.h b/include/libcamera/request.h
index 2d361c9d97dc..e378355f18fc 100644
--- a/include/libcamera/request.h
+++ b/include/libcamera/request.h
@@ -21,12 +21,13 @@ 
 namespace libcamera {
 
 class Camera;
-class CameraControlValidator;
 class FrameBuffer;
 class Stream;
 
-class Request
+class Request : public Extensible
 {
+	LIBCAMERA_DECLARE_PRIVATE()
+
 public:
 	enum Status {
 		RequestPending,
@@ -70,8 +71,6 @@  private:
 
 	bool completeBuffer(FrameBuffer *buffer);
 
-	Camera *camera_;
-	CameraControlValidator *validator_;
 	ControlList *controls_;
 	ControlList *metadata_;
 	BufferMap bufferMap_;
@@ -80,7 +79,6 @@  private:
 	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..cada864687ff 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,7 +312,7 @@  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);
 
@@ -360,7 +361,7 @@  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);
 }
@@ -381,7 +382,7 @@  bool PipelineHandler::completeBuffer(Request *request, FrameBuffer *buffer)
  */
 void PipelineHandler::completeRequest(Request *request)
 {
-	Camera *camera = request->camera_;
+	Camera *camera = request->_d()->camera();
 
 	request->complete();
 
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index f95ce4db5eaa..7863269f3139 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -19,10 +19,11 @@ 
 
 #include "libcamera/internal/camera_controls.h"
 #include "libcamera/internal/framebuffer.h"
+#include "libcamera/internal/request.h"
 #include "libcamera/internal/tracepoints.h"
 
 /**
- * \file request.h
+ * \file libcamera/request.h
  * \brief Describes a frame capture request to be processed by a camera
  */
 
@@ -30,6 +31,39 @@  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).
+ */
+
+Request::Private::Private(Camera *camera)
+	: camera_(camera), cancelled_(false)
+{
+	/**
+	 * \todo Should the Camera expose a validator instance, to avoid
+	 * creating a new instance for each request?
+	 */
+	validator_ = new CameraControlValidator(camera);
+}
+
+Request::Private::~Private()
+{
+	delete validator_;
+}
+
+/**
+ * \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
+ */
+
 /**
  * \enum Request::Status
  * Request completion status
@@ -74,15 +108,10 @@  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)), sequence_(0),
+	  cookie_(cookie), status_(RequestPending)
 {
-	/**
-	 * \todo Should the Camera expose a validator instance, to avoid
-	 * creating a new instance for each request?
-	 */
-	validator_ = new CameraControlValidator(camera);
-	controls_ = new ControlList(controls::controls, validator_);
+	controls_ = new ControlList(controls::controls, _d()->validator_);
 
 	/**
 	 * \todo: Add a validator for metadata controls.
@@ -100,7 +129,6 @@  Request::~Request()
 
 	delete metadata_;
 	delete controls_;
-	delete validator_;
 }
 
 /**
@@ -130,7 +158,7 @@  void Request::reuse(ReuseFlag flags)
 
 	sequence_ = 0;
 	status_ = RequestPending;
-	cancelled_ = false;
+	_d()->cancelled_ = false;
 
 	controls_->clear();
 	metadata_->clear();
@@ -286,7 +314,7 @@  void Request::complete()
 	ASSERT(status_ == RequestPending);
 	ASSERT(!hasPendingBuffers());
 
-	status_ = cancelled_ ? RequestCancelled : RequestComplete;
+	status_ = _d()->cancelled_ ? RequestCancelled : RequestComplete;
 
 	LOG(Request, Debug) << toString();
 
@@ -303,17 +331,19 @@  void Request::complete()
  */
 void Request::cancel()
 {
+	Private *const d = _d();
+
 	LIBCAMERA_TRACEPOINT(request_cancel, this);
 
 	ASSERT(status_ == RequestPending);
 
 	for (FrameBuffer *buffer : pending_) {
 		buffer->cancel();
-		camera_->bufferCompleted.emit(this, buffer);
+		d->camera_->bufferCompleted.emit(this, buffer);
 	}
 
 	pending_.clear();
-	cancelled_ = true;
+	d->cancelled_ = true;
 }
 
 /**
@@ -339,7 +369,7 @@  bool Request::completeBuffer(FrameBuffer *buffer)
 	buffer->_d()->setRequest(nullptr);
 
 	if (buffer->metadata().status == FrameMetadata::FrameCancelled)
-		cancelled_ = true;
+		_d()->cancelled_ = true;
 
 	return !hasPendingBuffers();
 }