[libcamera-devel,v2,19/21] qcam: viewfinder: Avoid memory copy when conversion isn't needed

Message ID 20200323173559.21109-20-laurent.pinchart@ideasonboard.com
State Accepted
Headers show
Series
  • qcam: Bypass format conversion when not required
Related show

Commit Message

Laurent Pinchart March 23, 2020, 5:35 p.m. UTC
If the frame buffer format is identical to the display format, the
viewfinder still invokes the converter to perform what is essentially a
slow memcpy(). Make it possible to skip that operation by creating a
QImage referencing the buffer memory instead. A reference to the frame
buffer is kept internally, and released when the next buffer is queued,
pushing the current one out.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
---
Changes since v1:

- Add stop() function to release the buffer
---
 src/qcam/main_window.cpp |  2 ++
 src/qcam/viewfinder.cpp  | 74 +++++++++++++++++++++++++++++++---------
 src/qcam/viewfinder.h    |  2 ++
 3 files changed, 61 insertions(+), 17 deletions(-)

Patch

diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index e872bdd7ba86..b13660bccb25 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -416,6 +416,8 @@  void MainWindow::stopCapture()
 	if (!isCapturing_)
 		return;
 
+	viewfinder_->stop();
+
 	int ret = camera_->stop();
 	if (ret)
 		qInfo() << "Failed to stop capture";
diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp
index 31b358da47dc..3f984efbf434 100644
--- a/src/qcam/viewfinder.cpp
+++ b/src/qcam/viewfinder.cpp
@@ -7,6 +7,8 @@ 
 
 #include "viewfinder.h"
 
+#include <utility>
+
 #include <QImage>
 #include <QImageWriter>
 #include <QMutexLocker>
@@ -16,7 +18,7 @@ 
 #include "format_converter.h"
 
 ViewFinder::ViewFinder(QWidget *parent)
-	: QWidget(parent)
+	: QWidget(parent), buffer_(nullptr)
 {
 }
 
@@ -27,17 +29,23 @@  ViewFinder::~ViewFinder()
 int ViewFinder::setFormat(const libcamera::PixelFormat &format,
 			  const QSize &size)
 {
-	int ret;
+	image_ = QImage();
 
-	ret = converter_.configure(format, size);
-	if (ret < 0)
-		return ret;
+	/*
+	 * If format conversion is needed, configure the converter and allocate
+	 * the destination image.
+	 */
+	if (format != DRM_FORMAT_ARGB8888) {
+		int ret = converter_.configure(format, size);
+		if (ret < 0)
+			return ret;
+
+		image_ = QImage(size, QImage::Format_RGB32);
+	}
 
 	format_ = format;
 	size_ = size;
 
-	image_ = QImage(size_, QImage::Format_RGB32);
-
 	updateGeometry();
 	return 0;
 }
@@ -49,19 +57,51 @@  void ViewFinder::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)
 		return;
 	}
 
-	QMutexLocker locker(&mutex_);
-
-	/*
-	 * \todo We're not supposed to block the pipeline handler thread
-	 * for long, implement a better way to save images without
-	 * impacting performances.
-	 */
+	unsigned char *memory = static_cast<unsigned char *>(map->memory);
+	size_t size = buffer->metadata().planes[0].bytesused;
+
+	{
+		QMutexLocker locker(&mutex_);
+
+		if (format_ == DRM_FORMAT_ARGB8888) {
+			/*
+			 * If the frame format is identical to the display
+			 * format, create a QImage that references the frame
+			 * and store a reference to the frame buffer. The
+			 * previously stored frame buffer, if any, will be
+			 * released.
+			 *
+			 * \todo Get the stride from the buffer instead of
+			 * computing it naively
+			 */
+			image_ = QImage(memory, size_.width(), size_.height(),
+					size / size_.height(), QImage::Format_RGB32);
+			std::swap(buffer, buffer_);
+		} else {
+			/*
+			 * Otherwise, convert the format and release the frame
+			 * buffer immediately.
+			 */
+			converter_.convert(memory, size, &image_);
+		}
+	}
 
-	converter_.convert(static_cast<unsigned char *>(map->memory),
-			   buffer->metadata().planes[0].bytesused, &image_);
 	update();
 
-	renderComplete(buffer);
+	if (buffer)
+		renderComplete(buffer);
+}
+
+void ViewFinder::stop()
+{
+	image_ = QImage();
+
+	if (buffer_) {
+		renderComplete(buffer_);
+		buffer_ = nullptr;
+	}
+
+	update();
 }
 
 QImage ViewFinder::getCurrentImage()
diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h
index 54c0fa9dd7a0..4d0622a8e4ea 100644
--- a/src/qcam/viewfinder.h
+++ b/src/qcam/viewfinder.h
@@ -36,6 +36,7 @@  public:
 
 	int setFormat(const libcamera::PixelFormat &format, const QSize &size);
 	void render(libcamera::FrameBuffer *buffer, MappedBuffer *map);
+	void stop();
 
 	QImage getCurrentImage();
 
@@ -52,6 +53,7 @@  private:
 	libcamera::PixelFormat format_;
 	QSize size_;
 
+	libcamera::FrameBuffer *buffer_;
 	QImage image_;
 	QMutex mutex_; /* Prevent concurrent access to image_ */
 };