[libcamera-devel] [PATCH v3 27/30] qcam: Use Image class to access pixel data

Kieran Bingham kieran.bingham at ideasonboard.com
Tue Sep 7 13:57:42 CEST 2021


On 06/09/2021 23:56, Laurent Pinchart wrote:
> Replace the manual implementation of frame buffer mapping with the Image
> class to improve code sharing. The ViewFinder API is updated to take an
> Image pointer in the render() function to prepare for multi-planar
> buffer support.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>

Much better IMO.

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


> ---
>  src/qcam/main_window.cpp   | 31 ++++++++-----------------------
>  src/qcam/main_window.h     |  4 ++--
>  src/qcam/meson.build       |  1 +
>  src/qcam/viewfinder.h      |  6 +++---
>  src/qcam/viewfinder_gl.cpp |  7 ++++---
>  src/qcam/viewfinder_gl.h   |  2 +-
>  src/qcam/viewfinder_qt.cpp |  6 +++---
>  src/qcam/viewfinder_qt.h   |  2 +-
>  8 files changed, 23 insertions(+), 36 deletions(-)
> 
> diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> index 0a00b1001570..168dd5ce30e3 100644
> --- a/src/qcam/main_window.cpp
> +++ b/src/qcam/main_window.cpp
> @@ -7,10 +7,9 @@
>  
>  #include "main_window.h"
>  
> +#include <assert.h>
>  #include <iomanip>
>  #include <string>
> -#include <sys/mman.h>
> -#include <unistd.h>
>  
>  #include <QComboBox>
>  #include <QCoreApplication>
> @@ -29,6 +28,7 @@
>  #include <libcamera/camera_manager.h>
>  #include <libcamera/version.h>
>  
> +#include "../cam/image.h"
>  #include "dng_writer.h"
>  #ifndef QT_NO_OPENGL
>  #include "viewfinder_gl.h"
> @@ -473,15 +473,10 @@ int MainWindow::startCapture()
>  
>  		for (const std::unique_ptr<FrameBuffer> &buffer : allocator_->buffers(stream)) {
>  			/* Map memory buffers and cache the mappings. */
> -			const FrameBuffer::Plane &plane = buffer->planes().front();
> -			size_t length = lseek(plane.fd.fd(), 0, SEEK_END);
> -			void *memory = mmap(NULL, length, PROT_READ, MAP_SHARED,
> -					    plane.fd.fd(), 0);
> -
> -			mappedBuffers_[buffer.get()] = { static_cast<uint8_t *>(memory),
> -							 plane.length };
> -			planeData_[buffer.get()] = { static_cast<uint8_t *>(memory) + plane.offset,
> -						     plane.length };
> +			std::unique_ptr<Image> image =
> +				Image::fromFrameBuffer(buffer.get(), Image::MapMode::ReadOnly);
> +			assert(image != nullptr);
> +			mappedBuffers_[buffer.get()] = std::move(image);
>  
>  			/* Store buffers on the free list. */
>  			freeBuffers_[stream].enqueue(buffer.get());
> @@ -543,12 +538,7 @@ error_disconnect:
>  error:
>  	requests_.clear();
>  
> -	for (auto &iter : mappedBuffers_) {
> -		const Span<uint8_t> &buffer = iter.second;
> -		munmap(buffer.data(), buffer.size());
> -	}
>  	mappedBuffers_.clear();
> -	planeData_.clear();
>  
>  	freeBuffers_.clear();
>  
> @@ -580,12 +570,7 @@ void MainWindow::stopCapture()
>  
>  	camera_->requestCompleted.disconnect(this);
>  
> -	for (auto &iter : mappedBuffers_) {
> -		const Span<uint8_t> &buffer = iter.second;
> -		munmap(buffer.data(), buffer.size());
> -	}
>  	mappedBuffers_.clear();
> -	planeData_.clear();
>  
>  	requests_.clear();
>  	freeQueue_.clear();
> @@ -682,7 +667,7 @@ void MainWindow::processRaw(FrameBuffer *buffer,
>  							"DNG Files (*.dng)");
>  
>  	if (!filename.isEmpty()) {
> -		uint8_t *memory = planeData_[buffer].data();
> +		uint8_t *memory = mappedBuffers_[buffer]->data(0).data();
>  		DNGWriter::write(filename.toStdString().c_str(), camera_.get(),
>  				 rawStream_->configuration(), metadata, buffer,
>  				 memory);
> @@ -766,7 +751,7 @@ void MainWindow::processViewfinder(FrameBuffer *buffer)
>  		<< "fps:" << Qt::fixed << qSetRealNumberPrecision(2) << fps;
>  
>  	/* Render the frame on the viewfinder. */
> -	viewfinder_->render(buffer, planeData_[buffer]);
> +	viewfinder_->render(buffer, mappedBuffers_[buffer].get());
>  }
>  
>  void MainWindow::queueRequest(FrameBuffer *buffer)
> diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> index 28244bca58b2..a16bea09eadc 100644
> --- a/src/qcam/main_window.h
> +++ b/src/qcam/main_window.h
> @@ -34,6 +34,7 @@ using namespace libcamera;
>  class QAction;
>  class QComboBox;
>  
> +class Image;
>  class HotplugEvent;
>  
>  enum {
> @@ -106,8 +107,7 @@ private:
>  	FrameBufferAllocator *allocator_;
>  
>  	std::unique_ptr<CameraConfiguration> config_;
> -	std::map<FrameBuffer *, Span<uint8_t>> mappedBuffers_;
> -	std::map<FrameBuffer *, Span<uint8_t>> planeData_;
> +	std::map<FrameBuffer *, std::unique_ptr<Image>> mappedBuffers_;
>  
>  	/* Capture state, buffers queue and statistics */
>  	bool isCapturing_;
> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> index 7d3621c93d41..c46f463130cd 100644
> --- a/src/qcam/meson.build
> +++ b/src/qcam/meson.build
> @@ -15,6 +15,7 @@ endif
>  qcam_enabled = true
>  
>  qcam_sources = files([
> +    '../cam/image.cpp',
>      '../cam/options.cpp',
>      '../cam/stream_options.cpp',
>      'format_converter.cpp',
> diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h
> index 42d40f1f33f0..fb462835fb5f 100644
> --- a/src/qcam/viewfinder.h
> +++ b/src/qcam/viewfinder.h
> @@ -11,11 +11,11 @@
>  #include <QList>
>  #include <QSize>
>  
> -#include <libcamera/base/span.h>
> -
>  #include <libcamera/formats.h>
>  #include <libcamera/framebuffer.h>
>  
> +class Image;
> +
>  class ViewFinder
>  {
>  public:
> @@ -24,7 +24,7 @@ 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, libcamera::Span<uint8_t> mem) = 0;
> +	virtual void render(libcamera::FrameBuffer *buffer, Image *image) = 0;
>  	virtual void stop() = 0;
>  
>  	virtual QImage getCurrentImage() = 0;
> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
> index d2ef036974f4..87e4fe03cb8d 100644
> --- a/src/qcam/viewfinder_gl.cpp
> +++ b/src/qcam/viewfinder_gl.cpp
> @@ -13,6 +13,8 @@
>  
>  #include <libcamera/formats.h>
>  
> +#include "../cam/image.h"
> +
>  static const QList<libcamera::PixelFormat> supportedFormats{
>  	/* YUV - packed (single plane) */
>  	libcamera::formats::UYVY,
> @@ -110,8 +112,7 @@ QImage ViewFinderGL::getCurrentImage()
>  	return grabFramebuffer();
>  }
>  
> -void ViewFinderGL::render(libcamera::FrameBuffer *buffer,
> -			  libcamera::Span<uint8_t> mem)
> +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, Image *image)
>  {
>  	if (buffer->planes().size() != 1) {
>  		qWarning() << "Multi-planar buffers are not supported";
> @@ -121,7 +122,7 @@ void ViewFinderGL::render(libcamera::FrameBuffer *buffer,
>  	if (buffer_)
>  		renderComplete(buffer_);
>  
> -	data_ = mem.data();
> +	data_ = image->data(0).data();
>  	/*
>  	 * \todo Get the stride from the buffer instead of computing it naively
>  	 */
> diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h
> index 3334549e0be4..7cd8ef3316b9 100644
> --- a/src/qcam/viewfinder_gl.h
> +++ b/src/qcam/viewfinder_gl.h
> @@ -39,7 +39,7 @@ public:
>  	const QList<libcamera::PixelFormat> &nativeFormats() const override;
>  
>  	int setFormat(const libcamera::PixelFormat &format, const QSize &size) override;
> -	void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override;
> +	void render(libcamera::FrameBuffer *buffer, Image *image) override;
>  	void stop() override;
>  
>  	QImage getCurrentImage() override;
> diff --git a/src/qcam/viewfinder_qt.cpp b/src/qcam/viewfinder_qt.cpp
> index a0bf99b0b522..fef6d53eef5e 100644
> --- a/src/qcam/viewfinder_qt.cpp
> +++ b/src/qcam/viewfinder_qt.cpp
> @@ -19,6 +19,7 @@
>  
>  #include <libcamera/formats.h>
>  
> +#include "../cam/image.h"
>  #include "format_converter.h"
>  
>  static const QMap<libcamera::PixelFormat, QImage::Format> nativeFormats
> @@ -78,15 +79,14 @@ int ViewFinderQt::setFormat(const libcamera::PixelFormat &format,
>  	return 0;
>  }
>  
> -void ViewFinderQt::render(libcamera::FrameBuffer *buffer,
> -			  libcamera::Span<uint8_t> mem)
> +void ViewFinderQt::render(libcamera::FrameBuffer *buffer, Image *image)
>  {
>  	if (buffer->planes().size() != 1) {
>  		qWarning() << "Multi-planar buffers are not supported";
>  		return;
>  	}
>  
> -	unsigned char *memory = mem.data();
> +	unsigned char *memory = image->data(0).data();
>  	size_t size = buffer->metadata().planes()[0].bytesused;
>  
>  	{
> diff --git a/src/qcam/viewfinder_qt.h b/src/qcam/viewfinder_qt.h
> index 1a569b9cee6e..6b48ef48a7d1 100644
> --- a/src/qcam/viewfinder_qt.h
> +++ b/src/qcam/viewfinder_qt.h
> @@ -32,7 +32,7 @@ public:
>  	const QList<libcamera::PixelFormat> &nativeFormats() const override;
>  
>  	int setFormat(const libcamera::PixelFormat &format, const QSize &size) override;
> -	void render(libcamera::FrameBuffer *buffer, libcamera::Span<uint8_t> mem) override;
> +	void render(libcamera::FrameBuffer *buffer, Image *image) override;
>  	void stop() override;
>  
>  	QImage getCurrentImage() override;
> 


More information about the libcamera-devel mailing list