[libcamera-devel] [PATCH] ipa: mojom: Move CameraSensorInfo struct exclusively to IPA IPC
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue May 11 20:46:42 CEST 2021
Hello,
On Tue, May 11, 2021 at 05:31:38PM +0100, Kieran Bingham wrote:
> On 11/05/2021 12:56, Umang Jain wrote:
> > CameraSensorInfo structure is designed to pass in camera sensor related
> > information from pipeline-handler to IPA. Since the pipeline-hander
> > 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. Also, update header paths to include
> > CameraSensorInfo definition from IPA interfaces instead of
> > "libcamera/internal/camera_sensor.h".
> >
> > Signed-off-by: Umang Jain <umang.jain at ideasonboard.com>
> > ---
> > include/libcamera/internal/camera_sensor.h | 17 +---
> > include/libcamera/ipa/core.mojom | 112 ++++++++++++++++++++-
> > include/libcamera/ipa/ipa_interface.h | 2 -
> > src/ipa/raspberrypi/raspberrypi.cpp | 1 -
> > src/libcamera/camera_sensor.cpp | 111 --------------------
> > 5 files changed, 112 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>
>
> Hrm, I feel like that deserves it's own 'group' (i.e. a new line above
> it) because it's the IPA as a sub component almost ... but equally, it's
> in the right place here by alphabetical sorting so ... I suspect it's fine.
>
> >
> > #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 6caaa63e..2a09b233 100644
> > --- a/include/libcamera/ipa/core.mojom
> > +++ b/include/libcamera/ipa/core.mojom
> > @@ -78,7 +78,117 @@ module libcamera;
> > uint32 height;
> > };
> >
> > -[skipHeader] struct CameraSensorInfo {
> > +/**
> > + * \struct CameraSensorInfo
I wonder if we should rename this to IPACameraSensorInfo to match the
other structures.
> > + * \brief Report the image sensor characteristics
>
> I haven't checked yet, but do we need to do anything to make sure
> doxygen parses this?
>
> Hopefully doxygen will pick it up from the generated header file - but
> can you check that it builds please?
As far as I can tell, mojo doesn't process comments, and doxygen can't
parse mojom files. We may need to move this to an otherwise empty .cpp
file.
> > + *
> > + * 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
> > + */
> > +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"
> > -
>
> Certainly glad to see this go now ;-)
>
> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
>
> > 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
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list