[libcamera-devel,RFC,v3,03/16] android: Add helpers for setting android metadata from libcamera controls
diff mbox series

Message ID 20210702103800.41291-4-paul.elder@ideasonboard.com
State Superseded
Delegated to: Paul Elder
Headers show
Series
  • Preliminary FULL plumbing
Related show

Commit Message

Paul Elder July 2, 2021, 10:37 a.m. UTC
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@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(+)

Comments

Jacopo Mondi July 5, 2021, 3:44 p.m. UTC | #1
Hi Paul,

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@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,
> +};
> +
> +/**

We don't parse the HAL with doxygen so you can spare the additional *
But it's more than fine to use the Doxygen syntax when documenting
things imho, mostly for consistency

> + * \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

s/Android metadata/Android metadata pack/

> + * \param[in] tag Android metadata tag to add

s/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

Set the android metadata \a tag in the \a metadata pack 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,

You should include <type_traits>

Do you need this to solve which overload ? I see three functions here
with the following prototypes

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

T setMetadata(CameraMetadata *metadata, uint32_t tag,
	      const ControlInfoMap &controlsInfo, const ControlId *control,
	      enum ControlRange controlRange, const V defaultValue)

std::vector<T> setMetadata(CameraMetadata *metadata, uint32_t tag,
			   const ControlInfoMap &controlsInfo,
			   const ControlId *control,
			   const std::vector<T> &defaultVector)

There doesn't seem to be any need to distinguish them on the template parameter
type.

> +	 typename V>
> +void setMetadata(CameraMetadata *metadata, uint32_t tag,
> +		 const ControlInfoMap &controlsInfo, const ControlId *control,
> +		 enum ControlRange controlRange)
> +{
> +	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
> + * \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,
> +	 typename V,

Isn't V the same type as T ?

> +	 std::enable_if_t<std::is_arithmetic_v<V>> * = nullptr>

This is probably functionally equivalent for now, but in the previous
version you were checking for the type T to be an arithmetic type.

> +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>

Fits on one line

> +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;

this is a copy that could be avoided by calling addEntry() in each
branch. Something like

	const auto &info = controlsInfo.find(control);
	if (info == controlsInfo.end()) {
                metadata->addEntry(tag, defaultVector);
                return defaultVector;
        }

        std::vector<T> ret(info->second.values().size());
        for (const auto &value : info->second.values())
                ret.push_back(value.get<V>());
        metadata->addEntry(tag, ret);

        return ret;


> +	}
> +
> +	metadata->addEntry(tag, ret);
> +	return ret;
> +}
> +
>  } /* namespace */
>
>  int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,
> --
> 2.27.0
>
Paul Elder July 9, 2021, 4:43 a.m. UTC | #2
Hi Jacopo,

On Mon, Jul 05, 2021 at 05:44:58PM +0200, Jacopo Mondi wrote:
> Hi Paul,
> 
> 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@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,
> > +};
> > +
> > +/**
> 
> We don't parse the HAL with doxygen so you can spare the additional *
> But it's more than fine to use the Doxygen syntax when documenting
> things imho, mostly for consistency
> 
> > + * \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
> 
> s/Android metadata/Android metadata pack/
> 
> > + * \param[in] tag Android metadata tag to add
> 
> s/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
> 
> Set the android metadata \a tag in the \a metadata pack 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,
> 
> You should include <type_traits>
> 
> Do you need this to solve which overload ? I see three functions here
> with the following prototypes
> 
> void setMetadata(CameraMetadata *metadata, uint32_t tag,
> 		 const ControlInfoMap &controlsInfo, const ControlId *control,
> 		 enum ControlRange controlRange)
> 
> T setMetadata(CameraMetadata *metadata, uint32_t tag,
> 	      const ControlInfoMap &controlsInfo, const ControlId *control,
> 	      enum ControlRange controlRange, const V defaultValue)
> 
> std::vector<T> setMetadata(CameraMetadata *metadata, uint32_t tag,
> 			   const ControlInfoMap &controlsInfo,
> 			   const ControlId *control,
> 			   const std::vector<T> &defaultVector)
> 
> There doesn't seem to be any need to distinguish them on the template parameter
> type.

I added the template parameters for consistency. The latter two V and T,
respectively, can be inferred, but the first one cannot. And the first
one cannot be merged into the second (with default value) as then we
have no way of differentiating between "unspecified" and zero.

> 
> > +	 typename V>
> > +void setMetadata(CameraMetadata *metadata, uint32_t tag,
> > +		 const ControlInfoMap &controlsInfo, const ControlId *control,
> > +		 enum ControlRange controlRange)
> > +{
> > +	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
> > + * \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,
> > +	 typename V,
> 
> Isn't V the same type as T ?

It is, but then as I mentioned earlier, this call would then be missing
a template parameter while the other one won't.

> 
> > +	 std::enable_if_t<std::is_arithmetic_v<V>> * = nullptr>
> 
> This is probably functionally equivalent for now, but in the previous
> version you were checking for the type T to be an arithmetic type.

I was wondering if the template parameters should be in alphabetical
order or not...


Thanks,

Paul

> 
> > +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>
> 
> Fits on one line
> 
> > +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;
> 
> this is a copy that could be avoided by calling addEntry() in each
> branch. Something like
> 
> 	const auto &info = controlsInfo.find(control);
> 	if (info == controlsInfo.end()) {
>                 metadata->addEntry(tag, defaultVector);
>                 return defaultVector;
>         }
> 
>         std::vector<T> ret(info->second.values().size());
>         for (const auto &value : info->second.values())
>                 ret.push_back(value.get<V>());
>         metadata->addEntry(tag, ret);
> 
>         return ret;
> 
> 
> > +	}
> > +
> > +	metadata->addEntry(tag, ret);
> > +	return ret;
> > +}
> > +
> >  } /* namespace */
> >
> >  int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,
> > --
> > 2.27.0
> >
Jacopo Mondi July 9, 2021, 7:52 a.m. UTC | #3
Hi Paul,

On Fri, Jul 09, 2021 at 01:43:06PM +0900, paul.elder@ideasonboard.com wrote:
> Hi Jacopo,
>
> On Mon, Jul 05, 2021 at 05:44:58PM +0200, Jacopo Mondi wrote:
> > Hi Paul,
> >
> > 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@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,
> > > +};
> > > +
> > > +/**
> >
> > We don't parse the HAL with doxygen so you can spare the additional *
> > But it's more than fine to use the Doxygen syntax when documenting
> > things imho, mostly for consistency
> >
> > > + * \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
> >
> > s/Android metadata/Android metadata pack/
> >
> > > + * \param[in] tag Android metadata tag to add
> >
> > s/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
> >
> > Set the android metadata \a tag in the \a metadata pack 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,
> >
> > You should include <type_traits>
> >
> > Do you need this to solve which overload ? I see three functions here
> > with the following prototypes
> >
> > void setMetadata(CameraMetadata *metadata, uint32_t tag,
> > 		 const ControlInfoMap &controlsInfo, const ControlId *control,
> > 		 enum ControlRange controlRange)
> >
> > T setMetadata(CameraMetadata *metadata, uint32_t tag,
> > 	      const ControlInfoMap &controlsInfo, const ControlId *control,
> > 	      enum ControlRange controlRange, const V defaultValue)
> >
> > std::vector<T> setMetadata(CameraMetadata *metadata, uint32_t tag,
> > 			   const ControlInfoMap &controlsInfo,
> > 			   const ControlId *control,
> > 			   const std::vector<T> &defaultVector)
> >
> > There doesn't seem to be any need to distinguish them on the template parameter
> > type.
>
> I added the template parameters for consistency. The latter two V and T,
> respectively, can be inferred, but the first one cannot. And the first
> one cannot be merged into the second (with default value) as then we
> have no way of differentiating between "unspecified" and zero.
>

What I meant was specificially regarding enable_if<> as it doesn't
seem to me you need it. Couldn't you get to the same result with a regular
function overload that does not involve template metaprogramming ?

> >
> > > +	 typename V>
> > > +void setMetadata(CameraMetadata *metadata, uint32_t tag,
> > > +		 const ControlInfoMap &controlsInfo, const ControlId *control,
> > > +		 enum ControlRange controlRange)
> > > +{
> > > +	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
> > > + * \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,
> > > +	 typename V,
> >
> > Isn't V the same type as T ?
>
> It is, but then as I mentioned earlier, this call would then be missing
> a template parameter while the other one won't.
>

I see three template parameters here and 2 above.
Also
        <typename T, .. , typename V>
        T setMetadata(..., V defaultValue)
        {
                T ret = defaultValue;
                ...
        }

Again feels like T == V


> >
> > > +	 std::enable_if_t<std::is_arithmetic_v<V>> * = nullptr>
> >
> > This is probably functionally equivalent for now, but in the previous
> > version you were checking for the type T to be an arithmetic type.
>
> I was wondering if the template parameters should be in alphabetical
> order or not...

Whatever, as long as you do your type checks on the same type as you
have:

	 std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
in one version, and
	 std::enable_if_t<std::is_arithmetic_v<V>> * = nullptr>
in this version

This seems to reinforce to me T==V, but if you could just drop
enable_if<> I think it would be better (iirc I compiled this patch
with enable_if<> removed, the firs time I reviewed it)

Thanks
    j

>
>
> Thanks,
>
> Paul
>
> >
> > > +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>
> >
> > Fits on one line
> >
> > > +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;
> >
> > this is a copy that could be avoided by calling addEntry() in each
> > branch. Something like
> >
> > 	const auto &info = controlsInfo.find(control);
> > 	if (info == controlsInfo.end()) {
> >                 metadata->addEntry(tag, defaultVector);
> >                 return defaultVector;
> >         }
> >
> >         std::vector<T> ret(info->second.values().size());
> >         for (const auto &value : info->second.values())
> >                 ret.push_back(value.get<V>());
> >         metadata->addEntry(tag, ret);
> >
> >         return ret;
> >
> >
> > > +	}
> > > +
> > > +	metadata->addEntry(tag, ret);
> > > +	return ret;
> > > +}
> > > +
> > >  } /* namespace */
> > >
> > >  int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,
> > > --
> > > 2.27.0
> > >
Paul Elder July 9, 2021, 8:08 a.m. UTC | #4
Hi Jacopo,

On Fri, Jul 09, 2021 at 09:52:55AM +0200, Jacopo Mondi wrote:
> Hi Paul,
> 
> On Fri, Jul 09, 2021 at 01:43:06PM +0900, paul.elder@ideasonboard.com wrote:
> > Hi Jacopo,
> >
> > On Mon, Jul 05, 2021 at 05:44:58PM +0200, Jacopo Mondi wrote:
> > > Hi Paul,
> > >
> > > 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@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,
> > > > +};
> > > > +
> > > > +/**
> > >
> > > We don't parse the HAL with doxygen so you can spare the additional *
> > > But it's more than fine to use the Doxygen syntax when documenting
> > > things imho, mostly for consistency
> > >
> > > > + * \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
> > >
> > > s/Android metadata/Android metadata pack/
> > >
> > > > + * \param[in] tag Android metadata tag to add
> > >
> > > s/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
> > >
> > > Set the android metadata \a tag in the \a metadata pack 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,
> > >
> > > You should include <type_traits>
> > >
> > > Do you need this to solve which overload ? I see three functions here
> > > with the following prototypes
> > >
> > > void setMetadata(CameraMetadata *metadata, uint32_t tag,
> > > 		 const ControlInfoMap &controlsInfo, const ControlId *control,
> > > 		 enum ControlRange controlRange)
> > >
> > > T setMetadata(CameraMetadata *metadata, uint32_t tag,
> > > 	      const ControlInfoMap &controlsInfo, const ControlId *control,
> > > 	      enum ControlRange controlRange, const V defaultValue)
> > >
> > > std::vector<T> setMetadata(CameraMetadata *metadata, uint32_t tag,
> > > 			   const ControlInfoMap &controlsInfo,
> > > 			   const ControlId *control,
> > > 			   const std::vector<T> &defaultVector)
> > >
> > > There doesn't seem to be any need to distinguish them on the template parameter
> > > type.
> >
> > I added the template parameters for consistency. The latter two V and T,
> > respectively, can be inferred, but the first one cannot. And the first
> > one cannot be merged into the second (with default value) as then we
> > have no way of differentiating between "unspecified" and zero.
> >
> 
> What I meant was specificially regarding enable_if<> as it doesn't
> seem to me you need it. Couldn't you get to the same result with a regular
> function overload that does not involve template metaprogramming ?

I guess the first one doesn't need it.

The second and third I think do? Otherwise we could get V = std::vector<*> ?

> > >
> > > > +	 typename V>
> > > > +void setMetadata(CameraMetadata *metadata, uint32_t tag,
> > > > +		 const ControlInfoMap &controlsInfo, const ControlId *control,
> > > > +		 enum ControlRange controlRange)
> > > > +{
> > > > +	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
> > > > + * \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,
> > > > +	 typename V,
> > >
> > > Isn't V the same type as T ?
> >
> > It is, but then as I mentioned earlier, this call would then be missing
> > a template parameter while the other one won't.
> >
> 
> I see three template parameters here and 2 above.
> Also
>         <typename T, .. , typename V>
>         T setMetadata(..., V defaultValue)
>         {
>                 T ret = defaultValue;
>                 ...
>         }
> 
> Again feels like T == V

My intention was to allow T != V as long as they can be implicitly
converted, like enum -> uint8_t, or uint8_t -> bool. It's quite common
that the android uses and enum while libcamera uses int32_t, for
example.

Which does mean that the template parameter needs || std::is_enum_v<V>.


Thanks,

Paul

> 
> > >
> > > > +	 std::enable_if_t<std::is_arithmetic_v<V>> * = nullptr>
> > >
> > > This is probably functionally equivalent for now, but in the previous
> > > version you were checking for the type T to be an arithmetic type.
> >
> > I was wondering if the template parameters should be in alphabetical
> > order or not...
> 
> Whatever, as long as you do your type checks on the same type as you
> have:
> 
> 	 std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> in one version, and
> 	 std::enable_if_t<std::is_arithmetic_v<V>> * = nullptr>
> in this version
> 
> This seems to reinforce to me T==V, but if you could just drop
> enable_if<> I think it would be better (iirc I compiled this patch
> with enable_if<> removed, the firs time I reviewed it)
> 
> Thanks
>     j
> 
> >
> >
> > Thanks,
> >
> > Paul
> >
> > >
> > > > +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>
> > >
> > > Fits on one line
> > >
> > > > +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;
> > >
> > > this is a copy that could be avoided by calling addEntry() in each
> > > branch. Something like
> > >
> > > 	const auto &info = controlsInfo.find(control);
> > > 	if (info == controlsInfo.end()) {
> > >                 metadata->addEntry(tag, defaultVector);
> > >                 return defaultVector;
> > >         }
> > >
> > >         std::vector<T> ret(info->second.values().size());
> > >         for (const auto &value : info->second.values())
> > >                 ret.push_back(value.get<V>());
> > >         metadata->addEntry(tag, ret);
> > >
> > >         return ret;
> > >
> > >
> > > > +	}
> > > > +
> > > > +	metadata->addEntry(tag, ret);
> > > > +	return ret;
> > > > +}
> > > > +
> > > >  } /* namespace */
> > > >
> > > >  int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,
> > > > --
> > > > 2.27.0
> > > >
Laurent Pinchart July 11, 2021, 9:26 p.m. UTC | #5
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@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,

Patch
diff mbox series

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>
+void setMetadata(CameraMetadata *metadata, uint32_t tag,
+		 const ControlInfoMap &controlsInfo, const ControlId *control,
+		 enum ControlRange controlRange)
+{
+	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
+ * \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,
+	 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,