[libcamera-devel] [PATCH v6 6/9] android: Add helpers for setting android metadata from libcamera controls

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Fri, Jul 30, 2021 at 07:35:33PM +0900, Paul Elder wrote:
> Add helpers for setting android metadata from libcamera controls.
> 
> There are two versions, for scalars and collections, both of which take
> a default value to fill in the android control if the libcamera control
> is not found. They both return the value that was set.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> Changes in v6:
> - remove unused scalar-no-default version
> - remove explicit template parameters
>   - infer the template parameters from the Control type
> 
> Changes in v4:
> - remove vector copy from the vector setter
> 
> Changes in v3:
> - setMetadata for collection only works with vectors
> - change enum to enum class
> - add two template parameters for android type and libcamera type
> - add docs
> 
> New in v2
> 
> TODO: make ControlList versions so that we can use them in result
> metadata
> ---
>  src/android/camera_capabilities.cpp | 88 +++++++++++++++++++++++++++++
>  1 file changed, 88 insertions(+)
> 
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index b59a854f..fa701843 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -11,6 +11,7 @@
>  #include <array>
>  #include <cmath>
>  #include <map>
> +#include <type_traits>
>  
>  #include <hardware/camera3.h>
>  
> @@ -125,6 +126,93 @@ hwLevelStrings = {
>  	{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_EXTERNAL, "EXTERNAL" },
>  };
>  
> +enum class ControlRange {
> +	Min,
> +	Def,
> +	Max,
> +};
> +
> +/**
> + * \brief Set android metadata from libcamera ControlInfo or a default value
> + * \tparam T Type of the control in libcamera
> + * \tparam U Type of the control in android

s/control/metadata/
s/android/Android/

> + * \param[in] metadata Android metadata pack to add the control value to
> + * \param[in] tag Android metadata tag
> + * \param[in] controlsInfo libcamera ControlInfoMap from which to find the control info
> + * \param[in] control libcamera ControlId to find from \a controlsInfo
> + * \param[in] controlRange Whether to use the min, def, or max value from the control info
> + * \param[in] defaultValue The value to set in \a metadata if \a control is not found
> + *
> + * Set the android metadata entry in \a metadata with tag \a tag based on the

s/android/Android/

> + * control info found for the libcamera control \a control in the libcamera
> + * ControlInfoMap \a controlsInfo. If no libcamera ControlInfo is found, then
> + * the android metadata entry is set to \a defaultValue.

Here too.

> + *
> + * This function is for scalar values.
> + */
> +template<typename T, typename U>
> +U setMetadata(CameraMetadata *metadata, uint32_t tag,
> +	      const ControlInfoMap &controlsInfo, const Control<T> *control,

As control can't be null, I'd pass it by reference.

> +	      enum ControlRange controlRange, const U defaultValue)
> +{
> +	U ret = defaultValue;

I'd name the variable value instead of ret.

> +
> +	const auto &info = controlsInfo.find(reinterpret_cast<const ControlId *>(control));

You don't need the cast here, Control<T> inherits from ControlId.

> +	if (info != controlsInfo.end()) {
> +		switch (controlRange) {
> +		case ControlRange::Min:
> +			ret = static_cast<U>(info->second.min().get<T>());

As a side effect, however, dropping the cast requires writing this as

			ret = static_cast<U>(info->second.min().template get<T>());

I have no idea why the cast makes a difference. Same below.

All these comments apply to the next function as well.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> +			break;
> +		case ControlRange::Def:
> +			ret = static_cast<U>(info->second.def().get<T>());
> +			break;
> +		case ControlRange::Max:
> +			ret = static_cast<U>(info->second.max().get<T>());
> +			break;
> +		}
> +	}
> +
> +	metadata->addEntry(tag, ret);
> +	return ret;
> +}
> +
> +/**
> + * \brief Set android metadata from libcamera ControlInfo or a default value
> + * \tparam T Type of the control in libcamera
> + * \tparam U Type of the control in android
> + * \param[in] metadata Android metadata pack to add the control value to
> + * \param[in] tag Android metadata tag
> + * \param[in] controlsInfo libcamera ControlInfoMap from which to find the control info
> + * \param[in] control libcamera ControlId to find from \a controlsInfo
> + * \param[in] defaultVector The value to set in \a metadata if \a control is not found
> + *
> + * Set the android metadata entry in \a metadata with tag \a tag based on the
> + * control info found for the libcamera control \a control in the libcamera
> + * ControlInfoMap \a controlsInfo. If no libcamera ControlInfo is found, then
> + * the android metadata entry is set to \a defaultVector.
> + *
> + * This function is for vector values.
> + */
> +template<typename T, typename U>
> +std::vector<U> setMetadata(CameraMetadata *metadata, uint32_t tag,
> +			   const ControlInfoMap &controlsInfo,
> +			   const Control<T> *control,
> +			   const std::vector<U> &defaultVector)
> +{
> +	const auto &info = controlsInfo.find(reinterpret_cast<const ControlId *>(control));
> +	if (info == controlsInfo.end()) {
> +		metadata->addEntry(tag, defaultVector);
> +		return defaultVector;
> +	}
> +
> +	std::vector<U> ret(info->second.values().size());
> +	for (const auto &value : info->second.values())
> +		ret.push_back(static_cast<U>(value.get<T>()));
> +	metadata->addEntry(tag, ret);
> +
> +	return ret;
> +}
> +
>  } /* namespace */
>  
>  bool CameraCapabilities::validateManualSensorCapability()

-- 
Regards,

Laurent Pinchart