[{"id":22302,"web_url":"https://patchwork.libcamera.org/comment/22302/","msgid":"<164751127339.123014.6958608925434044563@Monstersaurus>","date":"2022-03-17T10:01:13","subject":"Re: [libcamera-devel] [PATCH v2 2/2] ipa: raspberrypi: Control the\n\tlens position","submitter":{"id":97,"url":"https://patchwork.libcamera.org/api/people/97/","name":"Nicolas Dufresne via libcamera-devel","email":"libcamera-devel@lists.libcamera.org"},"content":"Hi JM\n\nQuoting Jean-Michel Hautbois via libcamera-devel (2022-03-17 09:19:38)\n> Now that the ancillary links are configured, we can use the CameraLens\n> class and control the VCM through the IPA.\n> For now, force a default value for the lens position, until the AF\n> algorithm is introduced.\n> \n> Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>\n> ---\n>  src/ipa/raspberrypi/raspberrypi.cpp | 41 ++++++++++++++++++++++++++++-\n>  1 file changed, 40 insertions(+), 1 deletion(-)\n> \n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index fd8fecb0..9e66f8fd 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -108,6 +108,7 @@ private:\n>         void setMode(const IPACameraSensorInfo &sensorInfo);\n>         bool validateSensorControls();\n>         bool validateIspControls();\n> +       bool validateLensControls();\n>         void queueRequest(const ControlList &controls);\n>         void returnEmbeddedBuffer(unsigned int bufferId);\n>         void prepareISP(const ipa::RPi::ISPConfig &data);\n> @@ -132,6 +133,7 @@ private:\n>  \n>         ControlInfoMap sensorCtrls_;\n>         ControlInfoMap ispCtrls_;\n> +       ControlInfoMap lensCtrls_;\n>         ControlList libcameraMetadata_;\n>  \n>         /* Camera sensor params. */\n> @@ -342,7 +344,7 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,\n>                       const ipa::RPi::IPAConfig &ipaConfig,\n>                       ControlList *controls)\n>  {\n> -       if (entityControls.size() != 2) {\n> +       if (entityControls.size() < 2) {\n>                 LOG(IPARPI, Error) << \"No ISP or sensor controls found.\";\n>                 return -1;\n>         }\n> @@ -360,6 +362,14 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,\n>                 return -1;\n>         }\n>  \n> +       /* Lens may not be present, don't make it an hard assumption. */\n> +       auto lensControl = entityControls.find(2);\n\nentityControls is already a map for std::map<unsigned int, ControlInfoMap>\n\nThis would be a lot clearer if there was an enum defining what the int\nmeans.\n\nenum entityControlType {\n\tSensorControls = 0,\n\tISPControls = 1,\n\tLensControls = 2,\n};\n\nI presume it's then possible to make the map a \n\tstd::map<entityControlType, ControlInfoMap>\n\n> +       if (lensControl != entityControls.end()) {\n> +               lensCtrls_ = lensControl->second;\n> +               if (!validateLensControls())\n> +                       LOG(IPARPI, Error) << \"Lens control validation failed.\";\n> +       }\n> +\n>         maxSensorGainCode_ = sensorCtrls_.at(V4L2_CID_ANALOGUE_GAIN).max().get<int32_t>();\n>  \n>         /* Setup a metadata ControlList to output metadata. */\n> @@ -578,6 +588,23 @@ bool IPARPi::validateIspControls()\n>         return true;\n>  }\n>  \n> +bool IPARPi::validateLensControls()\n> +{\n> +       static const uint32_t ctrls[] = {\n> +               V4L2_CID_FOCUS_ABSOLUTE,\n> +       };\n> +\n> +       for (auto c : ctrls) {\n> +               if (lensCtrls_.find(c) == lensCtrls_.end()) {\n> +                       LOG(IPARPI, Error) << \"Unable to find lens control \"\n> +                                          << utils::hex(c);\n> +                       return false;\n> +               }\n> +       }\n> +\n> +       return true;\n> +}\n> +\n>  /*\n>   * Converting between enums (used in the libcamera API) and the names that\n>   * we use to identify different modes. Unfortunately, the conversion tables\n> @@ -1066,6 +1093,18 @@ void IPARPi::processStats(unsigned int bufferId)\n>  \n>                 setDelayedControls.emit(ctrls);\n>         }\n> +\n> +       /*\n> +        * Set the focus position\n> +        * \\todo Use an AF algorithm and replace the arbitrary value\n> +        */\n> +       struct FocusStatus focusStatus;\n> +       if (rpiMetadata_.Get(\"focus.status\", focusStatus) == 0) {\n> +               ControlList lensCtrls(lensCtrls_);\n> +               lensCtrls.set(V4L2_CID_FOCUS_ABSOLUTE, static_cast<int32_t>(123));\n> +               setLensControls.emit(lensCtrls);\n\nI think we should probably introduce an algorithm to determine a value\nbefore we set one.\n\nI can't see a reason to set an aribtrary value here right now except as\nan example, which posting this patch has provided - but I wouldn't like\nto merge this as is, I'd prefer to see this rebased on top of the\nalgorithm being posted first, even if simplistic, and I assume that's\nnot far off.\n\n--\nKieran\n\n\n> +       }\n> +\n>  }\n>  \n>  void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\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 9E990BF415\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu, 17 Mar 2022 10:01:18 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CF67C604E7;\n\tThu, 17 Mar 2022 11:01:17 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 33BB5604DB\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu, 17 Mar 2022 11:01:16 +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 BB461493;\n\tThu, 17 Mar 2022 11:01:15 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1647511277;\n\tbh=ncxlM2Fy1a+0pJ/nxdxiz5DJEkp8P9Md5KXUqz30Lys=;\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=n0fEsHBlrI/diS5q5lrKDU5YcuSj7LteMSl/6y5EzD7qJYCzbBuUlTZaczojM5gAe\n\t8zLGSNO8PYFU/P5+vSB+BARScdKrAlarZzRQm73469gyAvVbFO4HWlAXQqyVi2rxwG\n\tP5twE/HaEGJweGcLPthZOS7TRyxKJySCHPlvePWPQZ+yAB5JilDl5ANPZgvEaQPD/q\n\tchRHFZdVqJ8mKZk6Y+IoyTCDrcz6DbKUL/73+osgJIc0a9+TC4IsyL2RT2dY+Cm+Ki\n\tQVJsznNXGEflQVKR8jUaAldplsYa9fUEH05JnSC13yfPhBIWQ0GgxKWfQu0+qgZ67h\n\tNDKA++Jii0K/g==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1647511275;\n\tbh=ncxlM2Fy1a+0pJ/nxdxiz5DJEkp8P9Md5KXUqz30Lys=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=Co+gsKIxu/KDsyudqUHaectRINEPnpxgO+55G20GbNJTtTMlXixBVYuzO2KkCu3bf\n\tQCdXaG9DJtwTP5ejaxIZUuRrWI/uWAFB0YbZ7tokGwgLRSeB2wYlTXFz5opFdZ1JdQ\n\teF1e5vtyZhFDh0CzQ20z9l4qGcZwDmNFXAdWaJA0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Co+gsKIx\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20220317091937.168527-2-jeanmichel.hautbois@ideasonboard.com>","References":"<20220317091937.168527-1-jeanmichel.hautbois@ideasonboard.com>\n\t<20220317091937.168527-2-jeanmichel.hautbois@ideasonboard.com>","To":"Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 17 Mar 2022 10:01:13 +0000","Message-ID":"<164751127339.123014.6958608925434044563@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] ipa: raspberrypi: Control the\n\tlens position","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>"}}]