Patch Detail
Show a patch.
GET /api/patches/11106/?format=api
{ "id": 11106, "url": "https://patchwork.libcamera.org/api/patches/11106/?format=api", "web_url": "https://patchwork.libcamera.org/patch/11106/", "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": "<20210202170537.11699-1-david.plowman@raspberrypi.com>", "date": "2021-02-02T17:05:37", "name": "[libcamera-devel,v2] ipa: raspberrypi: Warn when control algorithms are missing; do not fail", "commit_ref": "49bae441ed58f05a2c25cf6459afa3aa0839824d", "pull_url": null, "state": "accepted", "archived": false, "hash": "a633ea962fdc8c60e9129d3df3d55c97cd7c1821", "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/11106/mbox/", "series": [ { "id": 1639, "url": "https://patchwork.libcamera.org/api/series/1639/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1639", "date": "2021-02-02T17:05:37", "name": "[libcamera-devel,v2] ipa: raspberrypi: Warn when control algorithms are missing; do not fail", "version": 2, "mbox": "https://patchwork.libcamera.org/series/1639/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/11106/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/11106/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 AB4B3BD161\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue, 2 Feb 2021 17:05:48 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 340AA6842D;\n\tTue, 2 Feb 2021 18:05:48 +0100 (CET)", "from mail-ed1-x52c.google.com (mail-ed1-x52c.google.com\n\t[IPv6:2a00:1450:4864:20::52c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6C0DB60307\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 2 Feb 2021 18:05:46 +0100 (CET)", "by mail-ed1-x52c.google.com with SMTP id d2so23768919edz.3\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 02 Feb 2021 09:05:46 -0800 (PST)", "from pi4-davidp.lan (plowpeople3.plus.com. [80.229.223.72])\n\tby smtp.gmail.com with ESMTPSA id\n\tu16sm9673314ejn.117.2021.02.02.09.05.44\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tTue, 02 Feb 2021 09:05:44 -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=\"k/BVeZ/S\"; 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:mime-version\n\t:content-transfer-encoding;\n\tbh=MRkQGwixRVDHJM7wFkYJttDisg7LFhYxpKvPiXZz7Mo=;\n\tb=k/BVeZ/S/R2P2740XILmx2tcQP4p9lB/Gmm7FzvHQA4qm9Cy87uzt5S7Q8x+Al39tw\n\tU7vI2JYyDUBKeVuYMmBMOdbY+r6gCK97SonRlxIOijO2V+XiaiG8gjeWigsfa7WWYedo\n\tBOkbyWUX8am/Tfe0QnqTGTaBZoiyHSkpIQtZlYk3RtcrrFChjRqpfPu0ApkVSKIPA8mm\n\tjw6GmD/bgjrcSbX/7ItY1xRe/eNxbSxvWsWeUPhMU9aLzjpZdRz8iPntEeEt/+ASQEsC\n\tER5ny0b6GSVafFiKaZ2mUmdefQDyffW59kxAecd+olxtfUv2paCrC9YLJJvXQQjE2ncL\n\tnOfQ==", "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:mime-version\n\t:content-transfer-encoding;\n\tbh=MRkQGwixRVDHJM7wFkYJttDisg7LFhYxpKvPiXZz7Mo=;\n\tb=aSPB+vtyAh12GWnkh8R4IhRtQu3ppdLrQRkukPO6/NIxMkXOHT2HwYOzv1ctK24yOC\n\tbCBBg5LiLQUnY7wnKcVs1CxQF8yIr1V5uq4TyUbVd0kIw43E2zeQgwnbnBbO1CA+zd+U\n\t7lDT7CEzZkUOHZSIzilg7R/HaZ3seMA2dauhkaDTwfxCELVAduTLvpw6iM+/XmOpqgYZ\n\tEVc6G+zKSBdC8yvdzd/zSYGn0BQSALc8vZi6hyrZVPkrZUCHW4aetRrS29/PpG9GRB5j\n\ts80KX+XXAV9NS0zyc7OlTh4GFcWReVW7x852sC6AjFQJxGEIFKQ0W/ldJUpujLlbT+VB\n\ti7Aw==", "X-Gm-Message-State": "AOAM530NZMl3pjuvrvpena0WmQCOaQ3xbYmj1zkdeeLG6JogjskZ+Z3r\n\tON72Ob/r68y5rWEw3siTsa2nIg9SWT+skF+B", "X-Google-Smtp-Source": "ABdhPJyYPbeqFS/dDQ5wzzp5DzqPCi9CG2a75nQek2RFoRHY88pK5cdg8KdAvNxLXCWsNsvLLq6Alw==", "X-Received": "by 2002:a50:c403:: with SMTP id v3mr6349363edf.217.1612285545455;\n\tTue, 02 Feb 2021 09:05:45 -0800 (PST)", "From": "David Plowman <david.plowman@raspberrypi.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Tue, 2 Feb 2021 17:05:37 +0000", "Message-Id": "<20210202170537.11699-1-david.plowman@raspberrypi.com>", "X-Mailer": "git-send-email 2.20.1", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH v2] ipa: raspberrypi: Warn when control\n\talgorithms are missing; do not fail", "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": "Users are free to avoid loading certain control algorithms from the\njson tuning file if they wish, and this could mean that some libcamera\ncontrols will therefore not be available. Currently we don't have a\ngood means of indicating which these are, therefore failing completely\nwhen an application tries to use one is unhelpful - it is better just\nto issue a warning.\n\nNote that once we can indicate this properly, applications should\ncheck for supported controls as this change may be reverted.\n\nSigned-off-by: David Plowman <david.plowman@raspberrypi.com>\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n---\n include/libcamera/ipa/raspberrypi.h | 10 +++-\n src/ipa/raspberrypi/raspberrypi.cpp | 86 ++++++++++++++++++++++++-----\n 2 files changed, 81 insertions(+), 15 deletions(-)", "diff": "diff --git a/include/libcamera/ipa/raspberrypi.h b/include/libcamera/ipa/raspberrypi.h\nindex 5a943382..3500f7df 100644\n--- a/include/libcamera/ipa/raspberrypi.h\n+++ b/include/libcamera/ipa/raspberrypi.h\n@@ -49,7 +49,15 @@ enum BufferMask {\n /* Size of the LS grid allocation. */\n static constexpr unsigned int MaxLsGridSize = 32 << 10;\n \n-/* List of controls handled by the Raspberry Pi IPA */\n+/*\n+ * List of controls handled by the Raspberry Pi IPA\n+ *\n+ * \\todo This list will need to be built dynamically from the control\n+ * algorithms loaded by the json file, once this is supported. At that\n+ * point applications should check first whether a control is supported,\n+ * and the pipeline handler may be reverted so that it aborts when an\n+ * unsupported control is encountered.\n+ */\n static const ControlInfoMap Controls = {\n \t{ &controls::AeEnable, ControlInfo(false, true) },\n \t{ &controls::ExposureTime, ControlInfo(0, 999999) },\ndiff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\nindex 681ab921..8048bb26 100644\n--- a/src/ipa/raspberrypi/raspberrypi.cpp\n+++ b/src/ipa/raspberrypi/raspberrypi.cpp\n@@ -627,7 +627,12 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\tswitch (ctrl.first) {\n \t\tcase controls::AE_ENABLE: {\n \t\t\tRPiController::Algorithm *agc = controller_.GetAlgorithm(\"agc\");\n-\t\t\tASSERT(agc);\n+\t\t\tif (!agc) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set AE_ENABLE - no AGC algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n+\n \t\t\tif (ctrl.second.get<bool>() == false)\n \t\t\t\tagc->Pause();\n \t\t\telse\n@@ -640,7 +645,11 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\tcase controls::EXPOSURE_TIME: {\n \t\t\tRPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(\n \t\t\t\tcontroller_.GetAlgorithm(\"agc\"));\n-\t\t\tASSERT(agc);\n+\t\t\tif (!agc) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set EXPOSURE_TIME - no AGC algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n \n \t\t\t/* This expects units of micro-seconds. */\n \t\t\tagc->SetFixedShutter(ctrl.second.get<int32_t>());\n@@ -652,7 +661,12 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\tcase controls::ANALOGUE_GAIN: {\n \t\t\tRPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(\n \t\t\t\tcontroller_.GetAlgorithm(\"agc\"));\n-\t\t\tASSERT(agc);\n+\t\t\tif (!agc) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set ANALOGUE_GAIN - no AGC algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n+\n \t\t\tagc->SetFixedAnalogueGain(ctrl.second.get<float>());\n \n \t\t\tlibcameraMetadata_.set(controls::AnalogueGain,\n@@ -663,7 +677,11 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\tcase controls::AE_METERING_MODE: {\n \t\t\tRPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(\n \t\t\t\tcontroller_.GetAlgorithm(\"agc\"));\n-\t\t\tASSERT(agc);\n+\t\t\tif (!agc) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set AE_METERING_MODE - no AGC algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n \n \t\t\tint32_t idx = ctrl.second.get<int32_t>();\n \t\t\tif (MeteringModeTable.count(idx)) {\n@@ -679,7 +697,11 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\tcase controls::AE_CONSTRAINT_MODE: {\n \t\t\tRPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(\n \t\t\t\tcontroller_.GetAlgorithm(\"agc\"));\n-\t\t\tASSERT(agc);\n+\t\t\tif (!agc) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set AE_CONSTRAINT_MODE - no AGC algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n \n \t\t\tint32_t idx = ctrl.second.get<int32_t>();\n \t\t\tif (ConstraintModeTable.count(idx)) {\n@@ -695,7 +717,11 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\tcase controls::AE_EXPOSURE_MODE: {\n \t\t\tRPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(\n \t\t\t\tcontroller_.GetAlgorithm(\"agc\"));\n-\t\t\tASSERT(agc);\n+\t\t\tif (!agc) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set AE_EXPOSURE_MODE - no AGC algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n \n \t\t\tint32_t idx = ctrl.second.get<int32_t>();\n \t\t\tif (ExposureModeTable.count(idx)) {\n@@ -711,7 +737,11 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\tcase controls::EXPOSURE_VALUE: {\n \t\t\tRPiController::AgcAlgorithm *agc = dynamic_cast<RPiController::AgcAlgorithm *>(\n \t\t\t\tcontroller_.GetAlgorithm(\"agc\"));\n-\t\t\tASSERT(agc);\n+\t\t\tif (!agc) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set EXPOSURE_VALUE - no AGC algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n \n \t\t\t/*\n \t\t\t * The SetEv() method takes in a direct exposure multiplier.\n@@ -726,7 +756,11 @@ void IPARPi::queueRequest(const ControlList &controls)\n \n \t\tcase controls::AWB_ENABLE: {\n \t\t\tRPiController::Algorithm *awb = controller_.GetAlgorithm(\"awb\");\n-\t\t\tASSERT(awb);\n+\t\t\tif (!awb) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set AWB_ENABLE - no AWB algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n \n \t\t\tif (ctrl.second.get<bool>() == false)\n \t\t\t\tawb->Pause();\n@@ -741,7 +775,11 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\tcase controls::AWB_MODE: {\n \t\t\tRPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>(\n \t\t\t\tcontroller_.GetAlgorithm(\"awb\"));\n-\t\t\tASSERT(awb);\n+\t\t\tif (!awb) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set AWB_MODE - no AWB algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n \n \t\t\tint32_t idx = ctrl.second.get<int32_t>();\n \t\t\tif (AwbModeTable.count(idx)) {\n@@ -758,7 +796,11 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\t\tauto gains = ctrl.second.get<Span<const float>>();\n \t\t\tRPiController::AwbAlgorithm *awb = dynamic_cast<RPiController::AwbAlgorithm *>(\n \t\t\t\tcontroller_.GetAlgorithm(\"awb\"));\n-\t\t\tASSERT(awb);\n+\t\t\tif (!awb) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set COLOUR_GAINS - no AWB algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n \n \t\t\tawb->SetManualGains(gains[0], gains[1]);\n \t\t\tif (gains[0] != 0.0f && gains[1] != 0.0f)\n@@ -771,7 +813,11 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\tcase controls::BRIGHTNESS: {\n \t\t\tRPiController::ContrastAlgorithm *contrast = dynamic_cast<RPiController::ContrastAlgorithm *>(\n \t\t\t\tcontroller_.GetAlgorithm(\"contrast\"));\n-\t\t\tASSERT(contrast);\n+\t\t\tif (!contrast) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set BRIGHTNESS - no contrast algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n \n \t\t\tcontrast->SetBrightness(ctrl.second.get<float>() * 65536);\n \t\t\tlibcameraMetadata_.set(controls::Brightness,\n@@ -782,7 +828,11 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\tcase controls::CONTRAST: {\n \t\t\tRPiController::ContrastAlgorithm *contrast = dynamic_cast<RPiController::ContrastAlgorithm *>(\n \t\t\t\tcontroller_.GetAlgorithm(\"contrast\"));\n-\t\t\tASSERT(contrast);\n+\t\t\tif (!contrast) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set CONTRAST - no contrast algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n \n \t\t\tcontrast->SetContrast(ctrl.second.get<float>());\n \t\t\tlibcameraMetadata_.set(controls::Contrast,\n@@ -793,7 +843,11 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\tcase controls::SATURATION: {\n \t\t\tRPiController::CcmAlgorithm *ccm = dynamic_cast<RPiController::CcmAlgorithm *>(\n \t\t\t\tcontroller_.GetAlgorithm(\"ccm\"));\n-\t\t\tASSERT(ccm);\n+\t\t\tif (!ccm) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set SATURATION - no ccm algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n \n \t\t\tccm->SetSaturation(ctrl.second.get<float>());\n \t\t\tlibcameraMetadata_.set(controls::Saturation,\n@@ -804,7 +858,11 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t\tcase controls::SHARPNESS: {\n \t\t\tRPiController::SharpenAlgorithm *sharpen = dynamic_cast<RPiController::SharpenAlgorithm *>(\n \t\t\t\tcontroller_.GetAlgorithm(\"sharpen\"));\n-\t\t\tASSERT(sharpen);\n+\t\t\tif (!sharpen) {\n+\t\t\t\tLOG(IPARPI, Warning)\n+\t\t\t\t\t<< \"Could not set SHARPNESS - no sharpen algorithm\";\n+\t\t\t\tbreak;\n+\t\t\t}\n \n \t\t\tsharpen->SetStrength(ctrl.second.get<float>());\n \t\t\tlibcameraMetadata_.set(controls::Sharpness,\n", "prefixes": [ "libcamera-devel", "v2" ] }