From patchwork Thu Oct 14 07:05:32 2021 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Hanlin Chen X-Patchwork-Id: 14131 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 51D4ABDC71 for ; Thu, 14 Oct 2021 07:05:48 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2135168F52; Thu, 14 Oct 2021 09:05:48 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=chromium.org header.i=@chromium.org header.b="nOBMYGrp"; dkim-atps=neutral Received: from mail-pj1-x102c.google.com (mail-pj1-x102c.google.com [IPv6:2607:f8b0:4864:20::102c]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 65F6B68F4D for ; Thu, 14 Oct 2021 09:05:46 +0200 (CEST) Received: by mail-pj1-x102c.google.com with SMTP id g13-20020a17090a3c8d00b00196286963b9so6227323pjc.3 for ; Thu, 14 Oct 2021 00:05:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=oN5vcNhH75dAQR8GHsogOinBnVa6zaInbpTKnMN3SKE=; b=nOBMYGrpQn1U+KRUmZu27oyN4ucbEgN9aXUqzBNXhtVh1SqPvMiblu5cHXzpIP5cjr Bz41RENuT9L0Ax1Cy9C7y9RDzK4c8tBiEfSdijLIiy8ooK6FDK+H4bnlQv0dBCoHQNcS wRLBZosUnz8kxSWbYtYNjmyC948R4UDb6IXvA= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=oN5vcNhH75dAQR8GHsogOinBnVa6zaInbpTKnMN3SKE=; b=S+8KwfjIjjG3M8EtId4UVWg+psVkVZP1Q2WA14tsxoAnIwyUicU2AWZlPW50tFJ8u5 uH55Q5kf7JV4nWMSQg7wLQTnoxRRrWMPsQCM359HoGDz1vzOL77kl4Vy6nru9tPRBVWb LkwvZvuIRAKoi9tI+y0syoaZ/epA5uwlc8pHwbciY13ziH4I39iT+LvApHFfT1K44RhR Mdze+X7WFr7kZ1Ys3FBNlAUYtTewEAXL2rSBQIH0iuHU4iby8IgsVFb1RFaQ+QD9XSDG Tqk4sRhyDb0q1uKLTC2iiFsy9hy7WJwPAaCsh9JckIObYRh3Vfl9+i+Fj4MTzIxD0qol qIgg== X-Gm-Message-State: AOAM5304wHzEN0U8hof05U7gyOCdrBFrYPy2MZ8ii7iBXmwqPze6545y XKwp5YwpemsZVEPyyyCO+GK+yEsQ0QVEVBjC X-Google-Smtp-Source: ABdhPJzj+PrlYceeCl6dmscBKdyVRMitRygxVXEwPYWgUFw8m/xKdm4SVRv7xInx07rtKUlFjOPm1Q== X-Received: by 2002:a17:90a:e10:: with SMTP id v16mr18988845pje.86.1634195143191; Thu, 14 Oct 2021 00:05:43 -0700 (PDT) Received: from localhost ([2401:fa00:1:10:4dac:fd0c:fba7:b5ae]) by smtp.gmail.com with UTF8SMTPSA id x65sm631693pjj.42.2021.10.14.00.05.41 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 14 Oct 2021 00:05:42 -0700 (PDT) From: Han-Lin Chen To: libcamera-devel@lists.libcamera.org Date: Thu, 14 Oct 2021 15:05:32 +0800 Message-Id: <20211014070533.3720964-2-hanlinchen@chromium.org> X-Mailer: git-send-email 2.33.0.882.g93a45727a2-goog In-Reply-To: <20211014070533.3720964-1-hanlinchen@chromium.org> References: <20211014070533.3720964-1-hanlinchen@chromium.org> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 2/3] ipu3: Change Macro migrated from Chrome OS to its std version accordingly 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Change the Macro STDCOPY, MEMCPY_S and CLEAR to its std version to better suit the style. The patch also fix misusage of STDCOPY as memcpy, which leads to copying overflown in PA and SA results. Signed-off-by: Han-Lin Chen Reviewed-by: Umang Jain Reviewed-by: Kieran Bingham --- aiq/aiq_input_parameters.cpp | 50 +++++++++----------- aiq/aiq_results.cpp | 91 +++++++++++++++--------------------- 2 files changed, 58 insertions(+), 83 deletions(-) diff --git a/aiq/aiq_input_parameters.cpp b/aiq/aiq_input_parameters.cpp index 56301f6..8a53849 100644 --- a/aiq/aiq_input_parameters.cpp +++ b/aiq/aiq_input_parameters.cpp @@ -14,11 +14,6 @@ #include -/* Macros used by imported code */ -#define STDCOPY(dst, src, size) std::copy((src), ((src) + (size)), (dst)) -#define MEMCPY_S(dest, dmax, src, smax) memcpy((dest), (src), std::min((size_t)(dmax), (size_t)(smax))) -#define CLEAR(x) memset(&(x), 0, sizeof(x)) - namespace libcamera { LOG_DEFINE_CATEGORY(AIQInputParameters) @@ -27,26 +22,26 @@ namespace ipa::ipu3::aiq { void AiqInputParameters::init() { - CLEAR(aeInputParams); - CLEAR(afParams); - CLEAR(afBracketParams); - CLEAR(awbParams); - CLEAR(gbceParams); - CLEAR(paParams); - CLEAR(saParams); - CLEAR(sensorDescriptor); - CLEAR(exposureWindow); - CLEAR(exposureCoordinate); - CLEAR(aeFeatures); - CLEAR(aeManualLimits); - CLEAR(manualFocusParams); - CLEAR(focusRect); - CLEAR(manualCctRange); - CLEAR(manualWhiteCoordinate); - CLEAR(awbResults); - CLEAR(colorGains); - CLEAR(exposureParams); - CLEAR(sensorFrameParams); + aeInputParams = {}; + afParams = {}; + afBracketParams = {}; + awbParams = {}; + gbceParams = {}; + paParams = {}; + saParams = {}; + sensorDescriptor = {}; + exposureWindow = {}; + exposureCoordinate = {}; + aeFeatures = {}; + aeManualLimits = {}; + manualFocusParams = {}; + focusRect = {}; + manualCctRange = {}; + manualWhiteCoordinate = {}; + awbResults = {}; + colorGains = {}; + exposureParams = {}; + sensorFrameParams = {}; aeLock = false; awbLock = false; blackLevelLock = false; @@ -102,10 +97,7 @@ AiqInputParameters &AiqInputParameters::operator=(const AiqInputParameters &othe if (this == &other) return *this; - MEMCPY_S(this, - sizeof(AiqInputParameters), - &other, - sizeof(AiqInputParameters)); + memcpy(this, &other, sizeof(AiqInputParameters)); reset(); /* Exposure coordinate is nullptr in other than SPOT mode. */ diff --git a/aiq/aiq_results.cpp b/aiq/aiq_results.cpp index 9dda17c..f727f36 100644 --- a/aiq/aiq_results.cpp +++ b/aiq/aiq_results.cpp @@ -14,9 +14,6 @@ #include -/* Macros used by imported code */ -#define STDCOPY(dst, src, size) std::copy((src), ((src) + (size)), (dst)) - namespace libcamera { LOG_DEFINE_CATEGORY(AIQResults) @@ -111,17 +108,14 @@ void AiqResults::setAe(ia_aiq_ae_results *ae) ae->weight_grid->height; gridElements = std::clamp(gridElements, 1, MAX_AE_GRID_SIZE); - STDCOPY(ae_.weight_grid->weights, - ae->weight_grid->weights, - gridElements * sizeof(char)); + std::copy_n(ae->weight_grid->weights, gridElements, ae_.weight_grid->weights); } else { LOG(AIQResults, Error) << "Not copying AE Weight Grids"; } // Copy the flash info structure if (ae_.flashes && ae->flashes) { - STDCOPY((int8_t *)ae_.flashes, (int8_t *)ae->flashes, - NUM_FLASH_LEDS * sizeof(ia_aiq_flash_parameters)); + std::copy_n(ae->flashes, NUM_FLASH_LEDS, ae_.flashes); } else { LOG(AIQResults, Error) << "Not copying AE Flashes"; } @@ -172,20 +166,16 @@ void AiqResults::setGbce(ia_aiq_gbce_results *gbce) gbce_.gamma_lut_size = gbce->gamma_lut_size; - STDCOPY((int8_t *)gbce_.r_gamma_lut, (int8_t *)gbce->r_gamma_lut, - gbce->gamma_lut_size * sizeof(float)); - STDCOPY((int8_t *)gbce_.b_gamma_lut, (int8_t *)gbce->b_gamma_lut, - gbce->gamma_lut_size * sizeof(float)); - STDCOPY((int8_t *)gbce_.g_gamma_lut, (int8_t *)gbce->g_gamma_lut, - gbce->gamma_lut_size * sizeof(float)); + std::copy_n(gbce->r_gamma_lut, gbce->gamma_lut_size, gbce_.r_gamma_lut); + std::copy_n(gbce->b_gamma_lut, gbce->gamma_lut_size, gbce_.b_gamma_lut); + std::copy_n(gbce->g_gamma_lut, gbce->gamma_lut_size, gbce_.g_gamma_lut); } else { LOG(AIQResults, Error) << "Not copying Gamma LUT channels"; } if (gbce->tone_map_lut_size > 0) { gbce_.tone_map_lut_size = gbce->tone_map_lut_size; - STDCOPY((int8_t *)gbce_.tone_map_lut, (int8_t *)gbce->tone_map_lut, - gbce->tone_map_lut_size * sizeof(float)); + std::copy_n(gbce->tone_map_lut, gbce->tone_map_lut_size, gbce_.tone_map_lut); } else { LOG(AIQResults, Error) << "Not copying Tone Mapping Gain LUT"; } @@ -200,20 +190,20 @@ void AiqResults::setPa(ia_aiq_pa_results *pa) { ASSERT(pa); - STDCOPY(&pa_.color_conversion_matrix[0][0], &pa->color_conversion_matrix[0][0], - MAX_COLOR_CONVERSION_MATRIX * MAX_COLOR_CONVERSION_MATRIX * - sizeof(pa->color_conversion_matrix[0][0])); + std::copy_n(&pa->color_conversion_matrix[0][0], + MAX_COLOR_CONVERSION_MATRIX * MAX_COLOR_CONVERSION_MATRIX, + &pa_.color_conversion_matrix[0][0]); if (pa_.preferred_acm && pa->preferred_acm) { pa_.preferred_acm->sector_count = pa->preferred_acm->sector_count; - STDCOPY(pa_.preferred_acm->hue_of_sectors, - pa->preferred_acm->hue_of_sectors, - sizeof(*pa->preferred_acm->hue_of_sectors) * pa->preferred_acm->sector_count); + std::copy_n(pa->preferred_acm->hue_of_sectors, + pa->preferred_acm->sector_count, + pa_.preferred_acm->hue_of_sectors); - STDCOPY(pa_.preferred_acm->advanced_color_conversion_matrices[0][0], - pa->preferred_acm->advanced_color_conversion_matrices[0][0], - sizeof(*pa->preferred_acm->advanced_color_conversion_matrices) * pa->preferred_acm->sector_count); + std::copy_n(pa->preferred_acm->advanced_color_conversion_matrices[0][0], + pa->preferred_acm->sector_count, + pa_.preferred_acm->advanced_color_conversion_matrices[0][0]); } else { LOG(AIQResults, Error) << "Not copying PA hue of sectors"; } @@ -222,17 +212,17 @@ void AiqResults::setPa(ia_aiq_pa_results *pa) pa_.ir_weight->height = pa->ir_weight->height; pa_.ir_weight->width = pa->ir_weight->width; - STDCOPY(pa_.ir_weight->ir_weight_grid_R, - pa->ir_weight->ir_weight_grid_R, - sizeof(*pa->ir_weight->ir_weight_grid_R) * pa->ir_weight->height * pa->ir_weight->width); + std::copy_n(pa->ir_weight->ir_weight_grid_R, + pa->ir_weight->height * pa->ir_weight->width, + pa_.ir_weight->ir_weight_grid_R); - STDCOPY(pa_.ir_weight->ir_weight_grid_G, - pa->ir_weight->ir_weight_grid_G, - sizeof(*pa->ir_weight->ir_weight_grid_G) * pa->ir_weight->height * pa->ir_weight->width); + std::copy_n(pa->ir_weight->ir_weight_grid_G, + pa->ir_weight->height * pa->ir_weight->width, + pa_.ir_weight->ir_weight_grid_G); - STDCOPY(pa_.ir_weight->ir_weight_grid_B, - pa->ir_weight->ir_weight_grid_B, - sizeof(*pa->ir_weight->ir_weight_grid_B) * pa->ir_weight->height * pa->ir_weight->width); + std::copy_n(pa->ir_weight->ir_weight_grid_B, + pa->ir_weight->height * pa->ir_weight->width, + pa_.ir_weight->ir_weight_grid_B); } else { LOG(AIQResults, Error) << "Not copying IR weight"; } @@ -253,13 +243,13 @@ void AiqResults::setSa(ia_aiq_sa_results *sa) sa_.height = sa->height; sa_.lsc_update = sa->lsc_update; + uint32_t lscGridSize = sa_.width * sa_.height; /* Check against one of the vectors but resize applicable to all. */ - if (channelGr_.size() < (sa_.width * sa_.height)) { - int lscNewSize = sa_.width * sa_.height; - channelGr_.resize(lscNewSize); - channelGb_.resize(lscNewSize); - channelR_.resize(lscNewSize); - channelB_.resize(lscNewSize); + if (channelGr_.size() < lscGridSize) { + channelGr_.resize(lscGridSize); + channelGb_.resize(lscGridSize); + channelR_.resize(lscGridSize); + channelB_.resize(lscGridSize); /* Update the SA data pointers to new memory locations. */ sa_.channel_gr = channelGr_.data(); @@ -269,23 +259,16 @@ void AiqResults::setSa(ia_aiq_sa_results *sa) } if (sa->lsc_update) { - uint32_t memCopySize = sa->width * sa->height * sizeof(float); - - STDCOPY((int8_t *)sa_.channel_gr, (int8_t *)sa->channel_gb, - memCopySize); - STDCOPY((int8_t *)sa_.channel_gb, (int8_t *)sa->channel_gr, - memCopySize); - STDCOPY((int8_t *)sa_.channel_r, (int8_t *)sa->channel_r, - memCopySize); - STDCOPY((int8_t *)sa_.channel_b, (int8_t *)sa->channel_b, - memCopySize); + std::copy_n(sa->channel_gr, lscGridSize, sa_.channel_gr); + std::copy_n(sa->channel_gb, lscGridSize, sa_.channel_gb); + std::copy_n(sa->channel_r, lscGridSize, sa_.channel_r); + std::copy_n(sa->channel_b, lscGridSize, sa_.channel_b); } else { - LOG(AIQResults, Error) << "Not copying LSC tables."; + LOG(AIQResults, Debug) << "Not copying LSC tables."; } - STDCOPY(&sa_.light_source[0], - &sa->light_source[0], - CMC_NUM_LIGHTSOURCES * sizeof(sa->light_source[0])); + std::copy_n(&sa->light_source[0], CMC_NUM_LIGHTSOURCES, &sa_.light_source[0]); + sa_.scene_difficulty = sa->scene_difficulty; sa_.num_patches = sa->num_patches; sa_.covered_area = sa->covered_area;