[libcamera-devel,v2,4/6] ipa: meson: Allow IPAs to include internal headers

Message ID 20191004163734.15594-5-jacopo@jmondi.org
State Superseded
Headers show
Series
  • test: Add IPA interface test support
Related show

Commit Message

Jacopo Mondi Oct. 4, 2019, 4:37 p.m. UTC
Extend the list of inclusion paths for the IPA modules in src/ipa/ to
include internal libcamera headers.

Only Open Source IPA implementations will live in src/ipa/ and they link
against libcamera, so they should be able to include internal headers as
well as public ones.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/ipa/meson.build | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Comments

Laurent Pinchart Oct. 4, 2019, 8:43 p.m. UTC | #1
Hi Jacopo,

Thank you for the patch.

On Fri, Oct 04, 2019 at 06:37:32PM +0200, Jacopo Mondi wrote:
> Extend the list of inclusion paths for the IPA modules in src/ipa/ to
> include internal libcamera headers.
> 
> Only Open Source IPA implementations will live in src/ipa/ and they link
> against libcamera, so they should be able to include internal headers as
> well as public ones.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

I think that longer term we'll have to split the internal headers
between really internal and internal+IPA headers. For now this looks
good.

> ---
>  src/ipa/meson.build | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> index b5bcd7b2c3db..2827dc0303b2 100644
> --- a/src/ipa/meson.build
> +++ b/src/ipa/meson.build
> @@ -5,10 +5,15 @@ ipa_vimc_sources = [
>  
>  ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')
>  
> +ipa_includes = [
> +    libcamera_includes,
> +    libcamera_internal_includes,
> +]
> +
>  foreach t : ipa_vimc_sources
>      ipa = shared_module(t[0], 'ipa_vimc.cpp',
>                          name_prefix : '',
> -                        include_directories : libcamera_includes,
> +                        include_directories : ipa_includes,
>                          install : true,
>                          install_dir : ipa_install_dir,
>                          cpp_args : '-DLICENSE="' + t[1] + '"')

Don't you also need to link to libcamera ? A

	dependencies : libcamera_dep

would seem appropriate. With this fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Jacopo Mondi Oct. 5, 2019, 1 p.m. UTC | #2
Hi Laurent,

On Fri, Oct 04, 2019 at 11:43:44PM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Fri, Oct 04, 2019 at 06:37:32PM +0200, Jacopo Mondi wrote:
> > Extend the list of inclusion paths for the IPA modules in src/ipa/ to
> > include internal libcamera headers.
> >
> > Only Open Source IPA implementations will live in src/ipa/ and they link
> > against libcamera, so they should be able to include internal headers as
> > well as public ones.
> >
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
>
> I think that longer term we'll have to split the internal headers
> between really internal and internal+IPA headers. For now this looks
> good.
>
> > ---
> >  src/ipa/meson.build | 7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> > index b5bcd7b2c3db..2827dc0303b2 100644
> > --- a/src/ipa/meson.build
> > +++ b/src/ipa/meson.build
> > @@ -5,10 +5,15 @@ ipa_vimc_sources = [
> >
> >  ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')
> >
> > +ipa_includes = [
> > +    libcamera_includes,
> > +    libcamera_internal_includes,
> > +]
> > +
> >  foreach t : ipa_vimc_sources
> >      ipa = shared_module(t[0], 'ipa_vimc.cpp',
> >                          name_prefix : '',
> > -                        include_directories : libcamera_includes,
> > +                        include_directories : ipa_includes,
> >                          install : true,
> >                          install_dir : ipa_install_dir,
> >                          cpp_args : '-DLICENSE="' + t[1] + '"')
>
> Don't you also need to link to libcamera ? A
>
> 	dependencies : libcamera_dep
>
> would seem appropriate. With this fixed,

I wonder why I don't get linkage errors as I use LOG...

>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Thanks
  j

>
> --
> Regards,
>
> Laurent Pinchart

Patch

diff --git a/src/ipa/meson.build b/src/ipa/meson.build
index b5bcd7b2c3db..2827dc0303b2 100644
--- a/src/ipa/meson.build
+++ b/src/ipa/meson.build
@@ -5,10 +5,15 @@  ipa_vimc_sources = [
 
 ipa_install_dir = join_paths(get_option('libdir'), 'libcamera')
 
+ipa_includes = [
+    libcamera_includes,
+    libcamera_internal_includes,
+]
+
 foreach t : ipa_vimc_sources
     ipa = shared_module(t[0], 'ipa_vimc.cpp',
                         name_prefix : '',
-                        include_directories : libcamera_includes,
+                        include_directories : ipa_includes,
                         install : true,
                         install_dir : ipa_install_dir,
                         cpp_args : '-DLICENSE="' + t[1] + '"')