| Message ID | 20210611100703.2285-2-david.plowman@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi David, Thank you for your patch. On Fri, 11 Jun 2021 at 11:07, David Plowman <david.plowman@raspberrypi.com> wrote: > We use the CamHelper class to initialise it to the usual value of 1. > > The CamHelper's GetModeSensitivity method can be redefined to > implement a different behaviour for sensors that require it. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> > This is a bit awkward with the whole chicken-and-egg thing, but I cannot see what else to do. Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/ipa/raspberrypi/cam_helper.cpp | 11 +++++++++++ > src/ipa/raspberrypi/cam_helper.hpp | 3 +++ > src/ipa/raspberrypi/controller/camera_mode.h | 2 ++ > src/ipa/raspberrypi/raspberrypi.cpp | 6 ++++++ > 4 files changed, 22 insertions(+) > > diff --git a/src/ipa/raspberrypi/cam_helper.cpp > b/src/ipa/raspberrypi/cam_helper.cpp > index 062e94c4..d0104da7 100644 > --- a/src/ipa/raspberrypi/cam_helper.cpp > +++ b/src/ipa/raspberrypi/cam_helper.cpp > @@ -130,6 +130,17 @@ bool CamHelper::SensorEmbeddedDataPresent() const > return false; > } > > +double CamHelper::GetModeSensitivity([[maybe_unused]] const CameraMode > &mode) const > +{ > + /* > + * Most sensors have the same sensitivity in every mode, but this > + * method can be overridden for those that do not. Note that it is > + * called before mode_ is set, so it must return the sensitivity > + * of the mode that is passed in. > + */ > + return 1.0; > +} > + > unsigned int CamHelper::HideFramesStartup() const > { > /* > diff --git a/src/ipa/raspberrypi/cam_helper.hpp > b/src/ipa/raspberrypi/cam_helper.hpp > index f53f5c39..f524fe84 100644 > --- a/src/ipa/raspberrypi/cam_helper.hpp > +++ b/src/ipa/raspberrypi/cam_helper.hpp > @@ -40,6 +40,8 @@ namespace RPiController { > // > // A method to query if the sensor outputs embedded data that can be > parsed. > // > +// A method to return the sensitivity of a given 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 > @@ -83,6 +85,7 @@ public: > virtual void GetDelays(int &exposure_delay, int &gain_delay, > int &vblank_delay) const; > virtual bool SensorEmbeddedDataPresent() const; > + virtual double GetModeSensitivity(const CameraMode &mode) 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 2aa2335d..72c68dc6 100644 > --- a/src/ipa/raspberrypi/controller/camera_mode.h > +++ b/src/ipa/raspberrypi/controller/camera_mode.h > @@ -41,6 +41,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 1c1e802a..9ce0db08 100644 > --- a/src/ipa/raspberrypi/raspberrypi.cpp > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > @@ -324,6 +324,12 @@ void IPARPi::setMode(const IPACameraSensorInfo > &sensorInfo) > */ > mode_.min_frame_length = sensorInfo.minFrameLength; > mode_.max_frame_length = sensorInfo.maxFrameLength; > + > + /* > + * Some sensors may have different sensitivities in different > modes; > + * the CamHelper will know the correct value. > + */ > + mode_.sensitivity = helper_->GetModeSensitivity(mode_); > } > > int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, > -- > 2.20.1 > >
Hi David, On 11/06/2021 11:07, David Plowman wrote: > We use the CamHelper class to initialise it to the usual value of 1. > > The CamHelper's GetModeSensitivity method can be redefined to > implement a different behaviour for sensors that require it. > > Signed-off-by: David Plowman <david.plowman@raspberrypi.com> Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > src/ipa/raspberrypi/cam_helper.cpp | 11 +++++++++++ > src/ipa/raspberrypi/cam_helper.hpp | 3 +++ > src/ipa/raspberrypi/controller/camera_mode.h | 2 ++ > src/ipa/raspberrypi/raspberrypi.cpp | 6 ++++++ > 4 files changed, 22 insertions(+) > > diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp > index 062e94c4..d0104da7 100644 > --- a/src/ipa/raspberrypi/cam_helper.cpp > +++ b/src/ipa/raspberrypi/cam_helper.cpp > @@ -130,6 +130,17 @@ bool CamHelper::SensorEmbeddedDataPresent() const > return false; > } > > +double CamHelper::GetModeSensitivity([[maybe_unused]] const CameraMode &mode) const > +{ > + /* > + * Most sensors have the same sensitivity in every mode, but this > + * method can be overridden for those that do not. Note that it is > + * called before mode_ is set, so it must return the sensitivity > + * of the mode that is passed in. > + */ > + return 1.0; > +} > + > unsigned int CamHelper::HideFramesStartup() const > { > /* > diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp > index f53f5c39..f524fe84 100644 > --- a/src/ipa/raspberrypi/cam_helper.hpp > +++ b/src/ipa/raspberrypi/cam_helper.hpp > @@ -40,6 +40,8 @@ namespace RPiController { > // > // A method to query if the sensor outputs embedded data that can be parsed. > // > +// A method to return the sensitivity of a given 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 > @@ -83,6 +85,7 @@ public: > virtual void GetDelays(int &exposure_delay, int &gain_delay, > int &vblank_delay) const; > virtual bool SensorEmbeddedDataPresent() const; > + virtual double GetModeSensitivity(const CameraMode &mode) 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 2aa2335d..72c68dc6 100644 > --- a/src/ipa/raspberrypi/controller/camera_mode.h > +++ b/src/ipa/raspberrypi/controller/camera_mode.h > @@ -41,6 +41,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 1c1e802a..9ce0db08 100644 > --- a/src/ipa/raspberrypi/raspberrypi.cpp > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > @@ -324,6 +324,12 @@ void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo) > */ > mode_.min_frame_length = sensorInfo.minFrameLength; > mode_.max_frame_length = sensorInfo.maxFrameLength; > + > + /* > + * Some sensors may have different sensitivities in different modes; > + * the CamHelper will know the correct value. > + */ > + mode_.sensitivity = helper_->GetModeSensitivity(mode_); > } > > int IPARPi::configure(const IPACameraSensorInfo &sensorInfo, >
diff --git a/src/ipa/raspberrypi/cam_helper.cpp b/src/ipa/raspberrypi/cam_helper.cpp index 062e94c4..d0104da7 100644 --- a/src/ipa/raspberrypi/cam_helper.cpp +++ b/src/ipa/raspberrypi/cam_helper.cpp @@ -130,6 +130,17 @@ bool CamHelper::SensorEmbeddedDataPresent() const return false; } +double CamHelper::GetModeSensitivity([[maybe_unused]] const CameraMode &mode) const +{ + /* + * Most sensors have the same sensitivity in every mode, but this + * method can be overridden for those that do not. Note that it is + * called before mode_ is set, so it must return the sensitivity + * of the mode that is passed in. + */ + return 1.0; +} + unsigned int CamHelper::HideFramesStartup() const { /* diff --git a/src/ipa/raspberrypi/cam_helper.hpp b/src/ipa/raspberrypi/cam_helper.hpp index f53f5c39..f524fe84 100644 --- a/src/ipa/raspberrypi/cam_helper.hpp +++ b/src/ipa/raspberrypi/cam_helper.hpp @@ -40,6 +40,8 @@ namespace RPiController { // // A method to query if the sensor outputs embedded data that can be parsed. // +// A method to return the sensitivity of a given 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 @@ -83,6 +85,7 @@ public: virtual void GetDelays(int &exposure_delay, int &gain_delay, int &vblank_delay) const; virtual bool SensorEmbeddedDataPresent() const; + virtual double GetModeSensitivity(const CameraMode &mode) 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 2aa2335d..72c68dc6 100644 --- a/src/ipa/raspberrypi/controller/camera_mode.h +++ b/src/ipa/raspberrypi/controller/camera_mode.h @@ -41,6 +41,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 1c1e802a..9ce0db08 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -324,6 +324,12 @@ void IPARPi::setMode(const IPACameraSensorInfo &sensorInfo) */ mode_.min_frame_length = sensorInfo.minFrameLength; mode_.max_frame_length = sensorInfo.maxFrameLength; + + /* + * Some sensors may have different sensitivities in different modes; + * the CamHelper will know the correct value. + */ + mode_.sensitivity = helper_->GetModeSensitivity(mode_); } int IPARPi::configure(const IPACameraSensorInfo &sensorInfo,
We use the CamHelper class to initialise it to the usual value of 1. The CamHelper's GetModeSensitivity method can be redefined to implement a different behaviour for sensors that require it. Signed-off-by: David Plowman <david.plowman@raspberrypi.com> --- src/ipa/raspberrypi/cam_helper.cpp | 11 +++++++++++ src/ipa/raspberrypi/cam_helper.hpp | 3 +++ src/ipa/raspberrypi/controller/camera_mode.h | 2 ++ src/ipa/raspberrypi/raspberrypi.cpp | 6 ++++++ 4 files changed, 22 insertions(+)