[{"id":1893,"web_url":"https://patchwork.libcamera.org/comment/1893/","msgid":"<20190613164248.6mwh2gu56ux3ety7@uno.localdomain>","date":"2019-06-13T16:42:48","subject":"Re: [libcamera-devel] [PATCH v2 10/16] libcamera: stream: Add\n\tStreamFormats","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Wed, Jun 12, 2019 at 02:43:53AM +0200, Niklas Söderlund wrote:\n> Add a StreamFormats class which describes all the formats a stream can\n\ns/can support/supported by a stream/ ?\n\n> support. The object does not collect any formation itself but can\n\ninformation\n\n> simplify user interactions with formats as it's able to translate a\n> stream format range into discrete list and a discrete list to a range.\n>\n> Signed-off-by: Niklas Söderlund \n> ---\n> include/libcamera/stream.h | 16 +++\n> src/libcamera/stream.cpp | 228 +++++++++++++++++++++++++++++++++++++\n> 2 files changed, 244 insertions(+)\n>\n> diff --git a/include/libcamera/stream.h b/include/libcamera/stream.h\n> index e38c0e7e827d5888..48daf5ac23f55d85 100644\n> --- a/include/libcamera/stream.h\n> +++ b/include/libcamera/stream.h\n> @@ -7,6 +7,7 @@\n> #ifndef __LIBCAMERA_STREAM_H__\n> #define __LIBCAMERA_STREAM_H__\n>\n> +#include \n> #include \n> #include \n>\n> @@ -18,6 +19,21 @@ namespace libcamera {\n> class Camera;\n> class Stream;\n>\n> +class StreamFormats\n> +{\n> +public:\n> +\tStreamFormats();\n\nWhat's the purpose of an empy stream format, since you cannot add\nformats to it once created?\n\n\n> +\tStreamFormats(const std::map> &formats);\n> +\n> +\tstd::vector pixelformats() const;\n> +\tstd::vector sizes(unsigned int pixelformat) const;\n> +\n> +\tSizeRange range(unsigned int pixelformat) const;\n> +\n> +private:\n> +\tstd::map> formats_;\n> +};\n> +\n> struct StreamConfiguration {\n> \tStreamConfiguration()\n> \t\t: stream_(nullptr)\n> diff --git a/src/libcamera/stream.cpp b/src/libcamera/stream.cpp\n> index 0c59a31a3a0501d3..52340c2644aac8d8 100644\n> --- a/src/libcamera/stream.cpp\n> +++ b/src/libcamera/stream.cpp\n> @@ -7,9 +7,13 @@\n>\n> #include \n>\n> +#include \n> +#include \n> #include \n> #include \n>\n> +#include \"log.h\"\n> +\n> /**\n> * \\file stream.h\n> * \\brief Video stream for a Camera\n> @@ -32,6 +36,230 @@\n>\n> namespace libcamera {\n>\n> +LOG_DEFINE_CATEGORY(Stream)\n> +\n> +/**\n> + * \\class StreamFormats\n> + * \\brief Hold information about supported stream formats\n> + *\n> + * The StreamFormats class holds information about the pixel formats and frame\n> + * sizes a stream supports. The class groups size information by the pixel\n> + * format which can produce it.\n> + *\n> + * There are two ways to examine the size information, as a range or as a list\n> + * of discrete sizes. When sizes are viewed as a range it describes the minimum\n> + * and maximum width and height values. There is a possibility to supplement the\n> + * range description with a horizontal and vertical step. The step describes the\n> + * size in pixels from the minimum with/height.\n\npixels from/pixels starting from/\n\n> + *\n> + * When sizes are viewed as a list of discrete sizes they describe exact\n\nthe exact\n\n> + * dimensions which can be selected and used.\n> + *\n> + * Pipeline handlers can create StreamFormats describing each pixel format using\n> + * either a range or a list of discrete sizes. The StreamFormats class attempts\n> + * to translate between the two different ways to view them. The translations\n> + * are performed as:\n> + *\n> + * - If a pixel format is described as a list of discrete sizes then a range is\n\nIf the StreamFormat is constructed using a list of discrete image\nsizes and a range is requested, it gets created...\n\n> + * created by taking the minimum and maximum width/height in the list. The\n> + * step information is not recreated and is set to 0 to indicate it is\n\nto indicate the range is\n\n> + * generated.\n> + *\n> + * - If a pixel format is described as a range then a list of discrete sizes\n\nIf the image sizes used to construct a StreamFormat are expressed as a\nrange and a list of discrete sizes is requested, one which fits...\n\n> + * which fits inside that range are selected from a list of common sizes. The\n> + * step information is taken into consideration when generating the sizes.\n> + *\n> + * Applications examining sizes as a range with step values of 0 shall be\n> + * aware that the range are generated from a list of discrete sizes and there\n> + * could be a large quantity of possible Size combinations which may not\n> + * be supported by the Stream.\n> + *\n> + * All sizes retrieved from StreamFormats shall be treated as advisory and no\n> + * size shall be considered to be supported until its been verified using\n> + * CameraConfiguration::validate().\n> + */\n> +\n> +/**\n> + * \\brief Construct a empty StreamFormats object\n> + */\n> +StreamFormats::StreamFormats()\n> +{\n> +}\n\nI would drop this.\n\n> +\n> +/**\n> + * \\brief Construct a StreamFormats object\n\nwith a map of image formats\n\n> + * \\param[in] formats A map of pixel formats to a sizes description\n> + */\n> +StreamFormats::StreamFormats(const std::map> &formats)\n> +\t: formats_(formats)\n> +{\n> +}\n> +\n> +/**\n> + * \\brief Retrieve the list of supported pixel formats\n> + * \\returns The list of supported pixel formats\n> + */\n> +std::vector StreamFormats::pixelformats() const\n> +{\n> +\tstd::vector formats;\n> +\n> +\tfor (auto const &it : formats_)\n> +\t\tformats.push_back(it.first);\n> +\n> +\treturn formats;\n> +}\n> +\n> +/**\n> + * \\brief Retrieve the list of frame sizes\n\nsupported by \\a pixelformats\n?\n\n> + * \\param[in] pixelformat Pixel format to retrieve sizes for\n> + *\n> + * If the sizes described for \\a pixelformat are discrete they are returned\n> + * directly. If the size described is a range a list of discrete sizes are\n> + * computed from a set list of common resolutions which fits inside the\n> + * described range. When computing the discrete list step values are considered\n> + * but there are no guarantees that all sizes computed are supported.\n> + *\n> + * \\returns List description of frame sizes\n\nA list of frame sizes or an empty list on error\n\n> + */\n> +std::vector StreamFormats::sizes(unsigned int pixelformat) const\n> +{\n> +\t/*\n> +\t * Sizes to try and extract from ranges.\n> +\t * \\todo Verify list of resolutions are good, current list compiled\n> +\t * from v4l2 documentation and source code as well as lists of\n> +\t * common frame sizes.\n> +\t */\n> +\tstatic const std::vector rangeDiscreteSizes = {\n> +\t\tSize(160, 120),\n> +\t\tSize(240, 160),\n> +\t\tSize(320, 240),\n> +\t\tSize(400, 240),\n> +\t\tSize(480, 320),\n> +\t\tSize(640, 360),\n> +\t\tSize(640, 480),\n> +\t\tSize(720, 480),\n> +\t\tSize(720, 576),\n> +\t\tSize(768, 480),\n> +\t\tSize(800, 600),\n> +\t\tSize(854, 480),\n> +\t\tSize(960, 540),\n> +\t\tSize(960, 640),\n> +\t\tSize(1024, 576),\n> +\t\tSize(1024, 600),\n> +\t\tSize(1024, 768),\n> +\t\tSize(1152, 864),\n> +\t\tSize(1280, 1024),\n> +\t\tSize(1280, 1080),\n> +\t\tSize(1280, 720),\n> +\t\tSize(1280, 800),\n> +\t\tSize(1360, 768),\n> +\t\tSize(1366, 768),\n> +\t\tSize(1400, 1050),\n> +\t\tSize(1440, 900),\n> +\t\tSize(1536, 864),\n> +\t\tSize(1600, 1200),\n> +\t\tSize(1600, 900),\n> +\t\tSize(1680, 1050),\n> +\t\tSize(1920, 1080),\n> +\t\tSize(1920, 1200),\n> +\t\tSize(2048, 1080),\n> +\t\tSize(2048, 1152),\n> +\t\tSize(2048, 1536),\n> +\t\tSize(2160, 1080),\n> +\t\tSize(2560, 1080),\n> +\t\tSize(2560, 1440),\n> +\t\tSize(2560, 1600),\n> +\t\tSize(2560, 2048),\n> +\t\tSize(2960, 1440),\n> +\t\tSize(3200, 1800),\n> +\t\tSize(3200, 2048),\n> +\t\tSize(3200, 2400),\n> +\t\tSize(3440, 1440),\n> +\t\tSize(3840, 1080),\n> +\t\tSize(3840, 1600),\n> +\t\tSize(3840, 2160),\n> +\t\tSize(3840, 2400),\n> +\t\tSize(4096, 2160),\n> +\t\tSize(5120, 2160),\n> +\t\tSize(5120, 2880),\n> +\t\tSize(7680, 4320),\n> +\t};\n> +\tstd::vector sizes;\n> +\n> +\t/* Make sure pixel format exists. */\n> +\tauto const &it = formats_.find(pixelformat);\n> +\tif (it == formats_.end())\n> +\t\treturn {};\n> +\n> +\t/* Try creating a list of discrete sizes. */\n> +\tconst std::vector &ranges = it->second;\n> +\tbool discrete = true;\n> +\tfor (const SizeRange &range : ranges) {\n> +\t\tif (range.min != range.max) {\n> +\t\t\tdiscrete = false;\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tsizes.emplace_back(range.min);\n> +\t}\n> +\n> +\t/* If discrete not possible generate from range. */\n> +\tif (!discrete) {\n> +\t\tif (ranges.size() != 1) {\n> +\t\t\tLOG(Stream, Error) << \"Range format is ambiguous\";\n> +\t\t\treturn {};\n> +\t\t}\n> +\n> +\t\tconst SizeRange &limit = ranges.front();\n> +\t\tsizes.clear();\n> +\n> +\t\tfor (const Size &size : rangeDiscreteSizes)\n> +\t\t\tif (limit.contains(size))\n> +\t\t\t\tsizes.push_back(size);\n> +\t}\n> +\n> +\tstd::sort(sizes.begin(), sizes.end());\n> +\n> +\treturn sizes;\n> +}\n> +\n> +/**\n> + * \\brief Retrieve the frame size range\n\nRetrieve the range of minimum and maximu sizes ? (not sure to mention\npixelformat here...)\n\n> + * \\param[in] pixelformat Pixel format to retrieve range for\n> + *\n> + * If the size described for \\a pixelformat is a range the range is returned\n\nthat range\n\n> + * directly. If the sizes described are a list of discrete sizes a range is\n> + * computed from the minimum and maxim sizes in the list. The step values of\n> + * a computed range is set to 0 to indicate that the range is generated and\n> + * that the full range may not supported.\n\nthat not all image sizes contained in the range might be supported\n\n> + *\n> + * \\returns Range description of frame size\n\nA range of valid image sizes or an empty range on error.\n\n> + */\n> +SizeRange StreamFormats::range(unsigned int pixelformat) const\n> +{\n> +\tauto const it = formats_.find(pixelformat);\n> +\tif (it == formats_.end())\n> +\t\treturn {};\n\nWe're returning an empty range? How does the caller finds out?\n\n> +\n> +\tconst std::vector &ranges = it->second;\n> +\tif (ranges.size() == 1)\n> +\t\treturn ranges[0];\n> +\n> +\tLOG(Stream, Debug) << \"Building range from discrete\";\n\nleftover or intentional?\n\n> +\tSizeRange range(UINT_MAX, UINT_MAX, 0, 0);\n> +\tfor (const SizeRange &limit : ranges) {\n> +\t\tif (limit.min < range.min)\n> +\t\t\trange.min = limit.min;\n> +\n> +\t\tif (limit.max > range.max)\n> +\t\t\trange.max = limit.max;\n> +\t}\n> +\n> +\trange.hStep = 0;\n> +\trange.vStep = 0;\n> +\n> +\treturn range;\n> +}\n> +\n\nMostly notes on comments, take what you like from it.\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> /**\n> * \\struct StreamConfiguration\n> * \\brief Configuration parameters for a stream\n> --\n> 2.21.0\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C8BF16389F\n\tfor ;\n\tThu, 13 Jun 2019 18:41:35 +0200 (CEST)","from uno.localdomain (2-224-242-101.ip172.fastwebnet.it\n\t[2.224.242.101]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 46DC2FF806;\n\tThu, 13 Jun 2019 16:41:35 +0000 (UTC)"],"X-Originating-IP":"2.224.242.101","Date":"Thu, 13 Jun 2019 18:42:48 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190613164248.6mwh2gu56ux3ety7@uno.localdomain>","References":"<20190612004359.15772-1-niklas.soderlund@ragnatech.se>\n\t<20190612004359.15772-11-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"multipart/signed; micalg=pgp-sha256;\n\tprotocol=\"application/pgp-signature\"; boundary=\"p2vqqqsx2tlndkxs\"","Content-Disposition":"inline","In-Reply-To":"<20190612004359.15772-11-niklas.soderlund@ragnatech.se>","User-Agent":"NeoMutt/20180716","Subject":"Re: [libcamera-devel] [PATCH v2 10/16] libcamera: stream: Add\n\tStreamFormats","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":"Thu, 13 Jun 2019 16:41:36 -0000"}}]