| Message ID | 20211123104042.3100902-2-paul.elder@ideasonboard.com |
|---|---|
| State | Superseded |
| Delegated to: | Paul Elder |
| Headers | show |
| Series |
|
| Related | show |
Quoting Paul Elder (2021-11-23 10:40:36) > Print messages when some feature is missing that causes hardware level > FULL to not be supported. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > src/android/camera_capabilities.cpp | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp > index f357902e..8c138df4 100644 > --- a/src/android/camera_capabilities.cpp > +++ b/src/android/camera_capabilities.cpp > @@ -359,6 +359,9 @@ void CameraCapabilities::computeHwLevel( > { > camera_metadata_ro_entry_t entry; > bool found; > + > + const char *noFull = "Hardware level FULL unavailable: "; > + > camera_metadata_enum_android_info_supported_hardware_level > hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL; > > @@ -372,8 +375,10 @@ void CameraCapabilities::computeHwLevel( > hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED; > > found = staticMetadata_->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry); > - if (!found || *entry.data.i32 != 0) > + if (!found || *entry.data.i32 != 0) { > + LOG(HAL, Info) << noFull << "missing or invalid max sync latency"; > hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED; > + } Are the other settings that force _LIMITED affected? Or are the all the 'real' times when _LIMITED is expected. Should it only print if hwLevel is not already set to ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED by one of the statements above? Otherwise if it's a missing feature on a target that expects to be limited it would be noisy. If we always want to print it, I'd move it below the if (!found) to it's own statement otherwise. -- Kieran > > hwLevel_ = hwLevel; > } > -- > 2.27.0 >
Hi Kieran, On Thu, Nov 25, 2021 at 11:01:48AM +0000, Kieran Bingham wrote: > Quoting Paul Elder (2021-11-23 10:40:36) > > Print messages when some feature is missing that causes hardware level > > FULL to not be supported. > > > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > src/android/camera_capabilities.cpp | 7 ++++++- > > 1 file changed, 6 insertions(+), 1 deletion(-) > > > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp > > index f357902e..8c138df4 100644 > > --- a/src/android/camera_capabilities.cpp > > +++ b/src/android/camera_capabilities.cpp > > @@ -359,6 +359,9 @@ void CameraCapabilities::computeHwLevel( > > { > > camera_metadata_ro_entry_t entry; > > bool found; > > + > > + const char *noFull = "Hardware level FULL unavailable: "; > > + > > camera_metadata_enum_android_info_supported_hardware_level > > hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL; > > > > @@ -372,8 +375,10 @@ void CameraCapabilities::computeHwLevel( > > hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED; > > > > found = staticMetadata_->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry); > > - if (!found || *entry.data.i32 != 0) > > + if (!found || *entry.data.i32 != 0) { > > + LOG(HAL, Info) << noFull << "missing or invalid max sync latency"; > > hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED; > > + } > > Are the other settings that force _LIMITED affected? Or are the all the > 'real' times when _LIMITED is expected. The other settings that force LIMITED, at the point in time of this patch, are checking capabilities. The capability validators already print their own messages. In future patches, when we add more non-capability but yes-hardware-level-full checks, they will add prints similar to this one. > > > Should it only print if hwLevel is not already set to > ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED by one of the statements > above? > > Otherwise if it's a missing feature on a target that expects to be > limited it would be noisy. Yeah I realize it would be noisy on a device that expects to be limited. However, we don't have a way to declare that a device is limited. So for example while trying to get a device to support full, one would have to do many re-runs to get the full list of requirements. > > If we always want to print it, I'd move it below the if (!found) to it's > own statement otherwise. So I don't think this, nor printing only if (!hwLevel = LIMITED) are solutions. I think we need a way for a device to declare its desired hardware level. Perhaps the platform config? We can't put it in libcamera. Otherwise we just leave the noisy messages. After all, this is exactly the mechanism where we decide whether our camera is FULL or not. Paul > > > > > > hwLevel_ = hwLevel; > > } > > -- > > 2.27.0 > >
diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp index f357902e..8c138df4 100644 --- a/src/android/camera_capabilities.cpp +++ b/src/android/camera_capabilities.cpp @@ -359,6 +359,9 @@ void CameraCapabilities::computeHwLevel( { camera_metadata_ro_entry_t entry; bool found; + + const char *noFull = "Hardware level FULL unavailable: "; + camera_metadata_enum_android_info_supported_hardware_level hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_FULL; @@ -372,8 +375,10 @@ void CameraCapabilities::computeHwLevel( hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED; found = staticMetadata_->getEntry(ANDROID_SYNC_MAX_LATENCY, &entry); - if (!found || *entry.data.i32 != 0) + if (!found || *entry.data.i32 != 0) { + LOG(HAL, Info) << noFull << "missing or invalid max sync latency"; hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED; + } hwLevel_ = hwLevel; }
Print messages when some feature is missing that causes hardware level FULL to not be supported. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- src/android/camera_capabilities.cpp | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)