| Message ID | 20210324112527.63701-5-jacopo@jmondi.org |
|---|---|
| State | Superseded |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, Thanks for your work. On 2021-03-24 12:25:24 +0100, Jacopo Mondi wrote: > Add a CameraHalConfig class to the Android Camera3 HAL layer. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> There are a few small nits below but I think this is a great step in the right direction. Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > --- > src/android/camera_hal_config.cpp | 385 ++++++++++++++++++++++++++++++ > src/android/camera_hal_config.h | 54 +++++ > src/android/meson.build | 1 + > 3 files changed, 440 insertions(+) > create mode 100644 src/android/camera_hal_config.cpp > create mode 100644 src/android/camera_hal_config.h > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp > new file mode 100644 > index 000000000000..7e33dfb750ff > --- /dev/null > +++ b/src/android/camera_hal_config.cpp > @@ -0,0 +1,385 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2021, Google Inc. > + * > + * camera_hal_config.cpp - Camera HAL configuration file manager > + */ > +#include "camera_hal_config.h" > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > + > +#include "libcamera/internal/file.h" > +#include "libcamera/internal/log.h" > + > +using namespace libcamera; > + > +LOG_DEFINE_CATEGORY(HALConfig) > + > +const std::string CameraHalConfig::CameraBlock::toString() const > +{ > + std::stringstream ss; > + > + ss << "\'" << name << "\'" > + << " model: " << model > + << "(" << location << ")[" << rotation << "]"; nit: The usage of the pattern "key: <value>" have been discouraged in the core, do we extend the same to the HAL? > + > + return ss.str(); > +} > + > +CameraHalConfig::CameraHalConfig() > +{ > + if (!yaml_parser_initialize(&parser_)) > + LOG(HALConfig, Error) << "Failed to initialize yaml parser"; Fatal? > +} > + > +CameraHalConfig::~CameraHalConfig() > +{ > + yaml_parser_delete(&parser_); > +} > + > +/* > + * Configuration files can be stored in system paths, which are identified > + * through the build configuration. > + * > + * However, when running uninstalled - the source location takes precedence. > + */ > +const std::string CameraHalConfig::findFilePath(const std::string &filename) > +{ > + static std::array<std::string, 2> searchPaths = { > + LIBCAMERA_SYSCONF_DIR, > + LIBCAMERA_DATA_DIR, > + }; > + > + if (File::exists(filename)) > + return filename; > + > + std::string root = utils::libcameraSourcePath(); > + if (!root.empty()) { > + std::string configurationPath = root + "data/" + filename; > + > + if (File::exists(configurationPath)) > + return configurationPath; > + } > + > + for (std::string &path : searchPaths) { > + std::string configurationPath = path + "/" + filename; > + if (File::exists(configurationPath)) > + return configurationPath; > + } > + > + return ""; > +} > + > +/* > + * \brief Open the HAL configuration file and validate its content > + * \return 0 on success, a negative error code otherwise nit: We don't Doxygen the HAL is it not confusing to use it mark up here then? > + */ > +int CameraHalConfig::open() > +{ > + int ret; > + > + const std::string configPath = "camera_hal.yaml"; s/configPath/configFile/ ? > + const std::string filePath = findFilePath(configPath); As this is the only call site would it make sens to have findFilePath() return a FILE * to get compartmentalizing of the error paths? > + if (filePath.empty()) { > + LOG(HALConfig, Warning) > + << "File: \"" << configPath << "\" not found"; > + return -ENOENT; > + } > + > + FILE *fh = fopen(filePath.c_str(), "r"); > + if (!fh) { > + ret = -errno; > + LOG(HALConfig, Error) << "Failed to open file " << filePath > + << ": " << strerror(-ret); > + return ret; > + } > + > + yaml_parser_set_input_file(&parser_, fh); > + > + LOG(HALConfig, Debug) << "Reading configuration from " << filePath; > + > + ret = parseConfigFile(); > + fclose(fh); > + if (ret) > + return ret; > + > + LOG(HALConfig, Info) << "Device model: " << model_; > + LOG(HALConfig, Info) << "Device maker: " << maker_; > + for (const auto &c : cameras_) { > + const CameraBlock camera = c.second; > + LOG(HALConfig, Info) << camera.toString(); > + } > + > + return 0; > +} > + > +const std::string &CameraHalConfig::cameraLocation(const std::string &camera) const > +{ > + static const std::string empty(""); > + const auto &it = cameras_.find(camera); > + if (it == cameras_.end()) { > + LOG(HALConfig, Error) > + << "Camera '" << camera > + << "' not described in the HAL configuration file"; > + return empty; > + } > + > + const CameraBlock &cam = it->second; > + return cam.location; > +} > + > +unsigned int CameraHalConfig::cameraRotation(const std::string &camera) const > +{ > + static const std::string empty(""); empty not used. > + const auto &it = cameras_.find(camera); > + if (it == cameras_.end()) { > + LOG(HALConfig, Error) > + << "Camera '" << camera > + << "' not described in the HAL configuration file"; > + return 0; > + } > + > + const CameraBlock &cam = it->second; > + return cam.rotation; > +} > + > +const std::string &CameraHalConfig::cameraModel(const std::string &camera) const > +{ > + static const std::string empty(""); > + const auto &it = cameras_.find(camera); > + if (it == cameras_.end()) { > + LOG(HALConfig, Error) > + << "Camera '" << camera > + << "' not described in the HAL configuration file"; > + return empty; > + } > + > + const CameraBlock &cam = it->second; > + return cam.model; > +} > + > +std::string CameraHalConfig::parseValue(yaml_token_t &token) > +{ > + /* Make sure the token type is a value and get its content. */ > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_VALUE_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return ""; > + } > + yaml_token_delete(&token); > + > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_SCALAR_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return ""; > + } > + > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > + std::string value(scalar, token.data.scalar.length); > + yaml_token_delete(&token); > + > + return value; > +} > + > +std::string CameraHalConfig::parseKey(yaml_token_t &token) > +{ > + /* Make sure the token type is a key and get its value. */ > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_KEY_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return ""; > + } > + yaml_token_delete(&token); > + > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_SCALAR_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return ""; > + } > + > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > + std::string key(scalar, token.data.scalar.length); > + yaml_token_delete(&token); > + > + return key; > +} > + > +int CameraHalConfig::parseCameraBlock(yaml_token_t &token) > +{ > + /* The 'camera' block is a VALUE token type. */ > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_VALUE_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return -EINVAL; > + } > + yaml_token_delete(&token); > + > + /* > + * Parse the content of the camera block until we have an unbalanced > + * BLOCK_END_TOKEN which closes the camera block. > + * > + * Add a safety counter to make sure we don't loop indefinitely in case > + * the configuration file is malformed. > + */ > + unsigned int sentinel = 100; > + CameraBlock cameraBlock{}; > + int blockCounter = 0; > + do { > + yaml_parser_scan(&parser_, &token); > + switch (token.type) { > + case YAML_BLOCK_ENTRY_TOKEN: > + yaml_token_delete(&token); > + blockCounter++; > + break; > + case YAML_BLOCK_END_TOKEN: > + yaml_token_delete(&token); > + blockCounter--; > + break; > + case YAML_BLOCK_MAPPING_START_TOKEN: { > + yaml_token_delete(&token); > + > + std::string key = parseKey(token); > + std::string value = parseValue(token); > + if (key.empty() || value.empty()) { > + LOG(HALConfig, Error) > + << "Configuration file is not valid"; > + return -EINVAL; > + } > + > + /* Validate that the parsed key is valid. */ > + if (key == "location") { > + if (value != "front" && value != "back" && > + value != "external") { > + LOG(HALConfig, Error) > + << "Unknown location: " << value; > + return -EINVAL; > + } > + cameraBlock.location = value; > + } else if (key == "rotation") { > + cameraBlock.rotation = strtoul(value.c_str(), > + NULL, 10); > + if (cameraBlock.rotation < 0) { > + LOG(HALConfig, Error) > + << "Unknown rotation: " > + << cameraBlock.rotation; > + return -EINVAL; > + } > + } else if (key == "name") { > + cameraBlock.name = value; > + } else if (key == "model") { > + cameraBlock.model = value; > + } else { > + LOG(HALConfig, Error) > + << "Configuration file is not valid " > + << "unknown key: " << key; > + return -EINVAL; > + } > + break; > + } > + default: > + yaml_token_delete(&token); > + break; > + } > + --sentinel; > + } while (blockCounter >= 0 && sentinel); > + if (!sentinel) { > + LOG(HALConfig, Error) << "Configuration file is not valid "; > + return -EINVAL; > + } > + > + cameras_[cameraBlock.name] = cameraBlock; > + > + return 0; > +} > + > +int CameraHalConfig::parseEntryBlock(yaml_token_t &token) > +{ > + int ret; > + > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_BLOCK_MAPPING_START_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return -EINVAL; > + } > + yaml_token_delete(&token); > + > + std::string key = parseKey(token); > + if (key.empty()) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return -EINVAL; > + } > + > + if (key == "camera") { > + yaml_token_delete(&token); > + ret = parseCameraBlock(token); > + if (ret) > + return ret; > + } else if (key == "manufacturer") { > + yaml_token_delete(&token); > + maker_ = parseValue(token); > + if (maker_.empty()) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return -EINVAL; > + } > + } else if (key == "model") { > + yaml_token_delete(&token); > + model_ = parseValue(token); > + if (model_.empty()) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return -EINVAL; > + } > + } else { > + yaml_token_delete(&token); > + LOG(HALConfig, Error) << "Unknown key " << key; > + return -EINVAL; > + } > + > + return 0; > +} > + > +int CameraHalConfig::parseConfigFile() > +{ > + yaml_token_t token; > + int ret; > + > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_STREAM_START_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return EINVAL; > + } > + yaml_token_delete(&token); > + > + /* > + * Start parsing the content of the configuration file which > + * starts as with sequence block. > + */ > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_BLOCK_SEQUENCE_START_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return -EINVAL; > + } > + yaml_token_delete(&token); > + > + /* Parse the single entry blocks. */ > + do { > + yaml_parser_scan(&parser_, &token); > + switch (token.type) { > + case YAML_BLOCK_ENTRY_TOKEN: > + yaml_token_delete(&token); > + ret = parseEntryBlock(token); > + break; > + case YAML_STREAM_END_TOKEN: > + /* Resources are released after the loop exit. */ > + break; > + default: > + /* Release resources before re-parsing. */ > + yaml_token_delete(&token); > + break; > + } > + } while (token.type != YAML_STREAM_END_TOKEN && ret >= 0); > + yaml_token_delete(&token); > + > + return ret; > +} > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h > new file mode 100644 > index 000000000000..69d42658e90a > --- /dev/null > +++ b/src/android/camera_hal_config.h > @@ -0,0 +1,54 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2021, Google Inc. > + * > + * camera_hal_config.h - Camera HAL configuration file manager > + */ > +#ifndef __ANDROID_CAMERA_HAL_CONFIG_H__ > +#define __ANDROID_CAMERA_HAL_CONFIG_H__ > + > +#include <map> > +#include <string> > +#include <yaml.h> > + > +class CameraHalConfig > +{ > +public: > + CameraHalConfig(); > + ~CameraHalConfig(); > + > + int open(); > + > + const std::string &deviceModel() const { return model_; } > + const std::string &deviceMaker() const { return maker_; } > + > + const std::string &cameraLocation(const std::string &camera) const; > + unsigned int cameraRotation(const std::string &camera) const; > + const std::string &cameraModel(const std::string &camera) const; > + > +private: > + yaml_parser_t parser_; > + > + std::string model_; > + std::string maker_; > + class CameraBlock > + { > + public: > + std::string name; > + std::string location; > + std::string model; > + unsigned int rotation; > + > + const std::string toString() const; > + }; > + std::map<std::string, CameraBlock> cameras_; > + > + const std::string findFilePath(const std::string &filename); > + std::string parseValue(yaml_token_t &token); > + std::string parseKey(yaml_token_t &token); > + int parseCameraBlock(yaml_token_t &token); > + int parseEntryBlock(yaml_token_t &token); > + int parseConfigFile(); > +}; > + > +#endif /* __ANDROID_CAMERA_HAL_CONFIG_H__ */ > diff --git a/src/android/meson.build b/src/android/meson.build > index 19f94a4073f1..34e5c494a492 100644 > --- a/src/android/meson.build > +++ b/src/android/meson.build > @@ -44,6 +44,7 @@ android_hal_sources = files([ > 'camera3_hal.cpp', > 'camera_hal_manager.cpp', > 'camera_device.cpp', > + 'camera_hal_config.cpp', > 'camera_metadata.cpp', > 'camera_ops.cpp', > 'camera_stream.cpp', > -- > 2.30.0 > > _______________________________________________ > libcamera-devel mailing list > libcamera-devel@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, Thank you for the patch. On Wed, Mar 24, 2021 at 12:25:24PM +0100, Jacopo Mondi wrote: > Add a CameraHalConfig class to the Android Camera3 HAL layer. > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > --- > src/android/camera_hal_config.cpp | 385 ++++++++++++++++++++++++++++++ > src/android/camera_hal_config.h | 54 +++++ > src/android/meson.build | 1 + > 3 files changed, 440 insertions(+) > create mode 100644 src/android/camera_hal_config.cpp > create mode 100644 src/android/camera_hal_config.h > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp > new file mode 100644 > index 000000000000..7e33dfb750ff > --- /dev/null > +++ b/src/android/camera_hal_config.cpp > @@ -0,0 +1,385 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2021, Google Inc. > + * > + * camera_hal_config.cpp - Camera HAL configuration file manager > + */ > +#include "camera_hal_config.h" > + > +#include <stdio.h> > +#include <stdlib.h> > +#include <string.h> > + > +#include "libcamera/internal/file.h" > +#include "libcamera/internal/log.h" > + > +using namespace libcamera; > + > +LOG_DEFINE_CATEGORY(HALConfig) > + > +const std::string CameraHalConfig::CameraBlock::toString() const > +{ > + std::stringstream ss; > + > + ss << "\'" << name << "\'" > + << " model: " << model > + << "(" << location << ")[" << rotation << "]"; > + > + return ss.str(); > +} As the amount of information will grow, I don't think we'll be able to print everything. This function is only used in a single location below, for debugging purpose, I wonder if we should keep it. > + > +CameraHalConfig::CameraHalConfig() > +{ > + if (!yaml_parser_initialize(&parser_)) > + LOG(HALConfig, Error) << "Failed to initialize yaml parser"; > +} > + > +CameraHalConfig::~CameraHalConfig() > +{ > + yaml_parser_delete(&parser_); > +} > + > +/* > + * Configuration files can be stored in system paths, which are identified > + * through the build configuration. > + * > + * However, when running uninstalled - the source location takes precedence. Can we run the camera HAL uninstalled ? :-) > + */ > +const std::string CameraHalConfig::findFilePath(const std::string &filename) > +{ > + static std::array<std::string, 2> searchPaths = { > + LIBCAMERA_SYSCONF_DIR, > + LIBCAMERA_DATA_DIR, The data dir may not be very useful here, the configuration file is really system-specific, right ? I expect we may need to get the configuration file from system-specific locations, depending on whether we run on Chrome OS or Android, but we can handle that later. > + }; > + > + if (File::exists(filename)) > + return filename; > + > + std::string root = utils::libcameraSourcePath(); > + if (!root.empty()) { > + std::string configurationPath = root + "data/" + filename; > + > + if (File::exists(configurationPath)) > + return configurationPath; > + } > + > + for (std::string &path : searchPaths) { > + std::string configurationPath = path + "/" + filename; > + if (File::exists(configurationPath)) > + return configurationPath; > + } > + > + return ""; > +} > + > +/* > + * \brief Open the HAL configuration file and validate its content > + * \return 0 on success, a negative error code otherwise > + */ > +int CameraHalConfig::open() > +{ > + int ret; > + > + const std::string configPath = "camera_hal.yaml"; > + const std::string filePath = findFilePath(configPath); > + if (filePath.empty()) { > + LOG(HALConfig, Warning) This can be an Error, as we can't continue. > + << "File: \"" << configPath << "\" not found"; Maybe s/File:/Configuration file/ ? > + return -ENOENT; > + } > + > + FILE *fh = fopen(filePath.c_str(), "r"); > + if (!fh) { > + ret = -errno; > + LOG(HALConfig, Error) << "Failed to open file " << filePath Same here, "configuration file" ? > + << ": " << strerror(-ret); > + return ret; > + } > + > + yaml_parser_set_input_file(&parser_, fh); > + > + LOG(HALConfig, Debug) << "Reading configuration from " << filePath; > + > + ret = parseConfigFile(); > + fclose(fh); > + if (ret) > + return ret; > + > + LOG(HALConfig, Info) << "Device model: " << model_; > + LOG(HALConfig, Info) << "Device maker: " << maker_; > + for (const auto &c : cameras_) { > + const CameraBlock camera = c.second; > + LOG(HALConfig, Info) << camera.toString(); > + } I wonder if dumping the parsed file shouldn't be a debugging feature instead, or even if we need it at all. > + > + return 0; > +} > + > +const std::string &CameraHalConfig::cameraLocation(const std::string &camera) const > +{ > + static const std::string empty(""); > + const auto &it = cameras_.find(camera); > + if (it == cameras_.end()) { > + LOG(HALConfig, Error) > + << "Camera '" << camera > + << "' not described in the HAL configuration file"; > + return empty; > + } > + > + const CameraBlock &cam = it->second; > + return cam.location; > +} > + > +unsigned int CameraHalConfig::cameraRotation(const std::string &camera) const > +{ > + static const std::string empty(""); > + const auto &it = cameras_.find(camera); > + if (it == cameras_.end()) { > + LOG(HALConfig, Error) > + << "Camera '" << camera > + << "' not described in the HAL configuration file"; > + return 0; > + } > + > + const CameraBlock &cam = it->second; > + return cam.rotation; > +} > + > +const std::string &CameraHalConfig::cameraModel(const std::string &camera) const > +{ > + static const std::string empty(""); > + const auto &it = cameras_.find(camera); > + if (it == cameras_.end()) { > + LOG(HALConfig, Error) > + << "Camera '" << camera > + << "' not described in the HAL configuration file"; > + return empty; > + } > + > + const CameraBlock &cam = it->second; > + return cam.model; > +} > + > +std::string CameraHalConfig::parseValue(yaml_token_t &token) > +{ > + /* Make sure the token type is a value and get its content. */ > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_VALUE_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return ""; > + } > + yaml_token_delete(&token); > + > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_SCALAR_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return ""; > + } > + > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > + std::string value(scalar, token.data.scalar.length); > + yaml_token_delete(&token); > + > + return value; > +} > + > +std::string CameraHalConfig::parseKey(yaml_token_t &token) > +{ > + /* Make sure the token type is a key and get its value. */ > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_KEY_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return ""; > + } > + yaml_token_delete(&token); > + > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_SCALAR_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return ""; > + } > + > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > + std::string key(scalar, token.data.scalar.length); > + yaml_token_delete(&token); > + > + return key; > +} > + > +int CameraHalConfig::parseCameraBlock(yaml_token_t &token) > +{ > + /* The 'camera' block is a VALUE token type. */ > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_VALUE_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return -EINVAL; > + } > + yaml_token_delete(&token); > + > + /* > + * Parse the content of the camera block until we have an unbalanced > + * BLOCK_END_TOKEN which closes the camera block. > + * > + * Add a safety counter to make sure we don't loop indefinitely in case > + * the configuration file is malformed. > + */ > + unsigned int sentinel = 100; > + CameraBlock cameraBlock{}; > + int blockCounter = 0; > + do { > + yaml_parser_scan(&parser_, &token); > + switch (token.type) { > + case YAML_BLOCK_ENTRY_TOKEN: > + yaml_token_delete(&token); > + blockCounter++; > + break; > + case YAML_BLOCK_END_TOKEN: > + yaml_token_delete(&token); > + blockCounter--; > + break; > + case YAML_BLOCK_MAPPING_START_TOKEN: { > + yaml_token_delete(&token); > + > + std::string key = parseKey(token); > + std::string value = parseValue(token); > + if (key.empty() || value.empty()) { > + LOG(HALConfig, Error) > + << "Configuration file is not valid"; > + return -EINVAL; > + } > + > + /* Validate that the parsed key is valid. */ > + if (key == "location") { > + if (value != "front" && value != "back" && > + value != "external") { > + LOG(HALConfig, Error) > + << "Unknown location: " << value; > + return -EINVAL; > + } > + cameraBlock.location = value; > + } else if (key == "rotation") { > + cameraBlock.rotation = strtoul(value.c_str(), > + NULL, 10); > + if (cameraBlock.rotation < 0) { > + LOG(HALConfig, Error) > + << "Unknown rotation: " > + << cameraBlock.rotation; > + return -EINVAL; > + } > + } else if (key == "name") { > + cameraBlock.name = value; > + } else if (key == "model") { > + cameraBlock.model = value; > + } else { > + LOG(HALConfig, Error) > + << "Configuration file is not valid " > + << "unknown key: " << key; > + return -EINVAL; > + } > + break; > + } > + default: > + yaml_token_delete(&token); > + break; > + } > + --sentinel; > + } while (blockCounter >= 0 && sentinel); > + if (!sentinel) { > + LOG(HALConfig, Error) << "Configuration file is not valid "; > + return -EINVAL; > + } > + > + cameras_[cameraBlock.name] = cameraBlock; > + > + return 0; > +} > + > +int CameraHalConfig::parseEntryBlock(yaml_token_t &token) > +{ > + int ret; > + > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_BLOCK_MAPPING_START_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return -EINVAL; > + } > + yaml_token_delete(&token); > + > + std::string key = parseKey(token); > + if (key.empty()) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return -EINVAL; > + } > + > + if (key == "camera") { > + yaml_token_delete(&token); > + ret = parseCameraBlock(token); > + if (ret) > + return ret; > + } else if (key == "manufacturer") { > + yaml_token_delete(&token); > + maker_ = parseValue(token); > + if (maker_.empty()) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return -EINVAL; > + } > + } else if (key == "model") { > + yaml_token_delete(&token); > + model_ = parseValue(token); > + if (model_.empty()) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return -EINVAL; > + } > + } else { > + yaml_token_delete(&token); > + LOG(HALConfig, Error) << "Unknown key " << key; > + return -EINVAL; > + } > + > + return 0; > +} > + > +int CameraHalConfig::parseConfigFile() > +{ > + yaml_token_t token; > + int ret; > + > + yaml_parser_scan(&parser_, &token); Just to make sure you're aware of it, there's yaml_parser_load() that produces a parsed document that can then be walked. There's nothing wrong about using event-based parsing as you do though. I've skipped review of the parser implementation itself, as it may be changed depending on how patch 7/7 evolves. > + if (token.type != YAML_STREAM_START_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return EINVAL; -EINVAL > + } > + yaml_token_delete(&token); > + > + /* > + * Start parsing the content of the configuration file which > + * starts as with sequence block. s/as // ? > + */ > + yaml_parser_scan(&parser_, &token); > + if (token.type != YAML_BLOCK_SEQUENCE_START_TOKEN) { > + LOG(HALConfig, Error) << "Configuration file is not valid"; > + return -EINVAL; > + } > + yaml_token_delete(&token); > + > + /* Parse the single entry blocks. */ > + do { > + yaml_parser_scan(&parser_, &token); > + switch (token.type) { > + case YAML_BLOCK_ENTRY_TOKEN: > + yaml_token_delete(&token); > + ret = parseEntryBlock(token); > + break; > + case YAML_STREAM_END_TOKEN: > + /* Resources are released after the loop exit. */ > + break; > + default: > + /* Release resources before re-parsing. */ > + yaml_token_delete(&token); > + break; > + } > + } while (token.type != YAML_STREAM_END_TOKEN && ret >= 0); > + yaml_token_delete(&token); > + > + return ret; > +} > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h > new file mode 100644 > index 000000000000..69d42658e90a > --- /dev/null > +++ b/src/android/camera_hal_config.h > @@ -0,0 +1,54 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2021, Google Inc. > + * > + * camera_hal_config.h - Camera HAL configuration file manager > + */ > +#ifndef __ANDROID_CAMERA_HAL_CONFIG_H__ > +#define __ANDROID_CAMERA_HAL_CONFIG_H__ > + > +#include <map> > +#include <string> > +#include <yaml.h> It would be nice to hide the YAML perser from this header. It could be done by moving the parsing to a CameraHalConfigParser class, private to the camera_hal_config.cpp file. > + > +class CameraHalConfig > +{ > +public: > + CameraHalConfig(); > + ~CameraHalConfig(); > + > + int open(); > + > + const std::string &deviceModel() const { return model_; } > + const std::string &deviceMaker() const { return maker_; } > + > + const std::string &cameraLocation(const std::string &camera) const; > + unsigned int cameraRotation(const std::string &camera) const; > + const std::string &cameraModel(const std::string &camera) const; > + > +private: > + yaml_parser_t parser_; > + > + std::string model_; > + std::string maker_; > + class CameraBlock > + { > + public: > + std::string name; > + std::string location; > + std::string model; > + unsigned int rotation; > + > + const std::string toString() const; > + }; How about making this class public (and probably renaming it), and replacing the last three public functions with const CameraBlock *cameraConfig(const std::string &name) const; ? That way each CameraDevice could store a reference to the corresponding camera configuration only. > + std::map<std::string, CameraBlock> cameras_; > + > + const std::string findFilePath(const std::string &filename); No need for const at the beginning as you return by value, but you can add a const at the end. > + std::string parseValue(yaml_token_t &token); > + std::string parseKey(yaml_token_t &token); > + int parseCameraBlock(yaml_token_t &token); > + int parseEntryBlock(yaml_token_t &token); > + int parseConfigFile(); > +}; > + > +#endif /* __ANDROID_CAMERA_HAL_CONFIG_H__ */ > diff --git a/src/android/meson.build b/src/android/meson.build > index 19f94a4073f1..34e5c494a492 100644 > --- a/src/android/meson.build > +++ b/src/android/meson.build > @@ -44,6 +44,7 @@ android_hal_sources = files([ > 'camera3_hal.cpp', > 'camera_hal_manager.cpp', > 'camera_device.cpp', > + 'camera_hal_config.cpp', > 'camera_metadata.cpp', > 'camera_ops.cpp', > 'camera_stream.cpp',
Hi Niklas, On Thu, Mar 25, 2021 at 11:33:31PM +0100, Niklas Söderlund wrote: > Hi Jacopo, > > Thanks for your work. > > On 2021-03-24 12:25:24 +0100, Jacopo Mondi wrote: > > Add a CameraHalConfig class to the Android Camera3 HAL layer. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > There are a few small nits below but I think this is a great step in the > right direction. > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > --- > > src/android/camera_hal_config.cpp | 385 ++++++++++++++++++++++++++++++ > > src/android/camera_hal_config.h | 54 +++++ > > src/android/meson.build | 1 + > > 3 files changed, 440 insertions(+) > > create mode 100644 src/android/camera_hal_config.cpp > > create mode 100644 src/android/camera_hal_config.h > > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp > > new file mode 100644 > > index 000000000000..7e33dfb750ff > > --- /dev/null > > +++ b/src/android/camera_hal_config.cpp > > @@ -0,0 +1,385 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2021, Google Inc. > > + * > > + * camera_hal_config.cpp - Camera HAL configuration file manager > > + */ > > +#include "camera_hal_config.h" > > + > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <string.h> > > + > > +#include "libcamera/internal/file.h" > > +#include "libcamera/internal/log.h" > > + > > +using namespace libcamera; > > + > > +LOG_DEFINE_CATEGORY(HALConfig) > > + > > +const std::string CameraHalConfig::CameraBlock::toString() const > > +{ > > + std::stringstream ss; > > + > > + ss << "\'" << name << "\'" > > + << " model: " << model > > + << "(" << location << ")[" << rotation << "]"; > > nit: The usage of the pattern "key: <value>" have been discouraged in > the core, do we extend the same to the HAL? > Not sure what you mean here -.- > > + > > + return ss.str(); > > +} > > + > > +CameraHalConfig::CameraHalConfig() > > +{ > > + if (!yaml_parser_initialize(&parser_)) > > + LOG(HALConfig, Error) << "Failed to initialize yaml parser"; > > Fatal? > > > +} > > + > > +CameraHalConfig::~CameraHalConfig() > > +{ > > + yaml_parser_delete(&parser_); > > +} > > + > > +/* > > + * Configuration files can be stored in system paths, which are identified > > + * through the build configuration. > > + * > > + * However, when running uninstalled - the source location takes precedence. > > + */ > > +const std::string CameraHalConfig::findFilePath(const std::string &filename) > > +{ > > + static std::array<std::string, 2> searchPaths = { > > + LIBCAMERA_SYSCONF_DIR, > > + LIBCAMERA_DATA_DIR, > > + }; > > + > > + if (File::exists(filename)) > > + return filename; > > + > > + std::string root = utils::libcameraSourcePath(); > > + if (!root.empty()) { > > + std::string configurationPath = root + "data/" + filename; > > + > > + if (File::exists(configurationPath)) > > + return configurationPath; > > + } > > + > > + for (std::string &path : searchPaths) { > > + std::string configurationPath = path + "/" + filename; > > + if (File::exists(configurationPath)) > > + return configurationPath; > > + } > > + > > + return ""; > > +} > > + > > +/* > > + * \brief Open the HAL configuration file and validate its content > > + * \return 0 on success, a negative error code otherwise > > nit: We don't Doxygen the HAL is it not confusing to use it mark up here > then? > > > + */ > > +int CameraHalConfig::open() > > +{ > > + int ret; > > + > > + const std::string configPath = "camera_hal.yaml"; > > s/configPath/configFile/ ? > > > + const std::string filePath = findFilePath(configPath); > > As this is the only call site would it make sens to have findFilePath() > return a FILE * to get compartmentalizing of the error paths? > > > + if (filePath.empty()) { > > + LOG(HALConfig, Warning) > > + << "File: \"" << configPath << "\" not found"; > > + return -ENOENT; > > + } > > + > > + FILE *fh = fopen(filePath.c_str(), "r"); > > + if (!fh) { > > + ret = -errno; > > + LOG(HALConfig, Error) << "Failed to open file " << filePath > > + << ": " << strerror(-ret); > > + return ret; > > + } > > + > > + yaml_parser_set_input_file(&parser_, fh); > > + > > + LOG(HALConfig, Debug) << "Reading configuration from " << filePath; > > + > > + ret = parseConfigFile(); > > + fclose(fh); > > + if (ret) > > + return ret; > > + > > + LOG(HALConfig, Info) << "Device model: " << model_; > > + LOG(HALConfig, Info) << "Device maker: " << maker_; > > + for (const auto &c : cameras_) { > > + const CameraBlock camera = c.second; > > + LOG(HALConfig, Info) << camera.toString(); > > + } > > + > > + return 0; > > +} > > + > > +const std::string &CameraHalConfig::cameraLocation(const std::string &camera) const > > +{ > > + static const std::string empty(""); > > + const auto &it = cameras_.find(camera); > > + if (it == cameras_.end()) { > > + LOG(HALConfig, Error) > > + << "Camera '" << camera > > + << "' not described in the HAL configuration file"; > > + return empty; > > + } > > + > > + const CameraBlock &cam = it->second; > > + return cam.location; > > +} > > + > > +unsigned int CameraHalConfig::cameraRotation(const std::string &camera) const > > +{ > > + static const std::string empty(""); > > empty not used. > Ouch, leftover > > + const auto &it = cameras_.find(camera); > > + if (it == cameras_.end()) { > > + LOG(HALConfig, Error) > > + << "Camera '" << camera > > + << "' not described in the HAL configuration file"; > > + return 0; > > + } > > + > > + const CameraBlock &cam = it->second; > > + return cam.rotation; > > +} > > + > > +const std::string &CameraHalConfig::cameraModel(const std::string &camera) const > > +{ > > + static const std::string empty(""); > > + const auto &it = cameras_.find(camera); > > + if (it == cameras_.end()) { > > + LOG(HALConfig, Error) > > + << "Camera '" << camera > > + << "' not described in the HAL configuration file"; > > + return empty; > > + } > > + > > + const CameraBlock &cam = it->second; > > + return cam.model; > > +} > > + > > +std::string CameraHalConfig::parseValue(yaml_token_t &token) > > +{ > > + /* Make sure the token type is a value and get its content. */ > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_VALUE_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return ""; > > + } > > + yaml_token_delete(&token); > > + > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_SCALAR_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return ""; > > + } > > + > > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > > + std::string value(scalar, token.data.scalar.length); > > + yaml_token_delete(&token); > > + > > + return value; > > +} > > + > > +std::string CameraHalConfig::parseKey(yaml_token_t &token) > > +{ > > + /* Make sure the token type is a key and get its value. */ > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_KEY_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return ""; > > + } > > + yaml_token_delete(&token); > > + > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_SCALAR_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return ""; > > + } > > + > > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > > + std::string key(scalar, token.data.scalar.length); > > + yaml_token_delete(&token); > > + > > + return key; > > +} > > + > > +int CameraHalConfig::parseCameraBlock(yaml_token_t &token) > > +{ > > + /* The 'camera' block is a VALUE token type. */ > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_VALUE_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + yaml_token_delete(&token); > > + > > + /* > > + * Parse the content of the camera block until we have an unbalanced > > + * BLOCK_END_TOKEN which closes the camera block. > > + * > > + * Add a safety counter to make sure we don't loop indefinitely in case > > + * the configuration file is malformed. > > + */ > > + unsigned int sentinel = 100; > > + CameraBlock cameraBlock{}; > > + int blockCounter = 0; > > + do { > > + yaml_parser_scan(&parser_, &token); > > + switch (token.type) { > > + case YAML_BLOCK_ENTRY_TOKEN: > > + yaml_token_delete(&token); > > + blockCounter++; > > + break; > > + case YAML_BLOCK_END_TOKEN: > > + yaml_token_delete(&token); > > + blockCounter--; > > + break; > > + case YAML_BLOCK_MAPPING_START_TOKEN: { > > + yaml_token_delete(&token); > > + > > + std::string key = parseKey(token); > > + std::string value = parseValue(token); > > + if (key.empty() || value.empty()) { > > + LOG(HALConfig, Error) > > + << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + > > + /* Validate that the parsed key is valid. */ > > + if (key == "location") { > > + if (value != "front" && value != "back" && > > + value != "external") { > > + LOG(HALConfig, Error) > > + << "Unknown location: " << value; > > + return -EINVAL; > > + } > > + cameraBlock.location = value; > > + } else if (key == "rotation") { > > + cameraBlock.rotation = strtoul(value.c_str(), > > + NULL, 10); > > + if (cameraBlock.rotation < 0) { > > + LOG(HALConfig, Error) > > + << "Unknown rotation: " > > + << cameraBlock.rotation; > > + return -EINVAL; > > + } > > + } else if (key == "name") { > > + cameraBlock.name = value; > > + } else if (key == "model") { > > + cameraBlock.model = value; > > + } else { > > + LOG(HALConfig, Error) > > + << "Configuration file is not valid " > > + << "unknown key: " << key; > > + return -EINVAL; > > + } > > + break; > > + } > > + default: > > + yaml_token_delete(&token); > > + break; > > + } > > + --sentinel; > > + } while (blockCounter >= 0 && sentinel); > > + if (!sentinel) { > > + LOG(HALConfig, Error) << "Configuration file is not valid "; > > + return -EINVAL; > > + } > > + > > + cameras_[cameraBlock.name] = cameraBlock; > > + > > + return 0; > > +} > > + > > +int CameraHalConfig::parseEntryBlock(yaml_token_t &token) > > +{ > > + int ret; > > + > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_BLOCK_MAPPING_START_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + yaml_token_delete(&token); > > + > > + std::string key = parseKey(token); > > + if (key.empty()) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + > > + if (key == "camera") { > > + yaml_token_delete(&token); > > + ret = parseCameraBlock(token); > > + if (ret) > > + return ret; > > + } else if (key == "manufacturer") { > > + yaml_token_delete(&token); > > + maker_ = parseValue(token); > > + if (maker_.empty()) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + } else if (key == "model") { > > + yaml_token_delete(&token); > > + model_ = parseValue(token); > > + if (model_.empty()) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + } else { > > + yaml_token_delete(&token); > > + LOG(HALConfig, Error) << "Unknown key " << key; > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > +int CameraHalConfig::parseConfigFile() > > +{ > > + yaml_token_t token; > > + int ret; > > + > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_STREAM_START_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return EINVAL; > > + } > > + yaml_token_delete(&token); > > + > > + /* > > + * Start parsing the content of the configuration file which > > + * starts as with sequence block. > > + */ > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_BLOCK_SEQUENCE_START_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + yaml_token_delete(&token); > > + > > + /* Parse the single entry blocks. */ > > + do { > > + yaml_parser_scan(&parser_, &token); > > + switch (token.type) { > > + case YAML_BLOCK_ENTRY_TOKEN: > > + yaml_token_delete(&token); > > + ret = parseEntryBlock(token); > > + break; > > + case YAML_STREAM_END_TOKEN: > > + /* Resources are released after the loop exit. */ > > + break; > > + default: > > + /* Release resources before re-parsing. */ > > + yaml_token_delete(&token); > > + break; > > + } > > + } while (token.type != YAML_STREAM_END_TOKEN && ret >= 0); > > + yaml_token_delete(&token); > > + > > + return ret; > > +} > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h > > new file mode 100644 > > index 000000000000..69d42658e90a > > --- /dev/null > > +++ b/src/android/camera_hal_config.h > > @@ -0,0 +1,54 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2021, Google Inc. > > + * > > + * camera_hal_config.h - Camera HAL configuration file manager > > + */ > > +#ifndef __ANDROID_CAMERA_HAL_CONFIG_H__ > > +#define __ANDROID_CAMERA_HAL_CONFIG_H__ > > + > > +#include <map> > > +#include <string> > > +#include <yaml.h> > > + > > +class CameraHalConfig > > +{ > > +public: > > + CameraHalConfig(); > > + ~CameraHalConfig(); > > + > > + int open(); > > + > > + const std::string &deviceModel() const { return model_; } > > + const std::string &deviceMaker() const { return maker_; } > > + > > + const std::string &cameraLocation(const std::string &camera) const; > > + unsigned int cameraRotation(const std::string &camera) const; > > + const std::string &cameraModel(const std::string &camera) const; > > + > > +private: > > + yaml_parser_t parser_; > > + > > + std::string model_; > > + std::string maker_; > > + class CameraBlock > > + { > > + public: > > + std::string name; > > + std::string location; > > + std::string model; > > + unsigned int rotation; > > + > > + const std::string toString() const; > > + }; > > + std::map<std::string, CameraBlock> cameras_; > > + > > + const std::string findFilePath(const std::string &filename); > > + std::string parseValue(yaml_token_t &token); > > + std::string parseKey(yaml_token_t &token); > > + int parseCameraBlock(yaml_token_t &token); > > + int parseEntryBlock(yaml_token_t &token); > > + int parseConfigFile(); > > +}; > > + > > +#endif /* __ANDROID_CAMERA_HAL_CONFIG_H__ */ > > diff --git a/src/android/meson.build b/src/android/meson.build > > index 19f94a4073f1..34e5c494a492 100644 > > --- a/src/android/meson.build > > +++ b/src/android/meson.build > > @@ -44,6 +44,7 @@ android_hal_sources = files([ > > 'camera3_hal.cpp', > > 'camera_hal_manager.cpp', > > 'camera_device.cpp', > > + 'camera_hal_config.cpp', > > 'camera_metadata.cpp', > > 'camera_ops.cpp', > > 'camera_stream.cpp', > > -- > > 2.30.0 > > > > _______________________________________________ > > libcamera-devel mailing list > > libcamera-devel@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel > > -- > Regards, > Niklas Söderlund
Hi Jacopo, On 2021-03-29 16:23:08 +0200, Jacopo Mondi wrote: > Hi Niklas, > > On Thu, Mar 25, 2021 at 11:33:31PM +0100, Niklas Söderlund wrote: > > Hi Jacopo, > > > > Thanks for your work. > > > > On 2021-03-24 12:25:24 +0100, Jacopo Mondi wrote: > > > Add a CameraHalConfig class to the Android Camera3 HAL layer. > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > > There are a few small nits below but I think this is a great step in the > > right direction. > > > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > > > --- > > > src/android/camera_hal_config.cpp | 385 ++++++++++++++++++++++++++++++ > > > src/android/camera_hal_config.h | 54 +++++ > > > src/android/meson.build | 1 + > > > 3 files changed, 440 insertions(+) > > > create mode 100644 src/android/camera_hal_config.cpp > > > create mode 100644 src/android/camera_hal_config.h > > > > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp > > > new file mode 100644 > > > index 000000000000..7e33dfb750ff > > > --- /dev/null > > > +++ b/src/android/camera_hal_config.cpp > > > @@ -0,0 +1,385 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2021, Google Inc. > > > + * > > > + * camera_hal_config.cpp - Camera HAL configuration file manager > > > + */ > > > +#include "camera_hal_config.h" > > > + > > > +#include <stdio.h> > > > +#include <stdlib.h> > > > +#include <string.h> > > > + > > > +#include "libcamera/internal/file.h" > > > +#include "libcamera/internal/log.h" > > > + > > > +using namespace libcamera; > > > + > > > +LOG_DEFINE_CATEGORY(HALConfig) > > > + > > > +const std::string CameraHalConfig::CameraBlock::toString() const > > > +{ > > > + std::stringstream ss; > > > + > > > + ss << "\'" << name << "\'" > > > + << " model: " << model > > > + << "(" << location << ")[" << rotation << "]"; > > > > nit: The usage of the pattern "key: <value>" have been discouraged in > > the core, do we extend the same to the HAL? > > > > Not sure what you mean here -.- Sorry, I just noticed that the pattern of << " model: " << model Have been discouraged in the core in favor of << " model " << model > > > > + > > > + return ss.str(); > > > +} > > > + > > > +CameraHalConfig::CameraHalConfig() > > > +{ > > > + if (!yaml_parser_initialize(&parser_)) > > > + LOG(HALConfig, Error) << "Failed to initialize yaml parser"; > > > > Fatal? > > > > > +} > > > + > > > +CameraHalConfig::~CameraHalConfig() > > > +{ > > > + yaml_parser_delete(&parser_); > > > +} > > > + > > > +/* > > > + * Configuration files can be stored in system paths, which are identified > > > + * through the build configuration. > > > + * > > > + * However, when running uninstalled - the source location takes precedence. > > > + */ > > > +const std::string CameraHalConfig::findFilePath(const std::string &filename) > > > +{ > > > + static std::array<std::string, 2> searchPaths = { > > > + LIBCAMERA_SYSCONF_DIR, > > > + LIBCAMERA_DATA_DIR, > > > + }; > > > + > > > + if (File::exists(filename)) > > > + return filename; > > > + > > > + std::string root = utils::libcameraSourcePath(); > > > + if (!root.empty()) { > > > + std::string configurationPath = root + "data/" + filename; > > > + > > > + if (File::exists(configurationPath)) > > > + return configurationPath; > > > + } > > > + > > > + for (std::string &path : searchPaths) { > > > + std::string configurationPath = path + "/" + filename; > > > + if (File::exists(configurationPath)) > > > + return configurationPath; > > > + } > > > + > > > + return ""; > > > +} > > > + > > > +/* > > > + * \brief Open the HAL configuration file and validate its content > > > + * \return 0 on success, a negative error code otherwise > > > > nit: We don't Doxygen the HAL is it not confusing to use it mark up here > > then? > > > > > + */ > > > +int CameraHalConfig::open() > > > +{ > > > + int ret; > > > + > > > + const std::string configPath = "camera_hal.yaml"; > > > > s/configPath/configFile/ ? > > > > > + const std::string filePath = findFilePath(configPath); > > > > As this is the only call site would it make sens to have findFilePath() > > return a FILE * to get compartmentalizing of the error paths? > > > > > + if (filePath.empty()) { > > > + LOG(HALConfig, Warning) > > > + << "File: \"" << configPath << "\" not found"; > > > + return -ENOENT; > > > + } > > > + > > > + FILE *fh = fopen(filePath.c_str(), "r"); > > > + if (!fh) { > > > + ret = -errno; > > > + LOG(HALConfig, Error) << "Failed to open file " << filePath > > > + << ": " << strerror(-ret); > > > + return ret; > > > + } > > > + > > > + yaml_parser_set_input_file(&parser_, fh); > > > + > > > + LOG(HALConfig, Debug) << "Reading configuration from " << filePath; > > > + > > > + ret = parseConfigFile(); > > > + fclose(fh); > > > + if (ret) > > > + return ret; > > > + > > > + LOG(HALConfig, Info) << "Device model: " << model_; > > > + LOG(HALConfig, Info) << "Device maker: " << maker_; > > > + for (const auto &c : cameras_) { > > > + const CameraBlock camera = c.second; > > > + LOG(HALConfig, Info) << camera.toString(); > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +const std::string &CameraHalConfig::cameraLocation(const std::string &camera) const > > > +{ > > > + static const std::string empty(""); > > > + const auto &it = cameras_.find(camera); > > > + if (it == cameras_.end()) { > > > + LOG(HALConfig, Error) > > > + << "Camera '" << camera > > > + << "' not described in the HAL configuration file"; > > > + return empty; > > > + } > > > + > > > + const CameraBlock &cam = it->second; > > > + return cam.location; > > > +} > > > + > > > +unsigned int CameraHalConfig::cameraRotation(const std::string &camera) const > > > +{ > > > + static const std::string empty(""); > > > > empty not used. > > > > Ouch, leftover > > > > + const auto &it = cameras_.find(camera); > > > + if (it == cameras_.end()) { > > > + LOG(HALConfig, Error) > > > + << "Camera '" << camera > > > + << "' not described in the HAL configuration file"; > > > + return 0; > > > + } > > > + > > > + const CameraBlock &cam = it->second; > > > + return cam.rotation; > > > +} > > > + > > > +const std::string &CameraHalConfig::cameraModel(const std::string &camera) const > > > +{ > > > + static const std::string empty(""); > > > + const auto &it = cameras_.find(camera); > > > + if (it == cameras_.end()) { > > > + LOG(HALConfig, Error) > > > + << "Camera '" << camera > > > + << "' not described in the HAL configuration file"; > > > + return empty; > > > + } > > > + > > > + const CameraBlock &cam = it->second; > > > + return cam.model; > > > +} > > > + > > > +std::string CameraHalConfig::parseValue(yaml_token_t &token) > > > +{ > > > + /* Make sure the token type is a value and get its content. */ > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_VALUE_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return ""; > > > + } > > > + yaml_token_delete(&token); > > > + > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_SCALAR_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return ""; > > > + } > > > + > > > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > > > + std::string value(scalar, token.data.scalar.length); > > > + yaml_token_delete(&token); > > > + > > > + return value; > > > +} > > > + > > > +std::string CameraHalConfig::parseKey(yaml_token_t &token) > > > +{ > > > + /* Make sure the token type is a key and get its value. */ > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_KEY_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return ""; > > > + } > > > + yaml_token_delete(&token); > > > + > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_SCALAR_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return ""; > > > + } > > > + > > > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > > > + std::string key(scalar, token.data.scalar.length); > > > + yaml_token_delete(&token); > > > + > > > + return key; > > > +} > > > + > > > +int CameraHalConfig::parseCameraBlock(yaml_token_t &token) > > > +{ > > > + /* The 'camera' block is a VALUE token type. */ > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_VALUE_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + yaml_token_delete(&token); > > > + > > > + /* > > > + * Parse the content of the camera block until we have an unbalanced > > > + * BLOCK_END_TOKEN which closes the camera block. > > > + * > > > + * Add a safety counter to make sure we don't loop indefinitely in case > > > + * the configuration file is malformed. > > > + */ > > > + unsigned int sentinel = 100; > > > + CameraBlock cameraBlock{}; > > > + int blockCounter = 0; > > > + do { > > > + yaml_parser_scan(&parser_, &token); > > > + switch (token.type) { > > > + case YAML_BLOCK_ENTRY_TOKEN: > > > + yaml_token_delete(&token); > > > + blockCounter++; > > > + break; > > > + case YAML_BLOCK_END_TOKEN: > > > + yaml_token_delete(&token); > > > + blockCounter--; > > > + break; > > > + case YAML_BLOCK_MAPPING_START_TOKEN: { > > > + yaml_token_delete(&token); > > > + > > > + std::string key = parseKey(token); > > > + std::string value = parseValue(token); > > > + if (key.empty() || value.empty()) { > > > + LOG(HALConfig, Error) > > > + << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + > > > + /* Validate that the parsed key is valid. */ > > > + if (key == "location") { > > > + if (value != "front" && value != "back" && > > > + value != "external") { > > > + LOG(HALConfig, Error) > > > + << "Unknown location: " << value; > > > + return -EINVAL; > > > + } > > > + cameraBlock.location = value; > > > + } else if (key == "rotation") { > > > + cameraBlock.rotation = strtoul(value.c_str(), > > > + NULL, 10); > > > + if (cameraBlock.rotation < 0) { > > > + LOG(HALConfig, Error) > > > + << "Unknown rotation: " > > > + << cameraBlock.rotation; > > > + return -EINVAL; > > > + } > > > + } else if (key == "name") { > > > + cameraBlock.name = value; > > > + } else if (key == "model") { > > > + cameraBlock.model = value; > > > + } else { > > > + LOG(HALConfig, Error) > > > + << "Configuration file is not valid " > > > + << "unknown key: " << key; > > > + return -EINVAL; > > > + } > > > + break; > > > + } > > > + default: > > > + yaml_token_delete(&token); > > > + break; > > > + } > > > + --sentinel; > > > + } while (blockCounter >= 0 && sentinel); > > > + if (!sentinel) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid "; > > > + return -EINVAL; > > > + } > > > + > > > + cameras_[cameraBlock.name] = cameraBlock; > > > + > > > + return 0; > > > +} > > > + > > > +int CameraHalConfig::parseEntryBlock(yaml_token_t &token) > > > +{ > > > + int ret; > > > + > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_BLOCK_MAPPING_START_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + yaml_token_delete(&token); > > > + > > > + std::string key = parseKey(token); > > > + if (key.empty()) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + > > > + if (key == "camera") { > > > + yaml_token_delete(&token); > > > + ret = parseCameraBlock(token); > > > + if (ret) > > > + return ret; > > > + } else if (key == "manufacturer") { > > > + yaml_token_delete(&token); > > > + maker_ = parseValue(token); > > > + if (maker_.empty()) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + } else if (key == "model") { > > > + yaml_token_delete(&token); > > > + model_ = parseValue(token); > > > + if (model_.empty()) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + } else { > > > + yaml_token_delete(&token); > > > + LOG(HALConfig, Error) << "Unknown key " << key; > > > + return -EINVAL; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +int CameraHalConfig::parseConfigFile() > > > +{ > > > + yaml_token_t token; > > > + int ret; > > > + > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_STREAM_START_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return EINVAL; > > > + } > > > + yaml_token_delete(&token); > > > + > > > + /* > > > + * Start parsing the content of the configuration file which > > > + * starts as with sequence block. > > > + */ > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_BLOCK_SEQUENCE_START_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + yaml_token_delete(&token); > > > + > > > + /* Parse the single entry blocks. */ > > > + do { > > > + yaml_parser_scan(&parser_, &token); > > > + switch (token.type) { > > > + case YAML_BLOCK_ENTRY_TOKEN: > > > + yaml_token_delete(&token); > > > + ret = parseEntryBlock(token); > > > + break; > > > + case YAML_STREAM_END_TOKEN: > > > + /* Resources are released after the loop exit. */ > > > + break; > > > + default: > > > + /* Release resources before re-parsing. */ > > > + yaml_token_delete(&token); > > > + break; > > > + } > > > + } while (token.type != YAML_STREAM_END_TOKEN && ret >= 0); > > > + yaml_token_delete(&token); > > > + > > > + return ret; > > > +} > > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h > > > new file mode 100644 > > > index 000000000000..69d42658e90a > > > --- /dev/null > > > +++ b/src/android/camera_hal_config.h > > > @@ -0,0 +1,54 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2021, Google Inc. > > > + * > > > + * camera_hal_config.h - Camera HAL configuration file manager > > > + */ > > > +#ifndef __ANDROID_CAMERA_HAL_CONFIG_H__ > > > +#define __ANDROID_CAMERA_HAL_CONFIG_H__ > > > + > > > +#include <map> > > > +#include <string> > > > +#include <yaml.h> > > > + > > > +class CameraHalConfig > > > +{ > > > +public: > > > + CameraHalConfig(); > > > + ~CameraHalConfig(); > > > + > > > + int open(); > > > + > > > + const std::string &deviceModel() const { return model_; } > > > + const std::string &deviceMaker() const { return maker_; } > > > + > > > + const std::string &cameraLocation(const std::string &camera) const; > > > + unsigned int cameraRotation(const std::string &camera) const; > > > + const std::string &cameraModel(const std::string &camera) const; > > > + > > > +private: > > > + yaml_parser_t parser_; > > > + > > > + std::string model_; > > > + std::string maker_; > > > + class CameraBlock > > > + { > > > + public: > > > + std::string name; > > > + std::string location; > > > + std::string model; > > > + unsigned int rotation; > > > + > > > + const std::string toString() const; > > > + }; > > > + std::map<std::string, CameraBlock> cameras_; > > > + > > > + const std::string findFilePath(const std::string &filename); > > > + std::string parseValue(yaml_token_t &token); > > > + std::string parseKey(yaml_token_t &token); > > > + int parseCameraBlock(yaml_token_t &token); > > > + int parseEntryBlock(yaml_token_t &token); > > > + int parseConfigFile(); > > > +}; > > > + > > > +#endif /* __ANDROID_CAMERA_HAL_CONFIG_H__ */ > > > diff --git a/src/android/meson.build b/src/android/meson.build > > > index 19f94a4073f1..34e5c494a492 100644 > > > --- a/src/android/meson.build > > > +++ b/src/android/meson.build > > > @@ -44,6 +44,7 @@ android_hal_sources = files([ > > > 'camera3_hal.cpp', > > > 'camera_hal_manager.cpp', > > > 'camera_device.cpp', > > > + 'camera_hal_config.cpp', > > > 'camera_metadata.cpp', > > > 'camera_ops.cpp', > > > 'camera_stream.cpp', > > > -- > > > 2.30.0 > > > > > > _______________________________________________ > > > libcamera-devel mailing list > > > libcamera-devel@lists.libcamera.org > > > https://lists.libcamera.org/listinfo/libcamera-devel > > > > -- > > Regards, > > Niklas Söderlund
Hi Laurent, On Fri, Mar 26, 2021 at 06:07:07AM +0200, Laurent Pinchart wrote: > Hi Jacopo, > > Thank you for the patch. > > On Wed, Mar 24, 2021 at 12:25:24PM +0100, Jacopo Mondi wrote: > > Add a CameraHalConfig class to the Android Camera3 HAL layer. > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > --- > > src/android/camera_hal_config.cpp | 385 ++++++++++++++++++++++++++++++ > > src/android/camera_hal_config.h | 54 +++++ > > src/android/meson.build | 1 + > > 3 files changed, 440 insertions(+) > > create mode 100644 src/android/camera_hal_config.cpp > > create mode 100644 src/android/camera_hal_config.h > > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp > > new file mode 100644 > > index 000000000000..7e33dfb750ff > > --- /dev/null > > +++ b/src/android/camera_hal_config.cpp > > @@ -0,0 +1,385 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2021, Google Inc. > > + * > > + * camera_hal_config.cpp - Camera HAL configuration file manager > > + */ > > +#include "camera_hal_config.h" > > + > > +#include <stdio.h> > > +#include <stdlib.h> > > +#include <string.h> > > + > > +#include "libcamera/internal/file.h" > > +#include "libcamera/internal/log.h" > > + > > +using namespace libcamera; > > + > > +LOG_DEFINE_CATEGORY(HALConfig) > > + > > +const std::string CameraHalConfig::CameraBlock::toString() const > > +{ > > + std::stringstream ss; > > + > > + ss << "\'" << name << "\'" > > + << " model: " << model > > + << "(" << location << ")[" << rotation << "]"; > > + > > + return ss.str(); > > +} > > As the amount of information will grow, I don't think we'll be able to > print everything. This function is only used in a single location below, > for debugging purpose, I wonder if we should keep it. > > > + > > +CameraHalConfig::CameraHalConfig() > > +{ > > + if (!yaml_parser_initialize(&parser_)) > > + LOG(HALConfig, Error) << "Failed to initialize yaml parser"; > > +} > > + > > +CameraHalConfig::~CameraHalConfig() > > +{ > > + yaml_parser_delete(&parser_); > > +} > > + > > +/* > > + * Configuration files can be stored in system paths, which are identified > > + * through the build configuration. > > + * > > + * However, when running uninstalled - the source location takes precedence. > > Can we run the camera HAL uninstalled ? :-) > This meant "libcamera" uninstalled. > > + */ > > +const std::string CameraHalConfig::findFilePath(const std::string &filename) > > +{ > > + static std::array<std::string, 2> searchPaths = { > > + LIBCAMERA_SYSCONF_DIR, > > + LIBCAMERA_DATA_DIR, > > The data dir may not be very useful here, the configuration file is > really system-specific, right ? > > I expect we may need to get the configuration file from system-specific > locations, depending on whether we run on Chrome OS or Android, but we > can handle that later. > > > + }; > > + > > + if (File::exists(filename)) > > + return filename; > > + > > + std::string root = utils::libcameraSourcePath(); > > + if (!root.empty()) { > > + std::string configurationPath = root + "data/" + filename; > > + > > + if (File::exists(configurationPath)) > > + return configurationPath; > > + } > > + > > + for (std::string &path : searchPaths) { > > + std::string configurationPath = path + "/" + filename; > > + if (File::exists(configurationPath)) > > + return configurationPath; > > + } > > + > > + return ""; > > +} > > + > > +/* > > + * \brief Open the HAL configuration file and validate its content > > + * \return 0 on success, a negative error code otherwise > > + */ > > +int CameraHalConfig::open() > > +{ > > + int ret; > > + > > + const std::string configPath = "camera_hal.yaml"; > > + const std::string filePath = findFilePath(configPath); > > + if (filePath.empty()) { > > + LOG(HALConfig, Warning) > > This can be an Error, as we can't continue. > > > + << "File: \"" << configPath << "\" not found"; > > Maybe s/File:/Configuration file/ ? > > > + return -ENOENT; > > + } > > + > > + FILE *fh = fopen(filePath.c_str(), "r"); > > + if (!fh) { > > + ret = -errno; > > + LOG(HALConfig, Error) << "Failed to open file " << filePath > > Same here, "configuration file" ? > > > + << ": " << strerror(-ret); > > + return ret; > > + } > > + > > + yaml_parser_set_input_file(&parser_, fh); > > + > > + LOG(HALConfig, Debug) << "Reading configuration from " << filePath; > > + > > + ret = parseConfigFile(); > > + fclose(fh); > > + if (ret) > > + return ret; > > + > > + LOG(HALConfig, Info) << "Device model: " << model_; > > + LOG(HALConfig, Info) << "Device maker: " << maker_; > > + for (const auto &c : cameras_) { > > + const CameraBlock camera = c.second; > > + LOG(HALConfig, Info) << camera.toString(); > > + } > > I wonder if dumping the parsed file shouldn't be a debugging feature > instead, or even if we need it at all. > > > + > > + return 0; > > +} > > + > > +const std::string &CameraHalConfig::cameraLocation(const std::string &camera) const > > +{ > > + static const std::string empty(""); > > + const auto &it = cameras_.find(camera); > > + if (it == cameras_.end()) { > > + LOG(HALConfig, Error) > > + << "Camera '" << camera > > + << "' not described in the HAL configuration file"; > > + return empty; > > + } > > + > > + const CameraBlock &cam = it->second; > > + return cam.location; > > +} > > + > > +unsigned int CameraHalConfig::cameraRotation(const std::string &camera) const > > +{ > > + static const std::string empty(""); > > + const auto &it = cameras_.find(camera); > > + if (it == cameras_.end()) { > > + LOG(HALConfig, Error) > > + << "Camera '" << camera > > + << "' not described in the HAL configuration file"; > > + return 0; > > + } > > + > > + const CameraBlock &cam = it->second; > > + return cam.rotation; > > +} > > + > > +const std::string &CameraHalConfig::cameraModel(const std::string &camera) const > > +{ > > + static const std::string empty(""); > > + const auto &it = cameras_.find(camera); > > + if (it == cameras_.end()) { > > + LOG(HALConfig, Error) > > + << "Camera '" << camera > > + << "' not described in the HAL configuration file"; > > + return empty; > > + } > > + > > + const CameraBlock &cam = it->second; > > + return cam.model; > > +} > > + > > +std::string CameraHalConfig::parseValue(yaml_token_t &token) > > +{ > > + /* Make sure the token type is a value and get its content. */ > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_VALUE_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return ""; > > + } > > + yaml_token_delete(&token); > > + > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_SCALAR_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return ""; > > + } > > + > > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > > + std::string value(scalar, token.data.scalar.length); > > + yaml_token_delete(&token); > > + > > + return value; > > +} > > + > > +std::string CameraHalConfig::parseKey(yaml_token_t &token) > > +{ > > + /* Make sure the token type is a key and get its value. */ > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_KEY_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return ""; > > + } > > + yaml_token_delete(&token); > > + > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_SCALAR_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return ""; > > + } > > + > > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > > + std::string key(scalar, token.data.scalar.length); > > + yaml_token_delete(&token); > > + > > + return key; > > +} > > + > > +int CameraHalConfig::parseCameraBlock(yaml_token_t &token) > > +{ > > + /* The 'camera' block is a VALUE token type. */ > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_VALUE_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + yaml_token_delete(&token); > > + > > + /* > > + * Parse the content of the camera block until we have an unbalanced > > + * BLOCK_END_TOKEN which closes the camera block. > > + * > > + * Add a safety counter to make sure we don't loop indefinitely in case > > + * the configuration file is malformed. > > + */ > > + unsigned int sentinel = 100; > > + CameraBlock cameraBlock{}; > > + int blockCounter = 0; > > + do { > > + yaml_parser_scan(&parser_, &token); > > + switch (token.type) { > > + case YAML_BLOCK_ENTRY_TOKEN: > > + yaml_token_delete(&token); > > + blockCounter++; > > + break; > > + case YAML_BLOCK_END_TOKEN: > > + yaml_token_delete(&token); > > + blockCounter--; > > + break; > > + case YAML_BLOCK_MAPPING_START_TOKEN: { > > + yaml_token_delete(&token); > > + > > + std::string key = parseKey(token); > > + std::string value = parseValue(token); > > + if (key.empty() || value.empty()) { > > + LOG(HALConfig, Error) > > + << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + > > + /* Validate that the parsed key is valid. */ > > + if (key == "location") { > > + if (value != "front" && value != "back" && > > + value != "external") { > > + LOG(HALConfig, Error) > > + << "Unknown location: " << value; > > + return -EINVAL; > > + } > > + cameraBlock.location = value; > > + } else if (key == "rotation") { > > + cameraBlock.rotation = strtoul(value.c_str(), > > + NULL, 10); > > + if (cameraBlock.rotation < 0) { > > + LOG(HALConfig, Error) > > + << "Unknown rotation: " > > + << cameraBlock.rotation; > > + return -EINVAL; > > + } > > + } else if (key == "name") { > > + cameraBlock.name = value; > > + } else if (key == "model") { > > + cameraBlock.model = value; > > + } else { > > + LOG(HALConfig, Error) > > + << "Configuration file is not valid " > > + << "unknown key: " << key; > > + return -EINVAL; > > + } > > + break; > > + } > > + default: > > + yaml_token_delete(&token); > > + break; > > + } > > + --sentinel; > > + } while (blockCounter >= 0 && sentinel); > > + if (!sentinel) { > > + LOG(HALConfig, Error) << "Configuration file is not valid "; > > + return -EINVAL; > > + } > > + > > + cameras_[cameraBlock.name] = cameraBlock; > > + > > + return 0; > > +} > > + > > +int CameraHalConfig::parseEntryBlock(yaml_token_t &token) > > +{ > > + int ret; > > + > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_BLOCK_MAPPING_START_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + yaml_token_delete(&token); > > + > > + std::string key = parseKey(token); > > + if (key.empty()) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + > > + if (key == "camera") { > > + yaml_token_delete(&token); > > + ret = parseCameraBlock(token); > > + if (ret) > > + return ret; > > + } else if (key == "manufacturer") { > > + yaml_token_delete(&token); > > + maker_ = parseValue(token); > > + if (maker_.empty()) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + } else if (key == "model") { > > + yaml_token_delete(&token); > > + model_ = parseValue(token); > > + if (model_.empty()) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + } else { > > + yaml_token_delete(&token); > > + LOG(HALConfig, Error) << "Unknown key " << key; > > + return -EINVAL; > > + } > > + > > + return 0; > > +} > > + > > +int CameraHalConfig::parseConfigFile() > > +{ > > + yaml_token_t token; > > + int ret; > > + > > + yaml_parser_scan(&parser_, &token); > > Just to make sure you're aware of it, there's yaml_parser_load() that > produces a parsed document that can then be walked. There's nothing > wrong about using event-based parsing as you do though. I've found that feature under-documented with very few usage examples around. > > I've skipped review of the parser implementation itself, as it may be > changed depending on how patch 7/7 evolves. > > > + if (token.type != YAML_STREAM_START_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return EINVAL; > > -EINVAL > > > + } > > + yaml_token_delete(&token); > > + > > + /* > > + * Start parsing the content of the configuration file which > > + * starts as with sequence block. > > s/as // ? > > > + */ > > + yaml_parser_scan(&parser_, &token); > > + if (token.type != YAML_BLOCK_SEQUENCE_START_TOKEN) { > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > + return -EINVAL; > > + } > > + yaml_token_delete(&token); > > + > > + /* Parse the single entry blocks. */ > > + do { > > + yaml_parser_scan(&parser_, &token); > > + switch (token.type) { > > + case YAML_BLOCK_ENTRY_TOKEN: > > + yaml_token_delete(&token); > > + ret = parseEntryBlock(token); > > + break; > > + case YAML_STREAM_END_TOKEN: > > + /* Resources are released after the loop exit. */ > > + break; > > + default: > > + /* Release resources before re-parsing. */ > > + yaml_token_delete(&token); > > + break; > > + } > > + } while (token.type != YAML_STREAM_END_TOKEN && ret >= 0); > > + yaml_token_delete(&token); > > + > > + return ret; > > +} > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h > > new file mode 100644 > > index 000000000000..69d42658e90a > > --- /dev/null > > +++ b/src/android/camera_hal_config.h > > @@ -0,0 +1,54 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2021, Google Inc. > > + * > > + * camera_hal_config.h - Camera HAL configuration file manager > > + */ > > +#ifndef __ANDROID_CAMERA_HAL_CONFIG_H__ > > +#define __ANDROID_CAMERA_HAL_CONFIG_H__ > > + > > +#include <map> > > +#include <string> > > +#include <yaml.h> > > It would be nice to hide the YAML perser from this header. It could be > done by moving the parsing to a CameraHalConfigParser class, private to > the camera_hal_config.cpp file. > .. ok > > + > > +class CameraHalConfig > > +{ > > +public: > > + CameraHalConfig(); > > + ~CameraHalConfig(); > > + > > + int open(); > > + > > + const std::string &deviceModel() const { return model_; } > > + const std::string &deviceMaker() const { return maker_; } > > + > > + const std::string &cameraLocation(const std::string &camera) const; > > + unsigned int cameraRotation(const std::string &camera) const; > > + const std::string &cameraModel(const std::string &camera) const; > > + > > +private: > > + yaml_parser_t parser_; > > + > > + std::string model_; > > + std::string maker_; > > + class CameraBlock > > + { > > + public: > > + std::string name; > > + std::string location; > > + std::string model; > > + unsigned int rotation; > > + > > + const std::string toString() const; > > + }; > > How about making this class public (and probably renaming it), and replacing the last three public > functions with > > const CameraBlock *cameraConfig(const std::string &name) const; > > ? That way each CameraDevice could store a reference to the > corresponding camera configuration only. > I prefer the CameraHalConfig::cameraProperty() interface but ok > > + std::map<std::string, CameraBlock> cameras_; > > + > > + const std::string findFilePath(const std::string &filename); > > No need for const at the beginning as you return by value, but you can > add a const at the end. > > > + std::string parseValue(yaml_token_t &token); > > + std::string parseKey(yaml_token_t &token); > > + int parseCameraBlock(yaml_token_t &token); > > + int parseEntryBlock(yaml_token_t &token); > > + int parseConfigFile(); > > +}; > > + > > +#endif /* __ANDROID_CAMERA_HAL_CONFIG_H__ */ > > diff --git a/src/android/meson.build b/src/android/meson.build > > index 19f94a4073f1..34e5c494a492 100644 > > --- a/src/android/meson.build > > +++ b/src/android/meson.build > > @@ -44,6 +44,7 @@ android_hal_sources = files([ > > 'camera3_hal.cpp', > > 'camera_hal_manager.cpp', > > 'camera_device.cpp', > > + 'camera_hal_config.cpp', > > 'camera_metadata.cpp', > > 'camera_ops.cpp', > > 'camera_stream.cpp', > > -- > Regards, > > Laurent Pinchart
On Mon, Mar 29, 2021 at 04:27:01PM +0200, Niklas Söderlund wrote: > Hi Jacopo, > > On 2021-03-29 16:23:08 +0200, Jacopo Mondi wrote: > > Hi Niklas, > > > > On Thu, Mar 25, 2021 at 11:33:31PM +0100, Niklas Söderlund wrote: > > > Hi Jacopo, > > > > > > Thanks for your work. > > > > > > On 2021-03-24 12:25:24 +0100, Jacopo Mondi wrote: > > > > Add a CameraHalConfig class to the Android Camera3 HAL layer. > > > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > > > > There are a few small nits below but I think this is a great step in the > > > right direction. > > > > > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > > > > > --- > > > > src/android/camera_hal_config.cpp | 385 ++++++++++++++++++++++++++++++ > > > > src/android/camera_hal_config.h | 54 +++++ > > > > src/android/meson.build | 1 + > > > > 3 files changed, 440 insertions(+) > > > > create mode 100644 src/android/camera_hal_config.cpp > > > > create mode 100644 src/android/camera_hal_config.h > > > > > > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp > > > > new file mode 100644 > > > > index 000000000000..7e33dfb750ff > > > > --- /dev/null > > > > +++ b/src/android/camera_hal_config.cpp > > > > @@ -0,0 +1,385 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * Copyright (C) 2021, Google Inc. > > > > + * > > > > + * camera_hal_config.cpp - Camera HAL configuration file manager > > > > + */ > > > > +#include "camera_hal_config.h" > > > > + > > > > +#include <stdio.h> > > > > +#include <stdlib.h> > > > > +#include <string.h> > > > > + > > > > +#include "libcamera/internal/file.h" > > > > +#include "libcamera/internal/log.h" > > > > + > > > > +using namespace libcamera; > > > > + > > > > +LOG_DEFINE_CATEGORY(HALConfig) > > > > + > > > > +const std::string CameraHalConfig::CameraBlock::toString() const > > > > +{ > > > > + std::stringstream ss; > > > > + > > > > + ss << "\'" << name << "\'" > > > > + << " model: " << model > > > > + << "(" << location << ")[" << rotation << "]"; > > > > > > nit: The usage of the pattern "key: <value>" have been discouraged in > > > the core, do we extend the same to the HAL? > > > > > > > Not sure what you mean here -.- > > Sorry, I just noticed that the pattern of > > << " model: " << model > > Have been discouraged in the core in favor of > > << " model " << model > Ah ok, we're talking about the ":" in an Info debug message. Sorry I missed such change in the core. I'll fix > > > > > > + > > > > + return ss.str(); > > > > +} > > > > + > > > > +CameraHalConfig::CameraHalConfig() > > > > +{ > > > > + if (!yaml_parser_initialize(&parser_)) > > > > + LOG(HALConfig, Error) << "Failed to initialize yaml parser"; > > > > > > Fatal? > > > > > > > +} > > > > + > > > > +CameraHalConfig::~CameraHalConfig() > > > > +{ > > > > + yaml_parser_delete(&parser_); > > > > +} > > > > + > > > > +/* > > > > + * Configuration files can be stored in system paths, which are identified > > > > + * through the build configuration. > > > > + * > > > > + * However, when running uninstalled - the source location takes precedence. > > > > + */ > > > > +const std::string CameraHalConfig::findFilePath(const std::string &filename) > > > > +{ > > > > + static std::array<std::string, 2> searchPaths = { > > > > + LIBCAMERA_SYSCONF_DIR, > > > > + LIBCAMERA_DATA_DIR, > > > > + }; > > > > + > > > > + if (File::exists(filename)) > > > > + return filename; > > > > + > > > > + std::string root = utils::libcameraSourcePath(); > > > > + if (!root.empty()) { > > > > + std::string configurationPath = root + "data/" + filename; > > > > + > > > > + if (File::exists(configurationPath)) > > > > + return configurationPath; > > > > + } > > > > + > > > > + for (std::string &path : searchPaths) { > > > > + std::string configurationPath = path + "/" + filename; > > > > + if (File::exists(configurationPath)) > > > > + return configurationPath; > > > > + } > > > > + > > > > + return ""; > > > > +} > > > > + > > > > +/* > > > > + * \brief Open the HAL configuration file and validate its content > > > > + * \return 0 on success, a negative error code otherwise > > > > > > nit: We don't Doxygen the HAL is it not confusing to use it mark up here > > > then? > > > > > > > + */ > > > > +int CameraHalConfig::open() > > > > +{ > > > > + int ret; > > > > + > > > > + const std::string configPath = "camera_hal.yaml"; > > > > > > s/configPath/configFile/ ? > > > > > > > + const std::string filePath = findFilePath(configPath); > > > > > > As this is the only call site would it make sens to have findFilePath() > > > return a FILE * to get compartmentalizing of the error paths? > > > > > > > + if (filePath.empty()) { > > > > + LOG(HALConfig, Warning) > > > > + << "File: \"" << configPath << "\" not found"; > > > > + return -ENOENT; > > > > + } > > > > + > > > > + FILE *fh = fopen(filePath.c_str(), "r"); > > > > + if (!fh) { > > > > + ret = -errno; > > > > + LOG(HALConfig, Error) << "Failed to open file " << filePath > > > > + << ": " << strerror(-ret); > > > > + return ret; > > > > + } > > > > + > > > > + yaml_parser_set_input_file(&parser_, fh); > > > > + > > > > + LOG(HALConfig, Debug) << "Reading configuration from " << filePath; > > > > + > > > > + ret = parseConfigFile(); > > > > + fclose(fh); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + LOG(HALConfig, Info) << "Device model: " << model_; > > > > + LOG(HALConfig, Info) << "Device maker: " << maker_; > > > > + for (const auto &c : cameras_) { > > > > + const CameraBlock camera = c.second; > > > > + LOG(HALConfig, Info) << camera.toString(); > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +const std::string &CameraHalConfig::cameraLocation(const std::string &camera) const > > > > +{ > > > > + static const std::string empty(""); > > > > + const auto &it = cameras_.find(camera); > > > > + if (it == cameras_.end()) { > > > > + LOG(HALConfig, Error) > > > > + << "Camera '" << camera > > > > + << "' not described in the HAL configuration file"; > > > > + return empty; > > > > + } > > > > + > > > > + const CameraBlock &cam = it->second; > > > > + return cam.location; > > > > +} > > > > + > > > > +unsigned int CameraHalConfig::cameraRotation(const std::string &camera) const > > > > +{ > > > > + static const std::string empty(""); > > > > > > empty not used. > > > > > > > Ouch, leftover > > > > > > + const auto &it = cameras_.find(camera); > > > > + if (it == cameras_.end()) { > > > > + LOG(HALConfig, Error) > > > > + << "Camera '" << camera > > > > + << "' not described in the HAL configuration file"; > > > > + return 0; > > > > + } > > > > + > > > > + const CameraBlock &cam = it->second; > > > > + return cam.rotation; > > > > +} > > > > + > > > > +const std::string &CameraHalConfig::cameraModel(const std::string &camera) const > > > > +{ > > > > + static const std::string empty(""); > > > > + const auto &it = cameras_.find(camera); > > > > + if (it == cameras_.end()) { > > > > + LOG(HALConfig, Error) > > > > + << "Camera '" << camera > > > > + << "' not described in the HAL configuration file"; > > > > + return empty; > > > > + } > > > > + > > > > + const CameraBlock &cam = it->second; > > > > + return cam.model; > > > > +} > > > > + > > > > +std::string CameraHalConfig::parseValue(yaml_token_t &token) > > > > +{ > > > > + /* Make sure the token type is a value and get its content. */ > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_VALUE_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return ""; > > > > + } > > > > + yaml_token_delete(&token); > > > > + > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_SCALAR_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return ""; > > > > + } > > > > + > > > > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > > > > + std::string value(scalar, token.data.scalar.length); > > > > + yaml_token_delete(&token); > > > > + > > > > + return value; > > > > +} > > > > + > > > > +std::string CameraHalConfig::parseKey(yaml_token_t &token) > > > > +{ > > > > + /* Make sure the token type is a key and get its value. */ > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_KEY_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return ""; > > > > + } > > > > + yaml_token_delete(&token); > > > > + > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_SCALAR_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return ""; > > > > + } > > > > + > > > > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > > > > + std::string key(scalar, token.data.scalar.length); > > > > + yaml_token_delete(&token); > > > > + > > > > + return key; > > > > +} > > > > + > > > > +int CameraHalConfig::parseCameraBlock(yaml_token_t &token) > > > > +{ > > > > + /* The 'camera' block is a VALUE token type. */ > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_VALUE_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + yaml_token_delete(&token); > > > > + > > > > + /* > > > > + * Parse the content of the camera block until we have an unbalanced > > > > + * BLOCK_END_TOKEN which closes the camera block. > > > > + * > > > > + * Add a safety counter to make sure we don't loop indefinitely in case > > > > + * the configuration file is malformed. > > > > + */ > > > > + unsigned int sentinel = 100; > > > > + CameraBlock cameraBlock{}; > > > > + int blockCounter = 0; > > > > + do { > > > > + yaml_parser_scan(&parser_, &token); > > > > + switch (token.type) { > > > > + case YAML_BLOCK_ENTRY_TOKEN: > > > > + yaml_token_delete(&token); > > > > + blockCounter++; > > > > + break; > > > > + case YAML_BLOCK_END_TOKEN: > > > > + yaml_token_delete(&token); > > > > + blockCounter--; > > > > + break; > > > > + case YAML_BLOCK_MAPPING_START_TOKEN: { > > > > + yaml_token_delete(&token); > > > > + > > > > + std::string key = parseKey(token); > > > > + std::string value = parseValue(token); > > > > + if (key.empty() || value.empty()) { > > > > + LOG(HALConfig, Error) > > > > + << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + > > > > + /* Validate that the parsed key is valid. */ > > > > + if (key == "location") { > > > > + if (value != "front" && value != "back" && > > > > + value != "external") { > > > > + LOG(HALConfig, Error) > > > > + << "Unknown location: " << value; > > > > + return -EINVAL; > > > > + } > > > > + cameraBlock.location = value; > > > > + } else if (key == "rotation") { > > > > + cameraBlock.rotation = strtoul(value.c_str(), > > > > + NULL, 10); > > > > + if (cameraBlock.rotation < 0) { > > > > + LOG(HALConfig, Error) > > > > + << "Unknown rotation: " > > > > + << cameraBlock.rotation; > > > > + return -EINVAL; > > > > + } > > > > + } else if (key == "name") { > > > > + cameraBlock.name = value; > > > > + } else if (key == "model") { > > > > + cameraBlock.model = value; > > > > + } else { > > > > + LOG(HALConfig, Error) > > > > + << "Configuration file is not valid " > > > > + << "unknown key: " << key; > > > > + return -EINVAL; > > > > + } > > > > + break; > > > > + } > > > > + default: > > > > + yaml_token_delete(&token); > > > > + break; > > > > + } > > > > + --sentinel; > > > > + } while (blockCounter >= 0 && sentinel); > > > > + if (!sentinel) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid "; > > > > + return -EINVAL; > > > > + } > > > > + > > > > + cameras_[cameraBlock.name] = cameraBlock; > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +int CameraHalConfig::parseEntryBlock(yaml_token_t &token) > > > > +{ > > > > + int ret; > > > > + > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_BLOCK_MAPPING_START_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + yaml_token_delete(&token); > > > > + > > > > + std::string key = parseKey(token); > > > > + if (key.empty()) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + > > > > + if (key == "camera") { > > > > + yaml_token_delete(&token); > > > > + ret = parseCameraBlock(token); > > > > + if (ret) > > > > + return ret; > > > > + } else if (key == "manufacturer") { > > > > + yaml_token_delete(&token); > > > > + maker_ = parseValue(token); > > > > + if (maker_.empty()) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + } else if (key == "model") { > > > > + yaml_token_delete(&token); > > > > + model_ = parseValue(token); > > > > + if (model_.empty()) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + } else { > > > > + yaml_token_delete(&token); > > > > + LOG(HALConfig, Error) << "Unknown key " << key; > > > > + return -EINVAL; > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +int CameraHalConfig::parseConfigFile() > > > > +{ > > > > + yaml_token_t token; > > > > + int ret; > > > > + > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_STREAM_START_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return EINVAL; > > > > + } > > > > + yaml_token_delete(&token); > > > > + > > > > + /* > > > > + * Start parsing the content of the configuration file which > > > > + * starts as with sequence block. > > > > + */ > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_BLOCK_SEQUENCE_START_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + yaml_token_delete(&token); > > > > + > > > > + /* Parse the single entry blocks. */ > > > > + do { > > > > + yaml_parser_scan(&parser_, &token); > > > > + switch (token.type) { > > > > + case YAML_BLOCK_ENTRY_TOKEN: > > > > + yaml_token_delete(&token); > > > > + ret = parseEntryBlock(token); > > > > + break; > > > > + case YAML_STREAM_END_TOKEN: > > > > + /* Resources are released after the loop exit. */ > > > > + break; > > > > + default: > > > > + /* Release resources before re-parsing. */ > > > > + yaml_token_delete(&token); > > > > + break; > > > > + } > > > > + } while (token.type != YAML_STREAM_END_TOKEN && ret >= 0); > > > > + yaml_token_delete(&token); > > > > + > > > > + return ret; > > > > +} > > > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h > > > > new file mode 100644 > > > > index 000000000000..69d42658e90a > > > > --- /dev/null > > > > +++ b/src/android/camera_hal_config.h > > > > @@ -0,0 +1,54 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * Copyright (C) 2021, Google Inc. > > > > + * > > > > + * camera_hal_config.h - Camera HAL configuration file manager > > > > + */ > > > > +#ifndef __ANDROID_CAMERA_HAL_CONFIG_H__ > > > > +#define __ANDROID_CAMERA_HAL_CONFIG_H__ > > > > + > > > > +#include <map> > > > > +#include <string> > > > > +#include <yaml.h> > > > > + > > > > +class CameraHalConfig > > > > +{ > > > > +public: > > > > + CameraHalConfig(); > > > > + ~CameraHalConfig(); > > > > + > > > > + int open(); > > > > + > > > > + const std::string &deviceModel() const { return model_; } > > > > + const std::string &deviceMaker() const { return maker_; } > > > > + > > > > + const std::string &cameraLocation(const std::string &camera) const; > > > > + unsigned int cameraRotation(const std::string &camera) const; > > > > + const std::string &cameraModel(const std::string &camera) const; > > > > + > > > > +private: > > > > + yaml_parser_t parser_; > > > > + > > > > + std::string model_; > > > > + std::string maker_; > > > > + class CameraBlock > > > > + { > > > > + public: > > > > + std::string name; > > > > + std::string location; > > > > + std::string model; > > > > + unsigned int rotation; > > > > + > > > > + const std::string toString() const; > > > > + }; > > > > + std::map<std::string, CameraBlock> cameras_; > > > > + > > > > + const std::string findFilePath(const std::string &filename); > > > > + std::string parseValue(yaml_token_t &token); > > > > + std::string parseKey(yaml_token_t &token); > > > > + int parseCameraBlock(yaml_token_t &token); > > > > + int parseEntryBlock(yaml_token_t &token); > > > > + int parseConfigFile(); > > > > +}; > > > > + > > > > +#endif /* __ANDROID_CAMERA_HAL_CONFIG_H__ */ > > > > diff --git a/src/android/meson.build b/src/android/meson.build > > > > index 19f94a4073f1..34e5c494a492 100644 > > > > --- a/src/android/meson.build > > > > +++ b/src/android/meson.build > > > > @@ -44,6 +44,7 @@ android_hal_sources = files([ > > > > 'camera3_hal.cpp', > > > > 'camera_hal_manager.cpp', > > > > 'camera_device.cpp', > > > > + 'camera_hal_config.cpp', > > > > 'camera_metadata.cpp', > > > > 'camera_ops.cpp', > > > > 'camera_stream.cpp', > > > > -- > > > > 2.30.0 > > > > > > > > _______________________________________________ > > > > libcamera-devel mailing list > > > > libcamera-devel@lists.libcamera.org > > > > https://lists.libcamera.org/listinfo/libcamera-devel > > > > > > -- > > > Regards, > > > Niklas Söderlund > > -- > Regards, > Niklas Söderlund
Hi Jacopo, On Mon, Mar 29, 2021 at 04:27:22PM +0200, Jacopo Mondi wrote: > On Fri, Mar 26, 2021 at 06:07:07AM +0200, Laurent Pinchart wrote: > > On Wed, Mar 24, 2021 at 12:25:24PM +0100, Jacopo Mondi wrote: > > > Add a CameraHalConfig class to the Android Camera3 HAL layer. > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > --- > > > src/android/camera_hal_config.cpp | 385 ++++++++++++++++++++++++++++++ > > > src/android/camera_hal_config.h | 54 +++++ > > > src/android/meson.build | 1 + > > > 3 files changed, 440 insertions(+) > > > create mode 100644 src/android/camera_hal_config.cpp > > > create mode 100644 src/android/camera_hal_config.h > > > > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp > > > new file mode 100644 > > > index 000000000000..7e33dfb750ff > > > --- /dev/null > > > +++ b/src/android/camera_hal_config.cpp > > > @@ -0,0 +1,385 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2021, Google Inc. > > > + * > > > + * camera_hal_config.cpp - Camera HAL configuration file manager > > > + */ > > > +#include "camera_hal_config.h" > > > + > > > +#include <stdio.h> > > > +#include <stdlib.h> > > > +#include <string.h> > > > + > > > +#include "libcamera/internal/file.h" > > > +#include "libcamera/internal/log.h" > > > + > > > +using namespace libcamera; > > > + > > > +LOG_DEFINE_CATEGORY(HALConfig) > > > + > > > +const std::string CameraHalConfig::CameraBlock::toString() const > > > +{ > > > + std::stringstream ss; > > > + > > > + ss << "\'" << name << "\'" > > > + << " model: " << model > > > + << "(" << location << ")[" << rotation << "]"; > > > + > > > + return ss.str(); > > > +} > > > > As the amount of information will grow, I don't think we'll be able to > > print everything. This function is only used in a single location below, > > for debugging purpose, I wonder if we should keep it. > > > > > + > > > +CameraHalConfig::CameraHalConfig() > > > +{ > > > + if (!yaml_parser_initialize(&parser_)) > > > + LOG(HALConfig, Error) << "Failed to initialize yaml parser"; > > > +} > > > + > > > +CameraHalConfig::~CameraHalConfig() > > > +{ > > > + yaml_parser_delete(&parser_); > > > +} > > > + > > > +/* > > > + * Configuration files can be stored in system paths, which are identified > > > + * through the build configuration. > > > + * > > > + * However, when running uninstalled - the source location takes precedence. > > > > Can we run the camera HAL uninstalled ? :-) > > > > This meant "libcamera" uninstalled. Yes, but as this is the HAL configuration, I don't think we need to care about the uninstalled case. But now that I think about it, maybe you meant for this function to be ready for the libcamera core configuration file too ? > > > + */ > > > +const std::string CameraHalConfig::findFilePath(const std::string &filename) > > > +{ > > > + static std::array<std::string, 2> searchPaths = { > > > + LIBCAMERA_SYSCONF_DIR, > > > + LIBCAMERA_DATA_DIR, > > > > The data dir may not be very useful here, the configuration file is > > really system-specific, right ? > > > > I expect we may need to get the configuration file from system-specific > > locations, depending on whether we run on Chrome OS or Android, but we > > can handle that later. > > > > > + }; > > > + > > > + if (File::exists(filename)) > > > + return filename; > > > + > > > + std::string root = utils::libcameraSourcePath(); > > > + if (!root.empty()) { > > > + std::string configurationPath = root + "data/" + filename; > > > + > > > + if (File::exists(configurationPath)) > > > + return configurationPath; > > > + } > > > + > > > + for (std::string &path : searchPaths) { > > > + std::string configurationPath = path + "/" + filename; > > > + if (File::exists(configurationPath)) > > > + return configurationPath; > > > + } > > > + > > > + return ""; > > > +} > > > + > > > +/* > > > + * \brief Open the HAL configuration file and validate its content > > > + * \return 0 on success, a negative error code otherwise > > > + */ > > > +int CameraHalConfig::open() > > > +{ > > > + int ret; > > > + > > > + const std::string configPath = "camera_hal.yaml"; > > > + const std::string filePath = findFilePath(configPath); > > > + if (filePath.empty()) { > > > + LOG(HALConfig, Warning) > > > > This can be an Error, as we can't continue. > > > > > + << "File: \"" << configPath << "\" not found"; > > > > Maybe s/File:/Configuration file/ ? > > > > > + return -ENOENT; > > > + } > > > + > > > + FILE *fh = fopen(filePath.c_str(), "r"); > > > + if (!fh) { > > > + ret = -errno; > > > + LOG(HALConfig, Error) << "Failed to open file " << filePath > > > > Same here, "configuration file" ? > > > > > + << ": " << strerror(-ret); > > > + return ret; > > > + } > > > + > > > + yaml_parser_set_input_file(&parser_, fh); > > > + > > > + LOG(HALConfig, Debug) << "Reading configuration from " << filePath; > > > + > > > + ret = parseConfigFile(); > > > + fclose(fh); > > > + if (ret) > > > + return ret; > > > + > > > + LOG(HALConfig, Info) << "Device model: " << model_; > > > + LOG(HALConfig, Info) << "Device maker: " << maker_; > > > + for (const auto &c : cameras_) { > > > + const CameraBlock camera = c.second; > > > + LOG(HALConfig, Info) << camera.toString(); > > > + } > > > > I wonder if dumping the parsed file shouldn't be a debugging feature > > instead, or even if we need it at all. > > > > > + > > > + return 0; > > > +} > > > + > > > +const std::string &CameraHalConfig::cameraLocation(const std::string &camera) const > > > +{ > > > + static const std::string empty(""); > > > + const auto &it = cameras_.find(camera); > > > + if (it == cameras_.end()) { > > > + LOG(HALConfig, Error) > > > + << "Camera '" << camera > > > + << "' not described in the HAL configuration file"; > > > + return empty; > > > + } > > > + > > > + const CameraBlock &cam = it->second; > > > + return cam.location; > > > +} > > > + > > > +unsigned int CameraHalConfig::cameraRotation(const std::string &camera) const > > > +{ > > > + static const std::string empty(""); > > > + const auto &it = cameras_.find(camera); > > > + if (it == cameras_.end()) { > > > + LOG(HALConfig, Error) > > > + << "Camera '" << camera > > > + << "' not described in the HAL configuration file"; > > > + return 0; > > > + } > > > + > > > + const CameraBlock &cam = it->second; > > > + return cam.rotation; > > > +} > > > + > > > +const std::string &CameraHalConfig::cameraModel(const std::string &camera) const > > > +{ > > > + static const std::string empty(""); > > > + const auto &it = cameras_.find(camera); > > > + if (it == cameras_.end()) { > > > + LOG(HALConfig, Error) > > > + << "Camera '" << camera > > > + << "' not described in the HAL configuration file"; > > > + return empty; > > > + } > > > + > > > + const CameraBlock &cam = it->second; > > > + return cam.model; > > > +} > > > + > > > +std::string CameraHalConfig::parseValue(yaml_token_t &token) > > > +{ > > > + /* Make sure the token type is a value and get its content. */ > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_VALUE_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return ""; > > > + } > > > + yaml_token_delete(&token); > > > + > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_SCALAR_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return ""; > > > + } > > > + > > > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > > > + std::string value(scalar, token.data.scalar.length); > > > + yaml_token_delete(&token); > > > + > > > + return value; > > > +} > > > + > > > +std::string CameraHalConfig::parseKey(yaml_token_t &token) > > > +{ > > > + /* Make sure the token type is a key and get its value. */ > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_KEY_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return ""; > > > + } > > > + yaml_token_delete(&token); > > > + > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_SCALAR_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return ""; > > > + } > > > + > > > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > > > + std::string key(scalar, token.data.scalar.length); > > > + yaml_token_delete(&token); > > > + > > > + return key; > > > +} > > > + > > > +int CameraHalConfig::parseCameraBlock(yaml_token_t &token) > > > +{ > > > + /* The 'camera' block is a VALUE token type. */ > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_VALUE_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + yaml_token_delete(&token); > > > + > > > + /* > > > + * Parse the content of the camera block until we have an unbalanced > > > + * BLOCK_END_TOKEN which closes the camera block. > > > + * > > > + * Add a safety counter to make sure we don't loop indefinitely in case > > > + * the configuration file is malformed. > > > + */ > > > + unsigned int sentinel = 100; > > > + CameraBlock cameraBlock{}; > > > + int blockCounter = 0; > > > + do { > > > + yaml_parser_scan(&parser_, &token); > > > + switch (token.type) { > > > + case YAML_BLOCK_ENTRY_TOKEN: > > > + yaml_token_delete(&token); > > > + blockCounter++; > > > + break; > > > + case YAML_BLOCK_END_TOKEN: > > > + yaml_token_delete(&token); > > > + blockCounter--; > > > + break; > > > + case YAML_BLOCK_MAPPING_START_TOKEN: { > > > + yaml_token_delete(&token); > > > + > > > + std::string key = parseKey(token); > > > + std::string value = parseValue(token); > > > + if (key.empty() || value.empty()) { > > > + LOG(HALConfig, Error) > > > + << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + > > > + /* Validate that the parsed key is valid. */ > > > + if (key == "location") { > > > + if (value != "front" && value != "back" && > > > + value != "external") { > > > + LOG(HALConfig, Error) > > > + << "Unknown location: " << value; > > > + return -EINVAL; > > > + } > > > + cameraBlock.location = value; > > > + } else if (key == "rotation") { > > > + cameraBlock.rotation = strtoul(value.c_str(), > > > + NULL, 10); > > > + if (cameraBlock.rotation < 0) { > > > + LOG(HALConfig, Error) > > > + << "Unknown rotation: " > > > + << cameraBlock.rotation; > > > + return -EINVAL; > > > + } > > > + } else if (key == "name") { > > > + cameraBlock.name = value; > > > + } else if (key == "model") { > > > + cameraBlock.model = value; > > > + } else { > > > + LOG(HALConfig, Error) > > > + << "Configuration file is not valid " > > > + << "unknown key: " << key; > > > + return -EINVAL; > > > + } > > > + break; > > > + } > > > + default: > > > + yaml_token_delete(&token); > > > + break; > > > + } > > > + --sentinel; > > > + } while (blockCounter >= 0 && sentinel); > > > + if (!sentinel) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid "; > > > + return -EINVAL; > > > + } > > > + > > > + cameras_[cameraBlock.name] = cameraBlock; > > > + > > > + return 0; > > > +} > > > + > > > +int CameraHalConfig::parseEntryBlock(yaml_token_t &token) > > > +{ > > > + int ret; > > > + > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_BLOCK_MAPPING_START_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + yaml_token_delete(&token); > > > + > > > + std::string key = parseKey(token); > > > + if (key.empty()) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + > > > + if (key == "camera") { > > > + yaml_token_delete(&token); > > > + ret = parseCameraBlock(token); > > > + if (ret) > > > + return ret; > > > + } else if (key == "manufacturer") { > > > + yaml_token_delete(&token); > > > + maker_ = parseValue(token); > > > + if (maker_.empty()) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + } else if (key == "model") { > > > + yaml_token_delete(&token); > > > + model_ = parseValue(token); > > > + if (model_.empty()) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + } else { > > > + yaml_token_delete(&token); > > > + LOG(HALConfig, Error) << "Unknown key " << key; > > > + return -EINVAL; > > > + } > > > + > > > + return 0; > > > +} > > > + > > > +int CameraHalConfig::parseConfigFile() > > > +{ > > > + yaml_token_t token; > > > + int ret; > > > + > > > + yaml_parser_scan(&parser_, &token); > > > > Just to make sure you're aware of it, there's yaml_parser_load() that > > produces a parsed document that can then be walked. There's nothing > > wrong about using event-based parsing as you do though. > > I've found that feature under-documented with very few usage examples > around. Agreed, it's underdocumented in libyaml :-( I've found http://codepad.org/W7StVSkV that shows how to walk a document. If you think that would be simpler to use, feel free to do so, otherwise the token-based parser can be kept. > > I've skipped review of the parser implementation itself, as it may be > > changed depending on how patch 7/7 evolves. > > > > > + if (token.type != YAML_STREAM_START_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return EINVAL; > > > > -EINVAL > > > > > + } > > > + yaml_token_delete(&token); > > > + > > > + /* > > > + * Start parsing the content of the configuration file which > > > + * starts as with sequence block. > > > > s/as // ? > > > > > + */ > > > + yaml_parser_scan(&parser_, &token); > > > + if (token.type != YAML_BLOCK_SEQUENCE_START_TOKEN) { > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > + return -EINVAL; > > > + } > > > + yaml_token_delete(&token); > > > + > > > + /* Parse the single entry blocks. */ > > > + do { > > > + yaml_parser_scan(&parser_, &token); > > > + switch (token.type) { > > > + case YAML_BLOCK_ENTRY_TOKEN: > > > + yaml_token_delete(&token); > > > + ret = parseEntryBlock(token); > > > + break; > > > + case YAML_STREAM_END_TOKEN: > > > + /* Resources are released after the loop exit. */ > > > + break; > > > + default: > > > + /* Release resources before re-parsing. */ > > > + yaml_token_delete(&token); > > > + break; > > > + } > > > + } while (token.type != YAML_STREAM_END_TOKEN && ret >= 0); > > > + yaml_token_delete(&token); > > > + > > > + return ret; > > > +} > > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h > > > new file mode 100644 > > > index 000000000000..69d42658e90a > > > --- /dev/null > > > +++ b/src/android/camera_hal_config.h > > > @@ -0,0 +1,54 @@ > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > +/* > > > + * Copyright (C) 2021, Google Inc. > > > + * > > > + * camera_hal_config.h - Camera HAL configuration file manager > > > + */ > > > +#ifndef __ANDROID_CAMERA_HAL_CONFIG_H__ > > > +#define __ANDROID_CAMERA_HAL_CONFIG_H__ > > > + > > > +#include <map> > > > +#include <string> > > > +#include <yaml.h> > > > > It would be nice to hide the YAML perser from this header. It could be > > done by moving the parsing to a CameraHalConfigParser class, private to > > the camera_hal_config.cpp file. > > .. ok > > > > + > > > +class CameraHalConfig > > > +{ > > > +public: > > > + CameraHalConfig(); > > > + ~CameraHalConfig(); > > > + > > > + int open(); > > > + > > > + const std::string &deviceModel() const { return model_; } > > > + const std::string &deviceMaker() const { return maker_; } > > > + > > > + const std::string &cameraLocation(const std::string &camera) const; > > > + unsigned int cameraRotation(const std::string &camera) const; > > > + const std::string &cameraModel(const std::string &camera) const; > > > + > > > +private: > > > + yaml_parser_t parser_; > > > + > > > + std::string model_; > > > + std::string maker_; > > > + class CameraBlock > > > + { > > > + public: > > > + std::string name; > > > + std::string location; > > > + std::string model; > > > + unsigned int rotation; > > > + > > > + const std::string toString() const; > > > + }; > > > > How about making this class public (and probably renaming it), and replacing the last three public > > functions with > > > > const CameraBlock *cameraConfig(const std::string &name) const; > > > > ? That way each CameraDevice could store a reference to the > > corresponding camera configuration only. > > I prefer the CameraHalConfig::cameraProperty() interface but ok I wouldn't be surprised if we had to create a deeper hierarchy of properties in YAML, in which case cameraProperty() wouldn't scale well (there's also the lookup every time a property is accessed that we could save). > > > + std::map<std::string, CameraBlock> cameras_; > > > + > > > + const std::string findFilePath(const std::string &filename); > > > > No need for const at the beginning as you return by value, but you can > > add a const at the end. > > > > > + std::string parseValue(yaml_token_t &token); > > > + std::string parseKey(yaml_token_t &token); > > > + int parseCameraBlock(yaml_token_t &token); > > > + int parseEntryBlock(yaml_token_t &token); > > > + int parseConfigFile(); > > > +}; > > > + > > > +#endif /* __ANDROID_CAMERA_HAL_CONFIG_H__ */ > > > diff --git a/src/android/meson.build b/src/android/meson.build > > > index 19f94a4073f1..34e5c494a492 100644 > > > --- a/src/android/meson.build > > > +++ b/src/android/meson.build > > > @@ -44,6 +44,7 @@ android_hal_sources = files([ > > > 'camera3_hal.cpp', > > > 'camera_hal_manager.cpp', > > > 'camera_device.cpp', > > > + 'camera_hal_config.cpp', > > > 'camera_metadata.cpp', > > > 'camera_ops.cpp', > > > 'camera_stream.cpp',
Hi Niklas, On Mon, Mar 29, 2021 at 04:27:01PM +0200, Niklas Söderlund wrote: > On 2021-03-29 16:23:08 +0200, Jacopo Mondi wrote: > > On Thu, Mar 25, 2021 at 11:33:31PM +0100, Niklas Söderlund wrote: > > > On 2021-03-24 12:25:24 +0100, Jacopo Mondi wrote: > > > > Add a CameraHalConfig class to the Android Camera3 HAL layer. > > > > > > > > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> > > > > > > There are a few small nits below but I think this is a great step in the > > > right direction. > > > > > > Reviewed-by: Niklas Söderlund <niklas.soderlund@ragnatech.se> > > > > > > > --- > > > > src/android/camera_hal_config.cpp | 385 ++++++++++++++++++++++++++++++ > > > > src/android/camera_hal_config.h | 54 +++++ > > > > src/android/meson.build | 1 + > > > > 3 files changed, 440 insertions(+) > > > > create mode 100644 src/android/camera_hal_config.cpp > > > > create mode 100644 src/android/camera_hal_config.h > > > > > > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp > > > > new file mode 100644 > > > > index 000000000000..7e33dfb750ff > > > > --- /dev/null > > > > +++ b/src/android/camera_hal_config.cpp > > > > @@ -0,0 +1,385 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * Copyright (C) 2021, Google Inc. > > > > + * > > > > + * camera_hal_config.cpp - Camera HAL configuration file manager > > > > + */ > > > > +#include "camera_hal_config.h" > > > > + > > > > +#include <stdio.h> > > > > +#include <stdlib.h> > > > > +#include <string.h> > > > > + > > > > +#include "libcamera/internal/file.h" > > > > +#include "libcamera/internal/log.h" > > > > + > > > > +using namespace libcamera; > > > > + > > > > +LOG_DEFINE_CATEGORY(HALConfig) > > > > + > > > > +const std::string CameraHalConfig::CameraBlock::toString() const > > > > +{ > > > > + std::stringstream ss; > > > > + > > > > + ss << "\'" << name << "\'" > > > > + << " model: " << model > > > > + << "(" << location << ")[" << rotation << "]"; > > > > > > nit: The usage of the pattern "key: <value>" have been discouraged in > > > the core, do we extend the same to the HAL? > > > > > > > Not sure what you mean here -.- > > Sorry, I just noticed that the pattern of > > << " model: " << model > > Have been discouraged in the core in favor of > > << " model " << model As I suppose this comes from the fact I've asked to remove colons multiple times during review, I'd like to clarify that there's nothing wrong with the "key: value" syntax per se. When I asked for the colon to be dropped, that was because the message was written as a sentence. For instance, "The camera is rotated by 90 degrees" is more natural language than "The camera is rotated by: 90 degrees". I'd have no problem with a message such as "configuration: width: 1280, height: 960, format: NV12". > > > > + > > > > + return ss.str(); > > > > +} > > > > + > > > > +CameraHalConfig::CameraHalConfig() > > > > +{ > > > > + if (!yaml_parser_initialize(&parser_)) > > > > + LOG(HALConfig, Error) << "Failed to initialize yaml parser"; > > > > > > Fatal? > > > > > > > +} > > > > + > > > > +CameraHalConfig::~CameraHalConfig() > > > > +{ > > > > + yaml_parser_delete(&parser_); > > > > +} > > > > + > > > > +/* > > > > + * Configuration files can be stored in system paths, which are identified > > > > + * through the build configuration. > > > > + * > > > > + * However, when running uninstalled - the source location takes precedence. > > > > + */ > > > > +const std::string CameraHalConfig::findFilePath(const std::string &filename) > > > > +{ > > > > + static std::array<std::string, 2> searchPaths = { > > > > + LIBCAMERA_SYSCONF_DIR, > > > > + LIBCAMERA_DATA_DIR, > > > > + }; > > > > + > > > > + if (File::exists(filename)) > > > > + return filename; > > > > + > > > > + std::string root = utils::libcameraSourcePath(); > > > > + if (!root.empty()) { > > > > + std::string configurationPath = root + "data/" + filename; > > > > + > > > > + if (File::exists(configurationPath)) > > > > + return configurationPath; > > > > + } > > > > + > > > > + for (std::string &path : searchPaths) { > > > > + std::string configurationPath = path + "/" + filename; > > > > + if (File::exists(configurationPath)) > > > > + return configurationPath; > > > > + } > > > > + > > > > + return ""; > > > > +} > > > > + > > > > +/* > > > > + * \brief Open the HAL configuration file and validate its content > > > > + * \return 0 on success, a negative error code otherwise > > > > > > nit: We don't Doxygen the HAL is it not confusing to use it mark up here > > > then? > > > > > > > + */ > > > > +int CameraHalConfig::open() > > > > +{ > > > > + int ret; > > > > + > > > > + const std::string configPath = "camera_hal.yaml"; > > > > > > s/configPath/configFile/ ? > > > > > > > + const std::string filePath = findFilePath(configPath); > > > > > > As this is the only call site would it make sens to have findFilePath() > > > return a FILE * to get compartmentalizing of the error paths? > > > > > > > + if (filePath.empty()) { > > > > + LOG(HALConfig, Warning) > > > > + << "File: \"" << configPath << "\" not found"; > > > > + return -ENOENT; > > > > + } > > > > + > > > > + FILE *fh = fopen(filePath.c_str(), "r"); > > > > + if (!fh) { > > > > + ret = -errno; > > > > + LOG(HALConfig, Error) << "Failed to open file " << filePath > > > > + << ": " << strerror(-ret); > > > > + return ret; > > > > + } > > > > + > > > > + yaml_parser_set_input_file(&parser_, fh); > > > > + > > > > + LOG(HALConfig, Debug) << "Reading configuration from " << filePath; > > > > + > > > > + ret = parseConfigFile(); > > > > + fclose(fh); > > > > + if (ret) > > > > + return ret; > > > > + > > > > + LOG(HALConfig, Info) << "Device model: " << model_; > > > > + LOG(HALConfig, Info) << "Device maker: " << maker_; > > > > + for (const auto &c : cameras_) { > > > > + const CameraBlock camera = c.second; > > > > + LOG(HALConfig, Info) << camera.toString(); > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +const std::string &CameraHalConfig::cameraLocation(const std::string &camera) const > > > > +{ > > > > + static const std::string empty(""); > > > > + const auto &it = cameras_.find(camera); > > > > + if (it == cameras_.end()) { > > > > + LOG(HALConfig, Error) > > > > + << "Camera '" << camera > > > > + << "' not described in the HAL configuration file"; > > > > + return empty; > > > > + } > > > > + > > > > + const CameraBlock &cam = it->second; > > > > + return cam.location; > > > > +} > > > > + > > > > +unsigned int CameraHalConfig::cameraRotation(const std::string &camera) const > > > > +{ > > > > + static const std::string empty(""); > > > > > > empty not used. > > > > > > > Ouch, leftover > > > > > > + const auto &it = cameras_.find(camera); > > > > + if (it == cameras_.end()) { > > > > + LOG(HALConfig, Error) > > > > + << "Camera '" << camera > > > > + << "' not described in the HAL configuration file"; > > > > + return 0; > > > > + } > > > > + > > > > + const CameraBlock &cam = it->second; > > > > + return cam.rotation; > > > > +} > > > > + > > > > +const std::string &CameraHalConfig::cameraModel(const std::string &camera) const > > > > +{ > > > > + static const std::string empty(""); > > > > + const auto &it = cameras_.find(camera); > > > > + if (it == cameras_.end()) { > > > > + LOG(HALConfig, Error) > > > > + << "Camera '" << camera > > > > + << "' not described in the HAL configuration file"; > > > > + return empty; > > > > + } > > > > + > > > > + const CameraBlock &cam = it->second; > > > > + return cam.model; > > > > +} > > > > + > > > > +std::string CameraHalConfig::parseValue(yaml_token_t &token) > > > > +{ > > > > + /* Make sure the token type is a value and get its content. */ > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_VALUE_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return ""; > > > > + } > > > > + yaml_token_delete(&token); > > > > + > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_SCALAR_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return ""; > > > > + } > > > > + > > > > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > > > > + std::string value(scalar, token.data.scalar.length); > > > > + yaml_token_delete(&token); > > > > + > > > > + return value; > > > > +} > > > > + > > > > +std::string CameraHalConfig::parseKey(yaml_token_t &token) > > > > +{ > > > > + /* Make sure the token type is a key and get its value. */ > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_KEY_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return ""; > > > > + } > > > > + yaml_token_delete(&token); > > > > + > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_SCALAR_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return ""; > > > > + } > > > > + > > > > + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); > > > > + std::string key(scalar, token.data.scalar.length); > > > > + yaml_token_delete(&token); > > > > + > > > > + return key; > > > > +} > > > > + > > > > +int CameraHalConfig::parseCameraBlock(yaml_token_t &token) > > > > +{ > > > > + /* The 'camera' block is a VALUE token type. */ > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_VALUE_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + yaml_token_delete(&token); > > > > + > > > > + /* > > > > + * Parse the content of the camera block until we have an unbalanced > > > > + * BLOCK_END_TOKEN which closes the camera block. > > > > + * > > > > + * Add a safety counter to make sure we don't loop indefinitely in case > > > > + * the configuration file is malformed. > > > > + */ > > > > + unsigned int sentinel = 100; > > > > + CameraBlock cameraBlock{}; > > > > + int blockCounter = 0; > > > > + do { > > > > + yaml_parser_scan(&parser_, &token); > > > > + switch (token.type) { > > > > + case YAML_BLOCK_ENTRY_TOKEN: > > > > + yaml_token_delete(&token); > > > > + blockCounter++; > > > > + break; > > > > + case YAML_BLOCK_END_TOKEN: > > > > + yaml_token_delete(&token); > > > > + blockCounter--; > > > > + break; > > > > + case YAML_BLOCK_MAPPING_START_TOKEN: { > > > > + yaml_token_delete(&token); > > > > + > > > > + std::string key = parseKey(token); > > > > + std::string value = parseValue(token); > > > > + if (key.empty() || value.empty()) { > > > > + LOG(HALConfig, Error) > > > > + << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + > > > > + /* Validate that the parsed key is valid. */ > > > > + if (key == "location") { > > > > + if (value != "front" && value != "back" && > > > > + value != "external") { > > > > + LOG(HALConfig, Error) > > > > + << "Unknown location: " << value; > > > > + return -EINVAL; > > > > + } > > > > + cameraBlock.location = value; > > > > + } else if (key == "rotation") { > > > > + cameraBlock.rotation = strtoul(value.c_str(), > > > > + NULL, 10); > > > > + if (cameraBlock.rotation < 0) { > > > > + LOG(HALConfig, Error) > > > > + << "Unknown rotation: " > > > > + << cameraBlock.rotation; > > > > + return -EINVAL; > > > > + } > > > > + } else if (key == "name") { > > > > + cameraBlock.name = value; > > > > + } else if (key == "model") { > > > > + cameraBlock.model = value; > > > > + } else { > > > > + LOG(HALConfig, Error) > > > > + << "Configuration file is not valid " > > > > + << "unknown key: " << key; > > > > + return -EINVAL; > > > > + } > > > > + break; > > > > + } > > > > + default: > > > > + yaml_token_delete(&token); > > > > + break; > > > > + } > > > > + --sentinel; > > > > + } while (blockCounter >= 0 && sentinel); > > > > + if (!sentinel) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid "; > > > > + return -EINVAL; > > > > + } > > > > + > > > > + cameras_[cameraBlock.name] = cameraBlock; > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +int CameraHalConfig::parseEntryBlock(yaml_token_t &token) > > > > +{ > > > > + int ret; > > > > + > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_BLOCK_MAPPING_START_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + yaml_token_delete(&token); > > > > + > > > > + std::string key = parseKey(token); > > > > + if (key.empty()) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + > > > > + if (key == "camera") { > > > > + yaml_token_delete(&token); > > > > + ret = parseCameraBlock(token); > > > > + if (ret) > > > > + return ret; > > > > + } else if (key == "manufacturer") { > > > > + yaml_token_delete(&token); > > > > + maker_ = parseValue(token); > > > > + if (maker_.empty()) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + } else if (key == "model") { > > > > + yaml_token_delete(&token); > > > > + model_ = parseValue(token); > > > > + if (model_.empty()) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + } else { > > > > + yaml_token_delete(&token); > > > > + LOG(HALConfig, Error) << "Unknown key " << key; > > > > + return -EINVAL; > > > > + } > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +int CameraHalConfig::parseConfigFile() > > > > +{ > > > > + yaml_token_t token; > > > > + int ret; > > > > + > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_STREAM_START_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return EINVAL; > > > > + } > > > > + yaml_token_delete(&token); > > > > + > > > > + /* > > > > + * Start parsing the content of the configuration file which > > > > + * starts as with sequence block. > > > > + */ > > > > + yaml_parser_scan(&parser_, &token); > > > > + if (token.type != YAML_BLOCK_SEQUENCE_START_TOKEN) { > > > > + LOG(HALConfig, Error) << "Configuration file is not valid"; > > > > + return -EINVAL; > > > > + } > > > > + yaml_token_delete(&token); > > > > + > > > > + /* Parse the single entry blocks. */ > > > > + do { > > > > + yaml_parser_scan(&parser_, &token); > > > > + switch (token.type) { > > > > + case YAML_BLOCK_ENTRY_TOKEN: > > > > + yaml_token_delete(&token); > > > > + ret = parseEntryBlock(token); > > > > + break; > > > > + case YAML_STREAM_END_TOKEN: > > > > + /* Resources are released after the loop exit. */ > > > > + break; > > > > + default: > > > > + /* Release resources before re-parsing. */ > > > > + yaml_token_delete(&token); > > > > + break; > > > > + } > > > > + } while (token.type != YAML_STREAM_END_TOKEN && ret >= 0); > > > > + yaml_token_delete(&token); > > > > + > > > > + return ret; > > > > +} > > > > diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h > > > > new file mode 100644 > > > > index 000000000000..69d42658e90a > > > > --- /dev/null > > > > +++ b/src/android/camera_hal_config.h > > > > @@ -0,0 +1,54 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * Copyright (C) 2021, Google Inc. > > > > + * > > > > + * camera_hal_config.h - Camera HAL configuration file manager > > > > + */ > > > > +#ifndef __ANDROID_CAMERA_HAL_CONFIG_H__ > > > > +#define __ANDROID_CAMERA_HAL_CONFIG_H__ > > > > + > > > > +#include <map> > > > > +#include <string> > > > > +#include <yaml.h> > > > > + > > > > +class CameraHalConfig > > > > +{ > > > > +public: > > > > + CameraHalConfig(); > > > > + ~CameraHalConfig(); > > > > + > > > > + int open(); > > > > + > > > > + const std::string &deviceModel() const { return model_; } > > > > + const std::string &deviceMaker() const { return maker_; } > > > > + > > > > + const std::string &cameraLocation(const std::string &camera) const; > > > > + unsigned int cameraRotation(const std::string &camera) const; > > > > + const std::string &cameraModel(const std::string &camera) const; > > > > + > > > > +private: > > > > + yaml_parser_t parser_; > > > > + > > > > + std::string model_; > > > > + std::string maker_; > > > > + class CameraBlock > > > > + { > > > > + public: > > > > + std::string name; > > > > + std::string location; > > > > + std::string model; > > > > + unsigned int rotation; > > > > + > > > > + const std::string toString() const; > > > > + }; > > > > + std::map<std::string, CameraBlock> cameras_; > > > > + > > > > + const std::string findFilePath(const std::string &filename); > > > > + std::string parseValue(yaml_token_t &token); > > > > + std::string parseKey(yaml_token_t &token); > > > > + int parseCameraBlock(yaml_token_t &token); > > > > + int parseEntryBlock(yaml_token_t &token); > > > > + int parseConfigFile(); > > > > +}; > > > > + > > > > +#endif /* __ANDROID_CAMERA_HAL_CONFIG_H__ */ > > > > diff --git a/src/android/meson.build b/src/android/meson.build > > > > index 19f94a4073f1..34e5c494a492 100644 > > > > --- a/src/android/meson.build > > > > +++ b/src/android/meson.build > > > > @@ -44,6 +44,7 @@ android_hal_sources = files([ > > > > 'camera3_hal.cpp', > > > > 'camera_hal_manager.cpp', > > > > 'camera_device.cpp', > > > > + 'camera_hal_config.cpp', > > > > 'camera_metadata.cpp', > > > > 'camera_ops.cpp', > > > > 'camera_stream.cpp',
diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp new file mode 100644 index 000000000000..7e33dfb750ff --- /dev/null +++ b/src/android/camera_hal_config.cpp @@ -0,0 +1,385 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2021, Google Inc. + * + * camera_hal_config.cpp - Camera HAL configuration file manager + */ +#include "camera_hal_config.h" + +#include <stdio.h> +#include <stdlib.h> +#include <string.h> + +#include "libcamera/internal/file.h" +#include "libcamera/internal/log.h" + +using namespace libcamera; + +LOG_DEFINE_CATEGORY(HALConfig) + +const std::string CameraHalConfig::CameraBlock::toString() const +{ + std::stringstream ss; + + ss << "\'" << name << "\'" + << " model: " << model + << "(" << location << ")[" << rotation << "]"; + + return ss.str(); +} + +CameraHalConfig::CameraHalConfig() +{ + if (!yaml_parser_initialize(&parser_)) + LOG(HALConfig, Error) << "Failed to initialize yaml parser"; +} + +CameraHalConfig::~CameraHalConfig() +{ + yaml_parser_delete(&parser_); +} + +/* + * Configuration files can be stored in system paths, which are identified + * through the build configuration. + * + * However, when running uninstalled - the source location takes precedence. + */ +const std::string CameraHalConfig::findFilePath(const std::string &filename) +{ + static std::array<std::string, 2> searchPaths = { + LIBCAMERA_SYSCONF_DIR, + LIBCAMERA_DATA_DIR, + }; + + if (File::exists(filename)) + return filename; + + std::string root = utils::libcameraSourcePath(); + if (!root.empty()) { + std::string configurationPath = root + "data/" + filename; + + if (File::exists(configurationPath)) + return configurationPath; + } + + for (std::string &path : searchPaths) { + std::string configurationPath = path + "/" + filename; + if (File::exists(configurationPath)) + return configurationPath; + } + + return ""; +} + +/* + * \brief Open the HAL configuration file and validate its content + * \return 0 on success, a negative error code otherwise + */ +int CameraHalConfig::open() +{ + int ret; + + const std::string configPath = "camera_hal.yaml"; + const std::string filePath = findFilePath(configPath); + if (filePath.empty()) { + LOG(HALConfig, Warning) + << "File: \"" << configPath << "\" not found"; + return -ENOENT; + } + + FILE *fh = fopen(filePath.c_str(), "r"); + if (!fh) { + ret = -errno; + LOG(HALConfig, Error) << "Failed to open file " << filePath + << ": " << strerror(-ret); + return ret; + } + + yaml_parser_set_input_file(&parser_, fh); + + LOG(HALConfig, Debug) << "Reading configuration from " << filePath; + + ret = parseConfigFile(); + fclose(fh); + if (ret) + return ret; + + LOG(HALConfig, Info) << "Device model: " << model_; + LOG(HALConfig, Info) << "Device maker: " << maker_; + for (const auto &c : cameras_) { + const CameraBlock camera = c.second; + LOG(HALConfig, Info) << camera.toString(); + } + + return 0; +} + +const std::string &CameraHalConfig::cameraLocation(const std::string &camera) const +{ + static const std::string empty(""); + const auto &it = cameras_.find(camera); + if (it == cameras_.end()) { + LOG(HALConfig, Error) + << "Camera '" << camera + << "' not described in the HAL configuration file"; + return empty; + } + + const CameraBlock &cam = it->second; + return cam.location; +} + +unsigned int CameraHalConfig::cameraRotation(const std::string &camera) const +{ + static const std::string empty(""); + const auto &it = cameras_.find(camera); + if (it == cameras_.end()) { + LOG(HALConfig, Error) + << "Camera '" << camera + << "' not described in the HAL configuration file"; + return 0; + } + + const CameraBlock &cam = it->second; + return cam.rotation; +} + +const std::string &CameraHalConfig::cameraModel(const std::string &camera) const +{ + static const std::string empty(""); + const auto &it = cameras_.find(camera); + if (it == cameras_.end()) { + LOG(HALConfig, Error) + << "Camera '" << camera + << "' not described in the HAL configuration file"; + return empty; + } + + const CameraBlock &cam = it->second; + return cam.model; +} + +std::string CameraHalConfig::parseValue(yaml_token_t &token) +{ + /* Make sure the token type is a value and get its content. */ + yaml_parser_scan(&parser_, &token); + if (token.type != YAML_VALUE_TOKEN) { + LOG(HALConfig, Error) << "Configuration file is not valid"; + return ""; + } + yaml_token_delete(&token); + + yaml_parser_scan(&parser_, &token); + if (token.type != YAML_SCALAR_TOKEN) { + LOG(HALConfig, Error) << "Configuration file is not valid"; + return ""; + } + + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); + std::string value(scalar, token.data.scalar.length); + yaml_token_delete(&token); + + return value; +} + +std::string CameraHalConfig::parseKey(yaml_token_t &token) +{ + /* Make sure the token type is a key and get its value. */ + yaml_parser_scan(&parser_, &token); + if (token.type != YAML_KEY_TOKEN) { + LOG(HALConfig, Error) << "Configuration file is not valid"; + return ""; + } + yaml_token_delete(&token); + + yaml_parser_scan(&parser_, &token); + if (token.type != YAML_SCALAR_TOKEN) { + LOG(HALConfig, Error) << "Configuration file is not valid"; + return ""; + } + + char *scalar = reinterpret_cast<char *>(token.data.scalar.value); + std::string key(scalar, token.data.scalar.length); + yaml_token_delete(&token); + + return key; +} + +int CameraHalConfig::parseCameraBlock(yaml_token_t &token) +{ + /* The 'camera' block is a VALUE token type. */ + yaml_parser_scan(&parser_, &token); + if (token.type != YAML_VALUE_TOKEN) { + LOG(HALConfig, Error) << "Configuration file is not valid"; + return -EINVAL; + } + yaml_token_delete(&token); + + /* + * Parse the content of the camera block until we have an unbalanced + * BLOCK_END_TOKEN which closes the camera block. + * + * Add a safety counter to make sure we don't loop indefinitely in case + * the configuration file is malformed. + */ + unsigned int sentinel = 100; + CameraBlock cameraBlock{}; + int blockCounter = 0; + do { + yaml_parser_scan(&parser_, &token); + switch (token.type) { + case YAML_BLOCK_ENTRY_TOKEN: + yaml_token_delete(&token); + blockCounter++; + break; + case YAML_BLOCK_END_TOKEN: + yaml_token_delete(&token); + blockCounter--; + break; + case YAML_BLOCK_MAPPING_START_TOKEN: { + yaml_token_delete(&token); + + std::string key = parseKey(token); + std::string value = parseValue(token); + if (key.empty() || value.empty()) { + LOG(HALConfig, Error) + << "Configuration file is not valid"; + return -EINVAL; + } + + /* Validate that the parsed key is valid. */ + if (key == "location") { + if (value != "front" && value != "back" && + value != "external") { + LOG(HALConfig, Error) + << "Unknown location: " << value; + return -EINVAL; + } + cameraBlock.location = value; + } else if (key == "rotation") { + cameraBlock.rotation = strtoul(value.c_str(), + NULL, 10); + if (cameraBlock.rotation < 0) { + LOG(HALConfig, Error) + << "Unknown rotation: " + << cameraBlock.rotation; + return -EINVAL; + } + } else if (key == "name") { + cameraBlock.name = value; + } else if (key == "model") { + cameraBlock.model = value; + } else { + LOG(HALConfig, Error) + << "Configuration file is not valid " + << "unknown key: " << key; + return -EINVAL; + } + break; + } + default: + yaml_token_delete(&token); + break; + } + --sentinel; + } while (blockCounter >= 0 && sentinel); + if (!sentinel) { + LOG(HALConfig, Error) << "Configuration file is not valid "; + return -EINVAL; + } + + cameras_[cameraBlock.name] = cameraBlock; + + return 0; +} + +int CameraHalConfig::parseEntryBlock(yaml_token_t &token) +{ + int ret; + + yaml_parser_scan(&parser_, &token); + if (token.type != YAML_BLOCK_MAPPING_START_TOKEN) { + LOG(HALConfig, Error) << "Configuration file is not valid"; + return -EINVAL; + } + yaml_token_delete(&token); + + std::string key = parseKey(token); + if (key.empty()) { + LOG(HALConfig, Error) << "Configuration file is not valid"; + return -EINVAL; + } + + if (key == "camera") { + yaml_token_delete(&token); + ret = parseCameraBlock(token); + if (ret) + return ret; + } else if (key == "manufacturer") { + yaml_token_delete(&token); + maker_ = parseValue(token); + if (maker_.empty()) { + LOG(HALConfig, Error) << "Configuration file is not valid"; + return -EINVAL; + } + } else if (key == "model") { + yaml_token_delete(&token); + model_ = parseValue(token); + if (model_.empty()) { + LOG(HALConfig, Error) << "Configuration file is not valid"; + return -EINVAL; + } + } else { + yaml_token_delete(&token); + LOG(HALConfig, Error) << "Unknown key " << key; + return -EINVAL; + } + + return 0; +} + +int CameraHalConfig::parseConfigFile() +{ + yaml_token_t token; + int ret; + + yaml_parser_scan(&parser_, &token); + if (token.type != YAML_STREAM_START_TOKEN) { + LOG(HALConfig, Error) << "Configuration file is not valid"; + return EINVAL; + } + yaml_token_delete(&token); + + /* + * Start parsing the content of the configuration file which + * starts as with sequence block. + */ + yaml_parser_scan(&parser_, &token); + if (token.type != YAML_BLOCK_SEQUENCE_START_TOKEN) { + LOG(HALConfig, Error) << "Configuration file is not valid"; + return -EINVAL; + } + yaml_token_delete(&token); + + /* Parse the single entry blocks. */ + do { + yaml_parser_scan(&parser_, &token); + switch (token.type) { + case YAML_BLOCK_ENTRY_TOKEN: + yaml_token_delete(&token); + ret = parseEntryBlock(token); + break; + case YAML_STREAM_END_TOKEN: + /* Resources are released after the loop exit. */ + break; + default: + /* Release resources before re-parsing. */ + yaml_token_delete(&token); + break; + } + } while (token.type != YAML_STREAM_END_TOKEN && ret >= 0); + yaml_token_delete(&token); + + return ret; +} diff --git a/src/android/camera_hal_config.h b/src/android/camera_hal_config.h new file mode 100644 index 000000000000..69d42658e90a --- /dev/null +++ b/src/android/camera_hal_config.h @@ -0,0 +1,54 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2021, Google Inc. + * + * camera_hal_config.h - Camera HAL configuration file manager + */ +#ifndef __ANDROID_CAMERA_HAL_CONFIG_H__ +#define __ANDROID_CAMERA_HAL_CONFIG_H__ + +#include <map> +#include <string> +#include <yaml.h> + +class CameraHalConfig +{ +public: + CameraHalConfig(); + ~CameraHalConfig(); + + int open(); + + const std::string &deviceModel() const { return model_; } + const std::string &deviceMaker() const { return maker_; } + + const std::string &cameraLocation(const std::string &camera) const; + unsigned int cameraRotation(const std::string &camera) const; + const std::string &cameraModel(const std::string &camera) const; + +private: + yaml_parser_t parser_; + + std::string model_; + std::string maker_; + class CameraBlock + { + public: + std::string name; + std::string location; + std::string model; + unsigned int rotation; + + const std::string toString() const; + }; + std::map<std::string, CameraBlock> cameras_; + + const std::string findFilePath(const std::string &filename); + std::string parseValue(yaml_token_t &token); + std::string parseKey(yaml_token_t &token); + int parseCameraBlock(yaml_token_t &token); + int parseEntryBlock(yaml_token_t &token); + int parseConfigFile(); +}; + +#endif /* __ANDROID_CAMERA_HAL_CONFIG_H__ */ diff --git a/src/android/meson.build b/src/android/meson.build index 19f94a4073f1..34e5c494a492 100644 --- a/src/android/meson.build +++ b/src/android/meson.build @@ -44,6 +44,7 @@ android_hal_sources = files([ 'camera3_hal.cpp', 'camera_hal_manager.cpp', 'camera_device.cpp', + 'camera_hal_config.cpp', 'camera_metadata.cpp', 'camera_ops.cpp', 'camera_stream.cpp',
Add a CameraHalConfig class to the Android Camera3 HAL layer. Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> --- src/android/camera_hal_config.cpp | 385 ++++++++++++++++++++++++++++++ src/android/camera_hal_config.h | 54 +++++ src/android/meson.build | 1 + 3 files changed, 440 insertions(+) create mode 100644 src/android/camera_hal_config.cpp create mode 100644 src/android/camera_hal_config.h