[libcamera-devel] [RFC PATCH] wip: android: capabilities: Capability detection by population
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Jul 11 22:50:44 CEST 2021
Hi Paul,
Thank you for the patch.
On Fri, Jul 09, 2021 at 08:34:46PM +0900, paul.elder at 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 at 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: "
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list