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