[libcamera-devel] [PATCH] libcamera: raspberrypi: correct exposure values when camera mode changes
David Plowman
david.plowman at raspberrypi.com
Mon Jun 15 17:20:58 CEST 2020
Hi
I was wondering what folks wanted to do about reviewing patches like
this one, when they're very much in the domain of the Raspberry Pi
IPAs. Do we need a different procedure for them, do you think?
Thanks!
David
On Wed, 10 Jun 2020 at 15:24, David Plowman
<david.plowman at raspberrypi.com> wrote:
>
> When the sensor is switched into a different mode the line
> timings may change, yet the V4L2 driver remembers the number of
> exposure lines, not the total time. So this will change "under
> our feet".
>
> These patches extend the IPA handling of the camera mode switch
> so that correct exposure values are recalculated for the new mode
> and written to the sensor before it starts streaming again. They
> also allow the IPA to alter how the total exposure is divided
> between actual exposure and analogue gain, according to the
> selected exposure profile in the camera tuning.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> ---
> src/ipa/raspberrypi/controller/algorithm.cpp | 3 +-
> src/ipa/raspberrypi/controller/algorithm.hpp | 2 +-
> src/ipa/raspberrypi/controller/controller.cpp | 4 +--
> src/ipa/raspberrypi/controller/controller.hpp | 2 +-
> src/ipa/raspberrypi/controller/rpi/agc.cpp | 12 ++++++++
> src/ipa/raspberrypi/controller/rpi/agc.hpp | 1 +
> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 4 ++-
> src/ipa/raspberrypi/controller/rpi/alsc.hpp | 2 +-
> src/ipa/raspberrypi/controller/rpi/noise.cpp | 4 ++-
> src/ipa/raspberrypi/controller/rpi/noise.hpp | 2 +-
> .../raspberrypi/controller/rpi/sharpen.cpp | 4 ++-
> .../raspberrypi/controller/rpi/sharpen.hpp | 2 +-
> src/ipa/raspberrypi/raspberrypi.cpp | 28 ++++++++++---------
> 13 files changed, 46 insertions(+), 24 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/controller/algorithm.cpp b/src/ipa/raspberrypi/controller/algorithm.cpp
> index 9bd3df8..55cb201 100644
> --- a/src/ipa/raspberrypi/controller/algorithm.cpp
> +++ b/src/ipa/raspberrypi/controller/algorithm.cpp
> @@ -16,9 +16,10 @@ void Algorithm::Read(boost::property_tree::ptree const ¶ms)
>
> void Algorithm::Initialise() {}
>
> -void Algorithm::SwitchMode(CameraMode const &camera_mode)
> +void Algorithm::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
> {
> (void)camera_mode;
> + (void)metadata;
> }
>
> void Algorithm::Prepare(Metadata *image_metadata)
> diff --git a/src/ipa/raspberrypi/controller/algorithm.hpp b/src/ipa/raspberrypi/controller/algorithm.hpp
> index b82c184..187c50c 100644
> --- a/src/ipa/raspberrypi/controller/algorithm.hpp
> +++ b/src/ipa/raspberrypi/controller/algorithm.hpp
> @@ -37,7 +37,7 @@ public:
> virtual void Resume() { paused_ = false; }
> virtual void Read(boost::property_tree::ptree const ¶ms);
> virtual void Initialise();
> - virtual void SwitchMode(CameraMode const &camera_mode);
> + virtual void SwitchMode(CameraMode const &camera_mode, Metadata *metadata);
> virtual void Prepare(Metadata *image_metadata);
> virtual void Process(StatisticsPtr &stats, Metadata *image_metadata);
> Metadata &GetGlobalMetadata() const
> diff --git a/src/ipa/raspberrypi/controller/controller.cpp b/src/ipa/raspberrypi/controller/controller.cpp
> index 20dd4c7..7c4b04f 100644
> --- a/src/ipa/raspberrypi/controller/controller.cpp
> +++ b/src/ipa/raspberrypi/controller/controller.cpp
> @@ -56,11 +56,11 @@ void Controller::Initialise()
> RPI_LOG("Controller finished");
> }
>
> -void Controller::SwitchMode(CameraMode const &camera_mode)
> +void Controller::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
> {
> RPI_LOG("Controller starting");
> for (auto &algo : algorithms_)
> - algo->SwitchMode(camera_mode);
> + algo->SwitchMode(camera_mode, metadata);
> switch_mode_called_ = true;
> RPI_LOG("Controller finished");
> }
> diff --git a/src/ipa/raspberrypi/controller/controller.hpp b/src/ipa/raspberrypi/controller/controller.hpp
> index d685386..6ba9412 100644
> --- a/src/ipa/raspberrypi/controller/controller.hpp
> +++ b/src/ipa/raspberrypi/controller/controller.hpp
> @@ -39,7 +39,7 @@ public:
> Algorithm *CreateAlgorithm(char const *name);
> void Read(char const *filename);
> void Initialise();
> - void SwitchMode(CameraMode const &camera_mode);
> + void SwitchMode(CameraMode const &camera_mode, Metadata *metadata);
> void Prepare(Metadata *image_metadata);
> void Process(StatisticsPtr stats, Metadata *image_metadata);
> Metadata &GetGlobalMetadata();
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> index a474287..c02b5ec 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp
> @@ -221,6 +221,18 @@ void Agc::SetConstraintMode(std::string const &constraint_mode_name)
> constraint_mode_name_ = constraint_mode_name;
> }
>
> +void Agc::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
> +{
> + // On a mode switch, it's possible the exposure profile could change,
> + // so we run through the dividing up of exposure/gain again and
> + // write the results into the metadata we've been given.
> + if (status_.total_exposure_value) {
> + housekeepConfig();
> + divvyupExposure();
> + writeAndFinish(metadata, false);
> + }
> +}
> +
> void Agc::Prepare(Metadata *image_metadata)
> {
> AgcStatus status;
> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.hpp b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> index dbcefba..9a7e89c 100644
> --- a/src/ipa/raspberrypi/controller/rpi/agc.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/agc.hpp
> @@ -75,6 +75,7 @@ public:
> void SetMeteringMode(std::string const &metering_mode_name) override;
> void SetExposureMode(std::string const &exposure_mode_name) override;
> void SetConstraintMode(std::string const &contraint_mode_name) override;
> + void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;
> void Prepare(Metadata *image_metadata) override;
> void Process(StatisticsPtr &stats, Metadata *image_metadata) override;
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> index 821a0ca..76e2f04 100644
> --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp
> @@ -173,8 +173,10 @@ void Alsc::Initialise()
> lambda_r_[i] = lambda_b_[i] = 1.0;
> }
>
> -void Alsc::SwitchMode(CameraMode const &camera_mode)
> +void Alsc::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
> {
> + (void)metadata;
> +
> // There's a bit of a question what we should do if the "crop" of the
> // camera mode has changed. Any calculation currently in flight would
> // not be useful to the new mode, so arguably we should abort it, and
> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.hpp b/src/ipa/raspberrypi/controller/rpi/alsc.hpp
> index c8ed3d2..3806257 100644
> --- a/src/ipa/raspberrypi/controller/rpi/alsc.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.hpp
> @@ -50,7 +50,7 @@ public:
> ~Alsc();
> char const *Name() const override;
> void Initialise() override;
> - void SwitchMode(CameraMode const &camera_mode) override;
> + void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;
> void Read(boost::property_tree::ptree const ¶ms) override;
> void Prepare(Metadata *image_metadata) override;
> void Process(StatisticsPtr &stats, Metadata *image_metadata) override;
> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.cpp b/src/ipa/raspberrypi/controller/rpi/noise.cpp
> index 2209d79..2cafde3 100644
> --- a/src/ipa/raspberrypi/controller/rpi/noise.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/noise.cpp
> @@ -27,8 +27,10 @@ char const *Noise::Name() const
> return NAME;
> }
>
> -void Noise::SwitchMode(CameraMode const &camera_mode)
> +void Noise::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
> {
> + (void)metadata;
> +
> // For example, we would expect a 2x2 binned mode to have a "noise
> // factor" of sqrt(2x2) = 2. (can't be less than one, right?)
> mode_factor_ = std::max(1.0, camera_mode.noise_factor);
> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.hpp b/src/ipa/raspberrypi/controller/rpi/noise.hpp
> index 51d46a3..25bf188 100644
> --- a/src/ipa/raspberrypi/controller/rpi/noise.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/noise.hpp
> @@ -18,7 +18,7 @@ class Noise : public Algorithm
> public:
> Noise(Controller *controller);
> char const *Name() const override;
> - void SwitchMode(CameraMode const &camera_mode) override;
> + void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;
> void Read(boost::property_tree::ptree const ¶ms) override;
> void Prepare(Metadata *image_metadata) override;
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> index 1f07bb6..086952f 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> @@ -26,8 +26,10 @@ char const *Sharpen::Name() const
> return NAME;
> }
>
> -void Sharpen::SwitchMode(CameraMode const &camera_mode)
> +void Sharpen::SwitchMode(CameraMode const &camera_mode, Metadata *metadata)
> {
> + (void)metadata;
> +
> // can't be less than one, right?
> mode_factor_ = std::max(1.0, camera_mode.noise_factor);
> }
> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp
> index 3b0d680..f871aa6 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.hpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.hpp
> @@ -18,7 +18,7 @@ class Sharpen : public Algorithm
> public:
> Sharpen(Controller *controller);
> char const *Name() const override;
> - void SwitchMode(CameraMode const &camera_mode) override;
> + void SwitchMode(CameraMode const &camera_mode, Metadata *metadata) override;
> void Read(boost::property_tree::ptree const ¶ms) override;
> void Prepare(Metadata *image_metadata) override;
>
> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
> index 9669f21..42c84b1 100644
> --- a/src/ipa/raspberrypi/raspberrypi.cpp
> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
> @@ -247,27 +247,29 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,
> mistrust_count_ = helper_->MistrustFramesStartup();
> }
>
> + struct AgcStatus agcStatus;
> + /* These zero values mean not program anything (unless overwritten). */
> + agcStatus.shutter_time = 0.0;
> + agcStatus.analogue_gain = 0.0;
> +
> if (!controllerInit_) {
> /* Load the tuning file for this sensor. */
> controller_.Read(tuningFile_.c_str());
> controller_.Initialise();
> controllerInit_ = true;
>
> - /* Calculate initial values for gain and exposure. */
> - int32_t gain_code = helper_->GainCode(DEFAULT_ANALOGUE_GAIN);
> - int32_t exposure_lines = helper_->ExposureLines(DEFAULT_EXPOSURE_TIME);
> -
> - ControlList ctrls(unicam_ctrls_);
> - ctrls.set(V4L2_CID_ANALOGUE_GAIN, gain_code);
> - ctrls.set(V4L2_CID_EXPOSURE, exposure_lines);
> -
> - IPAOperationData op;
> - op.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED;
> - op.controls.push_back(ctrls);
> - queueFrameAction.emit(0, op);
> + /* Supply initial values for gain and exposure. */
> + agcStatus.shutter_time = DEFAULT_EXPOSURE_TIME;
> + agcStatus.analogue_gain = DEFAULT_ANALOGUE_GAIN;
> }
>
> - controller_.SwitchMode(mode_);
> + RPi::Metadata metadata;
> + controller_.SwitchMode(mode_, &metadata);
> +
> + /* SwitchMode may supply updated exposure/gain values to use. */
> + metadata.Get("agc.status", agcStatus);
> + if (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0)
> + applyAGC(&agcStatus);
>
> lastMode_ = mode_;
> }
> --
> 2.20.1
>
More information about the libcamera-devel
mailing list