{"id":3779,"url":"https://patchwork.libcamera.org/api/patches/3779/?format=json","web_url":"https://patchwork.libcamera.org/patch/3779/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20200512003903.20986-1-laurent.pinchart@ideasonboard.com>","date":"2020-05-12T00:39:02","name":"[libcamera-devel,1/2] libcamera: pipeline: raspberrypi: Don't inline all of StaggeredCtrl","commit_ref":null,"pull_url":null,"state":"accepted","archived":false,"hash":"cebe5367bf3f1e7538ff100b80492521434b90f8","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/3779/mbox/","series":[{"id":898,"url":"https://patchwork.libcamera.org/api/series/898/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=898","date":"2020-05-12T00:39:02","name":"[libcamera-devel,1/2] libcamera: pipeline: raspberrypi: Don't inline all of StaggeredCtrl","version":1,"mbox":"https://patchwork.libcamera.org/series/898/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/3779/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/3779/checks/","tags":{},"headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3D954603DD\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue, 12 May 2020 02:39:14 +0200 (CEST)","from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BEBBD33E;\n\tTue, 12 May 2020 02:39:13 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"mpPmmiqd\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1589243954;\n\tbh=iZfjMmp2fdlaVYmISIIQlfXXTrwk0I742mhfeKJjBvE=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=mpPmmiqdugXjAt1IMYDT5FIKIqlQ1d/5WmstYGS1q9dlN1T8PtFcJZFT/7EkfLZcp\n\tNGw+LYnfPCdx614t3C4+aNURLvGsWIL28eABLOhhMGUIv5bGJ9L1hw+kCQ4YZzDsWN\n\tTQPnTzrGVamzPFG8vyXrEUw9ZYpox43iPdpKc3hQ=","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"libcamera-devel@lists.libcamera.org","Date":"Tue, 12 May 2020 03:39:02 +0300","Message-Id":"<20200512003903.20986-1-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.26.2","MIME-Version":"1.0","Content-Transfer-Encoding":"8bit","Subject":"[libcamera-devel] [PATCH 1/2] libcamera: pipeline: raspberrypi:\n\tDon't inline all of StaggeredCtrl","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Tue, 12 May 2020 00:39:14 -0000"},"content":"The StaggeredCtrl class has large functions, move them to a .cpp file\ninstead of inlining them all to reduce the binary size.\n\nSigned-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n---\n .../pipeline/raspberrypi/meson.build          |   3 +-\n .../pipeline/raspberrypi/staggered_ctrl.cpp   | 173 ++++++++++++++++++\n .../pipeline/raspberrypi/staggered_ctrl.h     | 168 ++---------------\n 3 files changed, 186 insertions(+), 158 deletions(-)\n create mode 100644 src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp","diff":"diff --git a/src/libcamera/pipeline/raspberrypi/meson.build b/src/libcamera/pipeline/raspberrypi/meson.build\nindex 737857977831..565a8902f1f4 100644\n--- a/src/libcamera/pipeline/raspberrypi/meson.build\n+++ b/src/libcamera/pipeline/raspberrypi/meson.build\n@@ -1,3 +1,4 @@\n libcamera_sources += files([\n-    'raspberrypi.cpp'\n+    'raspberrypi.cpp',\n+    'staggered_ctrl.cpp',\n ])\ndiff --git a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp\nnew file mode 100644\nindex 000000000000..fbd87d3e791e\n--- /dev/null\n+++ b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.cpp\n@@ -0,0 +1,173 @@\n+/* SPDX-License-Identifier: BSD-2-Clause */\n+/*\n+ * Copyright (C) 2020, Raspberry Pi (Trading) Ltd.\n+ *\n+ * staggered_ctrl.cpp - Helper for writing staggered ctrls to a V4L2 device.\n+ */\n+\n+#include \"staggered_ctrl.h\"\n+\n+#include <algorithm>\n+\n+#include \"log.h\"\n+#include \"utils.h\"\n+\n+/* For logging... */\n+using libcamera::LogCategory;\n+using libcamera::LogDebug;\n+using libcamera::LogInfo;\n+using libcamera::utils::hex;\n+\n+LOG_DEFINE_CATEGORY(RPI_S_W);\n+\n+namespace RPi {\n+\n+void StaggeredCtrl::init(libcamera::V4L2VideoDevice *dev,\n+\t  std::initializer_list<std::pair<const uint32_t, uint8_t>> delayList)\n+{\n+\tstd::lock_guard<std::mutex> lock(lock_);\n+\n+\tdev_ = dev;\n+\tdelay_ = delayList;\n+\tctrl_.clear();\n+\n+\t/* Find the largest delay across all controls. */\n+\tmaxDelay_ = 0;\n+\tfor (auto const &p : delay_) {\n+\t\tLOG(RPI_S_W, Info) << \"Init ctrl \"\n+\t\t\t\t   << hex(p.first) << \" with delay \"\n+\t\t\t\t   << static_cast<int>(p.second);\n+\t\tmaxDelay_ = std::max(maxDelay_, p.second);\n+\t}\n+\n+\tinit_ = true;\n+}\n+\n+void StaggeredCtrl::reset()\n+{\n+\tstd::lock_guard<std::mutex> lock(lock_);\n+\n+\tint lastSetCount = std::max<int>(0, setCount_ - 1);\n+\tstd::unordered_map<uint32_t, int32_t> lastVal;\n+\n+\t/* Reset the counters. */\n+\tsetCount_ = getCount_ = 0;\n+\n+\t/* Look for the last set values. */\n+\tfor (auto const &c : ctrl_)\n+\t\tlastVal[c.first] = c.second[lastSetCount].value;\n+\n+\t/* Apply the last set values as the next to be applied. */\n+\tctrl_.clear();\n+\tfor (auto &c : lastVal)\n+\t\tctrl_[c.first][setCount_] = CtrlInfo(c.second);\n+}\n+\n+bool StaggeredCtrl::set(uint32_t ctrl, int32_t value)\n+{\n+\tstd::lock_guard<std::mutex> lock(lock_);\n+\n+\t/* Can we find this ctrl as one that is registered? */\n+\tif (delay_.find(ctrl) == delay_.end())\n+\t\treturn false;\n+\n+\tctrl_[ctrl][setCount_].value = value;\n+\tctrl_[ctrl][setCount_].updated = true;\n+\n+\treturn true;\n+}\n+\n+bool StaggeredCtrl::set(std::initializer_list<std::pair<const uint32_t, int32_t>> ctrlList)\n+{\n+\tstd::lock_guard<std::mutex> lock(lock_);\n+\n+\tfor (auto const &p : ctrlList) {\n+\t\t/* Can we find this ctrl? */\n+\t\tif (delay_.find(p.first) == delay_.end())\n+\t\t\treturn false;\n+\n+\t\tctrl_[p.first][setCount_] = CtrlInfo(p.second);\n+\t}\n+\n+\treturn true;\n+}\n+\n+bool StaggeredCtrl::set(libcamera::ControlList &controls)\n+{\n+\tstd::lock_guard<std::mutex> lock(lock_);\n+\n+\tfor (auto const &p : controls) {\n+\t\t/* Can we find this ctrl? */\n+\t\tif (delay_.find(p.first) == delay_.end())\n+\t\t\treturn false;\n+\n+\t\tctrl_[p.first][setCount_] = CtrlInfo(p.second.get<int32_t>());\n+\t\tLOG(RPI_S_W, Debug) << \"Setting ctrl \"\n+\t\t\t\t    << hex(p.first) << \" to \"\n+\t\t\t\t    << ctrl_[p.first][setCount_].value\n+\t\t\t\t    << \" at index \"\n+\t\t\t\t    << setCount_;\n+\t}\n+\n+\treturn true;\n+}\n+\n+int StaggeredCtrl::write()\n+{\n+\tstd::lock_guard<std::mutex> lock(lock_);\n+\tlibcamera::ControlList controls(dev_->controls());\n+\n+\tfor (auto &p : ctrl_) {\n+\t\tint delayDiff = maxDelay_ - delay_[p.first];\n+\t\tint index = std::max<int>(0, setCount_ - delayDiff);\n+\n+\t\tif (p.second[index].updated) {\n+\t\t\t/* We need to write this value out. */\n+\t\t\tcontrols.set(p.first, p.second[index].value);\n+\t\t\tp.second[index].updated = false;\n+\t\t\tLOG(RPI_S_W, Debug) << \"Writing ctrl \"\n+\t\t\t\t\t    << hex(p.first) << \" to \"\n+\t\t\t\t\t    << p.second[index].value\n+\t\t\t\t\t    << \" at index \"\n+\t\t\t\t\t    << index;\n+\t\t}\n+\t}\n+\n+\tnextFrame();\n+\treturn dev_->setControls(&controls);\n+}\n+\n+void StaggeredCtrl::get(std::unordered_map<uint32_t, int32_t> &ctrl, uint8_t offset)\n+{\n+\tstd::lock_guard<std::mutex> lock(lock_);\n+\n+\t/* Account for the offset to reset the getCounter. */\n+\tgetCount_ += offset + 1;\n+\n+\tctrl.clear();\n+\tfor (auto &p : ctrl_) {\n+\t\tint index = std::max<int>(0, getCount_ - maxDelay_);\n+\t\tctrl[p.first] = p.second[index].value;\n+\t\tLOG(RPI_S_W, Debug) << \"Getting ctrl \"\n+\t\t\t\t    << hex(p.first) << \" to \"\n+\t\t\t\t    << p.second[index].value\n+\t\t\t\t    << \" at index \"\n+\t\t\t\t    << index;\n+\t}\n+}\n+\n+void StaggeredCtrl::nextFrame()\n+{\n+\t/* Advance the control history to the next frame */\n+\tint prevCount = setCount_;\n+\tsetCount_++;\n+\n+\tLOG(RPI_S_W, Debug) << \"Next frame, set index is \" << setCount_;\n+\n+\tfor (auto &p : ctrl_) {\n+\t\tp.second[setCount_].value = p.second[prevCount].value;\n+\t\tp.second[setCount_].updated = false;\n+\t}\n+}\n+\n+} /* namespace RPi */\ndiff --git a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h\nindex 0403c087c686..c8f000a0b43c 100644\n--- a/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h\n+++ b/src/libcamera/pipeline/raspberrypi/staggered_ctrl.h\n@@ -6,24 +6,16 @@\n  */\n #pragma once\n \n-#include <algorithm>\n+#include <array>\n #include <initializer_list>\n #include <mutex>\n #include <unordered_map>\n+#include <utility>\n \n #include <libcamera/controls.h>\n-#include \"log.h\"\n-#include \"utils.h\"\n+\n #include \"v4l2_videodevice.h\"\n \n-/* For logging... */\n-using libcamera::LogCategory;\n-using libcamera::LogDebug;\n-using libcamera::LogInfo;\n-using libcamera::utils::hex;\n-\n-LOG_DEFINE_CATEGORY(RPI_S_W);\n-\n namespace RPi {\n \n class StaggeredCtrl\n@@ -34,163 +26,25 @@ public:\n \t{\n \t}\n \n-\t~StaggeredCtrl()\n-\t{\n-\t}\n-\n \toperator bool() const\n \t{\n \t\treturn init_;\n \t}\n \n \tvoid init(libcamera::V4L2VideoDevice *dev,\n-\t\t  std::initializer_list<std::pair<const uint32_t, uint8_t>> delayList)\n-\t{\n-\t\tstd::lock_guard<std::mutex> lock(lock_);\n+\t\t  std::initializer_list<std::pair<const uint32_t, uint8_t>> delayList);\n+\tvoid reset();\n \n-\t\tdev_ = dev;\n-\t\tdelay_ = delayList;\n-\t\tctrl_.clear();\n+\tvoid get(std::unordered_map<uint32_t, int32_t> &ctrl, uint8_t offset = 0);\n \n-\t\t/* Find the largest delay across all controls. */\n-\t\tmaxDelay_ = 0;\n-\t\tfor (auto const &p : delay_) {\n-\t\t\tLOG(RPI_S_W, Info) << \"Init ctrl \"\n-\t\t\t\t\t   << hex(p.first) << \" with delay \"\n-\t\t\t\t\t   << static_cast<int>(p.second);\n-\t\t\tmaxDelay_ = std::max(maxDelay_, p.second);\n-\t\t}\n+\tbool set(uint32_t ctrl, int32_t value);\n+\tbool set(std::initializer_list<std::pair<const uint32_t, int32_t>> ctrlList);\n+\tbool set(libcamera::ControlList &controls);\n \n-\t\tinit_ = true;\n-\t}\n-\n-\tvoid reset()\n-\t{\n-\t\tstd::lock_guard<std::mutex> lock(lock_);\n-\n-\t\tint lastSetCount = std::max<int>(0, setCount_ - 1);\n-\t\tstd::unordered_map<uint32_t, int32_t> lastVal;\n-\n-\t\t/* Reset the counters. */\n-\t\tsetCount_ = getCount_ = 0;\n-\n-\t\t/* Look for the last set values. */\n-\t\tfor (auto const &c : ctrl_)\n-\t\t\tlastVal[c.first] = c.second[lastSetCount].value;\n-\n-\t\t/* Apply the last set values as the next to be applied. */\n-\t\tctrl_.clear();\n-\t\tfor (auto &c : lastVal)\n-\t\t\tctrl_[c.first][setCount_] = CtrlInfo(c.second);\n-\t}\n-\n-\tbool set(uint32_t ctrl, int32_t value)\n-\t{\n-\t\tstd::lock_guard<std::mutex> lock(lock_);\n-\n-\t\t/* Can we find this ctrl as one that is registered? */\n-\t\tif (delay_.find(ctrl) == delay_.end())\n-\t\t\treturn false;\n-\n-\t\tctrl_[ctrl][setCount_].value = value;\n-\t\tctrl_[ctrl][setCount_].updated = true;\n-\n-\t\treturn true;\n-\t}\n-\n-\tbool set(std::initializer_list<std::pair<const uint32_t, int32_t>> ctrlList)\n-\t{\n-\t\tstd::lock_guard<std::mutex> lock(lock_);\n-\n-\t\tfor (auto const &p : ctrlList) {\n-\t\t\t/* Can we find this ctrl? */\n-\t\t\tif (delay_.find(p.first) == delay_.end())\n-\t\t\t\treturn false;\n-\n-\t\t\tctrl_[p.first][setCount_] = CtrlInfo(p.second);\n-\t\t}\n-\n-\t\treturn true;\n-\t}\n-\n-\tbool set(libcamera::ControlList &controls)\n-\t{\n-\t\tstd::lock_guard<std::mutex> lock(lock_);\n-\n-\t\tfor (auto const &p : controls) {\n-\t\t\t/* Can we find this ctrl? */\n-\t\t\tif (delay_.find(p.first) == delay_.end())\n-\t\t\t\treturn false;\n-\n-\t\t\tctrl_[p.first][setCount_] = CtrlInfo(p.second.get<int32_t>());\n-\t\t\tLOG(RPI_S_W, Debug) << \"Setting ctrl \"\n-\t\t\t\t\t    << hex(p.first) << \" to \"\n-\t\t\t\t\t    << ctrl_[p.first][setCount_].value\n-\t\t\t\t\t    << \" at index \"\n-\t\t\t\t\t    << setCount_;\n-\t\t}\n-\n-\t\treturn true;\n-\t}\n-\n-\tint write()\n-\t{\n-\t\tstd::lock_guard<std::mutex> lock(lock_);\n-\t\tlibcamera::ControlList controls(dev_->controls());\n-\n-\t\tfor (auto &p : ctrl_) {\n-\t\t\tint delayDiff = maxDelay_ - delay_[p.first];\n-\t\t\tint index = std::max<int>(0, setCount_ - delayDiff);\n-\n-\t\t\tif (p.second[index].updated) {\n-\t\t\t\t/* We need to write this value out. */\n-\t\t\t\tcontrols.set(p.first, p.second[index].value);\n-\t\t\t\tp.second[index].updated = false;\n-\t\t\t\tLOG(RPI_S_W, Debug) << \"Writing ctrl \"\n-\t\t\t\t\t\t    << hex(p.first) << \" to \"\n-\t\t\t\t\t\t    << p.second[index].value\n-\t\t\t\t\t\t    << \" at index \"\n-\t\t\t\t\t\t    << index;\n-\t\t\t}\n-\t\t}\n-\n-\t\tnextFrame();\n-\t\treturn dev_->setControls(&controls);\n-\t}\n-\n-\tvoid get(std::unordered_map<uint32_t, int32_t> &ctrl, uint8_t offset = 0)\n-\t{\n-\t\tstd::lock_guard<std::mutex> lock(lock_);\n-\n-\t\t/* Account for the offset to reset the getCounter. */\n-\t\tgetCount_ += offset + 1;\n-\n-\t\tctrl.clear();\n-\t\tfor (auto &p : ctrl_) {\n-\t\t\tint index = std::max<int>(0, getCount_ - maxDelay_);\n-\t\t\tctrl[p.first] = p.second[index].value;\n-\t\t\tLOG(RPI_S_W, Debug) << \"Getting ctrl \"\n-\t\t\t\t\t    << hex(p.first) << \" to \"\n-\t\t\t\t\t    << p.second[index].value\n-\t\t\t\t\t    << \" at index \"\n-\t\t\t\t\t    << index;\n-\t\t}\n-\t}\n+\tint write();\n \n private:\n-\tvoid nextFrame()\n-\t{\n-\t\t/* Advance the control history to the next frame */\n-\t\tint prevCount = setCount_;\n-\t\tsetCount_++;\n-\n-\t\tLOG(RPI_S_W, Debug) << \"Next frame, set index is \" << setCount_;\n-\n-\t\tfor (auto &p : ctrl_) {\n-\t\t\tp.second[setCount_].value = p.second[prevCount].value;\n-\t\t\tp.second[setCount_].updated = false;\n-\t\t}\n-\t}\n+\tvoid nextFrame();\n \n \t/* listSize must be a power of 2. */\n \tstatic constexpr int listSize = (1 << 4);\n","prefixes":["libcamera-devel","1/2"]}