Patch Detail
Show a patch.
GET /api/patches/20699/?format=api
{ "id": 20699, "url": "https://patchwork.libcamera.org/api/patches/20699/?format=api", "web_url": "https://patchwork.libcamera.org/patch/20699/", "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": "<20240724102812.27431-1-umang.jain@ideasonboard.com>", "date": "2024-07-24T10:28:12", "name": "pipeline: rkisp1: Fix validation when sensor max is larger than ISP input", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "e5266997ceeac8114e099acf0334affeb24c47c5", "submitter": { "id": 86, "url": "https://patchwork.libcamera.org/api/people/86/?format=api", "name": "Umang Jain", "email": "umang.jain@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/20699/mbox/", "series": [ { "id": 4459, "url": "https://patchwork.libcamera.org/api/series/4459/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4459", "date": "2024-07-24T10:28:12", "name": "pipeline: rkisp1: Fix validation when sensor max is larger than ISP input", "version": 1, "mbox": "https://patchwork.libcamera.org/series/4459/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/20699/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/20699/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 3453FC323E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 24 Jul 2024 10:28:22 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 58B256336F;\n\tWed, 24 Jul 2024 12:28:21 +0200 (CEST)", "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 5D00D619A0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 24 Jul 2024 12:28:19 +0200 (CEST)", "from fedora.local (unknown\n\t[IPv6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C738D4CF;\n\tWed, 24 Jul 2024 12:27:35 +0200 (CEST)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"hpedoLdJ\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1721816856;\n\tbh=vQFVVRQ7stnG1jEvt1cyI+bwTWCeE+3X1fKz+BidexM=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=hpedoLdJ8HQBzXR5/qCoUMYrz+uYyTkh9f8H5yVwG3HjkQdeGnGahO5XS3tNC6pl4\n\tKkCySebXdkGZRYbTw33TDkQJGoH3bgYwwf4xhirmpxZvKonkAYDyuLjCrmNradH7LH\n\tIjaqdHr+uOXDDiGG5lGkEayAp+3fqDXe4dRWpmX4=", "From": "Umang Jain <umang.jain@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Paul Elder <paul.elder@ideasonboard.com>,\n\tUmang Jain <umang.jain@ideasonboard.com>", "Subject": "[PATCH] pipeline: rkisp1: Fix validation when sensor max is larger\n\tthan ISP input", "Date": "Wed, 24 Jul 2024 15:58:12 +0530", "Message-ID": "<20240724102812.27431-1-umang.jain@ideasonboard.com>", "X-Mailer": "git-send-email 2.45.2", "MIME-Version": "1.0", "Content-Transfer-Encoding": "8bit", "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>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "From: Paul Elder <paul.elder@ideasonboard.com>\n\nIf the maximum sensor output size is larger than the maximum ISP input\nsize, the maximum sensor size could be selected and the pipeline would\nfail with an EPIPE. Fix this by validating a suitable sensor output size\nwhich is less than or equal to, the ISP input.\n\nSigned-off-by: Paul Elder <paul.elder@ideasonboard.com>\nSigned-off-by: Umang Jain <umang.jain@ideasonboard.com>\n---\nSplit out from https://patchwork.libcamera.org/project/libcamera/list/?series=4143\n\nChanges in v2:\n- trivial var rename\n- Properly obtain a resolution from sensor supported for ISP max input\n- Refactor slightly to fit better\n---\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 ++++++-\n src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 51 +++++++++++++------\n src/libcamera/pipeline/rkisp1/rkisp1_path.h | 5 +-\n 3 files changed, 55 insertions(+), 18 deletions(-)", "diff": "diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 4cbf105d..5f94f422 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -1202,11 +1202,24 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n \tif (param_->open() < 0)\n \t\treturn false;\n \n+\t/*\n+\t * Retrieve the ISP maximum input size for config validation in the\n+\t * path classes.\n+\t */\n+\tSize ispMaxInputSize;\n+\tV4L2Subdevice::Formats ispFormats = isp_->formats(0);\n+\tfor (const auto &[mbus, sizes] : ispFormats) {\n+\t\tfor (const auto &size : sizes) {\n+\t\t\tif (ispMaxInputSize < size.max)\n+\t\t\t\tispMaxInputSize = size.max;\n+\t\t}\n+\t}\n+\n \t/* Locate and open the ISP main and self paths. */\n-\tif (!mainPath_.init(media_))\n+\tif (!mainPath_.init(media_, ispMaxInputSize))\n \t\treturn false;\n \n-\tif (hasSelfPath_ && !selfPath_.init(media_))\n+\tif (hasSelfPath_ && !selfPath_.init(media_, ispMaxInputSize))\n \t\treturn false;\n \n \tmainPath_.bufferReady().connect(this, &PipelineHandlerRkISP1::bufferReady);\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\nindex c49017d1..6b40cdd2 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n@@ -62,7 +62,7 @@ RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,\n {\n }\n \n-bool RkISP1Path::init(MediaDevice *media)\n+bool RkISP1Path::init(MediaDevice *media, const Size &ispMaxInputSize)\n {\n \tstd::string resizer = std::string(\"rkisp1_resizer_\") + name_ + \"path\";\n \tstd::string video = std::string(\"rkisp1_\") + name_ + \"path\";\n@@ -75,6 +75,7 @@ bool RkISP1Path::init(MediaDevice *media)\n \tif (video_->open() < 0)\n \t\treturn false;\n \n+\tispMaxInputSize_ = ispMaxInputSize;\n \tpopulateFormats();\n \n \tlink_ = media->link(\"rkisp1_isp\", 2, resizer, 0);\n@@ -126,12 +127,33 @@ void RkISP1Path::populateFormats()\n \t}\n }\n \n+Size RkISP1Path::maxSupportedSensorResolution(const CameraSensor *sensor)\n+{\n+\tSize sensorResolution;\n+\n+\t/* Get highest sensor resolution which is just less than or equal to ISP input */\n+\tfor (const auto &format : streamFormats_) {\n+\t\tauto sizes = sensor->sizes(formatToMediaBus.at(format));\n+\t\tfor (auto &sz : sizes) {\n+\t\t\tif (sz <= ispMaxInputSize_ && sz > sensorResolution)\n+\t\t\t\tsensorResolution = sz;\n+\t\t}\n+\t}\n+\n+\treturn sensorResolution;\n+}\n+\n StreamConfiguration\n RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size,\n \t\t\t\t StreamRole role)\n {\n \tconst std::vector<unsigned int> &mbusCodes = sensor->mbusCodes();\n-\tconst Size &resolution = sensor->resolution();\n+\tSize resolution = maxSupportedSensorResolution(sensor);\n+\tif (resolution.isNull()) {\n+\t\tLOG(RkISP1, Error) << \"No suitable format/resolution found\"\n+\t\t\t\t << \"for ISP input\";\n+\t\treturn {};\n+\t}\n \n \t/* Min and max resolutions to populate the available stream formats. */\n \tSize maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n@@ -220,7 +242,12 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n \t\t\t\t\t\t StreamConfiguration *cfg)\n {\n \tconst std::vector<unsigned int> &mbusCodes = sensor->mbusCodes();\n-\tconst Size &resolution = sensor->resolution();\n+\tSize resolution = maxSupportedSensorResolution(sensor);\n+\tif (resolution.isNull()) {\n+\t\tLOG(RkISP1, Error) << \"No suitable format/resolution found\"\n+\t\t\t\t << \"for ISP input\";\n+\t\treturn {};\n+\t}\n \n \tconst StreamConfiguration reqCfg = *cfg;\n \tCameraConfiguration::Status status = CameraConfiguration::Valid;\n@@ -275,8 +302,8 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n \tif (!found)\n \t\tcfg->pixelFormat = isRaw ? rawFormat : formats::NV12;\n \n-\tSize minResolution;\n-\tSize maxResolution;\n+\tSize maxResolution = maxResolution_.boundedTo(resolution);\n+\tSize minResolution = minResolution_.expandedToAspectRatio(resolution);\n \n \tif (isRaw) {\n \t\t/*\n@@ -287,16 +314,10 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n \t\tV4L2SubdeviceFormat sensorFormat =\n \t\t\tsensor->getFormat({ mbusCode }, cfg->size);\n \n-\t\tminResolution = sensorFormat.size;\n-\t\tmaxResolution = sensorFormat.size;\n-\t} else {\n-\t\t/*\n-\t\t * Adjust the size based on the sensor resolution and absolute\n-\t\t * limits of the ISP.\n-\t\t */\n-\t\tminResolution = minResolution_.expandedToAspectRatio(resolution);\n-\t\tmaxResolution = maxResolution_.boundedToAspectRatio(resolution)\n-\t\t\t\t\t .boundedTo(resolution);\n+\t\tif (!sensorFormat.size.isNull()) {\n+\t\t\tminResolution = sensorFormat.size.boundedTo(ispMaxInputSize_);\n+\t\t\tmaxResolution = sensorFormat.size.boundedTo(ispMaxInputSize_);\n+\t\t}\n \t}\n \n \tcfg->size.boundTo(maxResolution);\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\nindex 08edefec..a9bcfe36 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n@@ -35,7 +35,7 @@ public:\n \tRkISP1Path(const char *name, const Span<const PixelFormat> &formats,\n \t\t const Size &minResolution, const Size &maxResolution);\n \n-\tbool init(MediaDevice *media);\n+\tbool init(MediaDevice *media, const Size &ispMaxInputSize);\n \n \tint setEnabled(bool enable) { return link_->setEnabled(enable); }\n \tbool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }\n@@ -63,6 +63,7 @@ public:\n \n private:\n \tvoid populateFormats();\n+\tSize maxSupportedSensorResolution(const CameraSensor *sensor);\n \n \tstatic constexpr unsigned int RKISP1_BUFFER_COUNT = 4;\n \n@@ -77,6 +78,8 @@ private:\n \tstd::unique_ptr<V4L2Subdevice> resizer_;\n \tstd::unique_ptr<V4L2VideoDevice> video_;\n \tMediaLink *link_;\n+\n+\tSize ispMaxInputSize_;\n };\n \n class RkISP1MainPath : public RkISP1Path\n", "prefixes": [] }