[libcamera-devel] [PATCH 4/5] android: camera_device: Get properties from configuration
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Apr 13 22:00:56 CEST 2021
Hi Jacopo,
Thank you for the patch.
On Tue, Apr 13, 2021 at 04:50:41PM +0200, Jacopo Mondi wrote:
> Open the HAL configuration file in the Camera HAL manager and get
> the camera properties for each created CameraDevice and initialize it
> with them.
>
> Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> ---
> src/android/camera_device.cpp | 56 +++++++++++++++++++++++++-----
> src/android/camera_device.h | 3 +-
> src/android/camera_hal_manager.cpp | 37 +++++++++++++++++++-
> src/android/camera_hal_manager.h | 3 ++
> 4 files changed, 89 insertions(+), 10 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 89044efa7ebe..2e93936fdb4b 100644
> --- a/src/android/camera_device.cpp
> +++ b/src/android/camera_device.cpp
> @@ -6,6 +6,7 @@
> */
>
> #include "camera_device.h"
> +#include "camera_hal_config.h"
> #include "camera_ops.h"
> #include "post_processor.h"
>
> @@ -443,12 +444,25 @@ std::unique_ptr<CameraDevice> CameraDevice::create(unsigned int id,
> }
>
> /*
> - * Initialize the camera static information.
> + * Initialize the camera static information retrieved from the
> + * Camera::properties or from the cameraPros.
> + *
> + * cameraProps is optional for external camera devices and is defaulted to
> + * nullptr.
> + *
> * This method is called before the camera device is opened.
> */
> -int CameraDevice::initialize()
> +int CameraDevice::initialize(const CameraProps *cameraProps)
> {
> - /* Initialize orientation and facing side of the camera. */
> + /*
> + * Initialize orientation and facing side of the camera.
> + *
> + * If the libcamera::Camera provides those information as retrieved
> + * from firmware use them, otherwise fallback to values parsed from
> + * the configuration file. If the configuration file is not available
> + * the camera is external so its location and rotation can be safely
> + * defaulted.
> + */
> const ControlList &properties = camera_->properties();
>
> if (properties.contains(properties::Location)) {
> @@ -464,12 +478,22 @@ int CameraDevice::initialize()
> facing_ = CAMERA_FACING_EXTERNAL;
> break;
> }
> +
> + if (cameraProps && cameraProps->facing != -1 &&
> + facing_ != cameraProps->facing) {
> + LOG(HAL, Warning)
> + << "Camera location does not match"
> + << " configuration file. Using " << facing_;
> + }
> + } else if (cameraProps) {
> + if (cameraProps->facing == -1) {
> + LOG(HAL, Error)
> + << "Camera facing not in configuration file";
> + return -EINVAL;
> + }
> + facing_ = cameraProps->facing;
> } else {
> - /*
> - * \todo Retrieve the camera location from configuration file
> - * if not available from the library.
> - */
> - facing_ = CAMERA_FACING_FRONT;
> + facing_ = CAMERA_FACING_EXTERNAL;
> }
>
> /*
> @@ -483,8 +507,24 @@ int CameraDevice::initialize()
> if (properties.contains(properties::Rotation)) {
> int rotation = properties.get(properties::Rotation);
> orientation_ = (360 - rotation) % 360;
> + if (cameraProps && cameraProps->rotation != -1 &&
> + orientation_ != cameraProps->rotation) {
> + LOG(HAL, Warning)
> + << "Camera orientation does not match"
> + << " configuration file. Using " << orientation_;
> + }
> + } else if (cameraProps) {
> + if (cameraProps->rotation == -1) {
> + LOG(HAL, Error)
> + << "Camera rotation not in configuration file";
> + return -EINVAL;
> + }
> + orientation_ = cameraProps->rotation;
> + } else {
> + orientation_ = 0;
> }
>
> + /* Acquire the camera and initialize available stream configurations. */
> int ret = camera_->acquire();
> if (ret) {
> LOG(HAL, Error) << "Failed to temporarily acquire the camera";
> diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> index 11bdfec8d587..598d89f1cff0 100644
> --- a/src/android/camera_device.h
> +++ b/src/android/camera_device.h
> @@ -29,6 +29,7 @@
> #include "camera_worker.h"
> #include "jpeg/encoder.h"
>
> +class CameraProps;
> class CameraDevice : protected libcamera::Loggable
> {
> public:
> @@ -36,7 +37,7 @@ public:
> std::shared_ptr<libcamera::Camera> cam);
> ~CameraDevice();
>
> - int initialize();
> + int initialize(const CameraProps *cameraProps = nullptr);
You can drop the = nullptr if you agree with the change below.
>
> int open(const hw_module_t *hardwareModule);
> void close();
> diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> index bf3fcda75237..1defc3f9c5bd 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -41,6 +41,16 @@ int CameraHalManager::init()
> {
> cameraManager_ = std::make_unique<CameraManager>();
>
> + /*
> + * Open and parse configuration file.
> + *
> + * If the configuration file is not available the HAL only supports
> + * external cameras. If it exists but it's not valid then error out.
> + */
> + halConfig_.open();
> + if (halConfig_.exists() && !halConfig_.valid())
> + return -EINVAL;
> +
> /* Support camera hotplug. */
> cameraManager_->cameraAdded.connect(this, &CameraHalManager::cameraAdded);
> cameraManager_->cameraRemoved.connect(this, &CameraHalManager::cameraRemoved);
> @@ -100,6 +110,8 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
> auto iter = cameraIdsMap_.find(cam->id());
> if (iter != cameraIdsMap_.end()) {
> id = iter->second;
> + if (id >= firstExternalCameraId_)
> + isCameraExternal = true;
> } else {
> isCameraNew = true;
>
> @@ -117,7 +129,30 @@ void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
>
> /* Create a CameraDevice instance to wrap the libcamera Camera. */
> std::unique_ptr<CameraDevice> camera = CameraDevice::create(id, cam);
> - int ret = camera->initialize();
> +
> + /*
> + * The configuration file must be valid for internal cameras.
> + * External cameras can be initialized without configuration file.
> + */
> + int ret = -EINVAL;
> + if (!halConfig_.exists()) {
> + if (isCameraExternal)
> + ret = camera->initialize();
> + } else {
> + /*
> + * Get camera properties from the configuration file which
> + * exists and is valid.
> + *
> + * Internal cameras are required to have a corresponding entry
> + * in the configuration file. External cameras are not required
> + * to.
> + */
> + const CameraProps *cameraProps = &halConfig_.cameraProps(cam->id());
> + if (cameraProps->valid)
> + ret = camera->initialize(cameraProps);
> + else if (isCameraExternal)
> + ret = camera->initialize();
> + }
> if (ret) {
> LOG(HAL, Error) << "Failed to initialize camera: " << cam->id();
> return;
If the configuration file does not exist and the camera is internal,
we'll only print "Failed to initialize camera", which will be confusing.
How about the following ?
/*
* The configuration file must be valid, and contain a corresponding
* entry for internal cameras. External cameras can be initialized
* without configuration file.
*/
if (!isCameraExternal && !halConfig_.exists()) {
LOG(HALConfig, Error)
<< "HAL configuration file is mandatory for internal cameras";
return -EINVAL;
}
const CameraProps *cameraProps = &halConfig_.cameraProps(cam->id());
if (!isCameraExternal && !cameraProps) {
LOG(HALConfig, Error)
<< "HAL configuration entry for camera " << cam->id()
<< " is missing";
return -EINVAL;
}
ret = camera->initialize(cameraProps);
if (ret) {
LOG(HAL, Error) << "Failed to initialize camera: " << cam->id();
return;
}
Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h
> index d9bf27989965..af1581da6579 100644
> --- a/src/android/camera_hal_manager.h
> +++ b/src/android/camera_hal_manager.h
> @@ -19,6 +19,8 @@
>
> #include <libcamera/camera_manager.h>
>
> +#include "camera_hal_config.h"
> +
> class CameraDevice;
>
> class CameraHalManager
> @@ -50,6 +52,7 @@ private:
> CameraDevice *cameraDeviceFromHalId(unsigned int id);
>
> std::unique_ptr<libcamera::CameraManager> cameraManager_;
> + CameraHalConfig halConfig_;
>
> const camera_module_callbacks_t *callbacks_;
> std::vector<std::unique_ptr<CameraDevice>> cameras_;
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list