[libcamera-devel] [PATCH v3 2/6] ipa: mojom: Move CameraSensorInfo struct exclusively to IPA IPC
Umang Jain
umang.jain at ideasonboard.com
Mon May 24 10:23:51 CEST 2021
Hi Laurent,
On 5/24/21 6:24 AM, Laurent Pinchart wrote:
> Hi Umang,
>
> Thank you for the patch.
>
> On Fri, May 21, 2021 at 06:58:19PM +0530, Umang Jain wrote:
>> CameraSensorInfo structure is designed to pass in camera sensor related
>> information from pipeline-handler to IPA. Since the pipeline-handler
>> and IPA are connected via mojom IPC IPA interface, the interface
>> itself provides a more suitable placement of CameraSensorInfo,
>> instead of camera_sensor.h (which is a libcamera internal header
>> ultimately, at this point).
>>
>> As CameraSensorInfo is already defined in core.mojom, it is just
>> a matter of removing [skipHeader] tag to allow code-generation
>> of CameraSensorInfo.
>>
>> Finally, update header paths to include CameraSensorInfo definition
>> from IPA interfaces instead of "libcamera/internal/camera_sensor.h".
>>
>> Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
>> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
>> Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois at ideasonboard.com>
>> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>> ---
>> include/libcamera/internal/camera_sensor.h | 17 +--
>> include/libcamera/ipa/core.mojom | 2 +-
>> include/libcamera/ipa/ipa_interface.h | 2 -
>> src/ipa/raspberrypi/raspberrypi.cpp | 1 -
>> src/libcamera/camera_sensor.cpp | 111 -----------------
>> src/libcamera/ipa/core_ipa_interface.cpp | 132 +++++++++++++++++++++
>> 6 files changed, 134 insertions(+), 131 deletions(-)
>>
>> diff --git a/include/libcamera/internal/camera_sensor.h b/include/libcamera/internal/camera_sensor.h
>> index 2a5c51a1..0905ebfa 100644
>> --- a/include/libcamera/internal/camera_sensor.h
>> +++ b/include/libcamera/internal/camera_sensor.h
>> @@ -14,6 +14,7 @@
>> #include <libcamera/class.h>
>> #include <libcamera/controls.h>
>> #include <libcamera/geometry.h>
>> +#include <libcamera/ipa/core_ipa_interface.h>
>>
>> #include "libcamera/internal/formats.h"
>> #include "libcamera/internal/log.h"
>> @@ -24,22 +25,6 @@ namespace libcamera {
>> class BayerFormat;
>> class MediaEntity;
>>
>> -struct CameraSensorInfo {
>> - std::string model;
>> -
>> - uint32_t bitsPerPixel;
>> -
>> - Size activeAreaSize;
>> - Rectangle analogCrop;
>> - Size outputSize;
>> -
>> - uint64_t pixelRate;
>> - uint32_t lineLength;
>> -
>> - uint32_t minFrameLength;
>> - uint32_t maxFrameLength;
>> -};
>> -
>> class CameraSensor : protected Loggable
>> {
>> public:
>> diff --git a/include/libcamera/ipa/core.mojom b/include/libcamera/ipa/core.mojom
>> index e49815d8..b95b3dc4 100644
>> --- a/include/libcamera/ipa/core.mojom
>> +++ b/include/libcamera/ipa/core.mojom
>> @@ -78,7 +78,7 @@ module libcamera;
>> uint32 height;
>> };
>>
>> -[skipHeader] struct CameraSensorInfo {
>> +struct CameraSensorInfo {
>> string model;
>>
>> uint32 bitsPerPixel;
>> diff --git a/include/libcamera/ipa/ipa_interface.h b/include/libcamera/ipa/ipa_interface.h
>> index dfe1b40a..4aefaa71 100644
>> --- a/include/libcamera/ipa/ipa_interface.h
>> +++ b/include/libcamera/ipa/ipa_interface.h
>> @@ -18,8 +18,6 @@
>> #include <libcamera/geometry.h>
>> #include <libcamera/signal.h>
>>
>> -#include "libcamera/internal/camera_sensor.h"
>> -
>> namespace libcamera {
>>
>> /*
>> diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp
>> index 52d91db2..87774500 100644
>> --- a/src/ipa/raspberrypi/raspberrypi.cpp
>> +++ b/src/ipa/raspberrypi/raspberrypi.cpp
>> @@ -25,7 +25,6 @@
>> #include <libcamera/span.h>
>>
>> #include "libcamera/internal/buffer.h"
>> -#include "libcamera/internal/camera_sensor.h"
>> #include "libcamera/internal/log.h"
>>
>> #include <linux/bcm2835-isp.h>
>> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
>> index eb84d9eb..170de827 100644
>> --- a/src/libcamera/camera_sensor.cpp
>> +++ b/src/libcamera/camera_sensor.cpp
>> @@ -33,117 +33,6 @@ namespace libcamera {
>>
>> LOG_DEFINE_CATEGORY(CameraSensor)
>>
>> -/**
>> - * \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
>> - */
>> -
>> /**
>> * \class CameraSensor
>> * \brief A camera sensor based on V4L2 subdevices
>> diff --git a/src/libcamera/ipa/core_ipa_interface.cpp b/src/libcamera/ipa/core_ipa_interface.cpp
>> index fe1ecce6..7fef2c36 100644
>> --- a/src/libcamera/ipa/core_ipa_interface.cpp
>> +++ b/src/libcamera/ipa/core_ipa_interface.cpp
>> @@ -102,4 +102,136 @@ namespace libcamera {
>> * \brief The stream size in pixels
>> */
>>
>> +/**
>> + * \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.
>> + */
>> +
>> +/**
>> + * \fn CameraSensorInfo::CameraSensorInfo(const std::string &model,
>> + * uint32_t bitsPerPixel,
>> + * const Size &activeAreaSize,
>> + * const Rectangle &analogCrop,
>> + * const Size &outputSize,
>> + * uint64_t pixelRate,
>> + * uint32_t lineLength,
>> + * uint32_t minFrameLength,
>> + * uint32_t maxFrameLength)
>> + *
>> + * \param[in] model
>> + * \param[in] bitsPerPixel
>> + * \param[in] activeAreaSize
>> + * \param[in] analogCrop
>> + * \param[in] outputSize
>> + * \param[in] pixelRate
>> + * \param[in] lineLength
>> + * \param[in] minFrameLength
>> + * \param[in] maxFrameLength
> Given that this provides very little value in the documentation, I
> wonder if we shouldn't hide those constructors. What do you think ? If
> you agree, the following would address the issue.
Yes, I agree with this premise. I am actually going to squash the diff
below with Patch 1/6 instead, as it will also eliminate the need of
documenting IPAStream IPABuffer and IPASetting constructors there
>
> diff --git a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl
> index cdd75f89e384..94bb49181a35 100644
> --- a/utils/ipc/generators/libcamera_templates/definition_functions.tmpl
> +++ b/utils/ipc/generators/libcamera_templates/definition_functions.tmpl
> @@ -25,6 +25,7 @@ enum {{enum.mojom_name}} {
> struct {{struct.mojom_name}}
> {
> public:
> +#ifndef __DOXYGEN__
> {{struct.mojom_name}}() {%- if struct|has_default_fields %}
> :{% endif %}
> {%- for field in struct.fields|with_default_values -%}
> @@ -44,6 +45,8 @@ public:
> {%- endfor %}
> {
> }
> +#endif
> +
> {% for field in struct.fields %}
> {{field|name}} {{field.mojom_name}};
> {%- endfor %}
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>
>> + */
>> +
>> +/**
>> + * \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
>> + */
>> } /* namespace libcamera */
More information about the libcamera-devel
mailing list