From patchwork Fri Nov 8 20:53:46 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2297 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 03AFD60180 for ; Fri, 8 Nov 2019 21:54:23 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 7E8BF31D for ; Fri, 8 Nov 2019 21:54:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246462; bh=G3EWCu0Cg+17Po+3IP+/CM6lTboTDnDa5wMsYjgM3yc=; h=From:To:Subject:Date:In-Reply-To:References:From; b=F5VwNyfpVfQZA3yvbjiVx8tO85fSCpYOgzbmdHBbU2MujSfmIKRpe9Ai+RUB++wTt NLZuyG8g7+foiAH8z4jlcqQGS7RLGAeqNdn3QBnzbPIsQb6uHDnoD9JgUKt+ClI8X/ TmHTfgnmRf2gfvcSUlaOJHrNZfy4QFu9XoikqsDY= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:46 +0200 Message-Id: <20191108205409.18845-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 01/24] test: Extract CameraTest class out of camera tests to libtest 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-List-Received-Date: Fri, 08 Nov 2019 20:54:23 -0000 Many tests other than the camera/ tests use a camera. To increase code sharing, move the base CameraTest class to the test library. The class becomes a helper that doesn't inherit from Test anymore (to avoid diamond inheritance issues when more such helpers will exist). Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- test/camera/buffer_import.cpp | 14 ++++----- test/camera/capture.cpp | 15 ++++++--- test/camera/configuration_default.cpp | 16 ++++++++-- test/camera/configuration_set.cpp | 15 ++++++--- test/camera/meson.build | 2 +- test/camera/statemachine.cpp | 15 ++++++--- test/controls/control_list.cpp | 39 +++++------------------- test/{camera => libtest}/camera_test.cpp | 24 +++++++++------ test/{camera => libtest}/camera_test.h | 12 +++----- test/libtest/meson.build | 1 + 10 files changed, 77 insertions(+), 76 deletions(-) rename test/{camera => libtest}/camera_test.cpp (55%) rename test/{camera => libtest}/camera_test.h (77%) diff --git a/test/camera/buffer_import.cpp b/test/camera/buffer_import.cpp index bbc5a25c4019..3efe02704c02 100644 --- a/test/camera/buffer_import.cpp +++ b/test/camera/buffer_import.cpp @@ -18,6 +18,7 @@ #include "v4l2_videodevice.h" #include "camera_test.h" +#include "test.h" using namespace libcamera; @@ -254,11 +255,11 @@ private: bool done_; }; -class BufferImportTest : public CameraTest +class BufferImportTest : public CameraTest, public Test { public: BufferImportTest() - : CameraTest() + : CameraTest("VIMC Sensor B") { } @@ -350,11 +351,10 @@ protected: int init() { - int ret = CameraTest::init(); - if (ret) - return ret; + if (status_ != TestPass) + return status_; - ret = sink_.init(); + int ret = sink_.init(); if (ret != TestPass) { cleanup(); return ret; @@ -422,8 +422,6 @@ protected: camera_->stop(); camera_->freeBuffers(); - - CameraTest::cleanup(); } private: diff --git a/test/camera/capture.cpp b/test/camera/capture.cpp index 791ccad15f70..08cce9c7cbaf 100644 --- a/test/camera/capture.cpp +++ b/test/camera/capture.cpp @@ -8,13 +8,20 @@ #include #include "camera_test.h" +#include "test.h" using namespace std; namespace { -class Capture : public CameraTest +class Capture : public CameraTest, public Test { +public: + Capture() + : CameraTest("VIMC Sensor B") + { + } + protected: unsigned int completeBuffersCount_; unsigned int completeRequestsCount_; @@ -46,14 +53,12 @@ protected: int init() override { - int ret = CameraTest::init(); - if (ret) - return ret; + if (status_ != TestPass) + return status_; config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); if (!config_ || config_->size() != 1) { cout << "Failed to generate default configuration" << endl; - CameraTest::cleanup(); return TestFail; } diff --git a/test/camera/configuration_default.cpp b/test/camera/configuration_default.cpp index ce2ec5d02e7b..31c908d2449e 100644 --- a/test/camera/configuration_default.cpp +++ b/test/camera/configuration_default.cpp @@ -8,15 +8,27 @@ #include #include "camera_test.h" +#include "test.h" using namespace std; namespace { -class ConfigurationDefault : public CameraTest +class ConfigurationDefault : public CameraTest, public Test { +public: + ConfigurationDefault() + : CameraTest("VIMC Sensor B") + { + } + protected: - int run() + int init() override + { + return status_; + } + + int run() override { std::unique_ptr config; diff --git a/test/camera/configuration_set.cpp b/test/camera/configuration_set.cpp index f88da96ca2b7..b4b5968115e8 100644 --- a/test/camera/configuration_set.cpp +++ b/test/camera/configuration_set.cpp @@ -8,24 +8,29 @@ #include #include "camera_test.h" +#include "test.h" using namespace std; namespace { -class ConfigurationSet : public CameraTest +class ConfigurationSet : public CameraTest, public Test { +public: + ConfigurationSet() + : CameraTest("VIMC Sensor B") + { + } + protected: int init() override { - int ret = CameraTest::init(); - if (ret) - return ret; + if (status_ != TestPass) + return status_; config_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); if (!config_ || config_->size() != 1) { cout << "Failed to generate default configuration" << endl; - CameraTest::cleanup(); return TestFail; } diff --git a/test/camera/meson.build b/test/camera/meson.build index d6fd66b8f89e..e2a6660a7a92 100644 --- a/test/camera/meson.build +++ b/test/camera/meson.build @@ -9,7 +9,7 @@ camera_tests = [ ] foreach t : camera_tests - exe = executable(t[0], [t[1], 'camera_test.cpp'], + exe = executable(t[0], t[1], dependencies : libcamera_dep, link_with : test_libraries, include_directories : test_includes_internal) diff --git a/test/camera/statemachine.cpp b/test/camera/statemachine.cpp index 12d5e0e1d534..afa13ba77b0b 100644 --- a/test/camera/statemachine.cpp +++ b/test/camera/statemachine.cpp @@ -8,13 +8,20 @@ #include #include "camera_test.h" +#include "test.h" using namespace std; namespace { -class Statemachine : public CameraTest +class Statemachine : public CameraTest, public Test { +public: + Statemachine() + : CameraTest("VIMC Sensor B") + { + } + protected: int testAvailable() { @@ -233,14 +240,12 @@ protected: int init() override { - int ret = CameraTest::init(); - if (ret) - return ret; + if (status_ != TestPass) + return status_; defconf_ = camera_->generateConfiguration({ StreamRole::VideoRecording }); if (!defconf_) { cout << "Failed to generate default configuration" << endl; - CameraTest::cleanup(); return TestFail; } diff --git a/test/controls/control_list.cpp b/test/controls/control_list.cpp index 5af53f64bb6c..4d212abd09e6 100644 --- a/test/controls/control_list.cpp +++ b/test/controls/control_list.cpp @@ -13,32 +13,22 @@ #include #include "camera_controls.h" + +#include "camera_test.h" #include "test.h" using namespace std; using namespace libcamera; -class ControlListTest : public Test +class ControlListTest : public CameraTest, public Test { -protected: - int init() +public: + ControlListTest() + : CameraTest("VIMC Sensor B") { - cm_ = new CameraManager(); - - if (cm_->start()) { - cout << "Failed to start camera manager" << endl; - return TestFail; - } - - camera_ = cm_->get("VIMC Sensor B"); - if (!camera_) { - cout << "Can not find VIMC camera" << endl; - return TestSkip; - } - - return TestPass; } +protected: int run() { CameraControlValidator validator(camera_.get()); @@ -156,21 +146,6 @@ protected: return TestPass; } - - void cleanup() - { - if (camera_) { - camera_->release(); - camera_.reset(); - } - - cm_->stop(); - delete cm_; - } - -private: - CameraManager *cm_; - std::shared_ptr camera_; }; TEST_REGISTER(ControlListTest) diff --git a/test/camera/camera_test.cpp b/test/libtest/camera_test.cpp similarity index 55% rename from test/camera/camera_test.cpp rename to test/libtest/camera_test.cpp index 101e31fbce79..2ae4d6776f2e 100644 --- a/test/camera/camera_test.cpp +++ b/test/libtest/camera_test.cpp @@ -8,35 +8,39 @@ #include #include "camera_test.h" +#include "test.h" using namespace libcamera; using namespace std; -int CameraTest::init() +CameraTest::CameraTest(const char *name) { cm_ = new CameraManager(); if (cm_->start()) { - cout << "Failed to start camera manager" << endl; - return TestFail; + cerr << "Failed to start camera manager" << endl; + status_ = TestFail; + return; } - camera_ = cm_->get("VIMC Sensor B"); + camera_ = cm_->get(name); if (!camera_) { - cout << "Can not find VIMC camera" << endl; - return TestSkip; + cerr << "Can not find '" << name << "' camera" << endl; + status_ = TestSkip; + return; } /* Sanity check that the camera has streams. */ if (camera_->streams().empty()) { - cout << "Camera has no stream" << endl; - return TestFail; + cerr << "Camera has no stream" << endl; + status_ = TestFail; + return; } - return TestPass; + status_ = TestPass; } -void CameraTest::cleanup() +CameraTest::~CameraTest() { if (camera_) { camera_->release(); diff --git a/test/camera/camera_test.h b/test/libtest/camera_test.h similarity index 77% rename from test/camera/camera_test.h rename to test/libtest/camera_test.h index e57b05eb28a9..0b6bad05e37c 100644 --- a/test/camera/camera_test.h +++ b/test/libtest/camera_test.h @@ -9,22 +9,18 @@ #include -#include "test.h" - using namespace libcamera; -class CameraTest : public Test +class CameraTest { public: - CameraTest() - : cm_(nullptr) {} + CameraTest(const char *name); + ~CameraTest(); protected: - int init(); - void cleanup(); - CameraManager *cm_; std::shared_ptr camera_; + int status_; }; #endif /* __LIBCAMERA_CAMERA_TEST_H__ */ diff --git a/test/libtest/meson.build b/test/libtest/meson.build index ca762b4421c2..3e798ef3810e 100644 --- a/test/libtest/meson.build +++ b/test/libtest/meson.build @@ -1,4 +1,5 @@ libtest_sources = files([ + 'camera_test.cpp', 'test.cpp', ]) From patchwork Fri Nov 8 20:53:47 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2298 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 35BAC6150A for ; Fri, 8 Nov 2019 21:54:23 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C8C792D1 for ; Fri, 8 Nov 2019 21:54:22 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246462; bh=aVv4tmUKK5hzM121JHAcTAX/nWTzokiWDabNINf7hpY=; h=From:To:Subject:Date:In-Reply-To:References:From; b=ZlWXPOsc4VsgUaxcKt5EfRghz3fsuqdSKq84Z6QgdnvzL8xdZXCAD7ae9kMWbaY22 ro80uooDXH67rTHUhSB55KLv/VDTCUjcwVoaZG5szyCZNPzJlC3XTUsLZuNtN04+c2 FFK9Xfb7YpnOdLwH7GQxXmfpSLIeTRx3nMAqaqm4= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:47 +0200 Message-Id: <20191108205409.18845-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 02/24] test: controls: Add ControlInfoMap test 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-List-Received-Date: Fri, 08 Nov 2019 20:54:23 -0000 Add a test to exercise the ControlInfoMap API. This currently tests at(), count(), find() and end(). Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- test/controls/control_info.cpp | 77 ++++++++++++++++++++++++++++++++++ test/controls/meson.build | 1 + 2 files changed, 78 insertions(+) create mode 100644 test/controls/control_info.cpp diff --git a/test/controls/control_info.cpp b/test/controls/control_info.cpp new file mode 100644 index 000000000000..c2678ea3e593 --- /dev/null +++ b/test/controls/control_info.cpp @@ -0,0 +1,77 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * control_info.cpp - ControlInfoMap tests + */ + +#include + +#include +#include +#include +#include + +#include "camera_controls.h" + +#include "camera_test.h" +#include "test.h" + +using namespace std; +using namespace libcamera; + +class ControlInfoMapTest : public CameraTest, public Test +{ +public: + ControlInfoMapTest() + : CameraTest("VIMC Sensor B") + { + } + +protected: + int run() + { + const ControlInfoMap &info = camera_->controls(); + + /* Test looking up a valid control by ControlId. */ + if (info.count(&controls::Brightness) != 1) { + cout << "count() on valid control failed" << endl; + return TestFail; + } + + if (info.find(&controls::Brightness) == info.end()) { + cout << "find() on valid control failed" << endl; + return TestFail; + } + + info.at(&controls::Brightness); + + /* Test looking up a valid control by numerical ID. */ + if (info.count(controls::Brightness.id()) != 1) { + cout << "count() on valid ID failed" << endl; + return TestFail; + } + + if (info.find(controls::Brightness.id()) == info.end()) { + cout << "find() on valid ID failed" << endl; + return TestFail; + } + + info.at(controls::Brightness.id()); + + /* Test looking up an invalid control by numerical ID. */ + if (info.count(12345) != 0) { + cout << "count() on invalid ID failed" << endl; + return TestFail; + } + + if (info.find(12345) != info.end()) { + cout << "find() on invalid ID failed" << endl; + return TestFail; + } + + return TestPass; + } +}; + +TEST_REGISTER(ControlInfoMapTest) diff --git a/test/controls/meson.build b/test/controls/meson.build index 9f0f005a2759..f0850df28c8a 100644 --- a/test/controls/meson.build +++ b/test/controls/meson.build @@ -1,4 +1,5 @@ control_tests = [ + [ 'control_info', 'control_info.cpp' ], [ 'control_list', 'control_list.cpp' ], [ 'control_range', 'control_range.cpp' ], [ 'control_value', 'control_value.cpp' ], From patchwork Fri Nov 8 20:53:48 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2299 X-Patchwork-Delegate: laurent.pinchart@ideasonboard.com Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7BADD6150A for ; Fri, 8 Nov 2019 21:54:23 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1FFCE31D for ; Fri, 8 Nov 2019 21:54:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246463; bh=Wb7kncZOf6ol9m4hG99xyRfJ9ENctf05jp1fANDSsJg=; h=From:To:Subject:Date:In-Reply-To:References:From; b=iIdTUm1BA6tRxqXu5ptOVgJJsS6bXwUPgrA8qyDZsoBnNeWP55rDvEkHQm8tUSv8o ixPUhZU8bMvMPPtvMXEArFJ7oGR+6DKxBZan+0V2uimIiyyv7cfmk79VTMHx1Nx6YF 0n/xPqRUrRcdN/Y282BBx90WhV2DiYNL2VTbE/gc= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:48 +0200 Message-Id: <20191108205409.18845-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 03/24] libcamera: controls: Avoid exception in ControlList count() and find() 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-List-Received-Date: Fri, 08 Nov 2019 20:54:23 -0000 The ControlList count() and find() methods use at() to lookup the control numerical ID in the idmap_. This causes an exception to be thrown if the ID doesn't exist in the map. Fix it by using the find() method instead. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- src/libcamera/controls.cpp | 18 +++++++++++++++--- 1 file changed, 15 insertions(+), 3 deletions(-) diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 93ad2fc6a276..0c7cd449ad64 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -491,7 +491,11 @@ const ControlInfoMap::mapped_type &ControlInfoMap::at(unsigned int id) const */ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const { - return count(idmap_.at(id)); + auto iter = idmap_.find(id); + if (iter == idmap_.end()) + return 0; + + return count(iter->second); } /** @@ -502,7 +506,11 @@ ControlInfoMap::size_type ControlInfoMap::count(unsigned int id) const */ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id) { - return find(idmap_.at(id)); + auto iter = idmap_.find(id); + if (iter == idmap_.end()) + return end(); + + return find(iter->second); } /** @@ -513,7 +521,11 @@ ControlInfoMap::iterator ControlInfoMap::find(unsigned int id) */ ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const { - return find(idmap_.at(id)); + auto iter = idmap_.find(id); + if (iter == idmap_.end()) + return end(); + + return find(iter->second); } /** From patchwork Fri Nov 8 20:53:49 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2300 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C12F46150A for ; Fri, 8 Nov 2019 21:54:23 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 6A8FD2D1 for ; Fri, 8 Nov 2019 21:54:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246463; bh=e3dtX6ahdlkXeAiEXmFYjzT+E4GcwvbePsaDqt/KQZ0=; h=From:To:Subject:Date:In-Reply-To:References:From; b=JxQloEf7tcJ5an0vG9735jV+VS8yoLHlGnIqSB0ESH9Q4aBWpnTQd77urCKGUGu8g R203229ow3eunoG4Dcd+rTMd5f8RnucRtO/QnXHGGNbLGC+JLXz1H8TX4wAgR8ax13 /md91NmtupQsJa3irPk3/YWX6LYK9bT2cNFla8go= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:49 +0200 Message-Id: <20191108205409.18845-5-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 04/24] libcamera: controls: Add operator== and operator!= to ControlRange 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-List-Received-Date: Fri, 08 Nov 2019 20:54:23 -0000 Allow comparision of control ranges by adding the required operators. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- include/libcamera/controls.h | 9 +++++++++ src/libcamera/controls.cpp | 11 +++++++++++ 2 files changed, 20 insertions(+) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 42e6df7e613d..19075858fbba 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -112,6 +112,15 @@ public: std::string toString() const; + bool operator==(const ControlRange &other) const + { + return min_ == other.min_ && max_ == other.max_; + } + bool operator!=(const ControlRange &other) const + { + return !(*this == other); + } + private: ControlValue min_; ControlValue max_; diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 0c7cd449ad64..2c5c98633585 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -383,6 +383,17 @@ std::string ControlRange::toString() const return ss.str(); } +/** + * \fn bool ControlRange::operator==() + * \brief Compare ControlRange instances for equality + * \return True if the ranges have identical min and max, false otherwise + */ +/** + * \fn bool ControlRange::operator!=() + * \brief Compare ControlRange instances for non equality + * \return False if the ranges have identical min and max, true otherwise + */ + /** * \typedef ControlIdMap * \brief A map of numerical control ID to ControlId From patchwork Fri Nov 8 20:53:50 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2301 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2355161518 for ; Fri, 8 Nov 2019 21:54:24 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id B34EE31D for ; Fri, 8 Nov 2019 21:54:23 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246463; bh=H6M8NSZF6jSyz1p/+mvTTR2Qq5ZmYJYZRMI1+csblmg=; h=From:To:Subject:Date:In-Reply-To:References:From; b=o9PLxABGMqywmoM+H86TL+A4Nhh8i7pwYSnmHif5+Y+vpxWbqC/6e6rbH7dCjuqM5 j9Xfq3NTqZLXI74IL277chN35D+xC9qaG6IZteBB4yFRJWRjGC3i+vwfNVHKAWl3on LxbowFQ6Ro4uZpP8gltvdmx1Xl6CDTiPJq8bPiD0= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:50 +0200 Message-Id: <20191108205409.18845-6-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 05/24] libcamera: controls: Index ControlList by unsigned int 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-List-Received-Date: Fri, 08 Nov 2019 20:54:25 -0000 In preparation for serialization, index the ControlList by unsigned int. This will allow deserializing a ControlList without requiring external information. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- include/libcamera/controls.h | 26 +++++++--- src/libcamera/camera_controls.cpp | 4 +- src/libcamera/controls.cpp | 59 +++++++++-------------- src/libcamera/include/camera_controls.h | 2 +- src/libcamera/include/control_validator.h | 2 +- src/libcamera/pipeline/uvcvideo.cpp | 4 +- src/libcamera/pipeline/vimc.cpp | 4 +- src/libcamera/v4l2_device.cpp | 23 ++++----- 8 files changed, 60 insertions(+), 64 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 19075858fbba..f9979a466eaa 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -78,12 +78,22 @@ private: ControlType type_; }; -static inline bool operator==(const ControlId &lhs, const ControlId &rhs) +static inline bool operator==(unsigned int lhs, const ControlId &rhs) { - return lhs.id() == rhs.id(); + return lhs == rhs.id(); } -static inline bool operator!=(const ControlId &lhs, const ControlId &rhs) +static inline bool operator!=(unsigned int lhs, const ControlId &rhs) +{ + return !(lhs == rhs); +} + +static inline bool operator==(const ControlId &lhs, unsigned int rhs) +{ + return lhs.id() == rhs; +} + +static inline bool operator!=(const ControlId &lhs, unsigned int rhs) { return !(lhs == rhs); } @@ -175,7 +185,7 @@ private: class ControlList { private: - using ControlListMap = std::unordered_map; + using ControlListMap = std::unordered_map; public: ControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr); @@ -199,7 +209,7 @@ public: template const T &get(const Control &ctrl) const { - const ControlValue *val = find(ctrl); + const ControlValue *val = find(ctrl.id()); if (!val) { static T t(0); return t; @@ -211,7 +221,7 @@ public: template void set(const Control &ctrl, const T &value) { - ControlValue *val = find(ctrl); + ControlValue *val = find(ctrl.id()); if (!val) return; @@ -222,8 +232,8 @@ public: void set(unsigned int id, const ControlValue &value); private: - const ControlValue *find(const ControlId &id) const; - ControlValue *find(const ControlId &id); + const ControlValue *find(unsigned int id) const; + ControlValue *find(unsigned int id); ControlValidator *validator_; const ControlIdMap *idmap_; diff --git a/src/libcamera/camera_controls.cpp b/src/libcamera/camera_controls.cpp index 341da56019f7..59dcede2ca36 100644 --- a/src/libcamera/camera_controls.cpp +++ b/src/libcamera/camera_controls.cpp @@ -44,10 +44,10 @@ const std::string &CameraControlValidator::name() const * \param[in] id The control ID * \return True if the control is valid, false otherwise */ -bool CameraControlValidator::validate(const ControlId &id) const +bool CameraControlValidator::validate(unsigned int id) const { const ControlInfoMap &controls = camera_->controls(); - return controls.find(&id) != controls.end(); + return controls.find(id) != controls.end(); } } /* namespace libcamera */ diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 2c5c98633585..021d5f0990e0 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -259,16 +259,21 @@ bool ControlValue::operator==(const ControlValue &other) const */ /** - * \fn bool operator==(const ControlId &lhs, const ControlId &rhs) - * \brief Compare two ControlId instances for equality - * \param[in] lhs Left-hand side ControlId + * \fn bool operator==(unsigned int lhs, const ControlId &rhs) + * \brief Compare a ControlId with a control numerical ID + * \param[in] lhs Left-hand side numerical ID * \param[in] rhs Right-hand side ControlId * - * ControlId instances are compared based on the numerical ControlId::id() - * only, as an object may not have two separate controls with the same - * numerical ID. + * \return True if \a lhs is equal to \a rhs.id(), false otherwise + */ + +/** + * \fn bool operator==(const ControlId &lhs, unsigned int rhs) + * \brief Compare a ControlId with a control numerical ID + * \param[in] lhs Left-hand side ControlId + * \param[in] rhs Right-hand side numerical ID * - * \return True if \a lhs and \a rhs have equal control IDs, false otherwise + * \return True if \a lhs.id() is equal to \a rhs, false otherwise */ /** @@ -655,7 +660,7 @@ ControlList::ControlList(const ControlInfoMap &info, ControlValidator *validator */ bool ControlList::contains(const ControlId &id) const { - return controls_.find(&id) != controls_.end(); + return controls_.find(id.id()) != controls_.end(); } /** @@ -666,11 +671,7 @@ bool ControlList::contains(const ControlId &id) const */ bool ControlList::contains(unsigned int id) const { - const auto iter = idmap_->find(id); - if (iter == idmap_->end()) - return false; - - return contains(*iter->second); + return controls_.find(id) != controls_.end(); } /** @@ -716,15 +717,7 @@ const ControlValue &ControlList::get(unsigned int id) const { static ControlValue zero; - const auto ctrl = idmap_->find(id); - if (ctrl == idmap_->end()) { - LOG(Controls, Error) - << "Control " << utils::hex(id) - << " is not supported"; - return zero; - } - - const ControlValue *val = find(*ctrl->second); + const ControlValue *val = find(id); if (!val) return zero; @@ -745,27 +738,19 @@ const ControlValue &ControlList::get(unsigned int id) const */ void ControlList::set(unsigned int id, const ControlValue &value) { - const auto ctrl = idmap_->find(id); - if (ctrl == idmap_->end()) { - LOG(Controls, Error) - << "Control 0x" << utils::hex(id) - << " is not supported"; - return; - } - - ControlValue *val = find(*ctrl->second); + ControlValue *val = find(id); if (!val) return; *val = value; } -const ControlValue *ControlList::find(const ControlId &id) const +const ControlValue *ControlList::find(unsigned int id) const { - const auto iter = controls_.find(&id); + const auto iter = controls_.find(id); if (iter == controls_.end()) { LOG(Controls, Error) - << "Control " << id.name() << " not found"; + << "Control " << utils::hex(id) << " not found"; return nullptr; } @@ -773,16 +758,16 @@ const ControlValue *ControlList::find(const ControlId &id) const return &iter->second; } -ControlValue *ControlList::find(const ControlId &id) +ControlValue *ControlList::find(unsigned int id) { if (validator_ && !validator_->validate(id)) { LOG(Controls, Error) - << "Control " << id.name() + << "Control " << utils::hex(id) << " is not valid for " << validator_->name(); return nullptr; } - return &controls_[&id]; + return &controls_[id]; } } /* namespace libcamera */ diff --git a/src/libcamera/include/camera_controls.h b/src/libcamera/include/camera_controls.h index 998a2d155a44..265c1fe379db 100644 --- a/src/libcamera/include/camera_controls.h +++ b/src/libcamera/include/camera_controls.h @@ -19,7 +19,7 @@ public: CameraControlValidator(Camera *camera); const std::string &name() const override; - bool validate(const ControlId &id) const override; + bool validate(unsigned int id) const override; private: Camera *camera_; diff --git a/src/libcamera/include/control_validator.h b/src/libcamera/include/control_validator.h index 3598b18f2f26..f1c9110b158f 100644 --- a/src/libcamera/include/control_validator.h +++ b/src/libcamera/include/control_validator.h @@ -19,7 +19,7 @@ public: virtual ~ControlValidator() {} virtual const std::string &name() const = 0; - virtual bool validate(const ControlId &id) const = 0; + virtual bool validate(unsigned int id) const = 0; }; } /* namespace libcamera */ diff --git a/src/libcamera/pipeline/uvcvideo.cpp b/src/libcamera/pipeline/uvcvideo.cpp index 45448d6f8c05..522229241ffb 100644 --- a/src/libcamera/pipeline/uvcvideo.cpp +++ b/src/libcamera/pipeline/uvcvideo.cpp @@ -231,7 +231,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) ControlList controls(data->video_->controls()); for (auto it : request->controls()) { - const ControlId &id = *it.first; + unsigned int id = it.first; ControlValue &value = it.second; if (id == controls::Brightness) { @@ -250,7 +250,7 @@ int PipelineHandlerUVC::processControls(UVCCameraData *data, Request *request) for (const auto &ctrl : controls) LOG(UVC, Debug) - << "Setting control " << ctrl.first->name() + << "Setting control " << utils::hex(ctrl.first) << " to " << ctrl.second.toString(); int ret = data->video_->setControls(&controls); diff --git a/src/libcamera/pipeline/vimc.cpp b/src/libcamera/pipeline/vimc.cpp index e6ab6a085824..06664fed42e7 100644 --- a/src/libcamera/pipeline/vimc.cpp +++ b/src/libcamera/pipeline/vimc.cpp @@ -298,7 +298,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) ControlList controls(data->sensor_->controls()); for (auto it : request->controls()) { - const ControlId &id = *it.first; + unsigned int id = it.first; ControlValue &value = it.second; if (id == controls::Brightness) @@ -311,7 +311,7 @@ int PipelineHandlerVimc::processControls(VimcCameraData *data, Request *request) for (const auto &ctrl : controls) LOG(VIMC, Debug) - << "Setting control " << ctrl.first->name() + << "Setting control " << utils::hex(ctrl.first) << " to " << ctrl.second.toString(); int ret = data->sensor_->setControls(&controls); diff --git a/src/libcamera/v4l2_device.cpp b/src/libcamera/v4l2_device.cpp index a2b0d891b12f..0452f801029c 100644 --- a/src/libcamera/v4l2_device.cpp +++ b/src/libcamera/v4l2_device.cpp @@ -176,15 +176,15 @@ int V4L2Device::getControls(ControlList *ctrls) unsigned int i = 0; for (const auto &ctrl : *ctrls) { - const ControlId *id = ctrl.first; - const auto iter = controls_.find(id->id()); + unsigned int id = ctrl.first; + const auto iter = controls_.find(id); if (iter == controls_.end()) { LOG(V4L2, Error) - << "Control '" << id->name() << "' not found"; + << "Control " << utils::hex(id) << " not found"; return -EINVAL; } - v4l2Ctrls[i].id = id->id(); + v4l2Ctrls[i].id = id; i++; } @@ -250,19 +250,19 @@ int V4L2Device::setControls(ControlList *ctrls) unsigned int i = 0; for (const auto &ctrl : *ctrls) { - const ControlId *id = ctrl.first; - const auto iter = controls_.find(id->id()); + unsigned int id = ctrl.first; + const auto iter = controls_.find(id); if (iter == controls_.end()) { LOG(V4L2, Error) - << "Control '" << id->name() << "' not found"; + << "Control " << utils::hex(id) << " not found"; return -EINVAL; } - v4l2Ctrls[i].id = id->id(); + v4l2Ctrls[i].id = id; /* Set the v4l2_ext_control value for the write operation. */ const ControlValue &value = ctrl.second; - switch (id->type()) { + switch (iter->first->type()) { case ControlTypeInteger64: v4l2Ctrls[i].value64 = value.get(); break; @@ -403,10 +403,11 @@ void V4L2Device::updateControls(ControlList *ctrls, break; const struct v4l2_ext_control *v4l2Ctrl = &v4l2Ctrls[i]; - const ControlId *id = ctrl.first; + unsigned int id = ctrl.first; ControlValue &value = ctrl.second; - switch (id->type()) { + const auto iter = controls_.find(id); + switch (iter->first->type()) { case ControlTypeInteger64: value.set(v4l2Ctrl->value64); break; From patchwork Fri Nov 8 20:53:51 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2302 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 62F6761373 for ; Fri, 8 Nov 2019 21:54:24 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 0DC4A2D1 for ; Fri, 8 Nov 2019 21:54:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246464; bh=O+T5l8wGjneoI9bPSa6I1IA4dwhgv70DhQpQ0caJs/w=; h=From:To:Subject:Date:In-Reply-To:References:From; b=TqGaZJ73j0hEhXtU02Jsddb2ro6FwFLgaeJ+F8lRaMel8M8HLeUnsJhrabQ4Q5l0m ERw38PGFrn5AM4JG9zOb9YX0RjXqQDrslak3Z+B8UYX0DglSayqzavC6RkLdtQeNrD q1pUhVJ7V46Z1l5l8rCco1IYkIq3P0mzyqHOp43s= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:51 +0200 Message-Id: <20191108205409.18845-7-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 06/24] libcamera: controls: Add move constructor to ControlInfoMap 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-List-Received-Date: Fri, 08 Nov 2019 20:54:25 -0000 The ControlInfoMap class has a move assignment operator from a plain map, but on corresponding move constructor. Add one. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund Reviewed-by: Kieran Bingham --- include/libcamera/controls.h | 1 + src/libcamera/controls.cpp | 13 +++++++++++++ 2 files changed, 14 insertions(+) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index f9979a466eaa..548c06c65bb6 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -146,6 +146,7 @@ public: ControlInfoMap() = default; ControlInfoMap(const ControlInfoMap &other) = default; ControlInfoMap(std::initializer_list init); + ControlInfoMap(Map &&info); ControlInfoMap &operator=(const ControlInfoMap &other) = default; ControlInfoMap &operator=(std::initializer_list init); diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 021d5f0990e0..ae6ca2a7cf7e 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -445,6 +445,19 @@ ControlInfoMap::ControlInfoMap(std::initializer_list init) generateIdmap(); } +/** + * \brief Construct a ControlInfoMap from a plain map + * \param[in] info The control info plain map + * + * Construct a new ControlInfoMap and populate its contents with those of + * \a info using move semantics. Upon return the \a info map will be empty. + */ +ControlInfoMap::ControlInfoMap(Map &&info) + : Map(std::move(info)) +{ + generateIdmap(); +} + /** * \fn ControlInfoMap &ControlInfoMap::operator=(const ControlInfoMap &other) * \brief Copy assignment operator, replace the contents with a copy of \a other From patchwork Fri Nov 8 20:53:52 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2303 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id C1E0D61516 for ; Fri, 8 Nov 2019 21:54:24 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 5991631D; Fri, 8 Nov 2019 21:54:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246464; bh=T+KwLux/OSOYDRaBtiIFJ3C9yYPkiGWFWzXvJP3/kXY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=IWqFzdzycqSSqfkR6LBWkovWaEhA4AN8fd3yxR674E5GqMGbLJdTbS4+ws7qmxwZC zqHItl/alYa3+yTE7fSj/U+1YhGEgNeLG6snXpe0Kj0p/VkL4+coMqgH/8m9A8TGvb oKkEu2pUc5TEedBCLvf+EA55B1diRNPvsNBfVPOQ= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:52 +0200 Message-Id: <20191108205409.18845-8-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 07/24] libcamera: controls: Make ControlId constructor public 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-List-Received-Date: Fri, 08 Nov 2019 20:54:26 -0000 From: Jacopo Mondi In order to be able to create a ControlId from serialized data, make its constructor public. Signed-off-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 548c06c65bb6..6bad36cbc369 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -59,16 +59,15 @@ private: class ControlId { public: - unsigned int id() const { return id_; } - const std::string &name() const { return name_; } - ControlType type() const { return type_; } - -protected: ControlId(unsigned int id, const std::string &name, ControlType type) : id_(id), name_(name), type_(type) { } + unsigned int id() const { return id_; } + const std::string &name() const { return name_; } + ControlType type() const { return type_; } + private: ControlId &operator=(const ControlId &) = delete; ControlId(const ControlId &) = delete; From patchwork Fri Nov 8 20:53:53 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2304 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2BA5F61520 for ; Fri, 8 Nov 2019 21:54:25 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BFC2871D for ; Fri, 8 Nov 2019 21:54:24 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246464; bh=U8IClzdsfFjEIH8l0jtC+GTgMg6JFpnH1Wr0kAPKv20=; h=From:To:Subject:Date:In-Reply-To:References:From; b=oLQvp+cjHFlXEteUUFvrZlqJCWJ0w8m5wnqiSrAWl4x6tuzCRHIqDmmZkbg+5ToxG Px5FepcPuzyJW2Ib+PEKpS7ORpBo4AkeFszxL867dPWHp3DZefU0/BVyGh7D/p4Wae ewzXpx0NWxGSUWJA2iS60x0Er4aSlEZzOEFq5ASo= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:53 +0200 Message-Id: <20191108205409.18845-9-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 08/24] libcamera: controls: Make ControList constructor public 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-List-Received-Date: Fri, 08 Nov 2019 20:54:26 -0000 We need to construct empty ControlList objects to serialization. Make the constructor public. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- include/libcamera/controls.h | 1 + src/libcamera/controls.cpp | 11 +++++++++++ 2 files changed, 12 insertions(+) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index 6bad36cbc369..b35e006bc046 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -188,6 +188,7 @@ private: using ControlListMap = std::unordered_map; public: + ControlList(); ControlList(const ControlIdMap &idmap, ControlValidator *validator = nullptr); ControlList(const ControlInfoMap &info, ControlValidator *validator = nullptr); diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index ae6ca2a7cf7e..178ce3d99bce 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -588,6 +588,17 @@ void ControlInfoMap::generateIdmap() * controls. */ +/** + * \brief Construct a ControlList not associated with any object + * + * This constructor is meant to support ControlList serialization and shall not + * be used directly by application. + */ +ControlList::ControlList() + : validator_(nullptr), idmap_(nullptr) +{ +} + /** * \brief Construct a ControlList with an optional control validator * \param[in] idmap The ControlId map for the control list target object From patchwork Fri Nov 8 20:53:54 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2305 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id DCB7E61518 for ; Fri, 8 Nov 2019 21:54:25 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 170EA31D; Fri, 8 Nov 2019 21:54:25 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246465; bh=0Ef+VMEo12M3yvF5ZfP5KQ4/VIdqLLwg5eVgLy7S4KY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=IOr02YPEp2ANx4kr8nbPn0K77tDfmwuzwM/L4mF62bbsg7j527pd2eI9iqI4Ysip3 46slNV25RQZZ1Mp4uKjF0zNc9UCH35xDvP1w4EPqNL4h/rQDxgg9157Cew2JCPeyZM Dmthr/yTvBRRvAkOaGN1Rs7CbzzEX8Mz1Tkyh7Bg= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:54 +0200 Message-Id: <20191108205409.18845-10-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 09/24] libcamera: controls: Store reference to the InfoMap 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-List-Received-Date: Fri, 08 Nov 2019 20:54:26 -0000 From: Jacopo Mondi Store a reference to the ControlInfoMap used to create a ControlList and provide an operation to retrieve it. This will be used to implement serialization of ControlList. Signed-off-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/libcamera/controls.h | 4 ++++ src/libcamera/controls.cpp | 16 +++++++++++++--- 2 files changed, 17 insertions(+), 3 deletions(-) diff --git a/include/libcamera/controls.h b/include/libcamera/controls.h index b35e006bc046..baca684444a7 100644 --- a/include/libcamera/controls.h +++ b/include/libcamera/controls.h @@ -232,12 +232,16 @@ public: const ControlValue &get(unsigned int id) const; void set(unsigned int id, const ControlValue &value); + const ControlInfoMap *infoMap() const { return infoMap_; } + private: const ControlValue *find(unsigned int id) const; ControlValue *find(unsigned int id); ControlValidator *validator_; const ControlIdMap *idmap_; + const ControlInfoMap *infoMap_; + ControlListMap controls_; }; diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index 178ce3d99bce..eae0250a92e3 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -595,7 +595,7 @@ void ControlInfoMap::generateIdmap() * be used directly by application. */ ControlList::ControlList() - : validator_(nullptr), idmap_(nullptr) + : validator_(nullptr), idmap_(nullptr), infoMap_(nullptr) { } @@ -609,7 +609,7 @@ ControlList::ControlList() * argument. */ ControlList::ControlList(const ControlIdMap &idmap, ControlValidator *validator) - : validator_(validator), idmap_(&idmap) + : validator_(validator), idmap_(&idmap), infoMap_(nullptr) { } @@ -619,7 +619,7 @@ ControlList::ControlList(const ControlIdMap &idmap, ControlValidator *validator) * \param[in] validator The validator (may be null) */ ControlList::ControlList(const ControlInfoMap &info, ControlValidator *validator) - : validator_(validator), idmap_(&info.idmap()) + : validator_(validator), idmap_(&info.idmap()), infoMap_(&info) { } @@ -769,6 +769,16 @@ void ControlList::set(unsigned int id, const ControlValue &value) *val = value; } +/** + * \fn ControlList::infoMap() + * \brief Retrieve the ControlInfoMap used to construct the ControlList + * + * \return The ControlInfoMap used to construct the ControlList. ControlList + * instances constructed with ControlList() or + * ControlList(const ControlIdMap &idmap, ControlValidator *validator) have no + * associated ControlInfoMap, nullptr is returned in that case. + */ + const ControlValue *ControlList::find(unsigned int id) const { const auto iter = controls_.find(id); From patchwork Fri Nov 8 20:53:55 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2306 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 342AD61373 for ; Fri, 8 Nov 2019 21:54:26 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id CF2362D1 for ; Fri, 8 Nov 2019 21:54:25 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246465; bh=rvel1WbwIi38a5JoPv5cDk5BY2zcxawNIjeP/GSlCEg=; h=From:To:Subject:Date:In-Reply-To:References:From; b=DmlL921pe8Bwp+k5qeFK3B3lWnyJMkriX7OjZSl8oXxAoLrEQur1GqRbOlsLJDxUr x6aPMZBtPFHbEsuEvOaKJnM+07lMpBJt883aPOLAdocU7LyM/OCRkRT2CYc79rTVY4 sKqPFxphygv4lpEQJyov+NJ9E1IJEXPJ62GlVMBU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:55 +0200 Message-Id: <20191108205409.18845-11-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 10/24] libcamera: controls: Catch type mismatch in ControlInfoMap 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-List-Received-Date: Fri, 08 Nov 2019 20:54:27 -0000 ControlInfoMap requires the ControlId and ControlRange of each entry to have identical types. Check for this and log an error if a mismatch is detected. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund Reviewed-by: Jacopo Mondi --- src/libcamera/controls.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/libcamera/controls.cpp b/src/libcamera/controls.cpp index eae0250a92e3..c488b2e4eb3f 100644 --- a/src/libcamera/controls.cpp +++ b/src/libcamera/controls.cpp @@ -571,8 +571,19 @@ ControlInfoMap::const_iterator ControlInfoMap::find(unsigned int id) const void ControlInfoMap::generateIdmap() { idmap_.clear(); - for (const auto &ctrl : *this) + + for (const auto &ctrl : *this) { + if (ctrl.first->type() != ctrl.second.min().type()) { + LOG(Controls, Error) + << "Control " << utils::hex(ctrl.first->id()) + << " type and range type mismatch"; + idmap_.clear(); + clear(); + return; + } + idmap_[ctrl.first->id()] = ctrl.first; + } } /** From patchwork Fri Nov 8 20:53:56 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2307 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7D06F61529 for ; Fri, 8 Nov 2019 21:54:26 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2BAA7A39 for ; Fri, 8 Nov 2019 21:54:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246466; bh=E/CpqVQl3laiOYaUjP6RsFCIj8xzLFAFk3y3ksKPzaI=; h=From:To:Subject:Date:In-Reply-To:References:From; b=pVJgc/fdVS9lNXQlmJ625pCFoPEVjK5WWah97gdnFDQofe+vyPsTMYM9cNlSDzqOF mUnRYE4wzmQq/Vz1vi2rozlIA2lMug5Gz5/hAsC6pVGq1x5NUbKmhdAUgXzfl7P95i LJvt/T2xfRRakq9AgCqgSTM5FP6HmkO32vnjWkO4= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:56 +0200 Message-Id: <20191108205409.18845-12-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 11/24] libcamera: v4l2_controls: Fix control range construction for bool 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-List-Received-Date: Fri, 08 Nov 2019 20:54:27 -0000 V4L2 controls of type V4L2_CTRL_TYPE_BOOLEAN are incorrectly described with a ControlRange of int32_t values. Fix it. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund Reviewed-by: Kieran Bingham --- src/libcamera/v4l2_controls.cpp | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/libcamera/v4l2_controls.cpp b/src/libcamera/v4l2_controls.cpp index 2462c3e2c264..b6547a7c627c 100644 --- a/src/libcamera/v4l2_controls.cpp +++ b/src/libcamera/v4l2_controls.cpp @@ -118,12 +118,22 @@ V4L2ControlId::V4L2ControlId(const struct v4l2_query_ext_ctrl &ctrl) */ V4L2ControlRange::V4L2ControlRange(const struct v4l2_query_ext_ctrl &ctrl) { - if (ctrl.type == V4L2_CTRL_TYPE_INTEGER64) + switch (ctrl.type) { + case V4L2_CTRL_TYPE_BOOLEAN: + ControlRange::operator=(ControlRange(static_cast(ctrl.minimum), + static_cast(ctrl.maximum))); + break; + + case V4L2_CTRL_TYPE_INTEGER64: ControlRange::operator=(ControlRange(static_cast(ctrl.minimum), static_cast(ctrl.maximum))); - else + break; + + default: ControlRange::operator=(ControlRange(static_cast(ctrl.minimum), static_cast(ctrl.maximum))); + break; + } } } /* namespace libcamera */ From patchwork Fri Nov 8 20:53:57 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2308 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D265F61518 for ; Fri, 8 Nov 2019 21:54:26 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 74CAC2D1 for ; Fri, 8 Nov 2019 21:54:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246466; bh=mUz2y97UlVf1Vpah379AqeKhS/xrm72J/EvsCEVFago=; h=From:To:Subject:Date:In-Reply-To:References:From; b=edUMUSywx2ZLFDn9x7c5+AGXKZ9mfENlqQgPaFpKkGXj9bswE5loeLauUPJVnMmI7 Pwf+16K674z0qdRU+dunHTk5NhrcInrUK/jDHEHl9WOIctZRfpbyYDKVDOj9FFv3VE MNMefzwaweiHfVvC1CO29vKdYZHU9aB17erdSRfM= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:57 +0200 Message-Id: <20191108205409.18845-13-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 12/24] libcamera: buffer: Add const accessor to Buffer planes 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-List-Received-Date: Fri, 08 Nov 2019 20:54:27 -0000 In order to inspect planes of a const Buffer, add a const accessor. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- include/libcamera/buffer.h | 1 + src/libcamera/buffer.cpp | 6 ++++++ 2 files changed, 7 insertions(+) diff --git a/include/libcamera/buffer.h b/include/libcamera/buffer.h index 7b657509ab5f..7a070205f0b5 100644 --- a/include/libcamera/buffer.h +++ b/include/libcamera/buffer.h @@ -43,6 +43,7 @@ private: class BufferMemory final { public: + const std::vector &planes() const { return planes_; } std::vector &planes() { return planes_; } private: diff --git a/src/libcamera/buffer.cpp b/src/libcamera/buffer.cpp index 4407201bd81c..960eeb8f7d19 100644 --- a/src/libcamera/buffer.cpp +++ b/src/libcamera/buffer.cpp @@ -181,6 +181,12 @@ void *Plane::mem() * image format is multi-planar. */ +/** + * \fn BufferMemory::planes() const + * \brief Retrieve the planes within the buffer + * \return A const reference to a vector holding all Planes within the buffer + */ + /** * \fn BufferMemory::planes() * \brief Retrieve the planes within the buffer From patchwork Fri Nov 8 20:53:58 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2309 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 36E086151D for ; Fri, 8 Nov 2019 21:54:27 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id BF80D31D; Fri, 8 Nov 2019 21:54:26 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246466; bh=kMklr1t3TFAoq4LOKrvO52dXWss1jMcLkT0p/Hfqflw=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=UN7W47GAC9IquZzPBLdYQaczK2XxHQkjjLa15VEewcegdH+wSNZiDquUrUH92xsFt 4lAG4nr0vDayf/Eo2lnOd0m6NuvWoUTFjqF5pYibKuV94o4xsTa8wDSJVifw3J2E8F luQDK19hk2JlpbcscByohZqfti/aHFDtRIOAWNZM= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:58 +0200 Message-Id: <20191108205409.18845-14-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 13/24] ipa: Define serialized controls 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-List-Received-Date: Fri, 08 Nov 2019 20:54:27 -0000 From: Jacopo Mondi Define data structures to be used during interaction between IPA modules and pipeline handlers to serialize control lists and control info maps. Signed-off-by: Jacopo Mondi Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/ipa/ipa_controls.h | 54 ++++++++++ include/ipa/meson.build | 1 + src/libcamera/ipa_controls.cpp | 187 +++++++++++++++++++++++++++++++++ src/libcamera/meson.build | 1 + 4 files changed, 243 insertions(+) create mode 100644 include/ipa/ipa_controls.h create mode 100644 src/libcamera/ipa_controls.cpp diff --git a/include/ipa/ipa_controls.h b/include/ipa/ipa_controls.h new file mode 100644 index 000000000000..de3a017b0179 --- /dev/null +++ b/include/ipa/ipa_controls.h @@ -0,0 +1,54 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * ipa_controls.h - IPA Control handling + */ +#ifndef __LIBCAMERA_IPA_CONTROLS_H__ +#define __LIBCAMERA_IPA_CONTROLS_H__ + +#ifdef __cplusplus +extern "C" { +#endif + +#define IPA_CONTROLS_FORMAT_VERSION 1 + +struct ipa_controls_header { + uint32_t version; + uint32_t handle; + uint32_t entries; + uint32_t size; + uint32_t data_offset; + uint32_t reserved[3]; +}; + +struct ipa_control_value_entry { + uint32_t id; + uint32_t type; + uint32_t count; + uint32_t offset; +}; + +struct ipa_control_range_entry { + uint32_t id; + uint32_t type; + uint32_t offset; + uint32_t padding[1]; +}; + +union ipa_control_value_data { + bool b; + int32_t i32; + int64_t i64; +}; + +struct ipa_control_range_data { + union ipa_control_value_data min; + union ipa_control_value_data max; +}; + +#ifdef __cplusplus +} +#endif + +#endif /* __LIBCAMERA_IPA_CONTROLS_H__ */ diff --git a/include/ipa/meson.build b/include/ipa/meson.build index a0ce96ba96c3..695a4183a0e8 100644 --- a/include/ipa/meson.build +++ b/include/ipa/meson.build @@ -1,4 +1,5 @@ libcamera_ipa_api = files([ + 'ipa_controls.h', 'ipa_interface.h', 'ipa_module_info.h', ]) diff --git a/src/libcamera/ipa_controls.cpp b/src/libcamera/ipa_controls.cpp new file mode 100644 index 000000000000..bbcc5301cd59 --- /dev/null +++ b/src/libcamera/ipa_controls.cpp @@ -0,0 +1,187 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * ipa_controls.h - IPA control handling + */ + +/** + * \file ipa_controls.h + * \brief Type definitions for serialized controls + * + * This file defines binary formats to store ControlList and ControlInfoMap + * instances in contiguous, self-contained memory areas called control packets. + * It describes the layout of the packets through a set of C structures. These + * formats shall be used when serializing ControlList and ControlInfoMap to + * transfer them through the IPA C interface and IPA IPC transports. + * + * A control packet contains a list of entries, each of them describing a single + * control range or control value. The packet starts with a fixed-size header + * described by the ipa_controls_header structure, followed by an array of + * fixed-size entries. Each entry is associated with data, stored either + * directly in the entry, or in a data section after the entries array. + * + * The following diagram describes the layout of the ControlList packet. + * + * ~~~~ + * +-------------------------+ . . + * Header / | ipa_controls_header | | | + * | | | | | + * \ | | | | + * +-------------------------+ | | + * / | ipa_control_value_entry | | hdr.data_offset | + * | | #0 | | | + * Control | +-------------------------+ | | + * value | | ... | | | + * entries | +-------------------------+ | | + * | | ipa_control_value_entry | | hdr.size | + * \ | #hdr.entries - 1 | | | + * +-------------------------+ | | + * | empty space (optional) | | | + * +-------------------------+ <--´ . | + * / | ... | | entry[n].offset | + * Data | | ... | | | + * section | | value data for entry #n | <-----´ | + * \ | ... | | + * +-------------------------+ | + * | empty space (optional) | | + * +-------------------------+ <-------------------------´ + * ~~~~ + * + * The packet header contains the size of the packet, the number of entries, and + * the offset from the beginning of the packet to the data section. The packet + * entries array immediately follows the header. The data section starts at the + * offset ipa_controls_header::data_offset from the beginning of the packed, and + * shall be aligned to a multiple of 8 bytes. + * + * Entries are described by the ipa_control_value_entry structure. They contain + * the numerical ID of the control, its type, and the number of control values. + * The control values are stored in the platform's native format. + * + * The control values are stored in the data section in the platform's native + * format. The ipa_control_value_entry::offset field stores the offset from the + * beginning of the data section to the values. + * + * All control values in the data section shall be stored in the same order as + * the controls in the entries array, shall be aligned to a multiple of 8 bytes, + * and shall be contiguous in memory. + * + * Empty spaces may be present between the end of the entries array and the + * data section, and after the data section. They shall be ignored when parsing + * the packet. + * + * The following diagram describes the layout of the ControlInfoMap packet. + * + * ~~~~ + * +-------------------------+ . . + * Header / | ipa_controls_header | | | + * | | | | | + * \ | | | | + * +-------------------------+ | | + * / | ipa_control_range_entry | | hdr.data_offset | + * | | #0 | | | + * Control | +-------------------------+ | | + * range | | ... | | | + * entries | +-------------------------+ | | + * | | ipa_control_range_entry | | hdr.size | + * \ | #hdr.entries - 1 | | | + * +-------------------------+ | | + * | empty space (optional) | | | + * +-------------------------+ <--´ . | + * / | ... | | entry[n].offset | + * Data | | ... | | | + * section | | range data for entry #n | <-----´ | + * \ | ... | | + * +-------------------------+ | + * | empty space (optional) | | + * +-------------------------+ <-------------------------´ + * ~~~~ + * + * The packet header is identical to the ControlList packet header. + * + * Entries are described by the ipa_control_range_entry structure. They contain + * the numerical ID and type of the control. The control range data is stored + * in the data section as described by the ipa_control_range_data structure. + * The ipa_control_range_entry::offset field stores the offset from the + * beginning of the data section to the range data. + * + * Range data in the data section shall be stored in the same order as the + * entries array, shall be aligned to a multiple of 8 bytes, and shall be + * contiguous in memory. + * + * As for the ControlList packet, empty spaces may be present between the end of + * the entries array and the data section, and after the data section. They + * shall be ignored when parsing the packet. + */ + +/** + * \def IPA_CONTROLS_FORMAT_VERSION + * \brief The current control serialization format version + */ + +/** + * \struct ipa_controls_header + * \brief Serialized control packet header + * \var ipa_controls_header::version + * Control packet format version number (shall be IPA_CONTROLS_FORMAT_VERSION) + * \var ipa_controls_header::handle + * For ControlInfoMap packets, this field contains a unique non-zero handle + * generated when the ControlInfoMap is serialized. For ControlList packets, + * this field contains the handle of the corresponding ControlInfoMap. + * \var ipa_controls_header::entries + * Number of entries in the packet + * \var ipa_controls_header::size + * The total packet size in bytes + * \var ipa_controls_header::data_offset + * Offset in bytes from the beginning of the packet of the data section start + * \var ipa_controls_header::reserved + * Reserved for future extensions + */ + +/** + * \struct ipa_control_value_entry + * \brief Description of a serialized ControlValue entry + * \var ipa_control_value_entry::id + * The numerical ID of the control + * \var ipa_control_value_entry::type + * The type of the control (defined by enum ControlType) + * \var ipa_control_value_entry::count + * The number of control array entries for array controls (1 otherwise) + * \var ipa_control_value_entry::offset + * The offset in bytes from the beginning of the data section to the control + * value data (shall be a multiple of 8 bytes). + */ + +/** + * \struct ipa_control_range_entry + * \brief Description of a serialized ControlRange entry + * \var ipa_control_range_entry::id + * The numerical ID of the control + * \var ipa_control_range_entry::type + * The type of the control (defined by enum ControlType) + * \var ipa_control_range_entry::offset + * The offset in bytes from the beginning of the data section to the control + * range data (shall be a multiple of 8 bytes) + * \var ipa_control_range_entry::padding + * Padding bytes (shall be set to 0) + */ + +/** + * \union ipa_control_value_data + * \brief Serialized control value + * \var ipa_control_value_data::b + * Value for ControlTypeBool controls + * \var ipa_control_value_data::i32 + * Value for ControlTypeInteger32 controls + * \var ipa_control_value_data::i64 + * Value for ControlTypeInteger64 controls + */ + +/** + * \struct ipa_control_range_data + * \brief Serialized control range + * \var ipa_control_range_data::min + * The control minimum value + * \var ipa_control_range_data::max + * The control maximum value + */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index afbca76968f9..be0cd53f6f10 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -14,6 +14,7 @@ libcamera_sources = files([ 'event_notifier.cpp', 'formats.cpp', 'geometry.cpp', + 'ipa_controls.cpp', 'ipa_interface.cpp', 'ipa_manager.cpp', 'ipa_module.cpp', From patchwork Fri Nov 8 20:53:59 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2310 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8749E6153B for ; Fri, 8 Nov 2019 21:54:27 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2338FA2A; Fri, 8 Nov 2019 21:54:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246467; bh=82aMwBs2We3GVW0iOAbGs0JATI6AI1mxiO9lrnljP9w=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=AGIt2ic9/3bPsM1cW+9IQQHmpcqjpKzzxd/dXupc9r26dOkF+iUp8vUzVscuukxx1 A4GY5gwwkehxEE+chr+upFoYnjyC+CzLuUT6H7dMLRTd6GonrxBpbZtxjJbJdT6TCy 0IUPGfAfRNbTDz04PSR5GHrBmrTsC2kuJZ7ezb2A= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:53:59 +0200 Message-Id: <20191108205409.18845-15-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 14/24] libcamera: Add ByteStreamBuffer 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-List-Received-Date: Fri, 08 Nov 2019 20:54:28 -0000 From: Jacopo Mondi The ByteStreamBuffer class wraps a memory area, expected to be allocated by the user of the class and provides operations to perform sequential access in read and write modes. Signed-off-by: Jacopo Mondi Signed-off-by: Laurent Pinchart --- src/libcamera/byte_stream_buffer.cpp | 286 +++++++++++++++++++++ src/libcamera/include/byte_stream_buffer.h | 63 +++++ src/libcamera/include/meson.build | 1 + src/libcamera/meson.build | 1 + 4 files changed, 351 insertions(+) create mode 100644 src/libcamera/byte_stream_buffer.cpp create mode 100644 src/libcamera/include/byte_stream_buffer.h diff --git a/src/libcamera/byte_stream_buffer.cpp b/src/libcamera/byte_stream_buffer.cpp new file mode 100644 index 000000000000..23d624dd0a06 --- /dev/null +++ b/src/libcamera/byte_stream_buffer.cpp @@ -0,0 +1,286 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * byte_stream_buffer.cpp - Byte stream buffer + */ + +#include "byte_stream_buffer.h" + +#include +#include + +#include "log.h" + +namespace libcamera { + +LOG_DEFINE_CATEGORY(Serialization); + +/** + * \file byte_stream_buffer.h + * \brief Managed memory container for serialized data + */ + +/** + * \class ByteStreamBuffer + * \brief Wrap a memory buffer and provide sequential data read and write + * + * The ByteStreamBuffer class wraps a memory buffer and exposes sequential read + * and write operation with integrated boundary checks. Access beyond the end + * of the buffer are blocked and logged, allowing error checks to take place at + * the of of access operations instead of at each access. This simplifies + * serialization and deserialization of data. + * + * A byte stream buffer is created with a base memory pointer and a size. If the + * memory pointer is const, the buffer operates in read-only mode, and write + * operations are denied. Otherwise the buffer operates in write-only mode, and + * read operations are denied. + * + * Once a buffer is created, data is read or written with read() and write() + * respectively. Access is strictly sequential, the buffer keeps track of the + * current access location and advances it automatically. Reading or writing + * the same location multiple times is thus not possible. Bytes may also be + * skipped with the skip() method. + * + * The ByteStreamBuffer also supports carving out pieces of memory into other + * ByteStreamBuffer instances. Like a read or write operation, a carveOut() + * advances the internal access location, but allows the carved out memory to + * be accessed at a later time. + * + * All accesses beyond the end of the buffer (read, write, skip or carve out) + * are blocked. The first of such accesses causes a message to be logged, and + * the buffer being marked as having overflown. If the buffer has been carved + * out from a parent buffer, the parent buffer is also marked as having + * overflown. Any later access on an overflown buffer is blocked. The buffer + * overflow status can be checked with the overflow() method. + */ + +/** + * \brief Construct a read ByteStreamBuffer from the memory area \a base + * of \a size + * \param[in] base The address of the memory area to wrap + * \param[in] size The size of the memory area to wrap + */ +ByteStreamBuffer::ByteStreamBuffer(const uint8_t *base, size_t size) + : parent_(nullptr), base_(base), size_(size), overflow_(false), + read_(base), write_(nullptr) +{ +} + +/** + * \brief Construct a write ByteStreamBuffer from the memory area \a base + * of \a size + * \param[in] base The address of the memory area to wrap + * \param[in] size The size of the memory area to wrap + */ +ByteStreamBuffer::ByteStreamBuffer(uint8_t *base, size_t size) + : parent_(nullptr), base_(base), size_(size), overflow_(false), + read_(nullptr), write_(base) +{ +} + +/** + * \brief Construct a ByteStreamBuffer from the contents of \a other using move + * semantics + * \param[in] other The other buffer + * + * After the move construction the \a other buffer is invalidated. Any attempt + * to access its contents will be considered as an overflow. + */ +ByteStreamBuffer::ByteStreamBuffer(ByteStreamBuffer &&other) +{ + *this = std::move(other); +} + +/** + * \brief Replace the contents of the buffer with those of \a other using move + * semantics + * \param[in] other The other buffer + * + * After the assignment the \a other buffer is invalidated. Any attempt to + * access its contents will be considered as an overflow. + */ +ByteStreamBuffer &ByteStreamBuffer::operator=(ByteStreamBuffer &&other) +{ + parent_ = other.parent_; + base_ = other.base_; + size_ = other.size_; + overflow_ = other.overflow_; + read_ = other.read_; + write_ = other.write_; + + other.parent_ = nullptr; + other.base_ = nullptr; + other.size_ = 0; + other.overflow_ = false; + other.read_ = nullptr; + other.write_ = nullptr; + + return *this; +} + +/** + * \fn ByteStreamBuffer::base() + * \brief Retrieve a pointer to the start location of the managed memory buffer + * \return A pointer to the managed memory buffer + */ + +/** + * \fn ByteStreamBuffer::offset() + * \brief Retrieve the offset of the current access location from the base + * \return The offset in bytes + */ + +/** + * \fn ByteStreamBuffer::size() + * \brief Retrieve the size of the managed memory buffer + * \return The size of managed memory buffer + */ + +/** + * \fn ByteStreamBuffer::overflow() + * \brief Check if the buffer has overflown + * \return True if the buffer has overflow, false otherwise + */ + +void ByteStreamBuffer::setOverflow() +{ + if (parent_) + parent_->setOverflow(); + + overflow_ = true; +} + +/** + * \brief Carve out an area of \a size bytes into a new ByteStreamBuffer + * \param[in] size The size of the newly created memory buffer + * + * This method carves out an area of \a size bytes from the buffer into a new + * ByteStreamBuffer, and returns the new buffer. It operates identically to a + * read or write access from the point of view of the current buffer, but allows + * the new buffer to be read or written at a later time after other read or + * write accesses on the current buffer. + * + * \return A newly created ByteStreamBuffer of \a size + */ +ByteStreamBuffer ByteStreamBuffer::carveOut(size_t size) +{ + if (!size_ || overflow_) + return ByteStreamBuffer(static_cast(nullptr), 0); + + const uint8_t *curr = read_ ? read_ : write_; + if (curr + size > base_ + size_) { + LOG(Serialization, Error) + << "Unable to reserve " << size << " bytes"; + setOverflow(); + + return ByteStreamBuffer(static_cast(nullptr), 0); + } + + if (read_) { + ByteStreamBuffer b(read_, size); + b.parent_ = this; + read_ += size; + return b; + } else { + ByteStreamBuffer b(write_, size); + b.parent_ = this; + write_ += size; + return b; + } +} + +/** + * \brief Skip \a size bytes from the buffer + * \param[in] size The number of bytes to skip + * + * This method skips the next \a size bytes from the buffer. + * + * \return 0 on success, a negative error code otherwise + * \retval -ENOSPC no more space is available in the managed memory buffer + */ +int ByteStreamBuffer::skip(size_t size) +{ + if (overflow_) + return -ENOSPC; + + const uint8_t *curr = read_ ? read_ : write_; + if (curr + size > base_ + size_) { + LOG(Serialization, Error) + << "Unable to skip " << size << " bytes"; + setOverflow(); + + return -ENOSPC; + } + + if (read_) { + read_ += size; + } else { + memset(write_, 0, size); + write_ += size; + } + + return 0; +} + +/** + * \fn template int ByteStreamBuffer::read(T *t) + * \brief Read \a size \a data from the managed memory buffer + * \param[out] t Pointer to the memory containing the read data + * \return 0 on success, a negative error code otherwise + * \retval -EACCES attempting to read from a write buffer + * \retval -ENOSPC no more space is available in the managed memory buffer + */ + +/** + * \fn template int ByteStreamBuffer::write(const T *t) + * \brief Write \a data of \a size to the managed memory buffer + * \param[in] t The data to write to memory + * \return 0 on success, a negative error code otherwise + * \retval -EACCES attempting to write to a read buffer + * \retval -ENOSPC no more space is available in the managed memory buffer + */ + +int ByteStreamBuffer::read(uint8_t *data, size_t size) +{ + if (!read_) + return -EACCES; + + if (overflow_) + return -ENOSPC; + + if (read_ + size > base_ + size_) { + LOG(Serialization, Error) + << "Unable to read " << size << " bytes: out of bounds"; + setOverflow(); + return -ENOSPC; + } + + memcpy(data, read_, size); + read_ += size; + + return 0; +} + +int ByteStreamBuffer::write(const uint8_t *data, size_t size) +{ + if (!write_) + return -EACCES; + + if (overflow_) + return -ENOSPC; + + if (write_ + size > base_ + size_) { + LOG(Serialization, Error) + << "Unable to write " << size << " bytes: no space left"; + setOverflow(); + return -ENOSPC; + } + + memcpy(write_, data, size); + write_ += size; + + return 0; +} + +} /* namespace libcamera */ diff --git a/src/libcamera/include/byte_stream_buffer.h b/src/libcamera/include/byte_stream_buffer.h new file mode 100644 index 000000000000..b5274c62b85e --- /dev/null +++ b/src/libcamera/include/byte_stream_buffer.h @@ -0,0 +1,63 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * byte_stream_buffer.h - Byte stream buffer + */ +#ifndef __LIBCAMERA_BYTE_STREAM_BUFFER_H__ +#define __LIBCAMERA_BYTE_STREAM_BUFFER_H__ + +#include +#include + +namespace libcamera { + +class ByteStreamBuffer +{ +public: + ByteStreamBuffer(const uint8_t *base, size_t size); + ByteStreamBuffer(uint8_t *base, size_t size); + ByteStreamBuffer(ByteStreamBuffer &&other); + ByteStreamBuffer &operator=(ByteStreamBuffer &&other); + + const uint8_t *base() const { return base_; } + uint32_t offset() const { return (write_ ? write_ : read_) - base_; } + size_t size() const { return size_; } + bool overflow() const { return overflow_; } + + ByteStreamBuffer carveOut(size_t size); + int skip(size_t size); + + template + int read(T *t) + { + return read(reinterpret_cast(t), sizeof(*t)); + } + template + int write(const T *t) + { + return write(reinterpret_cast(t), sizeof(*t)); + } + +private: + ByteStreamBuffer(const ByteStreamBuffer &other) = delete; + ByteStreamBuffer &operator=(const ByteStreamBuffer &other) = delete; + + void setOverflow(); + + int read(uint8_t *data, size_t size); + int write(const uint8_t *data, size_t size); + + ByteStreamBuffer *parent_; + + const uint8_t *base_; + size_t size_; + bool overflow_; + + const uint8_t *read_; + uint8_t *write_; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_BYTE_STREAM_BUFFER_H__ */ diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build index 64c2155f90cf..1ff0198662cc 100644 --- a/src/libcamera/include/meson.build +++ b/src/libcamera/include/meson.build @@ -1,4 +1,5 @@ libcamera_headers = files([ + 'byte_stream_buffer.h', 'camera_controls.h', 'camera_sensor.h', 'control_validator.h', diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index be0cd53f6f10..dab2d8ad2649 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -1,6 +1,7 @@ libcamera_sources = files([ 'bound_method.cpp', 'buffer.cpp', + 'byte_stream_buffer.cpp', 'camera.cpp', 'camera_controls.cpp', 'camera_manager.cpp', From patchwork Fri Nov 8 20:54:00 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2311 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id CA96D6153D for ; Fri, 8 Nov 2019 21:54:27 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 776C331D for ; Fri, 8 Nov 2019 21:54:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246467; bh=/o4v3ajYRaJ6lHjiWSB9hct6HF2EVFD4F9/qXSXbj9E=; h=From:To:Subject:Date:In-Reply-To:References:From; b=i6/pLUgwJTlrVd+hnKXbhNRLFhSMRvEapF9SHjiXd+Y8W1UoB1S5xcNFsTJRsJeQi Ca12lBScA1o+kI2x/Fy0QnOXqo4bUdOik5LUxfIJP2mLtAvrI5+emhF9ZoBcrMhv6s XO3cUw/f3JjYdX7Def6h0jJWP2YifluBhzfLtZvE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:54:00 +0200 Message-Id: <20191108205409.18845-16-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 15/24] test: Add ByteStreamBuffer test 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-List-Received-Date: Fri, 08 Nov 2019 20:54:29 -0000 The test exercises the API of the ByteStreamBuffer class in both read and write modes, including carve out buffers. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- test/byte-stream-buffer.cpp | 172 ++++++++++++++++++++++++++++++++++++ test/meson.build | 1 + 2 files changed, 173 insertions(+) create mode 100644 test/byte-stream-buffer.cpp diff --git a/test/byte-stream-buffer.cpp b/test/byte-stream-buffer.cpp new file mode 100644 index 000000000000..bc1d462ebf6f --- /dev/null +++ b/test/byte-stream-buffer.cpp @@ -0,0 +1,172 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2018, Google Inc. + * + * byte_stream_buffer.cpp - ByteStreamBuffer tests + */ + +#include +#include + +#include "byte_stream_buffer.h" +#include "test.h" + +using namespace std; +using namespace libcamera; + +class ByteStreamBufferTest : public Test +{ +protected: + int run() + { + std::array data; + unsigned int i; + uint32_t value; + int ret; + + /* + * Write mode. + */ + ByteStreamBuffer wbuf(data.data(), data.size()); + + if (wbuf.base() != data.data() || wbuf.size() != data.size() || + wbuf.offset() != 0 || wbuf.overflow()) { + cerr << "Write buffer incorrectly constructed" << endl; + return TestFail; + } + + /* Test write. */ + value = 0x12345678; + ret = wbuf.write(&value); + if (ret || wbuf.offset() != 4 || wbuf.overflow() || + *reinterpret_cast(data.data()) != 0x12345678) { + cerr << "Write failed on write buffer" << endl; + return TestFail; + } + + /* Test write carve out. */ + ByteStreamBuffer wco = wbuf.carveOut(10); + if (wco.base() != wbuf.base() + 4 || wco.size() != 10 || + wco.offset() != 0 || wco.overflow() || wbuf.offset() != 14 || + wbuf.overflow()) { + cerr << "Carving out write buffer failed" << endl; + return TestFail; + } + + /* Test write on the carved out buffer. */ + value = 0x87654321; + ret = wco.write(&value); + if (ret || wco.offset() != 4 || wco.overflow() || + *reinterpret_cast(data.data() + 4) != 0x87654321) { + cerr << "Write failed on carve out buffer" << endl; + return TestFail; + } + + if (wbuf.offset() != 14 || wbuf.overflow()) { + cerr << "Write on carve out buffer modified write buffer" << endl; + return TestFail; + } + + /* Test read, this should fail. */ + ret = wbuf.read(&value); + if (!ret || wbuf.overflow()) { + cerr << "Read should fail on write buffer" << endl; + return TestFail; + } + + /* Test overflow on carved out buffer. */ + for (i = 0; i < 2; ++i) { + ret = wco.write(&value); + if (ret < 0) + break; + } + + if (i != 1 || !wco.overflow() || !wbuf.overflow()) { + cerr << "Write on carve out buffer failed to overflow" << endl; + return TestFail; + } + + /* Test reinitialization of the buffer. */ + wbuf = ByteStreamBuffer(data.data(), data.size()); + if (wbuf.overflow() || wbuf.base() != data.data() || + wbuf.offset() != 0) { + cerr << "Write buffer reinitialization failed" << endl; + return TestFail; + } + + /* + * Read mode. + */ + ByteStreamBuffer rbuf(const_cast(data.data()), + data.size()); + + if (rbuf.base() != data.data() || rbuf.size() != data.size() || + rbuf.offset() != 0 || rbuf.overflow()) { + cerr << "Read buffer incorrectly constructed" << endl; + return TestFail; + } + + /* Test read. */ + value = 0; + ret = rbuf.read(&value); + if (ret || rbuf.offset() != 4 || rbuf.overflow() || + value != 0x12345678) { + cerr << "Write failed on write buffer" << endl; + return TestFail; + } + + /* Test read carve out. */ + ByteStreamBuffer rco = rbuf.carveOut(10); + if (rco.base() != rbuf.base() + 4 || rco.size() != 10 || + rco.offset() != 0 || rco.overflow() || rbuf.offset() != 14 || + rbuf.overflow()) { + cerr << "Carving out read buffer failed" << endl; + return TestFail; + } + + /* Test read on the carved out buffer. */ + value = 0; + ret = rco.read(&value); + if (ret || rco.offset() != 4 || rco.overflow() || value != 0x87654321) { + cerr << "Read failed on carve out buffer" << endl; + return TestFail; + } + + if (rbuf.offset() != 14 || rbuf.overflow()) { + cerr << "Read on carve out buffer modified read buffer" << endl; + return TestFail; + } + + /* Test write, this should fail. */ + ret = rbuf.write(&value); + if (!ret || rbuf.overflow()) { + cerr << "Write should fail on read buffer" << endl; + return TestFail; + } + + /* Test overflow on carved out buffer. */ + for (i = 0; i < 2; ++i) { + ret = rco.read(&value); + if (ret < 0) + break; + } + + if (i != 1 || !rco.overflow() || !rbuf.overflow()) { + cerr << "Read on carve out buffer failed to overflow" << endl; + return TestFail; + } + + /* Test reinitialization of the buffer. */ + rbuf = ByteStreamBuffer(const_cast(data.data()), + data.size()); + if (rbuf.overflow() || rbuf.base() != data.data() || + rbuf.offset() != 0) { + cerr << "Read buffer reinitialization failed" << endl; + return TestFail; + } + + return TestPass; + } +}; + +TEST_REGISTER(ByteStreamBufferTest) diff --git a/test/meson.build b/test/meson.build index cf5eb84d20b2..adb5b29e69f3 100644 --- a/test/meson.build +++ b/test/meson.build @@ -19,6 +19,7 @@ public_tests = [ ] internal_tests = [ + ['byte-stream-buffer', 'byte-stream-buffer.cpp'], ['camera-sensor', 'camera-sensor.cpp'], ['event', 'event.cpp'], ['event-dispatcher', 'event-dispatcher.cpp'], From patchwork Fri Nov 8 20:54:01 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2312 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2927E61546 for ; Fri, 8 Nov 2019 21:54:28 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id C3E942D1 for ; Fri, 8 Nov 2019 21:54:27 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246467; bh=xAZfJvSj5CodQ9KbrxTJtUQ38Gq1sAdzUb98Xlhq3YE=; h=From:To:Subject:Date:In-Reply-To:References:From; b=PcgWvkPGDes3MhZTQJD5MDr0CW9EFWYVer7CMX+JCkhsnQrQe/Sl6UNV4p66TcQ7Y XxlUtQ8b36s8Y8GnNs3Y3rlAcExNsVfpoqrWBMq1b3gkz21AXUGFKF4t/pzkpdV0hl 1R8vsouCgGtnAs1QEEb+GziFnGxyO4UaXL1q0P/o= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:54:01 +0200 Message-Id: <20191108205409.18845-17-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 16/24] libcamera: Add controls serializer 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-List-Received-Date: Fri, 08 Nov 2019 20:54:29 -0000 Add a new ControlSerializer helper to serialize and deserialize ControlInfoMap and ControlList instances. This will be used to implement the C IPA protocol and the communication with IPA through IPC. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/control_serializer.cpp | 501 +++++++++++++++++++++ src/libcamera/include/control_serializer.h | 52 +++ src/libcamera/include/meson.build | 1 + src/libcamera/meson.build | 1 + 4 files changed, 555 insertions(+) create mode 100644 src/libcamera/control_serializer.cpp create mode 100644 src/libcamera/include/control_serializer.h diff --git a/src/libcamera/control_serializer.cpp b/src/libcamera/control_serializer.cpp new file mode 100644 index 000000000000..5fe096128e49 --- /dev/null +++ b/src/libcamera/control_serializer.cpp @@ -0,0 +1,501 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * control_serializer.cpp - Control (de)serializer + */ + +#include "control_serializer.h" + +#include +#include +#include + +#include +#include +#include + +#include "byte_stream_buffer.h" +#include "log.h" + +/** + * \file control_serializer.h + * \brief Serialization and deserialization helpers for controls + */ + +namespace libcamera { + +LOG_DEFINE_CATEGORY(Serializer) + +namespace { + +static constexpr size_t ControlValueSize[] = { + [ControlTypeNone] = 1, + [ControlTypeBool] = sizeof(bool), + [ControlTypeInteger32] = sizeof(int32_t), + [ControlTypeInteger64] = sizeof(int64_t), +}; + +} /* namespace */ + +/** + * \class ControlSerializer + * \brief Serializer and deserializer for control-related classes + * + * The control serializer is a helper to serialize and deserialize + * ControlInfoMap and ControlValue instances for the purpose of communication + * with IPA modules. + * + * Neither the ControlInfoMap nor the ControlList are self-contained data + * container. ControlInfoMap references an external ControlId in each of its + * entries, and ControlList references a ControlInfoMap for the purpose of + * validation. Serializing and deserializing those objects thus requires a + * context that maintains the associations between them. The control serializer + * fulfils this task. + * + * ControlInfoMap instances can be serialized on their own, but require + * ControlId instances to be provided at deserialization time. The serializer + * recreates those ControlId instances and stores them in an internal cache, + * from which the ControlInfoMap is populated. + * + * ControlList instances need to be associated with a ControlInfoMap when + * deserialized. To make this possible, the control lists are serialized with a + * handle to their ControlInfoMap, and the map is looked up from the handle at + * deserialization time. To make this possible, the serializer assigns a + * numerical handle to ControlInfoList instances when they are serialized, and + * stores the mapping between handle and ControlInfoList both when serializing + * (for the pipeline handler side) and deserializing (for the IPA side) them. + * This mapping is used when serializing a ControlList to include the + * corresponding ControlInfoMap handle in the binary data, and when + * deserializing to retrieve the corresponding ControlInfoMap. + * + * In order to perform those tasks, the serializer keeps an internal state that + * needs to be properly populated. This mechanism requires the ControlInfoMap + * corresponding to a ControlList to have been serialized or deserialized + * before the ControlList is serialized or deserialized. Failure to comply with + * that constraint results in serialization or deserialization failure of the + * ControlList. + * + * The serializer can be reset() to clear its internal state. This may be + * performed when reconfiguring an IPA to avoid constant growth of the internal + * state, especially if the contents of the ControlInfoMap instances change at + * that time. A reset of the serializer invalidates all ControlList and + * ControlInfoMap that have been previously deserialized. The caller shall thus + * proceed with care to avoid stale references. + */ + +/** + * \brief Reset the serializer + * + * Reset the internal state of the serializer. This invalidates all the + * ControlList and ControlInfoMap that have been previously deserialized. + */ +void ControlSerializer::reset() +{ + serial_ = 0; + + infoMapHandles_.clear(); + infoMaps_.clear(); + controlIds_.clear(); +} + +size_t ControlSerializer::binarySize(const ControlValue &value) +{ + return ControlValueSize[value.type()]; +} + +size_t ControlSerializer::binarySize(const ControlRange &range) +{ + return binarySize(range.min()) + binarySize(range.max()); +} + +/** + * \brief Retrieve the size in bytes required to serialize a ControlInfoMap + * \param[in] info The control info map + * + * Compute and return the size in bytes required to store the serialized + * ControlInfoMap. + * + * \return The size in bytes required to store the serialized ControlInfoMap + */ +size_t ControlSerializer::binarySize(const ControlInfoMap &info) +{ + size_t size = sizeof(struct ipa_controls_header) + + info.size() * sizeof(struct ipa_control_range_entry); + + for (const auto &ctrl : info) + size += binarySize(ctrl.second); + + return size; +} + +/** + * \brief Retrieve the size in bytes required to serialize a ControlList + * \param[in] list The control list + * + * Compute and return the size in bytes required to store the serialized + * ControlList. + * + * \return The size in bytes required to store the serialized ControlList + */ +size_t ControlSerializer::binarySize(const ControlList &list) +{ + size_t size = sizeof(struct ipa_controls_header) + + list.size() * sizeof(struct ipa_control_value_entry); + + for (const auto &ctrl : list) + size += binarySize(ctrl.second); + + return size; +} + +void ControlSerializer::store(const ControlValue &value, + ByteStreamBuffer &buffer) +{ + switch (value.type()) { + case ControlTypeBool: { + bool data = value.get(); + buffer.write(&data); + break; + } + + case ControlTypeInteger32: { + int32_t data = value.get(); + buffer.write(&data); + break; + } + + case ControlTypeInteger64: { + uint64_t data = value.get(); + buffer.write(&data); + break; + } + + default: + break; + } +} + +void ControlSerializer::store(const ControlRange &range, + ByteStreamBuffer &buffer) +{ + store(range.min(), buffer); + store(range.max(), buffer); +} + +/** + * \brief Serialize a ControlInfoMap in a buffer + * \param[in] info The control info map to serialize + * \param[in] buffer The memory buffer where to serialize the ControlInfoMap + * + * Serialize the \a info map into the \a buffer using the serialization format + * defined by the IPA context interface in ipa_controls.h. + * + * The serializer stores a reference to the \a info internally. The caller + * shall ensure that \a info stays valid until the serializer is reset(). + * + * \return 0 on success, a negative error code otherwise + * \retval -ENOSPC Not enough space is available in the buffer + */ +int ControlSerializer::serialize(const ControlInfoMap &info, + ByteStreamBuffer &buffer) +{ + /* Compute entries and data required sizes. */ + size_t entriesSize = info.size() * sizeof(struct ipa_control_range_entry); + size_t valuesSize = 0; + for (const auto &ctrl : info) + valuesSize += binarySize(ctrl.second); + + /* Prepare the packet header, assign a handle to the ControlInfoMap. */ + struct ipa_controls_header hdr; + hdr.version = IPA_CONTROLS_FORMAT_VERSION; + hdr.handle = ++serial_; + hdr.entries = info.size(); + hdr.size = sizeof(hdr) + entriesSize + valuesSize; + hdr.data_offset = sizeof(hdr) + entriesSize; + + buffer.write(&hdr); + + /* + * Serialize all entries. + * \todo Serialize the control name too + */ + ByteStreamBuffer entries = buffer.carveOut(entriesSize); + ByteStreamBuffer values = buffer.carveOut(valuesSize); + + for (const auto &ctrl : info) { + const ControlId *id = ctrl.first; + const ControlRange &range = ctrl.second; + + struct ipa_control_range_entry entry; + entry.id = id->id(); + entry.type = id->type(); + entry.offset = values.offset(); + entries.write(&entry); + + store(range, values); + } + + if (buffer.overflow()) + return -ENOSPC; + + /* + * Store the map to handle association, to be used to serialize and + * deserialize control lists. + */ + infoMapHandles_[&info] = hdr.handle; + + return 0; +} + +/** + * \brief Serialize a ControlList in a buffer + * \param[in] list The control list to serialize + * \param[in] buffer The memory buffer where to serialize the ControlList + * + * Serialize the \a list into the \a buffer using the serialization format + * defined by the IPA context interface in ipa_controls.h. + * + * \return 0 on success, a negative error code otherwise + * \retval -ENOSPC Not enough space is available in the buffer + */ +int ControlSerializer::serialize(const ControlList &list, + ByteStreamBuffer &buffer) +{ + /* + * Find the ControlInfoMap handle for the ControlList if it has one, or + * use 0 for ControlList without a ControlInfoMap. + */ + unsigned int infoMapHandle; + if (list.infoMap()) { + auto iter = infoMapHandles_.find(list.infoMap()); + if (iter == infoMapHandles_.end()) { + LOG(Serializer, Error) + << "Can't serialize ControlList: unknown ControlInfoMap"; + return -ENOENT; + } + + infoMapHandle = iter->second; + } else { + infoMapHandle = 0; + } + + size_t entriesSize = list.size() * sizeof(struct ipa_control_value_entry); + size_t valuesSize = 0; + for (const auto &ctrl : list) + valuesSize += binarySize(ctrl.second); + + /* Prepare the packet header. */ + struct ipa_controls_header hdr; + hdr.version = IPA_CONTROLS_FORMAT_VERSION; + hdr.handle = infoMapHandle; + hdr.entries = list.size(); + hdr.size = sizeof(hdr) + entriesSize + valuesSize; + hdr.data_offset = sizeof(hdr) + entriesSize; + + buffer.write(&hdr); + + ByteStreamBuffer entries = buffer.carveOut(entriesSize); + ByteStreamBuffer values = buffer.carveOut(valuesSize); + + /* Serialize all entries. */ + for (const auto &ctrl : list) { + unsigned int id = ctrl.first; + const ControlValue &value = ctrl.second; + + struct ipa_control_value_entry entry; + entry.id = id; + entry.count = 1; + entry.type = value.type(); + entry.offset = values.offset(); + entries.write(&entry); + + store(value, values); + } + + if (buffer.overflow()) + return -ENOSPC; + + return 0; +} + +template<> +ControlValue ControlSerializer::load(ControlType type, + ByteStreamBuffer &b) +{ + switch (type) { + case ControlTypeBool: { + bool value; + b.read(&value); + return ControlValue(value); + } + + case ControlTypeInteger32: { + int32_t value; + b.read(&value); + return ControlValue(value); + } + + case ControlTypeInteger64: { + int64_t value; + b.read(&value); + return ControlValue(value); + } + + default: + return ControlValue(); + } +} + +template<> +ControlRange ControlSerializer::load(ControlType type, + ByteStreamBuffer &b) +{ + ControlValue min = load(type, b); + ControlValue max = load(type, b); + + return ControlRange(min, max); +} + +/** + * \fn template T ControlSerializer::deserialize(ByteStreamBuffer &buffer) + * \brief Deserialize an object from a binary buffer + * \param[in] buffer The memory buffer that contains the object + * + * This method is only valid when specialized for ControlInfoMap or + * ControlList. Any other typename \a T is not supported. + */ + +/** + * \brief Deserialize a ControlInfoMap from a binary buffer + * \param[in] buffer The memory buffer that contains the serialized map + * + * Re-construct a ControlInfoMap from a binary \a buffer containing data + * serialized using the serialize() method. + * + * \return The deserialized ControlInfoMap + */ +template<> +ControlInfoMap ControlSerializer::deserialize(ByteStreamBuffer &buffer) +{ + struct ipa_controls_header hdr; + buffer.read(&hdr); + + if (hdr.version != IPA_CONTROLS_FORMAT_VERSION) { + LOG(Serializer, Error) + << "Unsupported controls format version " + << hdr.version; + return {}; + } + + ByteStreamBuffer entries = buffer.carveOut(hdr.data_offset - sizeof(hdr)); + ByteStreamBuffer values = buffer.carveOut(hdr.size - hdr.data_offset); + + if (buffer.overflow()) + return {}; + + ControlInfoMap::Map ctrls; + + for (unsigned int i = 0; i < hdr.entries; ++i) { + struct ipa_control_range_entry entry; + entries.read(&entry); + + /* Create and cache the individual ControlId. */ + ControlType type = static_cast(entry.type); + controlIds_.emplace_back(utils::make_unique(entry.id, "", type)); + + if (entry.offset != values.offset()) { + LOG(Serializer, Error) + << "Bad data, entry offset mismatch (entry " + << i << ")"; + return {}; + } + + /* Create and store the ControlRange. */ + ctrls.emplace(controlIds_.back().get(), + load(type, values)); + } + + /* + * Create the ControlInfoMap in the cache, and store the map to handle + * association. + */ + ControlInfoMap &map = infoMaps_[hdr.handle] = std::move(ctrls); + infoMapHandles_[&map] = hdr.handle; + + return map; +} + +/** + * \brief Deserialize a ControlList from a binary buffer + * \param[in] buffer The memory buffer that contains the serialized list + * + * Re-construct a ControlList from a binary \a buffer containing data + * serialized using the serialize() method. + * + * \return The deserialized ControlList + */ +template<> +ControlList ControlSerializer::deserialize(ByteStreamBuffer &buffer) +{ + struct ipa_controls_header hdr; + buffer.read(&hdr); + + if (hdr.version != IPA_CONTROLS_FORMAT_VERSION) { + LOG(Serializer, Error) + << "Unsupported controls format version " + << hdr.version; + return {}; + } + + ByteStreamBuffer entries = buffer.carveOut(hdr.data_offset - sizeof(hdr)); + ByteStreamBuffer values = buffer.carveOut(hdr.size - hdr.data_offset); + + if (buffer.overflow()) + return {}; + + /* + * Retrieve the ControlInfoMap associated with the ControlList based on + * its ID. The mapping between infoMap and ID is set up when serializing + * or deserializing ControlInfoMap. If no mapping is found (which is + * currently the case for ControlList related to libcamera controls), + * use the global control::control idmap. + */ + const ControlInfoMap *infoMap; + if (hdr.handle) { + auto iter = std::find_if(infoMapHandles_.begin(), infoMapHandles_.end(), + [&](decltype(infoMapHandles_)::value_type &entry) { + return entry.second == hdr.handle; + }); + if (iter == infoMapHandles_.end()) { + LOG(Serializer, Error) + << "Can't deserialize ControlList: unknown ControlInfoMap"; + return {}; + } + + infoMap = iter->first; + } else { + infoMap = nullptr; + } + + ControlList ctrls(infoMap ? infoMap->idmap() : controls::controls); + + for (unsigned int i = 0; i < hdr.entries; ++i) { + struct ipa_control_value_entry entry; + entries.read(&entry); + + if (entry.offset != values.offset()) { + LOG(Serializer, Error) + << "Bad data, entry offset mismatch (entry " + << i << ")"; + return {}; + } + + ControlType type = static_cast(entry.type); + ctrls.set(entry.id, load(type, values)); + } + + return ctrls; +} + +} /* namespace libcamera */ diff --git a/src/libcamera/include/control_serializer.h b/src/libcamera/include/control_serializer.h new file mode 100644 index 000000000000..bb3cb8e7b904 --- /dev/null +++ b/src/libcamera/include/control_serializer.h @@ -0,0 +1,52 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * control_serializer.h - Control (de)serializer + */ +#ifndef __LIBCAMERA_CONTROL_SERIALIZER_H__ +#define __LIBCAMERA_CONTROL_SERIALIZER_H__ + +#include +#include +#include + +#include + +namespace libcamera { + +class ByteStreamBuffer; + +class ControlSerializer +{ +public: + void reset(); + + static size_t binarySize(const ControlInfoMap &info); + static size_t binarySize(const ControlList &list); + + int serialize(const ControlInfoMap &info, ByteStreamBuffer &buffer); + int serialize(const ControlList &list, ByteStreamBuffer &buffer); + + template + T deserialize(ByteStreamBuffer &buffer); + +private: + static size_t binarySize(const ControlValue &value); + static size_t binarySize(const ControlRange &range); + + static void store(const ControlValue &value, ByteStreamBuffer &buffer); + static void store(const ControlRange &range, ByteStreamBuffer &buffer); + + template + T load(ControlType type, ByteStreamBuffer &b); + + unsigned int serial_; + std::vector> controlIds_; + std::map infoMaps_; + std::map infoMapHandles_; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_CONTROL_SERIALIZER_H__ */ diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build index 1ff0198662cc..697294f4b09b 100644 --- a/src/libcamera/include/meson.build +++ b/src/libcamera/include/meson.build @@ -2,6 +2,7 @@ libcamera_headers = files([ 'byte_stream_buffer.h', 'camera_controls.h', 'camera_sensor.h', + 'control_serializer.h', 'control_validator.h', 'device_enumerator.h', 'device_enumerator_sysfs.h', diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index dab2d8ad2649..59cf582580c4 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -7,6 +7,7 @@ libcamera_sources = files([ 'camera_manager.cpp', 'camera_sensor.cpp', 'controls.cpp', + 'control_serializer.cpp', 'control_validator.cpp', 'device_enumerator.cpp', 'device_enumerator_sysfs.cpp', From patchwork Fri Nov 8 20:54:02 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2313 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 7F2CD61516 for ; Fri, 8 Nov 2019 21:54:28 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 1DE17A2A; Fri, 8 Nov 2019 21:54:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246468; bh=n6tFsNdq4kKSyjfbA/24/q0hZV5IBeeMjcD060i+gCY=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=AD6ISf/90phM+YR/EA5dGEpk64R9rV5h7RV9bklo+06bQB5LcSETH2tItSO4CWDZl H8MRmMyyYzLmDz2IxdEM8asxrBMs9rEf+bUBAwJcxNbUNxfWCN7Frzd/n2U6RjyzmW 9XTufEXTTJ3CgPnSTOvShtKWSU0Xm8pPp6ifn5rw= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:54:02 +0200 Message-Id: <20191108205409.18845-18-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 17/24] test: Add control serialization test 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-List-Received-Date: Fri, 08 Nov 2019 20:54:29 -0000 From: Jacopo Mondi Add a test that exercises the ControlSerializer to serialize and deserialize ControlInfoMap and ControlList. Signed-off-by: Jacopo Mondi Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- test/meson.build | 1 + test/serialization/control_serialization.cpp | 161 +++++++++++++++++++ test/serialization/meson.build | 11 ++ test/serialization/serialization_test.cpp | 89 ++++++++++ test/serialization/serialization_test.h | 33 ++++ 5 files changed, 295 insertions(+) create mode 100644 test/serialization/control_serialization.cpp create mode 100644 test/serialization/meson.build create mode 100644 test/serialization/serialization_test.cpp create mode 100644 test/serialization/serialization_test.h diff --git a/test/meson.build b/test/meson.build index adb5b29e69f3..1bb2161dc05a 100644 --- a/test/meson.build +++ b/test/meson.build @@ -8,6 +8,7 @@ subdir('log') subdir('media_device') subdir('pipeline') subdir('process') +subdir('serialization') subdir('stream') subdir('v4l2_subdevice') subdir('v4l2_videodevice') diff --git a/test/serialization/control_serialization.cpp b/test/serialization/control_serialization.cpp new file mode 100644 index 000000000000..adfb498b5bd2 --- /dev/null +++ b/test/serialization/control_serialization.cpp @@ -0,0 +1,161 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * control_serialization.cpp - Serialize and deserialize controls + */ + +#include + +#include +#include +#include + +#include "byte_stream_buffer.h" +#include "control_serializer.h" +#include "serialization_test.h" +#include "test.h" + +using namespace std; +using namespace libcamera; + +class ControlSerializationTest : public SerializationTest +{ +protected: + int run() override + { + ControlSerializer serializer; + ControlSerializer deserializer; + + std::vector infoData; + std::vector listData; + + size_t size; + int ret; + + /* Create a control list with three controls. */ + const ControlInfoMap &infoMap = camera_->controls(); + ControlList list(infoMap); + + list.set(controls::Brightness, 255); + list.set(controls::Contrast, 128); + list.set(controls::Saturation, 50); + + /* + * Serialize the control list, this should fail as the control + * info map hasn't been serialized. + */ + size = serializer.binarySize(list); + listData.resize(size); + ByteStreamBuffer buffer(listData.data(), listData.size()); + + ret = serializer.serialize(list, buffer); + if (!ret) { + cerr << "List serialization without info map should have failed" + << endl; + return TestFail; + } + + if (buffer.overflow() || buffer.offset()) { + cerr << "Failed list serialization modified the buffer" + << endl; + return TestFail; + } + + /* Serialize the control info map. */ + size = serializer.binarySize(infoMap); + infoData.resize(size); + buffer = ByteStreamBuffer(infoData.data(), infoData.size()); + + ret = serializer.serialize(infoMap, buffer); + if (ret < 0) { + cerr << "Failed to serialize ControlInfoMap" << endl; + return TestFail; + } + + if (buffer.overflow()) { + cerr << "Overflow when serializing ControlInfoMap" << endl; + return TestFail; + } + + /* Serialize the control list, this should now succeed. */ + size = serializer.binarySize(list); + listData.resize(size); + buffer = ByteStreamBuffer(listData.data(), listData.size()); + + ret = serializer.serialize(list, buffer); + if (ret) { + cerr << "Failed to serialize ControlList" << endl; + return TestFail; + } + + if (buffer.overflow()) { + cerr << "Overflow when serializing ControlList" << endl; + return TestFail; + } + + /* + * Deserialize the control list, this should fail as the control + * info map hasn't been deserialized. + */ + buffer = ByteStreamBuffer(const_cast(listData.data()), + listData.size()); + + ControlList newList = deserializer.deserialize(buffer); + if (!newList.empty()) { + cerr << "List deserialization without info map should have failed" + << endl; + return TestFail; + } + + if (buffer.overflow()) { + cerr << "Failed list deserialization modified the buffer" + << endl; + return TestFail; + } + + /* Deserialize the control info map and verify the contents. */ + buffer = ByteStreamBuffer(const_cast(infoData.data()), + infoData.size()); + + ControlInfoMap newInfoMap = deserializer.deserialize(buffer); + if (newInfoMap.empty()) { + cerr << "Failed to deserialize ControlInfoMap" << endl; + return TestFail; + } + + if (buffer.overflow()) { + cerr << "Overflow when deserializing ControlInfoMap" << endl; + return TestFail; + } + + if (!equals(infoMap, newInfoMap)) { + cerr << "Deserialized map doesn't match original" << endl; + return TestFail; + } + + /* Deserialize the control list and verify the contents. */ + buffer = ByteStreamBuffer(const_cast(listData.data()), + listData.size()); + + newList = deserializer.deserialize(buffer); + if (newList.empty()) { + cerr << "Failed to deserialize ControlList" << endl; + return TestFail; + } + + if (buffer.overflow()) { + cerr << "Overflow when deserializing ControlList" << endl; + return TestFail; + } + + if (!equals(list, newList)) { + cerr << "Deserialized list doesn't match original" << endl; + return TestFail; + } + + return TestPass; + } +}; + +TEST_REGISTER(ControlSerializationTest) diff --git a/test/serialization/meson.build b/test/serialization/meson.build new file mode 100644 index 000000000000..d78d92e61887 --- /dev/null +++ b/test/serialization/meson.build @@ -0,0 +1,11 @@ +serialization_tests = [ + [ 'control_serialization', 'control_serialization.cpp' ], +] + +foreach t : serialization_tests + exe = executable(t[0], [t[1], 'serialization_test.cpp'], + dependencies : libcamera_dep, + link_with : test_libraries, + include_directories : test_includes_internal) + test(t[0], exe, suite : 'serialization', is_parallel : true) +endforeach diff --git a/test/serialization/serialization_test.cpp b/test/serialization/serialization_test.cpp new file mode 100644 index 000000000000..68e0512a04ca --- /dev/null +++ b/test/serialization/serialization_test.cpp @@ -0,0 +1,89 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * serialization_test.cpp - Base class for serialization tests + */ + +#include "serialization_test.h" + +#include +#include +#include + +#include +#include +#include + +#include "test.h" + +using namespace std; +using namespace libcamera; + +bool SerializationTest::equals(const ControlInfoMap &lhs, const ControlInfoMap &rhs) +{ + std::map rlhs; + std::transform(lhs.begin(), lhs.end(), std::inserter(rlhs, rlhs.end()), + [](const ControlInfoMap::value_type &v) + -> decltype(rlhs)::value_type + { + return { v.first->id(), v.second }; + }); + + std::map rrhs; + std::transform(rhs.begin(), rhs.end(), std::inserter(rrhs, rrhs.end()), + [](const ControlInfoMap::value_type &v) + -> decltype(rrhs)::value_type + { + return { v.first->id(), v.second }; + }); + + if (rlhs == rrhs) + return true; + + cerr << "lhs:" << endl; + for (const auto &value : rlhs) + cerr << "- " << value.first << ": " + << value.second.toString() << endl; + + cerr << "rhs:" << endl; + for (const auto &value : rrhs) + cerr << "- " << value.first << ": " + << value.second.toString() << endl; + + return false; +} + +bool SerializationTest::equals(const ControlList &lhs, const ControlList &rhs) +{ + std::map rlhs; + std::transform(lhs.begin(), lhs.end(), std::inserter(rlhs, rlhs.end()), + [](const std::pair &v) + -> decltype(rlhs)::value_type + { + return { v.first, v.second }; + }); + + std::map rrhs; + std::transform(rhs.begin(), rhs.end(), std::inserter(rrhs, rrhs.end()), + [](const std::pair &v) + -> decltype(rrhs)::value_type + { + return { v.first, v.second }; + }); + + if (rlhs == rrhs) + return true; + + cerr << "lhs:" << endl; + for (const auto &value : rlhs) + cerr << "- " << value.first << ": " + << value.second.toString() << endl; + + cerr << "rhs:" << endl; + for (const auto &value : rrhs) + cerr << "- " << value.first << ": " + << value.second.toString() << endl; + + return false; +} diff --git a/test/serialization/serialization_test.h b/test/serialization/serialization_test.h new file mode 100644 index 000000000000..fe77221ef5d0 --- /dev/null +++ b/test/serialization/serialization_test.h @@ -0,0 +1,33 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * serialization_test.h - Base class for serialization tests + */ +#ifndef __LIBCAMERA_SERIALIZATION_TEST_H__ +#define __LIBCAMERA_SERIALIZATION_TEST_H__ + +#include +#include +#include + +#include "camera_test.h" +#include "test.h" + +using namespace libcamera; + +class SerializationTest : public CameraTest, public Test +{ +public: + SerializationTest() + : CameraTest("VIMC Sensor B") + { + } + + static bool equals(const ControlInfoMap &lhs, + const ControlInfoMap &rhs); + static bool equals(const ControlList &lhs, + const ControlList &rhs); +}; + +#endif /* __LIBCAMERA_SERIALIZATION_TEST_H__ */ From patchwork Fri Nov 8 20:54:03 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2314 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D5D1B60180 for ; Fri, 8 Nov 2019 21:54:28 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 78B9A71D for ; Fri, 8 Nov 2019 21:54:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246468; bh=SeM3/DvuD4bWBGpBt9k2BU9LufAwUKx+ds7visS/VYI=; h=From:To:Subject:Date:In-Reply-To:References:From; b=qHlK4mXewdmo1Jm9nT4mw4eqB5s92DqNNA+cqCEyd2eC75NikNjgmABlA6fKqJpoL G6wxzjc7xzdomsu+Zq7JTohsGA6WVlq8j0PjxTdjtkTZLVTO5QTH9rQTvdOMAhv1tv DGlYewa7B5VbPE927oIfBzpmkQgDp5Yh8QUFeEHY= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:54:03 +0200 Message-Id: <20191108205409.18845-19-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 18/24] ipa: Pass ControlInfoMap references to IPAInterface::configure() 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-List-Received-Date: Fri, 08 Nov 2019 20:54:29 -0000 The IPAInterface::configure() operation receives a map of ControlInfoMap instances. Pass const references instead to avoid copies when not required (the callee can still make manual copies), and to allow for the future serialization layer to keep references to the original object. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/ipa/ipa_interface.h | 2 +- src/ipa/ipa_vimc.cpp | 2 +- src/ipa/rkisp1/rkisp1.cpp | 4 ++-- src/libcamera/pipeline/rkisp1/rkisp1.cpp | 2 +- src/libcamera/proxy/ipa_proxy_linux.cpp | 2 +- 5 files changed, 6 insertions(+), 6 deletions(-) diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h index 8fd658af5fdb..35dc4b4e3165 100644 --- a/include/ipa/ipa_interface.h +++ b/include/ipa/ipa_interface.h @@ -43,7 +43,7 @@ public: virtual int init() = 0; virtual void configure(const std::map &streamConfig, - const std::map &entityControls) = 0; + const std::map &entityControls) = 0; virtual void mapBuffers(const std::vector &buffers) = 0; virtual void unmapBuffers(const std::vector &ids) = 0; diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp index 9fd5212b0381..50ca8dd805fb 100644 --- a/src/ipa/ipa_vimc.cpp +++ b/src/ipa/ipa_vimc.cpp @@ -31,7 +31,7 @@ public: int init() override; void configure(const std::map &streamConfig, - const std::map &entityControls) override {} + const std::map &entityControls) override {} void mapBuffers(const std::vector &buffers) override {} void unmapBuffers(const std::vector &ids) override {} void processEvent(const IPAOperationData &event) override {} diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index d741d5677d0e..41babf0c4140 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -33,7 +33,7 @@ public: int init() override { return 0; } void configure(const std::map &streamConfig, - const std::map &entityControls) override; + const std::map &entityControls) override; void mapBuffers(const std::vector &buffers) override; void unmapBuffers(const std::vector &ids) override; void processEvent(const IPAOperationData &event) override; @@ -62,7 +62,7 @@ private: }; void IPARkISP1::configure(const std::map &streamConfig, - const std::map &entityControls) + const std::map &entityControls) { if (entityControls.empty()) return; diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp index b21cf92435e7..4a583a7a1d7e 100644 --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp @@ -777,7 +777,7 @@ int PipelineHandlerRkISP1::start(Camera *camera) .size = data->stream_.configuration().size, }; - std::map entityControls; + std::map entityControls; entityControls.emplace(0, data->sensor_->controls()); data->ipa_->configure(streamConfig, entityControls); diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp index 27b6639d6312..c7218fb47249 100644 --- a/src/libcamera/proxy/ipa_proxy_linux.cpp +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp @@ -28,7 +28,7 @@ public: int init() override { return 0; } void configure(const std::map &streamConfig, - const std::map &entityControls) override {} + const std::map &entityControls) override {} void mapBuffers(const std::vector &buffers) override {} void unmapBuffers(const std::vector &ids) override {} void processEvent(const IPAOperationData &event) override {} From patchwork Fri Nov 8 20:54:04 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2315 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 2AFF561511 for ; Fri, 8 Nov 2019 21:54:29 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id CC38F2D1 for ; Fri, 8 Nov 2019 21:54:28 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246468; bh=CSTBNOQS+tReWgD9u8nPG595a0IYkFh6xwVf04Fcb50=; h=From:To:Subject:Date:In-Reply-To:References:From; b=lCq5PEFSlj+k6rI/45Yc3geQbFdnEQ5PfDy11/dGHrqMS8w7brYEbrCDx9BcHxjEB SOvx8MVocVrY3iGZ8R0jeJP5ROWLewfEgpb+zzSB2uZwnfEqT9ujPFkpbG+VojNBxi VgdOJ8VTvwfarH9hX7S7Fn/PEbmzr169SbEdyzaE= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:54:04 +0200 Message-Id: <20191108205409.18845-20-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 19/24] ipa: Define a plain C API 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-List-Received-Date: Fri, 08 Nov 2019 20:54:29 -0000 The C++ objects that are expected to convey data through the IPA API will have associated methods that would require IPAs to link to libcamera. Even though the libcamera license allows this, suppliers of closed-source IPAs may have a different interpretation. To ease their mind and clearly separate vendor code and libcamera code, define a plain C IPA API. The corresponding C objects are stored in plain C structures or have their binary format documented, removing the need for linking to libcamera code on the IPA side. The C API adds three new C structures, ipa_context, ipa_context_ops and ipa_callback_ops. The ipa_context_ops and ipa_callback_ops contain function pointers for all the IPA interface methods and signals, respectively. The ipa_context represents a context of operation for the IPA, and is passed to the IPA oparations. The IPAInterface class is retained as it is easier to use than a plain C API for pipeline handlers, and wrappers will be developed to translate between the C and C++ APIs. Switching to the C API internally will be done in a second step. Signed-off-by: Laurent Pinchart Signed-off-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/ipa/ipa_interface.h | 77 +++++++++ src/libcamera/ipa_interface.cpp | 295 ++++++++++++++++++++++++++++---- 2 files changed, 340 insertions(+), 32 deletions(-) diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h index 35dc4b4e3165..3a423e37671f 100644 --- a/include/ipa/ipa_interface.h +++ b/include/ipa/ipa_interface.h @@ -7,6 +7,82 @@ #ifndef __LIBCAMERA_IPA_INTERFACE_H__ #define __LIBCAMERA_IPA_INTERFACE_H__ +#include +#include + +#ifdef __cplusplus +extern "C" { +#endif + +struct ipa_context { + const struct ipa_context_ops *ops; +}; + +struct ipa_stream { + unsigned int id; + unsigned int pixel_format; + unsigned int width; + unsigned int height; +}; + +struct ipa_control_info_map { + unsigned int id; + const uint8_t *data; + size_t size; +}; + +struct ipa_buffer_plane { + int dmabuf; + size_t length; +}; + +struct ipa_buffer { + unsigned int id; + unsigned int num_planes; + struct ipa_buffer_plane planes[3]; +}; + +struct ipa_control_list { + const uint8_t *data; + unsigned int size; +}; + +struct ipa_operation_data { + unsigned int operation; + const uint32_t *data; + unsigned int num_data; + const struct ipa_control_list *lists; + unsigned int num_lists; +}; + +struct ipa_callback_ops { + void (*queue_frame_action)(void *cb_ctx, unsigned int frame, + struct ipa_operation_data &data); +}; + +struct ipa_context_ops { + void (*destroy)(struct ipa_context *ctx); + void (*init)(struct ipa_context *ctx); + void (*register_callbacks)(struct ipa_context *ctx, + const struct ipa_callback_ops *callbacks, + void *cb_ctx); + void (*configure)(struct ipa_context *ctx, + const struct ipa_stream *streams, + unsigned int num_streams, + const struct ipa_control_info_map *maps, + unsigned int num_maps); + void (*map_buffers)(struct ipa_context *ctx, + const struct ipa_buffer *buffers, + size_t num_buffers); + void (*unmap_buffers)(struct ipa_context *ctx, const unsigned int *ids, + size_t num_buffers); + void (*process_event)(struct ipa_context *ctx, + const struct ipa_operation_data *data); +}; + +#ifdef __cplusplus +} + #include #include @@ -53,5 +129,6 @@ public: }; } /* namespace libcamera */ +#endif #endif /* __LIBCAMERA_IPA_INTERFACE_H__ */ diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp index 0571b8e6bf8b..89fce0e8347f 100644 --- a/src/libcamera/ipa_interface.cpp +++ b/src/libcamera/ipa_interface.cpp @@ -11,27 +11,254 @@ * \file ipa_interface.h * \brief Image Processing Algorithm interface * - * Pipeline handlers communicate with IPAs through a C++ interface defined by - * the IPAInterface class. The class defines high-level methods and signals to - * configure the IPA, notify it of events, and receive actions back from the - * IPA. - * - * Pipeline handlers define the set of events and actions used to communicate - * with their IPA. These are collectively referred to as IPA operations and - * define the pipeline handler-specific IPA protocol. Each operation defines the - * data that it carries, and how the data is encoded in the IPAOperationData - * structure. - * - * IPAs can be isolated in a separate process. This implies that all arguments - * to the IPA interface may need to be transferred over IPC. The IPA interface - * thus uses serialisable data types only. The IPA interface defines custom data - * structures that mirror core libcamera structures when the latter are not - * suitable, such as IPAStream to carry StreamConfiguration data. + * Every pipeline handler in libcamera may attach some or all of its cameras to + * an Image Processing Algorithm (IPA) module. An IPA module is developed for a + * specific pipeline handler and each pipeline handler may be compatible with + * multiple IPA implementations, both open and closed source. To support this, + * libcamera communicates with IPA modules through a standard plain C interface. + * + * IPA modules shall expose a public function named ipaCreate() with the + * following prototype. + * + * \code{.c} + * struct ipa_context *ipaCreate(); + * \endcode + * + * The ipaCreate() function creates an instance of an IPA context, which models + * a context of execution for the IPA. IPA modules shall support creating one + * context per camera, as required by their associated pipeline handler. + * + * The IPA module context operations are defined in the struct ipa_context_ops. + * They model a low-level interface to configure the IPA, notify it of events, + * and receive IPA actions through callbacks. An IPA module stores a pointer to + * the operations corresponding to its context in the ipa_context::ops field. + * That pointer is immutable for the lifetime of the context, and may differ + * between different contexts created by the same IPA module. + * + * The IPA interface defines base data types and functions to exchange data. On + * top of this, each pipeline handler is responsible for defining the set of + * events and actions used to communicate with their IPA. These are collectively + * referred to as IPA operations and define the pipeline handler-specific IPA + * protocol. Each operation defines the data that it carries, and how that data + * is encoded in the ipa_context_ops functions arguments. + * + * \todo Add reference to how pipelines shall document their protocol. + * + * IPAs can be isolated in a separate process. This implies that arguments to + * the IPA interface functions may need to be transferred over IPC. All + * arguments use Plain Old Data types and are documented either in the form of C + * data types, or as a textual description of byte arrays for types that can't + * be expressed using C data types (such as arrays of mixed data types). IPA + * modules can thus use the C API without calling into libcamera to access the + * data passed to the IPA context operations. * * Due to IPC, synchronous communication between pipeline handlers and IPAs can * be costly. For that reason, the interface operates asynchronously. This - * implies that methods don't return a status, and that both methods and signals - * may copy their arguments. + * implies that methods don't return a status, and that all methods may copy + * their arguments. + * + * The IPAInterface class is a C++ representation of the ipa_context_ops, using + * C++ data classes provided by libcamera. This is the API exposed to pipeline + * handlers to communicate with IPA modules. IPA modules may use the + * IPAInterface API internally if they want to benefit from the data and helper + * classes offered by libcamera. + */ + +/** + * \struct ipa_context + * \brief IPA module context of execution + * + * This structure models a context of execution for an IPA module. It is + * instantiated by the IPA module ipaCreate() function. IPA modules allocate + * context instances in an implementation-defined way, contexts shall thus be + * destroyed using the ipa_operation::destroy function only. + * + * The ipa_context structure provides a pointer to the IPA context operations. + * It shall otherwise be treated as a constant black-box cookie and passed + * unmodified to the functions defined in struct ipa_context_ops. + * + * IPA modules are expected to extend struct ipa_context by inheriting from it, + * either through structure embedding to model inheritance in plain C, or + * through C++ class inheritance. A simple example of the latter is available + * in the IPAContextWrapper class implementation. + * + * \var ipa_context::ops + * \brief The IPA context operations + */ + +/** + * \struct ipa_stream + * \brief Stream information for the IPA context operations + * + * \var ipa_stream::id + * \brief Identifier for the stream, defined by the IPA protocol + * + * \var ipa_stream::pixel_format + * \brief The stream pixel format, as defined by the PixelFormat class + * + * \var ipa_stream::width + * \brief The stream width in pixels + * + * \var ipa_stream::height + * \brief The stream height in pixels + */ + +/** + * \struct ipa_control_info_map + * \brief ControlInfoMap description for the IPA context operations + * + * \var ipa_control_info_map::id + * \brief Identifier for the ControlInfoMap, defined by the IPA protocol + * + * \var ipa_control_info_map::data + * \brief Pointer to a control packet for the ControlInfoMap + * \sa ipa_controls.h + * + * \var ipa_control_info_map::size + * \brief The size of the control packet in bytes + */ + +/** + * \struct ipa_buffer_plane + * \brief A plane for an ipa_buffer + * + * \var ipa_buffer_plane::dmabuf + * \brief The dmabuf file descriptor for the plane (-1 for unused planes) + * + * \var ipa_buffer_plane::length + * \brief The plane length in bytes (0 for unused planes) + */ + +/** + * \struct ipa_buffer + * \brief Buffer information for the IPA context operations + * + * \var ipa_buffer::id + * \brief The buffer unique ID (see \ref libcamera::IPABuffer::id) + * + * \var ipa_buffer::num_planes + * \brief The number of used planes in the ipa_buffer::planes array + * + * \var ipa_buffer::planes + * \brief The buffer planes (up to 3) + */ + +/** + * \struct ipa_control_list + * \brief ControlList description for the IPA context operations + * + * \var ipa_control_list::data + * \brief Pointer to a control packet for the ControlList + * \sa ipa_controls.h + * + * \var ipa_control_list::size + * \brief The size of the control packet in bytes + */ + +/** + * \struct ipa_operation_data + * \brief IPA operation data for the IPA context operations + * \sa libcamera::IPAOperationData + * + * \var ipa_operation_data::operation + * \brief IPA protocol operation + * + * \var ipa_operation_data::data + * \brief Pointer to the operation data array + * + * \var ipa_operation_data::num_data + * \brief Number of entries in the ipa_operation_data::data array + * + * \var ipa_operation_data::lists + * \brief Pointer to an array of ipa_control_list + * + * \var ipa_operation_data::num_lists + * \brief Number of entries in the ipa_control_list array + */ + +/** + * \struct ipa_callback_ops + * \brief IPA context operations as a set of function pointers + */ + +/** + * \var ipa_callback_ops::queue_frame_action + * \brief Queue an action associated with a frame to the pipeline handler + * \param[in] cb_ctx The callback context registered with + * ipa_context_ops::register_callbacks + * \param[in] frame The frame number + * + * \sa libcamera::IPAInterface::queueFrameAction + */ + +/** + * \struct ipa_context_ops + * \brief IPA context operations as a set of function pointers + * + * To allow for isolation of IPA modules in separate processes, the functions + * defined in the ipa_context_ops structure return only data related to the + * libcamera side of the operations. In particular, error related to the + * libcamera side of the IPC may be returned. Data returned by the IPA, + * including status information, shall be provided through callbacks from the + * IPA to libcamera. + */ + +/** + * \var ipa_context_ops::destroy + * \brief Destroy the IPA context created by the module's ipaCreate() function + * \param[in] ctx The IPA context + */ + +/** + * \var ipa_context_ops::init + * \brief Initialise the IPA context + * \param[in] ctx The IPA context + * + * \sa libcamera::IPAInterface::init() + */ + +/** + * \var ipa_context_ops::register_callbacks + * \brief Register callback operation from the IPA to the pipeline handler + * \param[in] ctx The IPA context + * \param[in] callback The IPA callback operations + * \param[in] cb_ctx The callback context, passed to all callback operations + */ + +/** + * \var ipa_context_ops::configure + * \brief Configure the IPA stream and sensor settings + * \param[in] ctx The IPA context + * + * \sa libcamera::IPAInterface::configure() + */ + +/** + * \var ipa_context_ops::map_buffers + * \brief Map buffers shared between the pipeline handler and the IPA + * \param[in] ctx The IPA context + * \param[in] buffers The buffers to map + * \param[in] num_buffers The number of entries in the \a buffers array + * + * \sa libcamera::IPAInterface::mapBuffers() + */ + +/** + * \var ipa_context_ops::unmap_buffers + * \brief Unmap buffers shared by the pipeline to the IPA + * \param[in] ctx The IPA context + * \param[in] ids The IDs of the buffers to unmap + * \param[in] num_buffers The number of entries in the \a ids array + * + * \sa libcamera::IPAInterface::unmapBuffers() + */ + +/** + * \var ipa_context_ops::process_event + * \brief Process an event from the pipeline handler + * \param[in] ctx The IPA context + * + * \sa libcamera::IPAInterface::processEvent() */ namespace libcamera { @@ -122,26 +349,30 @@ namespace libcamera { /** * \class IPAInterface - * \brief Interface for IPA implementation + * \brief C++ Interface for IPA implementation * - * Every pipeline handler in libcamera may attach all or some of its cameras to - * an Image Processing Algorithm (IPA) module. An IPA module is developed for a - * specific pipeline handler and each pipeline handler may have multiple - * compatible IPA implementations, both open and closed source. + * This pure virtual class defines a C++ API corresponding to the ipa_context, + * ipa_context_ops and ipa_callback_ops API. It is used by pipeline handlers to + * interact with IPA modules, and may be used internally in IPA modules if + * desired to benefit from the data and helper classes provided by libcamera. + * + * Functions defined in the ipa_context_ops structure are mapped to IPAInterface + * methods, while functions defined in the ipa_callback_ops are mapped to + * IPAInterface signals. As with the C API, the IPA C++ interface uses + * serializable data types only. It reuses structures defined by the C API, or + * defines corresponding classes using C++ containers when required. * - * To allow for multiple IPA modules for the same pipeline handler, a standard - * interface for the pipeline handler and IPA communication is needed. - * IPAInterface is this interface. + * Due to process isolation all arguments to the IPAInterface methods and + * signals may need to be transferred over IPC. The class thus uses serializable + * data types only. The IPA C++ interface defines custom data structures that + * mirror core libcamera structures when the latter are not suitable, such as + * IPAStream to carry StreamConfiguration data. * - * The interface defines base data types and methods to exchange data. On top of - * this, each pipeline handler is responsible for defining specific operations - * that make up its IPA protocol, shared by all IPA modules compatible with the - * pipeline handler. + * As for the functions defined in struct ipa_context_ops, the methods defined + * by this class shall not return data from the IPA. * * The pipeline handler shall use the IPAManager to locate a compatible * IPAInterface. The interface may then be used to interact with the IPA module. - * - * \todo Add reference to how pipelines shall document their protocol. */ /** From patchwork Fri Nov 8 20:54:05 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2316 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id A126960180 for ; Fri, 8 Nov 2019 21:54:29 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2413531D; Fri, 8 Nov 2019 21:54:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246469; bh=H0SKLf2heJNBdl0GFtmEBtFSiH6ZwIjZw70rdL5BO5E=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=reOUUU0VNSoUQ5O+DMh22RKkpL+FVIiSw7/LIy6pu2bLdgsA7DUb5VtAKwq6f+XK7 tMa3C1mFrWbv6IA+tqDyDISkIqDeI3l522SzfCK2e57lqlgg3W75Gcv2f75xctt7cS IRHtfGabIjycxDac0HZe4z/uLzoxKTtAHikBB3Dw= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:54:05 +0200 Message-Id: <20191108205409.18845-21-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 20/24] ipa: Switch to the plain C API 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-List-Received-Date: Fri, 08 Nov 2019 20:54:29 -0000 From: Jacopo Mondi Switch IPA communication to the plain C API. As the IPAInterface class is easier to use for pipeline handlers than a plain C API, retain it and add an IPAContextWrapper that translate between the C++ and the C APIs. On the IPA module side usage of IPAInterface may be desired for IPAs implemented in C++ that want to link to libcamera. For those IPAs, a new IPAInterfaceWrapper helper class is introduced to wrap the IPAInterface implemented internally by the IPA module into an ipa_context, ipa_context_ops and ipa_callback_ops. Signed-off-by: Jacopo Mondi Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- Documentation/Doxyfile.in | 1 + Documentation/meson.build | 2 + src/ipa/ipa_vimc.cpp | 7 +- src/ipa/libipa/ipa_interface_wrapper.cpp | 240 ++++++++++++++++++ src/ipa/libipa/ipa_interface_wrapper.h | 56 ++++ src/ipa/libipa/meson.build | 13 + src/ipa/meson.build | 3 + src/ipa/rkisp1/meson.build | 3 +- src/ipa/rkisp1/rkisp1.cpp | 5 +- src/libcamera/include/ipa_context_wrapper.h | 43 ++++ src/libcamera/include/ipa_module.h | 5 +- src/libcamera/include/meson.build | 1 + src/libcamera/ipa_context_wrapper.cpp | 219 ++++++++++++++++ src/libcamera/ipa_manager.cpp | 67 ++++- src/libcamera/ipa_module.cpp | 23 +- src/libcamera/meson.build | 1 + .../proxy/worker/ipa_proxy_linux_worker.cpp | 8 +- 17 files changed, 674 insertions(+), 23 deletions(-) create mode 100644 src/ipa/libipa/ipa_interface_wrapper.cpp create mode 100644 src/ipa/libipa/ipa_interface_wrapper.h create mode 100644 src/ipa/libipa/meson.build create mode 100644 src/libcamera/include/ipa_context_wrapper.h create mode 100644 src/libcamera/ipa_context_wrapper.cpp diff --git a/Documentation/Doxyfile.in b/Documentation/Doxyfile.in index 24babfd8b366..840c1b4c76c5 100644 --- a/Documentation/Doxyfile.in +++ b/Documentation/Doxyfile.in @@ -793,6 +793,7 @@ WARN_LOGFILE = INPUT = "@TOP_SRCDIR@/include/ipa" \ "@TOP_SRCDIR@/include/libcamera" \ + "@TOP_SRCDIR@/src/ipa/libipa" \ "@TOP_SRCDIR@/src/libcamera" \ "@TOP_BUILDDIR@/include/libcamera" \ "@TOP_BUILDDIR@/src/libcamera" diff --git a/Documentation/meson.build b/Documentation/meson.build index 4ff3fbeb0674..9136506f5d9c 100644 --- a/Documentation/meson.build +++ b/Documentation/meson.build @@ -24,6 +24,8 @@ if doxygen.found() libcamera_ipa_api, libcamera_headers, libcamera_sources, + libipa_headers, + libipa_sources, ], output : 'api-html', command : [doxygen, doxyfile], diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp index 50ca8dd805fb..8f03e811acc7 100644 --- a/src/ipa/ipa_vimc.cpp +++ b/src/ipa/ipa_vimc.cpp @@ -17,7 +17,10 @@ #include #include +#include "libipa/ipa_interface_wrapper.h" + #include "log.h" +#include "utils.h" namespace libcamera { @@ -108,9 +111,9 @@ const struct IPAModuleInfo ipaModuleInfo = { LICENSE, }; -IPAInterface *ipaCreate() +struct ipa_context *ipaCreate() { - return new IPAVimc(); + return new IPAInterfaceWrapper(utils::make_unique()); } } diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp new file mode 100644 index 000000000000..80c5648ffed6 --- /dev/null +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp @@ -0,0 +1,240 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * ipa_interface_wrapper.cpp - Image Processing Algorithm interface wrapper + */ + +#include "ipa_interface_wrapper.h" + +#include +#include +#include +#include + +#include + +#include "byte_stream_buffer.h" + +/** + * \file ipa_interface_wrapper.h + * \brief Image Processing Algorithm interface wrapper + */ + +namespace libcamera { + +/** + * \class IPAInterfaceWrapper + * \brief Wrap an IPAInterface and expose it as an ipa_context + * + * This class implements the ipa_context API based on a provided IPAInterface. + * It helps IPAs that implement the IPAInterface API to provide the external + * ipa_context API. + * + * To use the wrapper, an IPA module simple creates a new instance of its + * IPAInterface implementation, and passes it to the constructor of the + * IPAInterfaceWrapper. As IPAInterfaceWrapper inherits from ipa_context, the + * constructed wrapper can then be directly returned from the IPA module's + * ipaCreate() function. + * + * \code{.cpp} + * class MyIPA : public IPAInterface + * { + * ... + * }; + * + * struct ipa_context *ipaCreate() + * { + * return new IPAInterfaceWrapper(utils::make_unique()); + * } + * \endcode + * + * The wrapper takes ownership of the IPAInterface and will automatically + * delete it when the wrapper is destroyed. + */ + +/** + * \brief Construct an IPAInterfaceWrapper wrapping \a interface + * \param[in] interface The interface to wrap + */ +IPAInterfaceWrapper::IPAInterfaceWrapper(std::unique_ptr interface) + : ipa_(std::move(interface)), callbacks_(nullptr), cb_ctx_(nullptr) +{ + ops = &operations_; + + ipa_->queueFrameAction.connect(this, &IPAInterfaceWrapper::queueFrameAction); +} + +void IPAInterfaceWrapper::destroy(struct ipa_context *_ctx) +{ + IPAInterfaceWrapper *ctx = static_cast(_ctx); + + delete ctx; +} + +void IPAInterfaceWrapper::init(struct ipa_context *_ctx) +{ + IPAInterfaceWrapper *ctx = static_cast(_ctx); + + ctx->ipa_->init(); +} + +void IPAInterfaceWrapper::register_callbacks(struct ipa_context *_ctx, + const struct ipa_callback_ops *callbacks, + void *cb_ctx) +{ + IPAInterfaceWrapper *ctx = static_cast(_ctx); + + ctx->callbacks_ = callbacks; + ctx->cb_ctx_ = cb_ctx; +} + +void IPAInterfaceWrapper::configure(struct ipa_context *_ctx, + const struct ipa_stream *streams, + unsigned int num_streams, + const struct ipa_control_info_map *maps, + unsigned int num_maps) +{ + IPAInterfaceWrapper *ctx = static_cast(_ctx); + + ctx->serializer_.reset(); + + /* Translate the IPA stream configurations map. */ + std::map ipaStreams; + + for (unsigned int i = 0; i < num_streams; ++i) { + const struct ipa_stream &stream = streams[i]; + + ipaStreams[stream.id] = { + stream.pixel_format, + Size(stream.width, stream.height), + }; + } + + /* Translate the IPA entity controls map. */ + std::map entityControls; + std::map infoMaps; + + for (unsigned int i = 0; i < num_maps; ++i) { + const struct ipa_control_info_map &ipa_map = maps[i]; + ByteStreamBuffer byteStream(ipa_map.data, ipa_map.size); + unsigned int id = ipa_map.id; + + infoMaps[id] = ctx->serializer_.deserialize(byteStream); + entityControls.emplace(id, infoMaps[id]); + } + + ctx->ipa_->configure(ipaStreams, entityControls); +} + +void IPAInterfaceWrapper::map_buffers(struct ipa_context *_ctx, + const struct ipa_buffer *_buffers, + size_t num_buffers) +{ + IPAInterfaceWrapper *ctx = static_cast(_ctx); + std::vector buffers(num_buffers); + + for (unsigned int i = 0; i < num_buffers; ++i) { + const struct ipa_buffer &_buffer = _buffers[i]; + IPABuffer &buffer = buffers[i]; + std::vector &planes = buffer.memory.planes(); + + buffer.id = _buffer.id; + + planes.resize(_buffer.num_planes); + for (unsigned int j = 0; j < _buffer.num_planes; ++j) { + if (_buffer.planes[j].dmabuf != -1) + planes[j].setDmabuf(_buffer.planes[j].dmabuf, + _buffer.planes[j].length); + /** \todo Create a Dmabuf class to implement RAII. */ + ::close(_buffer.planes[j].dmabuf); + } + } + + ctx->ipa_->mapBuffers(buffers); +} + +void IPAInterfaceWrapper::unmap_buffers(struct ipa_context *_ctx, + const unsigned int *_ids, + size_t num_buffers) +{ + IPAInterfaceWrapper *ctx = static_cast(_ctx); + std::vector ids(_ids, _ids + num_buffers); + ctx->ipa_->unmapBuffers(ids); +} + +void IPAInterfaceWrapper::process_event(struct ipa_context *_ctx, + const struct ipa_operation_data *data) +{ + IPAInterfaceWrapper *ctx = static_cast(_ctx); + IPAOperationData opData; + + opData.operation = data->operation; + + opData.data.resize(data->num_data); + memcpy(opData.data.data(), data->data, + data->num_data * sizeof(*data->data)); + + opData.controls.resize(data->num_lists); + for (unsigned int i = 0; i < data->num_lists; ++i) { + const struct ipa_control_list *c_list = &data->lists[i]; + ByteStreamBuffer byteStream(c_list->data, c_list->size); + opData.controls[i] = ctx->serializer_.deserialize(byteStream); + } + + ctx->ipa_->processEvent(opData); +} + +void IPAInterfaceWrapper::queueFrameAction(unsigned int frame, + const IPAOperationData &data) +{ + if (!callbacks_) + return; + + struct ipa_operation_data c_data; + c_data.operation = data.operation; + c_data.data = data.data.data(); + c_data.num_data = data.data.size(); + + struct ipa_control_list control_lists[data.controls.size()]; + c_data.lists = control_lists; + c_data.num_lists = data.controls.size(); + + std::size_t listsSize = 0; + for (const auto &list : data.controls) + listsSize += serializer_.binarySize(list); + + std::vector binaryData(listsSize); + ByteStreamBuffer byteStreamBuffer(binaryData.data(), listsSize); + + unsigned int i = 0; + for (const auto &list : data.controls) { + struct ipa_control_list &c_list = control_lists[i]; + c_list.size = serializer_.binarySize(list); + + ByteStreamBuffer b = byteStreamBuffer.carveOut(c_list.size); + serializer_.serialize(list, b); + + c_list.data = b.base(); + } + + callbacks_->queue_frame_action(cb_ctx_, frame, c_data); +} + +#ifndef __DOXYGEN__ +/* + * This construct confuses Doygen and makes it believe that all members of the + * operations is a member of IPAInterfaceWrapper. It must thus be hidden. + */ +const struct ipa_context_ops IPAInterfaceWrapper::operations_ = { + .destroy = &IPAInterfaceWrapper::destroy, + .init = &IPAInterfaceWrapper::init, + .register_callbacks = &IPAInterfaceWrapper::register_callbacks, + .configure = &IPAInterfaceWrapper::configure, + .map_buffers = &IPAInterfaceWrapper::map_buffers, + .unmap_buffers = &IPAInterfaceWrapper::unmap_buffers, + .process_event = &IPAInterfaceWrapper::process_event, +}; +#endif + +} /* namespace libcamera */ diff --git a/src/ipa/libipa/ipa_interface_wrapper.h b/src/ipa/libipa/ipa_interface_wrapper.h new file mode 100644 index 000000000000..17be2062b6c7 --- /dev/null +++ b/src/ipa/libipa/ipa_interface_wrapper.h @@ -0,0 +1,56 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * ipa_interface_wrapper.h - Image Processing Algorithm interface wrapper + */ +#ifndef __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__ +#define __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__ + +#include + +#include + +#include "control_serializer.h" + +namespace libcamera { + +class IPAInterfaceWrapper : public ipa_context +{ +public: + IPAInterfaceWrapper(std::unique_ptr interface); + +private: + static void destroy(struct ipa_context *ctx); + static void init(struct ipa_context *ctx); + static void register_callbacks(struct ipa_context *ctx, + const struct ipa_callback_ops *callbacks, + void *cb_ctx); + static void configure(struct ipa_context *ctx, + const struct ipa_stream *streams, + unsigned int num_streams, + const struct ipa_control_info_map *maps, + unsigned int num_maps); + static void map_buffers(struct ipa_context *ctx, + const struct ipa_buffer *c_buffers, + size_t num_buffers); + static void unmap_buffers(struct ipa_context *ctx, + const unsigned int *ids, + size_t num_buffers); + static void process_event(struct ipa_context *ctx, + const struct ipa_operation_data *data); + + static const struct ipa_context_ops operations_; + + void queueFrameAction(unsigned int frame, const IPAOperationData &data); + + std::unique_ptr ipa_; + const struct ipa_callback_ops *callbacks_; + void *cb_ctx_; + + ControlSerializer serializer_; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_IPA_INTERFACE_WRAPPER_H__ */ diff --git a/src/ipa/libipa/meson.build b/src/ipa/libipa/meson.build new file mode 100644 index 000000000000..6f3cd4866ce3 --- /dev/null +++ b/src/ipa/libipa/meson.build @@ -0,0 +1,13 @@ +libipa_headers = files([ + 'ipa_interface_wrapper.h', +]) + +libipa_sources = files([ + 'ipa_interface_wrapper.cpp', +]) + +libipa_includes = include_directories('..') + +libipa = static_library('ipa', libipa_sources, + include_directories : ipa_includes, + dependencies : libcamera_dep) diff --git a/src/ipa/meson.build b/src/ipa/meson.build index 4f2a45771201..421803243e32 100644 --- a/src/ipa/meson.build +++ b/src/ipa/meson.build @@ -10,11 +10,14 @@ ipa_includes = [ libcamera_internal_includes, ] +subdir('libipa') + foreach t : ipa_vimc_sources ipa = shared_module(t[0], 'ipa_vimc.cpp', name_prefix : '', include_directories : ipa_includes, dependencies : libcamera_dep, + link_with : libipa, install : true, install_dir : ipa_install_dir, cpp_args : '-DLICENSE="' + t[1] + '"') diff --git a/src/ipa/rkisp1/meson.build b/src/ipa/rkisp1/meson.build index 1cab319ce6be..521518bd1237 100644 --- a/src/ipa/rkisp1/meson.build +++ b/src/ipa/rkisp1/meson.build @@ -1,7 +1,8 @@ rkisp1_ipa = shared_module('ipa_rkisp1', 'rkisp1.cpp', name_prefix : '', - include_directories : ipa_includes, + include_directories : [ipa_includes, libipa_includes], dependencies : libcamera_dep, + link_with : libipa, install : true, install_dir : ipa_install_dir) diff --git a/src/ipa/rkisp1/rkisp1.cpp b/src/ipa/rkisp1/rkisp1.cpp index 41babf0c4140..744a16ae5b9a 100644 --- a/src/ipa/rkisp1/rkisp1.cpp +++ b/src/ipa/rkisp1/rkisp1.cpp @@ -19,6 +19,7 @@ #include #include #include +#include #include "log.h" #include "utils.h" @@ -247,9 +248,9 @@ const struct IPAModuleInfo ipaModuleInfo = { "LGPL-2.1-or-later", }; -IPAInterface *ipaCreate() +struct ipa_context *ipaCreate() { - return new IPARkISP1(); + return new IPAInterfaceWrapper(utils::make_unique()); } } diff --git a/src/libcamera/include/ipa_context_wrapper.h b/src/libcamera/include/ipa_context_wrapper.h new file mode 100644 index 000000000000..060888218838 --- /dev/null +++ b/src/libcamera/include/ipa_context_wrapper.h @@ -0,0 +1,43 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * ipa_context_wrapper.h - Image Processing Algorithm context wrapper + */ +#ifndef __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__ +#define __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__ + +#include + +#include "control_serializer.h" + +namespace libcamera { + +class IPAContextWrapper final : public IPAInterface +{ +public: + IPAContextWrapper(struct ipa_context *context); + ~IPAContextWrapper(); + + int init() override; + void configure(const std::map &streamConfig, + const std::map &entityControls) override; + + void mapBuffers(const std::vector &buffers) override; + void unmapBuffers(const std::vector &ids) override; + + virtual void processEvent(const IPAOperationData &data) override; + +private: + static void queue_frame_action(void *ctx, unsigned int frame, + struct ipa_operation_data &data); + static const struct ipa_callback_ops callbacks_; + + struct ipa_context *ctx_; + + ControlSerializer serializer_; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_IPA_CONTEXT_WRAPPER_H__ */ diff --git a/src/libcamera/include/ipa_module.h b/src/libcamera/include/ipa_module.h index 97737587ab3a..2028b76a1913 100644 --- a/src/libcamera/include/ipa_module.h +++ b/src/libcamera/include/ipa_module.h @@ -7,7 +7,6 @@ #ifndef __LIBCAMERA_IPA_MODULE_H__ #define __LIBCAMERA_IPA_MODULE_H__ -#include #include #include @@ -30,7 +29,7 @@ public: bool load(); - std::unique_ptr createInstance(); + struct ipa_context *createContext(); bool match(PipelineHandler *pipe, uint32_t minVersion, uint32_t maxVersion) const; @@ -45,7 +44,7 @@ private: bool loaded_; void *dlHandle_; - typedef IPAInterface *(*IPAIntfFactory)(void); + typedef struct ipa_context *(*IPAIntfFactory)(void); IPAIntfFactory ipaCreate_; int loadIPAModuleInfo(); diff --git a/src/libcamera/include/meson.build b/src/libcamera/include/meson.build index 697294f4b09b..17e2bed93fba 100644 --- a/src/libcamera/include/meson.build +++ b/src/libcamera/include/meson.build @@ -9,6 +9,7 @@ libcamera_headers = files([ 'device_enumerator_udev.h', 'event_dispatcher_poll.h', 'formats.h', + 'ipa_context_wrapper.h', 'ipa_manager.h', 'ipa_module.h', 'ipa_proxy.h', diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp new file mode 100644 index 000000000000..66fc59b82373 --- /dev/null +++ b/src/libcamera/ipa_context_wrapper.cpp @@ -0,0 +1,219 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * ipa_context_wrapper.cpp - Image Processing Algorithm context wrapper + */ + +#include "ipa_context_wrapper.h" + +#include + +#include + +#include "byte_stream_buffer.h" + +/** + * \file ipa_context_wrapper.h + * \brief Image Processing Algorithm context wrapper + */ + +namespace libcamera { + +/** + * \class IPAContextWrapper + * \brief Wrap an ipa_context and expose it as an IPAInterface + * + * The IPAContextWrapper class wraps an ipa_context, provided by an IPA module, and + * exposes an IPAInterface. This mechanism is used for IPAs that are not + * isolated in a separate process to allow direct calls from pipeline handler + * using the IPAInterface API instead of the lower-level ipa_context API. + * + * The IPAInterface methods are converted to the ipa_context API by translating + * all C++ arguments into plain C structures or byte arrays that contain no + * pointer, as required by the ipa_context API. + */ + +/** + * \brief Construct an IPAContextWrapper instance that wraps the \a context + * \param[in] context The IPA module context + * + * Ownership of the \a context is passed to the IPAContextWrapper. The context remains + * valid for the whole lifetime of the wrapper and is destroyed automatically + * with it. + */ +IPAContextWrapper::IPAContextWrapper(struct ipa_context *context) + : ctx_(context) +{ + if (!ctx_) + return; + + ctx_->ops->register_callbacks(ctx_, &IPAContextWrapper::callbacks_, + this); +} + +IPAContextWrapper::~IPAContextWrapper() +{ + if (ctx_) + ctx_->ops->destroy(ctx_); +} + +int IPAContextWrapper::init() +{ + if (!ctx_) + return 0; + + ctx_->ops->init(ctx_); + + return 0; +} + +void IPAContextWrapper::configure(const std::map &streamConfig, + const std::map &entityControls) +{ + if (!ctx_) + return; + + serializer_.reset(); + + /* Translate the IPA stream configurations map. */ + struct ipa_stream c_streams[streamConfig.size()]; + + unsigned int i = 0; + for (const auto &stream : streamConfig) { + struct ipa_stream *c_stream = &c_streams[i]; + unsigned int id = stream.first; + const IPAStream &ipaStream = stream.second; + + c_stream->id = id; + c_stream->pixel_format = ipaStream.pixelFormat; + c_stream->width = ipaStream.size.width; + c_stream->height = ipaStream.size.height; + + ++i; + } + + /* Translate the IPA entity controls map. */ + struct ipa_control_info_map c_info_maps[entityControls.size()]; + std::vector> data(entityControls.size()); + + i = 0; + for (const auto &info : entityControls) { + struct ipa_control_info_map &c_info_map = c_info_maps[i]; + unsigned int id = info.first; + const ControlInfoMap &infoMap = info.second; + + size_t infoMapSize = serializer_.binarySize(infoMap); + data[i].resize(infoMapSize); + ByteStreamBuffer byteStream(data[i].data(), data[i].size()); + serializer_.serialize(infoMap, byteStream); + + c_info_map.id = id; + c_info_map.data = byteStream.base(); + c_info_map.size = byteStream.size(); + + ++i; + } + + ctx_->ops->configure(ctx_, c_streams, streamConfig.size(), + c_info_maps, entityControls.size()); +} + +void IPAContextWrapper::mapBuffers(const std::vector &buffers) +{ + if (!ctx_) + return; + + struct ipa_buffer c_buffers[buffers.size()]; + + for (unsigned int i = 0; i < buffers.size(); ++i) { + struct ipa_buffer &c_buffer = c_buffers[i]; + const IPABuffer &buffer = buffers[i]; + const std::vector &planes = buffer.memory.planes(); + + c_buffer.id = buffer.id; + c_buffer.num_planes = planes.size(); + + for (unsigned int j = 0; j < planes.size(); ++j) { + const Plane &plane = planes[j]; + c_buffer.planes[j].dmabuf = plane.dmabuf(); + c_buffer.planes[j].length = plane.length(); + } + } + + ctx_->ops->map_buffers(ctx_, c_buffers, buffers.size()); +} + +void IPAContextWrapper::unmapBuffers(const std::vector &ids) +{ + if (!ctx_) + return; + + ctx_->ops->unmap_buffers(ctx_, ids.data(), ids.size()); +} + +void IPAContextWrapper::processEvent(const IPAOperationData &data) +{ + if (!ctx_) + return; + + struct ipa_operation_data c_data; + c_data.operation = data.operation; + c_data.data = data.data.data(); + c_data.num_data = data.data.size(); + + struct ipa_control_list control_lists[data.controls.size()]; + c_data.lists = control_lists; + c_data.num_lists = data.controls.size(); + + std::size_t listsSize = 0; + for (const auto &list : data.controls) + listsSize += serializer_.binarySize(list); + + std::vector binaryData(listsSize); + ByteStreamBuffer byteStreamBuffer(binaryData.data(), listsSize); + + unsigned int i = 0; + for (const auto &list : data.controls) { + struct ipa_control_list &c_list = control_lists[i]; + c_list.size = serializer_.binarySize(list); + ByteStreamBuffer b = byteStreamBuffer.carveOut(c_list.size); + + serializer_.serialize(list, b); + + c_list.data = b.base(); + } + + ctx_->ops->process_event(ctx_, &c_data); +} + +void IPAContextWrapper::queue_frame_action(void *ctx, unsigned int frame, + struct ipa_operation_data &data) +{ + IPAContextWrapper *_this = static_cast(ctx); + IPAOperationData opData; + + opData.operation = data.operation; + for (unsigned int i = 0; i < data.num_data; ++i) + opData.data.push_back(data.data[i]); + + for (unsigned int i = 0; i < data.num_lists; ++i) { + const struct ipa_control_list &c_list = data.lists[i]; + ByteStreamBuffer b(c_list.data, c_list.size); + opData.controls.push_back(_this->serializer_.deserialize(b)); + } + + _this->queueFrameAction.emit(frame, opData); +} + +#ifndef __DOXYGEN__ +/* + * This construct confuses Doygen and makes it believe that all members of the + * operations is a member of IPAInterfaceWrapper. It must thus be hidden. + */ +const struct ipa_callback_ops IPAContextWrapper::callbacks_ = { + .queue_frame_action = &IPAContextWrapper::queue_frame_action, +}; +#endif + +} /* namespace libcamera */ diff --git a/src/libcamera/ipa_manager.cpp b/src/libcamera/ipa_manager.cpp index f3180c0739cb..90eef12dbaf5 100644 --- a/src/libcamera/ipa_manager.cpp +++ b/src/libcamera/ipa_manager.cpp @@ -12,6 +12,7 @@ #include #include +#include "ipa_context_wrapper.h" #include "ipa_module.h" #include "ipa_proxy.h" #include "log.h" @@ -30,6 +31,66 @@ LOG_DEFINE_CATEGORY(IPAManager) /** * \class IPAManager * \brief Manager for IPA modules + * + * The IPA module manager discovers IPA modules from disk, queries and loads + * them, and creates IPA contexts. It supports isolation of the modules in a + * separate process with IPC communication and offers a unified IPAInterface + * view of the IPA contexts to pipeline handlers regardless of whether the + * modules are isolated or loaded in the same process. + * + * Module isolation is based on the module licence. Open-source modules are + * loaded without isolation, while closed-source module are forcefully isolated. + * The isolation mechanism ensures that no code from a closed-source module is + * ever run in the libcamera process. + * + * To create an IPA context, pipeline handlers call the IPAManager::ipaCreate() + * method. For a directly loaded module, the manager calls the module's + * ipaCreate() function directly and wraps the returned context in an + * IPAContextWrapper that exposes an IPAInterface. + * + * ~~~~ + * +---------------+ + * | Pipeline | + * | Handler | + * +---------------+ + * | + * v + * +---------------+ +---------------+ + * | IPA | | Open Source | + * | Interface | | IPA Module | + * | - - - - - - - | | - - - - - - - | + * | IPA Context | ipa_context_ops | ipa_context | + * | Wrapper | ----------------> | | + * +---------------+ +---------------+ + * ~~~~ + * + * For an isolated module, the manager instantiates an IPAProxy which spawns a + * new process for an IPA proxy worker. The worker loads the IPA module and + * creates the IPA context. The IPAProxy alse exposes an IPAInterface. + * + * ~~~~ + * +---------------+ +---------------+ + * | Pipeline | | Closed Source | + * | Handler | | IPA Module | + * +---------------+ | - - - - - - - | + * | | ipa_context | + * v | | + * +---------------+ +---------------+ + * | IPA | ipa_context_ops ^ + * | Interface | | + * | - - - - - - - | +---------------+ + * | IPA Proxy | operations | IPA Proxy | + * | | ----------------> | Worker | + * +---------------+ over IPC +---------------+ + * ~~~~ + * + * The IPAInterface implemented by the IPAContextWrapper or IPAProxy is + * returned to the pipeline handler, and all interactions with the IPA context + * go the same interface regardless of process isolation. + * + * In all cases the data passed to the IPAInterface methods is serialized to + * Plain Old Data, either for the purpose of passing it to the IPA context + * plain C API, or to transmit the data to the isolated process through IPC. */ IPAManager::IPAManager() @@ -199,7 +260,11 @@ std::unique_ptr IPAManager::createIPA(PipelineHandler *pipe, if (!m->load()) return nullptr; - return m->createInstance(); + struct ipa_context *ctx = m->createContext(); + if (!ctx) + return nullptr; + + return utils::make_unique(ctx); } } /* namespace libcamera */ diff --git a/src/libcamera/ipa_module.cpp b/src/libcamera/ipa_module.cpp index 99d308efd47b..2c355ea8b5e5 100644 --- a/src/libcamera/ipa_module.cpp +++ b/src/libcamera/ipa_module.cpp @@ -385,13 +385,13 @@ const std::string &IPAModule::path() const /** * \brief Load the IPA implementation factory from the shared object * - * The IPA module shared object implements an IPAInterface class to be used + * The IPA module shared object implements an ipa_context object to be used * by pipeline handlers. This method loads the factory function from the - * shared object. Later, createInstance() can be called to instantiate the - * IPAInterface. + * shared object. Later, createContext() can be called to instantiate the + * ipa_context. * * This method only needs to be called successfully once, after which - * createInstance() can be called as many times as IPAInterface instances are + * createContext() can be called as many times as ipa_context instances are * needed. * * Calling this function on an invalid module (as returned by isValid()) is @@ -433,24 +433,25 @@ bool IPAModule::load() } /** - * \brief Instantiate an IPAInterface + * \brief Instantiate an IPA context * - * After loading the IPA module with load(), this method creates an - * instance of the IPA module interface. + * After loading the IPA module with load(), this method creates an instance of + * the IPA module context. Ownership of the context is passed to the caller, and + * the context shall be destroyed by calling the \ref ipa_context_ops::destroy + * "ipa_context::ops::destroy()" function. * * Calling this function on a module that has not yet been loaded, or an * invalid module (as returned by load() and isValid(), respectively) is * an error. * - * \return The IPA implementation as a new IPAInterface instance on success, - * or nullptr on error + * \return The IPA context on success, or nullptr on error */ -std::unique_ptr IPAModule::createInstance() +struct ipa_context *IPAModule::createContext() { if (!valid_ || !loaded_) return nullptr; - return std::unique_ptr(ipaCreate_()); + return ipaCreate_(); } /** diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 59cf582580c4..c4f965bd7413 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -16,6 +16,7 @@ libcamera_sources = files([ 'event_notifier.cpp', 'formats.cpp', 'geometry.cpp', + 'ipa_context_wrapper.cpp', 'ipa_controls.cpp', 'ipa_interface.cpp', 'ipa_manager.cpp', diff --git a/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp b/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp index a10761e52b32..07380c16e2d5 100644 --- a/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp +++ b/src/libcamera/proxy/worker/ipa_proxy_linux_worker.cpp @@ -72,9 +72,9 @@ int main(int argc, char **argv) } socket.readyRead.connect(&readyRead); - std::unique_ptr ipa = ipam->createInstance(); - if (!ipa) { - LOG(IPAProxyLinuxWorker, Error) << "Failed to create IPA interface"; + struct ipa_context *ipac = ipam->createContext(); + if (!ipac) { + LOG(IPAProxyLinuxWorker, Error) << "Failed to create IPA context"; return EXIT_FAILURE; } @@ -85,5 +85,7 @@ int main(int argc, char **argv) while (1) dispatcher->processEvents(); + ipac->ops->destroy(ipac); + return 0; } From patchwork Fri Nov 8 20:54:06 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2317 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id E7FFE60180 for ; Fri, 8 Nov 2019 21:54:29 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8BD2F2D1 for ; Fri, 8 Nov 2019 21:54:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246469; bh=8Wn8OxFo8oHZj/VGcpU2ja0SkDrF2F/Wtw9rxZAG9QY=; h=From:To:Subject:Date:In-Reply-To:References:From; b=q8iK00Lv9mWWBNWcyx6hsK8XPH7cgvmb+MeCFrsXq4kwGfS4Q/y3+/R55KYd6lZZS b93YOJz9PJ+k58n62XBzyERB9Dw5gwCBEHz/SX/up3yLASHX6bP9pTUwk9D+lHxMGx Dl5XVlTVW2csO4xnq085EN1XL0JMOL220V4sHdNc= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:54:06 +0200 Message-Id: <20191108205409.18845-22-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 21/24] ipa: Declare the ipaCreate() function prototype 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-List-Received-Date: Fri, 08 Nov 2019 20:54:30 -0000 IPA modules have to implement a public ipaCreate() function, but its prototype isn't declared in any header file. This allows for modules to get the prototype wrong without being warned by the compiler. Fix it. Signed-off-by: Laurent Pinchart Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund --- include/ipa/ipa_interface.h | 2 ++ src/libcamera/ipa_interface.cpp | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h index 3a423e37671f..92f1aac50b85 100644 --- a/include/ipa/ipa_interface.h +++ b/include/ipa/ipa_interface.h @@ -80,6 +80,8 @@ struct ipa_context_ops { const struct ipa_operation_data *data); }; +struct ipa_context *ipaCreate(); + #ifdef __cplusplus } diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp index 89fce0e8347f..cb2767a04711 100644 --- a/src/libcamera/ipa_interface.cpp +++ b/src/libcamera/ipa_interface.cpp @@ -261,6 +261,16 @@ * \sa libcamera::IPAInterface::processEvent() */ +/** + * \fn ipaCreate() + * \brief Entry point to the IPA modules + * + * This function is the entry point to the IPA modules. It is implemented by + * every IPA module, and called by libcamera to create a new IPA context. + * + * \return A newly created IPA context + */ + namespace libcamera { /** From patchwork Fri Nov 8 20:54:07 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2318 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 341FF60180 for ; Fri, 8 Nov 2019 21:54:30 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id D71EA31D for ; Fri, 8 Nov 2019 21:54:29 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246470; bh=Hx++/Nk/hJPzZip9OMMNUIBlRBXX4uP2hOEBsyenWKs=; h=From:To:Subject:Date:In-Reply-To:References:From; b=G46dyv9b/4ffThfOWd/uSlGZF+yLxlGU1rwMDT7v5VHEE8Yi0CEngQuwQad7VkYdd gFLpYEXB5BrEn5UIh7Jd2pcyiSudDtKDvutwF1BPg+o57h4GJADel8HEp0yV4miDpu ez1T5STWFAn2sTIzI87i0TxWuZzVgUk6p9VedHFM= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:54:07 +0200 Message-Id: <20191108205409.18845-23-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 22/24] ipa: Allow short-circuiting the ipa_context_ops 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-List-Received-Date: Fri, 08 Nov 2019 20:54:30 -0000 When an IPA module is loaded without isolation and implements the IPAInterface internally, going through ipa_context_ops is a waste of time. Add an operation to retrieve the IPAInterface, and use it directly in the IPAContextWrapper. For debugging purpose, make it possible to forcing usage of the C API by defining the LIBCAMERA_IPA_FORCE_C_API environment variable. Signed-off-by: Laurent Pinchart Signed-off-by: Jacopo Mondi Reviewed-by: Jacopo Mondi Reviewed-by: Niklas Söderlund Reviewed-by: Kieran Bingham --- include/ipa/ipa_interface.h | 1 + src/ipa/libipa/ipa_interface_wrapper.cpp | 8 +++++ src/ipa/libipa/ipa_interface_wrapper.h | 1 + src/libcamera/include/ipa_context_wrapper.h | 4 +++ src/libcamera/ipa_context_wrapper.cpp | 34 +++++++++++++++++++-- src/libcamera/ipa_interface.cpp | 18 +++++++++++ 6 files changed, 64 insertions(+), 2 deletions(-) diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h index 92f1aac50b85..0ea53d120fe1 100644 --- a/include/ipa/ipa_interface.h +++ b/include/ipa/ipa_interface.h @@ -62,6 +62,7 @@ struct ipa_callback_ops { struct ipa_context_ops { void (*destroy)(struct ipa_context *ctx); + void *(*get_interface)(struct ipa_context *ctx); void (*init)(struct ipa_context *ctx); void (*register_callbacks)(struct ipa_context *ctx, const struct ipa_callback_ops *callbacks, diff --git a/src/ipa/libipa/ipa_interface_wrapper.cpp b/src/ipa/libipa/ipa_interface_wrapper.cpp index 80c5648ffed6..6a389dfa714a 100644 --- a/src/ipa/libipa/ipa_interface_wrapper.cpp +++ b/src/ipa/libipa/ipa_interface_wrapper.cpp @@ -72,6 +72,13 @@ void IPAInterfaceWrapper::destroy(struct ipa_context *_ctx) delete ctx; } +void *IPAInterfaceWrapper::get_interface(struct ipa_context *_ctx) +{ + IPAInterfaceWrapper *ctx = static_cast(_ctx); + + return ctx->ipa_.get(); +} + void IPAInterfaceWrapper::init(struct ipa_context *_ctx) { IPAInterfaceWrapper *ctx = static_cast(_ctx); @@ -228,6 +235,7 @@ void IPAInterfaceWrapper::queueFrameAction(unsigned int frame, */ const struct ipa_context_ops IPAInterfaceWrapper::operations_ = { .destroy = &IPAInterfaceWrapper::destroy, + .get_interface = &IPAInterfaceWrapper::get_interface, .init = &IPAInterfaceWrapper::init, .register_callbacks = &IPAInterfaceWrapper::register_callbacks, .configure = &IPAInterfaceWrapper::configure, diff --git a/src/ipa/libipa/ipa_interface_wrapper.h b/src/ipa/libipa/ipa_interface_wrapper.h index 17be2062b6c7..3fb7b4476439 100644 --- a/src/ipa/libipa/ipa_interface_wrapper.h +++ b/src/ipa/libipa/ipa_interface_wrapper.h @@ -22,6 +22,7 @@ public: private: static void destroy(struct ipa_context *ctx); + static void *get_interface(struct ipa_context *ctx); static void init(struct ipa_context *ctx); static void register_callbacks(struct ipa_context *ctx, const struct ipa_callback_ops *callbacks, diff --git a/src/libcamera/include/ipa_context_wrapper.h b/src/libcamera/include/ipa_context_wrapper.h index 060888218838..c9e194120de6 100644 --- a/src/libcamera/include/ipa_context_wrapper.h +++ b/src/libcamera/include/ipa_context_wrapper.h @@ -33,7 +33,11 @@ private: struct ipa_operation_data &data); static const struct ipa_callback_ops callbacks_; + void doQueueFrameAction(unsigned int frame, + const IPAOperationData &data); + struct ipa_context *ctx_; + IPAInterface *intf_; ControlSerializer serializer_; }; diff --git a/src/libcamera/ipa_context_wrapper.cpp b/src/libcamera/ipa_context_wrapper.cpp index 66fc59b82373..0efbae0f675f 100644 --- a/src/libcamera/ipa_context_wrapper.cpp +++ b/src/libcamera/ipa_context_wrapper.cpp @@ -12,6 +12,7 @@ #include #include "byte_stream_buffer.h" +#include "utils.h" /** * \file ipa_context_wrapper.h @@ -43,11 +44,19 @@ namespace libcamera { * with it. */ IPAContextWrapper::IPAContextWrapper(struct ipa_context *context) - : ctx_(context) + : ctx_(context), intf_(nullptr) { if (!ctx_) return; + bool forceCApi = !!utils::secure_getenv("LIBCAMERA_IPA_FORCE_C_API"); + + if (!forceCApi && ctx_ && ctx_->ops->get_interface) { + intf_ = reinterpret_cast(ctx_->ops->get_interface(ctx_)); + intf_->queueFrameAction.connect(this, &IPAContextWrapper::doQueueFrameAction); + return; + } + ctx_->ops->register_callbacks(ctx_, &IPAContextWrapper::callbacks_, this); } @@ -60,6 +69,9 @@ IPAContextWrapper::~IPAContextWrapper() int IPAContextWrapper::init() { + if (intf_) + return intf_->init(); + if (!ctx_) return 0; @@ -71,6 +83,9 @@ int IPAContextWrapper::init() void IPAContextWrapper::configure(const std::map &streamConfig, const std::map &entityControls) { + if (intf_) + return intf_->configure(streamConfig, entityControls); + if (!ctx_) return; @@ -121,6 +136,9 @@ void IPAContextWrapper::configure(const std::map &strea void IPAContextWrapper::mapBuffers(const std::vector &buffers) { + if (intf_) + return intf_->mapBuffers(buffers); + if (!ctx_) return; @@ -146,6 +164,9 @@ void IPAContextWrapper::mapBuffers(const std::vector &buffers) void IPAContextWrapper::unmapBuffers(const std::vector &ids) { + if (intf_) + return intf_->unmapBuffers(ids); + if (!ctx_) return; @@ -154,6 +175,9 @@ void IPAContextWrapper::unmapBuffers(const std::vector &ids) void IPAContextWrapper::processEvent(const IPAOperationData &data) { + if (intf_) + return intf_->processEvent(data); + if (!ctx_) return; @@ -187,6 +211,12 @@ void IPAContextWrapper::processEvent(const IPAOperationData &data) ctx_->ops->process_event(ctx_, &c_data); } +void IPAContextWrapper::doQueueFrameAction(unsigned int frame, + const IPAOperationData &data) +{ + IPAInterface::queueFrameAction.emit(frame, data); +} + void IPAContextWrapper::queue_frame_action(void *ctx, unsigned int frame, struct ipa_operation_data &data) { @@ -203,7 +233,7 @@ void IPAContextWrapper::queue_frame_action(void *ctx, unsigned int frame, opData.controls.push_back(_this->serializer_.deserialize(b)); } - _this->queueFrameAction.emit(frame, opData); + _this->doQueueFrameAction(frame, opData); } #ifndef __DOXYGEN__ diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp index cb2767a04711..715e7972271b 100644 --- a/src/libcamera/ipa_interface.cpp +++ b/src/libcamera/ipa_interface.cpp @@ -62,6 +62,12 @@ * handlers to communicate with IPA modules. IPA modules may use the * IPAInterface API internally if they want to benefit from the data and helper * classes offered by libcamera. + * + * When an IPA module is loaded directly into the libcamera process and uses + * the IPAInterface API internally, short-circuiting the path to the + * ipa_context_ops and back to IPAInterface is desirable. To support this, IPA + * modules may implement the ipa_context_ops::get_interface function to return a + * pointer to their internal IPAInterface. */ /** @@ -209,6 +215,18 @@ * \param[in] ctx The IPA context */ +/** + * \var ipa_context_ops::get_interface + * \brief Retrieve the IPAInterface implemented by the ipa_context (optional) + * \param[in] ctx The IPA context + * + * IPA modules may implement this function to expose their internal + * IPAInterface, if any. When implemented, libcamera may at its sole discretion + * call it and then bypass the ipa_context_ops API by calling the IPAInterface + * methods directly. IPA modules shall still implement and support the full + * ipa_context_ops API. + */ + /** * \var ipa_context_ops::init * \brief Initialise the IPA context From patchwork Fri Nov 8 20:54:08 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2319 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 8A5F26154E for ; Fri, 8 Nov 2019 21:54:30 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 2CAD92D1 for ; Fri, 8 Nov 2019 21:54:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246470; bh=ujs8C1zgbvG3p71JM9xJG+kgcNXVKvfDHbRY/bbudAw=; h=From:To:Subject:Date:In-Reply-To:References:From; b=IjScbwtWZFfLK63HdfKoue23I3xTxENi0QzrwwzD3jv+HMOyoDWXN0TnIW5U8jB4L y0DPNAJrX4AqFkhWiuOjn7PDlg8X3w3E59wSkTmO8E4czNkixUeInteVwAdVzOBhs3 Gs+K3luY4kciWPUr5+p6ihZl2oS1BaVSPlUOwkS8= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:54:08 +0200 Message-Id: <20191108205409.18845-24-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 23/24] test: ipa: Add IPA wrappers test 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-List-Received-Date: Fri, 08 Nov 2019 20:54:30 -0000 Wrap an IPAInterface in an IPAInterfaceWrapper in an IPAContextWrapper, and verify that the translation between C and C++ APIs work correctly. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- test/ipa/ipa_wrappers_test.cpp | 390 +++++++++++++++++++++++++++++++++ test/ipa/meson.build | 5 +- 2 files changed, 393 insertions(+), 2 deletions(-) create mode 100644 test/ipa/ipa_wrappers_test.cpp diff --git a/test/ipa/ipa_wrappers_test.cpp b/test/ipa/ipa_wrappers_test.cpp new file mode 100644 index 000000000000..c0717f0e301b --- /dev/null +++ b/test/ipa/ipa_wrappers_test.cpp @@ -0,0 +1,390 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * ipa_wrappers_test.cpp - Test the IPA interface and context wrappers + */ + +#include +#include +#include +#include +#include +#include + +#include +#include + +#include "device_enumerator.h" +#include "ipa_context_wrapper.h" +#include "media_device.h" +#include "utils.h" +#include "v4l2_subdevice.h" + +#include "test.h" + +using namespace libcamera; +using namespace std; + +enum Operation { + Op_init, + Op_configure, + Op_mapBuffers, + Op_unmapBuffers, + Op_processEvent, +}; + +class TestIPAInterface : public IPAInterface +{ +public: + TestIPAInterface() + : sequence_(0) + { + } + + int init() override + { + report(Op_init, TestPass); + return 0; + } + + void configure(const std::map &streamConfig, + const std::map &entityControls) override + { + /* Verify streamConfig. */ + if (streamConfig.size() != 2) { + cerr << "configure(): Invalid number of streams " + << streamConfig.size() << endl; + return report(Op_configure, TestFail); + } + + auto iter = streamConfig.find(1); + if (iter == streamConfig.end()) { + cerr << "configure(): No configuration for stream 1" << endl; + return report(Op_configure, TestFail); + } + const IPAStream *stream = &iter->second; + if (stream->pixelFormat != V4L2_PIX_FMT_YUYV || + stream->size != Size{ 1024, 768 }) { + cerr << "configure(): Invalid configuration for stream 1" << endl; + return report(Op_configure, TestFail); + } + + iter = streamConfig.find(2); + if (iter == streamConfig.end()) { + cerr << "configure(): No configuration for stream 2" << endl; + return report(Op_configure, TestFail); + } + stream = &iter->second; + if (stream->pixelFormat != V4L2_PIX_FMT_NV12 || + stream->size != Size{ 800, 600 }) { + cerr << "configure(): Invalid configuration for stream 2" << endl; + return report(Op_configure, TestFail); + } + + /* Verify entityControls. */ + auto ctrlIter = entityControls.find(42); + if (ctrlIter == entityControls.end()) { + cerr << "configure(): Controls not found" << endl; + return report(Op_configure, TestFail); + } + + const ControlInfoMap &infoMap = ctrlIter->second; + + if (infoMap.count(V4L2_CID_BRIGHTNESS) != 1 || + infoMap.count(V4L2_CID_CONTRAST) != 1 || + infoMap.count(V4L2_CID_SATURATION) != 1) { + cerr << "configure(): Invalid control IDs" << endl; + return report(Op_configure, TestFail); + } + + report(Op_configure, TestPass); + } + + void mapBuffers(const std::vector &buffers) override + { + if (buffers.size() != 2) { + cerr << "mapBuffers(): Invalid number of buffers " + << buffers.size() << endl; + return report(Op_mapBuffers, TestFail); + } + + if (buffers[0].id != 10 || + buffers[1].id != 11) { + cerr << "mapBuffers(): Invalid buffer IDs" << endl; + return report(Op_mapBuffers, TestFail); + } + + if (buffers[0].memory.planes().size() != 3 || + buffers[1].memory.planes().size() != 3) { + cerr << "mapBuffers(): Invalid number of planes" << endl; + return report(Op_mapBuffers, TestFail); + } + + if (buffers[0].memory.planes()[0].length() != 4096 || + buffers[0].memory.planes()[1].length() != 0 || + buffers[0].memory.planes()[2].length() != 0 || + buffers[0].memory.planes()[0].length() != 4096 || + buffers[1].memory.planes()[1].length() != 4096 || + buffers[1].memory.planes()[2].length() != 0) { + cerr << "mapBuffers(): Invalid length" << endl; + return report(Op_mapBuffers, TestFail); + } + + if (buffers[0].memory.planes()[0].dmabuf() == -1 || + buffers[0].memory.planes()[1].dmabuf() != -1 || + buffers[0].memory.planes()[2].dmabuf() != -1 || + buffers[0].memory.planes()[0].dmabuf() == -1 || + buffers[1].memory.planes()[1].dmabuf() == -1 || + buffers[1].memory.planes()[2].dmabuf() != -1) { + cerr << "mapBuffers(): Invalid dmabuf" << endl; + return report(Op_mapBuffers, TestFail); + } + + report(Op_mapBuffers, TestPass); + } + + void unmapBuffers(const std::vector &ids) override + { + if (ids.size() != 2) { + cerr << "unmapBuffers(): Invalid number of ids " + << ids.size() << endl; + return report(Op_unmapBuffers, TestFail); + } + + if (ids[0] != 10 || ids[1] != 11) { + cerr << "unmapBuffers(): Invalid buffer IDs" << endl; + return report(Op_unmapBuffers, TestFail); + } + + report(Op_unmapBuffers, TestPass); + } + + void processEvent(const IPAOperationData &data) override + { + /* Verify operation and data. */ + if (data.operation != Op_processEvent) { + cerr << "processEvent(): Invalid operation " + << data.operation << endl; + return report(Op_processEvent, TestFail); + } + + if (data.data != std::vector{ 1, 2, 3, 4 }) { + cerr << "processEvent(): Invalid data" << endl; + return report(Op_processEvent, TestFail); + } + + /* Verify controls. */ + if (data.controls.size() != 1) { + cerr << "processEvent(): Controls not found" << endl; + return report(Op_processEvent, TestFail); + } + + const ControlList &controls = data.controls[0]; + if (controls.get(V4L2_CID_BRIGHTNESS).get() != 10 || + controls.get(V4L2_CID_CONTRAST).get() != 20 || + controls.get(V4L2_CID_SATURATION).get() != 30) { + cerr << "processEvent(): Invalid controls" << endl; + return report(Op_processEvent, TestFail); + } + + report(Op_processEvent, TestPass); + } + +private: + void report(Operation op, int status) + { + IPAOperationData data; + data.operation = op; + data.data.resize(1); + data.data[0] = status; + queueFrameAction.emit(sequence_++, data); + } + + unsigned int sequence_; +}; + +#define INVOKE(method, ...) \ + invoke(&IPAInterface::method, Op_##method, #method, ##__VA_ARGS__) + +class IPAWrappersTest : public Test +{ +public: + IPAWrappersTest() + : subdev_(nullptr), wrapper_(nullptr), sequence_(0), fd_(-1) + { + } + +protected: + int init() override + { + /* Locate the VIMC Sensor B subdevice. */ + enumerator_ = unique_ptr(DeviceEnumerator::create()); + if (!enumerator_) { + cerr << "Failed to create device enumerator" << endl; + return TestFail; + } + + if (enumerator_->enumerate()) { + cerr << "Failed to enumerate media devices" << endl; + return TestFail; + } + + DeviceMatch dm("vimc"); + media_ = enumerator_->search(dm); + if (!media_) { + cerr << "No VIMC media device found: skip test" << endl; + return TestSkip; + } + + MediaEntity *entity = media_->getEntityByName("Sensor A"); + if (!entity) { + cerr << "Unable to find media entity 'Sensor A'" << endl; + return TestFail; + } + + subdev_ = new V4L2Subdevice(entity); + if (subdev_->open() < 0) { + cerr << "Unable to open 'Sensor A' subdevice" << endl; + return TestFail; + } + + /* Force usage of the C API as that's what we want to test. */ + int ret = setenv("LIBCAMERA_IPA_FORCE_C_API", "", 1); + if (ret) + return TestFail; + + std::unique_ptr intf = utils::make_unique(); + wrapper_ = new IPAContextWrapper(new IPAInterfaceWrapper(std::move(intf))); + wrapper_->queueFrameAction.connect(this, &IPAWrappersTest::queueFrameAction); + + /* Create a file descriptor for the buffer-related operations. */ + fd_ = open("/tmp", O_TMPFILE | O_RDWR, 0600); + if (fd_ == -1) + return TestFail; + + ftruncate(fd_, 4096); + + return TestPass; + } + + int run() override + { + int ret; + + /* Test configure(). */ + std::map config{ + { 1, { V4L2_PIX_FMT_YUYV, { 1024, 768 } } }, + { 2, { V4L2_PIX_FMT_NV12, { 800, 600 } } }, + }; + std::map controlInfo; + controlInfo.emplace(42, subdev_->controls()); + ret = INVOKE(configure, config, controlInfo); + if (ret == TestFail) + return TestFail; + + /* Test mapBuffers(). */ + std::vector buffers(2); + buffers[0].memory.planes().resize(3); + buffers[0].id = 10; + buffers[0].memory.planes()[0].setDmabuf(fd_, 4096); + buffers[1].id = 11; + buffers[1].memory.planes().resize(3); + buffers[1].memory.planes()[0].setDmabuf(fd_, 4096); + buffers[1].memory.planes()[1].setDmabuf(fd_, 4096); + + ret = INVOKE(mapBuffers, buffers); + if (ret == TestFail) + return TestFail; + + /* Test unmapBuffers(). */ + std::vector bufferIds = { 10, 11 }; + ret = INVOKE(unmapBuffers, bufferIds); + if (ret == TestFail) + return TestFail; + + /* Test processEvent(). */ + IPAOperationData data; + data.operation = Op_processEvent; + data.data = { 1, 2, 3, 4 }; + data.controls.emplace_back(subdev_->controls()); + + ControlList &controls = data.controls.back(); + controls.set(V4L2_CID_BRIGHTNESS, static_cast(10)); + controls.set(V4L2_CID_CONTRAST, static_cast(20)); + controls.set(V4L2_CID_SATURATION, static_cast(30)); + + ret = INVOKE(processEvent, data); + if (ret == TestFail) + return TestFail; + + /* + * Test init() last to ensure nothing in the wrappers or + * serializer depends on init() being called first. + */ + ret = INVOKE(init); + if (ret == TestFail) + return TestFail; + + return TestPass; + } + + void cleanup() override + { + delete wrapper_; + delete subdev_; + + if (fd_ != -1) + close(fd_); + } + +private: + template + int invoke(T (IPAInterface::*func)(Args1...), Operation op, + const char *name, Args2... args) + { + data_ = IPAOperationData(); + (wrapper_->*func)(args...); + + if (frame_ != sequence_) { + cerr << "IPAInterface::" << name + << "(): invalid frame number " << frame_ + << ", expected " << sequence_; + return TestFail; + } + + sequence_++; + + if (data_.operation != op) { + cerr << "IPAInterface::" << name + << "(): failed to propagate" << endl; + return TestFail; + } + + if (data_.data[0] != TestPass) { + cerr << "IPAInterface::" << name + << "(): reported an error" << endl; + return TestFail; + } + + return TestPass; + } + + void queueFrameAction(unsigned int frame, const IPAOperationData &data) + { + frame_ = frame; + data_ = data; + } + + std::shared_ptr media_; + std::unique_ptr enumerator_; + V4L2Subdevice *subdev_; + + IPAContextWrapper *wrapper_; + IPAOperationData data_; + unsigned int sequence_; + unsigned int frame_; + int fd_; +}; + +TEST_REGISTER(IPAWrappersTest) diff --git a/test/ipa/meson.build b/test/ipa/meson.build index c501fcf99aed..f925c50a085e 100644 --- a/test/ipa/meson.build +++ b/test/ipa/meson.build @@ -1,13 +1,14 @@ ipa_test = [ ['ipa_module_test', 'ipa_module_test.cpp'], ['ipa_interface_test', 'ipa_interface_test.cpp'], + ['ipa_wrappers_test', 'ipa_wrappers_test.cpp'], ] foreach t : ipa_test exe = executable(t[0], t[1], dependencies : libcamera_dep, - link_with : test_libraries, - include_directories : test_includes_internal) + link_with : [libipa, test_libraries], + include_directories : [libipa_includes, test_includes_internal]) test(t[0], exe, suite : 'ipa') endforeach From patchwork Fri Nov 8 20:54:09 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 2320 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id D00E061558 for ; Fri, 8 Nov 2019 21:54:30 +0100 (CET) Received: from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi [81.175.216.236]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 77D2C31D for ; Fri, 8 Nov 2019 21:54:30 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1573246470; bh=wVCZiiGTNQ6MmHzCjXs9DhUGhgJpMB+C/hI5ElOVcdo=; h=From:To:Subject:Date:In-Reply-To:References:From; b=KgRYNOHzs176iCWCm3q0PMJkOqI1MNXVDafwXW+Sm+K+YlMU51W8m+kUxdKZhUpE/ AaFhY+hUQ6Zvdf8K6M9rdyZMfIVrGOgHA3gKTIvyhzr/xfjuo8uTmHhW0kYaUvxKFM o0rDiyrwm3Vnd5y+vjjlN2BZ1h2mtA81u9j0fFew= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Fri, 8 Nov 2019 22:54:09 +0200 Message-Id: <20191108205409.18845-25-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.23.0 In-Reply-To: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> References: <20191108205409.18845-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v2 24/24] libcamera: Fix typo related to serialization 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-List-Received-Date: Fri, 08 Nov 2019 20:54:31 -0000 Oxford English spells "serialize", not "serialise". Fix it. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/ipa_interface.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp index 715e7972271b..ee3e3622f39a 100644 --- a/src/libcamera/ipa_interface.cpp +++ b/src/libcamera/ipa_interface.cpp @@ -297,7 +297,7 @@ namespace libcamera { * * The IPAStream structure stores stream configuration parameters needed by the * IPAInterface::configure() method. It mirrors the StreamConfiguration class - * that is not suitable for this purpose due to not being serialisable. + * that is not suitable for this purpose due to not being serializable. */ /**