[libcamera-devel] [PATCH v6 4/5] android: camera_device: Get properties from configuration
Jacopo Mondi
jacopo at jmondi.org
Fri May 21 17:56:41 CEST 2021
Hi Hiro and Han-lin,
On Fri, May 21, 2021 at 03:41:10PM +0800, Han-lin Chen wrote:
> Hi Jacopo,
> Thanks for the efforts. The commits should be contained in 13971 and
> any later versions.
Great, thanks, I have now verified my series works on an
out-of-the-box image.
However the manifests for the SDK in manifest-version have not yet
catch up with the images and are stuck to R91. We'll have to wait
before merging the series for the right manifest to land, so we can
have both the image and the SDK at the same version.
Thanks
j
>
> On Fri, May 21, 2021 at 3: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
> >
> > 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
> > > >
>
>
>
> --
> Cheers.
> Hanlin Chen
More information about the libcamera-devel
mailing list