[libcamera-devel,v4,3/6] libcamera: device_enumerator: Emit a signal when a new devices are added
diff mbox series

Message ID 20200611171528.9381-4-email@uajain.com
State Superseded
Headers show
Series
  • Introduce UVC hotplugging support
Related show

Commit Message

Umang Jain June 11, 2020, 5:16 p.m. UTC 
Emit a signal whenever a new MediaDevices are added to the
DeviceEnumerator. This will allow CameraManager to be notified
about the new devices and it can re-emumerate all the devices
currently present on the system.

Device enumeration by the CameraManger is a expensive operation hence,
we want one signal emission per 'x' milliseconds to notify multiple
devices additions as a single batch, by the DeviceEnumerator.
Add a \todo to investigate the support for that.

Signed-off-by: Umang Jain <email@uajain.com>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 include/libcamera/internal/device_enumerator.h |  4 ++++
 src/libcamera/camera_manager.cpp               |  4 ++--
 src/libcamera/device_enumerator.cpp            | 13 +++++++++++++
 3 files changed, 19 insertions(+), 2 deletions(-)

Comments

Laurent Pinchart June 12, 2020, 10:45 a.m. UTC | #1 
Hi Umang,

Thank you for the patch.

On Thu, Jun 11, 2020 at 05:16:05PM +0000, Umang Jain wrote:
> Emit a signal whenever a new MediaDevices are added to the

s/a new/new/

> DeviceEnumerator. This will allow CameraManager to be notified
> about the new devices and it can re-emumerate all the devices
> currently present on the system.
> 
> Device enumeration by the CameraManger is a expensive operation hence,
> we want one signal emission per 'x' milliseconds to notify multiple
> devices additions as a single batch, by the DeviceEnumerator.
> Add a \todo to investigate the support for that.
> 
> Signed-off-by: Umang Jain <email@uajain.com>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> ---
>  include/libcamera/internal/device_enumerator.h |  4 ++++
>  src/libcamera/camera_manager.cpp               |  4 ++--
>  src/libcamera/device_enumerator.cpp            | 13 +++++++++++++
>  3 files changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/include/libcamera/internal/device_enumerator.h b/include/libcamera/internal/device_enumerator.h
> index 25a3630..a985040 100644
> --- a/include/libcamera/internal/device_enumerator.h
> +++ b/include/libcamera/internal/device_enumerator.h
> @@ -13,6 +13,8 @@
>  
>  #include <linux/media.h>
>  
> +#include <libcamera/signal.h>
> +
>  namespace libcamera {
>  
>  class MediaDevice;
> @@ -43,6 +45,8 @@ public:
>  
>  	std::shared_ptr<MediaDevice> search(const DeviceMatch &dm);
>  
> +	Signal<> devicesAdded;
> +
>  protected:
>  	std::unique_ptr<MediaDevice> createDevice(const std::string &deviceNode);
>  	void addDevice(std::unique_ptr<MediaDevice> &&media);
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index aba5a0c..17a4afb 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -155,12 +155,12 @@ void CameraManager::Private::createPipelineHandlers()
>  		}
>  	}
>  
> -	/* \todo Register hot-plug callback here */
> +	enumerator_->devicesAdded.connect(this, &Private::createPipelineHandlers);
>  }
>  
>  void CameraManager::Private::cleanup()
>  {
> -	/* \todo Unregister hot-plug callback here */
> +	enumerator_->devicesAdded.disconnect(this, &Private::createPipelineHandlers);
>  
>  	/*
>  	 * Release all references to cameras and pipeline handlers to ensure
> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
> index e21a2a7..e3264a5 100644
> --- a/src/libcamera/device_enumerator.cpp
> +++ b/src/libcamera/device_enumerator.cpp
> @@ -227,6 +227,16 @@ std::unique_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &d
>  	return media;
>  }
>  
> +/**
> +* \var DeviceEnumerator::devicesAdded
> +* \brief Notify of new media devices being found
> +*
> +* This signal is emitted when the device enumerator finds new media devices in
> +* the system. It may be emitted for every newly detected device, or once for
> +* multiple of devices, at the discretion of the device enumerator. Not all
> +* device enumerator types may support dynamic detection of new devices.
> +*/
> +
>  /**
>   * \brief Add a media device to the enumerator
>   * \param[in] media media device instance to add
> @@ -242,6 +252,9 @@ void DeviceEnumerator::addDevice(std::unique_ptr<MediaDevice> &&media)
>  		<< "Added device " << media->deviceNode() << ": " << media->driver();
>  
>  	devices_.push_back(std::move(media));
> +
> +	/* \todo To batch multiple additions, emit with a small delay here. */
> +	devicesAdded.emit();
>  }
>  
>  /**
Kieran Bingham June 12, 2020, 3:18 p.m. UTC | #2 
Hi Umang,

On 12/06/2020 11:45, Laurent Pinchart wrote:
> Hi Umang,
> 
> Thank you for the patch.
> 
> On Thu, Jun 11, 2020 at 05:16:05PM +0000, Umang Jain wrote:
>> Emit a signal whenever a new MediaDevices are added to the
> 
> s/a new/new/
> 
>> DeviceEnumerator. This will allow CameraManager to be notified
>> about the new devices and it can re-emumerate all the devices
>> currently present on the system.
>>
>> Device enumeration by the CameraManger is a expensive operation hence,

I'd ignore this generally, but as you are already likely to update the
commit message from above:

s/is a/is an/


>> we want one signal emission per 'x' milliseconds to notify multiple
>> devices additions as a single batch, by the DeviceEnumerator.
>> Add a \todo to investigate the support for that.
>>
>> Signed-off-by: Umang Jain <email@uajain.com>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> ---
>>  include/libcamera/internal/device_enumerator.h |  4 ++++
>>  src/libcamera/camera_manager.cpp               |  4 ++--
>>  src/libcamera/device_enumerator.cpp            | 13 +++++++++++++
>>  3 files changed, 19 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/libcamera/internal/device_enumerator.h b/include/libcamera/internal/device_enumerator.h
>> index 25a3630..a985040 100644
>> --- a/include/libcamera/internal/device_enumerator.h
>> +++ b/include/libcamera/internal/device_enumerator.h
>> @@ -13,6 +13,8 @@
>>  
>>  #include <linux/media.h>
>>  
>> +#include <libcamera/signal.h>
>> +
>>  namespace libcamera {
>>  
>>  class MediaDevice;
>> @@ -43,6 +45,8 @@ public:
>>  
>>  	std::shared_ptr<MediaDevice> search(const DeviceMatch &dm);
>>  
>> +	Signal<> devicesAdded;
>> +
>>  protected:
>>  	std::unique_ptr<MediaDevice> createDevice(const std::string &deviceNode);
>>  	void addDevice(std::unique_ptr<MediaDevice> &&media);
>> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
>> index aba5a0c..17a4afb 100644
>> --- a/src/libcamera/camera_manager.cpp
>> +++ b/src/libcamera/camera_manager.cpp
>> @@ -155,12 +155,12 @@ void CameraManager::Private::createPipelineHandlers()
>>  		}
>>  	}
>>  
>> -	/* \todo Register hot-plug callback here */
>> +	enumerator_->devicesAdded.connect(this, &Private::createPipelineHandlers);
>>  }
>>  
>>  void CameraManager::Private::cleanup()
>>  {
>> -	/* \todo Unregister hot-plug callback here */
>> +	enumerator_->devicesAdded.disconnect(this, &Private::createPipelineHandlers);
>>  
>>  	/*
>>  	 * Release all references to cameras and pipeline handlers to ensure
>> diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
>> index e21a2a7..e3264a5 100644
>> --- a/src/libcamera/device_enumerator.cpp
>> +++ b/src/libcamera/device_enumerator.cpp
>> @@ -227,6 +227,16 @@ std::unique_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &d
>>  	return media;
>>  }
>>  
>> +/**
>> +* \var DeviceEnumerator::devicesAdded
>> +* \brief Notify of new media devices being found
>> +*
>> +* This signal is emitted when the device enumerator finds new media devices in
>> +* the system. It may be emitted for every newly detected device, or once for
>> +* multiple of devices, at the discretion of the device enumerator. Not all

's/ of //'
== "or once for multiple devices"

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


>> +* device enumerator types may support dynamic detection of new devices.
>> +*/
>> +
>>  /**
>>   * \brief Add a media device to the enumerator
>>   * \param[in] media media device instance to add
>> @@ -242,6 +252,9 @@ void DeviceEnumerator::addDevice(std::unique_ptr<MediaDevice> &&media)
>>  		<< "Added device " << media->deviceNode() << ": " << media->driver();
>>  
>>  	devices_.push_back(std::move(media));
>> +
>> +	/* \todo To batch multiple additions, emit with a small delay here. */
>> +	devicesAdded.emit();
>>  }
>>  
>>  /**
>

Patch
diff mbox series 

diff --git a/include/libcamera/internal/device_enumerator.h b/include/libcamera/internal/device_enumerator.h
index 25a3630..a985040 100644
--- a/include/libcamera/internal/device_enumerator.h
+++ b/include/libcamera/internal/device_enumerator.h
@@ -13,6 +13,8 @@ 
 
 #include <linux/media.h>
 
+#include <libcamera/signal.h>
+
 namespace libcamera {
 
 class MediaDevice;
@@ -43,6 +45,8 @@  public:
 
 	std::shared_ptr<MediaDevice> search(const DeviceMatch &dm);
 
+	Signal<> devicesAdded;
+
 protected:
 	std::unique_ptr<MediaDevice> createDevice(const std::string &deviceNode);
 	void addDevice(std::unique_ptr<MediaDevice> &&media);
diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
index aba5a0c..17a4afb 100644
--- a/src/libcamera/camera_manager.cpp
+++ b/src/libcamera/camera_manager.cpp
@@ -155,12 +155,12 @@  void CameraManager::Private::createPipelineHandlers()
 		}
 	}
 
-	/* \todo Register hot-plug callback here */
+	enumerator_->devicesAdded.connect(this, &Private::createPipelineHandlers);
 }
 
 void CameraManager::Private::cleanup()
 {
-	/* \todo Unregister hot-plug callback here */
+	enumerator_->devicesAdded.disconnect(this, &Private::createPipelineHandlers);
 
 	/*
 	 * Release all references to cameras and pipeline handlers to ensure
diff --git a/src/libcamera/device_enumerator.cpp b/src/libcamera/device_enumerator.cpp
index e21a2a7..e3264a5 100644
--- a/src/libcamera/device_enumerator.cpp
+++ b/src/libcamera/device_enumerator.cpp
@@ -227,6 +227,16 @@  std::unique_ptr<MediaDevice> DeviceEnumerator::createDevice(const std::string &d
 	return media;
 }
 
+/**
+* \var DeviceEnumerator::devicesAdded
+* \brief Notify of new media devices being found
+*
+* This signal is emitted when the device enumerator finds new media devices in
+* the system. It may be emitted for every newly detected device, or once for
+* multiple of devices, at the discretion of the device enumerator. Not all
+* device enumerator types may support dynamic detection of new devices.
+*/
+
 /**
  * \brief Add a media device to the enumerator
  * \param[in] media media device instance to add
@@ -242,6 +252,9 @@  void DeviceEnumerator::addDevice(std::unique_ptr<MediaDevice> &&media)
 		<< "Added device " << media->deviceNode() << ": " << media->driver();
 
 	devices_.push_back(std::move(media));
+
+	/* \todo To batch multiple additions, emit with a small delay here. */
+	devicesAdded.emit();
 }
 
 /**