From patchwork Fri Jun 14 21:22:59 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= X-Patchwork-Id: 20325 Return-Path: X-Original-To: parsemail@patchwork.libcamera.org Delivered-To: parsemail@patchwork.libcamera.org Received: from lancelot.ideasonboard.com (lancelot.ideasonboard.com [92.243.16.209]) by patchwork.libcamera.org (Postfix) with ESMTPS id F3F56BD87C for ; Fri, 14 Jun 2024 21:23:10 +0000 (UTC) Received: from lancelot.ideasonboard.com (localhost [IPv6:::1]) by lancelot.ideasonboard.com (Postfix) with ESMTP id 7FBE66548D; Fri, 14 Jun 2024 23:23:09 +0200 (CEST) Authentication-Results: lancelot.ideasonboard.com; dkim=pass (2048-bit key; unprotected) header.d=protonmail.com header.i=@protonmail.com header.b="OWeOlvJu"; dkim-atps=neutral Received: from mail-40134.protonmail.ch (mail-40134.protonmail.ch [185.70.40.134]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id ABC4661A2A for ; Fri, 14 Jun 2024 23:23:07 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com; s=protonmail3; t=1718400185; x=1718659385; bh=oME37TnBaJgkAox95RZh8FkXoB1eArJSiLyL4y9AMJY=; h=Date:To:From:Subject:Message-ID:Feedback-ID:From:To:Cc:Date: Subject:Reply-To:Feedback-ID:Message-ID:BIMI-Selector; b=OWeOlvJuPFOz1IRfsdQfD1W81b41pm5fPRkbPtCHzKHtCfjmCVWRKSKbnb3TvciNE jF6jV8GMEGfo275M8w/V7sqoOnATA7Sp2AsajG9fHbvcYAuTJDi65k9GNSwlhXnBxU kkLxeYcI+TkGrtV2FMHpIdupSY31fk5yJRGGa6aRwSzwlhSECJokdXJd5/LMv/HC8g MbZm2pzUj7KaTo31q1YRY3oBmJ/SMb28AZKe3QfpHzw5qWslfN7uHcfra6iu4bNlX9 JCzXbCwvqcDtgvtR9dKZ2ei6SVHywzxeVklBloZDVD1vb9MiuJzGWuB1WDbQ4MWAnr IJUp+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 possible Message-ID: <20240614212256.494291-1-pobrn@protonmail.com> Feedback-ID: 20568564:user:proton X-Pm-Message-ID: a59bb03915669824106bc3c595d418cccd343761 MIME-Version: 1.0 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: , Errors-To: libcamera-devel-bounces@lists.libcamera.org Sender: "libcamera-devel" Copy construction/assignment is more expensive than move construction/assignment because it involves managing the reference counts. In fact, libstdc++ even checks a global variable to determine whether threads are used. In contrast, move construction can be done without any conditionals or reference count changes. So prefer move construction and assignment where possible. Signed-off-by: Barnabás Pőcze Reviewed-by: Kieran Bingham --- src/android/camera_capabilities.cpp | 2 +- src/apps/cam/camera_session.cpp | 2 +- src/apps/cam/capture_script.cpp | 2 +- src/apps/lc-compliance/helpers/capture.cpp | 8 ++++---- src/apps/lc-compliance/main.cpp | 4 +--- src/apps/qcam/main_window.cpp | 4 ++-- src/gstreamer/gstlibcameraprovider.cpp | 3 +-- src/gstreamer/gstlibcamerasrc.cpp | 9 ++++----- src/libcamera/base/bound_method.cpp | 4 ++-- src/libcamera/base/message.cpp | 2 +- src/libcamera/camera_manager.cpp | 4 ++-- src/libcamera/pipeline_handler.cpp | 8 +++----- src/py/libcamera/py_main.cpp | 2 +- src/v4l2/v4l2_camera.cpp | 2 +- src/v4l2/v4l2_compat_manager.cpp | 2 +- 15 files changed, 26 insertions(+), 32 deletions(-) diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp index 71043e12..d34ed480 100644 --- a/src/android/camera_capabilities.cpp +++ b/src/android/camera_capabilities.cpp @@ -401,7 +401,7 @@ void CameraCapabilities::computeHwLevel( int CameraCapabilities::initialize(std::shared_ptr camera, int orientation, int facing) { - camera_ = camera; + camera_ = std::move(camera); orientation_ = orientation; facing_ = facing; rawStreamAvailable_ = false; diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp index 097dc479..4b6799b1 100644 --- a/src/apps/cam/camera_session.cpp +++ b/src/apps/cam/camera_session.cpp @@ -43,7 +43,7 @@ CameraSession::CameraSession(CameraManager *cm, if (*endptr == '\0' && index > 0) { auto cameras = cm->cameras(); if (index <= cameras.size()) - camera_ = cameras[index - 1]; + camera_ = std::move(cameras[index - 1]); } if (!camera_) diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp index fc1dfa75..7f502f5f 100644 --- a/src/apps/cam/capture_script.cpp +++ b/src/apps/cam/capture_script.cpp @@ -15,7 +15,7 @@ using namespace libcamera; CaptureScript::CaptureScript(std::shared_ptr camera, const std::string &fileName) - : camera_(camera), loop_(0), valid_(false) + : camera_(std::move(camera)), loop_(0), valid_(false) { FILE *fh = fopen(fileName.c_str(), "r"); if (!fh) { diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp index 90c1530b..d1dafb6c 100644 --- a/src/apps/lc-compliance/helpers/capture.cpp +++ b/src/apps/lc-compliance/helpers/capture.cpp @@ -12,8 +12,8 @@ using namespace libcamera; Capture::Capture(std::shared_ptr camera) - : loop_(nullptr), camera_(camera), - allocator_(std::make_unique(camera)) + : loop_(nullptr), camera_(std::move(camera)), + allocator_(std::make_unique(camera_)) { } @@ -72,7 +72,7 @@ void Capture::stop() /* CaptureBalanced */ CaptureBalanced::CaptureBalanced(std::shared_ptr camera) - : Capture(camera) + : Capture(std::move(camera)) { } @@ -144,7 +144,7 @@ void CaptureBalanced::requestComplete(Request *request) /* CaptureUnbalanced */ CaptureUnbalanced::CaptureUnbalanced(std::shared_ptr camera) - : Capture(camera) + : Capture(std::move(camera)) { } diff --git a/src/apps/lc-compliance/main.cpp b/src/apps/lc-compliance/main.cpp index 3f1d2a61..98f2573d 100644 --- a/src/apps/lc-compliance/main.cpp +++ b/src/apps/lc-compliance/main.cpp @@ -50,8 +50,6 @@ static void listCameras(CameraManager *cm) static int initCamera(CameraManager *cm, OptionsParser::Options options) { - std::shared_ptr camera; - int ret = cm->start(); if (ret) { std::cout << "Failed to start camera manager: " @@ -66,7 +64,7 @@ static int initCamera(CameraManager *cm, OptionsParser::Options options) } const std::string &cameraId = options[OptCamera]; - camera = cm->get(cameraId); + std::shared_ptr camera = cm->get(cameraId); if (!camera) { std::cout << "Camera " << cameraId << " not found, available cameras:" << std::endl; listCameras(cm); diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp index d515beed..bf7ddadb 100644 --- a/src/apps/qcam/main_window.cpp +++ b/src/apps/qcam/main_window.cpp @@ -269,7 +269,7 @@ void MainWindow::switchCamera() if (camera_ && newCameraId == camera_->id()) return; - const std::shared_ptr &cam = cm_->get(newCameraId); + std::shared_ptr cam = cm_->get(newCameraId); if (cam->acquire()) { qInfo() << "Failed to acquire camera" << cam->id().c_str(); @@ -287,7 +287,7 @@ void MainWindow::switchCamera() if (camera_) camera_->release(); - camera_ = cam; + camera_ = std::move(cam); startStopAction_->setChecked(true); diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp index 4fb1b007..60816562 100644 --- a/src/gstreamer/gstlibcameraprovider.cpp +++ b/src/gstreamer/gstlibcameraprovider.cpp @@ -189,7 +189,6 @@ static GList * gst_libcamera_provider_probe(GstDeviceProvider *provider) { GstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(provider); - std::shared_ptr cm; GList *devices = nullptr; gint ret; @@ -200,7 +199,7 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider) * gains monitoring support. Meanwhile we need to cycle start()/stop() * to ensure every probe() calls return the latest list. */ - cm = gst_libcamera_get_camera_manager(ret); + std::shared_ptr cm = gst_libcamera_get_camera_manager(ret); if (ret) { GST_ERROR_OBJECT(self, "Failed to retrieve device list: %s", g_strerror(-ret)); diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp index 9680d809..428170d8 100644 --- a/src/gstreamer/gstlibcamerasrc.cpp +++ b/src/gstreamer/gstlibcamerasrc.cpp @@ -355,13 +355,12 @@ void GstLibcameraSrcState::clearRequests() static bool gst_libcamera_src_open(GstLibcameraSrc *self) { - std::shared_ptr cm; std::shared_ptr cam; gint ret; GST_DEBUG_OBJECT(self, "Opening camera device ..."); - cm = gst_libcamera_get_camera_manager(ret); + std::shared_ptr cm = gst_libcamera_get_camera_manager(ret); if (ret) { GST_ELEMENT_ERROR(self, LIBRARY, INIT, ("Failed listing cameras."), @@ -392,7 +391,7 @@ gst_libcamera_src_open(GstLibcameraSrc *self) ("libcamera::CameraMananger::cameras() is empty")); return false; } - cam = cameras[0]; + cam = std::move(cameras[0]); } GST_INFO_OBJECT(self, "Using camera '%s'", cam->id().c_str()); @@ -408,8 +407,8 @@ gst_libcamera_src_open(GstLibcameraSrc *self) cam->requestCompleted.connect(self->state, &GstLibcameraSrcState::requestCompleted); /* No need to lock here, we didn't start our threads yet. */ - self->state->cm_ = cm; - self->state->cam_ = cam; + self->state->cm_ = std::move(cm); + self->state->cam_ = std::move(cam); return true; } diff --git a/src/libcamera/base/bound_method.cpp b/src/libcamera/base/bound_method.cpp index 322029a8..d888461e 100644 --- a/src/libcamera/base/bound_method.cpp +++ b/src/libcamera/base/bound_method.cpp @@ -89,7 +89,7 @@ bool BoundMethodBase::activatePack(std::shared_ptr pack, case ConnectionTypeQueued: { std::unique_ptr msg = - std::make_unique(this, pack, nullptr, deleteMethod); + std::make_unique(this, std::move(pack), nullptr, deleteMethod); object_->postMessage(std::move(msg)); return false; } @@ -98,7 +98,7 @@ bool BoundMethodBase::activatePack(std::shared_ptr pack, Semaphore semaphore; std::unique_ptr msg = - std::make_unique(this, pack, &semaphore, deleteMethod); + std::make_unique(this, std::move(pack), &semaphore, deleteMethod); object_->postMessage(std::move(msg)); semaphore.acquire(); diff --git a/src/libcamera/base/message.cpp b/src/libcamera/base/message.cpp index 098faac6..29ce1b21 100644 --- a/src/libcamera/base/message.cpp +++ b/src/libcamera/base/message.cpp @@ -127,7 +127,7 @@ Message::Type Message::registerMessageType() InvokeMessage::InvokeMessage(BoundMethodBase *method, std::shared_ptr pack, Semaphore *semaphore, bool deleteMethod) - : Message(Message::InvokeMessage), method_(method), pack_(pack), + : Message(Message::InvokeMessage), method_(method), pack_(std::move(pack)), semaphore_(semaphore), deleteMethod_(deleteMethod) { } diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp index 95a9e326..a20216a4 100644 --- a/src/libcamera/camera_manager.cpp +++ b/src/libcamera/camera_manager.cpp @@ -233,7 +233,7 @@ void CameraManager::Private::removeCamera(std::shared_ptr camera) MutexLocker locker(mutex_); auto iter = std::find_if(cameras_.begin(), cameras_.end(), - [camera](std::shared_ptr &c) { + [&camera](const std::shared_ptr &c) { return c.get() == camera.get(); }); if (iter == cameras_.end()) @@ -374,7 +374,7 @@ std::shared_ptr CameraManager::get(const std::string &id) MutexLocker locker(d->mutex_); - for (std::shared_ptr camera : d->cameras_) { + for (const std::shared_ptr &camera : d->cameras_) { if (camera->id() == id) return camera; } diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp index 5ea2ca78..ad1fe874 100644 --- a/src/libcamera/pipeline_handler.cpp +++ b/src/libcamera/pipeline_handler.cpp @@ -75,7 +75,7 @@ PipelineHandler::PipelineHandler(CameraManager *manager) PipelineHandler::~PipelineHandler() { - for (std::shared_ptr media : mediaDevices_) + for (std::shared_ptr &media : mediaDevices_) media->release(); } @@ -138,9 +138,7 @@ MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator, if (!media->acquire()) return nullptr; - mediaDevices_.push_back(media); - - return media.get(); + return mediaDevices_.emplace_back(std::move(media)).get(); } /** @@ -699,7 +697,7 @@ void PipelineHandler::disconnect() continue; camera->disconnect(); - manager_->_d()->removeCamera(camera); + manager_->_d()->removeCamera(std::move(camera)); } } diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp index bce08218..af683002 100644 --- a/src/py/libcamera/py_main.cpp +++ b/src/py/libcamera/py_main.cpp @@ -65,7 +65,7 @@ public: } explicit PyCameraSmartPtr(std::shared_ptr p) - : ptr_(p) + : ptr_(std::move(p)) { } diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp index 0f3b862f..9bd89f40 100644 --- a/src/v4l2/v4l2_camera.cpp +++ b/src/v4l2/v4l2_camera.cpp @@ -17,7 +17,7 @@ using namespace libcamera; LOG_DECLARE_CATEGORY(V4L2Compat) V4L2Camera::V4L2Camera(std::shared_ptr camera) - : camera_(camera), isRunning_(false), bufferAllocator_(nullptr), + : camera_(std::move(camera)), isRunning_(false), bufferAllocator_(nullptr), efd_(-1), bufferAvailableCount_(0) { camera_->requestCompleted.connect(this, &V4L2Camera::requestComplete); diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp index 6a00afb5..8d3ea8ee 100644 --- a/src/v4l2/v4l2_compat_manager.cpp +++ b/src/v4l2/v4l2_compat_manager.cpp @@ -85,7 +85,7 @@ int V4L2CompatManager::start() */ auto cameras = cm_->cameras(); for (auto [index, camera] : utils::enumerate(cameras)) { - V4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera); + V4L2CameraProxy *proxy = new V4L2CameraProxy(index, std::move(camera)); proxies_.emplace_back(proxy); }