[libcamera-devel,v8,1/4] libcamera: controls: Use std::optional to handle invalid control values

16190 diff mbox series

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>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
---
 include/libcamera/controls.h                  |  5 +++--
 src/android/camera_capabilities.cpp           | 12 +++++-----
 src/android/camera_device.cpp                 | 21 +++++++++---------
 src/android/camera_hal_manager.cpp            |  2 +-
 src/cam/main.cpp                              |  4 ++--
 src/ipa/raspberrypi/raspberrypi.cpp           |  2 +-
 src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 ++++----
 .../pipeline/raspberrypi/raspberrypi.cpp      |  9 ++++----
 src/qcam/dng_writer.cpp                       | 22 +++++++++----------
 9 files changed, 43 insertions(+), 43 deletions(-)

--
2.34.1

Hi Christian,

Thank you for the patch.

On Fri, Jun 10, 2022 at 01:03:35PM +0100, Christian Rauch via libcamera-devel wrote:
> 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>
> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> ---
>  include/libcamera/controls.h                  |  5 +++--
>  src/android/camera_capabilities.cpp           | 12 +++++-----
>  src/android/camera_device.cpp                 | 21 +++++++++---------
>  src/android/camera_hal_manager.cpp            |  2 +-
>  src/cam/main.cpp                              |  4 ++--
>  src/ipa/raspberrypi/raspberrypi.cpp           |  2 +-
>  src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 ++++----
>  .../pipeline/raspberrypi/raspberrypi.cpp      |  9 ++++----
>  src/qcam/dng_writer.cpp                       | 22 +++++++++----------
>  9 files changed, 43 insertions(+), 43 deletions(-)
> 
> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> index 665bcac1..192be784 100644
> --- a/include/libcamera/controls.h
> +++ b/include/libcamera/controls.h
> @@ -8,6 +8,7 @@
>  #pragma once
> 
>  #include <assert.h>
> +#include <optional>
>  #include <set>
>  #include <stdint.h>
>  #include <string>
> @@ -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>();

I think one of the perks of using std::optional here is that we could
return a reference to the value instead of copying it. To avoid a v9,
I'll send a patch on top.

>  	}
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 6f197eb8..5304b2da 100644
> --- a/src/android/camera_capabilities.cpp
> +++ b/src/android/camera_capabilities.cpp
> @@ -1042,7 +1042,7 @@ int CameraCapabilities::initializeStaticMetadata()
>  	/* Sensor static metadata. */
>  	std::array<int32_t, 2> pixelArraySize;
>  	{
> -		const Size &size = properties.get(properties::PixelArraySize);
> +		const Size &size = properties.get(properties::PixelArraySize).value_or(Size{});
>  		pixelArraySize[0] = size.width;
>  		pixelArraySize[1] = size.height;
>  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,
> @@ -1050,10 +1050,10 @@ int CameraCapabilities::initializeStaticMetadata()
>  	}
> 
>  	if (properties.contains(properties::UnitCellSize)) {
> -		const Size &cellSize = properties.get<Size>(properties::UnitCellSize);
> +		const auto &cellSize = properties.get<Size>(properties::UnitCellSize);

We can avoid the double-lookup here and in several locations below. I'll
send a patch on top too.

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

>  		std::array<float, 2> physicalSize{
> -			cellSize.width * pixelArraySize[0] / 1e6f,
> -			cellSize.height * pixelArraySize[1] / 1e6f
> +			cellSize->width * pixelArraySize[0] / 1e6f,
> +			cellSize->height * pixelArraySize[1] / 1e6f
>  		};
>  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PHYSICAL_SIZE,
>  					  physicalSize);
> @@ -1061,7 +1061,7 @@ int CameraCapabilities::initializeStaticMetadata()
> 
>  	{
>  		const Span<const Rectangle> &rects =
> -			properties.get(properties::PixelArrayActiveAreas);
> +			properties.get(properties::PixelArrayActiveAreas).value_or(Span<const Rectangle>{});
>  		std::vector<int32_t> data{
>  			static_cast<int32_t>(rects[0].x),
>  			static_cast<int32_t>(rects[0].y),
> @@ -1080,7 +1080,7 @@ int CameraCapabilities::initializeStaticMetadata()
> 
>  	/* Report the color filter arrangement if the camera reports it. */
>  	if (properties.contains(properties::draft::ColorFilterArrangement)) {
> -		uint8_t filterArr = properties.get(properties::draft::ColorFilterArrangement);
> +		uint8_t filterArr = *properties.get(properties::draft::ColorFilterArrangement);
>  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,
>  					  filterArr);
>  	}
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 8e804d4d..ec117101 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -305,7 +305,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)
>  	const ControlList &properties = camera_->properties();
> 
>  	if (properties.contains(properties::Location)) {
> -		int32_t location = properties.get(properties::Location);
> +		int32_t location = *properties.get(properties::Location);
>  		switch (location) {
>  		case properties::CameraLocationFront:
>  			facing_ = CAMERA_FACING_FRONT;
> @@ -355,7 +355,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)
>  	 * metadata.
>  	 */
>  	if (properties.contains(properties::Rotation)) {
> -		int rotation = properties.get(properties::Rotation);
> +		int rotation = *properties.get(properties::Rotation);
>  		orientation_ = (360 - rotation) % 360;
>  		if (cameraConfigData && cameraConfigData->rotation != -1 &&
>  		    orientation_ != cameraConfigData->rotation) {
> @@ -1094,7 +1094,8 @@ void CameraDevice::requestComplete(Request *request)
>  	 * as soon as possible, earlier than request completion time.
>  	 */
>  	uint64_t sensorTimestamp = static_cast<uint64_t>(request->metadata()
> -							 .get(controls::SensorTimestamp));
> +								 .get(controls::SensorTimestamp)
> +								 .value_or(0));
>  	notifyShutter(descriptor->frameNumber_, sensorTimestamp);
> 
>  	LOG(HAL, Debug) << "Request " << request->cookie() << " completed with "
> @@ -1473,29 +1474,28 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>  				 rolling_shutter_skew);
> 
>  	/* Add metadata tags reported by libcamera. */
> -	const int64_t timestamp = metadata.get(controls::SensorTimestamp);
> +	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);
> +		uint8_t pipeline_depth = *metadata.get<int32_t>(controls::draft::PipelineDepth);
>  		resultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH,
>  					 pipeline_depth);
>  	}
> 
>  	if (metadata.contains(controls::ExposureTime)) {
> -		int64_t exposure = metadata.get(controls::ExposureTime) * 1000ULL;
> +		int64_t exposure = *metadata.get(controls::ExposureTime) * 1000ULL;
>  		resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);
>  	}
> 
>  	if (metadata.contains(controls::FrameDuration)) {
> -		int64_t duration = metadata.get(controls::FrameDuration) * 1000;
> +		int64_t duration = *metadata.get(controls::FrameDuration) * 1000;
>  		resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,
>  					 duration);
>  	}
> 
>  	if (metadata.contains(controls::ScalerCrop)) {
> -		Rectangle crop = metadata.get(controls::ScalerCrop);
> +		Rectangle crop = *metadata.get(controls::ScalerCrop);
>  		int32_t cropRect[] = {
>  			crop.x, crop.y, static_cast<int32_t>(crop.width),
>  			static_cast<int32_t>(crop.height),
> @@ -1504,8 +1504,7 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>  	}
> 
>  	if (metadata.contains(controls::draft::TestPatternMode)) {
> -		const int32_t testPatternMode =
> -			metadata.get(controls::draft::TestPatternMode);
> +		const int32_t testPatternMode = *metadata.get(controls::draft::TestPatternMode);
>  		resultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,
>  					 testPatternMode);
>  	}
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index 5f7bfe26..0bffe96f 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -232,7 +232,7 @@ int32_t CameraHalManager::cameraLocation(const Camera *cam)
>  	if (!properties.contains(properties::Location))
>  		return -1;
> 
> -	return properties.get(properties::Location);
> +	return *properties.get(properties::Location);
>  }
> 
>  CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)
> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> index 79875ed7..d8115cd8 100644
> --- a/src/cam/main.cpp
> +++ b/src/cam/main.cpp
> @@ -301,7 +301,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)) {
>  		case properties::CameraLocationFront:
>  			addModel = false;
>  			name = "Internal front camera ";
> @@ -321,7 +321,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) + "' ";
>  	}
> 
>  	name += "(" + camera->id() + ")";
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 3b126bb5..f65a0680 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -939,7 +939,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(0);
>  	RPiController::Metadata lastMetadata;
>  	Span<uint8_t> embeddedBuffer;
> 
> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> index b7dda282..43db7b68 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));
>  		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));
>  	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(0),
>  				 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));
> 
>  	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 adc397e8..a62afdd4 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(0);
>  	bool success;
>  	Transform rotationTransform = transformFromRotation(rotation, &success);
>  	if (!success)
> @@ -1706,7 +1706,8 @@ 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);
>  		/* The control wants linear gains in the order B, Gb, Gr, R. */
>  		ControlList ctrls(sensor_->controls());
>  		std::array<int32_t, 4> gains{
> @@ -2041,7 +2042,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);
> 
>  		if (!nativeCrop.width || !nativeCrop.height)
>  			nativeCrop = { 0, 0, 1, 1 };
> @@ -2079,7 +2080,7 @@ void RPiCameraData::fillRequestMetadata(const ControlList &bufferControls,
>  					Request *request)
>  {
>  	request->metadata().set(controls::SensorTimestamp,
> -				bufferControls.get(controls::SensorTimestamp));
> +				bufferControls.get(controls::SensorTimestamp).value_or(0));
> 
>  	request->metadata().set(controls::ScalerCrop, scalerCrop_);
>  }
> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
> index 34c8df5a..4b5d8276 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);
>  		TIFFSetField(tif, TIFFTAG_MODEL, model.c_str());
>  		/* \todo set TIFFTAG_UNIQUECAMERAMODEL. */
>  	}
> @@ -438,16 +438,15 @@ 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);
> -		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 */
> +		const auto &colourGains = metadata.get(controls::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)) {
> -		Span<const float> const &coeffs = metadata.get(controls::ColourCorrectionMatrix);
> -		Matrix3d ccmSupplied(coeffs);
> +		Matrix3d ccmSupplied(*metadata.get(controls::ColourCorrectionMatrix));
>  		if (ccmSupplied.determinant() > eps)
>  			ccm = ccmSupplied;
>  	}
> @@ -515,7 +514,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);
> 
>  		/*
>  		 * The black levels control is specified in R, Gr, Gb, B order.
> @@ -593,13 +593,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);
>  		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) / 1e6;
>  		TIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);
>  	}
>

On Mon, Jul 04, 2022 at 12:45:19AM +0300, Laurent Pinchart wrote:
> Hi Christian,
> 
> Thank you for the patch.
> 
> On Fri, Jun 10, 2022 at 01:03:35PM +0100, Christian Rauch via libcamera-devel wrote:
> > 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>
> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> > ---
> >  include/libcamera/controls.h                  |  5 +++--
> >  src/android/camera_capabilities.cpp           | 12 +++++-----
> >  src/android/camera_device.cpp                 | 21 +++++++++---------
> >  src/android/camera_hal_manager.cpp            |  2 +-
> >  src/cam/main.cpp                              |  4 ++--
> >  src/ipa/raspberrypi/raspberrypi.cpp           |  2 +-
> >  src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 ++++----
> >  .../pipeline/raspberrypi/raspberrypi.cpp      |  9 ++++----
> >  src/qcam/dng_writer.cpp                       | 22 +++++++++----------
> >  9 files changed, 43 insertions(+), 43 deletions(-)
> > 
> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> > index 665bcac1..192be784 100644
> > --- a/include/libcamera/controls.h
> > +++ b/include/libcamera/controls.h
> > @@ -8,6 +8,7 @@
> >  #pragma once
> > 
> >  #include <assert.h>
> > +#include <optional>
> >  #include <set>
> >  #include <stdint.h>
> >  #include <string>
> > @@ -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>();
> 
> I think one of the perks of using std::optional here is that we could
> return a reference to the value instead of copying it. To avoid a v9,
> I'll send a patch on top.
> 
> >  	}
> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > index 6f197eb8..5304b2da 100644
> > --- a/src/android/camera_capabilities.cpp
> > +++ b/src/android/camera_capabilities.cpp
> > @@ -1042,7 +1042,7 @@ int CameraCapabilities::initializeStaticMetadata()
> >  	/* Sensor static metadata. */
> >  	std::array<int32_t, 2> pixelArraySize;
> >  	{
> > -		const Size &size = properties.get(properties::PixelArraySize);
> > +		const Size &size = properties.get(properties::PixelArraySize).value_or(Size{});
> >  		pixelArraySize[0] = size.width;
> >  		pixelArraySize[1] = size.height;
> >  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,
> > @@ -1050,10 +1050,10 @@ int CameraCapabilities::initializeStaticMetadata()
> >  	}
> > 
> >  	if (properties.contains(properties::UnitCellSize)) {
> > -		const Size &cellSize = properties.get<Size>(properties::UnitCellSize);
> > +		const auto &cellSize = properties.get<Size>(properties::UnitCellSize);
> 
> We can avoid the double-lookup here and in several locations below. I'll
> send a patch on top too.
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Turns out I spoke a bit too fast :-( When compiling with gcc 10.3.1, I'm
getting

[27/218] Compiling C++ object src/android/libcamera-hal.so.p/camera_capabilities.cpp.o
FAILED: src/android/libcamera-hal.so.p/camera_capabilities.cpp.o
g++-10.3.1 -Isrc/android/libcamera-hal.so.p -Isrc/android -I../../src/android -I../../include/android/hardware/libhardware/include -I../../include/android/metadata -I../../include/android/system/core/include -Iinclude -I../../include -I../../subprojects/libyuv/include -Isubprojects/libyuv/__CMake_build -I../../subprojects/libyuv/__CMake_build -Isubprojects/libyuv -I../../subprojects/libyuv -Iinclude/libcamera/ipa -Iinclude/libcamera -I/usr/include/libexif -fdiagnostics-color=always -fsanitize=address -fno-omit-frame-pointer -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O3 -Wshadow -include config.h -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/android/libcamera-hal.so.p/camera_capabilities.cpp.o -MF src/android/libcamera-hal.so.p/camera_capabilities.cpp.o.d -o src/android/libcamera-hal.so.p/camera_capabilities.cpp.o -c ../../src/android/camera_capabilities.cpp
In file included from ../../include/libcamera/camera.h:20,
                 from ../../src/android/camera_capabilities.h:17,
                 from ../../src/android/camera_capabilities.cpp:8:
../../include/libcamera/controls.h: In member function ‘int CameraCapabilities::initializeStaticMetadata()’:
../../include/libcamera/controls.h:381:16: error: ‘<anonymous>’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
  381 |    return std::nullopt;
      |                ^~~~~~~
../../include/libcamera/controls.h:381:16: error: ‘*((void*)&<anonymous> +4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]

The following change fixes it:

diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
index 5304b2da1f93..429054ae3633 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

If we do that, I'd like to apply the same change through this patch. I
can post a v9 of this patch with this tomorrow if you don't beat me to
it.

> >  		std::array<float, 2> physicalSize{
> > -			cellSize.width * pixelArraySize[0] / 1e6f,
> > -			cellSize.height * pixelArraySize[1] / 1e6f
> > +			cellSize->width * pixelArraySize[0] / 1e6f,
> > +			cellSize->height * pixelArraySize[1] / 1e6f
> >  		};
> >  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PHYSICAL_SIZE,
> >  					  physicalSize);
> > @@ -1061,7 +1061,7 @@ int CameraCapabilities::initializeStaticMetadata()
> > 
> >  	{
> >  		const Span<const Rectangle> &rects =
> > -			properties.get(properties::PixelArrayActiveAreas);
> > +			properties.get(properties::PixelArrayActiveAreas).value_or(Span<const Rectangle>{});
> >  		std::vector<int32_t> data{
> >  			static_cast<int32_t>(rects[0].x),
> >  			static_cast<int32_t>(rects[0].y),
> > @@ -1080,7 +1080,7 @@ int CameraCapabilities::initializeStaticMetadata()
> > 
> >  	/* Report the color filter arrangement if the camera reports it. */
> >  	if (properties.contains(properties::draft::ColorFilterArrangement)) {
> > -		uint8_t filterArr = properties.get(properties::draft::ColorFilterArrangement);
> > +		uint8_t filterArr = *properties.get(properties::draft::ColorFilterArrangement);
> >  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,
> >  					  filterArr);
> >  	}
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index 8e804d4d..ec117101 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -305,7 +305,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)
> >  	const ControlList &properties = camera_->properties();
> > 
> >  	if (properties.contains(properties::Location)) {
> > -		int32_t location = properties.get(properties::Location);
> > +		int32_t location = *properties.get(properties::Location);
> >  		switch (location) {
> >  		case properties::CameraLocationFront:
> >  			facing_ = CAMERA_FACING_FRONT;
> > @@ -355,7 +355,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)
> >  	 * metadata.
> >  	 */
> >  	if (properties.contains(properties::Rotation)) {
> > -		int rotation = properties.get(properties::Rotation);
> > +		int rotation = *properties.get(properties::Rotation);
> >  		orientation_ = (360 - rotation) % 360;
> >  		if (cameraConfigData && cameraConfigData->rotation != -1 &&
> >  		    orientation_ != cameraConfigData->rotation) {
> > @@ -1094,7 +1094,8 @@ void CameraDevice::requestComplete(Request *request)
> >  	 * as soon as possible, earlier than request completion time.
> >  	 */
> >  	uint64_t sensorTimestamp = static_cast<uint64_t>(request->metadata()
> > -							 .get(controls::SensorTimestamp));
> > +								 .get(controls::SensorTimestamp)
> > +								 .value_or(0));
> >  	notifyShutter(descriptor->frameNumber_, sensorTimestamp);
> > 
> >  	LOG(HAL, Debug) << "Request " << request->cookie() << " completed with "
> > @@ -1473,29 +1474,28 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> >  				 rolling_shutter_skew);
> > 
> >  	/* Add metadata tags reported by libcamera. */
> > -	const int64_t timestamp = metadata.get(controls::SensorTimestamp);
> > +	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);
> > +		uint8_t pipeline_depth = *metadata.get<int32_t>(controls::draft::PipelineDepth);
> >  		resultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH,
> >  					 pipeline_depth);
> >  	}
> > 
> >  	if (metadata.contains(controls::ExposureTime)) {
> > -		int64_t exposure = metadata.get(controls::ExposureTime) * 1000ULL;
> > +		int64_t exposure = *metadata.get(controls::ExposureTime) * 1000ULL;
> >  		resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);
> >  	}
> > 
> >  	if (metadata.contains(controls::FrameDuration)) {
> > -		int64_t duration = metadata.get(controls::FrameDuration) * 1000;
> > +		int64_t duration = *metadata.get(controls::FrameDuration) * 1000;
> >  		resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,
> >  					 duration);
> >  	}
> > 
> >  	if (metadata.contains(controls::ScalerCrop)) {
> > -		Rectangle crop = metadata.get(controls::ScalerCrop);
> > +		Rectangle crop = *metadata.get(controls::ScalerCrop);
> >  		int32_t cropRect[] = {
> >  			crop.x, crop.y, static_cast<int32_t>(crop.width),
> >  			static_cast<int32_t>(crop.height),
> > @@ -1504,8 +1504,7 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> >  	}
> > 
> >  	if (metadata.contains(controls::draft::TestPatternMode)) {
> > -		const int32_t testPatternMode =
> > -			metadata.get(controls::draft::TestPatternMode);
> > +		const int32_t testPatternMode = *metadata.get(controls::draft::TestPatternMode);
> >  		resultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,
> >  					 testPatternMode);
> >  	}
> > diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> > index 5f7bfe26..0bffe96f 100644
> > --- a/src/android/camera_hal_manager.cpp
> > +++ b/src/android/camera_hal_manager.cpp
> > @@ -232,7 +232,7 @@ int32_t CameraHalManager::cameraLocation(const Camera *cam)
> >  	if (!properties.contains(properties::Location))
> >  		return -1;
> > 
> > -	return properties.get(properties::Location);
> > +	return *properties.get(properties::Location);
> >  }
> > 
> >  CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)
> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> > index 79875ed7..d8115cd8 100644
> > --- a/src/cam/main.cpp
> > +++ b/src/cam/main.cpp
> > @@ -301,7 +301,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)) {
> >  		case properties::CameraLocationFront:
> >  			addModel = false;
> >  			name = "Internal front camera ";
> > @@ -321,7 +321,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) + "' ";
> >  	}
> > 
> >  	name += "(" + camera->id() + ")";
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> > index 3b126bb5..f65a0680 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -939,7 +939,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(0);
> >  	RPiController::Metadata lastMetadata;
> >  	Span<uint8_t> embeddedBuffer;
> > 
> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > index b7dda282..43db7b68 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));
> >  		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));
> >  	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(0),
> >  				 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));
> > 
> >  	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 adc397e8..a62afdd4 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(0);
> >  	bool success;
> >  	Transform rotationTransform = transformFromRotation(rotation, &success);
> >  	if (!success)
> > @@ -1706,7 +1706,8 @@ 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);
> >  		/* The control wants linear gains in the order B, Gb, Gr, R. */
> >  		ControlList ctrls(sensor_->controls());
> >  		std::array<int32_t, 4> gains{
> > @@ -2041,7 +2042,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);
> > 
> >  		if (!nativeCrop.width || !nativeCrop.height)
> >  			nativeCrop = { 0, 0, 1, 1 };
> > @@ -2079,7 +2080,7 @@ void RPiCameraData::fillRequestMetadata(const ControlList &bufferControls,
> >  					Request *request)
> >  {
> >  	request->metadata().set(controls::SensorTimestamp,
> > -				bufferControls.get(controls::SensorTimestamp));
> > +				bufferControls.get(controls::SensorTimestamp).value_or(0));
> > 
> >  	request->metadata().set(controls::ScalerCrop, scalerCrop_);
> >  }
> > diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
> > index 34c8df5a..4b5d8276 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);
> >  		TIFFSetField(tif, TIFFTAG_MODEL, model.c_str());
> >  		/* \todo set TIFFTAG_UNIQUECAMERAMODEL. */
> >  	}
> > @@ -438,16 +438,15 @@ 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);
> > -		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 */
> > +		const auto &colourGains = metadata.get(controls::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)) {
> > -		Span<const float> const &coeffs = metadata.get(controls::ColourCorrectionMatrix);
> > -		Matrix3d ccmSupplied(coeffs);
> > +		Matrix3d ccmSupplied(*metadata.get(controls::ColourCorrectionMatrix));
> >  		if (ccmSupplied.determinant() > eps)
> >  			ccm = ccmSupplied;
> >  	}
> > @@ -515,7 +514,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);
> > 
> >  		/*
> >  		 * The black levels control is specified in R, Gr, Gb, B order.
> > @@ -593,13 +593,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);
> >  		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) / 1e6;
> >  		TIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);
> >  	}
> >

Hi Laurent,

I added gcc/g++ 10 to my CI pipeline [1] but I still cannot reproduce
this. I suggest that you take my commits and squash your fixes into them
such that they compile in your setup.

Best,
Christian

[1] https://github.com/christianrauch/libcamera-ci/actions/runs/2611618641


Am 03.07.22 um 23:05 schrieb Laurent Pinchart:
> On Mon, Jul 04, 2022 at 12:45:19AM +0300, Laurent Pinchart wrote:
>> Hi Christian,
>>
>> Thank you for the patch.
>>
>> On Fri, Jun 10, 2022 at 01:03:35PM +0100, Christian Rauch via libcamera-devel wrote:
>>> 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>
>>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
>>> ---
>>>  include/libcamera/controls.h                  |  5 +++--
>>>  src/android/camera_capabilities.cpp           | 12 +++++-----
>>>  src/android/camera_device.cpp                 | 21 +++++++++---------
>>>  src/android/camera_hal_manager.cpp            |  2 +-
>>>  src/cam/main.cpp                              |  4 ++--
>>>  src/ipa/raspberrypi/raspberrypi.cpp           |  2 +-
>>>  src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 ++++----
>>>  .../pipeline/raspberrypi/raspberrypi.cpp      |  9 ++++----
>>>  src/qcam/dng_writer.cpp                       | 22 +++++++++----------
>>>  9 files changed, 43 insertions(+), 43 deletions(-)
>>>
>>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
>>> index 665bcac1..192be784 100644
>>> --- a/include/libcamera/controls.h
>>> +++ b/include/libcamera/controls.h
>>> @@ -8,6 +8,7 @@
>>>  #pragma once
>>>
>>>  #include <assert.h>
>>> +#include <optional>
>>>  #include <set>
>>>  #include <stdint.h>
>>>  #include <string>
>>> @@ -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>();
>>
>> I think one of the perks of using std::optional here is that we could
>> return a reference to the value instead of copying it. To avoid a v9,
>> I'll send a patch on top.
>>
>>>  	}
>>> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
>>> index 6f197eb8..5304b2da 100644
>>> --- a/src/android/camera_capabilities.cpp
>>> +++ b/src/android/camera_capabilities.cpp
>>> @@ -1042,7 +1042,7 @@ int CameraCapabilities::initializeStaticMetadata()
>>>  	/* Sensor static metadata. */
>>>  	std::array<int32_t, 2> pixelArraySize;
>>>  	{
>>> -		const Size &size = properties.get(properties::PixelArraySize);
>>> +		const Size &size = properties.get(properties::PixelArraySize).value_or(Size{});
>>>  		pixelArraySize[0] = size.width;
>>>  		pixelArraySize[1] = size.height;
>>>  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,
>>> @@ -1050,10 +1050,10 @@ int CameraCapabilities::initializeStaticMetadata()
>>>  	}
>>>
>>>  	if (properties.contains(properties::UnitCellSize)) {
>>> -		const Size &cellSize = properties.get<Size>(properties::UnitCellSize);
>>> +		const auto &cellSize = properties.get<Size>(properties::UnitCellSize);
>>
>> We can avoid the double-lookup here and in several locations below. I'll
>> send a patch on top too.
>>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>
> Turns out I spoke a bit too fast :-( When compiling with gcc 10.3.1, I'm
> getting
>
> [27/218] Compiling C++ object src/android/libcamera-hal.so.p/camera_capabilities.cpp.o
> FAILED: src/android/libcamera-hal.so.p/camera_capabilities.cpp.o
> g++-10.3.1 -Isrc/android/libcamera-hal.so.p -Isrc/android -I../../src/android -I../../include/android/hardware/libhardware/include -I../../include/android/metadata -I../../include/android/system/core/include -Iinclude -I../../include -I../../subprojects/libyuv/include -Isubprojects/libyuv/__CMake_build -I../../subprojects/libyuv/__CMake_build -Isubprojects/libyuv -I../../subprojects/libyuv -Iinclude/libcamera/ipa -Iinclude/libcamera -I/usr/include/libexif -fdiagnostics-color=always -fsanitize=address -fno-omit-frame-pointer -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O3 -Wshadow -include config.h -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/android/libcamera-hal.so.p/camera_capabilities.cpp.o -MF src/android/libcamera-hal.so.p/camera_capabilities.cpp.o.d -o src/android/libcamera-hal.so.p/camera_capabilities.cpp.o -c ../../src/android/camera_capabilities.cpp
> In file included from ../../include/libcamera/camera.h:20,
>                  from ../../src/android/camera_capabilities.h:17,
>                  from ../../src/android/camera_capabilities.cpp:8:
> ../../include/libcamera/controls.h: In member function ‘int CameraCapabilities::initializeStaticMetadata()’:
> ../../include/libcamera/controls.h:381:16: error: ‘<anonymous>’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>   381 |    return std::nullopt;
>       |                ^~~~~~~
> ../../include/libcamera/controls.h:381:16: error: ‘*((void*)&<anonymous> +4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
>
> The following change fixes it:
>
> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> index 5304b2da1f93..429054ae3633 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
>
> If we do that, I'd like to apply the same change through this patch. I
> can post a v9 of this patch with this tomorrow if you don't beat me to
> it.
>
>>>  		std::array<float, 2> physicalSize{
>>> -			cellSize.width * pixelArraySize[0] / 1e6f,
>>> -			cellSize.height * pixelArraySize[1] / 1e6f
>>> +			cellSize->width * pixelArraySize[0] / 1e6f,
>>> +			cellSize->height * pixelArraySize[1] / 1e6f
>>>  		};
>>>  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PHYSICAL_SIZE,
>>>  					  physicalSize);
>>> @@ -1061,7 +1061,7 @@ int CameraCapabilities::initializeStaticMetadata()
>>>
>>>  	{
>>>  		const Span<const Rectangle> &rects =
>>> -			properties.get(properties::PixelArrayActiveAreas);
>>> +			properties.get(properties::PixelArrayActiveAreas).value_or(Span<const Rectangle>{});
>>>  		std::vector<int32_t> data{
>>>  			static_cast<int32_t>(rects[0].x),
>>>  			static_cast<int32_t>(rects[0].y),
>>> @@ -1080,7 +1080,7 @@ int CameraCapabilities::initializeStaticMetadata()
>>>
>>>  	/* Report the color filter arrangement if the camera reports it. */
>>>  	if (properties.contains(properties::draft::ColorFilterArrangement)) {
>>> -		uint8_t filterArr = properties.get(properties::draft::ColorFilterArrangement);
>>> +		uint8_t filterArr = *properties.get(properties::draft::ColorFilterArrangement);
>>>  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,
>>>  					  filterArr);
>>>  	}
>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
>>> index 8e804d4d..ec117101 100644
>>> --- a/src/android/camera_device.cpp
>>> +++ b/src/android/camera_device.cpp
>>> @@ -305,7 +305,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)
>>>  	const ControlList &properties = camera_->properties();
>>>
>>>  	if (properties.contains(properties::Location)) {
>>> -		int32_t location = properties.get(properties::Location);
>>> +		int32_t location = *properties.get(properties::Location);
>>>  		switch (location) {
>>>  		case properties::CameraLocationFront:
>>>  			facing_ = CAMERA_FACING_FRONT;
>>> @@ -355,7 +355,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)
>>>  	 * metadata.
>>>  	 */
>>>  	if (properties.contains(properties::Rotation)) {
>>> -		int rotation = properties.get(properties::Rotation);
>>> +		int rotation = *properties.get(properties::Rotation);
>>>  		orientation_ = (360 - rotation) % 360;
>>>  		if (cameraConfigData && cameraConfigData->rotation != -1 &&
>>>  		    orientation_ != cameraConfigData->rotation) {
>>> @@ -1094,7 +1094,8 @@ void CameraDevice::requestComplete(Request *request)
>>>  	 * as soon as possible, earlier than request completion time.
>>>  	 */
>>>  	uint64_t sensorTimestamp = static_cast<uint64_t>(request->metadata()
>>> -							 .get(controls::SensorTimestamp));
>>> +								 .get(controls::SensorTimestamp)
>>> +								 .value_or(0));
>>>  	notifyShutter(descriptor->frameNumber_, sensorTimestamp);
>>>
>>>  	LOG(HAL, Debug) << "Request " << request->cookie() << " completed with "
>>> @@ -1473,29 +1474,28 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>>>  				 rolling_shutter_skew);
>>>
>>>  	/* Add metadata tags reported by libcamera. */
>>> -	const int64_t timestamp = metadata.get(controls::SensorTimestamp);
>>> +	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);
>>> +		uint8_t pipeline_depth = *metadata.get<int32_t>(controls::draft::PipelineDepth);
>>>  		resultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH,
>>>  					 pipeline_depth);
>>>  	}
>>>
>>>  	if (metadata.contains(controls::ExposureTime)) {
>>> -		int64_t exposure = metadata.get(controls::ExposureTime) * 1000ULL;
>>> +		int64_t exposure = *metadata.get(controls::ExposureTime) * 1000ULL;
>>>  		resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);
>>>  	}
>>>
>>>  	if (metadata.contains(controls::FrameDuration)) {
>>> -		int64_t duration = metadata.get(controls::FrameDuration) * 1000;
>>> +		int64_t duration = *metadata.get(controls::FrameDuration) * 1000;
>>>  		resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,
>>>  					 duration);
>>>  	}
>>>
>>>  	if (metadata.contains(controls::ScalerCrop)) {
>>> -		Rectangle crop = metadata.get(controls::ScalerCrop);
>>> +		Rectangle crop = *metadata.get(controls::ScalerCrop);
>>>  		int32_t cropRect[] = {
>>>  			crop.x, crop.y, static_cast<int32_t>(crop.width),
>>>  			static_cast<int32_t>(crop.height),
>>> @@ -1504,8 +1504,7 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
>>>  	}
>>>
>>>  	if (metadata.contains(controls::draft::TestPatternMode)) {
>>> -		const int32_t testPatternMode =
>>> -			metadata.get(controls::draft::TestPatternMode);
>>> +		const int32_t testPatternMode = *metadata.get(controls::draft::TestPatternMode);
>>>  		resultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,
>>>  					 testPatternMode);
>>>  	}
>>> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
>>> index 5f7bfe26..0bffe96f 100644
>>> --- a/src/android/camera_hal_manager.cpp
>>> +++ b/src/android/camera_hal_manager.cpp
>>> @@ -232,7 +232,7 @@ int32_t CameraHalManager::cameraLocation(const Camera *cam)
>>>  	if (!properties.contains(properties::Location))
>>>  		return -1;
>>>
>>> -	return properties.get(properties::Location);
>>> +	return *properties.get(properties::Location);
>>>  }
>>>
>>>  CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)
>>> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
>>> index 79875ed7..d8115cd8 100644
>>> --- a/src/cam/main.cpp
>>> +++ b/src/cam/main.cpp
>>> @@ -301,7 +301,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)) {
>>>  		case properties::CameraLocationFront:
>>>  			addModel = false;
>>>  			name = "Internal front camera ";
>>> @@ -321,7 +321,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) + "' ";
>>>  	}
>>>
>>>  	name += "(" + camera->id() + ")";
>>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
>>> index 3b126bb5..f65a0680 100644
>>> --- a/src/ipa/raspberrypi/raspberrypi.cpp
>>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
>>> @@ -939,7 +939,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(0);
>>>  	RPiController::Metadata lastMetadata;
>>>  	Span<uint8_t> embeddedBuffer;
>>>
>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
>>> index b7dda282..43db7b68 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));
>>>  		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));
>>>  	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(0),
>>>  				 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));
>>>
>>>  	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 adc397e8..a62afdd4 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(0);
>>>  	bool success;
>>>  	Transform rotationTransform = transformFromRotation(rotation, &success);
>>>  	if (!success)
>>> @@ -1706,7 +1706,8 @@ 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);
>>>  		/* The control wants linear gains in the order B, Gb, Gr, R. */
>>>  		ControlList ctrls(sensor_->controls());
>>>  		std::array<int32_t, 4> gains{
>>> @@ -2041,7 +2042,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);
>>>
>>>  		if (!nativeCrop.width || !nativeCrop.height)
>>>  			nativeCrop = { 0, 0, 1, 1 };
>>> @@ -2079,7 +2080,7 @@ void RPiCameraData::fillRequestMetadata(const ControlList &bufferControls,
>>>  					Request *request)
>>>  {
>>>  	request->metadata().set(controls::SensorTimestamp,
>>> -				bufferControls.get(controls::SensorTimestamp));
>>> +				bufferControls.get(controls::SensorTimestamp).value_or(0));
>>>
>>>  	request->metadata().set(controls::ScalerCrop, scalerCrop_);
>>>  }
>>> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
>>> index 34c8df5a..4b5d8276 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);
>>>  		TIFFSetField(tif, TIFFTAG_MODEL, model.c_str());
>>>  		/* \todo set TIFFTAG_UNIQUECAMERAMODEL. */
>>>  	}
>>> @@ -438,16 +438,15 @@ 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);
>>> -		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 */
>>> +		const auto &colourGains = metadata.get(controls::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)) {
>>> -		Span<const float> const &coeffs = metadata.get(controls::ColourCorrectionMatrix);
>>> -		Matrix3d ccmSupplied(coeffs);
>>> +		Matrix3d ccmSupplied(*metadata.get(controls::ColourCorrectionMatrix));
>>>  		if (ccmSupplied.determinant() > eps)
>>>  			ccm = ccmSupplied;
>>>  	}
>>> @@ -515,7 +514,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);
>>>
>>>  		/*
>>>  		 * The black levels control is specified in R, Gr, Gb, B order.
>>> @@ -593,13 +593,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);
>>>  		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) / 1e6;
>>>  		TIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);
>>>  	}
>>>
>

Hi Christian,

On Mon, Jul 04, 2022 at 06:42:28PM +0100, Christian Rauch via libcamera-devel wrote:
> Hi Laurent,
> 
> I added gcc/g++ 10 to my CI pipeline [1] but I still cannot reproduce
> this. I suggest that you take my commits and squash your fixes into them
> such that they compile in your setup.

I'll send a v9 of this patch for your review :-)

> [1] https://github.com/christianrauch/libcamera-ci/actions/runs/2611618641
> 
> Am 03.07.22 um 23:05 schrieb Laurent Pinchart:
> > On Mon, Jul 04, 2022 at 12:45:19AM +0300, Laurent Pinchart wrote:
> >> On Fri, Jun 10, 2022 at 01:03:35PM +0100, Christian Rauch via libcamera-devel wrote:
> >>> 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>
> >>> Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
> >>> ---
> >>>  include/libcamera/controls.h                  |  5 +++--
> >>>  src/android/camera_capabilities.cpp           | 12 +++++-----
> >>>  src/android/camera_device.cpp                 | 21 +++++++++---------
> >>>  src/android/camera_hal_manager.cpp            |  2 +-
> >>>  src/cam/main.cpp                              |  4 ++--
> >>>  src/ipa/raspberrypi/raspberrypi.cpp           |  2 +-
> >>>  src/libcamera/pipeline/ipu3/ipu3.cpp          |  9 ++++----
> >>>  .../pipeline/raspberrypi/raspberrypi.cpp      |  9 ++++----
> >>>  src/qcam/dng_writer.cpp                       | 22 +++++++++----------
> >>>  9 files changed, 43 insertions(+), 43 deletions(-)
> >>>
> >>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h
> >>> index 665bcac1..192be784 100644
> >>> --- a/include/libcamera/controls.h
> >>> +++ b/include/libcamera/controls.h
> >>> @@ -8,6 +8,7 @@
> >>>  #pragma once
> >>>
> >>>  #include <assert.h>
> >>> +#include <optional>
> >>>  #include <set>
> >>>  #include <stdint.h>
> >>>  #include <string>
> >>> @@ -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>();
> >>
> >> I think one of the perks of using std::optional here is that we could
> >> return a reference to the value instead of copying it. To avoid a v9,
> >> I'll send a patch on top.
> >>
> >>>  	}
> >>> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> >>> index 6f197eb8..5304b2da 100644
> >>> --- a/src/android/camera_capabilities.cpp
> >>> +++ b/src/android/camera_capabilities.cpp
> >>> @@ -1042,7 +1042,7 @@ int CameraCapabilities::initializeStaticMetadata()
> >>>  	/* Sensor static metadata. */
> >>>  	std::array<int32_t, 2> pixelArraySize;
> >>>  	{
> >>> -		const Size &size = properties.get(properties::PixelArraySize);
> >>> +		const Size &size = properties.get(properties::PixelArraySize).value_or(Size{});
> >>>  		pixelArraySize[0] = size.width;
> >>>  		pixelArraySize[1] = size.height;
> >>>  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,
> >>> @@ -1050,10 +1050,10 @@ int CameraCapabilities::initializeStaticMetadata()
> >>>  	}
> >>>
> >>>  	if (properties.contains(properties::UnitCellSize)) {
> >>> -		const Size &cellSize = properties.get<Size>(properties::UnitCellSize);
> >>> +		const auto &cellSize = properties.get<Size>(properties::UnitCellSize);
> >>
> >> We can avoid the double-lookup here and in several locations below. I'll
> >> send a patch on top too.
> >>
> >> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >
> > Turns out I spoke a bit too fast :-( When compiling with gcc 10.3.1, I'm
> > getting
> >
> > [27/218] Compiling C++ object src/android/libcamera-hal.so.p/camera_capabilities.cpp.o
> > FAILED: src/android/libcamera-hal.so.p/camera_capabilities.cpp.o
> > g++-10.3.1 -Isrc/android/libcamera-hal.so.p -Isrc/android -I../../src/android -I../../include/android/hardware/libhardware/include -I../../include/android/metadata -I../../include/android/system/core/include -Iinclude -I../../include -I../../subprojects/libyuv/include -Isubprojects/libyuv/__CMake_build -I../../subprojects/libyuv/__CMake_build -Isubprojects/libyuv -I../../subprojects/libyuv -Iinclude/libcamera/ipa -Iinclude/libcamera -I/usr/include/libexif -fdiagnostics-color=always -fsanitize=address -fno-omit-frame-pointer -D_FILE_OFFSET_BITS=64 -Wall -Winvalid-pch -Wnon-virtual-dtor -Wextra -Werror -std=c++17 -O3 -Wshadow -include config.h -fPIC -DLIBCAMERA_BASE_PRIVATE -MD -MQ src/android/libcamera-hal.so.p/camera_capabilities.cpp.o -MF src/android/libcamera-hal.so.p/camera_capabilities.cpp.o.d -o src/android/libcamera-hal.so.p/camera_capabilities.cpp.o -c ../../src/android/camera_capabilities.cpp
> > In file included from ../../include/libcamera/camera.h:20,
> >                  from ../../src/android/camera_capabilities.h:17,
> >                  from ../../src/android/camera_capabilities.cpp:8:
> > ../../include/libcamera/controls.h: In member function ‘int CameraCapabilities::initializeStaticMetadata()’:
> > ../../include/libcamera/controls.h:381:16: error: ‘<anonymous>’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >   381 |    return std::nullopt;
> >       |                ^~~~~~~
> > ../../include/libcamera/controls.h:381:16: error: ‘*((void*)&<anonymous> +4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
> >
> > The following change fixes it:
> >
> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp
> > index 5304b2da1f93..429054ae3633 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
> >
> > If we do that, I'd like to apply the same change through this patch. I
> > can post a v9 of this patch with this tomorrow if you don't beat me to
> > it.
> >
> >>>  		std::array<float, 2> physicalSize{
> >>> -			cellSize.width * pixelArraySize[0] / 1e6f,
> >>> -			cellSize.height * pixelArraySize[1] / 1e6f
> >>> +			cellSize->width * pixelArraySize[0] / 1e6f,
> >>> +			cellSize->height * pixelArraySize[1] / 1e6f
> >>>  		};
> >>>  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_PHYSICAL_SIZE,
> >>>  					  physicalSize);
> >>> @@ -1061,7 +1061,7 @@ int CameraCapabilities::initializeStaticMetadata()
> >>>
> >>>  	{
> >>>  		const Span<const Rectangle> &rects =
> >>> -			properties.get(properties::PixelArrayActiveAreas);
> >>> +			properties.get(properties::PixelArrayActiveAreas).value_or(Span<const Rectangle>{});
> >>>  		std::vector<int32_t> data{
> >>>  			static_cast<int32_t>(rects[0].x),
> >>>  			static_cast<int32_t>(rects[0].y),
> >>> @@ -1080,7 +1080,7 @@ int CameraCapabilities::initializeStaticMetadata()
> >>>
> >>>  	/* Report the color filter arrangement if the camera reports it. */
> >>>  	if (properties.contains(properties::draft::ColorFilterArrangement)) {
> >>> -		uint8_t filterArr = properties.get(properties::draft::ColorFilterArrangement);
> >>> +		uint8_t filterArr = *properties.get(properties::draft::ColorFilterArrangement);
> >>>  		staticMetadata_->addEntry(ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,
> >>>  					  filterArr);
> >>>  	}
> >>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> >>> index 8e804d4d..ec117101 100644
> >>> --- a/src/android/camera_device.cpp
> >>> +++ b/src/android/camera_device.cpp
> >>> @@ -305,7 +305,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)
> >>>  	const ControlList &properties = camera_->properties();
> >>>
> >>>  	if (properties.contains(properties::Location)) {
> >>> -		int32_t location = properties.get(properties::Location);
> >>> +		int32_t location = *properties.get(properties::Location);
> >>>  		switch (location) {
> >>>  		case properties::CameraLocationFront:
> >>>  			facing_ = CAMERA_FACING_FRONT;
> >>> @@ -355,7 +355,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)
> >>>  	 * metadata.
> >>>  	 */
> >>>  	if (properties.contains(properties::Rotation)) {
> >>> -		int rotation = properties.get(properties::Rotation);
> >>> +		int rotation = *properties.get(properties::Rotation);
> >>>  		orientation_ = (360 - rotation) % 360;
> >>>  		if (cameraConfigData && cameraConfigData->rotation != -1 &&
> >>>  		    orientation_ != cameraConfigData->rotation) {
> >>> @@ -1094,7 +1094,8 @@ void CameraDevice::requestComplete(Request *request)
> >>>  	 * as soon as possible, earlier than request completion time.
> >>>  	 */
> >>>  	uint64_t sensorTimestamp = static_cast<uint64_t>(request->metadata()
> >>> -							 .get(controls::SensorTimestamp));
> >>> +								 .get(controls::SensorTimestamp)
> >>> +								 .value_or(0));
> >>>  	notifyShutter(descriptor->frameNumber_, sensorTimestamp);
> >>>
> >>>  	LOG(HAL, Debug) << "Request " << request->cookie() << " completed with "
> >>> @@ -1473,29 +1474,28 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> >>>  				 rolling_shutter_skew);
> >>>
> >>>  	/* Add metadata tags reported by libcamera. */
> >>> -	const int64_t timestamp = metadata.get(controls::SensorTimestamp);
> >>> +	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);
> >>> +		uint8_t pipeline_depth = *metadata.get<int32_t>(controls::draft::PipelineDepth);
> >>>  		resultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH,
> >>>  					 pipeline_depth);
> >>>  	}
> >>>
> >>>  	if (metadata.contains(controls::ExposureTime)) {
> >>> -		int64_t exposure = metadata.get(controls::ExposureTime) * 1000ULL;
> >>> +		int64_t exposure = *metadata.get(controls::ExposureTime) * 1000ULL;
> >>>  		resultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);
> >>>  	}
> >>>
> >>>  	if (metadata.contains(controls::FrameDuration)) {
> >>> -		int64_t duration = metadata.get(controls::FrameDuration) * 1000;
> >>> +		int64_t duration = *metadata.get(controls::FrameDuration) * 1000;
> >>>  		resultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,
> >>>  					 duration);
> >>>  	}
> >>>
> >>>  	if (metadata.contains(controls::ScalerCrop)) {
> >>> -		Rectangle crop = metadata.get(controls::ScalerCrop);
> >>> +		Rectangle crop = *metadata.get(controls::ScalerCrop);
> >>>  		int32_t cropRect[] = {
> >>>  			crop.x, crop.y, static_cast<int32_t>(crop.width),
> >>>  			static_cast<int32_t>(crop.height),
> >>> @@ -1504,8 +1504,7 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons
> >>>  	}
> >>>
> >>>  	if (metadata.contains(controls::draft::TestPatternMode)) {
> >>> -		const int32_t testPatternMode =
> >>> -			metadata.get(controls::draft::TestPatternMode);
> >>> +		const int32_t testPatternMode = *metadata.get(controls::draft::TestPatternMode);
> >>>  		resultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,
> >>>  					 testPatternMode);
> >>>  	}
> >>> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> >>> index 5f7bfe26..0bffe96f 100644
> >>> --- a/src/android/camera_hal_manager.cpp
> >>> +++ b/src/android/camera_hal_manager.cpp
> >>> @@ -232,7 +232,7 @@ int32_t CameraHalManager::cameraLocation(const Camera *cam)
> >>>  	if (!properties.contains(properties::Location))
> >>>  		return -1;
> >>>
> >>> -	return properties.get(properties::Location);
> >>> +	return *properties.get(properties::Location);
> >>>  }
> >>>
> >>>  CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)
> >>> diff --git a/src/cam/main.cpp b/src/cam/main.cpp
> >>> index 79875ed7..d8115cd8 100644
> >>> --- a/src/cam/main.cpp
> >>> +++ b/src/cam/main.cpp
> >>> @@ -301,7 +301,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)) {
> >>>  		case properties::CameraLocationFront:
> >>>  			addModel = false;
> >>>  			name = "Internal front camera ";
> >>> @@ -321,7 +321,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) + "' ";
> >>>  	}
> >>>
> >>>  	name += "(" + camera->id() + ")";
> >>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> >>> index 3b126bb5..f65a0680 100644
> >>> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> >>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> >>> @@ -939,7 +939,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(0);
> >>>  	RPiController::Metadata lastMetadata;
> >>>  	Span<uint8_t> embeddedBuffer;
> >>>
> >>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> >>> index b7dda282..43db7b68 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));
> >>>  		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));
> >>>  	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(0),
> >>>  				 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));
> >>>
> >>>  	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 adc397e8..a62afdd4 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(0);
> >>>  	bool success;
> >>>  	Transform rotationTransform = transformFromRotation(rotation, &success);
> >>>  	if (!success)
> >>> @@ -1706,7 +1706,8 @@ 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);
> >>>  		/* The control wants linear gains in the order B, Gb, Gr, R. */
> >>>  		ControlList ctrls(sensor_->controls());
> >>>  		std::array<int32_t, 4> gains{
> >>> @@ -2041,7 +2042,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);
> >>>
> >>>  		if (!nativeCrop.width || !nativeCrop.height)
> >>>  			nativeCrop = { 0, 0, 1, 1 };
> >>> @@ -2079,7 +2080,7 @@ void RPiCameraData::fillRequestMetadata(const ControlList &bufferControls,
> >>>  					Request *request)
> >>>  {
> >>>  	request->metadata().set(controls::SensorTimestamp,
> >>> -				bufferControls.get(controls::SensorTimestamp));
> >>> +				bufferControls.get(controls::SensorTimestamp).value_or(0));
> >>>
> >>>  	request->metadata().set(controls::ScalerCrop, scalerCrop_);
> >>>  }
> >>> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp
> >>> index 34c8df5a..4b5d8276 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);
> >>>  		TIFFSetField(tif, TIFFTAG_MODEL, model.c_str());
> >>>  		/* \todo set TIFFTAG_UNIQUECAMERAMODEL. */
> >>>  	}
> >>> @@ -438,16 +438,15 @@ 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);
> >>> -		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 */
> >>> +		const auto &colourGains = metadata.get(controls::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)) {
> >>> -		Span<const float> const &coeffs = metadata.get(controls::ColourCorrectionMatrix);
> >>> -		Matrix3d ccmSupplied(coeffs);
> >>> +		Matrix3d ccmSupplied(*metadata.get(controls::ColourCorrectionMatrix));
> >>>  		if (ccmSupplied.determinant() > eps)
> >>>  			ccm = ccmSupplied;
> >>>  	}
> >>> @@ -515,7 +514,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);
> >>>
> >>>  		/*
> >>>  		 * The black levels control is specified in R, Gr, Gb, B order.
> >>> @@ -593,13 +593,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);
> >>>  		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) / 1e6;
> >>>  		TIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);
> >>>  	}
> >>>