Replace the dropFrameCount parameter returned from ipa::start() to the
pipeline handler by startupFrameCount and invalidFrameCount. The former
counts the number of frames required for AWB/AGC to converge, and the
latter counts the number of invalid frames produced by the sensor when
starting up.
In the pipeline handler, use the sum of these 2 values to replicate the
existing dropFrameCount behaviour.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Add a new status enum, FrameStartup, used to denote that even though
the frame has been successfully captured, the IQ parameters set by the
IPA will cause the frame to be unusable and applications are advised to
not consume this frame. An example of this would be on a cold-start of
the 3A algorithms, and there will be large oscillations to converge to
a stable state quickly.
Additional, update the definition of the FrameError state to cover the
usage when the sensor is known to produce a number of invalid/error
frames after stream-on.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Return `-EINVAL` from `Process::start()` if any of the file descriptors
are negative as those most likely signal some kind of issue such as
missed error checking.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Returning 0 when a running process is already managed can be confusing
since the parameters might be completely different, causing the caller
to mistakenly assume that the program it specified has been started.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Firstly, get the number of arguments first, and use that to determine the
size of the allocation instead of retrieving it twice.
Secondly, use `const_cast` instead of a C-style cast when calling `execv()`.
Third, use `size_t` to match the type of `args.size()`.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
A `Process` object has address identity because a pointer to it is
stored inside the `ProcessManager`. However, copy/move special
methods are still generated by the compiler. So disable them to
avoid potential issues and confusion.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The default argument already takes care of passing no fence to
`addBuffer()`, so there is no reason to specify `nullptr` explicitly.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The abi-compliance checker reports 100% compatibility in this release.
As such the SONAME is maintained at 0.5.
Binary compatibility: 100%
Source compatibility: 100%
Total binary compatibility problems: 0, warnings: 0
Total source compatibility problems: 0, warnings: 0
This release brings 93 commits with a large proportion of fixes and
cleanup againt earlier releases. Improvements have been made to the
Raspberry Pi Camera Tuning Tools, and the geometry, matrix and vector
class helpers have been expanded for greater reuse throughout the
project.
Notably for packagers - IPA modules now have their own subdirectory
which should prevent undesirable surrupticious error messages that would
occur if packagers choose to install the V4L2 adaptation layer in the
same folder as the IPA modules.
The RKISP1 can now adapt to more complex input pipelines, including
FPGAs and multiplexors, which has been beneficial for users on the
i.MX8MP, and the IPA algorithms for i.MX8MP and RKISP1 continue to get
improvements.
The software ISP has a new Saturation control (available when the CCM is
enabled).
The Documentation and pipeline handler writers guide has been
re-reviewed and cleaned up.
On the application and test side, lc-compliance now includes
multi-stream tests, and cam has extended support for display formats and
now prevents issues on non-display GPUs when rendering direct to DRM.
Contributors:
36 Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
15 Stefan Klug <stefan.klug@ideasonboard.com>
5 David Plowman <david.plowman@raspberrypi.com>
5 Kieran Bingham <kieran.bingham@ideasonboard.com>
5 Laurent Pinchart <laurent.pinchart@ideasonboard.com>
4 Milan Zamazal <mzamazal@redhat.com>
4 Quentin Schulz <quentin.schulz@cherry.de>
3 Daniel Scally <dan.scally@ideasonboard.com>
3 Paul Elder <paul.elder@ideasonboard.com>
2 Hou Qi <qi.hou@nxp.com>
2 Julien Vuillaumier <julien.vuillaumier@nxp.com>
2 Naushir Patuck <naush@raspberrypi.com>
2 Niklas Söderlund <niklas.soderlund@ragnatech.se>
2 Pavel Machek <pavel@ucw.cz>
1 Benjamin Mugnier <benjamin.mugnier@foss.st.com>
1 Nícolas F. R. A. Prado <nfraprado@collabora.com>
1 Sven Püschel <s.pueschel@pengutronix.de>
108 files changed, 3359 insertions(+), 528 deletions(-)
Integration overview:
The following commits in this release relate to either a bug fix or an
improvement to an existing commit.
- meson: Do not automatically build documentation if sphinx-build-3 is found
- Fixes: aba567338b ("Documentation: Move all dependencies into features")
- Revert "libcamera: rkisp1: Eliminate hard-coded resizer limits"
- Fixes: 761545407c ("pipeline: rkisp1: Filter out sensor sizes not supported by the pipeline")
- pipeline: rkisp1: Fix vblank delay
- Fixes: f72c76eb6e ("rkisp1: Honor the FrameDurationLimits control")
- utils: raspberrypi: ctt: Fix NaNs in lens shading tables
- Bug: https://github.com/raspberrypi/libcamera/issues/254
- utils: raspberrypi: ctt: Fix NaNs in chromatic aberration tables
- Bug: https://github.com/raspberrypi/libcamera/issues/254
- utils: raspberrypi: ctt: Fix integer division error calculating LSC cell size
- Bug: https://github.com/raspberrypi/libcamera/issues/260
- apps: qcam: Push the viewfinder role to vector
- Fixes: ee2b011b65 ("apps: cam: Try raw role if default viewfinder role fails")
- ipa: Move IPA installations to a subdir
- Bug: https://bugs.libcamera.org/show_bug.cgi?id=268
- ipa: rkisp1: algorithms: awb: Fix wrong colour temperature reporting
- Fixes: b60bd37b1a ("ipa: rkisp1: Move calculation of RGB means into own function")
- ipa: rkisp1: ccm/lsc: Fix CCM/LSC based on manual color temperature
- Fixes: 0230880954 ("ipa: rkisp1: awb: Implement ColourTemperature control")
- libcamera: controls: Fix `ControlInfoMap::count(unsigned int)`
- Fixes: 76b9923e55 ("libcamera: controls: Avoid exception in ControlInfoMap count() and find()")
- apps: cam: capture_script: Disallow arrays of strings
- Fixes: b35f04b3c1 ("cam: capture_script: Support parsing array controls")
- libcamera: matrix: Fix compilation error in inverse() function
- Fixes: 6287ceff5a ("libcamera: matrix: Add inverse() function")
- ipa: rpi: controller: rpi: Fix colour gain typo in AGC
- Fixes: 29892f1c56 ("ipa: libipa: colour: Use the RGB class to model RGB values")
And the following updates have been made in this release, grouped by category:
core:
- meson: Make the default value of "documentation" feature explicit
- meson: Do not automatically build documentation if sphinx-build-3 is found
- libcamera: request: Avoid double map lookup
- utils: rkisp1: gen-csc-table: Support printing CCM in decimal
- libcamera: ipa_module: Avoid unnecessary copy when getting signature
- libcamera: controls: Disallow arrays of arrays
- libcamera: media_device: Add helper to return matching entities
- libcamera: internal: Add MediaPipeline helper
- libcamera: stream: Add color space to configuration string representation
- README.rst: remove unnecessary dependency for qcam
- libcamera: v4l2_videodevice: Log buffer count on allocation error
- libcamera: matrix: Replace SFINAE with static_asserts
- libcamera: matrix: Make most functions constexpr
- libcamera: matrix: Add a Span based constructor
- libcamera: vector: Add a Span based constructor
- libcamera: matrix: Add inverse() function
- libcamera: matrix: Extend multiplication operator to heterogenous types
- libcamera: vector: Extend matrix multiplication operator to heterogenous types
- libcamera: controls: Fix `ControlInfoMap::count(unsigned int)`
- utils: codegen: Make users depend on `controls.py` in meson
- libcamera: matrix: Fix compilation error in inverse() function
- libcamera: sensor: Fix the gain delay for IMX283
- treewide: Do not use `*NameValueMap` for known values
- utils: codegen: ipc: Use `any()` instead of `len([]) > 0`
- utils: codegen: ipc: Remove `namespace` argument
- utils: codegen: ipc: Add `deserializer()` function
- utils: codegen: ipc: Log error code when remote call fails
- utils: codegen: ipc: Simplify `return` statements
- libcamera: ipa_data_serializer: Remove some vector `reserve()` calls
- libcamera: mali-c55: Remove tpgCodes_
- libcamera: mali-c55: Remove tpgSizes_ member from MaliC55CameraData
- libcamera: process: Use _exit in child process
- libcamera: process: Pass stderr and reserve stdin and stdout fds
- guides: pipeline-handler: Update name of pipeline handler stop function
- libcamera: mali-c55: Fix error paths in ::init()
pipeline:
- libcamera: software_isp: Add a clarification comment to AWB
- libcamera: pipeline: uvcvideo: Expose `Gamma` control
- libcamera: software_isp: Fix CCM multiplication
- libcamera: pipeline: virtual: Fix typo in log message
- libcamera: pipeline: imx8-isi: Remove unused variable
- pipeline: rkisp1: Fix vblank delay
- libcamera: pipeline: rkisp1: Convert to use MediaPipeline
- libcamera: pipeline: uvcvideo: Report new AeEnable control as available
- ipu3: cio2: Remove unused function definition
- libcamera: software_isp: Add saturation control
- Revert "libcamera: rkisp1: Eliminate hard-coded resizer limits"
apps:
- apps: lc-compliance: Support multiple streams in helpers
- apps: lc-compliance: Add multi-stream tests
- apps: cam: capture_script: Simplify bool array parsing
- gstreamer: Fixate colorimetry field during caps negotiation
- apps: cam: Try raw role if default viewfinder role fails
- apps: qcam: Push the viewfinder role to vector
- py: Set `PYTHONPATH` in devenv
- apps: cam: sdl_texture: Take list of buffers in span
- apps: cam: sdl_texture: Drop `&rect_` from `SDL_Update{NV,}Texture()` call
- apps: cam: sdl_texture: Add `SDLTexture1Plane`
- apps: cam: sdl_sink: Support more single-plane formats
- gstreamer: Add GstVideoMeta support
- apps: cam: capture_script: Disallow arrays of strings
- apps: cam: Skip non-display GPUs
ipa:
- utils: ipc: Do not duplicate signals in proxy object
- utils: ipc: Do not define variables in signal handler up front
- ipa: rpi: common: Avoid warnings when AeEnable control is used
- ipa: rpi: awb: Remove "fast" parameter
- ipa: Move IPA installations to a subdir
- ipa: rkisp1: awb: Declare ControlInfo in AWB
- ipa: rkisp1: awb: Ignore empty AWB statistics
- ipa: rkisp1: Refactor automatic/manual structure in IPAActiveState
- ipa: rkisp1: algorithms: awb: Fix wrong colour temperature reporting
- ipa: rkisp1: ccm/lsc: Fix CCM/LSC based on manual color temperature
- ipa: rkisp1: Implement manual ColourCorrectionMatrix control
- libipa: awb: Make result of gainsFromColourTemp optional
- ipa: rkisp1: Damp color temperature regulation
- ipa: rkisp1: awb: Take the CCM into account for the AWB gains calculation
- ipa: rkisp1: awb: Avoid division by zero
- ipa: rpi: controller: rpi: Fix colour gain typo in AGC
- ipa: rpi: Add tuning for IMX283
- ipa: rpi: Prevent segfault if AGC algorithm is absent
tuning:
- utils: raspberrypi: ctt: Fix NaNs in lens shading tables
- utils: raspberrypi: ctt: Fix NaNs in chromatic aberration tables
- utils: raspberrypi: ctt: Fix integer division error calculating LSC cell size
documentation:
- Documentation: guides: pipeline-handler: Fix camera creation
- Documentation: guides: pipeline-handler: Fix property list file name
- Documentation: guides: pipeline-handler: Fix configuration creation
- Documentation: guides: pipeline-handler: Fix `Camera::create()` link
- Documentation: guides: pipeline-handler: Simplify format collection
- Documentation: guides: pipeline-handler: Query pixel formats once
- Documentation: guides: application-developer: Remove unnecessary argument
- Documentation: Fix `INCLUDE_PATH` doxygen configuration option
- doc: Mention right meson version
- doc: document libtiff dependency for cam
test:
- test: Add minimal test for Matrix
- lc-compliance: Move camera setup to CameraHolder class
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
libcamera header files should be included using the `libcamera/...` prefix.
However, `INCLUDE_PATH` is currently set to `@TOP_SRCDIR@/include/libcamera`
meaning that doxygen, when encountering `libcamera/x.h`, will try to open
`@TOP_SRCDIR@/include/libcamera/libcamera/x.h`, which is not the correct
path.
Fix that by using `@TOP_{BUILD,SRC}DIR@/include`. This removes the extra
`libcamera` component from the path and adds the corresponding directory
from the build directory as well since that is an implicit include
directory added by meson.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Saturation control is added on top of the colour correction matrix. A
method of saturation adjustment that can be fully integrated into the
colour correction matrix is used. The control is available only if Ccm
algorithm is enabled.
The control uses 0.0-2.0 value range, with 1.0 being unmodified
saturation, 0.0 full desaturation and 2.0 quite saturated.
The saturation is adjusted by converting to Y'CbCr colour space,
applying the saturation value on the colour axes, and converting back to
RGB. ITU-R BT.601 conversion is used to convert between the colour
spaces, for no particular reason.
The colour correction matrix is applied before gamma and the given
matrix is suitable for such a case. Alternatively, the transformation
used in libcamera rpi ccm.cpp could be used.
Signed-off-by: Milan Zamazal <mzamazal@redhat.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>
In the MaliC55CameraData::init() function there are two places that
return values they shouldn't; the ret variable is returned after
checking a pointer is not null instead of an explicit -ENODEV and later
the boolean value false is returned on failure instead of the error
value returned by V4L2Subdevice::open() - fix both problems.
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Since commit f6b6f15b54 ("libcamera: pipeline: Introduce
stopDevice()") the stop function needed to be implemented by pipeline
handlers was renamed to stopDevice().
Update the pipeline handler writers guide to match this.
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Jai Luthra <jai.luthra@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The private function cio2BufferReady is defined but not implemented or
used, remove it for the class definition.
Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Device::openCard() in the cam DRM helpers looks for a /dev/dri/card*
device that can be opened and that doesn't fail when asked about
DRM_CAP_DUMB_BUFFER capability (regardless whether the capability is
supported by the device).
There can be matching devices that are not display devices. This can
lead to selection of such a device and inability to use KMS output with
the `cam' application. The ultimate goal is to display something on the
device and later the KMS sink will fail if there is no connector
attached to the device (although it can actually fail earlier, when
trying to set DRM_CLIENT_CAP_ATOMIC capability if this is not
supported).
Let's avoid selecting devices without connectors, CRTCs or encoders.
The added check makes the original check for DRM_CAP_DUMB_BUFFER API
most likely unnecessary, let's remove it.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Mattijs Korpershoek <mkorpershoek@kernel.org>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Even without AGC definition in the tuning file, the application would
still dereference agc unconditionally, leading to a segmentation fault
if AGC is absent.
This is relevant for sensors already providing AGC/AEC by themselves.
Check if AGC is present prior to setting maximum exposure time.
Signed-off-by: Benjamin Mugnier <benjamin.mugnier@foss.st.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Tested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com> # RPi4 + imx708_wide
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
When a child process is started from Process::start(), the file
descriptors inherited from the parent process are closed, except
the ones explicitly listed in the fds[] argument.
One issue is that the file descriptors for stdin, stdout and stderr
being closed, the subsequent file descriptors created by the child
process will reuse the values 0, 1 and 2 that are now available.
Thus, usage of printf(), assert() or alike may direct its output
to the new resource bound to one of these reused file descriptors.
The other issue is that the child process can no longer log on
the console because stderr has been closed.
To address the 2 issues, Process:start() is amended as below:
- Child process inherits from parent's stderr fd in order to share
the same logging descriptor
- Child process stdin, stdout and stderr fds are bound to /dev/null
if not inherited from parent. That is to prevent those descriptors
to be reused for any other resource, that could be corrupted by
the presence of printf(), assert() or alike.
Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.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>
Use _exit() in child process in case of execv() error. That is to
avoid interfering with the parent process as exit() may call its
atexit() handlers and flush its io buffers.
Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The tpgSizes_ vector is only used within the initTPGData() function.
Drop it and use a local variable instead.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
MaliC55CameraData stores a vector of the TPG's mbus codes (if the
camera in question is a TPG). This is never used - remove it.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
`appendPOD()` does a single insertion, so if only a single `appendPOD()`
will be called on a vector before returning, then calling `reserve()`
is not that useful, so remove it.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Returning an expression of type `void` from a function returning `void`
is legal, so do not handle those cases specially.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
The error code can be useful in diagnosing the underlying issue,
so log that as well, not just the existence of the issue.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Add `deserializer()` in `serializer.tmpl` to have a single function
that generates all the necessary functions into the template specialization
like `serializer()`. This also avoids the duplication of some
conditional logic.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
The `serializer()`, `deserializer_{fd,no_fd,simple}()` functions
take a string argument named "namespace", but they do not use it.
So remove the argument.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Use `any()` with a generator expression instead of constructing
a list and checking its length.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
When the value is known, do not look it up via the control's `NameValueMap`,
instead, just refer to the value directly.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The IMX283 uses a gain delay of 1 instead of the current value of 2 as
defined in the sensor properties. Fix it.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Acked-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Add calibrated tuning for the IMX283 sensor for pisp. Update the vc4
tuning file to match the new calibration.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Acked-by: Paul Elder <paul.elder@ideasonboard.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
A simple typo crept in where the red gain had been re-typed rather
than using the correct green gain. In particular, this was causing
very dark images for sensors that use large red gains, such as the
IMX477 outdoors.
Fixes: 29892f1c56 ("ipa: libipa: colour: Use the RGB class to model RGB values")
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Currently, modifying `controls.py` does not make those build targets dirty
that use a script that includes it (e.g. `gen-controls.py`) because meson
has no knowledge of this dependency. Add `depend_files` to each
`custom_target()` invocation to fix this.
Ideally it would be possible to attach this dependency to `gen_controls`,
`gen_gst_controls`, etc. objects themselves, so that repetition is
avoided, but this does not seem possible at the moment.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Acked-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
The current `ControlValue` mechanism does not support arrays
of strings, the assignment in the removed snippet will in fact
trigger an assertion failure in `ControlValue::set()` because
`sizeof(std::string) != ControlValueSize[ControlTypeString]`.
Fixes: b35f04b3c1 ("cam: capture_script: Support parsing array controls")
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
The `AeEnable` control is handled by the `Camera` class directly, but it
still has to be added because `ControlInfoMap`s are not easily modifiable.
See 338ba00e7a ("ipa: rkisp1: agc: Report new AeEnable control as available")
for more details and a similar change in rkisp1.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The two overloads of `find()` and `at()` have the same behaviour
regardless of the argument type: `unsigned int` or `const ControlId *`.
However, `count()` is not so because `count(unsigned int)` only checks
the `ControlIdMap`, and it does not check if the given id is actually
present in the map storing the `ControlInfo` objects.
So `count()` returns 1 for every control id that is present in the
associated `ControlIdMap` regardless of whether there is an actual
entry for the `ControlId` associated with the given numeric id.
Fix that by simply using `find()` to determine the return value.
Fixes: 76b9923e55 ("libcamera: controls: Avoid exception in ControlInfoMap count() and find()")
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
As the gains can also be specified manually, the regulation can run into
numeric instabilities by dividing by near zero. Mitigate that by
applying a small minium value.
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>
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>
Damp the regulation of the color temperature with the same factor as the
gains. Not damping the color temperature leads to visible flicker, as
the CCM changes too much.
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 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>
In commit b60bd37b1a ("ipa: rkisp1: Move calculation of RGB means into
own function") the output of the current measured colour temperature as
metadata was incorrectly added. Remove it.
Fixes: b60bd37b1a ("ipa: rkisp1: Move calculation of RGB means into own function")
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>
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>
It is useful to multiply matrices and vectors of heterogeneous types, for
instance float and double. Extend the multiplication operator to support
this, avoiding the need to convert one of the operations. The type of the
returned vector is selected automatically to avoid loosing precision.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
It is useful to multiply matrices of heterogneous types, for instance
float and double. Extend the multiplication operator to support this,
avoiding the need to convert one of the matrices. The type of the
returned matrix is selected automatically to avoid loosing precision.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Add a few tests for the Matrix class. This is not full fledged but at
least a starter.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
For calculations in upcoming algorithm patches, the inverse of a matrix
is required. Add an implementation of the inverse() function for square
matrices.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
When one wants to create a Vector from existing data, currently the only
way is via std::array. Add a Span based constructor to allow creation
from std::vectors and alike.
While at it, replace the manual loop with std::copy.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
When one wants to create a Matrix from existing data, currently the only
way is via std::array. Add a Span based constructor to allow creation
from vectors and alike.
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>
By zero-initializing the data_ member we can make most functions
constexpr which will come in handy in upcoming patches. Note that this
is due to C++17. In C++20 we will be able to leave data_ uninitialized
for constexpr. The Matrix(std::array) version of the constructor can
not be constexpr because std::copy only became constexpr in C++20.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
SFINAE is difficult to read and not needed in these cases. Replace it
with static_asserts. The idea came from [1] where it is stated:
"The use of enable_if seems misguided to me. SFINAE is useful for the
situation where we consider multiple candidates for something (overloads
or class template specializations) and try to choose the correct one,
without causing compilation to fail."
[1]: https://stackoverflow.com/questions/62109526/c-friend-template-that-use-sfinae
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
