| Message ID | 20190806195518.16739-7-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thank you for the patch. On Tue, Aug 06, 2019 at 09:55:18PM +0200, Jacopo Mondi wrote: > Add libcamera Android Camera HALv3 implementation. > > The initial camera HAL implementation supports the LIMITED hardware > level and uses statically defined metadata and camera characteristics. > > Add a build option named 'android' and adjust the build system to > selectively compile the Android camera HAL and link it against the > required Android libraries. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > meson_options.txt | 5 + > src/android/camera3_hal.cpp | 130 +++++ > src/android/camera_hal_manager.cpp | 173 +++++++ > src/android/camera_hal_manager.h | 56 ++ > src/android/camera_module.cpp | 799 +++++++++++++++++++++++++++++ > src/android/camera_module.h | 75 +++ > src/android/camera_proxy.cpp | 181 +++++++ > src/android/camera_proxy.h | 41 ++ > src/android/meson.build | 8 + > src/android/thread_rpc.cpp | 41 ++ > src/android/thread_rpc.h | 56 ++ > src/libcamera/meson.build | 9 + > src/meson.build | 5 +- > 13 files changed, 1578 insertions(+), 1 deletion(-) > create mode 100644 src/android/camera3_hal.cpp > create mode 100644 src/android/camera_hal_manager.cpp > create mode 100644 src/android/camera_hal_manager.h > create mode 100644 src/android/camera_module.cpp > create mode 100644 src/android/camera_module.h > create mode 100644 src/android/camera_proxy.cpp > create mode 100644 src/android/camera_proxy.h > create mode 100644 src/android/thread_rpc.cpp > create mode 100644 src/android/thread_rpc.h [snip] > diff --git a/src/android/camera3_hal.cpp b/src/android/camera3_hal.cpp > new file mode 100644 > index 000000000000..0a97a9333d20 > --- /dev/null > +++ b/src/android/camera3_hal.cpp > @@ -0,0 +1,130 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * Copyright (C) 2019, Google Inc. > + * > + * camera3_hal.cpp - Android Camera3 HAL module > + */ > + > +#include <iostream> > +#include <memory> > +#include <vector> > + > +#include <hardware/hardware.h> > +#include <system/camera_metadata.h> Do you need camera_metadata.h ? Isn't it enough to include camera_common.h ? I think hardware.h can also be dropped. > + > +#include <libcamera/libcamera.h> To speed up compilation, it would be better to include only the header files we need (here and everywhere else). > + > +#include "log.h" > + > +#include "camera_hal_manager.h" > +#include "camera_module.h" > +#include "camera_proxy.h" > + > +using namespace libcamera; > + > +LOG_DEFINE_CATEGORY(HAL) > + > +static CameraHalManager cameraManager; > + > +/*------------------------------------------------------------------------------ > + * Android Camera HAL callbacks > + */ > + > +static int hal_get_number_of_cameras(void) > +{ > + return cameraManager.numCameras(); > +} > + > +static int hal_get_camera_info(int id, struct camera_info *info) > +{ > + return cameraManager.getCameraInfo(id, info); > +} > + > +static int hal_set_callbacks(const camera_module_callbacks_t *callbacks) > +{ > + return 0; > +} > + > +static int hal_open_legacy(const struct hw_module_t *module, const char *id, > + uint32_t halVersion, struct hw_device_t **device) > +{ > + return -ENOSYS; > +} > + > +static int hal_set_torch_mode(const char *camera_id, bool enabled) > +{ > + return -ENOSYS; > +} > + > +/* > + * First entry point of the camera HAL module. > + * > + * Initialize the HAL but does not open any camera device yet (see hal_dev_open) > + */ > +static int hal_init() > +{ > + LOG(HAL, Info) << "Initialising Android camera HAL"; > + > + cameraManager.initialize(); > + > + return 0; > +} > + > +/*------------------------------------------------------------------------------ > + * Android Camera Device > + */ > + > +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 *cameraModule = reinterpret_cast<CameraProxy *>(dev->priv); s/cameraModule/proxy/ otherwise it's very confusing to have a CameraProxy instance called cameraModule when there's a CameraModule class. This comment applies to all other similar instances. > + > + return cameraManager.close(cameraModule); As cameraManager.close() only calls proxy->close(), how about defining the hal_dev_close() method as a static method of CameraProxy, and setting proxy->device()->common.close inside the CameraProxy constructor ? It would avoid touching the internals of the CameraProxy in hal_dev_open() below. > +} > + > +static int hal_dev_open(const hw_module_t* module, const char* name, > + hw_device_t** device) > +{ > + LOG(HAL, Debug) << "Open camera: " << name; s/camera:/camera/ > + > + int id = atoi(name); > + CameraProxy *proxy = cameraManager.open(id, module); > + if (!proxy) { > + LOG(HAL, Error) << "Failed to open camera module " << id; > + return -ENODEV; > + } > + proxy->device()->common.close = hal_dev_close; > + *device = &proxy->device()->common; > + > + return 0; > +} Would it make sense to define all these methods as static methods of the CameraHalManager class, and turn the class into a singleton with an instance() method ? > + > +static struct hw_module_methods_t hal_module_methods = { > + .open = hal_dev_open, > +}; > + > +camera_module_t HAL_MODULE_INFO_SYM = { > + .common = { > + .tag = HARDWARE_MODULE_TAG, > + .module_api_version = CAMERA_MODULE_API_VERSION_2_4, > + .hal_api_version = HARDWARE_HAL_API_VERSION, > + .id = CAMERA_HARDWARE_MODULE_ID, > + .name = "Libcamera Camera3HAL Module", s/Libcamera/libcamera/ > + .author = "libcamera", > + .methods = &hal_module_methods, > + .dso = nullptr, > + .reserved = {}, > + }, > + > + .get_number_of_cameras = hal_get_number_of_cameras, > + .get_camera_info = hal_get_camera_info, > + .set_callbacks = hal_set_callbacks, > + .get_vendor_tag_ops = nullptr, > + .open_legacy = hal_open_legacy, > + .set_torch_mode = hal_set_torch_mode, > + .init = hal_init, > + .reserved = {}, > +}; > diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp > new file mode 100644 > index 000000000000..734b5eebd9a5 > --- /dev/null > +++ b/src/android/camera_hal_manager.cpp > @@ -0,0 +1,173 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2019, Google Inc. > + * > + * camera_hal_manager.cpp - Libcamera Android Camera Manager > + */ > + > +#include "camera_hal_manager.h" > + > +#include <map> This shouldn't be needed. > + > +#include <hardware/hardware.h> > +#include <system/camera_metadata.h> > + > +#include <libcamera/libcamera.h> > +#include <libcamera/camera.h> > + > +#include "log.h" > + > +#include "camera_module.h" > +#include "camera_proxy.h" > + > +using namespace libcamera; Missing blank line. > +LOG_DECLARE_CATEGORY(HAL); > + > +CameraHalManager::~CameraHalManager() > +{ > + for (auto metadata : staticMetadataMap_) > + free_camera_metadata(metadata.second); > +} > + > +int CameraHalManager::initialize() > +{ > + /* > + * Thread::start() will create a std::thread which will execute > + * CameraHalManager::run() > + */ That's documenting the libcamera::Thread class (and the std::thread is an internal implementation detail). I would just state /* * Start the camera HAL manager thread and wait until its * initialisation completes to be fully operational before * receiving calls from the camera stack. */ (thus dropping the next comment) > + start(); > + > + /* > + * Wait for run() to notify us before returning to the caller to > + * make sure libcamera is fully initialized before we start handling > + * calls from the camera stack. > + */ > + MutexLocker locker(mutex_); > + cv_.wait(locker); > + > + return 0; > +} > + > +void CameraHalManager::run() > +{ > + /* > + * The run() operation is scheduled in its own thread; > + * It exec() to run the even dispatcher loop and initialize libcamera. s/even/event/ > + * > + * All the libcamera components initialized from here will be tied to > + * the same thread as the here below run event dispatcher. The first paragraph also mostly documents the Thread class. I would drop it, and write the second paragraph as /* * All the libcamera components must be initialised here, in * order to bind them to the camera HAL manager thread that * executes the event dispatcher. / > + */ > + libcameraManager_ = libcamera::CameraManager::instance(); > + > + int ret = libcameraManager_->start(); > + if (ret) { > + LOG(HAL, Error) << "Failed to start camera manager: " > + << strerror(-ret); > + return; > + } > + > + /* > + * For each Camera registered in the system, a CameraModule > + * get created here with an associated Camera and a proxy. s/get/gets/ > + * > + * \todo Support camera hotplug. > + */ > + unsigned int cameraIndex = 0; Maybe just index ? > + for (auto camera : libcameraManager_->cameras()) { auto &camera otherwise camera will be a copy of the shared pointer instead of a reference to it. > + cameras_.push_back(camera); The cameras_ vector isn't used after this. I would drop it, and store the shared pointer in CameraModule. > + > + CameraModule *module = new CameraModule(cameraIndex, > + camera.get()); > + modules_.emplace_back(module); > + > + CameraProxy *proxy = new CameraProxy(cameraIndex, > + module); > + proxies_.emplace_back(proxy); Similarly, how about sotring the module pointer inside the CameraProxy class ? That would ensure that all accesses to the module go through the proxy, which I think would be a good idea. CameraProxy *proxy = new CameraProxy(index, camera); proxies_.emplace_back(proxy); > + > + ++cameraIndex; > + } > + > + /* > + * libcamera has been initialized! Unlock the initialize() caller > + * as we're now ready to handle calls from the framework. > + */ > + cv_.notify_one(); > + > + /* Now start processing events and messages! */ s/!/./ in the above two messages, there's no need to be over-enthusiastic :-) > + exec(); > +} > + > +CameraProxy *CameraHalManager::open(unsigned int id, > + const hw_module_t *hardwareModule) > +{ > + if (id < 0 || id >= numCameras()) { > + LOG(HAL, Error) << "Invalid camera id '" << id << "'"; > + return nullptr; > + } > + > + CameraProxy *proxy = proxies_[id].get(); > + if (proxy->open(hardwareModule)) > + return proxy; Shouldn't you return nullprtr here ? > + > + LOG(HAL, Info) << "Camera: '" << id << "' opened"; s/Camera:/Camera/ > + > + return proxy; > +} > + > +int CameraHalManager::close(CameraProxy *proxy) > +{ > + proxy->close(); > + LOG(HAL, Info) << "Close camera: " << proxy->id(); > + > + return 0; > +} > + > +unsigned int CameraHalManager::numCameras() const > +{ > + return libcameraManager_->cameras().size(); > +} > + > +/* > + * Before the camera framework even tries to open a camera device with > + * hal_dev_open() it requires the camera HAL to report a list of static > + * informations on the camera device with id \a id in the hal_get_camera_info() > + * method. > + * > + * The static metadata should be then generated probably from a > + * platform-specific module, as we cannot operate on the camera at this time as > + * it has not yet been open by the framework. s/open/opened/ What prevents us from opening the camera internally ? I don't think we need a platform-specific module. > + * > + * This is what the Intel XML file is used for, it is parsed and the data there > + * combined with informations from the PSL service (which I -think- it's what > + * our IPA is) to generate a list of static metadata per-camera device. I'd drop this paragraph. > + */ > +int CameraHalManager::getCameraInfo(int id, struct camera_info *info) > +{ > + int cameras = numCameras(); > + if (id >= cameras || id < 0 || !info) { Can we be called with info == nullptr ? I think I'd drop that part of the check. > + LOG(HAL, Error) << "Invalid camera id: " << id; s/id:/id/ You may also want to add quotes around the value, as done in CameraHalManager::open() (or drop them there). > + return -EINVAL; > + } > + > + camera_metadata_t *staticMetadata; > + auto it = staticMetadataMap_.find(id); > + if (it != staticMetadataMap_.end()) { > + staticMetadata = it->second; > + } else { > + CameraModule *cameraModule = modules_[id].get(); > + > + staticMetadata = cameraModule->getStaticMetadata(); > + staticMetadataMap_[id] = staticMetadata; Why do you need the map, why can't you always retrieve the static metadata from the CameraModule ? > + } > + > + /* \todo: Get these info dynamically inspecting the camera module. */ s/:// > + info->facing = id ? CAMERA_FACING_FRONT : CAMERA_FACING_BACK; > + info->orientation = 0; > + info->device_version = 0; > + info->resource_cost = 0; > + info->static_camera_characteristics = staticMetadata; > + info->conflicting_devices = nullptr; > + info->conflicting_devices_length = 0; > + > + return 0; > +} > diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h > new file mode 100644 > index 000000000000..65d6fa3150ef > --- /dev/null > +++ b/src/android/camera_hal_manager.h > @@ -0,0 +1,56 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2019, Google Inc. > + * > + * camera_hal_manager.h - Libcamera Android Camera Manager > + */ > +#ifndef __ANDROID_CAMERA_MANAGER_H__ > +#define __ANDROID_CAMERA_MANAGER_H__ > + > +#include <condition_variable> > +#include <cstddef> > +#include <map> > +#include <memory> > +#include <vector> > + > +#include <hardware/hardware.h> > +#include <system/camera_metadata.h> > + > +#include <libcamera/libcamera.h> > + > +#include "thread.h" > +#include "thread_rpc.h" > + > +class CameraModule; > +class CameraProxy; > + > +class CameraHalManager : public libcamera::Thread > +{ > +public: > + ~CameraHalManager(); > + > + int initialize(); Our initialisation methods are usually called init(), not initialize(). > + > + CameraProxy *open(unsigned int id, const hw_module_t *module); > + int close(CameraProxy *proxy); > + > + unsigned int numCameras() const; > + int getCameraInfo(int id, struct camera_info *info); > + > +private: > + void run() override; > + camera_metadata_t *getStaticMetadata(unsigned int id); > + > + libcamera::CameraManager *libcameraManager_; I think you can call this manager_. > + > + std::mutex mutex_; > + std::condition_variable cv_; > + > + std::vector<std::shared_ptr<libcamera::Camera>> cameras_; > + std::vector<std::unique_ptr<CameraModule>> modules_; > + std::vector<std::unique_ptr<CameraProxy>> proxies_; > + > + std::map<unsigned int, camera_metadata_t *> staticMetadataMap_; > +}; > + > +#endif /* __ANDROID_CAMERA_MANAGER_H__ */ > diff --git a/src/android/camera_module.cpp b/src/android/camera_module.cpp > new file mode 100644 > index 000000000000..b64e377fb439 > --- /dev/null > +++ b/src/android/camera_module.cpp > @@ -0,0 +1,799 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2019, Google Inc. > + * > + * camera_module.cpp - Libcamera Android Camera Module > + */ > + > +#include "camera_module.h" > + > +#include <iostream> > +#include <memory> > + > +#include <system/camera_metadata.h> > + > +#include <hardware/camera3.h> I would group all the Android headers together, so #include <hardware/camera3.h> #include <system/camera_metadata.h> #include <libcamera/libcamera.h> > +#include <libcamera/libcamera.h> > + > +#include "message.h" > + > +#include "log.h" #include "log.h" #include "message.h" > + > +using namespace libcamera; > + > +LOG_DECLARE_CATEGORY(HAL); > + > +CameraModule::CameraModule(unsigned int id, libcamera::Camera *camera) > + : id_(id), camera_(camera), cameraState_(CameraStopped), > + requestTemplate_(nullptr) > +{ > + camera_->requestCompleted.connect(this, > + &CameraModule::requestComplete); This holds on a single line. > +} > + > +CameraModule::~CameraModule() > +{ > + if (requestTemplate_) > + free_camera_metadata(requestTemplate_); > + requestTemplate_ = nullptr; > +} > + > +void CameraModule::message(Message *message) > +{ > + ThreadRpcMessage *rpcMessage = static_cast<ThreadRpcMessage *> > + (message); > + > + if (rpcMessage->type() != ThreadRpcMessage::type()) > + return Object::message(message); As Message may not be a ThreadRpcMessage, you should write this as if (message->type() != ThreadRpcMessage::type()) return Object::message(message); ThreadRpcMessage *rpcMessage = static_cast<ThreadRpcMessage *>(message); > + > + ThreadRpc *rpc = rpcMessage->rpc; > + > + switch (rpc->tag) { > + case ThreadRpc::ProcessCaptureRequest: > + processCaptureRequest(rpc->request); > + break; > + case ThreadRpc::Close: > + close(); > + break; > + default: > + LOG(HAL, Error) << "Unknown RPC operation: " << rpc->tag; > + } > + > + rpc->notifyReception(); > +} > + > +int CameraModule::open() > +{ > + int ret = camera_->acquire(); > + if (ret) { > + LOG(HAL, Error) << "Failed to acquire the camera"; > + return ret; > + } > + > + return 0; > +} > + > +void CameraModule::close() > +{ > + camera_->stop(); > + > + camera_->freeBuffers(); > + camera_->release(); > + > + cameraState_ = CameraStopped; > +} > + > +void CameraModule::setCallbacks(const struct camera3_device *camera3Device, > + const camera3_callback_ops_t *callbacks) > +{ > + camera3Device_ = camera3Device; camera3Device_ seems unused. > + callbacks_ = callbacks; > +} > + > +camera_metadata_t *CameraModule::getStaticMetadata() > +{ > + int ret; > + > + /* > + * The below metadata has been added by inspecting the Intel XML > + * file and it's associated parsing routines in the Intel IPU3 HAL. > + * > + * The here below metadata are enough to satisfy the camera3-test > + * provided by ChromeOS, but a real camera implementation might require s/might/will/ > + * more. > + * > + * See CameraConf::AiqConf class implementation in the Intel HAL > + * to get a list of the static metadata registered by parsing the > + * XML config file in AiqConf::fillStaticMetadataFromCMC > + */ > + > + /* > + * FIXME: this sizes come from the Intel IPU3 HAL, and are way too large for s/FIXME:/\todo/ s/this/these/ > + * this intial HAL version. > + */ > + #define STATIC_ENTRY_CAP 256 > + #define STATIC_DATA_CAP 6688 > + camera_metadata_t *staticMetadata = > + allocate_camera_metadata(STATIC_ENTRY_CAP, STATIC_DATA_CAP); > + > + /* Sensor static metadata. */ > + int32_t pixelArraySize[] = { > + 2592, 1944, > + }; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE, > + &pixelArraySize, 2); > + METADATA_ASSERT(ret); > + > + int32_t sensorSizes[] = { > + 0, 0, 2560, 1920, > + }; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE, > + &sensorSizes, 4); > + METADATA_ASSERT(ret); > + > + int32_t sensitivityRange[] = { > + 32, 2400, > + }; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_SENSOR_INFO_SENSITIVITY_RANGE, > + &sensitivityRange, 2); > + METADATA_ASSERT(ret); > + > + uint16_t filterArr = ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT_GRBG; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT, > + &filterArr, 1); > + METADATA_ASSERT(ret); > + > + int64_t exposureTimeRange[] = { > + 100000, 200000000, > + }; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE, > + &exposureTimeRange, 2); > + METADATA_ASSERT(ret); > + > + int32_t orientation = 0; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_SENSOR_ORIENTATION, > + &orientation, 1); > + METADATA_ASSERT(ret); > + > + /* Flash static metadata. */ > + char flashAvailable = ANDROID_FLASH_INFO_AVAILABLE_FALSE; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_FLASH_INFO_AVAILABLE, > + &flashAvailable, 1); > + METADATA_ASSERT(ret); > + > + /* Lens static metadata. */ > + float fn = 2.53 / 100; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_LENS_INFO_AVAILABLE_APERTURES, &fn, 1); > + METADATA_ASSERT(ret); > + > + /* Control metadata. */ > + char controlMetadata = ANDROID_CONTROL_MODE_AUTO; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_CONTROL_AVAILABLE_MODES, > + &controlMetadata, 1); > + METADATA_ASSERT(ret); > + > + char availableAntiBandingModes[] = { > + ANDROID_CONTROL_AE_ANTIBANDING_MODE_OFF, > + ANDROID_CONTROL_AE_ANTIBANDING_MODE_50HZ, > + ANDROID_CONTROL_AE_ANTIBANDING_MODE_60HZ, > + ANDROID_CONTROL_AE_ANTIBANDING_MODE_AUTO, > + }; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES, > + availableAntiBandingModes, 4); > + METADATA_ASSERT(ret); > + > + char aeAvailableModes[] = { > + ANDROID_CONTROL_AE_MODE_ON, > + ANDROID_CONTROL_AE_MODE_OFF, > + }; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_CONTROL_AE_AVAILABLE_MODES, > + aeAvailableModes, 2); > + METADATA_ASSERT(ret); > + > + controlMetadata = ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_CONTROL_AE_LOCK_AVAILABLE, > + &controlMetadata, 1); > + METADATA_ASSERT(ret); > + > + uint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, > + &awbLockAvailable, 1); > + > + /* Scaler static metadata. */ > + std::vector<uint32_t> availableStreamFormats = { > + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, > + ANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, > + ANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, > + }; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_SCALER_AVAILABLE_FORMATS, > + availableStreamFormats.data(), > + availableStreamFormats.size()); > + METADATA_ASSERT(ret); > + > + std::vector<uint32_t> availableStreamConfigurations = { > + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, > + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT, > + ANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, 2560, 1920, > + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT, > + ANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, 2560, 1920, > + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT, > + }; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS, > + availableStreamConfigurations.data(), > + availableStreamConfigurations.size()); > + METADATA_ASSERT(ret); > + > + std::vector<int64_t> availableStallDurations = { > + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, 33333333, > + }; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_SCALER_AVAILABLE_STALL_DURATIONS, > + availableStallDurations.data(), > + availableStallDurations.size()); > + METADATA_ASSERT(ret); > + > + std::vector<int64_t> minFrameDurations = { > + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, 33333333, > + ANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, 2560, 1920, 33333333, > + ANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, 2560, 1920, 33333333, > + }; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS, > + minFrameDurations.data(), minFrameDurations.size()); > + METADATA_ASSERT(ret); > + > + /* Info static metadata. */ > + uint8_t supportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED; > + ret = add_camera_metadata_entry(staticMetadata, > + ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL, > + &supportedHWLevel, 1); > + > + return staticMetadata; > +} > + > +const camera_metadata_t *CameraModule::constructDefaultRequestMetadata(int type) > +{ > + int ret; > + > + /* > + * TODO: inspect type and pick the right metadata pack. s/TODO:/\todo/ > + * As of now just use a single one for all templates > + */ > + uint8_t captureIntent; > + switch (type) { > + case CAMERA3_TEMPLATE_PREVIEW: > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_PREVIEW; > + break; > + case CAMERA3_TEMPLATE_STILL_CAPTURE: > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_STILL_CAPTURE; > + break; > + case CAMERA3_TEMPLATE_VIDEO_RECORD: > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_VIDEO_RECORD; > + break; > + case CAMERA3_TEMPLATE_VIDEO_SNAPSHOT: > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_VIDEO_SNAPSHOT; > + break; > + case CAMERA3_TEMPLATE_ZERO_SHUTTER_LAG: > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_ZERO_SHUTTER_LAG; > + break; > + case CAMERA3_TEMPLATE_MANUAL: > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_MANUAL; > + break; > + default: > + LOG(HAL, Error) << "Invalid template request type: " << type; > + return nullptr; > + } > + > + if (requestTemplate_) > + return requestTemplate_; > + > + /* FIXME: this sizes are quite arbitrary ones */ > + #define REQUEST_TEMPLATE_ENTRIES 30 > + #define REQUEST_TEMPLATE_DATA 2048 > + requestTemplate_ = allocate_camera_metadata(REQUEST_TEMPLATE_ENTRIES, > + REQUEST_TEMPLATE_DATA); > + if (!requestTemplate_) { > + LOG(HAL, Error) << "Failed to allocate template metadata"; > + return nullptr; > + } > + > + /* > + * Fill in the required Request metadata. > + * > + * Part (most?) of this entries comes from inspecting the Intel's IPU3 > + * HAL xml configuration file. > + */ > + > + /* Set to 0 the number of 'processed and stalling' streams (ie JPEG). */ > + int32_t maxOutStream[] = { 0, 2, 0 }; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS, > + maxOutStream, 3); > + METADATA_ASSERT(ret); > + > + uint8_t maxPipelineDepth = 5; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_REQUEST_PIPELINE_MAX_DEPTH, > + &maxPipelineDepth, 1); > + METADATA_ASSERT(ret); > + > + int32_t inputStreams = 0; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_REQUEST_MAX_NUM_INPUT_STREAMS, > + &inputStreams, 1); > + METADATA_ASSERT(ret); > + > + int32_t partialResultCount = 1; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_REQUEST_PARTIAL_RESULT_COUNT, > + &partialResultCount, 1); > + METADATA_ASSERT(ret); > + > + uint8_t availableCapabilities[] = { > + ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE, > + }; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_REQUEST_AVAILABLE_CAPABILITIES, > + availableCapabilities, 1); > + METADATA_ASSERT(ret); > + > + uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_CONTROL_AE_MODE, > + &aeMode, 1); > + METADATA_ASSERT(ret); > + > + int32_t aeExposureCompensation = 0; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, > + &aeExposureCompensation, 1); > + METADATA_ASSERT(ret); > + > + uint8_t aePrecaptureTrigger = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, > + &aePrecaptureTrigger, 1); > + METADATA_ASSERT(ret); > + > + uint8_t aeLock = ANDROID_CONTROL_AE_LOCK_OFF; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_CONTROL_AE_LOCK, > + &aeLock, 1); > + METADATA_ASSERT(ret); > + > + uint8_t afTrigger = ANDROID_CONTROL_AF_TRIGGER_IDLE; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_CONTROL_AF_TRIGGER, > + &afTrigger, 1); > + METADATA_ASSERT(ret); > + > + uint8_t awbMode = ANDROID_CONTROL_AWB_MODE_AUTO; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_CONTROL_AWB_MODE, > + &awbMode, 1); > + METADATA_ASSERT(ret); > + > + uint8_t awbLock = ANDROID_CONTROL_AWB_LOCK_OFF; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_CONTROL_AWB_LOCK, > + &awbLock, 1); > + METADATA_ASSERT(ret); > + > + uint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, > + &awbLockAvailable, 1); > + METADATA_ASSERT(ret); > + > + uint8_t flashMode = ANDROID_FLASH_MODE_OFF; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_FLASH_MODE, > + &flashMode, 1); > + METADATA_ASSERT(ret); > + > + uint8_t faceDetectMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_STATISTICS_FACE_DETECT_MODE, > + &faceDetectMode, 1); > + METADATA_ASSERT(ret); > + > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_CONTROL_CAPTURE_INTENT, > + &captureIntent, 1); > + METADATA_ASSERT(ret); > + > + /* > + * This is quite hard to list at the moment wihtout knowing what > + * we could control. > + * > + * For now, just list in the available Request keys and in the available > + * result keys the control and reporting of the AE algorithm. > + */ > + std::vector<int32_t> availableRequestKeys = { > + ANDROID_CONTROL_AE_MODE, > + ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, > + ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, > + ANDROID_CONTROL_AE_LOCK, > + ANDROID_CONTROL_AF_TRIGGER, > + ANDROID_CONTROL_AWB_MODE, > + ANDROID_CONTROL_AWB_LOCK, > + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, > + ANDROID_CONTROL_CAPTURE_INTENT, > + ANDROID_FLASH_MODE, > + ANDROID_STATISTICS_FACE_DETECT_MODE, > + }; > + > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS, > + availableRequestKeys.data(), > + availableRequestKeys.size()); > + METADATA_ASSERT(ret); > + > + std::vector<int32_t> availableResultKeys = { > + ANDROID_CONTROL_AE_MODE, > + ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, > + ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, > + ANDROID_CONTROL_AE_LOCK, > + ANDROID_CONTROL_AF_TRIGGER, > + ANDROID_CONTROL_AWB_MODE, > + ANDROID_CONTROL_AWB_LOCK, > + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, > + ANDROID_CONTROL_CAPTURE_INTENT, > + ANDROID_FLASH_MODE, > + ANDROID_STATISTICS_FACE_DETECT_MODE, > + }; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_REQUEST_AVAILABLE_RESULT_KEYS, > + availableResultKeys.data(), > + availableResultKeys.size()); > + METADATA_ASSERT(ret); > + > + /* > + * The available characteristics should be the tags reported > + * as part of the static metadata reported at hal_get_camera_info() > + * time. The xml file sets those to 0 though. > + */ > + std::vector<int32_t> availableCharacteristicsKeys = {}; > + ret = add_camera_metadata_entry(requestTemplate_, > + ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS, > + availableCharacteristicsKeys.data(), > + availableCharacteristicsKeys.size()); > + METADATA_ASSERT(ret); > + > + return requestTemplate_; > +} > + > +/** > + * Inspect the stream_list to produce a list of StreamConfiguration to > + * be use to configure the Camera. > + */ > +int CameraModule::configureStreams(camera3_stream_configuration_t *stream_list) > +{ > + Extra blank line. > + for (unsigned int i = 0; i < stream_list->num_streams; ++i) { > + camera3_stream_t *stream = stream_list->streams[i]; > + > + LOG(HAL, Info) << "Stream #" << i > + << ": direction: " << stream->stream_type > + << " - width: " << stream->width > + << " - height: " << stream->height > + << " - format: " << std::hex << stream->format; s/ -/,/ in the above three lines And I would print size: ...x... instead of width and height. > + } > + > + /* Hardcode viewfinder role, collecting sizes from the stream config. */ > + if (stream_list->num_streams != 1) { > + LOG(HAL, Error) << "Only one stream supported"; > + return -EINVAL; > + } > + > + StreamRoles roles = {}; > + roles.push_back(StreamRole::StillCapture); I think you can write this as StreamRoles roles{ StreamRole::StillCapture }; > + > + config_ = camera_->generateConfiguration(roles); > + if (!config_ || config_->empty()) { > + LOG(HAL, Error) << "Failed to generate camera configuration"; > + return -EINVAL; > + } > + > + /* Only one stream is supported. */ > + camera3_stream_t *camera3Stream = stream_list->streams[0]; > + StreamConfiguration *streamConfiguration = &config_->at(0); > + streamConfiguration->size.width = camera3Stream->width; > + streamConfiguration->size.height = camera3Stream->height; > + streamConfiguration->memoryType = ExternalMemory; > + > + /* > + * FIXME: do not change the pixel format from the one returned > + * from the Camera::generateConfiguration(); > + * > + * We'll need to translate from Android defined pixel format codes > + * to whatever libcamera will use. > + */ > + > + switch (config_->validate()) { > + case CameraConfiguration::Valid: > + break; > + case CameraConfiguration::Adjusted: > + LOG(HAL, Info) << "Camera configuration adjusted"; Missing config_.reset() ? > + return -EINVAL; > + case CameraConfiguration::Invalid: > + LOG(HAL, Info) << "Camera configuration invalid"; > + config_.reset(); > + return -EINVAL; > + } > + > + camera3Stream->max_buffers = streamConfiguration->bufferCount; > + > + /* > + * Once the CameraConfiguration has been adjusted/validated > + * it can be applied to the camera. > + */ > + int ret = camera_->configure(config_.get()); > + if (ret) { > + LOG(HAL, Error) << "Failed to configure camera '" > + << camera_->name() << "'"; > + return ret; > + } > + > + return 0; > +} > + > +int CameraModule::processCaptureRequest(camera3_capture_request_t *camera3Request) > +{ > + StreamConfiguration *streamConfiguration = &config_->at(0); > + Stream *stream = streamConfiguration->stream(); > + > + if (camera3Request->num_output_buffers != 1) { > + LOG(HAL, Error) << "Invalid number of output buffers: " > + << camera3Request->num_output_buffers; > + return -EINVAL; > + } > + > + /* Start the camera if that's the first request we handle. */ > + if (cameraState_ == CameraStopped) { > + int ret = camera_->allocateBuffers(); > + if (ret) { > + LOG(HAL, Error) << "Failed to allocate buffers"; > + return ret; > + } > + > + ret = camera_->start(); > + if (ret) { > + LOG(HAL, Error) << "Failed to start camera"; > + camera_->freeBuffers(); > + return ret; > + } > + > + cameraState_ = CameraRunning; > + } > + > + /* > + * Queue a request for the Camera with the provided dmabuf file > + * descriptors. > + */ > + const camera3_stream_buffer_t *camera3Buffer = > + &camera3Request->output_buffers[0]; > + const buffer_handle_t camera3Handle = *camera3Buffer->buffer; > + > + std::array<int, 3> fds = { > + camera3Handle->data[0], > + camera3Handle->data[1], > + camera3Handle->data[2], > + }; > + std::unique_ptr<Buffer> buffer = stream->createBuffer(fds); > + if (!buffer) { > + LOG(HAL, Error) << "Failed to create buffer"; > + return -EINVAL; > + } > + > + Request *request = camera_->createRequest(); > + request->addBuffer(std::move(buffer)); > + int ret = camera_->queueRequest(request); > + if (ret) { > + LOG(HAL, Error) << "Failed to queue request"; > + return ret; > + } > + > + /* Save the request descriptors for use at completion time. */ > + Camera3RequestDescriptor descriptor = {}; > + descriptor.frameNumber = camera3Request->frame_number, > + descriptor.numBuffers = camera3Request->num_output_buffers, s/,$/;/ for the two lines above. I don't think you need to store numBuffers in the descriptors, it can be retrieved from buffers.size() in requestComplete(). > + descriptor.buffers = new camera3_stream_buffer_t[descriptor.numBuffers]; > + for (unsigned int i = 0; i < descriptor.numBuffers; ++i) { > + camera3_stream_buffer_t &buffer = descriptor.buffers[i]; > + buffer = camera3Request->output_buffers[i]; Just descriptor.buffers[i] = camera3Request->output_buffers[i]; > + } > + > + requestMap_[request] = descriptor; How about using the request cookie (passed to Camera::createRequest()) for this purpose ? > + > + return 0; > +} > + > +void CameraModule::requestComplete(Request *request, > + const std::map<Stream *, Buffer *> &buffers) > +{ > + Buffer *libcameraBuffer = buffers.begin()->second; > + camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; > + camera_metadata_t *resultMetadata = nullptr; > + > + if (request->status() != Request::RequestComplete) { > + LOG(HAL, Error) << "Request not succesfully completed: " > + << request->status(); > + status = CAMERA3_BUFFER_STATUS_ERROR; > + } > + > + /* Prepare to call back the Android camera stack. */ > + Camera3RequestDescriptor &descriptor = requestMap_[request]; > + > + camera3_capture_result_t captureResult = {}; > + captureResult.frame_number = descriptor.frameNumber; > + captureResult.num_output_buffers = descriptor.numBuffers; > + > + camera3_stream_buffer_t *resultBuffers = > + new camera3_stream_buffer_t[descriptor.numBuffers]; > + for (unsigned int i = 0; i < descriptor.numBuffers; ++i) { > + camera3_stream_buffer_t resultBuffer; > + > + resultBuffer = descriptor.buffers[i]; > + resultBuffer.acquire_fence = -1; > + resultBuffer.release_fence = -1; > + resultBuffer.status = status; > + > + resultBuffers[i] = resultBuffer; > + } > + captureResult.output_buffers = > + const_cast<const camera3_stream_buffer_t *>(resultBuffers); > + > + if (status == CAMERA3_BUFFER_STATUS_ERROR) { > + /* \todo Improve error handling. */ > + notifyError(descriptor.frameNumber, resultBuffers->stream); > + } else { > + notifyShutter(descriptor.frameNumber, libcameraBuffer->timestamp()); > + > + captureResult.partial_result = 1; > + resultMetadata = getResultMetadata(descriptor.frameNumber, > + libcameraBuffer->timestamp()); > + captureResult.result = resultMetadata; > + } > + > + callbacks_->process_capture_result(callbacks_, &captureResult); > + > + if (resultMetadata) > + free_camera_metadata(resultMetadata); > + > + delete[] resultBuffers; > + delete[] descriptor.buffers; > + requestMap_.erase(request); > + > + return; > +} > + > +void CameraModule::notifyShutter(uint32_t frameNumber, uint64_t timestamp) > +{ > + camera3_notify_msg_t notify = {}; > + > + notify.type = CAMERA3_MSG_SHUTTER; > + notify.message.shutter.frame_number = frameNumber; > + notify.message.shutter.timestamp = timestamp; > + > + callbacks_->notify(callbacks_, ¬ify); > +} > + > +void CameraModule::notifyError(uint32_t frameNumber, camera3_stream_t *stream) > +{ > + camera3_notify_msg_t notify = {}; > + > + notify.type = CAMERA3_MSG_ERROR; > + notify.message.error.error_stream = stream; > + notify.message.error.frame_number = frameNumber; > + notify.message.error.error_code = CAMERA3_MSG_ERROR_REQUEST; > + > + callbacks_->notify(callbacks_, ¬ify); > +} > + > +/* > + * Fixed result metadata, mostly imported from the UVC camera HAL, which > + * does not produces metadata and thus needs to generate a fixed set. > + */ > +camera_metadata_t *CameraModule::getResultMetadata(int frame_number, > + int64_t timestamp) > +{ > + int ret; > + > + /* FIXME: random "big enough" values. */ > + #define RESULT_ENTRY_CAP 256 > + #define RESULT_DATA_CAP 6688 > + camera_metadata_t *resultMetadata = > + allocate_camera_metadata(STATIC_ENTRY_CAP, STATIC_DATA_CAP); > + > + const uint8_t ae_state = ANDROID_CONTROL_AE_STATE_CONVERGED; > + ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AE_STATE, > + &ae_state, 1); > + METADATA_ASSERT(ret); > + > + const uint8_t ae_lock = ANDROID_CONTROL_AE_LOCK_OFF; > + ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AE_LOCK, > + &ae_lock, 1); > + METADATA_ASSERT(ret); > + > + uint8_t af_state = ANDROID_CONTROL_AF_STATE_INACTIVE; > + ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AF_STATE, > + &af_state, 1); > + METADATA_ASSERT(ret); > + > + const uint8_t awb_state = ANDROID_CONTROL_AWB_STATE_CONVERGED; > + ret = add_camera_metadata_entry(resultMetadata, > + ANDROID_CONTROL_AWB_STATE, > + &awb_state, 1); > + METADATA_ASSERT(ret); > + > + const uint8_t awb_lock = ANDROID_CONTROL_AWB_LOCK_OFF; > + ret = add_camera_metadata_entry(resultMetadata, > + ANDROID_CONTROL_AWB_LOCK, > + &awb_lock, 1); > + METADATA_ASSERT(ret); > + > + const uint8_t lens_state = ANDROID_LENS_STATE_STATIONARY; > + ret = add_camera_metadata_entry(resultMetadata, > + ANDROID_LENS_STATE, > + &lens_state, 1); > + METADATA_ASSERT(ret); > + > + int32_t sensorSizes[] = { > + 0, 0, 2560, 1920, > + }; > + ret = add_camera_metadata_entry(resultMetadata, > + ANDROID_SCALER_CROP_REGION, > + sensorSizes, 4); > + METADATA_ASSERT(ret); > + > + ret = add_camera_metadata_entry(resultMetadata, > + ANDROID_SENSOR_TIMESTAMP, > + ×tamp, 1); > + > + METADATA_ASSERT(ret); > + > + /* 33.3 msec */ > + const int64_t rolling_shutter_skew = 33300000; > + ret = add_camera_metadata_entry(resultMetadata, > + ANDROID_SENSOR_ROLLING_SHUTTER_SKEW, > + &rolling_shutter_skew, 1); > + METADATA_ASSERT(ret); > + > + /* 16.6 msec */ > + const int64_t exposure_time = 16600000; > + ret = add_camera_metadata_entry(resultMetadata, > + ANDROID_SENSOR_EXPOSURE_TIME, > + &exposure_time, 1); > + METADATA_ASSERT(ret); > + > + const uint8_t lens_shading_map_mode = > + ANDROID_STATISTICS_LENS_SHADING_MAP_MODE_OFF; > + ret = add_camera_metadata_entry(resultMetadata, > + ANDROID_STATISTICS_LENS_SHADING_MAP_MODE, > + &lens_shading_map_mode, 1); > + METADATA_ASSERT(ret); > + > + const uint8_t scene_flicker = ANDROID_STATISTICS_SCENE_FLICKER_NONE; > + ret = add_camera_metadata_entry(resultMetadata, > + ANDROID_STATISTICS_SCENE_FLICKER, > + &scene_flicker, 1); > + METADATA_ASSERT(ret); > + > + return resultMetadata; > +} > diff --git a/src/android/camera_module.h b/src/android/camera_module.h > new file mode 100644 > index 000000000000..67488d5940b1 > --- /dev/null > +++ b/src/android/camera_module.h > @@ -0,0 +1,75 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2019, Google Inc. > + * > + * camera_module.h - Libcamera Android Camera Module s/Libcamera/libcamera/ (here and everywhere else, libcamera is always written in all lowercase) > + */ > +#ifndef __ANDROID_CAMERA_MODULE_H__ > +#define __ANDROID_CAMERA_MODULE_H__ > + > +#include <memory> > + > +#include <hardware/camera3.h> > +#include <libcamera/libcamera.h> > + > +#include "message.h" > + > +#include "camera_hal_manager.h" > + > +#define METADATA_ASSERT(_r) \ > + do { \ > + if (!(_r)) break; \ > + LOG(HAL, Error) << "Error: " << __func__ << ":" << __LINE__; \ > + return nullptr; \ > + } while(0); I really, really dislike hiding return statements in macros. We can keep it for now, but please add a comment telling that it should be removed. > + Even if we don't document the whole implementation with Doxygen, I think a few short comments that explain what each class models would be useful. Otherwise the reader is left to wonder what CameraModule or CameraProxy stand for. A comment that explains how the various objects work together would be useful too. > +class CameraModule : public libcamera::Object I don't think CameraModule is a good name :-/ In HAL terminology, the module refers to camera_module_t, and corresponds more or less to libcamera::CameraManager, not libcamera::Camera. One option would be to name this class Camera in a HAL namespace. That would make it quite clear that it corresponds to a libcamra::Camera. > +{ > +public: > + CameraModule(unsigned int id, libcamera::Camera *camera); > + ~CameraModule(); > + > + void message(libcamera::Message *message); > + > + int open(); > + void close(); > + void setCallbacks(const struct camera3_device *cameraDevice, > + const camera3_callback_ops_t *callbacks); > + camera_metadata_t *getStaticMetadata(); > + const camera_metadata_t *constructDefaultRequestMetadata(int type); > + int configureStreams(camera3_stream_configuration_t *stream_list); > + int processCaptureRequest(camera3_capture_request_t *request); > + void requestComplete(libcamera::Request *request, > + const std::map<libcamera::Stream *, libcamera::Buffer *> &buffers); > + > + unsigned int id() const { return id_; } I think this method is not used. > + > +private: > + struct Camera3RequestDescriptor { > + uint32_t frameNumber; > + uint32_t numBuffers; > + camera3_stream_buffer_t *buffers; > + }; > + > + enum CameraState { > + CameraStopped, > + CameraRunning, > + }; With just two states you could instead have a bool running_ field. > + > + void notifyShutter(uint32_t frameNumber, uint64_t timestamp); > + void notifyError(uint32_t frameNumber, camera3_stream_t *stream); > + camera_metadata_t *getResultMetadata(int frame_number, int64_t timestamp); > + > + unsigned int id_; > + libcamera::Camera *camera_; > + std::unique_ptr<libcamera::CameraConfiguration> config_; > + CameraState cameraState_; > + > + camera_metadata_t *requestTemplate_; > + const camera3_callback_ops_t *callbacks_; > + const struct camera3_device *camera3Device_; > + > + std::map<libcamera::Request *, Camera3RequestDescriptor> requestMap_; > +}; > + > +#endif /* __ANDROID_CAMERA_MODULE_H__ */ > diff --git a/src/android/camera_proxy.cpp b/src/android/camera_proxy.cpp > new file mode 100644 > index 000000000000..af7817f29137 > --- /dev/null > +++ b/src/android/camera_proxy.cpp > @@ -0,0 +1,181 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2019, Google Inc. > + * > + * camera_proxy.cpp - Proxy to camera module instances > + */ > + > +#include "camera_proxy.h" > + > +#include <hardware/camera3.h> > +#include <libcamera/libcamera.h> > +#include <system/camera_metadata.h> > + > +#include <memory> > + > +#include "log.h" > +#include "message.h" > +#include "thread_rpc.h" > +#include "utils.h" > + > +using namespace libcamera; > + > +LOG_DECLARE_CATEGORY(HAL); > + > +#define LIBCAMERA_HAL_FUNC_DBG \ > + LOG(HAL, Debug); If we need to trace calls, I think something better than debug messages would be useful. I'm not sure what yet though. > + > +static int hal_dev_initialize(const struct camera3_device *dev, > + const camera3_callback_ops_t *callback_ops) > +{ > + LIBCAMERA_HAL_FUNC_DBG > + > + if (!dev) > + return -EINVAL; Can this happen ? > + > + CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); > + proxy->setCallbacks(callback_ops); > + > + return 0; > +} > + > +static int hal_dev_configure_streams(const struct camera3_device *dev, > + camera3_stream_configuration_t *stream_list) > +{ > + LIBCAMERA_HAL_FUNC_DBG > + > + if (!dev) > + return -EINVAL; > + > + CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); > + proxy->configureStreams(stream_list); > + > + return 0; Shouldn't you propagate the error value from proxy->configureStreams() ? > +} > + > +static const camera_metadata_t * > +hal_dev_construct_default_request_settings(const struct camera3_device *dev, > + int type) > +{ > + LIBCAMERA_HAL_FUNC_DBG > + > + if (!dev) > + return nullptr; > + > + CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); > + return proxy->constructDefaultRequest(type); How about renaming constructDefaultRequest() and constructDefaultRequestMetadata() to constructDefaultRequestSettings(), in order to match the HAL operation name ? > +} > + > +static int hal_dev_process_capture_request(const struct camera3_device *dev, > + camera3_capture_request_t *request) > +{ > + LIBCAMERA_HAL_FUNC_DBG > + > + 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) > +{ > + LIBCAMERA_HAL_FUNC_DBG > +} > + > +static int hal_dev_flush(const struct camera3_device *dev) > +{ > + LIBCAMERA_HAL_FUNC_DBG > + > + return 0; > +} You could define these methods as static methods of the CameraProxy class. For instance, hal_dev_configure_streams would become int CameraProxy::configureStreams(const struct camera3_device *dev, camera3_stream_configuration_t *stream_list) { CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); proxy->configureStreams(stream_list); } > + > +static camera3_device_ops hal_dev_ops = { I wish this could be const :-( > + .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 = { 0 }, s/0/nullptr/ ? > +}; > + > +CameraProxy::CameraProxy(unsigned int id, CameraModule *cameraModule) > + : open_(false), id_(id), cameraModule_(cameraModule) > +{ > +} > + > +int CameraProxy::open(const hw_module_t *hardwareModule) > +{ > + int ret = cameraModule_->open(); > + if (ret) > + return ret; > + > + /* Initialize the hw_device_t in the instance camera3_module_t. */ > + cameraDevice_.common.tag = HARDWARE_DEVICE_TAG; > + cameraDevice_.common.version = CAMERA_DEVICE_API_VERSION_3_3; > + cameraDevice_.common.module = (hw_module_t *)hardwareModule; > + > + /* > + * The camera device operations. These actually implement > + * the Android Camera HALv3 interface. > + */ > + cameraDevice_.ops = &hal_dev_ops; > + cameraDevice_.priv = this; > + > + open_ = true; > + > + return 0; > +} > + > +void CameraProxy::close() > +{ > + LIBCAMERA_HAL_FUNC_DBG > + > + ThreadRpc rpcRequest; > + rpcRequest.tag = ThreadRpc::Close; > + > + threadRpcCall(rpcRequest); > + > + open_ = false; > +} > + > +void CameraProxy::setCallbacks(const camera3_callback_ops_t *callbacks) I would name this initialize() to match hal_dev_initialize(). > +{ > + LIBCAMERA_HAL_FUNC_DBG > + > + cameraModule_->setCallbacks(&cameraDevice_, callbacks); > +} > + > +const camera_metadata_t *CameraProxy::constructDefaultRequest(int type) > +{ > + return cameraModule_->constructDefaultRequestMetadata(type); > +} > + > +int CameraProxy::configureStreams(camera3_stream_configuration_t *stream_list) > +{ > + return cameraModule_->configureStreams(stream_list); > +} > + > +int CameraProxy::processCaptureRequest(camera3_capture_request_t *request) > +{ > + ThreadRpc rpcRequest; > + rpcRequest.tag = ThreadRpc::ProcessCaptureRequest; > + rpcRequest.request = request; > + > + threadRpcCall(rpcRequest); > + > + return 0; Does this method need to be synchronous ? We can keep it as-is for now, but I wonder if it wouldn't make sense to later move (part of) the validation code here and make the call asynchronous. > +} > + > +void CameraProxy::threadRpcCall(ThreadRpc &rpcRequest) > +{ > + std::unique_ptr<ThreadRpcMessage> message = > + utils::make_unique<ThreadRpcMessage>(); > + message->rpc = &rpcRequest; > + > + cameraModule_->postMessage(std::move(message)); > + rpcRequest.waitDelivery(); > +} > diff --git a/src/android/camera_proxy.h b/src/android/camera_proxy.h > new file mode 100644 > index 000000000000..69e6878c4352 > --- /dev/null > +++ b/src/android/camera_proxy.h > @@ -0,0 +1,41 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2019, Google Inc. > + * > + * camera_proxy.h - Proxy to camera module instances > + */ > +#ifndef __ANDROID_CAMERA_PROXY_H__ > +#define __ANDROID_CAMERA_PROXY_H__ > + > +#include <hardware/camera3.h> > +#include <libcamera/libcamera.h> > + > +#include "camera_module.h" > + > +class CameraProxy > +{ > +public: > + CameraProxy(unsigned int id, CameraModule *cameraModule); > + > + int open(const hw_module_t *hardwareModule); > + void close(); > + > + void setCallbacks(const camera3_callback_ops_t *callbacks); > + const camera_metadata_t *constructDefaultRequest(int type); > + int configureStreams(camera3_stream_configuration_t *stream_list); > + int processCaptureRequest(camera3_capture_request_t *request); > + > + bool isOpen() const { return open_; } This isn't used. > + unsigned int id() const { return id_; } > + camera3_device_t *device() { return &cameraDevice_; } > + > +private: > + void threadRpcCall(ThreadRpc &rpcRequest); > + > + bool open_; And neither is this, if you drop the isOpen() method. > + unsigned int id_; > + CameraModule *cameraModule_; > + camera3_device_t cameraDevice_; > +}; > + > +#endif /* __ANDROID_CAMERA_PROXY_H__ */ > diff --git a/src/android/meson.build b/src/android/meson.build > index 1f242953db37..63b45832c0d2 100644 > --- a/src/android/meson.build > +++ b/src/android/meson.build > @@ -1,3 +1,11 @@ > +android_hal_sources = files([ > + 'camera3_hal.cpp', > + 'camera_hal_manager.cpp', > + 'camera_module.cpp', > + 'camera_proxy.cpp', > + 'thread_rpc.cpp' > +]) > + > android_camera_metadata_sources = files([ > 'metadata/camera_metadata.c', > ]) > diff --git a/src/android/thread_rpc.cpp b/src/android/thread_rpc.cpp > new file mode 100644 > index 000000000000..f13db59d0d75 > --- /dev/null > +++ b/src/android/thread_rpc.cpp > @@ -0,0 +1,41 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2019, Google Inc. > + * > + * thread_call.cpp - Inter-thread procedure call thread_rpc.cpp > + */ > + > +#include "message.h" > +#include "thread_rpc.h" > + > +using namespace libcamera; > + > +libcamera::Message::Type ThreadRpcMessage::rpcType_ = Message::Type::None; > + > +ThreadRpcMessage::ThreadRpcMessage() > + : Message(type()) > +{ > +} > + > +void ThreadRpc::notifyReception() > +{ > + { > + libcamera::MutexLocker locker(mutex_); > + delivered_ = true; > + } > + cv_.notify_one(); > +} > + > +void ThreadRpc::waitDelivery() > +{ > + libcamera::MutexLocker locker(mutex_); > + cv_.wait(locker, [&]{ return delivered_; }); > +} > + > +Message::Type ThreadRpcMessage::type() > +{ > + if (ThreadRpcMessage::rpcType_ == Message::Type::None) > + rpcType_ = Message::registerMessageType(); > + > + return rpcType_; > +} > diff --git a/src/android/thread_rpc.h b/src/android/thread_rpc.h > new file mode 100644 > index 000000000000..173caba1a515 > --- /dev/null > +++ b/src/android/thread_rpc.h > @@ -0,0 +1,56 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2019, Google Inc. > + * > + * thread_call.h - Inter-thread procedure call > + */ > +#ifndef __ANDROID_THREAD_CALL_H__ > +#define __ANDROID_THREAD_CALL_H__ s/CALL/RPC/ > + > +#include <condition_variable> > +#include <string> Do you need string ? You should #include <mutex> > + > +#include <hardware/camera3.h> > +#include <libcamera/libcamera.h> > + > +#include "message.h" > +#include "thread.h" > + > +class ThreadRpc > +{ > +public: > + enum RpcTag { > + ProcessCaptureRequest, > + Close, > + }; > + > + ThreadRpc() > + : delivered_(false) {} > + > + void notifyReception(); > + void waitDelivery(); > + > + RpcTag tag; > + > + camera3_capture_request_t *request; > + > +private: > + bool delivered_; > + std::mutex mutex_; > + std::condition_variable cv_; > +}; > + > +class ThreadRpcMessage : public libcamera::Message > +{ > +public: > + ThreadRpcMessage(); > + ThreadRpc *rpc; > + > + static Message::Type type(); > + > +private: > + static libcamera::Message::Type rpcType_; > +}; FYI, I'll probably move part of this to the libcamera core. > + > +#endif /* __ANDROID_THREAD_CALL_H__ */ > + [snip]
Hi Laurent, On Wed, Aug 07, 2019 at 06:32:08PM +0300, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Tue, Aug 06, 2019 at 09:55:18PM +0200, Jacopo Mondi wrote: > > Add libcamera Android Camera HALv3 implementation. > > > > The initial camera HAL implementation supports the LIMITED hardware > > level and uses statically defined metadata and camera characteristics. > > > > Add a build option named 'android' and adjust the build system to > > selectively compile the Android camera HAL and link it against the > > required Android libraries. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > meson_options.txt | 5 + > > src/android/camera3_hal.cpp | 130 +++++ > > src/android/camera_hal_manager.cpp | 173 +++++++ > > src/android/camera_hal_manager.h | 56 ++ > > src/android/camera_module.cpp | 799 +++++++++++++++++++++++++++++ > > src/android/camera_module.h | 75 +++ > > src/android/camera_proxy.cpp | 181 +++++++ > > src/android/camera_proxy.h | 41 ++ > > src/android/meson.build | 8 + > > src/android/thread_rpc.cpp | 41 ++ > > src/android/thread_rpc.h | 56 ++ > > src/libcamera/meson.build | 9 + > > src/meson.build | 5 +- > > 13 files changed, 1578 insertions(+), 1 deletion(-) > > create mode 100644 src/android/camera3_hal.cpp > > create mode 100644 src/android/camera_hal_manager.cpp > > create mode 100644 src/android/camera_hal_manager.h > > create mode 100644 src/android/camera_module.cpp > > create mode 100644 src/android/camera_module.h > > create mode 100644 src/android/camera_proxy.cpp > > create mode 100644 src/android/camera_proxy.h > > create mode 100644 src/android/thread_rpc.cpp > > create mode 100644 src/android/thread_rpc.h > > [snip] > > > diff --git a/src/android/camera3_hal.cpp b/src/android/camera3_hal.cpp > > new file mode 100644 > > index 000000000000..0a97a9333d20 > > --- /dev/null > > +++ b/src/android/camera3_hal.cpp > > @@ -0,0 +1,130 @@ > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > +/* > > + * Copyright (C) 2019, Google Inc. > > + * > > + * camera3_hal.cpp - Android Camera3 HAL module > > + */ > > + > > +#include <iostream> > > +#include <memory> > > +#include <vector> > > + > > +#include <hardware/hardware.h> > > +#include <system/camera_metadata.h> > > Do you need camera_metadata.h ? Isn't it enough to include > camera_common.h ? I think hardware.h can also be dropped. camera_common.h includes camera_metadata.h, so I could just include the one I need. > > > + > > +#include <libcamera/libcamera.h> > > To speed up compilation, it would be better to include only the header > files we need (here and everywhere else). > Indeed > > + > > +#include "log.h" > > + > > +#include "camera_hal_manager.h" > > +#include "camera_module.h" > > +#include "camera_proxy.h" > > + > > +using namespace libcamera; > > + > > +LOG_DEFINE_CATEGORY(HAL) > > + > > +static CameraHalManager cameraManager; > > + > > +/*------------------------------------------------------------------------------ > > + * Android Camera HAL callbacks > > + */ > > + > > +static int hal_get_number_of_cameras(void) > > +{ > > + return cameraManager.numCameras(); > > +} > > + > > +static int hal_get_camera_info(int id, struct camera_info *info) > > +{ > > + return cameraManager.getCameraInfo(id, info); > > +} > > + > > +static int hal_set_callbacks(const camera_module_callbacks_t *callbacks) > > +{ > > + return 0; > > +} > > + > > +static int hal_open_legacy(const struct hw_module_t *module, const char *id, > > + uint32_t halVersion, struct hw_device_t **device) > > +{ > > + return -ENOSYS; > > +} > > + > > +static int hal_set_torch_mode(const char *camera_id, bool enabled) > > +{ > > + return -ENOSYS; > > +} > > + > > +/* > > + * First entry point of the camera HAL module. > > + * > > + * Initialize the HAL but does not open any camera device yet (see hal_dev_open) > > + */ > > +static int hal_init() > > +{ > > + LOG(HAL, Info) << "Initialising Android camera HAL"; > > + > > + cameraManager.initialize(); > > + > > + return 0; > > +} > > + > > +/*------------------------------------------------------------------------------ > > + * Android Camera Device > > + */ > > + > > +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 *cameraModule = reinterpret_cast<CameraProxy *>(dev->priv); > > s/cameraModule/proxy/ otherwise it's very confusing to have a > CameraProxy instance called cameraModule when there's a CameraModule > class. > > This comment applies to all other similar instances. > Sorry, leftover. I don't see any other module/proxy in this file > > + > > + return cameraManager.close(cameraModule); > > As cameraManager.close() only calls proxy->close(), how about defining > the hal_dev_close() method as a static method of CameraProxy, and > setting proxy->device()->common.close inside the CameraProxy constructor > ? It would avoid touching the internals of the CameraProxy in > hal_dev_open() below. > I liked having hal_dev_open and hal_dev_close here, as part of the HAL module. But yes, we can save one function call and poking into the device.common fields below.. > > +} > > + > > +static int hal_dev_open(const hw_module_t* module, const char* name, > > + hw_device_t** device) > > +{ > > + LOG(HAL, Debug) << "Open camera: " << name; > > s/camera:/camera/ > > > + > > + int id = atoi(name); > > + CameraProxy *proxy = cameraManager.open(id, module); > > + if (!proxy) { > > + LOG(HAL, Error) << "Failed to open camera module " << id; > > + return -ENODEV; > > + } > > + proxy->device()->common.close = hal_dev_close; > > + *device = &proxy->device()->common; > > + > > + return 0; > > +} > > Would it make sense to define all these methods as static methods of the > CameraHalManager class, and turn the class into a singleton with an > instance() method ? > Which methods? hal_dev_open? All the hal_dev_ ones which are used to initialize the camera_module_t ? I'm not sure I see a clear win... > > + > > +static struct hw_module_methods_t hal_module_methods = { > > + .open = hal_dev_open, > > +}; > > + > > +camera_module_t HAL_MODULE_INFO_SYM = { > > + .common = { > > + .tag = HARDWARE_MODULE_TAG, > > + .module_api_version = CAMERA_MODULE_API_VERSION_2_4, > > + .hal_api_version = HARDWARE_HAL_API_VERSION, > > + .id = CAMERA_HARDWARE_MODULE_ID, > > + .name = "Libcamera Camera3HAL Module", > > s/Libcamera/libcamera/ > > > + .author = "libcamera", > > + .methods = &hal_module_methods, > > + .dso = nullptr, > > + .reserved = {}, > > + }, > > + > > + .get_number_of_cameras = hal_get_number_of_cameras, > > + .get_camera_info = hal_get_camera_info, > > + .set_callbacks = hal_set_callbacks, > > + .get_vendor_tag_ops = nullptr, > > + .open_legacy = hal_open_legacy, > > + .set_torch_mode = hal_set_torch_mode, > > + .init = hal_init, > > + .reserved = {}, > > +}; > > diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp > > new file mode 100644 > > index 000000000000..734b5eebd9a5 > > --- /dev/null > > +++ b/src/android/camera_hal_manager.cpp > > @@ -0,0 +1,173 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2019, Google Inc. > > + * > > + * camera_hal_manager.cpp - Libcamera Android Camera Manager > > + */ > > + > > +#include "camera_hal_manager.h" > > + > > +#include <map> > > This shouldn't be needed. > Yes > > + > > +#include <hardware/hardware.h> > > +#include <system/camera_metadata.h> > > + > > +#include <libcamera/libcamera.h> > > +#include <libcamera/camera.h> > > + > > +#include "log.h" > > + > > +#include "camera_module.h" > > +#include "camera_proxy.h" > > + > > +using namespace libcamera; > > Missing blank line. > > > +LOG_DECLARE_CATEGORY(HAL); > > + > > +CameraHalManager::~CameraHalManager() > > +{ > > + for (auto metadata : staticMetadataMap_) > > + free_camera_metadata(metadata.second); > > +} > > + > > +int CameraHalManager::initialize() > > +{ > > + /* > > + * Thread::start() will create a std::thread which will execute > > + * CameraHalManager::run() > > + */ > > That's documenting the libcamera::Thread class (and the std::thread is > an internal implementation detail). I would just state > > /* > * Start the camera HAL manager thread and wait until its > * initialisation completes to be fully operational before > * receiving calls from the camera stack. > */ > > (thus dropping the next comment) > As you might have noticed, the camera HAL still has comments and annotations I added while developing, I'll clear them up better. > > + start(); > > + > > + /* > > + * Wait for run() to notify us before returning to the caller to > > + * make sure libcamera is fully initialized before we start handling > > + * calls from the camera stack. > > + */ > > + MutexLocker locker(mutex_); > > + cv_.wait(locker); > > + > > + return 0; > > +} > > + > > +void CameraHalManager::run() > > +{ > > + /* > > + * The run() operation is scheduled in its own thread; > > + * It exec() to run the even dispatcher loop and initialize libcamera. > > s/even/event/ > > > + * > > + * All the libcamera components initialized from here will be tied to > > + * the same thread as the here below run event dispatcher. > > The first paragraph also mostly documents the Thread class. I would drop > it, and write the second paragraph as > > /* > * All the libcamera components must be initialised here, in > * order to bind them to the camera HAL manager thread that > * executes the event dispatcher. > / > > > + */ > > + libcameraManager_ = libcamera::CameraManager::instance(); > > + > > + int ret = libcameraManager_->start(); > > + if (ret) { > > + LOG(HAL, Error) << "Failed to start camera manager: " > > + << strerror(-ret); > > + return; > > + } > > + > > + /* > > + * For each Camera registered in the system, a CameraModule > > + * get created here with an associated Camera and a proxy. > > s/get/gets/ > > > + * > > + * \todo Support camera hotplug. > > + */ > > + unsigned int cameraIndex = 0; > > Maybe just index ? > > > + for (auto camera : libcameraManager_->cameras()) { > > auto &camera > > otherwise camera will be a copy of the shared pointer instead of a > reference to it. > > > + cameras_.push_back(camera); > > The cameras_ vector isn't used after this. I would drop it, and store > the shared pointer in CameraModule. > Correct, I missed this in the the latest rework of the proxies lifecycle. > > + > > + CameraModule *module = new CameraModule(cameraIndex, > > + camera.get()); > > + modules_.emplace_back(module); > > + > > + CameraProxy *proxy = new CameraProxy(cameraIndex, > > + module); > > + proxies_.emplace_back(proxy); > > Similarly, how about sotring the module pointer inside the CameraProxy > class ? That would ensure that all accesses to the module go through the > proxy, which I think would be a good idea. As I use the modules_ only to access the map of static metadata which should now got away, I think it's doable > > CameraProxy *proxy = new CameraProxy(index, camera); > proxies_.emplace_back(proxy); > > > + > > + ++cameraIndex; > > + } > > + > > + /* > > + * libcamera has been initialized! Unlock the initialize() caller > > + * as we're now ready to handle calls from the framework. > > + */ > > + cv_.notify_one(); > > + > > + /* Now start processing events and messages! */ > > s/!/./ in the above two messages, there's no need to be > over-enthusiastic :-) > > > + exec(); > > +} > > + > > +CameraProxy *CameraHalManager::open(unsigned int id, > > + const hw_module_t *hardwareModule) > > +{ > > + if (id < 0 || id >= numCameras()) { > > + LOG(HAL, Error) << "Invalid camera id '" << id << "'"; > > + return nullptr; > > + } > > + > > + CameraProxy *proxy = proxies_[id].get(); > > + if (proxy->open(hardwareModule)) > > + return proxy; > > Shouldn't you return nullprtr here ? > Ideed > > + > > + LOG(HAL, Info) << "Camera: '" << id << "' opened"; > > s/Camera:/Camera/ > > > + > > + return proxy; > > +} > > + > > +int CameraHalManager::close(CameraProxy *proxy) > > +{ > > + proxy->close(); > > + LOG(HAL, Info) << "Close camera: " << proxy->id(); > > + > > + return 0; > > +} > > + > > +unsigned int CameraHalManager::numCameras() const > > +{ > > + return libcameraManager_->cameras().size(); > > +} > > + > > +/* > > + * Before the camera framework even tries to open a camera device with > > + * hal_dev_open() it requires the camera HAL to report a list of static > > + * informations on the camera device with id \a id in the hal_get_camera_info() > > + * method. > > + * > > + * The static metadata should be then generated probably from a > > + * platform-specific module, as we cannot operate on the camera at this time as > > + * it has not yet been open by the framework. > > s/open/opened/ > > What prevents us from opening the camera internally ? I don't think we It would complicate the lifecycle handling a bit, but I guess it's doable. AS of now, where all metadata are static it is not worth it imo > need a platform-specific module. > We surely won't be able to get from kernel at this point all the information we need, in example, the camera position. > > + * > > + * This is what the Intel XML file is used for, it is parsed and the data there > > + * combined with informations from the PSL service (which I -think- it's what > > + * our IPA is) to generate a list of static metadata per-camera device. > > I'd drop this paragraph. > > > + */ > > +int CameraHalManager::getCameraInfo(int id, struct camera_info *info) > > +{ > > + int cameras = numCameras(); > > + if (id >= cameras || id < 0 || !info) { > > Can we be called with info == nullptr ? I think I'd drop that part of > the check. All (most?) the extra-paranoid checks I have added here are error conditions tested by the cros_camera_test suite. > > > + LOG(HAL, Error) << "Invalid camera id: " << id; > > s/id:/id/ > > You may also want to add quotes around the value, as done in > CameraHalManager::open() (or drop them there). > > > + return -EINVAL; > > + } > > + > > + camera_metadata_t *staticMetadata; > > + auto it = staticMetadataMap_.find(id); > > + if (it != staticMetadataMap_.end()) { > > + staticMetadata = it->second; > > + } else { > > + CameraModule *cameraModule = modules_[id].get(); > > + > > + staticMetadata = cameraModule->getStaticMetadata(); > > + staticMetadataMap_[id] = staticMetadata; > > Why do you need the map, why can't you always retrieve the static > metadata from the CameraModule ? > I should, yes, so we can frop modules_ > > + } > > + > > + /* \todo: Get these info dynamically inspecting the camera module. */ > > s/:// > > > + info->facing = id ? CAMERA_FACING_FRONT : CAMERA_FACING_BACK; > > + info->orientation = 0; > > + info->device_version = 0; > > + info->resource_cost = 0; > > + info->static_camera_characteristics = staticMetadata; > > + info->conflicting_devices = nullptr; > > + info->conflicting_devices_length = 0; > > + > > + return 0; > > +} > > diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h > > new file mode 100644 > > index 000000000000..65d6fa3150ef > > --- /dev/null > > +++ b/src/android/camera_hal_manager.h > > @@ -0,0 +1,56 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2019, Google Inc. > > + * > > + * camera_hal_manager.h - Libcamera Android Camera Manager > > + */ > > +#ifndef __ANDROID_CAMERA_MANAGER_H__ > > +#define __ANDROID_CAMERA_MANAGER_H__ > > + > > +#include <condition_variable> > > +#include <cstddef> > > +#include <map> > > +#include <memory> > > +#include <vector> > > + > > +#include <hardware/hardware.h> > > +#include <system/camera_metadata.h> > > + > > +#include <libcamera/libcamera.h> > > + > > +#include "thread.h" > > +#include "thread_rpc.h" > > + > > +class CameraModule; > > +class CameraProxy; > > + > > +class CameraHalManager : public libcamera::Thread > > +{ > > +public: > > + ~CameraHalManager(); > > + > > + int initialize(); > > Our initialisation methods are usually called init(), not initialize(). ok > > > + > > + CameraProxy *open(unsigned int id, const hw_module_t *module); > > + int close(CameraProxy *proxy); > > + > > + unsigned int numCameras() const; > > + int getCameraInfo(int id, struct camera_info *info); > > + > > +private: > > + void run() override; > > + camera_metadata_t *getStaticMetadata(unsigned int id); > > + > > + libcamera::CameraManager *libcameraManager_; > > I think you can call this manager_. > > > + > > + std::mutex mutex_; > > + std::condition_variable cv_; > > + > > + std::vector<std::shared_ptr<libcamera::Camera>> cameras_; > > + std::vector<std::unique_ptr<CameraModule>> modules_; > > + std::vector<std::unique_ptr<CameraProxy>> proxies_; > > + > > + std::map<unsigned int, camera_metadata_t *> staticMetadataMap_; > > +}; > > + > > +#endif /* __ANDROID_CAMERA_MANAGER_H__ */ > > diff --git a/src/android/camera_module.cpp b/src/android/camera_module.cpp > > new file mode 100644 > > index 000000000000..b64e377fb439 > > --- /dev/null > > +++ b/src/android/camera_module.cpp > > @@ -0,0 +1,799 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2019, Google Inc. > > + * > > + * camera_module.cpp - Libcamera Android Camera Module > > + */ > > + > > +#include "camera_module.h" > > + > > +#include <iostream> > > +#include <memory> > > + > > +#include <system/camera_metadata.h> > > + > > +#include <hardware/camera3.h> > > I would group all the Android headers together, so > > #include <hardware/camera3.h> > #include <system/camera_metadata.h> > > #include <libcamera/libcamera.h> > > > +#include <libcamera/libcamera.h> > > + > > +#include "message.h" > > + > > +#include "log.h" > > #include "log.h" > #include "message.h" > > > + > > +using namespace libcamera; > > + > > +LOG_DECLARE_CATEGORY(HAL); > > + > > +CameraModule::CameraModule(unsigned int id, libcamera::Camera *camera) > > + : id_(id), camera_(camera), cameraState_(CameraStopped), > > + requestTemplate_(nullptr) > > +{ > > + camera_->requestCompleted.connect(this, > > + &CameraModule::requestComplete); > > This holds on a single line. > > > +} > > + > > +CameraModule::~CameraModule() > > +{ > > + if (requestTemplate_) > > + free_camera_metadata(requestTemplate_); > > + requestTemplate_ = nullptr; > > +} > > + > > +void CameraModule::message(Message *message) > > +{ > > + ThreadRpcMessage *rpcMessage = static_cast<ThreadRpcMessage *> > > + (message); > > + > > + if (rpcMessage->type() != ThreadRpcMessage::type()) > > + return Object::message(message); > > As Message may not be a ThreadRpcMessage, you should write this as > > if (message->type() != ThreadRpcMessage::type()) > return Object::message(message); > > ThreadRpcMessage *rpcMessage = static_cast<ThreadRpcMessage *>(message); > > > + > > + ThreadRpc *rpc = rpcMessage->rpc; > > + > > + switch (rpc->tag) { > > + case ThreadRpc::ProcessCaptureRequest: > > + processCaptureRequest(rpc->request); > > + break; > > + case ThreadRpc::Close: > > + close(); > > + break; > > + default: > > + LOG(HAL, Error) << "Unknown RPC operation: " << rpc->tag; > > + } > > + > > + rpc->notifyReception(); > > +} > > + > > +int CameraModule::open() > > +{ > > + int ret = camera_->acquire(); > > + if (ret) { > > + LOG(HAL, Error) << "Failed to acquire the camera"; > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +void CameraModule::close() > > +{ > > + camera_->stop(); > > + > > + camera_->freeBuffers(); > > + camera_->release(); > > + > > + cameraState_ = CameraStopped; > > +} > > + > > +void CameraModule::setCallbacks(const struct camera3_device *camera3Device, > > + const camera3_callback_ops_t *callbacks) > > +{ > > + camera3Device_ = camera3Device; > > camera3Device_ seems unused. > > > + callbacks_ = callbacks; > > +} > > + > > +camera_metadata_t *CameraModule::getStaticMetadata() > > +{ > > + int ret; > > + > > + /* > > + * The below metadata has been added by inspecting the Intel XML > > + * file and it's associated parsing routines in the Intel IPU3 HAL. > > + * > > + * The here below metadata are enough to satisfy the camera3-test > > + * provided by ChromeOS, but a real camera implementation might require > > s/might/will/ > > > + * more. > > + * > > + * See CameraConf::AiqConf class implementation in the Intel HAL > > + * to get a list of the static metadata registered by parsing the > > + * XML config file in AiqConf::fillStaticMetadataFromCMC > > + */ > > + > > + /* > > + * FIXME: this sizes come from the Intel IPU3 HAL, and are way too large for > > s/FIXME:/\todo/ > s/this/these/ > > > + * this intial HAL version. > > + */ > > + #define STATIC_ENTRY_CAP 256 > > + #define STATIC_DATA_CAP 6688 > > + camera_metadata_t *staticMetadata = > > + allocate_camera_metadata(STATIC_ENTRY_CAP, STATIC_DATA_CAP); > > + > > + /* Sensor static metadata. */ > > + int32_t pixelArraySize[] = { > > + 2592, 1944, > > + }; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE, > > + &pixelArraySize, 2); > > + METADATA_ASSERT(ret); > > + > > + int32_t sensorSizes[] = { > > + 0, 0, 2560, 1920, > > + }; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE, > > + &sensorSizes, 4); > > + METADATA_ASSERT(ret); > > + > > + int32_t sensitivityRange[] = { > > + 32, 2400, > > + }; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_SENSOR_INFO_SENSITIVITY_RANGE, > > + &sensitivityRange, 2); > > + METADATA_ASSERT(ret); > > + > > + uint16_t filterArr = ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT_GRBG; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT, > > + &filterArr, 1); > > + METADATA_ASSERT(ret); > > + > > + int64_t exposureTimeRange[] = { > > + 100000, 200000000, > > + }; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE, > > + &exposureTimeRange, 2); > > + METADATA_ASSERT(ret); > > + > > + int32_t orientation = 0; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_SENSOR_ORIENTATION, > > + &orientation, 1); > > + METADATA_ASSERT(ret); > > + > > + /* Flash static metadata. */ > > + char flashAvailable = ANDROID_FLASH_INFO_AVAILABLE_FALSE; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_FLASH_INFO_AVAILABLE, > > + &flashAvailable, 1); > > + METADATA_ASSERT(ret); > > + > > + /* Lens static metadata. */ > > + float fn = 2.53 / 100; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_LENS_INFO_AVAILABLE_APERTURES, &fn, 1); > > + METADATA_ASSERT(ret); > > + > > + /* Control metadata. */ > > + char controlMetadata = ANDROID_CONTROL_MODE_AUTO; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_CONTROL_AVAILABLE_MODES, > > + &controlMetadata, 1); > > + METADATA_ASSERT(ret); > > + > > + char availableAntiBandingModes[] = { > > + ANDROID_CONTROL_AE_ANTIBANDING_MODE_OFF, > > + ANDROID_CONTROL_AE_ANTIBANDING_MODE_50HZ, > > + ANDROID_CONTROL_AE_ANTIBANDING_MODE_60HZ, > > + ANDROID_CONTROL_AE_ANTIBANDING_MODE_AUTO, > > + }; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES, > > + availableAntiBandingModes, 4); > > + METADATA_ASSERT(ret); > > + > > + char aeAvailableModes[] = { > > + ANDROID_CONTROL_AE_MODE_ON, > > + ANDROID_CONTROL_AE_MODE_OFF, > > + }; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_CONTROL_AE_AVAILABLE_MODES, > > + aeAvailableModes, 2); > > + METADATA_ASSERT(ret); > > + > > + controlMetadata = ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_CONTROL_AE_LOCK_AVAILABLE, > > + &controlMetadata, 1); > > + METADATA_ASSERT(ret); > > + > > + uint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, > > + &awbLockAvailable, 1); > > + > > + /* Scaler static metadata. */ > > + std::vector<uint32_t> availableStreamFormats = { > > + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, > > + ANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, > > + ANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, > > + }; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_SCALER_AVAILABLE_FORMATS, > > + availableStreamFormats.data(), > > + availableStreamFormats.size()); > > + METADATA_ASSERT(ret); > > + > > + std::vector<uint32_t> availableStreamConfigurations = { > > + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, > > + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT, > > + ANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, 2560, 1920, > > + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT, > > + ANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, 2560, 1920, > > + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT, > > + }; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS, > > + availableStreamConfigurations.data(), > > + availableStreamConfigurations.size()); > > + METADATA_ASSERT(ret); > > + > > + std::vector<int64_t> availableStallDurations = { > > + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, 33333333, > > + }; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_SCALER_AVAILABLE_STALL_DURATIONS, > > + availableStallDurations.data(), > > + availableStallDurations.size()); > > + METADATA_ASSERT(ret); > > + > > + std::vector<int64_t> minFrameDurations = { > > + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, 33333333, > > + ANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, 2560, 1920, 33333333, > > + ANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, 2560, 1920, 33333333, > > + }; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS, > > + minFrameDurations.data(), minFrameDurations.size()); > > + METADATA_ASSERT(ret); > > + > > + /* Info static metadata. */ > > + uint8_t supportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED; > > + ret = add_camera_metadata_entry(staticMetadata, > > + ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL, > > + &supportedHWLevel, 1); > > + > > + return staticMetadata; > > +} > > + > > +const camera_metadata_t *CameraModule::constructDefaultRequestMetadata(int type) > > +{ > > + int ret; > > + > > + /* > > + * TODO: inspect type and pick the right metadata pack. > > s/TODO:/\todo/ > > > + * As of now just use a single one for all templates > > + */ > > + uint8_t captureIntent; > > + switch (type) { > > + case CAMERA3_TEMPLATE_PREVIEW: > > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_PREVIEW; > > + break; > > + case CAMERA3_TEMPLATE_STILL_CAPTURE: > > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_STILL_CAPTURE; > > + break; > > + case CAMERA3_TEMPLATE_VIDEO_RECORD: > > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_VIDEO_RECORD; > > + break; > > + case CAMERA3_TEMPLATE_VIDEO_SNAPSHOT: > > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_VIDEO_SNAPSHOT; > > + break; > > + case CAMERA3_TEMPLATE_ZERO_SHUTTER_LAG: > > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_ZERO_SHUTTER_LAG; > > + break; > > + case CAMERA3_TEMPLATE_MANUAL: > > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_MANUAL; > > + break; > > + default: > > + LOG(HAL, Error) << "Invalid template request type: " << type; > > + return nullptr; > > + } > > + > > + if (requestTemplate_) > > + return requestTemplate_; > > + > > + /* FIXME: this sizes are quite arbitrary ones */ > > + #define REQUEST_TEMPLATE_ENTRIES 30 > > + #define REQUEST_TEMPLATE_DATA 2048 > > + requestTemplate_ = allocate_camera_metadata(REQUEST_TEMPLATE_ENTRIES, > > + REQUEST_TEMPLATE_DATA); > > + if (!requestTemplate_) { > > + LOG(HAL, Error) << "Failed to allocate template metadata"; > > + return nullptr; > > + } > > + > > + /* > > + * Fill in the required Request metadata. > > + * > > + * Part (most?) of this entries comes from inspecting the Intel's IPU3 > > + * HAL xml configuration file. > > + */ > > + > > + /* Set to 0 the number of 'processed and stalling' streams (ie JPEG). */ > > + int32_t maxOutStream[] = { 0, 2, 0 }; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS, > > + maxOutStream, 3); > > + METADATA_ASSERT(ret); > > + > > + uint8_t maxPipelineDepth = 5; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_REQUEST_PIPELINE_MAX_DEPTH, > > + &maxPipelineDepth, 1); > > + METADATA_ASSERT(ret); > > + > > + int32_t inputStreams = 0; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_REQUEST_MAX_NUM_INPUT_STREAMS, > > + &inputStreams, 1); > > + METADATA_ASSERT(ret); > > + > > + int32_t partialResultCount = 1; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_REQUEST_PARTIAL_RESULT_COUNT, > > + &partialResultCount, 1); > > + METADATA_ASSERT(ret); > > + > > + uint8_t availableCapabilities[] = { > > + ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE, > > + }; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_REQUEST_AVAILABLE_CAPABILITIES, > > + availableCapabilities, 1); > > + METADATA_ASSERT(ret); > > + > > + uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_CONTROL_AE_MODE, > > + &aeMode, 1); > > + METADATA_ASSERT(ret); > > + > > + int32_t aeExposureCompensation = 0; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, > > + &aeExposureCompensation, 1); > > + METADATA_ASSERT(ret); > > + > > + uint8_t aePrecaptureTrigger = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, > > + &aePrecaptureTrigger, 1); > > + METADATA_ASSERT(ret); > > + > > + uint8_t aeLock = ANDROID_CONTROL_AE_LOCK_OFF; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_CONTROL_AE_LOCK, > > + &aeLock, 1); > > + METADATA_ASSERT(ret); > > + > > + uint8_t afTrigger = ANDROID_CONTROL_AF_TRIGGER_IDLE; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_CONTROL_AF_TRIGGER, > > + &afTrigger, 1); > > + METADATA_ASSERT(ret); > > + > > + uint8_t awbMode = ANDROID_CONTROL_AWB_MODE_AUTO; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_CONTROL_AWB_MODE, > > + &awbMode, 1); > > + METADATA_ASSERT(ret); > > + > > + uint8_t awbLock = ANDROID_CONTROL_AWB_LOCK_OFF; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_CONTROL_AWB_LOCK, > > + &awbLock, 1); > > + METADATA_ASSERT(ret); > > + > > + uint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, > > + &awbLockAvailable, 1); > > + METADATA_ASSERT(ret); > > + > > + uint8_t flashMode = ANDROID_FLASH_MODE_OFF; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_FLASH_MODE, > > + &flashMode, 1); > > + METADATA_ASSERT(ret); > > + > > + uint8_t faceDetectMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_STATISTICS_FACE_DETECT_MODE, > > + &faceDetectMode, 1); > > + METADATA_ASSERT(ret); > > + > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_CONTROL_CAPTURE_INTENT, > > + &captureIntent, 1); > > + METADATA_ASSERT(ret); > > + > > + /* > > + * This is quite hard to list at the moment wihtout knowing what > > + * we could control. > > + * > > + * For now, just list in the available Request keys and in the available > > + * result keys the control and reporting of the AE algorithm. > > + */ > > + std::vector<int32_t> availableRequestKeys = { > > + ANDROID_CONTROL_AE_MODE, > > + ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, > > + ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, > > + ANDROID_CONTROL_AE_LOCK, > > + ANDROID_CONTROL_AF_TRIGGER, > > + ANDROID_CONTROL_AWB_MODE, > > + ANDROID_CONTROL_AWB_LOCK, > > + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, > > + ANDROID_CONTROL_CAPTURE_INTENT, > > + ANDROID_FLASH_MODE, > > + ANDROID_STATISTICS_FACE_DETECT_MODE, > > + }; > > + > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS, > > + availableRequestKeys.data(), > > + availableRequestKeys.size()); > > + METADATA_ASSERT(ret); > > + > > + std::vector<int32_t> availableResultKeys = { > > + ANDROID_CONTROL_AE_MODE, > > + ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, > > + ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, > > + ANDROID_CONTROL_AE_LOCK, > > + ANDROID_CONTROL_AF_TRIGGER, > > + ANDROID_CONTROL_AWB_MODE, > > + ANDROID_CONTROL_AWB_LOCK, > > + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, > > + ANDROID_CONTROL_CAPTURE_INTENT, > > + ANDROID_FLASH_MODE, > > + ANDROID_STATISTICS_FACE_DETECT_MODE, > > + }; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_REQUEST_AVAILABLE_RESULT_KEYS, > > + availableResultKeys.data(), > > + availableResultKeys.size()); > > + METADATA_ASSERT(ret); > > + > > + /* > > + * The available characteristics should be the tags reported > > + * as part of the static metadata reported at hal_get_camera_info() > > + * time. The xml file sets those to 0 though. > > + */ > > + std::vector<int32_t> availableCharacteristicsKeys = {}; > > + ret = add_camera_metadata_entry(requestTemplate_, > > + ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS, > > + availableCharacteristicsKeys.data(), > > + availableCharacteristicsKeys.size()); > > + METADATA_ASSERT(ret); > > + > > + return requestTemplate_; > > +} > > + > > +/** > > + * Inspect the stream_list to produce a list of StreamConfiguration to > > + * be use to configure the Camera. > > + */ > > +int CameraModule::configureStreams(camera3_stream_configuration_t *stream_list) > > +{ > > + > > Extra blank line. > > > + for (unsigned int i = 0; i < stream_list->num_streams; ++i) { > > + camera3_stream_t *stream = stream_list->streams[i]; > > + > > + LOG(HAL, Info) << "Stream #" << i > > + << ": direction: " << stream->stream_type > > + << " - width: " << stream->width > > + << " - height: " << stream->height > > + << " - format: " << std::hex << stream->format; > > s/ -/,/ in the above three lines > > And I would print size: ...x... instead of width and height. > > > + } > > + > > + /* Hardcode viewfinder role, collecting sizes from the stream config. */ > > + if (stream_list->num_streams != 1) { > > + LOG(HAL, Error) << "Only one stream supported"; > > + return -EINVAL; > > + } > > + > > + StreamRoles roles = {}; > > + roles.push_back(StreamRole::StillCapture); > > I think you can write this as > > StreamRoles roles{ StreamRole::StillCapture }; > > > + > > + config_ = camera_->generateConfiguration(roles); > > + if (!config_ || config_->empty()) { > > + LOG(HAL, Error) << "Failed to generate camera configuration"; > > + return -EINVAL; > > + } > > + > > + /* Only one stream is supported. */ > > + camera3_stream_t *camera3Stream = stream_list->streams[0]; > > + StreamConfiguration *streamConfiguration = &config_->at(0); > > + streamConfiguration->size.width = camera3Stream->width; > > + streamConfiguration->size.height = camera3Stream->height; > > + streamConfiguration->memoryType = ExternalMemory; > > + > > + /* > > + * FIXME: do not change the pixel format from the one returned > > + * from the Camera::generateConfiguration(); > > + * > > + * We'll need to translate from Android defined pixel format codes > > + * to whatever libcamera will use. > > + */ > > + > > + switch (config_->validate()) { > > + case CameraConfiguration::Valid: > > + break; > > + case CameraConfiguration::Adjusted: > > + LOG(HAL, Info) << "Camera configuration adjusted"; > > Missing config_.reset() ? > > > + return -EINVAL; > > + case CameraConfiguration::Invalid: > > + LOG(HAL, Info) << "Camera configuration invalid"; > > + config_.reset(); > > + return -EINVAL; > > + } > > + > > + camera3Stream->max_buffers = streamConfiguration->bufferCount; > > + > > + /* > > + * Once the CameraConfiguration has been adjusted/validated > > + * it can be applied to the camera. > > + */ > > + int ret = camera_->configure(config_.get()); > > + if (ret) { > > + LOG(HAL, Error) << "Failed to configure camera '" > > + << camera_->name() << "'"; > > + return ret; > > + } > > + > > + return 0; > > +} > > + > > +int CameraModule::processCaptureRequest(camera3_capture_request_t *camera3Request) > > +{ > > + StreamConfiguration *streamConfiguration = &config_->at(0); > > + Stream *stream = streamConfiguration->stream(); > > + > > + if (camera3Request->num_output_buffers != 1) { > > + LOG(HAL, Error) << "Invalid number of output buffers: " > > + << camera3Request->num_output_buffers; > > + return -EINVAL; > > + } > > + > > + /* Start the camera if that's the first request we handle. */ > > + if (cameraState_ == CameraStopped) { > > + int ret = camera_->allocateBuffers(); > > + if (ret) { > > + LOG(HAL, Error) << "Failed to allocate buffers"; > > + return ret; > > + } > > + > > + ret = camera_->start(); > > + if (ret) { > > + LOG(HAL, Error) << "Failed to start camera"; > > + camera_->freeBuffers(); > > + return ret; > > + } > > + > > + cameraState_ = CameraRunning; > > + } > > + > > + /* > > + * Queue a request for the Camera with the provided dmabuf file > > + * descriptors. > > + */ > > + const camera3_stream_buffer_t *camera3Buffer = > > + &camera3Request->output_buffers[0]; > > + const buffer_handle_t camera3Handle = *camera3Buffer->buffer; > > + > > + std::array<int, 3> fds = { > > + camera3Handle->data[0], > > + camera3Handle->data[1], > > + camera3Handle->data[2], > > + }; > > + std::unique_ptr<Buffer> buffer = stream->createBuffer(fds); > > + if (!buffer) { > > + LOG(HAL, Error) << "Failed to create buffer"; > > + return -EINVAL; > > + } > > + > > + Request *request = camera_->createRequest(); > > + request->addBuffer(std::move(buffer)); > > + int ret = camera_->queueRequest(request); > > + if (ret) { > > + LOG(HAL, Error) << "Failed to queue request"; > > + return ret; > > + } > > + > > + /* Save the request descriptors for use at completion time. */ > > + Camera3RequestDescriptor descriptor = {}; > > + descriptor.frameNumber = camera3Request->frame_number, > > + descriptor.numBuffers = camera3Request->num_output_buffers, > > s/,$/;/ for the two lines above. > > I don't think you need to store numBuffers in the descriptors, it can be > retrieved from buffers.size() in requestComplete(). numBuffers is the framework required number of buffers, buffers.size() is the number of buffers completed in the request. They -should- be the same, but I would keep them separate to make sure the completed request is correct. > > > > + descriptor.buffers = new camera3_stream_buffer_t[descriptor.numBuffers]; > > + for (unsigned int i = 0; i < descriptor.numBuffers; ++i) { > > + camera3_stream_buffer_t &buffer = descriptor.buffers[i]; > > + buffer = camera3Request->output_buffers[i]; > > Just > > descriptor.buffers[i] = camera3Request->output_buffers[i]; > > + } > > + > > + requestMap_[request] = descriptor; > > How about using the request cookie (passed to Camera::createRequest()) > for this purpose ? > Can we do that on top? > > + > > + return 0; > > +} > > + > > +void CameraModule::requestComplete(Request *request, > > + const std::map<Stream *, Buffer *> &buffers) > > +{ > > + Buffer *libcameraBuffer = buffers.begin()->second; > > + camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; > > + camera_metadata_t *resultMetadata = nullptr; > > + > > + if (request->status() != Request::RequestComplete) { > > + LOG(HAL, Error) << "Request not succesfully completed: " > > + << request->status(); > > + status = CAMERA3_BUFFER_STATUS_ERROR; > > + } > > + > > + /* Prepare to call back the Android camera stack. */ > > + Camera3RequestDescriptor &descriptor = requestMap_[request]; > > + > > + camera3_capture_result_t captureResult = {}; > > + captureResult.frame_number = descriptor.frameNumber; > > + captureResult.num_output_buffers = descriptor.numBuffers; > > + > > + camera3_stream_buffer_t *resultBuffers = > > + new camera3_stream_buffer_t[descriptor.numBuffers]; > > + for (unsigned int i = 0; i < descriptor.numBuffers; ++i) { > > + camera3_stream_buffer_t resultBuffer; > > + > > + resultBuffer = descriptor.buffers[i]; > > + resultBuffer.acquire_fence = -1; > > + resultBuffer.release_fence = -1; > > + resultBuffer.status = status; > > + > > + resultBuffers[i] = resultBuffer; > > + } > > + captureResult.output_buffers = > > + const_cast<const camera3_stream_buffer_t *>(resultBuffers); > > + > > + if (status == CAMERA3_BUFFER_STATUS_ERROR) { > > + /* \todo Improve error handling. */ > > + notifyError(descriptor.frameNumber, resultBuffers->stream); > > + } else { > > + notifyShutter(descriptor.frameNumber, libcameraBuffer->timestamp()); > > + > > + captureResult.partial_result = 1; > > + resultMetadata = getResultMetadata(descriptor.frameNumber, > > + libcameraBuffer->timestamp()); > > + captureResult.result = resultMetadata; > > + } > > + > > + callbacks_->process_capture_result(callbacks_, &captureResult); > > + > > + if (resultMetadata) > > + free_camera_metadata(resultMetadata); > > + > > + delete[] resultBuffers; > > + delete[] descriptor.buffers; > > + requestMap_.erase(request); > > + > > + return; > > +} > > + > > +void CameraModule::notifyShutter(uint32_t frameNumber, uint64_t timestamp) > > +{ > > + camera3_notify_msg_t notify = {}; > > + > > + notify.type = CAMERA3_MSG_SHUTTER; > > + notify.message.shutter.frame_number = frameNumber; > > + notify.message.shutter.timestamp = timestamp; > > + > > + callbacks_->notify(callbacks_, ¬ify); > > +} > > + > > +void CameraModule::notifyError(uint32_t frameNumber, camera3_stream_t *stream) > > +{ > > + camera3_notify_msg_t notify = {}; > > + > > + notify.type = CAMERA3_MSG_ERROR; > > + notify.message.error.error_stream = stream; > > + notify.message.error.frame_number = frameNumber; > > + notify.message.error.error_code = CAMERA3_MSG_ERROR_REQUEST; > > + > > + callbacks_->notify(callbacks_, ¬ify); > > +} > > + > > +/* > > + * Fixed result metadata, mostly imported from the UVC camera HAL, which > > + * does not produces metadata and thus needs to generate a fixed set. > > + */ > > +camera_metadata_t *CameraModule::getResultMetadata(int frame_number, > > + int64_t timestamp) > > +{ > > + int ret; > > + > > + /* FIXME: random "big enough" values. */ > > + #define RESULT_ENTRY_CAP 256 > > + #define RESULT_DATA_CAP 6688 > > + camera_metadata_t *resultMetadata = > > + allocate_camera_metadata(STATIC_ENTRY_CAP, STATIC_DATA_CAP); > > + > > + const uint8_t ae_state = ANDROID_CONTROL_AE_STATE_CONVERGED; > > + ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AE_STATE, > > + &ae_state, 1); > > + METADATA_ASSERT(ret); > > + > > + const uint8_t ae_lock = ANDROID_CONTROL_AE_LOCK_OFF; > > + ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AE_LOCK, > > + &ae_lock, 1); > > + METADATA_ASSERT(ret); > > + > > + uint8_t af_state = ANDROID_CONTROL_AF_STATE_INACTIVE; > > + ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AF_STATE, > > + &af_state, 1); > > + METADATA_ASSERT(ret); > > + > > + const uint8_t awb_state = ANDROID_CONTROL_AWB_STATE_CONVERGED; > > + ret = add_camera_metadata_entry(resultMetadata, > > + ANDROID_CONTROL_AWB_STATE, > > + &awb_state, 1); > > + METADATA_ASSERT(ret); > > + > > + const uint8_t awb_lock = ANDROID_CONTROL_AWB_LOCK_OFF; > > + ret = add_camera_metadata_entry(resultMetadata, > > + ANDROID_CONTROL_AWB_LOCK, > > + &awb_lock, 1); > > + METADATA_ASSERT(ret); > > + > > + const uint8_t lens_state = ANDROID_LENS_STATE_STATIONARY; > > + ret = add_camera_metadata_entry(resultMetadata, > > + ANDROID_LENS_STATE, > > + &lens_state, 1); > > + METADATA_ASSERT(ret); > > + > > + int32_t sensorSizes[] = { > > + 0, 0, 2560, 1920, > > + }; > > + ret = add_camera_metadata_entry(resultMetadata, > > + ANDROID_SCALER_CROP_REGION, > > + sensorSizes, 4); > > + METADATA_ASSERT(ret); > > + > > + ret = add_camera_metadata_entry(resultMetadata, > > + ANDROID_SENSOR_TIMESTAMP, > > + ×tamp, 1); > > + > > + METADATA_ASSERT(ret); > > + > > + /* 33.3 msec */ > > + const int64_t rolling_shutter_skew = 33300000; > > + ret = add_camera_metadata_entry(resultMetadata, > > + ANDROID_SENSOR_ROLLING_SHUTTER_SKEW, > > + &rolling_shutter_skew, 1); > > + METADATA_ASSERT(ret); > > + > > + /* 16.6 msec */ > > + const int64_t exposure_time = 16600000; > > + ret = add_camera_metadata_entry(resultMetadata, > > + ANDROID_SENSOR_EXPOSURE_TIME, > > + &exposure_time, 1); > > + METADATA_ASSERT(ret); > > + > > + const uint8_t lens_shading_map_mode = > > + ANDROID_STATISTICS_LENS_SHADING_MAP_MODE_OFF; > > + ret = add_camera_metadata_entry(resultMetadata, > > + ANDROID_STATISTICS_LENS_SHADING_MAP_MODE, > > + &lens_shading_map_mode, 1); > > + METADATA_ASSERT(ret); > > + > > + const uint8_t scene_flicker = ANDROID_STATISTICS_SCENE_FLICKER_NONE; > > + ret = add_camera_metadata_entry(resultMetadata, > > + ANDROID_STATISTICS_SCENE_FLICKER, > > + &scene_flicker, 1); > > + METADATA_ASSERT(ret); > > + > > + return resultMetadata; > > +} > > diff --git a/src/android/camera_module.h b/src/android/camera_module.h > > new file mode 100644 > > index 000000000000..67488d5940b1 > > --- /dev/null > > +++ b/src/android/camera_module.h > > @@ -0,0 +1,75 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2019, Google Inc. > > + * > > + * camera_module.h - Libcamera Android Camera Module > > s/Libcamera/libcamera/ > > (here and everywhere else, libcamera is always written in all lowercase) > > > + */ > > +#ifndef __ANDROID_CAMERA_MODULE_H__ > > +#define __ANDROID_CAMERA_MODULE_H__ > > + > > +#include <memory> > > + > > +#include <hardware/camera3.h> > > +#include <libcamera/libcamera.h> > > + > > +#include "message.h" > > + > > +#include "camera_hal_manager.h" > > + > > +#define METADATA_ASSERT(_r) \ > > + do { \ > > + if (!(_r)) break; \ > > + LOG(HAL, Error) << "Error: " << __func__ << ":" << __LINE__; \ > > + return nullptr; \ > > + } while(0); > > I really, really dislike hiding return statements in macros. We can keep > it for now, but please add a comment telling that it should be removed. > All the metadata handling will be nuked in the next versions, so I won't bother for now. > > + > > Even if we don't document the whole implementation with Doxygen, I think > a few short comments that explain what each class models would be > useful. Otherwise the reader is left to wonder what CameraModule or > CameraProxy stand for. A comment that explains how the various objects > work together would be useful too. > > > +class CameraModule : public libcamera::Object > > I don't think CameraModule is a good name :-/ In HAL terminology, the > module refers to camera_module_t, and corresponds more or less to > libcamera::CameraManager, not libcamera::Camera. Correct > > One option would be to name this class Camera in a HAL namespace. That > would make it quite clear that it corresponds to a libcamra::Camera. I had a go at isolating namespaces and I have problems with the logging infrastructure, and I would not duplicate the Camera name to be honest. CameraDevice ? > > > +{ > > +public: > > + CameraModule(unsigned int id, libcamera::Camera *camera); > > + ~CameraModule(); > > + > > + void message(libcamera::Message *message); > > + > > + int open(); > > + void close(); > > + void setCallbacks(const struct camera3_device *cameraDevice, > > + const camera3_callback_ops_t *callbacks); > > + camera_metadata_t *getStaticMetadata(); > > + const camera_metadata_t *constructDefaultRequestMetadata(int type); > > + int configureStreams(camera3_stream_configuration_t *stream_list); > > + int processCaptureRequest(camera3_capture_request_t *request); > > + void requestComplete(libcamera::Request *request, > > + const std::map<libcamera::Stream *, libcamera::Buffer *> &buffers); > > + > > + unsigned int id() const { return id_; } > > I think this method is not used. > > > + > > +private: > > + struct Camera3RequestDescriptor { > > + uint32_t frameNumber; > > + uint32_t numBuffers; > > + camera3_stream_buffer_t *buffers; > > + }; > > + > > + enum CameraState { > > + CameraStopped, > > + CameraRunning, > > + }; > > With just two states you could instead have a bool running_ field. > > > + > > + void notifyShutter(uint32_t frameNumber, uint64_t timestamp); > > + void notifyError(uint32_t frameNumber, camera3_stream_t *stream); > > + camera_metadata_t *getResultMetadata(int frame_number, int64_t timestamp); > > + > > + unsigned int id_; > > + libcamera::Camera *camera_; > > + std::unique_ptr<libcamera::CameraConfiguration> config_; > > + CameraState cameraState_; > > + > > + camera_metadata_t *requestTemplate_; > > + const camera3_callback_ops_t *callbacks_; > > + const struct camera3_device *camera3Device_; > > + > > + std::map<libcamera::Request *, Camera3RequestDescriptor> requestMap_; > > +}; > > + > > +#endif /* __ANDROID_CAMERA_MODULE_H__ */ > > diff --git a/src/android/camera_proxy.cpp b/src/android/camera_proxy.cpp > > new file mode 100644 > > index 000000000000..af7817f29137 > > --- /dev/null > > +++ b/src/android/camera_proxy.cpp > > @@ -0,0 +1,181 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2019, Google Inc. > > + * > > + * camera_proxy.cpp - Proxy to camera module instances > > + */ > > + > > +#include "camera_proxy.h" > > + > > +#include <hardware/camera3.h> > > +#include <libcamera/libcamera.h> > > +#include <system/camera_metadata.h> > > + > > +#include <memory> > > + > > +#include "log.h" > > +#include "message.h" > > +#include "thread_rpc.h" > > +#include "utils.h" > > + > > +using namespace libcamera; > > + > > +LOG_DECLARE_CATEGORY(HAL); > > + > > +#define LIBCAMERA_HAL_FUNC_DBG \ > > + LOG(HAL, Debug); > > If we need to trace calls, I think something better than debug messages > would be useful. I'm not sure what yet though. > Yeah, I'm not even sure we want to trace calls actually. The fact is that the camera HAL is somewhat a weird beast, as it is a very system specific libcamera extension, and I tried to mimic what is usually found in other HAL implementations I've seen. Should we drop tracing completely and make this more libcamera-like ? > > + > > +static int hal_dev_initialize(const struct camera3_device *dev, > > + const camera3_callback_ops_t *callback_ops) > > +{ > > + LIBCAMERA_HAL_FUNC_DBG > > + > > + if (!dev) > > + return -EINVAL; > > Can this happen ? I think cros_camera_test exercises this. > > > + > > + CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); > > + proxy->setCallbacks(callback_ops); > > + > > + return 0; > > +} > > + > > +static int hal_dev_configure_streams(const struct camera3_device *dev, > > + camera3_stream_configuration_t *stream_list) > > +{ > > + LIBCAMERA_HAL_FUNC_DBG > > + > > + if (!dev) > > + return -EINVAL; > > + > > + CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); > > + proxy->configureStreams(stream_list); > > + > > + return 0; > > Shouldn't you propagate the error value from proxy->configureStreams() ? > > > +} > > + > > +static const camera_metadata_t * > > +hal_dev_construct_default_request_settings(const struct camera3_device *dev, > > + int type) > > +{ > > + LIBCAMERA_HAL_FUNC_DBG > > + > > + if (!dev) > > + return nullptr; > > + > > + CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); > > + return proxy->constructDefaultRequest(type); > > How about renaming constructDefaultRequest() and > constructDefaultRequestMetadata() to constructDefaultRequestSettings(), > in order to match the HAL operation name ? Ok... > > > +} > > + > > +static int hal_dev_process_capture_request(const struct camera3_device *dev, > > + camera3_capture_request_t *request) > > +{ > > + LIBCAMERA_HAL_FUNC_DBG > > + > > + 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) > > +{ > > + LIBCAMERA_HAL_FUNC_DBG > > +} > > + > > +static int hal_dev_flush(const struct camera3_device *dev) > > +{ > > + LIBCAMERA_HAL_FUNC_DBG > > + > > + return 0; > > +} > > You could define these methods as static methods of the CameraProxy > class. For instance, hal_dev_configure_streams would become > > int CameraProxy::configureStreams(const struct camera3_device *dev, > camera3_stream_configuration_t *stream_list) > { > CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); > proxy->configureStreams(stream_list); > } To avoid the static C methods you mean ? Not sure what is the gain though. > > > + > > +static camera3_device_ops hal_dev_ops = { > > I wish this could be const :-( > > > + .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 = { 0 }, > > s/0/nullptr/ ? > > > +}; > > + > > +CameraProxy::CameraProxy(unsigned int id, CameraModule *cameraModule) > > + : open_(false), id_(id), cameraModule_(cameraModule) > > +{ > > +} > > + > > +int CameraProxy::open(const hw_module_t *hardwareModule) > > +{ > > + int ret = cameraModule_->open(); > > + if (ret) > > + return ret; > > + > > + /* Initialize the hw_device_t in the instance camera3_module_t. */ > > + cameraDevice_.common.tag = HARDWARE_DEVICE_TAG; > > + cameraDevice_.common.version = CAMERA_DEVICE_API_VERSION_3_3; > > + cameraDevice_.common.module = (hw_module_t *)hardwareModule; > > + > > + /* > > + * The camera device operations. These actually implement > > + * the Android Camera HALv3 interface. > > + */ > > + cameraDevice_.ops = &hal_dev_ops; > > + cameraDevice_.priv = this; > > + > > + open_ = true; > > + > > + return 0; > > +} > > + > > +void CameraProxy::close() > > +{ > > + LIBCAMERA_HAL_FUNC_DBG > > + > > + ThreadRpc rpcRequest; > > + rpcRequest.tag = ThreadRpc::Close; > > + > > + threadRpcCall(rpcRequest); > > + > > + open_ = false; > > +} > > + > > +void CameraProxy::setCallbacks(const camera3_callback_ops_t *callbacks) > > I would name this initialize() to match hal_dev_initialize(). > > > +{ > > + LIBCAMERA_HAL_FUNC_DBG > > + > > + cameraModule_->setCallbacks(&cameraDevice_, callbacks); > > +} > > + > > +const camera_metadata_t *CameraProxy::constructDefaultRequest(int type) > > +{ > > + return cameraModule_->constructDefaultRequestMetadata(type); > > +} > > + > > +int CameraProxy::configureStreams(camera3_stream_configuration_t *stream_list) > > +{ > > + return cameraModule_->configureStreams(stream_list); > > +} > > + > > +int CameraProxy::processCaptureRequest(camera3_capture_request_t *request) > > +{ > > + ThreadRpc rpcRequest; > > + rpcRequest.tag = ThreadRpc::ProcessCaptureRequest; > > + rpcRequest.request = request; > > + > > + threadRpcCall(rpcRequest); > > + > > + return 0; > > Does this method need to be synchronous ? We can keep it as-is for now, > but I wonder if it wouldn't make sense to later move (part of) the > validation code here and make the call asynchronous. > > > +} > > + > > +void CameraProxy::threadRpcCall(ThreadRpc &rpcRequest) > > +{ > > + std::unique_ptr<ThreadRpcMessage> message = > > + utils::make_unique<ThreadRpcMessage>(); > > + message->rpc = &rpcRequest; > > + > > + cameraModule_->postMessage(std::move(message)); > > + rpcRequest.waitDelivery(); > > +} > > diff --git a/src/android/camera_proxy.h b/src/android/camera_proxy.h > > new file mode 100644 > > index 000000000000..69e6878c4352 > > --- /dev/null > > +++ b/src/android/camera_proxy.h > > @@ -0,0 +1,41 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2019, Google Inc. > > + * > > + * camera_proxy.h - Proxy to camera module instances > > + */ > > +#ifndef __ANDROID_CAMERA_PROXY_H__ > > +#define __ANDROID_CAMERA_PROXY_H__ > > + > > +#include <hardware/camera3.h> > > +#include <libcamera/libcamera.h> > > + > > +#include "camera_module.h" > > + > > +class CameraProxy > > +{ > > +public: > > + CameraProxy(unsigned int id, CameraModule *cameraModule); > > + > > + int open(const hw_module_t *hardwareModule); > > + void close(); > > + > > + void setCallbacks(const camera3_callback_ops_t *callbacks); > > + const camera_metadata_t *constructDefaultRequest(int type); > > + int configureStreams(camera3_stream_configuration_t *stream_list); > > + int processCaptureRequest(camera3_capture_request_t *request); > > + > > + bool isOpen() const { return open_; } > > This isn't used. > > > + unsigned int id() const { return id_; } > > + camera3_device_t *device() { return &cameraDevice_; } > > + > > +private: > > + void threadRpcCall(ThreadRpc &rpcRequest); > > + > > + bool open_; > > And neither is this, if you drop the isOpen() method. > > > + unsigned int id_; > > + CameraModule *cameraModule_; > > + camera3_device_t cameraDevice_; > > +}; > > + > > +#endif /* __ANDROID_CAMERA_PROXY_H__ */ > > diff --git a/src/android/meson.build b/src/android/meson.build > > index 1f242953db37..63b45832c0d2 100644 > > --- a/src/android/meson.build > > +++ b/src/android/meson.build > > @@ -1,3 +1,11 @@ > > +android_hal_sources = files([ > > + 'camera3_hal.cpp', > > + 'camera_hal_manager.cpp', > > + 'camera_module.cpp', > > + 'camera_proxy.cpp', > > + 'thread_rpc.cpp' > > +]) > > + > > android_camera_metadata_sources = files([ > > 'metadata/camera_metadata.c', > > ]) > > diff --git a/src/android/thread_rpc.cpp b/src/android/thread_rpc.cpp > > new file mode 100644 > > index 000000000000..f13db59d0d75 > > --- /dev/null > > +++ b/src/android/thread_rpc.cpp > > @@ -0,0 +1,41 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2019, Google Inc. > > + * > > + * thread_call.cpp - Inter-thread procedure call > > thread_rpc.cpp > > > + */ > > + > > +#include "message.h" > > +#include "thread_rpc.h" > > + > > +using namespace libcamera; > > + > > +libcamera::Message::Type ThreadRpcMessage::rpcType_ = Message::Type::None; > > + > > +ThreadRpcMessage::ThreadRpcMessage() > > + : Message(type()) > > +{ > > +} > > + > > +void ThreadRpc::notifyReception() > > +{ > > + { > > + libcamera::MutexLocker locker(mutex_); > > + delivered_ = true; > > + } > > + cv_.notify_one(); > > +} > > + > > +void ThreadRpc::waitDelivery() > > +{ > > + libcamera::MutexLocker locker(mutex_); > > + cv_.wait(locker, [&]{ return delivered_; }); > > +} > > + > > +Message::Type ThreadRpcMessage::type() > > +{ > > + if (ThreadRpcMessage::rpcType_ == Message::Type::None) > > + rpcType_ = Message::registerMessageType(); > > + > > + return rpcType_; > > +} > > diff --git a/src/android/thread_rpc.h b/src/android/thread_rpc.h > > new file mode 100644 > > index 000000000000..173caba1a515 > > --- /dev/null > > +++ b/src/android/thread_rpc.h > > @@ -0,0 +1,56 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2019, Google Inc. > > + * > > + * thread_call.h - Inter-thread procedure call > > + */ > > +#ifndef __ANDROID_THREAD_CALL_H__ > > +#define __ANDROID_THREAD_CALL_H__ > > s/CALL/RPC/ > > > + > > +#include <condition_variable> > > +#include <string> > > Do you need string ? > > You should #include <mutex> > > > + > > +#include <hardware/camera3.h> > > +#include <libcamera/libcamera.h> > > + > > +#include "message.h" > > +#include "thread.h" > > + > > +class ThreadRpc > > +{ > > +public: > > + enum RpcTag { > > + ProcessCaptureRequest, > > + Close, > > + }; > > + > > + ThreadRpc() > > + : delivered_(false) {} > > + > > + void notifyReception(); > > + void waitDelivery(); > > + > > + RpcTag tag; > > + > > + camera3_capture_request_t *request; > > + > > +private: > > + bool delivered_; > > + std::mutex mutex_; > > + std::condition_variable cv_; > > +}; > > + > > +class ThreadRpcMessage : public libcamera::Message > > +{ > > +public: > > + ThreadRpcMessage(); > > + ThreadRpc *rpc; > > + > > + static Message::Type type(); > > + > > +private: > > + static libcamera::Message::Type rpcType_; > > +}; > > FYI, I'll probably move part of this to the libcamera core. > Let's merge the HAL first, if you're not working on this already please. Thanks j > > + > > +#endif /* __ANDROID_THREAD_CALL_H__ */ > > + > > [snip] > > -- > Regards, > > Laurent Pinchart
Hi Jacopo, On Thu, Aug 08, 2019 at 05:38:15PM +0200, Jacopo Mondi wrote: > On Wed, Aug 07, 2019 at 06:32:08PM +0300, Laurent Pinchart wrote: > > On Tue, Aug 06, 2019 at 09:55:18PM +0200, Jacopo Mondi wrote: > > > Add libcamera Android Camera HALv3 implementation. > > > > > > The initial camera HAL implementation supports the LIMITED hardware > > > level and uses statically defined metadata and camera characteristics. > > > > > > Add a build option named 'android' and adjust the build system to > > > selectively compile the Android camera HAL and link it against the > > > required Android libraries. > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > --- > > > meson_options.txt | 5 + > > > src/android/camera3_hal.cpp | 130 +++++ > > > src/android/camera_hal_manager.cpp | 173 +++++++ > > > src/android/camera_hal_manager.h | 56 ++ > > > src/android/camera_module.cpp | 799 +++++++++++++++++++++++++++++ > > > src/android/camera_module.h | 75 +++ > > > src/android/camera_proxy.cpp | 181 +++++++ > > > src/android/camera_proxy.h | 41 ++ > > > src/android/meson.build | 8 + > > > src/android/thread_rpc.cpp | 41 ++ > > > src/android/thread_rpc.h | 56 ++ > > > src/libcamera/meson.build | 9 + > > > src/meson.build | 5 +- > > > 13 files changed, 1578 insertions(+), 1 deletion(-) > > > create mode 100644 src/android/camera3_hal.cpp > > > create mode 100644 src/android/camera_hal_manager.cpp > > > create mode 100644 src/android/camera_hal_manager.h > > > create mode 100644 src/android/camera_module.cpp > > > create mode 100644 src/android/camera_module.h > > > create mode 100644 src/android/camera_proxy.cpp > > > create mode 100644 src/android/camera_proxy.h > > > create mode 100644 src/android/thread_rpc.cpp > > > create mode 100644 src/android/thread_rpc.h > > > > [snip] > > > > > diff --git a/src/android/camera3_hal.cpp b/src/android/camera3_hal.cpp > > > new file mode 100644 > > > index 000000000000..0a97a9333d20 > > > --- /dev/null > > > +++ b/src/android/camera3_hal.cpp > > > @@ -0,0 +1,130 @@ > > > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > > > +/* > > > + * Copyright (C) 2019, Google Inc. > > > + * > > > + * camera3_hal.cpp - Android Camera3 HAL module > > > + */ > > > + > > > +#include <iostream> > > > +#include <memory> > > > +#include <vector> > > > + > > > +#include <hardware/hardware.h> > > > +#include <system/camera_metadata.h> > > > > Do you need camera_metadata.h ? Isn't it enough to include > > camera_common.h ? I think hardware.h can also be dropped. > > camera_common.h includes camera_metadata.h, so I could just include > the one I need. > > > > + > > > +#include <libcamera/libcamera.h> > > > > To speed up compilation, it would be better to include only the header > > files we need (here and everywhere else). > > Indeed > > > > + > > > +#include "log.h" > > > + > > > +#include "camera_hal_manager.h" > > > +#include "camera_module.h" > > > +#include "camera_proxy.h" > > > + > > > +using namespace libcamera; > > > + > > > +LOG_DEFINE_CATEGORY(HAL) > > > + > > > +static CameraHalManager cameraManager; > > > + > > > +/*------------------------------------------------------------------------------ > > > + * Android Camera HAL callbacks > > > + */ > > > + > > > +static int hal_get_number_of_cameras(void) > > > +{ > > > + return cameraManager.numCameras(); > > > +} > > > + > > > +static int hal_get_camera_info(int id, struct camera_info *info) > > > +{ > > > + return cameraManager.getCameraInfo(id, info); > > > +} > > > + > > > +static int hal_set_callbacks(const camera_module_callbacks_t *callbacks) > > > +{ > > > + return 0; > > > +} > > > + > > > +static int hal_open_legacy(const struct hw_module_t *module, const char *id, > > > + uint32_t halVersion, struct hw_device_t **device) > > > +{ > > > + return -ENOSYS; > > > +} > > > + > > > +static int hal_set_torch_mode(const char *camera_id, bool enabled) > > > +{ > > > + return -ENOSYS; > > > +} > > > + > > > +/* > > > + * First entry point of the camera HAL module. > > > + * > > > + * Initialize the HAL but does not open any camera device yet (see hal_dev_open) > > > + */ > > > +static int hal_init() > > > +{ > > > + LOG(HAL, Info) << "Initialising Android camera HAL"; > > > + > > > + cameraManager.initialize(); > > > + > > > + return 0; > > > +} > > > + > > > +/*------------------------------------------------------------------------------ > > > + * Android Camera Device > > > + */ > > > + > > > +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 *cameraModule = reinterpret_cast<CameraProxy *>(dev->priv); > > > > s/cameraModule/proxy/ otherwise it's very confusing to have a > > CameraProxy instance called cameraModule when there's a CameraModule > > class. > > > > This comment applies to all other similar instances. > > Sorry, leftover. I don't see any other module/proxy in this file > > > > + > > > + return cameraManager.close(cameraModule); > > > > As cameraManager.close() only calls proxy->close(), how about defining > > the hal_dev_close() method as a static method of CameraProxy, and > > setting proxy->device()->common.close inside the CameraProxy constructor > > ? It would avoid touching the internals of the CameraProxy in > > hal_dev_open() below. > > I liked having hal_dev_open and hal_dev_close here, as part of the HAL > module. But yes, we can save one function call and poking into the > device.common fields below.. I agree that grouped hal_dev_open() and hal_dev_close() is nice, but it's offset by the needed to then poke into proxy->device()->common.close. Let's also remember that the HAL API separates open from all the other operations, so we should be sad to separate them here. > > > +} > > > + > > > +static int hal_dev_open(const hw_module_t* module, const char* name, > > > + hw_device_t** device) > > > +{ > > > + LOG(HAL, Debug) << "Open camera: " << name; > > > > s/camera:/camera/ > > > > > + > > > + int id = atoi(name); > > > + CameraProxy *proxy = cameraManager.open(id, module); > > > + if (!proxy) { > > > + LOG(HAL, Error) << "Failed to open camera module " << id; > > > + return -ENODEV; > > > + } > > > + proxy->device()->common.close = hal_dev_close; > > > + *device = &proxy->device()->common; > > > + > > > + return 0; > > > +} > > > > Would it make sense to define all these methods as static methods of the > > CameraHalManager class, and turn the class into a singleton with an > > instance() method ? > > Which methods? hal_dev_open? All the hal_dev_ ones which are used to > initialize the camera_module_t ? I'm not sure I see a clear win... hal_dev_open(), but also all the methods defined in camera_module_t. In general I think it's nice to avoid global methods, and use singletons instead of global variables. This could be addressed later too. > > > + > > > +static struct hw_module_methods_t hal_module_methods = { > > > + .open = hal_dev_open, > > > +}; > > > + > > > +camera_module_t HAL_MODULE_INFO_SYM = { > > > + .common = { > > > + .tag = HARDWARE_MODULE_TAG, > > > + .module_api_version = CAMERA_MODULE_API_VERSION_2_4, > > > + .hal_api_version = HARDWARE_HAL_API_VERSION, > > > + .id = CAMERA_HARDWARE_MODULE_ID, > > > + .name = "Libcamera Camera3HAL Module", > > > > s/Libcamera/libcamera/ > > > > > + .author = "libcamera", > > > + .methods = &hal_module_methods, > > > + .dso = nullptr, > > > + .reserved = {}, > > > + }, > > > + > > > + .get_number_of_cameras = hal_get_number_of_cameras, > > > + .get_camera_info = hal_get_camera_info, > > > + .set_callbacks = hal_set_callbacks, > > > + .get_vendor_tag_ops = nullptr, > > > + .open_legacy = hal_open_legacy, > > > + .set_torch_mode = hal_set_torch_mode, > > > + .init = hal_init, > > > + .reserved = {}, > > > +}; > > > diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp > > > new file mode 100644 > > > index 000000000000..734b5eebd9a5 > > > --- /dev/null > > > +++ b/src/android/camera_hal_manager.cpp > > > @@ -0,0 +1,173 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2019, Google Inc. > > > + * > > > + * camera_hal_manager.cpp - Libcamera Android Camera Manager > > > + */ > > > + > > > +#include "camera_hal_manager.h" > > > + > > > +#include <map> > > > > This shouldn't be needed. > > Yes > > > > + > > > +#include <hardware/hardware.h> > > > +#include <system/camera_metadata.h> > > > + > > > +#include <libcamera/libcamera.h> > > > +#include <libcamera/camera.h> > > > + > > > +#include "log.h" > > > + > > > +#include "camera_module.h" > > > +#include "camera_proxy.h" > > > + > > > +using namespace libcamera; > > > > Missing blank line. > > > > > +LOG_DECLARE_CATEGORY(HAL); > > > + > > > +CameraHalManager::~CameraHalManager() > > > +{ > > > + for (auto metadata : staticMetadataMap_) > > > + free_camera_metadata(metadata.second); > > > +} > > > + > > > +int CameraHalManager::initialize() > > > +{ > > > + /* > > > + * Thread::start() will create a std::thread which will execute > > > + * CameraHalManager::run() > > > + */ > > > > That's documenting the libcamera::Thread class (and the std::thread is > > an internal implementation detail). I would just state > > > > /* > > * Start the camera HAL manager thread and wait until its > > * initialisation completes to be fully operational before > > * receiving calls from the camera stack. > > */ > > > > (thus dropping the next comment) > > As you might have noticed, the camera HAL still has comments and > annotations I added while developing, I'll clear them up better. > > > > + start(); > > > + > > > + /* > > > + * Wait for run() to notify us before returning to the caller to > > > + * make sure libcamera is fully initialized before we start handling > > > + * calls from the camera stack. > > > + */ > > > + MutexLocker locker(mutex_); > > > + cv_.wait(locker); > > > + > > > + return 0; > > > +} > > > + > > > +void CameraHalManager::run() > > > +{ > > > + /* > > > + * The run() operation is scheduled in its own thread; > > > + * It exec() to run the even dispatcher loop and initialize libcamera. > > > > s/even/event/ > > > > > + * > > > + * All the libcamera components initialized from here will be tied to > > > + * the same thread as the here below run event dispatcher. > > > > The first paragraph also mostly documents the Thread class. I would drop > > it, and write the second paragraph as > > > > /* > > * All the libcamera components must be initialised here, in > > * order to bind them to the camera HAL manager thread that > > * executes the event dispatcher. > > / > > > > > + */ > > > + libcameraManager_ = libcamera::CameraManager::instance(); > > > + > > > + int ret = libcameraManager_->start(); > > > + if (ret) { > > > + LOG(HAL, Error) << "Failed to start camera manager: " > > > + << strerror(-ret); > > > + return; > > > + } > > > + > > > + /* > > > + * For each Camera registered in the system, a CameraModule > > > + * get created here with an associated Camera and a proxy. > > > > s/get/gets/ > > > > > + * > > > + * \todo Support camera hotplug. > > > + */ > > > + unsigned int cameraIndex = 0; > > > > Maybe just index ? > > > > > + for (auto camera : libcameraManager_->cameras()) { > > > > auto &camera > > > > otherwise camera will be a copy of the shared pointer instead of a > > reference to it. > > > > > + cameras_.push_back(camera); > > > > The cameras_ vector isn't used after this. I would drop it, and store > > the shared pointer in CameraModule. > > Correct, I missed this in the the latest rework of the proxies > lifecycle. > > > > + > > > + CameraModule *module = new CameraModule(cameraIndex, > > > + camera.get()); > > > + modules_.emplace_back(module); > > > + > > > + CameraProxy *proxy = new CameraProxy(cameraIndex, > > > + module); > > > + proxies_.emplace_back(proxy); > > > > Similarly, how about sotring the module pointer inside the CameraProxy > > class ? That would ensure that all accesses to the module go through the > > proxy, which I think would be a good idea. > > As I use the modules_ only to access the map of static metadata which > should now got away, I think it's doable > > > CameraProxy *proxy = new CameraProxy(index, camera); > > proxies_.emplace_back(proxy); > > > > > + > > > + ++cameraIndex; > > > + } > > > + > > > + /* > > > + * libcamera has been initialized! Unlock the initialize() caller > > > + * as we're now ready to handle calls from the framework. > > > + */ > > > + cv_.notify_one(); > > > + > > > + /* Now start processing events and messages! */ > > > > s/!/./ in the above two messages, there's no need to be > > over-enthusiastic :-) > > > > > + exec(); > > > +} > > > + > > > +CameraProxy *CameraHalManager::open(unsigned int id, > > > + const hw_module_t *hardwareModule) > > > +{ > > > + if (id < 0 || id >= numCameras()) { > > > + LOG(HAL, Error) << "Invalid camera id '" << id << "'"; > > > + return nullptr; > > > + } > > > + > > > + CameraProxy *proxy = proxies_[id].get(); > > > + if (proxy->open(hardwareModule)) > > > + return proxy; > > > > Shouldn't you return nullprtr here ? > > Ideed > > > > + > > > + LOG(HAL, Info) << "Camera: '" << id << "' opened"; > > > > s/Camera:/Camera/ > > > > > + > > > + return proxy; > > > +} > > > + > > > +int CameraHalManager::close(CameraProxy *proxy) > > > +{ > > > + proxy->close(); > > > + LOG(HAL, Info) << "Close camera: " << proxy->id(); > > > + > > > + return 0; > > > +} > > > + > > > +unsigned int CameraHalManager::numCameras() const > > > +{ > > > + return libcameraManager_->cameras().size(); > > > +} > > > + > > > +/* > > > + * Before the camera framework even tries to open a camera device with > > > + * hal_dev_open() it requires the camera HAL to report a list of static > > > + * informations on the camera device with id \a id in the hal_get_camera_info() > > > + * method. > > > + * > > > + * The static metadata should be then generated probably from a > > > + * platform-specific module, as we cannot operate on the camera at this time as > > > + * it has not yet been open by the framework. > > > > s/open/opened/ > > > > What prevents us from opening the camera internally ? I don't think we > > It would complicate the lifecycle handling a bit, but I guess it's > doable. AS of now, where all metadata are static it is not worth it > imo We don't need to do it now. I was mostly addressing the comment above that states we can't operate on the camera, which I think isn't correct as we can decouple opening the libcamera::Camera and the HAL camera. > > need a platform-specific module. > > We surely won't be able to get from kernel at this point all the > information we need, in example, the camera position. I can't dispute that :-) We should however get it from the libcamera::Camera. > > > + * > > > + * This is what the Intel XML file is used for, it is parsed and the data there > > > + * combined with informations from the PSL service (which I -think- it's what > > > + * our IPA is) to generate a list of static metadata per-camera device. > > > > I'd drop this paragraph. > > > > > + */ > > > +int CameraHalManager::getCameraInfo(int id, struct camera_info *info) > > > +{ > > > + int cameras = numCameras(); > > > + if (id >= cameras || id < 0 || !info) { > > > > Can we be called with info == nullptr ? I think I'd drop that part of > > the check. > > All (most?) the extra-paranoid checks I have added here are error conditions > tested by the cros_camera_test suite. Lovely test suite... I suppose we should consider ourselves lucky that it doesn't test with info = 0xdeadbeef or other random pointer values :-) > > > + LOG(HAL, Error) << "Invalid camera id: " << id; > > > > s/id:/id/ > > > > You may also want to add quotes around the value, as done in > > CameraHalManager::open() (or drop them there). > > > > > + return -EINVAL; > > > + } > > > + > > > + camera_metadata_t *staticMetadata; > > > + auto it = staticMetadataMap_.find(id); > > > + if (it != staticMetadataMap_.end()) { > > > + staticMetadata = it->second; > > > + } else { > > > + CameraModule *cameraModule = modules_[id].get(); > > > + > > > + staticMetadata = cameraModule->getStaticMetadata(); > > > + staticMetadataMap_[id] = staticMetadata; > > > > Why do you need the map, why can't you always retrieve the static > > metadata from the CameraModule ? > > I should, yes, so we can frop modules_ > > > > + } > > > + > > > + /* \todo: Get these info dynamically inspecting the camera module. */ > > > > s/:// > > > > > + info->facing = id ? CAMERA_FACING_FRONT : CAMERA_FACING_BACK; > > > + info->orientation = 0; > > > + info->device_version = 0; > > > + info->resource_cost = 0; > > > + info->static_camera_characteristics = staticMetadata; > > > + info->conflicting_devices = nullptr; > > > + info->conflicting_devices_length = 0; > > > + > > > + return 0; > > > +} > > > diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h > > > new file mode 100644 > > > index 000000000000..65d6fa3150ef > > > --- /dev/null > > > +++ b/src/android/camera_hal_manager.h > > > @@ -0,0 +1,56 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2019, Google Inc. > > > + * > > > + * camera_hal_manager.h - Libcamera Android Camera Manager > > > + */ > > > +#ifndef __ANDROID_CAMERA_MANAGER_H__ > > > +#define __ANDROID_CAMERA_MANAGER_H__ > > > + > > > +#include <condition_variable> > > > +#include <cstddef> > > > +#include <map> > > > +#include <memory> > > > +#include <vector> > > > + > > > +#include <hardware/hardware.h> > > > +#include <system/camera_metadata.h> > > > + > > > +#include <libcamera/libcamera.h> > > > + > > > +#include "thread.h" > > > +#include "thread_rpc.h" > > > + > > > +class CameraModule; > > > +class CameraProxy; > > > + > > > +class CameraHalManager : public libcamera::Thread > > > +{ > > > +public: > > > + ~CameraHalManager(); > > > + > > > + int initialize(); > > > > Our initialisation methods are usually called init(), not initialize(). > > ok > > > > + > > > + CameraProxy *open(unsigned int id, const hw_module_t *module); > > > + int close(CameraProxy *proxy); > > > + > > > + unsigned int numCameras() const; > > > + int getCameraInfo(int id, struct camera_info *info); > > > + > > > +private: > > > + void run() override; > > > + camera_metadata_t *getStaticMetadata(unsigned int id); > > > + > > > + libcamera::CameraManager *libcameraManager_; > > > > I think you can call this manager_. > > > > > + > > > + std::mutex mutex_; > > > + std::condition_variable cv_; > > > + > > > + std::vector<std::shared_ptr<libcamera::Camera>> cameras_; > > > + std::vector<std::unique_ptr<CameraModule>> modules_; > > > + std::vector<std::unique_ptr<CameraProxy>> proxies_; > > > + > > > + std::map<unsigned int, camera_metadata_t *> staticMetadataMap_; > > > +}; > > > + > > > +#endif /* __ANDROID_CAMERA_MANAGER_H__ */ > > > diff --git a/src/android/camera_module.cpp b/src/android/camera_module.cpp > > > new file mode 100644 > > > index 000000000000..b64e377fb439 > > > --- /dev/null > > > +++ b/src/android/camera_module.cpp > > > @@ -0,0 +1,799 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2019, Google Inc. > > > + * > > > + * camera_module.cpp - Libcamera Android Camera Module > > > + */ > > > + > > > +#include "camera_module.h" > > > + > > > +#include <iostream> > > > +#include <memory> > > > + > > > +#include <system/camera_metadata.h> > > > + > > > +#include <hardware/camera3.h> > > > > I would group all the Android headers together, so > > > > #include <hardware/camera3.h> > > #include <system/camera_metadata.h> > > > > #include <libcamera/libcamera.h> > > > > > +#include <libcamera/libcamera.h> > > > + > > > +#include "message.h" > > > + > > > +#include "log.h" > > > > #include "log.h" > > #include "message.h" > > > > > + > > > +using namespace libcamera; > > > + > > > +LOG_DECLARE_CATEGORY(HAL); > > > + > > > +CameraModule::CameraModule(unsigned int id, libcamera::Camera *camera) > > > + : id_(id), camera_(camera), cameraState_(CameraStopped), > > > + requestTemplate_(nullptr) > > > +{ > > > + camera_->requestCompleted.connect(this, > > > + &CameraModule::requestComplete); > > > > This holds on a single line. > > > > > +} > > > + > > > +CameraModule::~CameraModule() > > > +{ > > > + if (requestTemplate_) > > > + free_camera_metadata(requestTemplate_); > > > + requestTemplate_ = nullptr; > > > +} > > > + > > > +void CameraModule::message(Message *message) > > > +{ > > > + ThreadRpcMessage *rpcMessage = static_cast<ThreadRpcMessage *> > > > + (message); > > > + > > > + if (rpcMessage->type() != ThreadRpcMessage::type()) > > > + return Object::message(message); > > > > As Message may not be a ThreadRpcMessage, you should write this as > > > > if (message->type() != ThreadRpcMessage::type()) > > return Object::message(message); > > > > ThreadRpcMessage *rpcMessage = static_cast<ThreadRpcMessage *>(message); > > > > > + > > > + ThreadRpc *rpc = rpcMessage->rpc; > > > + > > > + switch (rpc->tag) { > > > + case ThreadRpc::ProcessCaptureRequest: > > > + processCaptureRequest(rpc->request); > > > + break; > > > + case ThreadRpc::Close: > > > + close(); > > > + break; > > > + default: > > > + LOG(HAL, Error) << "Unknown RPC operation: " << rpc->tag; > > > + } > > > + > > > + rpc->notifyReception(); > > > +} > > > + > > > +int CameraModule::open() > > > +{ > > > + int ret = camera_->acquire(); > > > + if (ret) { > > > + LOG(HAL, Error) << "Failed to acquire the camera"; > > > + return ret; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +void CameraModule::close() > > > +{ > > > + camera_->stop(); > > > + > > > + camera_->freeBuffers(); > > > + camera_->release(); > > > + > > > + cameraState_ = CameraStopped; > > > +} > > > + > > > +void CameraModule::setCallbacks(const struct camera3_device *camera3Device, > > > + const camera3_callback_ops_t *callbacks) > > > +{ > > > + camera3Device_ = camera3Device; > > > > camera3Device_ seems unused. > > > > > + callbacks_ = callbacks; > > > +} > > > + > > > +camera_metadata_t *CameraModule::getStaticMetadata() > > > +{ > > > + int ret; > > > + > > > + /* > > > + * The below metadata has been added by inspecting the Intel XML > > > + * file and it's associated parsing routines in the Intel IPU3 HAL. > > > + * > > > + * The here below metadata are enough to satisfy the camera3-test > > > + * provided by ChromeOS, but a real camera implementation might require > > > > s/might/will/ > > > > > + * more. > > > + * > > > + * See CameraConf::AiqConf class implementation in the Intel HAL > > > + * to get a list of the static metadata registered by parsing the > > > + * XML config file in AiqConf::fillStaticMetadataFromCMC > > > + */ > > > + > > > + /* > > > + * FIXME: this sizes come from the Intel IPU3 HAL, and are way too large for > > > > s/FIXME:/\todo/ > > s/this/these/ > > > > > + * this intial HAL version. > > > + */ > > > + #define STATIC_ENTRY_CAP 256 > > > + #define STATIC_DATA_CAP 6688 > > > + camera_metadata_t *staticMetadata = > > > + allocate_camera_metadata(STATIC_ENTRY_CAP, STATIC_DATA_CAP); > > > + > > > + /* Sensor static metadata. */ > > > + int32_t pixelArraySize[] = { > > > + 2592, 1944, > > > + }; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE, > > > + &pixelArraySize, 2); > > > + METADATA_ASSERT(ret); > > > + > > > + int32_t sensorSizes[] = { > > > + 0, 0, 2560, 1920, > > > + }; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE, > > > + &sensorSizes, 4); > > > + METADATA_ASSERT(ret); > > > + > > > + int32_t sensitivityRange[] = { > > > + 32, 2400, > > > + }; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_SENSOR_INFO_SENSITIVITY_RANGE, > > > + &sensitivityRange, 2); > > > + METADATA_ASSERT(ret); > > > + > > > + uint16_t filterArr = ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT_GRBG; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT, > > > + &filterArr, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + int64_t exposureTimeRange[] = { > > > + 100000, 200000000, > > > + }; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE, > > > + &exposureTimeRange, 2); > > > + METADATA_ASSERT(ret); > > > + > > > + int32_t orientation = 0; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_SENSOR_ORIENTATION, > > > + &orientation, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + /* Flash static metadata. */ > > > + char flashAvailable = ANDROID_FLASH_INFO_AVAILABLE_FALSE; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_FLASH_INFO_AVAILABLE, > > > + &flashAvailable, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + /* Lens static metadata. */ > > > + float fn = 2.53 / 100; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_LENS_INFO_AVAILABLE_APERTURES, &fn, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + /* Control metadata. */ > > > + char controlMetadata = ANDROID_CONTROL_MODE_AUTO; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_CONTROL_AVAILABLE_MODES, > > > + &controlMetadata, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + char availableAntiBandingModes[] = { > > > + ANDROID_CONTROL_AE_ANTIBANDING_MODE_OFF, > > > + ANDROID_CONTROL_AE_ANTIBANDING_MODE_50HZ, > > > + ANDROID_CONTROL_AE_ANTIBANDING_MODE_60HZ, > > > + ANDROID_CONTROL_AE_ANTIBANDING_MODE_AUTO, > > > + }; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES, > > > + availableAntiBandingModes, 4); > > > + METADATA_ASSERT(ret); > > > + > > > + char aeAvailableModes[] = { > > > + ANDROID_CONTROL_AE_MODE_ON, > > > + ANDROID_CONTROL_AE_MODE_OFF, > > > + }; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_CONTROL_AE_AVAILABLE_MODES, > > > + aeAvailableModes, 2); > > > + METADATA_ASSERT(ret); > > > + > > > + controlMetadata = ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_CONTROL_AE_LOCK_AVAILABLE, > > > + &controlMetadata, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + uint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, > > > + &awbLockAvailable, 1); > > > + > > > + /* Scaler static metadata. */ > > > + std::vector<uint32_t> availableStreamFormats = { > > > + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, > > > + ANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, > > > + ANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, > > > + }; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_SCALER_AVAILABLE_FORMATS, > > > + availableStreamFormats.data(), > > > + availableStreamFormats.size()); > > > + METADATA_ASSERT(ret); > > > + > > > + std::vector<uint32_t> availableStreamConfigurations = { > > > + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, > > > + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT, > > > + ANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, 2560, 1920, > > > + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT, > > > + ANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, 2560, 1920, > > > + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT, > > > + }; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS, > > > + availableStreamConfigurations.data(), > > > + availableStreamConfigurations.size()); > > > + METADATA_ASSERT(ret); > > > + > > > + std::vector<int64_t> availableStallDurations = { > > > + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, 33333333, > > > + }; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_SCALER_AVAILABLE_STALL_DURATIONS, > > > + availableStallDurations.data(), > > > + availableStallDurations.size()); > > > + METADATA_ASSERT(ret); > > > + > > > + std::vector<int64_t> minFrameDurations = { > > > + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, 33333333, > > > + ANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, 2560, 1920, 33333333, > > > + ANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, 2560, 1920, 33333333, > > > + }; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS, > > > + minFrameDurations.data(), minFrameDurations.size()); > > > + METADATA_ASSERT(ret); > > > + > > > + /* Info static metadata. */ > > > + uint8_t supportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED; > > > + ret = add_camera_metadata_entry(staticMetadata, > > > + ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL, > > > + &supportedHWLevel, 1); > > > + > > > + return staticMetadata; > > > +} > > > + > > > +const camera_metadata_t *CameraModule::constructDefaultRequestMetadata(int type) > > > +{ > > > + int ret; > > > + > > > + /* > > > + * TODO: inspect type and pick the right metadata pack. > > > > s/TODO:/\todo/ > > > > > + * As of now just use a single one for all templates > > > + */ > > > + uint8_t captureIntent; > > > + switch (type) { > > > + case CAMERA3_TEMPLATE_PREVIEW: > > > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_PREVIEW; > > > + break; > > > + case CAMERA3_TEMPLATE_STILL_CAPTURE: > > > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_STILL_CAPTURE; > > > + break; > > > + case CAMERA3_TEMPLATE_VIDEO_RECORD: > > > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_VIDEO_RECORD; > > > + break; > > > + case CAMERA3_TEMPLATE_VIDEO_SNAPSHOT: > > > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_VIDEO_SNAPSHOT; > > > + break; > > > + case CAMERA3_TEMPLATE_ZERO_SHUTTER_LAG: > > > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_ZERO_SHUTTER_LAG; > > > + break; > > > + case CAMERA3_TEMPLATE_MANUAL: > > > + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_MANUAL; > > > + break; > > > + default: > > > + LOG(HAL, Error) << "Invalid template request type: " << type; > > > + return nullptr; > > > + } > > > + > > > + if (requestTemplate_) > > > + return requestTemplate_; > > > + > > > + /* FIXME: this sizes are quite arbitrary ones */ > > > + #define REQUEST_TEMPLATE_ENTRIES 30 > > > + #define REQUEST_TEMPLATE_DATA 2048 > > > + requestTemplate_ = allocate_camera_metadata(REQUEST_TEMPLATE_ENTRIES, > > > + REQUEST_TEMPLATE_DATA); > > > + if (!requestTemplate_) { > > > + LOG(HAL, Error) << "Failed to allocate template metadata"; > > > + return nullptr; > > > + } > > > + > > > + /* > > > + * Fill in the required Request metadata. > > > + * > > > + * Part (most?) of this entries comes from inspecting the Intel's IPU3 > > > + * HAL xml configuration file. > > > + */ > > > + > > > + /* Set to 0 the number of 'processed and stalling' streams (ie JPEG). */ > > > + int32_t maxOutStream[] = { 0, 2, 0 }; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS, > > > + maxOutStream, 3); > > > + METADATA_ASSERT(ret); > > > + > > > + uint8_t maxPipelineDepth = 5; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_REQUEST_PIPELINE_MAX_DEPTH, > > > + &maxPipelineDepth, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + int32_t inputStreams = 0; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_REQUEST_MAX_NUM_INPUT_STREAMS, > > > + &inputStreams, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + int32_t partialResultCount = 1; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_REQUEST_PARTIAL_RESULT_COUNT, > > > + &partialResultCount, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + uint8_t availableCapabilities[] = { > > > + ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE, > > > + }; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_REQUEST_AVAILABLE_CAPABILITIES, > > > + availableCapabilities, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_CONTROL_AE_MODE, > > > + &aeMode, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + int32_t aeExposureCompensation = 0; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, > > > + &aeExposureCompensation, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + uint8_t aePrecaptureTrigger = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, > > > + &aePrecaptureTrigger, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + uint8_t aeLock = ANDROID_CONTROL_AE_LOCK_OFF; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_CONTROL_AE_LOCK, > > > + &aeLock, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + uint8_t afTrigger = ANDROID_CONTROL_AF_TRIGGER_IDLE; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_CONTROL_AF_TRIGGER, > > > + &afTrigger, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + uint8_t awbMode = ANDROID_CONTROL_AWB_MODE_AUTO; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_CONTROL_AWB_MODE, > > > + &awbMode, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + uint8_t awbLock = ANDROID_CONTROL_AWB_LOCK_OFF; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_CONTROL_AWB_LOCK, > > > + &awbLock, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + uint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, > > > + &awbLockAvailable, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + uint8_t flashMode = ANDROID_FLASH_MODE_OFF; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_FLASH_MODE, > > > + &flashMode, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + uint8_t faceDetectMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_STATISTICS_FACE_DETECT_MODE, > > > + &faceDetectMode, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_CONTROL_CAPTURE_INTENT, > > > + &captureIntent, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + /* > > > + * This is quite hard to list at the moment wihtout knowing what > > > + * we could control. > > > + * > > > + * For now, just list in the available Request keys and in the available > > > + * result keys the control and reporting of the AE algorithm. > > > + */ > > > + std::vector<int32_t> availableRequestKeys = { > > > + ANDROID_CONTROL_AE_MODE, > > > + ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, > > > + ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, > > > + ANDROID_CONTROL_AE_LOCK, > > > + ANDROID_CONTROL_AF_TRIGGER, > > > + ANDROID_CONTROL_AWB_MODE, > > > + ANDROID_CONTROL_AWB_LOCK, > > > + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, > > > + ANDROID_CONTROL_CAPTURE_INTENT, > > > + ANDROID_FLASH_MODE, > > > + ANDROID_STATISTICS_FACE_DETECT_MODE, > > > + }; > > > + > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS, > > > + availableRequestKeys.data(), > > > + availableRequestKeys.size()); > > > + METADATA_ASSERT(ret); > > > + > > > + std::vector<int32_t> availableResultKeys = { > > > + ANDROID_CONTROL_AE_MODE, > > > + ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, > > > + ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, > > > + ANDROID_CONTROL_AE_LOCK, > > > + ANDROID_CONTROL_AF_TRIGGER, > > > + ANDROID_CONTROL_AWB_MODE, > > > + ANDROID_CONTROL_AWB_LOCK, > > > + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, > > > + ANDROID_CONTROL_CAPTURE_INTENT, > > > + ANDROID_FLASH_MODE, > > > + ANDROID_STATISTICS_FACE_DETECT_MODE, > > > + }; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_REQUEST_AVAILABLE_RESULT_KEYS, > > > + availableResultKeys.data(), > > > + availableResultKeys.size()); > > > + METADATA_ASSERT(ret); > > > + > > > + /* > > > + * The available characteristics should be the tags reported > > > + * as part of the static metadata reported at hal_get_camera_info() > > > + * time. The xml file sets those to 0 though. > > > + */ > > > + std::vector<int32_t> availableCharacteristicsKeys = {}; > > > + ret = add_camera_metadata_entry(requestTemplate_, > > > + ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS, > > > + availableCharacteristicsKeys.data(), > > > + availableCharacteristicsKeys.size()); > > > + METADATA_ASSERT(ret); > > > + > > > + return requestTemplate_; > > > +} > > > + > > > +/** > > > + * Inspect the stream_list to produce a list of StreamConfiguration to > > > + * be use to configure the Camera. > > > + */ > > > +int CameraModule::configureStreams(camera3_stream_configuration_t *stream_list) > > > +{ > > > + > > > > Extra blank line. > > > > > + for (unsigned int i = 0; i < stream_list->num_streams; ++i) { > > > + camera3_stream_t *stream = stream_list->streams[i]; > > > + > > > + LOG(HAL, Info) << "Stream #" << i > > > + << ": direction: " << stream->stream_type > > > + << " - width: " << stream->width > > > + << " - height: " << stream->height > > > + << " - format: " << std::hex << stream->format; > > > > s/ -/,/ in the above three lines > > > > And I would print size: ...x... instead of width and height. > > > > > + } > > > + > > > + /* Hardcode viewfinder role, collecting sizes from the stream config. */ > > > + if (stream_list->num_streams != 1) { > > > + LOG(HAL, Error) << "Only one stream supported"; > > > + return -EINVAL; > > > + } > > > + > > > + StreamRoles roles = {}; > > > + roles.push_back(StreamRole::StillCapture); > > > > I think you can write this as > > > > StreamRoles roles{ StreamRole::StillCapture }; > > > > > + > > > + config_ = camera_->generateConfiguration(roles); > > > + if (!config_ || config_->empty()) { > > > + LOG(HAL, Error) << "Failed to generate camera configuration"; > > > + return -EINVAL; > > > + } > > > + > > > + /* Only one stream is supported. */ > > > + camera3_stream_t *camera3Stream = stream_list->streams[0]; > > > + StreamConfiguration *streamConfiguration = &config_->at(0); > > > + streamConfiguration->size.width = camera3Stream->width; > > > + streamConfiguration->size.height = camera3Stream->height; > > > + streamConfiguration->memoryType = ExternalMemory; > > > + > > > + /* > > > + * FIXME: do not change the pixel format from the one returned > > > + * from the Camera::generateConfiguration(); > > > + * > > > + * We'll need to translate from Android defined pixel format codes > > > + * to whatever libcamera will use. > > > + */ > > > + > > > + switch (config_->validate()) { > > > + case CameraConfiguration::Valid: > > > + break; > > > + case CameraConfiguration::Adjusted: > > > + LOG(HAL, Info) << "Camera configuration adjusted"; > > > > Missing config_.reset() ? > > > > > + return -EINVAL; > > > + case CameraConfiguration::Invalid: > > > + LOG(HAL, Info) << "Camera configuration invalid"; > > > + config_.reset(); > > > + return -EINVAL; > > > + } > > > + > > > + camera3Stream->max_buffers = streamConfiguration->bufferCount; > > > + > > > + /* > > > + * Once the CameraConfiguration has been adjusted/validated > > > + * it can be applied to the camera. > > > + */ > > > + int ret = camera_->configure(config_.get()); > > > + if (ret) { > > > + LOG(HAL, Error) << "Failed to configure camera '" > > > + << camera_->name() << "'"; > > > + return ret; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +int CameraModule::processCaptureRequest(camera3_capture_request_t *camera3Request) > > > +{ > > > + StreamConfiguration *streamConfiguration = &config_->at(0); > > > + Stream *stream = streamConfiguration->stream(); > > > + > > > + if (camera3Request->num_output_buffers != 1) { > > > + LOG(HAL, Error) << "Invalid number of output buffers: " > > > + << camera3Request->num_output_buffers; > > > + return -EINVAL; > > > + } > > > + > > > + /* Start the camera if that's the first request we handle. */ > > > + if (cameraState_ == CameraStopped) { > > > + int ret = camera_->allocateBuffers(); > > > + if (ret) { > > > + LOG(HAL, Error) << "Failed to allocate buffers"; > > > + return ret; > > > + } > > > + > > > + ret = camera_->start(); > > > + if (ret) { > > > + LOG(HAL, Error) << "Failed to start camera"; > > > + camera_->freeBuffers(); > > > + return ret; > > > + } > > > + > > > + cameraState_ = CameraRunning; > > > + } > > > + > > > + /* > > > + * Queue a request for the Camera with the provided dmabuf file > > > + * descriptors. > > > + */ > > > + const camera3_stream_buffer_t *camera3Buffer = > > > + &camera3Request->output_buffers[0]; > > > + const buffer_handle_t camera3Handle = *camera3Buffer->buffer; > > > + > > > + std::array<int, 3> fds = { > > > + camera3Handle->data[0], > > > + camera3Handle->data[1], > > > + camera3Handle->data[2], > > > + }; > > > + std::unique_ptr<Buffer> buffer = stream->createBuffer(fds); > > > + if (!buffer) { > > > + LOG(HAL, Error) << "Failed to create buffer"; > > > + return -EINVAL; > > > + } > > > + > > > + Request *request = camera_->createRequest(); > > > + request->addBuffer(std::move(buffer)); > > > + int ret = camera_->queueRequest(request); > > > + if (ret) { > > > + LOG(HAL, Error) << "Failed to queue request"; > > > + return ret; > > > + } > > > + > > > + /* Save the request descriptors for use at completion time. */ > > > + Camera3RequestDescriptor descriptor = {}; > > > + descriptor.frameNumber = camera3Request->frame_number, > > > + descriptor.numBuffers = camera3Request->num_output_buffers, > > > > s/,$/;/ for the two lines above. > > > > I don't think you need to store numBuffers in the descriptors, it can be > > retrieved from buffers.size() in requestComplete(). > > numBuffers is the framework required number of buffers, buffers.size() > is the number of buffers completed in the request. They -should- be > the same, but I would keep them separate to make sure the completed > request is correct. It boils down to how much paranoia we need to build in :-) How could they be different ? > > > + descriptor.buffers = new camera3_stream_buffer_t[descriptor.numBuffers]; > > > + for (unsigned int i = 0; i < descriptor.numBuffers; ++i) { > > > + camera3_stream_buffer_t &buffer = descriptor.buffers[i]; > > > + buffer = camera3Request->output_buffers[i]; > > > > Just > > > > descriptor.buffers[i] = camera3Request->output_buffers[i]; > > > + } > > > + > > > + requestMap_[request] = descriptor; > > > > How about using the request cookie (passed to Camera::createRequest()) > > for this purpose ? > > Can we do that on top? Sure, but please then keep track of the task, with a \todo here. > > > + > > > + return 0; > > > +} > > > + > > > +void CameraModule::requestComplete(Request *request, > > > + const std::map<Stream *, Buffer *> &buffers) > > > +{ > > > + Buffer *libcameraBuffer = buffers.begin()->second; > > > + camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; > > > + camera_metadata_t *resultMetadata = nullptr; > > > + > > > + if (request->status() != Request::RequestComplete) { > > > + LOG(HAL, Error) << "Request not succesfully completed: " > > > + << request->status(); > > > + status = CAMERA3_BUFFER_STATUS_ERROR; > > > + } > > > + > > > + /* Prepare to call back the Android camera stack. */ > > > + Camera3RequestDescriptor &descriptor = requestMap_[request]; > > > + > > > + camera3_capture_result_t captureResult = {}; > > > + captureResult.frame_number = descriptor.frameNumber; > > > + captureResult.num_output_buffers = descriptor.numBuffers; > > > + > > > + camera3_stream_buffer_t *resultBuffers = > > > + new camera3_stream_buffer_t[descriptor.numBuffers]; > > > + for (unsigned int i = 0; i < descriptor.numBuffers; ++i) { > > > + camera3_stream_buffer_t resultBuffer; > > > + > > > + resultBuffer = descriptor.buffers[i]; > > > + resultBuffer.acquire_fence = -1; > > > + resultBuffer.release_fence = -1; > > > + resultBuffer.status = status; > > > + > > > + resultBuffers[i] = resultBuffer; > > > + } > > > + captureResult.output_buffers = > > > + const_cast<const camera3_stream_buffer_t *>(resultBuffers); > > > + > > > + if (status == CAMERA3_BUFFER_STATUS_ERROR) { > > > + /* \todo Improve error handling. */ > > > + notifyError(descriptor.frameNumber, resultBuffers->stream); > > > + } else { > > > + notifyShutter(descriptor.frameNumber, libcameraBuffer->timestamp()); > > > + > > > + captureResult.partial_result = 1; > > > + resultMetadata = getResultMetadata(descriptor.frameNumber, > > > + libcameraBuffer->timestamp()); > > > + captureResult.result = resultMetadata; > > > + } > > > + > > > + callbacks_->process_capture_result(callbacks_, &captureResult); > > > + > > > + if (resultMetadata) > > > + free_camera_metadata(resultMetadata); > > > + > > > + delete[] resultBuffers; > > > + delete[] descriptor.buffers; > > > + requestMap_.erase(request); > > > + > > > + return; > > > +} > > > + > > > +void CameraModule::notifyShutter(uint32_t frameNumber, uint64_t timestamp) > > > +{ > > > + camera3_notify_msg_t notify = {}; > > > + > > > + notify.type = CAMERA3_MSG_SHUTTER; > > > + notify.message.shutter.frame_number = frameNumber; > > > + notify.message.shutter.timestamp = timestamp; > > > + > > > + callbacks_->notify(callbacks_, ¬ify); > > > +} > > > + > > > +void CameraModule::notifyError(uint32_t frameNumber, camera3_stream_t *stream) > > > +{ > > > + camera3_notify_msg_t notify = {}; > > > + > > > + notify.type = CAMERA3_MSG_ERROR; > > > + notify.message.error.error_stream = stream; > > > + notify.message.error.frame_number = frameNumber; > > > + notify.message.error.error_code = CAMERA3_MSG_ERROR_REQUEST; > > > + > > > + callbacks_->notify(callbacks_, ¬ify); > > > +} > > > + > > > +/* > > > + * Fixed result metadata, mostly imported from the UVC camera HAL, which > > > + * does not produces metadata and thus needs to generate a fixed set. > > > + */ > > > +camera_metadata_t *CameraModule::getResultMetadata(int frame_number, > > > + int64_t timestamp) > > > +{ > > > + int ret; > > > + > > > + /* FIXME: random "big enough" values. */ > > > + #define RESULT_ENTRY_CAP 256 > > > + #define RESULT_DATA_CAP 6688 > > > + camera_metadata_t *resultMetadata = > > > + allocate_camera_metadata(STATIC_ENTRY_CAP, STATIC_DATA_CAP); > > > + > > > + const uint8_t ae_state = ANDROID_CONTROL_AE_STATE_CONVERGED; > > > + ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AE_STATE, > > > + &ae_state, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + const uint8_t ae_lock = ANDROID_CONTROL_AE_LOCK_OFF; > > > + ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AE_LOCK, > > > + &ae_lock, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + uint8_t af_state = ANDROID_CONTROL_AF_STATE_INACTIVE; > > > + ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AF_STATE, > > > + &af_state, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + const uint8_t awb_state = ANDROID_CONTROL_AWB_STATE_CONVERGED; > > > + ret = add_camera_metadata_entry(resultMetadata, > > > + ANDROID_CONTROL_AWB_STATE, > > > + &awb_state, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + const uint8_t awb_lock = ANDROID_CONTROL_AWB_LOCK_OFF; > > > + ret = add_camera_metadata_entry(resultMetadata, > > > + ANDROID_CONTROL_AWB_LOCK, > > > + &awb_lock, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + const uint8_t lens_state = ANDROID_LENS_STATE_STATIONARY; > > > + ret = add_camera_metadata_entry(resultMetadata, > > > + ANDROID_LENS_STATE, > > > + &lens_state, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + int32_t sensorSizes[] = { > > > + 0, 0, 2560, 1920, > > > + }; > > > + ret = add_camera_metadata_entry(resultMetadata, > > > + ANDROID_SCALER_CROP_REGION, > > > + sensorSizes, 4); > > > + METADATA_ASSERT(ret); > > > + > > > + ret = add_camera_metadata_entry(resultMetadata, > > > + ANDROID_SENSOR_TIMESTAMP, > > > + ×tamp, 1); > > > + > > > + METADATA_ASSERT(ret); > > > + > > > + /* 33.3 msec */ > > > + const int64_t rolling_shutter_skew = 33300000; > > > + ret = add_camera_metadata_entry(resultMetadata, > > > + ANDROID_SENSOR_ROLLING_SHUTTER_SKEW, > > > + &rolling_shutter_skew, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + /* 16.6 msec */ > > > + const int64_t exposure_time = 16600000; > > > + ret = add_camera_metadata_entry(resultMetadata, > > > + ANDROID_SENSOR_EXPOSURE_TIME, > > > + &exposure_time, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + const uint8_t lens_shading_map_mode = > > > + ANDROID_STATISTICS_LENS_SHADING_MAP_MODE_OFF; > > > + ret = add_camera_metadata_entry(resultMetadata, > > > + ANDROID_STATISTICS_LENS_SHADING_MAP_MODE, > > > + &lens_shading_map_mode, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + const uint8_t scene_flicker = ANDROID_STATISTICS_SCENE_FLICKER_NONE; > > > + ret = add_camera_metadata_entry(resultMetadata, > > > + ANDROID_STATISTICS_SCENE_FLICKER, > > > + &scene_flicker, 1); > > > + METADATA_ASSERT(ret); > > > + > > > + return resultMetadata; > > > +} > > > diff --git a/src/android/camera_module.h b/src/android/camera_module.h > > > new file mode 100644 > > > index 000000000000..67488d5940b1 > > > --- /dev/null > > > +++ b/src/android/camera_module.h > > > @@ -0,0 +1,75 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2019, Google Inc. > > > + * > > > + * camera_module.h - Libcamera Android Camera Module > > > > s/Libcamera/libcamera/ > > > > (here and everywhere else, libcamera is always written in all lowercase) > > > > > + */ > > > +#ifndef __ANDROID_CAMERA_MODULE_H__ > > > +#define __ANDROID_CAMERA_MODULE_H__ > > > + > > > +#include <memory> > > > + > > > +#include <hardware/camera3.h> > > > +#include <libcamera/libcamera.h> > > > + > > > +#include "message.h" > > > + > > > +#include "camera_hal_manager.h" > > > + > > > +#define METADATA_ASSERT(_r) \ > > > + do { \ > > > + if (!(_r)) break; \ > > > + LOG(HAL, Error) << "Error: " << __func__ << ":" << __LINE__; \ > > > + return nullptr; \ > > > + } while(0); > > > > I really, really dislike hiding return statements in macros. We can keep > > it for now, but please add a comment telling that it should be removed. > > All the metadata handling will be nuked in the next versions, so I > won't bother for now. > > > > + > > > > Even if we don't document the whole implementation with Doxygen, I think > > a few short comments that explain what each class models would be > > useful. Otherwise the reader is left to wonder what CameraModule or > > CameraProxy stand for. A comment that explains how the various objects > > work together would be useful too. > > > > > +class CameraModule : public libcamera::Object > > > > I don't think CameraModule is a good name :-/ In HAL terminology, the > > module refers to camera_module_t, and corresponds more or less to > > libcamera::CameraManager, not libcamera::Camera. > > Correct > > > One option would be to name this class Camera in a HAL namespace. That > > would make it quite clear that it corresponds to a libcamra::Camera. > > I had a go at isolating namespaces and I have problems with the > logging infrastructure, Ah yes I rememeber that. > and I would not duplicate the Camera name to be honest. CameraDevice ? As it corresponds to camera3_device_t, CameraDevice makes sense. This just struck my mind now, should CameraHalManager be named CameraModule to match camera_module_t ? I'm fine either way. > > > +{ > > > +public: > > > + CameraModule(unsigned int id, libcamera::Camera *camera); > > > + ~CameraModule(); > > > + > > > + void message(libcamera::Message *message); > > > + > > > + int open(); > > > + void close(); > > > + void setCallbacks(const struct camera3_device *cameraDevice, > > > + const camera3_callback_ops_t *callbacks); > > > + camera_metadata_t *getStaticMetadata(); > > > + const camera_metadata_t *constructDefaultRequestMetadata(int type); > > > + int configureStreams(camera3_stream_configuration_t *stream_list); > > > + int processCaptureRequest(camera3_capture_request_t *request); > > > + void requestComplete(libcamera::Request *request, > > > + const std::map<libcamera::Stream *, libcamera::Buffer *> &buffers); > > > + > > > + unsigned int id() const { return id_; } > > > > I think this method is not used. > > > > > + > > > +private: > > > + struct Camera3RequestDescriptor { > > > + uint32_t frameNumber; > > > + uint32_t numBuffers; > > > + camera3_stream_buffer_t *buffers; > > > + }; > > > + > > > + enum CameraState { > > > + CameraStopped, > > > + CameraRunning, > > > + }; > > > > With just two states you could instead have a bool running_ field. > > > > > + > > > + void notifyShutter(uint32_t frameNumber, uint64_t timestamp); > > > + void notifyError(uint32_t frameNumber, camera3_stream_t *stream); > > > + camera_metadata_t *getResultMetadata(int frame_number, int64_t timestamp); > > > + > > > + unsigned int id_; > > > + libcamera::Camera *camera_; > > > + std::unique_ptr<libcamera::CameraConfiguration> config_; > > > + CameraState cameraState_; > > > + > > > + camera_metadata_t *requestTemplate_; > > > + const camera3_callback_ops_t *callbacks_; > > > + const struct camera3_device *camera3Device_; > > > + > > > + std::map<libcamera::Request *, Camera3RequestDescriptor> requestMap_; > > > +}; > > > + > > > +#endif /* __ANDROID_CAMERA_MODULE_H__ */ > > > diff --git a/src/android/camera_proxy.cpp b/src/android/camera_proxy.cpp > > > new file mode 100644 > > > index 000000000000..af7817f29137 > > > --- /dev/null > > > +++ b/src/android/camera_proxy.cpp > > > @@ -0,0 +1,181 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2019, Google Inc. > > > + * > > > + * camera_proxy.cpp - Proxy to camera module instances > > > + */ > > > + > > > +#include "camera_proxy.h" > > > + > > > +#include <hardware/camera3.h> > > > +#include <libcamera/libcamera.h> > > > +#include <system/camera_metadata.h> > > > + > > > +#include <memory> > > > + > > > +#include "log.h" > > > +#include "message.h" > > > +#include "thread_rpc.h" > > > +#include "utils.h" > > > + > > > +using namespace libcamera; > > > + > > > +LOG_DECLARE_CATEGORY(HAL); > > > + > > > +#define LIBCAMERA_HAL_FUNC_DBG \ > > > + LOG(HAL, Debug); > > > > If we need to trace calls, I think something better than debug messages > > would be useful. I'm not sure what yet though. > > Yeah, I'm not even sure we want to trace calls actually. The fact is > that the camera HAL is somewhat a weird beast, as it is a very system > specific libcamera extension, and I tried to mimic what is usually > found in other HAL implementations I've seen. Should we drop tracing > completely and make this more libcamera-like ? I think so. I also think we'll need tracing in the future, but that's for later. > > > + > > > +static int hal_dev_initialize(const struct camera3_device *dev, > > > + const camera3_callback_ops_t *callback_ops) > > > +{ > > > + LIBCAMERA_HAL_FUNC_DBG > > > + > > > + if (!dev) > > > + return -EINVAL; > > > > Can this happen ? > > I think cros_camera_test exercises this. > > > > > + > > > + CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); > > > + proxy->setCallbacks(callback_ops); > > > + > > > + return 0; > > > +} > > > + > > > +static int hal_dev_configure_streams(const struct camera3_device *dev, > > > + camera3_stream_configuration_t *stream_list) > > > +{ > > > + LIBCAMERA_HAL_FUNC_DBG > > > + > > > + if (!dev) > > > + return -EINVAL; > > > + > > > + CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); > > > + proxy->configureStreams(stream_list); > > > + > > > + return 0; > > > > Shouldn't you propagate the error value from proxy->configureStreams() ? > > > > > +} > > > + > > > +static const camera_metadata_t * > > > +hal_dev_construct_default_request_settings(const struct camera3_device *dev, > > > + int type) > > > +{ > > > + LIBCAMERA_HAL_FUNC_DBG > > > + > > > + if (!dev) > > > + return nullptr; > > > + > > > + CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); > > > + return proxy->constructDefaultRequest(type); > > > > How about renaming constructDefaultRequest() and > > constructDefaultRequestMetadata() to constructDefaultRequestSettings(), > > in order to match the HAL operation name ? > > Ok... > > > > +} > > > + > > > +static int hal_dev_process_capture_request(const struct camera3_device *dev, > > > + camera3_capture_request_t *request) > > > +{ > > > + LIBCAMERA_HAL_FUNC_DBG > > > + > > > + 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) > > > +{ > > > + LIBCAMERA_HAL_FUNC_DBG > > > +} > > > + > > > +static int hal_dev_flush(const struct camera3_device *dev) > > > +{ > > > + LIBCAMERA_HAL_FUNC_DBG > > > + > > > + return 0; > > > +} > > > > You could define these methods as static methods of the CameraProxy > > class. For instance, hal_dev_configure_streams would become > > > > int CameraProxy::configureStreams(const struct camera3_device *dev, > > camera3_stream_configuration_t *stream_list) > > { > > CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); > > proxy->configureStreams(stream_list); > > } > > To avoid the static C methods you mean ? Not sure what is the gain > though. Correct. The gain is that all related code would be grouped in the same class. It's not a big deal, and could be done on top of this series. I just think it would be a bit cleaner (but I could be wrong). > > > + > > > +static camera3_device_ops hal_dev_ops = { > > > > I wish this could be const :-( > > > > > + .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 = { 0 }, > > > > s/0/nullptr/ ? > > > > > +}; > > > + > > > +CameraProxy::CameraProxy(unsigned int id, CameraModule *cameraModule) > > > + : open_(false), id_(id), cameraModule_(cameraModule) > > > +{ > > > +} > > > + > > > +int CameraProxy::open(const hw_module_t *hardwareModule) > > > +{ > > > + int ret = cameraModule_->open(); > > > + if (ret) > > > + return ret; > > > + > > > + /* Initialize the hw_device_t in the instance camera3_module_t. */ > > > + cameraDevice_.common.tag = HARDWARE_DEVICE_TAG; > > > + cameraDevice_.common.version = CAMERA_DEVICE_API_VERSION_3_3; > > > + cameraDevice_.common.module = (hw_module_t *)hardwareModule; > > > + > > > + /* > > > + * The camera device operations. These actually implement > > > + * the Android Camera HALv3 interface. > > > + */ > > > + cameraDevice_.ops = &hal_dev_ops; > > > + cameraDevice_.priv = this; > > > + > > > + open_ = true; > > > + > > > + return 0; > > > +} > > > + > > > +void CameraProxy::close() > > > +{ > > > + LIBCAMERA_HAL_FUNC_DBG > > > + > > > + ThreadRpc rpcRequest; > > > + rpcRequest.tag = ThreadRpc::Close; > > > + > > > + threadRpcCall(rpcRequest); > > > + > > > + open_ = false; > > > +} > > > + > > > +void CameraProxy::setCallbacks(const camera3_callback_ops_t *callbacks) > > > > I would name this initialize() to match hal_dev_initialize(). > > > > > +{ > > > + LIBCAMERA_HAL_FUNC_DBG > > > + > > > + cameraModule_->setCallbacks(&cameraDevice_, callbacks); > > > +} > > > + > > > +const camera_metadata_t *CameraProxy::constructDefaultRequest(int type) > > > +{ > > > + return cameraModule_->constructDefaultRequestMetadata(type); > > > +} > > > + > > > +int CameraProxy::configureStreams(camera3_stream_configuration_t *stream_list) > > > +{ > > > + return cameraModule_->configureStreams(stream_list); > > > +} > > > + > > > +int CameraProxy::processCaptureRequest(camera3_capture_request_t *request) > > > +{ > > > + ThreadRpc rpcRequest; > > > + rpcRequest.tag = ThreadRpc::ProcessCaptureRequest; > > > + rpcRequest.request = request; > > > + > > > + threadRpcCall(rpcRequest); > > > + > > > + return 0; > > > > Does this method need to be synchronous ? We can keep it as-is for now, > > but I wonder if it wouldn't make sense to later move (part of) the > > validation code here and make the call asynchronous. > > > > > +} > > > + > > > +void CameraProxy::threadRpcCall(ThreadRpc &rpcRequest) > > > +{ > > > + std::unique_ptr<ThreadRpcMessage> message = > > > + utils::make_unique<ThreadRpcMessage>(); > > > + message->rpc = &rpcRequest; > > > + > > > + cameraModule_->postMessage(std::move(message)); > > > + rpcRequest.waitDelivery(); > > > +} > > > diff --git a/src/android/camera_proxy.h b/src/android/camera_proxy.h > > > new file mode 100644 > > > index 000000000000..69e6878c4352 > > > --- /dev/null > > > +++ b/src/android/camera_proxy.h > > > @@ -0,0 +1,41 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2019, Google Inc. > > > + * > > > + * camera_proxy.h - Proxy to camera module instances > > > + */ > > > +#ifndef __ANDROID_CAMERA_PROXY_H__ > > > +#define __ANDROID_CAMERA_PROXY_H__ > > > + > > > +#include <hardware/camera3.h> > > > +#include <libcamera/libcamera.h> > > > + > > > +#include "camera_module.h" > > > + > > > +class CameraProxy > > > +{ > > > +public: > > > + CameraProxy(unsigned int id, CameraModule *cameraModule); > > > + > > > + int open(const hw_module_t *hardwareModule); > > > + void close(); > > > + > > > + void setCallbacks(const camera3_callback_ops_t *callbacks); > > > + const camera_metadata_t *constructDefaultRequest(int type); > > > + int configureStreams(camera3_stream_configuration_t *stream_list); > > > + int processCaptureRequest(camera3_capture_request_t *request); > > > + > > > + bool isOpen() const { return open_; } > > > > This isn't used. > > > > > + unsigned int id() const { return id_; } > > > + camera3_device_t *device() { return &cameraDevice_; } > > > + > > > +private: > > > + void threadRpcCall(ThreadRpc &rpcRequest); > > > + > > > + bool open_; > > > > And neither is this, if you drop the isOpen() method. > > > > > + unsigned int id_; > > > + CameraModule *cameraModule_; > > > + camera3_device_t cameraDevice_; > > > +}; > > > + > > > +#endif /* __ANDROID_CAMERA_PROXY_H__ */ > > > diff --git a/src/android/meson.build b/src/android/meson.build > > > index 1f242953db37..63b45832c0d2 100644 > > > --- a/src/android/meson.build > > > +++ b/src/android/meson.build > > > @@ -1,3 +1,11 @@ > > > +android_hal_sources = files([ > > > + 'camera3_hal.cpp', > > > + 'camera_hal_manager.cpp', > > > + 'camera_module.cpp', > > > + 'camera_proxy.cpp', > > > + 'thread_rpc.cpp' > > > +]) > > > + > > > android_camera_metadata_sources = files([ > > > 'metadata/camera_metadata.c', > > > ]) > > > diff --git a/src/android/thread_rpc.cpp b/src/android/thread_rpc.cpp > > > new file mode 100644 > > > index 000000000000..f13db59d0d75 > > > --- /dev/null > > > +++ b/src/android/thread_rpc.cpp > > > @@ -0,0 +1,41 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2019, Google Inc. > > > + * > > > + * thread_call.cpp - Inter-thread procedure call > > > > thread_rpc.cpp > > > > > + */ > > > + > > > +#include "message.h" > > > +#include "thread_rpc.h" > > > + > > > +using namespace libcamera; > > > + > > > +libcamera::Message::Type ThreadRpcMessage::rpcType_ = Message::Type::None; > > > + > > > +ThreadRpcMessage::ThreadRpcMessage() > > > + : Message(type()) > > > +{ > > > +} > > > + > > > +void ThreadRpc::notifyReception() > > > +{ > > > + { > > > + libcamera::MutexLocker locker(mutex_); > > > + delivered_ = true; > > > + } > > > + cv_.notify_one(); > > > +} > > > + > > > +void ThreadRpc::waitDelivery() > > > +{ > > > + libcamera::MutexLocker locker(mutex_); > > > + cv_.wait(locker, [&]{ return delivered_; }); > > > +} > > > + > > > +Message::Type ThreadRpcMessage::type() > > > +{ > > > + if (ThreadRpcMessage::rpcType_ == Message::Type::None) > > > + rpcType_ = Message::registerMessageType(); > > > + > > > + return rpcType_; > > > +} > > > diff --git a/src/android/thread_rpc.h b/src/android/thread_rpc.h > > > new file mode 100644 > > > index 000000000000..173caba1a515 > > > --- /dev/null > > > +++ b/src/android/thread_rpc.h > > > @@ -0,0 +1,56 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2019, Google Inc. > > > + * > > > + * thread_call.h - Inter-thread procedure call > > > + */ > > > +#ifndef __ANDROID_THREAD_CALL_H__ > > > +#define __ANDROID_THREAD_CALL_H__ > > > > s/CALL/RPC/ > > > > > + > > > +#include <condition_variable> > > > +#include <string> > > > > Do you need string ? > > > > You should #include <mutex> > > > > > + > > > +#include <hardware/camera3.h> > > > +#include <libcamera/libcamera.h> > > > + > > > +#include "message.h" > > > +#include "thread.h" > > > + > > > +class ThreadRpc > > > +{ > > > +public: > > > + enum RpcTag { > > > + ProcessCaptureRequest, > > > + Close, > > > + }; > > > + > > > + ThreadRpc() > > > + : delivered_(false) {} > > > + > > > + void notifyReception(); > > > + void waitDelivery(); > > > + > > > + RpcTag tag; > > > + > > > + camera3_capture_request_t *request; > > > + > > > +private: > > > + bool delivered_; > > > + std::mutex mutex_; > > > + std::condition_variable cv_; > > > +}; > > > + > > > +class ThreadRpcMessage : public libcamera::Message > > > +{ > > > +public: > > > + ThreadRpcMessage(); > > > + ThreadRpc *rpc; > > > + > > > + static Message::Type type(); > > > + > > > +private: > > > + static libcamera::Message::Type rpcType_; > > > +}; > > > > FYI, I'll probably move part of this to the libcamera core. > > Let's merge the HAL first, if you're not working on this already > please. I'm not, don't worry :-) It will be a change on top. > > > + > > > +#endif /* __ANDROID_THREAD_CALL_H__ */ > > > + > > > > [snip]
diff --git a/meson_options.txt b/meson_options.txt index 97efc85b4412..2d78b8d91f9c 100644 --- a/meson_options.txt +++ b/meson_options.txt @@ -1,3 +1,8 @@ +option('android', + type : 'boolean', + value : false, + description : 'Compile libcamera with Android Camera3 HAL interface') + option('documentation', type : 'boolean', description : 'Generate the project documentation') diff --git a/src/android/camera3_hal.cpp b/src/android/camera3_hal.cpp new file mode 100644 index 000000000000..0a97a9333d20 --- /dev/null +++ b/src/android/camera3_hal.cpp @@ -0,0 +1,130 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * camera3_hal.cpp - Android Camera3 HAL module + */ + +#include <iostream> +#include <memory> +#include <vector> + +#include <hardware/hardware.h> +#include <system/camera_metadata.h> + +#include <libcamera/libcamera.h> + +#include "log.h" + +#include "camera_hal_manager.h" +#include "camera_module.h" +#include "camera_proxy.h" + +using namespace libcamera; + +LOG_DEFINE_CATEGORY(HAL) + +static CameraHalManager cameraManager; + +/*------------------------------------------------------------------------------ + * Android Camera HAL callbacks + */ + +static int hal_get_number_of_cameras(void) +{ + return cameraManager.numCameras(); +} + +static int hal_get_camera_info(int id, struct camera_info *info) +{ + return cameraManager.getCameraInfo(id, info); +} + +static int hal_set_callbacks(const camera_module_callbacks_t *callbacks) +{ + return 0; +} + +static int hal_open_legacy(const struct hw_module_t *module, const char *id, + uint32_t halVersion, struct hw_device_t **device) +{ + return -ENOSYS; +} + +static int hal_set_torch_mode(const char *camera_id, bool enabled) +{ + return -ENOSYS; +} + +/* + * First entry point of the camera HAL module. + * + * Initialize the HAL but does not open any camera device yet (see hal_dev_open) + */ +static int hal_init() +{ + LOG(HAL, Info) << "Initialising Android camera HAL"; + + cameraManager.initialize(); + + return 0; +} + +/*------------------------------------------------------------------------------ + * Android Camera Device + */ + +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 *cameraModule = reinterpret_cast<CameraProxy *>(dev->priv); + + return cameraManager.close(cameraModule); +} + +static int hal_dev_open(const hw_module_t* module, const char* name, + hw_device_t** device) +{ + LOG(HAL, Debug) << "Open camera: " << name; + + int id = atoi(name); + CameraProxy *proxy = cameraManager.open(id, module); + if (!proxy) { + LOG(HAL, Error) << "Failed to open camera module " << id; + return -ENODEV; + } + proxy->device()->common.close = hal_dev_close; + *device = &proxy->device()->common; + + return 0; +} + +static struct hw_module_methods_t hal_module_methods = { + .open = hal_dev_open, +}; + +camera_module_t HAL_MODULE_INFO_SYM = { + .common = { + .tag = HARDWARE_MODULE_TAG, + .module_api_version = CAMERA_MODULE_API_VERSION_2_4, + .hal_api_version = HARDWARE_HAL_API_VERSION, + .id = CAMERA_HARDWARE_MODULE_ID, + .name = "Libcamera Camera3HAL Module", + .author = "libcamera", + .methods = &hal_module_methods, + .dso = nullptr, + .reserved = {}, + }, + + .get_number_of_cameras = hal_get_number_of_cameras, + .get_camera_info = hal_get_camera_info, + .set_callbacks = hal_set_callbacks, + .get_vendor_tag_ops = nullptr, + .open_legacy = hal_open_legacy, + .set_torch_mode = hal_set_torch_mode, + .init = hal_init, + .reserved = {}, +}; diff --git a/src/android/camera_hal_manager.cpp b/src/android/camera_hal_manager.cpp new file mode 100644 index 000000000000..734b5eebd9a5 --- /dev/null +++ b/src/android/camera_hal_manager.cpp @@ -0,0 +1,173 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * camera_hal_manager.cpp - Libcamera Android Camera Manager + */ + +#include "camera_hal_manager.h" + +#include <map> + +#include <hardware/hardware.h> +#include <system/camera_metadata.h> + +#include <libcamera/libcamera.h> +#include <libcamera/camera.h> + +#include "log.h" + +#include "camera_module.h" +#include "camera_proxy.h" + +using namespace libcamera; +LOG_DECLARE_CATEGORY(HAL); + +CameraHalManager::~CameraHalManager() +{ + for (auto metadata : staticMetadataMap_) + free_camera_metadata(metadata.second); +} + +int CameraHalManager::initialize() +{ + /* + * Thread::start() will create a std::thread which will execute + * CameraHalManager::run() + */ + start(); + + /* + * Wait for run() to notify us before returning to the caller to + * make sure libcamera is fully initialized before we start handling + * calls from the camera stack. + */ + MutexLocker locker(mutex_); + cv_.wait(locker); + + return 0; +} + +void CameraHalManager::run() +{ + /* + * The run() operation is scheduled in its own thread; + * It exec() to run the even dispatcher loop and initialize libcamera. + * + * All the libcamera components initialized from here will be tied to + * the same thread as the here below run event dispatcher. + */ + libcameraManager_ = libcamera::CameraManager::instance(); + + int ret = libcameraManager_->start(); + if (ret) { + LOG(HAL, Error) << "Failed to start camera manager: " + << strerror(-ret); + return; + } + + /* + * For each Camera registered in the system, a CameraModule + * get created here with an associated Camera and a proxy. + * + * \todo Support camera hotplug. + */ + unsigned int cameraIndex = 0; + for (auto camera : libcameraManager_->cameras()) { + cameras_.push_back(camera); + + CameraModule *module = new CameraModule(cameraIndex, + camera.get()); + modules_.emplace_back(module); + + CameraProxy *proxy = new CameraProxy(cameraIndex, + module); + proxies_.emplace_back(proxy); + + ++cameraIndex; + } + + /* + * libcamera has been initialized! Unlock the initialize() caller + * as we're now ready to handle calls from the framework. + */ + cv_.notify_one(); + + /* Now start processing events and messages! */ + exec(); +} + +CameraProxy *CameraHalManager::open(unsigned int id, + const hw_module_t *hardwareModule) +{ + if (id < 0 || id >= numCameras()) { + LOG(HAL, Error) << "Invalid camera id '" << id << "'"; + return nullptr; + } + + CameraProxy *proxy = proxies_[id].get(); + if (proxy->open(hardwareModule)) + return proxy; + + LOG(HAL, Info) << "Camera: '" << id << "' opened"; + + return proxy; +} + +int CameraHalManager::close(CameraProxy *proxy) +{ + proxy->close(); + LOG(HAL, Info) << "Close camera: " << proxy->id(); + + return 0; +} + +unsigned int CameraHalManager::numCameras() const +{ + return libcameraManager_->cameras().size(); +} + +/* + * Before the camera framework even tries to open a camera device with + * hal_dev_open() it requires the camera HAL to report a list of static + * informations on the camera device with id \a id in the hal_get_camera_info() + * method. + * + * The static metadata should be then generated probably from a + * platform-specific module, as we cannot operate on the camera at this time as + * it has not yet been open by the framework. + * + * This is what the Intel XML file is used for, it is parsed and the data there + * combined with informations from the PSL service (which I -think- it's what + * our IPA is) to generate a list of static metadata per-camera device. + */ +int CameraHalManager::getCameraInfo(int id, struct camera_info *info) +{ + int cameras = numCameras(); + if (id >= cameras || id < 0 || !info) { + LOG(HAL, Error) << "Invalid camera id: " << id; + return -EINVAL; + } + + camera_metadata_t *staticMetadata; + auto it = staticMetadataMap_.find(id); + if (it != staticMetadataMap_.end()) { + staticMetadata = it->second; + } else { + CameraModule *cameraModule = modules_[id].get(); + + staticMetadata = cameraModule->getStaticMetadata(); + staticMetadataMap_[id] = staticMetadata; + } + + /* \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 = staticMetadata; + info->conflicting_devices = nullptr; + info->conflicting_devices_length = 0; + + return 0; +} diff --git a/src/android/camera_hal_manager.h b/src/android/camera_hal_manager.h new file mode 100644 index 000000000000..65d6fa3150ef --- /dev/null +++ b/src/android/camera_hal_manager.h @@ -0,0 +1,56 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * camera_hal_manager.h - Libcamera Android Camera Manager + */ +#ifndef __ANDROID_CAMERA_MANAGER_H__ +#define __ANDROID_CAMERA_MANAGER_H__ + +#include <condition_variable> +#include <cstddef> +#include <map> +#include <memory> +#include <vector> + +#include <hardware/hardware.h> +#include <system/camera_metadata.h> + +#include <libcamera/libcamera.h> + +#include "thread.h" +#include "thread_rpc.h" + +class CameraModule; +class CameraProxy; + +class CameraHalManager : public libcamera::Thread +{ +public: + ~CameraHalManager(); + + int initialize(); + + CameraProxy *open(unsigned int id, const hw_module_t *module); + int close(CameraProxy *proxy); + + unsigned int numCameras() const; + int getCameraInfo(int id, struct camera_info *info); + +private: + void run() override; + camera_metadata_t *getStaticMetadata(unsigned int id); + + libcamera::CameraManager *libcameraManager_; + + std::mutex mutex_; + std::condition_variable cv_; + + std::vector<std::shared_ptr<libcamera::Camera>> cameras_; + std::vector<std::unique_ptr<CameraModule>> modules_; + std::vector<std::unique_ptr<CameraProxy>> proxies_; + + std::map<unsigned int, camera_metadata_t *> staticMetadataMap_; +}; + +#endif /* __ANDROID_CAMERA_MANAGER_H__ */ diff --git a/src/android/camera_module.cpp b/src/android/camera_module.cpp new file mode 100644 index 000000000000..b64e377fb439 --- /dev/null +++ b/src/android/camera_module.cpp @@ -0,0 +1,799 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * camera_module.cpp - Libcamera Android Camera Module + */ + +#include "camera_module.h" + +#include <iostream> +#include <memory> + +#include <system/camera_metadata.h> + +#include <hardware/camera3.h> +#include <libcamera/libcamera.h> + +#include "message.h" + +#include "log.h" + +using namespace libcamera; + +LOG_DECLARE_CATEGORY(HAL); + +CameraModule::CameraModule(unsigned int id, libcamera::Camera *camera) + : id_(id), camera_(camera), cameraState_(CameraStopped), + requestTemplate_(nullptr) +{ + camera_->requestCompleted.connect(this, + &CameraModule::requestComplete); +} + +CameraModule::~CameraModule() +{ + if (requestTemplate_) + free_camera_metadata(requestTemplate_); + requestTemplate_ = nullptr; +} + +void CameraModule::message(Message *message) +{ + ThreadRpcMessage *rpcMessage = static_cast<ThreadRpcMessage *> + (message); + + if (rpcMessage->type() != ThreadRpcMessage::type()) + return Object::message(message); + + ThreadRpc *rpc = rpcMessage->rpc; + + switch (rpc->tag) { + case ThreadRpc::ProcessCaptureRequest: + processCaptureRequest(rpc->request); + break; + case ThreadRpc::Close: + close(); + break; + default: + LOG(HAL, Error) << "Unknown RPC operation: " << rpc->tag; + } + + rpc->notifyReception(); +} + +int CameraModule::open() +{ + int ret = camera_->acquire(); + if (ret) { + LOG(HAL, Error) << "Failed to acquire the camera"; + return ret; + } + + return 0; +} + +void CameraModule::close() +{ + camera_->stop(); + + camera_->freeBuffers(); + camera_->release(); + + cameraState_ = CameraStopped; +} + +void CameraModule::setCallbacks(const struct camera3_device *camera3Device, + const camera3_callback_ops_t *callbacks) +{ + camera3Device_ = camera3Device; + callbacks_ = callbacks; +} + +camera_metadata_t *CameraModule::getStaticMetadata() +{ + int ret; + + /* + * The below metadata has been added by inspecting the Intel XML + * file and it's associated parsing routines in the Intel IPU3 HAL. + * + * The here below metadata are enough to satisfy the camera3-test + * provided by ChromeOS, but a real camera implementation might require + * more. + * + * See CameraConf::AiqConf class implementation in the Intel HAL + * to get a list of the static metadata registered by parsing the + * XML config file in AiqConf::fillStaticMetadataFromCMC + */ + + /* + * FIXME: this sizes come from the Intel IPU3 HAL, and are way too large for + * this intial HAL version. + */ + #define STATIC_ENTRY_CAP 256 + #define STATIC_DATA_CAP 6688 + camera_metadata_t *staticMetadata = + allocate_camera_metadata(STATIC_ENTRY_CAP, STATIC_DATA_CAP); + + /* Sensor static metadata. */ + int32_t pixelArraySize[] = { + 2592, 1944, + }; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_SENSOR_INFO_PIXEL_ARRAY_SIZE, + &pixelArraySize, 2); + METADATA_ASSERT(ret); + + int32_t sensorSizes[] = { + 0, 0, 2560, 1920, + }; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_SENSOR_INFO_ACTIVE_ARRAY_SIZE, + &sensorSizes, 4); + METADATA_ASSERT(ret); + + int32_t sensitivityRange[] = { + 32, 2400, + }; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_SENSOR_INFO_SENSITIVITY_RANGE, + &sensitivityRange, 2); + METADATA_ASSERT(ret); + + uint16_t filterArr = ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT_GRBG; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_SENSOR_INFO_COLOR_FILTER_ARRANGEMENT, + &filterArr, 1); + METADATA_ASSERT(ret); + + int64_t exposureTimeRange[] = { + 100000, 200000000, + }; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_SENSOR_INFO_EXPOSURE_TIME_RANGE, + &exposureTimeRange, 2); + METADATA_ASSERT(ret); + + int32_t orientation = 0; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_SENSOR_ORIENTATION, + &orientation, 1); + METADATA_ASSERT(ret); + + /* Flash static metadata. */ + char flashAvailable = ANDROID_FLASH_INFO_AVAILABLE_FALSE; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_FLASH_INFO_AVAILABLE, + &flashAvailable, 1); + METADATA_ASSERT(ret); + + /* Lens static metadata. */ + float fn = 2.53 / 100; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_LENS_INFO_AVAILABLE_APERTURES, &fn, 1); + METADATA_ASSERT(ret); + + /* Control metadata. */ + char controlMetadata = ANDROID_CONTROL_MODE_AUTO; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_CONTROL_AVAILABLE_MODES, + &controlMetadata, 1); + METADATA_ASSERT(ret); + + char availableAntiBandingModes[] = { + ANDROID_CONTROL_AE_ANTIBANDING_MODE_OFF, + ANDROID_CONTROL_AE_ANTIBANDING_MODE_50HZ, + ANDROID_CONTROL_AE_ANTIBANDING_MODE_60HZ, + ANDROID_CONTROL_AE_ANTIBANDING_MODE_AUTO, + }; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_CONTROL_AE_AVAILABLE_ANTIBANDING_MODES, + availableAntiBandingModes, 4); + METADATA_ASSERT(ret); + + char aeAvailableModes[] = { + ANDROID_CONTROL_AE_MODE_ON, + ANDROID_CONTROL_AE_MODE_OFF, + }; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_CONTROL_AE_AVAILABLE_MODES, + aeAvailableModes, 2); + METADATA_ASSERT(ret); + + controlMetadata = ANDROID_CONTROL_AE_LOCK_AVAILABLE_TRUE; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_CONTROL_AE_LOCK_AVAILABLE, + &controlMetadata, 1); + METADATA_ASSERT(ret); + + uint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, + &awbLockAvailable, 1); + + /* Scaler static metadata. */ + std::vector<uint32_t> availableStreamFormats = { + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, + ANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, + ANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, + }; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_SCALER_AVAILABLE_FORMATS, + availableStreamFormats.data(), + availableStreamFormats.size()); + METADATA_ASSERT(ret); + + std::vector<uint32_t> availableStreamConfigurations = { + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT, + ANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, 2560, 1920, + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT, + ANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, 2560, 1920, + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS_OUTPUT, + }; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_SCALER_AVAILABLE_STREAM_CONFIGURATIONS, + availableStreamConfigurations.data(), + availableStreamConfigurations.size()); + METADATA_ASSERT(ret); + + std::vector<int64_t> availableStallDurations = { + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, 33333333, + }; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_SCALER_AVAILABLE_STALL_DURATIONS, + availableStallDurations.data(), + availableStallDurations.size()); + METADATA_ASSERT(ret); + + std::vector<int64_t> minFrameDurations = { + ANDROID_SCALER_AVAILABLE_FORMATS_BLOB, 2560, 1920, 33333333, + ANDROID_SCALER_AVAILABLE_FORMATS_IMPLEMENTATION_DEFINED, 2560, 1920, 33333333, + ANDROID_SCALER_AVAILABLE_FORMATS_YCbCr_420_888, 2560, 1920, 33333333, + }; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_SCALER_AVAILABLE_MIN_FRAME_DURATIONS, + minFrameDurations.data(), minFrameDurations.size()); + METADATA_ASSERT(ret); + + /* Info static metadata. */ + uint8_t supportedHWLevel = ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL_LIMITED; + ret = add_camera_metadata_entry(staticMetadata, + ANDROID_INFO_SUPPORTED_HARDWARE_LEVEL, + &supportedHWLevel, 1); + + return staticMetadata; +} + +const camera_metadata_t *CameraModule::constructDefaultRequestMetadata(int type) +{ + int ret; + + /* + * TODO: inspect type and pick the right metadata pack. + * As of now just use a single one for all templates + */ + uint8_t captureIntent; + switch (type) { + case CAMERA3_TEMPLATE_PREVIEW: + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_PREVIEW; + break; + case CAMERA3_TEMPLATE_STILL_CAPTURE: + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_STILL_CAPTURE; + break; + case CAMERA3_TEMPLATE_VIDEO_RECORD: + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_VIDEO_RECORD; + break; + case CAMERA3_TEMPLATE_VIDEO_SNAPSHOT: + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_VIDEO_SNAPSHOT; + break; + case CAMERA3_TEMPLATE_ZERO_SHUTTER_LAG: + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_ZERO_SHUTTER_LAG; + break; + case CAMERA3_TEMPLATE_MANUAL: + captureIntent = ANDROID_CONTROL_CAPTURE_INTENT_MANUAL; + break; + default: + LOG(HAL, Error) << "Invalid template request type: " << type; + return nullptr; + } + + if (requestTemplate_) + return requestTemplate_; + + /* FIXME: this sizes are quite arbitrary ones */ + #define REQUEST_TEMPLATE_ENTRIES 30 + #define REQUEST_TEMPLATE_DATA 2048 + requestTemplate_ = allocate_camera_metadata(REQUEST_TEMPLATE_ENTRIES, + REQUEST_TEMPLATE_DATA); + if (!requestTemplate_) { + LOG(HAL, Error) << "Failed to allocate template metadata"; + return nullptr; + } + + /* + * Fill in the required Request metadata. + * + * Part (most?) of this entries comes from inspecting the Intel's IPU3 + * HAL xml configuration file. + */ + + /* Set to 0 the number of 'processed and stalling' streams (ie JPEG). */ + int32_t maxOutStream[] = { 0, 2, 0 }; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_REQUEST_MAX_NUM_OUTPUT_STREAMS, + maxOutStream, 3); + METADATA_ASSERT(ret); + + uint8_t maxPipelineDepth = 5; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_REQUEST_PIPELINE_MAX_DEPTH, + &maxPipelineDepth, 1); + METADATA_ASSERT(ret); + + int32_t inputStreams = 0; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_REQUEST_MAX_NUM_INPUT_STREAMS, + &inputStreams, 1); + METADATA_ASSERT(ret); + + int32_t partialResultCount = 1; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_REQUEST_PARTIAL_RESULT_COUNT, + &partialResultCount, 1); + METADATA_ASSERT(ret); + + uint8_t availableCapabilities[] = { + ANDROID_REQUEST_AVAILABLE_CAPABILITIES_BACKWARD_COMPATIBLE, + }; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_REQUEST_AVAILABLE_CAPABILITIES, + availableCapabilities, 1); + METADATA_ASSERT(ret); + + uint8_t aeMode = ANDROID_CONTROL_AE_MODE_ON; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_CONTROL_AE_MODE, + &aeMode, 1); + METADATA_ASSERT(ret); + + int32_t aeExposureCompensation = 0; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, + &aeExposureCompensation, 1); + METADATA_ASSERT(ret); + + uint8_t aePrecaptureTrigger = ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER_IDLE; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, + &aePrecaptureTrigger, 1); + METADATA_ASSERT(ret); + + uint8_t aeLock = ANDROID_CONTROL_AE_LOCK_OFF; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_CONTROL_AE_LOCK, + &aeLock, 1); + METADATA_ASSERT(ret); + + uint8_t afTrigger = ANDROID_CONTROL_AF_TRIGGER_IDLE; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_CONTROL_AF_TRIGGER, + &afTrigger, 1); + METADATA_ASSERT(ret); + + uint8_t awbMode = ANDROID_CONTROL_AWB_MODE_AUTO; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_CONTROL_AWB_MODE, + &awbMode, 1); + METADATA_ASSERT(ret); + + uint8_t awbLock = ANDROID_CONTROL_AWB_LOCK_OFF; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_CONTROL_AWB_LOCK, + &awbLock, 1); + METADATA_ASSERT(ret); + + uint8_t awbLockAvailable = ANDROID_CONTROL_AWB_LOCK_AVAILABLE_FALSE; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, + &awbLockAvailable, 1); + METADATA_ASSERT(ret); + + uint8_t flashMode = ANDROID_FLASH_MODE_OFF; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_FLASH_MODE, + &flashMode, 1); + METADATA_ASSERT(ret); + + uint8_t faceDetectMode = ANDROID_STATISTICS_FACE_DETECT_MODE_OFF; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_STATISTICS_FACE_DETECT_MODE, + &faceDetectMode, 1); + METADATA_ASSERT(ret); + + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_CONTROL_CAPTURE_INTENT, + &captureIntent, 1); + METADATA_ASSERT(ret); + + /* + * This is quite hard to list at the moment wihtout knowing what + * we could control. + * + * For now, just list in the available Request keys and in the available + * result keys the control and reporting of the AE algorithm. + */ + std::vector<int32_t> availableRequestKeys = { + ANDROID_CONTROL_AE_MODE, + ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, + ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, + ANDROID_CONTROL_AE_LOCK, + ANDROID_CONTROL_AF_TRIGGER, + ANDROID_CONTROL_AWB_MODE, + ANDROID_CONTROL_AWB_LOCK, + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, + ANDROID_CONTROL_CAPTURE_INTENT, + ANDROID_FLASH_MODE, + ANDROID_STATISTICS_FACE_DETECT_MODE, + }; + + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_REQUEST_AVAILABLE_REQUEST_KEYS, + availableRequestKeys.data(), + availableRequestKeys.size()); + METADATA_ASSERT(ret); + + std::vector<int32_t> availableResultKeys = { + ANDROID_CONTROL_AE_MODE, + ANDROID_CONTROL_AE_EXPOSURE_COMPENSATION, + ANDROID_CONTROL_AE_PRECAPTURE_TRIGGER, + ANDROID_CONTROL_AE_LOCK, + ANDROID_CONTROL_AF_TRIGGER, + ANDROID_CONTROL_AWB_MODE, + ANDROID_CONTROL_AWB_LOCK, + ANDROID_CONTROL_AWB_LOCK_AVAILABLE, + ANDROID_CONTROL_CAPTURE_INTENT, + ANDROID_FLASH_MODE, + ANDROID_STATISTICS_FACE_DETECT_MODE, + }; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_REQUEST_AVAILABLE_RESULT_KEYS, + availableResultKeys.data(), + availableResultKeys.size()); + METADATA_ASSERT(ret); + + /* + * The available characteristics should be the tags reported + * as part of the static metadata reported at hal_get_camera_info() + * time. The xml file sets those to 0 though. + */ + std::vector<int32_t> availableCharacteristicsKeys = {}; + ret = add_camera_metadata_entry(requestTemplate_, + ANDROID_REQUEST_AVAILABLE_CHARACTERISTICS_KEYS, + availableCharacteristicsKeys.data(), + availableCharacteristicsKeys.size()); + METADATA_ASSERT(ret); + + return requestTemplate_; +} + +/** + * Inspect the stream_list to produce a list of StreamConfiguration to + * be use to configure the Camera. + */ +int CameraModule::configureStreams(camera3_stream_configuration_t *stream_list) +{ + + for (unsigned int i = 0; i < stream_list->num_streams; ++i) { + camera3_stream_t *stream = stream_list->streams[i]; + + LOG(HAL, Info) << "Stream #" << i + << ": direction: " << stream->stream_type + << " - width: " << stream->width + << " - height: " << stream->height + << " - format: " << std::hex << stream->format; + } + + /* Hardcode viewfinder role, collecting sizes from the stream config. */ + if (stream_list->num_streams != 1) { + LOG(HAL, Error) << "Only one stream supported"; + return -EINVAL; + } + + StreamRoles roles = {}; + roles.push_back(StreamRole::StillCapture); + + config_ = camera_->generateConfiguration(roles); + if (!config_ || config_->empty()) { + LOG(HAL, Error) << "Failed to generate camera configuration"; + return -EINVAL; + } + + /* Only one stream is supported. */ + camera3_stream_t *camera3Stream = stream_list->streams[0]; + StreamConfiguration *streamConfiguration = &config_->at(0); + streamConfiguration->size.width = camera3Stream->width; + streamConfiguration->size.height = camera3Stream->height; + streamConfiguration->memoryType = ExternalMemory; + + /* + * FIXME: do not change the pixel format from the one returned + * from the Camera::generateConfiguration(); + * + * We'll need to translate from Android defined pixel format codes + * to whatever libcamera will use. + */ + + switch (config_->validate()) { + case CameraConfiguration::Valid: + break; + case CameraConfiguration::Adjusted: + LOG(HAL, Info) << "Camera configuration adjusted"; + return -EINVAL; + case CameraConfiguration::Invalid: + LOG(HAL, Info) << "Camera configuration invalid"; + config_.reset(); + return -EINVAL; + } + + camera3Stream->max_buffers = streamConfiguration->bufferCount; + + /* + * Once the CameraConfiguration has been adjusted/validated + * it can be applied to the camera. + */ + int ret = camera_->configure(config_.get()); + if (ret) { + LOG(HAL, Error) << "Failed to configure camera '" + << camera_->name() << "'"; + return ret; + } + + return 0; +} + +int CameraModule::processCaptureRequest(camera3_capture_request_t *camera3Request) +{ + StreamConfiguration *streamConfiguration = &config_->at(0); + Stream *stream = streamConfiguration->stream(); + + if (camera3Request->num_output_buffers != 1) { + LOG(HAL, Error) << "Invalid number of output buffers: " + << camera3Request->num_output_buffers; + return -EINVAL; + } + + /* Start the camera if that's the first request we handle. */ + if (cameraState_ == CameraStopped) { + int ret = camera_->allocateBuffers(); + if (ret) { + LOG(HAL, Error) << "Failed to allocate buffers"; + return ret; + } + + ret = camera_->start(); + if (ret) { + LOG(HAL, Error) << "Failed to start camera"; + camera_->freeBuffers(); + return ret; + } + + cameraState_ = CameraRunning; + } + + /* + * Queue a request for the Camera with the provided dmabuf file + * descriptors. + */ + const camera3_stream_buffer_t *camera3Buffer = + &camera3Request->output_buffers[0]; + const buffer_handle_t camera3Handle = *camera3Buffer->buffer; + + std::array<int, 3> fds = { + camera3Handle->data[0], + camera3Handle->data[1], + camera3Handle->data[2], + }; + std::unique_ptr<Buffer> buffer = stream->createBuffer(fds); + if (!buffer) { + LOG(HAL, Error) << "Failed to create buffer"; + return -EINVAL; + } + + Request *request = camera_->createRequest(); + request->addBuffer(std::move(buffer)); + int ret = camera_->queueRequest(request); + if (ret) { + LOG(HAL, Error) << "Failed to queue request"; + return ret; + } + + /* Save the request descriptors for use at completion time. */ + Camera3RequestDescriptor descriptor = {}; + descriptor.frameNumber = camera3Request->frame_number, + descriptor.numBuffers = camera3Request->num_output_buffers, + descriptor.buffers = new camera3_stream_buffer_t[descriptor.numBuffers]; + for (unsigned int i = 0; i < descriptor.numBuffers; ++i) { + camera3_stream_buffer_t &buffer = descriptor.buffers[i]; + buffer = camera3Request->output_buffers[i]; + } + + requestMap_[request] = descriptor; + + return 0; +} + +void CameraModule::requestComplete(Request *request, + const std::map<Stream *, Buffer *> &buffers) +{ + Buffer *libcameraBuffer = buffers.begin()->second; + camera3_buffer_status status = CAMERA3_BUFFER_STATUS_OK; + camera_metadata_t *resultMetadata = nullptr; + + if (request->status() != Request::RequestComplete) { + LOG(HAL, Error) << "Request not succesfully completed: " + << request->status(); + status = CAMERA3_BUFFER_STATUS_ERROR; + } + + /* Prepare to call back the Android camera stack. */ + Camera3RequestDescriptor &descriptor = requestMap_[request]; + + camera3_capture_result_t captureResult = {}; + captureResult.frame_number = descriptor.frameNumber; + captureResult.num_output_buffers = descriptor.numBuffers; + + camera3_stream_buffer_t *resultBuffers = + new camera3_stream_buffer_t[descriptor.numBuffers]; + for (unsigned int i = 0; i < descriptor.numBuffers; ++i) { + camera3_stream_buffer_t resultBuffer; + + resultBuffer = descriptor.buffers[i]; + resultBuffer.acquire_fence = -1; + resultBuffer.release_fence = -1; + resultBuffer.status = status; + + resultBuffers[i] = resultBuffer; + } + captureResult.output_buffers = + const_cast<const camera3_stream_buffer_t *>(resultBuffers); + + if (status == CAMERA3_BUFFER_STATUS_ERROR) { + /* \todo Improve error handling. */ + notifyError(descriptor.frameNumber, resultBuffers->stream); + } else { + notifyShutter(descriptor.frameNumber, libcameraBuffer->timestamp()); + + captureResult.partial_result = 1; + resultMetadata = getResultMetadata(descriptor.frameNumber, + libcameraBuffer->timestamp()); + captureResult.result = resultMetadata; + } + + callbacks_->process_capture_result(callbacks_, &captureResult); + + if (resultMetadata) + free_camera_metadata(resultMetadata); + + delete[] resultBuffers; + delete[] descriptor.buffers; + requestMap_.erase(request); + + return; +} + +void CameraModule::notifyShutter(uint32_t frameNumber, uint64_t timestamp) +{ + camera3_notify_msg_t notify = {}; + + notify.type = CAMERA3_MSG_SHUTTER; + notify.message.shutter.frame_number = frameNumber; + notify.message.shutter.timestamp = timestamp; + + callbacks_->notify(callbacks_, ¬ify); +} + +void CameraModule::notifyError(uint32_t frameNumber, camera3_stream_t *stream) +{ + camera3_notify_msg_t notify = {}; + + notify.type = CAMERA3_MSG_ERROR; + notify.message.error.error_stream = stream; + notify.message.error.frame_number = frameNumber; + notify.message.error.error_code = CAMERA3_MSG_ERROR_REQUEST; + + callbacks_->notify(callbacks_, ¬ify); +} + +/* + * Fixed result metadata, mostly imported from the UVC camera HAL, which + * does not produces metadata and thus needs to generate a fixed set. + */ +camera_metadata_t *CameraModule::getResultMetadata(int frame_number, + int64_t timestamp) +{ + int ret; + + /* FIXME: random "big enough" values. */ + #define RESULT_ENTRY_CAP 256 + #define RESULT_DATA_CAP 6688 + camera_metadata_t *resultMetadata = + allocate_camera_metadata(STATIC_ENTRY_CAP, STATIC_DATA_CAP); + + const uint8_t ae_state = ANDROID_CONTROL_AE_STATE_CONVERGED; + ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AE_STATE, + &ae_state, 1); + METADATA_ASSERT(ret); + + const uint8_t ae_lock = ANDROID_CONTROL_AE_LOCK_OFF; + ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AE_LOCK, + &ae_lock, 1); + METADATA_ASSERT(ret); + + uint8_t af_state = ANDROID_CONTROL_AF_STATE_INACTIVE; + ret = add_camera_metadata_entry(resultMetadata, ANDROID_CONTROL_AF_STATE, + &af_state, 1); + METADATA_ASSERT(ret); + + const uint8_t awb_state = ANDROID_CONTROL_AWB_STATE_CONVERGED; + ret = add_camera_metadata_entry(resultMetadata, + ANDROID_CONTROL_AWB_STATE, + &awb_state, 1); + METADATA_ASSERT(ret); + + const uint8_t awb_lock = ANDROID_CONTROL_AWB_LOCK_OFF; + ret = add_camera_metadata_entry(resultMetadata, + ANDROID_CONTROL_AWB_LOCK, + &awb_lock, 1); + METADATA_ASSERT(ret); + + const uint8_t lens_state = ANDROID_LENS_STATE_STATIONARY; + ret = add_camera_metadata_entry(resultMetadata, + ANDROID_LENS_STATE, + &lens_state, 1); + METADATA_ASSERT(ret); + + int32_t sensorSizes[] = { + 0, 0, 2560, 1920, + }; + ret = add_camera_metadata_entry(resultMetadata, + ANDROID_SCALER_CROP_REGION, + sensorSizes, 4); + METADATA_ASSERT(ret); + + ret = add_camera_metadata_entry(resultMetadata, + ANDROID_SENSOR_TIMESTAMP, + ×tamp, 1); + + METADATA_ASSERT(ret); + + /* 33.3 msec */ + const int64_t rolling_shutter_skew = 33300000; + ret = add_camera_metadata_entry(resultMetadata, + ANDROID_SENSOR_ROLLING_SHUTTER_SKEW, + &rolling_shutter_skew, 1); + METADATA_ASSERT(ret); + + /* 16.6 msec */ + const int64_t exposure_time = 16600000; + ret = add_camera_metadata_entry(resultMetadata, + ANDROID_SENSOR_EXPOSURE_TIME, + &exposure_time, 1); + METADATA_ASSERT(ret); + + const uint8_t lens_shading_map_mode = + ANDROID_STATISTICS_LENS_SHADING_MAP_MODE_OFF; + ret = add_camera_metadata_entry(resultMetadata, + ANDROID_STATISTICS_LENS_SHADING_MAP_MODE, + &lens_shading_map_mode, 1); + METADATA_ASSERT(ret); + + const uint8_t scene_flicker = ANDROID_STATISTICS_SCENE_FLICKER_NONE; + ret = add_camera_metadata_entry(resultMetadata, + ANDROID_STATISTICS_SCENE_FLICKER, + &scene_flicker, 1); + METADATA_ASSERT(ret); + + return resultMetadata; +} diff --git a/src/android/camera_module.h b/src/android/camera_module.h new file mode 100644 index 000000000000..67488d5940b1 --- /dev/null +++ b/src/android/camera_module.h @@ -0,0 +1,75 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * camera_module.h - Libcamera Android Camera Module + */ +#ifndef __ANDROID_CAMERA_MODULE_H__ +#define __ANDROID_CAMERA_MODULE_H__ + +#include <memory> + +#include <hardware/camera3.h> +#include <libcamera/libcamera.h> + +#include "message.h" + +#include "camera_hal_manager.h" + +#define METADATA_ASSERT(_r) \ + do { \ + if (!(_r)) break; \ + LOG(HAL, Error) << "Error: " << __func__ << ":" << __LINE__; \ + return nullptr; \ + } while(0); + +class CameraModule : public libcamera::Object +{ +public: + CameraModule(unsigned int id, libcamera::Camera *camera); + ~CameraModule(); + + void message(libcamera::Message *message); + + int open(); + void close(); + void setCallbacks(const struct camera3_device *cameraDevice, + const camera3_callback_ops_t *callbacks); + camera_metadata_t *getStaticMetadata(); + const camera_metadata_t *constructDefaultRequestMetadata(int type); + int configureStreams(camera3_stream_configuration_t *stream_list); + int processCaptureRequest(camera3_capture_request_t *request); + void requestComplete(libcamera::Request *request, + const std::map<libcamera::Stream *, libcamera::Buffer *> &buffers); + + unsigned int id() const { return id_; } + +private: + struct Camera3RequestDescriptor { + uint32_t frameNumber; + uint32_t numBuffers; + camera3_stream_buffer_t *buffers; + }; + + enum CameraState { + CameraStopped, + CameraRunning, + }; + + void notifyShutter(uint32_t frameNumber, uint64_t timestamp); + void notifyError(uint32_t frameNumber, camera3_stream_t *stream); + camera_metadata_t *getResultMetadata(int frame_number, int64_t timestamp); + + unsigned int id_; + libcamera::Camera *camera_; + std::unique_ptr<libcamera::CameraConfiguration> config_; + CameraState cameraState_; + + camera_metadata_t *requestTemplate_; + const camera3_callback_ops_t *callbacks_; + const struct camera3_device *camera3Device_; + + std::map<libcamera::Request *, Camera3RequestDescriptor> requestMap_; +}; + +#endif /* __ANDROID_CAMERA_MODULE_H__ */ diff --git a/src/android/camera_proxy.cpp b/src/android/camera_proxy.cpp new file mode 100644 index 000000000000..af7817f29137 --- /dev/null +++ b/src/android/camera_proxy.cpp @@ -0,0 +1,181 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * camera_proxy.cpp - Proxy to camera module instances + */ + +#include "camera_proxy.h" + +#include <hardware/camera3.h> +#include <libcamera/libcamera.h> +#include <system/camera_metadata.h> + +#include <memory> + +#include "log.h" +#include "message.h" +#include "thread_rpc.h" +#include "utils.h" + +using namespace libcamera; + +LOG_DECLARE_CATEGORY(HAL); + +#define LIBCAMERA_HAL_FUNC_DBG \ + LOG(HAL, Debug); + +static int hal_dev_initialize(const struct camera3_device *dev, + const camera3_callback_ops_t *callback_ops) +{ + LIBCAMERA_HAL_FUNC_DBG + + if (!dev) + return -EINVAL; + + CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); + proxy->setCallbacks(callback_ops); + + return 0; +} + +static int hal_dev_configure_streams(const struct camera3_device *dev, + camera3_stream_configuration_t *stream_list) +{ + LIBCAMERA_HAL_FUNC_DBG + + if (!dev) + return -EINVAL; + + CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); + proxy->configureStreams(stream_list); + + return 0; +} + +static const camera_metadata_t * +hal_dev_construct_default_request_settings(const struct camera3_device *dev, + int type) +{ + LIBCAMERA_HAL_FUNC_DBG + + if (!dev) + return nullptr; + + CameraProxy *proxy = reinterpret_cast<CameraProxy *>(dev->priv); + return proxy->constructDefaultRequest(type); +} + +static int hal_dev_process_capture_request(const struct camera3_device *dev, + camera3_capture_request_t *request) +{ + LIBCAMERA_HAL_FUNC_DBG + + 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) +{ + LIBCAMERA_HAL_FUNC_DBG +} + +static int hal_dev_flush(const struct camera3_device *dev) +{ + LIBCAMERA_HAL_FUNC_DBG + + 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 = { 0 }, +}; + +CameraProxy::CameraProxy(unsigned int id, CameraModule *cameraModule) + : open_(false), id_(id), cameraModule_(cameraModule) +{ +} + +int CameraProxy::open(const hw_module_t *hardwareModule) +{ + int ret = cameraModule_->open(); + if (ret) + return ret; + + /* Initialize the hw_device_t in the instance camera3_module_t. */ + cameraDevice_.common.tag = HARDWARE_DEVICE_TAG; + cameraDevice_.common.version = CAMERA_DEVICE_API_VERSION_3_3; + cameraDevice_.common.module = (hw_module_t *)hardwareModule; + + /* + * The camera device operations. These actually implement + * the Android Camera HALv3 interface. + */ + cameraDevice_.ops = &hal_dev_ops; + cameraDevice_.priv = this; + + open_ = true; + + return 0; +} + +void CameraProxy::close() +{ + LIBCAMERA_HAL_FUNC_DBG + + ThreadRpc rpcRequest; + rpcRequest.tag = ThreadRpc::Close; + + threadRpcCall(rpcRequest); + + open_ = false; +} + +void CameraProxy::setCallbacks(const camera3_callback_ops_t *callbacks) +{ + LIBCAMERA_HAL_FUNC_DBG + + cameraModule_->setCallbacks(&cameraDevice_, callbacks); +} + +const camera_metadata_t *CameraProxy::constructDefaultRequest(int type) +{ + return cameraModule_->constructDefaultRequestMetadata(type); +} + +int CameraProxy::configureStreams(camera3_stream_configuration_t *stream_list) +{ + return cameraModule_->configureStreams(stream_list); +} + +int CameraProxy::processCaptureRequest(camera3_capture_request_t *request) +{ + ThreadRpc rpcRequest; + rpcRequest.tag = ThreadRpc::ProcessCaptureRequest; + rpcRequest.request = request; + + threadRpcCall(rpcRequest); + + return 0; +} + +void CameraProxy::threadRpcCall(ThreadRpc &rpcRequest) +{ + std::unique_ptr<ThreadRpcMessage> message = + utils::make_unique<ThreadRpcMessage>(); + message->rpc = &rpcRequest; + + cameraModule_->postMessage(std::move(message)); + rpcRequest.waitDelivery(); +} diff --git a/src/android/camera_proxy.h b/src/android/camera_proxy.h new file mode 100644 index 000000000000..69e6878c4352 --- /dev/null +++ b/src/android/camera_proxy.h @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * camera_proxy.h - Proxy to camera module instances + */ +#ifndef __ANDROID_CAMERA_PROXY_H__ +#define __ANDROID_CAMERA_PROXY_H__ + +#include <hardware/camera3.h> +#include <libcamera/libcamera.h> + +#include "camera_module.h" + +class CameraProxy +{ +public: + CameraProxy(unsigned int id, CameraModule *cameraModule); + + int open(const hw_module_t *hardwareModule); + void close(); + + void setCallbacks(const camera3_callback_ops_t *callbacks); + const camera_metadata_t *constructDefaultRequest(int type); + int configureStreams(camera3_stream_configuration_t *stream_list); + int processCaptureRequest(camera3_capture_request_t *request); + + bool isOpen() const { return open_; } + unsigned int id() const { return id_; } + camera3_device_t *device() { return &cameraDevice_; } + +private: + void threadRpcCall(ThreadRpc &rpcRequest); + + bool open_; + unsigned int id_; + CameraModule *cameraModule_; + camera3_device_t cameraDevice_; +}; + +#endif /* __ANDROID_CAMERA_PROXY_H__ */ diff --git a/src/android/meson.build b/src/android/meson.build index 1f242953db37..63b45832c0d2 100644 --- a/src/android/meson.build +++ b/src/android/meson.build @@ -1,3 +1,11 @@ +android_hal_sources = files([ + 'camera3_hal.cpp', + 'camera_hal_manager.cpp', + 'camera_module.cpp', + 'camera_proxy.cpp', + 'thread_rpc.cpp' +]) + android_camera_metadata_sources = files([ 'metadata/camera_metadata.c', ]) diff --git a/src/android/thread_rpc.cpp b/src/android/thread_rpc.cpp new file mode 100644 index 000000000000..f13db59d0d75 --- /dev/null +++ b/src/android/thread_rpc.cpp @@ -0,0 +1,41 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * thread_call.cpp - Inter-thread procedure call + */ + +#include "message.h" +#include "thread_rpc.h" + +using namespace libcamera; + +libcamera::Message::Type ThreadRpcMessage::rpcType_ = Message::Type::None; + +ThreadRpcMessage::ThreadRpcMessage() + : Message(type()) +{ +} + +void ThreadRpc::notifyReception() +{ + { + libcamera::MutexLocker locker(mutex_); + delivered_ = true; + } + cv_.notify_one(); +} + +void ThreadRpc::waitDelivery() +{ + libcamera::MutexLocker locker(mutex_); + cv_.wait(locker, [&]{ return delivered_; }); +} + +Message::Type ThreadRpcMessage::type() +{ + if (ThreadRpcMessage::rpcType_ == Message::Type::None) + rpcType_ = Message::registerMessageType(); + + return rpcType_; +} diff --git a/src/android/thread_rpc.h b/src/android/thread_rpc.h new file mode 100644 index 000000000000..173caba1a515 --- /dev/null +++ b/src/android/thread_rpc.h @@ -0,0 +1,56 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * thread_call.h - Inter-thread procedure call + */ +#ifndef __ANDROID_THREAD_CALL_H__ +#define __ANDROID_THREAD_CALL_H__ + +#include <condition_variable> +#include <string> + +#include <hardware/camera3.h> +#include <libcamera/libcamera.h> + +#include "message.h" +#include "thread.h" + +class ThreadRpc +{ +public: + enum RpcTag { + ProcessCaptureRequest, + Close, + }; + + ThreadRpc() + : delivered_(false) {} + + void notifyReception(); + void waitDelivery(); + + RpcTag tag; + + camera3_capture_request_t *request; + +private: + bool delivered_; + std::mutex mutex_; + std::condition_variable cv_; +}; + +class ThreadRpcMessage : public libcamera::Message +{ +public: + ThreadRpcMessage(); + ThreadRpc *rpc; + + static Message::Type type(); + +private: + static libcamera::Message::Type rpcType_; +}; + +#endif /* __ANDROID_THREAD_CALL_H__ */ + diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index a09b23d60022..7d5d3c04fba0 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -103,9 +103,18 @@ libcamera_deps = [ dependency('threads'), ] +libcamera_link_with = [] + +if get_option('android') + libcamera_sources += android_hal_sources + includes += android_includes + libcamera_link_with += android_camera_metadata +endif + libcamera = shared_library('camera', libcamera_sources, install : true, + link_with : libcamera_link_with, include_directories : includes, dependencies : libcamera_deps) diff --git a/src/meson.build b/src/meson.build index 7148baee3eda..67ad20aab86b 100644 --- a/src/meson.build +++ b/src/meson.build @@ -1,4 +1,7 @@ -subdir('android') +if get_option('android') + subdir('android') +endif + subdir('libcamera') subdir('ipa') subdir('cam')
Add libcamera Android Camera HALv3 implementation. The initial camera HAL implementation supports the LIMITED hardware level and uses statically defined metadata and camera characteristics. Add a build option named 'android' and adjust the build system to selectively compile the Android camera HAL and link it against the required Android libraries. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- meson_options.txt | 5 + src/android/camera3_hal.cpp | 130 +++++ src/android/camera_hal_manager.cpp | 173 +++++++ src/android/camera_hal_manager.h | 56 ++ src/android/camera_module.cpp | 799 +++++++++++++++++++++++++++++ src/android/camera_module.h | 75 +++ src/android/camera_proxy.cpp | 181 +++++++ src/android/camera_proxy.h | 41 ++ src/android/meson.build | 8 + src/android/thread_rpc.cpp | 41 ++ src/android/thread_rpc.h | 56 ++ src/libcamera/meson.build | 9 + src/meson.build | 5 +- 13 files changed, 1578 insertions(+), 1 deletion(-) create mode 100644 src/android/camera3_hal.cpp create mode 100644 src/android/camera_hal_manager.cpp create mode 100644 src/android/camera_hal_manager.h create mode 100644 src/android/camera_module.cpp create mode 100644 src/android/camera_module.h create mode 100644 src/android/camera_proxy.cpp create mode 100644 src/android/camera_proxy.h create mode 100644 src/android/thread_rpc.cpp create mode 100644 src/android/thread_rpc.h