[libcamera-devel] [PATCH 7/9] libcamera: ipu3: Add helper class PipeConfigPreference
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Wed Apr 6 03:47:27 CEST 2022
Hi Han-Lin,
Thank you for the patch.
On Wed, Feb 09, 2022 at 03:19:15PM +0800, Han-Lin Chen wrote:
> Add helper class PipeConfigPreference to load the caliberated ipu3
> pipeline configuration files, and provides query interface for the
> pipeline handler.
>
> Signed-off-by: Han-Lin Chen <hanlinchen at chromium.org>
> ---
> include/libcamera/geometry.h | 4 +
> src/libcamera/geometry.cpp | 20 ++
Could you please split the changes to geometry.h and geometry.cpp to a
separate patch ?
> src/libcamera/pipeline/ipu3/meson.build | 1 +
> .../pipeline/ipu3/pipe_config_pref.cpp | 285 ++++++++++++++++++
> .../pipeline/ipu3/pipe_config_pref.h | 80 +++++
> 5 files changed, 390 insertions(+)
> create mode 100644 src/libcamera/pipeline/ipu3/pipe_config_pref.cpp
> create mode 100644 src/libcamera/pipeline/ipu3/pipe_config_pref.h
>
> diff --git a/include/libcamera/geometry.h b/include/libcamera/geometry.h
> index 7838b679..ede54981 100644
> --- a/include/libcamera/geometry.h
> +++ b/include/libcamera/geometry.h
> @@ -46,6 +46,8 @@ static inline bool operator!=(const Point &lhs, const Point &rhs)
> return !(lhs == rhs);
> }
>
> +std::ostream &operator<<(std::ostream &out, const Point &d);
> +
> class Size
> {
> public:
> @@ -192,6 +194,8 @@ static inline bool operator>=(const Size &lhs, const Size &rhs)
> return !(lhs < rhs);
> }
>
> +std::ostream &operator<<(std::ostream &out, const Size &s);
> +
> class SizeRange
> {
> public:
> diff --git a/src/libcamera/geometry.cpp b/src/libcamera/geometry.cpp
> index cb3c2de1..a65f9f2f 100644
> --- a/src/libcamera/geometry.cpp
> +++ b/src/libcamera/geometry.cpp
> @@ -83,6 +83,16 @@ bool operator==(const Point &lhs, const Point &rhs)
> * \return True if the two points are not equal, false otherwise
> */
>
> +/**
> + * \brief Insert operation for Point with ostream
> + * \return The input std::ostream
> + */
> +std::ostream &operator<<(std::ostream &out, const Point &p)
> +{
> + out << "(" << p.x << ", " << p.y << ")";
This doesn't match the format used by Point::toString(), which can be
confusing.
I'm actually wondering if we could use toString() instead of adding
operator<<() overloads. I half-recall having the same discussion a long
time ago and advocating against operator<<(), but I can't recall why. Or
maybe I don't remember correctly, and operator<<() is fine :-) Does
anyone have an opinion on this ?
> + return out;
> +}
> +
> /**
> * \struct Size
> * \brief Describe a two-dimensional size
> @@ -428,6 +438,16 @@ bool operator<(const Size &lhs, const Size &rhs)
> * \sa bool operator<(const Size &lhs, const Size &rhs)
> */
>
> +/**
> + * \brief Insert operation for Size with ostream
> + * \return The input std::ostream
> + */
> +std::ostream &operator<<(std::ostream &out, const Size &s)
> +{
> + out << s.width << "x" << s.height;
> + return out;
> +}
> +
For completeness, can you add the operators for SizeRange and Rectangle
too ?
I'll review the IPU3 part separately, possibly after updating it to the
next version of the YAML parser class as it will result in some changes.
> /**
> * \struct SizeRange
> * \brief Describe a range of sizes
> diff --git a/src/libcamera/pipeline/ipu3/meson.build b/src/libcamera/pipeline/ipu3/meson.build
> index a1b0b31a..dcc450f0 100644
> --- a/src/libcamera/pipeline/ipu3/meson.build
> +++ b/src/libcamera/pipeline/ipu3/meson.build
> @@ -5,4 +5,5 @@ libcamera_sources += files([
> 'frames.cpp',
> 'imgu.cpp',
> 'ipu3.cpp',
> + 'pipe_config_pref.cpp',
> ])
> diff --git a/src/libcamera/pipeline/ipu3/pipe_config_pref.cpp b/src/libcamera/pipeline/ipu3/pipe_config_pref.cpp
> new file mode 100644
> index 00000000..5b4a17c9
> --- /dev/null
> +++ b/src/libcamera/pipeline/ipu3/pipe_config_pref.cpp
> @@ -0,0 +1,285 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Google Inc.
> + *
> + * pipe_config_pref.cpp - Helper class for IPU3 pipeline config preference
> + */
> +
> +#include "pipe_config_pref.h"
> +
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/internal/yaml_parser.h>
> +
> +namespace libcamera {
> +
> +LOG_DECLARE_CATEGORY(IPU3)
> +
> +namespace {
> +
> +std::ostream &operator<<(std::ostream &out,
> + const PipeConfigPreference::PipeConfig &d)
> +{
> + out << "cio2: " << d.cio2 << " bds: " << d.bds
> + << " gdc: " << d.gdc << " iff: " << d.iff
> + << " main: " << d.main << " viewfinder: " << d.viewfinder;
> + return out;
> +}
> +
> +int loadPipeConfig(const YamlObject &yamlObject,
> + PipeConfigPreference::PipeConfig &pipeConfig)
> +{
> + if (!yamlObject.isMember("bds") ||
> + !yamlObject.isMember("gdc") ||
> + !yamlObject.isMember("iff") ||
> + !yamlObject.isMember("cio2") ||
> + !yamlObject.isMember("main") ||
> + !yamlObject.isMember("viewfinder")) {
> + LOG(IPU3, Error) << "Missing mandatory attributes in a config";
> + return -EINVAL;
> + }
> +
> + pipeConfig.bds = yamlObject.get("bds").asSize();
> + pipeConfig.gdc = yamlObject.get("gdc").asSize();
> + pipeConfig.iff = yamlObject.get("iff").asSize();
> + pipeConfig.cio2 = yamlObject.get("cio2").asSize();
> + pipeConfig.main = yamlObject.get("main").asSize();
> + pipeConfig.viewfinder = yamlObject.get("viewfinder").asSize();
> +
> + return 0;
> +}
> +
> +int loadPipeConfigs(const YamlObject &yamlPipeConfigs,
> + std::vector<PipeConfigPreference::PipeConfig> &pipeConfigs,
> + Size &maxResolution, Size &minResolution)
> +{
> + for (int i = 0; i < yamlPipeConfigs.length(); i++) {
> + const YamlObject &yamlConfig = yamlPipeConfigs[i];
> + pipeConfigs.emplace_back();
> + if (loadPipeConfig(yamlConfig, pipeConfigs.back()))
> + return -EINVAL;
> + }
> +
> + if (pipeConfigs.size() == 0)
> + return -EINVAL;
> +
> + maxResolution = minResolution = pipeConfigs[0].main;
> +
> + for (const PipeConfigPreference::PipeConfig &config : pipeConfigs) {
> + maxResolution.expandTo(config.main);
> + minResolution.boundTo(config.main);
> + }
> +
> + /* Sort configs by the size of the cio2 */
> + sort(pipeConfigs.begin(), pipeConfigs.end(),
> + [](const auto &a, const auto &b) -> bool {
> + return a.cio2 < b.cio2;
> + });
> +
> + return 0;
> +}
> +
> +} /* namespace */
> +
> +PipeConfigPreference::PipeConfigPreference()
> + : valid_(false)
> +{
> +}
> +
> +/**
> + * \struct PipeConfig
> + * \brief Describe a valid ImgU configuration
> + *
> + * The ImgU unit processes images through several components, which have
> + * to be properly configured inspecting the input image size and the desired
> + * output sizes. This structure collects the ImgU configuration for IF, BDS
> + * and GDC, and the requested main output, viewfinder and the input (CIO2)
> + * resolutions.
> + */
> +
> +/**
> + * \brief Parse the configuration file from a path
> + * \param[in] path The path to the configuration file
> + *
> + * Parse the configuration file from a path and set isValid() to true if
> + * success.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int PipeConfigPreference::parsePreferenceFile(const std::string &path)
> +{
> + valid_ = false;
> +
> + FILE *fh = fopen(path.c_str(), "r");
> + if (!fh) {
> + LOG(IPU3, Error) << "Fail to open file: " << path;
> + return -EINVAL;
> + }
> +
> + YamlParser yamlParser;
> + YamlObject yamlObjectPreference;
> +
> + int ret = yamlParser.ParseAsYamlObject(fh, yamlObjectPreference);
> + fclose(fh);
> +
> + if (ret)
> + return -EINVAL;
> +
> + ret = load(yamlObjectPreference);
> +
> + if (ret)
> + return -EINVAL;
> +
> + valid_ = true;
> + return 0;
> +}
> +
> +/**
> + * \brief Query the valid pipeline configuration for video and still pipe
> + * \param[in] videoMain The size of main output from video pipe
> + * \param[in] videoViewfinder The size of viewfinder output from video pipe
> + * \param[in] stillMain The size of main output from still pipe
> + * \param[in] stillViewfinder The size of viewfinder output from still pipe
> + * \param[out] videoResult The ImgU setting for video pipe
> + * \param[out] stillResult The ImgU setting for still pipe
> + *
> + * Helper function to query valid settings for ImgU with the desired
> + * output resolutions. The query interface is based on the assumption
> + * that both video and still ImgU might be used together.
> + * An output can be set disabled if not required. If both main and viewfinder
> + * are set disabled for a ImgU, video or still, the corresponding pipeConfig
> + * undefined. For example, a typical usage is to only one video output is
> + * required, the user may set:
> + *
> + * videoMain = [width, height]
> + * videoViewfinder = Disabled
> + * stillMain = Disabled
> + * stillViewfinder = Disabled
> + *
> + * In the case, only the videoResult would be valid, since still pipe is not
> + * used. When both video and still ImgU are used, their cio2 will have the
> + * same resolution, since they should use the same raw capture.
> + *
> + * \return 0 on success or a negative error code otherwise
> + */
> +int PipeConfigPreference::queryPipeConfig(
> + const Size &videoMain, const Size &videoViewfinder,
> + const Size &stillMain, const Size &stillViewfinder,
> + PipeConfig &videoResult, PipeConfig &stillResult) const
> +{
> + bool hasVideo = (videoMain != Disabled) && (videoMain >= videoViewfinder);
> + bool hasStill = (stillMain != Disabled) && (stillMain >= stillViewfinder);
> +
> + if (!hasStill && !hasVideo)
> + return -EINVAL;
> +
> + std::vector<PipeConfig> validVideoConfigs;
> + std::vector<PipeConfig> validStillConfigs;
> +
> + for (auto &config : videoPipeConfigs_) {
> + if (config.main == videoMain && config.viewfinder == videoViewfinder)
> + validVideoConfigs.emplace_back(config);
> + }
> +
> + for (auto &config : stillPipeConfigs_) {
> + if (config.main == stillMain && config.viewfinder == stillViewfinder)
> + validStillConfigs.emplace_back(config);
> + }
> +
> + /*
> + * Since the configurations are sorted by the size of CIO2, pick
> + * the first valid resolution for lower bandwith.
> + */
> + if (hasVideo && !hasStill) {
> + if (validVideoConfigs.empty())
> + return -EINVAL;
> + videoResult = validVideoConfigs[0];
> + return 0;
> + }
> +
> + if (hasStill && !hasVideo) {
> + if (validStillConfigs.empty())
> + return -EINVAL;
> + stillResult = validStillConfigs[0];
> + return 0;
> + }
> +
> + /* (hasVideo && hasStill) */
> + bool found = false;
> + for (const PipeConfig &videoConfig : validVideoConfigs) {
> + for (const PipeConfig &stillConfig : validVideoConfigs) {
> + if (videoConfig.cio2 == stillConfig.cio2) {
> + found = true;
> + videoResult = videoConfig;
> + stillResult = stillConfig;
> + break;
> + }
> + }
> + }
> +
> + return (found) ? 0 : -EINVAL;
> +}
> +
> +void PipeConfigPreference::dump()
> +{
> + LOG(IPU3, Debug) << "Video Pipe configs: ";
> + for (auto &configs : videoPipeConfigs_) {
> + LOG(IPU3, Debug) << configs;
> + }
> +
> + LOG(IPU3, Debug) << "Still Pipe configs: ";
> + for (auto &configs : stillPipeConfigs_) {
> + LOG(IPU3, Debug) << configs;
> + }
> +}
> +
> +int PipeConfigPreference::load(const YamlObject &configs)
> +{
> + /*
> + * Load the pipeline configure file properties.
> + *
> + * Each valid configuration is a list of properties associated
> + * with the corresponding IMGU settings and grouped into still
> + * and video modes. For each configuration, the main output should
> + * be valid, and the viewfinder is optional. If the viewfinder is
> + * disabled, its width and height should be set to [0, 0];
> + *
> + * still_mode:
> + * - bds: [width, height]
> + * cio2: [width, height]
> + * gdc: [width, height]
> + * iff: [width, height]
> + * main: [width, height]
> + * viewfinder: [0, 0]
> + * ...
> + *
> + * video_mode:
> + * - bds: [width, height]
> + * cio2: [width, height]
> + * gdc: [width, height]
> + * iff: [width, height]
> + * main: [width, height]
> + * viewfinder: [0, 0]
> + * ...
> + */
> +
> + if (!configs.isMember("video_mode") || !configs.isMember("still_mode"))
> + return -EINVAL;
> +
> + videoPipeConfigs_.clear();
> + stillPipeConfigs_.clear();
> +
> + int ret = loadPipeConfigs(configs.get("video_mode"), videoPipeConfigs_,
> + maxVideoResolution_, minVideoResolution_);
> + if (ret)
> + return -EINVAL;
> +
> + ret = loadPipeConfigs(configs.get("still_mode"), stillPipeConfigs_,
> + maxStillResolution_, minStillResolution_);
> + if (ret)
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/ipu3/pipe_config_pref.h b/src/libcamera/pipeline/ipu3/pipe_config_pref.h
> new file mode 100644
> index 00000000..08626d4e
> --- /dev/null
> +++ b/src/libcamera/pipeline/ipu3/pipe_config_pref.h
> @@ -0,0 +1,80 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2022, Google Inc.
> + *
> + * pipe_config_pref.h - Helper class for IPU3 pipeline config preference
> + */
> +
> +#pragma once
> +
> +#include <vector>
> +
> +#include <libcamera/geometry.h>
> +
> +namespace libcamera {
> +
> +class YamlObject;
> +
> +class PipeConfigPreference final
> +{
> +public:
> + constexpr static Size Disabled = Size(0, 0);
> +
> + struct PipeConfig {
> + Size cio2;
> + Size bds;
> + Size gdc;
> + Size iff;
> + Size main;
> + Size viewfinder;
> + };
> +
> + PipeConfigPreference();
> +
> + int parsePreferenceFile(const std::string &path);
> + bool isValid() const
> + {
> + return valid_;
> + }
> + bool invalid()
> + {
> + return valid_ = false;
> + }
> + Size maxVideoResolution()
> + {
> + return maxVideoResolution_;
> + }
> + Size maxStillResolution()
> + {
> + return maxStillResolution_;
> + }
> + Size minVideoResolution()
> + {
> + return minVideoResolution_;
> + }
> + Size minStillResolution()
> + {
> + return minStillResolution_;
> + }
> +
> + int queryPipeConfig(const Size &videoMain, const Size &videoViewfinder,
> + const Size &stillMain, const Size &stillViewfinder,
> + PipeConfig &videoPipeConfig,
> + PipeConfig &stillPipeConfig) const;
> + void dump();
> +
> +private:
> + int load(const YamlObject &object);
> + bool valid_;
> +
> + std::vector<PipeConfig> videoPipeConfigs_;
> + std::vector<PipeConfig> stillPipeConfigs_;
> +
> + Size maxVideoResolution_;
> + Size maxStillResolution_;
> +
> + Size minVideoResolution_;
> + Size minStillResolution_;
> +};
> +
> +} /* namespace libcamera */
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list