[{"id":23866,"web_url":"https://patchwork.libcamera.org/comment/23866/","msgid":"<3aaaf7e6-4d2c-38d5-333a-185c18f685e6@ideasonboard.com>","date":"2022-07-14T07:38:41","subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Avoid double\n\tlookups","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nThank you for the patch.\n\nOn 7/11/22 06:09, Laurent Pinchart via libcamera-devel wrote:\n> Now that the ControlList::get() function returns an instance of\n> std::optional<>, we can replace the ControlList::contains() calls with a\n> nullopt check on the return value of get(). This avoids double lookups\n> of controls through the code base.\n\n\nThe patch looks good to me, but I had a follow-up question whether the \nchange of ControlList::get() returning a std::optional<> means we can \ndrop .contains() helper probably? What do you think?\n\n>\n> Signed-off-by: Laurent Pinchart \n\n\nReviewed-by: Umang Jain \n\n> ---\n> This patch applies on top of \"[PATCH v10 1/2] libcamera: controls: Use\n> std::optional to handle invalid control values\".\n> ---\n> src/android/camera_capabilities.cpp | 11 ++---\n> src/android/camera_device.cpp | 46 +++++++++----------\n> src/android/camera_hal_manager.cpp | 6 +--\n> src/cam/main.cpp | 5 +-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 25 +++++-----\n> .../pipeline/raspberrypi/raspberrypi.cpp | 14 +++---\n> src/qcam/dng_writer.cpp | 35 +++++++-------\n> 7 files changed, 68 insertions(+), 74 deletions(-)\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 5304b2da1f93..94ebfc860040 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> @@ -1079,11 +1079,10 @@ int CameraCapabilities::initializeStaticMetadata()\n> \t\t\t\t sensitivityRange);\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> +\tconst auto &filterArr = properties.get(properties::draft::ColorFilterArrangement);\n> +\tif (filterArr)\n> \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,\n> -\t\t\t\t\t filterArr);\n> -\t}\n> +\t\t\t\t\t *filterArr);\n> \n> \tconst auto &exposureInfo = controlsInfo.find(&controls::ExposureTime);\n> \tif (exposureInfo != controlsInfo.end()) {\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 4662123210c2..b20e389b9d30 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -305,9 +305,9 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> \t */\n> \tconst ControlList &properties = camera_->properties();\n> \n> -\tif (properties.contains(properties::Location)) {\n> -\t\tint32_t location = *properties.get(properties::Location);\n> -\t\tswitch (location) {\n> +\tconst auto &location = properties.get(properties::Location);\n> +\tif (location) {\n> +\t\tswitch (*location) {\n> \t\tcase properties::CameraLocationFront:\n> \t\t\tfacing_ = CAMERA_FACING_FRONT;\n> \t\t\tbreak;\n> @@ -355,9 +355,9 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> \t * value for clockwise direction as required by the Android orientation\n> \t * metadata.\n> \t */\n> -\tif (properties.contains(properties::Rotation)) {\n> -\t\tint rotation = *properties.get(properties::Rotation);\n> -\t\torientation_ = (360 - rotation) % 360;\n> +\tconst auto &rotation = properties.get(properties::Rotation);\n> +\tif (rotation) {\n> +\t\torientation_ = (360 - *rotation) % 360;\n> \t\tif (cameraConfigData && cameraConfigData->rotation != -1 &&\n> \t\t orientation_ != cameraConfigData->rotation) {\n> \t\t\tLOG(HAL, Warning)\n> @@ -1564,25 +1564,24 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\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 = *metadata.get(controls::draft::PipelineDepth);\n> +\tconst auto &pipelineDepth = metadata.get(controls::draft::PipelineDepth);\n> +\tif (pipelineDepth)\n> \t\tresultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH,\n> -\t\t\t\t\t pipeline_depth);\n> -\t}\n> +\t\t\t\t\t *pipelineDepth);\n> \n> -\tif (metadata.contains(controls::ExposureTime)) {\n> -\t\tint64_t exposure = *metadata.get(controls::ExposureTime) * 1000ULL;\n> -\t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);\n> -\t}\n> +\tconst auto &exposureTime = metadata.get(controls::ExposureTime);\n> +\tif (exposureTime)\n> +\t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME,\n> +\t\t\t\t\t *exposureTime * 1000ULL);\n> \n> -\tif (metadata.contains(controls::FrameDuration)) {\n> -\t\tint64_t duration = *metadata.get(controls::FrameDuration) * 1000;\n> +\tconst auto &frameDuration = metadata.get(controls::FrameDuration);\n> +\tif (frameDuration)\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> -\t\t\t\t\t duration);\n> -\t}\n> +\t\t\t\t\t *frameDuration * 1000);\n> \n> -\tif (metadata.contains(controls::ScalerCrop)) {\n> -\t\tRectangle crop = *metadata.get(controls::ScalerCrop);\n> +\tconst auto &scalerCrop = metadata.get(controls::ScalerCrop);\n> +\tif (scalerCrop) {\n> +\t\tconst Rectangle &crop = *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> @@ -1590,11 +1589,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t\tresultMetadata->addEntry(ANDROID_SCALER_CROP_REGION, cropRect);\n> \t}\n> \n> -\tif (metadata.contains(controls::draft::TestPatternMode)) {\n> -\t\tconst int32_t testPatternMode = *metadata.get(controls::draft::TestPatternMode);\n> +\tconst auto &testPatternMode = metadata.get(controls::draft::TestPatternMode);\n> +\tif (testPatternMode)\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,\n> -\t\t\t\t\t testPatternMode);\n> -\t}\n> +\t\t\t\t\t *testPatternMode);\n> \n> \t/*\n> \t * Return the result metadata pack even is not valid: get() will return\n> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp\n> index 0bffe96f6dbc..7512cc4e11a4 100644\n> --- a/src/android/camera_hal_manager.cpp\n> +++ b/src/android/camera_hal_manager.cpp\n> @@ -228,11 +228,7 @@ void CameraHalManager::cameraRemoved(std::shared_ptr cam)\n> \n> int32_t CameraHalManager::cameraLocation(const Camera *cam)\n> {\n> -\tconst ControlList &properties = cam->properties();\n> -\tif (!properties.contains(properties::Location))\n> -\t\treturn -1;\n> -\n> -\treturn *properties.get(properties::Location);\n> +\treturn cam->properties().get(properties::Location).value_or(-1);\n> }\n> \n> CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index d8115cd86825..13a3d9b93df8 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -300,8 +300,9 @@ std::string CamApp::cameraName(const Camera *camera)\n> \t * Construct the name from the camera location, model and ID. The model\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> +\tconst auto &location = props.get(properties::Location);\n> +\tif (location) {\n> +\t\tswitch (*location) {\n> \t\tcase properties::CameraLocationFront:\n> \t\t\taddModel = false;\n> \t\t\tname = \"Internal front camera \";\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 43db7b68dac8..d60f20b08e27 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1143,18 +1143,17 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\t\t\t\t\t &IPU3CameraData::frameStart);\n> \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\telse\n> +\t\tconst auto &rotation = data->properties_.get(properties::Rotation);\n> +\t\tif (!rotation)\n> \t\t\tLOG(IPU3, Warning) << \"Rotation control not exposed by \"\n> \t\t\t\t\t << cio2->sensor()->id()\n> \t\t\t\t\t << \". Assume rotation 0\";\n> \n> +\t\tint32_t rotationValue = rotation.value_or(0);\n> \t\tbool success;\n> -\t\tdata->rotationTransform_ = transformFromRotation(rotation, &success);\n> +\t\tdata->rotationTransform_ = transformFromRotation(rotationValue, &success);\n> \t\tif (!success)\n> -\t\t\tLOG(IPU3, Warning) << \"Invalid rotation of \" << rotation\n> +\t\t\tLOG(IPU3, Warning) << \"Invalid rotation of \" << rotationValue\n> \t\t\t\t\t << \" degrees: ignoring\";\n> \n> \t\tControlList ctrls = cio2->sensor()->getControls({ V4L2_CID_HFLIP });\n> @@ -1330,8 +1329,9 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> \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> +\tconst auto &scalerCrop = request->controls().get(controls::ScalerCrop);\n> +\tif (scalerCrop)\n> +\t\tcropRegion_ = *scalerCrop;\n> \trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n> \n> \tif (frameInfos_.tryComplete(info))\n> @@ -1455,13 +1455,12 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n> \tRequest *request = processingRequests_.front();\n> \tprocessingRequests_.pop();\n> \n> -\tif (!request->controls().contains(controls::draft::TestPatternMode))\n> +\tconst auto &testPatternMode = request->controls().get(controls::draft::TestPatternMode);\n> +\tif (!testPatternMode)\n> \t\treturn;\n> \n> -\tconst int32_t testPatternMode = *(request->controls().get(controls::draft::TestPatternMode));\n> -\n> \tint ret = cio2_.sensor()->setTestPatternMode(\n> -\t\tstatic_cast(testPatternMode));\n> +\t\tstatic_cast(*testPatternMode));\n> \tif (ret) {\n> \t\tLOG(IPU3, Error) << \"Failed to set test pattern mode: \"\n> \t\t\t\t << ret;\n> @@ -1469,7 +1468,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n> \t}\n> \n> \trequest->metadata().set(controls::draft::TestPatternMode,\n> -\t\t\t\ttestPatternMode);\n> +\t\t\t\t*testPatternMode);\n> }\n> \n> REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 28f054cb84c8..2e19531eecc4 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1716,16 +1716,15 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &\n> \t * Inform the sensor of the latest colour gains if it has the\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 =\n> -\t\t\t*controls.get(libcamera::controls::ColourGains);\n> +\tconst auto &colourGains = controls.get(libcamera::controls::ColourGains);\n> +\tif (notifyGainsUnity_ && 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> -\t\t\tstatic_cast(colourGains[1] * *notifyGainsUnity_),\n> +\t\t\tstatic_cast((*colourGains)[1] * *notifyGainsUnity_),\n> \t\t\t*notifyGainsUnity_,\n> \t\t\t*notifyGainsUnity_,\n> -\t\t\tstatic_cast(colourGains[0] * *notifyGainsUnity_)\n> +\t\t\tstatic_cast((*colourGains)[0] * *notifyGainsUnity_)\n> \t\t};\n> \t\tctrls.set(V4L2_CID_NOTIFY_GAINS, Span{ gains });\n> \n> @@ -2052,8 +2051,9 @@ Rectangle RPiCameraData::scaleIspCrop(const Rectangle &ispCrop) const\n> \n> void RPiCameraData::applyScalerCrop(const ControlList &controls)\n> {\n> -\tif (controls.contains(controls::ScalerCrop)) {\n> -\t\tRectangle nativeCrop = *controls.get(controls::ScalerCrop);\n> +\tconst auto &scalerCrop = controls.get(controls::ScalerCrop);\n> +\tif (scalerCrop) {\n> +\t\tRectangle nativeCrop = *scalerCrop;\n> \n> \t\tif (!nativeCrop.width || !nativeCrop.height)\n> \t\t\tnativeCrop = { 0, 0, 1, 1 };\n> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> index 4b5d8276564f..5896485cda5b 100644\n> --- a/src/qcam/dng_writer.cpp\n> +++ b/src/qcam/dng_writer.cpp\n> @@ -391,9 +391,9 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \tTIFFSetField(tif, TIFFTAG_FILLORDER, FILLORDER_MSB2LSB);\n> \tTIFFSetField(tif, TIFFTAG_MAKE, \"libcamera\");\n> \n> -\tif (cameraProperties.contains(properties::Model)) {\n> -\t\tstd::string model = *cameraProperties.get(properties::Model);\n> -\t\tTIFFSetField(tif, TIFFTAG_MODEL, model.c_str());\n> +\tconst auto &model = cameraProperties.get(properties::Model);\n> +\tif (model) {\n> +\t\tTIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n> \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n> \t}\n> \n> @@ -437,16 +437,18 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \t */\n> \tconst double eps = 1e-2;\n> \n> -\tif (metadata.contains(controls::ColourGains)) {\n> -\t\tconst auto &colourGains = metadata.get(controls::ColourGains);\n> +\tconst auto &colourGains = metadata.get(controls::ColourGains);\n> +\tif (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\tMatrix3d ccmSupplied(*metadata.get(controls::ColourCorrectionMatrix));\n> +\n> +\tconst auto &ccmControl = metadata.get(controls::ColourCorrectionMatrix);\n> +\tif (ccmControl) {\n> +\t\tMatrix3d ccmSupplied(*ccmControl);\n> \t\tif (ccmSupplied.determinant() > eps)\n> \t\t\tccm = ccmSupplied;\n> \t}\n> @@ -513,9 +515,9 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \tfloat blackLevel[] = { 0.0f, 0.0f, 0.0f, 0.0f };\n> \tuint32_t whiteLevel = (1 << info->bitsPerSample) - 1;\n> \n> -\tif (metadata.contains(controls::SensorBlackLevels)) {\n> -\t\tSpan levels =\n> -\t\t\t*metadata.get(controls::SensorBlackLevels);\n> +\tconst auto &blackLevels = metadata.get(controls::SensorBlackLevels);\n> +\tif (blackLevels) {\n> +\t\tSpan levels = *blackLevels;\n> \n> \t\t/*\n> \t\t * The black levels control is specified in R, Gr, Gb, B order.\n> @@ -592,16 +594,15 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \tTIFFSetField(tif, EXIFTAG_DATETIMEORIGINAL, strTime);\n> \tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n> \n> -\tif (metadata.contains(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> +\tconst auto &analogGain = metadata.get(controls::AnalogueGain);\n> +\tif (analogGain) {\n> +\t\tuint16_t iso = std::min(std::max(*analogGain * 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\tTIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);\n> -\t}\n> +\tconst auto &exposureTime = metadata.get(controls::ExposureTime);\n> +\tif (exposureTime)\n> +\t\tTIFFSetField(tif, EXIFTAG_EXPOSURETIME, *exposureTime / 1e6);\n> \n> \tTIFFWriteCustomDirectory(tif, &exifIFDOffset);\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 1AA42BD1F1\n\tfor ;\n\tThu, 14 Jul 2022 07:38:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8739363312;\n\tThu, 14 Jul 2022 09:38:50 +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 707D463309\n\tfor ;\n\tThu, 14 Jul 2022 09:38:48 +0200 (CEST)","from [IPV6:2401:4900:1f3f:dcbd:bc88:bb0e:d3d8:8610] (unknown\n\t[IPv6:2401:4900:1f3f:dcbd:bc88:bb0e:d3d8:8610])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DB81D383;\n\tThu, 14 Jul 2022 09:38:46 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657784330;\n\tbh=6Nkqbgkjb+UHoEEPMgFwKhq0A5SvRcgV8ab8t1+oZFI=;\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=N+BBVHOldTRxFdZV9NnxS6bW4ddv4iMF7MMWZPEiFZPhEpBjn/92KcNXK9oLqsNpN\n\tDnoUUphmT3sgcM9arT63ZDl4ouK1qXPrmAEVtxwXKYrTQ/aec+3Zy4/FzLn4GVV3PK\n\t9MjPMi25qGS89AyEoF/LDR7DvAJDHFWUhGgs+FzNRatqY3T8SlQcTjG9q4sBIhgGSQ\n\ty+l6PRMQ/lcm2UIpr5GVTjMnBxAO23LTALwZ0GWgRyRPLsuK5qHX0DiIdf84QBV6dN\n\txnb1wvnbU1cwQq2DUBlkWJjoKGaiC+Fjca8+IEq9N9gH7Evt17j0DcZ70dzgEDnSGj\n\tBfpkdhBkESg8Q==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1657784327;\n\tbh=6Nkqbgkjb+UHoEEPMgFwKhq0A5SvRcgV8ab8t1+oZFI=;\n\th=Date:Subject:To:References:From:In-Reply-To:From;\n\tb=hqx+DZY0BQHvhd+ed1K6nEzrwjL2ojsqdmFNsbR6/93dI3mcJAOF8uJozLwNO9vVC\n\tYWrDly3IzGeKt/04bd+U0V3d9orP6um7klFw8vUlM6B523Z8/QCZQ/IiBPFQBwtfU2\n\tvqVTAIQYKg9BFJNkWGRMMztDq+Iz/KPhycpw7epE="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"hqx+DZY0\"; dkim-atps=neutral","Message-ID":"<3aaaf7e6-4d2c-38d5-333a-185c18f685e6@ideasonboard.com>","Date":"Thu, 14 Jul 2022 13:08:41 +0530","MIME-Version":"1.0","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101\n\tThunderbird/91.4.1","Content-Language":"en-US","To":"Laurent Pinchart ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20220711003918.535-1-laurent.pinchart@ideasonboard.com>","In-Reply-To":"<20220711003918.535-1-laurent.pinchart@ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","Subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Avoid double\n\tlookups","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":"Umang Jain via libcamera-devel ","Reply-To":"Umang Jain ","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23867,"web_url":"https://patchwork.libcamera.org/comment/23867/","msgid":"<20220714080255.zsfudfou3lplgsub@uno.localdomain>","date":"2022-07-14T08:02:55","subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Avoid double\n\tlookups","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"HI Laurent,\n\nOn Mon, Jul 11, 2022 at 03:39:18AM +0300, Laurent Pinchart via libcamera-devel wrote:\n> Now that the ControlList::get() function returns an instance of\n> std::optional<>, we can replace the ControlList::contains() calls with a\n> nullopt check on the return value of get(). This avoids double lookups\n> of controls through the code base.\n>\n> Signed-off-by: Laurent Pinchart \n\nLooks good thank you!\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> This patch applies on top of \"[PATCH v10 1/2] libcamera: controls: Use\n> std::optional to handle invalid control values\".\n> ---\n> src/android/camera_capabilities.cpp | 11 ++---\n> src/android/camera_device.cpp | 46 +++++++++----------\n> src/android/camera_hal_manager.cpp | 6 +--\n> src/cam/main.cpp | 5 +-\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 25 +++++-----\n> .../pipeline/raspberrypi/raspberrypi.cpp | 14 +++---\n> src/qcam/dng_writer.cpp | 35 +++++++-------\n> 7 files changed, 68 insertions(+), 74 deletions(-)\n>\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 5304b2da1f93..94ebfc860040 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> @@ -1079,11 +1079,10 @@ int CameraCapabilities::initializeStaticMetadata()\n> \t\t\t\t sensitivityRange);\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> +\tconst auto &filterArr = properties.get(properties::draft::ColorFilterArrangement);\n> +\tif (filterArr)\n> \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,\n> -\t\t\t\t\t filterArr);\n> -\t}\n> +\t\t\t\t\t *filterArr);\n>\n> \tconst auto &exposureInfo = controlsInfo.find(&controls::ExposureTime);\n> \tif (exposureInfo != controlsInfo.end()) {\n> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> index 4662123210c2..b20e389b9d30 100644\n> --- a/src/android/camera_device.cpp\n> +++ b/src/android/camera_device.cpp\n> @@ -305,9 +305,9 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> \t */\n> \tconst ControlList &properties = camera_->properties();\n>\n> -\tif (properties.contains(properties::Location)) {\n> -\t\tint32_t location = *properties.get(properties::Location);\n> -\t\tswitch (location) {\n> +\tconst auto &location = properties.get(properties::Location);\n> +\tif (location) {\n> +\t\tswitch (*location) {\n> \t\tcase properties::CameraLocationFront:\n> \t\t\tfacing_ = CAMERA_FACING_FRONT;\n> \t\t\tbreak;\n> @@ -355,9 +355,9 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> \t * value for clockwise direction as required by the Android orientation\n> \t * metadata.\n> \t */\n> -\tif (properties.contains(properties::Rotation)) {\n> -\t\tint rotation = *properties.get(properties::Rotation);\n> -\t\torientation_ = (360 - rotation) % 360;\n> +\tconst auto &rotation = properties.get(properties::Rotation);\n> +\tif (rotation) {\n> +\t\torientation_ = (360 - *rotation) % 360;\n> \t\tif (cameraConfigData && cameraConfigData->rotation != -1 &&\n> \t\t orientation_ != cameraConfigData->rotation) {\n> \t\t\tLOG(HAL, Warning)\n> @@ -1564,25 +1564,24 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\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 = *metadata.get(controls::draft::PipelineDepth);\n> +\tconst auto &pipelineDepth = metadata.get(controls::draft::PipelineDepth);\n> +\tif (pipelineDepth)\n> \t\tresultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH,\n> -\t\t\t\t\t pipeline_depth);\n> -\t}\n> +\t\t\t\t\t *pipelineDepth);\n>\n> -\tif (metadata.contains(controls::ExposureTime)) {\n> -\t\tint64_t exposure = *metadata.get(controls::ExposureTime) * 1000ULL;\n> -\t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);\n> -\t}\n> +\tconst auto &exposureTime = metadata.get(controls::ExposureTime);\n> +\tif (exposureTime)\n> +\t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME,\n> +\t\t\t\t\t *exposureTime * 1000ULL);\n>\n> -\tif (metadata.contains(controls::FrameDuration)) {\n> -\t\tint64_t duration = *metadata.get(controls::FrameDuration) * 1000;\n> +\tconst auto &frameDuration = metadata.get(controls::FrameDuration);\n> +\tif (frameDuration)\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> -\t\t\t\t\t duration);\n> -\t}\n> +\t\t\t\t\t *frameDuration * 1000);\n>\n> -\tif (metadata.contains(controls::ScalerCrop)) {\n> -\t\tRectangle crop = *metadata.get(controls::ScalerCrop);\n> +\tconst auto &scalerCrop = metadata.get(controls::ScalerCrop);\n> +\tif (scalerCrop) {\n> +\t\tconst Rectangle &crop = *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> @@ -1590,11 +1589,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> \t\tresultMetadata->addEntry(ANDROID_SCALER_CROP_REGION, cropRect);\n> \t}\n>\n> -\tif (metadata.contains(controls::draft::TestPatternMode)) {\n> -\t\tconst int32_t testPatternMode = *metadata.get(controls::draft::TestPatternMode);\n> +\tconst auto &testPatternMode = metadata.get(controls::draft::TestPatternMode);\n> +\tif (testPatternMode)\n> \t\tresultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,\n> -\t\t\t\t\t testPatternMode);\n> -\t}\n> +\t\t\t\t\t *testPatternMode);\n>\n> \t/*\n> \t * Return the result metadata pack even is not valid: get() will return\n> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp\n> index 0bffe96f6dbc..7512cc4e11a4 100644\n> --- a/src/android/camera_hal_manager.cpp\n> +++ b/src/android/camera_hal_manager.cpp\n> @@ -228,11 +228,7 @@ void CameraHalManager::cameraRemoved(std::shared_ptr cam)\n>\n> int32_t CameraHalManager::cameraLocation(const Camera *cam)\n> {\n> -\tconst ControlList &properties = cam->properties();\n> -\tif (!properties.contains(properties::Location))\n> -\t\treturn -1;\n> -\n> -\treturn *properties.get(properties::Location);\n> +\treturn cam->properties().get(properties::Location).value_or(-1);\n> }\n>\n> CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)\n> diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> index d8115cd86825..13a3d9b93df8 100644\n> --- a/src/cam/main.cpp\n> +++ b/src/cam/main.cpp\n> @@ -300,8 +300,9 @@ std::string CamApp::cameraName(const Camera *camera)\n> \t * Construct the name from the camera location, model and ID. The model\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> +\tconst auto &location = props.get(properties::Location);\n> +\tif (location) {\n> +\t\tswitch (*location) {\n> \t\tcase properties::CameraLocationFront:\n> \t\t\taddModel = false;\n> \t\t\tname = \"Internal front camera \";\n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 43db7b68dac8..d60f20b08e27 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1143,18 +1143,17 @@ int PipelineHandlerIPU3::registerCameras()\n> \t\t\t\t\t\t &IPU3CameraData::frameStart);\n>\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\telse\n> +\t\tconst auto &rotation = data->properties_.get(properties::Rotation);\n> +\t\tif (!rotation)\n> \t\t\tLOG(IPU3, Warning) << \"Rotation control not exposed by \"\n> \t\t\t\t\t << cio2->sensor()->id()\n> \t\t\t\t\t << \". Assume rotation 0\";\n>\n> +\t\tint32_t rotationValue = rotation.value_or(0);\n> \t\tbool success;\n> -\t\tdata->rotationTransform_ = transformFromRotation(rotation, &success);\n> +\t\tdata->rotationTransform_ = transformFromRotation(rotationValue, &success);\n> \t\tif (!success)\n> -\t\t\tLOG(IPU3, Warning) << \"Invalid rotation of \" << rotation\n> +\t\t\tLOG(IPU3, Warning) << \"Invalid rotation of \" << rotationValue\n> \t\t\t\t\t << \" degrees: ignoring\";\n>\n> \t\tControlList ctrls = cio2->sensor()->getControls({ V4L2_CID_HFLIP });\n> @@ -1330,8 +1329,9 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n>\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> +\tconst auto &scalerCrop = request->controls().get(controls::ScalerCrop);\n> +\tif (scalerCrop)\n> +\t\tcropRegion_ = *scalerCrop;\n> \trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n>\n> \tif (frameInfos_.tryComplete(info))\n> @@ -1455,13 +1455,12 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n> \tRequest *request = processingRequests_.front();\n> \tprocessingRequests_.pop();\n>\n> -\tif (!request->controls().contains(controls::draft::TestPatternMode))\n> +\tconst auto &testPatternMode = request->controls().get(controls::draft::TestPatternMode);\n> +\tif (!testPatternMode)\n> \t\treturn;\n>\n> -\tconst int32_t testPatternMode = *(request->controls().get(controls::draft::TestPatternMode));\n> -\n> \tint ret = cio2_.sensor()->setTestPatternMode(\n> -\t\tstatic_cast(testPatternMode));\n> +\t\tstatic_cast(*testPatternMode));\n> \tif (ret) {\n> \t\tLOG(IPU3, Error) << \"Failed to set test pattern mode: \"\n> \t\t\t\t << ret;\n> @@ -1469,7 +1468,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n> \t}\n>\n> \trequest->metadata().set(controls::draft::TestPatternMode,\n> -\t\t\t\ttestPatternMode);\n> +\t\t\t\t*testPatternMode);\n> }\n>\n> REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)\n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 28f054cb84c8..2e19531eecc4 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -1716,16 +1716,15 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &\n> \t * Inform the sensor of the latest colour gains if it has the\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 =\n> -\t\t\t*controls.get(libcamera::controls::ColourGains);\n> +\tconst auto &colourGains = controls.get(libcamera::controls::ColourGains);\n> +\tif (notifyGainsUnity_ && 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> -\t\t\tstatic_cast(colourGains[1] * *notifyGainsUnity_),\n> +\t\t\tstatic_cast((*colourGains)[1] * *notifyGainsUnity_),\n> \t\t\t*notifyGainsUnity_,\n> \t\t\t*notifyGainsUnity_,\n> -\t\t\tstatic_cast(colourGains[0] * *notifyGainsUnity_)\n> +\t\t\tstatic_cast((*colourGains)[0] * *notifyGainsUnity_)\n> \t\t};\n> \t\tctrls.set(V4L2_CID_NOTIFY_GAINS, Span{ gains });\n>\n> @@ -2052,8 +2051,9 @@ Rectangle RPiCameraData::scaleIspCrop(const Rectangle &ispCrop) const\n>\n> void RPiCameraData::applyScalerCrop(const ControlList &controls)\n> {\n> -\tif (controls.contains(controls::ScalerCrop)) {\n> -\t\tRectangle nativeCrop = *controls.get(controls::ScalerCrop);\n> +\tconst auto &scalerCrop = controls.get(controls::ScalerCrop);\n> +\tif (scalerCrop) {\n> +\t\tRectangle nativeCrop = *scalerCrop;\n>\n> \t\tif (!nativeCrop.width || !nativeCrop.height)\n> \t\t\tnativeCrop = { 0, 0, 1, 1 };\n> diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> index 4b5d8276564f..5896485cda5b 100644\n> --- a/src/qcam/dng_writer.cpp\n> +++ b/src/qcam/dng_writer.cpp\n> @@ -391,9 +391,9 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \tTIFFSetField(tif, TIFFTAG_FILLORDER, FILLORDER_MSB2LSB);\n> \tTIFFSetField(tif, TIFFTAG_MAKE, \"libcamera\");\n>\n> -\tif (cameraProperties.contains(properties::Model)) {\n> -\t\tstd::string model = *cameraProperties.get(properties::Model);\n> -\t\tTIFFSetField(tif, TIFFTAG_MODEL, model.c_str());\n> +\tconst auto &model = cameraProperties.get(properties::Model);\n> +\tif (model) {\n> +\t\tTIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n> \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n> \t}\n>\n> @@ -437,16 +437,18 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \t */\n> \tconst double eps = 1e-2;\n>\n> -\tif (metadata.contains(controls::ColourGains)) {\n> -\t\tconst auto &colourGains = metadata.get(controls::ColourGains);\n> +\tconst auto &colourGains = metadata.get(controls::ColourGains);\n> +\tif (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\tMatrix3d ccmSupplied(*metadata.get(controls::ColourCorrectionMatrix));\n> +\n> +\tconst auto &ccmControl = metadata.get(controls::ColourCorrectionMatrix);\n> +\tif (ccmControl) {\n> +\t\tMatrix3d ccmSupplied(*ccmControl);\n> \t\tif (ccmSupplied.determinant() > eps)\n> \t\t\tccm = ccmSupplied;\n> \t}\n> @@ -513,9 +515,9 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \tfloat blackLevel[] = { 0.0f, 0.0f, 0.0f, 0.0f };\n> \tuint32_t whiteLevel = (1 << info->bitsPerSample) - 1;\n>\n> -\tif (metadata.contains(controls::SensorBlackLevels)) {\n> -\t\tSpan levels =\n> -\t\t\t*metadata.get(controls::SensorBlackLevels);\n> +\tconst auto &blackLevels = metadata.get(controls::SensorBlackLevels);\n> +\tif (blackLevels) {\n> +\t\tSpan levels = *blackLevels;\n>\n> \t\t/*\n> \t\t * The black levels control is specified in R, Gr, Gb, B order.\n> @@ -592,16 +594,15 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> \tTIFFSetField(tif, EXIFTAG_DATETIMEORIGINAL, strTime);\n> \tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n>\n> -\tif (metadata.contains(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> +\tconst auto &analogGain = metadata.get(controls::AnalogueGain);\n> +\tif (analogGain) {\n> +\t\tuint16_t iso = std::min(std::max(*analogGain * 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\tTIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);\n> -\t}\n> +\tconst auto &exposureTime = metadata.get(controls::ExposureTime);\n> +\tif (exposureTime)\n> +\t\tTIFFSetField(tif, EXIFTAG_EXPOSURETIME, *exposureTime / 1e6);\n>\n> \tTIFFWriteCustomDirectory(tif, &exifIFDOffset);\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 42EF6BE173\n\tfor ;\n\tThu, 14 Jul 2022 08:02:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B72D763312;\n\tThu, 14 Jul 2022 10:02:58 +0200 (CEST)","from relay7-d.mail.gandi.net (relay7-d.mail.gandi.net\n\t[217.70.183.200])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8CEBF63309\n\tfor ;\n\tThu, 14 Jul 2022 10:02:57 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id D45DF2000A;\n\tThu, 14 Jul 2022 08:02:56 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657785778;\n\tbh=vmjA6CfCFqMboawqyS5ww1iI+KnEcgtAvPtItyBV1vE=;\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=ZM4fAR1r//8pnGUVvYuZAtatd8ZRV2G9ZmVVHROFFIRr+9DigvUWoF8bUIYVb8dmM\n\tKLRzH5X9rYE4l8SOeHXiAhungQEpDAa3n1kPVNQs+J+/dMn53efN62WZ04Nbba8Mu0\n\t7BBSo09b2CA0REQKNE/UHWvAK/YXTvvnJUzkHFfbkqVjQgIx3f2r5fDvPZNYLZOElo\n\t8eM0dzR6zDR0TY9GE1DLRXIfE4bBM1g3P7rQnYFDGVb0p6H/mUfHmGZHQgUifslAgX\n\td/HjpTw0RHc9mwq269w/HvmMfYja4dfrgHcTFCSwP58Ypy9AyVY5rRdgz0bFP1CJwS\n\tfEfbr+jYgFR4Q==","Date":"Thu, 14 Jul 2022 10:02:55 +0200","To":"Laurent Pinchart ","Message-ID":"<20220714080255.zsfudfou3lplgsub@uno.localdomain>","References":"<20220711003918.535-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20220711003918.535-1-laurent.pinchart@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Avoid double\n\tlookups","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":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":23868,"web_url":"https://patchwork.libcamera.org/comment/23868/","msgid":"","date":"2022-07-14T08:55:23","subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Avoid double\n\tlookups","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nOn Thu, Jul 14, 2022 at 01:08:41PM +0530, Umang Jain wrote:\n> On 7/11/22 06:09, Laurent Pinchart via libcamera-devel wrote:\n> > Now that the ControlList::get() function returns an instance of\n> > std::optional<>, we can replace the ControlList::contains() calls with a\n> > nullopt check on the return value of get(). This avoids double lookups\n> > of controls through the code base.\n> \n> The patch looks good to me, but I had a follow-up question whether the \n> change of ControlList::get() returning a std::optional<> means we can \n> drop .contains() helper probably? What do you think?\n\nEventually yes, I think. There's a \"little\" issue I'd like to solve\nfirst though, if possible. ControlList::get() returns a\nstd::optional, which means it copies the value from the internal\nControlValue storage to std::optional. In most cases that's fine, but\nwhen we'll have large controls (large arrays for instance) that's not\noptimal. There would be a copy just to check if a control exists, which\nisn't great.\n\nI haven't found a solution I like yet. We can't use a std::optional\nas the type needs to be assignable (you can't change a reference after\nit's set), and std::optional> makes the syntax\nmore cumbersome in callers.\n\nThis being said, there should be very few cases where we're interested\nin knowing if a control exists without needing his value right after, so\nmaybe we could already drop the functions. I actually gave it a quick\ntry, and it looks like contains(const ControlId &id) is used in two\nplaces only:\n\n- src/cam/main.cpp, which I didn't notice when writing this patch, so\n I'll send a v2\n- test/controls/control_list.cpp, which we could change fairly easily\n\nI'll give it a go on top of this patch.\n\n> > Signed-off-by: Laurent Pinchart \n> \n> Reviewed-by: Umang Jain \n> \n> > ---\n> > This patch applies on top of \"[PATCH v10 1/2] libcamera: controls: Use\n> > std::optional to handle invalid control values\".\n> > ---\n> > src/android/camera_capabilities.cpp | 11 ++---\n> > src/android/camera_device.cpp | 46 +++++++++----------\n> > src/android/camera_hal_manager.cpp | 6 +--\n> > src/cam/main.cpp | 5 +-\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 25 +++++-----\n> > .../pipeline/raspberrypi/raspberrypi.cpp | 14 +++---\n> > src/qcam/dng_writer.cpp | 35 +++++++-------\n> > 7 files changed, 68 insertions(+), 74 deletions(-)\n> >\n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index 5304b2da1f93..94ebfc860040 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> > @@ -1079,11 +1079,10 @@ int CameraCapabilities::initializeStaticMetadata()\n> > \t\t\t\t sensitivityRange);\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> > +\tconst auto &filterArr = properties.get(properties::draft::ColorFilterArrangement);\n> > +\tif (filterArr)\n> > \t\tstaticMetadata_->addEntry(ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT,\n> > -\t\t\t\t\t filterArr);\n> > -\t}\n> > +\t\t\t\t\t *filterArr);\n> > \n> > \tconst auto &exposureInfo = controlsInfo.find(&controls::ExposureTime);\n> > \tif (exposureInfo != controlsInfo.end()) {\n> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp\n> > index 4662123210c2..b20e389b9d30 100644\n> > --- a/src/android/camera_device.cpp\n> > +++ b/src/android/camera_device.cpp\n> > @@ -305,9 +305,9 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> > \t */\n> > \tconst ControlList &properties = camera_->properties();\n> > \n> > -\tif (properties.contains(properties::Location)) {\n> > -\t\tint32_t location = *properties.get(properties::Location);\n> > -\t\tswitch (location) {\n> > +\tconst auto &location = properties.get(properties::Location);\n> > +\tif (location) {\n> > +\t\tswitch (*location) {\n> > \t\tcase properties::CameraLocationFront:\n> > \t\t\tfacing_ = CAMERA_FACING_FRONT;\n> > \t\t\tbreak;\n> > @@ -355,9 +355,9 @@ int CameraDevice::initialize(const CameraConfigData *cameraConfigData)\n> > \t * value for clockwise direction as required by the Android orientation\n> > \t * metadata.\n> > \t */\n> > -\tif (properties.contains(properties::Rotation)) {\n> > -\t\tint rotation = *properties.get(properties::Rotation);\n> > -\t\torientation_ = (360 - rotation) % 360;\n> > +\tconst auto &rotation = properties.get(properties::Rotation);\n> > +\tif (rotation) {\n> > +\t\torientation_ = (360 - *rotation) % 360;\n> > \t\tif (cameraConfigData && cameraConfigData->rotation != -1 &&\n> > \t\t orientation_ != cameraConfigData->rotation) {\n> > \t\t\tLOG(HAL, Warning)\n> > @@ -1564,25 +1564,24 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\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 = *metadata.get(controls::draft::PipelineDepth);\n> > +\tconst auto &pipelineDepth = metadata.get(controls::draft::PipelineDepth);\n> > +\tif (pipelineDepth)\n> > \t\tresultMetadata->addEntry(ANDROID_REQUEST_PIPELINE_DEPTH,\n> > -\t\t\t\t\t pipeline_depth);\n> > -\t}\n> > +\t\t\t\t\t *pipelineDepth);\n> > \n> > -\tif (metadata.contains(controls::ExposureTime)) {\n> > -\t\tint64_t exposure = *metadata.get(controls::ExposureTime) * 1000ULL;\n> > -\t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME, exposure);\n> > -\t}\n> > +\tconst auto &exposureTime = metadata.get(controls::ExposureTime);\n> > +\tif (exposureTime)\n> > +\t\tresultMetadata->addEntry(ANDROID_SENSOR_EXPOSURE_TIME,\n> > +\t\t\t\t\t *exposureTime * 1000ULL);\n> > \n> > -\tif (metadata.contains(controls::FrameDuration)) {\n> > -\t\tint64_t duration = *metadata.get(controls::FrameDuration) * 1000;\n> > +\tconst auto &frameDuration = metadata.get(controls::FrameDuration);\n> > +\tif (frameDuration)\n> > \t\tresultMetadata->addEntry(ANDROID_SENSOR_FRAME_DURATION,\n> > -\t\t\t\t\t duration);\n> > -\t}\n> > +\t\t\t\t\t *frameDuration * 1000);\n> > \n> > -\tif (metadata.contains(controls::ScalerCrop)) {\n> > -\t\tRectangle crop = *metadata.get(controls::ScalerCrop);\n> > +\tconst auto &scalerCrop = metadata.get(controls::ScalerCrop);\n> > +\tif (scalerCrop) {\n> > +\t\tconst Rectangle &crop = *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> > @@ -1590,11 +1589,10 @@ CameraDevice::getResultMetadata(const Camera3RequestDescriptor &descriptor) cons\n> > \t\tresultMetadata->addEntry(ANDROID_SCALER_CROP_REGION, cropRect);\n> > \t}\n> > \n> > -\tif (metadata.contains(controls::draft::TestPatternMode)) {\n> > -\t\tconst int32_t testPatternMode = *metadata.get(controls::draft::TestPatternMode);\n> > +\tconst auto &testPatternMode = metadata.get(controls::draft::TestPatternMode);\n> > +\tif (testPatternMode)\n> > \t\tresultMetadata->addEntry(ANDROID_SENSOR_TEST_PATTERN_MODE,\n> > -\t\t\t\t\t testPatternMode);\n> > -\t}\n> > +\t\t\t\t\t *testPatternMode);\n> > \n> > \t/*\n> > \t * Return the result metadata pack even is not valid: get() will return\n> > diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp\n> > index 0bffe96f6dbc..7512cc4e11a4 100644\n> > --- a/src/android/camera_hal_manager.cpp\n> > +++ b/src/android/camera_hal_manager.cpp\n> > @@ -228,11 +228,7 @@ void CameraHalManager::cameraRemoved(std::shared_ptr cam)\n> > \n> > int32_t CameraHalManager::cameraLocation(const Camera *cam)\n> > {\n> > -\tconst ControlList &properties = cam->properties();\n> > -\tif (!properties.contains(properties::Location))\n> > -\t\treturn -1;\n> > -\n> > -\treturn *properties.get(properties::Location);\n> > +\treturn cam->properties().get(properties::Location).value_or(-1);\n> > }\n> > \n> > CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)\n> > diff --git a/src/cam/main.cpp b/src/cam/main.cpp\n> > index d8115cd86825..13a3d9b93df8 100644\n> > --- a/src/cam/main.cpp\n> > +++ b/src/cam/main.cpp\n> > @@ -300,8 +300,9 @@ std::string CamApp::cameraName(const Camera *camera)\n> > \t * Construct the name from the camera location, model and ID. The model\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> > +\tconst auto &location = props.get(properties::Location);\n> > +\tif (location) {\n> > +\t\tswitch (*location) {\n> > \t\tcase properties::CameraLocationFront:\n> > \t\t\taddModel = false;\n> > \t\t\tname = \"Internal front camera \";\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 43db7b68dac8..d60f20b08e27 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -1143,18 +1143,17 @@ int PipelineHandlerIPU3::registerCameras()\n> > \t\t\t\t\t\t &IPU3CameraData::frameStart);\n> > \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\telse\n> > +\t\tconst auto &rotation = data->properties_.get(properties::Rotation);\n> > +\t\tif (!rotation)\n> > \t\t\tLOG(IPU3, Warning) << \"Rotation control not exposed by \"\n> > \t\t\t\t\t << cio2->sensor()->id()\n> > \t\t\t\t\t << \". Assume rotation 0\";\n> > \n> > +\t\tint32_t rotationValue = rotation.value_or(0);\n> > \t\tbool success;\n> > -\t\tdata->rotationTransform_ = transformFromRotation(rotation, &success);\n> > +\t\tdata->rotationTransform_ = transformFromRotation(rotationValue, &success);\n> > \t\tif (!success)\n> > -\t\t\tLOG(IPU3, Warning) << \"Invalid rotation of \" << rotation\n> > +\t\t\tLOG(IPU3, Warning) << \"Invalid rotation of \" << rotationValue\n> > \t\t\t\t\t << \" degrees: ignoring\";\n> > \n> > \t\tControlList ctrls = cio2->sensor()->getControls({ V4L2_CID_HFLIP });\n> > @@ -1330,8 +1329,9 @@ void IPU3CameraData::imguOutputBufferReady(FrameBuffer *buffer)\n> > \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> > +\tconst auto &scalerCrop = request->controls().get(controls::ScalerCrop);\n> > +\tif (scalerCrop)\n> > +\t\tcropRegion_ = *scalerCrop;\n> > \trequest->metadata().set(controls::ScalerCrop, cropRegion_);\n> > \n> > \tif (frameInfos_.tryComplete(info))\n> > @@ -1455,13 +1455,12 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n> > \tRequest *request = processingRequests_.front();\n> > \tprocessingRequests_.pop();\n> > \n> > -\tif (!request->controls().contains(controls::draft::TestPatternMode))\n> > +\tconst auto &testPatternMode = request->controls().get(controls::draft::TestPatternMode);\n> > +\tif (!testPatternMode)\n> > \t\treturn;\n> > \n> > -\tconst int32_t testPatternMode = *(request->controls().get(controls::draft::TestPatternMode));\n> > -\n> > \tint ret = cio2_.sensor()->setTestPatternMode(\n> > -\t\tstatic_cast(testPatternMode));\n> > +\t\tstatic_cast(*testPatternMode));\n> > \tif (ret) {\n> > \t\tLOG(IPU3, Error) << \"Failed to set test pattern mode: \"\n> > \t\t\t\t << ret;\n> > @@ -1469,7 +1468,7 @@ void IPU3CameraData::frameStart(uint32_t sequence)\n> > \t}\n> > \n> > \trequest->metadata().set(controls::draft::TestPatternMode,\n> > -\t\t\t\ttestPatternMode);\n> > +\t\t\t\t*testPatternMode);\n> > }\n> > \n> > REGISTER_PIPELINE_HANDLER(PipelineHandlerIPU3)\n> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index 28f054cb84c8..2e19531eecc4 100644\n> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > @@ -1716,16 +1716,15 @@ void RPiCameraData::statsMetadataComplete(uint32_t bufferId, const ControlList &\n> > \t * Inform the sensor of the latest colour gains if it has the\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 =\n> > -\t\t\t*controls.get(libcamera::controls::ColourGains);\n> > +\tconst auto &colourGains = controls.get(libcamera::controls::ColourGains);\n> > +\tif (notifyGainsUnity_ && 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> > -\t\t\tstatic_cast(colourGains[1] * *notifyGainsUnity_),\n> > +\t\t\tstatic_cast((*colourGains)[1] * *notifyGainsUnity_),\n> > \t\t\t*notifyGainsUnity_,\n> > \t\t\t*notifyGainsUnity_,\n> > -\t\t\tstatic_cast(colourGains[0] * *notifyGainsUnity_)\n> > +\t\t\tstatic_cast((*colourGains)[0] * *notifyGainsUnity_)\n> > \t\t};\n> > \t\tctrls.set(V4L2_CID_NOTIFY_GAINS, Span{ gains });\n> > \n> > @@ -2052,8 +2051,9 @@ Rectangle RPiCameraData::scaleIspCrop(const Rectangle &ispCrop) const\n> > \n> > void RPiCameraData::applyScalerCrop(const ControlList &controls)\n> > {\n> > -\tif (controls.contains(controls::ScalerCrop)) {\n> > -\t\tRectangle nativeCrop = *controls.get(controls::ScalerCrop);\n> > +\tconst auto &scalerCrop = controls.get(controls::ScalerCrop);\n> > +\tif (scalerCrop) {\n> > +\t\tRectangle nativeCrop = *scalerCrop;\n> > \n> > \t\tif (!nativeCrop.width || !nativeCrop.height)\n> > \t\t\tnativeCrop = { 0, 0, 1, 1 };\n> > diff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\n> > index 4b5d8276564f..5896485cda5b 100644\n> > --- a/src/qcam/dng_writer.cpp\n> > +++ b/src/qcam/dng_writer.cpp\n> > @@ -391,9 +391,9 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > \tTIFFSetField(tif, TIFFTAG_FILLORDER, FILLORDER_MSB2LSB);\n> > \tTIFFSetField(tif, TIFFTAG_MAKE, \"libcamera\");\n> > \n> > -\tif (cameraProperties.contains(properties::Model)) {\n> > -\t\tstd::string model = *cameraProperties.get(properties::Model);\n> > -\t\tTIFFSetField(tif, TIFFTAG_MODEL, model.c_str());\n> > +\tconst auto &model = cameraProperties.get(properties::Model);\n> > +\tif (model) {\n> > +\t\tTIFFSetField(tif, TIFFTAG_MODEL, model->c_str());\n> > \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n> > \t}\n> > \n> > @@ -437,16 +437,18 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > \t */\n> > \tconst double eps = 1e-2;\n> > \n> > -\tif (metadata.contains(controls::ColourGains)) {\n> > -\t\tconst auto &colourGains = metadata.get(controls::ColourGains);\n> > +\tconst auto &colourGains = metadata.get(controls::ColourGains);\n> > +\tif (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\tMatrix3d ccmSupplied(*metadata.get(controls::ColourCorrectionMatrix));\n> > +\n> > +\tconst auto &ccmControl = metadata.get(controls::ColourCorrectionMatrix);\n> > +\tif (ccmControl) {\n> > +\t\tMatrix3d ccmSupplied(*ccmControl);\n> > \t\tif (ccmSupplied.determinant() > eps)\n> > \t\t\tccm = ccmSupplied;\n> > \t}\n> > @@ -513,9 +515,9 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > \tfloat blackLevel[] = { 0.0f, 0.0f, 0.0f, 0.0f };\n> > \tuint32_t whiteLevel = (1 << info->bitsPerSample) - 1;\n> > \n> > -\tif (metadata.contains(controls::SensorBlackLevels)) {\n> > -\t\tSpan levels =\n> > -\t\t\t*metadata.get(controls::SensorBlackLevels);\n> > +\tconst auto &blackLevels = metadata.get(controls::SensorBlackLevels);\n> > +\tif (blackLevels) {\n> > +\t\tSpan levels = *blackLevels;\n> > \n> > \t\t/*\n> > \t\t * The black levels control is specified in R, Gr, Gb, B order.\n> > @@ -592,16 +594,15 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n> > \tTIFFSetField(tif, EXIFTAG_DATETIMEORIGINAL, strTime);\n> > \tTIFFSetField(tif, EXIFTAG_DATETIMEDIGITIZED, strTime);\n> > \n> > -\tif (metadata.contains(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> > +\tconst auto &analogGain = metadata.get(controls::AnalogueGain);\n> > +\tif (analogGain) {\n> > +\t\tuint16_t iso = std::min(std::max(*analogGain * 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\tTIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);\n> > -\t}\n> > +\tconst auto &exposureTime = metadata.get(controls::ExposureTime);\n> > +\tif (exposureTime)\n> > +\t\tTIFFSetField(tif, EXIFTAG_EXPOSURETIME, *exposureTime / 1e6);\n> > \n> > \tTIFFWriteCustomDirectory(tif, &exifIFDOffset);\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 BCCC7BD1F1\n\tfor ;\n\tThu, 14 Jul 2022 08:55:57 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 025BC63312;\n\tThu, 14 Jul 2022 10:55:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 59E3A63309\n\tfor ;\n\tThu, 14 Jul 2022 10:55:55 +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 AB7BF383;\n\tThu, 14 Jul 2022 10:55:54 +0200 (CEST)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1657788957;\n\tbh=Jp6wTAiev2B5BbkNEj7DPlxUVpaTsIOK8MeT1jH+MXI=;\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=xaqgTldFjxnPPOdLpfFtUBER0M1Ib/IzQnxGCr+LlnCxR+It8HNdTUTCm53Dt8DLi\n\te+dy2fQi71Fns9wL/COkVhd0gerqxpxDx1QHQURzjJ1gkaegkcIiCMVB3XIyE7UL7x\n\t59rWeyZhVWsClJHoWI+quPkdirNvO5Db3cTLa/yxoTvurZXrk4Stw3M4SyaM+2hRau\n\tnGmIM7aCLUCeVMFJvsKEXsTL6Lfh7O/qbyCW3DPgiOh4jJ+0MrMRNOQHe1Axk2WLM7\n\t3I+vXRHRyubGnYna2f6rBsksVJ8LAkpTbHgpR/suRz0zCzg8H8ipnM/ulHBAG6sDgz\n\tPkbXMiUUlo6fQ==","v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1657788954;\n\tbh=Jp6wTAiev2B5BbkNEj7DPlxUVpaTsIOK8MeT1jH+MXI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=QBAYyhcnZoEH3wsLEaB4uOzL6dZ2SI8dfyBOqFoNTq0j+t0e0MznoeM0JMhM2qAG7\n\tDjA4yd9/V9l9mIakl9IyBKiK+aLN/BSg6dwGbrHXoAq58c3atEOVZOJHrXuAku79aS\n\thaml404uTwLaYeXan6t8sdwysSbvNIgs6HiJJgCw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"QBAYyhcn\"; dkim-atps=neutral","Date":"Thu, 14 Jul 2022 11:55:23 +0300","To":"Umang Jain ","Message-ID":"","References":"<20220711003918.535-1-laurent.pinchart@ideasonboard.com>\n\t<3aaaf7e6-4d2c-38d5-333a-185c18f685e6@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<3aaaf7e6-4d2c-38d5-333a-185c18f685e6@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] libcamera: controls: Avoid double\n\tlookups","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\" "}}]