libcamera, android, cam, gstreamer, qcam, v4l2: Reuse Request

Allow reuse of the Request object by implementing reuse(). This means
the applications now have the responsibility of freeing the Request
objects, so make all libcamera users (cam, qcam, v4l2-compat, gstreamer,
android) do so.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

10 files changed, 138 insertions, 90 deletions

diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 0a94c1ae..c29fcb43 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -1375,8 +1375,12 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 		new Camera3RequestDescriptor(camera3Request->frame_number,
 					     camera3Request->num_output_buffers);
 
-	Request *request =
+	std::unique_ptr<Request> request =
 		camera_->createRequest(reinterpret_cast<uint64_t>(descriptor));
+	if (!request) {
+		LOG(HAL, Error) << "Failed to create request";
+		return -ENOMEM;
+	}
 
 	LOG(HAL, Debug) << "Queueing Request to libcamera with "
 			<< descriptor->numBuffers << " HAL streams";
@@ -1440,7 +1444,6 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 
 		if (!buffer) {
 			LOG(HAL, Error) << "Failed to create buffer";
-			delete request;
 			delete descriptor;
 			return -ENOMEM;
 		}
@@ -1448,14 +1451,16 @@ int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reques
 		request->addBuffer(cameraStream->stream(), buffer);
 	}
 
-	int ret = camera_->queueRequest(request);
+	int ret = camera_->queueRequest(request.get());
 	if (ret) {
 		LOG(HAL, Error) << "Failed to queue request";
-		delete request;
 		delete descriptor;
 		return ret;
 	}
 
+	/* The request will be deleted in the completion handler. */
+	request.release();
+
 	return 0;
 }
 
@@ -1559,6 +1564,7 @@ void CameraDevice::requestComplete(Request *request)
 	callbacks_->process_capture_result(callbacks_, &captureResult);
 
 	delete descriptor;
+	delete request;
 }
 
 std::string CameraDevice::logPrefix() const
diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
index 5510c009..8c8faa4b 100644
--- a/src/cam/capture.cpp
+++ b/src/cam/capture.cpp
@@ -65,6 +65,8 @@ int Capture::run(const OptionsParser::Options &options)
 		writer_ = nullptr;
 	}
 
+	requests_.clear();
+
 	delete allocator;
 
 	return ret;
@@ -92,9 +94,8 @@ int Capture::capture(FrameBufferAllocator *allocator)
 	 * example pushing a button. For now run all streams all the time.
 	 */
 
-	std::vector<Request *> requests;
 	for (unsigned int i = 0; i < nbuffers; i++) {
-		Request *request = camera_->createRequest();
+		std::unique_ptr<Request> request = camera_->createRequest();
 		if (!request) {
 			std::cerr << "Can't create request" << std::endl;
 			return -ENOMEM;
@@ -117,7 +118,7 @@ int Capture::capture(FrameBufferAllocator *allocator)
 				writer_->mapBuffer(buffer.get());
 		}
 
-		requests.push_back(request);
+		requests_.push_back(std::move(request));
 	}
 
 	ret = camera_->start();
@@ -126,8 +127,8 @@ int Capture::capture(FrameBufferAllocator *allocator)
 		return ret;
 	}
 
-	for (Request *request : requests) {
-		ret = camera_->queueRequest(request);
+	for (std::unique_ptr<Request> &request : requests_) {
+		ret = camera_->queueRequest(request.get());
 		if (ret < 0) {
 			std::cerr << "Can't queue request" << std::endl;
 			camera_->stop();
@@ -202,22 +203,6 @@ void Capture::requestComplete(Request *request)
 		return;
 	}
 
-	/*
-	 * Create a new request and populate it with one buffer for each
-	 * stream.
-	 */
-	request = camera_->createRequest();
-	if (!request) {
-		std::cerr << "Can't create request" << std::endl;
-		return;
-	}
-
-	for (auto it = buffers.begin(); it != buffers.end(); ++it) {
-		const Stream *stream = it->first;
-		FrameBuffer *buffer = it->second;
-
-		request->addBuffer(stream, buffer);
-	}
-
+	request->reuse(Request::ReuseBuffers);
 	camera_->queueRequest(request);
 }
diff --git a/src/cam/capture.h b/src/cam/capture.h
index 0aebdac9..45e5e8a9 100644
--- a/src/cam/capture.h
+++ b/src/cam/capture.h
@@ -9,6 +9,7 @@
 
 #include <memory>
 #include <stdint.h>
+#include <vector>
 
 #include <libcamera/buffer.h>
 #include <libcamera/camera.h>
@@ -43,6 +44,8 @@ private:
 	EventLoop *loop_;
 	unsigned int captureCount_;
 	unsigned int captureLimit_;
+
+	std::vector<std::unique_ptr<libcamera::Request>> requests_;
 };
 
 #endif /* __CAM_CAPTURE_H__ */
diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp
index 1bfc2e2f..5001083a 100644
--- a/src/gstreamer/gstlibcamerasrc.cpp
+++ b/src/gstreamer/gstlibcamerasrc.cpp
@@ -74,6 +74,8 @@ RequestWrap::~RequestWrap()
 		if (item.second)
 			gst_buffer_unref(item.second);
 	}
+
+	delete request_;
 }
 
 void RequestWrap::attachBuffer(GstBuffer *buffer)
@@ -266,8 +268,8 @@ gst_libcamera_src_task_run(gpointer user_data)
 	GstLibcameraSrc *self = GST_LIBCAMERA_SRC(user_data);
 	GstLibcameraSrcState *state = self->state;
 
-	Request *request = state->cam_->createRequest();
-	auto wrap = std::make_unique<RequestWrap>(request);
+	std::unique_ptr<Request> request = state->cam_->createRequest();
+	auto wrap = std::make_unique<RequestWrap>(request.get());
 	for (GstPad *srcpad : state->srcpads_) {
 		GstLibcameraPool *pool = gst_libcamera_pad_get_pool(srcpad);
 		GstBuffer *buffer;
@@ -280,8 +282,7 @@ gst_libcamera_src_task_run(gpointer user_data)
 			 * RequestWrap does not take ownership, and we won't be
 			 * queueing this one due to lack of buffers.
 			 */
-			delete request;
-			request = nullptr;
+			request.reset();
 			break;
 		}
 
@@ -291,8 +292,11 @@ gst_libcamera_src_task_run(gpointer user_data)
 	if (request) {
 		GLibLocker lock(GST_OBJECT(self));
 		GST_TRACE_OBJECT(self, "Requesting buffers");
-		state->cam_->queueRequest(request);
+		state->cam_->queueRequest(request.get());
 		state->requests_.push(std::move(wrap));
+
+		/* The request will be deleted in the completion handler. */
+		request.release();
 	}
 
 	GstFlowReturn ret = GST_FLOW_OK;
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index fb76077f..9590ab72 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -847,21 +847,22 @@ int Camera::configure(CameraConfiguration *config)
  * handler, and is completely opaque to libcamera.
  *
  * The ownership of the returned request is passed to the caller, which is
- * responsible for either queueing the request or deleting it.
+ * responsible for deleting it. The request may be deleted in the completion
+ * handler, or reused after resetting its state with Request::reuse().
  *
  * \context This function is \threadsafe. It may only be called when the camera
  * is in the Configured or Running state as defined in \ref camera_operation.
  *
  * \return A pointer to the newly created request, or nullptr on error
  */
-Request *Camera::createRequest(uint64_t cookie)
+std::unique_ptr<Request> Camera::createRequest(uint64_t cookie)
 {
 	int ret = p_->isAccessAllowed(Private::CameraConfigured,
 				      Private::CameraRunning);
 	if (ret < 0)
 		return nullptr;
 
-	return new Request(this, cookie);
+	return std::make_unique<Request>(this, cookie);
 }
 
 /**
@@ -877,9 +878,6 @@ Request *Camera::createRequest(uint64_t cookie)
  * Once the request has been queued, the camera will notify its completion
  * through the \ref requestCompleted signal.
  *
- * Ownership of the request is transferred to the camera. It will be deleted
- * automatically after it completes.
- *
  * \context This function is \threadsafe. It may only be called when the camera
  * is in the Running state as defined in \ref camera_operation.
  *
@@ -988,13 +986,11 @@ int Camera::stop()
  * \param[in] request The request that has completed
  *
  * This function is called by the pipeline handler to notify the camera that
- * the request has completed. It emits the requestCompleted signal and deletes
- * the request.
+ * the request has completed. It emits the requestCompleted signal.
  */
 void Camera::requestComplete(Request *request)
 {
 	requestCompleted.emit(request);
-	delete request;
 }
 
 } /* namespace libcamera */
diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
index 60b30692..ae8b1660 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -38,6 +38,15 @@ LOG_DEFINE_CATEGORY(Request)
  */
 
 /**
+ * \enum Request::ReuseFlag
+ * Flags to control the behavior of Request::reuse()
+ * \var Request::Default
+ * Don't reuse buffers
+ * \var Request::ReuseBuffers
+ * Reuse the buffers that were previously added by addBuffer()
+ */
+
+/**
  * \typedef Request::BufferMap
  * \brief A map of Stream to FrameBuffer pointers
  */
@@ -86,6 +95,35 @@ Request::~Request()
 }
 
 /**
+ * \brief Reset the request for reuse
+ * \param[in] flags Indicate whether or not to reuse the buffers
+ *
+ * Reset the status and controls associated with the request, to allow it to
+ * be reused and requeued without destruction. This function shall be called
+ * prior to queueing the request to the camera, in lieu of constructing a new
+ * request. The application can reuse the buffers that were previously added
+ * to the request via addBuffer() by setting \a flags to ReuseBuffers.
+ */
+void Request::reuse(ReuseFlag flags)
+{
+	pending_.clear();
+	if (flags & ReuseBuffers) {
+		for (auto pair : bufferMap_) {
+			pair.second->request_ = this;
+			pending_.insert(pair.second);
+		}
+	} else {
+		bufferMap_.clear();
+	}
+
+	status_ = RequestPending;
+	cancelled_ = false;
+
+	controls_->clear();
+	metadata_->clear();
+}
+
+/**
  * \fn Request::controls()
  * \brief Retrieve the request's ControlList
  *
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index ecb9dd66..0cbdab9a 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -367,7 +367,6 @@ void MainWindow::toggleCapture(bool start)
 int MainWindow::startCapture()
 {
 	StreamRoles roles = StreamKeyValueParser::roles(options_[OptStream]);
-	std::vector<Request *> requests;
 	int ret;
 
 	/* Verify roles are supported. */
@@ -486,7 +485,7 @@ int MainWindow::startCapture()
 	while (!freeBuffers_[vfStream_].isEmpty()) {
 		FrameBuffer *buffer = freeBuffers_[vfStream_].dequeue();
 
-		Request *request = camera_->createRequest();
+		std::unique_ptr<Request> request = camera_->createRequest();
 		if (!request) {
 			qWarning() << "Can't create request";
 			ret = -ENOMEM;
@@ -499,7 +498,7 @@ int MainWindow::startCapture()
 			goto error;
 		}
 
-		requests.push_back(request);
+		requests_.push_back(std::move(request));
 	}
 
 	/* Start the title timer and the camera. */
@@ -518,8 +517,8 @@ int MainWindow::startCapture()
 	camera_->requestCompleted.connect(this, &MainWindow::requestComplete);
 
 	/* Queue all requests. */
-	for (Request *request : requests) {
-		ret = camera_->queueRequest(request);
+	for (std::unique_ptr<Request> &request : requests_) {
+		ret = camera_->queueRequest(request.get());
 		if (ret < 0) {
 			qWarning() << "Can't queue request";
 			goto error_disconnect;
@@ -535,8 +534,7 @@ error_disconnect:
 	camera_->stop();
 
 error:
-	for (Request *request : requests)
-		delete request;
+	requests_.clear();
 
 	for (auto &iter : mappedBuffers_) {
 		const MappedBuffer &buffer = iter.second;
@@ -580,6 +578,8 @@ void MainWindow::stopCapture()
 	}
 	mappedBuffers_.clear();
 
+	requests_.clear();
+
 	delete allocator_;
 
 	isCapturing_ = false;
@@ -701,7 +701,7 @@ void MainWindow::requestComplete(Request *request)
 	 */
 	{
 		QMutexLocker locker(&mutex_);
-		doneQueue_.enqueue({ request->buffers(), request->metadata() });
+		doneQueue_.enqueue(request);
 	}
 
 	QCoreApplication::postEvent(this, new CaptureEvent);
@@ -714,8 +714,7 @@ void MainWindow::processCapture()
 	 * if stopCapture() has been called while a CaptureEvent was posted but
 	 * not processed yet. Return immediately in that case.
 	 */
-	CaptureRequest request;
-
+	Request *request;
 	{
 		QMutexLocker locker(&mutex_);
 		if (doneQueue_.isEmpty())
@@ -725,11 +724,15 @@ void MainWindow::processCapture()
 	}
 
 	/* Process buffers. */
-	if (request.buffers_.count(vfStream_))
-		processViewfinder(request.buffers_[vfStream_]);
+	if (request->buffers().count(vfStream_))
+		processViewfinder(request->buffers().at(vfStream_));
 
-	if (request.buffers_.count(rawStream_))
-		processRaw(request.buffers_[rawStream_], request.metadata_);
+	if (request->buffers().count(rawStream_))
+		processRaw(request->buffers().at(rawStream_), request->metadata());
+
+	request->reuse();
+	QMutexLocker locker(&mutex_);
+	freeQueue_.enqueue(request);
 }
 
 void MainWindow::processViewfinder(FrameBuffer *buffer)
@@ -754,10 +757,13 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
 
 void MainWindow::queueRequest(FrameBuffer *buffer)
 {
-	Request *request = camera_->createRequest();
-	if (!request) {
-		qWarning() << "Can't create request";
-		return;
+	Request *request;
+	{
+		QMutexLocker locker(&mutex_);
+		if (freeQueue_.isEmpty())
+			return;
+
+		request = freeQueue_.dequeue();
 	}
 
 	request->addBuffer(vfStream_, buffer);
diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
index 5c61a4df..64bcfebc 100644
--- a/src/qcam/main_window.h
+++ b/src/qcam/main_window.h
@@ -8,6 +8,7 @@
 #define __QCAM_MAIN_WINDOW_H__
 
 #include <memory>
+#include <vector>
 
 #include <QElapsedTimer>
 #include <QIcon>
@@ -22,6 +23,7 @@
 #include <libcamera/camera_manager.h>
 #include <libcamera/controls.h>
 #include <libcamera/framebuffer_allocator.h>
+#include <libcamera/request.h>
 #include <libcamera/stream.h>
 
 #include "../cam/stream_options.h"
@@ -41,23 +43,6 @@ enum {
 	OptStream = 's',
 };
 
-class CaptureRequest
-{
-public:
-	CaptureRequest()
-	{
-	}
-
-	CaptureRequest(const Request::BufferMap &buffers,
-		       const ControlList &metadata)
-		: buffers_(buffers), metadata_(metadata)
-	{
-	}
-
-	Request::BufferMap buffers_;
-	ControlList metadata_;
-};
-
 class MainWindow : public QMainWindow
 {
 	Q_OBJECT
@@ -128,13 +113,16 @@ private:
 	Stream *vfStream_;
 	Stream *rawStream_;
 	std::map<const Stream *, QQueue<FrameBuffer *>> freeBuffers_;
-	QQueue<CaptureRequest> doneQueue_;
-	QMutex mutex_; /* Protects freeBuffers_ and doneQueue_ */
+	QQueue<Request *> doneQueue_;
+	QQueue<Request *> freeQueue_;
+	QMutex mutex_; /* Protects freeBuffers_, doneQueue_, and freeQueue_ */
 
 	uint64_t lastBufferTime_;
 	QElapsedTimer frameRateInterval_;
 	uint32_t previousFrames_;
 	uint32_t framesCaptured_;
+
+	std::vector<std::unique_ptr<Request>> requests_;
 };
 
 #endif /* __QCAM_MAIN_WINDOW__ */
diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp
index 3565f369..35d3beda 100644
--- a/src/v4l2/v4l2_camera.cpp
+++ b/src/v4l2/v4l2_camera.cpp
@@ -49,6 +49,8 @@ int V4L2Camera::open(StreamConfiguration *streamConfig)
 
 void V4L2Camera::close()
 {
+	requestPool_.clear();
+
 	delete bufferAllocator_;
 	bufferAllocator_ = nullptr;
 
@@ -96,6 +98,7 @@ void V4L2Camera::requestComplete(Request *request)
 	if (ret != sizeof(data))
 		LOG(V4L2Compat, Error) << "Failed to signal eventfd POLLIN";
 
+	request->reuse();
 	{
 		MutexLocker locker(bufferMutex_);
 		bufferAvailableCount_++;
@@ -154,16 +157,30 @@ int V4L2Camera::validateConfiguration(const PixelFormat &pixelFormat,
 	return 0;
 }
 
-int V4L2Camera::allocBuffers([[maybe_unused]] unsigned int count)
+int V4L2Camera::allocBuffers(unsigned int count)
 {
 	Stream *stream = config_->at(0).stream();
 
-	return bufferAllocator_->allocate(stream);
+	int ret = bufferAllocator_->allocate(stream);
+	if (ret < 0)
+		return ret;
+
+	for (unsigned int i = 0; i < count; i++) {
+		std::unique_ptr<Request> request = camera_->createRequest(i);
+		if (!request) {
+			requestPool_.clear();
+			return -ENOMEM;
+		}
+		requestPool_.push_back(std::move(request));
+	}
+
+	return ret;
 }
 
 void V4L2Camera::freeBuffers()
 {
 	pendingRequests_.clear();
+	requestPool_.clear();
 
 	Stream *stream = config_->at(0).stream();
 	bufferAllocator_->free(stream);
@@ -192,9 +209,9 @@ int V4L2Camera::streamOn()
 
 	isRunning_ = true;
 
-	for (std::unique_ptr<Request> &req : pendingRequests_) {
+	for (Request *req : pendingRequests_) {
 		/* \todo What should we do if this returns -EINVAL? */
-		ret = camera_->queueRequest(req.release());
+		ret = camera_->queueRequest(req);
 		if (ret < 0)
 			return ret == -EACCES ? -EBUSY : ret;
 	}
@@ -206,8 +223,12 @@ int V4L2Camera::streamOn()
 
 int V4L2Camera::streamOff()
 {
-	if (!isRunning_)
+	if (!isRunning_) {
+		for (std::unique_ptr<Request> &req : requestPool_)
+			req->reuse();
+
 		return 0;
+	}
 
 	pendingRequests_.clear();
 
@@ -226,12 +247,11 @@ int V4L2Camera::streamOff()
 
 int V4L2Camera::qbuf(unsigned int index)
 {
-	std::unique_ptr<Request> request =
-		std::unique_ptr<Request>(camera_->createRequest(index));
-	if (!request) {
-		LOG(V4L2Compat, Error) << "Can't create request";
-		return -ENOMEM;
+	if (index >= requestPool_.size()) {
+		LOG(V4L2Compat, Error) << "Invalid index";
+		return -EINVAL;
 	}
+	Request *request = requestPool_[index].get();
 
 	Stream *stream = config_->at(0).stream();
 	FrameBuffer *buffer = bufferAllocator_->buffers(stream)[index].get();
@@ -242,11 +262,11 @@ int V4L2Camera::qbuf(unsigned int index)
 	}
 
 	if (!isRunning_) {
-		pendingRequests_.push_back(std::move(request));
+		pendingRequests_.push_back(request);
 		return 0;
 	}
 
-	ret = camera_->queueRequest(request.release());
+	ret = camera_->queueRequest(request);
 	if (ret < 0) {
 		LOG(V4L2Compat, Error) << "Can't queue request";
 		return ret == -EACCES ? -EBUSY : ret;
diff --git a/src/v4l2/v4l2_camera.h b/src/v4l2/v4l2_camera.h
index 1fc5ebef..a6c35a2e 100644
--- a/src/v4l2/v4l2_camera.h
+++ b/src/v4l2/v4l2_camera.h
@@ -76,7 +76,9 @@ private:
 	std::mutex bufferLock_;
 	FrameBufferAllocator *bufferAllocator_;
 
-	std::deque<std::unique_ptr<Request>> pendingRequests_;
+	std::vector<std::unique_ptr<Request>> requestPool_;
+
+	std::deque<Request *> pendingRequests_;
 	std::deque<std::unique_ptr<Buffer>> completedBuffers_;
 
 	int efd_;