[libcamera-devel] [PATCH v4 4/7] libcamera: camera_sensor: Define CameraSensorInfo
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Apr 28 03:50:15 CEST 2020
Hi Jacopo and Niklas,
On Tue, Apr 28, 2020 at 02:00:05AM +0200, Niklas Söderlund wrote:
> 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 ?
It already covers the mode, which consists on the active configuration
of the sensor's internal pipeline (formats and selection rectangles). I
think sensor mode is a well known term, but there would also be pedantic
arguments to not use it, as not all drivers are mode-based (but it could
also be argued that they configure a mode in any case). I'll let Jacopo
decide if he prefers to keep the text as is or write something along the
lines of "as well as information relative to the current configuration".
> > + *
> > + * 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
s/pixel/pixels/
> > + *
> > + * The pixel read out frequency in Hz. The property describes how many pixels
Technically speaking the unit is pixels per second, not Hz, but I'll
pretend not to know :-) If you want to fix it, I'd just drop the first
sentence as it would duplicate the second one with Hz replaced with
pixels per second.
> > + * per second are produced by the camera sensor, including blanking.
> > + *
> > + * To obtain the read-out time in second of a full line:
s/second/seconds/
> > + *
> > + * \verbatim
> > + lineDuration(s) = lineLength(pixel) / pixelRate(Hz)
And here that would be s/Hz/pixels per second/
> > + \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 :-)
I thought we could, but then realized we still need it at least to
select the right embedded data parser. We may be able to revisit that
later, but it will take time.
Great work Jacopo !
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > +
> > + uint32_t bitsPerPixel;
> > +
> > + Size activeAreaSize;
> > + Rectangle analogCrop;
> > + Size outputSize;
> > +
> > + uint32_t pixelRate;
> > + uint32_t lineLength;
> > +};
> > +
> > class CameraSensor : protected Loggable
> > {
> > public:
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list