| Message ID | 20210525114241.906280-4-naush@raspberrypi.com |
|---|---|
| State | Accepted |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, Thank you for the patch. On Tue, May 25, 2021 at 12:42:40PM +0100, Naushir Patuck wrote: > Switch the AgcAlgorithm API functions to use utils::Duration for all > time based variables. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > Reviewed-by: David Plowman <david.plowman@raspberrypi.com> > Reviewed-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/ipa/raspberrypi/controller/agc_algorithm.hpp | 7 ++++--- > src/ipa/raspberrypi/controller/rpi/agc.cpp | 13 +++++++------ > src/ipa/raspberrypi/controller/rpi/agc.hpp | 6 +++--- > src/ipa/raspberrypi/raspberrypi.cpp | 6 +++--- > 4 files changed, 17 insertions(+), 15 deletions(-) > > diff --git a/src/ipa/raspberrypi/controller/agc_algorithm.hpp b/src/ipa/raspberrypi/controller/agc_algorithm.hpp > index f97deb0fca59..579ee2c91fa8 100644 > --- a/src/ipa/raspberrypi/controller/agc_algorithm.hpp > +++ b/src/ipa/raspberrypi/controller/agc_algorithm.hpp > @@ -6,6 +6,7 @@ > */ > #pragma once > > +#include "libcamera/internal/utils.h" > #include "algorithm.hpp" > > namespace RPiController { > @@ -17,9 +18,9 @@ public: > // An AGC algorithm must provide the following: > virtual unsigned int GetConvergenceFrames() const = 0; > virtual void SetEv(double ev) = 0; > - virtual void SetFlickerPeriod(double flicker_period) = 0; > - virtual void SetFixedShutter(double fixed_shutter) = 0; // microseconds > - virtual void SetMaxShutter(double max_shutter) = 0; // microseconds > + virtual void SetFlickerPeriod(const libcamera::utils::Duration &flicker_period) = 0; > + virtual void SetFixedShutter(const libcamera::utils::Duration &fixed_shutter) = 0; > + virtual void SetMaxShutter(const libcamera::utils::Duration &max_shutter) = 0; > virtual void SetFixedAnalogueGain(double fixed_analogue_gain) = 0; > virtual void SetMeteringMode(std::string const &metering_mode_name) = 0; > virtual void SetExposureMode(std::string const &exposure_mode_name) = 0; > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp > index f4cd5d26fb4e..4e3318445cf3 100644 > --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp > +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp > @@ -21,6 +21,7 @@ > > using namespace RPiController; > using namespace libcamera; > +using libcamera::utils::Duration; I would have used utils::Duration instead of Duration below, but I don't mind. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > LOG_DEFINE_CATEGORY(RPiAgc) > > @@ -222,19 +223,19 @@ void Agc::SetEv(double ev) > ev_ = ev; > } > > -void Agc::SetFlickerPeriod(double flicker_period) > +void Agc::SetFlickerPeriod(const Duration &flicker_period) > { > - flicker_period_ = flicker_period; > + flicker_period_ = flicker_period.get<std::micro>(); > } > > -void Agc::SetMaxShutter(double max_shutter) > +void Agc::SetMaxShutter(const Duration &max_shutter) > { > - max_shutter_ = max_shutter; > + max_shutter_ = max_shutter.get<std::micro>(); > } > > -void Agc::SetFixedShutter(double fixed_shutter) > +void Agc::SetFixedShutter(const Duration &fixed_shutter) > { > - fixed_shutter_ = fixed_shutter; > + fixed_shutter_ = fixed_shutter.get<std::micro>(); > // Set this in case someone calls Pause() straight after. > status_.shutter_time = clipShutter(fixed_shutter_); > } > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp > index 0427fb59ec1b..2b39fcabada3 100644 > --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp > +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp > @@ -77,9 +77,9 @@ public: > void Resume() override; > unsigned int GetConvergenceFrames() const override; > void SetEv(double ev) override; > - void SetFlickerPeriod(double flicker_period) override; > - void SetMaxShutter(double max_shutter) override; // microseconds > - void SetFixedShutter(double fixed_shutter) override; // microseconds > + void SetFlickerPeriod(const libcamera::utils::Duration &flicker_period) override; > + void SetMaxShutter(const libcamera::utils::Duration &max_shutter) override; > + void SetFixedShutter(const libcamera::utils::Duration &fixed_shutter) override; > void SetFixedAnalogueGain(double fixed_analogue_gain) override; > void SetMeteringMode(std::string const &metering_mode_name) override; > void SetExposureMode(std::string const &exposure_mode_name) override; > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp > index 4e02e94084f7..e083f6c254cc 100644 > --- a/src/ipa/raspberrypi/raspberrypi.cpp > +++ b/src/ipa/raspberrypi/raspberrypi.cpp > @@ -640,8 +640,8 @@ void IPARPi::queueRequest(const ControlList &controls) > break; > } > > - /* This expects units of micro-seconds. */ > - agc->SetFixedShutter(ctrl.second.get<int32_t>()); > + /* The control provides units of microseconds. */ > + agc->SetFixedShutter(ctrl.second.get<int32_t>() * 1.0us); > > libcameraMetadata_.set(controls::ExposureTime, ctrl.second.get<int32_t>()); > break; > @@ -1094,7 +1094,7 @@ void IPARPi::applyFrameDurations(const Duration &minFrameDuration, > > RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>( > controller_.GetAlgorithm("agc")); > - agc->SetMaxShutter(maxShutter.get<std::micro>()); > + agc->SetMaxShutter(maxShutter); > } > > void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)
diff --git a/src/ipa/raspberrypi/controller/agc_algorithm.hpp b/src/ipa/raspberrypi/controller/agc_algorithm.hpp index f97deb0fca59..579ee2c91fa8 100644 --- a/src/ipa/raspberrypi/controller/agc_algorithm.hpp +++ b/src/ipa/raspberrypi/controller/agc_algorithm.hpp @@ -6,6 +6,7 @@ */ #pragma once +#include "libcamera/internal/utils.h" #include "algorithm.hpp" namespace RPiController { @@ -17,9 +18,9 @@ public: // An AGC algorithm must provide the following: virtual unsigned int GetConvergenceFrames() const = 0; virtual void SetEv(double ev) = 0; - virtual void SetFlickerPeriod(double flicker_period) = 0; - virtual void SetFixedShutter(double fixed_shutter) = 0; // microseconds - virtual void SetMaxShutter(double max_shutter) = 0; // microseconds + virtual void SetFlickerPeriod(const libcamera::utils::Duration &flicker_period) = 0; + virtual void SetFixedShutter(const libcamera::utils::Duration &fixed_shutter) = 0; + virtual void SetMaxShutter(const libcamera::utils::Duration &max_shutter) = 0; virtual void SetFixedAnalogueGain(double fixed_analogue_gain) = 0; virtual void SetMeteringMode(std::string const &metering_mode_name) = 0; virtual void SetExposureMode(std::string const &exposure_mode_name) = 0; diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp index f4cd5d26fb4e..4e3318445cf3 100644 --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp @@ -21,6 +21,7 @@ using namespace RPiController; using namespace libcamera; +using libcamera::utils::Duration; LOG_DEFINE_CATEGORY(RPiAgc) @@ -222,19 +223,19 @@ void Agc::SetEv(double ev) ev_ = ev; } -void Agc::SetFlickerPeriod(double flicker_period) +void Agc::SetFlickerPeriod(const Duration &flicker_period) { - flicker_period_ = flicker_period; + flicker_period_ = flicker_period.get<std::micro>(); } -void Agc::SetMaxShutter(double max_shutter) +void Agc::SetMaxShutter(const Duration &max_shutter) { - max_shutter_ = max_shutter; + max_shutter_ = max_shutter.get<std::micro>(); } -void Agc::SetFixedShutter(double fixed_shutter) +void Agc::SetFixedShutter(const Duration &fixed_shutter) { - fixed_shutter_ = fixed_shutter; + fixed_shutter_ = fixed_shutter.get<std::micro>(); // Set this in case someone calls Pause() straight after. status_.shutter_time = clipShutter(fixed_shutter_); } diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp index 0427fb59ec1b..2b39fcabada3 100644 --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp @@ -77,9 +77,9 @@ public: void Resume() override; unsigned int GetConvergenceFrames() const override; void SetEv(double ev) override; - void SetFlickerPeriod(double flicker_period) override; - void SetMaxShutter(double max_shutter) override; // microseconds - void SetFixedShutter(double fixed_shutter) override; // microseconds + void SetFlickerPeriod(const libcamera::utils::Duration &flicker_period) override; + void SetMaxShutter(const libcamera::utils::Duration &max_shutter) override; + void SetFixedShutter(const libcamera::utils::Duration &fixed_shutter) override; void SetFixedAnalogueGain(double fixed_analogue_gain) override; void SetMeteringMode(std::string const &metering_mode_name) override; void SetExposureMode(std::string const &exposure_mode_name) override; diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp index 4e02e94084f7..e083f6c254cc 100644 --- a/src/ipa/raspberrypi/raspberrypi.cpp +++ b/src/ipa/raspberrypi/raspberrypi.cpp @@ -640,8 +640,8 @@ void IPARPi::queueRequest(const ControlList &controls) break; } - /* This expects units of micro-seconds. */ - agc->SetFixedShutter(ctrl.second.get<int32_t>()); + /* The control provides units of microseconds. */ + agc->SetFixedShutter(ctrl.second.get<int32_t>() * 1.0us); libcameraMetadata_.set(controls::ExposureTime, ctrl.second.get<int32_t>()); break; @@ -1094,7 +1094,7 @@ void IPARPi::applyFrameDurations(const Duration &minFrameDuration, RPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>( controller_.GetAlgorithm("agc")); - agc->SetMaxShutter(maxShutter.get<std::micro>()); + agc->SetMaxShutter(maxShutter); } void IPARPi::applyAGC(const struct AgcStatus *agcStatus, ControlList &ctrls)