| Message ID | 20220418120923.453131-3-hanlinchen@chromium.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Han-Lin, Thank you for the patch. On Mon, Apr 18, 2022 at 08:09:23PM +0800, Han-Lin Chen via libcamera-devel wrote: > Use YamlParser to parse android HAL config files, instead of handling > YAML tokens directly, as a preparation for the further parameter extension. > > Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > src/android/camera_hal_config.cpp | 334 +++++++----------------------- > src/android/meson.build | 1 - > 2 files changed, 74 insertions(+), 261 deletions(-) > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp > index aa90dac7..d269ab25 100644 > --- a/src/android/camera_hal_config.cpp > +++ b/src/android/camera_hal_config.cpp > @@ -14,15 +14,15 @@ namespace filesystem = std::experimental::filesystem; > #else > #include <filesystem> > #endif > -#include <stdio.h> > #include <stdlib.h> > #include <string> > -#include <yaml.h> > > #include <hardware/camera3.h> > > #include <libcamera/base/log.h> > > +#include <libcamera/internal/yaml_parser.h> > + > using namespace libcamera; > > LOG_DEFINE_CATEGORY(HALConfig) > @@ -37,16 +37,10 @@ public: > int parseConfigFile(FILE *fh, std::map<std::string, CameraConfigData> *cameras); > > private: > - std::string parseValue(); > - std::string parseKey(); > - int parseValueBlock(); > - int parseCameraLocation(CameraConfigData *cameraConfigData, > - const std::string &location); > - int parseCameraConfigData(const std::string &cameraId); > - int parseCameras(); > - int parseEntry(); > - > - yaml_parser_t parser_; > + int parseCameraConfigData(const std::string &cameraId, const YamlObject &); > + int parseLocation(const YamlObject &, CameraConfigData &cameraConfigData); > + int parseRotation(const YamlObject &, CameraConfigData &cameraConfigData); > + > std::map<std::string, CameraConfigData> *cameras_; > }; > > @@ -54,290 +48,110 @@ CameraHalConfig::Private::Private() > { > } > > -std::string CameraHalConfig::Private::parseValue() > +int CameraHalConfig::Private::parseConfigFile(FILE *fh, > + std::map<std::string, CameraConfigData> *cameras) > { > - 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) { > - yaml_token_delete(&token); > - return ""; > - } > - yaml_token_delete(&token); > - > - yaml_parser_scan(&parser_, &token); > - if (token.type != YAML_SCALAR_TOKEN) { > - yaml_token_delete(&token); > - return ""; > - } > - > - std::string value(reinterpret_cast<char *>(token.data.scalar.value), > - token.data.scalar.length); > - yaml_token_delete(&token); > - > - return value; > -} > + /* > + * Parse the HAL properties. > + * > + * Each camera properties block is a list of properties associated > + * with the ID (as assembled by CameraSensor::generateId()) of the > + * camera they refer to. > + * > + * cameras: > + * "camera0 id": > + * location: value > + * rotation: value > + * ... > + * > + * "camera1 id": > + * location: value > + * rotation: value > + * ... > + */ > > -std::string CameraHalConfig::Private::parseKey() > -{ > - yaml_token_t token; > + cameras_ = cameras; > > - /* Make sure the token type is a key and get its value. */ > - yaml_parser_scan(&parser_, &token); > - if (token.type != YAML_SCALAR_TOKEN) { > - yaml_token_delete(&token); > - return ""; > - } > + std::unique_ptr<YamlObject> root = YamlParser::parse(fh); > + if (!root) > + return -EINVAL; > > - std::string value(reinterpret_cast<char *>(token.data.scalar.value), > - token.data.scalar.length); > - yaml_token_delete(&token); > + if (!root->isDictionary()) > + return -EINVAL; > > - return value; > -} > + /* Parse property "cameras" */ > + if (!root->contains("cameras")) > + return -EINVAL; > > -int CameraHalConfig::Private::parseValueBlock() > -{ > - yaml_token_t token; > + const YamlObject &yamlObjectCameras = (*root)["cameras"]; > > - /* Make sure the next token are VALUE and BLOCK_MAPPING_START. */ > - yaml_parser_scan(&parser_, &token); > - if (token.type != YAML_VALUE_TOKEN) { > - yaml_token_delete(&token); > + if (!yamlObjectCameras.isDictionary()) > return -EINVAL; > - } > - yaml_token_delete(&token); > > - yaml_parser_scan(&parser_, &token); > - if (token.type != YAML_BLOCK_MAPPING_START_TOKEN) { > - yaml_token_delete(&token); > - return -EINVAL; > + std::vector<std::string> cameraIds = yamlObjectCameras.getMemberNames(); > + for (const std::string &cameraId : cameraIds) { > + if (parseCameraConfigData(cameraId, > + yamlObjectCameras[cameraId])) > + return -EINVAL; > } > - yaml_token_delete(&token); > > return 0; > } > > -int CameraHalConfig::Private::parseCameraLocation(CameraConfigData *cameraConfigData, > - const std::string &location) > -{ > - if (location == "front") > - cameraConfigData->facing = CAMERA_FACING_FRONT; > - else if (location == "back") > - cameraConfigData->facing = CAMERA_FACING_BACK; > - else > - return -EINVAL; > +int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId, > + const YamlObject &cameraObject) > > - return 0; > -} > - > -int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId) > { > - int ret = parseValueBlock(); > - if (ret) > - return ret; > - > - /* > - * Parse the camera properties and store them in a cameraConfigData > - * instance. > - * > - * Add a safety counter to make sure we don't loop indefinitely in case > - * the configuration file is malformed. > - */ > - CameraConfigData cameraConfigData; > - unsigned int sentinel = 100; > - bool blockEnd = false; > - yaml_token_t token; > - > - do { > - yaml_parser_scan(&parser_, &token); > - switch (token.type) { > - case YAML_KEY_TOKEN: { > - yaml_token_delete(&token); > - > - /* > - * Parse the camera property key and make sure it is > - * valid. > - */ > - std::string key = parseKey(); > - std::string value = parseValue(); > - if (key.empty() || value.empty()) > - return -EINVAL; > - > - if (key == "location") { > - ret = parseCameraLocation(&cameraConfigData, value); > - if (ret) { > - LOG(HALConfig, Error) > - << "Unknown location: " << value; > - return -EINVAL; > - } > - } else if (key == "rotation") { > - ret = std::stoi(value); > - if (ret < 0 || ret >= 360) { > - LOG(HALConfig, Error) > - << "Unknown rotation: " << value; > - return -EINVAL; > - } > - cameraConfigData.rotation = ret; > - } else { > - LOG(HALConfig, Error) > - << "Unknown key: " << key; > - return -EINVAL; > - } > - break; > - } > - > - case YAML_BLOCK_END_TOKEN: > - blockEnd = true; > - [[fallthrough]]; > - default: > - yaml_token_delete(&token); > - break; > - } > - > - --sentinel; > - } while (!blockEnd && sentinel); > - if (!sentinel) > + if (!cameraObject.isDictionary()) > return -EINVAL; > > - (*cameras_)[cameraId] = cameraConfigData; > - > - return 0; > -} > + CameraConfigData &cameraConfigData = (*cameras_)[cameraId]; > > -int CameraHalConfig::Private::parseCameras() > -{ > - int ret = parseValueBlock(); > - if (ret) { > - LOG(HALConfig, Error) << "Configuration file is not valid"; > - return ret; > - } > + /* Parse property "location" */ > + if (parseLocation(cameraObject, cameraConfigData)) > + return -EINVAL; > > - /* > - * Parse the camera properties. > - * > - * Each camera properties block is a list of properties associated > - * with the ID (as assembled by CameraSensor::generateId()) of the > - * camera they refer to. > - * > - * cameras: > - * "camera0 id": > - * key: value > - * key: value > - * ... > - * > - * "camera1 id": > - * key: value > - * key: value > - * ... > - */ > - bool blockEnd = false; > - yaml_token_t token; > - do { > - yaml_parser_scan(&parser_, &token); > - switch (token.type) { > - case YAML_KEY_TOKEN: { > - yaml_token_delete(&token); > - > - /* Parse the camera ID as key of the property list. */ > - std::string cameraId = parseKey(); > - if (cameraId.empty()) > - return -EINVAL; > - > - ret = parseCameraConfigData(cameraId); > - if (ret) > - return -EINVAL; > - break; > - } > - case YAML_BLOCK_END_TOKEN: > - blockEnd = true; > - [[fallthrough]]; > - default: > - yaml_token_delete(&token); > - break; > - } > - } while (!blockEnd); > + /* Parse property "rotation" */ > + if (parseRotation(cameraObject, cameraConfigData)) > + return -EINVAL; > > return 0; > } > > -int CameraHalConfig::Private::parseEntry() > +int CameraHalConfig::Private::parseLocation(const YamlObject &cameraObject, > + CameraConfigData &cameraConfigData) > { > - int ret = -EINVAL; > + if (!cameraObject.contains("location")) > + return -EINVAL; > > - /* > - * Parse each key we find in the file. > - * > - * The 'cameras' keys maps to a list of (lists) of camera properties. > - */ > + std::string location = cameraObject["location"].get<std::string>(""); > > - std::string key = parseKey(); > - if (key.empty()) > - return ret; > - > - if (key == "cameras") > - ret = parseCameras(); > + if (location == "front") > + cameraConfigData.facing = CAMERA_FACING_FRONT; > + else if (location == "back") > + cameraConfigData.facing = CAMERA_FACING_BACK; > else > - LOG(HALConfig, Error) << "Unknown key: " << key; > + return -EINVAL; > > - return ret; > + return 0; > } > > -int CameraHalConfig::Private::parseConfigFile(FILE *fh, > - std::map<std::string, CameraConfigData> *cameras) > +int CameraHalConfig::Private::parseRotation(const YamlObject &cameraObject, > + CameraConfigData &cameraConfigData) > { > - cameras_ = cameras; > - > - int ret = yaml_parser_initialize(&parser_); > - if (!ret) { > - LOG(HALConfig, Error) << "Failed to initialize yaml parser"; > - return -EINVAL; > - } > - yaml_parser_set_input_file(&parser_, fh); > - > - yaml_token_t token; > - yaml_parser_scan(&parser_, &token); > - if (token.type != YAML_STREAM_START_TOKEN) { > - LOG(HALConfig, Error) << "Configuration file is not valid"; > - yaml_token_delete(&token); > - yaml_parser_delete(&parser_); > + if (!cameraObject.contains("rotation")) > return -EINVAL; > - } > - yaml_token_delete(&token); > > - yaml_parser_scan(&parser_, &token); > - if (token.type != YAML_BLOCK_MAPPING_START_TOKEN) { > - LOG(HALConfig, Error) << "Configuration file is not valid"; > - yaml_token_delete(&token); > - yaml_parser_delete(&parser_); > + int32_t rotation = cameraObject["rotation"].get<int32_t>(-1); > + > + if (rotation < 0 || rotation >= 360) { > + LOG(HALConfig, Error) > + << "Unknown rotation: " << rotation; > return -EINVAL; > } > - yaml_token_delete(&token); > - > - /* Parse the file and parse each single key one by one. */ > - do { > - yaml_parser_scan(&parser_, &token); > - switch (token.type) { > - case YAML_KEY_TOKEN: > - yaml_token_delete(&token); > - ret = parseEntry(); > - break; > - > - case YAML_STREAM_END_TOKEN: > - ret = -ENOENT; > - [[fallthrough]]; > - default: > - yaml_token_delete(&token); > - break; > - } > - } while (ret >= 0); > - yaml_parser_delete(&parser_); > - > - if (ret && ret != -ENOENT) > - LOG(HALConfig, Error) << "Configuration file is not valid"; > - > - return ret == -ENOENT ? 0 : ret; > + > + cameraConfigData.rotation = rotation; > + return 0; > } > > CameraHalConfig::CameraHalConfig() > diff --git a/src/android/meson.build b/src/android/meson.build > index 75b4bf20..1bba54de 100644 > --- a/src/android/meson.build > +++ b/src/android/meson.build > @@ -3,7 +3,6 @@ > android_deps = [ > dependency('libexif', required : get_option('android')), > dependency('libjpeg', required : get_option('android')), > - dependency('yaml-0.1', required : get_option('android')), > libcamera_private, > ] >
diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp index aa90dac7..d269ab25 100644 --- a/src/android/camera_hal_config.cpp +++ b/src/android/camera_hal_config.cpp @@ -14,15 +14,15 @@ namespace filesystem = std::experimental::filesystem; #else #include <filesystem> #endif -#include <stdio.h> #include <stdlib.h> #include <string> -#include <yaml.h> #include <hardware/camera3.h> #include <libcamera/base/log.h> +#include <libcamera/internal/yaml_parser.h> + using namespace libcamera; LOG_DEFINE_CATEGORY(HALConfig) @@ -37,16 +37,10 @@ public: int parseConfigFile(FILE *fh, std::map<std::string, CameraConfigData> *cameras); private: - std::string parseValue(); - std::string parseKey(); - int parseValueBlock(); - int parseCameraLocation(CameraConfigData *cameraConfigData, - const std::string &location); - int parseCameraConfigData(const std::string &cameraId); - int parseCameras(); - int parseEntry(); - - yaml_parser_t parser_; + int parseCameraConfigData(const std::string &cameraId, const YamlObject &); + int parseLocation(const YamlObject &, CameraConfigData &cameraConfigData); + int parseRotation(const YamlObject &, CameraConfigData &cameraConfigData); + std::map<std::string, CameraConfigData> *cameras_; }; @@ -54,290 +48,110 @@ CameraHalConfig::Private::Private() { } -std::string CameraHalConfig::Private::parseValue() +int CameraHalConfig::Private::parseConfigFile(FILE *fh, + std::map<std::string, CameraConfigData> *cameras) { - 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) { - yaml_token_delete(&token); - return ""; - } - yaml_token_delete(&token); - - yaml_parser_scan(&parser_, &token); - if (token.type != YAML_SCALAR_TOKEN) { - yaml_token_delete(&token); - return ""; - } - - std::string value(reinterpret_cast<char *>(token.data.scalar.value), - token.data.scalar.length); - yaml_token_delete(&token); - - return value; -} + /* + * Parse the HAL properties. + * + * Each camera properties block is a list of properties associated + * with the ID (as assembled by CameraSensor::generateId()) of the + * camera they refer to. + * + * cameras: + * "camera0 id": + * location: value + * rotation: value + * ... + * + * "camera1 id": + * location: value + * rotation: value + * ... + */ -std::string CameraHalConfig::Private::parseKey() -{ - yaml_token_t token; + cameras_ = cameras; - /* Make sure the token type is a key and get its value. */ - yaml_parser_scan(&parser_, &token); - if (token.type != YAML_SCALAR_TOKEN) { - yaml_token_delete(&token); - return ""; - } + std::unique_ptr<YamlObject> root = YamlParser::parse(fh); + if (!root) + return -EINVAL; - std::string value(reinterpret_cast<char *>(token.data.scalar.value), - token.data.scalar.length); - yaml_token_delete(&token); + if (!root->isDictionary()) + return -EINVAL; - return value; -} + /* Parse property "cameras" */ + if (!root->contains("cameras")) + return -EINVAL; -int CameraHalConfig::Private::parseValueBlock() -{ - yaml_token_t token; + const YamlObject &yamlObjectCameras = (*root)["cameras"]; - /* Make sure the next token are VALUE and BLOCK_MAPPING_START. */ - yaml_parser_scan(&parser_, &token); - if (token.type != YAML_VALUE_TOKEN) { - yaml_token_delete(&token); + if (!yamlObjectCameras.isDictionary()) return -EINVAL; - } - yaml_token_delete(&token); - yaml_parser_scan(&parser_, &token); - if (token.type != YAML_BLOCK_MAPPING_START_TOKEN) { - yaml_token_delete(&token); - return -EINVAL; + std::vector<std::string> cameraIds = yamlObjectCameras.getMemberNames(); + for (const std::string &cameraId : cameraIds) { + if (parseCameraConfigData(cameraId, + yamlObjectCameras[cameraId])) + return -EINVAL; } - yaml_token_delete(&token); return 0; } -int CameraHalConfig::Private::parseCameraLocation(CameraConfigData *cameraConfigData, - const std::string &location) -{ - if (location == "front") - cameraConfigData->facing = CAMERA_FACING_FRONT; - else if (location == "back") - cameraConfigData->facing = CAMERA_FACING_BACK; - else - return -EINVAL; +int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId, + const YamlObject &cameraObject) - return 0; -} - -int CameraHalConfig::Private::parseCameraConfigData(const std::string &cameraId) { - int ret = parseValueBlock(); - if (ret) - return ret; - - /* - * Parse the camera properties and store them in a cameraConfigData - * instance. - * - * Add a safety counter to make sure we don't loop indefinitely in case - * the configuration file is malformed. - */ - CameraConfigData cameraConfigData; - unsigned int sentinel = 100; - bool blockEnd = false; - yaml_token_t token; - - do { - yaml_parser_scan(&parser_, &token); - switch (token.type) { - case YAML_KEY_TOKEN: { - yaml_token_delete(&token); - - /* - * Parse the camera property key and make sure it is - * valid. - */ - std::string key = parseKey(); - std::string value = parseValue(); - if (key.empty() || value.empty()) - return -EINVAL; - - if (key == "location") { - ret = parseCameraLocation(&cameraConfigData, value); - if (ret) { - LOG(HALConfig, Error) - << "Unknown location: " << value; - return -EINVAL; - } - } else if (key == "rotation") { - ret = std::stoi(value); - if (ret < 0 || ret >= 360) { - LOG(HALConfig, Error) - << "Unknown rotation: " << value; - return -EINVAL; - } - cameraConfigData.rotation = ret; - } else { - LOG(HALConfig, Error) - << "Unknown key: " << key; - return -EINVAL; - } - break; - } - - case YAML_BLOCK_END_TOKEN: - blockEnd = true; - [[fallthrough]]; - default: - yaml_token_delete(&token); - break; - } - - --sentinel; - } while (!blockEnd && sentinel); - if (!sentinel) + if (!cameraObject.isDictionary()) return -EINVAL; - (*cameras_)[cameraId] = cameraConfigData; - - return 0; -} + CameraConfigData &cameraConfigData = (*cameras_)[cameraId]; -int CameraHalConfig::Private::parseCameras() -{ - int ret = parseValueBlock(); - if (ret) { - LOG(HALConfig, Error) << "Configuration file is not valid"; - return ret; - } + /* Parse property "location" */ + if (parseLocation(cameraObject, cameraConfigData)) + return -EINVAL; - /* - * Parse the camera properties. - * - * Each camera properties block is a list of properties associated - * with the ID (as assembled by CameraSensor::generateId()) of the - * camera they refer to. - * - * cameras: - * "camera0 id": - * key: value - * key: value - * ... - * - * "camera1 id": - * key: value - * key: value - * ... - */ - bool blockEnd = false; - yaml_token_t token; - do { - yaml_parser_scan(&parser_, &token); - switch (token.type) { - case YAML_KEY_TOKEN: { - yaml_token_delete(&token); - - /* Parse the camera ID as key of the property list. */ - std::string cameraId = parseKey(); - if (cameraId.empty()) - return -EINVAL; - - ret = parseCameraConfigData(cameraId); - if (ret) - return -EINVAL; - break; - } - case YAML_BLOCK_END_TOKEN: - blockEnd = true; - [[fallthrough]]; - default: - yaml_token_delete(&token); - break; - } - } while (!blockEnd); + /* Parse property "rotation" */ + if (parseRotation(cameraObject, cameraConfigData)) + return -EINVAL; return 0; } -int CameraHalConfig::Private::parseEntry() +int CameraHalConfig::Private::parseLocation(const YamlObject &cameraObject, + CameraConfigData &cameraConfigData) { - int ret = -EINVAL; + if (!cameraObject.contains("location")) + return -EINVAL; - /* - * Parse each key we find in the file. - * - * The 'cameras' keys maps to a list of (lists) of camera properties. - */ + std::string location = cameraObject["location"].get<std::string>(""); - std::string key = parseKey(); - if (key.empty()) - return ret; - - if (key == "cameras") - ret = parseCameras(); + if (location == "front") + cameraConfigData.facing = CAMERA_FACING_FRONT; + else if (location == "back") + cameraConfigData.facing = CAMERA_FACING_BACK; else - LOG(HALConfig, Error) << "Unknown key: " << key; + return -EINVAL; - return ret; + return 0; } -int CameraHalConfig::Private::parseConfigFile(FILE *fh, - std::map<std::string, CameraConfigData> *cameras) +int CameraHalConfig::Private::parseRotation(const YamlObject &cameraObject, + CameraConfigData &cameraConfigData) { - cameras_ = cameras; - - int ret = yaml_parser_initialize(&parser_); - if (!ret) { - LOG(HALConfig, Error) << "Failed to initialize yaml parser"; - return -EINVAL; - } - yaml_parser_set_input_file(&parser_, fh); - - yaml_token_t token; - yaml_parser_scan(&parser_, &token); - if (token.type != YAML_STREAM_START_TOKEN) { - LOG(HALConfig, Error) << "Configuration file is not valid"; - yaml_token_delete(&token); - yaml_parser_delete(&parser_); + if (!cameraObject.contains("rotation")) return -EINVAL; - } - yaml_token_delete(&token); - yaml_parser_scan(&parser_, &token); - if (token.type != YAML_BLOCK_MAPPING_START_TOKEN) { - LOG(HALConfig, Error) << "Configuration file is not valid"; - yaml_token_delete(&token); - yaml_parser_delete(&parser_); + int32_t rotation = cameraObject["rotation"].get<int32_t>(-1); + + if (rotation < 0 || rotation >= 360) { + LOG(HALConfig, Error) + << "Unknown rotation: " << rotation; return -EINVAL; } - yaml_token_delete(&token); - - /* Parse the file and parse each single key one by one. */ - do { - yaml_parser_scan(&parser_, &token); - switch (token.type) { - case YAML_KEY_TOKEN: - yaml_token_delete(&token); - ret = parseEntry(); - break; - - case YAML_STREAM_END_TOKEN: - ret = -ENOENT; - [[fallthrough]]; - default: - yaml_token_delete(&token); - break; - } - } while (ret >= 0); - yaml_parser_delete(&parser_); - - if (ret && ret != -ENOENT) - LOG(HALConfig, Error) << "Configuration file is not valid"; - - return ret == -ENOENT ? 0 : ret; + + cameraConfigData.rotation = rotation; + return 0; } CameraHalConfig::CameraHalConfig() diff --git a/src/android/meson.build b/src/android/meson.build index 75b4bf20..1bba54de 100644 --- a/src/android/meson.build +++ b/src/android/meson.build @@ -3,7 +3,6 @@ android_deps = [ dependency('libexif', required : get_option('android')), dependency('libjpeg', required : get_option('android')), - dependency('yaml-0.1', required : get_option('android')), libcamera_private, ]
Use YamlParser to parse android HAL config files, instead of handling YAML tokens directly, as a preparation for the further parameter extension. Signed-off-by: Han-Lin Chen <hanlinchen@chromium.org> --- src/android/camera_hal_config.cpp | 334 +++++++----------------------- src/android/meson.build | 1 - 2 files changed, 74 insertions(+), 261 deletions(-)