[libcamera-devel] [PATCH 2/2] libcamera: CameraManager: Remove ::get(dev_t)

[Laurent Pinchart]

Hi Kieran,

Thank you for the patch.

On Thu, Jun 15, 2023 at 11:51:33PM +0100, Kieran Bingham via libcamera-devel wrote:
> The CameraManager::get(dev_t) implementation was provided only for the
> V4L2 Adaptation layer. This has now been replaced with the use of the
> public SystemDevices property.
> 
> Remove the deprecated function entirely, along with the camerasByDevnum_
> map which was only used to support this functionality.
> 
> This is a clear (and intentional) breakage in both the API and ABI.
> 
> Signed-off-by: Kieran Bingham <kieran.bingham at ideasonboard.com>

Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>

> ---
>  include/libcamera/camera_manager.h          |  1 -
>  include/libcamera/internal/camera_manager.h |  3 +-
>  src/libcamera/camera_manager.cpp            | 42 ---------------------
>  3 files changed, 1 insertion(+), 45 deletions(-)
> 
> diff --git a/include/libcamera/camera_manager.h b/include/libcamera/camera_manager.h
> index 9767acc42c89..1a891cacf26a 100644
> --- a/include/libcamera/camera_manager.h
> +++ b/include/libcamera/camera_manager.h
> @@ -32,7 +32,6 @@ public:
>  
>  	std::vector<std::shared_ptr<Camera>> cameras() const;
>  	std::shared_ptr<Camera> get(const std::string &id);
> -	std::shared_ptr<Camera> get(dev_t devnum);
>  
>  	static const std::string &version() { return version_; }
>  
> diff --git a/include/libcamera/internal/camera_manager.h b/include/libcamera/internal/camera_manager.h
> index cdf009a9c626..2aaf4acab07a 100644
> --- a/include/libcamera/internal/camera_manager.h
> +++ b/include/libcamera/internal/camera_manager.h
> @@ -42,11 +42,10 @@ public:
>  	 * This mutex protects
>  	 *
>  	 * - initialized_ and status_ during initialization
> -	 * - cameras_ and camerasByDevnum_ after initialization
> +	 * - cameras_ after initialization
>  	 */
>  	mutable Mutex mutex_;
>  	std::vector<std::shared_ptr<Camera>> cameras_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
> -	std::map<dev_t, std::weak_ptr<Camera>> camerasByDevnum_ LIBCAMERA_TSA_GUARDED_BY(mutex_);
>  
>  protected:
>  	void run() override;
> diff --git a/src/libcamera/camera_manager.cpp b/src/libcamera/camera_manager.cpp
> index 31d45c42fde0..cd81663281dd 100644
> --- a/src/libcamera/camera_manager.cpp
> +++ b/src/libcamera/camera_manager.cpp
> @@ -178,15 +178,9 @@ void CameraManager::Private::addCamera(std::shared_ptr<Camera> camera)
>  		}
>  	}
>  
> -	auto devnums = camera->properties()
> -			       .get(properties::SystemDevices)
> -			       .value_or(Span<int64_t>{});
> -
>  	cameras_.push_back(std::move(camera));
>  
>  	unsigned int index = cameras_.size() - 1;
> -	for (dev_t devnum : devnums)
> -		camerasByDevnum_[devnum] = cameras_[index];
>  
>  	/* Report the addition to the public signal */
>  	CameraManager *const o = LIBCAMERA_O_PTR();
> @@ -219,13 +213,6 @@ void CameraManager::Private::removeCamera(std::shared_ptr<Camera> camera)
>  	LOG(Camera, Debug)
>  		<< "Unregistering camera '" << camera->id() << "'";
>  
> -	auto iter_d = std::find_if(camerasByDevnum_.begin(), camerasByDevnum_.end(),
> -				   [camera](const std::pair<dev_t, std::weak_ptr<Camera>> &p) {
> -					   return p.second.lock().get() == camera.get();
> -				   });
> -	if (iter_d != camerasByDevnum_.end())
> -		camerasByDevnum_.erase(iter_d);
> -
>  	cameras_.erase(iter);
>  
>  	/* Report the removal to the public signal */
> @@ -366,35 +353,6 @@ std::shared_ptr<Camera> CameraManager::get(const std::string &id)
>  	return nullptr;
>  }
>  
> -/**
> - * \brief Retrieve a camera based on device number
> - * \param[in] devnum Device number of camera to get
> - *
> - * This function is meant solely for the use of the V4L2 compatibility
> - * layer, to map device nodes to Camera instances. Applications shall
> - * not use it and shall instead retrieve cameras by name.
> - *
> - * Before calling this function the caller is responsible for ensuring that
> - * the camera manager is running.
> - *
> - * \context This function is \threadsafe.
> - *
> - * \return Shared pointer to Camera object, which is empty if the camera is
> - * not found
> - */
> -std::shared_ptr<Camera> CameraManager::get(dev_t devnum)
> -{
> -	Private *const d = _d();
> -
> -	MutexLocker locker(d->mutex_);
> -
> -	auto iter = d->camerasByDevnum_.find(devnum);
> -	if (iter == d->camerasByDevnum_.end())
> -		return nullptr;
> -
> -	return iter->second.lock();
> -}
> -
>  /**
>   * \var CameraManager::cameraAdded
>   * \brief Notify of a new camera added to the system

-- 
Regards,

Laurent Pinchart