[{"id":34554,"web_url":"https://patchwork.libcamera.org/comment/34554/","msgid":"","date":"2025-06-19T09:57:09","subject":"Re: [PATCH v2] EncoderLibJpeg: Migrate const global pixelInfo to\n\tstatic","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 06. 19. 10:20 keltezéssel, Harvey Yang írta:\n> From: Harvey Yang \n> \n> Previously, const global `pixelInfo` in `encoder_libjpeg.cpp` is\n> initialized before const global `pixelFormatInfo` in `formats.cpp`,\n> even if `formats.cpp` is wrapped in the base libcamera shared library,\n> and the android adapter (which contains `encoder_libjpeg.cpp`)\n> depends on the base libcamera shared library.\n\nI generally agree with any removal of non-constinit or mutable global state.\n\nSo were you getting `Unsupported pixel format ...` warnings previously,\nor a crash or something else?\n\n\n> \n> The reason should be that C++ initialization order doesn't follow the\n> linking process.\n\nI would have assumed that any reasonable linker will bring up the dependencies\nbefore the dependents, but I suppose it is not impossible that the ordering\nis different for some reason.\n\n\nRegards,\nBarnabás Pőcze\n\n> \n> This patch migrates const global pixelInfo to static to avoid the race\n> condition.\n> \n> Signed-off-by: Harvey Yang \n> ---\n> src/android/jpeg/encoder_libjpeg.cpp | 28 ++++++++++++++--------------\n> 1 file changed, 14 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> index cb242b5ec6a8..3ba5103d5ad5 100644\n> --- a/src/android/jpeg/encoder_libjpeg.cpp\n> +++ b/src/android/jpeg/encoder_libjpeg.cpp\n> @@ -38,26 +38,26 @@ struct JPEGPixelFormatInfo {\n> \tbool nvSwap;\n> };\n> \n> -const std::map pixelInfo{\n> -\t{ formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } },\n> -\n> -\t{ formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } },\n> -\t{ formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } },\n> -\n> -\t{ formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } },\n> -\t{ formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } },\n> -\t{ formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } },\n> -\t{ formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } },\n> -\t{ formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } },\n> -\t{ formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } },\n> -};\n> -\n> const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format)\n> {\n> \tstatic const struct JPEGPixelFormatInfo invalidPixelFormat {\n> \t\tJCS_UNKNOWN, PixelFormatInfo(), false\n> \t};\n> \n> +\tstatic const std::map pixelInfo{\n> +\t\t{ formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } },\n> +\n> +\t\t{ formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } },\n> +\t\t{ formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } },\n> +\n> +\t\t{ formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } },\n> +\t\t{ formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } },\n> +\t\t{ formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } },\n> +\t\t{ formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } },\n> +\t\t{ formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } },\n> +\t\t{ formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } },\n> +\t};\n> +\n> \tconst auto iter = pixelInfo.find(format);\n> \tif (iter == pixelInfo.end()) {\n> \t\tLOG(JPEG, Error) << \"Unsupported pixel format for JPEG encoder: \"","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 5FB71C3237\n\tfor ;\n\tThu, 19 Jun 2025 09:57:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 884E768DE0;\n\tThu, 19 Jun 2025 11:57:14 +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 CDCCF614E1\n\tfor ;\n\tThu, 19 Jun 2025 11:57:12 +0200 (CEST)","from [192.168.33.22] (185.221.143.107.nat.pool.zt.hu\n\t[185.221.143.107])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F01C443;\n\tThu, 19 Jun 2025 11:56:58 +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=\"qYqR0o0s\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750327019;\n\tbh=q/Qs8wKvOuuPrdOHMhUYdfdPbdET+BecV5PjSD5qIkk=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=qYqR0o0s9X15XIO499N97CjVMeuWbL3bwhzi5XeueEgjkRrtans71czBDUHWIoqr9\n\tZ7l67I69zCyf6mrrJqw6hKYfPgj52AL7tdCfHoDE0hiQ5S6JXX2ZLtlUZ8xLoI6XnB\n\tm/c2V5svU9AiAfG7v5sWA3esNSSqD9NCsPebsslY=","Message-ID":"","Date":"Thu, 19 Jun 2025 11:57:09 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2] EncoderLibJpeg: Migrate const global pixelInfo to\n\tstatic","To":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20250619082027.4058507-1-chenghaoyang@google.com>","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"<20250619082027.4058507-1-chenghaoyang@google.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":34556,"web_url":"https://patchwork.libcamera.org/comment/34556/","msgid":"","date":"2025-06-19T10:02:33","subject":"Re: [PATCH v2] EncoderLibJpeg: Migrate const global pixelInfo to\n\tstatic","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Barnabás,\n\nOn Thu, Jun 19, 2025 at 5:57 PM Barnabás Pőcze\n wrote:\n>\n> Hi\n>\n> 2025. 06. 19. 10:20 keltezéssel, Harvey Yang írta:\n> > From: Harvey Yang \n> >\n> > Previously, const global `pixelInfo` in `encoder_libjpeg.cpp` is\n> > initialized before const global `pixelFormatInfo` in `formats.cpp`,\n> > even if `formats.cpp` is wrapped in the base libcamera shared library,\n> > and the android adapter (which contains `encoder_libjpeg.cpp`)\n> > depends on the base libcamera shared library.\n>\n> I generally agree with any removal of non-constinit or mutable global state.\n>\n> So were you getting `Unsupported pixel format ...` warnings previously,\n> or a crash or something else?\n\nYes, I got `Unsupported pixel format ...` warnings, and after adding logs,\nI found `pixelFormatInfo` has size 0 when being queried.\n\n>\n>\n> >\n> > The reason should be that C++ initialization order doesn't follow the\n> > linking process.\n>\n> I would have assumed that any reasonable linker will bring up the dependencies\n> before the dependents, but I suppose it is not impossible that the ordering\n> is different for some reason.\n\nThe testing environment is Android OS, FYI.\n\nBR,\nHarvey\n\n>\n>\n> Regards,\n> Barnabás Pőcze\n>\n> >\n> > This patch migrates const global pixelInfo to static to avoid the race\n> > condition.\n> >\n> > Signed-off-by: Harvey Yang \n> > ---\n> > src/android/jpeg/encoder_libjpeg.cpp | 28 ++++++++++++++--------------\n> > 1 file changed, 14 insertions(+), 14 deletions(-)\n> >\n> > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> > index cb242b5ec6a8..3ba5103d5ad5 100644\n> > --- a/src/android/jpeg/encoder_libjpeg.cpp\n> > +++ b/src/android/jpeg/encoder_libjpeg.cpp\n> > @@ -38,26 +38,26 @@ struct JPEGPixelFormatInfo {\n> > bool nvSwap;\n> > };\n> >\n> > -const std::map pixelInfo{\n> > - { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } },\n> > -\n> > - { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } },\n> > - { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } },\n> > -\n> > - { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } },\n> > - { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } },\n> > - { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } },\n> > - { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } },\n> > - { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } },\n> > - { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } },\n> > -};\n> > -\n> > const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format)\n> > {\n> > static const struct JPEGPixelFormatInfo invalidPixelFormat {\n> > JCS_UNKNOWN, PixelFormatInfo(), false\n> > };\n> >\n> > + static const std::map pixelInfo{\n> > + { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } },\n> > +\n> > + { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } },\n> > + { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } },\n> > +\n> > + { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } },\n> > + { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } },\n> > + { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } },\n> > + { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } },\n> > + { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } },\n> > + { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } },\n> > + };\n> > +\n> > const auto iter = pixelInfo.find(format);\n> > if (iter == pixelInfo.end()) {\n> > LOG(JPEG, Error) << \"Unsupported pixel format for JPEG encoder: \"\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 6EF09BDE6B\n\tfor ;\n\tThu, 19 Jun 2025 10:02:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 338B368DDD;\n\tThu, 19 Jun 2025 12:02:48 +0200 (CEST)","from mail-lj1-x235.google.com (mail-lj1-x235.google.com\n\t[IPv6:2a00:1450:4864:20::235])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B293F61538\n\tfor ;\n\tThu, 19 Jun 2025 12:02:46 +0200 (CEST)","by mail-lj1-x235.google.com with SMTP id\n\t38308e7fff4ca-32b435ef653so5273941fa.2\n\tfor ;\n\tThu, 19 Jun 2025 03:02:46 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"EfQjFRnA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1750327366; x=1750932166;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=W3ZCzXWgZ0bxFQFpoSzU8/XPtD6oqHlxvl9SUVxot2s=;\n\tb=EfQjFRnA166A85eibkj/DgMh8+TKb+EtVpqDqppZj0fvPWUhMRtl/hOUWB8170zCcE\n\tLrZZ6Xbxewxlb9l60/rUdBGy8GL3fLAR17HeMH45cS96SMy2Lly6OXgGDDqURXiPFTAD\n\tABg6ItaANQMV/Pv4m6RfuKXqTirjMas8S/oa8=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1750327366; x=1750932166;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=W3ZCzXWgZ0bxFQFpoSzU8/XPtD6oqHlxvl9SUVxot2s=;\n\tb=ebzi0C4qh0S8aa0kQ1kkqj3Q2cchTI2Au+oZ5V78WYAAXvih8DAri7J7fkP0CxmI90\n\tGjStPiTgd/r73A1+5NKEYz/SVZYNe5W1QICFrKdD4tVsM1H4yTEruqBpI9sedSi+5J9C\n\t6DncO/6jdO66ROVnjfL1++wMQAwI4uMFYOur3z8gPkVexL9b+Of3X9ru+sqAq4frzawd\n\tQF4vjVP4OBQBnX1k2pO4qrsCZW67dr2f9/E8VeBQe11i5T+Mfy2tXc7cU1TZkwy0wTIM\n\taYpkuAYnD0jSwPOjS/zCfMkgEX14janLbVlzbb01o/BON3bYaHuXdKY9a6Nll9rgd57d\n\t7YEA==","X-Gm-Message-State":"AOJu0YzqzZ9hKwWJ3zJoZXAIGN1h1JS6Muz0Jy4WMiY9eYh6P/zq21I6\n\trtLYXPNLe582ncNkmJOs74v8Zpilq3VRu/6zh+fkYhyLH7eKIs7bf5KVUaS+GG0v3r+yZXGbWEo\n\tYicQuAKdi0pZ6dtKVlYMjyzOXrQbJ6iU3Pe63rMty","X-Gm-Gg":"ASbGncsmHaPBHv29xRA4k7i1kythTGcCqoksQ0dv6VYg155+cKQ9bFZoJXxqT68vSyy\n\tO+w6cHB7kh1sES0TstmMATLt//i/+9Gz4O4/eohq5aB3jMqfbnSBSx8Tp29kkVLaVtdrYwLaZRh\n\tUZouXlzKogXawwtznUHQ/F3nJ098pxYE3xmVsAgOFE7TL7HsilkN9BNCWdPMLhtjhUcuWLVw==","X-Google-Smtp-Source":"AGHT+IGS9XrdilYok3BAI00hlTH7Iy+09NUwoLjikKl5EmirXCmyDBLb6DXX2oS1KwD5+iP7j1zTJp82wBAQpOZe1rQ=","X-Received":"by 2002:a05:651c:2118:b0:32b:7413:50f with SMTP id\n\t38308e7fff4ca-32b74130864mr29783641fa.39.1750327364829;\n\tThu, 19 Jun 2025 03:02:44 -0700 (PDT)","MIME-Version":"1.0","References":"<20250619082027.4058507-1-chenghaoyang@google.com>\n\t","In-Reply-To":"","From":"Cheng-Hao Yang ","Date":"Thu, 19 Jun 2025 18:02:33 +0800","X-Gm-Features":"AX0GCFueuWx3lKjZeyoAF5tfMVdVt7hibngwHo-gxUSegyiW_qGaWBUtOs-iD9M","Message-ID":"","Subject":"Re: [PATCH v2] EncoderLibJpeg: Migrate const global pixelInfo to\n\tstatic","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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":34557,"web_url":"https://patchwork.libcamera.org/comment/34557/","msgid":"<20250619103004.GL22102@pendragon.ideasonboard.com>","date":"2025-06-19T10:30:04","subject":"Re: [PATCH v2] EncoderLibJpeg: Migrate const global pixelInfo to\n\tstatic","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Jun 19, 2025 at 06:02:33PM +0800, Cheng-Hao Yang wrote:\n> On Thu, Jun 19, 2025 at 5:57 PM Barnabás Pőcze wrote:\n> > 2025. 06. 19. 10:20 keltezéssel, Harvey Yang írta:\n> > > From: Harvey Yang \n> > >\n> > > Previously, const global `pixelInfo` in `encoder_libjpeg.cpp` is\n> > > initialized before const global `pixelFormatInfo` in `formats.cpp`,\n> > > even if `formats.cpp` is wrapped in the base libcamera shared library,\n> > > and the android adapter (which contains `encoder_libjpeg.cpp`)\n> > > depends on the base libcamera shared library.\n> >\n> > I generally agree with any removal of non-constinit or mutable global state.\n> >\n> > So were you getting `Unsupported pixel format ...` warnings previously,\n> > or a crash or something else?\n> \n> Yes, I got `Unsupported pixel format ...` warnings, and after adding logs,\n> I found `pixelFormatInfo` has size 0 when being queried.\n\nThere's something I don't quite get.\nsrc/android/jpeg/encoder_libjpeg.cpp is compiled in libcamera-hal.so,\nwhich links against libcamera.so. The latter provides\nPixelFormatInfo::info(). Does this mean that the global variables of\nlibcamera-hal.so get initialized before those of libcamera.so ? Is that\nallowed ? More investigation seems to be needed.\n\n> > > The reason should be that C++ initialization order doesn't follow the\n> > > linking process.\n> >\n> > I would have assumed that any reasonable linker will bring up the dependencies\n> > before the dependents, but I suppose it is not impossible that the ordering\n> > is different for some reason.\n> \n> The testing environment is Android OS, FYI.\n\nHow to you compile libcamera on Android ?\n\n> > > This patch migrates const global pixelInfo to static to avoid the race\n> > > condition.\n> > >\n> > > Signed-off-by: Harvey Yang \n> > > ---\n> > > src/android/jpeg/encoder_libjpeg.cpp | 28 ++++++++++++++--------------\n> > > 1 file changed, 14 insertions(+), 14 deletions(-)\n> > >\n> > > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> > > index cb242b5ec6a8..3ba5103d5ad5 100644\n> > > --- a/src/android/jpeg/encoder_libjpeg.cpp\n> > > +++ b/src/android/jpeg/encoder_libjpeg.cpp\n> > > @@ -38,26 +38,26 @@ struct JPEGPixelFormatInfo {\n> > > bool nvSwap;\n> > > };\n> > >\n> > > -const std::map pixelInfo{\n> > > - { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } },\n> > > -\n> > > - { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } },\n> > > - { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } },\n> > > -\n> > > - { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } },\n> > > - { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } },\n> > > - { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } },\n> > > - { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } },\n> > > - { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } },\n> > > - { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } },\n> > > -};\n> > > -\n> > > const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format)\n> > > {\n> > > static const struct JPEGPixelFormatInfo invalidPixelFormat {\n> > > JCS_UNKNOWN, PixelFormatInfo(), false\n> > > };\n> > >\n> > > + static const std::map pixelInfo{\n> > > + { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } },\n> > > +\n> > > + { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } },\n> > > + { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } },\n> > > +\n> > > + { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } },\n> > > + { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } },\n> > > + { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } },\n> > > + { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } },\n> > > + { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } },\n> > > + { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } },\n> > > + };\n> > > +\n> > > const auto iter = pixelInfo.find(format);\n> > > if (iter == pixelInfo.end()) {\n> > > LOG(JPEG, Error) << \"Unsupported pixel format for JPEG encoder: \"","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 F3185C3237\n\tfor ;\n\tThu, 19 Jun 2025 10:30:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 734F268DE9;\n\tThu, 19 Jun 2025 12:30:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9237A68DDB\n\tfor ;\n\tThu, 19 Jun 2025 12:30:22 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7B68C2EC;\n\tThu, 19 Jun 2025 12:30:08 +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=\"Lm0On1II\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750329008;\n\tbh=Q48xrgzVRJTmIJP2pcizk+HK4dhR42a5FLi/H9bvBxM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Lm0On1III9u7YBMuTFVBJnU7lmQDYi4Rj2gHZn2JdtWMP8HGjJh9qPmiGpVTTDoob\n\t1TS6gLgdGc7AcrgnsAuMQRmMnEqqRvPeLPq+XVsQaw3xfHs/mBv7ePH7poBZ3X6zjO\n\t59/VTXEqZw+n8EbrEFdms0qLN/QyJuUDDTm32vbU=","Date":"Thu, 19 Jun 2025 13:30:04 +0300","From":"Laurent Pinchart ","To":"Cheng-Hao Yang ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v2] EncoderLibJpeg: Migrate const global pixelInfo to\n\tstatic","Message-ID":"<20250619103004.GL22102@pendragon.ideasonboard.com>","References":"<20250619082027.4058507-1-chenghaoyang@google.com>\n\t\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","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":34584,"web_url":"https://patchwork.libcamera.org/comment/34584/","msgid":"","date":"2025-06-20T04:33:34","subject":"Re: [PATCH v2] EncoderLibJpeg: Migrate const global pixelInfo to\n\tstatic","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Laurent,\n\nOn Thu, Jun 19, 2025 at 6:30 PM Laurent Pinchart\n wrote:\n>\n> On Thu, Jun 19, 2025 at 06:02:33PM +0800, Cheng-Hao Yang wrote:\n> > On Thu, Jun 19, 2025 at 5:57 PM Barnabás Pőcze wrote:\n> > > 2025. 06. 19. 10:20 keltezéssel, Harvey Yang írta:\n> > > > From: Harvey Yang \n> > > >\n> > > > Previously, const global `pixelInfo` in `encoder_libjpeg.cpp` is\n> > > > initialized before const global `pixelFormatInfo` in `formats.cpp`,\n> > > > even if `formats.cpp` is wrapped in the base libcamera shared library,\n> > > > and the android adapter (which contains `encoder_libjpeg.cpp`)\n> > > > depends on the base libcamera shared library.\n> > >\n> > > I generally agree with any removal of non-constinit or mutable global state.\n> > >\n> > > So were you getting `Unsupported pixel format ...` warnings previously,\n> > > or a crash or something else?\n> >\n> > Yes, I got `Unsupported pixel format ...` warnings, and after adding logs,\n> > I found `pixelFormatInfo` has size 0 when being queried.\n>\n> There's something I don't quite get.\n> src/android/jpeg/encoder_libjpeg.cpp is compiled in libcamera-hal.so,\n> which links against libcamera.so. The latter provides\n> PixelFormatInfo::info(). Does this mean that the global variables of\n> libcamera-hal.so get initialized before those of libcamera.so ? Is that\n> allowed ? More investigation seems to be needed.\n\nI had the same question, while the experiments showed the other way\naround. There are also similar discussions like:\nhttps://stackoverflow.com/questions/3746238/c-global-initialization-order-ignores-dependencies\n\n\n>\n> > > > The reason should be that C++ initialization order doesn't follow the\n> > > > linking process.\n> > >\n> > > I would have assumed that any reasonable linker will bring up the dependencies\n> > > before the dependents, but I suppose it is not impossible that the ordering\n> > > is different for some reason.\n> >\n> > The testing environment is Android OS, FYI.\n>\n> How to you compile libcamera on Android ?\n\nCurrently we have to rewrite with Android.bp, instead of using the available\n`meson.build`s, while as I mentioned before, I built `libcamera_src_defaults`,\nwhich contains `src/libcamera/formats.cpp`. And built\n`libcamera_android_src_defaults` (depending on `libcamera_src_defaults`)\n, which contains `src/android/jpeg/encoder_libjpeg.cpp`.\n\nBR,\nHarvey\n\n>\n> > > > This patch migrates const global pixelInfo to static to avoid the race\n> > > > condition.\n> > > >\n> > > > Signed-off-by: Harvey Yang \n> > > > ---\n> > > > src/android/jpeg/encoder_libjpeg.cpp | 28 ++++++++++++++--------------\n> > > > 1 file changed, 14 insertions(+), 14 deletions(-)\n> > > >\n> > > > diff --git a/src/android/jpeg/encoder_libjpeg.cpp b/src/android/jpeg/encoder_libjpeg.cpp\n> > > > index cb242b5ec6a8..3ba5103d5ad5 100644\n> > > > --- a/src/android/jpeg/encoder_libjpeg.cpp\n> > > > +++ b/src/android/jpeg/encoder_libjpeg.cpp\n> > > > @@ -38,26 +38,26 @@ struct JPEGPixelFormatInfo {\n> > > > bool nvSwap;\n> > > > };\n> > > >\n> > > > -const std::map pixelInfo{\n> > > > - { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } },\n> > > > -\n> > > > - { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } },\n> > > > - { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } },\n> > > > -\n> > > > - { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } },\n> > > > - { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } },\n> > > > - { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } },\n> > > > - { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } },\n> > > > - { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } },\n> > > > - { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } },\n> > > > -};\n> > > > -\n> > > > const struct JPEGPixelFormatInfo &findPixelInfo(const PixelFormat &format)\n> > > > {\n> > > > static const struct JPEGPixelFormatInfo invalidPixelFormat {\n> > > > JCS_UNKNOWN, PixelFormatInfo(), false\n> > > > };\n> > > >\n> > > > + static const std::map pixelInfo{\n> > > > + { formats::R8, { JCS_GRAYSCALE, PixelFormatInfo::info(formats::R8), false } },\n> > > > +\n> > > > + { formats::RGB888, { JCS_EXT_BGR, PixelFormatInfo::info(formats::RGB888), false } },\n> > > > + { formats::BGR888, { JCS_EXT_RGB, PixelFormatInfo::info(formats::BGR888), false } },\n> > > > +\n> > > > + { formats::NV12, { JCS_YCbCr, PixelFormatInfo::info(formats::NV12), false } },\n> > > > + { formats::NV21, { JCS_YCbCr, PixelFormatInfo::info(formats::NV21), true } },\n> > > > + { formats::NV16, { JCS_YCbCr, PixelFormatInfo::info(formats::NV16), false } },\n> > > > + { formats::NV61, { JCS_YCbCr, PixelFormatInfo::info(formats::NV61), true } },\n> > > > + { formats::NV24, { JCS_YCbCr, PixelFormatInfo::info(formats::NV24), false } },\n> > > > + { formats::NV42, { JCS_YCbCr, PixelFormatInfo::info(formats::NV42), true } },\n> > > > + };\n> > > > +\n> > > > const auto iter = pixelInfo.find(format);\n> > > > if (iter == pixelInfo.end()) {\n> > > > LOG(JPEG, Error) << \"Unsupported pixel format for JPEG encoder: \"\n>\n> --\n> Regards,\n>\n> Laurent Pinchart","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 C6287BDE6B\n\tfor ;\n\tFri, 20 Jun 2025 04:33:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B348968DE1;\n\tFri, 20 Jun 2025 06:33:47 +0200 (CEST)","from mail-lj1-x234.google.com (mail-lj1-x234.google.com\n\t[IPv6:2a00:1450:4864:20::234])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2A40761534\n\tfor ;\n\tFri, 20 Jun 2025 06:33:46 +0200 (CEST)","by mail-lj1-x234.google.com with SMTP id\n\t38308e7fff4ca-32b910593edso8703621fa.1\n\tfor ;\n\tThu, 19 Jun 2025 21:33:46 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"Fb+ZMH8R\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1750394025; x=1750998825;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=lcfHJw4o/6GEKZhlu9jO+60O4yRH3yG38IkQc9TMb4c=;\n\tb=Fb+ZMH8R6IAWgrcRDXekAiCdRDzy+LTN1AaFpBKXFI8L0vU+x2FtPYfJWkCiNVX0zY\n\txPdSzh2+7FVf9RMFis4GScLZaWvawBt4SuSwXR18yUQTLuNK24HDojv1CYTohUdBUbvt\n\tfKNtJE5EDRivfW4+be/hxKmcirjoqqE79gUKk=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1750394025; x=1750998825;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=lcfHJw4o/6GEKZhlu9jO+60O4yRH3yG38IkQc9TMb4c=;\n\tb=ejJiuvqz5cr/W7J/1eFlh7/Avp4Zy3+VBffyvisqok4t3H/YyFSLGrFp8JaiaUcRjB\n\tL1P1IW3+WFJp5/B7/r5Bi8lFrsGuQm8IgaUzahGqYVpIqqwpz7G8bge72nDFnltZXcTT\n\t0qpOIt6qJkxr11/HTEDIOI35cqwB0HDWoKoQFyZD3VPj6VMOA6b958aZhUx+48uoVcc8\n\tE0bkWswWQGVkL2ES1LRowlOOVSwKr+jnzcaSYH4J0ytKuLjtURJFXPK+AXcUOUM20hGZ\n\tnSHZMEUANxoBHdVzm4v/rXgjoB3Y4ZCnan1g/fkQia8OYvvvzYpqYNkngApJCpYFZlt8\n\tf1EA==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCUEqaBxMIbrtCSlgNbom4uZx5S355nVNVvvXPvCvTWl2YneABxMGCETqI0E7AyQJxl8YSGxLq9GauKGjpirsW8=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YzbmDy4roRoe5IV56j0JMAR3M4RGXkiAyqOVnh286P4HSbBAIX4\n\tOE1gCLEqkJwLLIiogkR/Ua0zruxCsmGP5XgcJPnH0+rQxVJ+qZtdKzQ+3BswVfWRHWHBlf+R2ir\n\tqoN6RU+bd4gEdMx5D+XC10r6LcaFgsZVz7eY89oSq","X-Gm-Gg":"ASbGncuaLv3LlUZzbYPSAawHhOJZDIXm2tYFaxEuVyUIR2qDUOESxwlufkyG73iQ3hB\n\tP4A/z68XVdCXXvxUI/NdWrH8B89BUn7zMMDL8eM91hhTSi/RMa3pDBhdcgwSCgQ+WSDh/hfdCIJ\n\tiJ569VQ5oDuiMmYKQpON9MNXi7IfZWOJnYeCeDUD95bzU2VcPOT0L1tHfdXiP9zS8wDC0Jwg==","X-Google-Smtp-Source":"AGHT+IF3MbO14M8h4VDjrZiU64fNl2PpLXlnKuTVzzzwJa/Sk9i+eaJElMFrqJwCPu+LnUS1r8tSMkLXp3z9DuJLkps=","X-Received":"by 2002:a05:651c:2325:b0:32a:648d:5c0f with SMTP id\n\t38308e7fff4ca-32b98ef0cd8mr3420801fa.16.1750394025212;\n\tThu, 19 Jun 2025 21:33:45 -0700 (PDT)","MIME-Version":"1.0","References":"<20250619082027.4058507-1-chenghaoyang@google.com>\n\t\n\t\n\t<20250619103004.GL22102@pendragon.ideasonboard.com>","In-Reply-To":"<20250619103004.GL22102@pendragon.ideasonboard.com>","From":"Cheng-Hao Yang ","Date":"Fri, 20 Jun 2025 12:33:34 +0800","X-Gm-Features":"AX0GCFsaQjVxFNuwyavuVqxpERoUXoudPfJEihOf0d9RW6QbmnmJu3oHHBOrVIQ","Message-ID":"","Subject":"Re: [PATCH v2] EncoderLibJpeg: Migrate const global pixelInfo to\n\tstatic","To":"Laurent Pinchart ","Cc":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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":34585,"web_url":"https://patchwork.libcamera.org/comment/34585/","msgid":"<7978e258-d294-4ff0-b3ab-7330d713e58c@ideasonboard.com>","date":"2025-06-20T07:30:36","subject":"Re: [PATCH v2] EncoderLibJpeg: Migrate const global pixelInfo to\n\tstatic","submitter":{"id":216,"url":"https://patchwork.libcamera.org/api/people/216/","name":"Barnabás Pőcze","email":"barnabas.pocze@ideasonboard.com"},"content":"Hi\n\n2025. 06. 20. 6:33 keltezéssel, Cheng-Hao Yang írta:\n> Hi Laurent,\n> \n> On Thu, Jun 19, 2025 at 6:30 PM Laurent Pinchart\n> wrote:\n>>\n>> On Thu, Jun 19, 2025 at 06:02:33PM +0800, Cheng-Hao Yang wrote:\n>>> On Thu, Jun 19, 2025 at 5:57 PM Barnabás Pőcze wrote:\n>>>> 2025. 06. 19. 10:20 keltezéssel, Harvey Yang írta:\n>>>>> From: Harvey Yang \n>>>>>\n>>>>> Previously, const global `pixelInfo` in `encoder_libjpeg.cpp` is\n>>>>> initialized before const global `pixelFormatInfo` in `formats.cpp`,\n>>>>> even if `formats.cpp` is wrapped in the base libcamera shared library,\n>>>>> and the android adapter (which contains `encoder_libjpeg.cpp`)\n>>>>> depends on the base libcamera shared library.\n>>>>\n>>>> I generally agree with any removal of non-constinit or mutable global state.\n>>>>\n>>>> So were you getting `Unsupported pixel format ...` warnings previously,\n>>>> or a crash or something else?\n>>>\n>>> Yes, I got `Unsupported pixel format ...` warnings, and after adding logs,\n>>> I found `pixelFormatInfo` has size 0 when being queried.\n>>\n>> There's something I don't quite get.\n>> src/android/jpeg/encoder_libjpeg.cpp is compiled in libcamera-hal.so,\n>> which links against libcamera.so. The latter provides\n>> PixelFormatInfo::info(). Does this mean that the global variables of\n>> libcamera-hal.so get initialized before those of libcamera.so ? Is that\n>> allowed ? More investigation seems to be needed.\n> \n> I had the same question, while the experiments showed the other way\n> around. There are also similar discussions like:\n> https://stackoverflow.com/questions/3746238/c-global-initialization-order-ignores-dependencies\n\nThe case presented there I think is clear. But here we have two shared libraries\nwhere one depends on the other. It seems a bit odd that the dynamic linker would\nstart the initialization of the dependent before the initialization of the\ndependency is done.\n\n\n> \n> \n>>\n>>>>> The reason should be that C++ initialization order doesn't follow the\n>>>>> linking process.\n>>>>\n>>>> I would have assumed that any reasonable linker will bring up the dependencies\n>>>> before the dependents, but I suppose it is not impossible that the ordering\n>>>> is different for some reason.\n>>>\n>>> The testing environment is Android OS, FYI.\n>>\n>> How to you compile libcamera on Android ?\n> \n> Currently we have to rewrite with Android.bp, instead of using the available\n> `meson.build`s, while as I mentioned before, I built `libcamera_src_defaults`,\n> which contains `src/libcamera/formats.cpp`. And built\n> `libcamera_android_src_defaults` (depending on `libcamera_src_defaults`)\n> , which contains `src/android/jpeg/encoder_libjpeg.cpp`.\n\nCan you tell us where that `Android.bp` file is? I can't find it. It does not\nmake any of the two a static library, correct?\n\n\nRegards,\nBarnabás Pőcze\n\n\n> \n> BR,\n> Harvey\n> \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 A3D63C3237\n\tfor ;\n\tFri, 20 Jun 2025 07:30:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D236968DE2;\n\tFri, 20 Jun 2025 09:30:41 +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 F096361536\n\tfor ;\n\tFri, 20 Jun 2025 09:30:40 +0200 (CEST)","from [192.168.33.15] (185.221.143.107.nat.pool.zt.hu\n\t[185.221.143.107])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 345427FA;\n\tFri, 20 Jun 2025 09:30:26 +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=\"XVBE1USF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1750404626;\n\tbh=HwcpQDbT9YMoIZEe2PyzQOlG4fcdlGczds25smi0PSU=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=XVBE1USFXpu0BDHmbVc3JiEiQDSUxZqy/bkBqDYcfAwjaEYikQ+hzolpL/Wz/b7nM\n\tOFVdlUhrEGU9V9yB0czTpO9sNZwbawrtv0hUw8E07J2zHECP9YsB8OpHBydA2EhvZ9\n\trbKIyug+QqbhCndsaFTd9YoI+zIt36fXvZQuOvIA=","Message-ID":"<7978e258-d294-4ff0-b3ab-7330d713e58c@ideasonboard.com>","Date":"Fri, 20 Jun 2025 09:30:36 +0200","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v2] EncoderLibJpeg: Migrate const global pixelInfo to\n\tstatic","To":"Cheng-Hao Yang ,\n\tLaurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20250619082027.4058507-1-chenghaoyang@google.com>\n\t\n\t\n\t<20250619103004.GL22102@pendragon.ideasonboard.com>\n\t","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Content-Language":"en-US, hu-HU","In-Reply-To":"","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":34586,"web_url":"https://patchwork.libcamera.org/comment/34586/","msgid":"","date":"2025-06-20T08:02:02","subject":"Re: [PATCH v2] EncoderLibJpeg: Migrate const global pixelInfo to\n\tstatic","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Barnabás,\n\nOn Fri, Jun 20, 2025 at 3:30 PM Barnabás Pőcze\n wrote:\n>\n> Hi\n>\n> 2025. 06. 20. 6:33 keltezéssel, Cheng-Hao Yang írta:\n> > Hi Laurent,\n> >\n> > On Thu, Jun 19, 2025 at 6:30 PM Laurent Pinchart\n> > wrote:\n> >>\n> >> On Thu, Jun 19, 2025 at 06:02:33PM +0800, Cheng-Hao Yang wrote:\n> >>> On Thu, Jun 19, 2025 at 5:57 PM Barnabás Pőcze wrote:\n> >>>> 2025. 06. 19. 10:20 keltezéssel, Harvey Yang írta:\n> >>>>> From: Harvey Yang \n> >>>>>\n> >>>>> Previously, const global `pixelInfo` in `encoder_libjpeg.cpp` is\n> >>>>> initialized before const global `pixelFormatInfo` in `formats.cpp`,\n> >>>>> even if `formats.cpp` is wrapped in the base libcamera shared library,\n> >>>>> and the android adapter (which contains `encoder_libjpeg.cpp`)\n> >>>>> depends on the base libcamera shared library.\n> >>>>\n> >>>> I generally agree with any removal of non-constinit or mutable global state.\n> >>>>\n> >>>> So were you getting `Unsupported pixel format ...` warnings previously,\n> >>>> or a crash or something else?\n> >>>\n> >>> Yes, I got `Unsupported pixel format ...` warnings, and after adding logs,\n> >>> I found `pixelFormatInfo` has size 0 when being queried.\n> >>\n> >> There's something I don't quite get.\n> >> src/android/jpeg/encoder_libjpeg.cpp is compiled in libcamera-hal.so,\n> >> which links against libcamera.so. The latter provides\n> >> PixelFormatInfo::info(). Does this mean that the global variables of\n> >> libcamera-hal.so get initialized before those of libcamera.so ? Is that\n> >> allowed ? More investigation seems to be needed.\n> >\n> > I had the same question, while the experiments showed the other way\n> > around. There are also similar discussions like:\n> > https://stackoverflow.com/questions/3746238/c-global-initialization-order-ignores-dependencies\n>\n> The case presented there I think is clear. But here we have two shared libraries\n> where one depends on the other. It seems a bit odd that the dynamic linker would\n> start the initialization of the dependent before the initialization of the\n> dependency is done.\n>\n>\n> >\n> >\n> >>\n> >>>>> The reason should be that C++ initialization order doesn't follow the\n> >>>>> linking process.\n> >>>>\n> >>>> I would have assumed that any reasonable linker will bring up the dependencies\n> >>>> before the dependents, but I suppose it is not impossible that the ordering\n> >>>> is different for some reason.\n> >>>\n> >>> The testing environment is Android OS, FYI.\n> >>\n> >> How to you compile libcamera on Android ?\n> >\n> > Currently we have to rewrite with Android.bp, instead of using the available\n> > `meson.build`s, while as I mentioned before, I built `libcamera_src_defaults`,\n> > which contains `src/libcamera/formats.cpp`. And built\n> > `libcamera_android_src_defaults` (depending on `libcamera_src_defaults`)\n> > , which contains `src/android/jpeg/encoder_libjpeg.cpp`.\n>\n> Can you tell us where that `Android.bp` file is? I can't find it. It does not\n> make any of the two a static library, correct?\n\nAs it's in an Android vertical (for ChromeOS) repository, it's not\npublic now, sorry.\n\nAfter some more investigation, I think I found what I missed:\nI used `cc_defaults` in Android.bp to build the core sources and the Android\nAdapter sources, while it doesn't produce intermediate results/shared libraries.\nWhen I added the final shared library with Virtual pipeline handler, it combines\nall the defaults dependencies into a single shared library, instead of\ndoing it with\nlinking, I believe.\n\nSeems that it's only a special case for Android. I'll check if we\nshould refine our\nAndroid.bp to avoid cc_defaults or just adopt this patch in our repository.\nDoesn't need to merge this patch into the upstream though.\n\nBR,\nHarvey\n\n>\n>\n> Regards,\n> Barnabás Pőcze\n>\n>\n> >\n> > BR,\n> > Harvey\n> >\n> > [...]\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 532A0BDE6B\n\tfor ;\n\tFri, 20 Jun 2025 08:02:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7C37E68DE2;\n\tFri, 20 Jun 2025 10:02:17 +0200 (CEST)","from mail-lf1-x135.google.com (mail-lf1-x135.google.com\n\t[IPv6:2a00:1450:4864:20::135])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1CB7261536\n\tfor ;\n\tFri, 20 Jun 2025 10:02:16 +0200 (CEST)","by mail-lf1-x135.google.com with SMTP id\n\t2adb3069b0e04-54d98aa5981so2008113e87.0\n\tfor ;\n\tFri, 20 Jun 2025 01:02:16 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"YsW74idg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1750406535; x=1751011335;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=LyadNe/yZG26WSfOhT38FoGk6JNlP962qpGsMDXfb/4=;\n\tb=YsW74idgf6OTWLqIU1b2UMOj+tiXyp921isRyflhPkZ/hBFs5D+OKcVY4jKKwTEbB2\n\tW2H+GGSVPZfteCr6AO9VfCB0zjN7OqCljJwYtYY2q/IC1lNhMoYjr6Fq7XhIVMeNCrH+\n\t8oEsqOMlNG4Xc8sFqr2LqBu/bMmrGo1ppnbKk=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1750406535; x=1751011335;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=LyadNe/yZG26WSfOhT38FoGk6JNlP962qpGsMDXfb/4=;\n\tb=X5vIPCt4IKJN62CKTIB+4CxbWkKPS4MmLvX16UjVz5mvs67rDUQ7Nyoqhh7+26ghse\n\t7OpExMJwVacG8jh3eNEXneazfqfTrQOgsl2r+oOCUxSt5Y9XAzSy8nk6P+3t+8g4A26e\n\tCAoT78gHM6CifVwYHcJeOAre99dTpxESybjWgTBvZVCNrBtdOT5JV04550SIeOK5UJFH\n\tBYSKet2HC+u+wtHcRC+YlUb2u4vcpGlvyRXHVJ/B18rmk2YxpTmmiKW9hzjmOX3yGznb\n\t5k53cAjLVxuZgjqawFSko4lMOCgaz9vtd9haF5pJtjvHU1UZDDdbW/z2PY+2RM9DhzAx\n\tarng==","X-Forwarded-Encrypted":"i=1;\n\tAJvYcCUAseC26bswdmTVruujMTIYvW6NZuz62vRc6ffVYZC8Wq/Glb3J64X0SsYVuPEqkArIA+8RMSl9VvIJJWK7xSU=@lists.libcamera.org","X-Gm-Message-State":"AOJu0YylCeTeAMzTJdlQgVEnlgaWvNe5hWbQqt0hUoTSqi7P4QqSieB0\n\t6XkbO9tPuHrVnbZv3Dn87Xjv35SuFmNy71d0odUlMXo82JssGva9MImRWZSvLKmjNIkJpKGrb4R\n\t8kzwdNprpQRSINFwLCsFGMH5e5ekrl7iLyFDfq95i","X-Gm-Gg":"ASbGncs6ZrGOw2pB20ugGKpd74JpEt2sLA8l2hGXSdfKoMOEJZ5ktnNaxuAYA0/MLdW\n\tMxPchDLpAbijeVIzeZO/tail+l95DPp4VfzWpp1lBwkiW81EA8SPM/vhiYVo5eji3DuFAXP3jJ3\n\t7kwA2B71EuYXomOWeeYV8x5/wyOH8BAt8Qa4z7T6uZ6pg9CnWhSoWPXzKyWfFrcpF/g96pnNs=","X-Google-Smtp-Source":"AGHT+IGENSFnZdE72XB3Rvs1la9lmKKx/fzC58MFOITe+bXpdKaMiPax2hRcWiG7XhGXDVucFoSCPp4BJscNeVGS/0Y=","X-Received":"by 2002:a05:651c:b10:b0:32b:7165:d0 with SMTP id\n\t38308e7fff4ca-32b9919ed9emr5070021fa.10.1750406535122;\n\tFri, 20 Jun 2025 01:02:15 -0700 (PDT)","MIME-Version":"1.0","References":"<20250619082027.4058507-1-chenghaoyang@google.com>\n\t\n\t\n\t<20250619103004.GL22102@pendragon.ideasonboard.com>\n\t\n\t<7978e258-d294-4ff0-b3ab-7330d713e58c@ideasonboard.com>","In-Reply-To":"<7978e258-d294-4ff0-b3ab-7330d713e58c@ideasonboard.com>","From":"Cheng-Hao Yang ","Date":"Fri, 20 Jun 2025 16:02:02 +0800","X-Gm-Features":"AX0GCFvvud5pDQz-HCHI8wAJ4xn9n7QEJ6QpVJJ6N26mYU8U2dsqktIyFh4ys7Q","Message-ID":"","Subject":"Re: [PATCH v2] EncoderLibJpeg: Migrate const global pixelInfo to\n\tstatic","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","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\" "}}]