From patchwork Wed Jun 10 14:24:55 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 4015 Return-Path: Received: from mail-wr1-x431.google.com (mail-wr1-x431.google.com [IPv6:2a00:1450:4864:20::431]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id ECA5B603C4 for ; Wed, 10 Jun 2020 16:24:59 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="cpb5vVD0"; dkim-atps=neutral Received: by mail-wr1-x431.google.com with SMTP id r7so2527952wro.1 for ; Wed, 10 Jun 2020 07:24:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=raspberrypi.com; s=google; h=from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=cGYOngz5wHFZ0iZ1KsBkdaPYjzQo9fShw7QLyNjhwDc=; b=cpb5vVD0xCVoWEk87OJ/4SvgI8KsqAHGF41kXd2VBgrIs8Mo7BcxQri512a6zEkhL7 PXNCdZPtyHZMNqY5XOZlD6+q+OKInJgfPCkEKCgVl11OZXTy4bBE2y4c8vWFALxAkLFK CexT7LmdLKbXoQF4H41Qlr1abVFLIdrzhJ3CHM8ZlDFXsb8r9gwD3AtdopYyDe1UdhGc h3ctks8KtqBC8FgYV1FoAfEFwg3kCEFpKjKcRFgDNOLqVBxK80LQV1W+q6D6A1hv8cSz 4CJS0YW8NbYEzAnOoc5aCC3ao05Gq5Aj3dUdhao/YvfVOXkOgAItmZ8PVv1J5/1dem8u JVog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:mime-version :content-transfer-encoding; bh=cGYOngz5wHFZ0iZ1KsBkdaPYjzQo9fShw7QLyNjhwDc=; b=DqikPUqrfhFrfdst3DrvzjWy6TC6kkGeEtgoY6nZwAGYWaLTqXP86cdJlEbLIB/Ll4 2MXaKO6TOvH61G2klLCRgpoBwt4vw8AVzgQlIrdoDS5nAE9+REbf1MJkgL2Jo6dExUoT 1+1pxZR3kjG5gvjmI3u+dsAxmA32AVTFvhmzAQXA46xNgXIfWyqr5aQWqzOG6IVPfj3B 8l6I9lSNM2FJSOLTjdcOR/VmQFygmSIwHWYU3rcUxA4H2qi/AdNiqXKsYBWKn3qn1r7H 0KZi0SstX5i7NwfrVsT2chFnYh2FLgAHv5K36WW2aECrr7dmCDjxHuMaO3+PkgS6xsvk 2vpQ== X-Gm-Message-State: AOAM532/0NapvYhB5f+9nUH3LAHN3OvRo2XlWOpI3vSX+gEtbxYSLxXV LXi9/wkXHu1U2j8F2hNd3Lq4jf3LWBA= X-Google-Smtp-Source: ABdhPJz/qtq3mOPFm0Qp7NvpN0F5/EIjsUCCgA7sMJYC0UzZNHtYduycCm9mxQeyHedblBsk94GMZg== X-Received: by 2002:a5d:4ec3:: with SMTP id s3mr4312855wrv.103.1591799098963; Wed, 10 Jun 2020 07:24:58 -0700 (PDT) Received: from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72]) by smtp.gmail.com with ESMTPSA id o18sm7535615wme.19.2020.06.10.07.24.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 10 Jun 2020 07:24:58 -0700 (PDT) From: David Plowman To: libcamera-devel@lists.libcamera.org Date: Wed, 10 Jun 2020 15:24:55 +0100 Message-Id: <20200610142455.4641-1-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.20.1 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH] libcamera: raspberrypi: correct exposure values when camera mode changes X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 10 Jun 2020 14:25:00 -0000 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 Reviewed-by: Laurent Pinchart --- 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_; }