[libcamera-devel] [PATCH v2] libcamera: v4l2subdev: Print mbus string instead of code
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Jun 1 14:02:45 CEST 2020
Hi Kaaira,
On 01/06/2020 12:01, Kaaira Gupta wrote:
> Modify toString() to print mbus format string instead of its hex code as
> the string is easier to understand.
>
> Signed-off-by: Kaaira Gupta <kgupta at es.iitr.ac.in>
> ---
>
> Changes since v1:
> Add check for unsupported format.
> Rename struct
>
> src/libcamera/v4l2_subdevice.cpp | 172 +++++++++++++++++--------------
> 1 file changed, 93 insertions(+), 79 deletions(-)
>
> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp
> index 7aefc1b..e97982e 100644
> --- a/src/libcamera/v4l2_subdevice.cpp
> +++ b/src/libcamera/v4l2_subdevice.cpp
> @@ -35,84 +35,93 @@ LOG_DECLARE_CATEGORY(V4L2)
>
> namespace {
>
> +/*
For doxygen documentation comments, there should be two *'s:
/**
> + * \var mbusFormatInfo
> + * \brief A struct of bits per pixel and mbus format
We wouldn't normally describe a struct as "a struct" in the brief. The
type tells us that already.
Just saying what the data represented by it is enough.
> + */
> +struct mbusFormatInfo {
> + unsigned int bits;
> + std::string format;
> +};
> +
> /*
> * \var formatInfoMap
> - * \brief A map that associates bits per pixel to V4L2 media bus codes
> + * \brief A map that associates mbusFormatInfo struct to V4L2 media bus codes
> */
> -const std::map<uint32_t, unsigned int> formatInfoMap = {
> - { V4L2_MBUS_FMT_RGB444_2X8_PADHI_BE, 16 },
> - { V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE, 16 },
> - { V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE, 16 },
> - { V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE, 16 },
> - { V4L2_MBUS_FMT_BGR565_2X8_BE, 16 },
> - { V4L2_MBUS_FMT_BGR565_2X8_LE, 16 },
> - { V4L2_MBUS_FMT_RGB565_2X8_BE, 16 },
> - { V4L2_MBUS_FMT_RGB565_2X8_LE, 16 },
> - { V4L2_MBUS_FMT_RGB666_1X18, 18 },
> - { V4L2_MBUS_FMT_RGB888_1X24, 24 },
> - { V4L2_MBUS_FMT_RGB888_2X12_BE, 24 },
> - { V4L2_MBUS_FMT_RGB888_2X12_LE, 24 },
> - { V4L2_MBUS_FMT_ARGB8888_1X32, 32 },
> - { V4L2_MBUS_FMT_Y8_1X8, 8 },
> - { V4L2_MBUS_FMT_UV8_1X8, 8 },
> - { V4L2_MBUS_FMT_UYVY8_1_5X8, 12 },
> - { V4L2_MBUS_FMT_VYUY8_1_5X8, 12 },
> - { V4L2_MBUS_FMT_YUYV8_1_5X8, 12 },
> - { V4L2_MBUS_FMT_YVYU8_1_5X8, 12 },
> - { V4L2_MBUS_FMT_UYVY8_2X8, 16 },
> - { V4L2_MBUS_FMT_VYUY8_2X8, 16 },
> - { V4L2_MBUS_FMT_YUYV8_2X8, 16 },
> - { V4L2_MBUS_FMT_YVYU8_2X8, 16 },
> - { V4L2_MBUS_FMT_Y10_1X10, 10 },
> - { V4L2_MBUS_FMT_UYVY10_2X10, 20 },
> - { V4L2_MBUS_FMT_VYUY10_2X10, 20 },
> - { V4L2_MBUS_FMT_YUYV10_2X10, 20 },
> - { V4L2_MBUS_FMT_YVYU10_2X10, 20 },
> - { V4L2_MBUS_FMT_Y12_1X12, 12 },
> - { V4L2_MBUS_FMT_UYVY8_1X16, 16 },
> - { V4L2_MBUS_FMT_VYUY8_1X16, 16 },
> - { V4L2_MBUS_FMT_YUYV8_1X16, 16 },
> - { V4L2_MBUS_FMT_YVYU8_1X16, 16 },
> - { V4L2_MBUS_FMT_YDYUYDYV8_1X16, 16 },
> - { V4L2_MBUS_FMT_UYVY10_1X20, 20 },
> - { V4L2_MBUS_FMT_VYUY10_1X20, 20 },
> - { V4L2_MBUS_FMT_YUYV10_1X20, 20 },
> - { V4L2_MBUS_FMT_YVYU10_1X20, 20 },
> - { V4L2_MBUS_FMT_YUV10_1X30, 30 },
> - { V4L2_MBUS_FMT_AYUV8_1X32, 32 },
> - { V4L2_MBUS_FMT_UYVY12_2X12, 24 },
> - { V4L2_MBUS_FMT_VYUY12_2X12, 24 },
> - { V4L2_MBUS_FMT_YUYV12_2X12, 24 },
> - { V4L2_MBUS_FMT_YVYU12_2X12, 24 },
> - { V4L2_MBUS_FMT_UYVY12_1X24, 24 },
> - { V4L2_MBUS_FMT_VYUY12_1X24, 24 },
> - { V4L2_MBUS_FMT_YUYV12_1X24, 24 },
> - { V4L2_MBUS_FMT_YVYU12_1X24, 24 },
> - { V4L2_MBUS_FMT_SBGGR8_1X8, 8 },
> - { V4L2_MBUS_FMT_SGBRG8_1X8, 8 },
> - { V4L2_MBUS_FMT_SGRBG8_1X8, 8 },
> - { V4L2_MBUS_FMT_SRGGB8_1X8, 8 },
> - { V4L2_MBUS_FMT_SBGGR10_ALAW8_1X8, 8 },
> - { V4L2_MBUS_FMT_SGBRG10_ALAW8_1X8, 8 },
> - { V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8, 8 },
> - { V4L2_MBUS_FMT_SRGGB10_ALAW8_1X8, 8 },
> - { V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8, 8 },
> - { V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8, 8 },
> - { V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, 8 },
> - { V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8, 8 },
> - { V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE, 16 },
> - { V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE, 16 },
> - { V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE, 16 },
> - { V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE, 16 },
> - { V4L2_MBUS_FMT_SBGGR10_1X10, 10 },
> - { V4L2_MBUS_FMT_SGBRG10_1X10, 10 },
> - { V4L2_MBUS_FMT_SGRBG10_1X10, 10 },
> - { V4L2_MBUS_FMT_SRGGB10_1X10, 10 },
> - { V4L2_MBUS_FMT_SBGGR12_1X12, 12 },
> - { V4L2_MBUS_FMT_SGBRG12_1X12, 12 },
> - { V4L2_MBUS_FMT_SGRBG12_1X12, 12 },
> - { V4L2_MBUS_FMT_SRGGB12_1X12, 12 },
> - { V4L2_MBUS_FMT_AHSV8888_1X32, 32 },
> +const std::map<uint32_t, mbusFormatInfo> formatInfoMap = {
> + { V4L2_MBUS_FMT_RGB444_2X8_PADHI_BE, { 16, "RGB444_2X8_PADHI_BE" } },
> + { V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE, { 16, "RGB444_2X8_PADHI_LE" } },
> + { V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE, { 16, "RGB555_2X8_PADHI_BE" } },
> + { V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE, { 16, "RGB555_2X8_PADHI_LE" } },
> + { V4L2_MBUS_FMT_BGR565_2X8_BE, { 16, "BGR565_2X8_BE" } },
> + { V4L2_MBUS_FMT_BGR565_2X8_LE, { 16, "BGR565_2X8_LE" } },
> + { V4L2_MBUS_FMT_RGB565_2X8_BE, { 16, "RGB565_2X8_BE" } },
> + { V4L2_MBUS_FMT_RGB565_2X8_LE, { 16, "RGB565_2X8_LE" } },
> + { V4L2_MBUS_FMT_RGB666_1X18, { 18, "RGB666_1X18" } },
> + { V4L2_MBUS_FMT_RGB888_1X24, { 24, "RGB888_1X24" } },
> + { V4L2_MBUS_FMT_RGB888_2X12_BE, { 24, "RGB888_2X12_BE" } },
> + { V4L2_MBUS_FMT_RGB888_2X12_LE, { 24, "RGB888_2X12_LE" } },
> + { V4L2_MBUS_FMT_ARGB8888_1X32, { 32, "ARGB8888_1X32" } },
> + { V4L2_MBUS_FMT_Y8_1X8, { 8, "Y8_1X8" } },
> + { V4L2_MBUS_FMT_UV8_1X8, { 8, "UV8_1X8" } },
> + { V4L2_MBUS_FMT_UYVY8_1_5X8, { 12, "UYVY8_1_5X8" } },
> + { V4L2_MBUS_FMT_VYUY8_1_5X8, { 12, "VYUY8_1_5X8" } },
> + { V4L2_MBUS_FMT_YUYV8_1_5X8, { 12, "YUYV8_1_5X8" } },
> + { V4L2_MBUS_FMT_YVYU8_1_5X8, { 12, "YVYU8_1_5X8" } },
> + { V4L2_MBUS_FMT_UYVY8_2X8, { 16, "UYVY8_2X8" } },
> + { V4L2_MBUS_FMT_VYUY8_2X8, { 16, "VYUY8_2X8" } },
> + { V4L2_MBUS_FMT_YUYV8_2X8, { 16, "YUYV8_2X8" } },
> + { V4L2_MBUS_FMT_YVYU8_2X8, { 16, "YVYU8_2X8" } },
> + { V4L2_MBUS_FMT_Y10_1X10, { 10, "Y10_1X10" } },
> + { V4L2_MBUS_FMT_UYVY10_2X10, { 20, "UYVY10_2X10" } },
> + { V4L2_MBUS_FMT_VYUY10_2X10, { 20, "VYUY10_2X10" } },
> + { V4L2_MBUS_FMT_YUYV10_2X10, { 20, "YUYV10_2X10" } },
> + { V4L2_MBUS_FMT_YVYU10_2X10, { 20, "YVYU10_2X10" } },
> + { V4L2_MBUS_FMT_Y12_1X12, { 12, "Y12_1X12" } },
> + { V4L2_MBUS_FMT_UYVY8_1X16, { 16, "UYVY8_1X16" } },
> + { V4L2_MBUS_FMT_VYUY8_1X16, { 16, "VYUY8_1X16" } },
> + { V4L2_MBUS_FMT_YUYV8_1X16, { 16, "YUYV8_1X16" } },
> + { V4L2_MBUS_FMT_YVYU8_1X16, { 16, "YVYU8_1X16" } },
> + { V4L2_MBUS_FMT_YDYUYDYV8_1X16, { 16, "YDYUYDYV8_1X16" } },
> + { V4L2_MBUS_FMT_UYVY10_1X20, { 20, "UYVY10_1X20" } },
> + { V4L2_MBUS_FMT_VYUY10_1X20, { 20, "VYUY10_1X20" } },
> + { V4L2_MBUS_FMT_YUYV10_1X20, { 20, "YUYV10_1X20" } },
> + { V4L2_MBUS_FMT_YVYU10_1X20, { 20, "YVYU10_1X20" } },
> + { V4L2_MBUS_FMT_YUV10_1X30, { 30, "YUV10_1X30" } },
> + { V4L2_MBUS_FMT_AYUV8_1X32, { 32, "AYUV8_1X32" } },
> + { V4L2_MBUS_FMT_UYVY12_2X12, { 24, "UYVY12_2X12" } },
> + { V4L2_MBUS_FMT_VYUY12_2X12, { 24, "VYUY12_2X12" } },
> + { V4L2_MBUS_FMT_YUYV12_2X12, { 24, "YUYV12_2X12" } },
> + { V4L2_MBUS_FMT_YVYU12_2X12, { 24, "YVYU12_2X12" } },
> + { V4L2_MBUS_FMT_UYVY12_1X24, { 24, "UYVY12_1X24" } },
> + { V4L2_MBUS_FMT_VYUY12_1X24, { 24, "VYUY12_1X24" } },
> + { V4L2_MBUS_FMT_YUYV12_1X24, { 24, "YUYV12_1X24" } },
> + { V4L2_MBUS_FMT_YVYU12_1X24, { 24, "YVYU12_1X24" } },
> + { V4L2_MBUS_FMT_SBGGR8_1X8, { 8, "SBGGR8_1X8" } },
> + { V4L2_MBUS_FMT_SGBRG8_1X8, { 8, "SGBRG8_1X8" } },
> + { V4L2_MBUS_FMT_SGRBG8_1X8, { 8, "SGRBG8_1X8" } },
> + { V4L2_MBUS_FMT_SRGGB8_1X8, { 8, "SRGGB8_1X8" } },
> + { V4L2_MBUS_FMT_SBGGR10_ALAW8_1X8, { 8, "SBGGR10_ALAW8_1X8" } },
> + { V4L2_MBUS_FMT_SGBRG10_ALAW8_1X8, { 8, "SGBRG10_ALAW8_1X8" } },
> + { V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8, { 8, "SGRBG10_ALAW8_1X8" } },
> + { V4L2_MBUS_FMT_SRGGB10_ALAW8_1X8, { 8, "SRGGB10_ALAW8_1X8" } },
> + { V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8, { 8, "SBGGR10_DPCM8_1X8" } },
> + { V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8, { 8, "SGBRG10_DPCM8_1X8" } },
> + { V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, { 8, "SGRBG10_DPCM8_1X8" } },
> + { V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8, { 8, "SRGGB10_DPCM8_1X8" } },
> + { V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE, { 16, "SBGGR10_2X8_PADHI_BE" } },
> + { V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE, { 16, "SBGGR10_2X8_PADHI_LE" } },
> + { V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE, { 16, "SBGGR10_2X8_PADLO_BE" } },
> + { V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE, { 16, "SBGGR10_2X8_PADLO_LE" } },
> + { V4L2_MBUS_FMT_SBGGR10_1X10, { 10, "SBGGR10_1X10" } },
> + { V4L2_MBUS_FMT_SGBRG10_1X10, { 10, "SGBRG10_1X10" } },
> + { V4L2_MBUS_FMT_SGRBG10_1X10, { 10, "SGRBG10_1X10" } },
> + { V4L2_MBUS_FMT_SRGGB10_1X10, { 10, "SRGGB10_1X10" } },
> + { V4L2_MBUS_FMT_SBGGR12_1X12, { 12, "SBGGR12_1X12" } },
> + { V4L2_MBUS_FMT_SGBRG12_1X12, { 12, "SGBRG12_1X12" } },
> + { V4L2_MBUS_FMT_SGRBG12_1X12, { 12, "SGRBG12_1X12" } },
> + { V4L2_MBUS_FMT_SRGGB12_1X12, { 12, "SRGGB12_1X12" } },
> + { V4L2_MBUS_FMT_AHSV8888_1X32, { 32, "AHSV8888_1X32" } },
> };
>
> } /* namespace */
> @@ -161,9 +170,14 @@ const std::map<uint32_t, unsigned int> formatInfoMap = {
> */
> const std::string V4L2SubdeviceFormat::toString() const
> {
> - std::stringstream ss;
> - ss << size.toString() << "-" << utils::hex(mbus_code, 4);
> - return ss.str();
> + const auto it = formatInfoMap.find(mbus_code);
> + std::stringstream mbus;
And this last one is only a stylistic 'nit' and is quite minor, if a
function grows in complexity we would normally put extra spacing in to
ease readability.
The original function was three lines, so it was packed without spaces
to keep it short, but here we now have three parts.
{
Local Variables
Code
Return statement.
}
You've already got a space between the Code and return, so to match it
should probably have a space between the local variables and code.
and then, because the auto iterator calls formatInfoMap.find() I'd put
that after the mbus instantiation...
No need to send a v3 though - I'm just discussing stylistic things there.
Otherwise, this patch looks good to me, so if we get another reviewed-by
tag, I can apply with the small style fixups if you are happy.
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
--
Kieran
> + if (it == formatInfoMap.end())
> + mbus << size.toString() << "-" << utils::hex(mbus_code, 4);
> + else
> + mbus << size.toString() << "-" << it->second.format;
> +
> + return mbus.str();
> }
>
> /**
> @@ -180,7 +194,7 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const
> return 0;
> }
>
> - return it->second;
> + return it->second.bits;
> }
>
> /**
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list