Patch Detail
Show a patch.
GET /api/patches/11207/?format=api
{ "id": 11207, "url": "https://patchwork.libcamera.org/api/patches/11207/?format=api", "web_url": "https://patchwork.libcamera.org/patch/11207/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20210210111743.14374-3-david.plowman@raspberrypi.com>", "date": "2021-02-10T11:17:43", "name": "[libcamera-devel,2/2] ipa: raspberrypi: AWB: Fix race condition setting manual gains", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "914a73f5e3a64fcd84baa91b6f93e1b276ae3708", "submitter": { "id": 42, "url": "https://patchwork.libcamera.org/api/people/42/?format=api", "name": "David Plowman", "email": "david.plowman@raspberrypi.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/11207/mbox/", "series": [ { "id": 1673, "url": "https://patchwork.libcamera.org/api/series/1673/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1673", "date": "2021-02-10T11:17:41", "name": "Raspberry Pi AWB race condition and maintenance", "version": 1, "mbox": "https://patchwork.libcamera.org/series/1673/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/11207/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/11207/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 BD217BD160\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 10 Feb 2021 11:17:52 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8AEDF61629;\n\tWed, 10 Feb 2021 12:17:52 +0100 (CET)", "from mail-ed1-x52e.google.com (mail-ed1-x52e.google.com\n\t[IPv6:2a00:1450:4864:20::52e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 11337602FE\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 10 Feb 2021 12:17:50 +0100 (CET)", "by mail-ed1-x52e.google.com with SMTP id s5so2456219edw.8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 10 Feb 2021 03:17:50 -0800 (PST)", "from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72])\n\tby smtp.gmail.com with ESMTPSA id\n\tdt17sm892659ejb.70.2021.02.10.03.17.48\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 10 Feb 2021 03:17:48 -0800 (PST)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"J/PvRIvu\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=from:to:cc:subject:date:message-id:in-reply-to:references\n\t:mime-version:content-transfer-encoding;\n\tbh=wzFVQfU0T7qTGPk1WFMLhvt3/kvhwvTHUc836eiWnbw=;\n\tb=J/PvRIvuAvXS/eo0V2pL4W3MO4B7TRsoXv/Sa0o6J3mhYIufwsTbyee4DRtOcBuars\n\tvrBbMV3BGEZatiL2/9Wa1GT1tD2TM72z8+kYbnyXhy8+XGHEq2oVzAFo6Y0nzoGBZsq4\n\twDIOTeHNHkTuHfrZ/ZuKGNjspiHlAsv/eLtUJXeBpIDDhf7mrTDgVuWKhnn6TQt7Yizp\n\tPG5Fy0Mpq4MvwBIIfTbEdMbmjaZw04G/AACNt81DUekYqiBYV16joxYQdkWyl1dLVvZM\n\tAnWdZFhqVaN8YXFkrWD0QAaKCWwxKim7VUPq1ywunz2B/DhVlJE7SgQlVjHtIOrk7Pe3\n\tajaQ==", "X-Google-DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to\n\t:references:mime-version:content-transfer-encoding;\n\tbh=wzFVQfU0T7qTGPk1WFMLhvt3/kvhwvTHUc836eiWnbw=;\n\tb=HqGykOLQQkju9xqzMfBPm06yJdWSdp9G0hd9yJvdv+oHvN6ngbwIvRWXsKthOshnXZ\n\t/grNaDXnRChnfADfpd3Rb+MKAqiYWhBtnynNwR3GtTfj+NDl9XUwNzyX+jQESqTSkXny\n\tFB2oXyRQcC8Z5e4fJv3pQEy3ixKDydXE0lcaPCQAqZvGRw4tBRZ63QriRCz8z3B9wvLw\n\tdP+byQGdizwA/tnVX8KsglyR+UUQq7AW3RPU536dn55NhhUtWPti8j/uKfVvSiF2yHyQ\n\tlI4JsyuKXUq/c5c9dLjDF80h6d/VsAEEwDRkiBN/Od2vfJa8KK8iOb0gawumo9my9St4\n\t5nHw==", "X-Gm-Message-State": "AOAM532mwE7WLW66zRhJXyedq36jenTUTkXGHyCnSatV2D2qypXfh8wX\n\tKZxdDogiVISMI1MoqZnBJ24IhzBClcexPrYW", "X-Google-Smtp-Source": "ABdhPJyvmVOI9E+OfjhxME2CgxGqT08663Z2UtDnILyMnbeUDvM2HuxmFGlwGaTeVgb8dg4BRLf4Ag==", "X-Received": "by 2002:a50:bb47:: with SMTP id y65mr2691479ede.33.1612955869410;\n\tWed, 10 Feb 2021 03:17:49 -0800 (PST)", "From": "David Plowman <david.plowman@raspberrypi.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Wed, 10 Feb 2021 11:17:43 +0000", "Message-Id": "<20210210111743.14374-3-david.plowman@raspberrypi.com>", "X-Mailer": "git-send-email 2.20.1", "In-Reply-To": "<20210210111743.14374-1-david.plowman@raspberrypi.com>", "References": "<20210210111743.14374-1-david.plowman@raspberrypi.com>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH 2/2] ipa: raspberrypi: AWB: Fix race\n\tcondition setting manual gains", "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>", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "Applying the manual_r_ and manual_b_ values is entirely removed from\nthe asynchronous thread where their use constituted a race hazard. The\nmain thread now deals with them entirely, involving the following\nchanges.\n\n1. SetManualGains() applies the new values directly to the\n\"sync_results\", meaning that Prepare() will jump to the new values\nimmediately (which is a better behaviour).\n\n2. Process() does not restart the asynchronous thread when manual\ngains are in force.\n\n3. The asynchronous thread might be running when manual gains are set,\nso we ignore the results produced in this case.\n\nSigned-off-by: David Plowman <david.plowman@raspberrypi.com>\n---\n src/ipa/raspberrypi/controller/rpi/awb.cpp | 78 ++++++++++------------\n 1 file changed, 37 insertions(+), 41 deletions(-)", "diff": "diff --git a/src/ipa/raspberrypi/controller/rpi/awb.cpp b/src/ipa/raspberrypi/controller/rpi/awb.cpp\nindex 1c65eda8..46a8ce2a 100644\n--- a/src/ipa/raspberrypi/controller/rpi/awb.cpp\n+++ b/src/ipa/raspberrypi/controller/rpi/awb.cpp\n@@ -193,28 +193,30 @@ void Awb::SetManualGains(double manual_r, double manual_b)\n \t// If any of these are 0.0, we swich back to auto.\n \tmanual_r_ = manual_r;\n \tmanual_b_ = manual_b;\n+\t// If non-zero, set these values into the sync_results which means that\n+\t// Prepare() will adopt them immediately.\n+\tif (manual_r_ != 0.0 && manual_b_ != 0.0) {\n+\t\tsync_results_.gain_r = prev_sync_results_.gain_r = manual_r_;\n+\t\tsync_results_.gain_g = prev_sync_results_.gain_g = 1.0;\n+\t\tsync_results_.gain_b = prev_sync_results_.gain_b = manual_b_;\n+\t}\n }\n \n void Awb::SwitchMode([[maybe_unused]] CameraMode const &camera_mode,\n \t\t Metadata *metadata)\n {\n-\t// If fixed colour gains have been set, we should let other algorithms\n-\t// know by writing it into the image metadata.\n-\tif (manual_r_ != 0.0 && manual_b_ != 0.0) {\n-\t\tprev_sync_results_.gain_r = manual_r_;\n-\t\tprev_sync_results_.gain_g = 1.0;\n-\t\tprev_sync_results_.gain_b = manual_b_;\n-\t\t// If we're starting up for the first time, try and\n-\t\t// \"dead reckon\" the corresponding colour temperature.\n-\t\tif (first_switch_mode_ && config_.bayes) {\n-\t\t\tPwl ct_r_inverse = config_.ct_r.Inverse();\n-\t\t\tPwl ct_b_inverse = config_.ct_b.Inverse();\n-\t\t\tdouble ct_r = ct_r_inverse.Eval(ct_r_inverse.Domain().Clip(1 / manual_r_));\n-\t\t\tdouble ct_b = ct_b_inverse.Eval(ct_b_inverse.Domain().Clip(1 / manual_b_));\n-\t\t\tprev_sync_results_.temperature_K = (ct_r + ct_b) / 2;\n-\t\t}\n-\t\tsync_results_ = prev_sync_results_;\n+\t// On the first mode switch we'll have no meaningful colour\n+\t// temperature, so try to dead reckon one.\n+\tif (manual_r_ != 0.0 && manual_b_ != 0.0 &&\n+\t first_switch_mode_ && config_.bayes) {\n+\t\tPwl ct_r_inverse = config_.ct_r.Inverse();\n+\t\tPwl ct_b_inverse = config_.ct_b.Inverse();\n+\t\tdouble ct_r = ct_r_inverse.Eval(ct_r_inverse.Domain().Clip(1 / manual_r_));\n+\t\tdouble ct_b = ct_b_inverse.Eval(ct_b_inverse.Domain().Clip(1 / manual_b_));\n+\t\tprev_sync_results_.temperature_K = (ct_r + ct_b) / 2;\n+\t\tsync_results_.temperature_K = prev_sync_results_.temperature_K;\n \t}\n+\t// Let other algorithms know the current white balance values.\n \tmetadata->Set(\"awb.status\", prev_sync_results_);\n \tfirst_switch_mode_ = false;\n }\n@@ -224,7 +226,10 @@ void Awb::fetchAsyncResults()\n \tLOG(RPiAwb, Debug) << \"Fetch AWB results\";\n \tasync_finished_ = false;\n \tasync_started_ = false;\n-\tsync_results_ = async_results_;\n+\t// It's possible manual gains could be set even while the async\n+\t// thread was running. In this case, we ignore its results.\n+\tif (manual_r_ == 0.0 || manual_b_ == 0.0)\n+\t\tsync_results_ = async_results_;\n }\n \n void Awb::restartAsync(StatisticsPtr &stats, double lux)\n@@ -289,8 +294,10 @@ void Awb::Process(StatisticsPtr &stats, Metadata *image_metadata)\n \tif (frame_phase_ < (int)config_.frame_period)\n \t\tframe_phase_++;\n \tLOG(RPiAwb, Debug) << \"frame_phase \" << frame_phase_;\n-\tif (frame_phase_ >= (int)config_.frame_period ||\n-\t frame_count_ < (int)config_.startup_frames) {\n+\t// We do not restart the async thread when we have fixed colour gains.\n+\tif ((manual_r_ == 0.0 || manual_b_ == 0.0) &&\n+\t (frame_phase_ >= (int)config_.frame_period ||\n+\t frame_count_ < (int)config_.startup_frames)) {\n \t\t// Update any settings and any image metadata that we need.\n \t\tstruct LuxStatus lux_status = {};\n \t\tlux_status.lux = 400; // in case no metadata\n@@ -614,29 +621,18 @@ void Awb::awbGrey()\n \n void Awb::doAwb()\n {\n-\tif (manual_r_ != 0.0 && manual_b_ != 0.0) {\n-\t\tasync_results_.temperature_K = 4500; // don't know what it is\n-\t\tasync_results_.gain_r = manual_r_;\n-\t\tasync_results_.gain_g = 1.0;\n-\t\tasync_results_.gain_b = manual_b_;\n+\tprepareStats();\n+\tLOG(RPiAwb, Debug) << \"Valid zones: \" << zones_.size();\n+\tif (zones_.size() > config_.min_regions) {\n+\t\tif (config_.bayes)\n+\t\t\tawbBayes();\n+\t\telse\n+\t\t\tawbGrey();\n \t\tLOG(RPiAwb, Debug)\n-\t\t\t<< \"Using manual white balance: gain_r \"\n-\t\t\t<< async_results_.gain_r << \" gain_b \"\n-\t\t\t<< async_results_.gain_b;\n-\t} else {\n-\t\tprepareStats();\n-\t\tLOG(RPiAwb, Debug) << \"Valid zones: \" << zones_.size();\n-\t\tif (zones_.size() > config_.min_regions) {\n-\t\t\tif (config_.bayes)\n-\t\t\t\tawbBayes();\n-\t\t\telse\n-\t\t\t\tawbGrey();\n-\t\t\tLOG(RPiAwb, Debug)\n-\t\t\t\t<< \"CT found is \"\n-\t\t\t\t<< async_results_.temperature_K\n-\t\t\t\t<< \" with gains r \" << async_results_.gain_r\n-\t\t\t\t<< \" and b \" << async_results_.gain_b;\n-\t\t}\n+\t\t\t<< \"CT found is \"\n+\t\t\t<< async_results_.temperature_K\n+\t\t\t<< \" with gains r \" << async_results_.gain_r\n+\t\t\t<< \" and b \" << async_results_.gain_b;\n \t}\n }\n \n", "prefixes": [ "libcamera-devel", "2/2" ] }