[libcamera-devel,RFC] android: Plumb lens focus distance
diff mbox series

Message ID 20210913073125.1755107-1-paul.elder@ideasonboard.com
State New
Delegated to: Paul Elder
Headers show
Series
  • [libcamera-devel,RFC] android: Plumb lens focus distance
Related show

Commit Message

Paul Elder Sept. 13, 2021, 7:31 a.m. UTC 
Plumb the controls related to lens focus distance:
- LENS_INFO_FOCUS_DISTANCE_CALIBRATION (static)
- LENS_INFO_MINIMUM_FOCUS_DISTANCE (static)
- LENS_FOCUS_DISTANCE (request, result)

Conceptually, the first two controls determine the range of possible
values of the third. None of our lenses are calibrated, so hardcode the
first to uncalibrated. The valid focus distances are then [0, min].
Since we don't yet support variable-focus lenses, hardcode the second to
0.0f, making the range of valid focus distances [0, 0]. Hence, hardcode
the focus distance in the result metadata to 0, and ignore the control
in the request. Add todos for adding proper controls later, once we have
focus support.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
This is also on top of my ctsw branch, so it won't apply on master, but
I would like comments to confirm the direction and design.
---
 src/android/camera_capabilities.cpp | 13 +++++++++++++
 src/android/camera_device.cpp       |  4 ++++
 2 files changed, 17 insertions(+)

Comments

Laurent Pinchart Sept. 14, 2021, 11:26 p.m. UTC | #1 
Hi Paul,

Thank you for the patch.

On Mon, Sep 13, 2021 at 04:31:25PM +0900, Paul Elder wrote:
> Plumb the controls related to lens focus distance:
> - LENS_INFO_FOCUS_DISTANCE_CALIBRATION (static)
> - LENS_INFO_MINIMUM_FOCUS_DISTANCE (static)
> - LENS_FOCUS_DISTANCE (request, result)
> 
> Conceptually, the first two controls determine the range of possible
> values of the third. None of our lenses are calibrated, so hardcode the
> first to uncalibrated. The valid focus distances are then [0, min].
> Since we don't yet support variable-focus lenses, hardcode the second to
> 0.0f, making the range of valid focus distances [0, 0]. Hence, hardcode
> the focus distance in the result metadata to 0, and ignore the control
> in the request. Add todos for adding proper controls later, once we have
> focus support.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> This is also on top of my ctsw branch, so it won't apply on master, but
> I would like comments to confirm the direction and design.
> ---
>  src/android/camera_capabilities.cpp | 13 +++++++++++++
>  src/android/camera_device.cpp       |  4 ++++
>  2 files changed, 17 insertions(+)
> 
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 461c208b..a807712d 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -717,6 +717,7 @@ int CameraCapabilities::initializeStaticMetadata()
>  		ANDROID_LENS_INFO_AVAILABLE_APERTURES,
>  		ANDROID_LENS_INFO_AVAILABLE_FOCAL_LENGTHS,
>  		ANDROID_LENS_INFO_AVAILABLE_OPTICAL_STABILIZATION,
> +		ANDROID_LENS_INFO_FOCUS_DISTANCE_CALIBRATION,
>  		ANDROID_LENS_INFO_HYPERFOCAL_DISTANCE,
>  		ANDROID_LENS_INFO_MINIMUM_FOCUS_DISTANCE,
>  		ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> @@ -805,6 +806,7 @@ int CameraCapabilities::initializeStaticMetadata()
>  		ANDROID_JPEG_THUMBNAIL_SIZE,
>  		ANDROID_LENS_APERTURE,
>  		ANDROID_LENS_FOCAL_LENGTH,
> +		ANDROID_LENS_FOCUS_DISTANCE,
>  		ANDROID_LENS_OPTICAL_STABILIZATION_MODE,
>  		ANDROID_LENS_STATE,
>  		ANDROID_NOISE_REDUCTION_MODE,
> @@ -1247,10 +1249,21 @@ int CameraCapabilities::initializeStaticMetadata()
>  	staticMetadata_->addEntry(ANDROID_LENS_INFO_AVAILABLE_OPTICAL_STABILIZATION,
>  				  opticalStabilizations);
>  
> +	/*
> +	 * Must be populated for FULL, optional for LIMITED. The value doesn't
> +	 * matter, so just hardcode to uncalibrated. Uncalibrated means that
> +	 * the max focus distance is zero (= infinite focus) and the min focus
> +	 * distance is specified in LENS_INFO_MINIMUM_FOCUS_DISTANCE.
> +	 */
> +	int distanceCalibration = ANDROID_LENS_INFO_FOCUS_DISTANCE_CALIBRATION_UNCALIBRATED;
> +	staticMetadata_->addEntry(ANDROID_LENS_INFO_FOCUS_DISTANCE_CALIBRATION,
> +				  distanceCalibration);
> +
>  	float hypeFocalDistance = 0;
>  	staticMetadata_->addEntry(ANDROID_LENS_INFO_HYPERFOCAL_DISTANCE,
>  				  hypeFocalDistance);
>  
> +	/* \todo Get this from the HAL config. */

Once we'll have focus support in the libcamera core, I think this will
come from libcamera, not the HAL config. The minimum focus distance is
useful in general.

With this todo updated,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  	float minFocusDistance = 0;
>  	staticMetadata_->addEntry(ANDROID_LENS_INFO_MINIMUM_FOCUS_DISTANCE,
>  				  minFocusDistance);
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 373e0f98..cfbb84aa 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1396,6 +1396,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>  	float focal_length = 1.0;
>  	resultMetadata->addEntry(ANDROID_LENS_FOCAL_LENGTH, focal_length);
>  
> +	/* \todo Add a libcamera control for this, and plumb it. */
> +	float focalDistance = 0.0;
> +	resultMetadata->addEntry(ANDROID_LENS_FOCUS_DISTANCE, focalDistance);
> +
>  	value = ANDROID_LENS_STATE_STATIONARY;
>  	resultMetadata->addEntry(ANDROID_LENS_STATE, value);
>
Kieran Bingham Nov. 15, 2021, 2:11 p.m. UTC | #2 
Quoting Laurent Pinchart (2021-09-15 00:26:35)
> Hi Paul,
> 
> Thank you for the patch.
> 
> On Mon, Sep 13, 2021 at 04:31:25PM +0900, Paul Elder wrote:
> > Plumb the controls related to lens focus distance:
> > - LENS_INFO_FOCUS_DISTANCE_CALIBRATION (static)
> > - LENS_INFO_MINIMUM_FOCUS_DISTANCE (static)
> > - LENS_FOCUS_DISTANCE (request, result)
> > 
> > Conceptually, the first two controls determine the range of possible
> > values of the third. None of our lenses are calibrated, so hardcode the
> > first to uncalibrated. The valid focus distances are then [0, min].
> > Since we don't yet support variable-focus lenses, hardcode the second to
> > 0.0f, making the range of valid focus distances [0, 0]. Hence, hardcode
> > the focus distance in the result metadata to 0, and ignore the control
> > in the request. Add todos for adding proper controls later, once we have
> > focus support.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > ---
> > This is also on top of my ctsw branch, so it won't apply on master, but
> > I would like comments to confirm the direction and design.
> > ---
> >  src/android/camera_capabilities.cpp | 13 +++++++++++++
> >  src/android/camera_device.cpp       |  4 ++++
> >  2 files changed, 17 insertions(+)
> > 
> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > index 461c208b..a807712d 100644
> > --- a/src/android/camera_capabilities.cpp
> > +++ b/src/android/camera_capabilities.cpp
> > @@ -717,6 +717,7 @@ int CameraCapabilities::initializeStaticMetadata()
> >               ANDROID_LENS_INFO_AVAILABLE_APERTURES,
> >               ANDROID_LENS_INFO_AVAILABLE_FOCAL_LENGTHS,
> >               ANDROID_LENS_INFO_AVAILABLE_OPTICAL_STABILIZATION,
> > +             ANDROID_LENS_INFO_FOCUS_DISTANCE_CALIBRATION,
> >               ANDROID_LENS_INFO_HYPERFOCAL_DISTANCE,
> >               ANDROID_LENS_INFO_MINIMUM_FOCUS_DISTANCE,
> >               ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> > @@ -805,6 +806,7 @@ int CameraCapabilities::initializeStaticMetadata()
> >               ANDROID_JPEG_THUMBNAIL_SIZE,
> >               ANDROID_LENS_APERTURE,
> >               ANDROID_LENS_FOCAL_LENGTH,
> > +             ANDROID_LENS_FOCUS_DISTANCE,
> >               ANDROID_LENS_OPTICAL_STABILIZATION_MODE,
> >               ANDROID_LENS_STATE,
> >               ANDROID_NOISE_REDUCTION_MODE,
> > @@ -1247,10 +1249,21 @@ int CameraCapabilities::initializeStaticMetadata()
> >       staticMetadata_->addEntry(ANDROID_LENS_INFO_AVAILABLE_OPTICAL_STABILIZATION,
> >                                 opticalStabilizations);
> >  
> > +     /*
> > +      * Must be populated for FULL, optional for LIMITED. The value doesn't
> > +      * matter, so just hardcode to uncalibrated. Uncalibrated means that
> > +      * the max focus distance is zero (= infinite focus) and the min focus
> > +      * distance is specified in LENS_INFO_MINIMUM_FOCUS_DISTANCE.
> > +      */
> > +     int distanceCalibration = ANDROID_LENS_INFO_FOCUS_DISTANCE_CALIBRATION_UNCALIBRATED;
> > +     staticMetadata_->addEntry(ANDROID_LENS_INFO_FOCUS_DISTANCE_CALIBRATION,
> > +                               distanceCalibration);
> > +
> >       float hypeFocalDistance = 0;
> >       staticMetadata_->addEntry(ANDROID_LENS_INFO_HYPERFOCAL_DISTANCE,
> >                                 hypeFocalDistance);
> >  
> > +     /* \todo Get this from the HAL config. */
> 
> Once we'll have focus support in the libcamera core, I think this will
> come from libcamera, not the HAL config. The minimum focus distance is
> useful in general.
> 
> With this todo updated,
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> >       float minFocusDistance = 0;
> >       staticMetadata_->addEntry(ANDROID_LENS_INFO_MINIMUM_FOCUS_DISTANCE,
> >                                 minFocusDistance);
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 373e0f98..cfbb84aa 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -1396,6 +1396,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> >       float focal_length = 1.0;
> >       resultMetadata->addEntry(ANDROID_LENS_FOCAL_LENGTH, focal_length);
> >  
> > +     /* \todo Add a libcamera control for this, and plumb it. */

Given the discussions at : [0], Perhaps we should start by adding a
draft control directly?

[0] https://lists.libcamera.org/pipermail/libcamera-devel/2021-November/026991.html


> > +     float focalDistance = 0.0;
> > +     resultMetadata->addEntry(ANDROID_LENS_FOCUS_DISTANCE, focalDistance);

Will we mandate that metadata is provided in completed requests? (like
this one) or will we 'fill in a default' if it's not available.

If the completed request doesn't report a lens focus distance do we
simply not return a value to Android? or return the previous value
maybe?



> > +
> >       value = ANDROID_LENS_STATE_STATIONARY;
> >       resultMetadata->addEntry(ANDROID_LENS_STATE, value);
> >  
> 
> -- 
> Regards,
> 
> Laurent Pinchart
Umang Jain Nov. 23, 2021, 5:03 a.m. UTC | #3 
Hi Paul

On 9/13/21 1:01 PM, Paul Elder wrote:
> Plumb the controls related to lens focus distance:
> - LENS_INFO_FOCUS_DISTANCE_CALIBRATION (static)
> - LENS_INFO_MINIMUM_FOCUS_DISTANCE (static)
> - LENS_FOCUS_DISTANCE (request, result)
>
> Conceptually, the first two controls determine the range of possible
> values of the third. None of our lenses are calibrated, so hardcode the
> first to uncalibrated. The valid focus distances are then [0, min].
> Since we don't yet support variable-focus lenses, hardcode the second to
> 0.0f, making the range of valid focus distances [0, 0]. Hence, hardcode
> the focus distance in the result metadata to 0, and ignore the control
> in the request. Add todos for adding proper controls later, once we have
> focus support.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
> This is also on top of my ctsw branch, so it won't apply on master, but
> I would like comments to confirm the direction and design.
> ---
>   src/android/camera_capabilities.cpp | 13 +++++++++++++
>   src/android/camera_device.cpp       |  4 ++++
>   2 files changed, 17 insertions(+)
>
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 461c208b..a807712d 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -717,6 +717,7 @@ int CameraCapabilities::initializeStaticMetadata()
>   		ANDROID_LENS_INFO_AVAILABLE_APERTURES,
>   		ANDROID_LENS_INFO_AVAILABLE_FOCAL_LENGTHS,
>   		ANDROID_LENS_INFO_AVAILABLE_OPTICAL_STABILIZATION,
> +		ANDROID_LENS_INFO_FOCUS_DISTANCE_CALIBRATION,
>   		ANDROID_LENS_INFO_HYPERFOCAL_DISTANCE,
>   		ANDROID_LENS_INFO_MINIMUM_FOCUS_DISTANCE,
>   		ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> @@ -805,6 +806,7 @@ int CameraCapabilities::initializeStaticMetadata()
>   		ANDROID_JPEG_THUMBNAIL_SIZE,
>   		ANDROID_LENS_APERTURE,
>   		ANDROID_LENS_FOCAL_LENGTH,
> +		ANDROID_LENS_FOCUS_DISTANCE,
>   		ANDROID_LENS_OPTICAL_STABILIZATION_MODE,
>   		ANDROID_LENS_STATE,
>   		ANDROID_NOISE_REDUCTION_MODE,
> @@ -1247,10 +1249,21 @@ int CameraCapabilities::initializeStaticMetadata()
>   	staticMetadata_->addEntry(ANDROID_LENS_INFO_AVAILABLE_OPTICAL_STABILIZATION,
>   				  opticalStabilizations);
>   
> +	/*
> +	 * Must be populated for FULL, optional for LIMITED. The value doesn't
> +	 * matter, so just hardcode to uncalibrated. Uncalibrated means that
> +	 * the max focus distance is zero (= infinite focus) and the min focus
> +	 * distance is specified in LENS_INFO_MINIMUM_FOCUS_DISTANCE.
> +	 */
> +	int distanceCalibration = ANDROID_LENS_INFO_FOCUS_DISTANCE_CALIBRATION_UNCALIBRATED;
> +	staticMetadata_->addEntry(ANDROID_LENS_INFO_FOCUS_DISTANCE_CALIBRATION,
> +				  distanceCalibration);
> +
>   	float hypeFocalDistance = 0;
>   	staticMetadata_->addEntry(ANDROID_LENS_INFO_HYPERFOCAL_DISTANCE,
>   				  hypeFocalDistance);
>   
> +	/* \todo Get this from the HAL config. */
>   	float minFocusDistance = 0;
>   	staticMetadata_->addEntry(ANDROID_LENS_INFO_MINIMUM_FOCUS_DISTANCE,
>   				  minFocusDistance);
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 373e0f98..cfbb84aa 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -1396,6 +1396,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>   	float focal_length = 1.0;
>   	resultMetadata->addEntry(ANDROID_LENS_FOCAL_LENGTH, focal_length);
>   
> +	/* \todo Add a libcamera control for this, and plumb it. */


If I remember the discussion from last week correctly, we agreed to 
introduce controls to report this metadata right? The setting of the 
control will then be set in the IPA and read here ?

If that's something you need help with (or if the plans have changed), 
please let me know. I came here since I was looking at 
https://bugs.libcamera.org/show_bug.cgi?id=52

> +	float focalDistance = 0.0;
> +	resultMetadata->addEntry(ANDROID_LENS_FOCUS_DISTANCE, focalDistance);
> +
>   	value = ANDROID_LENS_STATE_STATIONARY;
>   	resultMetadata->addEntry(ANDROID_LENS_STATE, value);
>

Patch
diff mbox series 

diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
index 461c208b..a807712d 100644
--- a/src/android/camera_capabilities.cpp
+++ b/src/android/camera_capabilities.cpp
@@ -717,6 +717,7 @@  int CameraCapabilities::initializeStaticMetadata()
 		ANDROID_LENS_INFO_AVAILABLE_APERTURES,
 		ANDROID_LENS_INFO_AVAILABLE_FOCAL_LENGTHS,
 		ANDROID_LENS_INFO_AVAILABLE_OPTICAL_STABILIZATION,
+		ANDROID_LENS_INFO_FOCUS_DISTANCE_CALIBRATION,
 		ANDROID_LENS_INFO_HYPERFOCAL_DISTANCE,
 		ANDROID_LENS_INFO_MINIMUM_FOCUS_DISTANCE,
 		ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
@@ -805,6 +806,7 @@  int CameraCapabilities::initializeStaticMetadata()
 		ANDROID_JPEG_THUMBNAIL_SIZE,
 		ANDROID_LENS_APERTURE,
 		ANDROID_LENS_FOCAL_LENGTH,
+		ANDROID_LENS_FOCUS_DISTANCE,
 		ANDROID_LENS_OPTICAL_STABILIZATION_MODE,
 		ANDROID_LENS_STATE,
 		ANDROID_NOISE_REDUCTION_MODE,
@@ -1247,10 +1249,21 @@  int CameraCapabilities::initializeStaticMetadata()
 	staticMetadata_->addEntry(ANDROID_LENS_INFO_AVAILABLE_OPTICAL_STABILIZATION,
 				  opticalStabilizations);
 
+	/*
+	 * Must be populated for FULL, optional for LIMITED. The value doesn't
+	 * matter, so just hardcode to uncalibrated. Uncalibrated means that
+	 * the max focus distance is zero (= infinite focus) and the min focus
+	 * distance is specified in LENS_INFO_MINIMUM_FOCUS_DISTANCE.
+	 */
+	int distanceCalibration = ANDROID_LENS_INFO_FOCUS_DISTANCE_CALIBRATION_UNCALIBRATED;
+	staticMetadata_->addEntry(ANDROID_LENS_INFO_FOCUS_DISTANCE_CALIBRATION,
+				  distanceCalibration);
+
 	float hypeFocalDistance = 0;
 	staticMetadata_->addEntry(ANDROID_LENS_INFO_HYPERFOCAL_DISTANCE,
 				  hypeFocalDistance);
 
+	/* \todo Get this from the HAL config. */
 	float minFocusDistance = 0;
 	staticMetadata_->addEntry(ANDROID_LENS_INFO_MINIMUM_FOCUS_DISTANCE,
 				  minFocusDistance);
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 373e0f98..cfbb84aa 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -1396,6 +1396,10 @@  CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
 	float focal_length = 1.0;
 	resultMetadata->addEntry(ANDROID_LENS_FOCAL_LENGTH, focal_length);
 
+	/* \todo Add a libcamera control for this, and plumb it. */
+	float focalDistance = 0.0;
+	resultMetadata->addEntry(ANDROID_LENS_FOCUS_DISTANCE, focalDistance);
+
 	value = ANDROID_LENS_STATE_STATIONARY;
 	resultMetadata->addEntry(ANDROID_LENS_STATE, value);