[libcamera-devel] [PATCH v2 2/5] meson: libcamera: Split public and internal source arrays

[Kieran Bingham]

Quoting Jacopo Mondi via libcamera-devel (2024-01-09 13:41:51)
> Hi Dan
> 
> On Fri, Jan 05, 2024 at 04:41:01PM +0000, Daniel Scally via libcamera-devel wrote:
> > Meson array variables hold lists of libcamera's source files. To help
> > facilitate the splitting of Doxygen generated documentation into
> > distinct public and internal versions, split those arrays to separate
> > public and internal variables.
> >
> > Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> > ---
> > Changes in v2:
> >
> >       - New patch
> >
> >  include/libcamera/base/meson.build     |  2 +-
> >  include/libcamera/internal/meson.build | 21 +++++++++++----
> >  src/libcamera/base/meson.build         | 24 +++++++++++------
> >  src/libcamera/meson.build              | 36 ++++++++++++++++----------
> >  4 files changed, 55 insertions(+), 28 deletions(-)
> >
> > diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build
> > index bace25d5..f24f47de 100644
> > --- a/include/libcamera/base/meson.build
> > +++ b/include/libcamera/base/meson.build
> > @@ -10,7 +10,6 @@ libcamera_base_public_headers = files([
> >      'object.h',
> >      'shared_fd.h',
> >      'signal.h',
> > -    'span.h',
> >      'unique_fd.h',
> >  ])
> >
> > @@ -25,6 +24,7 @@ libcamera_base_private_headers = files([
> >      'mutex.h',
> >      'private.h',
> >      'semaphore.h',
> > +    'span.h',
> 
> Span<> is part of our public API, isn't it ?

I bet it's a real pain to get Doxygen to document though. We might want
a short explanation that the C++20 documentation can be used as a
reference and we'll move to that implementation when we can.

> 
> The rest looks good, I presume I'll see in the next patches how this
> split is used
> 
> Thanks
>   j
> 
> >      'thread.h',
> >      'thread_annotations.h',
> >      'timer.h',
> > diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> > index 7f1f3440..4d59cb2a 100644
> > --- a/include/libcamera/internal/meson.build
> > +++ b/include/libcamera/internal/meson.build
> > @@ -9,13 +9,21 @@ libcamera_tracepoint_header = custom_target(
> >      command : [gen_tracepoints_header, include_build_dir, '@OUTPUT@', '@INPUT@'],
> >  )
> >
> > -libcamera_internal_headers = files([
> > +# Where libcamera's public API classes have Extensible::Private counterparts we
> > +# need to include them in doxygen's public API run or it will complain that the
> > +# functions defined in the "public" source are undeclared.
> > +libcamera_internal_headers_publically_documented = files([
> > +    'camera.h',
> > +    'camera_manager.h',
> > +    'framebuffer.h',
> > +    'request.h',
> > +])
> > +
> > +libcamera_internal_headers_publically_undocumented = files([
> 
> I would just s/_publically//

Seconded.

I think treating Span as a special case is probably ok. But we should
probably add documentation for it 'somewhere', but it doesn't have to be
in this series, so I think I'd already add:


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

> 
> >      'bayer_format.h',
> >      'byte_stream_buffer.h',
> > -    'camera.h',
> >      'camera_controls.h',
> >      'camera_lens.h',
> > -    'camera_manager.h',
> >      'camera_sensor.h',
> >      'camera_sensor_properties.h',
> >      'control_serializer.h',
> > @@ -26,7 +34,6 @@ libcamera_internal_headers = files([
> >      'device_enumerator_sysfs.h',
> >      'device_enumerator_udev.h',
> >      'formats.h',
> > -    'framebuffer.h',
> >      'ipa_manager.h',
> >      'ipa_module.h',
> >      'ipa_proxy.h',
> > @@ -37,7 +44,6 @@ libcamera_internal_headers = files([
> >      'pipeline_handler.h',
> >      'process.h',
> >      'pub_key.h',
> > -    'request.h',
> >      'source_paths.h',
> >      'sysfs.h',
> >      'v4l2_device.h',
> > @@ -47,4 +53,9 @@ libcamera_internal_headers = files([
> >      'yaml_parser.h',
> >  ])
> >
> > +libcamera_internal_headers = [
> > +    libcamera_internal_headers_publically_documented,
> > +    libcamera_internal_headers_publically_undocumented
> > +]
> > +
> >  subdir('converter')
> > diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
> > index 7a7fd7e4..523c5885 100644
> > --- a/src/libcamera/base/meson.build
> > +++ b/src/libcamera/base/meson.build
> > @@ -1,27 +1,35 @@
> >  # SPDX-License-Identifier: CC0-1.0
> >
> > -libcamera_base_sources = files([
> > -    'backtrace.cpp',
> > -    'class.cpp',
> > +libcamera_base_public_sources = files([
> >      'bound_method.cpp',
> > +    'class.cpp',
> > +    'flags.cpp',
> > +    'object.cpp',
> > +    'shared_fd.cpp',
> > +    'signal.cpp',
> > +    'unique_fd.cpp',
> > +])
> > +
> > +libcamera_base_internal_sources = files([
> > +    'backtrace.cpp',
> >      'event_dispatcher.cpp',
> >      'event_dispatcher_poll.cpp',
> >      'event_notifier.cpp',
> >      'file.cpp',
> > -    'flags.cpp',
> >      'log.cpp',
> >      'message.cpp',
> >      'mutex.cpp',
> > -    'object.cpp',
> >      'semaphore.cpp',
> > -    'shared_fd.cpp',
> > -    'signal.cpp',
> >      'thread.cpp',
> >      'timer.cpp',
> > -    'unique_fd.cpp',
> >      'utils.cpp',
> >  ])
> >
> > +libcamera_base_sources = [
> > +     libcamera_base_public_sources,
> > +     libcamera_base_internal_sources
> > +]
> > +
> >  libdw = dependency('libdw', required : false)
> >  libunwind = dependency('libunwind', required : false)
> >
> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> > index 45f63e93..676470c1 100644
> > --- a/src/libcamera/meson.build
> > +++ b/src/libcamera/meson.build
> > @@ -1,27 +1,35 @@
> >  # SPDX-License-Identifier: CC0-1.0
> >
> > -libcamera_sources = files([
> > +libcamera_public_sources = files([
> > +    'camera.cpp',
> > +    'camera_manager.cpp',
> > +    'color_space.cpp',
> > +    'controls.cpp',
> > +    'fence.cpp',
> > +    'framebuffer.cpp',
> > +    'framebuffer_allocator.cpp',
> > +    'geometry.cpp',
> > +    'orientation.cpp',
> > +    'pixel_format.cpp',
> > +    'request.cpp',
> > +    'stream.cpp',
> > +    'transform.cpp',
> > +])
> > +
> > +libcamera_internal_sources = files([
> >      'bayer_format.cpp',
> >      'byte_stream_buffer.cpp',
> > -    'camera.cpp',
> >      'camera_controls.cpp',
> >      'camera_lens.cpp',
> > -    'camera_manager.cpp',
> >      'camera_sensor.cpp',
> >      'camera_sensor_properties.cpp',
> > -    'color_space.cpp',
> > -    'controls.cpp',
> >      'control_serializer.cpp',
> >      'control_validator.cpp',
> >      'converter.cpp',
> >      'delayed_controls.cpp',
> >      'device_enumerator.cpp',
> >      'device_enumerator_sysfs.cpp',
> > -    'fence.cpp',
> >      'formats.cpp',
> > -    'framebuffer.cpp',
> > -    'framebuffer_allocator.cpp',
> > -    'geometry.cpp',
> >      'ipa_controls.cpp',
> >      'ipa_data_serializer.cpp',
> >      'ipa_interface.cpp',
> > @@ -34,16 +42,11 @@ libcamera_sources = files([
> >      'mapped_framebuffer.cpp',
> >      'media_device.cpp',
> >      'media_object.cpp',
> > -    'orientation.cpp',
> >      'pipeline_handler.cpp',
> > -    'pixel_format.cpp',
> >      'process.cpp',
> >      'pub_key.cpp',
> > -    'request.cpp',
> >      'source_paths.cpp',
> > -    'stream.cpp',
> >      'sysfs.cpp',
> > -    'transform.cpp',
> >      'v4l2_device.cpp',
> >      'v4l2_pixelformat.cpp',
> >      'v4l2_subdevice.cpp',
> > @@ -51,6 +54,11 @@ libcamera_sources = files([
> >      'yaml_parser.cpp',
> >  ])
> >
> > +libcamera_sources = [
> > +     libcamera_public_sources,
> > +     libcamera_internal_sources
> > +]
> > +
> >  libcamera_sources += libcamera_public_headers
> >  libcamera_sources += libcamera_generated_ipa_headers
> >  libcamera_sources += libcamera_tracepoint_header
> > --
> > 2.34.1
> >