From patchwork Thu Oct 27 05:55:06 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Nicolas Dufresne via libcamera-devel X-Patchwork-Id: 17692 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 24ECEBD16B for ; Thu, 27 Oct 2022 05:55:28 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 6167062F7D; Thu, 27 Oct 2022 07:55:27 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1666850127; bh=PKFj24OOKVRuMKh0SSDIrf1PFnhIUyu7RiecwNokwWg=; 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=QGJT2zUstZkJ3o84+7tkBRMiWD4V4r70ce93I9k5qlUQqxSp+h8UxuZfMbrVJG8Q8 QIgIok1wsXgekhZ146/u793SgD61o5T8svlGDhr7J8osL4zHJsX232n3VarMaV4VJ8 oqstJjTqgZngMiBhAXlapbk4DeeIR3Gxc+/o9pWLaBKRsX8jRY1mCyoKem9Q2B8zRA qqg/X0+cUHakwS+5fJq0KFk7Ju0ZTpv3pfJUM+pjGi8MnueDcF9PYxvKcZDZQhY57b ir3TZslRl6EJHiyYqyEF2BfwuHm4LNZE59H7TSX09ZhgE5evPivFoYCIcMIrDz0LTf BcFJ41bwQZBTw== Received: from mail-oa1-x2c.google.com (mail-oa1-x2c.google.com [IPv6:2001:4860:4864:20::2c]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 93BCD62F69 for ; Thu, 27 Oct 2022 07:55:24 +0200 (CEST) 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="ijRGPi6M"; dkim-atps=neutral Received: by mail-oa1-x2c.google.com with SMTP id 586e51a60fabf-1322d768ba7so686318fac.5 for ; Wed, 26 Oct 2022 22:55:24 -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:reply-to:references :in-reply-to:message-id:date:subject:cc:to:from:from:to:cc:subject :date:message-id:reply-to; bh=Yq83/qBYhE/+rL6g+mTb35qh4EgpgtRGPtAyUBE/vcY=; b=ijRGPi6MDVJ8Mn1A5t7UEXJ905DiXc3iR70NNYAnpdW48NPA/SugRyCuAUdJ5jvCGA 8t81ywpv1iDmtjnMXn56zUsPCJLsXijsIty1Nv3yVnYY3embrwbTjh1CU7jMVlHLeKKD qWJDdg4SInmr+t5vqW4Tx0zJd+I7xzYbmZf45ebQ5v3uSg7ggBMxwkv94qLuYdMPOOd6 XtwTGjdlVAksqKXAJhvAwYdMAPWq9EraIQ7z0zlb3n5MT2XL+jLVpGkUKTH1yrwvPsLo 6RdxkNm+D8bqyOZsZ/Czb3/dRCf7+aLcHnaUzDDUUheYw2A1qEElcEW7p+Yp0bgORgEG Z+Nw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=content-transfer-encoding:mime-version:reply-to: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=Yq83/qBYhE/+rL6g+mTb35qh4EgpgtRGPtAyUBE/vcY=; b=CQ1DVALA2/R+/+nYcOvJXIZSD4V+rZNhbCzogvJFEkB49zTvtVb/nFHfWy1cuBg2OR WqVnRUCsC8LpZCDEPc8vR9yZKnbrNQH8ZVGTFUrIHxoOXSqfA8Wc06WQO1tkLOdcR6Vr d8axJy9kZQYlDoCC8QhIDn3vHEpC/z58FPYnvkb0LCLiJbBhtzSol6KwC8WfUzXwTRt/ AzdqCfQc9r0O6Ngu1SSUhgXUBDNuuR7C7yZO0O5AkWgKjqatUy+/yYqse46N9HyKO4+c EWSMvgzzDQ/6VfcgW0BcyCesiEE/dZjrtK5dtP2KNl4yM7meG58maO1ST6Z66y3+igfZ WkGA== X-Gm-Message-State: ACrzQf1Z4Naw25OpuctZxs/+xfUsxjpDV5VU49V2tfUSb2D9ZwHDDbqC GlfLqApyYE5TbuezVvCNicD74vF+Cm04hUXO X-Google-Smtp-Source: AMsMyM5hoqAOtkphOZxtcV9kVaWGwsRAulx87LY9wDxID86lBWoWafi2aQP8siE99L8fjYHXgIRgvQ== X-Received: by 2002:a05:6870:d186:b0:132:4315:1d28 with SMTP id a6-20020a056870d18600b0013243151d28mr4573633oac.147.1666850122668; Wed, 26 Oct 2022 22:55:22 -0700 (PDT) Received: from nroth-pc.attlocal.net ([2600:1700:20:20c0:6406:fc7a:e46d:1666]) by smtp.gmail.com with ESMTPSA id 9-20020a9d0c09000000b00661a05691fasm140021otr.79.2022.10.26.22.55.22 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 26 Oct 2022 22:55:22 -0700 (PDT) To: libcamera-devel@lists.libcamera.org Date: Thu, 27 Oct 2022 00:55:06 -0500 Message-Id: <20221027055515.321791-2-nicholas@rothemail.net> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221027055515.321791-1-nicholas@rothemail.net> References: <20221027055515.321791-1-nicholas@rothemail.net> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 01/10] 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: Nicolas Dufresne via libcamera-devel Reply-To: libcamera-devel@lists.libcamera.org Cc: nicholas@rothemail.net Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" From: Nicholas Roth 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 a1a3c38f..9b09ac94 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -100,7 +100,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; @@ -240,17 +241,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..65f76c3c 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..cfa5ed90 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..430642f0 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 04062a36..660f8327 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -74,7 +74,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: @@ -212,16 +213,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; }