[libcamera-devel] [PATCH v4 4/7] libcamera: camera_sensor: Define CameraSensorInfo

[Niklas Söderlund]

Hi Jacopo,

Thanks for your work.

On 2020-04-27 23:32:33 +0200, Jacopo Mondi wrote:
> Define the CameraSensorInfo structure that reports the current image sensor
> configuration.
> 
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
>  src/libcamera/camera_sensor.cpp       | 84 +++++++++++++++++++++++++++
>  src/libcamera/include/camera_sensor.h | 13 +++++
>  2 files changed, 97 insertions(+)
> 
> diff --git a/src/libcamera/camera_sensor.cpp b/src/libcamera/camera_sensor.cpp
> index ce585cab1a4f..4fd1ee84eb47 100644
> --- a/src/libcamera/camera_sensor.cpp
> +++ b/src/libcamera/camera_sensor.cpp
> @@ -28,6 +28,90 @@ 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 here reported information describe the sensor's intrinsics

s/here//
s/describe/describes/

> + * 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.

I see no reference to the 'mode' in the structure, can this paragraph be 
reworked to not mention it or will it be extended to cover mode later ?

> + *
> + * Instances of this structure are meant to be assembled by the CameraSensor
> + * class by inspecting the sensor static properties as well as the currently
> + * configured sensor mode.
> + */
> +
> +/**
> + * \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 pixel produced in a second
> + *
> + * The pixel read out frequency in Hz. The property describes how many pixels
> + * per second are produced by the camera sensor, including blanking.
> + *
> + * To obtain the read-out time in second of a full line:
> + *
> + * \verbatim
> +	lineDuration(s) = lineLength(pixel) / pixelRate(Hz)
> +   \endverbatim
> + */
> +
> +/**
> + * \var CameraSensorInfo::lineLength
> + * \brief Total line length in pixels
> + *
> + * The total line length in pixel clock periods, including blanking.
> + */
> +
>  /**
>   * \class CameraSensor
>   * \brief A camera sensor based on V4L2 subdevices
> diff --git a/src/libcamera/include/camera_sensor.h b/src/libcamera/include/camera_sensor.h
> index 5277f7f7fe87..28ebfaa30c22 100644
> --- a/src/libcamera/include/camera_sensor.h
> +++ b/src/libcamera/include/camera_sensor.h
> @@ -22,6 +22,19 @@ class V4L2Subdevice;
>  
>  struct V4L2SubdeviceFormat;
>  
> +struct CameraSensorInfo {
> +	std::string model;

I'm hearing whispers we can drop the model field which would make me 
happy :-)

> +
> +	uint32_t bitsPerPixel;
> +
> +	Size activeAreaSize;
> +	Rectangle analogCrop;
> +	Size outputSize;
> +
> +	uint32_t pixelRate;
> +	uint32_t lineLength;
> +};
> +
>  class CameraSensor : protected Loggable
>  {
>  public:
> -- 
> 2.26.1
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel

-- 
Regards,
Niklas Söderlund