[{"id":18210,"web_url":"https://patchwork.libcamera.org/comment/18210/","msgid":"<20210717101155.t2vrlrocs2yxsl7p@uno.localdomain>","date":"2021-07-17T10:11:55","subject":"Re: [libcamera-devel] [RFC PATCH v4 05/21] android: Add\n\tinfrastructure for determining 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\nOn Fri, Jul 16, 2021 at 07:56:15PM +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> Bug: https://bugs.libcamera.org/show_bug.cgi?id=55\n> Signed-off-by: Paul Elder \n>\n> ---\n> Changes in v5:\n> - change the whole thing to turn on capabilities after they are all\n> confirmed, instead of disabling them as conditions are not met\n>\n> Changes in v4:\n> - rebase on camera capabilities refactoring\n> - switch to std::set from std::map\n> - make hwlevel similar to capabilities\n>\n> Changes in v3:\n> - fix some compiler errors\n> - go ahead and initialize the capabilities to true, update the commit\n> message accordingly\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> src/android/camera_capabilities.cpp | 172 +++++++++++++++++++++++++---\n> src/android/camera_capabilities.h | 13 +++\n> 2 files changed, 170 insertions(+), 15 deletions(-)\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 9e2714f1..be55db88 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -9,6 +9,7 @@\n>\n> #include \n> #include \n> +#include \n>\n> #include \n>\n> @@ -114,8 +115,148 @@ const std::map camera3FormatsMap = {\n> \t},\n> };\n>\n> +const std::map\n> +hwLevelStrings = {\n> +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED, \"LIMITED\" },\n> +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL, \"FULL\" },\n> +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LEGACY, \"LEGACY\" },\n> +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_3, \"LEVEL_3\" },\n> +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_EXTERNAL, \"EXTERNAL\" },\n> +};\n> +\n> } /* namespace */\n>\n> +bool CameraCapabilities::validateManualSensorCapability(CameraMetadata *staticMetadata)\n> +{\n> +\tcamera_metadata_ro_entry_t entry;\n> +\tbool found;\n> +\n> +\tconst char *noMode = \"Manual sensor capability unavailable: \";\n> +\n> +\tif (!(found = staticMetadata->entryContains(ANDROID_CONTROL_AE_AVAILABLE_MODES,\n> +\t\t\t\t\t\t\t ANDROID_CONTROL_AE_MODE_OFF))) {\n> +\t\tLOG(HAL, Info)\n> +\t\t\t<< noMode << \"missing AE mode off: \"\n> +\t\t\t<< (found ? \"not supported\" : \"not found\");\n\nYou don't get here if (found) :)\n\n> +\t\treturn false;\n> +\t}\n> +\n> +\tfound = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,\n> +\t\t\t\t\t &entry);\n> +\tif (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {\n\nCan't this just be entryContains() ?\n\n> +\t\tLOG(HAL, Info) << noMode << \"missing AE lock\";\n> +\t\treturn false;\n> +\t}\n> +\n> +\treturn true;\n> +}\n> +\n> +bool CameraCapabilities::validateManualPostProcessingCapability(CameraMetadata *staticMetadata)\n> +{\n> +\tcamera_metadata_ro_entry_t entry;\n> +\tbool found;\n> +\n> +\tconst char *noMode = \"Manual post processing capability unavailable: \";\n> +\n> +\tif (!staticMetadata->entryContains(ANDROID_CONTROL_AWB_AVAILABLE_MODES,\n> +\t\t\t\t\t\t ANDROID_CONTROL_AWB_MODE_OFF)) {\n> +\t\tLOG(HAL, Info) << noMode << \"missing AWB mode off\";\n> +\t\treturn false;\n> +\t}\n> +\n> +\tfound = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n> +\t\t\t\t\t &entry);\n> +\tif (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {\n\nsame question, can this just be entryContains() ?\n\n> +\t\tLOG(HAL, Info) << noMode << \"missing AWB lock\";\n> +\t\treturn false;\n> +\t}\n> +\n> +\treturn true;\n> +}\n> +\n> +bool CameraCapabilities::validateBurstCaptureCapability(CameraMetadata *staticMetadata)\n> +{\n> +\tcamera_metadata_ro_entry_t entry;\n> +\tbool found;\n> +\n> +\tconst char *noMode = \"Burst capture capability unavailable: \";\n> +\n> +\tfound = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,\n> +\t\t\t\t\t &entry);\n> +\tif (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {\n\nditto, but I suspect I'm missing something as this is a recurring\npattern :)\n\n> +\t\tLOG(HAL, Info) << noMode << \"missing AE lock\";\n> +\t\treturn false;\n> +\t}\n> +\n> +\tfound = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n> +\t\t\t\t\t &entry);\n> +\tif (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {\n> +\t\tLOG(HAL, Info) << noMode << \"missing AWB lock\";\n> +\t\treturn false;\n> +\t}\n> +\n> +\tfound = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);\n> +\tif (!found || *entry.data.i32 < 0 || 4 < *entry.data.i32) {\n> +\t\tLOG(HAL, Info)\n> +\t\t\t<< noMode << \"max sync latency is \"\n> +\t\t\t<< (found ? std::to_string(*entry.data.i32) : \"not present\");\n> +\t\treturn false;\n> +\t}\n> +\n> +\treturn true;\n> +}\n> +\n> +std::set\n> +CameraCapabilities::computeCapabilities(CameraMetadata *staticMetadata,\n> +\tstd::set &existingCaps)\n> +{\n> +\tstd::set\n> +\tcapabilities = existingCaps;\n\nCan't you just add values to existingCaps ?\n\n> +\n> +\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE);\n> +\n> +\tif (validateManualSensorCapability(staticMetadata))\n> +\t\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);\n> +\n> +\tif (validateManualPostProcessingCapability(staticMetadata))\n> +\t\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);\n> +\n> +\tif (validateBurstCaptureCapability(staticMetadata))\n> +\t\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);\n> +\n> +\tif (rawStreamAvailable_)\n> +\t\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> +\n> +\treturn capabilities;\n> +}\n> +\n> +camera_metadata_enum_android_info_supported_hardware_level\n> +CameraCapabilities::computeHwLevel(CameraMetadata *staticMetadata,\n> +\tstd::set capabilities)\n> +{\n> +\tcamera_metadata_ro_entry_t entry;\n> +\tbool found;\n> +\tcamera_metadata_enum_android_info_supported_hardware_level\n> +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;\n> +\n> +\tif (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR))\n> +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> +\n> +\tif (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING))\n> +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> +\n> +\tif (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE))\n> +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> +\n> +\tfound = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);\n> +\tif (!found || *entry.data.i32 != 0)\n> +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> +\n> +\thwLevel_ = hwLevel;\n> +\n> +\treturn hwLevel;\n> +}\n> +\n> int CameraCapabilities::initialize(std::shared_ptr camera,\n> \t\t\t\t int orientation, int facing)\n> {\n> @@ -382,6 +523,9 @@ int CameraCapabilities::initializeStaticMetadata()\n> \tconst ControlInfoMap &controlsInfo = camera_->controls();\n> \tconst ControlList &properties = camera_->properties();\n>\n> +\tstd::set\n> +\tcapabilities = {};\n> +\n> \t/* Color correction static metadata. */\n> \t{\n> \t\tstd::vector data;\n> @@ -840,11 +984,6 @@ int CameraCapabilities::initializeStaticMetadata()\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> @@ -865,21 +1004,24 @@ int CameraCapabilities::initializeStaticMetadata()\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> -\tif (rawStreamAvailable_)\n> -\t\t\tavailableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> -\n> \t/* Number of { RAW, YUV, JPEG } supported output streams */\n> \tint32_t numOutStreams[] = { rawStreamAvailable_, 2, 1 };\n> \tstaticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,\n> \t\t\t\t numOutStreams);\n>\n> -\tstaticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,\n> -\t\t\t\t availableCapabilities);\n> +\t/* Check capabilities */\n> +\tcapabilities = computeCapabilities(staticMetadata_.get(), capabilities);\n\nCan you declate capabilities here ? It is not used before.\nAlso, what the puropse of 'existingCaps' in computeCapabilities() if\nit's empty ? Can you just return a vector to avoid the below\nconversion from set to vec ?\n\nI like the overall direction :)\n\nThanks\n j\n\n> +\tstd::vector\n> +\t\tcapsVec = std::vector(capabilities.begin(), capabilities.end());\n> +\tstaticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES, capsVec);\n> +\n> +\tcamera_metadata_enum_android_info_supported_hardware_level hwLevel =\n> +\t\tcomputeHwLevel(staticMetadata_.get(), capabilities);\n> +\tstaticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL, hwLevel);\n> +\n> +\tLOG(HAL, Info)\n> +\t\t<< \"Hardware level: \"\n> +\t\t<< hwLevelStrings.find((camera_metadata_enum_android_info_supported_hardware_level)hwLevel)->second;\n>\n> \tstd::vector availableCharacteristicsKeys = {\n> \t\tANDROID_COLOR_CORRECTION_AVAILABLE_ABERRATION_MODES,\n> diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> index 42a976d3..38ee97d0 100644\n> --- a/src/android/camera_capabilities.h\n> +++ b/src/android/camera_capabilities.h\n> @@ -42,6 +42,18 @@ private:\n> \t\tint androidFormat;\n> \t};\n>\n> +\tbool validateManualSensorCapability(CameraMetadata *staticMetadata);\n> +\tbool validateManualPostProcessingCapability(CameraMetadata *staticMetadata);\n> +\tbool validateBurstCaptureCapability(CameraMetadata *staticMetadata);\n> +\n> +\tstd::set\n> +\t\tcomputeCapabilities(CameraMetadata *staticMetadata,\n> +\t\t\tstd::set &existingCaps);\n> +\n> +\tcamera_metadata_enum_android_info_supported_hardware_level\n> +\t\tcomputeHwLevel(CameraMetadata *staticMetadata,\n> +\t\t\tstd::set capabilities);\n> +\n> \tstd::vector\n> \tinitializeYUVResolutions(const libcamera::PixelFormat &pixelFormat,\n> \t\t\t\t const std::vector &resolutions);\n> @@ -56,6 +68,7 @@ private:\n> \tint facing_;\n> \tint orientation_;\n> \tbool rawStreamAvailable_;\n> +\tcamera_metadata_enum_android_info_supported_hardware_level hwLevel_;\n>\n> \tstd::vector streamConfigurations_;\n> \tstd::map formatsMap_;\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 6B1CBC3228\n\tfor ;\n\tSat, 17 Jul 2021 10:11:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AA37768536;\n\tSat, 17 Jul 2021 12:11:09 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4275E6027B\n\tfor ;\n\tSat, 17 Jul 2021 12:11:08 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id B76FD60002;\n\tSat, 17 Jul 2021 10:11:07 +0000 (UTC)"],"Date":"Sat, 17 Jul 2021 12:11:55 +0200","From":"Jacopo Mondi ","To":"Paul Elder ","Message-ID":"<20210717101155.t2vrlrocs2yxsl7p@uno.localdomain>","References":"<20210716105631.158153-1-paul.elder@ideasonboard.com>\n\t<20210716105631.158153-6-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210716105631.158153-6-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFC PATCH v4 05/21] android: Add\n\tinfrastructure for determining 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":18223,"web_url":"https://patchwork.libcamera.org/comment/18223/","msgid":"<20210719054804.GJ2395@pyrite.rasen.tech>","date":"2021-07-19T05:48:04","subject":"Re: [libcamera-devel] [RFC PATCH v4 05/21] android: Add\n\tinfrastructure for determining 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 Jacopo,\n\nOn Sat, Jul 17, 2021 at 12:11:55PM +0200, Jacopo Mondi wrote:\n> Hi Paul,\n> \n> On Fri, Jul 16, 2021 at 07:56:15PM +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> > Bug: https://bugs.libcamera.org/show_bug.cgi?id=55\n> > Signed-off-by: Paul Elder \n> >\n> > ---\n> > Changes in v5:\n> > - change the whole thing to turn on capabilities after they are all\n> > confirmed, instead of disabling them as conditions are not met\n> >\n> > Changes in v4:\n> > - rebase on camera capabilities refactoring\n> > - switch to std::set from std::map\n> > - make hwlevel similar to capabilities\n> >\n> > Changes in v3:\n> > - fix some compiler errors\n> > - go ahead and initialize the capabilities to true, update the commit\n> > message accordingly\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> > src/android/camera_capabilities.cpp | 172 +++++++++++++++++++++++++---\n> > src/android/camera_capabilities.h | 13 +++\n> > 2 files changed, 170 insertions(+), 15 deletions(-)\n> >\n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index 9e2714f1..be55db88 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -9,6 +9,7 @@\n> >\n> > #include \n> > #include \n> > +#include \n> >\n> > #include \n> >\n> > @@ -114,8 +115,148 @@ const std::map camera3FormatsMap = {\n> > \t},\n> > };\n> >\n> > +const std::map\n> > +hwLevelStrings = {\n> > +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED, \"LIMITED\" },\n> > +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL, \"FULL\" },\n> > +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LEGACY, \"LEGACY\" },\n> > +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_3, \"LEVEL_3\" },\n> > +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_EXTERNAL, \"EXTERNAL\" },\n> > +};\n> > +\n> > } /* namespace */\n> >\n> > +bool CameraCapabilities::validateManualSensorCapability(CameraMetadata *staticMetadata)\n> > +{\n> > +\tcamera_metadata_ro_entry_t entry;\n> > +\tbool found;\n> > +\n> > +\tconst char *noMode = \"Manual sensor capability unavailable: \";\n> > +\n> > +\tif (!(found = staticMetadata->entryContains(ANDROID_CONTROL_AE_AVAILABLE_MODES,\n> > +\t\t\t\t\t\t\t ANDROID_CONTROL_AE_MODE_OFF))) {\n> > +\t\tLOG(HAL, Info)\n> > +\t\t\t<< noMode << \"missing AE mode off: \"\n> > +\t\t\t<< (found ? \"not supported\" : \"not found\");\n> \n> You don't get here if (found) :)\n> \n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\tfound = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,\n> > +\t\t\t\t\t &entry);\n> > +\tif (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {\n> \n> Can't this just be entryContains() ?\n\nEntry contains is if the the entry is a list of items. Here (and below)\nthe entry is a single item, so we can't use entryContains(). It's also\nnot worth a helper since it's just \"does the value equal X?\".\n\n> \n> > +\t\tLOG(HAL, Info) << noMode << \"missing AE lock\";\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\treturn true;\n> > +}\n> > +\n> > +bool CameraCapabilities::validateManualPostProcessingCapability(CameraMetadata *staticMetadata)\n> > +{\n> > +\tcamera_metadata_ro_entry_t entry;\n> > +\tbool found;\n> > +\n> > +\tconst char *noMode = \"Manual post processing capability unavailable: \";\n> > +\n> > +\tif (!staticMetadata->entryContains(ANDROID_CONTROL_AWB_AVAILABLE_MODES,\n> > +\t\t\t\t\t\t ANDROID_CONTROL_AWB_MODE_OFF)) {\n> > +\t\tLOG(HAL, Info) << noMode << \"missing AWB mode off\";\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\tfound = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n> > +\t\t\t\t\t &entry);\n> > +\tif (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {\n> \n> same question, can this just be entryContains() ?\n> \n> > +\t\tLOG(HAL, Info) << noMode << \"missing AWB lock\";\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\treturn true;\n> > +}\n> > +\n> > +bool CameraCapabilities::validateBurstCaptureCapability(CameraMetadata *staticMetadata)\n> > +{\n> > +\tcamera_metadata_ro_entry_t entry;\n> > +\tbool found;\n> > +\n> > +\tconst char *noMode = \"Burst capture capability unavailable: \";\n> > +\n> > +\tfound = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,\n> > +\t\t\t\t\t &entry);\n> > +\tif (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {\n> \n> ditto, but I suspect I'm missing something as this is a recurring\n> pattern :)\n> \n> > +\t\tLOG(HAL, Info) << noMode << \"missing AE lock\";\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\tfound = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n> > +\t\t\t\t\t &entry);\n> > +\tif (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {\n> > +\t\tLOG(HAL, Info) << noMode << \"missing AWB lock\";\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\tfound = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);\n> > +\tif (!found || *entry.data.i32 < 0 || 4 < *entry.data.i32) {\n> > +\t\tLOG(HAL, Info)\n> > +\t\t\t<< noMode << \"max sync latency is \"\n> > +\t\t\t<< (found ? std::to_string(*entry.data.i32) : \"not present\");\n> > +\t\treturn false;\n> > +\t}\n> > +\n> > +\treturn true;\n> > +}\n> > +\n> > +std::set\n> > +CameraCapabilities::computeCapabilities(CameraMetadata *staticMetadata,\n> > +\tstd::set &existingCaps)\n> > +{\n> > +\tstd::set\n> > +\tcapabilities = existingCaps;\n> \n> Can't you just add values to existingCaps ?\n\nOh I guess I can.\n\n> \n> > +\n> > +\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE);\n> > +\n> > +\tif (validateManualSensorCapability(staticMetadata))\n> > +\t\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);\n> > +\n> > +\tif (validateManualPostProcessingCapability(staticMetadata))\n> > +\t\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);\n> > +\n> > +\tif (validateBurstCaptureCapability(staticMetadata))\n> > +\t\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);\n> > +\n> > +\tif (rawStreamAvailable_)\n> > +\t\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> > +\n> > +\treturn capabilities;\n> > +}\n> > +\n> > +camera_metadata_enum_android_info_supported_hardware_level\n> > +CameraCapabilities::computeHwLevel(CameraMetadata *staticMetadata,\n> > +\tstd::set capabilities)\n> > +{\n> > +\tcamera_metadata_ro_entry_t entry;\n> > +\tbool found;\n> > +\tcamera_metadata_enum_android_info_supported_hardware_level\n> > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;\n> > +\n> > +\tif (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR))\n> > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > +\n> > +\tif (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING))\n> > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > +\n> > +\tif (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE))\n> > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > +\n> > +\tfound = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);\n> > +\tif (!found || *entry.data.i32 != 0)\n> > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > +\n> > +\thwLevel_ = hwLevel;\n> > +\n> > +\treturn hwLevel;\n> > +}\n> > +\n> > int CameraCapabilities::initialize(std::shared_ptr camera,\n> > \t\t\t\t int orientation, int facing)\n> > {\n> > @@ -382,6 +523,9 @@ int CameraCapabilities::initializeStaticMetadata()\n> > \tconst ControlInfoMap &controlsInfo = camera_->controls();\n> > \tconst ControlList &properties = camera_->properties();\n> >\n> > +\tstd::set\n> > +\tcapabilities = {};\n> > +\n> > \t/* Color correction static metadata. */\n> > \t{\n> > \t\tstd::vector data;\n> > @@ -840,11 +984,6 @@ int CameraCapabilities::initializeStaticMetadata()\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> > @@ -865,21 +1004,24 @@ int CameraCapabilities::initializeStaticMetadata()\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> > -\tif (rawStreamAvailable_)\n> > -\t\t\tavailableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> > -\n> > \t/* Number of { RAW, YUV, JPEG } supported output streams */\n> > \tint32_t numOutStreams[] = { rawStreamAvailable_, 2, 1 };\n> > \tstaticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,\n> > \t\t\t\t numOutStreams);\n> >\n> > -\tstaticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,\n> > -\t\t\t\t availableCapabilities);\n> > +\t/* Check capabilities */\n> > +\tcapabilities = computeCapabilities(staticMetadata_.get(), capabilities);\n> \n> Can you declate capabilities here ? It is not used before.\n\nOh, right.\n\n> Also, what the puropse of 'existingCaps' in computeCapabilities() if\n> it's empty ? Can you just return a vector to avoid the below\n> conversion from set to vec ?\n\nI guess we can return a vector. But we'd still have a conversion, it'll\njust be in computeCapabilities() instead of out.\n\n> \n> I like the overall direction :)\n\n\nThanks,\n\nPaul\n\n> \n> > +\tstd::vector\n> > +\t\tcapsVec = std::vector(capabilities.begin(), capabilities.end());\n> > +\tstaticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES, capsVec);\n> > +\n> > +\tcamera_metadata_enum_android_info_supported_hardware_level hwLevel =\n> > +\t\tcomputeHwLevel(staticMetadata_.get(), capabilities);\n> > +\tstaticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL, hwLevel);\n> > +\n> > +\tLOG(HAL, Info)\n> > +\t\t<< \"Hardware level: \"\n> > +\t\t<< hwLevelStrings.find((camera_metadata_enum_android_info_supported_hardware_level)hwLevel)->second;\n> >\n> > \tstd::vector availableCharacteristicsKeys = {\n> > \t\tANDROID_COLOR_CORRECTION_AVAILABLE_ABERRATION_MODES,\n> > diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> > index 42a976d3..38ee97d0 100644\n> > --- a/src/android/camera_capabilities.h\n> > +++ b/src/android/camera_capabilities.h\n> > @@ -42,6 +42,18 @@ private:\n> > \t\tint androidFormat;\n> > \t};\n> >\n> > +\tbool validateManualSensorCapability(CameraMetadata *staticMetadata);\n> > +\tbool validateManualPostProcessingCapability(CameraMetadata *staticMetadata);\n> > +\tbool validateBurstCaptureCapability(CameraMetadata *staticMetadata);\n> > +\n> > +\tstd::set\n> > +\t\tcomputeCapabilities(CameraMetadata *staticMetadata,\n> > +\t\t\tstd::set &existingCaps);\n> > +\n> > +\tcamera_metadata_enum_android_info_supported_hardware_level\n> > +\t\tcomputeHwLevel(CameraMetadata *staticMetadata,\n> > +\t\t\tstd::set capabilities);\n> > +\n> > \tstd::vector\n> > \tinitializeYUVResolutions(const libcamera::PixelFormat &pixelFormat,\n> > \t\t\t\t const std::vector &resolutions);\n> > @@ -56,6 +68,7 @@ private:\n> > \tint facing_;\n> > \tint orientation_;\n> > \tbool rawStreamAvailable_;\n> > +\tcamera_metadata_enum_android_info_supported_hardware_level hwLevel_;\n> >\n> > \tstd::vector streamConfigurations_;\n> > \tstd::map formatsMap_;\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 05033C322B\n\tfor ;\n\tMon, 19 Jul 2021 05:48:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 45A1A68521;\n\tMon, 19 Jul 2021 07:48:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4A60060278\n\tfor ;\n\tMon, 19 Jul 2021 07:48:13 +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 9B612465;\n\tMon, 19 Jul 2021 07:48:11 +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=\"c2oLMxDe\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1626673692;\n\tbh=x5uI0pLfEXQeMDmlgfdwHLjNdUwGo0fg6s63xBrGDXo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=c2oLMxDe1PkQ0assmVd9DJdPuziDulLk6n69OKb2e/aeyCdxk2tAoB8dZ3Lor44xD\n\toCcnz0T4td+1d172UHMhtYLoGI4JDpImmxDTvo1ufUoSOztu7q1atGsgX6Am7UkKDQ\n\tRHGl1PU2LTYV2GrZDp7Lrr8oZymJFCSwIhCMiHWA=","Date":"Mon, 19 Jul 2021 14:48:04 +0900","From":"paul.elder@ideasonboard.com","To":"Jacopo Mondi ","Message-ID":"<20210719054804.GJ2395@pyrite.rasen.tech>","References":"<20210716105631.158153-1-paul.elder@ideasonboard.com>\n\t<20210716105631.158153-6-paul.elder@ideasonboard.com>\n\t<20210717101155.t2vrlrocs2yxsl7p@uno.localdomain>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210717101155.t2vrlrocs2yxsl7p@uno.localdomain>","Subject":"Re: [libcamera-devel] [RFC PATCH v4 05/21] android: Add\n\tinfrastructure for determining 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":18229,"web_url":"https://patchwork.libcamera.org/comment/18229/","msgid":"<20210719102254.GO2395@pyrite.rasen.tech>","date":"2021-07-19T10:22:54","subject":"Re: [libcamera-devel] [RFC PATCH v4 05/21] android: Add\n\tinfrastructure for determining 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 Jacopo,\n\nOn Mon, Jul 19, 2021 at 02:48:04PM +0900, paul.elder@ideasonboard.com wrote:\n> Hi Jacopo,\n> \n> On Sat, Jul 17, 2021 at 12:11:55PM +0200, Jacopo Mondi wrote:\n> > Hi Paul,\n> > \n> > On Fri, Jul 16, 2021 at 07:56:15PM +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> > > Bug: https://bugs.libcamera.org/show_bug.cgi?id=55\n> > > Signed-off-by: Paul Elder \n> > >\n> > > ---\n> > > Changes in v5:\n> > > - change the whole thing to turn on capabilities after they are all\n> > > confirmed, instead of disabling them as conditions are not met\n> > >\n> > > Changes in v4:\n> > > - rebase on camera capabilities refactoring\n> > > - switch to std::set from std::map\n> > > - make hwlevel similar to capabilities\n> > >\n> > > Changes in v3:\n> > > - fix some compiler errors\n> > > - go ahead and initialize the capabilities to true, update the commit\n> > > message accordingly\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> > > src/android/camera_capabilities.cpp | 172 +++++++++++++++++++++++++---\n> > > src/android/camera_capabilities.h | 13 +++\n> > > 2 files changed, 170 insertions(+), 15 deletions(-)\n> > >\n> > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > > index 9e2714f1..be55db88 100644\n> > > --- a/src/android/camera_capabilities.cpp\n> > > +++ b/src/android/camera_capabilities.cpp\n> > > @@ -9,6 +9,7 @@\n> > >\n> > > #include \n> > > #include \n> > > +#include \n> > >\n> > > #include \n> > >\n> > > @@ -114,8 +115,148 @@ const std::map camera3FormatsMap = {\n> > > \t},\n> > > };\n> > >\n> > > +const std::map\n> > > +hwLevelStrings = {\n> > > +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED, \"LIMITED\" },\n> > > +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL, \"FULL\" },\n> > > +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LEGACY, \"LEGACY\" },\n> > > +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_3, \"LEVEL_3\" },\n> > > +\t{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_EXTERNAL, \"EXTERNAL\" },\n> > > +};\n> > > +\n> > > } /* namespace */\n> > >\n> > > +bool CameraCapabilities::validateManualSensorCapability(CameraMetadata *staticMetadata)\n> > > +{\n> > > +\tcamera_metadata_ro_entry_t entry;\n> > > +\tbool found;\n> > > +\n> > > +\tconst char *noMode = \"Manual sensor capability unavailable: \";\n> > > +\n> > > +\tif (!(found = staticMetadata->entryContains(ANDROID_CONTROL_AE_AVAILABLE_MODES,\n> > > +\t\t\t\t\t\t\t ANDROID_CONTROL_AE_MODE_OFF))) {\n> > > +\t\tLOG(HAL, Info)\n> > > +\t\t\t<< noMode << \"missing AE mode off: \"\n> > > +\t\t\t<< (found ? \"not supported\" : \"not found\");\n> > \n> > You don't get here if (found) :)\n> > \n> > > +\t\treturn false;\n> > > +\t}\n> > > +\n> > > +\tfound = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,\n> > > +\t\t\t\t\t &entry);\n> > > +\tif (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {\n> > \n> > Can't this just be entryContains() ?\n> \n> Entry contains is if the the entry is a list of items. Here (and below)\n> the entry is a single item, so we can't use entryContains(). It's also\n> not worth a helper since it's just \"does the value equal X?\".\n> \n> > \n> > > +\t\tLOG(HAL, Info) << noMode << \"missing AE lock\";\n> > > +\t\treturn false;\n> > > +\t}\n> > > +\n> > > +\treturn true;\n> > > +}\n> > > +\n> > > +bool CameraCapabilities::validateManualPostProcessingCapability(CameraMetadata *staticMetadata)\n> > > +{\n> > > +\tcamera_metadata_ro_entry_t entry;\n> > > +\tbool found;\n> > > +\n> > > +\tconst char *noMode = \"Manual post processing capability unavailable: \";\n> > > +\n> > > +\tif (!staticMetadata->entryContains(ANDROID_CONTROL_AWB_AVAILABLE_MODES,\n> > > +\t\t\t\t\t\t ANDROID_CONTROL_AWB_MODE_OFF)) {\n> > > +\t\tLOG(HAL, Info) << noMode << \"missing AWB mode off\";\n> > > +\t\treturn false;\n> > > +\t}\n> > > +\n> > > +\tfound = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n> > > +\t\t\t\t\t &entry);\n> > > +\tif (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {\n> > \n> > same question, can this just be entryContains() ?\n> > \n> > > +\t\tLOG(HAL, Info) << noMode << \"missing AWB lock\";\n> > > +\t\treturn false;\n> > > +\t}\n> > > +\n> > > +\treturn true;\n> > > +}\n> > > +\n> > > +bool CameraCapabilities::validateBurstCaptureCapability(CameraMetadata *staticMetadata)\n> > > +{\n> > > +\tcamera_metadata_ro_entry_t entry;\n> > > +\tbool found;\n> > > +\n> > > +\tconst char *noMode = \"Burst capture capability unavailable: \";\n> > > +\n> > > +\tfound = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,\n> > > +\t\t\t\t\t &entry);\n> > > +\tif (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {\n> > \n> > ditto, but I suspect I'm missing something as this is a recurring\n> > pattern :)\n> > \n> > > +\t\tLOG(HAL, Info) << noMode << \"missing AE lock\";\n> > > +\t\treturn false;\n> > > +\t}\n> > > +\n> > > +\tfound = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,\n> > > +\t\t\t\t\t &entry);\n> > > +\tif (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {\n> > > +\t\tLOG(HAL, Info) << noMode << \"missing AWB lock\";\n> > > +\t\treturn false;\n> > > +\t}\n> > > +\n> > > +\tfound = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);\n> > > +\tif (!found || *entry.data.i32 < 0 || 4 < *entry.data.i32) {\n> > > +\t\tLOG(HAL, Info)\n> > > +\t\t\t<< noMode << \"max sync latency is \"\n> > > +\t\t\t<< (found ? std::to_string(*entry.data.i32) : \"not present\");\n> > > +\t\treturn false;\n> > > +\t}\n> > > +\n> > > +\treturn true;\n> > > +}\n> > > +\n> > > +std::set\n> > > +CameraCapabilities::computeCapabilities(CameraMetadata *staticMetadata,\n> > > +\tstd::set &existingCaps)\n> > > +{\n> > > +\tstd::set\n> > > +\tcapabilities = existingCaps;\n> > \n> > Can't you just add values to existingCaps ?\n> \n> Oh I guess I can.\n> \n> > \n> > > +\n> > > +\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE);\n> > > +\n> > > +\tif (validateManualSensorCapability(staticMetadata))\n> > > +\t\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);\n> > > +\n> > > +\tif (validateManualPostProcessingCapability(staticMetadata))\n> > > +\t\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);\n> > > +\n> > > +\tif (validateBurstCaptureCapability(staticMetadata))\n> > > +\t\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);\n> > > +\n> > > +\tif (rawStreamAvailable_)\n> > > +\t\tcapabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> > > +\n> > > +\treturn capabilities;\n> > > +}\n> > > +\n> > > +camera_metadata_enum_android_info_supported_hardware_level\n> > > +CameraCapabilities::computeHwLevel(CameraMetadata *staticMetadata,\n> > > +\tstd::set capabilities)\n> > > +{\n> > > +\tcamera_metadata_ro_entry_t entry;\n> > > +\tbool found;\n> > > +\tcamera_metadata_enum_android_info_supported_hardware_level\n> > > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;\n> > > +\n> > > +\tif (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR))\n> > > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > > +\n> > > +\tif (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING))\n> > > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > > +\n> > > +\tif (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE))\n> > > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > > +\n> > > +\tfound = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);\n> > > +\tif (!found || *entry.data.i32 != 0)\n> > > +\t\thwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;\n> > > +\n> > > +\thwLevel_ = hwLevel;\n> > > +\n> > > +\treturn hwLevel;\n> > > +}\n> > > +\n> > > int CameraCapabilities::initialize(std::shared_ptr camera,\n> > > \t\t\t\t int orientation, int facing)\n> > > {\n> > > @@ -382,6 +523,9 @@ int CameraCapabilities::initializeStaticMetadata()\n> > > \tconst ControlInfoMap &controlsInfo = camera_->controls();\n> > > \tconst ControlList &properties = camera_->properties();\n> > >\n> > > +\tstd::set\n> > > +\tcapabilities = {};\n> > > +\n> > > \t/* Color correction static metadata. */\n> > > \t{\n> > > \t\tstd::vector data;\n> > > @@ -840,11 +984,6 @@ int CameraCapabilities::initializeStaticMetadata()\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> > > @@ -865,21 +1004,24 @@ int CameraCapabilities::initializeStaticMetadata()\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> > > -\tif (rawStreamAvailable_)\n> > > -\t\t\tavailableCapabilities.push_back(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);\n> > > -\n> > > \t/* Number of { RAW, YUV, JPEG } supported output streams */\n> > > \tint32_t numOutStreams[] = { rawStreamAvailable_, 2, 1 };\n> > > \tstaticMetadata_->addEntry(ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS,\n> > > \t\t\t\t numOutStreams);\n> > >\n> > > -\tstaticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,\n> > > -\t\t\t\t availableCapabilities);\n> > > +\t/* Check capabilities */\n> > > +\tcapabilities = computeCapabilities(staticMetadata_.get(), capabilities);\n> > \n> > Can you declate capabilities here ? It is not used before.\n> \n> Oh, right.\n> \n> > Also, what the puropse of 'existingCaps' in computeCapabilities() if\n> > it's empty ? Can you just return a vector to avoid the below\n\nOh, this was from before your raw capability patch. I had the raw\ncapability checker add the capability to the set before passing it in. I\nguess we can remove existingCaps now that we don't need it anymore.\n\n\nPaul\n\n> > conversion from set to vec ?\n> \n> I guess we can return a vector. But we'd still have a conversion, it'll\n> just be in computeCapabilities() instead of out.\n> \n> > \n> > I like the overall direction :)\n> \n> > \n> > > +\tstd::vector\n> > > +\t\tcapsVec = std::vector(capabilities.begin(), capabilities.end());\n> > > +\tstaticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES, capsVec);\n> > > +\n> > > +\tcamera_metadata_enum_android_info_supported_hardware_level hwLevel =\n> > > +\t\tcomputeHwLevel(staticMetadata_.get(), capabilities);\n> > > +\tstaticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL, hwLevel);\n> > > +\n> > > +\tLOG(HAL, Info)\n> > > +\t\t<< \"Hardware level: \"\n> > > +\t\t<< hwLevelStrings.find((camera_metadata_enum_android_info_supported_hardware_level)hwLevel)->second;\n> > >\n> > > \tstd::vector availableCharacteristicsKeys = {\n> > > \t\tANDROID_COLOR_CORRECTION_AVAILABLE_ABERRATION_MODES,\n> > > diff --git a/src/android/camera_capabilities.h b/src/android/camera_capabilities.h\n> > > index 42a976d3..38ee97d0 100644\n> > > --- a/src/android/camera_capabilities.h\n> > > +++ b/src/android/camera_capabilities.h\n> > > @@ -42,6 +42,18 @@ private:\n> > > \t\tint androidFormat;\n> > > \t};\n> > >\n> > > +\tbool validateManualSensorCapability(CameraMetadata *staticMetadata);\n> > > +\tbool validateManualPostProcessingCapability(CameraMetadata *staticMetadata);\n> > > +\tbool validateBurstCaptureCapability(CameraMetadata *staticMetadata);\n> > > +\n> > > +\tstd::set\n> > > +\t\tcomputeCapabilities(CameraMetadata *staticMetadata,\n> > > +\t\t\tstd::set &existingCaps);\n> > > +\n> > > +\tcamera_metadata_enum_android_info_supported_hardware_level\n> > > +\t\tcomputeHwLevel(CameraMetadata *staticMetadata,\n> > > +\t\t\tstd::set capabilities);\n> > > +\n> > > \tstd::vector\n> > > \tinitializeYUVResolutions(const libcamera::PixelFormat &pixelFormat,\n> > > \t\t\t\t const std::vector &resolutions);\n> > > @@ -56,6 +68,7 @@ private:\n> > > \tint facing_;\n> > > \tint orientation_;\n> > > \tbool rawStreamAvailable_;\n> > > +\tcamera_metadata_enum_android_info_supported_hardware_level hwLevel_;\n> > >\n> > > \tstd::vector streamConfigurations_;\n> > > \tstd::map formatsMap_;\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 21E27C0109\n\tfor ;\n\tMon, 19 Jul 2021 10:23:04 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 776EF6853A;\n\tMon, 19 Jul 2021 12:23:03 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9F2C868537\n\tfor ;\n\tMon, 19 Jul 2021 12:23:01 +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 0460B465;\n\tMon, 19 Jul 2021 12:22:59 +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=\"p5eVY1zb\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1626690181;\n\tbh=OhFGmIhfHXqcZu3QRIdwdMWA0v0DFGvGf80NJWr0Mo8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=p5eVY1zbwX8PsQqUbJvC1VoNjSi6+lnijbjy1Wqp0W0ArjuzKpVcy7lvoOWXRVyXM\n\t439wyYS6RqG7CqLkD3xQgFBmKgct3NANmpvbjQaGMeX4ljCRzNi3kqUA1DDEasTvAd\n\tuT0jS8M9kisRDyDflpvLh9sliTYZHUh0FSJ7MImM=","Date":"Mon, 19 Jul 2021 19:22:54 +0900","From":"paul.elder@ideasonboard.com","To":"Jacopo Mondi ","Message-ID":"<20210719102254.GO2395@pyrite.rasen.tech>","References":"<20210716105631.158153-1-paul.elder@ideasonboard.com>\n\t<20210716105631.158153-6-paul.elder@ideasonboard.com>\n\t<20210717101155.t2vrlrocs2yxsl7p@uno.localdomain>\n\t<20210719054804.GJ2395@pyrite.rasen.tech>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210719054804.GJ2395@pyrite.rasen.tech>","Subject":"Re: [libcamera-devel] [RFC PATCH v4 05/21] android: Add\n\tinfrastructure for determining 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\" "}}]