[{"id":32698,"web_url":"https://patchwork.libcamera.org/comment/32698/","msgid":"","date":"2024-12-12T12:03:19","subject":"Re: [PATCH v3 11/17] libcamera: rkisp1: Refactor path validation","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Fri, Dec 06, 2024 at 11:13:33AM +0100, Stefan Klug wrote:\n> Refactor validation code to prepare for extensions in the upcoming\n> patches. Code duplication is reduced by moving parts of the validation\n> logic into a lambda function. This patch does not include any functional\n> changes.\n> \n> Signed-off-by: Stefan Klug \n> Signed-off-by: Jacopo Mondi \n\nReviewed-by: Paul Elder \n\n> \n> ---\n> Changes in v3:\n> - Added this patch\n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 35 +++++++++++++-----------\n> 1 file changed, 19 insertions(+), 16 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 098c560ca5c8..7f41092ee2d5 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -558,50 +558,53 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> \tif (config_.size() == 2 && fitsAllPaths(config_[0]))\n> \t\tstd::reverse(order.begin(), order.end());\n> \n> +\tauto validateConfig = [&](StreamConfiguration &cfg, RkISP1Path *path,\n> +\t\t\t\t Stream *stream, Status expectedStatus) {\n> +\t\tStreamConfiguration tryCfg = cfg;\n> +\t\tif (path->validate(sensor, sensorConfig, &tryCfg) != expectedStatus)\n> +\t\t\treturn false;\n> +\n> +\t\tcfg = tryCfg;\n> +\t\tcfg.setStream(stream);\n> +\t\treturn true;\n> +\t};\n> +\n> \tbool mainPathAvailable = true;\n> \tbool selfPathAvailable = data_->selfPath_;\n> +\tRkISP1Path *mainPath = data_->mainPath_;\n> +\tRkISP1Path *selfPath = data_->selfPath_;\n> +\tStream *mainPathStream = const_cast(&data_->mainPathStream_);\n> +\tStream *selfPathStream = const_cast(&data_->selfPathStream_);\n> \tfor (unsigned int index : order) {\n> \t\tStreamConfiguration &cfg = config_[index];\n> \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(sensor, sensorConfig, &tryCfg) == Valid) {\n> +\t\t\tif (validateConfig(cfg, mainPath, mainPathStream, Valid)) {\n> \t\t\t\tmainPathAvailable = false;\n> -\t\t\t\tcfg = tryCfg;\n> -\t\t\t\tcfg.setStream(const_cast(&data_->mainPathStream_));\n> \t\t\t\tcontinue;\n> \t\t\t}\n> \t\t}\n> \n> \t\tif (selfPathAvailable) {\n> -\t\t\tStreamConfiguration tryCfg = cfg;\n> -\t\t\tif (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Valid) {\n> +\t\t\tif (validateConfig(cfg, selfPath, selfPathStream, Valid)) {\n> \t\t\t\tselfPathAvailable = false;\n> -\t\t\t\tcfg = tryCfg;\n> -\t\t\t\tcfg.setStream(const_cast(&data_->selfPathStream_));\n> \t\t\t\tcontinue;\n> \t\t\t}\n> \t\t}\n> \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(sensor, sensorConfig, &tryCfg) == Adjusted) {\n> +\t\t\tif (validateConfig(cfg, mainPath, mainPathStream, Adjusted)) {\n> \t\t\t\tmainPathAvailable = false;\n> -\t\t\t\tcfg = tryCfg;\n> -\t\t\t\tcfg.setStream(const_cast(&data_->mainPathStream_));\n> \t\t\t\tstatus = Adjusted;\n> \t\t\t\tcontinue;\n> \t\t\t}\n> \t\t}\n> \n> \t\tif (selfPathAvailable) {\n> -\t\t\tStreamConfiguration tryCfg = cfg;\n> -\t\t\tif (data_->selfPath_->validate(sensor, sensorConfig, &tryCfg) == Adjusted) {\n> +\t\t\tif (validateConfig(cfg, selfPath, selfPathStream, Adjusted)) {\n> \t\t\t\tselfPathAvailable = false;\n> -\t\t\t\tcfg = tryCfg;\n> -\t\t\t\tcfg.setStream(const_cast(&data_->selfPathStream_));\n> \t\t\t\tstatus = Adjusted;\n> \t\t\t\tcontinue;\n> \t\t\t}\n> -- \n> 2.43.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 56833BD80A\n\tfor ;\n\tThu, 12 Dec 2024 12:03:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 82BC467ED0;\n\tThu, 12 Dec 2024 13:03: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 8FA51608B6\n\tfor ;\n\tThu, 12 Dec 2024 13:03:26 +0100 (CET)","from pyrite.rasen.tech (unknown\n\t[IPv6:2404:7a81:160:2100:39eb:989c:b016:217b])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8DCA1316;\n\tThu, 12 Dec 2024 13:02:51 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"j/ZDTpqv\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1734004972;\n\tbh=3/EYbsITjOc7ueTxKDoFVTj/2Yy00DQxryRhz652pRU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=j/ZDTpqvP2wzokz4/NQkLjTvD9CUamuyEgnfxa+HqC0vcwC49Ib/pbHsOFjEUK3pj\n\tKf/pFLUxGXDnj13zXLu7mnC9wihuwM2W9hSXnsZdt2YKlcovNctw27jjiZjQ8EKB6C\n\t1nxStglyZXFw9CDf+6s00X7Y+9TCmGla+XlxuJ/w=","Date":"Thu, 12 Dec 2024 21:03:19 +0900","From":"Paul Elder ","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org,\n\tJacopo Mondi ","Subject":"Re: [PATCH v3 11/17] libcamera: rkisp1: Refactor path validation","Message-ID":"","References":"<20241206101344.767170-1-stefan.klug@ideasonboard.com>\n\t<20241206101344.767170-12-stefan.klug@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20241206101344.767170-12-stefan.klug@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\" "}}]