[libcamera-devel] [PATCH v1 3/9] ipa: raspberrypi: Pass lineLength into the CamHelper API
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Oct 4 19:02:00 CEST 2022
Hi Naush,
Thank you for the patch.
On Mon, Oct 03, 2022 at 09:39:29AM +0100, Naushir Patuck via libcamera-devel wrote:
> Update CamHelper::exposureLines() and CamHelper::exposure() to take a
> line length duration parameter for use in the exposure calculations.
>
> For now, only use the minimum line length for all the calculations to match
> the existing IPA behavior.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> src/ipa/raspberrypi/cam_helper.cpp | 12 ++++++------
> src/ipa/raspberrypi/cam_helper.h | 6 ++++--
> src/ipa/raspberrypi/cam_helper_imx219.cpp | 3 ++-
> src/ipa/raspberrypi/cam_helper_imx296.cpp | 10 ++++++----
> src/ipa/raspberrypi/cam_helper_imx477.cpp | 7 ++++---
> src/ipa/raspberrypi/cam_helper_imx519.cpp | 7 ++++---
> src/ipa/raspberrypi/controller/camera_mode.h | 2 +-
> src/ipa/raspberrypi/raspberrypi.cpp | 8 ++++----
> 8 files changed, 31 insertions(+), 24 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> index 42251ba29682..fd3527b94501 100644
> --- a/src/ipa/raspberrypi/cam_helper.cpp
> +++ b/src/ipa/raspberrypi/cam_helper.cpp
> @@ -61,16 +61,16 @@ void CamHelper::process([[maybe_unused]] StatisticsPtr &stats,
> {
> }
>
> -uint32_t CamHelper::exposureLines(const Duration exposure) const
> +uint32_t CamHelper::exposureLines(const Duration exposure, const Duration lineLength) const
> {
> assert(initialized_);
Is the assert still needed ? Same below.
> - return exposure / mode_.minLineLength;
> + return exposure / lineLength;
> }
>
> -Duration CamHelper::exposure(uint32_t exposureLines) const
> +Duration CamHelper::exposure(uint32_t exposureLines, const Duration lineLength) const
> {
> assert(initialized_);
> - return exposureLines * mode_.minLineLength;
> + return exposureLines * lineLength;
> }
>
> uint32_t CamHelper::getVBlanking(Duration &exposure,
> @@ -78,7 +78,7 @@ uint32_t CamHelper::getVBlanking(Duration &exposure,
> Duration maxFrameDuration) const
> {
> uint32_t frameLengthMin, frameLengthMax, vblank;
> - uint32_t exposureLines = CamHelper::exposureLines(exposure);
> + uint32_t exposureLines = CamHelper::exposureLines(exposure, mode_.minLineLength);
>
> assert(initialized_);
>
> @@ -94,7 +94,7 @@ uint32_t CamHelper::getVBlanking(Duration &exposure,
> * re-calculate if it has been clipped.
> */
> exposureLines = std::min(frameLengthMax - frameIntegrationDiff_, exposureLines);
> - exposure = CamHelper::exposure(exposureLines);
> + exposure = CamHelper::exposure(exposureLines, mode_.minLineLength);
>
> /* Limit the vblank to the range allowed by the frame length limits. */
> vblank = std::clamp(exposureLines + frameIntegrationDiff_,
> diff --git a/src/ipa/raspberrypi/cam_helper.h b/src/ipa/raspberrypi/cam_helper.h
> index 70d62719da86..9b5e602689f3 100644
> --- a/src/ipa/raspberrypi/cam_helper.h
> +++ b/src/ipa/raspberrypi/cam_helper.h
> @@ -78,8 +78,10 @@ public:
> virtual void prepare(libcamera::Span<const uint8_t> buffer,
> Metadata &metadata);
> virtual void process(StatisticsPtr &stats, Metadata &metadata);
> - virtual uint32_t exposureLines(libcamera::utils::Duration exposure) const;
> - virtual libcamera::utils::Duration exposure(uint32_t exposureLines) const;
> + virtual uint32_t exposureLines(const libcamera::utils::Duration exposure,
> + const libcamera::utils::Duration lineLength) const;
> + virtual libcamera::utils::Duration exposure(uint32_t exposureLines,
> + const libcamera::utils::Duration lineLength) const;
> virtual uint32_t getVBlanking(libcamera::utils::Duration &exposure,
> libcamera::utils::Duration minFrameDuration,
> libcamera::utils::Duration maxFrameDuration) const;
> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> index 7ded07a213cd..98a3b31956ec 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> @@ -94,7 +94,8 @@ void CamHelperImx219::populateMetadata(const MdParser::RegisterMap ®isters,
> {
> DeviceStatus deviceStatus;
>
> - deviceStatus.shutterSpeed = exposure(registers.at(expHiReg) * 256 + registers.at(expLoReg));
> + deviceStatus.shutterSpeed = exposure(registers.at(expHiReg) * 256 + registers.at(expLoReg),
> + mode_.minLineLength);
> deviceStatus.analogueGain = gain(registers.at(gainReg));
> deviceStatus.frameLength = registers.at(frameLengthHiReg) * 256 + registers.at(frameLengthLoReg);
>
> diff --git a/src/ipa/raspberrypi/cam_helper_imx296.cpp b/src/ipa/raspberrypi/cam_helper_imx296.cpp
> index ab1d157aaf45..74361ffe7cc2 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx296.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx296.cpp
> @@ -21,8 +21,8 @@ public:
> CamHelperImx296();
> uint32_t gainCode(double gain) const override;
> double gain(uint32_t gainCode) const override;
> - uint32_t exposureLines(Duration exposure) const override;
> - Duration exposure(uint32_t exposureLines) const override;
> + uint32_t exposureLines(const Duration exposure, const Duration lineLength) const override;
> + Duration exposure(uint32_t exposureLines, const Duration lineLength) const override;
>
> private:
> static constexpr uint32_t maxGainCode = 239;
> @@ -51,12 +51,14 @@ double CamHelperImx296::gain(uint32_t gainCode) const
> return std::pow(10.0, gainCode / 200.0);
> }
>
> -uint32_t CamHelperImx296::exposureLines(Duration exposure) const
> +uint32_t CamHelperImx296::exposureLines(const Duration exposure,
> + [[maybe_unused]] const Duration lineLength) const
Does this mean that the IMX296 has no configurable horizontal blanking ?
> {
> return (exposure - 14.26us) / timePerLine;
> }
>
> -Duration CamHelperImx296::exposure(uint32_t exposureLines) const
> +Duration CamHelperImx296::exposure(uint32_t exposureLines,
> + [[maybe_unused]] const Duration lineLength) const
> {
> return exposureLines * timePerLine + 14.26us;
> }
> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> index aa306d6661c6..71529bdd3034 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> @@ -144,9 +144,9 @@ uint32_t CamHelperImx477::getVBlanking(Duration &exposure,
> if (shift) {
> /* Account for any rounding in the scaled frame length value. */
> frameLength <<= shift;
> - exposureLines = CamHelperImx477::exposureLines(exposure);
> + exposureLines = CamHelperImx477::exposureLines(exposure, mode_.minLineLength);
> exposureLines = std::min(exposureLines, frameLength - frameIntegrationDiff);
> - exposure = CamHelperImx477::exposure(exposureLines);
> + exposure = CamHelperImx477::exposure(exposureLines, mode_.minLineLength);
> }
>
> return frameLength - mode_.height;
> @@ -170,7 +170,8 @@ void CamHelperImx477::populateMetadata(const MdParser::RegisterMap ®isters,
> {
> DeviceStatus deviceStatus;
>
> - deviceStatus.shutterSpeed = exposure(registers.at(expHiReg) * 256 + registers.at(expLoReg));
> + deviceStatus.shutterSpeed = exposure(registers.at(expHiReg) * 256 + registers.at(expLoReg),
> + mode_.minLineLength);
> deviceStatus.analogueGain = gain(registers.at(gainHiReg) * 256 + registers.at(gainLoReg));
> deviceStatus.frameLength = registers.at(frameLengthHiReg) * 256 + registers.at(frameLengthLoReg);
> deviceStatus.sensorTemperature = std::clamp<int8_t>(registers.at(temperatureReg), -20, 80);
> diff --git a/src/ipa/raspberrypi/cam_helper_imx519.cpp b/src/ipa/raspberrypi/cam_helper_imx519.cpp
> index 54e104e7659a..2c120dad1680 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx519.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx519.cpp
> @@ -144,9 +144,9 @@ uint32_t CamHelperImx519::getVBlanking(Duration &exposure,
> if (shift) {
> /* Account for any rounding in the scaled frame length value. */
> frameLength <<= shift;
> - exposureLines = CamHelperImx519::exposureLines(exposure);
> + exposureLines = CamHelperImx519::exposureLines(exposure, mode_.minLineLength);
> exposureLines = std::min(exposureLines, frameLength - frameIntegrationDiff);
> - exposure = CamHelperImx519::exposure(exposureLines);
> + exposure = CamHelperImx519::exposure(exposureLines, mode_.minLineLength);
> }
>
> return frameLength - mode_.height;
> @@ -170,7 +170,8 @@ void CamHelperImx519::populateMetadata(const MdParser::RegisterMap ®isters,
> {
> DeviceStatus deviceStatus;
>
> - deviceStatus.shutterSpeed = exposure(registers.at(expHiReg) * 256 + registers.at(expLoReg));
> + deviceStatus.shutterSpeed = exposure(registers.at(expHiReg) * 256 + registers.at(expLoReg),
> + mode_.minLineLength);
> deviceStatus.analogueGain = gain(registers.at(gainHiReg) * 256 + registers.at(gainLoReg));
> deviceStatus.frameLength = registers.at(frameLengthHiReg) * 256 + registers.at(frameLengthLoReg);
>
> diff --git a/src/ipa/raspberrypi/controller/camera_mode.h b/src/ipa/raspberrypi/controller/camera_mode.h
> index b7e73aae4698..cda6ac4e200a 100644
> --- a/src/ipa/raspberrypi/controller/camera_mode.h
> +++ b/src/ipa/raspberrypi/controller/camera_mode.h
> @@ -31,7 +31,7 @@ struct CameraMode {
> double scaleX, scaleY;
> /* scaling of the noise compared to the native sensor mode */
> double noiseFactor;
> - /* line time */
> + /* minimum and maximum line times */
This belongs to the previous patch.
> libcamera::utils::Duration minLineLength, maxLineLength;
> /* any camera transform *not* reflected already in the camera tuning */
> libcamera::Transform transform;
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 67326bcf4a14..13807f4d47f7 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -477,7 +477,7 @@ int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,
> const uint32_t exposureMin = sensorCtrls_.at(V4L2_CID_EXPOSURE).min().get<int32_t>();
>
> ctrlMap[&controls::ExposureTime] =
> - ControlInfo(static_cast<int32_t>(helper_->exposure(exposureMin).get<std::micro>()),
> + ControlInfo(static_cast<int32_t>(helper_->exposure(exposureMin, mode_.minLineLength).get<std::micro>()),
> static_cast<int32_t>(maxShutter.get<std::micro>()));
>
> result->controlInfo = ControlInfoMap(std::move(ctrlMap), controls::controls);
> @@ -550,7 +550,7 @@ void IPARPi::reportMetadata()
> deviceStatus->shutterSpeed.get<std::micro>());
> libcameraMetadata_.set(controls::AnalogueGain, deviceStatus->analogueGain);
> libcameraMetadata_.set(controls::FrameDuration,
> - helper_->exposure(deviceStatus->frameLength).get<std::micro>());
> + helper_->exposure(deviceStatus->frameLength, mode_.minLineLength).get<std::micro>());
> if (deviceStatus->sensorTemperature)
> libcameraMetadata_.set(controls::SensorTemperature, *deviceStatus->sensorTemperature);
> }
> @@ -1106,7 +1106,7 @@ void IPARPi::fillDeviceStatus(const ControlList &sensorControls)
> int32_t gainCode = sensorControls.get(V4L2_CID_ANALOGUE_GAIN).get<int32_t>();
> int32_t vblank = sensorControls.get(V4L2_CID_VBLANK).get<int32_t>();
>
> - deviceStatus.shutterSpeed = helper_->exposure(exposureLines);
> + deviceStatus.shutterSpeed = helper_->exposure(exposureLines, mode_.minLineLength);
> deviceStatus.analogueGain = helper_->gain(gainCode);
> deviceStatus.frameLength = mode_.height + vblank;
>
> @@ -1198,7 +1198,7 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
> /* getVBlanking might clip exposure time to the fps limits. */
> Duration exposure = agcStatus->shutterTime;
> int32_t vblanking = helper_->getVBlanking(exposure, minFrameDuration_, maxFrameDuration_);
> - int32_t exposureLines = helper_->exposureLines(exposure);
> + int32_t exposureLines = helper_->exposureLines(exposure, mode_.minLineLength);
>
> LOG(IPARPI, Debug) << "Applying AGC Exposure: " << exposure
> << " (Shutter lines: " << exposureLines << ", AGC requested "
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list