[PATCH v1] treewide: move construct/assign `shared_ptr`s where possible
Barnabás Pőcze
pobrn at protonmail.com
Fri Jun 14 23:22:59 CEST 2024
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 <pobrn at protonmail.com>
---
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> 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> 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> camera)
- : loop_(nullptr), camera_(camera),
- allocator_(std::make_unique<FrameBufferAllocator>(camera))
+ : loop_(nullptr), camera_(std::move(camera)),
+ allocator_(std::make_unique<FrameBufferAllocator>(camera_))
{
}
@@ -72,7 +72,7 @@ void Capture::stop()
/* CaptureBalanced */
CaptureBalanced::CaptureBalanced(std::shared_ptr<Camera> camera)
- : Capture(camera)
+ : Capture(std::move(camera))
{
}
@@ -144,7 +144,7 @@ void CaptureBalanced::requestComplete(Request *request)
/* CaptureUnbalanced */
CaptureUnbalanced::CaptureUnbalanced(std::shared_ptr<Camera> 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> 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> 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<Camera> &cam = cm_->get(newCameraId);
+ std::shared_ptr<Camera> 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<CameraManager> 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<CameraManager> 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<CameraManager> cm;
std::shared_ptr<Camera> cam;
gint ret;
GST_DEBUG_OBJECT(self, "Opening camera device ...");
- cm = gst_libcamera_get_camera_manager(ret);
+ std::shared_ptr<CameraManager> 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<BoundMethodPackBase> pack,
case ConnectionTypeQueued: {
std::unique_ptr<Message> msg =
- std::make_unique<InvokeMessage>(this, pack, nullptr, deleteMethod);
+ std::make_unique<InvokeMessage>(this, std::move(pack), nullptr, deleteMethod);
object_->postMessage(std::move(msg));
return false;
}
@@ -98,7 +98,7 @@ bool BoundMethodBase::activatePack(std::shared_ptr<BoundMethodPackBase> pack,
Semaphore semaphore;
std::unique_ptr<Message> msg =
- std::make_unique<InvokeMessage>(this, pack, &semaphore, deleteMethod);
+ std::make_unique<InvokeMessage>(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<BoundMethodPackBase> 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> camera)
MutexLocker locker(mutex_);
auto iter = std::find_if(cameras_.begin(), cameras_.end(),
- [camera](std::shared_ptr<Camera> &c) {
+ [&camera](const std::shared_ptr<Camera> &c) {
return c.get() == camera.get();
});
if (iter == cameras_.end())
@@ -374,7 +374,7 @@ std::shared_ptr<Camera> CameraManager::get(const std::string &id)
MutexLocker locker(d->mutex_);
- for (std::shared_ptr<Camera> camera : d->cameras_) {
+ for (const std::shared_ptr<Camera> &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<MediaDevice> media : mediaDevices_)
+ for (std::shared_ptr<MediaDevice> &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<T> 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_(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);
}
--
2.45.2
More information about the libcamera-devel
mailing list