[libcamera-devel] [PATCH v10 2/3] libcamera: raspberrypi: Add control of sensor vblanking
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Dec 20 06:52:13 CET 2020
Hi Naush,
Thank you for the patch.
On Fri, Dec 18, 2020 at 10:06:26AM +0000, Naushir Patuck wrote:
> Add support for setting V4L2_CID_VBLANK appropriately when setting
> V4L2_CID_EXPOSURE. This will allow adaptive framerates during
> viewfinder use cases (e.g. when the exposure time goes above 33ms, we
> can reduce the framerate to lower than 30fps).
>
> The minimum and maximum frame durations are provided via libcamera
> controls, and will prioritise exposure time limits over any AGC request.
>
> V4L2_CID_VBLANK is controlled through the staggered writer, just like
> the exposure and gain controls.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
> Tested-by: David Plowman <david.plowman at raspberrypi.com>
> Reviewed-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> include/libcamera/ipa/raspberrypi.h | 1 +
> src/ipa/raspberrypi/cam_helper.cpp | 35 ++++++++++++-
> src/ipa/raspberrypi/cam_helper.hpp | 15 +++++-
> src/ipa/raspberrypi/cam_helper_imx219.cpp | 13 ++++-
> src/ipa/raspberrypi/cam_helper_imx477.cpp | 11 +++-
> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 11 +++-
> src/ipa/raspberrypi/raspberrypi.cpp | 52 ++++++++++++++++---
> .../pipeline/raspberrypi/raspberrypi.cpp | 3 +-
> 8 files changed, 127 insertions(+), 14 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> index 01fe5abc..1de36039 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -65,6 +65,7 @@ static const ControlInfoMap Controls = {
> { &controls::Sharpness, ControlInfo(0.0f, 16.0f, 1.0f) },
> { &controls::ColourCorrectionMatrix, ControlInfo(-16.0f, 16.0f) },
> { &controls::ScalerCrop, ControlInfo(Rectangle{}, Rectangle(65535, 65535, 65535, 65535), Rectangle{}) },
> + { &controls::FrameDurations, ControlInfo(1000, 1000000000) },
> };
>
> } /* namespace RPi */
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> index 6efa0d7f..018c17b9 100644
> --- a/src/ipa/raspberrypi/cam_helper.cpp
> +++ b/src/ipa/raspberrypi/cam_helper.cpp
> @@ -34,8 +34,10 @@ CamHelper *CamHelper::Create(std::string const &cam_name)
> return nullptr;
> }
>
> -CamHelper::CamHelper(MdParser *parser)
> - : parser_(parser), initialized_(false)
> +CamHelper::CamHelper(MdParser *parser, unsigned int maxFrameLength,
> + unsigned int frameIntegrationDiff)
> + : parser_(parser), initialized_(false), maxFrameLength_(maxFrameLength),
> + frameIntegrationDiff_(frameIntegrationDiff)
> {
> }
>
> @@ -56,6 +58,35 @@ double CamHelper::Exposure(uint32_t exposure_lines) const
> return exposure_lines * mode_.line_length / 1000.0;
> }
>
> +uint32_t CamHelper::GetVBlanking(double &exposure, double minFrameDuration,
> + double maxFrameDuration) const
> +{
> + uint32_t frameLengthMin, frameLengthMax, vblank;
> + uint32_t exposureLines = ExposureLines(exposure);
> +
> + assert(initialized_);
> +
> + /*
> + * Clip frame length by the frame duration range and the maximum allowable
> + * value in the sensor, given by maxFrameLength_.
> + */
> + 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_);
> + /*
> + * Limit the exposure to the maximum frame duration requested, and
> + * re-calculate if it has been clipped.
> + */
> + exposureLines = std::min(frameLengthMax - frameIntegrationDiff_, exposureLines);
> + exposure = Exposure(exposureLines);
> +
> + /* Limit the vblank to the range allowed by the frame length limits. */
> + vblank = std::clamp(exposureLines + frameIntegrationDiff_,
> + frameLengthMin, frameLengthMax) - mode_.height;
> + return vblank;
> +}
> +
> void CamHelper::SetCameraMode(const CameraMode &mode)
> {
> mode_ = mode;
> diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp
> index 044c2866..b1739a57 100644
> --- a/src/ipa/raspberrypi/cam_helper.hpp
> +++ b/src/ipa/raspberrypi/cam_helper.hpp
> @@ -62,12 +62,15 @@ class CamHelper
> {
> public:
> static CamHelper *Create(std::string const &cam_name);
> - CamHelper(MdParser *parser);
> + CamHelper(MdParser *parser, unsigned int maxFrameLength,
> + unsigned int frameIntegrationDiff);
> virtual ~CamHelper();
> void SetCameraMode(const CameraMode &mode);
> MdParser &Parser() const { return *parser_; }
> 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;
> 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) const;
> @@ -76,10 +79,20 @@ public:
> virtual unsigned int HideFramesModeSwitch() const;
> virtual unsigned int MistrustFramesStartup() const;
> virtual unsigned int MistrustFramesModeSwitch() const;
> +
> protected:
> MdParser *parser_;
> CameraMode mode_;
> +
> +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.
> + */
> + unsigned int frameIntegrationDiff_;
> };
>
> // This is for registering camera helpers with the system, so that the
> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> index db8ab879..8688ec09 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> @@ -49,13 +49,22 @@ public:
> double Gain(uint32_t gain_code) const override;
> unsigned int MistrustFramesModeSwitch() const override;
> bool SensorEmbeddedDataPresent() const override;
> +
> +private:
> + /*
> + * Smallest difference between the frame length and integration time,
> + * 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())
> + : CamHelper(new MdParserImx219(), maxFrameLength, frameIntegrationDiff)
> #else
> - : CamHelper(new MdParserRPi())
> + : CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff)
> #endif
> {
> }
> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> index 0e896ac7..53961310 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> @@ -38,10 +38,19 @@ public:
> uint32_t GainCode(double gain) const override;
> double Gain(uint32_t gain_code) const override;
> bool SensorEmbeddedDataPresent() const override;
> +
> +private:
> + /*
> + * Smallest difference between the frame length and integration time,
> + * 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())
> + : CamHelper(new MdParserImx477(), maxFrameLength, frameIntegrationDiff)
> {
> }
>
> diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> index 0b841cd1..2bd8a754 100644
> --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> @@ -23,6 +23,15 @@ public:
> unsigned int HideFramesModeSwitch() const override;
> unsigned int MistrustFramesStartup() const override;
> unsigned int MistrustFramesModeSwitch() const override;
> +
> +private:
> + /*
> + * Smallest difference between the frame length and integration time,
> + * in units of lines.
> + */
> + static constexpr int frameIntegrationDiff = 4;
> + /* Largest possible frame length, in units of lines. */
> + static constexpr int maxFrameLength = 0xffff;
> };
>
> /*
> @@ -31,7 +40,7 @@ public:
> */
>
> CamHelperOv5647::CamHelperOv5647()
> - : CamHelper(new MdParserRPi())
> + : CamHelper(new MdParserRPi(), maxFrameLength, frameIntegrationDiff)
> {
> }
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index d087b07e..d309536b 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -58,6 +58,8 @@ namespace libcamera {
> /* 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;
>
> LOG_DEFINE_CATEGORY(IPARPI)
>
> @@ -150,6 +152,9 @@ private:
>
> /* Distinguish the first camera start from others. */
> bool firstStart_;
> +
> + /* Frame duration (1/fps) given in microseconds. */
> + double minFrameDuration_, maxFrameDuration_;
> };
>
> int IPARPi::init(const IPASettings &settings)
> @@ -332,7 +337,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> sensorMetadata = helper_->SensorEmbeddedDataPresent();
>
> result->data.push_back(gainDelay);
> - result->data.push_back(exposureDelay);
> + result->data.push_back(exposureDelay); /* For EXPOSURE ctrl */
> + result->data.push_back(exposureDelay); /* For VBLANK ctrl */
> result->data.push_back(sensorMetadata);
>
> result->operation |= RPi::IPA_CONFIG_STAGGERED_WRITE;
> @@ -377,6 +383,9 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> controller_.Initialise();
> controllerInit_ = true;
>
> + minFrameDuration_ = defaultMinFrameDuration;
> + maxFrameDuration_ = defaultMaxFrameDuration;
> +
> /* Supply initial values for gain and exposure. */
> ControlList ctrls(sensorCtrls_);
> AgcStatus agcStatus;
> @@ -524,7 +533,8 @@ bool IPARPi::validateSensorControls()
> {
> static const uint32_t ctrls[] = {
> V4L2_CID_ANALOGUE_GAIN,
> - V4L2_CID_EXPOSURE
> + V4L2_CID_EXPOSURE,
> + V4L2_CID_VBLANK
> };
>
> for (auto c : ctrls) {
> @@ -804,6 +814,24 @@ void IPARPi::queueRequest(const ControlList &controls)
> break;
> }
>
> + 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;
Should it be documented in patch 1/3 that setting the min or max to 0
will reset the limit to the default ?
Will the code behave correctly if min > max ?
> +
> + /*
> + * \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_) });
> + break;
> + }
> +
> default:
> LOG(IPARPI, Warning)
> << "Ctrl " << controls::controls.at(ctrl.first)->name()
> @@ -962,15 +990,27 @@ void IPARPi::applyAWB(const struct AwbStatus *awbStatus, ControlList &ctrls)
> void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
> {
> int32_t gainCode = helper_->GainCode(agcStatus->analogue_gain);
> - int32_t exposureLines = helper_->ExposureLines(agcStatus->shutter_time);
>
> - LOG(IPARPI, Debug) << "Applying AGC Exposure: " << agcStatus->shutter_time
> - << " (Shutter lines: " << exposureLines << ") Gain: "
> + /* GetVBlanking might clip exposure time to the fps limits. */
> + double exposure = agcStatus->shutter_time;
> + int32_t vblanking = helper_->GetVBlanking(exposure, minFrameDuration_,
> + maxFrameDuration_);
> + int32_t exposureLines = helper_->ExposureLines(exposure);
> +
> + LOG(IPARPI, Debug) << "Applying AGC Exposure: " << exposure
> + << " (Shutter lines: " << exposureLines << ", AGC requested "
> + << agcStatus->shutter_time << ") Gain: "
> << agcStatus->analogue_gain << " (Gain Code: "
> << gainCode << ")";
>
> - ctrls.set(V4L2_CID_ANALOGUE_GAIN, gainCode);
> + /*
> + * Due to the behavior of V4L2, the current value of VBLANK could clip the
> + * exposure time without us knowing. The next time though this function should
> + * clip exposure correctly.
> + */
This is something we should address in V4L2 (obviously not a blocker for
this series). If you could design it from scratch, what would be a good
API to configure the h/v blanking (possibly expressed as h/v active
instead of blanking) and exposure time ?
> + ctrls.set(V4L2_CID_VBLANK, vblanking);
> ctrls.set(V4L2_CID_EXPOSURE, exposureLines);
> + ctrls.set(V4L2_CID_ANALOGUE_GAIN, gainCode);
> }
>
> void IPARPi::applyDG(const struct AgcStatus *dgStatus, ControlList &ctrls)
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 7a5f5881..252cab64 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1233,7 +1233,8 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)
> if (!staggeredCtrl_) {
> staggeredCtrl_.init(unicam_[Unicam::Image].dev(),
> { { V4L2_CID_ANALOGUE_GAIN, result.data[resultIdx++] },
> - { V4L2_CID_EXPOSURE, result.data[resultIdx++] } });
> + { V4L2_CID_EXPOSURE, result.data[resultIdx++] },
> + { V4L2_CID_VBLANK, result.data[resultIdx++] } });
> sensorMetadata_ = result.data[resultIdx++];
> }
> }
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list