From patchwork Sun Oct 30 23:04:56 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicholas Roth X-Patchwork-Id: 17730 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 D293ABD16B for ; Sun, 30 Oct 2022 23:05:19 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 92DA463031; Mon, 31 Oct 2022 00:05:19 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1667171119; bh=1hsrjbrde8bBvjHQnOynnU9Y5CWmkye0DmPEbQouXzI=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=MTJTszaQJxAr1A5fSvAtM6iAxCiqrsWE0OsjPQ6O6GiPcxAycA/K/aS8z9jUbUprF i7gmn26kjuhMQKddRsNvwQe3O638Wz/88PHTodQs477EpygesN54dqQFVq69fVC802 XqEHNu1DrVzCJL225spih63D8ui9lyHx7nh6LPXvqJM+mQ/wgQ8tzBo73FDwxcnXG0 /4ef6jbVy5ssNajeidvlZ3Fkns7EnNCXzy5Atll95EVedmZDqKoccky+ja5g5GP4mA /PsoAb5f/txtKegSPXUpkSIkycX0+hFo3kObExNZV10LlL7bbanvbvzUewH2gpytDs oz3DAajzIfHXQ== Received: from mail-oi1-x231.google.com (mail-oi1-x231.google.com [IPv6:2607:f8b0:4864:20::231]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 812A462FFC for ; Mon, 31 Oct 2022 00:05:18 +0100 (CET) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=rothemail-net.20210112.gappssmtp.com header.i=@rothemail-net.20210112.gappssmtp.com header.b="U1WyLQu5"; dkim-atps=neutral Received: by mail-oi1-x231.google.com with SMTP id v81so2719568oie.5 for ; Sun, 30 Oct 2022 16:05:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rothemail-net.20210112.gappssmtp.com; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=T3FmDHIMAFBus0uMqBXIYh9g2aFEhrbzsIVmVSsen3U=; b=U1WyLQu5SJT9DZ6D98NiBDUCzSL0QAXaER9k9RuM/JY9IGYW4X5DHTMxyH3zs49U/P FCoH1yh/2q5qoHsmpBSQaJIkjaDrWmJNMnbKALLYixsTrrrTdKGcFTYdwzzQUxffnyv1 4ObTDVzSWu4WXKL69YY0x0XfEH8QZKiziShUx3id+1m+M18ifnhJgwLpov2qx0KjTSxK WpY5pHwSUpT5q6XIvoAhbmFjG1kk7o2rJue2E1sVMN6jNk3Qgp1ZALHi6QExDqyt6khN 9Fxc5pvwPtReMxorGlZ/mYT0puJEkt1Junjqks4egF+Qwy160FdIqoFHUou+NVZgfMnB JryA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=T3FmDHIMAFBus0uMqBXIYh9g2aFEhrbzsIVmVSsen3U=; b=YmWLJNkTwWF7gxbCW/LWmlZyy9q5W7LksTHyEkX0JcVOJDvzwK2h4vhJk9Pu2RJJ+f f3CqmYSoDCArRI8jEsqbNj3zCWdCtFEL37iJwTSzRZUrQLKYqEuFU1cy+8t7j3qGO2wE dAdOae6Pb7YY1SxbqDf+VzpnJN2sAvtsp7Hz+dulJVWblVcy0gZQHmK9nWmzjH1A/nU4 WWBFkLHtEkTRq0YjkwWX6OgfxXu3XY3LPG2lPua1eWUR/kg738HZHuHNGuWsOj1XFn4X Nptt7dab7RNfA55aTrwAaxyZNUpbT1+tvmlTcH3p0Gr/+qG8XYLXLCgT4nMV7Wdnvog4 rHgw== X-Gm-Message-State: ACrzQf1pNbRS47cOX7QKzUV3TcLYyNAwBqL8AgxZPMm8w4ZUJL639LkP f9THmu3mrZspq8Y9JMlqNf0IaDqz5o4dCA== X-Google-Smtp-Source: AMsMyM7m/UKBKAovTEBysw3jYM6bdmWH6zAFRJt8H5ctPfn0pr68N2Y1mCpg181fNvaJt5xpTUpkEQ== X-Received: by 2002:a05:6808:1304:b0:354:b62f:a027 with SMTP id y4-20020a056808130400b00354b62fa027mr5161730oiv.144.1667171116958; Sun, 30 Oct 2022 16:05:16 -0700 (PDT) Received: from nroth-pc.attlocal.net (104-5-61-214.lightspeed.austtx.sbcglobal.net. [104.5.61.214]) by smtp.gmail.com with ESMTPSA id u4-20020a056871008400b0013c8ae74a14sm2269403oaa.42.2022.10.30.16.05.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 30 Oct 2022 16:05:16 -0700 (PDT) To: libcamera-devel@lists.libcamera.org Date: Sun, 30 Oct 2022 18:04:56 -0500 Message-Id: <20221030230500.74842-2-nicholas@rothemail.net> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221030230500.74842-1-nicholas@rothemail.net> References: <20221030230500.74842-1-nicholas@rothemail.net> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v6 1/5] ipa: workaround libcxx duration limitation 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: , X-Patchwork-Original-From: Nicholas Roth via libcamera-devel From: Nicholas Roth Reply-To: Nicholas Roth Cc: Nicholas Roth Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" A bug in libcxx [0] used by clang version 11.0.2 is prevalent when building libcamera for Android SDK30. This has been fixed and integrated upstream with [1]. As a workaround, directly cast libcamera::utils::Duration objects to std::chrono::duration when dividing. Alternatives evaluated: Considered: Enable public inheritance of std::chrono::duration and override operator/ in the class. Outcome: Does not fix the original compiler error. Considered: Enable public inheritance of std::chrono::duration and override operator/ in the libcamera namespace. Outcome: new compiler error: ld.lld: error: duplicate symbol: libcamera::operator/ (libcamera::utils::Duration const&, libcamera::utils::Duration const&) Considered: Use private inheritance of std::chrono::duration and re-implement a pass-through version of each std::chrono::duration operator within libcamera::utils::Duration and use template metaprogramming to fix the division operator. Outcome: Testing shows that this would introduce substantial limitations, i.e. requring the Duration object to be on the LHS of any arithmetic operation with other numeric types. This also substantially increases implementation complexity. Considered: Extract double values from libcamera::utils::Duration objects and use those to divide. Outcome: This creates substantial readability and unit-safety issues. [0] https://github.com/llvm/llvm-project/issues/40475 [1] https://github.com/llvm/llvm-project/commit/efa6d803c624f9251d0ab7881122501bb9d27368 Bug: https://bugs.libcamera.org/show_bug.cgi?id=156 Signed-off-by: Nicholas Roth --- src/ipa/ipu3/algorithms/agc.cpp | 16 ++++++++++------ src/ipa/raspberrypi/cam_helper.cpp | 6 +++--- src/ipa/raspberrypi/cam_helper_imx296.cpp | 3 ++- src/ipa/raspberrypi/controller/rpi/agc.cpp | 12 ++++++++---- src/ipa/raspberrypi/controller/rpi/lux.cpp | 3 ++- src/ipa/rkisp1/algorithms/agc.cpp | 12 ++++++++---- 6 files changed, 33 insertions(+), 19 deletions(-) diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index b5309bdb..0f85a32e 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -101,7 +101,8 @@ int Agc::configure(IPAContext &context, /* Configure the default exposure and gain. */ activeState.agc.gain = std::max(minAnalogueGain_, kMinAnalogueGain); - activeState.agc.exposure = 10ms / configuration.sensor.lineDuration; + activeState.agc.exposure = 10ms / + std::chrono::duration(configuration.sensor.lineDuration); frameCount_ = 0; return 0; @@ -241,17 +242,20 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, * increase the gain. */ utils::Duration shutterTime = - std::clamp(exposureValue / minAnalogueGain_, - minShutterSpeed_, maxShutterSpeed_); - double stepGain = std::clamp(exposureValue / shutterTime, - minAnalogueGain_, maxAnalogueGain_); + std::clamp(std::chrono::duration(exposureValue) / + minAnalogueGain_, + minShutterSpeed_, maxShutterSpeed_); + double stepGain = std::clamp(std::chrono::duration(exposureValue) / + std::chrono::duration(shutterTime), + minAnalogueGain_, maxAnalogueGain_); LOG(IPU3Agc, Debug) << "Divided up shutter and gain are " << shutterTime << " and " << stepGain; IPAActiveState &activeState = context.activeState; /* Update the estimated exposure and gain. */ - activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration; + activeState.agc.exposure = std::chrono::duration(shutterTime) / + std::chrono::duration(configuration.sensor.lineDuration); activeState.agc.gain = stepGain; } diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp index d90ac1de..48a8a068 100644 --- a/src/ipa/raspberrypi/cam_helper.cpp +++ b/src/ipa/raspberrypi/cam_helper.cpp @@ -63,7 +63,7 @@ void CamHelper::process([[maybe_unused]] StatisticsPtr &stats, uint32_t CamHelper::exposureLines(const Duration exposure, const Duration lineLength) const { - return exposure / lineLength; + return std::chrono::duration(exposure) / std::chrono::duration(lineLength); } Duration CamHelper::exposure(uint32_t exposureLines, const Duration lineLength) const @@ -85,8 +85,8 @@ std::pair CamHelper::getBlanking(Duration &exposure, * frameLengthMax gets calculated on the smallest line length as we do * not want to extend that unless absolutely necessary. */ - frameLengthMin = minFrameDuration / mode_.minLineLength; - frameLengthMax = maxFrameDuration / mode_.minLineLength; + frameLengthMin = std::chrono::duration(minFrameDuration) / std::chrono::duration(mode_.minLineLength); + frameLengthMax = std::chrono::duration(maxFrameDuration) / std::chrono::duration(mode_.minLineLength); /* * Watch out for (exposureLines + frameIntegrationDiff_) overflowing a diff --git a/src/ipa/raspberrypi/cam_helper_imx296.cpp b/src/ipa/raspberrypi/cam_helper_imx296.cpp index ecb845e7..c5180de5 100644 --- a/src/ipa/raspberrypi/cam_helper_imx296.cpp +++ b/src/ipa/raspberrypi/cam_helper_imx296.cpp @@ -57,7 +57,8 @@ double CamHelperImx296::gain(uint32_t gainCode) const uint32_t CamHelperImx296::exposureLines(const Duration exposure, [[maybe_unused]] const Duration lineLength) const { - return std::max(minExposureLines, (exposure - 14.26us) / timePerLine); + return std::max(minExposureLines, std::chrono::duration(exposure - 14.26us) / + std::chrono::duration(timePerLine)); } Duration CamHelperImx296::exposure(uint32_t exposureLines, diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp index bd54a639..65baab99 100644 --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp @@ -418,7 +418,8 @@ void Agc::prepare(Metadata *imageMetadata) Duration actualExposure = deviceStatus.shutterSpeed * deviceStatus.analogueGain; if (actualExposure) { - status_.digitalGain = status_.totalExposureValue / actualExposure; + status_.digitalGain = std::chrono::duration(status_.totalExposureValue) / + std::chrono::duration(actualExposure); LOG(RPiAgc, Debug) << "Want total exposure " << status_.totalExposureValue; /* * Never ask for a gain < 1.0, and also impose @@ -823,7 +824,8 @@ void Agc::divideUpExposure() } if (status_.fixedAnalogueGain == 0.0) { if (exposureMode_->gain[stage] * shutterTime >= exposureValue) { - analogueGain = exposureValue / shutterTime; + analogueGain = std::chrono::duration(exposureValue) / + std::chrono::duration(shutterTime); break; } analogueGain = exposureMode_->gain[stage]; @@ -838,10 +840,12 @@ void Agc::divideUpExposure() */ if (!status_.fixedShutter && !status_.fixedAnalogueGain && status_.flickerPeriod) { - int flickerPeriods = shutterTime / status_.flickerPeriod; + int flickerPeriods = std::chrono::duration(shutterTime) / + std::chrono::duration(status_.flickerPeriod); if (flickerPeriods) { Duration newShutterTime = flickerPeriods * status_.flickerPeriod; - analogueGain *= shutterTime / newShutterTime; + analogueGain *= std::chrono::duration(shutterTime) / + std::chrono::duration(newShutterTime); /* * We should still not allow the ag to go over the * largest value in the exposure mode. Note that this diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp b/src/ipa/raspberrypi/controller/rpi/lux.cpp index 9759186a..410f6f44 100644 --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp @@ -94,7 +94,8 @@ void Lux::process(StatisticsPtr &stats, Metadata *imageMetadata) double currentY = sum / (double)num + .5; double gainRatio = referenceGain_ / currentGain; double shutterSpeedRatio = - referenceShutterSpeed_ / deviceStatus.shutterSpeed; + std::chrono::duration(referenceShutterSpeed_) / + std::chrono::duration(deviceStatus.shutterSpeed); double apertureRatio = referenceAperture_ / currentAperture; double yRatio = currentY * (65536 / numBins) / referenceY_; double estimatedLux = shutterSpeedRatio * gainRatio * diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp index b4fc7aed..e0192729 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -75,7 +75,8 @@ int Agc::configure(IPAContext &context, const IPACameraSensorInfo &configInfo) { /* Configure the default exposure and gain. */ context.activeState.agc.gain = std::max(context.configuration.agc.minAnalogueGain, kMinAnalogueGain); - context.activeState.agc.exposure = 10ms / context.configuration.sensor.lineDuration; + context.activeState.agc.exposure = 10ms / + std::chrono::duration(context.configuration.sensor.lineDuration); /* * According to the RkISP1 documentation: @@ -213,16 +214,19 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, * Push the shutter time up to the maximum first, and only then * increase the gain. */ - utils::Duration shutterTime = std::clamp(exposureValue / minAnalogueGain, + utils::Duration shutterTime = std::clamp(std::chrono::duration(exposureValue) / + minAnalogueGain, minShutterSpeed, maxShutterSpeed); - double stepGain = std::clamp(exposureValue / shutterTime, + double stepGain = std::clamp(std::chrono::duration(exposureValue) / + std::chrono::duration(shutterTime), minAnalogueGain, maxAnalogueGain); LOG(RkISP1Agc, Debug) << "Divided up shutter and gain are " << shutterTime << " and " << stepGain; /* Update the estimated exposure and gain. */ - activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration; + activeState.agc.exposure = std::chrono::duration(shutterTime) / + std::chrono::duration(configuration.sensor.lineDuration); activeState.agc.gain = stepGain; }