[{"id":4409,"web_url":"https://patchwork.libcamera.org/comment/4409/","msgid":"<20200407230921.GZ1716317@oden.dyn.berto.se>","date":"2020-04-07T23:09:21","subject":"Re: [libcamera-devel] [PATCH v4 6/6] DNI: libcamera: sensor:\n\tov5670: Add lens properties","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your discussion points ;-)\n\nOn 2020-03-26 15:59:27 +0100, Jacopo Mondi wrote:\n> Register lens properties in the ov5670 sensor handler.\n> \n> This patch is not intended for merge as we know lens properties do no\n> belong to the sensor handler, but I am including it anyhow to trigger\n> discussions on where they would be more appropriately defined.\n> \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/sensor/ov5670.cpp | 6 ++++++\n> 1 file changed, 6 insertions(+)\n> \n> diff --git a/src/libcamera/sensor/ov5670.cpp b/src/libcamera/sensor/ov5670.cpp\n> index d7339b9792e1..8c6a6114c9bf 100644\n> --- a/src/libcamera/sensor/ov5670.cpp\n> +++ b/src/libcamera/sensor/ov5670.cpp\n> @@ -46,6 +46,12 @@ int OV5670::initProperties()\n> \t\t\tproperties::BayerFilterGRBG);\n> \tproperties_.set(properties::ISOSensitivityRange, { 50, 800 });\n> \n> +\t/* Lens Properties. */\n> +\tproperties_.set(properties::LensApertures, { 0.0f });\n> +\tproperties_.set(properties::LensFocalLengths, { 3.69f });\n\nWould it make sens to try and record aperture information in device \ntree? It is one form of hardware description right? We still will need a \nway to express them some other way, maybe in a platform configuration \nfile?\n\n> +\tproperties_.set(properties::LensHyperfocalDistances, { 0.0f });\n> +\tproperties_.set(properties::LensMinimumFocusDistance, 3.69f);\n\nFor these I'm not sure as i recall my optics they some what depends to \nsome degree on the properties above?\n\n> +\n> \treturn CameraSensor::initProperties();\n> }\n> \n> -- \n> 2.25.1\n> \n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","headers":{"Return-Path":"","Received":["from mail-lf1-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 952DA600F3\n\tfor ;\n\tWed, 8 Apr 2020 01:09:22 +0200 (CEST)","by mail-lf1-x143.google.com with SMTP id m19so1673022lfq.13\n\tfor ;\n\tTue, 07 Apr 2020 16:09:22 -0700 (PDT)","from localhost (h-200-138.A463.priv.bahnhof.se. [176.10.200.138])\n\tby smtp.gmail.com with ESMTPSA id\n\tu25sm14156495lfo.71.2020.04.07.16.09.21\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 07 Apr 2020 16:09:21 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"T567DkK3\"; \n\tdkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=++hEyzKwGOTPgK0qSlapX8iik3IjTfcTc6bFrGLkG40=;\n\tb=T567DkK3iqyVQf7NhUPgYptQLyAVN41sBiDOr0TjrklW8B9eBQ1zaQdMEwel3c4kwb\n\ttfmbOsR+eoGOCavlXHz8ScZtbglq7NvEP8xjAADjv2AWxsb1d9zRRSqwPmbBfFw2t7gZ\n\tIlbJGehODzCWYWbEEdFSVa8Yr43v1ttscnx7wqwIHOkRLMMic9N2dMwejyju2DwAjbe1\n\txXiB9k4EA0n7BL/8TlSO8us5K3w9W6t5etJMMIJdJBAA+kBCgE28bQCchjtekeoe6x3K\n\tanS7DKsXEu7jg9UwIqrsZ5MoicY0Z4kJUXK8wPgdNSK2BctxAc4PBQzJlsIvoDt3o2Od\n\tN+mQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=++hEyzKwGOTPgK0qSlapX8iik3IjTfcTc6bFrGLkG40=;\n\tb=d2xzw0OEyHO1GBeSyYj80LQ6MmTBcdPmRbemNpk0Zp90bgZNuge1H8A/IV4+Gcxttw\n\tTdYAhMGlDg+3Ls3dOoqHVhKGd6ArlWI80cDR/1J6+MMsx+kMCklLb9uDfgMQdrMPaoES\n\t7zQ/2YttWnrDMb+4M9lH606KrGqCYPYsMfqJYxRYonSL8mNSAc0V4kUPTWUziKJauwYu\n\tvwB767HCETPMSC4/OgwGumYj+IKTnh8ugHCqIiEg5eh91TXK/o6Ivl+evzn7xl8NM5o9\n\t3jEn1V8vpIju/1azyK8bl6VJ1t8gCBN8em1d0gYRGLov5N2nEu1iIuOyP4iCJbAzvG5z\n\tJoWg==","X-Gm-Message-State":"AGi0PuaeoyAlVo9b6G8GVU0geMYOAU7ea2QhZFAecrBP4BlyR4q5h2ps\n\tVe8SAjuAUbmYzliibIaIO/w/Iw==","X-Google-Smtp-Source":"APiQypJeRgrS27dJ3yirG3C8+B+ZPo/nbq6Gl0QXAqqhyzkZOkRJtpMGRnoZ6UDU66qAZSjTaECM2A==","X-Received":"by 2002:a19:c8cf:: with SMTP id\n\ty198mr2604425lff.197.1586300962022; \n\tTue, 07 Apr 2020 16:09:22 -0700 (PDT)","Date":"Wed, 8 Apr 2020 01:09:21 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200407230921.GZ1716317@oden.dyn.berto.se>","References":"<20200326145927.324919-1-jacopo@jmondi.org>\n\t<20200326145927.324919-7-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200326145927.324919-7-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v4 6/6] DNI: libcamera: sensor:\n\tov5670: Add lens properties","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","X-List-Received-Date":"Tue, 07 Apr 2020 23:09:22 -0000"}},{"id":4473,"web_url":"https://patchwork.libcamera.org/comment/4473/","msgid":"<20200420100927.kmts4izdzs3tjhen@uno.localdomain>","date":"2020-04-20T10:09:27","subject":"Re: [libcamera-devel] [PATCH v4 6/6] DNI: libcamera: sensor:\n\tov5670: Add lens properties","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Wed, Apr 08, 2020 at 01:09:21AM +0200, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your discussion points ;-)\n>\n> On 2020-03-26 15:59:27 +0100, Jacopo Mondi wrote:\n> > Register lens properties in the ov5670 sensor handler.\n> >\n> > This patch is not intended for merge as we know lens properties do no\n> > belong to the sensor handler, but I am including it anyhow to trigger\n> > discussions on where they would be more appropriately defined.\n> >\n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/sensor/ov5670.cpp | 6 ++++++\n> > 1 file changed, 6 insertions(+)\n> >\n> > diff --git a/src/libcamera/sensor/ov5670.cpp b/src/libcamera/sensor/ov5670.cpp\n> > index d7339b9792e1..8c6a6114c9bf 100644\n> > --- a/src/libcamera/sensor/ov5670.cpp\n> > +++ b/src/libcamera/sensor/ov5670.cpp\n> > @@ -46,6 +46,12 @@ int OV5670::initProperties()\n> > \t\t\tproperties::BayerFilterGRBG);\n> > \tproperties_.set(properties::ISOSensitivityRange, { 50, 800 });\n> >\n> > +\t/* Lens Properties. */\n> > +\tproperties_.set(properties::LensApertures, { 0.0f });\n> > +\tproperties_.set(properties::LensFocalLengths, { 3.69f });\n>\n> Would it make sens to try and record aperture information in device\n> tree? It is one form of hardware description right? We still will need a\n> way to express them some other way, maybe in a platform configuration\n> file?\n>\n\nThat's an open discussion, the patch is DNI for this reason :)\n\nI see two possible issues in going for a DT based solution.\n\n1) We don't only have DT based devices\n\n But actually for ACPI based platforms there seems to be already\n support for parsing lens related properties\n https://elixir.bootlin.com/linux/latest/source/drivers/media/v4l2-core/v4l2-fwnode.c#L1093\n\n They're device properties\n https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/media/video-interfaces.txt#L80\n\n As well as the camera module related properties we have (almost)\n upstreamed\n https://patchwork.kernel.org/project/linux-media/list/?series=272849\n\n So they might be a good fit, but\n\n2) Lens properties depends on the camera module module, not on the sensor\n\n The DT/ACPI describes the image sensor as an i2c device. The\n board/platform .dts file usually adds an i2c device child node to\n describe the image sensor on the i2c bus, but the same sensor could\n actually be coupled with different lenses in different casing depending\n on the camera module manufacturer. Putting lens properties there would\n make the i2c child node describe the camera module, not the sensor\n only. I can't tell how a big deal that would be, to me it's more\n than acceptable if not desirable, but I might be missing some issue.\n Another option would be to provide a phandle in the i2c child node that\n points to a 'lens' device node which would only contain read-only\n properties but I'm not sure how well that would play with the\n existing 'lens-focus' property that points to the lens VCM\n controller...\n\n3) Stand-alone camera modules report lens/casing information, it's\n harder to get the same info for image sensor integrated in a device\n like a laptop/tablet/phone.\n\nA configuration file opens up a lot of other issues, mostly regarding\ndistribution of device specific information that libcamera would\nrequire to function properyl, and we have refreined from going that way\nfor this reason.\n\n\n> > +\tproperties_.set(properties::LensHyperfocalDistances, { 0.0f });\n> > +\tproperties_.set(properties::LensMinimumFocusDistance, 3.69f);\n>\n> For these I'm not sure as i recall my optics they some what depends to\n> some degree on the properties above?\n>\n\nDo you mean they could be calculated ? I honestly can't tell without\nrefreshing my already poor knowledge of optics...\n\n> > +\n> > \treturn CameraSensor::initProperties();\n> > }\n> >\n> > --\n> > 2.25.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\n>\n> --\n> Regards,\n> Niklas Söderlund","headers":{"Return-Path":"","Received":["from relay8-d.mail.gandi.net (relay8-d.mail.gandi.net\n\t[217.70.183.201])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DF48062E55\n\tfor ;\n\tMon, 20 Apr 2020 12:06:27 +0200 (CEST)","from uno.localdomain (unknown [87.13.136.104])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay8-d.mail.gandi.net (Postfix) with ESMTPSA id 741221BF22B;\n\tMon, 20 Apr 2020 10:06:23 +0000 (UTC)"],"X-Originating-IP":"87.13.136.104","Date":"Mon, 20 Apr 2020 12:09:27 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200420100927.kmts4izdzs3tjhen@uno.localdomain>","References":"<20200326145927.324919-1-jacopo@jmondi.org>\n\t<20200326145927.324919-7-jacopo@jmondi.org>\n\t<20200407230921.GZ1716317@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200407230921.GZ1716317@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH v4 6/6] DNI: libcamera: sensor:\n\tov5670: Add lens properties","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","X-List-Received-Date":"Mon, 20 Apr 2020 10:06:28 -0000"}}]