[libcamera-devel] [PATCH v2 4/5] ipa: raspberrypi: Replace Raspberry Pi debug with libcamera debug
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Jan 26 09:26:39 CET 2021
Hi David,
Thank you for the patch.
On Mon, Jan 25, 2021 at 06:48:57PM +0000, David Plowman wrote:
> This commit deals with all the "small" algorithms (that is, not
> Agc/Awb/Alsc). A few unnecessary debug messages have also been
> removed.
>
> Signed-off-by: David Plowman <david.plowman at raspberrypi.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
> .../controller/rpi/black_level.cpp | 11 ++++++--
> src/ipa/raspberrypi/controller/rpi/ccm.cpp | 26 ++++++++++++-------
> .../raspberrypi/controller/rpi/contrast.cpp | 20 ++++++++++----
> src/ipa/raspberrypi/controller/rpi/dpc.cpp | 8 ++++--
> src/ipa/raspberrypi/controller/rpi/geq.cpp | 20 +++++++++-----
> src/ipa/raspberrypi/controller/rpi/lux.cpp | 11 +++++---
> src/ipa/raspberrypi/controller/rpi/noise.cpp | 14 ++++++----
> src/ipa/raspberrypi/controller/rpi/sdn.cpp | 21 +++++++++------
> .../raspberrypi/controller/rpi/sharpen.cpp | 11 ++++++--
> 9 files changed, 97 insertions(+), 45 deletions(-)
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/black_level.cpp b/src/ipa/raspberrypi/controller/rpi/black_level.cpp
> index 0629b77c..9ac855f5 100644
> --- a/src/ipa/raspberrypi/controller/rpi/black_level.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/black_level.cpp
> @@ -8,12 +8,16 @@
> #include <math.h>
> #include <stdint.h>
>
> +#include "libcamera/internal/log.h"
> +
> #include "../black_level_status.h"
> -#include "../logging.hpp"
>
> #include "black_level.hpp"
>
> using namespace RPiController;
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY(RPiBlackLevel)
>
> #define NAME "rpi.black_level"
>
> @@ -29,12 +33,15 @@ char const *BlackLevel::Name() const
>
> void BlackLevel::Read(boost::property_tree::ptree const ¶ms)
> {
> - RPI_LOG(Name());
> uint16_t black_level = params.get<uint16_t>(
> "black_level", 4096); // 64 in 10 bits scaled to 16 bits
> black_level_r_ = params.get<uint16_t>("black_level_r", black_level);
> black_level_g_ = params.get<uint16_t>("black_level_g", black_level);
> black_level_b_ = params.get<uint16_t>("black_level_b", black_level);
> + LOG(RPiBlackLevel, Debug)
> + << " Read black levels red " << black_level_r_
> + << " green " << black_level_g_
> + << " blue " << black_level_b_;
> }
>
> void BlackLevel::Prepare(Metadata *image_metadata)
> diff --git a/src/ipa/raspberrypi/controller/rpi/ccm.cpp b/src/ipa/raspberrypi/controller/rpi/ccm.cpp
> index a8a2caff..dbdcbb39 100644
> --- a/src/ipa/raspberrypi/controller/rpi/ccm.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/ccm.cpp
> @@ -5,15 +5,19 @@
> * ccm.cpp - CCM (colour correction matrix) control algorithm
> */
>
> +#include "libcamera/internal/log.h"
> +
> #include "../awb_status.h"
> #include "../ccm_status.h"
> -#include "../logging.hpp"
> #include "../lux_status.h"
> #include "../metadata.hpp"
>
> #include "ccm.hpp"
>
> using namespace RPiController;
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY(RPiCcm)
>
> // This algorithm selects a CCM (Colour Correction Matrix) according to the
> // colour temperature estimated by AWB (interpolating between known matricies as
> @@ -129,9 +133,9 @@ void Ccm::Prepare(Metadata *image_metadata)
> lux_ok = get_locked(image_metadata, "lux.status", lux);
> }
> if (!awb_ok)
> - RPI_WARN("Ccm: no colour temperature found");
> + LOG(RPiCcm, Warning) << "no colour temperature found";
> if (!lux_ok)
> - RPI_WARN("Ccm: no lux value found");
> + LOG(RPiCcm, Warning) << "no lux value found";
> Matrix ccm = calculate_ccm(config_.ccms, awb.temperature_K);
> double saturation = saturation_;
> struct CcmStatus ccm_status;
> @@ -144,13 +148,15 @@ void Ccm::Prepare(Metadata *image_metadata)
> for (int i = 0; i < 3; i++)
> ccm_status.matrix[j * 3 + i] =
> std::max(-8.0, std::min(7.9999, ccm.m[j][i]));
> - RPI_LOG("CCM: colour temperature " << awb.temperature_K << "K");
> - RPI_LOG("CCM: " << ccm_status.matrix[0] << " " << ccm_status.matrix[1]
> - << " " << ccm_status.matrix[2] << " "
> - << ccm_status.matrix[3] << " " << ccm_status.matrix[4]
> - << " " << ccm_status.matrix[5] << " "
> - << ccm_status.matrix[6] << " " << ccm_status.matrix[7]
> - << " " << ccm_status.matrix[8]);
> + LOG(RPiCcm, Debug)
> + << "colour temperature " << awb.temperature_K << "K";
> + LOG(RPiCcm, Debug)
> + << "CCM: " << ccm_status.matrix[0] << " " << ccm_status.matrix[1]
> + << " " << ccm_status.matrix[2] << " "
> + << ccm_status.matrix[3] << " " << ccm_status.matrix[4]
> + << " " << ccm_status.matrix[5] << " "
> + << ccm_status.matrix[6] << " " << ccm_status.matrix[7]
> + << " " << ccm_status.matrix[8];
> image_metadata->Set("ccm.status", ccm_status);
> }
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/contrast.cpp b/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> index 103153db..05ed139f 100644
> --- a/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/contrast.cpp
> @@ -6,12 +6,17 @@
> */
> #include <stdint.h>
>
> +#include "libcamera/internal/log.h"
> +
> #include "../contrast_status.h"
> #include "../histogram.hpp"
>
> #include "contrast.hpp"
>
> using namespace RPiController;
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY(RPiContrast)
>
> // This is a very simple control algorithm which simply retrieves the results of
> // AGC and AWB via their "status" metadata, and applies digital gain to the
> @@ -97,11 +102,13 @@ Pwl compute_stretch_curve(Histogram const &histogram,
> double hist_lo = histogram.Quantile(config.lo_histogram) *
> (65536 / NUM_HISTOGRAM_BINS);
> double level_lo = config.lo_level * 65536;
> - RPI_LOG("Move histogram point " << hist_lo << " to " << level_lo);
> + LOG(RPiContrast, Debug)
> + << "Move histogram point " << hist_lo << " to " << level_lo;
> hist_lo = std::max(
> level_lo,
> std::min(65535.0, std::min(hist_lo, level_lo + config.lo_max)));
> - RPI_LOG("Final values " << hist_lo << " -> " << level_lo);
> + LOG(RPiContrast, Debug)
> + << "Final values " << hist_lo << " -> " << level_lo;
> enhance.Append(hist_lo, level_lo);
> // Keep the mid-point (median) in the same place, though, to limit the
> // apparent amount of global brightness shift.
> @@ -113,11 +120,13 @@ Pwl compute_stretch_curve(Histogram const &histogram,
> double hist_hi = histogram.Quantile(config.hi_histogram) *
> (65536 / NUM_HISTOGRAM_BINS);
> double level_hi = config.hi_level * 65536;
> - RPI_LOG("Move histogram point " << hist_hi << " to " << level_hi);
> + LOG(RPiContrast, Debug)
> + << "Move histogram point " << hist_hi << " to " << level_hi;
> hist_hi = std::min(
> level_hi,
> std::max(0.0, std::max(hist_hi, level_hi - config.hi_max)));
> - RPI_LOG("Final values " << hist_hi << " -> " << level_hi);
> + LOG(RPiContrast, Debug)
> + << "Final values " << hist_hi << " -> " << level_hi;
> enhance.Append(hist_hi, level_hi);
> enhance.Append(65535, 65535);
> return enhance;
> @@ -127,7 +136,8 @@ Pwl apply_manual_contrast(Pwl const &gamma_curve, double brightness,
> double contrast)
> {
> Pwl new_gamma_curve;
> - RPI_LOG("Manual brightness " << brightness << " contrast " << contrast);
> + LOG(RPiContrast, Debug)
> + << "Manual brightness " << brightness << " contrast " << contrast;
> gamma_curve.Map([&](double x, double y) {
> new_gamma_curve.Append(
> x, std::max(0.0, std::min(65535.0,
> diff --git a/src/ipa/raspberrypi/controller/rpi/dpc.cpp b/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> index 348e1609..e533cd53 100644
> --- a/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/dpc.cpp
> @@ -5,10 +5,14 @@
> * dpc.cpp - DPC (defective pixel correction) control algorithm
> */
>
> -#include "../logging.hpp"
> +#include "libcamera/internal/log.h"
> +
> #include "dpc.hpp"
>
> using namespace RPiController;
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY(RPiDpc)
>
> // We use the lux status so that we can apply stronger settings in darkness (if
> // necessary).
> @@ -37,7 +41,7 @@ void Dpc::Prepare(Metadata *image_metadata)
> DpcStatus dpc_status = {};
> // Should we vary this with lux level or analogue gain? TBD.
> dpc_status.strength = config_.strength;
> - RPI_LOG("Dpc: strength " << dpc_status.strength);
> + LOG(RPiDpc, Debug) << "strength " << dpc_status.strength;
> image_metadata->Set("dpc.status", dpc_status);
> }
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/geq.cpp b/src/ipa/raspberrypi/controller/rpi/geq.cpp
> index b6c98414..5eff116c 100644
> --- a/src/ipa/raspberrypi/controller/rpi/geq.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/geq.cpp
> @@ -5,14 +5,18 @@
> * geq.cpp - GEQ (green equalisation) control algorithm
> */
>
> +#include "libcamera/internal/log.h"
> +
> #include "../device_status.h"
> -#include "../logging.hpp"
> #include "../lux_status.h"
> #include "../pwl.hpp"
>
> #include "geq.hpp"
>
> using namespace RPiController;
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY(RPiGeq)
>
> // We use the lux status so that we can apply stronger settings in darkness (if
> // necessary).
> @@ -44,11 +48,12 @@ void Geq::Prepare(Metadata *image_metadata)
> LuxStatus lux_status = {};
> lux_status.lux = 400;
> if (image_metadata->Get("lux.status", lux_status))
> - RPI_WARN("Geq: no lux data found");
> + LOG(RPiGeq, Warning) << "no lux data found";
> DeviceStatus device_status = {};
> device_status.analogue_gain = 1.0; // in case not found
> if (image_metadata->Get("device.status", device_status))
> - RPI_WARN("Geq: no device metadata - use analogue gain of 1x");
> + LOG(RPiGeq, Warning)
> + << "no device metadata - use analogue gain of 1x";
> GeqStatus geq_status = {};
> double strength =
> config_.strength.Empty()
> @@ -60,10 +65,11 @@ void Geq::Prepare(Metadata *image_metadata)
> double slope = config_.slope * strength;
> geq_status.offset = std::min(65535.0, std::max(0.0, offset));
> geq_status.slope = std::min(.99999, std::max(0.0, slope));
> - RPI_LOG("Geq: offset " << geq_status.offset << " slope "
> - << geq_status.slope << " (analogue gain "
> - << device_status.analogue_gain << " lux "
> - << lux_status.lux << ")");
> + LOG(RPiGeq, Debug)
> + << "offset " << geq_status.offset << " slope "
> + << geq_status.slope << " (analogue gain "
> + << device_status.analogue_gain << " lux "
> + << lux_status.lux << ")";
> image_metadata->Set("geq.status", geq_status);
> }
>
> diff --git a/src/ipa/raspberrypi/controller/rpi/lux.cpp b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> index 5acd49a0..aefd92d4 100644
> --- a/src/ipa/raspberrypi/controller/rpi/lux.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/lux.cpp
> @@ -8,12 +8,16 @@
>
> #include "linux/bcm2835-isp.h"
>
> +#include "libcamera/internal/log.h"
> +
> #include "../device_status.h"
> -#include "../logging.hpp"
>
> #include "lux.hpp"
>
> using namespace RPiController;
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY(RPiLux)
>
> #define NAME "rpi.lux"
>
> @@ -33,7 +37,6 @@ char const *Lux::Name() const
>
> void Lux::Read(boost::property_tree::ptree const ¶ms)
> {
> - RPI_LOG(Name());
> reference_shutter_speed_ =
> params.get<double>("reference_shutter_speed");
> reference_gain_ = params.get<double>("reference_gain");
> @@ -84,7 +87,7 @@ void Lux::Process(StatisticsPtr &stats, Metadata *image_metadata)
> LuxStatus status;
> status.lux = estimated_lux;
> status.aperture = current_aperture;
> - RPI_LOG(Name() << ": estimated lux " << estimated_lux);
> + LOG(RPiLux, Debug) << ": estimated lux " << estimated_lux;
> {
> std::unique_lock<std::mutex> lock(mutex_);
> status_ = status;
> @@ -93,7 +96,7 @@ void Lux::Process(StatisticsPtr &stats, Metadata *image_metadata)
> // algorithms get the latest value.
> image_metadata->Set("lux.status", status);
> } else
> - RPI_WARN(Name() << ": no device metadata");
> + LOG(RPiLux, Warning) << ": no device metadata";
> }
>
> // Register algorithm with the system.
> diff --git a/src/ipa/raspberrypi/controller/rpi/noise.cpp b/src/ipa/raspberrypi/controller/rpi/noise.cpp
> index 9e9eaf1b..156f7501 100644
> --- a/src/ipa/raspberrypi/controller/rpi/noise.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/noise.cpp
> @@ -7,13 +7,17 @@
>
> #include <math.h>
>
> +#include "libcamera/internal/log.h"
> +
> #include "../device_status.h"
> -#include "../logging.hpp"
> #include "../noise_status.h"
>
> #include "noise.hpp"
>
> using namespace RPiController;
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY(RPiNoise)
>
> #define NAME "rpi.noise"
>
> @@ -37,7 +41,6 @@ void Noise::SwitchMode(CameraMode const &camera_mode,
>
> void Noise::Read(boost::property_tree::ptree const ¶ms)
> {
> - RPI_LOG(Name());
> reference_constant_ = params.get<double>("reference_constant");
> reference_slope_ = params.get<double>("reference_slope");
> }
> @@ -58,10 +61,11 @@ void Noise::Prepare(Metadata *image_metadata)
> status.noise_constant = reference_constant_ * factor;
> status.noise_slope = reference_slope_ * factor;
> image_metadata->Set("noise.status", status);
> - RPI_LOG(Name() << ": constant " << status.noise_constant
> - << " slope " << status.noise_slope);
> + LOG(RPiNoise, Debug)
> + << "constant " << status.noise_constant
> + << " slope " << status.noise_slope;
> } else
> - RPI_WARN(Name() << " no metadata");
> + LOG(RPiNoise, Warning) << " no metadata";
> }
>
> // Register algorithm with the system.
> diff --git a/src/ipa/raspberrypi/controller/rpi/sdn.cpp b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> index aa82830b..0fad2550 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sdn.cpp
> @@ -5,12 +5,17 @@
> * sdn.cpp - SDN (spatial denoise) control algorithm
> */
>
> +#include "libcamera/internal/log.h"
> +
> #include "../noise_status.h"
> #include "../sdn_status.h"
>
> #include "sdn.hpp"
>
> using namespace RPiController;
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY(RPiSdn)
>
> // Calculate settings for the spatial denoise block using the noise profile in
> // the image metadata.
> @@ -40,19 +45,19 @@ void Sdn::Prepare(Metadata *image_metadata)
> struct NoiseStatus noise_status = {};
> noise_status.noise_slope = 3.0; // in case no metadata
> if (image_metadata->Get("noise.status", noise_status) != 0)
> - RPI_WARN("Sdn: no noise profile found");
> - RPI_LOG("Noise profile: constant " << noise_status.noise_constant
> - << " slope "
> - << noise_status.noise_slope);
> + LOG(RPiSdn, Warning) << "no noise profile found";
> + LOG(RPiSdn, Debug)
> + << "Noise profile: constant " << noise_status.noise_constant
> + << " slope " << noise_status.noise_slope;
> struct SdnStatus status;
> status.noise_constant = noise_status.noise_constant * deviation_;
> status.noise_slope = noise_status.noise_slope * deviation_;
> status.strength = strength_;
> image_metadata->Set("sdn.status", status);
> - RPI_LOG("Sdn: programmed constant " << status.noise_constant
> - << " slope " << status.noise_slope
> - << " strength "
> - << status.strength);
> + LOG(RPiSdn, Debug)
> + << "programmed constant " << status.noise_constant
> + << " slope " << status.noise_slope
> + << " strength " << status.strength;
> }
>
> // Register algorithm with the system.
> diff --git a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> index c953a7d9..2cd861ca 100644
> --- a/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> +++ b/src/ipa/raspberrypi/controller/rpi/sharpen.cpp
> @@ -7,12 +7,16 @@
>
> #include <math.h>
>
> -#include "../logging.hpp"
> +#include "libcamera/internal/log.h"
> +
> #include "../sharpen_status.h"
>
> #include "sharpen.hpp"
>
> using namespace RPiController;
> +using namespace libcamera;
> +
> +LOG_DEFINE_CATEGORY(RPiSharpen)
>
> #define NAME "rpi.sharpen"
>
> @@ -35,10 +39,13 @@ void Sharpen::SwitchMode(CameraMode const &camera_mode,
>
> void Sharpen::Read(boost::property_tree::ptree const ¶ms)
> {
> - RPI_LOG(Name());
> threshold_ = params.get<double>("threshold", 1.0);
> strength_ = params.get<double>("strength", 1.0);
> limit_ = params.get<double>("limit", 1.0);
> + LOG(RPiSharpen, Debug)
> + << "Read threshold " << threshold_
> + << " strength " << strength_
> + << " limit " << limit_;
> }
>
> void Sharpen::SetStrength(double strength)
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list