[{"id":13589,"web_url":"https://patchwork.libcamera.org/comment/13589/","msgid":"<5d50967f-e9b2-2bc7-1442-c670b3d67ef7@ideasonboard.com>","date":"2020-11-03T23:00:28","subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 03/11/2020 15:50, Laurent Pinchart wrote:\n> Add support for 24-bit and 32-bit RGB formats. The fragment samples the\n> texture and reorders the components, using a pattern set through the\n> RGB_PATTERN macro.\n> \n> An alternative to manual reordering in the shader would be to set the\n> texture swizzling mask. This is however not available in OpenGL ES\n> before version 3.0, which we don't mandate at the moment.\n> \n> Only the BGR888 and RGB888 formats have been tested, with the vimc\n> pipeline handler.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> ---\n>  src/qcam/assets/shader/RGB.frag    | 22 +++++++++\n>  src/qcam/assets/shader/shaders.qrc |  1 +\n>  src/qcam/viewfinder_gl.cpp         | 71 ++++++++++++++++++++++++++++--\n>  3 files changed, 91 insertions(+), 3 deletions(-)\n>  create mode 100644 src/qcam/assets/shader/RGB.frag\n> \n> diff --git a/src/qcam/assets/shader/RGB.frag b/src/qcam/assets/shader/RGB.frag\n> new file mode 100644\n> index 000000000000..4c374ac98095\n> --- /dev/null\n> +++ b/src/qcam/assets/shader/RGB.frag\n> @@ -0,0 +1,22 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Laurent Pinchart\n> + *\n> + * RGB.frag - Fragment shader code for RGB formats\n> + */\n> +\n> +#ifdef GL_ES\n> +precision mediump float;\n> +#endif\n> +\n> +varying vec2 textureOut;\n> +uniform sampler2D tex_y;\n> +\n> +void main(void)\n> +{\n> +\tvec3 rgb;\n> +\n> +\trgb = texture2D(tex_y, textureOut).RGB_PATTERN;\n> +\n> +\tgl_FragColor = vec4(rgb, 1.0);\n> +}\n> diff --git a/src/qcam/assets/shader/shaders.qrc b/src/qcam/assets/shader/shaders.qrc\n> index 863109146281..8a8f9de1a5fa 100644\n> --- a/src/qcam/assets/shader/shaders.qrc\n> +++ b/src/qcam/assets/shader/shaders.qrc\n> @@ -1,6 +1,7 @@\n>  <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->\n>  <!DOCTYPE RCC><RCC version=\"1.0\">\n>  <qresource>\n> +\t<file>RGB.frag</file>\n>  \t<file>YUV_2_planes.frag</file>\n>  \t<file>YUV_3_planes.frag</file>\n>  \t<file>YUV_packed.frag</file>\n> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> index cbc1365500f5..5d9b442e7985 100644\n> --- a/src/qcam/viewfinder_gl.cpp\n> +++ b/src/qcam/viewfinder_gl.cpp\n> @@ -14,21 +14,28 @@\n>  #include <libcamera/formats.h>\n>  \n>  static const QList<libcamera::PixelFormat> supportedFormats{\n> -\t/* Packed (single plane) */\n> +\t/* YUV - packed (single plane) */\n>  \tlibcamera::formats::UYVY,\n>  \tlibcamera::formats::VYUY,\n>  \tlibcamera::formats::YUYV,\n>  \tlibcamera::formats::YVYU,\n> -\t/* Semi planar (two planes) */\n> +\t/* YUV - semi planar (two planes) */\n>  \tlibcamera::formats::NV12,\n>  \tlibcamera::formats::NV21,\n>  \tlibcamera::formats::NV16,\n>  \tlibcamera::formats::NV61,\n>  \tlibcamera::formats::NV24,\n>  \tlibcamera::formats::NV42,\n> -\t/* Fully planar (three planes) */\n> +\t/* YUV - fully planar (three planes) */\n>  \tlibcamera::formats::YUV420,\n>  \tlibcamera::formats::YVU420,\n> +\t/* RGB */\n> +\tlibcamera::formats::ABGR8888,\n> +\tlibcamera::formats::ARGB8888,\n> +\tlibcamera::formats::BGRA8888,\n> +\tlibcamera::formats::RGBA8888,\n> +\tlibcamera::formats::BGR888,\n> +\tlibcamera::formats::RGB888,\n>  };\n>  \n>  ViewFinderGL::ViewFinderGL(QWidget *parent)\n> @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)\n>  \t\tfragmentShaderDefines_.append(\"#define YUV_PATTERN_YVYU\");\n>  \t\tfragmentShaderFile_ = \":YUV_packed.frag\";\n>  \t\tbreak;\n> +\tcase libcamera::formats::ABGR8888:\n> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n\npattern rgb, the same as BGR888, so I presume the A is stripped by not\nbeing used as a channel or some other magic elsewhere?\n\n> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::ARGB8888:\n> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::BGRA8888:\n> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN gba\");\n\nYet here it's specified.... at the end\n\n> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::RGBA8888:\n> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN abg\");\n\nbut here at the beginning?\n\nWhat is the definition of the RGB_PATTERNs?\nThey don't seem to have a pattern that makes sense to me yet.\n\n> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::BGR888:\n> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::RGB888:\n> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> +\t\tbreak;\n>  \tdefault:\n>  \t\tret = false;\n>  \t\tqWarning() << \"[ViewFinderGL]:\"\n> @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()\n>  \t\t\t\t\t       1.0f / (size_.width() / 2 - 1));\n>  \t\tbreak;\n>  \n> +\tcase libcamera::formats::ABGR8888:\n> +\tcase libcamera::formats::ARGB8888:\n> +\tcase libcamera::formats::BGRA8888:\n> +\tcase libcamera::formats::RGBA8888:\n> +\t\tglActiveTexture(GL_TEXTURE0);\n> +\t\tconfigureTexture(*textures_[0]);\n> +\t\tglTexImage2D(GL_TEXTURE_2D,\n> +\t\t\t     0,\n> +\t\t\t     GL_RGBA,\n> +\t\t\t     size_.width(),\n> +\t\t\t     size_.height(),\n> +\t\t\t     0,\n> +\t\t\t     GL_RGBA,\n> +\t\t\t     GL_UNSIGNED_BYTE,\n> +\t\t\t     data_);\n> +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n> +\t\tbreak;\n> +\n> +\tcase libcamera::formats::BGR888:\n> +\tcase libcamera::formats::RGB888:\n> +\t\tglActiveTexture(GL_TEXTURE0);\n> +\t\tconfigureTexture(*textures_[0]);\n> +\t\tglTexImage2D(GL_TEXTURE_2D,\n> +\t\t\t     0,\n> +\t\t\t     GL_RGB,\n> +\t\t\t     size_.width(),\n> +\t\t\t     size_.height(),\n> +\t\t\t     0,\n> +\t\t\t     GL_RGB,\n> +\t\t\t     GL_UNSIGNED_BYTE,\n> +\t\t\t     data_);\n> +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n> +\t\tbreak;\n> +\n>  \tdefault:\n>  \t\tbreak;\n>  \t};\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 051FBBDB1E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  3 Nov 2020 23:00:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4E5D062BDA;\n\tWed,  4 Nov 2020 00:00:34 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EC13862081\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Nov 2020 00:00:31 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 113AD9E5;\n\tWed,  4 Nov 2020 00:00:30 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TPJv7PVO\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1604444431;\n\tbh=WHgeGCQyy1iOlBX9WvAxu7DA/gJn0y04ei2bsi6U2ug=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=TPJv7PVOPnUKWBFPmkTLTps/X9U4XA9alAexgGRP1VoBsJZ9QbaM+zrxhQ/tSIU3J\n\tGkDXFw8AA6l19vprXnN7jEP6H3Kap7cd6zLPkJclwg9KxTO2H5B/O/QlSJNCya/FX0\n\tgdCfNPWYmgSwp4EGM5B2vC2YhmFG4++xX8Idhlfw=","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20201103155025.5948-1-laurent.pinchart@ideasonboard.com>\n\t<20201103155025.5948-8-laurent.pinchart@ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<5d50967f-e9b2-2bc7-1442-c670b3d67ef7@ideasonboard.com>","Date":"Tue, 3 Nov 2020 23:00:28 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201103155025.5948-8-laurent.pinchart@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13592,"web_url":"https://patchwork.libcamera.org/comment/13592/","msgid":"<20201104020915.GI17496@pendragon.ideasonboard.com>","date":"2020-11-04T02:09:15","subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, Nov 03, 2020 at 11:00:28PM +0000, Kieran Bingham wrote:\n> On 03/11/2020 15:50, Laurent Pinchart wrote:\n> > Add support for 24-bit and 32-bit RGB formats. The fragment samples the\n> > texture and reorders the components, using a pattern set through the\n> > RGB_PATTERN macro.\n> > \n> > An alternative to manual reordering in the shader would be to set the\n> > texture swizzling mask. This is however not available in OpenGL ES\n> > before version 3.0, which we don't mandate at the moment.\n> > \n> > Only the BGR888 and RGB888 formats have been tested, with the vimc\n> > pipeline handler.\n> > \n> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > ---\n> >  src/qcam/assets/shader/RGB.frag    | 22 +++++++++\n> >  src/qcam/assets/shader/shaders.qrc |  1 +\n> >  src/qcam/viewfinder_gl.cpp         | 71 ++++++++++++++++++++++++++++--\n> >  3 files changed, 91 insertions(+), 3 deletions(-)\n> >  create mode 100644 src/qcam/assets/shader/RGB.frag\n> > \n> > diff --git a/src/qcam/assets/shader/RGB.frag b/src/qcam/assets/shader/RGB.frag\n> > new file mode 100644\n> > index 000000000000..4c374ac98095\n> > --- /dev/null\n> > +++ b/src/qcam/assets/shader/RGB.frag\n> > @@ -0,0 +1,22 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Laurent Pinchart\n> > + *\n> > + * RGB.frag - Fragment shader code for RGB formats\n> > + */\n> > +\n> > +#ifdef GL_ES\n> > +precision mediump float;\n> > +#endif\n> > +\n> > +varying vec2 textureOut;\n> > +uniform sampler2D tex_y;\n> > +\n> > +void main(void)\n> > +{\n> > +\tvec3 rgb;\n> > +\n> > +\trgb = texture2D(tex_y, textureOut).RGB_PATTERN;\n> > +\n> > +\tgl_FragColor = vec4(rgb, 1.0);\n> > +}\n> > diff --git a/src/qcam/assets/shader/shaders.qrc b/src/qcam/assets/shader/shaders.qrc\n> > index 863109146281..8a8f9de1a5fa 100644\n> > --- a/src/qcam/assets/shader/shaders.qrc\n> > +++ b/src/qcam/assets/shader/shaders.qrc\n> > @@ -1,6 +1,7 @@\n> >  <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->\n> >  <!DOCTYPE RCC><RCC version=\"1.0\">\n> >  <qresource>\n> > +\t<file>RGB.frag</file>\n> >  \t<file>YUV_2_planes.frag</file>\n> >  \t<file>YUV_3_planes.frag</file>\n> >  \t<file>YUV_packed.frag</file>\n> > diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> > index cbc1365500f5..5d9b442e7985 100644\n> > --- a/src/qcam/viewfinder_gl.cpp\n> > +++ b/src/qcam/viewfinder_gl.cpp\n> > @@ -14,21 +14,28 @@\n> >  #include <libcamera/formats.h>\n> >  \n> >  static const QList<libcamera::PixelFormat> supportedFormats{\n> > -\t/* Packed (single plane) */\n> > +\t/* YUV - packed (single plane) */\n> >  \tlibcamera::formats::UYVY,\n> >  \tlibcamera::formats::VYUY,\n> >  \tlibcamera::formats::YUYV,\n> >  \tlibcamera::formats::YVYU,\n> > -\t/* Semi planar (two planes) */\n> > +\t/* YUV - semi planar (two planes) */\n> >  \tlibcamera::formats::NV12,\n> >  \tlibcamera::formats::NV21,\n> >  \tlibcamera::formats::NV16,\n> >  \tlibcamera::formats::NV61,\n> >  \tlibcamera::formats::NV24,\n> >  \tlibcamera::formats::NV42,\n> > -\t/* Fully planar (three planes) */\n> > +\t/* YUV - fully planar (three planes) */\n> >  \tlibcamera::formats::YUV420,\n> >  \tlibcamera::formats::YVU420,\n> > +\t/* RGB */\n> > +\tlibcamera::formats::ABGR8888,\n> > +\tlibcamera::formats::ARGB8888,\n> > +\tlibcamera::formats::BGRA8888,\n> > +\tlibcamera::formats::RGBA8888,\n> > +\tlibcamera::formats::BGR888,\n> > +\tlibcamera::formats::RGB888,\n> >  };\n> >  \n> >  ViewFinderGL::ViewFinderGL(QWidget *parent)\n> > @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)\n> >  \t\tfragmentShaderDefines_.append(\"#define YUV_PATTERN_YVYU\");\n> >  \t\tfragmentShaderFile_ = \":YUV_packed.frag\";\n> >  \t\tbreak;\n> > +\tcase libcamera::formats::ABGR8888:\n> > +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n> \n> pattern rgb, the same as BGR888, so I presume the A is stripped by not\n> being used as a channel or some other magic elsewhere?\n> \n> > +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::ARGB8888:\n> > +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n> > +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::BGRA8888:\n> > +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN gba\");\n> \n> Yet here it's specified.... at the end\n> \n> > +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::RGBA8888:\n> > +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN abg\");\n> \n> but here at the beginning?\n> \n> What is the definition of the RGB_PATTERNs?\n> They don't seem to have a pattern that makes sense to me yet.\n\nThe shader language accesses vector components by name. The texture2D()\nfunction, which samples a texture, returns a vec4, a 4-components\nfloating point vector. Multiple component names are supported:\n\n- {x, y, z, w}\n- {r, g, b, a} \n- {s, t, p, q}\n\n.x, .r and .s are all equivalent to [0] in C syntax, and similarly for\nother names. The reason different names are defined and alias each other\nis that vectors can store different types of data, and standardizing on\n{r, g, b, a} would lead to weird-looking code when the vector stores a\n3D coordinnate for instance ({s, t, p, q} are used as a convention for\ntexture coordinates).\n\nThe indexing syntax also supports multiple components to be selected:\n\n\tvec4 colour(1.0, 2.0, 3.0, 4.0);\n\tvec2 value;\n\n\tvalue = colour.rb;\n\n\t// value now contains (1.0, 3.0)\n\nThe RGB fragment shader code simply contains\n\n\tvec3 rgb;\n\n\trgb = texture2D(tex_y, textureOut).RGB_PATTERN;\n\nThe pattern thus defines the components mapping.\n\nOne additional piece of information that is required is how OpenGL\nexpects texture data to be layed out in memory. The texture is created\nwith GL_RGBA and GL_UNSIGNED_BYTE, which means that the R, G, B and A\ncomponents are layed down in that order, in one byte each. The RGBA8888\nformat, has the opposite convention, it stores RGBA in that order in a\n32-bit word that is then stored in memory in little endian format, so\nbytes contains A, B, G and R in that order.\n\nSo, taking the RGBA8888 example, we have ABGR in memory, which is read\nin a vec4. We want to extract RGB (in that order) in the shader, so we\nneed components [3], [2] and [1] of the vector, which are expressed as\n.abg.\n\nI hope this makes it clearer.\n\n> > +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::BGR888:\n> > +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n> > +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> > +\t\tbreak;\n> > +\tcase libcamera::formats::RGB888:\n> > +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n> > +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> > +\t\tbreak;\n> >  \tdefault:\n> >  \t\tret = false;\n> >  \t\tqWarning() << \"[ViewFinderGL]:\"\n> > @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()\n> >  \t\t\t\t\t       1.0f / (size_.width() / 2 - 1));\n> >  \t\tbreak;\n> >  \n> > +\tcase libcamera::formats::ABGR8888:\n> > +\tcase libcamera::formats::ARGB8888:\n> > +\tcase libcamera::formats::BGRA8888:\n> > +\tcase libcamera::formats::RGBA8888:\n> > +\t\tglActiveTexture(GL_TEXTURE0);\n> > +\t\tconfigureTexture(*textures_[0]);\n> > +\t\tglTexImage2D(GL_TEXTURE_2D,\n> > +\t\t\t     0,\n> > +\t\t\t     GL_RGBA,\n> > +\t\t\t     size_.width(),\n> > +\t\t\t     size_.height(),\n> > +\t\t\t     0,\n> > +\t\t\t     GL_RGBA,\n> > +\t\t\t     GL_UNSIGNED_BYTE,\n> > +\t\t\t     data_);\n> > +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n> > +\t\tbreak;\n> > +\n> > +\tcase libcamera::formats::BGR888:\n> > +\tcase libcamera::formats::RGB888:\n> > +\t\tglActiveTexture(GL_TEXTURE0);\n> > +\t\tconfigureTexture(*textures_[0]);\n> > +\t\tglTexImage2D(GL_TEXTURE_2D,\n> > +\t\t\t     0,\n> > +\t\t\t     GL_RGB,\n> > +\t\t\t     size_.width(),\n> > +\t\t\t     size_.height(),\n> > +\t\t\t     0,\n> > +\t\t\t     GL_RGB,\n> > +\t\t\t     GL_UNSIGNED_BYTE,\n> > +\t\t\t     data_);\n> > +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n> > +\t\tbreak;\n> > +\n> >  \tdefault:\n> >  \t\tbreak;\n> >  \t};","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 298E0BDB1E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  4 Nov 2020 02:10:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7C98E62C09;\n\tWed,  4 Nov 2020 03:10:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7476660344\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Nov 2020 03:10:03 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DCAD09E5;\n\tWed,  4 Nov 2020 03:10:02 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"YutfpV39\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1604455803;\n\tbh=XcbrSuruTheI3Gk+Dbmx5SbsNLxTzvEq+IXu/COxrX0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YutfpV39VeUd4ZKFjcd1IChUTDbIlOJQrCvIlWDmLYOarAqla8aqUrz6TBA7TWBcy\n\tAFo2+xjFH3l9cp1q9obKmu6xW5oTfFyLw1RCpFtuIe7tkHKj/s/4hO4GH1YCeHIDw6\n\txKE1g/0nDqX05J6JXDw8YKd4dEjfqibiG/nTLGA0=","Date":"Wed, 4 Nov 2020 04:09:15 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20201104020915.GI17496@pendragon.ideasonboard.com>","References":"<20201103155025.5948-1-laurent.pinchart@ideasonboard.com>\n\t<20201103155025.5948-8-laurent.pinchart@ideasonboard.com>\n\t<5d50967f-e9b2-2bc7-1442-c670b3d67ef7@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<5d50967f-e9b2-2bc7-1442-c670b3d67ef7@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13594,"web_url":"https://patchwork.libcamera.org/comment/13594/","msgid":"<f660e99c-d81d-5de0-1965-24fd7ab38bda@linaro.org>","date":"2020-11-04T09:32:16","subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","submitter":{"id":25,"url":"https://patchwork.libcamera.org/api/people/25/","name":"Andrey Konovalov","email":"andrey.konovalov@linaro.org"},"content":"Hi Laurent,\n\nOn 04.11.2020 05:09, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Tue, Nov 03, 2020 at 11:00:28PM +0000, Kieran Bingham wrote:\n>> On 03/11/2020 15:50, Laurent Pinchart wrote:\n>>> Add support for 24-bit and 32-bit RGB formats. The fragment samples the\n>>> texture and reorders the components, using a pattern set through the\n>>> RGB_PATTERN macro.\n>>>\n>>> An alternative to manual reordering in the shader would be to set the\n>>> texture swizzling mask. This is however not available in OpenGL ES\n>>> before version 3.0, which we don't mandate at the moment.\n>>>\n>>> Only the BGR888 and RGB888 formats have been tested, with the vimc\n>>> pipeline handler.\n>>>\n>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>>> ---\n>>>   src/qcam/assets/shader/RGB.frag    | 22 +++++++++\n>>>   src/qcam/assets/shader/shaders.qrc |  1 +\n>>>   src/qcam/viewfinder_gl.cpp         | 71 ++++++++++++++++++++++++++++--\n>>>   3 files changed, 91 insertions(+), 3 deletions(-)\n>>>   create mode 100644 src/qcam/assets/shader/RGB.frag\n>>>\n>>> diff --git a/src/qcam/assets/shader/RGB.frag b/src/qcam/assets/shader/RGB.frag\n>>> new file mode 100644\n>>> index 000000000000..4c374ac98095\n>>> --- /dev/null\n>>> +++ b/src/qcam/assets/shader/RGB.frag\n>>> @@ -0,0 +1,22 @@\n>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>> +/*\n>>> + * Copyright (C) 2020, Laurent Pinchart\n>>> + *\n>>> + * RGB.frag - Fragment shader code for RGB formats\n>>> + */\n>>> +\n>>> +#ifdef GL_ES\n>>> +precision mediump float;\n>>> +#endif\n>>> +\n>>> +varying vec2 textureOut;\n>>> +uniform sampler2D tex_y;\n>>> +\n>>> +void main(void)\n>>> +{\n>>> +\tvec3 rgb;\n>>> +\n>>> +\trgb = texture2D(tex_y, textureOut).RGB_PATTERN;\n>>> +\n>>> +\tgl_FragColor = vec4(rgb, 1.0);\n>>> +}\n>>> diff --git a/src/qcam/assets/shader/shaders.qrc b/src/qcam/assets/shader/shaders.qrc\n>>> index 863109146281..8a8f9de1a5fa 100644\n>>> --- a/src/qcam/assets/shader/shaders.qrc\n>>> +++ b/src/qcam/assets/shader/shaders.qrc\n>>> @@ -1,6 +1,7 @@\n>>>   <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->\n>>>   <!DOCTYPE RCC><RCC version=\"1.0\">\n>>>   <qresource>\n>>> +\t<file>RGB.frag</file>\n>>>   \t<file>YUV_2_planes.frag</file>\n>>>   \t<file>YUV_3_planes.frag</file>\n>>>   \t<file>YUV_packed.frag</file>\n>>> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n>>> index cbc1365500f5..5d9b442e7985 100644\n>>> --- a/src/qcam/viewfinder_gl.cpp\n>>> +++ b/src/qcam/viewfinder_gl.cpp\n>>> @@ -14,21 +14,28 @@\n>>>   #include <libcamera/formats.h>\n>>>   \n>>>   static const QList<libcamera::PixelFormat> supportedFormats{\n>>> -\t/* Packed (single plane) */\n>>> +\t/* YUV - packed (single plane) */\n>>>   \tlibcamera::formats::UYVY,\n>>>   \tlibcamera::formats::VYUY,\n>>>   \tlibcamera::formats::YUYV,\n>>>   \tlibcamera::formats::YVYU,\n>>> -\t/* Semi planar (two planes) */\n>>> +\t/* YUV - semi planar (two planes) */\n>>>   \tlibcamera::formats::NV12,\n>>>   \tlibcamera::formats::NV21,\n>>>   \tlibcamera::formats::NV16,\n>>>   \tlibcamera::formats::NV61,\n>>>   \tlibcamera::formats::NV24,\n>>>   \tlibcamera::formats::NV42,\n>>> -\t/* Fully planar (three planes) */\n>>> +\t/* YUV - fully planar (three planes) */\n>>>   \tlibcamera::formats::YUV420,\n>>>   \tlibcamera::formats::YVU420,\n>>> +\t/* RGB */\n>>> +\tlibcamera::formats::ABGR8888,\n>>> +\tlibcamera::formats::ARGB8888,\n>>> +\tlibcamera::formats::BGRA8888,\n>>> +\tlibcamera::formats::RGBA8888,\n>>> +\tlibcamera::formats::BGR888,\n>>> +\tlibcamera::formats::RGB888,\n>>>   };\n>>>   \n>>>   ViewFinderGL::ViewFinderGL(QWidget *parent)\n>>> @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)\n>>>   \t\tfragmentShaderDefines_.append(\"#define YUV_PATTERN_YVYU\");\n>>>   \t\tfragmentShaderFile_ = \":YUV_packed.frag\";\n>>>   \t\tbreak;\n>>> +\tcase libcamera::formats::ABGR8888:\n>>> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n>>\n>> pattern rgb, the same as BGR888, so I presume the A is stripped by not\n>> being used as a channel or some other magic elsewhere?\n>>\n>>> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n>>> +\t\tbreak;\n>>> +\tcase libcamera::formats::ARGB8888:\n>>> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n>>> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n>>> +\t\tbreak;\n>>> +\tcase libcamera::formats::BGRA8888:\n>>> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN gba\");\n>>\n>> Yet here it's specified.... at the end\n>>\n>>> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n>>> +\t\tbreak;\n>>> +\tcase libcamera::formats::RGBA8888:\n>>> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN abg\");\n>>\n>> but here at the beginning?\n>>\n>> What is the definition of the RGB_PATTERNs?\n>> They don't seem to have a pattern that makes sense to me yet.\n> \n> The shader language accesses vector components by name. The texture2D()\n> function, which samples a texture, returns a vec4, a 4-components\n> floating point vector. Multiple component names are supported:\n> \n> - {x, y, z, w}\n> - {r, g, b, a}\n> - {s, t, p, q}\n> \n> .x, .r and .s are all equivalent to [0] in C syntax, and similarly for\n> other names. The reason different names are defined and alias each other\n> is that vectors can store different types of data, and standardizing on\n> {r, g, b, a} would lead to weird-looking code when the vector stores a\n> 3D coordinnate for instance ({s, t, p, q} are used as a convention for\n> texture coordinates).\n> \n> The indexing syntax also supports multiple components to be selected:\n> \n> \tvec4 colour(1.0, 2.0, 3.0, 4.0);\n> \tvec2 value;\n> \n> \tvalue = colour.rb;\n> \n> \t// value now contains (1.0, 3.0)\n> \n> The RGB fragment shader code simply contains\n> \n> \tvec3 rgb;\n> \n> \trgb = texture2D(tex_y, textureOut).RGB_PATTERN;\n> \n> The pattern thus defines the components mapping.\n> \n> One additional piece of information that is required is how OpenGL\n> expects texture data to be layed out in memory. The texture is created\n> with GL_RGBA and GL_UNSIGNED_BYTE, which means that the R, G, B and A\n> components are layed down in that order, in one byte each. The RGBA8888\n> format, has the opposite convention, it stores RGBA in that order in a\n> 32-bit word that is then stored in memory in little endian format\n\n- Ah... I've missed that little endian part of the puzzle, and have got confused too.\n\nThanks a lot for the detailed explanation!\n\nReviewed-by: Andrey Konovalov <andrey.konovalov@linaro.org>\n\n>, so\n> bytes contains A, B, G and R in that order.\n> \n> So, taking the RGBA8888 example, we have ABGR in memory, which is read\n> in a vec4. We want to extract RGB (in that order) in the shader, so we\n> need components [3], [2] and [1] of the vector, which are expressed as\n> .abg.\n> \n> I hope this makes it clearer.\n> \n>>> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n>>> +\t\tbreak;\n>>> +\tcase libcamera::formats::BGR888:\n>>> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n>>> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n>>> +\t\tbreak;\n>>> +\tcase libcamera::formats::RGB888:\n>>> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n>>> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n>>> +\t\tbreak;\n>>>   \tdefault:\n>>>   \t\tret = false;\n>>>   \t\tqWarning() << \"[ViewFinderGL]:\"\n>>> @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()\n>>>   \t\t\t\t\t       1.0f / (size_.width() / 2 - 1));\n>>>   \t\tbreak;\n>>>   \n>>> +\tcase libcamera::formats::ABGR8888:\n>>> +\tcase libcamera::formats::ARGB8888:\n>>> +\tcase libcamera::formats::BGRA8888:\n>>> +\tcase libcamera::formats::RGBA8888:\n>>> +\t\tglActiveTexture(GL_TEXTURE0);\n>>> +\t\tconfigureTexture(*textures_[0]);\n>>> +\t\tglTexImage2D(GL_TEXTURE_2D,\n>>> +\t\t\t     0,\n>>> +\t\t\t     GL_RGBA,\n>>> +\t\t\t     size_.width(),\n>>> +\t\t\t     size_.height(),\n>>> +\t\t\t     0,\n>>> +\t\t\t     GL_RGBA,\n>>> +\t\t\t     GL_UNSIGNED_BYTE,\n>>> +\t\t\t     data_);\n>>> +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n>>> +\t\tbreak;\n>>> +\n>>> +\tcase libcamera::formats::BGR888:\n>>> +\tcase libcamera::formats::RGB888:\n>>> +\t\tglActiveTexture(GL_TEXTURE0);\n>>> +\t\tconfigureTexture(*textures_[0]);\n>>> +\t\tglTexImage2D(GL_TEXTURE_2D,\n>>> +\t\t\t     0,\n>>> +\t\t\t     GL_RGB,\n>>> +\t\t\t     size_.width(),\n>>> +\t\t\t     size_.height(),\n>>> +\t\t\t     0,\n>>> +\t\t\t     GL_RGB,\n>>> +\t\t\t     GL_UNSIGNED_BYTE,\n>>> +\t\t\t     data_);\n>>> +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n>>> +\t\tbreak;\n>>> +\n>>>   \tdefault:\n>>>   \t\tbreak;\n>>>   \t};\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7D0FBBDB89\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  4 Nov 2020 09:32:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0C53B60346;\n\tWed,  4 Nov 2020 10:32:21 +0100 (CET)","from mail-lf1-x142.google.com (mail-lf1-x142.google.com\n\t[IPv6:2a00:1450:4864:20::142])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 95C0060344\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Nov 2020 10:32:19 +0100 (CET)","by mail-lf1-x142.google.com with SMTP id s30so3956602lfc.4\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 04 Nov 2020 01:32:19 -0800 (PST)","from [192.168.118.216] ([85.249.41.73])\n\tby smtp.gmail.com with ESMTPSA id\n\tn3sm459609ljc.132.2020.11.04.01.32.17\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tWed, 04 Nov 2020 01:32:17 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"gXGCS3ax\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google;\n\th=subject:to:cc:references:from:message-id:date:user-agent\n\t:mime-version:in-reply-to:content-language:content-transfer-encoding; \n\tbh=udP6fWkshc2iqNGCxb07oCXov3XEXfw7KG7qBv0QzdU=;\n\tb=gXGCS3ax+Mh0MY4jS0Pod5xCFXMa7INuGg/etcovhqgwGgvMHqwWkxtDUZt/Zu79pl\n\ttLAX7sh9Y2PDB1FPNcWJ+uVl1XWGzgjigU49iCMmsyGCKfhrE+EcwiwsT0ugA3gtACiS\n\tP5q2bF7g92ogGg9ChU3dNa4bvypUYojsNIRD8XyAVBktjk7mlaq2vzJtUJ4B+MAL2edC\n\tSJz4yRQusKfoePdZNc2lJXJapLqKfyj/eB174JKWvchCG12M/s3bnmcopq8icGeUtQVg\n\t9m1t6k6oxzgqD3I0KQAXnZj1cIIUVqotxp6sV9W15o8plRQCuKDDNcRSsTzG1fmOEyJQ\n\tubcg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:subject:to:cc:references:from:message-id:date\n\t:user-agent:mime-version:in-reply-to:content-language\n\t:content-transfer-encoding;\n\tbh=udP6fWkshc2iqNGCxb07oCXov3XEXfw7KG7qBv0QzdU=;\n\tb=gIdfwfoxVHXXUEYhkQondnPf/chDVmCWnS7bmXFeXCCLNbVVx/htmQL7SZ2qsJ1spl\n\tpmmXwuFtXLKS8hUz5BS39O1t9CCJpd4yrUuicDxKvuXYXeEGIqua2bILn/T9Ipfv7RyA\n\tOUqBQ1+e7Jv0iW2CPglLH09gEeTsrONcL/2Euvwgart0C+GbS6i4nlzBfm4qWiziUW8g\n\tXR1NK4edBR/Wi4/dwCC8g/1K2vMvs0YcQkS8SauIEVGTc0XKF8GqhXSg1taGXdsqACYV\n\tYH3UH+RN+r3thj1jSr69hrCJyzdevrk48urZe+x69ZokjyPrfA0BmoUMzUhpvqoAdwHO\n\t7rPA==","X-Gm-Message-State":"AOAM531hmnm8l2P2UR8ZT2kRdi/MSwCJ4qHPHeHKVe5aCCIi1aRmS6vl\n\tcGAdr//2cB29w/3T5+xO/8+0yw==","X-Google-Smtp-Source":"ABdhPJxzg139yDNWH/TV1IbwOZy6IlXBQi3TWdIlXbVt3pxES/K/k+KRBK4/LEFPRYxOgBZVmiUjTw==","X-Received":"by 2002:a19:e04e:: with SMTP id\n\tg14mr8681883lfj.590.1604482338681; \n\tWed, 04 Nov 2020 01:32:18 -0800 (PST)","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","References":"<20201103155025.5948-1-laurent.pinchart@ideasonboard.com>\n\t<20201103155025.5948-8-laurent.pinchart@ideasonboard.com>\n\t<5d50967f-e9b2-2bc7-1442-c670b3d67ef7@ideasonboard.com>\n\t<20201104020915.GI17496@pendragon.ideasonboard.com>","From":"Andrey Konovalov <andrey.konovalov@linaro.org>","Message-ID":"<f660e99c-d81d-5de0-1965-24fd7ab38bda@linaro.org>","Date":"Wed, 4 Nov 2020 12:32:16 +0300","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201104020915.GI17496@pendragon.ideasonboard.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13597,"web_url":"https://patchwork.libcamera.org/comment/13597/","msgid":"<fb810c76-b174-3a50-5ee7-5f08499ea208@ideasonboard.com>","date":"2020-11-04T09:49:43","subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 04/11/2020 09:32, Andrey Konovalov wrote:\n> Hi Laurent,\n> \n> On 04.11.2020 05:09, Laurent Pinchart wrote:\n>> Hi Kieran,\n>>\n>> On Tue, Nov 03, 2020 at 11:00:28PM +0000, Kieran Bingham wrote:\n>>> On 03/11/2020 15:50, Laurent Pinchart wrote:\n>>>> Add support for 24-bit and 32-bit RGB formats. The fragment samples the\n>>>> texture and reorders the components, using a pattern set through the\n>>>> RGB_PATTERN macro.\n>>>>\n>>>> An alternative to manual reordering in the shader would be to set the\n>>>> texture swizzling mask. This is however not available in OpenGL ES\n>>>> before version 3.0, which we don't mandate at the moment.\n>>>>\n>>>> Only the BGR888 and RGB888 formats have been tested, with the vimc\n>>>> pipeline handler.\n>>>>\n>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>>>> ---\n>>>>   src/qcam/assets/shader/RGB.frag    | 22 +++++++++\n>>>>   src/qcam/assets/shader/shaders.qrc |  1 +\n>>>>   src/qcam/viewfinder_gl.cpp         | 71\n>>>> ++++++++++++++++++++++++++++--\n>>>>   3 files changed, 91 insertions(+), 3 deletions(-)\n>>>>   create mode 100644 src/qcam/assets/shader/RGB.frag\n>>>>\n>>>> diff --git a/src/qcam/assets/shader/RGB.frag\n>>>> b/src/qcam/assets/shader/RGB.frag\n>>>> new file mode 100644\n>>>> index 000000000000..4c374ac98095\n>>>> --- /dev/null\n>>>> +++ b/src/qcam/assets/shader/RGB.frag\n>>>> @@ -0,0 +1,22 @@\n>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>>> +/*\n>>>> + * Copyright (C) 2020, Laurent Pinchart\n>>>> + *\n>>>> + * RGB.frag - Fragment shader code for RGB formats\n>>>> + */\n>>>> +\n>>>> +#ifdef GL_ES\n>>>> +precision mediump float;\n>>>> +#endif\n>>>> +\n>>>> +varying vec2 textureOut;\n>>>> +uniform sampler2D tex_y;\n>>>> +\n>>>> +void main(void)\n>>>> +{\n>>>> +    vec3 rgb;\n>>>> +\n>>>> +    rgb = texture2D(tex_y, textureOut).RGB_PATTERN;\n>>>> +\n>>>> +    gl_FragColor = vec4(rgb, 1.0);\n>>>> +}\n>>>> diff --git a/src/qcam/assets/shader/shaders.qrc\n>>>> b/src/qcam/assets/shader/shaders.qrc\n>>>> index 863109146281..8a8f9de1a5fa 100644\n>>>> --- a/src/qcam/assets/shader/shaders.qrc\n>>>> +++ b/src/qcam/assets/shader/shaders.qrc\n>>>> @@ -1,6 +1,7 @@\n>>>>   <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->\n>>>>   <!DOCTYPE RCC><RCC version=\"1.0\">\n>>>>   <qresource>\n>>>> +    <file>RGB.frag</file>\n>>>>       <file>YUV_2_planes.frag</file>\n>>>>       <file>YUV_3_planes.frag</file>\n>>>>       <file>YUV_packed.frag</file>\n>>>> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n>>>> index cbc1365500f5..5d9b442e7985 100644\n>>>> --- a/src/qcam/viewfinder_gl.cpp\n>>>> +++ b/src/qcam/viewfinder_gl.cpp\n>>>> @@ -14,21 +14,28 @@\n>>>>   #include <libcamera/formats.h>\n>>>>     static const QList<libcamera::PixelFormat> supportedFormats{\n>>>> -    /* Packed (single plane) */\n>>>> +    /* YUV - packed (single plane) */\n>>>>       libcamera::formats::UYVY,\n>>>>       libcamera::formats::VYUY,\n>>>>       libcamera::formats::YUYV,\n>>>>       libcamera::formats::YVYU,\n>>>> -    /* Semi planar (two planes) */\n>>>> +    /* YUV - semi planar (two planes) */\n>>>>       libcamera::formats::NV12,\n>>>>       libcamera::formats::NV21,\n>>>>       libcamera::formats::NV16,\n>>>>       libcamera::formats::NV61,\n>>>>       libcamera::formats::NV24,\n>>>>       libcamera::formats::NV42,\n>>>> -    /* Fully planar (three planes) */\n>>>> +    /* YUV - fully planar (three planes) */\n>>>>       libcamera::formats::YUV420,\n>>>>       libcamera::formats::YVU420,\n>>>> +    /* RGB */\n>>>> +    libcamera::formats::ABGR8888,\n>>>> +    libcamera::formats::ARGB8888,\n>>>> +    libcamera::formats::BGRA8888,\n>>>> +    libcamera::formats::RGBA8888,\n>>>> +    libcamera::formats::BGR888,\n>>>> +    libcamera::formats::RGB888,\n>>>>   };\n>>>>     ViewFinderGL::ViewFinderGL(QWidget *parent)\n>>>> @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const\n>>>> libcamera::PixelFormat &format)\n>>>>           fragmentShaderDefines_.append(\"#define YUV_PATTERN_YVYU\");\n>>>>           fragmentShaderFile_ = \":YUV_packed.frag\";\n>>>>           break;\n>>>> +    case libcamera::formats::ABGR8888:\n>>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n>>>\n>>> pattern rgb, the same as BGR888, so I presume the A is stripped by not\n>>> being used as a channel or some other magic elsewhere?\n>>>\n>>>> +        fragmentShaderFile_ = \":RGB.frag\";\n>>>> +        break;\n>>>> +    case libcamera::formats::ARGB8888:\n>>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n>>>> +        fragmentShaderFile_ = \":RGB.frag\";\n>>>> +        break;\n>>>> +    case libcamera::formats::BGRA8888:\n>>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN gba\");\n>>>\n>>> Yet here it's specified.... at the end\n>>>\n>>>> +        fragmentShaderFile_ = \":RGB.frag\";\n>>>> +        break;\n>>>> +    case libcamera::formats::RGBA8888:\n>>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN abg\");\n>>>\n>>> but here at the beginning?\n>>>\n>>> What is the definition of the RGB_PATTERNs?\n>>> They don't seem to have a pattern that makes sense to me yet.\n>>\n>> The shader language accesses vector components by name. The texture2D()\n>> function, which samples a texture, returns a vec4, a 4-components\n>> floating point vector. Multiple component names are supported:\n>>\n>> - {x, y, z, w}\n>> - {r, g, b, a}\n>> - {s, t, p, q}\n>>\n\nI wish they also had a set of component names which were simply 'linear'\n(e, f, g, h} for instance\n\n(also because those are unused above and 'almost' linear in a qwerty\nkeyboard ;D)\n\nOr using an index value 1,2,3,4 but I can see how using numeric indexes\nhere could cause difficulty\n\n>> .x, .r and .s are all equivalent to [0] in C syntax, and similarly for\n>> other names. The reason different names are defined and alias each other\n>> is that vectors can store different types of data, and standardizing on\n>> {r, g, b, a} would lead to weird-looking code when the vector stores a\n>> 3D coordinnate for instance ({s, t, p, q} are used as a convention for\n>> texture coordinates).\n>>\n>> The indexing syntax also supports multiple components to be selected:\n>>\n>>     vec4 colour(1.0, 2.0, 3.0, 4.0);\n>>     vec2 value;\n>>\n>>     value = colour.rb;\n>>\n>>     // value now contains (1.0, 3.0)\n>>\n>> The RGB fragment shader code simply contains\n>>\n>>     vec3 rgb;\n>>\n>>     rgb = texture2D(tex_y, textureOut).RGB_PATTERN;\n>>\n>> The pattern thus defines the components mapping.\n>>\n>> One additional piece of information that is required is how OpenGL\n>> expects texture data to be layed out in memory. The texture is created\n>> with GL_RGBA and GL_UNSIGNED_BYTE, which means that the R, G, B and A\n>> components are layed down in that order, in one byte each. The RGBA8888\n>> format, has the opposite convention, it stores RGBA in that order in a\n>> 32-bit word that is then stored in memory in little endian format\n> \n> - Ah... I've missed that little endian part of the puzzle, and have got\n> confused too.\n> \n> Thanks a lot for the detailed explanation!\n\nAgreed. Should we add all that to the commit message?\nIt's a really useful explanation - it seems valuable to keep it.\n\n\n> Reviewed-by: Andrey Konovalov <andrey.konovalov@linaro.org>\n\nAnd this too now ;-)\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n> \n>> , so\n>> bytes contains A, B, G and R in that order.\n>>\n>> So, taking the RGBA8888 example, we have ABGR in memory, which is read\n>> in a vec4. We want to extract RGB (in that order) in the shader, so we\n>> need components [3], [2] and [1] of the vector, which are expressed as\n>> .abg.\n>>\n>> I hope this makes it clearer.\n>>\n>>>> +        fragmentShaderFile_ = \":RGB.frag\";\n>>>> +        break;\n>>>> +    case libcamera::formats::BGR888:\n>>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n>>>> +        fragmentShaderFile_ = \":RGB.frag\";\n>>>> +        break;\n>>>> +    case libcamera::formats::RGB888:\n>>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n>>>> +        fragmentShaderFile_ = \":RGB.frag\";\n>>>> +        break;\n>>>>       default:\n>>>>           ret = false;\n>>>>           qWarning() << \"[ViewFinderGL]:\"\n>>>> @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()\n>>>>                              1.0f / (size_.width() / 2 - 1));\n>>>>           break;\n>>>>   +    case libcamera::formats::ABGR8888:\n>>>> +    case libcamera::formats::ARGB8888:\n>>>> +    case libcamera::formats::BGRA8888:\n>>>> +    case libcamera::formats::RGBA8888:\n>>>> +        glActiveTexture(GL_TEXTURE0);\n>>>> +        configureTexture(*textures_[0]);\n>>>> +        glTexImage2D(GL_TEXTURE_2D,\n>>>> +                 0,\n>>>> +                 GL_RGBA,\n>>>> +                 size_.width(),\n>>>> +                 size_.height(),\n>>>> +                 0,\n>>>> +                 GL_RGBA,\n>>>> +                 GL_UNSIGNED_BYTE,\n>>>> +                 data_);\n>>>> +        shaderProgram_.setUniformValue(textureUniformY_, 0);\n>>>> +        break;\n>>>> +\n>>>> +    case libcamera::formats::BGR888:\n>>>> +    case libcamera::formats::RGB888:\n>>>> +        glActiveTexture(GL_TEXTURE0);\n>>>> +        configureTexture(*textures_[0]);\n>>>> +        glTexImage2D(GL_TEXTURE_2D,\n>>>> +                 0,\n>>>> +                 GL_RGB,\n>>>> +                 size_.width(),\n>>>> +                 size_.height(),\n>>>> +                 0,\n>>>> +                 GL_RGB,\n>>>> +                 GL_UNSIGNED_BYTE,\n>>>> +                 data_);\n>>>> +        shaderProgram_.setUniformValue(textureUniformY_, 0);\n>>>> +        break;\n>>>> +\n>>>>       default:\n>>>>           break;\n>>>>       };\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 0CC32BDB89\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  4 Nov 2020 09:49:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 97CD660353;\n\tWed,  4 Nov 2020 10:49:48 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EEA9E60346\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Nov 2020 10:49:46 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 454CB51D;\n\tWed,  4 Nov 2020 10:49:46 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"YiNruGRi\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1604483386;\n\tbh=rLkANDcQFNzr5eSXtC+2D3wRu/KbzSAmtnYtfMPlk6o=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=YiNruGRigJvo/Bl0ifnbM7FVCmVNryM800Fd1jlQ4SgQIbrWHCPZb/J3Kip4JM1wO\n\t+r9YEVFQ53LWiFrgWA0jTnWOgUQgl2kxhOsJZibEhh//vNWVLLiRzDQ43bCWoC8/tR\n\tpzFtlSwRxbRrtLAipJ5vbOCyFPtKWgbv9XbXd94A=","To":"Andrey Konovalov <andrey.konovalov@linaro.org>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>","References":"<20201103155025.5948-1-laurent.pinchart@ideasonboard.com>\n\t<20201103155025.5948-8-laurent.pinchart@ideasonboard.com>\n\t<5d50967f-e9b2-2bc7-1442-c670b3d67ef7@ideasonboard.com>\n\t<20201104020915.GI17496@pendragon.ideasonboard.com>\n\t<f660e99c-d81d-5de0-1965-24fd7ab38bda@linaro.org>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<fb810c76-b174-3a50-5ee7-5f08499ea208@ideasonboard.com>","Date":"Wed, 4 Nov 2020 09:49:43 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<f660e99c-d81d-5de0-1965-24fd7ab38bda@linaro.org>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13598,"web_url":"https://patchwork.libcamera.org/comment/13598/","msgid":"<20201104101915.GF26171@pendragon.ideasonboard.com>","date":"2020-11-04T10:19:15","subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Nov 04, 2020 at 09:49:43AM +0000, Kieran Bingham wrote:\n> On 04/11/2020 09:32, Andrey Konovalov wrote:\n> > On 04.11.2020 05:09, Laurent Pinchart wrote:\n> >> On Tue, Nov 03, 2020 at 11:00:28PM +0000, Kieran Bingham wrote:\n> >>> On 03/11/2020 15:50, Laurent Pinchart wrote:\n> >>>> Add support for 24-bit and 32-bit RGB formats. The fragment samples the\n> >>>> texture and reorders the components, using a pattern set through the\n> >>>> RGB_PATTERN macro.\n> >>>>\n> >>>> An alternative to manual reordering in the shader would be to set the\n> >>>> texture swizzling mask. This is however not available in OpenGL ES\n> >>>> before version 3.0, which we don't mandate at the moment.\n> >>>>\n> >>>> Only the BGR888 and RGB888 formats have been tested, with the vimc\n> >>>> pipeline handler.\n> >>>>\n> >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> >>>> ---\n> >>>>   src/qcam/assets/shader/RGB.frag    | 22 +++++++++\n> >>>>   src/qcam/assets/shader/shaders.qrc |  1 +\n> >>>>   src/qcam/viewfinder_gl.cpp         | 71 ++++++++++++++++++++++++++++--\n> >>>>   3 files changed, 91 insertions(+), 3 deletions(-)\n> >>>>   create mode 100644 src/qcam/assets/shader/RGB.frag\n> >>>>\n> >>>> diff --git a/src/qcam/assets/shader/RGB.frag\n> >>>> b/src/qcam/assets/shader/RGB.frag\n> >>>> new file mode 100644\n> >>>> index 000000000000..4c374ac98095\n> >>>> --- /dev/null\n> >>>> +++ b/src/qcam/assets/shader/RGB.frag\n> >>>> @@ -0,0 +1,22 @@\n> >>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >>>> +/*\n> >>>> + * Copyright (C) 2020, Laurent Pinchart\n> >>>> + *\n> >>>> + * RGB.frag - Fragment shader code for RGB formats\n> >>>> + */\n> >>>> +\n> >>>> +#ifdef GL_ES\n> >>>> +precision mediump float;\n> >>>> +#endif\n> >>>> +\n> >>>> +varying vec2 textureOut;\n> >>>> +uniform sampler2D tex_y;\n> >>>> +\n> >>>> +void main(void)\n> >>>> +{\n> >>>> +    vec3 rgb;\n> >>>> +\n> >>>> +    rgb = texture2D(tex_y, textureOut).RGB_PATTERN;\n> >>>> +\n> >>>> +    gl_FragColor = vec4(rgb, 1.0);\n> >>>> +}\n> >>>> diff --git a/src/qcam/assets/shader/shaders.qrc\n> >>>> b/src/qcam/assets/shader/shaders.qrc\n> >>>> index 863109146281..8a8f9de1a5fa 100644\n> >>>> --- a/src/qcam/assets/shader/shaders.qrc\n> >>>> +++ b/src/qcam/assets/shader/shaders.qrc\n> >>>> @@ -1,6 +1,7 @@\n> >>>>   <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->\n> >>>>   <!DOCTYPE RCC><RCC version=\"1.0\">\n> >>>>   <qresource>\n> >>>> +    <file>RGB.frag</file>\n> >>>>       <file>YUV_2_planes.frag</file>\n> >>>>       <file>YUV_3_planes.frag</file>\n> >>>>       <file>YUV_packed.frag</file>\n> >>>> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> >>>> index cbc1365500f5..5d9b442e7985 100644\n> >>>> --- a/src/qcam/viewfinder_gl.cpp\n> >>>> +++ b/src/qcam/viewfinder_gl.cpp\n> >>>> @@ -14,21 +14,28 @@\n> >>>>   #include <libcamera/formats.h>\n> >>>>     static const QList<libcamera::PixelFormat> supportedFormats{\n> >>>> -    /* Packed (single plane) */\n> >>>> +    /* YUV - packed (single plane) */\n> >>>>       libcamera::formats::UYVY,\n> >>>>       libcamera::formats::VYUY,\n> >>>>       libcamera::formats::YUYV,\n> >>>>       libcamera::formats::YVYU,\n> >>>> -    /* Semi planar (two planes) */\n> >>>> +    /* YUV - semi planar (two planes) */\n> >>>>       libcamera::formats::NV12,\n> >>>>       libcamera::formats::NV21,\n> >>>>       libcamera::formats::NV16,\n> >>>>       libcamera::formats::NV61,\n> >>>>       libcamera::formats::NV24,\n> >>>>       libcamera::formats::NV42,\n> >>>> -    /* Fully planar (three planes) */\n> >>>> +    /* YUV - fully planar (three planes) */\n> >>>>       libcamera::formats::YUV420,\n> >>>>       libcamera::formats::YVU420,\n> >>>> +    /* RGB */\n> >>>> +    libcamera::formats::ABGR8888,\n> >>>> +    libcamera::formats::ARGB8888,\n> >>>> +    libcamera::formats::BGRA8888,\n> >>>> +    libcamera::formats::RGBA8888,\n> >>>> +    libcamera::formats::BGR888,\n> >>>> +    libcamera::formats::RGB888,\n> >>>>   };\n> >>>>     ViewFinderGL::ViewFinderGL(QWidget *parent)\n> >>>> @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const\n> >>>> libcamera::PixelFormat &format)\n> >>>>           fragmentShaderDefines_.append(\"#define YUV_PATTERN_YVYU\");\n> >>>>           fragmentShaderFile_ = \":YUV_packed.frag\";\n> >>>>           break;\n> >>>> +    case libcamera::formats::ABGR8888:\n> >>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n> >>>\n> >>> pattern rgb, the same as BGR888, so I presume the A is stripped by not\n> >>> being used as a channel or some other magic elsewhere?\n> >>>\n> >>>> +        fragmentShaderFile_ = \":RGB.frag\";\n> >>>> +        break;\n> >>>> +    case libcamera::formats::ARGB8888:\n> >>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n> >>>> +        fragmentShaderFile_ = \":RGB.frag\";\n> >>>> +        break;\n> >>>> +    case libcamera::formats::BGRA8888:\n> >>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN gba\");\n> >>>\n> >>> Yet here it's specified.... at the end\n> >>>\n> >>>> +        fragmentShaderFile_ = \":RGB.frag\";\n> >>>> +        break;\n> >>>> +    case libcamera::formats::RGBA8888:\n> >>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN abg\");\n> >>>\n> >>> but here at the beginning?\n> >>>\n> >>> What is the definition of the RGB_PATTERNs?\n> >>> They don't seem to have a pattern that makes sense to me yet.\n> >>\n> >> The shader language accesses vector components by name. The texture2D()\n> >> function, which samples a texture, returns a vec4, a 4-components\n> >> floating point vector. Multiple component names are supported:\n> >>\n> >> - {x, y, z, w}\n> >> - {r, g, b, a}\n> >> - {s, t, p, q}\n> >>\n> \n> I wish they also had a set of component names which were simply 'linear'\n> (e, f, g, h} for instance\n> \n> (also because those are unused above and 'almost' linear in a qwerty\n> keyboard ;D)\n> \n> Or using an index value 1,2,3,4 but I can see how using numeric indexes\n> here could cause difficulty\n> \n> >> .x, .r and .s are all equivalent to [0] in C syntax, and similarly for\n> >> other names. The reason different names are defined and alias each other\n> >> is that vectors can store different types of data, and standardizing on\n> >> {r, g, b, a} would lead to weird-looking code when the vector stores a\n> >> 3D coordinnate for instance ({s, t, p, q} are used as a convention for\n> >> texture coordinates).\n> >>\n> >> The indexing syntax also supports multiple components to be selected:\n> >>\n> >>     vec4 colour(1.0, 2.0, 3.0, 4.0);\n> >>     vec2 value;\n> >>\n> >>     value = colour.rb;\n> >>\n> >>     // value now contains (1.0, 3.0)\n> >>\n> >> The RGB fragment shader code simply contains\n> >>\n> >>     vec3 rgb;\n> >>\n> >>     rgb = texture2D(tex_y, textureOut).RGB_PATTERN;\n> >>\n> >> The pattern thus defines the components mapping.\n> >>\n> >> One additional piece of information that is required is how OpenGL\n> >> expects texture data to be layed out in memory. The texture is created\n> >> with GL_RGBA and GL_UNSIGNED_BYTE, which means that the R, G, B and A\n> >> components are layed down in that order, in one byte each. The RGBA8888\n> >> format, has the opposite convention, it stores RGBA in that order in a\n> >> 32-bit word that is then stored in memory in little endian format\n> > \n> > - Ah... I've missed that little endian part of the puzzle, and have got\n> > confused too.\n> > \n> > Thanks a lot for the detailed explanation!\n> \n> Agreed. Should we add all that to the commit message?\n> It's a really useful explanation - it seems valuable to keep it.\n\nI knew you would say that :-)\n\nI'm not sure we really need to document the OpenGL API and the shader\nlanguage in the commit message though. I've expanded the first paragraph\nto the following:\n\n\"Add support for 24-bit and 32-bit RGB formats. The fragment samples the\ntexture and reorders the components, using a pattern set through the\nRGB_PATTERN macro. The pattern stores the shader vec4 element indices \n(named {r, g, b, a} by convention, for elements 0 to 3) to be extracted \nfrom the texture samples, when interpreted by OpenGL as RGBA.\n\nNote that, as textures are created with GL_UNSIGNED_BYTE, the RGBA\norder corresponds to bytes in memory, while the libcamera formats are\nnamed based on the components order in a 32-bit word stored in memory in\nlittle endian format.\"\n\nI think this provides enough context, please please let me know if you\ndisagree.\n\n> > Reviewed-by: Andrey Konovalov <andrey.konovalov@linaro.org>\n> \n> And this too now ;-)\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> >> , so\n> >> bytes contains A, B, G and R in that order.\n> >>\n> >> So, taking the RGBA8888 example, we have ABGR in memory, which is read\n> >> in a vec4. We want to extract RGB (in that order) in the shader, so we\n> >> need components [3], [2] and [1] of the vector, which are expressed as\n> >> .abg.\n> >>\n> >> I hope this makes it clearer.\n> >>\n> >>>> +        fragmentShaderFile_ = \":RGB.frag\";\n> >>>> +        break;\n> >>>> +    case libcamera::formats::BGR888:\n> >>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n> >>>> +        fragmentShaderFile_ = \":RGB.frag\";\n> >>>> +        break;\n> >>>> +    case libcamera::formats::RGB888:\n> >>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n> >>>> +        fragmentShaderFile_ = \":RGB.frag\";\n> >>>> +        break;\n> >>>>       default:\n> >>>>           ret = false;\n> >>>>           qWarning() << \"[ViewFinderGL]:\"\n> >>>> @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()\n> >>>>                              1.0f / (size_.width() / 2 - 1));\n> >>>>           break;\n> >>>> +    case libcamera::formats::ABGR8888:\n> >>>> +    case libcamera::formats::ARGB8888:\n> >>>> +    case libcamera::formats::BGRA8888:\n> >>>> +    case libcamera::formats::RGBA8888:\n> >>>> +        glActiveTexture(GL_TEXTURE0);\n> >>>> +        configureTexture(*textures_[0]);\n> >>>> +        glTexImage2D(GL_TEXTURE_2D,\n> >>>> +                 0,\n> >>>> +                 GL_RGBA,\n> >>>> +                 size_.width(),\n> >>>> +                 size_.height(),\n> >>>> +                 0,\n> >>>> +                 GL_RGBA,\n> >>>> +                 GL_UNSIGNED_BYTE,\n> >>>> +                 data_);\n> >>>> +        shaderProgram_.setUniformValue(textureUniformY_, 0);\n> >>>> +        break;\n> >>>> +\n> >>>> +    case libcamera::formats::BGR888:\n> >>>> +    case libcamera::formats::RGB888:\n> >>>> +        glActiveTexture(GL_TEXTURE0);\n> >>>> +        configureTexture(*textures_[0]);\n> >>>> +        glTexImage2D(GL_TEXTURE_2D,\n> >>>> +                 0,\n> >>>> +                 GL_RGB,\n> >>>> +                 size_.width(),\n> >>>> +                 size_.height(),\n> >>>> +                 0,\n> >>>> +                 GL_RGB,\n> >>>> +                 GL_UNSIGNED_BYTE,\n> >>>> +                 data_);\n> >>>> +        shaderProgram_.setUniformValue(textureUniformY_, 0);\n> >>>> +        break;\n> >>>> +\n> >>>>       default:\n> >>>>           break;\n> >>>>       };","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B9DF7BE080\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  4 Nov 2020 10:20:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 319B362C46;\n\tWed,  4 Nov 2020 11:20:04 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2965960346\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Nov 2020 11:20:03 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 87F61563;\n\tWed,  4 Nov 2020 11:20:02 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"XjlmErdc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1604485202;\n\tbh=zIPXZ7HyEl89xMObXw5vtsGYs/GNmvLgByFjpTco5oE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XjlmErdcis8n08WMXe5pAdovgyDGaoPNzTD2lZq7RcfLkDtKj287MRYS4ZFp7hvWj\n\tpAoyHFF8HuzjtbSem6ddxAEtCPENdKEXGGJo9ow+aGDKBt2c4j0IRMy0ADalTsp+le\n\tBiS5iaOFlnAygeCaXN84J+Mq95zkdCcyVrSVVGw0=","Date":"Wed, 4 Nov 2020 12:19:15 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Message-ID":"<20201104101915.GF26171@pendragon.ideasonboard.com>","References":"<20201103155025.5948-1-laurent.pinchart@ideasonboard.com>\n\t<20201103155025.5948-8-laurent.pinchart@ideasonboard.com>\n\t<5d50967f-e9b2-2bc7-1442-c670b3d67ef7@ideasonboard.com>\n\t<20201104020915.GI17496@pendragon.ideasonboard.com>\n\t<f660e99c-d81d-5de0-1965-24fd7ab38bda@linaro.org>\n\t<fb810c76-b174-3a50-5ee7-5f08499ea208@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<fb810c76-b174-3a50-5ee7-5f08499ea208@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13599,"web_url":"https://patchwork.libcamera.org/comment/13599/","msgid":"<89974697-ea8d-30a5-c0a1-71b4aa078951@ideasonboard.com>","date":"2020-11-04T10:23:47","subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 04/11/2020 10:19, Laurent Pinchart wrote:\n> Hi Kieran,\n> \n> On Wed, Nov 04, 2020 at 09:49:43AM +0000, Kieran Bingham wrote:\n>> On 04/11/2020 09:32, Andrey Konovalov wrote:\n>>> On 04.11.2020 05:09, Laurent Pinchart wrote:\n>>>> On Tue, Nov 03, 2020 at 11:00:28PM +0000, Kieran Bingham wrote:\n>>>>> On 03/11/2020 15:50, Laurent Pinchart wrote:\n>>>>>> Add support for 24-bit and 32-bit RGB formats. The fragment samples the\n>>>>>> texture and reorders the components, using a pattern set through the\n>>>>>> RGB_PATTERN macro.\n>>>>>>\n>>>>>> An alternative to manual reordering in the shader would be to set the\n>>>>>> texture swizzling mask. This is however not available in OpenGL ES\n>>>>>> before version 3.0, which we don't mandate at the moment.\n>>>>>>\n>>>>>> Only the BGR888 and RGB888 formats have been tested, with the vimc\n>>>>>> pipeline handler.\n>>>>>>\n>>>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n>>>>>> ---\n>>>>>>   src/qcam/assets/shader/RGB.frag    | 22 +++++++++\n>>>>>>   src/qcam/assets/shader/shaders.qrc |  1 +\n>>>>>>   src/qcam/viewfinder_gl.cpp         | 71 ++++++++++++++++++++++++++++--\n>>>>>>   3 files changed, 91 insertions(+), 3 deletions(-)\n>>>>>>   create mode 100644 src/qcam/assets/shader/RGB.frag\n>>>>>>\n>>>>>> diff --git a/src/qcam/assets/shader/RGB.frag\n>>>>>> b/src/qcam/assets/shader/RGB.frag\n>>>>>> new file mode 100644\n>>>>>> index 000000000000..4c374ac98095\n>>>>>> --- /dev/null\n>>>>>> +++ b/src/qcam/assets/shader/RGB.frag\n>>>>>> @@ -0,0 +1,22 @@\n>>>>>> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>>>>>> +/*\n>>>>>> + * Copyright (C) 2020, Laurent Pinchart\n>>>>>> + *\n>>>>>> + * RGB.frag - Fragment shader code for RGB formats\n>>>>>> + */\n>>>>>> +\n>>>>>> +#ifdef GL_ES\n>>>>>> +precision mediump float;\n>>>>>> +#endif\n>>>>>> +\n>>>>>> +varying vec2 textureOut;\n>>>>>> +uniform sampler2D tex_y;\n>>>>>> +\n>>>>>> +void main(void)\n>>>>>> +{\n>>>>>> +    vec3 rgb;\n>>>>>> +\n>>>>>> +    rgb = texture2D(tex_y, textureOut).RGB_PATTERN;\n>>>>>> +\n>>>>>> +    gl_FragColor = vec4(rgb, 1.0);\n>>>>>> +}\n>>>>>> diff --git a/src/qcam/assets/shader/shaders.qrc\n>>>>>> b/src/qcam/assets/shader/shaders.qrc\n>>>>>> index 863109146281..8a8f9de1a5fa 100644\n>>>>>> --- a/src/qcam/assets/shader/shaders.qrc\n>>>>>> +++ b/src/qcam/assets/shader/shaders.qrc\n>>>>>> @@ -1,6 +1,7 @@\n>>>>>>   <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->\n>>>>>>   <!DOCTYPE RCC><RCC version=\"1.0\">\n>>>>>>   <qresource>\n>>>>>> +    <file>RGB.frag</file>\n>>>>>>       <file>YUV_2_planes.frag</file>\n>>>>>>       <file>YUV_3_planes.frag</file>\n>>>>>>       <file>YUV_packed.frag</file>\n>>>>>> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n>>>>>> index cbc1365500f5..5d9b442e7985 100644\n>>>>>> --- a/src/qcam/viewfinder_gl.cpp\n>>>>>> +++ b/src/qcam/viewfinder_gl.cpp\n>>>>>> @@ -14,21 +14,28 @@\n>>>>>>   #include <libcamera/formats.h>\n>>>>>>     static const QList<libcamera::PixelFormat> supportedFormats{\n>>>>>> -    /* Packed (single plane) */\n>>>>>> +    /* YUV - packed (single plane) */\n>>>>>>       libcamera::formats::UYVY,\n>>>>>>       libcamera::formats::VYUY,\n>>>>>>       libcamera::formats::YUYV,\n>>>>>>       libcamera::formats::YVYU,\n>>>>>> -    /* Semi planar (two planes) */\n>>>>>> +    /* YUV - semi planar (two planes) */\n>>>>>>       libcamera::formats::NV12,\n>>>>>>       libcamera::formats::NV21,\n>>>>>>       libcamera::formats::NV16,\n>>>>>>       libcamera::formats::NV61,\n>>>>>>       libcamera::formats::NV24,\n>>>>>>       libcamera::formats::NV42,\n>>>>>> -    /* Fully planar (three planes) */\n>>>>>> +    /* YUV - fully planar (three planes) */\n>>>>>>       libcamera::formats::YUV420,\n>>>>>>       libcamera::formats::YVU420,\n>>>>>> +    /* RGB */\n>>>>>> +    libcamera::formats::ABGR8888,\n>>>>>> +    libcamera::formats::ARGB8888,\n>>>>>> +    libcamera::formats::BGRA8888,\n>>>>>> +    libcamera::formats::RGBA8888,\n>>>>>> +    libcamera::formats::BGR888,\n>>>>>> +    libcamera::formats::RGB888,\n>>>>>>   };\n>>>>>>     ViewFinderGL::ViewFinderGL(QWidget *parent)\n>>>>>> @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const\n>>>>>> libcamera::PixelFormat &format)\n>>>>>>           fragmentShaderDefines_.append(\"#define YUV_PATTERN_YVYU\");\n>>>>>>           fragmentShaderFile_ = \":YUV_packed.frag\";\n>>>>>>           break;\n>>>>>> +    case libcamera::formats::ABGR8888:\n>>>>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n>>>>>\n>>>>> pattern rgb, the same as BGR888, so I presume the A is stripped by not\n>>>>> being used as a channel or some other magic elsewhere?\n>>>>>\n>>>>>> +        fragmentShaderFile_ = \":RGB.frag\";\n>>>>>> +        break;\n>>>>>> +    case libcamera::formats::ARGB8888:\n>>>>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n>>>>>> +        fragmentShaderFile_ = \":RGB.frag\";\n>>>>>> +        break;\n>>>>>> +    case libcamera::formats::BGRA8888:\n>>>>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN gba\");\n>>>>>\n>>>>> Yet here it's specified.... at the end\n>>>>>\n>>>>>> +        fragmentShaderFile_ = \":RGB.frag\";\n>>>>>> +        break;\n>>>>>> +    case libcamera::formats::RGBA8888:\n>>>>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN abg\");\n>>>>>\n>>>>> but here at the beginning?\n>>>>>\n>>>>> What is the definition of the RGB_PATTERNs?\n>>>>> They don't seem to have a pattern that makes sense to me yet.\n>>>>\n>>>> The shader language accesses vector components by name. The texture2D()\n>>>> function, which samples a texture, returns a vec4, a 4-components\n>>>> floating point vector. Multiple component names are supported:\n>>>>\n>>>> - {x, y, z, w}\n>>>> - {r, g, b, a}\n>>>> - {s, t, p, q}\n>>>>\n>>\n>> I wish they also had a set of component names which were simply 'linear'\n>> (e, f, g, h} for instance\n>>\n>> (also because those are unused above and 'almost' linear in a qwerty\n>> keyboard ;D)\n>>\n>> Or using an index value 1,2,3,4 but I can see how using numeric indexes\n>> here could cause difficulty\n>>\n>>>> .x, .r and .s are all equivalent to [0] in C syntax, and similarly for\n>>>> other names. The reason different names are defined and alias each other\n>>>> is that vectors can store different types of data, and standardizing on\n>>>> {r, g, b, a} would lead to weird-looking code when the vector stores a\n>>>> 3D coordinnate for instance ({s, t, p, q} are used as a convention for\n>>>> texture coordinates).\n>>>>\n>>>> The indexing syntax also supports multiple components to be selected:\n>>>>\n>>>>     vec4 colour(1.0, 2.0, 3.0, 4.0);\n>>>>     vec2 value;\n>>>>\n>>>>     value = colour.rb;\n>>>>\n>>>>     // value now contains (1.0, 3.0)\n>>>>\n>>>> The RGB fragment shader code simply contains\n>>>>\n>>>>     vec3 rgb;\n>>>>\n>>>>     rgb = texture2D(tex_y, textureOut).RGB_PATTERN;\n>>>>\n>>>> The pattern thus defines the components mapping.\n>>>>\n>>>> One additional piece of information that is required is how OpenGL\n>>>> expects texture data to be layed out in memory. The texture is created\n>>>> with GL_RGBA and GL_UNSIGNED_BYTE, which means that the R, G, B and A\n>>>> components are layed down in that order, in one byte each. The RGBA8888\n>>>> format, has the opposite convention, it stores RGBA in that order in a\n>>>> 32-bit word that is then stored in memory in little endian format\n>>>\n>>> - Ah... I've missed that little endian part of the puzzle, and have got\n>>> confused too.\n>>>\n>>> Thanks a lot for the detailed explanation!\n>>\n>> Agreed. Should we add all that to the commit message?\n>> It's a really useful explanation - it seems valuable to keep it.\n> \n> I knew you would say that :-)\n> \n> I'm not sure we really need to document the OpenGL API and the shader\n> language in the commit message though. I've expanded the first paragraph\n> to the following:\n\nThat's fine, but you could reference it if there is a suitable\nlink/reference ;-)\n\nI doubt many libcamera reader/developers are also OpenGL developers\n(well, I'm not - but maybe some others are ;D)\n\n\n> \"Add support for 24-bit and 32-bit RGB formats. The fragment samples the\n> texture and reorders the components, using a pattern set through the\n> RGB_PATTERN macro. The pattern stores the shader vec4 element indices \n> (named {r, g, b, a} by convention, for elements 0 to 3) to be extracted \n> from the texture samples, when interpreted by OpenGL as RGBA.\n> \n> Note that, as textures are created with GL_UNSIGNED_BYTE, the RGBA\n> order corresponds to bytes in memory, while the libcamera formats are\n> named based on the components order in a 32-bit word stored in memory in\n> little endian format.\"\n> \n> I think this provides enough context, please please let me know if you\n> disagree.\n\nHighlighting how the indexing works is enough for me to understand the\ncontext of what is not obvious in the code, so that works for me.\n\nThanks\n\nKieran\n\n\n> \n>>> Reviewed-by: Andrey Konovalov <andrey.konovalov@linaro.org>\n>>\n>> And this too now ;-)\n>>\n>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n>>\n>>>> , so\n>>>> bytes contains A, B, G and R in that order.\n>>>>\n>>>> So, taking the RGBA8888 example, we have ABGR in memory, which is read\n>>>> in a vec4. We want to extract RGB (in that order) in the shader, so we\n>>>> need components [3], [2] and [1] of the vector, which are expressed as\n>>>> .abg.\n>>>>\n>>>> I hope this makes it clearer.\n>>>>\n>>>>>> +        fragmentShaderFile_ = \":RGB.frag\";\n>>>>>> +        break;\n>>>>>> +    case libcamera::formats::BGR888:\n>>>>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n>>>>>> +        fragmentShaderFile_ = \":RGB.frag\";\n>>>>>> +        break;\n>>>>>> +    case libcamera::formats::RGB888:\n>>>>>> +        fragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n>>>>>> +        fragmentShaderFile_ = \":RGB.frag\";\n>>>>>> +        break;\n>>>>>>       default:\n>>>>>>           ret = false;\n>>>>>>           qWarning() << \"[ViewFinderGL]:\"\n>>>>>> @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()\n>>>>>>                              1.0f / (size_.width() / 2 - 1));\n>>>>>>           break;\n>>>>>> +    case libcamera::formats::ABGR8888:\n>>>>>> +    case libcamera::formats::ARGB8888:\n>>>>>> +    case libcamera::formats::BGRA8888:\n>>>>>> +    case libcamera::formats::RGBA8888:\n>>>>>> +        glActiveTexture(GL_TEXTURE0);\n>>>>>> +        configureTexture(*textures_[0]);\n>>>>>> +        glTexImage2D(GL_TEXTURE_2D,\n>>>>>> +                 0,\n>>>>>> +                 GL_RGBA,\n>>>>>> +                 size_.width(),\n>>>>>> +                 size_.height(),\n>>>>>> +                 0,\n>>>>>> +                 GL_RGBA,\n>>>>>> +                 GL_UNSIGNED_BYTE,\n>>>>>> +                 data_);\n>>>>>> +        shaderProgram_.setUniformValue(textureUniformY_, 0);\n>>>>>> +        break;\n>>>>>> +\n>>>>>> +    case libcamera::formats::BGR888:\n>>>>>> +    case libcamera::formats::RGB888:\n>>>>>> +        glActiveTexture(GL_TEXTURE0);\n>>>>>> +        configureTexture(*textures_[0]);\n>>>>>> +        glTexImage2D(GL_TEXTURE_2D,\n>>>>>> +                 0,\n>>>>>> +                 GL_RGB,\n>>>>>> +                 size_.width(),\n>>>>>> +                 size_.height(),\n>>>>>> +                 0,\n>>>>>> +                 GL_RGB,\n>>>>>> +                 GL_UNSIGNED_BYTE,\n>>>>>> +                 data_);\n>>>>>> +        shaderProgram_.setUniformValue(textureUniformY_, 0);\n>>>>>> +        break;\n>>>>>> +\n>>>>>>       default:\n>>>>>>           break;\n>>>>>>       };\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 8CE3FBDB89\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  4 Nov 2020 10:23:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 183FB60353;\n\tWed,  4 Nov 2020 11:23:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A931160344\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  4 Nov 2020 11:23:50 +0100 (CET)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 02382563;\n\tWed,  4 Nov 2020 11:23:49 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"smRQkRg2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1604485430;\n\tbh=0XlqWMsH+ijtWAVOMnsexuQ4rgSk7E0wAEhEAkny63c=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=smRQkRg2ZYfVNOi7g+w+ekeWluLpq9ibZIw5tIL5SCaEqxLV5jwxdTgfPmKmXlrah\n\tTpPiuQ/GEmZ0Nv91gQT6x+Ms64gShUbvfLUNzBmH6kwzjlv9f26mpst3mT07HdWBoi\n\trk+kh6N8KZz+N084HPp2nfmXxOHTz6nFQymZGBw0=","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","References":"<20201103155025.5948-1-laurent.pinchart@ideasonboard.com>\n\t<20201103155025.5948-8-laurent.pinchart@ideasonboard.com>\n\t<5d50967f-e9b2-2bc7-1442-c670b3d67ef7@ideasonboard.com>\n\t<20201104020915.GI17496@pendragon.ideasonboard.com>\n\t<f660e99c-d81d-5de0-1965-24fd7ab38bda@linaro.org>\n\t<fb810c76-b174-3a50-5ee7-5f08499ea208@ideasonboard.com>\n\t<20201104101915.GF26171@pendragon.ideasonboard.com>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<89974697-ea8d-30a5-c0a1-71b4aa078951@ideasonboard.com>","Date":"Wed, 4 Nov 2020 10:23:47 +0000","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.10.0","MIME-Version":"1.0","In-Reply-To":"<20201104101915.GF26171@pendragon.ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":13624,"web_url":"https://patchwork.libcamera.org/comment/13624/","msgid":"<20201106111731.GG3013826@oden.dyn.berto.se>","date":"2020-11-06T11:17:31","subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your patch.\n\nOn 2020-11-03 17:50:25 +0200, Laurent Pinchart wrote:\n> Add support for 24-bit and 32-bit RGB formats. The fragment samples the\n> texture and reorders the components, using a pattern set through the\n> RGB_PATTERN macro.\n> \n> An alternative to manual reordering in the shader would be to set the\n> texture swizzling mask. This is however not available in OpenGL ES\n> before version 3.0, which we don't mandate at the moment.\n> \n> Only the BGR888 and RGB888 formats have been tested, with the vimc\n> pipeline handler.\n> \n> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\n> ---\n>  src/qcam/assets/shader/RGB.frag    | 22 +++++++++\n>  src/qcam/assets/shader/shaders.qrc |  1 +\n>  src/qcam/viewfinder_gl.cpp         | 71 ++++++++++++++++++++++++++++--\n>  3 files changed, 91 insertions(+), 3 deletions(-)\n>  create mode 100644 src/qcam/assets/shader/RGB.frag\n> \n> diff --git a/src/qcam/assets/shader/RGB.frag b/src/qcam/assets/shader/RGB.frag\n> new file mode 100644\n> index 000000000000..4c374ac98095\n> --- /dev/null\n> +++ b/src/qcam/assets/shader/RGB.frag\n> @@ -0,0 +1,22 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Laurent Pinchart\n> + *\n> + * RGB.frag - Fragment shader code for RGB formats\n> + */\n> +\n> +#ifdef GL_ES\n> +precision mediump float;\n> +#endif\n> +\n> +varying vec2 textureOut;\n> +uniform sampler2D tex_y;\n> +\n> +void main(void)\n> +{\n> +\tvec3 rgb;\n> +\n> +\trgb = texture2D(tex_y, textureOut).RGB_PATTERN;\n> +\n> +\tgl_FragColor = vec4(rgb, 1.0);\n> +}\n> diff --git a/src/qcam/assets/shader/shaders.qrc b/src/qcam/assets/shader/shaders.qrc\n> index 863109146281..8a8f9de1a5fa 100644\n> --- a/src/qcam/assets/shader/shaders.qrc\n> +++ b/src/qcam/assets/shader/shaders.qrc\n> @@ -1,6 +1,7 @@\n>  <!-- SPDX-License-Identifier: LGPL-2.1-or-later -->\n>  <!DOCTYPE RCC><RCC version=\"1.0\">\n>  <qresource>\n> +\t<file>RGB.frag</file>\n>  \t<file>YUV_2_planes.frag</file>\n>  \t<file>YUV_3_planes.frag</file>\n>  \t<file>YUV_packed.frag</file>\n> diff --git a/src/qcam/viewfinder_gl.cpp b/src/qcam/viewfinder_gl.cpp\n> index cbc1365500f5..5d9b442e7985 100644\n> --- a/src/qcam/viewfinder_gl.cpp\n> +++ b/src/qcam/viewfinder_gl.cpp\n> @@ -14,21 +14,28 @@\n>  #include <libcamera/formats.h>\n>  \n>  static const QList<libcamera::PixelFormat> supportedFormats{\n> -\t/* Packed (single plane) */\n> +\t/* YUV - packed (single plane) */\n>  \tlibcamera::formats::UYVY,\n>  \tlibcamera::formats::VYUY,\n>  \tlibcamera::formats::YUYV,\n>  \tlibcamera::formats::YVYU,\n> -\t/* Semi planar (two planes) */\n> +\t/* YUV - semi planar (two planes) */\n>  \tlibcamera::formats::NV12,\n>  \tlibcamera::formats::NV21,\n>  \tlibcamera::formats::NV16,\n>  \tlibcamera::formats::NV61,\n>  \tlibcamera::formats::NV24,\n>  \tlibcamera::formats::NV42,\n> -\t/* Fully planar (three planes) */\n> +\t/* YUV - fully planar (three planes) */\n>  \tlibcamera::formats::YUV420,\n>  \tlibcamera::formats::YVU420,\n> +\t/* RGB */\n> +\tlibcamera::formats::ABGR8888,\n> +\tlibcamera::formats::ARGB8888,\n> +\tlibcamera::formats::BGRA8888,\n> +\tlibcamera::formats::RGBA8888,\n> +\tlibcamera::formats::BGR888,\n> +\tlibcamera::formats::RGB888,\n>  };\n>  \n>  ViewFinderGL::ViewFinderGL(QWidget *parent)\n> @@ -172,6 +179,30 @@ bool ViewFinderGL::selectFormat(const libcamera::PixelFormat &format)\n>  \t\tfragmentShaderDefines_.append(\"#define YUV_PATTERN_YVYU\");\n>  \t\tfragmentShaderFile_ = \":YUV_packed.frag\";\n>  \t\tbreak;\n> +\tcase libcamera::formats::ABGR8888:\n> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::ARGB8888:\n> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::BGRA8888:\n> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN gba\");\n> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::RGBA8888:\n> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN abg\");\n> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::BGR888:\n> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN rgb\");\n> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> +\t\tbreak;\n> +\tcase libcamera::formats::RGB888:\n> +\t\tfragmentShaderDefines_.append(\"#define RGB_PATTERN bgr\");\n> +\t\tfragmentShaderFile_ = \":RGB.frag\";\n> +\t\tbreak;\n>  \tdefault:\n>  \t\tret = false;\n>  \t\tqWarning() << \"[ViewFinderGL]:\"\n> @@ -481,6 +512,40 @@ void ViewFinderGL::doRender()\n>  \t\t\t\t\t       1.0f / (size_.width() / 2 - 1));\n>  \t\tbreak;\n>  \n> +\tcase libcamera::formats::ABGR8888:\n> +\tcase libcamera::formats::ARGB8888:\n> +\tcase libcamera::formats::BGRA8888:\n> +\tcase libcamera::formats::RGBA8888:\n> +\t\tglActiveTexture(GL_TEXTURE0);\n> +\t\tconfigureTexture(*textures_[0]);\n> +\t\tglTexImage2D(GL_TEXTURE_2D,\n> +\t\t\t     0,\n> +\t\t\t     GL_RGBA,\n> +\t\t\t     size_.width(),\n> +\t\t\t     size_.height(),\n> +\t\t\t     0,\n> +\t\t\t     GL_RGBA,\n> +\t\t\t     GL_UNSIGNED_BYTE,\n> +\t\t\t     data_);\n> +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n> +\t\tbreak;\n> +\n> +\tcase libcamera::formats::BGR888:\n> +\tcase libcamera::formats::RGB888:\n> +\t\tglActiveTexture(GL_TEXTURE0);\n> +\t\tconfigureTexture(*textures_[0]);\n> +\t\tglTexImage2D(GL_TEXTURE_2D,\n> +\t\t\t     0,\n> +\t\t\t     GL_RGB,\n> +\t\t\t     size_.width(),\n> +\t\t\t     size_.height(),\n> +\t\t\t     0,\n> +\t\t\t     GL_RGB,\n> +\t\t\t     GL_UNSIGNED_BYTE,\n> +\t\t\t     data_);\n> +\t\tshaderProgram_.setUniformValue(textureUniformY_, 0);\n> +\t\tbreak;\n> +\n>  \tdefault:\n>  \t\tbreak;\n>  \t};\n> -- \n> Regards,\n> \n> Laurent Pinchart\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2D2E9BDB89\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri,  6 Nov 2020 11:17:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B0DD762D22;\n\tFri,  6 Nov 2020 12:17:35 +0100 (CET)","from mail-lj1-x241.google.com (mail-lj1-x241.google.com\n\t[IPv6:2a00:1450:4864:20::241])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EDEEF62D1B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri,  6 Nov 2020 12:17:33 +0100 (CET)","by mail-lj1-x241.google.com with SMTP id t13so948894ljk.12\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 06 Nov 2020 03:17:33 -0800 (PST)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tq16sm110145ljj.32.2020.11.06.03.17.32\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 06 Nov 2020 03:17:32 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"F3TUMNWq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=X++two/eZcGkMBMS3EmnbxarhMC8VPsdBQwZLUPmcck=;\n\tb=F3TUMNWqm5OlsEqEI80Dt6ivmmSvlJS+F1VD14shhVarqx5kwCxe9PM07AseDrFi84\n\tvulgwDioQUk9EORZZPJAqcrzNZl98IfNdZkTGMA3wWhIFEJ2BjUMUlSy84PpTWbzxXbN\n\t2ojyPT7dela0s67hMfQv15osegPqhzBBQwQ3I82kgt/RVTx/WUmG6KvpAgmxLjlD/67V\n\t0i1MgaPvtROf3CIDQrbARwA7FOffI1AhAz5F6dJUcRXDllyzWMTtFQhuXFzOAn5xkSIi\n\t2Ok7zhgRJySBQHzvYVA1CVe1pdCy+9TvuWyxUkBd2+ae97DwlhKA2ItaPTHeFNk/wUIH\n\tlQ2g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=X++two/eZcGkMBMS3EmnbxarhMC8VPsdBQwZLUPmcck=;\n\tb=eSRieQm3M7PXF8z0uzy5Z3CrYAPmcZLP9TPXAMNhKkGyquqTyGoeT00I8eoReC7wl+\n\ty0WH2KmJ82XTMHt2M9EhnwGi7oP8Kq1s63QsBnNOfg9ZtMXc1Qip9a6pZ/u7pXqkRs2g\n\tHJHIOyNuKWtZzY4QYoK4Ml2OWxw1JOvIn5wQUVyZuY6m6345KuD8MeksbfkT0cWXZuRD\n\tGFGojvoDoDIBbUDEYt5++SoCf/aSvV7d7vb3g934gdxj8Bvq5uxtjZSqYaWe84MlLvXs\n\tybF38rrNbfyURtkAnsfY9+0/LUJFIehpruSKlpfZILSbEgJ1Z5ksJQ6D4nmf0cLBm7m4\n\tlMuQ==","X-Gm-Message-State":"AOAM5311gCnVufyCZwgKYv7k2jmIvNeFTSIc9Xn0T2JLOTR2Gg6t3xwT\n\tfcDJNVQE02SzVwkkLkBVN3tDtQ==","X-Google-Smtp-Source":"ABdhPJyqA1CTOWrTsyMPitHzy7DuafDst+nCkdUqgqGlK1nlOk0BIgYSIIfB5aVaw4LwOuF5PXAZhQ==","X-Received":"by 2002:a2e:9909:: with SMTP id v9mr597271lji.429.1604661453214; \n\tFri, 06 Nov 2020 03:17:33 -0800 (PST)","Date":"Fri, 6 Nov 2020 12:17:31 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Message-ID":"<20201106111731.GG3013826@oden.dyn.berto.se>","References":"<20201103155025.5948-1-laurent.pinchart@ideasonboard.com>\n\t<20201103155025.5948-8-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201103155025.5948-8-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 7/7] qcam: viewfinder_gl: Add support\n\tfor RGB formats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]