[{"id":20552,"web_url":"https://patchwork.libcamera.org/comment/20552/","msgid":"<163533126582.1184428.7446201495615796658@Monstersaurus>","date":"2021-10-27T10:41:05","subject":"Re: [libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format:\n\tRework BayerFormat conversion table","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck (2021-10-27 10:27:57)\n> Rename the bayerToV4l2 conversion table to bayerToFormat. Update the table to\n> hold both the PixelFormat and V4L2PixelFormat conversions for a given\n> BayerFormat. This will allow converting between BayerFormat and PixelFormat\n> types in a subsequent change.\n> \n> Signed-off-by: Naushir Patuck \n> ---\n> src/libcamera/bayer_format.cpp | 110 ++++++++++++++++++++++-----------\n> 1 file changed, 73 insertions(+), 37 deletions(-)\n> \n> diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> index 11355f144f66..94e2294d7f6c 100644\n> --- a/src/libcamera/bayer_format.cpp\n> +++ b/src/libcamera/bayer_format.cpp\n> @@ -13,6 +13,7 @@\n> \n> #include \n> \n> +#include \n> #include \n> \n> /**\n> @@ -84,37 +85,72 @@ struct BayerFormatComparator {\n> }\n> };\n> \n> -const std::map bayerToV4l2{\n> - { { BayerFormat::BGGR, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },\n> - { { BayerFormat::GBRG, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },\n> - { { BayerFormat::GRBG, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },\n> - { { BayerFormat::RGGB, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },\n> - { { BayerFormat::BGGR, 10, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },\n> - { { BayerFormat::GBRG, 10, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },\n> - { { BayerFormat::GRBG, 10, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },\n> - { { BayerFormat::RGGB, 10, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },\n> - { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },\n> - { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },\n> - { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },\n> - { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },\n> - { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) },\n> - { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) },\n> - { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) },\n> - { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) },\n> - { { BayerFormat::BGGR, 12, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },\n> - { { BayerFormat::GBRG, 12, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },\n> - { { BayerFormat::GRBG, 12, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },\n> - { { BayerFormat::RGGB, 12, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },\n> - { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },\n> - { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },\n> - { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },\n> - { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },\n> - { { BayerFormat::BGGR, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },\n> - { { BayerFormat::GBRG, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },\n> - { { BayerFormat::GRBG, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },\n> - { { BayerFormat::RGGB, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },\n> - { { BayerFormat::MONO, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_GREY) },\n> - { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_Y10P) },\n> +struct Formats {\n> + PixelFormat pixelFormat;\n> + V4L2PixelFormat v4l2Format;\n> +};\n> +\n> +const std::map bayerToFormat{\n> + { { BayerFormat::BGGR, 8, BayerFormat::None },\n> + { formats::SBGGR8, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) } },\n\nPersonally, I think tables should be kept tabular, disregarding the\nusual coding style line lengths.\n\nA table is much more readable with long lines to me ... but ... It may\nnot be preferred to everyone. So no need to change this just for me.\nJust voicing my opinion ;-)\n\n\n> + { { BayerFormat::GBRG, 8, BayerFormat::None },\n> + { formats::SGBRG8, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) } },\n> + { { BayerFormat::GRBG, 8, BayerFormat::None },\n> + { formats::SGRBG8, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) } },\n> + { { BayerFormat::RGGB, 8, BayerFormat::None },\n> + { formats::SRGGB8, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) } },\n> + { { BayerFormat::BGGR, 10, BayerFormat::None },\n> + { formats::SBGGR10, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) } },\n> + { { BayerFormat::GBRG, 10, BayerFormat::None },\n> + { formats::SGBRG10, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) } },\n> + { { BayerFormat::GRBG, 10, BayerFormat::None },\n> + { formats::SGRBG10, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) } },\n> + { { BayerFormat::RGGB, 10, BayerFormat::None },\n> + { formats::SRGGB10, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) } },\n> + { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed },\n> + { formats::SBGGR10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) } },\n> + { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed },\n> + { formats::SGBRG10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) } },\n> + { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed },\n> + { formats::SGRBG10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) } },\n> + { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed },\n> + { formats::SRGGB10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) } },\n> + { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed },\n> + { formats::SBGGR10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) } },\n> + { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed },\n> + { formats::SGBRG10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) } },\n> + { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed },\n> + { formats::SGRBG10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) } },\n> + { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed },\n> + { formats::SRGGB10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) } },\n> + { { BayerFormat::BGGR, 12, BayerFormat::None },\n> + { formats::SBGGR12, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) } },\n> + { { BayerFormat::GBRG, 12, BayerFormat::None },\n> + { formats::SGBRG12, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) } },\n> + { { BayerFormat::GRBG, 12, BayerFormat::None },\n> + { formats::SGRBG12, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) } },\n> + { { BayerFormat::RGGB, 12, BayerFormat::None },\n> + { formats::SRGGB12, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) } },\n> + { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed },\n> + { formats::SBGGR12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) } },\n> + { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed },\n> + { formats::SGBRG12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) } },\n> + { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed },\n> + { formats::SGRBG12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) } },\n> + { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed },\n> + { formats::SRGGB12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) } },\n> + { { BayerFormat::BGGR, 16, BayerFormat::None },\n> + { formats::SBGGR16, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) } },\n> + { { BayerFormat::GBRG, 16, BayerFormat::None },\n> + { formats::SGBRG16, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) } },\n> + { { BayerFormat::GRBG, 16, BayerFormat::None },\n> + { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },\n> + { { BayerFormat::RGGB, 16, BayerFormat::None },\n> + { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },\n> + { { BayerFormat::MONO, 8, BayerFormat::None },\n> + { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },\n> + { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed },\n> + { formats::R10, V4L2PixelFormat(V4L2_PIX_FMT_Y10P) } },\n\nIn [2/9] formats::R10 gets added with V4L2_PIX_FMT_Y10, but here we have\nV4L2_PIX_FMT_Y10P. will that cause any issues?\n\n\n> };\n> \n> const std::unordered_map mbusCodeToBayer{\n> @@ -245,9 +281,9 @@ bool operator==(const BayerFormat &lhs, const BayerFormat &rhs)\n> */\n> V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const\n> {\n> - const auto it = bayerToV4l2.find(*this);\n> - if (it != bayerToV4l2.end())\n> - return it->second;\n> + const auto it = bayerToFormat.find(*this);\n> + if (it != bayerToFormat.end())\n> + return it->second.v4l2Format;\n> \n> return V4L2PixelFormat();\n> }\n> @@ -259,11 +295,11 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const\n> */\n> BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)\n> {\n> - auto it = std::find_if(bayerToV4l2.begin(), bayerToV4l2.end(),\n> + auto it = std::find_if(bayerToFormat.begin(), bayerToFormat.end(),\n> [v4l2Format](const auto &i) {\n> - return i.second == v4l2Format;\n> + return i.second.v4l2Format == v4l2Format;\n> });\n> - if (it != bayerToV4l2.end())\n> + if (it != bayerToFormat.end())\n> return it->first;\n> \n> return BayerFormat();\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 97206BDB1C\n\tfor ;\n\tWed, 27 Oct 2021 10:41:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0987D6487F;\n\tWed, 27 Oct 2021 12:41:10 +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 C6C1B60123\n\tfor ;\n\tWed, 27 Oct 2021 12:41:08 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 64E37596;\n\tWed, 27 Oct 2021 12:41: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=\"HMJb23tC\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635331268;\n\tbh=oUVuukiRtjNrWjMKIKmDlIQA58VotNk8qDOFu7/gx8M=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=HMJb23tCD493cLDlIlYlhbd2wBOQIWCGGFwIDD+Jci7b6B20PpOmmhw1gQ2WLQGVW\n\tHP/OfHrMgrYE0AL1owY+RNNa+9fpFfC56bbuumqINR9fbzUCh0/jORhuQBlrqdI/qE\n\tVWKr4a/hcV5vtiO6PaWI4z/N0LpfPt4mwpHjWTCU=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211027092803.3671096-4-naush@raspberrypi.com>","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-4-naush@raspberrypi.com>","From":"Kieran Bingham ","To":"Naushir Patuck ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 27 Oct 2021 11:41:05 +0100","Message-ID":"<163533126582.1184428.7446201495615796658@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format:\n\tRework BayerFormat conversion table","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":20554,"web_url":"https://patchwork.libcamera.org/comment/20554/","msgid":"","date":"2021-10-27T10:48:45","subject":"Re: [libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format:\n\tRework BayerFormat conversion table","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Kieran,\n\nThank you for your feedback.\n\nOn Wed, 27 Oct 2021 at 11:41, Kieran Bingham <\nkieran.bingham@ideasonboard.com> wrote:\n\n> Quoting Naushir Patuck (2021-10-27 10:27:57)\n> > Rename the bayerToV4l2 conversion table to bayerToFormat. Update the\n> table to\n> > hold both the PixelFormat and V4L2PixelFormat conversions for a given\n> > BayerFormat. This will allow converting between BayerFormat and\n> PixelFormat\n> > types in a subsequent change.\n> >\n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/libcamera/bayer_format.cpp | 110 ++++++++++++++++++++++-----------\n> > 1 file changed, 73 insertions(+), 37 deletions(-)\n> >\n> > diff --git a/src/libcamera/bayer_format.cpp\n> b/src/libcamera/bayer_format.cpp\n> > index 11355f144f66..94e2294d7f6c 100644\n> > --- a/src/libcamera/bayer_format.cpp\n> > +++ b/src/libcamera/bayer_format.cpp\n> > @@ -13,6 +13,7 @@\n> >\n> > #include \n> >\n> > +#include \n> > #include \n> >\n> > /**\n> > @@ -84,37 +85,72 @@ struct BayerFormatComparator {\n> > }\n> > };\n> >\n> > -const std::map\n> bayerToV4l2{\n> > - { { BayerFormat::BGGR, 8, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },\n> > - { { BayerFormat::GBRG, 8, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },\n> > - { { BayerFormat::GRBG, 8, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },\n> > - { { BayerFormat::RGGB, 8, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },\n> > - { { BayerFormat::BGGR, 10, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },\n> > - { { BayerFormat::GBRG, 10, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },\n> > - { { BayerFormat::GRBG, 10, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },\n> > - { { BayerFormat::RGGB, 10, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },\n> > - { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },\n> > - { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },\n> > - { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },\n> > - { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },\n> > - { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) },\n> > - { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) },\n> > - { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) },\n> > - { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) },\n> > - { { BayerFormat::BGGR, 12, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },\n> > - { { BayerFormat::GBRG, 12, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },\n> > - { { BayerFormat::GRBG, 12, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },\n> > - { { BayerFormat::RGGB, 12, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },\n> > - { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },\n> > - { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },\n> > - { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },\n> > - { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },\n> > - { { BayerFormat::BGGR, 16, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },\n> > - { { BayerFormat::GBRG, 16, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },\n> > - { { BayerFormat::GRBG, 16, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },\n> > - { { BayerFormat::RGGB, 16, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },\n> > - { { BayerFormat::MONO, 8, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_GREY) },\n> > - { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_Y10P) },\n> > +struct Formats {\n> > + PixelFormat pixelFormat;\n> > + V4L2PixelFormat v4l2Format;\n> > +};\n> > +\n> > +const std::map\n> bayerToFormat{\n> > + { { BayerFormat::BGGR, 8, BayerFormat::None },\n> > + { formats::SBGGR8, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8)\n> } },\n>\n> Personally, I think tables should be kept tabular, disregarding the\n> usual coding style line lengths.\n>\n> A table is much more readable with long lines to me ... but ... It may\n> not be preferred to everyone. So no need to change this just for me.\n> Just voicing my opinion ;-)\n>\n\nI fully agree with the above!\nI set it this way as we would go over the 120 char limit. Happy to format\nit as a single line per row\nas an exception.\n\n\n>\n>\n> > + { { BayerFormat::GBRG, 8, BayerFormat::None },\n> > + { formats::SGBRG8, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8)\n> } },\n> > + { { BayerFormat::GRBG, 8, BayerFormat::None },\n> > + { formats::SGRBG8, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8)\n> } },\n> > + { { BayerFormat::RGGB, 8, BayerFormat::None },\n> > + { formats::SRGGB8, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8)\n> } },\n> > + { { BayerFormat::BGGR, 10, BayerFormat::None },\n> > + { formats::SBGGR10,\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) } },\n> > + { { BayerFormat::GBRG, 10, BayerFormat::None },\n> > + { formats::SGBRG10,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) } },\n> > + { { BayerFormat::GRBG, 10, BayerFormat::None },\n> > + { formats::SGRBG10,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) } },\n> > + { { BayerFormat::RGGB, 10, BayerFormat::None },\n> > + { formats::SRGGB10,\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) } },\n> > + { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed },\n> > + { formats::SBGGR10_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) } },\n> > + { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed },\n> > + { formats::SGBRG10_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) } },\n> > + { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed },\n> > + { formats::SGRBG10_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) } },\n> > + { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed },\n> > + { formats::SRGGB10_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) } },\n> > + { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed },\n> > + { formats::SBGGR10_IPU3,\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) } },\n> > + { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed },\n> > + { formats::SGBRG10_IPU3,\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) } },\n> > + { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed },\n> > + { formats::SGRBG10_IPU3,\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) } },\n> > + { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed },\n> > + { formats::SRGGB10_IPU3,\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) } },\n> > + { { BayerFormat::BGGR, 12, BayerFormat::None },\n> > + { formats::SBGGR12,\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) } },\n> > + { { BayerFormat::GBRG, 12, BayerFormat::None },\n> > + { formats::SGBRG12,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) } },\n> > + { { BayerFormat::GRBG, 12, BayerFormat::None },\n> > + { formats::SGRBG12,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) } },\n> > + { { BayerFormat::RGGB, 12, BayerFormat::None },\n> > + { formats::SRGGB12,\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) } },\n> > + { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed },\n> > + { formats::SBGGR12_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) } },\n> > + { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed },\n> > + { formats::SGBRG12_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) } },\n> > + { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed },\n> > + { formats::SGRBG12_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) } },\n> > + { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed },\n> > + { formats::SRGGB12_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) } },\n> > + { { BayerFormat::BGGR, 16, BayerFormat::None },\n> > + { formats::SBGGR16,\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) } },\n> > + { { BayerFormat::GBRG, 16, BayerFormat::None },\n> > + { formats::SGBRG16,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) } },\n> > + { { BayerFormat::GRBG, 16, BayerFormat::None },\n> > + { formats::SGRBG16,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },\n> > + { { BayerFormat::RGGB, 16, BayerFormat::None },\n> > + { formats::SRGGB16,\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },\n> > + { { BayerFormat::MONO, 8, BayerFormat::None },\n> > + { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },\n> > + { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed },\n> > + { formats::R10, V4L2PixelFormat(V4L2_PIX_FMT_Y10P) } },\n>\n> In [2/9] formats::R10 gets added with V4L2_PIX_FMT_Y10, but here we have\n> V4L2_PIX_FMT_Y10P. will that cause any issues?\n>\n\nQuite right, this should not be assigned as a packed format. Will fix this.\n\nNaush\n\n\n>\n>\n> > };\n> >\n> > const std::unordered_map mbusCodeToBayer{\n> > @@ -245,9 +281,9 @@ bool operator==(const BayerFormat &lhs, const\n> BayerFormat &rhs)\n> > */\n> > V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const\n> > {\n> > - const auto it = bayerToV4l2.find(*this);\n> > - if (it != bayerToV4l2.end())\n> > - return it->second;\n> > + const auto it = bayerToFormat.find(*this);\n> > + if (it != bayerToFormat.end())\n> > + return it->second.v4l2Format;\n> >\n> > return V4L2PixelFormat();\n> > }\n> > @@ -259,11 +295,11 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat()\n> const\n> > */\n> > BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)\n> > {\n> > - auto it = std::find_if(bayerToV4l2.begin(), bayerToV4l2.end(),\n> > + auto it = std::find_if(bayerToFormat.begin(),\n> bayerToFormat.end(),\n> > [v4l2Format](const auto &i) {\n> > - return i.second == v4l2Format;\n> > + return i.second.v4l2Format ==\n> v4l2Format;\n> > });\n> > - if (it != bayerToV4l2.end())\n> > + if (it != bayerToFormat.end())\n> > return it->first;\n> >\n> > return BayerFormat();\n> > --\n> > 2.25.1\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 642F2BF415\n\tfor ;\n\tWed, 27 Oct 2021 10:49:05 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1D96364882;\n\tWed, 27 Oct 2021 12:49:05 +0200 (CEST)","from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com\n\t[IPv6:2a00:1450:4864:20::12b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F36CB60123\n\tfor ;\n\tWed, 27 Oct 2021 12:49:02 +0200 (CEST)","by mail-lf1-x12b.google.com with SMTP id j9so5222319lfu.7\n\tfor ;\n\tWed, 27 Oct 2021 03:49:02 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"qK4HtKdh\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=/CXQWClNO5pQGKN2RA0S2sit1jiJZo/3wOR8NAXXhP8=;\n\tb=qK4HtKdhIeqFlrA8BiNXZi95/cBR50/TZOhHp3f7/jV2cmAltA0YFUQrSHl07PpvQR\n\tOFlrDv7KoQV63x7T0IHhjlkSc0GDkyalGkCif6WZcNDiei/pkv1o9IsgkKmP1a8eogir\n\tF6FWwu9BTE7e4gSpHboDDBk/12NqzG+3NIpUzuMqh8cGyCVlOPghD4jxvwv1ScOg0EWb\n\tRUMe6p7Phi4dtct8hRFDXLANLjxLN/bk8sQ2CYEo/f5WkXW8LaLJyIvUrMP3ap50OH7o\n\tquuz+RP4peNHqr91dytWUSwHwQGDJNo7oXe1of1dUt6MOrJWFdiMhLloEgxTm45NZLEo\n\tQ+jw==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=/CXQWClNO5pQGKN2RA0S2sit1jiJZo/3wOR8NAXXhP8=;\n\tb=H86oPbmpi7xrg08JAbZEe5mpqm+2J/5NiNIeSV6s9qgjFokOscntyTr+TghRqbADSf\n\tLZ1NbhEXtAmUqiU3/VKgDoIFlkvmteJFWk9Rd8owS4eqUw6QadnQBvmV4tblkHBm790c\n\tGHvp60qXANraaDfsMgARSdgCbYsV9G1+3Z2TnBrU/nP/TvYQ4uQOpoTKC8DWGazybYur\n\tMTaApSchgX0bg6+XJJVJN1jaj7UMFzGHl+3J8ZOd+h5Fo51tpFj0kKJEIUavIJPfsnC8\n\tVryX7DRa3nJr98WrWa4pzV/AUMLsTFj8ALGeAdDJrYNtH0MGyCiHasxFPsLgWlCzSoC5\n\tAfdA==","X-Gm-Message-State":"AOAM533s2/DyADThKov2mj/9EeTxJvqtNLcbvtEBg4qZ9QBCGYTrjA9s\n\tPlhqJ252uFByyRSw2lX+HycDQizWDRBDJ/MBolV/PB8hzabLSA==","X-Google-Smtp-Source":"ABdhPJwALYKQrYVRygye3tU5EjSvfVOtGO0vdA3gz6rfBET0nOs2a2ourtLQo8l3IqHtGsJv/YcUw3eMnNXPz/pZDns=","X-Received":"by 2002:a19:ad0d:: with SMTP id\n\tt13mr29318692lfc.161.1635331742094; \n\tWed, 27 Oct 2021 03:49:02 -0700 (PDT)","MIME-Version":"1.0","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-4-naush@raspberrypi.com>\n\t<163533126582.1184428.7446201495615796658@Monstersaurus>","In-Reply-To":"<163533126582.1184428.7446201495615796658@Monstersaurus>","From":"Naushir Patuck ","Date":"Wed, 27 Oct 2021 11:48:45 +0100","Message-ID":"","To":"Kieran Bingham ","Content-Type":"multipart/alternative; boundary=\"00000000000093eaf005cf535708\"","Subject":"Re: [libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format:\n\tRework BayerFormat conversion table","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20555,"web_url":"https://patchwork.libcamera.org/comment/20555/","msgid":"","date":"2021-10-27T10:54:58","subject":"Re: [libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format:\n\tRework BayerFormat conversion table","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"On Wed, 27 Oct 2021 at 11:48, Naushir Patuck wrote:\n\n> Hi Kieran,\n>\n> Thank you for your feedback.\n>\n> On Wed, 27 Oct 2021 at 11:41, Kieran Bingham <\n> kieran.bingham@ideasonboard.com> wrote:\n>\n>> Quoting Naushir Patuck (2021-10-27 10:27:57)\n>> > Rename the bayerToV4l2 conversion table to bayerToFormat. Update the\n>> table to\n>> > hold both the PixelFormat and V4L2PixelFormat conversions for a given\n>> > BayerFormat. This will allow converting between BayerFormat and\n>> PixelFormat\n>> > types in a subsequent change.\n>> >\n>> > Signed-off-by: Naushir Patuck \n>> > ---\n>> > src/libcamera/bayer_format.cpp | 110 ++++++++++++++++++++++-----------\n>> > 1 file changed, 73 insertions(+), 37 deletions(-)\n>> >\n>> > diff --git a/src/libcamera/bayer_format.cpp\n>> b/src/libcamera/bayer_format.cpp\n>> > index 11355f144f66..94e2294d7f6c 100644\n>> > --- a/src/libcamera/bayer_format.cpp\n>> > +++ b/src/libcamera/bayer_format.cpp\n>> > @@ -13,6 +13,7 @@\n>> >\n>> > #include \n>> >\n>> > +#include \n>> > #include \n>> >\n>> > /**\n>> > @@ -84,37 +85,72 @@ struct BayerFormatComparator {\n>> > }\n>> > };\n>> >\n>> > -const std::map\n>> bayerToV4l2{\n>> > - { { BayerFormat::BGGR, 8, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },\n>> > - { { BayerFormat::GBRG, 8, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },\n>> > - { { BayerFormat::GRBG, 8, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },\n>> > - { { BayerFormat::RGGB, 8, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },\n>> > - { { BayerFormat::BGGR, 10, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },\n>> > - { { BayerFormat::GBRG, 10, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },\n>> > - { { BayerFormat::GRBG, 10, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },\n>> > - { { BayerFormat::RGGB, 10, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },\n>> > - { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },\n>> > - { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },\n>> > - { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },\n>> > - { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },\n>> > - { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed },\n>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) },\n>> > - { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed },\n>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) },\n>> > - { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed },\n>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) },\n>> > - { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed },\n>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) },\n>> > - { { BayerFormat::BGGR, 12, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },\n>> > - { { BayerFormat::GBRG, 12, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },\n>> > - { { BayerFormat::GRBG, 12, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },\n>> > - { { BayerFormat::RGGB, 12, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },\n>> > - { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },\n>> > - { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },\n>> > - { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },\n>> > - { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },\n>> > - { { BayerFormat::BGGR, 16, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },\n>> > - { { BayerFormat::GBRG, 16, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },\n>> > - { { BayerFormat::GRBG, 16, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },\n>> > - { { BayerFormat::RGGB, 16, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },\n>> > - { { BayerFormat::MONO, 8, BayerFormat::None },\n>> V4L2PixelFormat(V4L2_PIX_FMT_GREY) },\n>> > - { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed },\n>> V4L2PixelFormat(V4L2_PIX_FMT_Y10P) },\n>> > +struct Formats {\n>> > + PixelFormat pixelFormat;\n>> > + V4L2PixelFormat v4l2Format;\n>> > +};\n>> > +\n>> > +const std::map\n>> bayerToFormat{\n>> > + { { BayerFormat::BGGR, 8, BayerFormat::None },\n>> > + { formats::SBGGR8, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8)\n>> } },\n>>\n>> Personally, I think tables should be kept tabular, disregarding the\n>> usual coding style line lengths.\n>>\n>> A table is much more readable with long lines to me ... but ... It may\n>> not be preferred to everyone. So no need to change this just for me.\n>> Just voicing my opinion ;-)\n>>\n>\n> I fully agree with the above!\n> I set it this way as we would go over the 120 char limit. Happy to format\n> it as a single line per row\n> as an exception.\n>\n>\n>>\n>>\n>> > + { { BayerFormat::GBRG, 8, BayerFormat::None },\n>> > + { formats::SGBRG8, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8)\n>> } },\n>> > + { { BayerFormat::GRBG, 8, BayerFormat::None },\n>> > + { formats::SGRBG8, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8)\n>> } },\n>> > + { { BayerFormat::RGGB, 8, BayerFormat::None },\n>> > + { formats::SRGGB8, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8)\n>> } },\n>> > + { { BayerFormat::BGGR, 10, BayerFormat::None },\n>> > + { formats::SBGGR10,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) } },\n>> > + { { BayerFormat::GBRG, 10, BayerFormat::None },\n>> > + { formats::SGBRG10,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) } },\n>> > + { { BayerFormat::GRBG, 10, BayerFormat::None },\n>> > + { formats::SGRBG10,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) } },\n>> > + { { BayerFormat::RGGB, 10, BayerFormat::None },\n>> > + { formats::SRGGB10,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) } },\n>> > + { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed },\n>> > + { formats::SBGGR10_CSI2P,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) } },\n>> > + { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed },\n>> > + { formats::SGBRG10_CSI2P,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) } },\n>> > + { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed },\n>> > + { formats::SGRBG10_CSI2P,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) } },\n>> > + { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed },\n>> > + { formats::SRGGB10_CSI2P,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) } },\n>> > + { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed },\n>> > + { formats::SBGGR10_IPU3,\n>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) } },\n>> > + { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed },\n>> > + { formats::SGBRG10_IPU3,\n>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) } },\n>> > + { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed },\n>> > + { formats::SGRBG10_IPU3,\n>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) } },\n>> > + { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed },\n>> > + { formats::SRGGB10_IPU3,\n>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) } },\n>> > + { { BayerFormat::BGGR, 12, BayerFormat::None },\n>> > + { formats::SBGGR12,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) } },\n>> > + { { BayerFormat::GBRG, 12, BayerFormat::None },\n>> > + { formats::SGBRG12,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) } },\n>> > + { { BayerFormat::GRBG, 12, BayerFormat::None },\n>> > + { formats::SGRBG12,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) } },\n>> > + { { BayerFormat::RGGB, 12, BayerFormat::None },\n>> > + { formats::SRGGB12,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) } },\n>> > + { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed },\n>> > + { formats::SBGGR12_CSI2P,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) } },\n>> > + { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed },\n>> > + { formats::SGBRG12_CSI2P,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) } },\n>> > + { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed },\n>> > + { formats::SGRBG12_CSI2P,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) } },\n>> > + { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed },\n>> > + { formats::SRGGB12_CSI2P,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) } },\n>> > + { { BayerFormat::BGGR, 16, BayerFormat::None },\n>> > + { formats::SBGGR16,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) } },\n>> > + { { BayerFormat::GBRG, 16, BayerFormat::None },\n>> > + { formats::SGBRG16,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) } },\n>> > + { { BayerFormat::GRBG, 16, BayerFormat::None },\n>> > + { formats::SGRBG16,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },\n>> > + { { BayerFormat::RGGB, 16, BayerFormat::None },\n>> > + { formats::SRGGB16,\n>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },\n>> > + { { BayerFormat::MONO, 8, BayerFormat::None },\n>> > + { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },\n>> > + { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed },\n>> > + { formats::R10, V4L2PixelFormat(V4L2_PIX_FMT_Y10P) } },\n>>\n>> In [2/9] formats::R10 gets added with V4L2_PIX_FMT_Y10, but here we have\n>> V4L2_PIX_FMT_Y10P. will that cause any issues?\n>>\n>\n> Quite right, this should not be assigned as a packed format. Will fix this.\n>\n\nActually looking at this again, we are going to need a separate\nformats:R10P packed version,\nas the mbus_code represents a packed format, and this cannot be\nrepresented with formats::R10.\nDo you and Laurent thnk it would be ok to for me to add this format patches\n1 and 2?\n\n\n>\n> Naush\n>\n>\n>>\n>>\n>> > };\n>> >\n>> > const std::unordered_map mbusCodeToBayer{\n>> > @@ -245,9 +281,9 @@ bool operator==(const BayerFormat &lhs, const\n>> BayerFormat &rhs)\n>> > */\n>> > V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const\n>> > {\n>> > - const auto it = bayerToV4l2.find(*this);\n>> > - if (it != bayerToV4l2.end())\n>> > - return it->second;\n>> > + const auto it = bayerToFormat.find(*this);\n>> > + if (it != bayerToFormat.end())\n>> > + return it->second.v4l2Format;\n>> >\n>> > return V4L2PixelFormat();\n>> > }\n>> > @@ -259,11 +295,11 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat()\n>> const\n>> > */\n>> > BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat\n>> v4l2Format)\n>> > {\n>> > - auto it = std::find_if(bayerToV4l2.begin(), bayerToV4l2.end(),\n>> > + auto it = std::find_if(bayerToFormat.begin(),\n>> bayerToFormat.end(),\n>> > [v4l2Format](const auto &i) {\n>> > - return i.second == v4l2Format;\n>> > + return i.second.v4l2Format ==\n>> v4l2Format;\n>> > });\n>> > - if (it != bayerToV4l2.end())\n>> > + if (it != bayerToFormat.end())\n>> > return it->first;\n>> >\n>> > return BayerFormat();\n>> > --\n>> > 2.25.1\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 464F6BDB1C\n\tfor ;\n\tWed, 27 Oct 2021 10:55:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 96A5664882;\n\tWed, 27 Oct 2021 12:55:16 +0200 (CEST)","from mail-lj1-x22a.google.com (mail-lj1-x22a.google.com\n\t[IPv6:2a00:1450:4864:20::22a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 424A860123\n\tfor ;\n\tWed, 27 Oct 2021 12:55:15 +0200 (CEST)","by mail-lj1-x22a.google.com with SMTP id u5so3951982ljo.8\n\tfor ;\n\tWed, 27 Oct 2021 03:55:15 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"dHw9xnkK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=1Uy2t0cX6m2UqG3dZTaIMdAwtj2xxQusomx0V5Zq6JQ=;\n\tb=dHw9xnkKg3cAUICSSAXKeGL8z5jqeKWeOz8jNXc98pMhseXjpJM7SFSPLLNLdhimDc\n\tRmeN95a+C0zA86cPbfCYA4vXVsHIGcX6Y2zHlNt68lkpVgsdFvUNmbOfi88qUTI7br3n\n\toqsyXxTwGu7kpfJT0WMXId07617vy2WwF4fo2yfqeIyLL56gkdp+EbNOePBCqBAhv9PU\n\tnYLKsXLmOeslgq3hfB/xFxw6oxd4d6GR3rE/12lUhVY6VOCrUMVAIZr34cxFK6g7VKqp\n\t/saWt9IqOw9da4ESG8p/do81y8w84FAmUkregPbgtk9TnH3KctHxKBi+EZs34nZPKkHu\n\tRPTQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=1Uy2t0cX6m2UqG3dZTaIMdAwtj2xxQusomx0V5Zq6JQ=;\n\tb=igpXTUuTfOHi9Qq3vQVpqH4N0bRuTOAvtv192INufuiK9m0r2c0RLaClvmUv3icoje\n\tQh24ByqxcedMqH7KI11DygkzJwTVtOCH9R+lvtSn1ddcB4gyoTkLWVKkXSyaKtb6+YtA\n\tIMvBB6EOUK3p5ZyoivMJ+GZkiKkFIzvf/xDVOPBvgto+6/8cK07qnGkofj+sEGgUuos1\n\tb2BeYTkVI9LpvX/GYN8GC9xtVnEvKdmU8neTxtAEhYclyekAQW/bf7nGnyk9WvQuJeOH\n\t2XbDch5wN0c0jn4WHJ8mQ9vm/Pgm+d7eLquBuaBO4s0n5vqCZdyO4ae9EbPFQUHNARJE\n\taFxA==","X-Gm-Message-State":"AOAM530keHtmhUcoXEIy16EC4mMpU0T7+ay4nrPSdIAdb22+bnLwwIiH\n\tjzvRew8tLRQRiLidAICeefNehl8MH8EC/hzeO8aIz1tiFBtpLQ==","X-Google-Smtp-Source":"ABdhPJxS8TJVPIpA2IRDY3+YkwfQYXiJYNrDIzxFHQBbrZ5GnPKjLacFdVjKh3Z2ln05zUdRLhdYzw04n59B/RgQVhA=","X-Received":"by 2002:a2e:968e:: with SMTP id\n\tq14mr3875514lji.280.1635332114484; \n\tWed, 27 Oct 2021 03:55:14 -0700 (PDT)","MIME-Version":"1.0","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-4-naush@raspberrypi.com>\n\t<163533126582.1184428.7446201495615796658@Monstersaurus>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 27 Oct 2021 11:54:58 +0100","Message-ID":"","To":"Kieran Bingham ","Content-Type":"multipart/alternative; boundary=\"000000000000c6264a05cf536ddc\"","Subject":"Re: [libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format:\n\tRework BayerFormat conversion table","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20556,"web_url":"https://patchwork.libcamera.org/comment/20556/","msgid":"","date":"2021-10-27T10:57:51","subject":"Re: [libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format:\n\tRework BayerFormat conversion table","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Oct 27, 2021 at 11:41:05AM +0100, Kieran Bingham wrote:\n> Quoting Naushir Patuck (2021-10-27 10:27:57)\n> > Rename the bayerToV4l2 conversion table to bayerToFormat. Update the table to\n> > hold both the PixelFormat and V4L2PixelFormat conversions for a given\n> > BayerFormat. This will allow converting between BayerFormat and PixelFormat\n> > types in a subsequent change.\n> > \n> > Signed-off-by: Naushir Patuck \n> > ---\n> > src/libcamera/bayer_format.cpp | 110 ++++++++++++++++++++++-----------\n> > 1 file changed, 73 insertions(+), 37 deletions(-)\n> > \n> > diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp\n> > index 11355f144f66..94e2294d7f6c 100644\n> > --- a/src/libcamera/bayer_format.cpp\n> > +++ b/src/libcamera/bayer_format.cpp\n> > @@ -13,6 +13,7 @@\n> > \n> > #include \n> > \n> > +#include \n> > #include \n> > \n> > /**\n> > @@ -84,37 +85,72 @@ struct BayerFormatComparator {\n> > }\n> > };\n> > \n> > -const std::map bayerToV4l2{\n> > - { { BayerFormat::BGGR, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },\n> > - { { BayerFormat::GBRG, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },\n> > - { { BayerFormat::GRBG, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },\n> > - { { BayerFormat::RGGB, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },\n> > - { { BayerFormat::BGGR, 10, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },\n> > - { { BayerFormat::GBRG, 10, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },\n> > - { { BayerFormat::GRBG, 10, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },\n> > - { { BayerFormat::RGGB, 10, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },\n> > - { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },\n> > - { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },\n> > - { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },\n> > - { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },\n> > - { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) },\n> > - { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) },\n> > - { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) },\n> > - { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) },\n> > - { { BayerFormat::BGGR, 12, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },\n> > - { { BayerFormat::GBRG, 12, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },\n> > - { { BayerFormat::GRBG, 12, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },\n> > - { { BayerFormat::RGGB, 12, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },\n> > - { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },\n> > - { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },\n> > - { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },\n> > - { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },\n> > - { { BayerFormat::BGGR, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },\n> > - { { BayerFormat::GBRG, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },\n> > - { { BayerFormat::GRBG, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },\n> > - { { BayerFormat::RGGB, 16, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },\n> > - { { BayerFormat::MONO, 8, BayerFormat::None }, V4L2PixelFormat(V4L2_PIX_FMT_GREY) },\n> > - { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed }, V4L2PixelFormat(V4L2_PIX_FMT_Y10P) },\n> > +struct Formats {\n> > + PixelFormat pixelFormat;\n> > + V4L2PixelFormat v4l2Format;\n> > +};\n> > +\n> > +const std::map bayerToFormat{\n> > + { { BayerFormat::BGGR, 8, BayerFormat::None },\n> > + { formats::SBGGR8, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) } },\n> \n> Personally, I think tables should be kept tabular, disregarding the\n> usual coding style line lengths.\n> \n> A table is much more readable with long lines to me ... but ... It may\n> not be preferred to everyone. So no need to change this just for me.\n> Just voicing my opinion ;-)\n\nI certainly don't mind going over 80 columns for this type of table, but\nit would get too long here.\n\n> > + { { BayerFormat::GBRG, 8, BayerFormat::None },\n> > + { formats::SGBRG8, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) } },\n> > + { { BayerFormat::GRBG, 8, BayerFormat::None },\n> > + { formats::SGRBG8, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) } },\n> > + { { BayerFormat::RGGB, 8, BayerFormat::None },\n> > + { formats::SRGGB8, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) } },\n> > + { { BayerFormat::BGGR, 10, BayerFormat::None },\n> > + { formats::SBGGR10, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) } },\n> > + { { BayerFormat::GBRG, 10, BayerFormat::None },\n> > + { formats::SGBRG10, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) } },\n> > + { { BayerFormat::GRBG, 10, BayerFormat::None },\n> > + { formats::SGRBG10, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) } },\n> > + { { BayerFormat::RGGB, 10, BayerFormat::None },\n> > + { formats::SRGGB10, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) } },\n> > + { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed },\n> > + { formats::SBGGR10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) } },\n> > + { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed },\n> > + { formats::SGBRG10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) } },\n> > + { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed },\n> > + { formats::SGRBG10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) } },\n> > + { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed },\n> > + { formats::SRGGB10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) } },\n> > + { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed },\n> > + { formats::SBGGR10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) } },\n> > + { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed },\n> > + { formats::SGBRG10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) } },\n> > + { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed },\n> > + { formats::SGRBG10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) } },\n> > + { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed },\n> > + { formats::SRGGB10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) } },\n> > + { { BayerFormat::BGGR, 12, BayerFormat::None },\n> > + { formats::SBGGR12, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) } },\n> > + { { BayerFormat::GBRG, 12, BayerFormat::None },\n> > + { formats::SGBRG12, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) } },\n> > + { { BayerFormat::GRBG, 12, BayerFormat::None },\n> > + { formats::SGRBG12, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) } },\n> > + { { BayerFormat::RGGB, 12, BayerFormat::None },\n> > + { formats::SRGGB12, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) } },\n> > + { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed },\n> > + { formats::SBGGR12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) } },\n> > + { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed },\n> > + { formats::SGBRG12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) } },\n> > + { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed },\n> > + { formats::SGRBG12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) } },\n> > + { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed },\n> > + { formats::SRGGB12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) } },\n> > + { { BayerFormat::BGGR, 16, BayerFormat::None },\n> > + { formats::SBGGR16, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) } },\n> > + { { BayerFormat::GBRG, 16, BayerFormat::None },\n> > + { formats::SGBRG16, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) } },\n> > + { { BayerFormat::GRBG, 16, BayerFormat::None },\n> > + { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },\n> > + { { BayerFormat::RGGB, 16, BayerFormat::None },\n> > + { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },\n> > + { { BayerFormat::MONO, 8, BayerFormat::None },\n> > + { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },\n> > + { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed },\n> > + { formats::R10, V4L2PixelFormat(V4L2_PIX_FMT_Y10P) } },\n> \n> In [2/9] formats::R10 gets added with V4L2_PIX_FMT_Y10, but here we have\n> V4L2_PIX_FMT_Y10P. will that cause any issues?\n\nformats::R10 is 10-bit data stored in 2 bytes. If we need to pack 4\npixels in 5 bytes, we should create formats::R10_CSI2P.\n\n> > };\n> > \n> > const std::unordered_map mbusCodeToBayer{\n> > @@ -245,9 +281,9 @@ bool operator==(const BayerFormat &lhs, const BayerFormat &rhs)\n> > */\n> > V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const\n> > {\n> > - const auto it = bayerToV4l2.find(*this);\n> > - if (it != bayerToV4l2.end())\n> > - return it->second;\n> > + const auto it = bayerToFormat.find(*this);\n> > + if (it != bayerToFormat.end())\n> > + return it->second.v4l2Format;\n> > \n> > return V4L2PixelFormat();\n> > }\n> > @@ -259,11 +295,11 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const\n> > */\n> > BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)\n> > {\n> > - auto it = std::find_if(bayerToV4l2.begin(), bayerToV4l2.end(),\n> > + auto it = std::find_if(bayerToFormat.begin(), bayerToFormat.end(),\n> > [v4l2Format](const auto &i) {\n> > - return i.second == v4l2Format;\n> > + return i.second.v4l2Format == v4l2Format;\n> > });\n> > - if (it != bayerToV4l2.end())\n> > + if (it != bayerToFormat.end())\n> > return it->first;\n> > \n> > return BayerFormat();","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 A1139BF415\n\tfor ;\n\tWed, 27 Oct 2021 10:58:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1D7DC6487F;\n\tWed, 27 Oct 2021 12:58:16 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 93B3360123\n\tfor ;\n\tWed, 27 Oct 2021 12:58:15 +0200 (CEST)","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 005D4596;\n\tWed, 27 Oct 2021 12:58:14 +0200 (CEST)"],"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=\"Aj0fx73m\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635332295;\n\tbh=/BYL5pJwSRW01T5sZsG36BXr51Q4hXP2/DwjcJA5zJc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Aj0fx73mKCdEZ3EE/1RKSrrVJRqqC+l+pbxXashWTE/ozCPizjkLtr3ZipyPe7qT9\n\tzy8HMY9ceV51o7mEEJ/4zp5d2UhZs4fQ6Y0u/RG+Ai10TZepMIhKV2xXM1naYQ49Ux\n\t/KcC9/BSA0tWLGjvbCGtU3awuUJslKi9jMv9To+w=","Date":"Wed, 27 Oct 2021 13:57:51 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-4-naush@raspberrypi.com>\n\t<163533126582.1184428.7446201495615796658@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<163533126582.1184428.7446201495615796658@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format:\n\tRework BayerFormat conversion table","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20563,"web_url":"https://patchwork.libcamera.org/comment/20563/","msgid":"","date":"2021-10-27T12:32:01","subject":"Re: [libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format:\n\tRework BayerFormat conversion table","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent amd Kieran,\n\nThanks for the feedback!\n\nOn Wed, 27 Oct 2021 at 11:58, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> On Wed, Oct 27, 2021 at 11:41:05AM +0100, Kieran Bingham wrote:\n> > Quoting Naushir Patuck (2021-10-27 10:27:57)\n> > > Rename the bayerToV4l2 conversion table to bayerToFormat. Update the\n> table to\n> > > hold both the PixelFormat and V4L2PixelFormat conversions for a given\n> > > BayerFormat. This will allow converting between BayerFormat and\n> PixelFormat\n> > > types in a subsequent change.\n> > >\n> > > Signed-off-by: Naushir Patuck \n> > > ---\n> > > src/libcamera/bayer_format.cpp | 110 ++++++++++++++++++++++-----------\n> > > 1 file changed, 73 insertions(+), 37 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/bayer_format.cpp\n> b/src/libcamera/bayer_format.cpp\n> > > index 11355f144f66..94e2294d7f6c 100644\n> > > --- a/src/libcamera/bayer_format.cpp\n> > > +++ b/src/libcamera/bayer_format.cpp\n> > > @@ -13,6 +13,7 @@\n> > >\n> > > #include \n> > >\n> > > +#include \n> > > #include \n> > >\n> > > /**\n> > > @@ -84,37 +85,72 @@ struct BayerFormatComparator {\n> > > }\n> > > };\n> > >\n> > > -const std::map\n> bayerToV4l2{\n> > > - { { BayerFormat::BGGR, 8, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },\n> > > - { { BayerFormat::GBRG, 8, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },\n> > > - { { BayerFormat::GRBG, 8, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },\n> > > - { { BayerFormat::RGGB, 8, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },\n> > > - { { BayerFormat::BGGR, 10, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },\n> > > - { { BayerFormat::GBRG, 10, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },\n> > > - { { BayerFormat::GRBG, 10, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },\n> > > - { { BayerFormat::RGGB, 10, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },\n> > > - { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },\n> > > - { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },\n> > > - { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },\n> > > - { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },\n> > > - { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) },\n> > > - { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) },\n> > > - { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) },\n> > > - { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) },\n> > > - { { BayerFormat::BGGR, 12, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },\n> > > - { { BayerFormat::GBRG, 12, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },\n> > > - { { BayerFormat::GRBG, 12, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },\n> > > - { { BayerFormat::RGGB, 12, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },\n> > > - { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },\n> > > - { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },\n> > > - { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },\n> > > - { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },\n> > > - { { BayerFormat::BGGR, 16, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },\n> > > - { { BayerFormat::GBRG, 16, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },\n> > > - { { BayerFormat::GRBG, 16, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },\n> > > - { { BayerFormat::RGGB, 16, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },\n> > > - { { BayerFormat::MONO, 8, BayerFormat::None },\n> V4L2PixelFormat(V4L2_PIX_FMT_GREY) },\n> > > - { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed },\n> V4L2PixelFormat(V4L2_PIX_FMT_Y10P) },\n> > > +struct Formats {\n> > > + PixelFormat pixelFormat;\n> > > + V4L2PixelFormat v4l2Format;\n> > > +};\n> > > +\n> > > +const std::map\n> bayerToFormat{\n> > > + { { BayerFormat::BGGR, 8, BayerFormat::None },\n> > > + { formats::SBGGR8,\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) } },\n> >\n> > Personally, I think tables should be kept tabular, disregarding the\n> > usual coding style line lengths.\n> >\n> > A table is much more readable with long lines to me ... but ... It may\n> > not be preferred to everyone. So no need to change this just for me.\n> > Just voicing my opinion ;-)\n>\n> I certainly don't mind going over 80 columns for this type of table, but\n> it would get too long here.\n\n\nFlattening the table would make some rows > 120 characters. Given this,\nperhaps\nI keep this reformatting.\n\n\n>\n> > > + { { BayerFormat::GBRG, 8, BayerFormat::None },\n> > > + { formats::SGBRG8,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) } },\n> > > + { { BayerFormat::GRBG, 8, BayerFormat::None },\n> > > + { formats::SGRBG8,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) } },\n> > > + { { BayerFormat::RGGB, 8, BayerFormat::None },\n> > > + { formats::SRGGB8,\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) } },\n> > > + { { BayerFormat::BGGR, 10, BayerFormat::None },\n> > > + { formats::SBGGR10,\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) } },\n> > > + { { BayerFormat::GBRG, 10, BayerFormat::None },\n> > > + { formats::SGBRG10,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) } },\n> > > + { { BayerFormat::GRBG, 10, BayerFormat::None },\n> > > + { formats::SGRBG10,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) } },\n> > > + { { BayerFormat::RGGB, 10, BayerFormat::None },\n> > > + { formats::SRGGB10,\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) } },\n> > > + { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed },\n> > > + { formats::SBGGR10_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) } },\n> > > + { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed },\n> > > + { formats::SGBRG10_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) } },\n> > > + { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed },\n> > > + { formats::SGRBG10_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) } },\n> > > + { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed },\n> > > + { formats::SRGGB10_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) } },\n> > > + { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed },\n> > > + { formats::SBGGR10_IPU3,\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) } },\n> > > + { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed },\n> > > + { formats::SGBRG10_IPU3,\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) } },\n> > > + { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed },\n> > > + { formats::SGRBG10_IPU3,\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) } },\n> > > + { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed },\n> > > + { formats::SRGGB10_IPU3,\n> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) } },\n> > > + { { BayerFormat::BGGR, 12, BayerFormat::None },\n> > > + { formats::SBGGR12,\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) } },\n> > > + { { BayerFormat::GBRG, 12, BayerFormat::None },\n> > > + { formats::SGBRG12,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) } },\n> > > + { { BayerFormat::GRBG, 12, BayerFormat::None },\n> > > + { formats::SGRBG12,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) } },\n> > > + { { BayerFormat::RGGB, 12, BayerFormat::None },\n> > > + { formats::SRGGB12,\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) } },\n> > > + { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed },\n> > > + { formats::SBGGR12_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) } },\n> > > + { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed },\n> > > + { formats::SGBRG12_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) } },\n> > > + { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed },\n> > > + { formats::SGRBG12_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) } },\n> > > + { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed },\n> > > + { formats::SRGGB12_CSI2P,\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) } },\n> > > + { { BayerFormat::BGGR, 16, BayerFormat::None },\n> > > + { formats::SBGGR16,\n> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) } },\n> > > + { { BayerFormat::GBRG, 16, BayerFormat::None },\n> > > + { formats::SGBRG16,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) } },\n> > > + { { BayerFormat::GRBG, 16, BayerFormat::None },\n> > > + { formats::SGRBG16,\n> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },\n> > > + { { BayerFormat::RGGB, 16, BayerFormat::None },\n> > > + { formats::SRGGB16,\n> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },\n> > > + { { BayerFormat::MONO, 8, BayerFormat::None },\n> > > + { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },\n> > > + { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed },\n> > > + { formats::R10, V4L2PixelFormat(V4L2_PIX_FMT_Y10P) } },\n> >\n> > In [2/9] formats::R10 gets added with V4L2_PIX_FMT_Y10, but here we have\n> > V4L2_PIX_FMT_Y10P. will that cause any issues?\n>\n> formats::R10 is 10-bit data stored in 2 bytes. If we need to pack 4\n> pixels in 5 bytes, we should create formats::R10_CSI2P.\n>\n\nWill add that format for the next revision.\n\nRegards,\nNaush\n\n\n>\n> > > };\n> > >\n> > > const std::unordered_map mbusCodeToBayer{\n> > > @@ -245,9 +281,9 @@ bool operator==(const BayerFormat &lhs, const\n> BayerFormat &rhs)\n> > > */\n> > > V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const\n> > > {\n> > > - const auto it = bayerToV4l2.find(*this);\n> > > - if (it != bayerToV4l2.end())\n> > > - return it->second;\n> > > + const auto it = bayerToFormat.find(*this);\n> > > + if (it != bayerToFormat.end())\n> > > + return it->second.v4l2Format;\n> > >\n> > > return V4L2PixelFormat();\n> > > }\n> > > @@ -259,11 +295,11 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat()\n> const\n> > > */\n> > > BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat\n> v4l2Format)\n> > > {\n> > > - auto it = std::find_if(bayerToV4l2.begin(), bayerToV4l2.end(),\n> > > + auto it = std::find_if(bayerToFormat.begin(),\n> bayerToFormat.end(),\n> > > [v4l2Format](const auto &i) {\n> > > - return i.second == v4l2Format;\n> > > + return i.second.v4l2Format ==\n> v4l2Format;\n> > > });\n> > > - if (it != bayerToV4l2.end())\n> > > + if (it != bayerToFormat.end())\n> > > return it->first;\n> > >\n> > > return BayerFormat();\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 EE7C7BDB1C\n\tfor ;\n\tWed, 27 Oct 2021 12:32:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3716B64871;\n\tWed, 27 Oct 2021 14:32:20 +0200 (CEST)","from mail-lf1-x130.google.com (mail-lf1-x130.google.com\n\t[IPv6:2a00:1450:4864:20::130])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D4EA060123\n\tfor ;\n\tWed, 27 Oct 2021 14:32:18 +0200 (CEST)","by mail-lf1-x130.google.com with SMTP id c28so5739728lfv.13\n\tfor ;\n\tWed, 27 Oct 2021 05:32:18 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"Ur2BUmJk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=74Ngn2K24f5hJwV5kY0lMfE0UIuKJggK332GoOFCQOA=;\n\tb=Ur2BUmJk86g5/kT5GL3+Y7KhuVT045vu/i/bOSoQYRVp2/CxqwF41IedFvBSFDZSfa\n\tow++8d3TR7dhQouFj7eXnJmORFvbZc1zggVt8O4FA+UbmPCSBgoKpON/MDCmKJ5kRtjK\n\teqr04JCm0T0SJT4oDWT2R6hmou7ckFQrrKJG4EqMyRIJP0Vw5/SoeWE6lNho9aAHfzKN\n\t1h9ouUfF+LmTnP13b0yjbQ+zBpA7l+MAnkdng4OxIa+o3Gx7Utfew7XRIHl9JaH2s/7m\n\txHpMFg1ZPPnZTOoo9GwE3UGiHFVRhX3MhNB1Z3+Yw33PTrWlAypnL1ONrmye6Y7IVAIy\n\tq32w==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=74Ngn2K24f5hJwV5kY0lMfE0UIuKJggK332GoOFCQOA=;\n\tb=AgOugEFoDKV1Fm9+8XwrCKApe7//FpXqv7JhWRdW95apW2jrpQng5/VOxUobrmUgFM\n\tQGxJIjRBZx512nUC23D2/TZI7qo86g/rwoXglV755CmUCeuDlyA5nxttm0Peej+8gmNd\n\taXG1EbByoWCIqoduVn25jHgczaItSLU/DWqAc+fvzJDvntKMLMQVFhu4aOlEfq1AfCmh\n\tolfb1eDm/tg4nXZrwBVI+OguaReexUAQ9Ozcx8hb+jFJmIyALx8q89WHf6Ra6jOtaYgq\n\t2LatgmjPa3ys0AyqF2lmWBUDu5UF9ulBPezQrfjQRW76WdrspqJmWAAfskSXZU3ErG3Y\n\taHgA==","X-Gm-Message-State":"AOAM530jSEnmtfW9jLQASgGL/LDFrRWNwBtryicbGGMfduU19h7NsBzo\n\tlu+3r/E8zy31MUF0HNkgaWe8Big7Ddo04pGT5A+YVA==","X-Google-Smtp-Source":"ABdhPJyG06ttZuVLF0LibMpiGYoVHPHBUZvTHgiNrIZWxUXalO9YL8/UVHSNINQjyLfOqQnigT2L2bAIE2ndZAO2TzA=","X-Received":"by 2002:ac2:5304:: with SMTP id\n\tc4mr20373176lfh.687.1635337937950; \n\tWed, 27 Oct 2021 05:32:17 -0700 (PDT)","MIME-Version":"1.0","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-4-naush@raspberrypi.com>\n\t<163533126582.1184428.7446201495615796658@Monstersaurus>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Wed, 27 Oct 2021 13:32:01 +0100","Message-ID":"","To":"Laurent Pinchart ","Content-Type":"multipart/alternative; boundary=\"000000000000e12af705cf54c856\"","Subject":"Re: [libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format:\n\tRework BayerFormat conversion table","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":20569,"web_url":"https://patchwork.libcamera.org/comment/20569/","msgid":"<163533833608.1184428.3634120655525350384@Monstersaurus>","date":"2021-10-27T12:38:56","subject":"Re: [libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format:\n\tRework BayerFormat conversion table","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Naushir Patuck (2021-10-27 11:54:58)\n> On Wed, 27 Oct 2021 at 11:48, Naushir Patuck wrote:\n> \n> > Hi Kieran,\n> >\n> > Thank you for your feedback.\n> >\n> > On Wed, 27 Oct 2021 at 11:41, Kieran Bingham <\n> > kieran.bingham@ideasonboard.com> wrote:\n> >\n> >> Quoting Naushir Patuck (2021-10-27 10:27:57)\n> >> > Rename the bayerToV4l2 conversion table to bayerToFormat. Update the\n> >> table to\n> >> > hold both the PixelFormat and V4L2PixelFormat conversions for a given\n> >> > BayerFormat. This will allow converting between BayerFormat and\n> >> PixelFormat\n> >> > types in a subsequent change.\n> >> >\n> >> > Signed-off-by: Naushir Patuck \n> >> > ---\n> >> > src/libcamera/bayer_format.cpp | 110 ++++++++++++++++++++++-----------\n> >> > 1 file changed, 73 insertions(+), 37 deletions(-)\n> >> >\n> >> > diff --git a/src/libcamera/bayer_format.cpp\n> >> b/src/libcamera/bayer_format.cpp\n> >> > index 11355f144f66..94e2294d7f6c 100644\n> >> > --- a/src/libcamera/bayer_format.cpp\n> >> > +++ b/src/libcamera/bayer_format.cpp\n> >> > @@ -13,6 +13,7 @@\n> >> >\n> >> > #include \n> >> >\n> >> > +#include \n> >> > #include \n> >> >\n> >> > /**\n> >> > @@ -84,37 +85,72 @@ struct BayerFormatComparator {\n> >> > }\n> >> > };\n> >> >\n> >> > -const std::map\n> >> bayerToV4l2{\n> >> > - { { BayerFormat::BGGR, 8, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },\n> >> > - { { BayerFormat::GBRG, 8, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },\n> >> > - { { BayerFormat::GRBG, 8, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },\n> >> > - { { BayerFormat::RGGB, 8, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },\n> >> > - { { BayerFormat::BGGR, 10, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },\n> >> > - { { BayerFormat::GBRG, 10, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },\n> >> > - { { BayerFormat::GRBG, 10, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },\n> >> > - { { BayerFormat::RGGB, 10, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },\n> >> > - { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },\n> >> > - { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },\n> >> > - { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },\n> >> > - { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },\n> >> > - { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) },\n> >> > - { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) },\n> >> > - { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) },\n> >> > - { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) },\n> >> > - { { BayerFormat::BGGR, 12, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },\n> >> > - { { BayerFormat::GBRG, 12, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },\n> >> > - { { BayerFormat::GRBG, 12, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },\n> >> > - { { BayerFormat::RGGB, 12, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },\n> >> > - { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },\n> >> > - { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },\n> >> > - { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },\n> >> > - { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },\n> >> > - { { BayerFormat::BGGR, 16, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },\n> >> > - { { BayerFormat::GBRG, 16, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },\n> >> > - { { BayerFormat::GRBG, 16, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },\n> >> > - { { BayerFormat::RGGB, 16, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },\n> >> > - { { BayerFormat::MONO, 8, BayerFormat::None },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_GREY) },\n> >> > - { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed },\n> >> V4L2PixelFormat(V4L2_PIX_FMT_Y10P) },\n> >> > +struct Formats {\n> >> > + PixelFormat pixelFormat;\n> >> > + V4L2PixelFormat v4l2Format;\n> >> > +};\n> >> > +\n> >> > +const std::map\n> >> bayerToFormat{\n> >> > + { { BayerFormat::BGGR, 8, BayerFormat::None },\n> >> > + { formats::SBGGR8, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8)\n> >> } },\n> >>\n> >> Personally, I think tables should be kept tabular, disregarding the\n> >> usual coding style line lengths.\n> >>\n> >> A table is much more readable with long lines to me ... but ... It may\n> >> not be preferred to everyone. So no need to change this just for me.\n> >> Just voicing my opinion ;-)\n> >>\n> >\n> > I fully agree with the above!\n> > I set it this way as we would go over the 120 char limit. Happy to format\n> > it as a single line per row\n> > as an exception.\n\nSeems Laurent prefers not to, as it would be too long ;-( I disagree but\nperhaps that is because my terminal is currently 194 chars wide ;-)\n\n> >>\n> >> > + { { BayerFormat::GBRG, 8, BayerFormat::None },\n> >> > + { formats::SGBRG8, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8)\n> >> } },\n> >> > + { { BayerFormat::GRBG, 8, BayerFormat::None },\n> >> > + { formats::SGRBG8, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8)\n> >> } },\n> >> > + { { BayerFormat::RGGB, 8, BayerFormat::None },\n> >> > + { formats::SRGGB8, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8)\n> >> } },\n> >> > + { { BayerFormat::BGGR, 10, BayerFormat::None },\n> >> > + { formats::SBGGR10,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) } },\n> >> > + { { BayerFormat::GBRG, 10, BayerFormat::None },\n> >> > + { formats::SGBRG10,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) } },\n> >> > + { { BayerFormat::GRBG, 10, BayerFormat::None },\n> >> > + { formats::SGRBG10,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) } },\n> >> > + { { BayerFormat::RGGB, 10, BayerFormat::None },\n> >> > + { formats::SRGGB10,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) } },\n> >> > + { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed },\n> >> > + { formats::SBGGR10_CSI2P,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) } },\n> >> > + { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed },\n> >> > + { formats::SGBRG10_CSI2P,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) } },\n> >> > + { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed },\n> >> > + { formats::SGRBG10_CSI2P,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) } },\n> >> > + { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed },\n> >> > + { formats::SRGGB10_CSI2P,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) } },\n> >> > + { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed },\n> >> > + { formats::SBGGR10_IPU3,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) } },\n> >> > + { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed },\n> >> > + { formats::SGBRG10_IPU3,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) } },\n> >> > + { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed },\n> >> > + { formats::SGRBG10_IPU3,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) } },\n> >> > + { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed },\n> >> > + { formats::SRGGB10_IPU3,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) } },\n> >> > + { { BayerFormat::BGGR, 12, BayerFormat::None },\n> >> > + { formats::SBGGR12,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) } },\n> >> > + { { BayerFormat::GBRG, 12, BayerFormat::None },\n> >> > + { formats::SGBRG12,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) } },\n> >> > + { { BayerFormat::GRBG, 12, BayerFormat::None },\n> >> > + { formats::SGRBG12,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) } },\n> >> > + { { BayerFormat::RGGB, 12, BayerFormat::None },\n> >> > + { formats::SRGGB12,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) } },\n> >> > + { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed },\n> >> > + { formats::SBGGR12_CSI2P,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) } },\n> >> > + { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed },\n> >> > + { formats::SGBRG12_CSI2P,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) } },\n> >> > + { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed },\n> >> > + { formats::SGRBG12_CSI2P,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) } },\n> >> > + { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed },\n> >> > + { formats::SRGGB12_CSI2P,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) } },\n> >> > + { { BayerFormat::BGGR, 16, BayerFormat::None },\n> >> > + { formats::SBGGR16,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) } },\n> >> > + { { BayerFormat::GBRG, 16, BayerFormat::None },\n> >> > + { formats::SGBRG16,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) } },\n> >> > + { { BayerFormat::GRBG, 16, BayerFormat::None },\n> >> > + { formats::SGRBG16,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },\n> >> > + { { BayerFormat::RGGB, 16, BayerFormat::None },\n> >> > + { formats::SRGGB16,\n> >> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },\n> >> > + { { BayerFormat::MONO, 8, BayerFormat::None },\n> >> > + { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },\n> >> > + { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed },\n> >> > + { formats::R10, V4L2PixelFormat(V4L2_PIX_FMT_Y10P) } },\n> >>\n> >> In [2/9] formats::R10 gets added with V4L2_PIX_FMT_Y10, but here we have\n> >> V4L2_PIX_FMT_Y10P. will that cause any issues?\n> >>\n> >\n> > Quite right, this should not be assigned as a packed format. Will fix this.\n> >\n> \n> Actually looking at this again, we are going to need a separate\n> formats:R10P packed version,\n> as the mbus_code represents a packed format, and this cannot be\n> represented with formats::R10.\n> Do you and Laurent thnk it would be ok to for me to add this format patches\n> 1 and 2?\n\nQuoting Laurent:\n\n│ formats::R10 is 10-bit data stored in 2 bytes. If we need to pack 4\n│ pixels in 5 bytes, we should create formats::R10_CSI2P.\n\nSo I think that's a yes. It could be added directly to those patches,\nbut that would give authorship to Laurent or split to show that you've \nadded the P variants. I suspect it doesn't matter too much either way\n;-)\n\nIf you update Laurent's patches add something like \n\n[Naush: Added formats::R10_CSI2P variant]\n\nabove your SoB.\n\n> \n> \n> >\n> > Naush\n> >\n> >\n> >>\n> >>\n> >> > };\n> >> >\n> >> > const std::unordered_map mbusCodeToBayer{\n> >> > @@ -245,9 +281,9 @@ bool operator==(const BayerFormat &lhs, const\n> >> BayerFormat &rhs)\n> >> > */\n> >> > V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const\n> >> > {\n> >> > - const auto it = bayerToV4l2.find(*this);\n> >> > - if (it != bayerToV4l2.end())\n> >> > - return it->second;\n> >> > + const auto it = bayerToFormat.find(*this);\n> >> > + if (it != bayerToFormat.end())\n> >> > + return it->second.v4l2Format;\n> >> >\n> >> > return V4L2PixelFormat();\n> >> > }\n> >> > @@ -259,11 +295,11 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat()\n> >> const\n> >> > */\n> >> > BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat\n> >> v4l2Format)\n> >> > {\n> >> > - auto it = std::find_if(bayerToV4l2.begin(), bayerToV4l2.end(),\n> >> > + auto it = std::find_if(bayerToFormat.begin(),\n> >> bayerToFormat.end(),\n> >> > [v4l2Format](const auto &i) {\n> >> > - return i.second == v4l2Format;\n> >> > + return i.second.v4l2Format ==\n> >> v4l2Format;\n> >> > });\n> >> > - if (it != bayerToV4l2.end())\n> >> > + if (it != bayerToFormat.end())\n> >> > return it->first;\n> >> >\n> >> > return BayerFormat();\n> >> > --\n> >> > 2.25.1\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 0E2D8BF415\n\tfor ;\n\tWed, 27 Oct 2021 12:39:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 39E0C6487F;\n\tWed, 27 Oct 2021 14:39:00 +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 4C2B260123\n\tfor ;\n\tWed, 27 Oct 2021 14:38:59 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D4BA1596;\n\tWed, 27 Oct 2021 14:38: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=\"bOpzwiA2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1635338338;\n\tbh=WkacBJ9zNxbDQm4tpjjJn0T5z1vyhge10LFdStq5QoI=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=bOpzwiA2OsbV7PsnMS+LcqxWerNk6TqOT0hueibRxhe/OW+iBfiaiMl97PuUdpF6E\n\tLKXaR3FshUG1unqnBA3TMtpUOq65/mSifxxTuZLcWCWAvqC6fo6jFZ7xUyo0aaii2T\n\tmkxawSBfZiVUlSbIcckNltV60t/TBGiNpZ396Sk8=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20211027092803.3671096-1-naush@raspberrypi.com>\n\t<20211027092803.3671096-4-naush@raspberrypi.com>\n\t<163533126582.1184428.7446201495615796658@Monstersaurus>\n\t\n\t","From":"Kieran Bingham ","To":"Naushir Patuck ","Date":"Wed, 27 Oct 2021 13:38:56 +0100","Message-ID":"<163533833608.1184428.3634120655525350384@Monstersaurus>","User-Agent":"alot/0.9.1","Subject":"Re: [libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format:\n\tRework BayerFormat conversion table","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 ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]