[libcamera-devel] [PATCH v3 3/4] libcamera: Documentation: Split public/private documentation

Laurent Pinchart laurent.pinchart at ideasonboard.com
Mon Jan 22 17:19:45 CET 2024


Hi Dan,

Thank you for the patch.

On Fri, Jan 12, 2024 at 12:14:33PM +0000, 📷-devel wrote:
> The API reference pages generated by Doxygen are comprehensive, but
> therefore quite overwhelming for application developers who will
> likely never need to use the majority of the library's objects. To
> reduce the complexity of the documentation, split it into two runs of
> doxygen.
> 
> In the first run of doxygen we pass a specific list of source files
> to parse, which is built from the File arrays in Meson's build files.
> This ensures that we only generate the documentation for code from
> those files.
> 
> In the second run allow doxygen to generate documentation for all of
> the library's objects as it currently does. This set will now be
> output into build/Documentation/internal-api-html.
> 
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
> ---
> Changes in v3:
> 
> 	- A typo
> 	- Explicitly excluded include/libcamera/internal/span.h in
> 	  Doxyfile-public.in
> 
> Changes in v2:
> 
> 	- Formatting fixes (Jacopo)
> 	- Phraseology (Laurent)
> 	- Switched to passing specific files to parse to doxygen rather than
> 	  relying on \internal to remove other docu-comments.
> 
>  Documentation/Doxyfile-internal.in     | 21 +++++++
>  Documentation/Doxyfile-public.in       |  7 +++
>  Documentation/Doxyfile.in              | 26 ++-------
>  Documentation/meson.build              | 76 +++++++++++++++++++++++---
>  include/libcamera/base/meson.build     |  7 +++
>  include/libcamera/internal/meson.build |  7 +++
>  include/libcamera/meson.build          | 10 ++++
>  meson.build                            |  8 +++
>  src/libcamera/base/class.cpp           |  1 +
>  src/libcamera/base/meson.build         |  7 +++
>  src/libcamera/camera.cpp               |  7 +++
>  src/libcamera/camera_manager.cpp       |  1 +
>  src/libcamera/framebuffer.cpp          |  6 +-
>  src/libcamera/meson.build              |  7 +++
>  src/libcamera/request.cpp              |  1 +
>  15 files changed, 162 insertions(+), 30 deletions(-)
>  create mode 100644 Documentation/Doxyfile-internal.in
>  create mode 100644 Documentation/Doxyfile-public.in
> 
> diff --git a/Documentation/Doxyfile-internal.in b/Documentation/Doxyfile-internal.in
> new file mode 100644
> index 00000000..7b3cce49
> --- /dev/null
> +++ b/Documentation/Doxyfile-internal.in
> @@ -0,0 +1,21 @@
> +# SPDX-License-Identifier: CC-BY-SA-4.0
> +
> +INPUT                  = "@TOP_SRCDIR@/Documentation" \
> +                         "@TOP_SRCDIR@/include/libcamera" \
> +                         "@TOP_SRCDIR@/src/ipa/ipu3" \
> +                         "@TOP_SRCDIR@/src/ipa/libipa" \
> +                         "@TOP_SRCDIR@/src/libcamera" \
> +                         "@TOP_BUILDDIR@/include/libcamera" \
> +                         "@TOP_BUILDDIR@/src/libcamera"
> +
> +EXCLUDE                = @TOP_SRCDIR@/include/libcamera/base/span.h \
> +                         @TOP_SRCDIR@/include/libcamera/internal/device_enumerator_sysfs.h \
> +                         @TOP_SRCDIR@/include/libcamera/internal/device_enumerator_udev.h \
> +                         @TOP_SRCDIR@/include/libcamera/internal/ipc_pipe_unixsocket.h \
> +                         @TOP_SRCDIR@/src/libcamera/device_enumerator_sysfs.cpp \
> +                         @TOP_SRCDIR@/src/libcamera/device_enumerator_udev.cpp \
> +                         @TOP_SRCDIR@/src/libcamera/ipc_pipe_unixsocket.cpp \
> +                         @TOP_SRCDIR@/src/libcamera/pipeline/ \
> +                         @TOP_SRCDIR@/src/libcamera/tracepoints.cpp \
> +                         @TOP_BUILDDIR@/include/libcamera/internal/tracepoints.h \
> +                         @TOP_BUILDDIR@/src/libcamera/proxy/
> diff --git a/Documentation/Doxyfile-public.in b/Documentation/Doxyfile-public.in
> new file mode 100644
> index 00000000..14c2556c
> --- /dev/null
> +++ b/Documentation/Doxyfile-public.in
> @@ -0,0 +1,7 @@
> +# SPDX-License-Identifier: CC-BY-SA-4.0
> +
> +INPUT                  = "@TOP_SRCDIR@/Documentation" \
> +                         "@INPUT@" \
> +                         "@TOP_BUILDDIR@/src/libcamera/version.cpp"
> +
> +EXCLUDE                = @TOP_SRCDIR@/include/libcamera/base/span.h
> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> index 48fea8bc..a271c7bc 100644
> --- a/Documentation/Doxyfile.in
> +++ b/Documentation/Doxyfile.in
> @@ -21,14 +21,6 @@ CASE_SENSE_NAMES       = YES
>  
>  QUIET                  = YES
>  
> -INPUT                  = "@TOP_SRCDIR@/Documentation" \
> -                         "@TOP_SRCDIR@/include/libcamera" \
> -                         "@TOP_SRCDIR@/src/ipa/ipu3" \
> -                         "@TOP_SRCDIR@/src/ipa/libipa" \
> -                         "@TOP_SRCDIR@/src/libcamera" \
> -                         "@TOP_BUILDDIR@/include/libcamera" \
> -                         "@TOP_BUILDDIR@/src/libcamera"
> -
>  FILE_PATTERNS          = *.c \
>                           *.cpp \
>                           *.h \
> @@ -36,17 +28,8 @@ FILE_PATTERNS          = *.c \
>  
>  RECURSIVE              = YES
>  
> -EXCLUDE                = @TOP_SRCDIR@/include/libcamera/base/span.h \
> -                         @TOP_SRCDIR@/include/libcamera/internal/device_enumerator_sysfs.h \
> -                         @TOP_SRCDIR@/include/libcamera/internal/device_enumerator_udev.h \
> -                         @TOP_SRCDIR@/include/libcamera/internal/ipc_pipe_unixsocket.h \
> -                         @TOP_SRCDIR@/src/libcamera/device_enumerator_sysfs.cpp \
> -                         @TOP_SRCDIR@/src/libcamera/device_enumerator_udev.cpp \
> -                         @TOP_SRCDIR@/src/libcamera/ipc_pipe_unixsocket.cpp \
> -                         @TOP_SRCDIR@/src/libcamera/pipeline/ \
> -                         @TOP_SRCDIR@/src/libcamera/tracepoints.cpp \
> -                         @TOP_BUILDDIR@/include/libcamera/internal/tracepoints.h \
> -                         @TOP_BUILDDIR@/src/libcamera/proxy/
> + at INCLUDE_PATH          = @TOP_BUILDDIR@/Documentation
> + at INCLUDE               = @INCLUDE_FILE@
>  
>  EXCLUDE_PATTERNS       = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \
>                           @TOP_BUILDDIR@/include/libcamera/ipa/*_proxy.h \
> @@ -70,7 +53,10 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
>  
>  EXCLUDE_SYMLINKS       = YES
>  
> -HTML_OUTPUT            = api-html
> +HIDE_UNDOC_CLASSES     = @HIDE_UNDOC_CLASSES@
> +HIDE_UNDOC_MEMBERS     = @HIDE_UNDOC_MEMBERS@
> +HTML_OUTPUT            = @HTML_OUTPUT@
> +INTERNAL_DOCS          = @INTERNAL_DOCS@
>  
>  GENERATE_LATEX         = NO
>  
> diff --git a/Documentation/meson.build b/Documentation/meson.build
> index 7a58fec8..afaad751 100644
> --- a/Documentation/meson.build
> +++ b/Documentation/meson.build
> @@ -23,12 +23,7 @@ if doxygen.found() and dot.found()
>  
>      cdata.set('PREDEFINED', ' \\\n\t\t\t '.join(doxygen_predefined))
>  
> -    doxyfile = configure_file(input : 'Doxyfile.in',
> -                              output : 'Doxyfile',
> -                              configuration : cdata)
> -
> -    doxygen_input = [
> -        doxyfile,
> +    global_doxygen_input = [
>          libcamera_base_headers,
>          libcamera_base_sources,
>          libcamera_internal_headers,
> @@ -41,16 +36,79 @@ if doxygen.found() and dot.found()
>      ]
>  
>      if is_variable('ipu3_ipa_sources')
> -        doxygen_input += [ipu3_ipa_sources]
> +        global_doxygen_input += [ipu3_ipa_sources]
>      endif
>  
> +    # We need to generate two "include" files for the final Doxyfile which
> +    # define a set of source files to use in the documentation parsing. We
> +    # collected a list of the public sources in doxygen_public_sources, so we
> +    # pass that to the doxyfiles so that Doxyfile-public refers only to those
> +    # files. Although INPUT is sent to both, Doxyfile-internal.in doesn't refer
> +    # to it and just hardcodes the directories to parse.
> +    cdata.set('INPUT', '" \\\n\t\t\t "'.join(doxygen_public_sources))
> +    doxyfile_include_public = configure_file(input : 'Doxyfile-public.in',
> +                                             output : 'Doxyfile-include-public',
> +                                             configuration : cdata)
> +    doxyfile_include_internal = configure_file(input : 'Doxyfile-internal.in',
> +                                               output : 'Doxyfile-include-internal',
> +                                               configuration : cdata)
> +
> +    # We run doxygen twice - the first run excludes internal API objects as it
> +    # is intended to document the public API only. A second run covers all of
> +    # the library's objects for libcamera developers. To achieve this we need to
> +    # flag as \internal some of the comments for objects which we wish to hide,
> +    # and remove the auto generated documents via HIDE_UNDOC_CLASSES and
> +    # HIDE_UNDOC_MEMBERS.
> +
> +    cdata_public = configuration_data()
> +    cdata_public.merge_from(cdata)
> +    cdata_public.set('HIDE_UNDOC_CLASSES', 'YES')
> +    cdata_public.set('HIDE_UNDOC_MEMBERS', 'YES')
> +    cdata_public.set('HTML_OUTPUT', 'api-html')
> +    cdata_public.set('INCLUDE_FILE', 'Doxyfile-include-public')
> +    cdata_public.set('INTERNAL_DOCS', 'NO')
> +
> +    doxyfile_public = configure_file(input : 'Doxyfile.in',
> +                                     output : 'Doxyfile-public',
> +                                     configuration : cdata_public)

Could we avoid generating two versions of this file by switching the
@INCLUDE@ logic around, and including the common file from the -internal
and -public files ? Anything that differs between the internal and
public runs would then be moved to the corresponding file, and you could
drop some of the configuratation data here (such as HIDE_UNDOC_CLASSES
for instance). I think that would simplify the code.

> +
> +    public_doxygen_input = global_doxygen_input
> +    public_doxygen_input += doxyfile_public
> +
>      custom_target('doxygen',
> -                  input : doxygen_input,
> +                  input : public_doxygen_input,
>                    output : 'api-html',
> -                  command : [doxygen, doxyfile],
> +                  command : [doxygen, doxyfile_public],
>                    install : true,
>                    install_dir : doc_install_dir,
>                    install_tag : 'doc')
> +
> +    # This is the internal documentation, so _don't_ hide undocumented classes
> +    # as we want everything to show up and warnings to be generated if any
> +    # documentation is missing.
> +
> +    cdata_internal = configuration_data()
> +    cdata_internal.merge_from(cdata)
> +    cdata_internal.set('HIDE_UNDOC_CLASSES', 'NO')
> +    cdata_internal.set('HIDE_UNDOC_MEMBERS', 'NO')
> +    cdata_internal.set('HTML_OUTPUT', 'internal-api-html')
> +    cdata_internal.set('INCLUDE_FILE', 'Doxyfile-include-internal')
> +    cdata_internal.set('INTERNAL_DOCS', 'YES')
> +
> +    doxyfile_internal = configure_file(input : 'Doxyfile.in',
> +                                       output : 'Doxyfile-internal',
> +                                       configuration : cdata_internal)
> +
> +    internal_doxygen_input = global_doxygen_input
> +    internal_doxygen_input += doxyfile_internal
> +
> +    custom_target('doxygen-internal',
> +                  input : internal_doxygen_input,
> +                  output : 'internal-api-html',
> +                  command : [doxygen, doxyfile_internal],
> +                  install : true,
> +                  install_dir : doc_install_dir,
> +                  install_tag : 'doc-internal')
>  endif
>  
>  #
> diff --git a/include/libcamera/base/meson.build b/include/libcamera/base/meson.build
> index bace25d5..837ef40a 100644
> --- a/include/libcamera/base/meson.build
> +++ b/include/libcamera/base/meson.build
> @@ -38,3 +38,10 @@ libcamera_base_headers = [
>  
>  install_headers(libcamera_base_public_headers,
>                  subdir : libcamera_base_include_dir)
> +
> +foreach lbph : libcamera_base_public_headers
> +	doxygen_public_sources += '/'.join(
> +		meson.project_source_root(),
> +		'@0@'.format(lbph)
> +	)

:'-(

I'll check if I can find a cleaner alternative.

> +endforeach
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index 4e36bf36..81721559 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -58,4 +58,11 @@ libcamera_internal_headers = [
>      libcamera_internal_headers_undocumented
>  ]
>  
> +foreach lph : libcamera_internal_headers_documented
> +	doxygen_public_sources += '/'.join(
> +		meson.project_source_root(),
> +		'@0@'.format(lph)
> +	)
> +endforeach
> +
>  subdir('converter')
> diff --git a/include/libcamera/meson.build b/include/libcamera/meson.build
> index 84c6c4cb..59273785 100644
> --- a/include/libcamera/meson.build
> +++ b/include/libcamera/meson.build
> @@ -26,6 +26,13 @@ subdir('ipa')
>  install_headers(libcamera_public_headers,
>                  subdir : libcamera_include_dir)
>  
> +foreach lph : libcamera_public_headers
> +	doxygen_public_sources += '/'.join(
> +		meson.project_source_root(),
> +		'@0@'.format(lph)
> +	)
> +endforeach
> +
>  #
>  # Generate headers from templates.
>  #
> @@ -86,6 +93,7 @@ foreach mode, entry : controls_map
>                                                  '-r', ranges_file, '@INPUT@'],
>                                       install : true,
>                                       install_dir : libcamera_headers_install_dir)
> +    doxygen_public_sources += control_headers.get(-1).full_path()
>  endforeach
>  
>  libcamera_public_headers += control_headers
> @@ -102,6 +110,7 @@ formats_h = custom_target('formats_h',
>                            install : true,
>                            install_dir : libcamera_headers_install_dir)
>  libcamera_public_headers += formats_h
> +doxygen_public_sources += formats_h.full_path()
>  
>  # libcamera.h
>  libcamera_h = custom_target('gen-header',
> @@ -112,6 +121,7 @@ libcamera_h = custom_target('gen-header',
>                              install_dir : libcamera_headers_install_dir)
>  
>  libcamera_public_headers += libcamera_h
> +doxygen_public_sources += libcamera_h.full_path()
>  
>  # version.h
>  version = libcamera_version.split('.')
> diff --git a/meson.build b/meson.build
> index cb6b666a..2530c94b 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -231,6 +231,14 @@ endif
>  # Utilities are parsed first to provide support for other components.
>  subdir('utils')
>  
> +# To support auto-generation of documentation we need an array of the paths to
> +# public headers and source files so that we can tell doxygen which files to
> +# look at later. Unfortunately the inclusion of custom targets in some of the
> +# existing arrays precludes using them directly and you cannot generate File
> +# objects from generated files, so we need to collect paths to relevant files
> +# within an array.
> +doxygen_public_sources = []
> +
>  subdir('include')
>  subdir('src')
>  
> diff --git a/src/libcamera/base/class.cpp b/src/libcamera/base/class.cpp
> index 9c2d9f21..70fd5cd5 100644
> --- a/src/libcamera/base/class.cpp
> +++ b/src/libcamera/base/class.cpp
> @@ -184,6 +184,7 @@ Extensible::Extensible(std::unique_ptr<Extensible::Private> d)
>   */
>  
>  /**
> + * \internal
>   * \class Extensible::Private
>   * \brief Base class for private data managed through a d-pointer
>   */
> diff --git a/src/libcamera/base/meson.build b/src/libcamera/base/meson.build
> index 523c5885..ae949a51 100644
> --- a/src/libcamera/base/meson.build
> +++ b/src/libcamera/base/meson.build
> @@ -30,6 +30,13 @@ libcamera_base_sources = [
>  	libcamera_base_internal_sources
>  ]
>  
> +foreach lbps : libcamera_base_public_sources
> +	doxygen_public_sources += '/'.join(
> +		meson.project_source_root(),
> +		'@0@'.format(lbps)
> +	)
> +endforeach
> +
>  libdw = dependency('libdw', required : false)
>  libunwind = dependency('libunwind', required : false)
>  
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index a71dc933..6e57f247 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -560,6 +560,13 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF
>   */
>  
>  /**
> + * \internal
> + * \file libcamera\internal\camera.h
> + * \brief Internal Camera device handling
> + */
> +
> +/**
> + * \internal
>   * \class Camera::Private
>   * \brief Base class for camera private data
>   *
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 355f3ada..61d45256 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -23,6 +23,7 @@
>   */
>  
>  /**
> + * \internal
>   * \file libcamera/internal/camera_manager.h
>   * \brief Internal camera manager support
>   */
> diff --git a/src/libcamera/framebuffer.cpp b/src/libcamera/framebuffer.cpp
> index 5a7f3c0b..db450e11 100644
> --- a/src/libcamera/framebuffer.cpp
> +++ b/src/libcamera/framebuffer.cpp
> @@ -16,7 +16,10 @@
>  /**
>   * \file libcamera/framebuffer.h
>   * \brief Frame buffer handling
> - *
> + */
> +
> +/**
> + * \internal
>   * \file libcamera/internal/framebuffer.h
>   * \brief Internal frame buffer handling support
>   */
> @@ -105,6 +108,7 @@ LOG_DEFINE_CATEGORY(Buffer)
>   */
>  
>  /**
> + * \internal
>   * \class FrameBuffer::Private
>   * \brief Base class for FrameBuffer private data
>   *
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 676470c1..413e14db 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -59,6 +59,13 @@ libcamera_sources = [
>  	libcamera_internal_sources
>  ]
>  
> +foreach lps : libcamera_public_sources
> +	doxygen_public_sources += '/'.join(
> +		meson.project_source_root(),
> +		'@0@'.format(lps)
> +	)
> +endforeach
> +
>  libcamera_sources += libcamera_public_headers
>  libcamera_sources += libcamera_generated_ipa_headers
>  libcamera_sources += libcamera_tracepoint_header
> diff --git a/src/libcamera/request.cpp b/src/libcamera/request.cpp
> index 949c556f..930d8c92 100644
> --- a/src/libcamera/request.cpp
> +++ b/src/libcamera/request.cpp
> @@ -33,6 +33,7 @@ namespace libcamera {
>  LOG_DEFINE_CATEGORY(Request)
>  
>  /**
> + * \internal
>   * \class Request::Private
>   * \brief Request private data
>   *

-- 
Regards,

Laurent Pinchart


More information about the libcamera-devel mailing list