The lens shading correction is always applied based on the sensor crop
bounds. This leads to incorrect lens shading correction for analog crops
that do not cover the whole sensor.
To fix that, we need to adapt the lens shading table for the selected
analog crop at configure time. Introduce an abstract ShadingDescriptor
class that holds the lens shading information that can then be sampled
at configure time for a specific crop rectangle.
Resampling for a specific crop is only implemented for polynomial lsc
data. For tabular data, a warning is logged and the unmodified table is
returned. This matches the current functionality for tabular data and is
a huge improvement for polynomial data.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
There is no need to recalculate the sampling positions over and over.
Pass them as parameter into the sampling function. The vectors are still
kept inside the loop as this is also a preparatory change for the
upcoming refactoring.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The quantization functionality in the Interpolator type hinders in
writing nice code. Don't use it and implement the functionality directly
in the algorithm.
While at it, reduce the threshold to half of the quantization step size,
otherwise it might happen that we skip a full quantization step. Rename
the kColourTemperatureChangeThreshhold to kColourTemperatureQuantization
to better express the usecase.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Move the function definitions out of the related classes. This was noted
in review after the series was already merged. After trying it out I
must admit that it really improves readability and reduces the
indentation level by one.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Rui Wang <rui.wang@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 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>
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>
Add a WDR algorithm to do global tone mapping. Global tone mapping is
used to increase the perceived dynamic range of an image. The typical
effect is that in areas that are normally overexposed, additional
structure becomes visible.
The overall idea is that the algorithm applies an exposure value
correction to underexpose the image to the point where only a small
number of saturated pixels is left. This artificial underexposure is
then mitigated by applying a tone mapping curve.
This algorithm implements 4 tone mapping strategies:
- Linear
- Power
- Exponential
- Histogram equalization
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Query the params device for RKISP1_CID_SUPPORTED_PARAMS_BLOCKS and
inject the information into the IPA hardware context for use by the
algorithms.
To be able to modify the hardware configuration at runtime, replace the
pointer with an instance and create a copy of the static hardware
specific data.
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>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The Y mode of the histogram gets captured at the ISP output, before the
output formatter. This has the side effect that the first and the last
bins are empty in case of limited YUV range. Another side effect is
that gamma and GWDR processing is included in the histogram which makes
algorithm development very difficult. In RGB mode the histogram is taken
after xtalk (CCM) and is therefore independent of gamma and WDR. The
limited range issue also does not apply. In the ISP reference it is
however stated that "it is not possible to calculate a luminance or
grayscale histogram from an RGB histogram since the position information
is lost during its generation".
During testing the RGB histogram provided good data and better
algorithmic stability at a possible (but not measured) inaccuracy.
Another option would be to pass the color space information into the IPA
and strip the histogram accordingly. For ease of implementation switch to
the RGB mode.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
There are several occasions where quantization can lead to visible
effects.
In WDR mode it can happen that exposure times get set to very low values
(Sometimes 2-3 lines). This intentionally introduced underexposure is
corrected by the GWDR module. As exposure time is quantized by lines,
the smallest possible change in exposure time now results in a quite
visible change in perceived brightness.
On some sensors the possible gain steps are also quite large leading to
visible jumps if e.g. if the exposure time is fixed.
Mitigate that by applying a global gain to account for the error
introduced by the exposure quantization.
ToDo: This needs perfect frame synchronous control of the sensor to work
properly which is not guaranteed in all cases. It still improves the
behavior with the current regulation and can easily be skipped, be
removing the compress algorithm from the tuning file.
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>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Calculate the error introduced by quantization as "quantization gain"
and return it separately from splitExposure(). It is not included in the
digital gain, to not silently ignore the limits imposed by the AGC
configuration.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The i.MX8 M Plus has a compression curve inside the compand block. This
curve is necessary to process HDR stitched data and is useful for other
aspects like applying a digital gain to the incoming sensor data.
Add a basic algorithm for the compression curve. This algorithm has a
hardcoded input width of 20bit and output width of 12bit which matches
the imx8mp pipeline. Only a static gain is supported in this version.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Header comment blocks used to contain the file name. Considered as
useless information, the names have been removed, with the last ones
supposed to be dropped in commit d3bf27180e ("libcamera: Drop
remaining file names from header comment blocks"). A few have however
been forgotten, and more crept back since. Remove them.
While at it, fix one typo in a header comment block by replacing
'MaliC55 with Mali-C55', and add a missing blank line in
src/ipa/rpi/pisp/pisp.cpp.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
The AWB measurements are taken after the CCM. This can be seen by
enabling debug logging on AWB, disabling AWB (stats will still be
processed) and manually chaning the CCM.
This means that the estimated colour temperature and the corresponding
CCM also lead to changed rgbMeans which in turn leads to oscillations.
Fix that by applying the inverse transform on the rgbMeans.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
In the grey world AWB case, if no colour gains are contained in the
tuning file, the colour gains get reset to 1 when the colour temperature
is set manually. This is unexpected and undesirable. Allow the
gainsFromColourTemp() function to return a std::nullopt to handle that
case.
While at it, remove an unnecessary import from rkisp1/algorithms/awb.h.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Add a manual ColourCorrectionMatrix control. This was already discussed
while implementing manual colour temperature but was never implemented.
The control allows to manually specify the CCM when AwbEnable is false.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
In RkISP1Awb::process(), the color temperature in the active state is
updated every time new statistics are available. The CCM/LSC algorithms
use that value in prepare() to update the CCM/LSC. This is not correct
if the color temperature was specified manually and leads to visible
flicker even when AwbEnable is set to false.
To fix that, track the auto and manual color temperature separately in
active state. In Awb::prepare() the current frame context is updated
with the corresponding value from active state. Change the algorithms to
fetch the color temperature from the frame context instead of the active
state in prepare().
Fixes: 0230880954 ("ipa: rkisp1: awb: Implement ColourTemperature control")
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Swap gains and automatic/manual in the IPAActiveState structure. This is
in preparation to adding another member, which is easier in the new
structure. The patch contains no functional changes.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
When the AWB engine doesn't find a valid pixel because all pixels lie
outside the configured colour range it returns an AWB measurement value
of 255, 255, 255. This leaves the regulation in an unrecoverable state
noticeable by a completely green image. Fix that by skipping the AWB
calculation in case there were no valid pixels.
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>
The ControlInfo information for AwbEnable and ColourGains are declared
and exposed in the top-level IPA. These should instead be exposed by the
AWB part of the IPA, as it doesn't make sense to support these controls
when AWB is disabled, for example.
Move the declaration of these controls out of the top-level IPA and into
AWB.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The call to setControls(0) is counter productive. At start() time, no
requests were queued and no stats were received. So setControls(0)
accesses a zeroed frame context and in turn sends 0 as gain, exposure
and vblank to the pipeline handler and DelayedControls. This leads to
strong oscillations on every start of the camera.
A proper fix for handling the startup controls still needs to be done
and was already started in [1] and [2].
From a DelayedControls point of view the call to setControls(0) is also
unnecessary as DelayedControls treat frame 0 as already being queued in
after initialization.
So it is safe to just remove it and the removal fixes the zero
effectiveExposureValue discussed in the previous patch for rkisp1.
[1]: https://patchwork.libcamera.org/patch/21708/
[2]: https://patchwork.libcamera.org/patch/22445/
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
With the availability of metering modes and the corresponding weights,
there is a flexible way of defining the area that gets taken into
account when AEGC is calculated. There is no need to reduce that window
to an arbitrary region anymore. If need arises we can make this
parameter user configurable or add a control for it.
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>
The weights for a given metering mode are applied to the histogram data
inside the histogram statistics block. The AE statistics do not contain
any weights. Therefore the weights are honored when AgcMeanLuminance
calculates the upper or lower constraints, but ignored in the
calculation of the frame luminance. Fix that by manually applying the
weights in the luminance calculation.
Fixes: 4c5152843a ("ipa: rkisp1: Derive rkisp1::algorithms::Agc from AgcMeanLuminance")
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>
The minimum FrameDurationLimit also limits the min exposure time and
results in overly bright AE regulation. Remove the limit on the minimum
exposure time as the vertical blanking ensures the minimum frame
duration limit.
Fixes: f72c76eb6e ("rkisp1: Honor the FrameDurationLimits control")
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
The configured line duration of the sensor is used frequently throughout
the AGC implementation.
It's available in the IPA context through the rather long:
context.configuration.sensor.lineDuration
Take a copy of the lineDuration early in the call and replace the two
current usages of the reference with the shorter copy to manage line
length and ease readibility.
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
The IPA calculates and reports the FrameDurationLimits to applications
by configuring the ControlInfo accordingly during
IPARkISP1::updateControls()
We later need to know these limits during Agc::configure() for
initialising the ActiveState of the AGC implementation with the limits.
Store the FrameDurationLimits ControlInfo in the ControlInfoMap which is
now present in the IPAContext so that it is commonly available for the
AGC algorithm, removing the 'todo' accordingly.
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
