[libcamera-devel] [PATCH v2 1/1] Add fake pipeline handler
Cheng-Hao Yang
chenghaoyang at chromium.org
Mon Oct 31 06:50:55 CET 2022
Thanks Kieran for the further review and the help of the new allocator!
On Fri, Oct 28, 2022 at 7:17 PM Kieran Bingham <
kieran.bingham at ideasonboard.com> wrote:
> 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/
>
>
I guess you're right. Let me start from an empty one instead.
It might take some time though.
> > + */
> > + 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
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.libcamera.org/pipermail/libcamera-devel/attachments/20221031/330288c0/attachment.htm>
More information about the libcamera-devel
mailing list