[libcamera-devel] [PATCH v2] android: Plumb Sharpness control into EDGE_MODE
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Jan 3 22:24:42 CET 2022
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 at 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 at 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
>
More information about the libcamera-devel
mailing list