<div dir="ltr"><div dir="ltr">Hi Kieran,<div><br></div><div>Thank you for your feedback.</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, 28 Oct 2021 at 13:37, Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com">kieran.bingham@ideasonboard.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Quoting Naushir Patuck (2021-10-28 09:46:41)<br>
> Rename the bayerToV4l2 conversion table to bayerToFormat. Update the table to<br>
> hold both the PixelFormat and V4L2PixelFormat conversions for a given<br>
> BayerFormat. This will allow converting between BayerFormat and PixelFormat<br>
> types in a subsequent change.<br>
> <br>
> Signed-off-by: Naushir Patuck <<a href="mailto:naush@raspberrypi.com" target="_blank">naush@raspberrypi.com</a>><br>
> ---<br>
> src/libcamera/bayer_format.cpp | 112 ++++++++++++++++++++++-----------<br>
> 1 file changed, 75 insertions(+), 37 deletions(-)<br>
> <br>
> diff --git a/src/libcamera/bayer_format.cpp b/src/libcamera/bayer_format.cpp<br>
> index 1c1e849a7e31..8665a025f7e0 100644<br>
> --- a/src/libcamera/bayer_format.cpp<br>
> +++ b/src/libcamera/bayer_format.cpp<br>
> @@ -13,6 +13,7 @@<br>
> <br>
> #include <linux/media-bus-format.h><br>
> <br>
> +#include <libcamera/formats.h><br>
> #include <libcamera/transform.h><br>
> <br>
> /**<br>
> @@ -84,37 +85,74 @@ struct BayerFormatComparator {<br>
> }<br>
> };<br>
> <br>
> -const std::map<BayerFormat, V4L2PixelFormat, BayerFormatComparator> bayerToV4l2{<br>
> - { { BayerFormat::BGGR, 8, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },<br>
> - { { BayerFormat::GBRG, 8, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },<br>
> - { { BayerFormat::GRBG, 8, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },<br>
> - { { BayerFormat::RGGB, 8, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },<br>
> - { { BayerFormat::BGGR, 10, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },<br>
> - { { BayerFormat::GBRG, 10, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },<br>
> - { { BayerFormat::GRBG, 10, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },<br>
> - { { BayerFormat::RGGB, 10, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },<br>
> - { { BayerFormat::BGGR, 10, BayerFormat::Packing::CSI2 }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },<br>
> - { { BayerFormat::GBRG, 10, BayerFormat::Packing::CSI2 }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },<br>
> - { { BayerFormat::GRBG, 10, BayerFormat::Packing::CSI2 }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },<br>
> - { { BayerFormat::RGGB, 10, BayerFormat::Packing::CSI2 }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },<br>
> - { { BayerFormat::BGGR, 10, BayerFormat::Packing::IPU3 }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) },<br>
> - { { BayerFormat::GBRG, 10, BayerFormat::Packing::IPU3 }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) },<br>
> - { { BayerFormat::GRBG, 10, BayerFormat::Packing::IPU3 }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) },<br>
> - { { BayerFormat::RGGB, 10, BayerFormat::Packing::IPU3 }, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) },<br>
> - { { BayerFormat::BGGR, 12, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },<br>
> - { { BayerFormat::GBRG, 12, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },<br>
> - { { BayerFormat::GRBG, 12, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },<br>
> - { { BayerFormat::RGGB, 12, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },<br>
> - { { BayerFormat::BGGR, 12, BayerFormat::Packing::CSI2 }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },<br>
> - { { BayerFormat::GBRG, 12, BayerFormat::Packing::CSI2 }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },<br>
> - { { BayerFormat::GRBG, 12, BayerFormat::Packing::CSI2 }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },<br>
> - { { BayerFormat::RGGB, 12, BayerFormat::Packing::CSI2 }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },<br>
> - { { BayerFormat::BGGR, 16, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },<br>
> - { { BayerFormat::GBRG, 16, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },<br>
> - { { BayerFormat::GRBG, 16, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },<br>
> - { { BayerFormat::RGGB, 16, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },<br>
> - { { BayerFormat::MONO, 8, BayerFormat::Packing::None }, V4L2PixelFormat(V4L2_PIX_FMT_GREY) },<br>
> - { { BayerFormat::MONO, 10, BayerFormat::Packing::CSI2 }, V4L2PixelFormat(V4L2_PIX_FMT_Y10P) },<br>
> +struct Formats {<br>
> + PixelFormat pixelFormat;<br>
> + V4L2PixelFormat v4l2Format;<br>
> +};<br>
> +<br>
> +const std::map<BayerFormat, Formats, BayerFormatComparator> bayerToFormat{<br>
> + { { BayerFormat::BGGR, 8, BayerFormat::Packing::None },<br>
> + { formats::SBGGR8, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) } },<br>
> + { { BayerFormat::GBRG, 8, BayerFormat::Packing::None },<br>
> + { formats::SGBRG8, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) } },<br>
> + { { BayerFormat::GRBG, 8, BayerFormat::Packing::None },<br>
> + { formats::SGRBG8, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) } },<br>
> + { { BayerFormat::RGGB, 8, BayerFormat::Packing::None },<br>
> + { formats::SRGGB8, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) } },<br>
> + { { BayerFormat::BGGR, 10, BayerFormat::Packing::None },<br>
> + { formats::SBGGR10, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) } },<br>
> + { { BayerFormat::GBRG, 10, BayerFormat::Packing::None },<br>
> + { formats::SGBRG10, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) } },<br>
> + { { BayerFormat::GRBG, 10, BayerFormat::Packing::None },<br>
> + { formats::SGRBG10, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) } },<br>
> + { { BayerFormat::RGGB, 10, BayerFormat::Packing::None },<br>
> + { formats::SRGGB10, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) } },<br>
> + { { BayerFormat::BGGR, 10, BayerFormat::Packing::CSI2 },<br>
> + { formats::SBGGR10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) } },<br>
> + { { BayerFormat::GBRG, 10, BayerFormat::Packing::CSI2 },<br>
> + { formats::SGBRG10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) } },<br>
> + { { BayerFormat::GRBG, 10, BayerFormat::Packing::CSI2 },<br>
> + { formats::SGRBG10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) } },<br>
> + { { BayerFormat::RGGB, 10, BayerFormat::Packing::CSI2 },<br>
> + { formats::SRGGB10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) } },<br>
> + { { BayerFormat::BGGR, 10, BayerFormat::Packing::IPU3 },<br>
> + { formats::SBGGR10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) } },<br>
> + { { BayerFormat::GBRG, 10, BayerFormat::Packing::IPU3 },<br>
> + { formats::SGBRG10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) } },<br>
> + { { BayerFormat::GRBG, 10, BayerFormat::Packing::IPU3 },<br>
> + { formats::SGRBG10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) } },<br>
> + { { BayerFormat::RGGB, 10, BayerFormat::Packing::IPU3 },<br>
> + { formats::SRGGB10_IPU3, V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) } },<br>
> + { { BayerFormat::BGGR, 12, BayerFormat::Packing::None },<br>
> + { formats::SBGGR12, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) } },<br>
> + { { BayerFormat::GBRG, 12, BayerFormat::Packing::None },<br>
> + { formats::SGBRG12, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) } },<br>
> + { { BayerFormat::GRBG, 12, BayerFormat::Packing::None },<br>
> + { formats::SGRBG12, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) } },<br>
> + { { BayerFormat::RGGB, 12, BayerFormat::Packing::None },<br>
> + { formats::SRGGB12, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) } },<br>
> + { { BayerFormat::BGGR, 12, BayerFormat::Packing::CSI2 },<br>
> + { formats::SBGGR12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) } },<br>
> + { { BayerFormat::GBRG, 12, BayerFormat::Packing::CSI2 },<br>
> + { formats::SGBRG12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) } },<br>
> + { { BayerFormat::GRBG, 12, BayerFormat::Packing::CSI2 },<br>
> + { formats::SGRBG12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) } },<br>
> + { { BayerFormat::RGGB, 12, BayerFormat::Packing::CSI2 },<br>
> + { formats::SRGGB12_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) } },<br>
> + { { BayerFormat::BGGR, 16, BayerFormat::Packing::None },<br>
> + { formats::SBGGR16, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) } },<br>
> + { { BayerFormat::GBRG, 16, BayerFormat::Packing::None },<br>
> + { formats::SGBRG16, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) } },<br>
> + { { BayerFormat::GRBG, 16, BayerFormat::Packing::None },<br>
> + { formats::SGRBG16, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },<br>
> + { { BayerFormat::RGGB, 16, BayerFormat::Packing::None },<br>
> + { formats::SRGGB16, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },<br>
> + { { BayerFormat::MONO, 8, BayerFormat::Packing::None },<br>
> + { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },<br>
> + { { BayerFormat::MONO, 10, BayerFormat::Packing::None },<br>
> + { formats::R10, V4L2PixelFormat(V4L2_PIX_FMT_Y10) } },<br>
<br>
This isn't in the original table, so this addition should be highlighted<br>
clearly.<br>
<br>
Ideally it would have been a patch on it's own to provide the addition,<br>
either before the conversion, or after. But as long as it's mentioned in<br>
the commit message, I'm fine with it being here.<br>
<br>
We could add:<br>
<br>
"""<br>
While updating the table, the unpacked V4L2_PIX_FMT_Y10 is mapped to the<br>
equivalent BayerFormat and formats::R10 accordingly.<br>
"""<br>
<br>
If we get there in this series, that could be added while applying.<br>
<br>
So with one of those solutions:<br></blockquote><div><br></div><div>I'll split this into 2 changes to be clearer.</div><div><br></div><div>Regards,</div><div>Naush</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
<br>
Reviewed-by: Kieran Bingham <<a href="mailto:kieran.bingham@ideasonboard.com" target="_blank">kieran.bingham@ideasonboard.com</a>><br>
<br>
> + { { BayerFormat::MONO, 10, BayerFormat::Packing::CSI2 },<br>
> + { formats::R10_CSI2P, V4L2PixelFormat(V4L2_PIX_FMT_Y10P) } },<br>
> };<br>
> <br>
> const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{<br>
> @@ -245,9 +283,9 @@ bool operator==(const BayerFormat &lhs, const BayerFormat &rhs)<br>
> */<br>
> V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const<br>
> {<br>
> - const auto it = bayerToV4l2.find(*this);<br>
> - if (it != bayerToV4l2.end())<br>
> - return it->second;<br>
> + const auto it = bayerToFormat.find(*this);<br>
> + if (it != bayerToFormat.end())<br>
> + return it->second.v4l2Format;<br>
> <br>
> return V4L2PixelFormat();<br>
> }<br>
> @@ -259,11 +297,11 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const<br>
> */<br>
> BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat v4l2Format)<br>
> {<br>
> - auto it = std::find_if(bayerToV4l2.begin(), bayerToV4l2.end(),<br>
> + auto it = std::find_if(bayerToFormat.begin(), bayerToFormat.end(),<br>
> [v4l2Format](const auto &i) {<br>
> - return i.second == v4l2Format;<br>
> + return i.second.v4l2Format == v4l2Format;<br>
> });<br>
> - if (it != bayerToV4l2.end())<br>
> + if (it != bayerToFormat.end())<br>
> return it->first;<br>
> <br>
> return BayerFormat();<br>
> -- <br>
> 2.25.1<br>
><br>
</blockquote></div></div>