[libcamera-devel] [RFC PATCH] libcamera, cam, qcam: Reuse Request

Paul Elder paul.elder at ideasonboard.com
Wed Sep 23 14:14:48 CEST 2020


Allow reuse of the Request object by implementing a reset() function.
This means that the applications now have the responsibility of freeing
the the Request objects, so make cam and qcam do so.

Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>

---
In this RFC, I've only made qcam and cam reuse the Request object. Just
gauging the direction, and then I'll put it in for andoird and gstreamer
and v4l2-compat as well.
---
 include/libcamera/request.h |  2 ++
 src/cam/capture.cpp         | 20 ++++++++++----------
 src/cam/capture.h           |  3 +++
 src/libcamera/camera.cpp    |  4 +---
 src/libcamera/request.cpp   |  9 +++++++++
 src/qcam/main_window.cpp    | 26 ++++++++++++--------------
 src/qcam/main_window.h      | 14 ++++++++++----
 src/qcam/viewfinder.h       |  4 +++-
 src/qcam/viewfinder_gl.cpp  |  9 +++++----
 src/qcam/viewfinder_gl.h    |  5 +++--
 src/qcam/viewfinder_qt.cpp  | 10 ++++++----
 src/qcam/viewfinder_qt.h    |  8 ++++++--
 12 files changed, 70 insertions(+), 44 deletions(-)

diff --git a/include/libcamera/request.h b/include/libcamera/request.h
index 5976ac50..9f615a24 100644
--- a/include/libcamera/request.h
+++ b/include/libcamera/request.h
@@ -38,6 +38,8 @@ public:
 	Request &operator=(const Request &) = delete;
 	~Request();
 
+	void reset();
+
 	ControlList &controls() { return *controls_; }
 	ControlList &metadata() { return *metadata_; }
 	const BufferMap &buffers() const { return bufferMap_; }
diff --git a/src/cam/capture.cpp b/src/cam/capture.cpp
index 5510c009..fe9d591a 100644
--- a/src/cam/capture.cpp
+++ b/src/cam/capture.cpp
@@ -65,6 +65,9 @@ int Capture::run(const OptionsParser::Options &options)
 		writer_ = nullptr;
 	}
 
+	for (Request *req : requests_)
+		delete req;
+
 	delete allocator;
 
 	return ret;
@@ -92,7 +95,6 @@ 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();
 		if (!request) {
@@ -117,7 +119,7 @@ int Capture::capture(FrameBufferAllocator *allocator)
 				writer_->mapBuffer(buffer.get());
 		}
 
-		requests.push_back(request);
+		requests_.push_back(request);
 	}
 
 	ret = camera_->start();
@@ -126,7 +128,7 @@ int Capture::capture(FrameBufferAllocator *allocator)
 		return ret;
 	}
 
-	for (Request *request : requests) {
+	for (Request *request : requests_) {
 		ret = camera_->queueRequest(request);
 		if (ret < 0) {
 			std::cerr << "Can't queue request" << std::endl;
@@ -153,10 +155,12 @@ int Capture::capture(FrameBufferAllocator *allocator)
 
 void Capture::requestComplete(Request *request)
 {
-	if (request->status() == Request::RequestCancelled)
+	if (request->status() == Request::RequestCancelled) {
+		request->reset();
 		return;
+	}
 
-	const Request::BufferMap &buffers = request->buffers();
+	const Request::BufferMap buffers = request->buffers();
 
 	/*
 	 * Compute the frame rate. The timestamp is arbitrarily retrieved from
@@ -206,11 +210,7 @@ void Capture::requestComplete(Request *request)
 	 * 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;
-	}
+	request->reset();
 
 	for (auto it = buffers.begin(); it != buffers.end(); ++it) {
 		const Stream *stream = it->first;
diff --git a/src/cam/capture.h b/src/cam/capture.h
index 0aebdac9..62cf9bf1 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<libcamera::Request *> requests_;
 };
 
 #endif /* __CAM_CAPTURE_H__ */
diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
index ae16a64a..4cc68bce 100644
--- a/src/libcamera/camera.cpp
+++ b/src/libcamera/camera.cpp
@@ -974,13 +974,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..606866a6 100644
--- a/src/libcamera/request.cpp
+++ b/src/libcamera/request.cpp
@@ -85,6 +85,15 @@ Request::~Request()
 	delete validator_;
 }
 
+void Request::reset()
+{
+	bufferMap_.clear();
+	pending_.clear();
+
+	status_ = RequestPending;
+	cancelled_ = false;
+}
+
 /**
  * \fn Request::controls()
  * \brief Retrieve the request's ControlList
diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index 985743f3..1531b44f 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. */
@@ -499,7 +498,7 @@ int MainWindow::startCapture()
 			goto error;
 		}
 
-		requests.push_back(request);
+		requests_.push_back(request);
 	}
 
 	/* Start the title timer and the camera. */
@@ -518,7 +517,7 @@ int MainWindow::startCapture()
 	camera_->requestCompleted.connect(this, &MainWindow::requestComplete);
 
 	/* Queue all requests. */
-	for (Request *request : requests) {
+	for (Request *request : requests_) {
 		ret = camera_->queueRequest(request);
 		if (ret < 0) {
 			qWarning() << "Can't queue request";
@@ -535,7 +534,7 @@ error_disconnect:
 	camera_->stop();
 
 error:
-	for (Request *request : requests)
+	for (Request *request : requests_)
 		delete request;
 
 	for (auto &iter : mappedBuffers_) {
@@ -580,6 +579,9 @@ void MainWindow::stopCapture()
 	}
 	mappedBuffers_.clear();
 
+	for (Request *req : requests_)
+		delete req;
+
 	delete allocator_;
 
 	isCapturing_ = false;
@@ -701,7 +703,7 @@ void MainWindow::requestComplete(Request *request)
 	 */
 	{
 		QMutexLocker locker(&mutex_);
-		doneQueue_.enqueue({ request->buffers(), request->metadata() });
+		doneQueue_.enqueue({ request->buffers(), request->metadata(), request });
 	}
 
 	QCoreApplication::postEvent(this, new CaptureEvent);
@@ -726,13 +728,13 @@ void MainWindow::processCapture()
 
 	/* Process buffers. */
 	if (request.buffers_.count(vfStream_))
-		processViewfinder(request.buffers_[vfStream_]);
+		processViewfinder(request.request_, request.buffers_[vfStream_]);
 
 	if (request.buffers_.count(rawStream_))
 		processRaw(request.buffers_[rawStream_], request.metadata_);
 }
 
-void MainWindow::processViewfinder(FrameBuffer *buffer)
+void MainWindow::processViewfinder(Request *request, FrameBuffer *buffer)
 {
 	framesCaptured_++;
 
@@ -749,16 +751,12 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
 		<< "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps;
 
 	/* Render the frame on the viewfinder. */
-	viewfinder_->render(buffer, &mappedBuffers_[buffer]);
+	viewfinder_->render(request, buffer, &mappedBuffers_[buffer]);
 }
 
-void MainWindow::queueRequest(FrameBuffer *buffer)
+void MainWindow::queueRequest(Request *request, FrameBuffer *buffer)
 {
-	Request *request = camera_->createRequest();
-	if (!request) {
-		qWarning() << "Can't create request";
-		return;
-	}
+	request->reset();
 
 	request->addBuffer(vfStream_, buffer);
 
diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
index 5c61a4df..83535373 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"
@@ -49,13 +51,15 @@ public:
 	}
 
 	CaptureRequest(const Request::BufferMap &buffers,
-		       const ControlList &metadata)
-		: buffers_(buffers), metadata_(metadata)
+		       const ControlList &metadata,
+		       Request * const request)
+		: buffers_(buffers), metadata_(metadata), request_(request)
 	{
 	}
 
 	Request::BufferMap buffers_;
 	ControlList metadata_;
+	Request *request_;
 };
 
 class MainWindow : public QMainWindow
@@ -79,7 +83,7 @@ private Q_SLOTS:
 	void captureRaw();
 	void processRaw(FrameBuffer *buffer, const ControlList &metadata);
 
-	void queueRequest(FrameBuffer *buffer);
+	void queueRequest(Request *request, FrameBuffer *buffer);
 
 private:
 	int createToolbars();
@@ -96,7 +100,7 @@ private:
 	void requestComplete(Request *request);
 	void processCapture();
 	void processHotplug(HotplugEvent *e);
-	void processViewfinder(FrameBuffer *buffer);
+	void processViewfinder(Request *request, FrameBuffer *buffer);
 
 	/* UI elements */
 	QToolBar *toolbar_;
@@ -135,6 +139,8 @@ private:
 	QElapsedTimer frameRateInterval_;
 	uint32_t previousFrames_;
 	uint32_t framesCaptured_;
+
+	std::vector<Request *> requests_;
 };
 
 #endif /* __QCAM_MAIN_WINDOW__ */
diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h
index 67da1df2..26c20f1b 100644
--- a/src/qcam/viewfinder.h
+++ b/src/qcam/viewfinder.h
@@ -27,7 +27,9 @@ public:
 	virtual const QList<libcamera::PixelFormat> &nativeFormats() const = 0;
 
 	virtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) = 0;
-	virtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) = 0;
+	virtual void render(libcamera::Request *request,
+			    libcamera::FrameBuffer *buffer,
+			    MappedBuffer *map) = 0;
 	virtual void stop() = 0;
 
 	virtual QImage getCurrentImage() = 0;
diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
index fbe21dcf..c65b41f6 100644
--- a/src/qcam/viewfinder_gl.cpp
+++ b/src/qcam/viewfinder_gl.cpp
@@ -23,7 +23,7 @@ static const QList<libcamera::PixelFormat> supportedFormats{
 };
 
 ViewFinderGL::ViewFinderGL(QWidget *parent)
-	: QOpenGLWidget(parent), buffer_(nullptr), yuvData_(nullptr),
+	: QOpenGLWidget(parent), request_(nullptr), buffer_(nullptr), yuvData_(nullptr),
 	  fragmentShader_(nullptr), vertexShader_(nullptr),
 	  vertexBuffer_(QOpenGLBuffer::VertexBuffer),
 	  textureU_(QOpenGLTexture::Target2D),
@@ -67,7 +67,7 @@ int ViewFinderGL::setFormat(const libcamera::PixelFormat &format,
 void ViewFinderGL::stop()
 {
 	if (buffer_) {
-		renderComplete(buffer_);
+		renderComplete(request_, buffer_);
 		buffer_ = nullptr;
 	}
 }
@@ -79,7 +79,7 @@ QImage ViewFinderGL::getCurrentImage()
 	return grabFramebuffer();
 }
 
-void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)
+void ViewFinderGL::render(libcamera::Request *request, libcamera::FrameBuffer *buffer, MappedBuffer *map)
 {
 	if (buffer->planes().size() != 1) {
 		qWarning() << "Multi-planar buffers are not supported";
@@ -87,11 +87,12 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)
 	}
 
 	if (buffer_)
-		renderComplete(buffer_);
+		renderComplete(request_, buffer_);
 
 	yuvData_ = static_cast<unsigned char *>(map->memory);
 	update();
 	buffer_ = buffer;
+	request_ = request;
 }
 
 bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)
diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h
index 69502b7a..7db41841 100644
--- a/src/qcam/viewfinder_gl.h
+++ b/src/qcam/viewfinder_gl.h
@@ -36,13 +36,13 @@ public:
 	const QList<libcamera::PixelFormat> &nativeFormats() const override;
 
 	int setFormat(const libcamera::PixelFormat &format, const QSize &size) override;
-	void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;
+	void render(libcamera::Request *request, libcamera::FrameBuffer *buffer, MappedBuffer *map) override;
 	void stop() override;
 
 	QImage getCurrentImage() override;
 
 Q_SIGNALS:
-	void renderComplete(libcamera::FrameBuffer *buffer);
+	void renderComplete(libcamera::Request *request, libcamera::FrameBuffer *buffer);
 
 protected:
 	void initializeGL() override;
@@ -60,6 +60,7 @@ private:
 	void doRender();
 
 	/* Captured image size, format and buffer */
+	libcamera::Request *request_;
 	libcamera::FrameBuffer *buffer_;
 	libcamera::PixelFormat format_;
 	QSize size_;
diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp
index e436714c..7f9f913b 100644
--- a/src/qcam/viewfinder_qt.cpp
+++ b/src/qcam/viewfinder_qt.cpp
@@ -34,7 +34,7 @@ static const QMap<libcamera::PixelFormat, QImage::Format> nativeFormats
 };
 
 ViewFinderQt::ViewFinderQt(QWidget *parent)
-	: QWidget(parent), buffer_(nullptr)
+	: QWidget(parent), request_(nullptr), buffer_(nullptr)
 {
 	icon_ = QIcon(":camera-off.svg");
 }
@@ -78,7 +78,8 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format,
 	return 0;
 }
 
-void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)
+void ViewFinderQt::render(libcamera::Request *request,
+			  libcamera::FrameBuffer *buffer, MappedBuffer *map)
 {
 	if (buffer->planes().size() != 1) {
 		qWarning() << "Multi-planar buffers are not supported";
@@ -106,6 +107,7 @@ void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)
 					size / size_.height(),
 					::nativeFormats[format_]);
 			std::swap(buffer, buffer_);
+			std::swap(request, request_);
 		} else {
 			/*
 			 * Otherwise, convert the format and release the frame
@@ -118,7 +120,7 @@ void ViewFinderQt::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)
 	update();
 
 	if (buffer)
-		renderComplete(buffer);
+		renderComplete(request, buffer);
 }
 
 void ViewFinderQt::stop()
@@ -126,7 +128,7 @@ void ViewFinderQt::stop()
 	image_ = QImage();
 
 	if (buffer_) {
-		renderComplete(buffer_);
+		renderComplete(request_, buffer_);
 		buffer_ = nullptr;
 	}
 
diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h
index d7554288..5f62d7f6 100644
--- a/src/qcam/viewfinder_qt.h
+++ b/src/qcam/viewfinder_qt.h
@@ -17,6 +17,7 @@
 #include <libcamera/buffer.h>
 #include <libcamera/formats.h>
 #include <libcamera/pixel_format.h>
+#include <libcamera/request.h>
 
 #include "format_converter.h"
 #include "viewfinder.h"
@@ -32,13 +33,15 @@ public:
 	const QList<libcamera::PixelFormat> &nativeFormats() const override;
 
 	int setFormat(const libcamera::PixelFormat &format, const QSize &size) override;
-	void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;
+	void render(libcamera::Request *request,
+		    libcamera::FrameBuffer *buffer,
+		    MappedBuffer *map) override;
 	void stop() override;
 
 	QImage getCurrentImage() override;
 
 Q_SIGNALS:
-	void renderComplete(libcamera::FrameBuffer *buffer);
+	void renderComplete(libcamera::Request *request, libcamera::FrameBuffer *buffer);
 
 protected:
 	void paintEvent(QPaintEvent *) override;
@@ -56,6 +59,7 @@ private:
 	QPixmap pixmap_;
 
 	/* Buffer and render image */
+	libcamera::Request *request_;
 	libcamera::FrameBuffer *buffer_;
 	QImage image_;
 	QMutex mutex_; /* Prevent concurrent access to image_ */
-- 
2.27.0



More information about the libcamera-devel mailing list