[{"id":21928,"web_url":"https://patchwork.libcamera.org/comment/21928/","msgid":"<164124508205.682966.4514424054181868959@Monstersaurus>","date":"2022-01-03T21:24:42","subject":"Re: [libcamera-devel] [PATCH v2] android: Plumb Sharpness control\n\tinto EDGE_MODE","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Paul Elder (2021-12-21 04:41:33)\n> Plumb the Sharpness control into the HAL for EDGE_MODE and other related\n> android controls.\n> \n> libcamera doesn't distinguish between fast and HQ, but rather with a\n> floating value for how much sharpening to apply. This is thus\n> unsufficient information for retaining whether the request was for fast\n\ns/unsufficient/insufficient/\n\n> or HQ, so save it in the request, and report what was requested.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=46\n> Signed-off-by: Paul Elder \n> \n> ---\n> This patch depends on \"android: camera_metadata: Add appendEntry helper\"\n> \n> Changes in v2:\n> - fix the assertions in template creation\n> - report the edge mode that was requested, instead of doing conversion\n> to and from the floating value and its ControlInfo\n> - move edge mode capability check out of manual control (as docs say\n> it's not required) and into the top-level FULL hardware level\n> validator\n> ---\n> src/android/camera_capabilities.cpp | 44 +++++++++++++++++++++++++++++\n> src/android/camera_device.cpp | 28 ++++++++++++++++++\n> src/android/camera_request.h | 4 +++\n> 3 files changed, 76 insertions(+)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index b9a1f6e5..727204b9 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -401,6 +401,11 @@ void CameraCapabilities::computeHwLevel(\n> if (!caps.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE))\n> hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> \n> + if (!staticMetadata_->hasEntry(ANDROID_EDGE_AVAILABLE_EDGE_MODES)) {\n> + LOG(HAL, Info) << noFull << \"missing edge modes\";\n> + hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> + }\n> +\n> found = staticMetadata_->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);\n> if (!found || *entry.data.i32 != 0) {\n> LOG(HAL, Info) << noFull << \"missing or invalid max sync latency\";\n> @@ -1078,6 +1083,21 @@ int CameraCapabilities::initializeStaticMetadata()\n> staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,\n> availableControlModes);\n> \n> + const auto &edgeInfo = controlsInfo.find(&controls::Sharpness);\n> + if (edgeInfo != controlsInfo.end()) {\n> + std::vector availableEdgeModes = {\n> + ANDROID_EDGE_MODE_OFF,\n> + ANDROID_EDGE_MODE_FAST,\n> + ANDROID_EDGE_MODE_HIGH_QUALITY,\n> + };\n> +\n> + staticMetadata_->addEntry(ANDROID_EDGE_AVAILABLE_EDGE_MODES,\n> + availableEdgeModes);\n> + availableCharacteristicsKeys_.insert(ANDROID_EDGE_AVAILABLE_EDGE_MODES);\n> + availableRequestKeys_.insert(ANDROID_EDGE_MODE);\n> + availableResultKeys_.insert(ANDROID_EDGE_MODE);\n> + }\n> +\n> /* JPEG static metadata. */\n> \n> /*\n> @@ -1593,6 +1613,12 @@ std::unique_ptr CameraCapabilities::requestTemplateManual() cons\n> manualTemplate->appendEntry(ANDROID_CONTROL_AE_MODE, aeMode);\n> }\n> \n> + if (staticMetadata_->entryContains(ANDROID_EDGE_AVAILABLE_EDGE_MODES,\n> + ANDROID_EDGE_MODE_OFF)) {\n> + uint8_t edgeMode = ANDROID_EDGE_MODE_OFF;\n> + manualTemplate->appendEntry(ANDROID_EDGE_MODE, edgeMode);\n> + }\n> +\n> return manualTemplate;\n> }\n> \n> @@ -1675,6 +1701,12 @@ std::unique_ptr CameraCapabilities::requestTemplatePreview() con\n> if (availableRequestKeys_.count(ANDROID_SENSOR_SENSITIVITY))\n> requestTemplate->addEntry(ANDROID_SENSOR_SENSITIVITY, minISO_);\n> \n> + if (staticMetadata_->entryContains(ANDROID_EDGE_AVAILABLE_EDGE_MODES,\n> + ANDROID_EDGE_MODE_FAST)) {\n> + uint8_t edgeMode = ANDROID_EDGE_MODE_FAST;\n> + requestTemplate->addEntry(ANDROID_EDGE_MODE, edgeMode);\n> + }\n> +\n> uint8_t flashMode = ANDROID_FLASH_MODE_OFF;\n> requestTemplate->addEntry(ANDROID_FLASH_MODE, flashMode);\n> \n> @@ -1713,6 +1745,12 @@ std::unique_ptr CameraCapabilities::requestTemplateStill() const\n> if (!stillTemplate)\n> return nullptr;\n> \n> + if (staticMetadata_->entryContains(ANDROID_EDGE_AVAILABLE_EDGE_MODES,\n> + ANDROID_EDGE_MODE_HIGH_QUALITY)) {\n> + uint8_t edgeMode = ANDROID_EDGE_MODE_HIGH_QUALITY;\n> + stillTemplate->appendEntry(ANDROID_EDGE_MODE, edgeMode);\n> + }\n> +\n> return stillTemplate;\n> }\n> \n> @@ -1730,6 +1768,12 @@ std::unique_ptr CameraCapabilities::requestTemplateVideo() const\n> staticMetadata_->getEntry(ANDROID_CONTROL_AE_AVAILABLE_TARGET_FPS_RANGES,\n> &entry);\n> \n> + if (staticMetadata_->entryContains(ANDROID_EDGE_AVAILABLE_EDGE_MODES,\n> + ANDROID_EDGE_MODE_FAST)) {\n> + uint8_t edgeMode = ANDROID_EDGE_MODE_FAST;\n> + previewTemplate->appendEntry(ANDROID_EDGE_MODE, edgeMode);\n> + }\n> +\n> /*\n> * Assume the AE_AVAILABLE_TARGET_FPS_RANGE static metadata\n> * has been assembled as {{min, max} {max, max}}.\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 1a508923..0d668ea6 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -920,6 +920,26 @@ int CameraDevice::processControls(Camera3RequestDescriptor *descriptor)\n> controls.set(controls::DigitalGain, lastDigitalGain_);\n> }\n> \n> + if (settings.getEntry(ANDROID_EDGE_MODE, &entry)) {\n> + const auto &info = camera_->controls().find(&controls::Sharpness);\n> + if (info != camera_->controls().end()) {\n\nAs the ANDROID_EDGE_MODE is only enabled if the camera has a Sharpness\ncontrol, this should always be found right?\n\nStill, it doesn't hurt to make sure I think.\n\n> + float min = info->second.min().get();\n> + float def = info->second.def().get();\n> + float max = info->second.max().get();\n> + /*\n> + * The default value might be unusable since control\n> + * serialization ignores it. Alternatively the default\n\nWhy? Should we fix the serialisation to pass the default values? or is\nthis troublesome?\n\n> + * could be simply set to zero or the minimum value.\n> + * Use the maximum sharpness value in these cases.\n> + */\n> + float val = (def == 0.0f || def == min) ? max : def;\n> + controls.set(controls::Sharpness,\n> + *entry.data.u8 == ANDROID_EDGE_MODE_OFF ? min : val);\n> +\n> + descriptor->edgeMode_ = *entry.data.u8;\n\n\nSo, if I infer correctly this should set:\n EDGE_MODE_OFF : min\n EDGE_MODE_FAST : def, unless def == min or def == 0, when max\n EDGE_MODE_HIGH_QUALITY : same as ^\n\nShould HIGH_QUALITY always set max?\n\n\tdescriptor->edgeMode_ = *entry.data.u8;\n\tswitch (descriptor->edgeMode_) {\n\t\tdefault:\n\t\tcase ANDROID_EDGE_MODE_OFF:\n\t\t\tval = min;\n\t\t\tbreak;\n\t\tcase ANDROID_EDGE_MODE_FAST:\n\t\t\t/* ... comment about def, or fix serialisations? */ \n\t\t\tval = (def == 0.0f || def == min) ? max : def;\n\t\t\tbreak;\n\t\tcase ANDROID_EDGE_MODE_HIGH_QUALITY:\n\t\t\tval = max;\n\t\t\tbreak;\n\t}\n\n\nHrm, not quite as concise though, so maybe your way is fine if we don't\nintend to distinguish between the fast/HQ anyway.\n\n\nIf there's a reason we can't or shouldn't be serialising the default\nvalues to be able to reference them correctly then:\n\nReviewed-by: Kieran Bingham \n\n\n> + }\n> + }\n> +\n> return 0;\n> }\n> \n> @@ -1602,6 +1622,14 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> duration);\n> }\n> \n> + if (metadata.contains(controls::Sharpness)) {\n> + /*\n> + * libcamera doesn't distinguish between fast vs HQ sharpening\n> + * modes. Report the mode that was requested.\n> + */\n> + resultMetadata->addEntry(ANDROID_EDGE_MODE, descriptor.edgeMode_);\n> + }\n> +\n> if (metadata.contains(controls::ScalerCrop)) {\n> Rectangle crop = metadata.get(controls::ScalerCrop);\n> int32_t cropRect[] = {\n> diff --git a/src/android/camera_request.h b/src/android/camera_request.h\n> index b2809179..69b6c8fc 100644\n> --- a/src/android/camera_request.h\n> +++ b/src/android/camera_request.h\n> @@ -87,6 +87,10 @@ public:\n> /* The libcamera internal AE state for this request */\n> AutoMode aeMode_ = AutoMode::Auto;\n> \n> + /* The android edge mode associated with this request */\n> + /* \\todo Wrap all such controls? */\n> + int32_t edgeMode_;\n> +\n> private:\n> LIBCAMERA_DISABLE_COPY(Camera3RequestDescriptor)\n> };\n> -- \n> 2.27.0\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 7DC01BE080\n\tfor ;\n\tMon, 3 Jan 2022 21:24:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A1791608E9;\n\tMon, 3 Jan 2022 22:24:46 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E7180604F4\n\tfor ;\n\tMon, 3 Jan 2022 22:24:44 +0100 (CET)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8CE77E53;\n\tMon, 3 Jan 2022 22:24:44 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"FEFEY1XG\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1641245084;\n\tbh=Qv36N74PyG2IvQfF0DXlbeQOiQe6zXb9pO8cGmRdYJQ=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=FEFEY1XG474DuhlaDxuJH7/45BtoycqlgpZanZ2kL0HO4zNOox9PmPPDePykNAoLr\n\tcKYwVb3Ut1Nuw7cdjUMqo8fJOFaetYQ1nOb1RUAFQtmr4w8ph6k15d3avaJn5ZHHx2\n\tkdOiOTGONTplKFqKIMEWenGrxwlEX9RGpSH/gVxM=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211221044133.2531278-1-paul.elder@ideasonboard.com>","References":"<20211221044133.2531278-1-paul.elder@ideasonboard.com>","From":"Kieran Bingham ","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Mon, 03 Jan 2022 21:24:42 +0000","Message-ID":"<164124508205.682966.4514424054181868959@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v2] android: Plumb Sharpness control\n\tinto EDGE_MODE","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]