Patch Detail
Show a patch.
GET /api/patches/10562/?format=api
{ "id": 10562, "url": "https://patchwork.libcamera.org/api/patches/10562/?format=api", "web_url": "https://patchwork.libcamera.org/patch/10562/", "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": "<20201204153121.66136-3-naush@raspberrypi.com>", "date": "2020-12-04T15:31:20", "name": "[libcamera-devel,v3,2/3] libcamera: ipa: Pass a set of controls and return results from ipa::start()", "commit_ref": null, "pull_url": null, "state": "accepted", "archived": false, "hash": "5b829bd251496e507afb59acadb67f1f7565967d", "submitter": { "id": 34, "url": "https://patchwork.libcamera.org/api/people/34/?format=api", "name": "Naushir Patuck", "email": "naush@raspberrypi.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/10562/mbox/", "series": [ { "id": 1505, "url": "https://patchwork.libcamera.org/api/series/1505/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1505", "date": "2020-12-04T15:31:18", "name": "Pass controls on camera:start()", "version": 3, "mbox": "https://patchwork.libcamera.org/series/1505/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/10562/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/10562/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 137E2BE178\n\tfor <parsemail@patchwork.libcamera.org>;\n\tFri, 4 Dec 2020 15:31:31 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5F48B635FA;\n\tFri, 4 Dec 2020 16:31:30 +0100 (CET)", "from mail-wr1-x42f.google.com (mail-wr1-x42f.google.com\n\t[IPv6:2a00:1450:4864:20::42f])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 457CC635D6\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 4 Dec 2020 16:31:29 +0100 (CET)", "by mail-wr1-x42f.google.com with SMTP id g14so5690012wrm.13\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 04 Dec 2020 07:31:29 -0800 (PST)", "from naushir-VirtualBox.pitowers.org\n\t([2a00:1098:3142:14:a00:27ff:fe4d:f6a2])\n\tby smtp.gmail.com with ESMTPSA id\n\tx66sm3465426wmg.26.2020.12.04.07.31.26\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 04 Dec 2020 07:31:26 -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=\"PhrssKzj\"; 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=JNvPjsKqf/Ia/ymHimIOfo8y4FhZtcQ109ptuF8ZPzE=;\n\tb=PhrssKzjAScSl1m7RmmQbst8C74ANFrlNJIrt5VlsSegMcENPc4Y8ZLZKSGNwAcqqQ\n\t5sIhLDL2h72NerlJeyLcd7rXpw7vwOUFie2cVFX4VPOCT3+AyHSsnqje4KAhd/1JeJy3\n\trDtXzvnmJMcQ54e1xht49EVfw4u969XJR4AduKOJaRR1dTxMiOPgS3Hf7FTMYwfTodb4\n\tHSIul2qtsex8hGiZtezWL+JosGuvW0idMbkjX1Y6xP+nX9oahmORJRRwXRuk+J7VSA6j\n\tgSOPru0GSApPYBuxkbblmRbgbtGAwsqR5UCpWKZFBuZm/duk4DeMTwzPPuiBsyjVgSIR\n\trSxg==", "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=JNvPjsKqf/Ia/ymHimIOfo8y4FhZtcQ109ptuF8ZPzE=;\n\tb=kpkhzxIyCQaWM08Lzl7uDuqBtfjYGeIc4nduJg6KoGxhHXsrzsJlSXmVzZMuln54g1\n\tfdE3+UEt7ItmTt6bVzwBaovxVomYLohR/csM063B3UVcUbQAS1G87IMgg0CW9dhydoIr\n\txa1mi7t2vj6E7nu1d51/1MwgDXc0XMBS3kSxVnwD3MKpb+/ih4dajDUE5vDtbgMaMVY+\n\tFB/bUJjsCCXQZL3rDxLdFURg11z7s0x3TefbmR4j21JS2FP8Ot40j0Q6F6Car+JoGIYT\n\t7atzEFucsWXtgwVVoZcpIzAv6URRx7rZDE+IdtReIw1huuKkfw4QtSTz5+db6vrM4HAc\n\tn2+A==", "X-Gm-Message-State": "AOAM5308Q7ukoilyVtnRpRxlYS98cqktB6Zoxf1EUzv0PX7OorXImmVa\n\t6+R853D30E1UySGIkngR9R2YjlRE5kT7fQ==", "X-Google-Smtp-Source": "ABdhPJxSTpZm06d7NW8RrkLwj1zNROjNI56vySwQeHIqZC2TUUOwBXSSpDGSYwbpqNGMHsz/PpULOg==", "X-Received": "by 2002:adf:f9c5:: with SMTP id w5mr5452508wrr.69.1607095887171; \n\tFri, 04 Dec 2020 07:31:27 -0800 (PST)", "From": "Naushir Patuck <naush@raspberrypi.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Fri, 4 Dec 2020 15:31:20 +0000", "Message-Id": "<20201204153121.66136-3-naush@raspberrypi.com>", "X-Mailer": "git-send-email 2.25.1", "In-Reply-To": "<20201204153121.66136-1-naush@raspberrypi.com>", "References": "<20201204153121.66136-1-naush@raspberrypi.com>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [PATCH v3 2/3] libcamera: ipa: Pass a set of\n\tcontrols and return results from ipa::start()", "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": "This change allows controls passed into PipelineHandler::start to be\nforwarded onto IPAInterface::start(). We also add a return channel if the\npipeline handler must action any of these controls, e.g. setting the\nanalogue gain or shutter speed in the sensor device.\n\ntodo: The IPA interface wrapper needs a translation for passing\nIPAOperationData into start() and configure()\n\nSigned-off-by: Naushir Patuck <naush@raspberrypi.com>\nReviewed-by: David Plowman <david.plowman@raspberrypi.com>\nTested-by: David Plowman <david.plowman@raspberrypi.com>\n---\n include/libcamera/internal/ipa_context_wrapper.h | 3 ++-\n include/libcamera/ipa/ipa_interface.h | 3 ++-\n src/ipa/libipa/ipa_interface_wrapper.cpp | 4 +++-\n src/ipa/raspberrypi/raspberrypi.cpp | 5 +++--\n src/ipa/rkisp1/rkisp1.cpp | 3 ++-\n src/ipa/vimc/vimc.cpp | 6 ++++--\n src/libcamera/ipa_context_wrapper.cpp | 5 +++--\n src/libcamera/ipa_interface.cpp | 7 +++++++\n src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 7 +++++--\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 3 ++-\n src/libcamera/pipeline/vimc/vimc.cpp | 3 ++-\n src/libcamera/proxy/ipa_proxy_linux.cpp | 3 ++-\n src/libcamera/proxy/ipa_proxy_thread.cpp | 13 ++++++++-----\n test/ipa/ipa_interface_test.cpp | 3 ++-\n test/ipa/ipa_wrappers_test.cpp | 5 +++--\n 15 files changed, 50 insertions(+), 23 deletions(-)", "diff": "diff --git a/include/libcamera/internal/ipa_context_wrapper.h b/include/libcamera/internal/ipa_context_wrapper.h\nindex 8f767e84..ec27e48e 100644\n--- a/include/libcamera/internal/ipa_context_wrapper.h\n+++ b/include/libcamera/internal/ipa_context_wrapper.h\n@@ -20,7 +20,8 @@ public:\n \t~IPAContextWrapper();\n \n \tint init(const IPASettings &settings) override;\n-\tint start() override;\n+\tint start(const IPAOperationData &data,\n+\t\t IPAOperationData *result) override;\n \tvoid stop() override;\n \tvoid configure(const CameraSensorInfo &sensorInfo,\n \t\t const std::map<unsigned int, IPAStream> &streamConfig,\ndiff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\nindex 322b7079..df12f9ce 100644\n--- a/include/libcamera/ipa/ipa_interface.h\n+++ b/include/libcamera/ipa/ipa_interface.h\n@@ -153,7 +153,8 @@ public:\n \tvirtual ~IPAInterface() = default;\n \n \tvirtual int init(const IPASettings &settings) = 0;\n-\tvirtual int start() = 0;\n+\tvirtual int start(const IPAOperationData &data,\n+\t\t\t IPAOperationData *result) = 0;\n \tvirtual void stop() = 0;\n \n \tvirtual void configure(const CameraSensorInfo &sensorInfo,\ndiff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp\nindex cee532e3..40628489 100644\n--- a/src/ipa/libipa/ipa_interface_wrapper.cpp\n+++ b/src/ipa/libipa/ipa_interface_wrapper.cpp\n@@ -95,7 +95,9 @@ int IPAInterfaceWrapper::start(struct ipa_context *_ctx)\n {\n \tIPAInterfaceWrapper *ctx = static_cast<IPAInterfaceWrapper *>(_ctx);\n \n-\treturn ctx->ipa_->start();\n+\t/* \\todo Translate the data and result. */\n+\tIPAOperationData data = {};\n+\treturn ctx->ipa_->start(data, nullptr);\n }\n \n void IPAInterfaceWrapper::stop(struct ipa_context *_ctx)\ndiff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\nindex 29d48b1b..b8c0f955 100644\n--- a/src/ipa/raspberrypi/raspberrypi.cpp\n+++ b/src/ipa/raspberrypi/raspberrypi.cpp\n@@ -78,13 +78,14 @@ public:\n \t}\n \n \tint init(const IPASettings &settings) override;\n-\tint start() override { return 0; }\n+\tint start([[maybe_unused]] const IPAOperationData &data,\n+\t\t [[maybe_unused]] IPAOperationData *result) override { return 0; }\n \tvoid stop() override {}\n \n \tvoid configure(const CameraSensorInfo &sensorInfo,\n \t\t const std::map<unsigned int, IPAStream> &streamConfig,\n \t\t const std::map<unsigned int, const ControlInfoMap &> &entityControls,\n-\t\t const IPAOperationData &data,\n+\t\t const IPAOperationData &ipaConfig,\n \t\t IPAOperationData *response) override;\n \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\ndiff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp\nindex 07d7f1b2..39783abd 100644\n--- a/src/ipa/rkisp1/rkisp1.cpp\n+++ b/src/ipa/rkisp1/rkisp1.cpp\n@@ -37,7 +37,8 @@ public:\n \t{\n \t\treturn 0;\n \t}\n-\tint start() override { return 0; }\n+\tint start([[maybe_unused]] const IPAOperationData &data,\n+\t\t [[maybe_unused]] IPAOperationData *result) override { return 0; }\n \tvoid stop() override {}\n \n \tvoid configure(const CameraSensorInfo &info,\ndiff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\nindex cf841135..074902ee 100644\n--- a/src/ipa/vimc/vimc.cpp\n+++ b/src/ipa/vimc/vimc.cpp\n@@ -34,7 +34,8 @@ public:\n \n \tint init(const IPASettings &settings) override;\n \n-\tint start() override;\n+\tint start(const IPAOperationData &data,\n+\t\t IPAOperationData *result) override;\n \tvoid stop() override;\n \n \tvoid configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,\n@@ -82,7 +83,8 @@ int IPAVimc::init(const IPASettings &settings)\n \treturn 0;\n }\n \n-int IPAVimc::start()\n+int IPAVimc::start([[maybe_unused]] const IPAOperationData &data,\n+\t\t [[maybe_unused]] IPAOperationData *result)\n {\n \ttrace(IPAOperationStart);\n \ndiff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp\nindex 231300ce..19c44ad8 100644\n--- a/src/libcamera/ipa_context_wrapper.cpp\n+++ b/src/libcamera/ipa_context_wrapper.cpp\n@@ -86,10 +86,11 @@ int IPAContextWrapper::init(const IPASettings &settings)\n \treturn 0;\n }\n \n-int IPAContextWrapper::start()\n+int IPAContextWrapper::start(const IPAOperationData &data,\n+\t\t\t IPAOperationData *result)\n {\n \tif (intf_)\n-\t\treturn intf_->start();\n+\t\treturn intf_->start(data, result);\n \n \tif (!ctx_)\n \t\treturn 0;\ndiff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\nindex 23fc56d7..5be6f787 100644\n--- a/src/libcamera/ipa_interface.cpp\n+++ b/src/libcamera/ipa_interface.cpp\n@@ -536,10 +536,17 @@ namespace libcamera {\n /**\n * \\fn IPAInterface::start()\n * \\brief Start the IPA\n+ * \\param[in] data Protocol-specific data for the start operation\n+ * \\param[out] result Result of the start operation\n *\n * This method informs the IPA module that the camera is about to be started.\n * The IPA module shall prepare any resources it needs to operate.\n *\n+ * The \\a data and \\a result parameters carry custom data passed by the\n+ * pipeline handler to the IPA and back. The pipeline handler may set the \\a\n+ * result parameter to null if the IPA protocol doesn't need to pass a result\n+ * back through the start() function.\n+ *\n * \\return 0 on success or a negative error code otherwise\n */\n \ndiff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\nindex 9937db73..bafd0b2d 100644\n--- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n+++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n@@ -752,7 +752,10 @@ int PipelineHandlerRPi::start(Camera *camera, [[maybe_unused]] ControlList *cont\n \t}\n \n \t/* Start the IPA. */\n-\tret = data->ipa_->start();\n+\tIPAOperationData ipaData = {}, result = {};\n+\tif (controls)\n+\t\tipaData.controls.emplace_back(*controls);\n+\tret = data->ipa_->start(ipaData, &result);\n \tif (ret) {\n \t\tLOG(RPI, Error)\n \t\t\t<< \"Failed to start IPA for \" << camera->id();\n@@ -1189,7 +1192,7 @@ int RPiCameraData::configureIPA(const CameraConfiguration *config)\n \t}\n \n \t/* Ready the IPA - it must know about the sensor resolution. */\n-\tIPAOperationData result;\n+\tIPAOperationData result = {};\n \n \tipa_->configure(sensorInfo_, streamConfig, entityControls, ipaConfig,\n \t\t\t&result);\ndiff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\nindex 4e959fde..eaa10f9f 100644\n--- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n+++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n@@ -842,7 +842,8 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] ControlList *c\n \tif (ret)\n \t\treturn ret;\n \n-\tret = data->ipa_->start();\n+\tIPAOperationData ipaData = {};\n+\tret = data->ipa_->start(ipaData, nullptr);\n \tif (ret) {\n \t\tfreeBuffers(camera);\n \t\tLOG(RkISP1, Error)\ndiff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\nindex d81b8598..2a5054a8 100644\n--- a/src/libcamera/pipeline/vimc/vimc.cpp\n+++ b/src/libcamera/pipeline/vimc/vimc.cpp\n@@ -322,7 +322,8 @@ int PipelineHandlerVimc::start(Camera *camera, [[maybe_unused]] ControlList *con\n \tif (ret < 0)\n \t\treturn ret;\n \n-\tret = data->ipa_->start();\n+\tIPAOperationData ipaData = {};\n+\tret = data->ipa_->start(ipaData, nullptr);\n \tif (ret) {\n \t\tdata->video_->releaseBuffers();\n \t\treturn ret;\ndiff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\nindex b78a0e45..ea6f3e5e 100644\n--- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n+++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n@@ -30,7 +30,8 @@ public:\n \t{\n \t\treturn 0;\n \t}\n-\tint start() override { return 0; }\n+\tint start([[maybe_unused]] const IPAOperationData &data,\n+\t\t [[maybe_unused]] IPAOperationData *result) override { return 0; }\n \tvoid stop() override {}\n \tvoid configure([[maybe_unused]] const CameraSensorInfo &sensorInfo,\n \t\t [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,\ndiff --git a/src/libcamera/proxy/ipa_proxy_thread.cpp b/src/libcamera/proxy/ipa_proxy_thread.cpp\nindex eead2883..a5fda2c8 100644\n--- a/src/libcamera/proxy/ipa_proxy_thread.cpp\n+++ b/src/libcamera/proxy/ipa_proxy_thread.cpp\n@@ -26,7 +26,8 @@ public:\n \tIPAProxyThread(IPAModule *ipam);\n \n \tint init(const IPASettings &settings) override;\n-\tint start() override;\n+\tint start(const IPAOperationData &data,\n+\t\t IPAOperationData *result) override;\n \tvoid stop() override;\n \n \tvoid configure(const CameraSensorInfo &sensorInfo,\n@@ -50,9 +51,9 @@ private:\n \t\t\tipa_ = ipa;\n \t\t}\n \n-\t\tint start()\n+\t\tint start(const IPAOperationData &data, IPAOperationData *result)\n \t\t{\n-\t\t\treturn ipa_->start();\n+\t\t\treturn ipa_->start(data, result);\n \t\t}\n \n \t\tvoid stop()\n@@ -111,12 +112,14 @@ int IPAProxyThread::init(const IPASettings &settings)\n \treturn 0;\n }\n \n-int IPAProxyThread::start()\n+int IPAProxyThread::start(const IPAOperationData &data,\n+\t\t\t IPAOperationData *result)\n {\n \trunning_ = true;\n \tthread_.start();\n \n-\treturn proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking);\n+\treturn proxy_.invokeMethod(&ThreadProxy::start, ConnectionTypeBlocking,\n+\t\t\t\t data, result);\n }\n \n void IPAProxyThread::stop()\ndiff --git a/test/ipa/ipa_interface_test.cpp b/test/ipa/ipa_interface_test.cpp\nindex 9f575f93..dbb6727f 100644\n--- a/test/ipa/ipa_interface_test.cpp\n+++ b/test/ipa/ipa_interface_test.cpp\n@@ -120,7 +120,8 @@ protected:\n \t\t}\n \n \t\t/* Test start of IPA module. */\n-\t\tipa_->start();\n+\t\tIPAOperationData data = {};\n+\t\tipa_->start(data, nullptr);\n \t\ttimer.start(1000);\n \t\twhile (timer.isRunning() && trace_ != IPAOperationStart)\n \t\t\tdispatcher->processEvents();\ndiff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp\nindex 59d991cb..47533d10 100644\n--- a/test/ipa/ipa_wrappers_test.cpp\n+++ b/test/ipa/ipa_wrappers_test.cpp\n@@ -56,7 +56,8 @@ public:\n \t\treturn 0;\n \t}\n \n-\tint start() override\n+\tint start([[maybe_unused]] const IPAOperationData &data,\n+\t\t [[maybe_unused]] IPAOperationData *result) override\n \t{\n \t\treport(Op_start, TestPass);\n \t\treturn 0;\n@@ -376,7 +377,7 @@ protected:\n \t\t\treturn TestFail;\n \t\t}\n \n-\t\tret = INVOKE(start);\n+\t\tret = INVOKE(start, ipaConfig, nullptr);\n \t\tif (ret == TestFail) {\n \t\t\tcerr << \"Failed to run start()\";\n \t\t\treturn TestFail;\n", "prefixes": [ "libcamera-devel", "v3", "2/3" ] }