| Message ID | 20240820172202.526547-6-chenghaoyang@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hello On Tue, Aug 20, 2024 at 04:23:36PM GMT, Harvey Yang wrote: > From: Konami Shu <konamiz@google.com> > > - Use Yaml::Parser to parse the config > - Add config file at "virtual/data/virtual.yaml" > - README.md contains the documentation for the format of config file and > the implementation of Parser class. > - Add Parser class to parse config file in Virtual > pipeline handler > - Add header file of virtual.cpp to use VirtualCameraData class in > Parser class > - Parse test patterns > - Raise Error when the width of a resolution is an odd number Please make a commit message out of this list > > Signed-off-by: Konami Shu <konamiz@google.com> > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org> > Co-developed-by: Yunke Cao <yunkec@chromium.org> > Co-developed-by: Tomasz Figa <tfiga@chromium.org> > --- > src/libcamera/pipeline/virtual/README.md | 68 ++++++ > .../pipeline/virtual/data/virtual.yaml | 49 +++++ > src/libcamera/pipeline/virtual/meson.build | 1 + > src/libcamera/pipeline/virtual/parser.cpp | 198 ++++++++++++++++++ > src/libcamera/pipeline/virtual/parser.h | 45 ++++ > src/libcamera/pipeline/virtual/virtual.cpp | 47 +++-- > src/libcamera/pipeline/virtual/virtual.h | 6 +- > 7 files changed, 389 insertions(+), 25 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 000000000..27d6283df > --- /dev/null > +++ b/src/libcamera/pipeline/virtual/README.md [snip] > diff --git a/src/libcamera/pipeline/virtual/data/virtual.yaml b/src/libcamera/pipeline/virtual/data/virtual.yaml > new file mode 100644 > index 000000000..4eb239e24 > --- /dev/null > +++ b/src/libcamera/pipeline/virtual/data/virtual.yaml > @@ -0,0 +1,49 @@ > +# 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: > + - 30 > + - 60 > + test_pattern: > + location: "back" > + model: "Virtual Video Device1" > +"Virtual2": > + supported_formats: > + - width: 100 > + height: 100 > + test_pattern: "lines" > + location: "front" > + model: "Virtual Video Device2" > +"Virtual3": > + supported_formats: > + - width: 100 > + height: 100 > + - width: 800 > + height: 600 > + - width: 1920 > + height: 1080 > + frame_rates: > + - 20 > + - 30 > + location: "a" I get an error here ERROR Virtual parser.cpp:221 location: a is not supported ERROR Virtual parser.cpp:51 Failed to parse config of the camera: Virtual3 > + model: "Virtual Video Device3" > +"Virtual4": Is this a valid entry ? > diff --git a/src/libcamera/pipeline/virtual/meson.build b/src/libcamera/pipeline/virtual/meson.build > index e1e65e68d..2e82e64cb 100644 > --- a/src/libcamera/pipeline/virtual/meson.build > +++ b/src/libcamera/pipeline/virtual/meson.build > @@ -3,6 +3,7 @@ > libcamera_sources += files([ > 'virtual.cpp', > 'test_pattern_generator.cpp', > + 'parser.cpp', > ]) > > libyuv_dep = dependency('libyuv', required : false) > diff --git a/src/libcamera/pipeline/virtual/parser.cpp b/src/libcamera/pipeline/virtual/parser.cpp > new file mode 100644 > index 000000000..032c0cd9d > --- /dev/null > +++ b/src/libcamera/pipeline/virtual/parser.cpp > @@ -0,0 +1,198 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2023, Google Inc. 2024 > + * > + * 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(1000 / data->supportedResolutions_[0].frameRates[1]), > + int64_t(1000 / data->supportedResolutions_[0].frameRates[0])); FrameDurationLimits is expressed in useconds > + data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls); > + configurations.push_back(std::move(data)); > + } > + return configurations; > +} > + > +std::unique_ptr<VirtualCameraData> Parser::parseCameraConfigData( > + const YamlObject &cameraConfigData, PipelineHandler *pipe) > +{ > + std::unique_ptr<VirtualCameraData> data = std::make_unique<VirtualCameraData>(pipe); > + > + if (parseSupportedFormats(cameraConfigData, data.get())) > + return nullptr; > + > + 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, VirtualCameraData *data) > +{ > + Size activeResolution{ 0, 0 }; > + 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) { How can they be < 0 ? It's unsigned and the default is 1920 or 1080 if the property is not present Do you want to have the properties mandatory ? Make the default 0 and reject it in this case. > + 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>().value(); > + if (frameRatesList.size() != 2) { > + LOG(Virtual, Error) << "frame_rates needs to be the two edge values of a range"; Can't a config support a single frame rate ? > + return -EINVAL; > + } > + if (frameRatesList[0] > frameRatesList[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[0]); > + frameRates.push_back(frameRatesList[1]); > + } else { > + frameRates.push_back(30); > + frameRates.push_back(60); > + } > + > + data->supportedResolutions_.emplace_back( > + VirtualCameraData::Resolution{ Size{ width, height }, > + frameRates }); > + > + activeResolution = std::max(activeResolution, Size{ width, height }); > + } > + } else { > + data->supportedResolutions_.emplace_back( > + VirtualCameraData::Resolution{ Size{ 1920, 1080 }, > + { 30, 60 } }); > + activeResolution = Size(1920, 1080); > + } > + > + data->properties_.set(properties::PixelArrayActiveAreas, > + { Rectangle(activeResolution) }); Do you need this property ? Otherwise I would skip it as a camera max resolution is often different from the sensor's pixel array size > + > + return 0; > +} > + > +int Parser::parseTestPattern( > + const YamlObject &cameraConfigData, VirtualCameraData *data) > +{ > + std::string testPattern = cameraConfigData["test_pattern"].get<std::string>().value(); > + > + /* Default value is "bars" */ > + if (testPattern == "bars" || testPattern == "") { > + data->testPattern_ = TestPattern::ColorBars; > + } else if (testPattern == "lines") { > + data->testPattern_ = TestPattern::DiagonalLines; > + } else { > + LOG(Virtual, Error) << "Test pattern: " << testPattern > + << "is not supported"; > + return -EINVAL; > + } > + return 0; > +} > + > +int Parser::parseLocation( > + const YamlObject &cameraConfigData, VirtualCameraData *data) > +{ > + std::string location = cameraConfigData["location"].get<std::string>().value(); > + > + /* 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>().value(); > + > + /* 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 000000000..a377d8aa1 > --- /dev/null > +++ b/src/libcamera/pipeline/virtual/parser.h > @@ -0,0 +1,45 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2023, 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; > + > + 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, VirtualCameraData *data); > + 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 357fdd035..0fe471f00 100644 > --- a/src/libcamera/pipeline/virtual/virtual.cpp > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > @@ -18,6 +18,10 @@ > #include "libcamera/internal/camera.h" > #include "libcamera/internal/formats.h" > #include "libcamera/internal/pipeline_handler.h" > +#include "libcamera/internal/yaml_parser.h" > + > +#include "frame_generator.h" This should probably have been added in the previous patch > +#include "parser.h" > > namespace libcamera { > > @@ -228,32 +232,31 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > > bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator) > { > - /* \todo Add virtual cameras according to a config file. */ > - > - std::unique_ptr<VirtualCameraData> data = std::make_unique<VirtualCameraData>(this); > - > - data->supportedResolutions_.resize(2); > - data->supportedResolutions_[0] = { .size = Size(1920, 1080), .frame_rates = { 30 } }; > - data->supportedResolutions_[1] = { .size = Size(1280, 720), .frame_rates = { 30, 60 } }; > - > - data->properties_.set(properties::Location, properties::CameraLocationFront); > - data->properties_.set(properties::Model, "Virtual Video Device"); > - data->properties_.set(properties::PixelArrayActiveAreas, { Rectangle(Size(1920, 1080)) }); > + File file(configurationFile("virtual", "virtual.yaml")); I might have missed what 'configurationFile' is... > + bool isOpen = file.open(File::OpenModeFlag::ReadOnly); > + if (!isOpen) { > + LOG(Virtual, Error) << "Failed to open config file: " << file.fileName(); > + return false; > + } > > - /* \todo Set FrameDurationLimits based on config. */ > - ControlInfoMap::Map controls; > - int64_t min_frame_duration = 30, max_frame_duration = 60; > - controls[&controls::FrameDurationLimits] = ControlInfo(min_frame_duration, max_frame_duration); > - data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls); > + 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; > + } > > - /* Create and register the camera. */ > - std::set<Stream *> streams{ &data->stream_ }; > - 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{ &data->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 false; // Prevent infinite loops for now Ok, this doesn't change. It's not big deal, you're missing just the printout below void CameraManager::Private::pipelineFactoryMatch(const PipelineHandlerFactoryBase *factory) { CameraManager *const o = LIBCAMERA_O_PTR(); /* Provide as many matching pipelines as possible. */ while (1) { std::shared_ptr<PipelineHandler> pipe = factory->create(o); if (!pipe->match(enumerator_.get())) break; LOG(Camera, Debug) << "Pipeline handler \"" << factory->name() << "\" matched"; } } However it still feels like working around the intended design a bit. I don't have much more to suggest if not keeping a static variable around to store if the pipeline handler has been matches already or not, and once it has been matched fail at the next call. > } > diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h > index fecd9fa6f..c1ac4eb90 100644 > --- a/src/libcamera/pipeline/virtual/virtual.h > +++ b/src/libcamera/pipeline/virtual/virtual.h > @@ -22,7 +22,7 @@ class VirtualCameraData : public Camera::Private > public: > struct Resolution { > Size size; > - std::vector<int> frame_rates; > + std::vector<int> frameRates; > }; > VirtualCameraData(PipelineHandler *pipe) > : Camera::Private(pipe) > @@ -31,9 +31,9 @@ public: > > ~VirtualCameraData() = default; > > - TestPattern testPattern_; > - > + std::string id_; > std::vector<Resolution> supportedResolutions_; > + TestPattern testPattern_; > > Stream stream_; > > -- > 2.46.0.184.g6999bdac58-goog >
Thanks Jacopo, On Tue, Aug 27, 2024 at 7:01 PM Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > Hello > > On Tue, Aug 20, 2024 at 04:23:36PM GMT, Harvey Yang wrote: > > From: Konami Shu <konamiz@google.com> > > > > - Use Yaml::Parser to parse the config > > - Add config file at "virtual/data/virtual.yaml" > > - README.md contains the documentation for the format of config file and > > the implementation of Parser class. > > - Add Parser class to parse config file in Virtual > > pipeline handler > > - Add header file of virtual.cpp to use VirtualCameraData class in > > Parser class > > - Parse test patterns > > - Raise Error when the width of a resolution is an odd number > > Please make a commit message out of this list > Updated. Please check. > > > > > Signed-off-by: Konami Shu <konamiz@google.com> > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org> > > Co-developed-by: Yunke Cao <yunkec@chromium.org> > > Co-developed-by: Tomasz Figa <tfiga@chromium.org> > > --- > > src/libcamera/pipeline/virtual/README.md | 68 ++++++ > > .../pipeline/virtual/data/virtual.yaml | 49 +++++ > > src/libcamera/pipeline/virtual/meson.build | 1 + > > src/libcamera/pipeline/virtual/parser.cpp | 198 ++++++++++++++++++ > > src/libcamera/pipeline/virtual/parser.h | 45 ++++ > > src/libcamera/pipeline/virtual/virtual.cpp | 47 +++-- > > src/libcamera/pipeline/virtual/virtual.h | 6 +- > > 7 files changed, 389 insertions(+), 25 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 000000000..27d6283df > > --- /dev/null > > +++ b/src/libcamera/pipeline/virtual/README.md > > [snip] > > > diff --git a/src/libcamera/pipeline/virtual/data/virtual.yaml > b/src/libcamera/pipeline/virtual/data/virtual.yaml > > new file mode 100644 > > index 000000000..4eb239e24 > > --- /dev/null > > +++ b/src/libcamera/pipeline/virtual/data/virtual.yaml > > @@ -0,0 +1,49 @@ > > +# 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: > > + - 30 > > + - 60 > > + test_pattern: > > + location: "back" > > + model: "Virtual Video Device1" > > +"Virtual2": > > + supported_formats: > > + - width: 100 > > + height: 100 > > + test_pattern: "lines" > > + location: "front" > > + model: "Virtual Video Device2" > > +"Virtual3": > > + supported_formats: > > + - width: 100 > > + height: 100 > > + - width: 800 > > + height: 600 > > + - width: 1920 > > + height: 1080 > > + frame_rates: > > + - 20 > > + - 30 > > + location: "a" > > I get an error here > > ERROR Virtual parser.cpp:221 location: a is not supported > ERROR Virtual parser.cpp:51 Failed to parse config of the camera: Virtual3 > Right, I guess it was to check the parser catches the error and aborts parsing the camera, while still supporting other ones. Removed the camera in the config file. > > > + model: "Virtual Video Device3" > > +"Virtual4": > > Is this a valid entry ? > Actually, yes. Every field has a valid default value. > > > diff --git a/src/libcamera/pipeline/virtual/meson.build > b/src/libcamera/pipeline/virtual/meson.build > > index e1e65e68d..2e82e64cb 100644 > > --- a/src/libcamera/pipeline/virtual/meson.build > > +++ b/src/libcamera/pipeline/virtual/meson.build > > @@ -3,6 +3,7 @@ > > libcamera_sources += files([ > > 'virtual.cpp', > > 'test_pattern_generator.cpp', > > + 'parser.cpp', > > ]) > > > > libyuv_dep = dependency('libyuv', required : false) > > diff --git a/src/libcamera/pipeline/virtual/parser.cpp > b/src/libcamera/pipeline/virtual/parser.cpp > > new file mode 100644 > > index 000000000..032c0cd9d > > --- /dev/null > > +++ b/src/libcamera/pipeline/virtual/parser.cpp > > @@ -0,0 +1,198 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2023, Google Inc. > > 2024 > Done > > > + * > > + * 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(1000 / > data->supportedResolutions_[0].frameRates[1]), > > + int64_t(1000 / > data->supportedResolutions_[0].frameRates[0])); > > FrameDurationLimits is expressed in useconds > Do you mean microseconds? Updated to be 1,000,000 / framerate. > > > + data->controlInfo_ = ControlInfoMap(std::move(controls), > controls::controls); > > + configurations.push_back(std::move(data)); > > + } > > + return configurations; > > +} > > + > > +std::unique_ptr<VirtualCameraData> Parser::parseCameraConfigData( > > + const YamlObject &cameraConfigData, PipelineHandler *pipe) > > +{ > > + std::unique_ptr<VirtualCameraData> data = > std::make_unique<VirtualCameraData>(pipe); > > + > > + if (parseSupportedFormats(cameraConfigData, data.get())) > > + return nullptr; > > + > > + 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, VirtualCameraData *data) > > +{ > > + Size activeResolution{ 0, 0 }; > > + 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) { > > How can they be < 0 ? It's unsigned and the default is 1920 or 1080 if > the property is not present > Right, updated to be `== 0`. > > Do you want to have the properties mandatory ? Make the default 0 and > reject it in this case. > Hmm, that's a good question. Currently we have all the default values valid. Do you think we should do the reverse? > > > + 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>().value(); > > + if (frameRatesList.size() != 2) { > > + LOG(Virtual, Error) << > "frame_rates needs to be the two edge values of a range"; > > Can't a config support a single frame rate ? > In the current design, the upperBound equals to the lowerBound, while in the yaml file, the developer still needs to set two values. Do you think we should support only one value set in the config file? > > + return -EINVAL; > > + } > > + if (frameRatesList[0] > frameRatesList[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[0]); > > + frameRates.push_back(frameRatesList[1]); > > + } else { > > + frameRates.push_back(30); > > + frameRates.push_back(60); > > + } > > + > > + data->supportedResolutions_.emplace_back( > > + VirtualCameraData::Resolution{ Size{ > width, height }, > > + frameRates > }); > > + > > + activeResolution = std::max(activeResolution, > Size{ width, height }); > > + } > > + } else { > > + data->supportedResolutions_.emplace_back( > > + VirtualCameraData::Resolution{ Size{ 1920, 1080 }, > > + { 30, 60 } }); > > + activeResolution = Size(1920, 1080); > > + } > > + > > + data->properties_.set(properties::PixelArrayActiveAreas, > > + { Rectangle(activeResolution) }); > > Do you need this property ? Otherwise I would skip it as a camera max > resolution is often different from the sensor's pixel array size > Yes, actually the Android adapter requires this property to be set [1]. [1]: https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_capabilities.cpp#n1085 > > > + > > + return 0; > > +} > > + > > +int Parser::parseTestPattern( > > + const YamlObject &cameraConfigData, VirtualCameraData *data) > > +{ > > + std::string testPattern = > cameraConfigData["test_pattern"].get<std::string>().value(); > > + > > + /* Default value is "bars" */ > > + if (testPattern == "bars" || testPattern == "") { > > + data->testPattern_ = TestPattern::ColorBars; > > + } else if (testPattern == "lines") { > > + data->testPattern_ = TestPattern::DiagonalLines; > > + } else { > > + LOG(Virtual, Error) << "Test pattern: " << testPattern > > + << "is not supported"; > > + return -EINVAL; > > + } > > + return 0; > > +} > > + > > +int Parser::parseLocation( > > + const YamlObject &cameraConfigData, VirtualCameraData *data) > > +{ > > + std::string location = > cameraConfigData["location"].get<std::string>().value(); > > + > > + /* 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>().value(); > > + > > + /* 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 000000000..a377d8aa1 > > --- /dev/null > > +++ b/src/libcamera/pipeline/virtual/parser.h > > @@ -0,0 +1,45 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2023, 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; > > + > > + 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, VirtualCameraData > *data); > > + 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 357fdd035..0fe471f00 100644 > > --- a/src/libcamera/pipeline/virtual/virtual.cpp > > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > > @@ -18,6 +18,10 @@ > > #include "libcamera/internal/camera.h" > > #include "libcamera/internal/formats.h" > > #include "libcamera/internal/pipeline_handler.h" > > +#include "libcamera/internal/yaml_parser.h" > > + > > +#include "frame_generator.h" > > This should probably have been added in the previous patch > Removed. > > > +#include "parser.h" > > > > namespace libcamera { > > > > @@ -228,32 +232,31 @@ int > PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > > > > bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator > *enumerator) > > { > > - /* \todo Add virtual cameras according to a config file. */ > > - > > - std::unique_ptr<VirtualCameraData> data = > std::make_unique<VirtualCameraData>(this); > > - > > - data->supportedResolutions_.resize(2); > > - data->supportedResolutions_[0] = { .size = Size(1920, 1080), > .frame_rates = { 30 } }; > > - data->supportedResolutions_[1] = { .size = Size(1280, 720), > .frame_rates = { 30, 60 } }; > > - > > - data->properties_.set(properties::Location, > properties::CameraLocationFront); > > - data->properties_.set(properties::Model, "Virtual Video Device"); > > - data->properties_.set(properties::PixelArrayActiveAreas, { > Rectangle(Size(1920, 1080)) }); > > + File file(configurationFile("virtual", "virtual.yaml")); > > I might have missed what 'configurationFile' is... > Sorry, I don't get what you mean. Do you mean that I should add a comment to explain how the path works [2]? [2]: https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline_handler.cpp#n558 > > > + bool isOpen = file.open(File::OpenModeFlag::ReadOnly); > > + if (!isOpen) { > > + LOG(Virtual, Error) << "Failed to open config file: " << > file.fileName(); > > + return false; > > + } > > > > - /* \todo Set FrameDurationLimits based on config. */ > > - ControlInfoMap::Map controls; > > - int64_t min_frame_duration = 30, max_frame_duration = 60; > > - controls[&controls::FrameDurationLimits] = > ControlInfo(min_frame_duration, max_frame_duration); > > - data->controlInfo_ = ControlInfoMap(std::move(controls), > controls::controls); > > + 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; > > + } > > > > - /* Create and register the camera. */ > > - std::set<Stream *> streams{ &data->stream_ }; > > - 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{ &data->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 false; // Prevent infinite loops for now > > Ok, this doesn't change. It's not big deal, you're missing just the > printout below > > void CameraManager::Private::pipelineFactoryMatch(const > PipelineHandlerFactoryBase *factory) > { > CameraManager *const o = LIBCAMERA_O_PTR(); > > /* Provide as many matching pipelines as possible. */ > while (1) { > std::shared_ptr<PipelineHandler> pipe = factory->create(o); > if (!pipe->match(enumerator_.get())) > break; > > LOG(Camera, Debug) > << "Pipeline handler \"" << factory->name() > << "\" matched"; > } > } > > However it still feels like working around the intended design a bit. > I don't have much more to suggest if not keeping a static variable > around to store if the pipeline handler has been matches already or > not, and once it has been matched fail at the next call. > > Yeah, I did exactly that in the 3rd patch. Thanks for checking! > > } > > diff --git a/src/libcamera/pipeline/virtual/virtual.h > b/src/libcamera/pipeline/virtual/virtual.h > > index fecd9fa6f..c1ac4eb90 100644 > > --- a/src/libcamera/pipeline/virtual/virtual.h > > +++ b/src/libcamera/pipeline/virtual/virtual.h > > @@ -22,7 +22,7 @@ class VirtualCameraData : public Camera::Private > > public: > > struct Resolution { > > Size size; > > - std::vector<int> frame_rates; > > + std::vector<int> frameRates; > > }; > > VirtualCameraData(PipelineHandler *pipe) > > : Camera::Private(pipe) > > @@ -31,9 +31,9 @@ public: > > > > ~VirtualCameraData() = default; > > > > - TestPattern testPattern_; > > - > > + std::string id_; > > std::vector<Resolution> supportedResolutions_; > > + TestPattern testPattern_; > > > > Stream stream_; > > > > -- > > 2.46.0.184.g6999bdac58-goog > > >
Hi Jacopo, On Sat, Aug 31, 2024 at 8:54 PM Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > Hi Harvey > > On Thu, Aug 29, 2024 at 09:58:01PM GMT, Cheng-Hao Yang wrote: > > Thanks Jacopo, > > > > On Tue, Aug 27, 2024 at 7:01 PM Jacopo Mondi < > jacopo.mondi@ideasonboard.com> > > wrote: > > > > > Hello > > > > > > On Tue, Aug 20, 2024 at 04:23:36PM GMT, Harvey Yang wrote: > > > > From: Konami Shu <konamiz@google.com> > > > > > > > > - Use Yaml::Parser to parse the config > > > > - Add config file at "virtual/data/virtual.yaml" > > > > - README.md contains the documentation for the format of config file > and > > > > the implementation of Parser class. > > > > - Add Parser class to parse config file in Virtual > > > > pipeline handler > > > > - Add header file of virtual.cpp to use VirtualCameraData class in > > > > Parser class > > > > - Parse test patterns > > > > - Raise Error when the width of a resolution is an odd number > > > > > > Please make a commit message out of this list > > > > > Updated. Please check. > > > > > > > > > > > > > > > Signed-off-by: Konami Shu <konamiz@google.com> > > > > Co-developed-by: Harvey Yang <chenghaoyang@chromium.org> > > > > Co-developed-by: Yunke Cao <yunkec@chromium.org> > > > > Co-developed-by: Tomasz Figa <tfiga@chromium.org> > > > > --- > > > > src/libcamera/pipeline/virtual/README.md | 68 ++++++ > > > > .../pipeline/virtual/data/virtual.yaml | 49 +++++ > > > > src/libcamera/pipeline/virtual/meson.build | 1 + > > > > src/libcamera/pipeline/virtual/parser.cpp | 198 > ++++++++++++++++++ > > > > src/libcamera/pipeline/virtual/parser.h | 45 ++++ > > > > src/libcamera/pipeline/virtual/virtual.cpp | 47 +++-- > > > > src/libcamera/pipeline/virtual/virtual.h | 6 +- > > > > 7 files changed, 389 insertions(+), 25 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 000000000..27d6283df > > > > --- /dev/null > > > > +++ b/src/libcamera/pipeline/virtual/README.md > > > > > > [snip] > > > > > > > diff --git a/src/libcamera/pipeline/virtual/data/virtual.yaml > > > b/src/libcamera/pipeline/virtual/data/virtual.yaml > > > > new file mode 100644 > > > > index 000000000..4eb239e24 > > > > --- /dev/null > > > > +++ b/src/libcamera/pipeline/virtual/data/virtual.yaml > > > > @@ -0,0 +1,49 @@ > > > > +# 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: > > > > + - 30 > > > > + - 60 > > > > + test_pattern: > > > > + location: "back" > > > > + model: "Virtual Video Device1" > > > > +"Virtual2": > > > > + supported_formats: > > > > + - width: 100 > > > > + height: 100 > > > > + test_pattern: "lines" > > > > + location: "front" > > > > + model: "Virtual Video Device2" > > > > +"Virtual3": > > > > + supported_formats: > > > > + - width: 100 > > > > + height: 100 > > > > + - width: 800 > > > > + height: 600 > > > > + - width: 1920 > > > > + height: 1080 > > > > + frame_rates: > > > > + - 20 > > > > + - 30 > > > > + location: "a" > > > > > > I get an error here > > > > > > ERROR Virtual parser.cpp:221 location: a is not supported > > > ERROR Virtual parser.cpp:51 Failed to parse config of the camera: > Virtual3 > > > > > Right, I guess it was to check the parser catches the error and > > aborts parsing the camera, while still supporting other ones. > > > > Removed the camera in the config file. > > > > > > > > > > > + model: "Virtual Video Device3" > > > > +"Virtual4": > > > > > > Is this a valid entry ? > > > > > Actually, yes. Every field has a valid default value. > > > > > > > > > > > diff --git a/src/libcamera/pipeline/virtual/meson.build > > > b/src/libcamera/pipeline/virtual/meson.build > > > > index e1e65e68d..2e82e64cb 100644 > > > > --- a/src/libcamera/pipeline/virtual/meson.build > > > > +++ b/src/libcamera/pipeline/virtual/meson.build > > > > @@ -3,6 +3,7 @@ > > > > libcamera_sources += files([ > > > > 'virtual.cpp', > > > > 'test_pattern_generator.cpp', > > > > + 'parser.cpp', > > > > ]) > > > > > > > > libyuv_dep = dependency('libyuv', required : false) > > > > diff --git a/src/libcamera/pipeline/virtual/parser.cpp > > > b/src/libcamera/pipeline/virtual/parser.cpp > > > > new file mode 100644 > > > > index 000000000..032c0cd9d > > > > --- /dev/null > > > > +++ b/src/libcamera/pipeline/virtual/parser.cpp > > > > @@ -0,0 +1,198 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * Copyright (C) 2023, Google Inc. > > > > > > 2024 > > > > > Done > > > > > > > > > > > + * > > > > + * 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(1000 / > > > data->supportedResolutions_[0].frameRates[1]), > > > > + int64_t(1000 / > > > data->supportedResolutions_[0].frameRates[0])); > > > > > > FrameDurationLimits is expressed in useconds > > > > > Do you mean microseconds? Updated to be 1,000,000 / framerate. > > > > > > > > > > > + data->controlInfo_ = > ControlInfoMap(std::move(controls), > > > controls::controls); > > > > + configurations.push_back(std::move(data)); > > > > + } > > > > + return configurations; > > > > +} > > > > + > > > > +std::unique_ptr<VirtualCameraData> Parser::parseCameraConfigData( > > > > + const YamlObject &cameraConfigData, PipelineHandler *pipe) > > > > +{ > > > > + std::unique_ptr<VirtualCameraData> data = > > > std::make_unique<VirtualCameraData>(pipe); > > > > + > > > > + if (parseSupportedFormats(cameraConfigData, data.get())) > > > > + return nullptr; > > > > + > > > > + 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, VirtualCameraData *data) > > > > +{ > > > > + Size activeResolution{ 0, 0 }; > > > > + 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) { > > > > > > How can they be < 0 ? It's unsigned and the default is 1920 or 1080 if > > > the property is not present > > > > > Right, updated to be `== 0`. > > > > > > > > > > Do you want to have the properties mandatory ? Make the default 0 and > > > reject it in this case. > > > > > Hmm, that's a good question. Currently we have all the default values > valid. > > Do you think we should do the reverse? > > > > > > > > > > > + 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>().value(); > > > > + if (frameRatesList.size() != 2) { > > > > + LOG(Virtual, Error) << > > > "frame_rates needs to be the two edge values of a range"; > > > > > > Can't a config support a single frame rate ? > > > > > In the current design, the upperBound equals to the lowerBound, > > while in the yaml file, the developer still needs to set two values. > > Do you think we should support only one value set in the config > > file? > > I would say why not, but as this pipeline is just for testing, up to > you... > Will be updated in v11. > > > > > > > > > + return -EINVAL; > > > > + } > > > > + if (frameRatesList[0] > > frameRatesList[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[0]); > > > > + > frameRates.push_back(frameRatesList[1]); > > > > + } else { > > > > + frameRates.push_back(30); > > > > + frameRates.push_back(60); > > > > + } > > > > + > > > > + data->supportedResolutions_.emplace_back( > > > > + VirtualCameraData::Resolution{ Size{ > > > width, height }, > > > > + > frameRates > > > }); > > > > + > > > > + activeResolution = std::max(activeResolution, > > > Size{ width, height }); > > > > + } > > > > + } else { > > > > + data->supportedResolutions_.emplace_back( > > > > + VirtualCameraData::Resolution{ Size{ 1920, > 1080 }, > > > > + { 30, 60 } }); > > > > + activeResolution = Size(1920, 1080); > > > > + } > > > > + > > > > + data->properties_.set(properties::PixelArrayActiveAreas, > > > > + { Rectangle(activeResolution) }); > > > > > > Do you need this property ? Otherwise I would skip it as a camera max > > > resolution is often different from the sensor's pixel array size > > > > > Yes, actually the Android adapter requires this property to be set [1]. > > > > [1]: > > > https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_capabilities.cpp#n1085 > > > > > > > > > > > + > > > > + return 0; > > > > +} > > > > + > > > > +int Parser::parseTestPattern( > > > > + const YamlObject &cameraConfigData, VirtualCameraData *data) > > > > +{ > > > > + std::string testPattern = > > > cameraConfigData["test_pattern"].get<std::string>().value(); > > > > + > > > > + /* Default value is "bars" */ > > > > + if (testPattern == "bars" || testPattern == "") { > > > > + data->testPattern_ = TestPattern::ColorBars; > > > > + } else if (testPattern == "lines") { > > > > + data->testPattern_ = TestPattern::DiagonalLines; > > > > + } else { > > > > + LOG(Virtual, Error) << "Test pattern: " << testPattern > > > > + << "is not supported"; > > > > + return -EINVAL; > > > > + } > > > > + return 0; > > > > +} > > > > + > > > > +int Parser::parseLocation( > > > > + const YamlObject &cameraConfigData, VirtualCameraData *data) > > > > +{ > > > > + std::string location = > > > cameraConfigData["location"].get<std::string>().value(); > > > > + > > > > + /* 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>().value(); > > > > + > > > > + /* 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 000000000..a377d8aa1 > > > > --- /dev/null > > > > +++ b/src/libcamera/pipeline/virtual/parser.h > > > > @@ -0,0 +1,45 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * Copyright (C) 2023, 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; > > > > + > > > > + 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, VirtualCameraData > > > *data); > > > > + 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 357fdd035..0fe471f00 100644 > > > > --- a/src/libcamera/pipeline/virtual/virtual.cpp > > > > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > > > > @@ -18,6 +18,10 @@ > > > > #include "libcamera/internal/camera.h" > > > > #include "libcamera/internal/formats.h" > > > > #include "libcamera/internal/pipeline_handler.h" > > > > +#include "libcamera/internal/yaml_parser.h" > > > > + > > > > +#include "frame_generator.h" > > > > > > This should probably have been added in the previous patch > > > > > Removed. > > > > > > > > > > > +#include "parser.h" > > > > > > > > namespace libcamera { > > > > > > > > @@ -228,32 +232,31 @@ int > > > PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera > *camera, > > > > > > > > bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator > > > *enumerator) > > > > { > > > > - /* \todo Add virtual cameras according to a config file. */ > > > > - > > > > - std::unique_ptr<VirtualCameraData> data = > > > std::make_unique<VirtualCameraData>(this); > > > > - > > > > - data->supportedResolutions_.resize(2); > > > > - data->supportedResolutions_[0] = { .size = Size(1920, 1080), > > > .frame_rates = { 30 } }; > > > > - data->supportedResolutions_[1] = { .size = Size(1280, 720), > > > .frame_rates = { 30, 60 } }; > > > > - > > > > - data->properties_.set(properties::Location, > > > properties::CameraLocationFront); > > > > - data->properties_.set(properties::Model, "Virtual Video > Device"); > > > > - data->properties_.set(properties::PixelArrayActiveAreas, { > > > Rectangle(Size(1920, 1080)) }); > > > > + File file(configurationFile("virtual", "virtual.yaml")); > > > > > > I might have missed what 'configurationFile' is... > > > > > Sorry, I don't get what you mean. Do you mean that I should add > > a comment to explain how the path works [2]? > > [2]: > > > https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline_handler.cpp#n558 > > I meant that I get a single hit for > > $ git grep configurationFile src/libcamera/pipeline/virtual/ > src/libcamera/pipeline/virtual/virtual.cpp: File > file(configurationFile("virtual", "virtual.yaml")); > > so I don't get what the "configuratioFile" symbol means here. It > compiles, so I am missing something for sure. > > Oh it's a function in the base class `PipelineHandler': https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline_handler.cpp#n589 > > > > > > > > > > > + bool isOpen = file.open(File::OpenModeFlag::ReadOnly); > > > > + if (!isOpen) { > > > > + LOG(Virtual, Error) << "Failed to open config file: " > << > > > file.fileName(); > > > > + return false; > > > > + } > > > > > > > > - /* \todo Set FrameDurationLimits based on config. */ > > > > - ControlInfoMap::Map controls; > > > > - int64_t min_frame_duration = 30, max_frame_duration = 60; > > > > - controls[&controls::FrameDurationLimits] = > > > ControlInfo(min_frame_duration, max_frame_duration); > > > > - data->controlInfo_ = ControlInfoMap(std::move(controls), > > > controls::controls); > > > > + 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; > > > > + } > > > > > > > > - /* Create and register the camera. */ > > > > - std::set<Stream *> streams{ &data->stream_ }; > > > > - 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{ &data->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 false; // Prevent infinite loops for now > > > > > > Ok, this doesn't change. It's not big deal, you're missing just the > > > printout below > > > > > > void CameraManager::Private::pipelineFactoryMatch(const > > > PipelineHandlerFactoryBase *factory) > > > { > > > CameraManager *const o = LIBCAMERA_O_PTR(); > > > > > > /* Provide as many matching pipelines as possible. */ > > > while (1) { > > > std::shared_ptr<PipelineHandler> pipe = > factory->create(o); > > > if (!pipe->match(enumerator_.get())) > > > break; > > > > > > LOG(Camera, Debug) > > > << "Pipeline handler \"" << factory->name() > > > << "\" matched"; > > > } > > > } > > > > > > However it still feels like working around the intended design a bit. > > > I don't have much more to suggest if not keeping a static variable > > > around to store if the pipeline handler has been matches already or > > > not, and once it has been matched fail at the next call. > > > > > > Yeah, I did exactly that in the 3rd patch. Thanks for checking! > > > > > > > > } > > > > diff --git a/src/libcamera/pipeline/virtual/virtual.h > > > b/src/libcamera/pipeline/virtual/virtual.h > > > > index fecd9fa6f..c1ac4eb90 100644 > > > > --- a/src/libcamera/pipeline/virtual/virtual.h > > > > +++ b/src/libcamera/pipeline/virtual/virtual.h > > > > @@ -22,7 +22,7 @@ class VirtualCameraData : public Camera::Private > > > > public: > > > > struct Resolution { > > > > Size size; > > > > - std::vector<int> frame_rates; > > > > + std::vector<int> frameRates; > > > > }; > > > > VirtualCameraData(PipelineHandler *pipe) > > > > : Camera::Private(pipe) > > > > @@ -31,9 +31,9 @@ public: > > > > > > > > ~VirtualCameraData() = default; > > > > > > > > - TestPattern testPattern_; > > > > - > > > > + std::string id_; > > > > std::vector<Resolution> supportedResolutions_; > > > > + TestPattern testPattern_; > > > > > > > > Stream stream_; > > > > > > > > -- > > > > 2.46.0.184.g6999bdac58-goog > > > > > > > >
diff --git a/src/libcamera/pipeline/virtual/README.md b/src/libcamera/pipeline/virtual/README.md new file mode 100644 index 000000000..27d6283df --- /dev/null +++ b/src/libcamera/pipeline/virtual/README.md @@ -0,0 +1,68 @@ +# 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. The list has to be two values of the lower bound and the upper bound of the frame rate. +- `test_pattern` (`string`, default="bars"): 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. + +A sample config file: +``` +--- +"Virtual0": + supported_formats: + - width: 1920 + height: 1080 + frame_rates: + - 30 + - 60 + - width: 1680 + height: 1050 + frame_rates: + - 70 + - 80 + test_pattern: "bars" + location: "front" + model: "Virtual Video Device" +"Virtual1": + supported_formats: + - width: 800 + test_pattern: "lines" + location: "back" + model: "Virtual Video Device1" +"Virtual2": +``` + +### 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 000000000..4eb239e24 --- /dev/null +++ b/src/libcamera/pipeline/virtual/data/virtual.yaml @@ -0,0 +1,49 @@ +# 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: + - 30 + - 60 + test_pattern: + location: "back" + model: "Virtual Video Device1" +"Virtual2": + supported_formats: + - width: 100 + height: 100 + test_pattern: "lines" + location: "front" + model: "Virtual Video Device2" +"Virtual3": + supported_formats: + - width: 100 + height: 100 + - width: 800 + height: 600 + - width: 1920 + height: 1080 + frame_rates: + - 20 + - 30 + location: "a" + model: "Virtual Video Device3" +"Virtual4": diff --git a/src/libcamera/pipeline/virtual/meson.build b/src/libcamera/pipeline/virtual/meson.build index e1e65e68d..2e82e64cb 100644 --- a/src/libcamera/pipeline/virtual/meson.build +++ b/src/libcamera/pipeline/virtual/meson.build @@ -3,6 +3,7 @@ libcamera_sources += files([ 'virtual.cpp', 'test_pattern_generator.cpp', + 'parser.cpp', ]) libyuv_dep = dependency('libyuv', required : false) diff --git a/src/libcamera/pipeline/virtual/parser.cpp b/src/libcamera/pipeline/virtual/parser.cpp new file mode 100644 index 000000000..032c0cd9d --- /dev/null +++ b/src/libcamera/pipeline/virtual/parser.cpp @@ -0,0 +1,198 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2023, 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(1000 / data->supportedResolutions_[0].frameRates[1]), + int64_t(1000 / data->supportedResolutions_[0].frameRates[0])); + data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls); + configurations.push_back(std::move(data)); + } + return configurations; +} + +std::unique_ptr<VirtualCameraData> Parser::parseCameraConfigData( + const YamlObject &cameraConfigData, PipelineHandler *pipe) +{ + std::unique_ptr<VirtualCameraData> data = std::make_unique<VirtualCameraData>(pipe); + + if (parseSupportedFormats(cameraConfigData, data.get())) + return nullptr; + + 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, VirtualCameraData *data) +{ + Size activeResolution{ 0, 0 }; + 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>().value(); + if (frameRatesList.size() != 2) { + LOG(Virtual, Error) << "frame_rates needs to be the two edge values of a range"; + return -EINVAL; + } + if (frameRatesList[0] > frameRatesList[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[0]); + frameRates.push_back(frameRatesList[1]); + } else { + frameRates.push_back(30); + frameRates.push_back(60); + } + + data->supportedResolutions_.emplace_back( + VirtualCameraData::Resolution{ Size{ width, height }, + frameRates }); + + activeResolution = std::max(activeResolution, Size{ width, height }); + } + } else { + data->supportedResolutions_.emplace_back( + VirtualCameraData::Resolution{ Size{ 1920, 1080 }, + { 30, 60 } }); + activeResolution = Size(1920, 1080); + } + + data->properties_.set(properties::PixelArrayActiveAreas, + { Rectangle(activeResolution) }); + + return 0; +} + +int Parser::parseTestPattern( + const YamlObject &cameraConfigData, VirtualCameraData *data) +{ + std::string testPattern = cameraConfigData["test_pattern"].get<std::string>().value(); + + /* Default value is "bars" */ + if (testPattern == "bars" || testPattern == "") { + data->testPattern_ = TestPattern::ColorBars; + } else if (testPattern == "lines") { + data->testPattern_ = TestPattern::DiagonalLines; + } else { + LOG(Virtual, Error) << "Test pattern: " << testPattern + << "is not supported"; + return -EINVAL; + } + return 0; +} + +int Parser::parseLocation( + const YamlObject &cameraConfigData, VirtualCameraData *data) +{ + std::string location = cameraConfigData["location"].get<std::string>().value(); + + /* 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>().value(); + + /* 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 000000000..a377d8aa1 --- /dev/null +++ b/src/libcamera/pipeline/virtual/parser.h @@ -0,0 +1,45 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2023, 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; + + 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, VirtualCameraData *data); + 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 357fdd035..0fe471f00 100644 --- a/src/libcamera/pipeline/virtual/virtual.cpp +++ b/src/libcamera/pipeline/virtual/virtual.cpp @@ -18,6 +18,10 @@ #include "libcamera/internal/camera.h" #include "libcamera/internal/formats.h" #include "libcamera/internal/pipeline_handler.h" +#include "libcamera/internal/yaml_parser.h" + +#include "frame_generator.h" +#include "parser.h" namespace libcamera { @@ -228,32 +232,31 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator) { - /* \todo Add virtual cameras according to a config file. */ - - std::unique_ptr<VirtualCameraData> data = std::make_unique<VirtualCameraData>(this); - - data->supportedResolutions_.resize(2); - data->supportedResolutions_[0] = { .size = Size(1920, 1080), .frame_rates = { 30 } }; - data->supportedResolutions_[1] = { .size = Size(1280, 720), .frame_rates = { 30, 60 } }; - - data->properties_.set(properties::Location, properties::CameraLocationFront); - data->properties_.set(properties::Model, "Virtual Video Device"); - data->properties_.set(properties::PixelArrayActiveAreas, { Rectangle(Size(1920, 1080)) }); + 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; + } - /* \todo Set FrameDurationLimits based on config. */ - ControlInfoMap::Map controls; - int64_t min_frame_duration = 30, max_frame_duration = 60; - controls[&controls::FrameDurationLimits] = ControlInfo(min_frame_duration, max_frame_duration); - data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls); + 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; + } - /* Create and register the camera. */ - std::set<Stream *> streams{ &data->stream_ }; - 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{ &data->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 false; // Prevent infinite loops for now } diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h index fecd9fa6f..c1ac4eb90 100644 --- a/src/libcamera/pipeline/virtual/virtual.h +++ b/src/libcamera/pipeline/virtual/virtual.h @@ -22,7 +22,7 @@ class VirtualCameraData : public Camera::Private public: struct Resolution { Size size; - std::vector<int> frame_rates; + std::vector<int> frameRates; }; VirtualCameraData(PipelineHandler *pipe) : Camera::Private(pipe) @@ -31,9 +31,9 @@ public: ~VirtualCameraData() = default; - TestPattern testPattern_; - + std::string id_; std::vector<Resolution> supportedResolutions_; + TestPattern testPattern_; Stream stream_;