{"id":16127,"url":"https://patchwork.libcamera.org/api/patches/16127/?format=json","web_url":"https://patchwork.libcamera.org/patch/16127/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20220601231802.16735-2-Rauch.Christian@gmx.de>","date":"2022-06-01T23:17:59","name":"[libcamera-devel,v5,1/4] libcamera: controls: Use std::optional to handle invalid control values","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"045b11a32d687198ef18bb8c059b588e01ee5362","submitter":{"id":111,"url":"https://patchwork.libcamera.org/api/people/111/?format=json","name":"Christian Rauch","email":"Rauch.Christian@gmx.de"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/16127/mbox/","series":[{"id":3152,"url":"https://patchwork.libcamera.org/api/series/3152/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=3152","date":"2022-06-01T23:17:58","name":"generate and use fixed-sized Span Control types","version":5,"mbox":"https://patchwork.libcamera.org/series/3152/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/16127/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/16127/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 35771C3256\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  1 Jun 2022 23:18:20 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0EEC165637;\n\tThu,  2 Jun 2022 01:18:19 +0200 (CEST)","from mout.gmx.net (mout.gmx.net [212.227.17.22])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A81EC60414\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  2 Jun 2022 01:18:15 +0200 (CEST)","from localhost.localdomain ([92.18.80.244]) by mail.gmx.net\n\t(mrgmx105 [212.227.17.168]) with ESMTPSA (Nemesis) id\n\t1MnJlc-1nUsHT0mOK-00jFUa; Thu, 02 Jun 2022 01:18:15 +0200"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1654125499;\n\tbh=dYd4SEO/wKUUSY9mxAYDnNUBt2P9MmuPdq+E3WZUQhc=;\n\th=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:\n\tFrom;\n\tb=fNjqzZhSXM/uzQRVco8/lbq1lDO8PSsuwwDrQziihg5YgDniK3VGg+1kEWHpBPE6f\n\trMBzj/bdvK/MzCtA1cRW/eOSsBF31el1KOT08wo9SEluY4gdnx3b9bGlpJ8DKLfvY5\n\tXyKXoXRHSoRTT1yhaas/CRwEa+jvxcpvWYDQeQoJK+qzJ6wIX88GQJZo7mS2KtDzyx\n\te8ILF0c9HhnLnVD6RCWYNENYCSzKSdX/j5wgD7Oid1bJp9E10jIndQ03xCMvb9PqU+\n\tdhbccKnUi8sXJJ7SBmJT/UQZuTxgf8wCZBixzCWf1TJJQ+sgQZum4Elfu1fyXt3AW2\n\tBRck5R6JfnvPw==","v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net;\n\ts=badeba3b8450; t=1654125495;\n\tbh=dYd4SEO/wKUUSY9mxAYDnNUBt2P9MmuPdq+E3WZUQhc=;\n\th=X-UI-Sender-Class:From:To:Cc:Subject:Date:In-Reply-To:References;\n\tb=PYSm5P3IxbFCv7t23iJEsJCLK5TIqyQ0WhU9R1WRiex9EZRsKIGEp7BYToLf2Rgd+\n\tnPN5f/GZCAlzrhH+6QCbiMWQU4Cxzqcjga2Cx+cGKXDDfjig2SRDqAZFw5pjY2UpTT\n\tGGz9Lo5wgp4DHs5o3K0MGM2kXO43ePWB1xAFH3Uw="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=gmx.net header.i=@gmx.net\n\theader.b=\"PYSm5P3I\"; dkim-atps=neutral","X-UI-Sender-Class":"01bb95c1-4bf8-414a-932a-4f6e2808ef9c","To":"libcamera-devel@lists.libcamera.org","Date":"Thu,  2 Jun 2022 00:17:59 +0100","Message-Id":"<20220601231802.16735-2-Rauch.Christian@gmx.de>","X-Mailer":"git-send-email 2.34.1","In-Reply-To":"<20220601231802.16735-1-Rauch.Christian@gmx.de>","References":"<20220601231802.16735-1-Rauch.Christian@gmx.de>","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","X-Provags-ID":"V03:K1:Op+qXHQQEKBCOheyQk/oTn6rNWtA5UwB5UlmUjMIYdkyMgHP7di\n\tGTqa8hPOhtizsErYUJc4d6FmDKg3uQLDpR/6RLDVKdM93tz5MndGZOmil4q2yfH0I1VeJUf\n\tHiktF/DUbp48Nx7fS7mxVgTUlJ83ZnHzxYcNokjzYttDxyQ0I2FRbnKn8lONWoRFIjhBf3o\n\tHb1hBKo2LkULtcs5BOGFQ==","X-Spam-Flag":"NO","X-UI-Out-Filterresults":"notjunk:1; V03:K0:c+PSh9tbOF0=:2Z85xk1X2cmusSkETqZGmE\n\tiSGhRldlUkP0MunAj8EN/XDeISBEne9zx7xrqukBqV2sJo8eDez8Kat+z7bclfL0OmF8jpu+S\n\tYqffFRKI6FNiNqBPVjozSrFvnGKytR+gFIT2b1vKe28OcrQOOHgfM7yFiiCwNiOsIBKraZuPN\n\t9yGxuwARLj5gBaRGQrY4BNtnP7jXzHIvvWcwEHZotV92hzpou8fYFBTDSjEStp38ccPL/zVFo\n\ty9f3MpDnCsUD9wqAKW4aF8gul0O/bEiQ8B02QyEBitYS+ppS0PzvxLaS231YXyAxQ7gKIw1Cx\n\tZAYy2AnXgehCfW8707KfrAWF/zwrOABKGOBcf96B0010O5EfrOm9qyrv7QzkV1OR/1nktemU6\n\tet8DbvCmtaLj4WYlCtrs9HluGrck9bqK/pGIoRWIxpdPO2fC7+NKtIhi5xF1FjxiiI4M+gbl5\n\tGhrQyfqiZJtkg5L+dnbKP1bWaDBZhQncJTQ+ad4VK1Pd5m8cbLcmR98JmyEAE0pMwEu8HI0Ih\n\t3AAECtmTMUdmrZPLbx3wAtYIquvMzAr/qyp5nijaPOpbDWzv0ANM9sXSwRuRCoDvETPuQ32pG\n\ta4nS2WgLGyCGNCB2i4aNGeh1EwAYbsRKFr22eTRyNrQKrCovcxtP/+H6TTfZkeFfz1ApZYdqf\n\tsCQQQglAIl+4VVVbpL/BUvhWYMmpWN3XZ5kVdwSruB4f+9PYfl6SJyZWy0+dJzKO1AgK+obDA\n\t/x5QbaNnj5uds6wCoG2US1Zhdje9JqGzmH4RdvINLXPo9I+9BUz7Wb9rO4ydYwyBmShcZQQsg\n\t70/2SvGYusRa2BsQcz29wu9T78BIqdS0IcauyZ7BECC7P6GrUm9ltg3g2gBUI785Ba8rR1WLO\n\tQuG4uPBIhvSo2PSIBRMdTYGyQP8TVfQUMKVA824BdVAlWewh8KZiBMNXfPQCeiUSHQPd5F0Kc\n\tuAuNN1MIWKvDGpIktM7MijPFBd6W5egr+VtpezJwrHhaE3PMQ92hxRAVjP5JGyQLhfljCuH2q\n\tbz3zfQGzmGlCh8PQwXndqMiDvO241jpTEJiG//ugDQGoIVG2nlhfq6Z8qryS1+N0cGd+EZrcV\n\tvIHQpJRKJnJnY48NTJFJhOKLNb+88M6fG4boLxkJdf4kEVShlYpnoqpxg==","Subject":"[libcamera-devel] [PATCH v5 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":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","From":"Christian Rauch via libcamera-devel\n\t<libcamera-devel@lists.libcamera.org>","Reply-To":"Christian Rauch <Rauch.Christian@gmx.de>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"Previously, ControlList::get<T>() would use default constructed objects to\nindicate that a ControlList does not have the requested Control. This has\nseveral disadvantages: 1) It requires types to be default constructible,\n2) it does not differentiate between a default constructed object and an\nobject that happens to have the same state as a default constructed object.\n\nstd::optional<T> additionally stores the information if the object is valid\nor not, and therefore is more expressive than a default constructed object.\n\nSigned-off-by: Christian Rauch <Rauch.Christian@gmx.de>\n---\n include/libcamera/controls.h                      |  7 ++++---\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          | 10 ++++++----\n src/qcam/dng_writer.cpp                           | 15 +++++++++------\n 6 files changed, 26 insertions(+), 21 deletions(-)\n\n--\n2.34.1","diff":"diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h\nindex 665bcac1..363e7809 100644\n--- a/include/libcamera/controls.h\n+++ b/include/libcamera/controls.h\n@@ -13,6 +13,7 @@\n #include <string>\n #include <unordered_map>\n #include <vector>\n+#include <optional>\n\n #include <libcamera/base/class.h>\n #include <libcamera/base/span.h>\n@@ -167,7 +168,7 @@ public:\n\n \t\tusing V = typename T::value_type;\n \t\tconst V *value = reinterpret_cast<const V *>(data().data());\n-\t\treturn { value, numElements_ };\n+\t\treturn T{ value, numElements_ };\n \t}\n\n #ifndef __DOXYGEN__\n@@ -373,11 +374,11 @@ public:\n \tbool contains(unsigned int id) const;\n\n \ttemplate<typename T>\n-\tT get(const Control<T> &ctrl) const\n+\tstd::optional<T> get(const Control<T> &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<T>();\n \t}\ndiff --git a/src/cam/main.cpp b/src/cam/main.cpp\nindex 79875ed7..9b773931 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).value_or(int32_t{})) {\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).value_or(std::string{}) + \"' \";\n \t}\n\n \tname += \"(\" + camera->id() + \")\";\ndiff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\nindex 3b126bb5..00600a2e 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(int64_t{});\n \tRPiController::Metadata lastMetadata;\n \tSpan<uint8_t> embeddedBuffer;\n\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex fd989e61..1e9e5081 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).value_or(int32_t{});\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).value_or(Rectangle{});\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(int64_t{}),\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).value_or(int32_t{});\n\n \tint ret = cio2_.sensor()->setTestPatternMode(\n \t\tstatic_cast<controls::draft::TestPatternModeEnum>(testPatternMode));\ndiff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex adc397e8..556af882 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(int32_t{});\n \tbool success;\n \tTransform rotationTransform = transformFromRotation(rotation, &success);\n \tif (!success)\n@@ -1706,7 +1706,9 @@ 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<const float> colourGains = controls.get(libcamera::controls::ColourGains);\n+\t\tlibcamera::Span<const float> colourGains =\n+\t\t\tcontrols.get(libcamera::controls::ColourGains).\\\n+\t\t\t\tvalue_or(libcamera::Span<const float>({ 0, 0 }));\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<int32_t, 4> gains{\n@@ -2041,7 +2043,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<Rectangle>(controls::ScalerCrop);\n+\t\tRectangle nativeCrop = controls.get<Rectangle>(controls::ScalerCrop).value_or(Rectangle{});\n\n \t\tif (!nativeCrop.width || !nativeCrop.height)\n \t\t\tnativeCrop = { 0, 0, 1, 1 };\n@@ -2079,7 +2081,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(int64_t{}));\n\n \trequest->metadata().set(controls::ScalerCrop, scalerCrop_);\n }\ndiff --git a/src/qcam/dng_writer.cpp b/src/qcam/dng_writer.cpp\nindex 34c8df5a..e119ca52 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).value_or(std::string{});\n \t\tTIFFSetField(tif, TIFFTAG_MODEL, model.c_str());\n \t\t/* \\todo set TIFFTAG_UNIQUECAMERAMODEL. */\n \t}\n@@ -438,7 +438,8 @@ 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 float> const &colourGains = metadata.get(controls::ColourGains);\n+\t\tSpan<const float> const &colourGains =\n+\t\t\tmetadata.get(controls::ColourGains).value_or(libcamera::Span<const float>({ 0, 0 }));\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@@ -446,7 +447,8 @@ int DNGWriter::write(const char *filename, const Camera *camera,\n \t\t}\n \t}\n \tif (metadata.contains(controls::ColourCorrectionMatrix)) {\n-\t\tSpan<const float> const &coeffs = metadata.get(controls::ColourCorrectionMatrix);\n+\t\tSpan<const float> const &coeffs =\n+\t\t\tmetadata.get(controls::ColourCorrectionMatrix).value_or(Span<const float>({ 0, 0, 0, 0, 0, 0, 0, 0, 0 }));\n \t\tMatrix3d ccmSupplied(coeffs);\n \t\tif (ccmSupplied.determinant() > eps)\n \t\t\tccm = ccmSupplied;\n@@ -515,7 +517,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<const int32_t> levels = metadata.get(controls::SensorBlackLevels);\n+\t\tSpan<const int32_t> levels =\n+\t\t\tmetadata.get(controls::SensorBlackLevels).value_or(Span<const int32_t>({ 0, 0, 0, 0 }));\n\n \t\t/*\n \t\t * The black levels control is specified in R, Gr, Gb, B order.\n@@ -593,13 +596,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).value_or(float{});\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).value_or(float{}) / 1e6;\n \t\tTIFFSetField(tif, EXIFTAG_EXPOSURETIME, exposureTime);\n \t}\n\n","prefixes":["libcamera-devel","v5","1/4"]}