[libcamera-devel] [PATCH] ipa: mojom: Move CameraSensorInfo struct exclusively to IPA IPC
Umang Jain
umang.jain at ideasonboard.com
Wed May 12 08:47:19 CEST 2021
Hi Laurent,
On 5/12/21 12:16 AM, Laurent Pinchart wrote:
> 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.
yes, I agree and have applied the changes locally.
>
>>> + * \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.
Pinged epaul for that on IRC and see what path we should take.
I will follow up there itself
>>> + *
>>> + * 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
More information about the libcamera-devel
mailing list