From patchwork Sat Aug 1 08:01:51 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 9128 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 4EA1CBD878 for ; Sat, 1 Aug 2020 08:02:02 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 188FC61F2E; Sat, 1 Aug 2020 10:02:02 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=raspberrypi.com header.i=@raspberrypi.com header.b="PGv3l83p"; dkim-atps=neutral Received: from mail-wr1-x432.google.com (mail-wr1-x432.google.com [IPv6:2a00:1450:4864:20::432]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2771B61F24 for ; Sat, 1 Aug 2020 10:02:00 +0200 (CEST) Received: by mail-wr1-x432.google.com with SMTP id r4so26872084wrx.9 for ; Sat, 01 Aug 2020 01:02:00 -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:in-reply-to:references :mime-version:content-transfer-encoding; bh=WY3rwcoThIxMWhmWUhlt0lQ29+IsYSrjXgY7k1TXiQM=; b=PGv3l83p3htpdtCnzfeKmPHEF5uQX3uO5syDZ+V3IDqlmzs8m+qHKa0ukwzkfuY1IH 05kEzX+Adyr65kbeGxXoKByH2vxUy/mo7tHJHgEgVdr9efRWbP16H2yD+WvXJLf+f+KU SjRs04Fdp5FewFFhdwOKM8BUylAA+6rgDQERaqXZLUZQTtjBKncj3CLfow+d3vPPBuE0 cV+xBWeQJFFieSRRFHSKRO691QuTtlLF9dtxHbuvKoyq7FPp+tN5KICcOOg2W1szSmb9 d9jmVxK/5NAfEhuvLADV+ISkCXD2iKGD5AaOKawS915ZnbJd+etnRzNohqa5WZp9BWfX xYhg== 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:in-reply-to :references:mime-version:content-transfer-encoding; bh=WY3rwcoThIxMWhmWUhlt0lQ29+IsYSrjXgY7k1TXiQM=; b=L/Mjbgb9ecqvaBHj2RWKGLMhF0LCL5N1chPF3BU44kWuHIr+Ok9Cx613fOMXUK8LCk xzsRQDGz6DOuvvOKOVJFScfriGwcUXhcgYDgqpLGkSUY7YSfNNLgl7Qnzig1AObyp1mm Bynn10BzLjasjUAH7hHiI6ygMnaMAsaKfiidf9iAv5+ie9zXrqsxeX0jJLYMXuSXat1I fFL3ciTwKBSgVsgR7X2z0T2TZMF4zO4vI9w8F7eg/rO9InJz0z3ZC3oPYZTEd7RpEiFm c6OC4x5GJf7NvVxFk+iEqSsWwwaAzob+4MYn7qqreo/7Yt8iVmC74igoOmVJik/qwp/g 0Q0w== X-Gm-Message-State: AOAM530l6Smk4b9Cra1Wl3F+7cHfHVvMZE6wtHOdmkiaU22ZW/1gt4B3 9cxqxf0mW9CefKlljXEH311OtpPs5dO7ww== X-Google-Smtp-Source: ABdhPJyLPqtXnAb1Uw67vzlD72l+MBw48t5x0FU1NJJBme0lx9gNx9PIIle4FxJU3WiDiUQmPX2aVw== X-Received: by 2002:adf:8282:: with SMTP id 2mr6454337wrc.76.1596268919406; Sat, 01 Aug 2020 01:01:59 -0700 (PDT) Received: from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72]) by smtp.gmail.com with ESMTPSA id l67sm17385253wml.13.2020.08.01.01.01.58 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sat, 01 Aug 2020 01:01:58 -0700 (PDT) From: David Plowman To: libcamera-devel@lists.libcamera.org Date: Sat, 1 Aug 2020 09:01:51 +0100 Message-Id: <20200801080151.4282-5-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200801080151.4282-1-david.plowman@raspberrypi.com> References: <20200801080151.4282-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v3 4/4] libcamera: ipa: raspberrypi: ALSC: Improve behaviour 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Now that we stop the asynchronous thread on a SwitchMode, we would do better to regenerate all the tables if the new camera mode crops in a significantly different way to the old one. A few minor tweaks make sense along with this: * Reset the lambda values when we reset everything. It wouldn't make sense to re-start with the old mode's values. * Use the last recorded colour temperature to generate new tables rather than any default value. * Set the frame "phase" counter to ensure the adaptive procedure will run asap. Signed-off-by: David Plowman Reviewed-by: Naushir Patuck Reviewed-by: Naushir Patuck --- src/ipa/raspberrypi/controller/rpi/alsc.cpp | 54 ++++++++++++++------- 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp index 4df9934..5e55954 100644 --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp @@ -166,11 +166,8 @@ void Alsc::Initialise() RPI_LOG("Alsc"); frame_count2_ = frame_count_ = frame_phase_ = 0; first_time_ = true; - // Initialise the lambdas. Each call to Process then restarts from the - // previous results. Also initialise the previous frame tables to the - // same harmless values. - for (int i = 0; i < XY; i++) - lambda_r_[i] = lambda_b_[i] = 1.0; + ct_ = config_.default_ct; + // The lambdas are initialised in the SwitchMode. } void Alsc::waitForAysncThread() @@ -185,31 +182,53 @@ void Alsc::waitForAysncThread() } } +static bool compare_modes(CameraMode const &cm0, CameraMode const &cm1) +{ + // Return true if the modes crop from the sensor significantly differently. + int left_diff = fabs(cm0.crop_x - cm1.crop_x); + int top_diff = fabs(cm0.crop_y - cm1.crop_y); + int right_diff = fabs(cm0.crop_x + cm0.scale_x * cm0.width - + cm1.crop_x - cm1.scale_x * cm1.width); + int bottom_diff = fabs(cm0.crop_y + cm0.scale_y * cm0.height - + cm1.crop_y - cm1.scale_y * cm1.height); + // These thresholds are a rather arbitrary amount chosen to trigger + // when carrying on with the previously calculated tables might be + // worse than regenerating them (but without the adaptive algorithm). + int threshold_x = cm0.sensor_width >> 4; + int threshold_y = cm0.sensor_height >> 4; + return left_diff > threshold_x || right_diff > threshold_x || + top_diff > threshold_y || bottom_diff > threshold_y; +} + void Alsc::SwitchMode(CameraMode const &camera_mode, Metadata *metadata) { (void)metadata; + // We're going to start over with the tables if there's any "significant" + // change. + bool reset_tables = first_time_ || compare_modes(camera_mode_, camera_mode); + // Ensure the other thread isn't running while we do this. waitForAysncThread(); - // There's a bit of a question what we should do if the "crop" of the - // camera mode has changed. Should we effectively reset the algorithm - // and start over? camera_mode_ = camera_mode; // We must resample the luminance table like we do the others, but it's // fixed so we can simply do it up front here. resample_cal_table(config_.luminance_lut, camera_mode_, luminance_table_); - if (first_time_) { - // On the first time, arrange for something sensible in the - // initial tables. Construct the tables for some default colour - // temperature. This echoes the code in doAlsc, without the - // adaptive algorithm. + if (reset_tables) { + // Upon every "table reset", arrange for something sensible to be + // generated. Construct the tables for the previous recorded colour + // temperature. In order to start over from scratch we initialise + // the lambdas, but the rest of this code then echoes the code in + // doAlsc, without the adaptive algorithm. + for (int i = 0; i < XY; i++) + lambda_r_[i] = lambda_b_[i] = 1.0; double cal_table_r[XY], cal_table_b[XY], cal_table_tmp[XY]; - get_cal_table(4000, config_.calibrations_Cr, cal_table_tmp); + get_cal_table(ct_, config_.calibrations_Cr, cal_table_tmp); resample_cal_table(cal_table_tmp, camera_mode_, cal_table_r); - get_cal_table(4000, config_.calibrations_Cb, cal_table_tmp); + get_cal_table(ct_, config_.calibrations_Cb, cal_table_tmp); resample_cal_table(cal_table_tmp, camera_mode_, cal_table_b); compensate_lambdas_for_cal(cal_table_r, lambda_r_, async_lambda_r_); @@ -220,6 +239,7 @@ void Alsc::SwitchMode(CameraMode const &camera_mode, Metadata *metadata) config_.luminance_strength); memcpy(prev_sync_results_, sync_results_, sizeof(prev_sync_results_)); + frame_phase_ = config_.frame_period; // run the algo again asap first_time_ = false; } } @@ -265,8 +285,8 @@ void Alsc::restartAsync(StatisticsPtr &stats, Metadata *image_metadata) { RPI_LOG("Starting ALSC thread"); // Get the current colour temperature. It's all we need from the - // metadata. - ct_ = get_ct(image_metadata, config_.default_ct); + // metadata. Default to the last CT value (which could be the default). + ct_ = get_ct(image_metadata, ct_); // We have to copy the statistics here, dividing out our best guess of // the LSC table that the pipeline applied to them. AlscStatus alsc_status;