[libcamera-devel,13/21] qcam: viewfinder: Add MappedBuffer to store memory mapping information

Message ID 20200323142205.28342-14-laurent.pinchart@ideasonboard.com
State Superseded
Headers show
Series
  • qcam: Bypass format conversion when not required
Related show

Commit Message

Laurent Pinchart March 23, 2020, 2:21 p.m. UTC
The new MappedBuffer structure replaces the std::pair<> used in the
mapped buffers map, and allows passing data to the ViewFinder::display()
function in a more structured way.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/qcam/main_window.cpp | 20 ++++++--------------
 src/qcam/main_window.h   |  4 ++--
 src/qcam/viewfinder.cpp  |  6 ++++--
 src/qcam/viewfinder.h    | 10 +++++++++-
 4 files changed, 21 insertions(+), 19 deletions(-)

Comments

Kieran Bingham March 23, 2020, 4:16 p.m. UTC | #1
Hi Laurent,

On 23/03/2020 14:21, Laurent Pinchart wrote:
> The new MappedBuffer structure replaces the std::pair<> used in the
> mapped buffers map, and allows passing data to the ViewFinder::display()
> function in a more structured way.
> 

Seems reasonable for now ...

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/qcam/main_window.cpp | 20 ++++++--------------
>  src/qcam/main_window.h   |  4 ++--
>  src/qcam/viewfinder.cpp  |  6 ++++--
>  src/qcam/viewfinder.h    | 10 +++++++++-
>  4 files changed, 21 insertions(+), 19 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 184345b0e2a6..c0feb91a6fb1 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -28,8 +28,6 @@
>  #include <libcamera/camera_manager.h>
>  #include <libcamera/version.h>
>  
> -#include "viewfinder.h"
> -
>  using namespace libcamera;
>  
>  /**
> @@ -350,8 +348,7 @@ int MainWindow::startCapture()
>  		const FrameBuffer::Plane &plane = buffer->planes().front();
>  		void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,
>  				    plane.fd.fd(), 0);
> -		mappedBuffers_[plane.fd.fd()] =
> -			std::make_pair(memory, plane.length);
> +		mappedBuffers_[buffer.get()] = { memory, plane.length };
>  	}
>  
>  	/* Start the title timer and the camera. */
> @@ -391,9 +388,8 @@ error:
>  		delete request;
>  
>  	for (auto &iter : mappedBuffers_) {
> -		void *memory = iter.second.first;
> -		unsigned int length = iter.second.second;
> -		munmap(memory, length);
> +		const MappedBuffer &buffer = iter.second;
> +		munmap(buffer.memory, buffer.size);
>  	}
>  	mappedBuffers_.clear();
>  
> @@ -421,9 +417,8 @@ void MainWindow::stopCapture()
>  	camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete);
>  
>  	for (auto &iter : mappedBuffers_) {
> -		void *memory = iter.second.first;
> -		unsigned int length = iter.second.second;
> -		munmap(memory, length);
> +		const MappedBuffer &buffer = iter.second;
> +		munmap(buffer.memory, buffer.size);
>  	}
>  	mappedBuffers_.clear();
>  
> @@ -529,10 +524,7 @@ int MainWindow::display(FrameBuffer *buffer)
>  	if (buffer->planes().size() != 1)
>  		return -EINVAL;
>  
> -	const FrameBuffer::Plane &plane = buffer->planes().front();
> -	void *memory = mappedBuffers_[plane.fd.fd()].first;
> -	unsigned char *raw = static_cast<unsigned char *>(memory);
> -	viewfinder_->display(raw, buffer->metadata().planes[0].bytesused);
> +	viewfinder_->display(buffer, &mappedBuffers_[buffer]);

Wow, yes that's simpler!

>  
>  	return 0;
>  }
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index fcde88c14f77..0918ae5307d8 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -23,11 +23,11 @@
>  #include <libcamera/stream.h>
>  
>  #include "../cam/options.h"
> +#include "viewfinder.h"
>  
>  using namespace libcamera;
>  
>  class QAction;
> -class ViewFinder;
>  
>  enum {
>  	OptCamera = 'c',
> @@ -85,7 +85,7 @@ private:
>  	FrameBufferAllocator *allocator_;
>  
>  	std::unique_ptr<CameraConfiguration> config_;
> -	std::map<int, std::pair<void *, unsigned int>> mappedBuffers_;
> +	std::map<FrameBuffer *, MappedBuffer> mappedBuffers_;
>  
>  	/* Capture state, buffers queue and statistics */
>  	bool isCapturing_;
> diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp
> index 066ac605e7b4..d00edc33dfb7 100644
> --- a/src/qcam/viewfinder.cpp
> +++ b/src/qcam/viewfinder.cpp
> @@ -24,7 +24,8 @@ ViewFinder::~ViewFinder()
>  	delete image_;
>  }
>  
> -void ViewFinder::display(const unsigned char *raw, size_t size)
> +void ViewFinder::display(const libcamera::FrameBuffer *buffer,
> +			 MappedBuffer *map)
>  {
>  	QMutexLocker locker(&mutex_);
>  
> @@ -34,7 +35,8 @@ void ViewFinder::display(const unsigned char *raw, size_t size)
>  	 * impacting performances.
>  	 */
>  
> -	converter_.convert(raw, size, image_);
> +	converter_.convert(static_cast<unsigned char *>(map->memory),
> +			   buffer->metadata().planes[0].bytesused, image_);
>  	update();
>  }
>  
> diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h
> index a019c3a470ea..735a6b67e91d 100644
> --- a/src/qcam/viewfinder.h
> +++ b/src/qcam/viewfinder.h
> @@ -7,16 +7,24 @@
>  #ifndef __QCAM_VIEWFINDER_H__
>  #define __QCAM_VIEWFINDER_H__
>  
> +#include <stddef.h>
> +
>  #include <QMutex>
>  #include <QSize>
>  #include <QWidget>
>  
> +#include <libcamera/buffer.h>
>  #include <libcamera/pixelformats.h>
>  
>  #include "format_converter.h"
>  
>  class QImage;
>  
> +struct MappedBuffer {
> +	void *memory;
> +	size_t size;
> +};
> +
>  class ViewFinder : public QWidget
>  {
>  public:
> @@ -24,7 +32,7 @@ public:
>  	~ViewFinder();
>  
>  	int setFormat(const libcamera::PixelFormat &format, const QSize &size);
> -	void display(const unsigned char *rgb, size_t size);
> +	void display(const libcamera::FrameBuffer *buffer, MappedBuffer *map);
>  
>  	QImage getCurrentImage();
>  
>

Patch

diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
index 184345b0e2a6..c0feb91a6fb1 100644
--- a/src/qcam/main_window.cpp
+++ b/src/qcam/main_window.cpp
@@ -28,8 +28,6 @@ 
 #include <libcamera/camera_manager.h>
 #include <libcamera/version.h>
 
-#include "viewfinder.h"
-
 using namespace libcamera;
 
 /**
@@ -350,8 +348,7 @@  int MainWindow::startCapture()
 		const FrameBuffer::Plane &plane = buffer->planes().front();
 		void *memory = mmap(NULL, plane.length, PROT_READ, MAP_SHARED,
 				    plane.fd.fd(), 0);
-		mappedBuffers_[plane.fd.fd()] =
-			std::make_pair(memory, plane.length);
+		mappedBuffers_[buffer.get()] = { memory, plane.length };
 	}
 
 	/* Start the title timer and the camera. */
@@ -391,9 +388,8 @@  error:
 		delete request;
 
 	for (auto &iter : mappedBuffers_) {
-		void *memory = iter.second.first;
-		unsigned int length = iter.second.second;
-		munmap(memory, length);
+		const MappedBuffer &buffer = iter.second;
+		munmap(buffer.memory, buffer.size);
 	}
 	mappedBuffers_.clear();
 
@@ -421,9 +417,8 @@  void MainWindow::stopCapture()
 	camera_->requestCompleted.disconnect(this, &MainWindow::requestComplete);
 
 	for (auto &iter : mappedBuffers_) {
-		void *memory = iter.second.first;
-		unsigned int length = iter.second.second;
-		munmap(memory, length);
+		const MappedBuffer &buffer = iter.second;
+		munmap(buffer.memory, buffer.size);
 	}
 	mappedBuffers_.clear();
 
@@ -529,10 +524,7 @@  int MainWindow::display(FrameBuffer *buffer)
 	if (buffer->planes().size() != 1)
 		return -EINVAL;
 
-	const FrameBuffer::Plane &plane = buffer->planes().front();
-	void *memory = mappedBuffers_[plane.fd.fd()].first;
-	unsigned char *raw = static_cast<unsigned char *>(memory);
-	viewfinder_->display(raw, buffer->metadata().planes[0].bytesused);
+	viewfinder_->display(buffer, &mappedBuffers_[buffer]);
 
 	return 0;
 }
diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
index fcde88c14f77..0918ae5307d8 100644
--- a/src/qcam/main_window.h
+++ b/src/qcam/main_window.h
@@ -23,11 +23,11 @@ 
 #include <libcamera/stream.h>
 
 #include "../cam/options.h"
+#include "viewfinder.h"
 
 using namespace libcamera;
 
 class QAction;
-class ViewFinder;
 
 enum {
 	OptCamera = 'c',
@@ -85,7 +85,7 @@  private:
 	FrameBufferAllocator *allocator_;
 
 	std::unique_ptr<CameraConfiguration> config_;
-	std::map<int, std::pair<void *, unsigned int>> mappedBuffers_;
+	std::map<FrameBuffer *, MappedBuffer> mappedBuffers_;
 
 	/* Capture state, buffers queue and statistics */
 	bool isCapturing_;
diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp
index 066ac605e7b4..d00edc33dfb7 100644
--- a/src/qcam/viewfinder.cpp
+++ b/src/qcam/viewfinder.cpp
@@ -24,7 +24,8 @@  ViewFinder::~ViewFinder()
 	delete image_;
 }
 
-void ViewFinder::display(const unsigned char *raw, size_t size)
+void ViewFinder::display(const libcamera::FrameBuffer *buffer,
+			 MappedBuffer *map)
 {
 	QMutexLocker locker(&mutex_);
 
@@ -34,7 +35,8 @@  void ViewFinder::display(const unsigned char *raw, size_t size)
 	 * impacting performances.
 	 */
 
-	converter_.convert(raw, size, image_);
+	converter_.convert(static_cast<unsigned char *>(map->memory),
+			   buffer->metadata().planes[0].bytesused, image_);
 	update();
 }
 
diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h
index a019c3a470ea..735a6b67e91d 100644
--- a/src/qcam/viewfinder.h
+++ b/src/qcam/viewfinder.h
@@ -7,16 +7,24 @@ 
 #ifndef __QCAM_VIEWFINDER_H__
 #define __QCAM_VIEWFINDER_H__
 
+#include <stddef.h>
+
 #include <QMutex>
 #include <QSize>
 #include <QWidget>
 
+#include <libcamera/buffer.h>
 #include <libcamera/pixelformats.h>
 
 #include "format_converter.h"
 
 class QImage;
 
+struct MappedBuffer {
+	void *memory;
+	size_t size;
+};
+
 class ViewFinder : public QWidget
 {
 public:
@@ -24,7 +32,7 @@  public:
 	~ViewFinder();
 
 	int setFormat(const libcamera::PixelFormat &format, const QSize &size);
-	void display(const unsigned char *rgb, size_t size);
+	void display(const libcamera::FrameBuffer *buffer, MappedBuffer *map);
 
 	QImage getCurrentImage();