Patch Detail
Show a patch.
GET /api/patches/3680/?format=api
{ "id": 3680, "url": "https://patchwork.libcamera.org/api/patches/3680/?format=api", "web_url": "https://patchwork.libcamera.org/patch/3680/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20200503123439.15994-1-laurent.pinchart@ideasonboard.com>", "date": "2020-05-03T12:34:39", "name": "[libcamera-devel] libcamera: camera_sensor: Relax restriction on sizes", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "1d64ca20d62954fbf45d4f38a5960bf0c1363745", "submitter": { "id": 2, "url": "https://patchwork.libcamera.org/api/people/2/?format=api", "name": "Laurent Pinchart", "email": "laurent.pinchart@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/3680/mbox/", "series": [ { "id": 878, "url": "https://patchwork.libcamera.org/api/series/878/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=878", "date": "2020-05-03T12:34:39", "name": "[libcamera-devel] libcamera: camera_sensor: Relax restriction on sizes", "version": 1, "mbox": "https://patchwork.libcamera.org/series/878/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/3680/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/3680/checks/", "tags": {}, "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 6E03461469\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 3 May 2020 14:34:47 +0200 (CEST)", "from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D2E51304\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 3 May 2020 14:34:46 +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=\"rOMdkTa/\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1588509287;\n\tbh=p5r+AuziC6aeavjFjvQ7PA9eNcHX+qNme886RpTiIYg=;\n\th=From:To:Subject:Date:From;\n\tb=rOMdkTa/ZoAL2ErjteLXsqewE1jdfGbfxX45S3jj3R59IX7d/hApmDyTIJCZ6FVpR\n\t8ULhBfi0GnHHgYVtkZyXVCwV+iZMggD2tRqQIJLyXIp52fGfnl+7r9qGLPqo+CNB14\n\tghgE83dhb+qFpBZrF1WDeaWs9LnaygCDzvx3VTJQ=", "From": "Laurent Pinchart <laurent.pinchart@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Sun, 3 May 2020 15:34:39 +0300", "Message-Id": "<20200503123439.15994-1-laurent.pinchart@ideasonboard.com>", "X-Mailer": "git-send-email 2.26.2", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "Subject": "[libcamera-devel] [PATCH] libcamera: camera_sensor: Relax\n\trestriction on sizes", "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": "Sun, 03 May 2020 12:34:47 -0000" }, "content": "The CameraSensor class assumes that camera sensors support the exact\nsame list of sizes of all media bus codes. While allowing a simpler API,\nthis assumption is incorrect and is blocking usage of some camera\nsensors.\n\nRelaxing the constraint is possible without changes to the CameraSensor\nAPI syntax, but requires changing its semantics. The sizes() function\nnow returns the list of all sizes for all media bus codes, and the\ngetFormat() function now searches in all supported media bus codes. The\nformer is likely not the most useful option for pipeline handlers, but\nthe sizes() function is currently unused. Designing a better API will\nrequire inspecting current and expected future use cases in pipeline\nhandlers to determine proper heuristics.\n\nWhile at it, fix a small typo in an unrelated comment.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/libcamera/camera_sensor.cpp | 135 +++++++++++++-------------\n src/libcamera/include/camera_sensor.h | 5 +-\n 2 files changed, 69 insertions(+), 71 deletions(-)", "diff": "diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp\nindex 4154700a19b5..abafb30eb977 100644\n--- a/src/libcamera/camera_sensor.cpp\n+++ b/src/libcamera/camera_sensor.cpp\n@@ -119,8 +119,7 @@ LOG_DEFINE_CATEGORY(CameraSensor);\n * information.\n *\n * The implementation is currently limited to sensors that expose a single V4L2\n- * subdevice with a single pad, and support the same frame sizes for all\n- * supported media bus codes. It will be extended to support more complex\n+ * subdevice with a single pad. It will be extended to support more complex\n * devices as the needs arise.\n */\n \n@@ -245,36 +244,34 @@ int CameraSensor::init()\n \t\tpropertyValue = 0;\n \tproperties_.set(properties::Rotation, propertyValue);\n \n-\t/* Enumerate and cache media bus codes and sizes. */\n-\tconst ImageFormats formats = subdev_->formats(pad_);\n-\tif (formats.isEmpty()) {\n+\t/* Enumerate, sort and cache media bus codes and sizes. */\n+\tformats_ = subdev_->formats(pad_);\n+\tif (formats_.isEmpty()) {\n \t\tLOG(CameraSensor, Error) << \"No image format found\";\n \t\treturn -EINVAL;\n \t}\n \n-\tmbusCodes_ = formats.formats();\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<SizeRange> &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 (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-\t\t}\n-\t}\n-\n-\t/* Sort the media bus codes and sizes. */\n+\tmbusCodes_ = formats_.formats();\n \tstd::sort(mbusCodes_.begin(), mbusCodes_.end());\n+\n+\tfor (const auto format : formats_.data()) {\n+\t\tconst std::vector<SizeRange> &ranges = format.second;\n+\t\tstd::transform(ranges.begin(), ranges.end(), std::back_inserter(sizes_),\n+\t\t\t [](const SizeRange &range) { return range.max; });\n+\t}\n+\n \tstd::sort(sizes_.begin(), sizes_.end());\n \n+\t/* Remove duplicates. */\n+\tauto last = std::unique(sizes_.begin(), sizes_.end());\n+\tsizes_.erase(last, sizes_.end());\n+\n+\t/*\n+\t * The sizes_ vector is sorted in ascending order, the resolution is\n+\t * thus the last element of the vector.\n+\t */\n+\tresolution_ = sizes_.back();\n+\n \treturn 0;\n }\n \n@@ -303,21 +300,18 @@ int CameraSensor::init()\n /**\n * \\fn CameraSensor::sizes()\n * \\brief Retrieve the frame sizes supported by the camera sensor\n+ *\n+ * The reported sizes span all media bus codes supported by the camera sensor.\n+ * Not all sizes may be supported by all media bus codes.\n+ *\n * \\return The supported frame sizes sorted in increasing order\n */\n \n /**\n+ * \\fn CameraSensor::resolution()\n * \\brief Retrieve the camera sensor resolution\n * \\return The camera sensor resolution in pixels\n */\n-const Size &CameraSensor::resolution() const\n-{\n-\t/*\n-\t * The sizes_ vector is sorted in ascending order, the resolution is\n-\t * thus the last element of the vector.\n-\t */\n-\treturn sizes_.back();\n-}\n \n /**\n * \\brief Retrieve the best sensor format for a desired output\n@@ -325,13 +319,13 @@ const Size &CameraSensor::resolution() const\n * \\param[in] size The desired size\n *\n * Media bus codes are selected from \\a mbusCodes, which lists all acceptable\n- * codes in decreasing order of preference. This method selects the first code\n- * from the list that is supported by the sensor. If none of the desired codes\n- * is supported, it returns an error.\n+ * codes in decreasing order of preference. Media bus codes supported by the\n+ * sensor but not listed in \\a mbusCodes are ignored. If none of the desired\n+ * codes is supported, it returns an error.\n *\n * \\a size indicates the desired size at the output of the sensor. This method\n- * selects the best size supported by the sensor according to the following\n- * criteria.\n+ * selects the best media bus code and size supported by the sensor according\n+ * to the following criteria.\n *\n * - The desired \\a size shall fit in the sensor output size to avoid the need\n * to up-scale.\n@@ -339,6 +333,11 @@ const Size &CameraSensor::resolution() const\n * need to crop the field of view.\n * - The sensor output size shall be as small as possible to lower the required\n * bandwidth.\n+ * - The desired \\a size shall be supported by one of the media bus code listed\n+ * in \\a mbusCodes.\n+ *\n+ * When multiple media bus codes can produce the same size, the code at the\n+ * lowest position in \\a mbusCodes is selected.\n *\n * The use of this method is optional, as the above criteria may not match the\n * needs of all pipeline handlers. Pipeline handlers may implement custom\n@@ -353,52 +352,48 @@ const Size &CameraSensor::resolution() const\n V4L2SubdeviceFormat CameraSensor::getFormat(const std::vector<unsigned int> &mbusCodes,\n \t\t\t\t\t const Size &size) const\n {\n-\tV4L2SubdeviceFormat format{};\n-\n-\tfor (unsigned int code : mbusCodes) {\n-\t\tif (std::any_of(mbusCodes_.begin(), mbusCodes_.end(),\n-\t\t\t\t[code](unsigned int c) { return c == code; })) {\n-\t\t\tformat.mbus_code = code;\n-\t\t\tbreak;\n-\t\t}\n-\t}\n-\n-\tif (!format.mbus_code) {\n-\t\tLOG(CameraSensor, Debug) << \"No supported format found\";\n-\t\treturn format;\n-\t}\n-\n \tunsigned int desiredArea = size.width * size.height;\n \tunsigned int bestArea = UINT_MAX;\n \tfloat desiredRatio = static_cast<float>(size.width) / size.height;\n \tfloat bestRatio = FLT_MAX;\n \tconst Size *bestSize = nullptr;\n+\tuint32_t bestCode = 0;\n \n-\tfor (const Size &sz : sizes_) {\n-\t\tif (sz.width < size.width || sz.height < size.height)\n-\t\t\tcontinue;\n+\tfor (unsigned int code : mbusCodes) {\n+\t\tconst std::vector<SizeRange> &ranges = formats_.sizes(code);\n \n-\t\tfloat ratio = static_cast<float>(sz.width) / sz.height;\n-\t\tfloat ratioDiff = fabsf(ratio - desiredRatio);\n-\t\tunsigned int area = sz.width * sz.height;\n-\t\tunsigned int areaDiff = area - desiredArea;\n+\t\tfor (const SizeRange &range : ranges) {\n+\t\t\tconst Size &sz = range.max;\n \n-\t\tif (ratioDiff > bestRatio)\n-\t\t\tcontinue;\n+\t\t\tif (sz.width < size.width || sz.height < size.height)\n+\t\t\t\tcontinue;\n \n-\t\tif (ratioDiff < bestRatio || areaDiff < bestArea) {\n-\t\t\tbestRatio = ratioDiff;\n-\t\t\tbestArea = areaDiff;\n-\t\t\tbestSize = &sz;\n+\t\t\tfloat ratio = static_cast<float>(sz.width) / sz.height;\n+\t\t\tfloat ratioDiff = fabsf(ratio - desiredRatio);\n+\t\t\tunsigned int area = sz.width * sz.height;\n+\t\t\tunsigned int areaDiff = area - desiredArea;\n+\n+\t\t\tif (ratioDiff > bestRatio)\n+\t\t\t\tcontinue;\n+\n+\t\t\tif (ratioDiff < bestRatio || areaDiff < bestArea) {\n+\t\t\t\tbestRatio = ratioDiff;\n+\t\t\t\tbestArea = areaDiff;\n+\t\t\t\tbestSize = &sz;\n+\t\t\t\tbestCode = code;\n+\t\t\t}\n \t\t}\n \t}\n \n \tif (!bestSize) {\n-\t\tLOG(CameraSensor, Debug) << \"No supported size found\";\n-\t\treturn format;\n+\t\tLOG(CameraSensor, Debug) << \"No supported format or size found\";\n+\t\treturn {};\n \t}\n \n-\tformat.size = *bestSize;\n+\tV4L2SubdeviceFormat format{\n+\t\t.mbus_code = bestCode,\n+\t\t.size = *bestSize,\n+\t};\n \n \treturn format;\n }\n@@ -424,7 +419,7 @@ const ControlInfoMap &CameraSensor::controls() const\n \n /**\n * \\brief Read controls from the sensor\n- * \\param[in] ids The list of control to read, specified by their ID\n+ * \\param[in] ids The list of controls to read, specified by their ID\n *\n * This method reads the value of all controls contained in \\a ids, and returns\n * their values as a ControlList.\ndiff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h\nindex 24993b74148f..30cf5f34f485 100644\n--- a/src/libcamera/include/camera_sensor.h\n+++ b/src/libcamera/include/camera_sensor.h\n@@ -14,6 +14,7 @@\n #include <libcamera/controls.h>\n #include <libcamera/geometry.h>\n \n+#include \"formats.h\"\n #include \"log.h\"\n \n namespace libcamera {\n@@ -51,7 +52,7 @@ public:\n \tconst MediaEntity *entity() const { return entity_; }\n \tconst std::vector<unsigned int> &mbusCodes() const { return mbusCodes_; }\n \tconst std::vector<Size> &sizes() const { return sizes_; }\n-\tconst Size &resolution() const;\n+\tconst Size &resolution() const { return resolution_; }\n \n \tV4L2SubdeviceFormat getFormat(const std::vector<unsigned int> &mbusCodes,\n \t\t\t\t const Size &size) const;\n@@ -74,6 +75,8 @@ private:\n \n \tstd::string model_;\n \n+\tImageFormats formats_;\n+\tSize resolution_;\n \tstd::vector<unsigned int> mbusCodes_;\n \tstd::vector<Size> sizes_;\n \n", "prefixes": [ "libcamera-devel" ] }