[{"id":25815,"web_url":"https://patchwork.libcamera.org/comment/25815/","msgid":"<166870106589.50677.16904470229012395935@Monstersaurus>","date":"2022-11-17T16:04:25","subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi David,\n\nQuoting David Plowman via libcamera-devel (2022-11-17 15:45:00)\n> The units for the LensPosition control, previously defined as being in\n> units of 1 / hyperfocal_distance, are changed to 1 / distance (in\n> metres).\n> \n> Signed-off-by: David Plowman \n> ---\n> src/libcamera/control_ids.yaml | 26 ++++++++++++++------------\n> 1 file changed, 14 insertions(+), 12 deletions(-)\n> \n> diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> index a456e6c0..adea5f90 100644\n> --- a/src/libcamera/control_ids.yaml\n> +++ b/src/libcamera/control_ids.yaml\n> @@ -591,25 +591,27 @@ controls:\n> AfModeManual, though the value is reported back unconditionally in all\n> modes.\n> \n> - The units are a reciprocal distance scale like dioptres but normalised\n> - for the hyperfocal distance. That is, for a lens with hyperfocal\n> - distance H, and setting it to a focal distance D, the lens position LP,\n> - which is generally a non-integer, is given by\n> + This value, which is generally a non-integer, is the reciprocal of the\n> + focal distance in metres, also known as dioptres. That is, to set a\n> + focal distance D, the lens position LP is given by\n> \n> - \\f$LP = \\frac{H}{D}\\f$\n> + \\f$LP = \\frac{1\\mathrm{m}}{D}\\f$\n> \n> For example:\n> \n> 0 moves the lens to infinity.\n> - 0.5 moves the lens to twice the hyperfocal distance.\n> - 1 moves the lens to the hyperfocal position.\n> - And larger values will focus the lens ever closer.\n> + 0.5 moves the lens to focus on objects 2m away.\n> + 2 moves the lens to focus on objects 50cm away.\n> + And larger values will focus the lens closer.\n> \n> - \\todo Define a property to report the Hyperforcal distance of calibrated\n> - lenses.\n> + The default value of the control should indicate a good general position\n> + for the lens, often corresponding to the hyperfocal distance (the\n> + closest position for which objects at infinity are still acceptably\n> + sharp). The minimum will often be zero (meaning infinity), and the\n> + maximum value defines the closest focus position.\n> \n> - \\todo Define a property to report the maximum and minimum positions of\n> - this lens. The minimum value will often be zero (meaning infinity).\n> + \\todo Define a property to report the Hyperfocal distance of calibrated\n> + lenses.\n\nIs this going to be a separate property, or the 'default' value of the\ncontrol?\n\nIn the cover letter you mentioned \"we've resolved that we will be\nable to read a good default position from the control info.\"\n\nPresumably, that means that when lacking other information, '1' is a\ngood default value?\n\n\n> \n> - AfState:\n> type: int32_t\n> -- \n> 2.30.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 55B87BD16B\n\tfor ;\n\tThu, 17 Nov 2022 16:04:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6F37A6308F;\n\tThu, 17 Nov 2022 17:04:30 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 94E6061F31\n\tfor ;\n\tThu, 17 Nov 2022 17:04:28 +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 D6BF0929;\n\tThu, 17 Nov 2022 17:04:27 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668701070;\n\tbh=TbRMhyskaDOEy0AziwmMUtb7tzMk4Bh0l0IroMbgaks=;\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=sf2OnlBQuRrHULiQ0kgEf7gJCsvXcOKcwr5epBvzZFK/ws83fZUmB8y7lbl/O95EL\n\tCrTLIx2Nbpqc20urybdtWFoRtNM5qbdipaucbbSDoM7BohTmr+NiXgVAkjIVx2oDHg\n\tcQyeGp43klZTGr+m7fNArwr3kHKRg4Aw3Ux/zGxkJeym7MhlFS+4DHrEDiKu7lTmZV\n\tEsS4akSBkR9xb4QQBlKJDFfi2r3ZVw4V8Nd+yk5aXOI0R3URvH8jjKXvMYYjKSyllg\n\twDbuxz03KTMB73njnnWJs4HHkKbUX+gL8wli7hKen7Tr7Y7b9IlmATAFt4PaTsx2PR\n\tIAdsphNTzvx3Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1668701068;\n\tbh=TbRMhyskaDOEy0AziwmMUtb7tzMk4Bh0l0IroMbgaks=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=V8eJnaADSSFnRmep177bL+g6EDPwUB2oAWg3vzp7s9WcnvinLadDNy+WDevxH0U1n\n\t0/AOgDVK/bu9J/JIdrkXCLLtgUrsjA93LKFGsQt1Vv4/s+di/Sz2XUuQB7w3YllwGW\n\t3rv4gGfl3+mNRLSsZyA4KZFVdst404Y4rew9mST8="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"V8eJnaAD\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20221117154500.8829-2-david.plowman@raspberrypi.com>","References":"<20221117154500.8829-1-david.plowman@raspberrypi.com>\n\t<20221117154500.8829-2-david.plowman@raspberrypi.com>","To":"David Plowman ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Thu, 17 Nov 2022 16:04:25 +0000","Message-ID":"<166870106589.50677.16904470229012395935@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25816,"web_url":"https://patchwork.libcamera.org/comment/25816/","msgid":"","date":"2022-11-17T16:11:38","subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Kieran\n\nOn Thu, 17 Nov 2022 at 16:04, Kieran Bingham\n wrote:\n>\n> Hi David,\n>\n> Quoting David Plowman via libcamera-devel (2022-11-17 15:45:00)\n> > The units for the LensPosition control, previously defined as being in\n> > units of 1 / hyperfocal_distance, are changed to 1 / distance (in\n> > metres).\n> >\n> > Signed-off-by: David Plowman \n> > ---\n> > src/libcamera/control_ids.yaml | 26 ++++++++++++++------------\n> > 1 file changed, 14 insertions(+), 12 deletions(-)\n> >\n> > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > index a456e6c0..adea5f90 100644\n> > --- a/src/libcamera/control_ids.yaml\n> > +++ b/src/libcamera/control_ids.yaml\n> > @@ -591,25 +591,27 @@ controls:\n> > AfModeManual, though the value is reported back unconditionally in all\n> > modes.\n> >\n> > - The units are a reciprocal distance scale like dioptres but normalised\n> > - for the hyperfocal distance. That is, for a lens with hyperfocal\n> > - distance H, and setting it to a focal distance D, the lens position LP,\n> > - which is generally a non-integer, is given by\n> > + This value, which is generally a non-integer, is the reciprocal of the\n> > + focal distance in metres, also known as dioptres. That is, to set a\n> > + focal distance D, the lens position LP is given by\n> >\n> > - \\f$LP = \\frac{H}{D}\\f$\n> > + \\f$LP = \\frac{1\\mathrm{m}}{D}\\f$\n> >\n> > For example:\n> >\n> > 0 moves the lens to infinity.\n> > - 0.5 moves the lens to twice the hyperfocal distance.\n> > - 1 moves the lens to the hyperfocal position.\n> > - And larger values will focus the lens ever closer.\n> > + 0.5 moves the lens to focus on objects 2m away.\n> > + 2 moves the lens to focus on objects 50cm away.\n> > + And larger values will focus the lens closer.\n> >\n> > - \\todo Define a property to report the Hyperforcal distance of calibrated\n> > - lenses.\n> > + The default value of the control should indicate a good general position\n> > + for the lens, often corresponding to the hyperfocal distance (the\n> > + closest position for which objects at infinity are still acceptably\n> > + sharp). The minimum will often be zero (meaning infinity), and the\n> > + maximum value defines the closest focus position.\n> >\n> > - \\todo Define a property to report the maximum and minimum positions of\n> > - this lens. The minimum value will often be zero (meaning infinity).\n> > + \\todo Define a property to report the Hyperfocal distance of calibrated\n> > + lenses.\n>\n> Is this going to be a separate property, or the 'default' value of the\n> control?\n\nWe're thinking we're going to use the default value of the control.\n\nActually I think there is still a point to a \"hyperfocal\" property,\nbecause it's not guaranteed that the \"default\" position is the\nhyperfocal one. But in our applications, I think we're taking the view\nthat we will always use this \"default\" position. Nearly always it will\nbe hyperfocal, and if not, it will be because we want to put the lens\nsomewhere else.\n\n>\n> In the cover letter you mentioned \"we've resolved that we will be\n> able to read a good default position from the control info.\"\n>\n> Presumably, that means that when lacking other information, '1' is a\n> good default value?\n\nLacking other information, numbers around 1 are probably not a bad\nguess for many of our small modules.\n\nDavid\n\n>\n>\n> >\n> > - AfState:\n> > type: int32_t\n> > --\n> > 2.30.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 70CFABE08B\n\tfor ;\n\tThu, 17 Nov 2022 16:11:52 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A7D436308F;\n\tThu, 17 Nov 2022 17:11:51 +0100 (CET)","from mail-pf1-x429.google.com (mail-pf1-x429.google.com\n\t[IPv6:2607:f8b0:4864:20::429])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AF72D61F31\n\tfor ;\n\tThu, 17 Nov 2022 17:11:50 +0100 (CET)","by mail-pf1-x429.google.com with SMTP id 130so2206260pfu.8\n\tfor ;\n\tThu, 17 Nov 2022 08:11:50 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668701511;\n\tbh=QYkgE25vXFF1A4/8W6l/CZcFnGeZAQprCjD5qraIvn8=;\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=USUW1HTRAE4PBQEOHCvdNMOPh921ljh2vjRLVZJB+oO2bg4x8PYf1pCxXw4Ogc/2i\n\tKDhIO/UrEPmt6XcZGVE/HnhPW40tItakDnwsqGRkak1UjjzsTdmcQrXob9FWPPHFqP\n\tHDpFS50hxdTHChkRreI9vxA7TPWcxZj5KfncVuJ3yPKvZK0HikEKVD4Qy10Po9b4eV\n\tPd89LqeGvdEjISh43ZkPHpkEZMsDK02/xmqD2Ea/GYpQPvwNZS3LPT6/qrLJR0rtM/\n\tQHp4HiyaXFzzcItP/1dZjJoaL/pl+8F1lZiHx5dj/9UE+yEH92wD1iq11Yi+wZ8vHK\n\tN8efNcoFhG3tw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=+oNA03l7pI7t9gMJoKfN2c6Y2+H/DWgjkz5ICySrXbw=;\n\tb=W3UJmfZHtFK1WVNbJOC10Og1vhjNqjByu1DaMDNCQXe/OGFP9miQdNhqtn+VN7ogvA\n\tmKTyIG547chCXg1x4PaGiL1e6hFuoh4sO0mBgcMBWxxS2zkKhO6J+4DKv1ib3yMbEwEr\n\t5F2c0wAk4nHOGtAKyTOY2elfYxnxuQo+XHZMsJO3fLE2SeT8Wmhu0aHcHUbxEtd57iHa\n\t5iJ/vqwbUjAbXZ9MnmmSiu03hGOhPDlMhq969WP/zkAAf6EH584GNI22cXiBJAAvWGHj\n\tr018Qga2mn+rujETvkVImJ59+JRdYhAAySL441nRTGC9HUKWBwQur3vGWURa9iVH4za1\n\taXHw=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"W3UJmfZH\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=+oNA03l7pI7t9gMJoKfN2c6Y2+H/DWgjkz5ICySrXbw=;\n\tb=NEkJ2uS0NE+QNqb0uRuy2D8KA+Pin2/OjQdBkMBGpc8Y6Tqn6VbxP5RXA/et/RDwJx\n\tvWmFUtx1+KNubYoUhh5xGIxqHthb2xmTChvUX60NiXMUPv1EwIhtRlh9kGpWfei6hd4r\n\tnp0dxzklSn3YAK9bh2KdGNldJqM3OGIIx8+eoerabP+79z8bIgrffr+IaI0xPyJ78pxp\n\tVH5LnKUXyfCmGdu/cjt1CwY+rpFV8d5u2lvklhEYjLMzJ/2NxSY/0VD0q/+iC4pWhaeB\n\t0jftd4S8RkdU6y+3Mdbtqta6Ykm2PcEKov6/xs+AKLkHJG9CPsjkOfO1NsdlE7TPaBv1\n\tOpQg==","X-Gm-Message-State":"ANoB5pn5E4nAGu4qR4whAuaDXRmj+XvQmmkpQe80P3iNxqIGl/EB9Zib\n\trvvgbr8H3XiGVO10SZXJ2cwB4lZlWy4TKS3CRQzy5l7TP40=","X-Google-Smtp-Source":"AA0mqf4QkeRD7uNjPfWEkIn+OUWa/wlIGbPj9RW119BTr/HRNNAbfOtu+ptvT1mPufqt8qHW8pZdjnUTOQUI4GG+fzg=","X-Received":"by 2002:a05:6a00:1bce:b0:565:b4fe:de85 with SMTP id\n\to14-20020a056a001bce00b00565b4fede85mr3545291pfw.81.1668701509083;\n\tThu, 17 Nov 2022 08:11:49 -0800 (PST)","MIME-Version":"1.0","References":"<20221117154500.8829-1-david.plowman@raspberrypi.com>\n\t<20221117154500.8829-2-david.plowman@raspberrypi.com>\n\t<166870106589.50677.16904470229012395935@Monstersaurus>","In-Reply-To":"<166870106589.50677.16904470229012395935@Monstersaurus>","Date":"Thu, 17 Nov 2022 16:11:38 +0000","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","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":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25820,"web_url":"https://patchwork.libcamera.org/comment/25820/","msgid":"<166872639942.1300052.15323145116698928957@Monstersaurus>","date":"2022-11-17T23:06:39","subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting David Plowman (2022-11-17 16:11:38)\n> Hi Kieran\n> \n> On Thu, 17 Nov 2022 at 16:04, Kieran Bingham\n> wrote:\n> >\n> > Hi David,\n> >\n> > Quoting David Plowman via libcamera-devel (2022-11-17 15:45:00)\n> > > The units for the LensPosition control, previously defined as being in\n> > > units of 1 / hyperfocal_distance, are changed to 1 / distance (in\n> > > metres).\n> > >\n> > > Signed-off-by: David Plowman \n> > > ---\n> > > src/libcamera/control_ids.yaml | 26 ++++++++++++++------------\n> > > 1 file changed, 14 insertions(+), 12 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > > index a456e6c0..adea5f90 100644\n> > > --- a/src/libcamera/control_ids.yaml\n> > > +++ b/src/libcamera/control_ids.yaml\n> > > @@ -591,25 +591,27 @@ controls:\n> > > AfModeManual, though the value is reported back unconditionally in all\n> > > modes.\n> > >\n> > > - The units are a reciprocal distance scale like dioptres but normalised\n> > > - for the hyperfocal distance. That is, for a lens with hyperfocal\n> > > - distance H, and setting it to a focal distance D, the lens position LP,\n> > > - which is generally a non-integer, is given by\n> > > + This value, which is generally a non-integer, is the reciprocal of the\n> > > + focal distance in metres, also known as dioptres. That is, to set a\n> > > + focal distance D, the lens position LP is given by\n> > >\n> > > - \\f$LP = \\frac{H}{D}\\f$\n> > > + \\f$LP = \\frac{1\\mathrm{m}}{D}\\f$\n> > >\n> > > For example:\n> > >\n> > > 0 moves the lens to infinity.\n> > > - 0.5 moves the lens to twice the hyperfocal distance.\n> > > - 1 moves the lens to the hyperfocal position.\n> > > - And larger values will focus the lens ever closer.\n> > > + 0.5 moves the lens to focus on objects 2m away.\n> > > + 2 moves the lens to focus on objects 50cm away.\n> > > + And larger values will focus the lens closer.\n> > >\n> > > - \\todo Define a property to report the Hyperforcal distance of calibrated\n> > > - lenses.\n> > > + The default value of the control should indicate a good general position\n> > > + for the lens, often corresponding to the hyperfocal distance (the\n> > > + closest position for which objects at infinity are still acceptably\n> > > + sharp). The minimum will often be zero (meaning infinity), and the\n> > > + maximum value defines the closest focus position.\n> > >\n> > > - \\todo Define a property to report the maximum and minimum positions of\n> > > - this lens. The minimum value will often be zero (meaning infinity).\n> > > + \\todo Define a property to report the Hyperfocal distance of calibrated\n> > > + lenses.\n> >\n> > Is this going to be a separate property, or the 'default' value of the\n> > control?\n> \n> We're thinking we're going to use the default value of the control.\n> \n> Actually I think there is still a point to a \"hyperfocal\" property,\n> because it's not guaranteed that the \"default\" position is the\n> hyperfocal one. But in our applications, I think we're taking the view\n> that we will always use this \"default\" position. Nearly always it will\n> be hyperfocal, and if not, it will be because we want to put the lens\n> somewhere else.\n\nUsing the 'default' value, we can set it if we know we have one, or\nleave the default as unset if there isn't a known one yet.\n\nWe can always add a dedicated hyperfocal property later if it's\napplicable easily.\n\nI expect we'll still need some calibration on the VCM/lens anyway to be\nable to map a distance to the range of the underlying controls, and I\nsuspect there will be some slight imprecisions, but at least this would\nlet us report (approximate) distance information in the metadata too!\n\nShould we be considering renaming this to FocusPosition / FocalPoint /\nFocalLength ? (As it's the focal point we're trying to move rather than\na specific position of the lens ?) - And that might make more sense when\nreporting the position in the metadata ?\n\nAnyway, I don't see anything against this change, and as you added the\nproperty in the first place, I think you're best suited to know if it\nwas better this way at the moment, so:\n\n\nReviewed-by: Kieran Bingham \n\nLets see what anyone else says ...\n\n\n> > In the cover letter you mentioned \"we've resolved that we will be\n> > able to read a good default position from the control info.\"\n> >\n> > Presumably, that means that when lacking other information, '1' is a\n> > good default value?\n> \n> Lacking other information, numbers around 1 are probably not a bad\n> guess for many of our small modules.\n> \n> David\n> \n> >\n> >\n> > >\n> > > - AfState:\n> > > type: int32_t\n> > > --\n> > > 2.30.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 B2B76BE08B\n\tfor ;\n\tThu, 17 Nov 2022 23:06:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id D2A746308F;\n\tFri, 18 Nov 2022 00:06:44 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 206F061F32\n\tfor ;\n\tFri, 18 Nov 2022 00:06:43 +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 8AD3C47C;\n\tFri, 18 Nov 2022 00:06:42 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668726404;\n\tbh=V9NqGY9mOgVRnDnV2MKsem7lxByzjr4OlTHd/GUKmNA=;\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:Cc:\n\tFrom;\n\tb=J4YPNzdOJJllGJEBWO2aFrsAtnMEqIEz6GsiW2YHSjKR6ZqC9fanVvuK0lofOTkFa\n\tAwa/LNjSe7a9oDCsrxjqonAk5Xitl+WMI+CaTJTE562YnV8PfMsOKs8ebe+LTUHjQm\n\tejxCVYZNp+utWmbD5xYbYvDpRICDl7hJOVzHXwvA4R0koJX3dOVDfHiuUOGNe7xTPX\n\tPSVSPZg2esAwrJgefswz3oivKJ5mfzpFTjNgby5WwSbVx3sjgkjOR7YDHs7yjwSLZf\n\tHZIxgtlRZZz93egt6NwgFmD8Vx+RBMi62XqQ4zkXqD0Gb8nYIDTPHPIIkSFv8jmxyv\n\tTiuqRAZdbbiiA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1668726402;\n\tbh=V9NqGY9mOgVRnDnV2MKsem7lxByzjr4OlTHd/GUKmNA=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=s5U0MSEeVA9z4cjU2N96onw45O1nS6P11h5U5xzln47pN4ppr2SZzElTj0LUku8rM\n\tDFbWBHT7KU0xmNYNlv7DDlIKtKF5Lr3EReVgLUVGSEGd5NkCCts2nY3xK21PsGZnFv\n\tv/yj3tXGxKTPZo/Oe0pN6ucoFeVbsBIAfNM6Icx4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"s5U0MSEe\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20221117154500.8829-1-david.plowman@raspberrypi.com>\n\t<20221117154500.8829-2-david.plowman@raspberrypi.com>\n\t<166870106589.50677.16904470229012395935@Monstersaurus>\n\t","To":"David Plowman ","Date":"Thu, 17 Nov 2022 23:06:39 +0000","Message-ID":"<166872639942.1300052.15323145116698928957@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25821,"web_url":"https://patchwork.libcamera.org/comment/25821/","msgid":"<20221118082136.ihcjxe46vyacsrtj@uno.localdomain>","date":"2022-11-18T08:21:36","subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hello\n\nOn Thu, Nov 17, 2022 at 11:06:39PM +0000, Kieran Bingham via libcamera-devel wrote:\n> Quoting David Plowman (2022-11-17 16:11:38)\n> > Hi Kieran\n> >\n> > On Thu, 17 Nov 2022 at 16:04, Kieran Bingham\n> > wrote:\n> > >\n> > > Hi David,\n> > >\n> > > Quoting David Plowman via libcamera-devel (2022-11-17 15:45:00)\n> > > > The units for the LensPosition control, previously defined as being in\n> > > > units of 1 / hyperfocal_distance, are changed to 1 / distance (in\n> > > > metres).\n> > > >\n> > > > Signed-off-by: David Plowman \n> > > > ---\n> > > > src/libcamera/control_ids.yaml | 26 ++++++++++++++------------\n> > > > 1 file changed, 14 insertions(+), 12 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > > > index a456e6c0..adea5f90 100644\n> > > > --- a/src/libcamera/control_ids.yaml\n> > > > +++ b/src/libcamera/control_ids.yaml\n> > > > @@ -591,25 +591,27 @@ controls:\n> > > > AfModeManual, though the value is reported back unconditionally in all\n> > > > modes.\n> > > >\n> > > > - The units are a reciprocal distance scale like dioptres but normalised\n> > > > - for the hyperfocal distance. That is, for a lens with hyperfocal\n> > > > - distance H, and setting it to a focal distance D, the lens position LP,\n> > > > - which is generally a non-integer, is given by\n> > > > + This value, which is generally a non-integer, is the reciprocal of the\n> > > > + focal distance in metres, also known as dioptres. That is, to set a\n> > > > + focal distance D, the lens position LP is given by\n> > > >\n> > > > - \\f$LP = \\frac{H}{D}\\f$\n> > > > + \\f$LP = \\frac{1\\mathrm{m}}{D}\\f$\n> > > >\n> > > > For example:\n> > > >\n> > > > 0 moves the lens to infinity.\n> > > > - 0.5 moves the lens to twice the hyperfocal distance.\n> > > > - 1 moves the lens to the hyperfocal position.\n> > > > - And larger values will focus the lens ever closer.\n> > > > + 0.5 moves the lens to focus on objects 2m away.\n> > > > + 2 moves the lens to focus on objects 50cm away.\n> > > > + And larger values will focus the lens closer.\n> > > >\n> > > > - \\todo Define a property to report the Hyperforcal distance of calibrated\n> > > > - lenses.\n> > > > + The default value of the control should indicate a good general position\n> > > > + for the lens, often corresponding to the hyperfocal distance (the\n> > > > + closest position for which objects at infinity are still acceptably\n> > > > + sharp). The minimum will often be zero (meaning infinity), and the\n> > > > + maximum value defines the closest focus position.\n> > > >\n> > > > - \\todo Define a property to report the maximum and minimum positions of\n> > > > - this lens. The minimum value will often be zero (meaning infinity).\n> > > > + \\todo Define a property to report the Hyperfocal distance of calibrated\n> > > > + lenses.\n> > >\n> > > Is this going to be a separate property, or the 'default' value of the\n> > > control?\n> >\n> > We're thinking we're going to use the default value of the control.\n> >\n> > Actually I think there is still a point to a \"hyperfocal\" property,\n> > because it's not guaranteed that the \"default\" position is the\n> > hyperfocal one. But in our applications, I think we're taking the view\n> > that we will always use this \"default\" position. Nearly always it will\n> > be hyperfocal, and if not, it will be because we want to put the lens\n> > somewhere else.\n>\n> Using the 'default' value, we can set it if we know we have one, or\n> leave the default as unset if there isn't a known one yet.\n>\n> We can always add a dedicated hyperfocal property later if it's\n> applicable easily.\n>\n\nI tend to agree with all the above...\n\nIt's been half an year since we have been trying to standardize\nthis property on \"1 / hyperfocal distance\" and so far not a single\nlens has been calibrated, mostly because the exact calibration of the\nthe hyperfocal distance is a rather complex process, if not\nimpossibile for uncalibrated lenses ?\n\nUsage of dioptres means an esier calibration process if I understand\nthings correctly, by simply using a rule meter and a procedure more\naccessible for non professional users, right ?\n\nAnyway, it then remains the fact we need a proper CameraLensHelper to\ntranslate from the generic unit to the exact v4l2-control value, same\nas it happens for the gain values, and if this unblocks that\ndevelopment I'm all for it..\n\n\n> I expect we'll still need some calibration on the VCM/lens anyway to be\n> able to map a distance to the range of the underlying controls, and I\n> suspect there will be some slight imprecisions, but at least this would\n> let us report (approximate) distance information in the metadata too!\n>\n> Should we be considering renaming this to FocusPosition / FocalPoint /\n> FocalLength ? (As it's the focal point we're trying to move rather than\n> a specific position of the lens ?) - And that might make more sense when\n> reporting the position in the metadata ?\n>\n> Anyway, I don't see anything against this change, and as you added the\n> property in the first place, I think you're best suited to know if it\n> was better this way at the moment, so:\n>\n>\n> Reviewed-by: Kieran Bingham \n\nLikewise\nReviewed-by: Jacopo Mondi \n\n>\n> Lets see what anyone else says ...\n>\n>\n> > > In the cover letter you mentioned \"we've resolved that we will be\n> > > able to read a good default position from the control info.\"\n> > >\n> > > Presumably, that means that when lacking other information, '1' is a\n> > > good default value?\n> >\n> > Lacking other information, numbers around 1 are probably not a bad\n> > guess for many of our small modules.\n> >\n> > David\n> >\n> > >\n> > >\n> > > >\n> > > > - AfState:\n> > > > type: int32_t\n> > > > --\n> > > > 2.30.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 C20F4BD16B\n\tfor ;\n\tFri, 18 Nov 2022 08:21:40 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 174A76308D;\n\tFri, 18 Nov 2022 09:21:40 +0100 (CET)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8879063087\n\tfor ;\n\tFri, 18 Nov 2022 09:21:38 +0100 (CET)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id B0336FF808;\n\tFri, 18 Nov 2022 08:21:37 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1668759700;\n\tbh=ajr3kg2P32TO3GR/Uo0UfWwDMcZwyMmg+v3aDt0VXXQ=;\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=IW4teEMwAYDop/Wr0lLlWG3nXU17/ilYlkx4XCFj4RbaA1rHBUwuj10UTmUbfD1Kx\n\tzr9NNKLqio31EefS52MwRfQIWbEPnpZc5a0iNCuGm0V+C6KObxatXD2M0oncRRfZ0I\n\ty5xMOxqeGn2Arkh7C1V5Ohkv/mjadrBuJrDr3R3jDHQBreHYeQuvimtJiSKj1ds5HS\n\tGixvtCERpetZyo3Dxi5FTW3rBQjduteu+XZnKl2XJt0/6QJHuJ6jTTtgrlBjDHemSW\n\tKkhuTg1LiSnko+rh03qLRA2ko4OAOHsCq217SC2rbbySJ+I0yexa3RZBLOfrVmZJwW\n\tWzOILTG1JfbiA==","Date":"Fri, 18 Nov 2022 09:21:36 +0100","To":"Kieran Bingham ","Message-ID":"<20221118082136.ihcjxe46vyacsrtj@uno.localdomain>","References":"<20221117154500.8829-1-david.plowman@raspberrypi.com>\n\t<20221117154500.8829-2-david.plowman@raspberrypi.com>\n\t<166870106589.50677.16904470229012395935@Monstersaurus>\n\t\n\t<166872639942.1300052.15323145116698928957@Monstersaurus>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<166872639942.1300052.15323145116698928957@Monstersaurus>","Subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","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":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26026,"web_url":"https://patchwork.libcamera.org/comment/26026/","msgid":"","date":"2022-12-08T11:22:19","subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi everyone\n\nThanks for the comments on this change. I was wondering if I might\ngive it a little nudge if there are no objections anywhere else?\n\nThanks!\nDavid\n\nOn Fri, 18 Nov 2022 at 08:21, Jacopo Mondi wrote:\n>\n> Hello\n>\n> On Thu, Nov 17, 2022 at 11:06:39PM +0000, Kieran Bingham via libcamera-devel wrote:\n> > Quoting David Plowman (2022-11-17 16:11:38)\n> > > Hi Kieran\n> > >\n> > > On Thu, 17 Nov 2022 at 16:04, Kieran Bingham\n> > > wrote:\n> > > >\n> > > > Hi David,\n> > > >\n> > > > Quoting David Plowman via libcamera-devel (2022-11-17 15:45:00)\n> > > > > The units for the LensPosition control, previously defined as being in\n> > > > > units of 1 / hyperfocal_distance, are changed to 1 / distance (in\n> > > > > metres).\n> > > > >\n> > > > > Signed-off-by: David Plowman \n> > > > > ---\n> > > > > src/libcamera/control_ids.yaml | 26 ++++++++++++++------------\n> > > > > 1 file changed, 14 insertions(+), 12 deletions(-)\n> > > > >\n> > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > > > > index a456e6c0..adea5f90 100644\n> > > > > --- a/src/libcamera/control_ids.yaml\n> > > > > +++ b/src/libcamera/control_ids.yaml\n> > > > > @@ -591,25 +591,27 @@ controls:\n> > > > > AfModeManual, though the value is reported back unconditionally in all\n> > > > > modes.\n> > > > >\n> > > > > - The units are a reciprocal distance scale like dioptres but normalised\n> > > > > - for the hyperfocal distance. That is, for a lens with hyperfocal\n> > > > > - distance H, and setting it to a focal distance D, the lens position LP,\n> > > > > - which is generally a non-integer, is given by\n> > > > > + This value, which is generally a non-integer, is the reciprocal of the\n> > > > > + focal distance in metres, also known as dioptres. That is, to set a\n> > > > > + focal distance D, the lens position LP is given by\n> > > > >\n> > > > > - \\f$LP = \\frac{H}{D}\\f$\n> > > > > + \\f$LP = \\frac{1\\mathrm{m}}{D}\\f$\n> > > > >\n> > > > > For example:\n> > > > >\n> > > > > 0 moves the lens to infinity.\n> > > > > - 0.5 moves the lens to twice the hyperfocal distance.\n> > > > > - 1 moves the lens to the hyperfocal position.\n> > > > > - And larger values will focus the lens ever closer.\n> > > > > + 0.5 moves the lens to focus on objects 2m away.\n> > > > > + 2 moves the lens to focus on objects 50cm away.\n> > > > > + And larger values will focus the lens closer.\n> > > > >\n> > > > > - \\todo Define a property to report the Hyperforcal distance of calibrated\n> > > > > - lenses.\n> > > > > + The default value of the control should indicate a good general position\n> > > > > + for the lens, often corresponding to the hyperfocal distance (the\n> > > > > + closest position for which objects at infinity are still acceptably\n> > > > > + sharp). The minimum will often be zero (meaning infinity), and the\n> > > > > + maximum value defines the closest focus position.\n> > > > >\n> > > > > - \\todo Define a property to report the maximum and minimum positions of\n> > > > > - this lens. The minimum value will often be zero (meaning infinity).\n> > > > > + \\todo Define a property to report the Hyperfocal distance of calibrated\n> > > > > + lenses.\n> > > >\n> > > > Is this going to be a separate property, or the 'default' value of the\n> > > > control?\n> > >\n> > > We're thinking we're going to use the default value of the control.\n> > >\n> > > Actually I think there is still a point to a \"hyperfocal\" property,\n> > > because it's not guaranteed that the \"default\" position is the\n> > > hyperfocal one. But in our applications, I think we're taking the view\n> > > that we will always use this \"default\" position. Nearly always it will\n> > > be hyperfocal, and if not, it will be because we want to put the lens\n> > > somewhere else.\n> >\n> > Using the 'default' value, we can set it if we know we have one, or\n> > leave the default as unset if there isn't a known one yet.\n> >\n> > We can always add a dedicated hyperfocal property later if it's\n> > applicable easily.\n> >\n>\n> I tend to agree with all the above...\n>\n> It's been half an year since we have been trying to standardize\n> this property on \"1 / hyperfocal distance\" and so far not a single\n> lens has been calibrated, mostly because the exact calibration of the\n> the hyperfocal distance is a rather complex process, if not\n> impossibile for uncalibrated lenses ?\n>\n> Usage of dioptres means an esier calibration process if I understand\n> things correctly, by simply using a rule meter and a procedure more\n> accessible for non professional users, right ?\n>\n> Anyway, it then remains the fact we need a proper CameraLensHelper to\n> translate from the generic unit to the exact v4l2-control value, same\n> as it happens for the gain values, and if this unblocks that\n> development I'm all for it..\n>\n>\n> > I expect we'll still need some calibration on the VCM/lens anyway to be\n> > able to map a distance to the range of the underlying controls, and I\n> > suspect there will be some slight imprecisions, but at least this would\n> > let us report (approximate) distance information in the metadata too!\n> >\n> > Should we be considering renaming this to FocusPosition / FocalPoint /\n> > FocalLength ? (As it's the focal point we're trying to move rather than\n> > a specific position of the lens ?) - And that might make more sense when\n> > reporting the position in the metadata ?\n> >\n> > Anyway, I don't see anything against this change, and as you added the\n> > property in the first place, I think you're best suited to know if it\n> > was better this way at the moment, so:\n> >\n> >\n> > Reviewed-by: Kieran Bingham \n>\n> Likewise\n> Reviewed-by: Jacopo Mondi \n>\n> >\n> > Lets see what anyone else says ...\n> >\n> >\n> > > > In the cover letter you mentioned \"we've resolved that we will be\n> > > > able to read a good default position from the control info.\"\n> > > >\n> > > > Presumably, that means that when lacking other information, '1' is a\n> > > > good default value?\n> > >\n> > > Lacking other information, numbers around 1 are probably not a bad\n> > > guess for many of our small modules.\n> > >\n> > > David\n> > >\n> > > >\n> > > >\n> > > > >\n> > > > > - AfState:\n> > > > > type: int32_t\n> > > > > --\n> > > > > 2.30.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 C5137BDE6B\n\tfor ;\n\tThu, 8 Dec 2022 11:22:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 51BE86334A;\n\tThu, 8 Dec 2022 12:22:33 +0100 (CET)","from mail-pg1-x534.google.com (mail-pg1-x534.google.com\n\t[IPv6:2607:f8b0:4864:20::534])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D11B661F1C\n\tfor ;\n\tThu, 8 Dec 2022 12:22:31 +0100 (CET)","by mail-pg1-x534.google.com with SMTP id w37so946135pga.5\n\tfor ;\n\tThu, 08 Dec 2022 03:22:31 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1670498553;\n\tbh=x/mUlph5D1eryrjY1Xns2o8l2zfS6Kf7gA8L6NFFvEg=;\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=rooPl/KqqegxS/8y7SJHn/8khUMhr18GYBupQ8/I0yXjW29yPbjb7v0kktbgldRM0\n\t43wuUz6/QST90rJUjBmLVoWTpa6jM3i6CIJ8yj8uHlqxIu0Y2HuN26wGB1HxMzxJwH\n\t86/C2gZ2uvUUWNxHT7dibSOB6/0TQPhECSU4yOrOq9TsXDH4gGM4RAr86lxFAJDE7p\n\t+H6yvnShaZYyY5SEY7n7mRTCAX/uBvgsg8gOWBk6ayJeFmN+mo/R4H636DzpeE0hn0\n\txn0+hAfxmN/3W5CVTZhqJT03qxgVzHwZP9ohVAK4TRHimJZ/DDPP33ZMmwNR0z13GV\n\tTMnfUrTi/FCuw==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=tGWXisBwAGOgpJBGZ6cjalbvtupr86yVSTHHsSvh3Rw=;\n\tb=J3NCjMx5DoTycly09qkB3RWQJ5JJb1SorSW9sjwcQDaTIRehNlDF28cURysDduX85u\n\tyQhuCYUQwwDmVVq+6iunXBA0LW3whKkV69wlKqdPEhmtcozLLbQyGU63wFgE6VIoKxjP\n\t2DevriYN5r21wwO502PXq9UBpp0nhTmHznMVY8y2qOqU0SoxekDewYgZhlgLGfNmqjvh\n\t/GZKYhEXUrvtqGMbFsUHlraC4Jub1WDFxFHMuVS8GsNbRYfDED/xWiGV7qCulq9muRkS\n\tlT1AsrE3tGosx617SMRGpP8zj2qYIgQIHdi3c86bmx/M+okDzGVkIS7VDDr7/kDxc0UT\n\taGiQ=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"J3NCjMx5\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=tGWXisBwAGOgpJBGZ6cjalbvtupr86yVSTHHsSvh3Rw=;\n\tb=u5e7S3K7xgChvCDP0IMVS/xsSrg7E8Q/wNOLu4wxmkHDpLKRBOVSUZiqeaNC007yjr\n\tzxe8W+zH+9gbk+5JBuCLrSMS8BcuuKg7qvRANw5uin95XBPnVD+qyrGUXL4nCQM8759U\n\t4oVFuNrfIR46x8u1XCVEIR8PLz07lujYuD/pwEw+qpQYiIY/zYceSVAhQqXPGDg/byvW\n\tt9ebc0w76CUd5CIx495sgCEEXIvf9W8RIVKYe2wKnKOIrUinOhq0tv1fCfVSvy5UZh38\n\tnbFQhbW755i0XJGh5GmZOfCvX/ig4t1yfw4O0CSO2CLedjwvv/kj/HcQLKwd338iOnAM\n\t6qdQ==","X-Gm-Message-State":"ANoB5pnzcqYsPTEtA7xOOunj+MTkHLMimQvgJe7dhYl4skb+Jfr1wrZS\n\tAYyy6yOnSnaZtRJMoVp0sMT4JmmKUYaYjzzAeG9HLw==","X-Google-Smtp-Source":"AA0mqf5asObH2u5DiJhEJHQXVyQqevBMG/3W8zIQxjaDrbN9fvoDdzJ3BaAX9uStv/WKIWh7+6ltrpk8Cishi1aiEz8=","X-Received":"by 2002:a63:cc48:0:b0:477:5515:8a9c with SMTP id\n\tq8-20020a63cc48000000b0047755158a9cmr67459736pgi.256.1670498550214;\n\tThu, 08 Dec 2022 03:22:30 -0800 (PST)","MIME-Version":"1.0","References":"<20221117154500.8829-1-david.plowman@raspberrypi.com>\n\t<20221117154500.8829-2-david.plowman@raspberrypi.com>\n\t<166870106589.50677.16904470229012395935@Monstersaurus>\n\t\n\t<166872639942.1300052.15323145116698928957@Monstersaurus>\n\t<20221118082136.ihcjxe46vyacsrtj@uno.localdomain>","In-Reply-To":"<20221118082136.ihcjxe46vyacsrtj@uno.localdomain>","Date":"Thu, 8 Dec 2022 11:22:19 +0000","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","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":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26027,"web_url":"https://patchwork.libcamera.org/comment/26027/","msgid":"<167049906280.9133.5362324309443207786@Monstersaurus>","date":"2022-12-08T11:31:02","subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi David,\n\nQuoting David Plowman (2022-12-08 11:22:19)\n> Hi everyone\n> \n> Thanks for the comments on this change. I was wondering if I might\n> give it a little nudge if there are no objections anywhere else?\n> \n> Thanks!\n> David\n> \n> On Fri, 18 Nov 2022 at 08:21, Jacopo Mondi wrote:\n> >\n> > Hello\n> >\n> > On Thu, Nov 17, 2022 at 11:06:39PM +0000, Kieran Bingham via libcamera-devel wrote:\n> > > Quoting David Plowman (2022-11-17 16:11:38)\n> > > > Hi Kieran\n> > > >\n> > > > On Thu, 17 Nov 2022 at 16:04, Kieran Bingham\n> > > > wrote:\n> > > > >\n> > > > > Hi David,\n> > > > >\n> > > > > Quoting David Plowman via libcamera-devel (2022-11-17 15:45:00)\n> > > > > > The units for the LensPosition control, previously defined as being in\n> > > > > > units of 1 / hyperfocal_distance, are changed to 1 / distance (in\n> > > > > > metres).\n> > > > > >\n> > > > > > Signed-off-by: David Plowman \n> > > > > > ---\n> > > > > > src/libcamera/control_ids.yaml | 26 ++++++++++++++------------\n> > > > > > 1 file changed, 14 insertions(+), 12 deletions(-)\n> > > > > >\n> > > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > > > > > index a456e6c0..adea5f90 100644\n> > > > > > --- a/src/libcamera/control_ids.yaml\n> > > > > > +++ b/src/libcamera/control_ids.yaml\n> > > > > > @@ -591,25 +591,27 @@ controls:\n> > > > > > AfModeManual, though the value is reported back unconditionally in all\n> > > > > > modes.\n> > > > > >\n> > > > > > - The units are a reciprocal distance scale like dioptres but normalised\n> > > > > > - for the hyperfocal distance. That is, for a lens with hyperfocal\n> > > > > > - distance H, and setting it to a focal distance D, the lens position LP,\n> > > > > > - which is generally a non-integer, is given by\n> > > > > > + This value, which is generally a non-integer, is the reciprocal of the\n> > > > > > + focal distance in metres, also known as dioptres. That is, to set a\n> > > > > > + focal distance D, the lens position LP is given by\n> > > > > >\n> > > > > > - \\f$LP = \\frac{H}{D}\\f$\n> > > > > > + \\f$LP = \\frac{1\\mathrm{m}}{D}\\f$\n> > > > > >\n> > > > > > For example:\n> > > > > >\n> > > > > > 0 moves the lens to infinity.\n> > > > > > - 0.5 moves the lens to twice the hyperfocal distance.\n> > > > > > - 1 moves the lens to the hyperfocal position.\n> > > > > > - And larger values will focus the lens ever closer.\n> > > > > > + 0.5 moves the lens to focus on objects 2m away.\n> > > > > > + 2 moves the lens to focus on objects 50cm away.\n> > > > > > + And larger values will focus the lens closer.\n> > > > > >\n> > > > > > - \\todo Define a property to report the Hyperforcal distance of calibrated\n> > > > > > - lenses.\n> > > > > > + The default value of the control should indicate a good general position\n> > > > > > + for the lens, often corresponding to the hyperfocal distance (the\n> > > > > > + closest position for which objects at infinity are still acceptably\n> > > > > > + sharp). The minimum will often be zero (meaning infinity), and the\n> > > > > > + maximum value defines the closest focus position.\n> > > > > >\n> > > > > > - \\todo Define a property to report the maximum and minimum positions of\n> > > > > > - this lens. The minimum value will often be zero (meaning infinity).\n> > > > > > + \\todo Define a property to report the Hyperfocal distance of calibrated\n> > > > > > + lenses.\n> > > > >\n> > > > > Is this going to be a separate property, or the 'default' value of the\n> > > > > control?\n> > > >\n> > > > We're thinking we're going to use the default value of the control.\n> > > >\n> > > > Actually I think there is still a point to a \"hyperfocal\" property,\n> > > > because it's not guaranteed that the \"default\" position is the\n> > > > hyperfocal one. But in our applications, I think we're taking the view\n> > > > that we will always use this \"default\" position. Nearly always it will\n> > > > be hyperfocal, and if not, it will be because we want to put the lens\n> > > > somewhere else.\n> > >\n> > > Using the 'default' value, we can set it if we know we have one, or\n> > > leave the default as unset if there isn't a known one yet.\n> > >\n> > > We can always add a dedicated hyperfocal property later if it's\n> > > applicable easily.\n> > >\n> >\n> > I tend to agree with all the above...\n> >\n> > It's been half an year since we have been trying to standardize\n> > this property on \"1 / hyperfocal distance\" and so far not a single\n> > lens has been calibrated, mostly because the exact calibration of the\n> > the hyperfocal distance is a rather complex process, if not\n> > impossibile for uncalibrated lenses ?\n> >\n> > Usage of dioptres means an esier calibration process if I understand\n> > things correctly, by simply using a rule meter and a procedure more\n> > accessible for non professional users, right ?\n> >\n> > Anyway, it then remains the fact we need a proper CameraLensHelper to\n> > translate from the generic unit to the exact v4l2-control value, same\n> > as it happens for the gain values, and if this unblocks that\n> > development I'm all for it..\n> >\n> >\n> > > I expect we'll still need some calibration on the VCM/lens anyway to be\n> > > able to map a distance to the range of the underlying controls, and I\n> > > suspect there will be some slight imprecisions, but at least this would\n> > > let us report (approximate) distance information in the metadata too!\n> > >\n> > > Should we be considering renaming this to FocusPosition / FocalPoint /\n> > > FocalLength ? (As it's the focal point we're trying to move rather than\n> > > a specific position of the lens ?) - And that might make more sense when\n> > > reporting the position in the metadata ?\n> > >\n> > > Anyway, I don't see anything against this change, and as you added the\n> > > property in the first place, I think you're best suited to know if it\n> > > was better this way at the moment, so:\n> > >\n> > >\n> > > Reviewed-by: Kieran Bingham \n> >\n> > Likewise\n> > Reviewed-by: Jacopo Mondi \n> >\n\nI see two RB tags, so I'll run this through the tests and merge if\nnothing here breaks!\n\n--\nKieran\n\n\n\n> > >\n> > > Lets see what anyone else says ...\n> > >\n> > >\n> > > > > In the cover letter you mentioned \"we've resolved that we will be\n> > > > > able to read a good default position from the control info.\"\n> > > > >\n> > > > > Presumably, that means that when lacking other information, '1' is a\n> > > > > good default value?\n> > > >\n> > > > Lacking other information, numbers around 1 are probably not a bad\n> > > > guess for many of our small modules.\n> > > >\n> > > > David\n> > > >\n> > > > >\n> > > > >\n> > > > > >\n> > > > > > - AfState:\n> > > > > > type: int32_t\n> > > > > > --\n> > > > > > 2.30.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 EC307BE08B\n\tfor ;\n\tThu, 8 Dec 2022 11:31:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 734596334A;\n\tThu, 8 Dec 2022 12:31:07 +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 B129261F1C\n\tfor ;\n\tThu, 8 Dec 2022 12:31:05 +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 295A225B;\n\tThu, 8 Dec 2022 12:31:05 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1670499067;\n\tbh=YTfLw6IyaGoRq/Q7GWCE9Q1rnh6BJb2g3BvaCYcKChw=;\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:Cc:\n\tFrom;\n\tb=0QT3QbSwgxw9oA5yhctIZTrN3WVIF2ccA+webew2hJ97JmU7le270s1I6Y+/WUhVw\n\tb1BDbX5dcxHTFetj8D/eN8tHXpCIeD1w0686I4ewpHcJ+MlUrm6HSR4Xpv/DD6/sbw\n\tT4r5LG4uc0tkvwb6iddY00HKtm0eWA2YlLYrkXXareFhl5sj3+NqLC6o98TksAyjlk\n\tk7Tty/hZPGN00LKNDEeMXSiShoFB24pKPPdi4DPG2tsRPrAnatK9wPGSB0EyXtWZT6\n\tH3lJS3+xyDjFuoxQjWjLsFaaq0lDwcOeUqdveHWy3w68AHUGyC7IBkvWpzfzyIsCOZ\n\tONnueSYOEiQhA==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1670499065;\n\tbh=YTfLw6IyaGoRq/Q7GWCE9Q1rnh6BJb2g3BvaCYcKChw=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=pPE59+QANn670fIFx0Fv51trh6ZlIxUQXZw5tktYhG3wA2TmI88DQenVRqVvjAuGl\n\tN5y50ypl8vSLnzYdqxk5+MflTpg6Qk+/mKgbxqkDG9LobzZhiGC1LFjqWER9T5THkS\n\tmTShWC+HxVV2auu+tNcbsRPTXLJJMP55wjG9+6xE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"pPE59+QA\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20221117154500.8829-1-david.plowman@raspberrypi.com>\n\t<20221117154500.8829-2-david.plowman@raspberrypi.com>\n\t<166870106589.50677.16904470229012395935@Monstersaurus>\n\t\n\t<166872639942.1300052.15323145116698928957@Monstersaurus>\n\t<20221118082136.ihcjxe46vyacsrtj@uno.localdomain>\n\t","To":"David Plowman ,\n\tJacopo Mondi ","Date":"Thu, 08 Dec 2022 11:31:02 +0000","Message-ID":"<167049906280.9133.5362324309443207786@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26028,"web_url":"https://patchwork.libcamera.org/comment/26028/","msgid":"<167049930255.9133.11856255309228418704@Monstersaurus>","date":"2022-12-08T11:35:02","subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Kieran Bingham (2022-12-08 11:31:02)\n> Hi David,\n> \n> Quoting David Plowman (2022-12-08 11:22:19)\n> > Hi everyone\n> > \n> > Thanks for the comments on this change. I was wondering if I might\n> > give it a little nudge if there are no objections anywhere else?\n> > \n> > Thanks!\n> > David\n> > \n> > On Fri, 18 Nov 2022 at 08:21, Jacopo Mondi wrote:\n> > >\n> > > Hello\n> > >\n> > > On Thu, Nov 17, 2022 at 11:06:39PM +0000, Kieran Bingham via libcamera-devel wrote:\n> > > > Quoting David Plowman (2022-11-17 16:11:38)\n> > > > > Hi Kieran\n> > > > >\n> > > > > On Thu, 17 Nov 2022 at 16:04, Kieran Bingham\n> > > > > wrote:\n> > > > > >\n> > > > > > Hi David,\n> > > > > >\n> > > > > > Quoting David Plowman via libcamera-devel (2022-11-17 15:45:00)\n> > > > > > > The units for the LensPosition control, previously defined as being in\n> > > > > > > units of 1 / hyperfocal_distance, are changed to 1 / distance (in\n> > > > > > > metres).\n> > > > > > >\n> > > > > > > Signed-off-by: David Plowman \n> > > > > > > ---\n> > > > > > > src/libcamera/control_ids.yaml | 26 ++++++++++++++------------\n> > > > > > > 1 file changed, 14 insertions(+), 12 deletions(-)\n> > > > > > >\n> > > > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > > > > > > index a456e6c0..adea5f90 100644\n> > > > > > > --- a/src/libcamera/control_ids.yaml\n> > > > > > > +++ b/src/libcamera/control_ids.yaml\n> > > > > > > @@ -591,25 +591,27 @@ controls:\n> > > > > > > AfModeManual, though the value is reported back unconditionally in all\n> > > > > > > modes.\n> > > > > > >\n> > > > > > > - The units are a reciprocal distance scale like dioptres but normalised\n> > > > > > > - for the hyperfocal distance. That is, for a lens with hyperfocal\n> > > > > > > - distance H, and setting it to a focal distance D, the lens position LP,\n> > > > > > > - which is generally a non-integer, is given by\n> > > > > > > + This value, which is generally a non-integer, is the reciprocal of the\n> > > > > > > + focal distance in metres, also known as dioptres. That is, to set a\n> > > > > > > + focal distance D, the lens position LP is given by\n> > > > > > >\n> > > > > > > - \\f$LP = \\frac{H}{D}\\f$\n> > > > > > > + \\f$LP = \\frac{1\\mathrm{m}}{D}\\f$\n> > > > > > >\n> > > > > > > For example:\n> > > > > > >\n> > > > > > > 0 moves the lens to infinity.\n> > > > > > > - 0.5 moves the lens to twice the hyperfocal distance.\n> > > > > > > - 1 moves the lens to the hyperfocal position.\n> > > > > > > - And larger values will focus the lens ever closer.\n> > > > > > > + 0.5 moves the lens to focus on objects 2m away.\n> > > > > > > + 2 moves the lens to focus on objects 50cm away.\n> > > > > > > + And larger values will focus the lens closer.\n> > > > > > >\n> > > > > > > - \\todo Define a property to report the Hyperforcal distance of calibrated\n> > > > > > > - lenses.\n> > > > > > > + The default value of the control should indicate a good general position\n> > > > > > > + for the lens, often corresponding to the hyperfocal distance (the\n> > > > > > > + closest position for which objects at infinity are still acceptably\n> > > > > > > + sharp). The minimum will often be zero (meaning infinity), and the\n> > > > > > > + maximum value defines the closest focus position.\n> > > > > > >\n> > > > > > > - \\todo Define a property to report the maximum and minimum positions of\n> > > > > > > - this lens. The minimum value will often be zero (meaning infinity).\n> > > > > > > + \\todo Define a property to report the Hyperfocal distance of calibrated\n> > > > > > > + lenses.\n> > > > > >\n> > > > > > Is this going to be a separate property, or the 'default' value of the\n> > > > > > control?\n> > > > >\n> > > > > We're thinking we're going to use the default value of the control.\n> > > > >\n> > > > > Actually I think there is still a point to a \"hyperfocal\" property,\n> > > > > because it's not guaranteed that the \"default\" position is the\n> > > > > hyperfocal one. But in our applications, I think we're taking the view\n> > > > > that we will always use this \"default\" position. Nearly always it will\n> > > > > be hyperfocal, and if not, it will be because we want to put the lens\n> > > > > somewhere else.\n> > > >\n> > > > Using the 'default' value, we can set it if we know we have one, or\n> > > > leave the default as unset if there isn't a known one yet.\n> > > >\n> > > > We can always add a dedicated hyperfocal property later if it's\n> > > > applicable easily.\n> > > >\n> > >\n> > > I tend to agree with all the above...\n> > >\n> > > It's been half an year since we have been trying to standardize\n> > > this property on \"1 / hyperfocal distance\" and so far not a single\n> > > lens has been calibrated, mostly because the exact calibration of the\n> > > the hyperfocal distance is a rather complex process, if not\n> > > impossibile for uncalibrated lenses ?\n> > >\n> > > Usage of dioptres means an esier calibration process if I understand\n> > > things correctly, by simply using a rule meter and a procedure more\n> > > accessible for non professional users, right ?\n> > >\n> > > Anyway, it then remains the fact we need a proper CameraLensHelper to\n> > > translate from the generic unit to the exact v4l2-control value, same\n> > > as it happens for the gain values, and if this unblocks that\n> > > development I'm all for it..\n> > >\n> > >\n> > > > I expect we'll still need some calibration on the VCM/lens anyway to be\n> > > > able to map a distance to the range of the underlying controls, and I\n> > > > suspect there will be some slight imprecisions, but at least this would\n> > > > let us report (approximate) distance information in the metadata too!\n> > > >\n> > > > Should we be considering renaming this to FocusPosition / FocalPoint /\n> > > > FocalLength ? (As it's the focal point we're trying to move rather than\n> > > > a specific position of the lens ?) - And that might make more sense when\n> > > > reporting the position in the metadata ?\n> > > >\n> > > > Anyway, I don't see anything against this change, and as you added the\n> > > > property in the first place, I think you're best suited to know if it\n> > > > was better this way at the moment, so:\n> > > >\n> > > >\n> > > > Reviewed-by: Kieran Bingham \n> > >\n> > > Likewise\n> > > Reviewed-by: Jacopo Mondi \n> > >\n> \n> I see two RB tags, so I'll run this through the tests and merge if\n> nothing here breaks!\n> \n\nAhem, with no actual code change - nothing can break ;-) So I guess this\ngets an easy pass.\n--\nKieran\n\n\n> --\n> Kieran\n> \n> \n> \n> > > >\n> > > > Lets see what anyone else says ...\n> > > >\n> > > >\n> > > > > > In the cover letter you mentioned \"we've resolved that we will be\n> > > > > > able to read a good default position from the control info.\"\n> > > > > >\n> > > > > > Presumably, that means that when lacking other information, '1' is a\n> > > > > > good default value?\n> > > > >\n> > > > > Lacking other information, numbers around 1 are probably not a bad\n> > > > > guess for many of our small modules.\n> > > > >\n> > > > > David\n> > > > >\n> > > > > >\n> > > > > >\n> > > > > > >\n> > > > > > > - AfState:\n> > > > > > > type: int32_t\n> > > > > > > --\n> > > > > > > 2.30.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 C7636BDE6B\n\tfor ;\n\tThu, 8 Dec 2022 11:35:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3B73763335;\n\tThu, 8 Dec 2022 12:35:07 +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 DE2DF61F1C\n\tfor ;\n\tThu, 8 Dec 2022 12:35:05 +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 5E42B25B;\n\tThu, 8 Dec 2022 12:35:05 +0100 (CET)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1670499307;\n\tbh=SbKnGKq+2zQgz31Nf7zNNCbjhPNaLOqnRwfPFYXopGo=;\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:Cc:\n\tFrom;\n\tb=EgfGjfzYFcF0ZcKpJCL5Avo8kmirAzmd+kGmru/IQGcojCxn/hqrY+rYAxgDtnq2J\n\t4k7uYDo0ru1Tmt48R8Zr8ZobVTbF0aZAYs5Zg6R+Ium3Ct1eEkJAicRh/KAAgMOpfg\n\twXQkKwhu/9wX47rwECeMCYomaTAzaDUY5nEo2c6f0oFqs1lBT1Vfxw8ugCpAVh2jOx\n\tBBoh36rH/J/+VP5Mo6YRVhCBhwnWRth5M6BIYQsJ04BJp0UUET59YsodvR2y8aeB1K\n\t/r3PPjKN1Id7jJ7NjZkNHLiDHVjeXAmKjS3I3veQQpgDigOpsrSmMltN9ttQSu2RLy\n\tG3lfG8P2miBpw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1670499305;\n\tbh=SbKnGKq+2zQgz31Nf7zNNCbjhPNaLOqnRwfPFYXopGo=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=IbMS58c6QctnAbqDsrotB67mTs88bwKUm4CpwMIpUEt5SRtFmxvlPdUlQsXtAvmng\n\tNoUeIafOIORIOD4rSK/9VYrWMAd91IuWyRgqoIUEcBBa8fpwyjhktOdW6M8qCV5U/0\n\t/fko2FyFQDHjPxQtwXDYythxR54KCu0vbaovcNPI="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"IbMS58c6\"; dkim-atps=neutral","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<167049906280.9133.5362324309443207786@Monstersaurus>","References":"<20221117154500.8829-1-david.plowman@raspberrypi.com>\n\t<20221117154500.8829-2-david.plowman@raspberrypi.com>\n\t<166870106589.50677.16904470229012395935@Monstersaurus>\n\t\n\t<166872639942.1300052.15323145116698928957@Monstersaurus>\n\t<20221118082136.ihcjxe46vyacsrtj@uno.localdomain>\n\t\n\t<167049906280.9133.5362324309443207786@Monstersaurus>","To":"David Plowman ,\n\tJacopo Mondi ","Date":"Thu, 08 Dec 2022 11:35:02 +0000","Message-ID":"<167049930255.9133.11856255309228418704@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Kieran Bingham via libcamera-devel\n\t","Reply-To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":26029,"web_url":"https://patchwork.libcamera.org/comment/26029/","msgid":"","date":"2022-12-08T11:43:47","subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Great, thank you Kieran!\n\nDavid\n\nOn Thu, 8 Dec 2022 at 11:35, Kieran Bingham\n wrote:\n>\n> Quoting Kieran Bingham (2022-12-08 11:31:02)\n> > Hi David,\n> >\n> > Quoting David Plowman (2022-12-08 11:22:19)\n> > > Hi everyone\n> > >\n> > > Thanks for the comments on this change. I was wondering if I might\n> > > give it a little nudge if there are no objections anywhere else?\n> > >\n> > > Thanks!\n> > > David\n> > >\n> > > On Fri, 18 Nov 2022 at 08:21, Jacopo Mondi wrote:\n> > > >\n> > > > Hello\n> > > >\n> > > > On Thu, Nov 17, 2022 at 11:06:39PM +0000, Kieran Bingham via libcamera-devel wrote:\n> > > > > Quoting David Plowman (2022-11-17 16:11:38)\n> > > > > > Hi Kieran\n> > > > > >\n> > > > > > On Thu, 17 Nov 2022 at 16:04, Kieran Bingham\n> > > > > > wrote:\n> > > > > > >\n> > > > > > > Hi David,\n> > > > > > >\n> > > > > > > Quoting David Plowman via libcamera-devel (2022-11-17 15:45:00)\n> > > > > > > > The units for the LensPosition control, previously defined as being in\n> > > > > > > > units of 1 / hyperfocal_distance, are changed to 1 / distance (in\n> > > > > > > > metres).\n> > > > > > > >\n> > > > > > > > Signed-off-by: David Plowman \n> > > > > > > > ---\n> > > > > > > > src/libcamera/control_ids.yaml | 26 ++++++++++++++------------\n> > > > > > > > 1 file changed, 14 insertions(+), 12 deletions(-)\n> > > > > > > >\n> > > > > > > > diff --git a/src/libcamera/control_ids.yaml b/src/libcamera/control_ids.yaml\n> > > > > > > > index a456e6c0..adea5f90 100644\n> > > > > > > > --- a/src/libcamera/control_ids.yaml\n> > > > > > > > +++ b/src/libcamera/control_ids.yaml\n> > > > > > > > @@ -591,25 +591,27 @@ controls:\n> > > > > > > > AfModeManual, though the value is reported back unconditionally in all\n> > > > > > > > modes.\n> > > > > > > >\n> > > > > > > > - The units are a reciprocal distance scale like dioptres but normalised\n> > > > > > > > - for the hyperfocal distance. That is, for a lens with hyperfocal\n> > > > > > > > - distance H, and setting it to a focal distance D, the lens position LP,\n> > > > > > > > - which is generally a non-integer, is given by\n> > > > > > > > + This value, which is generally a non-integer, is the reciprocal of the\n> > > > > > > > + focal distance in metres, also known as dioptres. That is, to set a\n> > > > > > > > + focal distance D, the lens position LP is given by\n> > > > > > > >\n> > > > > > > > - \\f$LP = \\frac{H}{D}\\f$\n> > > > > > > > + \\f$LP = \\frac{1\\mathrm{m}}{D}\\f$\n> > > > > > > >\n> > > > > > > > For example:\n> > > > > > > >\n> > > > > > > > 0 moves the lens to infinity.\n> > > > > > > > - 0.5 moves the lens to twice the hyperfocal distance.\n> > > > > > > > - 1 moves the lens to the hyperfocal position.\n> > > > > > > > - And larger values will focus the lens ever closer.\n> > > > > > > > + 0.5 moves the lens to focus on objects 2m away.\n> > > > > > > > + 2 moves the lens to focus on objects 50cm away.\n> > > > > > > > + And larger values will focus the lens closer.\n> > > > > > > >\n> > > > > > > > - \\todo Define a property to report the Hyperforcal distance of calibrated\n> > > > > > > > - lenses.\n> > > > > > > > + The default value of the control should indicate a good general position\n> > > > > > > > + for the lens, often corresponding to the hyperfocal distance (the\n> > > > > > > > + closest position for which objects at infinity are still acceptably\n> > > > > > > > + sharp). The minimum will often be zero (meaning infinity), and the\n> > > > > > > > + maximum value defines the closest focus position.\n> > > > > > > >\n> > > > > > > > - \\todo Define a property to report the maximum and minimum positions of\n> > > > > > > > - this lens. The minimum value will often be zero (meaning infinity).\n> > > > > > > > + \\todo Define a property to report the Hyperfocal distance of calibrated\n> > > > > > > > + lenses.\n> > > > > > >\n> > > > > > > Is this going to be a separate property, or the 'default' value of the\n> > > > > > > control?\n> > > > > >\n> > > > > > We're thinking we're going to use the default value of the control.\n> > > > > >\n> > > > > > Actually I think there is still a point to a \"hyperfocal\" property,\n> > > > > > because it's not guaranteed that the \"default\" position is the\n> > > > > > hyperfocal one. But in our applications, I think we're taking the view\n> > > > > > that we will always use this \"default\" position. Nearly always it will\n> > > > > > be hyperfocal, and if not, it will be because we want to put the lens\n> > > > > > somewhere else.\n> > > > >\n> > > > > Using the 'default' value, we can set it if we know we have one, or\n> > > > > leave the default as unset if there isn't a known one yet.\n> > > > >\n> > > > > We can always add a dedicated hyperfocal property later if it's\n> > > > > applicable easily.\n> > > > >\n> > > >\n> > > > I tend to agree with all the above...\n> > > >\n> > > > It's been half an year since we have been trying to standardize\n> > > > this property on \"1 / hyperfocal distance\" and so far not a single\n> > > > lens has been calibrated, mostly because the exact calibration of the\n> > > > the hyperfocal distance is a rather complex process, if not\n> > > > impossibile for uncalibrated lenses ?\n> > > >\n> > > > Usage of dioptres means an esier calibration process if I understand\n> > > > things correctly, by simply using a rule meter and a procedure more\n> > > > accessible for non professional users, right ?\n> > > >\n> > > > Anyway, it then remains the fact we need a proper CameraLensHelper to\n> > > > translate from the generic unit to the exact v4l2-control value, same\n> > > > as it happens for the gain values, and if this unblocks that\n> > > > development I'm all for it..\n> > > >\n> > > >\n> > > > > I expect we'll still need some calibration on the VCM/lens anyway to be\n> > > > > able to map a distance to the range of the underlying controls, and I\n> > > > > suspect there will be some slight imprecisions, but at least this would\n> > > > > let us report (approximate) distance information in the metadata too!\n> > > > >\n> > > > > Should we be considering renaming this to FocusPosition / FocalPoint /\n> > > > > FocalLength ? (As it's the focal point we're trying to move rather than\n> > > > > a specific position of the lens ?) - And that might make more sense when\n> > > > > reporting the position in the metadata ?\n> > > > >\n> > > > > Anyway, I don't see anything against this change, and as you added the\n> > > > > property in the first place, I think you're best suited to know if it\n> > > > > was better this way at the moment, so:\n> > > > >\n> > > > >\n> > > > > Reviewed-by: Kieran Bingham \n> > > >\n> > > > Likewise\n> > > > Reviewed-by: Jacopo Mondi \n> > > >\n> >\n> > I see two RB tags, so I'll run this through the tests and merge if\n> > nothing here breaks!\n> >\n>\n> Ahem, with no actual code change - nothing can break ;-) So I guess this\n> gets an easy pass.\n> --\n> Kieran\n>\n>\n> > --\n> > Kieran\n> >\n> >\n> >\n> > > > >\n> > > > > Lets see what anyone else says ...\n> > > > >\n> > > > >\n> > > > > > > In the cover letter you mentioned \"we've resolved that we will be\n> > > > > > > able to read a good default position from the control info.\"\n> > > > > > >\n> > > > > > > Presumably, that means that when lacking other information, '1' is a\n> > > > > > > good default value?\n> > > > > >\n> > > > > > Lacking other information, numbers around 1 are probably not a bad\n> > > > > > guess for many of our small modules.\n> > > > > >\n> > > > > > David\n> > > > > >\n> > > > > > >\n> > > > > > >\n> > > > > > > >\n> > > > > > > > - AfState:\n> > > > > > > > type: int32_t\n> > > > > > > > --\n> > > > > > > > 2.30.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 DF8EFBE08B\n\tfor ;\n\tThu, 8 Dec 2022 11:44:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5E3A56334A;\n\tThu, 8 Dec 2022 12:44:01 +0100 (CET)","from mail-pf1-x434.google.com (mail-pf1-x434.google.com\n\t[IPv6:2607:f8b0:4864:20::434])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BCF7361F1C\n\tfor ;\n\tThu, 8 Dec 2022 12:43:59 +0100 (CET)","by mail-pf1-x434.google.com with SMTP id k79so1080414pfd.7\n\tfor ;\n\tThu, 08 Dec 2022 03:43:59 -0800 (PST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1670499841;\n\tbh=DWsZ6LZ8xt6Bqr40/BZFSQqfLy3iEXRlCtVpwDWzkTY=;\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=jA9smCll2DaZ1f/bMCvO+w69D0PheWP80T4E+BVM+0FSiZjUeq9S847TreP7Yqenm\n\t2BPYdHb51jR/VS76o+FcqIBmV9ml0nWmvmBe+ipq1zZBnKpz8uOU5ENM4oeUJhY5X4\n\t+zeJwCvx+ec0mkp8NxmabxmY0cSFMqW/F2G5cDnsVPnNLVytc70pSGk31/PnjMwHJQ\n\tLUabTUkaWz1FOITM2v9rX0AUSWPj8ZMw8zYGsMoIPaSvYcsr2oYZZPYghhptNMItLq\n\tdZVidoYBW9gNki5KxtCMCQVfD5v0Z7e/fXG5L91jNbt9T0N23v192HG8zvulQEt9jr\n\tUpp5bTDjdmE9Q==","v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=b/BSOwHsruMxigp/R2RnrFAX36OCCqNXh3QafiSnGF0=;\n\tb=hY7VUsI9wMeiFDx6Z4F16e1WF6EnWmJFTACGXUdXE8d0APAtOS7JvtUi+GB2w88+7F\n\tJXgzeQWLPAHOMgUqof2P7+MPgLL9hMLdQNhJktgzkGvdkHtE+NWY/5YUw1JZ3ww9iVut\n\t3aS+BrLkXKiAqt7vZhIqvu2QBkbdIPbi64PPyWIJn7aee9dXY6eKcVXbXt46PKSCgF5D\n\t6Elkg8815j8lA56+6OpTtLmir15oN9QAEe67+eeFYdxFS0bWeuVSl4pb4q6wtM0q+F2n\n\tYOtWpV0RxhYljaAY7w068czBgA6N5OFzC2D32DJimeKaMI8ykUa8ATF4B5nL6cRQ2ZCc\n\tdQ8A=="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected) header.d=raspberrypi.com\n\theader.i=@raspberrypi.com\n\theader.b=\"hY7VUsI9\"; dkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=b/BSOwHsruMxigp/R2RnrFAX36OCCqNXh3QafiSnGF0=;\n\tb=suB1MK5xDeLho3o7ZjkUSt2q+ISpN1y+YdQXwZ4q2Wmz3sNRtCNijC58qnUuwoggcA\n\tSHeJRA97WwguDuAw0cFUx9pQxiRgtfMyf4XU31XdvAzyuhJrHBNIkbcWcS3KNWxB/6G3\n\tkAfZUotUi/r30WEyboJDkd4pR7e8K3ZafsONJ12l9+mmOmt1k59OKm8yIJ2JNFlN1t+j\n\tcADBNVYrKXC0vFtrW7JdMPKObLH7Hc8848PPWy8sJE1arEb+RSdWCogQ0MsRT+KnAS0s\n\tjrmWoLXvChcqtDB2hJ8WdF8UPnJ/TuDEXYuaFkG+qVpgc+Cp92aWwD+vszwXTMaJaEaC\n\tbg0A==","X-Gm-Message-State":"ANoB5pmefp2OXKCsOLnytKTm9PpehSqrk4Pr1jHHMOkUJ3KaiuZJS2Xf\n\tDivirvMgQ7p+u6SggisYsh1iCW4fFqPvj38uCqvuqg==","X-Google-Smtp-Source":"AA0mqf648p0pNlgvjtByvCKLU2XeaE2Bsh1gB14ZdQHW8u+1SrqLyWTkAvOoF2gLMEdVtJUsJxIfvoM42OZ8BAxP0Zs=","X-Received":"by 2002:a05:6a00:3409:b0:577:d130:6ff0 with SMTP id\n\tcn9-20020a056a00340900b00577d1306ff0mr185616pfb.78.1670499838152;\n\tThu, 08 Dec 2022 03:43:58 -0800 (PST)","MIME-Version":"1.0","References":"<20221117154500.8829-1-david.plowman@raspberrypi.com>\n\t<20221117154500.8829-2-david.plowman@raspberrypi.com>\n\t<166870106589.50677.16904470229012395935@Monstersaurus>\n\t\n\t<166872639942.1300052.15323145116698928957@Monstersaurus>\n\t<20221118082136.ihcjxe46vyacsrtj@uno.localdomain>\n\t\n\t<167049906280.9133.5362324309443207786@Monstersaurus>\n\t<167049930255.9133.11856255309228418704@Monstersaurus>","In-Reply-To":"<167049930255.9133.11856255309228418704@Monstersaurus>","Date":"Thu, 8 Dec 2022 11:43:47 +0000","Message-ID":"","To":"Kieran Bingham ","Content-Type":"text/plain; charset=\"UTF-8\"","Subject":"Re: [libcamera-devel] [PATCH 1/1] libcamera: controls: Change\n\tLensPosition units to dioptres","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":"David Plowman via libcamera-devel ","Reply-To":"David Plowman ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]