[{"id":26738,"web_url":"https://patchwork.libcamera.org/comment/26738/","msgid":"<20230324154207.as5slmewwirc4kru@uno.localdomain>","date":"2023-03-24T15:42:07","subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi daniel\n\nOn Fri, Mar 24, 2023 at 03:29:06PM +0100, Daniel Semkowicz via libcamera-devel wrote:\n> Add controls supported by the AF algorithm to the list of controls\n> supported by the RkISP1 IPA. This exposes the AF controls to the user\n> and allows controlling the AF algorithm using the top level API.\n>\n> Signed-off-by: Daniel Semkowicz \n> ---\n> src/ipa/rkisp1/rkisp1.cpp | 6 ++++++\n> 1 file changed, 6 insertions(+)\n>\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index cd1fbae3..4b30844f 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -101,10 +101,16 @@ namespace {\n> /* List of controls handled by the RkISP1 IPA */\n> const ControlInfoMap::Map rkisp1Controls{\n> \t{ &controls::AeEnable, ControlInfo(false, true) },\n> +\t{ &controls::AfMetering, ControlInfo(controls::AfMeteringValues) },\n> +\t{ &controls::AfMode, ControlInfo(controls::AfModeValues) },\n\nAfPauseDeferred is not supported\n\nOtherwise\nReviewed-by: Jacopo Mondi \n\n> +\t{ &controls::AfPause, ControlInfo(controls::AfPauseValues) },\n> +\t{ &controls::AfTrigger, ControlInfo(controls::AfTriggerValues) },\n> +\t{ &controls::AfWindows, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n\nWe should -really- move to instantiate control limits directly based\non the sensor configuration...\n\n> \t{ &controls::AwbEnable, ControlInfo(false, true) },\n> \t{ &controls::ColourGains, ControlInfo(0.0f, 3.996f, 1.0f) },\n> \t{ &controls::Brightness, ControlInfo(-1.0f, 0.993f, 0.0f) },\n> \t{ &controls::Contrast, ControlInfo(0.0f, 1.993f, 1.0f) },\n> +\t{ &controls::LensPosition, ControlInfo(0.0f, 2147483647.0f) },\n> \t{ &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },\n> \t{ &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },\n> \t{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n> --\n> 2.39.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 EC457C0F2A\n\tfor ;\n\tFri, 24 Mar 2023 15:42:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 32ED162719;\n\tFri, 24 Mar 2023 16:42:11 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 565B261ECF\n\tfor ;\n\tFri, 24 Mar 2023 16:42:10 +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 E9955A58;\n\tFri, 24 Mar 2023 16:42:09 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679672531;\n\tbh=XjH4g+h0T5hT/WDgfVqZ7pAY0eqDTjJ3dTbgGNjuJgI=;\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=EYUyWJv1+wAhIlCsXBDHsVTLCxD2a+OOCsBDft5snQ4jfRwuUx8mZGnj436tGvgQR\n\tuPy6Ty+zXdUYFk+vFewTgHW7wYmpzWKcTZPdmoS12NrSIZ52EkdRm8jWB8BSJJ6EUM\n\ti3NLnh4UMxbar67NGOka4KQtWFNE0qf8bxOeroTm6q3LtnxacKwKULyuR4dGW/WTJU\n\t3qG/X4bWEXo0w0zQaS7dwQcQrpQDxbO76d4nPirCf6GI640MObKVB6ZsbfkTnJNS5Z\n\thJHyhs4zZiKhllWGIdCh/ZOQNSp5smTv7Nb/t6RmJ6eC71BvWil6VNntVPgG/HZNLm\n\t1W5yshfZ4cyvQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679672530;\n\tbh=XjH4g+h0T5hT/WDgfVqZ7pAY0eqDTjJ3dTbgGNjuJgI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZRJQFITyQZx7ZKvYtatNmDUXgllRAdRnL7xZ2/U8jrHTm3Mf9IcRArGXnJ7mHuWLf\n\tG3dn0WvgvD7iyPZ+DmbcPH6LUs4Wo19NuJ8ejwiGt7ZW/+FetrgUgQYwuiNbFS1SSs\n\tNxKbu34re2NoQWpUlTtv5Dv5EEsUPSVzovQeNymY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ZRJQFITy\"; dkim-atps=neutral","Date":"Fri, 24 Mar 2023 16:42:07 +0100","To":"Daniel Semkowicz ","Message-ID":"<20230324154207.as5slmewwirc4kru@uno.localdomain>","References":"<20230324142908.64224-1-dse@thaumatec.com>\n\t<20230324142908.64224-9-dse@thaumatec.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230324142908.64224-9-dse@thaumatec.com>","Subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","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@ideasonboard.com, libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26741,"web_url":"https://patchwork.libcamera.org/comment/26741/","msgid":"<170c3b0e-fd16-5b83-4419-e9eab8c29e5f@ideasonboard.com>","date":"2023-03-24T15:47:53","subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 24/03/2023 15:42, Jacopo Mondi via libcamera-devel wrote:\n> Hi daniel\n> \n> On Fri, Mar 24, 2023 at 03:29:06PM +0100, Daniel Semkowicz via libcamera-devel wrote:\n>> Add controls supported by the AF algorithm to the list of controls\n>> supported by the RkISP1 IPA. This exposes the AF controls to the user\n>> and allows controlling the AF algorithm using the top level API.\n>>\n>> Signed-off-by: Daniel Semkowicz \n>> ---\n>> src/ipa/rkisp1/rkisp1.cpp | 6 ++++++\n>> 1 file changed, 6 insertions(+)\n>>\n>> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n>> index cd1fbae3..4b30844f 100644\n>> --- a/src/ipa/rkisp1/rkisp1.cpp\n>> +++ b/src/ipa/rkisp1/rkisp1.cpp\n>> @@ -101,10 +101,16 @@ namespace {\n>> /* List of controls handled by the RkISP1 IPA */\n>> const ControlInfoMap::Map rkisp1Controls{\n>> \t{ &controls::AeEnable, ControlInfo(false, true) },\n>> +\t{ &controls::AfMetering, ControlInfo(controls::AfMeteringValues) },\n>> +\t{ &controls::AfMode, ControlInfo(controls::AfModeValues) },\n> \n> AfPauseDeferred is not supported\n> \n\nWhat happens if there's no VCM though - Should we only expose the \ncontrols if a VCM is identified? (Though that requires reworking the \ncreation of this rkisp1Controls list to be handled by the algorithms)\n\n--\nKieran\n\n\n> Otherwise\n> Reviewed-by: Jacopo Mondi \n> \n>> +\t{ &controls::AfPause, ControlInfo(controls::AfPauseValues) },\n>> +\t{ &controls::AfTrigger, ControlInfo(controls::AfTriggerValues) },\n>> +\t{ &controls::AfWindows, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> \n> We should -really- move to instantiate control limits directly based\n> on the sensor configuration...\n> \n>> \t{ &controls::AwbEnable, ControlInfo(false, true) },\n>> \t{ &controls::ColourGains, ControlInfo(0.0f, 3.996f, 1.0f) },\n>> \t{ &controls::Brightness, ControlInfo(-1.0f, 0.993f, 0.0f) },\n>> \t{ &controls::Contrast, ControlInfo(0.0f, 1.993f, 1.0f) },\n>> +\t{ &controls::LensPosition, ControlInfo(0.0f, 2147483647.0f) },\n>> \t{ &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },\n>> \t{ &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },\n>> \t{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n>> --\n>> 2.39.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 31A82C0F1B\n\tfor ;\n\tFri, 24 Mar 2023 15:47:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5648A62715;\n\tFri, 24 Mar 2023 16:47:56 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A361C603AA\n\tfor ;\n\tFri, 24 Mar 2023 16:47:54 +0100 (CET)","from [192.168.0.32]\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 3741CA58;\n\tFri, 24 Mar 2023 16:47:54 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679672876;\n\tbh=22fxhwy4Rou2rfS4Y5WLCphOuNMQNwUs6tTgIq0DEvc=;\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=sH/END+NTNqnBU4C00sxQ92u3cXW9awxwkIRTmYPfPSkT4h7RHyuRelacFhG4FVl2\n\t+lG3YSG/evlyNBAp508IeAce8a2vmG7osiX4ICs8wAU0wjyLFBU8Ml1tFZIdYJF70M\n\tbSAfH7EzZVN0WMRJwFEPOXNAeNvlg4i5NZSIAN6smfKsq370pyvT/9y0T8FqOm+VTc\n\tNMuDPatvhSCF+swLsgWJ7lLOMVx7VOuJR/ORbCpHiIPNToLoUZpjL8wDb+hNQm5k63\n\tUCXtTO5ygGQNje1eyJbFWJv8bw+5Xp7x3qERQ1Yk758QokABkriDgvW7gU8IeKttWm\n\tVno2NPVeKypIg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679672874;\n\tbh=22fxhwy4Rou2rfS4Y5WLCphOuNMQNwUs6tTgIq0DEvc=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=N2eBa17Tc6uByeYE94k5Mmd0UF9ipW3v3kRtbNmbq6JIoHtuPHcz2vbEOEzdc5NRd\n\tapC/dKLxEvE57vbH82IMhsE1RQS2sIwv78DuCd8B6MuJtlcvX1DUGS4qc/i3pbkKWO\n\te0UhmXdCASMJuZjYZo9GG/ySjCJlI/Yx9e0g3SXI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"N2eBa17T\"; dkim-atps=neutral","Message-ID":"<170c3b0e-fd16-5b83-4419-e9eab8c29e5f@ideasonboard.com>","Date":"Fri, 24 Mar 2023 15:47:53 +0000","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101\n\tThunderbird/102.8.0","Content-Language":"en-US","To":"Jacopo Mondi ,\n\tDaniel Semkowicz ","References":"<20230324142908.64224-1-dse@thaumatec.com>\n\t<20230324142908.64224-9-dse@thaumatec.com>\n\t<20230324154207.as5slmewwirc4kru@uno.localdomain>","In-Reply-To":"<20230324154207.as5slmewwirc4kru@uno.localdomain>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26747,"web_url":"https://patchwork.libcamera.org/comment/26747/","msgid":"<20230325092729.ncsz2mcfqihnijl5@uno.localdomain>","date":"2023-03-25T09:27:29","subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Kieran\n\nOn Fri, Mar 24, 2023 at 03:47:53PM +0000, Kieran Bingham wrote:\n>\n>\n> On 24/03/2023 15:42, Jacopo Mondi via libcamera-devel wrote:\n> > Hi daniel\n> >\n> > On Fri, Mar 24, 2023 at 03:29:06PM +0100, Daniel Semkowicz via libcamera-devel wrote:\n> > > Add controls supported by the AF algorithm to the list of controls\n> > > supported by the RkISP1 IPA. This exposes the AF controls to the user\n> > > and allows controlling the AF algorithm using the top level API.\n> > >\n> > > Signed-off-by: Daniel Semkowicz \n> > > ---\n> > > src/ipa/rkisp1/rkisp1.cpp | 6 ++++++\n> > > 1 file changed, 6 insertions(+)\n> > >\n> > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > index cd1fbae3..4b30844f 100644\n> > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > @@ -101,10 +101,16 @@ namespace {\n> > > /* List of controls handled by the RkISP1 IPA */\n> > > const ControlInfoMap::Map rkisp1Controls{\n> > > \t{ &controls::AeEnable, ControlInfo(false, true) },\n> > > +\t{ &controls::AfMetering, ControlInfo(controls::AfMeteringValues) },\n> > > +\t{ &controls::AfMode, ControlInfo(controls::AfModeValues) },\n> >\n> > AfPauseDeferred is not supported\n> >\n>\n> What happens if there's no VCM though - Should we only expose the controls\n\nYou're very right, I overlooked that.\n\n> if a VCM is identified? (Though that requires reworking the creation of this\n> rkisp1Controls list to be handled by the algorithms)\n\nI'm not sure it needs to be done by algorithms, but the IPA should\ncertainly be made aware of the lens presence at init() time.\n\nTo be honest I would be fine with what RPi does in this regard. have a\nlook at the usage of lensPresent_ in their IPA module init() function.\nThis looks like the less invasive change.\n\nI presume this could even be made better by passing the lens controls'\nfrom the PH to the IPA at init() time so that the AF ControlInfo\nelements could be initialized with the proper limits and not with\ndefault values as it happens here and on RPi.\n\n>\n> --\n> Kieran\n>\n>\n> > Otherwise\n> > Reviewed-by: Jacopo Mondi \n> >\n> > > +\t{ &controls::AfPause, ControlInfo(controls::AfPauseValues) },\n> > > +\t{ &controls::AfTrigger, ControlInfo(controls::AfTriggerValues) },\n> > > +\t{ &controls::AfWindows, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> >\n> > We should -really- move to instantiate control limits directly based\n> > on the sensor configuration...\n> >\n> > > \t{ &controls::AwbEnable, ControlInfo(false, true) },\n> > > \t{ &controls::ColourGains, ControlInfo(0.0f, 3.996f, 1.0f) },\n> > > \t{ &controls::Brightness, ControlInfo(-1.0f, 0.993f, 0.0f) },\n> > > \t{ &controls::Contrast, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > > +\t{ &controls::LensPosition, ControlInfo(0.0f, 2147483647.0f) },\n> > > \t{ &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > > \t{ &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },\n> > > \t{ &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n> > > --\n> > > 2.39.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 DA20DC0F1B\n\tfor ;\n\tSat, 25 Mar 2023 09:27:34 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3223362719;\n\tSat, 25 Mar 2023 10:27:34 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DE7C4626DA\n\tfor ;\n\tSat, 25 Mar 2023 10:27:32 +0100 (CET)","from ideasonboard.com\n\t(host-213-45-201-116.pool21345.interbusiness.it [213.45.201.116])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 30C4E9A8;\n\tSat, 25 Mar 2023 10:27:32 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679736454;\n\tbh=U32kc6ZE8p20RNDC7LBQU3XVxFIoZpSLVoYn8SMuQBU=;\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=T9FHq3sXL3OXhYRAzPY+0NP4EfMlh7u2OqIcy71zpb5HBhILkUxpYHKdoYrZgIcXq\n\tvfH8rwMPDBdtcVpPLkPdsyyS5pD2xc5Eh+xhlQGAN4gGHlZkdwdn97vaFB8PDBx/Kd\n\trE50VvaaLklv3ywQ2FZ3/4FuRmnD5ADJqUzEkJ0oCUdQf3EMMPZjBksy0LjkwAA6Hj\n\tK7dUKdDRiQHbr3TC3BIx4UBL8ayDtrMnKPtZ+WNsZUMxced+yKmgTiY4QwL8/xHhHb\n\t/XUUPC9y/ujEpMNhs66yyGkp8vjDDgkFK6fzJ4foHdBeZVDR2JdgufmF+8HJn3DdHV\n\tfy5N0Xs7YqCig==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679736452;\n\tbh=U32kc6ZE8p20RNDC7LBQU3XVxFIoZpSLVoYn8SMuQBU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=WkRoInOQSAGKZMr/1eEUYj6EtRLuXo1DL5WvOAoWFsfVzWRV5GP4xBpEIQqXtslw+\n\tUJ0Ao55yJ2QrIWkweLzlW5kyf8/YEVcFlTqGAXXRrI/RYG1NfXS8mOMhtQ5tvqfsw5\n\tSJ4T9kYZRcfEbHq4XTTs9Iv4gnfz6OFE1uw7G7ps="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"WkRoInOQ\"; dkim-atps=neutral","Date":"Sat, 25 Mar 2023 10:27:29 +0100","To":"Kieran Bingham ","Message-ID":"<20230325092729.ncsz2mcfqihnijl5@uno.localdomain>","References":"<20230324142908.64224-1-dse@thaumatec.com>\n\t<20230324142908.64224-9-dse@thaumatec.com>\n\t<20230324154207.as5slmewwirc4kru@uno.localdomain>\n\t<170c3b0e-fd16-5b83-4419-e9eab8c29e5f@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<170c3b0e-fd16-5b83-4419-e9eab8c29e5f@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26756,"web_url":"https://patchwork.libcamera.org/comment/26756/","msgid":"","date":"2023-03-27T10:13:51","subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","submitter":{"id":126,"url":"https://patchwork.libcamera.org/api/people/126/","name":"Daniel Semkowicz","email":"dse@thaumatec.com"},"content":"Hi Jacopo, Hi Kieran,\n\nOn Sat, Mar 25, 2023 at 10:27 AM Jacopo Mondi\n wrote:\n>\n> Hi Kieran\n>\n> On Fri, Mar 24, 2023 at 03:47:53PM +0000, Kieran Bingham wrote:\n> >\n> >\n> > On 24/03/2023 15:42, Jacopo Mondi via libcamera-devel wrote:\n> > > Hi daniel\n> > >\n> > > On Fri, Mar 24, 2023 at 03:29:06PM +0100, Daniel Semkowicz via libcamera-devel wrote:\n> > > > Add controls supported by the AF algorithm to the list of controls\n> > > > supported by the RkISP1 IPA. This exposes the AF controls to the user\n> > > > and allows controlling the AF algorithm using the top level API.\n> > > >\n> > > > Signed-off-by: Daniel Semkowicz \n> > > > ---\n> > > > src/ipa/rkisp1/rkisp1.cpp | 6 ++++++\n> > > > 1 file changed, 6 insertions(+)\n> > > >\n> > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > > index cd1fbae3..4b30844f 100644\n> > > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > > @@ -101,10 +101,16 @@ namespace {\n> > > > /* List of controls handled by the RkISP1 IPA */\n> > > > const ControlInfoMap::Map rkisp1Controls{\n> > > > { &controls::AeEnable, ControlInfo(false, true) },\n> > > > + { &controls::AfMetering, ControlInfo(controls::AfMeteringValues) },\n> > > > + { &controls::AfMode, ControlInfo(controls::AfModeValues) },\n> > >\n> > > AfPauseDeferred is not supported\n> > >\n> >\n> > What happens if there's no VCM though - Should we only expose the controls\n>\n> You're very right, I overlooked that.\n>\n> > if a VCM is identified? (Though that requires reworking the creation of this\n> > rkisp1Controls list to be handled by the algorithms)\n>\n> I'm not sure it needs to be done by algorithms, but the IPA should\n> certainly be made aware of the lens presence at init() time.\n\nThis is indeed something that is needed, especially that number\nof algorithm implementations is increasing. The most appropriate\nsolution seems for the algorithms to expose the supported control list\nor to populate the list. This would also fix the problem of checking\noutside the algorithm, if certain configuration is available, like\nlens.\n\nI see at least two problems with the current approach:\n1. controls are exposed by the IPA, even if the algorithm is not\n enabled in the tuning file. This is confusing.\n2. What if there are, for example, two AF algorithm implementations,\n with different supported controls? Which controls should be added\n to the control list?\n\n>\n> To be honest I would be fine with what RPi does in this regard. have a\n> look at the usage of lensPresent_ in their IPA module init() function.\n> This looks like the less invasive change.\n>\n> I presume this could even be made better by passing the lens controls'\n> from the PH to the IPA at init() time so that the AF ControlInfo\n> elements could be initialized with the proper limits and not with\n> default values as it happens here and on RPi.\n\nFor me, moving it to init() is also a better solution. I would like to\navoid intermediate steps and initialize IPA lens config in one place\nfor clarity. This means the whole lens configuration that's currently\ndone in the configure(), will need to be moved to init().\nHowever, I see one problem. context_.configuration struct is reset\nin configure() for some reason. Even that\ncontext_.configuration.sensor.lineDuration is set in init(). It is\nreset and set again.\n\nWhat about making the IPASessionConfiguration::lens structure\nstd::optional, so it will be explicitly stated in the session config\nif the lens is available? Then, AF can fail in init() if lens\nare not available.\n\nThis will work for lens, but I am afraid We will not be able to\nevaluate all AF ControlInfo elements in the init() phase.\nFor example, AF window size limits depend on the sensor output size,\nwhich is only available in the configure().\n\nBest regards\nDaniel\n\n>\n> >\n> > --\n> > Kieran\n> >\n> >\n> > > Otherwise\n> > > Reviewed-by: Jacopo Mondi \n> > >\n> > > > + { &controls::AfPause, ControlInfo(controls::AfPauseValues) },\n> > > > + { &controls::AfTrigger, ControlInfo(controls::AfTriggerValues) },\n> > > > + { &controls::AfWindows, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > >\n> > > We should -really- move to instantiate control limits directly based\n> > > on the sensor configuration...\n> > >\n> > > > { &controls::AwbEnable, ControlInfo(false, true) },\n> > > > { &controls::ColourGains, ControlInfo(0.0f, 3.996f, 1.0f) },\n> > > > { &controls::Brightness, ControlInfo(-1.0f, 0.993f, 0.0f) },\n> > > > { &controls::Contrast, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > > > + { &controls::LensPosition, ControlInfo(0.0f, 2147483647.0f) },\n> > > > { &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > > > { &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },\n> > > > { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n> > > > --\n> > > > 2.39.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 26890BD160\n\tfor ;\n\tMon, 27 Mar 2023 10:14:06 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8C48D6271C;\n\tMon, 27 Mar 2023 12:14:05 +0200 (CEST)","from mail-ed1-x531.google.com (mail-ed1-x531.google.com\n\t[IPv6:2a00:1450:4864:20::531])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 26F1C626D7\n\tfor ;\n\tMon, 27 Mar 2023 12:14:03 +0200 (CEST)","by mail-ed1-x531.google.com with SMTP id h8so33741375ede.8\n\tfor ;\n\tMon, 27 Mar 2023 03:14:03 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679912045;\n\tbh=fHP+siDIgJ0MRDF3vaIgdOwhHTcM7c9kCj0Vnotsk9k=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=okEBDYYs3wyYt7sGEyX7cG5FcmysuWb8L9HX63kgAvg97djyPPTjtLk2VA1GMiP4W\n\tIR86EWEa7fos8LtAn78+0QvaKNa5QNphzKobZ1l1CIRS6tSCgScRnKt46l5VRHdpmJ\n\tTEvizAOlZvLpNW4Jphv378+xptBiDHMKrVxFba5SLEnstZ89vtqWskaETkUqP1Bnmd\n\tzAL3mwHX3dCLnrDZLTK+dgtg6FG5vo3kFcYnZ2Hu56h/K+DsXALEfBCVf6SBgqDJSu\n\tkR/DmMBQo1vNSLRrCBNRl0OpIpiRD0KmVLUmc7WwpIIpbvErwWYsIdleoEiaIXxtHp\n\t+u7NQcYduvalQ==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=thaumatec-com.20210112.gappssmtp.com; s=20210112; t=1679912042;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=JvO+qSSCn+0YhHlWbuW7w9Y8yVOpIUsXjBzdz8ZVGcU=;\n\tb=tT5cbrpDeC/uL6u9TUvwrhbek6BsSFiLN6yhRnbvPNUIs3YQXMpJCIi7ZkmpZQZfxp\n\tyXsryEhTFkvbrI02fV69IBqde6IXJ/bmerNBXH+djSeEzryby15Ng4wCKYIrNfHvOS8C\n\t+nEVY7Ml8h3+5PUpogK8EdxW5ANLtwXoaItHnB0BnFEqHCl3HFkBtq635TUyCoh1il2H\n\t3F1yNHoJ10JqKYWy1sGkJX6ln6krzfdlL2RvGDqeF9h7IrxPzEr6Ra+JGsVveP0DO6G7\n\t2OmV3rD75HQ3Neup60i7XSuOlADW40MvrtQSH1rGJJywb2PLudEB2P17iBr3I49Rmrv/\n\tiIcA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=thaumatec-com.20210112.gappssmtp.com\n\theader.i=@thaumatec-com.20210112.gappssmtp.com header.b=\"tT5cbrpD\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112; t=1679912042;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=JvO+qSSCn+0YhHlWbuW7w9Y8yVOpIUsXjBzdz8ZVGcU=;\n\tb=KkHcg9tpAQjFOLNDE2+M1bMA97MKWinnyAHLy8V7LbmUa3j6bfsxWr0vt2ocv4IOpv\n\tIWc20DJofKrNEhmW4B/PQ54iLYJbdQEbIL9/XKdg+TpvA8YJRQFPJ3dqjL/vLC1lidgW\n\tdiKB15Ea61pkNcbThdoyaqQuBEeJIC3bCq8a8d1upr6wMVAukYWoG0NYOEgQ6yFbwMxP\n\tIFJ88ytMgF7nh6e8byGEOme96HaccC2tmyEelTLG0MrrwkxGr096NtFI3kt8IFTCYz5q\n\tkKnuTay6ABnbi9NRfMWj/qnGT7GjE7Uu8o3b7yzzLNcfipBHwOC1xDWHE7Hpamt5db0o\n\tk28g==","X-Gm-Message-State":"AAQBX9deAra0BkJyAaQxW3HvqjwnRQQ5xdW1gxOhfZ/4y0hnsBIcVfOX\n\tQ+K7Y+AACN4Mb2ZK5SX096nzUXpiieXYlpxtLzIX7g==","X-Google-Smtp-Source":"AKy350bP8FpXo7J4ZRA71Cr6aF2oCkhW04x/xe4EggY96EnF9q2XLIIuZgnnxVE9fQ6RFOfaIcfaPwXugrMnufCiTOQ=","X-Received":"by 2002:a50:d543:0:b0:4fb:f19:87f with SMTP id\n\tf3-20020a50d543000000b004fb0f19087fmr5766664edj.3.1679912042585; \n\tMon, 27 Mar 2023 03:14:02 -0700 (PDT)","MIME-Version":"1.0","References":"<20230324142908.64224-1-dse@thaumatec.com>\n\t<20230324142908.64224-9-dse@thaumatec.com>\n\t<20230324154207.as5slmewwirc4kru@uno.localdomain>\n\t<170c3b0e-fd16-5b83-4419-e9eab8c29e5f@ideasonboard.com>\n\t<20230325092729.ncsz2mcfqihnijl5@uno.localdomain>","In-Reply-To":"<20230325092729.ncsz2mcfqihnijl5@uno.localdomain>","Date":"Mon, 27 Mar 2023 12:13:51 +0200","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","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":"Daniel Semkowicz via libcamera-devel\n\t","Reply-To":"Daniel Semkowicz ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26760,"web_url":"https://patchwork.libcamera.org/comment/26760/","msgid":"<20230327110734.ka47tu2vufbnfwj5@uno.localdomain>","date":"2023-03-27T11:07:34","subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Daniel\n\nOn Mon, Mar 27, 2023 at 12:13:51PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> Hi Jacopo, Hi Kieran,\n>\n> On Sat, Mar 25, 2023 at 10:27 AM Jacopo Mondi\n> wrote:\n> >\n> > Hi Kieran\n> >\n> > On Fri, Mar 24, 2023 at 03:47:53PM +0000, Kieran Bingham wrote:\n> > >\n> > >\n> > > On 24/03/2023 15:42, Jacopo Mondi via libcamera-devel wrote:\n> > > > Hi daniel\n> > > >\n> > > > On Fri, Mar 24, 2023 at 03:29:06PM +0100, Daniel Semkowicz via libcamera-devel wrote:\n> > > > > Add controls supported by the AF algorithm to the list of controls\n> > > > > supported by the RkISP1 IPA. This exposes the AF controls to the user\n> > > > > and allows controlling the AF algorithm using the top level API.\n> > > > >\n> > > > > Signed-off-by: Daniel Semkowicz \n> > > > > ---\n> > > > > src/ipa/rkisp1/rkisp1.cpp | 6 ++++++\n> > > > > 1 file changed, 6 insertions(+)\n> > > > >\n> > > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > > > index cd1fbae3..4b30844f 100644\n> > > > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > > > @@ -101,10 +101,16 @@ namespace {\n> > > > > /* List of controls handled by the RkISP1 IPA */\n> > > > > const ControlInfoMap::Map rkisp1Controls{\n> > > > > { &controls::AeEnable, ControlInfo(false, true) },\n> > > > > + { &controls::AfMetering, ControlInfo(controls::AfMeteringValues) },\n> > > > > + { &controls::AfMode, ControlInfo(controls::AfModeValues) },\n> > > >\n> > > > AfPauseDeferred is not supported\n> > > >\n> > >\n> > > What happens if there's no VCM though - Should we only expose the controls\n> >\n> > You're very right, I overlooked that.\n> >\n> > > if a VCM is identified? (Though that requires reworking the creation of this\n> > > rkisp1Controls list to be handled by the algorithms)\n> >\n> > I'm not sure it needs to be done by algorithms, but the IPA should\n> > certainly be made aware of the lens presence at init() time.\n>\n> This is indeed something that is needed, especially that number\n> of algorithm implementations is increasing. The most appropriate\n> solution seems for the algorithms to expose the supported control list\n> or to populate the list. This would also fix the problem of checking\n> outside the algorithm, if certain configuration is available, like\n> lens.\n>\n> I see at least two problems with the current approach:\n> 1. controls are exposed by the IPA, even if the algorithm is not\n> enabled in the tuning file. This is confusing.\n\nAgreed\n\n> 2. What if there are, for example, two AF algorithm implementations,\n> with different supported controls? Which controls should be added\n> to the control list?\n\nHandling multiple algos for the same functions is not something I\nthink there's a design for at the moment.\n\nIf we defer registering ControlInfo to the single algorithms I guess\nthe problem then reduces on how to run-time select which algorithm\nimplementation to use.\n\n>\n> >\n> > To be honest I would be fine with what RPi does in this regard. have a\n> > look at the usage of lensPresent_ in their IPA module init() function.\n> > This looks like the less invasive change.\n> >\n> > I presume this could even be made better by passing the lens controls'\n> > from the PH to the IPA at init() time so that the AF ControlInfo\n> > elements could be initialized with the proper limits and not with\n> > default values as it happens here and on RPi.\n>\n> For me, moving it to init() is also a better solution. I would like to\n> avoid intermediate steps and initialize IPA lens config in one place\n> for clarity. This means the whole lens configuration that's currently\n> done in the configure(), will need to be moved to init().\n\nThe part about the lens in configure() intializes\n\n lensConfig.minFocusPosition = focusAbsolute.min().get();\n lensConfig.maxFocusPosition = focusAbsolute.max().get();\n\nAs you said, the lens min/max position -shouldn't- change between\nconfigurations so this part could be moved at init() time\n\n> However, I see one problem. context_.configuration struct is reset\n> in configure() for some reason. Even that\n> context_.configuration.sensor.lineDuration is set in init(). It is\n> reset and set again.\n\nThe other parameters (such as the gains and shutter speed) change\ndepending on the currently configured sensor's mode, that's why\nthey're re-calculated at each configure() call.\n\n>\n> What about making the IPASessionConfiguration::lens structure\n> std::optional, so it will be explicitly stated in the session config\n> if the lens is available? Then, AF can fail in init() if lens\n> are not available.\n>\n\nLet's take a step back for a second.\n\nWe have an updateControls() function, that starting for the static map\nrkisp1Controls augments it with controls whose values are computed by\ninspecting the sensorControls.\n\nCan't we do the same for the lens controls ? Add them only\nconditionally to the validity of lensControls ? Then if we deem it\nbetter we can pass to each algorithm the sensor's and lens\nconfiguration and have them populate the list of controls they handle.\n\n\n> This will work for lens, but I am afraid We will not be able to\n> evaluate all AF ControlInfo elements in the init() phase.\n> For example, AF window size limits depend on the sensor output size,\n> which is only available in the configure().\n\nupdateControls() has access to sensorInfo which contains the sensor's\noutput size\n\n>\n> Best regards\n> Daniel\n>\n> >\n> > >\n> > > --\n> > > Kieran\n> > >\n> > >\n> > > > Otherwise\n> > > > Reviewed-by: Jacopo Mondi \n> > > >\n> > > > > + { &controls::AfPause, ControlInfo(controls::AfPauseValues) },\n> > > > > + { &controls::AfTrigger, ControlInfo(controls::AfTriggerValues) },\n> > > > > + { &controls::AfWindows, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > > >\n> > > > We should -really- move to instantiate control limits directly based\n> > > > on the sensor configuration...\n> > > >\n> > > > > { &controls::AwbEnable, ControlInfo(false, true) },\n> > > > > { &controls::ColourGains, ControlInfo(0.0f, 3.996f, 1.0f) },\n> > > > > { &controls::Brightness, ControlInfo(-1.0f, 0.993f, 0.0f) },\n> > > > > { &controls::Contrast, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > > > > + { &controls::LensPosition, ControlInfo(0.0f, 2147483647.0f) },\n> > > > > { &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > > > > { &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },\n> > > > > { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n> > > > > --\n> > > > > 2.39.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 B347FBD160\n\tfor ;\n\tMon, 27 Mar 2023 11:07:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0B00062722;\n\tMon, 27 Mar 2023 13:07:39 +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 A2601626D7\n\tfor ;\n\tMon, 27 Mar 2023 13:07:37 +0200 (CEST)","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 2E56918C3;\n\tMon, 27 Mar 2023 13:07:37 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679915259;\n\tbh=ACjxPWV3h5A3qaZhKmJJjfu03uuzekyExZNpmVkeTuc=;\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=GGGD882QO/yrjeqcIkh8As8UTP5MWxwHQzs4D4vH3y563N/1gDILHcpdIlJpp9rm0\n\tD4daRFQD8ePQAS2hOH4ba3tTa3cjagFfhK9NxMtNRAxs0nEQEao49b0qsubvcryM9i\n\tuprDkTJO/HYxTnWA4CaVnJGpAlOiuQprexZ6Gb1GoX8ak8qX+/rVMSkxlqZayZY7RW\n\tF/qOMD7LmUHv9Xey3zZZUHIOl8DpKpHc2oWQOu8Du9lxkaveYOHb6gGvW7dCITKTbV\n\tdL3eK03c6QKhMv9tBbDrWeEd6MCVcppUDGyB4izqOJh9pwxw7+XvaRkvfjPrbkLxHj\n\t9glZuAFHGZIEA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679915257;\n\tbh=ACjxPWV3h5A3qaZhKmJJjfu03uuzekyExZNpmVkeTuc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=EzQZkGjI4UPRZgLNIPWCtqe6XwHi44bL0xp2++mEvyow8UfTS9JDtX+LSwqNc8LZv\n\tDreBGtrb6tJtq3E8HrKhC6JSRddnPOETeXmlkVmxvkPdhZfjWd6Ahl+znWpjbGRO5Q\n\tscJp63Imqg4ZvmFKKj5jekNj4XKV9qWLmf1pUcRo="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"EzQZkGjI\"; dkim-atps=neutral","Date":"Mon, 27 Mar 2023 13:07:34 +0200","To":"Daniel Semkowicz ","Message-ID":"<20230327110734.ka47tu2vufbnfwj5@uno.localdomain>","References":"<20230324142908.64224-1-dse@thaumatec.com>\n\t<20230324142908.64224-9-dse@thaumatec.com>\n\t<20230324154207.as5slmewwirc4kru@uno.localdomain>\n\t<170c3b0e-fd16-5b83-4419-e9eab8c29e5f@ideasonboard.com>\n\t<20230325092729.ncsz2mcfqihnijl5@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26761,"web_url":"https://patchwork.libcamera.org/comment/26761/","msgid":"","date":"2023-03-27T11:48:28","subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","submitter":{"id":126,"url":"https://patchwork.libcamera.org/api/people/126/","name":"Daniel Semkowicz","email":"dse@thaumatec.com"},"content":"On Mon, Mar 27, 2023 at 1:07 PM Jacopo Mondi\n wrote:\n>\n> Hi Daniel\n>\n> On Mon, Mar 27, 2023 at 12:13:51PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > Hi Jacopo, Hi Kieran,\n> >\n> > On Sat, Mar 25, 2023 at 10:27 AM Jacopo Mondi\n> > wrote:\n> > >\n> > > Hi Kieran\n> > >\n> > > On Fri, Mar 24, 2023 at 03:47:53PM +0000, Kieran Bingham wrote:\n> > > >\n> > > >\n> > > > On 24/03/2023 15:42, Jacopo Mondi via libcamera-devel wrote:\n> > > > > Hi daniel\n> > > > >\n> > > > > On Fri, Mar 24, 2023 at 03:29:06PM +0100, Daniel Semkowicz via libcamera-devel wrote:\n> > > > > > Add controls supported by the AF algorithm to the list of controls\n> > > > > > supported by the RkISP1 IPA. This exposes the AF controls to the user\n> > > > > > and allows controlling the AF algorithm using the top level API.\n> > > > > >\n> > > > > > Signed-off-by: Daniel Semkowicz \n> > > > > > ---\n> > > > > > src/ipa/rkisp1/rkisp1.cpp | 6 ++++++\n> > > > > > 1 file changed, 6 insertions(+)\n> > > > > >\n> > > > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > > > > index cd1fbae3..4b30844f 100644\n> > > > > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > > > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > > > > @@ -101,10 +101,16 @@ namespace {\n> > > > > > /* List of controls handled by the RkISP1 IPA */\n> > > > > > const ControlInfoMap::Map rkisp1Controls{\n> > > > > > { &controls::AeEnable, ControlInfo(false, true) },\n> > > > > > + { &controls::AfMetering, ControlInfo(controls::AfMeteringValues) },\n> > > > > > + { &controls::AfMode, ControlInfo(controls::AfModeValues) },\n> > > > >\n> > > > > AfPauseDeferred is not supported\n> > > > >\n> > > >\n> > > > What happens if there's no VCM though - Should we only expose the controls\n> > >\n> > > You're very right, I overlooked that.\n> > >\n> > > > if a VCM is identified? (Though that requires reworking the creation of this\n> > > > rkisp1Controls list to be handled by the algorithms)\n> > >\n> > > I'm not sure it needs to be done by algorithms, but the IPA should\n> > > certainly be made aware of the lens presence at init() time.\n> >\n> > This is indeed something that is needed, especially that number\n> > of algorithm implementations is increasing. The most appropriate\n> > solution seems for the algorithms to expose the supported control list\n> > or to populate the list. This would also fix the problem of checking\n> > outside the algorithm, if certain configuration is available, like\n> > lens.\n> >\n> > I see at least two problems with the current approach:\n> > 1. controls are exposed by the IPA, even if the algorithm is not\n> > enabled in the tuning file. This is confusing.\n>\n> Agreed\n>\n> > 2. What if there are, for example, two AF algorithm implementations,\n> > with different supported controls? Which controls should be added\n> > to the control list?\n>\n> Handling multiple algos for the same functions is not something I\n> think there's a design for at the moment.\n>\n> If we defer registering ControlInfo to the single algorithms I guess\n> the problem then reduces on how to run-time select which algorithm\n> implementation to use.\n\nI meant the current situation with ControlInfo map populated\nin the IPA instead in algorithms itself. As you said, this should not\nbe a problem with the controls handled in the algorithms.\n\n>\n> >\n> > >\n> > > To be honest I would be fine with what RPi does in this regard. have a\n> > > look at the usage of lensPresent_ in their IPA module init() function.\n> > > This looks like the less invasive change.\n> > >\n> > > I presume this could even be made better by passing the lens controls'\n> > > from the PH to the IPA at init() time so that the AF ControlInfo\n> > > elements could be initialized with the proper limits and not with\n> > > default values as it happens here and on RPi.\n> >\n> > For me, moving it to init() is also a better solution. I would like to\n> > avoid intermediate steps and initialize IPA lens config in one place\n> > for clarity. This means the whole lens configuration that's currently\n> > done in the configure(), will need to be moved to init().\n>\n> The part about the lens in configure() intializes\n>\n> lensConfig.minFocusPosition = focusAbsolute.min().get();\n> lensConfig.maxFocusPosition = focusAbsolute.max().get();\n>\n> As you said, the lens min/max position -shouldn't- change between\n> configurations so this part could be moved at init() time\n>\n> > However, I see one problem. context_.configuration struct is reset\n> > in configure() for some reason. Even that\n> > context_.configuration.sensor.lineDuration is set in init(). It is\n> > reset and set again.\n>\n> The other parameters (such as the gains and shutter speed) change\n> depending on the currently configured sensor's mode, that's why\n> they're re-calculated at each configure() call.\n>\n> >\n> > What about making the IPASessionConfiguration::lens structure\n> > std::optional, so it will be explicitly stated in the session config\n> > if the lens is available? Then, AF can fail in init() if lens\n> > are not available.\n> >\n>\n> Let's take a step back for a second.\n>\n> We have an updateControls() function, that starting for the static map\n> rkisp1Controls augments it with controls whose values are computed by\n> inspecting the sensorControls.\n>\n> Can't we do the same for the lens controls ? Add them only\n> conditionally to the validity of lensControls ? Then if we deem it\n> better we can pass to each algorithm the sensor's and lens\n> configuration and have them populate the list of controls they handle.\n>\n\nRight, this makes sense. I can rework the code like this.\nAlso, don't you think it is worth to fail the AF init if it was\nenabled on camera without moveable lens?\n\n>\n> > This will work for lens, but I am afraid We will not be able to\n> > evaluate all AF ControlInfo elements in the init() phase.\n> > For example, AF window size limits depend on the sensor output size,\n> > which is only available in the configure().\n>\n> updateControls() has access to sensorInfo which contains the sensor's\n> output size\n\nAh, I didn't notice that... In such case this is not a problem :)\n\n>\n> >\n> > Best regards\n> > Daniel\n> >\n> > >\n> > > >\n> > > > --\n> > > > Kieran\n> > > >\n> > > >\n> > > > > Otherwise\n> > > > > Reviewed-by: Jacopo Mondi \n> > > > >\n> > > > > > + { &controls::AfPause, ControlInfo(controls::AfPauseValues) },\n> > > > > > + { &controls::AfTrigger, ControlInfo(controls::AfTriggerValues) },\n> > > > > > + { &controls::AfWindows, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > > > >\n> > > > > We should -really- move to instantiate control limits directly based\n> > > > > on the sensor configuration...\n> > > > >\n> > > > > > { &controls::AwbEnable, ControlInfo(false, true) },\n> > > > > > { &controls::ColourGains, ControlInfo(0.0f, 3.996f, 1.0f) },\n> > > > > > { &controls::Brightness, ControlInfo(-1.0f, 0.993f, 0.0f) },\n> > > > > > { &controls::Contrast, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > > > > > + { &controls::LensPosition, ControlInfo(0.0f, 2147483647.0f) },\n> > > > > > { &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > > > > > { &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },\n> > > > > > { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n> > > > > > --\n> > > > > > 2.39.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 3B24EC0F2A\n\tfor ;\n\tMon, 27 Mar 2023 11:48:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6E4D062722;\n\tMon, 27 Mar 2023 13:48:41 +0200 (CEST)","from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com\n\t[IPv6:2a00:1450:4864:20::52b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C7458626D7\n\tfor ;\n\tMon, 27 Mar 2023 13:48:39 +0200 (CEST)","by mail-ed1-x52b.google.com with SMTP id b20so35033866edd.1\n\tfor ;\n\tMon, 27 Mar 2023 04:48:39 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679917721;\n\tbh=J/F2Gl8E2jEL9NsLk3m2maPUbDwICuvtpYhn9V0/so8=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=kmhCDJE5uuC0HVwJ3lovnMEq/s5kmPURt/cyh3XlWH/VgKMJWPPp6vqf1lXsWQqyh\n\tiKPHxtmiKt+/y/gdinTTjQr2oMQOOah6dCdcQHb7lQG7r1ekCzAuMdKQUwNgXhEPOU\n\tJ9QyBrctIvbkdAU9IhHvgCPa9CzWlV9C9ABHvC1yhBwGVF2Q39ZOdmhn5epKIAKk4U\n\tLylBhFUTBHRISn9lUjipUvlUWa5may8MYcBw4tg441wFugEzagI6KGBUnofpxXt0Qs\n\taBTd4drpYK3hIPylxDmpP7AQZKPIQhvMSyz206Dr7V+pKxh5h8k4VQVgbQFTA0vAea\n\tCcknECGjhW/Jg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=thaumatec-com.20210112.gappssmtp.com; s=20210112; t=1679917719;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=SQEzQHWnw1j5StaGQJ28dWkwSgyH5iAhAuccBKaMeb4=;\n\tb=RALqAwRo24bHbenBu2Ah9NN0swZV42L89tUgelQJWRZb4O4ve7iumiiNlbkKN19G89\n\tddQteuz9KhQV7IGxRFaDHRjAET+GnlS8L7dP4yKWkcEImJ3TGO7SYhhXTXtZ1cJkbPRQ\n\tKdKW/laUtqeSeD9vn4w48mzgCx5xaetgMP2u1mwPLzGbz+atvbFtDmqslnsYGeA+pUlo\n\tjWe+8vYTZ/TNBsZp9Kmbk/3FHVRycwVbz9utF0hmC6u/8VohecqdHPTLDOYyjjLWGePf\n\tOoV5onsNxVdPfzWqJ+HSyE9tIZn8PtlJceRCzYnq//KDDK+3CZq2fi/3R9N+yWgmCnTF\n\trhNg=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=thaumatec-com.20210112.gappssmtp.com\n\theader.i=@thaumatec-com.20210112.gappssmtp.com header.b=\"RALqAwRo\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112; t=1679917719;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=SQEzQHWnw1j5StaGQJ28dWkwSgyH5iAhAuccBKaMeb4=;\n\tb=nCYuO0kGLSmjkc4elboR6p/EPcpf6jook6Wxdnl1vZklSEV8SuVeT+CmFmPLTbfAHw\n\tY1Yprs6xJ3khFrO1ZSQsSNZjb09PDZdhGqOIuzwY991q0nfm5XFW3GzVeKbHGskEvxWx\n\tpRryHhmAV2KPRIdHI4515PJhWufNOhiJfvy1EKyTzbm3f3F27a0vBI0+fNRc7VRUyZFt\n\tSfrfrTB27Ld3WMuOQzu+Tw5FsGCqifMY+t+DwpzUYE0q5NLS2mEqaFrM7Q+TUbW7vJdK\n\tps0Uf9loPGS0K7Ov6F2r5B7E9cByL63PmwRZ6d2u/Ccxf5pnU5sMN25nebwiprnZCmA7\n\tJqPg==","X-Gm-Message-State":"AAQBX9clngMBGdoeAwe8PpxvhE4Hb35URovqLkWF7Gql319tM8amupX+\n\tu+kp9xHZprqg0Zxhzvr6k8R8qRGGWt76Qsr3GFpJCg==","X-Google-Smtp-Source":"AKy350YogOHQr8KgPEFxAIBWD0nGnucKiyxIq3OXpPWYRDspVUFqvq8beTWKHs7ogWdn66l35tKOhtXM7dde27gZ6Yw=","X-Received":"by 2002:a50:d49e:0:b0:502:148d:9e1e with SMTP id\n\ts30-20020a50d49e000000b00502148d9e1emr5644213edi.3.1679917719228;\n\tMon, 27 Mar 2023 04:48:39 -0700 (PDT)","MIME-Version":"1.0","References":"<20230324142908.64224-1-dse@thaumatec.com>\n\t<20230324142908.64224-9-dse@thaumatec.com>\n\t<20230324154207.as5slmewwirc4kru@uno.localdomain>\n\t<170c3b0e-fd16-5b83-4419-e9eab8c29e5f@ideasonboard.com>\n\t<20230325092729.ncsz2mcfqihnijl5@uno.localdomain>\n\t\n\t<20230327110734.ka47tu2vufbnfwj5@uno.localdomain>","In-Reply-To":"<20230327110734.ka47tu2vufbnfwj5@uno.localdomain>","Date":"Mon, 27 Mar 2023 13:48:28 +0200","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","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":"Daniel Semkowicz via libcamera-devel\n\t","Reply-To":"Daniel Semkowicz ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26763,"web_url":"https://patchwork.libcamera.org/comment/26763/","msgid":"<20230327123656.4href5m6m6scgctm@uno.localdomain>","date":"2023-03-27T12:36:56","subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Daniel\n\nOn Mon, Mar 27, 2023 at 01:48:28PM +0200, Daniel Semkowicz wrote:\n> On Mon, Mar 27, 2023 at 1:07 PM Jacopo Mondi\n> wrote:\n> >\n> > Hi Daniel\n> >\n> > On Mon, Mar 27, 2023 at 12:13:51PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > Hi Jacopo, Hi Kieran,\n> > >\n> > > On Sat, Mar 25, 2023 at 10:27 AM Jacopo Mondi\n> > > wrote:\n> > > >\n> > > > Hi Kieran\n> > > >\n> > > > On Fri, Mar 24, 2023 at 03:47:53PM +0000, Kieran Bingham wrote:\n> > > > >\n> > > > >\n> > > > > On 24/03/2023 15:42, Jacopo Mondi via libcamera-devel wrote:\n> > > > > > Hi daniel\n> > > > > >\n> > > > > > On Fri, Mar 24, 2023 at 03:29:06PM +0100, Daniel Semkowicz via libcamera-devel wrote:\n> > > > > > > Add controls supported by the AF algorithm to the list of controls\n> > > > > > > supported by the RkISP1 IPA. This exposes the AF controls to the user\n> > > > > > > and allows controlling the AF algorithm using the top level API.\n> > > > > > >\n> > > > > > > Signed-off-by: Daniel Semkowicz \n> > > > > > > ---\n> > > > > > > src/ipa/rkisp1/rkisp1.cpp | 6 ++++++\n> > > > > > > 1 file changed, 6 insertions(+)\n> > > > > > >\n> > > > > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > > > > > index cd1fbae3..4b30844f 100644\n> > > > > > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > > > > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > > > > > @@ -101,10 +101,16 @@ namespace {\n> > > > > > > /* List of controls handled by the RkISP1 IPA */\n> > > > > > > const ControlInfoMap::Map rkisp1Controls{\n> > > > > > > { &controls::AeEnable, ControlInfo(false, true) },\n> > > > > > > + { &controls::AfMetering, ControlInfo(controls::AfMeteringValues) },\n> > > > > > > + { &controls::AfMode, ControlInfo(controls::AfModeValues) },\n> > > > > >\n> > > > > > AfPauseDeferred is not supported\n> > > > > >\n> > > > >\n> > > > > What happens if there's no VCM though - Should we only expose the controls\n> > > >\n> > > > You're very right, I overlooked that.\n> > > >\n> > > > > if a VCM is identified? (Though that requires reworking the creation of this\n> > > > > rkisp1Controls list to be handled by the algorithms)\n> > > >\n> > > > I'm not sure it needs to be done by algorithms, but the IPA should\n> > > > certainly be made aware of the lens presence at init() time.\n> > >\n> > > This is indeed something that is needed, especially that number\n> > > of algorithm implementations is increasing. The most appropriate\n> > > solution seems for the algorithms to expose the supported control list\n> > > or to populate the list. This would also fix the problem of checking\n> > > outside the algorithm, if certain configuration is available, like\n> > > lens.\n> > >\n> > > I see at least two problems with the current approach:\n> > > 1. controls are exposed by the IPA, even if the algorithm is not\n> > > enabled in the tuning file. This is confusing.\n> >\n> > Agreed\n> >\n> > > 2. What if there are, for example, two AF algorithm implementations,\n> > > with different supported controls? Which controls should be added\n> > > to the control list?\n> >\n> > Handling multiple algos for the same functions is not something I\n> > think there's a design for at the moment.\n> >\n> > If we defer registering ControlInfo to the single algorithms I guess\n> > the problem then reduces on how to run-time select which algorithm\n> > implementation to use.\n>\n> I meant the current situation with ControlInfo map populated\n> in the IPA instead in algorithms itself. As you said, this should not\n> be a problem with the controls handled in the algorithms.\n>\n> >\n> > >\n> > > >\n> > > > To be honest I would be fine with what RPi does in this regard. have a\n> > > > look at the usage of lensPresent_ in their IPA module init() function.\n> > > > This looks like the less invasive change.\n> > > >\n> > > > I presume this could even be made better by passing the lens controls'\n> > > > from the PH to the IPA at init() time so that the AF ControlInfo\n> > > > elements could be initialized with the proper limits and not with\n> > > > default values as it happens here and on RPi.\n> > >\n> > > For me, moving it to init() is also a better solution. I would like to\n> > > avoid intermediate steps and initialize IPA lens config in one place\n> > > for clarity. This means the whole lens configuration that's currently\n> > > done in the configure(), will need to be moved to init().\n> >\n> > The part about the lens in configure() intializes\n> >\n> > lensConfig.minFocusPosition = focusAbsolute.min().get();\n> > lensConfig.maxFocusPosition = focusAbsolute.max().get();\n> >\n> > As you said, the lens min/max position -shouldn't- change between\n> > configurations so this part could be moved at init() time\n> >\n> > > However, I see one problem. context_.configuration struct is reset\n> > > in configure() for some reason. Even that\n> > > context_.configuration.sensor.lineDuration is set in init(). It is\n> > > reset and set again.\n> >\n> > The other parameters (such as the gains and shutter speed) change\n> > depending on the currently configured sensor's mode, that's why\n> > they're re-calculated at each configure() call.\n> >\n> > >\n> > > What about making the IPASessionConfiguration::lens structure\n> > > std::optional, so it will be explicitly stated in the session config\n> > > if the lens is available? Then, AF can fail in init() if lens\n> > > are not available.\n> > >\n> >\n> > Let's take a step back for a second.\n> >\n> > We have an updateControls() function, that starting for the static map\n> > rkisp1Controls augments it with controls whose values are computed by\n> > inspecting the sensorControls.\n> >\n> > Can't we do the same for the lens controls ? Add them only\n> > conditionally to the validity of lensControls ? Then if we deem it\n> > better we can pass to each algorithm the sensor's and lens\n> > configuration and have them populate the list of controls they handle.\n> >\n>\n> Right, this makes sense. I can rework the code like this.\n> Also, don't you think it is worth to fail the AF init if it was\n> enabled on camera without moveable lens?\n>\n\nmmm, that would make the whole IPA initialization fail if I'm not\nmistaken. The algorithm is enabled by the sensor configuration file,\nwhich indeed shouldn't include it if the device doesn't have a lens.\n\nIt also seems to me that if we don't fail, the AF algorithm\nimplementation would need to protect agains a missing lens with\n\n if (!context.configuration.lens)\n return 0;\n\nin all its methods, as it will be part of the list of active\nalgorithms, and I'm not sure it's worth it.\n\nIf you agree with the above, then yes, I guess we should fail in\ninit() if no lens is available.\n\n\n> >\n> > > This will work for lens, but I am afraid We will not be able to\n> > > evaluate all AF ControlInfo elements in the init() phase.\n> > > For example, AF window size limits depend on the sensor output size,\n> > > which is only available in the configure().\n> >\n> > updateControls() has access to sensorInfo which contains the sensor's\n> > output size\n>\n> Ah, I didn't notice that... In such case this is not a problem :)\n>\n> >\n> > >\n> > > Best regards\n> > > Daniel\n> > >\n> > > >\n> > > > >\n> > > > > --\n> > > > > Kieran\n> > > > >\n> > > > >\n> > > > > > Otherwise\n> > > > > > Reviewed-by: Jacopo Mondi \n> > > > > >\n> > > > > > > + { &controls::AfPause, ControlInfo(controls::AfPauseValues) },\n> > > > > > > + { &controls::AfTrigger, ControlInfo(controls::AfTriggerValues) },\n> > > > > > > + { &controls::AfWindows, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > > > > >\n> > > > > > We should -really- move to instantiate control limits directly based\n> > > > > > on the sensor configuration...\n> > > > > >\n> > > > > > > { &controls::AwbEnable, ControlInfo(false, true) },\n> > > > > > > { &controls::ColourGains, ControlInfo(0.0f, 3.996f, 1.0f) },\n> > > > > > > { &controls::Brightness, ControlInfo(-1.0f, 0.993f, 0.0f) },\n> > > > > > > { &controls::Contrast, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > > > > > > + { &controls::LensPosition, ControlInfo(0.0f, 2147483647.0f) },\n> > > > > > > { &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > > > > > > { &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },\n> > > > > > > { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n> > > > > > > --\n> > > > > > > 2.39.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 DB5E5C0F2A\n\tfor ;\n\tMon, 27 Mar 2023 12:37:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0ED6962722;\n\tMon, 27 Mar 2023 14:37:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9262E626D7\n\tfor ;\n\tMon, 27 Mar 2023 14:37:00 +0200 (CEST)","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 EE00DAEC;\n\tMon, 27 Mar 2023 14:36:59 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679920623;\n\tbh=WdY38tmMJih/DAPxPJc7iYWKx+gHJtWZcjHxh+VdgBs=;\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=4HhD/g9y5Y9Uwx84A31/O8cuMolwyFzqT8lzD1TAz2WfSZLJekoLCAHqxtwHcRq0h\n\t51Ha+eBkKbcgk7nI/lJIgjRQzmK8Q7mIruXH3bEqc2SAAMLNpYieYBLUgnBdMMCb4L\n\tl3DedGk0AIGrsZ6gw/AdsnJHUhSqC1SRLV8y+hnRnUOtyLCr0Ar3A62Tx65+mciXfz\n\tXNEGpncmGF0uDVjMNg2pIEPAo7EDY4+SEoDakrj1294oOigxD+3a4d+/mQfnOanvnQ\n\tjneS8eC7zumn55LRbo7jv+4LCCHGb9/PJa1LNSZBNFPnOVq6NTQkodVLXUpBwQgu94\n\txcclEx3zSaMiQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679920620;\n\tbh=WdY38tmMJih/DAPxPJc7iYWKx+gHJtWZcjHxh+VdgBs=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=p3XmXPlr8JNMusHAHeOBWDoYb37heOTYvHQDRlpkppeZTtnVZUc4fKolrNWDQW+cA\n\tVCfIOXjE0NIfHuhktsfA8TbirUMRWiweJoak84sY46+jLnVp5MlHSLhX9tg+LDHJlU\n\t9Q+tJJ29yDeO72WV7d6cd6zD9pOZpHpqAim/kiYY="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"p3XmXPlr\"; dkim-atps=neutral","Date":"Mon, 27 Mar 2023 14:36:56 +0200","To":"Daniel Semkowicz ","Message-ID":"<20230327123656.4href5m6m6scgctm@uno.localdomain>","References":"<20230324142908.64224-1-dse@thaumatec.com>\n\t<20230324142908.64224-9-dse@thaumatec.com>\n\t<20230324154207.as5slmewwirc4kru@uno.localdomain>\n\t<170c3b0e-fd16-5b83-4419-e9eab8c29e5f@ideasonboard.com>\n\t<20230325092729.ncsz2mcfqihnijl5@uno.localdomain>\n\t\n\t<20230327110734.ka47tu2vufbnfwj5@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26764,"web_url":"https://patchwork.libcamera.org/comment/26764/","msgid":"","date":"2023-03-27T12:59:59","subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","submitter":{"id":126,"url":"https://patchwork.libcamera.org/api/people/126/","name":"Daniel Semkowicz","email":"dse@thaumatec.com"},"content":"On Mon, Mar 27, 2023 at 2:37 PM Jacopo Mondi\n wrote:\n>\n> Hi Daniel\n>\n> On Mon, Mar 27, 2023 at 01:48:28PM +0200, Daniel Semkowicz wrote:\n> > On Mon, Mar 27, 2023 at 1:07 PM Jacopo Mondi\n> > wrote:\n> > >\n> > > Hi Daniel\n> > >\n> > > On Mon, Mar 27, 2023 at 12:13:51PM +0200, Daniel Semkowicz via libcamera-devel wrote:\n> > > > Hi Jacopo, Hi Kieran,\n> > > >\n> > > > On Sat, Mar 25, 2023 at 10:27 AM Jacopo Mondi\n> > > > wrote:\n> > > > >\n> > > > > Hi Kieran\n> > > > >\n> > > > > On Fri, Mar 24, 2023 at 03:47:53PM +0000, Kieran Bingham wrote:\n> > > > > >\n> > > > > >\n> > > > > > On 24/03/2023 15:42, Jacopo Mondi via libcamera-devel wrote:\n> > > > > > > Hi daniel\n> > > > > > >\n> > > > > > > On Fri, Mar 24, 2023 at 03:29:06PM +0100, Daniel Semkowicz via libcamera-devel wrote:\n> > > > > > > > Add controls supported by the AF algorithm to the list of controls\n> > > > > > > > supported by the RkISP1 IPA. This exposes the AF controls to the user\n> > > > > > > > and allows controlling the AF algorithm using the top level API.\n> > > > > > > >\n> > > > > > > > Signed-off-by: Daniel Semkowicz \n> > > > > > > > ---\n> > > > > > > > src/ipa/rkisp1/rkisp1.cpp | 6 ++++++\n> > > > > > > > 1 file changed, 6 insertions(+)\n> > > > > > > >\n> > > > > > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > > > > > > index cd1fbae3..4b30844f 100644\n> > > > > > > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > > > > > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > > > > > > @@ -101,10 +101,16 @@ namespace {\n> > > > > > > > /* List of controls handled by the RkISP1 IPA */\n> > > > > > > > const ControlInfoMap::Map rkisp1Controls{\n> > > > > > > > { &controls::AeEnable, ControlInfo(false, true) },\n> > > > > > > > + { &controls::AfMetering, ControlInfo(controls::AfMeteringValues) },\n> > > > > > > > + { &controls::AfMode, ControlInfo(controls::AfModeValues) },\n> > > > > > >\n> > > > > > > AfPauseDeferred is not supported\n> > > > > > >\n> > > > > >\n> > > > > > What happens if there's no VCM though - Should we only expose the controls\n> > > > >\n> > > > > You're very right, I overlooked that.\n> > > > >\n> > > > > > if a VCM is identified? (Though that requires reworking the creation of this\n> > > > > > rkisp1Controls list to be handled by the algorithms)\n> > > > >\n> > > > > I'm not sure it needs to be done by algorithms, but the IPA should\n> > > > > certainly be made aware of the lens presence at init() time.\n> > > >\n> > > > This is indeed something that is needed, especially that number\n> > > > of algorithm implementations is increasing. The most appropriate\n> > > > solution seems for the algorithms to expose the supported control list\n> > > > or to populate the list. This would also fix the problem of checking\n> > > > outside the algorithm, if certain configuration is available, like\n> > > > lens.\n> > > >\n> > > > I see at least two problems with the current approach:\n> > > > 1. controls are exposed by the IPA, even if the algorithm is not\n> > > > enabled in the tuning file. This is confusing.\n> > >\n> > > Agreed\n> > >\n> > > > 2. What if there are, for example, two AF algorithm implementations,\n> > > > with different supported controls? Which controls should be added\n> > > > to the control list?\n> > >\n> > > Handling multiple algos for the same functions is not something I\n> > > think there's a design for at the moment.\n> > >\n> > > If we defer registering ControlInfo to the single algorithms I guess\n> > > the problem then reduces on how to run-time select which algorithm\n> > > implementation to use.\n> >\n> > I meant the current situation with ControlInfo map populated\n> > in the IPA instead in algorithms itself. As you said, this should not\n> > be a problem with the controls handled in the algorithms.\n> >\n> > >\n> > > >\n> > > > >\n> > > > > To be honest I would be fine with what RPi does in this regard. have a\n> > > > > look at the usage of lensPresent_ in their IPA module init() function.\n> > > > > This looks like the less invasive change.\n> > > > >\n> > > > > I presume this could even be made better by passing the lens controls'\n> > > > > from the PH to the IPA at init() time so that the AF ControlInfo\n> > > > > elements could be initialized with the proper limits and not with\n> > > > > default values as it happens here and on RPi.\n> > > >\n> > > > For me, moving it to init() is also a better solution. I would like to\n> > > > avoid intermediate steps and initialize IPA lens config in one place\n> > > > for clarity. This means the whole lens configuration that's currently\n> > > > done in the configure(), will need to be moved to init().\n> > >\n> > > The part about the lens in configure() intializes\n> > >\n> > > lensConfig.minFocusPosition = focusAbsolute.min().get();\n> > > lensConfig.maxFocusPosition = focusAbsolute.max().get();\n> > >\n> > > As you said, the lens min/max position -shouldn't- change between\n> > > configurations so this part could be moved at init() time\n> > >\n> > > > However, I see one problem. context_.configuration struct is reset\n> > > > in configure() for some reason. Even that\n> > > > context_.configuration.sensor.lineDuration is set in init(). It is\n> > > > reset and set again.\n> > >\n> > > The other parameters (such as the gains and shutter speed) change\n> > > depending on the currently configured sensor's mode, that's why\n> > > they're re-calculated at each configure() call.\n> > >\n> > > >\n> > > > What about making the IPASessionConfiguration::lens structure\n> > > > std::optional, so it will be explicitly stated in the session config\n> > > > if the lens is available? Then, AF can fail in init() if lens\n> > > > are not available.\n> > > >\n> > >\n> > > Let's take a step back for a second.\n> > >\n> > > We have an updateControls() function, that starting for the static map\n> > > rkisp1Controls augments it with controls whose values are computed by\n> > > inspecting the sensorControls.\n> > >\n> > > Can't we do the same for the lens controls ? Add them only\n> > > conditionally to the validity of lensControls ? Then if we deem it\n> > > better we can pass to each algorithm the sensor's and lens\n> > > configuration and have them populate the list of controls they handle.\n> > >\n> >\n> > Right, this makes sense. I can rework the code like this.\n> > Also, don't you think it is worth to fail the AF init if it was\n> > enabled on camera without moveable lens?\n> >\n>\n> mmm, that would make the whole IPA initialization fail if I'm not\n> mistaken. The algorithm is enabled by the sensor configuration file,\n> which indeed shouldn't include it if the device doesn't have a lens.\n\nYes, but I think it is better to fail early on startup, than trying\nto make sure there will be no side effects when someone will try to\nuse the AF with wrong configuration.\n\n\n>\n> It also seems to me that if we don't fail, the AF algorithm\n> implementation would need to protect agains a missing lens with\n>\n> if (!context.configuration.lens)\n> return 0;\n>\n> in all its methods, as it will be part of the list of active\n> algorithms, and I'm not sure it's worth it.\n>\n> If you agree with the above, then yes, I guess we should fail in\n> init() if no lens is available.\n\n\nYes, I agree and it is simpler to do one check in the Af::init()\n\n>\n>\n> > >\n> > > > This will work for lens, but I am afraid We will not be able to\n> > > > evaluate all AF ControlInfo elements in the init() phase.\n> > > > For example, AF window size limits depend on the sensor output size,\n> > > > which is only available in the configure().\n> > >\n> > > updateControls() has access to sensorInfo which contains the sensor's\n> > > output size\n> >\n> > Ah, I didn't notice that... In such case this is not a problem :)\n> >\n> > >\n> > > >\n> > > > Best regards\n> > > > Daniel\n> > > >\n> > > > >\n> > > > > >\n> > > > > > --\n> > > > > > Kieran\n> > > > > >\n> > > > > >\n> > > > > > > Otherwise\n> > > > > > > Reviewed-by: Jacopo Mondi \n> > > > > > >\n> > > > > > > > + { &controls::AfPause, ControlInfo(controls::AfPauseValues) },\n> > > > > > > > + { &controls::AfTrigger, ControlInfo(controls::AfTriggerValues) },\n> > > > > > > > + { &controls::AfWindows, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },\n> > > > > > >\n> > > > > > > We should -really- move to instantiate control limits directly based\n> > > > > > > on the sensor configuration...\n> > > > > > >\n> > > > > > > > { &controls::AwbEnable, ControlInfo(false, true) },\n> > > > > > > > { &controls::ColourGains, ControlInfo(0.0f, 3.996f, 1.0f) },\n> > > > > > > > { &controls::Brightness, ControlInfo(-1.0f, 0.993f, 0.0f) },\n> > > > > > > > { &controls::Contrast, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > > > > > > > + { &controls::LensPosition, ControlInfo(0.0f, 2147483647.0f) },\n> > > > > > > > { &controls::Saturation, ControlInfo(0.0f, 1.993f, 1.0f) },\n> > > > > > > > { &controls::Sharpness, ControlInfo(0.0f, 10.0f, 1.0f) },\n> > > > > > > > { &controls::draft::NoiseReductionMode, ControlInfo(controls::draft::NoiseReductionModeValues) },\n> > > > > > > > --\n> > > > > > > > 2.39.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 73853BD160\n\tfor ;\n\tMon, 27 Mar 2023 13:00:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7728962722;\n\tMon, 27 Mar 2023 15:00:13 +0200 (CEST)","from mail-ed1-x52b.google.com (mail-ed1-x52b.google.com\n\t[IPv6:2a00:1450:4864:20::52b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 813F5626D7\n\tfor ;\n\tMon, 27 Mar 2023 15:00:11 +0200 (CEST)","by mail-ed1-x52b.google.com with SMTP id er18so24581253edb.9\n\tfor ;\n\tMon, 27 Mar 2023 06:00:11 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679922013;\n\tbh=L3cemc5fFDes1OCVzo8ZwLaXlpeWlc2V/f07fIEpJGQ=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=HND4UMmjnw34N2KGcJ1WmENl3FOa1pJIlvyRXs7XJShcip37t31lDYJPRM51mKnN1\n\tbrrwfstQ4YoXlZKug0J4UlLpR4h5S1E6c0wS2zM0Ujh1C/dqw9+SFywor5E4zveWh3\n\tVZIynV9BSL0xKieKcARNbR+4jZ5NpVDj1WYvSpoGnJn1V87qjScLlbXnCerNSDPoH2\n\tT0Z3IK2DC0J9jdHdee8v2gTpeGke9LqIzJ6fi4OZee/XSNOHRnIojRPc8Z0ZXC6S4v\n\tIXlW9cMXKys7yDo8wvo7vNP7TPopROm4rR3Ob8LfOPoMzK4YU1Ooxmy5QqlhK4Xrw0\n\t8He2QQnA06qEw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=thaumatec-com.20210112.gappssmtp.com; s=20210112; t=1679922011;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=8njgmJfI9JYkL85iwQLirMgVVRlnEh+lqBjYePVBKQo=;\n\tb=vwD+MEL1q/4Tpnu7XtpRtpvWrHtEQ41xiWC2gWr1qbJRAkmF+uDHeGw2eybibhExUF\n\tDU/U8e0n/ZufA89qttE5WNxX9SNL0GTpsBWNBOjLHpzcwFfbt0V8sSgm44jZ/xDtBosh\n\t95NwjM+qtmPVWIUsUQFdupv1FBACmOQ9R8tbmos+aejhTQDWYQJGpOzjyKYgOvFjmWo3\n\tXvtKgJyNATbFxtDVpMNHEzUqP1mgvh5zIsGiiwgFmYM75nklpQ1ULj1PWfB8qPhIdI0Z\n\tg3c0+nTBgDkheMeNtza7mHkQI+aW+9FpqomjV7zAXPhF4xm90PXJj+Lq70+6xmjdgp30\n\tnTqQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=thaumatec-com.20210112.gappssmtp.com\n\theader.i=@thaumatec-com.20210112.gappssmtp.com header.b=\"vwD+MEL1\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112; t=1679922011;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=8njgmJfI9JYkL85iwQLirMgVVRlnEh+lqBjYePVBKQo=;\n\tb=CAz6SBmpiQG+eSz+T06nJLZ8k/NuPy7YfVts0yMrxFHjLYPMPRHsJGqxzZ10k9ByqN\n\tf6OGrXJabEzNqbiyFWFm2p/m0vY7BjNP0MdThXCgYiKhVd1NZpG8JIyRuZeW0vgzIRnR\n\tRipkNGh+YlxA6VUxWvrZfMNSl6vzJGc/1eMs5J/O7HjPbAyI5sYbhLtqsOBrtCLQ//Lo\n\tghz2D9Knu3EvjD0mYcprtwvODA2aGYLOdRLna+tOMfM9wbMEXr/+Yn0AI5n2S7TRbMii\n\t/9+UnOIfc0KzOLVE0+qEvLkJP3/rvfB1BLY7W+TrvaMj1sUEmb8Xmk2OnKnxfVHnmUu4\n\t2V4Q==","X-Gm-Message-State":"AAQBX9ezaLWWXkrf/nOKsR4N/qOejuRBABLVsUU5jkWV9Qiard/QN8U4\n\tOJxGPjd9lNLyBNZAzMwyb1Me7dGkSi76mvOWlYk80Wp7cspeBHTeBpQ=","X-Google-Smtp-Source":"AKy350ZrjfFo9gbt/vYBU5KBWIgeMH5yclwHxCZVj87Jk40u/fbtITaMWVhhnQQHghSxCY9VksJzfUm8PXzb1ht/3Rs=","X-Received":"by 2002:a17:906:edc9:b0:924:efbb:8a8b with SMTP id\n\tsb9-20020a170906edc900b00924efbb8a8bmr5800099ejb.6.1679922010929;\n\tMon, 27 Mar 2023 06:00:10 -0700 (PDT)","MIME-Version":"1.0","References":"<20230324142908.64224-1-dse@thaumatec.com>\n\t<20230324142908.64224-9-dse@thaumatec.com>\n\t<20230324154207.as5slmewwirc4kru@uno.localdomain>\n\t<170c3b0e-fd16-5b83-4419-e9eab8c29e5f@ideasonboard.com>\n\t<20230325092729.ncsz2mcfqihnijl5@uno.localdomain>\n\t\n\t<20230327110734.ka47tu2vufbnfwj5@uno.localdomain>\n\t\n\t<20230327123656.4href5m6m6scgctm@uno.localdomain>","In-Reply-To":"<20230327123656.4href5m6m6scgctm@uno.localdomain>","Date":"Mon, 27 Mar 2023 14:59:59 +0200","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v5 08/10] ipa: rkisp1: Add AF controls\n\tto the RkISP1 IPA","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":"Daniel Semkowicz via libcamera-devel\n\t","Reply-To":"Daniel Semkowicz ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]