[libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support for RGB formats
Kieran Bingham
kieran.bingham at ideasonboard.com
Wed Nov 4 11:23:47 CET 2020
Hi Laurent,
On 04/11/2020 10:19, Laurent Pinchart wrote:
> Hi Kieran,
>
> On Wed, Nov 04, 2020 at 09:49:43AM +0000, Kieran Bingham wrote:
>> On 04/11/2020 09:32, Andrey Konovalov wrote:
>>> On 04.11.2020 05:09, Laurent Pinchart wrote:
>>>> 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.
>
> I knew you would say that :-)
>
> I'm not sure we really need to document the OpenGL API and the shader
> language in the commit message though. I've expanded the first paragraph
> to the following:
That's fine, but you could reference it if there is a suitable
link/reference ;-)
I doubt many libcamera reader/developers are also OpenGL developers
(well, I'm not - but maybe some others are ;D)
> "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. The pattern stores the shader vec4 element indices
> (named {r, g, b, a} by convention, for elements 0 to 3) to be extracted
> from the texture samples, when interpreted by OpenGL as RGBA.
>
> Note that, as textures are created with GL_UNSIGNED_BYTE, the RGBA
> order corresponds to bytes in memory, while the libcamera formats are
> named based on the components order in a 32-bit word stored in memory in
> little endian format."
>
> I think this provides enough context, please please let me know if you
> disagree.
Highlighting how the indexing works is enough for me to understand the
context of what is not obvious in the code, so that works for me.
Thanks
Kieran
>
>>> 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