{"id":19506,"url":"https://patchwork.libcamera.org/api/1.1/patches/19506/?format=json","web_url":"https://patchwork.libcamera.org/patch/19506/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/1.1/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20240218164908.15921-2-laurent.pinchart@ideasonboard.com>","date":"2024-02-18T16:49:04","name":"[v3,1/5] ipa: rkisp1: agc: Wrap variable length C arrays in spans","commit_ref":"971c4904ff91e3525fb807567fdb4408b15df00e","pull_url":null,"state":"accepted","archived":false,"hash":"c7ba3eb84385370dc267595588458a39366ff224","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/1.1/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/19506/mbox/","series":[{"id":4171,"url":"https://patchwork.libcamera.org/api/1.1/series/4171/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4171","date":"2024-02-18T16:49:03","name":"ipa: rkisp1: Support the i.MX8MP ISP","version":3,"mbox":"https://patchwork.libcamera.org/series/4171/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/19506/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/19506/checks/","tags":{},"headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 B13D5C3261\n\tfor <parsemail@patchwork.libcamera.org>;\n\tSun, 18 Feb 2024 16:49:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4BD8C6280B;\n\tSun, 18 Feb 2024 17:49:08 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6CDF162805\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSun, 18 Feb 2024 17:49:06 +0100 (CET)","from pendragon.ideasonboard.com (89-27-53-110.bb.dnainternet.fi\n\t[89.27.53.110])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 755D3975;\n\tSun, 18 Feb 2024 17:49:00 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"CgvWHKxa\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1708274940;\n\tbh=ekL3UFx/W8KHH4sICYYS0S9s/wuLgXxMVSzzEZkPxAw=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=CgvWHKxaJjbwICQESEV8PJVzD63AYIIGZtojiMuJ0QqmPUXo+msDo3Q9UPUDHaYLQ\n\tlhixDtoPHF/vH/oPQoPF+DFBz4Swpo6t8jDr1dUlsXr8pruxEmwp+AaaSYnK7cO745\n\tCZO7CyJLfshzZbE+Bp/lyBlWDc2sPIClwfLt6MRI=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Subject":"[PATCH v3 1/5] ipa: rkisp1: agc: Wrap variable length C arrays in\n\tspans","Date":"Sun, 18 Feb 2024 18:49:04 +0200","Message-ID":"<20240218164908.15921-2-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.43.0","In-Reply-To":"<20240218164908.15921-1-laurent.pinchart@ideasonboard.com>","References":"<20240218164908.15921-1-laurent.pinchart@ideasonboard.com>","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"},"content":"The RkISP1 statistics structure contains multiple arrays whose length\nvaries depending on the hardware revision. Accessing those arrays is\nerror-prone, wrap them in spans at the top level to reduce risks of\nout-of-bound accesses.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n src/ipa/rkisp1/algorithms/agc.cpp | 27 +++++++++++++--------------\n src/ipa/rkisp1/algorithms/agc.h   |  5 +++--\n 2 files changed, 16 insertions(+), 16 deletions(-)","diff":"diff --git a/src/ipa/rkisp1/algorithms/agc.cpp b/src/ipa/rkisp1/algorithms/agc.cpp\nindex e5aeb3426eff..f83a9ba1686a 100644\n--- a/src/ipa/rkisp1/algorithms/agc.cpp\n+++ b/src/ipa/rkisp1/algorithms/agc.cpp\n@@ -186,8 +186,8 @@ void Agc::prepare(IPAContext &context, const uint32_t frame,\n \t/* Produce the luminance histogram. */\n \tparams->meas.hst_config.mode = RKISP1_CIF_ISP_HISTOGRAM_MODE_Y_HISTOGRAM;\n \t/* Set an average weighted histogram. */\n-\tfor (unsigned int histBin = 0; histBin < numHistBins_; histBin++)\n-\t\tparams->meas.hst_config.hist_weight[histBin] = 1;\n+\tSpan<uint8_t> weights{ params->meas.hst_config.hist_weight, numHistBins_ };\n+\tstd::fill(weights.begin(), weights.end(), 1);\n \t/* Step size can't be less than 3. */\n \tparams->meas.hst_config.histogram_predivider = 4;\n \n@@ -318,7 +318,7 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,\n \n /**\n  * \\brief Estimate the relative luminance of the frame with a given gain\n- * \\param[in] ae The RkISP1 statistics and ISP results\n+ * \\param[in] expMeans The mean luminance values, from the RkISP1 statistics\n  * \\param[in] gain The gain to apply to the frame\n  *\n  * This function estimates the average relative luminance of the frame that\n@@ -342,28 +342,27 @@ void Agc::computeExposure(IPAContext &context, IPAFrameContext &frameContext,\n  *\n  * \\return The relative luminance\n  */\n-double Agc::estimateLuminance(const rkisp1_cif_isp_ae_stat *ae,\n-\t\t\t      double gain)\n+double Agc::estimateLuminance(Span<const uint8_t> expMeans, double gain)\n {\n \tdouble ySum = 0.0;\n \n \t/* Sum the averages, saturated to 255. */\n-\tfor (unsigned int aeCell = 0; aeCell < numCells_; aeCell++)\n-\t\tySum += std::min(ae->exp_mean[aeCell] * gain, 255.0);\n+\tfor (uint8_t expMean : expMeans)\n+\t\tySum += std::min(expMean * gain, 255.0);\n \n \t/* \\todo Weight with the AWB gains */\n \n-\treturn ySum / numCells_ / 255;\n+\treturn ySum / expMeans.size() / 255;\n }\n \n /**\n  * \\brief Estimate the mean value of the top 2% of the histogram\n- * \\param[in] hist The histogram statistics computed by the ImgU\n+ * \\param[in] hist The histogram statistics computed by the RkISP1\n  * \\return The mean value of the top 2% of the histogram\n  */\n-double Agc::measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const\n+double Agc::measureBrightness(Span<const uint32_t> hist) const\n {\n-\tHistogram histogram{ Span<const uint32_t>(hist->hist_bins, numHistBins_) };\n+\tHistogram histogram{ hist };\n \t/* Estimate the quantile mean of the top 2% of the histogram. */\n \treturn histogram.interQuantileMean(0.98, 1.0);\n }\n@@ -415,11 +414,11 @@ void Agc::process(IPAContext &context, [[maybe_unused]] const uint32_t frame,\n \tconst rkisp1_cif_isp_stat *params = &stats->params;\n \tASSERT(stats->meas_type & RKISP1_CIF_ISP_STAT_AUTOEXP);\n \n-\tconst rkisp1_cif_isp_ae_stat *ae = &params->ae;\n-\tconst rkisp1_cif_isp_hist_stat *hist = &params->hist;\n+\tSpan<const uint8_t> ae{ params->ae.exp_mean, numCells_ };\n+\tSpan<const uint32_t> hist{ params->hist.hist_bins, numHistBins_ };\n \n \tdouble iqMean = measureBrightness(hist);\n-\tdouble iqMeanGain = kEvGainTarget * numHistBins_ / iqMean;\n+\tdouble iqMeanGain = kEvGainTarget * hist.size() / iqMean;\n \n \t/*\n \t * Estimate the gain needed to achieve a relative luminance target. To\ndiff --git a/src/ipa/rkisp1/algorithms/agc.h b/src/ipa/rkisp1/algorithms/agc.h\nindex 8a22263741b6..ce8594f393ab 100644\n--- a/src/ipa/rkisp1/algorithms/agc.h\n+++ b/src/ipa/rkisp1/algorithms/agc.h\n@@ -9,6 +9,7 @@\n \n #include <linux/rkisp1-config.h>\n \n+#include <libcamera/base/span.h>\n #include <libcamera/base/utils.h>\n \n #include <libcamera/geometry.h>\n@@ -42,8 +43,8 @@ private:\n \tvoid computeExposure(IPAContext &Context, IPAFrameContext &frameContext,\n \t\t\t     double yGain, double iqMeanGain);\n \tutils::Duration filterExposure(utils::Duration exposureValue);\n-\tdouble estimateLuminance(const rkisp1_cif_isp_ae_stat *ae, double gain);\n-\tdouble measureBrightness(const rkisp1_cif_isp_hist_stat *hist) const;\n+\tdouble estimateLuminance(Span<const uint8_t> expMeans, double gain);\n+\tdouble measureBrightness(Span<const uint32_t> hist) const;\n \tvoid fillMetadata(IPAContext &context, IPAFrameContext &frameContext,\n \t\t\t  ControlList &metadata);\n \n","prefixes":["v3","1/5"]}