[{"id":1930,"web_url":"https://patchwork.libcamera.org/comment/1930/","msgid":"<20190619094929.GC5045@pendragon.ideasonboard.com>","date":"2019-06-19T09:49:29","subject":"Re: [libcamera-devel] [PATCH v4 10/16] libcamera: stream: Add\n\tStreamFormats","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 Wed, Jun 19, 2019 at 04:51:23AM +0200, Niklas Söderlund wrote:\n> Add a StreamFormats class which describes all the formats supported by a\n> stream. The object does not collect any information itself but can\n> simplify user interactions with formats as it's able to translate a\n> stream format range into a discrete list and a discrete list to a range.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>\n> ---\n> * Changes since v3\n> - Fix spelling and grammar.\n> - Add \\todo markers for potential caching of calculated values.\n> - Change std::vector<> to std::array<> for list of sizes in sizes().\n> ---\n>  include/libcamera/stream.h |  16 +++\n>  src/libcamera/stream.cpp   | 230 +++++++++++++++++++++++++++++++++++++\n>  2 files changed, 246 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 <map>\n>  #include <string>\n>  #include <vector>\n>  \n> @@ -18,6 +19,21 @@ namespace libcamera {\n>  class Camera;\n>  class Stream;\n>  \n> +class StreamFormats\n> +{\n> +public:\n> +\tStreamFormats();\n> +\tStreamFormats(const std::map<unsigned int, std::vector<SizeRange>> &formats);\n> +\n> +\tstd::vector<unsigned int> pixelformats() const;\n> +\tstd::vector<Size> sizes(unsigned int pixelformat) const;\n> +\n> +\tSizeRange range(unsigned int pixelformat) const;\n> +\n> +private:\n> +\tstd::map<unsigned int, std::vector<SizeRange>> 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..bfb072d691ae0e25 100644\n> --- a/src/libcamera/stream.cpp\n> +++ b/src/libcamera/stream.cpp\n> @@ -7,9 +7,13 @@\n>  \n>  #include <libcamera/stream.h>\n>  \n> +#include <algorithm>\n> +#include <climits>\n>  #include <iomanip>\n>  #include <sstream>\n>  \n> +#include \"log.h\"\n> +\n>  /**\n>   * \\file stream.h\n>   * \\brief Video stream for a Camera\n> @@ -32,6 +36,232 @@\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. The range description can include\n> + * horizontal and vertical steps.\n> + *\n> + * When sizes are viewed as a list of discrete sizes they describe the exact\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 the StreamFormat is constructed using a list of discrete image sizes\n> + *    and a range is requested, it gets created by taking the minimum and\n> + *    maximum width/height in the list. The step information is not recreated\n> + *    and is set to 0 to indicate the range is generated.\n> + *\n> + *  - If the image sizes used to construct a StreamFormat are expressed as a\n> + *    range and a list of discrete sizes is requested, one which fits inside\n> + *    that range are selected from a list of common sizes. The step information\n> + *    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 number of possible Size combinations that may not be\n> + * 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 it has been verified using\n> + * CameraConfiguration::validate().\n\nI would move the todo you added below here, and write it as\n\n * \\todo Review the usage patterns of this class, and cache the\n * computed pixelformats(), sizes() and range() if this would improve\n * performances.\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> + */\n> +\n> +StreamFormats::StreamFormats()\n> +{\n> +}\n> +\n> +/**\n> + * \\brief Construct a StreamFormats object with a map of image formats\n> + * \\param[in] formats A map of pixel formats to a sizes description\n> + */\n> +StreamFormats::StreamFormats(const std::map<unsigned int, std::vector<SizeRange>> &formats)\n> +\t: formats_(formats)\n> +{\n> +}\n> +\n> +/**\n> + * \\brief Retrieve the list of supported pixel formats\n> + * \\return The list of supported pixel formats\n> + */\n> +std::vector<unsigned int> StreamFormats::pixelformats() const\n> +{\n> +\tstd::vector<unsigned int> formats;\n> +\n> +\t/* \\todo: Should this be cached instead of computed each time? */\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 supported for \\a pixelformat\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.\n> + *\n> + * If the sizes are described as a range, a list of discrete sizes are computed\n> + * from a list of common resolutions that fit inside the described range. When\n> + * computing the discrete list step values are considered but there are no\n> + * guarantees that all sizes computed are supported.\n> + *\n> + * \\todo: Should the sizes be cached instead of computed each time?\n> + *\n> + * \\return A list of frame sizes or an empty list on error\n> + */\n> +std::vector<Size> 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::array<Size, 53> 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<Size> 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<SizeRange> &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 range of minimum and maximum sizes\n> + * \\param[in] pixelformat Pixel format to retrieve range for\n> + *\n> + * If the size described for \\a pixelformat is a range, that range is returned\n> + * directly. If the sizes described are a list of discrete sizes, a range is\n> + * created from the minimum and maximum sizes in the list. The step values of\n> + * the range are set to 0 to indicate that the range is generated and that not\n> + * all image sizes contained in the range might be supported.\n> + *\n> + * \\return A range of valid image sizes or an empty range on error\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> +\n> +\tconst std::vector<SizeRange> &ranges = it->second;\n> +\tif (ranges.size() == 1)\n> +\t\treturn ranges[0];\n> +\n> +\tLOG(Stream, Debug) << \"Building range from discrete sizes\";\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>  /**\n>   * \\struct StreamConfiguration\n>   * \\brief Configuration parameters for a stream","headers":{"Return-Path":"<laurent.pinchart@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 DA34E61A13\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 19 Jun 2019 11:49:46 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E21EC333;\n\tWed, 19 Jun 2019 11:49:45 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1560937786;\n\tbh=iZseXH2XfCl3mNX++5difdZ3BsBXNdr4EZSaZYYT+do=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=t1Q6rP5dLbqO0o+uB3+8pWmCd9afcu0P1PaTxAlS1+rHv3QNVKK3L1oETomi5KbH3\n\tpaXnX8d2kss/upqN0XMqstwbcaej90IORSRts1eyiwRP7uVCBrmqLiFO7VHXzfArX+\n\tMZBUsUgxVGw9l1Qhm5vqxSX2EeEVfS4elJxFZ3aE=","Date":"Wed, 19 Jun 2019 12:49:29 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190619094929.GC5045@pendragon.ideasonboard.com>","References":"<20190619025129.21164-1-niklas.soderlund@ragnatech.se>\n\t<20190619025129.21164-11-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":"<20190619025129.21164-11-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v4 10/16] libcamera: stream: Add\n\tStreamFormats","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","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":"Wed, 19 Jun 2019 09:49:47 -0000"}}]