[{"id":12466,"web_url":"https://patchwork.libcamera.org/comment/12466/","msgid":"<20200912020910.GM6808@pendragon.ideasonboard.com>","date":"2020-09-12T02:09:10","subject":"Re: [libcamera-devel] [PATCH v6 3/4] qcam: add viewfinderGL class\n\tto accelerate the format convert","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Show,\n\nThank you for the patch.\n\nIn the subject line, s/viewfinderGL/ViewFinderGL/ and\ns/convert/conversion/\n\nOn Fri, Sep 11, 2020 at 04:55:13PM +0800, Show Liu wrote:\n> the viewfinderGL accelerates the format conversion by\n> using OpenGL ES shader\n\nI would add\n\n\"The minimum Qt version is bumped to v5.4, as QOpenGLWidget wasn't\navailable before that.\"\n\n> Signed-off-by: Show Liu \n> ---\n> meson.build | 1 +\n> src/qcam/meson.build | 17 +-\n> src/qcam/viewfinder_gl.cpp | 456 +++++++++++++++++++++++++++++++++++++\n> src/qcam/viewfinder_gl.h | 96 ++++++++\n> 4 files changed, 568 insertions(+), 2 deletions(-)\n> create mode 100644 src/qcam/viewfinder_gl.cpp\n> create mode 100644 src/qcam/viewfinder_gl.h\n> \n> diff --git a/meson.build b/meson.build\n> index 1ea35e9..c58d458 100644\n> --- a/meson.build\n> +++ b/meson.build\n> @@ -26,6 +26,7 @@ libcamera_version = libcamera_git_version.split('+')[0]\n> \n> # Configure the build environment.\n> cc = meson.get_compiler('c')\n> +cxx = meson.get_compiler('cpp')\n> config_h = configuration_data()\n> \n> if cc.has_header_symbol('execinfo.h', 'backtrace')\n> diff --git a/src/qcam/meson.build b/src/qcam/meson.build\n> index a4bad0a..9bb48c0 100644\n> --- a/src/qcam/meson.build\n> +++ b/src/qcam/meson.build\n> @@ -16,14 +16,14 @@ qcam_moc_headers = files([\n> \n> qcam_resources = files([\n> 'assets/feathericons/feathericons.qrc',\n> - 'assets/shader/shaders.qrc'\n> ])\n> \n> qt5 = import('qt5')\n> qt5_dep = dependency('qt5',\n> method : 'pkg-config',\n> modules : ['Core', 'Gui', 'Widgets'],\n> - required : get_option('qcam'))\n> + required : get_option('qcam'),\n> + version : '>=5.4')\n> \n> if qt5_dep.found()\n> qcam_deps = [\n> @@ -42,6 +42,19 @@ if qt5_dep.found()\n> ])\n> endif\n> \n> + if cxx.has_header_symbol('QOpenGLWidget', 'QOpenGLWidget',\n> + dependencies : qt5_dep, args : '-fPIC')\n> + qcam_sources += files([\n> + 'viewfinder_gl.cpp',\n> + ])\n> + qcam_moc_headers += files([\n> + 'viewfinder_gl.h',\n> + ])\n> + qcam_resources += files([\n> + 'assets/shader/shaders.qrc'\n> + ])\n> + endif\n> +\n> # gcc 9 introduced a deprecated-copy warning that is triggered by Qt until\n> # Qt 5.13. clang 10 introduced the same warning, but detects more issues\n> # that are not fixed in Qt yet. Disable the warning manually in both cases.\n> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> new file mode 100644\n> index 0000000..84f4866\n> --- /dev/null\n> +++ b/src/qcam/viewfinder_gl.cpp\n> @@ -0,0 +1,456 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Linaro\n> + *\n> + * viewfinderGL.cpp - OpenGL Viewfinder for rendering by OpenGL shader\n> + */\n> +\n> +#include \"viewfinder_gl.h\"\n> +\n> +#include \n> +\n> +#include \n> +\n> +static const QList supportedFormats{\n> +\tlibcamera::formats::NV12,\n> +\tlibcamera::formats::NV21,\n> +\tlibcamera::formats::NV16,\n> +\tlibcamera::formats::NV61,\n> +\tlibcamera::formats::NV24,\n> +\tlibcamera::formats::NV42,\n> +\tlibcamera::formats::YUV420,\n> +\tlibcamera::formats::YVU420\n> +};\n> +\n> +ViewFinderGL::ViewFinderGL(QWidget *parent)\n> +\t: QOpenGLWidget(parent), buffer_(nullptr), yuvData_(nullptr),\n> +\t fragmentShader_(nullptr), vertexShader_(nullptr),\n> +\t vertexBuffer_(QOpenGLBuffer::VertexBuffer),\n> +\t textureU_(QOpenGLTexture::Target2D),\n> +\t textureV_(QOpenGLTexture::Target2D),\n> +\t textureY_(QOpenGLTexture::Target2D)\n> +{\n> +}\n> +\n> +ViewFinderGL::~ViewFinderGL()\n> +{\n> +\tremoveShader();\n> +\n> +\tif (vertexBuffer_.isCreated())\n> +\t\tvertexBuffer_.destroy();\n\nI think the QOpenGLBuffer destructor destroys the buffer, you don't need\nthis.\n\n> +}\n> +\n> +const QList &ViewFinderGL::nativeFormats() const\n> +{\n> +\treturn supportedFormats;\n> +}\n> +\n> +int ViewFinderGL::setFormat(const libcamera::PixelFormat &format,\n> +\t\t\t const QSize &size)\n> +{\n> +\tint ret = 0;\n> +\n> +\t/* If the fragment is ceeated remove it and create a new one */\n\ns/ceeated/created/\n\n> +\tif (fragmentShader_) {\n> +\t\tif (shaderProgram_.isLinked()) {\n> +\t\t\tshaderProgram_.release();\n> +\t\t\tshaderProgram_.removeShader(fragmentShader_);\n> +\t\t\tdelete fragmentShader_;\n> +\t\t}\n> +\t}\n> +\n> +\tif (selectFormat(format)) {\n> +\t\tformat_ = format;\n> +\t\tsize_ = size;\n> +\t} else {\n> +\t\tret = -1;\n> +\t}\n> +\tupdateGeometry();\n> +\treturn ret;\n\nWe tend to exit early in case of errors (and updateGeometry() shouldn't\nbe called in that case):\n\n\tif (!selectFormat(format))\n\t\treturn -1;\n\n\tformat_ = format;\n\tsize_ = size;\n\n\tupdateGeometry();\n\treturn 0;\n\n> +}\n> +\n> +void ViewFinderGL::stop()\n> +{\n> +\tif (buffer_) {\n> +\t\trenderComplete(buffer_);\n> +\t\tbuffer_ = nullptr;\n> +\t}\n> +}\n> +\n> +QImage ViewFinderGL::getCurrentImage()\n> +{\n> +\tQMutexLocker locker(&mutex_);\n> +\n> +\treturn grabFramebuffer();\n> +}\n> +\n> +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> +{\n> +\tif (buffer->planes().size() != 1) {\n> +\t\tqWarning() << \"Multi-planar buffers are not supported\";\n> +\t\treturn;\n> +\t}\n> +\n> +\tif (buffer_)\n> +\t\trenderComplete(buffer_);\n> +\n> +\tyuvData_ = static_cast(map->memory);\n> +\tupdate();\n> +\tbuffer_ = buffer;\n> +}\n> +\n> +bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)\n> +{\n> +\tbool ret = true;\n> +\tswitch (format) {\n> +\tcase libcamera::formats::NV12:\n> +\t\thorzSubSample_ = 2;\n> +\t\tvertSubSample_ = 2;\n> +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> +\t\tfragmentShaderSrc_ = \":NV_2_planes_UV_f.glsl\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::NV21:\n> +\t\thorzSubSample_ = 2;\n> +\t\tvertSubSample_ = 2;\n> +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> +\t\tfragmentShaderSrc_ = \":NV_2_planes_VU_f.glsl\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::NV16:\n> +\t\thorzSubSample_ = 2;\n> +\t\tvertSubSample_ = 1;\n> +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> +\t\tfragmentShaderSrc_ = \":NV_2_planes_UV_f.glsl\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::NV61:\n> +\t\thorzSubSample_ = 2;\n> +\t\tvertSubSample_ = 1;\n> +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> +\t\tfragmentShaderSrc_ = \":NV_2_planes_VU_f.glsl\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::NV24:\n> +\t\thorzSubSample_ = 1;\n> +\t\tvertSubSample_ = 1;\n> +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> +\t\tfragmentShaderSrc_ = \":NV_2_planes_UV_f.glsl\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::NV42:\n> +\t\thorzSubSample_ = 1;\n> +\t\tvertSubSample_ = 1;\n> +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> +\t\tfragmentShaderSrc_ = \":NV_2_planes_VU_f.glsl\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::YUV420:\n> +\t\thorzSubSample_ = 2;\n> +\t\tvertSubSample_ = 2;\n> +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> +\t\tfragmentShaderSrc_ = \":NV_3_planes_f.glsl\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::YVU420:\n> +\t\thorzSubSample_ = 2;\n> +\t\tvertSubSample_ = 2;\n> +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> +\t\tfragmentShaderSrc_ = \":NV_3_planes_f.glsl\";\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tret = false;\n> +\t\tqWarning() << \"[ViewFinderGL]:\"\n> +\t\t\t << \"format not supported.\";\n> +\t\tbreak;\n> +\t};\n> +\n> +\treturn ret;\n> +}\n> +\n> +bool ViewFinderGL::createVertexShader()\n> +{\n> +\t/* Create Vertex Shader */\n> +\tvertexShader_ = new QOpenGLShader(QOpenGLShader::Vertex, this);\n> +\n> +\t/* Compile the vertex shader */\n> +\tif (!vertexShader_->compileSourceFile(vertexShaderSrc_)) {\n> +\t\tqWarning() << \"[ViewFinderGL]:\" << vertexShader_->log();\n> +\t\treturn false;\n> +\t}\n> +\n> +\tshaderProgram_.addShader(vertexShader_);\n> +\treturn true;\n> +}\n> +\n> +bool ViewFinderGL::createFragmentShader()\n> +{\n> +\tint attributeVertex;\n> +\tint attributeTexture;\n> +\n> +\t/* Create Fragment Shader */\n> +\tfragmentShader_ = new QOpenGLShader(QOpenGLShader::Fragment, this);\n> +\n> +\t/* Compile the fragment shader */\n> +\tif (!fragmentShader_->compileSourceFile(fragmentShaderSrc_)) {\n> +\t\tqWarning() << \"[ViewFinderGL]:\" << fragmentShader_->log();\n> +\t\treturn false;\n> +\t}\n> +\n> +\tshaderProgram_.addShader(fragmentShader_);\n> +\n> +\t/* Link shader pipeline */\n> +\tif (!shaderProgram_.link()) {\n> +\t\tqWarning() << \"[ViewFinderGL]:\" << shaderProgram_.log();\n> +\t\tclose();\n> +\t}\n> +\n> +\t/* Bind shader pipeline for use */\n> +\tif (!shaderProgram_.bind()) {\n> +\t\tqWarning() << \"[ViewFinderGL]:\" << shaderProgram_.log();\n> +\t\tclose();\n> +\t}\n> +\n> +\tattributeVertex = shaderProgram_.attributeLocation(\"vertexIn\");\n> +\tattributeTexture = shaderProgram_.attributeLocation(\"textureIn\");\n> +\n> +\tshaderProgram_.enableAttributeArray(attributeVertex);\n> +\tshaderProgram_.setAttributeBuffer(attributeVertex,\n> +\t\t\t\t\t GL_FLOAT,\n> +\t\t\t\t\t 0,\n> +\t\t\t\t\t 2,\n> +\t\t\t\t\t 2 * sizeof(GLfloat));\n> +\n> +\tshaderProgram_.enableAttributeArray(attributeTexture);\n> +\tshaderProgram_.setAttributeBuffer(attributeTexture,\n> +\t\t\t\t\t GL_FLOAT,\n> +\t\t\t\t\t 8 * sizeof(GLfloat),\n> +\t\t\t\t\t 2,\n> +\t\t\t\t\t 2 * sizeof(GLfloat));\n> +\n> +\ttextureUniformY_ = shaderProgram_.uniformLocation(\"tex_y\");\n> +\ttextureUniformU_ = shaderProgram_.uniformLocation(\"tex_u\");\n> +\ttextureUniformV_ = shaderProgram_.uniformLocation(\"tex_v\");\n> +\n> +\tif (!textureY_.isCreated())\n> +\t\ttextureY_.create();\n> +\n> +\tif (!textureU_.isCreated())\n> +\t\ttextureU_.create();\n> +\n> +\tif (!textureV_.isCreated())\n> +\t\ttextureV_.create();\n> +\n> +\tid_y_ = textureY_.textureId();\n> +\tid_u_ = textureU_.textureId();\n> +\tid_v_ = textureV_.textureId();\n> +\treturn true;\n> +}\n> +\n> +void ViewFinderGL::configureTexture(unsigned int id)\n> +{\n> +\tglBindTexture(GL_TEXTURE_2D, id);\n> +\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);\n> +\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);\n> +\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);\n> +\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);\n> +}\n> +\n> +void ViewFinderGL::removeShader()\n> +{\n> +\tif (shaderProgram_.isLinked()) {\n> +\t\tshaderProgram_.release();\n> +\t\tshaderProgram_.removeAllShaders();\n> +\t}\n> +\n> +\tif (fragmentShader_)\n> +\t\tdelete fragmentShader_;\n> +\n> +\tif (vertexShader_)\n> +\t\tdelete vertexShader_;\n> +}\n> +\n> +void ViewFinderGL::initializeGL()\n> +{\n> +\tinitializeOpenGLFunctions();\n> +\tglEnable(GL_TEXTURE_2D);\n> +\tglDisable(GL_DEPTH_TEST);\n> +\n> +\tstatic const GLfloat coordinates[2][4][2]{\n> +\t\t{\n> +\t\t\t/* Vertex coordinates */\n> +\t\t\t{ -1.0f, -1.0f },\n> +\t\t\t{ -1.0f, +1.0f },\n> +\t\t\t{ +1.0f, +1.0f },\n> +\t\t\t{ +1.0f, -1.0f },\n> +\t\t},\n> +\t\t{\n> +\t\t\t/* Texture coordinates */\n> +\t\t\t{ 1.0f, 0.0f },\n> +\t\t\t{ 1.0f, 1.0f },\n> +\t\t\t{ 0.0f, 1.0f },\n> +\t\t\t{ 0.0f, 0.0f },\n\nI *think* this should be\n\n\t\t\t{ 0.0f, 1.0f },\n\t\t\t{ 0.0f, 0.0f },\n\t\t\t{ 1.0f, 0.0f },\n\t\t\t{ 1.0f, 1.0f },\n\nI'll test it and try to understand :-)\n\n> +\t\t},\n> +\t};\n> +\n> +\tvertexBuffer_.create();\n> +\tvertexBuffer_.bind();\n> +\tvertexBuffer_.allocate(coordinates, sizeof(coordinates));\n> +\n> +\t/* Create Vertex Shader */\n> +\tif (!createVertexShader())\n> +\t\tqWarning() << \"[ViewFinderGL]: create vertex shader failed.\";\n> +\n> +\tglClearColor(1.0f, 1.0f, 1.0f, 0.0f);\n> +}\n> +\n> +void ViewFinderGL::doRender()\n> +{\n> +\tswitch (format_) {\n> +\tcase libcamera::formats::NV12:\n> +\tcase libcamera::formats::NV21:\n> +\tcase libcamera::formats::NV16:\n> +\tcase libcamera::formats::NV61:\n> +\tcase libcamera::formats::NV24:\n> +\tcase libcamera::formats::NV42:\n> +\t\t/* activate texture Y */\n\ns/activate/Activate/ (and similarly below).\n\n> +\t\tglActiveTexture(GL_TEXTURE0);\n> +\t\tconfigureTexture(id_y_);\n> +\t\tglTexImage2D(GL_TEXTURE_2D,\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t size_.width(),\n> +\t\t\t size_.height(),\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t GL_UNSIGNED_BYTE,\n> +\t\t\t yuvData_);\n> +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n> +\n> +\t\t/* activate texture UV/VU */\n> +\t\tglActiveTexture(GL_TEXTURE1);\n> +\t\tconfigureTexture(id_u_);\n> +\t\tglTexImage2D(GL_TEXTURE_2D,\n> +\t\t\t 0,\n> +\t\t\t GL_RG,\n> +\t\t\t size_.width() / horzSubSample_,\n> +\t\t\t size_.height() / vertSubSample_,\n> +\t\t\t 0,\n> +\t\t\t GL_RG,\n> +\t\t\t GL_UNSIGNED_BYTE,\n> +\t\t\t (char *)yuvData_ + size_.width() * size_.height());\n> +\t\tshaderProgram_.setUniformValue(textureUniformU_, 1);\n> +\t\tbreak;\n> +\n> +\tcase libcamera::formats::YUV420:\n> +\t\t/* activate texture Y */\n> +\t\tglActiveTexture(GL_TEXTURE0);\n> +\t\tconfigureTexture(id_y_);\n> +\t\tglTexImage2D(GL_TEXTURE_2D,\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t size_.width(),\n> +\t\t\t size_.height(),\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t GL_UNSIGNED_BYTE,\n> +\t\t\t yuvData_);\n> +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n> +\n> +\t\t/* activate texture U */\n> +\t\tglActiveTexture(GL_TEXTURE1);\n> +\t\tconfigureTexture(id_u_);\n> +\t\tglTexImage2D(GL_TEXTURE_2D,\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t size_.width() / horzSubSample_,\n> +\t\t\t size_.height() / vertSubSample_,\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t GL_UNSIGNED_BYTE,\n> +\t\t\t (char *)yuvData_ + size_.width() * size_.height());\n> +\t\tshaderProgram_.setUniformValue(textureUniformU_, 1);\n> +\n> +\t\t/* activate texture V */\n> +\t\tglActiveTexture(GL_TEXTURE2);\n> +\t\tconfigureTexture(id_v_);\n> +\t\tglTexImage2D(GL_TEXTURE_2D,\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t size_.width() / horzSubSample_,\n> +\t\t\t size_.height() / vertSubSample_,\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t GL_UNSIGNED_BYTE,\n> +\t\t\t (char *)yuvData_ + size_.width() * size_.height() * 5 / 4);\n> +\t\tshaderProgram_.setUniformValue(textureUniformV_, 2);\n> +\t\tbreak;\n> +\n> +\tcase libcamera::formats::YVU420:\n> +\t\t/* activate texture Y */\n> +\t\tglActiveTexture(GL_TEXTURE0);\n> +\t\tconfigureTexture(id_y_);\n> +\t\tglTexImage2D(GL_TEXTURE_2D,\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t size_.width(),\n> +\t\t\t size_.height(),\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t GL_UNSIGNED_BYTE,\n> +\t\t\t yuvData_);\n> +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n> +\n> +\t\t/* activate texture V */\n> +\t\tglActiveTexture(GL_TEXTURE2);\n> +\t\tconfigureTexture(id_v_);\n> +\t\tglTexImage2D(GL_TEXTURE_2D,\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t size_.width() / horzSubSample_,\n> +\t\t\t size_.height() / vertSubSample_,\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t GL_UNSIGNED_BYTE,\n> +\t\t\t (char *)yuvData_ + size_.width() * size_.height());\n> +\t\tshaderProgram_.setUniformValue(textureUniformV_, 1);\n\nI don't think this is correct. GL_TEXTURE1 stores the U plane, and you\nassign it to tex_v, which is the V plane in the shader.\n\nThere are two options, either s/1/2/ or s/GL_TEXTURE2/GL_TEXTURE1/ (and\nthe other way below).\n\nWith these small issues addressed,\n\nReviewed-by: Laurent Pinchart \n\nI can also fix these when applying the patch, but given that there are\nquite a few issues, I would then send a v7 to the list to make sure I\nhaven't done anything wrong.\n\n> +\n> +\t\t/* activate texture U */\n> +\t\tglActiveTexture(GL_TEXTURE1);\n> +\t\tconfigureTexture(id_u_);\n> +\t\tglTexImage2D(GL_TEXTURE_2D,\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t size_.width() / horzSubSample_,\n> +\t\t\t size_.height() / vertSubSample_,\n> +\t\t\t 0,\n> +\t\t\t GL_RED,\n> +\t\t\t GL_UNSIGNED_BYTE,\n> +\t\t\t (char *)yuvData_ + size_.width() * size_.height() * 5 / 4);\n> +\t\tshaderProgram_.setUniformValue(textureUniformU_, 2);\n> +\t\tbreak;\n> +\n> +\tdefault:\n> +\t\tbreak;\n> +\t};\n> +}\n> +\n> +void ViewFinderGL::paintGL()\n> +{\n> +\tif (!fragmentShader_)\n> +\t\tif (!createFragmentShader()) {\n> +\t\t\tqWarning() << \"[ViewFinderGL]:\"\n> +\t\t\t\t << \"create fragment shader failed.\";\n> +\t\t}\n> +\n> +\tif (yuvData_) {\n> +\t\tglClearColor(0.0, 0.0, 0.0, 1.0);\n> +\t\tglClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);\n> +\n> +\t\tdoRender();\n> +\t\tglDrawArrays(GL_TRIANGLE_FAN, 0, 4);\n> +\t}\n> +}\n> +\n> +void ViewFinderGL::resizeGL(int w, int h)\n> +{\n> +\tglViewport(0, 0, w, h);\n> +}\n> +\n> +QSize ViewFinderGL::sizeHint() const\n> +{\n> +\treturn size_.isValid() ? size_ : QSize(640, 480);\n> +}\n> diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h\n> new file mode 100644\n> index 0000000..69502b7\n> --- /dev/null\n> +++ b/src/qcam/viewfinder_gl.h\n> @@ -0,0 +1,96 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Linaro\n> + *\n> + * viewfinder_GL.h - OpenGL Viewfinder for rendering by OpenGL shader\n> + *\n> + */\n> +#ifndef __VIEWFINDER_GL_H__\n> +#define __VIEWFINDER_GL_H__\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"viewfinder.h\"\n> +\n> +class ViewFinderGL : public QOpenGLWidget,\n> +\t\t public ViewFinder,\n> +\t\t protected QOpenGLFunctions\n> +{\n> +\tQ_OBJECT\n> +\n> +public:\n> +\tViewFinderGL(QWidget *parent = nullptr);\n> +\t~ViewFinderGL();\n> +\n> +\tconst QList &nativeFormats() const override;\n> +\n> +\tint setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> +\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> +\tvoid stop() override;\n> +\n> +\tQImage getCurrentImage() override;\n> +\n> +Q_SIGNALS:\n> +\tvoid renderComplete(libcamera::FrameBuffer *buffer);\n> +\n> +protected:\n> +\tvoid initializeGL() override;\n> +\tvoid paintGL() override;\n> +\tvoid resizeGL(int w, int h) override;\n> +\tQSize sizeHint() const override;\n> +\n> +private:\n> +\tbool selectFormat(const libcamera::PixelFormat &format);\n> +\n> +\tvoid configureTexture(unsigned int id);\n> +\tbool createFragmentShader();\n> +\tbool createVertexShader();\n> +\tvoid removeShader();\n> +\tvoid doRender();\n> +\n> +\t/* Captured image size, format and buffer */\n> +\tlibcamera::FrameBuffer *buffer_;\n> +\tlibcamera::PixelFormat format_;\n> +\tQSize size_;\n> +\tunsigned char *yuvData_;\n> +\n> +\t/* OpenGL components for rendering */\n> +\tQOpenGLShader *fragmentShader_;\n> +\tQOpenGLShader *vertexShader_;\n> +\tQOpenGLShaderProgram shaderProgram_;\n> +\n> +\t/* Vertex buffer */\n> +\tQOpenGLBuffer vertexBuffer_;\n> +\n> +\t/* Fragment and Vertex shader file name */\n> +\tQString fragmentShaderSrc_;\n> +\tQString vertexShaderSrc_;\n> +\n> +\t/* YUV texture planars and parameters */\n> +\tGLuint id_u_;\n> +\tGLuint id_v_;\n> +\tGLuint id_y_;\n> +\tGLuint textureUniformU_;\n> +\tGLuint textureUniformV_;\n> +\tGLuint textureUniformY_;\n> +\tQOpenGLTexture textureU_;\n> +\tQOpenGLTexture textureV_;\n> +\tQOpenGLTexture textureY_;\n> +\tunsigned int horzSubSample_;\n> +\tunsigned int vertSubSample_;\n> +\n> +\tQMutex mutex_; /* Prevent concurrent access to image_ */\n> +};\n> +\n> +#endif /* __VIEWFINDER_GL_H__ */","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id D83CCC3B5B\n\tfor ;\n\tSat, 12 Sep 2020 02:09:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 69D7862DA1;\n\tSat, 12 Sep 2020 04:09:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C18B962C43\n\tfor ;\n\tSat, 12 Sep 2020 04:09:39 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2FE76276;\n\tSat, 12 Sep 2020 04:09:39 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"b/onhqu8\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599876579;\n\tbh=Epiuh8d4iB0YnVS+hCRmqX34C3cRVlWnjvAzNxFinbE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=b/onhqu8Oug380ebFDntyMek5FXXQQNwCXSn1oJKbnsaj+t87wpNv71XrSMcOMa0J\n\t67x/1a9I+qrEuWwUkgFdvfMKdfUh+FHScKh7wQ2orrpMiMEvofjsPWUE+ziwgdNMyR\n\tXC5vS0Go4BM6I+m65RhJ2voAB5Ao5PlIlgL5/bhM=","Date":"Sat, 12 Sep 2020 05:09:10 +0300","From":"Laurent Pinchart ","To":"Show Liu ","Message-ID":"<20200912020910.GM6808@pendragon.ideasonboard.com>","References":"<20200911085514.30021-1-show.liu@linaro.org>\n\t<20200911085514.30021-4-show.liu@linaro.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200911085514.30021-4-show.liu@linaro.org>","Subject":"Re: [libcamera-devel] [PATCH v6 3/4] qcam: add viewfinderGL class\n\tto accelerate the format convert","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12477,"web_url":"https://patchwork.libcamera.org/comment/12477/","msgid":"<20200912220315.GC3933@pendragon.ideasonboard.com>","date":"2020-09-12T22:03:15","subject":"Re: [libcamera-devel] [PATCH v6 3/4] qcam: add viewfinderGL class\n\tto accelerate the format convert","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Show,\n\nOn Sat, Sep 12, 2020 at 05:09:10AM +0300, Laurent Pinchart wrote:\n> Hi Show,\n> \n> Thank you for the patch.\n> \n> In the subject line, s/viewfinderGL/ViewFinderGL/ and\n> s/convert/conversion/\n> \n> On Fri, Sep 11, 2020 at 04:55:13PM +0800, Show Liu wrote:\n> > the viewfinderGL accelerates the format conversion by\n> > using OpenGL ES shader\n> \n> I would add\n> \n> \"The minimum Qt version is bumped to v5.4, as QOpenGLWidget wasn't\n> available before that.\"\n> \n> > Signed-off-by: Show Liu \n> > ---\n> > meson.build | 1 +\n> > src/qcam/meson.build | 17 +-\n> > src/qcam/viewfinder_gl.cpp | 456 +++++++++++++++++++++++++++++++++++++\n> > src/qcam/viewfinder_gl.h | 96 ++++++++\n> > 4 files changed, 568 insertions(+), 2 deletions(-)\n> > create mode 100644 src/qcam/viewfinder_gl.cpp\n> > create mode 100644 src/qcam/viewfinder_gl.h\n> > \n> > diff --git a/meson.build b/meson.build\n> > index 1ea35e9..c58d458 100644\n> > --- a/meson.build\n> > +++ b/meson.build\n> > @@ -26,6 +26,7 @@ libcamera_version = libcamera_git_version.split('+')[0]\n> > \n> > # Configure the build environment.\n> > cc = meson.get_compiler('c')\n> > +cxx = meson.get_compiler('cpp')\n> > config_h = configuration_data()\n> > \n> > if cc.has_header_symbol('execinfo.h', 'backtrace')\n> > diff --git a/src/qcam/meson.build b/src/qcam/meson.build\n> > index a4bad0a..9bb48c0 100644\n> > --- a/src/qcam/meson.build\n> > +++ b/src/qcam/meson.build\n> > @@ -16,14 +16,14 @@ qcam_moc_headers = files([\n> > \n> > qcam_resources = files([\n> > 'assets/feathericons/feathericons.qrc',\n> > - 'assets/shader/shaders.qrc'\n> > ])\n> > \n> > qt5 = import('qt5')\n> > qt5_dep = dependency('qt5',\n> > method : 'pkg-config',\n> > modules : ['Core', 'Gui', 'Widgets'],\n> > - required : get_option('qcam'))\n> > + required : get_option('qcam'),\n> > + version : '>=5.4')\n> > \n> > if qt5_dep.found()\n> > qcam_deps = [\n> > @@ -42,6 +42,19 @@ if qt5_dep.found()\n> > ])\n> > endif\n> > \n> > + if cxx.has_header_symbol('QOpenGLWidget', 'QOpenGLWidget',\n> > + dependencies : qt5_dep, args : '-fPIC')\n> > + qcam_sources += files([\n> > + 'viewfinder_gl.cpp',\n> > + ])\n> > + qcam_moc_headers += files([\n> > + 'viewfinder_gl.h',\n> > + ])\n> > + qcam_resources += files([\n> > + 'assets/shader/shaders.qrc'\n> > + ])\n> > + endif\n> > +\n> > # gcc 9 introduced a deprecated-copy warning that is triggered by Qt until\n> > # Qt 5.13. clang 10 introduced the same warning, but detects more issues\n> > # that are not fixed in Qt yet. Disable the warning manually in both cases.\n> > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> > new file mode 100644\n> > index 0000000..84f4866\n> > --- /dev/null\n> > +++ b/src/qcam/viewfinder_gl.cpp\n> > @@ -0,0 +1,456 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Linaro\n> > + *\n> > + * viewfinderGL.cpp - OpenGL Viewfinder for rendering by OpenGL shader\n> > + */\n> > +\n> > +#include \"viewfinder_gl.h\"\n> > +\n> > +#include \n> > +\n> > +#include \n> > +\n> > +static const QList supportedFormats{\n> > +\tlibcamera::formats::NV12,\n> > +\tlibcamera::formats::NV21,\n> > +\tlibcamera::formats::NV16,\n> > +\tlibcamera::formats::NV61,\n> > +\tlibcamera::formats::NV24,\n> > +\tlibcamera::formats::NV42,\n> > +\tlibcamera::formats::YUV420,\n> > +\tlibcamera::formats::YVU420\n> > +};\n> > +\n> > +ViewFinderGL::ViewFinderGL(QWidget *parent)\n> > +\t: QOpenGLWidget(parent), buffer_(nullptr), yuvData_(nullptr),\n> > +\t fragmentShader_(nullptr), vertexShader_(nullptr),\n> > +\t vertexBuffer_(QOpenGLBuffer::VertexBuffer),\n> > +\t textureU_(QOpenGLTexture::Target2D),\n> > +\t textureV_(QOpenGLTexture::Target2D),\n> > +\t textureY_(QOpenGLTexture::Target2D)\n> > +{\n> > +}\n> > +\n> > +ViewFinderGL::~ViewFinderGL()\n> > +{\n> > +\tremoveShader();\n> > +\n> > +\tif (vertexBuffer_.isCreated())\n> > +\t\tvertexBuffer_.destroy();\n> \n> I think the QOpenGLBuffer destructor destroys the buffer, you don't need\n> this.\n> \n> > +}\n> > +\n> > +const QList &ViewFinderGL::nativeFormats() const\n> > +{\n> > +\treturn supportedFormats;\n> > +}\n> > +\n> > +int ViewFinderGL::setFormat(const libcamera::PixelFormat &format,\n> > +\t\t\t const QSize &size)\n> > +{\n> > +\tint ret = 0;\n> > +\n> > +\t/* If the fragment is ceeated remove it and create a new one */\n> \n> s/ceeated/created/\n> \n> > +\tif (fragmentShader_) {\n> > +\t\tif (shaderProgram_.isLinked()) {\n> > +\t\t\tshaderProgram_.release();\n> > +\t\t\tshaderProgram_.removeShader(fragmentShader_);\n> > +\t\t\tdelete fragmentShader_;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\tif (selectFormat(format)) {\n> > +\t\tformat_ = format;\n> > +\t\tsize_ = size;\n> > +\t} else {\n> > +\t\tret = -1;\n> > +\t}\n> > +\tupdateGeometry();\n> > +\treturn ret;\n> \n> We tend to exit early in case of errors (and updateGeometry() shouldn't\n> be called in that case):\n> \n> \tif (!selectFormat(format))\n> \t\treturn -1;\n> \n> \tformat_ = format;\n> \tsize_ = size;\n> \n> \tupdateGeometry();\n> \treturn 0;\n> \n> > +}\n> > +\n> > +void ViewFinderGL::stop()\n> > +{\n> > +\tif (buffer_) {\n> > +\t\trenderComplete(buffer_);\n> > +\t\tbuffer_ = nullptr;\n> > +\t}\n> > +}\n> > +\n> > +QImage ViewFinderGL::getCurrentImage()\n> > +{\n> > +\tQMutexLocker locker(&mutex_);\n> > +\n> > +\treturn grabFramebuffer();\n> > +}\n> > +\n> > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> > +{\n> > +\tif (buffer->planes().size() != 1) {\n> > +\t\tqWarning() << \"Multi-planar buffers are not supported\";\n> > +\t\treturn;\n> > +\t}\n> > +\n> > +\tif (buffer_)\n> > +\t\trenderComplete(buffer_);\n> > +\n> > +\tyuvData_ = static_cast(map->memory);\n> > +\tupdate();\n> > +\tbuffer_ = buffer;\n> > +}\n> > +\n> > +bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)\n> > +{\n> > +\tbool ret = true;\n> > +\tswitch (format) {\n> > +\tcase libcamera::formats::NV12:\n> > +\t\thorzSubSample_ = 2;\n> > +\t\tvertSubSample_ = 2;\n> > +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > +\t\tfragmentShaderSrc_ = \":NV_2_planes_UV_f.glsl\";\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::NV21:\n> > +\t\thorzSubSample_ = 2;\n> > +\t\tvertSubSample_ = 2;\n> > +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > +\t\tfragmentShaderSrc_ = \":NV_2_planes_VU_f.glsl\";\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::NV16:\n> > +\t\thorzSubSample_ = 2;\n> > +\t\tvertSubSample_ = 1;\n> > +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > +\t\tfragmentShaderSrc_ = \":NV_2_planes_UV_f.glsl\";\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::NV61:\n> > +\t\thorzSubSample_ = 2;\n> > +\t\tvertSubSample_ = 1;\n> > +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > +\t\tfragmentShaderSrc_ = \":NV_2_planes_VU_f.glsl\";\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::NV24:\n> > +\t\thorzSubSample_ = 1;\n> > +\t\tvertSubSample_ = 1;\n> > +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > +\t\tfragmentShaderSrc_ = \":NV_2_planes_UV_f.glsl\";\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::NV42:\n> > +\t\thorzSubSample_ = 1;\n> > +\t\tvertSubSample_ = 1;\n> > +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > +\t\tfragmentShaderSrc_ = \":NV_2_planes_VU_f.glsl\";\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::YUV420:\n> > +\t\thorzSubSample_ = 2;\n> > +\t\tvertSubSample_ = 2;\n> > +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > +\t\tfragmentShaderSrc_ = \":NV_3_planes_f.glsl\";\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::YVU420:\n> > +\t\thorzSubSample_ = 2;\n> > +\t\tvertSubSample_ = 2;\n> > +\t\tvertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > +\t\tfragmentShaderSrc_ = \":NV_3_planes_f.glsl\";\n> > +\t\tbreak;\n> > +\tdefault:\n> > +\t\tret = false;\n> > +\t\tqWarning() << \"[ViewFinderGL]:\"\n> > +\t\t\t << \"format not supported.\";\n> > +\t\tbreak;\n> > +\t};\n> > +\n> > +\treturn ret;\n> > +}\n> > +\n> > +bool ViewFinderGL::createVertexShader()\n> > +{\n> > +\t/* Create Vertex Shader */\n> > +\tvertexShader_ = new QOpenGLShader(QOpenGLShader::Vertex, this);\n> > +\n> > +\t/* Compile the vertex shader */\n> > +\tif (!vertexShader_->compileSourceFile(vertexShaderSrc_)) {\n> > +\t\tqWarning() << \"[ViewFinderGL]:\" << vertexShader_->log();\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\tshaderProgram_.addShader(vertexShader_);\n> > +\treturn true;\n> > +}\n> > +\n> > +bool ViewFinderGL::createFragmentShader()\n> > +{\n> > +\tint attributeVertex;\n> > +\tint attributeTexture;\n> > +\n> > +\t/* Create Fragment Shader */\n> > +\tfragmentShader_ = new QOpenGLShader(QOpenGLShader::Fragment, this);\n> > +\n> > +\t/* Compile the fragment shader */\n> > +\tif (!fragmentShader_->compileSourceFile(fragmentShaderSrc_)) {\n> > +\t\tqWarning() << \"[ViewFinderGL]:\" << fragmentShader_->log();\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\tshaderProgram_.addShader(fragmentShader_);\n> > +\n> > +\t/* Link shader pipeline */\n> > +\tif (!shaderProgram_.link()) {\n> > +\t\tqWarning() << \"[ViewFinderGL]:\" << shaderProgram_.log();\n> > +\t\tclose();\n> > +\t}\n> > +\n> > +\t/* Bind shader pipeline for use */\n> > +\tif (!shaderProgram_.bind()) {\n> > +\t\tqWarning() << \"[ViewFinderGL]:\" << shaderProgram_.log();\n> > +\t\tclose();\n> > +\t}\n> > +\n> > +\tattributeVertex = shaderProgram_.attributeLocation(\"vertexIn\");\n> > +\tattributeTexture = shaderProgram_.attributeLocation(\"textureIn\");\n> > +\n> > +\tshaderProgram_.enableAttributeArray(attributeVertex);\n> > +\tshaderProgram_.setAttributeBuffer(attributeVertex,\n> > +\t\t\t\t\t GL_FLOAT,\n> > +\t\t\t\t\t 0,\n> > +\t\t\t\t\t 2,\n> > +\t\t\t\t\t 2 * sizeof(GLfloat));\n> > +\n> > +\tshaderProgram_.enableAttributeArray(attributeTexture);\n> > +\tshaderProgram_.setAttributeBuffer(attributeTexture,\n> > +\t\t\t\t\t GL_FLOAT,\n> > +\t\t\t\t\t 8 * sizeof(GLfloat),\n> > +\t\t\t\t\t 2,\n> > +\t\t\t\t\t 2 * sizeof(GLfloat));\n> > +\n> > +\ttextureUniformY_ = shaderProgram_.uniformLocation(\"tex_y\");\n> > +\ttextureUniformU_ = shaderProgram_.uniformLocation(\"tex_u\");\n> > +\ttextureUniformV_ = shaderProgram_.uniformLocation(\"tex_v\");\n> > +\n> > +\tif (!textureY_.isCreated())\n> > +\t\ttextureY_.create();\n> > +\n> > +\tif (!textureU_.isCreated())\n> > +\t\ttextureU_.create();\n> > +\n> > +\tif (!textureV_.isCreated())\n> > +\t\ttextureV_.create();\n> > +\n> > +\tid_y_ = textureY_.textureId();\n> > +\tid_u_ = textureU_.textureId();\n> > +\tid_v_ = textureV_.textureId();\n> > +\treturn true;\n> > +}\n> > +\n> > +void ViewFinderGL::configureTexture(unsigned int id)\n> > +{\n> > +\tglBindTexture(GL_TEXTURE_2D, id);\n> > +\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);\n> > +\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);\n> > +\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);\n> > +\tglTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);\n> > +}\n> > +\n> > +void ViewFinderGL::removeShader()\n> > +{\n> > +\tif (shaderProgram_.isLinked()) {\n> > +\t\tshaderProgram_.release();\n> > +\t\tshaderProgram_.removeAllShaders();\n> > +\t}\n> > +\n> > +\tif (fragmentShader_)\n> > +\t\tdelete fragmentShader_;\n> > +\n> > +\tif (vertexShader_)\n> > +\t\tdelete vertexShader_;\n> > +}\n> > +\n> > +void ViewFinderGL::initializeGL()\n> > +{\n> > +\tinitializeOpenGLFunctions();\n> > +\tglEnable(GL_TEXTURE_2D);\n> > +\tglDisable(GL_DEPTH_TEST);\n> > +\n> > +\tstatic const GLfloat coordinates[2][4][2]{\n> > +\t\t{\n> > +\t\t\t/* Vertex coordinates */\n> > +\t\t\t{ -1.0f, -1.0f },\n> > +\t\t\t{ -1.0f, +1.0f },\n> > +\t\t\t{ +1.0f, +1.0f },\n> > +\t\t\t{ +1.0f, -1.0f },\n> > +\t\t},\n> > +\t\t{\n> > +\t\t\t/* Texture coordinates */\n> > +\t\t\t{ 1.0f, 0.0f },\n> > +\t\t\t{ 1.0f, 1.0f },\n> > +\t\t\t{ 0.0f, 1.0f },\n> > +\t\t\t{ 0.0f, 0.0f },\n> \n> I *think* this should be\n> \n> \t\t\t{ 0.0f, 1.0f },\n> \t\t\t{ 0.0f, 0.0f },\n> \t\t\t{ 1.0f, 0.0f },\n> \t\t\t{ 1.0f, 1.0f },\n> \n> I'll test it and try to understand :-)\n\nI confirm that the original patch rotates the image by 180° for me,\nwhile the proposed values above show it in the right orientation. I've\ncompared -r qt and -r gles to check that.\n\n> > +\t\t},\n> > +\t};\n> > +\n> > +\tvertexBuffer_.create();\n> > +\tvertexBuffer_.bind();\n> > +\tvertexBuffer_.allocate(coordinates, sizeof(coordinates));\n> > +\n> > +\t/* Create Vertex Shader */\n> > +\tif (!createVertexShader())\n> > +\t\tqWarning() << \"[ViewFinderGL]: create vertex shader failed.\";\n> > +\n> > +\tglClearColor(1.0f, 1.0f, 1.0f, 0.0f);\n> > +}\n> > +\n> > +void ViewFinderGL::doRender()\n> > +{\n> > +\tswitch (format_) {\n> > +\tcase libcamera::formats::NV12:\n> > +\tcase libcamera::formats::NV21:\n> > +\tcase libcamera::formats::NV16:\n> > +\tcase libcamera::formats::NV61:\n> > +\tcase libcamera::formats::NV24:\n> > +\tcase libcamera::formats::NV42:\n> > +\t\t/* activate texture Y */\n> \n> s/activate/Activate/ (and similarly below).\n> \n> > +\t\tglActiveTexture(GL_TEXTURE0);\n> > +\t\tconfigureTexture(id_y_);\n> > +\t\tglTexImage2D(GL_TEXTURE_2D,\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t size_.width(),\n> > +\t\t\t size_.height(),\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t GL_UNSIGNED_BYTE,\n> > +\t\t\t yuvData_);\n> > +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n> > +\n> > +\t\t/* activate texture UV/VU */\n> > +\t\tglActiveTexture(GL_TEXTURE1);\n> > +\t\tconfigureTexture(id_u_);\n> > +\t\tglTexImage2D(GL_TEXTURE_2D,\n> > +\t\t\t 0,\n> > +\t\t\t GL_RG,\n> > +\t\t\t size_.width() / horzSubSample_,\n> > +\t\t\t size_.height() / vertSubSample_,\n> > +\t\t\t 0,\n> > +\t\t\t GL_RG,\n> > +\t\t\t GL_UNSIGNED_BYTE,\n> > +\t\t\t (char *)yuvData_ + size_.width() * size_.height());\n> > +\t\tshaderProgram_.setUniformValue(textureUniformU_, 1);\n> > +\t\tbreak;\n> > +\n> > +\tcase libcamera::formats::YUV420:\n> > +\t\t/* activate texture Y */\n> > +\t\tglActiveTexture(GL_TEXTURE0);\n> > +\t\tconfigureTexture(id_y_);\n> > +\t\tglTexImage2D(GL_TEXTURE_2D,\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t size_.width(),\n> > +\t\t\t size_.height(),\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t GL_UNSIGNED_BYTE,\n> > +\t\t\t yuvData_);\n> > +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n> > +\n> > +\t\t/* activate texture U */\n> > +\t\tglActiveTexture(GL_TEXTURE1);\n> > +\t\tconfigureTexture(id_u_);\n> > +\t\tglTexImage2D(GL_TEXTURE_2D,\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t size_.width() / horzSubSample_,\n> > +\t\t\t size_.height() / vertSubSample_,\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t GL_UNSIGNED_BYTE,\n> > +\t\t\t (char *)yuvData_ + size_.width() * size_.height());\n> > +\t\tshaderProgram_.setUniformValue(textureUniformU_, 1);\n> > +\n> > +\t\t/* activate texture V */\n> > +\t\tglActiveTexture(GL_TEXTURE2);\n> > +\t\tconfigureTexture(id_v_);\n> > +\t\tglTexImage2D(GL_TEXTURE_2D,\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t size_.width() / horzSubSample_,\n> > +\t\t\t size_.height() / vertSubSample_,\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t GL_UNSIGNED_BYTE,\n> > +\t\t\t (char *)yuvData_ + size_.width() * size_.height() * 5 / 4);\n> > +\t\tshaderProgram_.setUniformValue(textureUniformV_, 2);\n> > +\t\tbreak;\n> > +\n> > +\tcase libcamera::formats::YVU420:\n> > +\t\t/* activate texture Y */\n> > +\t\tglActiveTexture(GL_TEXTURE0);\n> > +\t\tconfigureTexture(id_y_);\n> > +\t\tglTexImage2D(GL_TEXTURE_2D,\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t size_.width(),\n> > +\t\t\t size_.height(),\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t GL_UNSIGNED_BYTE,\n> > +\t\t\t yuvData_);\n> > +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n> > +\n> > +\t\t/* activate texture V */\n> > +\t\tglActiveTexture(GL_TEXTURE2);\n> > +\t\tconfigureTexture(id_v_);\n> > +\t\tglTexImage2D(GL_TEXTURE_2D,\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t size_.width() / horzSubSample_,\n> > +\t\t\t size_.height() / vertSubSample_,\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t GL_UNSIGNED_BYTE,\n> > +\t\t\t (char *)yuvData_ + size_.width() * size_.height());\n> > +\t\tshaderProgram_.setUniformValue(textureUniformV_, 1);\n> \n> I don't think this is correct. GL_TEXTURE1 stores the U plane, and you\n> assign it to tex_v, which is the V plane in the shader.\n> \n> There are two options, either s/1/2/ or s/GL_TEXTURE2/GL_TEXTURE1/ (and\n> the other way below).\n> \n> With these small issues addressed,\n> \n> Reviewed-by: Laurent Pinchart \n> \n> I can also fix these when applying the patch, but given that there are\n> quite a few issues, I would then send a v7 to the list to make sure I\n> haven't done anything wrong.\n> \n> > +\n> > +\t\t/* activate texture U */\n> > +\t\tglActiveTexture(GL_TEXTURE1);\n> > +\t\tconfigureTexture(id_u_);\n> > +\t\tglTexImage2D(GL_TEXTURE_2D,\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t size_.width() / horzSubSample_,\n> > +\t\t\t size_.height() / vertSubSample_,\n> > +\t\t\t 0,\n> > +\t\t\t GL_RED,\n> > +\t\t\t GL_UNSIGNED_BYTE,\n> > +\t\t\t (char *)yuvData_ + size_.width() * size_.height() * 5 / 4);\n> > +\t\tshaderProgram_.setUniformValue(textureUniformU_, 2);\n> > +\t\tbreak;\n> > +\n> > +\tdefault:\n> > +\t\tbreak;\n> > +\t};\n> > +}\n> > +\n> > +void ViewFinderGL::paintGL()\n> > +{\n> > +\tif (!fragmentShader_)\n> > +\t\tif (!createFragmentShader()) {\n> > +\t\t\tqWarning() << \"[ViewFinderGL]:\"\n> > +\t\t\t\t << \"create fragment shader failed.\";\n> > +\t\t}\n> > +\n> > +\tif (yuvData_) {\n> > +\t\tglClearColor(0.0, 0.0, 0.0, 1.0);\n> > +\t\tglClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);\n> > +\n> > +\t\tdoRender();\n> > +\t\tglDrawArrays(GL_TRIANGLE_FAN, 0, 4);\n> > +\t}\n> > +}\n> > +\n> > +void ViewFinderGL::resizeGL(int w, int h)\n> > +{\n> > +\tglViewport(0, 0, w, h);\n> > +}\n> > +\n> > +QSize ViewFinderGL::sizeHint() const\n> > +{\n> > +\treturn size_.isValid() ? size_ : QSize(640, 480);\n> > +}\n> > diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h\n> > new file mode 100644\n> > index 0000000..69502b7\n> > --- /dev/null\n> > +++ b/src/qcam/viewfinder_gl.h\n> > @@ -0,0 +1,96 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Linaro\n> > + *\n> > + * viewfinder_GL.h - OpenGL Viewfinder for rendering by OpenGL shader\n> > + *\n> > + */\n> > +#ifndef __VIEWFINDER_GL_H__\n> > +#define __VIEWFINDER_GL_H__\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \"viewfinder.h\"\n> > +\n> > +class ViewFinderGL : public QOpenGLWidget,\n> > +\t\t public ViewFinder,\n> > +\t\t protected QOpenGLFunctions\n> > +{\n> > +\tQ_OBJECT\n> > +\n> > +public:\n> > +\tViewFinderGL(QWidget *parent = nullptr);\n> > +\t~ViewFinderGL();\n> > +\n> > +\tconst QList &nativeFormats() const override;\n> > +\n> > +\tint setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> > +\tvoid render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> > +\tvoid stop() override;\n> > +\n> > +\tQImage getCurrentImage() override;\n> > +\n> > +Q_SIGNALS:\n> > +\tvoid renderComplete(libcamera::FrameBuffer *buffer);\n> > +\n> > +protected:\n> > +\tvoid initializeGL() override;\n> > +\tvoid paintGL() override;\n> > +\tvoid resizeGL(int w, int h) override;\n> > +\tQSize sizeHint() const override;\n> > +\n> > +private:\n> > +\tbool selectFormat(const libcamera::PixelFormat &format);\n> > +\n> > +\tvoid configureTexture(unsigned int id);\n> > +\tbool createFragmentShader();\n> > +\tbool createVertexShader();\n> > +\tvoid removeShader();\n> > +\tvoid doRender();\n> > +\n> > +\t/* Captured image size, format and buffer */\n> > +\tlibcamera::FrameBuffer *buffer_;\n> > +\tlibcamera::PixelFormat format_;\n> > +\tQSize size_;\n> > +\tunsigned char *yuvData_;\n> > +\n> > +\t/* OpenGL components for rendering */\n> > +\tQOpenGLShader *fragmentShader_;\n> > +\tQOpenGLShader *vertexShader_;\n> > +\tQOpenGLShaderProgram shaderProgram_;\n> > +\n> > +\t/* Vertex buffer */\n> > +\tQOpenGLBuffer vertexBuffer_;\n> > +\n> > +\t/* Fragment and Vertex shader file name */\n> > +\tQString fragmentShaderSrc_;\n> > +\tQString vertexShaderSrc_;\n> > +\n> > +\t/* YUV texture planars and parameters */\n> > +\tGLuint id_u_;\n> > +\tGLuint id_v_;\n> > +\tGLuint id_y_;\n> > +\tGLuint textureUniformU_;\n> > +\tGLuint textureUniformV_;\n> > +\tGLuint textureUniformY_;\n> > +\tQOpenGLTexture textureU_;\n> > +\tQOpenGLTexture textureV_;\n> > +\tQOpenGLTexture textureY_;\n> > +\tunsigned int horzSubSample_;\n> > +\tunsigned int vertSubSample_;\n> > +\n> > +\tQMutex mutex_; /* Prevent concurrent access to image_ */\n> > +};\n> > +\n> > +#endif /* __VIEWFINDER_GL_H__ */","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id ED116C3B5B\n\tfor ;\n\tSat, 12 Sep 2020 22:03:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 67FB762D5B;\n\tSun, 13 Sep 2020 00:03:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 610B060534\n\tfor ;\n\tSun, 13 Sep 2020 00:03:44 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C97CB276;\n\tSun, 13 Sep 2020 00:03:43 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tOeYpZFG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1599948224;\n\tbh=mJhaxX+7m9hWHGXPhmi9ySVscTkWw5Jk/Lsv3caJqpM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tOeYpZFGOO5u9IbQ1y2w063cJytNyJ6jvF6S4CWtBYCv3zP1L/kDMak3e9zlkR4Ws\n\tDzHRCJzi1XQh45ip3Blkh0JPj+lHQuKSaM1HKluatjFXZHzUPKGtxYv4Z21CPmvc/x\n\tWIRwAqHRgImXGC7n1EfWuk9ZyJu4eefHWlIAjWGo=","Date":"Sun, 13 Sep 2020 01:03:15 +0300","From":"Laurent Pinchart ","To":"Show Liu ","Message-ID":"<20200912220315.GC3933@pendragon.ideasonboard.com>","References":"<20200911085514.30021-1-show.liu@linaro.org>\n\t<20200911085514.30021-4-show.liu@linaro.org>\n\t<20200912020910.GM6808@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200912020910.GM6808@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v6 3/4] qcam: add viewfinderGL class\n\tto accelerate the format convert","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12486,"web_url":"https://patchwork.libcamera.org/comment/12486/","msgid":"","date":"2020-09-14T01:58:42","subject":"Re: [libcamera-devel] [PATCH v6 3/4] qcam: add viewfinderGL class\n\tto accelerate the format convert","submitter":{"id":24,"url":"https://patchwork.libcamera.org/api/people/24/","name":"Show Liu","email":"show.liu@linaro.org"},"content":"Hi Laurent,\n\n\n\nOn Sun, Sep 13, 2020 at 6:03 AM Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Show,\n>\n> On Sat, Sep 12, 2020 at 05:09:10AM +0300, Laurent Pinchart wrote:\n> > Hi Show,\n> >\n> > Thank you for the patch.\n> >\n> > In the subject line, s/viewfinderGL/ViewFinderGL/ and\n> > s/convert/conversion/\n> >\n> > On Fri, Sep 11, 2020 at 04:55:13PM +0800, Show Liu wrote:\n> > > the viewfinderGL accelerates the format conversion by\n> > > using OpenGL ES shader\n> >\n> > I would add\n> >\n> > \"The minimum Qt version is bumped to v5.4, as QOpenGLWidget wasn't\n> > available before that.\"\n> >\n> > > Signed-off-by: Show Liu \n> > > ---\n> > > meson.build | 1 +\n> > > src/qcam/meson.build | 17 +-\n> > > src/qcam/viewfinder_gl.cpp | 456 +++++++++++++++++++++++++++++++++++++\n> > > src/qcam/viewfinder_gl.h | 96 ++++++++\n> > > 4 files changed, 568 insertions(+), 2 deletions(-)\n> > > create mode 100644 src/qcam/viewfinder_gl.cpp\n> > > create mode 100644 src/qcam/viewfinder_gl.h\n> > >\n> > > diff --git a/meson.build b/meson.build\n> > > index 1ea35e9..c58d458 100644\n> > > --- a/meson.build\n> > > +++ b/meson.build\n> > > @@ -26,6 +26,7 @@ libcamera_version =\n> libcamera_git_version.split('+')[0]\n> > >\n> > > # Configure the build environment.\n> > > cc = meson.get_compiler('c')\n> > > +cxx = meson.get_compiler('cpp')\n> > > config_h = configuration_data()\n> > >\n> > > if cc.has_header_symbol('execinfo.h', 'backtrace')\n> > > diff --git a/src/qcam/meson.build b/src/qcam/meson.build\n> > > index a4bad0a..9bb48c0 100644\n> > > --- a/src/qcam/meson.build\n> > > +++ b/src/qcam/meson.build\n> > > @@ -16,14 +16,14 @@ qcam_moc_headers = files([\n> > >\n> > > qcam_resources = files([\n> > > 'assets/feathericons/feathericons.qrc',\n> > > - 'assets/shader/shaders.qrc'\n> > > ])\n> > >\n> > > qt5 = import('qt5')\n> > > qt5_dep = dependency('qt5',\n> > > method : 'pkg-config',\n> > > modules : ['Core', 'Gui', 'Widgets'],\n> > > - required : get_option('qcam'))\n> > > + required : get_option('qcam'),\n> > > + version : '>=5.4')\n> > >\n> > > if qt5_dep.found()\n> > > qcam_deps = [\n> > > @@ -42,6 +42,19 @@ if qt5_dep.found()\n> > > ])\n> > > endif\n> > >\n> > > + if cxx.has_header_symbol('QOpenGLWidget', 'QOpenGLWidget',\n> > > + dependencies : qt5_dep, args : '-fPIC')\n> > > + qcam_sources += files([\n> > > + 'viewfinder_gl.cpp',\n> > > + ])\n> > > + qcam_moc_headers += files([\n> > > + 'viewfinder_gl.h',\n> > > + ])\n> > > + qcam_resources += files([\n> > > + 'assets/shader/shaders.qrc'\n> > > + ])\n> > > + endif\n> > > +\n> > > # gcc 9 introduced a deprecated-copy warning that is triggered by\n> Qt until\n> > > # Qt 5.13. clang 10 introduced the same warning, but detects more\n> issues\n> > > # that are not fixed in Qt yet. Disable the warning manually in\n> both cases.\n> > > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> > > new file mode 100644\n> > > index 0000000..84f4866\n> > > --- /dev/null\n> > > +++ b/src/qcam/viewfinder_gl.cpp\n> > > @@ -0,0 +1,456 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2020, Linaro\n> > > + *\n> > > + * viewfinderGL.cpp - OpenGL Viewfinder for rendering by OpenGL shader\n> > > + */\n> > > +\n> > > +#include \"viewfinder_gl.h\"\n> > > +\n> > > +#include \n> > > +\n> > > +#include \n> > > +\n> > > +static const QList supportedFormats{\n> > > + libcamera::formats::NV12,\n> > > + libcamera::formats::NV21,\n> > > + libcamera::formats::NV16,\n> > > + libcamera::formats::NV61,\n> > > + libcamera::formats::NV24,\n> > > + libcamera::formats::NV42,\n> > > + libcamera::formats::YUV420,\n> > > + libcamera::formats::YVU420\n> > > +};\n> > > +\n> > > +ViewFinderGL::ViewFinderGL(QWidget *parent)\n> > > + : QOpenGLWidget(parent), buffer_(nullptr), yuvData_(nullptr),\n> > > + fragmentShader_(nullptr), vertexShader_(nullptr),\n> > > + vertexBuffer_(QOpenGLBuffer::VertexBuffer),\n> > > + textureU_(QOpenGLTexture::Target2D),\n> > > + textureV_(QOpenGLTexture::Target2D),\n> > > + textureY_(QOpenGLTexture::Target2D)\n> > > +{\n> > > +}\n> > > +\n> > > +ViewFinderGL::~ViewFinderGL()\n> > > +{\n> > > + removeShader();\n> > > +\n> > > + if (vertexBuffer_.isCreated())\n> > > + vertexBuffer_.destroy();\n> >\n> > I think the QOpenGLBuffer destructor destroys the buffer, you don't need\n> > this.\n> >\n> > > +}\n> > > +\n> > > +const QList &ViewFinderGL::nativeFormats()\n> const\n> > > +{\n> > > + return supportedFormats;\n> > > +}\n> > > +\n> > > +int ViewFinderGL::setFormat(const libcamera::PixelFormat &format,\n> > > + const QSize &size)\n> > > +{\n> > > + int ret = 0;\n> > > +\n> > > + /* If the fragment is ceeated remove it and create a new one */\n> >\n> > s/ceeated/created/\n> >\n> > > + if (fragmentShader_) {\n> > > + if (shaderProgram_.isLinked()) {\n> > > + shaderProgram_.release();\n> > > + shaderProgram_.removeShader(fragmentShader_);\n> > > + delete fragmentShader_;\n> > > + }\n> > > + }\n> > > +\n> > > + if (selectFormat(format)) {\n> > > + format_ = format;\n> > > + size_ = size;\n> > > + } else {\n> > > + ret = -1;\n> > > + }\n> > > + updateGeometry();\n> > > + return ret;\n> >\n> > We tend to exit early in case of errors (and updateGeometry() shouldn't\n> > be called in that case):\n> >\n> > if (!selectFormat(format))\n> > return -1;\n> >\n> > format_ = format;\n> > size_ = size;\n> >\n> > updateGeometry();\n> > return 0;\n> >\n> > > +}\n> > > +\n> > > +void ViewFinderGL::stop()\n> > > +{\n> > > + if (buffer_) {\n> > > + renderComplete(buffer_);\n> > > + buffer_ = nullptr;\n> > > + }\n> > > +}\n> > > +\n> > > +QImage ViewFinderGL::getCurrentImage()\n> > > +{\n> > > + QMutexLocker locker(&mutex_);\n> > > +\n> > > + return grabFramebuffer();\n> > > +}\n> > > +\n> > > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer,\n> MappedBuffer *map)\n> > > +{\n> > > + if (buffer->planes().size() != 1) {\n> > > + qWarning() << \"Multi-planar buffers are not supported\";\n> > > + return;\n> > > + }\n> > > +\n> > > + if (buffer_)\n> > > + renderComplete(buffer_);\n> > > +\n> > > + yuvData_ = static_cast(map->memory);\n> > > + update();\n> > > + buffer_ = buffer;\n> > > +}\n> > > +\n> > > +bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)\n> > > +{\n> > > + bool ret = true;\n> > > + switch (format) {\n> > > + case libcamera::formats::NV12:\n> > > + horzSubSample_ = 2;\n> > > + vertSubSample_ = 2;\n> > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > + fragmentShaderSrc_ = \":NV_2_planes_UV_f.glsl\";\n> > > + break;\n> > > + case libcamera::formats::NV21:\n> > > + horzSubSample_ = 2;\n> > > + vertSubSample_ = 2;\n> > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > + fragmentShaderSrc_ = \":NV_2_planes_VU_f.glsl\";\n> > > + break;\n> > > + case libcamera::formats::NV16:\n> > > + horzSubSample_ = 2;\n> > > + vertSubSample_ = 1;\n> > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > + fragmentShaderSrc_ = \":NV_2_planes_UV_f.glsl\";\n> > > + break;\n> > > + case libcamera::formats::NV61:\n> > > + horzSubSample_ = 2;\n> > > + vertSubSample_ = 1;\n> > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > + fragmentShaderSrc_ = \":NV_2_planes_VU_f.glsl\";\n> > > + break;\n> > > + case libcamera::formats::NV24:\n> > > + horzSubSample_ = 1;\n> > > + vertSubSample_ = 1;\n> > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > + fragmentShaderSrc_ = \":NV_2_planes_UV_f.glsl\";\n> > > + break;\n> > > + case libcamera::formats::NV42:\n> > > + horzSubSample_ = 1;\n> > > + vertSubSample_ = 1;\n> > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > + fragmentShaderSrc_ = \":NV_2_planes_VU_f.glsl\";\n> > > + break;\n> > > + case libcamera::formats::YUV420:\n> > > + horzSubSample_ = 2;\n> > > + vertSubSample_ = 2;\n> > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > + fragmentShaderSrc_ = \":NV_3_planes_f.glsl\";\n> > > + break;\n> > > + case libcamera::formats::YVU420:\n> > > + horzSubSample_ = 2;\n> > > + vertSubSample_ = 2;\n> > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > + fragmentShaderSrc_ = \":NV_3_planes_f.glsl\";\n> > > + break;\n> > > + default:\n> > > + ret = false;\n> > > + qWarning() << \"[ViewFinderGL]:\"\n> > > + << \"format not supported.\";\n> > > + break;\n> > > + };\n> > > +\n> > > + return ret;\n> > > +}\n> > > +\n> > > +bool ViewFinderGL::createVertexShader()\n> > > +{\n> > > + /* Create Vertex Shader */\n> > > + vertexShader_ = new QOpenGLShader(QOpenGLShader::Vertex, this);\n> > > +\n> > > + /* Compile the vertex shader */\n> > > + if (!vertexShader_->compileSourceFile(vertexShaderSrc_)) {\n> > > + qWarning() << \"[ViewFinderGL]:\" << vertexShader_->log();\n> > > + return false;\n> > > + }\n> > > +\n> > > + shaderProgram_.addShader(vertexShader_);\n> > > + return true;\n> > > +}\n> > > +\n> > > +bool ViewFinderGL::createFragmentShader()\n> > > +{\n> > > + int attributeVertex;\n> > > + int attributeTexture;\n> > > +\n> > > + /* Create Fragment Shader */\n> > > + fragmentShader_ = new QOpenGLShader(QOpenGLShader::Fragment, this);\n> > > +\n> > > + /* Compile the fragment shader */\n> > > + if (!fragmentShader_->compileSourceFile(fragmentShaderSrc_)) {\n> > > + qWarning() << \"[ViewFinderGL]:\" << fragmentShader_->log();\n> > > + return false;\n> > > + }\n> > > +\n> > > + shaderProgram_.addShader(fragmentShader_);\n> > > +\n> > > + /* Link shader pipeline */\n> > > + if (!shaderProgram_.link()) {\n> > > + qWarning() << \"[ViewFinderGL]:\" << shaderProgram_.log();\n> > > + close();\n> > > + }\n> > > +\n> > > + /* Bind shader pipeline for use */\n> > > + if (!shaderProgram_.bind()) {\n> > > + qWarning() << \"[ViewFinderGL]:\" << shaderProgram_.log();\n> > > + close();\n> > > + }\n> > > +\n> > > + attributeVertex = shaderProgram_.attributeLocation(\"vertexIn\");\n> > > + attributeTexture = shaderProgram_.attributeLocation(\"textureIn\");\n> > > +\n> > > + shaderProgram_.enableAttributeArray(attributeVertex);\n> > > + shaderProgram_.setAttributeBuffer(attributeVertex,\n> > > + GL_FLOAT,\n> > > + 0,\n> > > + 2,\n> > > + 2 * sizeof(GLfloat));\n> > > +\n> > > + shaderProgram_.enableAttributeArray(attributeTexture);\n> > > + shaderProgram_.setAttributeBuffer(attributeTexture,\n> > > + GL_FLOAT,\n> > > + 8 * sizeof(GLfloat),\n> > > + 2,\n> > > + 2 * sizeof(GLfloat));\n> > > +\n> > > + textureUniformY_ = shaderProgram_.uniformLocation(\"tex_y\");\n> > > + textureUniformU_ = shaderProgram_.uniformLocation(\"tex_u\");\n> > > + textureUniformV_ = shaderProgram_.uniformLocation(\"tex_v\");\n> > > +\n> > > + if (!textureY_.isCreated())\n> > > + textureY_.create();\n> > > +\n> > > + if (!textureU_.isCreated())\n> > > + textureU_.create();\n> > > +\n> > > + if (!textureV_.isCreated())\n> > > + textureV_.create();\n> > > +\n> > > + id_y_ = textureY_.textureId();\n> > > + id_u_ = textureU_.textureId();\n> > > + id_v_ = textureV_.textureId();\n> > > + return true;\n> > > +}\n> > > +\n> > > +void ViewFinderGL::configureTexture(unsigned int id)\n> > > +{\n> > > + glBindTexture(GL_TEXTURE_2D, id);\n> > > + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);\n> > > + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);\n> > > + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S,\n> GL_CLAMP_TO_EDGE);\n> > > + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T,\n> GL_CLAMP_TO_EDGE);\n> > > +}\n> > > +\n> > > +void ViewFinderGL::removeShader()\n> > > +{\n> > > + if (shaderProgram_.isLinked()) {\n> > > + shaderProgram_.release();\n> > > + shaderProgram_.removeAllShaders();\n> > > + }\n> > > +\n> > > + if (fragmentShader_)\n> > > + delete fragmentShader_;\n> > > +\n> > > + if (vertexShader_)\n> > > + delete vertexShader_;\n> > > +}\n> > > +\n> > > +void ViewFinderGL::initializeGL()\n> > > +{\n> > > + initializeOpenGLFunctions();\n> > > + glEnable(GL_TEXTURE_2D);\n> > > + glDisable(GL_DEPTH_TEST);\n> > > +\n> > > + static const GLfloat coordinates[2][4][2]{\n> > > + {\n> > > + /* Vertex coordinates */\n> > > + { -1.0f, -1.0f },\n> > > + { -1.0f, +1.0f },\n> > > + { +1.0f, +1.0f },\n> > > + { +1.0f, -1.0f },\n> > > + },\n> > > + {\n> > > + /* Texture coordinates */\n> > > + { 1.0f, 0.0f },\n> > > + { 1.0f, 1.0f },\n> > > + { 0.0f, 1.0f },\n> > > + { 0.0f, 0.0f },\n> >\n> > I *think* this should be\n> >\n> > { 0.0f, 1.0f },\n> > { 0.0f, 0.0f },\n> > { 1.0f, 0.0f },\n> > { 1.0f, 1.0f },\n> >\n> > I'll test it and try to understand :-)\n>\n> I confirm that the original patch rotates the image by 180° for me,\n> while the proposed values above show it in the right orientation. I've\n> compared -r qt and -r gles to check that.\n>\nAre you going to apply above value to the v7 too? If so I will not send out\nthe patch to fix it.\n\nThanks,\nShow\n\n\n>\n> > > + },\n> > > + };\n> > > +\n> > > + vertexBuffer_.create();\n> > > + vertexBuffer_.bind();\n> > > + vertexBuffer_.allocate(coordinates, sizeof(coordinates));\n> > > +\n> > > + /* Create Vertex Shader */\n> > > + if (!createVertexShader())\n> > > + qWarning() << \"[ViewFinderGL]: create vertex shader\n> failed.\";\n> > > +\n> > > + glClearColor(1.0f, 1.0f, 1.0f, 0.0f);\n> > > +}\n> > > +\n> > > +void ViewFinderGL::doRender()\n> > > +{\n> > > + switch (format_) {\n> > > + case libcamera::formats::NV12:\n> > > + case libcamera::formats::NV21:\n> > > + case libcamera::formats::NV16:\n> > > + case libcamera::formats::NV61:\n> > > + case libcamera::formats::NV24:\n> > > + case libcamera::formats::NV42:\n> > > + /* activate texture Y */\n> >\n> > s/activate/Activate/ (and similarly below).\n> >\n> > > + glActiveTexture(GL_TEXTURE0);\n> > > + configureTexture(id_y_);\n> > > + glTexImage2D(GL_TEXTURE_2D,\n> > > + 0,\n> > > + GL_RED,\n> > > + size_.width(),\n> > > + size_.height(),\n> > > + 0,\n> > > + GL_RED,\n> > > + GL_UNSIGNED_BYTE,\n> > > + yuvData_);\n> > > + shaderProgram_.setUniformValue(textureUniformY_, 0);\n> > > +\n> > > + /* activate texture UV/VU */\n> > > + glActiveTexture(GL_TEXTURE1);\n> > > + configureTexture(id_u_);\n> > > + glTexImage2D(GL_TEXTURE_2D,\n> > > + 0,\n> > > + GL_RG,\n> > > + size_.width() / horzSubSample_,\n> > > + size_.height() / vertSubSample_,\n> > > + 0,\n> > > + GL_RG,\n> > > + GL_UNSIGNED_BYTE,\n> > > + (char *)yuvData_ + size_.width() *\n> size_.height());\n> > > + shaderProgram_.setUniformValue(textureUniformU_, 1);\n> > > + break;\n> > > +\n> > > + case libcamera::formats::YUV420:\n> > > + /* activate texture Y */\n> > > + glActiveTexture(GL_TEXTURE0);\n> > > + configureTexture(id_y_);\n> > > + glTexImage2D(GL_TEXTURE_2D,\n> > > + 0,\n> > > + GL_RED,\n> > > + size_.width(),\n> > > + size_.height(),\n> > > + 0,\n> > > + GL_RED,\n> > > + GL_UNSIGNED_BYTE,\n> > > + yuvData_);\n> > > + shaderProgram_.setUniformValue(textureUniformY_, 0);\n> > > +\n> > > + /* activate texture U */\n> > > + glActiveTexture(GL_TEXTURE1);\n> > > + configureTexture(id_u_);\n> > > + glTexImage2D(GL_TEXTURE_2D,\n> > > + 0,\n> > > + GL_RED,\n> > > + size_.width() / horzSubSample_,\n> > > + size_.height() / vertSubSample_,\n> > > + 0,\n> > > + GL_RED,\n> > > + GL_UNSIGNED_BYTE,\n> > > + (char *)yuvData_ + size_.width() *\n> size_.height());\n> > > + shaderProgram_.setUniformValue(textureUniformU_, 1);\n> > > +\n> > > + /* activate texture V */\n> > > + glActiveTexture(GL_TEXTURE2);\n> > > + configureTexture(id_v_);\n> > > + glTexImage2D(GL_TEXTURE_2D,\n> > > + 0,\n> > > + GL_RED,\n> > > + size_.width() / horzSubSample_,\n> > > + size_.height() / vertSubSample_,\n> > > + 0,\n> > > + GL_RED,\n> > > + GL_UNSIGNED_BYTE,\n> > > + (char *)yuvData_ + size_.width() *\n> size_.height() * 5 / 4);\n> > > + shaderProgram_.setUniformValue(textureUniformV_, 2);\n> > > + break;\n> > > +\n> > > + case libcamera::formats::YVU420:\n> > > + /* activate texture Y */\n> > > + glActiveTexture(GL_TEXTURE0);\n> > > + configureTexture(id_y_);\n> > > + glTexImage2D(GL_TEXTURE_2D,\n> > > + 0,\n> > > + GL_RED,\n> > > + size_.width(),\n> > > + size_.height(),\n> > > + 0,\n> > > + GL_RED,\n> > > + GL_UNSIGNED_BYTE,\n> > > + yuvData_);\n> > > + shaderProgram_.setUniformValue(textureUniformY_, 0);\n> > > +\n> > > + /* activate texture V */\n> > > + glActiveTexture(GL_TEXTURE2);\n> > > + configureTexture(id_v_);\n> > > + glTexImage2D(GL_TEXTURE_2D,\n> > > + 0,\n> > > + GL_RED,\n> > > + size_.width() / horzSubSample_,\n> > > + size_.height() / vertSubSample_,\n> > > + 0,\n> > > + GL_RED,\n> > > + GL_UNSIGNED_BYTE,\n> > > + (char *)yuvData_ + size_.width() *\n> size_.height());\n> > > + shaderProgram_.setUniformValue(textureUniformV_, 1);\n> >\n> > I don't think this is correct. GL_TEXTURE1 stores the U plane, and you\n> > assign it to tex_v, which is the V plane in the shader.\n> >\n> > There are two options, either s/1/2/ or s/GL_TEXTURE2/GL_TEXTURE1/ (and\n> > the other way below).\n> >\n> > With these small issues addressed,\n> >\n> > Reviewed-by: Laurent Pinchart \n> >\n> > I can also fix these when applying the patch, but given that there are\n> > quite a few issues, I would then send a v7 to the list to make sure I\n> > haven't done anything wrong.\n> >\n> > > +\n> > > + /* activate texture U */\n> > > + glActiveTexture(GL_TEXTURE1);\n> > > + configureTexture(id_u_);\n> > > + glTexImage2D(GL_TEXTURE_2D,\n> > > + 0,\n> > > + GL_RED,\n> > > + size_.width() / horzSubSample_,\n> > > + size_.height() / vertSubSample_,\n> > > + 0,\n> > > + GL_RED,\n> > > + GL_UNSIGNED_BYTE,\n> > > + (char *)yuvData_ + size_.width() *\n> size_.height() * 5 / 4);\n> > > + shaderProgram_.setUniformValue(textureUniformU_, 2);\n> > > + break;\n> > > +\n> > > + default:\n> > > + break;\n> > > + };\n> > > +}\n> > > +\n> > > +void ViewFinderGL::paintGL()\n> > > +{\n> > > + if (!fragmentShader_)\n> > > + if (!createFragmentShader()) {\n> > > + qWarning() << \"[ViewFinderGL]:\"\n> > > + << \"create fragment shader failed.\";\n> > > + }\n> > > +\n> > > + if (yuvData_) {\n> > > + glClearColor(0.0, 0.0, 0.0, 1.0);\n> > > + glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);\n> > > +\n> > > + doRender();\n> > > + glDrawArrays(GL_TRIANGLE_FAN, 0, 4);\n> > > + }\n> > > +}\n> > > +\n> > > +void ViewFinderGL::resizeGL(int w, int h)\n> > > +{\n> > > + glViewport(0, 0, w, h);\n> > > +}\n> > > +\n> > > +QSize ViewFinderGL::sizeHint() const\n> > > +{\n> > > + return size_.isValid() ? size_ : QSize(640, 480);\n> > > +}\n> > > diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h\n> > > new file mode 100644\n> > > index 0000000..69502b7\n> > > --- /dev/null\n> > > +++ b/src/qcam/viewfinder_gl.h\n> > > @@ -0,0 +1,96 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2020, Linaro\n> > > + *\n> > > + * viewfinder_GL.h - OpenGL Viewfinder for rendering by OpenGL shader\n> > > + *\n> > > + */\n> > > +#ifndef __VIEWFINDER_GL_H__\n> > > +#define __VIEWFINDER_GL_H__\n> > > +\n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \"viewfinder.h\"\n> > > +\n> > > +class ViewFinderGL : public QOpenGLWidget,\n> > > + public ViewFinder,\n> > > + protected QOpenGLFunctions\n> > > +{\n> > > + Q_OBJECT\n> > > +\n> > > +public:\n> > > + ViewFinderGL(QWidget *parent = nullptr);\n> > > + ~ViewFinderGL();\n> > > +\n> > > + const QList &nativeFormats() const\n> override;\n> > > +\n> > > + int setFormat(const libcamera::PixelFormat &format, const QSize\n> &size) override;\n> > > + void render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> override;\n> > > + void stop() override;\n> > > +\n> > > + QImage getCurrentImage() override;\n> > > +\n> > > +Q_SIGNALS:\n> > > + void renderComplete(libcamera::FrameBuffer *buffer);\n> > > +\n> > > +protected:\n> > > + void initializeGL() override;\n> > > + void paintGL() override;\n> > > + void resizeGL(int w, int h) override;\n> > > + QSize sizeHint() const override;\n> > > +\n> > > +private:\n> > > + bool selectFormat(const libcamera::PixelFormat &format);\n> > > +\n> > > + void configureTexture(unsigned int id);\n> > > + bool createFragmentShader();\n> > > + bool createVertexShader();\n> > > + void removeShader();\n> > > + void doRender();\n> > > +\n> > > + /* Captured image size, format and buffer */\n> > > + libcamera::FrameBuffer *buffer_;\n> > > + libcamera::PixelFormat format_;\n> > > + QSize size_;\n> > > + unsigned char *yuvData_;\n> > > +\n> > > + /* OpenGL components for rendering */\n> > > + QOpenGLShader *fragmentShader_;\n> > > + QOpenGLShader *vertexShader_;\n> > > + QOpenGLShaderProgram shaderProgram_;\n> > > +\n> > > + /* Vertex buffer */\n> > > + QOpenGLBuffer vertexBuffer_;\n> > > +\n> > > + /* Fragment and Vertex shader file name */\n> > > + QString fragmentShaderSrc_;\n> > > + QString vertexShaderSrc_;\n> > > +\n> > > + /* YUV texture planars and parameters */\n> > > + GLuint id_u_;\n> > > + GLuint id_v_;\n> > > + GLuint id_y_;\n> > > + GLuint textureUniformU_;\n> > > + GLuint textureUniformV_;\n> > > + GLuint textureUniformY_;\n> > > + QOpenGLTexture textureU_;\n> > > + QOpenGLTexture textureV_;\n> > > + QOpenGLTexture textureY_;\n> > > + unsigned int horzSubSample_;\n> > > + unsigned int vertSubSample_;\n> > > +\n> > > + QMutex mutex_; /* Prevent concurrent access to image_ */\n> > > +};\n> > > +\n> > > +#endif /* __VIEWFINDER_GL_H__ */\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A4E2DC3B5B\n\tfor ;\n\tMon, 14 Sep 2020 01:58:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1C85962DBE;\n\tMon, 14 Sep 2020 03:58:58 +0200 (CEST)","from mail-pj1-x1043.google.com (mail-pj1-x1043.google.com\n\t[IPv6:2607:f8b0:4864:20::1043])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 70C386036D\n\tfor ;\n\tMon, 14 Sep 2020 03:58:55 +0200 (CEST)","by mail-pj1-x1043.google.com with SMTP id md22so4441584pjb.0\n\tfor ;\n\tSun, 13 Sep 2020 18:58:55 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"SyPne0dr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=psGNhTi4ovxbs7zWjeoj3fSCn+BzFuCE2DoOSBCxOVI=;\n\tb=SyPne0dr3CYrBt5x4/Ztl5+e0sOK97i4yV0Az3D4XoxlMA855StY0I6yYtgDfzxGT7\n\tPhsyOo6jsddm6nS9TMYGNY//Kvxw1BTPZJekjXdoCi2p8fevTPdhJrrpB6+OKSqVl1pQ\n\twbB4wysh+rznyq4t78OYw5nBlHHmvX4Wkjd9+6uQnsWGPA6c8zjJVHDWbrEMptBI0uUO\n\trI9uYWEPGvlpc3xfSHLuWuCpgIh++Lt+y5k705WTVqcP5yajYH9AVVLhoG3B6A/euAMy\n\twT61jMTtd6L9l/Zv25I377DKsIIFpNgKId8F1ZZRunlOwTRfnYMR0vNCLjHxs1Go53At\n\tHhzw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=psGNhTi4ovxbs7zWjeoj3fSCn+BzFuCE2DoOSBCxOVI=;\n\tb=uAI0j9ZKOz9lVVtI3SCPsRX0J+reC3vRLnu3H3nPKYE+SUHARldY231apF3PMfzYGJ\n\tWPWiGQz3dKv29MlChBsf/TqZl9/b2vAbidvO/1blnQBVVf9qmXTu5CBb/gHIi03+0R2c\n\ttzy6JdEqzrcksBvANyYHk31ugnr7f1UpBtSx0WUzULQfVr9vYHSd/pxb5drl1+ZKvBbn\n\t/d3uLaJd+mYj0ChAGBKGFsfQE2hP9VuM7d0saLOliyGQL0YFRJoI8f5m0EbwyVXg4p1d\n\tPhChY6N2SEj6edn5sHD8gLzgcs6aKhx3pL4Ys0LJ8JyIVuzG82egPKALYGtsQNZfxkBH\n\tc6NA==","X-Gm-Message-State":"AOAM530cP3Rs2oLSX/cxjqRG2G/NED2nkC7fKucO/ea6YCClSFfBnFWG\n\tV/Phed8GTlj5cu048V54mAGeltnX0+KIPt20wES64BcXjmY=","X-Google-Smtp-Source":"ABdhPJwIoqTfqBaE+UyTB22KO1mhmmIo0FolCabxrODhi/HBIOTFzcZb/5O+BQg8umCFyBG/SgKKAfRxszKyB5oiDiU=","X-Received":"by 2002:a17:90b:4b0b:: with SMTP id\n\tlx11mr3479147pjb.104.1600048733388; \n\tSun, 13 Sep 2020 18:58:53 -0700 (PDT)","MIME-Version":"1.0","References":"<20200911085514.30021-1-show.liu@linaro.org>\n\t<20200911085514.30021-4-show.liu@linaro.org>\n\t<20200912020910.GM6808@pendragon.ideasonboard.com>\n\t<20200912220315.GC3933@pendragon.ideasonboard.com>","In-Reply-To":"<20200912220315.GC3933@pendragon.ideasonboard.com>","From":"Show Liu ","Date":"Mon, 14 Sep 2020 09:58:42 +0800","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v6 3/4] qcam: add viewfinderGL class\n\tto accelerate the format convert","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/mixed;\n\tboundary=\"===============8850239996702294782==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":12518,"web_url":"https://patchwork.libcamera.org/comment/12518/","msgid":"<20200915022119.GC15543@pendragon.ideasonboard.com>","date":"2020-09-15T02:21:19","subject":"Re: [libcamera-devel] [PATCH v6 3/4] qcam: add viewfinderGL class\n\tto accelerate the format convert","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Show,\n\nOn Mon, Sep 14, 2020 at 09:58:42AM +0800, Show Liu wrote:\n> On Sun, Sep 13, 2020 at 6:03 AM Laurent Pinchart wrote:\n> > On Sat, Sep 12, 2020 at 05:09:10AM +0300, Laurent Pinchart wrote:\n> > > Hi Show,\n> > >\n> > > Thank you for the patch.\n> > >\n> > > In the subject line, s/viewfinderGL/ViewFinderGL/ and\n> > > s/convert/conversion/\n> > >\n> > > On Fri, Sep 11, 2020 at 04:55:13PM +0800, Show Liu wrote:\n> > > > the viewfinderGL accelerates the format conversion by\n> > > > using OpenGL ES shader\n> > >\n> > > I would add\n> > >\n> > > \"The minimum Qt version is bumped to v5.4, as QOpenGLWidget wasn't\n> > > available before that.\"\n> > >\n> > > > Signed-off-by: Show Liu \n> > > > ---\n> > > > meson.build | 1 +\n> > > > src/qcam/meson.build | 17 +-\n> > > > src/qcam/viewfinder_gl.cpp | 456 +++++++++++++++++++++++++++++++++++++\n> > > > src/qcam/viewfinder_gl.h | 96 ++++++++\n> > > > 4 files changed, 568 insertions(+), 2 deletions(-)\n> > > > create mode 100644 src/qcam/viewfinder_gl.cpp\n> > > > create mode 100644 src/qcam/viewfinder_gl.h\n> > > >\n> > > > diff --git a/meson.build b/meson.build\n> > > > index 1ea35e9..c58d458 100644\n> > > > --- a/meson.build\n> > > > +++ b/meson.build\n> > > > @@ -26,6 +26,7 @@ libcamera_version = libcamera_git_version.split('+')[0]\n> > > >\n> > > > # Configure the build environment.\n> > > > cc = meson.get_compiler('c')\n> > > > +cxx = meson.get_compiler('cpp')\n> > > > config_h = configuration_data()\n> > > >\n> > > > if cc.has_header_symbol('execinfo.h', 'backtrace')\n> > > > diff --git a/src/qcam/meson.build b/src/qcam/meson.build\n> > > > index a4bad0a..9bb48c0 100644\n> > > > --- a/src/qcam/meson.build\n> > > > +++ b/src/qcam/meson.build\n> > > > @@ -16,14 +16,14 @@ qcam_moc_headers = files([\n> > > >\n> > > > qcam_resources = files([\n> > > > 'assets/feathericons/feathericons.qrc',\n> > > > - 'assets/shader/shaders.qrc'\n> > > > ])\n> > > >\n> > > > qt5 = import('qt5')\n> > > > qt5_dep = dependency('qt5',\n> > > > method : 'pkg-config',\n> > > > modules : ['Core', 'Gui', 'Widgets'],\n> > > > - required : get_option('qcam'))\n> > > > + required : get_option('qcam'),\n> > > > + version : '>=5.4')\n> > > >\n> > > > if qt5_dep.found()\n> > > > qcam_deps = [\n> > > > @@ -42,6 +42,19 @@ if qt5_dep.found()\n> > > > ])\n> > > > endif\n> > > >\n> > > > + if cxx.has_header_symbol('QOpenGLWidget', 'QOpenGLWidget',\n> > > > + dependencies : qt5_dep, args : '-fPIC')\n> > > > + qcam_sources += files([\n> > > > + 'viewfinder_gl.cpp',\n> > > > + ])\n> > > > + qcam_moc_headers += files([\n> > > > + 'viewfinder_gl.h',\n> > > > + ])\n> > > > + qcam_resources += files([\n> > > > + 'assets/shader/shaders.qrc'\n> > > > + ])\n> > > > + endif\n> > > > +\n> > > > # gcc 9 introduced a deprecated-copy warning that is triggered by Qt until\n> > > > # Qt 5.13. clang 10 introduced the same warning, but detects more issues\n> > > > # that are not fixed in Qt yet. Disable the warning manually in both cases.\n> > > > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> > > > new file mode 100644\n> > > > index 0000000..84f4866\n> > > > --- /dev/null\n> > > > +++ b/src/qcam/viewfinder_gl.cpp\n> > > > @@ -0,0 +1,456 @@\n> > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > +/*\n> > > > + * Copyright (C) 2020, Linaro\n> > > > + *\n> > > > + * viewfinderGL.cpp - OpenGL Viewfinder for rendering by OpenGL shader\n> > > > + */\n> > > > +\n> > > > +#include \"viewfinder_gl.h\"\n> > > > +\n> > > > +#include \n> > > > +\n> > > > +#include \n> > > > +\n> > > > +static const QList supportedFormats{\n> > > > + libcamera::formats::NV12,\n> > > > + libcamera::formats::NV21,\n> > > > + libcamera::formats::NV16,\n> > > > + libcamera::formats::NV61,\n> > > > + libcamera::formats::NV24,\n> > > > + libcamera::formats::NV42,\n> > > > + libcamera::formats::YUV420,\n> > > > + libcamera::formats::YVU420\n> > > > +};\n> > > > +\n> > > > +ViewFinderGL::ViewFinderGL(QWidget *parent)\n> > > > + : QOpenGLWidget(parent), buffer_(nullptr), yuvData_(nullptr),\n> > > > + fragmentShader_(nullptr), vertexShader_(nullptr),\n> > > > + vertexBuffer_(QOpenGLBuffer::VertexBuffer),\n> > > > + textureU_(QOpenGLTexture::Target2D),\n> > > > + textureV_(QOpenGLTexture::Target2D),\n> > > > + textureY_(QOpenGLTexture::Target2D)\n> > > > +{\n> > > > +}\n> > > > +\n> > > > +ViewFinderGL::~ViewFinderGL()\n> > > > +{\n> > > > + removeShader();\n> > > > +\n> > > > + if (vertexBuffer_.isCreated())\n> > > > + vertexBuffer_.destroy();\n> > >\n> > > I think the QOpenGLBuffer destructor destroys the buffer, you don't need\n> > > this.\n> > >\n> > > > +}\n> > > > +\n> > > > +const QList &ViewFinderGL::nativeFormats()\n> > const\n> > > > +{\n> > > > + return supportedFormats;\n> > > > +}\n> > > > +\n> > > > +int ViewFinderGL::setFormat(const libcamera::PixelFormat &format,\n> > > > + const QSize &size)\n> > > > +{\n> > > > + int ret = 0;\n> > > > +\n> > > > + /* If the fragment is ceeated remove it and create a new one */\n> > >\n> > > s/ceeated/created/\n> > >\n> > > > + if (fragmentShader_) {\n> > > > + if (shaderProgram_.isLinked()) {\n> > > > + shaderProgram_.release();\n> > > > + shaderProgram_.removeShader(fragmentShader_);\n> > > > + delete fragmentShader_;\n> > > > + }\n> > > > + }\n> > > > +\n> > > > + if (selectFormat(format)) {\n> > > > + format_ = format;\n> > > > + size_ = size;\n> > > > + } else {\n> > > > + ret = -1;\n> > > > + }\n> > > > + updateGeometry();\n> > > > + return ret;\n> > >\n> > > We tend to exit early in case of errors (and updateGeometry() shouldn't\n> > > be called in that case):\n> > >\n> > > if (!selectFormat(format))\n> > > return -1;\n> > >\n> > > format_ = format;\n> > > size_ = size;\n> > >\n> > > updateGeometry();\n> > > return 0;\n> > >\n> > > > +}\n> > > > +\n> > > > +void ViewFinderGL::stop()\n> > > > +{\n> > > > + if (buffer_) {\n> > > > + renderComplete(buffer_);\n> > > > + buffer_ = nullptr;\n> > > > + }\n> > > > +}\n> > > > +\n> > > > +QImage ViewFinderGL::getCurrentImage()\n> > > > +{\n> > > > + QMutexLocker locker(&mutex_);\n> > > > +\n> > > > + return grabFramebuffer();\n> > > > +}\n> > > > +\n> > > > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)\n> > > > +{\n> > > > + if (buffer->planes().size() != 1) {\n> > > > + qWarning() << \"Multi-planar buffers are not supported\";\n> > > > + return;\n> > > > + }\n> > > > +\n> > > > + if (buffer_)\n> > > > + renderComplete(buffer_);\n> > > > +\n> > > > + yuvData_ = static_cast(map->memory);\n> > > > + update();\n> > > > + buffer_ = buffer;\n> > > > +}\n> > > > +\n> > > > +bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)\n> > > > +{\n> > > > + bool ret = true;\n> > > > + switch (format) {\n> > > > + case libcamera::formats::NV12:\n> > > > + horzSubSample_ = 2;\n> > > > + vertSubSample_ = 2;\n> > > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > > + fragmentShaderSrc_ = \":NV_2_planes_UV_f.glsl\";\n> > > > + break;\n> > > > + case libcamera::formats::NV21:\n> > > > + horzSubSample_ = 2;\n> > > > + vertSubSample_ = 2;\n> > > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > > + fragmentShaderSrc_ = \":NV_2_planes_VU_f.glsl\";\n> > > > + break;\n> > > > + case libcamera::formats::NV16:\n> > > > + horzSubSample_ = 2;\n> > > > + vertSubSample_ = 1;\n> > > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > > + fragmentShaderSrc_ = \":NV_2_planes_UV_f.glsl\";\n> > > > + break;\n> > > > + case libcamera::formats::NV61:\n> > > > + horzSubSample_ = 2;\n> > > > + vertSubSample_ = 1;\n> > > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > > + fragmentShaderSrc_ = \":NV_2_planes_VU_f.glsl\";\n> > > > + break;\n> > > > + case libcamera::formats::NV24:\n> > > > + horzSubSample_ = 1;\n> > > > + vertSubSample_ = 1;\n> > > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > > + fragmentShaderSrc_ = \":NV_2_planes_UV_f.glsl\";\n> > > > + break;\n> > > > + case libcamera::formats::NV42:\n> > > > + horzSubSample_ = 1;\n> > > > + vertSubSample_ = 1;\n> > > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > > + fragmentShaderSrc_ = \":NV_2_planes_VU_f.glsl\";\n> > > > + break;\n> > > > + case libcamera::formats::YUV420:\n> > > > + horzSubSample_ = 2;\n> > > > + vertSubSample_ = 2;\n> > > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > > + fragmentShaderSrc_ = \":NV_3_planes_f.glsl\";\n> > > > + break;\n> > > > + case libcamera::formats::YVU420:\n> > > > + horzSubSample_ = 2;\n> > > > + vertSubSample_ = 2;\n> > > > + vertexShaderSrc_ = \":NV_vertex_shader.glsl\";\n> > > > + fragmentShaderSrc_ = \":NV_3_planes_f.glsl\";\n> > > > + break;\n> > > > + default:\n> > > > + ret = false;\n> > > > + qWarning() << \"[ViewFinderGL]:\"\n> > > > + << \"format not supported.\";\n> > > > + break;\n> > > > + };\n> > > > +\n> > > > + return ret;\n> > > > +}\n> > > > +\n> > > > +bool ViewFinderGL::createVertexShader()\n> > > > +{\n> > > > + /* Create Vertex Shader */\n> > > > + vertexShader_ = new QOpenGLShader(QOpenGLShader::Vertex, this);\n> > > > +\n> > > > + /* Compile the vertex shader */\n> > > > + if (!vertexShader_->compileSourceFile(vertexShaderSrc_)) {\n> > > > + qWarning() << \"[ViewFinderGL]:\" << vertexShader_->log();\n> > > > + return false;\n> > > > + }\n> > > > +\n> > > > + shaderProgram_.addShader(vertexShader_);\n> > > > + return true;\n> > > > +}\n> > > > +\n> > > > +bool ViewFinderGL::createFragmentShader()\n> > > > +{\n> > > > + int attributeVertex;\n> > > > + int attributeTexture;\n> > > > +\n> > > > + /* Create Fragment Shader */\n> > > > + fragmentShader_ = new QOpenGLShader(QOpenGLShader::Fragment, this);\n> > > > +\n> > > > + /* Compile the fragment shader */\n> > > > + if (!fragmentShader_->compileSourceFile(fragmentShaderSrc_)) {\n> > > > + qWarning() << \"[ViewFinderGL]:\" << fragmentShader_->log();\n> > > > + return false;\n> > > > + }\n> > > > +\n> > > > + shaderProgram_.addShader(fragmentShader_);\n> > > > +\n> > > > + /* Link shader pipeline */\n> > > > + if (!shaderProgram_.link()) {\n> > > > + qWarning() << \"[ViewFinderGL]:\" << shaderProgram_.log();\n> > > > + close();\n> > > > + }\n> > > > +\n> > > > + /* Bind shader pipeline for use */\n> > > > + if (!shaderProgram_.bind()) {\n> > > > + qWarning() << \"[ViewFinderGL]:\" << shaderProgram_.log();\n> > > > + close();\n> > > > + }\n> > > > +\n> > > > + attributeVertex = shaderProgram_.attributeLocation(\"vertexIn\");\n> > > > + attributeTexture = shaderProgram_.attributeLocation(\"textureIn\");\n> > > > +\n> > > > + shaderProgram_.enableAttributeArray(attributeVertex);\n> > > > + shaderProgram_.setAttributeBuffer(attributeVertex,\n> > > > + GL_FLOAT,\n> > > > + 0,\n> > > > + 2,\n> > > > + 2 * sizeof(GLfloat));\n> > > > +\n> > > > + shaderProgram_.enableAttributeArray(attributeTexture);\n> > > > + shaderProgram_.setAttributeBuffer(attributeTexture,\n> > > > + GL_FLOAT,\n> > > > + 8 * sizeof(GLfloat),\n> > > > + 2,\n> > > > + 2 * sizeof(GLfloat));\n> > > > +\n> > > > + textureUniformY_ = shaderProgram_.uniformLocation(\"tex_y\");\n> > > > + textureUniformU_ = shaderProgram_.uniformLocation(\"tex_u\");\n> > > > + textureUniformV_ = shaderProgram_.uniformLocation(\"tex_v\");\n> > > > +\n> > > > + if (!textureY_.isCreated())\n> > > > + textureY_.create();\n> > > > +\n> > > > + if (!textureU_.isCreated())\n> > > > + textureU_.create();\n> > > > +\n> > > > + if (!textureV_.isCreated())\n> > > > + textureV_.create();\n> > > > +\n> > > > + id_y_ = textureY_.textureId();\n> > > > + id_u_ = textureU_.textureId();\n> > > > + id_v_ = textureV_.textureId();\n> > > > + return true;\n> > > > +}\n> > > > +\n> > > > +void ViewFinderGL::configureTexture(unsigned int id)\n> > > > +{\n> > > > + glBindTexture(GL_TEXTURE_2D, id);\n> > > > + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);\n> > > > + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);\n> > > > + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);\n> > > > + glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);\n> > > > +}\n> > > > +\n> > > > +void ViewFinderGL::removeShader()\n> > > > +{\n> > > > + if (shaderProgram_.isLinked()) {\n> > > > + shaderProgram_.release();\n> > > > + shaderProgram_.removeAllShaders();\n> > > > + }\n> > > > +\n> > > > + if (fragmentShader_)\n> > > > + delete fragmentShader_;\n> > > > +\n> > > > + if (vertexShader_)\n> > > > + delete vertexShader_;\n> > > > +}\n> > > > +\n> > > > +void ViewFinderGL::initializeGL()\n> > > > +{\n> > > > + initializeOpenGLFunctions();\n> > > > + glEnable(GL_TEXTURE_2D);\n> > > > + glDisable(GL_DEPTH_TEST);\n> > > > +\n> > > > + static const GLfloat coordinates[2][4][2]{\n> > > > + {\n> > > > + /* Vertex coordinates */\n> > > > + { -1.0f, -1.0f },\n> > > > + { -1.0f, +1.0f },\n> > > > + { +1.0f, +1.0f },\n> > > > + { +1.0f, -1.0f },\n> > > > + },\n> > > > + {\n> > > > + /* Texture coordinates */\n> > > > + { 1.0f, 0.0f },\n> > > > + { 1.0f, 1.0f },\n> > > > + { 0.0f, 1.0f },\n> > > > + { 0.0f, 0.0f },\n> > >\n> > > I *think* this should be\n> > >\n> > > { 0.0f, 1.0f },\n> > > { 0.0f, 0.0f },\n> > > { 1.0f, 0.0f },\n> > > { 1.0f, 1.0f },\n> > >\n> > > I'll test it and try to understand :-)\n> >\n> > I confirm that the original patch rotates the image by 180° for me,\n> > while the proposed values above show it in the right orientation. I've\n> > compared -r qt and -r gles to check that.\n>\n> Are you going to apply above value to the v7 too? If so I will not send out\n> the patch to fix it.\n\nYes. I'll send the v7 shortly.\n\n> > > > + },\n> > > > + };\n> > > > +\n> > > > + vertexBuffer_.create();\n> > > > + vertexBuffer_.bind();\n> > > > + vertexBuffer_.allocate(coordinates, sizeof(coordinates));\n> > > > +\n> > > > + /* Create Vertex Shader */\n> > > > + if (!createVertexShader())\n> > > > + qWarning() << \"[ViewFinderGL]: create vertex shader failed.\";\n> > > > +\n> > > > + glClearColor(1.0f, 1.0f, 1.0f, 0.0f);\n> > > > +}\n> > > > +\n> > > > +void ViewFinderGL::doRender()\n> > > > +{\n> > > > + switch (format_) {\n> > > > + case libcamera::formats::NV12:\n> > > > + case libcamera::formats::NV21:\n> > > > + case libcamera::formats::NV16:\n> > > > + case libcamera::formats::NV61:\n> > > > + case libcamera::formats::NV24:\n> > > > + case libcamera::formats::NV42:\n> > > > + /* activate texture Y */\n> > >\n> > > s/activate/Activate/ (and similarly below).\n> > >\n> > > > + glActiveTexture(GL_TEXTURE0);\n> > > > + configureTexture(id_y_);\n> > > > + glTexImage2D(GL_TEXTURE_2D,\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + size_.width(),\n> > > > + size_.height(),\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + GL_UNSIGNED_BYTE,\n> > > > + yuvData_);\n> > > > + shaderProgram_.setUniformValue(textureUniformY_, 0);\n> > > > +\n> > > > + /* activate texture UV/VU */\n> > > > + glActiveTexture(GL_TEXTURE1);\n> > > > + configureTexture(id_u_);\n> > > > + glTexImage2D(GL_TEXTURE_2D,\n> > > > + 0,\n> > > > + GL_RG,\n> > > > + size_.width() / horzSubSample_,\n> > > > + size_.height() / vertSubSample_,\n> > > > + 0,\n> > > > + GL_RG,\n> > > > + GL_UNSIGNED_BYTE,\n> > > > + (char *)yuvData_ + size_.width() * size_.height());\n> > > > + shaderProgram_.setUniformValue(textureUniformU_, 1);\n> > > > + break;\n> > > > +\n> > > > + case libcamera::formats::YUV420:\n> > > > + /* activate texture Y */\n> > > > + glActiveTexture(GL_TEXTURE0);\n> > > > + configureTexture(id_y_);\n> > > > + glTexImage2D(GL_TEXTURE_2D,\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + size_.width(),\n> > > > + size_.height(),\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + GL_UNSIGNED_BYTE,\n> > > > + yuvData_);\n> > > > + shaderProgram_.setUniformValue(textureUniformY_, 0);\n> > > > +\n> > > > + /* activate texture U */\n> > > > + glActiveTexture(GL_TEXTURE1);\n> > > > + configureTexture(id_u_);\n> > > > + glTexImage2D(GL_TEXTURE_2D,\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + size_.width() / horzSubSample_,\n> > > > + size_.height() / vertSubSample_,\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + GL_UNSIGNED_BYTE,\n> > > > + (char *)yuvData_ + size_.width() * size_.height());\n> > > > + shaderProgram_.setUniformValue(textureUniformU_, 1);\n> > > > +\n> > > > + /* activate texture V */\n> > > > + glActiveTexture(GL_TEXTURE2);\n> > > > + configureTexture(id_v_);\n> > > > + glTexImage2D(GL_TEXTURE_2D,\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + size_.width() / horzSubSample_,\n> > > > + size_.height() / vertSubSample_,\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + GL_UNSIGNED_BYTE,\n> > > > + (char *)yuvData_ + size_.width() * size_.height() * 5 / 4);\n> > > > + shaderProgram_.setUniformValue(textureUniformV_, 2);\n> > > > + break;\n> > > > +\n> > > > + case libcamera::formats::YVU420:\n> > > > + /* activate texture Y */\n> > > > + glActiveTexture(GL_TEXTURE0);\n> > > > + configureTexture(id_y_);\n> > > > + glTexImage2D(GL_TEXTURE_2D,\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + size_.width(),\n> > > > + size_.height(),\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + GL_UNSIGNED_BYTE,\n> > > > + yuvData_);\n> > > > + shaderProgram_.setUniformValue(textureUniformY_, 0);\n> > > > +\n> > > > + /* activate texture V */\n> > > > + glActiveTexture(GL_TEXTURE2);\n> > > > + configureTexture(id_v_);\n> > > > + glTexImage2D(GL_TEXTURE_2D,\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + size_.width() / horzSubSample_,\n> > > > + size_.height() / vertSubSample_,\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + GL_UNSIGNED_BYTE,\n> > > > + (char *)yuvData_ + size_.width() * size_.height());\n> > > > + shaderProgram_.setUniformValue(textureUniformV_, 1);\n> > >\n> > > I don't think this is correct. GL_TEXTURE1 stores the U plane, and you\n> > > assign it to tex_v, which is the V plane in the shader.\n> > >\n> > > There are two options, either s/1/2/ or s/GL_TEXTURE2/GL_TEXTURE1/ (and\n> > > the other way below).\n> > >\n> > > With these small issues addressed,\n> > >\n> > > Reviewed-by: Laurent Pinchart \n> > >\n> > > I can also fix these when applying the patch, but given that there are\n> > > quite a few issues, I would then send a v7 to the list to make sure I\n> > > haven't done anything wrong.\n> > >\n> > > > +\n> > > > + /* activate texture U */\n> > > > + glActiveTexture(GL_TEXTURE1);\n> > > > + configureTexture(id_u_);\n> > > > + glTexImage2D(GL_TEXTURE_2D,\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + size_.width() / horzSubSample_,\n> > > > + size_.height() / vertSubSample_,\n> > > > + 0,\n> > > > + GL_RED,\n> > > > + GL_UNSIGNED_BYTE,\n> > > > + (char *)yuvData_ + size_.width() * size_.height() * 5 / 4);\n> > > > + shaderProgram_.setUniformValue(textureUniformU_, 2);\n> > > > + break;\n> > > > +\n> > > > + default:\n> > > > + break;\n> > > > + };\n> > > > +}\n> > > > +\n> > > > +void ViewFinderGL::paintGL()\n> > > > +{\n> > > > + if (!fragmentShader_)\n> > > > + if (!createFragmentShader()) {\n> > > > + qWarning() << \"[ViewFinderGL]:\"\n> > > > + << \"create fragment shader failed.\";\n> > > > + }\n> > > > +\n> > > > + if (yuvData_) {\n> > > > + glClearColor(0.0, 0.0, 0.0, 1.0);\n> > > > + glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);\n> > > > +\n> > > > + doRender();\n> > > > + glDrawArrays(GL_TRIANGLE_FAN, 0, 4);\n> > > > + }\n> > > > +}\n> > > > +\n> > > > +void ViewFinderGL::resizeGL(int w, int h)\n> > > > +{\n> > > > + glViewport(0, 0, w, h);\n> > > > +}\n> > > > +\n> > > > +QSize ViewFinderGL::sizeHint() const\n> > > > +{\n> > > > + return size_.isValid() ? size_ : QSize(640, 480);\n> > > > +}\n> > > > diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h\n> > > > new file mode 100644\n> > > > index 0000000..69502b7\n> > > > --- /dev/null\n> > > > +++ b/src/qcam/viewfinder_gl.h\n> > > > @@ -0,0 +1,96 @@\n> > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > > +/*\n> > > > + * Copyright (C) 2020, Linaro\n> > > > + *\n> > > > + * viewfinder_GL.h - OpenGL Viewfinder for rendering by OpenGL shader\n> > > > + *\n> > > > + */\n> > > > +#ifndef __VIEWFINDER_GL_H__\n> > > > +#define __VIEWFINDER_GL_H__\n> > > > +\n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +#include \n> > > > +\n> > > > +#include \n> > > > +#include \n> > > > +\n> > > > +#include \"viewfinder.h\"\n> > > > +\n> > > > +class ViewFinderGL : public QOpenGLWidget,\n> > > > + public ViewFinder,\n> > > > + protected QOpenGLFunctions\n> > > > +{\n> > > > + Q_OBJECT\n> > > > +\n> > > > +public:\n> > > > + ViewFinderGL(QWidget *parent = nullptr);\n> > > > + ~ViewFinderGL();\n> > > > +\n> > > > + const QList &nativeFormats() const override;\n> > > > +\n> > > > + int setFormat(const libcamera::PixelFormat &format, const QSize &size) override;\n> > > > + void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;\n> > > > + void stop() override;\n> > > > +\n> > > > + QImage getCurrentImage() override;\n> > > > +\n> > > > +Q_SIGNALS:\n> > > > + void renderComplete(libcamera::FrameBuffer *buffer);\n> > > > +\n> > > > +protected:\n> > > > + void initializeGL() override;\n> > > > + void paintGL() override;\n> > > > + void resizeGL(int w, int h) override;\n> > > > + QSize sizeHint() const override;\n> > > > +\n> > > > +private:\n> > > > + bool selectFormat(const libcamera::PixelFormat &format);\n> > > > +\n> > > > + void configureTexture(unsigned int id);\n> > > > + bool createFragmentShader();\n> > > > + bool createVertexShader();\n> > > > + void removeShader();\n> > > > + void doRender();\n> > > > +\n> > > > + /* Captured image size, format and buffer */\n> > > > + libcamera::FrameBuffer *buffer_;\n> > > > + libcamera::PixelFormat format_;\n> > > > + QSize size_;\n> > > > + unsigned char *yuvData_;\n> > > > +\n> > > > + /* OpenGL components for rendering */\n> > > > + QOpenGLShader *fragmentShader_;\n> > > > + QOpenGLShader *vertexShader_;\n> > > > + QOpenGLShaderProgram shaderProgram_;\n> > > > +\n> > > > + /* Vertex buffer */\n> > > > + QOpenGLBuffer vertexBuffer_;\n> > > > +\n> > > > + /* Fragment and Vertex shader file name */\n> > > > + QString fragmentShaderSrc_;\n> > > > + QString vertexShaderSrc_;\n> > > > +\n> > > > + /* YUV texture planars and parameters */\n> > > > + GLuint id_u_;\n> > > > + GLuint id_v_;\n> > > > + GLuint id_y_;\n> > > > + GLuint textureUniformU_;\n> > > > + GLuint textureUniformV_;\n> > > > + GLuint textureUniformY_;\n> > > > + QOpenGLTexture textureU_;\n> > > > + QOpenGLTexture textureV_;\n> > > > + QOpenGLTexture textureY_;\n> > > > + unsigned int horzSubSample_;\n> > > > + unsigned int vertSubSample_;\n> > > > +\n> > > > + QMutex mutex_; /* Prevent concurrent access to image_ */\n> > > > +};\n> > > > +\n> > > > +#endif /* __VIEWFINDER_GL_H__ */","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id C8172BF01C\n\tfor ;\n\tTue, 15 Sep 2020 02:21:55 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 40ADE62DAB;\n\tTue, 15 Sep 2020 04:21:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AB42562901\n\tfor ;\n\tTue, 15 Sep 2020 04:21:53 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 32F44275;\n\tTue, 15 Sep 2020 04:21:47 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Jx/U0B5c\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1600136507;\n\tbh=oV0vW80zN/be0Sx3gu7aPwOOMa0exP6d635fDpkTpdg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Jx/U0B5c8alZhI0jwj9Du+zlormzOXzV4IuMFeFHPduYK4KcyxCh6oD0vKhHraMi6\n\tqlBSL1POhOpxz67cy/Jo4WdilwHa5omssxN9Wc2AICMKvFvHIIO8SMO38lqqpOFvb7\n\tPxKjgjKH5iDXSkgU0m3ehBhi5kGtEc9Nvu4pzh/0=","Date":"Tue, 15 Sep 2020 05:21:19 +0300","From":"Laurent Pinchart ","To":"Show Liu ","Message-ID":"<20200915022119.GC15543@pendragon.ideasonboard.com>","References":"<20200911085514.30021-1-show.liu@linaro.org>\n\t<20200911085514.30021-4-show.liu@linaro.org>\n\t<20200912020910.GM6808@pendragon.ideasonboard.com>\n\t<20200912220315.GC3933@pendragon.ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v6 3/4] qcam: add viewfinderGL class\n\tto accelerate the format convert","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]