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

[Niklas Söderlund]

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 at 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 at 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

-- 
Regards,
Niklas Söderlund