The default CCM in uncalibrated.yaml is just an identity transformation
and has been enabled by default only to always provide a correction
matrix to GPU ISP. It slows down CPU ISP when CCM is not used.
Now, when a default correction matrix is always provided to GPU ISP, we
can disable the Ccm algorithm in uncalibrated.yaml again. The check for
ccmEnabled in GPU ISP is no longer needed and it must be removed in
order not to fail when Ccm algorithm is not enabled. ccmEnabled flag is
still needed in CPU ISP where the processing differs based on whether
CCM is present or not.
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Robert Mader <robert.mader@collabora.com>
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The Lut algorithm is not really an algorithm. Moreover, algorithms may
be enabled or disabled but with Lut disabled, nothing will work.
Let's move the construction of lookup tables to CPU debayering, where it
is used. The implied and related changes are:
- DebayerParams is changed to contain the real params rather than lookup
tables.
- contrastExp parameter introduced by GPU ISP is used for CPU ISP too.
- The params must be initialised so that debayering gets meaningful
parameter values even when some algorithms are disabled.
- combinedMatrix must be put to params everywhere where it is modified.
- Matrix changes needn't be tracked in the algorithms any more.
- CPU debayering must watch for changes of the corresponding parameters
to update the lookup tables when and only when needed.
- Swapping red and blue is integrated into lookup table constructions.
- gpuIspEnabled flags are removed as they are not needed any more.
Reviewed-by: Robert Mader <robert.mader@collabora.com>
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
control_ids.h defines the contrast type as float, let's use the same in
simple IPA, instead of double. Saturation and gamma already use float,
except for the knobs initializers, let's use float for the knobs too.
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Robert Mader <robert.mader@collabora.com>
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Saturation adjustments are implemented using matrix operations. They
are currently applied to the colour correction matrix. Let's move them
to a newly introduced separate "Adjust" algorithm.
This separation has the following advantages:
- It allows disabling general colour adjustments algorithms without
disabling the CCM algorithm.
- It keeps the CCM separated from other corrections.
- It's generally cleaner.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Robert Mader <robert.mader@collabora.com>
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The combined matrix must be reset to the initial value before each frame
is prepared. This must be done outside algorithms because any of the
algorithms may be disabled while the matrix must be always initialised.
Let's initialise the combined matrix to the identity matrix (which keeps
the pixel values unchanged) in software ISP just before calling
`prepare' on the algorithms.
Matrix updates can no longer be skipped in ccm.cpp, otherwise the CCM
won't be applied if there is no temperature or saturation change. We
explicitly track whether the CCM has been set up completely rather than
relying on the frame number, to avoid missing the initialisation in case
the first frame is skipped due to some error.
Reviewed-by: Robert Mader <robert.mader@collabora.com>
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Let's introduce IPAActiveState::combinedMatrix that is separate from
IPAActiveState::ccm and represents the overall correction matrix, not
only the sensor colour correction matrix.
IPAActiveState::ccm still includes everything; this is changed in the
followup patch.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
IPAActiveState::ccm stores the colour correction matrix (CCM) and
whether it has been changed. The change flag is later used when
recomputing or not the lookup tables.
But the CCM may include other corrections than just the sensor colour
correction, for example white balance and saturation adjustments. These
things should be separated and IPAActiveState::ccm should represent just
the CCM itself.
As the first step towards that cleanup, let's separate the change flag
from the CCM. And wrap the only remaining member of
IPAActiveState::ccm.
Also, let's reset the separated change flag in the lookup tables; it'll
be no longer tied to just CCM handling.
This patch doesn't change actual behaviour and it still reports the
combined matrix as CCM in metadata. This is addressed in the followup
patches.
Reviewed-by: Bryan O'Donoghue <bod.linux@nxsw.ie>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The black level offset subtracted in AWB is wrong. It assumes that the
stats contain sums of the individual colour pixels. But they actually
contain sums of the colour channels of larger "superpixels" consisting
of the individual colour pixels. Each of the RGB colour values and the
computed luminosity (a histogram entry) are added once to the stats per
such a superpixel. This means the offset computed from the black level
and the number of pixels should be used as it is, not divided.
The patch fixes the subtracted offset. Since the evaluation is the same
for all the three colours now, the individual class variables are
replaced with a single RGB variable.
Fixes: 4e13c6f55b ("Honor black level in AWB")
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Robert Mader <robert.mader@collabora.com>
Tested-by: Robert Mader <robert.mader@collabora.com>
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Update the tuning files to include the new AWB algorithm. It is
enabled by setting "enabled" to true for the AWB algorithm that you
want, and the same field to false for the one you don't want. Note
that you may enable only one of the two algorithms!
The AWB models themselves are not included with libcamera. They will
be supplied from the Raspberry Pi software repositories.
Signed-off-by: Peter Bailey <peter.bailey@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Algorithms may now contain an "enabled" field which can be set to
false to disable it and prevent it from being loaded. If not present,
algorithms are treated as enabled by default for backwards
compatability.
We additionally prevent duplicate versions of the same algorithm type
(such as AWB) from loading, and flag an error. This will prevent
undefined behaviour if (for example) two distinct AWB algorithms are
trying to run simultaneously.
Signed-off-by: Peter Bailey <peter.bailey@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Similar to what commit acfd602767 ("ipa: rkisp1: Fix algorithm controls
vanish after configure") did for the RkISP1 IPA, replace the usage of
unordered_map::merge() at updateControls() time with
unordered_map::insert().
As unordered_map::merge moves items from the source map, it deletes
controls registered at algorithms initialization time in the
ipaContext.ctrlMap. As at configure() time updateControls() is called
again and the list of Camera controls is refreshed, the controls
registered at algorithms initialization time are lost.
Fixes: 87353f2bba ("ipa: ipu3: Derive ipu3::algorithms::Agc from AgcMeanLuminance")
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Similar to what commit acfd602767 ("ipa: rkisp1: Fix algorithm controls
vanish after configure") did for the RkISP1 IPA, replace the usage of
unordered_map::merge() at updateControls() time with
unordered_map::insert().
As unordered_map::merge moves items from the source map, it deletes
controls registered at algorithms initialization time in the
ipaContext.ctrlMap. As at configure() time updateControls() is called
again and the list of Camera controls is refreshed, the controls
registered at algorithms initialization time are lost.
Fixes: fe989ee514 ("ipa: mali-c55: Add Mali-C55 ISP IPA module")
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The default values for controls::FrameDurationLimits is now an array but
the v4l2 proxy is fetching it as a scalar value, causing a runtime
error. Fix this by templating the getter with the correct
Span<const int64_t, 2> type.
This fix also requires the RPi initial default value for FrameDurationLimits
to be specified as a Span<const int64_t, 2>.
As a drive-by, remove the hard-coded 33ms min and 120ms max frame
duration values in the initial defaults, and use the defaultMinFrameDuration
and defaultMaxFrameDuration const values. This change is inconsequential
to runtime operation as these always get overridden on the first camera
configure call.
Fixes: 4e9be7d11b ("ipa: ipu3, mali-c55, rkisp1, rpi: Fix reporting non-scalar controls")
Closes: https://github.com/raspberrypi/libcamera/issues/321
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Pass contrastExp as calculated in lut to debayer params not the raw
contrast. This way we calculate contrastExp once per frame in lut and pass
the calculated value into the shaders, instead of passing contrast and
calculating contrastExp once per pixel in the shaders.
Signed-off-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Tested-by: Robert Mader <robert.mader@collabora.com>
Tested-by: Hans de Goede <johannes.goede@oss.qualcomm.com> # ThinkPad T14s gen 6 (arm64) ov02c10 + X1c gen 12 ov08x40
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com> # Lenovo X13s
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
If the first frame of a stream is bad, the IPA will not get called with
frame == 0, leaving activeState.agc expo/again uninitialized. This causes
the agc algorithm to set a very low gain and exposure on the next run
(where it will hit the if (!stats->valid) {} path) resulting in starting
with a black image.
Fix this by using a valid flag instead of checking for frame == 0.
The entire activeState gets cleared to 0 on configure() resetting the new
valid flag.
Signed-off-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
On most imx sensors the formula seems to be:
again_float = 512.0 / (512.0 - again_ctrl_value)
So the minimum again of 0 makes sense and actually translates to a gain of 1.0.
And the max gain of 400 leads to 512.0 / 112.0 = 4.57 which is about typical for
a maximum again. Since a minimum again value of 0 is actually normal, the special
handling of againMin == 0 is undesirable and this is actually causing problems
on these IMX sensors. again10 correctly gets set to 0 which is less than the
adjusted againMin which causes the AGC code to not work properly.
Fix this by dropping the special handling of againMin == 0.
Signed-off-by: Vasiliy Doylov <nekocwd@mainlining.org>
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The existing RkISP1Params helper class allows the RkISP1 IPA to handle
both the extensible parameters format and the legacy fixed-size format.
With the introduction of v4l2-isp.h in the Linux kernel the part of
the RkISP1Params helper class that handles extensible parameters can
be generalized so that other IPA modules can use the same helpers
to populate a parameters buffer compatible with v4l2-isp.h.
Generalize the RkISP1Params class to a new libipa component named
V4L2Params and derive the existing RkISP1Params from it, leaving
in the RkISP1-specific implementation the handling of the legacy format.
Deriving RkISP1Params from V4L2Params requires changing the size
associated to each block to include the size of v4l2_params_block_header
in the ipa:rkisp1::kBlockTypeInfo map as the V4L2Params::block()
implementation doesn't account for that as RkIS1Params::block()
implementation did.
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Tested-by: Antoine Bouyer <antoine.bouyer@nxp.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The AWB gains reported in the metadata are currently scaled against the
maximum gain, but in fact the gain itself is already stored as a
floating point gain value.
Remove the incorrect scaling and report the gains directly.
Fixes: a0b97475b1 ("ipa: simple: Report the ColourGains in metadata")
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The ControlInfos of non-scalar controls that are reported in controls()
must have non-scalar default values for controls that have a defined
size. This is because applications should be able to directly set the
default value from a ControlInfo to the control.
Currently this is relevant to the following controls:
- ColourGains
- ColourCorrectionMatrix
- FrameDurationLimits
- AfWindows
Fix the scalarness of these controls where relevant.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Tested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> # rkisp1
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The yTarget loading code is broken and works neither for plain values
nor for arrays of values to form a PWL. Fix this by supporting both
cases. If a list is provided in the tuning file construct a PWL,
otherwise construct a single point PWL with the given value.
Fixes: 24247a12c7 ("ipa: libipa: Add AgcMeanLuminance base class")
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
In some situations it is necessary to specify the target brightness
value depending on the overall lux level. Replace the float
relativeLuminanceTraget by a PWL. As the PWL loading code loads a plain
value as single point PWL, backwards compatibility to existing tuning
files is ensured.
While at it, order the class members in reverse xmas tree notation.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
With the upcoming regulation rework the processing order in the IPA is
updated a bit so that process() updates the active state with new
measurements and fills the metadata with the data from the corresponding
frame context. In prepare() all parameters for one frame are tied
together using the most up to date values from active state.
Change the lux algorithm to support that order of events. Also prepare
for cases where stats can be null which can happen with the upcoming
regulation rework.
While at it fix a formatting issue reported by checkstyle and drop a
unnecessary local variable.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
In the tuning files, it is useful to specify some values as PWL that
produces a different output value depending on an input value. For
example it is useful to specify the relativeLuminanceTarget depending on
the lux level, so that the regulation regulates a bit darker in low
light scenes. For simple setups this is not necessary and a single value
is sufficient.
This patch extends the yaml loading code, so that a single point PWL can
also be specified as a plain value. This way the following yaml
expressions are all valid:
yTarget: [ 1000, 0.15, 2000, 0.17 ] # Regular PWL
yTarget: [ 0, 0.17 ] # Single point PWL
yTarget: 0.17 # Same as above
For cases (I'm not aware of any) where a single point Pwl is not allowed
there is no change as that must be checked externally anyways.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
In commit f077c58e08 ("libipa: exposure_mode_helper: Take
exposure/gain quantization into account") calculation of the
quantization gain was added to ExposureModeHelper::clampGain(). This
works as expected when a sensor helper is configured but the gain is not
reset to 1.0 in case the sensor helper is not configured. This leads to
incorrect gain calculations in ExposureModeHelper::splitExposure() as
that expects the quantization gain to be valid in any case. Fix that by
setting the quantization gain to 1.0 in case no sensor helper is
configured.
Fixes: f077c58e08 ("libipa: exposure_mode_helper: Take exposure/gain quantization into account")
Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/292
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Tested-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
It is beneficial to have the option during development to disable and
enable algorithms via the tuning file without having to delete their
entries.
Add support for an optional "enabled" parameter to accomplish this.
Usage example:
version: 1
algorithms:
- Agc:
enabled: true
- Awb:
enabled: false
This will enable AGC, and disable AWB. If the enabled flag is not
present, the algorithm will be enabled.
Signed-off-by: Isaac Scott <isaac.scott@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
[Kieran: Reflow text]
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Software CPU ISP computes a gamma lookup table. The table incorporates
black level and contrast. All entries in the table below the black
level are set to 0. This is not necessarily correct all the time.
Let's consider this case: The CCM is
[1 0 0]
[0 1 0]
[0 0 0]
and contrast is set to zero. The gamma table has all the entries above
the black level set to 186 (due to zero contrast) and all the entries
below the black level set to 0. CCM is applied before gamma, a
non-black level pixel has the blue component set to 0 with the CCM
above. Now, when the gamma lookup is applied, the red and green
components are set to 186, while the blue component is set to 0. The
resulting pixel is then yellow rather than grey (as it should be with
zero contrast).
There are two ways to fix this: Either clamping pixel colour channels to
the black level in debayering or setting the below black level entries
in the gamma lookup table to the lowest value of the gamma table rather
than 0. Both should have the same effect. Let's opt for the latter for
its simplicity.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Hans de Goede <johannes.goede@oss.qualcomm.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The filter strength configuration block was previously only enabled on the
first frame (frame == 0). Apply the strength values when denoise is active.
This prevents the strength config from being disabled on subsequent frames.
Signed-off-by: Rui Wang <rui.wang@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
