[{"id":17271,"web_url":"https://patchwork.libcamera.org/comment/17271/","msgid":"","date":"2021-05-26T12:07:35","subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Rectify ControlInfoMap\n\tmatching in IPAConfigInfo","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Wed, May 26, 2021 at 05:18:14PM +0530, Umang Jain wrote:\n> The ControlInfoMap of entityControls member in IPAConfigInfo struct,\n> was not able to correctly match to the ControlInfoMap defined in\n> core.mojom. This resulted in a FATAL breakage when IPU3 IPA is meant to\n> run:\n> \n> FATAL IPADataSerializer ipa_data_serializer.cpp:437 ControlSerializer\n> not provided for serialization of ControlInfoMap\n\nFixes: c76ca01323d8 (\"ipa: ipu3: Introduce IPAConfigInfo in IPC\")\n\n> Signed-off-by: Umang Jain \n\nReviewed-by: Laurent Pinchart \n\nAs this fixes a breakage in master, I'll merge it right away.\n\nPaul, is there a way this issue could have been detected at compile time\n? If that's not doable, could you send at least a documentation patch ?\n\n> ---\n> include/libcamera/ipa/ipu3.mojom | 2 +-\n> 1 file changed, 1 insertion(+), 1 deletion(-)\n> \n> diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom\n> index 6b6b431f..32c046ad 100644\n> --- a/include/libcamera/ipa/ipu3.mojom\n> +++ b/include/libcamera/ipa/ipu3.mojom\n> @@ -32,7 +32,7 @@ struct IPU3Action {\n> \n> struct IPAConfigInfo {\n> \tlibcamera.IPACameraSensorInfo sensorInfo;\n> -\tmap entityControls;\n> +\tmap entityControls;\n> \tlibcamera.Size bdsOutputSize;\n> \tlibcamera.Size iif;\n> };","headers":{"Return-Path":"","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 3A95DC3203\n\tfor ;\n\tWed, 26 May 2021 12:07:44 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AB2086891D;\n\tWed, 26 May 2021 14:07:43 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3F269602AB\n\tfor ;\n\tWed, 26 May 2021 14:07:42 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AA472908;\n\tWed, 26 May 2021 14:07:41 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ZzRhbKlr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1622030861;\n\tbh=zr9d8sslRiPjRiIH3kAfs6uD9pcqwIp/yiONVBIUnTE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ZzRhbKlr4M9XCNtcj3bTPZOwndLNhomcC0wbU8xVlc4uI1AOYeixMrvaBQxqH1S+B\n\tcUBxCW0L00fO7wKugXaJLOFDFo+x80paKNpWJLE5nSHYLoHTaRqfH+yi+xCxaHs31j\n\tFw3VfW5EimDVIITsXArE90NP0uzHFWf6AkU4tjBA=","Date":"Wed, 26 May 2021 15:07:35 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20210526114814.650016-1-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210526114814.650016-1-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Rectify ControlInfoMap\n\tmatching in IPAConfigInfo","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":17272,"web_url":"https://patchwork.libcamera.org/comment/17272/","msgid":"","date":"2021-05-26T12:44:34","subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Rectify ControlInfoMap\n\tmatching in IPAConfigInfo","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nOn Wed, May 26, 2021 at 03:07:37PM +0300, Laurent Pinchart wrote:\n> On Wed, May 26, 2021 at 05:18:14PM +0530, Umang Jain wrote:\n> > The ControlInfoMap of entityControls member in IPAConfigInfo struct,\n> > was not able to correctly match to the ControlInfoMap defined in\n> > core.mojom. This resulted in a FATAL breakage when IPU3 IPA is meant to\n> > run:\n> > \n> > FATAL IPADataSerializer ipa_data_serializer.cpp:437 ControlSerializer\n> > not provided for serialization of ControlInfoMap\n> \n> Fixes: c76ca01323d8 (\"ipa: ipu3: Introduce IPAConfigInfo in IPC\")\n> \n> > Signed-off-by: Umang Jain \n> \n> Reviewed-by: Laurent Pinchart \n> \n> As this fixes a breakage in master, I'll merge it right away.\n> \n> Paul, is there a way this issue could have been detected at compile time\n> ? If that's not doable, could you send at least a documentation patch ?\n\nBeside the ControlInfoMap vs. libcamera::ControlInfoMap, here's the diff\nin the generated code.\n\ndiff -Naur x86-gcc-10.2.0-source/include/libcamera/ipa/ipu3_ipa_serializer.h x86-gcc-11.1.0-source/include/libcamera/ipa/ipu3_ipa_serializer.h\n--- x86-gcc-10.2.0-source/include/libcamera/ipa/ipu3_ipa_serializer.h\t2021-05-26 15:33:16.154762388 +0300\n+++ x86-gcc-11.1.0-source/include/libcamera/ipa/ipu3_ipa_serializer.h\t2021-05-25 12:19:18.146693517 +0300\n@@ -265,7 +265,7 @@\n public:\n \tstatic std::tuple, std::vector>\n \tserialize(const ipa::ipu3::IPAConfigInfo &data,\n+\t\t [[maybe_unused]] ControlSerializer *cs = nullptr)\n-\t\t ControlSerializer *cs)\n \t{\n \t\tstd::vector retData;\n\n@@ -298,7 +298,7 @@\n\n \tstatic ipa::ipu3::IPAConfigInfo\n \tdeserialize(std::vector &data,\n+\t\t ControlSerializer *cs = nullptr)\n-\t\t ControlSerializer *cs)\n \t{\n \t\treturn IPADataSerializer::deserialize(data.cbegin(), data.cend(), cs);\n \t}\n@@ -307,7 +307,7 @@\n \tstatic ipa::ipu3::IPAConfigInfo\n \tdeserialize(std::vector::const_iterator dataBegin,\n \t\t std::vector::const_iterator dataEnd,\n+\t\t [[maybe_unused]] ControlSerializer *cs = nullptr)\n-\t\t ControlSerializer *cs)\n \t{\n \t\tipa::ipu3::IPAConfigInfo ret;\n \t\tstd::vector::const_iterator m = dataBegin;\ndiff -Naur x86-gcc-10.2.0-source/src/libcamera/proxy/ipu3_ipa_proxy.cpp x86-gcc-11.1.0-source/src/libcamera/proxy/ipu3_ipa_proxy.cpp\n--- x86-gcc-10.2.0-source/src/libcamera/proxy/ipu3_ipa_proxy.cpp\t2021-05-26 15:33:16.226762395 +0300\n+++ x86-gcc-11.1.0-source/src/libcamera/proxy/ipu3_ipa_proxy.cpp\t2021-05-25 12:19:18.262693528 +0300\n@@ -272,7 +272,7 @@\n\n \tstd::vector configInfoBuf;\n \tstd::tie(configInfoBuf, std::ignore) =\n+\t\tIPADataSerializer::serialize(configInfo);\n-\t\tIPADataSerializer::serialize(configInfo, &controlSerializer_);\n \t_ipcInputBuf.data().insert(_ipcInputBuf.data().end(), configInfoBuf.begin(), configInfoBuf.end());\n\n\ndiff -Naur x86-gcc-10.2.0-source/src/libcamera/proxy/worker/ipu3_ipa_proxy_worker.cpp x86-gcc-11.1.0-source/src/libcamera/proxy/worker/ipu3_ipa_proxy_worker.cpp\n--- x86-gcc-10.2.0-source/src/libcamera/proxy/worker/ipu3_ipa_proxy_worker.cpp\t2021-05-26 15:33:16.224762394 +0300\n+++ x86-gcc-11.1.0-source/src/libcamera/proxy/worker/ipu3_ipa_proxy_worker.cpp\t2021-05-25 12:19:18.389693540 +0300\n@@ -146,7 +146,8 @@\n\n \tIPAConfigInfo configInfo = IPADataSerializer::deserialize(\n \t_ipcMessage.data().cbegin() + configInfoStart,\n- \t_ipcMessage.data().cend());\n+ \t_ipcMessage.data().cend(),\n+ \t&controlSerializer_);\n\n ipa_->configure(configInfo);\n\n> > ---\n> > include/libcamera/ipa/ipu3.mojom | 2 +-\n> > 1 file changed, 1 insertion(+), 1 deletion(-)\n> > \n> > diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom\n> > index 6b6b431f..32c046ad 100644\n> > --- a/include/libcamera/ipa/ipu3.mojom\n> > +++ b/include/libcamera/ipa/ipu3.mojom\n> > @@ -32,7 +32,7 @@ struct IPU3Action {\n> > \n> > struct IPAConfigInfo {\n> > \tlibcamera.IPACameraSensorInfo sensorInfo;\n> > -\tmap entityControls;\n> > +\tmap entityControls;\n> > \tlibcamera.Size bdsOutputSize;\n> > \tlibcamera.Size iif;\n> > };","headers":{"Return-Path":"","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 83036BDB80\n\tfor ;\n\tWed, 26 May 2021 12:44:42 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C7B166891D;\n\tWed, 26 May 2021 14:44:41 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D3393602AB\n\tfor ;\n\tWed, 26 May 2021 14:44:39 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 298B49C7;\n\tWed, 26 May 2021 14:44:39 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"YBxVO/Sw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1622033079;\n\tbh=LrC9eXDpmjJCmljD0vEaDJ2no44udgbsREcRi2EU4Eg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=YBxVO/SwDNOyxBzy+FZDKiFG+8Yi6yYxR02i9qckeBxb54M7hxTNXcX7GO7bgsKPL\n\tOw9C1gSvkpmXMHSd4CLBNl2M/bdRWIVskFLvQsl+gy706bP8wRxK4h45ERrmr3UHnG\n\tSModQv+1t/0iN5eyuJZ1qrxMsUAa58OWjQ6xF8KA=","Date":"Wed, 26 May 2021 15:44:34 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210526114814.650016-1-umang.jain@ideasonboard.com>\n\t","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH] ipa: ipu3: Rectify ControlInfoMap\n\tmatching in IPAConfigInfo","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]