[libcamera-devel] [PATCH v7 0/4] qcam: accelerate format conversion by OpenGL shader

Laurent Pinchart laurent.pinchart at ideasonboard.com
Wed Sep 16 04:33:07 CEST 2020


Hi Show,

On Tue, Sep 15, 2020 at 05:00:54PM +0800, Show Liu wrote:
> On Tue, Sep 15, 2020 at 10:46 AM Laurent Pinchart wrote:
> > Hello,
> >
> > This patch series is an updated version of Show's v6. I've updated the
> > patches during review as I wanted to test my review comments, and it
> > would be pointless for Show to do the same independently to post a v7.
> >
> > The series only incorporates review comments. Show, could you please let
> > me know if you're fine with the result ? To make the comparison easier,
> > here's the diff between v6 and v7.
> >
> > diff --git a/src/qcam/assets/shader/NV_2_planes_UV_f.glsl
> > b/src/qcam/assets/shader/NV_2_planes_UV_f.glsl
> > index 80478c5d0dd4..67633a11ee0f 100644
> > --- a/src/qcam/assets/shader/NV_2_planes_UV_f.glsl
> > +++ b/src/qcam/assets/shader/NV_2_planes_UV_f.glsl
> > @@ -18,10 +18,10 @@ void main(void)
> >         vec3 yuv;
> >         vec3 rgb;
> >         mat3 yuv2rgb_bt601_mat = mat3(
> > -                                     vec3(1.164,  1.164, 1.164),
> > -                                     vec3(0.000, -0.392, 2.017),
> > -                                     vec3(1.596, -0.813, 0.000)
> > -                                );
> > +               vec3(1.164,  1.164, 1.164),
> > +               vec3(0.000, -0.392, 2.017),
> > +               vec3(1.596, -0.813, 0.000)
> > +       );
> >
> >         yuv.x = texture2D(tex_y, textureOut).r - 0.063;
> >         yuv.y = texture2D(tex_u, textureOut).r - 0.500;
> > diff --git a/src/qcam/assets/shader/NV_2_planes_VU_f.glsl
> > b/src/qcam/assets/shader/NV_2_planes_VU_f.glsl
> > index 3794be843590..086c5b6d11bd 100644
> > --- a/src/qcam/assets/shader/NV_2_planes_VU_f.glsl
> > +++ b/src/qcam/assets/shader/NV_2_planes_VU_f.glsl
> > @@ -18,10 +18,10 @@ void main(void)
> >         vec3 yuv;
> >         vec3 rgb;
> >         mat3 yuv2rgb_bt601_mat = mat3(
> > -                                     vec3(1.164,  1.164, 1.164),
> > -                                     vec3(0.000, -0.392, 2.017),
> > -                                     vec3(1.596, -0.813, 0.000)
> > -                                );
> > +               vec3(1.164,  1.164, 1.164),
> > +               vec3(0.000, -0.392, 2.017),
> > +               vec3(1.596, -0.813, 0.000)
> > +       );
> >
> >         yuv.x = texture2D(tex_y, textureOut).r - 0.063;
> >         yuv.y = texture2D(tex_u, textureOut).g - 0.500;
> > diff --git a/src/qcam/assets/shader/NV_3_planes_f.glsl
> > b/src/qcam/assets/shader/NV_3_planes_f.glsl
> > index fca9b659ca05..4bc941842710 100644
> > --- a/src/qcam/assets/shader/NV_3_planes_f.glsl
> > +++ b/src/qcam/assets/shader/NV_3_planes_f.glsl
> > @@ -19,10 +19,10 @@ void main(void)
> >         vec3 yuv;
> >         vec3 rgb;
> >         mat3 yuv2rgb_bt601_mat = mat3(
> > -                                     vec3(1.164,  1.164, 1.164),
> > -                                     vec3(0.000, -0.392, 2.017),
> > -                                     vec3(1.596, -0.813, 0.000)
> > -                                );
> > +               vec3(1.164,  1.164, 1.164),
> > +               vec3(0.000, -0.392, 2.017),
> > +               vec3(1.596, -0.813, 0.000)
> > +       );
> >
> >         yuv.x = texture2D(tex_y, textureOut).r - 0.063;
> >         yuv.y = texture2D(tex_u, textureOut).r - 0.500;
> > diff --git a/src/qcam/main.cpp b/src/qcam/main.cpp
> > index 4b7d04100c08..f60d3cef0ecb 100644
> > --- a/src/qcam/main.cpp
> > +++ b/src/qcam/main.cpp
> > @@ -33,9 +33,9 @@ OptionsParser::Options parseOptions(int argc, char
> > *argv[])
> >                          ArgumentRequired, "camera");
> >         parser.addOption(OptHelp, OptionNone, "Display this help message",
> >                          "help");
> > -       parser.addOption(OptRendered, OptionString,
> > -                        "Choose the renderer type {qt,gles} (default:
> > qt)", "renderer",
> > -                        ArgumentRequired, "render");
> > +       parser.addOption(OptRenderer, OptionString,
> > +                        "Choose the renderer type {qt,gles} (default:
> > qt)",
> > +                        "renderer", ArgumentRequired, "renderer");
> >         parser.addOption(OptStream, &streamKeyValue,
> >                          "Set configuration of a camera stream", "stream",
> > true);
> >
> > diff --git a/src/qcam/main_window.cpp b/src/qcam/main_window.cpp
> > index 315102c39526..e5233f4fb706 100644
> > --- a/src/qcam/main_window.cpp
> > +++ b/src/qcam/main_window.cpp
> > @@ -28,6 +28,8 @@
> >  #include <libcamera/version.h>
> >
> >  #include "dng_writer.h"
> > +#include "viewfinder_gl.h"
> > +#include "viewfinder_qt.h"
> >
> >  using namespace libcamera;
> >
> > @@ -95,9 +97,6 @@ MainWindow::MainWindow(CameraManager *cm, const
> > OptionsParser::Options &options)
> >  {
> >         int ret;
> >
> > -       /* Render Type Qt or GLES, and set Qt by default */
> > -       std::string renderType_ = "qt";
> > -
> >         /*
> >          * Initialize the UI: Create the toolbar, set the window title and
> >          * create the viewfinder widget.
> > @@ -108,17 +107,19 @@ MainWindow::MainWindow(CameraManager *cm, const
> > OptionsParser::Options &options)
> >         setWindowTitle(title_);
> >         connect(&titleTimer_, SIGNAL(timeout()), this,
> > SLOT(updateTitle()));
> >
> > -       if (options_.isSet(OptRendered))
> > -               renderType_ = options_[OptRendered].toString();
> > +       /* Renderer type Qt or GLES, select Qt by default. */
> > +       std::string renderType = "qt";
> > +       if (options_.isSet(OptRenderer))
> > +               renderType = options_[OptRenderer].toString();
> >
> > -       if (renderType_ == "qt") {
> > +       if (renderType == "qt") {
> >                 ViewFinderQt *viewfinder = new ViewFinderQt(this);
> >                 connect(viewfinder, &ViewFinderQt::renderComplete,
> >                         this, &MainWindow::queueRequest);
> >                 viewfinder_ = viewfinder;
> >                 setCentralWidget(viewfinder);
> >  #ifndef QT_NO_OPENGL
> > -       } else if (renderType_ == "gles") {
> > +       } else if (renderType == "gles") {
> >                 ViewFinderGL *viewfinder = new ViewFinderGL(this);
> >                 connect(viewfinder, &ViewFinderGL::renderComplete,
> >                         this, &MainWindow::queueRequest);
> > @@ -127,7 +128,7 @@ MainWindow::MainWindow(CameraManager *cm, const
> > OptionsParser::Options &options)
> >  #endif
> >         } else {
> >                 qWarning() << "Invalid render type"
> > -                          << QString::fromStdString(renderType_);
> > +                          << QString::fromStdString(renderType);
> >                 quit();
> >                 return;
> >         }
> > diff --git a/src/qcam/main_window.h b/src/qcam/main_window.h
> > index 251f78bf6833..5c61a4dfce53 100644
> > --- a/src/qcam/main_window.h
> > +++ b/src/qcam/main_window.h
> > @@ -26,8 +26,6 @@
> >
> >  #include "../cam/stream_options.h"
> >  #include "viewfinder.h"
> > -#include "viewfinder_gl.h"
> > -#include "viewfinder_qt.h"
> >
> >  using namespace libcamera;
> >
> > @@ -39,7 +37,7 @@ class HotplugEvent;
> >  enum {
> >         OptCamera = 'c',
> >         OptHelp = 'h',
> > -       OptRendered = 'r',
> > +       OptRenderer = 'r',
> >         OptStream = 's',
> >  };
> >
> > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
> > index 84f48666af5b..fbe21dcf1ad2 100644
> > --- a/src/qcam/viewfinder_gl.cpp
> > +++ b/src/qcam/viewfinder_gl.cpp
> > @@ -19,7 +19,7 @@ static const QList<libcamera::PixelFormat>
> > supportedFormats{
> >         libcamera::formats::NV24,
> >         libcamera::formats::NV42,
> >         libcamera::formats::YUV420,
> > -       libcamera::formats::YVU420
> > +       libcamera::formats::YVU420,
> >  };
> >
> >  ViewFinderGL::ViewFinderGL(QWidget *parent)
> > @@ -35,9 +35,6 @@ ViewFinderGL::ViewFinderGL(QWidget *parent)
> >  ViewFinderGL::~ViewFinderGL()
> >  {
> >         removeShader();
> > -
> > -       if (vertexBuffer_.isCreated())
> > -               vertexBuffer_.destroy();
> >  }
> >
> >  const QList<libcamera::PixelFormat> &ViewFinderGL::nativeFormats() const
> > @@ -48,9 +45,7 @@ const QList<libcamera::PixelFormat>
> > &ViewFinderGL::nativeFormats() const
> >  int ViewFinderGL::setFormat(const libcamera::PixelFormat &format,
> >                             const QSize &size)
> >  {
> > -       int ret = 0;
> > -
> > -       /* If the fragment is ceeated remove it and create a new one */
> > +       /* If the fragment is created remove it and create a new one. */
> >         if (fragmentShader_) {
> >                 if (shaderProgram_.isLinked()) {
> >                         shaderProgram_.release();
> > @@ -59,14 +54,14 @@ int ViewFinderGL::setFormat(const
> > libcamera::PixelFormat &format,
> >                 }
> >         }
> >
> > -       if (selectFormat(format)) {
> > -               format_ = format;
> > -               size_ = size;
> > -       } else {
> > -               ret = -1;
> > -       }
> > +       if (!selectFormat(format))
> > +               return -1;
> > +
> > +       format_ = format;
> > +       size_ = size;
> > +
> >         updateGeometry();
> > -       return ret;
> > +       return 0;
> >  }
> >
> >  void ViewFinderGL::stop()
> > @@ -279,10 +274,10 @@ void ViewFinderGL::initializeGL()
> >                 },
> >                 {
> >                         /* Texture coordinates */
> > -                       { 1.0f, 0.0f },
> > -                       { 1.0f, 1.0f },
> >                         { 0.0f, 1.0f },
> >                         { 0.0f, 0.0f },
> > +                       { 1.0f, 0.0f },
> > +                       { 1.0f, 1.0f },
> >                 },
> >         };
> >
> > @@ -306,7 +301,7 @@ void ViewFinderGL::doRender()
> >         case libcamera::formats::NV61:
> >         case libcamera::formats::NV24:
> >         case libcamera::formats::NV42:
> > -               /* activate texture Y */
> > +               /* Activate texture Y */
> >                 glActiveTexture(GL_TEXTURE0);
> >                 configureTexture(id_y_);
> >                 glTexImage2D(GL_TEXTURE_2D,
> > @@ -320,7 +315,7 @@ void ViewFinderGL::doRender()
> >                              yuvData_);
> >                 shaderProgram_.setUniformValue(textureUniformY_, 0);
> >
> > -               /* activate texture UV/VU */
> > +               /* Activate texture UV/VU */
> >                 glActiveTexture(GL_TEXTURE1);
> >                 configureTexture(id_u_);
> >                 glTexImage2D(GL_TEXTURE_2D,
> > @@ -336,7 +331,7 @@ void ViewFinderGL::doRender()
> >                 break;
> >
> >         case libcamera::formats::YUV420:
> > -               /* activate texture Y */
> > +               /* Activate texture Y */
> >                 glActiveTexture(GL_TEXTURE0);
> >                 configureTexture(id_y_);
> >                 glTexImage2D(GL_TEXTURE_2D,
> > @@ -350,7 +345,7 @@ void ViewFinderGL::doRender()
> >                              yuvData_);
> >                 shaderProgram_.setUniformValue(textureUniformY_, 0);
> >
> > -               /* activate texture U */
> > +               /* Activate texture U */
> >                 glActiveTexture(GL_TEXTURE1);
> >                 configureTexture(id_u_);
> >                 glTexImage2D(GL_TEXTURE_2D,
> > @@ -364,7 +359,7 @@ void ViewFinderGL::doRender()
> >                              (char *)yuvData_ + size_.width() *
> > size_.height());
> >                 shaderProgram_.setUniformValue(textureUniformU_, 1);
> >
> > -               /* activate texture V */
> > +               /* Activate texture V */
> >                 glActiveTexture(GL_TEXTURE2);
> >                 configureTexture(id_v_);
> >                 glTexImage2D(GL_TEXTURE_2D,
> > @@ -380,7 +375,7 @@ void ViewFinderGL::doRender()
> >                 break;
> >
> >         case libcamera::formats::YVU420:
> > -               /* activate texture Y */
> > +               /* Activate texture Y */
> >                 glActiveTexture(GL_TEXTURE0);
> >                 configureTexture(id_y_);
> >                 glTexImage2D(GL_TEXTURE_2D,
> > @@ -394,7 +389,7 @@ void ViewFinderGL::doRender()
> >                              yuvData_);
> >                 shaderProgram_.setUniformValue(textureUniformY_, 0);
> >
> > -               /* activate texture V */
> > +               /* Activate texture V */
> >                 glActiveTexture(GL_TEXTURE2);
> >                 configureTexture(id_v_);
> >                 glTexImage2D(GL_TEXTURE_2D,
> > @@ -406,9 +401,9 @@ void ViewFinderGL::doRender()
> >                              GL_RED,
> >                              GL_UNSIGNED_BYTE,
> >                              (char *)yuvData_ + size_.width() *
> > size_.height());
> > -               shaderProgram_.setUniformValue(textureUniformV_, 1);
> > +               shaderProgram_.setUniformValue(textureUniformV_, 2);
> >
> > -               /* activate texture U */
> > +               /* Activate texture U */
> >                 glActiveTexture(GL_TEXTURE1);
> >                 configureTexture(id_u_);
> >                 glTexImage2D(GL_TEXTURE_2D,
> > @@ -420,7 +415,7 @@ void ViewFinderGL::doRender()
> >                              GL_RED,
> >                              GL_UNSIGNED_BYTE,
> >                              (char *)yuvData_ + size_.width() *
> > size_.height() * 5 / 4);
> > -               shaderProgram_.setUniformValue(textureUniformU_, 2);
> > +               shaderProgram_.setUniformValue(textureUniformU_, 1);
> >
> 
> I applied the above changes and tested on my rockpi4b with imx219. It's
> running well.
> But I have only the imx219 camera module to test with qcam, which means
> qcam only verified NV12 format is correct.
> So I use another small tool to test other formats and shader code.

Would it be possible to share that tool ?

> When the format YVU420 and YUV420 use the same shader code.
> +               shaderProgram_.setUniformValue(textureUniformV_, 2);
> ...
> +               shaderProgram_.setUniformValue(textureUniformU_, 1);
> above change cause the color became blue in format YVU420.
> The original value is correct I think.

There's something I don't get then. The common shader is written as

        yuv.x = texture2D(tex_y, textureOut).r - 0.063;
        yuv.y = texture2D(tex_u, textureOut).r - 0.500;
        yuv.z = texture2D(tex_v, textureOut).r - 0.500;

so it expects Y in the tex_y uniform, U in the tex_u uniform and V in
the tex_v uniform. We retrieve the uniforms with

        textureUniformY_ = shaderProgram_.uniformLocation("tex_y");
        textureUniformU_ = shaderProgram_.uniformLocation("tex_u");
        textureUniformV_ = shaderProgram_.uniformLocation("tex_v");

For YUV420, we have

        case libcamera::formats::YUV420:
                /* Activate texture Y */
                glActiveTexture(GL_TEXTURE0);
                configureTexture(id_y_);
                glTexImage2D(...);
                shaderProgram_.setUniformValue(textureUniformY_, 0);

                /* Activate texture U */
                glActiveTexture(GL_TEXTURE1);
                configureTexture(id_u_);
                glTexImage2D(GL_TEXTURE_2D,
                             0,
                             GL_RED,
                             size_.width() / horzSubSample_,
                             size_.height() / vertSubSample_,
                             0,
                             GL_RED,
                             GL_UNSIGNED_BYTE,
                             (char *)yuvData_ + size_.width() * size_.height());
                shaderProgram_.setUniformValue(textureUniformU_, 1);

                /* Activate texture V */
                glActiveTexture(GL_TEXTURE2);
                configureTexture(id_v_);
                glTexImage2D(GL_TEXTURE_2D,
                             0,
                             GL_RED,
                             size_.width() / horzSubSample_,
                             size_.height() / vertSubSample_,
                             0,
                             GL_RED,
                             GL_UNSIGNED_BYTE,
                             (char *)yuvData_ + size_.width() * size_.height() * 5 / 4);
                shaderProgram_.setUniformValue(textureUniformV_, 2);

We store 0, which I assume corresponds to GL_TEXTURE0, in
textureUniformY_, 1 in textureUniformU_ and 2 in textureUniformV_.
GL_TEXTURE[012] are filled with the Y, U and V data respectively. So
far, so good.

For YVU420, we have

        case libcamera::formats::YVU420:
                /* Activate texture Y */
                glActiveTexture(GL_TEXTURE0);
                configureTexture(id_y_);
                glTexImage2D(...);
                shaderProgram_.setUniformValue(textureUniformY_, 0);

                /* Activate texture V */
                glActiveTexture(GL_TEXTURE2);
                configureTexture(id_v_);
                glTexImage2D(GL_TEXTURE_2D,
                             0,
                             GL_RED,
                             size_.width() / horzSubSample_,
                             size_.height() / vertSubSample_,
                             0,
                             GL_RED,
                             GL_UNSIGNED_BYTE,
                             (char *)yuvData_ + size_.width() * size_.height());
                shaderProgram_.setUniformValue(textureUniformV_, 2);

                /* Activate texture U */
                glActiveTexture(GL_TEXTURE1);
                configureTexture(id_u_);
                glTexImage2D(GL_TEXTURE_2D,
                             0,
                             GL_RED,
                             size_.width() / horzSubSample_,
                             size_.height() / vertSubSample_,
                             0,
                             GL_RED,
                             GL_UNSIGNED_BYTE,
                             (char *)yuvData_ + size_.width() * size_.height() * 5 / 4);
                shaderProgram_.setUniformValue(textureUniformU_, 1);

We handle Y the same way. We also store V in GL_TEXTURE2, and set
textureUniformV_ to 2 accordingly. The difference is in the
glTexImage2D() call, that's where we swap U and V. What am I missing ?

> >               break;
> >
> >         default:
> >
> > Show Liu (4):
> >   qcam: Add OpenGL shader code as Qt resource
> >   qcam: New viewfinder hierarchy
> >   qcam: Add ViewFinderGL class to accelerate the format conversion
> >   qcam: Add additional command line option to select the renderer type
> >
> >  meson.build                                   |   1 +
> >  src/qcam/assets/shader/NV_2_planes_UV_f.glsl  |  32 ++
> >  src/qcam/assets/shader/NV_2_planes_VU_f.glsl  |  32 ++
> >  src/qcam/assets/shader/NV_3_planes_f.glsl     |  33 ++
> >  src/qcam/assets/shader/NV_vertex_shader.glsl  |  16 +
> >  src/qcam/assets/shader/shaders.qrc            |   9 +
> >  src/qcam/main.cpp                             |   3 +
> >  src/qcam/main_window.cpp                      |  32 +-
> >  src/qcam/main_window.h                        |   1 +
> >  src/qcam/meson.build                          |  20 +-
> >  src/qcam/viewfinder.h                         |  57 +--
> >  src/qcam/viewfinder_gl.cpp                    | 451 ++++++++++++++++++
> >  src/qcam/viewfinder_gl.h                      |  96 ++++
> >  .../{viewfinder.cpp => viewfinder_qt.cpp}     |  24 +-
> >  src/qcam/viewfinder_qt.h                      |  64 +++
> >  15 files changed, 806 insertions(+), 65 deletions(-)
> >  create mode 100644 src/qcam/assets/shader/NV_2_planes_UV_f.glsl
> >  create mode 100644 src/qcam/assets/shader/NV_2_planes_VU_f.glsl
> >  create mode 100644 src/qcam/assets/shader/NV_3_planes_f.glsl
> >  create mode 100644 src/qcam/assets/shader/NV_vertex_shader.glsl
> >  create mode 100644 src/qcam/assets/shader/shaders.qrc
> >  create mode 100644 src/qcam/viewfinder_gl.cpp
> >  create mode 100644 src/qcam/viewfinder_gl.h
> >  rename src/qcam/{viewfinder.cpp => viewfinder_qt.cpp} (86%)
> >  create mode 100644 src/qcam/viewfinder_qt.h

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list