[v1] android: camera_hal_manager: Remove `cameraLocation()`
diff mbox series

Message ID 20251103144755.438422-1-barnabas.pocze@ideasonboard.com
State Accepted
Headers show
Series
  • [v1] android: camera_hal_manager: Remove `cameraLocation()`
Related show

Commit Message

Barnabás Pőcze Nov. 3, 2025, 2:47 p.m. UTC 
Inline the function as it is only used in a single place and does not do
anything complicated. This also lets the `operator==` of `std::optional`
take care of the proper comparison instead of defaulting the value to -1
and comparing that.

Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
---
 src/android/camera_hal_manager.cpp | 7 +------
 src/android/camera_hal_manager.h   | 2 --
 2 files changed, 1 insertion(+), 8 deletions(-)

Comments

Laurent Pinchart Nov. 3, 2025, 2:50 p.m. UTC | #1 
On Mon, Nov 03, 2025 at 03:47:55PM +0100, Barnabás Pőcze wrote:
> Inline the function as it is only used in a single place and does not do
> anything complicated. This also lets the `operator==` of `std::optional`
> take care of the proper comparison instead of defaulting the value to -1
> and comparing that.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>

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

> ---
>  src/android/camera_hal_manager.cpp | 7 +------
>  src/android/camera_hal_manager.h   | 2 --
>  2 files changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index 7500c749be..a7a2571754 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -125,7 +125,7 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
>  		 * Now check if this is an external camera and assign
>  		 * its id accordingly.
>  		 */
> -		if (cameraLocation(cam.get()) == properties::CameraLocationExternal) {
> +		if (cam->properties().get(properties::Location) == properties::CameraLocationExternal) {
>  			isCameraExternal = true;
>  			id = nextExternalCameraId_;
>  		} else {
> @@ -227,11 +227,6 @@ void CameraHalManager::cameraRemoved(std::shared_ptr<Camera> cam)
>  	LOG(HAL, Debug) << "Camera ID: " << id << " removed successfully.";
>  }
>  
> -int32_t CameraHalManager::cameraLocation(const Camera *cam)
> -{
> -	return cam->properties().get(properties::Location).value_or(-1);
> -}
> -
>  CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)
>  {
>  	auto iter = std::find_if(cameras_.begin(), cameras_.end(),
> diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h
> index 836a8daf70..e2d4eacbb5 100644
> --- a/src/android/camera_hal_manager.h
> +++ b/src/android/camera_hal_manager.h
> @@ -48,8 +48,6 @@ private:
>  
>  	CameraHalManager();
>  
> -	static int32_t cameraLocation(const libcamera::Camera *cam);
> -
>  	void cameraAdded(std::shared_ptr<libcamera::Camera> cam);
>  	void cameraRemoved(std::shared_ptr<libcamera::Camera> cam);
>  
> -- 
> 2.51.2
>
Kieran Bingham Nov. 3, 2025, 3:25 p.m. UTC | #2 
Quoting Barnabás Pőcze (2025-11-03 14:47:55)
> Inline the function as it is only used in a single place and does not do
> anything complicated. This also lets the `operator==` of `std::optional`
> take care of the proper comparison instead of defaulting the value to -1
> and comparing that.
> 
> Signed-off-by: Barnabás Pőcze <barnabas.pocze@ideasonboard.com>
> ---
>  src/android/camera_hal_manager.cpp | 7 +------
>  src/android/camera_hal_manager.h   | 2 --
>  2 files changed, 1 insertion(+), 8 deletions(-)
> 
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index 7500c749be..a7a2571754 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -125,7 +125,7 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
>                  * Now check if this is an external camera and assign
>                  * its id accordingly.
>                  */
> -               if (cameraLocation(cam.get()) == properties::CameraLocationExternal) {
> +               if (cam->properties().get(properties::Location) == properties::CameraLocationExternal) {

This still looks sane so:

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

>                         isCameraExternal = true;
>                         id = nextExternalCameraId_;
>                 } else {
> @@ -227,11 +227,6 @@ void CameraHalManager::cameraRemoved(std::shared_ptr<Camera> cam)
>         LOG(HAL, Debug) << "Camera ID: " << id << " removed successfully.";
>  }
>  
> -int32_t CameraHalManager::cameraLocation(const Camera *cam)
> -{
> -       return cam->properties().get(properties::Location).value_or(-1);
> -}
> -
>  CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)
>  {
>         auto iter = std::find_if(cameras_.begin(), cameras_.end(),
> diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h
> index 836a8daf70..e2d4eacbb5 100644
> --- a/src/android/camera_hal_manager.h
> +++ b/src/android/camera_hal_manager.h
> @@ -48,8 +48,6 @@ private:
>  
>         CameraHalManager();
>  
> -       static int32_t cameraLocation(const libcamera::Camera *cam);
> -
>         void cameraAdded(std::shared_ptr<libcamera::Camera> cam);
>         void cameraRemoved(std::shared_ptr<libcamera::Camera> cam);
>  
> -- 
> 2.51.2
>

Patch
diff mbox series 

diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
index 7500c749be..a7a2571754 100644
--- a/src/android/camera_hal_manager.cpp
+++ b/src/android/camera_hal_manager.cpp
@@ -125,7 +125,7 @@  void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
 		 * Now check if this is an external camera and assign
 		 * its id accordingly.
 		 */
-		if (cameraLocation(cam.get()) == properties::CameraLocationExternal) {
+		if (cam->properties().get(properties::Location) == properties::CameraLocationExternal) {
 			isCameraExternal = true;
 			id = nextExternalCameraId_;
 		} else {
@@ -227,11 +227,6 @@  void CameraHalManager::cameraRemoved(std::shared_ptr<Camera> cam)
 	LOG(HAL, Debug) << "Camera ID: " << id << " removed successfully.";
 }
 
-int32_t CameraHalManager::cameraLocation(const Camera *cam)
-{
-	return cam->properties().get(properties::Location).value_or(-1);
-}
-
 CameraDevice *CameraHalManager::cameraDeviceFromHalId(unsigned int id)
 {
 	auto iter = std::find_if(cameras_.begin(), cameras_.end(),
diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h
index 836a8daf70..e2d4eacbb5 100644
--- a/src/android/camera_hal_manager.h
+++ b/src/android/camera_hal_manager.h
@@ -48,8 +48,6 @@  private:
 
 	CameraHalManager();
 
-	static int32_t cameraLocation(const libcamera::Camera *cam);
-
 	void cameraAdded(std::shared_ptr<libcamera::Camera> cam);
 	void cameraRemoved(std::shared_ptr<libcamera::Camera> cam);