[libcamera-devel] [PATCH 2/2] libcamera: Documentation: Split public/private documentation
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Dec 26 23:59:59 CET 2023
On Tue, Dec 26, 2023 at 05:24:37PM +0200, Laurent Pinchart via libcamera-devel wrote:
> On Tue, Dec 26, 2023 at 11:41:35AM +0100, Jacopo Mondi via libcamera-devel wrote:
> > Hi Dan
> > thanks for this big chunk of work
> >
> > On Wed, Dec 20, 2023 at 04:31:44PM +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
>
> More than likely, it's outright forbidden :-) You can write
>
> "who will only use the public API objects."
>
> > > reduce the complexity of the documentation, split it into two runs of
> > > doxygen. In the first run leverage doxygen's \internal and \if flags
> > > to hide library objects which are not intended to be exposed through
> > > the public headers.
>
> "to hide internal library objects" is enough.
>
> > >
> > > 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>
> > > ---
> > > This patch touches a lot of files, but almost exclusively adds doxygen's
> > > \internal flag to docu-comments along with the necessary changes to Doxyfile.in
> > > and meson.build to implement the hiding of those objects in the auto-generated
> > > documentation.
> > >
> > > The deviation from that rule is in object.cpp, where reference is made to
> > > the "internal" Thread class documentation. Because the Object class is retained
> > > in the public documents we can't remove the whole thing with \internal which
> > > means resorting to \if .. \endif. It's specifically a reference to the Stopping
> > > Threads section, and an alternative means of resolving this might be to move
> > > that whole section to the Thread Support _page_ instead of being part of the
> > > Thread class.
>
> I think you can use
>
> * \internal
> * ...
> * \endinternal
>
> instead of '\if internal' in object.cpp.
>
> > > Documentation/Doxyfile.in | 9 +++-
> > > Documentation/meson.build | 47 +++++++++++++++++--
> > > include/libcamera/ipa/core.mojom | 5 ++
> > > src/ipa/ipu3/algorithms/af.cpp | 2 +
> > > src/ipa/ipu3/algorithms/agc.cpp | 2 +
> > > src/ipa/ipu3/algorithms/awb.cpp | 4 ++
> > > src/ipa/ipu3/algorithms/blc.cpp | 2 +
> > > src/ipa/ipu3/algorithms/tone_mapping.cpp | 2 +
> > > src/ipa/ipu3/ipa_context.cpp | 5 ++
> > > src/ipa/ipu3/ipu3.cpp | 1 +
> > > src/ipa/libipa/algorithm.cpp | 3 ++
> > > src/ipa/libipa/camera_sensor_helper.cpp | 8 ++++
> > > src/ipa/libipa/fc_queue.cpp | 3 ++
> > > src/ipa/libipa/histogram.cpp | 2 +
> > > src/ipa/libipa/module.cpp | 2 +
> > > src/ipa/rkisp1/algorithms/agc.cpp | 2 +
> > > src/ipa/rkisp1/algorithms/awb.cpp | 2 +
> > > src/ipa/rkisp1/algorithms/blc.cpp | 2 +
> > > src/ipa/rkisp1/algorithms/cproc.cpp | 2 +
> > > src/ipa/rkisp1/algorithms/dpcc.cpp | 2 +
> > > src/ipa/rkisp1/algorithms/dpf.cpp | 2 +
> > > src/ipa/rkisp1/algorithms/filter.cpp | 2 +
> > > src/ipa/rkisp1/algorithms/gsl.cpp | 2 +
> > > src/ipa/rkisp1/algorithms/lsc.cpp | 2 +
> > > src/ipa/rkisp1/ipa_context.cpp | 13 +++++
> > > src/libcamera/base/backtrace.cpp | 2 +
> > > src/libcamera/base/event_dispatcher.cpp | 2 +
> > > src/libcamera/base/event_dispatcher_poll.cpp | 2 +
> > > src/libcamera/base/event_notifier.cpp | 3 ++
> > > src/libcamera/base/file.cpp | 4 ++
> > > src/libcamera/base/log.cpp | 7 +++
> > > src/libcamera/base/message.cpp | 4 ++
> > > src/libcamera/base/mutex.cpp | 4 ++
> > > src/libcamera/base/object.cpp | 13 +++--
> > > src/libcamera/base/semaphore.cpp | 2 +
> > > src/libcamera/base/thread.cpp | 5 ++
> > > src/libcamera/base/timer.cpp | 2 +
> > > src/libcamera/base/utils.cpp | 2 +
> > > src/libcamera/bayer_format.cpp | 4 ++
> > > src/libcamera/byte_stream_buffer.cpp | 2 +
> > > src/libcamera/camera.cpp | 1 +
> > > src/libcamera/camera_controls.cpp | 2 +
> > > src/libcamera/camera_lens.cpp | 2 +
> > > src/libcamera/camera_manager.cpp | 1 +
> > > src/libcamera/camera_sensor.cpp | 2 +
> > > src/libcamera/camera_sensor_properties.cpp | 2 +
> > > src/libcamera/control_serializer.cpp | 3 ++
> > > src/libcamera/control_validator.cpp | 2 +
> > > src/libcamera/converter.cpp | 4 ++
> > > .../converter/converter_v4l2_m2m.cpp | 2 +
> > > src/libcamera/delayed_controls.cpp | 3 ++
> > > src/libcamera/device_enumerator.cpp | 3 ++
> > > src/libcamera/framebuffer.cpp | 2 +
> > > src/libcamera/ipa_controls.cpp | 4 ++
> > > src/libcamera/ipa_data_serializer.cpp | 2 +
> > > src/libcamera/ipa_interface.cpp | 2 +
> > > src/libcamera/ipa_manager.cpp | 2 +
> > > src/libcamera/ipa_module.cpp | 4 ++
> > > src/libcamera/ipa_proxy.cpp | 3 ++
> > > src/libcamera/ipc_pipe.cpp | 4 ++
> > > src/libcamera/ipc_unixsocket.cpp | 3 ++
> > > src/libcamera/mapped_framebuffer.cpp | 4 ++
> > > src/libcamera/media_device.cpp | 2 +
> > > src/libcamera/media_object.cpp | 6 +++
> > > src/libcamera/pipeline/ipu3/imgu.cpp | 2 +
> > > .../pipeline/rpi/common/delayed_controls.cpp | 3 ++
> > > src/libcamera/pipeline_handler.cpp | 4 ++
> > > src/libcamera/process.cpp | 4 ++
> > > src/libcamera/pub_key.cpp | 2 +
> > > src/libcamera/request.cpp | 1 +
> > > src/libcamera/source_paths.cpp | 1 +
> > > src/libcamera/sysfs.cpp | 1 +
> > > src/libcamera/v4l2_device.cpp | 2 +
> > > src/libcamera/v4l2_pixelformat.cpp | 3 ++
> > > src/libcamera/v4l2_subdevice.cpp | 7 +++
> > > src/libcamera/v4l2_videodevice.cpp | 7 +++
> > > src/libcamera/yaml_parser.cpp | 5 ++
> > > 77 files changed, 285 insertions(+), 8 deletions(-)
[snip]
> > > diff --git a/src/libcamera/base/backtrace.cpp b/src/libcamera/base/backtrace.cpp
> > > index be30589d..8c0acdfc 100644
> > > --- a/src/libcamera/base/backtrace.cpp
> > > +++ b/src/libcamera/base/backtrace.cpp
> >
> > All components in src/libcamera/base except BoundMethod are internal.
>
> include/libcamera/base/meson.build contains
>
> libcamera_base_public_headers = files([
> 'bound_method.h',
> 'class.h',
> 'compiler.h',
> 'flags.h',
> 'object.h',
> 'shared_fd.h',
> 'signal.h',
> 'span.h',
> 'unique_fd.h',
> ])
>
> so there's more than just BoundMethod.
>
> > Should we exclude the directory ?
> >
> > Same for src/libcamera/pipeline. Have you considered the option of not
> > parsing the dir completely ?
>
> I think excluding some directories would make sense. I'll experiment.
For the record, I initially tried the set the list of input files
explicitly. Doxyfile contains an INPUT configuration parameter that we
currently set to a list of directories, and I thought I could pass it a
list of files instead. We have a partial list of files corresponding to
the public API, and it wasn't difficult to split libcamera_sources into
public and internal sources.
I hit a blocker because the list of files is stored in an array of File
objects. I couldn't find a way to convert that to the real path of the
files to pass to cdata.set() prior to meson 1.4.0 that introduced
file.full_path(). File objects get converted automatically to file paths
when passed to lots of functions, so I suppose it would be possible to
use a custom_target() to convert the array of Files to strings, but
that's becoming quite a bit of a hack.
The other option is to include/exclude directories, which we already do
actually. I'll see if that can be expanded.
> > Also, I see in the public documentation the Extensible::Private class
> > being documented. Is this intentional ?
>
> That class seems internal.
>
> > I'll skip the rest as I have enough questions already.
> >
> > > @@ -33,6 +33,7 @@
> > > #include <libcamera/base/utils.h>
> > >
> > > /**
> > > + * \internal
> > > * \file backtrace.h
> > > * \brief Generate call stack backtraces
> > > */
> > > @@ -135,6 +136,7 @@ std::string DwflParser::stackEntry(const void *ip)
> > > } /* namespace */
> > >
> > > /**
> > > + * \internal
> > > * \class Backtrace
> > > * \brief Representation of a call stack backtrace
> > > *
>
> [snip]
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list