{"id":12310,"url":"https://patchwork.libcamera.org/api/1.1/patches/12310/?format=json","web_url":"https://patchwork.libcamera.org/patch/12310/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/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":"<20210518100706.578526-3-naush@raspberrypi.com>","date":"2021-05-18T10:07:04","name":"[libcamera-devel,2/4] ipa: raspberrypi: Switch ipa/cam_helper to use RPiController::Duration","commit_ref":null,"pull_url":null,"state":"superseded","archived":false,"hash":"4edd78a6b4442d8b2833233e2b35e24e654499af","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/1.1/people/34/?format=json","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/12310/mbox/","series":[{"id":2034,"url":"https://patchwork.libcamera.org/api/1.1/series/2034/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2034","date":"2021-05-18T10:07:02","name":"Switch RaspberryPi IPA to use std::chrono::duration","version":1,"mbox":"https://patchwork.libcamera.org/series/2034/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/12310/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/12310/checks/","tags":{},"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 71ED5C31FF\n\tfor ;\n\tTue, 18 May 2021 10:09:17 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1384E68925;\n\tTue, 18 May 2021 12:09:17 +0200 (CEST)","from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com\n\t[IPv6:2a00:1450:4864:20::42f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B88D068918\n\tfor ;\n\tTue, 18 May 2021 12:09:12 +0200 (CEST)","by mail-wr1-x42f.google.com with SMTP id i17so9515847wrq.11\n\tfor ;\n\tTue, 18 May 2021 03:09:12 -0700 (PDT)","from naush-laptop.pitowers.org\n\t([2a00:1098:3142:14:34e4:187b:f2:ed28])\n\tby smtp.gmail.com with ESMTPSA id\n\to11sm6566682wrq.93.2021.05.18.03.09.11\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 18 May 2021 03:09:11 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"PYwHskUA\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=NCIOs3bWMPbQlzgpBqn2iparYJkE6e2ot9O/kNv2k7U=;\n\tb=PYwHskUA9vvBPtIZ5mqSRhFqEwHpkVVo9MSjENEPBW9tM7j+Ct8SMGScmzPrF8klWs\n\tm802f0gl9K6NJVrELfswley+xeX4gHYosn6Ys9nmKmyRdTbi3C590eaZEmB8LC44zpf8\n\tUoPRRTD1bZAE7u1ZZgPeEvjXjCeKXMC/j9mYcm+hGnKHYtEGrU2PXi38jmWQo6ppRJ+u\n\t268j5fkWpITw7O77pAulBRTZbS/PPwKTiYUaCDzRujKWzcId2qMuXoDAZM3xR9ss6BQa\n\tRfbuKuQCdZM275K3WF/r8oVuj2Qjicqn7F4Nqk9C4VD0262maUU+FLJrggwfXq3hHVE7\n\tvlpg==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=NCIOs3bWMPbQlzgpBqn2iparYJkE6e2ot9O/kNv2k7U=;\n\tb=HXNL0LHBr2aSds/ttUug7Nu62IcBgAiT4OoXWIlEQ6Hmk92WzZco4OmHqkbGgzOGyH\n\tNnvhOq3brHlj28LSQrjOWpuGI3hjx2m7ykF6ZdNqHAMAwt0BLhpk4sY9fgtRFK5VygpM\n\to2pLEqoIJTmIGCciMZRNCnVi1HNr8VplUFTWnM8Lc1CCDOAwgFGpzUs+oRNkmIugbq24\n\tW4Yf8/z/uiTFU4ZyLQfJa3Ay90QWbZc2UpqvmQHMhFTN9ImsHipzqPiVeVH/gZy17ngk\n\t3Cy/ROutzsbvUQO6ykmJk2WhE0rIoWNdnWHDfMp+NkmppOgwRpGqkiJqKgBEXf+I9z9+\n\tlkPQ==","X-Gm-Message-State":"AOAM530gBZ1zN4/rGMQ/5g3svj9P7OvFHPGdqxdXWKsueNc0eISdvwZ9\n\tMAR+LmpoxwFlPZ96fuAIoj7HBZ5U2lzUJg==","X-Google-Smtp-Source":"ABdhPJxzd5PaMoBnAEbSHjRst8jx49c0Fa3cSSr5Xwj0BnTM/Cjt9L+UWhje+YSuwxH56+SuAnAKLg==","X-Received":"by 2002:a05:6000:44:: with SMTP id\n\tk4mr5905595wrx.76.1621332551983; \n\tTue, 18 May 2021 03:09:11 -0700 (PDT)","From":"Naushir Patuck ","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 18 May 2021 11:07:04 +0100","Message-Id":"<20210518100706.578526-3-naush@raspberrypi.com>","X-Mailer":"git-send-email 2.25.1","In-Reply-To":"<20210518100706.578526-1-naush@raspberrypi.com>","References":"<20210518100706.578526-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH 2/4] ipa: raspberrypi: Switch\n\tipa/cam_helper to use RPiController::Duration","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"Switch the ipa and cam_helper code to use RPiController::Duration for\nall time based variables. This improves code readability and avoids\npossible errors when converting between time bases.\n\nSigned-off-by: Naushir Patuck \n---\n src/ipa/raspberrypi/cam_helper.cpp | 19 +++---\n src/ipa/raspberrypi/cam_helper.hpp | 10 +--\n src/ipa/raspberrypi/controller/camera_mode.h | 5 +-\n src/ipa/raspberrypi/raspberrypi.cpp | 67 +++++++++++---------\n 4 files changed, 56 insertions(+), 45 deletions(-)","diff":"diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\nindex 09917f3cc079..e2b6c8eb8e03 100644\n--- a/src/ipa/raspberrypi/cam_helper.cpp\n+++ b/src/ipa/raspberrypi/cam_helper.cpp\n@@ -61,20 +61,21 @@ void CamHelper::Process([[maybe_unused]] StatisticsPtr &stats,\n {\n }\n \n-uint32_t CamHelper::ExposureLines(double exposure_us) const\n+uint32_t CamHelper::ExposureLines(Duration exposure) const\n {\n \tassert(initialized_);\n-\treturn exposure_us * 1000.0 / mode_.line_length;\n+\treturn exposure / mode_.line_length;\n }\n \n-double CamHelper::Exposure(uint32_t exposure_lines) const\n+Duration CamHelper::Exposure(uint32_t exposure_lines) const\n {\n \tassert(initialized_);\n-\treturn exposure_lines * mode_.line_length / 1000.0;\n+\treturn exposure_lines * mode_.line_length;\n }\n \n-uint32_t CamHelper::GetVBlanking(double &exposure, double minFrameDuration,\n-\t\t\t\t double maxFrameDuration) const\n+uint32_t CamHelper::GetVBlanking(Duration &exposure,\n+\t\t\t\t Duration minFrameDuration,\n+\t\t\t\t Duration maxFrameDuration) const\n {\n \tuint32_t frameLengthMin, frameLengthMax, vblank;\n \tuint32_t exposureLines = ExposureLines(exposure);\n@@ -85,8 +86,8 @@ uint32_t CamHelper::GetVBlanking(double &exposure, double minFrameDuration,\n \t * minFrameDuration and maxFrameDuration are clamped by the caller\n \t * based on the limits for the active sensor mode.\n \t */\n-\tframeLengthMin = 1e3 * minFrameDuration / mode_.line_length;\n-\tframeLengthMax = 1e3 * maxFrameDuration / mode_.line_length;\n+\tframeLengthMin = minFrameDuration / mode_.line_length;\n+\tframeLengthMax = maxFrameDuration / mode_.line_length;\n \n \t/*\n \t * Limit the exposure to the maximum frame duration requested, and\n@@ -182,7 +183,7 @@ void CamHelper::parseEmbeddedData(Span buffer,\n \t\treturn;\n \t}\n \n-\tdeviceStatus.shutter_speed = Exposure(exposureLines);\n+\tdeviceStatus.shutter_speed = DurationValue(Exposure(exposureLines));\n \tdeviceStatus.analogue_gain = Gain(gainCode);\n \n \tLOG(IPARPI, Debug) << \"Metadata updated - Exposure : \"\ndiff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp\nindex a52f3f0b583c..a91afaa59909 100644\n--- a/src/ipa/raspberrypi/cam_helper.hpp\n+++ b/src/ipa/raspberrypi/cam_helper.hpp\n@@ -13,6 +13,7 @@\n #include \"camera_mode.h\"\n #include \"controller/controller.hpp\"\n #include \"controller/metadata.hpp\"\n+#include \"duration.hpp\"\n #include \"md_parser.hpp\"\n \n #include \"libcamera/internal/v4l2_videodevice.h\"\n@@ -72,10 +73,11 @@ public:\n \tvirtual void Prepare(libcamera::Span buffer,\n \t\t\t Metadata &metadata);\n \tvirtual void Process(StatisticsPtr &stats, Metadata &metadata);\n-\tuint32_t ExposureLines(double exposure_us) const;\n-\tdouble Exposure(uint32_t exposure_lines) const; // in us\n-\tvirtual uint32_t GetVBlanking(double &exposure_us, double minFrameDuration,\n-\t\t\t\t double maxFrameDuration) const;\n+\tuint32_t ExposureLines(Duration exposure) const;\n+\tDuration Exposure(uint32_t exposure_lines) const;\n+\tvirtual uint32_t GetVBlanking(Duration &exposure,\n+\t\t\t\t Duration minFrameDuration,\n+\t\t\t\t Duration maxFrameDuration) const;\n \tvirtual uint32_t GainCode(double gain) const = 0;\n \tvirtual double Gain(uint32_t gain_code) const = 0;\n \tvirtual void GetDelays(int &exposure_delay, int &gain_delay,\ndiff --git a/src/ipa/raspberrypi/controller/camera_mode.h b/src/ipa/raspberrypi/controller/camera_mode.h\nindex 256438c931d9..ab7cf2912a06 100644\n--- a/src/ipa/raspberrypi/controller/camera_mode.h\n+++ b/src/ipa/raspberrypi/controller/camera_mode.h\n@@ -6,6 +6,7 @@\n */\n #pragma once\n \n+#include \"duration.hpp\"\n #include \n \n // Description of a \"camera mode\", holding enough information for control\n@@ -33,8 +34,8 @@ struct CameraMode {\n \tdouble scale_x, scale_y;\n \t// scaling of the noise compared to the native sensor mode\n \tdouble noise_factor;\n-\t// line time in nanoseconds\n-\tdouble line_length;\n+\t// line time\n+\tRPiController::Duration line_length;\n \t// any camera transform *not* reflected already in the camera tuning\n \tlibcamera::Transform transform;\n \t// minimum and maximum fame lengths in units of lines\ndiff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\nindex 52d91db282ea..994fb7e057a9 100644\n--- a/src/ipa/raspberrypi/raspberrypi.cpp\n+++ b/src/ipa/raspberrypi/raspberrypi.cpp\n@@ -45,6 +45,7 @@\n #include \"denoise_algorithm.hpp\"\n #include \"denoise_status.h\"\n #include \"dpc_status.h\"\n+#include \"duration.hpp\"\n #include \"focus_status.h\"\n #include \"geq_status.h\"\n #include \"lux_status.h\"\n@@ -55,19 +56,24 @@\n \n namespace libcamera {\n \n+using namespace std::literals::chrono_literals;\n+using RPiController::Duration;\n+using RPiController::DurationValue;\n+using RPiController::operator<<;\n+\n /* Configure the sensor with these values initially. */\n constexpr double DefaultAnalogueGain = 1.0;\n-constexpr unsigned int DefaultExposureTime = 20000;\n-constexpr double defaultMinFrameDuration = 1e6 / 30.0;\n-constexpr double defaultMaxFrameDuration = 1e6 / 0.01;\n+constexpr Duration DefaultExposureTime = 20.0ms;\n+constexpr Duration defaultMinFrameDuration = 1.0s / 30.0;\n+constexpr Duration defaultMaxFrameDuration = 100.0s;\n \n /*\n- * Determine the minimum allowable inter-frame duration (in us) to run the\n- * controller algorithms. If the pipeline handler provider frames at a rate\n- * higher than this, we rate-limit the controller Prepare() and Process() calls\n- * to lower than or equal to this rate.\n+ * Determine the minimum allowable inter-frame duration to run the controller\n+ * algorithms. If the pipeline handler provider frames at a rate higher than this,\n+ * we rate-limit the controller Prepare() and Process() calls to lower than or\n+ * equal to this rate.\n */\n-constexpr double controllerMinFrameDuration = 1e6 / 60.0;\n+constexpr Duration controllerMinFrameDuration = 1.0s / 60.0;\n \n LOG_DEFINE_CATEGORY(IPARPI)\n \n@@ -111,7 +117,8 @@ private:\n \tvoid reportMetadata();\n \tvoid fillDeviceStatus(const ControlList &sensorControls);\n \tvoid processStats(unsigned int bufferId);\n-\tvoid applyFrameDurations(double minFrameDuration, double maxFrameDuration);\n+\tvoid applyFrameDurations(const Duration &minFrameDuration,\n+\t\t\t\t const Duration &maxFrameDuration);\n \tvoid applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls);\n \tvoid applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls);\n \tvoid applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls);\n@@ -167,9 +174,9 @@ private:\n \t/* Distinguish the first camera start from others. */\n \tbool firstStart_;\n \n-\t/* Frame duration (1/fps) limits, given in microseconds. */\n-\tdouble minFrameDuration_;\n-\tdouble maxFrameDuration_;\n+\t/* Frame duration (1/fps) limits. */\n+\tDuration minFrameDuration_;\n+\tDuration maxFrameDuration_;\n };\n \n int IPARPi::init(const IPASettings &settings, ipa::RPi::SensorConfig *sensorConfig)\n@@ -314,7 +321,7 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)\n \t * Calculate the line length in nanoseconds as the ratio between\n \t * the line length in pixels and the pixel rate.\n \t */\n-\tmode_.line_length = 1e9 * sensorInfo.lineLength / sensorInfo.pixelRate;\n+\tmode_.line_length = sensorInfo.lineLength * (1.0s / sensorInfo.pixelRate);\n \n \t/*\n \t * Set the frame length limits for the mode to ensure exposure and\n@@ -387,7 +394,7 @@ int IPARPi::configure(const CameraSensorInfo &sensorInfo,\n \t\t/* Supply initial values for gain and exposure. */\n \t\tControlList ctrls(sensorCtrls_);\n \t\tAgcStatus agcStatus;\n-\t\tagcStatus.shutter_time = DefaultExposureTime;\n+\t\tagcStatus.shutter_time = DurationValue(DefaultExposureTime);\n \t\tagcStatus.analogue_gain = DefaultAnalogueGain;\n \t\tapplyAGC(&agcStatus, ctrls);\n \n@@ -862,7 +869,7 @@ void IPARPi::queueRequest(const ControlList &controls)\n \n \t\tcase controls::FRAME_DURATIONS: {\n \t\t\tauto frameDurations = ctrl.second.get>();\n-\t\t\tapplyFrameDurations(frameDurations[0], frameDurations[1]);\n+\t\t\tapplyFrameDurations(frameDurations[0] * 1.0us, frameDurations[1] * 1.0us);\n \t\t\tbreak;\n \t\t}\n \n@@ -937,9 +944,9 @@ void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data)\n \t\treturnEmbeddedBuffer(data.embeddedBufferId);\n \n \t/* Allow a 10% margin on the comparison below. */\n-\tconstexpr double eps = controllerMinFrameDuration * 1e3 * 0.1;\n+\tDuration delta = (frameTimestamp - lastRunTimestamp_) * 1.0ns;\n \tif (lastRunTimestamp_ && frameCount_ > dropFrameCount_ &&\n-\t frameTimestamp - lastRunTimestamp_ + eps < controllerMinFrameDuration * 1e3) {\n+\t delta < controllerMinFrameDuration * 0.9) {\n \t\t/*\n \t\t * Ensure we merge the previous frame's metadata with the current\n \t\t * frame. This will not overwrite exposure/gain values for the\n@@ -1012,7 +1019,7 @@ void IPARPi::fillDeviceStatus(const ControlList &sensorControls)\n \tint32_t exposureLines = sensorControls.get(V4L2_CID_EXPOSURE).get();\n \tint32_t gainCode = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get();\n \n-\tdeviceStatus.shutter_speed = helper_->Exposure(exposureLines);\n+\tdeviceStatus.shutter_speed = DurationValue(helper_->Exposure(exposureLines));\n \tdeviceStatus.analogue_gain = helper_->Gain(gainCode);\n \n \tLOG(IPARPI, Debug) << \"Metadata - Exposure : \"\n@@ -1057,17 +1064,18 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)\n \t\t static_cast(awbStatus->gain_b * 1000));\n }\n \n-void IPARPi::applyFrameDurations(double minFrameDuration, double maxFrameDuration)\n+void IPARPi::applyFrameDurations(const Duration &minFrameDuration,\n+\t\t\t\t const Duration &maxFrameDuration)\n {\n-\tconst double minSensorFrameDuration = 1e-3 * mode_.min_frame_length * mode_.line_length;\n-\tconst double maxSensorFrameDuration = 1e-3 * mode_.max_frame_length * mode_.line_length;\n+\tconst Duration minSensorFrameDuration = mode_.min_frame_length * mode_.line_length;\n+\tconst Duration maxSensorFrameDuration = mode_.max_frame_length * mode_.line_length;\n \n \t/*\n \t * This will only be applied once AGC recalculations occur.\n \t * The values may be clamped based on the sensor mode capabilities as well.\n \t */\n-\tminFrameDuration_ = minFrameDuration ? minFrameDuration : defaultMaxFrameDuration;\n-\tmaxFrameDuration_ = maxFrameDuration ? maxFrameDuration : defaultMinFrameDuration;\n+\tminFrameDuration_ = (minFrameDuration > 0.0s) ? minFrameDuration : defaultMaxFrameDuration;\n+\tmaxFrameDuration_ = (maxFrameDuration > 0.0s) ? maxFrameDuration : defaultMinFrameDuration;\n \tminFrameDuration_ = std::clamp(minFrameDuration_,\n \t\t\t\t minSensorFrameDuration, maxSensorFrameDuration);\n \tmaxFrameDuration_ = std::clamp(maxFrameDuration_,\n@@ -1076,20 +1084,20 @@ void IPARPi::applyFrameDurations(double minFrameDuration, double maxFrameDuratio\n \n \t/* Return the validated limits via metadata. */\n \tlibcameraMetadata_.set(controls::FrameDurations,\n-\t\t\t { static_cast(minFrameDuration_),\n-\t\t\t\t static_cast(maxFrameDuration_) });\n+\t\t\t { static_cast(DurationValue(minFrameDuration_)),\n+\t\t\t\t static_cast(DurationValue(minFrameDuration_)) });\n \n \t/*\n \t * Calculate the maximum exposure time possible for the AGC to use.\n \t * GetVBlanking() will update maxShutter with the largest exposure\n \t * value possible.\n \t */\n-\tdouble maxShutter = std::numeric_limits::max();\n+\tDuration maxShutter = Duration::max();\n \thelper_->GetVBlanking(maxShutter, minFrameDuration_, maxFrameDuration_);\n \n \tRPiController::AgcAlgorithm *agc = dynamic_cast(\n \t\tcontroller_.GetAlgorithm(\"agc\"));\n-\tagc->SetMaxShutter(maxShutter);\n+\tagc->SetMaxShutter(DurationValue(maxShutter));\n }\n \n void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n@@ -1097,9 +1105,8 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n \tint32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);\n \n \t/* GetVBlanking might clip exposure time to the fps limits. */\n-\tdouble exposure = agcStatus->shutter_time;\n-\tint32_t vblanking = helper_->GetVBlanking(exposure, minFrameDuration_,\n-\t\t\t\t\t\t maxFrameDuration_);\n+\tDuration exposure = agcStatus->shutter_time * 1.0us;\n+\tint32_t vblanking = helper_->GetVBlanking(exposure, minFrameDuration_, maxFrameDuration_);\n \tint32_t exposureLines = helper_->ExposureLines(exposure);\n \n \tLOG(IPARPI, Debug) << \"Applying AGC Exposure: \" << exposure\n","prefixes":["libcamera-devel","2/4"]}