[libcamera-devel,v4,1/4] use std::optional to handle invalid control values
diff mbox series

Message ID 20220427223004.115381-2-Rauch.Christian@gmx.de
State Superseded
Headers show
Series
  • generate and use fixed-sized Span Control types
Related show

Commit Message

Christian Rauch April 27, 2022, 10:30 p.m. UTC 
Previously, ControlList::get<T>() would use default constructed objects to
indicate that a ControlList does not have the requested Control. This has
several disadvantages: 1) It requires types to be default constructible,
2) it does not differentiate between a default constructed object and an
object that happens to have the same state as a default constructed object.

std::optional<T> additionally stores the information if the object is valid
or not, and therefore is more expressive than a default constructed object.

Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
---
 include/libcamera/controls.h                      |  7 ++++---
 src/cam/main.cpp                                  |  4 ++--
 src/ipa/raspberrypi/raspberrypi.cpp               |  2 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp              |  9 ++++-----
 .../pipeline/raspberrypi/raspberrypi.cpp          | 10 ++++++----
 src/qcam/dng_writer.cpp                           | 15 +++++++++------
 6 files changed, 26 insertions(+), 21 deletions(-)

--
2.34.1

Patch
diff mbox series 

diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
index 665bcac1..363e7809 100644
--- a/include/libcamera/controls.h
+++ b/include/libcamera/controls.h
@@ -13,6 +13,7 @@ 
 #include <string>
 #include <unordered_map>
 #include <vector>
+#include <optional>

 #include <libcamera/base/class.h>
 #include <libcamera/base/span.h>
@@ -167,7 +168,7 @@  public:

 		using V = typename T::value_type;
 		const V *value = reinterpret_cast<const V *>(data().data());
-		return { value, numElements_ };
+		return T{ value, numElements_ };
 	}

 #ifndef __DOXYGEN__
@@ -373,11 +374,11 @@  public:
 	bool contains(unsigned int id) const;

 	template<typename T>
-	T get(const Control<T> &ctrl) const
+	std::optional<T> get(const Control<T> &ctrl) const
 	{
 		const ControlValue *val = find(ctrl.id());
 		if (!val)
-			return T{};
+			return std::nullopt;

 		return val->get<T>();
 	}
diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index c7f664b9..853a78ed 100644
--- a/src/cam/main.cpp
+++ b/src/cam/main.cpp
@@ -293,7 +293,7 @@  std::string CamApp::cameraName(const Camera *camera)
 	 * is only used if the location isn't present or is set to External.
 	 */
 	if (props.contains(properties::Location)) {
-		switch (props.get(properties::Location)) {
+		switch (props.get(properties::Location).value_or(int32_t{})) {
 		case properties::CameraLocationFront:
 			addModel = false;
 			name = "Internal front camera ";
@@ -313,7 +313,7 @@  std::string CamApp::cameraName(const Camera *camera)
 		 * If the camera location is not availble use the camera model
 		 * to build the camera name.
 		 */
-		name = "'" + props.get(properties::Model) + "' ";
+		name = "'" + props.get(properties::Model).value_or(std::string{}) + "' ";
 	}

 	name += "(" + camera->id() + ")";
diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
index 5efd051b..bf54f64f 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -936,7 +936,7 @@  void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)

 void IPARPi::prepareISP(const ISPConfig &data)
 {
-	int64_t frameTimestamp = data.controls.get(controls::SensorTimestamp);
+	int64_t frameTimestamp = data.controls.get(controls::SensorTimestamp).value_or(int64_t{});
 	RPiController::Metadata lastMetadata;
 	Span<uint8_t> embeddedBuffer;

diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index fd989e61..1e9e5081 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -1145,7 +1145,7 @@  int PipelineHandlerIPU3::registerCameras()
 		/* Convert the sensor rotation to a transformation */
 		int32_t rotation = 0;
 		if (data->properties_.contains(properties::Rotation))
-			rotation = data->properties_.get(properties::Rotation);
+			rotation = data->properties_.get(properties::Rotation).value_or(int32_t{});
 		else
 			LOG(IPU3, Warning) << "Rotation control not exposed by "
 					   << cio2->sensor()->id()
@@ -1331,7 +1331,7 @@  void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)
 	request->metadata().set(controls::draft::PipelineDepth, 3);
 	/* \todo Actually apply the scaler crop region to the ImgU. */
 	if (request->controls().contains(controls::ScalerCrop))
-		cropRegion_ = request->controls().get(controls::ScalerCrop);
+		cropRegion_ = request->controls().get(controls::ScalerCrop).value_or(Rectangle{});
 	request->metadata().set(controls::ScalerCrop, cropRegion_);

 	if (frameInfos_.tryComplete(info))
@@ -1424,7 +1424,7 @@  void IPU3CameraData::statBufferReady(FrameBuffer *buffer)
 		return;
 	}

-	ipa_->processStatsBuffer(info->id, request->metadata().get(controls::SensorTimestamp),
+	ipa_->processStatsBuffer(info->id, request->metadata().get(controls::SensorTimestamp).value_or(int64_t{}),
 				 info->statBuffer->cookie(), info->effectiveSensorControls);
 }

@@ -1458,8 +1458,7 @@  void IPU3CameraData::frameStart(uint32_t sequence)
 	if (!request->controls().contains(controls::draft::TestPatternMode))
 		return;

-	const int32_t testPatternMode = request->controls().get(
-		controls::draft::TestPatternMode);
+	const int32_t testPatternMode = request->controls().get(controls::draft::TestPatternMode).value_or(int32_t{});

 	int ret = cio2_.sensor()->setTestPatternMode(
 		static_cast<controls::draft::TestPatternModeEnum>(testPatternMode));
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index acc0beca..ce86ba45 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -365,7 +365,7 @@  CameraConfiguration::Status RPiCameraConfiguration::validate()
 	 * error means the platform can never run. Let's just print a warning
 	 * and continue regardless; the rotation is effectively set to zero.
 	 */
-	int32_t rotation = data_->sensor_->properties().get(properties::Rotation);
+	int32_t rotation = data_->sensor_->properties().get(properties::Rotation).value_or(int32_t{});
 	bool success;
 	Transform rotationTransform = transformFromRotation(rotation, &success);
 	if (!success)
@@ -1696,7 +1696,9 @@  void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &
 	 * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is set).
 	 */
 	if (notifyGainsUnity_ && controls.contains(libcamera::controls::ColourGains)) {
-		libcamera::Span<const float> colourGains = controls.get(libcamera::controls::ColourGains);
+		libcamera::Span<const float> colourGains =
+			controls.get(libcamera::controls::ColourGains).\
+				value_or(libcamera::Span<const float>({ 0, 0 }));
 		/* The control wants linear gains in the order B, Gb, Gr, R. */
 		ControlList ctrls(sensor_->controls());
 		std::array<int32_t, 4> gains{
@@ -2031,7 +2033,7 @@  Rectangle RPiCameraData::scaleIspCrop(const Rectangle &ispCrop) const
 void RPiCameraData::applyScalerCrop(const ControlList &controls)
 {
 	if (controls.contains(controls::ScalerCrop)) {
-		Rectangle nativeCrop = controls.get<Rectangle>(controls::ScalerCrop);
+		Rectangle nativeCrop = controls.get<Rectangle>(controls::ScalerCrop).value_or(Rectangle{});

 		if (!nativeCrop.width || !nativeCrop.height)
 			nativeCrop = { 0, 0, 1, 1 };
@@ -2069,7 +2071,7 @@  void RPiCameraData::fillRequestMetadata(const ControlList &bufferControls,
 					Request *request)
 {
 	request->metadata().set(controls::SensorTimestamp,
-				bufferControls.get(controls::SensorTimestamp));
+				bufferControls.get(controls::SensorTimestamp).value_or(int64_t{}));

 	request->metadata().set(controls::ScalerCrop, scalerCrop_);
 }
diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
index 34c8df5a..e119ca52 100644
--- a/src/qcam/dng_writer.cpp
+++ b/src/qcam/dng_writer.cpp
@@ -392,7 +392,7 @@  int DNGWriter::write(const char *filename, const Camera *camera,
 	TIFFSetField(tif, TIFFTAG_MAKE, "libcamera");

 	if (cameraProperties.contains(properties::Model)) {
-		std::string model = cameraProperties.get(properties::Model);
+		std::string model = cameraProperties.get(properties::Model).value_or(std::string{});
 		TIFFSetField(tif, TIFFTAG_MODEL, model.c_str());
 		/* \todo set TIFFTAG_UNIQUECAMERAMODEL. */
 	}
@@ -438,7 +438,8 @@  int DNGWriter::write(const char *filename, const Camera *camera,
 	const double eps = 1e-2;

 	if (metadata.contains(controls::ColourGains)) {
-		Span<const float> const &colourGains = metadata.get(controls::ColourGains);
+		Span<const float> const &colourGains =
+			metadata.get(controls::ColourGains).value_or(libcamera::Span<const float>({ 0, 0 }));
 		if (colourGains[0] > eps && colourGains[1] > eps) {
 			wbGain = Matrix3d::diag(colourGains[0], 1, colourGains[1]);
 			neutral[0] = 1.0 / colourGains[0]; /* red */
@@ -446,7 +447,8 @@  int DNGWriter::write(const char *filename, const Camera *camera,
 		}
 	}
 	if (metadata.contains(controls::ColourCorrectionMatrix)) {
-		Span<const float> const &coeffs = metadata.get(controls::ColourCorrectionMatrix);
+		Span<const float> const &coeffs =
+			metadata.get(controls::ColourCorrectionMatrix).value_or(Span<const float>({ 0, 0, 0, 0, 0, 0, 0, 0, 0 }));
 		Matrix3d ccmSupplied(coeffs);
 		if (ccmSupplied.determinant() > eps)
 			ccm = ccmSupplied;
@@ -515,7 +517,8 @@  int DNGWriter::write(const char *filename, const Camera *camera,
 	uint32_t whiteLevel = (1 << info->bitsPerSample) - 1;

 	if (metadata.contains(controls::SensorBlackLevels)) {
-		Span<const int32_t> levels = metadata.get(controls::SensorBlackLevels);
+		Span<const int32_t> levels =
+			metadata.get(controls::SensorBlackLevels).value_or(Span<const int32_t>({ 0, 0, 0, 0 }));

 		/*
 		 * The black levels control is specified in R, Gr, Gb, B order.
@@ -593,13 +596,13 @@  int DNGWriter::write(const char *filename, const Camera *camera,
 	TIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);

 	if (metadata.contains(controls::AnalogueGain)) {
-		float gain = metadata.get(controls::AnalogueGain);
+		float gain = metadata.get(controls::AnalogueGain).value_or(float{});
 		uint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f);
 		TIFFSetField(tif, EXIFTAG_ISOSPEEDRATINGS, 1, &iso);
 	}

 	if (metadata.contains(controls::ExposureTime)) {
-		float exposureTime = metadata.get(controls::ExposureTime) / 1e6;
+		float exposureTime = metadata.get(controls::ExposureTime).value_or(float{}) / 1e6;
 		TIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);
 	}