[libcamera-devel,v2,2/7] ipa: mojom: Move CameraSensorInfo struct exclusively to IPA IPC

12323 diff mbox series

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@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@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   | 131 +++++++++++++++++++++
 6 files changed, 133 insertions(+), 131 deletions(-)

Hi Umang and Paul,

On 19/05/2021 12:19, 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@ideasonboard.com>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jean-Michel Hautbois <jeanmichel.hautbois@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   | 131 +++++++++++++++++++++
>  6 files changed, 133 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..def8d184 100644
> --- a/src/libcamera/ipa/core_ipa_interface.cpp
> +++ b/src/libcamera/ipa/core_ipa_interface.cpp
> @@ -102,4 +102,135 @@ 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
> + */
> +
> +/**
> + * \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 */
>

Hi Umang,

On 19/05/2021 11:19, 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".
> 

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>
> Signed-off-by: Paul Elder <paul.elder@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   | 131 +++++++++++++++++++++
>  6 files changed, 133 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..def8d184 100644
> --- a/src/libcamera/ipa/core_ipa_interface.cpp
> +++ b/src/libcamera/ipa/core_ipa_interface.cpp
> @@ -102,4 +102,135 @@ 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
> + */
> +
> +/**
> + * \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 */
>

Hi Umang

On Wed, May 19, 2021 at 03:49:49PM +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@ideasonboard.com>
> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>

[snip]

> +
> +/**
> + * \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)

What's this and why does it have this peculiar style ?

> + * \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
> + */
> +
> +/**
> + * \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 */
> --
> 2.26.2
>

Hi Jacopo

On 5/21/21 3:01 PM, Jacopo Mondi wrote:
> Hi Umang
>
> On Wed, May 19, 2021 at 03:49:49PM +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@ideasonboard.com>
>> Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
> [snip]
>
>> +
>> +/**
>> + * \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)
> What's this and why does it have this peculiar style ?
ouch, it looked fine to me in vim and checkstyle wasn't able to catch 
this (I double checked right-now)
Fixing it in next iteration. Thanks for noticing
>
>> + * \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
>> + */
>> +
>> +/**
>> + * \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 */
>> --
>> 2.26.2
>>