<div dir="ltr"><div dir="ltr">Hi Laurent,</div><div dir="ltr"><br clear="all"><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"></div></div></div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Fri, Jul 3, 2020 at 6:14 PM Laurent Pinchart <<a href="mailto:laurent.pinchart@ideasonboard.com">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 Fri, Jul 03, 2020 at 03:46:32PM +0800, Show Liu wrote:<br>
> On Wed, Jul 1, 2020 at 12:26 AM Laurent Pinchart wrote:<br>
> > On Tue, Jun 30, 2020 at 05:03:43PM +0200, Niklas Söderlund wrote:<br>
> > > Hi Show,<br>
> > ><br>
> > > Thanks for your work.<br>
> ><br>
> > Likewise :-)<br>
> ><br>
> > > I really like this version! The structure is almost there and much<br>
> > > better then previous versions. As Kieran points out there are style<br>
> > > errors that checkstyle.py will help you point out so I will ignore them<br>
> > > in this review.<br>
><br>
> Thank you.:-)<br>
> <br>
> > > On 2020-06-24 15:37:05 +0800, Show Liu wrote:<br>
> > > > The patch is to render the NV family YUV formats by OpenGL shader.<br>
> > > > V3: refine the fragment shader for better pixel color.<br>
> > > ><br>
> > > > Signed-off-by: Show Liu <<a href="mailto:show.liu@linaro.org" target="_blank">show.liu@linaro.org</a>><br>
> > > > ---<br>
> > > >  src/qcam/main.cpp         |   2 +<br>
> > > >  src/qcam/main_window.cpp  |  19 ++-<br>
> > > >  src/qcam/main_window.h    |   3 +-<br>
> > > >  src/qcam/meson.build      |   2 +<br>
> > > >  src/qcam/shader.h         | 104 ++++++++++++<br>
> > > >  src/qcam/viewfinder.cpp   |  18 +-<br>
> > > >  src/qcam/viewfinder.h     |  23 ++-<br>
> > > >  src/qcam/viewfinderGL.cpp | 335 ++++++++++++++++++++++++++++++++++++++<br>
> > > >  src/qcam/viewfinderGL.h   | 101 ++++++++++++<br>
> > > >  9 files changed, 593 insertions(+), 14 deletions(-)<br>
> > > >  create mode 100644 src/qcam/shader.h<br>
> > > >  create mode 100644 src/qcam/viewfinderGL.cpp<br>
> > > >  create mode 100644 src/qcam/viewfinderGL.h<br>
> > > ><br>
> > > > diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp<br>
> > > > index b3468cb..a3ea471 100644<br>
> > > > --- a/src/qcam/main.cpp<br>
> > > > +++ b/src/qcam/main.cpp<br>
> > > > @@ -35,6 +35,8 @@ OptionsParser::Options parseOptions(int argc, char *argv[])<br>
> > > >                      "help");<br>
> > > >     parser.addOption(OptStream, &streamKeyValue,<br>
> > > >                      "Set configuration of a camera stream", "stream", true);<br>
> > > > +   parser.addOption(OptOpenGL, OptionNone, "Render YUV formats frame via OpenGL shader",<br>
> > > > +                    "opengl");<br>
> ><br>
> > Should we default to OpenGL when possible, and add an option to force a<br>
> > particular backend ? Maybe -r/--render={gles,qt}<br>
><br>
> I'd like to mention why I set OpenGL rendering as an option.<br>
> First, this depends on the "GPU enabled" platforms,<br>
> and it always needs some additional steps to make sure the GPU is ready.<br>
> Second, as I said this patch is only for NV family YUV formats at present,<br>
> and I believe it just covers very small parts of camera stream formats.<br>
> That's why I am still trying to make it support more formats as I can.:-)<br>
<br>
That's a good point, defaulting to the Qt renderer for now may be best.<br>
I'd still prefer a -r/--render option if possible though, to make it<br>
easier to change the default and to add more renderers later if needed.<br></blockquote><div>OK. Understood. I will try this in the next version. </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>
> > > >     OptionsParser::Options options = parser.parse(argc, argv);<br>
> > > >     if (options.isSet(OptHelp))<br>
> > > > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp<br>
> > > > index 7bc1360..6edf370 100644<br>
> > > > --- a/src/qcam/main_window.cpp<br>
> > > > +++ b/src/qcam/main_window.cpp<br>
> > > > @@ -28,6 +28,9 @@<br>
> > > >  #include <libcamera/version.h><br>
> > > ><br>
> > > >  #include "dng_writer.h"<br>
> > > > +#include "main_window.h"<br>
> > > > +#include "viewfinder.h"<br>
> > > > +#include "viewfinderGL.h"<br>
> ><br>
> > According to the name scheme we use, I think this should be<br>
> > viewfinder_gl.h.<br>
> <br>
> OK. will fix it.<br>
> <br>
> > > >  using namespace libcamera;<br>
> > > ><br>
> > > > @@ -105,10 +108,18 @@ MainWindow::MainWindow(CameraManager *cm, const OptionsParser::Options &options)<br>
> > > >     setWindowTitle(title_);<br>
> > > >     connect(&titleTimer_, SIGNAL(timeout()), this, SLOT(updateTitle()));<br>
> > > ><br>
> > > > -   viewfinder_ = new ViewFinder(this);<br>
> > > > -   connect(viewfinder_, &ViewFinder::renderComplete,<br>
> > > > -           this, &MainWindow::queueRequest);<br>
> > > > -   setCentralWidget(viewfinder_);<br>
> > > > +   if (options_.isSet(OptOpenGL)) {<br>
> > > > +           viewfinder_ = new ViewFinderGL(this);<br>
> > > > +           connect(dynamic_cast<ViewFinderGL *>(viewfinder_), &ViewFinderGL::renderComplete,<br>
> > > > +                           this, &MainWindow::queueRequest);<br>
> > > > +           setCentralWidget(dynamic_cast<ViewFinderGL *>(viewfinder_));<br>
> > > > +   } else {<br>
> > > > +           viewfinder_ = new ViewFinder(this);<br>
> > > > +           connect(dynamic_cast<ViewFinder *>(viewfinder_), &ViewFinder::renderComplete,<br>
> > > > +                           this, &MainWindow::queueRequest);<br>
> > > > +           setCentralWidget(dynamic_cast<ViewFinder *>(viewfinder_));<br>
> > > > +   }<br>
> > ><br>
> > > I understand that one can't inherit from QObject twice, but this looks<br>
> > > odd. Is there someway the base class could inherit from QObject or<br>
> > > possibly QWidget and the derived classes hide their specilization<br>
> > > somehow? I'm no Qt export so maybe I'm asking for something impossible<br>
> > > or really stupid.<br>
><br>
> No, I really appreciate your opinions, it'll help me to make this better. :)<br>
> <br>
> > If we assume all subclasses of Viewfinder will be QWidget, we could do<br>
> ><br>
> >         if (options_.isSet(OptOpenGL))<br>
> >                 viewfinder_ = new ViewFinderGL(this);<br>
> >         else<br>
> >                 viewfinder_ = new ViewFinder(this);<br>
> ><br>
> >         QWidget *vf>idget = dynamic_cast<QWidget>(viewfinder_);<br>
> >         connect(vfWidget, &ViewFinderHandler::renderComplete,<br>
> >                 this, &MainWindow::queueRequest);<br>
> >         setCentralWidget(vfWidget);<br>
> ><br>
> > With ViewFinderHandler::renderComplete() being a signal in the base<br>
> > class.<br>
><br>
> I will try this way in the next version.<br>
> <br>
> > > > +<br>
> > > >     adjustSize();<br>
> > > ><br>
> > > >     /* Hotplug/unplug support */<br>
> > > > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h<br>
> > > > index 4606fe4..a852ef4 100644<br>
> > > > --- a/src/qcam/main_window.h<br>
> > > > +++ b/src/qcam/main_window.h<br>
> > > > @@ -38,6 +38,7 @@ enum {<br>
> > > >     OptCamera = 'c',<br>
> > > >     OptHelp = 'h',<br>
> > > >     OptStream = 's',<br>
> > > > +   OptOpenGL = 'o',<br>
> > > >  };<br>
> > > ><br>
> > > >  class CaptureRequest<br>
> > > > @@ -102,7 +103,7 @@ private:<br>
> > > >     QAction *startStopAction_;<br>
> > > >     QComboBox *cameraCombo_;<br>
> > > >     QAction *saveRaw_;<br>
> > > > -   ViewFinder *viewfinder_;<br>
> > > > +   ViewFinderHandler *viewfinder_;<br>
> ><br>
> > I'd split this patch in two, with one patch that renames the existing<br>
> > ViewFinder to ViewFinderQt and creates a ViewFinder base class (which<br>
> > you call ViewFinderHandler here), and a second patch that adds<br>
> > ViewFinderGL.<br>
> <br>
> OK. will do.<br>
> <br>
> > > >     QIcon iconPlay_;<br>
> > > >     QIcon iconStop_;<br>
> > > > diff --git a/src/qcam/meson.build b/src/qcam/meson.build<br>
> > > > index 045db52..6a58947 100644<br>
> > > > --- a/src/qcam/meson.build<br>
> > > > +++ b/src/qcam/meson.build<br>
> > > > @@ -7,11 +7,13 @@ qcam_sources = files([<br>
> > > >      'main.cpp',<br>
> > > >      'main_window.cpp',<br>
> > > >      'viewfinder.cpp',<br>
> > > > +    'viewfinderGL.cpp'<br>
> > > >  ])<br>
> > > ><br>
> > > >  qcam_moc_headers = files([<br>
> > > >      'main_window.h',<br>
> > > >      'viewfinder.h',<br>
> > > > +    'viewfinderGL.h'<br>
> > > >  ])<br>
> > > ><br>
> > > >  qcam_resources = files([<br>
> > > > diff --git a/src/qcam/shader.h b/src/qcam/shader.h<br>
> > > > new file mode 100644<br>
> > > > index 0000000..f54c264<br>
> > > > --- /dev/null<br>
> > > > +++ b/src/qcam/shader.h<br>
> > > > @@ -0,0 +1,104 @@<br>
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */<br>
> > > > +/*<br>
> > > > + * Copyright (C) 2020, Linaro<br>
> > > > + *<br>
> > > > + * shader.h - YUV format converter shader source code<br>
> > > > + */<br>
> > > > +#ifndef __SHADER_H__<br>
> > > > +#define __SHADER_H__<br>
> > ><br>
> > > I think the content of this file should be moved inside viewfinderGL.cpp<br>
> ><br>
> > Or maybe to viewfinder_gl_shader.cpp if we want to keep it separate.<br>
> > Header files should only contain the declarations in any case, not the<br>
> > actual variable contents.<br>
> ><br>
> > Ideally we should store the shaders in files of their own, not in a C<br>
> > array, and have them pulled in as Qt resources<br>
> > (<a href="https://doc.qt.io/qt-5/resources.html#using-resources-in-the-application" rel="noreferrer" target="_blank">https://doc.qt.io/qt-5/resources.html#using-resources-in-the-application</a><br>
> > ).<br>
> > They can be read through a QFile. It's something we can do on top of<br>
> > this patch.<br>
><br>
> Actually,  QOpenGLShader/QOpenGLShaderProgram are provide some methods<br>
> to build/compile the shader source code from file directly but if I load it<br>
> from file which means<br>
> additional shader installation steps are required and also there are some<br>
> potential risks<br>
> if the shader is not in the right position. That's why I push them into one<br>
> header file.<br>
> Forgive my lazy ...:-P<br>
> Anyway refine this part in the next version.<br>
<br>
I agree with you that storing shaders in files that need to be installed<br>
isn't very nice, for the reasons you described. However, if you store<br>
them in files in the source tree that are then compiled as Qt resources,<br>
the data will be in the same binary, and always accessible. I think that<br>
would be a good middle-ground, making the shader sources nicer to read,<br>
modify and review, and still easy to use in the code through QFile. If<br>
QFile::map() works on resources, that will be very simple, otherwise<br>
QFile::readAll() will give you a QByteArray with the data.<br></blockquote><div>OK. sounds good. I will try this in the next version too. </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>
> > > > +/* Vertex shader for NV formats */<br>
> > > > +char NV_Vertex_shader[] = "attribute vec4 vertexIn;\n"<br>
> > ><br>
> > > could this (and bellow) be made static const ?<br>
> > ><br>
> > > > +                        "attribute vec2 textureIn;\n"<br>
> > > > +                        "varying vec2 textureOut;\n"<br>
> > > > +                        "void main(void)\n"<br>
> > > > +                        "{\n"<br>
> > > > +                        "gl_Position = vertexIn;\n"<br>
> > > > +                        "textureOut = textureIn;\n"<br>
> > > > +                        "}\n";<br>
> > > > +<br>
> > > > +/* Fragment shader for NV12, NV16 and NV24 */<br>
> > > > +char NV_2_planes_UV[] = "#ifdef GL_ES\n"<br>
> > > > +                "precision highp float;\n"<br>
> > > > +                "#endif\n"<br>
> > > > +                "varying vec2 textureOut;\n"<br>
> > > > +                "uniform sampler2D tex_y;\n"<br>
> > > > +                "uniform sampler2D tex_u;\n"<br>
> > > > +                "void main(void)\n"<br>
> > > > +                "{\n"<br>
> > > > +                "vec3 yuv;\n"<br>
> > > > +                "vec3 rgb;\n"<br>
> > > > +                "mat3 convert_mat = mat3(vec3(1.1640625,  1.1640625, 1.1640625),\n"<br>
> > > > +                "                        vec3(0.0,   -0.390625, 2.015625),\n"<br>
> > > > +                "                        vec3(1.5975625, -0.8125, 0.0));\n"<br>
> > > > +                "yuv.x = texture2D(tex_y, textureOut).r - 0.0625;\n"<br>
> > > > +                "yuv.y = texture2D(tex_u, textureOut).r - 0.5;\n"<br>
> > > > +                "yuv.z = texture2D(tex_u, textureOut).g - 0.5;\n"<br>
> > > > +                "rgb = convert_mat * yuv;\n"<br>
> > > > +                "gl_FragColor = vec4(rgb, 1.0);\n"<br>
> > > > +                "}\n";<br>
> > > > +<br>
> > > > +/* Fragment shader for NV21, NV61 and NV42 */<br>
> > > > +char NV_2_planes_VU[] = "#ifdef GL_ES\n"<br>
> > > > +                "precision highp float;\n"<br>
> > > > +                "#endif\n"<br>
> > > > +                "varying vec2 textureOut;\n"<br>
> > > > +                "uniform sampler2D tex_y;\n"<br>
> > > > +                "uniform sampler2D tex_u;\n"<br>
> > > > +                "void main(void)\n"<br>
> > > > +                "{\n"<br>
> > > > +                "vec3 yuv;\n"<br>
> > > > +                "vec3 rgb;\n"<br>
> > > > +                "mat3 convert_mat = mat3(vec3(1.1640625,  1.1640625, 1.1640625),\n"<br>
> > > > +                "                        vec3(0.0,   -0.390625, 2.015625),\n"<br>
> > > > +                "                        vec3(1.5975625, -0.8125, 0.0));\n"<br>
> > > > +                "yuv.x = texture2D(tex_y, textureOut).r - 0.0625;\n"<br>
> > > > +                "yuv.y = texture2D(tex_u, textureOut).g - 0.5;\n"<br>
> > > > +                "yuv.z = texture2D(tex_u, textureOut).r - 0.5;\n"<br>
> > > > +                "rgb = convert_mat * yuv;\n"<br>
> > > > +                "gl_FragColor = vec4(rgb, 1.0);\n"<br>
> > > > +                "}\n";<br>
> > > > +<br>
> > > > +/* Fragment shader for YUV420 */<br>
> > > > +char NV_3_planes_UV[] = "#ifdef GL_ES\n"<br>
> > > > +                "precision highp float;\n"<br>
> > > > +                "#endif\n"<br>
> > > > +                "varying vec2 textureOut;\n"<br>
> > > > +                "uniform sampler2D tex_y;\n"<br>
> > > > +                "uniform sampler2D tex_u;\n"<br>
> > > > +                "uniform sampler2D tex_v;\n"<br>
> > > > +                "void main(void)\n"<br>
> > > > +                "{\n"<br>
> > > > +                "vec3 yuv;\n"<br>
> > > > +                "vec3 rgb;\n"<br>
> > > > +                "mat3 convert_mat = mat3(vec3(1.1640625,  1.1640625, 1.1640625),\n"<br>
> > > > +                "                        vec3(0.0,   -0.390625, 2.015625),\n"<br>
> > > > +                "                        vec3(1.5975625, -0.8125, 0.0));\n"<br>
> > > > +                "yuv.x = texture2D(tex_y, textureOut).r - 0.0625;\n"<br>
> > > > +                "yuv.y = texture2D(tex_u, textureOut).r - 0.5;\n"<br>
> > > > +                "yuv.z = texture2D(tex_v, textureOut).g - 0.5;\n"<br>
> > > > +                "rgb = convert_mat * yuv;\n"<br>
> > > > +                "gl_FragColor = vec4(rgb, 1);\n"<br>
> > > > +                "}\n";<br>
> > > > +<br>
> > > > +/* Fragment shader for YVU420 */<br>
> > > > +char NV_3_planes_VU[] = "precision highp float;\n"<br>
> > > > +                "#endif\n"<br>
> > > > +                "varying vec2 textureOut;\n"<br>
> > > > +                "uniform sampler2D tex_y;\n"<br>
> > > > +                "uniform sampler2D tex_u;\n"<br>
> > > > +                "uniform sampler2D tex_v;\n"<br>
> > > > +                "void main(void)\n"<br>
> > > > +                "{\n"<br>
> > > > +                "vec3 yuv;\n"<br>
> > > > +                "vec3 rgb;\n"<br>
> > > > +                "mat3 convert_mat = mat3(vec3(1.1640625,  1.1640625, 1.1640625),\n"<br>
> > > > +                "                        vec3(0.0,   -0.390625, 2.015625),\n"<br>
> > > > +                "                        vec3(1.5975625, -0.8125, 0.0));\n"<br>
> > > > +                "yuv.x = texture2D(tex_y, textureOut).r - 0.0625;\n"<br>
> > > > +                "yuv.y = texture2D(tex_v, textureOut).g - 0.5;\n"<br>
> > > > +                "yuv.z = texture2D(tex_u, textureOut).r - 0.5;\n"<br>
> > > > +                "rgb = convert_mat * yuv;\n"<br>
> > > > +                "gl_FragColor = vec4(rgb, 1);\n"<br>
> > > > +                "}\n";<br>
> > > > +#endif // __SHADER_H__<br>
> > > > diff --git a/src/qcam/viewfinder.cpp b/src/qcam/viewfinder.cpp<br>
> > > > index dcf8a15..d3a2422 100644<br>
> > > > --- a/src/qcam/viewfinder.cpp<br>
> > > > +++ b/src/qcam/viewfinder.cpp<br>
> > > > @@ -33,22 +33,30 @@ static const QMap<libcamera::PixelFormat, QImage::Format> nativeFormats<br>
> > > >     { libcamera::formats::BGR888, QImage::Format_RGB888 },<br>
> > > >  };<br>
> > > ><br>
> > > > -ViewFinder::ViewFinder(QWidget *parent)<br>
> > > > -   : QWidget(parent), buffer_(nullptr)<br>
> > > > +ViewFinderHandler::ViewFinderHandler()<br>
> > > >  {<br>
> > > > -   icon_ = QIcon(":camera-off.svg");<br>
> > > >  }<br>
> ><br>
> > You don't need an empty constructor in the base class, you can just drop<br>
> > it.<br>
> ><br>
> > > ><br>
> > > > -ViewFinder::~ViewFinder()<br>
> > > > +ViewFinderHandler::~ViewFinderHandler()<br>
> > > >  {<br>
> > > >  }<br>
> > > ><br>
> > > > -const QList<libcamera::PixelFormat> &ViewFinder::nativeFormats() const<br>
> > > > +const QList<libcamera::PixelFormat> &ViewFinderHandler::nativeFormats() const<br>
> > > >  {<br>
> > > >     static const QList<libcamera::PixelFormat> formats = ::nativeFormats.keys();<br>
> > > >     return formats;<br>
> > > >  }<br>
> ><br>
> > I expect native formats to be different for ViewFinderQt and<br>
> > ViewFinderGL, so I'd make this a pure virtual function with<br>
> > implementations in the derived classes. ViewFinderGL should report all<br>
> > the formats for which you have conversion shaders.<br>
><br>
>  ok, I will rewrite this.<br>
> <br>
> > > ><br>
> > > > +ViewFinder::ViewFinder(QWidget *parent)<br>
> > > > +   : QWidget(parent), buffer_(nullptr)<br>
> > > > +{<br>
> > > > +   icon_ = QIcon(":camera-off.svg");<br>
> > > > +}<br>
> > ><br>
> > > I agree with Kieran here I think the base class should be named<br>
> > > ViewFinder and the two derived classes ViewFinderQt and ViewFinderGL or<br>
> > > something similar.<br>
> ><br>
> > Seems we all agree then :-)<br>
> ><br>
> > > I also think you should move the base class to its own .h file (and .cpp<br>
> > > file if needed).<br>
> ><br>
> > Agreed.<br>
><br>
> I will modify it accordingly.<br>
> <br>
> > > > +<br>
> > > > +ViewFinder::~ViewFinder()<br>
> > > > +{<br>
> > > > +}<br>
> > > > +<br>
> > > >  int ViewFinder::setFormat(const libcamera::PixelFormat &format,<br>
> > > >                       const QSize &size)<br>
> > > >  {<br>
> > > > diff --git a/src/qcam/viewfinder.h b/src/qcam/viewfinder.h<br>
> > > > index 26a1320..741d694 100644<br>
> > > > --- a/src/qcam/viewfinder.h<br>
> > > > +++ b/src/qcam/viewfinder.h<br>
> > > > @@ -13,6 +13,8 @@<br>
> > > >  #include <QList><br>
> > > >  #include <QImage><br>
> > > >  #include <QMutex><br>
> > > > +#include <QOpenGLWidget><br>
> > > > +#include <QOpenGLFunctions><br>
> > > >  #include <QSize><br>
> > > >  #include <QWidget><br>
> > > ><br>
> > > > @@ -28,7 +30,23 @@ struct MappedBuffer {<br>
> > > >     size_t size;<br>
> > > >  };<br>
> > > ><br>
> > > > -class ViewFinder : public QWidget<br>
> > > > +class ViewFinderHandler<br>
> > > > +{<br>
> > > > +public:<br>
> > > > +   ViewFinderHandler();<br>
> > > > +   virtual ~ViewFinderHandler();<br>
> > > > +<br>
> > > > +   const QList<libcamera::PixelFormat> &nativeFormats() const;<br>
> > > > +<br>
> > > > +   virtual int setFormat(const libcamera::PixelFormat &format, const QSize &size) =0;<br>
> > > > +   virtual void render(libcamera::FrameBuffer *buffer, MappedBuffer *map) =0;<br>
> > > > +   virtual void stop() =0;<br>
> > > > +<br>
> > > > +   virtual QImage getCurrentImage() =0;<br>
> > > > +<br>
> > > > +};<br>
> > > > +<br>
> > > > +class ViewFinder : public QWidget, public ViewFinderHandler<br>
> > > >  {<br>
> > > >     Q_OBJECT<br>
> > > ><br>
> > > > @@ -36,8 +54,6 @@ public:<br>
> > > >     ViewFinder(QWidget *parent);<br>
> > > >     ~ViewFinder();<br>
> > > ><br>
> > > > -   const QList<libcamera::PixelFormat> &nativeFormats() const;<br>
> > > > -<br>
> > > >     int setFormat(const libcamera::PixelFormat &format, const QSize &size);<br>
> > > >     void render(libcamera::FrameBuffer *buffer, MappedBuffer *map);<br>
> > > >     void stop();<br>
> > > > @@ -67,5 +83,4 @@ private:<br>
> > > >     QImage image_;<br>
> > > >     QMutex mutex_; /* Prevent concurrent access to image_ */<br>
> > > >  };<br>
> > > > -<br>
> ><br>
> > This blank line should be kept.<br>
><br>
> sure, will fix it.<br>
> <br>
> > > >  #endif /* __QCAM_VIEWFINDER__ */<br>
> > > > diff --git a/src/qcam/viewfinderGL.cpp b/src/qcam/viewfinderGL.cpp<br>
> > > > new file mode 100644<br>
> > > > index 0000000..7b47beb<br>
> > > > --- /dev/null<br>
> > > > +++ b/src/qcam/viewfinderGL.cpp<br>
> > > > @@ -0,0 +1,335 @@<br>
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */<br>
> > > > +/*<br>
> > > > + * Copyright (C) 2020, Linaro<br>
> > > > + *<br>
> > > > + * viewfinderGL.cpp - Render YUV format frame by OpenGL shader<br>
> > > > + */<br>
> > > > +<br>
> > > > +#include "shader.h"<br>
> > > > +#include "viewfinderGL.h"<br>
> > > > +<br>
> > > > +#include <QImage><br>
> > > > +<br>
> > > > +#include <libcamera/formats.h><br>
> > > > +<br>
> > > > +#define ATTRIB_VERTEX 0<br>
> > > > +#define ATTRIB_TEXTURE 1<br>
> > > > +<br>
> > > > +ViewFinderGL::ViewFinderGL(QWidget *parent)<br>
> > > > +   : QOpenGLWidget(parent),<br>
> > > > +   glBuffer(QOpenGLBuffer::VertexBuffer),<br>
> > > > +   pFShader(nullptr),<br>
> > > > +   pVShader(nullptr),<br>
> > > > +   textureU(QOpenGLTexture::Target2D),<br>
> > > > +   textureV(QOpenGLTexture::Target2D),<br>
> > > > +   textureY(QOpenGLTexture::Target2D),<br>
> > > > +   yuvDataPtr(nullptr)<br>
> > > > +<br>
> > > > +{<br>
> > > > +}<br>
> > > > +<br>
> > > > +ViewFinderGL::~ViewFinderGL()<br>
> > > > +{<br>
> > > > +   removeShader();<br>
> > > > +<br>
> > > > +   if(textureY.isCreated())<br>
> > > > +           textureY.destroy();<br>
> > > > +<br>
> > > > +   if(textureU.isCreated())<br>
> > > > +           textureU.destroy();<br>
> > > > +<br>
> > > > +   if(textureV.isCreated())<br>
> > > > +           textureV.destroy();<br>
> > > > +<br>
> > > > +   glBuffer.destroy();<br>
> > > > +}<br>
> > > > +<br>
> > > > +int ViewFinderGL::setFormat(const libcamera::PixelFormat &format,<br>
> > > > +                       const QSize &size)<br>
> > > > +{<br>
> > > > +   format_ = format;<br>
> > > > +   size_ = size;<br>
> > > > +<br>
> > > > +   updateGeometry();<br>
> > > > +   return 0;<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>
> > > > +   unsigned char *memory = static_cast<unsigned char *>(map->memory);<br>
> > > > +   if(memory) {<br>
> ><br>
> > Can memory be null here ?<br>
><br>
> I will rewrite this.<br>
> <br>
> > > > +           yuvDataPtr = memory;<br>
> > > > +           update();<br>
> > > > +           buffer_ = buffer;<br>
> > > > +   }<br>
> > > > +<br>
> > > > +   if (buffer)<br>
> > > > +           renderComplete(buffer);<br>
> ><br>
> > Here's you're supposed to signal completion of the previous buffer, not<br>
> > the current buffer. That's why there's a std::swap() in the existing<br>
> > ViewFinder::render().<br>
> <br>
> Is That means I should<br>
> emit  renderComplete(buffer_); after update();<br>
> then<br>
> buffer_ = buffer;<br>
> ...<br>
> I don't quite understand what you mean, would you please explain it in<br>
> detail?<br>
> Forgive my stupid..:-P<br>
<br>
A stupid person couldn't have sent this patch in the first place :-)<br>
<br>
The ViewFinder component performs rendering asynchronously. It receives<br>
a libcamera buffer in render(), and is tasked to display it on the<br>
screen. The render() function returns before the rendering is complete.<br>
We thus need to tell the caller when the rendering is complete, to let<br>
it reuse the buffer once ViewFinder doesn't need it anymore. That's what<br>
renderComplete() is for.<br>
<br>
With the code above, you signal that the buffer has been rendered right<br>
after calling update(). That's not true, rendering is still in progress<br>
at that point. You can only give the buffer back to the caller with<br>
renderComplete() once a new buffer has been received to be rendered.<br>
That's why the current ViewFinder implementation has<br>
<br>
        std::swap(buffer_, buffer);<br>
<br>
After this call, buffer contains the old buffer, and we then call<br>
<br>
        if (buffer)<br>
                renderComplete(buffer);<br>
<br>
to give the old buffer back. We need to check here if buffer is null, as<br>
when the first buffer is rendered, there's no old buffer to be given<br>
back.<br></blockquote><div>Thanks for the detailed explanation. I totally misunderstood this portion before.</div><div><br></div><div><br></div><div>BR,</div><div>Show Liu</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>
> > > > +void ViewFinderGL::updateFrame(unsigned char *buffer)<br>
> > > > +{<br>
> > > > +   if(buffer) {<br>
> > > > +           yuvDataPtr = buffer;<br>
> > > > +           update();<br>
> > > > +   }<br>
> > > > +}<br>
> > > > +<br>
> > > > +void ViewFinderGL::setFrameSize(int width, int height)<br>
> > > > +{<br>
> > > > +   if(width > 0 && height > 0) {<br>
> > > > +           width_ = width;<br>
> > > > +           height_ = height;<br>
> > > > +   }<br>
> > > > +}<br>
> > > > +<br>
> > > > +char *ViewFinderGL::selectFragmentShader(unsigned int format)<br>
> > > > +{<br>
> > > > +   char *fsrc = nullptr;<br>
> > > > +<br>
> > > > +   switch(format) {<br>
> > > > +   case libcamera::formats::NV12:<br>
> > > > +           horzSubSample_ = 2;<br>
> > > > +           vertSubSample_ = 2;<br>
> > > > +           fsrc = NV_2_planes_UV;<br>
> > > > +           break;<br>
> > > > +   case libcamera::formats::NV21:<br>
> > > > +           horzSubSample_ = 2;<br>
> > > > +           vertSubSample_ = 2;<br>
> > > > +           fsrc = NV_2_planes_VU;<br>
> > > > +           break;<br>
> > > > +   case libcamera::formats::NV16:<br>
> > > > +           horzSubSample_ = 2;<br>
> > > > +           vertSubSample_ = 1;<br>
> > > > +           fsrc = NV_2_planes_UV;<br>
> > > > +           break;<br>
> > > > +   case libcamera::formats::NV61:<br>
> > > > +           horzSubSample_ = 2;<br>
> > > > +           vertSubSample_ = 1;<br>
> > > > +           fsrc = NV_2_planes_VU;<br>
> > > > +           break;<br>
> > > > +   case libcamera::formats::NV24:<br>
> > > > +           horzSubSample_ = 1;<br>
> > > > +           vertSubSample_ = 1;<br>
> > > > +           fsrc = NV_2_planes_UV;<br>
> > > > +           break;<br>
> > > > +   case libcamera::formats::NV42:<br>
> > > > +           horzSubSample_ = 1;<br>
> > > > +           vertSubSample_ = 1;<br>
> > > > +           fsrc = NV_2_planes_VU;<br>
> > > > +           break;<br>
> > > > +   case libcamera::formats::YUV420:<br>
> > > > +           horzSubSample_ = 2;<br>
> > > > +           vertSubSample_ = 2;<br>
> > > > +           fsrc = NV_3_planes_UV;<br>
> > > > +           break;<br>
> > > > +   default:<br>
> > > > +           break;<br>
> > > > +   };<br>
> > > > +<br>
> > > > +   if(fsrc == nullptr) {<br>
> > > > +           qDebug() << __func__ << "[ERROR]:" <<" No suitable fragment shader can be select.";<br>
> > > > +   }<br>
> > > > +   return fsrc;<br>
> > > > +}<br>
> > > > +<br>
> > > > +void ViewFinderGL::createFragmentShader()<br>
> > > > +{<br>
> > > > +   bool bCompile;<br>
> > > > +<br>
> > > > +   pFShader = new QOpenGLShader(QOpenGLShader::Fragment, this);<br>
> > > > +   char *fsrc = selectFragmentShader(format_);<br>
> > > > +<br>
> > > > +   bCompile = pFShader->compileSourceCode(fsrc);<br>
> > > > +   if(!bCompile)<br>
> > > > +   {<br>
> > > > +           qDebug() << __func__ << ":" << pFShader->log();<br>
> > > > +   }<br>
> > > > +<br>
> > > > +   shaderProgram.addShader(pFShader);<br>
> > > > +<br>
> > > > +   // Link shader pipeline<br>
> > > > +   if (!shaderProgram.link()) {<br>
> > > > +           qDebug() << __func__ << ": shader program link failed.\n" << shaderProgram.log();<br>
> > > > +           close();<br>
> > > > +   }<br>
> > > > +<br>
> > > > +   // Bind shader pipeline for use<br>
> > > > +   if (!shaderProgram.bind()) {<br>
> > > > +           qDebug() << __func__ << ": shader program binding failed.\n" << shaderProgram.log();<br>
> > > > +           close();<br>
> > > > +   }<br>
> > > > +<br>
> > > > +   shaderProgram.enableAttributeArray(ATTRIB_VERTEX);<br>
> > > > +   shaderProgram.enableAttributeArray(ATTRIB_TEXTURE);<br>
> > > > +<br>
> > > > +  shaderProgram.setAttributeBuffer(ATTRIB_VERTEX,GL_FLOAT,0,2,2*sizeof(GLfloat));<br>
> > > > +  shaderProgram.setAttributeBuffer(ATTRIB_TEXTURE,GL_FLOAT,8*sizeof(GLfloat),2,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>
> > > > +}<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(pFShader)<br>
> > > > +           delete pFShader;<br>
> > > > +<br>
> > > > +   if(pVShader)<br>
> > > > +           delete pVShader;<br>
> ><br>
> > If we stop and restart the stream with a different format, the previous<br>
> > shader will be used, as removeShader() isn't called. I don't think<br>
> > that's right. I believe you need to call removeShader() at the beginning<br>
> > of setFormat().<br>
><br>
> Yes. you're right. I will refine the setFormat() function.<br>
> <br>
> > > > +}<br>
> > > > +<br>
> > > > +void ViewFinderGL::initializeGL()<br>
> > > > +{<br>
> > > > +   bool bCompile;<br>
> > > > +<br>
> > > > +   initializeOpenGLFunctions();<br>
> > > > +   glEnable(GL_TEXTURE_2D);<br>
> > > > +   glDisable(GL_DEPTH_TEST);<br>
> > > > +<br>
> > > > +   static const GLfloat vertices[] {<br>
> > > > +           -1.0f,-1.0f,<br>
> > > > +           -1.0f,+1.0f,<br>
> > > > +           +1.0f,+1.0f,<br>
> > > > +           +1.0f,-1.0f,<br>
> > > > +           0.0f,1.0f,<br>
> > > > +           0.0f,0.0f,<br>
> > > > +           1.0f,0.0f,<br>
> > > > +           1.0f,1.0f,<br>
> > > > +   };<br>
> > > > +<br>
> > > > +   glBuffer.create();<br>
> > > > +   glBuffer.bind();<br>
> > > > +   glBuffer.allocate(vertices,sizeof(vertices));<br>
> > > > +<br>
> > > > +   /* Create Vertex Shader */<br>
> > > > +   pVShader = new QOpenGLShader(QOpenGLShader::Vertex, this);<br>
> > > > +   char *vsrc =  NV_Vertex_shader;<br>
> > > > +<br>
> > > > +   bCompile = pVShader->compileSourceCode(vsrc);<br>
> > > > +   if(!bCompile) {<br>
> > > > +           qDebug() << __func__<< ":" << pVShader->log();<br>
> > > > +   }<br>
> > > > +<br>
> > > > +   shaderProgram.addShader(pVShader);<br>
> > > > +<br>
> > > > +   glClearColor(1.0f, 1.0f, 1.0f, 0.0f);<br>
> > > > +}<br>
> > > > +<br>
> > > > +void ViewFinderGL::render()<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 0 */<br>
> > > > +           glActiveTexture(GL_TEXTURE0);<br>
> > > > +           configureTexture(id_y);<br>
> > > > +           glTexImage2D(GL_TEXTURE_2D, 0, GL_RED, size_.width(), size_.height(), 0, GL_RED, GL_UNSIGNED_BYTE, yuvDataPtr);<br>
> > > > +           glUniform1i(textureUniformY, 0);<br>
> > > > +<br>
> > > > +           /* activate texture 1 */<br>
> > > > +           glActiveTexture(GL_TEXTURE1);<br>
> > > > +           configureTexture(id_u);<br>
> > > > +           glTexImage2D(GL_TEXTURE_2D, 0, GL_RG, size_.width()/horzSubSample_, size_.height()/vertSubSample_, 0, GL_RG, GL_UNSIGNED_BYTE, (char*)yuvDataPtr+size_.width()*size_.height());<br>
> > > > +           glUniform1i(textureUniformU, 1);<br>
> > > > +           break;<br>
> > > > +   case libcamera::formats::YUV420:<br>
> > > > +           /* activate texture 0 */<br>
> > > > +           glActiveTexture(GL_TEXTURE0);<br>
> > > > +           configureTexture(id_y);<br>
> > > > +           glTexImage2D(GL_TEXTURE_2D, 0, GL_RED, size_.width(), size_.height(), 0, GL_RED, GL_UNSIGNED_BYTE, yuvDataPtr);<br>
> > > > +           glUniform1i(textureUniformY, 0);<br>
> > > > +<br>
> > > > +           /* activate texture 1 */<br>
> > > > +           glActiveTexture(GL_TEXTURE1);<br>
> > > > +           configureTexture(id_u);<br>
> > > > +           glTexImage2D(GL_TEXTURE_2D, 0, GL_RG, size_.width()/horzSubSample_, size_.height()/vertSubSample_, 0, GL_RG, GL_UNSIGNED_BYTE, (char*)yuvDataPtr+size_.width()*size_.height());<br>
> > > > +           glUniform1i(textureUniformU, 1);<br>
> > > > +<br>
> > > > +           /* activate texture 2 */<br>
> > > > +           glActiveTexture(GL_TEXTURE2);<br>
> > > > +           configureTexture(id_v);<br>
> > > > +           glTexImage2D(GL_TEXTURE_2D, 0, GL_RG, size_.width()/horzSubSample_, size_.height()/vertSubSample_, 0, GL_RG, GL_UNSIGNED_BYTE, (char*)yuvDataPtr+size_.width()*size_.height()*5/4);<br>
> > > > +           glUniform1i(textureUniformV, 2);<br>
> > > > +           break;<br>
> > > > +   default:<br>
> > > > +           break;<br>
> > > > +   };<br>
> > > > +}<br>
> > > > +<br>
> > > > +void ViewFinderGL::paintGL()<br>
> > > > +{<br>
> > > > +   if(pFShader == nullptr)<br>
> ><br>
> > We tend to write<br>
> ><br>
> >         if (!pFShader)<br>
><br>
> ok will fix it.<br>
> <br>
> > > > +           createFragmentShader();<br>
> ><br>
> > I wonder if we could do this in setFormat().<br>
><br>
> That should be ok,  I will try to move this when setting Format.<br>
> <br>
> > > > +<br>
> > > > +   if(yuvDataPtr)<br>
> > > > +   {<br>
> > > > +           glClearColor(0.0, 0.0, 0.0, 1.0);<br>
> > > > +           glClear( GL_COLOR_BUFFER_BIT | GL_DEPTH_BUFFER_BIT );<br>
> > > > +<br>
> > > > +           render();<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/viewfinderGL.h b/src/qcam/viewfinderGL.h<br>
> > > > new file mode 100644<br>
> > > > index 0000000..08662ca<br>
> > > > --- /dev/null<br>
> > > > +++ b/src/qcam/viewfinderGL.h<br>
> > > > @@ -0,0 +1,101 @@<br>
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */<br>
> > > > +/*<br>
> > > > + * Copyright (C) 2020, Linaro<br>
> > > > + *<br>
> > > > + * viewfinderGL.h - Render YUV format frame by OpenGL shader<br>
> > > > + */<br>
> > > > +#ifndef __VIEWFINDERGL_H__<br>
> > > > +#define __VIEWFINDERGL_H__<br>
> > > > +<br>
> > > > +#include <fcntl.h><br>
> > > > +#include <string.h><br>
> > > > +#include <unistd.h><br>
> > > > +<br>
> > > > +#include <iomanip><br>
> > > > +#include <iostream><br>
> > > > +#include <sstream><br>
> > > > +<br>
> ><br>
> > Do you need all these headers in the .h file ? They seem to belong to<br>
> > the .cpp file.<br>
><br>
> OK I will check or just remove it.<br>
> <br>
> > > > +#include <QImage><br>
> > > > +#include <QOpenGLBuffer><br>
> > > > +#include <QOpenGLFunctions><br>
> > > > +#include <QOpenGLShader><br>
> > > > +#include <QOpenGLShaderProgram><br>
> > > > +#include <QSize><br>
> > > > +#include <QOpenGLTexture><br>
> > > > +#include <QOpenGLWidget><br>
> > > > +<br>
> > > > +#include <libcamera/buffer.h><br>
> > > > +#include <libcamera/pixel_format.h><br>
> ><br>
> > Blank line missing here.<br>
><br>
> sure, will do .<br>
> <br>
> > > > +#include "viewfinder.h"<br>
> > > > +<br>
> > > > +class ViewFinderGL : public QOpenGLWidget,<br>
> > > > +                                    public ViewFinderHandler,<br>
> > > > +                                    protected QOpenGLFunctions<br>
> > > > +{<br>
> > > > +   Q_OBJECT<br>
> > > > +<br>
> > > > +public:<br>
> > > > +   ViewFinderGL(QWidget *parent = 0);<br>
> > > > +   ~ViewFinderGL();<br>
> > > > +<br>
> > > > +   int setFormat(const libcamera::PixelFormat &format, const QSize &size);<br>
> ><br>
> > You should add "override" to qualify all overridden functions.<br>
> ><br>
> >         int setFormat(const libcamera::PixelFormat &format,<br>
> >                       const QSize &size) override;<br>
><br>
> ok will do.<br>
> <br>
> > > > +   void render(libcamera::FrameBuffer *buffer, MappedBuffer *map);<br>
> > > > +   void stop();<br>
> > > > +<br>
> > > > +   QImage getCurrentImage();<br>
> > > > +<br>
> > > > +   void setFrameSize(int width, int height);<br>
> > > > +   void updateFrame(unsigned char  *buffer);<br>
> ><br>
> > Those two functions seem unused.<br>
> <br>
> sure, I will remove it.<br>
> <br>
> > > > +<br>
> > > > +   char *selectFragmentShader(unsigned int format);<br>
> > > > +   void createFragmentShader();<br>
> ><br>
> > And these two functions can be private.<br>
><br>
> will move to private.<br>
> <br>
> > > > +   void render();<br>
> > > > +<br>
> > > > +Q_SIGNALS:<br>
> > > > +   void renderComplete(libcamera::FrameBuffer *buffer);<br>
> > > > +<br>
> > > > +protected:<br>
> > > > +   void initializeGL() Q_DECL_OVERRIDE;<br>
> ><br>
> > You can use "override" directly, we know the compiler supports it.<br>
><br>
> ok, I will fix it up.<br>
> <br>
> > > > +   void paintGL() Q_DECL_OVERRIDE;<br>
> > > > +   void resizeGL(int w, int h) Q_DECL_OVERRIDE;<br>
> > > > +   QSize sizeHint() const override;<br>
> > > > +<br>
> > > > +private:<br>
> > > > +<br>
> > > > +   void configureTexture(unsigned int id);<br>
> > > > +   void removeShader();<br>
> > > > +<br>
> > > > +   /* Captured image size, format and buffer */<br>
> > > > +   libcamera::FrameBuffer *buffer_;<br>
> > > > +   libcamera::PixelFormat format_;<br>
> > > > +   QOpenGLBuffer glBuffer;<br>
> > > > +   QSize size_;<br>
> > > > +<br>
> > > > +   GLuint id_u;<br>
> > > > +   GLuint id_v;<br>
> > > > +   GLuint id_y;<br>
> > > > +<br>
> > > > +   QMutex mutex_; /* Prevent concurrent access to image_ */<br>
> > > > +<br>
> > > > +   QOpenGLShader *pFShader;<br>
> > > > +   QOpenGLShader *pVShader;<br>
> > > > +   QOpenGLShaderProgram shaderProgram;<br>
> > > > +<br>
> > > > +   GLuint textureUniformU;<br>
> > > > +   GLuint textureUniformV;<br>
> > > > +   GLuint textureUniformY;<br>
> > > > +<br>
> > > > +   QOpenGLTexture textureU;<br>
> > > > +   QOpenGLTexture textureV;<br>
> > > > +   QOpenGLTexture textureY;<br>
> > > > +<br>
> > > > +   unsigned int width_;<br>
> > > > +   unsigned int height_;<br>
> > > > +<br>
> > > > +   unsigned char* yuvDataPtr;<br>
> > > > +<br>
> > > > +   /* NV parameters */<br>
> > > > +   unsigned int horzSubSample_ ;<br>
> > > > +   unsigned int vertSubSample_;<br>
> > > > +};<br>
> > > > +#endif // __VIEWFINDERGL_H__<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>