[libcamera-devel] [PATCH v2 3/4] android: camera_hal_manager: Support camera hotplug
Shik Chen
shik at google.com
Thu Aug 20 14:44:31 CEST 2020
Hi all,
On Thu, Aug 20, 2020 at 6:10 PM Laurent Pinchart
<laurent.pinchart at ideasonboard.com> wrote:
>
> Hi Niklas,
>
> On Thu, Aug 20, 2020 at 09:12:10AM +0200, Niklas Söderlund wrote:
> > On 2020-08-19 19:21:40 +0300, Laurent Pinchart wrote:
> > > On Thu, Aug 20, 2020 at 12:04:41AM +0800, Shik Chen wrote:
> > > > On Wed, Aug 19, 2020 at 11:52 PM Laurent Pinchart wrote:
> > > > > On Wed, Aug 19, 2020 at 11:41:45PM +0800, Shik Chen wrote:
> > > > > > On Tue, Aug 18, 2020 at 4:36 PM Laurent Pinchart wrote:
> > > > > > >
> > > > > > > Hi Shik,
> > > > > > >
> > > > > > > Would it be possible for you to have a look at the question below ?
> > > > > > >
> > > > > > > On Thu, Aug 13, 2020 at 03:05:30PM +0300, Laurent Pinchart wrote:
> > > > > > > > Hi Niklas,
> > > > > > > >
> > > > > > > > (CC'ing Shik)
> > > > > > > >
> > > > > > > > On Thu, Aug 13, 2020 at 01:44:18PM +0200, Niklas Söderlund wrote:
> > > > > > > > > On 2020-08-10 12:04:14 +0000, Umang Jain wrote:
> > > > > > > > > > Extend the support for camera hotplug from libcamera's CameraManager
> > > > > > > > > > to CameraHalManager. Use camera module callbacks to let the framework
> > > > > > > > > > know about the hotplug events and change the status of cameras being
> > > > > > > > > > being hotplugged or unplugged via camera_device_status_change().
> > > > > > > > > >
> > > > > > > > > > Introduce a map camerasMap_ which book-keeps all cameras seen in the
> > > > > > > > > > past by the CameraHalManager. If the camera is seen for the first time,
> > > > > > > > > > a new id is assigned to it. If the camera has been seen before by the
> > > > > > > > > > manager, it's old id is reused. IDs for internal cameras start with
> > > > > > > > > > '0' and for external cameras, they start with '1000'. Note, for the
> > > > > > > > > > current implementation, we assume all UVC cameras are external cameras.
> > > > > > > > >
> > > > > > > > > I wonder if keeping the cache of previously seen cameras or if we should
> > > > > > > > > not just treat any hot-plugged camera as a new one? I can't think of any
> > > > > > > > > benefit of preserving the same numerical ID between two plug events,
> > > > > > > > > while I can think of quiet a few cons.
> > > > > > > > >
> > > > > > > > > - It's confusing when debugging as un-plugging and then replugging the
> > > > > > > > > same camera will result in logs where the numerical ID is the same for
> > > > > > > > > both. This may even result in things working by "chance" is it reuses
> > > > > > > > > an already known numerical ID.
> > > > > > > > >
> > > > > > > > > - Looking at the code plugging a UVC camera in a different USB port will
> > > > > > > > > generate a different Camera::id() result and then be treated as a new
> > > > > > > > > camera vs plugging it in the same port and it then being treatad as a
> > > > > > > > > new camera.
> > > > > > > > >
> > > > > > > > > - The logic in this patch is more complex due to it both having to deal
> > > > > > > > > with known and new cameras.
> > > > > > > > >
> > > > > > > > > What is the benefit of this cache that I'm missing?
> > > > > > > >
> > > > > > > > It's not clear whether Android requires it. It requires different
> > > > > > > > cameras to have different IDs, but I couldn't find a mention of how
> > > > > > > > identical cameras should be treated. The Chrome OS UVC HAL caches the
> > > > > > > > ID, I don't know if it's mandatory though.
> > > > > > > >
> > > > > > > > Shik, as you've worked on the UVC HAL, would you happen to know if there
> > > > > > > > are requirements to keep the same ID when a camera is unplugged and
> > > > > > > > replugged ?
> > > > > >
> > > > > > It's for persistency in settings UI. Chrome uses the camera id as the key of
> > > > > > the default camera in chrome://settings/content/camera. Reuse the same id for
> > > > > > the same camera so the default camera would be changed back after replug.
> > > > > >
> > > > > > It's introduced in this CL: https://crrev.com/c/1608985
> > > > >
> > > > > Thank you for the information. We have a similar caching system, which
> > > > > however suffers from two limitations:
> >
> > We have more limitations I'm afraid :-(
> >
> > If I understand Shik correctly the numerical camera ID is stored
> > persistently in the camera settings UI. Our code does not guarantee that
> > the numerical ID in the libcamera HAL is persistent. The cameras in the
> > CameraManager is stored in a vector and is therefor susceptible to be
> > enumerated in different order each time the HAL is started.
> >
> > We could of course remedy this by sorting the camera vecotr before
> > assigning numerical IDs. But this would still not be guaranteed to be
> > persistent as more/less cameras registered in the HAL could alter the
> > read out order. If we really wish to do this should we save/load the
> > camera ID to numerical ID mapping to file (not nice)?
>
> Shik, does the ID cache need to be persistent across reboots (or
> restarts of the camera service) ?
For built-in cameras, yes. They need to be persistent across reboots and order
the camera ids by facing. There is a common implicit assumption in many
Android camera apps that assume the front camera is 0 and back camera is 1.
For external cameras, ideally yes. But for most of the use cases currently,
it's acceptable to use a solution that only persists the ids in a boot cycle,
and always assign the same id for the first external camera after boots.
>
> > > > > - If a camera is unplugged, and a different camera with the same VID:PID
> > > > > is plugged into the same USB port, the new camera be given the same ID
> > > > > as the previous camera.
> > > >
> > > > The current CrOS implementation will also use the same id in this case, and I
> > > > think it should be fine.
> > > >
> > > > > - If a camera is unplugged and replugged in a different USB port, it
> > > > > will be given a different ID.
> > > >
> > > > Hmm this looks non-ideal and may confuse the users.
> > >
> > > We will try to fix that on top
> > >
> > > This isn't trivial to handle, and there are corner cases. I'm thinking
> > > in particular about a system where two identical external USB cameras
> > > would be set up to point at particular locations. How to handle cameras
> > > stable IDs in that case is debatable, and I don't think the problem can
> > > be solved unless the cameras report distinct serial numbers (and most
> > > webcams don't :-S). At the moment, libcamera identifies each camera with
> > > a stable ID (guaranteed to be unique and stable across reboots, as long
> > > as the hardware and firmware are not modified) made of the concatenation
> > > of
> > >
> > > - The USB controller DT or ACPI path
> > > - The USB port(s) number(s) (starting at the root hub, going through
> > > every hub)
> > > - The USB device VID:PID
> > >
> > > The Android ID is then assigned based on the libcamera ID, with a cache
> > > to ensure that the same HAL ID will be given to the same libcamera ID.
> > > If we were to drop the USB port number, we'd have to find another way to
> > > create a stable and unique camera ID.
> > >
> > > > > Is that acceptable from a Chrome OS point of view ?
> > > > >
> > > > > Would you happen to know how Android uses camera IDs, and whether it
> > > > > requires or could benefit from ID caching ?
> > > >
> > > > Each camera app could have a similar preference caching system like the
> > > > settings UI, so it would be better to reuse the same id for the same camera.
> > > > AFAIK there is no such requirement at the framework level.
> >
> > This worries me a bit. Are we not altering the libcamera HAL to fit a
> > single use-case, all be it an important user. But if the framework does
> > not document an expected behavior is it not tricky to implement
> > correctly? What if we later find a different usage pattern in another
> > application which conflicts with what we are trying to do here?
>
> This is an undocumented area, so HALs can do pretty much what they want.
> If different platforms have different requirements, then we'll have to
> deal with it, possibly working with the platform vendors to align the
> requirements.
Yes and we need to live with Hyrum's Law :P
If it's not handled here, we would still need to have another id mapping layer
somewhere else.
>
> > > > > > > > > > CameraDevice is now a shared object and cameras_ vector stores shared
> > > > > > > > > > pointers to CameraDevice. This is done in order to introduce reference
> > > > > > > > > > counting for CameraDevice objects - especially to handle hot-unplug
> > > > > > > > > > events. Both camerasMap_ and cameras_ are protected by a mutex.
> > > > > > > > > >
> > > > > > > > > > Signed-off-by: Umang Jain <email at uajain.com>
> > > > > > > > > > ---
> > > > > > > > > > src/android/camera_device.cpp | 15 +++
> > > > > > > > > > src/android/camera_device.h | 8 +-
> > > > > > > > > > src/android/camera_hal_manager.cpp | 153 ++++++++++++++++++++++++-----
> > > > > > > > > > src/android/camera_hal_manager.h | 15 ++-
> > > > > > > > > > 4 files changed, 166 insertions(+), 25 deletions(-)
> > > > > > > > > >
> > > > > > > > > > diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp
> > > > > > > > > > index d918350..a79bb69 100644
> > > > > > > > > > --- a/src/android/camera_device.cpp
> > > > > > > > > > +++ b/src/android/camera_device.cpp
> > > > > > > > > > @@ -233,6 +233,21 @@ CameraDevice::~CameraDevice()
> > > > > > > > > > delete it.second;
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > +std::shared_ptr<CameraDevice> CameraDevice::create(unsigned int id,
> > > > > > > > > > + const std::shared_ptr<Camera> &cam)
> > > > > > > > > > +{
> > > > > > > > > > + struct Deleter : std::default_delete<CameraDevice> {
> > > > > > > > > > + void operator()(CameraDevice *camera)
> > > > > > > > > > + {
> > > > > > > > > > + delete camera;
> > > > > > > > > > + }
> > > > > > > > > > + };
> > > > > > > > > > +
> > > > > > > > > > + CameraDevice *camera = new CameraDevice(id, cam);
> > > > > > > > > > +
> > > > > > > > > > + return std::shared_ptr<CameraDevice>(camera, Deleter());
> > > > > > > > > > +}
> > > > > > > > >
> > > > > > > > > As Kieran points out I think this should be added in a separate as this
> > > > > > > > > one is quiet large and therefore hard to review.
> > > > > > > > >
> > > > > > > > > > +
> > > > > > > > > > /*
> > > > > > > > > > * Initialize the camera static information.
> > > > > > > > > > * This method is called before the camera device is opened.
> > > > > > > > > > diff --git a/src/android/camera_device.h b/src/android/camera_device.h
> > > > > > > > > > index 7be9e11..7f9e010 100644
> > > > > > > > > > --- a/src/android/camera_device.h
> > > > > > > > > > +++ b/src/android/camera_device.h
> > > > > > > > > > @@ -47,8 +47,8 @@ struct CameraStream {
> > > > > > > > > > class CameraDevice : protected libcamera::Loggable
> > > > > > > > > > {
> > > > > > > > > > public:
> > > > > > > > > > - CameraDevice(unsigned int id, const std::shared_ptr<libcamera::Camera> &camera);
> > > > > > > > > > - ~CameraDevice();
> > > > > > > > > > + static std::shared_ptr<CameraDevice> create(unsigned int id,
> > > > > > > > > > + const std::shared_ptr<libcamera::Camera> &cam);
> > > > > > > > > >
> > > > > > > > > > int initialize();
> > > > > > > > > >
> > > > > > > > > > @@ -57,6 +57,7 @@ public:
> > > > > > > > > >
> > > > > > > > > > unsigned int id() const { return id_; }
> > > > > > > > > > camera3_device_t *camera3Device() { return &camera3Device_; }
> > > > > > > > > > + const libcamera::Camera *getCamera() { return camera_.get(); };
> > > > > > > > >
> > > > > > > > > Same here this can be done in a separate commit. Also I think this could
> > > > > > > > > be named camera() instead of getCamera()
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > int facing() const { return facing_; }
> > > > > > > > > > int orientation() const { return orientation_; }
> > > > > > > > > > @@ -72,6 +73,9 @@ protected:
> > > > > > > > > > std::string logPrefix() const override;
> > > > > > > > > >
> > > > > > > > > > private:
> > > > > > > > > > + CameraDevice(unsigned int id, const std::shared_ptr<libcamera::Camera> &camera);
> > > > > > > > > > + ~CameraDevice();
> > > > > > > > > > +
> > > > > > > > > > struct Camera3RequestDescriptor {
> > > > > > > > > > Camera3RequestDescriptor(unsigned int frameNumber,
> > > > > > > > > > unsigned int numBuffers);
> > > > > > > > > > diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp
> > > > > > > > > > index 3d6d2b4..fdde2c0 100644
> > > > > > > > > > --- a/src/android/camera_hal_manager.cpp
> > > > > > > > > > +++ b/src/android/camera_hal_manager.cpp
> > > > > > > > > > @@ -8,6 +8,7 @@
> > > > > > > > > > #include "camera_hal_manager.h"
> > > > > > > > > >
> > > > > > > > > > #include <libcamera/camera.h>
> > > > > > > > > > +#include <libcamera/property_ids.h>
> > > > > > > > > >
> > > > > > > > > > #include "libcamera/internal/log.h"
> > > > > > > > > >
> > > > > > > > > > @@ -35,6 +36,7 @@ CameraHalManager::CameraHalManager()
> > > > > > > > > > CameraHalManager::~CameraHalManager()
> > > > > > > > > > {
> > > > > > > > > > cameras_.clear();
> > > > > > > > > > + camerasMap_.clear();
> > > > > > > > > >
> > > > > > > > > > if (cameraManager_) {
> > > > > > > > > > cameraManager_->stop();
> > > > > > > > > > @@ -47,6 +49,13 @@ int CameraHalManager::init()
> > > > > > > > > > {
> > > > > > > > > > cameraManager_ = new CameraManager();
> > > > > > > > > >
> > > > > > > > > > + /* Support camera hotplug. */
> > > > > > > > > > + cameraManager_->cameraAdded.connect(this, &CameraHalManager::cameraAdded);
> > > > > > > > > > + cameraManager_->cameraRemoved.connect(this, &CameraHalManager::cameraRemoved);
> > > > > > > > > > +
> > > > > > > > > > + cameraCounter_ = 0;
> > > > > > > > > > + externalCameraCounter_ = 1000;
> > > > > > > > > > +
> > > > > > > > > > int ret = cameraManager_->start();
> > > > > > > > > > if (ret) {
> > > > > > > > > > LOG(HAL, Error) << "Failed to start camera manager: "
> > > > > > > > > > @@ -56,35 +65,25 @@ int CameraHalManager::init()
> > > > > > > > > > return ret;
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > - /*
> > > > > > > > > > - * For each Camera registered in the system, a CameraDevice
> > > > > > > > > > - * gets created here to wraps a libcamera Camera instance.
> > > > > > > > > > - *
> > > > > > > > > > - * \todo Support camera hotplug.
> > > > > > > > > > - */
> > > > > > > > > > - unsigned int index = 0;
> > > > > > > > > > - for (auto &cam : cameraManager_->cameras()) {
> > > > > > > > > > - CameraDevice *camera = new CameraDevice(index, cam);
> > > > > > > > > > - ret = camera->initialize();
> > > > > > > > > > - if (ret)
> > > > > > > > > > - continue;
> > > > > > > > > > -
> > > > > > > > > > - cameras_.emplace_back(camera);
> > > > > > > > > > - ++index;
> > > > > > > > > > - }
> > > > > > > > > > -
> > > > > > > > > > return 0;
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > CameraDevice *CameraHalManager::open(unsigned int id,
> > > > > > > > > > const hw_module_t *hardwareModule)
> > > > > > > > > > {
> > > > > > > > > > - if (id >= numCameras()) {
> > > > > > > > > > + MutexLocker locker(mutex_);
> > > > > > > > > > +
> > > > > > > > > > + auto iter = std::find_if(cameras_.begin(), cameras_.end(),
> > > > > > > > > > + [id](std::shared_ptr<CameraDevice> &cam) {
> > > > > > > > > > + return cam->id() == id;
> > > > > > > > > > + });
> > > > > > > > > > + if (iter == cameras_.end()) {
> > > > > > > > >
> > > > > > > > > I think you should break this (and the similar ones below) into private
> > > > > > > > > helpers instead if implementing the logic in-place
> > > > > > > > >
> > > > > > > > > CameraHalManager::cameraFromAndroidId(..);
> > > > > > > > > CameraHalManager::androidIdFromCamera(..);
> > > > > > > > >
> > > > > > > > > > LOG(HAL, Error) << "Invalid camera id '" << id << "'";
> > > > > > > > > > return nullptr;
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > - CameraDevice *camera = cameras_[id].get();
> > > > > > > > > > + CameraDevice *camera = iter->get();
> > > > > > > > > > +
> > > > > > > > > > if (camera->open(hardwareModule))
> > > > > > > > > > return nullptr;
> > > > > > > > > >
> > > > > > > > > > @@ -93,9 +92,91 @@ CameraDevice *CameraHalManager::open(unsigned int id,
> > > > > > > > > > return camera;
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > +void CameraHalManager::cameraAdded(std::shared_ptr<Camera> cam)
> > > > > > > > > > +{
> > > > > > > > > > + unsigned int id;
> > > > > > > > > > + bool isCameraExternal = false;
> > > > > > > > > > + bool isCameraNew = false;
> > > > > > > > > > +
> > > > > > > > > > + MutexLocker locker(mutex_);
> > > > > > > > > > +
> > > > > > > > > > + /* Each camera is assigned a unique integer id when it is seen for the
> > > > > > > > > > + * first time. If the camera has been seen before, the id is reused and
> > > > > > > > > > + * the camera is marked as CAMERA_DEVICE_STATUS_PRESENT subsequently.
> > > > > > > > > > + *
> > > > > > > > > > + * ID starts from '0' for internal cameras and '1000' for external cameras.
> > > > > > > > > > + */
> > > > > > > > > > + auto iter = camerasMap_.find(cam->id());
> > > > > > > > > > + if (iter != camerasMap_.end()) {
> > > > > > > > > > + id = iter->second;
> > > > > > > > > > + } else {
> > > > > > > > > > + isCameraNew = true;
> > > > > > > > > > +
> > > > > > > > > > + /*
> > > > > > > > > > + * Now check if this is an external camera and assign
> > > > > > > > > > + * its id accordingly.
> > > > > > > > > > + */
> > > > > > > > > > + const ControlList &properties = cam->properties();
> > > > > > > > > > + if (properties.contains(properties::Location) &&
> > > > > > > > > > + properties.get(properties::Location) &
> > > > > > > > > > + properties::CameraLocationExternal) {
> > > > > > > > > > + isCameraExternal = true;
> > > > > > > > > > + id = externalCameraCounter_;
> > > > > > > > > > + } else {
> > > > > > > > > > + id = cameraCounter_;
> > > > > > > > > > + }
> > > > > > > > >
> > > > > > > > > As I understand it the information the HAL wants is whether or not the
> > > > > > > > > camera can be hot-plugged (id >= 1000) or nor (id < 1000)?
> > > > > > > > >
> > > > > > > > > If so I wonder if using the camera location as a judgment of the camera
> > > > > > > > > is hot-plugged or not is the way to go here? Imagine a device where the
> > > > > > > > > camera is permanently attached (not hot-unpluggable) but not fixated in
> > > > > > > > > a location. I'm thinking cameras mounted at the end of instruments
> > > > > > > > > (medical instruments, hand held tools) or robotics (mounted at the arm
> > > > > > > > > of a welding robot). I would imagine those cameras would be marked as
> > > > > > > > > located externally but they would not really be hot-pluggable.
> > > > > > > > >
> > > > > > > > > I understand we have no other way to report or detect this at the moment
> > > > > > > > > and I'm not pushing hard for this to be solved as part of this series if
> > > > > > > > > it's not easy. But I think a bigger comment here is needed explaining
> > > > > > > > > that the HAL wants to know if a camera is hot-pluggable or not and does
> > > > > > > > > not really care if it's located internally or externally. I also think a
> > > > > > > > > \todo should be added so it's not forgotten.
> > > > > > > > >
> > > > > > > > > > + }
> > > > > > > > > > +
> > > > > > > > > > + /*
> > > > > > > > > > + * For each Camera registered in the system, a CameraDevice
> > > > > > > > > > + * gets created here to wraps a libcamera Camera instance.
> > > > > > > > > > + */
> > > > > > > > > > + std::shared_ptr<CameraDevice> camera = CameraDevice::create(id, std::move(cam));
> > > > > > > > > > + int ret = camera->initialize();
> > > > > > > > > > + if (ret) {
> > > > > > > > > > + LOG(HAL, Error) << "Failed to initialize camera: " << cam->id();
> > > > > > > > > > + return;
> > > > > > > > > > + }
> > > > > > > > > > +
> > > > > > > > > > + if (isCameraNew) {
> > > > > > > > > > + camerasMap_.emplace(cam->id(), id);
> > > > > > > > > > +
> > > > > > > > > > + if (isCameraExternal)
> > > > > > > > > > + externalCameraCounter_++;
> > > > > > > > > > + else
> > > > > > > > > > + cameraCounter_++;
> > > > > > > > > > + }
> > > > > > > > > > +
> > > > > > > > > > + cameras_.emplace_back(std::move(camera));
> > > > > > > > > > +
> > > > > > > > > > + if (callbacks_)
> > > > > > > > > > + callbacks_->camera_device_status_change(callbacks_, id,
> > > > > > > > > > + CAMERA_DEVICE_STATUS_PRESENT);
> > > > > > > > > > + LOG(HAL, Debug) << "Camera ID: " << id << " added successfully.";
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > +void CameraHalManager::cameraRemoved(std::shared_ptr<Camera> cam)
> > > > > > > > > > +{
> > > > > > > > > > + MutexLocker locker(mutex_);
> > > > > > > > > > +
> > > > > > > > > > + auto iter = std::find_if(cameras_.begin(), cameras_.end(),
> > > > > > > > > > + [cam](std::shared_ptr<CameraDevice> &camera) {
> > > > > > > > > > + return cam.get() == camera->getCamera();
> > > > > > > > > > + });
> > > > > > > > > > + if (iter == cameras_.end())
> > > > > > > > > > + return;
> > > > > > > > > > +
> > > > > > > > > > + unsigned int id = (*iter)->id();
> > > > > > > > > > + callbacks_->camera_device_status_change(callbacks_, id,
> > > > > > > > > > + CAMERA_DEVICE_STATUS_NOT_PRESENT);
> > > > > > > > > > + cameras_.erase(iter);
> > > > > > > > > > +
> > > > > > > > > > + LOG(HAL, Debug) << "Camera ID: " << id << " removed successfully.";
> > > > > > > > > > +}
> > > > > > > > > > +
> > > > > > > > > > unsigned int CameraHalManager::numCameras() const
> > > > > > > > > > {
> > > > > > > > > > - return cameraManager_->cameras().size();
> > > > > > > > > > + return cameraCounter_;
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info)
> > > > > > > > > > @@ -103,12 +184,18 @@ int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info)
> > > > > > > > > > if (!info)
> > > > > > > > > > return -EINVAL;
> > > > > > > > > >
> > > > > > > > > > - if (id >= numCameras()) {
> > > > > > > > > > + MutexLocker locker(mutex_);
> > > > > > > > > > +
> > > > > > > > > > + auto iter = std::find_if(cameras_.begin(), cameras_.end(),
> > > > > > > > > > + [id](std::shared_ptr<CameraDevice> &cam) {
> > > > > > > > > > + return cam->id() == id;
> > > > > > > > > > + });
> > > > > > > > > > + if (iter == cameras_.end()) {
> > > > > > > > > > LOG(HAL, Error) << "Invalid camera id '" << id << "'";
> > > > > > > > > > return -EINVAL;
> > > > > > > > > > }
> > > > > > > > > >
> > > > > > > > > > - CameraDevice *camera = cameras_[id].get();
> > > > > > > > > > + CameraDevice *camera = iter->get();
> > > > > > > > > >
> > > > > > > > > > info->facing = camera->facing();
> > > > > > > > > > info->orientation = camera->orientation();
> > > > > > > > > > @@ -124,4 +211,26 @@ int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info)
> > > > > > > > > > void CameraHalManager::setCallbacks(const camera_module_callbacks_t *callbacks)
> > > > > > > > > > {
> > > > > > > > > > callbacks_ = callbacks;
> > > > > > > > > > +
> > > > > > > > > > + MutexLocker locker(mutex_);
> > > > > > > > > > + /*
> > > > > > > > > > + * Few cameras might have been hotplugged before setting callbacks_ here.
> > > > > > > > > > + * We need to mark CAMERA_DEVICE_STATUS_PRESENT for them explicitly.
> > > > > > > > > > + * This hold only for external cameras, as internal cameras are assumed to
> > > > > > > > > > + * be present at module load time, by the framework.
> > > > > > > > > > + */
> > > > > > > > > > + for (auto &cam : cameraManager_->cameras()) {
> > > > > > > > > > + auto iter = camerasMap_.find(cam->id());
> > > > > > > > > > + if (iter == camerasMap_.end())
> > > > > > > > > > + continue;
> > > > > > > > > > +
> > > > > > > > > > + unsigned int id = iter->second;
> > > > > > > > > > + const ControlList &properties = cam->properties();
> > > > > > > > > > + if (properties.contains(properties::Location) &&
> > > > > > > > > > + properties.get(properties::Location) &
> > > > > > > > > > + properties::CameraLocationExternal) {
> > > > > > > > > > + callbacks_->camera_device_status_change(callbacks_, id,
> > > > > > > > > > + CAMERA_DEVICE_STATUS_PRESENT);
> > > > > > > > > > + }
> > > > > > > > > > + }
> > > > > > > > > > }
> > > > > > > > > > diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h
> > > > > > > > > > index a582f04..7c481d4 100644
> > > > > > > > > > --- a/src/android/camera_hal_manager.h
> > > > > > > > > > +++ b/src/android/camera_hal_manager.h
> > > > > > > > > > @@ -7,6 +7,8 @@
> > > > > > > > > > #ifndef __ANDROID_CAMERA_MANAGER_H__
> > > > > > > > > > #define __ANDROID_CAMERA_MANAGER_H__
> > > > > > > > > >
> > > > > > > > > > +#include <map>
> > > > > > > > > > +#include <mutex>
> > > > > > > > > > #include <stddef.h>
> > > > > > > > > > #include <vector>
> > > > > > > > > >
> > > > > > > > > > @@ -18,6 +20,9 @@
> > > > > > > > > >
> > > > > > > > > > class CameraDevice;
> > > > > > > > > >
> > > > > > > > > > +using Mutex = std::mutex;
> > > > > > > > > > +using MutexLocker = std::unique_lock<std::mutex>;
> > > > > > > > > > +
> > > > > > > > > > class CameraHalManager
> > > > > > > > > > {
> > > > > > > > > > public:
> > > > > > > > > > @@ -33,10 +38,18 @@ public:
> > > > > > > > > > void setCallbacks(const camera_module_callbacks_t *callbacks);
> > > > > > > > > >
> > > > > > > > > > private:
> > > > > > > > > > + void cameraAdded(std::shared_ptr<libcamera::Camera> cam);
> > > > > > > > > > + void cameraRemoved(std::shared_ptr<libcamera::Camera> cam);
> > > > > > > > > > +
> > > > > > > > > > libcamera::CameraManager *cameraManager_;
> > > > > > > > > >
> > > > > > > > > > const camera_module_callbacks_t *callbacks_;
> > > > > > > > > > - std::vector<std::unique_ptr<CameraDevice>> cameras_;
> > > > > > > > > > + std::vector<std::shared_ptr<CameraDevice>> cameras_;
> > > > > > > > > > + std::map<std::string, unsigned int> camerasMap_;
> > > > > > > > >
> > > > > > > > > If each hot-plugged camera where treated as a new camera cameras_ and
> > > > > > > > > camerasMap_ could be merged to a
> > > > > > > > >
> > > > > > > > > std::map<unsigned int, std::shared_ptr<CameraDevice>> cameras_;
> > > > > > > > >
> > > > > > > > > Which would eliminate the possibility of them going out-of-sync.
> > > > > > > > >
> > > > > > > > > > + Mutex mutex_;
> > > > > > > > > > +
> > > > > > > > > > + unsigned int externalCameraCounter_;
> > > > > > > > > > + unsigned int cameraCounter_;
> > > > > > > > > > };
> > > > > > > > > >
> > > > > > > > > > #endif /* __ANDROID_CAMERA_MANAGER_H__ */
>
> --
> Regards,
>
> Laurent Pinchart
- shik
More information about the libcamera-devel
mailing list