[libcamera-devel] system locations of libcamera headers

[Laurent Pinchart]

On Fri, May 07, 2021 at 04:12:46PM +0530, Umang Jain wrote:
> On 5/7/21 2:30 PM, Kieran Bingham wrote:
> > On 07/05/2021 06:24, Umang Jain wrote:
> >> Hi all,
> >>
> >> If I compile the master branch with --prefix=/usr
> >> /meson --prefix=/usr -Dpipelines=uvcvideo,ipu3 build && sudo ninja -C
> >> build install/
> >>
> >>
> >> The headers turns out to have a directory structure as below:
> >>
> >>
> >> [uajain at localhost libcamera]$ tree /usr/include/libcamera/
> >> /usr/include/libcamera/
> >> ├── ipa
> >> │   ├── ipa_controls.h
> >> │   ├── ipa_interface.h
> >> │   └── ipa_module_info.h
> >> └── libcamera
> >>      ├── bound_method.h
> >>      ├── buffer.h
> >>      ├── camera.h
> >>      ├── camera_manager.h
> >>      ├── class.h
> >>      ├── compiler.h
> >>      ├── control_ids.h
> >>      ├── controls.h
> >>      ├── file_descriptor.h
> >>      ├── formats.h
> >>      ├── framebuffer_allocator.h
> >>      ├── geometry.h
> >>      ├── libcamera.h
> >>      ├── logging.h
> >>      ├── object.h
> >>      ├── pixel_format.h
> >>      ├── property_ids.h
> >>      ├── request.h
> >>      ├── signal.h
> >>      ├── span.h
> >>      ├── stream.h
> >>      ├── transform.h
> >>      └── version.h
> >>
> >> 2 directories, 26 files
> >>
> >>
> >> My question is, is this how we want public headers to be placed ? For
> >> e.g. the absolute path of signal.h
> >> is/usr/include/libcamera/libcamera/signal.h, hence, even the below diff
> >> will compile successfully (due to linker find path magic)
> >>
> >> diff --git a/src/libcamera/object.cpp b/src/libcamera/object.cpp
> >> index cd83c684..5f005d1b 100644
> >> --- a/src/libcamera/object.cpp
> >> +++ b/src/libcamera/object.cpp
> >> @@ -9,7 +9,7 @@
> >>   
> >>   #include <algorithm>
> >>   
> >> -#include <libcamera/signal.h>
> >> +#include <libcamera/libcamera/signal.h>
> >
> > This gets 'found' because the include path is including "/usr/include/"
> > and your include statement specifies the double libcamera, which then
> > maps to the full /usr/include/libcamera/libcamera/signal.h path.
> >
> > However, it's not correct to do so.
> >
> > The path there is:
> >
> > /usr/include/<project>/<component>/<header.h>
>
> Ok, this makes it clear - probably we should document <project> vs 
> <component> split somewhere?

If you look at the pkgconfig file that meson generates, we have

prefix=/usr/local
libdir=${prefix}/lib64
includedir=${prefix}/include

Name: libcamera
Description: Complex Camera Support Library
Version: 1.0
Libs: -L${libdir} -lcamera
Cflags: -I${includedir}/libcamera


The <project> part of the include path will likely become
'libcamera-1.0' once we release a first stable version. It will be
handled by pkgconfig.  The <component> part of the path will always be
'libcamera', and that's the part that has to be specified in the
#include directive.

> > In this instance, both the project and the component have the same name.
> >
> >> Considering the case where we also need to expose some of the internal
> >> libcamera headers on system locations (for IPAs to use) - It's better to
> >> kick off some dicussions around how we would structure the header
> >> locations. This will also clear some of my ongoing meson cleanup work
> >> (not all patches, but only a couple).
> >
> > We already have /usr/include/libcamera/ipa/ for IPA interfaces to be
> > installed to.
> >
> > The issue you are facing is that the IPA interfaces are referencing
> > 'internal' and not installed or public headers from
> > <sourcetree>/include/libcamera/internal/<private files>
>
> This + internal headers required for the actual IPA
> 
> If I take current status quo:
> The IPA interface/usr/include/libcamera/ipa/ipa_interface.h needs 
> internal header libcamera/internal/camera_sensor.h

We need to either move CameraSensorInfo to IPA headers (just the class,
not the whole camera_sensor.h), or create an IPACameraSensorInfo that
can be constructed from a CameraSensorInfo.

> And now I were to talk about the actual IPA (out-of-tree one), it would 
> require
> 
> #include "libcamera/internal/log.h"
> #include "libcamera/internal/file.h"
> #include "libcamera/internal/buffer.h"
> *1
>
> > We need to identify which files are required by the IPA interfaces, and
> > find a way (or location) to install them, which makes them available to
> > the IPA interface, but makes it clear that they are /not/ a part of the
> > public API interfaces, and should not be accessible by applications.
>
> Do you mean the headers(*1)  for actual IPA should be provided via the 
> IPA interface, i.e. the IPA interface shall wrap any internal headers 
> the /actual/ IPA might require?
>
> > Given that they are only to be used by the IPA, we could perhaps install
> > a subset of them into libcamera/ipa/internal/ or something like that.

They're not part of libipa, I'd rather split them to a utils library
(whose ABI will not be guaranteed as stable).

> > I would like to be able to see a list of what files are required. That
> > would help in how we decide what files need abstracting or installing,
> > and where they should go.

-- 
Regards,

Laurent Pinchart