[libcamera-devel,RFC,v2,10/12] android: camera_device: Translate android controls to libcamera controls

12077 diff mbox series

Translate the newly added android controls that are required to FULL
compliance into libcamera controls before sending them to the Camera.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
---
 src/android/camera_device.cpp | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

Hi Paul,

Thank you for the patch.

On Thu, Apr 22, 2021 at 06:41:00PM +0900, Paul Elder wrote:
> Translate the newly added android controls that are required to FULL
> compliance into libcamera controls before sending them to the Camera.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
> 
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 99c67555..30692a67 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -2136,15 +2136,46 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>  
>  	/* Translate the Android request settings to libcamera controls. */
>  	camera_metadata_ro_entry_t entry;
> +	ControlList &controls = descriptor->request_->controls();
> +
>  	if (settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry)) {
>  		const int32_t *data = entry.data.i32;
>  		Rectangle cropRegion{ data[0], data[1],
>  				      static_cast<unsigned int>(data[2]),
>  				      static_cast<unsigned int>(data[3]) };
> -		ControlList &controls = descriptor->request_->controls();
>  		controls.set(controls::ScalerCrop, cropRegion);
>  	}
>  
> +	if (settings.getEntry(ANDROID_SENSOR_EXPOSURE_TIME, &entry))
> +		controls.set(controls::ExposureTime, *entry.data.i64 / 1000ULL);
> +
> +	if (settings.getEntry(ANDROID_BLACK_LEVEL_LOCK, &entry))
> +		controls.set(controls::draft::BlackLevelLocked, *entry.data.u8);
> +
> +	if (settings.getEntry(ANDROID_CONTROL_AE_LOCK, &entry))
> +		controls.set(controls::AeLocked, *entry.data.u8);
> +
> +	if (settings.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry))
> +		controls.set(controls::draft::AePrecaptureTrigger, *entry.data.u8);
> +
> +	if (settings.getEntry(ANDROID_CONTROL_AWB_MODE, &entry))
> +		controls.set(controls::AwbMode, *entry.data.u8);
> +
> +	if (settings.getEntry(ANDROID_CONTROL_AWB_LOCK, &entry))
> +		controls.set(controls::AwbLocked, *entry.data.u8);

AwbLocked is currently defined as metadata reported by pipeline
handlers. Interestingly enough, it's not used currently. If you want to
use it with the Android meaning, it should be made a draft control and
its documentation should be updated.

AeLocked, on the other hand, is used, so you will need to create a new
draft control for the Android meaning.

> +
> +	if (settings.getEntry(ANDROID_EDGE_MODE, &entry))
> +		controls.set(controls::draft::EdgeMode, *entry.data.u8);
> +
> +	if (settings.getEntry(ANDROID_NOISE_REDUCTION_MODE, &entry))
> +		controls.set(controls::draft::NoiseReductionMode, *entry.data.u8);
> +
> +	if (settings.getEntry(ANDROID_SENSOR_SENSITIVITY, &entry))
> +		controls.set(controls::draft::SensorSensitivity, *entry.data.i32);
> +
> +	if (settings.getEntry(ANDROID_TONEMAP_MODE, &entry))
> +		controls.set(controls::draft::TonemapMode, *entry.data.u8);
> +
>  	return 0;
>  }
>

Hi Paul,

On Thu, Apr 22, 2021 at 06:41:00PM +0900, Paul Elder wrote:
> Translate the newly added android controls that are required to FULL
> compliance into libcamera controls before sending them to the Camera.
>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> ---
>  src/android/camera_device.cpp | 33 ++++++++++++++++++++++++++++++++-
>  1 file changed, 32 insertions(+), 1 deletion(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 99c67555..30692a67 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -2136,15 +2136,46 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>
>  	/* Translate the Android request settings to libcamera controls. */
>  	camera_metadata_ro_entry_t entry;
> +	ControlList &controls = descriptor->request_->controls();
> +
>  	if (settings.getEntry(ANDROID_SCALER_CROP_REGION, &entry)) {
>  		const int32_t *data = entry.data.i32;
>  		Rectangle cropRegion{ data[0], data[1],
>  				      static_cast<unsigned int>(data[2]),
>  				      static_cast<unsigned int>(data[3]) };
> -		ControlList &controls = descriptor->request_->controls();
>  		controls.set(controls::ScalerCrop, cropRegion);
>  	}
>
> +	if (settings.getEntry(ANDROID_SENSOR_EXPOSURE_TIME, &entry))
> +		controls.set(controls::ExposureTime, *entry.data.i64 / 1000ULL);

Ack. The android control is in nsecs, our one is micro-secs

> +
> +	if (settings.getEntry(ANDROID_BLACK_LEVEL_LOCK, &entry))
> +		controls.set(controls::draft::BlackLevelLocked, *entry.data.u8);
> +
> +	if (settings.getEntry(ANDROID_CONTROL_AE_LOCK, &entry))
> +		controls.set(controls::AeLocked, *entry.data.u8);
> +
> +	if (settings.getEntry(ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, &entry))
> +		controls.set(controls::draft::AePrecaptureTrigger, *entry.data.u8);
> +
> +	if (settings.getEntry(ANDROID_CONTROL_AWB_MODE, &entry))
> +		controls.set(controls::AwbMode, *entry.data.u8);
> +
> +	if (settings.getEntry(ANDROID_CONTROL_AWB_LOCK, &entry))
> +		controls.set(controls::AwbLocked, *entry.data.u8);
> +
> +	if (settings.getEntry(ANDROID_EDGE_MODE, &entry))
> +		controls.set(controls::draft::EdgeMode, *entry.data.u8);
> +
> +	if (settings.getEntry(ANDROID_NOISE_REDUCTION_MODE, &entry))
> +		controls.set(controls::draft::NoiseReductionMode, *entry.data.u8);
> +
> +	if (settings.getEntry(ANDROID_SENSOR_SENSITIVITY, &entry))
> +		controls.set(controls::draft::SensorSensitivity, *entry.data.i32);
> +
> +	if (settings.getEntry(ANDROID_TONEMAP_MODE, &entry))
> +		controls.set(controls::draft::TonemapMode, *entry.data.u8);

Seems good.
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>

I wonder going forward, if we should not walk all the controls in the
request, translate them one by one, and clearly print out the ones we
don't pass down to the ph. Is it worth a todo ?

> +
>  	return 0;
>  }
>
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel