[libcamera-devel] [PATCH] libcamera: yaml_parser: Drop std::optional<> from getList()

Jacopo Mondi jacopo at jmondi.org
Wed Aug 3 11:15:02 CEST 2022


The YamlParser::getList<T>() function returns an
std::optional<std::vector<T>>.

The obvious (and only) gain in using std::optional<> is to be able to
check the validity of the returned value with the dereference operator.

However, usage of std::vector<T> as the std::optional<> template
argument causes issues with the usage of utils::defopt with gcc8, as the
it fails to construct a vector with defopt:

/usr/include/c++/8/optional:1267:8: error: call of overloaded
‘vector(const libcamera::utils::details::defopt_t&)’ is ambiguous
      : static_cast<_Tp>(std::forward<_Up>(__u))

As the added value of std::optional<> is debatable when used in getList(),
drop it and return an std::vector<T> directly. The few characters more
that have to be typed to check the validity of the return value
(value.empty() in place of just !value) are compensated by the shorter
assignment statement (value = getList<T>() in place of value =
getList<T>.value_or(utils::defopt))

Reported-by: https://buildbot.libcamera.org/#/builders/6/builds/436
Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
---
 include/libcamera/internal/yaml_parser.h   |  2 +-
 src/ipa/raspberrypi/controller/rpi/agc.cpp | 10 +++++-----
 src/ipa/rkisp1/algorithms/gsl.cpp          |  8 ++++----
 src/ipa/rkisp1/algorithms/lsc.cpp          |  4 ++--
 src/libcamera/yaml_parser.cpp              | 18 +++++++++---------
 test/yaml-parser.cpp                       |  2 +-
 6 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/include/libcamera/internal/yaml_parser.h b/include/libcamera/internal/yaml_parser.h
index 5ba777d364fa..6d04c556f9b1 100644
--- a/include/libcamera/internal/yaml_parser.h
+++ b/include/libcamera/internal/yaml_parser.h
@@ -197,7 +197,7 @@ public:
 #else
 	template<typename T>
 #endif
-	std::optional<std::vector<T>> getList() const;
+	std::vector<T> getList() const;

 	DictAdapter asDict() const { return DictAdapter{ list_ }; }
 	ListAdapter asList() const { return ListAdapter{ list_ }; }
diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp
index bd54a639d637..bb15d3d84186 100644
--- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
+++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
@@ -74,16 +74,16 @@ readMeteringModes(std::map<std::string, AgcMeteringMode> &metering_modes,

 int AgcExposureMode::read(const libcamera::YamlObject &params)
 {
-	auto value = params["shutter"].getList<double>();
-	if (!value)
+	std::vector<double> value = params["shutter"].getList<double>();
+	if (value.empty())
 		return -EINVAL;
-	std::transform(value->begin(), value->end(), std::back_inserter(shutter),
+	std::transform(value.begin(), value.end(), std::back_inserter(shutter),
 		       [](double v) { return v * 1us; });

 	value = params["gain"].getList<double>();
-	if (!value)
+	if (value.empty())
 		return -EINVAL;
-	gain = std::move(*value);
+	gain = std::move(value);

 	if (shutter.size() < 2 || gain.size() < 2) {
 		LOG(RPiAgc, Error)
diff --git a/src/ipa/rkisp1/algorithms/gsl.cpp b/src/ipa/rkisp1/algorithms/gsl.cpp
index 2fd1a23d3a9b..0cff1b77b058 100644
--- a/src/ipa/rkisp1/algorithms/gsl.cpp
+++ b/src/ipa/rkisp1/algorithms/gsl.cpp
@@ -60,7 +60,7 @@ int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context,
 				   const YamlObject &tuningData)
 {
 	std::vector<uint16_t> xIntervals =
-		tuningData["x-intervals"].getList<uint16_t>().value_or(utils::defopt);
+		tuningData["x-intervals"].getList<uint16_t>();
 	if (xIntervals.size() != kDegammaXIntervals) {
 		LOG(RkISP1Gsl, Error)
 			<< "Invalid 'x' coordinates: expected "
@@ -84,7 +84,7 @@ int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context,
 		return -EINVAL;
 	}

-	curveYr_ = yObject["red"].getList<uint16_t>().value_or(utils::defopt);
+	curveYr_ = yObject["red"].getList<uint16_t>();
 	if (curveYr_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
 		LOG(RkISP1Gsl, Error)
 			<< "Invalid 'y:red' coordinates: expected "
@@ -93,7 +93,7 @@ int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context,
 		return -EINVAL;
 	}

-	curveYg_ = yObject["green"].getList<uint16_t>().value_or(utils::defopt);
+	curveYg_ = yObject["green"].getList<uint16_t>();
 	if (curveYg_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
 		LOG(RkISP1Gsl, Error)
 			<< "Invalid 'y:green' coordinates: expected "
@@ -102,7 +102,7 @@ int GammaSensorLinearization::init([[maybe_unused]] IPAContext &context,
 		return -EINVAL;
 	}

-	curveYb_ = yObject["blue"].getList<uint16_t>().value_or(utils::defopt);
+	curveYb_ = yObject["blue"].getList<uint16_t>();
 	if (curveYb_.size() != RKISP1_CIF_ISP_DEGAMMA_CURVE_SIZE) {
 		LOG(RkISP1Gsl, Error)
 			<< "Invalid 'y:blue' coordinates: expected "
diff --git a/src/ipa/rkisp1/algorithms/lsc.cpp b/src/ipa/rkisp1/algorithms/lsc.cpp
index 05c8c0dab5c8..d1a4332c9089 100644
--- a/src/ipa/rkisp1/algorithms/lsc.cpp
+++ b/src/ipa/rkisp1/algorithms/lsc.cpp
@@ -43,7 +43,7 @@ static std::vector<double> parseSizes(const YamlObject &tuningData,
 				      const char *prop)
 {
 	std::vector<double> sizes =
-		tuningData[prop].getList<double>().value_or(utils::defopt);
+		tuningData[prop].getList<double>();
 	if (sizes.size() != RKISP1_CIF_ISP_LSC_SECTORS_TBL_SIZE) {
 		LOG(RkISP1Lsc, Error)
 			<< "Invalid '" << prop << "' values: expected "
@@ -76,7 +76,7 @@ static std::vector<uint16_t> parseTable(const YamlObject &tuningData,
 		RKISP1_CIF_ISP_LSC_SAMPLES_MAX * RKISP1_CIF_ISP_LSC_SAMPLES_MAX;

 	std::vector<uint16_t> table =
-		tuningData[prop].getList<uint16_t>().value_or(utils::defopt);
+		tuningData[prop].getList<uint16_t>();
 	if (table.size() != kLscNumSamples) {
 		LOG(RkISP1Lsc, Error)
 			<< "Invalid '" << prop << "' values: expected "
diff --git a/src/libcamera/yaml_parser.cpp b/src/libcamera/yaml_parser.cpp
index 84cb57d6de83..d422e0811fa7 100644
--- a/src/libcamera/yaml_parser.cpp
+++ b/src/libcamera/yaml_parser.cpp
@@ -316,7 +316,7 @@ template<typename T,
 		 std::is_same_v<uint32_t, T> ||
 		 std::is_same_v<std::string, T> ||
 		 std::is_same_v<Size, T>> *>
-std::optional<std::vector<T>> YamlObject::getList() const
+std::vector<T> YamlObject::getList() const
 {
 	if (type_ != Type::List)
 		return {};
@@ -334,14 +334,14 @@ std::optional<std::vector<T>> YamlObject::getList() const
 	return values;
 }

-template std::optional<std::vector<bool>> YamlObject::getList<bool>() const;
-template std::optional<std::vector<double>> YamlObject::getList<double>() const;
-template std::optional<std::vector<int16_t>> YamlObject::getList<int16_t>() const;
-template std::optional<std::vector<uint16_t>> YamlObject::getList<uint16_t>() const;
-template std::optional<std::vector<int32_t>> YamlObject::getList<int32_t>() const;
-template std::optional<std::vector<uint32_t>> YamlObject::getList<uint32_t>() const;
-template std::optional<std::vector<std::string>> YamlObject::getList<std::string>() const;
-template std::optional<std::vector<Size>> YamlObject::getList<Size>() const;
+template std::vector<bool> YamlObject::getList<bool>() const;
+template std::vector<double> YamlObject::getList<double>() const;
+template std::vector<int16_t> YamlObject::getList<int16_t>() const;
+template std::vector<uint16_t> YamlObject::getList<uint16_t>() const;
+template std::vector<int32_t> YamlObject::getList<int32_t>() const;
+template std::vector<uint32_t> YamlObject::getList<uint32_t>() const;
+template std::vector<std::string> YamlObject::getList<std::string>() const;
+template std::vector<Size> YamlObject::getList<Size>() const;

 #endif /* __DOXYGEN__ */

diff --git a/test/yaml-parser.cpp b/test/yaml-parser.cpp
index 93ba88b8bcd5..e25944e7f6b8 100644
--- a/test/yaml-parser.cpp
+++ b/test/yaml-parser.cpp
@@ -530,7 +530,7 @@ protected:
 		}

 		const auto &values = firstElement.getList<uint16_t>();
-		if (!values || values->size() != 2 || (*values)[0] != 1 || (*values)[1] != 2) {
+		if (values.empty() || values.size() != 2 || values[0] != 1 || values[1] != 2) {
 			cerr << "getList() failed to return correct vector" << std::endl;
 			return TestFail;
 		}
--
2.37.1



More information about the libcamera-devel mailing list