Patch Detail
Show a patch.
GET /api/patches/9811/?format=api
{ "id": 9811, "url": "https://patchwork.libcamera.org/api/patches/9811/?format=api", "web_url": "https://patchwork.libcamera.org/patch/9811/", "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": "<20200925014207.1455796-14-niklas.soderlund@ragnatech.se>", "date": "2020-09-25T01:41:58", "name": "[libcamera-devel,v3,13/22] libcamera: pipeline: rkisp1: Add format validation for self path", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "d6a391bb9ae7f917d4ca07925e2ff30e17be58a0", "submitter": { "id": 5, "url": "https://patchwork.libcamera.org/api/people/5/?format=api", "name": "Niklas Söderlund", "email": "niklas.soderlund@ragnatech.se" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/9811/mbox/", "series": [ { "id": 1325, "url": "https://patchwork.libcamera.org/api/series/1325/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1325", "date": "2020-09-25T01:41:45", "name": "libcamera: pipeline: rkisp1: Extend to support two streams", "version": 3, "mbox": "https://patchwork.libcamera.org/series/1325/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/9811/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/9811/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id A8F31C3B5C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 25 Sep 2020 01:42:58 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 215C060576;\n\tFri, 25 Sep 2020 03:42:58 +0200 (CEST)", "from vsp-unauthed02.binero.net (vsp-unauthed02.binero.net\n\t[195.74.38.227])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E89663025\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 25 Sep 2020 03:42:51 +0200 (CEST)", "from bismarck.berto.se (p54ac52a8.dip0.t-ipconnect.de\n\t[84.172.82.168]) by bin-vsp-out-01.atm.binero.net (Halon) with ESMTPA\n\tid 6b8a842d-fed0-11ea-92dc-005056917a89;\n\tFri, 25 Sep 2020 03:42:50 +0200 (CEST)" ], "X-Halon-ID": "6b8a842d-fed0-11ea-92dc-005056917a89", "Authorized-sender": "niklas.soderlund@fsdn.se", "From": "=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Fri, 25 Sep 2020 03:41:58 +0200", "Message-Id": "<20200925014207.1455796-14-niklas.soderlund@ragnatech.se>", "X-Mailer": "git-send-email 2.28.0", "In-Reply-To": "<20200925014207.1455796-1-niklas.soderlund@ragnatech.se>", "References": "<20200925014207.1455796-1-niklas.soderlund@ragnatech.se>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH v3 13/22] libcamera: pipeline: rkisp1: Add\n\tformat validation for self path", "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>", "Content-Type": "text/plain; charset=\"utf-8\"", "Content-Transfer-Encoding": "base64", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Extend the format validation to work with both main and self paths. The\nheuristics honors that the first stream in the configuration has the\nhighest priority while still examining both streams for a best match.\n\nIt is not possible to capture from the self path as the self stream is\nnot yet exposed to applications.\n\nSigned-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n---\n* Changes since v2\n- Fix spelling in commit message.\n- Use Span<> instead of turning arrays to vectors.\n- Keep data_ const and cast 'const Streams*' to non-const using\n const_cast<Stream *>() to match the IPU3 pipeline.\n- Rename fitAnyPath() to fitsAllPaths().\n- Expand documentation for why second stream is evaluated first if the\n fist stream can use either stream.\n- Drop support for RGB888 and RGB656 for selfpath which was present in\n v2 as the driver delivers odd data when the frames are observed.\n\n* Changes since v1\n- Store formats in std::vector instead of std::array to avoid template\n usage for validate function.\n---\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 210 +++++++++++++++++------\n 1 file changed, 162 insertions(+), 48 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex cd3049485746edd6..bd53183a034efaff 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -9,6 +9,7 @@\n #include <array>\n #include <iomanip>\n #include <memory>\n+#include <numeric>\n #include <queue>\n \n #include <linux/media-bus-format.h>\n@@ -19,6 +20,7 @@\n #include <libcamera/formats.h>\n #include <libcamera/ipa/rkisp1.h>\n #include <libcamera/request.h>\n+#include <libcamera/span.h>\n #include <libcamera/stream.h>\n \n #include \"libcamera/internal/camera_sensor.h\"\n@@ -50,6 +52,19 @@ constexpr std::array<PixelFormat, 7> RKISP1_RSZ_MP_FORMATS{\n \tformats::NV12,\n \t/* \\todo Add support for 8-bit greyscale to DRM formats */\n };\n+\n+constexpr Size RKISP1_RSZ_SP_SRC_MIN{ 32, 16 };\n+constexpr Size RKISP1_RSZ_SP_SRC_MAX{ 1920, 1920 };\n+constexpr std::array<PixelFormat, 7> RKISP1_RSZ_SP_FORMATS{\n+\tformats::YUYV,\n+\tformats::YVYU,\n+\tformats::VYUY,\n+\tformats::NV16,\n+\tformats::NV61,\n+\tformats::NV21,\n+\tformats::NV12,\n+\t/* \\todo Add support for BGR888 and RGB565 */\n+};\n } /* namespace */\n \n class PipelineHandlerRkISP1;\n@@ -181,6 +196,14 @@ public:\n private:\n \tstatic constexpr unsigned int RKISP1_BUFFER_COUNT = 4;\n \n+\tCameraConfiguration::Status validatePath(StreamConfiguration *cfg,\n+\t\t\t\t\t\t const Span<const PixelFormat> &formats,\n+\t\t\t\t\t\t const Size &min, const Size &max,\n+\t\t\t\t\t\t V4L2VideoDevice *video);\n+\tCameraConfiguration::Status validateMainPath(StreamConfiguration *cfg);\n+\tCameraConfiguration::Status validateSelfPath(StreamConfiguration *cfg);\n+\tbool fitsAllPaths(const StreamConfiguration &cfg);\n+\n \t/*\n \t * The RkISP1CameraData instance is guaranteed to be valid as long as the\n \t * corresponding Camera instance is valid. In order to borrow a\n@@ -492,6 +515,69 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n \tdata_ = data;\n }\n \n+CameraConfiguration::Status RkISP1CameraConfiguration::validatePath(\n+\tStreamConfiguration *cfg, const Span<const PixelFormat> &formats,\n+\tconst Size &min, const Size &max, V4L2VideoDevice *video)\n+{\n+\tconst StreamConfiguration reqCfg = *cfg;\n+\tStatus status = Valid;\n+\n+\tif (std::find(formats.begin(), formats.end(), cfg->pixelFormat) ==\n+\t formats.end())\n+\t\tcfg->pixelFormat = formats::NV12;\n+\n+\tcfg->size.boundTo(max);\n+\tcfg->size.expandTo(min);\n+\tcfg->bufferCount = RKISP1_BUFFER_COUNT;\n+\n+\tV4L2DeviceFormat format = {};\n+\tformat.fourcc = video->toV4L2PixelFormat(cfg->pixelFormat);\n+\tformat.size = cfg->size;\n+\n+\tint ret = video->tryFormat(&format);\n+\tif (ret)\n+\t\treturn Invalid;\n+\n+\tcfg->stride = format.planes[0].bpl;\n+\tcfg->frameSize = format.planes[0].size;\n+\n+\tif (cfg->pixelFormat != reqCfg.pixelFormat || cfg->size != reqCfg.size) {\n+\t\tLOG(RkISP1, Debug)\n+\t\t\t<< \"Adjusting format from \" << reqCfg.toString()\n+\t\t\t<< \" to \" << cfg->toString();\n+\t\tstatus = Adjusted;\n+\t}\n+\n+\treturn status;\n+}\n+\n+CameraConfiguration::Status RkISP1CameraConfiguration::validateMainPath(StreamConfiguration *cfg)\n+{\n+\treturn validatePath(cfg, RKISP1_RSZ_MP_FORMATS, RKISP1_RSZ_MP_SRC_MIN,\n+\t\t\t RKISP1_RSZ_MP_SRC_MAX, data_->mainPathVideo_);\n+}\n+\n+CameraConfiguration::Status RkISP1CameraConfiguration::validateSelfPath(StreamConfiguration *cfg)\n+{\n+\treturn validatePath(cfg, RKISP1_RSZ_SP_FORMATS, RKISP1_RSZ_SP_SRC_MIN,\n+\t\t\t RKISP1_RSZ_SP_SRC_MAX, data_->selfPathVideo_);\n+}\n+\n+bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)\n+{\n+\tStreamConfiguration config;\n+\n+\tconfig = cfg;\n+\tif (validateMainPath(&config) != Valid)\n+\t\treturn false;\n+\n+\tconfig = cfg;\n+\tif (validateSelfPath(&config) != Valid)\n+\t\treturn false;\n+\n+\treturn true;\n+}\n+\n CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n {\n \tconst CameraSensor *sensor = data_->sensor_;\n@@ -501,22 +587,87 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n \t\treturn Invalid;\n \n \t/* Cap the number of entries to the available streams. */\n-\tif (config_.size() > 1) {\n-\t\tconfig_.resize(1);\n+\tif (config_.size() > 2) {\n+\t\tconfig_.resize(2);\n \t\tstatus = Adjusted;\n \t}\n \n-\tStreamConfiguration &cfg = config_[0];\n-\n-\t/* Adjust the pixel format. */\n-\tif (std::find(RKISP1_RSZ_MP_FORMATS.begin(), RKISP1_RSZ_MP_FORMATS.end(),\n-\t\t cfg.pixelFormat) == RKISP1_RSZ_MP_FORMATS.end()) {\n-\t\tLOG(RkISP1, Debug) << \"Adjusting format to NV12\";\n-\t\tcfg.pixelFormat = formats::NV12,\n-\t\tstatus = Adjusted;\n+\t/*\n+\t * If there are more then one stream in the configuration figure out the\n+\t * order to evaluate the streams. The first stream has the highest\n+\t * priority but if both main path and self path can satisfy it evaluate\n+\t * second stream first as the first stream is guaranteed to work with\n+\t * whichever path is not used by the second one.\n+\t */\n+\tstd::vector<unsigned int> order(config_.size());\n+\tstd::iota(order.begin(), order.end(), 0);\n+\tif (config_.size() == 2 && fitsAllPaths(config_[0]))\n+\t\tstd::reverse(order.begin(), order.end());\n+\n+\tbool mainPathAvailable = true;\n+\tbool selfPathAvailable = true;\n+\tfor (unsigned int index : order) {\n+\t\tStreamConfiguration &cfg = config_[index];\n+\n+\t\t/* Try to match stream without adjusting configuration. */\n+\t\tif (mainPathAvailable) {\n+\t\t\tStreamConfiguration tryCfg = cfg;\n+\t\t\tif (validateMainPath(&tryCfg) == Valid) {\n+\t\t\t\tmainPathAvailable = false;\n+\t\t\t\tcfg = tryCfg;\n+\t\t\t\tcfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));\n+\t\t\t\tLOG(RkISP1, Debug) << \"Exact match main\";\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (selfPathAvailable) {\n+\t\t\tStreamConfiguration tryCfg = cfg;\n+\t\t\tif (validateSelfPath(&tryCfg) == Valid) {\n+\t\t\t\tselfPathAvailable = false;\n+\t\t\t\tcfg = tryCfg;\n+\t\t\t\tcfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));\n+\t\t\t\tLOG(RkISP1, Debug) << \"Exact match self\";\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\n+\t\t/* Try to match stream allowing adjusting configuration. */\n+\t\tif (mainPathAvailable) {\n+\t\t\tStreamConfiguration tryCfg = cfg;\n+\t\t\tif (validateMainPath(&tryCfg) == Adjusted) {\n+\t\t\t\tmainPathAvailable = false;\n+\t\t\t\tcfg = tryCfg;\n+\t\t\t\tcfg.setStream(const_cast<Stream *>(&data_->mainPathStream_));\n+\t\t\t\tstatus = Adjusted;\n+\t\t\t\tLOG(RkISP1, Debug) << \"Adjust match main\";\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\n+\t\tif (selfPathAvailable) {\n+\t\t\tStreamConfiguration tryCfg = cfg;\n+\t\t\tif (validateSelfPath(&tryCfg) == Adjusted) {\n+\t\t\t\tselfPathAvailable = false;\n+\t\t\t\tcfg = tryCfg;\n+\t\t\t\tcfg.setStream(const_cast<Stream *>(&data_->selfPathStream_));\n+\t\t\t\tstatus = Adjusted;\n+\t\t\t\tLOG(RkISP1, Debug) << \"Adjust match self\";\n+\t\t\t\tcontinue;\n+\t\t\t}\n+\t\t}\n+\n+\t\t/* All paths rejected configuraiton. */\n+\t\tLOG(RkISP1, Debug) << \"Camera configuration not supported \"\n+\t\t\t\t << cfg.toString();\n+\t\treturn Invalid;\n \t}\n \n \t/* Select the sensor format. */\n+\tSize maxSize;\n+\tfor (const StreamConfiguration &cfg : config_)\n+\t\tmaxSize = std::max(maxSize, cfg.size);\n+\n \tsensorFormat_ = sensor->getFormat({ MEDIA_BUS_FMT_SBGGR12_1X12,\n \t\t\t\t\t MEDIA_BUS_FMT_SGBRG12_1X12,\n \t\t\t\t\t MEDIA_BUS_FMT_SGRBG12_1X12,\n@@ -529,47 +680,10 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n \t\t\t\t\t MEDIA_BUS_FMT_SGBRG8_1X8,\n \t\t\t\t\t MEDIA_BUS_FMT_SGRBG8_1X8,\n \t\t\t\t\t MEDIA_BUS_FMT_SRGGB8_1X8 },\n-\t\t\t\t\t cfg.size);\n+\t\t\t\t\t maxSize);\n \tif (sensorFormat_.size.isNull())\n \t\tsensorFormat_.size = sensor->resolution();\n \n-\t/*\n-\t * Provide a suitable default that matches the sensor aspect\n-\t * ratio and clamp the size to the hardware bounds.\n-\t *\n-\t * \\todo: Check the hardware alignment constraints.\n-\t */\n-\tconst Size size = cfg.size;\n-\n-\tif (cfg.size.isNull()) {\n-\t\tcfg.size.width = 1280;\n-\t\tcfg.size.height = 1280 * sensorFormat_.size.height\n-\t\t\t\t/ sensorFormat_.size.width;\n-\t}\n-\n-\tcfg.size.boundTo(RKISP1_RSZ_MP_SRC_MAX);\n-\tcfg.size.expandTo(RKISP1_RSZ_MP_SRC_MIN);\n-\n-\tif (cfg.size != size) {\n-\t\tLOG(RkISP1, Debug)\n-\t\t\t<< \"Adjusting size from \" << size.toString()\n-\t\t\t<< \" to \" << cfg.size.toString();\n-\t\tstatus = Adjusted;\n-\t}\n-\n-\tcfg.bufferCount = RKISP1_BUFFER_COUNT;\n-\n-\tV4L2DeviceFormat format = {};\n-\tformat.fourcc = data_->mainPathVideo_->toV4L2PixelFormat(cfg.pixelFormat);\n-\tformat.size = cfg.size;\n-\n-\tint ret = data_->mainPathVideo_->tryFormat(&format);\n-\tif (ret)\n-\t\treturn Invalid;\n-\n-\tcfg.stride = format.planes[0].bpl;\n-\tcfg.frameSize = format.planes[0].size;\n-\n \treturn status;\n }\n \n", "prefixes": [ "libcamera-devel", "v3", "13/22" ] }