| Message ID | 20240614212256.494291-1-pobrn@protonmail.com |
|---|---|
| State | New |
| Headers | show |
| Series |
|
| Related | show |
Oops, of course the title should have a capital "M". Does `utils/checkstyle.py` not check that or am I just holding it wrong? Regards, Barnabás Pőcze 2024. június 14., péntek 23:22 keltezéssel, Barnabás Pőcze <pobrn@protonmail.com> írta: > 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@protonmail.com > > --- > [...]
Quoting Barnabás Pőcze (2024-06-14 22:22:59) > 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. CI-'Green': https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1203640 Except for the Soraka hardware test - but I don't yet know if that's an expected failure or not ... > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> Otherwise this sounds reasonable to me, and the builds are fine... Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.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 > >
2024. június 18., kedd 1:12 keltezéssel, Kieran Bingham <kieran.bingham@ideasonboard.com> írta: > Quoting Barnabás Pőcze (2024-06-14 22:22:59) > > 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. > > CI-'Green': https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1203640 > > Except for the Soraka hardware test - but I don't yet know if that's > an expected failure or not ... > Is there anything I can help with here? Regards, Barnabás Pőcze > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > Otherwise this sounds reasonable to me, and the builds are fine... > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.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 > > > > >
Quoting Barnabás Pőcze (2024-07-29 19:33:52) > 2024. június 18., kedd 1:12 keltezéssel, Kieran Bingham <kieran.bingham@ideasonboard.com> írta: > > > Quoting Barnabás Pőcze (2024-06-14 22:22:59) > > > 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. > > > > CI-'Green': https://gitlab.freedesktop.org/camera/libcamera/-/pipelines/1203640 > > > > Except for the Soraka hardware test - but I don't yet know if that's > > an expected failure or not ... > > > > Is there anything I can help with here? > I was about to re-run ... but I think there's another failure (unrelated) on the Soraka hardware so I can't yet from what I can tell. Will try again once the ChromeOS tests are back online. -- Kieran > > Regards, > Barnabás Pőcze > > > > > > > > Signed-off-by: Barnabás Pőcze <pobrn@protonmail.com> > > > > Otherwise this sounds reasonable to me, and the builds are fine... > > > > > > Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.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 > > > > > > > >
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); }
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@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(-)