[libcamera-devel] [PATCH v3 1/3] utils: ipc: Include instead of forward-declare CameraSensorInfo
Kieran Bingham
kieran.bingham at ideasonboard.com
Tue Apr 27 22:30:37 CEST 2021
Hi Paul,
On 23/04/2021 11:47, 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.
>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> Tested-by: Umang Jain <umang.jain at ideasonboard.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
> ---
> Changes in v3:
> - update the documentation in core.mojom too
> ---
> include/libcamera/ipa/core.mojom | 3 +--
> include/libcamera/ipa/ipa_interface.h | 6 +++---
> 2 files changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom
> index 5363f1c5..70de71ea 100644
> --- a/include/libcamera/ipa/core.mojom
> +++ b/include/libcamera/ipa/core.mojom
> @@ -38,8 +38,7 @@
> * implemented in ipa_data_serializer.h, as it cannot be defined in mojom
> * - [skipHeader] and [skipSerdes] only work here in core.mojom.
> * - If a struct definition has [skipHeader], then the header where the
> - * struct is defined must be #included (or the struct forward-declared) in
> - * ipa_interface.h
> + * struct is defined must be #included in ipa_interface.h
> * - If a field in a struct has a FileDescriptor, but is not explicitly
> * defined so in mojom, then the field must be marked with the [hasFd]
> * attribute.
> 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"
I know this is merged already - but I have a question here ;-)
The libcamera/ipa/ipa_interface is installed as a 'public' header right?
(So that external IPA's can be built against the interface).
However you are including an /internal/ file here which is not installed.
Doesn't this mean that this commit breaks external builds of IPAs?
--
Kieran
> +
> 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:
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list