[{"id":13732,"web_url":"https://patchwork.libcamera.org/comment/13732/","msgid":"","date":"2020-11-17T10:30:23","subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: ipa: raspberrypi:\n\tagc: Use libcamera debug","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nOn Mon, 16 Nov 2020 at 16:49, David Plowman \nwrote:\n\n> Replace Raspberry Pi debug with libcamera debug.\n>\n> Signed-off-by: David Plowman \n>\n\nReviewed-by: Naushir Patuck \n\n\n> ---\n> src/ipa/raspberrypi/controller/rpi/agc.cpp | 90 +++++++++++-----------\n> 1 file changed, 47 insertions(+), 43 deletions(-)\n>\n> diff --git a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> index df4d3647..8079345b 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/agc.cpp\n> @@ -9,16 +9,20 @@\n>\n> #include \"linux/bcm2835-isp.h\"\n>\n> +#include \"libcamera/internal/log.h\"\n> +\n> #include \"../awb_status.h\"\n> #include \"../device_status.h\"\n> #include \"../histogram.hpp\"\n> -#include \"../logging.hpp\"\n> #include \"../lux_status.h\"\n> #include \"../metadata.hpp\"\n>\n> #include \"agc.hpp\"\n>\n> using namespace RPiController;\n> +using namespace libcamera;\n> +\n> +LOG_DEFINE_CATEGORY(RPiAgc)\n>\n> #define NAME \"rpi.agc\"\n>\n> @@ -128,7 +132,7 @@ static std::string read_constraint_modes(\n>\n> void AgcConfig::Read(boost::property_tree::ptree const ¶ms)\n> {\n> - RPI_LOG(\"AgcConfig\");\n> + LOG(RPiAgc, Debug) << \"AgcConfig\";\n> default_metering_mode = read_metering_modes(\n> metering_modes, params.get_child(\"metering_modes\"));\n> default_exposure_mode = read_exposure_modes(\n> @@ -166,7 +170,7 @@ char const *Agc::Name() const\n>\n> void Agc::Read(boost::property_tree::ptree const ¶ms)\n> {\n> - RPI_LOG(\"Agc\");\n> + LOG(RPiAgc, Debug) << \"Agc\";\n> config_.Read(params);\n> // Set the config's defaults (which are the first ones it read) as\n> our\n> // current modes, until someone changes them. (they're all known\n> to\n> @@ -254,15 +258,15 @@ void Agc::Prepare(Metadata *image_metadata)\n> status.digital_gain =\n> status_.total_exposure_value /\n> actual_exposure;\n> - RPI_LOG(\"Want total exposure \" <<\n> status_.total_exposure_value);\n> + LOG(RPiAgc, Debug) << \"Want total exposure\n> \" << status_.total_exposure_value;\n> // Never ask for a gain < 1.0, and also\n> impose\n> // some upper limit. Make it customisable?\n> status.digital_gain = std::max(\n> 1.0,\n> std::min(status.digital_gain,\n> 4.0));\n> - RPI_LOG(\"Actual exposure \" <<\n> actual_exposure);\n> - RPI_LOG(\"Use digital_gain \" <<\n> status.digital_gain);\n> - RPI_LOG(\"Effective exposure \" <<\n> actual_exposure * status.digital_gain);\n> + LOG(RPiAgc, Debug) << \"Actual exposure \"\n> << actual_exposure;\n> + LOG(RPiAgc, Debug) << \"Use digital_gain \"\n> << status.digital_gain;\n> + LOG(RPiAgc, Debug) << \"Effective exposure\n> \" << actual_exposure * status.digital_gain;\n> // Decide whether AEC/AGC has converged.\n> // Insist AGC is steady for MAX_LOCK_COUNT\n> // frames before we say we are \"locked\".\n> @@ -285,11 +289,11 @@ void Agc::Prepare(Metadata *image_metadata)\n>\n> status.target_exposure_value\n> - 1.5 * err)\n> lock_count_ = lock_count;\n> - RPI_LOG(\"Lock count: \" <<\n> lock_count_);\n> + LOG(RPiAgc, Debug) << \"Lock count:\n> \" << lock_count_;\n> }\n> }\n> } else\n> - RPI_LOG(Name() << \": no device metadata\");\n> + LOG(RPiAgc, Debug) << Name() << \": no device\n> metadata\";\n> status.locked = lock_count_ >= MAX_LOCK_COUNT;\n> //printf(\"%s\\n\", status.locked ? \"+++++++++\" : \"-\");\n> image_metadata->Set(\"agc.status\", status);\n> @@ -343,9 +347,9 @@ void Agc::housekeepConfig()\n> status_.fixed_analogue_gain = fixed_analogue_gain_;\n> status_.flicker_period = flicker_period_;\n> }\n> - RPI_LOG(\"ev \" << status_.ev << \" fixed_shutter \"\n> - << status_.fixed_shutter << \" fixed_analogue_gain \"\n> - << status_.fixed_analogue_gain);\n> + LOG(RPiAgc, Debug) << \"ev \" << status_.ev << \" fixed_shutter \"\n> + << status_.fixed_shutter << \"\n> fixed_analogue_gain \"\n> + << status_.fixed_analogue_gain;\n> // Make sure the \"mode\" pointers point to the up-to-date things, if\n> // they've changed.\n> if (strcmp(new_metering_mode_name.c_str(), status_.metering_mode))\n> {\n> @@ -376,10 +380,10 @@ void Agc::housekeepConfig()\n> copy_string(new_constraint_mode_name,\n> status_.constraint_mode,\n> sizeof(status_.constraint_mode));\n> }\n> - RPI_LOG(\"exposure_mode \"\n> - << new_exposure_mode_name << \" constraint_mode \"\n> - << new_constraint_mode_name << \" metering_mode \"\n> - << new_metering_mode_name);\n> + LOG(RPiAgc, Debug) << \"exposure_mode \"\n> + << new_exposure_mode_name << \" constraint_mode \"\n> + << new_constraint_mode_name << \" metering_mode \"\n> + << new_metering_mode_name;\n> }\n>\n> void Agc::fetchCurrentExposure(Metadata *image_metadata)\n> @@ -404,7 +408,7 @@ static double compute_initial_Y(bcm2835_isp_stats\n> *stats, Metadata *image_metada\n> struct AwbStatus awb;\n> awb.gain_r = awb.gain_g = awb.gain_b = 1.0; // in case no metadata\n> if (image_metadata->Get(\"awb.status\", awb) != 0)\n> - RPI_WARN(\"Agc: no AWB status found\");\n> + LOG(RPiAgc, Warning) << \"Agc: no AWB status found\";\n> double Y_sum = 0, weight_sum = 0;\n> for (int i = 0; i < AGC_STATS_SIZE; i++) {\n> if (regions[i].counted == 0)\n> @@ -443,7 +447,7 @@ void Agc::computeGain(bcm2835_isp_stats *statistics,\n> Metadata *image_metadata,\n> struct LuxStatus lux = {};\n> lux.lux = 400; // default lux level to 400 in case no metadata\n> found\n> if (image_metadata->Get(\"lux.status\", lux) != 0)\n> - RPI_WARN(\"Agc: no lux level found\");\n> + LOG(RPiAgc, Warning) << \"Agc: no lux level found\";\n> Histogram h(statistics->hist[0].g_hist, NUM_HISTOGRAM_BINS);\n> double ev_gain = status_.ev * config_.base_ev;\n> // The initial gain and target_Y come from some of the regions.\n> After\n> @@ -454,28 +458,28 @@ void Agc::computeGain(bcm2835_isp_stats *statistics,\n> Metadata *image_metadata,\n> double initial_Y = compute_initial_Y(statistics, image_metadata,\n> metering_mode_->weights);\n> gain = std::min(10.0, target_Y / (initial_Y + .001));\n> - RPI_LOG(\"Initially Y \" << initial_Y << \" target \" << target_Y\n> - << \" gives gain \" << gain);\n> + LOG(RPiAgc, Debug) << \"Initially Y \" << initial_Y << \" target \" <<\n> target_Y\n> + << \" gives gain \" << gain;\n> for (auto &c : *constraint_mode_) {\n> double new_target_Y;\n> double new_gain =\n> constraint_compute_gain(c, h, lux.lux, ev_gain,\n> new_target_Y);\n> - RPI_LOG(\"Constraint has target_Y \"\n> - << new_target_Y << \" giving gain \" << new_gain);\n> + LOG(RPiAgc, Debug) << \"Constraint has target_Y \"\n> + << new_target_Y << \" giving gain \" <<\n> new_gain;\n> if (c.bound == AgcConstraint::Bound::LOWER &&\n> new_gain > gain) {\n> - RPI_LOG(\"Lower bound constraint adopted\");\n> + LOG(RPiAgc, Debug) << \"Lower bound constraint\n> adopted\";\n> gain = new_gain, target_Y = new_target_Y;\n> } else if (c.bound == AgcConstraint::Bound::UPPER &&\n> new_gain < gain) {\n> - RPI_LOG(\"Upper bound constraint adopted\");\n> + LOG(RPiAgc, Debug) << \"Upper bound constraint\n> adopted\";\n> gain = new_gain, target_Y = new_target_Y;\n> }\n> }\n> - RPI_LOG(\"Final gain \" << gain << \" (target_Y \" << target_Y << \" ev\n> \"\n> - << status_.ev << \" base_ev \" <<\n> config_.base_ev\n> - << \")\");\n> + LOG(RPiAgc, Debug) << \"Final gain \" << gain << \" (target_Y \" <<\n> target_Y << \" ev \"\n> + << status_.ev << \" base_ev \" << config_.base_ev\n> + << \")\";\n> }\n>\n> void Agc::computeTargetExposure(double gain)\n> @@ -494,7 +498,7 @@ void Agc::computeTargetExposure(double gain)\n> : exposure_mode_->gain.back());\n> target_.total_exposure = std::min(target_.total_exposure,\n> max_total_exposure);\n> - RPI_LOG(\"Target total_exposure \" << target_.total_exposure);\n> + LOG(RPiAgc, Debug) << \"Target total_exposure \" <<\n> target_.total_exposure;\n> }\n>\n> bool Agc::applyDigitalGain(Metadata *image_metadata, double gain,\n> @@ -509,9 +513,9 @@ bool Agc::applyDigitalGain(Metadata *image_metadata,\n> double gain,\n> std::min(awb.gain_g,\n> awb.gain_b));\n> dg *= std::max(1.0, 1.0 / min_gain);\n> } else\n> - RPI_WARN(\"Agc: no AWB status found\");\n> - RPI_LOG(\"after AWB, target dg \" << dg << \" gain \" << gain\n> - << \" target_Y \" << target_Y);\n> + LOG(RPiAgc, Warning) << \"Agc: no AWB status found\";\n> + LOG(RPiAgc, Debug) << \"after AWB, target dg \" << dg << \" gain \" <<\n> gain\n> + << \" target_Y \" << target_Y;\n> // Finally, if we're trying to reduce exposure but the target_Y is\n> // \"close\" to 1.0, then the gain computed for that constraint will\n> be\n> // only slightly less than one, because the measured Y can never be\n> @@ -523,9 +527,9 @@ bool Agc::applyDigitalGain(Metadata *image_metadata,\n> double gain,\n> gain < sqrt(target_Y);\n> if (desaturate)\n> dg /= config_.fast_reduce_threshold;\n> - RPI_LOG(\"Digital gain \" << dg << \" desaturate? \" << desaturate);\n> + LOG(RPiAgc, Debug) << \"Digital gain \" << dg << \" desaturate? \" <<\n> desaturate;\n> target_.total_exposure_no_dg = target_.total_exposure / dg;\n> - RPI_LOG(\"Target total_exposure_no_dg \" <<\n> target_.total_exposure_no_dg);\n> + LOG(RPiAgc, Debug) << \"Target total_exposure_no_dg \" <<\n> target_.total_exposure_no_dg;\n> return desaturate;\n> }\n>\n> @@ -560,8 +564,8 @@ void Agc::filterExposure(bool desaturate)\n> filtered_.total_exposure * config_.fast_reduce_threshold)\n> filtered_.total_exposure_no_dg = filtered_.total_exposure *\n>\n> config_.fast_reduce_threshold;\n> - RPI_LOG(\"After filtering, total_exposure \" <<\n> filtered_.total_exposure <<\n> - \" no dg \" << filtered_.total_exposure_no_dg);\n> + LOG(RPiAgc, Debug) << \"After filtering, total_exposure \" <<\n> filtered_.total_exposure\n> + << \" no dg \" << filtered_.total_exposure_no_dg;\n> }\n>\n> void Agc::divvyupExposure()\n> @@ -602,8 +606,8 @@ void Agc::divvyupExposure()\n> }\n> }\n> }\n> - RPI_LOG(\"Divided up shutter and gain are \" << shutter_time << \"\n> and \"\n> - << analogue_gain);\n> + LOG(RPiAgc, Debug) << \"Divided up shutter and gain are \" <<\n> shutter_time << \" and \"\n> + << analogue_gain;\n> // Finally adjust shutter time for flicker avoidance (require both\n> // shutter and gain not to be fixed).\n> if (status_.fixed_shutter == 0.0 &&\n> @@ -621,8 +625,8 @@ void Agc::divvyupExposure()\n>\n> exposure_mode_->gain.back());\n> shutter_time = new_shutter_time;\n> }\n> - RPI_LOG(\"After flicker avoidance, shutter \"\n> - << shutter_time << \" gain \" << analogue_gain);\n> + LOG(RPiAgc, Debug) << \"After flicker avoidance, shutter \"\n> + << shutter_time << \" gain \" <<\n> analogue_gain;\n> }\n> filtered_.shutter = shutter_time;\n> filtered_.analogue_gain = analogue_gain;\n> @@ -641,10 +645,10 @@ void Agc::writeAndFinish(Metadata *image_metadata,\n> bool desaturate)\n> // Write to metadata as well, in case anyone wants to update the\n> camera\n> // immediately.\n> image_metadata->Set(\"agc.status\", status_);\n> - RPI_LOG(\"Output written, total exposure requested is \"\n> - << filtered_.total_exposure);\n> - RPI_LOG(\"Camera exposure update: shutter time \" <<\n> filtered_.shutter <<\n> - \" analogue gain \" << filtered_.analogue_gain);\n> + LOG(RPiAgc, Debug) << \"Output written, total exposure requested is\n> \"\n> + << filtered_.total_exposure;\n> + LOG(RPiAgc, Debug) << \"Camera exposure update: shutter time \" <<\n> filtered_.shutter\n> + << \" analogue gain \" << filtered_.analogue_gain;\n> }\n>\n> // Register algorithm with the system.\n> --\n> 2.20.1\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 39D07BE082\n\tfor ;\n\tTue, 17 Nov 2020 10:30:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 06C4563321;\n\tTue, 17 Nov 2020 11:30:42 +0100 (CET)","from mail-lf1-x12c.google.com (mail-lf1-x12c.google.com\n\t[IPv6:2a00:1450:4864:20::12c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 19E6C63282\n\tfor ;\n\tTue, 17 Nov 2020 11:30:40 +0100 (CET)","by mail-lf1-x12c.google.com with SMTP id z21so29362157lfe.12\n\tfor ;\n\tTue, 17 Nov 2020 02:30:40 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"lMkKAwSU\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=DrsbwR65mDnDqQxfN8qrDhvSkDfcguI8dMcVO96qtOE=;\n\tb=lMkKAwSUEAC0NXsocarPuB/B4k6DsPy2R8EbOkq0SubmHEOemMMz8t8dG38/1rxm7+\n\tto72ZFOx4vTxpj8p2jrhK5tr1l/U5HsqJC7dQbcBSFz12eAPdH96Scwrz7jjRT6EMlhD\n\tnvz8HWSpc+ZNb3s7yGDJC6i9tYPSdvs4B/IloIrgw4xnaPdReFyRMyA6EKOvNqbOIopW\n\tbGpbLtK8AzQpGlcbq1X4I4GMPWRmb/D+v7uaWLq4JoFumE4iIlB0JPSHL4JipfqRuSbv\n\tRujpF5ZeZlZs5FGWmX70MTa9nw+dOCOrReTRviy2UujxDy3jMG1FNJQdhjYiz90BBP4O\n\txDgQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=DrsbwR65mDnDqQxfN8qrDhvSkDfcguI8dMcVO96qtOE=;\n\tb=Y/tSO1qiRgxAx800XS+8YtbTuJt2IsYEVr72WMGgid6miD6kH8PYrP3Bm6yXzFQjcI\n\t68LFFVMMbnuXYxDBUW6VFUvzoETI305AGGCxV6nbseHdhDK0mmmSudyND5wv3AUbAWgc\n\tw/ar9yyGbpXtymtVWafe7B6fsXkoR1fnYkloDgKTsuOrx2xReBTYmk3QKVp07wxyeyw7\n\tN9lbHG7uq6zE9hW3Etyg5YIFLPcc9QCIkbT3/nyx4dioqnlMWAAebG3d1WLUh30Gr7IY\n\tflOp/hv2ch0HonYm5RUTHjwmZgSdiSw49aKlUmzlsuYY08YIT09p4NfrKTjrfrn3jVBb\n\tZHBw==","X-Gm-Message-State":"AOAM530UDzvNEwfus7GOTjfQeapIxrGxes2+B+8a7MLo94ULv1CIrSN/\n\tRu3qAlIwGp/+3DEDEzuXgVQUH4U68anP6NGCpWjsdA==","X-Google-Smtp-Source":"ABdhPJwmstb8ZfxAP0IvSJBaz1tTSXqDP7mDcUvsO+utc183IY5D2CJVY7zf4+x5bfTHi6u9RNrgb3KtL7xWao8eRg4=","X-Received":"by 2002:a05:6512:4c5:: with SMTP id\n\tw5mr1462538lfq.215.1605609039539; \n\tTue, 17 Nov 2020 02:30:39 -0800 (PST)","MIME-Version":"1.0","References":"<20201116164918.2055-1-david.plowman@raspberrypi.com>\n\t<20201116164918.2055-2-david.plowman@raspberrypi.com>","In-Reply-To":"<20201116164918.2055-2-david.plowman@raspberrypi.com>","From":"Naushir Patuck ","Date":"Tue, 17 Nov 2020 10:30:23 +0000","Message-ID":"","To":"David Plowman ","Subject":"Re: [libcamera-devel] [PATCH 01/10] libcamera: ipa: raspberrypi:\n\tagc: Use libcamera debug","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"multipart/mixed;\n\tboundary=\"===============3674787426593570530==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]