[{"id":25407,"web_url":"https://patchwork.libcamera.org/comment/25407/","msgid":"<20221012150607.76brmkwluibrqqzn@uno.localdomain>","date":"2022-10-12T15:06:07","subject":"Re: [libcamera-devel] [PATCH 1/1] Add fake pipeline handler","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Harvey,\n I had the same thought as Kieran had, why not vimc ? I understan\nyour reasons behind it and the requirement to backport patches for it\nto older kernel (btw, does it need anything more than CAP_IO_MC ?)\n\nI have some general questions\n\n1) What does this pipeline handler matches on ? It seems to me match()\nalways returns true, hence if the pipeline handler is compiled in, it\nwill always report one camera available ?\n\n2) What are the requirements for this pipeline handler ?\n\n Is it enough to test the Android layer to implement stub methods,\n or should frames be generated somehow (I'm thinking at CTS frame\n rate checks in example).\n\n Should it support multiple streams ? I guess it depends on the HW\n level one wants to test in CTS, or should it mimik the capabilities\n of a known device (ie IPU3 that can produce 2 YUV streams and one\n RAW)\n\n I guess however this can be started as a skeleton and be populated\n as required to complete CTS.\n\n3) Should all mentions of IPU3 related components be removed ?\n\n4) Possible bikeshedding, but I wonder if \"dummy\" isn't better than\n \"fake\"\n\nOn Wed, Oct 12, 2022 at 07:59:25AM +0000, Harvey Yang via libcamera-devel wrote:\n> ---\n> meson_options.txt | 2 +-\n> src/android/camera_capabilities.cpp | 1 +\n> src/libcamera/pipeline/fake/fake.cpp | 518 ++++++++++++++++++++++++\n> src/libcamera/pipeline/fake/meson.build | 3 +\n> src/libcamera/pipeline_handler.cpp | 2 +-\n> test/camera/camera_reconfigure.cpp | 2 +-\n> 6 files changed, 525 insertions(+), 3 deletions(-)\n> create mode 100644 src/libcamera/pipeline/fake/fake.cpp\n> create mode 100644 src/libcamera/pipeline/fake/meson.build\n>\n> diff --git a/meson_options.txt b/meson_options.txt\n> index f1d67808..f08dfc5f 100644\n> --- a/meson_options.txt\n> +++ b/meson_options.txt\n> @@ -37,7 +37,7 @@ option('lc-compliance',\n>\n> option('pipelines',\n> type : 'array',\n> - choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc'],\n> + choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple', 'uvcvideo', 'vimc', 'fake'],\n> description : 'Select which pipeline handlers to include')\n>\n> option('qcam',\n> diff --git a/src/android/camera_capabilities.cpp b/src/android/camera_capabilities.cpp\n> index 64bd8dde..730ceafc 100644\n> --- a/src/android/camera_capabilities.cpp\n> +++ b/src/android/camera_capabilities.cpp\n> @@ -1077,6 +1077,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> \t{\n> \t\tconst Span &rects =\n> \t\t\tproperties.get(properties::PixelArrayActiveAreas).value_or(Span{});\n> +\t\t// TODO: initialize at least one Rectangle as default.\n\nUnrelated, please drop\n\n> \t\tstd::vector data{\n> \t\t\tstatic_cast(rects[0].x),\n> \t\t\tstatic_cast(rects[0].y),\n> diff --git a/src/libcamera/pipeline/fake/fake.cpp b/src/libcamera/pipeline/fake/fake.cpp\n> new file mode 100644\n> index 00000000..518de6aa\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/fake/fake.cpp\n> @@ -0,0 +1,518 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * ipu3.cpp - Pipeline handler for Intel Fake\n> + */\n\nPlease update year and remove mentions of Intel ?\n\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n\nSeems like you're also using std::set\n\n> +\n> +#include \n> +#include \n> +\n> +#include \n\nall internal headers \"libcamera/internal/something.h\" should include\n, so if you include internal/camera.h you\nprobably can remove this\n\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/camera.h\"\n> +#include \"libcamera/internal/camera_lens.h\"\n> +#include \"libcamera/internal/camera_sensor.h\"\n> +#include \"libcamera/internal/delayed_controls.h\"\n\nThe three above are not used it seems\n\n> +#include \"libcamera/internal/device_enumerator.h\"\n> +#include \"libcamera/internal/framebuffer.h\"\n> +#include \"libcamera/internal/media_device.h\"\n> +#include \"libcamera/internal/pipeline_handler.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(Fake)\n> +\n> +uint64_t CurrentTimestamp()\n> +{\n> +\tstruct timespec ts;\n> +\tif (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) {\n> +\t\tLOG(Fake, Error) << \"Get clock time fails\";\n> +\t\treturn 0;\n> +\t}\n> +\n> +\treturn ts.tv_sec * 1'000'000'000LL + ts.tv_nsec;\n> +}\n> +\n> +static const ControlInfoMap::Map FakeControls = {\n\nMissing #include ?\n\n> +\t{ &controls::draft::PipelineDepth, ControlInfo(2, 3) },\n> +};\n> +\n> +class FakeCameraData : public Camera::Private\n> +{\n> +public:\n> +\tstruct Resolution {\n> +\t\tSize size;\n> +\t\tstd::vector frame_rates;\n> +\t\tstd::vector formats;\n\nI would rather use libcamera::formats to verify the Android HAL does\nthe translation right.\n\n> +\t};\n> +\n> +\tFakeCameraData(PipelineHandler *pipe)\n> +\t\t: Camera::Private(pipe), supportsFlips_(false)\n> +\t{\n> +\t}\n> +\n> +\tstd::vector supported_resolutions_;\n\nsupportedResolutions_\n\n> +\n> +\tStream outStream_;\n> +\tStream vfStream_;\n> +\tStream rawStream_;\n> +\n> +\t// TODO: Check if we should support.\n> +\tbool supportsFlips_;\n> +\tTransform rotationTransform_;\n\nFor sake of simplicity you can remoev flip/rotation support from the\nfirst skeleton\n\n> +\n> +\t// TODO: remove\n\nPlease :)\n\n> +\t/* Requests for which no buffer has been queued to the CIO2 device yet. */\n> +\tstd::queue pendingRequests_;\n> +\t/* Requests queued to the CIO2 device but not yet processed by the ImgU. */\n> +\tstd::queue processingRequests_;\n> +\n> +\t// ControlInfoMap ipaControls_;\n\nThis as well\n\n> +\tbool started_ = false;\n> +};\n> +\n> +class FakeCameraConfiguration : public CameraConfiguration\n> +{\n> +public:\n> +\tstatic constexpr unsigned int kBufferCount = 4; // 4~6\n> +\tstatic constexpr unsigned int kMaxStreams = 3;\n> +\n> +\tFakeCameraConfiguration(FakeCameraData *data);\n> +\n> +\tStatus validate() override;\n> +\n> +private:\n> +\t/*\n> +\t * The FakeCameraData instance is guaranteed to be valid as long as the\n> +\t * corresponding Camera instance is valid. In order to borrow a\n> +\t * reference to the camera data, store a new reference to the camera.\n> +\t */\n\nThis comes from the IPU3 pipeline handler, and I wonder if it still\napplies there or it should be removed.\n\n> +\tconst FakeCameraData *data_;\n> +};\n> +\n> +class PipelineHandlerFake : public PipelineHandler\n> +{\n> +public:\n> +\tstatic constexpr unsigned int V4L2_CID_Fake_PIPE_MODE = 0x009819c1;\n\nUnused, please drop\n\n> +\tstatic constexpr Size kViewfinderSize{ 1280, 720 };\n> +\n> +\tenum FakePipeModes {\n> +\t\tFakePipeModeVideo = 0,\n> +\t\tFakePipeModeStillCapture = 1,\n> +\t};\n\nditto\n\n> +\n> +\tPipelineHandlerFake(CameraManager *manager);\n> +\n> +\tCameraConfiguration *generateConfiguration(Camera *camera,\n> +\t\tconst StreamRoles &roles) override;\n\nAlign to open (\n\n> +\tint configure(Camera *camera, CameraConfiguration *config) override;\n> +\n> +\tint exportFrameBuffers(Camera *camera, Stream *stream,\n> +\t\t\t std::vector> *buffers) override;\n> +\n> +\tint start(Camera *camera, const ControlList *controls) override;\n> +\tvoid stopDevice(Camera *camera) override;\n> +\n> +\tint queueRequestDevice(Camera *camera, Request *request) override;\n> +\n> +\tbool match(DeviceEnumerator *enumerator) override;\n> +\n> +private:\n> +\tFakeCameraData *cameraData(Camera *camera)\n> +\t{\n> +\t\treturn static_cast(camera->_d());\n> +\t}\n> +\n> +\tint registerCameras();\n> +\n> +\tstatic bool registered_;\n> +};\n> +\n> +bool PipelineHandlerFake::registered_ = false;\n\nWhat purpose does this serve ? Do you get multiple calls to match()\nso that you need to keep a flag ?\n\n> +\n> +FakeCameraConfiguration::FakeCameraConfiguration(FakeCameraData *data)\n> +\t: CameraConfiguration()\n> +{\n> +\tdata_ = data;\n> +}\n> +\n> +CameraConfiguration::Status FakeCameraConfiguration::validate()\n> +{\n> +\tStatus status = Valid;\n> +\n> +\tif (config_.empty())\n> +\t\treturn Invalid;\n> +\n> +\t// Transform combined = transform * data_->rotationTransform_;\n> +\n> +\t// /*\n> +\t// * We combine the platform and user transform, but must \"adjust away\"\n> +\t// * any combined result that includes a transposition, as we can't do\n> +\t// * those. In this case, flipping only the transpose bit is helpful to\n> +\t// * applications - they either get the transform they requested, or have\n> +\t// * to do a simple transpose themselves (they don't have to worry about\n> +\t// * the other possible cases).\n> +\t// */\n> +\t// if (!!(combined & Transform::Transpose)) {\n> +\t// \t/*\n> +\t// \t * Flipping the transpose bit in \"transform\" flips it in the\n> +\t// \t * combined result too (as it's the last thing that happens),\n> +\t// \t * which is of course clearing it.\n> +\t// \t */\n> +\t// \ttransform ^= Transform::Transpose;\n> +\t// \tcombined &= ~Transform::Transpose;\n> +\t// \tstatus = Adjusted;\n> +\t// }\n> +\n> +\t// /*\n> +\t// * We also check if the sensor doesn't do h/vflips at all, in which\n> +\t// * case we clear them, and the application will have to do everything.\n> +\t// */\n> +\t// if (!data_->supportsFlips_ && !!combined) {\n> +\t// \t/*\n> +\t// \t * If the sensor can do no transforms, then combined must be\n> +\t// \t * changed to the identity. The only user transform that gives\n> +\t// \t * rise to this is the inverse of the rotation. (Recall that\n> +\t// \t * combined = transform * rotationTransform.)\n> +\t// \t */\n> +\t// \ttransform = -data_->rotationTransform_;\n> +\t// \tcombined = Transform::Identity;\n> +\t// \tstatus = Adjusted;\n> +\t// }\n> +\n> +\t/*\n> +\t * Store the final combined transform that configure() will need to\n> +\t * apply to the sensor to save us working it out again.\n> +\t */\n> +\t// combinedTransform_ = combined;\n\nPlease drop commented out code :0\n\n> +\n> +\t/* Cap the number of entries to the available streams. */\n> +\tif (config_.size() > kMaxStreams) {\n> +\t\tconfig_.resize(kMaxStreams);\n> +\t\tstatus = Adjusted;\n> +\t}\n> +\n> +\t/*\n> +\t * Validate the requested stream configuration and select the sensor\n> +\t * format by collecting the maximum RAW stream width and height and\n> +\t * picking the closest larger match.\n> +\t *\n> +\t * If no RAW stream is requested use the one of the largest YUV stream,\n> +\t * plus margin pixels for the IF and BDS rectangle to downscale.\n> +\t *\n> +\t * \\todo Clarify the IF and BDS margins requirements.\n\nPlease drop IPU3 related stuff\n\n> +\t */\n> +\tunsigned int rawCount = 0;\n> +\tunsigned int yuvCount = 0;\n> +\tSize rawRequirement;\n> +\tSize maxYuvSize;\n> +\tSize rawSize;\n> +\n> +\tfor (const StreamConfiguration &cfg : config_) {\n> +\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(cfg.pixelFormat);\n> +\n> +\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\n> +\t\t\trawCount++;\n> +\t\t\trawSize = std::max(rawSize, cfg.size);\n> +\t\t} else {\n> +\t\t\tyuvCount++;\n> +\t\t\tmaxYuvSize = std::max(maxYuvSize, cfg.size);\n> +\t\t\trawRequirement.expandTo(cfg.size);\n> +\t\t}\n> +\t}\n> +\n> +\t// TODO: Base on number of cameras?\n\nWell, should this pipeline register more than one camera ?\nAnd anyway, the number of streams is per-camera so the number of\ncameras should be not relevant here\n\n> +\tif (rawCount > 1 || yuvCount > 2) {\n> +\t\tLOG(Fake, Debug) << \"Camera configuration not supported\";\n> +\t\treturn Invalid;\n> +\t}\n> +\n> +\t/*\n> +\t * Generate raw configuration from CIO2.\n> +\t *\n> +\t * The output YUV streams will be limited in size to the maximum frame\n> +\t * size requested for the RAW stream, if present.\n> +\t *\n> +\t * If no raw stream is requested, generate a size from the largest YUV\n> +\t * stream, aligned to the ImgU constraints and bound\n> +\t * by the sensor's maximum resolution. See\n> +\t * https://bugs.libcamera.org/show_bug.cgi?id=32\n> +\t */\n> +\t// TODO\n> +\tif (rawSize.isNull())\n> +\t\trawSize = rawRequirement;\n\nAll of these serves on IPU3 to generate a size suitable for the CIO2\nand the sensor. You can remove it.\n\n> +\n> +\t/*\n> +\t * Adjust the configurations if needed and assign streams while\n> +\t * iterating them.\n> +\t */\n> +\tbool mainOutputAvailable = true;\n> +\tfor (unsigned int i = 0; i < config_.size(); ++i) {\n> +\t\tconst PixelFormatInfo &info = PixelFormatInfo::info(config_[i].pixelFormat);\n> +\t\tconst StreamConfiguration originalCfg = config_[i];\n> +\t\tStreamConfiguration *cfg = &config_[i];\n> +\n> +\t\tLOG(Fake, Debug) << \"Validating stream: \" << config_[i].toString();\n> +\n> +\t\tif (info.colourEncoding == PixelFormatInfo::ColourEncodingRAW) {\n> +\t\t\t/* Initialize the RAW stream with the CIO2 configuration. */\n> +\t\t\tcfg->size = rawSize;\n> +\t\t\t// TODO: check\n> +\t\t\tcfg->pixelFormat = formats::SBGGR10_IPU3;\n> +\t\t\tcfg->bufferCount = FakeCameraConfiguration::kBufferCount;\n> +\t\t\tcfg->stride = info.stride(cfg->size.width, 0, 64);\n> +\t\t\tcfg->frameSize = info.frameSize(cfg->size, 64);\n> +\t\t\tcfg->setStream(const_cast(&data_->rawStream_));\n> +\n> +\t\t\tLOG(Fake, Debug) << \"Assigned \" << cfg->toString()\n> +\t\t\t\t\t << \" to the raw stream\";\n> +\t\t} else {\n> +\t\t\t/* Assign and configure the main and viewfinder outputs. */\n> +\n> +\t\t\tcfg->pixelFormat = formats::NV12;\n> +\t\t\tcfg->bufferCount = kBufferCount;\n> +\t\t\tcfg->stride = info.stride(cfg->size.width, 0, 1);\n> +\t\t\tcfg->frameSize = info.frameSize(cfg->size, 1);\n> +\n> +\t\t\t/*\n> +\t\t\t * Use the main output stream in case only one stream is\n> +\t\t\t * requested or if the current configuration is the one\n> +\t\t\t * with the maximum YUV output size.\n> +\t\t\t */\n> +\t\t\tif (mainOutputAvailable &&\n> +\t\t\t (originalCfg.size == maxYuvSize || yuvCount == 1)) {\n> +\t\t\t\tcfg->setStream(const_cast(&data_->outStream_));\n> +\t\t\t\tmainOutputAvailable = false;\n> +\n> +\t\t\t\tLOG(Fake, Debug) << \"Assigned \" << cfg->toString()\n> +\t\t\t\t\t\t << \" to the main output\";\n> +\t\t\t} else {\n> +\t\t\t\tcfg->setStream(const_cast(&data_->vfStream_));\n> +\n> +\t\t\t\tLOG(Fake, Debug) << \"Assigned \" << cfg->toString()\n> +\t\t\t\t\t\t << \" to the viewfinder output\";\n> +\t\t\t}\n> +\t\t}\n\nAgain I'm not sure how much of the IPU3 should this pipeline mimick,\nor it should establish requirements and constraints (ie max 2 YUV\nstreams of max size width x height, and 1 RAW stream in format\nV4L2_PIX_FMT_..)\n\n> +\n> +\t\tif (cfg->pixelFormat != originalCfg.pixelFormat ||\n> +\t\t cfg->size != originalCfg.size) {\n> +\t\t\tLOG(Fake, Debug)\n> +\t\t\t\t<< \"Stream \" << i << \" configuration adjusted to \"\n> +\t\t\t\t<< cfg->toString();\n> +\t\t\tstatus = Adjusted;\n> +\t\t}\n> +\t}\n> +\n> +\treturn status;\n> +}\n> +\n> +PipelineHandlerFake::PipelineHandlerFake(CameraManager *manager)\n> +\t: PipelineHandler(manager)\n> +{\n> +\t// TODO: read the fake hal spec file.\n\nIf there's a design document with requirements, can you share it ?\n\n> +}\n> +\n> +CameraConfiguration *PipelineHandlerFake::generateConfiguration(Camera *camera,\n> +\t\t\t\t\t\t\t\tconst StreamRoles &roles)\n> +{\n> +\tFakeCameraData *data = cameraData(camera);\n> +\tFakeCameraConfiguration *config = new FakeCameraConfiguration(data);\n> +\n> +\tif (roles.empty())\n> +\t\treturn config;\n> +\n> +\tSize minSize, sensorResolution;\n> +\tfor (const auto& resolution : data->supported_resolutions_) {\n> +\t\tif (minSize.isNull() || minSize > resolution.size)\n> +\t\t\tminSize = resolution.size;\n> +\n> +\t\tsensorResolution = std::max(sensorResolution, resolution.size);\n> +\t}\n> +\n\nThose are hard-coded values, I think you can reuse them here.\nUnless the idea is to make this information come from a configuration\nfile or something similar.\n\n> +\tfor (const StreamRole role : roles) {\n> +\t\tstd::map> streamFormats;\n> +\t\tunsigned int bufferCount;\n> +\t\tPixelFormat pixelFormat;\n> +\t\tSize size;\n> +\n> +\t\tswitch (role) {\n> +\t\tcase StreamRole::StillCapture:\n> +\t\t\tsize = sensorResolution;\n> +\t\t\tpixelFormat = formats::NV12;\n> +\t\t\tbufferCount = FakeCameraConfiguration::kBufferCount;\n> +\t\t\tstreamFormats[pixelFormat] = { { minSize, size } };\n> +\n> +\t\t\tbreak;\n> +\n> +\t\tcase StreamRole::Raw: {\n> +\t\t\t// TODO: check\n> +\t\t\tpixelFormat = formats::SBGGR10_IPU3;\n> +\t\t\tsize = sensorResolution;\n> +\t\t\tbufferCount = FakeCameraConfiguration::kBufferCount;\n> +\t\t\tstreamFormats[pixelFormat] = { { minSize, size } };\n> +\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tcase StreamRole::Viewfinder:\n> +\t\tcase StreamRole::VideoRecording: {\n> +\t\t\t/*\n> +\t\t\t * Default viewfinder and videorecording to 1280x720,\n> +\t\t\t * capped to the maximum sensor resolution and aligned\n> +\t\t\t * to the ImgU output constraints.\n> +\t\t\t */\n> +\t\t\tsize = sensorResolution;\n> +\t\t\tpixelFormat = formats::NV12;\n> +\t\t\tbufferCount = FakeCameraConfiguration::kBufferCount;\n> +\t\t\tstreamFormats[pixelFormat] = { { minSize, size } };\n> +\n> +\t\t\tbreak;\n> +\t\t}\n> +\n> +\t\tdefault:\n> +\t\t\tLOG(Fake, Error)\n> +\t\t\t\t<< \"Requested stream role not supported: \" << role;\n> +\t\t\tdelete config;\n> +\t\t\treturn nullptr;\n> +\t\t}\n> +\n> +\t\tStreamFormats formats(streamFormats);\n> +\t\tStreamConfiguration cfg(formats);\n> +\t\tcfg.size = size;\n> +\t\tcfg.pixelFormat = pixelFormat;\n> +\t\tcfg.bufferCount = bufferCount;\n> +\t\tconfig->addConfiguration(cfg);\n> +\t}\n> +\n> +\tif (config->validate() == CameraConfiguration::Invalid)\n> +\t\treturn {};\n> +\n> +\treturn config;\n> +}\n> +\n> +int PipelineHandlerFake::configure(Camera *camera, CameraConfiguration *c)\n> +{\n> +\tif (camera || c)\n> +\t\treturn 0;\n> +\treturn 0;\n> +}\n> +\n> +int PipelineHandlerFake::exportFrameBuffers(Camera *camera, Stream *stream,\n> +\t\t\t\t\t std::vector> *buffers)\n> +{\n> +\t// Assume it's never called.\n> +\tLOG(Fake, Fatal) << \"exportFrameBuffers should never be called\";\n> +\tif (camera || stream || buffers)\n> +\t\treturn -EINVAL;\n> +\treturn -EINVAL;\n> +}\n> +\n> +int PipelineHandlerFake::start(Camera *camera, [[maybe_unused]] const ControlList *controls)\n> +{\n> +\tFakeCameraData *data = cameraData(camera);\n> +\tdata->started_ = true;\n> +\n> +\treturn 0;\n> +}\n> +\n> +void PipelineHandlerFake::stopDevice(Camera *camera)\n> +{\n> +\tFakeCameraData *data = cameraData(camera);\n> +\n> +\tdata->started_ = false;\n> +}\n> +\n> +int PipelineHandlerFake::queueRequestDevice(Camera *camera, Request *request)\n> +{\n> +\tif (!camera)\n> +\t\treturn -EINVAL;\n\nCan this happen ?\n\n> +\n> +\tfor (auto it : request->buffers())\n> +\t\tcompleteBuffer(request, it.second);\n> +\n> +\t// TODO: request.metadata()\n> +\trequest->metadata().set(controls::SensorTimestamp, CurrentTimestamp());\n> +\tcompleteRequest(request);\n> +\n> +\treturn 0;\n> +}\n> +\n> +bool PipelineHandlerFake::match(DeviceEnumerator *enumerator)\n> +{\n> +\t// TODO: exhaust all devices in |enumerator|.\n> +\tif (!enumerator)\n> +\t\tLOG(Fake, Info) << \"Invalid enumerator\";\n\nCan this happen ?\n\n> +\n> +\tif (registered_)\n> +\t\treturn false;\n> +\n> +\tregistered_ = true;\n> +\treturn registerCameras() == 0;\n> +}\n> +\n> +/**\n> + * \\brief Initialise ImgU and CIO2 devices associated with cameras\n> + *\n> + * Initialise the two ImgU instances and create cameras with an associated\n> + * CIO2 device instance.\n> + *\n> + * \\return 0 on success or a negative error code for error or if no camera\n> + * has been created\n> + * \\retval -ENODEV no camera has been created\n> + */\n> +int PipelineHandlerFake::registerCameras()\n> +{\n> +\tstd::unique_ptr data =\n> +\t\tstd::make_unique(this);\n> +\tstd::set streams = {\n> +\t\t&data->outStream_,\n> +\t\t&data->vfStream_,\n> +\t\t&data->rawStream_,\n> +\t};\n> +\n> +\t// TODO: Read from config or from IPC.\n> +\t// TODO: Check with Han-lin: Can this function be called more than once?\n> +\tdata->supported_resolutions_.resize(2);\n> +\tdata->supported_resolutions_[0].size = Size(1920, 1080);\n> +\tdata->supported_resolutions_[0].frame_rates.push_back(30);\n> +\tdata->supported_resolutions_[0].formats.push_back(\"YCbCr_420_888\");\n> +\tdata->supported_resolutions_[0].formats.push_back(\"BLOB\");\n> +\tdata->supported_resolutions_[1].size = Size(1280, 720);\n> +\tdata->supported_resolutions_[1].frame_rates.push_back(30);\n> +\tdata->supported_resolutions_[1].frame_rates.push_back(60);\n> +\tdata->supported_resolutions_[1].formats.push_back(\"YCbCr_420_888\");\n> +\tdata->supported_resolutions_[1].formats.push_back(\"BLOB\");\n> +\n> +\t// TODO: Assign different locations for different cameras based on config.\n> +\tdata->properties_.set(properties::Location, properties::CameraLocationFront);\n> +\tdata->properties_.set(properties::PixelArrayActiveAreas, { Rectangle(Size(1920, 1080)) });\n> +\n> +\t// TODO: Set FrameDurationLimits based on config.\n> +\tControlInfoMap::Map controls{};\n> +\tint64_t min_frame_duration = 30, max_frame_duration = 60;\n> +\tcontrols[&controls::FrameDurationLimits] = ControlInfo(min_frame_duration, max_frame_duration);\n> +\tdata->controlInfo_ = ControlInfoMap(std::move(controls), controls::controls);\n\nFakeControls is ignored\n\n> +\n> +\tstd::shared_ptr camera =\n> +\t\tCamera::create(std::move(data), \"\\\\_SB_.PCI0.I2C4.CAM1\" /* cameraId */, streams);\n> +\n> +\tregisterCamera(std::move(camera));\n> +\n> +\treturn 0;\n> +}\n> +\n> +REGISTER_PIPELINE_HANDLER(PipelineHandlerFake)\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/fake/meson.build b/src/libcamera/pipeline/fake/meson.build\n> new file mode 100644\n> index 00000000..13980b4a\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/fake/meson.build\n> @@ -0,0 +1,3 @@\n> +# SPDX-License-Identifier: CC0-1.0\n> +\n> +libcamera_sources += files([ 'fake.cpp' ])\n> diff --git a/src/libcamera/pipeline_handler.cpp b/src/libcamera/pipeline_handler.cpp\n> index e5cb751c..4261154d 100644\n> --- a/src/libcamera/pipeline_handler.cpp\n> +++ b/src/libcamera/pipeline_handler.cpp\n> @@ -536,7 +536,7 @@ void PipelineHandler::registerCamera(std::shared_ptr camera)\n> \tcameras_.push_back(camera);\n>\n> \tif (mediaDevices_.empty())\n> -\t\tLOG(Pipeline, Fatal)\n> +\t\tLOG(Pipeline, Error)\n\nI don't think we want that, you should probably open code the\n\n\tmanager_->addCamera(std::move(camera), devnums);\n\ncall, with an empty list of devnums, which should be fine for this use\ncase.\n\nThe PipelineHandler::cameras_ vector will remain unpopulated, but\nsince it is used only in case of a media device disconnection (afaict)\nit should be fine in your case\n\n\n> \t\t\t<< \"Registering camera with no media devices!\";\n>\n> \t/*\n> diff --git a/test/camera/camera_reconfigure.cpp b/test/camera/camera_reconfigure.cpp\n> index d12e2413..06c87730 100644\n> --- a/test/camera/camera_reconfigure.cpp\n> +++ b/test/camera/camera_reconfigure.cpp\n> @@ -98,7 +98,7 @@ private:\n> \t\t\t\treturn TestFail;\n> \t\t\t}\n>\n> -\t\t\trequests_.push_back(move(request));\n> +\t\t\trequests_.push_back(std::move(request));\n\nUnrelated and possibly not necessary, as the test has\n using namespace std;\n\nLooking forward to seeing this skeleton getting populated, it will help\ntesting the Android layer!\n\nThanks\n j\n\n> \t\t}\n>\n> \t\tcamera_->requestCompleted.connect(this, &CameraReconfigure::requestComplete);\n> --\n> 2.38.0.rc1.362.ged0d419d3c-goog\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 676E5BD16B\n\tfor ;\n\tWed, 12 Oct 2022 15:06:12 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 77E3962DA0;\n\tWed, 12 Oct 2022 17:06:11 +0200 (CEST)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[IPv6:2001:4b98:dc4:8::225])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DE7D9603D7\n\tfor ;\n\tWed, 12 Oct 2022 17:06:09 +0200 (CEST)","(Authenticated sender: jacopo@jmondi.org)\n\tby mail.gandi.net (Postfix) with ESMTPSA id 2A9EF1C000C;\n\tWed, 12 Oct 2022 15:06:08 +0000 (UTC)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1665587171;\n\tbh=gxiviLlQnbqUbZANz1vSDbAlKHe3aADXMO1cA7C5BKY=;\n\th=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=iSa0dWcdqZue4AL6JN0wwOf4GD+dhtnTt6ioIV53i4pQrCVgn3BI9OYFfJR8RXLuR\n\tP9do6DdBqZEVDveJEWX7pEDaU4JMug3t+uYopZbJtJlJHOWgwkqY+FS1eiilvW01EU\n\tE3+7tGN5FdXvAqLo0vZq/000A43DwwOrI3j/7Ukk66Ln1UV7KmW6ndriwILfJfONoc\n\tBeHUDQVjYvX1FZDe5fZD6sigILA43dI8IzGX4KDS8g+LmY01ID15az4l1WIACVQfpt\n\taKNgg2ySy+fQMOQKL+1Pzs65NLeU39SyBtcVqwLKWyMlXLtUX0mg6N1YLM6X6ik0BM\n\tdxwt1jXycLLhQ==","Date":"Wed, 12 Oct 2022 17:06:07 +0200","To":"Harvey Yang ","Message-ID":"<20221012150607.76brmkwluibrqqzn@uno.localdomain>","References":"<20221012075925.3971538-1-chenghaoyang@google.com>\n\t<20221012075925.3971538-2-chenghaoyang@google.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20221012075925.3971538-2-chenghaoyang@google.com>","Subject":"Re: [libcamera-devel] [PATCH 1/1] Add fake pipeline handler","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Jacopo Mondi via libcamera-devel ","Reply-To":"Jacopo Mondi ","Cc":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25420,"web_url":"https://patchwork.libcamera.org/comment/25420/","msgid":"","date":"2022-10-14T10:36:48","subject":"Re: [libcamera-devel] [PATCH 1/1] Add fake pipeline handler","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"Thanks Jacopo!\n\nThe patch v2 is uploaded based on your comments.\n\nOn Wed, Oct 12, 2022 at 11:06 PM Jacopo Mondi wrote:\n\n> Hi Harvey,\n> I had the same thought as Kieran had, why not vimc ? I understan\n> your reasons behind it and the requirement to backport patches for it\n> to older kernel (btw, does it need anything more than CAP_IO_MC ?)\n>\n>\nIs it a great effort to backport patches to older kernel versions?\nRegarding CAP_IO_MC, I don't know :p. Anyone can help with this?\n\n\n> I have some general questions\n>\n> 1) What does this pipeline handler matches on ? It seems to me match()\n> always returns true, hence if the pipeline handler is compiled in, it\n> will always report one camera available ?\n>\n>\nI think it'll eventually depend on the configuration file provided by the\ntester.\nWe might come up with some fake media devices, without matching the real\nones. But I haven't made up my mind yet.\n\n\n> 2) What are the requirements for this pipeline handler ?\n>\n> Is it enough to test the Android layer to implement stub methods,\n> or should frames be generated somehow (I'm thinking at CTS frame\n> rate checks in example).\n>\n> Should it support multiple streams ? I guess it depends on the HW\n> level one wants to test in CTS, or should it mimik the capabilities\n> of a known device (ie IPU3 that can produce 2 YUV streams and one\n> RAW)\n>\n> I guess however this can be started as a skeleton and be populated\n> as required to complete CTS.\n>\n> Additional features like custom configurations (of cameras) and frames\nfrom\na video file will be added.\n\nIn CrOS, it helps applications to test usages in different (fake) hardware\nsetup,\nwhile it also helps libcamera test the Android adapter.\n\n\n> 3) Should all mentions of IPU3 related components be removed ?\n>\n> Yes\n\n\n> 4) Possible bikeshedding, but I wonder if \"dummy\" isn't better than\n> \"fake\"\n>\n> As we need some more features, like a custom configuration file/cameras\nand frames from a video file, I believe \"fake\" is still more appropriate.\n\n\n> On Wed, Oct 12, 2022 at 07:59:25AM +0000, Harvey Yang via libcamera-devel\n> wrote:\n> > ---\n> > meson_options.txt | 2 +-\n> > src/android/camera_capabilities.cpp | 1 +\n> > src/libcamera/pipeline/fake/fake.cpp | 518 ++++++++++++++++++++++++\n> > src/libcamera/pipeline/fake/meson.build | 3 +\n> > src/libcamera/pipeline_handler.cpp | 2 +-\n> > test/camera/camera_reconfigure.cpp | 2 +-\n> > 6 files changed, 525 insertions(+), 3 deletions(-)\n> > create mode 100644 src/libcamera/pipeline/fake/fake.cpp\n> > create mode 100644 src/libcamera/pipeline/fake/meson.build\n> >\n> > diff --git a/meson_options.txt b/meson_options.txt\n> > index f1d67808..f08dfc5f 100644\n> > --- a/meson_options.txt\n> > +++ b/meson_options.txt\n> > @@ -37,7 +37,7 @@ option('lc-compliance',\n> >\n> > option('pipelines',\n> > type : 'array',\n> > - choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple',\n> 'uvcvideo', 'vimc'],\n> > + choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple',\n> 'uvcvideo', 'vimc', 'fake'],\n> > description : 'Select which pipeline handlers to include')\n> >\n> > option('qcam',\n> > diff --git a/src/android/camera_capabilities.cpp\n> b/src/android/camera_capabilities.cpp\n> > index 64bd8dde..730ceafc 100644\n> > --- a/src/android/camera_capabilities.cpp\n> > +++ b/src/android/camera_capabilities.cpp\n> > @@ -1077,6 +1077,7 @@ int CameraCapabilities::initializeStaticMetadata()\n> > {\n> > const Span &rects =\n> >\n> properties.get(properties::PixelArrayActiveAreas).value_or(Span Rectangle>{});\n> > + // TODO: initialize at least one Rectangle as default.\n>\n> Unrelated, please drop\n>\n>\nDone.\n\n\n> > std::vector data{\n> > static_cast(rects[0].x),\n> > static_cast(rects[0].y),\n> > diff --git a/src/libcamera/pipeline/fake/fake.cpp\n> b/src/libcamera/pipeline/fake/fake.cpp\n> > new file mode 100644\n> > index 00000000..518de6aa\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/fake/fake.cpp\n> > @@ -0,0 +1,518 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * ipu3.cpp - Pipeline handler for Intel Fake\n> > + */\n>\n> Please update year and remove mentions of Intel ?\n>\n>\nDone.\n\n\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n>\n> Seems like you're also using std::set\n>\n>\nDone.\n\n\n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \n>\n> all internal headers \"libcamera/internal/something.h\" should include\n> , so if you include internal/camera.h you\n> probably can remove this\n>\n>\nDone.\n\n\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \"libcamera/internal/camera.h\"\n> > +#include \"libcamera/internal/camera_lens.h\"\n> > +#include \"libcamera/internal/camera_sensor.h\"\n> > +#include \"libcamera/internal/delayed_controls.h\"\n>\n> The three above are not used it seems\n>\n>\nDone.\n\n\n> > +#include \"libcamera/internal/device_enumerator.h\"\n> > +#include \"libcamera/internal/framebuffer.h\"\n> > +#include \"libcamera/internal/media_device.h\"\n> > +#include \"libcamera/internal/pipeline_handler.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DEFINE_CATEGORY(Fake)\n> > +\n> > +uint64_t CurrentTimestamp()\n> > +{\n> > + struct timespec ts;\n> > + if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) {\n> > + LOG(Fake, Error) << \"Get clock time fails\";\n> > + return 0;\n> > + }\n> > +\n> > + return ts.tv_sec * 1'000'000'000LL + ts.tv_nsec;\n> > +}\n> > +\n> > +static const ControlInfoMap::Map FakeControls = {\n>\n> Missing #include ?\n>\n>\nDone.\n\n> + { &controls::draft::PipelineDepth, ControlInfo(2, 3) },\n> > +};\n> > +\n> > +class FakeCameraData : public Camera::Private\n> > +{\n> > +public:\n> > + struct Resolution {\n> > + Size size;\n> > + std::vector frame_rates;\n> > + std::vector formats;\n>\n> I would rather use libcamera::formats to verify the Android HAL does\n> the translation right.\n>\n> I believe you mean libcamera::PixelFormat. Done.\n\n\n> > + };\n> > +\n> > + FakeCameraData(PipelineHandler *pipe)\n> > + : Camera::Private(pipe), supportsFlips_(false)\n> > + {\n> > + }\n> > +\n> > + std::vector supported_resolutions_;\n>\n> supportedResolutions_\n>\n> Done.\n\n> > +\n> > + Stream outStream_;\n> > + Stream vfStream_;\n> > + Stream rawStream_;\n> > +\n> > + // TODO: Check if we should support.\n> > + bool supportsFlips_;\n> > + Transform rotationTransform_;\n>\n> For sake of simplicity you can remoev flip/rotation support from the\n> first skeleton\n>\n>\nDone.\n\n\n> > +\n> > + // TODO: remove\n>\n> Please :)\n>\n> Done.\n\n> > + /* Requests for which no buffer has been queued to the CIO2 device\n> yet. */\n> > + std::queue pendingRequests_;\n> > + /* Requests queued to the CIO2 device but not yet processed by the\n> ImgU. */\n> > + std::queue processingRequests_;\n> > +\n> > + // ControlInfoMap ipaControls_;\n>\n> This as well\n>\n> Done.\n\n> > + bool started_ = false;\n> > +};\n> > +\n> > +class FakeCameraConfiguration : public CameraConfiguration\n> > +{\n> > +public:\n> > + static constexpr unsigned int kBufferCount = 4; // 4~6\n> > + static constexpr unsigned int kMaxStreams = 3;\n> > +\n> > + FakeCameraConfiguration(FakeCameraData *data);\n> > +\n> > + Status validate() override;\n> > +\n> > +private:\n> > + /*\n> > + * The FakeCameraData instance is guaranteed to be valid as long\n> as the\n> > + * corresponding Camera instance is valid. In order to borrow a\n> > + * reference to the camera data, store a new reference to the\n> camera.\n> > + */\n>\n> This comes from the IPU3 pipeline handler, and I wonder if it still\n> applies there or it should be removed.\n>\n>\nI believe it's still needed in |validate()| to set streams, right?\n\n\n> > + const FakeCameraData *data_;\n> > +};\n> > +\n> > +class PipelineHandlerFake : public PipelineHandler\n> > +{\n> > +public:\n> > + static constexpr unsigned int V4L2_CID_Fake_PIPE_MODE = 0x009819c1;\n>\n> Unused, please drop\n>\n> Done.\n\n> > + static constexpr Size kViewfinderSize{ 1280, 720 };\n> > +\n> > + enum FakePipeModes {\n> > + FakePipeModeVideo = 0,\n> > + FakePipeModeStillCapture = 1,\n> > + };\n>\n> ditto\n>\n>\nDone.\n\n\n> > +\n> > + PipelineHandlerFake(CameraManager *manager);\n> > +\n> > + CameraConfiguration *generateConfiguration(Camera *camera,\n> > + const StreamRoles &roles) override;\n>\n> Align to open (\n>\n> Done.\n\n> > + int configure(Camera *camera, CameraConfiguration *config)\n> override;\n> > +\n> > + int exportFrameBuffers(Camera *camera, Stream *stream,\n> > + std::vector>\n> *buffers) override;\n> > +\n> > + int start(Camera *camera, const ControlList *controls) override;\n> > + void stopDevice(Camera *camera) override;\n> > +\n> > + int queueRequestDevice(Camera *camera, Request *request) override;\n> > +\n> > + bool match(DeviceEnumerator *enumerator) override;\n> > +\n> > +private:\n> > + FakeCameraData *cameraData(Camera *camera)\n> > + {\n> > + return static_cast(camera->_d());\n> > + }\n> > +\n> > + int registerCameras();\n> > +\n> > + static bool registered_;\n> > +};\n> > +\n> > +bool PipelineHandlerFake::registered_ = false;\n>\n> What purpose does this serve ? Do you get multiple calls to match()\n> so that you need to keep a flag ?\n>\n> Yes, in `src/libcamera/camera_manager.cpp`, it intentionally calls match()\nmultiple times\nuntil it returns false (which means no devices matched). Therefore, this is\nthe hack as\nthe fake media devices haven't been created yet.\n\n\n> > +\n> > +FakeCameraConfiguration::FakeCameraConfiguration(FakeCameraData *data)\n> > + : CameraConfiguration()\n> > +{\n> > + data_ = data;\n> > +}\n> > +\n> > +CameraConfiguration::Status FakeCameraConfiguration::validate()\n> > +{\n> > + Status status = Valid;\n> > +\n> > + if (config_.empty())\n> > + return Invalid;\n> > +\n> > + // Transform combined = transform * data_->rotationTransform_;\n> > +\n> > + // /*\n> > + // * We combine the platform and user transform, but must \"adjust\n> away\"\n> > + // * any combined result that includes a transposition, as we\n> can't do\n> > + // * those. In this case, flipping only the transpose bit is\n> helpful to\n> > + // * applications - they either get the transform they requested,\n> or have\n> > + // * to do a simple transpose themselves (they don't have to\n> worry about\n> > + // * the other possible cases).\n> > + // */\n> > + // if (!!(combined & Transform::Transpose)) {\n> > + // /*\n> > + // * Flipping the transpose bit in \"transform\" flips it in\n> the\n> > + // * combined result too (as it's the last thing that\n> happens),\n> > + // * which is of course clearing it.\n> > + // */\n> > + // transform ^= Transform::Transpose;\n> > + // combined &= ~Transform::Transpose;\n> > + // status = Adjusted;\n> > + // }\n> > +\n> > + // /*\n> > + // * We also check if the sensor doesn't do h/vflips at all, in\n> which\n> > + // * case we clear them, and the application will have to do\n> everything.\n> > + // */\n> > + // if (!data_->supportsFlips_ && !!combined) {\n> > + // /*\n> > + // * If the sensor can do no transforms, then combined must\n> be\n> > + // * changed to the identity. The only user transform that\n> gives\n> > + // * rise to this is the inverse of the rotation. (Recall\n> that\n> > + // * combined = transform * rotationTransform.)\n> > + // */\n> > + // transform = -data_->rotationTransform_;\n> > + // combined = Transform::Identity;\n> > + // status = Adjusted;\n> > + // }\n> > +\n> > + /*\n> > + * Store the final combined transform that configure() will need to\n> > + * apply to the sensor to save us working it out again.\n> > + */\n> > + // combinedTransform_ = combined;\n>\n> Please drop commented out code :0\n>\n> > +\n> > + /* Cap the number of entries to the available streams. */\n> > + if (config_.size() > kMaxStreams) {\n> > + config_.resize(kMaxStreams);\n> > + status = Adjusted;\n> > + }\n> > +\n> > + /*\n> > + * Validate the requested stream configuration and select the\n> sensor\n> > + * format by collecting the maximum RAW stream width and height and\n> > + * picking the closest larger match.\n> > + *\n> > + * If no RAW stream is requested use the one of the largest YUV\n> stream,\n> > + * plus margin pixels for the IF and BDS rectangle to downscale.\n> > + *\n> > + * \\todo Clarify the IF and BDS margins requirements.\n>\n> Please drop IPU3 related stuff\n>\n> Will do. I'll update the number of supported streams as well.\n\n> > + */\n> > + unsigned int rawCount = 0;\n> > + unsigned int yuvCount = 0;\n> > + Size rawRequirement;\n> > + Size maxYuvSize;\n> > + Size rawSize;\n> > +\n> > + for (const StreamConfiguration &cfg : config_) {\n> > + const PixelFormatInfo &info =\n> PixelFormatInfo::info(cfg.pixelFormat);\n> > +\n> > + if (info.colourEncoding ==\n> PixelFormatInfo::ColourEncodingRAW) {\n> > + rawCount++;\n> > + rawSize = std::max(rawSize, cfg.size);\n> > + } else {\n> > + yuvCount++;\n> > + maxYuvSize = std::max(maxYuvSize, cfg.size);\n> > + rawRequirement.expandTo(cfg.size);\n> > + }\n> > + }\n> > +\n> > + // TODO: Base on number of cameras?\n>\n> Well, should this pipeline register more than one camera ?\n> And anyway, the number of streams is per-camera so the number of\n> cameras should be not relevant here\n>\n>\nYes, and yes. The configuration file should specify the number of supported\nstreams.\n\n\n> > + if (rawCount > 1 || yuvCount > 2) {\n> > + LOG(Fake, Debug) << \"Camera configuration not supported\";\n> > + return Invalid;\n> > + }\n> > +\n> > + /*\n> > + * Generate raw configuration from CIO2.\n> > + *\n> > + * The output YUV streams will be limited in size to the maximum\n> frame\n> > + * size requested for the RAW stream, if present.\n> > + *\n> > + * If no raw stream is requested, generate a size from the largest\n> YUV\n> > + * stream, aligned to the ImgU constraints and bound\n> > + * by the sensor's maximum resolution. See\n> > + * https://bugs.libcamera.org/show_bug.cgi?id=32\n> > + */\n> > + // TODO\n> > + if (rawSize.isNull())\n> > + rawSize = rawRequirement;\n>\n> All of these serves on IPU3 to generate a size suitable for the CIO2\n> and the sensor. You can remove it.\n>\n>\nRight, thanks!\n\n> +\n> > + /*\n> > + * Adjust the configurations if needed and assign streams while\n> > + * iterating them.\n> > + */\n> > + bool mainOutputAvailable = true;\n> > + for (unsigned int i = 0; i < config_.size(); ++i) {\n> > + const PixelFormatInfo &info =\n> PixelFormatInfo::info(config_[i].pixelFormat);\n> > + const StreamConfiguration originalCfg = config_[i];\n> > + StreamConfiguration *cfg = &config_[i];\n> > +\n> > + LOG(Fake, Debug) << \"Validating stream: \" <<\n> config_[i].toString();\n> > +\n> > + if (info.colourEncoding ==\n> PixelFormatInfo::ColourEncodingRAW) {\n> > + /* Initialize the RAW stream with the CIO2\n> configuration. */\n> > + cfg->size = rawSize;\n> > + // TODO: check\n> > + cfg->pixelFormat = formats::SBGGR10_IPU3;\n> > + cfg->bufferCount =\n> FakeCameraConfiguration::kBufferCount;\n> > + cfg->stride = info.stride(cfg->size.width, 0, 64);\n> > + cfg->frameSize = info.frameSize(cfg->size, 64);\n> > + cfg->setStream(const_cast *>(&data_->rawStream_));\n> > +\n> > + LOG(Fake, Debug) << \"Assigned \" << cfg->toString()\n> > + << \" to the raw stream\";\n> > + } else {\n> > + /* Assign and configure the main and viewfinder\n> outputs. */\n> > +\n> > + cfg->pixelFormat = formats::NV12;\n> > + cfg->bufferCount = kBufferCount;\n> > + cfg->stride = info.stride(cfg->size.width, 0, 1);\n> > + cfg->frameSize = info.frameSize(cfg->size, 1);\n> > +\n> > + /*\n> > + * Use the main output stream in case only one\n> stream is\n> > + * requested or if the current configuration is\n> the one\n> > + * with the maximum YUV output size.\n> > + */\n> > + if (mainOutputAvailable &&\n> > + (originalCfg.size == maxYuvSize || yuvCount ==\n> 1)) {\n> > + cfg->setStream(const_cast *>(&data_->outStream_));\n> > + mainOutputAvailable = false;\n> > +\n> > + LOG(Fake, Debug) << \"Assigned \" <<\n> cfg->toString()\n> > + << \" to the main output\";\n> > + } else {\n> > + cfg->setStream(const_cast *>(&data_->vfStream_));\n> > +\n> > + LOG(Fake, Debug) << \"Assigned \" <<\n> cfg->toString()\n> > + << \" to the viewfinder\n> output\";\n> > + }\n> > + }\n>\n> Again I'm not sure how much of the IPU3 should this pipeline mimick,\n> or it should establish requirements and constraints (ie max 2 YUV\n> streams of max size width x height, and 1 RAW stream in format\n> V4L2_PIX_FMT_..)\n>\n>\nExactly. Will do.\n\n\n> > +\n> > + if (cfg->pixelFormat != originalCfg.pixelFormat ||\n> > + cfg->size != originalCfg.size) {\n> > + LOG(Fake, Debug)\n> > + << \"Stream \" << i << \" configuration\n> adjusted to \"\n> > + << cfg->toString();\n> > + status = Adjusted;\n> > + }\n> > + }\n> > +\n> > + return status;\n> > +}\n> > +\n> > +PipelineHandlerFake::PipelineHandlerFake(CameraManager *manager)\n> > + : PipelineHandler(manager)\n> > +{\n> > + // TODO: read the fake hal spec file.\n>\n> If there's a design document with requirements, can you share it ?\n>\n>\nWill do.\n\n\n> > +}\n> > +\n> > +CameraConfiguration *PipelineHandlerFake::generateConfiguration(Camera\n> *camera,\n> > + const\n> StreamRoles &roles)\n> > +{\n> > + FakeCameraData *data = cameraData(camera);\n> > + FakeCameraConfiguration *config = new\n> FakeCameraConfiguration(data);\n> > +\n> > + if (roles.empty())\n> > + return config;\n> > +\n> > + Size minSize, sensorResolution;\n> > + for (const auto& resolution : data->supported_resolutions_) {\n> > + if (minSize.isNull() || minSize > resolution.size)\n> > + minSize = resolution.size;\n> > +\n> > + sensorResolution = std::max(sensorResolution,\n> resolution.size);\n> > + }\n> > +\n>\n> Those are hard-coded values, I think you can reuse them here.\n> Unless the idea is to make this information come from a configuration\n> file or something similar.\n>\n> Yes, supportedResolutions_ should eventually come from a configuration\nfile, provided by the tester.\n\n\n> > + for (const StreamRole role : roles) {\n> > + std::map>\n> streamFormats;\n> > + unsigned int bufferCount;\n> > + PixelFormat pixelFormat;\n> > + Size size;\n> > +\n> > + switch (role) {\n> > + case StreamRole::StillCapture:\n> > + size = sensorResolution;\n> > + pixelFormat = formats::NV12;\n> > + bufferCount =\n> FakeCameraConfiguration::kBufferCount;\n> > + streamFormats[pixelFormat] = { { minSize, size } };\n> > +\n> > + break;\n> > +\n> > + case StreamRole::Raw: {\n> > + // TODO: check\n> > + pixelFormat = formats::SBGGR10_IPU3;\n> > + size = sensorResolution;\n> > + bufferCount =\n> FakeCameraConfiguration::kBufferCount;\n> > + streamFormats[pixelFormat] = { { minSize, size } };\n> > +\n> > + break;\n> > + }\n> > +\n> > + case StreamRole::Viewfinder:\n> > + case StreamRole::VideoRecording: {\n> > + /*\n> > + * Default viewfinder and videorecording to\n> 1280x720,\n> > + * capped to the maximum sensor resolution and\n> aligned\n> > + * to the ImgU output constraints.\n> > + */\n> > + size = sensorResolution;\n> > + pixelFormat = formats::NV12;\n> > + bufferCount =\n> FakeCameraConfiguration::kBufferCount;\n> > + streamFormats[pixelFormat] = { { minSize, size } };\n> > +\n> > + break;\n> > + }\n> > +\n> > + default:\n> > + LOG(Fake, Error)\n> > + << \"Requested stream role not supported: \"\n> << role;\n> > + delete config;\n> > + return nullptr;\n> > + }\n> > +\n> > + StreamFormats formats(streamFormats);\n> > + StreamConfiguration cfg(formats);\n> > + cfg.size = size;\n> > + cfg.pixelFormat = pixelFormat;\n> > + cfg.bufferCount = bufferCount;\n> > + config->addConfiguration(cfg);\n> > + }\n> > +\n> > + if (config->validate() == CameraConfiguration::Invalid)\n> > + return {};\n> > +\n> > + return config;\n> > +}\n> > +\n> > +int PipelineHandlerFake::configure(Camera *camera, CameraConfiguration\n> *c)\n> > +{\n> > + if (camera || c)\n> > + return 0;\n> > + return 0;\n> > +}\n> > +\n> > +int PipelineHandlerFake::exportFrameBuffers(Camera *camera, Stream\n> *stream,\n> > +\n> std::vector> *buffers)\n> > +{\n> > + // Assume it's never called.\n> > + LOG(Fake, Fatal) << \"exportFrameBuffers should never be called\";\n> > + if (camera || stream || buffers)\n> > + return -EINVAL;\n> > + return -EINVAL;\n> > +}\n> > +\n> > +int PipelineHandlerFake::start(Camera *camera, [[maybe_unused]] const\n> ControlList *controls)\n> > +{\n> > + FakeCameraData *data = cameraData(camera);\n> > + data->started_ = true;\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +void PipelineHandlerFake::stopDevice(Camera *camera)\n> > +{\n> > + FakeCameraData *data = cameraData(camera);\n> > +\n> > + data->started_ = false;\n> > +}\n> > +\n> > +int PipelineHandlerFake::queueRequestDevice(Camera *camera, Request\n> *request)\n> > +{\n> > + if (!camera)\n> > + return -EINVAL;\n>\n> Can this happen ?\n>\n> Actually it's a hack that CrOS compiler will fail if there's any unused\nargument...\nI'll try to clean it up later.\n\n\n> > +\n> > + for (auto it : request->buffers())\n> > + completeBuffer(request, it.second);\n> > +\n> > + // TODO: request.metadata()\n> > + request->metadata().set(controls::SensorTimestamp,\n> CurrentTimestamp());\n> > + completeRequest(request);\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +bool PipelineHandlerFake::match(DeviceEnumerator *enumerator)\n> > +{\n> > + // TODO: exhaust all devices in |enumerator|.\n> > + if (!enumerator)\n> > + LOG(Fake, Info) << \"Invalid enumerator\";\n>\n> Can this happen ?\n>\n> ditto\n\n> > +\n> > + if (registered_)\n> > + return false;\n> > +\n> > + registered_ = true;\n> > + return registerCameras() == 0;\n> > +}\n> > +\n> > +/**\n> > + * \\brief Initialise ImgU and CIO2 devices associated with cameras\n> > + *\n> > + * Initialise the two ImgU instances and create cameras with an\n> associated\n> > + * CIO2 device instance.\n> > + *\n> > + * \\return 0 on success or a negative error code for error or if no\n> camera\n> > + * has been created\n> > + * \\retval -ENODEV no camera has been created\n> > + */\n> > +int PipelineHandlerFake::registerCameras()\n> > +{\n> > + std::unique_ptr data =\n> > + std::make_unique(this);\n> > + std::set streams = {\n> > + &data->outStream_,\n> > + &data->vfStream_,\n> > + &data->rawStream_,\n> > + };\n> > +\n> > + // TODO: Read from config or from IPC.\n> > + // TODO: Check with Han-lin: Can this function be called more than\n> once?\n> > + data->supported_resolutions_.resize(2);\n> > + data->supported_resolutions_[0].size = Size(1920, 1080);\n> > + data->supported_resolutions_[0].frame_rates.push_back(30);\n> > + data->supported_resolutions_[0].formats.push_back(\"YCbCr_420_888\");\n> > + data->supported_resolutions_[0].formats.push_back(\"BLOB\");\n> > + data->supported_resolutions_[1].size = Size(1280, 720);\n> > + data->supported_resolutions_[1].frame_rates.push_back(30);\n> > + data->supported_resolutions_[1].frame_rates.push_back(60);\n> > + data->supported_resolutions_[1].formats.push_back(\"YCbCr_420_888\");\n> > + data->supported_resolutions_[1].formats.push_back(\"BLOB\");\n> > +\n> > + // TODO: Assign different locations for different cameras based on\n> config.\n> > + data->properties_.set(properties::Location,\n> properties::CameraLocationFront);\n> > + data->properties_.set(properties::PixelArrayActiveAreas, {\n> Rectangle(Size(1920, 1080)) });\n> > +\n> > + // TODO: Set FrameDurationLimits based on config.\n> > + ControlInfoMap::Map controls{};\n> > + int64_t min_frame_duration = 30, max_frame_duration = 60;\n> > + controls[&controls::FrameDurationLimits] =\n> ControlInfo(min_frame_duration, max_frame_duration);\n> > + data->controlInfo_ = ControlInfoMap(std::move(controls),\n> controls::controls);\n>\n> FakeControls is ignored\n>\n> Right. Use it to initialize |controls| here.\n\n\n> > +\n> > + std::shared_ptr camera =\n> > + Camera::create(std::move(data), \"\\\\_SB_.PCI0.I2C4.CAM1\" /*\n> cameraId */, streams);\n> > +\n> > + registerCamera(std::move(camera));\n> > +\n> > + return 0;\n> > +}\n> > +\n> > +REGISTER_PIPELINE_HANDLER(PipelineHandlerFake)\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/fake/meson.build\n> b/src/libcamera/pipeline/fake/meson.build\n> > new file mode 100644\n> > index 00000000..13980b4a\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/fake/meson.build\n> > @@ -0,0 +1,3 @@\n> > +# SPDX-License-Identifier: CC0-1.0\n> > +\n> > +libcamera_sources += files([ 'fake.cpp' ])\n> > diff --git a/src/libcamera/pipeline_handler.cpp\n> b/src/libcamera/pipeline_handler.cpp\n> > index e5cb751c..4261154d 100644\n> > --- a/src/libcamera/pipeline_handler.cpp\n> > +++ b/src/libcamera/pipeline_handler.cpp\n> > @@ -536,7 +536,7 @@ void\n> PipelineHandler::registerCamera(std::shared_ptr camera)\n> > cameras_.push_back(camera);\n> >\n> > if (mediaDevices_.empty())\n> > - LOG(Pipeline, Fatal)\n> > + LOG(Pipeline, Error)\n>\n> I don't think we want that, you should probably open code the\n>\n> manager_->addCamera(std::move(camera), devnums);\n>\n> call, with an empty list of devnums, which should be fine for this use\n> case.\n>\n> The PipelineHandler::cameras_ vector will remain unpopulated, but\n> since it is used only in case of a media device disconnection (afaict)\n> it should be fine in your case\n>\n>\nAh you're right. Thanks!\n\n\n>\n> > << \"Registering camera with no media devices!\";\n> >\n> > /*\n> > diff --git a/test/camera/camera_reconfigure.cpp\n> b/test/camera/camera_reconfigure.cpp\n> > index d12e2413..06c87730 100644\n> > --- a/test/camera/camera_reconfigure.cpp\n> > +++ b/test/camera/camera_reconfigure.cpp\n> > @@ -98,7 +98,7 @@ private:\n> > return TestFail;\n> > }\n> >\n> > - requests_.push_back(move(request));\n> > + requests_.push_back(std::move(request));\n>\n> Unrelated and possibly not necessary, as the test has\n> using namespace std;\n>\n>\nYeah... it's also a hack that it's required in the CrOS compiler... It\ndoesn't\naccept |move|...\n\n\n> Looking forward to seeing this skeleton getting populated, it will help\n> testing the Android layer!\n>\n> Thanks\n> j\n>\n> > }\n> >\n> > camera_->requestCompleted.connect(this,\n> &CameraReconfigure::requestComplete);\n> > --\n> > 2.38.0.rc1.362.ged0d419d3c-goog\n> >\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 6C6EFBD16B\n\tfor ;\n\tFri, 14 Oct 2022 10:37:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 195EF62D8E;\n\tFri, 14 Oct 2022 12:37:02 +0200 (CEST)","from mail-lj1-x233.google.com (mail-lj1-x233.google.com\n\t[IPv6:2a00:1450:4864:20::233])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3DF3062D8E\n\tfor ;\n\tFri, 14 Oct 2022 12:37:00 +0200 (CEST)","by mail-lj1-x233.google.com with SMTP id bs14so5482719ljb.9\n\tfor ;\n\tFri, 14 Oct 2022 03:37:00 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1665743822;\n\tbh=vgfX+4kJH0vmObPiLFCfYXri3SChhxGqVAa4hqBcCJc=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=J/SG1q/FkpdxaoZSBsbf00HhkkofEL0m7N26mRpBFEbPXzxzq4KH5Zmh57QMCVGfJ\n\tiaNKY/7AVA7USuK7WQXsSxGH4cZCQxu1zOGxUZn0cxnB9vLoR4FoBvnmo8o0EDMgrJ\n\tO+fXS3HbBzBasSua2EsH5c1vxJ9XxT1lbmcQcivUajJAplloDPTfUsf34E4qMAJb+N\n\t4hR8uFstnJGB0K7KodYynh3ffTidkq9nF76Pw04MgULGcboypW2gBghLKDNv+BZnk0\n\tMUzi2UREBL9gkBObSt5cTVhQ57bCxYBcSPlIWWF6eiCaZ+mUp2cxd58QmhRRyMyCjk\n\t0VLqOTKXkFTMA==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=287w8P4WF+dT3wWyNtD2sMcLVOmxTVB0b6hnYhr51ZU=;\n\tb=kh26PmHRV8PJhh3dgigGiG1Ikyrf53xwTNMdyLsTR6ysKbvU2DmIuf55PUYFK/mzdm\n\thFav4VtQYbMtJllEcToQgBX762itc5fI59JTeqn8FswVtvdNywmHhh0gFxMaZye78D2A\n\t6KdAu1mh1iBAm5wUDQ1+ZsZQat6MINjRTqC6c="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"kh26PmHR\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=287w8P4WF+dT3wWyNtD2sMcLVOmxTVB0b6hnYhr51ZU=;\n\tb=ia9MyavYfOnBT6LRl5q9IIa+Y4+EC0U6iXavkCoT0juCt8ZCUQHB31z7feXMKONsuf\n\tlqUx/Wi8oQ9JODD2csT1mUSEKmHbxTHaPKLaj5Cue24iqrXGumAGps2wBJo8doDA5lvp\n\tDf8LAhUztAPwSOTA6gxmRaWcLPoM03pHwj/kaxczMalSsCcixjQ7nwWsxxjo7y4tfHoQ\n\tfhWzUsjicBMNIgar+mLEsnBumzAZ35bPQpla3DWNpwal7u4sPghuZxlxzlEhPmuO0nud\n\tJO0q52ouKkrf+WcwXig7gee+N7Wndr0I+DAjYrLqh9CtgjQj6sb8T0UvRCym16cSuaIl\n\tOR2A==","X-Gm-Message-State":"ACrzQf3+Lxy7M5lxPMBuAVwIDgQ0PJZpXLcitIZorkGXYQEsrfoXIEll\n\tk2OHPdGhIMya8fHXxA0wNw4dZDfVDOF30B3BYTwL7g==","X-Google-Smtp-Source":"AMsMyM7LzuQOqOV90QI0Yx3VvBKkHd81+80SpoTAn1MjDK9+kIxR6MwO7i/oa0kMvMlKVVSgCwVruByN++l9pSJVLLo=","X-Received":"by 2002:a2e:1f01:0:b0:25f:ea3a:4ef0 with SMTP id\n\tf1-20020a2e1f01000000b0025fea3a4ef0mr1506457ljf.330.1665743819392;\n\tFri, 14 Oct 2022 03:36:59 -0700 (PDT)","MIME-Version":"1.0","References":"<20221012075925.3971538-1-chenghaoyang@google.com>\n\t<20221012075925.3971538-2-chenghaoyang@google.com>\n\t<20221012150607.76brmkwluibrqqzn@uno.localdomain>","In-Reply-To":"<20221012150607.76brmkwluibrqqzn@uno.localdomain>","Date":"Fri, 14 Oct 2022 18:36:48 +0800","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"000000000000a47acc05eafc34be\"","Subject":"Re: [libcamera-devel] [PATCH 1/1] Add fake pipeline handler","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Cheng-Hao Yang via libcamera-devel\n\t","Reply-To":"Cheng-Hao Yang ","Cc":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":25524,"web_url":"https://patchwork.libcamera.org/comment/25524/","msgid":"","date":"2022-10-21T22:38:10","subject":"Re: [libcamera-devel] [PATCH 1/1] Add fake pipeline handler","submitter":{"id":117,"url":"https://patchwork.libcamera.org/api/people/117/","name":"Cheng-Hao Yang","email":"chenghaoyang@chromium.org"},"content":"On Fri, Oct 14, 2022 at 7:36 PM Cheng-Hao Yang \nwrote:\n\n> Thanks Jacopo!\n>\n> The patch v2 is uploaded based on your comments.\n>\n> On Wed, Oct 12, 2022 at 11:06 PM Jacopo Mondi wrote:\n>\n>> Hi Harvey,\n>> I had the same thought as Kieran had, why not vimc ? I understan\n>> your reasons behind it and the requirement to backport patches for it\n>> to older kernel (btw, does it need anything more than CAP_IO_MC ?)\n>>\n>>\n> Is it a great effort to backport patches to older kernel versions?\n> Regarding CAP_IO_MC, I don't know :p. Anyone can help with this?\n>\n>\n>> I have some general questions\n>>\n>> 1) What does this pipeline handler matches on ? It seems to me match()\n>> always returns true, hence if the pipeline handler is compiled in, it\n>> will always report one camera available ?\n>>\n>>\n> I think it'll eventually depend on the configuration file provided by the\n> tester.\n> We might come up with some fake media devices, without matching the real\n> ones. But I haven't made up my mind yet.\n>\n>\n>> 2) What are the requirements for this pipeline handler ?\n>>\n>> Is it enough to test the Android layer to implement stub methods,\n>> or should frames be generated somehow (I'm thinking at CTS frame\n>> rate checks in example).\n>>\n>> Should it support multiple streams ? I guess it depends on the HW\n>> level one wants to test in CTS, or should it mimik the capabilities\n>> of a known device (ie IPU3 that can produce 2 YUV streams and one\n>> RAW)\n>>\n>> I guess however this can be started as a skeleton and be populated\n>> as required to complete CTS.\n>>\n>> Additional features like custom configurations (of cameras) and frames\n> from\n> a video file will be added.\n>\n> In CrOS, it helps applications to test usages in different (fake) hardware\n> setup,\n> while it also helps libcamera test the Android adapter.\n>\n>\n>> 3) Should all mentions of IPU3 related components be removed ?\n>>\n>> Yes\n>\n>\n>> 4) Possible bikeshedding, but I wonder if \"dummy\" isn't better than\n>> \"fake\"\n>>\n>> As we need some more features, like a custom configuration file/cameras\n> and frames from a video file, I believe \"fake\" is still more appropriate.\n>\n>\n>> On Wed, Oct 12, 2022 at 07:59:25AM +0000, Harvey Yang via libcamera-devel\n>> wrote:\n>> > ---\n>> > meson_options.txt | 2 +-\n>> > src/android/camera_capabilities.cpp | 1 +\n>> > src/libcamera/pipeline/fake/fake.cpp | 518 ++++++++++++++++++++++++\n>> > src/libcamera/pipeline/fake/meson.build | 3 +\n>> > src/libcamera/pipeline_handler.cpp | 2 +-\n>> > test/camera/camera_reconfigure.cpp | 2 +-\n>> > 6 files changed, 525 insertions(+), 3 deletions(-)\n>> > create mode 100644 src/libcamera/pipeline/fake/fake.cpp\n>> > create mode 100644 src/libcamera/pipeline/fake/meson.build\n>> >\n>> > diff --git a/meson_options.txt b/meson_options.txt\n>> > index f1d67808..f08dfc5f 100644\n>> > --- a/meson_options.txt\n>> > +++ b/meson_options.txt\n>> > @@ -37,7 +37,7 @@ option('lc-compliance',\n>> >\n>> > option('pipelines',\n>> > type : 'array',\n>> > - choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple',\n>> 'uvcvideo', 'vimc'],\n>> > + choices : ['ipu3', 'raspberrypi', 'rkisp1', 'simple',\n>> 'uvcvideo', 'vimc', 'fake'],\n>> > description : 'Select which pipeline handlers to include')\n>> >\n>> > option('qcam',\n>> > diff --git a/src/android/camera_capabilities.cpp\n>> b/src/android/camera_capabilities.cpp\n>> > index 64bd8dde..730ceafc 100644\n>> > --- a/src/android/camera_capabilities.cpp\n>> > +++ b/src/android/camera_capabilities.cpp\n>> > @@ -1077,6 +1077,7 @@ int CameraCapabilities::initializeStaticMetadata()\n>> > {\n>> > const Span &rects =\n>> >\n>> properties.get(properties::PixelArrayActiveAreas).value_or(Span> Rectangle>{});\n>> > + // TODO: initialize at least one Rectangle as default.\n>>\n>> Unrelated, please drop\n>>\n>>\n> Done.\n>\n>\n>> > std::vector data{\n>> > static_cast(rects[0].x),\n>> > static_cast(rects[0].y),\n>> > diff --git a/src/libcamera/pipeline/fake/fake.cpp\n>> b/src/libcamera/pipeline/fake/fake.cpp\n>> > new file mode 100644\n>> > index 00000000..518de6aa\n>> > --- /dev/null\n>> > +++ b/src/libcamera/pipeline/fake/fake.cpp\n>> > @@ -0,0 +1,518 @@\n>> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n>> > +/*\n>> > + * Copyright (C) 2019, Google Inc.\n>> > + *\n>> > + * ipu3.cpp - Pipeline handler for Intel Fake\n>> > + */\n>>\n>> Please update year and remove mentions of Intel ?\n>>\n>>\n> Done.\n>\n>\n>> > +\n>> > +#include \n>> > +#include \n>> > +#include \n>> > +#include \n>> > +#include \n>>\n>> Seems like you're also using std::set\n>>\n>>\n> Done.\n>\n>\n>> > +\n>> > +#include \n>> > +#include \n>> > +\n>> > +#include \n>>\n>> all internal headers \"libcamera/internal/something.h\" should include\n>> , so if you include internal/camera.h you\n>> probably can remove this\n>>\n>>\n> Done.\n>\n>\n>> > +#include \n>> > +#include \n>> > +#include \n>> > +#include \n>> > +#include \n>> > +\n>> > +#include \"libcamera/internal/camera.h\"\n>> > +#include \"libcamera/internal/camera_lens.h\"\n>> > +#include \"libcamera/internal/camera_sensor.h\"\n>> > +#include \"libcamera/internal/delayed_controls.h\"\n>>\n>> The three above are not used it seems\n>>\n>>\n> Done.\n>\n>\n>> > +#include \"libcamera/internal/device_enumerator.h\"\n>> > +#include \"libcamera/internal/framebuffer.h\"\n>> > +#include \"libcamera/internal/media_device.h\"\n>> > +#include \"libcamera/internal/pipeline_handler.h\"\n>> > +\n>> > +namespace libcamera {\n>> > +\n>> > +LOG_DEFINE_CATEGORY(Fake)\n>> > +\n>> > +uint64_t CurrentTimestamp()\n>> > +{\n>> > + struct timespec ts;\n>> > + if (clock_gettime(CLOCK_MONOTONIC, &ts) < 0) {\n>> > + LOG(Fake, Error) << \"Get clock time fails\";\n>> > + return 0;\n>> > + }\n>> > +\n>> > + return ts.tv_sec * 1'000'000'000LL + ts.tv_nsec;\n>> > +}\n>> > +\n>> > +static const ControlInfoMap::Map FakeControls = {\n>>\n>> Missing #include ?\n>>\n>>\n> Done.\n>\n> > + { &controls::draft::PipelineDepth, ControlInfo(2, 3) },\n>> > +};\n>> > +\n>> > +class FakeCameraData : public Camera::Private\n>> > +{\n>> > +public:\n>> > + struct Resolution {\n>> > + Size size;\n>> > + std::vector frame_rates;\n>> > + std::vector formats;\n>>\n>> I would rather use libcamera::formats to verify the Android HAL does\n>> the translation right.\n>>\n>> I believe you mean libcamera::PixelFormat. Done.\n>\n>\n>> > + };\n>> > +\n>> > + FakeCameraData(PipelineHandler *pipe)\n>> > + : Camera::Private(pipe), supportsFlips_(false)\n>> > + {\n>> > + }\n>> > +\n>> > + std::vector supported_resolutions_;\n>>\n>> supportedResolutions_\n>>\n>> Done.\n>\n>> > +\n>> > + Stream outStream_;\n>> > + Stream vfStream_;\n>> > + Stream rawStream_;\n>> > +\n>> > + // TODO: Check if we should support.\n>> > + bool supportsFlips_;\n>> > + Transform rotationTransform_;\n>>\n>> For sake of simplicity you can remoev flip/rotation support from the\n>> first skeleton\n>>\n>>\n> Done.\n>\n>\n>> > +\n>> > + // TODO: remove\n>>\n>> Please :)\n>>\n>> Done.\n>\n>> > + /* Requests for which no buffer has been queued to the CIO2\n>> device yet. */\n>> > + std::queue pendingRequests_;\n>> > + /* Requests queued to the CIO2 device but not yet processed by\n>> the ImgU. */\n>> > + std::queue processingRequests_;\n>> > +\n>> > + // ControlInfoMap ipaControls_;\n>>\n>> This as well\n>>\n>> Done.\n>\n>> > + bool started_ = false;\n>> > +};\n>> > +\n>> > +class FakeCameraConfiguration : public CameraConfiguration\n>> > +{\n>> > +public:\n>> > + static constexpr unsigned int kBufferCount = 4; // 4~6\n>> > + static constexpr unsigned int kMaxStreams = 3;\n>> > +\n>> > + FakeCameraConfiguration(FakeCameraData *data);\n>> > +\n>> > + Status validate() override;\n>> > +\n>> > +private:\n>> > + /*\n>> > + * The FakeCameraData instance is guaranteed to be valid as long\n>> as the\n>> > + * corresponding Camera instance is valid. In order to borrow a\n>> > + * reference to the camera data, store a new reference to the\n>> camera.\n>> > + */\n>>\n>> This comes from the IPU3 pipeline handler, and I wonder if it still\n>> applies there or it should be removed.\n>>\n>>\n> I believe it's still needed in |validate()| to set streams, right?\n>\n>\n>> > + const FakeCameraData *data_;\n>> > +};\n>> > +\n>> > +class PipelineHandlerFake : public PipelineHandler\n>> > +{\n>> > +public:\n>> > + static constexpr unsigned int V4L2_CID_Fake_PIPE_MODE =\n>> 0x009819c1;\n>>\n>> Unused, please drop\n>>\n>> Done.\n>\n>> > + static constexpr Size kViewfinderSize{ 1280, 720 };\n>> > +\n>> > + enum FakePipeModes {\n>> > + FakePipeModeVideo = 0,\n>> > + FakePipeModeStillCapture = 1,\n>> > + };\n>>\n>> ditto\n>>\n>>\n> Done.\n>\n>\n>> > +\n>> > + PipelineHandlerFake(CameraManager *manager);\n>> > +\n>> > + CameraConfiguration *generateConfiguration(Camera *camera,\n>> > + const StreamRoles &roles) override;\n>>\n>> Align to open (\n>>\n>> Done.\n>\n>> > + int configure(Camera *camera, CameraConfiguration *config)\n>> override;\n>> > +\n>> > + int exportFrameBuffers(Camera *camera, Stream *stream,\n>> > + std::vector>\n>> *buffers) override;\n>> > +\n>> > + int start(Camera *camera, const ControlList *controls) override;\n>> > + void stopDevice(Camera *camera) override;\n>> > +\n>> > + int queueRequestDevice(Camera *camera, Request *request) override;\n>> > +\n>> > + bool match(DeviceEnumerator *enumerator) override;\n>> > +\n>> > +private:\n>> > + FakeCameraData *cameraData(Camera *camera)\n>> > + {\n>> > + return static_cast(camera->_d());\n>> > + }\n>> > +\n>> > + int registerCameras();\n>> > +\n>> > + static bool registered_;\n>> > +};\n>> > +\n>> > +bool PipelineHandlerFake::registered_ = false;\n>>\n>> What purpose does this serve ? Do you get multiple calls to match()\n>> so that you need to keep a flag ?\n>>\n>> Yes, in `src/libcamera/camera_manager.cpp`, it intentionally calls\n> match() multiple times\n> until it returns false (which means no devices matched). Therefore, this\n> is the hack as\n> the fake media devices haven't been created yet.\n>\n>\n>> > +\n>> > +FakeCameraConfiguration::FakeCameraConfiguration(FakeCameraData *data)\n>> > + : CameraConfiguration()\n>> > +{\n>> > + data_ = data;\n>> > +}\n>> > +\n>> > +CameraConfiguration::Status FakeCameraConfiguration::validate()\n>> > +{\n>> > + Status status = Valid;\n>> > +\n>> > + if (config_.empty())\n>> > + return Invalid;\n>> > +\n>> > + // Transform combined = transform * data_->rotationTransform_;\n>> > +\n>> > + // /*\n>> > + // * We combine the platform and user transform, but must\n>> \"adjust away\"\n>> > + // * any combined result that includes a transposition, as we\n>> can't do\n>> > + // * those. In this case, flipping only the transpose bit is\n>> helpful to\n>> > + // * applications - they either get the transform they\n>> requested, or have\n>> > + // * to do a simple transpose themselves (they don't have to\n>> worry about\n>> > + // * the other possible cases).\n>> > + // */\n>> > + // if (!!(combined & Transform::Transpose)) {\n>> > + // /*\n>> > + // * Flipping the transpose bit in \"transform\" flips it in\n>> the\n>> > + // * combined result too (as it's the last thing that\n>> happens),\n>> > + // * which is of course clearing it.\n>> > + // */\n>> > + // transform ^= Transform::Transpose;\n>> > + // combined &= ~Transform::Transpose;\n>> > + // status = Adjusted;\n>> > + // }\n>> > +\n>> > + // /*\n>> > + // * We also check if the sensor doesn't do h/vflips at all, in\n>> which\n>> > + // * case we clear them, and the application will have to do\n>> everything.\n>> > + // */\n>> > + // if (!data_->supportsFlips_ && !!combined) {\n>> > + // /*\n>> > + // * If the sensor can do no transforms, then combined must\n>> be\n>> > + // * changed to the identity. The only user transform that\n>> gives\n>> > + // * rise to this is the inverse of the rotation. (Recall\n>> that\n>> > + // * combined = transform * rotationTransform.)\n>> > + // */\n>> > + // transform = -data_->rotationTransform_;\n>> > + // combined = Transform::Identity;\n>> > + // status = Adjusted;\n>> > + // }\n>> > +\n>> > + /*\n>> > + * Store the final combined transform that configure() will need\n>> to\n>> > + * apply to the sensor to save us working it out again.\n>> > + */\n>> > + // combinedTransform_ = combined;\n>>\n>> Please drop commented out code :0\n>>\n>> > +\n>> > + /* Cap the number of entries to the available streams. */\n>> > + if (config_.size() > kMaxStreams) {\n>> > + config_.resize(kMaxStreams);\n>> > + status = Adjusted;\n>> > + }\n>> > +\n>> > + /*\n>> > + * Validate the requested stream configuration and select the\n>> sensor\n>> > + * format by collecting the maximum RAW stream width and height\n>> and\n>> > + * picking the closest larger match.\n>> > + *\n>> > + * If no RAW stream is requested use the one of the largest YUV\n>> stream,\n>> > + * plus margin pixels for the IF and BDS rectangle to downscale.\n>> > + *\n>> > + * \\todo Clarify the IF and BDS margins requirements.\n>>\n>> Please drop IPU3 related stuff\n>>\n>> Will do. I'll update the number of supported streams as well.\n>\n>> > + */\n>> > + unsigned int rawCount = 0;\n>> > + unsigned int yuvCount = 0;\n>> > + Size rawRequirement;\n>> > + Size maxYuvSize;\n>> > + Size rawSize;\n>> > +\n>> > + for (const StreamConfiguration &cfg : config_) {\n>> > + const PixelFormatInfo &info =\n>> PixelFormatInfo::info(cfg.pixelFormat);\n>> > +\n>> > + if (info.colourEncoding ==\n>> PixelFormatInfo::ColourEncodingRAW) {\n>> > + rawCount++;\n>> > + rawSize = std::max(rawSize, cfg.size);\n>> > + } else {\n>> > + yuvCount++;\n>> > + maxYuvSize = std::max(maxYuvSize, cfg.size);\n>> > + rawRequirement.expandTo(cfg.size);\n>> > + }\n>> > + }\n>> > +\n>> > + // TODO: Base on number of cameras?\n>>\n>> Well, should this pipeline register more than one camera ?\n>> And anyway, the number of streams is per-camera so the number of\n>> cameras should be not relevant here\n>>\n>>\n> Yes, and yes. The configuration file should specify the number of\n> supported streams.\n>\n>\n>> > + if (rawCount > 1 || yuvCount > 2) {\n>> > + LOG(Fake, Debug) << \"Camera configuration not supported\";\n>> > + return Invalid;\n>> > + }\n>> > +\n>> > + /*\n>> > + * Generate raw configuration from CIO2.\n>> > + *\n>> > + * The output YUV streams will be limited in size to the maximum\n>> frame\n>> > + * size requested for the RAW stream, if present.\n>> > + *\n>> > + * If no raw stream is requested, generate a size from the\n>> largest YUV\n>> > + * stream, aligned to the ImgU constraints and bound\n>> > + * by the sensor's maximum resolution. See\n>> > + * https://bugs.libcamera.org/show_bug.cgi?id=32\n>> > + */\n>> > + // TODO\n>> > + if (rawSize.isNull())\n>> > + rawSize = rawRequirement;\n>>\n>> All of these serves on IPU3 to generate a size suitable for the CIO2\n>> and the sensor. You can remove it.\n>>\n>>\n> Right, thanks!\n>\n> > +\n>> > + /*\n>> > + * Adjust the configurations if needed and assign streams while\n>> > + * iterating them.\n>> > + */\n>> > + bool mainOutputAvailable = true;\n>> > + for (unsigned int i = 0; i < config_.size(); ++i) {\n>> > + const PixelFormatInfo &info =\n>> PixelFormatInfo::info(config_[i].pixelFormat);\n>> > + const StreamConfiguration originalCfg = config_[i];\n>> > + StreamConfiguration *cfg = &config_[i];\n>> > +\n>> > + LOG(Fake, Debug) << \"Validating stream: \" <<\n>> config_[i].toString();\n>> > +\n>> > + if (info.colourEncoding ==\n>> PixelFormatInfo::ColourEncodingRAW) {\n>> > + /* Initialize the RAW stream with the CIO2\n>> configuration. */\n>> > + cfg->size = rawSize;\n>> > + // TODO: check\n>> > + cfg->pixelFormat = formats::SBGGR10_IPU3;\n>> > + cfg->bufferCount =\n>> FakeCameraConfiguration::kBufferCount;\n>> > + cfg->stride = info.stride(cfg->size.width, 0, 64);\n>> > + cfg->frameSize = info.frameSize(cfg->size, 64);\n>> > + cfg->setStream(const_cast> *>(&data_->rawStream_));\n>> > +\n>> > + LOG(Fake, Debug) << \"Assigned \" << cfg->toString()\n>> > + << \" to the raw stream\";\n>> > + } else {\n>> > + /* Assign and configure the main and viewfinder\n>> outputs. */\n>> > +\n>> > + cfg->pixelFormat = formats::NV12;\n>> > + cfg->bufferCount = kBufferCount;\n>> > + cfg->stride = info.stride(cfg->size.width, 0, 1);\n>> > + cfg->frameSize = info.frameSize(cfg->size, 1);\n>> > +\n>> > + /*\n>> > + * Use the main output stream in case only one\n>> stream is\n>> > + * requested or if the current configuration is\n>> the one\n>> > + * with the maximum YUV output size.\n>> > + */\n>> > + if (mainOutputAvailable &&\n>> > + (originalCfg.size == maxYuvSize || yuvCount\n>> == 1)) {\n>> > + cfg->setStream(const_cast> *>(&data_->outStream_));\n>> > + mainOutputAvailable = false;\n>> > +\n>> > + LOG(Fake, Debug) << \"Assigned \" <<\n>> cfg->toString()\n>> > + << \" to the main output\";\n>> > + } else {\n>> > + cfg->setStream(const_cast> *>(&data_->vfStream_));\n>> > +\n>> > + LOG(Fake, Debug) << \"Assigned \" <<\n>> cfg->toString()\n>> > + << \" to the viewfinder\n>> output\";\n>> > + }\n>> > + }\n>>\n>> Again I'm not sure how much of the IPU3 should this pipeline mimick,\n>> or it should establish requirements and constraints (ie max 2 YUV\n>> streams of max size width x height, and 1 RAW stream in format\n>> V4L2_PIX_FMT_..)\n>>\n>>\n> Exactly. Will do.\n>\n>\n>> > +\n>> > + if (cfg->pixelFormat != originalCfg.pixelFormat ||\n>> > + cfg->size != originalCfg.size) {\n>> > + LOG(Fake, Debug)\n>> > + << \"Stream \" << i << \" configuration\n>> adjusted to \"\n>> > + << cfg->toString();\n>> > + status = Adjusted;\n>> > + }\n>> > + }\n>> > +\n>> > + return status;\n>> > +}\n>> > +\n>> > +PipelineHandlerFake::PipelineHandlerFake(CameraManager *manager)\n>> > + : PipelineHandler(manager)\n>> > +{\n>> > + // TODO: read the fake hal spec file.\n>>\n>> If there's a design document with requirements, can you share it ?\n>>\n>>\n> Will do.\n>\n>\n\nJust shared it with you, Umang, Paul, and Kieran. Please check if you get\nthe invitation email.\nAlthough it seems that the CrOS Camera is going on their own plan:\nmodifying the existing usb hal as the new fake hal. Therefore, we can focus\non our own use cases, and just take their design as a reference.\n\n\n> > +}\n>> > +\n>> > +CameraConfiguration *PipelineHandlerFake::generateConfiguration(Camera\n>> *camera,\n>> > + const\n>> StreamRoles &roles)\n>> > +{\n>> > + FakeCameraData *data = cameraData(camera);\n>> > + FakeCameraConfiguration *config = new\n>> FakeCameraConfiguration(data);\n>> > +\n>> > + if (roles.empty())\n>> > + return config;\n>> > +\n>> > + Size minSize, sensorResolution;\n>> > + for (const auto& resolution : data->supported_resolutions_) {\n>> > + if (minSize.isNull() || minSize > resolution.size)\n>> > + minSize = resolution.size;\n>> > +\n>> > + sensorResolution = std::max(sensorResolution,\n>> resolution.size);\n>> > + }\n>> > +\n>>\n>> Those are hard-coded values, I think you can reuse them here.\n>> Unless the idea is to make this information come from a configuration\n>> file or something similar.\n>>\n>> Yes, supportedResolutions_ should eventually come from a configuration\n> file, provided by the tester.\n>\n>\n>> > + for (const StreamRole role : roles) {\n>> > + std::map>\n>> streamFormats;\n>> > + unsigned int bufferCount;\n>> > + PixelFormat pixelFormat;\n>> > + Size size;\n>> > +\n>> > + switch (role) {\n>> > + case StreamRole::StillCapture:\n>> > + size = sensorResolution;\n>> > + pixelFormat = formats::NV12;\n>> > + bufferCount =\n>> FakeCameraConfiguration::kBufferCount;\n>> > + streamFormats[pixelFormat] = { { minSize, size }\n>> };\n>> > +\n>> > + break;\n>> > +\n>> > + case StreamRole::Raw: {\n>> > + // TODO: check\n>> > + pixelFormat = formats::SBGGR10_IPU3;\n>> > + size = sensorResolution;\n>> > + bufferCount =\n>> FakeCameraConfiguration::kBufferCount;\n>> > + streamFormats[pixelFormat] = { { minSize, size }\n>> };\n>> > +\n>> > + break;\n>> > + }\n>> > +\n>> > + case StreamRole::Viewfinder:\n>> > + case StreamRole::VideoRecording: {\n>> > + /*\n>> > + * Default viewfinder and videorecording to\n>> 1280x720,\n>> > + * capped to the maximum sensor resolution and\n>> aligned\n>> > + * to the ImgU output constraints.\n>> > + */\n>> > + size = sensorResolution;\n>> > + pixelFormat = formats::NV12;\n>> > + bufferCount =\n>> FakeCameraConfiguration::kBufferCount;\n>> > + streamFormats[pixelFormat] = { { minSize, size }\n>> };\n>> > +\n>> > + break;\n>> > + }\n>> > +\n>> > + default:\n>> > + LOG(Fake, Error)\n>> > + << \"Requested stream role not supported:\n>> \" << role;\n>> > + delete config;\n>> > + return nullptr;\n>> > + }\n>> > +\n>> > + StreamFormats formats(streamFormats);\n>> > + StreamConfiguration cfg(formats);\n>> > + cfg.size = size;\n>> > + cfg.pixelFormat = pixelFormat;\n>> > + cfg.bufferCount = bufferCount;\n>> > + config->addConfiguration(cfg);\n>> > + }\n>> > +\n>> > + if (config->validate() == CameraConfiguration::Invalid)\n>> > + return {};\n>> > +\n>> > + return config;\n>> > +}\n>> > +\n>> > +int PipelineHandlerFake::configure(Camera *camera, CameraConfiguration\n>> *c)\n>> > +{\n>> > + if (camera || c)\n>> > + return 0;\n>> > + return 0;\n>> > +}\n>> > +\n>> > +int PipelineHandlerFake::exportFrameBuffers(Camera *camera, Stream\n>> *stream,\n>> > +\n>> std::vector> *buffers)\n>> > +{\n>> > + // Assume it's never called.\n>> > + LOG(Fake, Fatal) << \"exportFrameBuffers should never be called\";\n>> > + if (camera || stream || buffers)\n>> > + return -EINVAL;\n>> > + return -EINVAL;\n>> > +}\n>> > +\n>> > +int PipelineHandlerFake::start(Camera *camera, [[maybe_unused]] const\n>> ControlList *controls)\n>> > +{\n>> > + FakeCameraData *data = cameraData(camera);\n>> > + data->started_ = true;\n>> > +\n>> > + return 0;\n>> > +}\n>> > +\n>> > +void PipelineHandlerFake::stopDevice(Camera *camera)\n>> > +{\n>> > + FakeCameraData *data = cameraData(camera);\n>> > +\n>> > + data->started_ = false;\n>> > +}\n>> > +\n>> > +int PipelineHandlerFake::queueRequestDevice(Camera *camera, Request\n>> *request)\n>> > +{\n>> > + if (!camera)\n>> > + return -EINVAL;\n>>\n>> Can this happen ?\n>>\n>> Actually it's a hack that CrOS compiler will fail if there's any unused\n> argument...\n> I'll try to clean it up later.\n>\n>\n>> > +\n>> > + for (auto it : request->buffers())\n>> > + completeBuffer(request, it.second);\n>> > +\n>> > + // TODO: request.metadata()\n>> > + request->metadata().set(controls::SensorTimestamp,\n>> CurrentTimestamp());\n>> > + completeRequest(request);\n>> > +\n>> > + return 0;\n>> > +}\n>> > +\n>> > +bool PipelineHandlerFake::match(DeviceEnumerator *enumerator)\n>> > +{\n>> > + // TODO: exhaust all devices in |enumerator|.\n>> > + if (!enumerator)\n>> > + LOG(Fake, Info) << \"Invalid enumerator\";\n>>\n>> Can this happen ?\n>>\n>> ditto\n>\n>> > +\n>> > + if (registered_)\n>> > + return false;\n>> > +\n>> > + registered_ = true;\n>> > + return registerCameras() == 0;\n>> > +}\n>> > +\n>> > +/**\n>> > + * \\brief Initialise ImgU and CIO2 devices associated with cameras\n>> > + *\n>> > + * Initialise the two ImgU instances and create cameras with an\n>> associated\n>> > + * CIO2 device instance.\n>> > + *\n>> > + * \\return 0 on success or a negative error code for error or if no\n>> camera\n>> > + * has been created\n>> > + * \\retval -ENODEV no camera has been created\n>> > + */\n>> > +int PipelineHandlerFake::registerCameras()\n>> > +{\n>> > + std::unique_ptr data =\n>> > + std::make_unique(this);\n>> > + std::set streams = {\n>> > + &data->outStream_,\n>> > + &data->vfStream_,\n>> > + &data->rawStream_,\n>> > + };\n>> > +\n>> > + // TODO: Read from config or from IPC.\n>> > + // TODO: Check with Han-lin: Can this function be called more\n>> than once?\n>> > + data->supported_resolutions_.resize(2);\n>> > + data->supported_resolutions_[0].size = Size(1920, 1080);\n>> > + data->supported_resolutions_[0].frame_rates.push_back(30);\n>> > +\n>> data->supported_resolutions_[0].formats.push_back(\"YCbCr_420_888\");\n>> > + data->supported_resolutions_[0].formats.push_back(\"BLOB\");\n>> > + data->supported_resolutions_[1].size = Size(1280, 720);\n>> > + data->supported_resolutions_[1].frame_rates.push_back(30);\n>> > + data->supported_resolutions_[1].frame_rates.push_back(60);\n>> > +\n>> data->supported_resolutions_[1].formats.push_back(\"YCbCr_420_888\");\n>> > + data->supported_resolutions_[1].formats.push_back(\"BLOB\");\n>> > +\n>> > + // TODO: Assign different locations for different cameras based\n>> on config.\n>> > + data->properties_.set(properties::Location,\n>> properties::CameraLocationFront);\n>> > + data->properties_.set(properties::PixelArrayActiveAreas, {\n>> Rectangle(Size(1920, 1080)) });\n>> > +\n>> > + // TODO: Set FrameDurationLimits based on config.\n>> > + ControlInfoMap::Map controls{};\n>> > + int64_t min_frame_duration = 30, max_frame_duration = 60;\n>> > + controls[&controls::FrameDurationLimits] =\n>> ControlInfo(min_frame_duration, max_frame_duration);\n>> > + data->controlInfo_ = ControlInfoMap(std::move(controls),\n>> controls::controls);\n>>\n>> FakeControls is ignored\n>>\n>> Right. Use it to initialize |controls| here.\n>\n>\n>> > +\n>> > + std::shared_ptr camera =\n>> > + Camera::create(std::move(data), \"\\\\_SB_.PCI0.I2C4.CAM1\"\n>> /* cameraId */, streams);\n>> > +\n>> > + registerCamera(std::move(camera));\n>> > +\n>> > + return 0;\n>> > +}\n>> > +\n>> > +REGISTER_PIPELINE_HANDLER(PipelineHandlerFake)\n>> > +\n>> > +} /* namespace libcamera */\n>> > diff --git a/src/libcamera/pipeline/fake/meson.build\n>> b/src/libcamera/pipeline/fake/meson.build\n>> > new file mode 100644\n>> > index 00000000..13980b4a\n>> > --- /dev/null\n>> > +++ b/src/libcamera/pipeline/fake/meson.build\n>> > @@ -0,0 +1,3 @@\n>> > +# SPDX-License-Identifier: CC0-1.0\n>> > +\n>> > +libcamera_sources += files([ 'fake.cpp' ])\n>> > diff --git a/src/libcamera/pipeline_handler.cpp\n>> b/src/libcamera/pipeline_handler.cpp\n>> > index e5cb751c..4261154d 100644\n>> > --- a/src/libcamera/pipeline_handler.cpp\n>> > +++ b/src/libcamera/pipeline_handler.cpp\n>> > @@ -536,7 +536,7 @@ void\n>> PipelineHandler::registerCamera(std::shared_ptr camera)\n>> > cameras_.push_back(camera);\n>> >\n>> > if (mediaDevices_.empty())\n>> > - LOG(Pipeline, Fatal)\n>> > + LOG(Pipeline, Error)\n>>\n>> I don't think we want that, you should probably open code the\n>>\n>> manager_->addCamera(std::move(camera), devnums);\n>>\n>> call, with an empty list of devnums, which should be fine for this use\n>> case.\n>>\n>> The PipelineHandler::cameras_ vector will remain unpopulated, but\n>> since it is used only in case of a media device disconnection (afaict)\n>> it should be fine in your case\n>>\n>>\n> Ah you're right. Thanks!\n>\n>\n>>\n>> > << \"Registering camera with no media devices!\";\n>> >\n>> > /*\n>> > diff --git a/test/camera/camera_reconfigure.cpp\n>> b/test/camera/camera_reconfigure.cpp\n>> > index d12e2413..06c87730 100644\n>> > --- a/test/camera/camera_reconfigure.cpp\n>> > +++ b/test/camera/camera_reconfigure.cpp\n>> > @@ -98,7 +98,7 @@ private:\n>> > return TestFail;\n>> > }\n>> >\n>> > - requests_.push_back(move(request));\n>> > + requests_.push_back(std::move(request));\n>>\n>> Unrelated and possibly not necessary, as the test has\n>> using namespace std;\n>>\n>>\n> Yeah... it's also a hack that it's required in the CrOS compiler... It\n> doesn't\n> accept |move|...\n>\n>\n>> Looking forward to seeing this skeleton getting populated, it will help\n>> testing the Android layer!\n>>\n>> Thanks\n>> j\n>>\n>> > }\n>> >\n>> > camera_->requestCompleted.connect(this,\n>> &CameraReconfigure::requestComplete);\n>> > --\n>> > 2.38.0.rc1.362.ged0d419d3c-goog\n>> >\n>>\n>","headers":{"Return-Path":"","X-Original-To":"parsemail@patchwork.libcamera.org","Delivered-To":"parsemail@patchwork.libcamera.org","Received":["from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 820B9BD16B\n\tfor ;\n\tFri, 21 Oct 2022 22:38:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B46A662EC3;\n\tSat, 22 Oct 2022 00:38:24 +0200 (CEST)","from mail-wr1-x42a.google.com (mail-wr1-x42a.google.com\n\t[IPv6:2a00:1450:4864:20::42a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1F53B62E75\n\tfor ;\n\tSat, 22 Oct 2022 00:38:22 +0200 (CEST)","by mail-wr1-x42a.google.com with SMTP id n12so7062731wrp.10\n\tfor ;\n\tFri, 21 Oct 2022 15:38:22 -0700 (PDT)"],"DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/simple; d=libcamera.org;\n\ts=mail; t=1666391904;\n\tbh=wTkxWYaogtRu+tRR3U1lYY/Ud5KIDnaoZ4XIvZ+PUCc=;\n\th=References:In-Reply-To:Date:To:Subject:List-Id:List-Unsubscribe:\n\tList-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc:\n\tFrom;\n\tb=YcmsQJVGxWcyiF3iWdJk0fm1RRA5ZbCwDcOgw8LBOYCobSkgQAXk5XW9FKGicXbPp\n\tHGAH5rpIjb+W+6wy3jXXC61x3Pa/gZ3Guvq0bDhl91Pj1thcq6VB477OOyKxOOpkj8\n\tEbb9yz2k+QLGT9J23TDvZHNXBVLg2vGcOXLSQdZrlByfBdscHy11P8cjJWodIUtNPH\n\t57gb6touMXGjaYU4W+Qn5RPFxy9WsD4GU+KRDt+rVjqs7eIkNVf2Lg+7v/gwnLacue\n\tzjz/0up0JVqKDrE6MN94iWiDwZxaqLgbb22EcFukrT9tp2XL2DG6eBzUd+6OO5avnG\n\tIPCUsE/kgTHUw==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org;\n\ts=google; \n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:from:to:cc:subject:date:message-id:reply-to;\n\tbh=ZcrKVfDtEFN28A153zxlOaO1gB0EQYwQeO9TlolPZBU=;\n\tb=Z/RtEwTzUz5UuLjo7BLsBSDWQ6M3xudNTmeK8dlti/7W1UzvT2cMh89z1D/yD9lm+5\n\t7U6FTkboqqdqO0nluuLNnw752AtupIFu8QEwGFIF7RfDiEJce2JCshwMN1PXY5PrKfpt\n\t2Hb6fqPiICIHoSh7szajcCv7zyMHa8IBsUjmU="],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=chromium.org\n\theader.i=@chromium.org header.b=\"Z/RtEwTz\"; \n\tdkim-atps=neutral","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20210112;\n\th=cc:to:subject:message-id:date:from:in-reply-to:references\n\t:mime-version:x-gm-message-state:from:to:cc:subject:date:message-id\n\t:reply-to;\n\tbh=ZcrKVfDtEFN28A153zxlOaO1gB0EQYwQeO9TlolPZBU=;\n\tb=I3ofAp15uybBVDN5ZqiTbrzNCNQf+vnoTrPr6MQ19DO6dzTDc9yP91szZ35MAfLitv\n\tUb9pgVnsOs/bWh6Xo94LpGCSPMw3u4zs+t+wyWbISRQXW+4TS+7kdXp2OecqrnuZE2aA\n\tmdIWvTQsfrxAGTm4p3+b78PrzqwU9wbY4IZRNpWyrm6c3XyQ0tpzIITC2lGKik6ieF0r\n\tGy91wHdmrSk9a1S54VQkro5XZz57pCrm3m053YXl4IEPnmXkrt7+9Lo2yol85nnTec17\n\taaLrlfVQdyeaw8QHr48Sbrnc8Wt6efdVm9pxdcziU02vX8I/G4s12GWqFVEzFPeai3aJ\n\t7+fQ==","X-Gm-Message-State":"ACrzQf3e+PR8rfZ+mF3SrY8G9EBONktuK8wE6CMtq42a9PgwJoNWFIY9\n\tNxtc/nV3PpMktm0O3jTjffntEsmt0pcFQpEUr5jeqvRiBrvNLQ==","X-Google-Smtp-Source":"AMsMyM5Z/bYPrGJUNAyRAAVEV8PUX/eOdJRghaIWZru0Kuh9+mW7RMuu/9MWxZ401JUKvif01zBzjQkwRCddhBrbJ8I=","X-Received":"by 2002:adf:e305:0:b0:22e:6b55:3ed9 with SMTP id\n\tb5-20020adfe305000000b0022e6b553ed9mr13358081wrj.684.1666391901520;\n\tFri, 21 Oct 2022 15:38:21 -0700 (PDT)","MIME-Version":"1.0","References":"<20221012075925.3971538-1-chenghaoyang@google.com>\n\t<20221012075925.3971538-2-chenghaoyang@google.com>\n\t<20221012150607.76brmkwluibrqqzn@uno.localdomain>\n\t","In-Reply-To":"","Date":"Sat, 22 Oct 2022 07:38:10 +0900","Message-ID":"","To":"Jacopo Mondi ","Content-Type":"multipart/alternative; boundary=\"00000000000058e0b405eb931943\"","Subject":"Re: [libcamera-devel] [PATCH 1/1] Add fake pipeline handler","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","From":"Cheng-Hao Yang via libcamera-devel\n\t","Reply-To":"Cheng-Hao Yang ","Cc":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]