[libcamera-devel,RFC] wip: android: capabilities: Capability detection by population
diff mbox series

Message ID 20210709112814.87917-1-paul.elder@ideasonboard.com
State Superseded
Headers show
Series
  • [libcamera-devel,RFC] wip: android: capabilities: Capability detection by population
Related show

Commit Message

Paul Elder July 9, 2021, 11:28 a.m. UTC 
Implement capability and hardware level detection based on the static
metadata that has been set, instead of disabling them as requirements
are not met. This results in cleaner code where the static metadata is
set.

Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

---
This currently does not work as the ControlInfo constructor for Spans is
broken. This is more of an RFC for Jacopo to check that this is the
direction that he wants.

Clearly this is not meant to be merged, as the actual capability
requirements are not fully specified yet. Plus they belong in separate
patches anyway.

This patch does not apply either, as it depends on many unreleased
patches.
---
 src/android/camera_capabilities.cpp | 219 ++++++++++++++++++++--------
 1 file changed, 157 insertions(+), 62 deletions(-)

Comments

Paul Elder July 9, 2021, 11:34 a.m. UTC | #1 
Hello everyone,

On Fri, Jul 09, 2021 at 08:28:14PM +0900, Paul Elder wrote:
> Implement capability and hardware level detection based on the static
> metadata that has been set, instead of disabling them as requirements
> are not met. This results in cleaner code where the static metadata is
> set.
> 
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> 
> ---
> This currently does not work as the ControlInfo constructor for Spans is
> broken. This is more of an RFC for Jacopo to check that this is the
> direction that he wants.
> 
> Clearly this is not meant to be merged, as the actual capability
> requirements are not fully specified yet. Plus they belong in separate
> patches anyway.
> 
> This patch does not apply either, as it depends on many unreleased
> patches.
> ---
>  src/android/camera_capabilities.cpp | 219 ++++++++++++++++++++--------
>  1 file changed, 157 insertions(+), 62 deletions(-)
> 
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 83c7f0d0..ceb5cfe8 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -262,6 +262,149 @@ std::vector<T> setMetadata(CameraMetadata *metadata, uint32_t tag,
>  	return ret;
>  }
>  
> +
> +template<typename T>
> +bool metadataContains(camera_metadata_ro_entry_t &entry, T value);
> +
> +template<>
> +bool metadataContains<uint8_t>(camera_metadata_ro_entry_t &entry, uint8_t value)
> +{
> +	for (unsigned int i = 0; i < entry.count; i++)
> +		if (entry.data.u8[i] == value)
> +			return true;
> +
> +	return false;
> +}
> +
> +bool validateManualSensorCapability(CameraMetadata *staticMetadata)
> +{
> +	camera_metadata_ro_entry_t entry;
> +	bool found;
> +
> +	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,
> +					 &entry);
> +	if (!found || !metadataContains<uint8_t>(entry, ANDROID_CONTROL_AE_MODE_OFF)) {
> +		LOG(HAL, Info)
> +			<< "Missing AE mode off: "
> +			<< (found ? "not supported" : "not found");
> +		return false;
> +	}
> +
> +	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> +					 &entry);
> +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {
> +		LOG(HAL, Info) << "Missing AE lock";
> +		return false;
> +	}
> +
> +	found = staticMetadata->getEntry(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
> +					 &entry);
> +	if (!found) {
> +		LOG(HAL, Info) << "Missing edge modes";
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +bool validateManualPostProcessingCapability(CameraMetadata *staticMetadata)
> +{
> +	camera_metadata_ro_entry_t entry;
> +	bool found;
> +
> +	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_AVAILABLE_MODES,
> +					 &entry);
> +	if (!found || !metadataContains<uint8_t>(entry, ANDROID_CONTROL_AWB_MODE_OFF)) {
> +		LOG(HAL, Info) << "Missing AWB mode off";
> +		return false;
> +	}
> +
> +	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> +					 &entry);
> +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {
> +		LOG(HAL, Info) << "Missing AWB lock";
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +bool validateBurstCaptureCapability(CameraMetadata *staticMetadata)
> +{
> +	camera_metadata_ro_entry_t entry;
> +	bool found;
> +
> +	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> +					 &entry);
> +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {
> +		LOG(HAL, Info) << "Missing AE lock";
> +		return false;
> +	}
> +
> +	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> +					 &entry);
> +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {
> +		LOG(HAL, Info) << "Missing AWB lock";
> +		return false;
> +	}
> +
> +	found = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);
> +	if (!found || *entry.data.i32 < 0 || 4 < *entry.data.i32) {
> +		LOG(HAL, Info)
> +			<< "Max sync latency is "
> +			<< (found ? std::to_string(*entry.data.i32) : "not present");
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +std::set<camera_metadata_enum_android_request_available_capabilities>
> +computeCapabilities(CameraMetadata *staticMetadata,
> +		    std::set<camera_metadata_enum_android_request_available_capabilities> &existingCaps)
> +{
> +	std::set<camera_metadata_enum_android_request_available_capabilities>
> +	capabilities = existingCaps;
> +
> +	capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE);
> +
> +	if (validateManualSensorCapability(staticMetadata))
> +		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> +
> +	if (validateManualPostProcessingCapability(staticMetadata))
> +		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
> +
> +	if (validateBurstCaptureCapability(staticMetadata))
> +		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> +
> +	return capabilities;
> +}
> +
> +camera_metadata_enum_android_info_supported_hardware_level
> +computeHwLevel(CameraMetadata *staticMetadata,
> +	       std::set<camera_metadata_enum_android_request_available_capabilities> capabilities)
> +{
> +	camera_metadata_ro_entry_t entry;
> +	bool found;
> +	camera_metadata_enum_android_info_supported_hardware_level
> +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;
> +
> +	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR))
> +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> +
> +	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING))
> +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> +
> +	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE))
> +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> +
> +	found = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);
> +	if (!found || *entry.data.i32 != 0)
> +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> +
> +	return hwLevel;
> +}
> +
>  } /* namespace */
>  
>  int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,
> @@ -655,17 +798,7 @@ int CameraCapabilities::initializeStaticMetadata()
>  	};
>  
>  	std::set<camera_metadata_enum_android_request_available_capabilities>
> -	capabilities = {
> -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE,
> -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR,
> -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING,
> -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE,
> -	};
> -
> -	std::set<camera_metadata_enum_android_info_supported_hardware_level>
> -	hwLevels = {
> -		ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL,
> -	};
> +	capabilities = {};
>  
>  	/* Color correction static metadata. */
>  	{
> @@ -692,19 +825,12 @@ int CameraCapabilities::initializeStaticMetadata()
>  	staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES,
>  				  aeAvailableAntiBandingModes);
>  
> -	std::vector<uint8_t> aeModes = setMetadata<uint8_t, bool>(
> +	setMetadata<uint8_t, bool>(
>  		staticMetadata_.get(),
>  		ANDROID_CONTROL_AE_AVAILABLE_MODES,
>  		controlsInfo, &controls::AeEnable,
>  		{ ANDROID_CONTROL_AE_MODE_ON });
>  
> -	if (std::find(aeModes.begin(), aeModes.end(),
> -		      ANDROID_CONTROL_AE_MODE_OFF) == aeModes.end()) {
> -		LOG(HAL, Info) << "AE cannot be turned off";
> -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> -	}
> -
>  	int64_t minFrameDurationNsec = -1;
>  	int64_t maxFrameDurationNsec = -1;
>  	const auto frameDurationsInfo = controlsInfo.find(&controls::FrameDurationLimits);
> @@ -791,17 +917,11 @@ int CameraCapabilities::initializeStaticMetadata()
>  	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_VIDEO_STABILIZATION_MODES,
>  				  availableStabilizationModes);
>  
> -	std::vector<uint8_t> awbModes = setMetadata<uint8_t, int32_t>(
> +	setMetadata<uint8_t, int32_t>(
>  		staticMetadata_.get(),
>  		ANDROID_CONTROL_AWB_AVAILABLE_MODES,
>  		controlsInfo, &controls::AwbMode,
>  		{ ANDROID_CONTROL_AWB_MODE_AUTO });
> -	if (std::find(awbModes.begin(), awbModes.end(),
> -		      ANDROID_CONTROL_AWB_MODE_OFF) == awbModes.end()) {
> -		LOG(HAL, Info) << "AWB cannot be turned off";
> -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
> -	}
>  
>  	std::vector<int32_t> availableMaxRegions = {
>  		0, 0, 0,
> @@ -817,29 +937,19 @@ int CameraCapabilities::initializeStaticMetadata()
>  	staticMetadata_->addEntry(ANDROID_CONTROL_SCENE_MODE_OVERRIDES,
>  				  sceneModesOverride);
>  
> -	uint8_t aeLockAvailable = setMetadata<uint8_t, bool>(
> +	setMetadata<uint8_t, bool>(
>  		staticMetadata_.get(),
>  		ANDROID_CONTROL_AE_LOCK_AVAILABLE,
>  		controlsInfo, &controls::AeLock,
>  		ControlRange::Max,
>  		ANDROID_CONTROL_AE_LOCK_AVAILABLE_FALSE);
> -	if (aeLockAvailable != ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE) {
> -		LOG(HAL, Info) << "AE lock is unavailable";
> -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> -	}
>  
> -	uint8_t awbLockAvailable = setMetadata<uint8_t, bool>(
> +	setMetadata<uint8_t, bool>(
>  		staticMetadata_.get(),
>  		ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
>  		controlsInfo, &controls::AwbLock,
>  		ControlRange::Max,
>  		ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE);
> -	if (awbLockAvailable != ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE) {
> -		LOG(HAL, Info) << "AWB lock is unavailable";
> -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
> -	}
>  
>  	char availableControlModes = ANDROID_CONTROL_MODE_AUTO;
>  	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,
> @@ -859,10 +969,6 @@ int CameraCapabilities::initializeStaticMetadata()
>  		availableCharacteristicsKeys_.insert(ANDROID_EDGE_AVAILABLE_EDGE_MODES);
>  		availableRequestKeys_.insert(ANDROID_EDGE_MODE);
>  		availableResultKeys_.insert(ANDROID_EDGE_MODE);
> -	} else {
> -		LOG(HAL, Info) << "Edge mode unavailable";
> -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
>  	}
>  
>  	/* JPEG static metadata. */
> @@ -1046,17 +1152,11 @@ int CameraCapabilities::initializeStaticMetadata()
>  	}
>  
>  	/* Sync static metadata. */
> -	int32_t maxLatency = setMetadata<int32_t, int32_t>(
> +	setMetadata<int32_t, int32_t>(
>  		staticMetadata_.get(), ANDROID_SYNC_MAX_LATENCY,
>  		controlsInfo, &controls::draft::MaxLatency,
>  		ControlRange::Def,
>  		ANDROID_SYNC_MAX_LATENCY_UNKNOWN);
> -	LOG(HAL, Info) << "Max sync latency is " << maxLatency;
> -	/* CTS allows a sync latency of up to 4 for burst capture capability */
> -	if (maxLatency < 0 || 4 < maxLatency)
> -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> -	if (maxLatency != 0)
> -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
>  
>  	/* Flash static metadata. */
>  	char flashAvailable = ANDROID_FLASH_INFO_AVAILABLE_FALSE;
> @@ -1220,19 +1320,14 @@ int CameraCapabilities::initializeStaticMetadata()
>  				  numOutStreams);
>  
>  	/* Check capabilities */
> -	std::vector<uint8_t> availableCapabilities(capabilities.begin(),
> -						   capabilities.end());
> -	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
> -				  availableCapabilities);
> -
> -	uint8_t hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> -	if (capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR) &&
> -	    capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING) &&
> -	    capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE) &&
> -	    hwLevels.count(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL))
> -		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;
> -	staticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,
> -				  hwLevel);
> +	capabilities = computeCapabilities(staticMetadata_.get(), capabilities);

Just to clarify, this is here because above this we have:

	/* Report if camera supports RAW. */
	bool rawStreamAvailable = false;
	std::unique_ptr<CameraConfiguration> cameraConfig =
		camera_->generateConfiguration({ StreamRole::Raw });
	if (cameraConfig && !cameraConfig->empty()) {
		const PixelFormatInfo &info =
			PixelFormatInfo::info(cameraConfig->at(0).pixelFormat);
		/* Only advertise RAW support if RAW16 is possible. */
		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&
		    info.bitsPerPixel == 16) {
			rawStreamAvailable = true;
			capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);
		}
	}

There's no way to capture this information in the static metadata
otherwise, and I don't think it's a good idea to feed arbitrary
variables into the capabilities gatherer, since at that point we might
as well keep the method that I had earlier (which has the benefit of not
having to walk the static metadata again after setting it, but hey, a
multiple of linear time is still linear time :D).

So yeah, we allow static metadata setters to directly set the
capabilities if it alone is sufficient and necessary to determine some
capability.


Paul

> +	std::vector<camera_metadata_enum_android_request_available_capabilities>
> +		capsVec = std::vector<camera_metadata_enum_android_request_available_capabilities>(capabilities.begin(), capabilities.end());
> +	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES, capsVec);
> +
> +	camera_metadata_enum_android_info_supported_hardware_level hwLevel =
> +		computeHwLevel(staticMetadata_.get(), capabilities);
> +	staticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL, hwLevel);
>  
>  	LOG(HAL, Info)
>  		<< "Hardware level: "
> -- 
> 2.27.0
>
Laurent Pinchart July 11, 2021, 8:50 p.m. UTC | #2 
Hi Paul,

Thank you for the patch.

On Fri, Jul 09, 2021 at 08:34:46PM +0900, paul.elder@ideasonboard.com wrote:
> On Fri, Jul 09, 2021 at 08:28:14PM +0900, Paul Elder wrote:
> > Implement capability and hardware level detection based on the static
> > metadata that has been set, instead of disabling them as requirements
> > are not met. This results in cleaner code where the static metadata is
> > set.
> > 
> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > 
> > ---
> > This currently does not work as the ControlInfo constructor for Spans is
> > broken. This is more of an RFC for Jacopo to check that this is the
> > direction that he wants.
> > 
> > Clearly this is not meant to be merged, as the actual capability
> > requirements are not fully specified yet. Plus they belong in separate
> > patches anyway.
> > 
> > This patch does not apply either, as it depends on many unreleased
> > patches.

That's quite a long disclaimers list :-)

> > ---
> >  src/android/camera_capabilities.cpp | 219 ++++++++++++++++++++--------
> >  1 file changed, 157 insertions(+), 62 deletions(-)
> > 
> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > index 83c7f0d0..ceb5cfe8 100644
> > --- a/src/android/camera_capabilities.cpp
> > +++ b/src/android/camera_capabilities.cpp
> > @@ -262,6 +262,149 @@ std::vector<T> setMetadata(CameraMetadata *metadata, uint32_t tag,
> >  	return ret;
> >  }
> >  
> > +
> > +template<typename T>
> > +bool metadataContains(camera_metadata_ro_entry_t &entry, T value);
> > +
> > +template<>
> > +bool metadataContains<uint8_t>(camera_metadata_ro_entry_t &entry, uint8_t value)
> > +{
> > +	for (unsigned int i = 0; i < entry.count; i++)
> > +		if (entry.data.u8[i] == value)
> > +			return true;
> > +
> > +	return false;
> > +}

This function could also be moved to the CameraMetadata class (named
entryContains() or something similar), and include the getEntry() call.

> > +
> > +bool validateManualSensorCapability(CameraMetadata *staticMetadata)
> > +{
> > +	camera_metadata_ro_entry_t entry;
> > +	bool found;
> > +
> > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,
> > +					 &entry);
> > +	if (!found || !metadataContains<uint8_t>(entry, ANDROID_CONTROL_AE_MODE_OFF)) {
> > +		LOG(HAL, Info)

Should this be downgraded to Debug ? An overall Info message that prints
the hardware level is good, but maybe detailed messages would be too
verbose. Or, as we only print a message on the first missing metadata
entry, would something along the lines of

	"Manual sensor capability unvailable: missing AE mode OFF"

be good to include as an Info message ?

> > +			<< "Missing AE mode off: "
> > +			<< (found ? "not supported" : "not found");
> > +		return false;
> > +	}
> > +
> > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> > +					 &entry);
> > +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {
> > +		LOG(HAL, Info) << "Missing AE lock";
> > +		return false;
> > +	}
> > +
> > +	found = staticMetadata->getEntry(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
> > +					 &entry);

If you think it could help making code more readable, the CameraMetadata
class could get a hasEntry() function.

> > +	if (!found) {
> > +		LOG(HAL, Info) << "Missing edge modes";
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +bool validateManualPostProcessingCapability(CameraMetadata *staticMetadata)
> > +{
> > +	camera_metadata_ro_entry_t entry;
> > +	bool found;
> > +
> > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_AVAILABLE_MODES,
> > +					 &entry);
> > +	if (!found || !metadataContains<uint8_t>(entry, ANDROID_CONTROL_AWB_MODE_OFF)) {
> > +		LOG(HAL, Info) << "Missing AWB mode off";
> > +		return false;
> > +	}
> > +
> > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> > +					 &entry);
> > +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {
> > +		LOG(HAL, Info) << "Missing AWB lock";
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +bool validateBurstCaptureCapability(CameraMetadata *staticMetadata)
> > +{
> > +	camera_metadata_ro_entry_t entry;
> > +	bool found;
> > +
> > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> > +					 &entry);
> > +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {
> > +		LOG(HAL, Info) << "Missing AE lock";
> > +		return false;
> > +	}
> > +
> > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> > +					 &entry);
> > +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {
> > +		LOG(HAL, Info) << "Missing AWB lock";
> > +		return false;
> > +	}
> > +
> > +	found = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);
> > +	if (!found || *entry.data.i32 < 0 || 4 < *entry.data.i32) {
> > +		LOG(HAL, Info)
> > +			<< "Max sync latency is "
> > +			<< (found ? std::to_string(*entry.data.i32) : "not present");
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +std::set<camera_metadata_enum_android_request_available_capabilities>
> > +computeCapabilities(CameraMetadata *staticMetadata,
> > +		    std::set<camera_metadata_enum_android_request_available_capabilities> &existingCaps)
> > +{
> > +	std::set<camera_metadata_enum_android_request_available_capabilities>
> > +	capabilities = existingCaps;
> > +
> > +	capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE);
> > +
> > +	if (validateManualSensorCapability(staticMetadata))
> > +		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> > +
> > +	if (validateManualPostProcessingCapability(staticMetadata))
> > +		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
> > +
> > +	if (validateBurstCaptureCapability(staticMetadata))
> > +		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> > +
> > +	return capabilities;
> > +}
> > +
> > +camera_metadata_enum_android_info_supported_hardware_level

It's impressive how long those names can be.

It could make sense to store the hardware level in the
CameraCapabilities class, as I expect we'll need to tune the behaviour
of the HAL based on it.

> > +computeHwLevel(CameraMetadata *staticMetadata,
> > +	       std::set<camera_metadata_enum_android_request_available_capabilities> capabilities)
> > +{
> > +	camera_metadata_ro_entry_t entry;
> > +	bool found;
> > +	camera_metadata_enum_android_info_supported_hardware_level
> > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;
> > +
> > +	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR))
> > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > +
> > +	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING))
> > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > +
> > +	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE))
> > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > +
> > +	found = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);
> > +	if (!found || *entry.data.i32 != 0)
> > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > +
> > +	return hwLevel;
> > +}
> > +
> >  } /* namespace */
> >  
> >  int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,
> > @@ -655,17 +798,7 @@ int CameraCapabilities::initializeStaticMetadata()
> >  	};
> >  
> >  	std::set<camera_metadata_enum_android_request_available_capabilities>
> > -	capabilities = {
> > -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE,
> > -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR,
> > -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING,
> > -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE,
> > -	};
> > -
> > -	std::set<camera_metadata_enum_android_info_supported_hardware_level>
> > -	hwLevels = {
> > -		ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL,
> > -	};
> > +	capabilities = {};
> >  
> >  	/* Color correction static metadata. */
> >  	{
> > @@ -692,19 +825,12 @@ int CameraCapabilities::initializeStaticMetadata()
> >  	staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES,
> >  				  aeAvailableAntiBandingModes);
> >  
> > -	std::vector<uint8_t> aeModes = setMetadata<uint8_t, bool>(
> > +	setMetadata<uint8_t, bool>(
> >  		staticMetadata_.get(),
> >  		ANDROID_CONTROL_AE_AVAILABLE_MODES,
> >  		controlsInfo, &controls::AeEnable,
> >  		{ ANDROID_CONTROL_AE_MODE_ON });
> >  
> > -	if (std::find(aeModes.begin(), aeModes.end(),
> > -		      ANDROID_CONTROL_AE_MODE_OFF) == aeModes.end()) {
> > -		LOG(HAL, Info) << "AE cannot be turned off";
> > -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> > -	}
> > -
> >  	int64_t minFrameDurationNsec = -1;
> >  	int64_t maxFrameDurationNsec = -1;
> >  	const auto frameDurationsInfo = controlsInfo.find(&controls::FrameDurationLimits);
> > @@ -791,17 +917,11 @@ int CameraCapabilities::initializeStaticMetadata()
> >  	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_VIDEO_STABILIZATION_MODES,
> >  				  availableStabilizationModes);
> >  
> > -	std::vector<uint8_t> awbModes = setMetadata<uint8_t, int32_t>(
> > +	setMetadata<uint8_t, int32_t>(
> >  		staticMetadata_.get(),
> >  		ANDROID_CONTROL_AWB_AVAILABLE_MODES,
> >  		controlsInfo, &controls::AwbMode,
> >  		{ ANDROID_CONTROL_AWB_MODE_AUTO });
> > -	if (std::find(awbModes.begin(), awbModes.end(),
> > -		      ANDROID_CONTROL_AWB_MODE_OFF) == awbModes.end()) {
> > -		LOG(HAL, Info) << "AWB cannot be turned off";
> > -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
> > -	}
> >  
> >  	std::vector<int32_t> availableMaxRegions = {
> >  		0, 0, 0,
> > @@ -817,29 +937,19 @@ int CameraCapabilities::initializeStaticMetadata()
> >  	staticMetadata_->addEntry(ANDROID_CONTROL_SCENE_MODE_OVERRIDES,
> >  				  sceneModesOverride);
> >  
> > -	uint8_t aeLockAvailable = setMetadata<uint8_t, bool>(
> > +	setMetadata<uint8_t, bool>(
> >  		staticMetadata_.get(),
> >  		ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> >  		controlsInfo, &controls::AeLock,
> >  		ControlRange::Max,
> >  		ANDROID_CONTROL_AE_LOCK_AVAILABLE_FALSE);
> > -	if (aeLockAvailable != ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE) {
> > -		LOG(HAL, Info) << "AE lock is unavailable";
> > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> > -	}
> >  
> > -	uint8_t awbLockAvailable = setMetadata<uint8_t, bool>(
> > +	setMetadata<uint8_t, bool>(
> >  		staticMetadata_.get(),
> >  		ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> >  		controlsInfo, &controls::AwbLock,
> >  		ControlRange::Max,
> >  		ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE);
> > -	if (awbLockAvailable != ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE) {
> > -		LOG(HAL, Info) << "AWB lock is unavailable";
> > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
> > -	}
> >  
> >  	char availableControlModes = ANDROID_CONTROL_MODE_AUTO;
> >  	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,
> > @@ -859,10 +969,6 @@ int CameraCapabilities::initializeStaticMetadata()
> >  		availableCharacteristicsKeys_.insert(ANDROID_EDGE_AVAILABLE_EDGE_MODES);
> >  		availableRequestKeys_.insert(ANDROID_EDGE_MODE);
> >  		availableResultKeys_.insert(ANDROID_EDGE_MODE);
> > -	} else {
> > -		LOG(HAL, Info) << "Edge mode unavailable";
> > -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> >  	}
> >  
> >  	/* JPEG static metadata. */
> > @@ -1046,17 +1152,11 @@ int CameraCapabilities::initializeStaticMetadata()
> >  	}
> >  
> >  	/* Sync static metadata. */
> > -	int32_t maxLatency = setMetadata<int32_t, int32_t>(
> > +	setMetadata<int32_t, int32_t>(
> >  		staticMetadata_.get(), ANDROID_SYNC_MAX_LATENCY,
> >  		controlsInfo, &controls::draft::MaxLatency,
> >  		ControlRange::Def,
> >  		ANDROID_SYNC_MAX_LATENCY_UNKNOWN);
> > -	LOG(HAL, Info) << "Max sync latency is " << maxLatency;
> > -	/* CTS allows a sync latency of up to 4 for burst capture capability */
> > -	if (maxLatency < 0 || 4 < maxLatency)
> > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> > -	if (maxLatency != 0)
> > -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> >  
> >  	/* Flash static metadata. */
> >  	char flashAvailable = ANDROID_FLASH_INFO_AVAILABLE_FALSE;
> > @@ -1220,19 +1320,14 @@ int CameraCapabilities::initializeStaticMetadata()
> >  				  numOutStreams);
> >  
> >  	/* Check capabilities */
> > -	std::vector<uint8_t> availableCapabilities(capabilities.begin(),
> > -						   capabilities.end());
> > -	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
> > -				  availableCapabilities);
> > -
> > -	uint8_t hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > -	if (capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR) &&
> > -	    capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING) &&
> > -	    capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE) &&
> > -	    hwLevels.count(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL))
> > -		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;
> > -	staticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,
> > -				  hwLevel);
> > +	capabilities = computeCapabilities(staticMetadata_.get(), capabilities);
> 
> Just to clarify, this is here because above this we have:
> 
> 	/* Report if camera supports RAW. */
> 	bool rawStreamAvailable = false;
> 	std::unique_ptr<CameraConfiguration> cameraConfig =
> 		camera_->generateConfiguration({ StreamRole::Raw });
> 	if (cameraConfig && !cameraConfig->empty()) {
> 		const PixelFormatInfo &info =
> 			PixelFormatInfo::info(cameraConfig->at(0).pixelFormat);
> 		/* Only advertise RAW support if RAW16 is possible. */
> 		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&
> 		    info.bitsPerPixel == 16) {
> 			rawStreamAvailable = true;
> 			capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);
> 		}
> 	}
> 
> There's no way to capture this information in the static metadata
> otherwise, and I don't think it's a good idea to feed arbitrary
> variables into the capabilities gatherer, since at that point we might
> as well keep the method that I had earlier (which has the benefit of not
> having to walk the static metadata again after setting it, but hey, a
> multiple of linear time is still linear time :D).
> 
> So yeah, we allow static metadata setters to directly set the
> capabilities if it alone is sufficient and necessary to determine some
> capability.

This looks fine to me. I'd have a slight preference for calling
computeCapabilities() first, and then extending it with
ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW (and possibly other
capabilities), but if that causes issue, I'm OK with the current order.

> > +	std::vector<camera_metadata_enum_android_request_available_capabilities>
> > +		capsVec = std::vector<camera_metadata_enum_android_request_available_capabilities>(capabilities.begin(), capabilities.end());
> > +	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES, capsVec);
> > +
> > +	camera_metadata_enum_android_info_supported_hardware_level hwLevel =
> > +		computeHwLevel(staticMetadata_.get(), capabilities);
> > +	staticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL, hwLevel);
> >  
> >  	LOG(HAL, Info)
> >  		<< "Hardware level: "
Jacopo Mondi July 15, 2021, 8:44 a.m. UTC | #3 
Hi Paul,
   I'm so sorry for getting that late to this one

On Sun, Jul 11, 2021 at 11:50:44PM +0300, Laurent Pinchart wrote:
> Hi Paul,
>
> Thank you for the patch.
>
> On Fri, Jul 09, 2021 at 08:34:46PM +0900, paul.elder@ideasonboard.com wrote:
> > On Fri, Jul 09, 2021 at 08:28:14PM +0900, Paul Elder wrote:
> > > Implement capability and hardware level detection based on the static
> > > metadata that has been set, instead of disabling them as requirements
> > > are not met. This results in cleaner code where the static metadata is
> > > set.
> > >
> > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > >
> > > ---
> > > This currently does not work as the ControlInfo constructor for Spans is
> > > broken. This is more of an RFC for Jacopo to check that this is the
> > > direction that he wants.

Thanks, this goes exactly in the direction I asked for

> > >
> > > Clearly this is not meant to be merged, as the actual capability
> > > requirements are not fully specified yet. Plus they belong in separate
> > > patches anyway.
> > >
> > > This patch does not apply either, as it depends on many unreleased
> > > patches.
>
> That's quite a long disclaimers list :-)
>
> > > ---
> > >  src/android/camera_capabilities.cpp | 219 ++++++++++++++++++++--------
> > >  1 file changed, 157 insertions(+), 62 deletions(-)
> > >
> > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > > index 83c7f0d0..ceb5cfe8 100644
> > > --- a/src/android/camera_capabilities.cpp
> > > +++ b/src/android/camera_capabilities.cpp
> > > @@ -262,6 +262,149 @@ std::vector<T> setMetadata(CameraMetadata *metadata, uint32_t tag,
> > >  	return ret;
> > >  }
> > >
> > > +
> > > +template<typename T>
> > > +bool metadataContains(camera_metadata_ro_entry_t &entry, T value);
> > > +
> > > +template<>
> > > +bool metadataContains<uint8_t>(camera_metadata_ro_entry_t &entry, uint8_t value)
> > > +{
> > > +	for (unsigned int i = 0; i < entry.count; i++)
> > > +		if (entry.data.u8[i] == value)
> > > +			return true;
> > > +
> > > +	return false;
> > > +}
>
> This function could also be moved to the CameraMetadata class (named
> entryContains() or something similar), and include the getEntry() call.
>
> > > +
> > > +bool validateManualSensorCapability(CameraMetadata *staticMetadata)
> > > +{
> > > +	camera_metadata_ro_entry_t entry;
> > > +	bool found;
> > > +
> > > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,
> > > +					 &entry);
> > > +	if (!found || !metadataContains<uint8_t>(entry, ANDROID_CONTROL_AE_MODE_OFF)) {
> > > +		LOG(HAL, Info)
>
> Should this be downgraded to Debug ? An overall Info message that prints
> the hardware level is good, but maybe detailed messages would be too
> verbose. Or, as we only print a message on the first missing metadata
> entry, would something along the lines of
>
> 	"Manual sensor capability unvailable: missing AE mode OFF"
>
> be good to include as an Info message ?
>
> > > +			<< "Missing AE mode off: "
> > > +			<< (found ? "not supported" : "not found");
> > > +		return false;
> > > +	}
> > > +
> > > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> > > +					 &entry);
> > > +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {
> > > +		LOG(HAL, Info) << "Missing AE lock";
> > > +		return false;
> > > +	}
> > > +
> > > +	found = staticMetadata->getEntry(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
> > > +					 &entry);
>
> If you think it could help making code more readable, the CameraMetadata
> class could get a hasEntry() function.
>
> > > +	if (!found) {
> > > +		LOG(HAL, Info) << "Missing edge modes";
> > > +		return false;
> > > +	}
> > > +
> > > +	return true;
> > > +}
> > > +
> > > +bool validateManualPostProcessingCapability(CameraMetadata *staticMetadata)
> > > +{
> > > +	camera_metadata_ro_entry_t entry;
> > > +	bool found;
> > > +
> > > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_AVAILABLE_MODES,
> > > +					 &entry);
> > > +	if (!found || !metadataContains<uint8_t>(entry, ANDROID_CONTROL_AWB_MODE_OFF)) {
> > > +		LOG(HAL, Info) << "Missing AWB mode off";
> > > +		return false;
> > > +	}
> > > +
> > > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> > > +					 &entry);
> > > +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {
> > > +		LOG(HAL, Info) << "Missing AWB lock";
> > > +		return false;
> > > +	}
> > > +
> > > +	return true;
> > > +}
> > > +
> > > +bool validateBurstCaptureCapability(CameraMetadata *staticMetadata)
> > > +{
> > > +	camera_metadata_ro_entry_t entry;
> > > +	bool found;
> > > +
> > > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> > > +					 &entry);
> > > +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {
> > > +		LOG(HAL, Info) << "Missing AE lock";
> > > +		return false;
> > > +	}
> > > +
> > > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> > > +					 &entry);
> > > +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {
> > > +		LOG(HAL, Info) << "Missing AWB lock";
> > > +		return false;
> > > +	}
> > > +
> > > +	found = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);
> > > +	if (!found || *entry.data.i32 < 0 || 4 < *entry.data.i32) {
> > > +		LOG(HAL, Info)
> > > +			<< "Max sync latency is "
> > > +			<< (found ? std::to_string(*entry.data.i32) : "not present");
> > > +		return false;
> > > +	}
> > > +
> > > +	return true;
> > > +}
> > > +
> > > +std::set<camera_metadata_enum_android_request_available_capabilities>
> > > +computeCapabilities(CameraMetadata *staticMetadata,
> > > +		    std::set<camera_metadata_enum_android_request_available_capabilities> &existingCaps)
> > > +{
> > > +	std::set<camera_metadata_enum_android_request_available_capabilities>
> > > +	capabilities = existingCaps;
> > > +
> > > +	capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE);
> > > +
> > > +	if (validateManualSensorCapability(staticMetadata))
> > > +		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> > > +
> > > +	if (validateManualPostProcessingCapability(staticMetadata))
> > > +		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
> > > +
> > > +	if (validateBurstCaptureCapability(staticMetadata))
> > > +		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> > > +
> > > +	return capabilities;
> > > +}
> > > +
> > > +camera_metadata_enum_android_info_supported_hardware_level
>
> It's impressive how long those names can be.
>
> It could make sense to store the hardware level in the
> CameraCapabilities class, as I expect we'll need to tune the behaviour
> of the HAL based on it.
>
> > > +computeHwLevel(CameraMetadata *staticMetadata,
> > > +	       std::set<camera_metadata_enum_android_request_available_capabilities> capabilities)
> > > +{
> > > +	camera_metadata_ro_entry_t entry;
> > > +	bool found;
> > > +	camera_metadata_enum_android_info_supported_hardware_level
> > > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;
> > > +
> > > +	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR))
> > > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > > +
> > > +	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING))
> > > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > > +
> > > +	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE))
> > > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > > +
> > > +	found = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);
> > > +	if (!found || *entry.data.i32 != 0)
> > > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > > +
> > > +	return hwLevel;
> > > +}
> > > +
> > >  } /* namespace */
> > >
> > >  int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,
> > > @@ -655,17 +798,7 @@ int CameraCapabilities::initializeStaticMetadata()
> > >  	};
> > >
> > >  	std::set<camera_metadata_enum_android_request_available_capabilities>
> > > -	capabilities = {
> > > -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE,
> > > -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR,
> > > -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING,
> > > -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE,
> > > -	};
> > > -
> > > -	std::set<camera_metadata_enum_android_info_supported_hardware_level>
> > > -	hwLevels = {
> > > -		ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL,
> > > -	};
> > > +	capabilities = {};
> > >
> > >  	/* Color correction static metadata. */
> > >  	{
> > > @@ -692,19 +825,12 @@ int CameraCapabilities::initializeStaticMetadata()
> > >  	staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES,
> > >  				  aeAvailableAntiBandingModes);
> > >
> > > -	std::vector<uint8_t> aeModes = setMetadata<uint8_t, bool>(
> > > +	setMetadata<uint8_t, bool>(
> > >  		staticMetadata_.get(),
> > >  		ANDROID_CONTROL_AE_AVAILABLE_MODES,
> > >  		controlsInfo, &controls::AeEnable,
> > >  		{ ANDROID_CONTROL_AE_MODE_ON });
> > >
> > > -	if (std::find(aeModes.begin(), aeModes.end(),
> > > -		      ANDROID_CONTROL_AE_MODE_OFF) == aeModes.end()) {
> > > -		LOG(HAL, Info) << "AE cannot be turned off";
> > > -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> > > -	}
> > > -
> > >  	int64_t minFrameDurationNsec = -1;
> > >  	int64_t maxFrameDurationNsec = -1;
> > >  	const auto frameDurationsInfo = controlsInfo.find(&controls::FrameDurationLimits);
> > > @@ -791,17 +917,11 @@ int CameraCapabilities::initializeStaticMetadata()
> > >  	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_VIDEO_STABILIZATION_MODES,
> > >  				  availableStabilizationModes);
> > >
> > > -	std::vector<uint8_t> awbModes = setMetadata<uint8_t, int32_t>(
> > > +	setMetadata<uint8_t, int32_t>(
> > >  		staticMetadata_.get(),
> > >  		ANDROID_CONTROL_AWB_AVAILABLE_MODES,
> > >  		controlsInfo, &controls::AwbMode,
> > >  		{ ANDROID_CONTROL_AWB_MODE_AUTO });
> > > -	if (std::find(awbModes.begin(), awbModes.end(),
> > > -		      ANDROID_CONTROL_AWB_MODE_OFF) == awbModes.end()) {
> > > -		LOG(HAL, Info) << "AWB cannot be turned off";
> > > -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
> > > -	}
> > >
> > >  	std::vector<int32_t> availableMaxRegions = {
> > >  		0, 0, 0,
> > > @@ -817,29 +937,19 @@ int CameraCapabilities::initializeStaticMetadata()
> > >  	staticMetadata_->addEntry(ANDROID_CONTROL_SCENE_MODE_OVERRIDES,
> > >  				  sceneModesOverride);
> > >
> > > -	uint8_t aeLockAvailable = setMetadata<uint8_t, bool>(
> > > +	setMetadata<uint8_t, bool>(
> > >  		staticMetadata_.get(),
> > >  		ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> > >  		controlsInfo, &controls::AeLock,
> > >  		ControlRange::Max,
> > >  		ANDROID_CONTROL_AE_LOCK_AVAILABLE_FALSE);
> > > -	if (aeLockAvailable != ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE) {
> > > -		LOG(HAL, Info) << "AE lock is unavailable";
> > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> > > -	}
> > >
> > > -	uint8_t awbLockAvailable = setMetadata<uint8_t, bool>(
> > > +	setMetadata<uint8_t, bool>(
> > >  		staticMetadata_.get(),
> > >  		ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> > >  		controlsInfo, &controls::AwbLock,
> > >  		ControlRange::Max,
> > >  		ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE);
> > > -	if (awbLockAvailable != ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE) {
> > > -		LOG(HAL, Info) << "AWB lock is unavailable";
> > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
> > > -	}
> > >
> > >  	char availableControlModes = ANDROID_CONTROL_MODE_AUTO;
> > >  	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,
> > > @@ -859,10 +969,6 @@ int CameraCapabilities::initializeStaticMetadata()
> > >  		availableCharacteristicsKeys_.insert(ANDROID_EDGE_AVAILABLE_EDGE_MODES);
> > >  		availableRequestKeys_.insert(ANDROID_EDGE_MODE);
> > >  		availableResultKeys_.insert(ANDROID_EDGE_MODE);
> > > -	} else {
> > > -		LOG(HAL, Info) << "Edge mode unavailable";
> > > -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> > >  	}
> > >
> > >  	/* JPEG static metadata. */
> > > @@ -1046,17 +1152,11 @@ int CameraCapabilities::initializeStaticMetadata()
> > >  	}
> > >
> > >  	/* Sync static metadata. */
> > > -	int32_t maxLatency = setMetadata<int32_t, int32_t>(
> > > +	setMetadata<int32_t, int32_t>(
> > >  		staticMetadata_.get(), ANDROID_SYNC_MAX_LATENCY,
> > >  		controlsInfo, &controls::draft::MaxLatency,
> > >  		ControlRange::Def,
> > >  		ANDROID_SYNC_MAX_LATENCY_UNKNOWN);
> > > -	LOG(HAL, Info) << "Max sync latency is " << maxLatency;
> > > -	/* CTS allows a sync latency of up to 4 for burst capture capability */
> > > -	if (maxLatency < 0 || 4 < maxLatency)
> > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> > > -	if (maxLatency != 0)
> > > -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> > >
> > >  	/* Flash static metadata. */
> > >  	char flashAvailable = ANDROID_FLASH_INFO_AVAILABLE_FALSE;
> > > @@ -1220,19 +1320,14 @@ int CameraCapabilities::initializeStaticMetadata()
> > >  				  numOutStreams);
> > >
> > >  	/* Check capabilities */
> > > -	std::vector<uint8_t> availableCapabilities(capabilities.begin(),
> > > -						   capabilities.end());
> > > -	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
> > > -				  availableCapabilities);
> > > -
> > > -	uint8_t hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > > -	if (capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR) &&
> > > -	    capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING) &&
> > > -	    capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE) &&
> > > -	    hwLevels.count(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL))
> > > -		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;
> > > -	staticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,
> > > -				  hwLevel);
> > > +	capabilities = computeCapabilities(staticMetadata_.get(), capabilities);

So far I share all comments Laurent made, there's space for
simplifying the implementation, but the direction is good!

> >
> > Just to clarify, this is here because above this we have:
> >
> > 	/* Report if camera supports RAW. */
> > 	bool rawStreamAvailable = false;
> > 	std::unique_ptr<CameraConfiguration> cameraConfig =
> > 		camera_->generateConfiguration({ StreamRole::Raw });
> > 	if (cameraConfig && !cameraConfig->empty()) {
> > 		const PixelFormatInfo &info =
> > 			PixelFormatInfo::info(cameraConfig->at(0).pixelFormat);
> > 		/* Only advertise RAW support if RAW16 is possible. */
> > 		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&
> > 		    info.bitsPerPixel == 16) {
> > 			rawStreamAvailable = true;
> > 			capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);
> > 		}
> > 	}
> >
> > There's no way to capture this information in the static metadata
> > otherwise, and I don't think it's a good idea to feed arbitrary
> > variables into the capabilities gatherer, since at that point we might
> > as well keep the method that I had earlier (which has the benefit of not
> > having to walk the static metadata again after setting it, but hey, a
> > multiple of linear time is still linear time :D).
> >
> > So yeah, we allow static metadata setters to directly set the
> > capabilities if it alone is sufficient and necessary to determine some
> > capability.
>
> This looks fine to me. I'd have a slight preference for calling
> computeCapabilities() first, and then extending it with
> ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW (and possibly other
> capabilities), but if that causes issue, I'm OK with the current order.
>

I have a patch in my backlog that moves RAW capabilities detection to
initializetreamConfigurations() as the check for RAW support was
duplicated. I recorded the RAW support information in a class
variable, and initializeStaticMetadata() inspects that to populate the
capabilities.

Patch has not been sent as it was part of the frameduration work, but
I can send it out with a few related cleanups. In the meantime
https://paste.debian.net/1204421/

Do you think it would be ok for the capabilities checker to inspect a
class variable ?

Thanks
  j

> > > +	std::vector<camera_metadata_enum_android_request_available_capabilities>
> > > +		capsVec = std::vector<camera_metadata_enum_android_request_available_capabilities>(capabilities.begin(), capabilities.end());
> > > +	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES, capsVec);
> > > +
> > > +	camera_metadata_enum_android_info_supported_hardware_level hwLevel =
> > > +		computeHwLevel(staticMetadata_.get(), capabilities);
> > > +	staticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL, hwLevel);
> > >
> > >  	LOG(HAL, Info)
> > >  		<< "Hardware level: "
>
> --
> Regards,
>
> Laurent Pinchart
Paul Elder July 15, 2021, 8:46 a.m. UTC | #4 
Hi Jacopo,

On Thu, Jul 15, 2021 at 10:44:01AM +0200, Jacopo Mondi wrote:
> Hi Paul,
>    I'm so sorry for getting that late to this one
> 
> On Sun, Jul 11, 2021 at 11:50:44PM +0300, Laurent Pinchart wrote:
> > Hi Paul,
> >
> > Thank you for the patch.
> >
> > On Fri, Jul 09, 2021 at 08:34:46PM +0900, paul.elder@ideasonboard.com wrote:
> > > On Fri, Jul 09, 2021 at 08:28:14PM +0900, Paul Elder wrote:
> > > > Implement capability and hardware level detection based on the static
> > > > metadata that has been set, instead of disabling them as requirements
> > > > are not met. This results in cleaner code where the static metadata is
> > > > set.
> > > >
> > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> > > >
> > > > ---
> > > > This currently does not work as the ControlInfo constructor for Spans is
> > > > broken. This is more of an RFC for Jacopo to check that this is the
> > > > direction that he wants.
> 
> Thanks, this goes exactly in the direction I asked for

Phew, that's good. I continue in this direction then.

> 
> > > >
> > > > Clearly this is not meant to be merged, as the actual capability
> > > > requirements are not fully specified yet. Plus they belong in separate
> > > > patches anyway.
> > > >
> > > > This patch does not apply either, as it depends on many unreleased
> > > > patches.
> >
> > That's quite a long disclaimers list :-)
> >
> > > > ---
> > > >  src/android/camera_capabilities.cpp | 219 ++++++++++++++++++++--------
> > > >  1 file changed, 157 insertions(+), 62 deletions(-)
> > > >
> > > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > > > index 83c7f0d0..ceb5cfe8 100644
> > > > --- a/src/android/camera_capabilities.cpp
> > > > +++ b/src/android/camera_capabilities.cpp
> > > > @@ -262,6 +262,149 @@ std::vector<T> setMetadata(CameraMetadata *metadata, uint32_t tag,
> > > >  	return ret;
> > > >  }
> > > >
> > > > +
> > > > +template<typename T>
> > > > +bool metadataContains(camera_metadata_ro_entry_t &entry, T value);
> > > > +
> > > > +template<>
> > > > +bool metadataContains<uint8_t>(camera_metadata_ro_entry_t &entry, uint8_t value)
> > > > +{
> > > > +	for (unsigned int i = 0; i < entry.count; i++)
> > > > +		if (entry.data.u8[i] == value)
> > > > +			return true;
> > > > +
> > > > +	return false;
> > > > +}
> >
> > This function could also be moved to the CameraMetadata class (named
> > entryContains() or something similar), and include the getEntry() call.
> >
> > > > +
> > > > +bool validateManualSensorCapability(CameraMetadata *staticMetadata)
> > > > +{
> > > > +	camera_metadata_ro_entry_t entry;
> > > > +	bool found;
> > > > +
> > > > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,
> > > > +					 &entry);
> > > > +	if (!found || !metadataContains<uint8_t>(entry, ANDROID_CONTROL_AE_MODE_OFF)) {
> > > > +		LOG(HAL, Info)
> >
> > Should this be downgraded to Debug ? An overall Info message that prints
> > the hardware level is good, but maybe detailed messages would be too
> > verbose. Or, as we only print a message on the first missing metadata
> > entry, would something along the lines of
> >
> > 	"Manual sensor capability unvailable: missing AE mode OFF"
> >
> > be good to include as an Info message ?
> >
> > > > +			<< "Missing AE mode off: "
> > > > +			<< (found ? "not supported" : "not found");
> > > > +		return false;
> > > > +	}
> > > > +
> > > > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> > > > +					 &entry);
> > > > +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {
> > > > +		LOG(HAL, Info) << "Missing AE lock";
> > > > +		return false;
> > > > +	}
> > > > +
> > > > +	found = staticMetadata->getEntry(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
> > > > +					 &entry);
> >
> > If you think it could help making code more readable, the CameraMetadata
> > class could get a hasEntry() function.
> >
> > > > +	if (!found) {
> > > > +		LOG(HAL, Info) << "Missing edge modes";
> > > > +		return false;
> > > > +	}
> > > > +
> > > > +	return true;
> > > > +}
> > > > +
> > > > +bool validateManualPostProcessingCapability(CameraMetadata *staticMetadata)
> > > > +{
> > > > +	camera_metadata_ro_entry_t entry;
> > > > +	bool found;
> > > > +
> > > > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_AVAILABLE_MODES,
> > > > +					 &entry);
> > > > +	if (!found || !metadataContains<uint8_t>(entry, ANDROID_CONTROL_AWB_MODE_OFF)) {
> > > > +		LOG(HAL, Info) << "Missing AWB mode off";
> > > > +		return false;
> > > > +	}
> > > > +
> > > > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> > > > +					 &entry);
> > > > +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {
> > > > +		LOG(HAL, Info) << "Missing AWB lock";
> > > > +		return false;
> > > > +	}
> > > > +
> > > > +	return true;
> > > > +}
> > > > +
> > > > +bool validateBurstCaptureCapability(CameraMetadata *staticMetadata)
> > > > +{
> > > > +	camera_metadata_ro_entry_t entry;
> > > > +	bool found;
> > > > +
> > > > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> > > > +					 &entry);
> > > > +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {
> > > > +		LOG(HAL, Info) << "Missing AE lock";
> > > > +		return false;
> > > > +	}
> > > > +
> > > > +	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> > > > +					 &entry);
> > > > +	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {
> > > > +		LOG(HAL, Info) << "Missing AWB lock";
> > > > +		return false;
> > > > +	}
> > > > +
> > > > +	found = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);
> > > > +	if (!found || *entry.data.i32 < 0 || 4 < *entry.data.i32) {
> > > > +		LOG(HAL, Info)
> > > > +			<< "Max sync latency is "
> > > > +			<< (found ? std::to_string(*entry.data.i32) : "not present");
> > > > +		return false;
> > > > +	}
> > > > +
> > > > +	return true;
> > > > +}
> > > > +
> > > > +std::set<camera_metadata_enum_android_request_available_capabilities>
> > > > +computeCapabilities(CameraMetadata *staticMetadata,
> > > > +		    std::set<camera_metadata_enum_android_request_available_capabilities> &existingCaps)
> > > > +{
> > > > +	std::set<camera_metadata_enum_android_request_available_capabilities>
> > > > +	capabilities = existingCaps;
> > > > +
> > > > +	capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE);
> > > > +
> > > > +	if (validateManualSensorCapability(staticMetadata))
> > > > +		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> > > > +
> > > > +	if (validateManualPostProcessingCapability(staticMetadata))
> > > > +		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
> > > > +
> > > > +	if (validateBurstCaptureCapability(staticMetadata))
> > > > +		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> > > > +
> > > > +	return capabilities;
> > > > +}
> > > > +
> > > > +camera_metadata_enum_android_info_supported_hardware_level
> >
> > It's impressive how long those names can be.
> >
> > It could make sense to store the hardware level in the
> > CameraCapabilities class, as I expect we'll need to tune the behaviour
> > of the HAL based on it.
> >
> > > > +computeHwLevel(CameraMetadata *staticMetadata,
> > > > +	       std::set<camera_metadata_enum_android_request_available_capabilities> capabilities)
> > > > +{
> > > > +	camera_metadata_ro_entry_t entry;
> > > > +	bool found;
> > > > +	camera_metadata_enum_android_info_supported_hardware_level
> > > > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;
> > > > +
> > > > +	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR))
> > > > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > > > +
> > > > +	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING))
> > > > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > > > +
> > > > +	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE))
> > > > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > > > +
> > > > +	found = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);
> > > > +	if (!found || *entry.data.i32 != 0)
> > > > +		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > > > +
> > > > +	return hwLevel;
> > > > +}
> > > > +
> > > >  } /* namespace */
> > > >
> > > >  int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,
> > > > @@ -655,17 +798,7 @@ int CameraCapabilities::initializeStaticMetadata()
> > > >  	};
> > > >
> > > >  	std::set<camera_metadata_enum_android_request_available_capabilities>
> > > > -	capabilities = {
> > > > -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE,
> > > > -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR,
> > > > -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING,
> > > > -		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE,
> > > > -	};
> > > > -
> > > > -	std::set<camera_metadata_enum_android_info_supported_hardware_level>
> > > > -	hwLevels = {
> > > > -		ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL,
> > > > -	};
> > > > +	capabilities = {};
> > > >
> > > >  	/* Color correction static metadata. */
> > > >  	{
> > > > @@ -692,19 +825,12 @@ int CameraCapabilities::initializeStaticMetadata()
> > > >  	staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES,
> > > >  				  aeAvailableAntiBandingModes);
> > > >
> > > > -	std::vector<uint8_t> aeModes = setMetadata<uint8_t, bool>(
> > > > +	setMetadata<uint8_t, bool>(
> > > >  		staticMetadata_.get(),
> > > >  		ANDROID_CONTROL_AE_AVAILABLE_MODES,
> > > >  		controlsInfo, &controls::AeEnable,
> > > >  		{ ANDROID_CONTROL_AE_MODE_ON });
> > > >
> > > > -	if (std::find(aeModes.begin(), aeModes.end(),
> > > > -		      ANDROID_CONTROL_AE_MODE_OFF) == aeModes.end()) {
> > > > -		LOG(HAL, Info) << "AE cannot be turned off";
> > > > -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> > > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> > > > -	}
> > > > -
> > > >  	int64_t minFrameDurationNsec = -1;
> > > >  	int64_t maxFrameDurationNsec = -1;
> > > >  	const auto frameDurationsInfo = controlsInfo.find(&controls::FrameDurationLimits);
> > > > @@ -791,17 +917,11 @@ int CameraCapabilities::initializeStaticMetadata()
> > > >  	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_VIDEO_STABILIZATION_MODES,
> > > >  				  availableStabilizationModes);
> > > >
> > > > -	std::vector<uint8_t> awbModes = setMetadata<uint8_t, int32_t>(
> > > > +	setMetadata<uint8_t, int32_t>(
> > > >  		staticMetadata_.get(),
> > > >  		ANDROID_CONTROL_AWB_AVAILABLE_MODES,
> > > >  		controlsInfo, &controls::AwbMode,
> > > >  		{ ANDROID_CONTROL_AWB_MODE_AUTO });
> > > > -	if (std::find(awbModes.begin(), awbModes.end(),
> > > > -		      ANDROID_CONTROL_AWB_MODE_OFF) == awbModes.end()) {
> > > > -		LOG(HAL, Info) << "AWB cannot be turned off";
> > > > -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> > > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
> > > > -	}
> > > >
> > > >  	std::vector<int32_t> availableMaxRegions = {
> > > >  		0, 0, 0,
> > > > @@ -817,29 +937,19 @@ int CameraCapabilities::initializeStaticMetadata()
> > > >  	staticMetadata_->addEntry(ANDROID_CONTROL_SCENE_MODE_OVERRIDES,
> > > >  				  sceneModesOverride);
> > > >
> > > > -	uint8_t aeLockAvailable = setMetadata<uint8_t, bool>(
> > > > +	setMetadata<uint8_t, bool>(
> > > >  		staticMetadata_.get(),
> > > >  		ANDROID_CONTROL_AE_LOCK_AVAILABLE,
> > > >  		controlsInfo, &controls::AeLock,
> > > >  		ControlRange::Max,
> > > >  		ANDROID_CONTROL_AE_LOCK_AVAILABLE_FALSE);
> > > > -	if (aeLockAvailable != ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE) {
> > > > -		LOG(HAL, Info) << "AE lock is unavailable";
> > > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> > > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> > > > -	}
> > > >
> > > > -	uint8_t awbLockAvailable = setMetadata<uint8_t, bool>(
> > > > +	setMetadata<uint8_t, bool>(
> > > >  		staticMetadata_.get(),
> > > >  		ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
> > > >  		controlsInfo, &controls::AwbLock,
> > > >  		ControlRange::Max,
> > > >  		ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE);
> > > > -	if (awbLockAvailable != ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE) {
> > > > -		LOG(HAL, Info) << "AWB lock is unavailable";
> > > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> > > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
> > > > -	}
> > > >
> > > >  	char availableControlModes = ANDROID_CONTROL_MODE_AUTO;
> > > >  	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,
> > > > @@ -859,10 +969,6 @@ int CameraCapabilities::initializeStaticMetadata()
> > > >  		availableCharacteristicsKeys_.insert(ANDROID_EDGE_AVAILABLE_EDGE_MODES);
> > > >  		availableRequestKeys_.insert(ANDROID_EDGE_MODE);
> > > >  		availableResultKeys_.insert(ANDROID_EDGE_MODE);
> > > > -	} else {
> > > > -		LOG(HAL, Info) << "Edge mode unavailable";
> > > > -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> > > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
> > > >  	}
> > > >
> > > >  	/* JPEG static metadata. */
> > > > @@ -1046,17 +1152,11 @@ int CameraCapabilities::initializeStaticMetadata()
> > > >  	}
> > > >
> > > >  	/* Sync static metadata. */
> > > > -	int32_t maxLatency = setMetadata<int32_t, int32_t>(
> > > > +	setMetadata<int32_t, int32_t>(
> > > >  		staticMetadata_.get(), ANDROID_SYNC_MAX_LATENCY,
> > > >  		controlsInfo, &controls::draft::MaxLatency,
> > > >  		ControlRange::Def,
> > > >  		ANDROID_SYNC_MAX_LATENCY_UNKNOWN);
> > > > -	LOG(HAL, Info) << "Max sync latency is " << maxLatency;
> > > > -	/* CTS allows a sync latency of up to 4 for burst capture capability */
> > > > -	if (maxLatency < 0 || 4 < maxLatency)
> > > > -		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
> > > > -	if (maxLatency != 0)
> > > > -		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
> > > >
> > > >  	/* Flash static metadata. */
> > > >  	char flashAvailable = ANDROID_FLASH_INFO_AVAILABLE_FALSE;
> > > > @@ -1220,19 +1320,14 @@ int CameraCapabilities::initializeStaticMetadata()
> > > >  				  numOutStreams);
> > > >
> > > >  	/* Check capabilities */
> > > > -	std::vector<uint8_t> availableCapabilities(capabilities.begin(),
> > > > -						   capabilities.end());
> > > > -	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
> > > > -				  availableCapabilities);
> > > > -
> > > > -	uint8_t hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> > > > -	if (capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR) &&
> > > > -	    capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING) &&
> > > > -	    capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE) &&
> > > > -	    hwLevels.count(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL))
> > > > -		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;
> > > > -	staticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,
> > > > -				  hwLevel);
> > > > +	capabilities = computeCapabilities(staticMetadata_.get(), capabilities);
> 
> So far I share all comments Laurent made, there's space for
> simplifying the implementation, but the direction is good!
> 
> > >
> > > Just to clarify, this is here because above this we have:
> > >
> > > 	/* Report if camera supports RAW. */
> > > 	bool rawStreamAvailable = false;
> > > 	std::unique_ptr<CameraConfiguration> cameraConfig =
> > > 		camera_->generateConfiguration({ StreamRole::Raw });
> > > 	if (cameraConfig && !cameraConfig->empty()) {
> > > 		const PixelFormatInfo &info =
> > > 			PixelFormatInfo::info(cameraConfig->at(0).pixelFormat);
> > > 		/* Only advertise RAW support if RAW16 is possible. */
> > > 		if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW &&
> > > 		    info.bitsPerPixel == 16) {
> > > 			rawStreamAvailable = true;
> > > 			capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW);
> > > 		}
> > > 	}
> > >
> > > There's no way to capture this information in the static metadata
> > > otherwise, and I don't think it's a good idea to feed arbitrary
> > > variables into the capabilities gatherer, since at that point we might
> > > as well keep the method that I had earlier (which has the benefit of not
> > > having to walk the static metadata again after setting it, but hey, a
> > > multiple of linear time is still linear time :D).
> > >
> > > So yeah, we allow static metadata setters to directly set the
> > > capabilities if it alone is sufficient and necessary to determine some
> > > capability.
> >
> > This looks fine to me. I'd have a slight preference for calling
> > computeCapabilities() first, and then extending it with
> > ANDROID_REQUEST_AVAILABLE_CAPABILITIES_RAW (and possibly other
> > capabilities), but if that causes issue, I'm OK with the current order.
> >
> 
> I have a patch in my backlog that moves RAW capabilities detection to
> initializetreamConfigurations() as the check for RAW support was
> duplicated. I recorded the RAW support information in a class
> variable, and initializeStaticMetadata() inspects that to populate the
> capabilities.
> 
> Patch has not been sent as it was part of the frameduration work, but
> I can send it out with a few related cleanups. In the meantime
> https://paste.debian.net/1204421/
> 
> Do you think it would be ok for the capabilities checker to inspect a
> class variable ?

Oh yeah I think that would be better.


Thanks,

Paul

> 
> > > > +	std::vector<camera_metadata_enum_android_request_available_capabilities>
> > > > +		capsVec = std::vector<camera_metadata_enum_android_request_available_capabilities>(capabilities.begin(), capabilities.end());
> > > > +	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES, capsVec);
> > > > +
> > > > +	camera_metadata_enum_android_info_supported_hardware_level hwLevel =
> > > > +		computeHwLevel(staticMetadata_.get(), capabilities);
> > > > +	staticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL, hwLevel);
> > > >
> > > >  	LOG(HAL, Info)
> > > >  		<< "Hardware level: "
> >
> > --
> > Regards,
> >
> > Laurent Pinchart

Patch
diff mbox series 

diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
index 83c7f0d0..ceb5cfe8 100644
--- a/src/android/camera_capabilities.cpp
+++ b/src/android/camera_capabilities.cpp
@@ -262,6 +262,149 @@  std::vector<T> setMetadata(CameraMetadata *metadata, uint32_t tag,
 	return ret;
 }
 
+
+template<typename T>
+bool metadataContains(camera_metadata_ro_entry_t &entry, T value);
+
+template<>
+bool metadataContains<uint8_t>(camera_metadata_ro_entry_t &entry, uint8_t value)
+{
+	for (unsigned int i = 0; i < entry.count; i++)
+		if (entry.data.u8[i] == value)
+			return true;
+
+	return false;
+}
+
+bool validateManualSensorCapability(CameraMetadata *staticMetadata)
+{
+	camera_metadata_ro_entry_t entry;
+	bool found;
+
+	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_AVAILABLE_MODES,
+					 &entry);
+	if (!found || !metadataContains<uint8_t>(entry, ANDROID_CONTROL_AE_MODE_OFF)) {
+		LOG(HAL, Info)
+			<< "Missing AE mode off: "
+			<< (found ? "not supported" : "not found");
+		return false;
+	}
+
+	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
+					 &entry);
+	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {
+		LOG(HAL, Info) << "Missing AE lock";
+		return false;
+	}
+
+	found = staticMetadata->getEntry(ANDROID_EDGE_AVAILABLE_EDGE_MODES,
+					 &entry);
+	if (!found) {
+		LOG(HAL, Info) << "Missing edge modes";
+		return false;
+	}
+
+	return true;
+}
+
+bool validateManualPostProcessingCapability(CameraMetadata *staticMetadata)
+{
+	camera_metadata_ro_entry_t entry;
+	bool found;
+
+	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_AVAILABLE_MODES,
+					 &entry);
+	if (!found || !metadataContains<uint8_t>(entry, ANDROID_CONTROL_AWB_MODE_OFF)) {
+		LOG(HAL, Info) << "Missing AWB mode off";
+		return false;
+	}
+
+	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
+					 &entry);
+	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {
+		LOG(HAL, Info) << "Missing AWB lock";
+		return false;
+	}
+
+	return true;
+}
+
+bool validateBurstCaptureCapability(CameraMetadata *staticMetadata)
+{
+	camera_metadata_ro_entry_t entry;
+	bool found;
+
+	found = staticMetadata->getEntry(ANDROID_CONTROL_AE_LOCK_AVAILABLE,
+					 &entry);
+	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE)) {
+		LOG(HAL, Info) << "Missing AE lock";
+		return false;
+	}
+
+	found = staticMetadata->getEntry(ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
+					 &entry);
+	if (!found || !(*entry.data.u8 == ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE)) {
+		LOG(HAL, Info) << "Missing AWB lock";
+		return false;
+	}
+
+	found = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);
+	if (!found || *entry.data.i32 < 0 || 4 < *entry.data.i32) {
+		LOG(HAL, Info)
+			<< "Max sync latency is "
+			<< (found ? std::to_string(*entry.data.i32) : "not present");
+		return false;
+	}
+
+	return true;
+}
+
+std::set<camera_metadata_enum_android_request_available_capabilities>
+computeCapabilities(CameraMetadata *staticMetadata,
+		    std::set<camera_metadata_enum_android_request_available_capabilities> &existingCaps)
+{
+	std::set<camera_metadata_enum_android_request_available_capabilities>
+	capabilities = existingCaps;
+
+	capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE);
+
+	if (validateManualSensorCapability(staticMetadata))
+		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
+
+	if (validateManualPostProcessingCapability(staticMetadata))
+		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
+
+	if (validateBurstCaptureCapability(staticMetadata))
+		capabilities.insert(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
+
+	return capabilities;
+}
+
+camera_metadata_enum_android_info_supported_hardware_level
+computeHwLevel(CameraMetadata *staticMetadata,
+	       std::set<camera_metadata_enum_android_request_available_capabilities> capabilities)
+{
+	camera_metadata_ro_entry_t entry;
+	bool found;
+	camera_metadata_enum_android_info_supported_hardware_level
+		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;
+
+	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR))
+		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
+
+	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING))
+		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
+
+	if (!capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE))
+		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
+
+	found = staticMetadata->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry);
+	if (!found || *entry.data.i32 != 0)
+		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
+
+	return hwLevel;
+}
+
 } /* namespace */
 
 int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,
@@ -655,17 +798,7 @@  int CameraCapabilities::initializeStaticMetadata()
 	};
 
 	std::set<camera_metadata_enum_android_request_available_capabilities>
-	capabilities = {
-		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE,
-		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR,
-		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING,
-		ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE,
-	};
-
-	std::set<camera_metadata_enum_android_info_supported_hardware_level>
-	hwLevels = {
-		ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL,
-	};
+	capabilities = {};
 
 	/* Color correction static metadata. */
 	{
@@ -692,19 +825,12 @@  int CameraCapabilities::initializeStaticMetadata()
 	staticMetadata_->addEntry(ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES,
 				  aeAvailableAntiBandingModes);
 
-	std::vector<uint8_t> aeModes = setMetadata<uint8_t, bool>(
+	setMetadata<uint8_t, bool>(
 		staticMetadata_.get(),
 		ANDROID_CONTROL_AE_AVAILABLE_MODES,
 		controlsInfo, &controls::AeEnable,
 		{ ANDROID_CONTROL_AE_MODE_ON });
 
-	if (std::find(aeModes.begin(), aeModes.end(),
-		      ANDROID_CONTROL_AE_MODE_OFF) == aeModes.end()) {
-		LOG(HAL, Info) << "AE cannot be turned off";
-		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
-		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
-	}
-
 	int64_t minFrameDurationNsec = -1;
 	int64_t maxFrameDurationNsec = -1;
 	const auto frameDurationsInfo = controlsInfo.find(&controls::FrameDurationLimits);
@@ -791,17 +917,11 @@  int CameraCapabilities::initializeStaticMetadata()
 	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_VIDEO_STABILIZATION_MODES,
 				  availableStabilizationModes);
 
-	std::vector<uint8_t> awbModes = setMetadata<uint8_t, int32_t>(
+	setMetadata<uint8_t, int32_t>(
 		staticMetadata_.get(),
 		ANDROID_CONTROL_AWB_AVAILABLE_MODES,
 		controlsInfo, &controls::AwbMode,
 		{ ANDROID_CONTROL_AWB_MODE_AUTO });
-	if (std::find(awbModes.begin(), awbModes.end(),
-		      ANDROID_CONTROL_AWB_MODE_OFF) == awbModes.end()) {
-		LOG(HAL, Info) << "AWB cannot be turned off";
-		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
-		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
-	}
 
 	std::vector<int32_t> availableMaxRegions = {
 		0, 0, 0,
@@ -817,29 +937,19 @@  int CameraCapabilities::initializeStaticMetadata()
 	staticMetadata_->addEntry(ANDROID_CONTROL_SCENE_MODE_OVERRIDES,
 				  sceneModesOverride);
 
-	uint8_t aeLockAvailable = setMetadata<uint8_t, bool>(
+	setMetadata<uint8_t, bool>(
 		staticMetadata_.get(),
 		ANDROID_CONTROL_AE_LOCK_AVAILABLE,
 		controlsInfo, &controls::AeLock,
 		ControlRange::Max,
 		ANDROID_CONTROL_AE_LOCK_AVAILABLE_FALSE);
-	if (aeLockAvailable != ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE) {
-		LOG(HAL, Info) << "AE lock is unavailable";
-		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
-		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
-	}
 
-	uint8_t awbLockAvailable = setMetadata<uint8_t, bool>(
+	setMetadata<uint8_t, bool>(
 		staticMetadata_.get(),
 		ANDROID_CONTROL_AWB_LOCK_AVAILABLE,
 		controlsInfo, &controls::AwbLock,
 		ControlRange::Max,
 		ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE);
-	if (awbLockAvailable != ANDROID_CONTROL_AWB_LOCK_AVAILABLE_TRUE) {
-		LOG(HAL, Info) << "AWB lock is unavailable";
-		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
-		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING);
-	}
 
 	char availableControlModes = ANDROID_CONTROL_MODE_AUTO;
 	staticMetadata_->addEntry(ANDROID_CONTROL_AVAILABLE_MODES,
@@ -859,10 +969,6 @@  int CameraCapabilities::initializeStaticMetadata()
 		availableCharacteristicsKeys_.insert(ANDROID_EDGE_AVAILABLE_EDGE_MODES);
 		availableRequestKeys_.insert(ANDROID_EDGE_MODE);
 		availableResultKeys_.insert(ANDROID_EDGE_MODE);
-	} else {
-		LOG(HAL, Info) << "Edge mode unavailable";
-		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
-		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR);
 	}
 
 	/* JPEG static metadata. */
@@ -1046,17 +1152,11 @@  int CameraCapabilities::initializeStaticMetadata()
 	}
 
 	/* Sync static metadata. */
-	int32_t maxLatency = setMetadata<int32_t, int32_t>(
+	setMetadata<int32_t, int32_t>(
 		staticMetadata_.get(), ANDROID_SYNC_MAX_LATENCY,
 		controlsInfo, &controls::draft::MaxLatency,
 		ControlRange::Def,
 		ANDROID_SYNC_MAX_LATENCY_UNKNOWN);
-	LOG(HAL, Info) << "Max sync latency is " << maxLatency;
-	/* CTS allows a sync latency of up to 4 for burst capture capability */
-	if (maxLatency < 0 || 4 < maxLatency)
-		capabilities.erase(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE);
-	if (maxLatency != 0)
-		hwLevels.erase(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL);
 
 	/* Flash static metadata. */
 	char flashAvailable = ANDROID_FLASH_INFO_AVAILABLE_FALSE;
@@ -1220,19 +1320,14 @@  int CameraCapabilities::initializeStaticMetadata()
 				  numOutStreams);
 
 	/* Check capabilities */
-	std::vector<uint8_t> availableCapabilities(capabilities.begin(),
-						   capabilities.end());
-	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES,
-				  availableCapabilities);
-
-	uint8_t hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
-	if (capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_SENSOR) &&
-	    capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_MANUAL_POST_PROCESSING) &&
-	    capabilities.count(ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BURST_CAPTURE) &&
-	    hwLevels.count(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL))
-		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL;
-	staticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,
-				  hwLevel);
+	capabilities = computeCapabilities(staticMetadata_.get(), capabilities);
+	std::vector<camera_metadata_enum_android_request_available_capabilities>
+		capsVec = std::vector<camera_metadata_enum_android_request_available_capabilities>(capabilities.begin(), capabilities.end());
+	staticMetadata_->addEntry(ANDROID_REQUEST_AVAILABLE_CAPABILITIES, capsVec);
+
+	camera_metadata_enum_android_info_supported_hardware_level hwLevel =
+		computeHwLevel(staticMetadata_.get(), capabilities);
+	staticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL, hwLevel);
 
 	LOG(HAL, Info)
 		<< "Hardware level: "