[{"id":31632,"web_url":"https://patchwork.libcamera.org/comment/31632/","msgid":"","date":"2024-10-09T07:57:02","subject":"Re: [PATCH 15 6/7] libcamera: virtual: Read config and register\n\tcameras based on the config","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Friendly ping: This is the last patch that hasn't got a review tag in\nthe series. Could we finish the review and merge the series recently?\nI can upload the last version with commit messages fixed with SoBs.\n\nBR,\nHarvey\n\nOn Fri, Oct 4, 2024 at 6:00 PM Harvey Yang wrote:\n>\n> This patch introduces the configuration file for Virtual Pipeline\n> Handler. The config file is written in yaml, and the format is\n> documented in `README.md`.\n>\n> The config file will define the camera with IDs, supported formats and\n> image sources, etc. In the default config file, only Test Patterns are\n> used. Developers can use real images loading if desired.\n>\n> Signed-off-by: Konami Shu \n> Co-developed-by: Harvey Yang \n> Co-developed-by: Yunke Cao \n> Co-developed-by: Tomasz Figa \n> ---\n> src/libcamera/pipeline/virtual/README.md | 65 +++++\n> .../pipeline/virtual/config_parser.cpp | 263 ++++++++++++++++++\n> .../pipeline/virtual/config_parser.h | 39 +++\n> .../pipeline/virtual/data/virtual.yaml | 36 +++\n> .../virtual/image_frame_generator.cpp | 16 +-\n> .../pipeline/virtual/image_frame_generator.h | 5 +-\n> src/libcamera/pipeline/virtual/meson.build | 1 +\n> src/libcamera/pipeline/virtual/virtual.cpp | 126 +++++----\n> src/libcamera/pipeline/virtual/virtual.h | 23 +-\n> 9 files changed, 504 insertions(+), 70 deletions(-)\n> create mode 100644 src/libcamera/pipeline/virtual/README.md\n> create mode 100644 src/libcamera/pipeline/virtual/config_parser.cpp\n> create mode 100644 src/libcamera/pipeline/virtual/config_parser.h\n> create mode 100644 src/libcamera/pipeline/virtual/data/virtual.yaml\n>\n> diff --git a/src/libcamera/pipeline/virtual/README.md b/src/libcamera/pipeline/virtual/README.md\n> new file mode 100644\n> index 000000000..5801e79b5\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/virtual/README.md\n> @@ -0,0 +1,65 @@\n> +# Virtual Pipeline Handler\n> +\n> +Virtual pipeline handler emulates fake external camera(s) on ChromeOS for testing.\n> +\n> +## Parse config file and register cameras\n> +\n> +- The config file is located at `src/libcamera/pipeline/virtual/data/virtual.yaml`\n> +\n> +### Config File Format\n> +The config file contains the information about cameras' properties to register.\n> +The config file should be a yaml file with dictionary of the cameraIds\n> +associated with their properties as top level. The default value will be applied\n> +when any property is empty.\n> +\n> +Each camera block is a dictionary, containing the following keys:\n> +- `supported_formats` (list of `VirtualCameraData::Resolution`, optional) :\n> + List of supported resolution and frame rates of the emulated camera\n> + - `width` (`unsigned int`, default=1920): Width of the window resolution.\n> + This needs to be even.\n> + - `height` (`unsigned int`, default=1080): Height of the window resolution.\n> + - `frame_rates` (list of `int`, default=`[30,60]` ): Range of the frame\n> + rate (per second). If the list contains one value, it's the lower bound\n> + and the upper bound. If the list contains two values, the first is the\n> + lower bound and the second is the upper bound. No other number of values\n> + is allowed.\n> +- `test_pattern` (`string`): Which test pattern to use as frames. The options\n> + are \"bars\", \"lines\". Cannot be set with `frames`.\n> +- `frames` (dictionary):\n> + - `path` (`string`): Path to an image, or path to a directory of a series of\n> + images. Cannot be set with `test_pattern`.\n> + - The test patterns are \"bars\" which means color bars, and \"lines\" which\n> + means diagonal lines.\n> + - The path to an image has \".jpg\" extension.\n> + - The path to a directory ends with \"/\". The name of the images in the\n> + directory are \"{n}.jpg\" with {n} is the sequence of images starting with 0.\n> +- `location` (`string`, default=\"front\"): The location of the camera. Support\n> + \"front\" and \"back\". This is displayed in qcam camera selection window but\n> + this does not change the output.\n> +- `model` (`string`, default=\"Unknown\"): The model name of the camera.\n> + This is displayed in qcam camera selection window but this does not change the output.\n> +\n> +Check `data/virtual.yaml` as the sample config file.\n> +\n> +### Implementation\n> +\n> +`Parser` class provides methods to parse the config file to register cameras\n> +in Virtual Pipeline Handler. `parseConfigFile()` is exposed to use in\n> +Virtual Pipeline Handler.\n> +\n> +This is the procedure of the Parser class:\n> +1. `parseConfigFile()` parses the config file to `YamlObject` using `YamlParser::parse()`.\n> + - Parse the top level of config file which are the camera ids and look into\n> + each camera properties.\n> +2. For each camera, `parseCameraConfigData()` returns a camera with the configuration.\n> + - The methods in the next step fill the data with the pointer to the Camera object.\n> + - If the config file contains invalid configuration, this method returns\n> + nullptr. The camera will be skipped.\n> +3. Parse each property and register the data.\n> + - `parseSupportedFormats()`: Parses `supported_formats` in the config, which\n> + contains resolutions and frame rates.\n> + - `parseFrameGenerator()`: Parses `test_pattern` or `frames` in the config.\n> + - `parseLocation()`: Parses `location` in the config.\n> + - `parseModel()`: Parses `model` in the config.\n> +4. Back to `parseConfigFile()` and append the camera configuration.\n> +5. Returns a list of camera configurations.\n> diff --git a/src/libcamera/pipeline/virtual/config_parser.cpp b/src/libcamera/pipeline/virtual/config_parser.cpp\n> new file mode 100644\n> index 000000000..467dde485\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/virtual/config_parser.cpp\n> @@ -0,0 +1,263 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Google Inc.\n> + *\n> + * config_parser.cpp - Virtual cameras helper to parse config file\n> + */\n> +\n> +#include \"config_parser.h\"\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/pipeline_handler.h\"\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"virtual.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(Virtual)\n> +\n> +std::vector>\n> +ConfigParser::parseConfigFile(File &file, PipelineHandler *pipe)\n> +{\n> + std::vector> configurations;\n> +\n> + std::unique_ptr cameras = YamlParser::parse(file);\n> + if (!cameras) {\n> + LOG(Virtual, Error) << \"Failed to pass config file.\";\n> + return configurations;\n> + }\n> +\n> + if (!cameras->isDictionary()) {\n> + LOG(Virtual, Error) << \"Config file is not a dictionary at the top level.\";\n> + return configurations;\n> + }\n> +\n> + /* Look into the configuration of each camera */\n> + for (const auto &[cameraId, cameraConfigData] : cameras->asDict()) {\n> + std::unique_ptr data =\n> + parseCameraConfigData(cameraConfigData, pipe);\n> + /* Parse configData to data */\n> + if (!data) {\n> + /* Skip the camera if it has invalid config */\n> + LOG(Virtual, Error) << \"Failed to parse config of the camera: \"\n> + << cameraId;\n> + continue;\n> + }\n> +\n> + data->config_.id = cameraId;\n> + ControlInfoMap::Map controls;\n> + /* todo: Check which resolution's frame rate to be reported */\n> + controls[&controls::FrameDurationLimits] =\n> + ControlInfo(1000000 / data->config_.resolutions[0].frameRates[1],\n> + 1000000 / data->config_.resolutions[0].frameRates[0]);\n> +\n> + std::vector supportedFaceDetectModes{\n> + static_cast(controls::draft::FaceDetectModeOff),\n> + };\n> + controls[&controls::draft::FaceDetectMode] = ControlInfo(supportedFaceDetectModes);\n> +\n> + data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);\n> + configurations.push_back(std::move(data));\n> + }\n> +\n> + return configurations;\n> +}\n> +\n> +std::unique_ptr\n> +ConfigParser::parseCameraConfigData(const YamlObject &cameraConfigData,\n> + PipelineHandler *pipe)\n> +{\n> + std::vector resolutions;\n> + if (parseSupportedFormats(cameraConfigData, &resolutions))\n> + return nullptr;\n> +\n> + std::unique_ptr data =\n> + std::make_unique(pipe, resolutions);\n> +\n> + if (parseFrameGenerator(cameraConfigData, data.get()))\n> + return nullptr;\n> +\n> + if (parseLocation(cameraConfigData, data.get()))\n> + return nullptr;\n> +\n> + if (parseModel(cameraConfigData, data.get()))\n> + return nullptr;\n> +\n> + return data;\n> +}\n> +\n> +int ConfigParser::parseSupportedFormats(const YamlObject &cameraConfigData,\n> + std::vector *resolutions)\n> +{\n> + if (cameraConfigData.contains(\"supported_formats\")) {\n> + const YamlObject &supportedResolutions = cameraConfigData[\"supported_formats\"];\n> +\n> + for (const YamlObject &supportedResolution : supportedResolutions.asList()) {\n> + unsigned int width = supportedResolution[\"width\"].get(1920);\n> + unsigned int height = supportedResolution[\"height\"].get(1080);\n> + if (width == 0 || height == 0) {\n> + LOG(Virtual, Error) << \"Invalid width or/and height\";\n> + return -EINVAL;\n> + }\n> + if (width % 2 != 0) {\n> + LOG(Virtual, Error) << \"Invalid width: width needs to be even\";\n> + return -EINVAL;\n> + }\n> +\n> + std::vector frameRates;\n> + if (supportedResolution.contains(\"frame_rates\")) {\n> + auto frameRatesList =\n> + supportedResolution[\"frame_rates\"].getList();\n> + if (!frameRatesList || (frameRatesList->size() != 1 &&\n> + frameRatesList->size() != 2)) {\n> + LOG(Virtual, Error) << \"Invalid frame_rates: either one or two values\";\n> + return -EINVAL;\n> + }\n> +\n> + if (frameRatesList->size() == 2 &&\n> + frameRatesList.value()[0] > frameRatesList.value()[1]) {\n> + LOG(Virtual, Error) << \"frame_rates's first value(lower bound)\"\n> + << \" is higher than the second value(upper bound)\";\n> + return -EINVAL;\n> + }\n> + /*\n> + * Push the min and max framerates. A\n> + * single rate is duplicated.\n> + */\n> + frameRates.push_back(frameRatesList.value().front());\n> + frameRates.push_back(frameRatesList.value().back());\n> + } else {\n> + frameRates.push_back(30);\n> + frameRates.push_back(60);\n> + }\n> +\n> + resolutions->emplace_back(\n> + VirtualCameraData::Resolution{ Size{ width, height },\n> + frameRates });\n> + }\n> + } else {\n> + resolutions->emplace_back(\n> + VirtualCameraData::Resolution{ Size{ 1920, 1080 },\n> + { 30, 60 } });\n> + }\n> +\n> + return 0;\n> +}\n> +\n> +int ConfigParser::parseFrameGenerator(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> +{\n> + const std::string testPatternKey = \"test_pattern\";\n> + const std::string framesKey = \"frames\";\n> + if (cameraConfigData.contains(testPatternKey)) {\n> + if (cameraConfigData.contains(framesKey)) {\n> + LOG(Virtual, Error) << \"A camera should use either \"\n> + << testPatternKey << \" or \" << framesKey;\n> + return -EINVAL;\n> + }\n> +\n> + auto testPattern = cameraConfigData[testPatternKey].get(\"\");\n> +\n> + if (testPattern == \"bars\") {\n> + data->config_.frame = TestPattern::ColorBars;\n> + } else if (testPattern == \"lines\") {\n> + data->config_.frame = TestPattern::DiagonalLines;\n> + } else {\n> + LOG(Virtual, Debug) << \"Test pattern: \" << testPattern\n> + << \" is not supported\";\n> + return -EINVAL;\n> + }\n> +\n> + return 0;\n> + }\n> +\n> + const YamlObject &frames = cameraConfigData[framesKey];\n> +\n> + /* When there is no frames provided in the config file, use color bar test pattern */\n> + if (!frames) {\n> + data->config_.frame = TestPattern::ColorBars;\n> + return 0;\n> + }\n> +\n> + if (!frames.isDictionary()) {\n> + LOG(Virtual, Error) << \"'frames' is not a dictionary.\";\n> + return -EINVAL;\n> + }\n> +\n> + auto path = frames[\"path\"].get();\n> +\n> + if (!path) {\n> + LOG(Virtual, Error) << \"Test pattern or path should be specified.\";\n> + return -EINVAL;\n> + }\n> +\n> + std::vector files;\n> +\n> + switch (std::filesystem::symlink_status(*path).type()) {\n> + case std::filesystem::file_type::regular:\n> + files.push_back(*path);\n> + break;\n> +\n> + case std::filesystem::file_type::directory:\n> + for (const auto &dentry : std::filesystem::directory_iterator{ *path }) {\n> + if (dentry.is_regular_file())\n> + files.push_back(dentry.path());\n> + }\n> +\n> + std::sort(files.begin(), files.end(), [](const auto &a, const auto &b) {\n> + return ::strverscmp(a.c_str(), b.c_str()) < 0;\n> + });\n> +\n> + if (files.empty()) {\n> + LOG(Virtual, Error) << \"Directory has no files: \" << *path;\n> + return -EINVAL;\n> + }\n> + break;\n> +\n> + default:\n> + LOG(Virtual, Error) << \"Frame: \" << *path << \" is not supported\";\n> + return -EINVAL;\n> + }\n> +\n> + data->config_.frame = ImageFrames{ std::move(files) };\n> +\n> + return 0;\n> +}\n> +\n> +int ConfigParser::parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> +{\n> + std::string location = cameraConfigData[\"location\"].get(\"front\");\n> +\n> + /* Default value is properties::CameraLocationFront */\n> + if (location == \"front\") {\n> + data->properties_.set(properties::Location,\n> + properties::CameraLocationFront);\n> + } else if (location == \"back\") {\n> + data->properties_.set(properties::Location,\n> + properties::CameraLocationBack);\n> + } else {\n> + LOG(Virtual, Error) << \"location: \" << location\n> + << \" is not supported\";\n> + return -EINVAL;\n> + }\n> +\n> + return 0;\n> +}\n> +\n> +int ConfigParser::parseModel(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> +{\n> + std::string model = cameraConfigData[\"model\"].get(\"Unknown\");\n> +\n> + data->properties_.set(properties::Model, model);\n> +\n> + return 0;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/virtual/config_parser.h b/src/libcamera/pipeline/virtual/config_parser.h\n> new file mode 100644\n> index 000000000..9abba62d0\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/virtual/config_parser.h\n> @@ -0,0 +1,39 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Google Inc.\n> + *\n> + * config_parser.h - Virtual cameras helper to parse config file\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/pipeline_handler.h\"\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"virtual.h\"\n> +\n> +namespace libcamera {\n> +\n> +class ConfigParser\n> +{\n> +public:\n> + std::vector>\n> + parseConfigFile(File &file, PipelineHandler *pipe);\n> +\n> +private:\n> + std::unique_ptr\n> + parseCameraConfigData(const YamlObject &cameraConfigData, PipelineHandler *pipe);\n> +\n> + int parseSupportedFormats(const YamlObject &cameraConfigData,\n> + std::vector *resolutions);\n> + int parseFrameGenerator(const YamlObject &cameraConfigData, VirtualCameraData *data);\n> + int parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data);\n> + int parseModel(const YamlObject &cameraConfigData, VirtualCameraData *data);\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/virtual/data/virtual.yaml b/src/libcamera/pipeline/virtual/data/virtual.yaml\n> new file mode 100644\n> index 000000000..6b73ddf2b\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/virtual/data/virtual.yaml\n> @@ -0,0 +1,36 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +%YAML 1.1\n> +---\n> +\"Virtual0\":\n> + supported_formats:\n> + - width: 1920\n> + height: 1080\n> + frame_rates:\n> + - 30\n> + - 60\n> + - width: 1680\n> + height: 1050\n> + frame_rates:\n> + - 70\n> + - 80\n> + test_pattern: \"lines\"\n> + location: \"front\"\n> + model: \"Virtual Video Device\"\n> +\"Virtual1\":\n> + supported_formats:\n> + - width: 800\n> + height: 600\n> + frame_rates:\n> + - 60\n> + test_pattern: \"bars\"\n> + location: \"back\"\n> + model: \"Virtual Video Device1\"\n> +\"Virtual2\":\n> + supported_formats:\n> + - width: 400\n> + height: 300\n> + test_pattern: \"lines\"\n> + location: \"front\"\n> + model: \"Virtual Video Device2\"\n> +\"Virtual3\":\n> + test_pattern: \"bars\"\n> diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.cpp b/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> index 04382beb7..36bdc20e5 100644\n> --- a/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> +++ b/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> @@ -40,15 +40,7 @@ ImageFrameGenerator::create(ImageFrames &imageFrames)\n> * For each file in the directory, load the image,\n> * convert it to NV12, and store the pointer.\n> */\n> - for (unsigned int i = 0; i < imageFrames.number.value_or(1); i++) {\n> - std::filesystem::path path;\n> - if (!imageFrames.number)\n> - /* If the path is to an image */\n> - path = imageFrames.path;\n> - else\n> - /* If the path is to a directory */\n> - path = imageFrames.path / (std::to_string(i) + \".jpg\");\n> -\n> + for (std::filesystem::path path : imageFrames.files) {\n> File file(path);\n> if (!file.open(File::OpenModeFlag::ReadOnly)) {\n> LOG(Virtual, Error) << \"Failed to open image file \" << file.fileName()\n> @@ -88,6 +80,8 @@ ImageFrameGenerator::create(ImageFrames &imageFrames)\n> Size(width, height) });\n> }\n>\n> + ASSERT(!imageFrameGenerator->imageFrameDatas_.empty());\n> +\n> return imageFrameGenerator;\n> }\n>\n> @@ -104,7 +98,7 @@ void ImageFrameGenerator::configure(const Size &size)\n> frameIndex_ = 0;\n> parameter_ = 0;\n>\n> - for (unsigned int i = 0; i < imageFrames_->number.value_or(1); i++) {\n> + for (unsigned int i = 0; i < imageFrameDatas_.size(); i++) {\n> /* Scale the imageFrameDatas_ to scaledY and scaledUV */\n> unsigned int halfSizeWidth = (size.width + 1) / 2;\n> unsigned int halfSizeHeight = (size.height + 1) / 2;\n> @@ -139,7 +133,7 @@ int ImageFrameGenerator::generateFrame(const Size &size, const FrameBuffer *buff\n> auto planes = mappedFrameBuffer.planes();\n>\n> /* Loop only around the number of images available */\n> - frameIndex_ %= imageFrames_->number.value_or(1);\n> + frameIndex_ %= imageFrameDatas_.size();\n>\n> /* Write the scaledY and scaledUV to the mapped frame buffer */\n> libyuv::NV12Copy(scaledFrameDatas_[frameIndex_].Y.get(), size.width,\n> diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.h b/src/libcamera/pipeline/virtual/image_frame_generator.h\n> index a68094a66..8554d647d 100644\n> --- a/src/libcamera/pipeline/virtual/image_frame_generator.h\n> +++ b/src/libcamera/pipeline/virtual/image_frame_generator.h\n> @@ -10,9 +10,9 @@\n>\n> #include \n> #include \n> -#include \n> #include \n> #include \n> +#include \n>\n> #include \"frame_generator.h\"\n>\n> @@ -20,8 +20,7 @@ namespace libcamera {\n>\n> /* Frame configuration provided by the config file */\n> struct ImageFrames {\n> - std::filesystem::path path;\n> - std::optional number;\n> + std::vector files;\n> };\n>\n> class ImageFrameGenerator : public FrameGenerator\n> diff --git a/src/libcamera/pipeline/virtual/meson.build b/src/libcamera/pipeline/virtual/meson.build\n> index bb38c193c..4786fe2e0 100644\n> --- a/src/libcamera/pipeline/virtual/meson.build\n> +++ b/src/libcamera/pipeline/virtual/meson.build\n> @@ -1,6 +1,7 @@\n> # SPDX-License-Identifier: CC0-1.0\n>\n> libcamera_internal_sources += files([\n> + 'config_parser.cpp',\n> 'image_frame_generator.cpp',\n> 'test_pattern_generator.cpp',\n> 'virtual.cpp',\n> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n> index 1ad7417c7..cafd395c3 100644\n> --- a/src/libcamera/pipeline/virtual/virtual.cpp\n> +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n> @@ -36,6 +36,9 @@\n> #include \"libcamera/internal/formats.h\"\n> #include \"libcamera/internal/framebuffer.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"pipeline/virtual/config_parser.h\"\n>\n> namespace libcamera {\n>\n> @@ -54,6 +57,13 @@ uint64_t currentTimestamp()\n>\n> } /* namespace */\n>\n> +template\n> +struct overloaded : Ts... {\n> + using Ts::operator()...;\n> +};\n> +template\n> +overloaded(Ts...) -> overloaded;\n> +\n> class VirtualCameraConfiguration : public CameraConfiguration\n> {\n> public:\n> @@ -95,7 +105,7 @@ private:\n> return static_cast(camera->_d());\n> }\n>\n> - void initFrameGenerator(Camera *camera);\n> + bool initFrameGenerator(Camera *camera);\n>\n> DmaBufAllocator dmaBufAllocator_;\n>\n> @@ -104,15 +114,19 @@ private:\n>\n> VirtualCameraData::VirtualCameraData(PipelineHandler *pipe,\n> const std::vector &supportedResolutions)\n> - : Camera::Private(pipe), supportedResolutions_(supportedResolutions)\n> + : Camera::Private(pipe)\n> {\n> - for (const auto &resolution : supportedResolutions_) {\n> - if (minResolutionSize_.isNull() || minResolutionSize_ > resolution.size)\n> - minResolutionSize_ = resolution.size;\n> + config_.resolutions = supportedResolutions;\n> + for (const auto &resolution : config_.resolutions) {\n> + if (config_.minResolutionSize.isNull() || config_.minResolutionSize > resolution.size)\n> + config_.minResolutionSize = resolution.size;\n>\n> - maxResolutionSize_ = std::max(maxResolutionSize_, resolution.size);\n> + config_.maxResolutionSize = std::max(config_.maxResolutionSize, resolution.size);\n> }\n>\n> + properties_.set(properties::PixelArrayActiveAreas,\n> + { Rectangle(config_.maxResolutionSize) });\n> +\n> /* \\todo Support multiple streams and pass multi_stream_test */\n> streamConfigs_.resize(kMaxStream);\n> }\n> @@ -140,7 +154,7 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()\n> for (StreamConfiguration &cfg : config_) {\n> bool adjusted = false;\n> bool found = false;\n> - for (const auto &resolution : data_->supportedResolutions_) {\n> + for (const auto &resolution : data_->config_.resolutions) {\n> if (resolution.size.width == cfg.size.width &&\n> resolution.size.height == cfg.size.height) {\n> found = true;\n> @@ -155,7 +169,7 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()\n> * Defining the default logic in PipelineHandler to\n> * find the closest resolution would be nice.\n> */\n> - cfg.size = data_->maxResolutionSize_;\n> + cfg.size = data_->config_.maxResolutionSize;\n> status = Adjusted;\n> adjusted = true;\n> }\n> @@ -224,12 +238,12 @@ PipelineHandlerVirtual::generateConfiguration(Camera *camera,\n>\n> std::map> streamFormats;\n> PixelFormat pixelFormat = formats::NV12;\n> - streamFormats[pixelFormat] = { { data->minResolutionSize_,\n> - data->maxResolutionSize_ } };\n> + streamFormats[pixelFormat] = { { data->config_.minResolutionSize,\n> + data->config_.maxResolutionSize } };\n> StreamFormats formats(streamFormats);\n> StreamConfiguration cfg(formats);\n> cfg.pixelFormat = pixelFormat;\n> - cfg.size = data->maxResolutionSize_;\n> + cfg.size = data->config_.maxResolutionSize;\n> cfg.bufferCount = VirtualCameraConfiguration::kBufferCount;\n>\n> config->addConfiguration(cfg);\n> @@ -246,6 +260,7 @@ int PipelineHandlerVirtual::configure(Camera *camera,\n> VirtualCameraData *data = cameraData(camera);\n> for (auto [i, c] : utils::enumerate(*config)) {\n> c.setStream(&data->streamConfigs_[i].stream);\n> + /* Start reading the images/generating test patterns */\n> data->streamConfigs_[i].frameGenerator->configure(c.size);\n> }\n>\n> @@ -315,56 +330,67 @@ bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator\n>\n> created_ = true;\n>\n> - /* \\todo Add virtual cameras according to a config file. */\n> -\n> - std::vector supportedResolutions;\n> - supportedResolutions.resize(2);\n> - supportedResolutions[0] = { .size = Size(1920, 1080), .frameRates = { 30 } };\n> - supportedResolutions[1] = { .size = Size(1280, 720), .frameRates = { 30 } };\n> -\n> - std::unique_ptr data =\n> - std::make_unique(this, supportedResolutions);\n> -\n> - data->properties_.set(properties::Location, properties::CameraLocationFront);\n> - data->properties_.set(properties::Model, \"Virtual Video Device\");\n> - data->properties_.set(properties::PixelArrayActiveAreas, { Rectangle(Size(1920, 1080)) });\n> -\n> - /* \\todo Set FrameDurationLimits based on config. */\n> - ControlInfoMap::Map controls;\n> - int64_t min_frame_duration = 33333, max_frame_duration = 33333;\n> - controls[&controls::FrameDurationLimits] = ControlInfo(min_frame_duration, max_frame_duration);\n> - std::vector supportedFaceDetectModes{\n> - static_cast(controls::draft::FaceDetectModeOff),\n> - };\n> - controls[&controls::draft::FaceDetectMode] = ControlInfo(supportedFaceDetectModes);\n> - data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);\n> -\n> - /* Create and register the camera. */\n> - std::set streams;\n> - for (auto &streamConfig : data->streamConfigs_)\n> - streams.insert(&streamConfig.stream);\n> + File file(configurationFile(\"virtual\", \"virtual.yaml\"));\n> + bool isOpen = file.open(File::OpenModeFlag::ReadOnly);\n> + if (!isOpen) {\n> + LOG(Virtual, Error) << \"Failed to open config file: \" << file.fileName();\n> + return false;\n> + }\n>\n> - const std::string id = \"Virtual0\";\n> - std::shared_ptr camera = Camera::create(std::move(data), id, streams);\n> + ConfigParser parser;\n> + auto configData = parser.parseConfigFile(file, this);\n> + if (configData.size() == 0) {\n> + LOG(Virtual, Error) << \"Failed to parse any cameras from the config file: \"\n> + << file.fileName();\n> + return false;\n> + }\n>\n> - initFrameGenerator(camera.get());\n> + /* Configure and register cameras with configData */\n> + for (auto &data : configData) {\n> + std::set streams;\n> + for (auto &streamConfig : data->streamConfigs_)\n> + streams.insert(&streamConfig.stream);\n> + std::string id = data->config_.id;\n> + std::shared_ptr camera = Camera::create(std::move(data), id, streams);\n> +\n> + if (!initFrameGenerator(camera.get())) {\n> + LOG(Virtual, Error) << \"Failed to initialize frame \"\n> + << \"generator for camera: \" << id;\n> + continue;\n> + }\n>\n> - registerCamera(std::move(camera));\n> + registerCamera(std::move(camera));\n> + }\n>\n> resetCreated_ = true;\n>\n> return true;\n> }\n>\n> -void PipelineHandlerVirtual::initFrameGenerator(Camera *camera)\n> +bool PipelineHandlerVirtual::initFrameGenerator(Camera *camera)\n> {\n> auto data = cameraData(camera);\n> - for (auto &streamConfig : data->streamConfigs_) {\n> - if (data->testPattern_ == TestPattern::DiagonalLines)\n> - streamConfig.frameGenerator = std::make_unique();\n> - else\n> - streamConfig.frameGenerator = std::make_unique();\n> - }\n> + auto &frame = data->config_.frame;\n> + std::visit(overloaded{\n> + [&](TestPattern &testPattern) {\n> + for (auto &streamConfig : data->streamConfigs_) {\n> + if (testPattern == TestPattern::DiagonalLines)\n> + streamConfig.frameGenerator = std::make_unique();\n> + else\n> + streamConfig.frameGenerator = std::make_unique();\n> + }\n> + },\n> + [&](ImageFrames &imageFrames) {\n> + for (auto &streamConfig : data->streamConfigs_)\n> + streamConfig.frameGenerator = ImageFrameGenerator::create(imageFrames);\n> + } },\n> + frame);\n> +\n> + for (auto &streamConfig : data->streamConfigs_)\n> + if (!streamConfig.frameGenerator)\n> + return false;\n> +\n> + return true;\n> }\n>\n> REGISTER_PIPELINE_HANDLER(PipelineHandlerVirtual, \"virtual\")\n> diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h\n> index 0b15f8d9c..8c4d6ae64 100644\n> --- a/src/libcamera/pipeline/virtual/virtual.h\n> +++ b/src/libcamera/pipeline/virtual/virtual.h\n> @@ -7,6 +7,8 @@\n>\n> #pragma once\n>\n> +#include \n> +#include \n> #include \n>\n> #include \n> @@ -15,10 +17,14 @@\n> #include \"libcamera/internal/camera.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n>\n> +#include \"frame_generator.h\"\n> +#include \"image_frame_generator.h\"\n> #include \"test_pattern_generator.h\"\n>\n> namespace libcamera {\n>\n> +using VirtualFrame = std::variant;\n> +\n> class VirtualCameraData : public Camera::Private\n> {\n> public:\n> @@ -26,23 +32,28 @@ public:\n>\n> struct Resolution {\n> Size size;\n> - std::vector frameRates;\n> + std::vector frameRates;\n> };\n> struct StreamConfig {\n> Stream stream;\n> std::unique_ptr frameGenerator;\n> };\n> + /* The config file is parsed to the Configuration struct */\n> + struct Configuration {\n> + std::string id;\n> + std::vector resolutions;\n> + VirtualFrame frame;\n> +\n> + Size maxResolutionSize;\n> + Size minResolutionSize;\n> + };\n>\n> VirtualCameraData(PipelineHandler *pipe,\n> const std::vector &supportedResolutions);\n>\n> ~VirtualCameraData() = default;\n>\n> - TestPattern testPattern_ = TestPattern::ColorBars;\n> -\n> - const std::vector supportedResolutions_;\n> - Size maxResolutionSize_;\n> - Size minResolutionSize_;\n> + Configuration config_;\n>\n> std::vector streamConfigs_;\n> };\n> --\n> 2.47.0.rc0.187.ge670bccf7e-goog\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 10354C32E0\n\tfor ;\n\tWed, 9 Oct 2024 07:57:19 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BA9756536B;\n\tWed, 9 Oct 2024 09:57:17 +0200 (CEST)","from mail-lj1-x233.google.com (mail-lj1-x233.google.com\n\t[IPv6:2a00:1450:4864:20::233])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1411063527\n\tfor ;\n\tWed, 9 Oct 2024 09:57:15 +0200 (CEST)","by mail-lj1-x233.google.com with SMTP id\n\t38308e7fff4ca-2fac47f0b1aso76919771fa.1\n\tfor ;\n\tWed, 09 Oct 2024 00:57:14 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"V2GuoBA2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1728460634; x=1729065434;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=SAOpameNKQd2zkm6IIW0g1UOlFVqmGlOlknHqx6l1zE=;\n\tb=V2GuoBA2FLaNCecY1Ry5rlHnkz3wVSa6wgxJuLI88NNqK/s6lad2Fi+XJqECYS011M\n\tMhy9NzorzxNcbd+2T7TII43TICWOx2LsTuNkVDwAhVKRm1K5h+qflKlb4FXVeAeuOcv4\n\taX6OejHq/YksAJLOhntzrjA+Es5NFncOD0DRY=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1728460634; x=1729065434;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=SAOpameNKQd2zkm6IIW0g1UOlFVqmGlOlknHqx6l1zE=;\n\tb=viumxFW74jcp4wf6G+k1bYjUWtK5aQtw5PjrFpzEUaeDZLMBzhd2ZHxv0vVvaor+ac\n\t/t6rnV4GnItgxLEGkeMnEsCy4z8omDdUoErinHkwSIOt0hujvYB1IH9rWAHQc+v0DFJs\n\tDU+oV2V17S9XH9b7KOx75YRXYzSZyc53QmuVvvcTJI1V4LpxMy9vBeBNy6gXnILKuXFd\n\tYrQUClNeChTm7F4b+oN5vqyhJzrWzcnCU/HeahzO1uQrvQtyGLwpAXZWoCIx6+GsSLss\n\txa6BOyGvSrlpJ+XiLHV1Ie9S1hbPEKJeTd9p5suIcOxcwwbRZCH9aWEcBNPKLYuDoZXw\n\t9Zag==","X-Gm-Message-State":"AOJu0YyVC6QP9DQe4rVrdRSiE82wjEYG/kEVeeuulo+MlHfHbs/TRAs+\n\t0covPmhQ5daZTZFgEkTnlyZYT2zflJEXBKQ4/+yyS5j4Nry4xVs2a2uFaCOE1Kxxok0n6IL0kKq\n\ttFuJapVybBSim2opUARD1QHgRgq38/bcorPzZ59NGYtkfbpI=","X-Google-Smtp-Source":"AGHT+IHmBMmZ/GPkBQpmcK5kTHbIuFFbHQjNxHi61jq9sym/26fLjiYJZ4vcs3W+7t1ew+aggBZJRS7lf3lJ0GqgMek=","X-Received":"by 2002:a2e:a99a:0:b0:2fa:c6b3:bf27 with SMTP id\n\t38308e7fff4ca-2fb1873f201mr10108821fa.6.1728460633498;\n\tWed, 09 Oct 2024 00:57:13 -0700 (PDT)","MIME-Version":"1.0","References":"<20241004095946.264537-1-chenghaoyang@google.com>\n\t<20241004095946.264537-7-chenghaoyang@google.com>","In-Reply-To":"<20241004095946.264537-7-chenghaoyang@google.com>","From":"Cheng-Hao Yang ","Date":"Wed, 9 Oct 2024 15:57:02 +0800","Message-ID":"","Subject":"Re: [PATCH 15 6/7] libcamera: virtual: Read config and register\n\tcameras based on the config","To":"libcamera-devel@lists.libcamera.org","Cc":"Harvey Yang , Konami Shu , \n\tYunke Cao , Tomasz Figa ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31868,"web_url":"https://patchwork.libcamera.org/comment/31868/","msgid":"<172955351674.1470935.9237357010380500470@ping.linuxembedded.co.uk>","date":"2024-10-21T23:31:56","subject":"Re: [PATCH 15 6/7] libcamera: virtual: Read config and register\n\tcameras based on the config","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Harvey Yang (2024-10-04 10:59:19)\n> This patch introduces the configuration file for Virtual Pipeline\n> Handler. The config file is written in yaml, and the format is\n> documented in `README.md`.\n> \n> The config file will define the camera with IDs, supported formats and\n> image sources, etc. In the default config file, only Test Patterns are\n> used. Developers can use real images loading if desired.\n> \n> Signed-off-by: Konami Shu \n> Co-developed-by: Harvey Yang \n> Co-developed-by: Yunke Cao \n> Co-developed-by: Tomasz Figa \n\nI think we determined Co-developed-by: need SoBs.\n\nCheckstyle reports:\n\n-----------------------------------------------------------------------------------------------------------------\n83f896b4afa8e5e7a4029d1e07bcb5dfe8cc5fb2 libcamera: virtual: Read config and register cameras based on the config\n-----------------------------------------------------------------------------------------------------------------\nNo 'Signed-off-by' trailer matching author 'Harvey Yang ', see Documentation/contributing.rst\n---\n1 potential issue detected, please review\n\nAnd that will apply to all the above.\n\n\n> ---\n> src/libcamera/pipeline/virtual/README.md | 65 +++++\n> .../pipeline/virtual/config_parser.cpp | 263 ++++++++++++++++++\n> .../pipeline/virtual/config_parser.h | 39 +++\n> .../pipeline/virtual/data/virtual.yaml | 36 +++\n> .../virtual/image_frame_generator.cpp | 16 +-\n> .../pipeline/virtual/image_frame_generator.h | 5 +-\n> src/libcamera/pipeline/virtual/meson.build | 1 +\n> src/libcamera/pipeline/virtual/virtual.cpp | 126 +++++----\n> src/libcamera/pipeline/virtual/virtual.h | 23 +-\n> 9 files changed, 504 insertions(+), 70 deletions(-)\n> create mode 100644 src/libcamera/pipeline/virtual/README.md\n> create mode 100644 src/libcamera/pipeline/virtual/config_parser.cpp\n> create mode 100644 src/libcamera/pipeline/virtual/config_parser.h\n> create mode 100644 src/libcamera/pipeline/virtual/data/virtual.yaml\n> \n> diff --git a/src/libcamera/pipeline/virtual/README.md b/src/libcamera/pipeline/virtual/README.md\n> new file mode 100644\n> index 000000000..5801e79b5\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/virtual/README.md\n> @@ -0,0 +1,65 @@\n> +# Virtual Pipeline Handler\n> +\n> +Virtual pipeline handler emulates fake external camera(s) on ChromeOS for testing.\n\nIt's not only on ChromeOS right ? It's just a virtual camera for any\nplatform.\n\n\n\n> +\n> +## Parse config file and register cameras\n> +\n> +- The config file is located at `src/libcamera/pipeline/virtual/data/virtual.yaml`\n\nI'm sure I mentioned this before - but this is the source location.\nWhile I preume the users would have to configure their 'installed'\nlocation. So this should probably reference the fact it would be\ninstalled in ...\n\nErr - where does it get installed ? What happens if libcamera is\ninstalled and tries to use the virtual pipeline handler here ?\n\nI don't see any reference in a meson file that installes\ndata/virtual.yaml ?\n\n\nLater on virtual.yaml is mentioned as a sample, while up here it's\nspecified as '\"The\" config file'. ? \n\nMaybe this should say \"A sample config file is located at ...\"\n\n> +\n> +### Config File Format\n> +The config file contains the information about cameras' properties to register.\n> +The config file should be a yaml file with dictionary of the cameraIds\n> +associated with their properties as top level. The default value will be applied\n> +when any property is empty.\n> +\n> +Each camera block is a dictionary, containing the following keys:\n> +- `supported_formats` (list of `VirtualCameraData::Resolution`, optional) :\n> + List of supported resolution and frame rates of the emulated camera\n> + - `width` (`unsigned int`, default=1920): Width of the window resolution.\n> + This needs to be even.\n> + - `height` (`unsigned int`, default=1080): Height of the window resolution.\n> + - `frame_rates` (list of `int`, default=`[30,60]` ): Range of the frame\n> + rate (per second). If the list contains one value, it's the lower bound\n> + and the upper bound. If the list contains two values, the first is the\n> + lower bound and the second is the upper bound. No other number of values\n> + is allowed.\n> +- `test_pattern` (`string`): Which test pattern to use as frames. The options\n> + are \"bars\", \"lines\". Cannot be set with `frames`.\n> +- `frames` (dictionary):\n> + - `path` (`string`): Path to an image, or path to a directory of a series of\n> + images. Cannot be set with `test_pattern`.\n> + - The test patterns are \"bars\" which means color bars, and \"lines\" which\n> + means diagonal lines.\n\nDoes this bullet point belong to the 'test_pattern' section above rather\nthan frames ? \n\n> + - The path to an image has \".jpg\" extension.\n> + - The path to a directory ends with \"/\". The name of the images in the\n> + directory are \"{n}.jpg\" with {n} is the sequence of images starting with 0.\n> +- `location` (`string`, default=\"front\"): The location of the camera. Support\n> + \"front\" and \"back\". This is displayed in qcam camera selection window but\n\nI don't think it's relevant to mention 'qcam' here. Setting the location\ndefines the location property of the camera which is exposed by the\ncamera in /any/ application. Not just qcam.\n\n\n> + this does not change the output.\n> +- `model` (`string`, default=\"Unknown\"): The model name of the camera.\n> + This is displayed in qcam camera selection window but this does not change the output.\n> +\n> +Check `data/virtual.yaml` as the sample config file.\n> +\n> +### Implementation\n> +\n> +`Parser` class provides methods to parse the config file to register cameras\n> +in Virtual Pipeline Handler. `parseConfigFile()` is exposed to use in\n> +Virtual Pipeline Handler.\n> +\n> +This is the procedure of the Parser class:\n> +1. `parseConfigFile()` parses the config file to `YamlObject` using `YamlParser::parse()`.\n> + - Parse the top level of config file which are the camera ids and look into\n> + each camera properties.\n> +2. For each camera, `parseCameraConfigData()` returns a camera with the configuration.\n> + - The methods in the next step fill the data with the pointer to the Camera object.\n> + - If the config file contains invalid configuration, this method returns\n> + nullptr. The camera will be skipped.\n> +3. Parse each property and register the data.\n> + - `parseSupportedFormats()`: Parses `supported_formats` in the config, which\n> + contains resolutions and frame rates.\n> + - `parseFrameGenerator()`: Parses `test_pattern` or `frames` in the config.\n> + - `parseLocation()`: Parses `location` in the config.\n> + - `parseModel()`: Parses `model` in the config.\n> +4. Back to `parseConfigFile()` and append the camera configuration.\n> +5. Returns a list of camera configurations.\n> diff --git a/src/libcamera/pipeline/virtual/config_parser.cpp b/src/libcamera/pipeline/virtual/config_parser.cpp\n> new file mode 100644\n> index 000000000..467dde485\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/virtual/config_parser.cpp\n> @@ -0,0 +1,263 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Google Inc.\n> + *\n> + * config_parser.cpp - Virtual cameras helper to parse config file\n\nDrop 'config_parser.cpp - ' We stopped putting file names in their own\nfiles as they bitrot and don't add value\n\n> + */\n> +\n> +#include \"config_parser.h\"\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/pipeline_handler.h\"\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"virtual.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(Virtual)\n> +\n> +std::vector>\n> +ConfigParser::parseConfigFile(File &file, PipelineHandler *pipe)\n> +{\n> + std::vector> configurations;\n> +\n> + std::unique_ptr cameras = YamlParser::parse(file);\n> + if (!cameras) {\n> + LOG(Virtual, Error) << \"Failed to pass config file.\";\n> + return configurations;\n> + }\n> +\n> + if (!cameras->isDictionary()) {\n> + LOG(Virtual, Error) << \"Config file is not a dictionary at the top level.\";\n> + return configurations;\n> + }\n> +\n> + /* Look into the configuration of each camera */\n> + for (const auto &[cameraId, cameraConfigData] : cameras->asDict()) {\n> + std::unique_ptr data =\n> + parseCameraConfigData(cameraConfigData, pipe);\n> + /* Parse configData to data */\n> + if (!data) {\n> + /* Skip the camera if it has invalid config */\n> + LOG(Virtual, Error) << \"Failed to parse config of the camera: \"\n> + << cameraId;\n> + continue;\n> + }\n> +\n> + data->config_.id = cameraId;\n> + ControlInfoMap::Map controls;\n> + /* todo: Check which resolution's frame rate to be reported */\n> + controls[&controls::FrameDurationLimits] =\n> + ControlInfo(1000000 / data->config_.resolutions[0].frameRates[1],\n> + 1000000 / data->config_.resolutions[0].frameRates[0]);\n> +\n> + std::vector supportedFaceDetectModes{\n> + static_cast(controls::draft::FaceDetectModeOff),\n> + };\n> + controls[&controls::draft::FaceDetectMode] = ControlInfo(supportedFaceDetectModes);\n> +\n> + data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);\n> + configurations.push_back(std::move(data));\n> + }\n> +\n> + return configurations;\n> +}\n> +\n> +std::unique_ptr\n> +ConfigParser::parseCameraConfigData(const YamlObject &cameraConfigData,\n> + PipelineHandler *pipe)\n> +{\n> + std::vector resolutions;\n> + if (parseSupportedFormats(cameraConfigData, &resolutions))\n> + return nullptr;\n> +\n> + std::unique_ptr data =\n> + std::make_unique(pipe, resolutions);\n> +\n> + if (parseFrameGenerator(cameraConfigData, data.get()))\n> + return nullptr;\n> +\n> + if (parseLocation(cameraConfigData, data.get()))\n> + return nullptr;\n> +\n> + if (parseModel(cameraConfigData, data.get()))\n> + return nullptr;\n> +\n> + return data;\n> +}\n> +\n> +int ConfigParser::parseSupportedFormats(const YamlObject &cameraConfigData,\n> + std::vector *resolutions)\n> +{\n> + if (cameraConfigData.contains(\"supported_formats\")) {\n> + const YamlObject &supportedResolutions = cameraConfigData[\"supported_formats\"];\n> +\n> + for (const YamlObject &supportedResolution : supportedResolutions.asList()) {\n> + unsigned int width = supportedResolution[\"width\"].get(1920);\n> + unsigned int height = supportedResolution[\"height\"].get(1080);\n> + if (width == 0 || height == 0) {\n> + LOG(Virtual, Error) << \"Invalid width or/and height\";\n> + return -EINVAL;\n> + }\n> + if (width % 2 != 0) {\n> + LOG(Virtual, Error) << \"Invalid width: width needs to be even\";\n> + return -EINVAL;\n> + }\n> +\n> + std::vector frameRates;\n> + if (supportedResolution.contains(\"frame_rates\")) {\n> + auto frameRatesList =\n> + supportedResolution[\"frame_rates\"].getList();\n> + if (!frameRatesList || (frameRatesList->size() != 1 &&\n> + frameRatesList->size() != 2)) {\n> + LOG(Virtual, Error) << \"Invalid frame_rates: either one or two values\";\n> + return -EINVAL;\n> + }\n> +\n> + if (frameRatesList->size() == 2 &&\n> + frameRatesList.value()[0] > frameRatesList.value()[1]) {\n> + LOG(Virtual, Error) << \"frame_rates's first value(lower bound)\"\n> + << \" is higher than the second value(upper bound)\";\n> + return -EINVAL;\n> + }\n> + /*\n> + * Push the min and max framerates. A\n> + * single rate is duplicated.\n> + */\n> + frameRates.push_back(frameRatesList.value().front());\n> + frameRates.push_back(frameRatesList.value().back());\n> + } else {\n> + frameRates.push_back(30);\n> + frameRates.push_back(60);\n> + }\n> +\n> + resolutions->emplace_back(\n> + VirtualCameraData::Resolution{ Size{ width, height },\n> + frameRates });\n> + }\n> + } else {\n> + resolutions->emplace_back(\n> + VirtualCameraData::Resolution{ Size{ 1920, 1080 },\n> + { 30, 60 } });\n> + }\n> +\n> + return 0;\n> +}\n> +\n> +int ConfigParser::parseFrameGenerator(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> +{\n> + const std::string testPatternKey = \"test_pattern\";\n> + const std::string framesKey = \"frames\";\n> + if (cameraConfigData.contains(testPatternKey)) {\n> + if (cameraConfigData.contains(framesKey)) {\n> + LOG(Virtual, Error) << \"A camera should use either \"\n> + << testPatternKey << \" or \" << framesKey;\n> + return -EINVAL;\n> + }\n> +\n> + auto testPattern = cameraConfigData[testPatternKey].get(\"\");\n> +\n> + if (testPattern == \"bars\") {\n> + data->config_.frame = TestPattern::ColorBars;\n> + } else if (testPattern == \"lines\") {\n> + data->config_.frame = TestPattern::DiagonalLines;\n> + } else {\n> + LOG(Virtual, Debug) << \"Test pattern: \" << testPattern\n> + << \" is not supported\";\n> + return -EINVAL;\n> + }\n> +\n> + return 0;\n> + }\n> +\n> + const YamlObject &frames = cameraConfigData[framesKey];\n> +\n> + /* When there is no frames provided in the config file, use color bar test pattern */\n> + if (!frames) {\n> + data->config_.frame = TestPattern::ColorBars;\n> + return 0;\n> + }\n> +\n> + if (!frames.isDictionary()) {\n> + LOG(Virtual, Error) << \"'frames' is not a dictionary.\";\n> + return -EINVAL;\n> + }\n> +\n> + auto path = frames[\"path\"].get();\n> +\n> + if (!path) {\n> + LOG(Virtual, Error) << \"Test pattern or path should be specified.\";\n> + return -EINVAL;\n> + }\n> +\n> + std::vector files;\n> +\n> + switch (std::filesystem::symlink_status(*path).type()) {\n> + case std::filesystem::file_type::regular:\n> + files.push_back(*path);\n> + break;\n> +\n> + case std::filesystem::file_type::directory:\n> + for (const auto &dentry : std::filesystem::directory_iterator{ *path }) {\n> + if (dentry.is_regular_file())\n> + files.push_back(dentry.path());\n> + }\n> +\n> + std::sort(files.begin(), files.end(), [](const auto &a, const auto &b) {\n> + return ::strverscmp(a.c_str(), b.c_str()) < 0;\n> + });\n> +\n> + if (files.empty()) {\n> + LOG(Virtual, Error) << \"Directory has no files: \" << *path;\n> + return -EINVAL;\n> + }\n> + break;\n> +\n> + default:\n> + LOG(Virtual, Error) << \"Frame: \" << *path << \" is not supported\";\n> + return -EINVAL;\n> + }\n> +\n> + data->config_.frame = ImageFrames{ std::move(files) };\n> +\n> + return 0;\n> +}\n> +\n> +int ConfigParser::parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> +{\n> + std::string location = cameraConfigData[\"location\"].get(\"front\");\n> +\n> + /* Default value is properties::CameraLocationFront */\n> + if (location == \"front\") {\n> + data->properties_.set(properties::Location,\n> + properties::CameraLocationFront);\n> + } else if (location == \"back\") {\n> + data->properties_.set(properties::Location,\n> + properties::CameraLocationBack);\n> + } else {\n> + LOG(Virtual, Error) << \"location: \" << location\n> + << \" is not supported\";\n\nWhat about external ? \n\nI think the control framework provides a map of string to id for the\nenum types in fact, so you can do something like:\n\t\n\nint ConfigParser::parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data)\n{\n std::string location = cameraConfigData[\"location\"].get(\"front\");\n\n auto it = properties::LocationNameValueMap.find(location);\n if (it == properties::LocationNameValueMap.end()) {\n LOG(Virtual, Error)\n << \"location: \" << location << \" is not supported\";\n\n return -EINVAL;\n }\n\n data->properties_.set(properties::Location, it->second);\n\n return 0;\n}\n\nWhich is probably more future proof too if we for some reason add any\nother locations.\n\n\n\n> + return -EINVAL;\n> + }\n> +\n> + return 0;\n> +}\n> +\n> +int ConfigParser::parseModel(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> +{\n> + std::string model = cameraConfigData[\"model\"].get(\"Unknown\");\n> +\n> + data->properties_.set(properties::Model, model);\n> +\n> + return 0;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/virtual/config_parser.h b/src/libcamera/pipeline/virtual/config_parser.h\n> new file mode 100644\n> index 000000000..9abba62d0\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/virtual/config_parser.h\n> @@ -0,0 +1,39 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2024, Google Inc.\n> + *\n> + * config_parser.h - Virtual cameras helper to parse config file\n\nDrop 'config_parser.h - ' from here.\n\n> + */\n> +\n> +#pragma once\n> +\n> +#include \n> +#include \n> +\n> +#include \n> +\n> +#include \"libcamera/internal/pipeline_handler.h\"\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"virtual.h\"\n> +\n> +namespace libcamera {\n> +\n> +class ConfigParser\n> +{\n> +public:\n> + std::vector>\n> + parseConfigFile(File &file, PipelineHandler *pipe);\n> +\n> +private:\n> + std::unique_ptr\n> + parseCameraConfigData(const YamlObject &cameraConfigData, PipelineHandler *pipe);\n> +\n> + int parseSupportedFormats(const YamlObject &cameraConfigData,\n> + std::vector *resolutions);\n> + int parseFrameGenerator(const YamlObject &cameraConfigData, VirtualCameraData *data);\n> + int parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data);\n> + int parseModel(const YamlObject &cameraConfigData, VirtualCameraData *data);\n> +};\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/virtual/data/virtual.yaml b/src/libcamera/pipeline/virtual/data/virtual.yaml\n> new file mode 100644\n> index 000000000..6b73ddf2b\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/virtual/data/virtual.yaml\n> @@ -0,0 +1,36 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +%YAML 1.1\n> +---\n> +\"Virtual0\":\n> + supported_formats:\n> + - width: 1920\n> + height: 1080\n> + frame_rates:\n> + - 30\n> + - 60\n> + - width: 1680\n> + height: 1050\n> + frame_rates:\n> + - 70\n> + - 80\n> + test_pattern: \"lines\"\n> + location: \"front\"\n> + model: \"Virtual Video Device\"\n> +\"Virtual1\":\n> + supported_formats:\n> + - width: 800\n> + height: 600\n> + frame_rates:\n> + - 60\n> + test_pattern: \"bars\"\n> + location: \"back\"\n> + model: \"Virtual Video Device1\"\n> +\"Virtual2\":\n> + supported_formats:\n> + - width: 400\n> + height: 300\n> + test_pattern: \"lines\"\n> + location: \"front\"\n> + model: \"Virtual Video Device2\"\n> +\"Virtual3\":\n> + test_pattern: \"bars\"\n> diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.cpp b/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> index 04382beb7..36bdc20e5 100644\n> --- a/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> +++ b/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> @@ -40,15 +40,7 @@ ImageFrameGenerator::create(ImageFrames &imageFrames)\n> * For each file in the directory, load the image,\n> * convert it to NV12, and store the pointer.\n> */\n> - for (unsigned int i = 0; i < imageFrames.number.value_or(1); i++) {\n> - std::filesystem::path path;\n> - if (!imageFrames.number)\n> - /* If the path is to an image */\n> - path = imageFrames.path;\n> - else\n> - /* If the path is to a directory */\n> - path = imageFrames.path / (std::to_string(i) + \".jpg\");\n> -\n> + for (std::filesystem::path path : imageFrames.files) {\n> File file(path);\n> if (!file.open(File::OpenModeFlag::ReadOnly)) {\n> LOG(Virtual, Error) << \"Failed to open image file \" << file.fileName()\n> @@ -88,6 +80,8 @@ ImageFrameGenerator::create(ImageFrames &imageFrames)\n> Size(width, height) });\n> }\n> \n> + ASSERT(!imageFrameGenerator->imageFrameDatas_.empty());\n> +\n> return imageFrameGenerator;\n> }\n> \n> @@ -104,7 +98,7 @@ void ImageFrameGenerator::configure(const Size &size)\n> frameIndex_ = 0;\n> parameter_ = 0;\n> \n> - for (unsigned int i = 0; i < imageFrames_->number.value_or(1); i++) {\n> + for (unsigned int i = 0; i < imageFrameDatas_.size(); i++) {\n> /* Scale the imageFrameDatas_ to scaledY and scaledUV */\n> unsigned int halfSizeWidth = (size.width + 1) / 2;\n> unsigned int halfSizeHeight = (size.height + 1) / 2;\n> @@ -139,7 +133,7 @@ int ImageFrameGenerator::generateFrame(const Size &size, const FrameBuffer *buff\n> auto planes = mappedFrameBuffer.planes();\n> \n> /* Loop only around the number of images available */\n> - frameIndex_ %= imageFrames_->number.value_or(1);\n> + frameIndex_ %= imageFrameDatas_.size();\n> \n> /* Write the scaledY and scaledUV to the mapped frame buffer */\n> libyuv::NV12Copy(scaledFrameDatas_[frameIndex_].Y.get(), size.width,\n> diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.h b/src/libcamera/pipeline/virtual/image_frame_generator.h\n> index a68094a66..8554d647d 100644\n> --- a/src/libcamera/pipeline/virtual/image_frame_generator.h\n> +++ b/src/libcamera/pipeline/virtual/image_frame_generator.h\n> @@ -10,9 +10,9 @@\n> \n> #include \n> #include \n> -#include \n> #include \n> #include \n> +#include \n> \n> #include \"frame_generator.h\"\n> \n> @@ -20,8 +20,7 @@ namespace libcamera {\n> \n> /* Frame configuration provided by the config file */\n> struct ImageFrames {\n> - std::filesystem::path path;\n> - std::optional number;\n> + std::vector files;\n> };\n> \n> class ImageFrameGenerator : public FrameGenerator\n> diff --git a/src/libcamera/pipeline/virtual/meson.build b/src/libcamera/pipeline/virtual/meson.build\n> index bb38c193c..4786fe2e0 100644\n> --- a/src/libcamera/pipeline/virtual/meson.build\n> +++ b/src/libcamera/pipeline/virtual/meson.build\n> @@ -1,6 +1,7 @@\n> # SPDX-License-Identifier: CC0-1.0\n> \n> libcamera_internal_sources += files([\n> + 'config_parser.cpp',\n> 'image_frame_generator.cpp',\n> 'test_pattern_generator.cpp',\n> 'virtual.cpp',\n> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n> index 1ad7417c7..cafd395c3 100644\n> --- a/src/libcamera/pipeline/virtual/virtual.cpp\n> +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n> @@ -36,6 +36,9 @@\n> #include \"libcamera/internal/formats.h\"\n> #include \"libcamera/internal/framebuffer.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> +#include \"libcamera/internal/yaml_parser.h\"\n> +\n> +#include \"pipeline/virtual/config_parser.h\"\n> \n> namespace libcamera {\n> \n> @@ -54,6 +57,13 @@ uint64_t currentTimestamp()\n> \n> } /* namespace */\n> \n> +template\n> +struct overloaded : Ts... {\n> + using Ts::operator()...;\n> +};\n> +template\n> +overloaded(Ts...) -> overloaded;\n> +\n> class VirtualCameraConfiguration : public CameraConfiguration\n> {\n> public:\n> @@ -95,7 +105,7 @@ private:\n> return static_cast(camera->_d());\n> }\n> \n> - void initFrameGenerator(Camera *camera);\n> + bool initFrameGenerator(Camera *camera);\n> \n> DmaBufAllocator dmaBufAllocator_;\n> \n> @@ -104,15 +114,19 @@ private:\n> \n> VirtualCameraData::VirtualCameraData(PipelineHandler *pipe,\n> const std::vector &supportedResolutions)\n> - : Camera::Private(pipe), supportedResolutions_(supportedResolutions)\n> + : Camera::Private(pipe)\n> {\n> - for (const auto &resolution : supportedResolutions_) {\n> - if (minResolutionSize_.isNull() || minResolutionSize_ > resolution.size)\n> - minResolutionSize_ = resolution.size;\n> + config_.resolutions = supportedResolutions;\n> + for (const auto &resolution : config_.resolutions) {\n> + if (config_.minResolutionSize.isNull() || config_.minResolutionSize > resolution.size)\n> + config_.minResolutionSize = resolution.size;\n> \n> - maxResolutionSize_ = std::max(maxResolutionSize_, resolution.size);\n> + config_.maxResolutionSize = std::max(config_.maxResolutionSize, resolution.size);\n> }\n> \n> + properties_.set(properties::PixelArrayActiveAreas,\n> + { Rectangle(config_.maxResolutionSize) });\n> +\n> /* \\todo Support multiple streams and pass multi_stream_test */\n> streamConfigs_.resize(kMaxStream);\n> }\n> @@ -140,7 +154,7 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()\n> for (StreamConfiguration &cfg : config_) {\n> bool adjusted = false;\n> bool found = false;\n> - for (const auto &resolution : data_->supportedResolutions_) {\n> + for (const auto &resolution : data_->config_.resolutions) {\n> if (resolution.size.width == cfg.size.width &&\n> resolution.size.height == cfg.size.height) {\n> found = true;\n> @@ -155,7 +169,7 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()\n> * Defining the default logic in PipelineHandler to\n> * find the closest resolution would be nice.\n> */\n> - cfg.size = data_->maxResolutionSize_;\n> + cfg.size = data_->config_.maxResolutionSize;\n> status = Adjusted;\n> adjusted = true;\n> }\n> @@ -224,12 +238,12 @@ PipelineHandlerVirtual::generateConfiguration(Camera *camera,\n> \n> std::map> streamFormats;\n> PixelFormat pixelFormat = formats::NV12;\n> - streamFormats[pixelFormat] = { { data->minResolutionSize_,\n> - data->maxResolutionSize_ } };\n> + streamFormats[pixelFormat] = { { data->config_.minResolutionSize,\n> + data->config_.maxResolutionSize } };\n> StreamFormats formats(streamFormats);\n> StreamConfiguration cfg(formats);\n> cfg.pixelFormat = pixelFormat;\n> - cfg.size = data->maxResolutionSize_;\n> + cfg.size = data->config_.maxResolutionSize;\n> cfg.bufferCount = VirtualCameraConfiguration::kBufferCount;\n> \n> config->addConfiguration(cfg);\n> @@ -246,6 +260,7 @@ int PipelineHandlerVirtual::configure(Camera *camera,\n> VirtualCameraData *data = cameraData(camera);\n> for (auto [i, c] : utils::enumerate(*config)) {\n> c.setStream(&data->streamConfigs_[i].stream);\n> + /* Start reading the images/generating test patterns */\n> data->streamConfigs_[i].frameGenerator->configure(c.size);\n> }\n> \n> @@ -315,56 +330,67 @@ bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator\n> \n> created_ = true;\n> \n> - /* \\todo Add virtual cameras according to a config file. */\n> -\n> - std::vector supportedResolutions;\n> - supportedResolutions.resize(2);\n> - supportedResolutions[0] = { .size = Size(1920, 1080), .frameRates = { 30 } };\n> - supportedResolutions[1] = { .size = Size(1280, 720), .frameRates = { 30 } };\n> -\n> - std::unique_ptr data =\n> - std::make_unique(this, supportedResolutions);\n> -\n> - data->properties_.set(properties::Location, properties::CameraLocationFront);\n> - data->properties_.set(properties::Model, \"Virtual Video Device\");\n> - data->properties_.set(properties::PixelArrayActiveAreas, { Rectangle(Size(1920, 1080)) });\n> -\n> - /* \\todo Set FrameDurationLimits based on config. */\n> - ControlInfoMap::Map controls;\n> - int64_t min_frame_duration = 33333, max_frame_duration = 33333;\n> - controls[&controls::FrameDurationLimits] = ControlInfo(min_frame_duration, max_frame_duration);\n> - std::vector supportedFaceDetectModes{\n> - static_cast(controls::draft::FaceDetectModeOff),\n> - };\n> - controls[&controls::draft::FaceDetectMode] = ControlInfo(supportedFaceDetectModes);\n> - data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);\n> -\n> - /* Create and register the camera. */\n> - std::set streams;\n> - for (auto &streamConfig : data->streamConfigs_)\n> - streams.insert(&streamConfig.stream);\n> + File file(configurationFile(\"virtual\", \"virtual.yaml\"));\n\nThis tells me that the sample virtual.yaml file should probably be\ninstalled in the libcamera data path...\n\nThis version doesn't currently apply cleanly to master, but I did have\nan branch with it on - and it rebased easily.\n\nBut it needs a new version posting at least to solve the SoB issues and\nso with the above tackled you can add \n\nReviewed-by: Kieran Bingham \n\n\n> + bool isOpen = file.open(File::OpenModeFlag::ReadOnly);\n> + if (!isOpen) {\n> + LOG(Virtual, Error) << \"Failed to open config file: \" << file.fileName();\n> + return false;\n> + }\n> \n> - const std::string id = \"Virtual0\";\n> - std::shared_ptr camera = Camera::create(std::move(data), id, streams);\n> + ConfigParser parser;\n> + auto configData = parser.parseConfigFile(file, this);\n> + if (configData.size() == 0) {\n> + LOG(Virtual, Error) << \"Failed to parse any cameras from the config file: \"\n> + << file.fileName();\n> + return false;\n> + }\n> \n> - initFrameGenerator(camera.get());\n> + /* Configure and register cameras with configData */\n> + for (auto &data : configData) {\n> + std::set streams;\n> + for (auto &streamConfig : data->streamConfigs_)\n> + streams.insert(&streamConfig.stream);\n> + std::string id = data->config_.id;\n> + std::shared_ptr camera = Camera::create(std::move(data), id, streams);\n> +\n> + if (!initFrameGenerator(camera.get())) {\n> + LOG(Virtual, Error) << \"Failed to initialize frame \"\n> + << \"generator for camera: \" << id;\n> + continue;\n> + }\n> \n> - registerCamera(std::move(camera));\n> + registerCamera(std::move(camera));\n> + }\n> \n> resetCreated_ = true;\n> \n> return true;\n> }\n> \n> -void PipelineHandlerVirtual::initFrameGenerator(Camera *camera)\n> +bool PipelineHandlerVirtual::initFrameGenerator(Camera *camera)\n> {\n> auto data = cameraData(camera);\n> - for (auto &streamConfig : data->streamConfigs_) {\n> - if (data->testPattern_ == TestPattern::DiagonalLines)\n> - streamConfig.frameGenerator = std::make_unique();\n> - else\n> - streamConfig.frameGenerator = std::make_unique();\n> - }\n> + auto &frame = data->config_.frame;\n> + std::visit(overloaded{\n> + [&](TestPattern &testPattern) {\n> + for (auto &streamConfig : data->streamConfigs_) {\n> + if (testPattern == TestPattern::DiagonalLines)\n> + streamConfig.frameGenerator = std::make_unique();\n> + else\n> + streamConfig.frameGenerator = std::make_unique();\n> + }\n> + },\n> + [&](ImageFrames &imageFrames) {\n> + for (auto &streamConfig : data->streamConfigs_)\n> + streamConfig.frameGenerator = ImageFrameGenerator::create(imageFrames);\n> + } },\n> + frame);\n> +\n> + for (auto &streamConfig : data->streamConfigs_)\n> + if (!streamConfig.frameGenerator)\n> + return false;\n> +\n> + return true;\n> }\n> \n> REGISTER_PIPELINE_HANDLER(PipelineHandlerVirtual, \"virtual\")\n> diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h\n> index 0b15f8d9c..8c4d6ae64 100644\n> --- a/src/libcamera/pipeline/virtual/virtual.h\n> +++ b/src/libcamera/pipeline/virtual/virtual.h\n> @@ -7,6 +7,8 @@\n> \n> #pragma once\n> \n> +#include \n> +#include \n> #include \n> \n> #include \n> @@ -15,10 +17,14 @@\n> #include \"libcamera/internal/camera.h\"\n> #include \"libcamera/internal/pipeline_handler.h\"\n> \n> +#include \"frame_generator.h\"\n> +#include \"image_frame_generator.h\"\n> #include \"test_pattern_generator.h\"\n> \n> namespace libcamera {\n> \n> +using VirtualFrame = std::variant;\n> +\n> class VirtualCameraData : public Camera::Private\n> {\n> public:\n> @@ -26,23 +32,28 @@ public:\n> \n> struct Resolution {\n> Size size;\n> - std::vector frameRates;\n> + std::vector frameRates;\n> };\n> struct StreamConfig {\n> Stream stream;\n> std::unique_ptr frameGenerator;\n> };\n> + /* The config file is parsed to the Configuration struct */\n> + struct Configuration {\n> + std::string id;\n> + std::vector resolutions;\n> + VirtualFrame frame;\n> +\n> + Size maxResolutionSize;\n> + Size minResolutionSize;\n> + };\n> \n> VirtualCameraData(PipelineHandler *pipe,\n> const std::vector &supportedResolutions);\n> \n> ~VirtualCameraData() = default;\n> \n> - TestPattern testPattern_ = TestPattern::ColorBars;\n> -\n> - const std::vector supportedResolutions_;\n> - Size maxResolutionSize_;\n> - Size minResolutionSize_;\n> + Configuration config_;\n> \n> std::vector streamConfigs_;\n> };\n> -- \n> 2.47.0.rc0.187.ge670bccf7e-goog\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id B5779C3273\n\tfor ;\n\tMon, 21 Oct 2024 23:32:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B153765392;\n\tTue, 22 Oct 2024 01:32:01 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 01E2665379\n\tfor ;\n\tTue, 22 Oct 2024 01:31:59 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 695ACCCC;\n\tTue, 22 Oct 2024 01:30:13 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ayGgdNl0\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729553413;\n\tbh=+u58Dyl0ttXqYEGUzZKebqagb3t6k+M0+aNSEWoOprg=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=ayGgdNl0KfVzyBiQgpg3ZcrO5XyCYsX2VSUUJCtW/zuvuG8l+jmgH+4hxCcgCkntb\n\tzWL/jRVnk5iz4Bzxr/ctzFDox9g8fqO6njG2Iliyh/kQWy6mzXoLHYEAsWOwCdxOkc\n\tRYE2yx1FvNMCxpPDNAA0QqzHHXmYPbXHEw6LqHqo=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241004095946.264537-7-chenghaoyang@google.com>","References":"<20241004095946.264537-1-chenghaoyang@google.com>\n\t<20241004095946.264537-7-chenghaoyang@google.com>","Subject":"Re: [PATCH 15 6/7] libcamera: virtual: Read config and register\n\tcameras based on the config","From":"Kieran Bingham ","Cc":"Harvey Yang , Konami Shu , \n\tHarvey Yang , Yunke Cao ,\n\tTomasz Figa ","To":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Tue, 22 Oct 2024 00:31:56 +0100","Message-ID":"<172955351674.1470935.9237357010380500470@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31871,"web_url":"https://patchwork.libcamera.org/comment/31871/","msgid":"","date":"2024-10-22T07:46:05","subject":"Re: [PATCH 15 6/7] libcamera: virtual: Read config and register\n\tcameras based on the config","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Hi Kieran,\n\nOn Tue, Oct 22, 2024 at 7:31 AM Kieran Bingham\n wrote:\n>\n> Quoting Harvey Yang (2024-10-04 10:59:19)\n> > This patch introduces the configuration file for Virtual Pipeline\n> > Handler. The config file is written in yaml, and the format is\n> > documented in `README.md`.\n> >\n> > The config file will define the camera with IDs, supported formats and\n> > image sources, etc. In the default config file, only Test Patterns are\n> > used. Developers can use real images loading if desired.\n> >\n> > Signed-off-by: Konami Shu \n> > Co-developed-by: Harvey Yang \n> > Co-developed-by: Yunke Cao \n> > Co-developed-by: Tomasz Figa \n>\n> I think we determined Co-developed-by: need SoBs.\n\nYes, updated. I didn't upload a new version to avoid the spam.\n\n>\n> Checkstyle reports:\n>\n> -----------------------------------------------------------------------------------------------------------------\n> 83f896b4afa8e5e7a4029d1e07bcb5dfe8cc5fb2 libcamera: virtual: Read config and register cameras based on the config\n> -----------------------------------------------------------------------------------------------------------------\n> No 'Signed-off-by' trailer matching author 'Harvey Yang ', see Documentation/contributing.rst\n> ---\n> 1 potential issue detected, please review\n>\n> And that will apply to all the above.\n\nDone.\n\n>\n>\n> > ---\n> > src/libcamera/pipeline/virtual/README.md | 65 +++++\n> > .../pipeline/virtual/config_parser.cpp | 263 ++++++++++++++++++\n> > .../pipeline/virtual/config_parser.h | 39 +++\n> > .../pipeline/virtual/data/virtual.yaml | 36 +++\n> > .../virtual/image_frame_generator.cpp | 16 +-\n> > .../pipeline/virtual/image_frame_generator.h | 5 +-\n> > src/libcamera/pipeline/virtual/meson.build | 1 +\n> > src/libcamera/pipeline/virtual/virtual.cpp | 126 +++++----\n> > src/libcamera/pipeline/virtual/virtual.h | 23 +-\n> > 9 files changed, 504 insertions(+), 70 deletions(-)\n> > create mode 100644 src/libcamera/pipeline/virtual/README.md\n> > create mode 100644 src/libcamera/pipeline/virtual/config_parser.cpp\n> > create mode 100644 src/libcamera/pipeline/virtual/config_parser.h\n> > create mode 100644 src/libcamera/pipeline/virtual/data/virtual.yaml\n> >\n> > diff --git a/src/libcamera/pipeline/virtual/README.md b/src/libcamera/pipeline/virtual/README.md\n> > new file mode 100644\n> > index 000000000..5801e79b5\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/virtual/README.md\n> > @@ -0,0 +1,65 @@\n> > +# Virtual Pipeline Handler\n> > +\n> > +Virtual pipeline handler emulates fake external camera(s) on ChromeOS for testing.\n>\n> It's not only on ChromeOS right ? It's just a virtual camera for any\n> platform.\n\nRight, removed.\n\n>\n>\n>\n> > +\n> > +## Parse config file and register cameras\n> > +\n> > +- The config file is located at `src/libcamera/pipeline/virtual/data/virtual.yaml`\n>\n> I'm sure I mentioned this before - but this is the source location.\n> While I preume the users would have to configure their 'installed'\n> location. So this should probably reference the fact it would be\n> installed in ...\n>\n> Err - where does it get installed ? What happens if libcamera is\n> installed and tries to use the virtual pipeline handler here ?\n>\n> I don't see any reference in a meson file that installes\n> data/virtual.yaml ?\n>\n>\n> Later on virtual.yaml is mentioned as a sample, while up here it's\n> specified as '\"The\" config file'. ?\n>\n> Maybe this should say \"A sample config file is located at ...\"\n\nYes, it depends on if libcamera is installed. Updated and added\nthe installed path:\n`shar/libcamera/pipeline/virtual/virtual.yaml`.\n\n>\n> > +\n> > +### Config File Format\n> > +The config file contains the information about cameras' properties to register.\n> > +The config file should be a yaml file with dictionary of the cameraIds\n> > +associated with their properties as top level. The default value will be applied\n> > +when any property is empty.\n> > +\n> > +Each camera block is a dictionary, containing the following keys:\n> > +- `supported_formats` (list of `VirtualCameraData::Resolution`, optional) :\n> > + List of supported resolution and frame rates of the emulated camera\n> > + - `width` (`unsigned int`, default=1920): Width of the window resolution.\n> > + This needs to be even.\n> > + - `height` (`unsigned int`, default=1080): Height of the window resolution.\n> > + - `frame_rates` (list of `int`, default=`[30,60]` ): Range of the frame\n> > + rate (per second). If the list contains one value, it's the lower bound\n> > + and the upper bound. If the list contains two values, the first is the\n> > + lower bound and the second is the upper bound. No other number of values\n> > + is allowed.\n> > +- `test_pattern` (`string`): Which test pattern to use as frames. The options\n> > + are \"bars\", \"lines\". Cannot be set with `frames`.\n> > +- `frames` (dictionary):\n> > + - `path` (`string`): Path to an image, or path to a directory of a series of\n> > + images. Cannot be set with `test_pattern`.\n> > + - The test patterns are \"bars\" which means color bars, and \"lines\" which\n> > + means diagonal lines.\n>\n> Does this bullet point belong to the 'test_pattern' section above rather\n> than frames ?\n\nRight, thanks! Updated.\n\n>\n> > + - The path to an image has \".jpg\" extension.\n> > + - The path to a directory ends with \"/\". The name of the images in the\n> > + directory are \"{n}.jpg\" with {n} is the sequence of images starting with 0.\n> > +- `location` (`string`, default=\"front\"): The location of the camera. Support\n> > + \"front\" and \"back\". This is displayed in qcam camera selection window but\n>\n> I don't think it's relevant to mention 'qcam' here. Setting the location\n> defines the location property of the camera which is exposed by the\n> camera in /any/ application. Not just qcam.\n\nRight, removed this and the one in `model`.\n\n>\n>\n> > + this does not change the output.\n> > +- `model` (`string`, default=\"Unknown\"): The model name of the camera.\n> > + This is displayed in qcam camera selection window but this does not change the output.\n> > +\n> > +Check `data/virtual.yaml` as the sample config file.\n> > +\n> > +### Implementation\n> > +\n> > +`Parser` class provides methods to parse the config file to register cameras\n> > +in Virtual Pipeline Handler. `parseConfigFile()` is exposed to use in\n> > +Virtual Pipeline Handler.\n> > +\n> > +This is the procedure of the Parser class:\n> > +1. `parseConfigFile()` parses the config file to `YamlObject` using `YamlParser::parse()`.\n> > + - Parse the top level of config file which are the camera ids and look into\n> > + each camera properties.\n> > +2. For each camera, `parseCameraConfigData()` returns a camera with the configuration.\n> > + - The methods in the next step fill the data with the pointer to the Camera object.\n> > + - If the config file contains invalid configuration, this method returns\n> > + nullptr. The camera will be skipped.\n> > +3. Parse each property and register the data.\n> > + - `parseSupportedFormats()`: Parses `supported_formats` in the config, which\n> > + contains resolutions and frame rates.\n> > + - `parseFrameGenerator()`: Parses `test_pattern` or `frames` in the config.\n> > + - `parseLocation()`: Parses `location` in the config.\n> > + - `parseModel()`: Parses `model` in the config.\n> > +4. Back to `parseConfigFile()` and append the camera configuration.\n> > +5. Returns a list of camera configurations.\n> > diff --git a/src/libcamera/pipeline/virtual/config_parser.cpp b/src/libcamera/pipeline/virtual/config_parser.cpp\n> > new file mode 100644\n> > index 000000000..467dde485\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/virtual/config_parser.cpp\n> > @@ -0,0 +1,263 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Google Inc.\n> > + *\n> > + * config_parser.cpp - Virtual cameras helper to parse config file\n>\n> Drop 'config_parser.cpp - ' We stopped putting file names in their own\n> files as they bitrot and don't add value\n\nDone\n\n>\n> > + */\n> > +\n> > +#include \"config_parser.h\"\n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \"libcamera/internal/pipeline_handler.h\"\n> > +#include \"libcamera/internal/yaml_parser.h\"\n> > +\n> > +#include \"virtual.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DECLARE_CATEGORY(Virtual)\n> > +\n> > +std::vector>\n> > +ConfigParser::parseConfigFile(File &file, PipelineHandler *pipe)\n> > +{\n> > + std::vector> configurations;\n> > +\n> > + std::unique_ptr cameras = YamlParser::parse(file);\n> > + if (!cameras) {\n> > + LOG(Virtual, Error) << \"Failed to pass config file.\";\n> > + return configurations;\n> > + }\n> > +\n> > + if (!cameras->isDictionary()) {\n> > + LOG(Virtual, Error) << \"Config file is not a dictionary at the top level.\";\n> > + return configurations;\n> > + }\n> > +\n> > + /* Look into the configuration of each camera */\n> > + for (const auto &[cameraId, cameraConfigData] : cameras->asDict()) {\n> > + std::unique_ptr data =\n> > + parseCameraConfigData(cameraConfigData, pipe);\n> > + /* Parse configData to data */\n> > + if (!data) {\n> > + /* Skip the camera if it has invalid config */\n> > + LOG(Virtual, Error) << \"Failed to parse config of the camera: \"\n> > + << cameraId;\n> > + continue;\n> > + }\n> > +\n> > + data->config_.id = cameraId;\n> > + ControlInfoMap::Map controls;\n> > + /* todo: Check which resolution's frame rate to be reported */\n> > + controls[&controls::FrameDurationLimits] =\n> > + ControlInfo(1000000 / data->config_.resolutions[0].frameRates[1],\n> > + 1000000 / data->config_.resolutions[0].frameRates[0]);\n> > +\n> > + std::vector supportedFaceDetectModes{\n> > + static_cast(controls::draft::FaceDetectModeOff),\n> > + };\n> > + controls[&controls::draft::FaceDetectMode] = ControlInfo(supportedFaceDetectModes);\n> > +\n> > + data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);\n> > + configurations.push_back(std::move(data));\n> > + }\n> > +\n> > + return configurations;\n> > +}\n> > +\n> > +std::unique_ptr\n> > +ConfigParser::parseCameraConfigData(const YamlObject &cameraConfigData,\n> > + PipelineHandler *pipe)\n> > +{\n> > + std::vector resolutions;\n> > + if (parseSupportedFormats(cameraConfigData, &resolutions))\n> > + return nullptr;\n> > +\n> > + std::unique_ptr data =\n> > + std::make_unique(pipe, resolutions);\n> > +\n> > + if (parseFrameGenerator(cameraConfigData, data.get()))\n> > + return nullptr;\n> > +\n> > + if (parseLocation(cameraConfigData, data.get()))\n> > + return nullptr;\n> > +\n> > + if (parseModel(cameraConfigData, data.get()))\n> > + return nullptr;\n> > +\n> > + return data;\n> > +}\n> > +\n> > +int ConfigParser::parseSupportedFormats(const YamlObject &cameraConfigData,\n> > + std::vector *resolutions)\n> > +{\n> > + if (cameraConfigData.contains(\"supported_formats\")) {\n> > + const YamlObject &supportedResolutions = cameraConfigData[\"supported_formats\"];\n> > +\n> > + for (const YamlObject &supportedResolution : supportedResolutions.asList()) {\n> > + unsigned int width = supportedResolution[\"width\"].get(1920);\n> > + unsigned int height = supportedResolution[\"height\"].get(1080);\n> > + if (width == 0 || height == 0) {\n> > + LOG(Virtual, Error) << \"Invalid width or/and height\";\n> > + return -EINVAL;\n> > + }\n> > + if (width % 2 != 0) {\n> > + LOG(Virtual, Error) << \"Invalid width: width needs to be even\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + std::vector frameRates;\n> > + if (supportedResolution.contains(\"frame_rates\")) {\n> > + auto frameRatesList =\n> > + supportedResolution[\"frame_rates\"].getList();\n> > + if (!frameRatesList || (frameRatesList->size() != 1 &&\n> > + frameRatesList->size() != 2)) {\n> > + LOG(Virtual, Error) << \"Invalid frame_rates: either one or two values\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + if (frameRatesList->size() == 2 &&\n> > + frameRatesList.value()[0] > frameRatesList.value()[1]) {\n> > + LOG(Virtual, Error) << \"frame_rates's first value(lower bound)\"\n> > + << \" is higher than the second value(upper bound)\";\n> > + return -EINVAL;\n> > + }\n> > + /*\n> > + * Push the min and max framerates. A\n> > + * single rate is duplicated.\n> > + */\n> > + frameRates.push_back(frameRatesList.value().front());\n> > + frameRates.push_back(frameRatesList.value().back());\n> > + } else {\n> > + frameRates.push_back(30);\n> > + frameRates.push_back(60);\n> > + }\n> > +\n> > + resolutions->emplace_back(\n> > + VirtualCameraData::Resolution{ Size{ width, height },\n> > + frameRates });\n> > + }\n> > + } else {\n> > + resolutions->emplace_back(\n> > + VirtualCameraData::Resolution{ Size{ 1920, 1080 },\n> > + { 30, 60 } });\n> > + }\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +int ConfigParser::parseFrameGenerator(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> > +{\n> > + const std::string testPatternKey = \"test_pattern\";\n> > + const std::string framesKey = \"frames\";\n> > + if (cameraConfigData.contains(testPatternKey)) {\n> > + if (cameraConfigData.contains(framesKey)) {\n> > + LOG(Virtual, Error) << \"A camera should use either \"\n> > + << testPatternKey << \" or \" << framesKey;\n> > + return -EINVAL;\n> > + }\n> > +\n> > + auto testPattern = cameraConfigData[testPatternKey].get(\"\");\n> > +\n> > + if (testPattern == \"bars\") {\n> > + data->config_.frame = TestPattern::ColorBars;\n> > + } else if (testPattern == \"lines\") {\n> > + data->config_.frame = TestPattern::DiagonalLines;\n> > + } else {\n> > + LOG(Virtual, Debug) << \"Test pattern: \" << testPattern\n> > + << \" is not supported\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + return 0;\n> > + }\n> > +\n> > + const YamlObject &frames = cameraConfigData[framesKey];\n> > +\n> > + /* When there is no frames provided in the config file, use color bar test pattern */\n> > + if (!frames) {\n> > + data->config_.frame = TestPattern::ColorBars;\n> > + return 0;\n> > + }\n> > +\n> > + if (!frames.isDictionary()) {\n> > + LOG(Virtual, Error) << \"'frames' is not a dictionary.\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + auto path = frames[\"path\"].get();\n> > +\n> > + if (!path) {\n> > + LOG(Virtual, Error) << \"Test pattern or path should be specified.\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + std::vector files;\n> > +\n> > + switch (std::filesystem::symlink_status(*path).type()) {\n> > + case std::filesystem::file_type::regular:\n> > + files.push_back(*path);\n> > + break;\n> > +\n> > + case std::filesystem::file_type::directory:\n> > + for (const auto &dentry : std::filesystem::directory_iterator{ *path }) {\n> > + if (dentry.is_regular_file())\n> > + files.push_back(dentry.path());\n> > + }\n> > +\n> > + std::sort(files.begin(), files.end(), [](const auto &a, const auto &b) {\n> > + return ::strverscmp(a.c_str(), b.c_str()) < 0;\n> > + });\n> > +\n> > + if (files.empty()) {\n> > + LOG(Virtual, Error) << \"Directory has no files: \" << *path;\n> > + return -EINVAL;\n> > + }\n> > + break;\n> > +\n> > + default:\n> > + LOG(Virtual, Error) << \"Frame: \" << *path << \" is not supported\";\n> > + return -EINVAL;\n> > + }\n> > +\n> > + data->config_.frame = ImageFrames{ std::move(files) };\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +int ConfigParser::parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> > +{\n> > + std::string location = cameraConfigData[\"location\"].get(\"front\");\n> > +\n> > + /* Default value is properties::CameraLocationFront */\n> > + if (location == \"front\") {\n> > + data->properties_.set(properties::Location,\n> > + properties::CameraLocationFront);\n> > + } else if (location == \"back\") {\n> > + data->properties_.set(properties::Location,\n> > + properties::CameraLocationBack);\n> > + } else {\n> > + LOG(Virtual, Error) << \"location: \" << location\n> > + << \" is not supported\";\n>\n> What about external ?\n>\n> I think the control framework provides a map of string to id for the\n> enum types in fact, so you can do something like:\n>\n>\n> int ConfigParser::parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> {\n> std::string location = cameraConfigData[\"location\"].get(\"front\");\n>\n> auto it = properties::LocationNameValueMap.find(location);\n> if (it == properties::LocationNameValueMap.end()) {\n> LOG(Virtual, Error)\n> << \"location: \" << location << \" is not supported\";\n>\n> return -EINVAL;\n> }\n>\n> data->properties_.set(properties::Location, it->second);\n>\n> return 0;\n> }\n>\n> Which is probably more future proof too if we for some reason add any\n> other locations.\n\nMakes sense, although the value has the prefix \"CameraLocationXXX\".\nUpdated.\n\n>\n>\n>\n> > + return -EINVAL;\n> > + }\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +int ConfigParser::parseModel(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> > +{\n> > + std::string model = cameraConfigData[\"model\"].get(\"Unknown\");\n> > +\n> > + data->properties_.set(properties::Model, model);\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/virtual/config_parser.h b/src/libcamera/pipeline/virtual/config_parser.h\n> > new file mode 100644\n> > index 000000000..9abba62d0\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/virtual/config_parser.h\n> > @@ -0,0 +1,39 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2024, Google Inc.\n> > + *\n> > + * config_parser.h - Virtual cameras helper to parse config file\n>\n> Drop 'config_parser.h - ' from here.\n\nDone\n\n>\n> > + */\n> > +\n> > +#pragma once\n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n> > +\n> > +#include \"libcamera/internal/pipeline_handler.h\"\n> > +#include \"libcamera/internal/yaml_parser.h\"\n> > +\n> > +#include \"virtual.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +class ConfigParser\n> > +{\n> > +public:\n> > + std::vector>\n> > + parseConfigFile(File &file, PipelineHandler *pipe);\n> > +\n> > +private:\n> > + std::unique_ptr\n> > + parseCameraConfigData(const YamlObject &cameraConfigData, PipelineHandler *pipe);\n> > +\n> > + int parseSupportedFormats(const YamlObject &cameraConfigData,\n> > + std::vector *resolutions);\n> > + int parseFrameGenerator(const YamlObject &cameraConfigData, VirtualCameraData *data);\n> > + int parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data);\n> > + int parseModel(const YamlObject &cameraConfigData, VirtualCameraData *data);\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/virtual/data/virtual.yaml b/src/libcamera/pipeline/virtual/data/virtual.yaml\n> > new file mode 100644\n> > index 000000000..6b73ddf2b\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/virtual/data/virtual.yaml\n> > @@ -0,0 +1,36 @@\n> > +# SPDX-License-Identifier: CC0-1.0\n> > +%YAML 1.1\n> > +---\n> > +\"Virtual0\":\n> > + supported_formats:\n> > + - width: 1920\n> > + height: 1080\n> > + frame_rates:\n> > + - 30\n> > + - 60\n> > + - width: 1680\n> > + height: 1050\n> > + frame_rates:\n> > + - 70\n> > + - 80\n> > + test_pattern: \"lines\"\n> > + location: \"front\"\n> > + model: \"Virtual Video Device\"\n> > +\"Virtual1\":\n> > + supported_formats:\n> > + - width: 800\n> > + height: 600\n> > + frame_rates:\n> > + - 60\n> > + test_pattern: \"bars\"\n> > + location: \"back\"\n> > + model: \"Virtual Video Device1\"\n> > +\"Virtual2\":\n> > + supported_formats:\n> > + - width: 400\n> > + height: 300\n> > + test_pattern: \"lines\"\n> > + location: \"front\"\n> > + model: \"Virtual Video Device2\"\n> > +\"Virtual3\":\n> > + test_pattern: \"bars\"\n> > diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.cpp b/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> > index 04382beb7..36bdc20e5 100644\n> > --- a/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> > +++ b/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> > @@ -40,15 +40,7 @@ ImageFrameGenerator::create(ImageFrames &imageFrames)\n> > * For each file in the directory, load the image,\n> > * convert it to NV12, and store the pointer.\n> > */\n> > - for (unsigned int i = 0; i < imageFrames.number.value_or(1); i++) {\n> > - std::filesystem::path path;\n> > - if (!imageFrames.number)\n> > - /* If the path is to an image */\n> > - path = imageFrames.path;\n> > - else\n> > - /* If the path is to a directory */\n> > - path = imageFrames.path / (std::to_string(i) + \".jpg\");\n> > -\n> > + for (std::filesystem::path path : imageFrames.files) {\n> > File file(path);\n> > if (!file.open(File::OpenModeFlag::ReadOnly)) {\n> > LOG(Virtual, Error) << \"Failed to open image file \" << file.fileName()\n> > @@ -88,6 +80,8 @@ ImageFrameGenerator::create(ImageFrames &imageFrames)\n> > Size(width, height) });\n> > }\n> >\n> > + ASSERT(!imageFrameGenerator->imageFrameDatas_.empty());\n> > +\n> > return imageFrameGenerator;\n> > }\n> >\n> > @@ -104,7 +98,7 @@ void ImageFrameGenerator::configure(const Size &size)\n> > frameIndex_ = 0;\n> > parameter_ = 0;\n> >\n> > - for (unsigned int i = 0; i < imageFrames_->number.value_or(1); i++) {\n> > + for (unsigned int i = 0; i < imageFrameDatas_.size(); i++) {\n> > /* Scale the imageFrameDatas_ to scaledY and scaledUV */\n> > unsigned int halfSizeWidth = (size.width + 1) / 2;\n> > unsigned int halfSizeHeight = (size.height + 1) / 2;\n> > @@ -139,7 +133,7 @@ int ImageFrameGenerator::generateFrame(const Size &size, const FrameBuffer *buff\n> > auto planes = mappedFrameBuffer.planes();\n> >\n> > /* Loop only around the number of images available */\n> > - frameIndex_ %= imageFrames_->number.value_or(1);\n> > + frameIndex_ %= imageFrameDatas_.size();\n> >\n> > /* Write the scaledY and scaledUV to the mapped frame buffer */\n> > libyuv::NV12Copy(scaledFrameDatas_[frameIndex_].Y.get(), size.width,\n> > diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.h b/src/libcamera/pipeline/virtual/image_frame_generator.h\n> > index a68094a66..8554d647d 100644\n> > --- a/src/libcamera/pipeline/virtual/image_frame_generator.h\n> > +++ b/src/libcamera/pipeline/virtual/image_frame_generator.h\n> > @@ -10,9 +10,9 @@\n> >\n> > #include \n> > #include \n> > -#include \n> > #include \n> > #include \n> > +#include \n> >\n> > #include \"frame_generator.h\"\n> >\n> > @@ -20,8 +20,7 @@ namespace libcamera {\n> >\n> > /* Frame configuration provided by the config file */\n> > struct ImageFrames {\n> > - std::filesystem::path path;\n> > - std::optional number;\n> > + std::vector files;\n> > };\n> >\n> > class ImageFrameGenerator : public FrameGenerator\n> > diff --git a/src/libcamera/pipeline/virtual/meson.build b/src/libcamera/pipeline/virtual/meson.build\n> > index bb38c193c..4786fe2e0 100644\n> > --- a/src/libcamera/pipeline/virtual/meson.build\n> > +++ b/src/libcamera/pipeline/virtual/meson.build\n> > @@ -1,6 +1,7 @@\n> > # SPDX-License-Identifier: CC0-1.0\n> >\n> > libcamera_internal_sources += files([\n> > + 'config_parser.cpp',\n> > 'image_frame_generator.cpp',\n> > 'test_pattern_generator.cpp',\n> > 'virtual.cpp',\n> > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n> > index 1ad7417c7..cafd395c3 100644\n> > --- a/src/libcamera/pipeline/virtual/virtual.cpp\n> > +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n> > @@ -36,6 +36,9 @@\n> > #include \"libcamera/internal/formats.h\"\n> > #include \"libcamera/internal/framebuffer.h\"\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> > +#include \"libcamera/internal/yaml_parser.h\"\n> > +\n> > +#include \"pipeline/virtual/config_parser.h\"\n> >\n> > namespace libcamera {\n> >\n> > @@ -54,6 +57,13 @@ uint64_t currentTimestamp()\n> >\n> > } /* namespace */\n> >\n> > +template\n> > +struct overloaded : Ts... {\n> > + using Ts::operator()...;\n> > +};\n> > +template\n> > +overloaded(Ts...) -> overloaded;\n> > +\n> > class VirtualCameraConfiguration : public CameraConfiguration\n> > {\n> > public:\n> > @@ -95,7 +105,7 @@ private:\n> > return static_cast(camera->_d());\n> > }\n> >\n> > - void initFrameGenerator(Camera *camera);\n> > + bool initFrameGenerator(Camera *camera);\n> >\n> > DmaBufAllocator dmaBufAllocator_;\n> >\n> > @@ -104,15 +114,19 @@ private:\n> >\n> > VirtualCameraData::VirtualCameraData(PipelineHandler *pipe,\n> > const std::vector &supportedResolutions)\n> > - : Camera::Private(pipe), supportedResolutions_(supportedResolutions)\n> > + : Camera::Private(pipe)\n> > {\n> > - for (const auto &resolution : supportedResolutions_) {\n> > - if (minResolutionSize_.isNull() || minResolutionSize_ > resolution.size)\n> > - minResolutionSize_ = resolution.size;\n> > + config_.resolutions = supportedResolutions;\n> > + for (const auto &resolution : config_.resolutions) {\n> > + if (config_.minResolutionSize.isNull() || config_.minResolutionSize > resolution.size)\n> > + config_.minResolutionSize = resolution.size;\n> >\n> > - maxResolutionSize_ = std::max(maxResolutionSize_, resolution.size);\n> > + config_.maxResolutionSize = std::max(config_.maxResolutionSize, resolution.size);\n> > }\n> >\n> > + properties_.set(properties::PixelArrayActiveAreas,\n> > + { Rectangle(config_.maxResolutionSize) });\n> > +\n> > /* \\todo Support multiple streams and pass multi_stream_test */\n> > streamConfigs_.resize(kMaxStream);\n> > }\n> > @@ -140,7 +154,7 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()\n> > for (StreamConfiguration &cfg : config_) {\n> > bool adjusted = false;\n> > bool found = false;\n> > - for (const auto &resolution : data_->supportedResolutions_) {\n> > + for (const auto &resolution : data_->config_.resolutions) {\n> > if (resolution.size.width == cfg.size.width &&\n> > resolution.size.height == cfg.size.height) {\n> > found = true;\n> > @@ -155,7 +169,7 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()\n> > * Defining the default logic in PipelineHandler to\n> > * find the closest resolution would be nice.\n> > */\n> > - cfg.size = data_->maxResolutionSize_;\n> > + cfg.size = data_->config_.maxResolutionSize;\n> > status = Adjusted;\n> > adjusted = true;\n> > }\n> > @@ -224,12 +238,12 @@ PipelineHandlerVirtual::generateConfiguration(Camera *camera,\n> >\n> > std::map> streamFormats;\n> > PixelFormat pixelFormat = formats::NV12;\n> > - streamFormats[pixelFormat] = { { data->minResolutionSize_,\n> > - data->maxResolutionSize_ } };\n> > + streamFormats[pixelFormat] = { { data->config_.minResolutionSize,\n> > + data->config_.maxResolutionSize } };\n> > StreamFormats formats(streamFormats);\n> > StreamConfiguration cfg(formats);\n> > cfg.pixelFormat = pixelFormat;\n> > - cfg.size = data->maxResolutionSize_;\n> > + cfg.size = data->config_.maxResolutionSize;\n> > cfg.bufferCount = VirtualCameraConfiguration::kBufferCount;\n> >\n> > config->addConfiguration(cfg);\n> > @@ -246,6 +260,7 @@ int PipelineHandlerVirtual::configure(Camera *camera,\n> > VirtualCameraData *data = cameraData(camera);\n> > for (auto [i, c] : utils::enumerate(*config)) {\n> > c.setStream(&data->streamConfigs_[i].stream);\n> > + /* Start reading the images/generating test patterns */\n> > data->streamConfigs_[i].frameGenerator->configure(c.size);\n> > }\n> >\n> > @@ -315,56 +330,67 @@ bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator\n> >\n> > created_ = true;\n> >\n> > - /* \\todo Add virtual cameras according to a config file. */\n> > -\n> > - std::vector supportedResolutions;\n> > - supportedResolutions.resize(2);\n> > - supportedResolutions[0] = { .size = Size(1920, 1080), .frameRates = { 30 } };\n> > - supportedResolutions[1] = { .size = Size(1280, 720), .frameRates = { 30 } };\n> > -\n> > - std::unique_ptr data =\n> > - std::make_unique(this, supportedResolutions);\n> > -\n> > - data->properties_.set(properties::Location, properties::CameraLocationFront);\n> > - data->properties_.set(properties::Model, \"Virtual Video Device\");\n> > - data->properties_.set(properties::PixelArrayActiveAreas, { Rectangle(Size(1920, 1080)) });\n> > -\n> > - /* \\todo Set FrameDurationLimits based on config. */\n> > - ControlInfoMap::Map controls;\n> > - int64_t min_frame_duration = 33333, max_frame_duration = 33333;\n> > - controls[&controls::FrameDurationLimits] = ControlInfo(min_frame_duration, max_frame_duration);\n> > - std::vector supportedFaceDetectModes{\n> > - static_cast(controls::draft::FaceDetectModeOff),\n> > - };\n> > - controls[&controls::draft::FaceDetectMode] = ControlInfo(supportedFaceDetectModes);\n> > - data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);\n> > -\n> > - /* Create and register the camera. */\n> > - std::set streams;\n> > - for (auto &streamConfig : data->streamConfigs_)\n> > - streams.insert(&streamConfig.stream);\n> > + File file(configurationFile(\"virtual\", \"virtual.yaml\"));\n>\n> This tells me that the sample virtual.yaml file should probably be\n> installed in the libcamera data path...\n\nYes, updated the README.md.\n\n\n>\n> This version doesn't currently apply cleanly to master, but I did have\n> an branch with it on - and it rebased easily.\n\nRebased on tot in the next version.\n\n\n>\n> But it needs a new version posting at least to solve the SoB issues and\n> so with the above tackled you can add\n>\n> Reviewed-by: Kieran Bingham \n>\n>\n> > + bool isOpen = file.open(File::OpenModeFlag::ReadOnly);\n> > + if (!isOpen) {\n> > + LOG(Virtual, Error) << \"Failed to open config file: \" << file.fileName();\n> > + return false;\n> > + }\n> >\n> > - const std::string id = \"Virtual0\";\n> > - std::shared_ptr camera = Camera::create(std::move(data), id, streams);\n> > + ConfigParser parser;\n> > + auto configData = parser.parseConfigFile(file, this);\n> > + if (configData.size() == 0) {\n> > + LOG(Virtual, Error) << \"Failed to parse any cameras from the config file: \"\n> > + << file.fileName();\n> > + return false;\n> > + }\n> >\n> > - initFrameGenerator(camera.get());\n> > + /* Configure and register cameras with configData */\n> > + for (auto &data : configData) {\n> > + std::set streams;\n> > + for (auto &streamConfig : data->streamConfigs_)\n> > + streams.insert(&streamConfig.stream);\n> > + std::string id = data->config_.id;\n> > + std::shared_ptr camera = Camera::create(std::move(data), id, streams);\n> > +\n> > + if (!initFrameGenerator(camera.get())) {\n> > + LOG(Virtual, Error) << \"Failed to initialize frame \"\n> > + << \"generator for camera: \" << id;\n> > + continue;\n> > + }\n> >\n> > - registerCamera(std::move(camera));\n> > + registerCamera(std::move(camera));\n> > + }\n> >\n> > resetCreated_ = true;\n> >\n> > return true;\n> > }\n> >\n> > -void PipelineHandlerVirtual::initFrameGenerator(Camera *camera)\n> > +bool PipelineHandlerVirtual::initFrameGenerator(Camera *camera)\n> > {\n> > auto data = cameraData(camera);\n> > - for (auto &streamConfig : data->streamConfigs_) {\n> > - if (data->testPattern_ == TestPattern::DiagonalLines)\n> > - streamConfig.frameGenerator = std::make_unique();\n> > - else\n> > - streamConfig.frameGenerator = std::make_unique();\n> > - }\n> > + auto &frame = data->config_.frame;\n> > + std::visit(overloaded{\n> > + [&](TestPattern &testPattern) {\n> > + for (auto &streamConfig : data->streamConfigs_) {\n> > + if (testPattern == TestPattern::DiagonalLines)\n> > + streamConfig.frameGenerator = std::make_unique();\n> > + else\n> > + streamConfig.frameGenerator = std::make_unique();\n> > + }\n> > + },\n> > + [&](ImageFrames &imageFrames) {\n> > + for (auto &streamConfig : data->streamConfigs_)\n> > + streamConfig.frameGenerator = ImageFrameGenerator::create(imageFrames);\n> > + } },\n> > + frame);\n> > +\n> > + for (auto &streamConfig : data->streamConfigs_)\n> > + if (!streamConfig.frameGenerator)\n> > + return false;\n> > +\n> > + return true;\n> > }\n> >\n> > REGISTER_PIPELINE_HANDLER(PipelineHandlerVirtual, \"virtual\")\n> > diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h\n> > index 0b15f8d9c..8c4d6ae64 100644\n> > --- a/src/libcamera/pipeline/virtual/virtual.h\n> > +++ b/src/libcamera/pipeline/virtual/virtual.h\n> > @@ -7,6 +7,8 @@\n> >\n> > #pragma once\n> >\n> > +#include \n> > +#include \n> > #include \n> >\n> > #include \n> > @@ -15,10 +17,14 @@\n> > #include \"libcamera/internal/camera.h\"\n> > #include \"libcamera/internal/pipeline_handler.h\"\n> >\n> > +#include \"frame_generator.h\"\n> > +#include \"image_frame_generator.h\"\n> > #include \"test_pattern_generator.h\"\n> >\n> > namespace libcamera {\n> >\n> > +using VirtualFrame = std::variant;\n> > +\n> > class VirtualCameraData : public Camera::Private\n> > {\n> > public:\n> > @@ -26,23 +32,28 @@ public:\n> >\n> > struct Resolution {\n> > Size size;\n> > - std::vector frameRates;\n> > + std::vector frameRates;\n> > };\n> > struct StreamConfig {\n> > Stream stream;\n> > std::unique_ptr frameGenerator;\n> > };\n> > + /* The config file is parsed to the Configuration struct */\n> > + struct Configuration {\n> > + std::string id;\n> > + std::vector resolutions;\n> > + VirtualFrame frame;\n> > +\n> > + Size maxResolutionSize;\n> > + Size minResolutionSize;\n> > + };\n> >\n> > VirtualCameraData(PipelineHandler *pipe,\n> > const std::vector &supportedResolutions);\n> >\n> > ~VirtualCameraData() = default;\n> >\n> > - TestPattern testPattern_ = TestPattern::ColorBars;\n> > -\n> > - const std::vector supportedResolutions_;\n> > - Size maxResolutionSize_;\n> > - Size minResolutionSize_;\n> > + Configuration config_;\n> >\n> > std::vector streamConfigs_;\n> > };\n> > --\n> > 2.47.0.rc0.187.ge670bccf7e-goog\n> >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 391D6C3272\n\tfor ;\n\tTue, 22 Oct 2024 07:46:21 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 716EE653AB;\n\tTue, 22 Oct 2024 09:46:20 +0200 (CEST)","from mail-lj1-x234.google.com (mail-lj1-x234.google.com\n\t[IPv6:2a00:1450:4864:20::234])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D2D346539F\n\tfor ;\n\tTue, 22 Oct 2024 09:46:17 +0200 (CEST)","by mail-lj1-x234.google.com with SMTP id\n\t38308e7fff4ca-2fc968b3545so2017611fa.2\n\tfor ;\n\tTue, 22 Oct 2024 00:46:17 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=chromium.org header.i=@chromium.org\n\theader.b=\"avPEAzb/\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=chromium.org; s=google; t=1729583177; x=1730187977;\n\tdarn=lists.libcamera.org; \n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:from:to:cc:subject:date\n\t:message-id:reply-to;\n\tbh=oFYUefZjc4acANF0z9/2FgmlowZR7EG6nEqa+pKVQNI=;\n\tb=avPEAzb/Wb8jS34sSxgvs7JgJdvsJsh2a0W+wmeSMj42HFsKaGfXj99pwh1R2g0k7s\n\t6hO0afQ4egmUtPWLWlw2gnkJ7CUKuPr9a5q+2vbPYBFAMDOqFtS12FjS17YoGF7rt7JS\n\tHMUOVABg73r8kgGpXvJyXse2RvO7+HDRDz5cE=","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20230601; t=1729583177; x=1730187977;\n\th=content-transfer-encoding:cc:to:subject:message-id:date:from\n\t:in-reply-to:references:mime-version:x-gm-message-state:from:to:cc\n\t:subject:date:message-id:reply-to;\n\tbh=oFYUefZjc4acANF0z9/2FgmlowZR7EG6nEqa+pKVQNI=;\n\tb=uI5fTRzbklJBaNcLsFv4pYC/CuXOLA9Fv2PM+prysklGwnMQyjuA1QtJt8FQSdIvg+\n\t7wHZL74DstoeZqNL3PNUZMLR4pup2YeNjWodA15jhB2AUtuFrTCF9gr6BgdopuC/5+VT\n\tysh/cdk+cezq/SuEpElG4qc3Iq6aPUeZLLrHVbg/OBaqsCl0SMVXiZYrnMHFOpyYmHLB\n\tLKr7uvsmhniZ1/Wk9b0YeogbwnGkSwiwGB0DVHUxREWCBBtTxUB/Ld3rN32a8rXvZ5FA\n\tOpCwHuvLac74X9HX7iNH93RA3fpoZhulV7Ux7BBlbEZoAJCSgbF+7KlR9kOixR0dsAgD\n\tvaSg==","X-Gm-Message-State":"AOJu0Yz7pWqjGY/paInNuSyR3dvjjguRPWO0rXAZIjm+WuaCUkLZ1CvX\n\tEGaO6+2fkLy4Vwut07NKEtzRQjhNiCXznfTNKUmsvKyZLxk0nAuCa5YnbP51FM+7+YB6yWlMIMM\n\tDBjxRaDqOsYbVzMBY/s+yADnoIVwPHMCkIWaB","X-Google-Smtp-Source":"AGHT+IFGdHKJX/diu8w3N0Hp6/9la0wdYzfGHaKtw4HJqODxt+y8AUBB49GGaiKNdwtTw8BBtQ+aocsG2AcIbBp3d7Y=","X-Received":"by 2002:a2e:610a:0:b0:2fb:6027:7bfd with SMTP id\n\t38308e7fff4ca-2fc933298a0mr6490151fa.27.1729583176775;\n\tTue, 22 Oct 2024 00:46:16 -0700 (PDT)","MIME-Version":"1.0","References":"<20241004095946.264537-1-chenghaoyang@google.com>\n\t<20241004095946.264537-7-chenghaoyang@google.com>\n\t<172955351674.1470935.9237357010380500470@ping.linuxembedded.co.uk>","In-Reply-To":"<172955351674.1470935.9237357010380500470@ping.linuxembedded.co.uk>","From":"Cheng-Hao Yang ","Date":"Tue, 22 Oct 2024 15:46:05 +0800","Message-ID":"","Subject":"Re: [PATCH 15 6/7] libcamera: virtual: Read config and register\n\tcameras based on the config","To":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tHarvey Yang , \n\tKonami Shu , Yunke Cao , \n\tTomasz Figa ","Content-Type":"text/plain; charset=\"UTF-8\"","Content-Transfer-Encoding":"quoted-printable","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31872,"web_url":"https://patchwork.libcamera.org/comment/31872/","msgid":"<172958388179.2721096.16253835886526595075@ping.linuxembedded.co.uk>","date":"2024-10-22T07:58:01","subject":"Re: [PATCH 15 6/7] libcamera: virtual: Read config and register\n\tcameras based on the config","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Quoting Cheng-Hao Yang (2024-10-22 08:46:05)\n> Hi Kieran,\n> \n> On Tue, Oct 22, 2024 at 7:31 AM Kieran Bingham\n> wrote:\n> >\n> > Quoting Harvey Yang (2024-10-04 10:59:19)\n> > > This patch introduces the configuration file for Virtual Pipeline\n> > > Handler. The config file is written in yaml, and the format is\n> > > documented in `README.md`.\n> > >\n> > > The config file will define the camera with IDs, supported formats and\n> > > image sources, etc. In the default config file, only Test Patterns are\n> > > used. Developers can use real images loading if desired.\n> > >\n> > > Signed-off-by: Konami Shu \n> > > Co-developed-by: Harvey Yang \n> > > Co-developed-by: Yunke Cao \n> > > Co-developed-by: Tomasz Figa \n> >\n> > I think we determined Co-developed-by: need SoBs.\n> \n> Yes, updated. I didn't upload a new version to avoid the spam.\n\nAck,\n\n> \n> >\n> > Checkstyle reports:\n> >\n> > -----------------------------------------------------------------------------------------------------------------\n> > 83f896b4afa8e5e7a4029d1e07bcb5dfe8cc5fb2 libcamera: virtual: Read config and register cameras based on the config\n> > -----------------------------------------------------------------------------------------------------------------\n> > No 'Signed-off-by' trailer matching author 'Harvey Yang ', see Documentation/contributing.rst\n> > ---\n> > 1 potential issue detected, please review\n> >\n> > And that will apply to all the above.\n> \n> Done.\n> \n\nThanks\n\n> >\n> >\n> > > ---\n> > > src/libcamera/pipeline/virtual/README.md | 65 +++++\n> > > .../pipeline/virtual/config_parser.cpp | 263 ++++++++++++++++++\n> > > .../pipeline/virtual/config_parser.h | 39 +++\n> > > .../pipeline/virtual/data/virtual.yaml | 36 +++\n> > > .../virtual/image_frame_generator.cpp | 16 +-\n> > > .../pipeline/virtual/image_frame_generator.h | 5 +-\n> > > src/libcamera/pipeline/virtual/meson.build | 1 +\n> > > src/libcamera/pipeline/virtual/virtual.cpp | 126 +++++----\n> > > src/libcamera/pipeline/virtual/virtual.h | 23 +-\n> > > 9 files changed, 504 insertions(+), 70 deletions(-)\n> > > create mode 100644 src/libcamera/pipeline/virtual/README.md\n> > > create mode 100644 src/libcamera/pipeline/virtual/config_parser.cpp\n> > > create mode 100644 src/libcamera/pipeline/virtual/config_parser.h\n> > > create mode 100644 src/libcamera/pipeline/virtual/data/virtual.yaml\n> > >\n> > > diff --git a/src/libcamera/pipeline/virtual/README.md b/src/libcamera/pipeline/virtual/README.md\n> > > new file mode 100644\n> > > index 000000000..5801e79b5\n> > > --- /dev/null\n> > > +++ b/src/libcamera/pipeline/virtual/README.md\n> > > @@ -0,0 +1,65 @@\n> > > +# Virtual Pipeline Handler\n> > > +\n> > > +Virtual pipeline handler emulates fake external camera(s) on ChromeOS for testing.\n> >\n> > It's not only on ChromeOS right ? It's just a virtual camera for any\n> > platform.\n> \n> Right, removed.\n> \n> >\n> >\n> >\n> > > +\n> > > +## Parse config file and register cameras\n> > > +\n> > > +- The config file is located at `src/libcamera/pipeline/virtual/data/virtual.yaml`\n> >\n> > I'm sure I mentioned this before - but this is the source location.\n> > While I preume the users would have to configure their 'installed'\n> > location. So this should probably reference the fact it would be\n> > installed in ...\n> >\n> > Err - where does it get installed ? What happens if libcamera is\n> > installed and tries to use the virtual pipeline handler here ?\n> >\n> > I don't see any reference in a meson file that installes\n> > data/virtual.yaml ?\n> >\n> >\n> > Later on virtual.yaml is mentioned as a sample, while up here it's\n> > specified as '\"The\" config file'. ?\n> >\n> > Maybe this should say \"A sample config file is located at ...\"\n> \n> Yes, it depends on if libcamera is installed. Updated and added\n> the installed path:\n> `shar/libcamera/pipeline/virtual/virtual.yaml`.\n\nI think that's the default - but it's probably worth mentioning in a way\nthat says it's will be installed in the 'datadir' which has a default\nlocation of 'share/libcamera/...' as this is configurable when doing the\nmeson setup stage.\n\n> > > +\n> > > +### Config File Format\n> > > +The config file contains the information about cameras' properties to register.\n> > > +The config file should be a yaml file with dictionary of the cameraIds\n> > > +associated with their properties as top level. The default value will be applied\n> > > +when any property is empty.\n> > > +\n> > > +Each camera block is a dictionary, containing the following keys:\n> > > +- `supported_formats` (list of `VirtualCameraData::Resolution`, optional) :\n> > > + List of supported resolution and frame rates of the emulated camera\n> > > + - `width` (`unsigned int`, default=1920): Width of the window resolution.\n> > > + This needs to be even.\n> > > + - `height` (`unsigned int`, default=1080): Height of the window resolution.\n> > > + - `frame_rates` (list of `int`, default=`[30,60]` ): Range of the frame\n> > > + rate (per second). If the list contains one value, it's the lower bound\n> > > + and the upper bound. If the list contains two values, the first is the\n> > > + lower bound and the second is the upper bound. No other number of values\n> > > + is allowed.\n> > > +- `test_pattern` (`string`): Which test pattern to use as frames. The options\n> > > + are \"bars\", \"lines\". Cannot be set with `frames`.\n> > > +- `frames` (dictionary):\n> > > + - `path` (`string`): Path to an image, or path to a directory of a series of\n> > > + images. Cannot be set with `test_pattern`.\n> > > + - The test patterns are \"bars\" which means color bars, and \"lines\" which\n> > > + means diagonal lines.\n> >\n> > Does this bullet point belong to the 'test_pattern' section above rather\n> > than frames ?\n> \n> Right, thanks! Updated.\n> \n> >\n> > > + - The path to an image has \".jpg\" extension.\n> > > + - The path to a directory ends with \"/\". The name of the images in the\n> > > + directory are \"{n}.jpg\" with {n} is the sequence of images starting with 0.\n> > > +- `location` (`string`, default=\"front\"): The location of the camera. Support\n> > > + \"front\" and \"back\". This is displayed in qcam camera selection window but\n> >\n> > I don't think it's relevant to mention 'qcam' here. Setting the location\n> > defines the location property of the camera which is exposed by the\n> > camera in /any/ application. Not just qcam.\n> \n> Right, removed this and the one in `model`.\n\nAha, I missed that one ;-)\n\n> \n> >\n> >\n> > > + this does not change the output.\n> > > +- `model` (`string`, default=\"Unknown\"): The model name of the camera.\n> > > + This is displayed in qcam camera selection window but this does not change the output.\n> > > +\n> > > +Check `data/virtual.yaml` as the sample config file.\n> > > +\n> > > +### Implementation\n> > > +\n> > > +`Parser` class provides methods to parse the config file to register cameras\n> > > +in Virtual Pipeline Handler. `parseConfigFile()` is exposed to use in\n> > > +Virtual Pipeline Handler.\n> > > +\n> > > +This is the procedure of the Parser class:\n> > > +1. `parseConfigFile()` parses the config file to `YamlObject` using `YamlParser::parse()`.\n> > > + - Parse the top level of config file which are the camera ids and look into\n> > > + each camera properties.\n> > > +2. For each camera, `parseCameraConfigData()` returns a camera with the configuration.\n> > > + - The methods in the next step fill the data with the pointer to the Camera object.\n> > > + - If the config file contains invalid configuration, this method returns\n> > > + nullptr. The camera will be skipped.\n> > > +3. Parse each property and register the data.\n> > > + - `parseSupportedFormats()`: Parses `supported_formats` in the config, which\n> > > + contains resolutions and frame rates.\n> > > + - `parseFrameGenerator()`: Parses `test_pattern` or `frames` in the config.\n> > > + - `parseLocation()`: Parses `location` in the config.\n> > > + - `parseModel()`: Parses `model` in the config.\n> > > +4. Back to `parseConfigFile()` and append the camera configuration.\n> > > +5. Returns a list of camera configurations.\n> > > diff --git a/src/libcamera/pipeline/virtual/config_parser.cpp b/src/libcamera/pipeline/virtual/config_parser.cpp\n> > > new file mode 100644\n> > > index 000000000..467dde485\n> > > --- /dev/null\n> > > +++ b/src/libcamera/pipeline/virtual/config_parser.cpp\n> > > @@ -0,0 +1,263 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2024, Google Inc.\n> > > + *\n> > > + * config_parser.cpp - Virtual cameras helper to parse config file\n> >\n> > Drop 'config_parser.cpp - ' We stopped putting file names in their own\n> > files as they bitrot and don't add value\n> \n> Done\n> \n> >\n> > > + */\n> > > +\n> > > +#include \"config_parser.h\"\n> > > +\n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \n> > > +\n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \"libcamera/internal/pipeline_handler.h\"\n> > > +#include \"libcamera/internal/yaml_parser.h\"\n> > > +\n> > > +#include \"virtual.h\"\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +LOG_DECLARE_CATEGORY(Virtual)\n> > > +\n> > > +std::vector>\n> > > +ConfigParser::parseConfigFile(File &file, PipelineHandler *pipe)\n> > > +{\n> > > + std::vector> configurations;\n> > > +\n> > > + std::unique_ptr cameras = YamlParser::parse(file);\n> > > + if (!cameras) {\n> > > + LOG(Virtual, Error) << \"Failed to pass config file.\";\n> > > + return configurations;\n> > > + }\n> > > +\n> > > + if (!cameras->isDictionary()) {\n> > > + LOG(Virtual, Error) << \"Config file is not a dictionary at the top level.\";\n> > > + return configurations;\n> > > + }\n> > > +\n> > > + /* Look into the configuration of each camera */\n> > > + for (const auto &[cameraId, cameraConfigData] : cameras->asDict()) {\n> > > + std::unique_ptr data =\n> > > + parseCameraConfigData(cameraConfigData, pipe);\n> > > + /* Parse configData to data */\n> > > + if (!data) {\n> > > + /* Skip the camera if it has invalid config */\n> > > + LOG(Virtual, Error) << \"Failed to parse config of the camera: \"\n> > > + << cameraId;\n> > > + continue;\n> > > + }\n> > > +\n> > > + data->config_.id = cameraId;\n> > > + ControlInfoMap::Map controls;\n> > > + /* todo: Check which resolution's frame rate to be reported */\n> > > + controls[&controls::FrameDurationLimits] =\n> > > + ControlInfo(1000000 / data->config_.resolutions[0].frameRates[1],\n> > > + 1000000 / data->config_.resolutions[0].frameRates[0]);\n> > > +\n> > > + std::vector supportedFaceDetectModes{\n> > > + static_cast(controls::draft::FaceDetectModeOff),\n> > > + };\n> > > + controls[&controls::draft::FaceDetectMode] = ControlInfo(supportedFaceDetectModes);\n> > > +\n> > > + data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);\n> > > + configurations.push_back(std::move(data));\n> > > + }\n> > > +\n> > > + return configurations;\n> > > +}\n> > > +\n> > > +std::unique_ptr\n> > > +ConfigParser::parseCameraConfigData(const YamlObject &cameraConfigData,\n> > > + PipelineHandler *pipe)\n> > > +{\n> > > + std::vector resolutions;\n> > > + if (parseSupportedFormats(cameraConfigData, &resolutions))\n> > > + return nullptr;\n> > > +\n> > > + std::unique_ptr data =\n> > > + std::make_unique(pipe, resolutions);\n> > > +\n> > > + if (parseFrameGenerator(cameraConfigData, data.get()))\n> > > + return nullptr;\n> > > +\n> > > + if (parseLocation(cameraConfigData, data.get()))\n> > > + return nullptr;\n> > > +\n> > > + if (parseModel(cameraConfigData, data.get()))\n> > > + return nullptr;\n> > > +\n> > > + return data;\n> > > +}\n> > > +\n> > > +int ConfigParser::parseSupportedFormats(const YamlObject &cameraConfigData,\n> > > + std::vector *resolutions)\n> > > +{\n> > > + if (cameraConfigData.contains(\"supported_formats\")) {\n> > > + const YamlObject &supportedResolutions = cameraConfigData[\"supported_formats\"];\n> > > +\n> > > + for (const YamlObject &supportedResolution : supportedResolutions.asList()) {\n> > > + unsigned int width = supportedResolution[\"width\"].get(1920);\n> > > + unsigned int height = supportedResolution[\"height\"].get(1080);\n> > > + if (width == 0 || height == 0) {\n> > > + LOG(Virtual, Error) << \"Invalid width or/and height\";\n> > > + return -EINVAL;\n> > > + }\n> > > + if (width % 2 != 0) {\n> > > + LOG(Virtual, Error) << \"Invalid width: width needs to be even\";\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + std::vector frameRates;\n> > > + if (supportedResolution.contains(\"frame_rates\")) {\n> > > + auto frameRatesList =\n> > > + supportedResolution[\"frame_rates\"].getList();\n> > > + if (!frameRatesList || (frameRatesList->size() != 1 &&\n> > > + frameRatesList->size() != 2)) {\n> > > + LOG(Virtual, Error) << \"Invalid frame_rates: either one or two values\";\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + if (frameRatesList->size() == 2 &&\n> > > + frameRatesList.value()[0] > frameRatesList.value()[1]) {\n> > > + LOG(Virtual, Error) << \"frame_rates's first value(lower bound)\"\n> > > + << \" is higher than the second value(upper bound)\";\n> > > + return -EINVAL;\n> > > + }\n> > > + /*\n> > > + * Push the min and max framerates. A\n> > > + * single rate is duplicated.\n> > > + */\n> > > + frameRates.push_back(frameRatesList.value().front());\n> > > + frameRates.push_back(frameRatesList.value().back());\n> > > + } else {\n> > > + frameRates.push_back(30);\n> > > + frameRates.push_back(60);\n> > > + }\n> > > +\n> > > + resolutions->emplace_back(\n> > > + VirtualCameraData::Resolution{ Size{ width, height },\n> > > + frameRates });\n> > > + }\n> > > + } else {\n> > > + resolutions->emplace_back(\n> > > + VirtualCameraData::Resolution{ Size{ 1920, 1080 },\n> > > + { 30, 60 } });\n> > > + }\n> > > +\n> > > + return 0;\n> > > +}\n> > > +\n> > > +int ConfigParser::parseFrameGenerator(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> > > +{\n> > > + const std::string testPatternKey = \"test_pattern\";\n> > > + const std::string framesKey = \"frames\";\n> > > + if (cameraConfigData.contains(testPatternKey)) {\n> > > + if (cameraConfigData.contains(framesKey)) {\n> > > + LOG(Virtual, Error) << \"A camera should use either \"\n> > > + << testPatternKey << \" or \" << framesKey;\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + auto testPattern = cameraConfigData[testPatternKey].get(\"\");\n> > > +\n> > > + if (testPattern == \"bars\") {\n> > > + data->config_.frame = TestPattern::ColorBars;\n> > > + } else if (testPattern == \"lines\") {\n> > > + data->config_.frame = TestPattern::DiagonalLines;\n> > > + } else {\n> > > + LOG(Virtual, Debug) << \"Test pattern: \" << testPattern\n> > > + << \" is not supported\";\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + return 0;\n> > > + }\n> > > +\n> > > + const YamlObject &frames = cameraConfigData[framesKey];\n> > > +\n> > > + /* When there is no frames provided in the config file, use color bar test pattern */\n> > > + if (!frames) {\n> > > + data->config_.frame = TestPattern::ColorBars;\n> > > + return 0;\n> > > + }\n> > > +\n> > > + if (!frames.isDictionary()) {\n> > > + LOG(Virtual, Error) << \"'frames' is not a dictionary.\";\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + auto path = frames[\"path\"].get();\n> > > +\n> > > + if (!path) {\n> > > + LOG(Virtual, Error) << \"Test pattern or path should be specified.\";\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + std::vector files;\n> > > +\n> > > + switch (std::filesystem::symlink_status(*path).type()) {\n> > > + case std::filesystem::file_type::regular:\n> > > + files.push_back(*path);\n> > > + break;\n> > > +\n> > > + case std::filesystem::file_type::directory:\n> > > + for (const auto &dentry : std::filesystem::directory_iterator{ *path }) {\n> > > + if (dentry.is_regular_file())\n> > > + files.push_back(dentry.path());\n> > > + }\n> > > +\n> > > + std::sort(files.begin(), files.end(), [](const auto &a, const auto &b) {\n> > > + return ::strverscmp(a.c_str(), b.c_str()) < 0;\n> > > + });\n> > > +\n> > > + if (files.empty()) {\n> > > + LOG(Virtual, Error) << \"Directory has no files: \" << *path;\n> > > + return -EINVAL;\n> > > + }\n> > > + break;\n> > > +\n> > > + default:\n> > > + LOG(Virtual, Error) << \"Frame: \" << *path << \" is not supported\";\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + data->config_.frame = ImageFrames{ std::move(files) };\n> > > +\n> > > + return 0;\n> > > +}\n> > > +\n> > > +int ConfigParser::parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> > > +{\n> > > + std::string location = cameraConfigData[\"location\"].get(\"front\");\n> > > +\n> > > + /* Default value is properties::CameraLocationFront */\n> > > + if (location == \"front\") {\n> > > + data->properties_.set(properties::Location,\n> > > + properties::CameraLocationFront);\n> > > + } else if (location == \"back\") {\n> > > + data->properties_.set(properties::Location,\n> > > + properties::CameraLocationBack);\n> > > + } else {\n> > > + LOG(Virtual, Error) << \"location: \" << location\n> > > + << \" is not supported\";\n> >\n> > What about external ?\n> >\n> > I think the control framework provides a map of string to id for the\n> > enum types in fact, so you can do something like:\n> >\n> >\n> > int ConfigParser::parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> > {\n> > std::string location = cameraConfigData[\"location\"].get(\"front\");\n> >\n> > auto it = properties::LocationNameValueMap.find(location);\n> > if (it == properties::LocationNameValueMap.end()) {\n> > LOG(Virtual, Error)\n> > << \"location: \" << location << \" is not supported\";\n> >\n> > return -EINVAL;\n> > }\n> >\n> > data->properties_.set(properties::Location, it->second);\n> >\n> > return 0;\n> > }\n> >\n> > Which is probably more future proof too if we for some reason add any\n> > other locations.\n> \n> Makes sense, although the value has the prefix \"CameraLocationXXX\".\n> Updated.\n\n\nThe enum values have CameraLocation indeed- but they are stored in the\nLocationEnum because they are associated with the Location property\nwhich has an associated array and map to convert between integer and\nControlValues:\n\nSee $(BUILDDIR)/include/libcamera/property_ids.h\n\nenum LocationEnum {\n CameraLocationFront = 0,\n CameraLocationBack = 1,\n CameraLocationExternal = 2,\n};\nextern const std::array LocationValues;\nextern const std::map LocationNameValueMap;\nextern const Control Location;\n\nwhich is autogenerated.\n\n\n> \n> >\n> >\n> >\n> > > + return -EINVAL;\n> > > + }\n> > > +\n> > > + return 0;\n> > > +}\n> > > +\n> > > +int ConfigParser::parseModel(const YamlObject &cameraConfigData, VirtualCameraData *data)\n> > > +{\n> > > + std::string model = cameraConfigData[\"model\"].get(\"Unknown\");\n> > > +\n> > > + data->properties_.set(properties::Model, model);\n> > > +\n> > > + return 0;\n> > > +}\n> > > +\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/libcamera/pipeline/virtual/config_parser.h b/src/libcamera/pipeline/virtual/config_parser.h\n> > > new file mode 100644\n> > > index 000000000..9abba62d0\n> > > --- /dev/null\n> > > +++ b/src/libcamera/pipeline/virtual/config_parser.h\n> > > @@ -0,0 +1,39 @@\n> > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > > +/*\n> > > + * Copyright (C) 2024, Google Inc.\n> > > + *\n> > > + * config_parser.h - Virtual cameras helper to parse config file\n> >\n> > Drop 'config_parser.h - ' from here.\n> \n> Done\n> \n> >\n> > > + */\n> > > +\n> > > +#pragma once\n> > > +\n> > > +#include \n> > > +#include \n> > > +\n> > > +#include \n> > > +\n> > > +#include \"libcamera/internal/pipeline_handler.h\"\n> > > +#include \"libcamera/internal/yaml_parser.h\"\n> > > +\n> > > +#include \"virtual.h\"\n> > > +\n> > > +namespace libcamera {\n> > > +\n> > > +class ConfigParser\n> > > +{\n> > > +public:\n> > > + std::vector>\n> > > + parseConfigFile(File &file, PipelineHandler *pipe);\n> > > +\n> > > +private:\n> > > + std::unique_ptr\n> > > + parseCameraConfigData(const YamlObject &cameraConfigData, PipelineHandler *pipe);\n> > > +\n> > > + int parseSupportedFormats(const YamlObject &cameraConfigData,\n> > > + std::vector *resolutions);\n> > > + int parseFrameGenerator(const YamlObject &cameraConfigData, VirtualCameraData *data);\n> > > + int parseLocation(const YamlObject &cameraConfigData, VirtualCameraData *data);\n> > > + int parseModel(const YamlObject &cameraConfigData, VirtualCameraData *data);\n> > > +};\n> > > +\n> > > +} /* namespace libcamera */\n> > > diff --git a/src/libcamera/pipeline/virtual/data/virtual.yaml b/src/libcamera/pipeline/virtual/data/virtual.yaml\n> > > new file mode 100644\n> > > index 000000000..6b73ddf2b\n> > > --- /dev/null\n> > > +++ b/src/libcamera/pipeline/virtual/data/virtual.yaml\n> > > @@ -0,0 +1,36 @@\n> > > +# SPDX-License-Identifier: CC0-1.0\n> > > +%YAML 1.1\n> > > +---\n> > > +\"Virtual0\":\n> > > + supported_formats:\n> > > + - width: 1920\n> > > + height: 1080\n> > > + frame_rates:\n> > > + - 30\n> > > + - 60\n> > > + - width: 1680\n> > > + height: 1050\n> > > + frame_rates:\n> > > + - 70\n> > > + - 80\n> > > + test_pattern: \"lines\"\n> > > + location: \"front\"\n> > > + model: \"Virtual Video Device\"\n> > > +\"Virtual1\":\n> > > + supported_formats:\n> > > + - width: 800\n> > > + height: 600\n> > > + frame_rates:\n> > > + - 60\n> > > + test_pattern: \"bars\"\n> > > + location: \"back\"\n> > > + model: \"Virtual Video Device1\"\n> > > +\"Virtual2\":\n> > > + supported_formats:\n> > > + - width: 400\n> > > + height: 300\n> > > + test_pattern: \"lines\"\n> > > + location: \"front\"\n> > > + model: \"Virtual Video Device2\"\n> > > +\"Virtual3\":\n> > > + test_pattern: \"bars\"\n> > > diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.cpp b/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> > > index 04382beb7..36bdc20e5 100644\n> > > --- a/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> > > +++ b/src/libcamera/pipeline/virtual/image_frame_generator.cpp\n> > > @@ -40,15 +40,7 @@ ImageFrameGenerator::create(ImageFrames &imageFrames)\n> > > * For each file in the directory, load the image,\n> > > * convert it to NV12, and store the pointer.\n> > > */\n> > > - for (unsigned int i = 0; i < imageFrames.number.value_or(1); i++) {\n> > > - std::filesystem::path path;\n> > > - if (!imageFrames.number)\n> > > - /* If the path is to an image */\n> > > - path = imageFrames.path;\n> > > - else\n> > > - /* If the path is to a directory */\n> > > - path = imageFrames.path / (std::to_string(i) + \".jpg\");\n> > > -\n> > > + for (std::filesystem::path path : imageFrames.files) {\n> > > File file(path);\n> > > if (!file.open(File::OpenModeFlag::ReadOnly)) {\n> > > LOG(Virtual, Error) << \"Failed to open image file \" << file.fileName()\n> > > @@ -88,6 +80,8 @@ ImageFrameGenerator::create(ImageFrames &imageFrames)\n> > > Size(width, height) });\n> > > }\n> > >\n> > > + ASSERT(!imageFrameGenerator->imageFrameDatas_.empty());\n> > > +\n> > > return imageFrameGenerator;\n> > > }\n> > >\n> > > @@ -104,7 +98,7 @@ void ImageFrameGenerator::configure(const Size &size)\n> > > frameIndex_ = 0;\n> > > parameter_ = 0;\n> > >\n> > > - for (unsigned int i = 0; i < imageFrames_->number.value_or(1); i++) {\n> > > + for (unsigned int i = 0; i < imageFrameDatas_.size(); i++) {\n> > > /* Scale the imageFrameDatas_ to scaledY and scaledUV */\n> > > unsigned int halfSizeWidth = (size.width + 1) / 2;\n> > > unsigned int halfSizeHeight = (size.height + 1) / 2;\n> > > @@ -139,7 +133,7 @@ int ImageFrameGenerator::generateFrame(const Size &size, const FrameBuffer *buff\n> > > auto planes = mappedFrameBuffer.planes();\n> > >\n> > > /* Loop only around the number of images available */\n> > > - frameIndex_ %= imageFrames_->number.value_or(1);\n> > > + frameIndex_ %= imageFrameDatas_.size();\n> > >\n> > > /* Write the scaledY and scaledUV to the mapped frame buffer */\n> > > libyuv::NV12Copy(scaledFrameDatas_[frameIndex_].Y.get(), size.width,\n> > > diff --git a/src/libcamera/pipeline/virtual/image_frame_generator.h b/src/libcamera/pipeline/virtual/image_frame_generator.h\n> > > index a68094a66..8554d647d 100644\n> > > --- a/src/libcamera/pipeline/virtual/image_frame_generator.h\n> > > +++ b/src/libcamera/pipeline/virtual/image_frame_generator.h\n> > > @@ -10,9 +10,9 @@\n> > >\n> > > #include \n> > > #include \n> > > -#include \n> > > #include \n> > > #include \n> > > +#include \n> > >\n> > > #include \"frame_generator.h\"\n> > >\n> > > @@ -20,8 +20,7 @@ namespace libcamera {\n> > >\n> > > /* Frame configuration provided by the config file */\n> > > struct ImageFrames {\n> > > - std::filesystem::path path;\n> > > - std::optional number;\n> > > + std::vector files;\n> > > };\n> > >\n> > > class ImageFrameGenerator : public FrameGenerator\n> > > diff --git a/src/libcamera/pipeline/virtual/meson.build b/src/libcamera/pipeline/virtual/meson.build\n> > > index bb38c193c..4786fe2e0 100644\n> > > --- a/src/libcamera/pipeline/virtual/meson.build\n> > > +++ b/src/libcamera/pipeline/virtual/meson.build\n> > > @@ -1,6 +1,7 @@\n> > > # SPDX-License-Identifier: CC0-1.0\n> > >\n> > > libcamera_internal_sources += files([\n> > > + 'config_parser.cpp',\n> > > 'image_frame_generator.cpp',\n> > > 'test_pattern_generator.cpp',\n> > > 'virtual.cpp',\n> > > diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp\n> > > index 1ad7417c7..cafd395c3 100644\n> > > --- a/src/libcamera/pipeline/virtual/virtual.cpp\n> > > +++ b/src/libcamera/pipeline/virtual/virtual.cpp\n> > > @@ -36,6 +36,9 @@\n> > > #include \"libcamera/internal/formats.h\"\n> > > #include \"libcamera/internal/framebuffer.h\"\n> > > #include \"libcamera/internal/pipeline_handler.h\"\n> > > +#include \"libcamera/internal/yaml_parser.h\"\n> > > +\n> > > +#include \"pipeline/virtual/config_parser.h\"\n> > >\n> > > namespace libcamera {\n> > >\n> > > @@ -54,6 +57,13 @@ uint64_t currentTimestamp()\n> > >\n> > > } /* namespace */\n> > >\n> > > +template\n> > > +struct overloaded : Ts... {\n> > > + using Ts::operator()...;\n> > > +};\n> > > +template\n> > > +overloaded(Ts...) -> overloaded;\n> > > +\n> > > class VirtualCameraConfiguration : public CameraConfiguration\n> > > {\n> > > public:\n> > > @@ -95,7 +105,7 @@ private:\n> > > return static_cast(camera->_d());\n> > > }\n> > >\n> > > - void initFrameGenerator(Camera *camera);\n> > > + bool initFrameGenerator(Camera *camera);\n> > >\n> > > DmaBufAllocator dmaBufAllocator_;\n> > >\n> > > @@ -104,15 +114,19 @@ private:\n> > >\n> > > VirtualCameraData::VirtualCameraData(PipelineHandler *pipe,\n> > > const std::vector &supportedResolutions)\n> > > - : Camera::Private(pipe), supportedResolutions_(supportedResolutions)\n> > > + : Camera::Private(pipe)\n> > > {\n> > > - for (const auto &resolution : supportedResolutions_) {\n> > > - if (minResolutionSize_.isNull() || minResolutionSize_ > resolution.size)\n> > > - minResolutionSize_ = resolution.size;\n> > > + config_.resolutions = supportedResolutions;\n> > > + for (const auto &resolution : config_.resolutions) {\n> > > + if (config_.minResolutionSize.isNull() || config_.minResolutionSize > resolution.size)\n> > > + config_.minResolutionSize = resolution.size;\n> > >\n> > > - maxResolutionSize_ = std::max(maxResolutionSize_, resolution.size);\n> > > + config_.maxResolutionSize = std::max(config_.maxResolutionSize, resolution.size);\n> > > }\n> > >\n> > > + properties_.set(properties::PixelArrayActiveAreas,\n> > > + { Rectangle(config_.maxResolutionSize) });\n> > > +\n> > > /* \\todo Support multiple streams and pass multi_stream_test */\n> > > streamConfigs_.resize(kMaxStream);\n> > > }\n> > > @@ -140,7 +154,7 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()\n> > > for (StreamConfiguration &cfg : config_) {\n> > > bool adjusted = false;\n> > > bool found = false;\n> > > - for (const auto &resolution : data_->supportedResolutions_) {\n> > > + for (const auto &resolution : data_->config_.resolutions) {\n> > > if (resolution.size.width == cfg.size.width &&\n> > > resolution.size.height == cfg.size.height) {\n> > > found = true;\n> > > @@ -155,7 +169,7 @@ CameraConfiguration::Status VirtualCameraConfiguration::validate()\n> > > * Defining the default logic in PipelineHandler to\n> > > * find the closest resolution would be nice.\n> > > */\n> > > - cfg.size = data_->maxResolutionSize_;\n> > > + cfg.size = data_->config_.maxResolutionSize;\n> > > status = Adjusted;\n> > > adjusted = true;\n> > > }\n> > > @@ -224,12 +238,12 @@ PipelineHandlerVirtual::generateConfiguration(Camera *camera,\n> > >\n> > > std::map> streamFormats;\n> > > PixelFormat pixelFormat = formats::NV12;\n> > > - streamFormats[pixelFormat] = { { data->minResolutionSize_,\n> > > - data->maxResolutionSize_ } };\n> > > + streamFormats[pixelFormat] = { { data->config_.minResolutionSize,\n> > > + data->config_.maxResolutionSize } };\n> > > StreamFormats formats(streamFormats);\n> > > StreamConfiguration cfg(formats);\n> > > cfg.pixelFormat = pixelFormat;\n> > > - cfg.size = data->maxResolutionSize_;\n> > > + cfg.size = data->config_.maxResolutionSize;\n> > > cfg.bufferCount = VirtualCameraConfiguration::kBufferCount;\n> > >\n> > > config->addConfiguration(cfg);\n> > > @@ -246,6 +260,7 @@ int PipelineHandlerVirtual::configure(Camera *camera,\n> > > VirtualCameraData *data = cameraData(camera);\n> > > for (auto [i, c] : utils::enumerate(*config)) {\n> > > c.setStream(&data->streamConfigs_[i].stream);\n> > > + /* Start reading the images/generating test patterns */\n> > > data->streamConfigs_[i].frameGenerator->configure(c.size);\n> > > }\n> > >\n> > > @@ -315,56 +330,67 @@ bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator\n> > >\n> > > created_ = true;\n> > >\n> > > - /* \\todo Add virtual cameras according to a config file. */\n> > > -\n> > > - std::vector supportedResolutions;\n> > > - supportedResolutions.resize(2);\n> > > - supportedResolutions[0] = { .size = Size(1920, 1080), .frameRates = { 30 } };\n> > > - supportedResolutions[1] = { .size = Size(1280, 720), .frameRates = { 30 } };\n> > > -\n> > > - std::unique_ptr data =\n> > > - std::make_unique(this, supportedResolutions);\n> > > -\n> > > - data->properties_.set(properties::Location, properties::CameraLocationFront);\n> > > - data->properties_.set(properties::Model, \"Virtual Video Device\");\n> > > - data->properties_.set(properties::PixelArrayActiveAreas, { Rectangle(Size(1920, 1080)) });\n> > > -\n> > > - /* \\todo Set FrameDurationLimits based on config. */\n> > > - ControlInfoMap::Map controls;\n> > > - int64_t min_frame_duration = 33333, max_frame_duration = 33333;\n> > > - controls[&controls::FrameDurationLimits] = ControlInfo(min_frame_duration, max_frame_duration);\n> > > - std::vector supportedFaceDetectModes{\n> > > - static_cast(controls::draft::FaceDetectModeOff),\n> > > - };\n> > > - controls[&controls::draft::FaceDetectMode] = ControlInfo(supportedFaceDetectModes);\n> > > - data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);\n> > > -\n> > > - /* Create and register the camera. */\n> > > - std::set streams;\n> > > - for (auto &streamConfig : data->streamConfigs_)\n> > > - streams.insert(&streamConfig.stream);\n> > > + File file(configurationFile(\"virtual\", \"virtual.yaml\"));\n> >\n> > This tells me that the sample virtual.yaml file should probably be\n> > installed in the libcamera data path...\n> \n> Yes, updated the README.md.\n> \n> \n> >\n> > This version doesn't currently apply cleanly to master, but I did have\n> > an branch with it on - and it rebased easily.\n> \n> Rebased on tot in the next version.\n\nThanks - let try and get this merged then :-)\n\n--\nKieran\n\n> \n> \n> >\n> > But it needs a new version posting at least to solve the SoB issues and\n> > so with the above tackled you can add\n> >\n> > Reviewed-by: Kieran Bingham \n> >\n> >\n> > > + bool isOpen = file.open(File::OpenModeFlag::ReadOnly);\n> > > + if (!isOpen) {\n> > > + LOG(Virtual, Error) << \"Failed to open config file: \" << file.fileName();\n> > > + return false;\n> > > + }\n> > >\n> > > - const std::string id = \"Virtual0\";\n> > > - std::shared_ptr camera = Camera::create(std::move(data), id, streams);\n> > > + ConfigParser parser;\n> > > + auto configData = parser.parseConfigFile(file, this);\n> > > + if (configData.size() == 0) {\n> > > + LOG(Virtual, Error) << \"Failed to parse any cameras from the config file: \"\n> > > + << file.fileName();\n> > > + return false;\n> > > + }\n> > >\n> > > - initFrameGenerator(camera.get());\n> > > + /* Configure and register cameras with configData */\n> > > + for (auto &data : configData) {\n> > > + std::set streams;\n> > > + for (auto &streamConfig : data->streamConfigs_)\n> > > + streams.insert(&streamConfig.stream);\n> > > + std::string id = data->config_.id;\n> > > + std::shared_ptr camera = Camera::create(std::move(data), id, streams);\n> > > +\n> > > + if (!initFrameGenerator(camera.get())) {\n> > > + LOG(Virtual, Error) << \"Failed to initialize frame \"\n> > > + << \"generator for camera: \" << id;\n> > > + continue;\n> > > + }\n> > >\n> > > - registerCamera(std::move(camera));\n> > > + registerCamera(std::move(camera));\n> > > + }\n> > >\n> > > resetCreated_ = true;\n> > >\n> > > return true;\n> > > }\n> > >\n> > > -void PipelineHandlerVirtual::initFrameGenerator(Camera *camera)\n> > > +bool PipelineHandlerVirtual::initFrameGenerator(Camera *camera)\n> > > {\n> > > auto data = cameraData(camera);\n> > > - for (auto &streamConfig : data->streamConfigs_) {\n> > > - if (data->testPattern_ == TestPattern::DiagonalLines)\n> > > - streamConfig.frameGenerator = std::make_unique();\n> > > - else\n> > > - streamConfig.frameGenerator = std::make_unique();\n> > > - }\n> > > + auto &frame = data->config_.frame;\n> > > + std::visit(overloaded{\n> > > + [&](TestPattern &testPattern) {\n> > > + for (auto &streamConfig : data->streamConfigs_) {\n> > > + if (testPattern == TestPattern::DiagonalLines)\n> > > + streamConfig.frameGenerator = std::make_unique();\n> > > + else\n> > > + streamConfig.frameGenerator = std::make_unique();\n> > > + }\n> > > + },\n> > > + [&](ImageFrames &imageFrames) {\n> > > + for (auto &streamConfig : data->streamConfigs_)\n> > > + streamConfig.frameGenerator = ImageFrameGenerator::create(imageFrames);\n> > > + } },\n> > > + frame);\n> > > +\n> > > + for (auto &streamConfig : data->streamConfigs_)\n> > > + if (!streamConfig.frameGenerator)\n> > > + return false;\n> > > +\n> > > + return true;\n> > > }\n> > >\n> > > REGISTER_PIPELINE_HANDLER(PipelineHandlerVirtual, \"virtual\")\n> > > diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h\n> > > index 0b15f8d9c..8c4d6ae64 100644\n> > > --- a/src/libcamera/pipeline/virtual/virtual.h\n> > > +++ b/src/libcamera/pipeline/virtual/virtual.h\n> > > @@ -7,6 +7,8 @@\n> > >\n> > > #pragma once\n> > >\n> > > +#include \n> > > +#include \n> > > #include \n> > >\n> > > #include \n> > > @@ -15,10 +17,14 @@\n> > > #include \"libcamera/internal/camera.h\"\n> > > #include \"libcamera/internal/pipeline_handler.h\"\n> > >\n> > > +#include \"frame_generator.h\"\n> > > +#include \"image_frame_generator.h\"\n> > > #include \"test_pattern_generator.h\"\n> > >\n> > > namespace libcamera {\n> > >\n> > > +using VirtualFrame = std::variant;\n> > > +\n> > > class VirtualCameraData : public Camera::Private\n> > > {\n> > > public:\n> > > @@ -26,23 +32,28 @@ public:\n> > >\n> > > struct Resolution {\n> > > Size size;\n> > > - std::vector frameRates;\n> > > + std::vector frameRates;\n> > > };\n> > > struct StreamConfig {\n> > > Stream stream;\n> > > std::unique_ptr frameGenerator;\n> > > };\n> > > + /* The config file is parsed to the Configuration struct */\n> > > + struct Configuration {\n> > > + std::string id;\n> > > + std::vector resolutions;\n> > > + VirtualFrame frame;\n> > > +\n> > > + Size maxResolutionSize;\n> > > + Size minResolutionSize;\n> > > + };\n> > >\n> > > VirtualCameraData(PipelineHandler *pipe,\n> > > const std::vector &supportedResolutions);\n> > >\n> > > ~VirtualCameraData() = default;\n> > >\n> > > - TestPattern testPattern_ = TestPattern::ColorBars;\n> > > -\n> > > - const std::vector supportedResolutions_;\n> > > - Size maxResolutionSize_;\n> > > - Size minResolutionSize_;\n> > > + Configuration config_;\n> > >\n> > > std::vector streamConfigs_;\n> > > };\n> > > --\n> > > 2.47.0.rc0.187.ge670bccf7e-goog\n> > >","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 9C37EC3274\n\tfor ;\n\tTue, 22 Oct 2024 07:58:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A3E4C65393;\n\tTue, 22 Oct 2024 09:58:06 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 73EC76053E\n\tfor ;\n\tTue, 22 Oct 2024 09:58:04 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BB241D49;\n\tTue, 22 Oct 2024 09:56:17 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"g40mjUI2\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1729583777;\n\tbh=ZnOReyOKmI7LIHr0QT+Cdyjn7xsayNu5LOD1/o/B72g=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=g40mjUI2CzpQ48P84JenwatU85xdrguyj6krae1zXeJ2yT23DCvErBEe5L2kbIcH0\n\tGoxIcLgjQ8nQqreavOBESZPkF4quD42lHlta2zmcziFFZKR5WN7Wwuo3N1hFNXVVS6\n\ttZWqCMXHMuJdFYzv25RSdIFtOaCfx4UGCxyfyRb4=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"","References":"<20241004095946.264537-1-chenghaoyang@google.com>\n\t<20241004095946.264537-7-chenghaoyang@google.com>\n\t<172955351674.1470935.9237357010380500470@ping.linuxembedded.co.uk>\n\t","Subject":"Re: [PATCH 15 6/7] libcamera: virtual: Read config and register\n\tcameras based on the config","From":"Kieran Bingham ","Cc":"libcamera-devel@lists.libcamera.org,\n\tHarvey Yang , \n\tKonami Shu , Yunke Cao ,\n\tTomasz Figa ","To":"Cheng-Hao Yang ","Date":"Tue, 22 Oct 2024 08:58:01 +0100","Message-ID":"<172958388179.2721096.16253835886526595075@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]