From da3f50ee9cdb6896b365357b0d35577344f72ba4 Mon Sep 17 00:00:00 2001 From: Laurent Pinchart Date: Mon, 20 Jan 2020 01:09:34 +0200 Subject: android: Remove internal thread Now that libcamera creates threads internally and doesn't rely on an application-provided event loop, remove the thread from the Android Camera HAL layer. The CameraProxy class becomes meaningless, remove it and communicate directly from the CameraHalManager to the CameraDevice. Signed-off-by: Laurent Pinchart Acked-by: Jacopo Mondi --- src/android/camera3_hal.cpp | 8 +- src/android/camera_device.cpp | 48 ++++++---- src/android/camera_device.h | 16 ++-- src/android/camera_hal_manager.cpp | 78 ++++++---------- src/android/camera_hal_manager.h | 17 ++-- src/android/camera_ops.cpp | 96 ++++++++++++++++++++ src/android/camera_ops.h | 15 ++++ src/android/camera_proxy.cpp | 180 ------------------------------------- src/android/camera_proxy.h | 42 --------- src/android/meson.build | 2 +- 10 files changed, 188 insertions(+), 314 deletions(-) create mode 100644 src/android/camera_ops.cpp create mode 100644 src/android/camera_ops.h delete mode 100644 src/android/camera_proxy.cpp delete mode 100644 src/android/camera_proxy.h diff --git a/src/android/camera3_hal.cpp b/src/android/camera3_hal.cpp index 8d2629ca..d6fc1ecc 100644 --- a/src/android/camera3_hal.cpp +++ b/src/android/camera3_hal.cpp @@ -7,8 +7,8 @@ #include +#include "camera_device.h" #include "camera_hal_manager.h" -#include "camera_proxy.h" #include "log.h" using namespace libcamera; @@ -71,14 +71,14 @@ static int hal_dev_open(const hw_module_t *module, const char *name, LOG(HAL, Debug) << "Open camera " << name; int id = atoi(name); - CameraProxy *proxy = cameraManager.open(id, module); - if (!proxy) { + CameraDevice *camera = cameraManager.open(id, module); + if (!camera) { LOG(HAL, Error) << "Failed to open camera module '" << id << "'"; return -ENODEV; } - *device = &proxy->camera3Device()->common; + *device = &camera->camera3Device()->common; return 0; } diff --git a/src/android/camera_device.cpp b/src/android/camera_device.cpp index 67c1d47e..b460d499 100644 --- a/src/android/camera_device.cpp +++ b/src/android/camera_device.cpp @@ -6,6 +6,7 @@ */ #include "camera_device.h" +#include "camera_ops.h" #include "log.h" #include "utils.h" @@ -39,13 +40,13 @@ CameraDevice::Camera3RequestDescriptor::~Camera3RequestDescriptor() * \class CameraDevice * * The CameraDevice class wraps a libcamera::Camera instance, and implements - * the camera_device_t interface by handling RPC requests received from its - * associated CameraProxy. + * the camera3_device_t interface, bridging calls received from the Android + * camera service to the CameraDevice. * - * It translate parameters and operations from Camera HALv3 API to the libcamera - * ones to provide static information for a Camera, create request templates - * for it, process capture requests and then deliver capture results back - * to the framework using the designated callbacks. + * The class translates parameters and operations from the Camera HALv3 API to + * the libcamera API to provide static information for a Camera, create request + * templates for it, process capture requests and then deliver capture results + * back to the framework using the designated callbacks. */ CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr &camera) @@ -63,7 +64,7 @@ CameraDevice::~CameraDevice() delete it.second; } -int CameraDevice::open() +int CameraDevice::open(const hw_module_t *hardwareModule) { int ret = camera_->acquire(); if (ret) { @@ -71,6 +72,19 @@ int CameraDevice::open() return ret; } + /* Initialize the hw_device_t in the instance camera3_module_t. */ + camera3Device_.common.tag = HARDWARE_DEVICE_TAG; + camera3Device_.common.version = CAMERA_DEVICE_API_VERSION_3_3; + camera3Device_.common.module = (hw_module_t *)hardwareModule; + camera3Device_.common.close = hal_dev_close; + + /* + * The camera device operations. These actually implement + * the Android Camera HALv3 interface. + */ + camera3Device_.ops = &hal_dev_ops; + camera3Device_.priv = this; + return 0; } @@ -90,7 +104,7 @@ void CameraDevice::setCallbacks(const camera3_callback_ops_t *callbacks) /* * Return static information for the camera. */ -camera_metadata_t *CameraDevice::getStaticMetadata() +const camera_metadata_t *CameraDevice::getStaticMetadata() { if (staticMetadata_) return staticMetadata_->get(); @@ -675,7 +689,7 @@ int CameraDevice::configureStreams(camera3_stream_configuration_t *stream_list) return 0; } -void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request) +int CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Request) { StreamConfiguration *streamConfiguration = &config_->at(0); Stream *stream = streamConfiguration->stream(); @@ -683,7 +697,7 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque if (camera3Request->num_output_buffers != 1) { LOG(HAL, Error) << "Invalid number of output buffers: " << camera3Request->num_output_buffers; - return; + return -EINVAL; } /* Start the camera if that's the first request we handle. */ @@ -691,7 +705,7 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque int ret = camera_->start(); if (ret) { LOG(HAL, Error) << "Failed to start camera"; - return; + return ret; } running_ = true; @@ -747,7 +761,7 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque if (!buffer) { LOG(HAL, Error) << "Failed to create buffer"; delete descriptor; - return; + return -ENOMEM; } Request *request = @@ -757,14 +771,12 @@ void CameraDevice::processCaptureRequest(camera3_capture_request_t *camera3Reque int ret = camera_->queueRequest(request); if (ret) { LOG(HAL, Error) << "Failed to queue request"; - goto error; + delete request; + delete descriptor; + return ret; } - return; - -error: - delete request; - delete descriptor; + return 0; } void CameraDevice::requestComplete(Request *request) diff --git a/src/android/camera_device.h b/src/android/camera_device.h index caa617dc..55eac317 100644 --- a/src/android/camera_device.h +++ b/src/android/camera_device.h @@ -13,7 +13,6 @@ #include #include -#include #include #include @@ -21,19 +20,23 @@ class CameraMetadata; -class CameraDevice : public libcamera::Object +class CameraDevice { public: CameraDevice(unsigned int id, const std::shared_ptr &camera); ~CameraDevice(); - int open(); + int open(const hw_module_t *hardwareModule); void close(); + + unsigned int id() const { return id_; } + camera3_device_t *camera3Device() { return &camera3Device_; } + void setCallbacks(const camera3_callback_ops_t *callbacks); - camera_metadata_t *getStaticMetadata(); + const camera_metadata_t *getStaticMetadata(); const camera_metadata_t *constructDefaultRequestSettings(int type); int configureStreams(camera3_stream_configuration_t *stream_list); - void processCaptureRequest(camera3_capture_request_t *request); + int processCaptureRequest(camera3_capture_request_t *request); void requestComplete(libcamera::Request *request); private: @@ -52,6 +55,9 @@ private: std::unique_ptr getResultMetadata(int frame_number, int64_t timestamp); + unsigned int id_; + camera3_device_t camera3Device_; + bool running_; std::shared_ptr camera_; std::unique_ptr config_; diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp index 22f0323b..5bd3bdba 100644 --- a/src/android/camera_hal_manager.cpp +++ b/src/android/camera_hal_manager.cpp @@ -12,7 +12,6 @@ #include "log.h" #include "camera_device.h" -#include "camera_proxy.h" using namespace libcamera; @@ -28,92 +27,67 @@ LOG_DECLARE_CATEGORY(HAL); * their static information and to open camera devices. */ -CameraHalManager::~CameraHalManager() +CameraHalManager::CameraHalManager() + : cameraManager_(nullptr) { - if (isRunning()) { - exit(0); - /* \todo Wait with a timeout, just in case. */ - wait(); - } } -int CameraHalManager::init() +CameraHalManager::~CameraHalManager() { - /* - * Start the camera HAL manager thread and wait until its - * initialisation completes to be fully operational before - * receiving calls from the camera stack. - */ - start(); - - MutexLocker locker(mutex_); - cv_.wait(locker); + cameras_.clear(); - return 0; + if (cameraManager_) { + cameraManager_->stop(); + delete cameraManager_; + cameraManager_ = nullptr; + } } -void CameraHalManager::run() +int CameraHalManager::init() { - /* - * All the libcamera components must be initialised here, in - * order to bind them to the camera HAL manager thread that - * executes the event dispatcher. - */ cameraManager_ = new CameraManager(); int ret = cameraManager_->start(); if (ret) { LOG(HAL, Error) << "Failed to start camera manager: " << strerror(-ret); - return; + delete cameraManager_; + cameraManager_ = nullptr; + return ret; } /* - * For each Camera registered in the system, a CameraProxy - * gets created here to wraps a camera device. + * 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 &camera : cameraManager_->cameras()) { - CameraProxy *proxy = new CameraProxy(index, camera); - proxies_.emplace_back(proxy); + for (auto &cam : cameraManager_->cameras()) { + CameraDevice *camera = new CameraDevice(index, cam); + cameras_.emplace_back(camera); ++index; } - /* - * libcamera has been initialized. Unlock the init() caller - * as we're now ready to handle calls from the framework. - */ - cv_.notify_one(); - - /* Now start processing events and messages. */ - exec(); - - /* Clean up the resources we have allocated. */ - proxies_.clear(); - - cameraManager_->stop(); - delete cameraManager_; - cameraManager_ = nullptr; + return 0; } -CameraProxy *CameraHalManager::open(unsigned int id, - const hw_module_t *hardwareModule) +CameraDevice *CameraHalManager::open(unsigned int id, + const hw_module_t *hardwareModule) { if (id >= numCameras()) { LOG(HAL, Error) << "Invalid camera id '" << id << "'"; return nullptr; } - CameraProxy *proxy = proxies_[id].get(); - if (proxy->open(hardwareModule)) + CameraDevice *camera = cameras_[id].get(); + if (camera->open(hardwareModule)) return nullptr; LOG(HAL, Info) << "Open camera '" << id << "'"; - return proxy; + return camera; } unsigned int CameraHalManager::numCameras() const @@ -131,14 +105,14 @@ int CameraHalManager::getCameraInfo(unsigned int id, struct camera_info *info) return -EINVAL; } - CameraProxy *proxy = proxies_[id].get(); + CameraDevice *camera = cameras_[id].get(); /* \todo Get these info dynamically inspecting the camera module. */ info->facing = id ? CAMERA_FACING_FRONT : CAMERA_FACING_BACK; info->orientation = 0; info->device_version = 0; info->resource_cost = 0; - info->static_camera_characteristics = proxy->getStaticMetadata(); + info->static_camera_characteristics = camera->getStaticMetadata(); info->conflicting_devices = nullptr; info->conflicting_devices_length = 0; diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h index c23abd1c..94d8f005 100644 --- a/src/android/camera_hal_manager.h +++ b/src/android/camera_hal_manager.h @@ -7,8 +7,7 @@ #ifndef __ANDROID_CAMERA_MANAGER_H__ #define __ANDROID_CAMERA_MANAGER_H__ -#include -#include +#include #include #include @@ -16,33 +15,27 @@ #include -#include "thread.h" - class CameraDevice; -class CameraProxy; -class CameraHalManager : public libcamera::Thread +class CameraHalManager { public: + CameraHalManager(); ~CameraHalManager(); int init(); - CameraProxy *open(unsigned int id, const hw_module_t *module); + CameraDevice *open(unsigned int id, const hw_module_t *module); unsigned int numCameras() const; int getCameraInfo(unsigned int id, struct camera_info *info); private: - void run() override; camera_metadata_t *getStaticMetadata(unsigned int id); libcamera::CameraManager *cameraManager_; - std::mutex mutex_; - std::condition_variable cv_; - - std::vector> proxies_; + std::vector> cameras_; }; #endif /* __ANDROID_CAMERA_MANAGER_H__ */ diff --git a/src/android/camera_ops.cpp b/src/android/camera_ops.cpp new file mode 100644 index 00000000..9dfc2e65 --- /dev/null +++ b/src/android/camera_ops.cpp @@ -0,0 +1,96 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * camera_ops.h - Android Camera HAL Operations + */ + +#include "camera_ops.h" + +#include + +#include "camera_device.h" + +using namespace libcamera; + +/* + * Translatation layer between the Android Camera HAL device operations and the + * CameraDevice. + */ + +static int hal_dev_initialize(const struct camera3_device *dev, + const camera3_callback_ops_t *callback_ops) +{ + if (!dev) + return -EINVAL; + + CameraDevice *camera = reinterpret_cast(dev->priv); + camera->setCallbacks(callback_ops); + + return 0; +} + +static int hal_dev_configure_streams(const struct camera3_device *dev, + camera3_stream_configuration_t *stream_list) +{ + if (!dev) + return -EINVAL; + + CameraDevice *camera = reinterpret_cast(dev->priv); + return camera->configureStreams(stream_list); +} + +static const camera_metadata_t * +hal_dev_construct_default_request_settings(const struct camera3_device *dev, + int type) +{ + if (!dev) + return nullptr; + + CameraDevice *camera = reinterpret_cast(dev->priv); + return camera->constructDefaultRequestSettings(type); +} + +static int hal_dev_process_capture_request(const struct camera3_device *dev, + camera3_capture_request_t *request) +{ + if (!dev) + return -EINVAL; + + CameraDevice *camera = reinterpret_cast(dev->priv); + return camera->processCaptureRequest(request); +} + +static void hal_dev_dump(const struct camera3_device *dev, int fd) +{ +} + +static int hal_dev_flush(const struct camera3_device *dev) +{ + return 0; +} + +int hal_dev_close(hw_device_t *hw_device) +{ + if (!hw_device) + return -EINVAL; + + camera3_device_t *dev = reinterpret_cast(hw_device); + CameraDevice *camera = reinterpret_cast(dev->priv); + + camera->close(); + + return 0; +} + +camera3_device_ops hal_dev_ops = { + .initialize = hal_dev_initialize, + .configure_streams = hal_dev_configure_streams, + .register_stream_buffers = nullptr, + .construct_default_request_settings = hal_dev_construct_default_request_settings, + .process_capture_request = hal_dev_process_capture_request, + .get_metadata_vendor_tag_ops = nullptr, + .dump = hal_dev_dump, + .flush = hal_dev_flush, + .reserved = { nullptr }, +}; diff --git a/src/android/camera_ops.h b/src/android/camera_ops.h new file mode 100644 index 00000000..304e7b85 --- /dev/null +++ b/src/android/camera_ops.h @@ -0,0 +1,15 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * camera_ops.h - Android Camera HAL Operations + */ +#ifndef __ANDROID_CAMERA_OPS_H__ +#define __ANDROID_CAMERA_OPS_H__ + +#include + +int hal_dev_close(hw_device_t *hw_device); +extern camera3_device_ops hal_dev_ops; + +#endif /* __ANDROID_CAMERA_OPS_H__ */ diff --git a/src/android/camera_proxy.cpp b/src/android/camera_proxy.cpp deleted file mode 100644 index 3964b566..00000000 --- a/src/android/camera_proxy.cpp +++ /dev/null @@ -1,180 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2019, Google Inc. - * - * camera_proxy.cpp - Proxy to camera devices - */ - -#include "camera_proxy.h" - -#include - -#include - -#include "log.h" -#include "message.h" -#include "utils.h" - -#include "camera_device.h" - -using namespace libcamera; - -LOG_DECLARE_CATEGORY(HAL); - -/* - * \class CameraProxy - * - * The CameraProxy wraps a CameraDevice and implements the camera3_device_t - * API, bridging calls received from the camera framework to the CameraDevice. - * - * Bridging operation calls between the framework and the CameraDevice is - * required as the two run in two different threads and certain operations, - * such as queueing a new capture request to the camera, shall be called in - * the thread that dispatches events. Other operations do not require any - * bridging and resolve to direct function calls on the CameraDevice instance - * instead. - */ - -static int hal_dev_initialize(const struct camera3_device *dev, - const camera3_callback_ops_t *callback_ops) -{ - if (!dev) - return -EINVAL; - - CameraProxy *proxy = reinterpret_cast(dev->priv); - proxy->initialize(callback_ops); - - return 0; -} - -static int hal_dev_configure_streams(const struct camera3_device *dev, - camera3_stream_configuration_t *stream_list) -{ - if (!dev) - return -EINVAL; - - CameraProxy *proxy = reinterpret_cast(dev->priv); - return proxy->configureStreams(stream_list); -} - -static const camera_metadata_t * -hal_dev_construct_default_request_settings(const struct camera3_device *dev, - int type) -{ - if (!dev) - return nullptr; - - CameraProxy *proxy = reinterpret_cast(dev->priv); - return proxy->constructDefaultRequestSettings(type); -} - -static int hal_dev_process_capture_request(const struct camera3_device *dev, - camera3_capture_request_t *request) -{ - if (!dev) - return -EINVAL; - - CameraProxy *proxy = reinterpret_cast(dev->priv); - return proxy->processCaptureRequest(request); -} - -static void hal_dev_dump(const struct camera3_device *dev, int fd) -{ -} - -static int hal_dev_flush(const struct camera3_device *dev) -{ - return 0; -} - -static int hal_dev_close(hw_device_t *hw_device) -{ - if (!hw_device) - return -EINVAL; - - camera3_device_t *dev = reinterpret_cast(hw_device); - CameraProxy *proxy = reinterpret_cast(dev->priv); - - proxy->close(); - - return 0; -} - -static camera3_device_ops hal_dev_ops = { - .initialize = hal_dev_initialize, - .configure_streams = hal_dev_configure_streams, - .register_stream_buffers = nullptr, - .construct_default_request_settings = hal_dev_construct_default_request_settings, - .process_capture_request = hal_dev_process_capture_request, - .get_metadata_vendor_tag_ops = nullptr, - .dump = hal_dev_dump, - .flush = hal_dev_flush, - .reserved = { nullptr }, -}; - -CameraProxy::CameraProxy(unsigned int id, const std::shared_ptr &camera) - : id_(id) -{ - cameraDevice_ = new CameraDevice(id, camera); -} - -CameraProxy::~CameraProxy() -{ - delete cameraDevice_; -} - -int CameraProxy::open(const hw_module_t *hardwareModule) -{ - int ret = cameraDevice_->open(); - if (ret) - return ret; - - /* Initialize the hw_device_t in the instance camera3_module_t. */ - camera3Device_.common.tag = HARDWARE_DEVICE_TAG; - camera3Device_.common.version = CAMERA_DEVICE_API_VERSION_3_3; - camera3Device_.common.module = (hw_module_t *)hardwareModule; - camera3Device_.common.close = hal_dev_close; - - /* - * The camera device operations. These actually implement - * the Android Camera HALv3 interface. - */ - camera3Device_.ops = &hal_dev_ops; - camera3Device_.priv = this; - - return 0; -} - -void CameraProxy::close() -{ - cameraDevice_->invokeMethod(&CameraDevice::close, - ConnectionTypeBlocking); -} - -void CameraProxy::initialize(const camera3_callback_ops_t *callbacks) -{ - cameraDevice_->setCallbacks(callbacks); -} - -const camera_metadata_t *CameraProxy::getStaticMetadata() -{ - return cameraDevice_->getStaticMetadata(); -} - -const camera_metadata_t *CameraProxy::constructDefaultRequestSettings(int type) -{ - return cameraDevice_->constructDefaultRequestSettings(type); -} - -int CameraProxy::configureStreams(camera3_stream_configuration_t *stream_list) -{ - return cameraDevice_->configureStreams(stream_list); -} - -int CameraProxy::processCaptureRequest(camera3_capture_request_t *request) -{ - cameraDevice_->invokeMethod(&CameraDevice::processCaptureRequest, - ConnectionTypeBlocking, request); - - return 0; -} diff --git a/src/android/camera_proxy.h b/src/android/camera_proxy.h deleted file mode 100644 index e8cfbc9d..00000000 --- a/src/android/camera_proxy.h +++ /dev/null @@ -1,42 +0,0 @@ -/* SPDX-License-Identifier: LGPL-2.1-or-later */ -/* - * Copyright (C) 2019, Google Inc. - * - * camera_proxy.h - Proxy to camera devices - */ -#ifndef __ANDROID_CAMERA_PROXY_H__ -#define __ANDROID_CAMERA_PROXY_H__ - -#include - -#include - -#include - -class CameraDevice; - -class CameraProxy -{ -public: - CameraProxy(unsigned int id, const std::shared_ptr &camera); - ~CameraProxy(); - - int open(const hw_module_t *hardwareModule); - void close(); - - void initialize(const camera3_callback_ops_t *callbacks); - const camera_metadata_t *getStaticMetadata(); - const camera_metadata_t *constructDefaultRequestSettings(int type); - int configureStreams(camera3_stream_configuration_t *stream_list); - int processCaptureRequest(camera3_capture_request_t *request); - - unsigned int id() const { return id_; } - camera3_device_t *camera3Device() { return &camera3Device_; } - -private: - unsigned int id_; - CameraDevice *cameraDevice_; - camera3_device_t camera3Device_; -}; - -#endif /* __ANDROID_CAMERA_PROXY_H__ */ diff --git a/src/android/meson.build b/src/android/meson.build index 70dfcc1d..5a5a332e 100644 --- a/src/android/meson.build +++ b/src/android/meson.build @@ -3,7 +3,7 @@ android_hal_sources = files([ 'camera_hal_manager.cpp', 'camera_device.cpp', 'camera_metadata.cpp', - 'camera_proxy.cpp', + 'camera_ops.cpp', ]) android_camera_metadata_sources = files([ -- cgit v1.2.1