Inline the function as it is only used in a single place and does not do
anything complicated. This also lets the `operator==` of `std::optional`
take care of the proper comparison instead of defaulting the value to -1
and comparing that.
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 libcamera hex string adaptor specifies and casts each type
specifically to map the size of each type.
This needlessly repeats itself for each type and further more has a bug
with signed integer extension which causes values such as 0x80 to be
printed as 0xffffffffffffff80 instead.
Remove the template specialisations for each type, and unify with a
single templated constructor of the struct hex trait.
Suggested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
The imx708 sensor driver has long been available, especially in raspberry
pi kernels; and the raspberry pi ipa module has had the corresponding
helper class since 2023 (952ef94ed7). The camera sensor properties
database also has an entry for it (2fb0f25019), but the camera sensor
helper class is missing from the common libipa component. So add it, with
the same gain formula present in the raspberry pi ipa module, and the black
level taken from the rpi tuning files.
Handling the raspberry pi specific "wide" / "noir" suffixes is omitted.
They are not present in the camera sensor properties database either.
Signed-off-by: Daniel Scally <dan.scally@ideasonboard.com>
[Add black level, extend commit message.]
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
When a converter or the software ISP is used, output sizes do not equal
input sizes - they notably can be smaller.
Previous to this patch only capture sizes were considered, in some cases
resulting in configs with too small maximum output sizes being selected,
such as 1912x1080 for stream sizes of 1920x1080.
Check that the maximum output sizes are big enough instead, while continuing
to minimize capture sizes.
Closes: https://gitlab.freedesktop.org/camera/libcamera/-/issues/236
Signed-off-by: Robert Mader <robert.mader@collabora.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Acked-by: Umang Jain <uajain@igalia.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Some comments use `text' to quote words, but the unbalanced backticks lead
doxygen to complain. Fix those by using `text`, also add `%` in front of
the words "configuration" and "version" to suppress automatic linking
of the quoted text to the corresponding member functions.
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>
We're going to use a "floating statistics region" to store a full
image Y sum. The VC4 platform actually has no floating region for
this, but we can synthesize such a region as follows in software.
We know that the 15 AGC regions that we do have are arranged to cover
the whole image, and they cannot be changed. Adding up the R, G and B
values here will get us most of the way to Y. But we do also need to
know the most recent colour gains, so code must also be added to
remember the last AWB status.
With this change, algorithms can now look at the first floating region
on both VC4 and PiSP platforms to get a full image Y average value.
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Floating regions are currently unused on the PiSP platform, so we can
use one of them to get an average Y value for the whole image.
If an algorithm (such as the "lux" algorithm) wants a scene-referred
estimate of absolute brightness, then this would be a better choice
than a value from Y histogram as the latter is not invariant to the
metering mode (e.g. centre-weighted, spot etc.). (So note that for the
AGC/AEC algorithm, where the metering mode is relevant, the Y
histogram is the appropriate choice.)
We also fix the loop that copies the hardware's floating region values
into our statistics structure; it was using the wrong limit which was
causing it not to do anything.
A future commit will update the lux algorithm to use this feature.
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
When CPU ISP is asked to apply the CCM matrix
[0 1 0]
[0 0 0]
[0 0 0]
for a format that requires swapping red and blue channels, the resulting
image has a wrong colour. The CCM matrix above should take green from
pixels and make it red. Instead, the image is blue.
The problem is that the lookup tables setup in CPU debayering swaps red
and blue in the lookup tables for red and blue, but not for green. The
colours must be swapped also in the lookup table for green, which this
patch adds.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Using auto variables for simple types reduces readability. Spell out
unsigned int explicitly here, and replace the <= 0 check with a zero
check now that the explicit type shows the value can't be negative.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Umang Jain <uajain@igalia.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Debayering is carried out on `ispWorkerThread_`. When stopping, the queued
work needs to be flushed or cancelled to ensure that the next time it starts,
it won't process stale data. So remove all messages targeting the `Debayer`
object on the worker thread.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Robert Mader <robert.mader@collabora.com>
Update the feature TODO list of libcamerasrc as it hasn't been
updated since its introduction. Following entries have been dropped
since they are supported:
- Implement GstElement::request-new-pad (multi stream)
commit 53a0d80af0 ("gstreamer: Added virtual functions needed to support request pads")
- Add framerate control
- Add framerate negotiation support
commit ccfe0a1af7 ("gstreamer: Provide framerate support for libcamerasrc")
- Add colorimetry support
commit fc9783acc6 ("gstreamer: Provide colorimetry <> ColorSpace mappings")
- Use unique names to select the camera devices
commit 2c93810ec1 ("gst: libcamerasrc: Add camera-name property")
(The property that is set here is fed into CameraManager::get()
eventually, ensuring we can select the camera devices by unique IDs.)
- Add GstVideoMeta support (strides and offsets)
commit 848a3017b8 ("gstreamer: Add GstVideoMeta support")
At the same time, append the buffer importation support entry to
mention the potential usage of memory:DMAbuf, that landed in
gstreamer-1.26.
Signed-off-by: Umang Jain <uajain@igalia.com>
Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
While a default value of 4 buffers appears to be a good default that is
used by other pipelines as well, allowing both higher and lower values
can be desirable, notably for:
1. Video encoding, e.g. encoding multiple buffers in parallel.
2. Clients requesting a single buffer - e.g. in multi-stream scenarios.
Thus allow buffer counts between 1 and 32 buffers - following the default
maximum from vb2 core - while keeping the default to the previous 4.
While on it mark the config as adjusted when appropriate.
Signed-off-by: Robert Mader <robert.mader@collabora.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>
The Simple Pipeline handler supports a variety of hardware with
different capabilities and performances.
To improve performance and reliability of the cameras across the
supported range, increase the number of internal buffers to 4.
This allows lower performance devices more opportunity to process the
frames and increases stability.
Align the Simple Pipeline handler and Soft ISP buffering with the other
hardware based platforms and use 4 internal buffers.
Signed-off-by: Robert Mader <robert.mader@collabora.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>
When retrieving the value from a `ControlValue` usually one of two
things happen: a small, trivially copyable object is returned by
value; or a view into the internal buffer is provided. This is true
for everything except strings, which are returned in `std::string`,
incurring the overhead of string construction.
To guarantee no potentially "expensive" copies, use `std::string_view`
pointing to the internal buffer to return the value. This is similar
to how other array-like types are returned with a `Span<>`.
This is an API break, but its scope is limited to just `properties::Model`.
Bug: https://bugs.libcamera.org/show_bug.cgi?id=256
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>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
In the current code, decompand will only set a curve in the prepare
phase, which will only run after 1-2 frames pass through the FE. This
is fixed by adding an initialValues() member function to the decompand
algorithm, which will be called in the IPA before we start the hardware
streaming.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Don't let applyDecompand() directly change the FE global enables.
Instead pass the global mask into the function and set it in the caller.
This will be needed when a future commit will add setting the decompand
initial values.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
This patch integrates a new decompand algorithm that utilizes the PiSP
FE hardware block available on Raspberry Pi 5. The implementation
enables conversion of companded sensor data into linear format prior to
ISP processing.
Changes include:
- Implementation of decompand logic for controller and pipe interfaces
- Enabling decompand block by "rpi.decompand" in tuning.json
Signed-off-by: Sena Asotani <aso.fam429@gmail.com>
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Tested-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
If the histogram size is non-zero but lower than the number of bins,
yHistValsPerBin is zero and then the AGC processing crashes on division
by it. Let's check yHistValsPerBin for being zero and stop AGC
processing in such a case. The condition also covers the cases where
histogramSize or yHistValsPerBinMod are zero.
Tested-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Hans de Goede <hansg@kernel.org>
Tested-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The result type of the std::accumulate() call is uint64_t; let's use the
same type for the initial value (rather than the default int) to prevent
summing up the values in a different integer type.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Hans de Goede <hansg@kernel.org>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Tested-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The R/G/B sums computed in AWB simple IPA may be zero or perhaps even
negative. Let's make sure the sums are always positive, to prevent
division by zero or completely nonsense results.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Hans de Goede <hansg@kernel.org>
Tested-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The window coordinates passed to SwStatsCpu::setWindow are confusing.
Let's clarify what the coordinates should be.
A source of confusion is that the specified window is relative to the
processed area. Debayering adjusts line pointers for its processed area
and this is what's also passed to stats processing. The window passed
to SwStatsCpu::setWindow should either specify the size of the whole
processed (not image) area, or its cropping in case the stats shouldn't
be gathered over the whole processed area. This patch should clarify
this in the code.
Reviewed-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
Reviewed-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Tested-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
SwStatsCpu::setWindow reduces the window width by the added x-offset, to
prevent exceeding image bounds. But if the window width is smaller than
the x-offset, we get unsigned integer underflow. Fix it by setting the
window width to 0 in such a case.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Hans de Goede <hansg@kernel.org>
Tested-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Use a signalfd to detect `SIGINT` instead of registering a signal handler in
order to remove the `CamApp` singleton. This is better than using a normal
signal handler: a signal can arrive after the `CamApp` object has been destroyed,
leading to a use-after-free. Modifying the `CamApp::app_` in the destructor is
not a good alternative because that would not be async signal safe (unless it
is made atomic).
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>
At the moment the blc code uses 255 as starting blacklevel for sensors
where there is no blacklevel info in the sensor-helper.
There are a number of issues with this:
1. When the first frame is bad (e.g. mostly white) which happens sometimes
the initial blacklevel will be kept leading to a divide by zero problem
in the AGC code (this divide by 0 problem is avoided in the AGC code by
not running the AGC algorithm).
2. Not runnning the AGC algorithm means that the gain/exposure do not
change, which causes the BLC algorithm to not run on the next frames,
so we keep the bad 255 blacklevel which stops AGC from running which
stops BLC from running. Leaving things stuck at a 255 blacklevel
resulting in an unusuable image.
3. Sometimes the auto-blc code leads to an unrealistic high
blacklevel detection which results in lower image quality.
To fix this start with a blacklevel of 16, which is the highest
(4096 / 256) blacklevel used for any sensor listing a blackLevel_
value in the sensor-helper class.
Note 2. could alternatively be fixed by disabling the check for
exposure/gain changes when the blacklevel is unrealistic high,
but that still leaves the other problems.
Signed-off-by: Hans de Goede <hansg@kernel.org>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Run sw-statistics once every 4th frame, instead of every frame. There are
2 reasons for this:
1. There really is no need to have statistics for every frame and only
doing this every 4th frame helps save some CPU time.
2. The generic nature of the simple pipeline-handler, so no information
about possible CSI receiver frame-delays. In combination with the software
ISP often being used with sensors without sensor info in the sensor-helper
code, so no reliable control-delay information means that the software ISP
is prone to AGC oscillation. Skipping statistics gathering also means
skipping running the AGC algorithm slowing it down, avoiding this
oscillation.
Note ideally the AGC oscillation problem would be fixed by adding sensor
metadata support all through the stack so that the exact gain and exposure
used for a specific frame are reliably provided by the sensor metadata.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Tested-by: Milan Zamazal <mzamazal@redhat.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Hans de Goede <hansg@kernel.org>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Generating statistics for every single frame is not really necessary.
However a roundtrip through ipa_->processStats() still need to be done
every frame, even if there are no stats to make the IPA generate metadata
for every frame.
Add a valid flag to the statistics struct to let the IPA know when there
are no statistics for the frame being processed and modify the IPA to
only generate metadata for frames without valid statistics.
This is a preparation patch for skipping statistics generation for some
frames.
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Tested-by: Milan Zamazal <mzamazal@redhat.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
At the moment when the overall image brightness is considered too high
the AGC code will lower the gain all the way down to againMin before
considering lowering the exposure.
What should happen instead is lower the gain no lower than 1.0 and after
that lower the exposure instead of lowering the gain.
Otherwise there might be a heavily overexposed image (e.g. all white)
which then is made less white by a gain < 1.0 which is no good.
When there is no sensor-helper, assume the driver reported default-gain
value is close to a gain of 1.0 .
While at it also remove the weird limitation to only lower the gain
when exposure is set to the maximum. As long as the gain is higher
than the default gain, the gain should be lowered first.
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Tested-by: Milan Zamazal <mzamazal@redhat.com>
Tested-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Isaac Scott <isaac.scott@ideasonboard.com>
Signed-off-by: Hans de Goede <hansg@kernel.org>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Currently the virtual pipeline generates the images synchronously. This is not
ideal because it blocks the camera manager's internal thread, and because its
behaviour is different from other existing pipeline handlers, all of which
complete requests asynchronously.
So move the image generation to a separate thread by deriving `VirtualCameraData`
from `Thread`, as well as `Object` and using the existing asynchronous signal
and method call mechanism.
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
While optional, libunwind integration is enabled when meson finds it without
having a way to disable it. This is the case for Debian where libunwind is
installed by build dependencies. Since we want to reduce dependencies on
libunwind in Debian due to several issues with it[0], we need an option to
control its activation.
[0]: https://bugs.debian.org/1093688
Signed-off-by: Dylan Aïssi <dylan.aissi@collabora.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
[Longer commit message lines, reworded meson option description.]
Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
The code was only allowing the exposure mode to be updated when both
exposure/gain were in manual mode, which is a mistake. It needs to be
updatable precisely when the auto modes are enabled.
The fix is to ignore the enabled/disabled status of AEC/AGC, matching
our other controls (metering mode, constraint mode etc.). While there
might be a debate to be had about what the actual behaviour of all
these controls should be, for the time being we'll just match
everything else.
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Software ISP performs performance measurement on certain part of initial
frames. Let's make this range configurable.
For this purpose, this patch introduces new configuration options
software_isp.measure.skip and software_isp.measure.number. Setting the
latter one to 0 disables the measurement.
Instead of the last frame, the class member and its configuration
specify the number of frames to measure. This is easier to use for
users and doesn't require to adjust two configuration parameters when
the number of the initially skipped frames is changed.
The patch also changes the names of the class members to make them more
accurate.
Completes software ISP TODO #7.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
On some platforms, working directly on the input buffer is very slow due
to disabled caching. This is why we copy the input buffer into standard
(cached) memory. This is an unnecessary overhead on platforms with
cached buffers.
Let's make input buffer copying configurable. The default is still
copying, as its overhead is much lower than contingent operations on
non-cached memory. Ideally, we should improve this in future to set the
default to non-copying if we can be sure under observable circumstances
that we are working with cached buffers.
Completes software ISP TODO #6.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
This patch allows enabling or disabling software ISP via config file in
addition to compile time. This can be useful for software ISP testing
on various platforms as well as for overriding the defaults in case the
defaults don't work well (e.g. hardware ISP may or may not work on
i.MX8MP depending on the kernel and libcamera patches present in the
given system).
The configuration is specified as follows:
configuration:
pipelines:
simple:
supported_devices:
- driver: DRIVER-NAME
software_isp: BOOLEAN
- ...
For example:
configuration:
pipelines:
simple:
supported_devices:
- driver: mxc-isi
software_isp: true
The overall configuration of enabling or disabling software ISP may get
dropped in future but this patch is still useful in the meantime.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
