<div dir="ltr"><div dir="ltr">Thanks Jacopo,</div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">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>> 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">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></blockquote><div>Updated. Please check.</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>
> 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 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 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></blockquote><div>Right, I guess it was to check the parser catches the error and</div><div>aborts parsing the camera, while still supporting other ones.</div><div><br></div><div>Removed the camera in the config file.</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>
> +  model: "Virtual Video Device3"<br>
> +"Virtual4":<br>
<br>
Is this a valid entry ?<br></blockquote><div>Actually, yes. Every field has a valid default value.</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>
> diff --git a/src/libcamera/pipeline/virtual/meson.build 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 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></blockquote><div>Done</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>
> + * 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 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>
> +             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 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 reported */<br>
> +             controls[&controls::FrameDurationLimits] =<br>
> +                     ControlInfo(int64_t(1000 / data->supportedResolutions_[0].frameRates[1]),<br>
> +                                 int64_t(1000 / data->supportedResolutions_[0].frameRates[0]));<br>
<br>
FrameDurationLimits is expressed in useconds<br></blockquote><div>Do you mean microseconds? Updated to be 1,000,000 / framerate.</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>
> +             data->controlInfo_ = ControlInfoMap(std::move(controls), 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 = 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 = cameraConfigData["supported_formats"];<br>
> +<br>
> +             for (const YamlObject &supportedResolution : supportedResolutions.asList()) {<br>
> +                     unsigned int width = supportedResolution["width"].get<unsigned int>(1920);<br>
> +                     unsigned int height = 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></blockquote><div>Right, updated to be `== 0`.</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>
Do you want to have the properties mandatory ? Make the default 0 and<br>
reject it in this case.<br></blockquote><div>Hmm, that's a good question. Currently we have all the default values valid.</div><div>Do you think we should do the reverse?</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>
> +                             LOG(Virtual, Error) << "Invalid width or/and height";<br>
> +                             return -EINVAL;<br>
> +                     }<br>
> +                     if (width % 2 != 0) {<br>
> +                             LOG(Virtual, Error) << "Invalid width: 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>
> +                                     supportedResolution["frame_rates"].getList<int>().value();<br>
> +                             if (frameRatesList.size() != 2) {<br>
> +                                     LOG(Virtual, Error) << "frame_rates needs to be the two edge values of a range";<br>
<br>
Can't a config support a single frame rate ?<br></blockquote><div>In the current design, the upperBound equals to the lowerBound,</div><div>while in the yaml file, the developer still needs to set two values.</div><div>Do you think we should support only one value set in the config</div><div>file?</div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
> +                                     return -EINVAL;<br>
> +                             }<br>
> +                             if (frameRatesList[0] > frameRatesList[1]) {<br>
> +                                     LOG(Virtual, Error) << "frame_rates's first value(lower bound) is higher than the second 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{ width, height },<br>
> +                                                            frameRates });<br>
> +<br>
> +                     activeResolution = std::max(activeResolution, 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></blockquote><div>Yes, actually the Android adapter requires this property to be set [1].</div><div><br></div><div>[1]: <a href="https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_capabilities.cpp#n1085" target="_blank">https://git.libcamera.org/libcamera/libcamera.git/tree/src/android/camera_capabilities.cpp#n1085</a></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>
> +     return 0;<br>
> +}<br>
> +<br>
> +int Parser::parseTestPattern(<br>
> +     const YamlObject &cameraConfigData, VirtualCameraData *data)<br>
> +{<br>
> +     std::string testPattern = 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 = 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 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 *data);<br>
> +     int parseTestPattern(<br>
> +             const YamlObject &cameraConfigData, VirtualCameraData *data);<br>
> +     int parseLocation(<br>
> +             const YamlObject &cameraConfigData, VirtualCameraData *data);<br>
> +     int parseModel(<br>
> +             const YamlObject &cameraConfigData, VirtualCameraData *data);<br>
> +};<br>
> +<br>
> +} // namespace libcamera<br>
> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp 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></blockquote><div>Removed.</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>
> +#include "parser.h"<br>
><br>
>  namespace libcamera {<br>
><br>
> @@ -228,32 +232,31 @@ int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera,<br>
><br>
>  bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator)<br>
>  {<br>
> -     /* \todo Add virtual cameras according to a config file. */<br>
> -<br>
> -     std::unique_ptr<VirtualCameraData> data = std::make_unique<VirtualCameraData>(this);<br>
> -<br>
> -     data->supportedResolutions_.resize(2);<br>
> -     data->supportedResolutions_[0] = { .size = Size(1920, 1080), .frame_rates = { 30 } };<br>
> -     data->supportedResolutions_[1] = { .size = Size(1280, 720), .frame_rates = { 30, 60 } };<br>
> -<br>
> -     data->properties_.set(properties::Location, properties::CameraLocationFront);<br>
> -     data->properties_.set(properties::Model, "Virtual Video Device");<br>
> -     data->properties_.set(properties::PixelArrayActiveAreas, { Rectangle(Size(1920, 1080)) });<br>
> +     File file(configurationFile("virtual", "virtual.yaml"));<br>
<br>
I might have missed what 'configurationFile' is...<br></blockquote><div>Sorry, I don't get what you mean. Do you mean that I should add</div><div>a comment to explain how the path works [2]?</div><div>[2]: <a href="https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline_handler.cpp#n558" target="_blank">https://git.libcamera.org/libcamera/libcamera.git/tree/src/libcamera/pipeline_handler.cpp#n558</a></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>
> +     bool isOpen = file.open(File::OpenModeFlag::ReadOnly);<br>
> +     if (!isOpen) {<br>
> +             LOG(Virtual, Error) << "Failed to open config file: " << 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] = ControlInfo(min_frame_duration, max_frame_duration);<br>
> -     data->controlInfo_ = ControlInfoMap(std::move(controls), 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 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), 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 = 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 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></blockquote><div>Yeah, I did exactly that in the 3rd patch. Thanks for checking!</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>
> diff --git a/src/libcamera/pipeline/virtual/virtual.h 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>
</blockquote></div></div>