[{"id":12234,"web_url":"https://patchwork.libcamera.org/comment/12234/","msgid":"<20200901002516.GA16155@pendragon.ideasonboard.com>","date":"2020-09-01T00:25:16","subject":"Re: [libcamera-devel] [PATCH v5 4/8] libcamera: Allow Bayer pixel\n\tformats to be transformed","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nThank you for the patch.\n\nOn Sat, Aug 29, 2020 at 12:54:25PM +0100, David Plowman wrote:\n> Add a transform method to the V4L2PixelFormat class which allows Bayer\n> formats to be re-ordered into a different Bayer order when horizontal\n> or vertical flips are requested from the sensor.\n> \n> Signed-off-by: David Plowman \n> ---\n> include/libcamera/internal/v4l2_pixelformat.h | 3 +\n> src/libcamera/v4l2_pixelformat.cpp | 94 ++++++++++++++++++-\n> 2 files changed, 96 insertions(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h\n> index 9bfd81a..cddac86 100644\n> --- a/include/libcamera/internal/v4l2_pixelformat.h\n> +++ b/include/libcamera/internal/v4l2_pixelformat.h\n> @@ -14,6 +14,7 @@\n> #include \n> \n> #include \n> +#include \n\nYou can use a forward declaration of enum class Transform in this file,\nand include transform.h in v4l2_pixelformat.cpp.\n\n> \n> namespace libcamera {\n> \n> @@ -40,6 +41,8 @@ public:\n> \tstatic V4L2PixelFormat fromPixelFormat(const PixelFormat &pixelFormat,\n> \t\t\t\t\t bool multiplanar);\n> \n> +\tV4L2PixelFormat transform(Transform t) const;\n> +\n> private:\n> \tuint32_t fourcc_;\n> };\n> diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp\n> index 30c94bb..28bcd51 100644\n> --- a/src/libcamera/v4l2_pixelformat.cpp\n> +++ b/src/libcamera/v4l2_pixelformat.cpp\n> @@ -101,6 +101,62 @@ const std::map vpf2pf{\n> \t{ V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), formats::MJPEG },\n> };\n> \n> +/* Table giving the result of applying an hflip to each Bayer pixel format. */\n> +const std::map hflipTable{\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },\n> +};\n> +\n> +/* Table giving the result of applying a vflip to each Bayer pixel format. */\n> +const std::map vflipTable{\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },\n> +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },\n> +};\n> +\n> } /* namespace */\n> \n> /**\n> @@ -113,7 +169,7 @@ const std::map vpf2pf{\n> \n> /**\n> * \\fn V4L2PixelFormat::V4L2PixelFormat(uint32_t fourcc)\n> - * \\brief Construct a V4L2PixelFormat from a FourCC value\n> +p * \\brief Construct a V4L2PixelFormat from a FourCC value\n\nProbably an unwanted addition ?\n\n> * \\param[in] fourcc The pixel format FourCC numerical value\n> */\n> \n> @@ -205,4 +261,40 @@ V4L2PixelFormat V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat,\n> \treturn info.v4l2Format;\n> }\n> \n> +/**\n> + * \\brief Transform a Bayer V4L2PixelFormat.\n> + * \\param[in] t The Transform to be applied to the pixel format.\n> + *\n> + * Some sensors that produce Bayer V4L2PixelFormats may change their Bayer\n> + * order when the V4L2_CID_HFLIP or V4L2_CID_VFLIP controls are applied to the\n> + * device. For such sensors, this function computes the new V4L2PixelFormat\n> + * for the transformed Bayer order.\n> + *\n> + * Transposes are ignored as sensors do not implement them.\n> + *\n> + * \\return The Bayer V4L2PixelFormat resulting from applying transform \\a t\n> + * to the given V4L2PixelFormat.\n> + */\n> +V4L2PixelFormat V4L2PixelFormat::transform(Transform t) const\n> +{\n> +\tV4L2PixelFormat result = *this;\n> +\n> +\tif (!!(t & Transform::HFlip)) {\n> +\t\tconst auto iter = hflipTable.find(*this);\n> +\t\tif (iter == hflipTable.end()) {\n> +\t\t\tLOG(V4L2, Warning)\n> +\t\t\t\t<< \"Cannot transpose V4L2 pixel format \"\n> +\t\t\t\t<< toString();\n> +\t\t\treturn V4L2PixelFormat();\n> +\t\t}\n> +\t\tresult = iter->second;\n> +\t}\n> +\n> +\t/* This will be found as the tables have the same keys. */\n\nExcept that if HFlip isn't set, the above check will not have run.\n\nIn order to simplify this a bit, and avoid two lookups, could we merge\nthe two tables ? We can store a\n\nstruct BayerFlipResult {\n\tV4L2PixelFormat hflip;\n\tV4L2PixelFormat vflip;\n\tV4L2PixelFormat hvflip;\n};\n\nin the table, and this function could then become\n\n\tt &= ~Transform::Transpose;\n\tif (!t)\n\t\treturn *this;\n\n\tconst auto iter = bayerFlipTable.find(*this);\n\tif (iter == bayerFlipTable.end()) {\n\t\tLOG(V4L2, Warning)\n\t\t\t<< \"Cannot transpose V4L2 pixel format \" << toString();\n\t\treturn *this;\n\t}\n\n\tconst BayerFlipResult &flip = iter->second;\n\n\tswitch (t) {\n\tcase Transform::HFlip:\n\t\treturn flip.hflip;\n\tcase Transform::VFlip:\n\t\treturn flip.vflip;\n\tcase Transform::HVFlip:\n\tdefault:\n\t\treturn flip.hvflip;\n\t}\n\nWe would need a\n\nconstexpr Transform operator~(Transform t)\n{\n\treturn static_cast(~static_cast(t) & 7);\n}\n\nAlternatively the first line could become\n\n\tt &= (Transform::HFlip | TransformVFlip);\n\nThe downside is that the table would become a bit less readable:\n\nconst std::map bayerFlipTable{\n\t{\n\t\tV4L2PixelFormat(V4L2_PIX_FMT_SBGGR8),\n\t\t{\n\t\t\t.hflip = V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8),\n\t\t\t.vflip = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8),\n\t\t\t.hvflip = V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8),\n\t\t},\n\t}, {\n\t\tV4L2PixelFormat(V4L2_PIX_FMT_SGBRG8),\n\t\t{\n\t\t\t.hflip = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8),\n\t\t\t.vflip = V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8),\n\t\t\t.hvflip = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8),\n\t\t},\n\t}, {\n\t...\n};\n\nbut that's not that bad I think.\n\nNote that in the future we are considering not expressing the bayer\npattern as part of the DRM fourcc, but using DRM_FORMAT_R8 and conveying\nthe Bayer pattern separately. It would be nice if we could handle all\nthis at the PixelFormat level instead, we could avoid a large table, but\nI fear that won't be possible as we can't change the V4L2 side.\n\n> +\tif (!!(t & Transform::VFlip))\n> +\t\tresult = vflipTable.find(result)->second;\n> +\n> +\treturn result;\n> +}\n> +\n> } /* namespace libcamera */","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 B0F57BF019\n\tfor ;\n\tTue, 1 Sep 2020 00:25:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4294562920;\n\tTue, 1 Sep 2020 02:25:53 +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 290D66037B\n\tfor ;\n\tTue, 1 Sep 2020 02:25:52 +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 5617A277;\n\tTue, 1 Sep 2020 02:25:38 +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=\"VPfzMAaB\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598919938;\n\tbh=DMEmM+lY/DirpiLvNMGr98RZqwoTNbHKc7jEu05bJSw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VPfzMAaB51eVNXhfZof1da9aCY3Bab9Dku1NH6EvgO8nzYpjB5Q3Ii6uL9HxUWp6+\n\tP27+p1TowxVKmFjZoAl1EVowHrq94YwBLJxPKAG9SQlR7J25LA2oXiK0YJ8teSckdK\n\tZ/EEPF2YmKXbvFAu8M4505lmo8EGAYvubG8227Ps=","Date":"Tue, 1 Sep 2020 03:25:16 +0300","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"<20200901002516.GA16155@pendragon.ideasonboard.com>","References":"<20200829115429.30010-1-david.plowman@raspberrypi.com>\n\t<20200829115429.30010-5-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200829115429.30010-5-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v5 4/8] libcamera: Allow Bayer pixel\n\tformats to be transformed","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":12235,"web_url":"https://patchwork.libcamera.org/comment/12235/","msgid":"<20200901002823.GB16155@pendragon.ideasonboard.com>","date":"2020-09-01T00:28:23","subject":"Re: [libcamera-devel] [PATCH v5 4/8] libcamera: Allow Bayer pixel\n\tformats to be transformed","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nI forgot to review the documentation, sorry.\n\nOn Tue, Sep 01, 2020 at 03:25:23AM +0300, Laurent Pinchart wrote:\n> On Sat, Aug 29, 2020 at 12:54:25PM +0100, David Plowman wrote:\n> > Add a transform method to the V4L2PixelFormat class which allows Bayer\n> > formats to be re-ordered into a different Bayer order when horizontal\n> > or vertical flips are requested from the sensor.\n> > \n> > Signed-off-by: David Plowman \n> > ---\n> > include/libcamera/internal/v4l2_pixelformat.h | 3 +\n> > src/libcamera/v4l2_pixelformat.cpp | 94 ++++++++++++++++++-\n> > 2 files changed, 96 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/include/libcamera/internal/v4l2_pixelformat.h b/include/libcamera/internal/v4l2_pixelformat.h\n> > index 9bfd81a..cddac86 100644\n> > --- a/include/libcamera/internal/v4l2_pixelformat.h\n> > +++ b/include/libcamera/internal/v4l2_pixelformat.h\n> > @@ -14,6 +14,7 @@\n> > #include \n> > \n> > #include \n> > +#include \n> \n> You can use a forward declaration of enum class Transform in this file,\n> and include transform.h in v4l2_pixelformat.cpp.\n> \n> > \n> > namespace libcamera {\n> > \n> > @@ -40,6 +41,8 @@ public:\n> > \tstatic V4L2PixelFormat fromPixelFormat(const PixelFormat &pixelFormat,\n> > \t\t\t\t\t bool multiplanar);\n> > \n> > +\tV4L2PixelFormat transform(Transform t) const;\n> > +\n> > private:\n> > \tuint32_t fourcc_;\n> > };\n> > diff --git a/src/libcamera/v4l2_pixelformat.cpp b/src/libcamera/v4l2_pixelformat.cpp\n> > index 30c94bb..28bcd51 100644\n> > --- a/src/libcamera/v4l2_pixelformat.cpp\n> > +++ b/src/libcamera/v4l2_pixelformat.cpp\n> > @@ -101,6 +101,62 @@ const std::map vpf2pf{\n> > \t{ V4L2PixelFormat(V4L2_PIX_FMT_MJPEG), formats::MJPEG },\n> > };\n> > \n> > +/* Table giving the result of applying an hflip to each Bayer pixel format. */\n> > +const std::map hflipTable{\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },\n> > +};\n> > +\n> > +/* Table giving the result of applying a vflip to each Bayer pixel format. */\n> > +const std::map vflipTable{\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16), V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16), V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16), V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },\n> > +\t{ V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16), V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },\n> > +};\n> > +\n> > } /* namespace */\n> > \n> > /**\n> > @@ -113,7 +169,7 @@ const std::map vpf2pf{\n> > \n> > /**\n> > * \\fn V4L2PixelFormat::V4L2PixelFormat(uint32_t fourcc)\n> > - * \\brief Construct a V4L2PixelFormat from a FourCC value\n> > +p * \\brief Construct a V4L2PixelFormat from a FourCC value\n> \n> Probably an unwanted addition ?\n> \n> > * \\param[in] fourcc The pixel format FourCC numerical value\n> > */\n> > \n> > @@ -205,4 +261,40 @@ V4L2PixelFormat V4L2PixelFormat::fromPixelFormat(const PixelFormat &pixelFormat,\n> > \treturn info.v4l2Format;\n> > }\n> > \n> > +/**\n> > + * \\brief Transform a Bayer V4L2PixelFormat.\n> > + * \\param[in] t The Transform to be applied to the pixel format.\n\nNo need for a trailing . for \\brief and \\param.\ns/pixel format/V4L2 pixel format/\n\n> > + *\n> > + * Some sensors that produce Bayer V4L2PixelFormats may change their Bayer\n\ns/V4L2PixelFormats/formats/ (we try not to pluralize class names)\n\n> > + * order when the V4L2_CID_HFLIP or V4L2_CID_VFLIP controls are applied to the\n> > + * device. For such sensors, this function computes the new V4L2PixelFormat\n> > + * for the transformed Bayer order.\n> > + *\n> > + * Transposes are ignored as sensors do not implement them.\n> > + *\n> > + * \\return The Bayer V4L2PixelFormat resulting from applying transform \\a t\n> > + * to the given V4L2PixelFormat.\n> > + */\n> > +V4L2PixelFormat V4L2PixelFormat::transform(Transform t) const\n> > +{\n> > +\tV4L2PixelFormat result = *this;\n> > +\n> > +\tif (!!(t & Transform::HFlip)) {\n> > +\t\tconst auto iter = hflipTable.find(*this);\n> > +\t\tif (iter == hflipTable.end()) {\n> > +\t\t\tLOG(V4L2, Warning)\n> > +\t\t\t\t<< \"Cannot transpose V4L2 pixel format \"\n> > +\t\t\t\t<< toString();\n> > +\t\t\treturn V4L2PixelFormat();\n> > +\t\t}\n> > +\t\tresult = iter->second;\n> > +\t}\n> > +\n> > +\t/* This will be found as the tables have the same keys. */\n> \n> Except that if HFlip isn't set, the above check will not have run.\n> \n> In order to simplify this a bit, and avoid two lookups, could we merge\n> the two tables ? We can store a\n> \n> struct BayerFlipResult {\n> \tV4L2PixelFormat hflip;\n> \tV4L2PixelFormat vflip;\n> \tV4L2PixelFormat hvflip;\n> };\n> \n> in the table, and this function could then become\n> \n> \tt &= ~Transform::Transpose;\n> \tif (!t)\n> \t\treturn *this;\n> \n> \tconst auto iter = bayerFlipTable.find(*this);\n> \tif (iter == bayerFlipTable.end()) {\n> \t\tLOG(V4L2, Warning)\n> \t\t\t<< \"Cannot transpose V4L2 pixel format \" << toString();\n> \t\treturn *this;\n> \t}\n> \n> \tconst BayerFlipResult &flip = iter->second;\n> \n> \tswitch (t) {\n> \tcase Transform::HFlip:\n> \t\treturn flip.hflip;\n> \tcase Transform::VFlip:\n> \t\treturn flip.vflip;\n> \tcase Transform::HVFlip:\n> \tdefault:\n> \t\treturn flip.hvflip;\n> \t}\n> \n> We would need a\n> \n> constexpr Transform operator~(Transform t)\n> {\n> \treturn static_cast(~static_cast(t) & 7);\n> }\n> \n> Alternatively the first line could become\n> \n> \tt &= (Transform::HFlip | TransformVFlip);\n> \n> The downside is that the table would become a bit less readable:\n> \n> const std::map bayerFlipTable{\n> \t{\n> \t\tV4L2PixelFormat(V4L2_PIX_FMT_SBGGR8),\n> \t\t{\n> \t\t\t.hflip = V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8),\n> \t\t\t.vflip = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8),\n> \t\t\t.hvflip = V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8),\n> \t\t},\n> \t}, {\n> \t\tV4L2PixelFormat(V4L2_PIX_FMT_SGBRG8),\n> \t\t{\n> \t\t\t.hflip = V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8),\n> \t\t\t.vflip = V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8),\n> \t\t\t.hvflip = V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8),\n> \t\t},\n> \t}, {\n> \t...\n> };\n> \n> but that's not that bad I think.\n> \n> Note that in the future we are considering not expressing the bayer\n> pattern as part of the DRM fourcc, but using DRM_FORMAT_R8 and conveying\n> the Bayer pattern separately. It would be nice if we could handle all\n> this at the PixelFormat level instead, we could avoid a large table, but\n> I fear that won't be possible as we can't change the V4L2 side.\n> \n> > +\tif (!!(t & Transform::VFlip))\n> > +\t\tresult = vflipTable.find(result)->second;\n> > +\n> > +\treturn result;\n> > +}\n> > +\n> > } /* namespace libcamera */","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 5B34ABF019\n\tfor ;\n\tTue, 1 Sep 2020 00:28:56 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D3BC762920;\n\tTue, 1 Sep 2020 02:28:55 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 508346037B\n\tfor ;\n\tTue, 1 Sep 2020 02:28:54 +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 95FB5277;\n\tTue, 1 Sep 2020 02:28:44 +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=\"v+6pfVKy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598920124;\n\tbh=Rh95KU4hfoh1ikfjZUvPwgTQ1TTb4pFYryHeGM8uw4k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=v+6pfVKyjw6tbRxdPiBkdDQzDy8cZqFOckGxexJpJ2H3q7kZF36l/lnY/PMJR245j\n\t3steMFe7UOF6JRb5G8mEG+T6Yt9wM4EkTLWeJFz59dmVtww2BQ42RDqGXdAoBGxqkJ\n\tiMe2xeUFLeK/991CRw1rWbWxTFt40viM08bXdhdc=","Date":"Tue, 1 Sep 2020 03:28:23 +0300","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"<20200901002823.GB16155@pendragon.ideasonboard.com>","References":"<20200829115429.30010-1-david.plowman@raspberrypi.com>\n\t<20200829115429.30010-5-david.plowman@raspberrypi.com>\n\t<20200901002516.GA16155@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200901002516.GA16155@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 4/8] libcamera: Allow Bayer pixel\n\tformats to be transformed","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\" "}}]