[libcamera-devel] [RFC PATCH] ipa: Documentation: Enable documentation of mojom files

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Thu, May 13, 2021 at 04:53:46PM +0900, Paul Elder wrote:
> Enable generation of documentation for mojom files. As doxygen cannot
> parse mojom files directly, put the comments in a cpp file instead, and
> plumb meson and doxygen accordingly.

A really open question: would it make sense to write a small python
script that would extract the comments from the .mojom files and
generate .cpp files, to keep the documentation close to the definition
of the data types ? We don't do so for C++ data types, so maybe it's
fine to move the documentation to .cpp files manually. Conceptually we
could generate code in different languages, in which case it would make
more sense to have the documentation in the .mojom file, but I doubt
we'll ever do so.

> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> Obviously we'll have to remove the documentation from core.mojom, and
> add documentation accordingly to the other mojom files. This is still
> and RFC just to see if this is the direction we want to go.
> ---
>  Documentation/Doxyfile.in                    |   4 +-
>  Documentation/meson.build                    |   1 +
>  include/libcamera/ipa/core_ipa_interface.cpp | 190 +++++++++++++++++++
>  include/libcamera/ipa/meson.build            |   4 +
>  4 files changed, 197 insertions(+), 2 deletions(-)
>  create mode 100644 include/libcamera/ipa/core_ipa_interface.cpp
> 
> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> index af006724..213adb9b 100644
> --- a/Documentation/Doxyfile.in
> +++ b/Documentation/Doxyfile.in
> @@ -844,7 +844,6 @@ EXCLUDE                = @TOP_SRCDIR@/include/libcamera/span.h \
>  			 @TOP_SRCDIR@/src/libcamera/pipeline/ \
>  			 @TOP_SRCDIR@/src/libcamera/tracepoints.cpp \
>  			 @TOP_BUILDDIR@/include/libcamera/internal/tracepoints.h \
> -			 @TOP_BUILDDIR@/include/libcamera/ipa/ \
>  			 @TOP_BUILDDIR@/src/libcamera/proxy/
>  
>  # The EXCLUDE_SYMLINKS tag can be used to select whether or not files or
> @@ -861,7 +860,8 @@ EXCLUDE_SYMLINKS       = NO
>  # Note that the wildcards are matched against the file with absolute path, so to
>  # exclude all test directories for example use the pattern */test/*
>  
> -EXCLUDE_PATTERNS       =
> +EXCLUDE_PATTERNS       = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \
> +			 @TOP_BUILDDIR@/include/libcamera/ipa/*_proxy.h
>  
>  # The EXCLUDE_SYMBOLS tag can be used to specify one or more symbol names
>  # (namespaces, classes, functions, etc.) that should be excluded from the
> diff --git a/Documentation/meson.build b/Documentation/meson.build
> index c8521574..86f1134b 100644
> --- a/Documentation/meson.build
> +++ b/Documentation/meson.build
> @@ -23,6 +23,7 @@ if doxygen.found() and dot.found()
>                    input : [
>                        doxyfile,
>                        libcamera_internal_headers,
> +                      libcamera_ipa_docs,
>                        libcamera_ipa_headers,
>                        libcamera_public_headers,
>                        libcamera_sources,
> diff --git a/include/libcamera/ipa/core_ipa_interface.cpp b/include/libcamera/ipa/core_ipa_interface.cpp
> new file mode 100644
> index 00000000..f9820b92
> --- /dev/null
> +++ b/include/libcamera/ipa/core_ipa_interface.cpp
> @@ -0,0 +1,190 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +
> +namespace libcamera {
> +
> +/**
> + * \struct CameraSensorInfo
> + * \brief Report the image sensor characteristics
> + *
> + * The structure reports image sensor characteristics used by IPA modules to
> + * tune their algorithms based on the image sensor model currently in use and
> + * its configuration.
> + *
> + * The reported information describes the sensor's intrinsics characteristics,
> + * such as its pixel array size and the sensor model name, as well as
> + * information relative to the currently configured mode, such as the produced
> + * image size and the bit depth of the requested image format.
> + *
> + * Instances of this structure are meant to be assembled by the CameraSensor
> + * class by inspecting the sensor static properties as well as information
> + * relative to the current configuration.
> + */
> +
> +/**
> + * \var CameraSensorInfo::model
> + * \brief The image sensor model name
> + *
> + * The sensor model name is a free-formed string that uniquely identifies the
> + * sensor model.
> + */
> +
> +/**
> + * \var CameraSensorInfo::bitsPerPixel
> + * \brief The number of bits per pixel of the image format produced by the
> + * image sensor
> + */
> +
> +/**
> + * \var CameraSensorInfo::activeAreaSize
> + * \brief The size of the pixel array active area of the sensor
> + */
> +
> +/**
> + * \var CameraSensorInfo::analogCrop
> + * \brief The portion of the pixel array active area which is read-out and
> + * processed
> + *
> + * The analog crop rectangle top-left corner is defined as the displacement
> + * from the top-left corner of the pixel array active area. The rectangle
> + * horizontal and vertical sizes define the portion of the pixel array which
> + * is read-out and provided to the sensor's internal processing pipeline, before
> + * any pixel sub-sampling method, such as pixel binning, skipping and averaging
> + * take place.
> + */
> +
> +/**
> + * \var CameraSensorInfo::outputSize
> + * \brief The size of the images produced by the camera sensor
> + *
> + * The output image size defines the horizontal and vertical sizes of the images
> + * produced by the image sensor. The output image size is defined as the end
> + * result of the sensor's internal image processing pipeline stages, applied on
> + * the pixel array portion defined by the analog crop rectangle. Each image
> + * processing stage that performs pixel sub-sampling techniques, such as pixel
> + * binning or skipping, or perform any additional digital scaling concur in the
> + * definition of the output image size.
> + */
> +
> +/**
> + * \var CameraSensorInfo::pixelRate
> + * \brief The number of pixels produced in a second
> + *
> + * To obtain the read-out time in seconds of a full line:
> + *
> + * \verbatim
> +	lineDuration(s) = lineLength(pixels) / pixelRate(pixels per second)
> +   \endverbatim
> + */
> +
> +/**
> + * \var CameraSensorInfo::lineLength
> + * \brief Total line length in pixels
> + *
> + * The total line length in pixel clock periods, including blanking.
> + */
> +
> +/**
> + * \var CameraSensorInfo::minFrameLength
> + * \brief The minimum allowable frame length in units of lines
> + *
> + * The sensor frame length comprises of active output lines and blanking lines
> + * in a frame. The minimum frame length value dictates the minimum allowable
> + * frame duration of the sensor mode.
> + *
> + * To obtain the minimum frame duration:
> + *
> + * \verbatim
> +	frameDuration(s) = minFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)
> +   \endverbatim
> + */
> +
> +/**
> + * \var CameraSensorInfo::maxFrameLength
> + * \brief The maximum allowable frame length in units of lines
> + *
> + * The sensor frame length comprises of active output lines and blanking lines
> + * in a frame. The maximum frame length value dictates the maximum allowable
> + * frame duration of the sensor mode.
> + *
> + * To obtain the maximum frame duration:
> + *
> + * \verbatim
> +	frameDuration(s) = maxFrameLength(lines) * lineLength(pixels) / pixelRate(pixels per second)
> +   \endverbatim
> + */
> +
> +/**
> + * \struct IPABuffer
> + * \brief Buffer information for the IPA interface
> + *
> + * The IPABuffer structure associates buffer memory with a unique ID. It is
> + * used to map buffers to the IPA with IPAInterface::mapBuffers(), after which
> + * buffers will be identified by their ID in the IPA interface.
> + */
> +
> +/**
> + * \var IPABuffer::id
> + * \brief The buffer unique ID
> + *
> + * Buffers mapped to the IPA are identified by numerical unique IDs. The IDs
> + * are chosen by the pipeline handler to fulfil the following constraints:
> + *
> + * - IDs shall be positive integers different than zero
> + * - IDs shall be unique among all mapped buffers
> + *
> + * When buffers are unmapped with IPAInterface::unmapBuffers() their IDs are
> + * freed and may be reused for new buffer mappings.
> + */
> +
> +/**
> + * \var IPABuffer::planes
> + * \brief The buffer planes description
> + *
> + * Stores the dmabuf handle and length for each plane of the buffer.
> + */
> +
> +/**
> + * \struct IPASettings
> + * \brief IPA interface initialization settings
> + *
> + * The IPASettings structure stores data passed to the IPAInterface::init()
> + * function. The data contains settings that don't depend on a particular camera
> + * or pipeline configuration and are valid for the whole life time of the IPA
> + * interface.
> + */
> +
> +/**
> + * \var IPASettings::configurationFile
> + * \brief The name of the IPA configuration file
> + *
> + * This field may be an empty string if the IPA doesn't require a configuration
> + * file.
> + */
> +
> + /**
> + * \var IPASettings::sensorModel
> + * \brief The sensor model name
> + *
> + * Provides the sensor model name to the IPA.
> + */
> +
> +/**
> + * \struct IPAStream
> + * \brief Stream configuration for the IPA interface
> + *
> + * The IPAStream structure stores stream configuration parameters needed by the
> + * IPAInterface::configure() method. It mirrors the StreamConfiguration class
> + * that is not suitable for this purpose due to not being serializable.
> + */
> +
> +/**
> + * \var IPAStream::pixelFormat
> + * \brief The stream pixel format
> + */
> +
> +/**
> + * \var IPAStream::size
> + * \brief The stream size in pixels
> + */
> +
> +} /* namespace libcamera */
> diff --git a/include/libcamera/ipa/meson.build b/include/libcamera/ipa/meson.build
> index 40c4e737..69bc855e 100644
> --- a/include/libcamera/ipa/meson.build
> +++ b/include/libcamera/ipa/meson.build
> @@ -6,6 +6,10 @@ libcamera_ipa_headers = files([
>      'ipa_module_info.h',
>  ])
>  
> +libcamera_ipa_docs = files([
> +    'core_ipa_interface.cpp',
> +])
> +
>  install_headers(libcamera_ipa_headers,
>                  subdir: libcamera_include_dir / 'ipa')
>  

-- 
Regards,

Laurent Pinchart