[v5,16/18] Documentation: Split public/private documentation

20802 diff mbox series

From: Daniel Scally <dan.scally@ideasonboard.com>

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.

The first run of doxygen is for the public API. We pass a specific list
of source files to parse, which is built from the arrays of public
headers and sources in meson build files. This ensures that we only
generate the documentation for code from those files.

A custom Python script is needed to add the list of input files to
Doxyfile, as several of the objects included in the header and source
array are custom_tgt objects, which can't be handled as strings to
populate a variable in the configuration data.

The headers defining the Extensible and Object classes (class.h and
object.h respectively), as well as the corresponding source files, are
excluded from the public API documentation despite being referenced in
the meson public headers and sources arrays. This is due to the fact
that public API classes inherit from Extensible and Object, making the
Extensible and Object classes part of the public ABI. Those two base
classes are however implementation details and must not be accessed
directly by application code.

The second run of doxygen is for the internal API. This contains
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@ideasonboard.com>
Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 .../{Doxyfile.in => Doxyfile-common.in}       | 24 -------
 Documentation/Doxyfile-internal.in            | 30 ++++++++
 Documentation/Doxyfile-public.in              | 20 ++++++
 Documentation/gen-doxyfile.py                 | 46 ++++++++++++
 Documentation/meson.build                     | 70 +++++++++++++++++--
 5 files changed, 160 insertions(+), 30 deletions(-)
 rename Documentation/{Doxyfile.in => Doxyfile-common.in} (62%)
 create mode 100644 Documentation/Doxyfile-internal.in
 create mode 100644 Documentation/Doxyfile-public.in
 create mode 100755 Documentation/gen-doxyfile.py

Hi Laurent - thanks for the patch

On 05/08/2024 15:36, Laurent Pinchart wrote:
> From: Daniel Scally <dan.scally@ideasonboard.com>
>
> 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.
>
> The first run of doxygen is for the public API. We pass a specific list
> of source files to parse, which is built from the arrays of public
> headers and sources in meson build files. This ensures that we only
> generate the documentation for code from those files.
>
> A custom Python script is needed to add the list of input files to
> Doxyfile, as several of the objects included in the header and source
> array are custom_tgt objects, which can't be handled as strings to
> populate a variable in the configuration data.
>
> The headers defining the Extensible and Object classes (class.h and
> object.h respectively), as well as the corresponding source files, are
> excluded from the public API documentation despite being referenced in
> the meson public headers and sources arrays. This is due to the fact
> that public API classes inherit from Extensible and Object, making the
> Extensible and Object classes part of the public ABI. Those two base
> classes are however implementation details and must not be accessed
> directly by application code.
>
> The second run of doxygen is for the internal API. This contains
> 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@ideasonboard.com>
> Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---

The python script to generate the list of input files is definitely better than scattered foreach 
loops collecting a list in a hacky way - good improvement.

>   .../{Doxyfile.in => Doxyfile-common.in}       | 24 -------
>   Documentation/Doxyfile-internal.in            | 30 ++++++++
>   Documentation/Doxyfile-public.in              | 20 ++++++
>   Documentation/gen-doxyfile.py                 | 46 ++++++++++++
>   Documentation/meson.build                     | 70 +++++++++++++++++--
>   5 files changed, 160 insertions(+), 30 deletions(-)
>   rename Documentation/{Doxyfile.in => Doxyfile-common.in} (62%)
>   create mode 100644 Documentation/Doxyfile-internal.in
>   create mode 100644 Documentation/Doxyfile-public.in
>   create mode 100755 Documentation/gen-doxyfile.py
>
> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile-common.in
> similarity index 62%
> rename from Documentation/Doxyfile.in
> rename to Documentation/Doxyfile-common.in
> index 6e5a3830ec38..a70aee430e77 100644
> --- a/Documentation/Doxyfile.in
> +++ b/Documentation/Doxyfile-common.in
> @@ -17,20 +17,11 @@ EXTENSION_MAPPING      = h=C++
>   
>   TOC_INCLUDE_HEADINGS   = 0
>   
> -INTERNAL_DOCS          = YES
>   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 \
> @@ -38,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 \
> @@ -73,8 +51,6 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
>   
>   EXCLUDE_SYMLINKS       = YES
>   
> -HTML_OUTPUT            = api-html
> -
>   GENERATE_LATEX         = NO
>   
>   MACRO_EXPANSION        = YES
> diff --git a/Documentation/Doxyfile-internal.in b/Documentation/Doxyfile-internal.in
> new file mode 100644
> index 000000000000..5e0310091416
> --- /dev/null
> +++ b/Documentation/Doxyfile-internal.in
> @@ -0,0 +1,30 @@
> +# SPDX-License-Identifier: CC-BY-SA-4.0
> +
> +@INCLUDE_PATH          = @TOP_BUILDDIR@/Documentation
> +@INCLUDE               = Doxyfile-common
> +
> +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 000000000000..36bb57584a07
> --- /dev/null
> +++ b/Documentation/Doxyfile-public.in
> @@ -0,0 +1,20 @@
> +# SPDX-License-Identifier: CC-BY-SA-4.0
> +
> +@INCLUDE_PATH          = @TOP_BUILDDIR@/Documentation
> +@INCLUDE               = Doxyfile-common
> +
> +HIDE_UNDOC_CLASSES     = YES
> +HIDE_UNDOC_MEMBERS     = YES
> +HTML_OUTPUT            = api-html
> +INTERNAL_DOCS          = NO
> +
> +INPUT                  = "@TOP_SRCDIR@/Documentation" \
> +                         ${inputs}
> +
> +EXCLUDE                = @TOP_SRCDIR@/include/libcamera/base/class.h \
> +                         @TOP_SRCDIR@/include/libcamera/base/object.h \
> +                         @TOP_SRCDIR@/include/libcamera/base/span.h \
> +                         @TOP_SRCDIR@/src/libcamera/base/class.cpp \
> +                         @TOP_SRCDIR@/src/libcamera/base/object.cpp
> +
> +PREDEFINED            += __DOXYGEN_PUBLIC__
> diff --git a/Documentation/gen-doxyfile.py b/Documentation/gen-doxyfile.py
> new file mode 100755
> index 000000000000..c2a4184dd195
> --- /dev/null
> +++ b/Documentation/gen-doxyfile.py
> @@ -0,0 +1,46 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (C) 2024, Google Inc.
Does this mean it's drawn from elsewhere? Or is it a copy-paste error?
> +#
> +# Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> +#
> +# Generate Doxyfile from a template
> +
> +import argparse
> +import os
> +import string
> +import sys
> +
> +
> +def fill_template(template, data):
> +
> +    template = open(template, 'rb').read()
> +    template = template.decode('utf-8')
> +    template = string.Template(template)
I'd add a line break here.
> +    return template.substitute(data)
> +
> +
> +def main(argv):
> +
> +    # Parse command line arguments
The comment can probably be skipped.
> +    parser = argparse.ArgumentParser()
> +    parser.add_argument('-o', dest='output', metavar='file',
> +                        type=argparse.FileType('w', encoding='utf-8'),
> +                        default=sys.stdout,
> +                        help='Output file name (default: standard output)')
> +    parser.add_argument('template', metavar='doxyfile.tmpl', type=str,
> +                        help='Doxyfile template')
> +    parser.add_argument('inputs', type=str, nargs='*',
> +                        help='Input files')
> +
> +    args = parser.parse_args(argv[1:])
> +
> +    inputs = [f'"{os.path.realpath(input)}"' for input in args.inputs]
> +    data = fill_template(args.template, {'inputs': (' \\\n' + ' ' * 25).join(inputs)})
> +    args.output.write(data)
> +
> +    return 0
> +
> +
> +if __name__ == '__main__':
> +    sys.exit(main(sys.argv))
> diff --git a/Documentation/meson.build b/Documentation/meson.build
> index 1d84ed815b50..bf28233a2ce8 100644
> --- a/Documentation/meson.build
> +++ b/Documentation/meson.build
> @@ -24,9 +24,9 @@ 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)
> +    doxyfile_common = configure_file(input : 'Doxyfile-common.in',
> +                                     output : 'Doxyfile-common',
> +                                     configuration : cdata)
>   
>       doxygen_public_input = [
>           libcamera_base_public_headers,
> @@ -50,17 +50,75 @@ if doxygen.found() and dot.found()
>           doxygen_internal_input += [ipu3_ipa_sources]
>       endif
>   
> -    custom_target('doxygen',
> +    # 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.
This comment needs correcting now as it references the older method.
> +
> +    # This is the "public" run of doxygen generating an abridged version of the
> +    # API's documentation.
> +
> +    doxyfile_tmpl = configure_file(input : 'Doxyfile-public.in',
> +                                   output : 'Doxyfile-public.tmpl',
> +                                   configuration : cdata)
> +
> +    doxyfile = custom_target('doxyfile-public',
> +                             input : [
> +                                 doxygen_public_input,
> +                             ],
> +                             output : 'Doxyfile-public',
> +                             command : [
> +                                 'gen-doxyfile.py',
> +                                 '-o', '@OUTPUT@',
> +                                 doxyfile_tmpl,
> +                                 '@INPUT@',
> +                             ])
> +
> +    custom_target('doxygen-public',
>                     input : [
>                         doxyfile,
> -                      doxygen_public_input,
> -                      doxygen_internal_input,
> +                      doxyfile_common,
>                     ],
>                     output : 'api-html',
>                     command : [doxygen, doxyfile],
>                     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.
> +
> +    doxyfile_tmpl = configure_file(input : 'Doxyfile-internal.in',
> +                                   output : 'Doxyfile-internal.tmpl',
> +                                   configuration : cdata)
> +
> +    doxyfile = custom_target('doxyfile-internal',
> +                             input : [
> +                                 doxygen_public_input,
> +                                 doxygen_internal_input,
> +                             ],
> +                             output : 'Doxyfile-internal',
> +                             command : [
> +                                 'gen-doxyfile.py',
> +                                 '-o', '@OUTPUT@',
> +                                 doxyfile_tmpl,
> +                                 '@INPUT@',
> +                             ])
I think we can skip the script for the internal run -there's no templated values to be replaced and 
the generated Doxyfile-internal is identical to the Doxyfile-internal.tmpl.
> +
> +    custom_target('doxygen-internal',
> +                  input : [
> +                      doxyfile,
> +                      doxyfile_common,
> +                  ],
> +                  output : 'internal-api-html',
> +                  command : [doxygen, doxyfile],
> +                  install : true,
> +                  install_dir : doc_install_dir,
> +                  install_tag : 'doc-internal')
>   endif
>   
>   #

Quoting Dan Scally (2024-08-07 09:30:22)
> Hi Laurent - thanks for the patch
> 
> On 05/08/2024 15:36, Laurent Pinchart wrote:
> > From: Daniel Scally <dan.scally@ideasonboard.com>
> >
> > 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.
> >
> > The first run of doxygen is for the public API. We pass a specific list
> > of source files to parse, which is built from the arrays of public
> > headers and sources in meson build files. This ensures that we only
> > generate the documentation for code from those files.
> >
> > A custom Python script is needed to add the list of input files to
> > Doxyfile, as several of the objects included in the header and source
> > array are custom_tgt objects, which can't be handled as strings to
> > populate a variable in the configuration data.
> >
> > The headers defining the Extensible and Object classes (class.h and
> > object.h respectively), as well as the corresponding source files, are
> > excluded from the public API documentation despite being referenced in
> > the meson public headers and sources arrays. This is due to the fact
> > that public API classes inherit from Extensible and Object, making the
> > Extensible and Object classes part of the public ABI. Those two base
> > classes are however implementation details and must not be accessed
> > directly by application code.
> >
> > The second run of doxygen is for the internal API. This contains
> > documentation for all of the library's objects as it currently does.
> > This set will now be output into build/Documentation/internal-api-html.

Sounds good.

> >
> > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> 
> The python script to generate the list of input files is definitely better than scattered foreach 
> loops collecting a list in a hacky way - good improvement.
> 
> >   .../{Doxyfile.in => Doxyfile-common.in}       | 24 -------
> >   Documentation/Doxyfile-internal.in            | 30 ++++++++
> >   Documentation/Doxyfile-public.in              | 20 ++++++
> >   Documentation/gen-doxyfile.py                 | 46 ++++++++++++
> >   Documentation/meson.build                     | 70 +++++++++++++++++--
> >   5 files changed, 160 insertions(+), 30 deletions(-)
> >   rename Documentation/{Doxyfile.in => Doxyfile-common.in} (62%)
> >   create mode 100644 Documentation/Doxyfile-internal.in
> >   create mode 100644 Documentation/Doxyfile-public.in
> >   create mode 100755 Documentation/gen-doxyfile.py
> >
> > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile-common.in
> > similarity index 62%
> > rename from Documentation/Doxyfile.in
> > rename to Documentation/Doxyfile-common.in
> > index 6e5a3830ec38..a70aee430e77 100644
> > --- a/Documentation/Doxyfile.in
> > +++ b/Documentation/Doxyfile-common.in
> > @@ -17,20 +17,11 @@ EXTENSION_MAPPING      = h=C++
> >   
> >   TOC_INCLUDE_HEADINGS   = 0
> >   
> > -INTERNAL_DOCS          = YES
> >   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 \
> > @@ -38,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 \
> > @@ -73,8 +51,6 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
> >   
> >   EXCLUDE_SYMLINKS       = YES
> >   
> > -HTML_OUTPUT            = api-html
> > -
> >   GENERATE_LATEX         = NO
> >   
> >   MACRO_EXPANSION        = YES
> > diff --git a/Documentation/Doxyfile-internal.in b/Documentation/Doxyfile-internal.in
> > new file mode 100644
> > index 000000000000..5e0310091416
> > --- /dev/null
> > +++ b/Documentation/Doxyfile-internal.in
> > @@ -0,0 +1,30 @@
> > +# SPDX-License-Identifier: CC-BY-SA-4.0
> > +
> > +@INCLUDE_PATH          = @TOP_BUILDDIR@/Documentation
> > +@INCLUDE               = Doxyfile-common
> > +
> > +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 000000000000..36bb57584a07
> > --- /dev/null
> > +++ b/Documentation/Doxyfile-public.in
> > @@ -0,0 +1,20 @@
> > +# SPDX-License-Identifier: CC-BY-SA-4.0
> > +
> > +@INCLUDE_PATH          = @TOP_BUILDDIR@/Documentation
> > +@INCLUDE               = Doxyfile-common
> > +
> > +HIDE_UNDOC_CLASSES     = YES
> > +HIDE_UNDOC_MEMBERS     = YES
> > +HTML_OUTPUT            = api-html
> > +INTERNAL_DOCS          = NO
> > +
> > +INPUT                  = "@TOP_SRCDIR@/Documentation" \
> > +                         ${inputs}
> > +
> > +EXCLUDE                = @TOP_SRCDIR@/include/libcamera/base/class.h \
> > +                         @TOP_SRCDIR@/include/libcamera/base/object.h \
> > +                         @TOP_SRCDIR@/include/libcamera/base/span.h \
> > +                         @TOP_SRCDIR@/src/libcamera/base/class.cpp \
> > +                         @TOP_SRCDIR@/src/libcamera/base/object.cpp
> > +
> > +PREDEFINED            += __DOXYGEN_PUBLIC__
> > diff --git a/Documentation/gen-doxyfile.py b/Documentation/gen-doxyfile.py
> > new file mode 100755
> > index 000000000000..c2a4184dd195
> > --- /dev/null
> > +++ b/Documentation/gen-doxyfile.py
> > @@ -0,0 +1,46 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +# Copyright (C) 2024, Google Inc.
> Does this mean it's drawn from elsewhere? Or is it a copy-paste error?
> > +#
> > +# Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > +#
> > +# Generate Doxyfile from a template
> > +
> > +import argparse
> > +import os
> > +import string
> > +import sys
> > +
> > +
> > +def fill_template(template, data):
> > +
> > +    template = open(template, 'rb').read()
> > +    template = template.decode('utf-8')
> > +    template = string.Template(template)
> I'd add a line break here.
> > +    return template.substitute(data)
> > +
> > +
> > +def main(argv):
> > +
> > +    # Parse command line arguments
> The comment can probably be skipped.
> > +    parser = argparse.ArgumentParser()
> > +    parser.add_argument('-o', dest='output', metavar='file',
> > +                        type=argparse.FileType('w', encoding='utf-8'),
> > +                        default=sys.stdout,
> > +                        help='Output file name (default: standard output)')
> > +    parser.add_argument('template', metavar='doxyfile.tmpl', type=str,
> > +                        help='Doxyfile template')
> > +    parser.add_argument('inputs', type=str, nargs='*',
> > +                        help='Input files')
> > +
> > +    args = parser.parse_args(argv[1:])
> > +
> > +    inputs = [f'"{os.path.realpath(input)}"' for input in args.inputs]
> > +    data = fill_template(args.template, {'inputs': (' \\\n' + ' ' * 25).join(inputs)})
> > +    args.output.write(data)
> > +
> > +    return 0
> > +
> > +
> > +if __name__ == '__main__':
> > +    sys.exit(main(sys.argv))
> > diff --git a/Documentation/meson.build b/Documentation/meson.build
> > index 1d84ed815b50..bf28233a2ce8 100644
> > --- a/Documentation/meson.build
> > +++ b/Documentation/meson.build
> > @@ -24,9 +24,9 @@ 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)
> > +    doxyfile_common = configure_file(input : 'Doxyfile-common.in',
> > +                                     output : 'Doxyfile-common',
> > +                                     configuration : cdata)
> >   
> >       doxygen_public_input = [
> >           libcamera_base_public_headers,
> > @@ -50,17 +50,75 @@ if doxygen.found() and dot.found()
> >           doxygen_internal_input += [ipu3_ipa_sources]
> >       endif
> >   
> > -    custom_target('doxygen',
> > +    # 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.
> This comment needs correcting now as it references the older method.
> > +
> > +    # This is the "public" run of doxygen generating an abridged version of the
> > +    # API's documentation.
> > +
> > +    doxyfile_tmpl = configure_file(input : 'Doxyfile-public.in',
> > +                                   output : 'Doxyfile-public.tmpl',
> > +                                   configuration : cdata)
> > +
> > +    doxyfile = custom_target('doxyfile-public',
> > +                             input : [
> > +                                 doxygen_public_input,
> > +                             ],
> > +                             output : 'Doxyfile-public',
> > +                             command : [
> > +                                 'gen-doxyfile.py',
> > +                                 '-o', '@OUTPUT@',
> > +                                 doxyfile_tmpl,
> > +                                 '@INPUT@',
> > +                             ])
> > +
> > +    custom_target('doxygen-public',
> >                     input : [
> >                         doxyfile,
> > -                      doxygen_public_input,
> > -                      doxygen_internal_input,
> > +                      doxyfile_common,

At least in my mail client - the indentation looks odd here and might
want checking.

> >                     ],
> >                     output : 'api-html',
> >                     command : [doxygen, doxyfile],
> >                     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.
> > +
> > +    doxyfile_tmpl = configure_file(input : 'Doxyfile-internal.in',
> > +                                   output : 'Doxyfile-internal.tmpl',
> > +                                   configuration : cdata)
> > +
> > +    doxyfile = custom_target('doxyfile-internal',
> > +                             input : [
> > +                                 doxygen_public_input,
> > +                                 doxygen_internal_input,
> > +                             ],
> > +                             output : 'Doxyfile-internal',
> > +                             command : [
> > +                                 'gen-doxyfile.py',
> > +                                 '-o', '@OUTPUT@',
> > +                                 doxyfile_tmpl,
> > +                                 '@INPUT@',
> > +                             ])
> I think we can skip the script for the internal run -there's no templated values to be replaced and 
> the generated Doxyfile-internal is identical to the Doxyfile-internal.tmpl.

I wonder if that's "future proofing" but I don't mind either way.

I have nothing much to add except a potential whitespace comment on top
of Dan's comments, so with those handled:

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

> > +
> > +    custom_target('doxygen-internal',
> > +                  input : [
> > +                      doxyfile,
> > +                      doxyfile_common,
> > +                  ],
> > +                  output : 'internal-api-html',
> > +                  command : [doxygen, doxyfile],
> > +                  install : true,
> > +                  install_dir : doc_install_dir,
> > +                  install_tag : 'doc-internal')
> >   endif
> >   
> >   #

On Wed, Aug 07, 2024 at 12:45:31PM +0100, Kieran Bingham wrote:
> Quoting Dan Scally (2024-08-07 09:30:22)
> > On 05/08/2024 15:36, Laurent Pinchart wrote:
> > > From: Daniel Scally <dan.scally@ideasonboard.com>
> > >
> > > 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.
> > >
> > > The first run of doxygen is for the public API. We pass a specific list
> > > of source files to parse, which is built from the arrays of public
> > > headers and sources in meson build files. This ensures that we only
> > > generate the documentation for code from those files.
> > >
> > > A custom Python script is needed to add the list of input files to
> > > Doxyfile, as several of the objects included in the header and source
> > > array are custom_tgt objects, which can't be handled as strings to
> > > populate a variable in the configuration data.
> > >
> > > The headers defining the Extensible and Object classes (class.h and
> > > object.h respectively), as well as the corresponding source files, are
> > > excluded from the public API documentation despite being referenced in
> > > the meson public headers and sources arrays. This is due to the fact
> > > that public API classes inherit from Extensible and Object, making the
> > > Extensible and Object classes part of the public ABI. Those two base
> > > classes are however implementation details and must not be accessed
> > > directly by application code.
> > >
> > > The second run of doxygen is for the internal API. This contains
> > > documentation for all of the library's objects as it currently does.
> > > This set will now be output into build/Documentation/internal-api-html.
> 
> Sounds good.
> 
> > >
> > > Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> > > Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > 
> > The python script to generate the list of input files is definitely better than scattered foreach 
> > loops collecting a list in a hacky way - good improvement.
> > 
> > >   .../{Doxyfile.in => Doxyfile-common.in}       | 24 -------
> > >   Documentation/Doxyfile-internal.in            | 30 ++++++++
> > >   Documentation/Doxyfile-public.in              | 20 ++++++
> > >   Documentation/gen-doxyfile.py                 | 46 ++++++++++++
> > >   Documentation/meson.build                     | 70 +++++++++++++++++--
> > >   5 files changed, 160 insertions(+), 30 deletions(-)
> > >   rename Documentation/{Doxyfile.in => Doxyfile-common.in} (62%)
> > >   create mode 100644 Documentation/Doxyfile-internal.in
> > >   create mode 100644 Documentation/Doxyfile-public.in
> > >   create mode 100755 Documentation/gen-doxyfile.py
> > >
> > > diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile-common.in
> > > similarity index 62%
> > > rename from Documentation/Doxyfile.in
> > > rename to Documentation/Doxyfile-common.in
> > > index 6e5a3830ec38..a70aee430e77 100644
> > > --- a/Documentation/Doxyfile.in
> > > +++ b/Documentation/Doxyfile-common.in
> > > @@ -17,20 +17,11 @@ EXTENSION_MAPPING      = h=C++
> > >   
> > >   TOC_INCLUDE_HEADINGS   = 0
> > >   
> > > -INTERNAL_DOCS          = YES
> > >   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 \
> > > @@ -38,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 \
> > > @@ -73,8 +51,6 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
> > >   
> > >   EXCLUDE_SYMLINKS       = YES
> > >   
> > > -HTML_OUTPUT            = api-html
> > > -
> > >   GENERATE_LATEX         = NO
> > >   
> > >   MACRO_EXPANSION        = YES
> > > diff --git a/Documentation/Doxyfile-internal.in b/Documentation/Doxyfile-internal.in
> > > new file mode 100644
> > > index 000000000000..5e0310091416
> > > --- /dev/null
> > > +++ b/Documentation/Doxyfile-internal.in
> > > @@ -0,0 +1,30 @@
> > > +# SPDX-License-Identifier: CC-BY-SA-4.0
> > > +
> > > +@INCLUDE_PATH          = @TOP_BUILDDIR@/Documentation
> > > +@INCLUDE               = Doxyfile-common
> > > +
> > > +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 000000000000..36bb57584a07
> > > --- /dev/null
> > > +++ b/Documentation/Doxyfile-public.in
> > > @@ -0,0 +1,20 @@
> > > +# SPDX-License-Identifier: CC-BY-SA-4.0
> > > +
> > > +@INCLUDE_PATH          = @TOP_BUILDDIR@/Documentation
> > > +@INCLUDE               = Doxyfile-common
> > > +
> > > +HIDE_UNDOC_CLASSES     = YES
> > > +HIDE_UNDOC_MEMBERS     = YES
> > > +HTML_OUTPUT            = api-html
> > > +INTERNAL_DOCS          = NO
> > > +
> > > +INPUT                  = "@TOP_SRCDIR@/Documentation" \
> > > +                         ${inputs}
> > > +
> > > +EXCLUDE                = @TOP_SRCDIR@/include/libcamera/base/class.h \
> > > +                         @TOP_SRCDIR@/include/libcamera/base/object.h \
> > > +                         @TOP_SRCDIR@/include/libcamera/base/span.h \
> > > +                         @TOP_SRCDIR@/src/libcamera/base/class.cpp \
> > > +                         @TOP_SRCDIR@/src/libcamera/base/object.cpp
> > > +
> > > +PREDEFINED            += __DOXYGEN_PUBLIC__
> > > diff --git a/Documentation/gen-doxyfile.py b/Documentation/gen-doxyfile.py
> > > new file mode 100755
> > > index 000000000000..c2a4184dd195
> > > --- /dev/null
> > > +++ b/Documentation/gen-doxyfile.py
> > > @@ -0,0 +1,46 @@
> > > +#!/usr/bin/env python3
> > > +# SPDX-License-Identifier: GPL-2.0-or-later
> > > +# Copyright (C) 2024, Google Inc.
> >
> > Does this mean it's drawn from elsewhere? Or is it a copy-paste error?
> >
> > > +#
> > > +# Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > +#
> > > +# Generate Doxyfile from a template
> > > +
> > > +import argparse
> > > +import os
> > > +import string
> > > +import sys
> > > +
> > > +
> > > +def fill_template(template, data):
> > > +
> > > +    template = open(template, 'rb').read()
> > > +    template = template.decode('utf-8')
> > > +    template = string.Template(template)
> >
> > I'd add a line break here.

Done.

> > > +    return template.substitute(data)
> > > +
> > > +
> > > +def main(argv):
> > > +
> > > +    # Parse command line arguments
> >
> > The comment can probably be skipped.

I'm fine with that.


> > > +    parser = argparse.ArgumentParser()
> > > +    parser.add_argument('-o', dest='output', metavar='file',
> > > +                        type=argparse.FileType('w', encoding='utf-8'),
> > > +                        default=sys.stdout,
> > > +                        help='Output file name (default: standard output)')
> > > +    parser.add_argument('template', metavar='doxyfile.tmpl', type=str,
> > > +                        help='Doxyfile template')
> > > +    parser.add_argument('inputs', type=str, nargs='*',
> > > +                        help='Input files')
> > > +
> > > +    args = parser.parse_args(argv[1:])
> > > +
> > > +    inputs = [f'"{os.path.realpath(input)}"' for input in args.inputs]
> > > +    data = fill_template(args.template, {'inputs': (' \\\n' + ' ' * 25).join(inputs)})
> > > +    args.output.write(data)
> > > +
> > > +    return 0
> > > +
> > > +
> > > +if __name__ == '__main__':
> > > +    sys.exit(main(sys.argv))
> > > diff --git a/Documentation/meson.build b/Documentation/meson.build
> > > index 1d84ed815b50..bf28233a2ce8 100644
> > > --- a/Documentation/meson.build
> > > +++ b/Documentation/meson.build
> > > @@ -24,9 +24,9 @@ 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)
> > > +    doxyfile_common = configure_file(input : 'Doxyfile-common.in',
> > > +                                     output : 'Doxyfile-common',
> > > +                                     configuration : cdata)
> > >   
> > >       doxygen_public_input = [
> > >           libcamera_base_public_headers,
> > > @@ -50,17 +50,75 @@ if doxygen.found() and dot.found()
> > >           doxygen_internal_input += [ipu3_ipa_sources]
> > >       endif
> > >   
> > > -    custom_target('doxygen',
> > > +    # 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.
> >
> > This comment needs correcting now as it references the older method.

I'll fix it.

> > > +
> > > +    # This is the "public" run of doxygen generating an abridged version of the
> > > +    # API's documentation.
> > > +
> > > +    doxyfile_tmpl = configure_file(input : 'Doxyfile-public.in',
> > > +                                   output : 'Doxyfile-public.tmpl',
> > > +                                   configuration : cdata)
> > > +
> > > +    doxyfile = custom_target('doxyfile-public',
> > > +                             input : [
> > > +                                 doxygen_public_input,
> > > +                             ],
> > > +                             output : 'Doxyfile-public',
> > > +                             command : [
> > > +                                 'gen-doxyfile.py',
> > > +                                 '-o', '@OUTPUT@',
> > > +                                 doxyfile_tmpl,
> > > +                                 '@INPUT@',
> > > +                             ])
> > > +
> > > +    custom_target('doxygen-public',
> > >                     input : [
> > >                         doxyfile,
> > > -                      doxygen_public_input,
> > > -                      doxygen_internal_input,
> > > +                      doxyfile_common,
> 
> At least in my mail client - the indentation looks odd here and might
> want checking.

It looks fine in the source code as far as I can tell. Can you fix your
mail client ? :-)

> > >                     ],
> > >                     output : 'api-html',
> > >                     command : [doxygen, doxyfile],
> > >                     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.
> > > +
> > > +    doxyfile_tmpl = configure_file(input : 'Doxyfile-internal.in',
> > > +                                   output : 'Doxyfile-internal.tmpl',
> > > +                                   configuration : cdata)
> > > +
> > > +    doxyfile = custom_target('doxyfile-internal',
> > > +                             input : [
> > > +                                 doxygen_public_input,
> > > +                                 doxygen_internal_input,
> > > +                             ],
> > > +                             output : 'Doxyfile-internal',
> > > +                             command : [
> > > +                                 'gen-doxyfile.py',
> > > +                                 '-o', '@OUTPUT@',
> > > +                                 doxyfile_tmpl,
> > > +                                 '@INPUT@',
> > > +                             ])
> >
> > I think we can skip the script for the internal run -there's no
> > templated values to be replaced and the generated Doxyfile-internal
> > is identical to the Doxyfile-internal.tmpl.
> 
> I wonder if that's "future proofing" but I don't mind either way.

The goal was to keep the code flow identical, and I considered
future-proofing too. I don't mind dropping it though.

> I have nothing much to add except a potential whitespace comment on top
> of Dan's comments, so with those handled:
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > > +
> > > +    custom_target('doxygen-internal',
> > > +                  input : [
> > > +                      doxyfile,
> > > +                      doxyfile_common,
> > > +                  ],
> > > +                  output : 'internal-api-html',
> > > +                  command : [doxygen, doxyfile],
> > > +                  install : true,
> > > +                  install_dir : doc_install_dir,
> > > +                  install_tag : 'doc-internal')
> > >   endif
> > >   
> > >   #

Hi Laurent

On 07/08/2024 14:31, Laurent Pinchart wrote:
> On Wed, Aug 07, 2024 at 12:45:31PM +0100, Kieran Bingham wrote:
>> Quoting Dan Scally (2024-08-07 09:30:22)
>>> On 05/08/2024 15:36, Laurent Pinchart wrote:
>>>> From: Daniel Scally <dan.scally@ideasonboard.com>
>>>>
>>>> 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.
>>>>
>>>> The first run of doxygen is for the public API. We pass a specific list
>>>> of source files to parse, which is built from the arrays of public
>>>> headers and sources in meson build files. This ensures that we only
>>>> generate the documentation for code from those files.
>>>>
>>>> A custom Python script is needed to add the list of input files to
>>>> Doxyfile, as several of the objects included in the header and source
>>>> array are custom_tgt objects, which can't be handled as strings to
>>>> populate a variable in the configuration data.
>>>>
>>>> The headers defining the Extensible and Object classes (class.h and
>>>> object.h respectively), as well as the corresponding source files, are
>>>> excluded from the public API documentation despite being referenced in
>>>> the meson public headers and sources arrays. This is due to the fact
>>>> that public API classes inherit from Extensible and Object, making the
>>>> Extensible and Object classes part of the public ABI. Those two base
>>>> classes are however implementation details and must not be accessed
>>>> directly by application code.
>>>>
>>>> The second run of doxygen is for the internal API. This contains
>>>> documentation for all of the library's objects as it currently does.
>>>> This set will now be output into build/Documentation/internal-api-html.
>> Sounds good.
>>
>>>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
>>>> Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>> ---
>>> The python script to generate the list of input files is definitely better than scattered foreach
>>> loops collecting a list in a hacky way - good improvement.
>>>
>>>>    .../{Doxyfile.in => Doxyfile-common.in}       | 24 -------
>>>>    Documentation/Doxyfile-internal.in            | 30 ++++++++
>>>>    Documentation/Doxyfile-public.in              | 20 ++++++
>>>>    Documentation/gen-doxyfile.py                 | 46 ++++++++++++
>>>>    Documentation/meson.build                     | 70 +++++++++++++++++--
>>>>    5 files changed, 160 insertions(+), 30 deletions(-)
>>>>    rename Documentation/{Doxyfile.in => Doxyfile-common.in} (62%)
>>>>    create mode 100644 Documentation/Doxyfile-internal.in
>>>>    create mode 100644 Documentation/Doxyfile-public.in
>>>>    create mode 100755 Documentation/gen-doxyfile.py
>>>>
>>>> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile-common.in
>>>> similarity index 62%
>>>> rename from Documentation/Doxyfile.in
>>>> rename to Documentation/Doxyfile-common.in
>>>> index 6e5a3830ec38..a70aee430e77 100644
>>>> --- a/Documentation/Doxyfile.in
>>>> +++ b/Documentation/Doxyfile-common.in
>>>> @@ -17,20 +17,11 @@ EXTENSION_MAPPING      = h=C++
>>>>    
>>>>    TOC_INCLUDE_HEADINGS   = 0
>>>>    
>>>> -INTERNAL_DOCS          = YES
>>>>    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 \
>>>> @@ -38,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 \
>>>> @@ -73,8 +51,6 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
>>>>    
>>>>    EXCLUDE_SYMLINKS       = YES
>>>>    
>>>> -HTML_OUTPUT            = api-html
>>>> -
>>>>    GENERATE_LATEX         = NO
>>>>    
>>>>    MACRO_EXPANSION        = YES
>>>> diff --git a/Documentation/Doxyfile-internal.in b/Documentation/Doxyfile-internal.in
>>>> new file mode 100644
>>>> index 000000000000..5e0310091416
>>>> --- /dev/null
>>>> +++ b/Documentation/Doxyfile-internal.in
>>>> @@ -0,0 +1,30 @@
>>>> +# SPDX-License-Identifier: CC-BY-SA-4.0
>>>> +
>>>> +@INCLUDE_PATH          = @TOP_BUILDDIR@/Documentation
>>>> +@INCLUDE               = Doxyfile-common
>>>> +
>>>> +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 000000000000..36bb57584a07
>>>> --- /dev/null
>>>> +++ b/Documentation/Doxyfile-public.in
>>>> @@ -0,0 +1,20 @@
>>>> +# SPDX-License-Identifier: CC-BY-SA-4.0
>>>> +
>>>> +@INCLUDE_PATH          = @TOP_BUILDDIR@/Documentation
>>>> +@INCLUDE               = Doxyfile-common
>>>> +
>>>> +HIDE_UNDOC_CLASSES     = YES
>>>> +HIDE_UNDOC_MEMBERS     = YES
>>>> +HTML_OUTPUT            = api-html
>>>> +INTERNAL_DOCS          = NO
>>>> +
>>>> +INPUT                  = "@TOP_SRCDIR@/Documentation" \
>>>> +                         ${inputs}
>>>> +
>>>> +EXCLUDE                = @TOP_SRCDIR@/include/libcamera/base/class.h \
>>>> +                         @TOP_SRCDIR@/include/libcamera/base/object.h \
>>>> +                         @TOP_SRCDIR@/include/libcamera/base/span.h \
>>>> +                         @TOP_SRCDIR@/src/libcamera/base/class.cpp \
>>>> +                         @TOP_SRCDIR@/src/libcamera/base/object.cpp
>>>> +
>>>> +PREDEFINED            += __DOXYGEN_PUBLIC__
>>>> diff --git a/Documentation/gen-doxyfile.py b/Documentation/gen-doxyfile.py
>>>> new file mode 100755
>>>> index 000000000000..c2a4184dd195
>>>> --- /dev/null
>>>> +++ b/Documentation/gen-doxyfile.py
>>>> @@ -0,0 +1,46 @@
>>>> +#!/usr/bin/env python3
>>>> +# SPDX-License-Identifier: GPL-2.0-or-later
>>>> +# Copyright (C) 2024, Google Inc.
>>> Does this mean it's drawn from elsewhere? Or is it a copy-paste error?
>>>
>>>> +#
>>>> +# Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>> +#
>>>> +# Generate Doxyfile from a template
>>>> +
>>>> +import argparse
>>>> +import os
>>>> +import string
>>>> +import sys
>>>> +
>>>> +
>>>> +def fill_template(template, data):
>>>> +
>>>> +    template = open(template, 'rb').read()
>>>> +    template = template.decode('utf-8')
>>>> +    template = string.Template(template)
>>> I'd add a line break here.
> Done.
>
>>>> +    return template.substitute(data)
>>>> +
>>>> +
>>>> +def main(argv):
>>>> +
>>>> +    # Parse command line arguments
>>> The comment can probably be skipped.
> I'm fine with that.
>
>
>>>> +    parser = argparse.ArgumentParser()
>>>> +    parser.add_argument('-o', dest='output', metavar='file',
>>>> +                        type=argparse.FileType('w', encoding='utf-8'),
>>>> +                        default=sys.stdout,
>>>> +                        help='Output file name (default: standard output)')
>>>> +    parser.add_argument('template', metavar='doxyfile.tmpl', type=str,
>>>> +                        help='Doxyfile template')
>>>> +    parser.add_argument('inputs', type=str, nargs='*',
>>>> +                        help='Input files')
>>>> +
>>>> +    args = parser.parse_args(argv[1:])
>>>> +
>>>> +    inputs = [f'"{os.path.realpath(input)}"' for input in args.inputs]
>>>> +    data = fill_template(args.template, {'inputs': (' \\\n' + ' ' * 25).join(inputs)})
>>>> +    args.output.write(data)
>>>> +
>>>> +    return 0
>>>> +
>>>> +
>>>> +if __name__ == '__main__':
>>>> +    sys.exit(main(sys.argv))
>>>> diff --git a/Documentation/meson.build b/Documentation/meson.build
>>>> index 1d84ed815b50..bf28233a2ce8 100644
>>>> --- a/Documentation/meson.build
>>>> +++ b/Documentation/meson.build
>>>> @@ -24,9 +24,9 @@ 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)
>>>> +    doxyfile_common = configure_file(input : 'Doxyfile-common.in',
>>>> +                                     output : 'Doxyfile-common',
>>>> +                                     configuration : cdata)
>>>>    
>>>>        doxygen_public_input = [
>>>>            libcamera_base_public_headers,
>>>> @@ -50,17 +50,75 @@ if doxygen.found() and dot.found()
>>>>            doxygen_internal_input += [ipu3_ipa_sources]
>>>>        endif
>>>>    
>>>> -    custom_target('doxygen',
>>>> +    # 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.
>>> This comment needs correcting now as it references the older method.
> I'll fix it.
>
>>>> +
>>>> +    # This is the "public" run of doxygen generating an abridged version of the
>>>> +    # API's documentation.
>>>> +
>>>> +    doxyfile_tmpl = configure_file(input : 'Doxyfile-public.in',
>>>> +                                   output : 'Doxyfile-public.tmpl',
>>>> +                                   configuration : cdata)
>>>> +
>>>> +    doxyfile = custom_target('doxyfile-public',
>>>> +                             input : [
>>>> +                                 doxygen_public_input,
>>>> +                             ],
>>>> +                             output : 'Doxyfile-public',
>>>> +                             command : [
>>>> +                                 'gen-doxyfile.py',
>>>> +                                 '-o', '@OUTPUT@',
>>>> +                                 doxyfile_tmpl,
>>>> +                                 '@INPUT@',
>>>> +                             ])
>>>> +
>>>> +    custom_target('doxygen-public',
>>>>                      input : [
>>>>                          doxyfile,
>>>> -                      doxygen_public_input,
>>>> -                      doxygen_internal_input,
>>>> +                      doxyfile_common,
>> At least in my mail client - the indentation looks odd here and might
>> want checking.
> It looks fine in the source code as far as I can tell. Can you fix your
> mail client ? :-)
>
>>>>                      ],
>>>>                      output : 'api-html',
>>>>                      command : [doxygen, doxyfile],
>>>>                      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.
>>>> +
>>>> +    doxyfile_tmpl = configure_file(input : 'Doxyfile-internal.in',
>>>> +                                   output : 'Doxyfile-internal.tmpl',
>>>> +                                   configuration : cdata)
>>>> +
>>>> +    doxyfile = custom_target('doxyfile-internal',
>>>> +                             input : [
>>>> +                                 doxygen_public_input,
>>>> +                                 doxygen_internal_input,
>>>> +                             ],
>>>> +                             output : 'Doxyfile-internal',
>>>> +                             command : [
>>>> +                                 'gen-doxyfile.py',
>>>> +                                 '-o', '@OUTPUT@',
>>>> +                                 doxyfile_tmpl,
>>>> +                                 '@INPUT@',
>>>> +                             ])
>>> I think we can skip the script for the internal run -there's no
>>> templated values to be replaced and the generated Doxyfile-internal
>>> is identical to the Doxyfile-internal.tmpl.
>> I wonder if that's "future proofing" but I don't mind either way.
> The goal was to keep the code flow identical, and I considered
> future-proofing too. I don't mind dropping it though.

Fair enough; leave it in then (unless you've already dropped it...I also don't mind either way so 
whatever state it's currently in, is fine).


Is an R-b from me appropriate here given I'm down as the author? If so:


Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>

>> I have nothing much to add except a potential whitespace comment on top
>> of Dan's comments, so with those handled:
>>
>> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
>>
>>>> +
>>>> +    custom_target('doxygen-internal',
>>>> +                  input : [
>>>> +                      doxyfile,
>>>> +                      doxyfile_common,
>>>> +                  ],
>>>> +                  output : 'internal-api-html',
>>>> +                  command : [doxygen, doxyfile],
>>>> +                  install : true,
>>>> +                  install_dir : doc_install_dir,
>>>> +                  install_tag : 'doc-internal')
>>>>    endif
>>>>    
>>>>    #

On Wed, Aug 07, 2024 at 02:45:03PM +0100, Daniel Scally wrote:
> Hi Laurent
> 
> On 07/08/2024 14:31, Laurent Pinchart wrote:
> > On Wed, Aug 07, 2024 at 12:45:31PM +0100, Kieran Bingham wrote:
> >> Quoting Dan Scally (2024-08-07 09:30:22)
> >>> On 05/08/2024 15:36, Laurent Pinchart wrote:
> >>>> From: Daniel Scally <dan.scally@ideasonboard.com>
> >>>>
> >>>> 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.
> >>>>
> >>>> The first run of doxygen is for the public API. We pass a specific list
> >>>> of source files to parse, which is built from the arrays of public
> >>>> headers and sources in meson build files. This ensures that we only
> >>>> generate the documentation for code from those files.
> >>>>
> >>>> A custom Python script is needed to add the list of input files to
> >>>> Doxyfile, as several of the objects included in the header and source
> >>>> array are custom_tgt objects, which can't be handled as strings to
> >>>> populate a variable in the configuration data.
> >>>>
> >>>> The headers defining the Extensible and Object classes (class.h and
> >>>> object.h respectively), as well as the corresponding source files, are
> >>>> excluded from the public API documentation despite being referenced in
> >>>> the meson public headers and sources arrays. This is due to the fact
> >>>> that public API classes inherit from Extensible and Object, making the
> >>>> Extensible and Object classes part of the public ABI. Those two base
> >>>> classes are however implementation details and must not be accessed
> >>>> directly by application code.
> >>>>
> >>>> The second run of doxygen is for the internal API. This contains
> >>>> documentation for all of the library's objects as it currently does.
> >>>> This set will now be output into build/Documentation/internal-api-html.
> >> Sounds good.
> >>
> >>>> Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
> >>>> Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>> ---
> >>> The python script to generate the list of input files is definitely better than scattered foreach
> >>> loops collecting a list in a hacky way - good improvement.
> >>>
> >>>>    .../{Doxyfile.in => Doxyfile-common.in}       | 24 -------
> >>>>    Documentation/Doxyfile-internal.in            | 30 ++++++++
> >>>>    Documentation/Doxyfile-public.in              | 20 ++++++
> >>>>    Documentation/gen-doxyfile.py                 | 46 ++++++++++++
> >>>>    Documentation/meson.build                     | 70 +++++++++++++++++--
> >>>>    5 files changed, 160 insertions(+), 30 deletions(-)
> >>>>    rename Documentation/{Doxyfile.in => Doxyfile-common.in} (62%)
> >>>>    create mode 100644 Documentation/Doxyfile-internal.in
> >>>>    create mode 100644 Documentation/Doxyfile-public.in
> >>>>    create mode 100755 Documentation/gen-doxyfile.py
> >>>>
> >>>> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile-common.in
> >>>> similarity index 62%
> >>>> rename from Documentation/Doxyfile.in
> >>>> rename to Documentation/Doxyfile-common.in
> >>>> index 6e5a3830ec38..a70aee430e77 100644
> >>>> --- a/Documentation/Doxyfile.in
> >>>> +++ b/Documentation/Doxyfile-common.in
> >>>> @@ -17,20 +17,11 @@ EXTENSION_MAPPING      = h=C++
> >>>>    
> >>>>    TOC_INCLUDE_HEADINGS   = 0
> >>>>    
> >>>> -INTERNAL_DOCS          = YES
> >>>>    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 \
> >>>> @@ -38,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 \
> >>>> @@ -73,8 +51,6 @@ EXCLUDE_SYMBOLS        = libcamera::BoundMethodArgs \
> >>>>    
> >>>>    EXCLUDE_SYMLINKS       = YES
> >>>>    
> >>>> -HTML_OUTPUT            = api-html
> >>>> -
> >>>>    GENERATE_LATEX         = NO
> >>>>    
> >>>>    MACRO_EXPANSION        = YES
> >>>> diff --git a/Documentation/Doxyfile-internal.in b/Documentation/Doxyfile-internal.in
> >>>> new file mode 100644
> >>>> index 000000000000..5e0310091416
> >>>> --- /dev/null
> >>>> +++ b/Documentation/Doxyfile-internal.in
> >>>> @@ -0,0 +1,30 @@
> >>>> +# SPDX-License-Identifier: CC-BY-SA-4.0
> >>>> +
> >>>> +@INCLUDE_PATH          = @TOP_BUILDDIR@/Documentation
> >>>> +@INCLUDE               = Doxyfile-common
> >>>> +
> >>>> +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 000000000000..36bb57584a07
> >>>> --- /dev/null
> >>>> +++ b/Documentation/Doxyfile-public.in
> >>>> @@ -0,0 +1,20 @@
> >>>> +# SPDX-License-Identifier: CC-BY-SA-4.0
> >>>> +
> >>>> +@INCLUDE_PATH          = @TOP_BUILDDIR@/Documentation
> >>>> +@INCLUDE               = Doxyfile-common
> >>>> +
> >>>> +HIDE_UNDOC_CLASSES     = YES
> >>>> +HIDE_UNDOC_MEMBERS     = YES
> >>>> +HTML_OUTPUT            = api-html
> >>>> +INTERNAL_DOCS          = NO
> >>>> +
> >>>> +INPUT                  = "@TOP_SRCDIR@/Documentation" \
> >>>> +                         ${inputs}
> >>>> +
> >>>> +EXCLUDE                = @TOP_SRCDIR@/include/libcamera/base/class.h \
> >>>> +                         @TOP_SRCDIR@/include/libcamera/base/object.h \
> >>>> +                         @TOP_SRCDIR@/include/libcamera/base/span.h \
> >>>> +                         @TOP_SRCDIR@/src/libcamera/base/class.cpp \
> >>>> +                         @TOP_SRCDIR@/src/libcamera/base/object.cpp
> >>>> +
> >>>> +PREDEFINED            += __DOXYGEN_PUBLIC__
> >>>> diff --git a/Documentation/gen-doxyfile.py b/Documentation/gen-doxyfile.py
> >>>> new file mode 100755
> >>>> index 000000000000..c2a4184dd195
> >>>> --- /dev/null
> >>>> +++ b/Documentation/gen-doxyfile.py
> >>>> @@ -0,0 +1,46 @@
> >>>> +#!/usr/bin/env python3
> >>>> +# SPDX-License-Identifier: GPL-2.0-or-later
> >>>> +# Copyright (C) 2024, Google Inc.
> >>> Does this mean it's drawn from elsewhere? Or is it a copy-paste error?
> >>>
> >>>> +#
> >>>> +# Author: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>> +#
> >>>> +# Generate Doxyfile from a template
> >>>> +
> >>>> +import argparse
> >>>> +import os
> >>>> +import string
> >>>> +import sys
> >>>> +
> >>>> +
> >>>> +def fill_template(template, data):
> >>>> +
> >>>> +    template = open(template, 'rb').read()
> >>>> +    template = template.decode('utf-8')
> >>>> +    template = string.Template(template)
> >>> I'd add a line break here.
> > Done.
> >
> >>>> +    return template.substitute(data)
> >>>> +
> >>>> +
> >>>> +def main(argv):
> >>>> +
> >>>> +    # Parse command line arguments
> >>> The comment can probably be skipped.
> > I'm fine with that.
> >
> >
> >>>> +    parser = argparse.ArgumentParser()
> >>>> +    parser.add_argument('-o', dest='output', metavar='file',
> >>>> +                        type=argparse.FileType('w', encoding='utf-8'),
> >>>> +                        default=sys.stdout,
> >>>> +                        help='Output file name (default: standard output)')
> >>>> +    parser.add_argument('template', metavar='doxyfile.tmpl', type=str,
> >>>> +                        help='Doxyfile template')
> >>>> +    parser.add_argument('inputs', type=str, nargs='*',
> >>>> +                        help='Input files')
> >>>> +
> >>>> +    args = parser.parse_args(argv[1:])
> >>>> +
> >>>> +    inputs = [f'"{os.path.realpath(input)}"' for input in args.inputs]
> >>>> +    data = fill_template(args.template, {'inputs': (' \\\n' + ' ' * 25).join(inputs)})
> >>>> +    args.output.write(data)
> >>>> +
> >>>> +    return 0
> >>>> +
> >>>> +
> >>>> +if __name__ == '__main__':
> >>>> +    sys.exit(main(sys.argv))
> >>>> diff --git a/Documentation/meson.build b/Documentation/meson.build
> >>>> index 1d84ed815b50..bf28233a2ce8 100644
> >>>> --- a/Documentation/meson.build
> >>>> +++ b/Documentation/meson.build
> >>>> @@ -24,9 +24,9 @@ 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)
> >>>> +    doxyfile_common = configure_file(input : 'Doxyfile-common.in',
> >>>> +                                     output : 'Doxyfile-common',
> >>>> +                                     configuration : cdata)
> >>>>    
> >>>>        doxygen_public_input = [
> >>>>            libcamera_base_public_headers,
> >>>> @@ -50,17 +50,75 @@ if doxygen.found() and dot.found()
> >>>>            doxygen_internal_input += [ipu3_ipa_sources]
> >>>>        endif
> >>>>    
> >>>> -    custom_target('doxygen',
> >>>> +    # 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.
> >>> This comment needs correcting now as it references the older method.
> > I'll fix it.
> >
> >>>> +
> >>>> +    # This is the "public" run of doxygen generating an abridged version of the
> >>>> +    # API's documentation.
> >>>> +
> >>>> +    doxyfile_tmpl = configure_file(input : 'Doxyfile-public.in',
> >>>> +                                   output : 'Doxyfile-public.tmpl',
> >>>> +                                   configuration : cdata)
> >>>> +
> >>>> +    doxyfile = custom_target('doxyfile-public',
> >>>> +                             input : [
> >>>> +                                 doxygen_public_input,
> >>>> +                             ],
> >>>> +                             output : 'Doxyfile-public',
> >>>> +                             command : [
> >>>> +                                 'gen-doxyfile.py',
> >>>> +                                 '-o', '@OUTPUT@',
> >>>> +                                 doxyfile_tmpl,
> >>>> +                                 '@INPUT@',
> >>>> +                             ])
> >>>> +
> >>>> +    custom_target('doxygen-public',
> >>>>                      input : [
> >>>>                          doxyfile,
> >>>> -                      doxygen_public_input,
> >>>> -                      doxygen_internal_input,
> >>>> +                      doxyfile_common,
> >> At least in my mail client - the indentation looks odd here and might
> >> want checking.
> > It looks fine in the source code as far as I can tell. Can you fix your
> > mail client ? :-)
> >
> >>>>                      ],
> >>>>                      output : 'api-html',
> >>>>                      command : [doxygen, doxyfile],
> >>>>                      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.
> >>>> +
> >>>> +    doxyfile_tmpl = configure_file(input : 'Doxyfile-internal.in',
> >>>> +                                   output : 'Doxyfile-internal.tmpl',
> >>>> +                                   configuration : cdata)
> >>>> +
> >>>> +    doxyfile = custom_target('doxyfile-internal',
> >>>> +                             input : [
> >>>> +                                 doxygen_public_input,
> >>>> +                                 doxygen_internal_input,
> >>>> +                             ],
> >>>> +                             output : 'Doxyfile-internal',
> >>>> +                             command : [
> >>>> +                                 'gen-doxyfile.py',
> >>>> +                                 '-o', '@OUTPUT@',
> >>>> +                                 doxyfile_tmpl,
> >>>> +                                 '@INPUT@',
> >>>> +                             ])
> >>> I think we can skip the script for the internal run -there's no
> >>> templated values to be replaced and the generated Doxyfile-internal
> >>> is identical to the Doxyfile-internal.tmpl.
> >> I wonder if that's "future proofing" but I don't mind either way.
> >
> > The goal was to keep the code flow identical, and I considered
> > future-proofing too. I don't mind dropping it though.
> 
> Fair enough; leave it in then (unless you've already dropped it...I also don't mind either way so 
> whatever state it's currently in, is fine).

Done already :-)

> Is an R-b from me appropriate here given I'm down as the author? If so:

I think it is, to record you've reviewed the changes I've made. This is
why I try to sort the trailers in chronological order, and add the
reviewed-by tags after my SoB line.

> Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
> 
> >> I have nothing much to add except a potential whitespace comment on top
> >> of Dan's comments, so with those handled:
> >>
> >> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> >>
> >>>> +
> >>>> +    custom_target('doxygen-internal',
> >>>> +                  input : [
> >>>> +                      doxyfile,
> >>>> +                      doxyfile_common,
> >>>> +                  ],
> >>>> +                  output : 'internal-api-html',
> >>>> +                  command : [doxygen, doxyfile],
> >>>> +                  install : true,
> >>>> +                  install_dir : doc_install_dir,
> >>>> +                  install_tag : 'doc-internal')
> >>>>    endif
> >>>>    
> >>>>    #