[{"id":25535,"web_url":"https://patchwork.libcamera.org/comment/25535/","msgid":"<20221024090652.GE3874866@pyrite.rasen.tech>","date":"2022-10-24T09:06:52","subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"On Mon, Oct 24, 2022 at 03:03:54AM +0300, Laurent Pinchart wrote:\n> Unlike RkISP1Path::generateConfiguration(), the validate() function\n> doesn't take the camera sensor resolution into account but only\n> considers the absolute minimum and maximum sizes support by the ISP to\n\ns/support/supported/\n\n> validate the stream size. Fix it by using the same logic as when\n> generating the configuration.\n> \n> Instead of passing the sensor resolution to the validate() function,\n> pass the CameraSensor pointer to prepare for subsequent changes that\n> will require access to more camera sensor data. While at it, update the\n> generateConfiguration() function similarly for the same reason.\n> \n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Paul Elder \n\n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++++-------\n> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 22 +++++++++++++++----\n> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 6 +++--\n> 3 files changed, 31 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index cca89cc13bff..dcab5286aa56 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -404,14 +404,15 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n> \n> bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)\n> {\n> +\tconst CameraSensor *sensor = data_->sensor_.get();\n> \tStreamConfiguration config;\n> \n> \tconfig = cfg;\n> -\tif (data_->mainPath_->validate(&config) != Valid)\n> +\tif (data_->mainPath_->validate(sensor, &config) != Valid)\n> \t\treturn false;\n> \n> \tconfig = cfg;\n> -\tif (data_->selfPath_ && data_->selfPath_->validate(&config) != Valid)\n> +\tif (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid)\n> \t\treturn false;\n> \n> \treturn true;\n> @@ -459,7 +460,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \t\t/* Try to match stream without adjusting configuration. */\n> \t\tif (mainPathAvailable) {\n> \t\t\tStreamConfiguration tryCfg = cfg;\n> -\t\t\tif (data_->mainPath_->validate(&tryCfg) == Valid) {\n> +\t\t\tif (data_->mainPath_->validate(sensor, &tryCfg) == Valid) {\n> \t\t\t\tmainPathAvailable = false;\n> \t\t\t\tcfg = tryCfg;\n> \t\t\t\tcfg.setStream(const_cast(&data_->mainPathStream_));\n> @@ -469,7 +470,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \n> \t\tif (selfPathAvailable) {\n> \t\t\tStreamConfiguration tryCfg = cfg;\n> -\t\t\tif (data_->selfPath_->validate(&tryCfg) == Valid) {\n> +\t\t\tif (data_->selfPath_->validate(sensor, &tryCfg) == Valid) {\n> \t\t\t\tselfPathAvailable = false;\n> \t\t\t\tcfg = tryCfg;\n> \t\t\t\tcfg.setStream(const_cast(&data_->selfPathStream_));\n> @@ -480,7 +481,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \t\t/* Try to match stream allowing adjusting configuration. */\n> \t\tif (mainPathAvailable) {\n> \t\t\tStreamConfiguration tryCfg = cfg;\n> -\t\t\tif (data_->mainPath_->validate(&tryCfg) == Adjusted) {\n> +\t\t\tif (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) {\n> \t\t\t\tmainPathAvailable = false;\n> \t\t\t\tcfg = tryCfg;\n> \t\t\t\tcfg.setStream(const_cast(&data_->mainPathStream_));\n> @@ -491,7 +492,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \n> \t\tif (selfPathAvailable) {\n> \t\t\tStreamConfiguration tryCfg = cfg;\n> -\t\t\tif (data_->selfPath_->validate(&tryCfg) == Adjusted) {\n> +\t\t\tif (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) {\n> \t\t\t\tselfPathAvailable = false;\n> \t\t\t\tcfg = tryCfg;\n> \t\t\t\tcfg.setStream(const_cast(&data_->selfPathStream_));\n> @@ -603,11 +604,11 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> \t\tStreamConfiguration cfg;\n> \t\tif (useMainPath) {\n> \t\t\tcfg = data->mainPath_->generateConfiguration(\n> -\t\t\t\tdata->sensor_->resolution());\n> +\t\t\t\tdata->sensor_.get());\n> \t\t\tmainPathAvailable = false;\n> \t\t} else {\n> \t\t\tcfg = data->selfPath_->generateConfiguration(\n> -\t\t\t\tdata->sensor_->resolution());\n> +\t\t\t\tdata->sensor_.get());\n> \t\t\tselfPathAvailable = false;\n> \t\t}\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> index 8077a54494a5..cc2ac66e6939 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> \n> +#include \"libcamera/internal/camera_sensor.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/v4l2_subdevice.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> @@ -85,8 +86,10 @@ void RkISP1Path::populateFormats()\n> \t}\n> }\n> \n> -StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n> +StreamConfiguration RkISP1Path::generateConfiguration(const CameraSensor *sensor)\n> {\n> +\tconst Size &resolution = sensor->resolution();\n> +\n> \tSize maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n> \t\t\t\t\t .boundedTo(resolution);\n> \tSize minResolution = minResolution_.expandedToAspectRatio(resolution);\n> @@ -104,8 +107,11 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n> \treturn cfg;\n> }\n> \n> -CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> +CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n> +\t\t\t\t\t\t StreamConfiguration *cfg)\n> {\n> +\tconst Size &resolution = sensor->resolution();\n> +\n> \tconst StreamConfiguration reqCfg = *cfg;\n> \tCameraConfiguration::Status status = CameraConfiguration::Valid;\n> \n> @@ -117,8 +123,16 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> \tif (!streamFormats_.count(cfg->pixelFormat))\n> \t\tcfg->pixelFormat = formats::NV12;\n> \n> -\tcfg->size.boundTo(maxResolution_);\n> -\tcfg->size.expandTo(minResolution_);\n> +\t/*\n> +\t * Adjust the size based on the sensor resolution and absolute limits\n> +\t * of the ISP.\n> +\t */\n> +\tSize maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n> +\t\t\t\t\t .boundedTo(resolution);\n> +\tSize minResolution = minResolution_.expandedToAspectRatio(resolution);\n> +\n> +\tcfg->size.boundTo(maxResolution);\n> +\tcfg->size.expandTo(minResolution);\n> \tcfg->bufferCount = RKISP1_BUFFER_COUNT;\n> \n> \tV4L2DeviceFormat format;\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> index d88effbb6f56..bf4ad18fbbf2 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> @@ -23,6 +23,7 @@\n> \n> namespace libcamera {\n> \n> +class CameraSensor;\n> class MediaDevice;\n> class V4L2Subdevice;\n> struct StreamConfiguration;\n> @@ -39,8 +40,9 @@ public:\n> \tint setEnabled(bool enable) { return link_->setEnabled(enable); }\n> \tbool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }\n> \n> -\tStreamConfiguration generateConfiguration(const Size &resolution);\n> -\tCameraConfiguration::Status validate(StreamConfiguration *cfg);\n> +\tStreamConfiguration generateConfiguration(const CameraSensor *sensor);\n> +\tCameraConfiguration::Status validate(const CameraSensor *sensor,\n> +\t\t\t\t\t StreamConfiguration *cfg);\n> \n> \tint configure(const StreamConfiguration &config,\n> \t\t const V4L2SubdeviceFormat &inputFormat);\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 45B41BD16B\n\tfor ;\n\tMon, 24 Oct 2022 09:07:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 59A4862F00;\n\tMon, 24 Oct 2022 11:07: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 679B761F4D\n\tfor ;\n\tMon, 24 Oct 2022 11:07:00 +0200 (CEST)","from pyrite.rasen.tech (h175-177-042-159.catv02.itscom.jp\n\t[175.177.42.159])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7278A471;\n\tMon, 24 Oct 2022 11:06:58 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666602422;\n\tbh=tq7x9HZD7qRlMzxfVz7jGO8Co7cddxtboYKrvqJ9W+U=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=xLO5/pgjXdklTeh2YvOc6Cwf/YNYQRLfE/vKUSHP/PuqPNWLyJt76VpthvEFFg4iM\n\tEMk5FTnRKvlYHhdcyqVdVXloXRGzzF9+9GU0yTygRgk7BsoGfcPzzqXaRKFtkqLnJC\n\tHP8pK2ixa3KiwBpi4Ha0ej/s7XYqvpLozuCnoxivUWcgbb4b/N5bEP21okFqspKVKL\n\t0B+4rmoq4xWa8kEVeWgyxm84LPjtQNG0eOsp35wKfYhZKGHN0VyfA8U0HWspdnLVci\n\t1H02wrYtRzpK5qpH+VV9Ko5/DUQyv/eTsJLb4GkQTv/1ykJZuO7m356UEJsmmYAvLg\n\t1kdJp6RdDIjAw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666602419;\n\tbh=tq7x9HZD7qRlMzxfVz7jGO8Co7cddxtboYKrvqJ9W+U=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=OV4T3hU7ZlJ3Yo9ljt6i77dHmSrs7TfRmx89Yxau1KLuYjMtkkP9j6QwOFgC+fqO8\n\tRT4tlX06gH10RE9EFf5d9BYDFBWmzKsW7DEQP0iZdYjeB/eZloylRnfq+M3A11tsDn\n\t1rxWd1Z8qpobsVWXyINz32qqG71dUa9tf2mS49vo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"OV4T3hU7\"; dkim-atps=neutral","Date":"Mon, 24 Oct 2022 18:06:52 +0900","To":"Laurent Pinchart ","Message-ID":"<20221024090652.GE3874866@pyrite.rasen.tech>","References":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>\n\t<20221024000356.29521-12-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20221024000356.29521-12-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","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","From":"Paul Elder via libcamera-devel ","Reply-To":"paul.elder@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25608,"web_url":"https://patchwork.libcamera.org/comment/25608/","msgid":"<20221026154031.o5mt6l6uaqs4xw5d@uno.localdomain>","date":"2022-10-26T15:40:31","subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Laurent\n\nOn Mon, Oct 24, 2022 at 03:03:54AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Unlike RkISP1Path::generateConfiguration(), the validate() function\n> doesn't take the camera sensor resolution into account but only\n> considers the absolute minimum and maximum sizes support by the ISP to\n> validate the stream size. Fix it by using the same logic as when\n> generating the configuration.\n>\n> Instead of passing the sensor resolution to the validate() function,\n> pass the CameraSensor pointer to prepare for subsequent changes that\n> will require access to more camera sensor data. While at it, update the\n> generateConfiguration() function similarly for the same reason.\n>\n> Signed-off-by: Laurent Pinchart \n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++++-------\n> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 22 +++++++++++++++----\n> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 6 +++--\n> 3 files changed, 31 insertions(+), 14 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index cca89cc13bff..dcab5286aa56 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -404,14 +404,15 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n>\n> bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)\n> {\n> +\tconst CameraSensor *sensor = data_->sensor_.get();\n> \tStreamConfiguration config;\n>\n> \tconfig = cfg;\n> -\tif (data_->mainPath_->validate(&config) != Valid)\n> +\tif (data_->mainPath_->validate(sensor, &config) != Valid)\n> \t\treturn false;\n>\n> \tconfig = cfg;\n> -\tif (data_->selfPath_ && data_->selfPath_->validate(&config) != Valid)\n> +\tif (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid)\n> \t\treturn false;\n>\n> \treturn true;\n> @@ -459,7 +460,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \t\t/* Try to match stream without adjusting configuration. */\n> \t\tif (mainPathAvailable) {\n> \t\t\tStreamConfiguration tryCfg = cfg;\n> -\t\t\tif (data_->mainPath_->validate(&tryCfg) == Valid) {\n> +\t\t\tif (data_->mainPath_->validate(sensor, &tryCfg) == Valid) {\n> \t\t\t\tmainPathAvailable = false;\n> \t\t\t\tcfg = tryCfg;\n> \t\t\t\tcfg.setStream(const_cast(&data_->mainPathStream_));\n> @@ -469,7 +470,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n>\n> \t\tif (selfPathAvailable) {\n> \t\t\tStreamConfiguration tryCfg = cfg;\n> -\t\t\tif (data_->selfPath_->validate(&tryCfg) == Valid) {\n> +\t\t\tif (data_->selfPath_->validate(sensor, &tryCfg) == Valid) {\n> \t\t\t\tselfPathAvailable = false;\n> \t\t\t\tcfg = tryCfg;\n> \t\t\t\tcfg.setStream(const_cast(&data_->selfPathStream_));\n> @@ -480,7 +481,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \t\t/* Try to match stream allowing adjusting configuration. */\n> \t\tif (mainPathAvailable) {\n> \t\t\tStreamConfiguration tryCfg = cfg;\n> -\t\t\tif (data_->mainPath_->validate(&tryCfg) == Adjusted) {\n> +\t\t\tif (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) {\n> \t\t\t\tmainPathAvailable = false;\n> \t\t\t\tcfg = tryCfg;\n> \t\t\t\tcfg.setStream(const_cast(&data_->mainPathStream_));\n> @@ -491,7 +492,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n>\n> \t\tif (selfPathAvailable) {\n> \t\t\tStreamConfiguration tryCfg = cfg;\n> -\t\t\tif (data_->selfPath_->validate(&tryCfg) == Adjusted) {\n> +\t\t\tif (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) {\n> \t\t\t\tselfPathAvailable = false;\n> \t\t\t\tcfg = tryCfg;\n> \t\t\t\tcfg.setStream(const_cast(&data_->selfPathStream_));\n> @@ -603,11 +604,11 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> \t\tStreamConfiguration cfg;\n> \t\tif (useMainPath) {\n> \t\t\tcfg = data->mainPath_->generateConfiguration(\n> -\t\t\t\tdata->sensor_->resolution());\n> +\t\t\t\tdata->sensor_.get());\n> \t\t\tmainPathAvailable = false;\n> \t\t} else {\n> \t\t\tcfg = data->selfPath_->generateConfiguration(\n> -\t\t\t\tdata->sensor_->resolution());\n> +\t\t\t\tdata->sensor_.get());\n> \t\t\tselfPathAvailable = false;\n> \t\t}\n>\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> index 8077a54494a5..cc2ac66e6939 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n>\n> +#include \"libcamera/internal/camera_sensor.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/v4l2_subdevice.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> @@ -85,8 +86,10 @@ void RkISP1Path::populateFormats()\n> \t}\n> }\n>\n> -StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n> +StreamConfiguration RkISP1Path::generateConfiguration(const CameraSensor *sensor)\n> {\n> +\tconst Size &resolution = sensor->resolution();\n> +\n> \tSize maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n> \t\t\t\t\t .boundedTo(resolution);\n> \tSize minResolution = minResolution_.expandedToAspectRatio(resolution);\n> @@ -104,8 +107,11 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n> \treturn cfg;\n> }\n>\n> -CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> +CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n> +\t\t\t\t\t\t StreamConfiguration *cfg)\n> {\n> +\tconst Size &resolution = sensor->resolution();\n> +\n> \tconst StreamConfiguration reqCfg = *cfg;\n> \tCameraConfiguration::Status status = CameraConfiguration::Valid;\n>\n> @@ -117,8 +123,16 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> \tif (!streamFormats_.count(cfg->pixelFormat))\n> \t\tcfg->pixelFormat = formats::NV12;\n>\n> -\tcfg->size.boundTo(maxResolution_);\n> -\tcfg->size.expandTo(minResolution_);\n> +\t/*\n> +\t * Adjust the size based on the sensor resolution and absolute limits\n> +\t * of the ISP.\n> +\t */\n> +\tSize maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n> +\t\t\t\t\t .boundedTo(resolution);\n> +\tSize minResolution = minResolution_.expandedToAspectRatio(resolution);\n> +\n> +\tcfg->size.boundTo(maxResolution);\n> +\tcfg->size.expandTo(minResolution);\n> \tcfg->bufferCount = RKISP1_BUFFER_COUNT;\n>\n> \tV4L2DeviceFormat format;\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> index d88effbb6f56..bf4ad18fbbf2 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> @@ -23,6 +23,7 @@\n>\n> namespace libcamera {\n>\n> +class CameraSensor;\n> class MediaDevice;\n> class V4L2Subdevice;\n> struct StreamConfiguration;\n> @@ -39,8 +40,9 @@ public:\n> \tint setEnabled(bool enable) { return link_->setEnabled(enable); }\n> \tbool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }\n>\n> -\tStreamConfiguration generateConfiguration(const Size &resolution);\n> -\tCameraConfiguration::Status validate(StreamConfiguration *cfg);\n> +\tStreamConfiguration generateConfiguration(const CameraSensor *sensor);\n> +\tCameraConfiguration::Status validate(const CameraSensor *sensor,\n> +\t\t\t\t\t StreamConfiguration *cfg);\n>\n> \tint configure(const StreamConfiguration &config,\n> \t\t const V4L2SubdeviceFormat &inputFormat);\n> --\n> Regards,\n>\n> Laurent Pinchart\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 F14B7BD16B\n\tfor ;\n\tWed, 26 Oct 2022 15:40:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 59BBF62F5C;\n\tWed, 26 Oct 2022 17:40:35 +0200 (CEST)","from relay10.mail.gandi.net (relay10.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::230])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 87AEF61F4B\n\tfor ;\n\tWed, 26 Oct 2022 17:40:33 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id E6498240004;\n\tWed, 26 Oct 2022 15:40:32 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666798835;\n\tbh=OqQKnA93Di6YybfbxLzcYSdUv6goszW1l8JRvi/90aA=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=XepYbSwqptgfjL3gBshyVsta6OQEEn4agDk7wOlcKRRXLUcPUblnI90/K+phCNTJf\n\tTaRe463LU8hfqIxVqb0cUmzzFC6xcgcIxrZNzwQ/0WG8EsOrNVdb5PkHBgMHZD/PM9\n\trjHxHX8YMfXp6prnJwRmnhBFH5AjQ51wGfc6J4LHbYJfPeQeXehfrST3iMTPkD32xV\n\tUPlL2EQI47zIC607Ha+yZEQcaDegHoDcrAGKgI+XYNsh6waUMJMgd3fCaOSzaVB5ol\n\thx4TPayv+592YNPkrkHrP0uU3OYNkgTRXHhyGYhtomi5WqFDjmiaxygjn/odocGjhk\n\tLpklmZud1RMog==","Date":"Wed, 26 Oct 2022 17:40:31 +0200","To":"Laurent Pinchart ","Message-ID":"<20221026154031.o5mt6l6uaqs4xw5d@uno.localdomain>","References":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>\n\t<20221024000356.29521-12-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221024000356.29521-12-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","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","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25677,"web_url":"https://patchwork.libcamera.org/comment/25677/","msgid":"<166699506602.404886.879894391167680910@Monstersaurus>","date":"2022-10-28T22:11:06","subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart via libcamera-devel (2022-10-24 01:03:54)\n> Unlike RkISP1Path::generateConfiguration(), the validate() function\n> doesn't take the camera sensor resolution into account but only\n> considers the absolute minimum and maximum sizes support by the ISP to\n> validate the stream size. Fix it by using the same logic as when\n> generating the configuration.\n> \n> Instead of passing the sensor resolution to the validate() function,\n> pass the CameraSensor pointer to prepare for subsequent changes that\n> will require access to more camera sensor data. While at it, update the\n> generateConfiguration() function similarly for the same reason.\n \n\nI have quite the surprising curveball on this patch.\n\nWhile the validation seems to aim to restrict sizes to the capabilities\nof the ISP and the Sensor size, it seems to miss something.\n\nI've hooked up an Arducam 16MP to test this - and the set up fails\nbecause the sensor size selected is larger than the maximum input size\nof the ISP.\n\nCan this somehow take the input limtiations of the ISP into account when\nselecting and filtering sensor sizes easily ?\n\n--\nKieran\n\n\n> Signed-off-by: Laurent Pinchart \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++++-------\n> src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 22 +++++++++++++++----\n> src/libcamera/pipeline/rkisp1/rkisp1_path.h | 6 +++--\n> 3 files changed, 31 insertions(+), 14 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index cca89cc13bff..dcab5286aa56 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -404,14 +404,15 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n> \n> bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)\n> {\n> + const CameraSensor *sensor = data_->sensor_.get();\n> StreamConfiguration config;\n> \n> config = cfg;\n> - if (data_->mainPath_->validate(&config) != Valid)\n> + if (data_->mainPath_->validate(sensor, &config) != Valid)\n> return false;\n> \n> config = cfg;\n> - if (data_->selfPath_ && data_->selfPath_->validate(&config) != Valid)\n> + if (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid)\n> return false;\n> \n> return true;\n> @@ -459,7 +460,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> /* Try to match stream without adjusting configuration. */\n> if (mainPathAvailable) {\n> StreamConfiguration tryCfg = cfg;\n> - if (data_->mainPath_->validate(&tryCfg) == Valid) {\n> + if (data_->mainPath_->validate(sensor, &tryCfg) == Valid) {\n> mainPathAvailable = false;\n> cfg = tryCfg;\n> cfg.setStream(const_cast(&data_->mainPathStream_));\n> @@ -469,7 +470,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \n> if (selfPathAvailable) {\n> StreamConfiguration tryCfg = cfg;\n> - if (data_->selfPath_->validate(&tryCfg) == Valid) {\n> + if (data_->selfPath_->validate(sensor, &tryCfg) == Valid) {\n> selfPathAvailable = false;\n> cfg = tryCfg;\n> cfg.setStream(const_cast(&data_->selfPathStream_));\n> @@ -480,7 +481,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> /* Try to match stream allowing adjusting configuration. */\n> if (mainPathAvailable) {\n> StreamConfiguration tryCfg = cfg;\n> - if (data_->mainPath_->validate(&tryCfg) == Adjusted) {\n> + if (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) {\n> mainPathAvailable = false;\n> cfg = tryCfg;\n> cfg.setStream(const_cast(&data_->mainPathStream_));\n> @@ -491,7 +492,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \n> if (selfPathAvailable) {\n> StreamConfiguration tryCfg = cfg;\n> - if (data_->selfPath_->validate(&tryCfg) == Adjusted) {\n> + if (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) {\n> selfPathAvailable = false;\n> cfg = tryCfg;\n> cfg.setStream(const_cast(&data_->selfPathStream_));\n> @@ -603,11 +604,11 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> StreamConfiguration cfg;\n> if (useMainPath) {\n> cfg = data->mainPath_->generateConfiguration(\n> - data->sensor_->resolution());\n> + data->sensor_.get());\n> mainPathAvailable = false;\n> } else {\n> cfg = data->selfPath_->generateConfiguration(\n> - data->sensor_->resolution());\n> + data->sensor_.get());\n> selfPathAvailable = false;\n> }\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> index 8077a54494a5..cc2ac66e6939 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> @@ -12,6 +12,7 @@\n> #include \n> #include \n> \n> +#include \"libcamera/internal/camera_sensor.h\"\n> #include \"libcamera/internal/media_device.h\"\n> #include \"libcamera/internal/v4l2_subdevice.h\"\n> #include \"libcamera/internal/v4l2_videodevice.h\"\n> @@ -85,8 +86,10 @@ void RkISP1Path::populateFormats()\n> }\n> }\n> \n> -StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n> +StreamConfiguration RkISP1Path::generateConfiguration(const CameraSensor *sensor)\n> {\n> + const Size &resolution = sensor->resolution();\n> +\n> Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n> .boundedTo(resolution);\n> Size minResolution = minResolution_.expandedToAspectRatio(resolution);\n> @@ -104,8 +107,11 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n> return cfg;\n> }\n> \n> -CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> +CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n> + StreamConfiguration *cfg)\n> {\n> + const Size &resolution = sensor->resolution();\n> +\n> const StreamConfiguration reqCfg = *cfg;\n> CameraConfiguration::Status status = CameraConfiguration::Valid;\n> \n> @@ -117,8 +123,16 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> if (!streamFormats_.count(cfg->pixelFormat))\n> cfg->pixelFormat = formats::NV12;\n> \n> - cfg->size.boundTo(maxResolution_);\n> - cfg->size.expandTo(minResolution_);\n> + /*\n> + * Adjust the size based on the sensor resolution and absolute limits\n> + * of the ISP.\n> + */\n> + Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n> + .boundedTo(resolution);\n\nI played around here and expected that limiting this maxResolution to\nthe maximum Input resolution should help - but I'm missing something and\nhaven't delved deep enough to figure out what yet.\n\n\n> + Size minResolution = minResolution_.expandedToAspectRatio(resolution);\n> +\n> + cfg->size.boundTo(maxResolution);\n> + cfg->size.expandTo(minResolution);\n> cfg->bufferCount = RKISP1_BUFFER_COUNT;\n> \n> V4L2DeviceFormat format;\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> index d88effbb6f56..bf4ad18fbbf2 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> @@ -23,6 +23,7 @@\n> \n> namespace libcamera {\n> \n> +class CameraSensor;\n> class MediaDevice;\n> class V4L2Subdevice;\n> struct StreamConfiguration;\n> @@ -39,8 +40,9 @@ public:\n> int setEnabled(bool enable) { return link_->setEnabled(enable); }\n> bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }\n> \n> - StreamConfiguration generateConfiguration(const Size &resolution);\n> - CameraConfiguration::Status validate(StreamConfiguration *cfg);\n> + StreamConfiguration generateConfiguration(const CameraSensor *sensor);\n> + CameraConfiguration::Status validate(const CameraSensor *sensor,\n> + StreamConfiguration *cfg);\n> \n> int configure(const StreamConfiguration &config,\n> const V4L2SubdeviceFormat &inputFormat);\n> -- \n> Regards,\n> \n> Laurent Pinchart\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 C85C0BDB16\n\tfor ;\n\tFri, 28 Oct 2022 22:11:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 29EB162FEE;\n\tSat, 29 Oct 2022 00:11:10 +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 C6F7862F89\n\tfor ;\n\tSat, 29 Oct 2022 00:11:08 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 46CC1440;\n\tSat, 29 Oct 2022 00:11:08 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666995070;\n\tbh=mCZUtukpRccVUaZ4BwJtzKK5NJycsLqGnYxgr9I8+cA=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=ZjyS8Zv/IqLAJFFrgaKRdJGtJZx0iU0WR3cUJf3OiA+OR0msObMWdmHyNPQwu+WTc\n\t2pVQATgeR8bPDFVRWrT3wzMIiJsvpgR9M7byHImRUCovWR8z/pG3TOrPFUD94S63up\n\t99j6lL50U7mUSSOefFwEHaZkmT7qxCwcqmkdVS95/A9hjO9YqKqJrrMYT3pOeMGCR0\n\ta3IFMWevRjhc2BN0Qmybxeg8c+cSk7kWiTYDk/QPzGIcGuabHP0IvawdcaMjgDIfW1\n\tHtLZvf6bMK21vh3mbYNpCQgC4zuUgCQWXop8i4agIdupfZMnlh4uM/yiksk4aotl7+\n\tVv93nEqIaFVpQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666995068;\n\tbh=mCZUtukpRccVUaZ4BwJtzKK5NJycsLqGnYxgr9I8+cA=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=nOxNk0+kPr+g+1ZSFNJK1LEl7nMDKMWOdtid/+mf//jaJulsMLKdMLv8xrjT+IXmu\n\tTotGZF4RgeAgcQhD+pXinYfNfEc8DxS9oeg/SgJYtVy3upcZXJeDM38kabo2Ix8ehK\n\teurhENmf1AU4EXB36PIfiBp8O5U819GGN9/nKsiI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"nOxNk0+k\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20221024000356.29521-12-laurent.pinchart@ideasonboard.com>","References":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>\n\t<20221024000356.29521-12-laurent.pinchart@ideasonboard.com>","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 28 Oct 2022 23:11:06 +0100","Message-ID":"<166699506602.404886.879894391167680910@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25686,"web_url":"https://patchwork.libcamera.org/comment/25686/","msgid":"","date":"2022-10-30T17:24:49","subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Fri, Oct 28, 2022 at 11:11:06PM +0100, Kieran Bingham wrote:\n> Quoting Laurent Pinchart via libcamera-devel (2022-10-24 01:03:54)\n> > Unlike RkISP1Path::generateConfiguration(), the validate() function\n> > doesn't take the camera sensor resolution into account but only\n> > considers the absolute minimum and maximum sizes support by the ISP to\n> > validate the stream size. Fix it by using the same logic as when\n> > generating the configuration.\n> > \n> > Instead of passing the sensor resolution to the validate() function,\n> > pass the CameraSensor pointer to prepare for subsequent changes that\n> > will require access to more camera sensor data. While at it, update the\n> > generateConfiguration() function similarly for the same reason.\n> \n> I have quite the surprising curveball on this patch.\n> \n> While the validation seems to aim to restrict sizes to the capabilities\n> of the ISP and the Sensor size, it seems to miss something.\n> \n> I've hooked up an Arducam 16MP to test this - and the set up fails\n> because the sensor size selected is larger than the maximum input size\n> of the ISP.\n> \n> Can this somehow take the input limtiations of the ISP into account when\n> selecting and filtering sensor sizes easily ?\n\nIs this a regression ?\n\n> > Signed-off-by: Laurent Pinchart \n> > ---\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++++-------\n> > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 22 +++++++++++++++----\n> > src/libcamera/pipeline/rkisp1/rkisp1_path.h | 6 +++--\n> > 3 files changed, 31 insertions(+), 14 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index cca89cc13bff..dcab5286aa56 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -404,14 +404,15 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n> > \n> > bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)\n> > {\n> > + const CameraSensor *sensor = data_->sensor_.get();\n> > StreamConfiguration config;\n> > \n> > config = cfg;\n> > - if (data_->mainPath_->validate(&config) != Valid)\n> > + if (data_->mainPath_->validate(sensor, &config) != Valid)\n> > return false;\n> > \n> > config = cfg;\n> > - if (data_->selfPath_ && data_->selfPath_->validate(&config) != Valid)\n> > + if (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid)\n> > return false;\n> > \n> > return true;\n> > @@ -459,7 +460,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > /* Try to match stream without adjusting configuration. */\n> > if (mainPathAvailable) {\n> > StreamConfiguration tryCfg = cfg;\n> > - if (data_->mainPath_->validate(&tryCfg) == Valid) {\n> > + if (data_->mainPath_->validate(sensor, &tryCfg) == Valid) {\n> > mainPathAvailable = false;\n> > cfg = tryCfg;\n> > cfg.setStream(const_cast(&data_->mainPathStream_));\n> > @@ -469,7 +470,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > \n> > if (selfPathAvailable) {\n> > StreamConfiguration tryCfg = cfg;\n> > - if (data_->selfPath_->validate(&tryCfg) == Valid) {\n> > + if (data_->selfPath_->validate(sensor, &tryCfg) == Valid) {\n> > selfPathAvailable = false;\n> > cfg = tryCfg;\n> > cfg.setStream(const_cast(&data_->selfPathStream_));\n> > @@ -480,7 +481,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > /* Try to match stream allowing adjusting configuration. */\n> > if (mainPathAvailable) {\n> > StreamConfiguration tryCfg = cfg;\n> > - if (data_->mainPath_->validate(&tryCfg) == Adjusted) {\n> > + if (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) {\n> > mainPathAvailable = false;\n> > cfg = tryCfg;\n> > cfg.setStream(const_cast(&data_->mainPathStream_));\n> > @@ -491,7 +492,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > \n> > if (selfPathAvailable) {\n> > StreamConfiguration tryCfg = cfg;\n> > - if (data_->selfPath_->validate(&tryCfg) == Adjusted) {\n> > + if (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) {\n> > selfPathAvailable = false;\n> > cfg = tryCfg;\n> > cfg.setStream(const_cast(&data_->selfPathStream_));\n> > @@ -603,11 +604,11 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> > StreamConfiguration cfg;\n> > if (useMainPath) {\n> > cfg = data->mainPath_->generateConfiguration(\n> > - data->sensor_->resolution());\n> > + data->sensor_.get());\n> > mainPathAvailable = false;\n> > } else {\n> > cfg = data->selfPath_->generateConfiguration(\n> > - data->sensor_->resolution());\n> > + data->sensor_.get());\n> > selfPathAvailable = false;\n> > }\n> > \n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > index 8077a54494a5..cc2ac66e6939 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > @@ -12,6 +12,7 @@\n> > #include \n> > #include \n> > \n> > +#include \"libcamera/internal/camera_sensor.h\"\n> > #include \"libcamera/internal/media_device.h\"\n> > #include \"libcamera/internal/v4l2_subdevice.h\"\n> > #include \"libcamera/internal/v4l2_videodevice.h\"\n> > @@ -85,8 +86,10 @@ void RkISP1Path::populateFormats()\n> > }\n> > }\n> > \n> > -StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n> > +StreamConfiguration RkISP1Path::generateConfiguration(const CameraSensor *sensor)\n> > {\n> > + const Size &resolution = sensor->resolution();\n> > +\n> > Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n> > .boundedTo(resolution);\n> > Size minResolution = minResolution_.expandedToAspectRatio(resolution);\n> > @@ -104,8 +107,11 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n> > return cfg;\n> > }\n> > \n> > -CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> > +CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n> > + StreamConfiguration *cfg)\n> > {\n> > + const Size &resolution = sensor->resolution();\n> > +\n> > const StreamConfiguration reqCfg = *cfg;\n> > CameraConfiguration::Status status = CameraConfiguration::Valid;\n> > \n> > @@ -117,8 +123,16 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> > if (!streamFormats_.count(cfg->pixelFormat))\n> > cfg->pixelFormat = formats::NV12;\n> > \n> > - cfg->size.boundTo(maxResolution_);\n> > - cfg->size.expandTo(minResolution_);\n> > + /*\n> > + * Adjust the size based on the sensor resolution and absolute limits\n> > + * of the ISP.\n> > + */\n> > + Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n> > + .boundedTo(resolution);\n> \n> I played around here and expected that limiting this maxResolution to\n> the maximum Input resolution should help - but I'm missing something and\n> haven't delved deep enough to figure out what yet.\n> \n> > + Size minResolution = minResolution_.expandedToAspectRatio(resolution);\n> > +\n> > + cfg->size.boundTo(maxResolution);\n> > + cfg->size.expandTo(minResolution);\n> > cfg->bufferCount = RKISP1_BUFFER_COUNT;\n> > \n> > V4L2DeviceFormat format;\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> > index d88effbb6f56..bf4ad18fbbf2 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> > @@ -23,6 +23,7 @@\n> > \n> > namespace libcamera {\n> > \n> > +class CameraSensor;\n> > class MediaDevice;\n> > class V4L2Subdevice;\n> > struct StreamConfiguration;\n> > @@ -39,8 +40,9 @@ public:\n> > int setEnabled(bool enable) { return link_->setEnabled(enable); }\n> > bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }\n> > \n> > - StreamConfiguration generateConfiguration(const Size &resolution);\n> > - CameraConfiguration::Status validate(StreamConfiguration *cfg);\n> > + StreamConfiguration generateConfiguration(const CameraSensor *sensor);\n> > + CameraConfiguration::Status validate(const CameraSensor *sensor,\n> > + StreamConfiguration *cfg);\n> > \n> > int configure(const StreamConfiguration &config,\n> > const V4L2SubdeviceFormat &inputFormat);","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 E7C98BDB16\n\tfor ;\n\tSun, 30 Oct 2022 17:25:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4129063024;\n\tSun, 30 Oct 2022 18:25:17 +0100 (CET)","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 E33CE61F47\n\tfor ;\n\tSun, 30 Oct 2022 18:25:15 +0100 (CET)","from pendragon.ideasonboard.com (85-76-13-148-nat.elisa-mobile.fi\n\t[85.76.13.148])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 979802F5;\n\tSun, 30 Oct 2022 18:25:14 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1667150717;\n\tbh=Uqvz5uY9C08MJS7Txg0oo9w+QT6U8sgtmg9+3JgeD1A=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=JBRMvfeFVooPKmM1Y9CmN4cnsJQwSATduSTno/AKEP1n3JrEtCpbF2AQWtndUEk67\n\tRwDQ6Ciy36Q9oWedVEkun7UXwhIJIUecQ+ipwYNhK+dJsr+SrCaJhu2wVsC1vykhne\n\t82qMnFdkOS5fVI02V//MQ/P3FHDIqFbvzxB66jTWNPiV+NO6qSNrWUjqMrv9Ie7hjl\n\trP0mr0ACwXjsFP/MkKgj3eQGtykTfRA0/D8pVTvjq7vkUiXlhm34GzKNGVVNh93zOp\n\te21MBcGv++QT6dSF5kbPkHcfSW1qluwd/2MLw7cv1sx+q1oA/paxFA8/o33z8hBXId\n\tF+QG+Sl/Krm3w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1667150715;\n\tbh=Uqvz5uY9C08MJS7Txg0oo9w+QT6U8sgtmg9+3JgeD1A=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=o07Ay47GFuHFEqsPYgu1vDSwNKKY5v6XzQXo4HT+J6CJSbHybybKQL9slV45UaGJn\n\tJb7WPMdQOtGtb+xRrbQL28sx/4dS32kd2kSfjrhkOAoNLuj5h2GPQXCUm7FEA+Hg+X\n\taRusJCVYnCeKDeMZbW0XHYd9CiPASEjHhXABcy5k="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"o07Ay47G\"; dkim-atps=neutral","Date":"Sun, 30 Oct 2022 19:24:49 +0200","To":"Kieran Bingham ","Message-ID":"","References":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>\n\t<20221024000356.29521-12-laurent.pinchart@ideasonboard.com>\n\t<166699506602.404886.879894391167680910@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<166699506602.404886.879894391167680910@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25862,"web_url":"https://patchwork.libcamera.org/comment/25862/","msgid":"<166919736649.1079859.10921833042765076716@Monstersaurus>","date":"2022-11-23T09:56:06","subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2022-10-30 17:24:49)\n> Hi Kieran,\n> \n> On Fri, Oct 28, 2022 at 11:11:06PM +0100, Kieran Bingham wrote:\n> > Quoting Laurent Pinchart via libcamera-devel (2022-10-24 01:03:54)\n> > > Unlike RkISP1Path::generateConfiguration(), the validate() function\n> > > doesn't take the camera sensor resolution into account but only\n> > > considers the absolute minimum and maximum sizes support by the ISP to\n> > > validate the stream size. Fix it by using the same logic as when\n> > > generating the configuration.\n> > > \n> > > Instead of passing the sensor resolution to the validate() function,\n> > > pass the CameraSensor pointer to prepare for subsequent changes that\n> > > will require access to more camera sensor data. While at it, update the\n> > > generateConfiguration() function similarly for the same reason.\n> > \n> > I have quite the surprising curveball on this patch.\n> > \n> > While the validation seems to aim to restrict sizes to the capabilities\n> > of the ISP and the Sensor size, it seems to miss something.\n> > \n> > I've hooked up an Arducam 16MP to test this - and the set up fails\n> > because the sensor size selected is larger than the maximum input size\n> > of the ISP.\n> > \n> > Can this somehow take the input limtiations of the ISP into account when\n> > selecting and filtering sensor sizes easily ?\n> \n> Is this a regression ?\n\nI wasn't able to test this camera before on this platform. So it's not a\nregression for me, it's just never worked...\n\nBut ... if we're \"fixing the validation of the stream size\", I'd call\nthis a bug report that this patch doesn't seem to account for the ISP\nlimitations when doing so.\n\nI've worked around this by removing the larger image sizes from the\nsensor driver currently... (Then I hit an issue where the Pixelrate of\nthe sensor is not supported by the ISP ... so something else is wrong\nthere).\n\n\n> > > Signed-off-by: Laurent Pinchart \n> > > ---\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++++-------\n> > > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 22 +++++++++++++++----\n> > > src/libcamera/pipeline/rkisp1/rkisp1_path.h | 6 +++--\n> > > 3 files changed, 31 insertions(+), 14 deletions(-)\n> > > \n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index cca89cc13bff..dcab5286aa56 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -404,14 +404,15 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n> > > \n> > > bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)\n> > > {\n> > > + const CameraSensor *sensor = data_->sensor_.get();\n> > > StreamConfiguration config;\n> > > \n> > > config = cfg;\n> > > - if (data_->mainPath_->validate(&config) != Valid)\n> > > + if (data_->mainPath_->validate(sensor, &config) != Valid)\n> > > return false;\n> > > \n> > > config = cfg;\n> > > - if (data_->selfPath_ && data_->selfPath_->validate(&config) != Valid)\n> > > + if (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid)\n> > > return false;\n> > > \n> > > return true;\n> > > @@ -459,7 +460,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > > /* Try to match stream without adjusting configuration. */\n> > > if (mainPathAvailable) {\n> > > StreamConfiguration tryCfg = cfg;\n> > > - if (data_->mainPath_->validate(&tryCfg) == Valid) {\n> > > + if (data_->mainPath_->validate(sensor, &tryCfg) == Valid) {\n> > > mainPathAvailable = false;\n> > > cfg = tryCfg;\n> > > cfg.setStream(const_cast(&data_->mainPathStream_));\n> > > @@ -469,7 +470,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > > \n> > > if (selfPathAvailable) {\n> > > StreamConfiguration tryCfg = cfg;\n> > > - if (data_->selfPath_->validate(&tryCfg) == Valid) {\n> > > + if (data_->selfPath_->validate(sensor, &tryCfg) == Valid) {\n> > > selfPathAvailable = false;\n> > > cfg = tryCfg;\n> > > cfg.setStream(const_cast(&data_->selfPathStream_));\n> > > @@ -480,7 +481,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > > /* Try to match stream allowing adjusting configuration. */\n> > > if (mainPathAvailable) {\n> > > StreamConfiguration tryCfg = cfg;\n> > > - if (data_->mainPath_->validate(&tryCfg) == Adjusted) {\n> > > + if (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) {\n> > > mainPathAvailable = false;\n> > > cfg = tryCfg;\n> > > cfg.setStream(const_cast(&data_->mainPathStream_));\n> > > @@ -491,7 +492,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > > \n> > > if (selfPathAvailable) {\n> > > StreamConfiguration tryCfg = cfg;\n> > > - if (data_->selfPath_->validate(&tryCfg) == Adjusted) {\n> > > + if (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) {\n> > > selfPathAvailable = false;\n> > > cfg = tryCfg;\n> > > cfg.setStream(const_cast(&data_->selfPathStream_));\n> > > @@ -603,11 +604,11 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> > > StreamConfiguration cfg;\n> > > if (useMainPath) {\n> > > cfg = data->mainPath_->generateConfiguration(\n> > > - data->sensor_->resolution());\n> > > + data->sensor_.get());\n> > > mainPathAvailable = false;\n> > > } else {\n> > > cfg = data->selfPath_->generateConfiguration(\n> > > - data->sensor_->resolution());\n> > > + data->sensor_.get());\n> > > selfPathAvailable = false;\n> > > }\n> > > \n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > > index 8077a54494a5..cc2ac66e6939 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > > @@ -12,6 +12,7 @@\n> > > #include \n> > > #include \n> > > \n> > > +#include \"libcamera/internal/camera_sensor.h\"\n> > > #include \"libcamera/internal/media_device.h\"\n> > > #include \"libcamera/internal/v4l2_subdevice.h\"\n> > > #include \"libcamera/internal/v4l2_videodevice.h\"\n> > > @@ -85,8 +86,10 @@ void RkISP1Path::populateFormats()\n> > > }\n> > > }\n> > > \n> > > -StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n> > > +StreamConfiguration RkISP1Path::generateConfiguration(const CameraSensor *sensor)\n> > > {\n> > > + const Size &resolution = sensor->resolution();\n> > > +\n> > > Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n> > > .boundedTo(resolution);\n> > > Size minResolution = minResolution_.expandedToAspectRatio(resolution);\n> > > @@ -104,8 +107,11 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n> > > return cfg;\n> > > }\n> > > \n> > > -CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> > > +CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n> > > + StreamConfiguration *cfg)\n> > > {\n> > > + const Size &resolution = sensor->resolution();\n> > > +\n> > > const StreamConfiguration reqCfg = *cfg;\n> > > CameraConfiguration::Status status = CameraConfiguration::Valid;\n> > > \n> > > @@ -117,8 +123,16 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> > > if (!streamFormats_.count(cfg->pixelFormat))\n> > > cfg->pixelFormat = formats::NV12;\n> > > \n> > > - cfg->size.boundTo(maxResolution_);\n> > > - cfg->size.expandTo(minResolution_);\n> > > + /*\n> > > + * Adjust the size based on the sensor resolution and absolute limits\n> > > + * of the ISP.\n> > > + */\n> > > + Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n> > > + .boundedTo(resolution);\n> > \n> > I played around here and expected that limiting this maxResolution to\n> > the maximum Input resolution should help - but I'm missing something and\n> > haven't delved deep enough to figure out what yet.\n> > \n> > > + Size minResolution = minResolution_.expandedToAspectRatio(resolution);\n> > > +\n> > > + cfg->size.boundTo(maxResolution);\n> > > + cfg->size.expandTo(minResolution);\n> > > cfg->bufferCount = RKISP1_BUFFER_COUNT;\n> > > \n> > > V4L2DeviceFormat format;\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> > > index d88effbb6f56..bf4ad18fbbf2 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> > > @@ -23,6 +23,7 @@\n> > > \n> > > namespace libcamera {\n> > > \n> > > +class CameraSensor;\n> > > class MediaDevice;\n> > > class V4L2Subdevice;\n> > > struct StreamConfiguration;\n> > > @@ -39,8 +40,9 @@ public:\n> > > int setEnabled(bool enable) { return link_->setEnabled(enable); }\n> > > bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }\n> > > \n> > > - StreamConfiguration generateConfiguration(const Size &resolution);\n> > > - CameraConfiguration::Status validate(StreamConfiguration *cfg);\n> > > + StreamConfiguration generateConfiguration(const CameraSensor *sensor);\n> > > + CameraConfiguration::Status validate(const CameraSensor *sensor,\n> > > + StreamConfiguration *cfg);\n> > > \n> > > int configure(const StreamConfiguration &config,\n> > > const V4L2SubdeviceFormat &inputFormat);\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 B71B7BDE6B\n\tfor ;\n\tWed, 23 Nov 2022 09:56:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3A78563319;\n\tWed, 23 Nov 2022 10:56:10 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E54FF6330D\n\tfor ;\n\tWed, 23 Nov 2022 10:56:08 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 7E9BD88F;\n\tWed, 23 Nov 2022 10:56:08 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669197370;\n\tbh=bRSRqiS3N5t7XL89V25VijtNXvUsWM65zmimIZt0CZQ=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=JwVqLYYSiv5f8T6QX9oZxmzvYJU1sMq1e4eigDtwm8go/BizSYsnONijASMUm4v4l\n\t8E5rqNbGi7rFNDFF/i7N/7bFSRFUAo4FxDoWy67+GbgE1GODOVKhOh/hm6SPuh+kTL\n\tnxXROcIiNm1rSM4VfNdOrwDUT58TV98tJhMTajGYxt5Jwc+H7voVS1ofKdMFOyr0q5\n\t1o6TF/5SO8XKE5pQd8K1XlsJN7i/+l6eBAzDI3m3yUYeDxWdf6p2vwps+wJxSx5wuv\n\t54HkLabdq7j+X6xoTn0xwxnDyB7ygmKh6nRuv1Wr2o/7qdrI8bZGwrHrc5rC1/BsYv\n\tpfBCDVavjv8MQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669197368;\n\tbh=bRSRqiS3N5t7XL89V25VijtNXvUsWM65zmimIZt0CZQ=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=gNHuNZGahm5AJvs11E10FQWFry6AK0/JKc8D9tEv1LpseK2Jewsw2OincDhdzVP6g\n\tJ4p8Ju/qyuNq0/TpEf0SXp8uQF3ps1qskKEOz+x4DE8OgdtCpBNkUk6Y4IpBxZ+rpY\n\t1iujpFlU04iPfN8b5J+MeLO4qvV5F11wbTlwxsZk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"gNHuNZGa\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>\n\t<20221024000356.29521-12-laurent.pinchart@ideasonboard.com>\n\t<166699506602.404886.879894391167680910@Monstersaurus>\n\t","To":"Laurent Pinchart ","Date":"Wed, 23 Nov 2022 09:56:06 +0000","Message-ID":"<166919736649.1079859.10921833042765076716@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25868,"web_url":"https://patchwork.libcamera.org/comment/25868/","msgid":"","date":"2022-11-23T11:06:09","subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Wed, Nov 23, 2022 at 09:56:06AM +0000, Kieran Bingham wrote:\n> Quoting Laurent Pinchart (2022-10-30 17:24:49)\n> > On Fri, Oct 28, 2022 at 11:11:06PM +0100, Kieran Bingham wrote:\n> > > Quoting Laurent Pinchart via libcamera-devel (2022-10-24 01:03:54)\n> > > > Unlike RkISP1Path::generateConfiguration(), the validate() function\n> > > > doesn't take the camera sensor resolution into account but only\n> > > > considers the absolute minimum and maximum sizes support by the ISP to\n> > > > validate the stream size. Fix it by using the same logic as when\n> > > > generating the configuration.\n> > > > \n> > > > Instead of passing the sensor resolution to the validate() function,\n> > > > pass the CameraSensor pointer to prepare for subsequent changes that\n> > > > will require access to more camera sensor data. While at it, update the\n> > > > generateConfiguration() function similarly for the same reason.\n> > > \n> > > I have quite the surprising curveball on this patch.\n> > > \n> > > While the validation seems to aim to restrict sizes to the capabilities\n> > > of the ISP and the Sensor size, it seems to miss something.\n> > > \n> > > I've hooked up an Arducam 16MP to test this - and the set up fails\n> > > because the sensor size selected is larger than the maximum input size\n> > > of the ISP.\n> > > \n> > > Can this somehow take the input limtiations of the ISP into account when\n> > > selecting and filtering sensor sizes easily ?\n> > \n> > Is this a regression ?\n> \n> I wasn't able to test this camera before on this platform. So it's not a\n> regression for me, it's just never worked...\n> \n> But ... if we're \"fixing the validation of the stream size\", I'd call\n> this a bug report that this patch doesn't seem to account for the ISP\n> limitations when doing so.\n\nNo disagreement there :-) What I'm wondering is if it has to be fixed in\nhere, or could be addressed on top. And of course if someone with access\nto a 16MP camera module could do it, that would be even better ;-)\n\n> I've worked around this by removing the larger image sizes from the\n> sensor driver currently... (Then I hit an issue where the Pixelrate of\n> the sensor is not supported by the ISP ... so something else is wrong\n> there).\n> \n> > > > Signed-off-by: Laurent Pinchart \n> > > > ---\n> > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++++-------\n> > > > src/libcamera/pipeline/rkisp1/rkisp1_path.cpp | 22 +++++++++++++++----\n> > > > src/libcamera/pipeline/rkisp1/rkisp1_path.h | 6 +++--\n> > > > 3 files changed, 31 insertions(+), 14 deletions(-)\n> > > > \n> > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > index cca89cc13bff..dcab5286aa56 100644\n> > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > @@ -404,14 +404,15 @@ RkISP1CameraConfiguration::RkISP1CameraConfiguration(Camera *camera,\n> > > > \n> > > > bool RkISP1CameraConfiguration::fitsAllPaths(const StreamConfiguration &cfg)\n> > > > {\n> > > > + const CameraSensor *sensor = data_->sensor_.get();\n> > > > StreamConfiguration config;\n> > > > \n> > > > config = cfg;\n> > > > - if (data_->mainPath_->validate(&config) != Valid)\n> > > > + if (data_->mainPath_->validate(sensor, &config) != Valid)\n> > > > return false;\n> > > > \n> > > > config = cfg;\n> > > > - if (data_->selfPath_ && data_->selfPath_->validate(&config) != Valid)\n> > > > + if (data_->selfPath_ && data_->selfPath_->validate(sensor, &config) != Valid)\n> > > > return false;\n> > > > \n> > > > return true;\n> > > > @@ -459,7 +460,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > > > /* Try to match stream without adjusting configuration. */\n> > > > if (mainPathAvailable) {\n> > > > StreamConfiguration tryCfg = cfg;\n> > > > - if (data_->mainPath_->validate(&tryCfg) == Valid) {\n> > > > + if (data_->mainPath_->validate(sensor, &tryCfg) == Valid) {\n> > > > mainPathAvailable = false;\n> > > > cfg = tryCfg;\n> > > > cfg.setStream(const_cast(&data_->mainPathStream_));\n> > > > @@ -469,7 +470,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > > > \n> > > > if (selfPathAvailable) {\n> > > > StreamConfiguration tryCfg = cfg;\n> > > > - if (data_->selfPath_->validate(&tryCfg) == Valid) {\n> > > > + if (data_->selfPath_->validate(sensor, &tryCfg) == Valid) {\n> > > > selfPathAvailable = false;\n> > > > cfg = tryCfg;\n> > > > cfg.setStream(const_cast(&data_->selfPathStream_));\n> > > > @@ -480,7 +481,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > > > /* Try to match stream allowing adjusting configuration. */\n> > > > if (mainPathAvailable) {\n> > > > StreamConfiguration tryCfg = cfg;\n> > > > - if (data_->mainPath_->validate(&tryCfg) == Adjusted) {\n> > > > + if (data_->mainPath_->validate(sensor, &tryCfg) == Adjusted) {\n> > > > mainPathAvailable = false;\n> > > > cfg = tryCfg;\n> > > > cfg.setStream(const_cast(&data_->mainPathStream_));\n> > > > @@ -491,7 +492,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > > > \n> > > > if (selfPathAvailable) {\n> > > > StreamConfiguration tryCfg = cfg;\n> > > > - if (data_->selfPath_->validate(&tryCfg) == Adjusted) {\n> > > > + if (data_->selfPath_->validate(sensor, &tryCfg) == Adjusted) {\n> > > > selfPathAvailable = false;\n> > > > cfg = tryCfg;\n> > > > cfg.setStream(const_cast(&data_->selfPathStream_));\n> > > > @@ -603,11 +604,11 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> > > > StreamConfiguration cfg;\n> > > > if (useMainPath) {\n> > > > cfg = data->mainPath_->generateConfiguration(\n> > > > - data->sensor_->resolution());\n> > > > + data->sensor_.get());\n> > > > mainPathAvailable = false;\n> > > > } else {\n> > > > cfg = data->selfPath_->generateConfiguration(\n> > > > - data->sensor_->resolution());\n> > > > + data->sensor_.get());\n> > > > selfPathAvailable = false;\n> > > > }\n> > > > \n> > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > > > index 8077a54494a5..cc2ac66e6939 100644\n> > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > > > @@ -12,6 +12,7 @@\n> > > > #include \n> > > > #include \n> > > > \n> > > > +#include \"libcamera/internal/camera_sensor.h\"\n> > > > #include \"libcamera/internal/media_device.h\"\n> > > > #include \"libcamera/internal/v4l2_subdevice.h\"\n> > > > #include \"libcamera/internal/v4l2_videodevice.h\"\n> > > > @@ -85,8 +86,10 @@ void RkISP1Path::populateFormats()\n> > > > }\n> > > > }\n> > > > \n> > > > -StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n> > > > +StreamConfiguration RkISP1Path::generateConfiguration(const CameraSensor *sensor)\n> > > > {\n> > > > + const Size &resolution = sensor->resolution();\n> > > > +\n> > > > Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n> > > > .boundedTo(resolution);\n> > > > Size minResolution = minResolution_.expandedToAspectRatio(resolution);\n> > > > @@ -104,8 +107,11 @@ StreamConfiguration RkISP1Path::generateConfiguration(const Size &resolution)\n> > > > return cfg;\n> > > > }\n> > > > \n> > > > -CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> > > > +CameraConfiguration::Status RkISP1Path::validate(const CameraSensor *sensor,\n> > > > + StreamConfiguration *cfg)\n> > > > {\n> > > > + const Size &resolution = sensor->resolution();\n> > > > +\n> > > > const StreamConfiguration reqCfg = *cfg;\n> > > > CameraConfiguration::Status status = CameraConfiguration::Valid;\n> > > > \n> > > > @@ -117,8 +123,16 @@ CameraConfiguration::Status RkISP1Path::validate(StreamConfiguration *cfg)\n> > > > if (!streamFormats_.count(cfg->pixelFormat))\n> > > > cfg->pixelFormat = formats::NV12;\n> > > > \n> > > > - cfg->size.boundTo(maxResolution_);\n> > > > - cfg->size.expandTo(minResolution_);\n> > > > + /*\n> > > > + * Adjust the size based on the sensor resolution and absolute limits\n> > > > + * of the ISP.\n> > > > + */\n> > > > + Size maxResolution = maxResolution_.boundedToAspectRatio(resolution)\n> > > > + .boundedTo(resolution);\n> > > \n> > > I played around here and expected that limiting this maxResolution to\n> > > the maximum Input resolution should help - but I'm missing something and\n> > > haven't delved deep enough to figure out what yet.\n> > > \n> > > > + Size minResolution = minResolution_.expandedToAspectRatio(resolution);\n> > > > +\n> > > > + cfg->size.boundTo(maxResolution);\n> > > > + cfg->size.expandTo(minResolution);\n> > > > cfg->bufferCount = RKISP1_BUFFER_COUNT;\n> > > > \n> > > > V4L2DeviceFormat format;\n> > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> > > > index d88effbb6f56..bf4ad18fbbf2 100644\n> > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> > > > @@ -23,6 +23,7 @@\n> > > > \n> > > > namespace libcamera {\n> > > > \n> > > > +class CameraSensor;\n> > > > class MediaDevice;\n> > > > class V4L2Subdevice;\n> > > > struct StreamConfiguration;\n> > > > @@ -39,8 +40,9 @@ public:\n> > > > int setEnabled(bool enable) { return link_->setEnabled(enable); }\n> > > > bool isEnabled() const { return link_->flags() & MEDIA_LNK_FL_ENABLED; }\n> > > > \n> > > > - StreamConfiguration generateConfiguration(const Size &resolution);\n> > > > - CameraConfiguration::Status validate(StreamConfiguration *cfg);\n> > > > + StreamConfiguration generateConfiguration(const CameraSensor *sensor);\n> > > > + CameraConfiguration::Status validate(const CameraSensor *sensor,\n> > > > + StreamConfiguration *cfg);\n> > > > \n> > > > int configure(const StreamConfiguration &config,\n> > > > const V4L2SubdeviceFormat &inputFormat);","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 8E85DBE08B\n\tfor ;\n\tWed, 23 Nov 2022 11:06:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1558E63313;\n\tWed, 23 Nov 2022 12:06:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7164263311\n\tfor ;\n\tWed, 23 Nov 2022 12:06:26 +0100 (CET)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A5AFC88F;\n\tWed, 23 Nov 2022 12:06:25 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669201588;\n\tbh=2poE3Q6cY6CNN0bh82w2jRrZtENJpcAX0npdDvPZJbQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=WYTDOEo2YYxwwRZphpgf2pR8d0sQYlNQKOs8A25kapj6mGHBiTelapMJwN5P4AGx2\n\tWLsPZxSjTv33IgZiow2240EOetTUQ+ojaWeZlKBvqogc4VuXEQM2kr4eIxJGiK5JgR\n\tLR+HkgK2KNq1MWKo8r9bOuc4Kk7LUBjbrBm5/d65V9FFZiMi+Qj70BQbYLxFBCGlDF\n\tN4+QAp361gHuM00SCZUGyCq8gWHZ0HK/NOCWoP/huy9Ho5JDfN1c2CzbXM4h44uocV\n\tDrgGv+NeX4AKZcO3whMWfxiCLkew4ePKOhehmK4P5L3B6Tp70cJV0ZcrUqN5Sy+KV0\n\tACAxd7EZn+H/w==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669201585;\n\tbh=2poE3Q6cY6CNN0bh82w2jRrZtENJpcAX0npdDvPZJbQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=O7N3uc3AM2ApURaCqFxXgVPKNvJ3Ksnq5gnj5TqapwsA1Y9bJAzfapbyo3G7g60JX\n\t+wZsrOlp9x2O6vpaLcN0KjXUDVLjA8d6+fN5sR6/nrYqiFnt+aguLmFWihS4KwQAjq\n\t28wG2UP+Nv2rQEAr7YTxeNXogZ8zOdPH7G+rglWk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"O7N3uc3A\"; dkim-atps=neutral","Date":"Wed, 23 Nov 2022 13:06:09 +0200","To":"Kieran Bingham ","Message-ID":"","References":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>\n\t<20221024000356.29521-12-laurent.pinchart@ideasonboard.com>\n\t<166699506602.404886.879894391167680910@Monstersaurus>\n\t\n\t<166919736649.1079859.10921833042765076716@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<166919736649.1079859.10921833042765076716@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","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","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25874,"web_url":"https://patchwork.libcamera.org/comment/25874/","msgid":"<166920900482.1079859.6336055767256105804@Monstersaurus>","date":"2022-11-23T13:10:04","subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Laurent Pinchart (2022-11-23 11:06:09)\n> Hi Kieran,\n> \n> On Wed, Nov 23, 2022 at 09:56:06AM +0000, Kieran Bingham wrote:\n> > Quoting Laurent Pinchart (2022-10-30 17:24:49)\n> > > On Fri, Oct 28, 2022 at 11:11:06PM +0100, Kieran Bingham wrote:\n> > > > Quoting Laurent Pinchart via libcamera-devel (2022-10-24 01:03:54)\n> > > > > Unlike RkISP1Path::generateConfiguration(), the validate() function\n> > > > > doesn't take the camera sensor resolution into account but only\n> > > > > considers the absolute minimum and maximum sizes support by the ISP to\n> > > > > validate the stream size. Fix it by using the same logic as when\n> > > > > generating the configuration.\n> > > > > \n> > > > > Instead of passing the sensor resolution to the validate() function,\n> > > > > pass the CameraSensor pointer to prepare for subsequent changes that\n> > > > > will require access to more camera sensor data. While at it, update the\n> > > > > generateConfiguration() function similarly for the same reason.\n> > > > \n> > > > I have quite the surprising curveball on this patch.\n> > > > \n> > > > While the validation seems to aim to restrict sizes to the capabilities\n> > > > of the ISP and the Sensor size, it seems to miss something.\n> > > > \n> > > > I've hooked up an Arducam 16MP to test this - and the set up fails\n> > > > because the sensor size selected is larger than the maximum input size\n> > > > of the ISP.\n> > > > \n> > > > Can this somehow take the input limtiations of the ISP into account when\n> > > > selecting and filtering sensor sizes easily ?\n> > > \n> > > Is this a regression ?\n> > \n> > I wasn't able to test this camera before on this platform. So it's not a\n> > regression for me, it's just never worked...\n> > \n> > But ... if we're \"fixing the validation of the stream size\", I'd call\n> > this a bug report that this patch doesn't seem to account for the ISP\n> > limitations when doing so.\n> \n> No disagreement there :-) What I'm wondering is if it has to be fixed in\n> here, or could be addressed on top. And of course if someone with access\n> to a 16MP camera module could do it, that would be even better ;-)\n\nIt can be done on top.\n--\nKieran","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 53D4EBE08B\n\tfor ;\n\tWed, 23 Nov 2022 13:10:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CB34063313;\n\tWed, 23 Nov 2022 14:10:09 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1A1F363311\n\tfor ;\n\tWed, 23 Nov 2022 14:10:08 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id A7D7C890;\n\tWed, 23 Nov 2022 14:10:07 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1669209009;\n\tbh=SWGw9QzWzmuUUrs6VxB6Yulr0WIcXubWyV9XcKg9OcE=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=jeOs7vyG3cA7jgF4XslYIGO1y7X/VMjxyfTbH3WMLWB1JmZucs3oPZgq3ZYWqqUsS\n\tQbJqkw1SvLxPmRdxiAIiZgneSyFa0Zcgy14A8GFTie44Pd2JeMeCb11iLPdtmXf+zU\n\tf86NPgQXid6BYKqUL3tMZ9rs5dJO3B//vfBG455N5LrN67dmybf8twU1vdKlR+E2L6\n\t7SL4rO82YoF2gZxIIIwQYeuZ3deXqczIrAZY6G4QnSOM5xe9HpSJRgCOqU2S5qsTjv\n\tjD50m8EvfGzAnZeRvaOm5zXCcRpR0RWW3ncxqdaIfcVHbN3CxNsxI5L6zefEBZ9iOS\n\tkBZ1+Jo13z/ZA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1669209007;\n\tbh=SWGw9QzWzmuUUrs6VxB6Yulr0WIcXubWyV9XcKg9OcE=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=rGl3Sciau9Z9dsrK4teQ0JDkke1VTvtWx8etWnxXVmsQL0BZH5ybUC56s3d1uDBpd\n\tX9rxN72FRuO47j8U/YxF5/L7sEVlToNjcjtLHBT6sxXwmJkfu/GEjVwYvWhjjRqlXo\n\t0T1tlXjRyRvUejj1NUOZJ14NxA7t5EvK2PRjCe2w="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"rGl3Scia\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>\n\t<20221024000356.29521-12-laurent.pinchart@ideasonboard.com>\n\t<166699506602.404886.879894391167680910@Monstersaurus>\n\t\n\t<166919736649.1079859.10921833042765076716@Monstersaurus>\n\t","To":"Laurent Pinchart ","Date":"Wed, 23 Nov 2022 13:10:04 +0000","Message-ID":"<166920900482.1079859.6336055767256105804@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v3 11/13] pipeline: rkisp1: Fix stream\n\tsize validation","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","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]