From patchwork Tue Aug 16 01:54:05 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 17127 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id 2FE07C3272 for ; Tue, 16 Aug 2022 01:54:31 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 2F46561FC1; Tue, 16 Aug 2022 03:54:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org; s=mail; t=1660614870; bh=X1oFG4gW4FpXgNOsFOALDn/saaPmrvMVpjQ3k993bRw=; h=To:Date:Subject:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:From; b=gpsXW2VHvhhmVExaKfygxHjMCiHESHJQyo9gBrW/9hoqb+thMiQUVNiZv/BQ10DBL nfShjf+YLBFzlkGPe0cj3TlGMTmTO3Ei32IWPFy4J7D9xCUMxgb8+/u69iBWjpsImr 9QFri71gutVfmeTPvT1cU+TkTVT+Oo61lrYFP+hbwjesPGp21CZrbI+PWK+Yu/W6+J MeALRCnu8M54rFCf4HNwie9slmdBOdY1g+HEHbofzVHvWF5gO931nwsVVMXCYp+c3v fgVS7H9TWMu1hdCqu6GPacIMijNZN7cD8OQVGPYnmnHmjf3teq0HW1jckSWfUlnsSV snIC5II9mXkaA== Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 88019603E3 for ; Tue, 16 Aug 2022 03:54:28 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (1024-bit key; unprotected) header.d=ideasonboard.com header.i=@ideasonboard.com header.b="esvD3o65"; dkim-atps=neutral Received: from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi [62.78.145.57]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id E6D95496; Tue, 16 Aug 2022 03:54:27 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1660614868; bh=X1oFG4gW4FpXgNOsFOALDn/saaPmrvMVpjQ3k993bRw=; h=From:To:Cc:Subject:Date:From; b=esvD3o65gX0dc/56SPQ18r2pHLfOgDafnejbrS4voO4RpO+FavDn8UMQjnLpbX1mc fEyfcSXHe44pvKlDMa3Cdh0iP3BBgK7AjIz3CxKNAEEuqVTuI57Qattp6LuEDoN9tb Du78fuLHvBBtgt5q4M1fqO2UPY0ryo81D1Ywi42k= To: libcamera-devel@lists.libcamera.org Date: Tue, 16 Aug 2022 04:54:05 +0300 Message-Id: <20220816015414.7462-1-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.35.1 MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 0/9] Add DPF tuning support for RkISP1 X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Laurent Pinchart via libcamera-devel From: Laurent Pinchart Reply-To: Laurent Pinchart Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" 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 #include extern unsigned char dst[6]; int foo(const std::vector &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::__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 >; _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 >; _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 >; _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 >; _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&)’ 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&)’: 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::__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 >; _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 >; _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 >; _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 >; _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&)’ 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&)’: 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