[PATCH v4 3/4] libcamera: Documentation: Split public/private documentation
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sun Aug 4 20:48:51 CEST 2024
Hi Dan,
Thank you for the patch.
On Wed, Jul 31, 2024 at 02:52:00PM +0100, Daniel Scally 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 v4:
>
> - Reversed the split methodology to include a common Doxyfile in both of
> the internal and public Doxyfiles, rather than generating a separate
> include for each. This simplies the meson.build.
>
> 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 | 30 ++++++++++++++++
> Documentation/Doxyfile-public.in | 15 ++++++++
> Documentation/Doxyfile.in | 23 ------------
> Documentation/meson.build | 48 +++++++++++++++++++++++---
> 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, 149 insertions(+), 29 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..9d4cf9d9
> --- /dev/null
> +++ b/Documentation/Doxyfile-internal.in
> @@ -0,0 +1,30 @@
> +# SPDX-License-Identifier: CC-BY-SA-4.0
> +
> + at INCLUDE_PATH = @TOP_BUILDDIR@/Documentation
> + at INCLUDE = Doxyfile
> +
> +HIDE_UNDOC_CLASSES = NO
> +HIDE_UNDOC_MEMBERS = NO
> +HTML_OUTPUT = internal-api-html
> +INTERNAL_DOCS = 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"
> +
> +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@/include/libcamera/ipa/soft_ipa_interface.h \
> + @TOP_BUILDDIR@/src/libcamera/proxy/
> diff --git a/Documentation/Doxyfile-public.in b/Documentation/Doxyfile-public.in
> new file mode 100644
> index 00000000..7fbe53d5
> --- /dev/null
> +++ b/Documentation/Doxyfile-public.in
> @@ -0,0 +1,15 @@
> +# SPDX-License-Identifier: CC-BY-SA-4.0
> +
> + at INCLUDE_PATH = @TOP_BUILDDIR@/Documentation
> + at INCLUDE = Doxyfile
> +
> +HIDE_UNDOC_CLASSES = YES
> +HIDE_UNDOC_MEMBERS = YES
> +HTML_OUTPUT = api-html
> +INTERNAL_DOCS = NO
> +
> +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 dd1d909b..a70aee43 100644
> --- a/Documentation/Doxyfile.in
> +++ b/Documentation/Doxyfile.in
> @@ -22,14 +22,6 @@ CASE_SENSE_NAMES = YES
> QUIET = YES
> WARN_AS_ERROR = @WARN_AS_ERROR@
>
> -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 \
> *.dox \
> @@ -37,19 +29,6 @@ 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@/include/libcamera/ipa/soft_ipa_interface.h \
> - @TOP_BUILDDIR@/src/libcamera/proxy/
> -
> EXCLUDE_PATTERNS = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \
> @TOP_BUILDDIR@/include/libcamera/ipa/*_proxy.h \
> @TOP_BUILDDIR@/include/libcamera/ipa/ipu3_*.h \
> @@ -72,8 +51,6 @@ EXCLUDE_SYMBOLS = libcamera::BoundMethodArgs \
>
> EXCLUDE_SYMLINKS = YES
>
> -HTML_OUTPUT = api-html
> -
> GENERATE_LATEX = NO
>
> MACRO_EXPANSION = YES
> diff --git a/Documentation/meson.build b/Documentation/meson.build
> index 30d39523..8349d924 100644
> --- a/Documentation/meson.build
> +++ b/Documentation/meson.build
> @@ -24,12 +24,22 @@ if doxygen.found() and dot.found()
>
> cdata.set('PREDEFINED', ' \\\n\t\t\t '.join(doxygen_predefined))
>
> + # 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. Common configuration is
> + # set in an initially generated Doxyfile, which is then included by the two
> + # final Doxyfiles. We collected a list of the public sources in
> + # doxygen_public_sources to pass to the public run and include it in cdata
> + # here - although both Doxyfile and Doxyfile-internal will also receive
> + # the same parameter they simply ignore it.
> +
> + cdata.set('INPUT', '" \\\n\t\t\t "'.join(doxygen_public_sources))
> +
> doxyfile = configure_file(input : 'Doxyfile.in',
> output : 'Doxyfile',
> configuration : cdata)
>
> - doxygen_input = [
> - doxyfile,
Doxygen includes Doxyfile in Doxyfile-public and Doxyfile-internal, but
meson doesn't know about that dependency. I think you need to keep
doxyfile here to handle the dependency correctly and ensure the
documentation gets properly recompiled.
> + global_doxygen_input = [
> libcamera_base_headers,
> libcamera_base_sources,
> libcamera_internal_headers,
> @@ -42,16 +52,44 @@ if doxygen.found() and dot.found()
> ]
>
> if is_variable('ipu3_ipa_sources')
> - doxygen_input += [ipu3_ipa_sources]
> + global_doxygen_input += [ipu3_ipa_sources]
> endif
I think that's only needed for the internal documentation.
>
> + # This is the "public" run of doxygen generating an abridged version of the
> + # API's documentation, as informed by the INPUT parameter in cdata.
> +
> + doxyfile_public = configure_file(input : 'Doxyfile-public.in',
> + output : 'Doxyfile-public',
> + configuration : cdata)
> +
> + 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, which hard-codes a list of directories
> + # to parse in its doxyfile and so ignores the INPUT parameter from cdata.
> +
> + doxyfile_internal = configure_file(input : 'Doxyfile-internal.in',
> + output : 'Doxyfile-internal',
> + configuration : cdata)
> +
> + 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)
Looks like we'll need to do better:
include/libcamera/base/meson.build:46: DEPRECATION: Project uses feature that was always broken, and is now deprecated since '1.3.0': str.format: Value other than strings, integers, bools, options, dictionaries and lists thereof..
:-(
The file object gained a full_path() method in v1.4.0, but we can't
depend on that yet.
For the public headers, I think we could set RECURSIVE = NO in
Doxyfile-public.in, and add
@TOP_SRCDIR@/include/libcamera
@TOP_BUILDDIR@/include/libcamera
the INPUT. Due to what I think could be considered as a bug in doxygen,
IMAGE_PATH seems to be affected by RECURSIVE:
[2/104] Generating Documentation/doxygen with a custom command
FAILED: Documentation/api-html
/usr/bin/doxygen Documentation/Doxyfile-public
src/libcamera/orientation.cpp:42: error: image file rotation/rotate0.svg is not found in IMAGE_PATH: assuming external image. (warning treated as error, aborting now)
Setting IMAGE_PATH to
IMAGE_PATH = "@TOP_SRCDIR@/Documentation/images" \
"@TOP_SRCDIR@/Documentation/images/rotation"
seems to work around the issue. If we submit a bug report to get this
fixed in Doxygen, I think we can live with this for now.
This won't help for the source files, and would also not be a good
option for include/libcamera/base/, as it contains a mix of public and
internal headers. We could move the latter to
include/libcamera/base/internal (or include/libcamera/internal/base),
but I'm not thrilled. So it looks like we need something better. I'm
experimenting and will likely send a v5.
> + )
> +endforeach
> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
> index 76c939bf..53fc2846 100644
> --- a/include/libcamera/internal/meson.build
> +++ b/include/libcamera/internal/meson.build
> @@ -60,5 +60,12 @@ 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')
> subdir('software_isp')
> 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 2acd8c3e..8084f2fb 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -241,6 +241,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 61998398..b7ef3a1e 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 67f34901..63285e2b 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 95a9e326..5a21132a 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 63d679cb..0f8c8f9d 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 9c74241d..1cc24cdc 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 cfb451e9..e8604121 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