{"id":12411,"url":"https://patchwork.libcamera.org/api/1.1/patches/12411/?format=json","web_url":"https://patchwork.libcamera.org/patch/12411/","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":"<20210525114241.906280-3-naush@raspberrypi.com>","date":"2021-05-25T11:42:39","name":"[libcamera-devel,v5,2/4] ipa: raspberrypi: Switch ipa and cam_helper to use utils::Duration","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"6d385af52553e48926e6b9db78a3ac60f96b1d85","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/12411/mbox/","series":[{"id":2066,"url":"https://patchwork.libcamera.org/api/1.1/series/2066/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=2066","date":"2021-05-25T11:42:37","name":"Switch RaspberryPi IPA to use std::chrono::duration","version":5,"mbox":"https://patchwork.libcamera.org/series/2066/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/12411/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/12411/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 C96A0C3203\n\tfor ;\n\tTue, 25 May 2021 11:42:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 84B056891F;\n\tTue, 25 May 2021 13:42:51 +0200 (CEST)","from mail-wm1-x331.google.com (mail-wm1-x331.google.com\n\t[IPv6:2a00:1450:4864:20::331])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id C49656891D\n\tfor ;\n\tTue, 25 May 2021 13:42:47 +0200 (CEST)","by mail-wm1-x331.google.com with SMTP id o127so16566312wmo.4\n\tfor ;\n\tTue, 25 May 2021 04:42:47 -0700 (PDT)","from naush-laptop.pitowers.org\n\t([2a00:1098:3142:14:f3e2:c7ee:8a9d:4c3a])\n\tby smtp.gmail.com with ESMTPSA id\n\tv15sm2471443wmj.39.2021.05.25.04.42.46\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 25 May 2021 04:42:46 -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=\"EDn/2Lke\"; 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=qxNaJL4XU2oX+VMWzO423IF/cmdKTCrbqyKNFA8tTkI=;\n\tb=EDn/2LkexNzy67o6RIf0QsgSXv3HVkKKGqH4DZ9jGGBRXAlmtwD0A/WTgeOg9th96x\n\tVpx4Wgh/0PGlXgzbELl0adf7AWP5U72Jwy0pS64AEs8u7EWpTBH5Vr+5axaMle9MK9U7\n\tKSMQ5xTqK5EAsbTlIEZRggyf++/GKr0bvwutIYfsJirmYGt4pgd2ZfdYAnchBr061DIv\n\tVM4fK7D9GBv59wOiKDqovy1BtqCvdHLnG3cgP0RApR55+Zmi3Scz/HjXH4jWnd1ZbMes\n\tFJ5/d2blOUsf5+rik304FnHb9UjIIAM5le8G2y3+DT5fd4gCCH004p84XXj4RN/IG5QP\n\tiwHQ==","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=qxNaJL4XU2oX+VMWzO423IF/cmdKTCrbqyKNFA8tTkI=;\n\tb=ItQi72XOH7GSWgiQW24JD/rpr+8PXDPNojxepeHUn+Jy4s+Unk5fLmMUTd4SUljF+7\n\tmuft6B9lTIMGuquCuph03S8gviinB8/wR/xO5Kfa+YQsA/K0abEy+Bd3q7tdV9Mh0QSh\n\tmL/U2g0a3QNYXWTODyoW74joshNvmFnmxsMYUOlXMxkJWGGZtWC7utxKkNmkFB29MKY+\n\tqg4L6l9YJczB9QOAgbX5F8HVGL5JL12chxR+n1W1X9NOK5R+jhMd4kZLRpfaYf60FLTu\n\tq4H9jCHZSJJBOYGcigE1/HmV+jlIeHaP7p7W2kPE0GXWnf85YTzXxl2/HYuE2SgqFhq7\n\tMbPQ==","X-Gm-Message-State":"AOAM533Cm8AcYBxt5kHPxiceyDwaF/JO1siXv48M05JkURrNAdwGSB1Q\n\t7Fr349S5cckFrd2Pg2YDRl6r3QxYUm5a+Q==","X-Google-Smtp-Source":"ABdhPJxMXi+heLYL4khKWyOGfpB89JfBa8f1bacKxlYcxFY0o82UHuV4wzZ/35BY6/Crfzimxn6oDQ==","X-Received":"by 2002:a7b:cd01:: with SMTP id f1mr3417613wmj.177.1621942967144;\n\tTue, 25 May 2021 04:42:47 -0700 (PDT)","From":"Naushir Patuck ","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 25 May 2021 12:42:39 +0100","Message-Id":"<20210525114241.906280-3-naush@raspberrypi.com>","X-Mailer":"git-send-email 2.25.1","In-Reply-To":"<20210525114241.906280-1-naush@raspberrypi.com>","References":"<20210525114241.906280-1-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH v5 2/4] ipa: raspberrypi: Switch ipa and\n\tcam_helper to use utils::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 libcamera::utils::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 \nReviewed-by: David Plowman \n---\n src/ipa/raspberrypi/cam_helper.cpp | 20 +++---\n src/ipa/raspberrypi/cam_helper.hpp | 10 +--\n src/ipa/raspberrypi/controller/camera_mode.h | 6 +-\n src/ipa/raspberrypi/raspberrypi.cpp | 64 +++++++++++---------\n 4 files changed, 55 insertions(+), 45 deletions(-)","diff":"diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp\nindex 09917f3cc079..7b4331a87a42 100644\n--- a/src/ipa/raspberrypi/cam_helper.cpp\n+++ b/src/ipa/raspberrypi/cam_helper.cpp\n@@ -18,6 +18,7 @@\n \n using namespace RPiController;\n using namespace libcamera;\n+using libcamera::utils::Duration;\n \n namespace libcamera {\n LOG_DECLARE_CATEGORY(IPARPI)\n@@ -61,20 +62,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(const 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 const Duration &minFrameDuration,\n+\t\t\t\t const Duration &maxFrameDuration) const\n {\n \tuint32_t frameLengthMin, frameLengthMax, vblank;\n \tuint32_t exposureLines = ExposureLines(exposure);\n@@ -85,8 +87,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 +184,7 @@ void CamHelper::parseEmbeddedData(Span buffer,\n \t\treturn;\n \t}\n \n-\tdeviceStatus.shutter_speed = Exposure(exposureLines);\n+\tdeviceStatus.shutter_speed = Exposure(exposureLines).get();\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..07481e4174a7 100644\n--- a/src/ipa/raspberrypi/cam_helper.hpp\n+++ b/src/ipa/raspberrypi/cam_helper.hpp\n@@ -15,6 +15,7 @@\n #include \"controller/metadata.hpp\"\n #include \"md_parser.hpp\"\n \n+#include \"libcamera/internal/utils.h\"\n #include \"libcamera/internal/v4l2_videodevice.h\"\n \n namespace RPiController {\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(const libcamera::utils::Duration &exposure) const;\n+\tlibcamera::utils::Duration Exposure(uint32_t exposure_lines) const;\n+\tvirtual uint32_t GetVBlanking(libcamera::utils::Duration &exposure,\n+\t\t\t\t const libcamera::utils::Duration &minFrameDuration,\n+\t\t\t\t const libcamera::utils::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..2aa2335dcf90 100644\n--- a/src/ipa/raspberrypi/controller/camera_mode.h\n+++ b/src/ipa/raspberrypi/controller/camera_mode.h\n@@ -8,6 +8,8 @@\n \n #include \n \n+#include \"libcamera/internal/utils.h\"\n+\n // Description of a \"camera mode\", holding enough information for control\n // algorithms to adapt their behaviour to the different modes of the camera,\n // including binning, scaling, cropping etc.\n@@ -33,8 +35,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+\tlibcamera::utils::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..4e02e94084f7 100644\n--- a/src/ipa/raspberrypi/raspberrypi.cpp\n+++ b/src/ipa/raspberrypi/raspberrypi.cpp\n@@ -55,19 +55,22 @@\n \n namespace libcamera {\n \n+using namespace std::literals::chrono_literals;\n+using utils::Duration;\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 +114,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 +171,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@@ -311,10 +315,10 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)\n \tmode_.noise_factor = sqrt(mode_.bin_x * mode_.bin_y);\n \n \t/*\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 * Calculate the line length as the ratio between the line length in\n+\t * 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 +391,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 = DefaultExposureTime.get();\n \t\tagcStatus.analogue_gain = DefaultAnalogueGain;\n \t\tapplyAGC(&agcStatus, ctrls);\n \n@@ -862,7 +866,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 +941,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 +1016,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 = helper_->Exposure(exposureLines).get();\n \tdeviceStatus.analogue_gain = helper_->Gain(gainCode);\n \n \tLOG(IPARPI, Debug) << \"Metadata - Exposure : \"\n@@ -1057,10 +1061,11 @@ 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@@ -1076,20 +1081,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(minFrameDuration_.get()),\n+\t\t\t\t static_cast(maxFrameDuration_.get()) });\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(maxShutter.get());\n }\n \n void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)\n@@ -1097,9 +1102,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","v5","2/4"]}