[{"id":26543,"web_url":"https://patchwork.libcamera.org/comment/26543/","msgid":"","date":"2023-03-03T08:42:33","subject":"Re: [libcamera-devel] [PATCH v2 3/4] libcamera: rkisp1: Reserve\n\tmain path for StillCapture","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Wed, Feb 22, 2023 at 04:19:16PM +0100, Jacopo Mondi via libcamera-devel wrote:\n> The main output path can produce images in higher resolution and\n> should be reserved for the StillCapture role when a configuration\n> is generated.\n> \n> Before this change if StillCapture was not requested first it got\n> assigned to the self-path and thus down-scaled to 1920x1920.\n> \n> With this change, StillCapture can be asked last and it would take\n> precedence over other streams for the usage of the main path.\n> \n> $ cam -c1 --stream role=viewfinder --stream role=still\n> Camera camera.cpp:969 streams configuration: (0) 1920x1080-NV12 (1) 4208x3120-NV12\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 15 ++++++++++++++-\n> 1 file changed, 14 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 05a7ba03b2d2..23ff859e3835 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -626,6 +626,16 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> \tif (roles.empty())\n> \t\treturn config;\n> \n> +\t/* If still capture is requested, reserve the main path for it. */\n> +\tbool reserveMainPath = false;\n> +\tfor (const StreamRole role : roles) {\n> +\t\tif (role != StreamRole::StillCapture)\n> +\t\t\tcontinue;\n> +\n> +\t\treserveMainPath = true;\n> +\t\tbreak;\n> +\t}\n> +\n\nHow about...\n\n\tbool reserveMainPath =\n\t\tstd::find(roles.cbegin(), roles.cend(),\n\t\t\t StreamRole::StillCapture) != roles.cend();\n\nWhich doesn't actually look that nice either. Oh well. Up to you.\n\n\nReviewed-by: Paul Elder \n\n> \t/*\n> \t * As the ISP can't output different color spaces for the main and self\n> \t * path, pick a sensible default color space based on the role of the\n> @@ -644,6 +654,9 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> \t\t\tif (!colorSpace)\n> \t\t\t\tcolorSpace = ColorSpace::Sycc;\n> \n> +\t\t\t/* Unlock usage of main path which was reserved. */\n> +\t\t\treserveMainPath = false;\n> +\n> \t\t\tsize = data->sensor_->resolution();\n> \t\t\tbreak;\n> \n> @@ -685,7 +698,7 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> \n> \t\tRkISP1Path *path;\n> \n> -\t\tif (useMainPath) {\n> +\t\tif (useMainPath && !reserveMainPath) {\n> \t\t\tpath = data->mainPath_;\n> \t\t\tmainPathAvailable = false;\n> \t\t} else {\n> -- \n> 2.39.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 48D04BE080\n\tfor ;\n\tFri, 3 Mar 2023 08:42:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9B989626E6;\n\tFri, 3 Mar 2023 09:42:42 +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 27559626E4\n\tfor ;\n\tFri, 3 Mar 2023 09:42:41 +0100 (CET)","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 6482D8BE;\n\tFri, 3 Mar 2023 09:42:39 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1677832962;\n\tbh=S4KmlAxmVRXDVU6/MXHSS93fYb2eT4PIxcPpt7BC6cE=;\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=x78cfeVsBCWWyiAPW4tsvTLHsua9pSa7ZzXZvRBtdMvk4ZYv7auA0/BffS0Mhfpdm\n\tWGMe4a67VdZ6/kpCm0MqMHMp4YbuMxOGvBgu17jf6ct5MeeqxkZWBlSXhSCCCd1H+3\n\tux4iZZOth2xF6UBOI6QudXrEuCFyQKYBXUg1ENCicsN+tW/ykN/pkTt2ssW61YdB2S\n\tGkEIH2cC+Gn32U/4TulQKGqum0kIRZui/bYZA0gmBh6TPOpLh4FbCNvN9t8yM26Vg3\n\tmsFjQrbEXww08SEwPpmuAWIm19N2Pn6Px8Qr05lcFatX9g0OGkWvTOi/ThRJpCiOaj\n\tZjAppQaoKBxPA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1677832960;\n\tbh=S4KmlAxmVRXDVU6/MXHSS93fYb2eT4PIxcPpt7BC6cE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=jG5Xsn24mXQYgYBRWNWoV9c71zovaLhFj4aCYz3jkmgFZbCkv8ftR1AFdS1K9Np6r\n\tzvYo24YwMWfFgUKpFH+F8qKExSIJ1MgFmpeCuVm+IL3gxvNhdlARbQY4hQx7/075vp\n\t73iFeyvc+z/TPTVUJ6WJAK0Yzd0RjkXTca1soMU0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"jG5Xsn24\"; dkim-atps=neutral","Date":"Fri, 3 Mar 2023 17:42:33 +0900","To":"Jacopo Mondi ","Message-ID":"","References":"<20230222151917.669526-1-jacopo.mondi@ideasonboard.com>\n\t<20230222151917.669526-4-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20230222151917.669526-4-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] libcamera: rkisp1: Reserve\n\tmain path for StillCapture","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 ","Cc":"libcamera-devel@lists.libcamera.org, libcamera@luigi311.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26574,"web_url":"https://patchwork.libcamera.org/comment/26574/","msgid":"<167817955975.3105479.17551084355737168761@Monstersaurus>","date":"2023-03-07T08:59:19","subject":"Re: [libcamera-devel] [PATCH v2 3/4] libcamera: rkisp1: Reserve\n\tmain path for StillCapture","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 (2023-03-03 08:42:33)\n> On Wed, Feb 22, 2023 at 04:19:16PM +0100, Jacopo Mondi via libcamera-devel wrote:\n> > The main output path can produce images in higher resolution and\n> > should be reserved for the StillCapture role when a configuration\n> > is generated.\n> > \n> > Before this change if StillCapture was not requested first it got\n> > assigned to the self-path and thus down-scaled to 1920x1920.\n> > \n> > With this change, StillCapture can be asked last and it would take\n> > precedence over other streams for the usage of the main path.\n> > \n> > $ cam -c1 --stream role=viewfinder --stream role=still\n> > Camera camera.cpp:969 streams configuration: (0) 1920x1080-NV12 (1) 4208x3120-NV12\n> > \n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 15 ++++++++++++++-\n> > 1 file changed, 14 insertions(+), 1 deletion(-)\n> > \n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 05a7ba03b2d2..23ff859e3835 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -626,6 +626,16 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> > if (roles.empty())\n> > return config;\n> > \n> > + /* If still capture is requested, reserve the main path for it. */\n> > + bool reserveMainPath = false;\n> > + for (const StreamRole role : roles) {\n> > + if (role != StreamRole::StillCapture)\n> > + continue;\n> > +\n> > + reserveMainPath = true;\n> > + break;\n> > + }\n> > +\n> \n> How about...\n> \n> bool reserveMainPath =\n> std::find(roles.cbegin(), roles.cend(),\n> StreamRole::StillCapture) != roles.cend();\n> \n> Which doesn't actually look that nice either. Oh well. Up to you.\n> \n> \n> Reviewed-by: Paul Elder \n\nEither way there should be fine. Pauls way is a bit more concise?\n\n\n\n> \n> > /*\n> > * As the ISP can't output different color spaces for the main and self\n> > * path, pick a sensible default color space based on the role of the\n> > @@ -644,6 +654,9 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> > if (!colorSpace)\n> > colorSpace = ColorSpace::Sycc;\n> > \n> > + /* Unlock usage of main path which was reserved. */\n> > + reserveMainPath = false;\n> > +\n\nIf there were more paths we might need something different here - but I\nbelieve this works from what I see.\n\n\nReviewed-by: Kieran Bingham \n\n> > size = data->sensor_->resolution();\n> > break;\n> > \n> > @@ -685,7 +698,7 @@ PipelineHandlerRkISP1::generateConfiguration(Camera *camera,\n> > \n> > RkISP1Path *path;\n> > \n> > - if (useMainPath) {\n> > + if (useMainPath && !reserveMainPath) {\n> > path = data->mainPath_;\n> > mainPathAvailable = false;\n> > } else {\n> > -- \n> > 2.39.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 B5C9EBE080\n\tfor ;\n\tTue, 7 Mar 2023 08:59:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2FD90626B4;\n\tTue, 7 Mar 2023 09:59:24 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D58516265E\n\tfor ;\n\tTue, 7 Mar 2023 09:59:22 +0100 (CET)","from pendragon.ideasonboard.com\n\t(aztw-30-b2-v4wan-166917-cust845.vm26.cable.virginm.net\n\t[82.37.23.78])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 81DD89A8;\n\tTue, 7 Mar 2023 09:59:22 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1678179564;\n\tbh=/h0ns8TDs0ha/nfr9/DvyoO1+BAk3HBiHQLHLSwZqUc=;\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=3oDmh2ogPsWTa/3XYETrd+RXNYi1kMuAkIDcjx43kymmDr217KlcoQ0ApHVU00cG9\n\tR3+YzkqdmNWSEDibuCQCEGadTEQzdeFlIZNvJu3yY+l/KpP5M2mGhLHlQE1iqFmOrL\n\tPw2TXlDT6xKdWk0spyl0i8ab+6H4sxd2pv3vdXHvBlQmayNbtexo+VJk4YTJRjLtto\n\tIHx5QXlTlrksUjlaGqemBmt5LJIOccJb0hr3I0R8A1Pf85BDcqgsMmlOSYXGXNwUa9\n\ttPW+A7oP8Yusg5nwk3BCShSHxy+pIPkVfIrISeQJUXMMk4d/5OtfVB5CJKNANaYxsA\n\txDjkirj50GHnw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1678179562;\n\tbh=/h0ns8TDs0ha/nfr9/DvyoO1+BAk3HBiHQLHLSwZqUc=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Z6iwIC1Ekeh+efZ7o7vIzw3O88Lvt30y+UauLMUfFHr5NPqKqf+nEC1iVjl+y/lQP\n\tzIGauhqG2GsDNoAuCiCSoeEb/RpZDApZKT/PnAWwBibK2ukIpCpU8rY15JKuRUV5M7\n\tTFC+fcOastiVFHwesz3ampthcya2HUtSpSmLRlSg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Z6iwIC1E\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20230222151917.669526-1-jacopo.mondi@ideasonboard.com>\n\t<20230222151917.669526-4-jacopo.mondi@ideasonboard.com>\n\t","To":"Jacopo Mondi ,\n\tPaul Elder ,\n\tPaul Elder via libcamera-devel ","Date":"Tue, 07 Mar 2023 08:59:19 +0000","Message-ID":"<167817955975.3105479.17551084355737168761@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 3/4] libcamera: rkisp1: Reserve\n\tmain path for StillCapture","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, libcamera@luigi311.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]