[PATCH 15 6/7] libcamera: virtual: Read config and register cameras based on the config

Kieran Bingham kieran.bingham at ideasonboard.com
Tue Oct 22 01:31:56 CEST 2024


Quoting Harvey Yang (2024-10-04 10:59:19)
> This patch introduces the configuration file for Virtual Pipeline
> Handler. The config file is written in yaml, and the format is
> documented in `README.md`.
> 
> The config file will define the camera with IDs, supported formats and
> image sources, etc. In the default config file, only Test Patterns are
> used. Developers can use real images loading if desired.
> 
> Signed-off-by: Konami Shu <konamiz at google.com>
> Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> Co-developed-by: Yunke Cao <yunkec at chromium.org>
> Co-developed-by: Tomasz Figa <tfiga at chromium.org>

I think we determined Co-developed-by: need SoBs.

Checkstyle reports:

-----------------------------------------------------------------------------------------------------------------
83f896b4afa8e5e7a4029d1e07bcb5dfe8cc5fb2 libcamera: virtual: Read config and register cameras based on the config
-----------------------------------------------------------------------------------------------------------------
No 'Signed-off-by' trailer matching author 'Harvey Yang <chenghaoyang at chromium.org>', see Documentation/contributing.rst
---
1 potential issue detected, please review

And that will apply to all the above.


> ---
>  src/libcamera/pipeline/virtual/README.md      |  65 +++++
>  .../pipeline/virtual/config_parser.cpp        | 263 ++++++++++++++++++
>  .../pipeline/virtual/config_parser.h          |  39 +++
>  .../pipeline/virtual/data/virtual.yaml        |  36 +++
>  .../virtual/image_frame_generator.cpp         |  16 +-
>  .../pipeline/virtual/image_frame_generator.h  |   5 +-
>  src/libcamera/pipeline/virtual/meson.build    |   1 +
>  src/libcamera/pipeline/virtual/virtual.cpp    | 126 +++++----
>  src/libcamera/pipeline/virtual/virtual.h      |  23 +-
>  9 files changed, 504 insertions(+), 70 deletions(-)
>  create mode 100644 src/libcamera/pipeline/virtual/README.md
>  create mode 100644 src/libcamera/pipeline/virtual/config_parser.cpp
>  create mode 100644 src/libcamera/pipeline/virtual/config_parser.h
>  create mode 100644 src/libcamera/pipeline/virtual/data/virtual.yaml
> 
> diff --git a/src/libcamera/pipeline/virtual/README.md b/src/libcamera/pipeline/virtual/README.md
> new file mode 100644
> index 000000000..5801e79b5
> --- /dev/null
> +++ b/src/libcamera/pipeline/virtual/README.md
> @@ -0,0 +1,65 @@
> +# Virtual Pipeline Handler
> +
> +Virtual pipeline handler emulates fake external camera(s) on ChromeOS for testing.

It's not only on ChromeOS right ? It's just a virtual camera for any
platform.



> +
> +## Parse config file and register cameras
> +
> +- The config file is located at `src/libcamera/pipeline/virtual/data/virtual.yaml`

I'm sure I mentioned this before - but this is the source location.
While I preume the users would have to configure their 'installed'
location. So this should probably reference the fact it would be
installed in ...

Err - where does it get installed ? What happens if libcamera is
installed and tries to use the virtual pipeline handler here ?

I don't see any reference in a meson file that installes
data/virtual.yaml ?


Later on virtual.yaml is mentioned as a sample, while up here it's
specified as '"The" config file'. ? 

Maybe this should say "A sample config file is located at ..."

> +
> +### Config File Format
> +The config file contains the information about cameras' properties to register.
> +The config file should be a yaml file with dictionary of the cameraIds
> +associated with their properties as top level. The default value will be applied
> +when any property is empty.
> +
> +Each camera block is a dictionary, containing the following keys:
> +- `supported_formats` (list of `VirtualCameraData::Resolution`, optional) :
> +  List of supported resolution and frame rates of the emulated camera
> +    - `width` (`unsigned int`, default=1920): Width of the window resolution.
> +      This needs to be even.
> +    - `height` (`unsigned int`, default=1080): Height of the window resolution.
> +    - `frame_rates` (list of `int`, default=`[30,60]` ): Range of the frame
> +      rate (per second). If the list contains one value, it's the lower bound
> +      and the upper bound. If the list contains two values, the first is the
> +      lower bound and the second is the upper bound. No other number of values
> +      is allowed.
> +- `test_pattern` (`string`): Which test pattern to use as frames. The options
> +  are "bars", "lines". Cannot be set with `frames`.
> +- `frames` (dictionary):
> +  - `path` (`string`): Path to an image, or path to a directory of a series of
> +    images. Cannot be set with `test_pattern`.
> +    - The test patterns are "bars" which means color bars, and "lines" which
> +      means diagonal lines.

Does this bullet point belong to the 'test_pattern' section above rather
than frames ? 

> +    - The path to an image has ".jpg" extension.
> +    - The path to a directory ends with "/". The name of the images in the
> +      directory are "{n}.jpg" with {n} is the sequence of images starting with 0.
> +- `location` (`string`, default="front"): The location of the camera. Support
> +  "front" and "back". This is displayed in qcam camera selection window but

I don't think it's relevant to mention 'qcam' here. Setting the location
defines the location property of the camera which is exposed by the
camera in /any/ application. Not just qcam.


> +  this does not change the output.
> +- `model` (`string`, default="Unknown"): The model name of the camera.
> +  This is displayed in qcam camera selection window but this does not change the output.
> +
> +Check `data/virtual.yaml` as the sample config file.
> +
> +### Implementation
> +
> +`Parser` class provides methods to parse the config file to register cameras
> +in Virtual Pipeline Handler. `parseConfigFile()` is exposed to use in
> +Virtual Pipeline Handler.
> +
> +This is the procedure of the Parser class:
> +1. `parseConfigFile()` parses the config file to `YamlObject` using `YamlParser::parse()`.
> +    - Parse the top level of config file which are the camera ids and look into
> +      each camera properties.
> +2. For each camera, `parseCameraConfigData()` returns a camera with the configuration.
> +    - The methods in the next step fill the data with the pointer to the Camera object.
> +    - If the config file contains invalid configuration, this method returns
> +      nullptr. The camera will be skipped.
> +3. Parse each property and register the data.
> +    - `parseSupportedFormats()`: Parses `supported_formats` in the config, which
> +      contains resolutions and frame rates.
> +    - `parseFrameGenerator()`: Parses `test_pattern` or `frames` 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/config_parser.cpp b/src/libcamera/pipeline/virtual/config_parser.cpp
> new file mode 100644
> index 000000000..467dde485
> --- /dev/null
> +++ b/src/libcamera/pipeline/virtual/config_parser.cpp
> @@ -0,0 +1,263 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Google Inc.
> + *
> + * config_parser.cpp - Virtual cameras helper to parse config file

Drop 'config_parser.cpp - ' We stopped putting file names in their own
files as they bitrot and don't add value

> + */
> +
> +#include "config_parser.h"
> +
> +#include <string.h>
> +#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>>
> +ConfigParser::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->config_.id = cameraId;
> +               ControlInfoMap::Map controls;
> +               /* todo: Check which resolution's frame rate to be reported */
> +               controls[&controls::FrameDurationLimits] =
> +                       ControlInfo(1000000 / data->config_.resolutions[0].frameRates[1],
> +                                   1000000 / data->config_.resolutions[0].frameRates[0]);
> +
> +               std::vector<ControlValue> supportedFaceDetectModes{
> +                       static_cast<int32_t>(controls::draft::FaceDetectModeOff),
> +               };
> +               controls[&controls::draft::FaceDetectMode] = ControlInfo(supportedFaceDetectModes);
> +
> +               data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);
> +               configurations.push_back(std::move(data));
> +       }
> +
> +       return configurations;
> +}
> +
> +std::unique_ptr<VirtualCameraData>
> +ConfigParser::parseCameraConfigData(const YamlObject &cameraConfigData,
> +                                   PipelineHandler *pipe)
> +{
> +       std::vector<VirtualCameraData::Resolution> resolutions;
> +       if (parseSupportedFormats(cameraConfigData, &resolutions))
> +               return nullptr;
> +
> +       std::unique_ptr<VirtualCameraData> data =
> +               std::make_unique<VirtualCameraData>(pipe, resolutions);
> +
> +       if (parseFrameGenerator(cameraConfigData, data.get()))
> +               return nullptr;
> +
> +       if (parseLocation(cameraConfigData, data.get()))
> +               return nullptr;
> +
> +       if (parseModel(cameraConfigData, data.get()))
> +               return nullptr;
> +
> +       return data;
> +}
> +
> +int ConfigParser::parseSupportedFormats(const YamlObject &cameraConfigData,
> +                                       std::vector<VirtualCameraData::Resolution> *resolutions)
> +{
> +       if (cameraConfigData.contains("supported_formats")) {
> +               const YamlObject &supportedResolutions = cameraConfigData["supported_formats"];
> +
> +               for (const YamlObject &supportedResolution : supportedResolutions.asList()) {
> +                       unsigned int width = supportedResolution["width"].get<unsigned int>(1920);
> +                       unsigned int height = supportedResolution["height"].get<unsigned int>(1080);
> +                       if (width == 0 || height == 0) {
> +                               LOG(Virtual, Error) << "Invalid width or/and height";
> +                               return -EINVAL;
> +                       }
> +                       if (width % 2 != 0) {
> +                               LOG(Virtual, Error) << "Invalid width: width needs to be even";
> +                               return -EINVAL;
> +                       }
> +
> +                       std::vector<int64_t> frameRates;
> +                       if (supportedResolution.contains("frame_rates")) {
> +                               auto frameRatesList =
> +                                       supportedResolution["frame_rates"].getList<int>();
> +                               if (!frameRatesList || (frameRatesList->size() != 1 &&
> +                                                       frameRatesList->size() != 2)) {
> +                                       LOG(Virtual, Error) << "Invalid frame_rates: either one or two values";
> +                                       return -EINVAL;
> +                               }
> +
> +                               if (frameRatesList->size() == 2 &&
> +                                   frameRatesList.value()[0] > frameRatesList.value()[1]) {
> +                                       LOG(Virtual, Error) << "frame_rates's first value(lower bound)"
> +                                                           << " is higher than the second value(upper bound)";
> +                                       return -EINVAL;
> +                               }
> +                               /*
> +                                 * Push the min and max framerates. A
> +                                 * single rate is duplicated.
> +                                 */
> +                               frameRates.push_back(frameRatesList.value().front());
> +                               frameRates.push_back(frameRatesList.value().back());
> +                       } else {
> +                               frameRates.push_back(30);
> +                               frameRates.push_back(60);
> +                       }
> +
> +                       resolutions->emplace_back(
> +                               VirtualCameraData::Resolution{ Size{ width, height },
> +                                                              frameRates });
> +               }
> +       } else {
> +               resolutions->emplace_back(
> +                       VirtualCameraData::Resolution{ Size{ 1920, 1080 },
> +                                                      { 30, 60 } });
> +       }
> +
> +       return 0;
> +}
> +
> +int ConfigParser::parseFrameGenerator(const YamlObject &cameraConfigData, VirtualCameraData *data)
> +{
> +       const std::string testPatternKey = "test_pattern";
> +       const std::string framesKey = "frames";
> +       if (cameraConfigData.contains(testPatternKey)) {
> +               if (cameraConfigData.contains(framesKey)) {
> +                       LOG(Virtual, Error) << "A camera should use either "
> +                                           << testPatternKey << " or " << framesKey;
> +                       return -EINVAL;
> +               }
> +
> +               auto testPattern = cameraConfigData[testPatternKey].get<std::string>("");
> +
> +               if (testPattern == "bars") {
> +                       data->config_.frame = TestPattern::ColorBars;
> +               } else if (testPattern == "lines") {
> +                       data->config_.frame = TestPattern::DiagonalLines;
> +               } else {
> +                       LOG(Virtual, Debug) << "Test pattern: " << testPattern
> +                                           << " is not supported";
> +                       return -EINVAL;
> +               }
> +
> +               return 0;
> +       }
> +
> +       const YamlObject &frames = cameraConfigData[framesKey];
> +
> +       /* When there is no frames provided in the config file, use color bar test pattern */
> +       if (!frames) {
> +               data->config_.frame = TestPattern::ColorBars;
> +               return 0;
> +       }
> +
> +       if (!frames.isDictionary()) {
> +               LOG(Virtual, Error) << "'frames' is not a dictionary.";
> +               return -EINVAL;
> +       }
> +
> +       auto path = frames["path"].get<std::string>();
> +
> +       if (!path) {
> +               LOG(Virtual, Error) << "Test pattern or path should be specified.";
> +               return -EINVAL;
> +       }
> +
> +       std::vector<std::filesystem::path> files;
> +
> +       switch (std::filesystem::symlink_status(*path).type()) {
> +       case std::filesystem::file_type::regular:
> +               files.push_back(*path);
> +               break;
> +
> +       case std::filesystem::file_type::directory:
> +               for (const auto &dentry : std::filesystem::directory_iterator{ *path }) {
> +                       if (dentry.is_regular_file())
> +                               files.push_back(dentry.path());
> +               }
> +
> +               std::sort(files.begin(), files.end(), [](const auto &a, const auto &b) {
> +                       return ::strverscmp(a.c_str(), b.c_str()) < 0;
> +               });
> +
> +               if (files.empty()) {
> +                       LOG(Virtual, Error) << "Directory has no files: " << *path;
> +                       return -EINVAL;
> +               }
> +               break;
> +
> +       default:
> +               LOG(Virtual, Error) << "Frame: " << *path << " is not supported";
> +               return -EINVAL;
> +       }
> +
> +       data->config_.frame = ImageFrames{ std::move(files) };
> +
> +       return 0;
> +}
> +
> +int ConfigParser::parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data)
> +{
> +       std::string location = cameraConfigData["location"].get<std::string>("front");
> +
> +       /* Default value is properties::CameraLocationFront */
> +       if (location == "front") {
> +               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";

What about external ? 

I think the control framework provides a map of string to id for the
enum types in fact, so you can do something like:
	

int ConfigParser::parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data)
{
        std::string location = cameraConfigData["location"].get<std::string>("front");

        auto it = properties::LocationNameValueMap.find(location);
        if (it == properties::LocationNameValueMap.end()) {
                LOG(Virtual, Error)
                       << "location: " << location << " is not supported";

                return -EINVAL;
        }

        data->properties_.set(properties::Location, it->second);

        return 0;
}

Which is probably more future proof too if we for some reason add any
other locations.



> +               return -EINVAL;
> +       }
> +
> +       return 0;
> +}
> +
> +int ConfigParser::parseModel(const YamlObject &cameraConfigData, VirtualCameraData *data)
> +{
> +       std::string model = cameraConfigData["model"].get<std::string>("Unknown");
> +
> +       data->properties_.set(properties::Model, model);
> +
> +       return 0;
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/virtual/config_parser.h b/src/libcamera/pipeline/virtual/config_parser.h
> new file mode 100644
> index 000000000..9abba62d0
> --- /dev/null
> +++ b/src/libcamera/pipeline/virtual/config_parser.h
> @@ -0,0 +1,39 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Google Inc.
> + *
> + * config_parser.h - Virtual cameras helper to parse config file

Drop 'config_parser.h - ' from here.

> + */
> +
> +#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 ConfigParser
> +{
> +public:
> +       std::vector<std::unique_ptr<VirtualCameraData>>
> +       parseConfigFile(File &file, PipelineHandler *pipe);
> +
> +private:
> +       std::unique_ptr<VirtualCameraData>
> +       parseCameraConfigData(const YamlObject &cameraConfigData, PipelineHandler *pipe);
> +
> +       int parseSupportedFormats(const YamlObject &cameraConfigData,
> +                                 std::vector<VirtualCameraData::Resolution> *resolutions);
> +       int parseFrameGenerator(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/data/virtual.yaml b/src/libcamera/pipeline/virtual/data/virtual.yaml
> new file mode 100644
> index 000000000..6b73ddf2b
> --- /dev/null
> +++ b/src/libcamera/pipeline/virtual/data/virtual.yaml
> @@ -0,0 +1,36 @@
> +# SPDX-License-Identifier: CC0-1.0
> +%YAML 1.1
> +---
> +"Virtual0":
> +  supported_formats:
> +  - width: 1920
> +    height: 1080
> +    frame_rates:
> +    - 30
> +    - 60
> +  - width: 1680
> +    height: 1050
> +    frame_rates:
> +    - 70
> +    - 80
> +  test_pattern: "lines"
> +  location: "front"
> +  model: "Virtual Video Device"
> +"Virtual1":
> +  supported_formats:
> +  - width: 800
> +    height: 600
> +    frame_rates:
> +    - 60
> +  test_pattern: "bars"
> +  location: "back"
> +  model: "Virtual Video Device1"
> +"Virtual2":
> +  supported_formats:
> +  - width: 400
> +    height: 300
> +  test_pattern: "lines"
> +  location: "front"
> +  model: "Virtual Video Device2"
> +"Virtual3":
> +  test_pattern: "bars"
> diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.cpp b/src/libcamera/pipeline/virtual/image_frame_generator.cpp
> index 04382beb7..36bdc20e5 100644
> --- a/src/libcamera/pipeline/virtual/image_frame_generator.cpp
> +++ b/src/libcamera/pipeline/virtual/image_frame_generator.cpp
> @@ -40,15 +40,7 @@ ImageFrameGenerator::create(ImageFrames &imageFrames)
>          * For each file in the directory, load the image,
>          * convert it to NV12, and store the pointer.
>          */
> -       for (unsigned int i = 0; i < imageFrames.number.value_or(1); i++) {
> -               std::filesystem::path path;
> -               if (!imageFrames.number)
> -                       /* If the path is to an image */
> -                       path = imageFrames.path;
> -               else
> -                       /* If the path is to a directory */
> -                       path = imageFrames.path / (std::to_string(i) + ".jpg");
> -
> +       for (std::filesystem::path path : imageFrames.files) {
>                 File file(path);
>                 if (!file.open(File::OpenModeFlag::ReadOnly)) {
>                         LOG(Virtual, Error) << "Failed to open image file " << file.fileName()
> @@ -88,6 +80,8 @@ ImageFrameGenerator::create(ImageFrames &imageFrames)
>                                         Size(width, height) });
>         }
>  
> +       ASSERT(!imageFrameGenerator->imageFrameDatas_.empty());
> +
>         return imageFrameGenerator;
>  }
>  
> @@ -104,7 +98,7 @@ void ImageFrameGenerator::configure(const Size &size)
>         frameIndex_ = 0;
>         parameter_ = 0;
>  
> -       for (unsigned int i = 0; i < imageFrames_->number.value_or(1); i++) {
> +       for (unsigned int i = 0; i < imageFrameDatas_.size(); i++) {
>                 /* Scale the imageFrameDatas_ to scaledY and scaledUV */
>                 unsigned int halfSizeWidth = (size.width + 1) / 2;
>                 unsigned int halfSizeHeight = (size.height + 1) / 2;
> @@ -139,7 +133,7 @@ int ImageFrameGenerator::generateFrame(const Size &size, const FrameBuffer *buff
>         auto planes = mappedFrameBuffer.planes();
>  
>         /* Loop only around the number of images available */
> -       frameIndex_ %= imageFrames_->number.value_or(1);
> +       frameIndex_ %= imageFrameDatas_.size();
>  
>         /* Write the scaledY and scaledUV to the mapped frame buffer */
>         libyuv::NV12Copy(scaledFrameDatas_[frameIndex_].Y.get(), size.width,
> diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.h b/src/libcamera/pipeline/virtual/image_frame_generator.h
> index a68094a66..8554d647d 100644
> --- a/src/libcamera/pipeline/virtual/image_frame_generator.h
> +++ b/src/libcamera/pipeline/virtual/image_frame_generator.h
> @@ -10,9 +10,9 @@
>  
>  #include <filesystem>
>  #include <memory>
> -#include <optional>
>  #include <stdint.h>
>  #include <sys/types.h>
> +#include <vector>
>  
>  #include "frame_generator.h"
>  
> @@ -20,8 +20,7 @@ namespace libcamera {
>  
>  /* Frame configuration provided by the config file */
>  struct ImageFrames {
> -       std::filesystem::path path;
> -       std::optional<unsigned int> number;
> +       std::vector<std::filesystem::path> files;
>  };
>  
>  class ImageFrameGenerator : public FrameGenerator
> diff --git a/src/libcamera/pipeline/virtual/meson.build b/src/libcamera/pipeline/virtual/meson.build
> index bb38c193c..4786fe2e0 100644
> --- a/src/libcamera/pipeline/virtual/meson.build
> +++ b/src/libcamera/pipeline/virtual/meson.build
> @@ -1,6 +1,7 @@
>  # SPDX-License-Identifier: CC0-1.0
>  
>  libcamera_internal_sources += files([
> +    'config_parser.cpp',
>      'image_frame_generator.cpp',
>      'test_pattern_generator.cpp',
>      'virtual.cpp',
> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp
> index 1ad7417c7..cafd395c3 100644
> --- a/src/libcamera/pipeline/virtual/virtual.cpp
> +++ b/src/libcamera/pipeline/virtual/virtual.cpp
> @@ -36,6 +36,9 @@
>  #include "libcamera/internal/formats.h"
>  #include "libcamera/internal/framebuffer.h"
>  #include "libcamera/internal/pipeline_handler.h"
> +#include "libcamera/internal/yaml_parser.h"
> +
> +#include "pipeline/virtual/config_parser.h"
>  
>  namespace libcamera {
>  
> @@ -54,6 +57,13 @@ uint64_t currentTimestamp()
>  
>  } /* namespace */
>  
> +template<class... Ts>
> +struct overloaded : Ts... {
> +       using Ts::operator()...;
> +};
> +template<class... Ts>
> +overloaded(Ts...) -> overloaded<Ts...>;
> +
>  class VirtualCameraConfiguration : public CameraConfiguration
>  {
>  public:
> @@ -95,7 +105,7 @@ private:
>                 return static_cast<VirtualCameraData *>(camera->_d());
>         }
>  
> -       void initFrameGenerator(Camera *camera);
> +       bool initFrameGenerator(Camera *camera);
>  
>         DmaBufAllocator dmaBufAllocator_;
>  
> @@ -104,15 +114,19 @@ private:
>  
>  VirtualCameraData::VirtualCameraData(PipelineHandler *pipe,
>                                      const std::vector<Resolution> &supportedResolutions)
> -       : Camera::Private(pipe), supportedResolutions_(supportedResolutions)
> +       : Camera::Private(pipe)
>  {
> -       for (const auto &resolution : supportedResolutions_) {
> -               if (minResolutionSize_.isNull() || minResolutionSize_ > resolution.size)
> -                       minResolutionSize_ = resolution.size;
> +       config_.resolutions = supportedResolutions;
> +       for (const auto &resolution : config_.resolutions) {
> +               if (config_.minResolutionSize.isNull() || config_.minResolutionSize > resolution.size)
> +                       config_.minResolutionSize = resolution.size;
>  
> -               maxResolutionSize_ = std::max(maxResolutionSize_, resolution.size);
> +               config_.maxResolutionSize = std::max(config_.maxResolutionSize, resolution.size);
>         }
>  
> +       properties_.set(properties::PixelArrayActiveAreas,
> +                       { Rectangle(config_.maxResolutionSize) });
> +
>         /* \todo Support multiple streams and pass multi_stream_test */
>         streamConfigs_.resize(kMaxStream);
>  }
> @@ -140,7 +154,7 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()
>         for (StreamConfiguration &cfg : config_) {
>                 bool adjusted = false;
>                 bool found = false;
> -               for (const auto &resolution : data_->supportedResolutions_) {
> +               for (const auto &resolution : data_->config_.resolutions) {
>                         if (resolution.size.width == cfg.size.width &&
>                             resolution.size.height == cfg.size.height) {
>                                 found = true;
> @@ -155,7 +169,7 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()
>                          * Defining the default logic in PipelineHandler to
>                          * find the closest resolution would be nice.
>                          */
> -                       cfg.size = data_->maxResolutionSize_;
> +                       cfg.size = data_->config_.maxResolutionSize;
>                         status = Adjusted;
>                         adjusted = true;
>                 }
> @@ -224,12 +238,12 @@ PipelineHandlerVirtual::generateConfiguration(Camera *camera,
>  
>                 std::map<PixelFormat, std::vector<SizeRange>> streamFormats;
>                 PixelFormat pixelFormat = formats::NV12;
> -               streamFormats[pixelFormat] = { { data->minResolutionSize_,
> -                                                data->maxResolutionSize_ } };
> +               streamFormats[pixelFormat] = { { data->config_.minResolutionSize,
> +                                                data->config_.maxResolutionSize } };
>                 StreamFormats formats(streamFormats);
>                 StreamConfiguration cfg(formats);
>                 cfg.pixelFormat = pixelFormat;
> -               cfg.size = data->maxResolutionSize_;
> +               cfg.size = data->config_.maxResolutionSize;
>                 cfg.bufferCount = VirtualCameraConfiguration::kBufferCount;
>  
>                 config->addConfiguration(cfg);
> @@ -246,6 +260,7 @@ int PipelineHandlerVirtual::configure(Camera *camera,
>         VirtualCameraData *data = cameraData(camera);
>         for (auto [i, c] : utils::enumerate(*config)) {
>                 c.setStream(&data->streamConfigs_[i].stream);
> +               /* Start reading the images/generating test patterns */
>                 data->streamConfigs_[i].frameGenerator->configure(c.size);
>         }
>  
> @@ -315,56 +330,67 @@ bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator
>  
>         created_ = true;
>  
> -       /* \todo Add virtual cameras according to a config file. */
> -
> -       std::vector<VirtualCameraData::Resolution> supportedResolutions;
> -       supportedResolutions.resize(2);
> -       supportedResolutions[0] = { .size = Size(1920, 1080), .frameRates = { 30 } };
> -       supportedResolutions[1] = { .size = Size(1280, 720), .frameRates = { 30 } };
> -
> -       std::unique_ptr<VirtualCameraData> data =
> -               std::make_unique<VirtualCameraData>(this, supportedResolutions);
> -
> -       data->properties_.set(properties::Location, properties::CameraLocationFront);
> -       data->properties_.set(properties::Model, "Virtual Video Device");
> -       data->properties_.set(properties::PixelArrayActiveAreas, { Rectangle(Size(1920, 1080)) });
> -
> -       /* \todo Set FrameDurationLimits based on config. */
> -       ControlInfoMap::Map controls;
> -       int64_t min_frame_duration = 33333, max_frame_duration = 33333;
> -       controls[&controls::FrameDurationLimits] = ControlInfo(min_frame_duration, max_frame_duration);
> -       std::vector<ControlValue> supportedFaceDetectModes{
> -               static_cast<int32_t>(controls::draft::FaceDetectModeOff),
> -       };
> -       controls[&controls::draft::FaceDetectMode] = ControlInfo(supportedFaceDetectModes);
> -       data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);
> -
> -       /* Create and register the camera. */
> -       std::set<Stream *> streams;
> -       for (auto &streamConfig : data->streamConfigs_)
> -               streams.insert(&streamConfig.stream);
> +       File file(configurationFile("virtual", "virtual.yaml"));

This tells me that the sample virtual.yaml file should probably be
installed in the libcamera data path...

This version doesn't currently apply cleanly to master, but I did have
an branch with it on - and it rebased easily.

But it needs a new version posting at least to solve the SoB issues and
so with the above tackled you can add 

Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


> +       bool isOpen = file.open(File::OpenModeFlag::ReadOnly);
> +       if (!isOpen) {
> +               LOG(Virtual, Error) << "Failed to open config file: " << file.fileName();
> +               return false;
> +       }
>  
> -       const std::string id = "Virtual0";
> -       std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams);
> +       ConfigParser 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;
> +       }
>  
> -       initFrameGenerator(camera.get());
> +       /* Configure and register cameras with configData */
> +       for (auto &data : configData) {
> +               std::set<Stream *> streams;
> +               for (auto &streamConfig : data->streamConfigs_)
> +                       streams.insert(&streamConfig.stream);
> +               std::string id = data->config_.id;
> +               std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams);
> +
> +               if (!initFrameGenerator(camera.get())) {
> +                       LOG(Virtual, Error) << "Failed to initialize frame "
> +                                           << "generator for camera: " << id;
> +                       continue;
> +               }
>  
> -       registerCamera(std::move(camera));
> +               registerCamera(std::move(camera));
> +       }
>  
>         resetCreated_ = true;
>  
>         return true;
>  }
>  
> -void PipelineHandlerVirtual::initFrameGenerator(Camera *camera)
> +bool PipelineHandlerVirtual::initFrameGenerator(Camera *camera)
>  {
>         auto data = cameraData(camera);
> -       for (auto &streamConfig : data->streamConfigs_) {
> -               if (data->testPattern_ == TestPattern::DiagonalLines)
> -                       streamConfig.frameGenerator = std::make_unique<DiagonalLinesGenerator>();
> -               else
> -                       streamConfig.frameGenerator = std::make_unique<ColorBarsGenerator>();
> -       }
> +       auto &frame = data->config_.frame;
> +       std::visit(overloaded{
> +                          [&](TestPattern &testPattern) {
> +                                  for (auto &streamConfig : data->streamConfigs_) {
> +                                          if (testPattern == TestPattern::DiagonalLines)
> +                                                  streamConfig.frameGenerator = std::make_unique<DiagonalLinesGenerator>();
> +                                          else
> +                                                  streamConfig.frameGenerator = std::make_unique<ColorBarsGenerator>();
> +                                  }
> +                          },
> +                          [&](ImageFrames &imageFrames) {
> +                                  for (auto &streamConfig : data->streamConfigs_)
> +                                          streamConfig.frameGenerator = ImageFrameGenerator::create(imageFrames);
> +                          } },
> +                  frame);
> +
> +       for (auto &streamConfig : data->streamConfigs_)
> +               if (!streamConfig.frameGenerator)
> +                       return false;
> +
> +       return true;
>  }
>  
>  REGISTER_PIPELINE_HANDLER(PipelineHandlerVirtual, "virtual")
> diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h
> index 0b15f8d9c..8c4d6ae64 100644
> --- a/src/libcamera/pipeline/virtual/virtual.h
> +++ b/src/libcamera/pipeline/virtual/virtual.h
> @@ -7,6 +7,8 @@
>  
>  #pragma once
>  
> +#include <string>
> +#include <variant>
>  #include <vector>
>  
>  #include <libcamera/geometry.h>
> @@ -15,10 +17,14 @@
>  #include "libcamera/internal/camera.h"
>  #include "libcamera/internal/pipeline_handler.h"
>  
> +#include "frame_generator.h"
> +#include "image_frame_generator.h"
>  #include "test_pattern_generator.h"
>  
>  namespace libcamera {
>  
> +using VirtualFrame = std::variant<TestPattern, ImageFrames>;
> +
>  class VirtualCameraData : public Camera::Private
>  {
>  public:
> @@ -26,23 +32,28 @@ public:
>  
>         struct Resolution {
>                 Size size;
> -               std::vector<int> frameRates;
> +               std::vector<int64_t> frameRates;
>         };
>         struct StreamConfig {
>                 Stream stream;
>                 std::unique_ptr<FrameGenerator> frameGenerator;
>         };
> +       /* The config file is parsed to the Configuration struct */
> +       struct Configuration {
> +               std::string id;
> +               std::vector<Resolution> resolutions;
> +               VirtualFrame frame;
> +
> +               Size maxResolutionSize;
> +               Size minResolutionSize;
> +       };
>  
>         VirtualCameraData(PipelineHandler *pipe,
>                           const std::vector<Resolution> &supportedResolutions);
>  
>         ~VirtualCameraData() = default;
>  
> -       TestPattern testPattern_ = TestPattern::ColorBars;
> -
> -       const std::vector<Resolution> supportedResolutions_;
> -       Size maxResolutionSize_;
> -       Size minResolutionSize_;
> +       Configuration config_;
>  
>         std::vector<StreamConfig> streamConfigs_;
>  };
> -- 
> 2.47.0.rc0.187.ge670bccf7e-goog
>


More information about the libcamera-devel mailing list