[libcamera-devel] [PATCH v2 1/7] ipa: Move core IPA interface documentation to a .cpp file
Kieran Bingham
kieran.bingham at ideasonboard.com
Wed May 19 13:11:53 CEST 2021
Hi Umang,
On 19/05/2021 11:19, Umang Jain wrote:
> Moving the core.mojom documentation to its corresponding .cpp file
> (core_ipa_interface.cpp). This will allow Doxygen to generate the
> documentation for IPABuffer, IPASettings and IPAStream structures.
> Since the .mojom files are placed in include/ directory, the .cpp file
> will live in $sourcedir/src/libcamera/ipa/ - which can also contain
> documentation for other mojom generated IPA interfaces in subsequent
> commit.
>
> Also, fix the Doxygen warning for the above IPA structs regarding their
> constructors not being documented.
>
> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> ---
> Documentation/Doxyfile.in | 8 +-
> Documentation/meson.build | 1 +
> include/libcamera/ipa/core.mojom | 72 ----------------
> src/libcamera/ipa/core_ipa_interface.cpp | 105 +++++++++++++++++++++++
> src/libcamera/ipa/meson.build | 5 ++
> src/libcamera/meson.build | 1 +
> 6 files changed, 118 insertions(+), 74 deletions(-)
> create mode 100644 src/libcamera/ipa/core_ipa_interface.cpp
> create mode 100644 src/libcamera/ipa/meson.build
>
> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in
> index af006724..f4d578fa 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,12 @@ 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 \
> + @TOP_BUILDDIR@/include/libcamera/ipa/raspberrypi_*.h \
> + @TOP_BUILDDIR@/include/libcamera/ipa/vimc_*.h \
> + @TOP_BUILDDIR@/include/libcamera/ipa/rkisp1_*.h \
> + @TOP_BUILDDIR@/include/libcamera/ipa/ipu3_*.h \
I was going to suggest that perhaps this should be
@TOP_BUILDDIR@/include/libcamera/ipa/*_interface.h ?
But that would exclude the core_ipa_interface.h too, which I presume we
need to include for Doxygen to map the documentation to.
I wonder if there's a way to include only the files we need under
include/libcamera/ipa if that list is smaller, rather than trying to
exclude lots of combinations which would need to be updated with every
new platform supported.
>
> # 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..9ecf4dfc 100644
> --- a/Documentation/meson.build
> +++ b/Documentation/meson.build
> @@ -24,6 +24,7 @@ if doxygen.found() and dot.found()
> doxyfile,
> libcamera_internal_headers,
> libcamera_ipa_headers,
> + libcamera_ipa_interfaces,
> libcamera_public_headers,
> libcamera_sources,
> libipa_headers,
> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom
> index 6caaa63e..e49815d8 100644
> --- a/include/libcamera/ipa/core.mojom
> +++ b/include/libcamera/ipa/core.mojom
> @@ -94,88 +94,16 @@ module libcamera;
> uint32 maxFrameLength;
> };
>
> -/**
> - * \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 IPABuffer {
> uint32 id;
> [hasFd] array<FrameBuffer.Plane> planes;
> };
>
> -/**
> - * \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 IPASettings {
> string configurationFile;
> string sensorModel;
> };
>
> -/**
> - * \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
> - */
> struct IPAStream {
> uint32 pixelFormat;
> Size size;
> diff --git a/src/libcamera/ipa/core_ipa_interface.cpp b/src/libcamera/ipa/core_ipa_interface.cpp
> new file mode 100644
> index 00000000..fe1ecce6
> --- /dev/null
> +++ b/src/libcamera/ipa/core_ipa_interface.cpp
> @@ -0,0 +1,105 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2021, Google Inc.
> + *
> + * core_ipa_interface.cpp - Docs file for core.mojom generated header
> + */
> +
> +namespace libcamera {
> +
> +/**
> + * \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.
> + */
> +
> +/**
> + * \fn IPABuffer::IPABuffer(uint32_t id, const std::vector<FrameBuffer::Plane> &planes)
> + * \param[in] id
> + * \param[in] planes
> + * \sa id and planes
> + */
> +
> +/**
> + * \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.
> + */
> +
> +/**
> + * \fn IPASettings::IPASettings(const std::string &configurationFile, const std::string &sensorModel)
> + * \param[in] configurationFile
> + * \param[in] sensorModel
> + * \sa configurationFile and sensorModel
> + */
> +
> +/**
> + * \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.
> + */
> +
> +/**
> + * \fn IPAStream::IPAStream(uint32_t pixelFormat, const Size &size)
> + * \param[in] pixelFormat
> + * \param[in] size
> + * \sa pixelFormat and size
> + */
> +
> +/**
> + * \var IPAStream::pixelFormat
> + * \brief The stream pixel format
> + */
> +
> +/**
> + * \var IPAStream::size
> + * \brief The stream size in pixels
> + */
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/ipa/meson.build b/src/libcamera/ipa/meson.build
> new file mode 100644
> index 00000000..0a16d197
> --- /dev/null
> +++ b/src/libcamera/ipa/meson.build
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: LGPL-2.1-or-later
> +
> +libcamera_ipa_interfaces = files([
> + 'core_ipa_interface.cpp',
> +])
I'm sure someone will disagree with me, but I almost feel like this .cpp
file should get run through a compiler, and therefore just added to the
libcamera sources as normal (which I think would then already get picked
up by doxygen).
That way the syntax of the file is maintained too, given that it is a
C++ file.
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index 7bc59b84..61b5fe21 100644
> --- a/src/libcamera/meson.build
> +++ b/src/libcamera/meson.build
> @@ -67,6 +67,7 @@ includes = [
> libcamera_includes,
> ]
>
> +subdir('ipa')
> subdir('pipeline')
> subdir('proxy')
>
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list