[libcamera-devel,09/13] libcamera: ipa: meson: Allow access to internal libcamera headers

1874 diff mbox series

Allow IPA implementations to use internal libcamera headers so they can
utilize helpers not exposed to applications.

Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
---
 src/ipa/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Hi Niklas,

On 28/08/2019 02:17, Niklas Söderlund wrote:
> Allow IPA implementations to use internal libcamera headers so they can
> utilize helpers not exposed to applications.
> 

This sounds reasonable, but we're interacting against a list of
'ipa_dummy_sources', I suspect we should change the list name to
something more like 'ipa_sources' or such.

I see you've later added the rkisp-IPA to the same list to make use of
this shared_module definition.

Do you anticipate that IPA's will be single file implementations? (in
which case keeping them all at this level, and in this list is fine) -
or could they become more complex? In which case we might want to try to
generalise this shared_module definition some other way too.

All that said, I think we can defer worrying about complex IPAs until we
have one - and keep single files for now.

But that would suggest that the rename of the list might be appropriate
either jointly in this patch, or as an extra.

Anyway, for the include_directories change:

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

But don't let that stop you updating this patch to rename the
ipa_dummy_source list as well if you feel it's appropriate here as a
similar preparatory step.


> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> ---
>  src/ipa/meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> index 2b9863bce3eabe80..dca7a9461385b68d 100644
> --- a/src/ipa/meson.build
> +++ b/src/ipa/meson.build
> @@ -9,7 +9,7 @@ foreach t : ipa_dummy_sources
>      ipa = shared_module(t[0],
>                          t[1],
>                          name_prefix : '',
> -                        include_directories : libcamera_includes,
> +                        include_directories : includes,
>                          install : true,
>                          install_dir : ipa_install_dir)
>  endforeach
>

HI Kieran,

Thanks for your feedback.

On 2019-08-29 15:10:43 +0100, Kieran Bingham wrote:
> Hi Niklas,
> 
> On 28/08/2019 02:17, Niklas Söderlund wrote:
> > Allow IPA implementations to use internal libcamera headers so they can
> > utilize helpers not exposed to applications.
> > 
> 
> This sounds reasonable, but we're interacting against a list of
> 'ipa_dummy_sources', I suspect we should change the list name to
> something more like 'ipa_sources' or such.

Good spot, not sure how I missed that.

> 
> I see you've later added the rkisp-IPA to the same list to make use of
> this shared_module definition.

I will rename this list in the patch where I add the rkisp1 IPA as I 
think it makes most sens.

> 
> Do you anticipate that IPA's will be single file implementations? (in
> which case keeping them all at this level, and in this list is fine) -
> or could they become more complex? In which case we might want to try to
> generalise this shared_module definition some other way too.

I think over time they will grow to span multiple files, but for now I 
like to keep it simple and build out once we know what we need.

> 
> All that said, I think we can defer worrying about complex IPAs until we
> have one - and keep single files for now.

I agree :-)

> 
> But that would suggest that the rename of the list might be appropriate
> either jointly in this patch, or as an extra.

I will do so when I add the rkisp1 IPA.

> 
> Anyway, for the include_directories change:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> But don't let that stop you updating this patch to rename the
> ipa_dummy_source list as well if you feel it's appropriate here as a
> similar preparatory step.
> 
> 
> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
> > ---
> >  src/ipa/meson.build | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/src/ipa/meson.build b/src/ipa/meson.build
> > index 2b9863bce3eabe80..dca7a9461385b68d 100644
> > --- a/src/ipa/meson.build
> > +++ b/src/ipa/meson.build
> > @@ -9,7 +9,7 @@ foreach t : ipa_dummy_sources
> >      ipa = shared_module(t[0],
> >                          t[1],
> >                          name_prefix : '',
> > -                        include_directories : libcamera_includes,
> > +                        include_directories : includes,
> >                          install : true,
> >                          install_dir : ipa_install_dir)
> >  endforeach
> > 
> 
> -- 
> Regards
> --
> Kieran