[libcamera-devel] [PATCH] android: Add flag for external camera support

Jacopo Mondi jacopo at jmondi.org
Wed Mar 3 14:34:01 CET 2021


Hi Paul,

On Wed, Mar 03, 2021 at 06:37:56PM +0900, Paul Elder wrote:
> In the static metadata, we claim that we don't support external cameras,
> yet libcamera core defaults the camera location to external. In the HAL
> CameraDevice we defaulted external to front, but we hadn't handled
> CameraHalManager when counting the internal vs external cameras for
> hal_get_number_of_cameras().
>
> To solve this, add a flag to hold if we have external camera support
> (hardcoded to false until we do), and add that to the condition in
> CameraHalManager when deciding whether a camera should be counted as
> internal or external. CameraDevice also uses this flag to decide on the
> default location.

I have missed what condition should trigger for us setting
externalCameraSupport_ to true in the camera_hal_manager.

and once we do so, won't cameras that report location=external because
of the missing information from the firmware be reported as external
even if they're not ?

I think we're a bit running in circles here. In my opinion the best
course of actions would have been not registering properties::Location
if libcamera cannot find that information from firmware (or from
somewhere else in future), and the camera HAL should resort to a
configuration file in that case, and fail registering the camera if
said file is not available.

>
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> ---
>  src/android/camera_device.cpp      | 24 ++++++++++++++----------
>  src/android/camera_device.h        |  8 ++++++--
>  src/android/camera_hal_manager.cpp | 11 +++++++----
>  src/android/camera_hal_manager.h   |  2 ++
>  4 files changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index ae01c362..bb33c922 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -311,9 +311,11 @@ CameraDevice::Camera3RequestDescriptor::~Camera3RequestDescriptor()
>   * back to the framework using the designated callbacks.
>   */
>
> -CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr<Camera> &camera)
> +CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr<Camera> &camera,
> +			   bool externalCameraSupport)
>  	: id_(id), running_(false), camera_(camera), staticMetadata_(nullptr),
> -	  facing_(CAMERA_FACING_FRONT), orientation_(0)
> +	  facing_(CAMERA_FACING_FRONT), orientation_(0),
> +	  externalCameraSupport_(externalCameraSupport)
>  {
>  	camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
>
> @@ -350,9 +352,10 @@ CameraDevice::~CameraDevice()
>  }
>
>  std::shared_ptr<CameraDevice> CameraDevice::create(unsigned int id,
> -						   const std::shared_ptr<Camera> &cam)
> +						   const std::shared_ptr<Camera> &cam,
> +						   bool externalCameraSupport)
>  {
> -	CameraDevice *camera = new CameraDevice(id, cam);
> +	CameraDevice *camera = new CameraDevice(id, cam, externalCameraSupport);
>  	return std::shared_ptr<CameraDevice>(camera);
>  }
>
> @@ -375,11 +378,10 @@ int CameraDevice::initialize()
>  			facing_ = CAMERA_FACING_BACK;
>  			break;
>  		case properties::CameraLocationExternal:
> -			/*
> -			 * \todo Set this to EXTERNAL once we support
> -			 * HARDWARE_LEVEL_EXTERNAL
> -			 */
> -			facing_ = CAMERA_FACING_FRONT;
> +			if (externalCameraSupport_)
> +				facing_ = CAMERA_FACING_EXTERNAL;
> +			else
> +				facing_ = CAMERA_FACING_FRONT;
>  			break;
>  		}
>  	}
> @@ -1121,7 +1123,9 @@ const camera_metadata_t *CameraDevice::getStaticMetadata()
>  	staticMetadata_->addEntry(ANDROID_SCALER_CROPPING_TYPE, &croppingType, 1);
>
>  	/* Info static metadata. */
> -	uint8_t supportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
> +	uint8_t supportedHWLevel = externalCameraSupport_ ?
> +				   ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_EXTERNAL :
> +				   ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED;
>  	staticMetadata_->addEntry(ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL,
>  				  &supportedHWLevel, 1);
>
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 4905958e..f96934db 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -33,7 +33,8 @@ class CameraDevice : protected libcamera::Loggable
>  {
>  public:
>  	static std::shared_ptr<CameraDevice> create(unsigned int id,
> -						    const std::shared_ptr<libcamera::Camera> &cam);
> +						    const std::shared_ptr<libcamera::Camera> &cam,
> +						    bool externalCameraSupport);
>  	~CameraDevice();
>
>  	int initialize();
> @@ -66,7 +67,8 @@ protected:
>  	std::string logPrefix() const override;
>
>  private:
> -	CameraDevice(unsigned int id, const std::shared_ptr<libcamera::Camera> &camera);
> +	CameraDevice(unsigned int id, const std::shared_ptr<libcamera::Camera> &camera,
> +		     bool externalCameraSupport);
>
>  	struct Camera3RequestDescriptor {
>  		Camera3RequestDescriptor(libcamera::Camera *camera,
> @@ -130,6 +132,8 @@ private:
>  	unsigned int maxJpegBufferSize_;
>
>  	CameraMetadata lastSettings_;
> +
> +	const bool externalCameraSupport_;
>  };
>
>  #endif /* __ANDROID_CAMERA_DEVICE_H__ */
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index 189eda2b..e4ca4c82 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -29,7 +29,8 @@ LOG_DECLARE_CATEGORY(HAL)
>   */
>
>  CameraHalManager::CameraHalManager()
> -	: cameraManager_(nullptr), callbacks_(nullptr), numInternalCameras_(0),
> +	: cameraManager_(nullptr), callbacks_(nullptr),
> +	  externalCameraSupport_(false), numInternalCameras_(0),
>  	  nextExternalCameraId_(firstExternalCameraId_)
>  {
>  }
> @@ -115,7 +116,8 @@ 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 (cameraLocation(cam.get()) == properties::CameraLocationExternal &&
> +		    externalCameraSupport_) {
>  			isCameraExternal = true;
>  			id = nextExternalCameraId_;
>  		} else {
> @@ -124,7 +126,8 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
>  	}
>
>  	/* Create a CameraDevice instance to wrap the libcamera Camera. */
> -	std::shared_ptr<CameraDevice> camera = CameraDevice::create(id, std::move(cam));
> +	std::shared_ptr<CameraDevice> camera =
> +		CameraDevice::create(id, std::move(cam), externalCameraSupport_);
>  	int ret = camera->initialize();
>  	if (ret) {
>  		LOG(HAL, Error) << "Failed to initialize camera: " << cam->id();
> @@ -134,7 +137,7 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
>  	if (isCameraNew) {
>  		cameraIdsMap_.emplace(cam->id(), id);
>
> -		if (isCameraExternal)
> +		if (isCameraExternal && externalCameraSupport_)
>  			nextExternalCameraId_++;
>  		else
>  			numInternalCameras_++;
> diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h
> index a91decc7..74dc6b3e 100644
> --- a/src/android/camera_hal_manager.h
> +++ b/src/android/camera_hal_manager.h
> @@ -54,6 +54,8 @@ private:
>  	std::map<std::string, unsigned int> cameraIdsMap_;
>  	Mutex mutex_;
>
> +	const bool externalCameraSupport_;
> +
>  	unsigned int numInternalCameras_;
>  	unsigned int nextExternalCameraId_;
>  };
> --
> 2.27.0
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel


More information about the libcamera-devel mailing list