[libcamera-devel] [PATCH v2 2/8] android: CameraHalManager: Hold CameraManager with std::unique_ptr

[Laurent Pinchart]

Hi Hiro,

Thank you for the patch.

On Wed, Mar 24, 2021 at 04:07:51PM +0900, Hirokazu Honda wrote:
> CameraManager is owned by CameraHalManager. The ownership of the
> object is not shared with other classes. So CameraHalManager
> should manage CameraManager with std::unique_ptr.
> 
> Signed-off-by: Hirokazu Honda <hiroh at chromium.org>

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

> ---
>  src/android/camera_hal_manager.cpp | 15 ++++-----------
>  src/android/camera_hal_manager.h   |  2 +-
>  2 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index b3c85406..b501f8db 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -34,18 +34,12 @@ CameraHalManager::CameraHalManager()
>  {
>  }
> 
> -CameraHalManager::~CameraHalManager()
> -{
> -	if (cameraManager_) {
> -		cameraManager_->stop();
> -		delete cameraManager_;
> -		cameraManager_ = nullptr;
> -	}
> -}
> +/* CameraManager calls stop() in the destructor. */
> +CameraHalManager::~CameraHalManager() = default;
> 
>  int CameraHalManager::init()
>  {
> -	cameraManager_ = new CameraManager();
> +	cameraManager_ = std::make_unique<CameraManager>();
> 
>  	/* Support camera hotplug. */
>  	cameraManager_->cameraAdded.connect(this, &CameraHalManager::cameraAdded);
> @@ -55,8 +49,7 @@ int CameraHalManager::init()
>  	if (ret) {
>  		LOG(HAL, Error) << "Failed to start camera manager: "
>  				<< strerror(-ret);
> -		delete cameraManager_;
> -		cameraManager_ = nullptr;
> +		cameraManager_.reset();
>  		return ret;
>  	}
> 
> diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h
> index 65bb3554..dc4e37e5 100644
> --- a/src/android/camera_hal_manager.h
> +++ b/src/android/camera_hal_manager.h
> @@ -47,7 +47,7 @@ private:
> 
>  	CameraDevice *cameraDeviceFromHalId(unsigned int id);
> 
> -	libcamera::CameraManager *cameraManager_;
> +	std::unique_ptr<libcamera::CameraManager> cameraManager_;
> 
>  	const camera_module_callbacks_t *callbacks_;
>  	std::vector<std::unique_ptr<CameraDevice>> cameras_;

-- 
Regards,

Laurent Pinchart