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

Message ID 20200911085514.30021-4-show.liu@linaro.org
State Accepted
Headers show
Series
  • qcam: accelerate format conversion by OpenGL shader
Related show

Commit Message

Show Liu Sept. 11, 2020, 8:55 a.m. UTC
the viewfinderGL accelerates the format conversion by
using OpenGL ES shader

Signed-off-by: Show Liu <show.liu@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

Comments

Laurent Pinchart Sept. 12, 2020, 2:09 a.m. UTC | #1
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@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@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__ */
Laurent Pinchart Sept. 12, 2020, 10:03 p.m. UTC | #2
Hi Show,

On Sat, Sep 12, 2020 at 05:09:10AM +0300, Laurent Pinchart wrote:
> 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@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 :-)

I confirm that the original patch rotates the image by 180° for me,
while the proposed values above show it in the right orientation. I've
compared -r qt and -r gles to check that.

> > +		},
> > +	};
> > +
> > +	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@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__ */
Show Liu Sept. 14, 2020, 1:58 a.m. UTC | #3
Hi Laurent,



On Sun, Sep 13, 2020 at 6:03 AM Laurent Pinchart <
laurent.pinchart@ideasonboard.com> wrote:

> Hi Show,
>
> On Sat, Sep 12, 2020 at 05:09:10AM +0300, Laurent Pinchart wrote:
> > 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@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 :-)
>
> I confirm that the original patch rotates the image by 180° for me,
> while the proposed values above show it in the right orientation. I've
> compared -r qt and -r gles to check that.
>
Are you going to apply above value to the v7 too? If so I will not send out
the patch to fix it.

Thanks,
Show


>
> > > +           },
> > > +   };
> > > +
> > > +   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@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
>
Laurent Pinchart Sept. 15, 2020, 2:21 a.m. UTC | #4
Hi Show,

On Mon, Sep 14, 2020 at 09:58:42AM +0800, Show Liu wrote:
> On Sun, Sep 13, 2020 at 6:03 AM Laurent Pinchart wrote:
> > On Sat, Sep 12, 2020 at 05:09:10AM +0300, Laurent Pinchart wrote:
> > > 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@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 :-)
> >
> > I confirm that the original patch rotates the image by 180° for me,
> > while the proposed values above show it in the right orientation. I've
> > compared -r qt and -r gles to check that.
>
> Are you going to apply above value to the v7 too? If so I will not send out
> the patch to fix it.

Yes. I'll send the v7 shortly.

> > > > +           },
> > > > +   };
> > > > +
> > > > +   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@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__ */

Patch

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();
+}
+
+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 */
+	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;
+}
+
+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 },
+		},
+	};
+
+	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 */
+		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);
+
+		/* 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__ */