[libcamera-devel] [PATCH v3 09/23] libcamera: camera_sensor_properties: Add sensor delays
Jacopo Mondi
jacopo at jmondi.org
Fri Jul 1 10:59:17 CEST 2022
Hi Kieran
On Thu, Jun 30, 2022 at 11:37:32PM +0100, Kieran Bingham wrote:
> Quoting Jacopo Mondi via libcamera-devel (2022-06-30 14:38:48)
> > Add to the camera sensor properties a map of control identifiers to
> > their latencies.
> >
> > Some camera sensor controls take a variable number of frames to take
> > effect, hence they should be set in advance. The newly introduced map
> > records that information as a property of the camera sensor class.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > .../internal/camera_sensor_properties.h | 4 ++
> > .../camera_sensor_properties.cpp | 40 +++++++++++++++++++
> > 2 files changed, 44 insertions(+)
> >
> > diff --git a/include/libcamera/internal/camera_sensor_properties.h b/include/libcamera/internal/camera_sensor_properties.h
> > index 1ee3cb994106..2404b0674323 100644
> > --- a/include/libcamera/internal/camera_sensor_properties.h
> > +++ b/include/libcamera/internal/camera_sensor_properties.h
> > @@ -9,6 +9,7 @@
> >
> > #include <map>
> > #include <string>
> > +#include <unordered_map>
> >
> > #include <libcamera/control_ids.h>
> > #include <libcamera/geometry.h>
> > @@ -16,10 +17,13 @@
> > namespace libcamera {
> >
> > struct CameraSensorProperties {
> > + using SensorDelays = std::unordered_map<const ControlId *, unsigned int>;
> > +
> > static const CameraSensorProperties *get(const std::string &sensor);
> >
> > Size unitCellSize;
> > std::map<controls::draft::TestPatternModeEnum, int32_t> testPatternModes;
> > + SensorDelays sensorDelays;
> > };
> >
> > } /* namespace libcamera */
> > diff --git a/src/libcamera/camera_sensor/camera_sensor_properties.cpp b/src/libcamera/camera_sensor/camera_sensor_properties.cpp
> > index e5f27f06eb1d..4b5ab716abe2 100644
> > --- a/src/libcamera/camera_sensor/camera_sensor_properties.cpp
> > +++ b/src/libcamera/camera_sensor/camera_sensor_properties.cpp
> > @@ -13,6 +13,8 @@
> >
> > #include <libcamera/control_ids.h>
> >
> > +#include <libcamera/internal/control_ids.h>
> > +
> > /**
> > * \file camera_sensor_properties.h
> > * \brief Database of camera sensor properties
> > @@ -41,6 +43,18 @@ LOG_DEFINE_CATEGORY(CameraSensorProperties)
> > * \brief Map that associates the TestPattern control value with the indexes of
> > * the corresponding sensor test pattern modes as returned by
> > * V4L2_CID_TEST_PATTERN.
> > + *
> > + * \var CameraSensorProperties::sensorDelays
> > + * \brief Map that associates control identifiers with their latencies,
> > + * expressed in frame periods.
> > + */
> > +
> > +/**
> > + * \typedef CameraSensorProperties::SensorDelays
> > + * \brief Camera sensor controls delays
> > + *
> > + * Type to represent a map of camera sensor controls associated with their
> > + * latencies, expressed in frame periods.
> > */
> >
>
> I wonder if defining a 'common' struct of sensor delays here could ease
> referencing it in the default case.
>
> static const SensorDelays commonDelays = {
> { &controls::internal::FrameDuration, 2 },
> { &controls::internal::ExposureTime, 2 },
> { &controls::internal::AnalogueGain, 1 },
> };
>
If we want something like this, I think it should not be here but
rather in CameraSensor.
Have a look at
[PATCH v3 10/23] libcamera: camera_sensor: Initialize delayed controls
+int CameraSensor::initDelayedControls()
+{
+ /* If no static priorities are available, all controls have delay 0. */
+ static const CameraSensorProperties::SensorDelays noDelays{};
+ const CameraSensorProperties::SensorDelays *delays = staticProps_
+ ? &staticProps_->sensorDelays
+ : &noDelays;
The noDelays{} there can be made a defaultDelays map.
I actually had it but I had no idea how "default" those defaults
actually are..
> Or ideally, if we knew 'all ccs' cameras or such used these delays we
> could have a better name ?
>
> defaultDelays ?
>
> I know I commented on this before saying if we don't have the numbers we
> should leave the the delays as zero, but I think there I meant on an
> individual entry.
>
> But perhaps now I see this I'm changing my mind too - and leaning
> towards a default set can easily be defined and referenced, and
> overridden in the case of a difference?
>
>
> > /**
> > @@ -69,6 +83,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > * 9: "Resolution Pattern"
> > */
> > },
> > + .sensorDelays = { },
>
> then
> .sensorDelays = &commonDelays;
> ?
>
> Though this is the hi846, so we should identify what the delays really
> are there.
>
> > } },
> > { "imx219", {
> > .unitCellSize = { 1120, 1120 },
> > @@ -79,6 +94,11 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
> > { controls::draft::TestPatternModePn9, 4 },
> > },
> > + .sensorDelays = {
> > + { &controls::internal::FrameDuration, 2 },
> > + { &controls::internal::ExposureTime, 2 },
> > + { &controls::internal::AnalogueGain, 1 },
> > + },
>
> But this one could be:
> .sensorDelays = &commonDelays;
>
> > } },
> > { "imx258", {
> > .unitCellSize = { 1120, 1120 },
> > @@ -89,18 +109,26 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > { controls::draft::TestPatternModeColorBarsFadeToGray, 3 },
> > { controls::draft::TestPatternModePn9, 4 },
> > },
> > + .sensorDelays = { },
> > } },
> > { "imx290", {
> > .unitCellSize = { 2900, 2900 },
> > .testPatternModes = {},
> > + .sensorDelays = {
> > + { &controls::internal::FrameDuration, 2 },
> > + { &controls::internal::ExposureTime, 2 },
> > + { &controls::internal::AnalogueGain, 2 },
> > + },
> > } },
> > { "imx296", {
> > .unitCellSize = { 3450, 3450 },
> > .testPatternModes = {},
> > + .sensorDelays = { },
>
> The IMX296 is listed in RPi's cam_helper_imx296 without an override.
> That means in their implementation that this is expected to use
>
> .sensorDelays = &commonDelays;
Ah ok, missed that !
>
> > } },
> > { "imx477", {
> > .unitCellSize = { 1550, 1550 },
> > .testPatternModes = {},
> > + .sensorDelays = { },
>
> cam_helper_imx477 has the following overrides:
>
> exposure_delay = 2;
> gain_delay = 2;
> vblank_delay = 3;
>
Ah thanks, I'll fix
> > } },
> > { "ov2740", {
> > .unitCellSize = { 1400, 1400 },
> > @@ -108,6 +136,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > { controls::draft::TestPatternModeOff, 0 },
> > { controls::draft::TestPatternModeColorBars, 1},
> > },
> > + .sensorDelays = { },
> > } },
> > { "ov5640", {
> > .unitCellSize = { 1400, 1400 },
> > @@ -115,10 +144,16 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > { controls::draft::TestPatternModeOff, 0 },
> > { controls::draft::TestPatternModeColorBars, 1 },
> > },
> > + .sensorDelays = { },
> > } },
> > { "ov5647", {
> > .unitCellSize = { 1400, 1400 },
> > .testPatternModes = {},
> > + .sensorDelays = {
> > + { &controls::internal::FrameDuration, 2 },
> > + { &controls::internal::ExposureTime, 2 },
> > + { &controls::internal::AnalogueGain, 2 },
> > + },
> > } },
> > { "ov5670", {
> > .unitCellSize = { 1120, 1120 },
> > @@ -126,6 +161,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > { controls::draft::TestPatternModeOff, 0 },
> > { controls::draft::TestPatternModeColorBars, 1 },
> > },
> > + .sensorDelays = { },
> > } },
> > { "ov5675", {
> > .unitCellSize = { 1120, 1120 },
> > @@ -133,6 +169,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > { controls::draft::TestPatternModeOff, 0 },
> > { controls::draft::TestPatternModeColorBars, 1 },
> > },
> > + .sensorDelays = { },
> > } },
> > { "ov5693", {
> > .unitCellSize = { 1400, 1400 },
> > @@ -145,6 +182,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > * Rolling Bar".
> > */
> > },
> > + .sensorDelays = { },
> > } },
> > { "ov8865", {
> > .unitCellSize = { 1400, 1400 },
> > @@ -159,6 +197,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > * 5: "Color squares with rolling bar"
> > */
> > },
> > + .sensorDelays = { },
> > } },
> > { "ov13858", {
> > .unitCellSize = { 1120, 1120 },
> > @@ -166,6 +205,7 @@ const CameraSensorProperties *CameraSensorProperties::get(const std::string &sen
> > { controls::draft::TestPatternModeOff, 0 },
> > { controls::draft::TestPatternModeColorBars, 1 },
> > },
> > + .sensorDelays = { },
> > } },
> > };
> >
> > --
> > 2.36.1
> >
More information about the libcamera-devel
mailing list