[libcamera-devel] [PATCH v5 02/13] libcamera: camera: Introduce SensorConfiguration
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Thu Oct 19 00:27:44 CEST 2023
Hi Nicolas,
On Thu, Oct 05, 2023 at 09:30:53AM -0400, Nicolas Dufresne wrote:
> Le mercredi 27 septembre 2023 à 00:33 +0300, Laurent Pinchart via libcamera-devel a écrit :
> > 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>
>
> Did my first read of this, with considering the libcamera-app and gstreamer RFC
> for sensor configuration. I think in order to actually solve someone problem, we
> need a mechanism to retrieve the IPA chosen configuration after validation with
> automatic configuration.
>
> This implementation only focus on hardware aware software, so does not really
> address any publicly discussed use cases. My general advise if you want to make
> new API that are clearly useful, is to take one of the integration you control
> and demonstrate its usability. Just adding configuration is agreed valid, but is
> it usable by anyone using libcamera in real ?
I don't like this API much (I don't think this is a surprise to anyone,
I've voiced that opinion already). I however think it's a step towards
solving the problem. We'll take further steps :-)
> > > + */
> > > +
> > > /**
> > > * \var CameraConfiguration::transform
> > > * \brief User-specified transform to be applied to the image
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list