[libcamera-devel] [RFC PATCH v3 03/16] android: Add helpers for setting android metadata from libcamera controls

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Fri, Jul 02, 2021 at 07:37:47PM +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. A version for scalars exists for no default, to not set
> the android control at all if it is not found in libcamera. The
> functions take two template parameters, the first for the android type,
> and the second for the libcamera type of the control. They can be
> different, for example, if the former is an enum and the latter is a
> boolean, or if the former is an enum (uint8_t) and the latter is an enum
> (int32_t).
> 
> The versions that take a default value return the value that was set in
> the android metadata.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> 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 | 137 ++++++++++++++++++++++++++++
>  1 file changed, 137 insertions(+)
> 
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 54bd71da..1d4c44ce 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -124,6 +124,143 @@ hwLevelStrings = {
>  	{ ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_EXTERNAL, "EXTERNAL" },
>  };
>  
> +enum class ControlRange {
> +	Min,
> +	Def,
> +	Max,
> +};
> +
> +/**
> + * \brief Set android metadata from libcamera ControlInfo
> + * \tparam T Type of the control in android
> + * \tparam V Type of the control in libcamera
> + * \param[in] metadata Android metadata to add the control value to
> + * \param[in] tag Android metadata tag to add
> + * \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
> + *
> + * 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 function returns without modifying anything.
> + *
> + * This function is for scalar values.
> + */
> +template<typename T,
> +	 std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr,
> +	 typename V>

This really puzzles me. The second template argument has a default
value, but the third doesn't. I thought it would generate a compilation
error. This function doesn't seem to be used, so maybe that's why the
compiler doesn't choke ?

Do you have plans to use this function in the near future ? If not, I'd
drop it for now.

I also agree with Jacopo that enable_if doesn't seem to be needed here,
nor below (the second version of the function has a ControlRange
argument that the third one doesn't have)..

> +void setMetadata(CameraMetadata *metadata, uint32_t tag,
> +		 const ControlInfoMap &controlsInfo, const ControlId *control,
> +		 enum ControlRange controlRange)

Shouldn't these be member functions of CameraMetadata ?

> +{
> +	const auto &info = controlsInfo.find(control);
> +	if (info == controlsInfo.end())
> +		return;
> +
> +	T ret;
> +	switch (controlRange) {
> +	case ControlRange::Min:
> +		ret = info->second.min().get<V>();
> +		break;
> +	case ControlRange::Def:
> +		ret = info->second.def().get<V>();
> +		break;
> +	case ControlRange::Max:
> +		ret = info->second.max().get<V>();
> +		break;
> +	}
> +
> +	metadata->addEntry(tag, ret);
> +	return;
> +}
> +
> +/**
> + * \brief Set android metadata from libcamera ControlInfo or a default value
> + * \tparam T Type of the control in android
> + * \tparam U Type of the control in libcamera

V is missing.

> + * \param[in] metadata Android metadata to add the control value to
> + * \param[in] tag Android metadata tag to add
> + * \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
> + * 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.
> + *
> + * This function is for scalar values.
> + */
> +template<typename T,
> +	 typename U,

Having to specify the libcamera type manually is error-prone. Could we
handle this dynamically, based on control->type() ?

> +	 typename V,
> +	 std::enable_if_t<std::is_arithmetic_v<V>> * = nullptr>
> +T setMetadata(CameraMetadata *metadata, uint32_t tag,
> +	      const ControlInfoMap &controlsInfo, const ControlId *control,
> +	      enum ControlRange controlRange, const V defaultValue)
> +{
> +	T ret = defaultValue;
> +
> +	const auto &info = controlsInfo.find(control);
> +	if (info != controlsInfo.end()) {
> +		switch (controlRange) {
> +		case ControlRange::Min:
> +			ret = info->second.min().get<U>();
> +			break;
> +		case ControlRange::Def:
> +			ret = info->second.def().get<U>();
> +			break;
> +		case ControlRange::Max:
> +			ret = info->second.max().get<U>();
> +			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 android
> + * \tparam V Type of the control in libcamera
> + * \param[in] metadata Android metadata to add the control value to
> + * \param[in] tag Android metadata tag to add
> + * \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 V>
> +std::vector<T> setMetadata(CameraMetadata *metadata, uint32_t tag,
> +			   const ControlInfoMap &controlsInfo,
> +			   const ControlId *control,
> +			   const std::vector<T> &defaultVector)
> +{
> +	std::vector<T> ret = {};
> +
> +	const auto &info = controlsInfo.find(control);
> +	if (info != controlsInfo.end()) {
> +		ret.reserve(info->second.values().size());
> +		for (const auto &value : info->second.values())
> +			ret.push_back(value.get<V>());
> +	} else {
> +		ret = defaultVector;
> +	}
> +
> +	metadata->addEntry(tag, ret);
> +	return ret;
> +}
> +
>  } /* namespace */
>  
>  int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,

-- 
Regards,

Laurent Pinchart