[{"id":21021,"web_url":"https://patchwork.libcamera.org/comment/21021/","msgid":"<163731991444.1089182.5131218462796518887@Monstersaurus>","date":"2021-11-19T11:05:14","subject":"Re: [libcamera-devel] [PATCH v4] android: Check exposure time range\n\tfor manual sensor capability","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Paul Elder (2021-11-19 09:48:22)\n> In the manual sensor capability validator, add a check for the presence\n> of the exposure time range key, and for the maximum exposure time. The\n> minimum exposure time is a requirement for adding the key in the first\n> place; add a check for this as well.\n> \n> If either requirement is not met, the manual sensor capability\n> validation will fail, therefore disabling the FULL hardware level. The\n> exposure time range key is optional in non-FULL hardware levels.\n> \n> Signed-off-by: Paul Elder \n> Reviewed-by: Laurent Pinchart \n> Reviewed-by: Jean-Michel Hautbois \n> Reviewed-by: Kieran Bingham \n> Reviewed-by: Umang Jain \n> \n> ---\n> Changes in v4:\n> - s/i32/i64\n> \n> Changes in v3:\n> - squash \"android: capabilities: Add exposure time keys only if\n> available\"\n> - fix the minumum exposure time check\n> - only make the exposure time range key available if this check\n> passes. additionally, if the max exposure time passes its check,\n> tick the box for manual sensor and FULL\n> - update commit message accordingly\n> \n> Changes in v2:\n> - fix comparator order (cosmetic)\n> - change comparators and comments to \"equal or\", as that is what is\n> specificied in the hal docs\n> - add check for minimum exposure time when initializing static metadata\n> ---\n> src/android/camera_capabilities.cpp | 33 +++++++++++++++++++++++++----\n> 1 file changed, 29 insertions(+), 4 deletions(-)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index f357902e..2a1a428c 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -217,6 +217,8 @@ std::vector setMetadata(CameraMetadata *metadata, uint32_t tag,\n> \n> bool CameraCapabilities::validateManualSensorCapability()\n> {\n> + camera_metadata_ro_entry_t entry;\n> +\n> const char *noMode = \"Manual sensor capability unavailable: \";\n> \n> if (!staticMetadata_->entryContains(ANDROID_CONTROL_AE_AVAILABLE_MODES,\n> @@ -231,6 +233,19 @@ bool CameraCapabilities::validateManualSensorCapability()\n> return false;\n> }\n> \n> + if (!staticMetadata_->hasEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE)) {\n> + LOG(HAL, Info) << noMode << \"missing exposure time range\";\n> + return false;\n> + }\n> +\n> + staticMetadata_->getEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE, &entry);\n> + if (entry.data.i64[1] <= 100000000) {\n> + LOG(HAL, Info)\n> + << noMode\n> + << \"exposure time range maximum must be larger than 100ms\";\n> + return false;\n> + }\n> +\n> /*\n> * \\todo Return true here after we satisfy all the requirements:\n> * https://developer.android.com/reference/android/hardware/camera2/CameraMetadata#REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR\n> @@ -790,7 +805,6 @@ int CameraCapabilities::initializeStaticMetadata()\n> ANDROID_SENSOR_AVAILABLE_TEST_PATTERN_MODES,\n> ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE,\n> ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,\n> - ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE,\n> ANDROID_SENSOR_INFO_MAX_FRAME_DURATION,\n> ANDROID_SENSOR_INFO_PHYSICAL_SIZE,\n> ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,\n> @@ -868,7 +882,6 @@ int CameraCapabilities::initializeStaticMetadata()\n> ANDROID_NOISE_REDUCTION_MODE,\n> ANDROID_REQUEST_PIPELINE_DEPTH,\n> ANDROID_SCALER_CROP_REGION,\n> - ANDROID_SENSOR_EXPOSURE_TIME,\n> ANDROID_SENSOR_FRAME_DURATION,\n> ANDROID_SENSOR_ROLLING_SHUTTER_SKEW,\n> ANDROID_SENSOR_TEST_PATTERN_MODE,\n> @@ -1074,8 +1087,20 @@ int CameraCapabilities::initializeStaticMetadata()\n> exposureInfo->second.min().get() * 1000LL,\n> exposureInfo->second.max().get() * 1000LL,\n> };\n> - staticMetadata_->addEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE,\n> - exposureTimeRange, 2);\n> +\n> + if (exposureTimeRange[0] < 100000) {\n> + staticMetadata_->addEntry(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE,\n> + exposureTimeRange, 2);\n> +\n> + availableCharacteristicsKeys_.insert(ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE);\n> + availableRequestKeys_.insert(ANDROID_SENSOR_EXPOSURE_TIME);\n> + availableResultKeys_.insert(ANDROID_SENSOR_EXPOSURE_TIME);\n\nLooks fine since the last update I looked at.\n\n\nI've seen this pattern come in a couple of times in your patches.\n\n(Not for this patch) should we have a helper...\n\taddOptionalMetadata(characteristic, key, ....)\n\nso that those four lines can be wrapped to:\n\taddOptionalMetadata(ANDROID_SENSOR_EXPOSURE_TIME,\n\t\t\t ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE,\n\t\t\t exposureTimeRange, 2);\n\n\nNot essential, just wondering if it would help if that's a repeatable\npattern.\n\nAnd if it ends up obfuscating the intentions then certainly not worth it\n;-)\n\n--\nKieran\n\n\n> + } else {\n> + LOG(HAL, Info)\n> + << \"Minimum exposure time \"\n> + << exposureTimeRange[0]\n> + << \"ns is too big (should be smaller than 100us)\";\n> + }\n> }\n> \n> staticMetadata_->addEntry(ANDROID_SENSOR_ORIENTATION, orientation_);\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 31F11BF415\n\tfor ;\n\tFri, 19 Nov 2021 11:05:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8DC3060233;\n\tFri, 19 Nov 2021 12:05:19 +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 4EA8C600B5\n\tfor ;\n\tFri, 19 Nov 2021 12:05:17 +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 BC51D1974;\n\tFri, 19 Nov 2021 12:05:16 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"Uz//GjlQ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1637319916;\n\tbh=D8BPNiqV9KD49xDggflQA4mIROe3Hf1Yqz/YpvTFdK4=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=Uz//GjlQ14ip0N4Cq48ZZ7VmG97Jy3H6KyprZ91C/hqqD4B43U9/g2V8xOmWo+B9/\n\tSgbJp0T7uAtMRcdooVEUn0hL4ICg3u0EVpZZ+GRE5alG2Wu+5vr6TPKEUas366s2j1\n\tzqBCy4al++ZjxzTYhwUxRGG7osGi6HahgywGODzA=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20211119094822.856444-1-paul.elder@ideasonboard.com>","References":"<20211119094822.856444-1-paul.elder@ideasonboard.com>","From":"Kieran Bingham ","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Fri, 19 Nov 2021 11:05:14 +0000","Message-ID":"<163731991444.1089182.5131218462796518887@Monstersaurus>","User-Agent":"alot/0.10","Subject":"Re: [libcamera-devel] [PATCH v4] android: Check exposure time range\n\tfor manual sensor capability","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\" "}}]