[libcamera-devel] [PATCH v2 4/5] libcamera: Documentation: Split public/private documentation
Dan Scally
dan.scally at ideasonboard.com
Fri Jan 12 11:57:45 CET 2024
Hi Jacopo
On 09/01/2024 14:28, Jacopo Mondi wrote:
> Hi Dan
> I really like the split!
>
> On Fri, Jan 05, 2024 at 04:41:03PM +0000, Daniel Scally via libcamera-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.
>>
>> Signed-off-by: Daniel Scally <dan.scally at ideasonboard.com>
>> ---
>> 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 | 5 ++
>> 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, 160 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 \
> I'm still a bit puzzled on why we don't document Span<>, but this was
> already here
>
>> + @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..cdbc03a0
>> --- /dev/null
>> +++ b/Documentation/Doxyfile-public.in
>> @@ -0,0 +1,5 @@
>> +# SPDX-License-Identifier: CC-BY-SA-4.0
>> +
>> +INPUT = "@TOP_SRCDIR@/Documentation" \
>> + "@INPUT@" \
>> + "@TOP_BUILDDIR@/src/libcamera/version.cpp"
>> 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.
> Thanks, this is quite clear now!
>
>> + 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)
>> +
>> + 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 f24f47de..82277f46 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)
>> + )
>> +endforeach
>> diff --git a/include/libcamera/internal/meson.build b/include/libcamera/internal/meson.build
>> index 4d59cb2a..4af36884 100644
>> --- a/include/libcamera/internal/meson.build
>> +++ b/include/libcamera/internal/meson.build
>> @@ -58,4 +58,11 @@ libcamera_internal_headers = [
>> libcamera_internal_headers_publically_undocumented
>> ]
>>
>> +foreach lph : libcamera_internal_headers_publically_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 bab858a3..25e2f8a4 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.
>> #
>> @@ -85,6 +92,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
>> @@ -101,6 +109,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',
>> @@ -111,6 +120,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 e49de4c2..cca1883e 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
> s/We/we
>
>> +# 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 0ad1a4b5..ed46f853 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
>> */
> That's weird, I don't see CameraManager::Private::addCamera() and
> CameraManager::Private::removeCamera() being documented. It was not
> documented even before this commit though :/
Turns out CameraManager::Private is actually explicitly excluded at the symbol level in
Documentation/Doxyfile.in, so that's why. So; probably unecessary to flag these...but perhaps for
consistency's sake it's best to follow the rule that any public class deriving a private from
::Extensible needs flagging.
>
>> 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
>> *
> So we should remember to mark every classes that derives a private
> implementation from Extensible to mark it as \internal. I think it's
> fine, and I like the patch a lot!
>
> With the few above points clarified
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>
> Thanks
> j
>
>> --
>> 2.34.1
>>
More information about the libcamera-devel
mailing list