ipa: raspberrypi: Limit the calculated vblank based on the sensor mode

The existing framerate/vblank calculations did not account for the
sensor mode limits. This commit extracts vblank limits from the sensor
v4l2 controls, and stores it in the camera modes structure.

Exposure and vblank value calculations now use values clamped to the
sensor mode limits. The libcamera::controls::FrameDurations metadata
return values now reflect the minimum and maximum after clamping.

Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo@jmondi.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

src/ipa/raspberrypi/cam_helper.cpp

@@ -34,9 +34,8 @@ CamHelper *CamHelper::Create(std::string const &cam_name)
 	return nullptr;
 }
 
-CamHelper::CamHelper(MdParser *parser, unsigned int maxFrameLength,
-		     unsigned int frameIntegrationDiff)
-	: parser_(parser), initialized_(false), maxFrameLength_(maxFrameLength),
+CamHelper::CamHelper(MdParser *parser, unsigned int frameIntegrationDiff)
+	: parser_(parser), initialized_(false),
 	  frameIntegrationDiff_(frameIntegrationDiff)
 {
 }
@@ -67,13 +66,12 @@ uint32_t CamHelper::GetVBlanking(double &exposure, double minFrameDuration,
 	assert(initialized_);
 
 	/*
-	 * Clamp frame length by the frame duration range and the maximum allowable
-	 * value in the sensor, given by maxFrameLength_.
+	 * minFrameDuration and maxFrameDuration are clamped by the caller
+	 * based on the limits for the active sensor mode.
 	 */
-	frameLengthMin = std::clamp<uint32_t>(1e3 * minFrameDuration / mode_.line_length,
-					      mode_.height, maxFrameLength_);
-	frameLengthMax = std::clamp<uint32_t>(1e3 * maxFrameDuration / mode_.line_length,
-					      mode_.height, maxFrameLength_);
+	frameLengthMin = 1e3 * minFrameDuration / mode_.line_length;
+	frameLengthMax = 1e3 * maxFrameDuration / mode_.line_length;
+
 	/*
 	 * Limit the exposure to the maximum frame duration requested, and
 	 * re-calculate if it has been clipped.

src/ipa/raspberrypi/cam_helper.hpp

@@ -62,8 +62,7 @@ class CamHelper
 {
 public:
 	static CamHelper *Create(std::string const &cam_name);
-	CamHelper(MdParser *parser, unsigned int maxFrameLength,
-		  unsigned int frameIntegrationDiff);
+	CamHelper(MdParser *parser, unsigned int frameIntegrationDiff);
 	virtual ~CamHelper();
 	void SetCameraMode(const CameraMode &mode);
 	MdParser &Parser() const { return *parser_; }
@@ -86,8 +85,6 @@ protected:
 
 private:
 	bool initialized_;
-	/* Largest possible frame length, in units of lines. */
-	unsigned int maxFrameLength_;
 	/*
 	 * Smallest difference between the frame length and integration time,
 	 * in units of lines.

src/ipa/raspberrypi/cam_helper_imx219.cpp

@@ -56,15 +56,13 @@ private:
 	 * in units of lines.
 	 */
 	static constexpr int frameIntegrationDiff = 4;
-	/* Largest possible frame length, in units of lines. */
-	static constexpr int maxFrameLength = 0xffff;
 };
 
 CamHelperImx219::CamHelperImx219()
 #if ENABLE_EMBEDDED_DATA
-	: CamHelper(new MdParserImx219(), maxFrameLength, frameIntegrationDiff)
+	: CamHelper(new MdParserImx219(), frameIntegrationDiff)
 #else
-	: CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff)
+	: CamHelper(new MdParserRPi(), frameIntegrationDiff)
 #endif
 {
 }

src/ipa/raspberrypi/cam_helper_imx477.cpp

@@ -45,12 +45,10 @@ private:
 	 * in units of lines.
 	 */
 	static constexpr int frameIntegrationDiff = 22;
-	/* Largest possible frame length, in units of lines. */
-	static constexpr int maxFrameLength = 0xffdc;
 };
 
 CamHelperImx477::CamHelperImx477()
-	: CamHelper(new MdParserImx477(), maxFrameLength, frameIntegrationDiff)
+	: CamHelper(new MdParserImx477(), frameIntegrationDiff)
 {
 }
 

src/ipa/raspberrypi/cam_helper_ov5647.cpp

@@ -30,8 +30,6 @@ private:
 	 * in units of lines.
 	 */
 	static constexpr int frameIntegrationDiff = 4;
-	/* Largest possible frame length, in units of lines. */
-	static constexpr int maxFrameLength = 0xffff;
 };
 
 /*
@@ -40,7 +38,7 @@ private:
  */
 
 CamHelperOv5647::CamHelperOv5647()
-	: CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff)
+	: CamHelper(new MdParserRPi(), frameIntegrationDiff)
 {
 }
 

src/ipa/raspberrypi/controller/camera_mode.h

@@ -37,6 +37,8 @@ struct CameraMode {
 	double line_length;
 	// any camera transform *not* reflected already in the camera tuning
 	libcamera::Transform transform;
+	// minimum and maximum fame lengths in units of lines
+	uint32_t min_frame_length, max_frame_length;
 };
 
 #ifdef __cplusplus

src/ipa/raspberrypi/raspberrypi.cpp

@@ -102,6 +102,7 @@ private:
 	void reportMetadata();
 	bool parseEmbeddedData(unsigned int bufferId, struct DeviceStatus &deviceStatus);
 	void processStats(unsigned int bufferId);
+	void applyFrameDurations(double minFrameDuration, double 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);
@@ -279,6 +280,13 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)
 	 * the line length in pixels and the pixel rate.
 	 */
 	mode_.line_length = 1e9 * sensorInfo.lineLength / sensorInfo.pixelRate;
+
+	/*
+	 * Set the frame length limits for the mode to ensure exposure and
+	 * framerate calculations are clipped appropriately.
+	 */
+	mode_.min_frame_length = sensorInfo.minFrameLength;
+	mode_.max_frame_length = sensorInfo.maxFrameLength;
 }
 
 void IPARPi::configure(const CameraSensorInfo &sensorInfo,
@@ -384,8 +392,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
 		controller_.Initialise();
 		controllerInit_ = true;
 
-		minFrameDuration_ = defaultMinFrameDuration;
-		maxFrameDuration_ = defaultMaxFrameDuration;
+		/* Supply initial values for frame durations. */
+		applyFrameDurations(defaultMinFrameDuration, defaultMaxFrameDuration);
 
 		/* Supply initial values for gain and exposure. */
 		ControlList ctrls(sensorCtrls_);
@@ -877,20 +885,7 @@ void IPARPi::queueRequest(const ControlList &controls)
 
 		case controls::FRAME_DURATIONS: {
 			auto frameDurations = ctrl.second.get<Span<const int64_t>>();
-
-			/* This will be applied once AGC recalculations occur. */
-			minFrameDuration_ = frameDurations[0] ? frameDurations[0] : defaultMinFrameDuration;
-			maxFrameDuration_ = frameDurations[1] ? frameDurations[1] : defaultMaxFrameDuration;
-			maxFrameDuration_ = std::max(maxFrameDuration_, minFrameDuration_);
-
-			/*
-			 * \todo The values returned in the metadata below must be
-			 * correctly clipped by what the sensor mode supports and
-			 * what the AGC exposure mode or manual shutter speed limits
-			 */
-			libcameraMetadata_.set(controls::FrameDurations,
-					       { static_cast<int64_t>(minFrameDuration_),
-						 static_cast<int64_t>(maxFrameDuration_) });
+			applyFrameDurations(frameDurations[0], frameDurations[1]);
 			break;
 		}
 
@@ -1049,6 +1044,29 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)
 		  static_cast<int32_t>(awbStatus->gain_b * 1000));
 }
 
+void IPARPi::applyFrameDurations(double minFrameDuration, double 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;
+
+	/*
+	 * 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_ = std::clamp(minFrameDuration_,
+				       minSensorFrameDuration, maxSensorFrameDuration);
+	maxFrameDuration_ = std::clamp(maxFrameDuration_,
+				       minSensorFrameDuration, maxSensorFrameDuration);
+	maxFrameDuration_ = std::max(maxFrameDuration_, minFrameDuration_);
+
+	/* Return the validated limits out though metadata. */
+	libcameraMetadata_.set(controls::FrameDurations,
+			       { static_cast<int64_t>(minFrameDuration_),
+				 static_cast<int64_t>(maxFrameDuration_) });
+}
+
 void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
 {
 	int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);