[libcamera-devel] [PATCH v4 0/9] Add DPF tuning support for RkISP1
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Aug 16 03:54:05 CEST 2022
Hello,
It turned out to be one of those days where you're about to push a
series of patches, and the last minute compilation breakage with an
obscure combination of compiler version and optimization flags turn 3
patches into 9.
So, compared to v3, this series starts with 6 patches that add support
for 8-bit integer parsing to the YamlObject class. That could have been
done in a single patch (5/9), if it wasn't for the fact that the
corresponding unit test uncovered an issue with bounds checking when
parsing 16-bit integers, as shown by a new unit test (3/9) and then
fixed (4/9). I wasn't happy with the resulting code duplication, which I
fixed for the unit tests (1/9 and 2/9) and the YamlObject class (6/9).
After that, it's "just" a new version of the DPF patches. Patch 9/9 now
uses the uint8_t version of the YamlObject::getList() function for the
domain filter coefficients, which fixes the aforementioned compilation
warning with gcc 12.1.0 in -O3 mode (OK, it's not *that* obscure).
If anyone is curious about the gcc warning, which I believe is a false
positive and a compiler bug, here's a minimal test case:
--------
#include <algorithm>
#include <vector>
extern unsigned char dst[6];
int foo(const std::vector<unsigned short> &src)
{
if (src.size() != 5 && src.size() != 6)
return 1;
std::copy_n(src.begin(), src.size(), std::begin(dst));
return 0;
}
--------
$ g++-12.1.0 -Wall -Werror -std=c++17 -O3 -o stringop-overflow.o -c stringop-overflow.cpp
In file included from /usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/algorithm:60,
from stringop-overflow.cpp:1:
In static member function ‘static _OI std::__copy_move<false, false, std::random_access_iterator_tag>::__copy_m(_II, _II, _OI) [with _II = const short unsigned int*; _OI = unsigned char*]’,
inlined from ‘_OI std::__copy_move_a2(_II, _II, _OI) [with bool _IsMove = false; _II = const short unsigned int*; _OI = unsigned char*]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algobase.h:495:30,
inlined from ‘_OI std::__copy_move_a1(_II, _II, _OI) [with bool _IsMove = false; _II = const short unsigned int*; _OI = unsigned char*]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algobase.h:522:42,
inlined from ‘_OI std::__copy_move_a(_II, _II, _OI) [with bool _IsMove = false; _II = __gnu_cxx::__normal_iterator<const short unsigned int*, vector<short unsigned int> >; _OI = unsigned char*]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algobase.h:529:31,
inlined from ‘_OI std::copy(_II, _II, _OI) [with _II = __gnu_cxx::__normal_iterator<const short unsigned int*, vector<short unsigned int> >; _OI = unsigned char*]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algobase.h:620:7,
inlined from ‘_OutputIterator std::__copy_n(_RandomAccessIterator, _Size, _OutputIterator, random_access_iterator_tag) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<const short unsigned int*, vector<short unsigned int> >; _Size = long unsigned int; _OutputIterator = unsigned char*]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algo.h:728:23,
inlined from ‘_OIter std::copy_n(_IIter, _Size, _OIter) [with _IIter = __gnu_cxx::__normal_iterator<const short unsigned int*, vector<short unsigned int> >; _Size = long unsigned int; _OIter = unsigned char*]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algo.h:760:27,
inlined from ‘int foo(const std::vector<short unsigned int>&)’ at stringop-overflow.cpp:11:13:
/usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algobase.h:385:25: error: writing 8 bytes into a region of size 6 [-Werror=stringop-overflow=]
385 | *__result = *__first;
| ~~~~~~~~~~^~~~~~~~~~
stringop-overflow.cpp: In function ‘int foo(const std::vector<short unsigned int>&)’:
stringop-overflow.cpp:4:22: note: destination object ‘dst’ of size 6
4 | extern unsigned char dst[6];
| ^~~
In static member function ‘static _OI std::__copy_move<false, false, std::random_access_iterator_tag>::__copy_m(_II, _II, _OI) [with _II = const short unsigned int*; _OI = unsigned char*]’,
inlined from ‘_OI std::__copy_move_a2(_II, _II, _OI) [with bool _IsMove = false; _II = const short unsigned int*; _OI = unsigned char*]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algobase.h:495:30,
inlined from ‘_OI std::__copy_move_a1(_II, _II, _OI) [with bool _IsMove = false; _II = const short unsigned int*; _OI = unsigned char*]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algobase.h:522:42,
inlined from ‘_OI std::__copy_move_a(_II, _II, _OI) [with bool _IsMove = false; _II = __gnu_cxx::__normal_iterator<const short unsigned int*, vector<short unsigned int> >; _OI = unsigned char*]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algobase.h:529:31,
inlined from ‘_OI std::copy(_II, _II, _OI) [with _II = __gnu_cxx::__normal_iterator<const short unsigned int*, vector<short unsigned int> >; _OI = unsigned char*]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algobase.h:620:7,
inlined from ‘_OutputIterator std::__copy_n(_RandomAccessIterator, _Size, _OutputIterator, random_access_iterator_tag) [with _RandomAccessIterator = __gnu_cxx::__normal_iterator<const short unsigned int*, vector<short unsigned int> >; _Size = long unsigned int; _OutputIterator = unsigned char*]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algo.h:728:23,
inlined from ‘_OIter std::copy_n(_IIter, _Size, _OIter) [with _IIter = __gnu_cxx::__normal_iterator<const short unsigned int*, vector<short unsigned int> >; _Size = long unsigned int; _OIter = unsigned char*]’ at /usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algo.h:760:27,
inlined from ‘int foo(const std::vector<short unsigned int>&)’ at stringop-overflow.cpp:11:13:
/usr/lib/gcc/x86_64-pc-linux-gnu/12.1.0/include/g++-v12/bits/stl_algobase.h:385:25: error: writing 1 byte into a region of size 0 [-Werror=stringop-overflow=]
385 | *__result = *__first;
| ~~~~~~~~~~^~~~~~~~~~
stringop-overflow.cpp: In function ‘int foo(const std::vector<short unsigned int>&)’:
stringop-overflow.cpp:4:22: note: at offset [22, 4611686018427387894] into destination object ‘dst’ of size 6
4 | extern unsigned char dst[6];
| ^~~
stringop-overflow.cpp:4:22: note: at offset 6 into destination object ‘dst’ of size 6
stringop-overflow.cpp:4:22: note: at offset [22, 4611686018427387894] into destination object ‘dst’ of size 6
stringop-overflow.cpp:4:22: note: at offset 6 into destination object ‘dst’ of size 6
cc1plus: all warnings being treated as errors
Florian Sylvestre (3):
ipa: rkisp1: Add enable field for AWB algorithm in IPA context
ipa: rkisp1: Add enable field for LSC algorithm in IPA context
ipa: rkisp1: Add support of Denoise Pre-Filter control
Laurent Pinchart (6):
test: yaml-parser: Simplify code by centralizing parse error checks
test: yaml-parser: Centralize integer parse checks
test: yaml-parser: Test out-of-range checks on integer parsing
libcamera: yaml_parser: Fix bounds checking for 16-bit
YamlObject::get()
libcamera: yaml_parser: Enable YamlObject::get() for int8_t and
uint8_t
libcamera: yaml_parser: De-duplicate common code in YamlObject::get()
include/libcamera/internal/yaml_parser.h | 4 +
src/ipa/rkisp1/algorithms/awb.cpp | 2 +
src/ipa/rkisp1/algorithms/dpf.cpp | 258 ++++++++++++
src/ipa/rkisp1/algorithms/dpf.h | 36 ++
src/ipa/rkisp1/algorithms/lsc.cpp | 10 +
src/ipa/rkisp1/algorithms/lsc.h | 1 +
src/ipa/rkisp1/algorithms/meson.build | 1 +
src/ipa/rkisp1/data/ov5640.yaml | 15 +
src/ipa/rkisp1/ipa_context.cpp | 22 +
src/ipa/rkisp1/ipa_context.h | 10 +
src/libcamera/yaml_parser.cpp | 157 ++++---
test/yaml-parser.cpp | 504 ++++++++++++-----------
12 files changed, 712 insertions(+), 308 deletions(-)
create mode 100644 src/ipa/rkisp1/algorithms/dpf.cpp
create mode 100644 src/ipa/rkisp1/algorithms/dpf.h
base-commit: dfc6d711c9f7f0a9868afa5158aa2089163bded3
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list