| Message ID | 20210521132823.322076-2-umang.jain@ideasonboard.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Umang, Thank you for the patch. On Fri, May 21, 2021 at 06:58:18PM +0530, 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. I wonder if we should store those files in src/libcamera/pipeline/. We can think about this later. > Also, fix the Doxygen warning for the above IPA structs regarding their > constructors not being documented. > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com> > Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > --- > Documentation/Doxyfile.in | 8 +- > Documentation/meson.build | 1 + > include/libcamera/ipa/core.mojom | 72 ---------------- > include/libcamera/ipa/ipu3.mojom | 5 ++ > include/libcamera/ipa/raspberrypi.mojom | 5 ++ > include/libcamera/ipa/rkisp1.mojom | 5 ++ > include/libcamera/ipa/vimc.mojom | 5 ++ > src/libcamera/ipa/core_ipa_interface.cpp | 105 +++++++++++++++++++++++ > src/libcamera/ipa/meson.build | 5 ++ > src/libcamera/meson.build | 1 + > 10 files changed, 138 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 \ Alphabetically sorted please :-) > > # 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/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom > index a717b1e6..9e3cd885 100644 > --- a/include/libcamera/ipa/ipu3.mojom > +++ b/include/libcamera/ipa/ipu3.mojom > @@ -1,5 +1,10 @@ > /* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > + * \todo Document the interface as src/libcamera/ipa/ipu3_ipa_interface.cpp > + * and remove the EXCLUDE_PATTERNS entry in Doxygen.in for its generation. > + */ > + > module ipa.ipu3; > > import "include/libcamera/ipa/core.mojom"; > diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom > index 42321bee..770e3049 100644 > --- a/include/libcamera/ipa/raspberrypi.mojom > +++ b/include/libcamera/ipa/raspberrypi.mojom > @@ -1,5 +1,10 @@ > /* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > + * \todo Document the interface as src/libcamera/ipa/raspberrypi_ipa_interface.cpp > + * and remove the EXCLUDE_PATTERNS entry in Doxygen.in for its generation. > + */ > + > module ipa.RPi; > > import "include/libcamera/ipa/core.mojom"; > diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom > index cca871a0..da6646df 100644 > --- a/include/libcamera/ipa/rkisp1.mojom > +++ b/include/libcamera/ipa/rkisp1.mojom > @@ -1,5 +1,10 @@ > /* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > + * \todo Document the interface as src/libcamera/ipa/rkisp1_ipa_interface.cpp > + * and remove the EXCLUDE_PATTERNS entry in Doxygen.in for its generation. > + */ > + > module ipa.rkisp1; > > import "include/libcamera/ipa/core.mojom"; > diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom > index be4b85b8..9ffd93bb 100644 > --- a/include/libcamera/ipa/vimc.mojom > +++ b/include/libcamera/ipa/vimc.mojom > @@ -1,5 +1,10 @@ > /* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > + * \todo Document the interface as src/libcamera/ipa/vimc_ipa_interface.cpp > + * and remove the EXCLUDE_PATTERNS entry in Doxygen.in for its generation. > + */ > + > module ipa.vimc; > > import "include/libcamera/ipa/core.mojom"; > 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 > + */ You're missing a \file block here, Doxygen will not link the header in files.html without that. > + > +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 The license for meson.build files should be CC0-1.0. Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + > +libcamera_ipa_interfaces = files([ > + 'core_ipa_interface.cpp', > +]) > 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') >
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 \ # 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/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom index a717b1e6..9e3cd885 100644 --- a/include/libcamera/ipa/ipu3.mojom +++ b/include/libcamera/ipa/ipu3.mojom @@ -1,5 +1,10 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * \todo Document the interface as src/libcamera/ipa/ipu3_ipa_interface.cpp + * and remove the EXCLUDE_PATTERNS entry in Doxygen.in for its generation. + */ + module ipa.ipu3; import "include/libcamera/ipa/core.mojom"; diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom index 42321bee..770e3049 100644 --- a/include/libcamera/ipa/raspberrypi.mojom +++ b/include/libcamera/ipa/raspberrypi.mojom @@ -1,5 +1,10 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * \todo Document the interface as src/libcamera/ipa/raspberrypi_ipa_interface.cpp + * and remove the EXCLUDE_PATTERNS entry in Doxygen.in for its generation. + */ + module ipa.RPi; import "include/libcamera/ipa/core.mojom"; diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom index cca871a0..da6646df 100644 --- a/include/libcamera/ipa/rkisp1.mojom +++ b/include/libcamera/ipa/rkisp1.mojom @@ -1,5 +1,10 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * \todo Document the interface as src/libcamera/ipa/rkisp1_ipa_interface.cpp + * and remove the EXCLUDE_PATTERNS entry in Doxygen.in for its generation. + */ + module ipa.rkisp1; import "include/libcamera/ipa/core.mojom"; diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom index be4b85b8..9ffd93bb 100644 --- a/include/libcamera/ipa/vimc.mojom +++ b/include/libcamera/ipa/vimc.mojom @@ -1,5 +1,10 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * \todo Document the interface as src/libcamera/ipa/vimc_ipa_interface.cpp + * and remove the EXCLUDE_PATTERNS entry in Doxygen.in for its generation. + */ + module ipa.vimc; import "include/libcamera/ipa/core.mojom"; 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', +]) 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')