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

[Barnabás Pőcze]

Hi


2024. szeptember 26., csütörtök 18:29 keltezéssel, Harvey Yang <chenghaoyang at chromium.org> írta:

> This patch introduces the configuration file for Virtual Pipeline
> Handler. The config file is written in yaml, and the format is
> documented in `README.md`.
> 
> The config file will define the camera with IDs, supported formats and
> image sources, etc. In the default config file, only Test Patterns are
> used. Developers can use real images loading if desired.
> 
> Signed-off-by: Konami Shu <konamiz at google.com>
> Co-developed-by: Harvey Yang <chenghaoyang at chromium.org>
> Co-developed-by: Yunke Cao <yunkec at chromium.org>
> Co-developed-by: Tomasz Figa <tfiga at chromium.org>
> ---
> [...]
> +int Parser::parseFrameGenerator(const YamlObject &cameraConfigData, VirtualCameraData *data)
> +{
> +	const std::string testPatternKey = "test_pattern";
> +	const std::string framesKey = "frames";
> +	if (cameraConfigData.contains(testPatternKey)) {
> +		if (cameraConfigData.contains(framesKey)) {
> +			LOG(Virtual, Error) << "A camera should use either "
> +					    << testPatternKey << " or " << framesKey;
> +			return -EINVAL;
> +		}
> +
> +		auto testPattern = cameraConfigData[testPatternKey].get<std::string>("");
> +
> +		if (testPattern == "bars") {
> +			data->config_.frame = TestPattern::ColorBars;
> +		} else if (testPattern == "lines") {
> +			data->config_.frame = TestPattern::DiagonalLines;
> +		} else {
> +			LOG(Virtual, Debug) << "Test pattern: " << testPattern
> +					    << " is not supported";
> +			return -EINVAL;
> +		}
> +
> +		return 0;
> +	}
> +
> +	const YamlObject &frames = cameraConfigData[framesKey];
> +
> +	/* When there is no frames provided in the config file, use color bar test pattern */
> +	if (!frames) {
> +		data->config_.frame = TestPattern::ColorBars;
> +		return 0;
> +	}
> +
> +	if (!frames.isDictionary()) {
> +		LOG(Virtual, Error) << "'frames' is not a dictionary.";
> +		return -EINVAL;
> +	}
> +
> +	auto path = frames["path"].get<std::string>();
> +
> +	if (!path) {
> +		LOG(Virtual, Error) << "Test pattern or path should be specified.";
> +		return -EINVAL;
> +	} else if (auto ext = std::filesystem::path(*path).extension();
> +		   ext == ".jpg" || ext == ".jpeg") {
> +		data->config_.frame = ImageFrames{ *path, std::nullopt };
> +	} else if (std::filesystem::is_directory(std::filesystem::symlink_status(*path))) {
> +		using std::filesystem::directory_iterator;
> +		unsigned int numOfFiles = std::distance(directory_iterator(*path), directory_iterator{});
> +		if (numOfFiles == 0) {
> +			LOG(Virtual, Error) << "Empty directory";
> +			return -EINVAL;
> +		}
> +		data->config_.frame = ImageFrames{ *path, numOfFiles };
> +	} else {
> +		LOG(Virtual, Error) << "Frame: " << *path << " is not supported";
> +		return -EINVAL;
> +	}
> [...]

I wanted to reflect a bit on this part. I think the `ImageFrames` type can be made
more easily understandable as follows:

  struct ImageFrames {
    std::vector<std::filesystem::path> files;
  };

and then the above part would become something like this:

  if (!path) {
    LOG(Virtual, Error) << "Test pattern or path should be specified.";
    return -EINVAL;
  }

  std::vector<std::filesystem::path> files;

  switch (std::filesystem::symlink_status(*path).type()) {
  case std::filesystem::file_type::regular:
    files.push_back(*path);
    break;
  case std::filesystem::file_type::directory:
    for (const auto& dentry : std::filesystem::directory_iterator { *path })
      if (dentry.is_regular_file())
        files.push_back(dentry.path());
    std::sort(files.begin(), files.end(), [](const auto& a, const auto& b) {
      return ::strverscmp(a.c_str(), b.c_str()) < 0;
    });
    if (files.empty()) {
      LOG(Virtual, Error) << "Directory has no files: " << *path;
      return -EINVAL;
    }
    break;
  default:
    LOG(Virtual, Error) << "Frame: " << *path << " is not supported";
    return -EINVAL;
  }
  
  data->config_.frame = ImageFrames { std::move(files) };

So there would simply be a list of files. I think this is simple, and does not
need any special code for the "single file" scenario when using an `ImageFrames`
object.

Note, that the files in the dictionary are sorted using `strverscmp()`, so
frame-99 should be placed before frame-100, as one would expect.

Also notice that there is no extension checking anymore, as I don't believe
that to be too useful since it does not guarantee anything, and only jpeg files
are supported, so whether or not the file can be read will be found out when
the program tries to load them.

Any thoughts?


Regards,
Barnabás Pőcze