[{"id":23715,"web_url":"https://patchwork.libcamera.org/comment/23715/","msgid":"","date":"2022-07-03T21:45:18","subject":"Re: [libcamera-devel] [PATCH v8 1/4] libcamera: controls: Use\n\tstd::optional to handle invalid control values","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Christian,\n\nThank you for the patch.\n\nOn Fri, Jun 10, 2022 at 01:03:35PM +0100, Christian Rauch via libcamera-devel wrote:\n> Previously, ControlList::get() would use default constructed objects to\n> indicate that a ControlList does not have the requested Control. This has\n> several disadvantages: 1) It requires types to be default constructible,\n> 2) it does not differentiate between a default constructed object and an\n> object that happens to have the same state as a default constructed object.\n> \n> std::optional additionally stores the information if the object is valid\n> or not, and therefore is more expressive than a default constructed object.\n> \n> Signed-off-by: Christian Rauch \n> Reviewed-by: Jacopo Mondi \n> ---\n> include/libcamera/controls.h | 5 +++--\n> src/android/camera_capabilities.cpp | 12 +++++-----\n> src/android/camera_device.cpp | 21 +++++++++---------\n> src/android/camera_hal_manager.cpp | 2 +-\n> src/cam/main.cpp | 4 ++--\n> src/ipa/raspberrypi/raspberrypi.cpp | 2 +-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 9 ++++----\n> .../pipeline/raspberrypi/raspberrypi.cpp | 9 ++++----\n> src/qcam/dng_writer.cpp | 22 +++++++++----------\n> 9 files changed, 43 insertions(+), 43 deletions(-)\n> \n> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> index 665bcac1..192be784 100644\n> --- a/include/libcamera/controls.h\n> +++ b/include/libcamera/controls.h\n> @@ -8,6 +8,7 @@\n> #pragma once\n> \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -373,11 +374,11 @@ public:\n> \tbool contains(unsigned int id) const;\n> \n> \ttemplate\n> -\tT get(const Control &ctrl) const\n> +\tstd::optional get(const Control &ctrl) const\n> \t{\n> \t\tconst ControlValue *val = find(ctrl.id());\n> \t\tif (!val)\n> -\t\t\treturn T{};\n> +\t\t\treturn std::nullopt;\n> \n> \t\treturn val->get();\n\nI think one of the perks of using std::optional here is that we could\nreturn a reference to the value instead of copying it. To avoid a v9,\nI'll send a patch on top.\n\n> \t}\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 6f197eb8..5304b2da 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -1042,7 +1042,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> \t/* Sensor static metadata. */\n> \tstd::array pixelArraySize;\n> \t{\n> -\t\tconst Size &size = properties.get(properties::PixelArraySize);\n> +\t\tconst Size &size = properties.get(properties::PixelArraySize).value_or(Size{});\n> \t\tpixelArraySize[0] = size.width;\n> \t\tpixelArraySize[1] = size.height;\n> \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,\n> @@ -1050,10 +1050,10 @@ int CameraCapabilities::initializeStaticMetadata()\n> \t}\n> \n> \tif (properties.contains(properties::UnitCellSize)) {\n> -\t\tconst Size &cellSize = properties.get(properties::UnitCellSize);\n> +\t\tconst auto &cellSize = properties.get(properties::UnitCellSize);\n\nWe can avoid the double-lookup here and in several locations below. I'll\nsend a patch on top too.\n\nReviewed-by: Laurent Pinchart \n\n> \t\tstd::array physicalSize{\n> -\t\t\tcellSize.width * pixelArraySize[0] / 1e6f,\n> -\t\t\tcellSize.height * pixelArraySize[1] / 1e6f\n> +\t\t\tcellSize->width * pixelArraySize[0] / 1e6f,\n> +\t\t\tcellSize->height * pixelArraySize[1] / 1e6f\n> \t\t};\n> \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_PHYSICAL_SIZE,\n> \t\t\t\t\t physicalSize);\n> @@ -1061,7 +1061,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> \n> \t{\n> \t\tconst Span &rects =\n> -\t\t\tproperties.get(properties::PixelArrayActiveAreas);\n> +\t\t\tproperties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> \t\tstd::vector data{\n> \t\t\tstatic_cast(rects[0].x),\n> \t\t\tstatic_cast(rects[0].y),\n> @@ -1080,7 +1080,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> \n> \t/* Report the color filter arrangement if the camera reports it. */\n> \tif (properties.contains(properties::draft::ColorFilterArrangement)) {\n> -\t\tuint8_t filterArr = properties.get(properties::draft::ColorFilterArrangement);\n> +\t\tuint8_t filterArr = *properties.get(properties::draft::ColorFilterArrangement);\n> \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,\n> \t\t\t\t\t filterArr);\n> \t}\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 8e804d4d..ec117101 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -305,7 +305,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> \tconst ControlList &properties = camera_->properties();\n> \n> \tif (properties.contains(properties::Location)) {\n> -\t\tint32_t location = properties.get(properties::Location);\n> +\t\tint32_t location = *properties.get(properties::Location);\n> \t\tswitch (location) {\n> \t\tcase properties::CameraLocationFront:\n> \t\t\tfacing_ = CAMERA_FACING_FRONT;\n> @@ -355,7 +355,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> \t * metadata.\n> \t */\n> \tif (properties.contains(properties::Rotation)) {\n> -\t\tint rotation = properties.get(properties::Rotation);\n> +\t\tint rotation = *properties.get(properties::Rotation);\n> \t\torientation_ = (360 - rotation) % 360;\n> \t\tif (cameraConfigData && cameraConfigData->rotation != -1 &&\n> \t\t orientation_ != cameraConfigData->rotation) {\n> @@ -1094,7 +1094,8 @@ void CameraDevice::requestComplete(Request *request)\n> \t * as soon as possible, earlier than request completion time.\n> \t */\n> \tuint64_t sensorTimestamp = static_cast(request->metadata()\n> -\t\t\t\t\t\t\t .get(controls::SensorTimestamp));\n> +\t\t\t\t\t\t\t\t .get(controls::SensorTimestamp)\n> +\t\t\t\t\t\t\t\t .value_or(0));\n> \tnotifyShutter(descriptor->frameNumber_, sensorTimestamp);\n> \n> \tLOG(HAL, Debug) << \"Request \" << request->cookie() << \" completed with \"\n> @@ -1473,29 +1474,28 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t\t\t\t rolling_shutter_skew);\n> \n> \t/* Add metadata tags reported by libcamera. */\n> -\tconst int64_t timestamp = metadata.get(controls::SensorTimestamp);\n> +\tconst int64_t timestamp = metadata.get(controls::SensorTimestamp).value_or(0);\n> \tresultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, timestamp);\n> \n> \tif (metadata.contains(controls::draft::PipelineDepth)) {\n> -\t\tuint8_t pipeline_depth =\n> -\t\t\tmetadata.get(controls::draft::PipelineDepth);\n> +\t\tuint8_t pipeline_depth = *metadata.get(controls::draft::PipelineDepth);\n> \t\tresultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH,\n> \t\t\t\t\t pipeline_depth);\n> \t}\n> \n> \tif (metadata.contains(controls::ExposureTime)) {\n> -\t\tint64_t exposure = metadata.get(controls::ExposureTime) * 1000ULL;\n> +\t\tint64_t exposure = *metadata.get(controls::ExposureTime) * 1000ULL;\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);\n> \t}\n> \n> \tif (metadata.contains(controls::FrameDuration)) {\n> -\t\tint64_t duration = metadata.get(controls::FrameDuration) * 1000;\n> +\t\tint64_t duration = *metadata.get(controls::FrameDuration) * 1000;\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> \t\t\t\t\t duration);\n> \t}\n> \n> \tif (metadata.contains(controls::ScalerCrop)) {\n> -\t\tRectangle crop = metadata.get(controls::ScalerCrop);\n> +\t\tRectangle crop = *metadata.get(controls::ScalerCrop);\n> \t\tint32_t cropRect[] = {\n> \t\t\tcrop.x, crop.y, static_cast(crop.width),\n> \t\t\tstatic_cast(crop.height),\n> @@ -1504,8 +1504,7 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t}\n> \n> \tif (metadata.contains(controls::draft::TestPatternMode)) {\n> -\t\tconst int32_t testPatternMode =\n> -\t\t\tmetadata.get(controls::draft::TestPatternMode);\n> +\t\tconst int32_t testPatternMode = *metadata.get(controls::draft::TestPatternMode);\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,\n> \t\t\t\t\t testPatternMode);\n> \t}\n> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp\n> index 5f7bfe26..0bffe96f 100644\n> --- a/src/android/camera_hal_manager.cpp\n> +++ b/src/android/camera_hal_manager.cpp\n> @@ -232,7 +232,7 @@ int32_t CameraHalManager::cameraLocation(const Camera *cam)\n> \tif (!properties.contains(properties::Location))\n> \t\treturn -1;\n> \n> -\treturn properties.get(properties::Location);\n> +\treturn *properties.get(properties::Location);\n> }\n> \n> CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index 79875ed7..d8115cd8 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -301,7 +301,7 @@ std::string CamApp::cameraName(const Camera *camera)\n> \t * is only used if the location isn't present or is set to External.\n> \t */\n> \tif (props.contains(properties::Location)) {\n> -\t\tswitch (props.get(properties::Location)) {\n> +\t\tswitch (*props.get(properties::Location)) {\n> \t\tcase properties::CameraLocationFront:\n> \t\t\taddModel = false;\n> \t\t\tname = \"Internal front camera \";\n> @@ -321,7 +321,7 @@ std::string CamApp::cameraName(const Camera *camera)\n> \t\t * If the camera location is not availble use the camera model\n> \t\t * to build the camera name.\n> \t\t */\n> -\t\tname = \"'\" + props.get(properties::Model) + \"' \";\n> +\t\tname = \"'\" + *props.get(properties::Model) + \"' \";\n> \t}\n> \n> \tname += \"(\" + camera->id() + \")\";\n> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> index 3b126bb5..f65a0680 100644\n> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> @@ -939,7 +939,7 @@ void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)\n> \n> void IPARPi::prepareISP(const ISPConfig &data)\n> {\n> -\tint64_t frameTimestamp = data.controls.get(controls::SensorTimestamp);\n> +\tint64_t frameTimestamp = data.controls.get(controls::SensorTimestamp).value_or(0);\n> \tRPiController::Metadata lastMetadata;\n> \tSpan embeddedBuffer;\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index b7dda282..43db7b68 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1145,7 +1145,7 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\t/* Convert the sensor rotation to a transformation */\n> \t\tint32_t rotation = 0;\n> \t\tif (data->properties_.contains(properties::Rotation))\n> -\t\t\trotation = data->properties_.get(properties::Rotation);\n> +\t\t\trotation = *(data->properties_.get(properties::Rotation));\n> \t\telse\n> \t\t\tLOG(IPU3, Warning) << \"Rotation control not exposed by \"\n> \t\t\t\t\t << cio2->sensor()->id()\n> @@ -1331,7 +1331,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n> \t/* \\todo Actually apply the scaler crop region to the ImgU. */\n> \tif (request->controls().contains(controls::ScalerCrop))\n> -\t\tcropRegion_ = request->controls().get(controls::ScalerCrop);\n> +\t\tcropRegion_ = *(request->controls().get(controls::ScalerCrop));\n> \trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n> \n> \tif (frameInfos_.tryComplete(info))\n> @@ -1424,7 +1424,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n> \t\treturn;\n> \t}\n> \n> -\tipa_->processStatsBuffer(info->id, request->metadata().get(controls::SensorTimestamp),\n> +\tipa_->processStatsBuffer(info->id, request->metadata().get(controls::SensorTimestamp).value_or(0),\n> \t\t\t\t info->statBuffer->cookie(), info->effectiveSensorControls);\n> }\n> \n> @@ -1458,8 +1458,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n> \tif (!request->controls().contains(controls::draft::TestPatternMode))\n> \t\treturn;\n> \n> -\tconst int32_t testPatternMode = request->controls().get(\n> -\t\tcontrols::draft::TestPatternMode);\n> +\tconst int32_t testPatternMode = *(request->controls().get(controls::draft::TestPatternMode));\n> \n> \tint ret = cio2_.sensor()->setTestPatternMode(\n> \t\tstatic_cast(testPatternMode));\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index adc397e8..a62afdd4 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -365,7 +365,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> \t * error means the platform can never run. Let's just print a warning\n> \t * and continue regardless; the rotation is effectively set to zero.\n> \t */\n> -\tint32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n> +\tint32_t rotation = data_->sensor_->properties().get(properties::Rotation).value_or(0);\n> \tbool success;\n> \tTransform rotationTransform = transformFromRotation(rotation, &success);\n> \tif (!success)\n> @@ -1706,7 +1706,8 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &\n> \t * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is set).\n> \t */\n> \tif (notifyGainsUnity_ && controls.contains(libcamera::controls::ColourGains)) {\n> -\t\tlibcamera::Span colourGains = controls.get(libcamera::controls::ColourGains);\n> +\t\tlibcamera::Span colourGains =\n> +\t\t\t*controls.get(libcamera::controls::ColourGains);\n> \t\t/* The control wants linear gains in the order B, Gb, Gr, R. */\n> \t\tControlList ctrls(sensor_->controls());\n> \t\tstd::array gains{\n> @@ -2041,7 +2042,7 @@ Rectangle RPiCameraData::scaleIspCrop(const Rectangle &ispCrop) const\n> void RPiCameraData::applyScalerCrop(const ControlList &controls)\n> {\n> \tif (controls.contains(controls::ScalerCrop)) {\n> -\t\tRectangle nativeCrop = controls.get(controls::ScalerCrop);\n> +\t\tRectangle nativeCrop = *controls.get(controls::ScalerCrop);\n> \n> \t\tif (!nativeCrop.width || !nativeCrop.height)\n> \t\t\tnativeCrop = { 0, 0, 1, 1 };\n> @@ -2079,7 +2080,7 @@ void RPiCameraData::fillRequestMetadata(const ControlList &bufferControls,\n> \t\t\t\t\tRequest *request)\n> {\n> \trequest->metadata().set(controls::SensorTimestamp,\n> -\t\t\t\tbufferControls.get(controls::SensorTimestamp));\n> +\t\t\t\tbufferControls.get(controls::SensorTimestamp).value_or(0));\n> \n> \trequest->metadata().set(controls::ScalerCrop, scalerCrop_);\n> }\n> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> index 34c8df5a..4b5d8276 100644\n> --- a/src/qcam/dng_writer.cpp\n> +++ b/src/qcam/dng_writer.cpp\n> @@ -392,7 +392,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \tTIFFSetField(tif, TIFFTAG_MAKE, \"libcamera\");\n> \n> \tif (cameraProperties.contains(properties::Model)) {\n> -\t\tstd::string model = cameraProperties.get(properties::Model);\n> +\t\tstd::string model = *cameraProperties.get(properties::Model);\n> \t\tTIFFSetField(tif, TIFFTAG_MODEL, model.c_str());\n> \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n> \t}\n> @@ -438,16 +438,15 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \tconst double eps = 1e-2;\n> \n> \tif (metadata.contains(controls::ColourGains)) {\n> -\t\tSpan const &colourGains = metadata.get(controls::ColourGains);\n> -\t\tif (colourGains[0] > eps && colourGains[1] > eps) {\n> -\t\t\twbGain = Matrix3d::diag(colourGains[0], 1, colourGains[1]);\n> -\t\t\tneutral[0] = 1.0 / colourGains[0]; /* red */\n> -\t\t\tneutral[2] = 1.0 / colourGains[1]; /* blue */\n> +\t\tconst auto &colourGains = metadata.get(controls::ColourGains);\n> +\t\tif ((*colourGains)[0] > eps && (*colourGains)[1] > eps) {\n> +\t\t\twbGain = Matrix3d::diag((*colourGains)[0], 1, (*colourGains)[1]);\n> +\t\t\tneutral[0] = 1.0 / (*colourGains)[0]; /* red */\n> +\t\t\tneutral[2] = 1.0 / (*colourGains)[1]; /* blue */\n> \t\t}\n> \t}\n> \tif (metadata.contains(controls::ColourCorrectionMatrix)) {\n> -\t\tSpan const &coeffs = metadata.get(controls::ColourCorrectionMatrix);\n> -\t\tMatrix3d ccmSupplied(coeffs);\n> +\t\tMatrix3d ccmSupplied(*metadata.get(controls::ColourCorrectionMatrix));\n> \t\tif (ccmSupplied.determinant() > eps)\n> \t\t\tccm = ccmSupplied;\n> \t}\n> @@ -515,7 +514,8 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \tuint32_t whiteLevel = (1 << info->bitsPerSample) - 1;\n> \n> \tif (metadata.contains(controls::SensorBlackLevels)) {\n> -\t\tSpan levels = metadata.get(controls::SensorBlackLevels);\n> +\t\tSpan levels =\n> +\t\t\t*metadata.get(controls::SensorBlackLevels);\n> \n> \t\t/*\n> \t\t * The black levels control is specified in R, Gr, Gb, B order.\n> @@ -593,13 +593,13 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n> \n> \tif (metadata.contains(controls::AnalogueGain)) {\n> -\t\tfloat gain = metadata.get(controls::AnalogueGain);\n> +\t\tfloat gain = *metadata.get(controls::AnalogueGain);\n> \t\tuint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f);\n> \t\tTIFFSetField(tif, EXIFTAG_ISOSPEEDRATINGS, 1, &iso);\n> \t}\n> \n> \tif (metadata.contains(controls::ExposureTime)) {\n> -\t\tfloat exposureTime = metadata.get(controls::ExposureTime) / 1e6;\n> +\t\tfloat exposureTime = *metadata.get(controls::ExposureTime) / 1e6;\n> \t\tTIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);\n> \t}\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5541EBD808\n\tfor ;\n\tSun, 3 Jul 2022 21:45:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A69DC6564E;\n\tSun, 3 Jul 2022 23:45:42 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 028426054A\n\tfor ;\n\tSun, 3 Jul 2022 23:45:40 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 6F88649C;\n\tSun, 3 Jul 2022 23:45:40 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656884742;\n\tbh=3zMce+ufgUQsBsGD0yLlU0xJCwPqWTc0ww+tlZTPbdE=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=k+CByYMkkz9hIebX1oOS1tgfGdBOQhoptm5tYTqTGcLojNIfeDn5drJCzAhD6kX0v\n\tnenamG+niEAtsBmtejZvdJHhg/QEgW8mM4Z8Bpuc9MSniSDqBN020JEP6owQaFaiW/\n\ta7D1pAhZDAhyh3q9RnW5o0+xYgBsL9M24CVu358zeTO2WGqFFa63a6rLtf+WW+IXEF\n\tgzuAI2JEyknNpwUnOs0FB6osaxnehOCjXxF8bUkJDXyWqbpGTec8YkDG6RfOku96Ap\n\tl4w+k2JPDNRtKjo70pwmm22VkSA4Ixs4T4B3tzZTZGc3xzDyuHu7M4umS7u9pH/qwt\n\t81IbynUrpLMlw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656884740;\n\tbh=3zMce+ufgUQsBsGD0yLlU0xJCwPqWTc0ww+tlZTPbdE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ti7RhV37wCJNIzcnbtiCpn6j7phpflfRlxc+0MuDOOK8VeC1gOnmK7JbeYYklazHA\n\tfBWig+8xi5K5IOEMFWgLc42ImUg8sfHCtoQTWOhhi73hpoJ7tx2kbv5v1Oc3nAV7q2\n\tl9TNM44NhDeRP30+KN0AYdhnvzHIuzkXAIjcljws="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"ti7RhV37\"; dkim-atps=neutral","Date":"Mon, 4 Jul 2022 00:45:18 +0300","To":"Christian Rauch ","Message-ID":"","References":"<20220610120338.96883-1-Rauch.Christian@gmx.de>\n\t<20220610120338.96883-2-Rauch.Christian@gmx.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220610120338.96883-2-Rauch.Christian@gmx.de>","Subject":"Re: [libcamera-devel] [PATCH v8 1/4] libcamera: controls: Use\n\tstd::optional to handle invalid control values","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23716,"web_url":"https://patchwork.libcamera.org/comment/23716/","msgid":"","date":"2022-07-03T22:05:46","subject":"Re: [libcamera-devel] [PATCH v8 1/4] libcamera: controls: Use\n\tstd::optional to handle invalid control values","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Mon, Jul 04, 2022 at 12:45:19AM +0300, Laurent Pinchart wrote:\n> Hi Christian,\n> \n> Thank you for the patch.\n> \n> On Fri, Jun 10, 2022 at 01:03:35PM +0100, Christian Rauch via libcamera-devel wrote:\n> > Previously, ControlList::get() would use default constructed objects to\n> > indicate that a ControlList does not have the requested Control. This has\n> > several disadvantages: 1) It requires types to be default constructible,\n> > 2) it does not differentiate between a default constructed object and an\n> > object that happens to have the same state as a default constructed object.\n> > \n> > std::optional additionally stores the information if the object is valid\n> > or not, and therefore is more expressive than a default constructed object.\n> > \n> > Signed-off-by: Christian Rauch \n> > Reviewed-by: Jacopo Mondi \n> > ---\n> > include/libcamera/controls.h | 5 +++--\n> > src/android/camera_capabilities.cpp | 12 +++++-----\n> > src/android/camera_device.cpp | 21 +++++++++---------\n> > src/android/camera_hal_manager.cpp | 2 +-\n> > src/cam/main.cpp | 4 ++--\n> > src/ipa/raspberrypi/raspberrypi.cpp | 2 +-\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 9 ++++----\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 9 ++++----\n> > src/qcam/dng_writer.cpp | 22 +++++++++----------\n> > 9 files changed, 43 insertions(+), 43 deletions(-)\n> > \n> > diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> > index 665bcac1..192be784 100644\n> > --- a/include/libcamera/controls.h\n> > +++ b/include/libcamera/controls.h\n> > @@ -8,6 +8,7 @@\n> > #pragma once\n> > \n> > #include \n> > +#include \n> > #include \n> > #include \n> > #include \n> > @@ -373,11 +374,11 @@ public:\n> > \tbool contains(unsigned int id) const;\n> > \n> > \ttemplate\n> > -\tT get(const Control &ctrl) const\n> > +\tstd::optional get(const Control &ctrl) const\n> > \t{\n> > \t\tconst ControlValue *val = find(ctrl.id());\n> > \t\tif (!val)\n> > -\t\t\treturn T{};\n> > +\t\t\treturn std::nullopt;\n> > \n> > \t\treturn val->get();\n> \n> I think one of the perks of using std::optional here is that we could\n> return a reference to the value instead of copying it. To avoid a v9,\n> I'll send a patch on top.\n> \n> > \t}\n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index 6f197eb8..5304b2da 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -1042,7 +1042,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> > \t/* Sensor static metadata. */\n> > \tstd::array pixelArraySize;\n> > \t{\n> > -\t\tconst Size &size = properties.get(properties::PixelArraySize);\n> > +\t\tconst Size &size = properties.get(properties::PixelArraySize).value_or(Size{});\n> > \t\tpixelArraySize[0] = size.width;\n> > \t\tpixelArraySize[1] = size.height;\n> > \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,\n> > @@ -1050,10 +1050,10 @@ int CameraCapabilities::initializeStaticMetadata()\n> > \t}\n> > \n> > \tif (properties.contains(properties::UnitCellSize)) {\n> > -\t\tconst Size &cellSize = properties.get(properties::UnitCellSize);\n> > +\t\tconst auto &cellSize = properties.get(properties::UnitCellSize);\n> \n> We can avoid the double-lookup here and in several locations below. I'll\n> send a patch on top too.\n> \n> Reviewed-by: Laurent Pinchart \n\nTurns out I spoke a bit too fast :-( When compiling with gcc 10.3.1, I'm\ngetting\n\n[27/218] Compiling C++ object src/android/libcamera-hal.so.p/camera_capabilities.cpp.o\nFAILED: src/android/libcamera-hal.so.p/camera_capabilities.cpp.o\ng++-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\nIn file included from ../../include/libcamera/camera.h:20,\n from ../../src/android/camera_capabilities.h:17,\n from ../../src/android/camera_capabilities.cpp:8:\n../../include/libcamera/controls.h: In member function ‘int CameraCapabilities::initializeStaticMetadata()’:\n../../include/libcamera/controls.h:381:16: error: ‘’ may be used uninitialized in this function [-Werror=maybe-uninitialized]\n 381 | return std::nullopt;\n | ^~~~~~~\n../../include/libcamera/controls.h:381:16: error: ‘*((void*)& +4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]\n\nThe following change fixes it:\n\ndiff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\nindex 5304b2da1f93..429054ae3633 100644\n--- a/src/android/camera_capabilities.cpp\n+++ b/src/android/camera_capabilities.cpp\n@@ -1049,8 +1049,8 @@ int CameraCapabilities::initializeStaticMetadata()\n \t\t\t\t\t pixelArraySize);\n \t}\n\n-\tif (properties.contains(properties::UnitCellSize)) {\n-\t\tconst auto &cellSize = properties.get(properties::UnitCellSize);\n+\tconst auto &cellSize = properties.get(properties::UnitCellSize);\n+\tif (cellSize) {\n \t\tstd::array physicalSize{\n \t\t\tcellSize->width * pixelArraySize[0] / 1e6f,\n \t\t\tcellSize->height * pixelArraySize[1] / 1e6f\n\nIf we do that, I'd like to apply the same change through this patch. I\ncan post a v9 of this patch with this tomorrow if you don't beat me to\nit.\n\n> > \t\tstd::array physicalSize{\n> > -\t\t\tcellSize.width * pixelArraySize[0] / 1e6f,\n> > -\t\t\tcellSize.height * pixelArraySize[1] / 1e6f\n> > +\t\t\tcellSize->width * pixelArraySize[0] / 1e6f,\n> > +\t\t\tcellSize->height * pixelArraySize[1] / 1e6f\n> > \t\t};\n> > \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_PHYSICAL_SIZE,\n> > \t\t\t\t\t physicalSize);\n> > @@ -1061,7 +1061,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> > \n> > \t{\n> > \t\tconst Span &rects =\n> > -\t\t\tproperties.get(properties::PixelArrayActiveAreas);\n> > +\t\t\tproperties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> > \t\tstd::vector data{\n> > \t\t\tstatic_cast(rects[0].x),\n> > \t\t\tstatic_cast(rects[0].y),\n> > @@ -1080,7 +1080,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> > \n> > \t/* Report the color filter arrangement if the camera reports it. */\n> > \tif (properties.contains(properties::draft::ColorFilterArrangement)) {\n> > -\t\tuint8_t filterArr = properties.get(properties::draft::ColorFilterArrangement);\n> > +\t\tuint8_t filterArr = *properties.get(properties::draft::ColorFilterArrangement);\n> > \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,\n> > \t\t\t\t\t filterArr);\n> > \t}\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 8e804d4d..ec117101 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -305,7 +305,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> > \tconst ControlList &properties = camera_->properties();\n> > \n> > \tif (properties.contains(properties::Location)) {\n> > -\t\tint32_t location = properties.get(properties::Location);\n> > +\t\tint32_t location = *properties.get(properties::Location);\n> > \t\tswitch (location) {\n> > \t\tcase properties::CameraLocationFront:\n> > \t\t\tfacing_ = CAMERA_FACING_FRONT;\n> > @@ -355,7 +355,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> > \t * metadata.\n> > \t */\n> > \tif (properties.contains(properties::Rotation)) {\n> > -\t\tint rotation = properties.get(properties::Rotation);\n> > +\t\tint rotation = *properties.get(properties::Rotation);\n> > \t\torientation_ = (360 - rotation) % 360;\n> > \t\tif (cameraConfigData && cameraConfigData->rotation != -1 &&\n> > \t\t orientation_ != cameraConfigData->rotation) {\n> > @@ -1094,7 +1094,8 @@ void CameraDevice::requestComplete(Request *request)\n> > \t * as soon as possible, earlier than request completion time.\n> > \t */\n> > \tuint64_t sensorTimestamp = static_cast(request->metadata()\n> > -\t\t\t\t\t\t\t .get(controls::SensorTimestamp));\n> > +\t\t\t\t\t\t\t\t .get(controls::SensorTimestamp)\n> > +\t\t\t\t\t\t\t\t .value_or(0));\n> > \tnotifyShutter(descriptor->frameNumber_, sensorTimestamp);\n> > \n> > \tLOG(HAL, Debug) << \"Request \" << request->cookie() << \" completed with \"\n> > @@ -1473,29 +1474,28 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> > \t\t\t\t rolling_shutter_skew);\n> > \n> > \t/* Add metadata tags reported by libcamera. */\n> > -\tconst int64_t timestamp = metadata.get(controls::SensorTimestamp);\n> > +\tconst int64_t timestamp = metadata.get(controls::SensorTimestamp).value_or(0);\n> > \tresultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, timestamp);\n> > \n> > \tif (metadata.contains(controls::draft::PipelineDepth)) {\n> > -\t\tuint8_t pipeline_depth =\n> > -\t\t\tmetadata.get(controls::draft::PipelineDepth);\n> > +\t\tuint8_t pipeline_depth = *metadata.get(controls::draft::PipelineDepth);\n> > \t\tresultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH,\n> > \t\t\t\t\t pipeline_depth);\n> > \t}\n> > \n> > \tif (metadata.contains(controls::ExposureTime)) {\n> > -\t\tint64_t exposure = metadata.get(controls::ExposureTime) * 1000ULL;\n> > +\t\tint64_t exposure = *metadata.get(controls::ExposureTime) * 1000ULL;\n> > \t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);\n> > \t}\n> > \n> > \tif (metadata.contains(controls::FrameDuration)) {\n> > -\t\tint64_t duration = metadata.get(controls::FrameDuration) * 1000;\n> > +\t\tint64_t duration = *metadata.get(controls::FrameDuration) * 1000;\n> > \t\tresultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> > \t\t\t\t\t duration);\n> > \t}\n> > \n> > \tif (metadata.contains(controls::ScalerCrop)) {\n> > -\t\tRectangle crop = metadata.get(controls::ScalerCrop);\n> > +\t\tRectangle crop = *metadata.get(controls::ScalerCrop);\n> > \t\tint32_t cropRect[] = {\n> > \t\t\tcrop.x, crop.y, static_cast(crop.width),\n> > \t\t\tstatic_cast(crop.height),\n> > @@ -1504,8 +1504,7 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> > \t}\n> > \n> > \tif (metadata.contains(controls::draft::TestPatternMode)) {\n> > -\t\tconst int32_t testPatternMode =\n> > -\t\t\tmetadata.get(controls::draft::TestPatternMode);\n> > +\t\tconst int32_t testPatternMode = *metadata.get(controls::draft::TestPatternMode);\n> > \t\tresultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,\n> > \t\t\t\t\t testPatternMode);\n> > \t}\n> > diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp\n> > index 5f7bfe26..0bffe96f 100644\n> > --- a/src/android/camera_hal_manager.cpp\n> > +++ b/src/android/camera_hal_manager.cpp\n> > @@ -232,7 +232,7 @@ int32_t CameraHalManager::cameraLocation(const Camera *cam)\n> > \tif (!properties.contains(properties::Location))\n> > \t\treturn -1;\n> > \n> > -\treturn properties.get(properties::Location);\n> > +\treturn *properties.get(properties::Location);\n> > }\n> > \n> > CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)\n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index 79875ed7..d8115cd8 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -301,7 +301,7 @@ std::string CamApp::cameraName(const Camera *camera)\n> > \t * is only used if the location isn't present or is set to External.\n> > \t */\n> > \tif (props.contains(properties::Location)) {\n> > -\t\tswitch (props.get(properties::Location)) {\n> > +\t\tswitch (*props.get(properties::Location)) {\n> > \t\tcase properties::CameraLocationFront:\n> > \t\t\taddModel = false;\n> > \t\t\tname = \"Internal front camera \";\n> > @@ -321,7 +321,7 @@ std::string CamApp::cameraName(const Camera *camera)\n> > \t\t * If the camera location is not availble use the camera model\n> > \t\t * to build the camera name.\n> > \t\t */\n> > -\t\tname = \"'\" + props.get(properties::Model) + \"' \";\n> > +\t\tname = \"'\" + *props.get(properties::Model) + \"' \";\n> > \t}\n> > \n> > \tname += \"(\" + camera->id() + \")\";\n> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> > index 3b126bb5..f65a0680 100644\n> > --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> > @@ -939,7 +939,7 @@ void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)\n> > \n> > void IPARPi::prepareISP(const ISPConfig &data)\n> > {\n> > -\tint64_t frameTimestamp = data.controls.get(controls::SensorTimestamp);\n> > +\tint64_t frameTimestamp = data.controls.get(controls::SensorTimestamp).value_or(0);\n> > \tRPiController::Metadata lastMetadata;\n> > \tSpan embeddedBuffer;\n> > \n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index b7dda282..43db7b68 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -1145,7 +1145,7 @@ int PipelineHandlerIPU3::registerCameras()\n> > \t\t/* Convert the sensor rotation to a transformation */\n> > \t\tint32_t rotation = 0;\n> > \t\tif (data->properties_.contains(properties::Rotation))\n> > -\t\t\trotation = data->properties_.get(properties::Rotation);\n> > +\t\t\trotation = *(data->properties_.get(properties::Rotation));\n> > \t\telse\n> > \t\t\tLOG(IPU3, Warning) << \"Rotation control not exposed by \"\n> > \t\t\t\t\t << cio2->sensor()->id()\n> > @@ -1331,7 +1331,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> > \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n> > \t/* \\todo Actually apply the scaler crop region to the ImgU. */\n> > \tif (request->controls().contains(controls::ScalerCrop))\n> > -\t\tcropRegion_ = request->controls().get(controls::ScalerCrop);\n> > +\t\tcropRegion_ = *(request->controls().get(controls::ScalerCrop));\n> > \trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n> > \n> > \tif (frameInfos_.tryComplete(info))\n> > @@ -1424,7 +1424,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n> > \t\treturn;\n> > \t}\n> > \n> > -\tipa_->processStatsBuffer(info->id, request->metadata().get(controls::SensorTimestamp),\n> > +\tipa_->processStatsBuffer(info->id, request->metadata().get(controls::SensorTimestamp).value_or(0),\n> > \t\t\t\t info->statBuffer->cookie(), info->effectiveSensorControls);\n> > }\n> > \n> > @@ -1458,8 +1458,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n> > \tif (!request->controls().contains(controls::draft::TestPatternMode))\n> > \t\treturn;\n> > \n> > -\tconst int32_t testPatternMode = request->controls().get(\n> > -\t\tcontrols::draft::TestPatternMode);\n> > +\tconst int32_t testPatternMode = *(request->controls().get(controls::draft::TestPatternMode));\n> > \n> > \tint ret = cio2_.sensor()->setTestPatternMode(\n> > \t\tstatic_cast(testPatternMode));\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index adc397e8..a62afdd4 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -365,7 +365,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> > \t * error means the platform can never run. Let's just print a warning\n> > \t * and continue regardless; the rotation is effectively set to zero.\n> > \t */\n> > -\tint32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n> > +\tint32_t rotation = data_->sensor_->properties().get(properties::Rotation).value_or(0);\n> > \tbool success;\n> > \tTransform rotationTransform = transformFromRotation(rotation, &success);\n> > \tif (!success)\n> > @@ -1706,7 +1706,8 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &\n> > \t * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is set).\n> > \t */\n> > \tif (notifyGainsUnity_ && controls.contains(libcamera::controls::ColourGains)) {\n> > -\t\tlibcamera::Span colourGains = controls.get(libcamera::controls::ColourGains);\n> > +\t\tlibcamera::Span colourGains =\n> > +\t\t\t*controls.get(libcamera::controls::ColourGains);\n> > \t\t/* The control wants linear gains in the order B, Gb, Gr, R. */\n> > \t\tControlList ctrls(sensor_->controls());\n> > \t\tstd::array gains{\n> > @@ -2041,7 +2042,7 @@ Rectangle RPiCameraData::scaleIspCrop(const Rectangle &ispCrop) const\n> > void RPiCameraData::applyScalerCrop(const ControlList &controls)\n> > {\n> > \tif (controls.contains(controls::ScalerCrop)) {\n> > -\t\tRectangle nativeCrop = controls.get(controls::ScalerCrop);\n> > +\t\tRectangle nativeCrop = *controls.get(controls::ScalerCrop);\n> > \n> > \t\tif (!nativeCrop.width || !nativeCrop.height)\n> > \t\t\tnativeCrop = { 0, 0, 1, 1 };\n> > @@ -2079,7 +2080,7 @@ void RPiCameraData::fillRequestMetadata(const ControlList &bufferControls,\n> > \t\t\t\t\tRequest *request)\n> > {\n> > \trequest->metadata().set(controls::SensorTimestamp,\n> > -\t\t\t\tbufferControls.get(controls::SensorTimestamp));\n> > +\t\t\t\tbufferControls.get(controls::SensorTimestamp).value_or(0));\n> > \n> > \trequest->metadata().set(controls::ScalerCrop, scalerCrop_);\n> > }\n> > diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> > index 34c8df5a..4b5d8276 100644\n> > --- a/src/qcam/dng_writer.cpp\n> > +++ b/src/qcam/dng_writer.cpp\n> > @@ -392,7 +392,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > \tTIFFSetField(tif, TIFFTAG_MAKE, \"libcamera\");\n> > \n> > \tif (cameraProperties.contains(properties::Model)) {\n> > -\t\tstd::string model = cameraProperties.get(properties::Model);\n> > +\t\tstd::string model = *cameraProperties.get(properties::Model);\n> > \t\tTIFFSetField(tif, TIFFTAG_MODEL, model.c_str());\n> > \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n> > \t}\n> > @@ -438,16 +438,15 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > \tconst double eps = 1e-2;\n> > \n> > \tif (metadata.contains(controls::ColourGains)) {\n> > -\t\tSpan const &colourGains = metadata.get(controls::ColourGains);\n> > -\t\tif (colourGains[0] > eps && colourGains[1] > eps) {\n> > -\t\t\twbGain = Matrix3d::diag(colourGains[0], 1, colourGains[1]);\n> > -\t\t\tneutral[0] = 1.0 / colourGains[0]; /* red */\n> > -\t\t\tneutral[2] = 1.0 / colourGains[1]; /* blue */\n> > +\t\tconst auto &colourGains = metadata.get(controls::ColourGains);\n> > +\t\tif ((*colourGains)[0] > eps && (*colourGains)[1] > eps) {\n> > +\t\t\twbGain = Matrix3d::diag((*colourGains)[0], 1, (*colourGains)[1]);\n> > +\t\t\tneutral[0] = 1.0 / (*colourGains)[0]; /* red */\n> > +\t\t\tneutral[2] = 1.0 / (*colourGains)[1]; /* blue */\n> > \t\t}\n> > \t}\n> > \tif (metadata.contains(controls::ColourCorrectionMatrix)) {\n> > -\t\tSpan const &coeffs = metadata.get(controls::ColourCorrectionMatrix);\n> > -\t\tMatrix3d ccmSupplied(coeffs);\n> > +\t\tMatrix3d ccmSupplied(*metadata.get(controls::ColourCorrectionMatrix));\n> > \t\tif (ccmSupplied.determinant() > eps)\n> > \t\t\tccm = ccmSupplied;\n> > \t}\n> > @@ -515,7 +514,8 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > \tuint32_t whiteLevel = (1 << info->bitsPerSample) - 1;\n> > \n> > \tif (metadata.contains(controls::SensorBlackLevels)) {\n> > -\t\tSpan levels = metadata.get(controls::SensorBlackLevels);\n> > +\t\tSpan levels =\n> > +\t\t\t*metadata.get(controls::SensorBlackLevels);\n> > \n> > \t\t/*\n> > \t\t * The black levels control is specified in R, Gr, Gb, B order.\n> > @@ -593,13 +593,13 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > \tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n> > \n> > \tif (metadata.contains(controls::AnalogueGain)) {\n> > -\t\tfloat gain = metadata.get(controls::AnalogueGain);\n> > +\t\tfloat gain = *metadata.get(controls::AnalogueGain);\n> > \t\tuint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f);\n> > \t\tTIFFSetField(tif, EXIFTAG_ISOSPEEDRATINGS, 1, &iso);\n> > \t}\n> > \n> > \tif (metadata.contains(controls::ExposureTime)) {\n> > -\t\tfloat exposureTime = metadata.get(controls::ExposureTime) / 1e6;\n> > +\t\tfloat exposureTime = *metadata.get(controls::ExposureTime) / 1e6;\n> > \t\tTIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);\n> > \t}\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 61327BE173\n\tfor ;\n\tSun, 3 Jul 2022 22:06:11 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B035F6564E;\n\tMon, 4 Jul 2022 00:06:10 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 84B856054C\n\tfor ;\n\tMon, 4 Jul 2022 00:06:08 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id EB5E149C;\n\tMon, 4 Jul 2022 00:06:07 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656885970;\n\tbh=UYNxNr8qiQs5HMzuLjdYR+g8DdAuKbOFqis+yaiqucQ=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=LlfyKq2+JNg86KwBlmj6mX5V845ISCsI0GWVUghHqi9tPUsMeHSgB5hSv2ht78Af5\n\t8iFrvEEEQIuBmxFL4CbPxjqKxGIHgR+Z/9ipWGMIepskELHPcvCOZKu0VTRKDonC3U\n\tEeFdZC7OJTPwPsgYkZclnVfNVFTLA41TqbPhoVW1rU0cfjbMCx9x9391bsus3K6wD3\n\tH/JL600wq5RqRys5qWZjbNc9hy3KfyICr33PKxmQoh5ZK0FKUHZuKW+STsxyDTMReZ\n\tGMngPiweoogdqUDy5S8tkSdLSZpeJRN5OGUSEZ7dpd5UiO5X2cspxErbQ/U2mZGV4/\n\twL1YIIY48w58Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656885968;\n\tbh=UYNxNr8qiQs5HMzuLjdYR+g8DdAuKbOFqis+yaiqucQ=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=XtGr8wwHb45PPId51mllmnJp1ymE90I8LWaGevBebGSCMRaCr4JayS5xoe7y8BVx/\n\tQHHu2agQ/NOQl3qZzqK6vphc0mbydmIXQ8KhrNxB9hU87go4+umRPr6rUwFlLxailD\n\tMoiw2vk49pBHM9ODv1VDpm2bUFOQrU7sQZOCYykg="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"XtGr8wwH\"; dkim-atps=neutral","Date":"Mon, 4 Jul 2022 01:05:46 +0300","To":"Christian Rauch ","Message-ID":"","References":"<20220610120338.96883-1-Rauch.Christian@gmx.de>\n\t<20220610120338.96883-2-Rauch.Christian@gmx.de>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v8 1/4] libcamera: controls: Use\n\tstd::optional to handle invalid control values","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23721,"web_url":"https://patchwork.libcamera.org/comment/23721/","msgid":"<0ea4afd9-f3fc-41a3-eee0-cae96188152c@gmx.de>","date":"2022-07-04T17:42:28","subject":"Re: [libcamera-devel] [PATCH v8 1/4] libcamera: controls: Use\n\tstd::optional to handle invalid control values","submitter":{"id":111,"url":"https://patchwork.libcamera.org/api/people/111/","name":"Christian Rauch","email":"Rauch.Christian@gmx.de"},"content":"Hi Laurent,\n\nI added gcc/g++ 10 to my CI pipeline [1] but I still cannot reproduce\nthis. I suggest that you take my commits and squash your fixes into them\nsuch that they compile in your setup.\n\nBest,\nChristian\n\n[1] https://github.com/christianrauch/libcamera-ci/actions/runs/2611618641\n\n\nAm 03.07.22 um 23:05 schrieb Laurent Pinchart:\n> On Mon, Jul 04, 2022 at 12:45:19AM +0300, Laurent Pinchart wrote:\n>> Hi Christian,\n>>\n>> Thank you for the patch.\n>>\n>> On Fri, Jun 10, 2022 at 01:03:35PM +0100, Christian Rauch via libcamera-devel wrote:\n>>> Previously, ControlList::get() would use default constructed objects to\n>>> indicate that a ControlList does not have the requested Control. This has\n>>> several disadvantages: 1) It requires types to be default constructible,\n>>> 2) it does not differentiate between a default constructed object and an\n>>> object that happens to have the same state as a default constructed object.\n>>>\n>>> std::optional additionally stores the information if the object is valid\n>>> or not, and therefore is more expressive than a default constructed object.\n>>>\n>>> Signed-off-by: Christian Rauch \n>>> Reviewed-by: Jacopo Mondi \n>>> ---\n>>> include/libcamera/controls.h | 5 +++--\n>>> src/android/camera_capabilities.cpp | 12 +++++-----\n>>> src/android/camera_device.cpp | 21 +++++++++---------\n>>> src/android/camera_hal_manager.cpp | 2 +-\n>>> src/cam/main.cpp | 4 ++--\n>>> src/ipa/raspberrypi/raspberrypi.cpp | 2 +-\n>>> src/libcamera/pipeline/ipu3/ipu3.cpp | 9 ++++----\n>>> .../pipeline/raspberrypi/raspberrypi.cpp | 9 ++++----\n>>> src/qcam/dng_writer.cpp | 22 +++++++++----------\n>>> 9 files changed, 43 insertions(+), 43 deletions(-)\n>>>\n>>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n>>> index 665bcac1..192be784 100644\n>>> --- a/include/libcamera/controls.h\n>>> +++ b/include/libcamera/controls.h\n>>> @@ -8,6 +8,7 @@\n>>> #pragma once\n>>>\n>>> #include \n>>> +#include \n>>> #include \n>>> #include \n>>> #include \n>>> @@ -373,11 +374,11 @@ public:\n>>> \tbool contains(unsigned int id) const;\n>>>\n>>> \ttemplate\n>>> -\tT get(const Control &ctrl) const\n>>> +\tstd::optional get(const Control &ctrl) const\n>>> \t{\n>>> \t\tconst ControlValue *val = find(ctrl.id());\n>>> \t\tif (!val)\n>>> -\t\t\treturn T{};\n>>> +\t\t\treturn std::nullopt;\n>>>\n>>> \t\treturn val->get();\n>>\n>> I think one of the perks of using std::optional here is that we could\n>> return a reference to the value instead of copying it. To avoid a v9,\n>> I'll send a patch on top.\n>>\n>>> \t}\n>>> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n>>> index 6f197eb8..5304b2da 100644\n>>> --- a/src/android/camera_capabilities.cpp\n>>> +++ b/src/android/camera_capabilities.cpp\n>>> @@ -1042,7 +1042,7 @@ int CameraCapabilities::initializeStaticMetadata()\n>>> \t/* Sensor static metadata. */\n>>> \tstd::array pixelArraySize;\n>>> \t{\n>>> -\t\tconst Size &size = properties.get(properties::PixelArraySize);\n>>> +\t\tconst Size &size = properties.get(properties::PixelArraySize).value_or(Size{});\n>>> \t\tpixelArraySize[0] = size.width;\n>>> \t\tpixelArraySize[1] = size.height;\n>>> \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,\n>>> @@ -1050,10 +1050,10 @@ int CameraCapabilities::initializeStaticMetadata()\n>>> \t}\n>>>\n>>> \tif (properties.contains(properties::UnitCellSize)) {\n>>> -\t\tconst Size &cellSize = properties.get(properties::UnitCellSize);\n>>> +\t\tconst auto &cellSize = properties.get(properties::UnitCellSize);\n>>\n>> We can avoid the double-lookup here and in several locations below. I'll\n>> send a patch on top too.\n>>\n>> Reviewed-by: Laurent Pinchart \n>\n> Turns out I spoke a bit too fast :-( When compiling with gcc 10.3.1, I'm\n> getting\n>\n> [27/218] Compiling C++ object src/android/libcamera-hal.so.p/camera_capabilities.cpp.o\n> FAILED: src/android/libcamera-hal.so.p/camera_capabilities.cpp.o\n> 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\n> In file included from ../../include/libcamera/camera.h:20,\n> from ../../src/android/camera_capabilities.h:17,\n> from ../../src/android/camera_capabilities.cpp:8:\n> ../../include/libcamera/controls.h: In member function ‘int CameraCapabilities::initializeStaticMetadata()’:\n> ../../include/libcamera/controls.h:381:16: error: ‘’ may be used uninitialized in this function [-Werror=maybe-uninitialized]\n> 381 | return std::nullopt;\n> | ^~~~~~~\n> ../../include/libcamera/controls.h:381:16: error: ‘*((void*)& +4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]\n>\n> The following change fixes it:\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 5304b2da1f93..429054ae3633 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -1049,8 +1049,8 @@ int CameraCapabilities::initializeStaticMetadata()\n> \t\t\t\t\t pixelArraySize);\n> \t}\n>\n> -\tif (properties.contains(properties::UnitCellSize)) {\n> -\t\tconst auto &cellSize = properties.get(properties::UnitCellSize);\n> +\tconst auto &cellSize = properties.get(properties::UnitCellSize);\n> +\tif (cellSize) {\n> \t\tstd::array physicalSize{\n> \t\t\tcellSize->width * pixelArraySize[0] / 1e6f,\n> \t\t\tcellSize->height * pixelArraySize[1] / 1e6f\n>\n> If we do that, I'd like to apply the same change through this patch. I\n> can post a v9 of this patch with this tomorrow if you don't beat me to\n> it.\n>\n>>> \t\tstd::array physicalSize{\n>>> -\t\t\tcellSize.width * pixelArraySize[0] / 1e6f,\n>>> -\t\t\tcellSize.height * pixelArraySize[1] / 1e6f\n>>> +\t\t\tcellSize->width * pixelArraySize[0] / 1e6f,\n>>> +\t\t\tcellSize->height * pixelArraySize[1] / 1e6f\n>>> \t\t};\n>>> \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_PHYSICAL_SIZE,\n>>> \t\t\t\t\t physicalSize);\n>>> @@ -1061,7 +1061,7 @@ int CameraCapabilities::initializeStaticMetadata()\n>>>\n>>> \t{\n>>> \t\tconst Span &rects =\n>>> -\t\t\tproperties.get(properties::PixelArrayActiveAreas);\n>>> +\t\t\tproperties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n>>> \t\tstd::vector data{\n>>> \t\t\tstatic_cast(rects[0].x),\n>>> \t\t\tstatic_cast(rects[0].y),\n>>> @@ -1080,7 +1080,7 @@ int CameraCapabilities::initializeStaticMetadata()\n>>>\n>>> \t/* Report the color filter arrangement if the camera reports it. */\n>>> \tif (properties.contains(properties::draft::ColorFilterArrangement)) {\n>>> -\t\tuint8_t filterArr = properties.get(properties::draft::ColorFilterArrangement);\n>>> +\t\tuint8_t filterArr = *properties.get(properties::draft::ColorFilterArrangement);\n>>> \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,\n>>> \t\t\t\t\t filterArr);\n>>> \t}\n>>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n>>> index 8e804d4d..ec117101 100644\n>>> --- a/src/android/camera_device.cpp\n>>> +++ b/src/android/camera_device.cpp\n>>> @@ -305,7 +305,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n>>> \tconst ControlList &properties = camera_->properties();\n>>>\n>>> \tif (properties.contains(properties::Location)) {\n>>> -\t\tint32_t location = properties.get(properties::Location);\n>>> +\t\tint32_t location = *properties.get(properties::Location);\n>>> \t\tswitch (location) {\n>>> \t\tcase properties::CameraLocationFront:\n>>> \t\t\tfacing_ = CAMERA_FACING_FRONT;\n>>> @@ -355,7 +355,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n>>> \t * metadata.\n>>> \t */\n>>> \tif (properties.contains(properties::Rotation)) {\n>>> -\t\tint rotation = properties.get(properties::Rotation);\n>>> +\t\tint rotation = *properties.get(properties::Rotation);\n>>> \t\torientation_ = (360 - rotation) % 360;\n>>> \t\tif (cameraConfigData && cameraConfigData->rotation != -1 &&\n>>> \t\t orientation_ != cameraConfigData->rotation) {\n>>> @@ -1094,7 +1094,8 @@ void CameraDevice::requestComplete(Request *request)\n>>> \t * as soon as possible, earlier than request completion time.\n>>> \t */\n>>> \tuint64_t sensorTimestamp = static_cast(request->metadata()\n>>> -\t\t\t\t\t\t\t .get(controls::SensorTimestamp));\n>>> +\t\t\t\t\t\t\t\t .get(controls::SensorTimestamp)\n>>> +\t\t\t\t\t\t\t\t .value_or(0));\n>>> \tnotifyShutter(descriptor->frameNumber_, sensorTimestamp);\n>>>\n>>> \tLOG(HAL, Debug) << \"Request \" << request->cookie() << \" completed with \"\n>>> @@ -1473,29 +1474,28 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n>>> \t\t\t\t rolling_shutter_skew);\n>>>\n>>> \t/* Add metadata tags reported by libcamera. */\n>>> -\tconst int64_t timestamp = metadata.get(controls::SensorTimestamp);\n>>> +\tconst int64_t timestamp = metadata.get(controls::SensorTimestamp).value_or(0);\n>>> \tresultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, timestamp);\n>>>\n>>> \tif (metadata.contains(controls::draft::PipelineDepth)) {\n>>> -\t\tuint8_t pipeline_depth =\n>>> -\t\t\tmetadata.get(controls::draft::PipelineDepth);\n>>> +\t\tuint8_t pipeline_depth = *metadata.get(controls::draft::PipelineDepth);\n>>> \t\tresultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH,\n>>> \t\t\t\t\t pipeline_depth);\n>>> \t}\n>>>\n>>> \tif (metadata.contains(controls::ExposureTime)) {\n>>> -\t\tint64_t exposure = metadata.get(controls::ExposureTime) * 1000ULL;\n>>> +\t\tint64_t exposure = *metadata.get(controls::ExposureTime) * 1000ULL;\n>>> \t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);\n>>> \t}\n>>>\n>>> \tif (metadata.contains(controls::FrameDuration)) {\n>>> -\t\tint64_t duration = metadata.get(controls::FrameDuration) * 1000;\n>>> +\t\tint64_t duration = *metadata.get(controls::FrameDuration) * 1000;\n>>> \t\tresultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n>>> \t\t\t\t\t duration);\n>>> \t}\n>>>\n>>> \tif (metadata.contains(controls::ScalerCrop)) {\n>>> -\t\tRectangle crop = metadata.get(controls::ScalerCrop);\n>>> +\t\tRectangle crop = *metadata.get(controls::ScalerCrop);\n>>> \t\tint32_t cropRect[] = {\n>>> \t\t\tcrop.x, crop.y, static_cast(crop.width),\n>>> \t\t\tstatic_cast(crop.height),\n>>> @@ -1504,8 +1504,7 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n>>> \t}\n>>>\n>>> \tif (metadata.contains(controls::draft::TestPatternMode)) {\n>>> -\t\tconst int32_t testPatternMode =\n>>> -\t\t\tmetadata.get(controls::draft::TestPatternMode);\n>>> +\t\tconst int32_t testPatternMode = *metadata.get(controls::draft::TestPatternMode);\n>>> \t\tresultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,\n>>> \t\t\t\t\t testPatternMode);\n>>> \t}\n>>> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp\n>>> index 5f7bfe26..0bffe96f 100644\n>>> --- a/src/android/camera_hal_manager.cpp\n>>> +++ b/src/android/camera_hal_manager.cpp\n>>> @@ -232,7 +232,7 @@ int32_t CameraHalManager::cameraLocation(const Camera *cam)\n>>> \tif (!properties.contains(properties::Location))\n>>> \t\treturn -1;\n>>>\n>>> -\treturn properties.get(properties::Location);\n>>> +\treturn *properties.get(properties::Location);\n>>> }\n>>>\n>>> CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)\n>>> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n>>> index 79875ed7..d8115cd8 100644\n>>> --- a/src/cam/main.cpp\n>>> +++ b/src/cam/main.cpp\n>>> @@ -301,7 +301,7 @@ std::string CamApp::cameraName(const Camera *camera)\n>>> \t * is only used if the location isn't present or is set to External.\n>>> \t */\n>>> \tif (props.contains(properties::Location)) {\n>>> -\t\tswitch (props.get(properties::Location)) {\n>>> +\t\tswitch (*props.get(properties::Location)) {\n>>> \t\tcase properties::CameraLocationFront:\n>>> \t\t\taddModel = false;\n>>> \t\t\tname = \"Internal front camera \";\n>>> @@ -321,7 +321,7 @@ std::string CamApp::cameraName(const Camera *camera)\n>>> \t\t * If the camera location is not availble use the camera model\n>>> \t\t * to build the camera name.\n>>> \t\t */\n>>> -\t\tname = \"'\" + props.get(properties::Model) + \"' \";\n>>> +\t\tname = \"'\" + *props.get(properties::Model) + \"' \";\n>>> \t}\n>>>\n>>> \tname += \"(\" + camera->id() + \")\";\n>>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n>>> index 3b126bb5..f65a0680 100644\n>>> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n>>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n>>> @@ -939,7 +939,7 @@ void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)\n>>>\n>>> void IPARPi::prepareISP(const ISPConfig &data)\n>>> {\n>>> -\tint64_t frameTimestamp = data.controls.get(controls::SensorTimestamp);\n>>> +\tint64_t frameTimestamp = data.controls.get(controls::SensorTimestamp).value_or(0);\n>>> \tRPiController::Metadata lastMetadata;\n>>> \tSpan embeddedBuffer;\n>>>\n>>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> index b7dda282..43db7b68 100644\n>>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n>>> @@ -1145,7 +1145,7 @@ int PipelineHandlerIPU3::registerCameras()\n>>> \t\t/* Convert the sensor rotation to a transformation */\n>>> \t\tint32_t rotation = 0;\n>>> \t\tif (data->properties_.contains(properties::Rotation))\n>>> -\t\t\trotation = data->properties_.get(properties::Rotation);\n>>> +\t\t\trotation = *(data->properties_.get(properties::Rotation));\n>>> \t\telse\n>>> \t\t\tLOG(IPU3, Warning) << \"Rotation control not exposed by \"\n>>> \t\t\t\t\t << cio2->sensor()->id()\n>>> @@ -1331,7 +1331,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n>>> \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n>>> \t/* \\todo Actually apply the scaler crop region to the ImgU. */\n>>> \tif (request->controls().contains(controls::ScalerCrop))\n>>> -\t\tcropRegion_ = request->controls().get(controls::ScalerCrop);\n>>> +\t\tcropRegion_ = *(request->controls().get(controls::ScalerCrop));\n>>> \trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n>>>\n>>> \tif (frameInfos_.tryComplete(info))\n>>> @@ -1424,7 +1424,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n>>> \t\treturn;\n>>> \t}\n>>>\n>>> -\tipa_->processStatsBuffer(info->id, request->metadata().get(controls::SensorTimestamp),\n>>> +\tipa_->processStatsBuffer(info->id, request->metadata().get(controls::SensorTimestamp).value_or(0),\n>>> \t\t\t\t info->statBuffer->cookie(), info->effectiveSensorControls);\n>>> }\n>>>\n>>> @@ -1458,8 +1458,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n>>> \tif (!request->controls().contains(controls::draft::TestPatternMode))\n>>> \t\treturn;\n>>>\n>>> -\tconst int32_t testPatternMode = request->controls().get(\n>>> -\t\tcontrols::draft::TestPatternMode);\n>>> +\tconst int32_t testPatternMode = *(request->controls().get(controls::draft::TestPatternMode));\n>>>\n>>> \tint ret = cio2_.sensor()->setTestPatternMode(\n>>> \t\tstatic_cast(testPatternMode));\n>>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>>> index adc397e8..a62afdd4 100644\n>>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n>>> @@ -365,7 +365,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n>>> \t * error means the platform can never run. Let's just print a warning\n>>> \t * and continue regardless; the rotation is effectively set to zero.\n>>> \t */\n>>> -\tint32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n>>> +\tint32_t rotation = data_->sensor_->properties().get(properties::Rotation).value_or(0);\n>>> \tbool success;\n>>> \tTransform rotationTransform = transformFromRotation(rotation, &success);\n>>> \tif (!success)\n>>> @@ -1706,7 +1706,8 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &\n>>> \t * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is set).\n>>> \t */\n>>> \tif (notifyGainsUnity_ && controls.contains(libcamera::controls::ColourGains)) {\n>>> -\t\tlibcamera::Span colourGains = controls.get(libcamera::controls::ColourGains);\n>>> +\t\tlibcamera::Span colourGains =\n>>> +\t\t\t*controls.get(libcamera::controls::ColourGains);\n>>> \t\t/* The control wants linear gains in the order B, Gb, Gr, R. */\n>>> \t\tControlList ctrls(sensor_->controls());\n>>> \t\tstd::array gains{\n>>> @@ -2041,7 +2042,7 @@ Rectangle RPiCameraData::scaleIspCrop(const Rectangle &ispCrop) const\n>>> void RPiCameraData::applyScalerCrop(const ControlList &controls)\n>>> {\n>>> \tif (controls.contains(controls::ScalerCrop)) {\n>>> -\t\tRectangle nativeCrop = controls.get(controls::ScalerCrop);\n>>> +\t\tRectangle nativeCrop = *controls.get(controls::ScalerCrop);\n>>>\n>>> \t\tif (!nativeCrop.width || !nativeCrop.height)\n>>> \t\t\tnativeCrop = { 0, 0, 1, 1 };\n>>> @@ -2079,7 +2080,7 @@ void RPiCameraData::fillRequestMetadata(const ControlList &bufferControls,\n>>> \t\t\t\t\tRequest *request)\n>>> {\n>>> \trequest->metadata().set(controls::SensorTimestamp,\n>>> -\t\t\t\tbufferControls.get(controls::SensorTimestamp));\n>>> +\t\t\t\tbufferControls.get(controls::SensorTimestamp).value_or(0));\n>>>\n>>> \trequest->metadata().set(controls::ScalerCrop, scalerCrop_);\n>>> }\n>>> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n>>> index 34c8df5a..4b5d8276 100644\n>>> --- a/src/qcam/dng_writer.cpp\n>>> +++ b/src/qcam/dng_writer.cpp\n>>> @@ -392,7 +392,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n>>> \tTIFFSetField(tif, TIFFTAG_MAKE, \"libcamera\");\n>>>\n>>> \tif (cameraProperties.contains(properties::Model)) {\n>>> -\t\tstd::string model = cameraProperties.get(properties::Model);\n>>> +\t\tstd::string model = *cameraProperties.get(properties::Model);\n>>> \t\tTIFFSetField(tif, TIFFTAG_MODEL, model.c_str());\n>>> \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n>>> \t}\n>>> @@ -438,16 +438,15 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n>>> \tconst double eps = 1e-2;\n>>>\n>>> \tif (metadata.contains(controls::ColourGains)) {\n>>> -\t\tSpan const &colourGains = metadata.get(controls::ColourGains);\n>>> -\t\tif (colourGains[0] > eps && colourGains[1] > eps) {\n>>> -\t\t\twbGain = Matrix3d::diag(colourGains[0], 1, colourGains[1]);\n>>> -\t\t\tneutral[0] = 1.0 / colourGains[0]; /* red */\n>>> -\t\t\tneutral[2] = 1.0 / colourGains[1]; /* blue */\n>>> +\t\tconst auto &colourGains = metadata.get(controls::ColourGains);\n>>> +\t\tif ((*colourGains)[0] > eps && (*colourGains)[1] > eps) {\n>>> +\t\t\twbGain = Matrix3d::diag((*colourGains)[0], 1, (*colourGains)[1]);\n>>> +\t\t\tneutral[0] = 1.0 / (*colourGains)[0]; /* red */\n>>> +\t\t\tneutral[2] = 1.0 / (*colourGains)[1]; /* blue */\n>>> \t\t}\n>>> \t}\n>>> \tif (metadata.contains(controls::ColourCorrectionMatrix)) {\n>>> -\t\tSpan const &coeffs = metadata.get(controls::ColourCorrectionMatrix);\n>>> -\t\tMatrix3d ccmSupplied(coeffs);\n>>> +\t\tMatrix3d ccmSupplied(*metadata.get(controls::ColourCorrectionMatrix));\n>>> \t\tif (ccmSupplied.determinant() > eps)\n>>> \t\t\tccm = ccmSupplied;\n>>> \t}\n>>> @@ -515,7 +514,8 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n>>> \tuint32_t whiteLevel = (1 << info->bitsPerSample) - 1;\n>>>\n>>> \tif (metadata.contains(controls::SensorBlackLevels)) {\n>>> -\t\tSpan levels = metadata.get(controls::SensorBlackLevels);\n>>> +\t\tSpan levels =\n>>> +\t\t\t*metadata.get(controls::SensorBlackLevels);\n>>>\n>>> \t\t/*\n>>> \t\t * The black levels control is specified in R, Gr, Gb, B order.\n>>> @@ -593,13 +593,13 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n>>> \tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n>>>\n>>> \tif (metadata.contains(controls::AnalogueGain)) {\n>>> -\t\tfloat gain = metadata.get(controls::AnalogueGain);\n>>> +\t\tfloat gain = *metadata.get(controls::AnalogueGain);\n>>> \t\tuint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f);\n>>> \t\tTIFFSetField(tif, EXIFTAG_ISOSPEEDRATINGS, 1, &iso);\n>>> \t}\n>>>\n>>> \tif (metadata.contains(controls::ExposureTime)) {\n>>> -\t\tfloat exposureTime = metadata.get(controls::ExposureTime) / 1e6;\n>>> +\t\tfloat exposureTime = *metadata.get(controls::ExposureTime) / 1e6;\n>>> \t\tTIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);\n>>> \t}\n>>>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 2DB9BBE173\n\tfor ;\n\tMon, 4 Jul 2022 17:42:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 633C96564E;\n\tMon, 4 Jul 2022 19:42:32 +0200 (CEST)","from mout.gmx.net (mout.gmx.net [212.227.17.21])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AA57261FB1\n\tfor ;\n\tMon, 4 Jul 2022 19:42:30 +0200 (CEST)","from [192.168.1.209] ([92.18.80.244]) by mail.gmx.net (mrgmx105\n\t[212.227.17.168]) with ESMTPSA (Nemesis) id 1MbivG-1nc6wa04xq-00dD9R\n\tfor\n\t; Mon, 04 Jul 2022 19:42:30 +0200"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656956552;\n\tbh=ucTwYsDfbTIqns5ZWHc0OV4LKEHbY59ZDxXJUyPp6Pg=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=O5mQ/SwGgI2331unr4yT6tRKMu06LObx4tnsNK0soSeMC5jJmZZYFuDvG5iNdMv7I\n\tR7OMZjHtlxR83nWuINi8VSMeyiDnkcgM1IbYTIlK2Mdkdu8T4p7L2i0wQCadITnL9k\n\tVoga7hkKZxutlanWfuKZ9I2i/EdUFNtLRXgf4VNxCqOSJ27I39lzs3gIWVBz3KFn2Y\n\tTrLhf0DvdWI5mdk3T8hSjF8sr7R996qViE+g8Mc3EL31FxDFSu2CUTYU7dvvcJk74N\n\tm6Ldcd77ogKCwPbjmC53KFPfCJV18/GMBvXvaNflwG9BZdnTqGZP0edBfl6urPsEAW\n\ttDHQOwM/W98Jw==","v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net;\n\ts=badeba3b8450; t=1656956550;\n\tbh=ucTwYsDfbTIqns5ZWHc0OV4LKEHbY59ZDxXJUyPp6Pg=;\n\th=X-UI-Sender-Class:Date:Subject:To:References:From:In-Reply-To;\n\tb=GZDWPOg+ochPv1yu1aTTa4gGK8/sn8cbBIbse3xc2F38ui3yr66oyz4NpwagHW7zo\n\t9VkYT6gwCb6LSH34csPi4QktAungtRwos6yF72n09TAeUifVdWUtS4XJzKk/768dn7\n\tlybxOf7C1qVF2rdRWTBxs2J4d2jIcHWYxO0CEUlE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=gmx.net header.i=@gmx.net\n\theader.b=\"GZDWPOg+\"; dkim-atps=neutral","X-UI-Sender-Class":"01bb95c1-4bf8-414a-932a-4f6e2808ef9c","Message-ID":"<0ea4afd9-f3fc-41a3-eee0-cae96188152c@gmx.de>","Date":"Mon, 4 Jul 2022 18:42:28 +0100","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.9.1","Content-Language":"en-GB","To":"libcamera-devel@lists.libcamera.org","References":"<20220610120338.96883-1-Rauch.Christian@gmx.de>\n\t<20220610120338.96883-2-Rauch.Christian@gmx.de>\n\t\n\t","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"quoted-printable","X-Provags-ID":"V03:K1:WEuNMnn2td5+gagy9tP08sAG/b2UqnGIkvsSWDJ5VTLkzPRTuNe\n\tlym965BMnV/dBMqg1l6iVcJHnc2Ir6bugFasrQaZa+OlOvdJmdxENej7/Y2bgTSgkOU2Adb\n\tlQnlfIxjABqWVj5wjHpwlJuZzZQcYAOQrUW+QbP/xeY09C4Zer7z6ZJQhhgYwSXkm5MUikC\n\tYVBKhBOS/ExzV9uVsX/1g==","X-Spam-Flag":"NO","X-UI-Out-Filterresults":"notjunk:1; V03:K0:juEAQqs7Vs0=:CtYLXz5Slm3EiCo2pIAu6I\n\ttNX0yPsEL1fLmM+GERyur/AaDWG7P/8Rozl8+T+zyZ9p1npc93gYS7vBTq/dUXsSe+yjRi3wX\n\tZ7HqjdTribP/J8kF7feqOg3gX9hPV50spPcCbcLKOM9o0ebRRESQt3vPt2inOVzrjWFX2PqzL\n\tSL6x7RZxLm3r05KfBfW508Z8swP0wN+yc18ytzkbfwrA1vwQlMFEjrdb+fmowwz05YWWnsuYH\n\ttwPgvXvkjXxw27VcV/fxB+DP+oS1KAOrnVzjxgL3M60uf4d0sallK0SryQg87ZdP/Uu9OHXYe\n\tFEqtrN81Wu23aqdQk6YOENdNEgQjcsJFHc9k2KUDJmMTCOiIfp0q0Ce0Uqie0Db/r5EaHcNZ+\n\tLq0RYuTDLp2cgNocCoqZ7zh3Y9buzwPzWcukSkA2whYRcBuyoLltqbMYaFlXU5xcJHpQm+R2h\n\tVaeKsVo6cK3xg2WIf4irJtz6ecxwxqRL5S8qUJB9Tcx4XyRQ0Lyi8jca6tchES51CIbxypjI6\n\tsxtX7awX4sEOnUzXvNubmizb4Gd9PkOx1ah3pEF4RexQJA2fW1TbbanS0OjpXAvRMcNHFoqaA\n\tPwriacMwt9E7YY5mjxKXNPEAen3VDeobiLc15xlODNfXUYZD2smczIcWJjhKFi1/4saKGX/bK\n\tTDnKhKIlxkAU0ZkEDfs/WpF9VIclaGl71yuvkaAH6C4nr5lXM/crNlHF69hluDB9KnbIvgJO8\n\tvFQbd5IyQd4r+m7UwQIl2jS7aOjMGHT3g12TzF4jz0WvixDJOqIIfp2NlFfI87y2DK3i6oQvn\n\thlJ3RQCMIaH3PNeXECjpHIb4V9Q5xRbuYW9TY/Te+S74y2SDzDYPcZVbf9hCucp/+Ij0urgRz\n\t3bz2Oihd4NeK9NfHyPbOXIhctO/0cHt/HkCpbeD1GVnx4cAcdBREfgOAQ7PunR2V7IhDkAMNH\n\t4fzbgMCAMP10clLnejRNk3Hhqev589ezZZTX5n97Ez2ltOYKoDFo61By7lS6oscfWpLRi0X2k\n\tZ69Vr8rdKz+gramJCKguuVuMGTccDIpboD9nidJftihuh764jJihzpl90nxE41eqPuuBV3jLp\n\tYpBzxKmjv92K7jXadB13QppiByajZt70SLjA+K9qqXbHhUgkoIT1S3pjQ==","Subject":"Re: [libcamera-devel] [PATCH v8 1/4] libcamera: controls: Use\n\tstd::optional to handle invalid control values","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Christian Rauch via libcamera-devel\n\t","Reply-To":"Christian Rauch ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23722,"web_url":"https://patchwork.libcamera.org/comment/23722/","msgid":"","date":"2022-07-04T17:50:58","subject":"Re: [libcamera-devel] [PATCH v8 1/4] libcamera: controls: Use\n\tstd::optional to handle invalid control values","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Christian,\n\nOn Mon, Jul 04, 2022 at 06:42:28PM +0100, Christian Rauch via libcamera-devel wrote:\n> Hi Laurent,\n> \n> I added gcc/g++ 10 to my CI pipeline [1] but I still cannot reproduce\n> this. I suggest that you take my commits and squash your fixes into them\n> such that they compile in your setup.\n\nI'll send a v9 of this patch for your review :-)\n\n> [1] https://github.com/christianrauch/libcamera-ci/actions/runs/2611618641\n> \n> Am 03.07.22 um 23:05 schrieb Laurent Pinchart:\n> > On Mon, Jul 04, 2022 at 12:45:19AM +0300, Laurent Pinchart wrote:\n> >> On Fri, Jun 10, 2022 at 01:03:35PM +0100, Christian Rauch via libcamera-devel wrote:\n> >>> Previously, ControlList::get() would use default constructed objects to\n> >>> indicate that a ControlList does not have the requested Control. This has\n> >>> several disadvantages: 1) It requires types to be default constructible,\n> >>> 2) it does not differentiate between a default constructed object and an\n> >>> object that happens to have the same state as a default constructed object.\n> >>>\n> >>> std::optional additionally stores the information if the object is valid\n> >>> or not, and therefore is more expressive than a default constructed object.\n> >>>\n> >>> Signed-off-by: Christian Rauch \n> >>> Reviewed-by: Jacopo Mondi \n> >>> ---\n> >>> include/libcamera/controls.h | 5 +++--\n> >>> src/android/camera_capabilities.cpp | 12 +++++-----\n> >>> src/android/camera_device.cpp | 21 +++++++++---------\n> >>> src/android/camera_hal_manager.cpp | 2 +-\n> >>> src/cam/main.cpp | 4 ++--\n> >>> src/ipa/raspberrypi/raspberrypi.cpp | 2 +-\n> >>> src/libcamera/pipeline/ipu3/ipu3.cpp | 9 ++++----\n> >>> .../pipeline/raspberrypi/raspberrypi.cpp | 9 ++++----\n> >>> src/qcam/dng_writer.cpp | 22 +++++++++----------\n> >>> 9 files changed, 43 insertions(+), 43 deletions(-)\n> >>>\n> >>> diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\n> >>> index 665bcac1..192be784 100644\n> >>> --- a/include/libcamera/controls.h\n> >>> +++ b/include/libcamera/controls.h\n> >>> @@ -8,6 +8,7 @@\n> >>> #pragma once\n> >>>\n> >>> #include \n> >>> +#include \n> >>> #include \n> >>> #include \n> >>> #include \n> >>> @@ -373,11 +374,11 @@ public:\n> >>> \tbool contains(unsigned int id) const;\n> >>>\n> >>> \ttemplate\n> >>> -\tT get(const Control &ctrl) const\n> >>> +\tstd::optional get(const Control &ctrl) const\n> >>> \t{\n> >>> \t\tconst ControlValue *val = find(ctrl.id());\n> >>> \t\tif (!val)\n> >>> -\t\t\treturn T{};\n> >>> +\t\t\treturn std::nullopt;\n> >>>\n> >>> \t\treturn val->get();\n> >>\n> >> I think one of the perks of using std::optional here is that we could\n> >> return a reference to the value instead of copying it. To avoid a v9,\n> >> I'll send a patch on top.\n> >>\n> >>> \t}\n> >>> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> >>> index 6f197eb8..5304b2da 100644\n> >>> --- a/src/android/camera_capabilities.cpp\n> >>> +++ b/src/android/camera_capabilities.cpp\n> >>> @@ -1042,7 +1042,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> >>> \t/* Sensor static metadata. */\n> >>> \tstd::array pixelArraySize;\n> >>> \t{\n> >>> -\t\tconst Size &size = properties.get(properties::PixelArraySize);\n> >>> +\t\tconst Size &size = properties.get(properties::PixelArraySize).value_or(Size{});\n> >>> \t\tpixelArraySize[0] = size.width;\n> >>> \t\tpixelArraySize[1] = size.height;\n> >>> \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE,\n> >>> @@ -1050,10 +1050,10 @@ int CameraCapabilities::initializeStaticMetadata()\n> >>> \t}\n> >>>\n> >>> \tif (properties.contains(properties::UnitCellSize)) {\n> >>> -\t\tconst Size &cellSize = properties.get(properties::UnitCellSize);\n> >>> +\t\tconst auto &cellSize = properties.get(properties::UnitCellSize);\n> >>\n> >> We can avoid the double-lookup here and in several locations below. I'll\n> >> send a patch on top too.\n> >>\n> >> Reviewed-by: Laurent Pinchart \n> >\n> > Turns out I spoke a bit too fast :-( When compiling with gcc 10.3.1, I'm\n> > getting\n> >\n> > [27/218] Compiling C++ object src/android/libcamera-hal.so.p/camera_capabilities.cpp.o\n> > FAILED: src/android/libcamera-hal.so.p/camera_capabilities.cpp.o\n> > 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\n> > In file included from ../../include/libcamera/camera.h:20,\n> > from ../../src/android/camera_capabilities.h:17,\n> > from ../../src/android/camera_capabilities.cpp:8:\n> > ../../include/libcamera/controls.h: In member function ‘int CameraCapabilities::initializeStaticMetadata()’:\n> > ../../include/libcamera/controls.h:381:16: error: ‘’ may be used uninitialized in this function [-Werror=maybe-uninitialized]\n> > 381 | return std::nullopt;\n> > | ^~~~~~~\n> > ../../include/libcamera/controls.h:381:16: error: ‘*((void*)& +4)’ may be used uninitialized in this function [-Werror=maybe-uninitialized]\n> >\n> > The following change fixes it:\n> >\n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index 5304b2da1f93..429054ae3633 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -1049,8 +1049,8 @@ int CameraCapabilities::initializeStaticMetadata()\n> > \t\t\t\t\t pixelArraySize);\n> > \t}\n> >\n> > -\tif (properties.contains(properties::UnitCellSize)) {\n> > -\t\tconst auto &cellSize = properties.get(properties::UnitCellSize);\n> > +\tconst auto &cellSize = properties.get(properties::UnitCellSize);\n> > +\tif (cellSize) {\n> > \t\tstd::array physicalSize{\n> > \t\t\tcellSize->width * pixelArraySize[0] / 1e6f,\n> > \t\t\tcellSize->height * pixelArraySize[1] / 1e6f\n> >\n> > If we do that, I'd like to apply the same change through this patch. I\n> > can post a v9 of this patch with this tomorrow if you don't beat me to\n> > it.\n> >\n> >>> \t\tstd::array physicalSize{\n> >>> -\t\t\tcellSize.width * pixelArraySize[0] / 1e6f,\n> >>> -\t\t\tcellSize.height * pixelArraySize[1] / 1e6f\n> >>> +\t\t\tcellSize->width * pixelArraySize[0] / 1e6f,\n> >>> +\t\t\tcellSize->height * pixelArraySize[1] / 1e6f\n> >>> \t\t};\n> >>> \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_PHYSICAL_SIZE,\n> >>> \t\t\t\t\t physicalSize);\n> >>> @@ -1061,7 +1061,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> >>>\n> >>> \t{\n> >>> \t\tconst Span &rects =\n> >>> -\t\t\tproperties.get(properties::PixelArrayActiveAreas);\n> >>> +\t\t\tproperties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> >>> \t\tstd::vector data{\n> >>> \t\t\tstatic_cast(rects[0].x),\n> >>> \t\t\tstatic_cast(rects[0].y),\n> >>> @@ -1080,7 +1080,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> >>>\n> >>> \t/* Report the color filter arrangement if the camera reports it. */\n> >>> \tif (properties.contains(properties::draft::ColorFilterArrangement)) {\n> >>> -\t\tuint8_t filterArr = properties.get(properties::draft::ColorFilterArrangement);\n> >>> +\t\tuint8_t filterArr = *properties.get(properties::draft::ColorFilterArrangement);\n> >>> \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,\n> >>> \t\t\t\t\t filterArr);\n> >>> \t}\n> >>> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> >>> index 8e804d4d..ec117101 100644\n> >>> --- a/src/android/camera_device.cpp\n> >>> +++ b/src/android/camera_device.cpp\n> >>> @@ -305,7 +305,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> >>> \tconst ControlList &properties = camera_->properties();\n> >>>\n> >>> \tif (properties.contains(properties::Location)) {\n> >>> -\t\tint32_t location = properties.get(properties::Location);\n> >>> +\t\tint32_t location = *properties.get(properties::Location);\n> >>> \t\tswitch (location) {\n> >>> \t\tcase properties::CameraLocationFront:\n> >>> \t\t\tfacing_ = CAMERA_FACING_FRONT;\n> >>> @@ -355,7 +355,7 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> >>> \t * metadata.\n> >>> \t */\n> >>> \tif (properties.contains(properties::Rotation)) {\n> >>> -\t\tint rotation = properties.get(properties::Rotation);\n> >>> +\t\tint rotation = *properties.get(properties::Rotation);\n> >>> \t\torientation_ = (360 - rotation) % 360;\n> >>> \t\tif (cameraConfigData && cameraConfigData->rotation != -1 &&\n> >>> \t\t orientation_ != cameraConfigData->rotation) {\n> >>> @@ -1094,7 +1094,8 @@ void CameraDevice::requestComplete(Request *request)\n> >>> \t * as soon as possible, earlier than request completion time.\n> >>> \t */\n> >>> \tuint64_t sensorTimestamp = static_cast(request->metadata()\n> >>> -\t\t\t\t\t\t\t .get(controls::SensorTimestamp));\n> >>> +\t\t\t\t\t\t\t\t .get(controls::SensorTimestamp)\n> >>> +\t\t\t\t\t\t\t\t .value_or(0));\n> >>> \tnotifyShutter(descriptor->frameNumber_, sensorTimestamp);\n> >>>\n> >>> \tLOG(HAL, Debug) << \"Request \" << request->cookie() << \" completed with \"\n> >>> @@ -1473,29 +1474,28 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> >>> \t\t\t\t rolling_shutter_skew);\n> >>>\n> >>> \t/* Add metadata tags reported by libcamera. */\n> >>> -\tconst int64_t timestamp = metadata.get(controls::SensorTimestamp);\n> >>> +\tconst int64_t timestamp = metadata.get(controls::SensorTimestamp).value_or(0);\n> >>> \tresultMetadata->addEntry(ANDROID_SENSOR_TIMESTAMP, timestamp);\n> >>>\n> >>> \tif (metadata.contains(controls::draft::PipelineDepth)) {\n> >>> -\t\tuint8_t pipeline_depth =\n> >>> -\t\t\tmetadata.get(controls::draft::PipelineDepth);\n> >>> +\t\tuint8_t pipeline_depth = *metadata.get(controls::draft::PipelineDepth);\n> >>> \t\tresultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH,\n> >>> \t\t\t\t\t pipeline_depth);\n> >>> \t}\n> >>>\n> >>> \tif (metadata.contains(controls::ExposureTime)) {\n> >>> -\t\tint64_t exposure = metadata.get(controls::ExposureTime) * 1000ULL;\n> >>> +\t\tint64_t exposure = *metadata.get(controls::ExposureTime) * 1000ULL;\n> >>> \t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);\n> >>> \t}\n> >>>\n> >>> \tif (metadata.contains(controls::FrameDuration)) {\n> >>> -\t\tint64_t duration = metadata.get(controls::FrameDuration) * 1000;\n> >>> +\t\tint64_t duration = *metadata.get(controls::FrameDuration) * 1000;\n> >>> \t\tresultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> >>> \t\t\t\t\t duration);\n> >>> \t}\n> >>>\n> >>> \tif (metadata.contains(controls::ScalerCrop)) {\n> >>> -\t\tRectangle crop = metadata.get(controls::ScalerCrop);\n> >>> +\t\tRectangle crop = *metadata.get(controls::ScalerCrop);\n> >>> \t\tint32_t cropRect[] = {\n> >>> \t\t\tcrop.x, crop.y, static_cast(crop.width),\n> >>> \t\t\tstatic_cast(crop.height),\n> >>> @@ -1504,8 +1504,7 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> >>> \t}\n> >>>\n> >>> \tif (metadata.contains(controls::draft::TestPatternMode)) {\n> >>> -\t\tconst int32_t testPatternMode =\n> >>> -\t\t\tmetadata.get(controls::draft::TestPatternMode);\n> >>> +\t\tconst int32_t testPatternMode = *metadata.get(controls::draft::TestPatternMode);\n> >>> \t\tresultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,\n> >>> \t\t\t\t\t testPatternMode);\n> >>> \t}\n> >>> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp\n> >>> index 5f7bfe26..0bffe96f 100644\n> >>> --- a/src/android/camera_hal_manager.cpp\n> >>> +++ b/src/android/camera_hal_manager.cpp\n> >>> @@ -232,7 +232,7 @@ int32_t CameraHalManager::cameraLocation(const Camera *cam)\n> >>> \tif (!properties.contains(properties::Location))\n> >>> \t\treturn -1;\n> >>>\n> >>> -\treturn properties.get(properties::Location);\n> >>> +\treturn *properties.get(properties::Location);\n> >>> }\n> >>>\n> >>> CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)\n> >>> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> >>> index 79875ed7..d8115cd8 100644\n> >>> --- a/src/cam/main.cpp\n> >>> +++ b/src/cam/main.cpp\n> >>> @@ -301,7 +301,7 @@ std::string CamApp::cameraName(const Camera *camera)\n> >>> \t * is only used if the location isn't present or is set to External.\n> >>> \t */\n> >>> \tif (props.contains(properties::Location)) {\n> >>> -\t\tswitch (props.get(properties::Location)) {\n> >>> +\t\tswitch (*props.get(properties::Location)) {\n> >>> \t\tcase properties::CameraLocationFront:\n> >>> \t\t\taddModel = false;\n> >>> \t\t\tname = \"Internal front camera \";\n> >>> @@ -321,7 +321,7 @@ std::string CamApp::cameraName(const Camera *camera)\n> >>> \t\t * If the camera location is not availble use the camera model\n> >>> \t\t * to build the camera name.\n> >>> \t\t */\n> >>> -\t\tname = \"'\" + props.get(properties::Model) + \"' \";\n> >>> +\t\tname = \"'\" + *props.get(properties::Model) + \"' \";\n> >>> \t}\n> >>>\n> >>> \tname += \"(\" + camera->id() + \")\";\n> >>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\n> >>> index 3b126bb5..f65a0680 100644\n> >>> --- a/src/ipa/raspberrypi/raspberrypi.cpp\n> >>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp\n> >>> @@ -939,7 +939,7 @@ void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)\n> >>>\n> >>> void IPARPi::prepareISP(const ISPConfig &data)\n> >>> {\n> >>> -\tint64_t frameTimestamp = data.controls.get(controls::SensorTimestamp);\n> >>> +\tint64_t frameTimestamp = data.controls.get(controls::SensorTimestamp).value_or(0);\n> >>> \tRPiController::Metadata lastMetadata;\n> >>> \tSpan embeddedBuffer;\n> >>>\n> >>> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >>> index b7dda282..43db7b68 100644\n> >>> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >>> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> >>> @@ -1145,7 +1145,7 @@ int PipelineHandlerIPU3::registerCameras()\n> >>> \t\t/* Convert the sensor rotation to a transformation */\n> >>> \t\tint32_t rotation = 0;\n> >>> \t\tif (data->properties_.contains(properties::Rotation))\n> >>> -\t\t\trotation = data->properties_.get(properties::Rotation);\n> >>> +\t\t\trotation = *(data->properties_.get(properties::Rotation));\n> >>> \t\telse\n> >>> \t\t\tLOG(IPU3, Warning) << \"Rotation control not exposed by \"\n> >>> \t\t\t\t\t << cio2->sensor()->id()\n> >>> @@ -1331,7 +1331,7 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> >>> \trequest->metadata().set(controls::draft::PipelineDepth, 3);\n> >>> \t/* \\todo Actually apply the scaler crop region to the ImgU. */\n> >>> \tif (request->controls().contains(controls::ScalerCrop))\n> >>> -\t\tcropRegion_ = request->controls().get(controls::ScalerCrop);\n> >>> +\t\tcropRegion_ = *(request->controls().get(controls::ScalerCrop));\n> >>> \trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n> >>>\n> >>> \tif (frameInfos_.tryComplete(info))\n> >>> @@ -1424,7 +1424,7 @@ void IPU3CameraData::statBufferReady(FrameBuffer *buffer)\n> >>> \t\treturn;\n> >>> \t}\n> >>>\n> >>> -\tipa_->processStatsBuffer(info->id, request->metadata().get(controls::SensorTimestamp),\n> >>> +\tipa_->processStatsBuffer(info->id, request->metadata().get(controls::SensorTimestamp).value_or(0),\n> >>> \t\t\t\t info->statBuffer->cookie(), info->effectiveSensorControls);\n> >>> }\n> >>>\n> >>> @@ -1458,8 +1458,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n> >>> \tif (!request->controls().contains(controls::draft::TestPatternMode))\n> >>> \t\treturn;\n> >>>\n> >>> -\tconst int32_t testPatternMode = request->controls().get(\n> >>> -\t\tcontrols::draft::TestPatternMode);\n> >>> +\tconst int32_t testPatternMode = *(request->controls().get(controls::draft::TestPatternMode));\n> >>>\n> >>> \tint ret = cio2_.sensor()->setTestPatternMode(\n> >>> \t\tstatic_cast(testPatternMode));\n> >>> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> index adc397e8..a62afdd4 100644\n> >>> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> >>> @@ -365,7 +365,7 @@ CameraConfiguration::Status RPiCameraConfiguration::validate()\n> >>> \t * error means the platform can never run. Let's just print a warning\n> >>> \t * and continue regardless; the rotation is effectively set to zero.\n> >>> \t */\n> >>> -\tint32_t rotation = data_->sensor_->properties().get(properties::Rotation);\n> >>> +\tint32_t rotation = data_->sensor_->properties().get(properties::Rotation).value_or(0);\n> >>> \tbool success;\n> >>> \tTransform rotationTransform = transformFromRotation(rotation, &success);\n> >>> \tif (!success)\n> >>> @@ -1706,7 +1706,8 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &\n> >>> \t * V4L2_CID_NOTIFY_GAINS control (which means notifyGainsUnity_ is set).\n> >>> \t */\n> >>> \tif (notifyGainsUnity_ && controls.contains(libcamera::controls::ColourGains)) {\n> >>> -\t\tlibcamera::Span colourGains = controls.get(libcamera::controls::ColourGains);\n> >>> +\t\tlibcamera::Span colourGains =\n> >>> +\t\t\t*controls.get(libcamera::controls::ColourGains);\n> >>> \t\t/* The control wants linear gains in the order B, Gb, Gr, R. */\n> >>> \t\tControlList ctrls(sensor_->controls());\n> >>> \t\tstd::array gains{\n> >>> @@ -2041,7 +2042,7 @@ Rectangle RPiCameraData::scaleIspCrop(const Rectangle &ispCrop) const\n> >>> void RPiCameraData::applyScalerCrop(const ControlList &controls)\n> >>> {\n> >>> \tif (controls.contains(controls::ScalerCrop)) {\n> >>> -\t\tRectangle nativeCrop = controls.get(controls::ScalerCrop);\n> >>> +\t\tRectangle nativeCrop = *controls.get(controls::ScalerCrop);\n> >>>\n> >>> \t\tif (!nativeCrop.width || !nativeCrop.height)\n> >>> \t\t\tnativeCrop = { 0, 0, 1, 1 };\n> >>> @@ -2079,7 +2080,7 @@ void RPiCameraData::fillRequestMetadata(const ControlList &bufferControls,\n> >>> \t\t\t\t\tRequest *request)\n> >>> {\n> >>> \trequest->metadata().set(controls::SensorTimestamp,\n> >>> -\t\t\t\tbufferControls.get(controls::SensorTimestamp));\n> >>> +\t\t\t\tbufferControls.get(controls::SensorTimestamp).value_or(0));\n> >>>\n> >>> \trequest->metadata().set(controls::ScalerCrop, scalerCrop_);\n> >>> }\n> >>> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> >>> index 34c8df5a..4b5d8276 100644\n> >>> --- a/src/qcam/dng_writer.cpp\n> >>> +++ b/src/qcam/dng_writer.cpp\n> >>> @@ -392,7 +392,7 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> >>> \tTIFFSetField(tif, TIFFTAG_MAKE, \"libcamera\");\n> >>>\n> >>> \tif (cameraProperties.contains(properties::Model)) {\n> >>> -\t\tstd::string model = cameraProperties.get(properties::Model);\n> >>> +\t\tstd::string model = *cameraProperties.get(properties::Model);\n> >>> \t\tTIFFSetField(tif, TIFFTAG_MODEL, model.c_str());\n> >>> \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n> >>> \t}\n> >>> @@ -438,16 +438,15 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> >>> \tconst double eps = 1e-2;\n> >>>\n> >>> \tif (metadata.contains(controls::ColourGains)) {\n> >>> -\t\tSpan const &colourGains = metadata.get(controls::ColourGains);\n> >>> -\t\tif (colourGains[0] > eps && colourGains[1] > eps) {\n> >>> -\t\t\twbGain = Matrix3d::diag(colourGains[0], 1, colourGains[1]);\n> >>> -\t\t\tneutral[0] = 1.0 / colourGains[0]; /* red */\n> >>> -\t\t\tneutral[2] = 1.0 / colourGains[1]; /* blue */\n> >>> +\t\tconst auto &colourGains = metadata.get(controls::ColourGains);\n> >>> +\t\tif ((*colourGains)[0] > eps && (*colourGains)[1] > eps) {\n> >>> +\t\t\twbGain = Matrix3d::diag((*colourGains)[0], 1, (*colourGains)[1]);\n> >>> +\t\t\tneutral[0] = 1.0 / (*colourGains)[0]; /* red */\n> >>> +\t\t\tneutral[2] = 1.0 / (*colourGains)[1]; /* blue */\n> >>> \t\t}\n> >>> \t}\n> >>> \tif (metadata.contains(controls::ColourCorrectionMatrix)) {\n> >>> -\t\tSpan const &coeffs = metadata.get(controls::ColourCorrectionMatrix);\n> >>> -\t\tMatrix3d ccmSupplied(coeffs);\n> >>> +\t\tMatrix3d ccmSupplied(*metadata.get(controls::ColourCorrectionMatrix));\n> >>> \t\tif (ccmSupplied.determinant() > eps)\n> >>> \t\t\tccm = ccmSupplied;\n> >>> \t}\n> >>> @@ -515,7 +514,8 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> >>> \tuint32_t whiteLevel = (1 << info->bitsPerSample) - 1;\n> >>>\n> >>> \tif (metadata.contains(controls::SensorBlackLevels)) {\n> >>> -\t\tSpan levels = metadata.get(controls::SensorBlackLevels);\n> >>> +\t\tSpan levels =\n> >>> +\t\t\t*metadata.get(controls::SensorBlackLevels);\n> >>>\n> >>> \t\t/*\n> >>> \t\t * The black levels control is specified in R, Gr, Gb, B order.\n> >>> @@ -593,13 +593,13 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> >>> \tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n> >>>\n> >>> \tif (metadata.contains(controls::AnalogueGain)) {\n> >>> -\t\tfloat gain = metadata.get(controls::AnalogueGain);\n> >>> +\t\tfloat gain = *metadata.get(controls::AnalogueGain);\n> >>> \t\tuint16_t iso = std::min(std::max(gain * 100, 0.0f), 65535.0f);\n> >>> \t\tTIFFSetField(tif, EXIFTAG_ISOSPEEDRATINGS, 1, &iso);\n> >>> \t}\n> >>>\n> >>> \tif (metadata.contains(controls::ExposureTime)) {\n> >>> -\t\tfloat exposureTime = metadata.get(controls::ExposureTime) / 1e6;\n> >>> +\t\tfloat exposureTime = *metadata.get(controls::ExposureTime) / 1e6;\n> >>> \t\tTIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);\n> >>> \t}\n> >>>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 4D06CBD808\n\tfor ;\n\tMon, 4 Jul 2022 17:51:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8D00B6564E;\n\tMon, 4 Jul 2022 19:51:23 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DA1B361FB1\n\tfor ;\n\tMon, 4 Jul 2022 19:51:21 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 18074802;\n\tMon, 4 Jul 2022 19:51:21 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1656957083;\n\tbh=Sr/KRscfEywyyAIXAfzlFSuAi+MSXX+UgglctJgifjY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=sy0ZiQvGsLmJ8a7x7FkMlUUeuTacwwCijJXYBzOVFfTFnRHZpXAeDb2N619GGth84\n\tsCoa5+tObS59MfszrtnY0Z+t9b29ogek2Y50F3Iwh3VMJLckMXBb/Bks49Gu3p4sEa\n\t4s1bp1ogzj2j2WJfWzlnL46VPllmwJJO/wf/WfOOdIpnYyex1y/Dei8TTfc2Chrpzs\n\tAYcrvpXVZPxOVwDFKG06eH0Z3IvKSl4tgehawpaitkgW0h/C5i6/v1vSwFS2vLAexu\n\tVmR2+IVF+pdLoRtQsL0BV4ybRr63LEqSmdTfUBLW7gtxSS0jrfTSrrJXsH3DDeO6s2\n\tOSLEQymMip4lw==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1656957081;\n\tbh=Sr/KRscfEywyyAIXAfzlFSuAi+MSXX+UgglctJgifjY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=AIlXAusSTH3/A1BYtmR/XGJdHs2y7F7/RXVH0/aelZaKfcfXr3Z8EzzU+2lo3ysgT\n\tqBb43dMXVbTbZT5uWkhUhtFecfMEb3jLPTXghN9LCiREk0JU98K5NeHnfNOFo92qZV\n\tV0iZJ6pY+yXuLtGjTxXuBoio2foKoXDduvJHbxc4="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"AIlXAusS\"; dkim-atps=neutral","Date":"Mon, 4 Jul 2022 20:50:58 +0300","To":"Christian Rauch ","Message-ID":"","References":"<20220610120338.96883-1-Rauch.Christian@gmx.de>\n\t<20220610120338.96883-2-Rauch.Christian@gmx.de>\n\t\n\t\n\t<0ea4afd9-f3fc-41a3-eee0-cae96188152c@gmx.de>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<0ea4afd9-f3fc-41a3-eee0-cae96188152c@gmx.de>","Subject":"Re: [libcamera-devel] [PATCH v8 1/4] libcamera: controls: Use\n\tstd::optional to handle invalid control values","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Laurent Pinchart via libcamera-devel\n\t","Reply-To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]