[libcamera-devel,v2,3/3] android: Plumb noise reduction
diff mbox series

Message ID 20211221051023.2628625-4-paul.elder@ideasonboard.com
State New
Delegated to: Paul Elder
Headers show
Series
  • Noise reduction
Related show

Commit Message

Paul Elder Dec. 21, 2021, 5:10 a.m. UTC 
Add:
- hardware level detection based on available noise reduction values
  (no specific capabilities seem to require it; only FULL in general)
- Conversion from android to libcamera noise reduction modes for request
  (use switch-case instead of direct copy just in case)
- Conversion from libcamera to android noise reduction modes for result
- noise reduction values to the templates

We already have the mechanism to report the available noise reduction
modes, so that is not added.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
Changes in v2:
- change result metadata generation to switch-case
---
 src/android/camera_capabilities.cpp | 34 +++++++++++++-
 src/android/camera_device.cpp       | 70 +++++++++++++++++++++++++++--
 2 files changed, 100 insertions(+), 4 deletions(-)

Comments

Kieran Bingham Jan. 3, 2022, 10:30 p.m. UTC | #1 
Quoting Paul Elder (2021-12-21 05:10:23)
> Add:
> - hardware level detection based on available noise reduction values
>   (no specific capabilities seem to require it; only FULL in general)
> - Conversion from android to libcamera noise reduction modes for request
>   (use switch-case instead of direct copy just in case)
> - Conversion from libcamera to android noise reduction modes for result
> - noise reduction values to the templates
> 
> We already have the mechanism to report the available noise reduction
> modes, so that is not added.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> Changes in v2:
> - change result metadata generation to switch-case
> ---
>  src/android/camera_capabilities.cpp | 34 +++++++++++++-
>  src/android/camera_device.cpp       | 70 +++++++++++++++++++++++++++--
>  2 files changed, 100 insertions(+), 4 deletions(-)
> 
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index d6a1589e..365937ed 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -412,6 +412,31 @@ void CameraCapabilities::computeHwLevel(
>                 hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
>         }
>  
> +       found = staticMetadata_->entryContains<uint8_t>(
> +               ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> +               ANDROID_NOISE_REDUCTION_MODE_OFF);
> +       if (!found) {
> +               LOG(HAL, Info) << noFull << "missing noise reduction mode off";
> +               hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> +       }
> +
> +       found = staticMetadata_->entryContains<uint8_t>(
> +               ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> +               ANDROID_NOISE_REDUCTION_MODE_FAST);
> +       if (!found) {
> +               LOG(HAL, Info) << noFull << "missing noise reduction mode fast";
> +               hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> +       }
> +
> +       /* Android docs don't say this is required, but CTS does. */
> +       found = staticMetadata_->entryContains<uint8_t>(
> +               ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> +               ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY);
> +       if (!found) {
> +               LOG(HAL, Info) << noFull << "missing noise reduction mode high quality";
> +               hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> +       }
> +
>         hwLevel_ = hwLevel;
>  }
>  
> @@ -1727,7 +1752,7 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplatePreview() con
>         requestTemplate->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE,
>                                   faceDetectMode);
>  
> -       uint8_t noiseReduction = ANDROID_NOISE_REDUCTION_MODE_OFF;
> +       uint8_t noiseReduction = ANDROID_NOISE_REDUCTION_MODE_FAST;
>         requestTemplate->addEntry(ANDROID_NOISE_REDUCTION_MODE,
>                                   noiseReduction);
>  
> @@ -1764,6 +1789,13 @@ std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateStill() const
>                 stillTemplate->appendEntry(ANDROID_EDGE_MODE, edgeMode);
>         }
>  
> +       if (staticMetadata_->entryContains<uint8_t>(
> +                       ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
> +                       ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY)) {
> +               uint8_t noiseReduction = ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY;
> +               stillTemplate->appendEntry(ANDROID_NOISE_REDUCTION_MODE, noiseReduction);
> +       }
> +
>         return stillTemplate;
>  }
>  
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 8861447d..e521ae0a 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -920,6 +920,39 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
>                         controls.set(controls::DigitalGain, lastDigitalGain_);
>         }
>  
> +       if (settings.getEntry(ANDROID_NOISE_REDUCTION_MODE, &entry)) {
> +               const uint8_t data = *entry.data.u8;
> +               int32_t noiseReductionMode;
> +               switch (data) {
> +               case ANDROID_NOISE_REDUCTION_MODE_OFF:
> +                       noiseReductionMode = controls::NoiseReductionModeOff;
> +                       break;
> +
> +               case ANDROID_NOISE_REDUCTION_MODE_FAST:
> +                       noiseReductionMode = controls::NoiseReductionModeFast;
> +                       break;
> +
> +               case ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY:
> +                       noiseReductionMode = controls::NoiseReductionModeHighQuality;
> +                       break;
> +
> +               case ANDROID_NOISE_REDUCTION_MODE_MINIMAL:
> +                       noiseReductionMode = controls::NoiseReductionModeRaw;
> +                       break;
> +
> +               case ANDROID_NOISE_REDUCTION_MODE_ZERO_SHUTTER_LAG:
> +                       noiseReductionMode = controls::NoiseReductionModeZSL;
> +                       break;
> +
> +               default:
> +                       LOG(HAL, Error)
> +                               << "Unknown noise reduction mode: " << data;
> +                       return -EINVAL;

Is it defined/documented that one control failing should stop processing
all other controls? or should it try to handle the other controls as
best as it could too?

I guess a failure is a failure all the same.

> +               }
> +
> +               controls.set(controls::NoiseReductionMode, noiseReductionMode);
> +       }
> +

I wonder if those one-to-one mappings would be cleaner in a map ... but
I don't think that's essential.

>         if (settings.getEntry(ANDROID_EDGE_MODE, &entry)) {
>                 const auto &info = camera_->controls().find(&controls::Sharpness);
>                 if (info != camera_->controls().end()) {
> @@ -1587,9 +1620,6 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>         value = ANDROID_STATISTICS_SCENE_FLICKER_NONE;
>         resultMetadata->addEntry(ANDROID_STATISTICS_SCENE_FLICKER, value);
>  
> -       value = ANDROID_NOISE_REDUCTION_MODE_OFF;
> -       resultMetadata->addEntry(ANDROID_NOISE_REDUCTION_MODE, value);
> -
>         /* 33.3 msec */
>         const int64_t rolling_shutter_skew = 33300000;
>         resultMetadata->addEntry(ANDROID_SENSOR_ROLLING_SHUTTER_SKEW,
> @@ -1643,6 +1673,40 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>                 resultMetadata->addEntry(ANDROID_SCALER_CROP_REGION, cropRect);
>         }
>  
> +       if (metadata.contains(controls::NoiseReductionMode)) {
> +               bool valid;
> +               switch (metadata.get(controls::NoiseReductionMode)) {
> +               case controls::NoiseReductionModeOff:
> +                       value = ANDROID_NOISE_REDUCTION_MODE_OFF;
> +                       valid = true;
> +                       break;
> +               case controls::NoiseReductionModeFast:
> +                       value = ANDROID_NOISE_REDUCTION_MODE_FAST;
> +                       valid = true;
> +                       break;
> +               case controls::NoiseReductionModeHighQuality:
> +                       value = ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY;
> +                       valid = true;
> +                       break;
> +               case controls::NoiseReductionModeRaw:
> +                       value = ANDROID_NOISE_REDUCTION_MODE_MINIMAL;
> +                       valid = true;
> +                       break;
> +               case controls::NoiseReductionModeZSL:
> +                       value = ANDROID_NOISE_REDUCTION_MODE_ZERO_SHUTTER_LAG;
> +                       valid = true;
> +                       break;
> +               default:
> +                       LOG(HAL, Error)
> +                               << "Unknown noise reduction mode";
> +                       valid = false;
> +               }
> +
> +               /* Can be null on non-FULL */

Does that mean non-FULL will print 'unknown noise reduction mode' on
every frame?


> +               if (valid)
> +                       resultMetadata->addEntry(ANDROID_NOISE_REDUCTION_MODE, value);

Hrm though, here a map might be far more concise here, as then it would
only addEntry if the key was found.

	if (metadata.contains(controls::NoiseReductionMode)) {
		static std::map<int, int> modes = {
			{ controls::NoiseReductionModeOff, ANDROID_NOISE_REDUCTION_MODE_OFF },
			{ controls::NoiseReductionModeFast, ANDROID_NOISE_REDUCTION_MODE_FAST },
			{ controls::NoiseReductionModeHighQuality, ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY },
			{ controls::NoiseReductionModeMinimal, ANDROID_NOISE_REDUCTION_MODE_MINIMAL },
			{ controls::NoiseReductionModeZSL, ANDROID_NOISE_REDUCTION_MODE_ZERO_SHUTTER_LAG },
		};

		auto it = modes.find(metadata.get(controls::NoiseReductionMode);
		if (it != modes.end())
			resultMetadata->addEntry(ANDROID_NOISE_REDUCTION_MODE, it->second);
		else
			LOG(HAL, Error) << "Unknown noise reduction mode";
	}

(I know that goes up to 112 chars there, but I think it's better
formatted with lines as a table)

Anyway, I see you changed this specifically to a switch case here
before, so either way:

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

> +       }
> +
>         if (metadata.contains(controls::draft::TestPatternMode)) {
>                 const int32_t testPatternMode =
>                         metadata.get(controls::draft::TestPatternMode);
> -- 
> 2.27.0
>

Patch
diff mbox series 

diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
index d6a1589e..365937ed 100644
--- a/src/android/camera_capabilities.cpp
+++ b/src/android/camera_capabilities.cpp
@@ -412,6 +412,31 @@  void CameraCapabilities::computeHwLevel(
 		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
 	}
 
+	found = staticMetadata_->entryContains<uint8_t>(
+		ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
+		ANDROID_NOISE_REDUCTION_MODE_OFF);
+	if (!found) {
+		LOG(HAL, Info) << noFull << "missing noise reduction mode off";
+		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
+	}
+
+	found = staticMetadata_->entryContains<uint8_t>(
+		ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
+		ANDROID_NOISE_REDUCTION_MODE_FAST);
+	if (!found) {
+		LOG(HAL, Info) << noFull << "missing noise reduction mode fast";
+		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
+	}
+
+	/* Android docs don't say this is required, but CTS does. */
+	found = staticMetadata_->entryContains<uint8_t>(
+		ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
+		ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY);
+	if (!found) {
+		LOG(HAL, Info) << noFull << "missing noise reduction mode high quality";
+		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
+	}
+
 	hwLevel_ = hwLevel;
 }
 
@@ -1727,7 +1752,7 @@  std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplatePreview() con
 	requestTemplate->addEntry(ANDROID_STATISTICS_FACE_DETECT_MODE,
 				  faceDetectMode);
 
-	uint8_t noiseReduction = ANDROID_NOISE_REDUCTION_MODE_OFF;
+	uint8_t noiseReduction = ANDROID_NOISE_REDUCTION_MODE_FAST;
 	requestTemplate->addEntry(ANDROID_NOISE_REDUCTION_MODE,
 				  noiseReduction);
 
@@ -1764,6 +1789,13 @@  std::unique_ptr<CameraMetadata> CameraCapabilities::requestTemplateStill() const
 		stillTemplate->appendEntry(ANDROID_EDGE_MODE, edgeMode);
 	}
 
+	if (staticMetadata_->entryContains<uint8_t>(
+			ANDROID_NOISE_REDUCTION_AVAILABLE_NOISE_REDUCTION_MODES,
+			ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY)) {
+		uint8_t noiseReduction = ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY;
+		stillTemplate->appendEntry(ANDROID_NOISE_REDUCTION_MODE, noiseReduction);
+	}
+
 	return stillTemplate;
 }
 
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 8861447d..e521ae0a 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -920,6 +920,39 @@  int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)
 			controls.set(controls::DigitalGain, lastDigitalGain_);
 	}
 
+	if (settings.getEntry(ANDROID_NOISE_REDUCTION_MODE, &entry)) {
+		const uint8_t data = *entry.data.u8;
+		int32_t noiseReductionMode;
+		switch (data) {
+		case ANDROID_NOISE_REDUCTION_MODE_OFF:
+			noiseReductionMode = controls::NoiseReductionModeOff;
+			break;
+
+		case ANDROID_NOISE_REDUCTION_MODE_FAST:
+			noiseReductionMode = controls::NoiseReductionModeFast;
+			break;
+
+		case ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY:
+			noiseReductionMode = controls::NoiseReductionModeHighQuality;
+			break;
+
+		case ANDROID_NOISE_REDUCTION_MODE_MINIMAL:
+			noiseReductionMode = controls::NoiseReductionModeRaw;
+			break;
+
+		case ANDROID_NOISE_REDUCTION_MODE_ZERO_SHUTTER_LAG:
+			noiseReductionMode = controls::NoiseReductionModeZSL;
+			break;
+
+		default:
+			LOG(HAL, Error)
+				<< "Unknown noise reduction mode: " << data;
+			return -EINVAL;
+		}
+
+		controls.set(controls::NoiseReductionMode, noiseReductionMode);
+	}
+
 	if (settings.getEntry(ANDROID_EDGE_MODE, &entry)) {
 		const auto &info = camera_->controls().find(&controls::Sharpness);
 		if (info != camera_->controls().end()) {
@@ -1587,9 +1620,6 @@  CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
 	value = ANDROID_STATISTICS_SCENE_FLICKER_NONE;
 	resultMetadata->addEntry(ANDROID_STATISTICS_SCENE_FLICKER, value);
 
-	value = ANDROID_NOISE_REDUCTION_MODE_OFF;
-	resultMetadata->addEntry(ANDROID_NOISE_REDUCTION_MODE, value);
-
 	/* 33.3 msec */
 	const int64_t rolling_shutter_skew = 33300000;
 	resultMetadata->addEntry(ANDROID_SENSOR_ROLLING_SHUTTER_SKEW,
@@ -1643,6 +1673,40 @@  CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
 		resultMetadata->addEntry(ANDROID_SCALER_CROP_REGION, cropRect);
 	}
 
+	if (metadata.contains(controls::NoiseReductionMode)) {
+		bool valid;
+		switch (metadata.get(controls::NoiseReductionMode)) {
+		case controls::NoiseReductionModeOff:
+			value = ANDROID_NOISE_REDUCTION_MODE_OFF;
+			valid = true;
+			break;
+		case controls::NoiseReductionModeFast:
+			value = ANDROID_NOISE_REDUCTION_MODE_FAST;
+			valid = true;
+			break;
+		case controls::NoiseReductionModeHighQuality:
+			value = ANDROID_NOISE_REDUCTION_MODE_HIGH_QUALITY;
+			valid = true;
+			break;
+		case controls::NoiseReductionModeRaw:
+			value = ANDROID_NOISE_REDUCTION_MODE_MINIMAL;
+			valid = true;
+			break;
+		case controls::NoiseReductionModeZSL:
+			value = ANDROID_NOISE_REDUCTION_MODE_ZERO_SHUTTER_LAG;
+			valid = true;
+			break;
+		default:
+			LOG(HAL, Error)
+				<< "Unknown noise reduction mode";
+			valid = false;
+		}
+
+		/* Can be null on non-FULL */
+		if (valid)
+			resultMetadata->addEntry(ANDROID_NOISE_REDUCTION_MODE, value);
+	}
+
 	if (metadata.contains(controls::draft::TestPatternMode)) {
 		const int32_t testPatternMode =
 			metadata.get(controls::draft::TestPatternMode);