[libcamera-devel] [PATCH v5 02/13] libcamera: camera: Introduce SensorConfiguration

[Laurent Pinchart]

Hi Jacopo,

Thank you for the patch.

On Thu, Sep 21, 2023 at 06:55:39PM +0200, Jacopo Mondi via libcamera-devel wrote:
> Introduce SensorConfiguration in the libcamera API.
> 
> The SensorConfiguration is part of the CameraConfiguration class
> and allows applications to control the sensor settings.
> 
> Signed-off-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> ---
>  include/libcamera/camera.h |  27 +++++++
>  src/libcamera/camera.cpp   | 144 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 171 insertions(+)
> 
> diff --git a/include/libcamera/camera.h b/include/libcamera/camera.h
> index 004bc89455f5..92a948e0f53b 100644
> --- a/include/libcamera/camera.h
> +++ b/include/libcamera/camera.h
> @@ -9,6 +9,7 @@
>  
>  #include <initializer_list>
>  #include <memory>
> +#include <optional>
>  #include <set>
>  #include <stdint.h>
>  #include <string>
> @@ -19,6 +20,7 @@
>  #include <libcamera/base/signal.h>
>  
>  #include <libcamera/controls.h>
> +#include <libcamera/geometry.h>
>  #include <libcamera/request.h>
>  #include <libcamera/stream.h>
>  #include <libcamera/transform.h>
> @@ -30,6 +32,30 @@ class FrameBufferAllocator;
>  class PipelineHandler;
>  class Request;
>  
> +class SensorConfiguration
> +{
> +public:
> +	unsigned int bitDepth = 0;
> +
> +	Rectangle analogCrop;
> +
> +	struct {
> +		unsigned int binX = 1;
> +		unsigned int binY = 1;
> +	} binning;
> +
> +	struct {
> +		unsigned int xOddInc = 1;
> +		unsigned int xEvenInc = 1;
> +		unsigned int yOddInc = 1;
> +		unsigned int yEvenInc = 1;
> +	} skipping;
> +
> +	Size outputSize;
> +
> +	bool valid() const;

We call those functions isValid() in libcamera.

> +};
> +
>  class CameraConfiguration
>  {
>  public:
> @@ -66,6 +92,7 @@ public:
>  	bool empty() const;
>  	std::size_t size() const;
>  
> +	std::optional<SensorConfiguration> sensorConfig;
>  	Transform transform;
>  
>  protected:
> diff --git a/src/libcamera/camera.cpp b/src/libcamera/camera.cpp
> index 0eecee766f00..05a47444be89 100644
> --- a/src/libcamera/camera.cpp
> +++ b/src/libcamera/camera.cpp
> @@ -97,6 +97,16 @@
>   * implemented in the above order at the hardware level. The libcamera pipeline
>   * handlers translate the pipeline model to the real hardware configuration.
>   *
> + * \subsection camera-sensor-model Camera Sensor Model
> + *
> + * By default, libcamera configures the camera sensor automatically based on the
> + * configuration of the streams. Applications may instead specify a manual
> + * configuration for the camera sensor. This allows precise control of the frame
> + * geometry and frame rate delivered by the sensor.
> + *
> + * More details about the camera sensor model implemented by libcamera are
> + * available in the libcamera camera-sensor-model documentation page.
> + *
>   * \subsection digital-zoom Digital Zoom
>   *
>   * Digital zoom is implemented as a combination of the cropping and scaling
> @@ -111,6 +121,127 @@ namespace libcamera {
>  
>  LOG_DECLARE_CATEGORY(Camera)
>  
> +/**
> + * \class SensorConfiguration
> + * \brief Camera sensor configuration
> + *
> + * The SensorConfiguration class collects parameters to control the operations
> + * of the camera sensor, accordingly to the abstract camera sensor model

s/accordingly/according/

> + * implemented by libcamera.
> + *
> + * \todo Applications shall fully populate all fields of the
> + * CameraConfiguration::sensorConfig class members before validating the
> + * CameraConfiguration. If the SensorConfiguration is not fully populated, or if
> + * any of its parameters cannot be applied to the sensor in use, the
> + * CameraConfiguration validation process will fail and return
> + * CameraConfiguration::Status::Invalid.

I think we should implement this already, right away. To avoid delaying
this series I will submit a patch on top.

> + *
> + * Applications that populate the SensorConfiguration class members are
> + * expected to be highly-specialized applications that know what sensor
> + * they are operating with and what parameters are valid for the sensor in use.
> + *
> + * A detailed description of the abstract camera sensor model implemented by
> + * libcamera and the description of its configuration parameters is available
> + * in the libcamera documentation camera-sensor-model file.
> + */
> +
> +/**
> + * \var SensorConfiguration::bitDepth
> + * \brief The sensor image format bit depth
> + *
> + * The number of bits (resolution) used to represent a pixel sample.
> + */
> +
> +/**
> + * \var SensorConfiguration::analogCrop
> + * \brief The analog crop rectangle
> + *
> + * The selected portion of the active pixel array used to produce the image
> + * frame.
> + */
> +
> +/**
> + * \var SensorConfiguration::binning
> + * \brief Sensor binning configuration
> + *
> + * Refer to the camera-sensor-model documentation for an accurate description
> + * of the binning operations. Disabled by default.
> + */
> +
> +/**
> + * \var SensorConfiguration::binX
> + * \brief Horizontal binning factor
> + *
> + * The horizontal binning factor. Default to 1.
> + */
> +
> +/**
> + * \var SensorConfiguration::binY
> + * \brief Vertical binning factor
> + *
> + * The vertical binning factor. Default to 1.
> + */
> +
> +/**
> + * \var SensorConfiguration::skipping
> + * \brief The sensor skipping configuration
> + *
> + * Refer to the camera-sensor-model documentation for an accurate description
> + * of the skipping operations.
> + *
> + * If no skipping is performed, all the structure fields should be
> + * set to 1. Disabled by default.
> + */
> +
> +/**
> + * \var SensorConfiguration::xOddInc
> + * \brief Horizontal increment for odd rows. Default to 1.
> + */
> +
> +/**
> + * \var SensorConfiguration::xEvenInc
> + * \brief Horizontal increment for even rows. Default to 1.
> + */
> +
> +/**
> + * \var SensorConfiguration::yOddInc
> + * \brief Vertical increment for odd columns. Default to 1.
> + */
> +
> +/**
> + * \var SensorConfiguration::yEvenInc
> + * \brief Vertical increment for even columns. Default to 1.
> + */
> +
> +/**
> + * \var SensorConfiguration::outputSize
> + * \brief The frame output (visible) size
> + *
> + * The size of the data frame as received by the host processor.
> + */
> +
> +/**
> + * \brief Validate the sensor configuration

* \brief Check if the sensor configuration is valid

to be consistent with the rest of the documentation.

> + *
> + * A sensor configuration is valid if it's fully populated.
> + *
> + * \todo For now allow applications to populate the bitDepth and the outputSize
> + * only as skipping and binnings factors are initialized to 1 and the analog
> + * crop is ignored.

I will fix this on top too.

> + *
> + * \return True if the sensor configuration is valid, false otherwise.

s/.$//

> + */
> +bool SensorConfiguration::valid() const
> +{
> +	if (bitDepth && binning.binX && binning.binY &&
> +	    skipping.xOddInc && skipping.yOddInc &&
> +	    skipping.xEvenInc && skipping.yEvenInc &&
> +	    !outputSize.isNull())
> +		return true;
> +
> +	return false;
> +}
> +
>  /**
>   * \class CameraConfiguration
>   * \brief Hold configuration for streams of the camera
> @@ -391,6 +522,19 @@ CameraConfiguration::Status CameraConfiguration::validateColorSpaces(ColorSpaceF
>  	return status;
>  }
>  
> +/**
> + * \var CameraConfiguration::sensorConfig
> + * \brief The camera sensor configuration
> + *
> + * The sensorConfig member allows control of the configuration of the camera
> + * sensor. Refer to the camera-sensor-model documentation and to the
> + * SensorConfiguration class documentation for details about the sensor
> + * configuration process.
> + *
> + * The camera sensor configuration applies to all streams produced by a camera
> + * from the same image source.

I'd like to add

 * If sensorConfig is not set, the camera will configure the sensor
 * automatically based on the configuration of the streams.

We can rephrase the block as follows:

 * The sensorConfig member allows manual control of the configuration of the
 * camera sensor. By default, if sensorConfig is not set, the camera will
 * configure the sensor automatically based on the configuration of the streams.
 * Applications can override this by manually specifying the full sensor
 * configuration.
 *
 * Refer to the camera-sensor-model documentation and to the SensorConfiguration
 * class documentation for details about the sensor configuration process.
 *
 * The camera sensor configuration applies to all streams produced by a camera
 * from the same image source.

If those changes are fine with you, I can update the patch when pushing.

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> + */
> +
>  /**
>   * \var CameraConfiguration::transform
>   * \brief User-specified transform to be applied to the image

-- 
Regards,

Laurent Pinchart