[{"id":39516,"web_url":"https://patchwork.libcamera.org/comment/39516/","msgid":"<85echo1g6q.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","date":"2026-06-30T10:22:05","subject":"Re: [PATCH 4/4] libcamera: shaders: Replace C array with std::array","submitter":{"id":177,"url":"https://patchwork.libcamera.org/api/people/177/","name":"Milan Zamazal","email":"mzamazal@redhat.com"},"content":"Laurent Pinchart writes:\n\n> Using C arrays and separate length variables is error-prone. Replace\n> them with std::array in the generated shader header, and update the\n> software ISP code accordingly.\n>\n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Milan Zamazal \n\n> ---\n> src/libcamera/software_isp/debayer_egl.cpp | 23 +++++++---------------\n> utils/codegen/gen-shader-header.py | 7 ++++---\n> 2 files changed, 11 insertions(+), 19 deletions(-)\n>\n> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp\n> index af9b9d426920..1f5fc6a4466d 100644\n> --- a/src/libcamera/software_isp/debayer_egl.cpp\n> +++ b/src/libcamera/software_isp/debayer_egl.cpp\n> @@ -17,6 +17,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> #include \n> @@ -145,10 +146,8 @@ int DebayerEGL::getShaderVariableLocations(void)\n> int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputFormat)\n> {\n> \tstd::vector shaderEnv;\n> -\tunsigned int fragmentShaderDataLen = 0;\n> -\tconst unsigned char *fragmentShaderData = 0;\n> -\tunsigned int vertexShaderDataLen = 0;\n> -\tconst unsigned char *vertexShaderData = 0;\n> +\tSpan fragmentShaderData;\n> +\tSpan vertexShaderData;\n> \tGLenum err;\n> \n> \t/* Target gles 100 glsl requires \"#version x\" as first directive in shader */\n> @@ -216,9 +215,7 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm\n> \tcase libcamera::formats::SGRBG8:\n> \tcase libcamera::formats::SRGGB8:\n> \t\tfragmentShaderData = bayer_unpacked_frag;\n> -\t\tfragmentShaderDataLen = bayer_unpacked_frag_len;\n> \t\tvertexShaderData = bayer_unpacked_vert;\n> -\t\tvertexShaderDataLen = bayer_unpacked_vert_len;\n> \t\tbreak;\n> \tcase libcamera::formats::SBGGR10_CSI2P:\n> \tcase libcamera::formats::SGBRG10_CSI2P:\n> @@ -227,16 +224,12 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm\n> \t\tegl_.pushEnv(shaderEnv, \"#define RAW10P\");\n> \t\tif (BayerFormat::fromPixelFormat(inputFormat).packing == BayerFormat::Packing::None) {\n> \t\t\tfragmentShaderData = bayer_unpacked_frag;\n> -\t\t\tfragmentShaderDataLen = bayer_unpacked_frag_len;\n> \t\t\tvertexShaderData = bayer_unpacked_vert;\n> -\t\t\tvertexShaderDataLen = bayer_unpacked_vert_len;\n> \t\t\tglFormat_ = GL_RG;\n> \t\t\tbytesPerPixel_ = 2;\n> \t\t} else {\n> \t\t\tfragmentShaderData = bayer_1x_packed_frag;\n> -\t\t\tfragmentShaderDataLen = bayer_1x_packed_frag_len;\n> \t\t\tvertexShaderData = identity_vert;\n> -\t\t\tvertexShaderDataLen = identity_vert_len;\n> \t\t\tshaderStridePixels_ = width_;\n> \t\t}\n> \t\tbreak;\n> @@ -247,28 +240,26 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm\n> \t\tegl_.pushEnv(shaderEnv, \"#define RAW12P\");\n> \t\tif (BayerFormat::fromPixelFormat(inputFormat).packing == BayerFormat::Packing::None) {\n> \t\t\tfragmentShaderData = bayer_unpacked_frag;\n> -\t\t\tfragmentShaderDataLen = bayer_unpacked_frag_len;\n> \t\t\tvertexShaderData = bayer_unpacked_vert;\n> -\t\t\tvertexShaderDataLen = bayer_unpacked_vert_len;\n> \t\t\tglFormat_ = GL_RG;\n> \t\t\tbytesPerPixel_ = 2;\n> \t\t} else {\n> \t\t\tfragmentShaderData = bayer_1x_packed_frag;\n> -\t\t\tfragmentShaderDataLen = bayer_1x_packed_frag_len;\n> \t\t\tvertexShaderData = identity_vert;\n> -\t\t\tvertexShaderDataLen = identity_vert_len;\n> \t\t\tshaderStridePixels_ = width_;\n> \t\t}\n> \t\tbreak;\n> \t};\n> \n> -\tif (egl_.compileVertexShader(vertexShaderId_, vertexShaderData, vertexShaderDataLen, shaderEnv)) {\n> +\tif (egl_.compileVertexShader(vertexShaderId_, vertexShaderData.data(),\n> +\t\t\t\t vertexShaderData.size(), shaderEnv)) {\n> \t\tLOG(Debayer, Error) << \"Compile vertex shader fail\";\n> \t\treturn -ENODEV;\n> \t}\n> \tutils::scope_exit vShaderGuard([&] { glDeleteShader(vertexShaderId_); });\n> \n> -\tif (egl_.compileFragmentShader(fragmentShaderId_, fragmentShaderData, fragmentShaderDataLen, shaderEnv)) {\n> +\tif (egl_.compileFragmentShader(fragmentShaderId_, fragmentShaderData.data(),\n> +\t\t\t\t fragmentShaderData.size(), shaderEnv)) {\n> \t\tLOG(Debayer, Error) << \"Compile fragment shader fail\";\n> \t\treturn -ENODEV;\n> \t}\n> diff --git a/utils/codegen/gen-shader-header.py b/utils/codegen/gen-shader-header.py\n> index 908217a2bd2e..0bcf76653d77 100755\n> --- a/utils/codegen/gen-shader-header.py\n> +++ b/utils/codegen/gen-shader-header.py\n> @@ -19,15 +19,14 @@ def process_file(name, out):\n> hex_data = [f'0x{c:02x}' for c in data]\n> var_name = os.path.basename(name).replace('.', '_')\n> \n> - out.write(f'unsigned char const {var_name}[] = {{\\n')\n> + out.write(f'static constexpr std::array {var_name}{{\\n')\n> \n> for i in range(math.ceil(len(data) / 16)):\n> out.write('\\t')\n> out.write(', '.join(hex_data[16 * i:16 * (i + 1)]))\n> out.write(',\\n')\n> \n> - out.write('};\\n\\n')\n> - out.write(f'const unsigned int {var_name}_len = {len(data)};\\n')\n> + out.write('};\\n')\n> \n> \n> def main(argv):\n> @@ -45,6 +44,8 @@ def main(argv):\n> \n> #pragma once\n> \n> +#include \n> +\n> /*\n> * List the names of the shaders at the top of header for readability's sake.\n> *","headers":{"Return-Path":"","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 206D7C3264\n\tfor ;\n\tTue, 30 Jun 2026 10:22:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 583E565F5A;\n\tTue, 30 Jun 2026 12:22:16 +0200 (CEST)","from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.133.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E7C8F656D3\n\tfor ;\n\tTue, 30 Jun 2026 12:22:13 +0200 (CEST)","from mail-wr1-f72.google.com (mail-wr1-f72.google.com\n\t[209.85.221.72]) by relay.mimecast.com with ESMTP with STARTTLS\n\t(version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id\n\tus-mta-246-gqNoGKA4Oe6qcFO3-_LAcg-1; Tue, 30 Jun 2026 06:22:11 -0400","by mail-wr1-f72.google.com with SMTP id\n\tffacd0b85a97d-4629f312a67so3768550f8f.2\n\tfor ;\n\tTue, 30 Jun 2026 03:22:11 -0700 (PDT)","from mzamazal-thinkpadp1gen7.tpbc.csb\n\t(ip-77-48-47-4.net.vodafone.cz. [77.48.47.4])\n\tby smtp.gmail.com with ESMTPSA id\n\tffacd0b85a97d-47563e0eee5sm6388193f8f.5.2026.06.30.03.22.08\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 30 Jun 2026 03:22:08 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"XgzhGICA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1782814932;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=/FRNe6bFte93QQe/eRanX8A6itAAk+LNL1wuIunAfrk=;\n\tb=XgzhGICAzSrzrkvHFFtd/iVcrJ1j5k90O8OguZMiTUnx9WShSlUl2pAgP8lM1cr+4NzQ5R\n\t7BDhmlkL4eEjyCGC6ZuKwXUVq3iBoDXaKT7FOFMzFi/y6iDZ+/nUrAghtGJ28ychnmRCXK\n\tdJSzSxDmTQhHD1zFv5qMG6MPGXK2mGU=","X-MC-Unique":"gqNoGKA4Oe6qcFO3-_LAcg-1","X-Mimecast-MFC-AGG-ID":"gqNoGKA4Oe6qcFO3-_LAcg_1782814930","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1782814930; x=1783419730;\n\th=mime-version:user-agent:message-id:date:references:in-reply-to\n\t:subject:cc:to:from:x-gm-gg:x-gm-message-state:from:to:cc:subject\n\t:date:message-id:reply-to;\n\tbh=/FRNe6bFte93QQe/eRanX8A6itAAk+LNL1wuIunAfrk=;\n\tb=esPkNK7l5kcT+AKNJMaFgRXBA+8zXXHfm2A1RobgjR1sLojjuUozt8gQqgiGnrNsC7\n\tVMz0OeGfogb1Q2PlP3oCYAG6Ynn7eU9P6kz6ZnSGCZaYv4ogogF1FW+70BGF3Cbtvyjv\n\toMWTW0X9IanhWIH/Pp142ojn9zydqYhq+fNlQR+cBf2H93D8r/q7ThUPDaX0FPRb9pSU\n\tDxtQx6qb/oc7L7Yn7ZxNDqpPPII/6mLcPSbS2gNRd0fHWuhF6rXXbKhePu0k80HERzAf\n\t3LYyeyIJ13YRglQpv405DoK/hSMywAir1xvbnvdfjXRN0eHiG9jI/RKbaDg6H+58/dOu\n\t3dNw==","X-Gm-Message-State":"AOJu0Ywofu39J+3COrhAndgW5yPWIEWcaDFXB0+++cRZGXKuPB3LBiTh\n\tDktFhPhs76JvYFPLwCHjbSKVydF7Il2j+rOtM06n+/Y+zhGbRdVhpnysTDeKYOcW5ZQtEPGib9i\n\tdW9cmZ18x2DPUKxzs6AL4B3MZ2K36Hv8Uob95O3fuLZf0hv4b6RiekUp9CkJ52t7HpfP6qUl8MT\n\t1yJyJfl5w=","X-Gm-Gg":"AfdE7cky3MWLzPkTzNjWGLLoVJz0ILAOoVf7yHGy6iI23pQsykdqJppXwwUj2M25igS\n\tjH6iHNb47YL7EaArgQadThLpIHC/O0+/LbVc8cpnqtDkEK4PG5594ZoJdBB05SHpRJlaRDifk0g\n\t33xBwcpmN8CaUiNFP9sWiQ6cbfQ0bbje45OE6U7JvKnuP2DvjT0JpdpzwPQtgApg4REjw1hb3Yr\n\tHUJJ+Yi0ag/r6/4YdMpQQPeWNtWcSncWhQyIiaWlKWgoFDOnjZ3ir3BpQBoj9Ktvhbkhu5uTemD\n\t/Of8y3FvXzFC7VX8apegU3kba3V1+pPkGj2UaidPO7sboxrodPltABQujOqjStlrlDSTW7uxtN2\n\tfPvVqRUKTbo7JuK/iVEDan0L+TnJjW1dHvc4jFRmURPNBwUZiJQJeMM+V6ee06E6M","X-Received":["by 2002:a05:6000:460d:b0:472:f8b5:aaca with SMTP id\n\tffacd0b85a97d-475507ddf80mr4207937f8f.15.1782814929874; \n\tTue, 30 Jun 2026 03:22:09 -0700 (PDT)","by 2002:a05:6000:460d:b0:472:f8b5:aaca with SMTP id\n\tffacd0b85a97d-475507ddf80mr4207910f8f.15.1782814929475; \n\tTue, 30 Jun 2026 03:22:09 -0700 (PDT)"],"From":"Milan Zamazal ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, Bryan O'Donoghue\n\t","Subject":"Re: [PATCH 4/4] libcamera: shaders: Replace C array with std::array","In-Reply-To":"<20260630083031.3197714-5-laurent.pinchart@ideasonboard.com>\n\t(Laurent Pinchart's message of \"Tue, 30 Jun 2026 11:30:31 +0300\")","References":"<20260630083031.3197714-1-laurent.pinchart@ideasonboard.com>\n\t<20260630083031.3197714-5-laurent.pinchart@ideasonboard.com>","Date":"Tue, 30 Jun 2026 12:22:05 +0200","Message-ID":"<85echo1g6q.fsf@mzamazal-thinkpadp1gen7.tpbc.csb>","User-Agent":"Gnus/5.13 (Gnus v5.13)","MIME-Version":"1.0","X-Mimecast-Spam-Score":"0","X-Mimecast-MFC-PROC-ID":"EVmWpf2N0SanDUcb5LGjfiZDOV1ToEm9J6YrnkRYsok_1782814930","X-Mimecast-Originator":"redhat.com","Content-Type":"text/plain","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":39521,"web_url":"https://patchwork.libcamera.org/comment/39521/","msgid":"<5942dd86-aff5-45bd-9983-630700930412@ideasonboard.com>","date":"2026-06-30T10:47:27","subject":"Re: [PATCH 4/4] libcamera: shaders: Replace C array with std::array","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"2026. 06. 30. 10:30 keltezéssel, Laurent Pinchart írta:\n> Using C arrays and separate length variables is error-prone. Replace\n> them with std::array in the generated shader header, and update the\n> software ISP code accordingly.\n\nSince the array is defined in the header, I think there is not much difference\nbetween an std::array and a plain array, especially since they are const.\nBut I support the removal of the separate length field.\n\n\n> \n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/software_isp/debayer_egl.cpp | 23 +++++++---------------\n> utils/codegen/gen-shader-header.py | 7 ++++---\n> 2 files changed, 11 insertions(+), 19 deletions(-)\n> \n> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp\n> index af9b9d426920..1f5fc6a4466d 100644\n> --- a/src/libcamera/software_isp/debayer_egl.cpp\n> +++ b/src/libcamera/software_isp/debayer_egl.cpp\n> @@ -17,6 +17,7 @@\n> #include \n> #include \n> \n> +#include \n> #include \n> \n> #include \n> @@ -145,10 +146,8 @@ int DebayerEGL::getShaderVariableLocations(void)\n> int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputFormat)\n> {\n> \tstd::vector shaderEnv;\n> -\tunsigned int fragmentShaderDataLen = 0;\n> -\tconst unsigned char *fragmentShaderData = 0;\n> -\tunsigned int vertexShaderDataLen = 0;\n> -\tconst unsigned char *vertexShaderData = 0;\n> +\tSpan fragmentShaderData;\n> +\tSpan vertexShaderData;\n> \tGLenum err;\n> \n> \t/* Target gles 100 glsl requires \"#version x\" as first directive in shader */\n> @@ -216,9 +215,7 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm\n> \tcase libcamera::formats::SGRBG8:\n> \tcase libcamera::formats::SRGGB8:\n> \t\tfragmentShaderData = bayer_unpacked_frag;\n> -\t\tfragmentShaderDataLen = bayer_unpacked_frag_len;\n> \t\tvertexShaderData = bayer_unpacked_vert;\n> -\t\tvertexShaderDataLen = bayer_unpacked_vert_len;\n> \t\tbreak;\n> \tcase libcamera::formats::SBGGR10_CSI2P:\n> \tcase libcamera::formats::SGBRG10_CSI2P:\n> @@ -227,16 +224,12 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm\n> \t\tegl_.pushEnv(shaderEnv, \"#define RAW10P\");\n> \t\tif (BayerFormat::fromPixelFormat(inputFormat).packing == BayerFormat::Packing::None) {\n> \t\t\tfragmentShaderData = bayer_unpacked_frag;\n> -\t\t\tfragmentShaderDataLen = bayer_unpacked_frag_len;\n> \t\t\tvertexShaderData = bayer_unpacked_vert;\n> -\t\t\tvertexShaderDataLen = bayer_unpacked_vert_len;\n> \t\t\tglFormat_ = GL_RG;\n> \t\t\tbytesPerPixel_ = 2;\n> \t\t} else {\n> \t\t\tfragmentShaderData = bayer_1x_packed_frag;\n> -\t\t\tfragmentShaderDataLen = bayer_1x_packed_frag_len;\n> \t\t\tvertexShaderData = identity_vert;\n> -\t\t\tvertexShaderDataLen = identity_vert_len;\n> \t\t\tshaderStridePixels_ = width_;\n> \t\t}\n> \t\tbreak;\n> @@ -247,28 +240,26 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm\n> \t\tegl_.pushEnv(shaderEnv, \"#define RAW12P\");\n> \t\tif (BayerFormat::fromPixelFormat(inputFormat).packing == BayerFormat::Packing::None) {\n> \t\t\tfragmentShaderData = bayer_unpacked_frag;\n> -\t\t\tfragmentShaderDataLen = bayer_unpacked_frag_len;\n> \t\t\tvertexShaderData = bayer_unpacked_vert;\n> -\t\t\tvertexShaderDataLen = bayer_unpacked_vert_len;\n> \t\t\tglFormat_ = GL_RG;\n> \t\t\tbytesPerPixel_ = 2;\n> \t\t} else {\n> \t\t\tfragmentShaderData = bayer_1x_packed_frag;\n> -\t\t\tfragmentShaderDataLen = bayer_1x_packed_frag_len;\n> \t\t\tvertexShaderData = identity_vert;\n> -\t\t\tvertexShaderDataLen = identity_vert_len;\n> \t\t\tshaderStridePixels_ = width_;\n> \t\t}\n> \t\tbreak;\n> \t};\n> \n> -\tif (egl_.compileVertexShader(vertexShaderId_, vertexShaderData, vertexShaderDataLen, shaderEnv)) {\n> +\tif (egl_.compileVertexShader(vertexShaderId_, vertexShaderData.data(),\n> +\t\t\t\t vertexShaderData.size(), shaderEnv)) {\n> \t\tLOG(Debayer, Error) << \"Compile vertex shader fail\";\n> \t\treturn -ENODEV;\n> \t}\n> \tutils::scope_exit vShaderGuard([&] { glDeleteShader(vertexShaderId_); });\n> \n> -\tif (egl_.compileFragmentShader(fragmentShaderId_, fragmentShaderData, fragmentShaderDataLen, shaderEnv)) {\n> +\tif (egl_.compileFragmentShader(fragmentShaderId_, fragmentShaderData.data(),\n> +\t\t\t\t fragmentShaderData.size(), shaderEnv)) {\n\nI think `eGL::compileShader` et al should be modified to take `Span source`.\n\nReviewed-by: Barnabás Pőcze \n\n\n> \t\tLOG(Debayer, Error) << \"Compile fragment shader fail\";\n> \t\treturn -ENODEV;\n> \t}\n> diff --git a/utils/codegen/gen-shader-header.py b/utils/codegen/gen-shader-header.py\n> index 908217a2bd2e..0bcf76653d77 100755\n> --- a/utils/codegen/gen-shader-header.py\n> +++ b/utils/codegen/gen-shader-header.py\n> @@ -19,15 +19,14 @@ def process_file(name, out):\n> hex_data = [f'0x{c:02x}' for c in data]\n> var_name = os.path.basename(name).replace('.', '_')\n> \n> - out.write(f'unsigned char const {var_name}[] = {{\\n')\n> + out.write(f'static constexpr std::array {var_name}{{\\n')\n> \n> for i in range(math.ceil(len(data) / 16)):\n> out.write('\\t')\n> out.write(', '.join(hex_data[16 * i:16 * (i + 1)]))\n> out.write(',\\n')\n> \n> - out.write('};\\n\\n')\n> - out.write(f'const unsigned int {var_name}_len = {len(data)};\\n')\n> + out.write('};\\n')\n> \n> \n> def main(argv):\n> @@ -45,6 +44,8 @@ def main(argv):\n> \n> #pragma once\n> \n> +#include \n> +\n> /*\n> * List the names of the shaders at the top of header for readability's sake.\n> *","headers":{"Return-Path":"","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 5FFCDC3303\n\tfor ;\n\tTue, 30 Jun 2026 10:47:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D79DC65F79;\n\tTue, 30 Jun 2026 12:47:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E2A9365F5E\n\tfor ;\n\tTue, 30 Jun 2026 12:47:27 +0200 (CEST)","from [192.168.33.31] (185.221.140.128.nat.pool.zt.hu\n\t[185.221.140.128])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 07B03D52;\n\tTue, 30 Jun 2026 12:46:43 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"GrtY6MKk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1782816404;\n\tbh=hW0G0BEIXgGUudAjMkD267cSgcMQNkY09cGWn/XW8b4=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=GrtY6MKkH8F1Hh27uD/SY2hOSXRO3Ajgf+MraXHWM7k/adgL/ZwakrhWTL0a0UlVI\n\tyxnG+OtAmywFK06OUnr0SZ3wxnb8Yy4p6tCFl0WvxF2m1NaUZcoWtseQeMt+v3mhA5\n\tKzUND1qVwp/tYTURcJWbpCpqm+z0Wb/COPja+Hm0=","Message-ID":"<5942dd86-aff5-45bd-9983-630700930412@ideasonboard.com>","Date":"Tue, 30 Jun 2026 12:47:27 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 4/4] libcamera: shaders: Replace C array with std::array","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Cc":"Bryan O'Donoghue ","References":"<20260630083031.3197714-1-laurent.pinchart@ideasonboard.com>\n\t<20260630083031.3197714-5-laurent.pinchart@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260630083031.3197714-5-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":39525,"web_url":"https://patchwork.libcamera.org/comment/39525/","msgid":"<20260630110543.GA3220219@killaraus.ideasonboard.com>","date":"2026-06-30T11:05:43","subject":"Re: [PATCH 4/4] libcamera: shaders: Replace C array with std::array","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Tue, Jun 30, 2026 at 12:47:27PM +0200, Barnabás Pőcze wrote:\n> 2026. 06. 30. 10:30 keltezéssel, Laurent Pinchart írta:\n> > Using C arrays and separate length variables is error-prone. Replace\n> > them with std::array in the generated shader header, and update the\n> > software ISP code accordingly.\n> \n> Since the array is defined in the header, I think there is not much difference\n> between an std::array and a plain array, especially since they are const.\n> But I support the removal of the separate length field.\n\nTrue, I could construct the Span from a plain C array and get the same\nbenefits. But as I don't see there's any drawback to using std::array\nhere, I'll keep it just out of laziness (I think the more positive term\nis efficiency) unless someone thinks a C array has advantages.\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/software_isp/debayer_egl.cpp | 23 +++++++---------------\n> > utils/codegen/gen-shader-header.py | 7 ++++---\n> > 2 files changed, 11 insertions(+), 19 deletions(-)\n> > \n> > diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp\n> > index af9b9d426920..1f5fc6a4466d 100644\n> > --- a/src/libcamera/software_isp/debayer_egl.cpp\n> > +++ b/src/libcamera/software_isp/debayer_egl.cpp\n> > @@ -17,6 +17,7 @@\n> > #include \n> > #include \n> > \n> > +#include \n> > #include \n> > \n> > #include \n> > @@ -145,10 +146,8 @@ int DebayerEGL::getShaderVariableLocations(void)\n> > int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputFormat)\n> > {\n> > \tstd::vector shaderEnv;\n> > -\tunsigned int fragmentShaderDataLen = 0;\n> > -\tconst unsigned char *fragmentShaderData = 0;\n> > -\tunsigned int vertexShaderDataLen = 0;\n> > -\tconst unsigned char *vertexShaderData = 0;\n> > +\tSpan fragmentShaderData;\n> > +\tSpan vertexShaderData;\n> > \tGLenum err;\n> > \n> > \t/* Target gles 100 glsl requires \"#version x\" as first directive in shader */\n> > @@ -216,9 +215,7 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm\n> > \tcase libcamera::formats::SGRBG8:\n> > \tcase libcamera::formats::SRGGB8:\n> > \t\tfragmentShaderData = bayer_unpacked_frag;\n> > -\t\tfragmentShaderDataLen = bayer_unpacked_frag_len;\n> > \t\tvertexShaderData = bayer_unpacked_vert;\n> > -\t\tvertexShaderDataLen = bayer_unpacked_vert_len;\n> > \t\tbreak;\n> > \tcase libcamera::formats::SBGGR10_CSI2P:\n> > \tcase libcamera::formats::SGBRG10_CSI2P:\n> > @@ -227,16 +224,12 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm\n> > \t\tegl_.pushEnv(shaderEnv, \"#define RAW10P\");\n> > \t\tif (BayerFormat::fromPixelFormat(inputFormat).packing == BayerFormat::Packing::None) {\n> > \t\t\tfragmentShaderData = bayer_unpacked_frag;\n> > -\t\t\tfragmentShaderDataLen = bayer_unpacked_frag_len;\n> > \t\t\tvertexShaderData = bayer_unpacked_vert;\n> > -\t\t\tvertexShaderDataLen = bayer_unpacked_vert_len;\n> > \t\t\tglFormat_ = GL_RG;\n> > \t\t\tbytesPerPixel_ = 2;\n> > \t\t} else {\n> > \t\t\tfragmentShaderData = bayer_1x_packed_frag;\n> > -\t\t\tfragmentShaderDataLen = bayer_1x_packed_frag_len;\n> > \t\t\tvertexShaderData = identity_vert;\n> > -\t\t\tvertexShaderDataLen = identity_vert_len;\n> > \t\t\tshaderStridePixels_ = width_;\n> > \t\t}\n> > \t\tbreak;\n> > @@ -247,28 +240,26 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm\n> > \t\tegl_.pushEnv(shaderEnv, \"#define RAW12P\");\n> > \t\tif (BayerFormat::fromPixelFormat(inputFormat).packing == BayerFormat::Packing::None) {\n> > \t\t\tfragmentShaderData = bayer_unpacked_frag;\n> > -\t\t\tfragmentShaderDataLen = bayer_unpacked_frag_len;\n> > \t\t\tvertexShaderData = bayer_unpacked_vert;\n> > -\t\t\tvertexShaderDataLen = bayer_unpacked_vert_len;\n> > \t\t\tglFormat_ = GL_RG;\n> > \t\t\tbytesPerPixel_ = 2;\n> > \t\t} else {\n> > \t\t\tfragmentShaderData = bayer_1x_packed_frag;\n> > -\t\t\tfragmentShaderDataLen = bayer_1x_packed_frag_len;\n> > \t\t\tvertexShaderData = identity_vert;\n> > -\t\t\tvertexShaderDataLen = identity_vert_len;\n> > \t\t\tshaderStridePixels_ = width_;\n> > \t\t}\n> > \t\tbreak;\n> > \t};\n> > \n> > -\tif (egl_.compileVertexShader(vertexShaderId_, vertexShaderData, vertexShaderDataLen, shaderEnv)) {\n> > +\tif (egl_.compileVertexShader(vertexShaderId_, vertexShaderData.data(),\n> > +\t\t\t\t vertexShaderData.size(), shaderEnv)) {\n> > \t\tLOG(Debayer, Error) << \"Compile vertex shader fail\";\n> > \t\treturn -ENODEV;\n> > \t}\n> > \tutils::scope_exit vShaderGuard([&] { glDeleteShader(vertexShaderId_); });\n> > \n> > -\tif (egl_.compileFragmentShader(fragmentShaderId_, fragmentShaderData, fragmentShaderDataLen, shaderEnv)) {\n> > +\tif (egl_.compileFragmentShader(fragmentShaderId_, fragmentShaderData.data(),\n> > +\t\t\t\t fragmentShaderData.size(), shaderEnv)) {\n> \n> I think `eGL::compileShader` et al should be modified to take `Span source`.\n\nDefinitely. I may submit a patch :-)\n\n> Reviewed-by: Barnabás Pőcze \n> \n> > \t\tLOG(Debayer, Error) << \"Compile fragment shader fail\";\n> > \t\treturn -ENODEV;\n> > \t}\n> > diff --git a/utils/codegen/gen-shader-header.py b/utils/codegen/gen-shader-header.py\n> > index 908217a2bd2e..0bcf76653d77 100755\n> > --- a/utils/codegen/gen-shader-header.py\n> > +++ b/utils/codegen/gen-shader-header.py\n> > @@ -19,15 +19,14 @@ def process_file(name, out):\n> > hex_data = [f'0x{c:02x}' for c in data]\n> > var_name = os.path.basename(name).replace('.', '_')\n> > \n> > - out.write(f'unsigned char const {var_name}[] = {{\\n')\n> > + out.write(f'static constexpr std::array {var_name}{{\\n')\n> > \n> > for i in range(math.ceil(len(data) / 16)):\n> > out.write('\\t')\n> > out.write(', '.join(hex_data[16 * i:16 * (i + 1)]))\n> > out.write(',\\n')\n> > \n> > - out.write('};\\n\\n')\n> > - out.write(f'const unsigned int {var_name}_len = {len(data)};\\n')\n> > + out.write('};\\n')\n> > \n> > \n> > def main(argv):\n> > @@ -45,6 +44,8 @@ def main(argv):\n> > \n> > #pragma once\n> > \n> > +#include \n> > +\n> > /*\n> > * List the names of the shaders at the top of header for readability's sake.\n> > *","headers":{"Return-Path":"","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 4B018C3261\n\tfor ;\n\tTue, 30 Jun 2026 11:05:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8FF0C65F6F;\n\tTue, 30 Jun 2026 13:05:46 +0200 (CEST)","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 11DAE65F55\n\tfor ;\n\tTue, 30 Jun 2026 13:05:45 +0200 (CEST)","from killaraus.ideasonboard.com\n\t(2001-14ba-70f3-e800--a06.rev.dnainternet.fi\n\t[IPv6:2001:14ba:70f3:e800::a06])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E18BD22B;\n\tTue, 30 Jun 2026 13:05:00 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"tAQmZjor\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1782817501;\n\tbh=iulHJC1sQSFLEEsxrojSTZAWUh0Ql/FyARAL0lnqw98=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=tAQmZjorBfdiMu5gJrDVEw38bxx/TemSBF8YxOC06F8IqOniiIMPuOniPoPvTdvR/\n\tNDQ4AdgR8WOjCz0sX1mwDFWUoiu3JdOH3sHDfskEqeG3YtXwpkGNt0a4FUjJVRb/FW\n\tKoCpzftRkw8IMzafmVbyx4/eKHUt0C3MIJKvNxEI=","Date":"Tue, 30 Jun 2026 14:05:43 +0300","From":"Laurent Pinchart ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org,\n\tBryan O'Donoghue ","Subject":"Re: [PATCH 4/4] libcamera: shaders: Replace C array with std::array","Message-ID":"<20260630110543.GA3220219@killaraus.ideasonboard.com>","References":"<20260630083031.3197714-1-laurent.pinchart@ideasonboard.com>\n\t<20260630083031.3197714-5-laurent.pinchart@ideasonboard.com>\n\t<5942dd86-aff5-45bd-9983-630700930412@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<5942dd86-aff5-45bd-9983-630700930412@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":39528,"web_url":"https://patchwork.libcamera.org/comment/39528/","msgid":"","date":"2026-06-30T11:44:46","subject":"Re: [PATCH 4/4] libcamera: shaders: Replace C array with std::array","submitter":{"id":175,"url":"https://patchwork.libcamera.org/api/people/175/","name":"Bryan O'Donoghue","email":"bryan.odonoghue@linaro.org"},"content":"On 30/06/2026 09:30, Laurent Pinchart wrote:\n> Using C arrays and separate length variables is error-prone. Replace\n> them with std::array in the generated shader header, and update the\n> software ISP code accordingly.\n> \n> Signed-off-by: Laurent Pinchart\n\nReviewed-by: Bryan O'Donoghue \n\n---\nbod","headers":{"Return-Path":"","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 25140C3261\n\tfor ;\n\tTue, 30 Jun 2026 11:44:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6414365F69;\n\tTue, 30 Jun 2026 13:44:50 +0200 (CEST)","from mail-wm1-x32e.google.com (mail-wm1-x32e.google.com\n\t[IPv6:2a00:1450:4864:20::32e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 083A365F55\n\tfor ;\n\tTue, 30 Jun 2026 13:44:49 +0200 (CEST)","by mail-wm1-x32e.google.com with SMTP id\n\t5b1f17b1804b1-493b1710405so10855195e9.2\n\tfor ;\n\tTue, 30 Jun 2026 04:44:49 -0700 (PDT)","from [192.168.0.101] ([109.76.103.114])\n\tby smtp.gmail.com with ESMTPSA id\n\t5b1f17b1804b1-493b8c79baasm64852845e9.7.2026.06.30.04.44.47\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tTue, 30 Jun 2026 04:44:47 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=linaro.org header.i=@linaro.org\n\theader.b=\"H0cZFR83\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=linaro.org; s=google; t=1782819888; x=1783424688;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:in-reply-to:content-language:from\n\t:references:to:subject:user-agent:mime-version:date:message-id:from\n\t:to:cc:subject:date:message-id:reply-to;\n\tbh=xNAkOUM+D51+oNuOIpUjMHojhY4Ki+86RQ2kJK6tQbI=;\n\tb=H0cZFR831wmZLAil2dC83qx9qb22PuuIBfIA8i7VOoTJjhIMa8KsOjRRnnR5PjAw2m\n\tKprkZbUrMc96LRfE7FDnwsETzp26Y5Gf2+C0QAdqeDEY2mJhj3eGVsjY3eDP58Vjls5n\n\t4NZTGZceYFOFMooymL1rsGypQMLw8j1v9Tq4+XWsHecKi5tWf4TziBxUUNgoTSgm8mHy\n\tGEzYal54tYJwKLra2ABJN5iRXAi1L6Kda2iLqe1d0qieeAltb6hJHky8Tge0TXvS5EeO\n\t36ZWnADWlJ/mtpWeGqYXTKCzgqf3+1aw7N8BuzXaQ/JL50umUSheBZBaS3Yf+kgKqjbb\n\tXhaA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1782819888; x=1783424688;\n\th=content-transfer-encoding:in-reply-to:content-language:from\n\t:references:to:subject:user-agent:mime-version:date:message-id\n\t:x-gm-gg:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=xNAkOUM+D51+oNuOIpUjMHojhY4Ki+86RQ2kJK6tQbI=;\n\tb=Zmf92Brgc40YjvIVmabsos2J88EdFc0zu94gmyiSZeH7DChfWehHNiY4BzRFmksDjq\n\tW7nyGPjO2HMA+p9FPnIo+rTIL7NCQc8jE729Ix8fg1udMk2fEPhkZ2BudyDgCxAl1P4H\n\tOWpgo84yPSkFRdk9eXpMdXgFIXRpF8PfkqiARYE6gUS/z9YNAjn1k6MWVd+O5wXks/NQ\n\t8y1DMQYeBf8B/kDQZovArng22lxs5pwJXWa1X7kfsNl8fcVEZfs5UO7sJDwOY0/m76NT\n\tQeM7NIny3BYFFaAFmuKy+/juOCX6hTm+X+YvRm2iEMxBhjWKoSdkHJSd2ajO+FKjRwdN\n\tBjYA==","X-Forwarded-Encrypted":"i=1;\n\tAFNElJ9MRG6IHwLAUJ/tSEMkapBUf+s219jJiXGq51DXiGJNJflvFYq6Hqr/otEfOGyAenTg4fyr1cmbMrywmPxpsZI=@lists.libcamera.org","X-Gm-Message-State":"AOJu0Yx2xOhmOyXWxfhPEI/lp5QSaoRLmg5BpDgGv65JR8q0iWP/sEET\n\tq0d9lzRZnVxbFCHwwzOqCJcBzI9648iE2x0i7K9rXS9LOtDh8Ctpmz9WutOQ3X+rrEw=","X-Gm-Gg":"AfdE7cmji7bomfsT3Wxn78SZNu8TcFcRTi8CKrTW8/MCFkAJBe1DO56fn7ccY28izoZ\n\tdJFe03husLIsk9jlc2874I9eMEShTbAjtprG5FhHRZxOunry9unJrXezxH+PUzAb9aTdE3UnVRZ\n\tBl8QawclhXoNoAMOP5Mfu1BAgtpfHvL2x2kt4XfzcryEukqLp59x/RrMojXRQWmfnX3a20UQwRR\n\t2ynLgv32MpznUOxD78QU1kvvGgKjCjtUSI9w3+ovcvKzMRKE4H0RGZTLMKXkC0seLg+R7AJgaHh\n\t93C3drB71xJSVqLVCOwPod86aRFOzNutoIBvKi4hblGHMprqEqIze79TA5x9iRo7itZ8p2HkGuq\n\t8TaTP4wfoU4DdVtMAqcOmSuI3ytfDCJa+6Q77vnc3Cs00L/WCyDZgOBCLcJXLTBcYZBQOjjJaD4\n\tAxSuSW4eiihcHxOqYS5KbeBycY2Q==","X-Received":"by 2002:a05:600c:3114:b0:492:45a0:dcef with SMTP id\n\t5b1f17b1804b1-493b827caeemr54489475e9.5.1782819888408; \n\tTue, 30 Jun 2026 04:44:48 -0700 (PDT)","Message-ID":"","Date":"Tue, 30 Jun 2026 12:44:46 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 4/4] libcamera: shaders: Replace C array with std::array","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260630083031.3197714-1-laurent.pinchart@ideasonboard.com>\n\t<20260630083031.3197714-5-laurent.pinchart@ideasonboard.com>","From":"Bryan O'Donoghue ","Content-Language":"en-US","In-Reply-To":"<20260630083031.3197714-5-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]