[libcamera-devel] [PATCH v2 3/6] android: Add CameraHalConfig class
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Sat Apr 3 04:47:51 CEST 2021
Hi Hiro,
On Tue, Mar 30, 2021 at 01:16:22PM +0900, Hirokazu Honda wrote:
> On Tue, Mar 30, 2021 at 12:53 PM Laurent Pinchart wrote:
> > On Tue, Mar 30, 2021 at 12:48:20PM +0900, Hirokazu Honda wrote:
> > > On Tue, Mar 30, 2021 at 9:40 AM Laurent Pinchartwrote:
> > > > On Mon, Mar 29, 2021 at 05:28:04PM +0200, Jacopo Mondi wrote:
> > > > > Add a CameraHalConfig class to the Android Camera3 HAL layer.
> > > >
> > > > There are quite a few comments from v1 that haven't been addressed.
> > > > Maybe that's because my last reply to v1 came after you've sent this
> > > > series. The comments may also not all be correct, applicable and/or
> > > > desired, but could you then reply to them to explain why they're ignored
> > > > ?
> > > >
> > > > In particular, I think that exposing CameraProps would simplify the
> > > > implementation quite a bit, by removing the need for the camera*()
> > > > functions, with the lookup of the CameraProps entry every time, and the
> > > > associated error handling.
> > >
> > > +1. So we should pass the structure to CameraDevice by value.
> >
> > Why not by const reference ?
>
> I would not like having const reference in member variables.
> I actually thought this was stated in Google C++ Style Guide, and
> searched it, but found there was no statement like that.
> So this is just my preference.
> For the purpose, I usually use a const pointer.
> Considering why I don't like having const reference member variables,
> one of reasons might be what I don't get used to :p, but also from a
> caller of the constructor the lifetime of CameraHalConfig is unclear.
> For instance, CameraDevice(.., config) is called, but a caller has no
> idea when the config should be alive by, while CameraDevice(...,
> &config) tells a caller to keep the config object outlive to
> CameraDevice.
We have coding style rules to handle borrowed references, explained in
Documentation/coding-style.rst:
When the object is stored in a std::unique_ptr<>, borrowing passes a
reference to the object, not to the std::unique_ptr<>, as
* a 'const &' when the object doesn't need to be modified and may not be
null.
* a pointer when the object may be modified or may be null. Unless
otherwise specified, pointers passed to functions are considered as
borrowed references valid for the duration of the function only.
(I've just realized that we may want to update this, as the rule isn't
specific to std::unique_ptr<>, it can apply equally to, for instance,
member variables of a class.)
The difference between const reference and non-const pointer is not
related to object ownership, but to constness. We tend to favour
pointers for variables that can be modified by the callee, and const
references for variables that can't. This means that we can't use
reference vs. pointer to express ownership and life time of objects.
This can of course be reconsidered, but I'd like the coding style to
remain consistent, and thus first update the documentation and the code
base.
> Again, this might be my personal preference. But does this description
> make sense?
>
> > > > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > > > ---
> > > > > README.rst | 2 +-
> > > > > src/android/camera_hal_config.cpp | 463 ++++++++++++++++++++++++++++++
> > > > > src/android/camera_hal_config.h | 53 ++++
> > > > > src/android/meson.build | 2 +
> > > > > 4 files changed, 519 insertions(+), 1 deletion(-)
> > > > > create mode 100644 src/android/camera_hal_config.cpp
> > > > > create mode 100644 src/android/camera_hal_config.h
> > > > >
> > > > > diff --git a/README.rst b/README.rst
> > > > > index 7a98164bbb0a..f68d435196de 100644
> > > > > --- a/README.rst
> > > > > +++ b/README.rst
> > > > > @@ -88,7 +88,7 @@ for tracing with lttng: [optional]
> > > > > liblttng-ust-dev python3-jinja2 lttng-tools
> > > > >
> > > > > for android: [optional]
> > > > > - libexif libjpeg
> > > > > + libexif libjpeg libyaml
> > > > >
> > > > > Using GStreamer plugin
> > > > > ~~~~~~~~~~~~~~~~~~~~~~
> > > > > diff --git a/src/android/camera_hal_config.cpp b/src/android/camera_hal_config.cpp
> > > > > new file mode 100644
> > > > > index 000000000000..846dd7357b0a
> > > > > --- /dev/null
> > > > > +++ b/src/android/camera_hal_config.cpp
> > > > > @@ -0,0 +1,463 @@
> > > > > +/* 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 <hardware/camera3.h>
> > > > > +
> > > > > +#include "libcamera/internal/file.h"
> > > > > +#include "libcamera/internal/log.h"
> > > > > +
> > > > > +using namespace libcamera;
> > > > > +
> > > > > +LOG_DEFINE_CATEGORY(HALConfig)
> > > > > +
> > > > > +CameraHalConfig::CameraHalConfig()
> > > > > +{
> > > > > + if (!yaml_parser_initialize(&parser_))
> > > > > + LOG(HALConfig, Fatal) << "Failed to initialize yaml parser";
> > > > > +}
> > > > > +
> > > > > +CameraHalConfig::~CameraHalConfig()
> > > > > +{
> > > > > + yaml_parser_delete(&parser_);
> > > > > +}
> > > > > +
> > >
> > > It looks strange to keep parser_ alive as long as CameraHalConfig,
> > > because parsing is done in open().
> > > I would initialize the parser in open() and destroy it in the end of open().
> > > In this direction, we would prefer moving parse* functions to .cpp
> > > file and also moving yaml include to .cpp.
> >
> > I've mentioned that before :-) It could be done on top though.
> >
> > > > > +/*
> > > > > + * Configuration files can be stored in system paths, which are identified
> > > > > + * through the build configuration.
> > > > > + *
> > > > > + * However, when running uninstalled - the source location takes precedence.
> > > > > + */
> > > > > +std::string CameraHalConfig::findFilePath(const std::string &filename) const
> > > > > +{
> > > > > + static std::array<std::string, 2> searchPaths = {
> > > > > + LIBCAMERA_SYSCONF_DIR,
> > > > > + LIBCAMERA_DATA_DIR,
> > > >
> > > > There's also a comment in v1 about data dir. Data dir is typically
> > > > /usr/share/libcamera/. Given that the configuration file is
> > > > device-specific, I don't think we'll ever look for it there, but only in
> > > > sysconf 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 "";
> > > > > +}
> > > > > +
> > > > > +FILE *CameraHalConfig::openFile(const std::string &filename)
> > > > > +{
> > > > > + const std::string filePath = findFilePath(filename);
> > > > > + if (filePath.empty()) {
> > > > > + LOG(HALConfig, Error)
> > > > > + << "Configuration file: \"" << filename << "\" not found";
> > > > > + return nullptr;
> > > > > + }
> > > > > +
> > > > > + LOG(HALConfig, Debug) << "Reading configuration file from " << filePath;
> > > > > +
> > > > > + FILE *fh = fopen(filePath.c_str(), "r");
> > > > > + if (!fh) {
> > > > > + int ret = -errno;
> > > > > + LOG(HALConfig, Error) << "Failed to open configuration file "
> > > > > + << filePath << ": " << strerror(-ret);
> > > > > + return nullptr;
> > > > > + }
> > > > > +
> > > > > + return fh;
> > > > > +}
> > > > > +
> > > > > +/*
> > > > > + * Open the HAL configuration file and validate its content.
> > > > > + * Return 0 on success, a negative error code otherwise
> > > > > + */
> > > > > +int CameraHalConfig::open()
> > > > > +{
> > > > > + int ret;
> > > > > +
> > > > > + FILE *fh = openFile("camera_hal.yaml");
> > > > > + if (!fh)
> > > > > + return -ENOENT;
> > > > > +
> > > > > + yaml_parser_set_input_file(&parser_, fh);
> > > > > + 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 std::string &cameraId = c.first;
> > > > > + const CameraProps &camera = c.second;
> > > > > + LOG(HALConfig, Info) << "'" << cameraId << "' "
> > > > > + << " model: " << camera.model
> > > > > + << "(" << camera.location << ")["
> > > > > + << camera.rotation << "]";
> > > > > + }
> > > >
> > > > There's also a comment in v1 about whether this should be a debugging
> > > > feature.
> > > >
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +int CameraHalConfig::cameraLocation(const std::string &camera) const
> > > > > +{
> > > > > + const auto &it = cameras_.find(camera);
> > > > > + if (it == cameras_.end()) {
> > > > > + LOG(HALConfig, Error)
> > > > > + << "Camera '" << camera
> > > > > + << "' not described in the HAL configuration file";
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + const CameraProps &cam = it->second;
> > > > > + if (cam.location.empty()) {
> > > > > + LOG(HALConfig, Error) << "Location for camera '" << camera
> > > > > + << "' not available";
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + if (cam.location == "front")
> > > > > + return CAMERA_FACING_FRONT;
> > > > > + else if (cam.location == "back")
> > > > > + return CAMERA_FACING_BACK;
> > > > > + else if (cam.location == "external")
> > > > > + return CAMERA_FACING_EXTERNAL;
> > > > > +
> > > > > + LOG(HALConfig, Error) << "Unsupported camera location " << cam.location;
> > > > > + return -EINVAL;
> > > >
> > > > Can we do this conversion when parsing the configuration file instead ?
> > > > That will ensure at parse time that the data is valid, instead of
> > > > delaying the check to later. The rotation is already handled that way,
> > > > and you already check that the location string is valid in
> > > > CameraHalConfig::parseCameraProps().
> > > >
> > > > > +}
> > > > > +
> > > > > +unsigned int CameraHalConfig::cameraRotation(const std::string &camera) const
> > > > > +{
> > > > > + 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 CameraProps &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 CameraProps &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);
> > > >
> > > > Does the token need to be deleted in the error cases above and below ?
> > > >
> > > > > +
> > > > > + 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_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::parseCameraProps(yaml_token_t &token,
> > > > > + const std::string &cameraID)
> > > > > +{
> > > > > + 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);
> > > > > +
> > > > > + 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);
> > > > > +
> > > > > + /*
> > > > > + * Parse the camera properties and store them in a cameraBlock instance.
> > > > > + *
> > > > > + * Add a safety counter to make sure we don't loop indefinitely in case
> > > > > + * the configuration file is malformed.
> > > > > + */
> > > > > + unsigned int sentinel = 100;
> > > > > + CameraProps cameraBlock{};
> > > > > + bool blockEnd = false;
> > > > > + 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(token);
> > > > > + std::string value = parseValue(token);
> > > > > + if (key.empty() || value.empty()) {
> > > > > + LOG(HALConfig, Error)
> > > > > + << "Configuration file is not valid";
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + 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 == "model") {
> > > > > + cameraBlock.model = value;
> > > > > + } else {
> > > > > + LOG(HALConfig, Error)
> > > > > + << "Configuration file is not valid "
> > > > > + << "unknown key: " << key;
> > > > > + return -EINVAL;
> > > > > + }
> > > > > + break;
> > > > > + }
> > > > > + case YAML_BLOCK_END_TOKEN:
> > > > > + yaml_token_delete(&token);
> > > > > + blockEnd = true;
> > > > > + break;
> > > > > + default:
> > > > > + yaml_token_delete(&token);
> > > > > + break;
> > > > > + }
> > > > > +
> > > > > + --sentinel;
> > > > > + } while (!blockEnd && sentinel);
> > > >
> > > > Missing blank line.
> > > >
> > > > > + if (!sentinel) {
> > > > > + LOG(HALConfig, Error) << "Configuration file is not valid ";
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + cameras_[cameraID] = cameraBlock;
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +int CameraHalConfig::parseCameras(yaml_token_t &token)
> > > > > +{
> > > > > + int ret;
> > > > > +
> > > > > + /* The 'cameras' key maps a BLOCK_MAPPING_START block. */
> > > > > + 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);
> > > > > +
> > > > > + 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);
> > > > > +
> > > > > + /*
> > > > > + * 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;
> > > > > + 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(token);
> > > > > + if (cameraId.empty()) {
> > > > > + LOG(HALConfig, Error)
> > > > > + << "Configuration file is not valid";
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + ret = parseCameraProps(token, cameraId);
> > > > > + if (ret) {
> > > > > + LOG(HALConfig, Error)
> > > > > + << "Configuration file is not valid";
> > > > > + return -EINVAL;
> > > > > + }
> > > > > + break;
> > > > > + }
> > > > > + case YAML_BLOCK_END_TOKEN:
> > > > > + yaml_token_delete(&token);
> > > > > + blockEnd = true;
> > > > > + break;
> > > > > + default:
> > > > > + yaml_token_delete(&token);
> > > > > + LOG(HALConfig, Error)
> > > > > + << "Configuration file is not valid";
> > > > > + return -EINVAL;
> > > > > + }
> > > > > + } while (!blockEnd);
> > > > > +
> > > > > + return 0;
> > > > > +}
> > > > > +
> > > > > +int CameraHalConfig::parseEntry(yaml_token_t &token)
> > > > > +{
> > > > > + int ret;
> > > > > +
> > > > > + /*
> > > > > + * Parse each key we find in the file.
> > > > > + *
> > > > > + * Keys like 'model' and 'maker' are device properties and gets
> > > > > + * stored as class members.
> > > > > + *
> > > > > + * The 'cameras' keys maps to a list of (lists) of camera properties.
> > > > > + */
> > > > > +
> > > > > + std::string key = parseKey(token);
> > > > > + if (key.empty()) {
> > > > > + LOG(HALConfig, Error) << "Configuration file is not valid";
> > > > > + return -EINVAL;
> > > > > + }
> > > > > +
> > > > > + if (key == "cameras") {
> > > > > + ret = parseCameras(token);
> > > > > + if (ret)
> > > > > + return ret;
> > > > > + } else if (key == "manufacturer") {
> > > > > + maker_ = parseValue(token);
> > > > > + if (maker_.empty()) {
> > > > > + LOG(HALConfig, Error) << "Configuration file is not valid";
> > > > > + return -EINVAL;
> > > > > + }
> > > > > + } else if (key == "model") {
> > > > > + model_ = parseValue(token);
> > > > > + if (model_.empty()) {
> > > > > + LOG(HALConfig, Error) << "Configuration file is not valid";
> > > > > + return -EINVAL;
> > > > > + }
> > > > > + } else {
> > > > > + 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);
> > > > > +
> > > > > + 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);
> > > > > +
> > > > > + /* 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(token);
> > > >
> > > > Do you need to pass the token to parseEntry(), given that you've just
> > > > deleted it ? It looks to me like the token is only the return value of
> > > > yaml_parse_scan(), and can thus be a local variable everywhere.
> > > >
> > > > > + break;
> > > > > + case YAML_STREAM_END_TOKEN:
> > > > > + /* Resources are released after the loop exit. */
> > > > > + break;
> > > > > + default:
> > > > > + yaml_token_delete(&token);
> > > > > + break;
> > > >
> > > > The YAML_STREAM_END_TOKEN and default case are different, but the
> > > > while() condition below accesses token.type. Is that intended ?
> > > >
> > > > > + }
> > > > > + } 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..ce77b1ee1582
> > > > > --- /dev/null
> > > > > +++ b/src/android/camera_hal_config.h
> > > > > @@ -0,0 +1,53 @@
> > > > > +/* 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>
> > > >
> > > > As commented in v1, hiding the parser would be nice, but it can be done
> > > > later, on top.
> > > >
> > > > > +
> > > > > +class CameraHalConfig
> > > > > +{
> > > > > +public:
> > > > > + CameraHalConfig();
> > > > > + ~CameraHalConfig();
> > > > > +
> > > > > + int open();
> > > > > +
> > > > > + const std::string &deviceModel() const { return model_; }
> > > > > + const std::string &deviceMaker() const { return maker_; }
> > > > > +
> > > > > + int 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 CameraProps
> > > > > + {
> > > > > + public:
> > > > > + std::string location;
> > > > > + std::string model;
> > > > > + unsigned int rotation;
> > > > > + };
> > > > > + std::map<std::string, CameraProps> cameras_;
> > > > > +
> > > > > + std::string findFilePath(const std::string &filename) const;
> > > > > + FILE *openFile(const std::string &filename);
> > > > > + std::string parseValue(yaml_token_t &token);
> > > > > + std::string parseKey(yaml_token_t &token);
> > > > > + int parseCameraProps(yaml_token_t &token, const std::string &cameraID);
> > > > > + int parseCameras(yaml_token_t &token);
> > > > > + int parseEntry(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 8e7d07d9be3c..c0ede407bc38 100644
> > > > > --- a/src/android/meson.build
> > > > > +++ b/src/android/meson.build
> > > > > @@ -3,6 +3,7 @@
> > > > > android_deps = [
> > > > > dependency('libexif', required : get_option('android')),
> > > > > dependency('libjpeg', required : get_option('android')),
> > > > > + dependency('yaml-0.1', required : get_option('android')),
> > > > > ]
> > > > >
> > > > > android_enabled = true
> > > > > @@ -41,6 +42,7 @@ android_hal_sources = files([
> > > > > 'camera3_hal.cpp',
> > > > > 'camera_hal_manager.cpp',
> > > > > 'camera_device.cpp',
> > > > > + 'camera_hal_config.cpp',
> > > >
> > > > While at it, could you move camera_hal_manager to the right place ?
> > > >
> > > > > 'camera_metadata.cpp',
> > > > > 'camera_ops.cpp',
> > > > > 'camera_stream.cpp',
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list