[{"id":14431,"web_url":"https://patchwork.libcamera.org/comment/14431/","msgid":"<20210101095025.3etcwdzahk47fnag@basti-TUXEDO-Book-XA1510>","date":"2021-01-01T09:50:25","subject":"Re: [libcamera-devel] [PATCH v2 1/4] libcamera: Add the\n\tfromV4L2PixelFormat function","submitter":{"id":73,"url":"https://patchwork.libcamera.org/api/people/73/","name":"Sebastian Fricke","email":"sebastian.fricke.linux@gmail.com"},"content":"Hey everyone,\n\nI have noticed that I forgot to remove the declaration from the header\nfile. This will be fixed in V3.\n\nOn 31.12.2020 16:53, Sebastian Fricke wrote:\n>Add a static member function to get the corresponding Bayer-format\n>from a given V4L2PixelFormat.\n>Replace an existing method to instantiate an object from a matching\n>V4l2PixelFormat, to not duplicate the code.\n>The motivation behind this patch is to align the overall structure\n>of the BayerFormat class with other parts of the code base, such as\n>the V4L2PixelFormat class.\n>\n>Remove the v4l2ToBayer mapping table and use the bayerToV4l2 mapping\n>table by searching for a mapped element to get the corresponding key.\n>The downside of this approach is a slightly worse time complexity, but\n>the upside is a smaller codebase and lower memory consumption. As the\n>function is probably not used very frequently, I tend to favor the\n>mentioned upsides.\n>\n>Signed-off-by: Sebastian Fricke \n>---\n> include/libcamera/internal/bayer_format.h | 1 +\n> src/libcamera/bayer_format.cpp | 63 +++++++----------------\n> 2 files changed, 19 insertions(+), 45 deletions(-)\n>\n>diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h\n>index 4280b76b..8efe1382 100644\n>--- a/include/libcamera/internal/bayer_format.h\n>+++ b/include/libcamera/internal/bayer_format.h\n>@@ -48,6 +48,7 @@ public:\n> \tstd::string toString() const;\n>\n> \tV4L2PixelFormat toV4L2PixelFormat() const;\n>+\tstatic BayerFormat fromV4L2PixelFormat(V4L2PixelFormat v4l2Format);\n> \tBayerFormat transform(Transform t) const;\n>\n> \tOrder order;\n>diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n>index c42792ff..26065b66 100644\n>--- a/src/libcamera/bayer_format.cpp\n>+++ b/src/libcamera/bayer_format.cpp\n>@@ -7,6 +7,7 @@\n>\n> #include \"libcamera/internal/bayer_format.h\"\n>\n>+#include \n> #include \n>\n> #include \n>@@ -57,37 +58,6 @@ namespace libcamera {\n>\n> namespace {\n>\n>-const std::map v4l2ToBayer{\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), { BayerFormat::BGGR, 8, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), { BayerFormat::GBRG, 8, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), { BayerFormat::GRBG, 8, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), { BayerFormat::RGGB, 8, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), { BayerFormat::BGGR, 12, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), { BayerFormat::GBRG, 12, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), { BayerFormat::GRBG, 12, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), { BayerFormat::RGGB, 12, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), { BayerFormat::BGGR, 16, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), { BayerFormat::GBRG, 16, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), { BayerFormat::GRBG, 16, BayerFormat::None } },\n>-\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), { BayerFormat::RGGB, 16, BayerFormat::None } },\n>-};\n>-\n> /* Define a slightly arbitrary ordering so that we can use a std::map. */\n> struct BayerFormatComparator {\n> \tconstexpr bool operator()(const BayerFormat &lhs, const BayerFormat &rhs) const\n>@@ -155,20 +125,6 @@ const std::map bayerToV4l2{\n> * \\param[in] p The type of packing applied to the pixel values\n> */\n>\n>-/**\n>- * \\brief Construct a BayerFormat from a V4L2PixelFormat\n>- * \\param[in] v4l2Format The raw format to convert into a BayerFormat\n>- */\n>-BayerFormat::BayerFormat(V4L2PixelFormat v4l2Format)\n>-\t: order(BGGR), packing(None)\n>-{\n>-\tconst auto it = v4l2ToBayer.find(v4l2Format);\n>-\tif (it == v4l2ToBayer.end())\n>-\t\tbitDepth = 0;\n>-\telse\n>-\t\t*this = it->second;\n>-}\n>-\n> /**\n> * \\fn BayerFormat::isValid()\n> * \\brief Return whether a BayerFormat is valid\n>@@ -217,6 +173,23 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const\n> \treturn V4L2PixelFormat();\n> }\n>\n>+/**\n>+ * \\brief Convert \\a v4l2Format into the corresponding BayerFormat\n>+ * \\param[in] v4l2Format The raw format to convert into a BayerFormat\n>+ * \\return The BayerFormat corresponding to \\a v4l2Format\n>+ */\n>+BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)\n>+{\n>+\tauto it = std::find_if(\n>+\t\tbayerToV4l2.begin(),\n>+\t\tbayerToV4l2.end(),\n>+\t\t[v4l2Format](const auto &i) { return i.second == v4l2Format; });\n>+\tif (it != bayerToV4l2.end())\n>+\t\treturn it->first;\n>+\n>+\treturn BayerFormat();\n>+}\n>+\n> /**\n> * \\brief Apply a transform to this BayerFormat\n> * \\param[in] t The transform to apply\n>-- \n>2.25.1\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 7AD0AC0F1A\n\tfor ;\n\tFri, 1 Jan 2021 09:50:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E5DDF615D2;\n\tFri, 1 Jan 2021 10:50:28 +0100 (CET)","from mail-wm1-x32d.google.com (mail-wm1-x32d.google.com\n\t[IPv6:2a00:1450:4864:20::32d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 60C5C60319\n\tfor ;\n\tFri, 1 Jan 2021 10:50:27 +0100 (CET)","by mail-wm1-x32d.google.com with SMTP id n16so4628704wmc.0\n\tfor ;\n\tFri, 01 Jan 2021 01:50:27 -0800 (PST)","from localhost\n\t(p200300d1ff286c0039590ed9402fa8f3.dip0.t-ipconnect.de.\n\t[2003:d1:ff28:6c00:3959:ed9:402f:a8f3])\n\tby smtp.gmail.com with ESMTPSA id\n\tb7sm68584145wrv.47.2021.01.01.01.50.26\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 01 Jan 2021 01:50:26 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=gmail.com header.i=@gmail.com\n\theader.b=\"cYGiJAo6\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025;\n\th=date:from:to:cc:subject:message-id:reply-to:references:mime-version\n\t:content-disposition:in-reply-to;\n\tbh=XxS++ClAMV2sOMIYPaVjKqr29mwnmy3ny8S1QO3wshw=;\n\tb=cYGiJAo65o4yEymAuzpbq7e+KigZ9yqAqkOqe5DbFEEygXADEhbbrVAUn7Et66kmy0\n\tbRBYHCizIpQHyP9Fa6aMC5oBBEKtUYCkmBALKJg/FoKnNRBSm5joeMuzZO9t1Efgws85\n\tqfk8I3NWzLe5h0FW4X4F2AAti/sudNCvbJR58/b6n218wN+qPRfg+MyMKRcq82eqlsVy\n\tAmrdtvKBlIQD3QCpm73iocHj+7I1ZRpnO041YFJHaAYsKiPMSYlfRxLvLFTROcg9x9Vm\n\tPFPDkHOjlbOBy2XcbNq1nlgD5r7Y6SAFugnrZzu5AvNuS4VjfwsEvNuEkZAJXUIOvmEW\n\t+vtQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:reply-to\n\t:references:mime-version:content-disposition:in-reply-to;\n\tbh=XxS++ClAMV2sOMIYPaVjKqr29mwnmy3ny8S1QO3wshw=;\n\tb=ClzXuPv/XTl958LU30RJg4IEtAhX5KP76cqfIa518qSGmhv3d0dX25U7gBf3tf++hB\n\tFaFFCSBl11RAtrwFP6DmOE/yA8nGhtzeShjBHH8z3700yfnbiLAFQYsTMsScAP6NwSXP\n\tRTi3/DiQyM9SXq4wkaolHd9JQZCpWdTpnTTIW14ULTDw8uiHK6DArG6WjSbWiW144Yz+\n\tTqn7w1RxMM+ecjnv2iw78rG8Y7Ma7MipQhOU7vOYObBP8U8Wg26fRsuBWMUnyQuCTo42\n\toyT70cg/hmdtSRyK98VHqiyhxEYCYLwA+jH6pDmrapIe9SgSstmSBowZm9kkCMS1NNWp\n\toImg==","X-Gm-Message-State":"AOAM533SiCbvYWij/0EgORkqEwBCGWI8YU4N1MAEpILKfEr1CyV2qlZ1\n\tyKJQByTmOWQgBuq9IT0FiOKEBG55V9U=","X-Google-Smtp-Source":"ABdhPJy+bix0LbNdTimur4iMUv4bDpLDTqe4dsEvRG6isYG3CfVJtoOuZfSak2Lsx7yKdCWaPffW/A==","X-Received":"by 2002:a1c:6a13:: with SMTP id\n\tf19mr14883998wmc.10.1609494626992; \n\tFri, 01 Jan 2021 01:50:26 -0800 (PST)","Date":"Fri, 1 Jan 2021 10:50:25 +0100","From":"Sebastian Fricke ","To":"libcamera-devel@lists.libcamera.org","Message-ID":"<20210101095025.3etcwdzahk47fnag@basti-TUXEDO-Book-XA1510>","References":"<20201231155336.7058-1-sebastian.fricke.linux@gmail.com>\n\t<20201231155336.7058-2-sebastian.fricke.linux@gmail.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201231155336.7058-2-sebastian.fricke.linux@gmail.com>","Subject":"Re: [libcamera-devel] [PATCH v2 1/4] libcamera: Add the\n\tfromV4L2PixelFormat function","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","Reply-To":"Sebastian Fricke ","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14474,"web_url":"https://patchwork.libcamera.org/comment/14474/","msgid":"<20210108112930.5ioeepovrjyvstgt@uno.localdomain>","date":"2021-01-08T11:29:30","subject":"Re: [libcamera-devel] [PATCH v2 1/4] libcamera: Add the\n\tfromV4L2PixelFormat function","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Sebastian,\n\nOn Fri, Jan 01, 2021 at 10:50:25AM +0100, Sebastian Fricke wrote:\n> Hey everyone,\n>\n> I have noticed that I forgot to remove the declaration from the header\n> file. This will be fixed in V3.\n>\n\nYep, you will also have to rebase, but that should be quite painless\n\nAlso, this patch breaks the RPi pipeline handler here.\nWhich shuld probably be ported to the new API in this path too\n\ndiff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex f121328ee9a9..ede153651ce9 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -358,7 +358,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n */\n V4L2PixelFormat fourcc = sensorFormat.fourcc;\n if (data_->flipsAlterBayerOrder_) {\n- BayerFormat bayer(fourcc);\n+ BayerFormat bayer = BayerFormat::fromV4L2PixelFormat(fourcc);\n bayer.order = data_->nativeBayerOrder_;\n bayer = bayer.transform(combined);\n fourcc = bayer.toV4L2PixelFormat();\n@@ -1007,7 +1007,7 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n BayerFormat bayerFormat;\n for (const auto &iter : dev->formats()) {\n V4L2PixelFormat v4l2Format = iter.first;\n- bayerFormat = BayerFormat(v4l2Format);\n+ bayerFormat = BayerFormat::fromV4L2PixelFormat(v4l2Format);\n if (bayerFormat.isValid())\n break;\n }\n\n\n> On 31.12.2020 16:53, Sebastian Fricke wrote:\n> > Add a static member function to get the corresponding Bayer-format\n\nBayerFormat\n\n> > from a given V4L2PixelFormat.\n> > Replace an existing method to instantiate an object from a matching\n> > V4l2PixelFormat, to not duplicate the code.\n\nmethod to instantiate an object = constructor :)\n\n> > The motivation behind this patch is to align the overall structure\n> > of the BayerFormat class with other parts of the code base, such as\n> > the V4L2PixelFormat class.\n> >\n> > Remove the v4l2ToBayer mapping table and use the bayerToV4l2 mapping\n> > table by searching for a mapped element to get the corresponding key.\n> > The downside of this approach is a slightly worse time complexity, but\n> > the upside is a smaller codebase and lower memory consumption. As the\n> > function is probably not used very frequently, I tend to favor the\n> > mentioned upsides.\n\nAgreed!\n\n> >\n> > Signed-off-by: Sebastian Fricke \n> > ---\n> > include/libcamera/internal/bayer_format.h | 1 +\n> > src/libcamera/bayer_format.cpp | 63 +++++++----------------\n> > 2 files changed, 19 insertions(+), 45 deletions(-)\n> >\n> > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h\n> > index 4280b76b..8efe1382 100644\n> > --- a/include/libcamera/internal/bayer_format.h\n> > +++ b/include/libcamera/internal/bayer_format.h\n> > @@ -48,6 +48,7 @@ public:\n> > \tstd::string toString() const;\n> >\n> > \tV4L2PixelFormat toV4L2PixelFormat() const;\n> > +\tstatic BayerFormat fromV4L2PixelFormat(V4L2PixelFormat v4l2Format);\n\nstatic const BayerFormat &\nconst V4L2PixelFormat &v4l2Format\n\n> > \tBayerFormat transform(Transform t) const;\n> >\n> > \tOrder order;\n> > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> > index c42792ff..26065b66 100644\n> > --- a/src/libcamera/bayer_format.cpp\n> > +++ b/src/libcamera/bayer_format.cpp\n> > @@ -7,6 +7,7 @@\n> >\n> > #include \"libcamera/internal/bayer_format.h\"\n> >\n> > +#include \n> > #include \n> >\n> > #include \n> > @@ -57,37 +58,6 @@ namespace libcamera {\n> >\n> > namespace {\n> >\n> > -const std::map v4l2ToBayer{\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), { BayerFormat::BGGR, 8, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), { BayerFormat::GBRG, 8, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), { BayerFormat::GRBG, 8, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), { BayerFormat::RGGB, 8, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), { BayerFormat::BGGR, 12, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), { BayerFormat::GBRG, 12, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), { BayerFormat::GRBG, 12, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), { BayerFormat::RGGB, 12, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), { BayerFormat::BGGR, 16, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), { BayerFormat::GBRG, 16, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), { BayerFormat::GRBG, 16, BayerFormat::None } },\n> > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), { BayerFormat::RGGB, 16, BayerFormat::None } },\n> > -};\n> > -\n> > /* Define a slightly arbitrary ordering so that we can use a std::map. */\n> > struct BayerFormatComparator {\n> > \tconstexpr bool operator()(const BayerFormat &lhs, const BayerFormat &rhs) const\n> > @@ -155,20 +125,6 @@ const std::map bayerToV4l2{\n> > * \\param[in] p The type of packing applied to the pixel values\n> > */\n> >\n> > -/**\n> > - * \\brief Construct a BayerFormat from a V4L2PixelFormat\n> > - * \\param[in] v4l2Format The raw format to convert into a BayerFormat\n> > - */\n> > -BayerFormat::BayerFormat(V4L2PixelFormat v4l2Format)\n> > -\t: order(BGGR), packing(None)\n> > -{\n> > -\tconst auto it = v4l2ToBayer.find(v4l2Format);\n> > -\tif (it == v4l2ToBayer.end())\n> > -\t\tbitDepth = 0;\n> > -\telse\n> > -\t\t*this = it->second;\n> > -}\n> > -\n> > /**\n> > * \\fn BayerFormat::isValid()\n> > * \\brief Return whether a BayerFormat is valid\n> > @@ -217,6 +173,23 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const\n> > \treturn V4L2PixelFormat();\n> > }\n> >\n> > +/**\n> > + * \\brief Convert \\a v4l2Format into the corresponding BayerFormat\n> > + * \\param[in] v4l2Format The raw format to convert into a BayerFormat\n> > + * \\return The BayerFormat corresponding to \\a v4l2Format\n> > + */\n> > +BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)\n> > +{\n> > +\tauto it = std::find_if(\n> > +\t\tbayerToV4l2.begin(),\n> > +\t\tbayerToV4l2.end(),\n> > +\t\t[v4l2Format](const auto &i) { return i.second == v4l2Format; });\n\n&v4l2Format\n\nMinors apart, I like the direction this patch takes\n\nThanks\n j\n\n> > +\tif (it != bayerToV4l2.end())\n> > +\t\treturn it->first;\n> > +\n> > +\treturn BayerFormat();\n> > +}\n> > +\n> > /**\n> > * \\brief Apply a transform to this BayerFormat\n> > * \\param[in] t The transform to apply\n> > --\n> > 2.25.1\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 E149FC3256\n\tfor ;\n\tFri, 8 Jan 2021 11:29:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6203E635A0;\n\tFri, 8 Jan 2021 12:29:17 +0100 (CET)","from relay12.mail.gandi.net (relay12.mail.gandi.net\n\t[217.70.178.232])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E0AC763138\n\tfor ;\n\tFri, 8 Jan 2021 12:29:15 +0100 (CET)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay12.mail.gandi.net (Postfix) with ESMTPSA id 95528200006;\n\tFri, 8 Jan 2021 11:29:14 +0000 (UTC)"],"Date":"Fri, 8 Jan 2021 12:29:30 +0100","From":"Jacopo Mondi ","To":"Sebastian Fricke ","Message-ID":"<20210108112930.5ioeepovrjyvstgt@uno.localdomain>","References":"<20201231155336.7058-1-sebastian.fricke.linux@gmail.com>\n\t<20201231155336.7058-2-sebastian.fricke.linux@gmail.com>\n\t<20210101095025.3etcwdzahk47fnag@basti-TUXEDO-Book-XA1510>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210101095025.3etcwdzahk47fnag@basti-TUXEDO-Book-XA1510>","Subject":"Re: [libcamera-devel] [PATCH v2 1/4] libcamera: Add the\n\tfromV4L2PixelFormat function","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14514,"web_url":"https://patchwork.libcamera.org/comment/14514/","msgid":"","date":"2021-01-11T00:53:11","subject":"Re: [libcamera-devel] [PATCH v2 1/4] libcamera: Add the\n\tfromV4L2PixelFormat function","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Sebastian,\n\nThank you for the patch.\n\nOn Fri, Jan 08, 2021 at 12:29:30PM +0100, Jacopo Mondi wrote:\n> On Fri, Jan 01, 2021 at 10:50:25AM +0100, Sebastian Fricke wrote:\n> > Hey everyone,\n> >\n> > I have noticed that I forgot to remove the declaration from the header\n> > file. This will be fixed in V3.\n> \n> Yep, you will also have to rebase, but that should be quite painless\n> \n> Also, this patch breaks the RPi pipeline handler here.\n> Which shuld probably be ported to the new API in this path too\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index f121328ee9a9..ede153651ce9 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -358,7 +358,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> */\n> V4L2PixelFormat fourcc = sensorFormat.fourcc;\n> if (data_->flipsAlterBayerOrder_) {\n> - BayerFormat bayer(fourcc);\n> + BayerFormat bayer = BayerFormat::fromV4L2PixelFormat(fourcc);\n> bayer.order = data_->nativeBayerOrder_;\n> bayer = bayer.transform(combined);\n> fourcc = bayer.toV4L2PixelFormat();\n> @@ -1007,7 +1007,7 @@ bool PipelineHandlerRPi::match(DeviceEnumerator *enumerator)\n> BayerFormat bayerFormat;\n> for (const auto &iter : dev->formats()) {\n> V4L2PixelFormat v4l2Format = iter.first;\n> - bayerFormat = BayerFormat(v4l2Format);\n> + bayerFormat = BayerFormat::fromV4L2PixelFormat(v4l2Format);\n> if (bayerFormat.isValid())\n> break;\n> }\n> \n> \n> > On 31.12.2020 16:53, Sebastian Fricke wrote:\n> > > Add a static member function to get the corresponding Bayer-format\n> \n> BayerFormat\n> \n> > > from a given V4L2PixelFormat.\n> > > Replace an existing method to instantiate an object from a matching\n> > > V4l2PixelFormat, to not duplicate the code.\n> \n> method to instantiate an object = constructor :)\n> \n> > > The motivation behind this patch is to align the overall structure\n> > > of the BayerFormat class with other parts of the code base, such as\n> > > the V4L2PixelFormat class.\n> > >\n> > > Remove the v4l2ToBayer mapping table and use the bayerToV4l2 mapping\n> > > table by searching for a mapped element to get the corresponding key.\n> > > The downside of this approach is a slightly worse time complexity, but\n> > > the upside is a smaller codebase and lower memory consumption. As the\n> > > function is probably not used very frequently, I tend to favor the\n> > > mentioned upsides.\n\nThese are two separate changes, which would be best split in two\npatches. No big deal for this patch, just something to keep in mind for\nlater.\n\n> \n> Agreed!\n> \n> > >\n> > > Signed-off-by: Sebastian Fricke \n> > > ---\n> > > include/libcamera/internal/bayer_format.h | 1 +\n> > > src/libcamera/bayer_format.cpp | 63 +++++++----------------\n> > > 2 files changed, 19 insertions(+), 45 deletions(-)\n> > >\n> > > diff --git a/include/libcamera/internal/bayer_format.h b/include/libcamera/internal/bayer_format.h\n> > > index 4280b76b..8efe1382 100644\n> > > --- a/include/libcamera/internal/bayer_format.h\n> > > +++ b/include/libcamera/internal/bayer_format.h\n> > > @@ -48,6 +48,7 @@ public:\n> > > \tstd::string toString() const;\n> > >\n> > > \tV4L2PixelFormat toV4L2PixelFormat() const;\n> > > +\tstatic BayerFormat fromV4L2PixelFormat(V4L2PixelFormat v4l2Format);\n> \n> static const BayerFormat &\n> const V4L2PixelFormat &v4l2Format\n\nV4L2PixelFormat is just a u32, we can pass it by value instead of\nreference. Returning a reference is good.\n\n> > > \tBayerFormat transform(Transform t) const;\n> > >\n> > > \tOrder order;\n> > > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> > > index c42792ff..26065b66 100644\n> > > --- a/src/libcamera/bayer_format.cpp\n> > > +++ b/src/libcamera/bayer_format.cpp\n> > > @@ -7,6 +7,7 @@\n> > >\n> > > #include \"libcamera/internal/bayer_format.h\"\n> > >\n> > > +#include \n> > > #include \n> > >\n> > > #include \n> > > @@ -57,37 +58,6 @@ namespace libcamera {\n> > >\n> > > namespace {\n> > >\n> > > -const std::map v4l2ToBayer{\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), { BayerFormat::BGGR, 8, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), { BayerFormat::GBRG, 8, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), { BayerFormat::GRBG, 8, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), { BayerFormat::RGGB, 8, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10), { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10), { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10), { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10), { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), { BayerFormat::BGGR, 12, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), { BayerFormat::GBRG, 12, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), { BayerFormat::GRBG, 12, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), { BayerFormat::RGGB, 12, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), { BayerFormat::BGGR, 16, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), { BayerFormat::GBRG, 16, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), { BayerFormat::GRBG, 16, BayerFormat::None } },\n> > > -\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), { BayerFormat::RGGB, 16, BayerFormat::None } },\n> > > -};\n> > > -\n> > > /* Define a slightly arbitrary ordering so that we can use a std::map. */\n> > > struct BayerFormatComparator {\n> > > \tconstexpr bool operator()(const BayerFormat &lhs, const BayerFormat &rhs) const\n> > > @@ -155,20 +125,6 @@ const std::map bayerToV4l2{\n> > > * \\param[in] p The type of packing applied to the pixel values\n> > > */\n> > >\n> > > -/**\n> > > - * \\brief Construct a BayerFormat from a V4L2PixelFormat\n> > > - * \\param[in] v4l2Format The raw format to convert into a BayerFormat\n> > > - */\n> > > -BayerFormat::BayerFormat(V4L2PixelFormat v4l2Format)\n> > > -\t: order(BGGR), packing(None)\n> > > -{\n> > > -\tconst auto it = v4l2ToBayer.find(v4l2Format);\n> > > -\tif (it == v4l2ToBayer.end())\n> > > -\t\tbitDepth = 0;\n> > > -\telse\n> > > -\t\t*this = it->second;\n> > > -}\n> > > -\n> > > /**\n> > > * \\fn BayerFormat::isValid()\n> > > * \\brief Return whether a BayerFormat is valid\n> > > @@ -217,6 +173,23 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const\n> > > \treturn V4L2PixelFormat();\n> > > }\n> > >\n> > > +/**\n> > > + * \\brief Convert \\a v4l2Format into the corresponding BayerFormat\n\ns/into/to/\n\n> > > + * \\param[in] v4l2Format The raw format to convert into a BayerFormat\n> > > + * \\return The BayerFormat corresponding to \\a v4l2Format\n> > > + */\n> > > +BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)\n> > > +{\n> > > +\tauto it = std::find_if(\n> > > +\t\tbayerToV4l2.begin(),\n> > > +\t\tbayerToV4l2.end(),\n> > > +\t\t[v4l2Format](const auto &i) { return i.second == v4l2Format; });\n\nThe indentation is strange.\n\n\tauto it = std::find_if(bayerToV4l2.begin(), bayerToV4l2.end(),\n\t\t\t [v4l2Format](const auto &i) {\n\t\t\t\t return i.second == v4l2Format;\n\t\t\t });\n\n> &v4l2Format\n\nYou can even write [&](const auto &i).\n\n> Minors apart, I like the direction this patch takes\n\nWith these small issues fixed,\n\nReviewed-by: Laurent Pinchart \n\n> > > +\tif (it != bayerToV4l2.end())\n> > > +\t\treturn it->first;\n> > > +\n> > > +\treturn BayerFormat();\n> > > +}\n> > > +\n> > > /**\n> > > * \\brief Apply a transform to this BayerFormat\n> > > * \\param[in] t The transform to apply\n> > > --\n> > > 2.25.1\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 4D06DBD80C\n\tfor ;\n\tMon, 11 Jan 2021 00:53:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C074E68090;\n\tMon, 11 Jan 2021 01:53:44 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B1BEB60317\n\tfor ;\n\tMon, 11 Jan 2021 01:53:42 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2FCFBEC;\n\tMon, 11 Jan 2021 01:53:41 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"oWPkZFyv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610326421;\n\tbh=t9H/Zea8Z9yvorjsceX9o4ZuMfvSWWAJVvpQVmXnZYQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oWPkZFyvrdi4w98GZ3nF3QgqD+VLypq876PNMNSlekdE3Z+hNT/UjCrNHamtb6sBQ\n\tYSsJgarMsbzKPOxUvK6Apis8yfbclhOYtqNMUFqoqK7uYYShpIsqi+0W1B6QThNcol\n\tJcoqFVGbYwBpVaPr8Ttumo08mZe9wJ827aWEtVmo=","Date":"Mon, 11 Jan 2021 02:53:11 +0200","From":"Laurent Pinchart ","To":"Sebastian Fricke ","Message-ID":"","References":"<20201231155336.7058-1-sebastian.fricke.linux@gmail.com>\n\t<20201231155336.7058-2-sebastian.fricke.linux@gmail.com>\n\t<20210101095025.3etcwdzahk47fnag@basti-TUXEDO-Book-XA1510>\n\t<20210108112930.5ioeepovrjyvstgt@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210108112930.5ioeepovrjyvstgt@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v2 1/4] libcamera: Add the\n\tfromV4L2PixelFormat function","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","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]