[PATCH v11 5/7] libcamera: virtual: Read config and register cameras based on the config
Cheng-Hao Yang
chenghaoyang at chromium.org
Tue Sep 10 06:50:15 CEST 2024
Hi Jacopo,
On Mon, Sep 9, 2024 at 5:28 PM Jacopo Mondi <jacopo.mondi at ideasonboard.com>
wrote:
> Hi
>
> On Sat, Sep 07, 2024 at 02:28:30PM GMT, Harvey Yang wrote:
> > From: Konami Shu <konamiz at google.com>
> >
> > This patch introduces the configuration file for Virtual Pipeline
> > Handler. The config file is written in yaml, and the format is
> > documented in `README.md`.
> >
> > The config file will define the camera with IDs, supported formats and
> > image sources, etc. Only Test Patterns are supported now, while the next
> > patches will add generators to read from real images.
> >
> > Signed-off-by: Konami Shu <konamiz at google.com>
> > Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> > Co-developed-by: Yunke Cao <yunkec at chromium.org>
> > Co-developed-by: Tomasz Figa <tfiga at chromium.org>
> > ---
> > src/libcamera/pipeline/virtual/README.md | 43 ++++
> > .../pipeline/virtual/data/virtual.yaml | 36 ++++
> > .../pipeline/virtual/frame_generator.h | 3 +-
> > src/libcamera/pipeline/virtual/meson.build | 1 +
> > src/libcamera/pipeline/virtual/parser.cpp | 197 ++++++++++++++++++
> > src/libcamera/pipeline/virtual/parser.h | 42 ++++
> > src/libcamera/pipeline/virtual/virtual.cpp | 56 ++---
> > src/libcamera/pipeline/virtual/virtual.h | 1 +
> > 8 files changed, 350 insertions(+), 29 deletions(-)
> > create mode 100644 src/libcamera/pipeline/virtual/README.md
> > create mode 100644 src/libcamera/pipeline/virtual/data/virtual.yaml
> > create mode 100644 src/libcamera/pipeline/virtual/parser.cpp
> > create mode 100644 src/libcamera/pipeline/virtual/parser.h
> >
> > diff --git a/src/libcamera/pipeline/virtual/README.md
> b/src/libcamera/pipeline/virtual/README.md
> > new file mode 100644
> > index 00000000..ef80bb48
> > --- /dev/null
> > +++ b/src/libcamera/pipeline/virtual/README.md
> > @@ -0,0 +1,43 @@
> > +# Virtual Pipeline Handler
> > +
> > +Virtual pipeline handler emulates fake external camera(s) on ChromeOS
> for testing.
> > +
> > +## Parse config file and register cameras
> > +
> > +- The config file is located at
> `src/libcamera/pipeline/virtual/data/virtual.yaml`
> > +
> > +### Config File Format
> > +The config file contains the information about cameras' properties to
> register.
> > +The config file should be a yaml file with dictionary of the cameraIds
> > +associated with their properties as top level. The default value will
> be applied when any property is empty.
> > +
> > +Each camera block is a dictionary, containing the following keys:
> > +- `supported_formats` (list of `VirtualCameraData::Resolution`,
> optional) : List of supported resolution and frame rates of the emulated
> camera
> > + - `width` (`unsigned int`, default=1920): Width of the window
> resolution. This needs to be even.
> > + - `height` (`unsigned int`, default=1080): Height of the window
> resolution.
> > + - `frame_rates` (list of `int`, default=`[30,60]` ): Range of the
> frame rate (per second). If the list contains one value, it's the lower
> bound and the upper bound. If the list contains two values, the first is
> the lower bound and the second is the upper bound. No other number of
> values is allowed.
> > +- `test_pattern` (`string`): Which test pattern to use as frames. The
> options are "bars", "lines".
> > +- `location` (`string`, default="front"): The location of the camera.
> Support "front" and "back". This is displayed in qcam camera selection
> window but this does not change the output.
> > +- `model` (`string`, default="Unknown"): The model name of the camera.
> This is displayed in qcam camera selection window but this does not change
> the output.
> > +
> > +Check `data/virtual.yaml` as the sample config file.
> > +
> > +### Implementation
> > +
> > +`Parser` class provides methods to parse the config file to register
> cameras
> > +in Virtual Pipeline Handler. `parseConfigFile()` is exposed to use in
> > +Virtual Pipeline Handler.
> > +
> > +This is the procedure of the Parser class:
> > +1. `parseConfigFile()` parses the config file to `YamlObject` using
> `YamlParser::parse()`.
> > + - Parse the top level of config file which are the camera ids and
> look into each camera properties.
> > +2. For each camera, `parseCameraConfigData()` returns a camera with the
> configuration.
> > + - The methods in the next step fill the data with the pointer to
> the Camera object.
> > + - If the config file contains invalid configuration, this method
> returns nullptr. The camera will be skipped.
> > +3. Parse each property and register the data.
> > + - `parseSupportedFormats()`: Parses `supported_formats` in the
> config, which contains resolutions and frame rates.
> > + - `parseTestPattern()`: Parses `test_pattern` in the config.
> > + - `parseLocation()`: Parses `location` in the config.
> > + - `parseModel()`: Parses `model` in the config.
> > +4. Back to `parseConfigFile()` and append the camera configuration.
> > +5. Returns a list of camera configurations.
> > diff --git a/src/libcamera/pipeline/virtual/data/virtual.yaml
> b/src/libcamera/pipeline/virtual/data/virtual.yaml
> > new file mode 100644
> > index 00000000..6b73ddf2
> > --- /dev/null
> > +++ b/src/libcamera/pipeline/virtual/data/virtual.yaml
> > @@ -0,0 +1,36 @@
> > +# SPDX-License-Identifier: CC0-1.0
> > +%YAML 1.1
> > +---
> > +"Virtual0":
> > + supported_formats:
> > + - width: 1920
> > + height: 1080
> > + frame_rates:
> > + - 30
> > + - 60
> > + - width: 1680
> > + height: 1050
> > + frame_rates:
> > + - 70
> > + - 80
> > + test_pattern: "lines"
> > + location: "front"
> > + model: "Virtual Video Device"
> > +"Virtual1":
> > + supported_formats:
> > + - width: 800
> > + height: 600
> > + frame_rates:
> > + - 60
> > + test_pattern: "bars"
> > + location: "back"
> > + model: "Virtual Video Device1"
> > +"Virtual2":
> > + supported_formats:
> > + - width: 400
> > + height: 300
> > + test_pattern: "lines"
> > + location: "front"
> > + model: "Virtual Video Device2"
> > +"Virtual3":
> > + test_pattern: "bars"
> > diff --git a/src/libcamera/pipeline/virtual/frame_generator.h
> b/src/libcamera/pipeline/virtual/frame_generator.h
> > index d8727b8f..f68a795f 100644
> > --- a/src/libcamera/pipeline/virtual/frame_generator.h
> > +++ b/src/libcamera/pipeline/virtual/frame_generator.h
> > @@ -19,8 +19,7 @@ public:
> >
> > virtual void configure(const Size &size) = 0;
> >
> > - virtual void generateFrame(const Size &size,
> > - const FrameBuffer *buffer) = 0;
> > + virtual void generateFrame(const Size &size, const FrameBuffer
> *buffer) = 0;
>
> Why ?
>
Reverted.
>
> >
> > protected:
> > FrameGenerator() {}
> > diff --git a/src/libcamera/pipeline/virtual/meson.build
> b/src/libcamera/pipeline/virtual/meson.build
> > index 0c537777..d72ac5be 100644
> > --- a/src/libcamera/pipeline/virtual/meson.build
> > +++ b/src/libcamera/pipeline/virtual/meson.build
> > @@ -1,6 +1,7 @@
> > # SPDX-License-Identifier: CC0-1.0
> >
> > libcamera_internal_sources += files([
> > + 'parser.cpp',
> > 'test_pattern_generator.cpp',
> > 'virtual.cpp',
> > ])
> > diff --git a/src/libcamera/pipeline/virtual/parser.cpp
> b/src/libcamera/pipeline/virtual/parser.cpp
> > new file mode 100644
> > index 00000000..d861a52a
> > --- /dev/null
> > +++ b/src/libcamera/pipeline/virtual/parser.cpp
> > @@ -0,0 +1,197 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2024, Google Inc.
> > + *
> > + * parser.cpp - Virtual cameras helper to parse config file
> > + */
> > +
> > +#include "parser.h"
> > +
> > +#include <memory>
> > +#include <utility>
> > +
> > +#include <libcamera/base/log.h>
> > +
> > +#include <libcamera/control_ids.h>
> > +#include <libcamera/property_ids.h>
> > +
> > +#include "libcamera/internal/pipeline_handler.h"
> > +#include "libcamera/internal/yaml_parser.h"
> > +
> > +#include "virtual.h"
> > +
> > +namespace libcamera {
> > +
> > +LOG_DECLARE_CATEGORY(Virtual)
> > +
> > +std::vector<std::unique_ptr<VirtualCameraData>>
> > +Parser::parseConfigFile(File &file, PipelineHandler *pipe)
> > +{
> > + std::vector<std::unique_ptr<VirtualCameraData>> configurations;
> > +
> > + std::unique_ptr<YamlObject> cameras = YamlParser::parse(file);
> > + if (!cameras) {
> > + LOG(Virtual, Error) << "Failed to pass config file.";
> > + return configurations;
> > + }
> > +
> > + if (!cameras->isDictionary()) {
> > + LOG(Virtual, Error) << "Config file is not a dictionary at
> the top level.";
> > + return configurations;
> > + }
> > +
> > + /* Look into the configuration of each camera */
> > + for (const auto &[cameraId, cameraConfigData] : cameras->asDict())
> {
> > + std::unique_ptr<VirtualCameraData> data =
> > + parseCameraConfigData(cameraConfigData, pipe);
> > + /* Parse configData to data*/
> > + if (!data) {
> > + /* Skip the camera if it has invalid config */
> > + LOG(Virtual, Error) << "Failed to parse config of
> the camera: "
> > + << cameraId;
> > + continue;
> > + }
> > +
> > + data->id_ = cameraId;
> > + ControlInfoMap::Map controls;
> > + /* todo: Check which resolution's frame rate to be
> reported */
> > + controls[&controls::FrameDurationLimits] =
> > + ControlInfo(int64_t(1000000 /
> data->supportedResolutions_[0].frameRates[1]),
> > + int64_t(1000000 /
> data->supportedResolutions_[0].frameRates[0]));
>
> Is frameRates[1] first and frameRates[0] second correct ? Isn't the
> the first value the minimum one ?
>
Minimum FrameDuration(Limit) implies maximum FPS, as basically
`FrameDuration == 1/FPS` :)
>
> > + data->controlInfo_ = ControlInfoMap(std::move(controls),
> controls::controls);
> > + configurations.push_back(std::move(data));
> > + }
>
> Empty line
>
>
Done.
> > + return configurations;
> > +}
> > +
> > +std::unique_ptr<VirtualCameraData>
> > +Parser::parseCameraConfigData(const YamlObject &cameraConfigData,
> > + PipelineHandler *pipe)
> > +{
> > + std::vector<VirtualCameraData::Resolution> resolutions;
> > + if (parseSupportedFormats(cameraConfigData, &resolutions))
> > + return nullptr;
> > +
> > + std::unique_ptr<VirtualCameraData> data =
> > + std::make_unique<VirtualCameraData>(pipe, resolutions);
> > +
> > + if (parseTestPattern(cameraConfigData, data.get()))
> > + return nullptr;
> > +
> > + if (parseLocation(cameraConfigData, data.get()))
> > + return nullptr;
> > +
> > + if (parseModel(cameraConfigData, data.get()))
> > + return nullptr;
> > +
> > + return data;
> > +}
> > +
> > +int Parser::parseSupportedFormats(const YamlObject &cameraConfigData,
> > +
> std::vector<VirtualCameraData::Resolution> *resolutions)
> > +{
> > + if (cameraConfigData.contains("supported_formats")) {
> > + const YamlObject &supportedResolutions =
> cameraConfigData["supported_formats"];
> > +
> > + for (const YamlObject &supportedResolution :
> supportedResolutions.asList()) {
> > + unsigned int width =
> supportedResolution["width"].get<unsigned int>(1920);
> > + unsigned int height =
> supportedResolution["height"].get<unsigned int>(1080);
> > + if (width == 0 || height == 0) {
> > + LOG(Virtual, Error) << "Invalid width
> or/and height";
> > + return -EINVAL;
> > + }
> > + if (width % 2 != 0) {
> > + LOG(Virtual, Error) << "Invalid width:
> width needs to be even";
> > + return -EINVAL;
> > + }
> > +
> > + std::vector<int> frameRates;
> > + if (supportedResolution.contains("frame_rates")) {
> > + auto frameRatesList =
> > +
> supportedResolution["frame_rates"].getList<int>();
> > + if (!frameRatesList ||
> (frameRatesList->size() != 1 &&
> > +
> frameRatesList->size() != 2)) {
> > + LOG(Virtual, Error) << "Invalid
> frame_rates: either one or two values";
> > + return -EINVAL;
> > + }
> > +
> > + if (frameRatesList->size() == 2 &&
> > + frameRatesList.value()[0] >
> frameRatesList.value()[1]) {
> > + LOG(Virtual, Error) <<
> "frame_rates's first value(lower bound)"
> > + << " is higher
> than the second value(upper bound)";
> > + return -EINVAL;
> > + }
> > +
> frameRates.push_back(frameRatesList.value()[0]);
> > + if (frameRatesList->size() == 2)
> > +
> frameRates.push_back(frameRatesList.value()[1]);
> > + else
> > +
> frameRates.push_back(frameRatesList.value()[0]);
> > + } else {
> > + frameRates.push_back(30);
> > + frameRates.push_back(60);
> > + }
> > +
> > + resolutions->emplace_back(
> > + VirtualCameraData::Resolution{ Size{
> width, height },
> > + frameRates
> });
> > + }
> > + } else {
> > + resolutions->emplace_back(
> > + VirtualCameraData::Resolution{ Size{ 1920, 1080 },
> > + { 30, 60 } });
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int Parser::parseTestPattern(const YamlObject &cameraConfigData,
> VirtualCameraData *data)
> > +{
> > + std::string testPattern =
> cameraConfigData["test_pattern"].get<std::string>("");
> > +
> > + /* Default value is "bars" */
> > + if (testPattern == "bars") {
> > + data->testPattern_ = TestPattern::ColorBars;
> > + } else if (testPattern == "lines") {
> > + data->testPattern_ = TestPattern::DiagonalLines;
> > + } else {
> > + LOG(Virtual, Error) << "Test pattern: " << testPattern
> > + << "is not supported";
> > + return -EINVAL;
> > + }
>
> empty line like you have in the other functions
>
>
Done.
> > + return 0;
> > +}
> > +
> > +int Parser::parseLocation(const YamlObject &cameraConfigData,
> VirtualCameraData *data)
> > +{
> > + std::string location =
> cameraConfigData["location"].get<std::string>("");
> > +
> > + /* Default value is properties::CameraLocationFront */
> > + if (location == "front" || location == "") {
> > + data->properties_.set(properties::Location,
> > + properties::CameraLocationFront);
> > + } else if (location == "back") {
> > + data->properties_.set(properties::Location,
> > + properties::CameraLocationBack);
> > + } else {
> > + LOG(Virtual, Error) << "location: " << location
> > + << " is not supported";
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +int Parser::parseModel(const YamlObject &cameraConfigData,
> VirtualCameraData *data)
> > +{
> > + std::string model = cameraConfigData["model"].get<std::string>("");
> > +
> > + /* Default value is "Unknown" */
> > + if (model == "")
> > + data->properties_.set(properties::Model, "Unknown");
> > + else
> > + data->properties_.set(properties::Model, model);
> > +
> > + return 0;
> > +}
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/pipeline/virtual/parser.h
> b/src/libcamera/pipeline/virtual/parser.h
> > new file mode 100644
> > index 00000000..09c3c56b
> > --- /dev/null
> > +++ b/src/libcamera/pipeline/virtual/parser.h
> > @@ -0,0 +1,42 @@
> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> > +/*
> > + * Copyright (C) 2024, Google Inc.
> > + *
> > + * parser.h - Virtual cameras helper to parse config file
> > + */
> > +
> > +#pragma once
> > +
> > +#include <memory>
> > +#include <vector>
> > +
> > +#include <libcamera/base/file.h>
> > +
> > +#include "libcamera/internal/pipeline_handler.h"
> > +#include "libcamera/internal/yaml_parser.h"
> > +
> > +#include "virtual.h"
> > +
> > +namespace libcamera {
> > +
> > +class Parser
> > +{
> > +public:
> > + Parser() {}
> > + ~Parser() = default;
>
> You can drop these
>
>
Done.
> > +
> > + std::vector<std::unique_ptr<VirtualCameraData>>
> > + parseConfigFile(File &file, PipelineHandler *pipe);
> > +
> > +private:
> > + std::unique_ptr<VirtualCameraData>
> > + parseCameraConfigData(const YamlObject &cameraConfigData,
> PipelineHandler *pipe);
> > +
> > + int parseSupportedFormats(const YamlObject &cameraConfigData,
> > +
> std::vector<VirtualCameraData::Resolution> *resolutions);
> > + int parseTestPattern(const YamlObject &cameraConfigData,
> VirtualCameraData *data);
> > + int parseLocation(const YamlObject &cameraConfigData,
> VirtualCameraData *data);
> > + int parseModel(const YamlObject &cameraConfigData,
> VirtualCameraData *data);
> > +};
> > +
> > +} /* namespace libcamera */
> > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp
> b/src/libcamera/pipeline/virtual/virtual.cpp
> > index 6e64aeee..55bc30df 100644
> > --- a/src/libcamera/pipeline/virtual/virtual.cpp
> > +++ b/src/libcamera/pipeline/virtual/virtual.cpp
> > @@ -24,6 +24,9 @@
> > #include "libcamera/internal/camera.h"
> > #include "libcamera/internal/formats.h"
> > #include "libcamera/internal/pipeline_handler.h"
> > +#include "libcamera/internal/yaml_parser.h"
> > +
> > +#include "parser.h"
> >
> > namespace libcamera {
> >
> > @@ -55,6 +58,9 @@ VirtualCameraData::VirtualCameraData(PipelineHandler
> *pipe,
> > maxResolutionSize_ = std::max(maxResolutionSize_,
> resolution.size);
> > }
> >
> > + properties_.set(properties::PixelArrayActiveAreas,
> > + { Rectangle(maxResolutionSize_) });
> > +
> > /* \todo Support multiple streams and pass multi_stream_test */
> > streamConfigs_.resize(kMaxStream);
> > }
> > @@ -248,37 +254,33 @@ bool
> PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator
> >
> > created_ = true;
> >
> > - /* \todo Add virtual cameras according to a config file. */
> > -
> > - std::vector<VirtualCameraData::Resolution> supportedResolutions;
> > - supportedResolutions.resize(2);
> > - supportedResolutions[0] = { .size = Size(1920, 1080), .frameRates
> = { 30 } };
> > - supportedResolutions[1] = { .size = Size(1280, 720), .frameRates =
> { 30 } };
> > -
> > - std::unique_ptr<VirtualCameraData> data =
> > - std::make_unique<VirtualCameraData>(this,
> supportedResolutions);
> > -
> > - data->properties_.set(properties::Location,
> properties::CameraLocationFront);
> > - data->properties_.set(properties::Model, "Virtual Video Device");
> > - data->properties_.set(properties::PixelArrayActiveAreas, {
> Rectangle(Size(1920, 1080)) });
> > -
> > - /* \todo Set FrameDurationLimits based on config. */
> > - ControlInfoMap::Map controls;
> > - int64_t min_frame_duration = 33333, max_frame_duration = 33333;
> > - controls[&controls::FrameDurationLimits] =
> ControlInfo(min_frame_duration, max_frame_duration);
> > - data->controlInfo_ = ControlInfoMap(std::move(controls),
> controls::controls);
> > + File file(configurationFile("virtual", "virtual.yaml"));
> > + bool isOpen = file.open(File::OpenModeFlag::ReadOnly);
> > + if (!isOpen) {
> > + LOG(Virtual, Error) << "Failed to open config file: " <<
> file.fileName();
> > + return false;
> > + }
> >
> > - /* Create and register the camera. */
> > - std::set<Stream *> streams;
> > - for (auto &streamConfig : data->streamConfigs_)
> > - streams.insert(&streamConfig.stream);
> > + Parser parser;
> > + auto configData = parser.parseConfigFile(file, this);
> > + if (configData.size() == 0) {
> > + LOG(Virtual, Error) << "Failed to parse any cameras from
> the config file: "
> > + << file.fileName();
> > + return false;
> > + }
> >
> > - const std::string id = "Virtual0";
> > - std::shared_ptr<Camera> camera = Camera::create(std::move(data),
> id, streams);
> > + /* Configure and register cameras with configData */
> > + for (auto &data : configData) {
> > + std::set<Stream *> streams;
> > + for (auto &streamConfig : data->streamConfigs_)
> > + streams.insert(&streamConfig.stream);
> > + std::string id = data->id_;
> > + std::shared_ptr<Camera> camera =
> Camera::create(std::move(data), id, streams);
> >
> > - initFrameGenerator(camera.get());
> > + initFrameGenerator(camera.get());
> >
> > - registerCamera(std::move(camera));
> > + registerCamera(std::move(camera));
> > + }
> >
> > return true;
> > }
> > diff --git a/src/libcamera/pipeline/virtual/virtual.h
> b/src/libcamera/pipeline/virtual/virtual.h
> > index 09d73051..8830e00f 100644
> > --- a/src/libcamera/pipeline/virtual/virtual.h
> > +++ b/src/libcamera/pipeline/virtual/virtual.h
> > @@ -39,6 +39,7 @@ public:
> >
> > ~VirtualCameraData() = default;
> >
> > + std::string id_;
>
> Mostly minors
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
>
As I reordered 5th and 6th patches, and this patch is appended,
I kept your review tag on hold :)
>
> > TestPattern testPattern_ = TestPattern::ColorBars;
> >
> > const std::vector<Resolution> supportedResolutions_;
> > --
> > 2.46.0.469.g59c65b2a67-goog
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20240910/0eb53287/attachment.htm>
More information about the libcamera-devel
mailing list