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

Kieran Bingham kieran.bingham at ideasonboard.com
Fri May 14 11:54:17 CEST 2021


Heya,

On 13/05/2021 09:17, Umang Jain wrote:
> Hi Paul
> 
> Thanks for your work.
> 
> On 5/13/21 1:23 PM, 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.
>>
>> 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.
> Perfectly understand-able. And this is already looking in a decent shape
> for starters.
>> ---
>>   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
> 2 questions
>  - Is it okay to have a .cpp in /include/ $includedir? Maybe yes, but I
> am not sure

I would say no ;-) Not unless someone intends to include the .cpp file.


>  - I have a struct to be documented in ipu3.mojom, so should I be making
> ipu3_ipa_interface.cpp in the same way, for documenting it? (or have
> only one "doc" file consolidation)?

Yes, it should be separated (as you have done). But it should be under
src/libcamera/ipa/ IMO



>> 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')
>>   
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards
--
Kieran


More information about the libcamera-devel mailing list