{"id":17674,"url":"https://patchwork.libcamera.org/api/1.1/patches/17674/?format=json","web_url":"https://patchwork.libcamera.org/patch/17674/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20221024000356.29521-12-laurent.pinchart@ideasonboard.com>","date":"2022-10-24T00:03:54","name":"[libcamera-devel,v3,11/13] pipeline: rkisp1: Fix stream size validation","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"a39b13a62e551ef7f9183db0ee4377987c3fefd4","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/17674/mbox/","series":[{"id":3574,"url":"https://patchwork.libcamera.org/api/1.1/series/3574/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=3574","date":"2022-10-24T00:03:43","name":"Add Bayer format support for RkISP1","version":3,"mbox":"https://patchwork.libcamera.org/series/3574/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/17674/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/17674/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 59BD7C328B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 Oct 2022 00:04:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D45B462F08;\n\tMon, 24 Oct 2022 02:04:39 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4264962EDF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 Oct 2022 02:04:36 +0200 (CEST)","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 C9998471;\n\tMon, 24 Oct 2022 02:04:35 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666569879;\n\tbh=4PooF5sHVc7qAJ/Kx6rffl96ItXM2SZY+ALQvtsoE2c=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=e3gVVW4q/s9pUnUeMpvRWEyTrHv5k848EAlCX16CxS+kgog67DQm92ppMX5EMKRzR\n\t963RDm20CWURBjjGyak3WMA56hIW2EFPZdq3McNgi5Dc3E+WPfsbkdvWdI/F+ZY3T1\n\tV72miDrL/lMAh+JnguvtWghuZW+rPemRbVDLIkW5DwK+hoD6eIvtLjVTzPXegJAFnz\n\tbG2pmI8WgLPN89R/hxWEp0Ec343L57tprcsJRU0FWuvksvuj3tBn/2sT7t04roUMu4\n\t/ZmS0FMTO125Pjv5wp7NtekqbSqNPEjjtE+1+WZX/gTk4G6xGaXb+cu6C+5mV5aqHr\n\tXZ6AgC1QNvzLg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1666569876;\n\tbh=4PooF5sHVc7qAJ/Kx6rffl96ItXM2SZY+ALQvtsoE2c=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=YqtqsQxrSi7xk+g1sA0h3M460R6RphgeHbFNiBdowESsCnnwBrsjs7zo/MzPTJ+wF\n\tLGlne04DulguSNW9g1NQyE2RdOiTS1ybA0p9lazK/lISj2kjv2qcIptbVLmqgx7p3r\n\tAKRR1RkX7YQfw1IT/6Rz6+uwOOtc/v8I2rPmRzsI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"YqtqsQxr\"; dkim-atps=neutral","To":"libcamera-devel@lists.libcamera.org","Date":"Mon, 24 Oct 2022 03:03:54 +0300","Message-Id":"<20221024000356.29521-12-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.37.4","In-Reply-To":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>","References":"<20221024000356.29521-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"Unlike RkISP1Path::generateConfiguration(), the validate() function\ndoesn't take the camera sensor resolution into account but only\nconsiders the absolute minimum and maximum sizes support by the ISP to\nvalidate the stream size. Fix it by using the same logic as when\ngenerating the configuration.\n\nInstead of passing the sensor resolution to the validate() function,\npass the CameraSensor pointer to prepare for subsequent changes that\nwill require access to more camera sensor data. While at it, update the\ngenerateConfiguration() function similarly for the same reason.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\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(-)","diff":"diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 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<Stream *>(&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<Stream *>(&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<Stream *>(&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<Stream *>(&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 \ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\nindex 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 <libcamera/formats.h>\n #include <libcamera/stream.h>\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;\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\nindex 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","prefixes":["libcamera-devel","v3","11/13"]}