From patchwork Mon Oct 24 05:55:33 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: 17677 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 78E41BD16B for ; Mon, 24 Oct 2022 05:55:53 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 3F6A062EEC; Mon, 24 Oct 2022 07:55:53 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1666590953; bh=2DJHBIv3lJ1ncEjTiHiFNMOOJnyb+5sStiN4rc78Rzs=; 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=iQkXxcdkTcP1JOoiaDV6oPXCPVGCJxNCoga1gAaUXOmQYpnx/mPsqiuPAMYY7Isgl W1ZkJqVTA57c423Q+5Uj4sXhkLO6x51JRXG3RLG2vGF5rTpfmfIHEw1NKMipw81mXn LzsI7kiX4p+AOKmsXLOSWkrYbZ7V/5iidRRnvLAbTsipWF7om5jCs9i/3PLVXziGF4 9gRllde19RVJHWuaWb1lWTnfIUStHhT9FJOZBHUS6oqod17I/o6/quN2E2a+0+ulpK 8uusJaXlE25Xo1grX0kD/qPKgkrqrQzm3KwwHsWEniV3Zme2b9E1pu9z0oWZZoutCE DFZ4J/vXi6xmw== Received: from mail-ot1-x333.google.com (mail-ot1-x333.google.com [IPv6:2607:f8b0:4864:20::333]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C46F261F4C for ; Mon, 24 Oct 2022 07:55:51 +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="T7bvSu+s"; dkim-atps=neutral Received: by mail-ot1-x333.google.com with SMTP id br15-20020a056830390f00b0061c9d73b8bdso5361922otb.6 for ; Sun, 23 Oct 2022 22:55:51 -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=9rplnuUN0pnsIlI3cmDpVxuZ3BxsKKTvsZ2YHWy1u2E=; b=T7bvSu+sYZ+j/4XjWP00dW8fH131PWir0TDkanUfUb+kKuGDHAg2aNkOX5zK1EbYbh GEBj/LXehlWk/99YYApSaJQcZ1MEMnIPfYlnSZXPmxKOgW7UvBkDZLiI/cVzaPRj0DkX N8W186sqqT6Ogffv2930F0uo4ziztbKOk7DI3ah7OheXtkYEJCKjK4p5wE+1i1h/8ptV lAC5AZ6oqHH2oQ0ByktaYPf5Q0t0/z4o4ZvR3UpDy1XHG2uaiAGr2wfwpF2EK0O2Y0W9 vMrB66/PPZkPzsIZYal8sOj41o59ZHYoBOTA+EUiXDkXjVguLLyqlqwFbtiCgNhpLS+3 CuTQ== 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=9rplnuUN0pnsIlI3cmDpVxuZ3BxsKKTvsZ2YHWy1u2E=; b=vfv0+9/rj7phsgP5Ops8HsJrJY2jxTWY3uPJZfhWFCguQD6QOfTD/fYbqGx8nkWWzz ZnGN5rKtoectz3yRsj6ZuJwgGMzI67fkZWy4KPxJbEG3g9gmGb8Qpzy8nL5TVXaQKRKM M0MJIm+vX4fC7GKM9KPZzP84jlgh4N8hYE5zVpJZnzJ/rVmwR8bifnKo2bPFs7aWxYa/ qPLxSJMc2AoC3wO1npMIYJoR72dmOzh/WD0GVjWKxMQDkXTvIqC4owLuqPU0JVERVFHp VXxhIoYslNW8tk3wDVFF7zAFwoJKOAbcDNxQPCJprAxmQxIf95Fc2bo0ZJQ2iyxZW+Is SFAw== X-Gm-Message-State: ACrzQf2eHG0hFETdvMtOpqThye7AXtnJcVt0Hwx3jBHiqfu+06XvpSUW vB8la/gXf/LROF7dTFyXcWxJV1IsOPDcE+yh X-Google-Smtp-Source: AMsMyM6DZCVIkXT9K+HvkFjqmlj2Y43DoxOTWtah64pohKtUmlFq/TqiwoP/zi/r/ZzravC9NtsJKg== X-Received: by 2002:a05:6830:148e:b0:661:9422:f0e1 with SMTP id s14-20020a056830148e00b006619422f0e1mr16471640otq.205.1666590950213; Sun, 23 Oct 2022 22:55:50 -0700 (PDT) Received: from nroth-pc.attlocal.net ([2600:1700:20:20c0:7bc3:aed3:676f:10a0]) by smtp.gmail.com with ESMTPSA id x15-20020a9d628f000000b0066193df8edasm3980278otk.34.2022.10.23.22.55.49 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 23 Oct 2022 22:55:49 -0700 (PDT) To: libcamera-devel@lists.libcamera.org Date: Mon, 24 Oct 2022 00:55:33 -0500 Message-Id: <20221024055543.116040-2-nicholas@rothemail.net> X-Mailer: git-send-email 2.34.1 In-Reply-To: <20221024055543.116040-1-nicholas@rothemail.net> References: <20221024055543.116040-1-nicholas@rothemail.net> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 01/11] Fixes Bug 156, which breaks libcamera on Android < 12. 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 Signed-off-by: Nicholas Roth Signed-off-by: Signed-off-by: Nicholas Roth --- src/ipa/ipu3/algorithms/agc.cpp | 18 ++++++++++++++---- src/ipa/raspberrypi/cam_helper.cpp | 9 ++++++--- src/ipa/raspberrypi/cam_helper_imx296.cpp | 5 ++++- src/ipa/raspberrypi/controller/rpi/agc.cpp | 18 ++++++++++++++---- src/ipa/raspberrypi/controller/rpi/lux.cpp | 5 ++++- src/ipa/rkisp1/algorithms/agc.cpp | 22 ++++++++++++++++++---- 6 files changed, 60 insertions(+), 17 deletions(-) diff --git a/src/ipa/ipu3/algorithms/agc.cpp b/src/ipa/ipu3/algorithms/agc.cpp index a1a3c38f..80c551bb 100644 --- a/src/ipa/ipu3/algorithms/agc.cpp +++ b/src/ipa/ipu3/algorithms/agc.cpp @@ -100,7 +100,10 @@ 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; + /* TODO(Bug 156): Workaround for LLVM bug. */ + double ten_millis = utils::Duration(10ms).get(); + activeState.agc.exposure = ten_millis / + configuration.sensor.lineDuration.get(); frameCount_ = 0; return 0; @@ -238,11 +241,16 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, * * Push the shutter time up to the maximum first, and only then * increase the gain. + * + * TODO(Bug 156): Workaround for LLVM bug. */ + double exposureValueDouble = exposureValue.get(); + utils::Duration shutterTimeRaw(exposureValueDouble / minAnalogueGain_); utils::Duration shutterTime = - std::clamp(exposureValue / minAnalogueGain_, + std::clamp(shutterTimeRaw, minShutterSpeed_, maxShutterSpeed_); - double stepGain = std::clamp(exposureValue / shutterTime, + double shutterTimeDouble = shutterTime.get(); + double stepGain = std::clamp(exposureValueDouble / shutterTimeDouble, minAnalogueGain_, maxAnalogueGain_); LOG(IPU3Agc, Debug) << "Divided up shutter and gain are " << shutterTime << " and " @@ -250,7 +258,9 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, IPAActiveState &activeState = context.activeState; /* Update the estimated exposure and gain. */ - activeState.agc.exposure = shutterTime / configuration.sensor.lineDuration; + /* TODO(Bug 156): Workaround for LLVM bug. */ + double lineDurationDouble = configuration.sensor.lineDuration.get(); + activeState.agc.exposure = shutterTimeDouble / lineDurationDouble; activeState.agc.gain = stepGain; } diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp index d90ac1de..31a9a1ef 100644 --- a/src/ipa/raspberrypi/cam_helper.cpp +++ b/src/ipa/raspberrypi/cam_helper.cpp @@ -63,7 +63,8 @@ void CamHelper::process([[maybe_unused]] StatisticsPtr &stats, uint32_t CamHelper::exposureLines(const Duration exposure, const Duration lineLength) const { - return exposure / lineLength; + /* TODO(Bug 156): Workaround for LLVM bug. */ + return exposure.get() / lineLength.get(); } Duration CamHelper::exposure(uint32_t exposureLines, const Duration lineLength) const @@ -84,9 +85,11 @@ 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. + * + * TODO(Bug 156): Workaround for LLVM bug. */ - frameLengthMin = minFrameDuration / mode_.minLineLength; - frameLengthMax = maxFrameDuration / mode_.minLineLength; + frameLengthMin = minFrameDuration.get() / mode_.minLineLength.get(); + frameLengthMax = maxFrameDuration.get() / mode_.minLineLength.get(); /* * 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..e48f5cf2 100644 --- a/src/ipa/raspberrypi/cam_helper_imx296.cpp +++ b/src/ipa/raspberrypi/cam_helper_imx296.cpp @@ -57,7 +57,10 @@ 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); + /* TODO(Bug 156): Workaround for LLVM bug. */ + double exposureTime = Duration(exposure - 14.26us).get(); + double timePerLineNano = timePerLine.get(); + return std::max(minExposureLines, exposureTime / timePerLineNano); } 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..720ba788 100644 --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp @@ -418,7 +418,10 @@ void Agc::prepare(Metadata *imageMetadata) Duration actualExposure = deviceStatus.shutterSpeed * deviceStatus.analogueGain; if (actualExposure) { - status_.digitalGain = status_.totalExposureValue / actualExposure; + /* TODO(Bug 156): Workaround for LLVM bug. */ + double totalExposureDouble = status_.totalExposureValue.get(); + double actualExposureDouble = actualExposure.get(); + status_.digitalGain = totalExposureDouble / actualExposureDouble; LOG(RPiAgc, Debug) << "Want total exposure " << status_.totalExposureValue; /* * Never ask for a gain < 1.0, and also impose @@ -823,7 +826,10 @@ void Agc::divideUpExposure() } if (status_.fixedAnalogueGain == 0.0) { if (exposureMode_->gain[stage] * shutterTime >= exposureValue) { - analogueGain = exposureValue / shutterTime; + /* TODO(Bug 156): Workaround for LLVM bug. */ + double exposureDouble = exposureValue.get(); + double shutterTimeDouble = shutterTime.get(); + analogueGain = exposureDouble / shutterTimeDouble; break; } analogueGain = exposureMode_->gain[stage]; @@ -838,10 +844,14 @@ void Agc::divideUpExposure() */ if (!status_.fixedShutter && !status_.fixedAnalogueGain && status_.flickerPeriod) { - int flickerPeriods = shutterTime / status_.flickerPeriod; + /* TODO(Bug 156): Workaround for LLVM bug. */ + double shutterTimeDouble = shutterTime.get(); + double flickerPeriod = status_.flickerPeriod.get(); + int flickerPeriods = shutterTimeDouble / flickerPeriod; if (flickerPeriods) { Duration newShutterTime = flickerPeriods * status_.flickerPeriod; - analogueGain *= shutterTime / newShutterTime; + double newShutterTimeDouble = newShutterTime.get(); + analogueGain *= shutterTimeDouble / newShutterTimeDouble; /* * 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..49303409 100644 --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp @@ -93,8 +93,11 @@ void Lux::process(StatisticsPtr &stats, Metadata *imageMetadata) /* add .5 to reflect the mid-points of bins */ double currentY = sum / (double)num + .5; double gainRatio = referenceGain_ / currentGain; + /* TODO(Bug 156): Workaround for LLVM bug. */ + double referenceShutterSpeedDouble = referenceShutterSpeed_.get(); + double deviceShutterSpeed = deviceStatus.shutterSpeed.get(); double shutterSpeedRatio = - referenceShutterSpeed_ / deviceStatus.shutterSpeed; + referenceShutterSpeedDouble / deviceShutterSpeed; 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..3ea0b732 100644 --- a/src/ipa/rkisp1/algorithms/agc.cpp +++ b/src/ipa/rkisp1/algorithms/agc.cpp @@ -74,7 +74,13 @@ 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; + /* TODO(Bug 156): Explicit division of ticks (e.g., `x.get() / + * y.get()` as opposed to `x / y`) is a workaround for + * LLVM bug 41130 and should be reverted once we no longer target + * Android 11 / sdk30 since it compromises unit safety and readability. */ + constexpr libcamera::utils::Duration ten_millis(10ms); + long double exposure = ten_millis.get() / context.configuration.sensor.lineDuration.get(); + context.activeState.agc.exposure = uint32_t(exposure); /* * According to the RkISP1 documentation: @@ -211,17 +217,25 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext, /* * Push the shutter time up to the maximum first, and only then * increase the gain. + * + * TODO(Bug 156): Explicit division of ticks (e.g., `x.get() / + * y.get()` as opposed to `x / y`) is a workaround for + * LLVM bug 41130 and should be reverted once we no longer target + * Android 11 / sdk30 since it compromises unit safety and readability. */ - utils::Duration shutterTime = std::clamp(exposureValue / minAnalogueGain, + utils::Duration shutterTimeUnclamped(exposureValue.get() / minAnalogueGain); + utils::Duration shutterTime = std::clamp(shutterTimeUnclamped, minShutterSpeed, maxShutterSpeed); - double stepGain = std::clamp(exposureValue / shutterTime, + double stepGainUnclamped = exposureValue.get() / shutterTime.get(); + double stepGain = std::clamp(stepGainUnclamped, 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 = uint32_t(shutterTime.get() / + configuration.sensor.lineDuration.get()); activeState.agc.gain = stepGain; }