[libcamera-devel] [PATCH 1/1] ipa/raspberrypi: Remove generic "pause" mechanism from Algorithm
Nick Hollinghurst
nick.hollinghurst at raspberrypi.com
Mon Nov 21 15:38:03 CET 2022
Whoops. I will try that again...
Thanks,
Nick
On Mon, 21 Nov 2022 at 14:30, Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Nick,
>
> Thank you for the patch.
>
> On Mon, Nov 21, 2022 at 02:21:05PM +0000, Nick Hollinghurst via libcamera-devel wrote:
> > No existing Algorithm used the base pause(), resume() functions
> > or the paused_ flag, nor is there a need or a generic pause API.
> > Remove these. The AGC and AWB algorithms now have methods named
> > disableAuto(), enableAuto() which better describe their functionality.
> >
> > Signed-off-by: Nick Hollinghurst <nick.hollinghurst at raspberrypi.com>
> > ---
> > src/ipa/raspberrypi/controller/agc_algorithm.h | 2 ++
> > src/ipa/raspberrypi/controller/algorithm.h | 6 +-----
> > src/ipa/raspberrypi/controller/awb_algorithm.h | 2 ++
> > src/ipa/raspberrypi/controller/controller.cpp | 6 ++----
> > src/ipa/raspberrypi/controller/rpi/agc.cpp | 13 ++++---------
> > src/ipa/raspberrypi/controller/rpi/agc.h | 6 ++----
> > src/ipa/raspberrypi/controller/rpi/awb.cpp | 11 +++--------
> > src/ipa/raspberrypi/controller/rpi/awb.h | 6 ++----
> > src/ipa/raspberrypi/raspberrypi.cpp | 14 ++++++++------
> > 9 files changed, 26 insertions(+), 40 deletions(-)
> >
> > diff --git a/src/ipa/raspberrypi/controller/agc_algorithm.h
> > b/src/ipa/raspberrypi/controller/agc_algorithm.h
> > index 3a91444c..36e6c110 100644
> > --- a/src/ipa/raspberrypi/controller/agc_algorithm.h
> > +++ b/src/ipa/raspberrypi/controller/agc_algorithm.h
> > @@ -26,6 +26,8 @@ public:
> > virtual void setMeteringMode(std::string const &meteringModeName) = 0;
> > virtual void setExposureMode(std::string const &exposureModeName) = 0;
> > virtual void setConstraintMode(std::string const &contraintModeName) = 0;
> > + virtual void enableAuto() = 0;
> > + virtual void disableAuto() = 0;
>
> It looks like your mail client and/or server mess up with white space. I
> recommend using git-send-email to send patches, you will avoid lots of
> trouble.
>
> > };
> >
> > } /* namespace RPiController */
> > diff --git a/src/ipa/raspberrypi/controller/algorithm.h
> > b/src/ipa/raspberrypi/controller/algorithm.h
> > index cbbb13ba..4f327598 100644
> > --- a/src/ipa/raspberrypi/controller/algorithm.h
> > +++ b/src/ipa/raspberrypi/controller/algorithm.h
> > @@ -27,14 +27,11 @@ class Algorithm
> > {
> > public:
> > Algorithm(Controller *controller)
> > - : controller_(controller), paused_(false)
> > + : controller_(controller)
> > {
> > }
> > virtual ~Algorithm() = default;
> > virtual char const *name() const = 0;
> > - virtual bool isPaused() const { return paused_; }
> > - virtual void pause() { paused_ = true; }
> > - virtual void resume() { paused_ = false; }
> > virtual int read(const libcamera::YamlObject ¶ms);
> > virtual void initialise();
> > virtual void switchMode(CameraMode const &cameraMode, Metadata *metadata);
> > @@ -47,7 +44,6 @@ public:
> >
> > private:
> > Controller *controller_;
> > - bool paused_;
> > };
> >
> > /*
> > diff --git a/src/ipa/raspberrypi/controller/awb_algorithm.h
> > b/src/ipa/raspberrypi/controller/awb_algorithm.h
> > index 48e08b60..8462c4db 100644
> > --- a/src/ipa/raspberrypi/controller/awb_algorithm.h
> > +++ b/src/ipa/raspberrypi/controller/awb_algorithm.h
> > @@ -18,6 +18,8 @@ public:
> > virtual unsigned int getConvergenceFrames() const = 0;
> > virtual void setMode(std::string const &modeName) = 0;
> > virtual void setManualGains(double manualR, double manualB) = 0;
> > + virtual void enableAuto() = 0;
> > + virtual void disableAuto() = 0;
> > };
> >
> > } /* namespace RPiController */
> > diff --git a/src/ipa/raspberrypi/controller/controller.cpp
> > b/src/ipa/raspberrypi/controller/controller.cpp
> > index f4892786..e9156785 100644
> > --- a/src/ipa/raspberrypi/controller/controller.cpp
> > +++ b/src/ipa/raspberrypi/controller/controller.cpp
> > @@ -108,16 +108,14 @@ void Controller::prepare(Metadata *imageMetadata)
> > {
> > assert(switchModeCalled_);
> > for (auto &algo : algorithms_)
> > - if (!algo->isPaused())
> > - algo->prepare(imageMetadata);
> > + algo->prepare(imageMetadata);
> > }
> >
> > void Controller::process(StatisticsPtr stats, Metadata *imageMetadata)
> > {
> > assert(switchModeCalled_);
> > for (auto &algo : algorithms_)
> > - if (!algo->isPaused())
> > - algo->process(stats, imageMetadata);
> > + algo->process(stats, imageMetadata);
> > }
> >
> > Metadata &Controller::getGlobalMetadata()
> > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > index bd54a639..a30e50c1 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> > @@ -270,18 +270,13 @@ int Agc::read(const libcamera::YamlObject ¶ms)
> > return 0;
> > }
> >
> > -bool Agc::isPaused() const
> > -{
> > - return false;
> > -}
> > -
> > -void Agc::pause()
> > +void Agc::disableAuto()
> > {
> > fixedShutter_ = status_.shutterTime;
> > fixedAnalogueGain_ = status_.analogueGain;
> > }
> >
> > -void Agc::resume()
> > +void Agc::enableAuto()
> > {
> > fixedShutter_ = 0s;
> > fixedAnalogueGain_ = 0;
> > @@ -317,14 +312,14 @@ void Agc::setMaxShutter(Duration maxShutter)
> > void Agc::setFixedShutter(Duration fixedShutter)
> > {
> > fixedShutter_ = fixedShutter;
> > - /* Set this in case someone calls Pause() straight after. */
> > + /* Set this in case someone calls disableAuto() straight after. */
> > status_.shutterTime = clipShutter(fixedShutter_);
> > }
> >
> > void Agc::setFixedAnalogueGain(double fixedAnalogueGain)
> > {
> > fixedAnalogueGain_ = fixedAnalogueGain;
> > - /* Set this in case someone calls Pause() straight after. */
> > + /* Set this in case someone calls disableAuto() straight after. */
> > status_.analogueGain = fixedAnalogueGain;
> > }
> >
> > diff --git a/src/ipa/raspberrypi/controller/rpi/agc.h
> > b/src/ipa/raspberrypi/controller/rpi/agc.h
> > index 6d6b0e5a..cf04da19 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/agc.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/agc.h
> > @@ -75,10 +75,6 @@ public:
> > Agc(Controller *controller);
> > char const *name() const override;
> > int read(const libcamera::YamlObject ¶ms) override;
> > - /* AGC handles "pausing" for itself. */
> > - bool isPaused() const override;
> > - void pause() override;
> > - void resume() override;
> > unsigned int getConvergenceFrames() const override;
> > void setEv(double ev) override;
> > void setFlickerPeriod(libcamera::utils::Duration flickerPeriod) override;
> > @@ -88,6 +84,8 @@ public:
> > void setMeteringMode(std::string const &meteringModeName) override;
> > void setExposureMode(std::string const &exposureModeName) override;
> > void setConstraintMode(std::string const &contraintModeName) override;
> > + void enableAuto() override;
> > + void disableAuto() override;
> > void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;
> > void prepare(Metadata *imageMetadata) override;
> > void process(StatisticsPtr &stats, Metadata *imageMetadata) override;
> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > index 8d8ddf09..4f6af4ba 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp
> > @@ -222,21 +222,16 @@ void Awb::initialise()
> > asyncResults_ = syncResults_;
> > }
> >
> > -bool Awb::isPaused() const
> > +void Awb::disableAuto()
> > {
> > - return false;
> > -}
> > -
> > -void Awb::pause()
> > -{
> > - /* "Pause" by fixing everything to the most recent values. */
> > + /* Freeze the most recent values, and treat them as manual gains */
> > manualR_ = syncResults_.gainR = prevSyncResults_.gainR;
> > manualB_ = syncResults_.gainB = prevSyncResults_.gainB;
> > syncResults_.gainG = prevSyncResults_.gainG;
> > syncResults_.temperatureK = prevSyncResults_.temperatureK;
> > }
> >
> > -void Awb::resume()
> > +void Awb::enableAuto()
> > {
> > manualR_ = 0.0;
> > manualB_ = 0.0;
> > diff --git a/src/ipa/raspberrypi/controller/rpi/awb.h
> > b/src/ipa/raspberrypi/controller/rpi/awb.h
> > index 30acd89d..2254c3ed 100644
> > --- a/src/ipa/raspberrypi/controller/rpi/awb.h
> > +++ b/src/ipa/raspberrypi/controller/rpi/awb.h
> > @@ -93,13 +93,11 @@ public:
> > char const *name() const override;
> > void initialise() override;
> > int read(const libcamera::YamlObject ¶ms) override;
> > - /* AWB handles "pausing" for itself. */
> > - bool isPaused() const override;
> > - void pause() override;
> > - void resume() override;
> > unsigned int getConvergenceFrames() const override;
> > void setMode(std::string const &name) override;
> > void setManualGains(double manualR, double manualB) override;
> > + void enableAuto() override;
> > + void disableAuto() override;
> > void switchMode(CameraMode const &cameraMode, Metadata *metadata) override;
> > void prepare(Metadata *imageMetadata) override;
> > void process(StatisticsPtr &stats, Metadata *imageMetadata) override;
> > diff --git a/src/ipa/raspberrypi/raspberrypi.cpp
> > b/src/ipa/raspberrypi/raspberrypi.cpp
> > index b74f1ecf..beb076dc 100644
> > --- a/src/ipa/raspberrypi/raspberrypi.cpp
> > +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> > @@ -706,7 +706,8 @@ void IPARPi::queueRequest(const ControlList &controls)
> >
> > switch (ctrl.first) {
> > case controls::AE_ENABLE: {
> > - RPiController::Algorithm *agc = controller_.getAlgorithm("agc");
> > + RPiController::AgcAlgorithm *agc =
> > dynamic_cast<RPiController::AgcAlgorithm *>(
> > + controller_.getAlgorithm("agc"));
> > if (!agc) {
> > LOG(IPARPI, Warning)
> > << "Could not set AE_ENABLE - no AGC algorithm";
> > @@ -714,9 +715,9 @@ void IPARPi::queueRequest(const ControlList &controls)
> > }
> >
> > if (ctrl.second.get<bool>() == false)
> > - agc->pause();
> > + agc->disableAuto();
> > else
> > - agc->resume();
> > + agc->enableAuto();
> >
> > libcameraMetadata_.set(controls::AeEnable, ctrl.second.get<bool>());
> > break;
> > @@ -835,7 +836,8 @@ void IPARPi::queueRequest(const ControlList &controls)
> > }
> >
> > case controls::AWB_ENABLE: {
> > - RPiController::Algorithm *awb = controller_.getAlgorithm("awb");
> > + RPiController::AwbAlgorithm *awb =
> > dynamic_cast<RPiController::AwbAlgorithm *>(
> > + controller_.getAlgorithm("awb"));
> > if (!awb) {
> > LOG(IPARPI, Warning)
> > << "Could not set AWB_ENABLE - no AWB algorithm";
> > @@ -843,9 +845,9 @@ void IPARPi::queueRequest(const ControlList &controls)
> > }
> >
> > if (ctrl.second.get<bool>() == false)
> > - awb->pause();
> > + awb->disableAuto();
> > else
> > - awb->resume();
> > + awb->enableAuto();
> >
> > libcameraMetadata_.set(controls::AwbEnable,
> > ctrl.second.get<bool>());
> --
> Regards,
>
> Laurent Pinchart
More information about the libcamera-devel
mailing list