{"id":20325,"url":"https://patchwork.libcamera.org/api/patches/20325/?format=json","web_url":"https://patchwork.libcamera.org/patch/20325/","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":"<20240614212256.494291-1-pobrn@protonmail.com>","date":"2024-06-14T21:22:59","name":"[v1] treewide: move construct/assign `shared_ptr`s where possible","commit_ref":null,"pull_url":null,"state":"deferred","archived":false,"hash":"6504a67806572c20b010e669089293d6c10ea76d","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/?format=json","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"delegate":null,"mbox":"https://patchwork.libcamera.org/patch/20325/mbox/","series":[{"id":4398,"url":"https://patchwork.libcamera.org/api/series/4398/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=4398","date":"2024-06-14T21:22:59","name":"[v1] treewide: move construct/assign `shared_ptr`s where possible","version":1,"mbox":"https://patchwork.libcamera.org/series/4398/mbox/"}],"comments":"https://patchwork.libcamera.org/api/patches/20325/comments/","check":"pending","checks":"https://patchwork.libcamera.org/api/patches/20325/checks/","tags":{},"headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id F3F56BD87C\n\tfor ;\n\tFri, 14 Jun 2024 21:23:10 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7FBE66548D;\n\tFri, 14 Jun 2024 23:23:09 +0200 (CEST)","from mail-40134.protonmail.ch (mail-40134.protonmail.ch\n\t[185.70.40.134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id ABC4661A2A\n\tfor ;\n\tFri, 14 Jun 2024 23:23:07 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key;\n\tunprotected) header.d=protonmail.com header.i=@protonmail.com\n\theader.b=\"OWeOlvJu\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1718400185; x=1718659385;\n\tbh=oME37TnBaJgkAox95RZh8FkXoB1eArJSiLyL4y9AMJY=;\n\th=Date:To:From:Subject:Message-ID:Feedback-ID:From:To:Cc:Date:\n\tSubject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector;\n\tb=OWeOlvJuPFOz1IRfsdQfD1W81b41pm5fPRkbPtCHzKHtCfjmCVWRKSKbnb3TvciNE\n\tjF6jV8GMEGfo275M8w/V7sqoOnATA7Sp2AsajG9fHbvcYAuTJDi65k9GNSwlhXnBxU\n\tkkLxeYcI+TkGrtV2FMHpIdupSY31fk5yJRGGa6aRwSzwlhSECJokdXJd5/LMv/HC8g\n\tMbZm2pzUj7KaTo31q1YRY3oBmJ/SMb28AZKe3QfpHzw5qWslfN7uHcfra6iu4bNlX9\n\tJCzXbCwvqcDtgvtR9dKZ2ei6SVHywzxeVklBloZDVD1vb9MiuJzGWuB1WDbQ4MWAnr\n\tIJUp+GxjL2lFw==","Date":"Fri, 14 Jun 2024 21:22:59 +0000","To":"libcamera-devel@lists.libcamera.org","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Subject":"[PATCH v1] treewide: move construct/assign `shared_ptr`s where\n\tpossible","Message-ID":"<20240614212256.494291-1-pobrn@protonmail.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"a59bb03915669824106bc3c595d418cccd343761","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"Copy construction/assignment is more expensive than move construction/assignment\nbecause it involves managing the reference counts. In fact, libstdc++ even\nchecks a global variable to determine whether threads are used.\n\nIn contrast, move construction can be done without any conditionals\nor reference count changes. So prefer move construction and assignment\nwhere possible.\n\nSigned-off-by: Barnabás Pőcze \n---\n src/android/camera_capabilities.cpp | 2 +-\n src/apps/cam/camera_session.cpp | 2 +-\n src/apps/cam/capture_script.cpp | 2 +-\n src/apps/lc-compliance/helpers/capture.cpp | 8 ++++----\n src/apps/lc-compliance/main.cpp | 4 +---\n src/apps/qcam/main_window.cpp | 4 ++--\n src/gstreamer/gstlibcameraprovider.cpp | 3 +--\n src/gstreamer/gstlibcamerasrc.cpp | 9 ++++-----\n src/libcamera/base/bound_method.cpp | 4 ++--\n src/libcamera/base/message.cpp | 2 +-\n src/libcamera/camera_manager.cpp | 4 ++--\n src/libcamera/pipeline_handler.cpp | 8 +++-----\n src/py/libcamera/py_main.cpp | 2 +-\n src/v4l2/v4l2_camera.cpp | 2 +-\n src/v4l2/v4l2_compat_manager.cpp | 2 +-\n 15 files changed, 26 insertions(+), 32 deletions(-)","diff":"diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\nindex 71043e12..d34ed480 100644\n--- a/src/android/camera_capabilities.cpp\n+++ b/src/android/camera_capabilities.cpp\n@@ -401,7 +401,7 @@ void CameraCapabilities::computeHwLevel(\n int CameraCapabilities::initialize(std::shared_ptr camera,\n \t\t\t\t int orientation, int facing)\n {\n-\tcamera_ = camera;\n+\tcamera_ = std::move(camera);\n \torientation_ = orientation;\n \tfacing_ = facing;\n \trawStreamAvailable_ = false;\ndiff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\nindex 097dc479..4b6799b1 100644\n--- a/src/apps/cam/camera_session.cpp\n+++ b/src/apps/cam/camera_session.cpp\n@@ -43,7 +43,7 @@ CameraSession::CameraSession(CameraManager *cm,\n \tif (*endptr == '\\0' && index > 0) {\n \t\tauto cameras = cm->cameras();\n \t\tif (index <= cameras.size())\n-\t\t\tcamera_ = cameras[index - 1];\n+\t\t\tcamera_ = std::move(cameras[index - 1]);\n \t}\n \n \tif (!camera_)\ndiff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\nindex fc1dfa75..7f502f5f 100644\n--- a/src/apps/cam/capture_script.cpp\n+++ b/src/apps/cam/capture_script.cpp\n@@ -15,7 +15,7 @@ using namespace libcamera;\n \n CaptureScript::CaptureScript(std::shared_ptr camera,\n \t\t\t const std::string &fileName)\n-\t: camera_(camera), loop_(0), valid_(false)\n+\t: camera_(std::move(camera)), loop_(0), valid_(false)\n {\n \tFILE *fh = fopen(fileName.c_str(), \"r\");\n \tif (!fh) {\ndiff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp\nindex 90c1530b..d1dafb6c 100644\n--- a/src/apps/lc-compliance/helpers/capture.cpp\n+++ b/src/apps/lc-compliance/helpers/capture.cpp\n@@ -12,8 +12,8 @@\n using namespace libcamera;\n \n Capture::Capture(std::shared_ptr camera)\n-\t: loop_(nullptr), camera_(camera),\n-\t allocator_(std::make_unique(camera))\n+\t: loop_(nullptr), camera_(std::move(camera)),\n+\t allocator_(std::make_unique(camera_))\n {\n }\n \n@@ -72,7 +72,7 @@ void Capture::stop()\n /* CaptureBalanced */\n \n CaptureBalanced::CaptureBalanced(std::shared_ptr camera)\n-\t: Capture(camera)\n+\t: Capture(std::move(camera))\n {\n }\n \n@@ -144,7 +144,7 @@ void CaptureBalanced::requestComplete(Request *request)\n /* CaptureUnbalanced */\n \n CaptureUnbalanced::CaptureUnbalanced(std::shared_ptr camera)\n-\t: Capture(camera)\n+\t: Capture(std::move(camera))\n {\n }\n \ndiff --git a/src/apps/lc-compliance/main.cpp b/src/apps/lc-compliance/main.cpp\nindex 3f1d2a61..98f2573d 100644\n--- a/src/apps/lc-compliance/main.cpp\n+++ b/src/apps/lc-compliance/main.cpp\n@@ -50,8 +50,6 @@ static void listCameras(CameraManager *cm)\n \n static int initCamera(CameraManager *cm, OptionsParser::Options options)\n {\n-\tstd::shared_ptr camera;\n-\n \tint ret = cm->start();\n \tif (ret) {\n \t\tstd::cout << \"Failed to start camera manager: \"\n@@ -66,7 +64,7 @@ static int initCamera(CameraManager *cm, OptionsParser::Options options)\n \t}\n \n \tconst std::string &cameraId = options[OptCamera];\n-\tcamera = cm->get(cameraId);\n+\tstd::shared_ptr camera = cm->get(cameraId);\n \tif (!camera) {\n \t\tstd::cout << \"Camera \" << cameraId << \" not found, available cameras:\" << std::endl;\n \t\tlistCameras(cm);\ndiff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp\nindex d515beed..bf7ddadb 100644\n--- a/src/apps/qcam/main_window.cpp\n+++ b/src/apps/qcam/main_window.cpp\n@@ -269,7 +269,7 @@ void MainWindow::switchCamera()\n \tif (camera_ && newCameraId == camera_->id())\n \t\treturn;\n \n-\tconst std::shared_ptr &cam = cm_->get(newCameraId);\n+\tstd::shared_ptr cam = cm_->get(newCameraId);\n \n \tif (cam->acquire()) {\n \t\tqInfo() << \"Failed to acquire camera\" << cam->id().c_str();\n@@ -287,7 +287,7 @@ void MainWindow::switchCamera()\n \tif (camera_)\n \t\tcamera_->release();\n \n-\tcamera_ = cam;\n+\tcamera_ = std::move(cam);\n \n \tstartStopAction_->setChecked(true);\n \ndiff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\nindex 4fb1b007..60816562 100644\n--- a/src/gstreamer/gstlibcameraprovider.cpp\n+++ b/src/gstreamer/gstlibcameraprovider.cpp\n@@ -189,7 +189,6 @@ static GList *\n gst_libcamera_provider_probe(GstDeviceProvider *provider)\n {\n \tGstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(provider);\n-\tstd::shared_ptr cm;\n \tGList *devices = nullptr;\n \tgint ret;\n \n@@ -200,7 +199,7 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider)\n \t * gains monitoring support. Meanwhile we need to cycle start()/stop()\n \t * to ensure every probe() calls return the latest list.\n \t */\n-\tcm = gst_libcamera_get_camera_manager(ret);\n+\tstd::shared_ptr cm = gst_libcamera_get_camera_manager(ret);\n \tif (ret) {\n \t\tGST_ERROR_OBJECT(self, \"Failed to retrieve device list: %s\",\n \t\t\t\t g_strerror(-ret));\ndiff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\nindex 9680d809..428170d8 100644\n--- a/src/gstreamer/gstlibcamerasrc.cpp\n+++ b/src/gstreamer/gstlibcamerasrc.cpp\n@@ -355,13 +355,12 @@ void GstLibcameraSrcState::clearRequests()\n static bool\n gst_libcamera_src_open(GstLibcameraSrc *self)\n {\n-\tstd::shared_ptr cm;\n \tstd::shared_ptr cam;\n \tgint ret;\n \n \tGST_DEBUG_OBJECT(self, \"Opening camera device ...\");\n \n-\tcm = gst_libcamera_get_camera_manager(ret);\n+\tstd::shared_ptr cm = gst_libcamera_get_camera_manager(ret);\n \tif (ret) {\n \t\tGST_ELEMENT_ERROR(self, LIBRARY, INIT,\n \t\t\t\t (\"Failed listing cameras.\"),\n@@ -392,7 +391,7 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n \t\t\t\t\t (\"libcamera::CameraMananger::cameras() is empty\"));\n \t\t\treturn false;\n \t\t}\n-\t\tcam = cameras[0];\n+\t\tcam = std::move(cameras[0]);\n \t}\n \n \tGST_INFO_OBJECT(self, \"Using camera '%s'\", cam->id().c_str());\n@@ -408,8 +407,8 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n \tcam->requestCompleted.connect(self->state, &GstLibcameraSrcState::requestCompleted);\n \n \t/* No need to lock here, we didn't start our threads yet. */\n-\tself->state->cm_ = cm;\n-\tself->state->cam_ = cam;\n+\tself->state->cm_ = std::move(cm);\n+\tself->state->cam_ = std::move(cam);\n \n \treturn true;\n }\ndiff --git a/src/libcamera/base/bound_method.cpp b/src/libcamera/base/bound_method.cpp\nindex 322029a8..d888461e 100644\n--- a/src/libcamera/base/bound_method.cpp\n+++ b/src/libcamera/base/bound_method.cpp\n@@ -89,7 +89,7 @@ bool BoundMethodBase::activatePack(std::shared_ptr pack,\n \n \tcase ConnectionTypeQueued: {\n \t\tstd::unique_ptr msg =\n-\t\t\tstd::make_unique(this, pack, nullptr, deleteMethod);\n+\t\t\tstd::make_unique(this, std::move(pack), nullptr, deleteMethod);\n \t\tobject_->postMessage(std::move(msg));\n \t\treturn false;\n \t}\n@@ -98,7 +98,7 @@ bool BoundMethodBase::activatePack(std::shared_ptr pack,\n \t\tSemaphore semaphore;\n \n \t\tstd::unique_ptr msg =\n-\t\t\tstd::make_unique(this, pack, &semaphore, deleteMethod);\n+\t\t\tstd::make_unique(this, std::move(pack), &semaphore, deleteMethod);\n \t\tobject_->postMessage(std::move(msg));\n \n \t\tsemaphore.acquire();\ndiff --git a/src/libcamera/base/message.cpp b/src/libcamera/base/message.cpp\nindex 098faac6..29ce1b21 100644\n--- a/src/libcamera/base/message.cpp\n+++ b/src/libcamera/base/message.cpp\n@@ -127,7 +127,7 @@ Message::Type Message::registerMessageType()\n InvokeMessage::InvokeMessage(BoundMethodBase *method,\n \t\t\t std::shared_ptr pack,\n \t\t\t Semaphore *semaphore, bool deleteMethod)\n-\t: Message(Message::InvokeMessage), method_(method), pack_(pack),\n+\t: Message(Message::InvokeMessage), method_(method), pack_(std::move(pack)),\n \t semaphore_(semaphore), deleteMethod_(deleteMethod)\n {\n }\ndiff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\nindex 95a9e326..a20216a4 100644\n--- a/src/libcamera/camera_manager.cpp\n+++ b/src/libcamera/camera_manager.cpp\n@@ -233,7 +233,7 @@ void CameraManager::Private::removeCamera(std::shared_ptr camera)\n \tMutexLocker locker(mutex_);\n \n \tauto iter = std::find_if(cameras_.begin(), cameras_.end(),\n-\t\t\t\t [camera](std::shared_ptr &c) {\n+\t\t\t\t [&camera](const std::shared_ptr &c) {\n \t\t\t\t\t return c.get() == camera.get();\n \t\t\t\t });\n \tif (iter == cameras_.end())\n@@ -374,7 +374,7 @@ std::shared_ptr CameraManager::get(const std::string &id)\n \n \tMutexLocker locker(d->mutex_);\n \n-\tfor (std::shared_ptr camera : d->cameras_) {\n+\tfor (const std::shared_ptr &camera : d->cameras_) {\n \t\tif (camera->id() == id)\n \t\t\treturn camera;\n \t}\ndiff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\nindex 5ea2ca78..ad1fe874 100644\n--- a/src/libcamera/pipeline_handler.cpp\n+++ b/src/libcamera/pipeline_handler.cpp\n@@ -75,7 +75,7 @@ PipelineHandler::PipelineHandler(CameraManager *manager)\n \n PipelineHandler::~PipelineHandler()\n {\n-\tfor (std::shared_ptr media : mediaDevices_)\n+\tfor (std::shared_ptr &media : mediaDevices_)\n \t\tmedia->release();\n }\n \n@@ -138,9 +138,7 @@ MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator,\n \tif (!media->acquire())\n \t\treturn nullptr;\n \n-\tmediaDevices_.push_back(media);\n-\n-\treturn media.get();\n+\treturn mediaDevices_.emplace_back(std::move(media)).get();\n }\n \n /**\n@@ -699,7 +697,7 @@ void PipelineHandler::disconnect()\n \t\t\tcontinue;\n \n \t\tcamera->disconnect();\n-\t\tmanager_->_d()->removeCamera(camera);\n+\t\tmanager_->_d()->removeCamera(std::move(camera));\n \t}\n }\n \ndiff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\nindex bce08218..af683002 100644\n--- a/src/py/libcamera/py_main.cpp\n+++ b/src/py/libcamera/py_main.cpp\n@@ -65,7 +65,7 @@ public:\n \t}\n \n \texplicit PyCameraSmartPtr(std::shared_ptr p)\n-\t\t: ptr_(p)\n+\t\t: ptr_(std::move(p))\n \t{\n \t}\n \ndiff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\nindex 0f3b862f..9bd89f40 100644\n--- a/src/v4l2/v4l2_camera.cpp\n+++ b/src/v4l2/v4l2_camera.cpp\n@@ -17,7 +17,7 @@ using namespace libcamera;\n LOG_DECLARE_CATEGORY(V4L2Compat)\n \n V4L2Camera::V4L2Camera(std::shared_ptr camera)\n-\t: camera_(camera), isRunning_(false), bufferAllocator_(nullptr),\n+\t: camera_(std::move(camera)), isRunning_(false), bufferAllocator_(nullptr),\n \t efd_(-1), bufferAvailableCount_(0)\n {\n \tcamera_->requestCompleted.connect(this, &V4L2Camera::requestComplete);\ndiff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\nindex 6a00afb5..8d3ea8ee 100644\n--- a/src/v4l2/v4l2_compat_manager.cpp\n+++ b/src/v4l2/v4l2_compat_manager.cpp\n@@ -85,7 +85,7 @@ int V4L2CompatManager::start()\n \t */\n \tauto cameras = cm_->cameras();\n \tfor (auto [index, camera] : utils::enumerate(cameras)) {\n-\t\tV4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera);\n+\t\tV4L2CameraProxy *proxy = new V4L2CameraProxy(index, std::move(camera));\n \t\tproxies_.emplace_back(proxy);\n \t}\n \n","prefixes":["v1"]}