[{"id":29952,"web_url":"https://patchwork.libcamera.org/comment/29952/","msgid":"","date":"2024-06-14T21:29:42","subject":"Re: [PATCH v1] treewide: move construct/assign `shared_ptr`s where\n\tpossible","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"Oops, of course the title should have a capital \"M\".\nDoes `utils/checkstyle.py` not check that or am I just holding it wrong?\n\n\nRegards,\nBarnabás Pőcze\n\n\n2024. június 14., péntek 23:22 keltezéssel, Barnabás Pőcze írta:\n\n> Copy construction/assignment is more expensive than move construction/assignment\n> because it involves managing the reference counts. In fact, libstdc++ even\n> checks a global variable to determine whether threads are used.\n> \n> In contrast, move construction can be done without any conditionals\n> or reference count changes. So prefer move construction and assignment\n> where possible.\n> \n> Signed-off-by: Barnabás Pőcze pobrn@protonmail.com\n> \n> ---\n> [...]","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 57269C3237\n\tfor ;\n\tFri, 14 Jun 2024 21:29:49 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 46E336548D;\n\tFri, 14 Jun 2024 23:29:48 +0200 (CEST)","from mail-4322.protonmail.ch (mail-4322.protonmail.ch\n\t[185.70.43.22])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 181D061A2A\n\tfor ;\n\tFri, 14 Jun 2024 23:29:47 +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=\"tvFmgh48\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1718400586; x=1718659786;\n\tbh=v3XOpfeyiG9ggcStyPkzOLFqpzEAbo9D9vPsYqX/HVw=;\n\th=Date:To:From:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=tvFmgh481pdjgNxFUY/J1HRdkxzQR/L5ewtrCMHriP5uyjGzqCZaMwkxW2dI1BgCa\n\tTwfAJ1rDI4S2MAzp6mKudS+7dUA220d3BhC/OEXcosJAn9BAu8XBNkHaPOBcB+k/xJ\n\tSj337bWgeHi8T47Vrk75ggZUqhk4gAFNnN78Xve7R5ADoFL0RItfk51LXmFsxLCLjH\n\tFvDrM6WwSLMBKKk6GhF4I9kQjPKZ9z20qp4QtCMznBSVm8q6scII25nqI5UF5El08l\n\thE9FDRwEhTh/HHVXpBTEZMixOAvq5PCwb8ljT55KIx2MQr75o5RxfwuW1TokTh7S0v\n\tALzt8DO4n4kGw==","Date":"Fri, 14 Jun 2024 21:29:42 +0000","To":"libcamera-devel@lists.libcamera.org","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Subject":"Re: [PATCH v1] treewide: move construct/assign `shared_ptr`s where\n\tpossible","Message-ID":"","In-Reply-To":"<20240614212256.494291-1-pobrn@protonmail.com>","References":"<20240614212256.494291-1-pobrn@protonmail.com>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"130a6acba46ff35bf98b278113613c3c752e844b","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\" "}},{"id":30016,"web_url":"https://patchwork.libcamera.org/comment/30016/","msgid":"<171866595818.804094.376698690946371877@ping.linuxembedded.co.uk>","date":"2024-06-17T23:12:38","subject":"Re: [PATCH v1] treewide: move construct/assign `shared_ptr`s where\n\tpossible","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2024-06-14 22:22:59)\n> Copy construction/assignment is more expensive than move construction/assignment\n> because it involves managing the reference counts. In fact, libstdc++ even\n> checks a global variable to determine whether threads are used.\n> \n> In contrast, move construction can be done without any conditionals\n> or reference count changes. So prefer move construction and assignment\n> where possible.\n\nCI-'Green': https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1203640\n\nExcept for the Soraka hardware test - but I don't yet know if that's\nan expected failure or not ...\n\n> \n> Signed-off-by: Barnabás Pőcze \n\nOtherwise this sounds reasonable to me, and the builds are fine...\n\n\nReviewed-by: Kieran Bingham \n\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(-)\n> \n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 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> int orientation, int facing)\n> {\n> - camera_ = camera;\n> + camera_ = std::move(camera);\n> orientation_ = orientation;\n> facing_ = facing;\n> rawStreamAvailable_ = false;\n> diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> index 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> if (*endptr == '\\0' && index > 0) {\n> auto cameras = cm->cameras();\n> if (index <= cameras.size())\n> - camera_ = cameras[index - 1];\n> + camera_ = std::move(cameras[index - 1]);\n> }\n> \n> if (!camera_)\n> diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n> index 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> const std::string &fileName)\n> - : camera_(camera), loop_(0), valid_(false)\n> + : camera_(std::move(camera)), loop_(0), valid_(false)\n> {\n> FILE *fh = fopen(fileName.c_str(), \"r\");\n> if (!fh) {\n> diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp\n> index 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> - : loop_(nullptr), camera_(camera),\n> - allocator_(std::make_unique(camera))\n> + : loop_(nullptr), camera_(std::move(camera)),\n> + 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> - : Capture(camera)\n> + : 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> - : Capture(camera)\n> + : Capture(std::move(camera))\n> {\n> }\n> \n> diff --git a/src/apps/lc-compliance/main.cpp b/src/apps/lc-compliance/main.cpp\n> index 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> - std::shared_ptr camera;\n> -\n> int ret = cm->start();\n> if (ret) {\n> std::cout << \"Failed to start camera manager: \"\n> @@ -66,7 +64,7 @@ static int initCamera(CameraManager *cm, OptionsParser::Options options)\n> }\n> \n> const std::string &cameraId = options[OptCamera];\n> - camera = cm->get(cameraId);\n> + std::shared_ptr camera = cm->get(cameraId);\n> if (!camera) {\n> std::cout << \"Camera \" << cameraId << \" not found, available cameras:\" << std::endl;\n> listCameras(cm);\n> diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp\n> index 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> if (camera_ && newCameraId == camera_->id())\n> return;\n> \n> - const std::shared_ptr &cam = cm_->get(newCameraId);\n> + std::shared_ptr cam = cm_->get(newCameraId);\n> \n> if (cam->acquire()) {\n> qInfo() << \"Failed to acquire camera\" << cam->id().c_str();\n> @@ -287,7 +287,7 @@ void MainWindow::switchCamera()\n> if (camera_)\n> camera_->release();\n> \n> - camera_ = cam;\n> + camera_ = std::move(cam);\n> \n> startStopAction_->setChecked(true);\n> \n> diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> index 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> GstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(provider);\n> - std::shared_ptr cm;\n> GList *devices = nullptr;\n> gint ret;\n> \n> @@ -200,7 +199,7 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider)\n> * gains monitoring support. Meanwhile we need to cycle start()/stop()\n> * to ensure every probe() calls return the latest list.\n> */\n> - cm = gst_libcamera_get_camera_manager(ret);\n> + std::shared_ptr cm = gst_libcamera_get_camera_manager(ret);\n> if (ret) {\n> GST_ERROR_OBJECT(self, \"Failed to retrieve device list: %s\",\n> g_strerror(-ret));\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 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> - std::shared_ptr cm;\n> std::shared_ptr cam;\n> gint ret;\n> \n> GST_DEBUG_OBJECT(self, \"Opening camera device ...\");\n> \n> - cm = gst_libcamera_get_camera_manager(ret);\n> + std::shared_ptr cm = gst_libcamera_get_camera_manager(ret);\n> if (ret) {\n> GST_ELEMENT_ERROR(self, LIBRARY, INIT,\n> (\"Failed listing cameras.\"),\n> @@ -392,7 +391,7 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n> (\"libcamera::CameraMananger::cameras() is empty\"));\n> return false;\n> }\n> - cam = cameras[0];\n> + cam = std::move(cameras[0]);\n> }\n> \n> GST_INFO_OBJECT(self, \"Using camera '%s'\", cam->id().c_str());\n> @@ -408,8 +407,8 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n> cam->requestCompleted.connect(self->state, &GstLibcameraSrcState::requestCompleted);\n> \n> /* No need to lock here, we didn't start our threads yet. */\n> - self->state->cm_ = cm;\n> - self->state->cam_ = cam;\n> + self->state->cm_ = std::move(cm);\n> + self->state->cam_ = std::move(cam);\n> \n> return true;\n> }\n> diff --git a/src/libcamera/base/bound_method.cpp b/src/libcamera/base/bound_method.cpp\n> index 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> case ConnectionTypeQueued: {\n> std::unique_ptr msg =\n> - std::make_unique(this, pack, nullptr, deleteMethod);\n> + std::make_unique(this, std::move(pack), nullptr, deleteMethod);\n> object_->postMessage(std::move(msg));\n> return false;\n> }\n> @@ -98,7 +98,7 @@ bool BoundMethodBase::activatePack(std::shared_ptr pack,\n> Semaphore semaphore;\n> \n> std::unique_ptr msg =\n> - std::make_unique(this, pack, &semaphore, deleteMethod);\n> + std::make_unique(this, std::move(pack), &semaphore, deleteMethod);\n> object_->postMessage(std::move(msg));\n> \n> semaphore.acquire();\n> diff --git a/src/libcamera/base/message.cpp b/src/libcamera/base/message.cpp\n> index 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> std::shared_ptr pack,\n> Semaphore *semaphore, bool deleteMethod)\n> - : Message(Message::InvokeMessage), method_(method), pack_(pack),\n> + : Message(Message::InvokeMessage), method_(method), pack_(std::move(pack)),\n> semaphore_(semaphore), deleteMethod_(deleteMethod)\n> {\n> }\n> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> index 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> MutexLocker locker(mutex_);\n> \n> auto iter = std::find_if(cameras_.begin(), cameras_.end(),\n> - [camera](std::shared_ptr &c) {\n> + [&camera](const std::shared_ptr &c) {\n> return c.get() == camera.get();\n> });\n> if (iter == cameras_.end())\n> @@ -374,7 +374,7 @@ std::shared_ptr CameraManager::get(const std::string &id)\n> \n> MutexLocker locker(d->mutex_);\n> \n> - for (std::shared_ptr camera : d->cameras_) {\n> + for (const std::shared_ptr &camera : d->cameras_) {\n> if (camera->id() == id)\n> return camera;\n> }\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index 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> - for (std::shared_ptr media : mediaDevices_)\n> + for (std::shared_ptr &media : mediaDevices_)\n> media->release();\n> }\n> \n> @@ -138,9 +138,7 @@ MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator,\n> if (!media->acquire())\n> return nullptr;\n> \n> - mediaDevices_.push_back(media);\n> -\n> - return media.get();\n> + return mediaDevices_.emplace_back(std::move(media)).get();\n> }\n> \n> /**\n> @@ -699,7 +697,7 @@ void PipelineHandler::disconnect()\n> continue;\n> \n> camera->disconnect();\n> - manager_->_d()->removeCamera(camera);\n> + manager_->_d()->removeCamera(std::move(camera));\n> }\n> }\n> \n> diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> index 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> }\n> \n> explicit PyCameraSmartPtr(std::shared_ptr p)\n> - : ptr_(p)\n> + : ptr_(std::move(p))\n> {\n> }\n> \n> diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\n> index 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> - : camera_(camera), isRunning_(false), bufferAllocator_(nullptr),\n> + : camera_(std::move(camera)), isRunning_(false), bufferAllocator_(nullptr),\n> efd_(-1), bufferAvailableCount_(0)\n> {\n> camera_->requestCompleted.connect(this, &V4L2Camera::requestComplete);\n> diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> index 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> */\n> auto cameras = cm_->cameras();\n> for (auto [index, camera] : utils::enumerate(cameras)) {\n> - V4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera);\n> + V4L2CameraProxy *proxy = new V4L2CameraProxy(index, std::move(camera));\n> proxies_.emplace_back(proxy);\n> }\n> \n> -- \n> 2.45.2\n> \n>","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 8D8A0C3237\n\tfor ;\n\tMon, 17 Jun 2024 23:12:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 375186548B;\n\tTue, 18 Jun 2024 01:12:43 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 4AC2361A20\n\tfor ;\n\tTue, 18 Jun 2024 01:12:41 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 25000289;\n\tTue, 18 Jun 2024 01:12:24 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"TqPzkfuq\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1718665944;\n\tbh=+PgKl7dO4LWZi6T0EwCaQUWFIYSKatEwdqx8fiBNNBQ=;\n\th=In-Reply-To:References:Subject:From:To:Date:From;\n\tb=TqPzkfuq0F6v05JgBkfUY27YxEzg4NCpWkVGkABDr3HV65OcAlJWUeiOUYZTywjlO\n\ttjOLmcHdzvEIJ86vHewHoRIMTcLvYxYTLr59dmTCrlbhePuDVt+4g1z0dgqLCRO3Si\n\tGFQPR7t+myfXRWsKNW6wskpkdoWOvbexWFWScR04=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20240614212256.494291-1-pobrn@protonmail.com>","References":"<20240614212256.494291-1-pobrn@protonmail.com>","Subject":"Re: [PATCH v1] treewide: move construct/assign `shared_ptr`s where\n\tpossible","From":"Kieran Bingham ","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 18 Jun 2024 00:12:38 +0100","Message-ID":"<171866595818.804094.376698690946371877@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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\" "}},{"id":30494,"web_url":"https://patchwork.libcamera.org/comment/30494/","msgid":"<5j8WgrtwWcO4K5dd8ICTZA0HsZsnNZzi37Y3r64JpSGKeWtSS9B-PSS6OCLaTH7KEe2D6I-8VRkxoYWCLjqwx5eYgG4RLRcaUOhPwoJttyo=@protonmail.com>","date":"2024-07-29T18:33:52","subject":"Re: [PATCH v1] treewide: move construct/assign `shared_ptr`s where\n\tpossible","submitter":{"id":133,"url":"https://patchwork.libcamera.org/api/people/133/","name":"Pőcze Barnabás","email":"pobrn@protonmail.com"},"content":"2024. június 18., kedd 1:12 keltezéssel, Kieran Bingham írta:\n\n> Quoting Barnabás Pőcze (2024-06-14 22:22:59)\n> > Copy construction/assignment is more expensive than move construction/assignment\n> > because it involves managing the reference counts. In fact, libstdc++ even\n> > checks a global variable to determine whether threads are used.\n> >\n> > In contrast, move construction can be done without any conditionals\n> > or reference count changes. So prefer move construction and assignment\n> > where possible.\n> \n> CI-'Green': https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1203640\n> \n> Except for the Soraka hardware test - but I don't yet know if that's\n> an expected failure or not ...\n> \n\nIs there anything I can help with here?\n\n\nRegards,\nBarnabás Pőcze\n\n\n> >\n> > Signed-off-by: Barnabás Pőcze \n> \n> Otherwise this sounds reasonable to me, and the builds are fine...\n> \n> \n> Reviewed-by: Kieran Bingham \n> \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(-)\n> >\n> > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > index 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> > int orientation, int facing)\n> > {\n> > - camera_ = camera;\n> > + camera_ = std::move(camera);\n> > orientation_ = orientation;\n> > facing_ = facing;\n> > rawStreamAvailable_ = false;\n> > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> > index 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> > if (*endptr == '\\0' && index > 0) {\n> > auto cameras = cm->cameras();\n> > if (index <= cameras.size())\n> > - camera_ = cameras[index - 1];\n> > + camera_ = std::move(cameras[index - 1]);\n> > }\n> >\n> > if (!camera_)\n> > diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n> > index 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> > const std::string &fileName)\n> > - : camera_(camera), loop_(0), valid_(false)\n> > + : camera_(std::move(camera)), loop_(0), valid_(false)\n> > {\n> > FILE *fh = fopen(fileName.c_str(), \"r\");\n> > if (!fh) {\n> > diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp\n> > index 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> > - : loop_(nullptr), camera_(camera),\n> > - allocator_(std::make_unique(camera))\n> > + : loop_(nullptr), camera_(std::move(camera)),\n> > + 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> > - : Capture(camera)\n> > + : 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> > - : Capture(camera)\n> > + : Capture(std::move(camera))\n> > {\n> > }\n> >\n> > diff --git a/src/apps/lc-compliance/main.cpp b/src/apps/lc-compliance/main.cpp\n> > index 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> > - std::shared_ptr camera;\n> > -\n> > int ret = cm->start();\n> > if (ret) {\n> > std::cout << \"Failed to start camera manager: \"\n> > @@ -66,7 +64,7 @@ static int initCamera(CameraManager *cm, OptionsParser::Options options)\n> > }\n> >\n> > const std::string &cameraId = options[OptCamera];\n> > - camera = cm->get(cameraId);\n> > + std::shared_ptr camera = cm->get(cameraId);\n> > if (!camera) {\n> > std::cout << \"Camera \" << cameraId << \" not found, available cameras:\" << std::endl;\n> > listCameras(cm);\n> > diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp\n> > index 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> > if (camera_ && newCameraId == camera_->id())\n> > return;\n> >\n> > - const std::shared_ptr &cam = cm_->get(newCameraId);\n> > + std::shared_ptr cam = cm_->get(newCameraId);\n> >\n> > if (cam->acquire()) {\n> > qInfo() << \"Failed to acquire camera\" << cam->id().c_str();\n> > @@ -287,7 +287,7 @@ void MainWindow::switchCamera()\n> > if (camera_)\n> > camera_->release();\n> >\n> > - camera_ = cam;\n> > + camera_ = std::move(cam);\n> >\n> > startStopAction_->setChecked(true);\n> >\n> > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> > index 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> > GstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(provider);\n> > - std::shared_ptr cm;\n> > GList *devices = nullptr;\n> > gint ret;\n> >\n> > @@ -200,7 +199,7 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider)\n> > * gains monitoring support. Meanwhile we need to cycle start()/stop()\n> > * to ensure every probe() calls return the latest list.\n> > */\n> > - cm = gst_libcamera_get_camera_manager(ret);\n> > + std::shared_ptr cm = gst_libcamera_get_camera_manager(ret);\n> > if (ret) {\n> > GST_ERROR_OBJECT(self, \"Failed to retrieve device list: %s\",\n> > g_strerror(-ret));\n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index 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> > - std::shared_ptr cm;\n> > std::shared_ptr cam;\n> > gint ret;\n> >\n> > GST_DEBUG_OBJECT(self, \"Opening camera device ...\");\n> >\n> > - cm = gst_libcamera_get_camera_manager(ret);\n> > + std::shared_ptr cm = gst_libcamera_get_camera_manager(ret);\n> > if (ret) {\n> > GST_ELEMENT_ERROR(self, LIBRARY, INIT,\n> > (\"Failed listing cameras.\"),\n> > @@ -392,7 +391,7 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n> > (\"libcamera::CameraMananger::cameras() is empty\"));\n> > return false;\n> > }\n> > - cam = cameras[0];\n> > + cam = std::move(cameras[0]);\n> > }\n> >\n> > GST_INFO_OBJECT(self, \"Using camera '%s'\", cam->id().c_str());\n> > @@ -408,8 +407,8 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n> > cam->requestCompleted.connect(self->state, &GstLibcameraSrcState::requestCompleted);\n> >\n> > /* No need to lock here, we didn't start our threads yet. */\n> > - self->state->cm_ = cm;\n> > - self->state->cam_ = cam;\n> > + self->state->cm_ = std::move(cm);\n> > + self->state->cam_ = std::move(cam);\n> >\n> > return true;\n> > }\n> > diff --git a/src/libcamera/base/bound_method.cpp b/src/libcamera/base/bound_method.cpp\n> > index 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> > case ConnectionTypeQueued: {\n> > std::unique_ptr msg =\n> > - std::make_unique(this, pack, nullptr, deleteMethod);\n> > + std::make_unique(this, std::move(pack), nullptr, deleteMethod);\n> > object_->postMessage(std::move(msg));\n> > return false;\n> > }\n> > @@ -98,7 +98,7 @@ bool BoundMethodBase::activatePack(std::shared_ptr pack,\n> > Semaphore semaphore;\n> >\n> > std::unique_ptr msg =\n> > - std::make_unique(this, pack, &semaphore, deleteMethod);\n> > + std::make_unique(this, std::move(pack), &semaphore, deleteMethod);\n> > object_->postMessage(std::move(msg));\n> >\n> > semaphore.acquire();\n> > diff --git a/src/libcamera/base/message.cpp b/src/libcamera/base/message.cpp\n> > index 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> > std::shared_ptr pack,\n> > Semaphore *semaphore, bool deleteMethod)\n> > - : Message(Message::InvokeMessage), method_(method), pack_(pack),\n> > + : Message(Message::InvokeMessage), method_(method), pack_(std::move(pack)),\n> > semaphore_(semaphore), deleteMethod_(deleteMethod)\n> > {\n> > }\n> > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > index 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> > MutexLocker locker(mutex_);\n> >\n> > auto iter = std::find_if(cameras_.begin(), cameras_.end(),\n> > - [camera](std::shared_ptr &c) {\n> > + [&camera](const std::shared_ptr &c) {\n> > return c.get() == camera.get();\n> > });\n> > if (iter == cameras_.end())\n> > @@ -374,7 +374,7 @@ std::shared_ptr CameraManager::get(const std::string &id)\n> >\n> > MutexLocker locker(d->mutex_);\n> >\n> > - for (std::shared_ptr camera : d->cameras_) {\n> > + for (const std::shared_ptr &camera : d->cameras_) {\n> > if (camera->id() == id)\n> > return camera;\n> > }\n> > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > index 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> > - for (std::shared_ptr media : mediaDevices_)\n> > + for (std::shared_ptr &media : mediaDevices_)\n> > media->release();\n> > }\n> >\n> > @@ -138,9 +138,7 @@ MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator,\n> > if (!media->acquire())\n> > return nullptr;\n> >\n> > - mediaDevices_.push_back(media);\n> > -\n> > - return media.get();\n> > + return mediaDevices_.emplace_back(std::move(media)).get();\n> > }\n> >\n> > /**\n> > @@ -699,7 +697,7 @@ void PipelineHandler::disconnect()\n> > continue;\n> >\n> > camera->disconnect();\n> > - manager_->_d()->removeCamera(camera);\n> > + manager_->_d()->removeCamera(std::move(camera));\n> > }\n> > }\n> >\n> > diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> > index 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> > }\n> >\n> > explicit PyCameraSmartPtr(std::shared_ptr p)\n> > - : ptr_(p)\n> > + : ptr_(std::move(p))\n> > {\n> > }\n> >\n> > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\n> > index 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> > - : camera_(camera), isRunning_(false), bufferAllocator_(nullptr),\n> > + : camera_(std::move(camera)), isRunning_(false), bufferAllocator_(nullptr),\n> > efd_(-1), bufferAvailableCount_(0)\n> > {\n> > camera_->requestCompleted.connect(this, &V4L2Camera::requestComplete);\n> > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> > index 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> > */\n> > auto cameras = cm_->cameras();\n> > for (auto [index, camera] : utils::enumerate(cameras)) {\n> > - V4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera);\n> > + V4L2CameraProxy *proxy = new V4L2CameraProxy(index, std::move(camera));\n> > proxies_.emplace_back(proxy);\n> > }\n> >\n> > --\n> > 2.45.2\n> >\n> >\n>","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 CD710C323E\n\tfor ;\n\tMon, 29 Jul 2024 18:34:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C2A4E63370;\n\tMon, 29 Jul 2024 20:33:59 +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 F278F61984\n\tfor ;\n\tMon, 29 Jul 2024 20:33:57 +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=\"Wd18BQuw\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed; d=protonmail.com;\n\ts=protonmail3; t=1722278037; x=1722537237;\n\tbh=OGSc7Fks1o+iUD5tgVIsVU6NoqZLuvFLG+PXtsNpNBs=;\n\th=Date:To:From:Cc:Subject:Message-ID:In-Reply-To:References:\n\tFeedback-ID:From:To:Cc:Date:Subject:Reply-To:Feedback-ID:\n\tMessage-ID:BIMI-Selector;\n\tb=Wd18BQuwoafkambEUaROdpAeyjj4ba//DMJRgccoEkkGDcCXXkqFyXwfuIQXLMdUw\n\t3LPuiRdovR/y+UO0cUHskl9LwZiPqrs/xFeGVlyn7hV+AyYDzhnF8xxB71pp0IDebB\n\tkw//wOwU3tPLbXe95g/mpFTHAlcsyXuLL/QPxoaVeYxFlgWIF4jFoNljwK2qYL1+BC\n\tbBZwp0DExAXRG4g5ygobwVP+4kS4ezjtvO2QdHKt22lIcOz1KkbPcLFjHKCOgykwWF\n\td2VMQLdlAUZpG2I4JTx9jEv3p1P+Ox19wc+E4TUVm+tUFR9YhewF167u4pJ/2p1cA5\n\tjuVrfNCP/YRBw==","Date":"Mon, 29 Jul 2024 18:33:52 +0000","To":"Kieran Bingham ","From":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v1] treewide: move construct/assign `shared_ptr`s where\n\tpossible","Message-ID":"<5j8WgrtwWcO4K5dd8ICTZA0HsZsnNZzi37Y3r64JpSGKeWtSS9B-PSS6OCLaTH7KEe2D6I-8VRkxoYWCLjqwx5eYgG4RLRcaUOhPwoJttyo=@protonmail.com>","In-Reply-To":"<171866595818.804094.376698690946371877@ping.linuxembedded.co.uk>","References":"<20240614212256.494291-1-pobrn@protonmail.com>\n\t<171866595818.804094.376698690946371877@ping.linuxembedded.co.uk>","Feedback-ID":"20568564:user:proton","X-Pm-Message-ID":"e7407fb059ebbd7153db6825c3d97ec1f91aeddd","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\" "}},{"id":30521,"web_url":"https://patchwork.libcamera.org/comment/30521/","msgid":"<172242629823.392292.1966838620806583894@ping.linuxembedded.co.uk>","date":"2024-07-31T11:44:58","subject":"Re: [PATCH v1] treewide: move construct/assign `shared_ptr`s where\n\tpossible","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Barnabás Pőcze (2024-07-29 19:33:52)\n> 2024. június 18., kedd 1:12 keltezéssel, Kieran Bingham írta:\n> \n> > Quoting Barnabás Pőcze (2024-06-14 22:22:59)\n> > > Copy construction/assignment is more expensive than move construction/assignment\n> > > because it involves managing the reference counts. In fact, libstdc++ even\n> > > checks a global variable to determine whether threads are used.\n> > >\n> > > In contrast, move construction can be done without any conditionals\n> > > or reference count changes. So prefer move construction and assignment\n> > > where possible.\n> > \n> > CI-'Green': https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1203640\n> > \n> > Except for the Soraka hardware test - but I don't yet know if that's\n> > an expected failure or not ...\n> > \n> \n> Is there anything I can help with here?\n> \n\nI was about to re-run ... but I think there's another failure\n(unrelated) on the Soraka hardware so I can't yet from what I can tell.\n\nWill try again once the ChromeOS tests are back online.\n\n--\nKieran\n\n\n> \n> Regards,\n> Barnabás Pőcze\n> \n> \n> > >\n> > > Signed-off-by: Barnabás Pőcze \n> > \n> > Otherwise this sounds reasonable to me, and the builds are fine...\n> > \n> > \n> > Reviewed-by: Kieran Bingham \n> > \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(-)\n> > >\n> > > diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> > > index 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> > > int orientation, int facing)\n> > > {\n> > > - camera_ = camera;\n> > > + camera_ = std::move(camera);\n> > > orientation_ = orientation;\n> > > facing_ = facing;\n> > > rawStreamAvailable_ = false;\n> > > diff --git a/src/apps/cam/camera_session.cpp b/src/apps/cam/camera_session.cpp\n> > > index 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> > > if (*endptr == '\\0' && index > 0) {\n> > > auto cameras = cm->cameras();\n> > > if (index <= cameras.size())\n> > > - camera_ = cameras[index - 1];\n> > > + camera_ = std::move(cameras[index - 1]);\n> > > }\n> > >\n> > > if (!camera_)\n> > > diff --git a/src/apps/cam/capture_script.cpp b/src/apps/cam/capture_script.cpp\n> > > index 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> > > const std::string &fileName)\n> > > - : camera_(camera), loop_(0), valid_(false)\n> > > + : camera_(std::move(camera)), loop_(0), valid_(false)\n> > > {\n> > > FILE *fh = fopen(fileName.c_str(), \"r\");\n> > > if (!fh) {\n> > > diff --git a/src/apps/lc-compliance/helpers/capture.cpp b/src/apps/lc-compliance/helpers/capture.cpp\n> > > index 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> > > - : loop_(nullptr), camera_(camera),\n> > > - allocator_(std::make_unique(camera))\n> > > + : loop_(nullptr), camera_(std::move(camera)),\n> > > + 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> > > - : Capture(camera)\n> > > + : 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> > > - : Capture(camera)\n> > > + : Capture(std::move(camera))\n> > > {\n> > > }\n> > >\n> > > diff --git a/src/apps/lc-compliance/main.cpp b/src/apps/lc-compliance/main.cpp\n> > > index 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> > > - std::shared_ptr camera;\n> > > -\n> > > int ret = cm->start();\n> > > if (ret) {\n> > > std::cout << \"Failed to start camera manager: \"\n> > > @@ -66,7 +64,7 @@ static int initCamera(CameraManager *cm, OptionsParser::Options options)\n> > > }\n> > >\n> > > const std::string &cameraId = options[OptCamera];\n> > > - camera = cm->get(cameraId);\n> > > + std::shared_ptr camera = cm->get(cameraId);\n> > > if (!camera) {\n> > > std::cout << \"Camera \" << cameraId << \" not found, available cameras:\" << std::endl;\n> > > listCameras(cm);\n> > > diff --git a/src/apps/qcam/main_window.cpp b/src/apps/qcam/main_window.cpp\n> > > index 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> > > if (camera_ && newCameraId == camera_->id())\n> > > return;\n> > >\n> > > - const std::shared_ptr &cam = cm_->get(newCameraId);\n> > > + std::shared_ptr cam = cm_->get(newCameraId);\n> > >\n> > > if (cam->acquire()) {\n> > > qInfo() << \"Failed to acquire camera\" << cam->id().c_str();\n> > > @@ -287,7 +287,7 @@ void MainWindow::switchCamera()\n> > > if (camera_)\n> > > camera_->release();\n> > >\n> > > - camera_ = cam;\n> > > + camera_ = std::move(cam);\n> > >\n> > > startStopAction_->setChecked(true);\n> > >\n> > > diff --git a/src/gstreamer/gstlibcameraprovider.cpp b/src/gstreamer/gstlibcameraprovider.cpp\n> > > index 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> > > GstLibcameraProvider *self = GST_LIBCAMERA_PROVIDER(provider);\n> > > - std::shared_ptr cm;\n> > > GList *devices = nullptr;\n> > > gint ret;\n> > >\n> > > @@ -200,7 +199,7 @@ gst_libcamera_provider_probe(GstDeviceProvider *provider)\n> > > * gains monitoring support. Meanwhile we need to cycle start()/stop()\n> > > * to ensure every probe() calls return the latest list.\n> > > */\n> > > - cm = gst_libcamera_get_camera_manager(ret);\n> > > + std::shared_ptr cm = gst_libcamera_get_camera_manager(ret);\n> > > if (ret) {\n> > > GST_ERROR_OBJECT(self, \"Failed to retrieve device list: %s\",\n> > > g_strerror(-ret));\n> > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > > index 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> > > - std::shared_ptr cm;\n> > > std::shared_ptr cam;\n> > > gint ret;\n> > >\n> > > GST_DEBUG_OBJECT(self, \"Opening camera device ...\");\n> > >\n> > > - cm = gst_libcamera_get_camera_manager(ret);\n> > > + std::shared_ptr cm = gst_libcamera_get_camera_manager(ret);\n> > > if (ret) {\n> > > GST_ELEMENT_ERROR(self, LIBRARY, INIT,\n> > > (\"Failed listing cameras.\"),\n> > > @@ -392,7 +391,7 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n> > > (\"libcamera::CameraMananger::cameras() is empty\"));\n> > > return false;\n> > > }\n> > > - cam = cameras[0];\n> > > + cam = std::move(cameras[0]);\n> > > }\n> > >\n> > > GST_INFO_OBJECT(self, \"Using camera '%s'\", cam->id().c_str());\n> > > @@ -408,8 +407,8 @@ gst_libcamera_src_open(GstLibcameraSrc *self)\n> > > cam->requestCompleted.connect(self->state, &GstLibcameraSrcState::requestCompleted);\n> > >\n> > > /* No need to lock here, we didn't start our threads yet. */\n> > > - self->state->cm_ = cm;\n> > > - self->state->cam_ = cam;\n> > > + self->state->cm_ = std::move(cm);\n> > > + self->state->cam_ = std::move(cam);\n> > >\n> > > return true;\n> > > }\n> > > diff --git a/src/libcamera/base/bound_method.cpp b/src/libcamera/base/bound_method.cpp\n> > > index 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> > > case ConnectionTypeQueued: {\n> > > std::unique_ptr msg =\n> > > - std::make_unique(this, pack, nullptr, deleteMethod);\n> > > + std::make_unique(this, std::move(pack), nullptr, deleteMethod);\n> > > object_->postMessage(std::move(msg));\n> > > return false;\n> > > }\n> > > @@ -98,7 +98,7 @@ bool BoundMethodBase::activatePack(std::shared_ptr pack,\n> > > Semaphore semaphore;\n> > >\n> > > std::unique_ptr msg =\n> > > - std::make_unique(this, pack, &semaphore, deleteMethod);\n> > > + std::make_unique(this, std::move(pack), &semaphore, deleteMethod);\n> > > object_->postMessage(std::move(msg));\n> > >\n> > > semaphore.acquire();\n> > > diff --git a/src/libcamera/base/message.cpp b/src/libcamera/base/message.cpp\n> > > index 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> > > std::shared_ptr pack,\n> > > Semaphore *semaphore, bool deleteMethod)\n> > > - : Message(Message::InvokeMessage), method_(method), pack_(pack),\n> > > + : Message(Message::InvokeMessage), method_(method), pack_(std::move(pack)),\n> > > semaphore_(semaphore), deleteMethod_(deleteMethod)\n> > > {\n> > > }\n> > > diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp\n> > > index 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> > > MutexLocker locker(mutex_);\n> > >\n> > > auto iter = std::find_if(cameras_.begin(), cameras_.end(),\n> > > - [camera](std::shared_ptr &c) {\n> > > + [&camera](const std::shared_ptr &c) {\n> > > return c.get() == camera.get();\n> > > });\n> > > if (iter == cameras_.end())\n> > > @@ -374,7 +374,7 @@ std::shared_ptr CameraManager::get(const std::string &id)\n> > >\n> > > MutexLocker locker(d->mutex_);\n> > >\n> > > - for (std::shared_ptr camera : d->cameras_) {\n> > > + for (const std::shared_ptr &camera : d->cameras_) {\n> > > if (camera->id() == id)\n> > > return camera;\n> > > }\n> > > diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> > > index 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> > > - for (std::shared_ptr media : mediaDevices_)\n> > > + for (std::shared_ptr &media : mediaDevices_)\n> > > media->release();\n> > > }\n> > >\n> > > @@ -138,9 +138,7 @@ MediaDevice *PipelineHandler::acquireMediaDevice(DeviceEnumerator *enumerator,\n> > > if (!media->acquire())\n> > > return nullptr;\n> > >\n> > > - mediaDevices_.push_back(media);\n> > > -\n> > > - return media.get();\n> > > + return mediaDevices_.emplace_back(std::move(media)).get();\n> > > }\n> > >\n> > > /**\n> > > @@ -699,7 +697,7 @@ void PipelineHandler::disconnect()\n> > > continue;\n> > >\n> > > camera->disconnect();\n> > > - manager_->_d()->removeCamera(camera);\n> > > + manager_->_d()->removeCamera(std::move(camera));\n> > > }\n> > > }\n> > >\n> > > diff --git a/src/py/libcamera/py_main.cpp b/src/py/libcamera/py_main.cpp\n> > > index 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> > > }\n> > >\n> > > explicit PyCameraSmartPtr(std::shared_ptr p)\n> > > - : ptr_(p)\n> > > + : ptr_(std::move(p))\n> > > {\n> > > }\n> > >\n> > > diff --git a/src/v4l2/v4l2_camera.cpp b/src/v4l2/v4l2_camera.cpp\n> > > index 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> > > - : camera_(camera), isRunning_(false), bufferAllocator_(nullptr),\n> > > + : camera_(std::move(camera)), isRunning_(false), bufferAllocator_(nullptr),\n> > > efd_(-1), bufferAvailableCount_(0)\n> > > {\n> > > camera_->requestCompleted.connect(this, &V4L2Camera::requestComplete);\n> > > diff --git a/src/v4l2/v4l2_compat_manager.cpp b/src/v4l2/v4l2_compat_manager.cpp\n> > > index 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> > > */\n> > > auto cameras = cm_->cameras();\n> > > for (auto [index, camera] : utils::enumerate(cameras)) {\n> > > - V4L2CameraProxy *proxy = new V4L2CameraProxy(index, camera);\n> > > + V4L2CameraProxy *proxy = new V4L2CameraProxy(index, std::move(camera));\n> > > proxies_.emplace_back(proxy);\n> > > }\n> > >\n> > > --\n> > > 2.45.2\n> > >\n> > >\n> >","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 D70F5C323E\n\tfor ;\n\tWed, 31 Jul 2024 11:45:03 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7834F63374;\n\tWed, 31 Jul 2024 13:45:02 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 2AD846198E\n\tfor ;\n\tWed, 31 Jul 2024 13:45:01 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 45354842;\n\tWed, 31 Jul 2024 13:44:13 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"S8GMOjrn\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1722426253;\n\tbh=bP1LemRxXPrXQhB/EvnxeVIsEb7zLqp5d9Z2bBCCHrk=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=S8GMOjrn6FPFkj1PF7tv+KFVSqVrHQID0TO+deykAn4q2FvHQiqxqCf9PuoFO7Gtb\n\t8GHWBjXN4OcC+u3lAPl0lIZnGvVrbVDOchcL3pCgjelgffj96/fXY2qYdr9icMULnN\n\tY1o8viAXzJ8ld1AYiFAKRDSXrtkSs8v72/m0aSjM=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<5j8WgrtwWcO4K5dd8ICTZA0HsZsnNZzi37Y3r64JpSGKeWtSS9B-PSS6OCLaTH7KEe2D6I-8VRkxoYWCLjqwx5eYgG4RLRcaUOhPwoJttyo=@protonmail.com>","References":"<20240614212256.494291-1-pobrn@protonmail.com>\n\t<171866595818.804094.376698690946371877@ping.linuxembedded.co.uk>\n\t<5j8WgrtwWcO4K5dd8ICTZA0HsZsnNZzi37Y3r64JpSGKeWtSS9B-PSS6OCLaTH7KEe2D6I-8VRkxoYWCLjqwx5eYgG4RLRcaUOhPwoJttyo=@protonmail.com>","Subject":"Re: [PATCH v1] treewide: move construct/assign `shared_ptr`s where\n\tpossible","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org","To":"=?utf-8?q?Barnab=C3=A1s_P=C5=91cze?= ","Date":"Wed, 31 Jul 2024 12:44:58 +0100","Message-ID":"<172242629823.392292.1966838620806583894@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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\" "}}]