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

19373 diff mbox series

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@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(-)

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@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 ?

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

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

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

On 09/01/2024 22:12, Kieran Bingham wrote:
> 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@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.


Probably this needed explaining better - apologies. Span<> is specifically excluded at the moment 
via Doxygen's EXCLUDE keyword in the Doxyfile - so it looks a bit weird in this commit, but it's 
maintaining the current treatment for the file.


Alternatively I could leave these arrays untouched and simply keep the EXCLUDE in Doxyfile-public.in 
to exclude Span<> that way - is that less weird?

>
>> 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@ideasonboard.com>


Thanks!

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

Quoting Dan Scally (2024-01-12 10:25:09)
> 
> On 09/01/2024 22:12, Kieran Bingham wrote:
> > 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@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.
> 
> 
> Probably this needed explaining better - apologies. Span<> is specifically excluded at the moment 
> via Doxygen's EXCLUDE keyword in the Doxyfile - so it looks a bit weird in this commit, but it's 
> maintaining the current treatment for the file.
> 
> 

> Alternatively I could leave these arrays untouched and simply keep the
> EXCLUDE in Doxyfile-public.in to exclude Span<> that way - is that
> less weird?

I think so. It's probably important to keep span.h in the
libcamera_base_public_headers array so that it gets installed as part of
a libcamera package.

In fact, I think I overlooked that before. If this prevents the span.h
being installed, that's a breakage so we can't do that.

In otherwords, your new proposal sounds like something we should do.

--
Kieran

On 12/01/2024 10:51, Kieran Bingham wrote:
> Quoting Dan Scally (2024-01-12 10:25:09)
>> On 09/01/2024 22:12, Kieran Bingham wrote:
>>> 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@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.
>>
>> Probably this needed explaining better - apologies. Span<> is specifically excluded at the moment
>> via Doxygen's EXCLUDE keyword in the Doxyfile - so it looks a bit weird in this commit, but it's
>> maintaining the current treatment for the file.
>>
>>
>> Alternatively I could leave these arrays untouched and simply keep the
>> EXCLUDE in Doxyfile-public.in to exclude Span<> that way - is that
>> less weird?
> I think so. It's probably important to keep span.h in the
> libcamera_base_public_headers array so that it gets installed as part of
> a libcamera package.
>
> In fact, I think I overlooked that before. If this prevents the span.h
> being installed, that's a breakage so we can't do that.
>
> In otherwords, your new proposal sounds like something we should do.


Ack - doing that now then

> --
> Kieran