[{"id":17608,"web_url":"https://patchwork.libcamera.org/comment/17608/","msgid":"<20210617075429.GH1351869@pyrite.rasen.tech>","date":"2021-06-17T07:54:29","subject":"Re: [libcamera-devel] [PATCH v2] android: Add infrastructure for\n\tdetermining capabilities and hardware level","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi me,\n\nOn Thu, Jun 17, 2021 at 04:38:56PM +0900, Paul Elder wrote:\n> Add the infrastructure for checking and reporting capabilities. Use\n> these capabilities to determine the hardware level as well.\n> \n> Since the raw capability is set to true when support is determined to be\n> available, leave that as default false and set to true, since copying\n> the pattern of the other capabilities would cause redundant code.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=55\n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v2:\n> - add a flag for FULL, since there are a few requirements that are not\n> obtained from capabilities alone\n> - add burst capture capability, since that is required for FULL as well\n> \n> This is my vision of how we would support the various capabilities.\n> Although we don't have anything for FULL yet; I imagine that we would\n> start the flags for manual sensor and manual post processing with true,\n> and then if a required control is unavailable, then we would set the\n> flag to false.\n> \n> I considered declaring an enum in CameraDevice to mirror the android\n> ones, just for shorthand, but it seemed like a lot of code for not much\n> gain. Unless the shorthand would be valuable because these constant\n> names are so long?\n> \n> I think the available keys lists will have to be moved to the head of\n> the function, and then as available controls are discovered add them to\n> that list.\n> ---\n> src/android/camera_device.cpp | 45 +++++++++++++++++++++++++++--------\n> 1 file changed, 35 insertions(+), 10 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 8c71fd06..0dcdda23 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -13,6 +13,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -840,6 +841,19 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tconst ControlInfoMap &controlsInfo = camera_->controls();\n> \tconst ControlList &properties = camera_->properties();\n> \n> +\tstd::map\n> +\tcapabilities = {\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE, true },\n> +\t\t/* \\todo Change these three to true when we have checks for them */\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE, false },\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR, false },\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING, false },\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW, false },\n> +\t};\n> +\n> +\t/* \\todo Change this to true when we have checks for it */\n> +\tbool fullSupport = false;\n> +\n> \t/* Color correction static metadata. */\n> \t{\n> \t\tstd::vector data;\n> @@ -1298,11 +1312,6 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tuint8_t croppingType = ANDROID_SCALER_CROPPING_TYPE_CENTER_ONLY;\n> \tstaticMetadata_->addEntry(ANDROID_SCALER_CROPPING_TYPE, croppingType);\n> \n> -\t/* Info static metadata. */\n> -\tuint8_t supportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> -\tstaticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,\n> -\t\t\t\t supportedHWLevel);\n> -\n> \t/* Request static metadata. */\n> \tint32_t partialResultCount = 1;\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_PARTIAL_RESULT_COUNT,\n> @@ -1323,10 +1332,6 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_INPUT_STREAMS,\n> \t\t\t\t maxNumInputStreams);\n> \n> -\tstd::vector availableCapabilities = {\n> -\t\tANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE,\n> -\t};\n> -\n> \t/* Report if camera supports RAW. */\n> \tbool rawStreamAvailable = false;\n> \tstd::unique_ptr cameraConfig =\n> @@ -1338,7 +1343,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&\n> \t\t info.bitsPerPixel == 16) {\n> \t\t\trawStreamAvailable = true;\n> -\t\t\tavailableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> +\t\t\tcapabilities[ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW] = true;\n> \t\t}\n> \t}\n> \n> @@ -1347,9 +1352,29 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,\n> \t\t\t\t numOutStreams);\n> \n> +\t/* Check capabilities */\n> +\tstd::vector availableCapabilities;\n> +\tfor (auto cap : capabilities) {\n> +\t\tif (cap.second)\n> +\t\t\tavailableCapabilities.push_back(CapabilityTable.at(cap.first));\n\nCapabilityTable doesn't exist as of v1.\n\n> +\t}\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,\n> \t\t\t\t availableCapabilities);\n> \n> +\tuint8_t supportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> +\tif (capabilities[ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR] &&\n> +\t capabilities[ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING] &&\n> +\t capabilities[ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE] &&\n> +\t fullSupport)\n> +\t\tsupportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;\n> +\tstaticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,\n> +\t\t\t\t supportedHWLevel);\n> +\n> +\tLOG(HAL, Info)\n> +\t\t<< \"Hardware level: \"\n> +\t\t<< supportedHWLevel == ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL\n\n<< has higher precedence than ==.\n\n\nPaul\n\n> +\t\t ? \"FULL\" : \"LIMITED\";\n> +\n> \tstd::vector availableCharacteristicsKeys = {\n> \t\tANDROID_COLOR_CORRECTION_AVAILABLE_ABERRATION_MODES,\n> \t\tANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES,\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 EBF8FC3218\n\tfor ;\n\tThu, 17 Jun 2021 07:54:38 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6CE9368944;\n\tThu, 17 Jun 2021 09:54:38 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CA1C660297\n\tfor ;\n\tThu, 17 Jun 2021 09:54:36 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B3B9C3E5\n\tfor ;\n\tThu, 17 Jun 2021 09:54:35 +0200 (CEST)"],"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=\"mwNcgzqc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623916476;\n\tbh=9vf0hkS6j94hbpau1NkzExYI39z60IfuE0rhUPMMPsc=;\n\th=Date:From:To:Subject:References:In-Reply-To:From;\n\tb=mwNcgzqcBO0dChLoO2FloUAfSUDH5sMPIdBPB84yftc72CdL+degQHzNTkMkx0ZQa\n\tKIptMomsYemIhoyvs2jeSJBw84ANN1jAO+QMKgtyMQKxIZibTlTqHRB3WF1yueHNlG\n\tmntrmDthmSlg3mb32L36oUATcJLq4evjS1LQ2n84=","Date":"Thu, 17 Jun 2021 16:54:29 +0900","From":"paul.elder@ideasonboard.com","To":"libcamera-devel@lists.libcamera.org","Message-ID":"<20210617075429.GH1351869@pyrite.rasen.tech>","References":"<20210617073856.1457185-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210617073856.1457185-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] android: Add infrastructure for\n\tdetermining capabilities and hardware level","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\" "}},{"id":17617,"web_url":"https://patchwork.libcamera.org/comment/17617/","msgid":"","date":"2021-06-17T14:48:55","subject":"Re: [libcamera-devel] [PATCH v2] android: Add infrastructure for\n\tdetermining capabilities and hardware level","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Thu, Jun 17, 2021 at 04:38:56PM +0900, Paul Elder wrote:\n> Add the infrastructure for checking and reporting capabilities. Use\n> these capabilities to determine the hardware level as well.\n> \n> Since the raw capability is set to true when support is determined to be\n> available, leave that as default false and set to true, since copying\n> the pattern of the other capabilities would cause redundant code.\n> \n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=55\n> Signed-off-by: Paul Elder \n> \n> ---\n> Changes in v2:\n> - add a flag for FULL, since there are a few requirements that are not\n> obtained from capabilities alone\n> - add burst capture capability, since that is required for FULL as well\n> \n> This is my vision of how we would support the various capabilities.\n> Although we don't have anything for FULL yet; I imagine that we would\n> start the flags for manual sensor and manual post processing with true,\n> and then if a required control is unavailable, then we would set the\n> flag to false.\n> \n> I considered declaring an enum in CameraDevice to mirror the android\n> ones, just for shorthand, but it seemed like a lot of code for not much\n> gain. Unless the shorthand would be valuable because these constant\n> names are so long?\n\nThey're long, but all lines are long in that function, so I don't mind.\nIf we start shortening one enum then we'll do a second one, and I fear\nwe'll rewrite all constants :-)\n\n> I think the available keys lists will have to be moved to the head of\n> the function, and then as available controls are discovered add them to\n> that list.\n> ---\n> src/android/camera_device.cpp | 45 +++++++++++++++++++++++++++--------\n> 1 file changed, 35 insertions(+), 10 deletions(-)\n> \n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 8c71fd06..0dcdda23 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -13,6 +13,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -840,6 +841,19 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tconst ControlInfoMap &controlsInfo = camera_->controls();\n> \tconst ControlList &properties = camera_->properties();\n> \n> +\tstd::map\n\nI think std::set\nwould be a better data type. You can easily add, remove and check for\npresence of entries. The drawback is that lookups would be slightly\nlonger to write. Instead of\n\n\tif (capabilities[FOO])\n\nyou would need\n\n\tif (capabilities.count(FOO))\n\nThere's also\n\n\tif (capabilities.contains(FOO))\n\nbut only in C++20 :-S\n\nI'd rule out\n\n\tif (capabilities.find(FOO) != capabilities.end())\n\nas that's too verbose here.\n\nAnother option would be a bitmask :-) (std::bitset can be useful).\n\n> +\tcapabilities = {\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE, true },\n> +\t\t/* \\todo Change these three to true when we have checks for them */\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE, false },\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR, false },\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING, false },\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW, false },\n> +\t};\n> +\n> +\t/* \\todo Change this to true when we have checks for it */\n> +\tbool fullSupport = false;\n> +\n> \t/* Color correction static metadata. */\n> \t{\n> \t\tstd::vector data;\n> @@ -1298,11 +1312,6 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tuint8_t croppingType = ANDROID_SCALER_CROPPING_TYPE_CENTER_ONLY;\n> \tstaticMetadata_->addEntry(ANDROID_SCALER_CROPPING_TYPE, croppingType);\n> \n> -\t/* Info static metadata. */\n> -\tuint8_t supportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> -\tstaticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,\n> -\t\t\t\t supportedHWLevel);\n> -\n> \t/* Request static metadata. */\n> \tint32_t partialResultCount = 1;\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_PARTIAL_RESULT_COUNT,\n> @@ -1323,10 +1332,6 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_INPUT_STREAMS,\n> \t\t\t\t maxNumInputStreams);\n> \n> -\tstd::vector availableCapabilities = {\n> -\t\tANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE,\n> -\t};\n> -\n> \t/* Report if camera supports RAW. */\n> \tbool rawStreamAvailable = false;\n> \tstd::unique_ptr cameraConfig =\n> @@ -1338,7 +1343,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&\n> \t\t info.bitsPerPixel == 16) {\n> \t\t\trawStreamAvailable = true;\n> -\t\t\tavailableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> +\t\t\tcapabilities[ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW] = true;\n> \t\t}\n> \t}\n> \n> @@ -1347,9 +1352,29 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,\n> \t\t\t\t numOutStreams);\n> \n> +\t/* Check capabilities */\n> +\tstd::vector availableCapabilities;\n> +\tfor (auto cap : capabilities) {\n> +\t\tif (cap.second)\n> +\t\t\tavailableCapabilities.push_back(CapabilityTable.at(cap.first));\n> +\t}\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,\n> \t\t\t\t availableCapabilities);\n> \n> +\tuint8_t supportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> +\tif (capabilities[ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR] &&\n> +\t capabilities[ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING] &&\n> +\t capabilities[ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE] &&\n> +\t fullSupport)\n> +\t\tsupportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;\n> +\tstaticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,\n> +\t\t\t\t supportedHWLevel);\n> +\n> +\tLOG(HAL, Info)\n> +\t\t<< \"Hardware level: \"\n> +\t\t<< supportedHWLevel == ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL\n> +\t\t ? \"FULL\" : \"LIMITED\";\n\nI'd add a static table of strings indexed by\ncamera_metadata_enum_android_info_supported_hardware_level value, that\nway we'll be ready for other hardware levels (including external).\n\nThis otherwise looks fine to me.\n\n> +\n> \tstd::vector availableCharacteristicsKeys = {\n> \t\tANDROID_COLOR_CORRECTION_AVAILABLE_ABERRATION_MODES,\n> \t\tANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES,","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 6FBD5BD78E\n\tfor ;\n\tThu, 17 Jun 2021 14:49:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AE54568944;\n\tThu, 17 Jun 2021 16:49:18 +0200 (CEST)","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 01A036050C\n\tfor ;\n\tThu, 17 Jun 2021 16:49:17 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 72881E7B;\n\tThu, 17 Jun 2021 16:49:17 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"SzpkkTrL\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1623941357;\n\tbh=VAAHPLXoRC5e5CBeEed0Hp/vkwdHmK6HrbmPUOcNDMQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=SzpkkTrLwANp0yw76mk7CwUFSAKBtgGdPdHojsYH2nyrYFrtqTcm73vYoH59Zyx2W\n\tJqbqmCkn7/4cfTYsNI/iWuXkPaQfNDxriFm5k9fPBtytqI4RvHDOadU6vKjRLQYs/e\n\tN7ORAmm8yuAzDgB6aiP3F7SGVNTsfqJ5RXv+/Jm4=","Date":"Thu, 17 Jun 2021 17:48:55 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210617073856.1457185-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210617073856.1457185-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] android: Add infrastructure for\n\tdetermining capabilities and hardware level","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17620,"web_url":"https://patchwork.libcamera.org/comment/17620/","msgid":"<20210618084515.h2ximadpsomuukfc@uno.localdomain>","date":"2021-06-18T08:45:15","subject":"Re: [libcamera-devel] [PATCH v2] android: Add infrastructure for\n\tdetermining capabilities and hardware level","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Paul,\n thanks for addressing this.\n\nOn Thu, Jun 17, 2021 at 04:38:56PM +0900, Paul Elder wrote:\n> Add the infrastructure for checking and reporting capabilities. Use\n> these capabilities to determine the hardware level as well.\n>\n> Since the raw capability is set to true when support is determined to be\n> available, leave that as default false and set to true, since copying\n> the pattern of the other capabilities would cause redundant code.\n>\n> Bug: https://bugs.libcamera.org/show_bug.cgi?id=55\n> Signed-off-by: Paul Elder \n>\n> ---\n> Changes in v2:\n> - add a flag for FULL, since there are a few requirements that are not\n> obtained from capabilities alone\n> - add burst capture capability, since that is required for FULL as well\n>\n> This is my vision of how we would support the various capabilities.\n> Although we don't have anything for FULL yet; I imagine that we would\n> start the flags for manual sensor and manual post processing with true,\n> and then if a required control is unavailable, then we would set the\n> flag to false.\n>\n> I considered declaring an enum in CameraDevice to mirror the android\n> ones, just for shorthand, but it seemed like a lot of code for not much\n> gain. Unless the shorthand would be valuable because these constant\n> names are so long?\n>\n> I think the available keys lists will have to be moved to the head of\n> the function, and then as available controls are discovered add them to\n> that list.\n> ---\n\nMore than a comment on the patch itself, let me try to formalize the\nproblem a bit\n\nWe have 4 different 'items':\n- supported streams\n- static metadata\n- capabilities\n- hw level\n\n- supported streams: self descriptive, we already collect them in\n initializeStreamConfigurations()\n\n- static metadata: List of static properties as collected in\n getStaticMetadata with an associated set of values\n\n This is what we collect in getStaticMetadata by inspecting the\n libcamera::Camera capabilities plus some other information from the\n configuration file.\n\n A static metadata might be present/not present, and if it is present\n it might have associated values\n\n- capabilities: A list of static metadata which has to be present in the static\n metadata pack with associated values and a list of required streams\n\n In example, from the android.request.availableCapabilities\n documentation the RAW capability requires:\n - An output stream in RAW_FORMAT format with an output size equal to\n the sensor pixel array\n - DNG related metadata (the list to be formalized)\n\n For MANUAL_SENSOR capability:\n - android.control.aeMode contains OFF\n - android.control.awbMode contains OFF\n - android.control.afMode contains OFF\n - ANDROID_REQUEST_AVAILABLE_RESULT_KEYS contains android.sensor.frameDuration\n - ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS contains\n android.sensor.info.maxFrameDuration\n - ...\n\n- hw level: a list of requested 'capabilities', a list of static\n metadata that has to be present in the static metadata pack and a\n list of request streams\n\n In example, FULL requires\n - The stream list required for FULL, LIMITED and LEGACY levels\n (see the table here\n https://developer.android.com/reference/android/hardware/camera2/CameraDevice.html#regular-capture)\n - The following capabilities:\n - BURST_CAPTURE, MANUAL_SENSOR, MANUAL_POST_PROCESSING\n - A list of static metadata and associated requirements for values\n - android.sync.maxLatency == PER_FRAME_CONTROL\n - max(android.sensor.info.exposureTimeRange) >= 100ms\n - max(android.sensor.info.maxFrameDuration) >= 100ms\n - android.scaler.croppingType contains CENTERED\n\nIt's not easy to formalize this in a table and I'm afraid all\nthese checks should be open coded and built iteratively\n\n* Build the list of supported streams like we're doing\n* Build the list of static metadata like we're doing\n* Build the list of capabilities inspecting the supported streams and\n static metadata\n* Infer the HW level by inspecting capabilities, static metadata and\n the list of supported streams\n\nI think we're good for the first two steps, the last two have to be\ndone and as I've said I suspect it's not easy to formalize all the\nrequirements in a table (not impossible though, we can associate to\neach capability a list of streams and metadata, to each HW level a\nlist of capabilities, streams and metadata... Although how to express\nconstrains like \"maxFrameDuration should be 1s and MUST be > 100ms\"\nmight end up requiring more complexity than open coding)\n\nNow, that's potentially quite some code, and I would really take the\noccasion to break out from camera_device.cpp which is already ~2400\nloc the parts that deal with:\n- stream list contruction (initializeStreamConfigurations() function)\n- static metadata pack contructions (getStaticMetadata() function)\n- capabilities building\n- HW level inferring\n\nIf we move all of this to a CameraCapabilities (?) class we can logically\nseparate it from the CameraDevice. I don't see huge blockers if not\nthat the CameraCapabilities class should receive quite some parameters\nfrom CameraDevice, but we can work it out.\n\nShould we start moving the existing code to a new class and the build\nthe last two steps (capabilities and hw level) there ?\n\nWhat do you think ?\n\nThanks\n j\n\n> src/android/camera_device.cpp | 45 +++++++++++++++++++++++++++--------\n> 1 file changed, 35 insertions(+), 10 deletions(-)\n>\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 8c71fd06..0dcdda23 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -13,6 +13,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -840,6 +841,19 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tconst ControlInfoMap &controlsInfo = camera_->controls();\n> \tconst ControlList &properties = camera_->properties();\n>\n> +\tstd::map\n> +\tcapabilities = {\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE, true },\n> +\t\t/* \\todo Change these three to true when we have checks for them */\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE, false },\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR, false },\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING, false },\n> +\t\t{ ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW, false },\n> +\t};\n> +\n> +\t/* \\todo Change this to true when we have checks for it */\n> +\tbool fullSupport = false;\n> +\n> \t/* Color correction static metadata. */\n> \t{\n> \t\tstd::vector data;\n> @@ -1298,11 +1312,6 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tuint8_t croppingType = ANDROID_SCALER_CROPPING_TYPE_CENTER_ONLY;\n> \tstaticMetadata_->addEntry(ANDROID_SCALER_CROPPING_TYPE, croppingType);\n>\n> -\t/* Info static metadata. */\n> -\tuint8_t supportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> -\tstaticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,\n> -\t\t\t\t supportedHWLevel);\n> -\n> \t/* Request static metadata. */\n> \tint32_t partialResultCount = 1;\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_PARTIAL_RESULT_COUNT,\n> @@ -1323,10 +1332,6 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_INPUT_STREAMS,\n> \t\t\t\t maxNumInputStreams);\n>\n> -\tstd::vector availableCapabilities = {\n> -\t\tANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE,\n> -\t};\n> -\n> \t/* Report if camera supports RAW. */\n> \tbool rawStreamAvailable = false;\n> \tstd::unique_ptr cameraConfig =\n> @@ -1338,7 +1343,7 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&\n> \t\t info.bitsPerPixel == 16) {\n> \t\t\trawStreamAvailable = true;\n> -\t\t\tavailableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> +\t\t\tcapabilities[ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW] = true;\n> \t\t}\n> \t}\n>\n> @@ -1347,9 +1352,29 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,\n> \t\t\t\t numOutStreams);\n>\n> +\t/* Check capabilities */\n> +\tstd::vector availableCapabilities;\n> +\tfor (auto cap : capabilities) {\n> +\t\tif (cap.second)\n> +\t\t\tavailableCapabilities.push_back(CapabilityTable.at(cap.first));\n> +\t}\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,\n> \t\t\t\t availableCapabilities);\n>\n> +\tuint8_t supportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> +\tif (capabilities[ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR] &&\n> +\t capabilities[ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING] &&\n> +\t capabilities[ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE] &&\n> +\t fullSupport)\n> +\t\tsupportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;\n> +\tstaticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,\n> +\t\t\t\t supportedHWLevel);\n> +\n> +\tLOG(HAL, Info)\n> +\t\t<< \"Hardware level: \"\n> +\t\t<< supportedHWLevel == ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL\n> +\t\t ? \"FULL\" : \"LIMITED\";\n> +\n> \tstd::vector availableCharacteristicsKeys = {\n> \t\tANDROID_COLOR_CORRECTION_AVAILABLE_ABERRATION_MODES,\n> \t\tANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES,\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 A5B0ABD78E\n\tfor ;\n\tFri, 18 Jun 2021 08:44:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B73326892E;\n\tFri, 18 Jun 2021 10:44:28 +0200 (CEST)","from relay1-d.mail.gandi.net (relay1-d.mail.gandi.net\n\t[217.70.183.193])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 99CC560296\n\tfor ;\n\tFri, 18 Jun 2021 10:44:27 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 9B0B9240005;\n\tFri, 18 Jun 2021 08:44:26 +0000 (UTC)"],"Date":"Fri, 18 Jun 2021 10:45:15 +0200","From":"Jacopo Mondi ","To":"Paul Elder ","Message-ID":"<20210618084515.h2ximadpsomuukfc@uno.localdomain>","References":"<20210617073856.1457185-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210617073856.1457185-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2] android: Add infrastructure for\n\tdetermining capabilities and hardware level","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]