[libcamera-devel] [PATCH v2 1/6] android: camera_capabilities: Add messages for lack of FULL support

[Jacopo Mondi]

Hi Paul,

On Mon, Dec 20, 2021 at 05:26:24PM -0600, Paul Elder wrote:
> Print messages when some feature is missing that causes hardware level
> FULL to not be supported.
>
> Signed-off-by: Paul Elder <paul.elder at 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 c4c26089..6ae2cd4d 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;
> +
You  can drop this empty line. I would move the noFull declaration at
the beginning of the function.

> +	const char *noFull = "Hardware level FULL unavailable: ";
> +

This should be a const std::string ?

However I see other similar construct using a char * already. Should
they be fixed ?

	const char *noMode = "Manual post processing capability 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";

Or you could simply drop noFull and add the string here.

Either ways
Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>

Thanks
   j
>  		hwLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> +	}
>
>  	hwLevel_ = hwLevel;
>  }
> --
> 2.27.0
>