<div dir="ltr"><div dir="ltr"><div>Hi Laurent,</div><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Aug 26, 2020 at 3:37 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">Hi Show,<br>
<br>
On Tue, Aug 25, 2020 at 05:38:55PM +0800, Show Liu wrote:<br>
> On Mon, Aug 24, 2020 at 8:07 AM Laurent Pinchart wrote:<br>
> > On Mon, Aug 24, 2020 at 03:04:56AM +0300, Laurent Pinchart wrote:<br>
> > > On Sat, Aug 22, 2020 at 12:16:00AM +0800, Show Liu wrote:<br>
> > > > qcam: add OpenGL shader code as QT resource<br>
> > ><br>
> > > s/QT/Qt/<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/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_UV_f.glsl | 33 ++++++++++++++++++++<br>
> > > >  src/qcam/assets/shader/NV_3_planes_VU_f.glsl | 33 ++++++++++++++++++++<br>
> > > >  src/qcam/assets/shader/NV_vertex_shader.glsl | 16 ++++++++++<br>
> > > >  src/qcam/assets/shader/shaders.qrc           | 10 ++++++<br>
> > > >  src/qcam/meson.build                         |  1 +<br>
> > > >  7 files changed, 157 insertions(+)<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_UV_f.glsl<br>
> > > >  create mode 100644 src/qcam/assets/shader/NV_3_planes_VU_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>
> > > ><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>
> > > > new file mode 100644<br>
> > > > index 0000000..32c6e90<br>
> > > > --- /dev/null<br>
> > > > +++ b/src/qcam/assets/shader/NV_2_planes_UV_f.glsl<br>
> > > > @@ -0,0 +1,32 @@<br>
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */<br>
> > > > +/*<br>
> > > > + * Copyright (C) 2020, Linaro<br>
> > > > + *<br>
> > > > + * NV_2_planes_UV_f.glsl - Fragment shader code for NV12, NV16 and NV24 formats<br>
> > > > + */<br>
> > > > +<br>
> > > > +#ifdef GL_ES<br>
> > > > +precision highp float;<br>
> > > > +#endif<br>
> > > > +<br>
> > > > +varying vec2 textureOut;<br>
> > > > +uniform sampler2D tex_y;<br>
> > > > +uniform sampler2D tex_u;<br>
> > > > +<br>
> > > > +void main(void)<br>
> > > > +{<br>
> > > > +   vec3 yuv;<br>
> > > > +   vec3 rgb;<br>
> > > > +   mat3 convert_mat = mat3(<br>
> > ><br>
> > > Maybe yuv2rgb or yuv2rgb_mat instead of convert_mat ? Or yuv2rgb_bt601<br>
> > > or yuv2rgb_bt601_mat ?<br>
><br>
> sure, I will fix it.<br>
> <br>
> > > > +                                                   vec3(1.1640625, 1.1640625, 1.1640625),<br>
> > > > +                                                   vec3(0.0, -0.390625, 2.015625),<br>
> > > > +                                                   vec3(1.5975625, -0.8125, 0.0)<br>
> > ><br>
> > > Where are the coefficients from ? <a href="https://en.wikipedia.org/wiki/YCbCr" rel="noreferrer" target="_blank">https://en.wikipedia.org/wiki/YCbCr</a><br>
> > > lists values very similar but slightly different. Are the values here<br>
> > > rounded to a multiple of 1/256 on purpose (except for 1.5975625) ?<br>
><br>
> The coefficients matrix refer from the yuv_to_rgb function in<br>
> format_converter.cpp file<br>
> 298/256=1.1640625<br>
> I just conert the elements into floating by dividing 256.<br>
<br>
Of course. I should have looked there :-) That code uses fixed-point<br>
calculations to speed things up. As we can use floating point values<br>
here, let's specify the exact values where possible.<br>
<br>
Can we use arithemtic expression to let the shader compiler pick the<br>
highest precision ?<br>
<br>
                vec3(255.0/219.0, 255.0/219.0, 255.0/219.0),<br>
                vec3(0.0, -255.0/224.0*1.772*0.114/0.587, 255.0/224.0*1.772),<br>
                vec3(255.0/224.0*1.402, -255.0/224.0*1.402*0.299/0.587, 0.0)<br>
<br>
or should we use numerical values directly ? I suppose a 8-bit precision<br>
will be enough in practice as that's what we have in the input image and<br>
what we will render, so rounding after 3 decimal points should be fine<br>
and would probably be more readable.<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>
What do you think ?<br></blockquote><div> </div><div>Sure. I will try to apply it on the next version. </div><div><br></div><div><br></div><div>Thanks,</div><div>Show</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>
> > > The indentation is a bit weird.<br>
> > ><br>
> > > > +<br>
> > > > +   yuv.x = texture2D(tex_y, textureOut).r - 0.0625;<br>
> > ><br>
> > > Is 0.0625 (16/256) the right value, or should it be 16/255 ? The Y range<br>
> > > on 8-bit is 0-255, which is mapped to 0.0-1.0, right ?<br>
> > ><br>
> > > > +   yuv.y = texture2D(tex_u, textureOut).r - 0.5;<br>
> > > > +   yuv.z = texture2D(tex_u, textureOut).g - 0.5;<br>
> > > > +<br>
> > > > +   rgb = convert_mat * yuv;<br>
> > > > +   gl_FragColor = vec4(rgb, 1.0);<br>
> > > > +}<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>
> > > > new file mode 100644<br>
> > > > index 0000000..aae12de<br>
> > > > --- /dev/null<br>
> > > > +++ b/src/qcam/assets/shader/NV_2_planes_VU_f.glsl<br>
> > > > @@ -0,0 +1,32 @@<br>
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */<br>
> > > > +/*<br>
> > > > + * Copyright (C) 2020, Linaro<br>
> > > > + *<br>
> > > > + * NV_2_planes_VU_f.glsl - Fragment shader code for NV21, NV61 and NV42 formats<br>
> > > > + */<br>
> > > > +<br>
> > > > +#ifdef GL_ES<br>
> > > > +precision highp float;<br>
> > > > +#endif<br>
> > > > +<br>
> > > > +varying vec2 textureOut;<br>
> > > > +uniform sampler2D tex_y;<br>
> > > > +uniform sampler2D tex_u;<br>
> > > > +<br>
> > > > +void main(void)<br>
> > > > +{<br>
> > > > +   vec3 yuv;<br>
> > > > +   vec3 rgb;<br>
> > > > +   mat3 convert_mat = mat3(<br>
> > > > +                                                   vec3(1.1640625, 1.1640625, 1.1640625),<br>
> > > > +                                                   vec3(0.0, -0.390625, 2.015625),<br>
> > > > +                                                   vec3(1.5975625, -0.8125, 0.0)<br>
> > > > +                                              );<br>
> > ><br>
> > > Here too.<br>
> > ><br>
> > > > +<br>
> > > > +   yuv.x = texture2D(tex_y, textureOut).r - 0.0625;<br>
> > > > +   yuv.y = texture2D(tex_u, textureOut).g - 0.5;<br>
> > > > +   yuv.z = texture2D(tex_u, textureOut).r - 0.5;<br>
> > > > +<br>
> > ><br>
> > > And there are some white spaces at the end of lines (below too).<br>
><br>
> ok. I will remove it.<br>
> <br>
> > ><br>
> > > We'll have to make the coefficients configurable to support different<br>
> > > colorspaces, but that's for later.<br>
> > ><br>
> > > > +   rgb = convert_mat * yuv;<br>
> > > > +   gl_FragColor = vec4(rgb, 1.0);<br>
> > > > +}<br>
> > > > diff --git a/src/qcam/assets/shader/NV_3_planes_UV_f.glsl b/src/qcam/assets/shader/NV_3_planes_UV_f.glsl<br>
> > > > new file mode 100644<br>
> > > > index 0000000..21fff3a<br>
> > > > --- /dev/null<br>
> > > > +++ b/src/qcam/assets/shader/NV_3_planes_UV_f.glsl<br>
> > > > @@ -0,0 +1,33 @@<br>
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */<br>
> > > > +/*<br>
> > > > + * Copyright (C) 2020, Linaro<br>
> > > > + *<br>
> > > > + * NV_3_planes_UV_f.glsl - Fragment shader code for YUV420 format<br>
> > > > + */<br>
> > > > +<br>
> > > > +#ifdef GL_ES<br>
> > > > +precision highp float;<br>
> > > > +#endif<br>
> > > > +<br>
> > > > +varying vec2 textureOut;<br>
> > > > +uniform sampler2D tex_y;<br>
> > > > +uniform sampler2D tex_u;<br>
> > > > +uniform sampler2D tex_v;<br>
> > > > +<br>
> > > > +void main(void)<br>
> > > > +{<br>
> > > > +   vec3 yuv;<br>
> > > > +   vec3 rgb;<br>
> > > > +   mat3 convert_mat = mat3(<br>
> > > > +                                                   vec3(1.1640625, 1.1640625, 1.1640625),<br>
> > > > +                                                   vec3(0.0, -0.390625, 2.015625),<br>
> > > > +                                                   vec3(1.5975625, -0.8125, 0.0)<br>
> > > > +                                              );<br>
> > > > +<br>
> > > > +   yuv.x = texture2D(tex_y, textureOut).r - 0.0625;<br>
> > > > +   yuv.y = texture2D(tex_u, textureOut).r - 0.5;<br>
> > > > +   yuv.z = texture2D(tex_v, textureOut).g - 0.5;<br>
> > ><br>
> > > I'm not sure to understand the .g here, with YUV420 having three planes,<br>
> > > shouldn't it be .r ?<br>
> <br>
> Yes. you are right. it's a problem here. I will fix it.<br>
> <br>
> > I could be wrong, my GLSL knowledge is very very rudimentary :-)<br>
><br>
> You definitely are a GLSL expert...don't cheat me, I know that. :-)<br>
> <br>
> > > > +<br>
> > > > +   rgb = convert_mat * yuv;<br>
> > > > +   gl_FragColor = vec4(rgb, 1.0);<br>
> > > > +}<br>
> > > > diff --git a/src/qcam/assets/shader/NV_3_planes_VU_f.glsl b/src/qcam/assets/shader/NV_3_planes_VU_f.glsl<br>
> > > > new file mode 100644<br>
> > > > index 0000000..df00170<br>
> > > > --- /dev/null<br>
> > > > +++ b/src/qcam/assets/shader/NV_3_planes_VU_f.glsl<br>
> > > > @@ -0,0 +1,33 @@<br>
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */<br>
> > > > +/*<br>
> > > > + * Copyright (C) 2020, Linaro<br>
> > > > + *<br>
> > > > + * NV_3_planes_VU_f.glsl - Fragment shader code for YVU420 format<br>
> > > > + */<br>
> > > > +<br>
> > > > +#ifdef GL_ES<br>
> > > > +precision highp float;<br>
> > > > +#endif<br>
> > > > +<br>
> > > > +varying vec2 textureOut;<br>
> > > > +uniform sampler2D tex_y;<br>
> > > > +uniform sampler2D tex_u;<br>
> > > > +uniform sampler2D tex_v;<br>
> > > > +<br>
> > > > +void main(void)<br>
> > > > +{<br>
> > > > +   vec3 yuv;<br>
> > > > +   vec3 rgb;<br>
> > > > +   mat3 convert_mat = mat3(<br>
> > > > +                                                   vec3(1.1640625, 1.1640625, 1.1640625),<br>
> > > > +                                                   vec3(0.0, -0.390625, 2.015625),<br>
> > > > +                                                   vec3(1.5975625, -0.8125, 0.0)<br>
> > > > +                                              );<br>
> > > > +<br>
> > > > +   yuv.x = texture2D(tex_y, textureOut).r - 0.0625;<br>
> > > > +   yuv.y = texture2D(tex_u, textureOut).g - 0.5;<br>
> > > > +   yuv.z = texture2D(tex_v, textureOut).r - 0.5;<br>
> > ><br>
> > > Same here, I was expecting<br>
><br>
> will fix it.<br>
> <br>
> > >       yuv.x = texture2D(tex_y, textureOut).r - 0.0625;<br>
> > >       yuv.y = texture2D(tex_v, textureOut).r - 0.5;<br>
> > >       yuv.z = texture2D(tex_u, textureOut).r - 0.5;<br>
> > ><br>
> > > > +<br>
> > > > +   rgb = convert_mat * yuv;<br>
> > > > +   gl_FragColor = vec4(rgb, 1.0);<br>
> > > > +}<br>
> > > > diff --git a/src/qcam/assets/shader/NV_vertex_shader.glsl<br>
> > b/src/qcam/assets/shader/NV_vertex_shader.glsl<br>
> > > > new file mode 100644<br>
> > > > index 0000000..403b791<br>
> > > > --- /dev/null<br>
> > > > +++ b/src/qcam/assets/shader/NV_vertex_shader.glsl<br>
> > > > @@ -0,0 +1,16 @@<br>
> > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */<br>
> > > > +/*<br>
> > > > + * Copyright (C) 2020, Linaro<br>
> > > > + *<br>
> > > > + * NV_vertex_shader.glsl - Vertex shader code for NV family<br>
> > > > + */<br>
> > > > +<br>
> > > > +attribute vec4 vertexIn;<br>
> > > > +attribute vec2 textureIn;<br>
> > > > +varying vec2 textureOut;<br>
> > > > +<br>
> > > > +void main(void)<br>
> > > > +{<br>
> > > > +   gl_Position = vertexIn;<br>
> > > > +   textureOut = textureIn;<br>
> > > > +}<br>
> > > > diff --git a/src/qcam/assets/shader/shaders.qrc<br>
> > b/src/qcam/assets/shader/shaders.qrc<br>
> > > > new file mode 100644<br>
> > > > index 0000000..6fe4c7f<br>
> > > > --- /dev/null<br>
> > > > +++ b/src/qcam/assets/shader/shaders.qrc<br>
> > > > @@ -0,0 +1,10 @@<br>
> > > > +<!-- SPDX-License-Identifier: GPL-2.0-or-later --><br>
> > > > +<!DOCTYPE RCC><RCC version="1.0"><br>
> > > > +<qresource><br>
> > > > +<file>./NV_vertex_shader.glsl</file><br>
> > > > +<file>./NV_2_planes_UV_f.glsl</file><br>
> > > > +<file>./NV_2_planes_VU_f.glsl</file><br>
> > > > +<file>./NV_3_planes_UV_f.glsl</file><br>
> > > > +<file>./NV_3_planes_VU_f.glsl</file><br>
> > > > +</qresource><br>
> > > > +</RCC><br>
> > > > diff --git a/src/qcam/meson.build b/src/qcam/meson.build<br>
> > > > index 6ea886a..e0c6f26 100644<br>
> > > > --- a/src/qcam/meson.build<br>
> > > > +++ b/src/qcam/meson.build<br>
> > > > @@ -16,6 +16,7 @@ qcam_moc_headers = files([<br>
> > > ><br>
> > > >  qcam_resources = files([<br>
> > > >      'assets/feathericons/feathericons.qrc',<br>
> > > > +    'assets/shader/shaders.qrc'<br>
> > > >  ])<br>
> > > ><br>
> > > >  qt5 = import('qt5')<br>
<br>
-- <br>
Regards,<br>
<br>
Laurent Pinchart<br>
</blockquote></div></div>