[{"id":26670,"web_url":"https://patchwork.libcamera.org/comment/26670/","msgid":"<20230319200238.GG13726@pendragon.ideasonboard.com>","date":"2023-03-19T20:02:38","subject":"Re: [libcamera-devel] [PATCH v3 3/4] libcamera: rkisp1: Reserve\n\tmain path for StillCapture","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nThank you for the patch.\n\nOn Tue, Mar 07, 2023 at 12:48:03PM +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\nThe order in which roles are requested matters, applications must\nspecify them in decreasing order of priority. Why is it better to\npriority the still capture role even if specified last ?\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> Reviewed-by: Paul Elder \n> Reviewed-by: Kieran Bingham \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++++++++-\n> 1 file changed, 8 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 60ab7380034c..cd92e99a50b3 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -643,6 +643,10 @@ 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 = std::find(roles.cbegin(), roles.cend(),\n> +\t\t\t\t\t StreamRole::StillCapture) != roles.cend();\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> @@ -661,6 +665,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> @@ -702,7 +709,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 {","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 7029BC0F1B\n\tfor ;\n\tSun, 19 Mar 2023 20:02:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id EFA5A626D4;\n\tSun, 19 Mar 2023 21:02:38 +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 5F0B4626D4\n\tfor ;\n\tSun, 19 Mar 2023 21:02:37 +0100 (CET)","from pendragon.ideasonboard.com (213-209-177-213.ip.skylogicnet.com\n\t[213.209.177.213])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 01458189A;\n\tSun, 19 Mar 2023 21:02:35 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679256159;\n\tbh=AI6tcNGXAtWIdjaTXfJbE1Wv5/2bDqxhKJksl1Gtols=;\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=ZQ9GfLXD4EbShBrYP7GVhXoPJ9xgUiNrF8VqxklIMI9tHzM9QyOM8QrT6Py1IYSGt\n\tJ+F/D3zena36zOH8Q5/1YP2PdMmxj9rO6STyUrJCbfeZolyyKRb8a0raq11yjMgfAv\n\tRvFRAtPQ8xCaJLEjt4/FeaEIe8J4A1//5/Bjin0hPAi9kn6q2ik9ngKnRvmRVcwHuE\n\tL6Mpldm90oh+TdzK2leac+V2rSp0HnCHuZ/8rqF5dowf0hz6tfAIb121w2tvuvni8o\n\tsGA2UyvJjtIvMCY6xIUU6Aaso1FTGEa+r2XApiIVhvnuJtJifBH2Fh5nb/M1WL4Bpm\n\ttCEsXal5wV10g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679256157;\n\tbh=AI6tcNGXAtWIdjaTXfJbE1Wv5/2bDqxhKJksl1Gtols=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Giu2C1bMxDfe3axZJgRwPcohiPH6hzHyxujhjG7sAw8zn3NEhqyS+8BTbR1Mqtqyv\n\t0azHjFMqcWDt7T/vOYlRMNLvvYhioYl7YE6pI3BNAzXvErC0SFj5JcZ4UvsJQVMJFA\n\tzn5H88xOlUc9m7rxn6UAiPuO5bO0xVlYom2upQgY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Giu2C1bM\"; dkim-atps=neutral","Date":"Sun, 19 Mar 2023 22:02:38 +0200","To":"Jacopo Mondi ","Message-ID":"<20230319200238.GG13726@pendragon.ideasonboard.com>","References":"<20230307114804.42291-1-jacopo.mondi@ideasonboard.com>\n\t<20230307114804.42291-4-jacopo.mondi@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230307114804.42291-4-jacopo.mondi@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, libcamera@luigi311.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26675,"web_url":"https://patchwork.libcamera.org/comment/26675/","msgid":"<20230320072725.v7mcor6kcrqgxm3s@uno.localdomain>","date":"2023-03-20T07:27:25","subject":"Re: [libcamera-devel] [PATCH v3 3/4] libcamera: rkisp1: Reserve\n\tmain path for StillCapture","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Laurent\n\nOn Sun, Mar 19, 2023 at 10:02:38PM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> Thank you for the patch.\n>\n> On Tue, Mar 07, 2023 at 12:48:03PM +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> The order in which roles are requested matters, applications must\n> specify them in decreasing order of priority. Why is it better to\n> priority the still capture role even if specified last ?\n>\n> > $ cam -c1 --stream role=viewfinder --stream role=still\n\nIt is not about prioritizing it, it's about being capable of\nsupporting a configuration that can the hw support and we currently\nfail to implement.\n\nRegardless of the roles order, the platform can do\n\n { vf: 1080p; still: full-res }\n\nI don't see a reason why we should expose to applications a\nplatform-specific constraint such as the fact that the secondary\noutput is limited in res compard to the first one, which requires them to\ntake that into account when configuring roles.\n\nTL;DR application should be able to get the same config with\n[vf; still] and [still; vf] as the hw is capable of doing that, it\nsimply was not implemented.\n\n> > Camera camera.cpp:969 streams configuration: (0) 1920x1080-NV12 (1) 4208x3120-NV12\n> >\n> > Signed-off-by: Jacopo Mondi \n> > Reviewed-by: Paul Elder \n> > Reviewed-by: Kieran Bingham \n> > ---\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++++++++-\n> > 1 file changed, 8 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 60ab7380034c..cd92e99a50b3 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -643,6 +643,10 @@ 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 = std::find(roles.cbegin(), roles.cend(),\n> > +\t\t\t\t\t StreamRole::StillCapture) != roles.cend();\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> > @@ -661,6 +665,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> > @@ -702,7 +709,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> --\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 6243CBD80A\n\tfor ;\n\tMon, 20 Mar 2023 07:27:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2C883626DB;\n\tMon, 20 Mar 2023 08:27:30 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 89BAA603A9\n\tfor ;\n\tMon, 20 Mar 2023 08:27:28 +0100 (CET)","from ideasonboard.com (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0CE6C1373;\n\tMon, 20 Mar 2023 08:27:27 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679297250;\n\tbh=rJuQd1S/+nS4FqqaeAhckDKc0U0QKoX/E3iIvWGkCn4=;\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=Ro1ubGBHh5OtAMao5W4JN80BqxJ+l2V/FkjrwQ4a6pEhdCO7BS7Z6MMqHsqNplAX5\n\tOt0PUVSw8F9NUxX7SxO3ffwsw9/557y5KxbIpYLnpCBS34qh+Dpa03SAl8aUV2ZIvq\n\t8lRlQtS0FgBz7T5e3yYd5DZx2fw0dOCmuoBZjAfF9gk8VnBaw2zJg/I+JJtP2UiugA\n\txPRQZQfUiLXzdn/EEdepeNQxOOCp7azd0B6v7Yzf8Ob8n46UvCO6SGEeZFmft+4oOg\n\t+RCke2HuIFTiDELoZm1wKAIEqt3GHADLu6cizKSonqlle9sZnZI6L5F5l6DBVOjHMO\n\tRvC4mJLX784Rw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679297248;\n\tbh=rJuQd1S/+nS4FqqaeAhckDKc0U0QKoX/E3iIvWGkCn4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=VzTD6yOKZa/roLZcHaq2C3DRiIF5Tf/cOEWCH4M87yZqY2v82hZ1qaD47kaWF2uv/\n\taHvsCjUrSAGSLFOVJD1Xl1W++uALULRuKM768cv5IK0XFQnpoVHKyuc/NztQ4e9vuP\n\tszGSW/R7yIFa/BJGMftHYTm1esh958NlOvP90uaw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"VzTD6yOK\"; dkim-atps=neutral","Date":"Mon, 20 Mar 2023 08:27:25 +0100","To":"Laurent Pinchart ","Message-ID":"<20230320072725.v7mcor6kcrqgxm3s@uno.localdomain>","References":"<20230307114804.42291-1-jacopo.mondi@ideasonboard.com>\n\t<20230307114804.42291-4-jacopo.mondi@ideasonboard.com>\n\t<20230319200238.GG13726@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230319200238.GG13726@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 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":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org, libcamera@luigi311.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26685,"web_url":"https://patchwork.libcamera.org/comment/26685/","msgid":"<20230320233146.GT20234@pendragon.ideasonboard.com>","date":"2023-03-20T23:31:46","subject":"Re: [libcamera-devel] [PATCH v3 3/4] libcamera: rkisp1: Reserve\n\tmain path for StillCapture","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Jacopo,\n\nOn Mon, Mar 20, 2023 at 08:27:25AM +0100, Jacopo Mondi wrote:\n> On Sun, Mar 19, 2023 at 10:02:38PM +0200, Laurent Pinchart wrote:\n> > On Tue, Mar 07, 2023 at 12:48:03PM +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> > The order in which roles are requested matters, applications must\n> > specify them in decreasing order of priority. Why is it better to\n> > priority the still capture role even if specified last ?\n> >\n> > > $ cam -c1 --stream role=viewfinder --stream role=still\n> \n> It is not about prioritizing it, it's about being capable of\n> supporting a configuration that can the hw support and we currently\n> fail to implement.\n> \n> Regardless of the roles order, the platform can do\n> \n> { vf: 1080p; still: full-res }\n> \n> I don't see a reason why we should expose to applications a\n> platform-specific constraint such as the fact that the secondary\n> output is limited in res compard to the first one, which requires them to\n> take that into account when configuring roles.\n> \n> TL;DR application should be able to get the same config with\n> [vf; still] and [still; vf] as the hw is capable of doing that, it\n> simply was not implemented.\n\nWhat if the application wants to capture the 1080p resolution in an RGB\nformat ? If you always assign the selfpath to the viewfinder when a\nstill image capture role is specified, that won't be possible. It may\neven be that the application would like to capture still images in a\ndownscaled resolution (supported by the selfpath) in YUV format, and use\nan RGB format for CPU processing. The hardware supports that, but with\nthis patch it won't be possible using the generate configuration.\n\n> > > Camera camera.cpp:969 streams configuration: (0) 1920x1080-NV12 (1) 4208x3120-NV12\n> > >\n> > > Signed-off-by: Jacopo Mondi \n> > > Reviewed-by: Paul Elder \n> > > Reviewed-by: Kieran Bingham \n> > > ---\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++++++++-\n> > > 1 file changed, 8 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 60ab7380034c..cd92e99a50b3 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -643,6 +643,10 @@ 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 = std::find(roles.cbegin(), roles.cend(),\n> > > +\t\t\t\t\t StreamRole::StillCapture) != roles.cend();\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> > > @@ -661,6 +665,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> > > @@ -702,7 +709,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 {","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 6F16BC0F1B\n\tfor ;\n\tMon, 20 Mar 2023 23:31:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8DAD8626DA;\n\tTue, 21 Mar 2023 00:31: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 0675E626CA\n\tfor ;\n\tTue, 21 Mar 2023 00:31:41 +0100 (CET)","from pendragon.ideasonboard.com (213-243-189-158.bb.dnainternet.fi\n\t[213.243.189.158])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6C4CE496;\n\tTue, 21 Mar 2023 00:31:40 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679355102;\n\tbh=Sq2Ll2Fh7qv1dko8C2fuapoCObf6oJ3JiLTZGvhMytc=;\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=cXtyAOyFYmLzkrbrIcjWvzSG7jOUGQ9i+EjharFrlEiuOwqLYUNYWIv+jrcZixgdj\n\tOxSmmsVyx8MihFDghVNj4e6srtxQYj7Gi0IJ1XPxfsnWLtp3TGrcRG/9SKhbLrJe2z\n\tMr6dA/LY7DaudWz5c9MDQfAKahxlyA9FUZa8AxjUfIcDFWF93YXdCtFHXiFdGgHH7l\n\tw9XGz0flIHsQhE7OaeOjUG/FFrEKCefEa8sfaO7y3viaGhqZCl01sKsBhRiLdS7JK7\n\tC/JxL9wNhVCXnZ9BeQB+NX58olszFsBXzrquP5bgevaY4uIqAOPsjp5tS36cSlWcBU\n\tGgFcZwhpuMFQA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679355100;\n\tbh=Sq2Ll2Fh7qv1dko8C2fuapoCObf6oJ3JiLTZGvhMytc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=qUWUCW60B7PETB+rBhwYSIKzvNfQeqneTXKZR2+xwZDYMEQClVLYFXsn1ekeicoQp\n\tdE1XyJQnevkLMlWwmkrrRY40mNCdpPSw15G/HASty2yZgRrGnkMukJfnes5cV/xY5V\n\tDnXww5SP56rYb6Ml8D7NgcGqPySA/IlCrUD5s2Nk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"qUWUCW60\"; dkim-atps=neutral","Date":"Tue, 21 Mar 2023 01:31:46 +0200","To":"Jacopo Mondi ","Message-ID":"<20230320233146.GT20234@pendragon.ideasonboard.com>","References":"<20230307114804.42291-1-jacopo.mondi@ideasonboard.com>\n\t<20230307114804.42291-4-jacopo.mondi@ideasonboard.com>\n\t<20230319200238.GG13726@pendragon.ideasonboard.com>\n\t<20230320072725.v7mcor6kcrqgxm3s@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230320072725.v7mcor6kcrqgxm3s@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v3 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":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org, libcamera@luigi311.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26696,"web_url":"https://patchwork.libcamera.org/comment/26696/","msgid":"<20230321082211.6kd3s7rhlg6xegi5@uno.localdomain>","date":"2023-03-21T08:22:11","subject":"Re: [libcamera-devel] [PATCH v3 3/4] libcamera: rkisp1: Reserve\n\tmain path for StillCapture","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Laurent\n\nOn Tue, Mar 21, 2023 at 01:31:46AM +0200, Laurent Pinchart wrote:\n> Hi Jacopo,\n>\n> On Mon, Mar 20, 2023 at 08:27:25AM +0100, Jacopo Mondi wrote:\n> > On Sun, Mar 19, 2023 at 10:02:38PM +0200, Laurent Pinchart wrote:\n> > > On Tue, Mar 07, 2023 at 12:48:03PM +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> > > The order in which roles are requested matters, applications must\n> > > specify them in decreasing order of priority. Why is it better to\n> > > priority the still capture role even if specified last ?\n> > >\n> > > > $ cam -c1 --stream role=viewfinder --stream role=still\n> >\n> > It is not about prioritizing it, it's about being capable of\n> > supporting a configuration that can the hw support and we currently\n> > fail to implement.\n> >\n> > Regardless of the roles order, the platform can do\n> >\n> > { vf: 1080p; still: full-res }\n> >\n> > I don't see a reason why we should expose to applications a\n> > platform-specific constraint such as the fact that the secondary\n> > output is limited in res compard to the first one, which requires them to\n> > take that into account when configuring roles.\n> >\n> > TL;DR application should be able to get the same config with\n> > [vf; still] and [still; vf] as the hw is capable of doing that, it\n> > simply was not implemented.\n>\n> What if the application wants to capture the 1080p resolution in an RGB\n> format ? If you always assign the selfpath to the viewfinder when a\n\nI guess we can go on the whole day listing use cases that can or\ncannot be achived with or without this patch.\n\n> still image capture role is specified, that won't be possible. It may\n> even be that the application would like to capture still images in a\n> downscaled resolution (supported by the selfpath) in YUV format, and use\n> an RGB format for CPU processing. The hardware supports that, but with\n> this patch it won't be possible using the generate configuration.\n>\n\nLet's drop this patch and 3/4\n\n> > > > Camera camera.cpp:969 streams configuration: (0) 1920x1080-NV12 (1) 4208x3120-NV12\n> > > >\n> > > > Signed-off-by: Jacopo Mondi \n> > > > Reviewed-by: Paul Elder \n> > > > Reviewed-by: Kieran Bingham \n> > > > ---\n> > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++++++++-\n> > > > 1 file changed, 8 insertions(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > index 60ab7380034c..cd92e99a50b3 100644\n> > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > @@ -643,6 +643,10 @@ 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 = std::find(roles.cbegin(), roles.cend(),\n> > > > +\t\t\t\t\t StreamRole::StillCapture) != roles.cend();\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> > > > @@ -661,6 +665,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> > > > @@ -702,7 +709,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> --\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 D361CC0F1B\n\tfor ;\n\tTue, 21 Mar 2023 08:22:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id ED7C4626E5;\n\tTue, 21 Mar 2023 09:22:16 +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 AE883626D8\n\tfor ;\n\tTue, 21 Mar 2023 09:22:15 +0100 (CET)","from ideasonboard.com (host-87-18-61-243.retail.telecomitalia.it\n\t[87.18.61.243])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 03851496;\n\tTue, 21 Mar 2023 09:22:14 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679386937;\n\tbh=dEBFNEfNEOwEwvS4U1HUsYJRDRSHAXSjQ2YnksNtxME=;\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=EIzbqXWS5GM/6FYGTNx9n/o/LAVKYaV33tb3qoikMvlLcp+obzVKNU5WbtpfBBnwX\n\tX3UYFFrPUlHn4sqiy9J2INzL9WFj3txnmD2R0e9ZvFqhe67GIDoHaKKg9PBPu3l3/y\n\tHKF7Ok3+1EkooLL2U6BNtp+nIhIN20IRFsPoMzpiG69VTNDQ/cQW2ro3dAfAWr7whJ\n\t16ZaQ6qtzNDAA5DK7gHxaYSmi0eDCZJWX4vF580Al5AzDtog+kIKwgpLUAAAMuoM4A\n\t62uAuVvTE7bW9U0BCdMutad2MPowK+8Yo4SOd4zWHYDdBNIVVp1CEIuPNgTq8eJifg\n\tpm7/WQ6LaRQsQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679386935;\n\tbh=dEBFNEfNEOwEwvS4U1HUsYJRDRSHAXSjQ2YnksNtxME=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=t1P3sabmm2rkZ1i1yX5voKk4EYoov9tnC9FXpjN1BIcKrfxXtd0uuQSfEXR2+WmxV\n\tzNDvq/bE03jGJXGOtDMWbXqhE3IAk5syzdViehRuBlkdstptIT24/kUu/k1lePFaWu\n\t4dmVv8KaY9oMOAPt6KFLPvb2gNDDvkxIzWOHDmt4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"t1P3sabm\"; dkim-atps=neutral","Date":"Tue, 21 Mar 2023 09:22:11 +0100","To":"Laurent Pinchart ","Message-ID":"<20230321082211.6kd3s7rhlg6xegi5@uno.localdomain>","References":"<20230307114804.42291-1-jacopo.mondi@ideasonboard.com>\n\t<20230307114804.42291-4-jacopo.mondi@ideasonboard.com>\n\t<20230319200238.GG13726@pendragon.ideasonboard.com>\n\t<20230320072725.v7mcor6kcrqgxm3s@uno.localdomain>\n\t<20230320233146.GT20234@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230320233146.GT20234@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 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":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Jacopo Mondi ,\n\tlibcamera-devel@lists.libcamera.org, libcamera@luigi311.com","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]