[libcamera-devel] [PATCH v4 6/6] DNI: libcamera: sensor: ov5670: Add lens properties
Jacopo Mondi
jacopo at jmondi.org
Mon Apr 20 12:09:27 CEST 2020
Hi Niklas,
On Wed, Apr 08, 2020 at 01:09:21AM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your discussion points ;-)
>
> On 2020-03-26 15:59:27 +0100, Jacopo Mondi wrote:
> > Register lens properties in the ov5670 sensor handler.
> >
> > This patch is not intended for merge as we know lens properties do no
> > belong to the sensor handler, but I am including it anyhow to trigger
> > discussions on where they would be more appropriately defined.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > src/libcamera/sensor/ov5670.cpp | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/src/libcamera/sensor/ov5670.cpp b/src/libcamera/sensor/ov5670.cpp
> > index d7339b9792e1..8c6a6114c9bf 100644
> > --- a/src/libcamera/sensor/ov5670.cpp
> > +++ b/src/libcamera/sensor/ov5670.cpp
> > @@ -46,6 +46,12 @@ int OV5670::initProperties()
> > properties::BayerFilterGRBG);
> > properties_.set(properties::ISOSensitivityRange, { 50, 800 });
> >
> > + /* Lens Properties. */
> > + properties_.set(properties::LensApertures, { 0.0f });
> > + properties_.set(properties::LensFocalLengths, { 3.69f });
>
> Would it make sens to try and record aperture information in device
> tree? It is one form of hardware description right? We still will need a
> way to express them some other way, maybe in a platform configuration
> file?
>
That's an open discussion, the patch is DNI for this reason :)
I see two possible issues in going for a DT based solution.
1) We don't only have DT based devices
But actually for ACPI based platforms there seems to be already
support for parsing lens related properties
https://elixir.bootlin.com/linux/latest/source/drivers/media/v4l2-core/v4l2-fwnode.c#L1093
They're device properties
https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/media/video-interfaces.txt#L80
As well as the camera module related properties we have (almost)
upstreamed
https://patchwork.kernel.org/project/linux-media/list/?series=272849
So they might be a good fit, but
2) Lens properties depends on the camera module module, not on the sensor
The DT/ACPI describes the image sensor as an i2c device. The
board/platform .dts file usually adds an i2c device child node to
describe the image sensor on the i2c bus, but the same sensor could
actually be coupled with different lenses in different casing depending
on the camera module manufacturer. Putting lens properties there would
make the i2c child node describe the camera module, not the sensor
only. I can't tell how a big deal that would be, to me it's more
than acceptable if not desirable, but I might be missing some issue.
Another option would be to provide a phandle in the i2c child node that
points to a 'lens' device node which would only contain read-only
properties but I'm not sure how well that would play with the
existing 'lens-focus' property that points to the lens VCM
controller...
3) Stand-alone camera modules report lens/casing information, it's
harder to get the same info for image sensor integrated in a device
like a laptop/tablet/phone.
A configuration file opens up a lot of other issues, mostly regarding
distribution of device specific information that libcamera would
require to function properyl, and we have refreined from going that way
for this reason.
> > + properties_.set(properties::LensHyperfocalDistances, { 0.0f });
> > + properties_.set(properties::LensMinimumFocusDistance, 3.69f);
>
> For these I'm not sure as i recall my optics they some what depends to
> some degree on the properties above?
>
Do you mean they could be calculated ? I honestly can't tell without
refreshing my already poor knowledge of optics...
> > +
> > return CameraSensor::initProperties();
> > }
> >
> > --
> > 2.25.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund
More information about the libcamera-devel
mailing list