[libcamera-devel] [PATCH v2 02/10] ipa: raspberrypi: Add minimum and maximum line length fields to CameraMode

[Laurent Pinchart]

Hi Naush,

Thank you for the patch.

On Thu, Oct 06, 2022 at 02:17:36PM +0100, Naushir Patuck via libcamera-devel wrote:
> Add fields for minimum and maximum line length duration to the CameraMode
> structure. This replaces the existing lineLength field.
> 
> Any use of the existing lineLength field is replaced by the new minLineLength
> field, as logically we always want to use the fastest sensor readout by default.
> 
> As a drive-by cosmetic change, split all fields in the CameraMode structure into
> separate lines.
> 
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Tested-by: Dave Stevenson <dave.stevenson at raspberrypi.com>
> ---
>  src/ipa/raspberrypi/cam_helper.cpp           |  8 +++----
>  src/ipa/raspberrypi/controller/camera_mode.h | 23 +++++++++++++-------
>  src/ipa/raspberrypi/raspberrypi.cpp          | 13 ++++++-----
>  3 files changed, 26 insertions(+), 18 deletions(-)
> 
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> index cac8f39ee763..42251ba29682 100644
> --- a/src/ipa/raspberrypi/cam_helper.cpp
> +++ b/src/ipa/raspberrypi/cam_helper.cpp
> @@ -64,13 +64,13 @@ void CamHelper::process([[maybe_unused]] StatisticsPtr &stats,
>  uint32_t CamHelper::exposureLines(const Duration exposure) const
>  {
>  	assert(initialized_);
> -	return exposure / mode_.lineLength;
> +	return exposure / mode_.minLineLength;
>  }
>  
>  Duration CamHelper::exposure(uint32_t exposureLines) const
>  {
>  	assert(initialized_);
> -	return exposureLines * mode_.lineLength;
> +	return exposureLines * mode_.minLineLength;
>  }
>  
>  uint32_t CamHelper::getVBlanking(Duration &exposure,
> @@ -86,8 +86,8 @@ uint32_t CamHelper::getVBlanking(Duration &exposure,
>  	 * minFrameDuration and maxFrameDuration are clamped by the caller
>  	 * based on the limits for the active sensor mode.
>  	 */
> -	frameLengthMin = minFrameDuration / mode_.lineLength;
> -	frameLengthMax = maxFrameDuration / mode_.lineLength;
> +	frameLengthMin = minFrameDuration / mode_.minLineLength;
> +	frameLengthMax = maxFrameDuration / mode_.minLineLength;
>  
>  	/*
>  	 * Limit the exposure to the maximum frame duration requested, and
> diff --git a/src/ipa/raspberrypi/controller/camera_mode.h b/src/ipa/raspberrypi/controller/camera_mode.h
> index a6ccf8c1c600..6bc35b771946 100644
> --- a/src/ipa/raspberrypi/controller/camera_mode.h
> +++ b/src/ipa/raspberrypi/controller/camera_mode.h
> @@ -20,23 +20,30 @@ struct CameraMode {
>  	/* bit depth of the raw camera output */
>  	uint32_t bitdepth;
>  	/* size in pixels of frames in this mode */
> -	uint16_t width, height;
> +	uint16_t width;
> +	uint16_t height;
>  	/* size of full resolution uncropped frame ("sensor frame") */
> -	uint16_t sensorWidth, sensorHeight;
> +	uint16_t sensorWidth;
> +	uint16_t sensorHeight;
>  	/* binning factor (1 = no binning, 2 = 2-pixel binning etc.) */
> -	uint8_t binX, binY;
> +	uint8_t binX;
> +	uint8_t binY;
>  	/* location of top left pixel in the sensor frame */
> -	uint16_t cropX, cropY;
> +	uint16_t cropX;
> +	uint16_t cropY;
>  	/* scaling factor (so if uncropped, width*scaleX is sensorWidth) */
> -	double scaleX, scaleY;
> +	double scaleX;
> +	double scaleY;
>  	/* scaling of the noise compared to the native sensor mode */
>  	double noiseFactor;
> -	/* line time */
> -	libcamera::utils::Duration lineLength;
> +	/* minimum and maximum line time */
> +	libcamera::utils::Duration minLineLength;
> +	libcamera::utils::Duration maxLineLength;
>  	/* any camera transform *not* reflected already in the camera tuning */
>  	libcamera::Transform transform;
>  	/* minimum and maximum fame lengths in units of lines */

s/fame/frame/ as another drive-by fix.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

No need to resubmit for just this.

> -	uint32_t minFrameLength, maxFrameLength;
> +	uint32_t minFrameLength;
> +	uint32_t maxFrameLength;
>  	/* sensitivity of this mode */
>  	double sensitivity;
>  };
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 358a119da222..67326bcf4a14 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -314,7 +314,7 @@ void IPARPi::start(const ControlList &controls, StartConfig *startConfig)
>  	}
>  
>  	startConfig->dropFrameCount = dropFrameCount_;
> -	const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.lineLength;
> +	const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.minLineLength;
>  	startConfig->maxSensorFrameLengthMs = maxSensorFrameDuration.get<std::milli>();
>  
>  	firstStart_ = false;
> @@ -356,7 +356,8 @@ void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo)
>  	 * Calculate the line length as the ratio between the line length in
>  	 * pixels and the pixel rate.
>  	 */
> -	mode_.lineLength = sensorInfo.minLineLength * (1.0s / sensorInfo.pixelRate);
> +	mode_.minLineLength = sensorInfo.minLineLength * (1.0s / sensorInfo.pixelRate);
> +	mode_.maxLineLength = sensorInfo.maxLineLength * (1.0s / sensorInfo.pixelRate);
>  
>  	/*
>  	 * Set the frame length limits for the mode to ensure exposure and
> @@ -458,8 +459,8 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,
>  	 * based on the current sensor mode.
>  	 */
>  	ControlInfoMap::Map ctrlMap = ipaControls;
> -	const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.lineLength;
> -	const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.lineLength;
> +	const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.minLineLength;
> +	const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.minLineLength;
>  	ctrlMap[&controls::FrameDurationLimits] =
>  		ControlInfo(static_cast<int64_t>(minSensorFrameDuration.get<std::micro>()),
>  			    static_cast<int64_t>(maxSensorFrameDuration.get<std::micro>()));
> @@ -1150,8 +1151,8 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)
>  
>  void IPARPi::applyFrameDurations(Duration minFrameDuration, Duration maxFrameDuration)
>  {
> -	const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.lineLength;
> -	const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.lineLength;
> +	const Duration minSensorFrameDuration = mode_.minFrameLength * mode_.minLineLength;
> +	const Duration maxSensorFrameDuration = mode_.maxFrameLength * mode_.minLineLength;
>  
>  	/*
>  	 * This will only be applied once AGC recalculations occur.

-- 
Regards,

Laurent Pinchart