[{"id":23521,"web_url":"https://patchwork.libcamera.org/comment/23521/","msgid":"<165588670259.1149771.11402904120895037417@Monstersaurus>","date":"2022-06-22T08:31:42","subject":"Re: [libcamera-devel] [PATCH] libcamera: rkisp1: Generate\n\tconfiguration from main path if only one role","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Paul Elder via libcamera-devel (2022-06-22 08:46:08)\n> The current logic for generating configurations assumes that we have\n> multiple roles. The consequence of this is that if only one role is\n> requested, and it is for viewfinder or recording, then the self path's\n> configuration generator would be used instead of the main path's. This\n> is what causes the default resolution on the rkisp1 pipeline handler to\n> be 1920x1920 (since it's the max resolution of the self path). Note that\n> the main path is still used for streaming, just that it is using self\n> path's default configuraion (if it isn't changed by the application).\n> \n> This patch skips all the logic for determining which path to assign to\n> which role in the event that only one role is requested. In this case,\n> we simply generate the configuration from the math path. This makes the\n\ns/math/main/\n\n> default resolution for a single stream 2592x1944.\n\nThis sounds like quite a large effect, but I suspect it's ok. If a\nStreamRole is set to ViewFinder, should this default resolution be\nsmaller?\n\n\n\n> \n> Signed-off-by: Paul Elder \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 10 ++++++++++\n> 1 file changed, 10 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 7cf36524..43b76e14 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -515,6 +515,16 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> if (roles.empty())\n> return config;\n> \n> + if (roles.size() == 1) {\n> + StreamConfiguration cfg = data->mainPath_->generateConfiguration(\n> + data->sensor_->resolution());\n> +\n> + config->addConfiguration(cfg);\n> + config->validate();\n> +\n> + return config;\n> + }\n> +\n> bool mainPathAvailable = true;\n> bool selfPathAvailable = true;\n> for (const StreamRole role : roles) {\n> -- \n> 2.30.2\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 96A40BD808\n\tfor ;\n\tWed, 22 Jun 2022 08:31:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A5A2865635;\n\tWed, 22 Jun 2022 10:31:46 +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 5F8786559A\n\tfor ;\n\tWed, 22 Jun 2022 10:31:45 +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 DACD8DD;\n\tWed, 22 Jun 2022 10:31:44 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655886706;\n\tbh=3CyN6VLSPYDjPmYNAa1WilZ55Zmsoa+xKT+PXsuaXm0=;\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=lgnQJCAhVLvQC12Ob9c69Ox8hxQ3PbltyfrMpDFrfTWz80bMegc96Q11fsLOKipqi\n\tabPf9h4111GJMGCX/9fKwCd80ge6jtVXU70YhlUi88edUKD/bdn5HTfML2iUxd6g5w\n\tsvmp8ZK1zxghMD5KesKAl3d7fq9chBDgXqFBWF6oXin2FVTV8DR9OpA9TZrASgRZaN\n\tU8BU4g1i+w1dnzObL/FrNu1ZnRa6zIWZFvAvRvRQ5C2ySrAFv+mYAZq4Hgi0cfrY9g\n\tL7wDcxxQZWuQmSYFNYpIchhwUpbF3Ug32g2pCBfjiayS40KCDY2cM6Kt5F2Z0cAkmn\n\tg4cUyxzZqp+HA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655886705;\n\tbh=3CyN6VLSPYDjPmYNAa1WilZ55Zmsoa+xKT+PXsuaXm0=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=BKaVS0TDidYzQyu3suTUFNRClxQ9w2Mfdu0NfwHJZEDvykS7hYoQJOr8hJvm6KzTF\n\trPVHbwyT5ZeJmZcXDezLb2TL4IR++dLxD9oQJlBMsSkqDIJTqup0JlqRMxGbjVhw78\n\t2QyNPXhJdJKuexWL9a2adIF56FSEI4pdBmvVEKyc="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"BKaVS0TD\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220622074608.142669-1-paul.elder@ideasonboard.com>","References":"<20220622074608.142669-1-paul.elder@ideasonboard.com>","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 22 Jun 2022 09:31:42 +0100","Message-ID":"<165588670259.1149771.11402904120895037417@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH] libcamera: rkisp1: Generate\n\tconfiguration from main path if only one role","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":23524,"web_url":"https://patchwork.libcamera.org/comment/23524/","msgid":"","date":"2022-06-22T09:02:09","subject":"Re: [libcamera-devel] [PATCH] libcamera: rkisp1: Generate\n\tconfiguration from main path if only one role","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Wed, Jun 22, 2022 at 04:46:08PM +0900, Paul Elder via libcamera-devel wrote:\n> The current logic for generating configurations assumes that we have\n> multiple roles. The consequence of this is that if only one role is\n> requested, and it is for viewfinder or recording, then the self path's\n> configuration generator would be used instead of the main path's. This\n> is what causes the default resolution on the rkisp1 pipeline handler to\n> be 1920x1920 (since it's the max resolution of the self path). Note that\n\nThis sounds like a bug, RkISP1Path::generateConfiguration() should take\nthe sensor aspect ratio into account. Could you check what is happening\nthere ?\n\n> the main path is still used for streaming, just that it is using self\n> path's default configuraion (if it isn't changed by the application).\n> \n> This patch skips all the logic for determining which path to assign to\n> which role in the event that only one role is requested. In this case,\n> we simply generate the configuration from the math path. This makes the\n> default resolution for a single stream 2592x1944.\n\nI'm a bit bothered by the rationale here. It's not so much that\ngenerateConfiguration() assumes that there *are* multiple roles, but\nthat there *can* be multiple roles because it assumes there are multiple\n*paths*. What we need to drop is the assumption we have a self path (and\nthe commit message should mention that's because the ISP8000Nano in the\ni.MX8MP has a main path only). The availability of the self path should\nbe detected (possibly at init() time and cached, or in\ngenerateConfiguration() if it's cheap), and generateConfiguration()\nshould then act accordingly, restricting the configuration to a single\nstream in that case.\n\nThere's a bit more work needed on the kernel side to not register the\nself path for the i.MX8MP.\n\n> Signed-off-by: Paul Elder \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 10 ++++++++++\n> 1 file changed, 10 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 7cf36524..43b76e14 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -515,6 +515,16 @@ CameraConfiguration *PipelineHandlerRkISP1::generateConfiguration(Camera *camera\n> \tif (roles.empty())\n> \t\treturn config;\n> \n> +\tif (roles.size() == 1) {\n> +\t\tStreamConfiguration cfg = data->mainPath_->generateConfiguration(\n> +\t\t\tdata->sensor_->resolution());\n> +\n> +\t\tconfig->addConfiguration(cfg);\n> +\t\tconfig->validate();\n> +\n> +\t\treturn config;\n> +\t}\n> +\n> \tbool mainPathAvailable = true;\n> \tbool selfPathAvailable = true;\n> \tfor (const StreamRole role : roles) {","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 E9A10BE173\n\tfor ;\n\tWed, 22 Jun 2022 09:02:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0E6FA65635;\n\tWed, 22 Jun 2022 11:02:27 +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 9F18E6559A\n\tfor ;\n\tWed, 22 Jun 2022 11:02:25 +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 08A64DD;\n\tWed, 22 Jun 2022 11:02:24 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1655888547;\n\tbh=APZWncdSNloyHmetD1wrvuuqBxaoTNtuE9LRgY1VLgo=;\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=vhiPRER6ZzcAocOLMMaIWjlr3SV1YM2lz3ogtNws7LXS8wgdDcpT0ZL8cNFC5ioq4\n\tBWZPcdoz7K6iUE+CHmayTzEybWRUL+O0yu5N3GdTP2PFQl3Yy77MFbHVj2Z3pR7jEa\n\ttF0MVvJnr69rq0s/5QY8gvSoX1Y8pyKdn5f98D+GD9pokdXw1tcth/XStLTp5xltlm\n\tunKSKClpXSDT3LsU7o08Us4fcFlxHScmSyd6g5YBsvTUy7nTYitqnqK+zar4jK/NhF\n\tlx19T420zwIKK38ODh0ZgWAgrwsTF39slPu7uI2JDYm2xpDLH6ydMnupwNJqMKzZbk\n\tVDQwsiTq6SViA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1655888545;\n\tbh=APZWncdSNloyHmetD1wrvuuqBxaoTNtuE9LRgY1VLgo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WICr0E9sHuRUdrfaMNzroDoJ/AC1VjGeyphN45lB9QXsn2wSr848bTM6pvZDH47hI\n\t5134pRlFk068razTmIBwC7H3TrsVkcMURjcd/InSjTmgoZOBXDV4FT7Bia+NVihuU9\n\to0aUVoCaSNAY9So4JRfVj58Mow56+PtVABXNtNlU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"WICr0E9s\"; dkim-atps=neutral","Date":"Wed, 22 Jun 2022 12:02:09 +0300","To":"Paul Elder ","Message-ID":"","References":"<20220622074608.142669-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220622074608.142669-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: rkisp1: Generate\n\tconfiguration from main path if only one role","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\" "}}]