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