[{"id":11722,"web_url":"https://patchwork.libcamera.org/comment/11722/","msgid":"<20200729152957.GJ6183@pendragon.ideasonboard.com>","date":"2020-07-29T15:29:57","subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: raspberrypi: ALSC:\n\tHandle transform in colour tables","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi David,\n\nThank you for the patch.\n\nOn Wed, Jul 29, 2020 at 10:31:53AM +0100, David Plowman wrote:\n> This commit updates ALSC (Auto Lens Shading Correction) to handle the\n> transform now passed in the SwitchMode method.\n> \n> The transform is applied by the sensor, so the statistics we get\n> already incorporates it, and the adpative algorithm is agnostic to\n> it. That leaves only the calibrated tables (assumed measured without\n> any user transform) that need to be flipped, and resample_cal_table -\n> where we resample the calibrated table anyway - is an easy place to do\n> it.\n> ---\n> src/ipa/raspberrypi/controller/rpi/alsc.cpp | 22 ++++++++++++++-------\n> src/ipa/raspberrypi/controller/rpi/alsc.hpp | 3 ++-\n> 2 files changed, 17 insertions(+), 8 deletions(-)\n> \n> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.cpp b/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> index 5fac9dd..33c1a88 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.cpp\n> @@ -151,8 +151,9 @@ static void get_cal_table(double ct,\n> \t\t\t std::vector const &calibrations,\n> \t\t\t double cal_table[XY]);\n> static void resample_cal_table(double const cal_table_in[XY],\n> -\t\t\t CameraMode const &camera_mode,\n> -\t\t\t double cal_table_out[XY]);\n> +\t\t\t\t\t\t\t CameraMode const &camera_mode,\n> +\t\t\t\t\t\t\t Transform transform,\n> +\t\t\t\t\t\t\t double cal_table_out[XY]);\n\n:set tabstop=8\n\nbut you may not be using vim :-)\n\n> static void compensate_lambdas_for_cal(double const cal_table[XY],\n> \t\t\t\t double const old_lambdas[XY],\n> \t\t\t\t double new_lambdas[XY]);\n> @@ -187,6 +188,7 @@ void Alsc::SwitchMode(CameraMode const &camera_mode, Transform transform, Metada\n> \t// change, any effects should be transient, and if they're not transient\n> \t// enough, we'll revisit the question then.\n> \tcamera_mode_ = camera_mode;\n> +\ttransform_ = transform;\n> \tif (first_time_) {\n> \t\t// On the first time, arrange for something sensible in the\n> \t\t// initial tables. Construct the tables for some default colour\n> @@ -194,9 +196,9 @@ void Alsc::SwitchMode(CameraMode const &camera_mode, Transform transform, Metada\n> \t\t// adaptive algorithm.\n> \t\tdouble cal_table_r[XY], cal_table_b[XY], cal_table_tmp[XY];\n> \t\tget_cal_table(4000, config_.calibrations_Cr, cal_table_tmp);\n> -\t\tresample_cal_table(cal_table_tmp, camera_mode_, cal_table_r);\n> +\t\tresample_cal_table(cal_table_tmp, camera_mode_, transform_, cal_table_r);\n> \t\tget_cal_table(4000, config_.calibrations_Cb, cal_table_tmp);\n> -\t\tresample_cal_table(cal_table_tmp, camera_mode_, cal_table_b);\n> +\t\tresample_cal_table(cal_table_tmp, camera_mode_, transform_, cal_table_b);\n> \t\tcompensate_lambdas_for_cal(cal_table_r, lambda_r_,\n> \t\t\t\t\t async_lambda_r_);\n> \t\tcompensate_lambdas_for_cal(cal_table_b, lambda_b_,\n> @@ -379,7 +381,9 @@ void get_cal_table(double ct, std::vector const &calibrations,\n> }\n> \n> void resample_cal_table(double const cal_table_in[XY],\n> -\t\t\tCameraMode const &camera_mode, double cal_table_out[XY])\n> +\t\t\t\t\t\tCameraMode const &camera_mode,\n> +\t\t\t\t\t\tTransform transform,\n> +\t\t\t\t\t\tdouble cal_table_out[XY])\n> {\n> \t// Precalculate and cache the x sampling locations and phases to save\n> \t// recomputing them on every row.\n> @@ -395,6 +399,8 @@ void resample_cal_table(double const cal_table_in[XY],\n> \t\txf[i] = x - x_lo[i];\n> \t\tx_hi[i] = std::min(x_lo[i] + 1, X - 1);\n> \t\tx_lo[i] = std::max(x_lo[i], 0);\n> +\t\tif (transform.contains(Transform::hflip()))\n> +\t\t\tx_lo[i] = X - 1 - x_lo[i], x_hi[i] = X - 1 - x_hi[i];\n\n\t\tif (transform.contains(Transform::hflip())) {\n\t\t\tx_lo[i] = X - 1 - x_lo[i];\n\t\t\tx_hi[i] = X - 1 - x_hi[i];\n\t\t}\n\nNo need to hide the code :-)\n\n> \t}\n> \t// Now march over the output table generating the new values.\n> \tdouble scale_y = camera_mode.sensor_height /\n> @@ -407,6 +413,8 @@ void resample_cal_table(double const cal_table_in[XY],\n> \t\tdouble yf = y - y_lo;\n> \t\tint y_hi = std::min(y_lo + 1, Y - 1);\n> \t\ty_lo = std::max(y_lo, 0);\n> +\t\tif (transform.contains(Transform::vflip()))\n> +\t\t\ty_lo = Y - 1 - y_lo, y_hi = Y - 1 - y_hi;\n\nSame here.\n\n> \t\tdouble const *row_above = cal_table_in + X * y_lo;\n> \t\tdouble const *row_below = cal_table_in + X * y_hi;\n> \t\tfor (int i = 0; i < X; i++) {\n> @@ -671,9 +679,9 @@ void Alsc::doAlsc()\n> \t// Fetch the new calibrations (if any) for this CT. Resample them in\n> \t// case the camera mode is not full-frame.\n> \tget_cal_table(ct_, config_.calibrations_Cr, cal_table_tmp);\n> -\tresample_cal_table(cal_table_tmp, async_camera_mode_, cal_table_r);\n> +\tresample_cal_table(cal_table_tmp, async_camera_mode_, transform_, cal_table_r);\n> \tget_cal_table(ct_, config_.calibrations_Cb, cal_table_tmp);\n> -\tresample_cal_table(cal_table_tmp, async_camera_mode_, cal_table_b);\n> +\tresample_cal_table(cal_table_tmp, async_camera_mode_, transform_, cal_table_b);\n> \t// You could print out the cal tables for this image here, if you're\n> \t// tuning the algorithm...\n> \t(void)print_cal_table;\n> diff --git a/src/ipa/raspberrypi/controller/rpi/alsc.hpp b/src/ipa/raspberrypi/controller/rpi/alsc.hpp\n> index 9b54578..9001e8a 100644\n> --- a/src/ipa/raspberrypi/controller/rpi/alsc.hpp\n> +++ b/src/ipa/raspberrypi/controller/rpi/alsc.hpp\n> @@ -60,7 +60,8 @@ private:\n> \t// configuration is read-only, and available to both threads\n> \tAlscConfig config_;\n> \tbool first_time_;\n> -\tstd::atomic camera_mode_;\n> +\tCameraMode camera_mode_;\n\nThis seems unrelated. If you don't want to split it to a separate patch,\ncould you please mention it in the commit message with a rationale ?\n\n> +\tlibcamera::Transform transform_;\n> \tstd::thread async_thread_;\n> \tvoid asyncFunc(); // asynchronous thread function\n> \tstd::mutex mutex_;","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 B88EFBD878\n\tfor ;\n\tWed, 29 Jul 2020 15:30:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 24069617AF;\n\tWed, 29 Jul 2020 17:30:07 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5BECA6039D\n\tfor ;\n\tWed, 29 Jul 2020 17:30:06 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DB8B131F;\n\tWed, 29 Jul 2020 17:30:05 +0200 (CEST)"],"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=\"Rb6031xk\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1596036606;\n\tbh=wtyizTcux4YlRUd4iYdnQGyUcMpYE9XBCEAD2MRQA6U=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Rb6031xk/fyghVwjArhk8BkWomLwV6KrWYgpjsCO5byhj9DcVMkH+G8uvNaBRA4bU\n\tMQD/XKFHDNhElE7TFBZw8ridnFVhXwFhbYcY2lF1p0faeTlEGYFk7Npz6uJ6xquyHJ\n\tAbVFlfXbcwfnJxoglWRFyk2WGY6KGBx/K2dha9CQ=","Date":"Wed, 29 Jul 2020 18:29:57 +0300","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"<20200729152957.GJ6183@pendragon.ideasonboard.com>","References":"<20200729093154.12296-1-david.plowman@raspberrypi.com>\n\t<20200729093154.12296-5-david.plowman@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200729093154.12296-5-david.plowman@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH 4/5] libcamera: raspberrypi: ALSC:\n\tHandle transform in colour tables","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":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]