For debugging purposes, threads can be assigned a name, which eases
distinguishing between them in e.g. htop or gdb. This uses a
Linux-specific API for now which is limited to 15 characters (+ null
terminator), so truncation is done and names for existing thread
instantiations were chosen to be consise.
[Kieran: Apply checkstyle suggestions, rebase on proxy rework]
Signed-off-by: Schulz, Andreas <andreas.schulz2@karlstorz.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The DelayedControls class works around a limitation of the V4L2 controls
API by assigning to controls that modify the limits of other controls a
'priority' flag.
'Priority' controls are written to the device before others to make sure
the limits of dependent controls are correctly updated.
A typical example of a priority control is VBLANK, whose value changes the
limits of the EXPOSURE control. This doesn't apply to a specific hardware
platform but to all V4L2 sensors.
The RkISP1 pipeline handler doesn't mark VBLANK as a priority control, an
issue which might result in the exposure being clamped to an outdated frame
length.
Fix the rkisp1 pipeline by marking VBLANK as a priority control.
Fixes: f72c76eb6e ("rkisp1: Honor the FrameDurationLimits control")
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
GstPad is the link point where the buffers are pushed/pulled in the
gstreamer pipeline hence, it makes more sense to associate and
track GstBuffer(s) using GstPad rather than libcamera::Stream in
RequestWrap.
No functional changes intended as there is only one stream per pad
configuration followed in libcamerasrc.
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>
Instead of trying to figure out which libcamera::Stream, the GstPad
is configured with (from the libcamera pool), associate the Stream
with the GstPad directly. The association can be set using
gst_pad_set_element_private().
Add the gst_libcamera_pad_set_stream() helper to associate the stream
with the pad. Additionally, modify the gst_libcamera_pad_get_stream()
to use to the getter helper gst_pad_get_element_private().
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>
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>
`std::{begin,end}()` support C-style arrays, thus there is no need for
a second overload. The only reason it is currently needed is that the
trailing return type of the first overload uses `iterable.begin()`, which
leads to a substitution failure, so that overload is not considered.
So remove the array overload, and let CTAD deduce the `Base` template
parameter of `enumerate_adapter`, which will make things work for
arrays as well.
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>
