[libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support for RGB formats
Kieran Bingham
kieran.bingham at ideasonboard.com
Wed Nov 4 10:49:43 CET 2020
Hi Laurent,
On 04/11/2020 09:32, Andrey Konovalov wrote:
> Hi Laurent,
>
> On 04.11.2020 05:09, Laurent Pinchart wrote:
>> Hi Kieran,
>>
>> On Tue, Nov 03, 2020 at 11:00:28PM +0000, Kieran Bingham wrote:
>>> On 03/11/2020 15:50, Laurent Pinchart wrote:
>>>> Add support for 24-bit and 32-bit RGB formats. The fragment samples the
>>>> texture and reorders the components, using a pattern set through the
>>>> RGB_PATTERN macro.
>>>>
>>>> An alternative to manual reordering in the shader would be to set the
>>>> texture swizzling mask. This is however not available in OpenGL ES
>>>> before version 3.0, which we don't mandate at the moment.
>>>>
>>>> Only the BGR888 and RGB888 formats have been tested, with the vimc
>>>> pipeline handler.
>>>>
>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>>>> ---
>>>> src/qcam/assets/shader/RGB.frag | 22 +++++++++
>>>> src/qcam/assets/shader/shaders.qrc | 1 +
>>>> src/qcam/viewfinder_gl.cpp | 71
>>>> ++++++++++++++++++++++++++++--
>>>> 3 files changed, 91 insertions(+), 3 deletions(-)
>>>> create mode 100644 src/qcam/assets/shader/RGB.frag
>>>>
>>>> diff --git a/src/qcam/assets/shader/RGB.frag
>>>> b/src/qcam/assets/shader/RGB.frag
>>>> new file mode 100644
>>>> index 000000000000..4c374ac98095
>>>> --- /dev/null
>>>> +++ b/src/qcam/assets/shader/RGB.frag
>>>> @@ -0,0 +1,22 @@
>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
>>>> +/*
>>>> + * Copyright (C) 2020, Laurent Pinchart
>>>> + *
>>>> + * RGB.frag - Fragment shader code for RGB formats
>>>> + */
>>>> +
>>>> +#ifdef GL_ES
>>>> +precision mediump float;
>>>> +#endif
>>>> +
>>>> +varying vec2 textureOut;
>>>> +uniform sampler2D tex_y;
>>>> +
>>>> +void main(void)
>>>> +{
>>>> + vec3 rgb;
>>>> +
>>>> + rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
>>>> +
>>>> + gl_FragColor = vec4(rgb, 1.0);
>>>> +}
>>>> diff --git a/src/qcam/assets/shader/shaders.qrc
>>>> b/src/qcam/assets/shader/shaders.qrc
>>>> index 863109146281..8a8f9de1a5fa 100644
>>>> --- a/src/qcam/assets/shader/shaders.qrc
>>>> +++ b/src/qcam/assets/shader/shaders.qrc
>>>> @@ -1,6 +1,7 @@
>>>> <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->
>>>> <!DOCTYPE RCC><RCC version="1.0">
>>>> <qresource>
>>>> + <file>RGB.frag</file>
>>>> <file>YUV_2_planes.frag</file>
>>>> <file>YUV_3_planes.frag</file>
>>>> <file>YUV_packed.frag</file>
>>>> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp
>>>> index cbc1365500f5..5d9b442e7985 100644
>>>> --- a/src/qcam/viewfinder_gl.cpp
>>>> +++ b/src/qcam/viewfinder_gl.cpp
>>>> @@ -14,21 +14,28 @@
>>>> #include <libcamera/formats.h>
>>>> static const QList<libcamera::PixelFormat> supportedFormats{
>>>> - /* Packed (single plane) */
>>>> + /* YUV - packed (single plane) */
>>>> libcamera::formats::UYVY,
>>>> libcamera::formats::VYUY,
>>>> libcamera::formats::YUYV,
>>>> libcamera::formats::YVYU,
>>>> - /* Semi planar (two planes) */
>>>> + /* YUV - semi planar (two planes) */
>>>> libcamera::formats::NV12,
>>>> libcamera::formats::NV21,
>>>> libcamera::formats::NV16,
>>>> libcamera::formats::NV61,
>>>> libcamera::formats::NV24,
>>>> libcamera::formats::NV42,
>>>> - /* Fully planar (three planes) */
>>>> + /* YUV - fully planar (three planes) */
>>>> libcamera::formats::YUV420,
>>>> libcamera::formats::YVU420,
>>>> + /* RGB */
>>>> + libcamera::formats::ABGR8888,
>>>> + libcamera::formats::ARGB8888,
>>>> + libcamera::formats::BGRA8888,
>>>> + libcamera::formats::RGBA8888,
>>>> + libcamera::formats::BGR888,
>>>> + libcamera::formats::RGB888,
>>>> };
>>>> ViewFinderGL::ViewFinderGL(QWidget *parent)
>>>> @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const
>>>> libcamera::PixelFormat &format)
>>>> fragmentShaderDefines_.append("#define YUV_PATTERN_YVYU");
>>>> fragmentShaderFile_ = ":YUV_packed.frag";
>>>> break;
>>>> + case libcamera::formats::ABGR8888:
>>>> + fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
>>>
>>> pattern rgb, the same as BGR888, so I presume the A is stripped by not
>>> being used as a channel or some other magic elsewhere?
>>>
>>>> + fragmentShaderFile_ = ":RGB.frag";
>>>> + break;
>>>> + case libcamera::formats::ARGB8888:
>>>> + fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
>>>> + fragmentShaderFile_ = ":RGB.frag";
>>>> + break;
>>>> + case libcamera::formats::BGRA8888:
>>>> + fragmentShaderDefines_.append("#define RGB_PATTERN gba");
>>>
>>> Yet here it's specified.... at the end
>>>
>>>> + fragmentShaderFile_ = ":RGB.frag";
>>>> + break;
>>>> + case libcamera::formats::RGBA8888:
>>>> + fragmentShaderDefines_.append("#define RGB_PATTERN abg");
>>>
>>> but here at the beginning?
>>>
>>> What is the definition of the RGB_PATTERNs?
>>> They don't seem to have a pattern that makes sense to me yet.
>>
>> The shader language accesses vector components by name. The texture2D()
>> function, which samples a texture, returns a vec4, a 4-components
>> floating point vector. Multiple component names are supported:
>>
>> - {x, y, z, w}
>> - {r, g, b, a}
>> - {s, t, p, q}
>>
I wish they also had a set of component names which were simply 'linear'
(e, f, g, h} for instance
(also because those are unused above and 'almost' linear in a qwerty
keyboard ;D)
Or using an index value 1,2,3,4 but I can see how using numeric indexes
here could cause difficulty
>> .x, .r and .s are all equivalent to [0] in C syntax, and similarly for
>> other names. The reason different names are defined and alias each other
>> is that vectors can store different types of data, and standardizing on
>> {r, g, b, a} would lead to weird-looking code when the vector stores a
>> 3D coordinnate for instance ({s, t, p, q} are used as a convention for
>> texture coordinates).
>>
>> The indexing syntax also supports multiple components to be selected:
>>
>> vec4 colour(1.0, 2.0, 3.0, 4.0);
>> vec2 value;
>>
>> value = colour.rb;
>>
>> // value now contains (1.0, 3.0)
>>
>> The RGB fragment shader code simply contains
>>
>> vec3 rgb;
>>
>> rgb = texture2D(tex_y, textureOut).RGB_PATTERN;
>>
>> The pattern thus defines the components mapping.
>>
>> One additional piece of information that is required is how OpenGL
>> expects texture data to be layed out in memory. The texture is created
>> with GL_RGBA and GL_UNSIGNED_BYTE, which means that the R, G, B and A
>> components are layed down in that order, in one byte each. The RGBA8888
>> format, has the opposite convention, it stores RGBA in that order in a
>> 32-bit word that is then stored in memory in little endian format
>
> - Ah... I've missed that little endian part of the puzzle, and have got
> confused too.
>
> Thanks a lot for the detailed explanation!
Agreed. Should we add all that to the commit message?
It's a really useful explanation - it seems valuable to keep it.
> Reviewed-by: Andrey Konovalov <andrey.konovalov at linaro.org>
And this too now ;-)
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
>> , so
>> bytes contains A, B, G and R in that order.
>>
>> So, taking the RGBA8888 example, we have ABGR in memory, which is read
>> in a vec4. We want to extract RGB (in that order) in the shader, so we
>> need components [3], [2] and [1] of the vector, which are expressed as
>> .abg.
>>
>> I hope this makes it clearer.
>>
>>>> + fragmentShaderFile_ = ":RGB.frag";
>>>> + break;
>>>> + case libcamera::formats::BGR888:
>>>> + fragmentShaderDefines_.append("#define RGB_PATTERN rgb");
>>>> + fragmentShaderFile_ = ":RGB.frag";
>>>> + break;
>>>> + case libcamera::formats::RGB888:
>>>> + fragmentShaderDefines_.append("#define RGB_PATTERN bgr");
>>>> + fragmentShaderFile_ = ":RGB.frag";
>>>> + break;
>>>> default:
>>>> ret = false;
>>>> qWarning() << "[ViewFinderGL]:"
>>>> @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()
>>>> 1.0f / (size_.width() / 2 - 1));
>>>> break;
>>>> + case libcamera::formats::ABGR8888:
>>>> + case libcamera::formats::ARGB8888:
>>>> + case libcamera::formats::BGRA8888:
>>>> + case libcamera::formats::RGBA8888:
>>>> + glActiveTexture(GL_TEXTURE0);
>>>> + configureTexture(*textures_[0]);
>>>> + glTexImage2D(GL_TEXTURE_2D,
>>>> + 0,
>>>> + GL_RGBA,
>>>> + size_.width(),
>>>> + size_.height(),
>>>> + 0,
>>>> + GL_RGBA,
>>>> + GL_UNSIGNED_BYTE,
>>>> + data_);
>>>> + shaderProgram_.setUniformValue(textureUniformY_, 0);
>>>> + break;
>>>> +
>>>> + case libcamera::formats::BGR888:
>>>> + case libcamera::formats::RGB888:
>>>> + glActiveTexture(GL_TEXTURE0);
>>>> + configureTexture(*textures_[0]);
>>>> + glTexImage2D(GL_TEXTURE_2D,
>>>> + 0,
>>>> + GL_RGB,
>>>> + size_.width(),
>>>> + size_.height(),
>>>> + 0,
>>>> + GL_RGB,
>>>> + GL_UNSIGNED_BYTE,
>>>> + data_);
>>>> + shaderProgram_.setUniformValue(textureUniformY_, 0);
>>>> + break;
>>>> +
>>>> default:
>>>> break;
>>>> };
>>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list