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

Hirokazu Honda hiroh at chromium.org
Fri May 21 09:40:42 CEST 2021


Hi Jacopo,

On Fri, May 21, 2021 at 4:29 PM Jacopo Mondi <jacopo at jmondi.org> wrote:

> Hi Hiro,
>
> On Fri, May 21, 2021 at 02:15:51PM +0900, Hirokazu Honda wrote:
> > 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?
>
> My plan was to update my SDK to the latest version which contains the
> CL to the libcamera and libcamera-configs ebuild, so I could have
> tested one more time on a fresh image. Am I too paranoid ?
>
> Could you or Han-lin tell me which is the id of the first CPFE image
> that contains:
>
> https://chromium-review.googlesource.com/c/chromiumos/overlays/board-overlays/+/2887093
>
> https://chromium-review.googlesource.com/c/chromiumos/overlays/chromiumos-overlay/+/2886535


They landed on 13971.0.0.

-Hiro

>
>
> I've tested those patches applied on my rather ancient SDK version,
> but I would like to try with an out-of-the-box image
>
> Thanks
>    j
>
> >
> > -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/b64b046b/attachment-0001.htm>


More information about the libcamera-devel mailing list