From patchwork Wed Jul 29 09:31:54 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Plowman X-Patchwork-Id: 9065 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 A6697BD86F for ; Wed, 29 Jul 2020 09:32:11 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 726DE61978; Wed, 29 Jul 2020 11:32:11 +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="boVR00H7"; dkim-atps=neutral Received: from mail-wm1-x32c.google.com (mail-wm1-x32c.google.com [IPv6:2a00:1450:4864:20::32c]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id ED0B761978 for ; Wed, 29 Jul 2020 11:32:04 +0200 (CEST) Received: by mail-wm1-x32c.google.com with SMTP id f18so2240311wml.3 for ; Wed, 29 Jul 2020 02:32:04 -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=ZtZWQ4r+KnY4SNVh2eWcER4lIk+ceFuugUZ27hqXcQ4=; b=boVR00H7Cs92h2wQ0+a9mbjZMIiqxdMEmKcWWcVTg9kVhHrSa70HgOlcRlNtp/vzXl LER60nkiH24gD2QHmDu2VNNb01f5ZEb4zpb0Rq1xVkPwvPJmI4FkWAdhl90iBSzGseQj +wMbhOgY/bNKngq9+FBW6F/dktekRjexT9I1oo4Om0bETPg0dG0SGeuRi97wobjrIe+j 000QHl7qXwR6cVhFCAe45nPMpbCa/yBPEBrVTTU+0vnCh6V9LBVK4ZPp9NPu7bx8xNNi S+Z+9ST/50hxjYFrHXUFwa4rA2EgM5NoNyIpwXV1uJtCZjx/tk53Abe3pu0p2u1Rz31I V1vA== 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=ZtZWQ4r+KnY4SNVh2eWcER4lIk+ceFuugUZ27hqXcQ4=; b=kifyuWQZluhyRgJLZd5lrcppoTAuYLTPfgeDB2sjaQsTQAoxWo1fzDYhp6CTKtc87a 7Nh8qBoWgXoD/2kvc1h+m0aN9JeqB26DZC1LXnqmODkEjiW80ij4iKzbNt0TU2hF7UCt 57iTrtv0xNsLoTBTF+dwNoBeV3rlFr/Nn6W801K6Fa9TiAI0AzD3wAgcXfueWi1mld7C YZ0B+rUFbi+DsJgngfJ4t18H5z+cgwDmg7Rzln7KNXIyTlFIlNEicIY4jeDrrA+LLAHZ Z+zyaWuQnOvZM5WsJsW2h7LKChLfiGiTnLQQDJESaJRIHFUPnkCTUHR2cBZamIn30rbT EU7g== X-Gm-Message-State: AOAM533qru8AJZW4AF4UPTkg/WYmuE3/Z2A+Vhx9DrSZmkavc34aOZZU /9w8kCtrBNtP4hn3HthBq8XKRcAeKmSJuA== X-Google-Smtp-Source: ABdhPJwwTk0TczdxlG65a/nxWq0MLRiSLGogIFnpgxOQptPRo19fEpxVZ2gOOD/5+suBNFzIzztMMw== X-Received: by 2002:a1c:283:: with SMTP id 125mr8301614wmc.12.1596015124196; Wed, 29 Jul 2020 02:32:04 -0700 (PDT) Received: from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72]) by smtp.gmail.com with ESMTPSA id c15sm3709921wme.23.2020.07.29.02.32.03 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 29 Jul 2020 02:32:03 -0700 (PDT) From: David Plowman To: libcamera-devel@lists.libcamera.org Date: Wed, 29 Jul 2020 10:31:54 +0100 Message-Id: <20200729093154.12296-6-david.plowman@raspberrypi.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20200729093154.12296-1-david.plowman@raspberrypi.com> References: <20200729093154.12296-1-david.plowman@raspberrypi.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH 5/5] libcamera: raspberrypi: ALSC: Fix crop/transform of luminance table 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" The code was not handling a change of crop correctly in the luminance table. This change fixes that, and also makes it work for a change of transform. As part of this it seemed sensible to ensure the asynchronous thread is paused in the SwitchMode, at which point it also makes sense to detect "significant" mode changes and to restart the whole ALSC algorithm when that happens. --- src/ipa/raspberrypi/controller/rpi/alsc.cpp | 87 ++++++++++++++------- src/ipa/raspberrypi/controller/rpi/alsc.hpp | 3 +- 2 files changed, 61 insertions(+), 29 deletions(-) diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp index 33c1a88..fa53314 100644 --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp @@ -168,46 +168,79 @@ 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() +{ + std::unique_lock lock(mutex_); + if (async_started_) { + sync_signal_.wait(lock, [&] { + return async_finished_; + }); + async_started_ = false; + async_finished_ = false; + } +} + +static bool compare_crops(CameraMode const &cm0, CameraMode const &cm1) +{ + // Return true if the modes crop from the sensor significantly differently. + int left_diff = std::abs(cm0.crop_x - cm1.crop_x); + int top_diff = std::abs(cm0.crop_y - cm1.crop_y); + int right_diff = std::abs(cm0.crop_x + cm0.scale_x * cm0.width - + cm1.crop_x - cm1.scale_x * cm1.width); + int bottom_diff = std::abs(cm0.crop_y + cm0.scale_y * cm0.height - + cm1.crop_y - cm1.scale_y * cm1.height); + 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, Transform transform, Metadata *metadata) { (void)metadata; - // There's a bit of a question what we should do if the "crop" of the - // camera mode has changed. Any calculation currently in flight would - // not be useful to the new mode, so arguably we should abort it, and - // generate a new table (like the "first_time" code already here). When - // the crop doesn't change, we can presumably just leave things - // alone. For now, I think we'll just wait and see. When the crop does - // change, any effects should be transient, and if they're not transient - // enough, we'll revisit the question then. + // We're going to start over with the tables if there's any "significant" + // change. + bool reset_tables = first_time_ || transform_ != transform || + compare_crops(camera_mode_, camera_mode); + + // Ensure the other thread isn't running while we do this. + waitForAysncThread(); + camera_mode_ = camera_mode; transform_ = transform; - 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. + + // 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_, transform_, luminance_table_); + + 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_, transform_, 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_, transform_, cal_table_b); compensate_lambdas_for_cal(cal_table_r, lambda_r_, async_lambda_r_); compensate_lambdas_for_cal(cal_table_b, lambda_b_, async_lambda_b_); add_luminance_to_tables(sync_results_, async_lambda_r_, 1.0, - async_lambda_b_, config_.luminance_lut, + async_lambda_b_, luminance_table_, 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; } } @@ -253,8 +286,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. Use the previous value if none found. + 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; @@ -269,8 +302,6 @@ void Alsc::restartAsync(StatisticsPtr &stats, Metadata *image_metadata) } copy_stats(statistics_, stats, alsc_status); frame_phase_ = 0; - // copy the camera mode so it won't change during the calculations - async_camera_mode_ = camera_mode_; async_start_ = true; async_started_ = true; async_signal_.notify_one(); @@ -679,9 +710,9 @@ void Alsc::doAlsc() // Fetch the new calibrations (if any) for this CT. Resample them in // case the camera mode is not full-frame. get_cal_table(ct_, config_.calibrations_Cr, cal_table_tmp); - resample_cal_table(cal_table_tmp, async_camera_mode_, transform_, cal_table_r); + resample_cal_table(cal_table_tmp, camera_mode_, transform_, cal_table_r); get_cal_table(ct_, config_.calibrations_Cb, cal_table_tmp); - resample_cal_table(cal_table_tmp, async_camera_mode_, transform_, cal_table_b); + resample_cal_table(cal_table_tmp, camera_mode_, transform_, cal_table_b); // You could print out the cal tables for this image here, if you're // tuning the algorithm... (void)print_cal_table; @@ -704,7 +735,7 @@ void Alsc::doAlsc() compensate_lambdas_for_cal(cal_table_b, lambda_b_, async_lambda_b_); // Fold in the luminance table at the appropriate strength. add_luminance_to_tables(async_results_, async_lambda_r_, 1.0, - async_lambda_b_, config_.luminance_lut, + async_lambda_b_, luminance_table_, config_.luminance_strength); } diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.hpp b/src/ipa/raspberrypi/controller/rpi/alsc.hpp index 9001e8a..63bf28f 100644 --- a/src/ipa/raspberrypi/controller/rpi/alsc.hpp +++ b/src/ipa/raspberrypi/controller/rpi/alsc.hpp @@ -62,10 +62,10 @@ private: bool first_time_; CameraMode camera_mode_; libcamera::Transform transform_; + double luminance_table_[ALSC_CELLS_X * ALSC_CELLS_Y]; std::thread async_thread_; void asyncFunc(); // asynchronous thread function std::mutex mutex_; - CameraMode async_camera_mode_; // condvar for async thread to wait on std::condition_variable async_signal_; // condvar for synchronous thread to wait on @@ -88,6 +88,7 @@ private: int frame_count2_; double sync_results_[3][ALSC_CELLS_Y][ALSC_CELLS_X]; double prev_sync_results_[3][ALSC_CELLS_Y][ALSC_CELLS_X]; + void waitForAysncThread(); // The following are for the asynchronous thread to use, though the main // thread can set/reset them if the async thread is known to be idle: void restartAsync(StatisticsPtr &stats, Metadata *image_metadata);