| Message ID | 20200511100150.5205-3-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
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@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";
Hi Laurent, On Tue, 12 May 2020 at 01:38, Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote: > > 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. Sure, I have no problem with that. > > 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@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. It was a coin toss between putting it in the base class vs derived class. I'll move it to the derived class as a virtual function so it can be overridden if needed. > > > + > > + 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; > > }; In patch v2, I will add a control for fps so that this can be removed as well. Regards, Naush > > > > 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
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); + + 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";
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. 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@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(-)