[libcamera-devel] [PATCH 1/2] ipa: raspberrypi: Add sensitivity field to camera mode

[Naushir Patuck]

Hi David,

Thank you for your work.


On Thu, 27 May 2021 at 09:45, David Plowman <david.plowman at raspberrypi.com>
wrote:

> Initialise it to the usual value of 1, until such time as we have a
> mechanism that allows us to discover the correct value.
>
> The CamHelper class also gets a method to return this sensitivity
> value. This method is virtual so that it can be overridden for
> specific sensors. Once the correct value is obtainable elsewhere, this
> can be removed.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
>  src/ipa/raspberrypi/cam_helper.cpp           |  5 +++++
>  src/ipa/raspberrypi/cam_helper.hpp           |  3 +++
>  src/ipa/raspberrypi/controller/camera_mode.h |  2 ++
>  src/ipa/raspberrypi/raspberrypi.cpp          | 12 ++++++++++++
>  4 files changed, 22 insertions(+)
>
> diff --git a/src/ipa/raspberrypi/cam_helper.cpp
> b/src/ipa/raspberrypi/cam_helper.cpp
> index 09917f3c..71ea21ff 100644
> --- a/src/ipa/raspberrypi/cam_helper.cpp
> +++ b/src/ipa/raspberrypi/cam_helper.cpp
> @@ -128,6 +128,11 @@ bool CamHelper::SensorEmbeddedDataPresent() const
>         return false;
>  }
>
> +double CamHelper::Sensitivity() const
> +{
> +       return mode_.sensitivity;
> +}
> +
>  unsigned int CamHelper::HideFramesStartup() const
>  {
>         /*
> diff --git a/src/ipa/raspberrypi/cam_helper.hpp
> b/src/ipa/raspberrypi/cam_helper.hpp
> index a52f3f0b..14c4714c 100644
> --- a/src/ipa/raspberrypi/cam_helper.hpp
> +++ b/src/ipa/raspberrypi/cam_helper.hpp
> @@ -39,6 +39,8 @@ namespace RPiController {
>  //
>  // A method to query if the sensor outputs embedded data that can be
> parsed.
>  //
> +// A method to return the sensitivity of the current camera mode.
> +//
>  // A parser to parse the embedded data buffers provided by some sensors
> (for
>  // example, the imx219 does; the ov5647 doesn't). This allows us to know
> for
>  // sure the exposure and gain of the frame we're looking at. CamHelper
> @@ -81,6 +83,7 @@ public:
>         virtual void GetDelays(int &exposure_delay, int &gain_delay,
>                                int &vblank_delay) const;
>         virtual bool SensorEmbeddedDataPresent() const;
> +       virtual double Sensitivity() const;
>         virtual unsigned int HideFramesStartup() const;
>         virtual unsigned int HideFramesModeSwitch() const;
>         virtual unsigned int MistrustFramesStartup() const;
> diff --git a/src/ipa/raspberrypi/controller/camera_mode.h
> b/src/ipa/raspberrypi/controller/camera_mode.h
> index 256438c9..326f4f20 100644
> --- a/src/ipa/raspberrypi/controller/camera_mode.h
> +++ b/src/ipa/raspberrypi/controller/camera_mode.h
> @@ -39,6 +39,8 @@ struct CameraMode {
>         libcamera::Transform transform;
>         // minimum and maximum fame lengths in units of lines
>         uint32_t min_frame_length, max_frame_length;
> +       // sensitivity of this mode
> +       double sensitivity;
>  };
>
>  #ifdef __cplusplus
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> b/src/ipa/raspberrypi/raspberrypi.cpp
> index e5bb8159..da42d279 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -321,6 +321,12 @@ void IPARPi::setMode(const IPACameraSensorInfo
> &sensorInfo)
>          */
>         mode_.min_frame_length = sensorInfo.minFrameLength;
>         mode_.max_frame_length = sensorInfo.maxFrameLength;
> +
> +       /*
> +        * For now, initialise the mode sensitivity to the usual value of
> 1.
> +        * \todo Obtain the correct sensitivity number automatically.
> +        */
> +       mode_.sensitivity = 1.0;
>  }
>
>  int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,
> @@ -379,6 +385,12 @@ int IPARPi::configure(const IPACameraSensorInfo
> &sensorInfo,
>         /* Pass the camera mode to the CamHelper to setup algorithms. */
>         helper_->SetCameraMode(mode_);
>
> +       /*
> +        * For now, the CamHelper knows the sensitivity correctly.
> +        * \todo This can be removed once the sensitivity is initialised
> properly.
> +        */
> +       mode_.sensitivity = helper_->Sensitivity();
> +
>

I'm afraid I don't quite follow the logic here.  setMode sets the
sensitivity in mode_,
then this gets updated via the helper to the same value.  Obviously some of
this is
placeholder code right now, but could you explain how this may be expected
to work
eventually?

Another related question, do we need to involve the CamHelper at all?
Could we do
everything in the setMode function?

Thanks,
Naush


>         if (firstStart_) {
>                 /* Supply initial values for frame durations. */
>                 applyFrameDurations(defaultMinFrameDuration,
> defaultMaxFrameDuration);
>
-- 
> 2.20.1
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210602/eeba2045/attachment.htm>