[{"id":4944,"web_url":"https://patchwork.libcamera.org/comment/4944/","msgid":"<1877c028-1b67-4cfa-8549-6973db1524be@ideasonboard.com>","date":"2020-06-01T12:02:45","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: v4l2subdev: Print mbus\n\tstring instead of code","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Kaaira,\n\nOn 01/06/2020 12:01, Kaaira Gupta wrote:\n> Modify toString() to print mbus format string instead of its hex code as\n> the string is easier to understand.\n> \n> Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>\n> ---\n> \n> Changes since v1:\n> \tAdd check for unsupported format.\n> \tRename struct\n> \n>  src/libcamera/v4l2_subdevice.cpp | 172 +++++++++++++++++--------------\n>  1 file changed, 93 insertions(+), 79 deletions(-)\n> \n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 7aefc1b..e97982e 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -35,84 +35,93 @@ LOG_DECLARE_CATEGORY(V4L2)\n>  \n>  namespace {\n>  \n> +/*\n\nFor doxygen documentation comments, there should be two *'s:\n\n /**\n\n> + * \\var mbusFormatInfo\n> + * \\brief A struct of bits per pixel and mbus format\nWe wouldn't normally describe a struct as \"a struct\" in the brief. The\ntype tells us that already.\n\nJust saying what the data represented by it is enough.\n\n\n> + */\n> +struct mbusFormatInfo {\n> +\tunsigned int bits;\n> +\tstd::string format;\n> +};\n> +\n>  /*\n>   * \\var formatInfoMap\n> - * \\brief A map that associates bits per pixel to V4L2 media bus codes\n> + * \\brief A map that associates mbusFormatInfo struct to V4L2 media bus codes\n>   */\n> -const std::map<uint32_t, unsigned int> formatInfoMap = {\n> -\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_BE, 16 },\n> -\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE, 16 },\n> -\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE, 16 },\n> -\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE, 16 },\n> -\t{ V4L2_MBUS_FMT_BGR565_2X8_BE, 16 },\n> -\t{ V4L2_MBUS_FMT_BGR565_2X8_LE, 16 },\n> -\t{ V4L2_MBUS_FMT_RGB565_2X8_BE, 16 },\n> -\t{ V4L2_MBUS_FMT_RGB565_2X8_LE, 16 },\n> -\t{ V4L2_MBUS_FMT_RGB666_1X18, 18 },\n> -\t{ V4L2_MBUS_FMT_RGB888_1X24, 24 },\n> -\t{ V4L2_MBUS_FMT_RGB888_2X12_BE, 24 },\n> -\t{ V4L2_MBUS_FMT_RGB888_2X12_LE, 24 },\n> -\t{ V4L2_MBUS_FMT_ARGB8888_1X32, 32 },\n> -\t{ V4L2_MBUS_FMT_Y8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_UV8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_UYVY8_1_5X8, 12 },\n> -\t{ V4L2_MBUS_FMT_VYUY8_1_5X8, 12 },\n> -\t{ V4L2_MBUS_FMT_YUYV8_1_5X8, 12 },\n> -\t{ V4L2_MBUS_FMT_YVYU8_1_5X8, 12 },\n> -\t{ V4L2_MBUS_FMT_UYVY8_2X8, 16 },\n> -\t{ V4L2_MBUS_FMT_VYUY8_2X8, 16 },\n> -\t{ V4L2_MBUS_FMT_YUYV8_2X8, 16 },\n> -\t{ V4L2_MBUS_FMT_YVYU8_2X8, 16 },\n> -\t{ V4L2_MBUS_FMT_Y10_1X10, 10 },\n> -\t{ V4L2_MBUS_FMT_UYVY10_2X10, 20 },\n> -\t{ V4L2_MBUS_FMT_VYUY10_2X10, 20 },\n> -\t{ V4L2_MBUS_FMT_YUYV10_2X10, 20 },\n> -\t{ V4L2_MBUS_FMT_YVYU10_2X10, 20 },\n> -\t{ V4L2_MBUS_FMT_Y12_1X12, 12 },\n> -\t{ V4L2_MBUS_FMT_UYVY8_1X16, 16 },\n> -\t{ V4L2_MBUS_FMT_VYUY8_1X16, 16 },\n> -\t{ V4L2_MBUS_FMT_YUYV8_1X16, 16 },\n> -\t{ V4L2_MBUS_FMT_YVYU8_1X16, 16 },\n> -\t{ V4L2_MBUS_FMT_YDYUYDYV8_1X16, 16 },\n> -\t{ V4L2_MBUS_FMT_UYVY10_1X20, 20 },\n> -\t{ V4L2_MBUS_FMT_VYUY10_1X20, 20 },\n> -\t{ V4L2_MBUS_FMT_YUYV10_1X20, 20 },\n> -\t{ V4L2_MBUS_FMT_YVYU10_1X20, 20 },\n> -\t{ V4L2_MBUS_FMT_YUV10_1X30, 30 },\n> -\t{ V4L2_MBUS_FMT_AYUV8_1X32, 32 },\n> -\t{ V4L2_MBUS_FMT_UYVY12_2X12, 24 },\n> -\t{ V4L2_MBUS_FMT_VYUY12_2X12, 24 },\n> -\t{ V4L2_MBUS_FMT_YUYV12_2X12, 24 },\n> -\t{ V4L2_MBUS_FMT_YVYU12_2X12, 24 },\n> -\t{ V4L2_MBUS_FMT_UYVY12_1X24, 24 },\n> -\t{ V4L2_MBUS_FMT_VYUY12_1X24, 24 },\n> -\t{ V4L2_MBUS_FMT_YUYV12_1X24, 24 },\n> -\t{ V4L2_MBUS_FMT_YVYU12_1X24, 24 },\n> -\t{ V4L2_MBUS_FMT_SBGGR8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_SGBRG8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_SGRBG8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_SRGGB8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_SBGGR10_ALAW8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_SGBRG10_ALAW8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_SRGGB10_ALAW8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8, 8 },\n> -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE, 16 },\n> -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE, 16 },\n> -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE, 16 },\n> -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE, 16 },\n> -\t{ V4L2_MBUS_FMT_SBGGR10_1X10, 10 },\n> -\t{ V4L2_MBUS_FMT_SGBRG10_1X10, 10 },\n> -\t{ V4L2_MBUS_FMT_SGRBG10_1X10, 10 },\n> -\t{ V4L2_MBUS_FMT_SRGGB10_1X10, 10 },\n> -\t{ V4L2_MBUS_FMT_SBGGR12_1X12, 12 },\n> -\t{ V4L2_MBUS_FMT_SGBRG12_1X12, 12 },\n> -\t{ V4L2_MBUS_FMT_SGRBG12_1X12, 12 },\n> -\t{ V4L2_MBUS_FMT_SRGGB12_1X12, 12 },\n> -\t{ V4L2_MBUS_FMT_AHSV8888_1X32, 32 },\n> +const std::map<uint32_t, mbusFormatInfo> formatInfoMap = {\n> +\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_BE, { 16, \"RGB444_2X8_PADHI_BE\" } },\n> +\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE, { 16, \"RGB444_2X8_PADHI_LE\" } },\n> +\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE, { 16, \"RGB555_2X8_PADHI_BE\" } },\n> +\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE, { 16, \"RGB555_2X8_PADHI_LE\" } },\n> +\t{ V4L2_MBUS_FMT_BGR565_2X8_BE, { 16, \"BGR565_2X8_BE\" } },\n> +\t{ V4L2_MBUS_FMT_BGR565_2X8_LE, { 16, \"BGR565_2X8_LE\" } },\n> +\t{ V4L2_MBUS_FMT_RGB565_2X8_BE, { 16, \"RGB565_2X8_BE\" } },\n> +\t{ V4L2_MBUS_FMT_RGB565_2X8_LE, { 16, \"RGB565_2X8_LE\" } },\n> +\t{ V4L2_MBUS_FMT_RGB666_1X18, { 18, \"RGB666_1X18\" } },\n> +\t{ V4L2_MBUS_FMT_RGB888_1X24, { 24, \"RGB888_1X24\" } },\n> +\t{ V4L2_MBUS_FMT_RGB888_2X12_BE, { 24, \"RGB888_2X12_BE\" } },\n> +\t{ V4L2_MBUS_FMT_RGB888_2X12_LE, { 24, \"RGB888_2X12_LE\" } },\n> +\t{ V4L2_MBUS_FMT_ARGB8888_1X32, { 32, \"ARGB8888_1X32\" } },\n> +\t{ V4L2_MBUS_FMT_Y8_1X8, { 8, \"Y8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_UV8_1X8, { 8, \"UV8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_UYVY8_1_5X8, { 12, \"UYVY8_1_5X8\" } },\n> +\t{ V4L2_MBUS_FMT_VYUY8_1_5X8, { 12, \"VYUY8_1_5X8\" } },\n> +\t{ V4L2_MBUS_FMT_YUYV8_1_5X8, { 12, \"YUYV8_1_5X8\" } },\n> +\t{ V4L2_MBUS_FMT_YVYU8_1_5X8, { 12, \"YVYU8_1_5X8\" } },\n> +\t{ V4L2_MBUS_FMT_UYVY8_2X8, { 16, \"UYVY8_2X8\" } },\n> +\t{ V4L2_MBUS_FMT_VYUY8_2X8, { 16, \"VYUY8_2X8\" } },\n> +\t{ V4L2_MBUS_FMT_YUYV8_2X8, { 16, \"YUYV8_2X8\" } },\n> +\t{ V4L2_MBUS_FMT_YVYU8_2X8, { 16, \"YVYU8_2X8\" } },\n> +\t{ V4L2_MBUS_FMT_Y10_1X10, { 10, \"Y10_1X10\" } },\n> +\t{ V4L2_MBUS_FMT_UYVY10_2X10, { 20, \"UYVY10_2X10\" } },\n> +\t{ V4L2_MBUS_FMT_VYUY10_2X10, { 20, \"VYUY10_2X10\" } },\n> +\t{ V4L2_MBUS_FMT_YUYV10_2X10, { 20, \"YUYV10_2X10\" } },\n> +\t{ V4L2_MBUS_FMT_YVYU10_2X10, { 20, \"YVYU10_2X10\" } },\n> +\t{ V4L2_MBUS_FMT_Y12_1X12, { 12, \"Y12_1X12\" } },\n> +\t{ V4L2_MBUS_FMT_UYVY8_1X16, { 16, \"UYVY8_1X16\" } },\n> +\t{ V4L2_MBUS_FMT_VYUY8_1X16, { 16, \"VYUY8_1X16\" } },\n> +\t{ V4L2_MBUS_FMT_YUYV8_1X16, { 16, \"YUYV8_1X16\" } },\n> +\t{ V4L2_MBUS_FMT_YVYU8_1X16, { 16, \"YVYU8_1X16\" } },\n> +\t{ V4L2_MBUS_FMT_YDYUYDYV8_1X16, { 16, \"YDYUYDYV8_1X16\" } },\n> +\t{ V4L2_MBUS_FMT_UYVY10_1X20, { 20, \"UYVY10_1X20\" } },\n> +\t{ V4L2_MBUS_FMT_VYUY10_1X20, { 20, \"VYUY10_1X20\" } },\n> +\t{ V4L2_MBUS_FMT_YUYV10_1X20, { 20, \"YUYV10_1X20\" } },\n> +\t{ V4L2_MBUS_FMT_YVYU10_1X20, { 20, \"YVYU10_1X20\" } },\n> +\t{ V4L2_MBUS_FMT_YUV10_1X30, { 30, \"YUV10_1X30\" } },\n> +\t{ V4L2_MBUS_FMT_AYUV8_1X32, { 32, \"AYUV8_1X32\" } },\n> +\t{ V4L2_MBUS_FMT_UYVY12_2X12, { 24, \"UYVY12_2X12\" } },\n> +\t{ V4L2_MBUS_FMT_VYUY12_2X12, { 24, \"VYUY12_2X12\" } },\n> +\t{ V4L2_MBUS_FMT_YUYV12_2X12, { 24, \"YUYV12_2X12\" } },\n> +\t{ V4L2_MBUS_FMT_YVYU12_2X12, { 24, \"YVYU12_2X12\" } },\n> +\t{ V4L2_MBUS_FMT_UYVY12_1X24, { 24, \"UYVY12_1X24\" } },\n> +\t{ V4L2_MBUS_FMT_VYUY12_1X24, { 24, \"VYUY12_1X24\" } },\n> +\t{ V4L2_MBUS_FMT_YUYV12_1X24, { 24, \"YUYV12_1X24\" } },\n> +\t{ V4L2_MBUS_FMT_YVYU12_1X24, { 24, \"YVYU12_1X24\" } },\n> +\t{ V4L2_MBUS_FMT_SBGGR8_1X8, { 8, \"SBGGR8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_SGBRG8_1X8, { 8, \"SGBRG8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_SGRBG8_1X8, { 8, \"SGRBG8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_SRGGB8_1X8, { 8, \"SRGGB8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_SBGGR10_ALAW8_1X8, { 8, \"SBGGR10_ALAW8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_SGBRG10_ALAW8_1X8, { 8, \"SGBRG10_ALAW8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8, { 8, \"SGRBG10_ALAW8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_SRGGB10_ALAW8_1X8, { 8, \"SRGGB10_ALAW8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8, { 8, \"SBGGR10_DPCM8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8, { 8, \"SGBRG10_DPCM8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, { 8, \"SGRBG10_DPCM8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8, { 8, \"SRGGB10_DPCM8_1X8\" } },\n> +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE, { 16, \"SBGGR10_2X8_PADHI_BE\" } },\n> +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE, { 16, \"SBGGR10_2X8_PADHI_LE\" } },\n> +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE, { 16, \"SBGGR10_2X8_PADLO_BE\" } },\n> +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE, { 16, \"SBGGR10_2X8_PADLO_LE\" } },\n> +\t{ V4L2_MBUS_FMT_SBGGR10_1X10, { 10, \"SBGGR10_1X10\" } },\n> +\t{ V4L2_MBUS_FMT_SGBRG10_1X10, { 10, \"SGBRG10_1X10\" } },\n> +\t{ V4L2_MBUS_FMT_SGRBG10_1X10, { 10, \"SGRBG10_1X10\" } },\n> +\t{ V4L2_MBUS_FMT_SRGGB10_1X10, { 10, \"SRGGB10_1X10\" } },\n> +\t{ V4L2_MBUS_FMT_SBGGR12_1X12, { 12, \"SBGGR12_1X12\" } },\n> +\t{ V4L2_MBUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\" } },\n> +\t{ V4L2_MBUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\" } },\n> +\t{ V4L2_MBUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\" } },\n> +\t{ V4L2_MBUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\" } },\n>  };\n>  \n>  } /* namespace */\n> @@ -161,9 +170,14 @@ const std::map<uint32_t, unsigned int> formatInfoMap = {\n>   */\n>  const std::string V4L2SubdeviceFormat::toString() const\n>  {\n> -\tstd::stringstream ss;\n> -\tss << size.toString() << \"-\" << utils::hex(mbus_code, 4);\n> -\treturn ss.str();\n> +\tconst auto it = formatInfoMap.find(mbus_code);\n> +\tstd::stringstream mbus;\n\nAnd this last one is only a stylistic 'nit' and is quite minor, if a\nfunction grows in complexity we would normally put extra spacing in to\nease readability.\n\nThe original function was three lines, so it was packed without spaces\nto keep it short, but here we now have three parts.\n\n{\n\tLocal Variables\n\n \tCode\n\n\tReturn statement.\n}\n\nYou've already got a space between the Code and return, so to match it\nshould probably have a space between the local variables and code.\n\nand then, because the auto iterator calls formatInfoMap.find() I'd put\nthat after the mbus instantiation...\n\nNo need to send a v3 though - I'm just discussing stylistic things there.\n\nOtherwise, this patch looks good to me, so if we get another reviewed-by\ntag, I can apply with the small style fixups if you are happy.\n\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\n--\nKieran\n\n\n> +\tif (it == formatInfoMap.end())\n> +\t\tmbus << size.toString() << \"-\" << utils::hex(mbus_code, 4);\n> +\telse\n> +\t\tmbus << size.toString() << \"-\" << it->second.format;\n> +\n> +\treturn mbus.str();\n>  }\n>  \n>  /**\n> @@ -180,7 +194,7 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const\n>  \t\treturn 0;\n>  \t}\n>  \n> -\treturn it->second;\n> +\treturn it->second.bits;\n>  }\n>  \n>  /**\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id F0B08603CC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jun 2020 14:02:48 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 5BF51304;\n\tMon,  1 Jun 2020 14:02:48 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AtXrZ/At\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591012968;\n\tbh=Tn8ttyJEn1sZ/HeBw5ewvg2R56fLMhEYHRyyNvmGlDI=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=AtXrZ/Atoh86krGb1hR5BA9Jfy1vXAM/shRSDE/AjzQ8ZZyS64YO0nqYL7tm2yotU\n\tTcuqPGh8fAa32TrtGfT9y2HuW9EGke78KKu/7ngti8xdleY2tDq7AwTeDL5hubv2xv\n\tpldzUUuIH1Cl+pRI/hAhmQnicfs6DCM09MRwgsT8=","Reply-To":"kieran.bingham@ideasonboard.com","To":"Kaaira Gupta <kgupta@es.iitr.ac.in>, libcamera-devel@lists.libcamera.org","References":"<20200601110116.GA10868@kaaira-HP-Pavilion-Notebook>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAlcEEwEKAEECGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEWIQSQLdeYP70o/eNy1HqhHkZyEKRh/QUCXWTtygUJ\n\tCyJXZAAKCRChHkZyEKRh/f8dEACTDsbLN2nioNZMwyLuQRUAFcXNolDX48xcUXsWS2QjxaPm\n\tVsJx8Uy8aYkS85mdPBh0C83OovQR/OVbr8AxhGvYqBs3nQvbWuTl/+4od7DfK2VZOoKBAu5S\n\tQK2FYuUcikDqYcFWJ8DQnubxfE8dvzojHEkXw0sA4igINHDDFX3HJGZtLio+WpEFQtCbfTAG\n\tYZslasz1YZRbwEdSsmO3/kqy5eMnczlm8a21A3fKUo3g8oAZEFM+f4DUNzqIltg31OAB/kZS\n\tenKZQ/SWC8PmLg/ZXBrReYakxXtkP6w3FwMlzOlhGxqhIRNiAJfXJBaRhuUWzPOpEDE9q5YJ\n\tBmqQL2WJm1VSNNVxbXJHpaWMH1sA2R00vmvRrPXGwyIO0IPYeUYQa3gsy6k+En/aMQJd27dp\n\taScf9am9PFICPY5T4ppneeJLif2lyLojo0mcHOV+uyrds9XkLpp14GfTkeKPdPMrLLTsHRfH\n\tfA4I4OBpRrEPiGIZB/0im98MkGY/Mu6qxeZmYLCcgD6qz4idOvfgVOrNh+aA8HzIVR+RMW8H\n\tQGBN9f0E3kfwxuhl3omo6V7lDw8XOdmuWZNC9zPq1UfryVHANYbLGz9KJ4Aw6M+OgBC2JpkD\n\thXMdHUkC+d20dwXrwHTlrJi1YNp6rBc+xald3wsUPOZ5z8moTHUX/uPA/qhGsbkCDQRWBP1m\n\tARAAzijkb+Sau4hAncr1JjOY+KyFEdUNxRy+hqTJdJfaYihxyaj0Ee0P0zEi35CbE6lgU0Uz\n\ttih9fiUbSV3wfsWqg1Ut3/5rTKu7kLFp15kF7eqvV4uezXRD3Qu4yjv/rMmEJbbD4cTvGCYI\n\td6MDC417f7vK3hCbCVIZSp3GXxyC1LU+UQr3fFcOyCwmP9vDUR9JV0BSqHHxRDdpUXE26Dk6\n\tmhf0V1YkspE5St814ETXpEus2urZE5yJIUROlWPIL+hm3NEWfAP06vsQUyLvr/GtbOT79vXl\n\tEn1aulcYyu20dRRxhkQ6iILaURcxIAVJJKPi8dsoMnS8pB0QW12AHWuirPF0g6DiuUfPmrA5\n\tPKe56IGlpkjc8cO51lIxHkWTpCMWigRdPDexKX+Sb+W9QWK/0JjIc4t3KBaiG8O4yRX8ml2R\n\t+rxfAVKM6V769P/hWoRGdgUMgYHFpHGSgEt80OKK5HeUPy2cngDUXzwrqiM5Sz6Od0qw5pCk\n\tNlXqI0W/who0iSVM+8+RmyY0OEkxEcci7rRLsGnM15B5PjLJjh1f2ULYkv8s4SnDwMZ/kE04\n\t/UqCMK/KnX8pwXEMCjz0h6qWNpGwJ0/tYIgQJZh6bqkvBrDogAvuhf60Sogw+mH8b+PBlx1L\n\toeTK396wc+4c3BfiC6pNtUS5GpsPMMjYMk7kVvEAEQEAAYkCPAQYAQoAJgIbDBYhBJAt15g/\n\tvSj943LUeqEeRnIQpGH9BQJdizzIBQkLSKZiAAoJEKEeRnIQpGH9eYgQAJpjaWNgqNOnMTmD\n\tMJggbwjIotypzIXfhHNCeTkG7+qCDlSaBPclcPGYrTwCt0YWPU2TgGgJrVhYT20ierN8LUvj\n\t6qOPTd+Uk7NFzL65qkh80ZKNBFddx1AabQpSVQKbdcLb8OFs85kuSvFdgqZwgxA1vl4TFhNz\n\tPZ79NAmXLackAx3sOVFhk4WQaKRshCB7cSl+RIng5S/ThOBlwNlcKG7j7W2MC06BlTbdEkUp\n\tECzuuRBv8wX4OQl+hbWbB/VKIx5HKlLu1eypen/5lNVzSqMMIYkkZcjV2SWQyUGxSwq0O/sx\n\tS0A8/atCHUXOboUsn54qdxrVDaK+6jIAuo8JiRWctP16KjzUM7MO0/+4zllM8EY57rXrj48j\n\tsbEYX0YQnzaj+jO6kJtoZsIaYR7rMMq9aUAjyiaEZpmP1qF/2sYenDx0Fg2BSlLvLvXM0vU8\n\tpQk3kgDu7kb/7PRYrZvBsr21EIQoIjXbZxDz/o7z95frkP71EaICttZ6k9q5oxxA5WC6sTXc\n\tMW8zs8avFNuA9VpXt0YupJd2ijtZy2mpZNG02fFVXhIn4G807G7+9mhuC4XG5rKlBBUXTvPU\n\tAfYnB4JBDLmLzBFavQfvonSfbitgXwCG3vS+9HEwAjU30Bar1PEOmIbiAoMzuKeRm2LVpmq4\n\tWZw01QYHU/GUV/zHJSFk","Organization":"Ideas on Board","Message-ID":"<1877c028-1b67-4cfa-8549-6973db1524be@ideasonboard.com>","Date":"Mon, 1 Jun 2020 13:02:45 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.7.0","MIME-Version":"1.0","In-Reply-To":"<20200601110116.GA10868@kaaira-HP-Pavilion-Notebook>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: v4l2subdev: Print mbus\n\tstring instead of code","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 01 Jun 2020 12:02:49 -0000"}},{"id":4945,"web_url":"https://patchwork.libcamera.org/comment/4945/","msgid":"<20200601121700.ikvicqmvlcyfkyug@uno.localdomain>","date":"2020-06-01T12:17:00","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: v4l2subdev: Print mbus\n\tstring instead of code","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Kaaira, Kieran\n\nOn Mon, Jun 01, 2020 at 01:02:45PM +0100, Kieran Bingham wrote:\n> Hi Kaaira,\n>\n> On 01/06/2020 12:01, Kaaira Gupta wrote:\n> > Modify toString() to print mbus format string instead of its hex code as\n> > the string is easier to understand.\n> >\n> > Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>\n> > ---\n> >\n> > Changes since v1:\n> > \tAdd check for unsupported format.\n> > \tRename struct\n> >\n> >  src/libcamera/v4l2_subdevice.cpp | 172 +++++++++++++++++--------------\n> >  1 file changed, 93 insertions(+), 79 deletions(-)\n> >\n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index 7aefc1b..e97982e 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -35,84 +35,93 @@ LOG_DECLARE_CATEGORY(V4L2)\n> >\n> >  namespace {\n> >\n> > +/*\n>\n> For doxygen documentation comments, there should be two *'s:\n>\n>  /**\n>\n\nThis is declared in an anonymous namespace in order to keep it hidden\nfrom outside. I think using doxygen syntax is ok for sake of consistency,\nbut no documentation has to be generated for this private constuct. I\nwould keep a single * here.\n\n> > + * \\var mbusFormatInfo\n\nBut use \\struct\n\n> > + * \\brief A struct of bits per pixel and mbus format\n> We wouldn't normally describe a struct as \"a struct\" in the brief. The\n> type tells us that already.\n>\n> Just saying what the data represented by it is enough.\n>\n>\n> > + */\n> > +struct mbusFormatInfo {\n> > +\tunsigned int bits;\n> > +\tstd::string format;\n> > +};\n> > +\n> >  /*\n> >   * \\var formatInfoMap\n> > - * \\brief A map that associates bits per pixel to V4L2 media bus codes\n> > + * \\brief A map that associates mbusFormatInfo struct to V4L2 media bus codes\n> >   */\n> > -const std::map<uint32_t, unsigned int> formatInfoMap = {\n> > -\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_BGR565_2X8_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_BGR565_2X8_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB565_2X8_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB565_2X8_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB666_1X18, 18 },\n> > -\t{ V4L2_MBUS_FMT_RGB888_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_RGB888_2X12_BE, 24 },\n> > -\t{ V4L2_MBUS_FMT_RGB888_2X12_LE, 24 },\n> > -\t{ V4L2_MBUS_FMT_ARGB8888_1X32, 32 },\n> > -\t{ V4L2_MBUS_FMT_Y8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_UV8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_UYVY8_1_5X8, 12 },\n> > -\t{ V4L2_MBUS_FMT_VYUY8_1_5X8, 12 },\n> > -\t{ V4L2_MBUS_FMT_YUYV8_1_5X8, 12 },\n> > -\t{ V4L2_MBUS_FMT_YVYU8_1_5X8, 12 },\n> > -\t{ V4L2_MBUS_FMT_UYVY8_2X8, 16 },\n> > -\t{ V4L2_MBUS_FMT_VYUY8_2X8, 16 },\n> > -\t{ V4L2_MBUS_FMT_YUYV8_2X8, 16 },\n> > -\t{ V4L2_MBUS_FMT_YVYU8_2X8, 16 },\n> > -\t{ V4L2_MBUS_FMT_Y10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_UYVY10_2X10, 20 },\n> > -\t{ V4L2_MBUS_FMT_VYUY10_2X10, 20 },\n> > -\t{ V4L2_MBUS_FMT_YUYV10_2X10, 20 },\n> > -\t{ V4L2_MBUS_FMT_YVYU10_2X10, 20 },\n> > -\t{ V4L2_MBUS_FMT_Y12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_UYVY8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_VYUY8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_YUYV8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_YVYU8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_YDYUYDYV8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_UYVY10_1X20, 20 },\n> > -\t{ V4L2_MBUS_FMT_VYUY10_1X20, 20 },\n> > -\t{ V4L2_MBUS_FMT_YUYV10_1X20, 20 },\n> > -\t{ V4L2_MBUS_FMT_YVYU10_1X20, 20 },\n> > -\t{ V4L2_MBUS_FMT_YUV10_1X30, 30 },\n> > -\t{ V4L2_MBUS_FMT_AYUV8_1X32, 32 },\n> > -\t{ V4L2_MBUS_FMT_UYVY12_2X12, 24 },\n> > -\t{ V4L2_MBUS_FMT_VYUY12_2X12, 24 },\n> > -\t{ V4L2_MBUS_FMT_YUYV12_2X12, 24 },\n> > -\t{ V4L2_MBUS_FMT_YVYU12_2X12, 24 },\n> > -\t{ V4L2_MBUS_FMT_UYVY12_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_VYUY12_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_YUYV12_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_YVYU12_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_ALAW8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG10_ALAW8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB10_ALAW8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_AHSV8888_1X32, 32 },\n> > +const std::map<uint32_t, mbusFormatInfo> formatInfoMap = {\n> > +\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_BE, { 16, \"RGB444_2X8_PADHI_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE, { 16, \"RGB444_2X8_PADHI_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE, { 16, \"RGB555_2X8_PADHI_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE, { 16, \"RGB555_2X8_PADHI_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_BGR565_2X8_BE, { 16, \"BGR565_2X8_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_BGR565_2X8_LE, { 16, \"BGR565_2X8_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB565_2X8_BE, { 16, \"RGB565_2X8_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB565_2X8_LE, { 16, \"RGB565_2X8_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB666_1X18, { 18, \"RGB666_1X18\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB888_1X24, { 24, \"RGB888_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB888_2X12_BE, { 24, \"RGB888_2X12_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB888_2X12_LE, { 24, \"RGB888_2X12_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_ARGB8888_1X32, { 32, \"ARGB8888_1X32\" } },\n> > +\t{ V4L2_MBUS_FMT_Y8_1X8, { 8, \"Y8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_UV8_1X8, { 8, \"UV8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY8_1_5X8, { 12, \"UYVY8_1_5X8\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY8_1_5X8, { 12, \"VYUY8_1_5X8\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV8_1_5X8, { 12, \"YUYV8_1_5X8\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU8_1_5X8, { 12, \"YVYU8_1_5X8\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY8_2X8, { 16, \"UYVY8_2X8\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY8_2X8, { 16, \"VYUY8_2X8\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV8_2X8, { 16, \"YUYV8_2X8\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU8_2X8, { 16, \"YVYU8_2X8\" } },\n> > +\t{ V4L2_MBUS_FMT_Y10_1X10, { 10, \"Y10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY10_2X10, { 20, \"UYVY10_2X10\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY10_2X10, { 20, \"VYUY10_2X10\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV10_2X10, { 20, \"YUYV10_2X10\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU10_2X10, { 20, \"YVYU10_2X10\" } },\n> > +\t{ V4L2_MBUS_FMT_Y12_1X12, { 12, \"Y12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY8_1X16, { 16, \"UYVY8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY8_1X16, { 16, \"VYUY8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV8_1X16, { 16, \"YUYV8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU8_1X16, { 16, \"YVYU8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_YDYUYDYV8_1X16, { 16, \"YDYUYDYV8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY10_1X20, { 20, \"UYVY10_1X20\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY10_1X20, { 20, \"VYUY10_1X20\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV10_1X20, { 20, \"YUYV10_1X20\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU10_1X20, { 20, \"YVYU10_1X20\" } },\n> > +\t{ V4L2_MBUS_FMT_YUV10_1X30, { 30, \"YUV10_1X30\" } },\n> > +\t{ V4L2_MBUS_FMT_AYUV8_1X32, { 32, \"AYUV8_1X32\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY12_2X12, { 24, \"UYVY12_2X12\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY12_2X12, { 24, \"VYUY12_2X12\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV12_2X12, { 24, \"YUYV12_2X12\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU12_2X12, { 24, \"YVYU12_2X12\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY12_1X24, { 24, \"UYVY12_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY12_1X24, { 24, \"VYUY12_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV12_1X24, { 24, \"YUYV12_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU12_1X24, { 24, \"YVYU12_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR8_1X8, { 8, \"SBGGR8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG8_1X8, { 8, \"SGBRG8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG8_1X8, { 8, \"SGRBG8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB8_1X8, { 8, \"SRGGB8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_ALAW8_1X8, { 8, \"SBGGR10_ALAW8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG10_ALAW8_1X8, { 8, \"SGBRG10_ALAW8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8, { 8, \"SGRBG10_ALAW8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB10_ALAW8_1X8, { 8, \"SRGGB10_ALAW8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8, { 8, \"SBGGR10_DPCM8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8, { 8, \"SGBRG10_DPCM8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, { 8, \"SGRBG10_DPCM8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8, { 8, \"SRGGB10_DPCM8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE, { 16, \"SBGGR10_2X8_PADHI_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE, { 16, \"SBGGR10_2X8_PADHI_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE, { 16, \"SBGGR10_2X8_PADLO_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE, { 16, \"SBGGR10_2X8_PADLO_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_1X10, { 10, \"SBGGR10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG10_1X10, { 10, \"SGBRG10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG10_1X10, { 10, \"SGRBG10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB10_1X10, { 10, \"SRGGB10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR12_1X12, { 12, \"SBGGR12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\" } },\n> >  };\n> >\n> >  } /* namespace */\n> > @@ -161,9 +170,14 @@ const std::map<uint32_t, unsigned int> formatInfoMap = {\n> >   */\n> >  const std::string V4L2SubdeviceFormat::toString() const\n> >  {\n> > -\tstd::stringstream ss;\n> > -\tss << size.toString() << \"-\" << utils::hex(mbus_code, 4);\n> > -\treturn ss.str();\n> > +\tconst auto it = formatInfoMap.find(mbus_code);\n> > +\tstd::stringstream mbus;\n>\n> And this last one is only a stylistic 'nit' and is quite minor, if a\n> function grows in complexity we would normally put extra spacing in to\n> ease readability.\n>\n> The original function was three lines, so it was packed without spaces\n> to keep it short, but here we now have three parts.\n>\n> {\n> \tLocal Variables\n>\n>  \tCode\n>\n> \tReturn statement.\n> }\n>\n> You've already got a space between the Code and return, so to match it\n> should probably have a space between the local variables and code.\n>\n> and then, because the auto iterator calls formatInfoMap.find() I'd put\n> that after the mbus instantiation...\n>\n> No need to send a v3 though - I'm just discussing stylistic things there.\n>\n> Otherwise, this patch looks good to me, so if we get another reviewed-by\n> tag, I can apply with the small style fixups if you are happy.\n>\n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n\nAcked-by: Jacopo Mondi <jacopo@jmondi.org>\n\nThanks\n  j\n\n>\n> --\n> Kieran\n>\n>\n> > +\tif (it == formatInfoMap.end())\n> > +\t\tmbus << size.toString() << \"-\" << utils::hex(mbus_code, 4);\n> > +\telse\n> > +\t\tmbus << size.toString() << \"-\" << it->second.format;\n> > +\n> > +\treturn mbus.str();\n> >  }\n> >\n> >  /**\n> > @@ -180,7 +194,7 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const\n> >  \t\treturn 0;\n> >  \t}\n> >\n> > -\treturn it->second;\n> > +\treturn it->second.bits;\n> >  }\n> >\n> >  /**\n> >\n>\n> --\n> Regards\n> --\n> Kieran\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"<jacopo@jmondi.org>","Received":["from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AD42E603CC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jun 2020 14:13:42 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id 7908A40004;\n\tMon,  1 Jun 2020 12:13:41 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 1 Jun 2020 14:17:00 +0200","From":"Jacopo Mondi <jacopo@jmondi.org>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Kaaira Gupta <kgupta@es.iitr.ac.in>, libcamera-devel@lists.libcamera.org","Message-ID":"<20200601121700.ikvicqmvlcyfkyug@uno.localdomain>","References":"<20200601110116.GA10868@kaaira-HP-Pavilion-Notebook>\n\t<1877c028-1b67-4cfa-8549-6973db1524be@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<1877c028-1b67-4cfa-8549-6973db1524be@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: v4l2subdev: Print mbus\n\tstring instead of code","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 01 Jun 2020 12:13:42 -0000"}},{"id":4946,"web_url":"https://patchwork.libcamera.org/comment/4946/","msgid":"<20200601121930.GB13308@kaaira-HP-Pavilion-Notebook>","date":"2020-06-01T12:19:31","subject":"[libcamera-devel] [PATCH v2] libcamera: v4l2subdev: Print mbus\n\tstring instead of code","submitter":{"id":39,"url":"https://patchwork.libcamera.org/api/people/39/","name":"Kaaira Gupta","email":"kgupta@es.iitr.ac.in"},"content":"On Mon, Jun 01, 2020 at 01:02:45PM +0100, Kieran Bingham wrote:\n> Hi Kaaira,\n> \n> On 01/06/2020 12:01, Kaaira Gupta wrote:\n> > Modify toString() to print mbus format string instead of its hex code as\n> > the string is easier to understand.\n> > \n> > Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>\n> > ---\n> > \n> > Changes since v1:\n> > \tAdd check for unsupported format.\n> > \tRename struct\n> > \n> >  src/libcamera/v4l2_subdevice.cpp | 172 +++++++++++++++++--------------\n> >  1 file changed, 93 insertions(+), 79 deletions(-)\n> > \n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index 7aefc1b..e97982e 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -35,84 +35,93 @@ LOG_DECLARE_CATEGORY(V4L2)\n> >  \n> >  namespace {\n> >  \n> > +/*\n> \n> For doxygen documentation comments, there should be two *'s:\n\nSorry, I forgot that\n\n> \n>  /**\n> \n> > + * \\var mbusFormatInfo\n> > + * \\brief A struct of bits per pixel and mbus format\n> We wouldn't normally describe a struct as \"a struct\" in the brief. The\n> type tells us that already.\n> \n> Just saying what the data represented by it is enough.\n\nOkay, I'll remember it the next time. Thanks!\n\n> \n> \n> > + */\n> > +struct mbusFormatInfo {\n> > +\tunsigned int bits;\n> > +\tstd::string format;\n> > +};\n> > +\n> >  /*\n> >   * \\var formatInfoMap\n> > - * \\brief A map that associates bits per pixel to V4L2 media bus codes\n> > + * \\brief A map that associates mbusFormatInfo struct to V4L2 media bus codes\n> >   */\n> > -const std::map<uint32_t, unsigned int> formatInfoMap = {\n> > -\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_BGR565_2X8_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_BGR565_2X8_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB565_2X8_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB565_2X8_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB666_1X18, 18 },\n> > -\t{ V4L2_MBUS_FMT_RGB888_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_RGB888_2X12_BE, 24 },\n> > -\t{ V4L2_MBUS_FMT_RGB888_2X12_LE, 24 },\n> > -\t{ V4L2_MBUS_FMT_ARGB8888_1X32, 32 },\n> > -\t{ V4L2_MBUS_FMT_Y8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_UV8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_UYVY8_1_5X8, 12 },\n> > -\t{ V4L2_MBUS_FMT_VYUY8_1_5X8, 12 },\n> > -\t{ V4L2_MBUS_FMT_YUYV8_1_5X8, 12 },\n> > -\t{ V4L2_MBUS_FMT_YVYU8_1_5X8, 12 },\n> > -\t{ V4L2_MBUS_FMT_UYVY8_2X8, 16 },\n> > -\t{ V4L2_MBUS_FMT_VYUY8_2X8, 16 },\n> > -\t{ V4L2_MBUS_FMT_YUYV8_2X8, 16 },\n> > -\t{ V4L2_MBUS_FMT_YVYU8_2X8, 16 },\n> > -\t{ V4L2_MBUS_FMT_Y10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_UYVY10_2X10, 20 },\n> > -\t{ V4L2_MBUS_FMT_VYUY10_2X10, 20 },\n> > -\t{ V4L2_MBUS_FMT_YUYV10_2X10, 20 },\n> > -\t{ V4L2_MBUS_FMT_YVYU10_2X10, 20 },\n> > -\t{ V4L2_MBUS_FMT_Y12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_UYVY8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_VYUY8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_YUYV8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_YVYU8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_YDYUYDYV8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_UYVY10_1X20, 20 },\n> > -\t{ V4L2_MBUS_FMT_VYUY10_1X20, 20 },\n> > -\t{ V4L2_MBUS_FMT_YUYV10_1X20, 20 },\n> > -\t{ V4L2_MBUS_FMT_YVYU10_1X20, 20 },\n> > -\t{ V4L2_MBUS_FMT_YUV10_1X30, 30 },\n> > -\t{ V4L2_MBUS_FMT_AYUV8_1X32, 32 },\n> > -\t{ V4L2_MBUS_FMT_UYVY12_2X12, 24 },\n> > -\t{ V4L2_MBUS_FMT_VYUY12_2X12, 24 },\n> > -\t{ V4L2_MBUS_FMT_YUYV12_2X12, 24 },\n> > -\t{ V4L2_MBUS_FMT_YVYU12_2X12, 24 },\n> > -\t{ V4L2_MBUS_FMT_UYVY12_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_VYUY12_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_YUYV12_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_YVYU12_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_ALAW8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG10_ALAW8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB10_ALAW8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_AHSV8888_1X32, 32 },\n> > +const std::map<uint32_t, mbusFormatInfo> formatInfoMap = {\n> > +\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_BE, { 16, \"RGB444_2X8_PADHI_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE, { 16, \"RGB444_2X8_PADHI_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE, { 16, \"RGB555_2X8_PADHI_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE, { 16, \"RGB555_2X8_PADHI_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_BGR565_2X8_BE, { 16, \"BGR565_2X8_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_BGR565_2X8_LE, { 16, \"BGR565_2X8_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB565_2X8_BE, { 16, \"RGB565_2X8_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB565_2X8_LE, { 16, \"RGB565_2X8_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB666_1X18, { 18, \"RGB666_1X18\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB888_1X24, { 24, \"RGB888_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB888_2X12_BE, { 24, \"RGB888_2X12_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB888_2X12_LE, { 24, \"RGB888_2X12_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_ARGB8888_1X32, { 32, \"ARGB8888_1X32\" } },\n> > +\t{ V4L2_MBUS_FMT_Y8_1X8, { 8, \"Y8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_UV8_1X8, { 8, \"UV8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY8_1_5X8, { 12, \"UYVY8_1_5X8\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY8_1_5X8, { 12, \"VYUY8_1_5X8\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV8_1_5X8, { 12, \"YUYV8_1_5X8\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU8_1_5X8, { 12, \"YVYU8_1_5X8\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY8_2X8, { 16, \"UYVY8_2X8\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY8_2X8, { 16, \"VYUY8_2X8\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV8_2X8, { 16, \"YUYV8_2X8\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU8_2X8, { 16, \"YVYU8_2X8\" } },\n> > +\t{ V4L2_MBUS_FMT_Y10_1X10, { 10, \"Y10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY10_2X10, { 20, \"UYVY10_2X10\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY10_2X10, { 20, \"VYUY10_2X10\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV10_2X10, { 20, \"YUYV10_2X10\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU10_2X10, { 20, \"YVYU10_2X10\" } },\n> > +\t{ V4L2_MBUS_FMT_Y12_1X12, { 12, \"Y12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY8_1X16, { 16, \"UYVY8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY8_1X16, { 16, \"VYUY8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV8_1X16, { 16, \"YUYV8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU8_1X16, { 16, \"YVYU8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_YDYUYDYV8_1X16, { 16, \"YDYUYDYV8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY10_1X20, { 20, \"UYVY10_1X20\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY10_1X20, { 20, \"VYUY10_1X20\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV10_1X20, { 20, \"YUYV10_1X20\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU10_1X20, { 20, \"YVYU10_1X20\" } },\n> > +\t{ V4L2_MBUS_FMT_YUV10_1X30, { 30, \"YUV10_1X30\" } },\n> > +\t{ V4L2_MBUS_FMT_AYUV8_1X32, { 32, \"AYUV8_1X32\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY12_2X12, { 24, \"UYVY12_2X12\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY12_2X12, { 24, \"VYUY12_2X12\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV12_2X12, { 24, \"YUYV12_2X12\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU12_2X12, { 24, \"YVYU12_2X12\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY12_1X24, { 24, \"UYVY12_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY12_1X24, { 24, \"VYUY12_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV12_1X24, { 24, \"YUYV12_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU12_1X24, { 24, \"YVYU12_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR8_1X8, { 8, \"SBGGR8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG8_1X8, { 8, \"SGBRG8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG8_1X8, { 8, \"SGRBG8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB8_1X8, { 8, \"SRGGB8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_ALAW8_1X8, { 8, \"SBGGR10_ALAW8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG10_ALAW8_1X8, { 8, \"SGBRG10_ALAW8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8, { 8, \"SGRBG10_ALAW8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB10_ALAW8_1X8, { 8, \"SRGGB10_ALAW8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8, { 8, \"SBGGR10_DPCM8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8, { 8, \"SGBRG10_DPCM8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, { 8, \"SGRBG10_DPCM8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8, { 8, \"SRGGB10_DPCM8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE, { 16, \"SBGGR10_2X8_PADHI_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE, { 16, \"SBGGR10_2X8_PADHI_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE, { 16, \"SBGGR10_2X8_PADLO_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE, { 16, \"SBGGR10_2X8_PADLO_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_1X10, { 10, \"SBGGR10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG10_1X10, { 10, \"SGBRG10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG10_1X10, { 10, \"SGRBG10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB10_1X10, { 10, \"SRGGB10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR12_1X12, { 12, \"SBGGR12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\" } },\n> >  };\n> >  \n> >  } /* namespace */\n> > @@ -161,9 +170,14 @@ const std::map<uint32_t, unsigned int> formatInfoMap = {\n> >   */\n> >  const std::string V4L2SubdeviceFormat::toString() const\n> >  {\n> > -\tstd::stringstream ss;\n> > -\tss << size.toString() << \"-\" << utils::hex(mbus_code, 4);\n> > -\treturn ss.str();\n> > +\tconst auto it = formatInfoMap.find(mbus_code);\n> > +\tstd::stringstream mbus;\n> \n> And this last one is only a stylistic 'nit' and is quite minor, if a\n> function grows in complexity we would normally put extra spacing in to\n> ease readability.\n> \n> The original function was three lines, so it was packed without spaces\n> to keep it short, but here we now have three parts.\n> \n> {\n> \tLocal Variables\n> \n>  \tCode\n> \n> \tReturn statement.\n> }\n> \n> You've already got a space between the Code and return, so to match it\n> should probably have a space between the local variables and code.\n> \n> and then, because the auto iterator calls formatInfoMap.find() I'd put\n> that after the mbus instantiation...\n> \n> No need to send a v3 though - I'm just discussing stylistic things there.\n> \n> Otherwise, this patch looks good to me, so if we get another reviewed-by\n> tag, I can apply with the small style fixups if you are happy.\n\nYes, that's great..thanks! I'll remember to put correct spaces the next\ntime :D\n\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> --\n> Kieran\n> \n> \n> > +\tif (it == formatInfoMap.end())\n> > +\t\tmbus << size.toString() << \"-\" << utils::hex(mbus_code, 4);\n> > +\telse\n> > +\t\tmbus << size.toString() << \"-\" << it->second.format;\n> > +\n> > +\treturn mbus.str();\n> >  }\n> >  \n> >  /**\n> > @@ -180,7 +194,7 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const\n> >  \t\treturn 0;\n> >  \t}\n> >  \n> > -\treturn it->second;\n> > +\treturn it->second.bits;\n> >  }\n> >  \n> >  /**\n> > \n> \n> -- \n> Regards\n> --\n> Kieran","headers":{"Return-Path":"<kgupta@es.iitr.ac.in>","Received":["from mail-pj1-x1042.google.com (mail-pj1-x1042.google.com\n\t[IPv6:2607:f8b0:4864:20::1042])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DBF5A603CC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jun 2020 14:19:38 +0200 (CEST)","by mail-pj1-x1042.google.com with SMTP id fs4so5073941pjb.5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 01 Jun 2020 05:19:38 -0700 (PDT)","from kaaira-HP-Pavilion-Notebook ([103.113.213.174])\n\tby smtp.gmail.com with ESMTPSA id\n\ty126sm1779933pfb.101.2020.06.01.05.19.35\n\t(version=TLS1_2 cipher=ECDHE-ECDSA-CHACHA20-POLY1305 bits=256/256);\n\tMon, 01 Jun 2020 05:19:36 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=es-iitr-ac-in.20150623.gappssmtp.com\n\theader.i=@es-iitr-ac-in.20150623.gappssmtp.com header.b=\"o/iefLDS\"; \n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=es-iitr-ac-in.20150623.gappssmtp.com; s=20150623;\n\th=from:date:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:in-reply-to:user-agent;\n\tbh=4oxxdIPessKhpjHIPTbIKhtAMf/ik4rRW7GcQMszyLk=;\n\tb=o/iefLDSQ8V584JpIqdIm2t56ZOv9E++nFpWElexPH1elSmskIw2PBK4osiY7GafD+\n\tdj2hbR0Fc9OAQJK1zH2dgdRgfokLTOM5qYHcNBuODEkQjPryseEHmcJyBShiuO7SlcGn\n\tqf9WjByKATj/auTuqgVrEaYjX1LsMyzgf38iWqFVwx6MZs7m/bZ64Y/JiaxcoKyUqM1N\n\tNciyEpHnntAcfDug02tf25EmpV7tBNOvKQ4meK9fBC3G3Hqq5ZUHsXvsky33wO0Wzpfo\n\thd5JZo/6LzqwDFAHsGRLeDmYrkWPZK2XvBCCqH4Crho1NBx+QPAnkTzWbQ5ME6zWQPnW\n\tufXQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:date:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:in-reply-to:user-agent;\n\tbh=4oxxdIPessKhpjHIPTbIKhtAMf/ik4rRW7GcQMszyLk=;\n\tb=W3+hjUMt2F6JqdTkoan7ILHdCFe5ms3H9wIeKjCDNaHbN/JP3xYAE+psxgQGMuAJ6X\n\t5gk/qSLQk8SbNednPMQnjvyTZk97Vak1oTpZmevSUQIap5GP70d9Eu63T5ijvOab7Qmg\n\tMvXQBiErhSB+e+7rwx1UVFvAyy6P/jnKJdhRIcaqlFqd58/NcImnLFtwUVJFKccP5Plk\n\tBaTGpFE9Br6tfIR5KafJSA/SwMR6bD6VnfbxDOS9/eDTu5y/ZTvxECO5b+kD5POaGaox\n\t3eniHzIR6vV7u4W6VJ4jtiXJc0utaEJTWh6WYGTHmrXo6mjVItO51pwYfm8L82HYQ0Nn\n\tgQIw==","X-Gm-Message-State":"AOAM531kPb76DuJ0XTiThyUcA9Z2yQa4S5BMBXjsJJMuMrCy5inRo9hT\n\tymYxYH/XScyhDbjlElmByD+7Kg==","X-Google-Smtp-Source":"ABdhPJzfGyaKmXewtkDMMZxD5hQAuP5/3OljZ4kXVEigBj3rq8WZdglTDbeGPJ2rC/ZMl1qhOtQxHQ==","X-Received":"by 2002:a17:90a:aa8d:: with SMTP id\n\tl13mr23919319pjq.92.1591013977338; \n\tMon, 01 Jun 2020 05:19:37 -0700 (PDT)","From":"Kaaira Gupta <kgupta@es.iitr.ac.in>","X-Google-Original-From":"Kaaira Gupta <Kaairakgupta@es.iitr.ac.in>","Date":"Mon, 1 Jun 2020 17:49:31 +0530","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Kaaira Gupta <kgupta@es.iitr.ac.in>, libcamera-devel@lists.libcamera.org","Message-ID":"<20200601121930.GB13308@kaaira-HP-Pavilion-Notebook>","References":"<20200601110116.GA10868@kaaira-HP-Pavilion-Notebook>\n\t<1877c028-1b67-4cfa-8549-6973db1524be@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1877c028-1b67-4cfa-8549-6973db1524be@ideasonboard.com>","User-Agent":"Mutt/1.9.4 (2018-02-28)","Subject":"[libcamera-devel] [PATCH v2] libcamera: v4l2subdev: Print mbus\n\tstring instead of code","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 01 Jun 2020 12:19:39 -0000"}},{"id":4947,"web_url":"https://patchwork.libcamera.org/comment/4947/","msgid":"<20200601122020.GD5886@pendragon.ideasonboard.com>","date":"2020-06-01T12:20:20","subject":"Re: [libcamera-devel] [PATCH v2] libcamera: v4l2subdev: Print mbus\n\tstring instead of code","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Jun 01, 2020 at 01:02:45PM +0100, Kieran Bingham wrote:\n> Hi Kaaira,\n> \n> On 01/06/2020 12:01, Kaaira Gupta wrote:\n> > Modify toString() to print mbus format string instead of its hex code as\n> > the string is easier to understand.\n> > \n> > Signed-off-by: Kaaira Gupta <kgupta@es.iitr.ac.in>\n> > ---\n> > \n> > Changes since v1:\n> > \tAdd check for unsupported format.\n> > \tRename struct\n> > \n> >  src/libcamera/v4l2_subdevice.cpp | 172 +++++++++++++++++--------------\n> >  1 file changed, 93 insertions(+), 79 deletions(-)\n> > \n> > diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> > index 7aefc1b..e97982e 100644\n> > --- a/src/libcamera/v4l2_subdevice.cpp\n> > +++ b/src/libcamera/v4l2_subdevice.cpp\n> > @@ -35,84 +35,93 @@ LOG_DECLARE_CATEGORY(V4L2)\n> >  \n> >  namespace {\n> >  \n> > +/*\n> \n> For doxygen documentation comments, there should be two *'s:\n> \n>  /**\n> \n> > + * \\var mbusFormatInfo\n> > + * \\brief A struct of bits per pixel and mbus format\n> We wouldn't normally describe a struct as \"a struct\" in the brief. The\n> type tells us that already.\n> \n> Just saying what the data represented by it is enough.\n> \n> \n> > + */\n> > +struct mbusFormatInfo {\n> > +\tunsigned int bits;\n> > +\tstd::string format;\n> > +};\n> > +\n> >  /*\n> >   * \\var formatInfoMap\n> > - * \\brief A map that associates bits per pixel to V4L2 media bus codes\n> > + * \\brief A map that associates mbusFormatInfo struct to V4L2 media bus codes\n> >   */\n> > -const std::map<uint32_t, unsigned int> formatInfoMap = {\n> > -\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_BGR565_2X8_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_BGR565_2X8_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB565_2X8_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB565_2X8_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_RGB666_1X18, 18 },\n> > -\t{ V4L2_MBUS_FMT_RGB888_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_RGB888_2X12_BE, 24 },\n> > -\t{ V4L2_MBUS_FMT_RGB888_2X12_LE, 24 },\n> > -\t{ V4L2_MBUS_FMT_ARGB8888_1X32, 32 },\n> > -\t{ V4L2_MBUS_FMT_Y8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_UV8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_UYVY8_1_5X8, 12 },\n> > -\t{ V4L2_MBUS_FMT_VYUY8_1_5X8, 12 },\n> > -\t{ V4L2_MBUS_FMT_YUYV8_1_5X8, 12 },\n> > -\t{ V4L2_MBUS_FMT_YVYU8_1_5X8, 12 },\n> > -\t{ V4L2_MBUS_FMT_UYVY8_2X8, 16 },\n> > -\t{ V4L2_MBUS_FMT_VYUY8_2X8, 16 },\n> > -\t{ V4L2_MBUS_FMT_YUYV8_2X8, 16 },\n> > -\t{ V4L2_MBUS_FMT_YVYU8_2X8, 16 },\n> > -\t{ V4L2_MBUS_FMT_Y10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_UYVY10_2X10, 20 },\n> > -\t{ V4L2_MBUS_FMT_VYUY10_2X10, 20 },\n> > -\t{ V4L2_MBUS_FMT_YUYV10_2X10, 20 },\n> > -\t{ V4L2_MBUS_FMT_YVYU10_2X10, 20 },\n> > -\t{ V4L2_MBUS_FMT_Y12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_UYVY8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_VYUY8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_YUYV8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_YVYU8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_YDYUYDYV8_1X16, 16 },\n> > -\t{ V4L2_MBUS_FMT_UYVY10_1X20, 20 },\n> > -\t{ V4L2_MBUS_FMT_VYUY10_1X20, 20 },\n> > -\t{ V4L2_MBUS_FMT_YUYV10_1X20, 20 },\n> > -\t{ V4L2_MBUS_FMT_YVYU10_1X20, 20 },\n> > -\t{ V4L2_MBUS_FMT_YUV10_1X30, 30 },\n> > -\t{ V4L2_MBUS_FMT_AYUV8_1X32, 32 },\n> > -\t{ V4L2_MBUS_FMT_UYVY12_2X12, 24 },\n> > -\t{ V4L2_MBUS_FMT_VYUY12_2X12, 24 },\n> > -\t{ V4L2_MBUS_FMT_YUYV12_2X12, 24 },\n> > -\t{ V4L2_MBUS_FMT_YVYU12_2X12, 24 },\n> > -\t{ V4L2_MBUS_FMT_UYVY12_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_VYUY12_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_YUYV12_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_YVYU12_1X24, 24 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_ALAW8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG10_ALAW8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB10_ALAW8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8, 8 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE, 16 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE, 16 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB10_1X10, 10 },\n> > -\t{ V4L2_MBUS_FMT_SBGGR12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_SGBRG12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_SGRBG12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_SRGGB12_1X12, 12 },\n> > -\t{ V4L2_MBUS_FMT_AHSV8888_1X32, 32 },\n> > +const std::map<uint32_t, mbusFormatInfo> formatInfoMap = {\n> > +\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_BE, { 16, \"RGB444_2X8_PADHI_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB444_2X8_PADHI_LE, { 16, \"RGB444_2X8_PADHI_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_BE, { 16, \"RGB555_2X8_PADHI_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB555_2X8_PADHI_LE, { 16, \"RGB555_2X8_PADHI_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_BGR565_2X8_BE, { 16, \"BGR565_2X8_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_BGR565_2X8_LE, { 16, \"BGR565_2X8_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB565_2X8_BE, { 16, \"RGB565_2X8_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB565_2X8_LE, { 16, \"RGB565_2X8_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB666_1X18, { 18, \"RGB666_1X18\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB888_1X24, { 24, \"RGB888_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB888_2X12_BE, { 24, \"RGB888_2X12_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_RGB888_2X12_LE, { 24, \"RGB888_2X12_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_ARGB8888_1X32, { 32, \"ARGB8888_1X32\" } },\n> > +\t{ V4L2_MBUS_FMT_Y8_1X8, { 8, \"Y8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_UV8_1X8, { 8, \"UV8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY8_1_5X8, { 12, \"UYVY8_1_5X8\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY8_1_5X8, { 12, \"VYUY8_1_5X8\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV8_1_5X8, { 12, \"YUYV8_1_5X8\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU8_1_5X8, { 12, \"YVYU8_1_5X8\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY8_2X8, { 16, \"UYVY8_2X8\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY8_2X8, { 16, \"VYUY8_2X8\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV8_2X8, { 16, \"YUYV8_2X8\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU8_2X8, { 16, \"YVYU8_2X8\" } },\n> > +\t{ V4L2_MBUS_FMT_Y10_1X10, { 10, \"Y10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY10_2X10, { 20, \"UYVY10_2X10\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY10_2X10, { 20, \"VYUY10_2X10\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV10_2X10, { 20, \"YUYV10_2X10\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU10_2X10, { 20, \"YVYU10_2X10\" } },\n> > +\t{ V4L2_MBUS_FMT_Y12_1X12, { 12, \"Y12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY8_1X16, { 16, \"UYVY8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY8_1X16, { 16, \"VYUY8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV8_1X16, { 16, \"YUYV8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU8_1X16, { 16, \"YVYU8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_YDYUYDYV8_1X16, { 16, \"YDYUYDYV8_1X16\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY10_1X20, { 20, \"UYVY10_1X20\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY10_1X20, { 20, \"VYUY10_1X20\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV10_1X20, { 20, \"YUYV10_1X20\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU10_1X20, { 20, \"YVYU10_1X20\" } },\n> > +\t{ V4L2_MBUS_FMT_YUV10_1X30, { 30, \"YUV10_1X30\" } },\n> > +\t{ V4L2_MBUS_FMT_AYUV8_1X32, { 32, \"AYUV8_1X32\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY12_2X12, { 24, \"UYVY12_2X12\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY12_2X12, { 24, \"VYUY12_2X12\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV12_2X12, { 24, \"YUYV12_2X12\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU12_2X12, { 24, \"YVYU12_2X12\" } },\n> > +\t{ V4L2_MBUS_FMT_UYVY12_1X24, { 24, \"UYVY12_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_VYUY12_1X24, { 24, \"VYUY12_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_YUYV12_1X24, { 24, \"YUYV12_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_YVYU12_1X24, { 24, \"YVYU12_1X24\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR8_1X8, { 8, \"SBGGR8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG8_1X8, { 8, \"SGBRG8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG8_1X8, { 8, \"SGRBG8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB8_1X8, { 8, \"SRGGB8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_ALAW8_1X8, { 8, \"SBGGR10_ALAW8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG10_ALAW8_1X8, { 8, \"SGBRG10_ALAW8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG10_ALAW8_1X8, { 8, \"SGRBG10_ALAW8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB10_ALAW8_1X8, { 8, \"SRGGB10_ALAW8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_DPCM8_1X8, { 8, \"SBGGR10_DPCM8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG10_DPCM8_1X8, { 8, \"SGBRG10_DPCM8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG10_DPCM8_1X8, { 8, \"SGRBG10_DPCM8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB10_DPCM8_1X8, { 8, \"SRGGB10_DPCM8_1X8\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_BE, { 16, \"SBGGR10_2X8_PADHI_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADHI_LE, { 16, \"SBGGR10_2X8_PADHI_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_BE, { 16, \"SBGGR10_2X8_PADLO_BE\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_2X8_PADLO_LE, { 16, \"SBGGR10_2X8_PADLO_LE\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR10_1X10, { 10, \"SBGGR10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG10_1X10, { 10, \"SGBRG10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG10_1X10, { 10, \"SGRBG10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB10_1X10, { 10, \"SRGGB10_1X10\" } },\n> > +\t{ V4L2_MBUS_FMT_SBGGR12_1X12, { 12, \"SBGGR12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_SGBRG12_1X12, { 12, \"SGBRG12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_SGRBG12_1X12, { 12, \"SGRBG12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_SRGGB12_1X12, { 12, \"SRGGB12_1X12\" } },\n> > +\t{ V4L2_MBUS_FMT_AHSV8888_1X32, { 32, \"AHSV8888_1X32\" } },\n> >  };\n> >  \n> >  } /* namespace */\n> > @@ -161,9 +170,14 @@ const std::map<uint32_t, unsigned int> formatInfoMap = {\n> >   */\n> >  const std::string V4L2SubdeviceFormat::toString() const\n> >  {\n> > -\tstd::stringstream ss;\n> > -\tss << size.toString() << \"-\" << utils::hex(mbus_code, 4);\n> > -\treturn ss.str();\n> > +\tconst auto it = formatInfoMap.find(mbus_code);\n> > +\tstd::stringstream mbus;\n> \n> And this last one is only a stylistic 'nit' and is quite minor, if a\n> function grows in complexity we would normally put extra spacing in to\n> ease readability.\n> \n> The original function was three lines, so it was packed without spaces\n> to keep it short, but here we now have three parts.\n> \n> {\n> \tLocal Variables\n> \n>  \tCode\n> \n> \tReturn statement.\n> }\n> \n> You've already got a space between the Code and return, so to match it\n> should probably have a space between the local variables and code.\n> \n> and then, because the auto iterator calls formatInfoMap.find() I'd put\n> that after the mbus instantiation...\n> \n> No need to send a v3 though - I'm just discussing stylistic things there.\n> \n> Otherwise, this patch looks good to me, so if we get another reviewed-by\n> tag, I can apply with the small style fixups if you are happy.\n> \n> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> \n> > +\tif (it == formatInfoMap.end())\n> > +\t\tmbus << size.toString() << \"-\" << utils::hex(mbus_code, 4);\n> > +\telse\n> > +\t\tmbus << size.toString() << \"-\" << it->second.format;\n\nHow about the following ?\n\n\tstd::stringstream mbus;\n\tmbus << size.toString() << \"-\";\n\n\tconst auto it = formatInfoMap.find(mbus_code);\n\tif (it == formatInfoMap.end())\n\t\tmbus << utils::hex(mbus_code, 4);\n\telse\n\t\tmbus << it->second.format;\n\n\treturn mbus.str();\n\n> > +\n> > +\treturn mbus.str();\n> >  }\n> >  \n> >  /**\n> > @@ -180,7 +194,7 @@ uint8_t V4L2SubdeviceFormat::bitsPerPixel() const\n> >  \t\treturn 0;\n> >  \t}\n> >  \n> > -\treturn it->second;\n> > +\treturn it->second.bits;\n> >  }\n> >  \n> >  /**","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["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 ACC64603CC\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  1 Jun 2020 14:20:35 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2F710304;\n\tMon,  1 Jun 2020 14:20:35 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"lQER/Olr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1591014035;\n\tbh=8wUo28WBMPW7WRYkOMN/5rE73V3jcsiPymHHdfxrDxQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=lQER/OlrTDlvWAX1kfhOk+HVZvPokHIePBA+OJvAvJqNJ87aVwvgGpXXa2tDatoHv\n\tDRICzq/MF0ZbUa0qx5n8STHgoV682AdOKv0QZSpZro+v3fHi30sppxJjIDGPDRQefL\n\tpv71LEyg7N78h8g4Yms2gqn1vh+AK3gEtDBFMVyk=","Date":"Mon, 1 Jun 2020 15:20:20 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"Kaaira Gupta <kgupta@es.iitr.ac.in>, libcamera-devel@lists.libcamera.org","Message-ID":"<20200601122020.GD5886@pendragon.ideasonboard.com>","References":"<20200601110116.GA10868@kaaira-HP-Pavilion-Notebook>\n\t<1877c028-1b67-4cfa-8549-6973db1524be@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<1877c028-1b67-4cfa-8549-6973db1524be@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] libcamera: v4l2subdev: Print mbus\n\tstring instead of code","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 01 Jun 2020 12:20:35 -0000"}}]