Patch Detail
Show a patch.
GET /api/patches/20737/?format=api
{ "id": 20737, "url": "https://patchwork.libcamera.org/api/patches/20737/?format=api", "web_url": "https://patchwork.libcamera.org/patch/20737/", "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": "<20240731183616.1977348-4-mzamazal@redhat.com>", "date": "2024-07-31T18:36:16", "name": "[v3,3/3] libcamera: ipa_proxy: Report a missing configuration as a warning", "commit_ref": "6c17996879d0f01cf92fc77431d70723a20be70b", "pull_url": null, "state": "accepted", "archived": false, "hash": "c59b2078290a303013deed1b1d510add7eb99b18", "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/20737/mbox/", "series": [ { "id": 4471, "url": "https://patchwork.libcamera.org/api/series/4471/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=4471", "date": "2024-07-31T18:36:13", "name": "Fix misleading error messages", "version": 3, "mbox": "https://patchwork.libcamera.org/series/4471/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/20737/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/20737/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 B776ABDC71\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 31 Jul 2024 18:36:43 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 52C026337B;\n\tWed, 31 Jul 2024 20:36:43 +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 7DE146198E\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 31 Jul 2024 20:36:40 +0200 (CEST)", "from mx-prod-mc-03.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-187-1WrkZW9gPbuTsrhvnSf05A-1;\n\tWed, 31 Jul 2024 14:36:33 -0400", "from mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com\n\t(mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com [10.30.177.4])\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-03.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTPS id D6FD219560AA; Wed, 31 Jul 2024 18:36:32 +0000 (UTC)", "from nuthatch.redhat.com (unknown [10.45.224.46])\n\tby mx-prod-int-01.mail-002.prod.us-west-2.aws.redhat.com (Postfix)\n\twith ESMTP id EFB45300018D; Wed, 31 Jul 2024 18:36:30 +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=\"cMTZEFNK\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com;\n\ts=mimecast20190719; t=1722450999;\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=YYuwgoj85QfR6JWuJNPLTgilIEd+iBBXF6q3nd2GZ5k=;\n\tb=cMTZEFNKeumzzw2kSOa/N+V6XLgZar/IlMiWGI6Kz0sKV3VPBW3DEpUFx69RbO+ed6+kvB\n\tPli2X5zQgqcLWmtOAUyITv4JFx7MbnzeCDGQZe+GD4EO0p0/5LhscpgwkAom2P+sp3zGFs\n\tYGjOuQAXSZHJmmiCZRpnlL0GH32uOA0=", "X-MC-Unique": "1WrkZW9gPbuTsrhvnSf05A-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 v3 3/3] libcamera: ipa_proxy: Report a missing configuration\n\tas a warning", "Date": "Wed, 31 Jul 2024 20:36:16 +0200", "Message-ID": "<20240731183616.1977348-4-mzamazal@redhat.com>", "In-Reply-To": "<20240731183616.1977348-1-mzamazal@redhat.com>", "References": "<20240731183616.1977348-1-mzamazal@redhat.com>", "MIME-Version": "1.0", "X-Scanned-By": "MIMEDefang 3.4.1 on 10.30.177.4", "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>\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\nReviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n---\n include/libcamera/internal/ipa_proxy.h | 3 ++-\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, 25 insertions(+), 19 deletions(-)", "diff": "diff --git a/include/libcamera/internal/ipa_proxy.h b/include/libcamera/internal/ipa_proxy.h\nindex 5240f69f..0f564d99 100644\n--- a/include/libcamera/internal/ipa_proxy.h\n+++ b/include/libcamera/internal/ipa_proxy.h\n@@ -31,7 +31,8 @@ public:\n \n \tbool isValid() const { return valid_; }\n \n-\tstd::string configurationFile(const std::string &name) const;\n+\tstd::string configurationFile(const std::string &name,\n+\t\t\t\t const 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..69975d8f 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,14 @@ 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(const std::string &name,\n+\t\t\t\t\tconst std::string &fallbackName) const\n {\n \tstruct stat statbuf;\n \tint ret;\n@@ -146,11 +151,18 @@ 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\treturn std::string();\n+\t}\n \n-\treturn std::string();\n+\tLOG(IPAProxy, Warning)\n+\t\t<< \"Configuration file '\" << name\n+\t\t<< \"' not found for IPA module '\" << ipaName\n+\t\t<< \"', falling back to '\" << fallbackName << \"'\";\n+\treturn configurationFile(fallbackName);\n }\n \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 4cbf105d..eec5bf94 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -351,13 +351,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 c8748d88..1140372c 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": [ "v3", "3/3" ] }