[libcamera-devel] [PATCH v2 3/3] android: Plumb noise reduction
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Jan 3 23:30:21 CET 2022
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 at 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 at ideasonboard.com>
> + }
> +
> if (metadata.contains(controls::draft::TestPatternMode)) {
> const int32_t testPatternMode =
> metadata.get(controls::draft::TestPatternMode);
> --
> 2.27.0
>
More information about the libcamera-devel
mailing list