[libcamera-devel] [PATCH] libcamera: controls: Avoid double lookups

Laurent Pinchart laurent.pinchart at ideasonboard.com
Mon Jul 11 02:39:18 CEST 2022


Now that the ControlList::get() function returns an instance of
std::optional<>, we can replace the ControlList::contains() calls with a
nullopt check on the return value of get(). This avoids double lookups
of controls through the code base.

Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
---
This patch applies on top of "[PATCH v10 1/2] libcamera: controls: Use
std::optional to handle invalid control values".
---
 src/android/camera_capabilities.cpp           | 11 ++---
 src/android/camera_device.cpp                 | 46 +++++++++----------
 src/android/camera_hal_manager.cpp            |  6 +--
 src/cam/main.cpp                              |  5 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp          | 25 +++++-----
 .../pipeline/raspberrypi/raspberrypi.cpp      | 14 +++---
 src/qcam/dng_writer.cpp                       | 35 +++++++-------
 7 files changed, 68 insertions(+), 74 deletions(-)

diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
index 5304b2da1f93..94ebfc860040 100644
--- a/src/android/camera_capabilities.cpp
+++ b/src/android/camera_capabilities.cpp
@@ -1049,8 +1049,8 @@ int CameraCapabilities::initializeStaticMetadata()
 					  pixelArraySize);
 	}
 
-	if (properties.contains(properties::UnitCellSize)) {
-		const auto &cellSize = properties.get<Size>(properties::UnitCellSize);
+	const auto &cellSize = properties.get<Size>(properties::UnitCellSize);
+	if (cellSize) {
 		std::array<float, 2> physicalSize{
 			cellSize->width * pixelArraySize[0] / 1e6f,
 			cellSize->height * pixelArraySize[1] / 1e6f
@@ -1079,11 +1079,10 @@ int CameraCapabilities::initializeStaticMetadata()
 				  sensitivityRange);
 
 	/* Report the color filter arrangement if the camera reports it. */
-	if (properties.contains(properties::draft::ColorFilterArrangement)) {
-		uint8_t filterArr = *properties.get(properties::draft::ColorFilterArrangement);
+	const auto &filterArr = properties.get(properties::draft::ColorFilterArrangement);
+	if (filterArr)
 		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,
-					  filterArr);
-	}
+					  *filterArr);
 
 	const auto &exposureInfo = controlsInfo.find(&controls::ExposureTime);
 	if (exposureInfo != controlsInfo.end()) {
diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
index 4662123210c2..b20e389b9d30 100644
--- a/src/android/camera_device.cpp
+++ b/src/android/camera_device.cpp
@@ -305,9 +305,9 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)
 	 */
 	const ControlList &properties = camera_->properties();
 
-	if (properties.contains(properties::Location)) {
-		int32_t location = *properties.get(properties::Location);
-		switch (location) {
+	const auto &location = properties.get(properties::Location);
+	if (location) {
+		switch (*location) {
 		case properties::CameraLocationFront:
 			facing_ = CAMERA_FACING_FRONT;
 			break;
@@ -355,9 +355,9 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)
 	 * value for clockwise direction as required by the Android orientation
 	 * metadata.
 	 */
-	if (properties.contains(properties::Rotation)) {
-		int rotation = *properties.get(properties::Rotation);
-		orientation_ = (360 - rotation) % 360;
+	const auto &rotation = properties.get(properties::Rotation);
+	if (rotation) {
+		orientation_ = (360 - *rotation) % 360;
 		if (cameraConfigData && cameraConfigData->rotation != -1 &&
 		    orientation_ != cameraConfigData->rotation) {
 			LOG(HAL, Warning)
@@ -1564,25 +1564,24 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
 	const int64_t timestamp = metadata.get(controls::SensorTimestamp).value_or(0);
 	resultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, timestamp);
 
-	if (metadata.contains(controls::draft::PipelineDepth)) {
-		uint8_t pipeline_depth = *metadata.get<int32_t>(controls::draft::PipelineDepth);
+	const auto &pipelineDepth = metadata.get(controls::draft::PipelineDepth);
+	if (pipelineDepth)
 		resultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH,
-					 pipeline_depth);
-	}
+					 *pipelineDepth);
 
-	if (metadata.contains(controls::ExposureTime)) {
-		int64_t exposure = *metadata.get(controls::ExposureTime) * 1000ULL;
-		resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);
-	}
+	const auto &exposureTime = metadata.get(controls::ExposureTime);
+	if (exposureTime)
+		resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME,
+					 *exposureTime * 1000ULL);
 
-	if (metadata.contains(controls::FrameDuration)) {
-		int64_t duration = *metadata.get(controls::FrameDuration) * 1000;
+	const auto &frameDuration = metadata.get(controls::FrameDuration);
+	if (frameDuration)
 		resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,
-					 duration);
-	}
+					 *frameDuration * 1000);
 
-	if (metadata.contains(controls::ScalerCrop)) {
-		Rectangle crop = *metadata.get(controls::ScalerCrop);
+	const auto &scalerCrop = metadata.get(controls::ScalerCrop);
+	if (scalerCrop) {
+		const Rectangle &crop = *scalerCrop;
 		int32_t cropRect[] = {
 			crop.x, crop.y, static_cast<int32_t>(crop.width),
 			static_cast<int32_t>(crop.height),
@@ -1590,11 +1589,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
 		resultMetadata->addEntry(ANDROID_SCALER_CROP_REGION, cropRect);
 	}
 
-	if (metadata.contains(controls::draft::TestPatternMode)) {
-		const int32_t testPatternMode = *metadata.get(controls::draft::TestPatternMode);
+	const auto &testPatternMode = metadata.get(controls::draft::TestPatternMode);
+	if (testPatternMode)
 		resultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,
-					 testPatternMode);
-	}
+					 *testPatternMode);
 
 	/*
 	 * Return the result metadata pack even is not valid: get() will return
diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
index 0bffe96f6dbc..7512cc4e11a4 100644
--- a/src/android/camera_hal_manager.cpp
+++ b/src/android/camera_hal_manager.cpp
@@ -228,11 +228,7 @@ void CameraHalManager::cameraRemoved(std::shared_ptr<Camera> cam)
 
 int32_t CameraHalManager::cameraLocation(const Camera *cam)
 {
-	const ControlList &properties = cam->properties();
-	if (!properties.contains(properties::Location))
-		return -1;
-
-	return *properties.get(properties::Location);
+	return cam->properties().get(properties::Location).value_or(-1);
 }
 
 CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)
diff --git a/src/cam/main.cpp b/src/cam/main.cpp
index d8115cd86825..13a3d9b93df8 100644
--- a/src/cam/main.cpp
+++ b/src/cam/main.cpp
@@ -300,8 +300,9 @@ std::string CamApp::cameraName(const Camera *camera)
 	 * Construct the name from the camera location, model and ID. The model
 	 * is only used if the location isn't present or is set to External.
 	 */
-	if (props.contains(properties::Location)) {
-		switch (*props.get(properties::Location)) {
+	const auto &location = props.get(properties::Location);
+	if (location) {
+		switch (*location) {
 		case properties::CameraLocationFront:
 			addModel = false;
 			name = "Internal front camera ";
diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
index 43db7b68dac8..d60f20b08e27 100644
--- a/src/libcamera/pipeline/ipu3/ipu3.cpp
+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
@@ -1143,18 +1143,17 @@ int PipelineHandlerIPU3::registerCameras()
 						 &IPU3CameraData::frameStart);
 
 		/* Convert the sensor rotation to a transformation */
-		int32_t rotation = 0;
-		if (data->properties_.contains(properties::Rotation))
-			rotation = *(data->properties_.get(properties::Rotation));
-		else
+		const auto &rotation = data->properties_.get(properties::Rotation);
+		if (!rotation)
 			LOG(IPU3, Warning) << "Rotation control not exposed by "
 					   << cio2->sensor()->id()
 					   << ". Assume rotation 0";
 
+		int32_t rotationValue = rotation.value_or(0);
 		bool success;
-		data->rotationTransform_ = transformFromRotation(rotation, &success);
+		data->rotationTransform_ = transformFromRotation(rotationValue, &success);
 		if (!success)
-			LOG(IPU3, Warning) << "Invalid rotation of " << rotation
+			LOG(IPU3, Warning) << "Invalid rotation of " << rotationValue
 					   << " degrees: ignoring";
 
 		ControlList ctrls = cio2->sensor()->getControls({ V4L2_CID_HFLIP });
@@ -1330,8 +1329,9 @@ 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));
+	const auto &scalerCrop = request->controls().get(controls::ScalerCrop);
+	if (scalerCrop)
+		cropRegion_ = *scalerCrop;
 	request->metadata().set(controls::ScalerCrop, cropRegion_);
 
 	if (frameInfos_.tryComplete(info))
@@ -1455,13 +1455,12 @@ void IPU3CameraData::frameStart(uint32_t sequence)
 	Request *request = processingRequests_.front();
 	processingRequests_.pop();
 
-	if (!request->controls().contains(controls::draft::TestPatternMode))
+	const auto &testPatternMode = request->controls().get(controls::draft::TestPatternMode);
+	if (!testPatternMode)
 		return;
 
-	const int32_t testPatternMode = *(request->controls().get(controls::draft::TestPatternMode));
-
 	int ret = cio2_.sensor()->setTestPatternMode(
-		static_cast<controls::draft::TestPatternModeEnum>(testPatternMode));
+		static_cast<controls::draft::TestPatternModeEnum>(*testPatternMode));
 	if (ret) {
 		LOG(IPU3, Error) << "Failed to set test pattern mode: "
 				 << ret;
@@ -1469,7 +1468,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)
 	}
 
 	request->metadata().set(controls::draft::TestPatternMode,
-				testPatternMode);
+				*testPatternMode);
 }
 
 REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
index 28f054cb84c8..2e19531eecc4 100644
--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
@@ -1716,16 +1716,15 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &
 	 * Inform the sensor of the latest colour gains if it has the
 	 * 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);
+	const auto &colourGains = controls.get(libcamera::controls::ColourGains);
+	if (notifyGainsUnity_ && colourGains) {
 		/* The control wants linear gains in the order B, Gb, Gr, R. */
 		ControlList ctrls(sensor_->controls());
 		std::array<int32_t, 4> gains{
-			static_cast<int32_t>(colourGains[1] * *notifyGainsUnity_),
+			static_cast<int32_t>((*colourGains)[1] * *notifyGainsUnity_),
 			*notifyGainsUnity_,
 			*notifyGainsUnity_,
-			static_cast<int32_t>(colourGains[0] * *notifyGainsUnity_)
+			static_cast<int32_t>((*colourGains)[0] * *notifyGainsUnity_)
 		};
 		ctrls.set(V4L2_CID_NOTIFY_GAINS, Span<const int32_t>{ gains });
 
@@ -2052,8 +2051,9 @@ 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);
+	const auto &scalerCrop = controls.get<Rectangle>(controls::ScalerCrop);
+	if (scalerCrop) {
+		Rectangle nativeCrop = *scalerCrop;
 
 		if (!nativeCrop.width || !nativeCrop.height)
 			nativeCrop = { 0, 0, 1, 1 };
diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
index 4b5d8276564f..5896485cda5b 100644
--- a/src/qcam/dng_writer.cpp
+++ b/src/qcam/dng_writer.cpp
@@ -391,9 +391,9 @@ int DNGWriter::write(const char *filename, const Camera *camera,
 	TIFFSetField(tif, TIFFTAG_FILLORDER, FILLORDER_MSB2LSB);
 	TIFFSetField(tif, TIFFTAG_MAKE, "libcamera");
 
-	if (cameraProperties.contains(properties::Model)) {
-		std::string model = *cameraProperties.get(properties::Model);
-		TIFFSetField(tif, TIFFTAG_MODEL, model.c_str());
+	const auto &model = cameraProperties.get(properties::Model);
+	if (model) {
+		TIFFSetField(tif, TIFFTAG_MODEL, model->c_str());
 		/* \todo set TIFFTAG_UNIQUECAMERAMODEL. */
 	}
 
@@ -437,16 +437,18 @@ int DNGWriter::write(const char *filename, const Camera *camera,
 	 */
 	const double eps = 1e-2;
 
-	if (metadata.contains(controls::ColourGains)) {
-		const auto &colourGains = metadata.get(controls::ColourGains);
+	const auto &colourGains = metadata.get(controls::ColourGains);
+	if (colourGains) {
 		if ((*colourGains)[0] > eps && (*colourGains)[1] > eps) {
 			wbGain = Matrix3d::diag((*colourGains)[0], 1, (*colourGains)[1]);
 			neutral[0] = 1.0 / (*colourGains)[0]; /* red */
 			neutral[2] = 1.0 / (*colourGains)[1]; /* blue */
 		}
 	}
-	if (metadata.contains(controls::ColourCorrectionMatrix)) {
-		Matrix3d ccmSupplied(*metadata.get(controls::ColourCorrectionMatrix));
+
+	const auto &ccmControl = metadata.get(controls::ColourCorrectionMatrix);
+	if (ccmControl) {
+		Matrix3d ccmSupplied(*ccmControl);
 		if (ccmSupplied.determinant() > eps)
 			ccm = ccmSupplied;
 	}
@@ -513,9 +515,9 @@ int DNGWriter::write(const char *filename, const Camera *camera,
 	float blackLevel[] = { 0.0f, 0.0f, 0.0f, 0.0f };
 	uint32_t whiteLevel = (1 << info->bitsPerSample) - 1;
 
-	if (metadata.contains(controls::SensorBlackLevels)) {
-		Span<const int32_t> levels =
-			*metadata.get(controls::SensorBlackLevels);
+	const auto &blackLevels = metadata.get(controls::SensorBlackLevels);
+	if (blackLevels) {
+		Span<const int32_t> levels = *blackLevels;
 
 		/*
 		 * The black levels control is specified in R, Gr, Gb, B order.
@@ -592,16 +594,15 @@ int DNGWriter::write(const char *filename, const Camera *camera,
 	TIFFSetField(tif, EXIFTAG_DATETIMEORIGINAL, strTime);
 	TIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);
 
-	if (metadata.contains(controls::AnalogueGain)) {
-		float gain = *metadata.get(controls::AnalogueGain);
-		uint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f);
+	const auto &analogGain = metadata.get(controls::AnalogueGain);
+	if (analogGain) {
+		uint16_t iso = std::min(std::max(*analogGain * 100, 0.0f), 65535.0f);
 		TIFFSetField(tif, EXIFTAG_ISOSPEEDRATINGS, 1, &iso);
 	}
 
-	if (metadata.contains(controls::ExposureTime)) {
-		float exposureTime = *metadata.get(controls::ExposureTime) / 1e6;
-		TIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);
-	}
+	const auto &exposureTime = metadata.get(controls::ExposureTime);
+	if (exposureTime)
+		TIFFSetField(tif, EXIFTAG_EXPOSURETIME, *exposureTime / 1e6);
 
 	TIFFWriteCustomDirectory(tif, &exifIFDOffset);
 
-- 
Regards,

Laurent Pinchart



More information about the libcamera-devel mailing list