[libcamera-devel] [PATCH v2 1/1] Add fake pipeline handler
Kieran Bingham
kieran.bingham at ideasonboard.com
Fri Oct 28 12:17:40 CEST 2022
Hi Harvey,
I do think I see benefits to a no-hardware (no-kernel) camera support.
I believe it would help developments and testing. So I'm keen to see
this progress.
The big part here that worries me is the Fatal when trying to handle
buffer alloction.
We need to do better than that, as it means qcam just aborts when I
apply this patch and try to test it.
Quoting Harvey Yang via libcamera-devel (2022-10-14 11:36:12)
Missing commit message, and an SoB.
> ---
> meson_options.txt | 2 +-
> src/libcamera/pipeline/fake/fake.cpp | 441 ++++++++++++++++++++++++
> src/libcamera/pipeline/fake/meson.build | 3 +
> test/camera/camera_reconfigure.cpp | 2 +-
> 4 files changed, 446 insertions(+), 2 deletions(-)
> create mode 100644 src/libcamera/pipeline/fake/fake.cpp
> create mode 100644 src/libcamera/pipeline/fake/meson.build
>
> diff --git a/meson_options.txt b/meson_options.txt
> index f1d67808..f08dfc5f 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -37,7 +37,7 @@ option('lc-compliance',
>
> option('pipelines',
> type : 'array',
> - choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc'],
> + choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc', 'fake'],
I know it's bikeshedding territory - but the more I've thought about
this, the more I think 'virtual' would be a better name to get this in.
'Fake' sounds so ... negative ? fake ?
But it's just a color of the shed.
> description : 'Select which pipeline handlers to include')
>
> option('qcam',
> diff --git a/src/libcamera/pipeline/fake/fake.cpp b/src/libcamera/pipeline/fake/fake.cpp
> new file mode 100644
> index 00000000..1ee24e3b
> --- /dev/null
> +++ b/src/libcamera/pipeline/fake/fake.cpp
> @@ -0,0 +1,441 @@
> +/* SPDX-License-Identifier: LGPL-2.1-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
2022
> + *
> + * fake.cpp - Pipeline handler for fake cameras
> + */
> +
> +#include <algorithm>
> +#include <iomanip>
> +#include <memory>
> +#include <queue>
> +#include <set>
> +#include <vector>
> +
> +#include <libcamera/base/log.h>
> +#include <libcamera/base/utils.h>
> +
> +#include <libcamera/camera_manager.h>
> +#include <libcamera/controls.h>
> +#include <libcamera/control_ids.h>
> +#include <libcamera/formats.h>
> +#include <libcamera/property_ids.h>
> +#include <libcamera/request.h>
> +#include <libcamera/stream.h>
> +
> +#include "libcamera/internal/camera.h"
> +#include "libcamera/internal/device_enumerator.h"
> +#include "libcamera/internal/formats.h"
> +#include "libcamera/internal/framebuffer.h"
> +#include "libcamera/internal/media_device.h"
> +#include "libcamera/internal/pipeline_handler.h"
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(Fake)
> +
> +uint64_t CurrentTimestamp()
> +{
> + struct timespec ts;
> + if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) {
> + LOG(Fake, Error) << "Get clock time fails";
> + return 0;
> + }
> +
> + return ts.tv_sec * 1'000'000'000LL + ts.tv_nsec;
> +}
> +
> +static const ControlInfoMap::Map FakeControls = {
> + { &controls::draft::PipelineDepth, ControlInfo(2, 3) },
> +};
> +
> +class FakeCameraData : public Camera::Private
> +{
> +public:
> + struct Resolution {
> + Size size;
> + std::vector<int> frame_rates;
> + std::vector<PixelFormat> formats;
> + };
> +
> + FakeCameraData(PipelineHandler *pipe)
> + : Camera::Private(pipe)
> + {
> + }
> +
> + std::vector<Resolution> supportedResolutions_;
> +
> + Stream outStream_;
> + Stream vfStream_;
> + Stream rawStream_;
> +
> + bool started_ = false;
> +};
> +
> +class FakeCameraConfiguration : public CameraConfiguration
> +{
> +public:
> + static constexpr unsigned int kBufferCount = 4; // 4~6
> + static constexpr unsigned int kMaxStreams = 3;
> +
> + FakeCameraConfiguration(FakeCameraData *data);
> +
> + Status validate() override;
> +
> +private:
> + /*
> + * The FakeCameraData instance is guaranteed to be valid as long as the
> + * corresponding Camera instance is valid. In order to borrow a
> + * reference to the camera data, store a new reference to the camera.
> + */
> + const FakeCameraData *data_;
> +};
> +
> +class PipelineHandlerFake : public PipelineHandler
> +{
> +public:
> + PipelineHandlerFake(CameraManager *manager);
> +
> + CameraConfiguration *generateConfiguration(Camera *camera,
> + const StreamRoles &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:
> + FakeCameraData *cameraData(Camera *camera)
> + {
> + return static_cast<FakeCameraData *>(camera->_d());
> + }
> +
> + int registerCameras();
> +
> + static bool registered_;
> +};
> +
> +bool PipelineHandlerFake::registered_ = false;
> +
> +FakeCameraConfiguration::FakeCameraConfiguration(FakeCameraData *data)
> + : CameraConfiguration()
> +{
> + data_ = data;
> +}
> +
> +CameraConfiguration::Status FakeCameraConfiguration::validate()
> +{
> + Status status = Valid;
> +
> + if (config_.empty())
> + return Invalid;
> +
> + /* Cap the number of entries to the available streams. */
> + if (config_.size() > kMaxStreams) {
> + config_.resize(kMaxStreams);
> + status = Adjusted;
> + }
> +
> + /*
> + * Validate the requested stream configuration and select the sensor
> + * format by collecting the maximum RAW stream width and height and
> + * picking the closest larger match.
> + *
> + * If no RAW stream is requested use the one of the largest YUV stream,
> + * plus margin pixels for the IF and BDS rectangle to downscale.
> + *
> + * \todo Clarify the IF and BDS margins requirements.
A fake camera doesn't have any IF/BDS ?
Still lots of clean up required.
It may be cleaner to start from an empty pipeline handler (or the vivid
pipeline handler [0]) and work up, rather work down from the IPU3.
[0] https://git.libcamera.org/libcamera/vivid.git/log/
> + */
> + unsigned int rawCount = 0;
> + unsigned int yuvCount = 0;
> + Size rawRequirement;
> + Size maxYuvSize;
> + Size rawSize;
> +
> + for (const StreamConfiguration &cfg : config_) {
> + const PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);
> +
> + if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
> + rawCount++;
> + rawSize = std::max(rawSize, cfg.size);
> + } else {
> + yuvCount++;
> + maxYuvSize = std::max(maxYuvSize, cfg.size);
> + rawRequirement.expandTo(cfg.size);
> + }
> + }
> +
> + // TODO: Check the configuration file.
> + if (rawCount > 1 || yuvCount > 2) {
> + LOG(Fake, Debug) << "Camera configuration not supported";
> + return Invalid;
> + }
> +
> + /*
> + * Adjust the configurations if needed and assign streams while
> + * iterating them.
> + */
> + bool mainOutputAvailable = true;
> + for (unsigned int i = 0; i < config_.size(); ++i) {
> + const PixelFormatInfo &info = PixelFormatInfo::info(config_[i].pixelFormat);
> + const StreamConfiguration originalCfg = config_[i];
> + StreamConfiguration *cfg = &config_[i];
> +
> + LOG(Fake, Debug) << "Validating stream: " << config_[i].toString();
> +
> + if (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {
> + /* Initialize the RAW stream with the CIO2 configuration. */
No CIO2.
> + cfg->size = rawSize;
> + // TODO: check
> + cfg->pixelFormat = formats::SBGGR10;
> + cfg->bufferCount = FakeCameraConfiguration::kBufferCount;
> + cfg->stride = info.stride(cfg->size.width, 0, 64);
> + cfg->frameSize = info.frameSize(cfg->size, 64);
Are these limits you /want/ to impose?
> + cfg->setStream(const_cast<Stream *>(&data_->rawStream_));
> +
> + LOG(Fake, Debug) << "Assigned " << cfg->toString()
> + << " to the raw stream";
> + } else {
> + /* Assign and configure the main and viewfinder outputs. */
> +
> + cfg->pixelFormat = formats::NV12;
> + cfg->bufferCount = kBufferCount;
> + cfg->stride = info.stride(cfg->size.width, 0, 1);
> + cfg->frameSize = info.frameSize(cfg->size, 1);
> +
> + /*
> + * Use the main output stream in case only one stream is
> + * requested or if the current configuration is the one
> + * with the maximum YUV output size.
> + */
> + if (mainOutputAvailable &&
> + (originalCfg.size == maxYuvSize || yuvCount == 1)) {
> + cfg->setStream(const_cast<Stream *>(&data_->outStream_));
> + mainOutputAvailable = false;
> +
> + LOG(Fake, Debug) << "Assigned " << cfg->toString()
> + << " to the main output";
> + } else {
> + cfg->setStream(const_cast<Stream *>(&data_->vfStream_));
> +
> + LOG(Fake, Debug) << "Assigned " << cfg->toString()
> + << " to the viewfinder output";
> + }
> + }
> +
> + if (cfg->pixelFormat != originalCfg.pixelFormat ||
> + cfg->size != originalCfg.size) {
> + LOG(Fake, Debug)
> + << "Stream " << i << " configuration adjusted to "
> + << cfg->toString();
> + status = Adjusted;
> + }
> + }
> +
> + return status;
> +}
> +
> +PipelineHandlerFake::PipelineHandlerFake(CameraManager *manager)
> + : PipelineHandler(manager)
> +{
> + // TODO: read the fake hal spec file.
> +}
> +
> +CameraConfiguration *PipelineHandlerFake::generateConfiguration(Camera *camera,
> + const StreamRoles &roles)
> +{
> + FakeCameraData *data = cameraData(camera);
> + FakeCameraConfiguration *config = new FakeCameraConfiguration(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;
> + Size size;
> +
> + switch (role) {
> + case StreamRole::StillCapture:
> + size = sensorResolution;
> + pixelFormat = formats::NV12;
> + bufferCount = FakeCameraConfiguration::kBufferCount;
> + streamFormats[pixelFormat] = { { minSize, size } };
> +
> + break;
> +
> + case StreamRole::Raw: {
> + // TODO: check
> + pixelFormat = formats::SBGGR10;
> + size = sensorResolution;
> + bufferCount = FakeCameraConfiguration::kBufferCount;
> + streamFormats[pixelFormat] = { { minSize, size } };
> +
> + break;
> + }
> +
> + case StreamRole::Viewfinder:
> + case StreamRole::VideoRecording: {
> + /*
> + * Default viewfinder and videorecording to 1280x720,
> + * capped to the maximum sensor resolution and aligned
> + * to the ImgU output constraints.
> + */
> + size = sensorResolution;
> + pixelFormat = formats::NV12;
> + bufferCount = FakeCameraConfiguration::kBufferCount;
> + streamFormats[pixelFormat] = { { minSize, size } };
> +
> + break;
> + }
> +
> + default:
> + LOG(Fake, Error)
> + << "Requested stream role not supported: " << role;
> + delete config;
> + return nullptr;
> + }
> +
> + StreamFormats formats(streamFormats);
> + StreamConfiguration cfg(formats);
> + cfg.size = size;
> + cfg.pixelFormat = pixelFormat;
> + cfg.bufferCount = bufferCount;
> + config->addConfiguration(cfg);
> + }
> +
> + if (config->validate() == CameraConfiguration::Invalid)
> + return {};
> +
> + return config;
> +}
> +
> +int PipelineHandlerFake::configure(Camera *camera, CameraConfiguration *c)
> +{
> + if (camera || c)
> + return 0;
This doesn't look right ... or helpful ?
Oh - it's just to stop unused warnings I bet.
Maybe we should use them?
> + return 0;
> +}
> +
> +int PipelineHandlerFake::exportFrameBuffers(Camera *camera, Stream *stream,
> + std::vector<std::unique_ptr<FrameBuffer>> *buffers)
> +{
> + // Assume it's never called.
> + LOG(Fake, Fatal) << "exportFrameBuffers should never be called";
I /really/ don't like this though.
And this falls to a fairly big yak-shave... we need a way to get dma
bufs easier. I think we can do this by creating an allocator with
/dev/udmabuf
It's a yak though - but is it something you'd be able/willing/interested
to explore? If not - I'll try to look at it.
But I don't see benefit to merging a virtual/fake pipeline handler that
can't at least be run with qcam, and currently when I test this patch -
this is where it fails and breaks with this Fatal error.
> + if (camera || stream || buffers)
> + return -EINVAL;
> + return -EINVAL;
> +}
> +
> +int PipelineHandlerFake::start(Camera *camera, [[maybe_unused]] const ControlList *controls)
> +{
> + FakeCameraData *data = cameraData(camera);
> + data->started_ = true;
> +
> + return 0;
> +}
> +
> +void PipelineHandlerFake::stopDevice(Camera *camera)
> +{
> + FakeCameraData *data = cameraData(camera);
> +
> + data->started_ = false;
> +}
> +
> +int PipelineHandlerFake::queueRequestDevice(Camera *camera, Request *request)
> +{
> + if (!camera)
> + return -EINVAL;
> +
> + for (auto it : request->buffers())
> + completeBuffer(request, it.second);
> +
> + // TODO: request.metadata()
> + request->metadata().set(controls::SensorTimestamp, CurrentTimestamp());
> + completeRequest(request);
> +
> + return 0;
> +}
> +
> +bool PipelineHandlerFake::match(DeviceEnumerator *enumerator)
> +{
> + // TODO: exhaust all devices in |enumerator|.
> + if (!enumerator)
> + LOG(Fake, Info) << "Invalid enumerator";
> +
I bet this might cause us issues if we don't have any devices found by
the enumerator !
> + if (registered_)
> + return false;
> +
> + registered_ = true;
> + return registerCameras() == 0;
> +}
> +
> +/**
> + * \brief Initialise ImgU and CIO2 devices associated with cameras
> + *
> + * Initialise the two ImgU instances and create cameras with an associated
> + * CIO2 device instance.
Not an IPU3
> + *
> + * \return 0 on success or a negative error code for error or if no camera
> + * has been created
> + * \retval -ENODEV no camera has been created
> + */
> +int PipelineHandlerFake::registerCameras()
> +{
> + std::unique_ptr<FakeCameraData> data =
> + std::make_unique<FakeCameraData>(this);
> + std::set<Stream *> streams = {
> + &data->outStream_,
> + &data->vfStream_,
> + &data->rawStream_,
> + };
> +
> + // TODO: Read from config or from IPC.
> + // TODO: Check with Han-lin: Can this function be called more than once?
> + data->supportedResolutions_.resize(2);
> + data->supportedResolutions_[0].size = Size(1920, 1080);
> + data->supportedResolutions_[0].frame_rates.push_back(30);
> + data->supportedResolutions_[0].formats.push_back(formats::NV12);
> + data->supportedResolutions_[0].formats.push_back(formats::MJPEG);
> + data->supportedResolutions_[1].size = Size(1280, 720);
> + data->supportedResolutions_[1].frame_rates.push_back(30);
> + data->supportedResolutions_[1].frame_rates.push_back(60);
> + data->supportedResolutions_[1].formats.push_back(formats::NV12);
> + data->supportedResolutions_[1].formats.push_back(formats::MJPEG);
I'd be weary that if we report we can send MJPEG data, and then send
buffers which do not contain mjpeg, it could cause problems or
essentially feed jpeg decoders with bad data.
That 'might' be a suitable test case, but it's something to consider
explicitly.
> +
> + // TODO: Assign different locations for different cameras based on config.
> + data->properties_.set(properties::Location, properties::CameraLocationFront);
> + data->properties_.set(properties::PixelArrayActiveAreas, { Rectangle(Size(1920, 1080)) });
> +
> + // TODO: Set FrameDurationLimits based on config.
> + ControlInfoMap::Map controls = FakeControls;
> + 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);
> +
> + std::shared_ptr<Camera> camera =
> + Camera::create(std::move(data), "\\_SB_.PCI0.I2C4.CAM1" /* cameraId */, streams);
I don't like hardcoding a path like that. I'd prefer it was explicitly
called 'Fake' or 'Virtual' ... perhaps even with an index,
> +
> + manager_->addCamera(std::move(camera), {});
> +
> + return 0;
> +}
> +
> +REGISTER_PIPELINE_HANDLER(PipelineHandlerFake)
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/pipeline/fake/meson.build b/src/libcamera/pipeline/fake/meson.build
> new file mode 100644
> index 00000000..13980b4a
> --- /dev/null
> +++ b/src/libcamera/pipeline/fake/meson.build
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: CC0-1.0
> +
> +libcamera_sources += files([ 'fake.cpp' ])
> diff --git a/test/camera/camera_reconfigure.cpp b/test/camera/camera_reconfigure.cpp
> index d12e2413..06c87730 100644
> --- a/test/camera/camera_reconfigure.cpp
> +++ b/test/camera/camera_reconfigure.cpp
> @@ -98,7 +98,7 @@ private:
> return TestFail;
> }
>
> - requests_.push_back(move(request));
> + requests_.push_back(std::move(request));
What is this change for ? If it's required it should be broken out to a
seperate patch.
--
Kieran
> }
>
> camera_->requestCompleted.connect(this, &CameraReconfigure::requestComplete);
> --
> 2.38.0.413.g74048e4d9e-goog
>
More information about the libcamera-devel
mailing list