[libcamera-devel] [PATCH v3 3/9] libcamera: bayer_format: Rework BayerFormat conversion table
Naushir Patuck
naush at raspberrypi.com
Wed Oct 27 12:54:58 CEST 2021
On Wed, 27 Oct 2021 at 11:48, Naushir Patuck <naush at raspberrypi.com> wrote:
> Hi Kieran,
>
> Thank you for your feedback.
>
> On Wed, 27 Oct 2021 at 11:41, Kieran Bingham <
> kieran.bingham at ideasonboard.com> wrote:
>
>> Quoting Naushir Patuck (2021-10-27 10:27:57)
>> > Rename the bayerToV4l2 conversion table to bayerToFormat. Update the
>> table to
>> > hold both the PixelFormat and V4L2PixelFormat conversions for a given
>> > BayerFormat. This will allow converting between BayerFormat and
>> PixelFormat
>> > types in a subsequent change.
>> >
>> > Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
>> > ---
>> > src/libcamera/bayer_format.cpp | 110 ++++++++++++++++++++++-----------
>> > 1 file changed, 73 insertions(+), 37 deletions(-)
>> >
>> > diff --git a/src/libcamera/bayer_format.cpp
>> b/src/libcamera/bayer_format.cpp
>> > index 11355f144f66..94e2294d7f6c 100644
>> > --- a/src/libcamera/bayer_format.cpp
>> > +++ b/src/libcamera/bayer_format.cpp
>> > @@ -13,6 +13,7 @@
>> >
>> > #include <linux/media-bus-format.h>
>> >
>> > +#include <libcamera/formats.h>
>> > #include <libcamera/transform.h>
>> >
>> > /**
>> > @@ -84,37 +85,72 @@ struct BayerFormatComparator {
>> > }
>> > };
>> >
>> > -const std::map<BayerFormat, V4L2PixelFormat, BayerFormatComparator>
>> bayerToV4l2{
>> > - { { BayerFormat::BGGR, 8, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8) },
>> > - { { BayerFormat::GBRG, 8, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8) },
>> > - { { BayerFormat::GRBG, 8, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8) },
>> > - { { BayerFormat::RGGB, 8, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8) },
>> > - { { BayerFormat::BGGR, 10, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) },
>> > - { { BayerFormat::GBRG, 10, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) },
>> > - { { BayerFormat::GRBG, 10, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) },
>> > - { { BayerFormat::RGGB, 10, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) },
>> > - { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed },
>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) },
>> > - { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed },
>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) },
>> > - { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed },
>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) },
>> > - { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed },
>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) },
>> > - { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed },
>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) },
>> > - { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed },
>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) },
>> > - { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed },
>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) },
>> > - { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed },
>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) },
>> > - { { BayerFormat::BGGR, 12, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) },
>> > - { { BayerFormat::GBRG, 12, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) },
>> > - { { BayerFormat::GRBG, 12, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) },
>> > - { { BayerFormat::RGGB, 12, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) },
>> > - { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed },
>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) },
>> > - { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed },
>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) },
>> > - { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed },
>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) },
>> > - { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed },
>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) },
>> > - { { BayerFormat::BGGR, 16, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) },
>> > - { { BayerFormat::GBRG, 16, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) },
>> > - { { BayerFormat::GRBG, 16, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) },
>> > - { { BayerFormat::RGGB, 16, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) },
>> > - { { BayerFormat::MONO, 8, BayerFormat::None },
>> V4L2PixelFormat(V4L2_PIX_FMT_GREY) },
>> > - { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed },
>> V4L2PixelFormat(V4L2_PIX_FMT_Y10P) },
>> > +struct Formats {
>> > + PixelFormat pixelFormat;
>> > + V4L2PixelFormat v4l2Format;
>> > +};
>> > +
>> > +const std::map<BayerFormat, Formats, BayerFormatComparator>
>> bayerToFormat{
>> > + { { BayerFormat::BGGR, 8, BayerFormat::None },
>> > + { formats::SBGGR8, V4L2PixelFormat(V4L2_PIX_FMT_SBGGR8)
>> } },
>>
>> Personally, I think tables should be kept tabular, disregarding the
>> usual coding style line lengths.
>>
>> A table is much more readable with long lines to me ... but ... It may
>> not be preferred to everyone. So no need to change this just for me.
>> Just voicing my opinion ;-)
>>
>
> I fully agree with the above!
> I set it this way as we would go over the 120 char limit. Happy to format
> it as a single line per row
> as an exception.
>
>
>>
>>
>> > + { { BayerFormat::GBRG, 8, BayerFormat::None },
>> > + { formats::SGBRG8, V4L2PixelFormat(V4L2_PIX_FMT_SGBRG8)
>> } },
>> > + { { BayerFormat::GRBG, 8, BayerFormat::None },
>> > + { formats::SGRBG8, V4L2PixelFormat(V4L2_PIX_FMT_SGRBG8)
>> } },
>> > + { { BayerFormat::RGGB, 8, BayerFormat::None },
>> > + { formats::SRGGB8, V4L2PixelFormat(V4L2_PIX_FMT_SRGGB8)
>> } },
>> > + { { BayerFormat::BGGR, 10, BayerFormat::None },
>> > + { formats::SBGGR10,
>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10) } },
>> > + { { BayerFormat::GBRG, 10, BayerFormat::None },
>> > + { formats::SGBRG10,
>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10) } },
>> > + { { BayerFormat::GRBG, 10, BayerFormat::None },
>> > + { formats::SGRBG10,
>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10) } },
>> > + { { BayerFormat::RGGB, 10, BayerFormat::None },
>> > + { formats::SRGGB10,
>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10) } },
>> > + { { BayerFormat::BGGR, 10, BayerFormat::CSI2Packed },
>> > + { formats::SBGGR10_CSI2P,
>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR10P) } },
>> > + { { BayerFormat::GBRG, 10, BayerFormat::CSI2Packed },
>> > + { formats::SGBRG10_CSI2P,
>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG10P) } },
>> > + { { BayerFormat::GRBG, 10, BayerFormat::CSI2Packed },
>> > + { formats::SGRBG10_CSI2P,
>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG10P) } },
>> > + { { BayerFormat::RGGB, 10, BayerFormat::CSI2Packed },
>> > + { formats::SRGGB10_CSI2P,
>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB10P) } },
>> > + { { BayerFormat::BGGR, 10, BayerFormat::IPU3Packed },
>> > + { formats::SBGGR10_IPU3,
>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SBGGR10) } },
>> > + { { BayerFormat::GBRG, 10, BayerFormat::IPU3Packed },
>> > + { formats::SGBRG10_IPU3,
>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGBRG10) } },
>> > + { { BayerFormat::GRBG, 10, BayerFormat::IPU3Packed },
>> > + { formats::SGRBG10_IPU3,
>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SGRBG10) } },
>> > + { { BayerFormat::RGGB, 10, BayerFormat::IPU3Packed },
>> > + { formats::SRGGB10_IPU3,
>> V4L2PixelFormat(V4L2_PIX_FMT_IPU3_SRGGB10) } },
>> > + { { BayerFormat::BGGR, 12, BayerFormat::None },
>> > + { formats::SBGGR12,
>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12) } },
>> > + { { BayerFormat::GBRG, 12, BayerFormat::None },
>> > + { formats::SGBRG12,
>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12) } },
>> > + { { BayerFormat::GRBG, 12, BayerFormat::None },
>> > + { formats::SGRBG12,
>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12) } },
>> > + { { BayerFormat::RGGB, 12, BayerFormat::None },
>> > + { formats::SRGGB12,
>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12) } },
>> > + { { BayerFormat::BGGR, 12, BayerFormat::CSI2Packed },
>> > + { formats::SBGGR12_CSI2P,
>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR12P) } },
>> > + { { BayerFormat::GBRG, 12, BayerFormat::CSI2Packed },
>> > + { formats::SGBRG12_CSI2P,
>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG12P) } },
>> > + { { BayerFormat::GRBG, 12, BayerFormat::CSI2Packed },
>> > + { formats::SGRBG12_CSI2P,
>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG12P) } },
>> > + { { BayerFormat::RGGB, 12, BayerFormat::CSI2Packed },
>> > + { formats::SRGGB12_CSI2P,
>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB12P) } },
>> > + { { BayerFormat::BGGR, 16, BayerFormat::None },
>> > + { formats::SBGGR16,
>> V4L2PixelFormat(V4L2_PIX_FMT_SBGGR16) } },
>> > + { { BayerFormat::GBRG, 16, BayerFormat::None },
>> > + { formats::SGBRG16,
>> V4L2PixelFormat(V4L2_PIX_FMT_SGBRG16) } },
>> > + { { BayerFormat::GRBG, 16, BayerFormat::None },
>> > + { formats::SGRBG16,
>> V4L2PixelFormat(V4L2_PIX_FMT_SGRBG16) } },
>> > + { { BayerFormat::RGGB, 16, BayerFormat::None },
>> > + { formats::SRGGB16,
>> V4L2PixelFormat(V4L2_PIX_FMT_SRGGB16) } },
>> > + { { BayerFormat::MONO, 8, BayerFormat::None },
>> > + { formats::R8, V4L2PixelFormat(V4L2_PIX_FMT_GREY) } },
>> > + { { BayerFormat::MONO, 10, BayerFormat::CSI2Packed },
>> > + { formats::R10, V4L2PixelFormat(V4L2_PIX_FMT_Y10P) } },
>>
>> In [2/9] formats::R10 gets added with V4L2_PIX_FMT_Y10, but here we have
>> V4L2_PIX_FMT_Y10P. will that cause any issues?
>>
>
> Quite right, this should not be assigned as a packed format. Will fix this.
>
Actually looking at this again, we are going to need a separate
formats:R10P packed version,
as the mbus_code represents a packed format, and this cannot be
represented with formats::R10.
Do you and Laurent thnk it would be ok to for me to add this format patches
1 and 2?
>
> Naush
>
>
>>
>>
>> > };
>> >
>> > const std::unordered_map<unsigned int, BayerFormat> mbusCodeToBayer{
>> > @@ -245,9 +281,9 @@ bool operator==(const BayerFormat &lhs, const
>> BayerFormat &rhs)
>> > */
>> > V4L2PixelFormat BayerFormat::toV4L2PixelFormat() const
>> > {
>> > - const auto it = bayerToV4l2.find(*this);
>> > - if (it != bayerToV4l2.end())
>> > - return it->second;
>> > + const auto it = bayerToFormat.find(*this);
>> > + if (it != bayerToFormat.end())
>> > + return it->second.v4l2Format;
>> >
>> > return V4L2PixelFormat();
>> > }
>> > @@ -259,11 +295,11 @@ V4L2PixelFormat BayerFormat::toV4L2PixelFormat()
>> const
>> > */
>> > BayerFormat BayerFormat::fromV4L2PixelFormat(V4L2PixelFormat
>> v4l2Format)
>> > {
>> > - auto it = std::find_if(bayerToV4l2.begin(), bayerToV4l2.end(),
>> > + auto it = std::find_if(bayerToFormat.begin(),
>> bayerToFormat.end(),
>> > [v4l2Format](const auto &i) {
>> > - return i.second == v4l2Format;
>> > + return i.second.v4l2Format ==
>> v4l2Format;
>> > });
>> > - if (it != bayerToV4l2.end())
>> > + if (it != bayerToFormat.end())
>> > return it->first;
>> >
>> > return BayerFormat();
>> > --
>> > 2.25.1
>> >
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20211027/43e20809/attachment-0001.htm>
More information about the libcamera-devel
mailing list