| Message ID | 20210422080816.364948-3-paul.elder@ideasonboard.com |
|---|---|
| State | Accepted |
| Delegated to: | Paul Elder |
| Headers | show |
| Series |
|
| Related | show |
Hi Paul, Thank you for the patch. On Thu, Apr 22, 2021 at 05:08:16PM +0900, Paul Elder wrote: > For structs defined in core.mojom that have the skipHeader tag, if > they're only used in function parameters (in a mojom file) then a > forward-declaration is sufficient. However, if the struct is used in > another struct in a mojom file, then the forward-declaration is > insufficient, and the definition needs to be included. Do so for > CameraSensorInfo, which is the only forward-declared struct in > ipa_interface.h, and update the documentation comment. Makes sense. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > --- > New in v2 > > include/libcamera/ipa/ipa_interface.h | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h > index 5d99e2cf..dfe1b40a 100644 > --- a/include/libcamera/ipa/ipa_interface.h > +++ b/include/libcamera/ipa/ipa_interface.h > @@ -18,15 +18,15 @@ > #include <libcamera/geometry.h> > #include <libcamera/signal.h> > > +#include "libcamera/internal/camera_sensor.h" > + > namespace libcamera { > > /* > * Structs that are defined in core.mojom and have the skipHeader tag must be > - * forward-declared or #included here. > + * #included here. > */ > > -struct CameraSensorInfo; > - > class IPAInterface > { > public:
On Fri, Apr 23, 2021 at 06:09:42AM +0300, Laurent Pinchart wrote: > On Thu, Apr 22, 2021 at 05:08:16PM +0900, Paul Elder wrote: > > For structs defined in core.mojom that have the skipHeader tag, if > > they're only used in function parameters (in a mojom file) then a > > forward-declaration is sufficient. However, if the struct is used in > > another struct in a mojom file, then the forward-declaration is > > insufficient, and the definition needs to be included. Do so for > > CameraSensorInfo, which is the only forward-declared struct in > > ipa_interface.h, and update the documentation comment. > > Makes sense. > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> Actually, you also need to update a comment in core.mojom. > > Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> > > --- > > New in v2 > > > > include/libcamera/ipa/ipa_interface.h | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h > > index 5d99e2cf..dfe1b40a 100644 > > --- a/include/libcamera/ipa/ipa_interface.h > > +++ b/include/libcamera/ipa/ipa_interface.h > > @@ -18,15 +18,15 @@ > > #include <libcamera/geometry.h> > > #include <libcamera/signal.h> > > > > +#include "libcamera/internal/camera_sensor.h" > > + > > namespace libcamera { > > > > /* > > * Structs that are defined in core.mojom and have the skipHeader tag must be > > - * forward-declared or #included here. > > + * #included here. > > */ > > > > -struct CameraSensorInfo; > > - > > class IPAInterface > > { > > public:
diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h index 5d99e2cf..dfe1b40a 100644 --- a/include/libcamera/ipa/ipa_interface.h +++ b/include/libcamera/ipa/ipa_interface.h @@ -18,15 +18,15 @@ #include <libcamera/geometry.h> #include <libcamera/signal.h> +#include "libcamera/internal/camera_sensor.h" + namespace libcamera { /* * Structs that are defined in core.mojom and have the skipHeader tag must be - * forward-declared or #included here. + * #included here. */ -struct CameraSensorInfo; - class IPAInterface { public:
For structs defined in core.mojom that have the skipHeader tag, if they're only used in function parameters (in a mojom file) then a forward-declaration is sufficient. However, if the struct is used in another struct in a mojom file, then the forward-declaration is insufficient, and the definition needs to be included. Do so for CameraSensorInfo, which is the only forward-declared struct in ipa_interface.h, and update the documentation comment. Signed-off-by: Paul Elder <paul.elder@ideasonboard.com> --- New in v2 include/libcamera/ipa/ipa_interface.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)