[{"id":26680,"web_url":"https://patchwork.libcamera.org/comment/26680/","msgid":"<20230320192517.dd5giien2b7bf4fz@uno.localdomain>","date":"2023-03-20T19:25:17","subject":"Re: [libcamera-devel] [PATCH v4 02/10] ipa: rkisp1: Add lens limits\n\tto the session config","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Daniel\n\nOn Tue, Mar 14, 2023 at 03:48:26PM +0100, Daniel Semkowicz wrote:\n> Add information about focus lens position limits to the IPA session\n> configuration. These information can then be used by IPA algorithms\n> to know which focus positions are valid.\n>\n> Signed-off-by: Daniel Semkowicz \n> ---\n> src/ipa/rkisp1/ipa_context.cpp | 12 ++++++++++++\n> src/ipa/rkisp1/ipa_context.h | 5 +++++\n> src/ipa/rkisp1/rkisp1.cpp | 13 ++++++++++++-\n> 3 files changed, 29 insertions(+), 1 deletion(-)\n>\n> diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> index 9bbf3684..401c098f 100644\n> --- a/src/ipa/rkisp1/ipa_context.cpp\n> +++ b/src/ipa/rkisp1/ipa_context.cpp\n> @@ -89,6 +89,18 @@ namespace libcamera::ipa::rkisp1 {\n> * \\brief Sensor output resolution\n> */\n>\n> +/**\n> + * \\var IPASessionConfiguration::lens\n> + * \\brief Lens-specific information\n> + *\n> + * \\var IPASessionConfiguration::lens.minFocusPosition\n> + * \\brief Minimum position supported by the camera focus lens\n> + *\n> + * \\var IPASessionConfiguration::lens.maxFocusPosition\n> + * \\brief Maximum position supported by the camera focus lens\n\nThis doesn't report units, but the ::sensor fields don't either\n\n> + *\n\nAdditional blank line\n\n> + */\n> +\n> /**\n> * \\var IPASessionConfiguration::raw\n> * \\brief Indicates if the camera is configured to capture raw frames\n> diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> index b9b20653..bfb6e1b7 100644\n> --- a/src/ipa/rkisp1/ipa_context.h\n> +++ b/src/ipa/rkisp1/ipa_context.h\n> @@ -45,6 +45,11 @@ struct IPASessionConfiguration {\n> \t\tSize size;\n> \t} sensor;\n>\n> +\tstruct {\n> +\t\tint32_t minFocusPosition;\n> +\t\tint32_t maxFocusPosition;\n> +\t} lens;\n> +\n> \tstruct {\n> \t\trkisp1_cif_isp_version revision;\n> \t} hw;\n> diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> index 248cf5e0..cd1fbae3 100644\n> --- a/src/ipa/rkisp1/rkisp1.cpp\n> +++ b/src/ipa/rkisp1/rkisp1.cpp\n> @@ -267,9 +267,20 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,\n> \t\t\treturn format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> \t\t});\n>\n> -\tif (!ipaConfig.lensControls.empty())\n> +\tif (!ipaConfig.lensControls.empty()) {\n> \t\tlensControls_ = ipaConfig.lensControls;\n>\n> +\t\tconst ControlInfo focusAbsolute =\n> +\t\t\tlensControls_->find(V4L2_CID_FOCUS_ABSOLUTE)->second;\n\nCan we assume V4L2_CID_FOCUS_ABSOLUTE is there ? I would fail, as it\nseems to me neither the CameraSensor nor the pipeline make sure the\ncontrol is there..\n\n> +\n> +\t\tLOG(IPARkISP1, Debug)\n> +\t\t\t<< \"Focus lens: \" << focusAbsolute.toString();\n> +\n> +\t\tauto &lensConfig = context_.configuration.lens;\n> +\t\tlensConfig.minFocusPosition = focusAbsolute.min().get();\n> +\t\tlensConfig.maxFocusPosition = focusAbsolute.max().get();\n> +\t}\n> +\n> \tfor (auto const &a : algorithms()) {\n> \t\tAlgorithm *algo = static_cast(a.get());\n>\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 0F49EC0F2A\n\tfor ;\n\tMon, 20 Mar 2023 19:25:41 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 177B6626E3;\n\tMon, 20 Mar 2023 20:25:40 +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 B14C761ED4\n\tfor ;\n\tMon, 20 Mar 2023 20:25:38 +0100 (CET)","from ideasonboard.com (host-87-18-61-243.retail.telecomitalia.it\n\t[87.18.61.243])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3BADC832;\n\tMon, 20 Mar 2023 20:25:34 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679340340;\n\tbh=r6WvG+rVep1L+5UXUNKiwZ8OcXNrw7a8ZV81VJHCNoE=;\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=kmBRIFAHHTBj6SgmmJqHE7TbtgsezVZMrFcBGOASAmHNZT9tIHRT5fROHoBL0zGqS\n\tb9QRJhoGwzXBdVH5o09Ivb/2d58BfQjbr1ScYmoUH8w/8JsXu4GWgHbpJqXzQOCLxh\n\tSyNwgc6wL5k9IoCMLTGDjNLBVDuZxcJE98O2mGUpJmA6kGhxPK6b7VhZBOy+1ZMExn\n\tGQurgKxHiyXSHkC6+6kWk2/k1Hb5AKUM2UHBDzTJXrRVNRJoShLfqt6vvLvN9V8TuH\n\tOfSFk7VZ6nHDBr6JiaiKEHuF0HxIEQDLr2NIPqAn/6i0OPpd7K72mYuGgMfanlM9Ym\n\t12Mj+no0yjkrg==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679340338;\n\tbh=r6WvG+rVep1L+5UXUNKiwZ8OcXNrw7a8ZV81VJHCNoE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GTgQI/t6gus0zMRZ6RgPV6uglB+RMI/AHwuYkM/e2sW1PJQpFKbSWTo9zXf5PxeVu\n\tuFM+nscXDELvtWy+bBp444fPIF4jR+d4HLfkGXSPUOjd1MZvFVBpSwrvZuI0b+o2eK\n\tcHcpymKexCl8nFfuiPD6Vms0wPppVAtuwPV4YuT0="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"GTgQI/t6\"; dkim-atps=neutral","Date":"Mon, 20 Mar 2023 20:25:17 +0100","To":"Daniel Semkowicz ","Message-ID":"<20230320192517.dd5giien2b7bf4fz@uno.localdomain>","References":"<20230314144834.85193-1-dse@thaumatec.com>\n\t<20230314144834.85193-3-dse@thaumatec.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20230314144834.85193-3-dse@thaumatec.com>","Subject":"Re: [libcamera-devel] [PATCH v4 02/10] ipa: rkisp1: Add lens limits\n\tto the session config","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":26689,"web_url":"https://patchwork.libcamera.org/comment/26689/","msgid":"","date":"2023-03-21T06:42:53","subject":"Re: [libcamera-devel] [PATCH v4 02/10] ipa: rkisp1: Add lens limits\n\tto the session config","submitter":{"id":126,"url":"https://patchwork.libcamera.org/api/people/126/","name":"Daniel Semkowicz","email":"dse@thaumatec.com"},"content":"Hi Jacopo,\n\nThank you for spending your time on the review :)\n\nOn Mon, Mar 20, 2023 at 8:25 PM Jacopo Mondi\n wrote:\n>\n> Hi Daniel\n>\n> On Tue, Mar 14, 2023 at 03:48:26PM +0100, Daniel Semkowicz wrote:\n> > Add information about focus lens position limits to the IPA session\n> > configuration. These information can then be used by IPA algorithms\n> > to know which focus positions are valid.\n> >\n> > Signed-off-by: Daniel Semkowicz \n> > ---\n> > src/ipa/rkisp1/ipa_context.cpp | 12 ++++++++++++\n> > src/ipa/rkisp1/ipa_context.h | 5 +++++\n> > src/ipa/rkisp1/rkisp1.cpp | 13 ++++++++++++-\n> > 3 files changed, 29 insertions(+), 1 deletion(-)\n> >\n> > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > index 9bbf3684..401c098f 100644\n> > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > @@ -89,6 +89,18 @@ namespace libcamera::ipa::rkisp1 {\n> > * \\brief Sensor output resolution\n> > */\n> >\n> > +/**\n> > + * \\var IPASessionConfiguration::lens\n> > + * \\brief Lens-specific information\n> > + *\n> > + * \\var IPASessionConfiguration::lens.minFocusPosition\n> > + * \\brief Minimum position supported by the camera focus lens\n> > + *\n> > + * \\var IPASessionConfiguration::lens.maxFocusPosition\n> > + * \\brief Maximum position supported by the camera focus lens\n>\n> This doesn't report units, but the ::sensor fields don't either\n\nI am just exposing raw values from the v4l subnode, so it is not even\npossible to know if any unit is assigned to the values.\nAnd until we have a standardized focus values in libcamera, it will be\ndifficult to do it differently.\n\n>\n> > + *\n>\n> Additional blank line\n>\n> > + */\n> > +\n> > /**\n> > * \\var IPASessionConfiguration::raw\n> > * \\brief Indicates if the camera is configured to capture raw frames\n> > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > index b9b20653..bfb6e1b7 100644\n> > --- a/src/ipa/rkisp1/ipa_context.h\n> > +++ b/src/ipa/rkisp1/ipa_context.h\n> > @@ -45,6 +45,11 @@ struct IPASessionConfiguration {\n> > Size size;\n> > } sensor;\n> >\n> > + struct {\n> > + int32_t minFocusPosition;\n> > + int32_t maxFocusPosition;\n> > + } lens;\n> > +\n> > struct {\n> > rkisp1_cif_isp_version revision;\n> > } hw;\n> > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > index 248cf5e0..cd1fbae3 100644\n> > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > @@ -267,9 +267,20 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,\n> > return format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> > });\n> >\n> > - if (!ipaConfig.lensControls.empty())\n> > + if (!ipaConfig.lensControls.empty()) {\n> > lensControls_ = ipaConfig.lensControls;\n> >\n> > + const ControlInfo focusAbsolute =\n> > + lensControls_->find(V4L2_CID_FOCUS_ABSOLUTE)->second;\n>\n> Can we assume V4L2_CID_FOCUS_ABSOLUTE is there ? I would fail, as it\n> seems to me neither the CameraSensor nor the pipeline make sure the\n> control is there..\n\nThere is a CameraLens::validateLensDriver() function to assure there\nis a V4L2_CID_FOCUS_ABSOLUTE control, and it's called on\nCameraLens::init(). If CameraLens::init() called in the\nCameraSensor::discoverAncillaryDevices() returns error, the focusLens_\npointer is reset. This means the focusLens_ pointer in CameraLens is\nset only if lens have a V4L2_CID_FOCUS_ABSOLUTE control. I base on this\nbehaviour to expose lens controls from pipeline to the IPA only if the\npointer was set. It is a bit hidden under several layers, but I tried\nto avoid checking the same thing in different places.\n\n>\n> > +\n> > + LOG(IPARkISP1, Debug)\n> > + << \"Focus lens: \" << focusAbsolute.toString();\n> > +\n> > + auto &lensConfig = context_.configuration.lens;\n> > + lensConfig.minFocusPosition = focusAbsolute.min().get();\n> > + lensConfig.maxFocusPosition = focusAbsolute.max().get();\n> > + }\n> > +\n> > for (auto const &a : algorithms()) {\n> > Algorithm *algo = static_cast(a.get());\n> >\n> > --\n> > 2.39.2\n> >\n\nBest regards\nDaniel","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 8A0D3C0F1B\n\tfor ;\n\tTue, 21 Mar 2023 06:43:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 57206626E3;\n\tTue, 21 Mar 2023 07:43:06 +0100 (CET)","from mail-ed1-x536.google.com (mail-ed1-x536.google.com\n\t[IPv6:2a00:1450:4864:20::536])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9220E61ECE\n\tfor ;\n\tTue, 21 Mar 2023 07:43:04 +0100 (CET)","by mail-ed1-x536.google.com with SMTP id ek18so55689799edb.6\n\tfor ;\n\tMon, 20 Mar 2023 23:43:04 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679380986;\n\tbh=cEGr+JCRLkL6x3a1cECuKpTPw1N7b+4M22xRPikqpfo=;\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=oAsqRoCSu9b3HqLKc/uEp+huCNgAf4oSXYw2Frj+AD27UmuO8eZwfljwHViok0yUG\n\t3KOd8FPhr/hI2DFF/GLvgdN5INMhE0J03mfni1OxhNOaWdTjZFUwzRIATtS+zC2+UU\n\tM3mW76L0fDRR1dX/qcYN4OZY1+kSoapwLZn8iuzaAAufLuxYLcFyuCcXNteLOJtIIb\n\t9RKvqQ6VhC5AXTrUY55LqliBZrPerNhDinOZjkfkmgDfWuiMmd9UqesPco4l4cX/4o\n\t8hN8IEP5TcQSIU5fiQsILj243MpWHqF28GdNO7ye0q3GV3CNb5xW5O5MpWbfC0XIn9\n\tybkrBmAIwEtPg==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=thaumatec-com.20210112.gappssmtp.com; s=20210112; t=1679380984;\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=7XEpZdYQuRzmscATd9qqezC/kce+OulUW1eNl0ZKXWE=;\n\tb=jqi2LKfpPfe++CfhCLhBTfqsb2irz0KIYOd3bfnA9DQRfyG5owvhOqCxK34uiNGVzF\n\tllBZzVzcCqjzRZoa6pHW0WIQHjqAU3dI/44gsiP2yWGvNq8vF/xhfjn0tIhcXYiUv0j2\n\tg95b5zeN0KVv2F94JfthLpxl5umEz+KZzfOL5w+zNw5XtsUZf/jAXzP4DCdFSriWXgWi\n\t0Kro8G76RPVxi8obLkPTVXiGLlnAtF67/JcpdxgPk8baQKfksdnMy85nEIblQrMSVyjW\n\tP5GPfyySbCD/nJURYYkRygMUObJEX8mdfO+Td9FuQoYLD0HiaBY3pWvi4tNvHml9whih\n\t2VCQ=="],"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=\"jqi2LKfp\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112; t=1679380984;\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=7XEpZdYQuRzmscATd9qqezC/kce+OulUW1eNl0ZKXWE=;\n\tb=MhOlw2VQyjA7igBinQxd90pnLYK9LV9HJcCNoXuFzu+FCwleJNi1WOyU9LxBaIaYnC\n\tDN07BhC6qRVLEsCw+O9+tRuRcS4nUC7he93EnxKEIcyLmcSyK27wzOzyggPi+ZTz+mKg\n\tXZ+bNADpzcTLs+hkQX9uhCSOP7kvvGweA6CKXWszFLhDD2BzWuZPQ04iFCqd4/tm3qFF\n\t0HvnftwyxIhOMz0VOytduikVClPPN2JbofYAmewoyLO4nJKSJGevrONuV5vli5Gvn0rG\n\t/N78d6OVeJTAZ11Mhl2NAGwKB07VbXsymUJBvFvgV5/N/QP7J4K3Q5141EFN02CGKYay\n\tRdFQ==","X-Gm-Message-State":"AO0yUKUproNmI08JfzFn81WS3ruajlXf86AROneNeJqvaQR9NW1m46KJ\n\tnmy5LtgQulr8c06vvN4JrRiTQMjmSAZXzbJvbG7hyg==","X-Google-Smtp-Source":"AK7set9Gkf7Oww0KPS4TUJ7SUZujUMVB3DQCzeAAWjv3xEdjBcdckkeNQvMTtQVA27BYIxUQIAJy3ysXP0sMeyIQYBA=","X-Received":"by 2002:a17:906:2e09:b0:878:4a24:1a5c with SMTP id\n\tqq9-20020a1709062e0900b008784a241a5cmr764758ejc.6.1679380984023;\n\tMon, 20 Mar 2023 23:43:04 -0700 (PDT)","MIME-Version":"1.0","References":"<20230314144834.85193-1-dse@thaumatec.com>\n\t<20230314144834.85193-3-dse@thaumatec.com>\n\t<20230320192517.dd5giien2b7bf4fz@uno.localdomain>","In-Reply-To":"<20230320192517.dd5giien2b7bf4fz@uno.localdomain>","Date":"Tue, 21 Mar 2023 07:42:53 +0100","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v4 02/10] ipa: rkisp1: Add lens limits\n\tto the session config","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":26693,"web_url":"https://patchwork.libcamera.org/comment/26693/","msgid":"<20230321080523.dipntx7kmnoj3rei@uno.localdomain>","date":"2023-03-21T08:05:23","subject":"Re: [libcamera-devel] [PATCH v4 02/10] ipa: rkisp1: Add lens limits\n\tto the session config","submitter":{"id":143,"url":"https://patchwork.libcamera.org/api/people/143/","name":"Jacopo Mondi","email":"jacopo.mondi@ideasonboard.com"},"content":"Hi Daniel\n\nOn Tue, Mar 21, 2023 at 07:42:53AM +0100, Daniel Semkowicz wrote:\n> Hi Jacopo,\n>\n> Thank you for spending your time on the review :)\n>\n> On Mon, Mar 20, 2023 at 8:25 PM Jacopo Mondi\n> wrote:\n> >\n> > Hi Daniel\n> >\n> > On Tue, Mar 14, 2023 at 03:48:26PM +0100, Daniel Semkowicz wrote:\n> > > Add information about focus lens position limits to the IPA session\n> > > configuration. These information can then be used by IPA algorithms\n> > > to know which focus positions are valid.\n> > >\n> > > Signed-off-by: Daniel Semkowicz \n> > > ---\n> > > src/ipa/rkisp1/ipa_context.cpp | 12 ++++++++++++\n> > > src/ipa/rkisp1/ipa_context.h | 5 +++++\n> > > src/ipa/rkisp1/rkisp1.cpp | 13 ++++++++++++-\n> > > 3 files changed, 29 insertions(+), 1 deletion(-)\n> > >\n> > > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > > index 9bbf3684..401c098f 100644\n> > > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > > @@ -89,6 +89,18 @@ namespace libcamera::ipa::rkisp1 {\n> > > * \\brief Sensor output resolution\n> > > */\n> > >\n> > > +/**\n> > > + * \\var IPASessionConfiguration::lens\n> > > + * \\brief Lens-specific information\n> > > + *\n> > > + * \\var IPASessionConfiguration::lens.minFocusPosition\n> > > + * \\brief Minimum position supported by the camera focus lens\n> > > + *\n> > > + * \\var IPASessionConfiguration::lens.maxFocusPosition\n> > > + * \\brief Maximum position supported by the camera focus lens\n> >\n> > This doesn't report units, but the ::sensor fields don't either\n>\n> I am just exposing raw values from the v4l subnode, so it is not even\n> possible to know if any unit is assigned to the values.\n> And until we have a standardized focus values in libcamera, it will be\n> difficult to do it differently.\n>\n> >\n> > > + *\n> >\n> > Additional blank line\n> >\n> > > + */\n> > > +\n> > > /**\n> > > * \\var IPASessionConfiguration::raw\n> > > * \\brief Indicates if the camera is configured to capture raw frames\n> > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > index b9b20653..bfb6e1b7 100644\n> > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > @@ -45,6 +45,11 @@ struct IPASessionConfiguration {\n> > > Size size;\n> > > } sensor;\n> > >\n> > > + struct {\n> > > + int32_t minFocusPosition;\n> > > + int32_t maxFocusPosition;\n> > > + } lens;\n> > > +\n> > > struct {\n> > > rkisp1_cif_isp_version revision;\n> > > } hw;\n> > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > index 248cf5e0..cd1fbae3 100644\n> > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > @@ -267,9 +267,20 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,\n> > > return format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> > > });\n> > >\n> > > - if (!ipaConfig.lensControls.empty())\n> > > + if (!ipaConfig.lensControls.empty()) {\n> > > lensControls_ = ipaConfig.lensControls;\n> > >\n> > > + const ControlInfo focusAbsolute =\n> > > + lensControls_->find(V4L2_CID_FOCUS_ABSOLUTE)->second;\n> >\n> > Can we assume V4L2_CID_FOCUS_ABSOLUTE is there ? I would fail, as it\n> > seems to me neither the CameraSensor nor the pipeline make sure the\n> > control is there..\n>\n> There is a CameraLens::validateLensDriver() function to assure there\n> is a V4L2_CID_FOCUS_ABSOLUTE control, and it's called on\n> CameraLens::init(). If CameraLens::init() called in the\n> CameraSensor::discoverAncillaryDevices() returns error, the focusLens_\n> pointer is reset. This means the focusLens_ pointer in CameraLens is\n> set only if lens have a V4L2_CID_FOCUS_ABSOLUTE control. I base on this\n> behaviour to expose lens controls from pipeline to the IPA only if the\n> pointer was set. It is a bit hidden under several layers, but I tried\n> to avoid checking the same thing in different places.\n>\n\nGood point, you're right, please ignore the comment then ;)\n\n> >\n> > > +\n> > > + LOG(IPARkISP1, Debug)\n> > > + << \"Focus lens: \" << focusAbsolute.toString();\n> > > +\n> > > + auto &lensConfig = context_.configuration.lens;\n> > > + lensConfig.minFocusPosition = focusAbsolute.min().get();\n> > > + lensConfig.maxFocusPosition = focusAbsolute.max().get();\n> > > + }\n> > > +\n> > > for (auto const &a : algorithms()) {\n> > > Algorithm *algo = static_cast(a.get());\n> > >\n> > > --\n> > > 2.39.2\n> > >\n>\n> Best regards\n> Daniel","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 80D2EC0F1B\n\tfor ;\n\tTue, 21 Mar 2023 08:05:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 91EDC626E3;\n\tTue, 21 Mar 2023 09:05:28 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 87070626D8\n\tfor ;\n\tTue, 21 Mar 2023 09:05:26 +0100 (CET)","from ideasonboard.com (host-87-18-61-243.retail.telecomitalia.it\n\t[87.18.61.243])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B9541496;\n\tTue, 21 Mar 2023 09:05:25 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679385928;\n\tbh=4ZehIgTmwiLYZ9N7tFBm5mqUCvx2hl+wecm1mfb94Y4=;\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=Mkxvpd6BtY3Vd4505YfdB9Su1NZYRr/rt0Tec/KwkfOQlB5IwJXiids9EZkw7Gsp3\n\tMP4pxKXPrpccUwQgUqXxLtok2KPSp02iph75n89y3oGnSeReXAiYpEgQF5zK67mM0T\n\tuFwqQxFzJYuGurhe1VamFSXG3TcmBmYG/gn6MfYg3mC5V6skEDzIKyAsk3jdYko+Q+\n\td8EPhcW+FAG8lyfOsZ8rBthhu8BZmaD9PK4fNnt7oKWE4qEiC8UBMTXEgeyAt2dHB5\n\tdvUlf+0TUVRvTeZztmrCNFtIx5Y1n0ZYVOLVZwMbdFiSfcu29RbF3YVu6PjxlmY+wj\n\tfmN7+x7Db7F0A==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1679385926;\n\tbh=4ZehIgTmwiLYZ9N7tFBm5mqUCvx2hl+wecm1mfb94Y4=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=KXQbJN751wCtNX5V0+AXKWGnrsHK13Qmity4Qm0Oz8IfKN1B9dfBJ/I4m5+5bWb5K\n\tHwchJcPAvwaQArn0lIZWXr13btBHZcMxBJpAdhuLsDOC3DxuVSUE/lZG463fH/Aj/a\n\tT9dxpb/pr9dVBb7/UfVqMsnZ2sHH9DMiOfbs3pzM="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"KXQbJN75\"; dkim-atps=neutral","Date":"Tue, 21 Mar 2023 09:05:23 +0100","To":"Daniel Semkowicz ","Message-ID":"<20230321080523.dipntx7kmnoj3rei@uno.localdomain>","References":"<20230314144834.85193-1-dse@thaumatec.com>\n\t<20230314144834.85193-3-dse@thaumatec.com>\n\t<20230320192517.dd5giien2b7bf4fz@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 v4 02/10] ipa: rkisp1: Add lens limits\n\tto the session config","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":26719,"web_url":"https://patchwork.libcamera.org/comment/26719/","msgid":"","date":"2023-03-24T06:58:35","subject":"Re: [libcamera-devel] [PATCH v4 02/10] ipa: rkisp1: Add lens limits\n\tto the session config","submitter":{"id":126,"url":"https://patchwork.libcamera.org/api/people/126/","name":"Daniel Semkowicz","email":"dse@thaumatec.com"},"content":"Hi Jacopo,\n\nOn Tue, Mar 21, 2023 at 9:05 AM Jacopo Mondi\n wrote:\n>\n> Hi Daniel\n>\n> On Tue, Mar 21, 2023 at 07:42:53AM +0100, Daniel Semkowicz wrote:\n> > Hi Jacopo,\n> >\n> > Thank you for spending your time on the review :)\n> >\n> > On Mon, Mar 20, 2023 at 8:25 PM Jacopo Mondi\n> > wrote:\n> > >\n> > > Hi Daniel\n> > >\n> > > On Tue, Mar 14, 2023 at 03:48:26PM +0100, Daniel Semkowicz wrote:\n> > > > Add information about focus lens position limits to the IPA session\n> > > > configuration. These information can then be used by IPA algorithms\n> > > > to know which focus positions are valid.\n> > > >\n> > > > Signed-off-by: Daniel Semkowicz \n> > > > ---\n> > > > src/ipa/rkisp1/ipa_context.cpp | 12 ++++++++++++\n> > > > src/ipa/rkisp1/ipa_context.h | 5 +++++\n> > > > src/ipa/rkisp1/rkisp1.cpp | 13 ++++++++++++-\n> > > > 3 files changed, 29 insertions(+), 1 deletion(-)\n> > > >\n> > > > diff --git a/src/ipa/rkisp1/ipa_context.cpp b/src/ipa/rkisp1/ipa_context.cpp\n> > > > index 9bbf3684..401c098f 100644\n> > > > --- a/src/ipa/rkisp1/ipa_context.cpp\n> > > > +++ b/src/ipa/rkisp1/ipa_context.cpp\n> > > > @@ -89,6 +89,18 @@ namespace libcamera::ipa::rkisp1 {\n> > > > * \\brief Sensor output resolution\n> > > > */\n> > > >\n> > > > +/**\n> > > > + * \\var IPASessionConfiguration::lens\n> > > > + * \\brief Lens-specific information\n> > > > + *\n> > > > + * \\var IPASessionConfiguration::lens.minFocusPosition\n> > > > + * \\brief Minimum position supported by the camera focus lens\n> > > > + *\n> > > > + * \\var IPASessionConfiguration::lens.maxFocusPosition\n> > > > + * \\brief Maximum position supported by the camera focus lens\n> > >\n> > > This doesn't report units, but the ::sensor fields don't either\n> >\n> > I am just exposing raw values from the v4l subnode, so it is not even\n> > possible to know if any unit is assigned to the values.\n> > And until we have a standardized focus values in libcamera, it will be\n> > difficult to do it differently.\n> >\n> > >\n> > > > + *\n> > >\n> > > Additional blank line\n\nRegarding the additional blank line, it is intentional. For some reason\ndoxygen does not include the last field \"maxFocusPosition\" in the\ndocumetnation without this blank line at the end of the block with\nmultiples fields.\n\nThis behaves the same for other already existing documentation blocks.\nPlease see for example: IPASessionConfiguration::grid.stride from the\nipu3/ipa_context.cpp. Other fields above like \"bdsOutputSize\" are\ncorrectly visible in the doxygen html docs, however stride brief\ndescription is not included.\n\nBest regards\nDaniel\n\n\n> > >\n> > > > + */\n> > > > +\n> > > > /**\n> > > > * \\var IPASessionConfiguration::raw\n> > > > * \\brief Indicates if the camera is configured to capture raw frames\n> > > > diff --git a/src/ipa/rkisp1/ipa_context.h b/src/ipa/rkisp1/ipa_context.h\n> > > > index b9b20653..bfb6e1b7 100644\n> > > > --- a/src/ipa/rkisp1/ipa_context.h\n> > > > +++ b/src/ipa/rkisp1/ipa_context.h\n> > > > @@ -45,6 +45,11 @@ struct IPASessionConfiguration {\n> > > > Size size;\n> > > > } sensor;\n> > > >\n> > > > + struct {\n> > > > + int32_t minFocusPosition;\n> > > > + int32_t maxFocusPosition;\n> > > > + } lens;\n> > > > +\n> > > > struct {\n> > > > rkisp1_cif_isp_version revision;\n> > > > } hw;\n> > > > diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\n> > > > index 248cf5e0..cd1fbae3 100644\n> > > > --- a/src/ipa/rkisp1/rkisp1.cpp\n> > > > +++ b/src/ipa/rkisp1/rkisp1.cpp\n> > > > @@ -267,9 +267,20 @@ int IPARkISP1::configure(const IPAConfigInfo &ipaConfig,\n> > > > return format.colourEncoding == PixelFormatInfo::ColourEncodingRAW;\n> > > > });\n> > > >\n> > > > - if (!ipaConfig.lensControls.empty())\n> > > > + if (!ipaConfig.lensControls.empty()) {\n> > > > lensControls_ = ipaConfig.lensControls;\n> > > >\n> > > > + const ControlInfo focusAbsolute =\n> > > > + lensControls_->find(V4L2_CID_FOCUS_ABSOLUTE)->second;\n> > >\n> > > Can we assume V4L2_CID_FOCUS_ABSOLUTE is there ? I would fail, as it\n> > > seems to me neither the CameraSensor nor the pipeline make sure the\n> > > control is there..\n> >\n> > There is a CameraLens::validateLensDriver() function to assure there\n> > is a V4L2_CID_FOCUS_ABSOLUTE control, and it's called on\n> > CameraLens::init(). If CameraLens::init() called in the\n> > CameraSensor::discoverAncillaryDevices() returns error, the focusLens_\n> > pointer is reset. This means the focusLens_ pointer in CameraLens is\n> > set only if lens have a V4L2_CID_FOCUS_ABSOLUTE control. I base on this\n> > behaviour to expose lens controls from pipeline to the IPA only if the\n> > pointer was set. It is a bit hidden under several layers, but I tried\n> > to avoid checking the same thing in different places.\n> >\n>\n> Good point, you're right, please ignore the comment then ;)\n>\n> > >\n> > > > +\n> > > > + LOG(IPARkISP1, Debug)\n> > > > + << \"Focus lens: \" << focusAbsolute.toString();\n> > > > +\n> > > > + auto &lensConfig = context_.configuration.lens;\n> > > > + lensConfig.minFocusPosition = focusAbsolute.min().get();\n> > > > + lensConfig.maxFocusPosition = focusAbsolute.max().get();\n> > > > + }\n> > > > +\n> > > > for (auto const &a : algorithms()) {\n> > > > Algorithm *algo = static_cast(a.get());\n> > > >\n> > > > --\n> > > > 2.39.2\n> > > >\n> >\n> > Best regards\n> > Daniel","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 2C78BC0F2A\n\tfor ;\n\tFri, 24 Mar 2023 06:58:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 38D356270D;\n\tFri, 24 Mar 2023 07:58:49 +0100 (CET)","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 32D0E603A8\n\tfor ;\n\tFri, 24 Mar 2023 07:58:47 +0100 (CET)","by mail-ed1-x531.google.com with SMTP id eh3so3919093edb.11\n\tfor ;\n\tThu, 23 Mar 2023 23:58:47 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1679641129;\n\tbh=sm0o+XmLR5WQbcaThXNBfmY/O7Km2jGMF7cV4tiu6tE=;\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=gUd4FgSNxvNUybxeSJ4oQJYfBcqIliDpFfAC3kJdOWD3YUwMD+do+IWT+DU71qPhN\n\tFYntFOCWSuHtieTUZS+ibod3aKi74hk/tPd4ZZRB98aoYwpxMBxMpAw/RGruDjGJyu\n\tTwclCElA+tiwdJT/U6feE21CjEGIEhifk34llOffpUqkzw6k0460BA/8bPEfBzgImS\n\tTueb/bNnffolCQd74Cymu2lMUt6/GMs1AFbrxvyiX3lHUCkEigy2My+Ecdo3p2+jYS\n\tw5BF/X6ktkMipplYAvWkqWC9dGaxaB/izXlxl01GLbcyL2bWkUHPJoyKTah96sPMw7\n\t9ZvReUrutQT6Q==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=thaumatec-com.20210112.gappssmtp.com; s=20210112; t=1679641126;\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=SVJng+dNyuw4IUXmp41otSs5cGA9ygVT2W3oS8qm1KA=;\n\tb=aUioZBsqhiq0BdpuCiEIp67HwMP92DMAEyJ3oDvoJW4GWrGmZpENsGPRbe3dLcSDEE\n\twnCoCGhNKNZJ3E7cD3wFONekbic85AdcNM5JMFyTZsUMKg3BnFT54TITJADoMPB332m0\n\tWqmgRIzEOUaSOEhfvrWH3xzmdqujGhL58Ao3P6YqwAQpu1beZtDilqp6WqnGhOwdAAuR\n\t5kn2i/1BRTmAKO/FifT2kuvUTNQMMICVIRYCwkqzHeXU2m7q9nzBjP8uS5yUNalyRbuf\n\tHZjrQtJrpuX0kyltpVqrx+yAYWpCikK5VHFVK4LA/9W7tPyp9nUhPHMLc5ev+V2sYrOq\n\ttqlg=="],"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=\"aUioZBsq\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112; t=1679641126;\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=SVJng+dNyuw4IUXmp41otSs5cGA9ygVT2W3oS8qm1KA=;\n\tb=PBUz8LnxiQzrVhwXI3/udbnsP8wWU9yZXgf6oNG9HIMSG1hk5Mb0wL/+tSmp7n68sE\n\tcayqSAH/u5/y/FI09zxSLxGwlFOWVy2f9gF2m59cTGV5+QMSl0rCE9qNay39+yBrT1g+\n\tOq5tI3XWGiaDzm396NIAdBBtCS+KZnuKRYVT0tABe8BUN/AgWXQAEASPlacnC9tgIoxE\n\tftSD0T0FeQIyELPHvrbzTaQVf6kfSxPTfMDBz5gHS/MD26wkcg3spJc50QPTBroCqOGz\n\tJUvSvtpJpfFclq3yRXApA/Z6C0KCf2VcpIy054oHHD2EHTH/X6KQ+0m64dm0NEQzXnxT\n\tgIUw==","X-Gm-Message-State":"AAQBX9ezTqWkJ99xqHgiWivE41xCt6xUrT50TCaQCBg4U67fI9nf21/r\n\tsYLpcmLug6qCaHZa6yPmDtC19TMvx+RX/tuSdz3AGPPTpcDaQvKyDRA=","X-Google-Smtp-Source":"AKy350bbRlCxIrNUInIzcOFIytwlpOFCtGJpnFD6OP1U2cjPzxJZT6eJJzAlU6F8uV2M3XC/+P/YGSDRAJ05AFmGde8=","X-Received":"by 2002:a50:d543:0:b0:4fb:f19:87f with SMTP id\n\tf3-20020a50d543000000b004fb0f19087fmr944267edj.3.1679641126598; \n\tThu, 23 Mar 2023 23:58:46 -0700 (PDT)","MIME-Version":"1.0","References":"<20230314144834.85193-1-dse@thaumatec.com>\n\t<20230314144834.85193-3-dse@thaumatec.com>\n\t<20230320192517.dd5giien2b7bf4fz@uno.localdomain>\n\t\n\t<20230321080523.dipntx7kmnoj3rei@uno.localdomain>","In-Reply-To":"<20230321080523.dipntx7kmnoj3rei@uno.localdomain>","Date":"Fri, 24 Mar 2023 07:58:35 +0100","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","Subject":"Re: [libcamera-devel] [PATCH v4 02/10] ipa: rkisp1: Add lens limits\n\tto the session config","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\" "}}]