[libcamera-devel] [PATCH 2/2] libcamera: raspberrypi: Limit the maximum framerate to 30.0fps
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue May 12 02:38:04 CEST 2020
Hi Naush,
Thank you for the patch.
On Mon, May 11, 2020 at 11:01:50AM +0100, Naushir Patuck wrote:
> With the previous change to adapt framerate using VBLANK, the maximum
> framerate was limited by the sensor mode. This may not be entirely
> appropriate for what an application expects in the case where a mode
> can produce > 30.0fps.
This answers a question in my review of the previous patch :-) I
wouldn't be opposed to squashing the two patches together.
> Provide a mechanism to limit the maximum framerate. This is currently
> a const set to 30.0fps. In future, this value could be passed in via
> a Request or stream configuration.
>
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> src/ipa/raspberrypi/cam_helper.hpp | 2 +-
> src/ipa/raspberrypi/cam_helper_imx219.cpp | 18 ++++++++++++++----
> src/ipa/raspberrypi/cam_helper_imx477.cpp | 18 ++++++++++++++----
> src/ipa/raspberrypi/cam_helper_ov5647.cpp | 18 ++++++++++++++----
> src/ipa/raspberrypi/raspberrypi.cpp | 8 +++++++-
> 5 files changed, 50 insertions(+), 14 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp
> index 37281c78..4f7a3897 100644
> --- a/src/ipa/raspberrypi/cam_helper.hpp
> +++ b/src/ipa/raspberrypi/cam_helper.hpp
> @@ -74,7 +74,7 @@ public:
> 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) const = 0;
> + virtual uint32_t GetVBlanking(double exposure_us, double maxFps) const = 0;
> 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;
> diff --git a/src/ipa/raspberrypi/cam_helper_imx219.cpp b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> index d34a1f1f..51a18aca 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx219.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx219.cpp
> @@ -45,7 +45,7 @@ class CamHelperImx219 : public CamHelper
> {
> public:
> CamHelperImx219();
> - uint32_t GetVBlanking(double exposure) const override;
> + uint32_t GetVBlanking(double exposure, double maxFps) const override;
> uint32_t GainCode(double gain) const override;
> double Gain(uint32_t gain_code) const override;
> unsigned int MistrustFramesModeSwitch() const override;
> @@ -55,6 +55,8 @@ public:
> private:
> /* Smallest difference between the frame length and integration time. */
> static constexpr int frameIntegrationDiff = 4;
> + /* Largest possible frame length. */
> + static constexpr int maxFrameLength = 0xffff;
> };
>
> CamHelperImx219::CamHelperImx219()
> @@ -66,12 +68,20 @@ CamHelperImx219::CamHelperImx219()
> {
> }
>
> -uint32_t CamHelperImx219::GetVBlanking(double exposure) const
> +uint32_t CamHelperImx219::GetVBlanking(double exposure, double maxFps) const
> {
> + uint32_t frameLengthMin, vblank;
> uint32_t exposureLines = ExposureLines(exposure);
>
> - return std::max<uint32_t>(mode_.height, exposureLines) -
> - mode_.height + frameIntegrationDiff;
> + vblank = std::max<uint32_t>(mode_.height, exposureLines) -
> + mode_.height + frameIntegrationDiff;
> +
> + /* Limit the vblank to the maximum framerate requested. */
> + frameLengthMin = std::min<uint32_t>(1e9 / (mode_.line_length * maxFps),
> + maxFrameLength);
> + vblank = std::max(vblank, frameLengthMin - mode_.height);
> +
> + return vblank;
> }
>
> uint32_t CamHelperImx219::GainCode(double gain) const
> diff --git a/src/ipa/raspberrypi/cam_helper_imx477.cpp b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> index 242d9689..ff815f9c 100644
> --- a/src/ipa/raspberrypi/cam_helper_imx477.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_imx477.cpp
> @@ -35,7 +35,7 @@ class CamHelperImx477 : public CamHelper
> {
> public:
> CamHelperImx477();
> - uint32_t GetVBlanking(double exposure) const override;
> + uint32_t GetVBlanking(double exposure, double maxFps) const override;
> uint32_t GainCode(double gain) const override;
> double Gain(uint32_t gain_code) const override;
> bool SensorEmbeddedDataPresent() const override;
> @@ -44,6 +44,8 @@ public:
> private:
> /* Smallest difference between the frame length and integration time. */
> static constexpr int frameIntegrationDiff = 22;
> + /* Largest possible frame length. */
> + static constexpr int maxFrameLength = 0xffdc;
> };
>
> CamHelperImx477::CamHelperImx477()
> @@ -51,12 +53,20 @@ CamHelperImx477::CamHelperImx477()
> {
> }
>
> -uint32_t CamHelperImx477::GetVBlanking(double exposure) const
> +uint32_t CamHelperImx477::GetVBlanking(double exposure, double maxFps) const
> {
> + uint32_t frameLengthMin, vblank;
> uint32_t exposureLines = ExposureLines(exposure);
>
> - return std::max<uint32_t>(mode_.height, exposureLines) -
> - mode_.height + frameIntegrationDiff;
> + vblank = std::max<uint32_t>(mode_.height, exposureLines) -
> + mode_.height + frameIntegrationDiff;
> +
> + /* Limit the vblank to the maximum framerate requested. */
> + frameLengthMin = std::min<uint32_t>(1e9 / (mode_.line_length * maxFps),
> + maxFrameLength);
> + vblank = std::max(vblank, frameLengthMin - mode_.height);
> +
> + return vblank;
> }
>
> uint32_t CamHelperImx477::GainCode(double gain) const
> diff --git a/src/ipa/raspberrypi/cam_helper_ov5647.cpp b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> index ff37779c..53ac3c3b 100644
> --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> @@ -16,7 +16,7 @@ class CamHelperOv5647 : public CamHelper
> {
> public:
> CamHelperOv5647();
> - uint32_t GetVBlanking(double exposure) const override;
> + uint32_t GetVBlanking(double exposure, double maxFps) const override;
> uint32_t GainCode(double gain) const override;
> double Gain(uint32_t gain_code) const override;
> void GetDelays(int &exposure_delay, int &gain_delay) const override;
> @@ -27,6 +27,8 @@ public:
> private:
> /* Smallest difference between the frame length and integration time. */
> static constexpr int frameIntegrationDiff = 4;
> + /* Largest possible frame length. */
> + static constexpr int maxFrameLength = 0xffff;
> };
>
> /*
> @@ -39,12 +41,20 @@ CamHelperOv5647::CamHelperOv5647()
> {
> }
>
> -uint32_t CamHelperOv5647::GetVBlanking(double exposure) const
> +uint32_t CamHelperOv5647::GetVBlanking(double exposure, double maxFps) const
> {
> + uint32_t frameLengthMin, vblank;
> uint32_t exposureLines = ExposureLines(exposure);
>
> - return std::max<uint32_t>(mode_.height, exposureLines) -
> - mode_.height + frameIntegrationDiff;
> + vblank = std::max<uint32_t>(mode_.height, exposureLines) -
> + mode_.height + frameIntegrationDiff;
> +
> + /* Limit the vblank to the maximum framerate requested. */
> + frameLengthMin = std::min<uint32_t>(1e9 / (mode_.line_length * maxFps),
> + maxFrameLength);
> + vblank = std::max(vblank, frameLengthMin - mode_.height);
The implementations of this functions are growing and are still nearly
identical. Unless there's a good reason to keep them separate, I would
try to only expose frameIntegrationDiff from the derived classes and
GetVBlanking() in the base class.
> +
> + return vblank;
> }
>
> uint32_t CamHelperOv5647::GainCode(double gain) const
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 104727ea..3a2cc16e 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -136,6 +136,12 @@ private:
> /* LS table allocation passed in from the pipeline handler. */
> uint32_t lsTableHandle_;
> void *lsTable_;
> +
> + /*
> + * Have some defaults here, but eventually it should come from the
> + * application via a Request, or perhaps stream configuration.
> + */
> + static constexpr double fpsMax = 30.0;
> };
>
> int IPARPi::init(const IPASettings &settings)
> @@ -795,7 +801,7 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus)
>
> int32_t gain_code = helper_->GainCode(agcStatus->analogue_gain);
> int32_t exposure_lines = helper_->ExposureLines(agcStatus->shutter_time);
> - int32_t vblanking = helper_->GetVBlanking(agcStatus->shutter_time);
> + int32_t vblanking = helper_->GetVBlanking(agcStatus->shutter_time, fpsMax);
>
> if (unicam_ctrls_.find(V4L2_CID_ANALOGUE_GAIN) == unicam_ctrls_.end()) {
> LOG(IPARPI, Error) << "Can't find analogue gain control";
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list