[libcamera-devel,v2] android: Plumb Sharpness control into EDGE_MODE
diff mbox series

Message ID 20211221044133.2531278-1-paul.elder@ideasonboard.com
State New
Delegated to: Paul Elder
Headers show
Series
  • [libcamera-devel,v2] android: Plumb Sharpness control into EDGE_MODE
Related show

Commit Message

Paul Elder Dec. 21, 2021, 4:41 a.m. UTC 
Plumb the Sharpness control into the HAL for EDGE_MODE and other related
android controls.

libcamera doesn't distinguish between fast and HQ, but rather with a
floating value for how much sharpening to apply. This is thus
unsufficient information for retaining whether the request was for fast
or HQ, so save it in the request, and report what was requested.

Bug: https://bugs.libcamera.org/show_bug.cgi?id=46
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
This patch depends on "android: camera_metadata: Add appendEntry helper"

Changes in v2:
- fix the assertions in template creation
- report the edge mode that was requested, instead of doing conversion
  to and from the floating value and its ControlInfo
- move edge mode capability check out of manual control (as docs say
  it's not required) and into the top-level FULL hardware level
  validator
---
 src/android/camera_capabilities.cpp | 44 +++++++++++++++++++++++++++++
 src/android/camera_device.cpp       | 28 ++++++++++++++++++
 src/android/camera_request.h        |  4 +++
 3 files changed, 76 insertions(+)

Comments

Kieran Bingham Jan. 3, 2022, 9:24 p.m. UTC | #1 
Quoting Paul Elder (2021-12-21 04:41:33)
> Plumb the Sharpness control into the HAL for EDGE_MODE and other related
> android controls.
> 
> libcamera doesn't distinguish between fast and HQ, but rather with a
> floating value for how much sharpening to apply. This is thus
> unsufficient information for retaining whether the request was for fast

s/unsufficient/insufficient/

> or HQ, so save it in the request, and report what was requested.
> 
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=46
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> This patch depends on "android: camera_metadata: Add appendEntry helper"
> 
> Changes in v2:
> - fix the assertions in template creation
> - report the edge mode that was requested, instead of doing conversion
>   to and from the floating value and its ControlInfo
> - move edge mode capability check out of manual control (as docs say
>   it's not required) and into the top-level FULL hardware level
>   validator
> ---
>  src/android/camera_capabilities.cpp | 44 +++++++++++++++++++++++++++++
>  src/android/camera_device.cpp       | 28 ++++++++++++++++++
>  src/android/camera_request.h        |  4 +++
>  3 files changed, 76 insertions(+)
> 
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index b9a1f6e5..727204b9 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -401,6 +401,11 @@ void CameraCapabilities::computeHwLevel(
>         if (!caps.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE))
>                 hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
>  
> +       if (!staticMetadata_->hasEntry(ANDROID_EDGE_AVAILABLE_EDGE_MODES)) {
> +               LOG(HAL, Info) << noFull << "missing edge modes";
> +               hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> +       }
> +
>         found = staticMetadata_->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);
>         if (!found || *entry.data.i32 != 0) {
>                 LOG(HAL, Info) << noFull << "missing or invalid max sync latency";
> @@ -1078,6 +1083,21 @@ int CameraCapabilities::initializeStaticMetadata()
>         staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,
>                                   availableControlModes);
>  
> +       const auto &edgeInfo = controlsInfo.find(&controls::Sharpness);
> +       if (edgeInfo != controlsInfo.end()) {
> +               std::vector<uint8_t> availableEdgeModes = {
> +                       ANDROID_EDGE_MODE_OFF,
> +                       ANDROID_EDGE_MODE_FAST,
> +                       ANDROID_EDGE_MODE_HIGH_QUALITY,
> +               };
> +
> +               staticMetadata_->addEntry(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
> +                                         availableEdgeModes);
> +               availableCharacteristicsKeys_.insert(ANDROID_EDGE_AVAILABLE_EDGE_MODES);
> +               availableRequestKeys_.insert(ANDROID_EDGE_MODE);
> +               availableResultKeys_.insert(ANDROID_EDGE_MODE);
> +       }
> +
>         /* JPEG static metadata. */
>  
>         /*
> @@ -1593,6 +1613,12 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateManual() cons
>                 manualTemplate->appendEntry(ANDROID_CONTROL_AE_MODE, aeMode);
>         }
>  
> +       if (staticMetadata_->entryContains<uint8_t>(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
> +                                                   ANDROID_EDGE_MODE_OFF)) {
> +               uint8_t edgeMode = ANDROID_EDGE_MODE_OFF;
> +               manualTemplate->appendEntry(ANDROID_EDGE_MODE, edgeMode);
> +       }
> +
>         return manualTemplate;
>  }
>  
> @@ -1675,6 +1701,12 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplatePreview() con
>         if (availableRequestKeys_.count(ANDROID_SENSOR_SENSITIVITY))
>                 requestTemplate->addEntry(ANDROID_SENSOR_SENSITIVITY, minISO_);
>  
> +       if (staticMetadata_->entryContains<uint8_t>(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
> +                                                   ANDROID_EDGE_MODE_FAST)) {
> +               uint8_t edgeMode = ANDROID_EDGE_MODE_FAST;
> +               requestTemplate->addEntry(ANDROID_EDGE_MODE, edgeMode);
> +       }
> +
>         uint8_t flashMode = ANDROID_FLASH_MODE_OFF;
>         requestTemplate->addEntry(ANDROID_FLASH_MODE, flashMode);
>  
> @@ -1713,6 +1745,12 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateStill() const
>         if (!stillTemplate)
>                 return nullptr;
>  
> +       if (staticMetadata_->entryContains<uint8_t>(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
> +                                                   ANDROID_EDGE_MODE_HIGH_QUALITY)) {
> +               uint8_t edgeMode = ANDROID_EDGE_MODE_HIGH_QUALITY;
> +               stillTemplate->appendEntry(ANDROID_EDGE_MODE, edgeMode);
> +       }
> +
>         return stillTemplate;
>  }
>  
> @@ -1730,6 +1768,12 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateVideo() const
>         staticMetadata_->getEntry(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES,
>                                   &entry);
>  
> +       if (staticMetadata_->entryContains<uint8_t>(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
> +                                                   ANDROID_EDGE_MODE_FAST)) {
> +               uint8_t edgeMode = ANDROID_EDGE_MODE_FAST;
> +               previewTemplate->appendEntry(ANDROID_EDGE_MODE, edgeMode);
> +       }
> +
>         /*
>          * Assume the AE_AVAILABLE_TARGET_FPS_RANGE static metadata
>          * has been assembled as {{min, max} {max, max}}.
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 1a508923..0d668ea6 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -920,6 +920,26 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>                         controls.set(controls::DigitalGain, lastDigitalGain_);
>         }
>  
> +       if (settings.getEntry(ANDROID_EDGE_MODE, &entry)) {
> +               const auto &info = camera_->controls().find(&controls::Sharpness);
> +               if (info != camera_->controls().end()) {

As the ANDROID_EDGE_MODE is only enabled if the camera has a Sharpness
control, this should always be found right?

Still, it doesn't hurt to make sure I think.

> +                       float min = info->second.min().get<float>();
> +                       float def = info->second.def().get<float>();
> +                       float max = info->second.max().get<float>();
> +                       /*
> +                        * The default value might be unusable since control
> +                        * serialization ignores it. Alternatively the default

Why? Should we fix the serialisation to pass the default values? or is
this troublesome?

> +                        * could be simply set to zero or the minimum value.
> +                        * Use the maximum sharpness value in these cases.
> +                        */
> +                       float val = (def == 0.0f || def == min) ? max : def;
> +                       controls.set(controls::Sharpness,
> +                                    *entry.data.u8 == ANDROID_EDGE_MODE_OFF ? min : val);
> +
> +                       descriptor->edgeMode_ = *entry.data.u8;


So, if I infer correctly this should set:
  EDGE_MODE_OFF  : min
  EDGE_MODE_FAST : def, unless def == min or def == 0, when max
  EDGE_MODE_HIGH_QUALITY : same as ^

Should HIGH_QUALITY always set max?

	descriptor->edgeMode_ = *entry.data.u8;
	switch (descriptor->edgeMode_) {
		default:
		case ANDROID_EDGE_MODE_OFF:
			val = min;
			break;
		case ANDROID_EDGE_MODE_FAST:
			/* ... comment about def, or fix serialisations? */ 
			val = (def == 0.0f || def == min) ? max : def;
			break;
		case ANDROID_EDGE_MODE_HIGH_QUALITY:
			val = max;
			break;
	}


Hrm, not quite as concise though, so maybe your way is fine if we don't
intend to distinguish between the fast/HQ anyway.


If there's a reason we can't or shouldn't be serialising the default
values to be able to reference them correctly then:

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>


> +               }
> +       }
> +
>         return 0;
>  }
>  
> @@ -1602,6 +1622,14 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>                                          duration);
>         }
>  
> +       if (metadata.contains(controls::Sharpness)) {
> +               /*
> +                * libcamera doesn't distinguish between fast vs HQ sharpening
> +                * modes. Report the mode that was requested.
> +                */
> +               resultMetadata->addEntry(ANDROID_EDGE_MODE, descriptor.edgeMode_);
> +       }
> +
>         if (metadata.contains(controls::ScalerCrop)) {
>                 Rectangle crop = metadata.get(controls::ScalerCrop);
>                 int32_t cropRect[] = {
> diff --git a/src/android/camera_request.h b/src/android/camera_request.h
> index b2809179..69b6c8fc 100644
> --- a/src/android/camera_request.h
> +++ b/src/android/camera_request.h
> @@ -87,6 +87,10 @@ public:
>         /* The libcamera internal AE state for this request */
>         AutoMode aeMode_ = AutoMode::Auto;
>  
> +       /* The android edge mode associated with this request */
> +       /* \todo Wrap all such controls? */
> +       int32_t edgeMode_;
> +
>  private:
>         LIBCAMERA_DISABLE_COPY(Camera3RequestDescriptor)
>  };
> -- 
> 2.27.0
>

Patch
diff mbox series 

diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
index b9a1f6e5..727204b9 100644
--- a/src/android/camera_capabilities.cpp
+++ b/src/android/camera_capabilities.cpp
@@ -401,6 +401,11 @@  void CameraCapabilities::computeHwLevel(
 	if (!caps.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE))
 		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
 
+	if (!staticMetadata_->hasEntry(ANDROID_EDGE_AVAILABLE_EDGE_MODES)) {
+		LOG(HAL, Info) << noFull << "missing edge modes";
+		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
+	}
+
 	found = staticMetadata_->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);
 	if (!found || *entry.data.i32 != 0) {
 		LOG(HAL, Info) << noFull << "missing or invalid max sync latency";
@@ -1078,6 +1083,21 @@  int CameraCapabilities::initializeStaticMetadata()
 	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,
 				  availableControlModes);
 
+	const auto &edgeInfo = controlsInfo.find(&controls::Sharpness);
+	if (edgeInfo != controlsInfo.end()) {
+		std::vector<uint8_t> availableEdgeModes = {
+			ANDROID_EDGE_MODE_OFF,
+			ANDROID_EDGE_MODE_FAST,
+			ANDROID_EDGE_MODE_HIGH_QUALITY,
+		};
+
+		staticMetadata_->addEntry(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
+					  availableEdgeModes);
+		availableCharacteristicsKeys_.insert(ANDROID_EDGE_AVAILABLE_EDGE_MODES);
+		availableRequestKeys_.insert(ANDROID_EDGE_MODE);
+		availableResultKeys_.insert(ANDROID_EDGE_MODE);
+	}
+
 	/* JPEG static metadata. */
 
 	/*
@@ -1593,6 +1613,12 @@  std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateManual() cons
 		manualTemplate->appendEntry(ANDROID_CONTROL_AE_MODE, aeMode);
 	}
 
+	if (staticMetadata_->entryContains<uint8_t>(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
+						    ANDROID_EDGE_MODE_OFF)) {
+		uint8_t edgeMode = ANDROID_EDGE_MODE_OFF;
+		manualTemplate->appendEntry(ANDROID_EDGE_MODE, edgeMode);
+	}
+
 	return manualTemplate;
 }
 
@@ -1675,6 +1701,12 @@  std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplatePreview() con
 	if (availableRequestKeys_.count(ANDROID_SENSOR_SENSITIVITY))
 		requestTemplate->addEntry(ANDROID_SENSOR_SENSITIVITY, minISO_);
 
+	if (staticMetadata_->entryContains<uint8_t>(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
+						    ANDROID_EDGE_MODE_FAST)) {
+		uint8_t edgeMode = ANDROID_EDGE_MODE_FAST;
+		requestTemplate->addEntry(ANDROID_EDGE_MODE, edgeMode);
+	}
+
 	uint8_t flashMode = ANDROID_FLASH_MODE_OFF;
 	requestTemplate->addEntry(ANDROID_FLASH_MODE, flashMode);
 
@@ -1713,6 +1745,12 @@  std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateStill() const
 	if (!stillTemplate)
 		return nullptr;
 
+	if (staticMetadata_->entryContains<uint8_t>(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
+						    ANDROID_EDGE_MODE_HIGH_QUALITY)) {
+		uint8_t edgeMode = ANDROID_EDGE_MODE_HIGH_QUALITY;
+		stillTemplate->appendEntry(ANDROID_EDGE_MODE, edgeMode);
+	}
+
 	return stillTemplate;
 }
 
@@ -1730,6 +1768,12 @@  std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateVideo() const
 	staticMetadata_->getEntry(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES,
 				  &entry);
 
+	if (staticMetadata_->entryContains<uint8_t>(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
+						    ANDROID_EDGE_MODE_FAST)) {
+		uint8_t edgeMode = ANDROID_EDGE_MODE_FAST;
+		previewTemplate->appendEntry(ANDROID_EDGE_MODE, edgeMode);
+	}
+
 	/*
 	 * Assume the AE_AVAILABLE_TARGET_FPS_RANGE static metadata
 	 * has been assembled as {{min, max} {max, max}}.
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 1a508923..0d668ea6 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -920,6 +920,26 @@  int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
 			controls.set(controls::DigitalGain, lastDigitalGain_);
 	}
 
+	if (settings.getEntry(ANDROID_EDGE_MODE, &entry)) {
+		const auto &info = camera_->controls().find(&controls::Sharpness);
+		if (info != camera_->controls().end()) {
+			float min = info->second.min().get<float>();
+			float def = info->second.def().get<float>();
+			float max = info->second.max().get<float>();
+			/*
+			 * The default value might be unusable since control
+			 * serialization ignores it. Alternatively the default
+			 * could be simply set to zero or the minimum value.
+			 * Use the maximum sharpness value in these cases.
+			 */
+			float val = (def == 0.0f || def == min) ? max : def;
+			controls.set(controls::Sharpness,
+				     *entry.data.u8 == ANDROID_EDGE_MODE_OFF ? min : val);
+
+			descriptor->edgeMode_ = *entry.data.u8;
+		}
+	}
+
 	return 0;
 }
 
@@ -1602,6 +1622,14 @@  CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
 					 duration);
 	}
 
+	if (metadata.contains(controls::Sharpness)) {
+		/*
+		 * libcamera doesn't distinguish between fast vs HQ sharpening
+		 * modes. Report the mode that was requested.
+		 */
+		resultMetadata->addEntry(ANDROID_EDGE_MODE, descriptor.edgeMode_);
+	}
+
 	if (metadata.contains(controls::ScalerCrop)) {
 		Rectangle crop = metadata.get(controls::ScalerCrop);
 		int32_t cropRect[] = {
diff --git a/src/android/camera_request.h b/src/android/camera_request.h
index b2809179..69b6c8fc 100644
--- a/src/android/camera_request.h
+++ b/src/android/camera_request.h
@@ -87,6 +87,10 @@  public:
 	/* The libcamera internal AE state for this request */
 	AutoMode aeMode_ = AutoMode::Auto;
 
+	/* The android edge mode associated with this request */
+	/* \todo Wrap all such controls? */
+	int32_t edgeMode_;
+
 private:
 	LIBCAMERA_DISABLE_COPY(Camera3RequestDescriptor)
 };