[libcamera-devel] [PATCH v6 4/5] android: camera_device: Get properties from configuration

Hirokazu Honda hiroh at chromium.org
Fri May 21 07:15:51 CEST 2021


Hi Jacopo,

On Thu, Apr 15, 2021 at 10:51 PM Jacopo Mondi <jacopo at jmondi.org> 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.
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>

Reviewed-by: Hirokazu Honda <hiroh at chromium.org>

When would you merge this series?

-Hiro

> 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 | 33 +++++++++++++++++-
>  src/android/camera_hal_manager.h   |  3 ++
>  4 files changed, 85 insertions(+), 10 deletions(-)
>
> diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> index 89044efa7ebe..50809c6ffbdc 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 cameraConfigData.
> + *
> + * cameraConfigData is optional for external camera devices and can be
> + * nullptr.
> + *
>   * This method is called before the camera device is opened.
>   */
> -int CameraDevice::initialize()
> +int CameraDevice::initialize(const CameraConfigData *cameraConfigData)
>  {
> -       /* 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 (cameraConfigData && cameraConfigData->facing != -1 &&
> +                   facing_ != cameraConfigData->facing) {
> +                       LOG(HAL, Warning)
> +                               << "Camera location does not match"
> +                               << " configuration file. Using " <<
> facing_;
> +               }
> +       } else if (cameraConfigData) {
> +               if (cameraConfigData->facing == -1) {
> +                       LOG(HAL, Error)
> +                               << "Camera facing not in configuration
> file";
> +                       return -EINVAL;
> +               }
> +               facing_ = cameraConfigData->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 (cameraConfigData && cameraConfigData->rotation != -1 &&
> +                   orientation_ != cameraConfigData->rotation) {
> +                       LOG(HAL, Warning)
> +                               << "Camera orientation does not match"
> +                               << " configuration file. Using " <<
> orientation_;
> +               }
> +       } else if (cameraConfigData) {
> +               if (cameraConfigData->rotation == -1) {
> +                       LOG(HAL, Error)
> +                               << "Camera rotation not in configuration
> file";
> +                       return -EINVAL;
> +               }
> +               orientation_ = cameraConfigData->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..9cc0d4005242 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"
>
> +struct CameraConfigData;
>  class CameraDevice : protected libcamera::Loggable
>  {
>  public:
> @@ -36,7 +37,7 @@ public:
>
> std::shared_ptr<libcamera::Camera> cam);
>         ~CameraDevice();
>
> -       int initialize();
> +       int initialize(const CameraConfigData *cameraConfigData);
>
>         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..f5b86974e8e3 100644
> --- a/src/android/camera_hal_manager.cpp
> +++ b/src/android/camera_hal_manager.cpp
> @@ -41,6 +41,15 @@ int CameraHalManager::init()
>  {
>         cameraManager_ = std::make_unique<CameraManager>();
>
> +       /*
> +        * If the configuration file is not available the HAL only supports
> +        * external cameras. If it exists but it's not valid then error
> out.
> +        */
> +       if (halConfig_.exists() && !halConfig_.isValid()) {
> +               LOG(HAL, Error) << "HAL configuration file is not valid";
> +               return -EINVAL;
> +       }
> +
>         /* Support camera hotplug. */
>         cameraManager_->cameraAdded.connect(this,
> &CameraHalManager::cameraAdded);
>         cameraManager_->cameraRemoved.connect(this,
> &CameraHalManager::cameraRemoved);
> @@ -100,6 +109,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 +128,27 @@ 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, and contain a
> corresponding
> +        * entry for internal cameras. External cameras can be initialized
> +        * without configuration file.
> +        */
> +       if (!isCameraExternal && !halConfig_.exists()) {
> +               LOG(HAL, Error)
> +                       << "HAL configuration file is mandatory for
> internal cameras";
> +               return;
> +       }
> +
> +       const CameraConfigData *cameraConfigData =
> halConfig_.cameraConfigData(cam->id());
> +       if (!isCameraExternal && !cameraConfigData) {
> +               LOG(HAL, Error)
> +                       << "HAL configuration entry for internal camera "
> +                       << cam->id() << " is missing";
> +               return;
> +       }
> +
> +       int ret = camera->initialize(cameraConfigData);
>         if (ret) {
>                 LOG(HAL, Error) << "Failed to initialize camera: " <<
> cam->id();
>                 return;
> 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_;
> --
> 2.31.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20210521/349448b4/attachment-0001.htm>


More information about the libcamera-devel mailing list