libcamera: controls: Improve the API towards applications

Rework the control-related classes to improve the API towards
applications. The goal is to enable writing code similar to

	Request *req = ...;
	ControlList &controls = req->controls();
	controls->set(controls::AwbEnable, false);
	controls->set(controls::ManualExposure, 1000);

	...

	int32_t exposure = controls->get(controls::ManualExposure);

with the get and set operations ensuring type safety for the control
values. This is achieved by creating the following classes:

- Control defines controls and is the main way to reference a control.
  It is a template class to allow methods using it to refer to the
  control type.

- ControlId is the base class of Control. It stores the control ID, name
  and type, and can be used in contexts where a control needs to be
  referenced regardless of its type (for instance in lists of controls).
  This class replaces ControlIdentifier.

- ControlValue is kept as-is.

The ControlList class now exposes two template get() and set() methods
that replace the operator[]. They ensure type safety by infering the
value type from the Control reference that they receive.

The main way to refer to a control is now through the Control class, and
optionally through its base ControlId class. The ControlId enumeration
is removed, replaced by a list of global Control instances. Numerical
control IDs are turned into macros, and are still exposed as they are
required to communicate with IPAs (especially to deserialise control
lists). They should however not be used by applications.

Auto-generation of header and source files is removed for now to keep
the change simple. It will be added back in the future in a more
elaborate form.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

6 files changed, 236 insertions, 319 deletions

diff --git a/src/libcamera/control_ids.cpp b/src/libcamera/control_ids.cpp
new file mode 100644
index 00000000..3af23a45
--- /dev/null
+++ b/src/libcamera/control_ids.cpp
@@ -0,0 +1,52 @@
+/* SPDX-License-Identifier: LGPL-2.1-or-later */
+/*
+ * Copyright (C) 2019, Google Inc.
+ *
+ * control_ids.cpp : Control ID list
+ */
+
+#include <libcamera/control_ids.h>
+
+/**
+ * \file control_ids.h
+ * \brief Camera control identifiers
+ */
+
+namespace libcamera {
+
+namespace controls {
+
+/**
+ * \brief Enables or disables the AWB.
+ * \sa ManualGain
+ */
+extern const Control<bool> AwbEnable(AWB_ENABLE, "AwbEnable");
+
+/**
+ * \brief Specify a fixed brightness parameter.
+ */
+extern const Control<int32_t> Brightness(BRIGHTNESS, "Brightness");
+
+/**
+ * \brief Specify a fixed contrast parameter.
+ */
+extern const Control<int32_t> Contrast(CONTRAST, "Contrast");
+
+/**
+ * \brief Specify a fixed saturation parameter.
+ */
+extern const Control<int32_t> Saturation(SATURATION, "Saturation");
+
+/**
+ * \brief Specify a fixed exposure time in milli-seconds
+ */
+extern const Control<int32_t> ManualExposure(MANUAL_EXPOSURE, "ManualExposure");
+
+/**
+ * \brief Specify a fixed gain parameter
+ */
+extern const Control<int32_t> ManualGain(MANUAL_GAIN, "ManualGain");
+
+} /* namespace controls */
+
+} /* namespace libcamera */
diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp
index 295ccd55..a34af588 100644
--- a/src/libcamera/controls.cpp
+++ b/src/libcamera/controls.cpp
@@ -18,6 +18,29 @@
 /**
  * \file controls.h
  * \brief Describes control framework and controls supported by a camera
+ *
+ * A control is a mean to govern or influence the operation of a camera. Every
+ * control is defined by a unique numerical ID, a name string and the data type
+ * of the value it stores. The libcamera API defines a set of standard controls
+ * in the libcamera::controls namespace, as a set of instances of the Control
+ * class.
+ *
+ * The main way for applications to interact with controls is through the
+ * ControlList stored in the Request class:
+ *
+ * \code{.cpp}
+ * Request *req = ...;
+ * ControlList &controls = req->controls();
+ * controls->set(controls::AwbEnable, false);
+ * controls->set(controls::ManualExposure, 1000);
+ *
+ * ...
+ *
+ * int32_t exposure = controls->get(controls::ManualExposure);
+ * \endcode
+ *
+ * The ControlList::get() and ControlList::set() methods automatically deduce
+ * the data type based on the control.
  */
 
 namespace libcamera {
@@ -173,76 +196,120 @@ std::string ControlValue::toString() const
 }
 
 /**
- * \enum ControlId
- * \brief Numerical control ID
+ * \class ControlId
+ * \brief Control static metadata
+ *
+ * The ControlId class stores a control ID, name and data type. It provides
+ * unique identification of a control, but without support for compile-time
+ * type deduction that the derived template Control class supports. See the
+ * Control class for more information.
  */
 
 /**
- * \var AwbEnable
- * ControlType: Bool
- *
- * Enables or disables the AWB. See also \a libcamera::ControlId::ManualGain
+ * \fn ControlId::ControlId(unsigned int id, const char *name, ControlType type)
+ * \brief Construct a ControlId instance
+ * \param[in] id The control numerical ID
+ * \param[in] name The control name
+ * \param[in] type The control data type
  */
 
 /**
- * \var Brightness
- * ControlType: Integer32
- *
- * Specify a fixed brightness parameter.
+ * \fn unsigned int ControlId::id() const
+ * \brief Retrieve the control numerical ID
+ * \return The control numerical ID
  */
 
 /**
- * \var Contrast
- * ControlType: Integer32
- *
- * Specify a fixed contrast parameter.
+ * \fn const char *ControlId::name() const
+ * \brief Retrieve the control name
+ * \return The control name
  */
 
 /**
- * \var Saturation
- * ControlType: Integer32
- *
- * Specify a fixed saturation parameter.
+ * \fn ControlType ControlId::type() const
+ * \brief Retrieve the control data type
+ * \return The control data type
  */
 
 /**
- * \var ManualExposure
- * ControlType: Integer32
+ * \fn bool operator==(const ControlId &lhs, const ControlId &rhs)
+ * \brief Compare two ControlId instances for equality
+ * \param[in] lhs Left-hand side ControlId
+ * \param[in] rhs Right-hand side ControlId
+ *
+ * ControlId instances are compared based on the numerical ControlId::id()
+ * only, as an object may not have two separate controls with the same
+ * numerical ID.
  *
- * Specify a fixed exposure time in milli-seconds
+ * \return True if \a lhs and \a rhs have equal control IDs, false otherwise
  */
 
 /**
- * \var ManualGain
- * ControlType: Integer32
+ * \class Control
+ * \brief Describe a control and its intrinsic properties
  *
- * Specify a fixed gain parameter
+ * The Control class models a control exposed by a camera. Its template type
+ * name T refers to the control data type, and allows methods that operate on
+ * control values to be defined as template methods using the same type T for
+ * the control value. See for instance how the ControlList::get() method
+ * returns a value corresponding to the type of the requested control.
+ *
+ * While this class is the main mean to refer to a control, the control
+ * identifying information are stored in the non-template base ControlId class.
+ * This allows code that operates on a set of controls of different types to
+ * reference those controls through a ControlId instead of a Control. For
+ * instance, the list of controls supported by a camera is exposed as ControlId
+ * instead of Control.
+ *
+ * Controls of any type can be defined through template specialisation, but
+ * libcamera only supports the bool, int32_t and int64_t types natively (this
+ * includes types that are equivalent to the supported types, such as int and
+ * long int).
+ *
+ * Controls IDs shall be unique. While nothing prevents multiple instances of
+ * the Control class to be created with the same ID, this may lead to undefined
+ * behaviour.
  */
 
 /**
- * \struct ControlIdentifier
- * \brief Describe a ControlId with control specific constant meta-data
- *
- * Defines a Control with a unique ID, a name, and a type.
- * This structure is used as static part of the auto-generated control
- * definitions, which are generated from the ControlId documentation.
+ * \fn Control::Control(unsigned int id, const char *name)
+ * \brief Construct a Control instance
+ * \param[in] id The control numerical ID
+ * \param[in] name The control name
  *
- * \var ControlIdentifier::id
- * The unique ID for a control
- * \var ControlIdentifier::name
- * The string representation of the control
- * \var ControlIdentifier::type
- * The ValueType required to represent the control value
+ * The control data type is automatically deduced from the template type T.
  */
 
-/*
- * The controlTypes are automatically generated to produce a control_types.cpp
- * output. This file is not for public use, and so no suitable header exists
- * for this sole usage of the controlTypes reference. As such the extern is
- * only defined here for use during the ControlInfo constructor and should not
- * be referenced directly elsewhere.
+/**
+ * \typedef Control::type
+ * \brief The Control template type T
  */
-extern const std::unordered_map<ControlId, ControlIdentifier> controlTypes;
+
+#ifndef __DOXYGEN__
+template<>
+Control<void>::Control(unsigned int id, const char *name)
+	: ControlId(id, name, ControlTypeNone)
+{
+}
+
+template<>
+Control<bool>::Control(unsigned int id, const char *name)
+	: ControlId(id, name, ControlTypeBool)
+{
+}
+
+template<>
+Control<int32_t>::Control(unsigned int id, const char *name)
+	: ControlId(id, name, ControlTypeInteger32)
+{
+}
+
+template<>
+Control<int64_t>::Control(unsigned int id, const char *name)
+	: ControlId(id, name, ControlTypeInteger64)
+{
+}
+#endif /* __DOXYGEN__ */
 
 /**
  * \class ControlInfo
@@ -260,17 +327,10 @@ extern const std::unordered_map<ControlId, ControlIdentifier> controlTypes;
  * \param[in] min The control minimum value
  * \param[in] max The control maximum value
  */
-ControlInfo::ControlInfo(ControlId id, const ControlValue &min,
+ControlInfo::ControlInfo(const ControlId &id, const ControlValue &min,
 			 const ControlValue &max)
-	: min_(min), max_(max)
+	: id_(id), min_(min), max_(max)
 {
-	auto iter = controlTypes.find(id);
-	if (iter == controlTypes.end()) {
-		LOG(Controls, Fatal) << "Attempt to create invalid ControlInfo";
-		return;
-	}
-
-	ident_ = &iter->second;
 }
 
 /**
@@ -280,18 +340,6 @@ ControlInfo::ControlInfo(ControlId id, const ControlValue &min,
  */
 
 /**
- * \fn ControlInfo::name()
- * \brief Retrieve the control name string
- * \return The control name string
- */
-
-/**
- * \fn ControlInfo::type()
- * \brief Retrieve the control data type
- * \return The control data type
- */
-
-/**
  * \fn ControlInfo::min()
  * \brief Retrieve the minimum value of the control
  * \return A ControlValue with the minimum value for the control
@@ -310,57 +358,12 @@ std::string ControlInfo::toString() const
 {
 	std::stringstream ss;
 
-	ss << name() << "[" << min_.toString() << ".." << max_.toString() << "]";
+	ss << id_.name() << "[" << min_.toString() << ".." << max_.toString() << "]";
 
 	return ss.str();
 }
 
 /**
- * \brief Compare control information for equality
- * \param[in] lhs Left-hand side control information
- * \param[in] rhs Right-hand side control information
- *
- * Control information is compared based on the ID only, as a camera may not
- * have two separate controls with the same ID.
- *
- * \return True if \a lhs and \a rhs are equal, false otherwise
- */
-bool operator==(const ControlInfo &lhs, const ControlInfo &rhs)
-{
-	return lhs.id() == rhs.id();
-}
-
-/**
- * \brief Compare control ID and information for equality
- * \param[in] lhs Left-hand side control identifier
- * \param[in] rhs Right-hand side control information
- *
- * Control information is compared based on the ID only, as a camera may not
- * have two separate controls with the same ID.
- *
- * \return True if \a lhs and \a rhs are equal, false otherwise
- */
-bool operator==(const ControlId &lhs, const ControlInfo &rhs)
-{
-	return lhs == rhs.id();
-}
-
-/**
- * \brief Compare control information and ID for equality
- * \param[in] lhs Left-hand side control information
- * \param[in] rhs Right-hand side control identifier
- *
- * Control information is compared based on the ID only, as a camera may not
- * have two separate controls with the same ID.
- *
- * \return True if \a lhs and \a rhs are equal, false otherwise
- */
-bool operator==(const ControlInfo &lhs, const ControlId &rhs)
-{
-	return lhs.id() == rhs;
-}
-
-/**
  * \typedef ControlInfoMap
  * \brief A map of ControlId to ControlInfo
  */
@@ -431,29 +434,9 @@ ControlList::ControlList(Camera *camera)
  *
  * \return True if the list contains a matching control, false otherwise
  */
-bool ControlList::contains(ControlId id) const
-{
-	const ControlInfoMap &controls = camera_->controls();
-	const auto iter = controls.find(id);
-	if (iter == controls.end()) {
-		LOG(Controls, Error)
-			<< "Camera " << camera_->name()
-			<< " does not support control " << id;
-
-		return false;
-	}
-
-	return controls_.find(&iter->second) != controls_.end();
-}
-
-/**
- * \brief Check if the list contains a control with the specified \a info
- * \param[in] info The control info
- * \return True if the list contains a matching control, false otherwise
- */
-bool ControlList::contains(const ControlInfo *info) const
+bool ControlList::contains(const ControlId &id) const
 {
-	return controls_.find(info) != controls_.end();
+	return controls_.find(&id) != controls_.end();
 }
 
 /**
@@ -474,45 +457,62 @@ bool ControlList::contains(const ControlInfo *info) const
  */
 
 /**
- * \brief Access or insert the control specified by \a id
- * \param[in] id The control ID
+ * \fn template<typename T> const T &ControlList::get() const
+ * \brief Get the value of a control
+ * \param[in] ctrl The control
  *
- * This method returns a reference to the control identified by \a id, inserting
- * it in the list if the ID is not already present.
+ * The behaviour is undefined if the control \a ctrl is not present in the
+ * list. Use ControlList::contains() to test for the presence of a control in
+ * the list before retrieving its value.
  *
- * The behaviour is undefined if the control \a id is not supported by the
- * camera that the ControlList refers to.
+ * The control value type shall match the type T, otherwise the behaviour is
+ * undefined.
+ *
+ * \return The control value
+ */
+
+/**
+ * \fn template<typename T> void ControlList::set()
+ * \brief Set the control value to \a value
+ * \param[in] ctrl The control
+ * \param[in] value The control value
+ *
+ * This method sets the value of a control in the control list. If the control
+ * is already present in the list, its value is updated, otherwise it is added
+ * to the list.
  *
- * \return A reference to the value of the control identified by \a id
+ * The behaviour is undefined if the control \a ctrl is not supported by the
+ * camera that the list refers to.
  */
-ControlValue &ControlList::operator[](ControlId id)
+
+const ControlValue *ControlList::find(const ControlId &id) const
+{
+	const auto iter = controls_.find(&id);
+	if (iter == controls_.end()) {
+		LOG(Controls, Error)
+			<< "Control " << id.name() << " not found";
+
+		return nullptr;
+	}
+
+	return &iter->second;
+}
+
+ControlValue *ControlList::find(const ControlId &id)
 {
 	const ControlInfoMap &controls = camera_->controls();
-	const auto iter = controls.find(id);
+	const auto iter = controls.find(&id);
 	if (iter == controls.end()) {
 		LOG(Controls, Error)
 			<< "Camera " << camera_->name()
-			<< " does not support control " << id;
-
-		static ControlValue empty;
-		return empty;
+			<< " does not support control " << id.name();
+		return nullptr;
 	}
 
-	return controls_[&iter->second];
+	return &controls_[&id];
 }
 
 /**
- * \fn ControlList::operator[](const ControlInfo *info)
- * \brief Access or insert the control specified by \a info
- * \param[in] info The control info
- *
- * This method returns a reference to the control identified by \a info,
- * inserting it in the list if the info is not already present.
- *
- * \return A reference to the value of the control identified by \a info
- */
-
-/**
  * \brief Update the list with a union of itself and \a other
  * \param other The other list
  *
@@ -533,10 +533,10 @@ void ControlList::update(const ControlList &other)
 	}
 
 	for (auto it : other) {
-		const ControlInfo *info = it.first;
+		const ControlId *id = it.first;
 		const ControlValue &value = it.second;
 
-		controls_[info] = value;
+		controls_[id] = value;
 	}
 }
 
diff --git a/src/libcamera/gen-controls.awk b/src/libcamera/gen-controls.awk
deleted file mode 100755
index a3f291e7..00000000
--- a/src/libcamera/gen-controls.awk
+++ /dev/null
@@ -1,106 +0,0 @@
-#!/usr/bin/awk -f
-
-# SPDX-License-Identifier: LGPL-2.1-or-later
-
-# Controls are documented using Doxygen in the main controls.cpp source.
-#
-# Generate control tables directly from the documentation, creating enumerations
-# to support the IDs and static type information regarding each control.
-
-BEGIN {
-	id=0
-	input=ARGV[1]
-	mode=ARGV[2]
-	output=ARGV[3]
-}
-
-# Detect Doxygen style comment blocks and ignore other lines
-/^\/\*\*$/ { in_doxygen=1; first_line=1; next }
-// { if (!in_doxygen) next }
-
-# Entry point for the Control Documentation
-/ * \\enum ControlId$/ { in_controls=1; first_line=0; next }
-// { if (!in_controls) next }
-
-# Extract control information
-/ \* \\var/ { names[++id]=$3; first_line=0; next }
-/ \* ControlType:/ { types[id] = $3 }
-
-# End of comment blocks
-/^ \*\// { in_doxygen=0 }
-
-# Identify the end of controls
-/^ \* \\/ { if (first_line) exit }
-// { first_line=0 }
-
-################################
-# Support output file generation
-
-function basename(file) {
-	sub(".*/", "", file)
-	return file
-}
-
-function Header(file, description) {
-	print "/* SPDX-License-Identifier: LGPL-2.1-or-later */" > file
-	print "/*" > file
-	print " * Copyright (C) 2019, Google Inc." > file
-	print " *" > file
-	print " * " basename(file) " - " description > file
-	print " *" > file
-	print " * This file is auto-generated. Do not edit." > file
-	print " */" > file
-	print "" > file
-}
-
-function EnterNameSpace(file) {
-	print "namespace libcamera {" > file
-	print "" > file
-}
-
-function ExitNameSpace(file) {
-	print "" > file
-	print "} /* namespace libcamera */" > file
-}
-
-function GenerateHeader(file) {
-	Header(file, "Control ID list")
-
-	print "#ifndef __LIBCAMERA_CONTROL_IDS_H__" > file
-	print "#define __LIBCAMERA_CONTROL_IDS_H__" > file
-	print "" > file
-
-	EnterNameSpace(file)
-	print "enum ControlId {" > file
-	for (i=1; i <= id; ++i) {
-		printf "\t%s,\n", names[i] > file
-	}
-	print "};" > file
-	ExitNameSpace(file)
-
-	print "" > file
-	print "#endif // __LIBCAMERA_CONTROL_IDS_H__" > file
-}
-
-function GenerateTable(file) {
-	Header(file, "Control types")
-	print "#include <libcamera/controls.h>" > file
-	print "" > file
-
-	EnterNameSpace(file)
-
-	print "extern const std::unordered_map<ControlId, ControlIdentifier>" > file
-	print "controlTypes {" > file
-	for (i=1; i <= id; ++i) {
-		printf "\t{ %s, { %s, \"%s\", ControlType%s } },\n", names[i], names[i], names[i], types[i] > file
-	}
-	print "};" > file
-	ExitNameSpace(file)
-}
-
-END {
-	if (mode == "--header")
-		GenerateHeader(output)
-	else
-		GenerateTable(output)
-}
diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
index 755149c5..8123d1d5 100644
--- a/src/libcamera/meson.build
+++ b/src/libcamera/meson.build
@@ -5,6 +5,7 @@ libcamera_sources = files([
     'camera_manager.cpp',
     'camera_sensor.cpp',
     'controls.cpp',
+    'control_ids.cpp',
     'device_enumerator.cpp',
     'device_enumerator_sysfs.cpp',
     'event_dispatcher.cpp',
@@ -57,16 +58,6 @@ if libudev.found()
     ])
 endif
 
-gen_controls = files('gen-controls.awk')
-
-control_types_cpp = custom_target('control_types_cpp',
-                                  input : files('controls.cpp'),
-                                  output : 'control_types.cpp',
-                                  depend_files : gen_controls,
-                                  command : [gen_controls, '@INPUT@', '--code', '@OUTPUT@'])
-
-libcamera_sources += control_types_cpp
-
 gen_version = join_paths(meson.source_root(), 'utils', 'gen-version.sh')
 
 version_cpp = vcs_tag(command : [gen_version, meson.build_root()],
diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp
index 0d56758e..d5d30932 100644
--- a/src/libcamera/pipeline/uvcvideo.cpp
+++ b/src/libcamera/pipeline/uvcvideo.cpp
@@ -10,6 +10,7 @@
 #include <tuple>
 
 #include <libcamera/camera.h>
+#include <libcamera/control_ids.h>
 #include <libcamera/controls.h>
 #include <libcamera/request.h>
 #include <libcamera/stream.h>
@@ -230,33 +231,20 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request)
 	V4L2ControlList controls;
 
 	for (auto it : request->controls()) {
-		const ControlInfo *ci = it.first;
+		const ControlId &id = *it.first;
 		ControlValue &value = it.second;
 
-		switch (ci->id()) {
-		case Brightness:
+		if (id == controls::Brightness) {
 			controls.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());
-			break;
-
-		case Contrast:
+		} else if (id == controls::Contrast) {
 			controls.add(V4L2_CID_CONTRAST, value.get<int32_t>());
-			break;
-
-		case Saturation:
+		} else if (id == controls::Saturation) {
 			controls.add(V4L2_CID_SATURATION, value.get<int32_t>());
-			break;
-
-		case ManualExposure:
+		} else if (id == controls::ManualExposure) {
 			controls.add(V4L2_CID_EXPOSURE_AUTO, 1);
 			controls.add(V4L2_CID_EXPOSURE_ABSOLUTE, value.get<int32_t>());
-			break;
-
-		case ManualGain:
+		} else if (id == controls::ManualGain) {
 			controls.add(V4L2_CID_GAIN, value.get<int32_t>());
-			break;
-
-		default:
-			break;
 		}
 	}
 
@@ -352,23 +340,23 @@ int UVCCameraData::init(MediaEntity *entity)
 	for (const auto &ctrl : controls) {
 		unsigned int v4l2Id = ctrl.first;
 		const V4L2ControlInfo &info = ctrl.second;
-		ControlId id;
+		const ControlId *id;
 
 		switch (v4l2Id) {
 		case V4L2_CID_BRIGHTNESS:
-			id = Brightness;
+			id = &controls::Brightness;
 			break;
 		case V4L2_CID_CONTRAST:
-			id = Contrast;
+			id = &controls::Contrast;
 			break;
 		case V4L2_CID_SATURATION:
-			id = Saturation;
+			id = &controls::Saturation;
 			break;
 		case V4L2_CID_EXPOSURE_ABSOLUTE:
-			id = ManualExposure;
+			id = &controls::ManualExposure;
 			break;
 		case V4L2_CID_GAIN:
-			id = ManualGain;
+			id = &controls::ManualGain;
 			break;
 		default:
 			continue;
@@ -376,7 +364,7 @@ int UVCCameraData::init(MediaEntity *entity)
 
 		controlInfo_.emplace(std::piecewise_construct,
 				     std::forward_as_tuple(id),
-				     std::forward_as_tuple(id, info.min(), info.max()));
+				     std::forward_as_tuple(*id, info.min(), info.max()));
 	}
 
 	return 0;
diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp
index e549dc64..608a47ae 100644
--- a/src/libcamera/pipeline/vimc.cpp
+++ b/src/libcamera/pipeline/vimc.cpp
@@ -15,6 +15,7 @@
 #include <ipa/ipa_interface.h>
 #include <ipa/ipa_module_info.h>
 #include <libcamera/camera.h>
+#include <libcamera/control_ids.h>
 #include <libcamera/controls.h>
 #include <libcamera/request.h>
 #include <libcamera/stream.h>
@@ -283,24 +284,15 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request)
 	V4L2ControlList controls;
 
 	for (auto it : request->controls()) {
-		const ControlInfo *ci = it.first;
+		const ControlId &id = *it.first;
 		ControlValue &value = it.second;
 
-		switch (ci->id()) {
-		case Brightness:
+		if (id == controls::Brightness) {
 			controls.add(V4L2_CID_BRIGHTNESS, value.get<int32_t>());
-			break;
-
-		case Contrast:
+		} else if (id == controls::Contrast) {
 			controls.add(V4L2_CID_CONTRAST, value.get<int32_t>());
-			break;
-
-		case Saturation:
+		} else if (id == controls::Saturation) {
 			controls.add(V4L2_CID_SATURATION, value.get<int32_t>());
-			break;
-
-		default:
-			break;
 		}
 	}
 
@@ -427,17 +419,17 @@ int VimcCameraData::init(MediaDevice *media)
 	for (const auto &ctrl : controls) {
 		unsigned int v4l2Id = ctrl.first;
 		const V4L2ControlInfo &info = ctrl.second;
-		ControlId id;
+		const ControlId *id;
 
 		switch (v4l2Id) {
 		case V4L2_CID_BRIGHTNESS:
-			id = Brightness;
+			id = &controls::Brightness;
 			break;
 		case V4L2_CID_CONTRAST:
-			id = Contrast;
+			id = &controls::Contrast;
 			break;
 		case V4L2_CID_SATURATION:
-			id = Saturation;
+			id = &controls::Saturation;
 			break;
 		default:
 			continue;
@@ -445,7 +437,7 @@ int VimcCameraData::init(MediaDevice *media)
 
 		controlInfo_.emplace(std::piecewise_construct,
 				     std::forward_as_tuple(id),
-				     std::forward_as_tuple(id, info.min(), info.max()));
+				     std::forward_as_tuple(*id, info.min(), info.max()));
 	}
 
 	return 0;