[{"id":17184,"web_url":"https://patchwork.libcamera.org/comment/17184/","msgid":"<20210524094638.GK902042@pyrite.rasen.tech>","date":"2021-05-24T09:46:38","subject":"Re: [libcamera-devel] [PATCH v4 1/6] ipa: Move core IPA interface\n\tdocumentation to a .cpp file","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"Hi Umang,\n\nOn Mon, May 24, 2021 at 02:50:18PM +0530, Umang Jain wrote:\n> Moving the core.mojom documentation to its corresponding .cpp file\n> (core_ipa_interface.cpp). This will allow Doxygen to generate the\n> documentation for IPABuffer, IPASettings and IPAStream structures.\n> Since the .mojom files are placed in include/ directory, the .cpp file\n> will live in $sourcedir/src/libcamera/ipa/ - which can also contain\n> documentation for other mojom generated IPA interfaces in subsequent\n> commit.\n> \n> Also hide the constructors in generated IPA interface from doxygen,\n> via  #ifndef __DOXYGEN__. These constructors provide no major value in\n> documenting them, instead will spew out doxygen warnings during the\n> build.\n> \n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>\n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\nReviewed-by: Paul Elder <paul.elder@ideasonboard.com>\n\n> ---\n>  Documentation/Doxyfile.in                     |  8 +-\n>  Documentation/meson.build                     |  1 +\n>  include/libcamera/ipa/core.mojom              | 72 ---------------\n>  include/libcamera/ipa/ipu3.mojom              |  5 ++\n>  include/libcamera/ipa/raspberrypi.mojom       |  5 ++\n>  include/libcamera/ipa/rkisp1.mojom            |  5 ++\n>  include/libcamera/ipa/vimc.mojom              |  5 ++\n>  src/libcamera/ipa/core_ipa_interface.cpp      | 89 +++++++++++++++++++\n>  src/libcamera/ipa/meson.build                 |  5 ++\n>  src/libcamera/meson.build                     |  1 +\n>  .../definition_functions.tmpl                 |  3 +\n>  11 files changed, 125 insertions(+), 74 deletions(-)\n>  create mode 100644 src/libcamera/ipa/core_ipa_interface.cpp\n>  create mode 100644 src/libcamera/ipa/meson.build\n> \n> diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in\n> index af006724..8305f56a 100644\n> --- a/Documentation/Doxyfile.in\n> +++ b/Documentation/Doxyfile.in\n> @@ -844,7 +844,6 @@ EXCLUDE                = @TOP_SRCDIR@/include/libcamera/span.h \\\n>  \t\t\t @TOP_SRCDIR@/src/libcamera/pipeline/ \\\n>  \t\t\t @TOP_SRCDIR@/src/libcamera/tracepoints.cpp \\\n>  \t\t\t @TOP_BUILDDIR@/include/libcamera/internal/tracepoints.h \\\n> -\t\t\t @TOP_BUILDDIR@/include/libcamera/ipa/ \\\n>  \t\t\t @TOP_BUILDDIR@/src/libcamera/proxy/\n>  \n>  # The EXCLUDE_SYMLINKS tag can be used to select whether or not files or\n> @@ -861,7 +860,12 @@ EXCLUDE_SYMLINKS       = NO\n>  # Note that the wildcards are matched against the file with absolute path, so to\n>  # exclude all test directories for example use the pattern */test/*\n>  \n> -EXCLUDE_PATTERNS       =\n> +EXCLUDE_PATTERNS       = @TOP_BUILDDIR@/include/libcamera/ipa/*_serializer.h \\\n> +                         @TOP_BUILDDIR@/include/libcamera/ipa/*_proxy.h \\\n> +                         @TOP_BUILDDIR@/include/libcamera/ipa/ipu3_*.h \\\n> +                         @TOP_BUILDDIR@/include/libcamera/ipa/raspberrypi_*.h \\\n> +                         @TOP_BUILDDIR@/include/libcamera/ipa/rkisp1_*.h \\\n> +                         @TOP_BUILDDIR@/include/libcamera/ipa/vimc_*.h \\\n>  \n>  # The EXCLUDE_SYMBOLS tag can be used to specify one or more symbol names\n>  # (namespaces, classes, functions, etc.) that should be excluded from the\n> diff --git a/Documentation/meson.build b/Documentation/meson.build\n> index c8521574..9ecf4dfc 100644\n> --- a/Documentation/meson.build\n> +++ b/Documentation/meson.build\n> @@ -24,6 +24,7 @@ if doxygen.found() and dot.found()\n>                        doxyfile,\n>                        libcamera_internal_headers,\n>                        libcamera_ipa_headers,\n> +                      libcamera_ipa_interfaces,\n>                        libcamera_public_headers,\n>                        libcamera_sources,\n>                        libipa_headers,\n> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom\n> index 6caaa63e..e49815d8 100644\n> --- a/include/libcamera/ipa/core.mojom\n> +++ b/include/libcamera/ipa/core.mojom\n> @@ -94,88 +94,16 @@ module libcamera;\n>  \tuint32 maxFrameLength;\n>  };\n>  \n> -/**\n> - * \\struct IPABuffer\n> - * \\brief Buffer information for the IPA interface\n> - *\n> - * The IPABuffer structure associates buffer memory with a unique ID. It is\n> - * used to map buffers to the IPA with IPAInterface::mapBuffers(), after which\n> - * buffers will be identified by their ID in the IPA interface.\n> - */\n> -\n> -/**\n> - * \\var IPABuffer::id\n> - * \\brief The buffer unique ID\n> - *\n> - * Buffers mapped to the IPA are identified by numerical unique IDs. The IDs\n> - * are chosen by the pipeline handler to fulfil the following constraints:\n> - *\n> - * - IDs shall be positive integers different than zero\n> - * - IDs shall be unique among all mapped buffers\n> - *\n> - * When buffers are unmapped with IPAInterface::unmapBuffers() their IDs are\n> - * freed and may be reused for new buffer mappings.\n> - */\n> -\n> -/**\n> - * \\var IPABuffer::planes\n> - * \\brief The buffer planes description\n> - *\n> - * Stores the dmabuf handle and length for each plane of the buffer.\n> - */\n> -\n>  struct IPABuffer {\n>  \tuint32 id;\n>  \t[hasFd] array<FrameBuffer.Plane> planes;\n>  };\n>  \n> -/**\n> - * \\struct IPASettings\n> - * \\brief IPA interface initialization settings\n> - *\n> - * The IPASettings structure stores data passed to the IPAInterface::init()\n> - * function. The data contains settings that don't depend on a particular camera\n> - * or pipeline configuration and are valid for the whole life time of the IPA\n> - * interface.\n> - */\n> -\n> -/**\n> - * \\var IPASettings::configurationFile\n> - * \\brief The name of the IPA configuration file\n> - *\n> - * This field may be an empty string if the IPA doesn't require a configuration\n> - * file.\n> - */\n> -\n> - /**\n> - * \\var IPASettings::sensorModel\n> - * \\brief The sensor model name\n> - *\n> - * Provides the sensor model name to the IPA.\n> - */\n>  struct IPASettings {\n>  \tstring configurationFile;\n>  \tstring sensorModel;\n>  };\n>  \n> -/**\n> - * \\struct IPAStream\n> - * \\brief Stream configuration for the IPA interface\n> - *\n> - * The IPAStream structure stores stream configuration parameters needed by the\n> - * IPAInterface::configure() method. It mirrors the StreamConfiguration class\n> - * that is not suitable for this purpose due to not being serializable.\n> - */\n> -\n> -/**\n> - * \\var IPAStream::pixelFormat\n> - * \\brief The stream pixel format\n> - */\n> -\n> -/**\n> - * \\var IPAStream::size\n> - * \\brief The stream size in pixels\n> - */\n>  struct IPAStream {\n>  \tuint32 pixelFormat;\n>  \tSize size;\n> diff --git a/include/libcamera/ipa/ipu3.mojom b/include/libcamera/ipa/ipu3.mojom\n> index a717b1e6..9e3cd885 100644\n> --- a/include/libcamera/ipa/ipu3.mojom\n> +++ b/include/libcamera/ipa/ipu3.mojom\n> @@ -1,5 +1,10 @@\n>  /* SPDX-License-Identifier: LGPL-2.1-or-later */\n>  \n> +/*\n> + * \\todo Document the interface as src/libcamera/ipa/ipu3_ipa_interface.cpp\n> + * and remove the EXCLUDE_PATTERNS entry in Doxygen.in for its generation.\n> + */\n> +\n>  module ipa.ipu3;\n>  \n>  import \"include/libcamera/ipa/core.mojom\";\n> diff --git a/include/libcamera/ipa/raspberrypi.mojom b/include/libcamera/ipa/raspberrypi.mojom\n> index 42321bee..770e3049 100644\n> --- a/include/libcamera/ipa/raspberrypi.mojom\n> +++ b/include/libcamera/ipa/raspberrypi.mojom\n> @@ -1,5 +1,10 @@\n>  /* SPDX-License-Identifier: LGPL-2.1-or-later */\n>  \n> +/*\n> + * \\todo Document the interface as src/libcamera/ipa/raspberrypi_ipa_interface.cpp\n> + * and remove the EXCLUDE_PATTERNS entry in Doxygen.in for its generation.\n> + */\n> +\n>  module ipa.RPi;\n>  \n>  import \"include/libcamera/ipa/core.mojom\";\n> diff --git a/include/libcamera/ipa/rkisp1.mojom b/include/libcamera/ipa/rkisp1.mojom\n> index cca871a0..da6646df 100644\n> --- a/include/libcamera/ipa/rkisp1.mojom\n> +++ b/include/libcamera/ipa/rkisp1.mojom\n> @@ -1,5 +1,10 @@\n>  /* SPDX-License-Identifier: LGPL-2.1-or-later */\n>  \n> +/*\n> + * \\todo Document the interface as src/libcamera/ipa/rkisp1_ipa_interface.cpp\n> + * and remove the EXCLUDE_PATTERNS entry in Doxygen.in for its generation.\n> + */\n> +\n>  module ipa.rkisp1;\n>  \n>  import \"include/libcamera/ipa/core.mojom\";\n> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom\n> index be4b85b8..9ffd93bb 100644\n> --- a/include/libcamera/ipa/vimc.mojom\n> +++ b/include/libcamera/ipa/vimc.mojom\n> @@ -1,5 +1,10 @@\n>  /* SPDX-License-Identifier: LGPL-2.1-or-later */\n>  \n> +/*\n> + * \\todo Document the interface as src/libcamera/ipa/vimc_ipa_interface.cpp\n> + * and remove the EXCLUDE_PATTERNS entry in Doxygen.in for its generation.\n> + */\n> +\n>  module ipa.vimc;\n>  \n>  import \"include/libcamera/ipa/core.mojom\";\n> diff --git a/src/libcamera/ipa/core_ipa_interface.cpp b/src/libcamera/ipa/core_ipa_interface.cpp\n> new file mode 100644\n> index 00000000..a6711e0e\n> --- /dev/null\n> +++ b/src/libcamera/ipa/core_ipa_interface.cpp\n> @@ -0,0 +1,89 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2021, Google Inc.\n> + *\n> + * core_ipa_interface.cpp - Docs file for core.mojom generated header\n> + */\n> +\n> +namespace libcamera {\n> +\n> +/**\n> + * \\file core_ipa_interface.h\n> + * \\brief Core IPA inteface\n> + */\n> +\n> +/**\n> + * \\struct IPABuffer\n> + * \\brief Buffer information for the IPA interface\n> + *\n> + * The IPABuffer structure associates buffer memory with a unique ID. It is\n> + * used to map buffers to the IPA with IPAInterface::mapBuffers(), after which\n> + * buffers will be identified by their ID in the IPA interface.\n> + */\n> +\n> +/**\n> + * \\var IPABuffer::id\n> + * \\brief The buffer unique ID\n> + *\n> + * Buffers mapped to the IPA are identified by numerical unique IDs. The IDs\n> + * are chosen by the pipeline handler to fulfil the following constraints:\n> + *\n> + * - IDs shall be positive integers different than zero\n> + * - IDs shall be unique among all mapped buffers\n> + *\n> + * When buffers are unmapped with IPAInterface::unmapBuffers() their IDs are\n> + * freed and may be reused for new buffer mappings.\n> + */\n> +\n> +/**\n> + * \\var IPABuffer::planes\n> + * \\brief The buffer planes description\n> + *\n> + * Stores the dmabuf handle and length for each plane of the buffer.\n> + */\n> +\n> +/**\n> + * \\struct IPASettings\n> + * \\brief IPA interface initialization settings\n> + *\n> + * The IPASettings structure stores data passed to the IPAInterface::init()\n> + * function. The data contains settings that don't depend on a particular camera\n> + * or pipeline configuration and are valid for the whole life time of the IPA\n> + * interface.\n> + */\n> +\n> +/**\n> + * \\var IPASettings::configurationFile\n> + * \\brief The name of the IPA configuration file\n> + *\n> + * This field may be an empty string if the IPA doesn't require a configuration\n> + * file.\n> + */\n> +\n> +/**\n> + * \\var IPASettings::sensorModel\n> + * \\brief The sensor model name\n> + *\n> + * Provides the sensor model name to the IPA.\n> + */\n> +\n> +/**\n> + * \\struct IPAStream\n> + * \\brief Stream configuration for the IPA interface\n> + *\n> + * The IPAStream structure stores stream configuration parameters needed by the\n> + * IPAInterface::configure() method. It mirrors the StreamConfiguration class\n> + * that is not suitable for this purpose due to not being serializable.\n> + */\n> +\n> +/**\n> + * \\var IPAStream::pixelFormat\n> + * \\brief The stream pixel format\n> + */\n> +\n> +/**\n> + * \\var IPAStream::size\n> + * \\brief The stream size in pixels\n> + */\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/ipa/meson.build b/src/libcamera/ipa/meson.build\n> new file mode 100644\n> index 00000000..560b2fdd\n> --- /dev/null\n> +++ b/src/libcamera/ipa/meson.build\n> @@ -0,0 +1,5 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +libcamera_ipa_interfaces = files([\n> +    'core_ipa_interface.cpp',\n> +])\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 7bc59b84..61b5fe21 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -67,6 +67,7 @@ includes = [\n>      libcamera_includes,\n>  ]\n>  \n> +subdir('ipa')\n>  subdir('pipeline')\n>  subdir('proxy')\n>  \n> diff --git a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl\n> index cdd75f89..94bb4918 100644\n> --- a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl\n> +++ b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl\n> @@ -25,6 +25,7 @@ enum {{enum.mojom_name}} {\n>  struct {{struct.mojom_name}}\n>  {\n>  public:\n> +#ifndef __DOXYGEN__\n>  \t{{struct.mojom_name}}() {%- if struct|has_default_fields %}\n>  \t\t:{% endif %}\n>  {%- for field in struct.fields|with_default_values -%}\n> @@ -44,6 +45,8 @@ public:\n>  {%- endfor %}\n>  \t{\n>  \t}\n> +#endif\n> +\n>  {% for field in struct.fields %}\n>  \t{{field|name}} {{field.mojom_name}};\n>  {%- endfor %}\n> -- \n> 2.26.2\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 5818FC3201\n\tfor <parsemail@patchwork.libcamera.org>;\n\tMon, 24 May 2021 09:46:47 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9FA706891E;\n\tMon, 24 May 2021 11:46:46 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 84F35602B1\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon, 24 May 2021 11:46:45 +0200 (CEST)","from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id E5A781315;\n\tMon, 24 May 2021 11:46:43 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"uCMZe8MP\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1621849605;\n\tbh=tO8j1JRzQ90vqXmHVZRpTpYuzi+fz1q9mEZg6q9QFHY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=uCMZe8MP03eNpgZkMfjanBBAcqJWjTfgoMonLHFUAvwgO9n5W9+BNNZIKLWxAh497\n\tINtiLDyRksBd5Yl989V2DP4uIrWoX6oV3m/E97fXxM+d7JKpPnaLTz8WV7Sd5ZvfBH\n\tw46zkBlRKVYo5hNwdTaOXzK5OpBQBTdMCE8DPTPw=","Date":"Mon, 24 May 2021 18:46:38 +0900","From":"paul.elder@ideasonboard.com","To":"Umang Jain <umang.jain@ideasonboard.com>","Message-ID":"<20210524094638.GK902042@pyrite.rasen.tech>","References":"<20210524092023.91779-1-umang.jain@ideasonboard.com>\n\t<20210524092023.91779-2-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20210524092023.91779-2-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v4 1/6] ipa: Move core IPA interface\n\tdocumentation to a .cpp file","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]