| Age | Commit message (Collapse) | Author |
|---|
|
Using a const lvalue reference to `std::function<>` is not ideal
because it forces a copy to happen. Use an rvalue reference and
`std::move()` to avoid that.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
|
|
Wrap the `LogCategory` pointers in `std::unique_ptr` to avoid
the need for manual deletion in the destructor.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
|
|
Log categories may be added from any thread, so it is important to
synchronize access to the `Logger::categories_` list between its two
users: category creation (by LogCategory::create(), which calls
Logger::findCategory() and Logger::registerCategory()); and log level
setting (by Logger::logSetLevel()).
The LogCategory::create() function uses a mutex to serialize category
creation, but Logger::logSetLevel() can access `Logger::categories_`
concurrently without any protection. To fix the issue, move the mutex to
the Logger class, and use it to protect all accesses to the categories
list. This requires moving all the logic of LogCategory::create() to a
new Logger::findOrCreateCategory() function that combines both
Logger::findCategory() and Logger::registerCategory() in order to make
the two operations exacute atomically.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Use move construction to essentially pass through the string
returned by `Loggable::logPrefix()` to avoid an unnecessary copy.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
|
|
Use `std::string_view` to avoid some largely unnecessary copies, and
to make string comparisong potentially faster by eliminating repeated
`strlen()` calls.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
|
|
The severity of a log category may be changed from a different thread,
so it is important to ensure that the reads and writes happen atomically.
Using `std::memory_order_relaxed` should not introduce any synchronization
overhead, it should only guarantee that the operation itself is atomic.
Secondly, inline `LogCategory::setSeverity()`, as it is merely an
assignment, so going through a DSO call is a big pessimization.
`LogCategory` is not part of the public API, so this change has
no external effects.
Thirdly, assert that the atomic variable is lock free so as to ensure
it won't silently fall back to libatomic (or similar) on any platform.
If this assertion fails, this needs to be revisited.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
|
|
It is a short function that can be merged into the constructor with
essentially no change in observable behaviour, so do that.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
|
|
Use the `std::from_chars()` function from `<charconv>` to
parse the integral log level instead of `strtoul` as it
provides an easier to use interface and better type safety.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
|
|
C++17 guarantees move and copy elision in certain cases,
such as when returning a prvalue of the same type as the
return type of the function.
This is what the `_log()` functions do, thus there is no need
for the move constructor, so remove it. Furthermore, do not
just remove the implementation, but instead delete it as well.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Add support to rkisp1 for controlling the framerate via the
FrameDurationLimits control.
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
The configured line duration of the sensor is used frequently throughout
the AGC implementation.
It's available in the IPA context through the rather long:
context.configuration.sensor.lineDuration
Take a copy of the lineDuration early in the call and replace the two
current usages of the reference with the shorter copy to manage line
length and ease readibility.
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
|
|
The IPA calculates and reports the FrameDurationLimits to applications
by configuring the ControlInfo accordingly during
IPARkISP1::updateControls()
We later need to know these limits during Agc::configure() for
initialising the ActiveState of the AGC implementation with the limits.
Store the FrameDurationLimits ControlInfo in the ControlInfoMap which is
now present in the IPAContext so that it is commonly available for the
AGC algorithm, removing the 'todo' accordingly.
Signed-off-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Umang Jain <umang.jain@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
|
|
The clang formatter removes spaces in places of empty expressions in for
loops. For example, it changes
for (init; condition; )
to
for (init; condition;)
libcamera currently uses both the styles and we should use only one of
them for consistency. Since there is apparently no option to override
the formatter behavior (see
https://clang.llvm.org/docs/ClangFormatStyleOptions.html), let's remove
the extra spaces to make the formatter happy.
Signed-off-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
A base class with virtual functions and a non-virtual public destructor
is prone to undefined behaviourif deleted from a pointer to the base.
Enable the -Wnon-virtual-dtor warning to report those issues.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Milan Zamazal <mzamazal@redhat.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
The AwbStats structure has virtual functions but a publicly accessible
non-virtual destructors. This can cause undefined behaviour if deleting
a derived class instance through a pointer to the base AwbStats class.
The problem is theoretical only as no code in libcamera is expected to
perform such deletion, but compilers can't know that and will emit a
warning if the -Wnon-virtual-dtor option is enabled.
Fixing this can be done by declaring a virtual public destructor in the
AwbStats class. A more efficient alternative is to declare a protected
non-virtual destructor, ensuring that instances can't be deleted through
a pointer to the base class. Do so, and mark the derived RkISP1AwbStats
as final to avoid the same warning.
Fixes: 6f663990a0f7 ("libipa: Add AWB algorithm base class")
Reported-by: Milan Zamazal <mzamazal@redhat.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
Tested-by: Milan Zamazal <mzamazal@redhat.com>
|
|
Fix the order of the rgbMeans vector that got broken accidentally
during refactoring. As there is currently no way to enable rgb mode at
runtime it went unnoticed.
Fixes: 29892f1c56c6 ("ipa: libipa: colour: Use the RGB class to model RGB values")
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>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
The doxygen \todo directive doesn't need to be followed by a colon. Drop
it. While at it, turn one 'todo:' into '\todo'.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
AWB is an abbreviation, capitalize it in comments and log messages for
consistency.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
When statistics are missing we can't meaningfully calculate the RGB
means. Move their calculation after checking if stats are available.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
Fix the top-level file description to mention the file contains an AWB
grey world implementation, not a base class, and fix a grammar mistake
in a documentation block.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
Drop unneeded headers and add missing ones.
The yaml_parser.h header is dropped from awb_grey.h as the classes it
provides are only used in virtual functions defined by the base class,
so any required definitions are guaranteed to be available already.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
The convention in libcamera is not to prefix getters with a 'get'
prefix. Rename the AwbStats::getRGBMeans() function accordingly.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
The lux value can never be negative. Pass it as an unsigned int.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
The AwbStats documentation incorrectly references pipeline handlers when
it means IPA modules. Fix it.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
All locations but one spell 'grey' instead of 'gray'. Fix the outlier.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
Function names are followed by parentheses in doxygen documentation
blocks as convention in libcamera. Add missing parentheses in the
AwbAlgorithm documentation.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
AWB is an abbreviation, capitalize it in comments and log messages for
consistency.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
Sort the documentation of the class members in the same order as the
member declaration in the class definition, as is customary in
libcamera.
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
|
The original code used to specify the probabilities in log space and
scaled for the RaspberryPi hardware with 192 AWB measurement points.
This is reasonable as the whole algorithm makes use of unitless numbers
to prefer some colour temperatures based on a lux level. These numbers
are then hand tuned with the specific device in mind.
This has two shortcomings:
1. The linear interpolation of PWLs in log space is mathematically
incorrect. The outcome might still be ok, as both spaces (log and
linear) are monotonic, but it is still not "right".
2. Having unitless numbers gets more error prone when we try to
harmonize the behavior over multiple platforms.
Change the algorithm to interpret the numbers as being in linear space.
This makes the interpolation mathematically correct at the expense of a
few log operations.
To account for that change, update the numbers in the tuning example
file with the linear counterparts scaled to one AWB zone measurement.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
Logging every search step is too verbose even with debug messages
enabled and it hides the more important messages (min max values of
errors and likelihoods). Remove the debug messages in a separate commit,
so that it can easily be reverted if needed.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
When tuning the AWB algorithm it is more helpful to get a feeling for
the value ranges than to get verbose output of every single step. Add a
small utility class to track the limits and log them.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
By normalizing the referenceY value to 1 (which is the usual range for
Y) in the tuning file, the bins_ value is no longer needed. Remove it.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
Now that the lux module is available, add it to the rkisp1 tuner.
While at it, sort the imports correctly.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
For the lux algorithm, reference values get calculated based on a tuning
image taken at a known lux level. The reference data contains the mean Y
of the image, lux level, exposure time, gain and aperture. This module
calculates these values for insertion into the tuning file.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
The colour gains are initialized with a default value of 1. Improve that
by querying the auto white balance algorithm for the gains for a default
colour temperature. This is still not based on measurements, but it is
still better than the current implementation. If the algorithm doesn't
implement mapping from colour temperature to gains, it will internally
fallback to 1.0.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
Now that libipa contains a bayes AWB algorithm, add it as supported
algorithm to the rkisp1 ipa.
The decision between the grey world algorithm and the bayesian is done
based on the "algorithm" property of the "Awb" algorithm in the tuning
file. If the lux value in the frameContext is set by the Lux algorithm
it is taken into account. If the lux value is 0 the prior likelihood
estimation gets ignored in the AWB calculations.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
The bayesian AWB algorithm is an AWB algorithm that takes prior
probabilities for a given light source dependent on the current lux
level into account.
The biggest improvement compared to the grey world model comes from the
search of the ideal white point on the CT curve. The algorithm walks the
CT curve to minimize the colour error for a given statistics. After the
minimium is found it additionally tries to search the area around that
spot and also off the curve. So even without defined prior probabilities
this algorithm provides much better results than the grey world
algorithm.
The logic for this code was taken from the RaspberryPi implementation.
The logic was only minimally adjusted for usage with the rkisp1 and a
few things were left out (see doxygen doc for the AwbBayes class). The
code is refactored to better fit the libcamera code style and to make
use of the syntactic sugar provided by the Interpolator and Vector
classes.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
To support the bayesian AWB algorithm in libtuning, the necessary data
needs to be collected and written to the tuning file.
Extend libtuning to calculate and output that additional data.
Prior probabilities and AwbModes are manually specified and not
calculated in the tuning process. Add sample values from the RaspberryPi
tuning files to the example config file.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
|
|
Now that libipa contains a grey world algorithm, use that.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
Move the calculation of the RGB means into an own function for better
code clarity. This commit doesn't contain any functional changes.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
Add the grey world algorithm that is currently used in rkisp1 to libipa.
No changes in functionality were made.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
|
|
The AWB modes are specified in the libcamera core controls. It is
therefore quite likely that every AWB algorithm will implement them. Add
helper functions for parsing and storing the configured modes in the
AwbAlgorithm base class.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
Add a class to provide a generic interface for auto white balance
algorithms. Concrete AWB algorithms are expected to subclass the
AwbAlgorithm class to implement their functionality.
IPAs are expected to subclass the AwbStats class and implement the
necessary functions to give the algorithm access to the hardware
specific statistics data.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
|
|
Sometimes it is necessary to clear a pwl. Add a function for that.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
The only way to access the internal data of an Interpolator is through
the getInterpolated() method. Sometimes it is necessary to to access the
internal data directly to iterate over it. Add an accessor for that.
While at it, remove a line break from the doxygen documentation for
interpolate() so that doxygen is able to correctly match the function.
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Paul Elder <paul.elder@ideasonboard.com>
Reviewed-by: Daniel Scally <dan.scally@ideasonboard.com>
|
|
In f5da05ed03ee ("libcamera: pipeline: Move tuning file override
handling to IPAProxy") a incorrect comparison slipped through. That
broke the handling of LIBCAMERA_<NAME>_TUNING_FILE. Fix that.
Fixes: f5da05ed03ee ("libcamera: pipeline: Move tuning file override handling to IPAProxy")
Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Check if the buffer has a fence before making any modifications because
otherwise it is possible for `Request::addBuffer()` to return an error code
while at the same time the buffer - for all intents and purposes - is added
to the request.
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com>
|
|
Fix a copy/paste/replace typo. Without this fix, the last element (4th)
is always zero.
Signed-off-by: Nerijus Bendžiūnas <nerijus.bendziunas@gmail.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
|
|
Drop trailing whitespace introduced by mistake.
Fixes: 0e0e32b1890c ("ipa: rkisp1: algorithms: agc: Check for correct stats type")
Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
Reviewed-by: Stefan Klug <stefan.klug@ideasonboard.com>
|
