The terms "shutter" and "shutter speed" are used through libcamera to
mean "exposure time". This is confusing, both due to "speed" being used
as "time" while it should be the inverse (i.e. a maximum speed should
correspond to the minimum time), and due to "shutter speed" and
"exposure time" being used in different places with the same meaning.
To improve clarity of the code base and the documentation, use "exposure
time" consistently to replace "shutter speed".
This rename highlighted another vocabulary issue in libcamera. The
ExposureModeHelper::splitExposure() function used to document that it
splits "exposure time into shutter time and gain". It has been reworded
to "split exposure into exposure time and gain". That is not entirely
satisfactory, as "exposure" has a defined meaning in photography (see
https://en.wikipedia.org/wiki/Exposure_(photography)) that is not
expressed as a duration. This issue if left to be addressed separately.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Add a default tuning file for Sony IMX327 sensor. This tuning file
is a copy of the IMX290 and is added to make the IMX327 sensor
just work without hassle. Note the extra description field to
clarify this is just an interim tuning file untill someone
provides a proper one.
Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@gmail.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Add a default tuning file for Sony IMX462 sensor. This tuning file
is a copy of the IMX290 and is added to make the IMX462 sensor
just work without hassle. Note the extra description field to
clarify this is just an interim tuning file untill someone
provides a proper one.
Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@gmail.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
The IMX327 sensor is largely compatible with the already supported
Sony IMX290 so we can reuse the same helpers for the analogue gain
conversion functions.
Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@gmail.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
The sensor is largely compatible with the already supported
Sony IMX290 so we can reuse the same helpers for the analogue
gain conversion functions.
Signed-off-by: Geoffrey Van Landeghem <geoffrey.vl@gmail.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Add a vendor control rpi::ScalerCrops that is analogous to the current
core::ScalerCrop, but can apply a different crop to each configured
stream.
This control takes a span of Rectangle structures - the order of
rectangles must match the order of streams configured by the application.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
In the case of an AWB search failure, the current algorithm logic will
return a point on the CT curve closest to where the search finisned.
This can be quite undesirable. Instead, add some bias params to the AWB
algorithm which will direct the search to a set CT value in the case
where statistics become unreliable causing the search to fail.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
As described in the coding style document, libcamera favours <cmath>
over <math.h>. Replace the last few occurrences of the latter with the
former in the Raspberry Pi IPA and adapt the code accordingly. In some
cases, the <math.h> include is simply dropped as it isn't needed.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
As explained in the coding style document, usage of std::abs() is
preferred over abs() or fabs() as it picks the correct function based on
the argument type. Replace calls to abs() and fabs() with std::abs() in
the Raspberry Pi algorithms.
This fixes a reported warning from clang:
../src/ipa/rpi/controller/rpi/awb.cpp:508:6: error: using integer absolute value function 'abs' when argument is of floating point type [-Werror,-Wabsolute-value]
if (abs(denominator) > eps) {
^
../src/ipa/rpi/controller/rpi/awb.cpp:508:6: note: use function 'std::abs' instead
if (abs(denominator) > eps) {
^~~
std::abs
Reported-by: Maarten Lankhorst <dev@lankhorst.se>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Tested-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
When a user is taking control of exposure and gain, setting them
manually, we set the AGC "stable region" to zero. This means that any
user changes, however small, will be applied, and they won't be
regarded as "too small to bother with".
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The libcamera_generated_ipa_headers variable, containing the list of
generated IPA headers, is listed in the sources of IPA modules, as well
as IPA tests. This was done to ensure that the modules and tests get
rebuilt when the generate IPA headers change. However, the dependency is
already handled through the libcamera_private dependency object,
specified for all those modules and tests. There's no need to list the
IPA generated headers as sources. Drop them.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Unlike in C where they have been standardized since C99, variable-length
arrays in C++ are an extension supported by gcc and clang. Clang started
warning about this with -Wall in version 18:
src/libcamera/ipc_unixsocket.cpp:250:11: error: variable length arrays in C++ are a Clang extension [-Werror,-Wvla-cxx-extension]
250 | char buf[CMSG_SPACE(num * sizeof(uint32_t))];
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
One simple option is to disable the warning. However, usage of VLAs in
C++ is discouraged by some, usually due to security reasons, based on
the rationale that developers are often unaware of unintentional use of
VLAs and how they may affect the security of the code when the array
size is not properly validated.
This rationale may sound dubious, as the most commonly proposed fix is
to replace VLAs with vectors (or just arrays dynamically allocated with
new() wrapped in unique pointers), without adding any size validation.
This will not produce much better results. However, keeping the VLA
warning and converting the code to dynamic allocation may still be
slightly better, as it can prompt developers to notice VLAs and check if
size validation is required.
For these reasons, convert all VLAs to std::vector. Most of the VLAs
don't need extra size validation, as the size is bound through different
constraints (e.g. image width for line buffers). An arguable exception
may be the buffers in IPCUnixSocket::sendData() and
IPCUnixSocket::recvData() as the number of fds is not bound-checked
locally, but we will run out of file descriptors before we could
overflow the buffer size calculation.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Acked-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Multiple local functions are defined in the global namespace without the
static keyword. This compiles fine for now, but will cause a missing
declaration warning when we enable them. To prepare for that, move the
function declaration to an anonymous namespace.
While at it, for consistency, include an existing static function in the
namespace and drop the static keyword.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Add support for the following HDR modes in the Raspberry Pi IPA:
- Night mode
- Single exposure mode
- Multi-exposure (merged and unmerged)
The algorithm is updated to expect the HDR short channel to meter
explicitly for highlights. This means that it will not in general
under-expose the short channel more than is actually necessary.
When images don't have much saturation, it's good to detect this so
that some of the boost we want to apply to the dark areas can be
implemented as regular gain. This means we can then adjust the tone
curve less, leading to less flat looking images.
The impact on the HDR algorithm is then that this determines how we
build tonemaps dynamically. The highlights are more-or-less correct
now, so we have to build a power-type curve that gives us the
appropriately configured targets in the lower part of the histogram.
We allow the tuning file to supply the maximum spatial gain value,
rather than the whole curve (though it can supply this if it
wants). Some parameter defaults are tweaked to be generally better
across the range of our cameras.
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Acked-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The PipelineHandlerFactoryBase class has a name that is propagated to
the PipelineHandler instance it creates.
In present implementation, this name comes from the
REGISTER_PIPELINE_HANDLER registration macro. It corresponds to the
stringified name of the PipelineHandler derived class. Therefore,
PipelineHandler factories and instances names can be quite long such as
"PipelineHandlerRkISP1".
A libcamera user may have to explicitly refer to a PipelineHandler name
for configuration purpose: one usage of the name can be to define a
pipeline handlers match list and their priorities. It is desired, for
user convenience, to use a short name to designate a pipeline handler.
Reusing the short pipeline names already defined in the meson option
files is an existing and consistent way of naming pipelines.
This change adds an explicit name parameter to the
REGISTER_PIPELINE_HANDLER registration macro. That parameter is used to
define the name of a pipeline handler factory, instead of the current
pipeline handler class name.
Each pipeline registration is updated accordingly. The short name
assigned corresponds to the pipeline directory name in the source tree.
It is consistent with pipelines names used in meson.
Changing the pipeline name has an impact on the IPA modules: each module
defines a IPAModuleInfo structure. This structure has a pipelineName
member defining the pipeline handler name it shall match with.
Therefore, each internal IPA module definition has to be changed to have
its IPAModuleInfo pipelineName name updated with the short pipeline
handler name.
In addition to this pipelineName member, the IPAModuleInfo structure
also has a name member, associated to the IPA module name. Having
renamed the pipelines to a short name, the pipeline name and the IPA
module names of the IPAModuleInfo structure are the same: for in-tree
IPA, they correspond to the respective pipeline and IPA subdirectories
in the source tree. However the IPA name could be different, for
instance with a close source IPA implementation built out-of-tree. Thus,
it makes sense to keep the IPA name in that structure, as the 2
definitions may not always be redundant.
Signed-off-by: Julien Vuillaumier <julien.vuillaumier@nxp.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
[Kieran: Adjust for clang-format style fix, reformat commitmsg]
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Source files in libcamera start by a comment block header, which
includes the file name and a one-line description of the file contents.
While the latter is useful to get a quick overview of the file contents
at a glance, the former is mostly a source of inconvenience. The name in
the comments can easily get out of sync with the file name when files
are renamed, and copy & paste during development have often lead to
incorrect names being used to start with.
Readers of the source code are expected to know which file they're
looking it. Drop the file name from the header comment blocks in all
remaining locations that were not caught by the automated script as they
are out of sync with the file name.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
The maximum shutter speed calculation in the cam-helper relied on
the frame duration limits being correctly set in the cam-helper's mode
structure. This was not the case on first startup, so the maximum
shutter speed reported back via the ControlInfo was incorrect.
Fix this by setting up the camera mode in the cam-helper before querying
for the max shutter value.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Source files in libcamera start by a comment block header, which
includes the file name and a one-line description of the file contents.
While the latter is useful to get a quick overview of the file contents
at a glance, the former is mostly a source of inconvenience. The name in
the comments can easily get out of sync with the file name when files
are renamed, and copy & paste during development have often lead to
incorrect names being used to start with.
Readers of the source code are expected to know which file they're
looking it. Drop the file name from the header comment block.
The change was generated with the following script:
----------------------------------------
dirs="include/libcamera src test utils"
declare -rA patterns=(
['c']=' \* '
['cpp']=' \* '
['h']=' \* '
['py']='# '
['sh']='# '
)
for ext in ${!patterns[@]} ; do
files=$(for dir in $dirs ; do find $dir -name "*.${ext}" ; done)
pattern=${patterns[${ext}]}
for file in $files ; do
name=$(basename ${file})
sed -i "s/^\(${pattern}\)${name} - /\1/" "$file"
done
done
----------------------------------------
This misses several files that are out of sync with the comment block
header. Those will be addressed separately and manually.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
All the Raspberry Pi official camera tuning files are updated for
HDR. As stated previously, there is no mechanism in the hardware for
combining images so all this does is enable multi-channel AGC to
produce short and long exposure frames. It will be up to the
application to deal with them.
The changes are identical in every tuning file.
1. The existing AGC tuning is duplicated twice so that we have 3 AGC
channels.
2. The first is left alone (the default AGC channel), the second is
tweaked to under-expose significantly (ev -3) and the final one is
tweaked to over-exposure slightly (ev +0.5)
3. Control parameters are provided to the "rpi.hdr" algorithm to
associate these AGC channels correctly with the HDR modes.
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Sufficient plumbing is added so that the HDR mode control can be used
to engage HDR modes on platforms that support them. On the vc4
platform, this allows multi-channel AGC to run, though there is no
image merging.
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>
Implement the StatsOutputEnable control for the VC4 IPA. When set,
this outputs the ISP statistics as a uint8_t span through the Bcm2835StatsOutput
metadata control.
To get this working, IpaBase::libcameraMetadata_ is moved from a private
to a protected member variable. This makes it accessable to the VC4
derived IPA class.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
This allows the IPA to get reasonable default colour gains before AWB
has run. This is particularly important on the PiSP platform where
these numbers are helpful in programming the Front End statistics
block in advance.
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>
This allows the IPA to discover the correct black level values even
before any frames have been processed. This is important on the PiSP
platform where the front end black level blocks must be programmed in
advance.
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>
Advertise hardware constraints on the pixel processing rate through the
Controller::HardwareConfig structure. When calculating the minimum line
length during a configure() operation, ensure that we don't exceed this
constraint.
If we do exceed the hardware constraints, increase the modes's minimum
line length so the pixel processing rate falls below the hardware limit.
If this is not possible, throw a loud error message in the logs.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Meson uses tags to sort installed files in categories, and makes it
possible to install a subset of the files using the '--tags' argument to
'meson install'. This is typically used by distributions to split the
runtime, development and documentation files into separate packages.
By default, meson tries to guess the correct tag for installed files,
but can't always do so properly. Mark the install targets that meson
can't guess with the correct install_tag.
As the feature has been introduced in meson 0.60, bump the minimum meson
version. The latest LTS release of all major distributions that
libcamera currently targets ship a recent enough meson version.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
We make a few small improvements to the code:
* The arrayToSet method is prevented from overwriting the end of the
array if there are too many values in the input table. If you supply
a table, it will force you to put the correct number of elements in
it.
* The arrayToSet and setStrength member functions are turned into
static functions. (There may be a different public setStrength
member function in future.)
* When no tables at all are given, the configuration is flagged as
being disabled, so that we can avoid copying tables full of zeroes
around. As a consequence, the pipeline handler too will disable this
hardware block rather than run it needlessly. (Note that the tuning
tool will put in a completely empty "rpi.cac" block if no CAC tuning
images are supplied, benefiting from this behaviour.)
* The initialise member function is removed as it does nothing.
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The recent change where time-filtering is done before sorting out the
digital gain means that the target exposure without digital gain is no
longer set, breaking the 'AeLocked' calculation.
We can use the regular (full) target exposure instead.
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Fixes: 84b6327789 ("ipa: rpi: agc: Filter exposures before dealing with digital gain")
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Acked-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Label draft controls and properties through the "draft" vendor tag
and deprecate the existing "draft: true" mechanism. This uses the new
vendor tags mechanism to place draft controls in the same
libcamera::controls::draft namespace and provide a defined control id
range for these controls. This requires moving all draft controls from
control_ids.yaml to control_ids_draft.yaml.
One breaking change in this commit is that draft control ids also move
to the libcamera::controls::draft namespace from the existing
libcamera::controls namespace. This is desirable to avoid API breakages
when adding new libcamera controls. So, for example, the use of
controls::NOISE_REDUCTION_MODE will need to be replaced with
controls::draft::NOISE_REDUCTION_MODE.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
The algorithm computes R/G and B/G colour ratio statistics which we
should not allow to go to zero because there is clearly no gain you
could apply to R or B to equalise them. Instead flag such regions as
having "insufficient data" in the normal manner.
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
We need to be able to do things like enable/disable AGC for all the
channels, so most of the AGC controls are updated to be applied to all
channels. There are a couple of exceptions, such as setting explicit
shutter/gain values, which apply only to channel 0.
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
AWB writes this out during prepare, so we may as well read it in AGC
prepare as well. Reading it in process is wrong on the PiSP platform
because process runs before prepare, so the AWB status won't be there
(on vc4 it made no difference).
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Since noise control handling differs between the VC4 and PiSP IPAs,
move the current denoise control handler from ipa base into the vc4 IPA
derived class.
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: David Plowman <david.plowman@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
"Fast desaturation" is a technique that can help the AGC algorithm to
desaturate images more quickly when they are very
over-exposed. However, it uses digital gain to do this which can
confuse our HDR techniques. Therefore make it optional.
Signed-off-by: David Plowman <david.plowman@raspberrypi.com>
Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
Reviewed-by: Naushir Patuck <naush@raspberrypi.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Laurent Pinchart
Geoffrey Van Landeghem
Naushir Patuck
Daniel Scally
Dave Stevenson
will whang
David Plowman
Stefan Klug
Paul Elder
Julien Vuillaumier
Nick Hollinghurst
Lee Jackson
Jacopo Mondi