[libcamera-devel] [PATCH v3 2/4] ipa: raspberrypi: Switch ipa and cam_helper to use utils::Duration

Naushir Patuck naush at raspberrypi.com
Fri May 21 10:05:28 CEST 2021


Switch the ipa and cam_helper code to use libcamera::utils::Duration for
all time based variables. This improves code readability and avoids
possible errors when converting between time bases.

Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
---
 src/ipa/raspberrypi/cam_helper.cpp           | 19 +++---
 src/ipa/raspberrypi/cam_helper.hpp           | 12 ++--
 src/ipa/raspberrypi/controller/camera_mode.h |  6 +-
 src/ipa/raspberrypi/raspberrypi.cpp          | 65 +++++++++++---------
 4 files changed, 57 insertions(+), 45 deletions(-)

diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
index 09917f3cc079..feb02d1806b2 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(const 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,
+				 const Duration &minFrameDuration,
+				 const 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<const uint8_t> buffer,
 		return;
 	}
 
-	deviceStatus.shutter_speed = Exposure(exposureLines);
+	deviceStatus.shutter_speed = Exposure(exposureLines).get<std::micro>();
 	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..71ee15463a71 100644
--- a/src/ipa/raspberrypi/cam_helper.hpp
+++ b/src/ipa/raspberrypi/cam_helper.hpp
@@ -15,10 +15,13 @@
 #include "controller/metadata.hpp"
 #include "md_parser.hpp"
 
+#include "libcamera/internal/utils.h"
 #include "libcamera/internal/v4l2_videodevice.h"
 
 namespace RPiController {
 
+using libcamera::utils::Duration;
+
 // The CamHelper class provides a number of facilities that anyone trying
 // to drive a camera will need to know, but which are not provided by the
 // standard driver framework. Specifically, it provides:
@@ -72,10 +75,11 @@ public:
 	virtual void Prepare(libcamera::Span<const uint8_t> 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(const Duration &exposure) const;
+	Duration Exposure(uint32_t exposure_lines) const;
+	virtual uint32_t GetVBlanking(Duration &exposure,
+				      const Duration &minFrameDuration,
+				      const 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..2aa2335dcf90 100644
--- a/src/ipa/raspberrypi/controller/camera_mode.h
+++ b/src/ipa/raspberrypi/controller/camera_mode.h
@@ -8,6 +8,8 @@
 
 #include <libcamera/transform.h>
 
+#include "libcamera/internal/utils.h"
+
 // Description of a "camera mode", holding enough information for control
 // algorithms to adapt their behaviour to the different modes of the camera,
 // including binning, scaling, cropping etc.
@@ -33,8 +35,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
+	libcamera::utils::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..e5d4c92a8e01 100644
--- a/src/ipa/raspberrypi/raspberrypi.cpp
+++ b/src/ipa/raspberrypi/raspberrypi.cpp
@@ -55,19 +55,23 @@
 
 namespace libcamera {
 
+using namespace std::literals::chrono_literals;
+using utils::Duration;
+using utils::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 +115,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 +172,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)
@@ -311,10 +316,10 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
 	mode_.noise_factor = sqrt(mode_.bin_x * mode_.bin_y);
 
 	/*
-	 * Calculate the line length in nanoseconds as the ratio between
-	 * the line length in pixels and the pixel rate.
+	 * Calculate the line length 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 +392,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 = DefaultExposureTime.get<std::micro>();
 		agcStatus.analogue_gain = DefaultAnalogueGain;
 		applyAGC(&agcStatus, ctrls);
 
@@ -862,7 +867,7 @@ void IPARPi::queueRequest(const ControlList &controls)
 
 		case controls::FRAME_DURATIONS: {
 			auto frameDurations = ctrl.second.get<Span<const int64_t>>();
-			applyFrameDurations(frameDurations[0], frameDurations[1]);
+			applyFrameDurations(frameDurations[0] * 1.0us, frameDurations[1] * 1.0us);
 			break;
 		}
 
@@ -937,9 +942,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 +1017,7 @@ void IPARPi::fillDeviceStatus(const ControlList &sensorControls)
 	int32_t exposureLines = sensorControls.get(V4L2_CID_EXPOSURE).get<int32_t>();
 	int32_t gainCode = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();
 
-	deviceStatus.shutter_speed = helper_->Exposure(exposureLines);
+	deviceStatus.shutter_speed = helper_->Exposure(exposureLines).get<std::micro>();
 	deviceStatus.analogue_gain = helper_->Gain(gainCode);
 
 	LOG(IPARPI, Debug) << "Metadata - Exposure : "
@@ -1057,10 +1062,11 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)
 		  static_cast<int32_t>(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.
@@ -1076,20 +1082,20 @@ void IPARPi::applyFrameDurations(double minFrameDuration, double maxFrameDuratio
 
 	/* Return the validated limits via metadata. */
 	libcameraMetadata_.set(controls::FrameDurations,
-			       { static_cast<int64_t>(minFrameDuration_),
-				 static_cast<int64_t>(maxFrameDuration_) });
+			       { static_cast<int64_t>(minFrameDuration_.get<std::micro>()),
+				 static_cast<int64_t>(maxFrameDuration_.get<std::micro>()) });
 
 	/*
 	 * 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<double>::max();
+	Duration maxShutter = Duration::max();
 	helper_->GetVBlanking(maxShutter, minFrameDuration_, maxFrameDuration_);
 
 	RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(
 		controller_.GetAlgorithm("agc"));
-	agc->SetMaxShutter(maxShutter);
+	agc->SetMaxShutter(maxShutter.get<std::micro>());
 }
 
 void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
@@ -1097,9 +1103,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
-- 
2.25.1



More information about the libcamera-devel mailing list