[libcamera-devel] [PATCH v7 2/3] libcamera: raspberrypi: Add control of sensor vblanking

David Plowman david.plowman at raspberrypi.com
Fri Dec 11 14:57:49 CET 2020


Hi Naush

Thanks for the revised patch!

On Fri, 11 Dec 2020 at 13:35, Naushir Patuck <naush at raspberrypi.com> 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>
> ---
>  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           | 42 ++++++++++++++++---
>  .../pipeline/raspberrypi/raspberrypi.cpp      |  3 +-
>  8 files changed, 118 insertions(+), 13 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h
> index 01fe5abc..160ca681 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(1.0e3f, 1.0e9f) },

Just wondering whether it looks a bit strange to initialise explicitly
with floats now that they're int64_t, though I guess it makes no real
difference...

But other than this tiny thing:

Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
Tested-by: David Plowman <david.plowman at raspberrypi.com>

Thanks!
David

>  };
>
>  } /* namespace RPi */
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp
> index c8ac3232..7fd3e7f6 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 dc5d8275..7630c127 100644
> --- a/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> +++ b/src/ipa/raspberrypi/cam_helper_ov5647.cpp
> @@ -22,6 +22,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;
>  };
>
>  /*
> @@ -30,7 +39,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 60cfdc27..f6d2b543 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)
>
> @@ -145,6 +147,9 @@ private:
>         /* LS table allocation passed in from the pipeline handler. */
>         FileDescriptor lsTableHandle_;
>         void *lsTable_;
> +
> +       /* Frame duration (1/fps) given in microseconds. */
> +       double minFrameDuration_, maxFrameDuration_;
>  };
>
>  int IPARPi::init(const IPASettings &settings)
> @@ -266,7 +271,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;
> @@ -335,6 +341,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
>                 AgcStatus agcStatus;
>                 agcStatus.shutter_time = DefaultExposureTime;
>                 agcStatus.analogue_gain = DefaultAnalogueGain;
> +               minFrameDuration_ = defaultMinFrameDuration;
> +               maxFrameDuration_ = defaultMaxFrameDuration;
>                 applyAGC(&agcStatus, ctrls);
>
>                 result->controls.emplace_back(ctrls);
> @@ -712,6 +720,18 @@ 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;
> +                       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()
> @@ -882,7 +902,12 @@ 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);
> +
> +       /* 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);
>
>         if (unicamCtrls_.find(V4L2_CID_ANALOGUE_GAIN) == unicamCtrls_.end()) {
>                 LOG(IPARPI, Error) << "Can't find analogue gain control";
> @@ -894,13 +919,20 @@ void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
>                 return;
>         }
>
> -       LOG(IPARPI, Debug) << "Applying AGC Exposure: " << agcStatus->shutter_time
> -                          << " (Shutter lines: " << exposureLines << ") Gain: "
> +       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.
> +        */
> +       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 2ec1f6e7..13349f31 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -1221,7 +1221,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++];
>                 }
>         }
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel


More information about the libcamera-devel mailing list