From patchwork Tue May 18 10:07:04 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Naushir Patuck X-Patchwork-Id: 12310 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 71ED5C31FF for ; Tue, 18 May 2021 10:09:17 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 1384E68925; Tue, 18 May 2021 12:09:17 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="PYwHskUA"; dkim-atps=neutral Received: from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com [IPv6:2a00:1450:4864:20::42f]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id B88D068918 for ; Tue, 18 May 2021 12:09:12 +0200 (CEST) Received: by mail-wr1-x42f.google.com with SMTP id i17so9515847wrq.11 for ; Tue, 18 May 2021 03:09:12 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=NCIOs3bWMPbQlzgpBqn2iparYJkE6e2ot9O/kNv2k7U=; b=PYwHskUA9vvBPtIZ5mqSRhFqEwHpkVVo9MSjENEPBW9tM7j+Ct8SMGScmzPrF8klWs m802f0gl9K6NJVrELfswley+xeX4gHYosn6Ys9nmKmyRdTbi3C590eaZEmB8LC44zpf8 UoPRRTD1bZAE7u1ZZgPeEvjXjCeKXMC/j9mYcm+hGnKHYtEGrU2PXi38jmWQo6ppRJ+u 268j5fkWpITw7O77pAulBRTZbS/PPwKTiYUaCDzRujKWzcId2qMuXoDAZM3xR9ss6BQa RfbuKuQCdZM275K3WF/r8oVuj2Qjicqn7F4Nqk9C4VD0262maUU+FLJrggwfXq3hHVE7 vlpg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=NCIOs3bWMPbQlzgpBqn2iparYJkE6e2ot9O/kNv2k7U=; b=HXNL0LHBr2aSds/ttUug7Nu62IcBgAiT4OoXWIlEQ6Hmk92WzZco4OmHqkbGgzOGyH NnvhOq3brHlj28LSQrjOWpuGI3hjx2m7ykF6ZdNqHAMAwt0BLhpk4sY9fgtRFK5VygpM o2pLEqoIJTmIGCciMZRNCnVi1HNr8VplUFTWnM8Lc1CCDOAwgFGpzUs+oRNkmIugbq24 W4Yf8/z/uiTFU4ZyLQfJa3Ay90QWbZc2UpqvmQHMhFTN9ImsHipzqPiVeVH/gZy17ngk 3Cy/ROutzsbvUQO6ykmJk2WhE0rIoWNdnWHDfMp+NkmppOgwRpGqkiJqKgBEXf+I9z9+ lkPQ== X-Gm-Message-State: AOAM530gBZ1zN4/rGMQ/5g3svj9P7OvFHPGdqxdXWKsueNc0eISdvwZ9 MAR+LmpoxwFlPZ96fuAIoj7HBZ5U2lzUJg== X-Google-Smtp-Source: ABdhPJxzd5PaMoBnAEbSHjRst8jx49c0Fa3cSSr5Xwj0BnTM/Cjt9L+UWhje+YSuwxH56+SuAnAKLg== X-Received: by 2002:a05:6000:44:: with SMTP id k4mr5905595wrx.76.1621332551983; Tue, 18 May 2021 03:09:11 -0700 (PDT) Received: from naush-laptop.pitowers.org ([2a00:1098:3142:14:34e4:187b:f2:ed28]) by smtp.gmail.com with ESMTPSA id o11sm6566682wrq.93.2021.05.18.03.09.11 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 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 Subject: [libcamera-devel] [PATCH 2/4] ipa: raspberrypi: Switch ipa/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: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Switch the ipa and cam_helper code to use RPiController::Duration for all time based variables. This improves code readability and avoids possible errors when converting between time bases. Signed-off-by: Naushir Patuck Reviewed-by: David Plowman --- src/ipa/raspberrypi/cam_helper.cpp | 19 +++--- src/ipa/raspberrypi/cam_helper.hpp | 10 +-- src/ipa/raspberrypi/controller/camera_mode.h | 5 +- src/ipa/raspberrypi/raspberrypi.cpp | 67 +++++++++++--------- 4 files changed, 56 insertions(+), 45 deletions(-) diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp index 09917f3cc079..e2b6c8eb8e03 100644 --- a/src/ipa/raspberrypi/cam_helper.cpp +++ b/src/ipa/raspberrypi/cam_helper.cpp @@ -61,20 +61,21 @@ void CamHelper::Process([[maybe_unused]] StatisticsPtr &stats, { } -uint32_t CamHelper::ExposureLines(double exposure_us) const +uint32_t CamHelper::ExposureLines(Duration exposure) const { assert(initialized_); - return exposure_us * 1000.0 / mode_.line_length; + return exposure / mode_.line_length; } -double CamHelper::Exposure(uint32_t exposure_lines) const +Duration CamHelper::Exposure(uint32_t exposure_lines) const { assert(initialized_); - return exposure_lines * mode_.line_length / 1000.0; + return exposure_lines * mode_.line_length; } -uint32_t CamHelper::GetVBlanking(double &exposure, double minFrameDuration, - double maxFrameDuration) const +uint32_t CamHelper::GetVBlanking(Duration &exposure, + Duration minFrameDuration, + Duration maxFrameDuration) const { uint32_t frameLengthMin, frameLengthMax, vblank; uint32_t exposureLines = ExposureLines(exposure); @@ -85,8 +86,8 @@ uint32_t CamHelper::GetVBlanking(double &exposure, double minFrameDuration, * minFrameDuration and maxFrameDuration are clamped by the caller * based on the limits for the active sensor mode. */ - frameLengthMin = 1e3 * minFrameDuration / mode_.line_length; - frameLengthMax = 1e3 * maxFrameDuration / mode_.line_length; + frameLengthMin = minFrameDuration / mode_.line_length; + frameLengthMax = maxFrameDuration / mode_.line_length; /* * Limit the exposure to the maximum frame duration requested, and @@ -182,7 +183,7 @@ void CamHelper::parseEmbeddedData(Span buffer, return; } - deviceStatus.shutter_speed = Exposure(exposureLines); + deviceStatus.shutter_speed = DurationValue(Exposure(exposureLines)); deviceStatus.analogue_gain = Gain(gainCode); LOG(IPARPI, Debug) << "Metadata updated - Exposure : " diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp index a52f3f0b583c..a91afaa59909 100644 --- a/src/ipa/raspberrypi/cam_helper.hpp +++ b/src/ipa/raspberrypi/cam_helper.hpp @@ -13,6 +13,7 @@ #include "camera_mode.h" #include "controller/controller.hpp" #include "controller/metadata.hpp" +#include "duration.hpp" #include "md_parser.hpp" #include "libcamera/internal/v4l2_videodevice.h" @@ -72,10 +73,11 @@ public: virtual void Prepare(libcamera::Span buffer, Metadata &metadata); virtual void Process(StatisticsPtr &stats, Metadata &metadata); - uint32_t ExposureLines(double exposure_us) const; - double Exposure(uint32_t exposure_lines) const; // in us - virtual uint32_t GetVBlanking(double &exposure_us, double minFrameDuration, - double maxFrameDuration) const; + uint32_t ExposureLines(Duration exposure) const; + Duration Exposure(uint32_t exposure_lines) const; + virtual uint32_t GetVBlanking(Duration &exposure, + Duration minFrameDuration, + Duration maxFrameDuration) const; virtual uint32_t GainCode(double gain) const = 0; virtual double Gain(uint32_t gain_code) const = 0; virtual void GetDelays(int &exposure_delay, int &gain_delay, diff --git a/src/ipa/raspberrypi/controller/camera_mode.h b/src/ipa/raspberrypi/controller/camera_mode.h index 256438c931d9..ab7cf2912a06 100644 --- a/src/ipa/raspberrypi/controller/camera_mode.h +++ b/src/ipa/raspberrypi/controller/camera_mode.h @@ -6,6 +6,7 @@ */ #pragma once +#include "duration.hpp" #include // Description of a "camera mode", holding enough information for control @@ -33,8 +34,8 @@ struct CameraMode { double scale_x, scale_y; // scaling of the noise compared to the native sensor mode double noise_factor; - // line time in nanoseconds - double line_length; + // line time + RPiController::Duration line_length; // any camera transform *not* reflected already in the camera tuning libcamera::Transform transform; // minimum and maximum fame lengths in units of lines diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 52d91db282ea..994fb7e057a9 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -45,6 +45,7 @@ #include "denoise_algorithm.hpp" #include "denoise_status.h" #include "dpc_status.h" +#include "duration.hpp" #include "focus_status.h" #include "geq_status.h" #include "lux_status.h" @@ -55,19 +56,24 @@ namespace libcamera { +using namespace std::literals::chrono_literals; +using RPiController::Duration; +using RPiController::DurationValue; +using RPiController::operator<<; + /* Configure the sensor with these values initially. */ constexpr double DefaultAnalogueGain = 1.0; -constexpr unsigned int DefaultExposureTime = 20000; -constexpr double defaultMinFrameDuration = 1e6 / 30.0; -constexpr double defaultMaxFrameDuration = 1e6 / 0.01; +constexpr Duration DefaultExposureTime = 20.0ms; +constexpr Duration defaultMinFrameDuration = 1.0s / 30.0; +constexpr Duration defaultMaxFrameDuration = 100.0s; /* - * Determine the minimum allowable inter-frame duration (in us) to run the - * controller algorithms. If the pipeline handler provider frames at a rate - * higher than this, we rate-limit the controller Prepare() and Process() calls - * to lower than or equal to this rate. + * Determine the minimum allowable inter-frame duration to run the controller + * algorithms. If the pipeline handler provider frames at a rate higher than this, + * we rate-limit the controller Prepare() and Process() calls to lower than or + * equal to this rate. */ -constexpr double controllerMinFrameDuration = 1e6 / 60.0; +constexpr Duration controllerMinFrameDuration = 1.0s / 60.0; LOG_DEFINE_CATEGORY(IPARPI) @@ -111,7 +117,8 @@ private: void reportMetadata(); void fillDeviceStatus(const ControlList &sensorControls); void processStats(unsigned int bufferId); - void applyFrameDurations(double minFrameDuration, double maxFrameDuration); + void applyFrameDurations(const Duration &minFrameDuration, + const Duration &maxFrameDuration); void applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls); void applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls); void applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls); @@ -167,9 +174,9 @@ private: /* Distinguish the first camera start from others. */ bool firstStart_; - /* Frame duration (1/fps) limits, given in microseconds. */ - double minFrameDuration_; - double maxFrameDuration_; + /* Frame duration (1/fps) limits. */ + Duration minFrameDuration_; + Duration maxFrameDuration_; }; int IPARPi::init(const IPASettings &settings, ipa::RPi::SensorConfig *sensorConfig) @@ -314,7 +321,7 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo) * Calculate the line length in nanoseconds as the ratio between * the line length in pixels and the pixel rate. */ - mode_.line_length = 1e9 * sensorInfo.lineLength / sensorInfo.pixelRate; + mode_.line_length = sensorInfo.lineLength * (1.0s / sensorInfo.pixelRate); /* * Set the frame length limits for the mode to ensure exposure and @@ -387,7 +394,7 @@ int IPARPi::configure(const CameraSensorInfo &sensorInfo, /* Supply initial values for gain and exposure. */ ControlList ctrls(sensorCtrls_); AgcStatus agcStatus; - agcStatus.shutter_time = DefaultExposureTime; + agcStatus.shutter_time = DurationValue(DefaultExposureTime); agcStatus.analogue_gain = DefaultAnalogueGain; applyAGC(&agcStatus, ctrls); @@ -862,7 +869,7 @@ void IPARPi::queueRequest(const ControlList &controls) case controls::FRAME_DURATIONS: { auto frameDurations = ctrl.second.get>(); - applyFrameDurations(frameDurations[0], frameDurations[1]); + applyFrameDurations(frameDurations[0] * 1.0us, frameDurations[1] * 1.0us); break; } @@ -937,9 +944,9 @@ void IPARPi::prepareISP(const ipa::RPi::ISPConfig &data) returnEmbeddedBuffer(data.embeddedBufferId); /* Allow a 10% margin on the comparison below. */ - constexpr double eps = controllerMinFrameDuration * 1e3 * 0.1; + Duration delta = (frameTimestamp - lastRunTimestamp_) * 1.0ns; if (lastRunTimestamp_ && frameCount_ > dropFrameCount_ && - frameTimestamp - lastRunTimestamp_ + eps < controllerMinFrameDuration * 1e3) { + delta < controllerMinFrameDuration * 0.9) { /* * Ensure we merge the previous frame's metadata with the current * frame. This will not overwrite exposure/gain values for the @@ -1012,7 +1019,7 @@ void IPARPi::fillDeviceStatus(const ControlList &sensorControls) int32_t exposureLines = sensorControls.get(V4L2_CID_EXPOSURE).get(); int32_t gainCode = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get(); - deviceStatus.shutter_speed = helper_->Exposure(exposureLines); + deviceStatus.shutter_speed = DurationValue(helper_->Exposure(exposureLines)); deviceStatus.analogue_gain = helper_->Gain(gainCode); LOG(IPARPI, Debug) << "Metadata - Exposure : " @@ -1057,17 +1064,18 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls) static_cast(awbStatus->gain_b * 1000)); } -void IPARPi::applyFrameDurations(double minFrameDuration, double maxFrameDuration) +void IPARPi::applyFrameDurations(const Duration &minFrameDuration, + const Duration &maxFrameDuration) { - const double minSensorFrameDuration = 1e-3 * mode_.min_frame_length * mode_.line_length; - const double maxSensorFrameDuration = 1e-3 * mode_.max_frame_length * mode_.line_length; + const Duration minSensorFrameDuration = mode_.min_frame_length * mode_.line_length; + const Duration maxSensorFrameDuration = mode_.max_frame_length * mode_.line_length; /* * This will only be applied once AGC recalculations occur. * The values may be clamped based on the sensor mode capabilities as well. */ - minFrameDuration_ = minFrameDuration ? minFrameDuration : defaultMaxFrameDuration; - maxFrameDuration_ = maxFrameDuration ? maxFrameDuration : defaultMinFrameDuration; + minFrameDuration_ = (minFrameDuration > 0.0s) ? minFrameDuration : defaultMaxFrameDuration; + maxFrameDuration_ = (maxFrameDuration > 0.0s) ? maxFrameDuration : defaultMinFrameDuration; minFrameDuration_ = std::clamp(minFrameDuration_, minSensorFrameDuration, maxSensorFrameDuration); maxFrameDuration_ = std::clamp(maxFrameDuration_, @@ -1076,20 +1084,20 @@ void IPARPi::applyFrameDurations(double minFrameDuration, double maxFrameDuratio /* Return the validated limits via metadata. */ libcameraMetadata_.set(controls::FrameDurations, - { static_cast(minFrameDuration_), - static_cast(maxFrameDuration_) }); + { static_cast(DurationValue(minFrameDuration_)), + static_cast(DurationValue(minFrameDuration_)) }); /* * Calculate the maximum exposure time possible for the AGC to use. * GetVBlanking() will update maxShutter with the largest exposure * value possible. */ - double maxShutter = std::numeric_limits::max(); + Duration maxShutter = Duration::max(); helper_->GetVBlanking(maxShutter, minFrameDuration_, maxFrameDuration_); RPiController::AgcAlgorithm *agc = dynamic_cast( controller_.GetAlgorithm("agc")); - agc->SetMaxShutter(maxShutter); + agc->SetMaxShutter(DurationValue(maxShutter)); } void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls) @@ -1097,9 +1105,8 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls) int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain); /* GetVBlanking might clip exposure time to the fps limits. */ - double exposure = agcStatus->shutter_time; - int32_t vblanking = helper_->GetVBlanking(exposure, minFrameDuration_, - maxFrameDuration_); + Duration exposure = agcStatus->shutter_time * 1.0us; + int32_t vblanking = helper_->GetVBlanking(exposure, minFrameDuration_, maxFrameDuration_); int32_t exposureLines = helper_->ExposureLines(exposure); LOG(IPARPI, Debug) << "Applying AGC Exposure: " << exposure