[libcamera-devel,RFC,7/7] libcamera: sensor: ov5670: Add lens properties

Message ID 20191218145001.22283-8-jacopo@jmondi.org
State Superseded
Delegated to: Jacopo Mondi
Headers show
Series
  • Define and register 'sensor' and 'lens' properties
Related show

Commit Message

Jacopo Mondi Dec. 18, 2019, 2:50 p.m. UTC
Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/sensor/ov5670.cpp | 6 ++++++
 1 file changed, 6 insertions(+)

Comments

Andrey Konovalov Dec. 18, 2019, 7:41 p.m. UTC | #1
Hi Jacopo,

On 18.12.2019 17:50, Jacopo Mondi wrote:
> Signed-off-by: Jacopo Mondi <jacopo@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 c2d996785717..a25bfd246f8b 100644
> --- a/src/libcamera/sensor/ov5670.cpp
> +++ b/src/libcamera/sensor/ov5670.cpp
> @@ -30,6 +30,12 @@ int OV5670CameraSensor::initProperties(const ControlInfoMap &controlMap)
>   	properties_.set(properties::BayerFilterArrangement, bayerFilter);
>   	properties_.set(properties::ISOSensitivityRange, { 50, 800 });
>   
> +	/* Lens Properties. */
> +	properties_.set(properties::LensApertures, 0.0f);

What is the meaning of the aperture of zero?

> +	properties_.set(properties::LensFocalDistance, 3.69f);

This means that the lens has the focal length of 3.69 mm, correct?

> +	properties_.set(properties::LensHyperfocalDistance, 0.0f);

Same question as for the aperture above (zero value for the property which is essentially non-zero)

> +	properties_.set(properties::LensMinimumFocalDistance, 3.69f);

Why is it the same as the focal length?
If this is "The shortest distance in millimeters from the lens surface in which an
object could be brought into sharp focus", then setting it to 3.69 is most probably
wrong...


Thanks,
Andrey

> +
>   	return CameraSensor::initProperties(controlMap);
>   }
>   
>
Jacopo Mondi Dec. 19, 2019, 11:59 a.m. UTC | #2
Hi Andrey,

   I premit I don't have a manual for the sensor nor the optical
   specification, so I referred to the ChromeOS BSP to retrieve this
   information. All the values here are copied from there.

On Wed, Dec 18, 2019 at 10:41:59PM +0300, Andrey Konovalov wrote:
> Hi Jacopo,
>
> On 18.12.2019 17:50, Jacopo Mondi wrote:
> > Signed-off-by: Jacopo Mondi <jacopo@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 c2d996785717..a25bfd246f8b 100644
> > --- a/src/libcamera/sensor/ov5670.cpp
> > +++ b/src/libcamera/sensor/ov5670.cpp
> > @@ -30,6 +30,12 @@ int OV5670CameraSensor::initProperties(const ControlInfoMap &controlMap)
> >   	properties_.set(properties::BayerFilterArrangement, bayerFilter);
> >   	properties_.set(properties::ISOSensitivityRange, { 50, 800 });
> > +	/* Lens Properties. */
> > +	properties_.set(properties::LensApertures, 0.0f);
>
> What is the meaning of the aperture of zero?
>
> > +	properties_.set(properties::LensFocalDistance, 3.69f);
>
> This means that the lens has the focal length of 3.69 mm, correct?
>

very short right ? Can't tell if it's plausible or not though

> > +	properties_.set(properties::LensHyperfocalDistance, 0.0f);
>
> Same question as for the aperture above (zero value for the property which is essentially non-zero)
>
> > +	properties_.set(properties::LensMinimumFocalDistance, 3.69f);
>
> Why is it the same as the focal length?
> If this is "The shortest distance in millimeters from the lens surface in which an
> object could be brought into sharp focus", then setting it to 3.69 is most probably
> wrong...
>

Probably, but as I've said those values come from the ChromeOS BSP and
I think they're overridden by the binary 3A implementation for the
platform where the sensor is installed on. Honestly, I am not sure
how to retrieve those values from if not from there...

Thanks for you feedback
    j


>
> Thanks,
> Andrey
>
> > +
> >   	return CameraSensor::initProperties(controlMap);
> >   }
> >
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Andrey Konovalov Dec. 27, 2019, 5:30 p.m. UTC | #3
Hi Jacopo,

On 19.12.2019 14:59, Jacopo Mondi wrote:
> Hi Andrey,
> 
>     I premit I don't have a manual for the sensor nor the optical
>     specification, so I referred to the ChromeOS BSP to retrieve this
>     information. All the values here are copied from there.
> 
> On Wed, Dec 18, 2019 at 10:41:59PM +0300, Andrey Konovalov wrote:
>> Hi Jacopo,
>>
>> On 18.12.2019 17:50, Jacopo Mondi wrote:
>>> Signed-off-by: Jacopo Mondi <jacopo@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 c2d996785717..a25bfd246f8b 100644
>>> --- a/src/libcamera/sensor/ov5670.cpp
>>> +++ b/src/libcamera/sensor/ov5670.cpp
>>> @@ -30,6 +30,12 @@ int OV5670CameraSensor::initProperties(const ControlInfoMap &controlMap)
>>>    	properties_.set(properties::BayerFilterArrangement, bayerFilter);
>>>    	properties_.set(properties::ISOSensitivityRange, { 50, 800 });
>>> +	/* Lens Properties. */
>>> +	properties_.set(properties::LensApertures, 0.0f);
>>
>> What is the meaning of the aperture of zero?

Zero value of aperture looks like "value not available" for me.
(In strict sense, if the aperture is exactly zero then no light can pass through)

>>> +	properties_.set(properties::LensFocalDistance, 3.69f);
>>
>> This means that the lens has the focal length of 3.69 mm, correct?
>>
> 
> very short right ? Can't tell if it's plausible or not though

For ov5670 (1/5" sized sensor with the image area of about 2.9mm x 2.2mm) this is quite typical focal length.
It will give approximately the same field of view as a 44mm focal length lens on a full frame (36mm x 24mm sensor) camera.
(A "normal" lens for a full frame camera - which produces the perspective closest to how a human eye see - is considered
to be a 50mm one)

>>> +	properties_.set(properties::LensHyperfocalDistance, 0.0f);

Hyperfocal distance of 0 is invalid. So I can only take it as "not known" or "not specified".

>> Same question as for the aperture above (zero value for the property which is essentially non-zero)
>>
>>> +	properties_.set(properties::LensMinimumFocalDistance, 3.69f);
>>
>> Why is it the same as the focal length?
>> If this is "The shortest distance in millimeters from the lens surface in which an
>> object could be brought into sharp focus", then setting it to 3.69 is most probably
>> wrong...

I meant that the minimal *focus distance* being equal to properties::LensFocalDistance is by 99.99% wrong.
But if "properties::LensMinimumFocalDistance" is misspelled "properties::LensMinimumFocalLength", then the same value
as properties::LensFocalDistance is correct (for a fixed focal length lens).

> Probably, but as I've said those values come from the ChromeOS BSP and
> I think they're overridden by the binary 3A implementation for the
> platform where the sensor is installed on. Honestly, I am not sure
> how to retrieve those values from if not from there...

All those numbers are characteristics of a camera module, the lens used in particular.
So all these numbers must come from the camera module or the lens specification.
Of course the manufacturer is not obliged to share this info with its customers, and we can't do
much with that.

Guess the (special) value used for such a property when the actual value of the property is not known
must be explicitly defined somewhere.

Thanks,
Andrey

> Thanks for you feedback
>      j
> 
> 
>>
>> Thanks,
>> Andrey
>>
>>> +
>>>    	return CameraSensor::initProperties(controlMap);
>>>    }
>>>
>> _______________________________________________
>> libcamera-devel mailing list
>> libcamera-devel@lists.libcamera.org
>> https://lists.libcamera.org/listinfo/libcamera-devel

Patch

diff --git a/src/libcamera/sensor/ov5670.cpp b/src/libcamera/sensor/ov5670.cpp
index c2d996785717..a25bfd246f8b 100644
--- a/src/libcamera/sensor/ov5670.cpp
+++ b/src/libcamera/sensor/ov5670.cpp
@@ -30,6 +30,12 @@  int OV5670CameraSensor::initProperties(const ControlInfoMap &controlMap)
 	properties_.set(properties::BayerFilterArrangement, bayerFilter);
 	properties_.set(properties::ISOSensitivityRange, { 50, 800 });
 
+	/* Lens Properties. */
+	properties_.set(properties::LensApertures, 0.0f);
+	properties_.set(properties::LensFocalDistance, 3.69f);
+	properties_.set(properties::LensHyperfocalDistance, 0.0f);
+	properties_.set(properties::LensMinimumFocalDistance, 3.69f);
+
 	return CameraSensor::initProperties(controlMap);
 }