[v2,1/4] libcamera: Allow pipeline to provide a Private request
diff mbox series

Message ID 20240311123234.32925-2-jacopo.mondi@ideasonboard.com
State New
Headers show
Series
  • libcamera: Replace IPU3/RkISP1FrameInfo
Related show

Commit Message

Jacopo Mondi March 11, 2024, 12:32 p.m. UTC
In order to allow each pipeline handler to create a Request::Private
derived class to store ancillary data associated with a capture request,
modify the way a Request is created by the PipelineHandler base class.

Introduce a virtual PipelineHandler::createRequestDevice() that pipeline
handler implementation can optionally override to provide a custom
private data type to initialize the Request with and make the existing
Request() public constructor not usable by applications.

As we can't anymore create a Request without going through an active
camera, drop the queueRequest() checks from the Camera statemachine
test.

Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Tested-by: Daniel Scally <dan.scally@ideasonboard.com>
---
 include/libcamera/internal/pipeline_handler.h |  5 ++-
 include/libcamera/request.h                   |  3 +-
 src/libcamera/camera.cpp                      |  8 +---
 src/libcamera/pipeline_handler.cpp            | 38 ++++++++++++++++---
 src/libcamera/request.cpp                     | 15 +++++---
 test/camera/statemachine.cpp                  | 12 ------
 6 files changed, 50 insertions(+), 31 deletions(-)

Patch
diff mbox series

diff --git a/include/libcamera/internal/pipeline_handler.h b/include/libcamera/internal/pipeline_handler.h
index c96944f4ecc4..bbe74f22e5ae 100644
--- a/include/libcamera/internal/pipeline_handler.h
+++ b/include/libcamera/internal/pipeline_handler.h
@@ -21,6 +21,7 @@ 
 #include <libcamera/stream.h>
 
 #include "libcamera/internal/ipa_proxy.h"
+#include "libcamera/internal/request.h"
 
 namespace libcamera {
 
@@ -59,7 +60,7 @@  public:
 	void stop(Camera *camera);
 	bool hasPendingRequests(const Camera *camera) const;
 
-	void registerRequest(Request *request);
+	std::unique_ptr<Request> createRequest(Camera *camera, uint64_t cookie);
 	void queueRequest(Request *request);
 
 	bool completeBuffer(Request *request, FrameBuffer *buffer);
@@ -74,6 +75,8 @@  protected:
 	void registerCamera(std::shared_ptr<Camera> camera);
 	void hotplugMediaDevice(MediaDevice *media);
 
+	virtual std::unique_ptr<Request> createRequestDevice(Camera *camera,
+							     uint64_t cookie);
 	virtual int queueRequestDevice(Camera *camera, Request *request) = 0;
 	virtual void stopDevice(Camera *camera) = 0;
 
diff --git a/include/libcamera/request.h b/include/libcamera/request.h
index dffde1536cad..e16e61a93873 100644
--- a/include/libcamera/request.h
+++ b/include/libcamera/request.h
@@ -45,9 +45,10 @@  public:
 
 	using BufferMap = std::map<const Stream *, FrameBuffer *>;
 
-	Request(Camera *camera, uint64_t cookie = 0);
+	Request(std::unique_ptr<Private> d, uint64_t cookie = 0);
 	~Request();
 
+	static std::unique_ptr<Request> create(std::unique_ptr<Private> d, uint64_t);
 	void reuse(ReuseFlag flags = Default);
 
 	ControlList &controls() { return *controls_; }
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index a71dc933b911..b7053576fe42 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -1236,12 +1236,8 @@  std::unique_ptr<Request> Camera::createRequest(uint64_t cookie)
 	if (ret < 0)
 		return nullptr;
 
-	std::unique_ptr<Request> request = std::make_unique<Request>(this, cookie);
-
-	/* Associate the request with the pipeline handler. */
-	d->pipe_->registerRequest(request.get());
-
-	return request;
+	/* Create a Request from the pipeline handler. */
+	return d->pipe_->createRequest(this, cookie);
 }
 
 /**
diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp
index 29e0c98a6db5..f2a8cdac0408 100644
--- a/src/libcamera/pipeline_handler.cpp
+++ b/src/libcamera/pipeline_handler.cpp
@@ -371,20 +371,25 @@  bool PipelineHandler::hasPendingRequests(const Camera *camera) const
 }
 
 /**
- * \fn PipelineHandler::registerRequest()
- * \brief Register a request for use by the pipeline handler
- * \param[in] request The request to register
+ * \fn PipelineHandler::createRequest()
+ * \brief Create a request and register it for use by the pipeline handler
+ * \param[in] camera The camera the Request belongs to
+ * \param[in] cookie The Request unique identifier
  *
- * This function is called when the request is created, and allows the pipeline
- * handler to perform any one-time initialization it requries for the request.
+ * This function is called to create a Request by calling createRequestDevice()
+ * which can be optionally provided by the PipelineHandler derived classes.
  */
-void PipelineHandler::registerRequest(Request *request)
+std::unique_ptr<Request> PipelineHandler::createRequest(Camera *camera, uint64_t cookie)
 {
+	std::unique_ptr<Request> request = createRequestDevice(camera, cookie);
+
 	/*
 	 * Connect the request prepared signal to notify the pipeline handler
 	 * when a request is ready to be processed.
 	 */
 	request->_d()->prepared.connect(this, &PipelineHandler::doQueueRequests);
+
+	return request;
 }
 
 /**
@@ -462,6 +467,27 @@  void PipelineHandler::doQueueRequests()
 	}
 }
 
+/**
+ * \brief Create a Request from the pipeline handler
+ * \param[in] camera The camera the Request belongs to
+ * \param[in] cookie The Request unique identifier
+ *
+ * A virtual function that PipelineHandler derived classes are free to override
+ * in order to initialize a Request with a custom Request::Private derived
+ * class.
+ *
+ * This is the base class implementation that use Request::Private to
+ * initialize the Request.
+ *
+ * \return A unique pointer to a newly created Request
+ */
+std::unique_ptr<Request>
+PipelineHandler::createRequestDevice(Camera *camera, uint64_t cookie)
+{
+	auto d = std::make_unique<Request::Private>(camera);
+	return Request::create(std::move(d), cookie);
+}
+
 /**
  * \fn PipelineHandler::queueRequestDevice()
  * \brief Queue a request to the device
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index 949c556fa437..d1051ad3d25e 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -336,7 +336,7 @@  void Request::Private::timeout()
 
 /**
  * \brief Create a capture request for a camera
- * \param[in] camera The camera that creates the request
+ * \param[in] d The request private data
  * \param[in] cookie Opaque cookie for application use
  *
  * The \a cookie is stored in the request and is accessible through the
@@ -344,12 +344,17 @@  void Request::Private::timeout()
  * the request to an external resource in the request completion handler, and is
  * completely opaque to libcamera.
  */
-Request::Request(Camera *camera, uint64_t cookie)
-	: Extensible(std::make_unique<Private>(camera)),
-	  cookie_(cookie), status_(RequestPending)
+std::unique_ptr<Request> Request::create(std::unique_ptr<Private> d,
+					 uint64_t cookie)
+{
+	return std::make_unique<Request>(std::move(d), cookie);
+}
+
+Request::Request(std::unique_ptr<Request::Private> d, uint64_t cookie)
+	: Extensible(std::move(d)), cookie_(cookie), status_(RequestPending)
 {
 	controls_ = new ControlList(controls::controls,
-				    camera->_d()->validator());
+				    _d()->camera()->_d()->validator());
 
 	/**
 	 * \todo Add a validator for metadata controls.
diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp
index 9c2b0c6a7d99..5714061f88b5 100644
--- a/test/camera/statemachine.cpp
+++ b/test/camera/statemachine.cpp
@@ -38,10 +38,6 @@  protected:
 		if (camera_->start() != -EACCES)
 			return TestFail;
 
-		Request request(camera_.get());
-		if (camera_->queueRequest(&request) != -EACCES)
-			return TestFail;
-
 		/* Test operations which should pass. */
 		if (camera_->release())
 			return TestFail;
@@ -68,10 +64,6 @@  protected:
 		if (camera_->start() != -EACCES)
 			return TestFail;
 
-		Request request(camera_.get());
-		if (camera_->queueRequest(&request) != -EACCES)
-			return TestFail;
-
 		/* Test operations which should pass. */
 		if (camera_->stop())
 			return TestFail;
@@ -95,10 +87,6 @@  protected:
 		if (camera_->acquire() != -EBUSY)
 			return TestFail;
 
-		Request request1(camera_.get());
-		if (camera_->queueRequest(&request1) != -EACCES)
-			return TestFail;
-
 		/* Test operations which should pass. */
 		std::unique_ptr<Request> request2 = camera_->createRequest();
 		if (!request2)