| Message ID | 20200915024623.30667-1-laurent.pinchart@ideasonboard.com |
|---|---|
| State | Accepted |
| Delegated to: | Laurent Pinchart |
| Headers | show |
Hi Laurent, On Tue, Sep 15, 2020 at 10:46 AM Laurent Pinchart < laurent.pinchart@ideasonboard.com> 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. 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. Thanks, Show 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 > >
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
Hi Laurent, On Wed, Sep 16, 2020 at 10:33 AM Laurent Pinchart < laurent.pinchart@ideasonboard.com> wrote: > 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 ? > Yeah. you are right. Sorry, my fault. I activated the wrong texture(GL_TEXTURE1/2) in that tool. Thanks, Show > > > 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 >
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); break; default:
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. 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