[libcamera-devel] [PATCH v2 3/7] libcamera: pipeline: raspberrypi: Support the new AE controls
paul.elder at ideasonboard.com
paul.elder at ideasonboard.com
Mon Oct 4 11:08:12 CEST 2021
Hi Naush,
Thanks for the feedback.
On Mon, Oct 04, 2021 at 09:31:06AM +0100, Naushir Patuck wrote:
> Hi Paul,
>
> Thank you for your work.
>
> On Fri, 1 Oct 2021 at 11:33, Paul Elder <paul.elder at ideasonboard.com> wrote:
>
> Add support for the new AE controls in the raspberrypi pipeline handler,
> and in the IPA.
>
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=42
> Bug: https://bugs.libcamera.org/show_bug.cgi?id=43
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>
> ---
> Changes in v2:
> - fix the rebase error where some uvc stuff was in rasberrypi
> - i haven't yet taken in the comments to move the new Pause/Resume
> functions
>
> Initial versoin:
> This is very hacky. I wasn't sure what the best way was to plumb it into
> the raspberrypi IPA as it was a bit hairy...
> ---
> include/libcamera/ipa/raspberrypi.h | 3 +-
> src/ipa/raspberrypi/controller/rpi/agc.cpp | 18 +++++++++-
> src/ipa/raspberrypi/controller/rpi/agc.hpp | 5 +++
> src/ipa/raspberrypi/raspberrypi.cpp | 42 +++++++++++++++++-----
> 4 files changed, 58 insertions(+), 10 deletions(-)
>
> diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/
> raspberrypi.h
> index 521eaecd..363ea038 100644
> --- a/include/libcamera/ipa/raspberrypi.h
> +++ b/include/libcamera/ipa/raspberrypi.h
> @@ -28,8 +28,9 @@ namespace RPi {
> * unsupported control is encountered.
> */
> static const ControlInfoMap Controls({
> - { &controls::AeEnable, ControlInfo(false, true) },
> + { &controls::ExposureTimeMode, ControlInfo
> (controls::ExposureTimeModeValues) },
> { &controls::ExposureTime, ControlInfo(0, 999999) },
> + { &controls::AnalogueGainMode, ControlInfo
> (controls::AnalogueGainModeValues) },
> { &controls::AnalogueGain, ControlInfo(1.0f, 32.0f) },
> { &controls::AeMeteringMode, ControlInfo
> (controls::AeMeteringModeValues) },
> { &controls::AeConstraintMode, ControlInfo
> (controls::AeConstraintModeValues) },
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/
> raspberrypi/controller/rpi/agc.cpp
> index 289c1fce..b45ea454 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> @@ -203,14 +203,30 @@ bool Agc::IsPaused() const
> }
>
> void Agc::Pause()
> +{
> +}
> +
> +void Agc::Resume()
> +{
> +}
> +
> +void Agc::PauseExposure()
>
>
> One small suggestion, could this be called PauseShutter() instead? Helps
We can call it whatever you want :D
I stil need to move these to AgcAlgorithm. Is really just that and
renaming these sufficient?
Thanks,
Paul
> clarify what part
> of the exposure is paused.
>
> Thanks,
> Naush
>
>
> {
> fixed_shutter_ = status_.shutter_time;
> +}
> +
> +void Agc::PauseGain()
> +{
> fixed_analogue_gain_ = status_.analogue_gain;
> }
>
> -void Agc::Resume()
> +void Agc::ResumeExposure()
> {
> fixed_shutter_ = 0s;
> +}
> +
> +void Agc::ResumeGain()
> +{
> fixed_analogue_gain_ = 0;
> }
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/
> raspberrypi/controller/rpi/agc.hpp
> index 82063636..7ca3ca2f 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> @@ -92,6 +92,11 @@ public:
> void Prepare(Metadata *image_metadata) override;
> void Process(StatisticsPtr &stats, Metadata *image_metadata)
> override;
>
> + void PauseExposure();
> + void PauseGain();
> + void ResumeExposure();
> + void ResumeGain();
> +
> private:
> void updateLockStatus(DeviceStatus const &device_status);
> AgcConfig config_;
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/
> raspberrypi.cpp
> index 047123ab..99935515 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -53,6 +53,8 @@
> #include "sharpen_algorithm.hpp"
> #include "sharpen_status.h"
>
> +#include "controller/rpi/agc.hpp"
> +
> namespace libcamera {
>
> using namespace std::literals::chrono_literals;
> @@ -478,7 +480,10 @@ void IPARPi::reportMetadata()
>
> AgcStatus *agcStatus = rpiMetadata_.GetLocked<AgcStatus>
> ("agc.status");
> if (agcStatus) {
> - libcameraMetadata_.set(controls::AeLocked, agcStatus->
> locked);
> + libcameraMetadata_.set(controls::AeState,
> + agcStatus->locked ?
> + controls::AeStateConverged :
> + controls::AeStateSearching);
> libcameraMetadata_.set(controls::DigitalGain, agcStatus->
> digital_gain);
> }
>
> @@ -623,20 +628,22 @@ void IPARPi::queueRequest(const ControlList &
> controls)
> << " = " << ctrl.second.toString();
>
> switch (ctrl.first) {
> - case controls::AE_ENABLE: {
> - RPiController::Algorithm *agc =
> controller_.GetAlgorithm("agc");
> + case controls::EXPOSURE_TIME_MODE: {
> + RPiController::Algorithm *algo =
> controller_.GetAlgorithm("agc");
> + RPiController::Agc *agc = reinterpret_cast
> <RPiController::Agc *>(algo);
> if (!agc) {
> LOG(IPARPI, Warning)
> - << "Could not set AE_ENABLE - no
> AGC algorithm";
> + << "Could not set
> EXPOSURE_TIME_MODE - no AGC algorithm";
> break;
> }
>
> - if (ctrl.second.get<bool>() == false)
> - agc->Pause();
> + if (ctrl.second.get<int32_t>() ==
> controls::ExposureTimeModeDisabled)
> + agc->PauseExposure();
> else
> - agc->Resume();
> + agc->ResumeExposure();
>
> - libcameraMetadata_.set(controls::AeEnable,
> ctrl.second.get<bool>());
> + libcameraMetadata_.set(controls::ExposureTimeMode,
> + ctrl.second.get<int32_t>());
> break;
> }
>
> @@ -656,6 +663,25 @@ void IPARPi::queueRequest(const ControlList &controls)
> break;
> }
>
> + case controls::ANALOGUE_GAIN_MODE: {
> + RPiController::Algorithm *algo =
> controller_.GetAlgorithm("agc");
> + RPiController::Agc *agc = reinterpret_cast
> <RPiController::Agc *>(algo);
> + if (!agc) {
> + LOG(IPARPI, Warning)
> + << "Could not set
> ANALOGUE_GAIN_MODE - no AGC algorithm";
> + break;
> + }
> +
> + if (ctrl.second.get<int32_t>() ==
> controls::AnalogueGainModeDisabled)
> + agc->PauseGain();
> + else
> + agc->ResumeGain();
> +
> + libcameraMetadata_.set(controls::AnalogueGainMode,
> + ctrl.second.get<int32_t>());
> + break;
> + }
> +
> case controls::ANALOGUE_GAIN: {
> RPiController::AgcAlgorithm *agc = dynamic_cast
> <RPiController::AgcAlgorithm *>(
> controller_.GetAlgorithm("agc"));
> --
> 2.27.0
>
>
More information about the libcamera-devel
mailing list