[{"id":1805,"web_url":"https://patchwork.libcamera.org/comment/1805/","msgid":"<20190609125122.GH4778@pendragon.ideasonboard.com>","date":"2019-06-09T12:51:22","subject":"Re: [libcamera-devel] [PATCH 08/17] libcamera: v4l2_subdevice:\n\tReplace FormatEnum with V4L2SubdeviceFormats","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Mon, May 27, 2019 at 02:15:34AM +0200, Niklas Söderlund wrote:\n> Replace all usage of FormatEnum with V4L2SubdeviceFormats and completely\n> remove FormatEnum which is no longer needed.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/camera_sensor.cpp | 12 +++++-------\n> src/libcamera/formats.cpp | 11 -----------\n> src/libcamera/include/formats.h | 2 --\n> src/libcamera/include/v4l2_subdevice.h | 2 +-\n> src/libcamera/v4l2_subdevice.cpp | 6 +++---\n> test/v4l2_subdevice/list_formats.cpp | 10 +++++-----\n> 6 files changed, 14 insertions(+), 29 deletions(-)\n> \n> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\n> index 2b9d8fa593c13177..e46bc28c7ac4a8e1 100644\n> --- a/src/libcamera/camera_sensor.cpp\n> +++ b/src/libcamera/camera_sensor.cpp\n> @@ -90,27 +90,25 @@ int CameraSensor::init()\n> \t\treturn ret;\n> \n> \t/* Enumerate and cache media bus codes and sizes. */\n> -\tconst FormatEnum formats = subdev_->formats(0);\n> +\tconst V4L2SubdeviceFormats formats = subdev_->formats(0);\n> \tif (formats.empty()) {\n> \t\tLOG(CameraSensor, Error) << \"No image format found\";\n> \t\treturn -EINVAL;\n> \t}\n> \n> -\tstd::transform(formats.begin(), formats.end(),\n> -\t\t std::back_inserter(mbusCodes_),\n> -\t\t [](decltype(*formats.begin()) f) { return f.first; });\n> +\tmbusCodes_ = formats.codes();\n> \n> \t/*\n> \t * Extract the supported sizes from the first format as we only support\n> \t * sensors that offer the same frame sizes for all media bus codes.\n> \t * Verify this assumption and reject the sensor if it isn't true.\n> \t */\n> -\tconst std::vector &sizes = formats.begin()->second;\n> +\tconst std::vector &sizes = formats.sizes(mbusCodes_[0]);\n> \tstd::transform(sizes.begin(), sizes.end(), std::back_inserter(sizes_),\n> \t\t [](const SizeRange &range) { return range.max; });\n> \n> -\tfor (auto it = ++formats.begin(); it != formats.end(); ++it) {\n> -\t\tif (it->second != sizes) {\n> +\tfor (unsigned int code : mbusCodes_) {\n> +\t\tif (formats.sizes(code) != sizes) {\n> \t\t\tLOG(CameraSensor, Error)\n> \t\t\t\t<< \"Frame sizes differ between media bus codes\";\n> \t\t\treturn -EINVAL;\n> diff --git a/src/libcamera/formats.cpp b/src/libcamera/formats.cpp\n> index f5841b38cea5686c..7b2097413c3b7378 100644\n> --- a/src/libcamera/formats.cpp\n> +++ b/src/libcamera/formats.cpp\n> @@ -14,17 +14,6 @@\n> \n> namespace libcamera {\n> \n> -/**\n> - * \\typedef FormatEnum\n> - * \\brief Type definition for the map of image formats and sizes\n> - *\n> - * Type definition used to enumerate the supported pixel formats and image\n> - * frame sizes. The type associates in a map a pixel format (for memory\n> - * formats) or a media bus code (for bus formats), to a vector of image\n> - * resolutions represented by SizeRange items.\n> - */\n> -\n> -\n> /**\n> * \\class DeviceFormats\n> * \\brief Base class for V4L2Device and V4L2SubDevice Formats\n> diff --git a/src/libcamera/include/formats.h b/src/libcamera/include/formats.h\n> index 372f6e6d71b236dd..2e849e70c4a88aeb 100644\n> --- a/src/libcamera/include/formats.h\n> +++ b/src/libcamera/include/formats.h\n> @@ -15,8 +15,6 @@\n> \n> namespace libcamera {\n> \n> -typedef std::map> FormatEnum;\n> -\n> class DeviceFormats\n> {\n> public:\n> diff --git a/src/libcamera/include/v4l2_subdevice.h b/src/libcamera/include/v4l2_subdevice.h\n> index c6fdf417b43c0423..a6d691ea540a4e5e 100644\n> --- a/src/libcamera/include/v4l2_subdevice.h\n> +++ b/src/libcamera/include/v4l2_subdevice.h\n> @@ -45,7 +45,7 @@ public:\n> \tint setCrop(unsigned int pad, Rectangle *rect);\n> \tint setCompose(unsigned int pad, Rectangle *rect);\n> \n> -\tFormatEnum formats(unsigned int pad);\n> +\tV4L2SubdeviceFormats formats(unsigned int pad);\n> \n> \tint getFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> \tint setFormat(unsigned int pad, V4L2SubdeviceFormat *format);\n> diff --git a/src/libcamera/v4l2_subdevice.cpp b/src/libcamera/v4l2_subdevice.cpp\n> index 8720013600ddb95f..fe393c66729e8511 100644\n> --- a/src/libcamera/v4l2_subdevice.cpp\n> +++ b/src/libcamera/v4l2_subdevice.cpp\n> @@ -200,9 +200,9 @@ int V4L2Subdevice::setCompose(unsigned int pad, Rectangle *rect)\n> * \\return A map of image formats associated with a list of image sizes, or\n> * an empty map on error or if the pad does not exist\n> */\n> -FormatEnum V4L2Subdevice::formats(unsigned int pad)\n> +V4L2SubdeviceFormats V4L2Subdevice::formats(unsigned int pad)\n> {\n> -\tFormatEnum formatMap = {};\n> +\tstd::map> formatMap = {};\n> \n> \tif (pad >= entity_->pads().size()) {\n> \t\tLOG(V4L2Subdev, Error) << \"Invalid pad: \" << pad;\n> @@ -212,7 +212,7 @@ FormatEnum V4L2Subdevice::formats(unsigned int pad)\n> \tfor (unsigned int code : enumPadCodes(pad))\n> \t\tformatMap[code] = enumPadSizes(pad, code);\n> \n> -\treturn formatMap;\n> +\treturn V4L2SubdeviceFormats(formatMap);\n\nApart from the V4L2SubdeviceFormats constructor that requires building a\nmap first this patch looks good to me.\n\nReviewed-by: Laurent Pinchart \n\n> }\n> \n> /**\n> diff --git a/test/v4l2_subdevice/list_formats.cpp b/test/v4l2_subdevice/list_formats.cpp\n> index 3f0edafcdcd72d6b..1c044a969961d172 100644\n> --- a/test/v4l2_subdevice/list_formats.cpp\n> +++ b/test/v4l2_subdevice/list_formats.cpp\n> @@ -47,7 +47,7 @@ void ListFormatsTest::printFormats(unsigned int pad,\n> int ListFormatsTest::run()\n> {\n> \t/* List all formats available on existing \"Scaler\" pads. */\n> -\tstd::map> formats;\n> +\tV4L2SubdeviceFormats formats;\n> \n> \tformats = scaler_->formats(0);\n> \tif (formats.empty()) {\n> @@ -55,8 +55,8 @@ int ListFormatsTest::run()\n> \t\t << scaler_->entity()->name() << endl;\n> \t\treturn TestFail;\n> \t}\n> -\tfor (auto it = formats.begin(); it != formats.end(); ++it)\n> -\t\tprintFormats(0, it->first, it->second);\n> +\tfor (unsigned int code : formats.codes())\n> +\t\tprintFormats(0, code, formats.sizes(code));\n> \n> \tformats = scaler_->formats(1);\n> \tif (formats.empty()) {\n> @@ -64,8 +64,8 @@ int ListFormatsTest::run()\n> \t\t << scaler_->entity()->name() << endl;\n> \t\treturn TestFail;\n> \t}\n> -\tfor (auto it = formats.begin(); it != formats.end(); ++it)\n> -\t\tprintFormats(1, it->first, it->second);\n> +\tfor (unsigned int code : formats.codes())\n> +\t\tprintFormats(1, code, formats.sizes(code));\n> \n> \t/* List format on a non-existing pad, format vector shall be empty. */\n> \tformats = scaler_->formats(2);","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A7C4E62EFB\n\tfor ;\n\tSun, 9 Jun 2019 14:51:36 +0200 (CEST)","from pendragon.ideasonboard.com (unknown\n\t[IPv6:2a02:a03f:44f0:8500:ca05:8177:199c:fed4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 39F2D5D;\n\tSun, 9 Jun 2019 14:51:36 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1560084696;\n\tbh=M8L/MrRN7viyiznGWBy76uBGWcl2W//h69iYGP7SdWA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=RQ3KNKVg2//wDEch1j4M+g3apoVS0mtxWjhDSBXIldFcwKj7vVSxlVDCkFtQqNf50\n\t64xWDpD+xr5kMnb3/XCgCNR6dAnBP2OKyXtL3z3fhi5xS3wSNTG2C6LKY7E6ceVoqn\n\thAA6TZqPAamva72WbUDXtmWrrl84Ta+Wel5Yauyw=","Date":"Sun, 9 Jun 2019 15:51:22 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190609125122.GH4778@pendragon.ideasonboard.com>","References":"<20190527001543.13593-1-niklas.soderlund@ragnatech.se>\n\t<20190527001543.13593-9-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190527001543.13593-9-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 08/17] libcamera: v4l2_subdevice:\n\tReplace FormatEnum with V4L2SubdeviceFormats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sun, 09 Jun 2019 12:51:36 -0000"}}]