[{"id":17344,"web_url":"https://patchwork.libcamera.org/comment/17344/","msgid":"","date":"2021-05-28T12:42:10","subject":"Re: [libcamera-devel] [PATCH] utils: ipc: mojo: Error if\n\tControlInfoMap/List doesn't prefix libcamera","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Paul,\n\nThank you for the patch.\n\nOn Thu, May 27, 2021 at 02:32:30PM +0900, Paul Elder wrote:\n> The mojo parser is fine if there are types that are used in array/map\n> members that it does not know about. These are usually caught by the C++\n> compiler, because the generated code refers to unknown types. This\n> feature is necessary for us for supporting FrameBuffer::Plane as an\n> array/map member, since as long as the type has an IPADataSerializer and\n> the struct defined in C++, the generated code will run fine\n> (FrameBuffer::Plane is not defined anywhere in mojom but is used as an\n> array member in IPABuffer).\n> \n> The types that are defined in controls.h (or any header included in\n> ipa_interface.h) will all be compiled by the C++ compiler fine, since\n> the generated files all include controls.h. The types that are there\n> that are not ControlInfoMap or ControlList (like ControlValue) will\n> still fail at the linker stage. For example:\n> \n> struct A {\n> \tarray a;\n> };\n> \n> will compile fine, but will fail to link, since\n> IPADataSerializer doesn't exist. This behavior, although\n> not the best, is acceptable.\n> \n> The issue is that if ControlInfoMap or ControlList are used as array/map\n> members without the libcamera prefix, the compiler will not complain, as\n> the types are valid, and the linker will also not complain, as\n> IPADataSerializer and IPADataSerializer\n> both exist. However, the code generator will not recognize them as\n> types that require a ControlSerializer (since mojo doesn't recognize\n> them, so they are different from the ones that it does recognize with\n> the libcamera namespace), and so the ControlSerializer will not be\n> passed to the serializer in the generated code. This is the cause of the\n> FATAL breakage:\n> \n> FATAL IPADataSerializer ipa_data_serializer.cpp:437 ControlSerializer\n> not provided for serialization of ControlInfoMap\n> \n> Since ControlInfoMap and ControlList are the only types that will run\n> into this issue, we solve this by simply detecting if they are used\n> without the prefix, and produce an error at that point in the code\n> generator. As the code generator stage no longer has information on the\n> source code file and line, we output the struct name in which the error\n> was found (ninja will output the file name).\n\nVery clear explanation, thank you.\n\n> Signed-off-by: Paul Elder \n> ---\n> utils/ipc/generators/mojom_libcamera_generator.py | 4 ++++\n> 1 file changed, 4 insertions(+)\n> \n> diff --git a/utils/ipc/generators/mojom_libcamera_generator.py b/utils/ipc/generators/mojom_libcamera_generator.py\n> index effdfed6..25cacf9a 100644\n> --- a/utils/ipc/generators/mojom_libcamera_generator.py\n> +++ b/utils/ipc/generators/mojom_libcamera_generator.py\n> @@ -129,6 +129,10 @@ def GetAllAttrs(element):\n> \n> def NeedsControlSerializer(element):\n> types = GetAllTypes(element)\n> + if 'x:ControlList' in types:\n> + raise Exception(f'Unknown type \"ControlList\" in {element.mojom_name}, did you mean \"libcamera.ControlList\"?')\n> + if 'x:ControlInfoMap' in types:\n> + raise Exception(f'Unknown type \"ControlInfoMap\" in {element.mojom_name}, did you mean \"libcamera.ControlInfoMap\"?')\n\nFollowing the 0, 1, N rule,\n\n for type in ['ControlList', 'ControlInfoMap']:\n if f'x:{type}' in types:\n raise Exception(f'Unknown type \"{type}\" in {element.mojom_name}, did you mean \"libcamera.{type}\"?')\n\nReviewed-by: Laurent Pinchart \n\n> return \"ControlList\" in types or \"ControlInfoMap\" in types\n> \n> def HasFd(element):","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 09AADC3205\n\tfor ;\n\tFri, 28 May 2021 12:42:23 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 6511C6891F;\n\tFri, 28 May 2021 14:42:19 +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 1F339602AA\n\tfor ;\n\tFri, 28 May 2021 14:42:18 +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 8EDCF148F;\n\tFri, 28 May 2021 14:42:17 +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=\"GVYuSWvF\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1622205737;\n\tbh=Xw+60fDD1KLHFq4lV2EJbD+NBLEFUyULOArIRHG9x00=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GVYuSWvF8gAo73r57n8X+Up4+IAV9EXh0clRlzQbelhwJhnJ6bDu/zxzwkPtVEz8J\n\tq6LJp8DdIhLt+Yt038zZKVdsyIsBwgJO7yEXg5Rxlp+tmHz0VZXrH1n/vXxuc3wmtA\n\tTMoJK/QerGydsPU5SJvJFy9+Q+jCNZOGteN3sU7I=","Date":"Fri, 28 May 2021 15:42:10 +0300","From":"Laurent Pinchart ","To":"Paul Elder ","Message-ID":"","References":"<20210527053230.1329602-1-paul.elder@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210527053230.1329602-1-paul.elder@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH] utils: ipc: mojo: Error if\n\tControlInfoMap/List doesn't prefix libcamera","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\" "}}]