[{"id":30546,"web_url":"https://patchwork.libcamera.org/comment/30546/","msgid":"<20240802190356.GB21917@pendragon.ideasonboard.com>","date":"2024-08-02T19:03:56","subject":"Re: [PATCH] pipeline: rkisp1: Fix validation when sensor max is\n\tlarger than ISP input","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang and Paul,\n\nThank you for the patch.\n\nOn Wed, Jul 24, 2024 at 03:58:12PM +0530, Umang Jain wrote:\n> From: Paul Elder \n> \n> If the maximum sensor output size is larger than the maximum ISP input\n> size, the maximum sensor size could be selected and the pipeline would\n> fail with an EPIPE. Fix this by validating a suitable sensor output size\n> which is less than or equal to, the ISP input.\n\ns/to,/to/\n\n(or \"less than, or equal to,\")\n\n> \n> Signed-off-by: Paul Elder \n> Signed-off-by: Umang Jain \n> ---\n> Split out from https://patchwork.libcamera.org/project/libcamera/list/?series=4143\n> \n> Changes 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(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 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\nconst if you don't need to modify this.\n\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\nI've left a review comment on this in v1, please address it or reply to\nit if you think it's wrong.\n\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);\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> index 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 &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\nAre you sure ? streamFormats_ contains the formats supported on the ISP\noutput. Translating that to a media bus code and then considering it\nmatches the sensor format doesn't make much sense to me.\n\n> +\t\tfor (auto &sz : sizes) {\n> +\t\t\tif (sz <= ispMaxInputSize_ && sz > sensorResolution)\n\nI don't think this is right, see how the comparison operator is defined\nfor the Size class.\n\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 &mbusCodes = sensor->mbusCodes();\n> -\tconst Size &resolution = sensor->resolution();\n> +\tSize resolution = maxSupportedSensorResolution(sensor);\n\nThis is a sensor invariant, isn't it ? Why do you recompute it every\ntime a configuration is generate ?\n\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 &mbusCodes = sensor->mbusCodes();\n> -\tconst Size &resolution = sensor->resolution();\n> +\tSize resolution = maxSupportedSensorResolution(sensor);\n\nAnd validated too.\n\nAnd furthermore, the maximum supported sensor resolution should be the\nsame for both paths, shouldn't it ?\n\nOverall this patch feels a bit too much of a hack for me to be\ncomfortable with it.\n\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);\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> index 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 &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 resizer_;\n> \tstd::unique_ptr video_;\n> \tMediaLink *link_;\n> +\n> +\tSize ispMaxInputSize_;\n> };\n> \n> class RkISP1MainPath : public RkISP1Path","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 B950DBDC71\n\tfor ;\n\tFri, 2 Aug 2024 19:04:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B324863370;\n\tFri, 2 Aug 2024 21:04: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 526DE6336B\n\tfor ;\n\tFri, 2 Aug 2024 21:04:19 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 71BE5524;\n\tFri, 2 Aug 2024 21:03: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=\"cGzcpYL7\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722625409;\n\tbh=SqtMw1DCgs4X7NzLmMLKLer10o17+VYhsdLSk0qb34k=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=cGzcpYL7L+U00nLgBxsE0T+7bYux5rTG83ueEzMtI8wLTN3qflIu0uLcupIDEHdKk\n\t5rrMDx0pMnLwjSuHT4j5CdsXmE1knBdrWbDmutOhQO5yFswXxMORoFHQIx1mAU7LOH\n\tyVscUA5lP6IWuVXmLDGPKF9pcypQ+cg8WIQZUQgU=","Date":"Fri, 2 Aug 2024 22:03:56 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org,\n\tPaul Elder ","Subject":"Re: [PATCH] pipeline: rkisp1: Fix validation when sensor max is\n\tlarger than ISP input","Message-ID":"<20240802190356.GB21917@pendragon.ideasonboard.com>","References":"<20240724102812.27431-1-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240724102812.27431-1-umang.jain@ideasonboard.com>","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\" "}},{"id":30568,"web_url":"https://patchwork.libcamera.org/comment/30568/","msgid":"<9ef20237-b02b-41dc-8dd4-828dec3d1c16@ideasonboard.com>","date":"2024-08-05T06:00:56","subject":"Re: [PATCH] pipeline: rkisp1: Fix validation when sensor max is\n\tlarger than ISP input","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent\n\nOn 03/08/24 12:33 am, Laurent Pinchart wrote:\n> Hi Umang and Paul,\n>\n> Thank you for the patch.\n>\n> On Wed, Jul 24, 2024 at 03:58:12PM +0530, Umang Jain wrote:\n>> From: Paul Elder\n>>\n>> If the maximum sensor output size is larger than the maximum ISP input\n>> size, the maximum sensor size could be selected and the pipeline would\n>> fail with an EPIPE. Fix this by validating a suitable sensor output size\n>> which is less than or equal to, the ISP input.\n> s/to,/to/\n>\n> (or \"less than, or equal to,\")\n>\n>> Signed-off-by: Paul Elder\n>> Signed-off-by: Umang Jain\n>> ---\n>> Split out fromhttps://patchwork.libcamera.org/project/libcamera/list/?series=4143\n>>\n>> Changes 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(-)\n>>\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index 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> const if you don't need to modify this.\n>\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> I've left a review comment on this in v1, please address it or reply to\n> it if you think it's wrong.\n>\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);\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n>> index 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 &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> Are you sure ? streamFormats_ contains the formats supported on the ISP\n> output. Translating that to a media bus code and then considering it\n> matches the sensor format doesn't make much sense to me.\n\nThe goal here is to filter out sensor resolutions which cannot be \nsupported on the ISP. And return the resolution which is equal to or \njust less than the max ISP input.\n\nThe notion was brought up in v1 of the patch here:\nhttps://patchwork.libcamera.org/patch/19411/#28485\n\nwhich  I agreed (it was never addressed until this patch).\n\n```\n\nShould `resolution` be replaced by the maximum sensor resolution\nsmaller than the isp max input size (maybe computed in\npopulateFormats() ? )\n```\n\n\nFor example, the platform I'm working with has imx283 and imx335 working \nwith following sizes (in decreasing order of resolution):\n\nIMX283: { 5472x3648, 4096x3072, 2736x1824 ...}\nIMX335: { 2592x1944, ... }\n\nSo the function should filter out 5472x3648 for IMX283 which is higher \nthan maxIspInputSize_ and return 4096x3072\nFor imx335, it returns 2592x1944 because that's the highest resolution \nhere and the ISP shall support it.\n\n>> +\t\tfor (auto &sz : sizes) {\n>> +\t\t\tif (sz <= ispMaxInputSize_ && sz > sensorResolution)\n> I don't think this is right, see how the comparison operator is defined\n> for the Size class.\n\nAh, you are correct here. I will probably have to expand here by \ncomparing the heights and widths individually.\n>\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 &mbusCodes = sensor->mbusCodes();\n>> -\tconst Size &resolution = sensor->resolution();\n>> +\tSize resolution = maxSupportedSensorResolution(sensor);\n> This is a sensor invariant, isn't it ? Why do you recompute it every\n> time a configuration is generate ?\n\nAs I have explained briefly above, it varies with the sensor - similarly \nhow sensor->resolution() varies which has been replaced with \nmaxSupportedSensorResolution() here.\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 &mbusCodes = sensor->mbusCodes();\n>> -\tconst Size &resolution = sensor->resolution();\n>> +\tSize resolution = maxSupportedSensorResolution(sensor);\n> And validated too.\n>\n> And furthermore, the maximum supported sensor resolution should be the\n> same for both paths, shouldn't it ?\n\nDo you mean here main and self paths here? It's probably  be a guess at \nthis time, I will have to check if there is some restriction when main \nand self paths are used simultaneously - and whether it affects the ISP \ninput size.\n> Overall this patch feels a bit too much of a hack for me to be\n> comfortable with it.\n\nOk - I think I started out with v1 (had two R-b tags already) and only \nmade improvement (maxSupportedSensorResolution()) which was also a \nreview comment on the series.\n\nIf it feels like a hack - I'll drop this and probably need to approach \nthe problem from a different angle here?\n\n>\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);\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n>> index 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 &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 resizer_;\n>> \tstd::unique_ptr video_;\n>> \tMediaLink *link_;\n>> +\n>> +\tSize ispMaxInputSize_;\n>> };\n>> \n>> class RkISP1MainPath : public RkISP1Path","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 E5E93BE173\n\tfor ;\n\tMon, 5 Aug 2024 06:01:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DA4276337F;\n\tMon, 5 Aug 2024 08:01:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EA1E36337C\n\tfor ;\n\tMon, 5 Aug 2024 08:01:00 +0200 (CEST)","from [IPV6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f] (unknown\n\t[IPv6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E2248BD1;\n\tMon, 5 Aug 2024 08:00:08 +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=\"S7dn55Lm\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722837609;\n\tbh=gTbyoz2sNGQFUQyf9jqVOuQQIlBjkWp6OGfAMXjPCDk=;\n\th=Date:From:Subject:To:Cc:References:In-Reply-To:From;\n\tb=S7dn55LmFadXqt/x62WBazE27NtJJW3lPhMDMfdXpTnzwBLYGS1FMdRijeTx44UGq\n\tzPlIfXzL5fX9jwiG2V28d3pFt5O5cebibCkkg/OFP0vvjSqmKjI6X64WXrrPKnkOGU\n\tZqYWbB8DkekjXeegmJDoewu+JZTiCkJVrPIA18+M=","Message-ID":"<9ef20237-b02b-41dc-8dd4-828dec3d1c16@ideasonboard.com>","Date":"Mon, 5 Aug 2024 11:30:56 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","From":"Umang Jain ","Subject":"Re: [PATCH] pipeline: rkisp1: Fix validation when sensor max is\n\tlarger than ISP input","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org,\n\tPaul Elder ","References":"<20240724102812.27431-1-umang.jain@ideasonboard.com>\n\t<20240802190356.GB21917@pendragon.ideasonboard.com>","Content-Language":"en-US","In-Reply-To":"<20240802190356.GB21917@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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\" "}}]