[{"id":39535,"web_url":"https://patchwork.libcamera.org/comment/39535/","msgid":"","date":"2026-06-30T12:54:06","subject":"Re: [PATCH v2 5/5] libcamera: egl: Replace pointer and length with\n\tspan for shader sources","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 13:52, Laurent Pinchart wrote:\n> The compileVertexShader() and compileFragmentShader() functions take a\n> pointer and length to the shader source code as separate arguments. This\n> is an error-prone practice. Replace them with a Span<>.\n> \n> Suggested-by: Barnabás Pőcze \n> Signed-off-by: Laurent Pinchart \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 22DF5C3303\n\tfor ;\n\tTue, 30 Jun 2026 12:54:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BFDA665F7D;\n\tTue, 30 Jun 2026 14:54:09 +0200 (CEST)","from mail-ed1-x529.google.com (mail-ed1-x529.google.com\n\t[IPv6:2a00:1450:4864:20::529])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ADE5165F79\n\tfor ;\n\tTue, 30 Jun 2026 14:54:08 +0200 (CEST)","by mail-ed1-x529.google.com with SMTP id\n\t4fb4d7f45d1cf-697cee2eb6dso4277193a12.0\n\tfor ;\n\tTue, 30 Jun 2026 05:54:08 -0700 (PDT)","from [192.168.0.101] ([109.76.103.114])\n\tby smtp.gmail.com with ESMTPSA id\n\ta640c23a62f3a-c1288d19df8sm120485266b.10.2026.06.30.05.54.07\n\t(version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128);\n\tTue, 30 Jun 2026 05:54:07 -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=\"vuf+DeK3\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=linaro.org; s=google; t=1782824048; x=1783428848;\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=zs75Jf9GsUxaJOMMALqFssCJjG/6wW8cspw4K3E3SBA=;\n\tb=vuf+DeK3gc8EZtIm3tX5SUUZBpFuMwmKeadJj9im0XXAVukWVQl2LIyLTKvJv1LsmW\n\thgc2Clov3U3QPLsrcc66SZ2VCiVYTqF82rQJF53L3WRQCwGx0hLa6d1+VGxGQtEu9TjV\n\tuatnjJaaj9PccQDHN8yx/9WDGacjyu3/D7iKx1PIAmGXK+wDOZsnezCTowmvDFe2OKBz\n\ttEVHQQECi7KdXU9ek2gKDQv/WTYlkUCl+sKpVflKh3F0Vw0LCF75/My4ybPd2gbtWfdQ\n\tmJqfVtSKXaoimBcPP4zMlUOLVWf+MPSXG11ZYxh7TBgyoyEYnl9kuQl3VMUyOV37Plg7\n\t14ZA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20251104; t=1782824048; x=1783428848;\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=zs75Jf9GsUxaJOMMALqFssCJjG/6wW8cspw4K3E3SBA=;\n\tb=gtC+1qXC6HcZ8jgCse6+n/wfvL+jceusyacLjsj2h5Z60F4cudX13s1F9NPSVE0cfS\n\tdQ5l1JUOp/iEzl9xPXZVzAB02FAaSUirXH/B72P4qDnjzu9YssjC50/ZuCv+XbW6troh\n\twTUrXO+G+TR3U5XeD6X4T1+8S2iw+Y6Rqe/RqXXnnIuO8qj+ctusWLgZAN9am/jR3YdP\n\tlbgxAKg+j8LZbFXwicwEPU/f7kWt8AS8wnonLssZ9SmdNif/mksdbezZboGlM/rcvTUe\n\tMNdLJbgr3HmND/hCdM5Js8rbEhf6mgGGTiF14Wohrp5qRvJHB69wpu9XYMufvgOdeaXZ\n\tE0dw==","X-Forwarded-Encrypted":"i=1;\n\tAFNElJ9f3I2BliBcXu5GN5O0btBuFbsd8fCdDOxGt9b9kMErSewfVgiBak2oQQm0xn1mPXlRZqww9oTP93sDLh7TCZA=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YxLBz2+2KBay7dg+DuypSnu071VWCG++ShbxoWeXShk3oioPScY\n\tvG5TGi4FJ/PRzCuC3G2lcqOALN6Cp0T6XsY6XYwF3vlb9Fakoo5IzJZoZnpwBQOPuN4=","X-Gm-Gg":"AfdE7cklKpJ/+tImIjERnxsYKxWRF4jbQUgOxTvhBcv7eCh027cYYEucMjkhWDsni8T\n\t6sfeHQBo7hMQxupwjBRAmFY6J0yFATMe1UeRhufKqESDerIw8t2AJjc6Ze4jbpwpzPsIyzEbKzb\n\tlFWISGJvmvQjh7rHRBuTtZtZ/qsc6rfXZyvXPeU5tViBH5yyV6T0HVvaJ5FOiYDNt7NpqbOKSLF\n\tCFXUhy/kQw95fBwphAtRA2pbHyPauMb0JzAUsxS4e6q1P79i3XhhfhNOsDkmZlWAcc2cW0tGj+J\n\tAogXlC/fh6/I/f3Tiz21jd6LzVUY+oyz59O1TdVV7y/J4SmP6VKW7C/CHjeev77wQgbnKe0LhAj\n\tRAFrcZgmjtSfy2/hG+FMRQDOhlIGjiTC4MScqSl2TY2c1oZHeJ3exKs4roGNf5ykaH1DkdLGcRt\n\ty7YFzsB+hfKvYitFQJCTVdPmM0Kg==","X-Received":"by 2002:a17:907:9607:b0:c12:5bf7:4ac2 with SMTP id\n\ta640c23a62f3a-c128702681fmr166101266b.8.1782824047968; \n\tTue, 30 Jun 2026 05:54:07 -0700 (PDT)","Message-ID":"","Date":"Tue, 30 Jun 2026 13:54:06 +0100","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 5/5] libcamera: egl: Replace pointer and length with\n\tspan for shader sources","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20260630125214.3327516-1-laurent.pinchart@ideasonboard.com>\n\t<20260630125214.3327516-6-laurent.pinchart@ideasonboard.com>","From":"Bryan O'Donoghue ","Content-Language":"en-US","In-Reply-To":"<20260630125214.3327516-6-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":39536,"web_url":"https://patchwork.libcamera.org/comment/39536/","msgid":"<44e85241-bce9-4496-9e41-8811155e3678@ideasonboard.com>","date":"2026-06-30T13:02:58","subject":"Re: [PATCH v2 5/5] libcamera: egl: Replace pointer and length with\n\tspan for shader sources","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. 14:52 keltezéssel, Laurent Pinchart írta:\n> The compileVertexShader() and compileFragmentShader() functions take a\n> pointer and length to the shader source code as separate arguments. This\n> is an error-prone practice. Replace them with a Span<>.\n> \n> Suggested-by: Barnabás Pőcze \n> Signed-off-by: Laurent Pinchart \n> ---\n\nLooks ok to me.\n\nReviewed-by: Barnabás Pőcze \n\n\n> include/libcamera/internal/egl.h | 9 +++----\n> src/libcamera/egl.cpp | 29 ++++++++++------------\n> src/libcamera/software_isp/debayer_egl.cpp | 8 +++---\n> 3 files changed, 20 insertions(+), 26 deletions(-)\n> \n> diff --git a/include/libcamera/internal/egl.h b/include/libcamera/internal/egl.h\n> index f7bfb28d4e9a..523a62aa89d5 100644\n> --- a/include/libcamera/internal/egl.h\n> +++ b/include/libcamera/internal/egl.h\n> @@ -112,11 +112,9 @@ public:\n> \tvoid pushEnv(std::vector &shaderEnv, const char *str);\n> \tvoid makeCurrent();\n> \n> -\tint compileVertexShader(GLuint &shaderId, const unsigned char *shaderData,\n> -\t\t\t\tunsigned int shaderDataLen,\n> +\tint compileVertexShader(GLuint &shaderId, Span shaderData,\n> \t\t\t\tSpan shaderEnv);\n> -\tint compileFragmentShader(GLuint &shaderId, const unsigned char *shaderData,\n> -\t\t\t\t unsigned int shaderDataLen,\n> +\tint compileFragmentShader(GLuint &shaderId, Span shaderData,\n> \t\t\t\t Span shaderEnv);\n> \tint linkProgram(GLuint &programId, GLuint fragmentshaderId, GLuint vertexshaderId);\n> \tvoid dumpShaderSource(GLuint shaderId);\n> @@ -135,8 +133,7 @@ private:\n> \tEGLSurface surface_ = EGL_NO_SURFACE;\n> \n> \tstatic EGLDisplay probeDisplay();\n> -\tint compileShader(int shaderType, GLuint &shaderId, const unsigned char *shaderData,\n> -\t\t\t unsigned int shaderDataLen,\n> +\tint compileShader(int shaderType, GLuint &shaderId, Span shaderData,\n> \t\t\t Span shaderEnv);\n> \n> \tint createDMABufTexture2D(eGLImage &eglImage, int fd, bool output);\n> diff --git a/src/libcamera/egl.cpp b/src/libcamera/egl.cpp\n> index f03abb8ae07d..b5a5dc60b110 100644\n> --- a/src/libcamera/egl.cpp\n> +++ b/src/libcamera/egl.cpp\n> @@ -463,8 +463,7 @@ void eGL::pushEnv(std::vector &shaderEnv, const char *str)\n> /**\n> * \\brief Compile a vertex shader\n> * \\param[out] shaderId OpenGL shader object ID\n> - * \\param[in] shaderData Pointer to shader source code\n> - * \\param[in] shaderDataLen Length of shader source in bytes\n> + * \\param[in] shaderData Shader source code\n> * \\param[in] shaderEnv Span of preprocessor definitions to prepend\n> *\n> * Compiles a vertex shader from source code with optional preprocessor\n> @@ -472,18 +471,17 @@ void eGL::pushEnv(std::vector &shaderEnv, const char *str)\n> *\n> * \\return 0 on success, or -EINVAL on compilation failure\n> */\n> -int eGL::compileVertexShader(GLuint &shaderId, const unsigned char *shaderData,\n> -\t\t\t unsigned int shaderDataLen,\n> +int eGL::compileVertexShader(GLuint &shaderId,\n> +\t\t\t Span shaderData,\n> \t\t\t Span shaderEnv)\n> {\n> -\treturn compileShader(GL_VERTEX_SHADER, shaderId, shaderData, shaderDataLen, shaderEnv);\n> +\treturn compileShader(GL_VERTEX_SHADER, shaderId, shaderData, shaderEnv);\n> }\n> \n> /**\n> * \\brief Compile a fragment shader\n> * \\param[out] shaderId OpenGL shader object ID\n> - * \\param[in] shaderData Pointer to shader source code\n> - * \\param[in] shaderDataLen Length of shader source in bytes\n> + * \\param[in] shaderData Shader source code\n> * \\param[in] shaderEnv Span of preprocessor definitions to prepend\n> *\n> * Compiles a fragment shader from source code with optional preprocessor\n> @@ -491,19 +489,18 @@ int eGL::compileVertexShader(GLuint &shaderId, const unsigned char *shaderData,\n> *\n> * \\return 0 on success, or -EINVAL on compilation failure\n> */\n> -int eGL::compileFragmentShader(GLuint &shaderId, const unsigned char *shaderData,\n> -\t\t\t unsigned int shaderDataLen,\n> +int eGL::compileFragmentShader(GLuint &shaderId,\n> +\t\t\t Span shaderData,\n> \t\t\t Span shaderEnv)\n> {\n> -\treturn compileShader(GL_FRAGMENT_SHADER, shaderId, shaderData, shaderDataLen, shaderEnv);\n> +\treturn compileShader(GL_FRAGMENT_SHADER, shaderId, shaderData, shaderEnv);\n> }\n> \n> /**\n> * \\brief Compile a shader of specified type\n> * \\param[in] shaderType GL_VERTEX_SHADER or GL_FRAGMENT_SHADER\n> * \\param[out] shaderId OpenGL shader object ID\n> - * \\param[in] shaderData Pointer to shader source code\n> - * \\param[in] shaderDataLen Length of shader source in bytes\n> + * \\param[in] shaderData Shader source code\n> * \\param[in] shaderEnv Span of preprocessor definitions to prepend\n> *\n> * Internal helper function for shader compilation. Prepends environment\n> @@ -511,8 +508,8 @@ int eGL::compileFragmentShader(GLuint &shaderId, const unsigned char *shaderData\n> *\n> * \\return 0 on success, or -EINVAL on compilation failure\n> */\n> -int eGL::compileShader(int shaderType, GLuint &shaderId, const unsigned char *shaderData,\n> -\t\t unsigned int shaderDataLen,\n> +int eGL::compileShader(int shaderType, GLuint &shaderId,\n> +\t\t Span shaderData,\n> \t\t Span shaderEnv)\n> {\n> \tGLint success;\n> @@ -531,8 +528,8 @@ int eGL::compileShader(int shaderType, GLuint &shaderId, const unsigned char *sh\n> \t}\n> \n> \t// Now the main body of the shader program\n> -\tshaderSourceData[i] = reinterpret_cast(shaderData);\n> -\tshaderDataLengths[i] = shaderDataLen;\n> +\tshaderSourceData[i] = reinterpret_cast(shaderData.data());\n> +\tshaderDataLengths[i] = shaderData.size();\n> \n> \t// And create the shader\n> \tshaderId = glCreateShader(shaderType);\n> diff --git a/src/libcamera/software_isp/debayer_egl.cpp b/src/libcamera/software_isp/debayer_egl.cpp\n> index 1f5fc6a4466d..56545816ead5 100644\n> --- a/src/libcamera/software_isp/debayer_egl.cpp\n> +++ b/src/libcamera/software_isp/debayer_egl.cpp\n> @@ -251,15 +251,15 @@ int DebayerEGL::initBayerShaders(PixelFormat inputFormat, PixelFormat outputForm\n> \t\tbreak;\n> \t};\n> \n> -\tif (egl_.compileVertexShader(vertexShaderId_, vertexShaderData.data(),\n> -\t\t\t\t vertexShaderData.size(), shaderEnv)) {\n> +\tif (egl_.compileVertexShader(vertexShaderId_, vertexShaderData,\n> +\t\t\t\t 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.data(),\n> -\t\t\t\t fragmentShaderData.size(), shaderEnv)) {\n> +\tif (egl_.compileFragmentShader(fragmentShaderId_, fragmentShaderData,\n> +\t\t\t\t shaderEnv)) {\n> \t\tLOG(Debayer, Error) << \"Compile fragment shader fail\";\n> \t\treturn -ENODEV;\n> \t}","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 87DE2C3261\n\tfor ;\n\tTue, 30 Jun 2026 13:03:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A6C3765F7C;\n\tTue, 30 Jun 2026 15:03:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D078F65F79\n\tfor ;\n\tTue, 30 Jun 2026 15:03:01 +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 BBC571F1E;\n\tTue, 30 Jun 2026 15:02:17 +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=\"aLrkffnE\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1782824537;\n\tbh=KXC8hBmRqE80kM9ESn3f+ImTEnSmkT+saQamGhJ1too=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=aLrkffnE/FO/ij3uqXnLSumUzsNdhMZRN6r9SJnaLDRbTm5K/DTf+HloGmA9fhCU9\n\tHHATolcqpWiv4yixftSZAZGTHRA0NRY8HfeZXgCNLp5yO79sdF1981/a6i0DC9dNsN\n\thXrCRjG3RrsfL9PfBsCbgBjHdEAF+CNK39p6QdoI=","Message-ID":"<44e85241-bce9-4496-9e41-8811155e3678@ideasonboard.com>","Date":"Tue, 30 Jun 2026 15:02:58 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2 5/5] libcamera: egl: Replace pointer and length with\n\tspan for shader sources","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Cc":"Bryan O'Donoghue ","References":"<20260630125214.3327516-1-laurent.pinchart@ideasonboard.com>\n\t<20260630125214.3327516-6-laurent.pinchart@ideasonboard.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20260630125214.3327516-6-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\" "}}]