[libcamera-devel,2/2] libcamera: device_enumerator: Convey device ownership through unique_ptr
diff mbox series

Message ID 20200321180951.32685-2-laurent.pinchart@ideasonboard.com
State Accepted
Commit 9ab024f7c27d9b6b3ab433502eab02d4a29a3da4
Headers show
Series
  • [libcamera-devel,1/2] libcamera: device_enumerator_udev: Update pending list in addUdevDevice
Related show

Commit Message

Laurent Pinchart March 21, 2020, 6:09 p.m. UTC 
Replace usage of shared_ptr with unique_ptr to convey media device
ownership internally in the enumerators when creating the media device.
Once a media device has all its dependencies met, it is converted to a
shared_ptr to keep the external API unchanged.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 src/libcamera/device_enumerator.cpp             | 10 +++++-----
 src/libcamera/device_enumerator_sysfs.cpp       |  8 ++++----
 src/libcamera/device_enumerator_udev.cpp        |  8 ++++----
 src/libcamera/include/device_enumerator.h       |  4 ++--
 src/libcamera/include/device_enumerator_sysfs.h |  2 +-
 src/libcamera/include/device_enumerator_udev.h  |  8 ++++----
 6 files changed, 20 insertions(+), 20 deletions(-)

Comments

Niklas Söderlund March 23, 2020, 12:02 p.m. UTC | #1 
Hi Laurent,

Thanks for your work.

On 2020-03-21 20:09:51 +0200, Laurent Pinchart wrote:
> Replace usage of shared_ptr with unique_ptr to convey media device
> ownership internally in the enumerators when creating the media device.
> Once a media device has all its dependencies met, it is converted to a
> shared_ptr to keep the external API unchanged.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>

> ---
>  src/libcamera/device_enumerator.cpp             | 10 +++++-----
>  src/libcamera/device_enumerator_sysfs.cpp       |  8 ++++----
>  src/libcamera/device_enumerator_udev.cpp        |  8 ++++----
>  src/libcamera/include/device_enumerator.h       |  4 ++--
>  src/libcamera/include/device_enumerator_sysfs.h |  2 +-
>  src/libcamera/include/device_enumerator_udev.h  |  8 ++++----
>  6 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
> index 89ed67ebb115..dd17e3e32e6c 100644
> --- a/src/libcamera/device_enumerator.cpp
> +++ b/src/libcamera/device_enumerator.cpp
> @@ -208,9 +208,9 @@ DeviceEnumerator::~DeviceEnumerator()
>   *
>   * \return Created media device instance on success, or nullptr otherwise
>   */
> -std::shared_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &deviceNode)
> +std::unique_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &deviceNode)
>  {
> -	std::shared_ptr<MediaDevice> media = std::make_shared<MediaDevice>(deviceNode);
> +	std::unique_ptr<MediaDevice> media = std::make_unique<MediaDevice>(deviceNode);
>  
>  	int ret = media->populate();
>  	if (ret < 0) {
> @@ -236,12 +236,12 @@ std::shared_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &d
>   * This method shall be called after all members of the entities of the
>   * media graph have been confirmed to be initialized.
>   */
> -void DeviceEnumerator::addDevice(const std::shared_ptr<MediaDevice> &media)
> +void DeviceEnumerator::addDevice(std::unique_ptr<MediaDevice> &&media)
>  {
>  	LOG(DeviceEnumerator, Debug)
>  		<< "Added device " << media->deviceNode() << ": " << media->driver();
>  
> -	devices_.push_back(media);
> +	devices_.push_back(std::move(media));
>  }
>  
>  /**
> @@ -290,7 +290,7 @@ void DeviceEnumerator::removeDevice(const std::string &deviceNode)
>   */
>  std::shared_ptr<MediaDevice> DeviceEnumerator::search(const DeviceMatch &dm)
>  {
> -	for (std::shared_ptr<MediaDevice> media : devices_) {
> +	for (std::shared_ptr<MediaDevice> &media : devices_) {
>  		if (media->busy())
>  			continue;
>  
> diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp
> index 197ca235879b..3446db59e9d4 100644
> --- a/src/libcamera/device_enumerator_sysfs.cpp
> +++ b/src/libcamera/device_enumerator_sysfs.cpp
> @@ -72,11 +72,11 @@ int DeviceEnumeratorSysfs::enumerate()
>  			continue;
>  		}
>  
> -		std::shared_ptr<MediaDevice> media = createDevice(devnode);
> +		std::unique_ptr<MediaDevice> media = createDevice(devnode);
>  		if (!media)
>  			continue;
>  
> -		if (populateMediaDevice(media) < 0) {
> +		if (populateMediaDevice(media.get()) < 0) {
>  			LOG(DeviceEnumerator, Warning)
>  				<< "Failed to populate media device "
>  				<< media->deviceNode()
> @@ -84,7 +84,7 @@ int DeviceEnumeratorSysfs::enumerate()
>  			continue;
>  		}
>  
> -		addDevice(media);
> +		addDevice(std::move(media));
>  	}
>  
>  	closedir(dir);
> @@ -92,7 +92,7 @@ int DeviceEnumeratorSysfs::enumerate()
>  	return 0;
>  }
>  
> -int DeviceEnumeratorSysfs::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)
> +int DeviceEnumeratorSysfs::populateMediaDevice(MediaDevice *media)
>  {
>  	/* Associate entities to device node paths. */
>  	for (MediaEntity *entity : media->entities()) {
> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> index ea3f6b7c9ae0..9cbc7e47d2d9 100644
> --- a/src/libcamera/device_enumerator_udev.cpp
> +++ b/src/libcamera/device_enumerator_udev.cpp
> @@ -76,7 +76,7 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
>  		return -ENODEV;
>  
>  	if (!strcmp(subsystem, "media")) {
> -		std::shared_ptr<MediaDevice> media =
> +		std::unique_ptr<MediaDevice> media =
>  			createDevice(udev_device_get_devnode(dev));
>  		if (!media)
>  			return -ENODEV;
> @@ -96,7 +96,7 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
>  				<< "Defer media device " << media->deviceNode()
>  				<< " due to " << ret << " missing dependencies";
>  
> -			pending_.emplace_back(media, deps);
> +			pending_.emplace_back(std::move(media), std::move(deps));
>  			MediaDeviceDeps *mediaDeps = &pending_.back();
>  			for (const auto &dep : mediaDeps->deps_)
>  				devMap_[dep.first] = mediaDeps;
> @@ -104,7 +104,7 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
>  			return 0;
>  		}
>  
> -		addDevice(media);
> +		addDevice(std::move(media));
>  		return 0;
>  	}
>  
> @@ -319,7 +319,7 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)
>  		LOG(DeviceEnumerator, Debug)
>  			<< "All dependencies for media device "
>  			<< deps->media_->deviceNode() << " found";
> -		addDevice(deps->media_);
> +		addDevice(std::move(deps->media_));
>  		pending_.remove(*deps);
>  	}
>  
> diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h
> index 770f42772270..433e357aebae 100644
> --- a/src/libcamera/include/device_enumerator.h
> +++ b/src/libcamera/include/device_enumerator.h
> @@ -44,8 +44,8 @@ public:
>  	std::shared_ptr<MediaDevice> search(const DeviceMatch &dm);
>  
>  protected:
> -	std::shared_ptr<MediaDevice> createDevice(const std::string &deviceNode);
> -	void addDevice(const std::shared_ptr<MediaDevice> &media);
> +	std::unique_ptr<MediaDevice> createDevice(const std::string &deviceNode);
> +	void addDevice(std::unique_ptr<MediaDevice> &&media);
>  	void removeDevice(const std::string &deviceNode);
>  
>  private:
> diff --git a/src/libcamera/include/device_enumerator_sysfs.h b/src/libcamera/include/device_enumerator_sysfs.h
> index 9063f6a75e11..5a5c9b0f5a31 100644
> --- a/src/libcamera/include/device_enumerator_sysfs.h
> +++ b/src/libcamera/include/device_enumerator_sysfs.h
> @@ -23,7 +23,7 @@ public:
>  	int enumerate();
>  
>  private:
> -	int populateMediaDevice(const std::shared_ptr<MediaDevice> &media);
> +	int populateMediaDevice(MediaDevice *media);
>  	std::string lookupDeviceNode(int major, int minor);
>  };
>  
> diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h
> index efaefe5dc4a3..fdce4520f33a 100644
> --- a/src/libcamera/include/device_enumerator_udev.h
> +++ b/src/libcamera/include/device_enumerator_udev.h
> @@ -43,9 +43,9 @@ private:
>  	using DependencyMap = std::map<dev_t, std::list<MediaEntity *>>;
>  
>  	struct MediaDeviceDeps {
> -		MediaDeviceDeps(const std::shared_ptr<MediaDevice> &media,
> -				const DependencyMap &deps)
> -			: media_(media), deps_(deps)
> +		MediaDeviceDeps(std::unique_ptr<MediaDevice> &&media,
> +				DependencyMap &&deps)
> +			: media_(std::move(media)), deps_(std::move(deps))
>  		{
>  		}
>  
> @@ -54,7 +54,7 @@ private:
>  			return media_ == other.media_;
>  		}
>  
> -		std::shared_ptr<MediaDevice> media_;
> +		std::unique_ptr<MediaDevice> media_;
>  		DependencyMap deps_;
>  	};
>  
> -- 
> Regards,
> 
> Laurent Pinchart
> 
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel@lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
Kieran Bingham March 23, 2020, 2:08 p.m. UTC | #2 
Hi Laurent,

On 21/03/2020 18:09, Laurent Pinchart wrote:
> Replace usage of shared_ptr with unique_ptr to convey media device
> ownership internally in the enumerators when creating the media device.
> Once a media device has all its dependencies met, it is converted to a
> shared_ptr to keep the external API unchanged.


Looks ok, my only worry is that it could become easy to 'forget' to move
when needed? Or will unique_ptr fail/be loud when we need to move and
don't ?

Anyway,

Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>

> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  src/libcamera/device_enumerator.cpp             | 10 +++++-----
>  src/libcamera/device_enumerator_sysfs.cpp       |  8 ++++----
>  src/libcamera/device_enumerator_udev.cpp        |  8 ++++----
>  src/libcamera/include/device_enumerator.h       |  4 ++--
>  src/libcamera/include/device_enumerator_sysfs.h |  2 +-
>  src/libcamera/include/device_enumerator_udev.h  |  8 ++++----
>  6 files changed, 20 insertions(+), 20 deletions(-)
> 
> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
> index 89ed67ebb115..dd17e3e32e6c 100644
> --- a/src/libcamera/device_enumerator.cpp
> +++ b/src/libcamera/device_enumerator.cpp
> @@ -208,9 +208,9 @@ DeviceEnumerator::~DeviceEnumerator()
>   *
>   * \return Created media device instance on success, or nullptr otherwise
>   */
> -std::shared_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &deviceNode)
> +std::unique_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &deviceNode)
>  {
> -	std::shared_ptr<MediaDevice> media = std::make_shared<MediaDevice>(deviceNode);
> +	std::unique_ptr<MediaDevice> media = std::make_unique<MediaDevice>(deviceNode);
>  
>  	int ret = media->populate();
>  	if (ret < 0) {
> @@ -236,12 +236,12 @@ std::shared_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &d
>   * This method shall be called after all members of the entities of the
>   * media graph have been confirmed to be initialized.
>   */
> -void DeviceEnumerator::addDevice(const std::shared_ptr<MediaDevice> &media)
> +void DeviceEnumerator::addDevice(std::unique_ptr<MediaDevice> &&media)
>  {
>  	LOG(DeviceEnumerator, Debug)
>  		<< "Added device " << media->deviceNode() << ": " << media->driver();
>  
> -	devices_.push_back(media);
> +	devices_.push_back(std::move(media));
>  }
>  
>  /**
> @@ -290,7 +290,7 @@ void DeviceEnumerator::removeDevice(const std::string &deviceNode)
>   */
>  std::shared_ptr<MediaDevice> DeviceEnumerator::search(const DeviceMatch &dm)
>  {
> -	for (std::shared_ptr<MediaDevice> media : devices_) {
> +	for (std::shared_ptr<MediaDevice> &media : devices_) {
>  		if (media->busy())
>  			continue;
>  
> diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp
> index 197ca235879b..3446db59e9d4 100644
> --- a/src/libcamera/device_enumerator_sysfs.cpp
> +++ b/src/libcamera/device_enumerator_sysfs.cpp
> @@ -72,11 +72,11 @@ int DeviceEnumeratorSysfs::enumerate()
>  			continue;
>  		}
>  
> -		std::shared_ptr<MediaDevice> media = createDevice(devnode);
> +		std::unique_ptr<MediaDevice> media = createDevice(devnode);
>  		if (!media)
>  			continue;
>  
> -		if (populateMediaDevice(media) < 0) {
> +		if (populateMediaDevice(media.get()) < 0) {
>  			LOG(DeviceEnumerator, Warning)
>  				<< "Failed to populate media device "
>  				<< media->deviceNode()
> @@ -84,7 +84,7 @@ int DeviceEnumeratorSysfs::enumerate()
>  			continue;
>  		}
>  
> -		addDevice(media);
> +		addDevice(std::move(media));
>  	}
>  
>  	closedir(dir);
> @@ -92,7 +92,7 @@ int DeviceEnumeratorSysfs::enumerate()
>  	return 0;
>  }
>  
> -int DeviceEnumeratorSysfs::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)
> +int DeviceEnumeratorSysfs::populateMediaDevice(MediaDevice *media)
>  {
>  	/* Associate entities to device node paths. */
>  	for (MediaEntity *entity : media->entities()) {
> diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> index ea3f6b7c9ae0..9cbc7e47d2d9 100644
> --- a/src/libcamera/device_enumerator_udev.cpp
> +++ b/src/libcamera/device_enumerator_udev.cpp
> @@ -76,7 +76,7 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
>  		return -ENODEV;
>  
>  	if (!strcmp(subsystem, "media")) {
> -		std::shared_ptr<MediaDevice> media =
> +		std::unique_ptr<MediaDevice> media =
>  			createDevice(udev_device_get_devnode(dev));
>  		if (!media)
>  			return -ENODEV;
> @@ -96,7 +96,7 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
>  				<< "Defer media device " << media->deviceNode()
>  				<< " due to " << ret << " missing dependencies";
>  
> -			pending_.emplace_back(media, deps);
> +			pending_.emplace_back(std::move(media), std::move(deps));
>  			MediaDeviceDeps *mediaDeps = &pending_.back();
>  			for (const auto &dep : mediaDeps->deps_)
>  				devMap_[dep.first] = mediaDeps;
> @@ -104,7 +104,7 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
>  			return 0;
>  		}
>  
> -		addDevice(media);
> +		addDevice(std::move(media));
>  		return 0;
>  	}
>  
> @@ -319,7 +319,7 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)
>  		LOG(DeviceEnumerator, Debug)
>  			<< "All dependencies for media device "
>  			<< deps->media_->deviceNode() << " found";
> -		addDevice(deps->media_);
> +		addDevice(std::move(deps->media_));
>  		pending_.remove(*deps);
>  	}
>  
> diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h
> index 770f42772270..433e357aebae 100644
> --- a/src/libcamera/include/device_enumerator.h
> +++ b/src/libcamera/include/device_enumerator.h
> @@ -44,8 +44,8 @@ public:
>  	std::shared_ptr<MediaDevice> search(const DeviceMatch &dm);
>  
>  protected:
> -	std::shared_ptr<MediaDevice> createDevice(const std::string &deviceNode);
> -	void addDevice(const std::shared_ptr<MediaDevice> &media);
> +	std::unique_ptr<MediaDevice> createDevice(const std::string &deviceNode);
> +	void addDevice(std::unique_ptr<MediaDevice> &&media);
>  	void removeDevice(const std::string &deviceNode);
>  
>  private:
> diff --git a/src/libcamera/include/device_enumerator_sysfs.h b/src/libcamera/include/device_enumerator_sysfs.h
> index 9063f6a75e11..5a5c9b0f5a31 100644
> --- a/src/libcamera/include/device_enumerator_sysfs.h
> +++ b/src/libcamera/include/device_enumerator_sysfs.h
> @@ -23,7 +23,7 @@ public:
>  	int enumerate();
>  
>  private:
> -	int populateMediaDevice(const std::shared_ptr<MediaDevice> &media);
> +	int populateMediaDevice(MediaDevice *media);
>  	std::string lookupDeviceNode(int major, int minor);
>  };
>  
> diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h
> index efaefe5dc4a3..fdce4520f33a 100644
> --- a/src/libcamera/include/device_enumerator_udev.h
> +++ b/src/libcamera/include/device_enumerator_udev.h
> @@ -43,9 +43,9 @@ private:
>  	using DependencyMap = std::map<dev_t, std::list<MediaEntity *>>;
>  
>  	struct MediaDeviceDeps {
> -		MediaDeviceDeps(const std::shared_ptr<MediaDevice> &media,
> -				const DependencyMap &deps)
> -			: media_(media), deps_(deps)
> +		MediaDeviceDeps(std::unique_ptr<MediaDevice> &&media,
> +				DependencyMap &&deps)
> +			: media_(std::move(media)), deps_(std::move(deps))
>  		{
>  		}
>  
> @@ -54,7 +54,7 @@ private:
>  			return media_ == other.media_;
>  		}
>  
> -		std::shared_ptr<MediaDevice> media_;
> +		std::unique_ptr<MediaDevice> media_;
>  		DependencyMap deps_;
>  	};
>  
>
Laurent Pinchart March 23, 2020, 2:30 p.m. UTC | #3 
Hi Kieran,

On Mon, Mar 23, 2020 at 02:08:52PM +0000, Kieran Bingham wrote:
> On 21/03/2020 18:09, Laurent Pinchart wrote:
> > Replace usage of shared_ptr with unique_ptr to convey media device
> > ownership internally in the enumerators when creating the media device.
> > Once a media device has all its dependencies met, it is converted to a
> > shared_ptr to keep the external API unchanged.
> 
> Looks ok, my only worry is that it could become easy to 'forget' to move
> when needed? Or will unique_ptr fail/be loud when we need to move and
> don't ?

The compiler will be loud, brace yourself :-)

> Anyway,
> 
> Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com>
> 
> > 
> > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > ---
> >  src/libcamera/device_enumerator.cpp             | 10 +++++-----
> >  src/libcamera/device_enumerator_sysfs.cpp       |  8 ++++----
> >  src/libcamera/device_enumerator_udev.cpp        |  8 ++++----
> >  src/libcamera/include/device_enumerator.h       |  4 ++--
> >  src/libcamera/include/device_enumerator_sysfs.h |  2 +-
> >  src/libcamera/include/device_enumerator_udev.h  |  8 ++++----
> >  6 files changed, 20 insertions(+), 20 deletions(-)
> > 
> > diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
> > index 89ed67ebb115..dd17e3e32e6c 100644
> > --- a/src/libcamera/device_enumerator.cpp
> > +++ b/src/libcamera/device_enumerator.cpp
> > @@ -208,9 +208,9 @@ DeviceEnumerator::~DeviceEnumerator()
> >   *
> >   * \return Created media device instance on success, or nullptr otherwise
> >   */
> > -std::shared_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &deviceNode)
> > +std::unique_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &deviceNode)
> >  {
> > -	std::shared_ptr<MediaDevice> media = std::make_shared<MediaDevice>(deviceNode);
> > +	std::unique_ptr<MediaDevice> media = std::make_unique<MediaDevice>(deviceNode);
> >  
> >  	int ret = media->populate();
> >  	if (ret < 0) {
> > @@ -236,12 +236,12 @@ std::shared_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &d
> >   * This method shall be called after all members of the entities of the
> >   * media graph have been confirmed to be initialized.
> >   */
> > -void DeviceEnumerator::addDevice(const std::shared_ptr<MediaDevice> &media)
> > +void DeviceEnumerator::addDevice(std::unique_ptr<MediaDevice> &&media)
> >  {
> >  	LOG(DeviceEnumerator, Debug)
> >  		<< "Added device " << media->deviceNode() << ": " << media->driver();
> >  
> > -	devices_.push_back(media);
> > +	devices_.push_back(std::move(media));
> >  }
> >  
> >  /**
> > @@ -290,7 +290,7 @@ void DeviceEnumerator::removeDevice(const std::string &deviceNode)
> >   */
> >  std::shared_ptr<MediaDevice> DeviceEnumerator::search(const DeviceMatch &dm)
> >  {
> > -	for (std::shared_ptr<MediaDevice> media : devices_) {
> > +	for (std::shared_ptr<MediaDevice> &media : devices_) {
> >  		if (media->busy())
> >  			continue;
> >  
> > diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp
> > index 197ca235879b..3446db59e9d4 100644
> > --- a/src/libcamera/device_enumerator_sysfs.cpp
> > +++ b/src/libcamera/device_enumerator_sysfs.cpp
> > @@ -72,11 +72,11 @@ int DeviceEnumeratorSysfs::enumerate()
> >  			continue;
> >  		}
> >  
> > -		std::shared_ptr<MediaDevice> media = createDevice(devnode);
> > +		std::unique_ptr<MediaDevice> media = createDevice(devnode);
> >  		if (!media)
> >  			continue;
> >  
> > -		if (populateMediaDevice(media) < 0) {
> > +		if (populateMediaDevice(media.get()) < 0) {
> >  			LOG(DeviceEnumerator, Warning)
> >  				<< "Failed to populate media device "
> >  				<< media->deviceNode()
> > @@ -84,7 +84,7 @@ int DeviceEnumeratorSysfs::enumerate()
> >  			continue;
> >  		}
> >  
> > -		addDevice(media);
> > +		addDevice(std::move(media));
> >  	}
> >  
> >  	closedir(dir);
> > @@ -92,7 +92,7 @@ int DeviceEnumeratorSysfs::enumerate()
> >  	return 0;
> >  }
> >  
> > -int DeviceEnumeratorSysfs::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)
> > +int DeviceEnumeratorSysfs::populateMediaDevice(MediaDevice *media)
> >  {
> >  	/* Associate entities to device node paths. */
> >  	for (MediaEntity *entity : media->entities()) {
> > diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
> > index ea3f6b7c9ae0..9cbc7e47d2d9 100644
> > --- a/src/libcamera/device_enumerator_udev.cpp
> > +++ b/src/libcamera/device_enumerator_udev.cpp
> > @@ -76,7 +76,7 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
> >  		return -ENODEV;
> >  
> >  	if (!strcmp(subsystem, "media")) {
> > -		std::shared_ptr<MediaDevice> media =
> > +		std::unique_ptr<MediaDevice> media =
> >  			createDevice(udev_device_get_devnode(dev));
> >  		if (!media)
> >  			return -ENODEV;
> > @@ -96,7 +96,7 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
> >  				<< "Defer media device " << media->deviceNode()
> >  				<< " due to " << ret << " missing dependencies";
> >  
> > -			pending_.emplace_back(media, deps);
> > +			pending_.emplace_back(std::move(media), std::move(deps));
> >  			MediaDeviceDeps *mediaDeps = &pending_.back();
> >  			for (const auto &dep : mediaDeps->deps_)
> >  				devMap_[dep.first] = mediaDeps;
> > @@ -104,7 +104,7 @@ int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
> >  			return 0;
> >  		}
> >  
> > -		addDevice(media);
> > +		addDevice(std::move(media));
> >  		return 0;
> >  	}
> >  
> > @@ -319,7 +319,7 @@ int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)
> >  		LOG(DeviceEnumerator, Debug)
> >  			<< "All dependencies for media device "
> >  			<< deps->media_->deviceNode() << " found";
> > -		addDevice(deps->media_);
> > +		addDevice(std::move(deps->media_));
> >  		pending_.remove(*deps);
> >  	}
> >  
> > diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h
> > index 770f42772270..433e357aebae 100644
> > --- a/src/libcamera/include/device_enumerator.h
> > +++ b/src/libcamera/include/device_enumerator.h
> > @@ -44,8 +44,8 @@ public:
> >  	std::shared_ptr<MediaDevice> search(const DeviceMatch &dm);
> >  
> >  protected:
> > -	std::shared_ptr<MediaDevice> createDevice(const std::string &deviceNode);
> > -	void addDevice(const std::shared_ptr<MediaDevice> &media);
> > +	std::unique_ptr<MediaDevice> createDevice(const std::string &deviceNode);
> > +	void addDevice(std::unique_ptr<MediaDevice> &&media);
> >  	void removeDevice(const std::string &deviceNode);
> >  
> >  private:
> > diff --git a/src/libcamera/include/device_enumerator_sysfs.h b/src/libcamera/include/device_enumerator_sysfs.h
> > index 9063f6a75e11..5a5c9b0f5a31 100644
> > --- a/src/libcamera/include/device_enumerator_sysfs.h
> > +++ b/src/libcamera/include/device_enumerator_sysfs.h
> > @@ -23,7 +23,7 @@ public:
> >  	int enumerate();
> >  
> >  private:
> > -	int populateMediaDevice(const std::shared_ptr<MediaDevice> &media);
> > +	int populateMediaDevice(MediaDevice *media);
> >  	std::string lookupDeviceNode(int major, int minor);
> >  };
> >  
> > diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h
> > index efaefe5dc4a3..fdce4520f33a 100644
> > --- a/src/libcamera/include/device_enumerator_udev.h
> > +++ b/src/libcamera/include/device_enumerator_udev.h
> > @@ -43,9 +43,9 @@ private:
> >  	using DependencyMap = std::map<dev_t, std::list<MediaEntity *>>;
> >  
> >  	struct MediaDeviceDeps {
> > -		MediaDeviceDeps(const std::shared_ptr<MediaDevice> &media,
> > -				const DependencyMap &deps)
> > -			: media_(media), deps_(deps)
> > +		MediaDeviceDeps(std::unique_ptr<MediaDevice> &&media,
> > +				DependencyMap &&deps)
> > +			: media_(std::move(media)), deps_(std::move(deps))
> >  		{
> >  		}
> >  
> > @@ -54,7 +54,7 @@ private:
> >  			return media_ == other.media_;
> >  		}
> >  
> > -		std::shared_ptr<MediaDevice> media_;
> > +		std::unique_ptr<MediaDevice> media_;
> >  		DependencyMap deps_;
> >  	};
> >

Patch
diff mbox series 

diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
index 89ed67ebb115..dd17e3e32e6c 100644
--- a/src/libcamera/device_enumerator.cpp
+++ b/src/libcamera/device_enumerator.cpp
@@ -208,9 +208,9 @@  DeviceEnumerator::~DeviceEnumerator()
  *
  * \return Created media device instance on success, or nullptr otherwise
  */
-std::shared_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &deviceNode)
+std::unique_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &deviceNode)
 {
-	std::shared_ptr<MediaDevice> media = std::make_shared<MediaDevice>(deviceNode);
+	std::unique_ptr<MediaDevice> media = std::make_unique<MediaDevice>(deviceNode);
 
 	int ret = media->populate();
 	if (ret < 0) {
@@ -236,12 +236,12 @@  std::shared_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &d
  * This method shall be called after all members of the entities of the
  * media graph have been confirmed to be initialized.
  */
-void DeviceEnumerator::addDevice(const std::shared_ptr<MediaDevice> &media)
+void DeviceEnumerator::addDevice(std::unique_ptr<MediaDevice> &&media)
 {
 	LOG(DeviceEnumerator, Debug)
 		<< "Added device " << media->deviceNode() << ": " << media->driver();
 
-	devices_.push_back(media);
+	devices_.push_back(std::move(media));
 }
 
 /**
@@ -290,7 +290,7 @@  void DeviceEnumerator::removeDevice(const std::string &deviceNode)
  */
 std::shared_ptr<MediaDevice> DeviceEnumerator::search(const DeviceMatch &dm)
 {
-	for (std::shared_ptr<MediaDevice> media : devices_) {
+	for (std::shared_ptr<MediaDevice> &media : devices_) {
 		if (media->busy())
 			continue;
 
diff --git a/src/libcamera/device_enumerator_sysfs.cpp b/src/libcamera/device_enumerator_sysfs.cpp
index 197ca235879b..3446db59e9d4 100644
--- a/src/libcamera/device_enumerator_sysfs.cpp
+++ b/src/libcamera/device_enumerator_sysfs.cpp
@@ -72,11 +72,11 @@  int DeviceEnumeratorSysfs::enumerate()
 			continue;
 		}
 
-		std::shared_ptr<MediaDevice> media = createDevice(devnode);
+		std::unique_ptr<MediaDevice> media = createDevice(devnode);
 		if (!media)
 			continue;
 
-		if (populateMediaDevice(media) < 0) {
+		if (populateMediaDevice(media.get()) < 0) {
 			LOG(DeviceEnumerator, Warning)
 				<< "Failed to populate media device "
 				<< media->deviceNode()
@@ -84,7 +84,7 @@  int DeviceEnumeratorSysfs::enumerate()
 			continue;
 		}
 
-		addDevice(media);
+		addDevice(std::move(media));
 	}
 
 	closedir(dir);
@@ -92,7 +92,7 @@  int DeviceEnumeratorSysfs::enumerate()
 	return 0;
 }
 
-int DeviceEnumeratorSysfs::populateMediaDevice(const std::shared_ptr<MediaDevice> &media)
+int DeviceEnumeratorSysfs::populateMediaDevice(MediaDevice *media)
 {
 	/* Associate entities to device node paths. */
 	for (MediaEntity *entity : media->entities()) {
diff --git a/src/libcamera/device_enumerator_udev.cpp b/src/libcamera/device_enumerator_udev.cpp
index ea3f6b7c9ae0..9cbc7e47d2d9 100644
--- a/src/libcamera/device_enumerator_udev.cpp
+++ b/src/libcamera/device_enumerator_udev.cpp
@@ -76,7 +76,7 @@  int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
 		return -ENODEV;
 
 	if (!strcmp(subsystem, "media")) {
-		std::shared_ptr<MediaDevice> media =
+		std::unique_ptr<MediaDevice> media =
 			createDevice(udev_device_get_devnode(dev));
 		if (!media)
 			return -ENODEV;
@@ -96,7 +96,7 @@  int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
 				<< "Defer media device " << media->deviceNode()
 				<< " due to " << ret << " missing dependencies";
 
-			pending_.emplace_back(media, deps);
+			pending_.emplace_back(std::move(media), std::move(deps));
 			MediaDeviceDeps *mediaDeps = &pending_.back();
 			for (const auto &dep : mediaDeps->deps_)
 				devMap_[dep.first] = mediaDeps;
@@ -104,7 +104,7 @@  int DeviceEnumeratorUdev::addUdevDevice(struct udev_device *dev)
 			return 0;
 		}
 
-		addDevice(media);
+		addDevice(std::move(media));
 		return 0;
 	}
 
@@ -319,7 +319,7 @@  int DeviceEnumeratorUdev::addV4L2Device(dev_t devnum)
 		LOG(DeviceEnumerator, Debug)
 			<< "All dependencies for media device "
 			<< deps->media_->deviceNode() << " found";
-		addDevice(deps->media_);
+		addDevice(std::move(deps->media_));
 		pending_.remove(*deps);
 	}
 
diff --git a/src/libcamera/include/device_enumerator.h b/src/libcamera/include/device_enumerator.h
index 770f42772270..433e357aebae 100644
--- a/src/libcamera/include/device_enumerator.h
+++ b/src/libcamera/include/device_enumerator.h
@@ -44,8 +44,8 @@  public:
 	std::shared_ptr<MediaDevice> search(const DeviceMatch &dm);
 
 protected:
-	std::shared_ptr<MediaDevice> createDevice(const std::string &deviceNode);
-	void addDevice(const std::shared_ptr<MediaDevice> &media);
+	std::unique_ptr<MediaDevice> createDevice(const std::string &deviceNode);
+	void addDevice(std::unique_ptr<MediaDevice> &&media);
 	void removeDevice(const std::string &deviceNode);
 
 private:
diff --git a/src/libcamera/include/device_enumerator_sysfs.h b/src/libcamera/include/device_enumerator_sysfs.h
index 9063f6a75e11..5a5c9b0f5a31 100644
--- a/src/libcamera/include/device_enumerator_sysfs.h
+++ b/src/libcamera/include/device_enumerator_sysfs.h
@@ -23,7 +23,7 @@  public:
 	int enumerate();
 
 private:
-	int populateMediaDevice(const std::shared_ptr<MediaDevice> &media);
+	int populateMediaDevice(MediaDevice *media);
 	std::string lookupDeviceNode(int major, int minor);
 };
 
diff --git a/src/libcamera/include/device_enumerator_udev.h b/src/libcamera/include/device_enumerator_udev.h
index efaefe5dc4a3..fdce4520f33a 100644
--- a/src/libcamera/include/device_enumerator_udev.h
+++ b/src/libcamera/include/device_enumerator_udev.h
@@ -43,9 +43,9 @@  private:
 	using DependencyMap = std::map<dev_t, std::list<MediaEntity *>>;
 
 	struct MediaDeviceDeps {
-		MediaDeviceDeps(const std::shared_ptr<MediaDevice> &media,
-				const DependencyMap &deps)
-			: media_(media), deps_(deps)
+		MediaDeviceDeps(std::unique_ptr<MediaDevice> &&media,
+				DependencyMap &&deps)
+			: media_(std::move(media)), deps_(std::move(deps))
 		{
 		}
 
@@ -54,7 +54,7 @@  private:
 			return media_ == other.media_;
 		}
 
-		std::shared_ptr<MediaDevice> media_;
+		std::unique_ptr<MediaDevice> media_;
 		DependencyMap deps_;
 	};