[{"id":31325,"web_url":"https://patchwork.libcamera.org/comment/31325/","msgid":"<172712777264.189305.5216317800314111527@ping.linuxembedded.co.uk>","date":"2024-09-23T21:42:52","subject":"Re: [PATCH v2 2/2] pipeline: rkisp1: Filter out sensor sizes not\n\tsupported by the pipeline","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Umang Jain (2024-09-13 11:37:59)\n> It is possible that the maximum sensor size (returned by\n> CameraSensor::resolution()) is not supported by the pipeline. In such\n> cases, a filter function is required to filter out resolutions of the\n> camera sensor, which cannot be supported by the pipeline.\n> \n> Introduce the filter function filterSensorResolution() in RkISP1Path\n> class and use it for validate() and generateConfiguration(). The\n> maximum sensor resolution is picked from that filtered set of\n> resolutions instead of CameraSensor::resolution().\n> \n> Signed-off-by: Umang Jain \n> Reviewed-by: Kieran Bingham \n\nAha, and at last I've found a way to validate/test this distinctly.\n\nRunning a PPP with an IMX258 which fails to configure the pipe on the\nlargest frame size, now excludes that frame size with this patch series:\n\nWithout this patch:\n\nmobian@mobian:~/libcamera$ cam -c2 -I -srole=raw\n\nUsing camera /base/i2c@ff110000/camera@1a as cam0\n0: 4208x3120-SRGGB10\n * Pixelformat: SRGGB10 (1048x780)-(4208x3120)/(+0,+0)\n - 1048x780\n - 2104x1560\n - 4208x3120\n\n\nWith this patch:\n\nmobian@mobian:~/libcamera$ ./build/src/apps/cam/cam -c2 -I -srole=raw\nUsing camera /base/i2c@ff110000/camera@1a as cam0\n0: 2104x1560-SRGGB10\n * Pixelformat: SRGGB10 (1048x780)-(2104x1560)/(+0,+0)\n - 1048x780\n - 2104x1560\n\n\nTested-by: Kieran Bingham \n\n\nNow ... technically - I think\nhttps://patchwork.kernel.org/project/linux-arm-kernel/patch/20240220235720.3010608-1-megi@xff.cz/\ntells me that the RK3399 might actually support the highest mode (-\nRK3399 has input/output limit of main path 4416 x 3312) but I don't\nthink the support for different platform limits has made it into 6.11\nyet.\n\nhttps://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/media/platform/rockchip/rkisp1/rkisp1-common.h?h=v6.11\nstill shows\n\n#define RKISP1_ISP_MAX_WIDTH\t\t\t4032\n#define RKISP1_ISP_MAX_HEIGHT\t\t\t3024\n\nWhich means, the PPP could be 'fixed' to get full output from the\nIMX258, but for now - it's really useful for testing this patch series\n;-)\n\n\n\n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 51 ++++++++++++++++++-\n> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 8 +++\n> 2 files changed, 57 insertions(+), 2 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> index 08b39e1e..1d818b32 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> @@ -137,12 +137,59 @@ void RkISP1Path::populateFormats()\n> }\n> }\n> \n> +/**\n> + * \\brief Filter the sensor resolutions that can be supported\n> + * \\param[in] sensor The camera sensor\n> + *\n> + * This function retrieves all the sizes supported by the sensor and\n> + * filters all the resolutions that can be supported on the pipeline.\n> + * It is possible that the sensor's maximum output resolution is higher\n> + * than the ISP maximum input. In that case, this function filters out all\n> + * the resolution incapable of being supported and returns the maximum\n> + * sensor resolution that can be supported by the pipeline.\n> + *\n> + * \\return Maximum sensor size supported on the pipeline\n> + */\n> +Size RkISP1Path::filterSensorResolution(const CameraSensor *sensor)\n> +{\n> + auto iter = sensorSizesMap_.find(sensor);\n> + if (iter != sensorSizesMap_.end() && !iter->second.empty())\n> + return iter->second.back();\n> +\n> + sensorSizesMap_.emplace(sensor, std::vector());\n> +\n> + std::vector sensorSizes;\n> + const std::vector &mbusCodes = sensor->mbusCodes();\n> + for (const auto it : mbusCodes) {\n> + std::vector sizes = sensor->sizes(it);\n> + for (Size sz : sizes)\n> + sensorSizes.push_back(sz);\n> + }\n> +\n> + std::sort(sensorSizes.begin(), sensorSizes.end());\n> +\n> + /* Remove duplicates. */\n> + auto last = std::unique(sensorSizes.begin(), sensorSizes.end());\n> + sensorSizes.erase(last, sensorSizes.end());\n> +\n> + /* Discard any sizes that the pipeline is unable to support. */\n> + for (auto sz : sensorSizes) {\n> + if (sz.width > maxResolution_.width ||\n> + sz.height > maxResolution_.height)\n> + continue;\n> +\n> + sensorSizesMap_[sensor].push_back(sz);\n> + }\n> +\n> + return sensorSizesMap_[sensor].back();\n> +}\n> +\n> StreamConfiguration\n> RkISP1Path::generateConfiguration(const CameraSensor *sensor, const Size &size,\n> StreamRole role)\n> {\n> const std::vector &mbusCodes = sensor->mbusCodes();\n> - const Size &resolution = sensor->resolution();\n> + Size resolution = filterSensorResolution(sensor);\n> \n> /* Min and max resolutions to populate the available stream formats. */\n> Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n> @@ -231,7 +278,7 @@ CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n> StreamConfiguration *cfg)\n> {\n> const std::vector &mbusCodes = sensor->mbusCodes();\n> - const Size &resolution = sensor->resolution();\n> + Size resolution = filterSensorResolution(sensor);\n> \n> const StreamConfiguration reqCfg = *cfg;\n> CameraConfiguration::Status status = CameraConfiguration::Valid;\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> index 13ba4b62..457c9ac9 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> @@ -7,6 +7,7 @@\n> \n> #pragma once\n> \n> +#include \n> #include \n> #include \n> #include \n> @@ -63,6 +64,7 @@ public:\n> \n> private:\n> void populateFormats();\n> + Size filterSensorResolution(const CameraSensor *sensor);\n> \n> static constexpr unsigned int RKISP1_BUFFER_COUNT = 4;\n> \n> @@ -77,6 +79,12 @@ private:\n> std::unique_ptr resizer_;\n> std::unique_ptr video_;\n> MediaLink *link_;\n> +\n> + /*\n> + * Map from camera sensors to the sizes (in increasing order),\n> + * which are guaranteed to be supported by the pipeline.\n> + */\n> + std::map> sensorSizesMap_;\n> };\n> \n> class RkISP1MainPath : public RkISP1Path\n> -- \n> 2.45.0\n>","headers":{"Return-Path":"","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 9A6F1C0F1B\n\tfor ;\n\tMon, 23 Sep 2024 21:42:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3D1736350B;\n\tMon, 23 Sep 2024 23:42:58 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4BF966037E\n\tfor ;\n\tMon, 23 Sep 2024 23:42:56 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E956E9CA;\n\tMon, 23 Sep 2024 23:41:29 +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=\"HP6MJvs0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727127690;\n\tbh=+TzVnbgEnGH5lVLcUvKAuVHq9jJrZS94NR1WgkkoC0M=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=HP6MJvs0bSPpv2RgcHqnEy6075gw5IM4FqudKyjMvmp+lB5h1fgd3ih/Hio9z3akc\n\tzbJr/W+hdMEmdaQF+hTtdzMsS5QqNo9O+wyQdGJOjc8FuDykSWqALd0q0Ts5p1HODp\n\ti0ltwScITEtrvzTL93jTLWtYzo4IAWvh855dXYzY=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240913103759.2166-3-umang.jain@ideasonboard.com>","References":"<20240913103759.2166-1-umang.jain@ideasonboard.com>\n\t<20240913103759.2166-3-umang.jain@ideasonboard.com>","Subject":"Re: [PATCH v2 2/2] pipeline: rkisp1: Filter out sensor sizes not\n\tsupported by the pipeline","From":"Kieran Bingham ","Cc":"Umang Jain ","To":"Umang Jain ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 23 Sep 2024 22:42:52 +0100","Message-ID":"<172712777264.189305.5216317800314111527@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]