[libcamera-devel] [RFC PATCH v2 1/3] android: Add helpers for setting android metadata from libcamera controls

[Hirokazu Honda]

Hi Paul,

On Thu, Jun 24, 2021 at 9:40 PM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Paul,
>
> Thank you for the patch.
>
> On Thu, Jun 24, 2021 at 07:50:52PM +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 last one
> > requires the type to be specified, but the other two do not.
> >
> > 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>
> >
> > ---
> > New in v2
> >
> > TODO: make ControlList versions so that we can use them in result
> > metadata
> > ---
> >  src/android/camera_capabilities.cpp | 81 +++++++++++++++++++++++++++++
> >  1 file changed, 81 insertions(+)
> >
> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > index 00224a6d..7d795bf5 100644
> > --- a/src/android/camera_capabilities.cpp
> > +++ b/src/android/camera_capabilities.cpp
> > @@ -113,6 +113,87 @@ const std::map<int, const Camera3Format> camera3FormatsMap = {
> >       },
> >  };
> >
> > +enum ControlRange {
nit: enum class

> > +     Min,
> > +     Def,
> > +     Max,
> > +};
> > +
> > +template<typename T,
> > +      std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>

Perhaps std::enable_if_t<std::is_arithmetic_v<T>::value, bool> = true?

> > +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 Min:
> > +             ret = info->second.min().get<T>();
> > +             break;
> > +     case Def:
> > +             ret = info->second.def().get<T>();
> > +             break;
> > +     case Max:
> > +             ret = info->second.max().get<T>();
> > +             break;
> > +     }
> > +
> > +     metadata->addEntry(tag, ret);
> > +     return;
> > +}
> > +
> > +template<typename T,
> > +      std::enable_if_t<std::is_arithmetic_v<T>> * = nullptr>
> > +T setMetadata(CameraMetadata *metadata, uint32_t tag,
> > +           const ControlInfoMap &controlsInfo, const ControlId *control,
> > +           const T defaultValue, enum ControlRange controlRange)

nit: I would put defaultValue to the last argument.

> > +{
> > +     T ret = defaultValue;
> > +
> > +     const auto &info = controlsInfo.find(control);
> > +     if (info != controlsInfo.end()) {
> > +             switch (controlRange) {
> > +             case Min:
> > +                     ret = info->second.min().get<T>();
> > +                     break;
> > +             case Def:
> > +                     ret = info->second.def().get<T>();
> > +                     break;
> > +             case Max:
> > +                     ret = info->second.max().get<T>();
> > +                     break;
> > +             }
> > +     }
> > +
> > +     metadata->addEntry(tag, ret);
> > +     return ret;
> > +}
> > +
> > +template<typename S,
> > +      typename T = typename S::value_type>
> > +S setMetadata(CameraMetadata *metadata, uint32_t tag,
> > +           const ControlInfoMap &controlsInfo, const ControlId *control,
> > +           const S &defaultVector)
>
> If S is required to be a vector, could this be
>
> template<typename T>,
> 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<T>());
> > +     } else {
> > +             ret = defaultVector;
> > +     }
> > +
> > +     metadata->addEntry(tag, ret);
> > +     return ret;
> > +}
> > +
> >  } /* namespace */
> >
> >  int CameraCapabilities::initialize(std::shared_ptr<libcamera::Camera> camera,
>
> --
> Regards,
>
> Laurent Pinchart