[libcamera-devel] [PATCH v6 3/4] qcam: add viewfinderGL class to accelerate the format convert

Laurent Pinchart laurent.pinchart at ideasonboard.com
Sat Sep 12 04:09:10 CEST 2020


Hi Show,

Thank you for the patch.

In the subject line, s/viewfinderGL/ViewFinderGL/ and
s/convert/conversion/

On Fri, Sep 11, 2020 at 04:55:13PM +0800, Show Liu wrote:
> the viewfinderGL accelerates the format conversion by
> using OpenGL ES shader

I would add

"The minimum Qt version is bumped to v5.4, as QOpenGLWidget wasn't
available before that."

> Signed-off-by: Show Liu <show.liu at linaro.org>
> ---
>  meson.build                |   1 +
>  src/qcam/meson.build       |  17 +-
>  src/qcam/viewfinder_gl.cpp | 456 +++++++++++++++++++++++++++++++++++++
>  src/qcam/viewfinder_gl.h   |  96 ++++++++
>  4 files changed, 568 insertions(+), 2 deletions(-)
>  create mode 100644 src/qcam/viewfinder_gl.cpp
>  create mode 100644 src/qcam/viewfinder_gl.h
> 
> diff --git a/meson.build b/meson.build
> index 1ea35e9..c58d458 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -26,6 +26,7 @@ libcamera_version = libcamera_git_version.split('+')[0]
>  
>  # Configure the build environment.
>  cc = meson.get_compiler('c')
> +cxx = meson.get_compiler('cpp')
>  config_h = configuration_data()
>  
>  if cc.has_header_symbol('execinfo.h', 'backtrace')
> diff --git a/src/qcam/meson.build b/src/qcam/meson.build
> index a4bad0a..9bb48c0 100644
> --- a/src/qcam/meson.build
> +++ b/src/qcam/meson.build
> @@ -16,14 +16,14 @@ qcam_moc_headers = files([
>  
>  qcam_resources = files([
>      'assets/feathericons/feathericons.qrc',
> -    'assets/shader/shaders.qrc'
>  ])
>  
>  qt5 = import('qt5')
>  qt5_dep = dependency('qt5',
>                       method : 'pkg-config',
>                       modules : ['Core', 'Gui', 'Widgets'],
> -                     required : get_option('qcam'))
> +                     required : get_option('qcam'),
> +                     version : '>=5.4')
>  
>  if qt5_dep.found()
>      qcam_deps = [
> @@ -42,6 +42,19 @@ if qt5_dep.found()
>          ])
>      endif
>  
> +    if cxx.has_header_symbol('QOpenGLWidget', 'QOpenGLWidget',
> +                             dependencies : qt5_dep, args : '-fPIC')
> +        qcam_sources += files([
> +            'viewfinder_gl.cpp',
> +        ])
> +        qcam_moc_headers += files([
> +            'viewfinder_gl.h',
> +        ])
> +        qcam_resources += files([
> +            'assets/shader/shaders.qrc'
> +        ])
> +    endif
> +
>      # gcc 9 introduced a deprecated-copy warning that is triggered by Qt until
>      # Qt 5.13. clang 10 introduced the same warning, but detects more issues
>      # that are not fixed in Qt yet. Disable the warning manually in both cases.
> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
> new file mode 100644
> index 0000000..84f4866
> --- /dev/null
> +++ b/src/qcam/viewfinder_gl.cpp
> @@ -0,0 +1,456 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Linaro
> + *
> + * viewfinderGL.cpp - OpenGL Viewfinder for rendering by OpenGL shader
> + */
> +
> +#include "viewfinder_gl.h"
> +
> +#include <QImage>
> +
> +#include <libcamera/formats.h>
> +
> +static const QList<libcamera::PixelFormat> supportedFormats{
> +	libcamera::formats::NV12,
> +	libcamera::formats::NV21,
> +	libcamera::formats::NV16,
> +	libcamera::formats::NV61,
> +	libcamera::formats::NV24,
> +	libcamera::formats::NV42,
> +	libcamera::formats::YUV420,
> +	libcamera::formats::YVU420
> +};
> +
> +ViewFinderGL::ViewFinderGL(QWidget *parent)
> +	: QOpenGLWidget(parent), buffer_(nullptr), yuvData_(nullptr),
> +	  fragmentShader_(nullptr), vertexShader_(nullptr),
> +	  vertexBuffer_(QOpenGLBuffer::VertexBuffer),
> +	  textureU_(QOpenGLTexture::Target2D),
> +	  textureV_(QOpenGLTexture::Target2D),
> +	  textureY_(QOpenGLTexture::Target2D)
> +{
> +}
> +
> +ViewFinderGL::~ViewFinderGL()
> +{
> +	removeShader();
> +
> +	if (vertexBuffer_.isCreated())
> +		vertexBuffer_.destroy();

I think the QOpenGLBuffer destructor destroys the buffer, you don't need
this.

> +}
> +
> +const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const
> +{
> +	return supportedFormats;
> +}
> +
> +int ViewFinderGL::setFormat(const libcamera::PixelFormat &format,
> +			    const QSize &size)
> +{
> +	int ret = 0;
> +
> +	/* If the fragment is ceeated remove it and create a new one */

s/ceeated/created/

> +	if (fragmentShader_) {
> +		if (shaderProgram_.isLinked()) {
> +			shaderProgram_.release();
> +			shaderProgram_.removeShader(fragmentShader_);
> +			delete fragmentShader_;
> +		}
> +	}
> +
> +	if (selectFormat(format)) {
> +		format_ = format;
> +		size_ = size;
> +	} else {
> +		ret = -1;
> +	}
> +	updateGeometry();
> +	return ret;

We tend to exit early in case of errors (and updateGeometry() shouldn't
be called in that case):

	if (!selectFormat(format))
		return -1;

	format_ = format;
	size_ = size;

	updateGeometry();
	return 0;

> +}
> +
> +void ViewFinderGL::stop()
> +{
> +	if (buffer_) {
> +		renderComplete(buffer_);
> +		buffer_ = nullptr;
> +	}
> +}
> +
> +QImage ViewFinderGL::getCurrentImage()
> +{
> +	QMutexLocker locker(&mutex_);
> +
> +	return grabFramebuffer();
> +}
> +
> +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)
> +{
> +	if (buffer->planes().size() != 1) {
> +		qWarning() << "Multi-planar buffers are not supported";
> +		return;
> +	}
> +
> +	if (buffer_)
> +		renderComplete(buffer_);
> +
> +	yuvData_ = static_cast<unsigned char *>(map->memory);
> +	update();
> +	buffer_ = buffer;
> +}
> +
> +bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)
> +{
> +	bool ret = true;
> +	switch (format) {
> +	case libcamera::formats::NV12:
> +		horzSubSample_ = 2;
> +		vertSubSample_ = 2;
> +		vertexShaderSrc_ = ":NV_vertex_shader.glsl";
> +		fragmentShaderSrc_ = ":NV_2_planes_UV_f.glsl";
> +		break;
> +	case libcamera::formats::NV21:
> +		horzSubSample_ = 2;
> +		vertSubSample_ = 2;
> +		vertexShaderSrc_ = ":NV_vertex_shader.glsl";
> +		fragmentShaderSrc_ = ":NV_2_planes_VU_f.glsl";
> +		break;
> +	case libcamera::formats::NV16:
> +		horzSubSample_ = 2;
> +		vertSubSample_ = 1;
> +		vertexShaderSrc_ = ":NV_vertex_shader.glsl";
> +		fragmentShaderSrc_ = ":NV_2_planes_UV_f.glsl";
> +		break;
> +	case libcamera::formats::NV61:
> +		horzSubSample_ = 2;
> +		vertSubSample_ = 1;
> +		vertexShaderSrc_ = ":NV_vertex_shader.glsl";
> +		fragmentShaderSrc_ = ":NV_2_planes_VU_f.glsl";
> +		break;
> +	case libcamera::formats::NV24:
> +		horzSubSample_ = 1;
> +		vertSubSample_ = 1;
> +		vertexShaderSrc_ = ":NV_vertex_shader.glsl";
> +		fragmentShaderSrc_ = ":NV_2_planes_UV_f.glsl";
> +		break;
> +	case libcamera::formats::NV42:
> +		horzSubSample_ = 1;
> +		vertSubSample_ = 1;
> +		vertexShaderSrc_ = ":NV_vertex_shader.glsl";
> +		fragmentShaderSrc_ = ":NV_2_planes_VU_f.glsl";
> +		break;
> +	case libcamera::formats::YUV420:
> +		horzSubSample_ = 2;
> +		vertSubSample_ = 2;
> +		vertexShaderSrc_ = ":NV_vertex_shader.glsl";
> +		fragmentShaderSrc_ = ":NV_3_planes_f.glsl";
> +		break;
> +	case libcamera::formats::YVU420:
> +		horzSubSample_ = 2;
> +		vertSubSample_ = 2;
> +		vertexShaderSrc_ = ":NV_vertex_shader.glsl";
> +		fragmentShaderSrc_ = ":NV_3_planes_f.glsl";
> +		break;
> +	default:
> +		ret = false;
> +		qWarning() << "[ViewFinderGL]:"
> +			   << "format not supported.";
> +		break;
> +	};
> +
> +	return ret;
> +}
> +
> +bool ViewFinderGL::createVertexShader()
> +{
> +	/* Create Vertex Shader */
> +	vertexShader_ = new QOpenGLShader(QOpenGLShader::Vertex, this);
> +
> +	/* Compile the vertex shader */
> +	if (!vertexShader_->compileSourceFile(vertexShaderSrc_)) {
> +		qWarning() << "[ViewFinderGL]:" << vertexShader_->log();
> +		return false;
> +	}
> +
> +	shaderProgram_.addShader(vertexShader_);
> +	return true;
> +}
> +
> +bool ViewFinderGL::createFragmentShader()
> +{
> +	int attributeVertex;
> +	int attributeTexture;
> +
> +	/* Create Fragment Shader */
> +	fragmentShader_ = new QOpenGLShader(QOpenGLShader::Fragment, this);
> +
> +	/* Compile the fragment shader */
> +	if (!fragmentShader_->compileSourceFile(fragmentShaderSrc_)) {
> +		qWarning() << "[ViewFinderGL]:" << fragmentShader_->log();
> +		return false;
> +	}
> +
> +	shaderProgram_.addShader(fragmentShader_);
> +
> +	/* Link shader pipeline */
> +	if (!shaderProgram_.link()) {
> +		qWarning() << "[ViewFinderGL]:" << shaderProgram_.log();
> +		close();
> +	}
> +
> +	/* Bind shader pipeline for use */
> +	if (!shaderProgram_.bind()) {
> +		qWarning() << "[ViewFinderGL]:" << shaderProgram_.log();
> +		close();
> +	}
> +
> +	attributeVertex = shaderProgram_.attributeLocation("vertexIn");
> +	attributeTexture = shaderProgram_.attributeLocation("textureIn");
> +
> +	shaderProgram_.enableAttributeArray(attributeVertex);
> +	shaderProgram_.setAttributeBuffer(attributeVertex,
> +					  GL_FLOAT,
> +					  0,
> +					  2,
> +					  2 * sizeof(GLfloat));
> +
> +	shaderProgram_.enableAttributeArray(attributeTexture);
> +	shaderProgram_.setAttributeBuffer(attributeTexture,
> +					  GL_FLOAT,
> +					  8 * sizeof(GLfloat),
> +					  2,
> +					  2 * sizeof(GLfloat));
> +
> +	textureUniformY_ = shaderProgram_.uniformLocation("tex_y");
> +	textureUniformU_ = shaderProgram_.uniformLocation("tex_u");
> +	textureUniformV_ = shaderProgram_.uniformLocation("tex_v");
> +
> +	if (!textureY_.isCreated())
> +		textureY_.create();
> +
> +	if (!textureU_.isCreated())
> +		textureU_.create();
> +
> +	if (!textureV_.isCreated())
> +		textureV_.create();
> +
> +	id_y_ = textureY_.textureId();
> +	id_u_ = textureU_.textureId();
> +	id_v_ = textureV_.textureId();
> +	return true;
> +}
> +
> +void ViewFinderGL::configureTexture(unsigned int id)
> +{
> +	glBindTexture(GL_TEXTURE_2D, id);
> +	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
> +	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);
> +	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);
> +	glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);
> +}
> +
> +void ViewFinderGL::removeShader()
> +{
> +	if (shaderProgram_.isLinked()) {
> +		shaderProgram_.release();
> +		shaderProgram_.removeAllShaders();
> +	}
> +
> +	if (fragmentShader_)
> +		delete fragmentShader_;
> +
> +	if (vertexShader_)
> +		delete vertexShader_;
> +}
> +
> +void ViewFinderGL::initializeGL()
> +{
> +	initializeOpenGLFunctions();
> +	glEnable(GL_TEXTURE_2D);
> +	glDisable(GL_DEPTH_TEST);
> +
> +	static const GLfloat coordinates[2][4][2]{
> +		{
> +			/* Vertex coordinates */
> +			{ -1.0f, -1.0f },
> +			{ -1.0f, +1.0f },
> +			{ +1.0f, +1.0f },
> +			{ +1.0f, -1.0f },
> +		},
> +		{
> +			/* Texture coordinates */
> +			{ 1.0f, 0.0f },
> +			{ 1.0f, 1.0f },
> +			{ 0.0f, 1.0f },
> +			{ 0.0f, 0.0f },

I *think* this should be

			{ 0.0f, 1.0f },
			{ 0.0f, 0.0f },
			{ 1.0f, 0.0f },
			{ 1.0f, 1.0f },

I'll test it and try to understand :-)

> +		},
> +	};
> +
> +	vertexBuffer_.create();
> +	vertexBuffer_.bind();
> +	vertexBuffer_.allocate(coordinates, sizeof(coordinates));
> +
> +	/* Create Vertex Shader */
> +	if (!createVertexShader())
> +		qWarning() << "[ViewFinderGL]: create vertex shader failed.";
> +
> +	glClearColor(1.0f, 1.0f, 1.0f, 0.0f);
> +}
> +
> +void ViewFinderGL::doRender()
> +{
> +	switch (format_) {
> +	case libcamera::formats::NV12:
> +	case libcamera::formats::NV21:
> +	case libcamera::formats::NV16:
> +	case libcamera::formats::NV61:
> +	case libcamera::formats::NV24:
> +	case libcamera::formats::NV42:
> +		/* activate texture Y */

s/activate/Activate/ (and similarly below).

> +		glActiveTexture(GL_TEXTURE0);
> +		configureTexture(id_y_);
> +		glTexImage2D(GL_TEXTURE_2D,
> +			     0,
> +			     GL_RED,
> +			     size_.width(),
> +			     size_.height(),
> +			     0,
> +			     GL_RED,
> +			     GL_UNSIGNED_BYTE,
> +			     yuvData_);
> +		shaderProgram_.setUniformValue(textureUniformY_, 0);
> +
> +		/* activate texture UV/VU */
> +		glActiveTexture(GL_TEXTURE1);
> +		configureTexture(id_u_);
> +		glTexImage2D(GL_TEXTURE_2D,
> +			     0,
> +			     GL_RG,
> +			     size_.width() / horzSubSample_,
> +			     size_.height() / vertSubSample_,
> +			     0,
> +			     GL_RG,
> +			     GL_UNSIGNED_BYTE,
> +			     (char *)yuvData_ + size_.width() * size_.height());
> +		shaderProgram_.setUniformValue(textureUniformU_, 1);
> +		break;
> +
> +	case libcamera::formats::YUV420:
> +		/* activate texture Y */
> +		glActiveTexture(GL_TEXTURE0);
> +		configureTexture(id_y_);
> +		glTexImage2D(GL_TEXTURE_2D,
> +			     0,
> +			     GL_RED,
> +			     size_.width(),
> +			     size_.height(),
> +			     0,
> +			     GL_RED,
> +			     GL_UNSIGNED_BYTE,
> +			     yuvData_);
> +		shaderProgram_.setUniformValue(textureUniformY_, 0);
> +
> +		/* activate texture U */
> +		glActiveTexture(GL_TEXTURE1);
> +		configureTexture(id_u_);
> +		glTexImage2D(GL_TEXTURE_2D,
> +			     0,
> +			     GL_RED,
> +			     size_.width() / horzSubSample_,
> +			     size_.height() / vertSubSample_,
> +			     0,
> +			     GL_RED,
> +			     GL_UNSIGNED_BYTE,
> +			     (char *)yuvData_ + size_.width() * size_.height());
> +		shaderProgram_.setUniformValue(textureUniformU_, 1);
> +
> +		/* activate texture V */
> +		glActiveTexture(GL_TEXTURE2);
> +		configureTexture(id_v_);
> +		glTexImage2D(GL_TEXTURE_2D,
> +			     0,
> +			     GL_RED,
> +			     size_.width() / horzSubSample_,
> +			     size_.height() / vertSubSample_,
> +			     0,
> +			     GL_RED,
> +			     GL_UNSIGNED_BYTE,
> +			     (char *)yuvData_ + size_.width() * size_.height() * 5 / 4);
> +		shaderProgram_.setUniformValue(textureUniformV_, 2);
> +		break;
> +
> +	case libcamera::formats::YVU420:
> +		/* activate texture Y */
> +		glActiveTexture(GL_TEXTURE0);
> +		configureTexture(id_y_);
> +		glTexImage2D(GL_TEXTURE_2D,
> +			     0,
> +			     GL_RED,
> +			     size_.width(),
> +			     size_.height(),
> +			     0,
> +			     GL_RED,
> +			     GL_UNSIGNED_BYTE,
> +			     yuvData_);
> +		shaderProgram_.setUniformValue(textureUniformY_, 0);
> +
> +		/* activate texture V */
> +		glActiveTexture(GL_TEXTURE2);
> +		configureTexture(id_v_);
> +		glTexImage2D(GL_TEXTURE_2D,
> +			     0,
> +			     GL_RED,
> +			     size_.width() / horzSubSample_,
> +			     size_.height() / vertSubSample_,
> +			     0,
> +			     GL_RED,
> +			     GL_UNSIGNED_BYTE,
> +			     (char *)yuvData_ + size_.width() * size_.height());
> +		shaderProgram_.setUniformValue(textureUniformV_, 1);

I don't think this is correct. GL_TEXTURE1 stores the U plane, and you
assign it to tex_v, which is the V plane in the shader.

There are two options, either s/1/2/ or s/GL_TEXTURE2/GL_TEXTURE1/ (and
the other way below).

With these small issues addressed,

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

I can also fix these when applying the patch, but given that there are
quite a few issues, I would then send a v7 to the list to make sure I
haven't done anything wrong.

> +
> +		/* activate texture U */
> +		glActiveTexture(GL_TEXTURE1);
> +		configureTexture(id_u_);
> +		glTexImage2D(GL_TEXTURE_2D,
> +			     0,
> +			     GL_RED,
> +			     size_.width() / horzSubSample_,
> +			     size_.height() / vertSubSample_,
> +			     0,
> +			     GL_RED,
> +			     GL_UNSIGNED_BYTE,
> +			     (char *)yuvData_ + size_.width() * size_.height() * 5 / 4);
> +		shaderProgram_.setUniformValue(textureUniformU_, 2);
> +		break;
> +
> +	default:
> +		break;
> +	};
> +}
> +
> +void ViewFinderGL::paintGL()
> +{
> +	if (!fragmentShader_)
> +		if (!createFragmentShader()) {
> +			qWarning() << "[ViewFinderGL]:"
> +				   << "create fragment shader failed.";
> +		}
> +
> +	if (yuvData_) {
> +		glClearColor(0.0, 0.0, 0.0, 1.0);
> +		glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);
> +
> +		doRender();
> +		glDrawArrays(GL_TRIANGLE_FAN, 0, 4);
> +	}
> +}
> +
> +void ViewFinderGL::resizeGL(int w, int h)
> +{
> +	glViewport(0, 0, w, h);
> +}
> +
> +QSize ViewFinderGL::sizeHint() const
> +{
> +	return size_.isValid() ? size_ : QSize(640, 480);
> +}
> diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h
> new file mode 100644
> index 0000000..69502b7
> --- /dev/null
> +++ b/src/qcam/viewfinder_gl.h
> @@ -0,0 +1,96 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2020, Linaro
> + *
> + * viewfinder_GL.h - OpenGL Viewfinder for rendering by OpenGL shader
> + *
> + */
> +#ifndef __VIEWFINDER_GL_H__
> +#define __VIEWFINDER_GL_H__
> +
> +#include <QImage>
> +#include <QMutex>
> +#include <QOpenGLBuffer>
> +#include <QOpenGLFunctions>
> +#include <QOpenGLShader>
> +#include <QOpenGLShaderProgram>
> +#include <QOpenGLTexture>
> +#include <QOpenGLWidget>
> +#include <QSize>
> +
> +#include <libcamera/buffer.h>
> +#include <libcamera/formats.h>
> +
> +#include "viewfinder.h"
> +
> +class ViewFinderGL : public QOpenGLWidget,
> +		     public ViewFinder,
> +		     protected QOpenGLFunctions
> +{
> +	Q_OBJECT
> +
> +public:
> +	ViewFinderGL(QWidget *parent = nullptr);
> +	~ViewFinderGL();
> +
> +	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 stop() override;
> +
> +	QImage getCurrentImage() override;
> +
> +Q_SIGNALS:
> +	void renderComplete(libcamera::FrameBuffer *buffer);
> +
> +protected:
> +	void initializeGL() override;
> +	void paintGL() override;
> +	void resizeGL(int w, int h) override;
> +	QSize sizeHint() const override;
> +
> +private:
> +	bool selectFormat(const libcamera::PixelFormat &format);
> +
> +	void configureTexture(unsigned int id);
> +	bool createFragmentShader();
> +	bool createVertexShader();
> +	void removeShader();
> +	void doRender();
> +
> +	/* Captured image size, format and buffer */
> +	libcamera::FrameBuffer *buffer_;
> +	libcamera::PixelFormat format_;
> +	QSize size_;
> +	unsigned char *yuvData_;
> +
> +	/* OpenGL components for rendering */
> +	QOpenGLShader *fragmentShader_;
> +	QOpenGLShader *vertexShader_;
> +	QOpenGLShaderProgram shaderProgram_;
> +
> +	/* Vertex buffer */
> +	QOpenGLBuffer vertexBuffer_;
> +
> +	/* Fragment and Vertex shader file name */
> +	QString fragmentShaderSrc_;
> +	QString vertexShaderSrc_;
> +
> +	/* YUV texture planars and parameters */
> +	GLuint id_u_;
> +	GLuint id_v_;
> +	GLuint id_y_;
> +	GLuint textureUniformU_;
> +	GLuint textureUniformV_;
> +	GLuint textureUniformY_;
> +	QOpenGLTexture textureU_;
> +	QOpenGLTexture textureV_;
> +	QOpenGLTexture textureY_;
> +	unsigned int horzSubSample_;
> +	unsigned int vertSubSample_;
> +
> +	QMutex mutex_; /* Prevent concurrent access to image_ */
> +};
> +
> +#endif /* __VIEWFINDER_GL_H__ */

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list