Patch Detail
Show a patch.
GET /api/patches/20616/?format=api
{ "id": 20616, "url": "https://patchwork.libcamera.org/api/patches/20616/?format=api", "web_url": "https://patchwork.libcamera.org/patch/20616/", "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": "<20240708123803.1006689-4-mzamazal@redhat.com>", "date": "2024-07-08T12:38:03", "name": "[v2,3/3] libcamera: ipa_proxy: Report a missing configuration as a warning", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "5ec690e0f328ce76a1e9e209497197e5d33a9f09", "submitter": { "id": 177, "url": "https://patchwork.libcamera.org/api/people/177/?format=api", "name": "Milan Zamazal", "email": "mzamazal@redhat.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/20616/mbox/", "series": [ { "id": 4447, "url": "https://patchwork.libcamera.org/api/series/4447/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4447", "date": "2024-07-08T12:38:00", "name": "Fix misleading error messages", "version": 2, "mbox": "https://patchwork.libcamera.org/series/4447/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/20616/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/20616/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 7B820BD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 8 Jul 2024 12:38:32 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DB9A163374;\n\tMon, 8 Jul 2024 14:38:31 +0200 (CEST)", "from us-smtp-delivery-124.mimecast.com\n\t(us-smtp-delivery-124.mimecast.com [170.10.129.124])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5746A63370\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 8 Jul 2024 14:38:28 +0200 (CEST)", "from mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com\n\t(ec2-54-186-198-63.us-west-2.compute.amazonaws.com [54.186.198.63])\n\tby relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3,\n\tcipher=TLS_AES_256_GCM_SHA384) id us-mta-245-eEja4cAPPTCy4szZIm8OAw-1;\n\tMon, 08 Jul 2024 08:38:23 -0400", "from mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com\n\t(mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com\n\t[10.30.177.17])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\tkey-exchange X25519 server-signature RSA-PSS (2048 bits)\n\tserver-digest SHA256) (No client certificate requested)\n\tby mx-prod-mc-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTPS id CAC0A1935D3C; Mon, 8 Jul 2024 12:38:22 +0000 (UTC)", "from nuthatch.redhat.com (unknown [10.45.226.113])\n\tby mx-prod-int-05.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTP id 167031955F40; Mon, 8 Jul 2024 12:38:20 +0000 (UTC)" ], "Authentication-Results": "lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=redhat.com header.i=@redhat.com\n\theader.b=\"Ayab6SbQ\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1720442307;\n\th=from:from:reply-to:subject:subject:date:date:message-id:message-id:\n\tto:to:cc:cc:mime-version:mime-version:content-type:content-type:\n\tcontent-transfer-encoding:content-transfer-encoding:\n\tin-reply-to:in-reply-to:references:references;\n\tbh=jc2bRj816/5fOUr3Z2Kx47xMNlzbk8806TxGsL0uXFI=;\n\tb=Ayab6SbQPyVMYIqznqQOPpYYoiTL9f57ws2GMzAmz7wudKKuGY8UpM0ARTpVF83RQrtEDI\n\tlKcC9SAkMFt+TxBbFVRegBqFyLo3alY57+gMfQfxoD2A8zLC96M5R/qXskY3ZDQ6HqsfL3\n\t3XEGeNXvnvz0BtJrbsri/B64ZWtVqeU=", "X-MC-Unique": "eEja4cAPPTCy4szZIm8OAw-1", "From": "Milan Zamazal <mzamazal@redhat.com>", "To": "libcamera-devel@lists.libcamera.org", "Cc": "Milan Zamazal <mzamazal@redhat.com>,\n\tKieran Bingham <kieran.bingham@ideasonboard.com>,\n\tLaurent Pinchart <laurent.pinchart@ideasonboard.com>", "Subject": "[PATCH v2 3/3] libcamera: ipa_proxy: Report a missing configuration\n\tas a warning", "Date": "Mon, 8 Jul 2024 14:38:03 +0200", "Message-ID": "<20240708123803.1006689-4-mzamazal@redhat.com>", "In-Reply-To": "<20240708123803.1006689-1-mzamazal@redhat.com>", "References": "<20240708123803.1006689-1-mzamazal@redhat.com>", "MIME-Version": "1.0", "X-Scanned-By": "MIMEDefang 3.0 on 10.30.177.17", "X-Mimecast-Spam-Score": "0", "X-Mimecast-Originator": "redhat.com", "Content-Transfer-Encoding": "8bit", "Content-Type": "text/plain; charset=\"US-ASCII\"; x-default=true", "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>", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "When the configuration file for an IPA module is missing, it is reported\nas an error in the log, for example:\n\n ERROR IPAProxy ipa_proxy.cpp:149 Configuration file 'imx219.yaml' not found for IPA module 'simple'\n\nThis is misleading because several pipelines use uncalibrated.yaml in\nsuch a case and can continue working. And in case of software ISP,\nthere is currently no other configuration file so the error is always\nreported.\n\nOn the other hand, in some other cases the presence of the configuration\nfile is required and it is an error if it is missing.\n\nLet's introduce a new optional argument to IPAProxy::configurationFile\nthat specifies a fallback file if the requested file is not found. If\nthe primary requested file is not found and a non-empty fallback file is\nspecified then a warning is logged and the fallback file is looked up.\nIf neither the fallback file can be found then only then an error is\nlogged and the method returns an empty string. This change has also the\nbenefit of putting the common fallback file (\"uncalibrated.yaml\")\npattern to a single place.\n\nSigned-off-by: Milan Zamazal <mzamazal@redhat.com>\n---\n include/libcamera/internal/ipa_proxy.h | 4 +++-\n src/libcamera/ipa_proxy.cpp | 22 +++++++++++++++++----\n src/libcamera/pipeline/ipu3/ipu3.cpp | 5 ++---\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 ++-------\n src/libcamera/software_isp/software_isp.cpp | 5 ++---\n 5 files changed, 27 insertions(+), 18 deletions(-)", "diff": "diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h\nindex 5240f69f..88068de7 100644\n--- a/include/libcamera/internal/ipa_proxy.h\n+++ b/include/libcamera/internal/ipa_proxy.h\n@@ -31,7 +31,9 @@ public:\n \n \tbool isValid() const { return valid_; }\n \n-\tstd::string configurationFile(const std::string &name) const;\n+\tstd::string configurationFile(\n+\t\tconst std::string &name,\n+\t\tconst std::string &fallbackName = std::string()) const;\n \n protected:\n \tstd::string resolvePath(const std::string &file) const;\ndiff --git a/src/libcamera/ipa_proxy.cpp b/src/libcamera/ipa_proxy.cpp\nindex 6c17c456..247c6fa0 100644\n--- a/src/libcamera/ipa_proxy.cpp\n+++ b/src/libcamera/ipa_proxy.cpp\n@@ -72,6 +72,7 @@ IPAProxy::~IPAProxy()\n /**\n * \\brief Retrieve the absolute path to an IPA configuration file\n * \\param[in] name The configuration file name\n+ * \\param[in] fallbackName The name of a fallback configuration file\n *\n * This function locates the configuration file for an IPA and returns its\n * absolute path. It searches the following directories, in order:\n@@ -89,10 +90,15 @@ IPAProxy::~IPAProxy()\n * named after the IPA module name, as reported in IPAModuleInfo::name, and for\n * a file named \\a name within that directory. The \\a name is IPA-specific.\n *\n+ * If the file named \\a name is not found and \\a fallbackName is non-empty then\n+ * the whole search is repeated for \\a fallbackName.\n+ *\n * \\return The full path to the IPA configuration file, or an empty string if\n * no configuration file can be found\n */\n-std::string IPAProxy::configurationFile(const std::string &name) const\n+std::string IPAProxy::configurationFile(\n+\tconst std::string &name,\n+\tconst std::string &fallbackName) const\n {\n \tstruct stat statbuf;\n \tint ret;\n@@ -146,9 +152,17 @@ std::string IPAProxy::configurationFile(const std::string &name) const\n \t\t}\n \t}\n \n-\tLOG(IPAProxy, Error)\n-\t\t<< \"Configuration file '\" << name\n-\t\t<< \"' not found for IPA module '\" << ipaName << \"'\";\n+\tif (fallbackName.empty()) {\n+\t\tLOG(IPAProxy, Error)\n+\t\t\t<< \"Configuration file '\" << name\n+\t\t\t<< \"' not found for IPA module '\" << ipaName << \"'\";\n+\t} else {\n+\t\tLOG(IPAProxy, Warning)\n+\t\t\t<< \"Configuration file '\" << name\n+\t\t\t<< \"' not found for IPA module '\" << ipaName\n+\t\t\t<< \"', trying '\" << fallbackName << \"'\";\n+\t\treturn configurationFile(fallbackName);\n+\t}\n \n \treturn std::string();\n }\ndiff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\nindex 066fd4a2..2071c338 100644\n--- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n+++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n@@ -1186,9 +1186,8 @@ int IPU3CameraData::loadIPA()\n \t * The API tuning file is made from the sensor name. If the tuning file\n \t * isn't found, fall back to the 'uncalibrated' file.\n \t */\n-\tstd::string ipaTuningFile = ipa_->configurationFile(sensor->model() + \".yaml\");\n-\tif (ipaTuningFile.empty())\n-\t\tipaTuningFile = ipa_->configurationFile(\"uncalibrated.yaml\");\n+\tstd::string ipaTuningFile =\n+\t\tipa_->configurationFile(sensor->model() + \".yaml\", \"uncalibrated.yaml\");\n \n \tret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() },\n \t\t\t sensorInfo, sensor->controls(), &ipaControls_);\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 97cd78a7..c3dae003 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -350,13 +350,8 @@ int RkISP1CameraData::loadIPA(unsigned int hwRevision)\n \tstd::string ipaTuningFile;\n \tchar const *configFromEnv = utils::secure_getenv(\"LIBCAMERA_RKISP1_TUNING_FILE\");\n \tif (!configFromEnv || *configFromEnv == '\\0') {\n-\t\tipaTuningFile = ipa_->configurationFile(sensor_->model() + \".yaml\");\n-\t\t/*\n-\t\t * If the tuning file isn't found, fall back to the\n-\t\t * 'uncalibrated' configuration file.\n-\t\t */\n-\t\tif (ipaTuningFile.empty())\n-\t\t\tipaTuningFile = ipa_->configurationFile(\"uncalibrated.yaml\");\n+\t\tipaTuningFile =\n+\t\t\tipa_->configurationFile(sensor_->model() + \".yaml\", \"uncalibrated.yaml\");\n \t} else {\n \t\tipaTuningFile = std::string(configFromEnv);\n \t}\ndiff --git a/src/libcamera/software_isp/software_isp.cpp b/src/libcamera/software_isp/software_isp.cpp\nindex 20fb6f48..aeef28e8 100644\n--- a/src/libcamera/software_isp/software_isp.cpp\n+++ b/src/libcamera/software_isp/software_isp.cpp\n@@ -121,9 +121,8 @@ SoftwareIsp::SoftwareIsp(PipelineHandler *pipe, const CameraSensor *sensor)\n \t * The API tuning file is made from the sensor name. If the tuning file\n \t * isn't found, fall back to the 'uncalibrated' file.\n \t */\n-\tstd::string ipaTuningFile = ipa_->configurationFile(sensor->model() + \".yaml\");\n-\tif (ipaTuningFile.empty())\n-\t\tipaTuningFile = ipa_->configurationFile(\"uncalibrated.yaml\");\n+\tstd::string ipaTuningFile =\n+\t\tipa_->configurationFile(sensor->model() + \".yaml\", \"uncalibrated.yaml\");\n \n \tint ret = ipa_->init(IPASettings{ ipaTuningFile, sensor->model() },\n \t\t\t debayer_->getStatsFD(),\n", "prefixes": [ "v2", "3/3" ] }