[{"id":22418,"web_url":"https://patchwork.libcamera.org/comment/22418/","msgid":"<164807967161.1103026.16374791177432451797@Monstersaurus>","date":"2022-03-23T23:54:31","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/4] libcamera: raspberrypi:\n\tControl the lens from pipeline","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Jean-Michel Hautbois via libcamera-devel (2022-03-23 16:01:44)\n> The lens focus is controled by a VCM, which is linked to the sensor\n> using the ancillary links.\n> Pass the control to the config info structure and make it possible to\n> update by the IPA.\n> \n> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> ---\n>  include/libcamera/ipa/raspberrypi.mojom         |  1 +\n>  .../pipeline/raspberrypi/raspberrypi.cpp        | 17 +++++++++++++++++\n>  2 files changed, 18 insertions(+)\n> \n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> index acd3cafe..0c3922b0 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -125,4 +125,5 @@ interface IPARPiEventInterface {\n>         embeddedComplete(uint32 bufferId);\n>         setIspControls(libcamera.ControlList controls);\n>         setDelayedControls(libcamera.ControlList controls);\n> +       setLensControls(libcamera.ControlList controls);\n>  };\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index c2230199..970f9fe7 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -33,6 +33,7 @@\n>  \n>  #include \"libcamera/internal/bayer_format.h\"\n>  #include \"libcamera/internal/camera.h\"\n> +#include \"libcamera/internal/camera_lens.h\"\n>  #include \"libcamera/internal/camera_sensor.h\"\n>  #include \"libcamera/internal/delayed_controls.h\"\n>  #include \"libcamera/internal/device_enumerator.h\"\n> @@ -202,6 +203,7 @@ public:\n>         void setIspControls(const ControlList &controls);\n>         void setDelayedControls(const ControlList &controls);\n>         void setSensorControls(ControlList &controls);\n> +       void setLensControls(const ControlList &controls);\n>  \n>         /* bufferComplete signal handlers. */\n>         void unicamBufferDequeue(FrameBuffer *buffer);\n> @@ -1483,6 +1485,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)\n>         ipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete);\n>         ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);\n>         ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);\n> +       ipa_->setLensControls.connect(this, &RPiCameraData::setLensControls);\n>  \n>         /*\n>          * The configuration (tuning file) is made from the sensor name unless\n> @@ -1519,6 +1522,10 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n>         entityControls.emplace(0, sensor_->controls());\n>         entityControls.emplace(1, isp_[Isp::Input].dev()->controls());\n>  \n> +       CameraLens *lens = sensor_->focusLens();\n> +       if (lens)\n> +               entityControls.emplace(2, lens->controls());\n> +\n>         /* Always send the user transform to the IPA. */\n>         ipaConfig.transform = static_cast<unsigned int>(config->transform);\n>  \n> @@ -1735,6 +1742,16 @@ void RPiCameraData::setDelayedControls(const ControlList &controls)\n>         handleState();\n>  }\n>  \n> +void RPiCameraData::setLensControls(const ControlList &ctrls)\n> +{\n> +       CameraLens *lens = sensor_->focusLens();\n> +       if (!lens)\n> +               return;\n> +\n> +       /* \\todo Should we keep track of the latest value applied ? */\n> +       lens->setFocusPosition(ctrls.get(V4L2_CID_FOCUS_ABSOLUTE).get<int32_t>());\n\nDoes the algorithm currently take into account what position the lens is\nat for the statistics it is considering? I'm concerned that it isn't, or\nthat it's considering the position / top of the hill as a value which is\nmis-matched against the frame...?\n\nI presume there are delays that mean it might even be hard to determine\nhow long it takes to move the VCM to a given position - so that might\nmake it more difficult to state which frame had which position?\n\n\n\n> +}\n> +\n>  void RPiCameraData::setSensorControls(ControlList &controls)\n>  {\n>         /*\n> -- \n> 2.32.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 E0652C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 23 Mar 2022 23:54:35 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 2B8D5604DA;\n\tThu, 24 Mar 2022 00:54:35 +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 DB536604C5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 24 Mar 2022 00:54:33 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 758BC12E9;\n\tThu, 24 Mar 2022 00:54:33 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648079675;\n\tbh=IQZIh9HHl1bMoQJzwhBnwtE95L0xfixTPLXYoGbZ8FE=;\n\th=In-Reply-To:References:To:Date:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=SSzHz5OjrHrMaWPVSaMRet/bAkb2Dn+Z8rZbzA0fNzdMM5Z4n9nDh505uV6l8GJUn\n\tLJpjiGD0UCinuZsGxPqdjW3/Obia8y+zBMX/Yc28/iRXdba+kbNTypzmoQWQrYZfiE\n\tuRIx9c/yUQjUupg/4xfA0P4Jkf6HO+7I3a7gerbV/bxmfRVh24p+se+naM/zzJfyes\n\t68MzAKp9uQiO7WV27Mbq23Z/OphkNvL6FymzrekTfU8gCgvovPK+pLybPPu1dEA/YA\n\th6HdOuvmZmGwx/rejwMq5z5B5Jw4o7dPvm5q6bmKWOHPPHH0k0a4bQPq33FxP5cjZI\n\t6/b549vRnRdgw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648079673;\n\tbh=IQZIh9HHl1bMoQJzwhBnwtE95L0xfixTPLXYoGbZ8FE=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=Wy1kAOnHTsQrefj6T/JwqGQMzdUEp7e0R9C//JKb48ItsiOpBd9vo/brNWCSzIJ8Y\n\tewbp3i16gYkKnuszn2uGpJB+3ASj2TcUZ7nnHojbNNAWpj9mYtdb0EcifNK7RCkUJV\n\t5gw04wonc33oNUrSLs5vqejAHeVlAlTZ651VV3fk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Wy1kAOnH\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220323160145.90606-4-jeanmichel.hautbois@ideasonboard.com>","References":"<20220323160145.90606-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220323160145.90606-4-jeanmichel.hautbois@ideasonboard.com>","To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 23 Mar 2022 23:54:31 +0000","Message-ID":"<164807967161.1103026.16374791177432451797@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/4] libcamera: raspberrypi:\n\tControl the lens from pipeline","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Kieran Bingham via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22423,"web_url":"https://patchwork.libcamera.org/comment/22423/","msgid":"<34e3a747-63d2-961e-6e86-0a1ced6a95b2@ideasonboard.com>","date":"2022-03-24T08:22:47","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/4] libcamera: raspberrypi:\n\tControl the lens from pipeline","submitter":{"id":75,"url":"https://patchwork.libcamera.org/api/people/75/","name":"Jean-Michel Hautbois","email":"jeanmichel.hautbois@ideasonboard.com"},"content":"Hi Kieran,\n\nOn 24/03/2022 00:54, Kieran Bingham wrote:\n> Quoting Jean-Michel Hautbois via libcamera-devel (2022-03-23 16:01:44)\n>> The lens focus is controled by a VCM, which is linked to the sensor\n>> using the ancillary links.\n>> Pass the control to the config info structure and make it possible to\n>> update by the IPA.\n>>\n>> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n>> ---\n>>   include/libcamera/ipa/raspberrypi.mojom         |  1 +\n>>   .../pipeline/raspberrypi/raspberrypi.cpp        | 17 +++++++++++++++++\n>>   2 files changed, 18 insertions(+)\n>>\n>> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n>> index acd3cafe..0c3922b0 100644\n>> --- a/include/libcamera/ipa/raspberrypi.mojom\n>> +++ b/include/libcamera/ipa/raspberrypi.mojom\n>> @@ -125,4 +125,5 @@ interface IPARPiEventInterface {\n>>          embeddedComplete(uint32 bufferId);\n>>          setIspControls(libcamera.ControlList controls);\n>>          setDelayedControls(libcamera.ControlList controls);\n>> +       setLensControls(libcamera.ControlList controls);\n>>   };\n>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> index c2230199..970f9fe7 100644\n>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>> @@ -33,6 +33,7 @@\n>>   \n>>   #include \"libcamera/internal/bayer_format.h\"\n>>   #include \"libcamera/internal/camera.h\"\n>> +#include \"libcamera/internal/camera_lens.h\"\n>>   #include \"libcamera/internal/camera_sensor.h\"\n>>   #include \"libcamera/internal/delayed_controls.h\"\n>>   #include \"libcamera/internal/device_enumerator.h\"\n>> @@ -202,6 +203,7 @@ public:\n>>          void setIspControls(const ControlList &controls);\n>>          void setDelayedControls(const ControlList &controls);\n>>          void setSensorControls(ControlList &controls);\n>> +       void setLensControls(const ControlList &controls);\n>>   \n>>          /* bufferComplete signal handlers. */\n>>          void unicamBufferDequeue(FrameBuffer *buffer);\n>> @@ -1483,6 +1485,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)\n>>          ipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete);\n>>          ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);\n>>          ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);\n>> +       ipa_->setLensControls.connect(this, &RPiCameraData::setLensControls);\n>>   \n>>          /*\n>>           * The configuration (tuning file) is made from the sensor name unless\n>> @@ -1519,6 +1522,10 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n>>          entityControls.emplace(0, sensor_->controls());\n>>          entityControls.emplace(1, isp_[Isp::Input].dev()->controls());\n>>   \n>> +       CameraLens *lens = sensor_->focusLens();\n>> +       if (lens)\n>> +               entityControls.emplace(2, lens->controls());\n>> +\n>>          /* Always send the user transform to the IPA. */\n>>          ipaConfig.transform = static_cast<unsigned int>(config->transform);\n>>   \n>> @@ -1735,6 +1742,16 @@ void RPiCameraData::setDelayedControls(const ControlList &controls)\n>>          handleState();\n>>   }\n>>   \n>> +void RPiCameraData::setLensControls(const ControlList &ctrls)\n>> +{\n>> +       CameraLens *lens = sensor_->focusLens();\n>> +       if (!lens)\n>> +               return;\n>> +\n>> +       /* \\todo Should we keep track of the latest value applied ? */\n>> +       lens->setFocusPosition(ctrls.get(V4L2_CID_FOCUS_ABSOLUTE).get<int32_t>());\n> \n> Does the algorithm currently take into account what position the lens is\n> at for the statistics it is considering? I'm concerned that it isn't, or\n> that it's considering the position / top of the hill as a value which is\n> mis-matched against the frame...?\n> \n> I presume there are delays that mean it might even be hard to determine\n> how long it takes to move the VCM to a given position - so that might\n> make it more difficult to state which frame had which position?\n\nThat's a good remark, and I don't really have an answer to this yet. I \nsuppose there is a delay, but I don't know how to measure it easily. And \nI am not sure it is a very big one, so it could very well not be a big \nissue (opened question !) ?\n\n> \n> \n>> +}\n>> +\n>>   void RPiCameraData::setSensorControls(ControlList &controls)\n>>   {\n>>          /*\n>> -- \n>> 2.32.0\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 51561C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 24 Mar 2022 08:22:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 09AFE604D5;\n\tThu, 24 Mar 2022 09:22:52 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 52862601F5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 24 Mar 2022 09:22:50 +0100 (CET)","from [IPV6:2a01:e0a:169:7140:62c2:96e1:1c82:440d] (unknown\n\t[IPv6:2a01:e0a:169:7140:62c2:96e1:1c82:440d])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 0139BFEF;\n\tThu, 24 Mar 2022 09:22:49 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648110172;\n\tbh=YyhkbCf2fwm0jyYU7IJ2KbgPw77iUl1Os7rcdVQx1bQ=;\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:\n\tFrom;\n\tb=iZCiTJs4oykOmEixPV4/FA6wYlKcdZBgkch/czmleVYZVuspGWaUkwATtbCPc8GFL\n\t/G2LFXyzwpocTSwibQgQSDLWC8nJpKtrOsQzUwrBx4b+/aSg8Qdtwee4xKONJ+z8bD\n\tUEL37zMasZ7kZNyx2csJSj+jf1Xb0AEpj8u0cMwRy+RUol/0wrEY0HJxL0YbsYs6Gj\n\tLYbE/8wZk1XEGBcIxPW49myA7yal5Xaj4nrPPbkdMjn/kuJf1z6byDb1NXidi2ARnb\n\t8Kbi1IlXoqoD9viDErr2F1ZdAzwxUW9cJdfxE0IWJCTCrtKZXijPozDs+rOVeX0TCD\n\t7ry6WMG3FB/FA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648110170;\n\tbh=YyhkbCf2fwm0jyYU7IJ2KbgPw77iUl1Os7rcdVQx1bQ=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=lJ3KtUjMXfctGEQwuO2GrN3jDmGXUgZkT+2hBaOj4HlUneyLmtzcw2Gc32tEHo0PI\n\t1kCJcSoQbr22NY0GrT1E8I4ResoW0oOO8RlluBodo40j4FQiZeJbL9XYmDDKKKjm+E\n\tRVDnnY2eO5JcvIaNl28qd68ZUAOslOpQa/tnbA/w="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"lJ3KtUjM\"; dkim-atps=neutral","Message-ID":"<34e3a747-63d2-961e-6e86-0a1ced6a95b2@ideasonboard.com>","Date":"Thu, 24 Mar 2022 09:22:47 +0100","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.7.0","Content-Language":"en-US","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220323160145.90606-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220323160145.90606-4-jeanmichel.hautbois@ideasonboard.com>\n\t<164807967161.1103026.16374791177432451797@Monstersaurus>","In-Reply-To":"<164807967161.1103026.16374791177432451797@Monstersaurus>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/4] libcamera: raspberrypi:\n\tControl the lens from pipeline","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Jean-Michel Hautbois via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22427,"web_url":"https://patchwork.libcamera.org/comment/22427/","msgid":"<CAEmqJPr2-FeMw27qJK3hjCGB1BqxnFsMxCjo98bZ87YDaDP_VA@mail.gmail.com>","date":"2022-03-24T10:28:33","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/4] libcamera: raspberrypi:\n\tControl the lens from pipeline","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi,\n\nOn Thu, 24 Mar 2022 at 08:22, Jean-Michel Hautbois via libcamera-devel <\nlibcamera-devel@lists.libcamera.org> wrote:\n\n> Hi Kieran,\n>\n> On 24/03/2022 00:54, Kieran Bingham wrote:\n> > Quoting Jean-Michel Hautbois via libcamera-devel (2022-03-23 16:01:44)\n> >> The lens focus is controled by a VCM, which is linked to the sensor\n> >> using the ancillary links.\n> >> Pass the control to the config info structure and make it possible to\n> >> update by the IPA.\n> >>\n> >> Signed-off-by: Jean-Michel Hautbois <\n> jeanmichel.hautbois@ideasonboard.com>\n> >> ---\n> >>   include/libcamera/ipa/raspberrypi.mojom         |  1 +\n> >>   .../pipeline/raspberrypi/raspberrypi.cpp        | 17 +++++++++++++++++\n> >>   2 files changed, 18 insertions(+)\n> >>\n> >> diff --git a/include/libcamera/ipa/raspberrypi.mojom\n> b/include/libcamera/ipa/raspberrypi.mojom\n> >> index acd3cafe..0c3922b0 100644\n> >> --- a/include/libcamera/ipa/raspberrypi.mojom\n> >> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> >> @@ -125,4 +125,5 @@ interface IPARPiEventInterface {\n> >>          embeddedComplete(uint32 bufferId);\n> >>          setIspControls(libcamera.ControlList controls);\n> >>          setDelayedControls(libcamera.ControlList controls);\n> >> +       setLensControls(libcamera.ControlList controls);\n> >>   };\n> >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> index c2230199..970f9fe7 100644\n> >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >> @@ -33,6 +33,7 @@\n> >>\n> >>   #include \"libcamera/internal/bayer_format.h\"\n> >>   #include \"libcamera/internal/camera.h\"\n> >> +#include \"libcamera/internal/camera_lens.h\"\n> >>   #include \"libcamera/internal/camera_sensor.h\"\n> >>   #include \"libcamera/internal/delayed_controls.h\"\n> >>   #include \"libcamera/internal/device_enumerator.h\"\n> >> @@ -202,6 +203,7 @@ public:\n> >>          void setIspControls(const ControlList &controls);\n> >>          void setDelayedControls(const ControlList &controls);\n> >>          void setSensorControls(ControlList &controls);\n> >> +       void setLensControls(const ControlList &controls);\n> >>\n> >>          /* bufferComplete signal handlers. */\n> >>          void unicamBufferDequeue(FrameBuffer *buffer);\n> >> @@ -1483,6 +1485,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig\n> *sensorConfig)\n> >>          ipa_->embeddedComplete.connect(this,\n> &RPiCameraData::embeddedComplete);\n> >>          ipa_->setIspControls.connect(this,\n> &RPiCameraData::setIspControls);\n> >>          ipa_->setDelayedControls.connect(this,\n> &RPiCameraData::setDelayedControls);\n> >> +       ipa_->setLensControls.connect(this,\n> &RPiCameraData::setLensControls);\n> >>\n> >>          /*\n> >>           * The configuration (tuning file) is made from the sensor\n> name unless\n> >> @@ -1519,6 +1522,10 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> >>          entityControls.emplace(0, sensor_->controls());\n> >>          entityControls.emplace(1, isp_[Isp::Input].dev()->controls());\n> >>\n> >> +       CameraLens *lens = sensor_->focusLens();\n> >> +       if (lens)\n> >> +               entityControls.emplace(2, lens->controls());\n> >> +\n> >>          /* Always send the user transform to the IPA. */\n> >>          ipaConfig.transform = static_cast<unsigned\n> int>(config->transform);\n> >>\n> >> @@ -1735,6 +1742,16 @@ void RPiCameraData::setDelayedControls(const\n> ControlList &controls)\n> >>          handleState();\n> >>   }\n> >>\n> >> +void RPiCameraData::setLensControls(const ControlList &ctrls)\n> >> +{\n> >> +       CameraLens *lens = sensor_->focusLens();\n> >> +       if (!lens)\n> >> +               return;\n> >> +\n> >> +       /* \\todo Should we keep track of the latest value applied ? */\n> >> +\n>  lens->setFocusPosition(ctrls.get(V4L2_CID_FOCUS_ABSOLUTE).get<int32_t>());\n> >\n> > Does the algorithm currently take into account what position the lens is\n> > at for the statistics it is considering? I'm concerned that it isn't, or\n> > that it's considering the position / top of the hill as a value which is\n> > mis-matched against the frame...?\n> >\n> > I presume there are delays that mean it might even be hard to determine\n> > how long it takes to move the VCM to a given position - so that might\n> > make it more difficult to state which frame had which position?\n>\n> That's a good remark, and I don't really have an answer to this yet. I\n> suppose there is a delay, but I don't know how to measure it easily. And\n> I am not sure it is a very big one, so it could very well not be a big\n> issue (opened question !) ?\n>\n\nThis is quite a tricky thing to do.  One option is have the af algorithm\ncount time/frames and assume it knows about the characteristics of the\nactuator and knows when the lens stopped moving to sample the stats.\n\nAnother option would be to use the DelayedControls to return the \"correct\"\nlens position for each frame.  The problem with this is the lens movement\ntime is dependent on displacement needed.  So, we need to adapt\nDelayedControls to allow something like \"set the control now but inform me\nof the change N frames later.\" That would work, and keeps in line with how\ndelayed controls report shutter/gain from the sensor.\n\nNaush\n\n\n>\n> >\n> >\n> >> +}\n> >> +\n> >>   void RPiCameraData::setSensorControls(ControlList &controls)\n> >>   {\n> >>          /*\n> >> --\n> >> 2.32.0\n> >>\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 18E68C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 24 Mar 2022 10:28:53 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 72964604C5;\n\tThu, 24 Mar 2022 11:28:52 +0100 (CET)","from mail-lf1-x12d.google.com (mail-lf1-x12d.google.com\n\t[IPv6:2a00:1450:4864:20::12d])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 98847601F5\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 24 Mar 2022 11:28:50 +0100 (CET)","by mail-lf1-x12d.google.com with SMTP id h7so7268987lfl.2\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 24 Mar 2022 03:28:50 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648117732;\n\tbh=JUDg6pl6tnR60od2ZH2GY3IYtFoXwphdaDijTgvZIHU=;\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=F6pOmt9jCYHjVH3xwPoLvr66i9H1W49P7MOtmNAERd2h1Rcb8dJxiWbt9x83jfC5W\n\teq8i25kERldyLqFlnNclM/hzG0VZwZIgUEwuUVqOsEIl2yXA8UmB67a1yBEy4Rj7hX\n\t+AyF6s9VhvOhBIKg64+ygDMSPSwvHP0PtsfJyvIsBm+7cx+oTpXGqTXl/kveqN1mNt\n\t0wvV+Vl5biSeCwEyGwd2nU+KmG8C7OTgNP+adtAuB7JDaQKKbs0esuianghGG+fI3B\n\tsbBmhPtatKVrWer357Sx9HGIiIK+kWy4cqkbVHbEmT7pka8pKp8amyytvjLahE8qsE\n\tOIuGEqz5HSoaA==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=yIJkbvrARLDbj9/mxxLYEAgMwYk09+iFAzvl1yylQRc=;\n\tb=qE8Cjm+3iL3oIhzd1gzLy6GMSw5DNVjLUSYJ0VOmed60VqzvTj/by9+xVWdGJhxno7\n\tmGuz8q3e/ZjPQ2CLr4szbfsmeXneVBeCjQboV0Nltep1a3GUBjKIWYCFTHiKSpVCSVw9\n\tmSjI6wWYwV1NjQPAaILKX65Jrmi5e4sl0Lf2pIeOIz+oSFt087Mi/w72j95KKl/RneKl\n\tftvDTsPDAP+Y2ux5hI2kbq03zK1s7OKIbzD2Ig1EuisdUAB4MC6nlP0MHXPUWITVc0rY\n\twrSnZVO0+ti/4DF/Y5xXTrJ5wEADSvOyLEDLpM2DLcKMLrdbBt1ceNi7lfbAIA5xTkJI\n\tf8IA=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"qE8Cjm+3\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=yIJkbvrARLDbj9/mxxLYEAgMwYk09+iFAzvl1yylQRc=;\n\tb=zhXAbnsI402wSGmhxrftIRdg9TueSzaG0UfOR/2cc0OTg4HzdBZixh9QjOgdmbicil\n\tNjbPZpnMCYuRg16MbHROf4pTydkugKDni5Os96QgguJPP8QcjxtpfWubUe9PiUV42cDf\n\tUGc4AqhN0kUqjO4q4vogDxjLmjIOgn9rI/qb9r7vv9BHiLpQjbXorflPn1lj1Z6fI4mH\n\tk5D3hupLuyO6QxVkLDTBzUDdeaUFqj25RGEAtJy4BckygZE4bktkGvcKpDGIUJxTa15I\n\t4cYlVznVQyMjrWnXxD7MUrj+YQMEhNVPJ7BZGZtKvfJvbfcfb5tD67g7TK94M0LIRGnJ\n\tanCg==","X-Gm-Message-State":"AOAM5301/FIyI5QSaCzWFYwWD2ZXfo7Ud7MIICLvv2Dqhnw81uLdLPGZ\n\t2Fu1mayrd8jGEGCwVz9eEhzuFG7hGUAORJg40BFhrV1HHR8=","X-Google-Smtp-Source":"ABdhPJzdiBk8MX64u6Lsml7Gsgf+Tm4fBdM/N+e2dKfbOgIXjd8Bc9953Qco/R7xj0r1jv5rwbl7C2uv2I2T+YOIFbA=","X-Received":"by 2002:a05:6512:25a:b0:44a:3f55:1746 with SMTP id\n\tb26-20020a056512025a00b0044a3f551746mr3299505lfo.122.1648117729478;\n\tThu, 24 Mar 2022 03:28:49 -0700 (PDT)","MIME-Version":"1.0","References":"<20220323160145.90606-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220323160145.90606-4-jeanmichel.hautbois@ideasonboard.com>\n\t<164807967161.1103026.16374791177432451797@Monstersaurus>\n\t<34e3a747-63d2-961e-6e86-0a1ced6a95b2@ideasonboard.com>","In-Reply-To":"<34e3a747-63d2-961e-6e86-0a1ced6a95b2@ideasonboard.com>","Date":"Thu, 24 Mar 2022 10:28:33 +0000","Message-ID":"<CAEmqJPr2-FeMw27qJK3hjCGB1BqxnFsMxCjo98bZ87YDaDP_VA@mail.gmail.com>","To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"000000000000d05ba005daf44fbd\"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/4] libcamera: raspberrypi:\n\tControl the lens from pipeline","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22470,"web_url":"https://patchwork.libcamera.org/comment/22470/","msgid":"<YkDetOIiu+fhTE9A@pendragon.ideasonboard.com>","date":"2022-03-27T22:01:24","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/4] libcamera: raspberrypi:\n\tControl the lens from pipeline","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Thu, Mar 24, 2022 at 10:28:33AM +0000, Naushir Patuck wrote:\n> On Thu, 24 Mar 2022 at 08:22, Jean-Michel Hautbois  wrote:\n> > On 24/03/2022 00:54, Kieran Bingham wrote:\n> > > Quoting Jean-Michel Hautbois via libcamera-devel (2022-03-23 16:01:44)\n> > >> The lens focus is controled by a VCM, which is linked to the sensor\n> > >> using the ancillary links.\n> > >> Pass the control to the config info structure and make it possible to\n> > >> update by the IPA.\n> > >>\n> > >> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> > >> ---\n> > >>   include/libcamera/ipa/raspberrypi.mojom         |  1 +\n> > >>   .../pipeline/raspberrypi/raspberrypi.cpp        | 17 +++++++++++++++++\n> > >>   2 files changed, 18 insertions(+)\n> > >>\n> > >> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> > >> index acd3cafe..0c3922b0 100644\n> > >> --- a/include/libcamera/ipa/raspberrypi.mojom\n> > >> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > >> @@ -125,4 +125,5 @@ interface IPARPiEventInterface {\n> > >>          embeddedComplete(uint32 bufferId);\n> > >>          setIspControls(libcamera.ControlList controls);\n> > >>          setDelayedControls(libcamera.ControlList controls);\n> > >> +       setLensControls(libcamera.ControlList controls);\n> > >>   };\n> > >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > >> index c2230199..970f9fe7 100644\n> > >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > >> @@ -33,6 +33,7 @@\n> > >>\n> > >>   #include \"libcamera/internal/bayer_format.h\"\n> > >>   #include \"libcamera/internal/camera.h\"\n> > >> +#include \"libcamera/internal/camera_lens.h\"\n> > >>   #include \"libcamera/internal/camera_sensor.h\"\n> > >>   #include \"libcamera/internal/delayed_controls.h\"\n> > >>   #include \"libcamera/internal/device_enumerator.h\"\n> > >> @@ -202,6 +203,7 @@ public:\n> > >>          void setIspControls(const ControlList &controls);\n> > >>          void setDelayedControls(const ControlList &controls);\n> > >>          void setSensorControls(ControlList &controls);\n> > >> +       void setLensControls(const ControlList &controls);\n> > >>\n> > >>          /* bufferComplete signal handlers. */\n> > >>          void unicamBufferDequeue(FrameBuffer *buffer);\n> > >> @@ -1483,6 +1485,7 @@ int RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)\n> > >>          ipa_->embeddedComplete.connect(this, &RPiCameraData::embeddedComplete);\n> > >>          ipa_->setIspControls.connect(this, &RPiCameraData::setIspControls);\n> > >>          ipa_->setDelayedControls.connect(this, &RPiCameraData::setDelayedControls);\n> > >> +       ipa_->setLensControls.connect(this, &RPiCameraData::setLensControls);\n> > >>\n> > >>          /*\n> > >>           * The configuration (tuning file) is made from the sensor name unless\n> > >> @@ -1519,6 +1522,10 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n> > >>          entityControls.emplace(0, sensor_->controls());\n> > >>          entityControls.emplace(1, isp_[Isp::Input].dev()->controls());\n> > >>\n> > >> +       CameraLens *lens = sensor_->focusLens();\n> > >> +       if (lens)\n> > >> +               entityControls.emplace(2, lens->controls());\n> > >> +\n> > >>          /* Always send the user transform to the IPA. */\n> > >>          ipaConfig.transform = static_cast<unsigned int>(config->transform);\n> > >>\n> > >> @@ -1735,6 +1742,16 @@ void RPiCameraData::setDelayedControls(const ControlList &controls)\n> > >>          handleState();\n> > >>   }\n> > >>\n> > >> +void RPiCameraData::setLensControls(const ControlList &ctrls)\n> > >> +{\n> > >> +       CameraLens *lens = sensor_->focusLens();\n> > >> +       if (!lens)\n> > >> +               return;\n> > >> +\n> > >> +       /* \\todo Should we keep track of the latest value applied ? */\n> > >> +       lens->setFocusPosition(ctrls.get(V4L2_CID_FOCUS_ABSOLUTE).get<int32_t>());\n> > >\n> > > Does the algorithm currently take into account what position the lens is\n> > > at for the statistics it is considering? I'm concerned that it isn't, or\n> > > that it's considering the position / top of the hill as a value which is\n> > > mis-matched against the frame...?\n> > >\n> > > I presume there are delays that mean it might even be hard to determine\n> > > how long it takes to move the VCM to a given position - so that might\n> > > make it more difficult to state which frame had which position?\n> >\n> > That's a good remark, and I don't really have an answer to this yet. I\n> > suppose there is a delay, but I don't know how to measure it easily. And\n> > I am not sure it is a very big one, so it could very well not be a big\n> > issue (opened question !) ?\n> \n> This is quite a tricky thing to do.  One option is have the af algorithm\n> count time/frames and assume it knows about the characteristics of the\n> actuator and knows when the lens stopped moving to sample the stats.\n\nI think we'll need that. VCM actuators can be very tricky, especially\nwhen they don't have a control loop. Large swing values will result in\novershoot and ringing. Some open-loop VCM actuators have hardware\nfeatures to generate a ramp instead of a step, which can already help\n(the ramp parameters can be programmable).\n\n> Another option would be to use the DelayedControls to return the \"correct\"\n> lens position for each frame.  The problem with this is the lens movement\n> time is dependent on displacement needed.  So, we need to adapt\n> DelayedControls to allow something like \"set the control now but inform me\n> of the change N frames later.\" That would work, and keeps in line with how\n> delayed controls report shutter/gain from the sensor.\n\nI don't think DelayedControl should handle this, it will be very\nVCM-specific. A VCM helper (or rather, most likely, VCM helper*s*) would\nbe better.\n\n> > >> +}\n> > >> +\n> > >>   void RPiCameraData::setSensorControls(ControlList &controls)\n> > >>   {\n> > >>          /*","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 C0D41C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 27 Mar 2022 22:01:30 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DF13765631;\n\tMon, 28 Mar 2022 00:01:29 +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 586AF600B0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Mar 2022 00:01:28 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8F2A82F7;\n\tMon, 28 Mar 2022 00:01:26 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648418489;\n\tbh=LpiKC5zlwzZMM/qVQFFa4BrprGV21IS5J7uVWJl6eeE=;\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=TfAL2Tt1waJTmF3JywSAgD12k53gSVCsraW4faLd/32QoNZaEEmTprr/RI3zfDjXr\n\t/kPP0EDB/TBsFu0NMoudT1BFpHNUbK7qLsMn1g+/D5H0UN1tHxIuDTPVEen0dCtXyr\n\tmlp+8JS9GF2QK8RbnktiUnT4HNG7ahKT0eakP8H5Gy0R2YLQg+2ahN9+gZQDLjlxDG\n\tAr32c0ohNZzUZ3n++quXOxddgnFmjv+h8JC+xaOqk7VMJpsPsWaJhVGRxO3/jljw+t\n\tmEXNcdCpnTk4pIiYpGDM4mPB3bU4vKaDizNG+zPKEitCVFEIeQYe1mokke7hKqta3S\n\tb2IYQizlN2gBA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1648418488;\n\tbh=LpiKC5zlwzZMM/qVQFFa4BrprGV21IS5J7uVWJl6eeE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=u6iG7WUnquGinGrn6iS6qh2TWVXwT2QBBxiwfDwMW2KphwUv9xj1LzVg6SE1d7n3j\n\t7sC5Wd4flZIcWKfWyKoXBTvdPZ5OVK3GJF2RXHczPODGdJOtES1Yvx0msdVec/mdrO\n\t22U4aRwz0fMdRG+Bo2mbt7UUueRLLdXWj7ov4Ugk="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"u6iG7WUn\"; dkim-atps=neutral","Date":"Mon, 28 Mar 2022 01:01:24 +0300","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<YkDetOIiu+fhTE9A@pendragon.ideasonboard.com>","References":"<20220323160145.90606-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220323160145.90606-4-jeanmichel.hautbois@ideasonboard.com>\n\t<164807967161.1103026.16374791177432451797@Monstersaurus>\n\t<34e3a747-63d2-961e-6e86-0a1ced6a95b2@ideasonboard.com>\n\t<CAEmqJPr2-FeMw27qJK3hjCGB1BqxnFsMxCjo98bZ87YDaDP_VA@mail.gmail.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<CAEmqJPr2-FeMw27qJK3hjCGB1BqxnFsMxCjo98bZ87YDaDP_VA@mail.gmail.com>","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/4] libcamera: raspberrypi:\n\tControl the lens from pipeline","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Laurent Pinchart via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":22478,"web_url":"https://patchwork.libcamera.org/comment/22478/","msgid":"<CAEmqJPpWhRNEVTSG9RBfvH=MSw5Otb7dmPzHLK1H6Bt5Z_5Pag@mail.gmail.com>","date":"2022-03-28T07:50:36","subject":"Re: [libcamera-devel] [RFC PATCH v2 3/4] libcamera: raspberrypi:\n\tControl the lens from pipeline","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi,\n\nOn Sun, 27 Mar 2022 at 23:01, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> On Thu, Mar 24, 2022 at 10:28:33AM +0000, Naushir Patuck wrote:\n> > On Thu, 24 Mar 2022 at 08:22, Jean-Michel Hautbois  wrote:\n> > > On 24/03/2022 00:54, Kieran Bingham wrote:\n> > > > Quoting Jean-Michel Hautbois via libcamera-devel (2022-03-23\n> 16:01:44)\n> > > >> The lens focus is controled by a VCM, which is linked to the sensor\n> > > >> using the ancillary links.\n> > > >> Pass the control to the config info structure and make it possible\n> to\n> > > >> update by the IPA.\n> > > >>\n> > > >> Signed-off-by: Jean-Michel Hautbois <\n> jeanmichel.hautbois@ideasonboard.com>\n> > > >> ---\n> > > >>   include/libcamera/ipa/raspberrypi.mojom         |  1 +\n> > > >>   .../pipeline/raspberrypi/raspberrypi.cpp        | 17\n> +++++++++++++++++\n> > > >>   2 files changed, 18 insertions(+)\n> > > >>\n> > > >> diff --git a/include/libcamera/ipa/raspberrypi.mojom\n> b/include/libcamera/ipa/raspberrypi.mojom\n> > > >> index acd3cafe..0c3922b0 100644\n> > > >> --- a/include/libcamera/ipa/raspberrypi.mojom\n> > > >> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> > > >> @@ -125,4 +125,5 @@ interface IPARPiEventInterface {\n> > > >>          embeddedComplete(uint32 bufferId);\n> > > >>          setIspControls(libcamera.ControlList controls);\n> > > >>          setDelayedControls(libcamera.ControlList controls);\n> > > >> +       setLensControls(libcamera.ControlList controls);\n> > > >>   };\n> > > >> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > >> index c2230199..970f9fe7 100644\n> > > >> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > >> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > > >> @@ -33,6 +33,7 @@\n> > > >>\n> > > >>   #include \"libcamera/internal/bayer_format.h\"\n> > > >>   #include \"libcamera/internal/camera.h\"\n> > > >> +#include \"libcamera/internal/camera_lens.h\"\n> > > >>   #include \"libcamera/internal/camera_sensor.h\"\n> > > >>   #include \"libcamera/internal/delayed_controls.h\"\n> > > >>   #include \"libcamera/internal/device_enumerator.h\"\n> > > >> @@ -202,6 +203,7 @@ public:\n> > > >>          void setIspControls(const ControlList &controls);\n> > > >>          void setDelayedControls(const ControlList &controls);\n> > > >>          void setSensorControls(ControlList &controls);\n> > > >> +       void setLensControls(const ControlList &controls);\n> > > >>\n> > > >>          /* bufferComplete signal handlers. */\n> > > >>          void unicamBufferDequeue(FrameBuffer *buffer);\n> > > >> @@ -1483,6 +1485,7 @@ int\n> RPiCameraData::loadIPA(ipa::RPi::SensorConfig *sensorConfig)\n> > > >>          ipa_->embeddedComplete.connect(this,\n> &RPiCameraData::embeddedComplete);\n> > > >>          ipa_->setIspControls.connect(this,\n> &RPiCameraData::setIspControls);\n> > > >>          ipa_->setDelayedControls.connect(this,\n> &RPiCameraData::setDelayedControls);\n> > > >> +       ipa_->setLensControls.connect(this,\n> &RPiCameraData::setLensControls);\n> > > >>\n> > > >>          /*\n> > > >>           * The configuration (tuning file) is made from the sensor\n> name unless\n> > > >> @@ -1519,6 +1522,10 @@ int RPiCameraData::configureIPA(const\n> CameraConfiguration *config)\n> > > >>          entityControls.emplace(0, sensor_->controls());\n> > > >>          entityControls.emplace(1,\n> isp_[Isp::Input].dev()->controls());\n> > > >>\n> > > >> +       CameraLens *lens = sensor_->focusLens();\n> > > >> +       if (lens)\n> > > >> +               entityControls.emplace(2, lens->controls());\n> > > >> +\n> > > >>          /* Always send the user transform to the IPA. */\n> > > >>          ipaConfig.transform = static_cast<unsigned\n> int>(config->transform);\n> > > >>\n> > > >> @@ -1735,6 +1742,16 @@ void RPiCameraData::setDelayedControls(const\n> ControlList &controls)\n> > > >>          handleState();\n> > > >>   }\n> > > >>\n> > > >> +void RPiCameraData::setLensControls(const ControlList &ctrls)\n> > > >> +{\n> > > >> +       CameraLens *lens = sensor_->focusLens();\n> > > >> +       if (!lens)\n> > > >> +               return;\n> > > >> +\n> > > >> +       /* \\todo Should we keep track of the latest value applied ?\n> */\n> > > >> +\n>  lens->setFocusPosition(ctrls.get(V4L2_CID_FOCUS_ABSOLUTE).get<int32_t>());\n> > > >\n> > > > Does the algorithm currently take into account what position the\n> lens is\n> > > > at for the statistics it is considering? I'm concerned that it\n> isn't, or\n> > > > that it's considering the position / top of the hill as a value\n> which is\n> > > > mis-matched against the frame...?\n> > > >\n> > > > I presume there are delays that mean it might even be hard to\n> determine\n> > > > how long it takes to move the VCM to a given position - so that might\n> > > > make it more difficult to state which frame had which position?\n> > >\n> > > That's a good remark, and I don't really have an answer to this yet. I\n> > > suppose there is a delay, but I don't know how to measure it easily.\n> And\n> > > I am not sure it is a very big one, so it could very well not be a big\n> > > issue (opened question !) ?\n> >\n> > This is quite a tricky thing to do.  One option is have the af algorithm\n> > count time/frames and assume it knows about the characteristics of the\n> > actuator and knows when the lens stopped moving to sample the stats.\n>\n> I think we'll need that. VCM actuators can be very tricky, especially\n> when they don't have a control loop. Large swing values will result in\n> overshoot and ringing. Some open-loop VCM actuators have hardware\n> features to generate a ramp instead of a step, which can already help\n> (the ramp parameters can be programmable).\n>\n> > Another option would be to use the DelayedControls to return the\n> \"correct\"\n> > lens position for each frame.  The problem with this is the lens movement\n> > time is dependent on displacement needed.  So, we need to adapt\n> > DelayedControls to allow something like \"set the control now but inform\n> me\n> > of the change N frames later.\" That would work, and keeps in line with\n> how\n> > delayed controls report shutter/gain from the sensor.\n>\n> I don't think DelayedControl should handle this, it will be very\n> VCM-specific. A VCM helper (or rather, most likely, VCM helper*s*) would\n> be better.\n>\n\nI agree that DelayedControls is not the right place for this.  Perhaps a\nnew member\nfunction CameraLens::getrFocusPosition that takes in the frame\nsequence number\nand returns the expected position through some dead-reckoning calculations?\n\nRegards,\nNaush\n\n\n>\n> > > >> +}\n> > > >> +\n> > > >>   void RPiCameraData::setSensorControls(ControlList &controls)\n> > > >>   {\n> > > >>          /*\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 7B3E2C0F1B\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 28 Mar 2022 07:50:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF7FF65633;\n\tMon, 28 Mar 2022 09:50:53 +0200 (CEST)","from mail-lf1-x130.google.com (mail-lf1-x130.google.com\n\t[IPv6:2a00:1450:4864:20::130])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A115F604BF\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Mar 2022 09:50:52 +0200 (CEST)","by mail-lf1-x130.google.com with SMTP id z12so9451486lfu.10\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 28 Mar 2022 00:50:52 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1648453853;\n\tbh=5A7Ayr9aqfRou3qdt9+aWetzkQ7aZzqg8V895N5CSsk=;\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=L4o/nS4WunBK19Q3qiGGJuTo38rP5jhpocfpjqkOuS1uRnf70R6vZQe8MlbJ9Z1MW\n\tev+BFIx5a/WtDvUHgnt/T9qElPNaIqqbJGUVh9h4EURdpPEAraAtsTFIF3HpyCI7KS\n\tvu8iEJKMxQLyd08b3vMXBFaS2QOh7Ma0DkBUKVMVYN5iz1C3xLTS7GOtN4K8z3/lDJ\n\tOnUeYH1LlM9tIAheWB2JJ2WMbnYfmd76oUlEtJaCfc1MxQQFebqQ9xPTlcRLanJj4w\n\tX5luNeSTwuGR4QLI0wgOh+yqcKA41/b2ACDQn8HflqDAnc6RuYYLm09WJ+8jVqawj5\n\tFdod5jxEOlUvw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=5lOK9PesZkKizCK35ZeI0pee77q8umY38urgjfNna6w=;\n\tb=MK+G4GADOnVy2eO0kqNmbF7Nwt695x7uTznv5SX/sof9lZYzDRMpaE5BzvL9Bc2bSd\n\ttsA96CtwUNmUBkfrVuYqLqZ1cCuTo0gu8BvUHutIb1Y6df+jZY9jN1uTP4zrUfhqUd9G\n\tqiwnsuIIwh+L6Eo9hG1UrXmnOKdwL1fjyviJ+TqhM0OM1BGiFmJHXMvR+2Y4W7uqO0WS\n\tnitCjGRlWVzUOuhAWcN5cdsBIQ8OctsUFy0x2SiexFsAt/d7JBP1P5um0zrIVCiydKdv\n\tp2OZI4z0daT+uQ4bowo2cd6UH9GWURe+687T6uQv7cwfGRoYHxTIG5bC9+rGLRwtBNzq\n\tB1fw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"MK+G4GAD\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=5lOK9PesZkKizCK35ZeI0pee77q8umY38urgjfNna6w=;\n\tb=KEvXOEN8w0rfF/BpMyftOeQkuY1B1EhMbtnD/YAZ3qWnGMckvQ3EN03y+2aVMACspn\n\twMjoUa73Mden5xgRFQcF2KsrrqSjupCInVyjn6e74rE4MzXSQIn8ZjhZeLT7VMyL6msf\n\t5KFAI4fZJ/xL2W6ys9uGDdi0K6qFXXRSex8KLetfVlIFFkCIlIjAYb+1XvDIqXSlcQ+P\n\tFobwyi9pEAezAPAiJWpVEoOh0339XQQCmkxjHaRD0j9vfM8a9iVSwhqZ4qSSvO0WkxG3\n\t1NJXCA4tyRHi+aD/0CzCW2n/itVe0kNLnVst9+1N5MVha+daAp8zHndtUsY6Ybin7Yoo\n\th3bw==","X-Gm-Message-State":"AOAM530Lb+3D2TEdgm0cM1sCWeYi7ATI+pDe8AP3TfBChfdvH3676WyW\n\tHcoM64X6uiydzpchXH3htVBqgj5RIqbpM5Im9j2Sxw==","X-Google-Smtp-Source":"ABdhPJydOTynj7UoUXRx1iDg/RICuMMJb/dnLZcG+9QXbFz3I+A+mJF3CemOo9lGKldz0MHNq6vUlzOsup5O9rkF8DY=","X-Received":"by 2002:a05:6512:1585:b0:448:3936:a5a0 with SMTP id\n\tbp5-20020a056512158500b004483936a5a0mr18392561lfb.108.1648453851882;\n\tMon, 28 Mar 2022 00:50:51 -0700 (PDT)","MIME-Version":"1.0","References":"<20220323160145.90606-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220323160145.90606-4-jeanmichel.hautbois@ideasonboard.com>\n\t<164807967161.1103026.16374791177432451797@Monstersaurus>\n\t<34e3a747-63d2-961e-6e86-0a1ced6a95b2@ideasonboard.com>\n\t<CAEmqJPr2-FeMw27qJK3hjCGB1BqxnFsMxCjo98bZ87YDaDP_VA@mail.gmail.com>\n\t<YkDetOIiu+fhTE9A@pendragon.ideasonboard.com>","In-Reply-To":"<YkDetOIiu+fhTE9A@pendragon.ideasonboard.com>","Date":"Mon, 28 Mar 2022 07:50:36 +0000","Message-ID":"<CAEmqJPpWhRNEVTSG9RBfvH=MSw5Otb7dmPzHLK1H6Bt5Z_5Pag@mail.gmail.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Content-Type":"multipart/alternative; boundary=\"0000000000004534b305db42924d\"","Subject":"Re: [libcamera-devel] [RFC PATCH v2 3/4] libcamera: raspberrypi:\n\tControl the lens from pipeline","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Naushir Patuck via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Naushir Patuck <naush@raspberrypi.com>","Cc":"libcamera devel <libcamera-devel@lists.libcamera.org>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]