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 <laurent.pinchart@ideasonboard.com>
Acked-by: Jacopo Mondi <jacopo@jmondi.org>

src/android/camera3_hal.cpp

@@ -7,8 +7,8 @@
 
 #include <hardware/camera_common.h>
 
+#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);
+	CameraDevice *camera = cameraManager.open(id, module);
-	if (!proxy) {
+	if (!camera) {
 		LOG(HAL, Error)
 			<< "Failed to open camera module '" << id << "'";
 		return -ENODEV;
 	}
 
-	*device = &proxy->camera3Device()->common;
+	*device = &camera->camera3Device()->common;
 
 	return 0;
 }

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
+ * the camera3_device_t interface, bridging calls received from the Android
- * associated CameraProxy.
+ * camera service to the CameraDevice.
  *
- * It translate parameters and operations from Camera HALv3 API to the libcamera
+ * The class translates parameters and operations from the Camera HALv3 API to
- * ones to provide static information for a Camera, create request templates
+ * the libcamera API to provide static information for a Camera, create request
- * for it, process capture requests and then deliver capture results back
+ * templates for it, process capture requests and then deliver capture results
- * to the framework using the designated callbacks.
+ * back to the framework using the designated callbacks.
  */
 
 CameraDevice::CameraDevice(unsigned int id, const std::shared_ptr<Camera> &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;
-	}
-
-	return;
-
-error:
 		delete request;
 		delete descriptor;
+		return ret;
+	}
+
+	return 0;
 }
 
 void CameraDevice::requestComplete(Request *request)

src/android/camera_device.h

@@ -13,7 +13,6 @@
 
 #include <libcamera/buffer.h>
 #include <libcamera/camera.h>
-#include <libcamera/object.h>
 #include <libcamera/request.h>
 #include <libcamera/stream.h>
 
@@ -21,19 +20,23 @@
 
 class CameraMetadata;
 
-class CameraDevice : public libcamera::Object
+class CameraDevice
 {
 public:
 	CameraDevice(unsigned int id, const std::shared_ptr<libcamera::Camera> &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<CameraMetadata> getResultMetadata(int frame_number,
 							  int64_t timestamp);
 
+	unsigned int id_;
+	camera3_device_t camera3Device_;
+
 	bool running_;
 	std::shared_ptr<libcamera::Camera> camera_;
 	std::unique_ptr<libcamera::CameraConfiguration> config_;

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,78 +27,53 @@ LOG_DECLARE_CATEGORY(HAL);
  * their static information and to open camera devices.
  */
 
+CameraHalManager::CameraHalManager()
+	: cameraManager_(nullptr)
+{
+}
+
 CameraHalManager::~CameraHalManager()
 {
-	if (isRunning()) {
+	cameras_.clear();
-		exit(0);
+
-		/* \todo Wait with a timeout, just in case. */
+	if (cameraManager_) {
-		wait();
+		cameraManager_->stop();
+		delete cameraManager_;
+		cameraManager_ = nullptr;
 	}
 }
 
 int CameraHalManager::init()
 {
-	/*
-	 * 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);
-
-	return 0;
-}
-
-void CameraHalManager::run()
-{
-	/*
-	 * 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
+	 * For each Camera registered in the system, a CameraDevice
-	 * gets created here to wraps a camera device.
+	 * gets created here to wraps a libcamera Camera instance.
 	 *
 	 * \todo Support camera hotplug.
 	 */
 	unsigned int index = 0;
-	for (auto &camera : cameraManager_->cameras()) {
+	for (auto &cam : cameraManager_->cameras()) {
-		CameraProxy *proxy = new CameraProxy(index, camera);
+		CameraDevice *camera = new CameraDevice(index, cam);
-		proxies_.emplace_back(proxy);
+		cameras_.emplace_back(camera);
 
 		++index;
 	}
 
-	/*
+	return 0;
-	 * 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;
 }
 
-CameraProxy *CameraHalManager::open(unsigned int id,
+CameraDevice *CameraHalManager::open(unsigned int id,
 				     const hw_module_t *hardwareModule)
 {
 	if (id >= numCameras()) {
@@ -107,13 +81,13 @@ CameraProxy *CameraHalManager::open(unsigned int id,
 		return nullptr;
 	}
 
-	CameraProxy *proxy = proxies_[id].get();
+	CameraDevice *camera = cameras_[id].get();
-	if (proxy->open(hardwareModule))
+	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;
 

src/android/camera_hal_manager.h

@@ -7,8 +7,7 @@
 #ifndef __ANDROID_CAMERA_MANAGER_H__
 #define __ANDROID_CAMERA_MANAGER_H__
 
-#include <condition_variable>
+#include <stddef.h>
-#include <mutex>
 #include <vector>
 
 #include <hardware/hardware.h>
@@ -16,33 +15,27 @@
 
 #include <libcamera/camera_manager.h>
 
-#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::vector<std::unique_ptr<CameraDevice>> cameras_;
-	std::condition_variable cv_;
-
-	std::vector<std::unique_ptr<CameraProxy>> proxies_;
 };
 
 #endif /* __ANDROID_CAMERA_MANAGER_H__ */

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 <system/camera_metadata.h>
+
+#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<CameraDevice *>(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<CameraDevice *>(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<CameraDevice *>(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<CameraDevice *>(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<camera3_device_t *>(hw_device);
+	CameraDevice *camera = reinterpret_cast<CameraDevice *>(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 },
+};

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 <hardware/camera3.h>
+
+int hal_dev_close(hw_device_t *hw_device);
+extern camera3_device_ops hal_dev_ops;
+
+#endif /* __ANDROID_CAMERA_OPS_H__ */

src/android/camera_proxy.cpp

@@ -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 <system/camera_metadata.h>
-
-#include <libcamera/object.h>
-
-#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<CameraProxy *>(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<CameraProxy *>(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<CameraProxy *>(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<CameraProxy *>(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<camera3_device_t *>(hw_device);
-	CameraProxy *proxy = reinterpret_cast<CameraProxy *>(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> &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;
-}

src/android/camera_proxy.h

@@ -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 <memory>
-
-#include <hardware/camera3.h>
-
-#include <libcamera/camera.h>
-
-class CameraDevice;
-
-class CameraProxy
-{
-public:
-	CameraProxy(unsigned int id, const std::shared_ptr<libcamera::Camera> &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__ */

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([