[v1] treewide: move construct/assign `shared_ptr`s where possible
diff mbox series

Message ID 20240614212256.494291-1-pobrn@protonmail.com
State New
Headers show
Series
  • [v1] treewide: move construct/assign `shared_ptr`s where possible
Related show

Commit Message

Barnabás Pőcze June 14, 2024, 9:22 p.m. UTC
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(-)

Comments

Barnabás Pőcze June 14, 2024, 9:29 p.m. UTC | #1
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
> 
> ---
> [...]
Kieran Bingham June 17, 2024, 11:12 p.m. UTC | #2
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
> 
>
Barnabás Pőcze July 29, 2024, 6:33 p.m. UTC | #3
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
> >
> >
>
Kieran Bingham July 31, 2024, 11:44 a.m. UTC | #4
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
> > >
> > >
> >

Patch
diff mbox series

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);
 	}