[{"id":16662,"web_url":"https://patchwork.libcamera.org/comment/16662/","msgid":"<9420c812-146e-04cf-a5e6-77cd7460e677@ideasonboard.com>","date":"2021-04-27T20:30:37","subject":"Re: [libcamera-devel] [PATCH v3 1/3] utils: ipc: Include instead of\n\tforward-declare CameraSensorInfo","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Paul,\n\nOn 23/04/2021 11:47, Paul Elder wrote:\n> For structs defined in core.mojom that have the skipHeader tag, if\n> they're only used in function parameters (in a mojom file) then a\n> forward-declaration is sufficient. However, if the struct is used in\n> another struct in a mojom file, then the forward-declaration is\n> insufficient, and the definition needs to be included. Do so for\n> CameraSensorInfo, which is the only forward-declared struct in\n> ipa_interface.h, and update the documentation comment.\n> \n> Signed-off-by: Paul Elder \n> Tested-by: Umang Jain \n> Reviewed-by: Laurent Pinchart \n> \n> ---\n> Changes in v3:\n> - update the documentation in core.mojom too\n> ---\n> include/libcamera/ipa/core.mojom | 3 +--\n> include/libcamera/ipa/ipa_interface.h | 6 +++---\n> 2 files changed, 4 insertions(+), 5 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n> index 5363f1c5..70de71ea 100644\n> --- a/include/libcamera/ipa/core.mojom\n> +++ b/include/libcamera/ipa/core.mojom\n> @@ -38,8 +38,7 @@\n> * implemented in ipa_data_serializer.h, as it cannot be defined in mojom\n> * - [skipHeader] and [skipSerdes] only work here in core.mojom.\n> * - If a struct definition has [skipHeader], then the header where the\n> - * struct is defined must be #included (or the struct forward-declared) in\n> - * ipa_interface.h\n> + * struct is defined must be #included in ipa_interface.h\n> * - If a field in a struct has a FileDescriptor, but is not explicitly\n> * defined so in mojom, then the field must be marked with the [hasFd]\n> * attribute.\n> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> index 5d99e2cf..dfe1b40a 100644\n> --- a/include/libcamera/ipa/ipa_interface.h\n> +++ b/include/libcamera/ipa/ipa_interface.h\n> @@ -18,15 +18,15 @@\n> #include \n> #include \n> \n> +#include \"libcamera/internal/camera_sensor.h\"\n\nI know this is merged already - but I have a question here ;-)\n\nThe libcamera/ipa/ipa_interface is installed as a 'public' header right?\n(So that external IPA's can be built against the interface).\n\nHowever you are including an /internal/ file here which is not installed.\n\nDoesn't this mean that this commit breaks external builds of IPAs?\n\n--\nKieran\n\n\n\n> +\n> namespace libcamera {\n> \n> /*\n> * Structs that are defined in core.mojom and have the skipHeader tag must be\n> - * forward-declared or #included here.\n> + * #included here.\n> */\n> \n> -struct CameraSensorInfo;\n> -\n> class IPAInterface\n> {\n> public:\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 C5708BDE3F\n\tfor ;\n\tTue, 27 Apr 2021 20:30:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3997568878;\n\tTue, 27 Apr 2021 22:30: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 D6992602C2\n\tfor ;\n\tTue, 27 Apr 2021 22:30:41 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 408CFE9;\n\tTue, 27 Apr 2021 22:30:41 +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=\"id8ALAF4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619555441;\n\tbh=ZYtQLWk5S5xd5swrWqvUt5azLLsQnVcBeemNZIfNmEw=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=id8ALAF4eYgFpGHsJshc2jK0qjTjsaf7Uc/6qojUf5wIimr4KCRElYBM+BPnu6GG5\n\t16ARzo7OjYeg1hKURHiR0MfRN+tqYJ322ZuEWq68XG7lEfCzwCb4ci0Bn+FWLOIugh\n\ti7UjDFi/rD0pJZEZzqWC90Xv2+rnWUHFRqANjGl8=","To":"Paul Elder ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210423104711.401547-1-paul.elder@ideasonboard.com>\n\t<20210423104711.401547-2-paul.elder@ideasonboard.com>","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"<9420c812-146e-04cf-a5e6-77cd7460e677@ideasonboard.com>","Date":"Tue, 27 Apr 2021 21:30:37 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"<20210423104711.401547-2-paul.elder@ideasonboard.com>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] utils: ipc: Include instead of\n\tforward-declare CameraSensorInfo","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","Reply-To":"kieran.bingham@ideasonboard.com","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16664,"web_url":"https://patchwork.libcamera.org/comment/16664/","msgid":"","date":"2021-04-28T06:44:30","subject":"Re: [libcamera-devel] [PATCH v3 1/3] utils: ipc: Include instead of\n\tforward-declare CameraSensorInfo","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Kieran,\n\nOn 4/28/21 2:00 AM, Kieran Bingham wrote:\n> Hi Paul,\n>\n> On 23/04/2021 11:47, Paul Elder wrote:\n>> For structs defined in core.mojom that have the skipHeader tag, if\n>> they're only used in function parameters (in a mojom file) then a\n>> forward-declaration is sufficient. However, if the struct is used in\n>> another struct in a mojom file, then the forward-declaration is\n>> insufficient, and the definition needs to be included. Do so for\n>> CameraSensorInfo, which is the only forward-declared struct in\n>> ipa_interface.h, and update the documentation comment.\n>>\n>> Signed-off-by: Paul Elder \n>> Tested-by: Umang Jain \n>> Reviewed-by: Laurent Pinchart \n>>\n>> ---\n>> Changes in v3:\n>> - update the documentation in core.mojom too\n>> ---\n>> include/libcamera/ipa/core.mojom | 3 +--\n>> include/libcamera/ipa/ipa_interface.h | 6 +++---\n>> 2 files changed, 4 insertions(+), 5 deletions(-)\n>>\n>> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n>> index 5363f1c5..70de71ea 100644\n>> --- a/include/libcamera/ipa/core.mojom\n>> +++ b/include/libcamera/ipa/core.mojom\n>> @@ -38,8 +38,7 @@\n>> * implemented in ipa_data_serializer.h, as it cannot be defined in mojom\n>> * - [skipHeader] and [skipSerdes] only work here in core.mojom.\n>> * - If a struct definition has [skipHeader], then the header where the\n>> - * struct is defined must be #included (or the struct forward-declared) in\n>> - * ipa_interface.h\n>> + * struct is defined must be #included in ipa_interface.h\n>> * - If a field in a struct has a FileDescriptor, but is not explicitly\n>> * defined so in mojom, then the field must be marked with the [hasFd]\n>> * attribute.\n>> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n>> index 5d99e2cf..dfe1b40a 100644\n>> --- a/include/libcamera/ipa/ipa_interface.h\n>> +++ b/include/libcamera/ipa/ipa_interface.h\n>> @@ -18,15 +18,15 @@\n>> #include \n>> #include \n>> \n>> +#include \"libcamera/internal/camera_sensor.h\"\n> I know this is merged already - but I have a question here ;-)\n>\n> The libcamera/ipa/ipa_interface is installed as a 'public' header right?\n> (So that external IPA's can be built against the interface).\n>\n> However you are including an /internal/ file here which is not installed.\n>\n> Doesn't this mean that this commit breaks external builds of IPAs?\nProbably it's broken already as I don't think we thought out the design \nconstraints while writing the IPA skeleton as well. For e.g. I am \nreading the internal headers used for writing the IPA IPU3 skeleton and \nit already uses \"libcamera/internal/buffer.h\" for MappedFrameBuffer.\n\nThere are others too, in our closed source IPA, like \nlibcamera/internal/log.h & libcamera/internal/file.h, that we probably \nneed to write/copy some kind of drop-in replacement.\n>\n> --\n> Kieran\n>\n>\n>\n>> +\n>> namespace libcamera {\n>> \n>> /*\n>> * Structs that are defined in core.mojom and have the skipHeader tag must be\n>> - * forward-declared or #included here.\n>> + * #included here.\n>> */\n>> \n>> -struct CameraSensorInfo;\n>> -\n>> class IPAInterface\n>> {\n>> public:\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 361BABDE41\n\tfor ;\n\tWed, 28 Apr 2021 06:44:39 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 72C1A688B7;\n\tWed, 28 Apr 2021 08:44:38 +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 CD8A760511\n\tfor ;\n\tWed, 28 Apr 2021 08:44:36 +0200 (CEST)","from localhost.localdomain (unknown [103.238.109.25])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 96AB72C1;\n\tWed, 28 Apr 2021 08:44:35 +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=\"nrJn15tJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619592276;\n\tbh=wsp/QtazwHgOyJ8HYegInKWvOYevutLOUgy4hRU+kHo=;\n\th=Subject:To:References:From:Date:In-Reply-To:From;\n\tb=nrJn15tJPWW/9zfHfScRVRUD2FeypVXmnh+vr1POgPoTVRkjHfh0gYWMA9aY18JQP\n\tOehwOp1ydcB66ukj14Yd0dL+L9pE5TenCl84cLA6SW8Ut34yUqxqPuABtSwcV9v/An\n\ty0OXdFlscHM4MMQ+Lz2jI3Ebz9Th0LBsBTx4YCiQ=","To":"kieran.bingham@ideasonboard.com, Paul Elder\n\t, libcamera-devel@lists.libcamera.org","References":"<20210423104711.401547-1-paul.elder@ideasonboard.com>\n\t<20210423104711.401547-2-paul.elder@ideasonboard.com>\n\t<9420c812-146e-04cf-a5e6-77cd7460e677@ideasonboard.com>","From":"Umang Jain ","Message-ID":"","Date":"Wed, 28 Apr 2021 12:14:30 +0530","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101\n\tThunderbird/68.11.0","MIME-Version":"1.0","In-Reply-To":"<9420c812-146e-04cf-a5e6-77cd7460e677@ideasonboard.com>","Content-Language":"en-US","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] utils: ipc: Include instead of\n\tforward-declare CameraSensorInfo","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","Content-Transfer-Encoding":"7bit","Content-Type":"text/plain; charset=\"us-ascii\"; Format=\"flowed\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16678,"web_url":"https://patchwork.libcamera.org/comment/16678/","msgid":"<20210429031511.GM195599@pyrite.rasen.tech>","date":"2021-04-29T03:15:11","subject":"Re: [libcamera-devel] [PATCH v3 1/3] utils: ipc: Include instead of\n\tforward-declare CameraSensorInfo","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Kieran,\n\nOn Tue, Apr 27, 2021 at 09:30:37PM +0100, Kieran Bingham wrote:\n> Hi Paul,\n> \n> On 23/04/2021 11:47, Paul Elder wrote:\n> > For structs defined in core.mojom that have the skipHeader tag, if\n> > they're only used in function parameters (in a mojom file) then a\n> > forward-declaration is sufficient. However, if the struct is used in\n> > another struct in a mojom file, then the forward-declaration is\n> > insufficient, and the definition needs to be included. Do so for\n> > CameraSensorInfo, which is the only forward-declared struct in\n> > ipa_interface.h, and update the documentation comment.\n> > \n> > Signed-off-by: Paul Elder \n> > Tested-by: Umang Jain \n> > Reviewed-by: Laurent Pinchart \n> > \n> > ---\n> > Changes in v3:\n> > - update the documentation in core.mojom too\n> > ---\n> > include/libcamera/ipa/core.mojom | 3 +--\n> > include/libcamera/ipa/ipa_interface.h | 6 +++---\n> > 2 files changed, 4 insertions(+), 5 deletions(-)\n> > \n> > diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n> > index 5363f1c5..70de71ea 100644\n> > --- a/include/libcamera/ipa/core.mojom\n> > +++ b/include/libcamera/ipa/core.mojom\n> > @@ -38,8 +38,7 @@\n> > * implemented in ipa_data_serializer.h, as it cannot be defined in mojom\n> > * - [skipHeader] and [skipSerdes] only work here in core.mojom.\n> > * - If a struct definition has [skipHeader], then the header where the\n> > - * struct is defined must be #included (or the struct forward-declared) in\n> > - * ipa_interface.h\n> > + * struct is defined must be #included in ipa_interface.h\n> > * - If a field in a struct has a FileDescriptor, but is not explicitly\n> > * defined so in mojom, then the field must be marked with the [hasFd]\n> > * attribute.\n> > diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n> > index 5d99e2cf..dfe1b40a 100644\n> > --- a/include/libcamera/ipa/ipa_interface.h\n> > +++ b/include/libcamera/ipa/ipa_interface.h\n> > @@ -18,15 +18,15 @@\n> > #include \n> > #include \n> > \n> > +#include \"libcamera/internal/camera_sensor.h\"\n> \n> I know this is merged already - but I have a question here ;-)\n> \n> The libcamera/ipa/ipa_interface is installed as a 'public' header right?\n> (So that external IPA's can be built against the interface).\n> \n> However you are including an /internal/ file here which is not installed.\n> \n> Doesn't this mean that this commit breaks external builds of IPAs?\n\nOh yeah... it does... maybe that's why it was forward-declared in the\nfirst place.\n\nBut also, if somebody defines a struct in mojom what uses\nCameraSensorInfo, the code generator needs to generate a struct that\nembeds CameraSensorInfo, so the definition is kind of necessary (that's\nwhy the compiler was complaining in the first place). Maybe the struct\ndefinition should be moved here? Or it should be moved to a public\nheader?\n\nThough, as Umang mentioned, there are a lot of other things that are\nbroken in external IPA builds. I think it'll get even worse after we\nfactor in the external IPA authors that are paranoid about importing any\nheaders at all from us.\n\n\nPaul\n\n> > namespace libcamera {\n> > \n> > /*\n> > * Structs that are defined in core.mojom and have the skipHeader tag must be\n> > - * forward-declared or #included here.\n> > + * #included here.\n> > */\n> > \n> > -struct CameraSensorInfo;\n> > -\n> > class IPAInterface\n> > {\n> > public:\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 6E0D6BDE45\n\tfor ;\n\tThu, 29 Apr 2021 03:15:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8C20768907;\n\tThu, 29 Apr 2021 05:15:21 +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 0C078602C1\n\tfor ;\n\tThu, 29 Apr 2021 05:15:20 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 64070BC0;\n\tThu, 29 Apr 2021 05:15:18 +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=\"NATQ1uez\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619666119;\n\tbh=N5qtgzqHW13zk11JfPhvmELrPOZAzj/eCYbv03Szf8w=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=NATQ1uezv3J4SWWFGa1CxNbGID8xrBSFAqMhsZmmuK8pPyYFsAlx46ZMBygmkjtJ7\n\t65vGYhg4TMcdA1Zacq0Ke1I5zaQrhnLJc94JMHFMrkCaZerUd6/JbRTjmE8y+vmAY+\n\tQcahP4Gm+cgnfI3Hnzt/SkNbBrVQ6PWNBwoMoNZ4=","Date":"Thu, 29 Apr 2021 12:15:11 +0900","From":"paul.elder@ideasonboard.com","To":"Kieran Bingham ","Message-ID":"<20210429031511.GM195599@pyrite.rasen.tech>","References":"<20210423104711.401547-1-paul.elder@ideasonboard.com>\n\t<20210423104711.401547-2-paul.elder@ideasonboard.com>\n\t<9420c812-146e-04cf-a5e6-77cd7460e677@ideasonboard.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<9420c812-146e-04cf-a5e6-77cd7460e677@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] utils: ipc: Include instead of\n\tforward-declare CameraSensorInfo","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":16684,"web_url":"https://patchwork.libcamera.org/comment/16684/","msgid":"<98599749-dcf1-a96b-f573-964c45c5d16d@ideasonboard.com>","date":"2021-04-29T08:34:41","subject":"Re: [libcamera-devel] [PATCH v3 1/3] utils: ipc: Include instead of\n\tforward-declare CameraSensorInfo","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Paul,\n\nOn 29/04/2021 04:15, paul.elder@ideasonboard.com wrote:\n> Hi Kieran,\n> \n> On Tue, Apr 27, 2021 at 09:30:37PM +0100, Kieran Bingham wrote:\n>> Hi Paul,\n>>\n>> On 23/04/2021 11:47, Paul Elder wrote:\n>>> For structs defined in core.mojom that have the skipHeader tag, if\n>>> they're only used in function parameters (in a mojom file) then a\n>>> forward-declaration is sufficient. However, if the struct is used in\n>>> another struct in a mojom file, then the forward-declaration is\n>>> insufficient, and the definition needs to be included. Do so for\n>>> CameraSensorInfo, which is the only forward-declared struct in\n>>> ipa_interface.h, and update the documentation comment.\n>>>\n>>> Signed-off-by: Paul Elder \n>>> Tested-by: Umang Jain \n>>> Reviewed-by: Laurent Pinchart \n>>>\n>>> ---\n>>> Changes in v3:\n>>> - update the documentation in core.mojom too\n>>> ---\n>>> include/libcamera/ipa/core.mojom | 3 +--\n>>> include/libcamera/ipa/ipa_interface.h | 6 +++---\n>>> 2 files changed, 4 insertions(+), 5 deletions(-)\n>>>\n>>> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n>>> index 5363f1c5..70de71ea 100644\n>>> --- a/include/libcamera/ipa/core.mojom\n>>> +++ b/include/libcamera/ipa/core.mojom\n>>> @@ -38,8 +38,7 @@\n>>> * implemented in ipa_data_serializer.h, as it cannot be defined in mojom\n>>> * - [skipHeader] and [skipSerdes] only work here in core.mojom.\n>>> * - If a struct definition has [skipHeader], then the header where the\n>>> - * struct is defined must be #included (or the struct forward-declared) in\n>>> - * ipa_interface.h\n>>> + * struct is defined must be #included in ipa_interface.h\n>>> * - If a field in a struct has a FileDescriptor, but is not explicitly\n>>> * defined so in mojom, then the field must be marked with the [hasFd]\n>>> * attribute.\n>>> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h\n>>> index 5d99e2cf..dfe1b40a 100644\n>>> --- a/include/libcamera/ipa/ipa_interface.h\n>>> +++ b/include/libcamera/ipa/ipa_interface.h\n>>> @@ -18,15 +18,15 @@\n>>> #include \n>>> #include \n>>> \n>>> +#include \"libcamera/internal/camera_sensor.h\"\n>>\n>> I know this is merged already - but I have a question here ;-)\n>>\n>> The libcamera/ipa/ipa_interface is installed as a 'public' header right?\n>> (So that external IPA's can be built against the interface).\n>>\n>> However you are including an /internal/ file here which is not installed.\n>>\n>> Doesn't this mean that this commit breaks external builds of IPAs?\n> \n> Oh yeah... it does... maybe that's why it was forward-declared in the\n> first place.\n\n\nI noticed because it also breaks my ABI-Compliance checker, which parses\nall the public headers, and has no visibility of the private headers ...\nso it broke... :D\n\n\n> But also, if somebody defines a struct in mojom what uses\n> CameraSensorInfo, the code generator needs to generate a struct that\n> embeds CameraSensorInfo, so the definition is kind of necessary (that's\n> why the compiler was complaining in the first place). Maybe the struct\n> definition should be moved here? Or it should be moved to a public\n> header?\n\nYes, but ... not 'public' headers, as these are not libcamera public API\nheaders - but they are needed by an internal interface which is\navailable only to IPAs. (of course external IPA's mean a separate sort\nof public interface that we need to now handle).\n\nSo we need to be able to break these out from include/internal/ and have\nthem available to the IPA without being available to applications.\n\n\n> Though, as Umang mentioned, there are a lot of other things that are\n> broken in external IPA builds. I think it'll get even worse after we\n> factor in the external IPA authors that are paranoid about importing any\n> headers at all from us.\n\nIndeed, we may have to consider specific licensing issues on those\ninterface headers, and/or ensure that we can facilitate the entire\nreconstruction of those binary protocols from scratch ...\n\n\n> Paul\n> \n>>> namespace libcamera {\n>>> \n>>> /*\n>>> * Structs that are defined in core.mojom and have the skipHeader tag must be\n>>> - * forward-declared or #included here.\n>>> + * #included here.\n>>> */\n>>> \n>>> -struct CameraSensorInfo;\n>>> -\n>>> class IPAInterface\n>>> {\n>>> public:\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 D622CBDE45\n\tfor ;\n\tThu, 29 Apr 2021 08:34:46 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 4A8D36890D;\n\tThu, 29 Apr 2021 10:34:46 +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 69DB368909\n\tfor ;\n\tThu, 29 Apr 2021 10:34:45 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id DA286BC0;\n\tThu, 29 Apr 2021 10:34:44 +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=\"eTGDi7Aj\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1619685285;\n\tbh=NWp542rGH/SGu3HWAYGhowYxghP9wLtHVmkGIaAuB6g=;\n\th=Reply-To:Subject:To:Cc:References:From:Date:In-Reply-To:From;\n\tb=eTGDi7Ajr9Fs1hhneLuW+XNKR5TplrhWVEKAxTSv3Xfyuhtix2CYiExz/Xu+JzKqY\n\tq+qmEbMWtrENmrrVkBf/P/Ng0A0lGRhbTm35CNZ+lDlKj3UlR708ynuSi/JzCJHBq7\n\tpFHDT25HxCI6X0BqCAoKBOp+9Y1qjyEeXKtFg2mQ=","To":"paul.elder@ideasonboard.com","References":"<20210423104711.401547-1-paul.elder@ideasonboard.com>\n\t<20210423104711.401547-2-paul.elder@ideasonboard.com>\n\t<9420c812-146e-04cf-a5e6-77cd7460e677@ideasonboard.com>\n\t<20210429031511.GM195599@pyrite.rasen.tech>","From":"Kieran Bingham ","Organization":"Ideas on Board","Message-ID":"<98599749-dcf1-a96b-f573-964c45c5d16d@ideasonboard.com>","Date":"Thu, 29 Apr 2021 09:34:41 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.7.1","MIME-Version":"1.0","In-Reply-To":"<20210429031511.GM195599@pyrite.rasen.tech>","Content-Language":"en-GB","Subject":"Re: [libcamera-devel] [PATCH v3 1/3] utils: ipc: Include instead of\n\tforward-declare CameraSensorInfo","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","Reply-To":"kieran.bingham@ideasonboard.com","Cc":"libcamera-devel@lists.libcamera.org","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]