| Message ID | 20240820172202.526547-4-chenghaoyang@google.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Harvey On Tue, Aug 20, 2024 at 04:23:34PM GMT, Harvey Yang wrote: > From: Harvey Yang <chenghaoyang@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@chromium.org> > --- > meson.build | 1 + > meson_options.txt | 3 +- > src/libcamera/pipeline/virtual/meson.build | 5 + > src/libcamera/pipeline/virtual/virtual.cpp | 251 +++++++++++++++++++++ > src/libcamera/pipeline/virtual/virtual.h | 78 +++++++ Do you expect other components to include this header in future ? If not, you can move its content to the .cpp file I guess > 5 files changed, 337 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 f946eba94..3cad3249a 100644 > --- a/meson.build > +++ b/meson.build > @@ -222,6 +222,7 @@ pipelines_support = { > 'simple': arch_arm, > 'uvcvideo': ['any'], > 'vimc': ['test'], > + 'virtual': ['test'], > } > > if pipelines.contains('all') > diff --git a/meson_options.txt b/meson_options.txt > index 7aa412491..c91cd241a 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 000000000..ba7ff754e > --- /dev/null > +++ b/src/libcamera/pipeline/virtual/meson.build > @@ -0,0 +1,5 @@ > +# SPDX-License-Identifier: CC0-1.0 > + > +libcamera_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 000000000..74eb8c7ad > --- /dev/null > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > @@ -0,0 +1,251 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2023, Google Inc. > + * > + * virtual.cpp - Pipeline handler for virtual cameras > + */ > + You should include the header for the standard library construct you use. I see vectors, maps, unique_ptrs etc > +#include "virtual.h" > + > +#include <libcamera/base/log.h> > + > +#include <libcamera/camera.h> > +#include <libcamera/control_ids.h> > +#include <libcamera/controls.h> > +#include <libcamera/formats.h> > +#include <libcamera/property_ids.h> > + > +#include "libcamera/internal/camera.h" The internal header includes the public one by definition > +#include "libcamera/internal/formats.h" This doesn't as <libcamera/formats.h> is generated. I wonder if it should. > +#include "libcamera/internal/pipeline_handler.h" > + > +namespace libcamera { > + > +LOG_DEFINE_CATEGORY(Virtual) > + > +namespace { > + > +uint64_t currentTimestamp() > +{ > + struct timespec ts; > + if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) { > + LOG(Virtual, Error) << "Get clock time fails"; > + return 0; > + } > + > + return ts.tv_sec * 1'000'000'000LL + ts.tv_nsec; > +} Could https://en.cppreference.com/w/cpp/chrono/steady_clock/now save you a custom function ? In example: const auto now = std::chrono::steady_clock::now(); auto nsecs = std::chrono::duration_cast<std::chrono::nanoseconds>(now.time_since_epoch()); std::cout << nsecs.count(); should give you the time in nanoseconds since system boot (if I got it right) > + > +} // namespace nit: /* namespace */ here and in other places > + > +VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData *data) > + : CameraConfiguration(), data_(data) > +{ > +} > + > +CameraConfiguration::Status VirtualCameraConfiguration::validate() > +{ > + Status status = Valid; > + > + if (config_.empty()) { > + LOG(Virtual, Error) << "Empty config"; > + return Invalid; > + } > + > + /* Currently only one stream is supported */ > + if (config_.size() > 1) { > + config_.resize(1); > + status = Adjusted; > + } > + > + Size maxSize; > + for (const auto &resolution : data_->supportedResolutions_) > + maxSize = std::max(maxSize, resolution.size); > + > + for (StreamConfiguration &cfg : config_) { you only have config, or in the next patches will this be augmented ? > + 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) { > + cfg.size = maxSize; > + status = Adjusted; > + } so it's either the exact resolution or the biggest available one ? As you have a single config it's easy to get the closest one to the requested size, if it doesn't match exactly one of the supported resolutions. > + > + const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat); > + cfg.stride = info.stride(cfg.size.width, 0, 1); > + cfg.frameSize = info.frameSize(cfg.size, 1); > + > + cfg.setStream(const_cast<Stream *>(&data_->stream_)); > + > + cfg.bufferCount = VirtualCameraConfiguration::kBufferCount; > + } > + > + return status; > +} > + > +PipelineHandlerVirtual::PipelineHandlerVirtual(CameraManager *manager) > + : PipelineHandler(manager) > +{ > +} > + > +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; > + > + Size minSize, sensorResolution; > + for (const auto &resolution : data->supportedResolutions_) { > + if (minSize.isNull() || minSize > resolution.size) > + minSize = resolution.size; > + > + sensorResolution = std::max(sensorResolution, resolution.size); As you do this min/max search in a few places, why not doing it when you construct data->supportedResolutions_ once ? > + } > + > + for (const StreamRole role : roles) { If the pipeline handler works with a single stream, you should only consider the first role maybe > + std::map<PixelFormat, std::vector<SizeRange>> streamFormats; > + unsigned int bufferCount; > + PixelFormat pixelFormat; > + > + switch (role) { > + case StreamRole::StillCapture: > + pixelFormat = formats::NV12; > + bufferCount = VirtualCameraConfiguration::kBufferCount; > + streamFormats[pixelFormat] = { { minSize, sensorResolution } }; bufferCount and streamFormats can be assigned outsize of the cases, they're the same for all roles > + > + break; > + > + case StreamRole::Raw: { > + /* \todo check */ > + pixelFormat = formats::SBGGR10; > + bufferCount = VirtualCameraConfiguration::kBufferCount; > + streamFormats[pixelFormat] = { { minSize, sensorResolution } }; > + > + break; > + } > + > + case StreamRole::Viewfinder: > + case StreamRole::VideoRecording: { > + pixelFormat = formats::NV12; > + bufferCount = VirtualCameraConfiguration::kBufferCount; > + streamFormats[pixelFormat] = { { minSize, sensorResolution } }; > + > + break; > + } > + > + default: > + LOG(Virtual, Error) > + << "Requested stream role not supported: " << role; > + config.reset(); > + return config; > + } > + > + StreamFormats formats(streamFormats); > + StreamConfiguration cfg(formats); > + cfg.size = sensorResolution; > + cfg.pixelFormat = pixelFormat; > + cfg.bufferCount = bufferCount; > + config->addConfiguration(cfg); > + } > + > + if (config->validate() == CameraConfiguration::Invalid) > + config.reset(); > + > + return config; > +} > + > +int PipelineHandlerVirtual::configure( > + [[maybe_unused]] Camera *camera, > + [[maybe_unused]] CameraConfiguration *config) int PipelineHandlerVirtual::configure([[maybe_unused]] Camera *camera, [[maybe_unused]] CameraConfiguration *config) > +{ > + // Nothing to be done. > + return 0; > +} > + > +int PipelineHandlerVirtual::exportFrameBuffers( > + [[maybe_unused]] Camera *camera, > + Stream *stream, > + std::vector<std::unique_ptr<FrameBuffer>> *buffers) int PipelineHandlerVirtual::exportFrameBuffers([[maybe_unused]] Camera *camera, Stream *stream, std::vector<std::unique_ptr<FrameBuffer>> *buffers) if you prefer > +{ > + if (!dmaBufAllocator_.isValid()) > + return -ENOBUFS; > + > + const StreamConfiguration &config = stream->configuration(); > + > + auto info = PixelFormatInfo::info(config.pixelFormat); > + > + std::vector<std::size_t> 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); ah that's probably why you return count from DmaBufferAllocator::exportBuffers() > +} > + > +int PipelineHandlerVirtual::start([[maybe_unused]] Camera *camera, > + [[maybe_unused]] const ControlList *controls) > +{ > + /* \todo Start reading the virtual video if any. */ > + return 0; > +} > + > +void PipelineHandlerVirtual::stopDevice([[maybe_unused]] Camera *camera) > +{ > + /* \todo Reset the virtual video if any. */ > +} > + > +int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, > + Request *request) > +{ > + /* \todo Read from the virtual video if any. */ > + 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) > +{ > + /* \todo Add virtual cameras according to a config file. */ > + > + std::unique_ptr<VirtualCameraData> data = std::make_unique<VirtualCameraData>(this); > + > + data->supportedResolutions_.resize(2); > + data->supportedResolutions_[0] = { .size = Size(1920, 1080), .frame_rates = { 30 } }; > + data->supportedResolutions_[1] = { .size = Size(1280, 720), .frame_rates = { 30, 60 } }; > + > + 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 = 30, max_frame_duration = 60; doesn't match the above frame rates and it should be expressed in microseconds. I would suggest for this patch to set both framerates to 30 and initialize FrameDurationLimits with {33333, 333333} It's not a big deal however, it will be replaced later in the series > + 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{ &data->stream_ }; > + const std::string id = "Virtual0"; > + std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams); > + registerCamera(std::move(camera)); > + > + return false; // Prevent infinite loops for now I presume this will also be changed in next patches... > +} > + > +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 000000000..6fc6b34d8 > --- /dev/null > +++ b/src/libcamera/pipeline/virtual/virtual.h > @@ -0,0 +1,78 @@ > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > +/* > + * Copyright (C) 2023, Google Inc. > + * > + * virtual.h - Pipeline handler for virtual cameras > + */ > + > +#pragma once > + > +#include <libcamera/base/file.h> > + > +#include "libcamera/internal/camera.h" > +#include "libcamera/internal/dma_buf_allocator.h" > +#include "libcamera/internal/pipeline_handler.h" > + > +namespace libcamera { > + > +class VirtualCameraData : public Camera::Private > +{ > +public: > + struct Resolution { > + Size size; > + std::vector<int> frame_rates; > + }; > + VirtualCameraData(PipelineHandler *pipe) > + : Camera::Private(pipe) > + { > + } > + > + ~VirtualCameraData() = default; > + > + std::vector<Resolution> supportedResolutions_; > + > + Stream stream_; > +}; > + > +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: > + VirtualCameraData *cameraData(Camera *camera) > + { > + return static_cast<VirtualCameraData *>(camera->_d()); > + } > + > + DmaBufAllocator dmaBufAllocator_; > +}; > + > +} // namespace libcamera > -- > 2.46.0.184.g6999bdac58-goog >
Thanks for the review, Jacopo! On Tue, Aug 27, 2024 at 5:43 PM Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > Hi Harvey > > On Tue, Aug 20, 2024 at 04:23:34PM GMT, Harvey Yang wrote: > > From: Harvey Yang <chenghaoyang@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@chromium.org> > > --- > > meson.build | 1 + > > meson_options.txt | 3 +- > > src/libcamera/pipeline/virtual/meson.build | 5 + > > src/libcamera/pipeline/virtual/virtual.cpp | 251 +++++++++++++++++++++ > > src/libcamera/pipeline/virtual/virtual.h | 78 +++++++ > > Do you expect other components to include this header in future ? If > not, you can move its content to the .cpp file I guess > > Actually `virtual/parser.h` needs to include it to return VirtualCameraData when parsing the yaml config file [1]. Does that count :p? [1]: https://patchwork.libcamera.org/patch/20971/ > 5 files changed, 337 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 f946eba94..3cad3249a 100644 > > --- a/meson.build > > +++ b/meson.build > > @@ -222,6 +222,7 @@ pipelines_support = { > > 'simple': arch_arm, > > 'uvcvideo': ['any'], > > 'vimc': ['test'], > > + 'virtual': ['test'], > > } > > > > if pipelines.contains('all') > > diff --git a/meson_options.txt b/meson_options.txt > > index 7aa412491..c91cd241a 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 000000000..ba7ff754e > > --- /dev/null > > +++ b/src/libcamera/pipeline/virtual/meson.build > > @@ -0,0 +1,5 @@ > > +# SPDX-License-Identifier: CC0-1.0 > > + > > +libcamera_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 000000000..74eb8c7ad > > --- /dev/null > > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > > @@ -0,0 +1,251 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2023, Google Inc. > > + * > > + * virtual.cpp - Pipeline handler for virtual cameras > > + */ > > + > > You should include the header for the standard library construct you > use. I see vectors, maps, unique_ptrs etc > Done, please check again. > > > +#include "virtual.h" > > + > > +#include <libcamera/base/log.h> > > + > > +#include <libcamera/camera.h> > > +#include <libcamera/control_ids.h> > > +#include <libcamera/controls.h> > > +#include <libcamera/formats.h> > > +#include <libcamera/property_ids.h> > > + > > +#include "libcamera/internal/camera.h" > > The internal header includes the public one by definition > Ack. Removed the public one. > > > +#include "libcamera/internal/formats.h" > > This doesn't as <libcamera/formats.h> is generated. I wonder if it > should. > Keeping `#include <libcamera/formats.h>`. > > > +#include "libcamera/internal/pipeline_handler.h" > > + > > +namespace libcamera { > > + > > +LOG_DEFINE_CATEGORY(Virtual) > > + > > +namespace { > > + > > +uint64_t currentTimestamp() > > +{ > > + struct timespec ts; > > + if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) { > > + LOG(Virtual, Error) << "Get clock time fails"; > > + return 0; > > + } > > + > > + return ts.tv_sec * 1'000'000'000LL + ts.tv_nsec; > > +} > > Could https://en.cppreference.com/w/cpp/chrono/steady_clock/now save > you a custom function ? > > In example: > > const auto now = std::chrono::steady_clock::now(); > auto nsecs = > std::chrono::duration_cast<std::chrono::nanoseconds>(now.time_since_epoch()); > std::cout << nsecs.count(); > > should give you the time in nanoseconds since system boot (if I got it > right) > > > > + > > +} // namespace > > nit: /* namespace */ > here and in other places > > Done > > + > > > +VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData > *data) > > + : CameraConfiguration(), data_(data) > > +{ > > +} > > + > > +CameraConfiguration::Status VirtualCameraConfiguration::validate() > > +{ > > + Status status = Valid; > > + > > + if (config_.empty()) { > > + LOG(Virtual, Error) << "Empty config"; > > + return Invalid; > > + } > > + > > + /* Currently only one stream is supported */ > > + if (config_.size() > 1) { > > + config_.resize(1); > > + status = Adjusted; > > + } > > + > > + Size maxSize; > > + for (const auto &resolution : data_->supportedResolutions_) > > + maxSize = std::max(maxSize, resolution.size); > > + > > + for (StreamConfiguration &cfg : config_) { > > you only have config, or in the next patches will this be augmented ? > > Do you mean that I should check `sensorConfig` or `orientation` as well? > + 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) { > > + cfg.size = maxSize; > > + status = Adjusted; > > + } > > so it's either the exact resolution or the biggest available one ? > > As you have a single config it's easy to get the closest one to the > requested size, if it doesn't match exactly one of the supported > resolutions. > Hmm, I think it's a bit hard to define the "closest". Do we compare the area size directly? Do we prefer a size that has both larger or the same height and width? > > > + > > + const PixelFormatInfo &info = > PixelFormatInfo::info(cfg.pixelFormat); > > + cfg.stride = info.stride(cfg.size.width, 0, 1); > > + cfg.frameSize = info.frameSize(cfg.size, 1); > > + > > + cfg.setStream(const_cast<Stream *>(&data_->stream_)); > > + > > + cfg.bufferCount = VirtualCameraConfiguration::kBufferCount; > > + } > > + > > + return status; > > +} > > + > > +PipelineHandlerVirtual::PipelineHandlerVirtual(CameraManager *manager) > > + : PipelineHandler(manager) > > +{ > > +} > > + > > +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; > > + > > + Size minSize, sensorResolution; > > + for (const auto &resolution : data->supportedResolutions_) { > > + if (minSize.isNull() || minSize > resolution.size) > > + minSize = resolution.size; > > + > > + sensorResolution = std::max(sensorResolution, > resolution.size); > > As you do this min/max search in a few places, why not doing it when > you construct data->supportedResolutions_ once ? > Added in VirtualCameraData. > > > + } > > + > > + for (const StreamRole role : roles) { > > If the pipeline handler works with a single stream, you should only > consider the first role maybe > Actually I think there's no reason to only support one Stream in Virtual Pipeline Handler. The raw stream doesn't seem to be properly supported in the following patches though. I think we should drop the support of raw. > > > + std::map<PixelFormat, std::vector<SizeRange>> > streamFormats; > > + unsigned int bufferCount; > > + PixelFormat pixelFormat; > > + > > + switch (role) { > > + case StreamRole::StillCapture: > > + pixelFormat = formats::NV12; > > + bufferCount = > VirtualCameraConfiguration::kBufferCount; > > + streamFormats[pixelFormat] = { { minSize, > sensorResolution } }; > > bufferCount and streamFormats can be assigned outsize of the cases, > they're the same for all roles > Done > > > + > > + break; > > + > > + case StreamRole::Raw: { > > + /* \todo check */ > > + pixelFormat = formats::SBGGR10; > > + bufferCount = > VirtualCameraConfiguration::kBufferCount; > > + streamFormats[pixelFormat] = { { minSize, > sensorResolution } }; > > + > > + break; > > + } > > + > > + case StreamRole::Viewfinder: > > + case StreamRole::VideoRecording: { > > + pixelFormat = formats::NV12; > > + bufferCount = > VirtualCameraConfiguration::kBufferCount; > > + streamFormats[pixelFormat] = { { minSize, > sensorResolution } }; > > + > > + break; > > + } > > + > > + default: > > + LOG(Virtual, Error) > > + << "Requested stream role not supported: " > << role; > > + config.reset(); > > + return config; > > + } > > + > > + StreamFormats formats(streamFormats); > > + StreamConfiguration cfg(formats); > > + cfg.size = sensorResolution; > > + cfg.pixelFormat = pixelFormat; > > + cfg.bufferCount = bufferCount; > > + config->addConfiguration(cfg); > > + } > > + > > + if (config->validate() == CameraConfiguration::Invalid) > > + config.reset(); > > + > > + return config; > > +} > > + > > +int PipelineHandlerVirtual::configure( > > + [[maybe_unused]] Camera *camera, > > + [[maybe_unused]] CameraConfiguration *config) > > int PipelineHandlerVirtual::configure([[maybe_unused]] Camera *camera, > [[maybe_unused]] CameraConfiguration > *config) > > Done > > +{ > > + // Nothing to be done. > > + return 0; > > +} > > + > > +int PipelineHandlerVirtual::exportFrameBuffers( > > + [[maybe_unused]] Camera *camera, > > + Stream *stream, > > + std::vector<std::unique_ptr<FrameBuffer>> *buffers) > > int PipelineHandlerVirtual::exportFrameBuffers([[maybe_unused]] Camera > *camera, > Stream *stream, > > std::vector<std::unique_ptr<FrameBuffer>> *buffers) > > if you prefer > > Done > > +{ > > + if (!dmaBufAllocator_.isValid()) > > + return -ENOBUFS; > > + > > + const StreamConfiguration &config = stream->configuration(); > > + > > + auto info = PixelFormatInfo::info(config.pixelFormat); > > + > > + std::vector<std::size_t> 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); > > ah that's probably why you return count from > DmaBufferAllocator::exportBuffers() > Exactly :) > > > +} > > + > > +int PipelineHandlerVirtual::start([[maybe_unused]] Camera *camera, > > + [[maybe_unused]] const ControlList > *controls) > > +{ > > + /* \todo Start reading the virtual video if any. */ > > + return 0; > > +} > > + > > +void PipelineHandlerVirtual::stopDevice([[maybe_unused]] Camera *camera) > > +{ > > + /* \todo Reset the virtual video if any. */ > > +} > > + > > +int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera > *camera, > > + Request *request) > > +{ > > + /* \todo Read from the virtual video if any. */ > > + 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) > > +{ > > + /* \todo Add virtual cameras according to a config file. */ > > + > > + std::unique_ptr<VirtualCameraData> data = > std::make_unique<VirtualCameraData>(this); > > + > > + data->supportedResolutions_.resize(2); > > + data->supportedResolutions_[0] = { .size = Size(1920, 1080), > .frame_rates = { 30 } }; > > + data->supportedResolutions_[1] = { .size = Size(1280, 720), > .frame_rates = { 30, 60 } }; > > + > > + 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 = 30, max_frame_duration = 60; > > doesn't match the above frame rates and it should be expressed in > microseconds. I would suggest for this patch to set both framerates to > 30 and initialize FrameDurationLimits with {33333, 333333} > > It's not a big deal however, it will be replaced later in the series > > Done, thanks! > > > + 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{ &data->stream_ }; > > + const std::string id = "Virtual0"; > > + std::shared_ptr<Camera> camera = Camera::create(std::move(data), > id, streams); > > + registerCamera(std::move(camera)); > > + > > + return false; // Prevent infinite loops for now > > I presume this will also be changed in next patches... > Updated in this patch, with a static boolean though. > > > +} > > + > > +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 000000000..6fc6b34d8 > > --- /dev/null > > +++ b/src/libcamera/pipeline/virtual/virtual.h > > @@ -0,0 +1,78 @@ > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > +/* > > + * Copyright (C) 2023, Google Inc. > > + * > > + * virtual.h - Pipeline handler for virtual cameras > > + */ > > + > > +#pragma once > > + > > +#include <libcamera/base/file.h> > > + > > +#include "libcamera/internal/camera.h" > > +#include "libcamera/internal/dma_buf_allocator.h" > > +#include "libcamera/internal/pipeline_handler.h" > > + > > +namespace libcamera { > > + > > +class VirtualCameraData : public Camera::Private > > +{ > > +public: > > + struct Resolution { > > + Size size; > > + std::vector<int> frame_rates; > > + }; > > + VirtualCameraData(PipelineHandler *pipe) > > + : Camera::Private(pipe) > > + { > > + } > > + > > + ~VirtualCameraData() = default; > > + > > + std::vector<Resolution> supportedResolutions_; > > + > > + Stream stream_; > > +}; > > + > > +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: > > + VirtualCameraData *cameraData(Camera *camera) > > + { > > + return static_cast<VirtualCameraData *>(camera->_d()); > > + } > > + > > + DmaBufAllocator dmaBufAllocator_; > > +}; > > + > > +} // namespace libcamera > > -- > > 2.46.0.184.g6999bdac58-goog > > >
Hi Jacopo, On Sat, Aug 31, 2024 at 8:49 PM Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > Hi Harvey > > On Sat, Aug 31, 2024 at 02:44:40PM GMT, Jacopo Mondi wrote: > > Hi > > > > On Thu, Aug 29, 2024 at 09:57:58PM GMT, Cheng-Hao Yang wrote: > > > Thanks for the review, Jacopo! > > > > > > On Tue, Aug 27, 2024 at 5:43 PM Jacopo Mondi < > jacopo.mondi@ideasonboard.com> > > > wrote: > > > > > > > Hi Harvey > > > > > > > > On Tue, Aug 20, 2024 at 04:23:34PM GMT, Harvey Yang wrote: > > > > > From: Harvey Yang <chenghaoyang@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@chromium.org> > > > > > --- > > > > > meson.build | 1 + > > > > > meson_options.txt | 3 +- > > > > > src/libcamera/pipeline/virtual/meson.build | 5 + > > > > > src/libcamera/pipeline/virtual/virtual.cpp | 251 > +++++++++++++++++++++ > > > > > src/libcamera/pipeline/virtual/virtual.h | 78 +++++++ > > > > > > > > Do you expect other components to include this header in future ? If > > > > not, you can move its content to the .cpp file I guess > > > > > > > > > > > Actually `virtual/parser.h` needs to include it to return > VirtualCameraData > > > when parsing the yaml config file [1]. Does that count :p? > > > > I guess it does :) > > > > > > > > [1]: https://patchwork.libcamera.org/patch/20971/ > > > > > > > 5 files changed, 337 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 f946eba94..3cad3249a 100644 > > > > > --- a/meson.build > > > > > +++ b/meson.build > > > > > @@ -222,6 +222,7 @@ pipelines_support = { > > > > > 'simple': arch_arm, > > > > > 'uvcvideo': ['any'], > > > > > 'vimc': ['test'], > > > > > + 'virtual': ['test'], > > > > > } > > > > > > > > > > if pipelines.contains('all') > > > > > diff --git a/meson_options.txt b/meson_options.txt > > > > > index 7aa412491..c91cd241a 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 000000000..ba7ff754e > > > > > --- /dev/null > > > > > +++ b/src/libcamera/pipeline/virtual/meson.build > > > > > @@ -0,0 +1,5 @@ > > > > > +# SPDX-License-Identifier: CC0-1.0 > > > > > + > > > > > +libcamera_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 000000000..74eb8c7ad > > > > > --- /dev/null > > > > > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > > > > > @@ -0,0 +1,251 @@ > > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > > +/* > > > > > + * Copyright (C) 2023, Google Inc. > > > > > + * > > > > > + * virtual.cpp - Pipeline handler for virtual cameras > > > > > + */ > > > > > + > > > > > > > > You should include the header for the standard library construct you > > > > use. I see vectors, maps, unique_ptrs etc > > > > > > > Done, please check again. > > > > > > > > > > > > > > > +#include "virtual.h" > > > > > + > > > > > +#include <libcamera/base/log.h> > > > > > + > > > > > +#include <libcamera/camera.h> > > > > > +#include <libcamera/control_ids.h> > > > > > +#include <libcamera/controls.h> > > > > > +#include <libcamera/formats.h> > > > > > +#include <libcamera/property_ids.h> > > > > > + > > > > > +#include "libcamera/internal/camera.h" > > > > > > > > The internal header includes the public one by definition > > > > > > > Ack. Removed the public one. > > > > > > > > > > > > > > > +#include "libcamera/internal/formats.h" > > > > > > > > This doesn't as <libcamera/formats.h> is generated. I wonder if it > > > > should. > > > > > > > Keeping `#include <libcamera/formats.h>`. > > > > > > > > > > > > > > > +#include "libcamera/internal/pipeline_handler.h" > > > > > + > > > > > +namespace libcamera { > > > > > + > > > > > +LOG_DEFINE_CATEGORY(Virtual) > > > > > + > > > > > +namespace { > > > > > + > > > > > +uint64_t currentTimestamp() > > > > > +{ > > > > > + struct timespec ts; > > > > > + if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) { > > > > > + LOG(Virtual, Error) << "Get clock time fails"; > > > > > + return 0; > > > > > + } > > > > > + > > > > > + return ts.tv_sec * 1'000'000'000LL + ts.tv_nsec; > > > > > +} > > > > > > > > Could https://en.cppreference.com/w/cpp/chrono/steady_clock/now save > > > > you a custom function ? > > > > > > > > In example: > > > > > > > > const auto now = std::chrono::steady_clock::now(); > > > > auto nsecs = > > > > > std::chrono::duration_cast<std::chrono::nanoseconds>(now.time_since_epoch()); > > > > std::cout << nsecs.count(); > > > > > > > > should give you the time in nanoseconds since system boot (if I got > it > > > > right) > > > > > > > > > > > > > + > > > > > +} // namespace > > > > > > > > nit: /* namespace */ > > > > here and in other places > > > > > > > > Done > > > > > > > > > > > + > > > > > > > > > > +VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData > > > > *data) > > > > > + : CameraConfiguration(), data_(data) > > > > > +{ > > > > > +} > > > > > + > > > > > +CameraConfiguration::Status VirtualCameraConfiguration::validate() > > > > > +{ > > > > > + Status status = Valid; > > > > > + > > > > > + if (config_.empty()) { > > > > > + LOG(Virtual, Error) << "Empty config"; > > > > > + return Invalid; > > > > > + } > > > > > + > > > > > + /* Currently only one stream is supported */ > > > > > + if (config_.size() > 1) { > > > > > + config_.resize(1); > > > > > + status = Adjusted; > > > > > + } > > > > > + > > > > > + Size maxSize; > > > > > + for (const auto &resolution : data_->supportedResolutions_) > > > > > + maxSize = std::max(maxSize, resolution.size); > > > > > + > > > > > + for (StreamConfiguration &cfg : config_) { > > > > > > > > you only have config, or in the next patches will this be augmented ? > > > > > > > > Do you mean that I should check `sensorConfig` or `orientation` as > well? > > > > > > > No I meant I was assuming the virtual pipline works with a single > > stream by design. But seeing your replies to the next patches makes me > > think my assumption was wrong. > > > > Let me clarify this once and for all: you currently have a single > stream in VirtualCameraData and your pipeline works with a single > stream, right ? > That's correct. > > Do you plane to change this and the pipeline handler code is > structured in this way to prepare for that ? > Yes, as I mentioned in the next patch version, I failed to pass multi-stream-test when enabling multiple streams. I need your help :)
Hi Jacopo, On Sat, Aug 31, 2024 at 8:44 PM Jacopo Mondi <jacopo.mondi@ideasonboard.com> wrote: > Hi > > On Thu, Aug 29, 2024 at 09:57:58PM GMT, Cheng-Hao Yang wrote: > > Thanks for the review, Jacopo! > > > > On Tue, Aug 27, 2024 at 5:43 PM Jacopo Mondi < > jacopo.mondi@ideasonboard.com> > > wrote: > > > > > Hi Harvey > > > > > > On Tue, Aug 20, 2024 at 04:23:34PM GMT, Harvey Yang wrote: > > > > From: Harvey Yang <chenghaoyang@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@chromium.org> > > > > --- > > > > meson.build | 1 + > > > > meson_options.txt | 3 +- > > > > src/libcamera/pipeline/virtual/meson.build | 5 + > > > > src/libcamera/pipeline/virtual/virtual.cpp | 251 > +++++++++++++++++++++ > > > > src/libcamera/pipeline/virtual/virtual.h | 78 +++++++ > > > > > > Do you expect other components to include this header in future ? If > > > not, you can move its content to the .cpp file I guess > > > > > > > > Actually `virtual/parser.h` needs to include it to return > VirtualCameraData > > when parsing the yaml config file [1]. Does that count :p? > > I guess it does :) > > > > > [1]: https://patchwork.libcamera.org/patch/20971/ > > > > > 5 files changed, 337 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 f946eba94..3cad3249a 100644 > > > > --- a/meson.build > > > > +++ b/meson.build > > > > @@ -222,6 +222,7 @@ pipelines_support = { > > > > 'simple': arch_arm, > > > > 'uvcvideo': ['any'], > > > > 'vimc': ['test'], > > > > + 'virtual': ['test'], > > > > } > > > > > > > > if pipelines.contains('all') > > > > diff --git a/meson_options.txt b/meson_options.txt > > > > index 7aa412491..c91cd241a 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 000000000..ba7ff754e > > > > --- /dev/null > > > > +++ b/src/libcamera/pipeline/virtual/meson.build > > > > @@ -0,0 +1,5 @@ > > > > +# SPDX-License-Identifier: CC0-1.0 > > > > + > > > > +libcamera_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 000000000..74eb8c7ad > > > > --- /dev/null > > > > +++ b/src/libcamera/pipeline/virtual/virtual.cpp > > > > @@ -0,0 +1,251 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * Copyright (C) 2023, Google Inc. > > > > + * > > > > + * virtual.cpp - Pipeline handler for virtual cameras > > > > + */ > > > > + > > > > > > You should include the header for the standard library construct you > > > use. I see vectors, maps, unique_ptrs etc > > > > > Done, please check again. > > > > > > > > > > > +#include "virtual.h" > > > > + > > > > +#include <libcamera/base/log.h> > > > > + > > > > +#include <libcamera/camera.h> > > > > +#include <libcamera/control_ids.h> > > > > +#include <libcamera/controls.h> > > > > +#include <libcamera/formats.h> > > > > +#include <libcamera/property_ids.h> > > > > + > > > > +#include "libcamera/internal/camera.h" > > > > > > The internal header includes the public one by definition > > > > > Ack. Removed the public one. > > > > > > > > > > > +#include "libcamera/internal/formats.h" > > > > > > This doesn't as <libcamera/formats.h> is generated. I wonder if it > > > should. > > > > > Keeping `#include <libcamera/formats.h>`. > > > > > > > > > > > +#include "libcamera/internal/pipeline_handler.h" > > > > + > > > > +namespace libcamera { > > > > + > > > > +LOG_DEFINE_CATEGORY(Virtual) > > > > + > > > > +namespace { > > > > + > > > > +uint64_t currentTimestamp() > > > > +{ > > > > + struct timespec ts; > > > > + if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) { > > > > + LOG(Virtual, Error) << "Get clock time fails"; > > > > + return 0; > > > > + } > > > > + > > > > + return ts.tv_sec * 1'000'000'000LL + ts.tv_nsec; > > > > +} > > > > > > Could https://en.cppreference.com/w/cpp/chrono/steady_clock/now save > > > you a custom function ? > > > > > > In example: > > > > > > const auto now = std::chrono::steady_clock::now(); > > > auto nsecs = > > > > std::chrono::duration_cast<std::chrono::nanoseconds>(now.time_since_epoch()); > > > std::cout << nsecs.count(); > > > > > > should give you the time in nanoseconds since system boot (if I got it > > > right) > > > > > > > > > > + > > > > +} // namespace > > > > > > nit: /* namespace */ > > > here and in other places > > > > > > Done > > > > > > > > + > > > > > > > > +VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData > > > *data) > > > > + : CameraConfiguration(), data_(data) > > > > +{ > > > > +} > > > > + > > > > +CameraConfiguration::Status VirtualCameraConfiguration::validate() > > > > +{ > > > > + Status status = Valid; > > > > + > > > > + if (config_.empty()) { > > > > + LOG(Virtual, Error) << "Empty config"; > > > > + return Invalid; > > > > + } > > > > + > > > > + /* Currently only one stream is supported */ > > > > + if (config_.size() > 1) { > > > > + config_.resize(1); > > > > + status = Adjusted; > > > > + } > > > > + > > > > + Size maxSize; > > > > + for (const auto &resolution : data_->supportedResolutions_) > > > > + maxSize = std::max(maxSize, resolution.size); > > > > + > > > > + for (StreamConfiguration &cfg : config_) { > > > > > > you only have config, or in the next patches will this be augmented ? > > > > > > Do you mean that I should check `sensorConfig` or `orientation` as > well? > > > > No I meant I was assuming the virtual pipline works with a single > stream by design. But seeing your replies to the next patches makes me > think my assumption was wrong. > > > > + 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) { > > > > + cfg.size = maxSize; > > > > + status = Adjusted; > > > > + } > > > > > > so it's either the exact resolution or the biggest available one ? > > > > > > As you have a single config it's easy to get the closest one to the > > > requested size, if it doesn't match exactly one of the supported > > > resolutions. > > > > > > > Hmm, I think it's a bit hard to define the "closest". Do we compare > > the area size directly? Do we prefer a size that has both larger or > > the same height and width? > > > > Good question, it's generally a pipeline decision (which is not > optimal, as we should aim to a unified behaviour among pipelines). > > As this is for testing, I think it's fine to keep what you have, but > could you add a comment to highlight this implementation decision ? > > Sure, will be updated in v11. Please take another look. > > > > > > > > > + > > > > + const PixelFormatInfo &info = > > > PixelFormatInfo::info(cfg.pixelFormat); > > > > + cfg.stride = info.stride(cfg.size.width, 0, 1); > > > > + cfg.frameSize = info.frameSize(cfg.size, 1); > > > > + > > > > + cfg.setStream(const_cast<Stream *>(&data_->stream_)); > > > > + > > > > + cfg.bufferCount = > VirtualCameraConfiguration::kBufferCount; > > > > + } > > > > + > > > > + return status; > > > > +} > > > > + > > > > +PipelineHandlerVirtual::PipelineHandlerVirtual(CameraManager > *manager) > > > > + : PipelineHandler(manager) > > > > +{ > > > > +} > > > > + > > > > +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; > > > > + > > > > + Size minSize, sensorResolution; > > > > + for (const auto &resolution : data->supportedResolutions_) { > > > > + if (minSize.isNull() || minSize > resolution.size) > > > > + minSize = resolution.size; > > > > + > > > > + sensorResolution = std::max(sensorResolution, > > > resolution.size); > > > > > > As you do this min/max search in a few places, why not doing it when > > > you construct data->supportedResolutions_ once ? > > > > > Added in VirtualCameraData. > > > > > > > > > > > + } > > > > + > > > > + for (const StreamRole role : roles) { > > > > > > If the pipeline handler works with a single stream, you should only > > > consider the first role maybe > > > > > Actually I think there's no reason to only support one Stream in > > Virtual Pipeline Handler. The raw stream doesn't seem to be > > properly supported in the following patches though. I think we > > should drop the support of raw. > > > > I assumed (maybe from a previous discussion) you were going to have a > single stream. I'm sorry, I was wrong. > Nah, I prepared for multiple streams, but haven't enabled it yet :p. > > Please drop RAW, yes. > Done in v10. > > > > > > > > > > + std::map<PixelFormat, std::vector<SizeRange>> > > > streamFormats; > > > > + unsigned int bufferCount; > > > > + PixelFormat pixelFormat; > > > > + > > > > + switch (role) { > > > > + case StreamRole::StillCapture: > > > > + pixelFormat = formats::NV12; > > > > + bufferCount = > > > VirtualCameraConfiguration::kBufferCount; > > > > + streamFormats[pixelFormat] = { { minSize, > > > sensorResolution } }; > > > > > > bufferCount and streamFormats can be assigned outsize of the cases, > > > they're the same for all roles > > > > > Done > > > > > > > > > > > + > > > > + break; > > > > + > > > > + case StreamRole::Raw: { > > > > + /* \todo check */ > > > > + pixelFormat = formats::SBGGR10; > > > > + bufferCount = > > > VirtualCameraConfiguration::kBufferCount; > > > > + streamFormats[pixelFormat] = { { minSize, > > > sensorResolution } }; > > > > + > > > > + break; > > > > + } > > > > + > > > > + case StreamRole::Viewfinder: > > > > + case StreamRole::VideoRecording: { > > > > + pixelFormat = formats::NV12; > > > > + bufferCount = > > > VirtualCameraConfiguration::kBufferCount; > > > > + streamFormats[pixelFormat] = { { minSize, > > > sensorResolution } }; > > > > + > > > > + break; > > > > + } > > > > + > > > > + default: > > > > + LOG(Virtual, Error) > > > > + << "Requested stream role not > supported: " > > > << role; > > > > + config.reset(); > > > > + return config; > > > > + } > > > > + > > > > + StreamFormats formats(streamFormats); > > > > + StreamConfiguration cfg(formats); > > > > + cfg.size = sensorResolution; > > > > + cfg.pixelFormat = pixelFormat; > > > > + cfg.bufferCount = bufferCount; > > > > + config->addConfiguration(cfg); > > > > + } > > > > + > > > > + if (config->validate() == CameraConfiguration::Invalid) > > > > + config.reset(); > > > > + > > > > + return config; > > > > +} > > > > + > > > > +int PipelineHandlerVirtual::configure( > > > > + [[maybe_unused]] Camera *camera, > > > > + [[maybe_unused]] CameraConfiguration *config) > > > > > > int PipelineHandlerVirtual::configure([[maybe_unused]] Camera *camera, > > > [[maybe_unused]] > CameraConfiguration > > > *config) > > > > > > Done > > > > > > > > +{ > > > > + // Nothing to be done. > > > > + return 0; > > > > +} > > > > + > > > > +int PipelineHandlerVirtual::exportFrameBuffers( > > > > + [[maybe_unused]] Camera *camera, > > > > + Stream *stream, > > > > + std::vector<std::unique_ptr<FrameBuffer>> *buffers) > > > > > > int PipelineHandlerVirtual::exportFrameBuffers([[maybe_unused]] Camera > > > *camera, > > > Stream *stream, > > > > > > std::vector<std::unique_ptr<FrameBuffer>> *buffers) > > > > > > if you prefer > > > > > > Done > > > > > > > > +{ > > > > + if (!dmaBufAllocator_.isValid()) > > > > + return -ENOBUFS; > > > > + > > > > + const StreamConfiguration &config = stream->configuration(); > > > > + > > > > + auto info = PixelFormatInfo::info(config.pixelFormat); > > > > + > > > > + std::vector<std::size_t> 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); > > > > > > ah that's probably why you return count from > > > DmaBufferAllocator::exportBuffers() > > > > > Exactly :) > > > > > > > > > > > +} > > > > + > > > > +int PipelineHandlerVirtual::start([[maybe_unused]] Camera *camera, > > > > + [[maybe_unused]] const ControlList > > > *controls) > > > > +{ > > > > + /* \todo Start reading the virtual video if any. */ > > > > + return 0; > > > > +} > > > > + > > > > +void PipelineHandlerVirtual::stopDevice([[maybe_unused]] Camera > *camera) > > > > +{ > > > > + /* \todo Reset the virtual video if any. */ > > > > +} > > > > + > > > > +int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] > Camera > > > *camera, > > > > + Request *request) > > > > +{ > > > > + /* \todo Read from the virtual video if any. */ > > > > + 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) > > > > +{ > > > > + /* \todo Add virtual cameras according to a config file. */ > > > > + > > > > + std::unique_ptr<VirtualCameraData> data = > > > std::make_unique<VirtualCameraData>(this); > > > > + > > > > + data->supportedResolutions_.resize(2); > > > > + data->supportedResolutions_[0] = { .size = Size(1920, 1080), > > > .frame_rates = { 30 } }; > > > > + data->supportedResolutions_[1] = { .size = Size(1280, 720), > > > .frame_rates = { 30, 60 } }; > > > > + > > > > + 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 = 30, max_frame_duration = 60; > > > > > > doesn't match the above frame rates and it should be expressed in > > > microseconds. I would suggest for this patch to set both framerates to > > > 30 and initialize FrameDurationLimits with {33333, 333333} > > > > > > It's not a big deal however, it will be replaced later in the series > > > > > > > > Done, thanks! > > > > > > > > > > > + 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{ &data->stream_ }; > > > > + const std::string id = "Virtual0"; > > > > + std::shared_ptr<Camera> camera = > Camera::create(std::move(data), > > > id, streams); > > > > + registerCamera(std::move(camera)); > > > > + > > > > + return false; // Prevent infinite loops for now > > > > > > I presume this will also be changed in next patches... > > > > > Updated in this patch, with a static boolean though. > > > > > > > > > > > +} > > > > + > > > > +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 000000000..6fc6b34d8 > > > > --- /dev/null > > > > +++ b/src/libcamera/pipeline/virtual/virtual.h > > > > @@ -0,0 +1,78 @@ > > > > +/* SPDX-License-Identifier: LGPL-2.1-or-later */ > > > > +/* > > > > + * Copyright (C) 2023, Google Inc. > > > > + * > > > > + * virtual.h - Pipeline handler for virtual cameras > > > > + */ > > > > + > > > > +#pragma once > > > > + > > > > +#include <libcamera/base/file.h> > > > > + > > > > +#include "libcamera/internal/camera.h" > > > > +#include "libcamera/internal/dma_buf_allocator.h" > > > > +#include "libcamera/internal/pipeline_handler.h" > > > > + > > > > +namespace libcamera { > > > > + > > > > +class VirtualCameraData : public Camera::Private > > > > +{ > > > > +public: > > > > + struct Resolution { > > > > + Size size; > > > > + std::vector<int> frame_rates; > > > > + }; > > > > + VirtualCameraData(PipelineHandler *pipe) > > > > + : Camera::Private(pipe) > > > > + { > > > > + } > > > > + > > > > + ~VirtualCameraData() = default; > > > > + > > > > + std::vector<Resolution> supportedResolutions_; > > > > + > > > > + Stream stream_; > > > > +}; > > > > + > > > > +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: > > > > + VirtualCameraData *cameraData(Camera *camera) > > > > + { > > > > + return static_cast<VirtualCameraData *>(camera->_d()); > > > > + } > > > > + > > > > + DmaBufAllocator dmaBufAllocator_; > > > > +}; > > > > + > > > > +} // namespace libcamera > > > > -- > > > > 2.46.0.184.g6999bdac58-goog > > > > > > > >
diff --git a/meson.build b/meson.build index f946eba94..3cad3249a 100644 --- a/meson.build +++ b/meson.build @@ -222,6 +222,7 @@ pipelines_support = { 'simple': arch_arm, 'uvcvideo': ['any'], 'vimc': ['test'], + 'virtual': ['test'], } if pipelines.contains('all') diff --git a/meson_options.txt b/meson_options.txt index 7aa412491..c91cd241a 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 000000000..ba7ff754e --- /dev/null +++ b/src/libcamera/pipeline/virtual/meson.build @@ -0,0 +1,5 @@ +# SPDX-License-Identifier: CC0-1.0 + +libcamera_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 000000000..74eb8c7ad --- /dev/null +++ b/src/libcamera/pipeline/virtual/virtual.cpp @@ -0,0 +1,251 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2023, Google Inc. + * + * virtual.cpp - Pipeline handler for virtual cameras + */ + +#include "virtual.h" + +#include <libcamera/base/log.h> + +#include <libcamera/camera.h> +#include <libcamera/control_ids.h> +#include <libcamera/controls.h> +#include <libcamera/formats.h> +#include <libcamera/property_ids.h> + +#include "libcamera/internal/camera.h" +#include "libcamera/internal/formats.h" +#include "libcamera/internal/pipeline_handler.h" + +namespace libcamera { + +LOG_DEFINE_CATEGORY(Virtual) + +namespace { + +uint64_t currentTimestamp() +{ + struct timespec ts; + if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) { + LOG(Virtual, Error) << "Get clock time fails"; + return 0; + } + + return ts.tv_sec * 1'000'000'000LL + ts.tv_nsec; +} + +} // namespace + +VirtualCameraConfiguration::VirtualCameraConfiguration(VirtualCameraData *data) + : CameraConfiguration(), data_(data) +{ +} + +CameraConfiguration::Status VirtualCameraConfiguration::validate() +{ + Status status = Valid; + + if (config_.empty()) { + LOG(Virtual, Error) << "Empty config"; + return Invalid; + } + + /* Currently only one stream is supported */ + if (config_.size() > 1) { + config_.resize(1); + status = Adjusted; + } + + Size maxSize; + for (const auto &resolution : data_->supportedResolutions_) + maxSize = std::max(maxSize, resolution.size); + + 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) { + cfg.size = maxSize; + 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.setStream(const_cast<Stream *>(&data_->stream_)); + + cfg.bufferCount = VirtualCameraConfiguration::kBufferCount; + } + + return status; +} + +PipelineHandlerVirtual::PipelineHandlerVirtual(CameraManager *manager) + : PipelineHandler(manager) +{ +} + +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; + + Size minSize, sensorResolution; + for (const auto &resolution : data->supportedResolutions_) { + if (minSize.isNull() || minSize > resolution.size) + minSize = resolution.size; + + sensorResolution = std::max(sensorResolution, resolution.size); + } + + for (const StreamRole role : roles) { + std::map<PixelFormat, std::vector<SizeRange>> streamFormats; + unsigned int bufferCount; + PixelFormat pixelFormat; + + switch (role) { + case StreamRole::StillCapture: + pixelFormat = formats::NV12; + bufferCount = VirtualCameraConfiguration::kBufferCount; + streamFormats[pixelFormat] = { { minSize, sensorResolution } }; + + break; + + case StreamRole::Raw: { + /* \todo check */ + pixelFormat = formats::SBGGR10; + bufferCount = VirtualCameraConfiguration::kBufferCount; + streamFormats[pixelFormat] = { { minSize, sensorResolution } }; + + break; + } + + case StreamRole::Viewfinder: + case StreamRole::VideoRecording: { + pixelFormat = formats::NV12; + bufferCount = VirtualCameraConfiguration::kBufferCount; + streamFormats[pixelFormat] = { { minSize, sensorResolution } }; + + break; + } + + default: + LOG(Virtual, Error) + << "Requested stream role not supported: " << role; + config.reset(); + return config; + } + + StreamFormats formats(streamFormats); + StreamConfiguration cfg(formats); + cfg.size = sensorResolution; + cfg.pixelFormat = pixelFormat; + cfg.bufferCount = bufferCount; + config->addConfiguration(cfg); + } + + if (config->validate() == CameraConfiguration::Invalid) + config.reset(); + + return config; +} + +int PipelineHandlerVirtual::configure( + [[maybe_unused]] Camera *camera, + [[maybe_unused]] CameraConfiguration *config) +{ + // Nothing to be done. + 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<std::size_t> 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) +{ + /* \todo Start reading the virtual video if any. */ + return 0; +} + +void PipelineHandlerVirtual::stopDevice([[maybe_unused]] Camera *camera) +{ + /* \todo Reset the virtual video if any. */ +} + +int PipelineHandlerVirtual::queueRequestDevice([[maybe_unused]] Camera *camera, + Request *request) +{ + /* \todo Read from the virtual video if any. */ + 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) +{ + /* \todo Add virtual cameras according to a config file. */ + + std::unique_ptr<VirtualCameraData> data = std::make_unique<VirtualCameraData>(this); + + data->supportedResolutions_.resize(2); + data->supportedResolutions_[0] = { .size = Size(1920, 1080), .frame_rates = { 30 } }; + data->supportedResolutions_[1] = { .size = Size(1280, 720), .frame_rates = { 30, 60 } }; + + 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 = 30, max_frame_duration = 60; + 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{ &data->stream_ }; + const std::string id = "Virtual0"; + std::shared_ptr<Camera> camera = Camera::create(std::move(data), id, streams); + registerCamera(std::move(camera)); + + return false; // Prevent infinite loops for now +} + +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 000000000..6fc6b34d8 --- /dev/null +++ b/src/libcamera/pipeline/virtual/virtual.h @@ -0,0 +1,78 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2023, Google Inc. + * + * virtual.h - Pipeline handler for virtual cameras + */ + +#pragma once + +#include <libcamera/base/file.h> + +#include "libcamera/internal/camera.h" +#include "libcamera/internal/dma_buf_allocator.h" +#include "libcamera/internal/pipeline_handler.h" + +namespace libcamera { + +class VirtualCameraData : public Camera::Private +{ +public: + struct Resolution { + Size size; + std::vector<int> frame_rates; + }; + VirtualCameraData(PipelineHandler *pipe) + : Camera::Private(pipe) + { + } + + ~VirtualCameraData() = default; + + std::vector<Resolution> supportedResolutions_; + + Stream stream_; +}; + +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: + VirtualCameraData *cameraData(Camera *camera) + { + return static_cast<VirtualCameraData *>(camera->_d()); + } + + DmaBufAllocator dmaBufAllocator_; +}; + +} // namespace libcamera