[libcamera-devel] [PATCH v2 5/6] android: camera_device: Get location from config
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Mar 30 02:17:41 CEST 2021
On Tue, Mar 30, 2021 at 03:17:21AM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
>
> Thank you for the patch.
>
> On Mon, Mar 29, 2021 at 05:28:06PM +0200, Jacopo Mondi wrote:
> > Create the CameraDevice with a reference to the HAL configuration
> > file and use it to retrieve the camera location if not available
> > from the Camera.
>
> Overall this looks good, but I still think we should pass the camera
> configuration to the CameraDevice class, not the full CameraHalConfig.
> I'll comment further on this topic in the review of 4/6.
Sorry, I meant 3/6.
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> > src/android/camera_device.cpp | 18 +++++++++---------
> > src/android/camera_device.h | 8 ++++++--
> > src/android/camera_hal_manager.cpp | 3 ++-
> > 3 files changed, 17 insertions(+), 12 deletions(-)
> >
> > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > index ae6936647660..1731fe166887 100644
> > --- a/src/android/camera_device.cpp
> > +++ b/src/android/camera_device.cpp
> > @@ -309,9 +309,10 @@ CameraDevice::Camera3RequestDescriptor::~Camera3RequestDescriptor() = default;
> > * back to the framework using the designated callbacks.
> > */
> >
> > -CameraDevice::CameraDevice(unsigned int id, std::shared_ptr<Camera> camera)
> > +CameraDevice::CameraDevice(unsigned int id, std::shared_ptr<Camera> camera,
> > + CameraHalConfig &halConfig)
> > : id_(id), running_(false), camera_(std::move(camera)),
> > - facing_(CAMERA_FACING_FRONT), orientation_(0)
> > + halConfig_(halConfig), facing_(CAMERA_FACING_FRONT), orientation_(0)
> > {
> > camera_->requestCompleted.connect(this, &CameraDevice::requestComplete);
> >
> > @@ -341,10 +342,11 @@ CameraDevice::CameraDevice(unsigned int id, std::shared_ptr<Camera> camera)
> > CameraDevice::~CameraDevice() = default;
> >
> > std::unique_ptr<CameraDevice> CameraDevice::create(unsigned int id,
> > - std::shared_ptr<Camera> cam)
> > + std::shared_ptr<Camera> cam,
> > + CameraHalConfig &halConfig)
> > {
> > return std::unique_ptr<CameraDevice>(
> > - new CameraDevice(id, std::move(cam)));
> > + new CameraDevice(id, std::move(cam), halConfig));
> > }
> >
> > /*
> > @@ -370,11 +372,9 @@ int CameraDevice::initialize()
> > break;
> > }
> > } else {
> > - /*
> > - * \todo Retrieve the camera location from configuration file
> > - * if not available from the library.
> > - */
> > - facing_ = CAMERA_FACING_FRONT;
> > + facing_ = halConfig_.cameraLocation(camera_->id());
> > + if (facing_ < 0)
> > + return facing_;
> > }
> >
> > /*
> > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > index 11bdfec8d587..6355e8d8c26a 100644
> > --- a/src/android/camera_device.h
> > +++ b/src/android/camera_device.h
> > @@ -24,6 +24,7 @@
> > #include "libcamera/internal/log.h"
> > #include "libcamera/internal/message.h"
> >
> > +#include "camera_hal_config.h"
> > #include "camera_metadata.h"
> > #include "camera_stream.h"
> > #include "camera_worker.h"
> > @@ -33,7 +34,8 @@ class CameraDevice : protected libcamera::Loggable
> > {
> > public:
> > static std::unique_ptr<CameraDevice> create(unsigned int id,
> > - std::shared_ptr<libcamera::Camera> cam);
> > + std::shared_ptr<libcamera::Camera> cam,
> > + CameraHalConfig &halConfig);
> > ~CameraDevice();
> >
> > int initialize();
> > @@ -66,7 +68,8 @@ protected:
> > std::string logPrefix() const override;
> >
> > private:
> > - CameraDevice(unsigned int id, std::shared_ptr<libcamera::Camera> camera);
> > + CameraDevice(unsigned int id, std::shared_ptr<libcamera::Camera> camera,
> > + CameraHalConfig &halConfig);
> >
> > struct Camera3RequestDescriptor {
> > Camera3RequestDescriptor(libcamera::Camera *camera,
> > @@ -113,6 +116,7 @@ private:
> > bool running_;
> > std::shared_ptr<libcamera::Camera> camera_;
> > std::unique_ptr<libcamera::CameraConfiguration> config_;
> > + const CameraHalConfig &halConfig_;
> >
> > std::unique_ptr<CameraMetadata> staticMetadata_;
> > std::map<unsigned int, std::unique_ptr<CameraMetadata>> requestTemplates_;
> > diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> > index f79789b5bfb8..9ff7534a16f3 100644
> > --- a/src/android/camera_hal_manager.cpp
> > +++ b/src/android/camera_hal_manager.cpp
> > @@ -120,7 +120,8 @@ 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);
> > + std::unique_ptr<CameraDevice> camera = CameraDevice::create(id, cam,
> > + halConfig_);
> > int ret = camera->initialize();
> > if (ret) {
> > LOG(HAL, Error) << "Failed to initialize camera: " << cam->id();
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list