[PATCH v14 3/7] libcamera: virtual: Add VirtualPipelineHandler
Kieran Bingham
kieran.bingham at ideasonboard.com
Thu Oct 3 10:38:26 CEST 2024
Quoting Harvey Yang (2024-09-30 07:29:44)
> From: Harvey Yang <chenghaoyang at chromium.org>
>
> Add VirtualPipelineHandler for more unit tests and verfiy libcamera
> infrastructure works on devices without using hardware cameras.
>
> Signed-off-by: Harvey Yang <chenghaoyang at chromium.org>
> Reviewed-by: Jacopo Mondi <jacopo.mondi at ideasonboard.com>
> ---
> meson.build | 1 +
> meson_options.txt | 3 +-
> src/libcamera/pipeline/virtual/meson.build | 5 +
> src/libcamera/pipeline/virtual/virtual.cpp | 317 +++++++++++++++++++++
> src/libcamera/pipeline/virtual/virtual.h | 45 +++
> 5 files changed, 370 insertions(+), 1 deletion(-)
> create mode 100644 src/libcamera/pipeline/virtual/meson.build
> create mode 100644 src/libcamera/pipeline/virtual/virtual.cpp
> create mode 100644 src/libcamera/pipeline/virtual/virtual.h
>
> diff --git a/meson.build b/meson.build
> index 63e45465..5e533b0c 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -214,6 +214,7 @@ pipelines_support = {
> 'simple': ['any'],
> 'uvcvideo': ['any'],
> 'vimc': ['test'],
> + 'virtual': ['test'],
> }
>
> if pipelines.contains('all')
> diff --git a/meson_options.txt b/meson_options.txt
> index 7aa41249..c91cd241 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -53,7 +53,8 @@ option('pipelines',
> 'rpi/vc4',
> 'simple',
> 'uvcvideo',
> - 'vimc'
> + 'vimc',
> + 'virtual'
> ],
> description : 'Select which pipeline handlers to build. If this is set to "auto", all the pipelines applicable to the target architecture will be built. If this is set to "all", all the pipelines will be built. If both are selected then "all" will take precedence.')
>
> diff --git a/src/libcamera/pipeline/virtual/meson.build b/src/libcamera/pipeline/virtual/meson.build
> new file mode 100644
> index 00000000..ada1b335
> --- /dev/null
> +++ b/src/libcamera/pipeline/virtual/meson.build
> @@ -0,0 +1,5 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +libcamera_internal_sources += files([
> + 'virtual.cpp',
> +])
> diff --git a/src/libcamera/pipeline/virtual/virtual.cpp b/src/libcamera/pipeline/virtual/virtual.cpp
> new file mode 100644
> index 00000000..d1584f59
> --- /dev/null
> +++ b/src/libcamera/pipeline/virtual/virtual.cpp
> @@ -0,0 +1,317 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Google Inc.
> + *
> + * virtual.cpp - Pipeline handler for virtual cameras
> + */
> +
> +#include "virtual.h"
> +
> +#include <algorithm>
> +#include <array>
> +#include <chrono>
> +#include <errno.h>
> +#include <map>
> +#include <memory>
> +#include <ostream>
> +#include <set>
> +#include <stdint.h>
> +#include <string>
> +#include <time.h>
> +#include <utility>
> +#include <vector>
> +
> +#include <libcamera/base/flags.h>
> +#include <libcamera/base/log.h>
> +
> +#include <libcamera/control_ids.h>
> +#include <libcamera/controls.h>
> +#include <libcamera/formats.h>
> +#include <libcamera/pixel_format.h>
> +#include <libcamera/property_ids.h>
> +#include <libcamera/request.h>
> +
> +#include "libcamera/internal/camera.h"
> +#include "libcamera/internal/dma_buf_allocator.h"
> +#include "libcamera/internal/formats.h"
> +#include "libcamera/internal/pipeline_handler.h"
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(Virtual)
> +
> +namespace {
> +
> +uint64_t currentTimestamp()
> +{
> + const auto now = std::chrono::steady_clock::now();
> + auto nsecs = std::chrono::duration_cast<std::chrono::nanoseconds>(
> + now.time_since_epoch());
> +
> + return nsecs.count();
> +}
> +
> +} /* namespace */
> +
> +class VirtualCameraConfiguration : public CameraConfiguration
> +{
> +public:
> + static constexpr unsigned int kBufferCount = 4;
> +
> + VirtualCameraConfiguration(VirtualCameraData *data);
> +
> + Status validate() override;
> +
> +private:
> + const VirtualCameraData *data_;
> +};
> +
> +class PipelineHandlerVirtual : public PipelineHandler
> +{
> +public:
> + PipelineHandlerVirtual(CameraManager *manager);
> +
> + std::unique_ptr<CameraConfiguration> generateConfiguration(Camera *camera,
> + Span<const StreamRole> roles) override;
> + int configure(Camera *camera, CameraConfiguration *config) override;
> +
> + int exportFrameBuffers(Camera *camera, Stream *stream,
> + std::vector<std::unique_ptr<FrameBuffer>> *buffers) override;
> +
> + int start(Camera *camera, const ControlList *controls) override;
> + void stopDevice(Camera *camera) override;
> +
> + int queueRequestDevice(Camera *camera, Request *request) override;
> +
> + bool match(DeviceEnumerator *enumerator) override;
> +
> +private:
> + static bool created_;
Could this be
static bool created_ = false;
saving the need for the outer initialisation below?
> +
> + VirtualCameraData *cameraData(Camera *camera)
> + {
> + return static_cast<VirtualCameraData *>(camera->_d());
> + }
> +
> + DmaBufAllocator dmaBufAllocator_;
> +};
> +
> +/* static */
> +bool PipelineHandlerVirtual::created_ = false;
This stands out as a curious way to do this initialisation, but I
wouldn't block on this here ...
I'm curious why we 'need' a static created_ and how that will be used
though...
> +
> +VirtualCameraData::VirtualCameraData(PipelineHandler *pipe,
> + const std::vector<Resolution> &supportedResolutions)
> + : Camera::Private(pipe), supportedResolutions_(supportedResolutions)
> +{
> + for (const auto &resolution : supportedResolutions_) {
> + if (minResolutionSize_.isNull() || minResolutionSize_ > resolution.size)
> + minResolutionSize_ = resolution.size;
> +
> + maxResolutionSize_ = std::max(maxResolutionSize_, resolution.size);
> + }
> +
> + /* \todo Support multiple streams and pass multi_stream_test */
> + streamConfigs_.resize(kMaxStream);
> +}
> +
> +VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData *data)
> + : CameraConfiguration(), data_(data)
> +{
> +}
> +
> +CameraConfiguration::Status VirtualCameraConfiguration::validate()
> +{
> + Status status = Valid;
> +
> + if (config_.empty()) {
> + LOG(Virtual, Error) << "Empty config";
> + return Invalid;
> + }
> +
> + /* Only one stream is supported */
> + if (config_.size() > VirtualCameraData::kMaxStream) {
> + config_.resize(VirtualCameraData::kMaxStream);
> + status = Adjusted;
> + }
> +
> + for (StreamConfiguration &cfg : config_) {
> + bool found = false;
> + for (const auto &resolution : data_->supportedResolutions_) {
> + if (resolution.size.width == cfg.size.width &&
> + resolution.size.height == cfg.size.height) {
> + found = true;
> + break;
> + }
> + }
> +
> + if (!found) {
> + /*
> + * \todo It's a pipeline's decision to choose a
> + * resolution when the exact one is not supported.
> + * Defining the default logic in PipelineHandler to
> + * find the closest resolution would be nice.
> + */
> + cfg.size = data_->maxResolutionSize_;
> + status = Adjusted;
> + }
> +
> + if (cfg.pixelFormat != formats::NV12) {
> + cfg.pixelFormat = formats::NV12;
> + LOG(Virtual, Debug)
> + << "Stream configuration adjusted to " << cfg.toString();
Wouldn't it be worth reporting this if any adjustment is made?
I'd probably have put this at the end of this scope as
if (status == Adjusted)
LOG(Virtual, Info)
<< "Stream configuration adjusted to " << cfg.toString();
And I'd think 'info' would be suitable here over debug - as this is a
test / virtual pipeline handler so the additional log would be
beneficial IMO.
> + status = Adjusted;
> + }
> +
> + const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
> + cfg.stride = info.stride(cfg.size.width, 0, 1);
> + cfg.frameSize = info.frameSize(cfg.size, 1);
> +
> + cfg.bufferCount = VirtualCameraConfiguration::kBufferCount;
> + }
> +
> + return status;
> +}
> +
> +PipelineHandlerVirtual::PipelineHandlerVirtual(CameraManager *manager)
> + : PipelineHandler(manager),
> + dmaBufAllocator_(DmaBufAllocator::DmaBufAllocatorFlag::CmaHeap |
> + DmaBufAllocator::DmaBufAllocatorFlag::SystemHeap |
> + DmaBufAllocator::DmaBufAllocatorFlag::UDmaBuf)
> +{
> +}
> +
> +std::unique_ptr<CameraConfiguration>
> +PipelineHandlerVirtual::generateConfiguration(Camera *camera,
> + Span<const StreamRole> roles)
> +{
> + VirtualCameraData *data = cameraData(camera);
> + auto config = std::make_unique<VirtualCameraConfiguration>(data);
> +
> + if (roles.empty())
> + return config;
> +
> + for (const StreamRole role : roles) {
> + switch (role) {
> + case StreamRole::StillCapture:
> + case StreamRole::VideoRecording:
> + case StreamRole::Viewfinder:
> + break;
> +
> + case StreamRole::Raw:
> + default:
> + LOG(Virtual, Error)
> + << "Requested stream role not supported: " << role;
> + config.reset();
> + return config;
> + }
> +
> + std::map<PixelFormat, std::vector<SizeRange>> streamFormats;
> + PixelFormat pixelFormat = formats::NV12;
> + streamFormats[pixelFormat] = { { data->minResolutionSize_,
> + data->maxResolutionSize_ } };
> + StreamFormats formats(streamFormats);
> + StreamConfiguration cfg(formats);
> + cfg.pixelFormat = pixelFormat;
> + cfg.size = data->maxResolutionSize_;
> + cfg.bufferCount = VirtualCameraConfiguration::kBufferCount;
> +
> + config->addConfiguration(cfg);
> + }
> +
> + ASSERT(config->validate() != CameraConfiguration::Invalid);
> +
> + return config;
> +}
> +
> +int PipelineHandlerVirtual::configure(Camera *camera,
> + CameraConfiguration *config)
> +{
> + VirtualCameraData *data = cameraData(camera);
> + for (auto [i, c] : utils::enumerate(*config))
> + c.setStream(&data->streamConfigs_[i].stream);
> +
> + return 0;
> +}
> +
> +int PipelineHandlerVirtual::exportFrameBuffers([[maybe_unused]] Camera *camera,
> + Stream *stream,
> + std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> + if (!dmaBufAllocator_.isValid())
> + return -ENOBUFS;
> +
> + const StreamConfiguration &config = stream->configuration();
> +
> + auto info = PixelFormatInfo::info(config.pixelFormat);
> +
> + std::vector<unsigned int> planeSizes;
> + for (size_t i = 0; i < info.planes.size(); ++i)
> + planeSizes.push_back(info.planeSize(config.size, i));
> +
> + return dmaBufAllocator_.exportBuffers(config.bufferCount, planeSizes, buffers);
> +}
> +
> +int PipelineHandlerVirtual::start([[maybe_unused]] Camera *camera,
> + [[maybe_unused]] const ControlList *controls)
> +{
> + return 0;
> +}
> +
> +void PipelineHandlerVirtual::stopDevice([[maybe_unused]] Camera *camera)
> +{
> +}
> +
> +int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera,
> + Request *request)
> +{
> + for (auto it : request->buffers())
> + completeBuffer(request, it.second);
> +
> + request->metadata().set(controls::SensorTimestamp, currentTimestamp());
> + completeRequest(request);
> +
> + return 0;
> +}
> +
> +bool PipelineHandlerVirtual::match([[maybe_unused]] DeviceEnumerator *enumerator)
> +{
> + if (created_)
> + return false;
> +
> + created_ = true;
> +
> + /* \todo Add virtual cameras according to a config file. */
> +
> + std::vector<VirtualCameraData::Resolution> supportedResolutions;
> + supportedResolutions.resize(2);
> + supportedResolutions[0] = { .size = Size(1920, 1080), .frameRates = { 30 } };
> + supportedResolutions[1] = { .size = Size(1280, 720), .frameRates = { 30 } };
> +
> + std::unique_ptr<VirtualCameraData> data =
> + std::make_unique<VirtualCameraData>(this, supportedResolutions);
> +
> + data->properties_.set(properties::Location, properties::CameraLocationFront);
> + data->properties_.set(properties::Model, "Virtual Video Device");
> + data->properties_.set(properties::PixelArrayActiveAreas, { Rectangle(Size(1920, 1080)) });
> +
> + /* \todo Set FrameDurationLimits based on config. */
> + ControlInfoMap::Map controls;
> + int64_t min_frame_duration = 33333, max_frame_duration = 33333;
> + controls[&controls::FrameDurationLimits] = ControlInfo(min_frame_duration, max_frame_duration);
> + data->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);
> +
> + /* Create and register the camera. */
> + std::set<Stream *> streams;
> + for (auto &streamConfig : data->streamConfigs_)
> + streams.insert(&streamConfig.stream);
> +
> + const std::string id = "Virtual0";
> + std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams);
> + registerCamera(std::move(camera));
> +
> + return true;
I think if you return false here, you don't need any of the created_;
/*
* Ensure we only ever register and create a single virtual
* pipeline handler.
*/
return false;
This is core libcamera not your patch, - I still dislike this return
usage of match(). It's either not clear or not fitting well to the needs
of most pipeline handlers.
It's should be clearer at core that returning true here simply means
'please try to create more pipeline handlers'
Anyway, comments above could be applied on top if you wish - or handled
how you prefer.
The created_ just seems like an awkward workaround, but it's a
workaround for trying to return the right thing at the end of match. But
I think the 'right thing' to return there is poorly defined maybe...
Anyway, with any of the above handled or not - or handled on top - none
of that blocks this patch for me.
Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>
> +}
> +
> +REGISTER_PIPELINE_HANDLER(PipelineHandlerVirtual, "virtual")
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/virtual/virtual.h b/src/libcamera/pipeline/virtual/virtual.h
> new file mode 100644
> index 00000000..4df70a13
> --- /dev/null
> +++ b/src/libcamera/pipeline/virtual/virtual.h
> @@ -0,0 +1,45 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2024, Google Inc.
> + *
> + * virtual.h - Pipeline handler for virtual cameras
> + */
> +
> +#pragma once
> +
> +#include <vector>
> +
> +#include <libcamera/geometry.h>
> +#include <libcamera/stream.h>
> +
> +#include "libcamera/internal/camera.h"
> +#include "libcamera/internal/pipeline_handler.h"
> +
> +namespace libcamera {
> +
> +class VirtualCameraData : public Camera::Private
> +{
> +public:
> + const static unsigned int kMaxStream = 1;
> +
> + struct Resolution {
> + Size size;
> + std::vector<int> frameRates;
> + };
> + struct StreamConfig {
> + Stream stream;
> + };
> +
> + VirtualCameraData(PipelineHandler *pipe,
> + const std::vector<Resolution> &supportedResolutions);
> +
> + ~VirtualCameraData() = default;
> +
> + const std::vector<Resolution> supportedResolutions_;
> + Size maxResolutionSize_;
> + Size minResolutionSize_;
> +
> + std::vector<StreamConfig> streamConfigs_;
> +};
> +
> +} /* namespace libcamera */
> --
> 2.46.1.824.gd892dcdcdd-goog
>
More information about the libcamera-devel
mailing list