| Message ID | 20200601110116.GA10868@kaaira-HP-Pavilion-Notebook |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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@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; > } > > /** >
Hi Kaaira, Kieran On Mon, Jun 01, 2020 at 01:02:45PM +0100, Kieran Bingham wrote: > 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@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: > > /** > This is declared in an anonymous namespace in order to keep it hidden from outside. I think using doxygen syntax is ok for sake of consistency, but no documentation has to be generated for this private constuct. I would keep a single * here. > > + * \var mbusFormatInfo But use \struct > > + * \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@ideasonboard.com> Acked-by: Jacopo Mondi <jacopo@jmondi.org> Thanks j > > -- > 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 > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
On Mon, Jun 01, 2020 at 01:02:45PM +0100, Kieran Bingham wrote: > 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@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: Sorry, I forgot that > > /** > > > + * \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. Okay, I'll remember it the next time. Thanks! > > > > + */ > > +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. Yes, that's great..thanks! I'll remember to put correct spaces the next time :D > > Reviewed-by: Kieran Bingham <kieran.bingham@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
On Mon, Jun 01, 2020 at 01:02:45PM +0100, Kieran Bingham wrote: > 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@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@ideasonboard.com> > > > + if (it == formatInfoMap.end()) > > + mbus << size.toString() << "-" << utils::hex(mbus_code, 4); > > + else > > + mbus << size.toString() << "-" << it->second.format; How about the following ? std::stringstream mbus; mbus << size.toString() << "-"; const auto it = formatInfoMap.find(mbus_code); if (it == formatInfoMap.end()) mbus << utils::hex(mbus_code, 4); else mbus << it->second.format; return mbus.str(); > > + > > + return mbus.str(); > > } > > > > /** > > @@ -180,7 +194,7 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const > > return 0; > > } > > > > - return it->second; > > + return it->second.bits; > > } > > > > /**
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 { +/* + * \var mbusFormatInfo + * \brief A struct of bits per pixel and mbus format + */ +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; + 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; } /**
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@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(-)