<div dir="ltr"><div dir="ltr">Hi Laurent,<div><br clear="all"><div><div dir="ltr" data-smartmail="gmail_signature"><div dir="ltr"></div></div></div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Sun, Sep 13, 2020 at 6:03 AM Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Hi Show,<br>
<br>
On Sat, Sep 12, 2020 at 05:09:10AM +0300, Laurent Pinchart wrote:<br>
> Hi Show,<br>
> <br>
> Thank you for the patch.<br>
> <br>
> In the subject line, s/viewfinderGL/ViewFinderGL/ and<br>
> s/convert/conversion/<br>
> <br>
> On Fri, Sep 11, 2020 at 04:55:13PM +0800, Show Liu wrote:<br>
> > the viewfinderGL accelerates the format conversion by<br>
> > using OpenGL ES shader<br>
> <br>
> I would add<br>
> <br>
> "The minimum Qt version is bumped to v5.4, as QOpenGLWidget wasn't<br>
> available before that."<br>
> <br>
> > Signed-off-by: Show Liu <<a href="mailto:show.liu@linaro.org" target="_blank">show.liu@linaro.org</a>><br>
> > ---<br>
> >  meson.build                |   1 +<br>
> >  src/qcam/meson.build       |  17 +-<br>
> >  src/qcam/viewfinder_gl.cpp | 456 +++++++++++++++++++++++++++++++++++++<br>
> >  src/qcam/viewfinder_gl.h   |  96 ++++++++<br>
> >  4 files changed, 568 insertions(+), 2 deletions(-)<br>
> >  create mode 100644 src/qcam/viewfinder_gl.cpp<br>
> >  create mode 100644 src/qcam/viewfinder_gl.h<br>
> > <br>
> > diff --git a/meson.build b/meson.build<br>
> > index 1ea35e9..c58d458 100644<br>
> > --- a/meson.build<br>
> > +++ b/meson.build<br>
> > @@ -26,6 +26,7 @@ libcamera_version = libcamera_git_version.split('+')[0]<br>
> >  <br>
> >  # Configure the build environment.<br>
> >  cc = meson.get_compiler('c')<br>
> > +cxx = meson.get_compiler('cpp')<br>
> >  config_h = configuration_data()<br>
> >  <br>
> >  if cc.has_header_symbol('execinfo.h', 'backtrace')<br>
> > diff --git a/src/qcam/meson.build b/src/qcam/meson.build<br>
> > index a4bad0a..9bb48c0 100644<br>
> > --- a/src/qcam/meson.build<br>
> > +++ b/src/qcam/meson.build<br>
> > @@ -16,14 +16,14 @@ qcam_moc_headers = files([<br>
> >  <br>
> >  qcam_resources = files([<br>
> >      'assets/feathericons/feathericons.qrc',<br>
> > -    'assets/shader/shaders.qrc'<br>
> >  ])<br>
> >  <br>
> >  qt5 = import('qt5')<br>
> >  qt5_dep = dependency('qt5',<br>
> >                       method : 'pkg-config',<br>
> >                       modules : ['Core', 'Gui', 'Widgets'],<br>
> > -                     required : get_option('qcam'))<br>
> > +                     required : get_option('qcam'),<br>
> > +                     version : '>=5.4')<br>
> >  <br>
> >  if qt5_dep.found()<br>
> >      qcam_deps = [<br>
> > @@ -42,6 +42,19 @@ if qt5_dep.found()<br>
> >          ])<br>
> >      endif<br>
> >  <br>
> > +    if cxx.has_header_symbol('QOpenGLWidget', 'QOpenGLWidget',<br>
> > +                             dependencies : qt5_dep, args : '-fPIC')<br>
> > +        qcam_sources += files([<br>
> > +            'viewfinder_gl.cpp',<br>
> > +        ])<br>
> > +        qcam_moc_headers += files([<br>
> > +            'viewfinder_gl.h',<br>
> > +        ])<br>
> > +        qcam_resources += files([<br>
> > +            'assets/shader/shaders.qrc'<br>
> > +        ])<br>
> > +    endif<br>
> > +<br>
> >      # gcc 9 introduced a deprecated-copy warning that is triggered by Qt until<br>
> >      # Qt 5.13. clang 10 introduced the same warning, but detects more issues<br>
> >      # that are not fixed in Qt yet. Disable the warning manually in both cases.<br>
> > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp<br>
> > new file mode 100644<br>
> > index 0000000..84f4866<br>
> > --- /dev/null<br>
> > +++ b/src/qcam/viewfinder_gl.cpp<br>
> > @@ -0,0 +1,456 @@<br>
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> > +/*<br>
> > + * Copyright (C) 2020, Linaro<br>
> > + *<br>
> > + * viewfinderGL.cpp - OpenGL Viewfinder for rendering by OpenGL shader<br>
> > + */<br>
> > +<br>
> > +#include "viewfinder_gl.h"<br>
> > +<br>
> > +#include <QImage><br>
> > +<br>
> > +#include <libcamera/formats.h><br>
> > +<br>
> > +static const QList<libcamera::PixelFormat> supportedFormats{<br>
> > +   libcamera::formats::NV12,<br>
> > +   libcamera::formats::NV21,<br>
> > +   libcamera::formats::NV16,<br>
> > +   libcamera::formats::NV61,<br>
> > +   libcamera::formats::NV24,<br>
> > +   libcamera::formats::NV42,<br>
> > +   libcamera::formats::YUV420,<br>
> > +   libcamera::formats::YVU420<br>
> > +};<br>
> > +<br>
> > +ViewFinderGL::ViewFinderGL(QWidget *parent)<br>
> > +   : QOpenGLWidget(parent), buffer_(nullptr), yuvData_(nullptr),<br>
> > +     fragmentShader_(nullptr), vertexShader_(nullptr),<br>
> > +     vertexBuffer_(QOpenGLBuffer::VertexBuffer),<br>
> > +     textureU_(QOpenGLTexture::Target2D),<br>
> > +     textureV_(QOpenGLTexture::Target2D),<br>
> > +     textureY_(QOpenGLTexture::Target2D)<br>
> > +{<br>
> > +}<br>
> > +<br>
> > +ViewFinderGL::~ViewFinderGL()<br>
> > +{<br>
> > +   removeShader();<br>
> > +<br>
> > +   if (vertexBuffer_.isCreated())<br>
> > +           vertexBuffer_.destroy();<br>
> <br>
> I think the QOpenGLBuffer destructor destroys the buffer, you don't need<br>
> this.<br>
> <br>
> > +}<br>
> > +<br>
> > +const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const<br>
> > +{<br>
> > +   return supportedFormats;<br>
> > +}<br>
> > +<br>
> > +int ViewFinderGL::setFormat(const libcamera::PixelFormat &format,<br>
> > +                       const QSize &size)<br>
> > +{<br>
> > +   int ret = 0;<br>
> > +<br>
> > +   /* If the fragment is ceeated remove it and create a new one */<br>
> <br>
> s/ceeated/created/<br>
> <br>
> > +   if (fragmentShader_) {<br>
> > +           if (shaderProgram_.isLinked()) {<br>
> > +                   shaderProgram_.release();<br>
> > +                   shaderProgram_.removeShader(fragmentShader_);<br>
> > +                   delete fragmentShader_;<br>
> > +           }<br>
> > +   }<br>
> > +<br>
> > +   if (selectFormat(format)) {<br>
> > +           format_ = format;<br>
> > +           size_ = size;<br>
> > +   } else {<br>
> > +           ret = -1;<br>
> > +   }<br>
> > +   updateGeometry();<br>
> > +   return ret;<br>
> <br>
> We tend to exit early in case of errors (and updateGeometry() shouldn't<br>
> be called in that case):<br>
> <br>
>       if (!selectFormat(format))<br>
>               return -1;<br>
> <br>
>       format_ = format;<br>
>       size_ = size;<br>
> <br>
>       updateGeometry();<br>
>       return 0;<br>
> <br>
> > +}<br>
> > +<br>
> > +void ViewFinderGL::stop()<br>
> > +{<br>
> > +   if (buffer_) {<br>
> > +           renderComplete(buffer_);<br>
> > +           buffer_ = nullptr;<br>
> > +   }<br>
> > +}<br>
> > +<br>
> > +QImage ViewFinderGL::getCurrentImage()<br>
> > +{<br>
> > +   QMutexLocker locker(&mutex_);<br>
> > +<br>
> > +   return grabFramebuffer();<br>
> > +}<br>
> > +<br>
> > +void ViewFinderGL::render(libcamera::FrameBuffer *buffer, MappedBuffer *map)<br>
> > +{<br>
> > +   if (buffer->planes().size() != 1) {<br>
> > +           qWarning() << "Multi-planar buffers are not supported";<br>
> > +           return;<br>
> > +   }<br>
> > +<br>
> > +   if (buffer_)<br>
> > +           renderComplete(buffer_);<br>
> > +<br>
> > +   yuvData_ = static_cast<unsigned char *>(map->memory);<br>
> > +   update();<br>
> > +   buffer_ = buffer;<br>
> > +}<br>
> > +<br>
> > +bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)<br>
> > +{<br>
> > +   bool ret = true;<br>
> > +   switch (format) {<br>
> > +   case libcamera::formats::NV12:<br>
> > +           horzSubSample_ = 2;<br>
> > +           vertSubSample_ = 2;<br>
> > +           vertexShaderSrc_ = ":NV_vertex_shader.glsl";<br>
> > +           fragmentShaderSrc_ = ":NV_2_planes_UV_f.glsl";<br>
> > +           break;<br>
> > +   case libcamera::formats::NV21:<br>
> > +           horzSubSample_ = 2;<br>
> > +           vertSubSample_ = 2;<br>
> > +           vertexShaderSrc_ = ":NV_vertex_shader.glsl";<br>
> > +           fragmentShaderSrc_ = ":NV_2_planes_VU_f.glsl";<br>
> > +           break;<br>
> > +   case libcamera::formats::NV16:<br>
> > +           horzSubSample_ = 2;<br>
> > +           vertSubSample_ = 1;<br>
> > +           vertexShaderSrc_ = ":NV_vertex_shader.glsl";<br>
> > +           fragmentShaderSrc_ = ":NV_2_planes_UV_f.glsl";<br>
> > +           break;<br>
> > +   case libcamera::formats::NV61:<br>
> > +           horzSubSample_ = 2;<br>
> > +           vertSubSample_ = 1;<br>
> > +           vertexShaderSrc_ = ":NV_vertex_shader.glsl";<br>
> > +           fragmentShaderSrc_ = ":NV_2_planes_VU_f.glsl";<br>
> > +           break;<br>
> > +   case libcamera::formats::NV24:<br>
> > +           horzSubSample_ = 1;<br>
> > +           vertSubSample_ = 1;<br>
> > +           vertexShaderSrc_ = ":NV_vertex_shader.glsl";<br>
> > +           fragmentShaderSrc_ = ":NV_2_planes_UV_f.glsl";<br>
> > +           break;<br>
> > +   case libcamera::formats::NV42:<br>
> > +           horzSubSample_ = 1;<br>
> > +           vertSubSample_ = 1;<br>
> > +           vertexShaderSrc_ = ":NV_vertex_shader.glsl";<br>
> > +           fragmentShaderSrc_ = ":NV_2_planes_VU_f.glsl";<br>
> > +           break;<br>
> > +   case libcamera::formats::YUV420:<br>
> > +           horzSubSample_ = 2;<br>
> > +           vertSubSample_ = 2;<br>
> > +           vertexShaderSrc_ = ":NV_vertex_shader.glsl";<br>
> > +           fragmentShaderSrc_ = ":NV_3_planes_f.glsl";<br>
> > +           break;<br>
> > +   case libcamera::formats::YVU420:<br>
> > +           horzSubSample_ = 2;<br>
> > +           vertSubSample_ = 2;<br>
> > +           vertexShaderSrc_ = ":NV_vertex_shader.glsl";<br>
> > +           fragmentShaderSrc_ = ":NV_3_planes_f.glsl";<br>
> > +           break;<br>
> > +   default:<br>
> > +           ret = false;<br>
> > +           qWarning() << "[ViewFinderGL]:"<br>
> > +                      << "format not supported.";<br>
> > +           break;<br>
> > +   };<br>
> > +<br>
> > +   return ret;<br>
> > +}<br>
> > +<br>
> > +bool ViewFinderGL::createVertexShader()<br>
> > +{<br>
> > +   /* Create Vertex Shader */<br>
> > +   vertexShader_ = new QOpenGLShader(QOpenGLShader::Vertex, this);<br>
> > +<br>
> > +   /* Compile the vertex shader */<br>
> > +   if (!vertexShader_->compileSourceFile(vertexShaderSrc_)) {<br>
> > +           qWarning() << "[ViewFinderGL]:" << vertexShader_->log();<br>
> > +           return false;<br>
> > +   }<br>
> > +<br>
> > +   shaderProgram_.addShader(vertexShader_);<br>
> > +   return true;<br>
> > +}<br>
> > +<br>
> > +bool ViewFinderGL::createFragmentShader()<br>
> > +{<br>
> > +   int attributeVertex;<br>
> > +   int attributeTexture;<br>
> > +<br>
> > +   /* Create Fragment Shader */<br>
> > +   fragmentShader_ = new QOpenGLShader(QOpenGLShader::Fragment, this);<br>
> > +<br>
> > +   /* Compile the fragment shader */<br>
> > +   if (!fragmentShader_->compileSourceFile(fragmentShaderSrc_)) {<br>
> > +           qWarning() << "[ViewFinderGL]:" << fragmentShader_->log();<br>
> > +           return false;<br>
> > +   }<br>
> > +<br>
> > +   shaderProgram_.addShader(fragmentShader_);<br>
> > +<br>
> > +   /* Link shader pipeline */<br>
> > +   if (!shaderProgram_.link()) {<br>
> > +           qWarning() << "[ViewFinderGL]:" << shaderProgram_.log();<br>
> > +           close();<br>
> > +   }<br>
> > +<br>
> > +   /* Bind shader pipeline for use */<br>
> > +   if (!shaderProgram_.bind()) {<br>
> > +           qWarning() << "[ViewFinderGL]:" << shaderProgram_.log();<br>
> > +           close();<br>
> > +   }<br>
> > +<br>
> > +   attributeVertex = shaderProgram_.attributeLocation("vertexIn");<br>
> > +   attributeTexture = shaderProgram_.attributeLocation("textureIn");<br>
> > +<br>
> > +   shaderProgram_.enableAttributeArray(attributeVertex);<br>
> > +   shaderProgram_.setAttributeBuffer(attributeVertex,<br>
> > +                                     GL_FLOAT,<br>
> > +                                     0,<br>
> > +                                     2,<br>
> > +                                     2 * sizeof(GLfloat));<br>
> > +<br>
> > +   shaderProgram_.enableAttributeArray(attributeTexture);<br>
> > +   shaderProgram_.setAttributeBuffer(attributeTexture,<br>
> > +                                     GL_FLOAT,<br>
> > +                                     8 * sizeof(GLfloat),<br>
> > +                                     2,<br>
> > +                                     2 * sizeof(GLfloat));<br>
> > +<br>
> > +   textureUniformY_ = shaderProgram_.uniformLocation("tex_y");<br>
> > +   textureUniformU_ = shaderProgram_.uniformLocation("tex_u");<br>
> > +   textureUniformV_ = shaderProgram_.uniformLocation("tex_v");<br>
> > +<br>
> > +   if (!textureY_.isCreated())<br>
> > +           textureY_.create();<br>
> > +<br>
> > +   if (!textureU_.isCreated())<br>
> > +           textureU_.create();<br>
> > +<br>
> > +   if (!textureV_.isCreated())<br>
> > +           textureV_.create();<br>
> > +<br>
> > +   id_y_ = textureY_.textureId();<br>
> > +   id_u_ = textureU_.textureId();<br>
> > +   id_v_ = textureV_.textureId();<br>
> > +   return true;<br>
> > +}<br>
> > +<br>
> > +void ViewFinderGL::configureTexture(unsigned int id)<br>
> > +{<br>
> > +   glBindTexture(GL_TEXTURE_2D, id);<br>
> > +   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);<br>
> > +   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_LINEAR);<br>
> > +   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_S, GL_CLAMP_TO_EDGE);<br>
> > +   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_WRAP_T, GL_CLAMP_TO_EDGE);<br>
> > +}<br>
> > +<br>
> > +void ViewFinderGL::removeShader()<br>
> > +{<br>
> > +   if (shaderProgram_.isLinked()) {<br>
> > +           shaderProgram_.release();<br>
> > +           shaderProgram_.removeAllShaders();<br>
> > +   }<br>
> > +<br>
> > +   if (fragmentShader_)<br>
> > +           delete fragmentShader_;<br>
> > +<br>
> > +   if (vertexShader_)<br>
> > +           delete vertexShader_;<br>
> > +}<br>
> > +<br>
> > +void ViewFinderGL::initializeGL()<br>
> > +{<br>
> > +   initializeOpenGLFunctions();<br>
> > +   glEnable(GL_TEXTURE_2D);<br>
> > +   glDisable(GL_DEPTH_TEST);<br>
> > +<br>
> > +   static const GLfloat coordinates[2][4][2]{<br>
> > +           {<br>
> > +                   /* Vertex coordinates */<br>
> > +                   { -1.0f, -1.0f },<br>
> > +                   { -1.0f, +1.0f },<br>
> > +                   { +1.0f, +1.0f },<br>
> > +                   { +1.0f, -1.0f },<br>
> > +           },<br>
> > +           {<br>
> > +                   /* Texture coordinates */<br>
> > +                   { 1.0f, 0.0f },<br>
> > +                   { 1.0f, 1.0f },<br>
> > +                   { 0.0f, 1.0f },<br>
> > +                   { 0.0f, 0.0f },<br>
> <br>
> I *think* this should be<br>
> <br>
>                       { 0.0f, 1.0f },<br>
>                       { 0.0f, 0.0f },<br>
>                       { 1.0f, 0.0f },<br>
>                       { 1.0f, 1.0f },<br>
> <br>
> I'll test it and try to understand :-)<br>
<br>
I confirm that the original patch rotates the image by 180° for me,<br>
while the proposed values above show it in the right orientation. I've<br>
compared -r qt and -r gles to check that.<br></blockquote><div>Are you going to apply above value to the v7 too? If so I will not send out the patch to fix it.</div><div><br></div><div>Thanks,</div><div>Show</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> > +           },<br>
> > +   };<br>
> > +<br>
> > +   vertexBuffer_.create();<br>
> > +   vertexBuffer_.bind();<br>
> > +   vertexBuffer_.allocate(coordinates, sizeof(coordinates));<br>
> > +<br>
> > +   /* Create Vertex Shader */<br>
> > +   if (!createVertexShader())<br>
> > +           qWarning() << "[ViewFinderGL]: create vertex shader failed.";<br>
> > +<br>
> > +   glClearColor(1.0f, 1.0f, 1.0f, 0.0f);<br>
> > +}<br>
> > +<br>
> > +void ViewFinderGL::doRender()<br>
> > +{<br>
> > +   switch (format_) {<br>
> > +   case libcamera::formats::NV12:<br>
> > +   case libcamera::formats::NV21:<br>
> > +   case libcamera::formats::NV16:<br>
> > +   case libcamera::formats::NV61:<br>
> > +   case libcamera::formats::NV24:<br>
> > +   case libcamera::formats::NV42:<br>
> > +           /* activate texture Y */<br>
> <br>
> s/activate/Activate/ (and similarly below).<br>
> <br>
> > +           glActiveTexture(GL_TEXTURE0);<br>
> > +           configureTexture(id_y_);<br>
> > +           glTexImage2D(GL_TEXTURE_2D,<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        size_.width(),<br>
> > +                        size_.height(),<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        GL_UNSIGNED_BYTE,<br>
> > +                        yuvData_);<br>
> > +           shaderProgram_.setUniformValue(textureUniformY_, 0);<br>
> > +<br>
> > +           /* activate texture UV/VU */<br>
> > +           glActiveTexture(GL_TEXTURE1);<br>
> > +           configureTexture(id_u_);<br>
> > +           glTexImage2D(GL_TEXTURE_2D,<br>
> > +                        0,<br>
> > +                        GL_RG,<br>
> > +                        size_.width() / horzSubSample_,<br>
> > +                        size_.height() / vertSubSample_,<br>
> > +                        0,<br>
> > +                        GL_RG,<br>
> > +                        GL_UNSIGNED_BYTE,<br>
> > +                        (char *)yuvData_ + size_.width() * size_.height());<br>
> > +           shaderProgram_.setUniformValue(textureUniformU_, 1);<br>
> > +           break;<br>
> > +<br>
> > +   case libcamera::formats::YUV420:<br>
> > +           /* activate texture Y */<br>
> > +           glActiveTexture(GL_TEXTURE0);<br>
> > +           configureTexture(id_y_);<br>
> > +           glTexImage2D(GL_TEXTURE_2D,<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        size_.width(),<br>
> > +                        size_.height(),<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        GL_UNSIGNED_BYTE,<br>
> > +                        yuvData_);<br>
> > +           shaderProgram_.setUniformValue(textureUniformY_, 0);<br>
> > +<br>
> > +           /* activate texture U */<br>
> > +           glActiveTexture(GL_TEXTURE1);<br>
> > +           configureTexture(id_u_);<br>
> > +           glTexImage2D(GL_TEXTURE_2D,<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        size_.width() / horzSubSample_,<br>
> > +                        size_.height() / vertSubSample_,<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        GL_UNSIGNED_BYTE,<br>
> > +                        (char *)yuvData_ + size_.width() * size_.height());<br>
> > +           shaderProgram_.setUniformValue(textureUniformU_, 1);<br>
> > +<br>
> > +           /* activate texture V */<br>
> > +           glActiveTexture(GL_TEXTURE2);<br>
> > +           configureTexture(id_v_);<br>
> > +           glTexImage2D(GL_TEXTURE_2D,<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        size_.width() / horzSubSample_,<br>
> > +                        size_.height() / vertSubSample_,<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        GL_UNSIGNED_BYTE,<br>
> > +                        (char *)yuvData_ + size_.width() * size_.height() * 5 / 4);<br>
> > +           shaderProgram_.setUniformValue(textureUniformV_, 2);<br>
> > +           break;<br>
> > +<br>
> > +   case libcamera::formats::YVU420:<br>
> > +           /* activate texture Y */<br>
> > +           glActiveTexture(GL_TEXTURE0);<br>
> > +           configureTexture(id_y_);<br>
> > +           glTexImage2D(GL_TEXTURE_2D,<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        size_.width(),<br>
> > +                        size_.height(),<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        GL_UNSIGNED_BYTE,<br>
> > +                        yuvData_);<br>
> > +           shaderProgram_.setUniformValue(textureUniformY_, 0);<br>
> > +<br>
> > +           /* activate texture V */<br>
> > +           glActiveTexture(GL_TEXTURE2);<br>
> > +           configureTexture(id_v_);<br>
> > +           glTexImage2D(GL_TEXTURE_2D,<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        size_.width() / horzSubSample_,<br>
> > +                        size_.height() / vertSubSample_,<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        GL_UNSIGNED_BYTE,<br>
> > +                        (char *)yuvData_ + size_.width() * size_.height());<br>
> > +           shaderProgram_.setUniformValue(textureUniformV_, 1);<br>
> <br>
> I don't think this is correct. GL_TEXTURE1 stores the U plane, and you<br>
> assign it to tex_v, which is the V plane in the shader.<br>
> <br>
> There are two options, either s/1/2/ or s/GL_TEXTURE2/GL_TEXTURE1/ (and<br>
> the other way below).<br>
> <br>
> With these small issues addressed,<br>
> <br>
> Reviewed-by: Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com" target="_blank">laurent.pinchart@ideasonboard.com</a>><br>
> <br>
> I can also fix these when applying the patch, but given that there are<br>
> quite a few issues, I would then send a v7 to the list to make sure I<br>
> haven't done anything wrong.<br>
> <br>
> > +<br>
> > +           /* activate texture U */<br>
> > +           glActiveTexture(GL_TEXTURE1);<br>
> > +           configureTexture(id_u_);<br>
> > +           glTexImage2D(GL_TEXTURE_2D,<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        size_.width() / horzSubSample_,<br>
> > +                        size_.height() / vertSubSample_,<br>
> > +                        0,<br>
> > +                        GL_RED,<br>
> > +                        GL_UNSIGNED_BYTE,<br>
> > +                        (char *)yuvData_ + size_.width() * size_.height() * 5 / 4);<br>
> > +           shaderProgram_.setUniformValue(textureUniformU_, 2);<br>
> > +           break;<br>
> > +<br>
> > +   default:<br>
> > +           break;<br>
> > +   };<br>
> > +}<br>
> > +<br>
> > +void ViewFinderGL::paintGL()<br>
> > +{<br>
> > +   if (!fragmentShader_)<br>
> > +           if (!createFragmentShader()) {<br>
> > +                   qWarning() << "[ViewFinderGL]:"<br>
> > +                              << "create fragment shader failed.";<br>
> > +           }<br>
> > +<br>
> > +   if (yuvData_) {<br>
> > +           glClearColor(0.0, 0.0, 0.0, 1.0);<br>
> > +           glClear(GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT);<br>
> > +<br>
> > +           doRender();<br>
> > +           glDrawArrays(GL_TRIANGLE_FAN, 0, 4);<br>
> > +   }<br>
> > +}<br>
> > +<br>
> > +void ViewFinderGL::resizeGL(int w, int h)<br>
> > +{<br>
> > +   glViewport(0, 0, w, h);<br>
> > +}<br>
> > +<br>
> > +QSize ViewFinderGL::sizeHint() const<br>
> > +{<br>
> > +   return size_.isValid() ? size_ : QSize(640, 480);<br>
> > +}<br>
> > diff --git a/src/qcam/viewfinder_gl.h b/src/qcam/viewfinder_gl.h<br>
> > new file mode 100644<br>
> > index 0000000..69502b7<br>
> > --- /dev/null<br>
> > +++ b/src/qcam/viewfinder_gl.h<br>
> > @@ -0,0 +1,96 @@<br>
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */<br>
> > +/*<br>
> > + * Copyright (C) 2020, Linaro<br>
> > + *<br>
> > + * viewfinder_GL.h - OpenGL Viewfinder for rendering by OpenGL shader<br>
> > + *<br>
> > + */<br>
> > +#ifndef __VIEWFINDER_GL_H__<br>
> > +#define __VIEWFINDER_GL_H__<br>
> > +<br>
> > +#include <QImage><br>
> > +#include <QMutex><br>
> > +#include <QOpenGLBuffer><br>
> > +#include <QOpenGLFunctions><br>
> > +#include <QOpenGLShader><br>
> > +#include <QOpenGLShaderProgram><br>
> > +#include <QOpenGLTexture><br>
> > +#include <QOpenGLWidget><br>
> > +#include <QSize><br>
> > +<br>
> > +#include <libcamera/buffer.h><br>
> > +#include <libcamera/formats.h><br>
> > +<br>
> > +#include "viewfinder.h"<br>
> > +<br>
> > +class ViewFinderGL : public QOpenGLWidget,<br>
> > +                public ViewFinder,<br>
> > +                protected QOpenGLFunctions<br>
> > +{<br>
> > +   Q_OBJECT<br>
> > +<br>
> > +public:<br>
> > +   ViewFinderGL(QWidget *parent = nullptr);<br>
> > +   ~ViewFinderGL();<br>
> > +<br>
> > +   const QList<libcamera::PixelFormat> &nativeFormats() const override;<br>
> > +<br>
> > +   int setFormat(const libcamera::PixelFormat &format, const QSize &size) override;<br>
> > +   void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) override;<br>
> > +   void stop() override;<br>
> > +<br>
> > +   QImage getCurrentImage() override;<br>
> > +<br>
> > +Q_SIGNALS:<br>
> > +   void renderComplete(libcamera::FrameBuffer *buffer);<br>
> > +<br>
> > +protected:<br>
> > +   void initializeGL() override;<br>
> > +   void paintGL() override;<br>
> > +   void resizeGL(int w, int h) override;<br>
> > +   QSize sizeHint() const override;<br>
> > +<br>
> > +private:<br>
> > +   bool selectFormat(const libcamera::PixelFormat &format);<br>
> > +<br>
> > +   void configureTexture(unsigned int id);<br>
> > +   bool createFragmentShader();<br>
> > +   bool createVertexShader();<br>
> > +   void removeShader();<br>
> > +   void doRender();<br>
> > +<br>
> > +   /* Captured image size, format and buffer */<br>
> > +   libcamera::FrameBuffer *buffer_;<br>
> > +   libcamera::PixelFormat format_;<br>
> > +   QSize size_;<br>
> > +   unsigned char *yuvData_;<br>
> > +<br>
> > +   /* OpenGL components for rendering */<br>
> > +   QOpenGLShader *fragmentShader_;<br>
> > +   QOpenGLShader *vertexShader_;<br>
> > +   QOpenGLShaderProgram shaderProgram_;<br>
> > +<br>
> > +   /* Vertex buffer */<br>
> > +   QOpenGLBuffer vertexBuffer_;<br>
> > +<br>
> > +   /* Fragment and Vertex shader file name */<br>
> > +   QString fragmentShaderSrc_;<br>
> > +   QString vertexShaderSrc_;<br>
> > +<br>
> > +   /* YUV texture planars and parameters */<br>
> > +   GLuint id_u_;<br>
> > +   GLuint id_v_;<br>
> > +   GLuint id_y_;<br>
> > +   GLuint textureUniformU_;<br>
> > +   GLuint textureUniformV_;<br>
> > +   GLuint textureUniformY_;<br>
> > +   QOpenGLTexture textureU_;<br>
> > +   QOpenGLTexture textureV_;<br>
> > +   QOpenGLTexture textureY_;<br>
> > +   unsigned int horzSubSample_;<br>
> > +   unsigned int vertSubSample_;<br>
> > +<br>
> > +   QMutex mutex_; /* Prevent concurrent access to image_ */<br>
> > +};<br>
> > +<br>
> > +#endif /* __VIEWFINDER_GL_H__ */<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>