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