[{"id":30224,"web_url":"https://patchwork.libcamera.org/comment/30224/","msgid":"<ZoQEhp5_9ErfaoSA@pyrite.rasen.tech>","date":"2024-07-02T13:45:42","subject":"Re: [PATCH v4 5/5] libcamera: rkisp1: Plumb the ConverterDW100\n\tconverter","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Thu, Jun 27, 2024 at 07:16:56PM +0530, Umang Jain wrote:\n> Plumb the ConverterDW100 converter in the rkisp1 pipeline handler.\n> If the dewarper is found, it is instantiated and buffers are exported\n> from it, instead of RkISP1Path. Internal buffers are allocated for the\n> RkISP1Path in case where dewarper is going to be used.\n> \n> The RKISP1 pipeline handler now supports scaler crop control through\n> ConverterDW100. Register the ScalerCrop control for the cameras created\n> in the RKISP1 pipeline handler.\n> \n> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> ---\n>  src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 144 +++++++++++++++++-\n>  src/libcamera/pipeline/rkisp1/rkisp1_path.cpp |  12 +-\n>  src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  14 ++\n>  3 files changed, 161 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index bfc44239..eb373be4 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -8,9 +8,11 @@\n>  #include <algorithm>\n>  #include <array>\n>  #include <iomanip>\n> +#include <map>\n>  #include <memory>\n>  #include <numeric>\n>  #include <queue>\n> +#include <vector>\n>  \n>  #include <linux/media-bus-format.h>\n>  #include <linux/rkisp1-config.h>\n> @@ -33,6 +35,7 @@\n>  \n>  #include \"libcamera/internal/camera.h\"\n>  #include \"libcamera/internal/camera_sensor.h\"\n> +#include \"libcamera/internal/converter/converter_dw100.h\"\n>  #include \"libcamera/internal/delayed_controls.h\"\n>  #include \"libcamera/internal/device_enumerator.h\"\n>  #include \"libcamera/internal/framebuffer.h\"\n> @@ -62,6 +65,8 @@ struct RkISP1FrameInfo {\n>  \n>  \tbool paramDequeued;\n>  \tbool metadataProcessed;\n> +\n> +\tstd::optional<Rectangle> scalerCrop;\n>  };\n>  \n>  class RkISP1Frames\n> @@ -181,6 +186,7 @@ private:\n>  \tvoid bufferReady(FrameBuffer *buffer);\n>  \tvoid paramReady(FrameBuffer *buffer);\n>  \tvoid statReady(FrameBuffer *buffer);\n> +\tvoid dewarpBufferReady(FrameBuffer *buffer);\n>  \tvoid frameStart(uint32_t sequence);\n>  \n>  \tint allocateBuffers(Camera *camera);\n> @@ -200,6 +206,13 @@ private:\n>  \tRkISP1MainPath mainPath_;\n>  \tRkISP1SelfPath selfPath_;\n>  \n> +\tstd::unique_ptr<ConverterDW100> dewarper_;\n> +\tbool useDewarper_;\n> +\n> +\t/* Internal buffers used when dewarper is being used */\n> +\tstd::vector<std::unique_ptr<FrameBuffer>> mainPathBuffers_;\n> +\tstd::queue<FrameBuffer *> availableMainPathBuffers_;\n> +\n>  \tstd::vector<std::unique_ptr<FrameBuffer>> paramBuffers_;\n>  \tstd::vector<std::unique_ptr<FrameBuffer>> statBuffers_;\n>  \tstd::queue<FrameBuffer *> availableParamBuffers_;\n> @@ -222,6 +235,8 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n>  \n>  \tFrameBuffer *paramBuffer = nullptr;\n>  \tFrameBuffer *statBuffer = nullptr;\n> +\tFrameBuffer *mainPathBuffer = nullptr;\n> +\tFrameBuffer *selfPathBuffer = nullptr;\n\nI don't think you need the selfPathBuffer?\n\n>  \n>  \tif (!isRaw) {\n>  \t\tif (pipe_->availableParamBuffers_.empty()) {\n> @@ -239,10 +254,16 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n>  \n>  \t\tstatBuffer = pipe_->availableStatBuffers_.front();\n>  \t\tpipe_->availableStatBuffers_.pop();\n> +\n> +\t\tif (pipe_->useDewarper_) {\n> +\t\t\tmainPathBuffer = pipe_->availableMainPathBuffers_.front();\n> +\t\t\tpipe_->availableMainPathBuffers_.pop();\n> +\t\t}\n>  \t}\n>  \n> -\tFrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);\n> -\tFrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);\n> +\tif (!mainPathBuffer)\n> +\t\tmainPathBuffer = request->findBuffer(&data->mainPathStream_);\n> +\tselfPathBuffer = request->findBuffer(&data->selfPathStream_);\n>  \n>  \tRkISP1FrameInfo *info = new RkISP1FrameInfo;\n>  \n> @@ -268,6 +289,7 @@ int RkISP1Frames::destroy(unsigned int frame)\n>  \n>  \tpipe_->availableParamBuffers_.push(info->paramBuffer);\n>  \tpipe_->availableStatBuffers_.push(info->statBuffer);\n> +\tpipe_->availableMainPathBuffers_.push(info->mainPathBuffer);\n>  \n>  \tframeInfo_.erase(info->frame);\n>  \n> @@ -283,6 +305,7 @@ void RkISP1Frames::clear()\n>  \n>  \t\tpipe_->availableParamBuffers_.push(info->paramBuffer);\n>  \t\tpipe_->availableStatBuffers_.push(info->statBuffer);\n> +\t\tpipe_->availableMainPathBuffers_.push(info->mainPathBuffer);\n>  \n>  \t\tdelete info;\n>  \t}\n> @@ -607,7 +630,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n>   */\n>  \n>  PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n> -\t: PipelineHandler(manager), hasSelfPath_(true)\n> +\t: PipelineHandler(manager), hasSelfPath_(true), useDewarper_(false)\n>  {\n>  }\n>  \n> @@ -785,12 +808,26 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n>  \t\t<< \" crop \" << rect;\n>  \n>  \tstd::map<unsigned int, IPAStream> streamConfig;\n> +\tstd::vector<std::reference_wrapper<StreamConfiguration>> outputCfgs;\n>  \n>  \tfor (const StreamConfiguration &cfg : *config) {\n>  \t\tif (cfg.stream() == &data->mainPathStream_) {\n>  \t\t\tret = mainPath_.configure(cfg, format);\n>  \t\t\tstreamConfig[0] = IPAStream(cfg.pixelFormat,\n>  \t\t\t\t\t\t    cfg.size);\n> +\t\t\t/* Configure dewarp */\n> +\t\t\tif (dewarper_ && !isRaw_) {\n> +\t\t\t\t/*\n> +\t\t\t\t * \\todo Converter::configure() API should be changed\n> +\t\t\t\t * to use std::vector<std::reference_wrapper<const StreamConfiguration>> ?\n> +\t\t\t\t */\n> +\t\t\t\toutputCfgs.push_back(const_cast<StreamConfiguration &>(cfg));\n> +\t\t\t\tdewarper_->configure(cfg, outputCfgs);\n> +\t\t\t\tuseDewarper_ = true;\n> +\t\t\t} else {\n> +\t\t\t\tuseDewarper_ = false;\n> +\t\t\t}\n> +\n>  \t\t} else if (hasSelfPath_) {\n>  \t\t\tret = selfPath_.configure(cfg, format);\n>  \t\t\tstreamConfig[1] = IPAStream(cfg.pixelFormat,\n> @@ -839,6 +876,9 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S\n>  \tRkISP1CameraData *data = cameraData(camera);\n>  \tunsigned int count = stream->configuration().bufferCount;\n>  \n> +\tif (useDewarper_)\n> +\t\treturn dewarper_->exportBuffers(&data->mainPathStream_, count, buffers);\n> +\n>  \tif (stream == &data->mainPathStream_)\n>  \t\treturn mainPath_.exportBuffers(count, buffers);\n>  \telse if (hasSelfPath_ && stream == &data->selfPathStream_)\n> @@ -866,6 +906,16 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>  \t\tret = stat_->allocateBuffers(maxCount, &statBuffers_);\n>  \t\tif (ret < 0)\n>  \t\t\tgoto error;\n> +\n> +\t\t/* If the dewarper is being used, allocate internal buffers for ISP */\n> +\t\tif (useDewarper_) {\n> +\t\t\tret = mainPath_.allocateBuffers(maxCount, &mainPathBuffers_);\n> +\t\t\tif (ret < 0)\n> +\t\t\t\tgoto error;\n> +\n> +\t\t\tfor (std::unique_ptr<FrameBuffer> &buffer : mainPathBuffers_)\n> +\t\t\t\tavailableMainPathBuffers_.push(buffer.get());\n> +\t\t}\n>  \t}\n>  \n>  \tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {\n> @@ -889,6 +939,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>  error:\n>  \tparamBuffers_.clear();\n>  \tstatBuffers_.clear();\n> +\tmainPathBuffers_.clear();\n>  \n>  \treturn ret;\n>  }\n> @@ -903,8 +954,12 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n>  \twhile (!availableParamBuffers_.empty())\n>  \t\tavailableParamBuffers_.pop();\n>  \n> +\twhile (!availableMainPathBuffers_.empty())\n> +\t\tavailableMainPathBuffers_.pop();\n> +\n>  \tparamBuffers_.clear();\n>  \tstatBuffers_.clear();\n> +\tmainPathBuffers_.clear();\n>  \n>  \tstd::vector<unsigned int> ids;\n>  \tfor (IPABuffer &ipabuf : data->ipaBuffers_)\n> @@ -919,6 +974,9 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n>  \tif (stat_->releaseBuffers())\n>  \t\tLOG(RkISP1, Error) << \"Failed to release stat buffers\";\n>  \n> +\tif (useDewarper_)\n> +\t\tmainPath_.releaseBuffers();\n> +\n>  \treturn 0;\n>  }\n>  \n> @@ -961,6 +1019,14 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n>  \t\t\t\t<< \"Failed to start statistics \" << camera->id();\n>  \t\t\treturn ret;\n>  \t\t}\n> +\n> +\t\tif (useDewarper_) {\n> +\t\t\tret = dewarper_->start();\n> +\t\t\tif (ret) {\n> +\t\t\t\tLOG(RkISP1, Error) << \"Failed to start dewarper\";\n> +\t\t\t\treturn ret;\n> +\t\t\t}\n> +\t\t}\n>  \t}\n>  \n>  \tif (data->mainPath_->isEnabled()) {\n> @@ -1015,6 +1081,9 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)\n>  \t\tif (ret)\n>  \t\t\tLOG(RkISP1, Warning)\n>  \t\t\t\t<< \"Failed to stop parameters for \" << camera->id();\n> +\n> +\t\tif (useDewarper_)\n> +\t\t\tdewarper_->stop();\n>  \t}\n>  \n>  \tASSERT(data->queuedRequests_.empty());\n> @@ -1045,6 +1114,14 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n>  \t\t\t\t\t     info->paramBuffer->cookie());\n>  \t}\n>  \n> +\tconst auto &crop = request->controls().get(controls::ScalerCrop);\n> +\tif (crop && useDewarper_) {\n> +\t\tRectangle rect = crop.value();\n> +\t\tint ret = dewarper_->setScalerCrop(&data->mainPathStream_, &rect);\n> +\t\tif (!ret)\n> +\t\t\tinfo->scalerCrop = crop;\n> +\t}\n> +\n>  \tdata->frame_++;\n>  \n>  \treturn 0;\n> @@ -1110,6 +1187,12 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n>  {\n>  \tControlInfoMap::Map rkisp1Controls;\n>  \n> +\tif (dewarper_) {\n\nDoes this not need to check useDewarper_?\n\nOtherwise looks good to me.\n\n\nPaul\n\n> +\t\tauto [minCrop, maxCrop] = dewarper_->getCropBounds(&data->mainPathStream_);\n> +\n> +\t\trkisp1Controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);\n> +\t}\n> +\n>  \t/* Add the IPA registered controls to list of camera controls. */\n>  \tfor (const auto &ipaControl : data->ipaControls_)\n>  \t\trkisp1Controls[ipaControl.first] = ipaControl.second;\n> @@ -1173,6 +1256,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n>  \n>  bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n>  {\n> +\tstd::shared_ptr<MediaDevice> dwpMediaDevice;\n>  \tconst MediaPad *pad;\n>  \n>  \tDeviceMatch dm(\"rkisp1\");\n> @@ -1237,6 +1321,26 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n>  \tstat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n>  \tparam_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\n>  \n> +\t/* If dewarper is present, create its instance. */\n> +\tDeviceMatch dwp(\"dw100\");\n> +\tdwp.add(\"dw100-source\");\n> +\tdwp.add(\"dw100-sink\");\n> +\n> +\tdwpMediaDevice = enumerator->search(dwp);\n> +\tif (dwpMediaDevice) {\n> +\t\tdewarper_ = std::make_unique<ConverterDW100>(std::move(dwpMediaDevice));\n> +\t\tif (dewarper_->isValid()) {\n> +\t\t\tdewarper_->outputBufferReady.connect(\n> +\t\t\t\tthis, &PipelineHandlerRkISP1::dewarpBufferReady);\n> +\n> +\t\t\tLOG(RkISP1, Info) << \"Using DW100 dewarper \" << dewarper_->deviceNode();\n> +\t\t} else {\n> +\t\t\tLOG(RkISP1, Debug) << \"Found DW100 dewarper \" << dewarper_->deviceNode()\n> +\t\t\t\t\t   << \" but invalid\";\n> +\t\t\tdewarper_.reset();\n> +\t\t}\n> +\t}\n> +\n>  \t/*\n>  \t * Enumerate all sensors connected to the ISP and create one\n>  \t * camera instance for each of them.\n> @@ -1283,7 +1387,7 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n>  \t\treturn;\n>  \n>  \tconst FrameMetadata &metadata = buffer->metadata();\n> -\tRequest *request = buffer->request();\n> +\tRequest *request = info->request;\n>  \n>  \tif (metadata.status != FrameMetadata::FrameCancelled) {\n>  \t\t/*\n> @@ -1300,11 +1404,43 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n>  \t\t\t\tdata->delayedCtrls_->get(metadata.sequence);\n>  \t\t\tdata->ipa_->processStatsBuffer(info->frame, 0, ctrls);\n>  \t\t}\n> +\n>  \t} else {\n>  \t\tif (isRaw_)\n>  \t\t\tinfo->metadataProcessed = true;\n>  \t}\n>  \n> +\tif (useDewarper_) {\n> +\t\t/*\n> +\t\t * Queue input and output buffers to the dewarper. The output\n> +\t\t * buffers for the dewarper are the buffers of the request, supplied\n> +\t\t * by the application.\n> +\t\t */\n> +\t\tint ret = dewarper_->queueBuffers(buffer, request->buffers());\n> +\t\tif (ret < 0)\n> +\t\t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n> +\t\t\t\t\t   << strerror(-ret);\n> +\n> +\t\treturn;\n> +\t}\n> +\n> +\tcompleteBuffer(request, buffer);\n> +\ttryCompleteRequest(info);\n> +}\n> +\n> +void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer)\n> +{\n> +\tASSERT(activeCamera_);\n> +\tRkISP1CameraData *data = cameraData(activeCamera_);\n> +\tRequest *request = buffer->request();\n> +\n> +\tRkISP1FrameInfo *info = data->frameInfo_.find(buffer->request());\n> +\tif (!info)\n> +\t\treturn;\n> +\n> +\tif (info->scalerCrop)\n> +\t\trequest->metadata().set(controls::ScalerCrop, info->scalerCrop.value());\n> +\n>  \tcompleteBuffer(request, buffer);\n>  \ttryCompleteRequest(info);\n>  }\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> index c49017d1..c96ec1d6 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> @@ -56,7 +56,7 @@ const std::map<PixelFormat, uint32_t> formatToMediaBus = {\n>  \n>  RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,\n>  \t\t       const Size &minResolution, const Size &maxResolution)\n> -\t: name_(name), running_(false), formats_(formats),\n> +\t: name_(name), running_(false), internalBufs_(false), formats_(formats),\n>  \t  minResolution_(minResolution), maxResolution_(maxResolution),\n>  \t  link_(nullptr)\n>  {\n> @@ -402,10 +402,12 @@ int RkISP1Path::start()\n>  \tif (running_)\n>  \t\treturn -EBUSY;\n>  \n> -\t/* \\todo Make buffer count user configurable. */\n> -\tret = video_->importBuffers(RKISP1_BUFFER_COUNT);\n> -\tif (ret)\n> -\t\treturn ret;\n> +\tif (!internalBufs_) {\n> +\t\t/* \\todo Make buffer count user configurable. */\n> +\t\tret = video_->importBuffers(RKISP1_BUFFER_COUNT);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n>  \n>  \tret = video_->streamOn();\n>  \tif (ret) {\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> index 08edefec..b7fa82d0 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> @@ -55,6 +55,19 @@ public:\n>  \t\treturn video_->exportBuffers(bufferCount, buffers);\n>  \t}\n>  \n> +\tint allocateBuffers(unsigned int bufferCount,\n> +\t\t\t    std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n> +\t{\n> +\t\tinternalBufs_ = true;\n> +\t\treturn video_->allocateBuffers(bufferCount, buffers);\n> +\t}\n> +\n> +\tint releaseBuffers()\n> +\t{\n> +\t\tASSERT(internalBufs_);\n> +\t\treturn video_->releaseBuffers();\n> +\t}\n> +\n>  \tint start();\n>  \tvoid stop();\n>  \n> @@ -68,6 +81,7 @@ private:\n>  \n>  \tconst char *name_;\n>  \tbool running_;\n> +\tbool internalBufs_;\n>  \n>  \tconst Span<const PixelFormat> formats_;\n>  \tstd::set<PixelFormat> streamFormats_;\n> -- \n> 2.44.0\n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 2B6D5BEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  2 Jul 2024 13:45:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BAEAC62E25;\n\tTue,  2 Jul 2024 15:45:50 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id A6CE762C96\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Jul 2024 15:45:48 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 69DB6664;\n\tTue,  2 Jul 2024 15:45:20 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"pExfMO7Q\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719927921;\n\tbh=P7KAelUmKZ4ywJpIM1QeFH7X4M9yOhofKaimuFnZ3FM=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=pExfMO7QtvL29LxXOUZWcB3IrMaadjTMpRQLE9szoh24vYLj9gy0/JFBEqXsCCiVG\n\tUa95XDhDCrOd/hPGs0EE9SR8tanFMEBFYiyfORCmPLiI/CWrReDWI1CSi+ozxYvmdH\n\t7/oJDyyZwv67cIfprEIXaNSlWMMwnQBcx90a08mc=","Date":"Tue, 2 Jul 2024 22:45:42 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Umang Jain <umang.jain@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 5/5] libcamera: rkisp1: Plumb the ConverterDW100\n\tconverter","Message-ID":"<ZoQEhp5_9ErfaoSA@pyrite.rasen.tech>","References":"<20240627134656.582462-1-umang.jain@ideasonboard.com>\n\t<20240627134656.582462-6-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<20240627134656.582462-6-umang.jain@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30233,"web_url":"https://patchwork.libcamera.org/comment/30233/","msgid":"<1c3c8828-17db-4253-b852-85eead2e9217@ideasonboard.com>","date":"2024-07-03T03:54:05","subject":"Re: [PATCH v4 5/5] libcamera: rkisp1: Plumb the ConverterDW100\n\tconverter","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Paul\n\nOn 02/07/24 7:15 pm, Paul Elder wrote:\n> On Thu, Jun 27, 2024 at 07:16:56PM +0530, Umang Jain wrote:\n>> Plumb the ConverterDW100 converter in the rkisp1 pipeline handler.\n>> If the dewarper is found, it is instantiated and buffers are exported\n>> from it, instead of RkISP1Path. Internal buffers are allocated for the\n>> RkISP1Path in case where dewarper is going to be used.\n>>\n>> The RKISP1 pipeline handler now supports scaler crop control through\n>> ConverterDW100. Register the ScalerCrop control for the cameras created\n>> in the RKISP1 pipeline handler.\n>>\n>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n>> ---\n>>   src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 144 +++++++++++++++++-\n>>   src/libcamera/pipeline/rkisp1/rkisp1_path.cpp |  12 +-\n>>   src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  14 ++\n>>   3 files changed, 161 insertions(+), 9 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index bfc44239..eb373be4 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -8,9 +8,11 @@\n>>   #include <algorithm>\n>>   #include <array>\n>>   #include <iomanip>\n>> +#include <map>\n>>   #include <memory>\n>>   #include <numeric>\n>>   #include <queue>\n>> +#include <vector>\n>>   \n>>   #include <linux/media-bus-format.h>\n>>   #include <linux/rkisp1-config.h>\n>> @@ -33,6 +35,7 @@\n>>   \n>>   #include \"libcamera/internal/camera.h\"\n>>   #include \"libcamera/internal/camera_sensor.h\"\n>> +#include \"libcamera/internal/converter/converter_dw100.h\"\n>>   #include \"libcamera/internal/delayed_controls.h\"\n>>   #include \"libcamera/internal/device_enumerator.h\"\n>>   #include \"libcamera/internal/framebuffer.h\"\n>> @@ -62,6 +65,8 @@ struct RkISP1FrameInfo {\n>>   \n>>   \tbool paramDequeued;\n>>   \tbool metadataProcessed;\n>> +\n>> +\tstd::optional<Rectangle> scalerCrop;\n>>   };\n>>   \n>>   class RkISP1Frames\n>> @@ -181,6 +186,7 @@ private:\n>>   \tvoid bufferReady(FrameBuffer *buffer);\n>>   \tvoid paramReady(FrameBuffer *buffer);\n>>   \tvoid statReady(FrameBuffer *buffer);\n>> +\tvoid dewarpBufferReady(FrameBuffer *buffer);\n>>   \tvoid frameStart(uint32_t sequence);\n>>   \n>>   \tint allocateBuffers(Camera *camera);\n>> @@ -200,6 +206,13 @@ private:\n>>   \tRkISP1MainPath mainPath_;\n>>   \tRkISP1SelfPath selfPath_;\n>>   \n>> +\tstd::unique_ptr<ConverterDW100> dewarper_;\n>> +\tbool useDewarper_;\n>> +\n>> +\t/* Internal buffers used when dewarper is being used */\n>> +\tstd::vector<std::unique_ptr<FrameBuffer>> mainPathBuffers_;\n>> +\tstd::queue<FrameBuffer *> availableMainPathBuffers_;\n>> +\n>>   \tstd::vector<std::unique_ptr<FrameBuffer>> paramBuffers_;\n>>   \tstd::vector<std::unique_ptr<FrameBuffer>> statBuffers_;\n>>   \tstd::queue<FrameBuffer *> availableParamBuffers_;\n>> @@ -222,6 +235,8 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n>>   \n>>   \tFrameBuffer *paramBuffer = nullptr;\n>>   \tFrameBuffer *statBuffer = nullptr;\n>> +\tFrameBuffer *mainPathBuffer = nullptr;\n>> +\tFrameBuffer *selfPathBuffer = nullptr;\n> I don't think you need the selfPathBuffer?\n\nJust a comestic change. See selfPathBuffer hunk declaration below.\nI have just moved it up here (not inlined)...\n\n>\n>>   \n>>   \tif (!isRaw) {\n>>   \t\tif (pipe_->availableParamBuffers_.empty()) {\n>> @@ -239,10 +254,16 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n>>   \n>>   \t\tstatBuffer = pipe_->availableStatBuffers_.front();\n>>   \t\tpipe_->availableStatBuffers_.pop();\n>> +\n>> +\t\tif (pipe_->useDewarper_) {\n>> +\t\t\tmainPathBuffer = pipe_->availableMainPathBuffers_.front();\n>> +\t\t\tpipe_->availableMainPathBuffers_.pop();\n>> +\t\t}\n>>   \t}\n>>   \n>> -\tFrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);\n>> -\tFrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);\n>> +\tif (!mainPathBuffer)\n>> +\t\tmainPathBuffer = request->findBuffer(&data->mainPathStream_);\n>> +\tselfPathBuffer = request->findBuffer(&data->selfPathStream_);\n>>   \n>>   \tRkISP1FrameInfo *info = new RkISP1FrameInfo;\n>>   \n>> @@ -268,6 +289,7 @@ int RkISP1Frames::destroy(unsigned int frame)\n>>   \n>>   \tpipe_->availableParamBuffers_.push(info->paramBuffer);\n>>   \tpipe_->availableStatBuffers_.push(info->statBuffer);\n>> +\tpipe_->availableMainPathBuffers_.push(info->mainPathBuffer);\n>>   \n>>   \tframeInfo_.erase(info->frame);\n>>   \n>> @@ -283,6 +305,7 @@ void RkISP1Frames::clear()\n>>   \n>>   \t\tpipe_->availableParamBuffers_.push(info->paramBuffer);\n>>   \t\tpipe_->availableStatBuffers_.push(info->statBuffer);\n>> +\t\tpipe_->availableMainPathBuffers_.push(info->mainPathBuffer);\n>>   \n>>   \t\tdelete info;\n>>   \t}\n>> @@ -607,7 +630,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n>>    */\n>>   \n>>   PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n>> -\t: PipelineHandler(manager), hasSelfPath_(true)\n>> +\t: PipelineHandler(manager), hasSelfPath_(true), useDewarper_(false)\n>>   {\n>>   }\n>>   \n>> @@ -785,12 +808,26 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n>>   \t\t<< \" crop \" << rect;\n>>   \n>>   \tstd::map<unsigned int, IPAStream> streamConfig;\n>> +\tstd::vector<std::reference_wrapper<StreamConfiguration>> outputCfgs;\n>>   \n>>   \tfor (const StreamConfiguration &cfg : *config) {\n>>   \t\tif (cfg.stream() == &data->mainPathStream_) {\n>>   \t\t\tret = mainPath_.configure(cfg, format);\n>>   \t\t\tstreamConfig[0] = IPAStream(cfg.pixelFormat,\n>>   \t\t\t\t\t\t    cfg.size);\n>> +\t\t\t/* Configure dewarp */\n>> +\t\t\tif (dewarper_ && !isRaw_) {\n>> +\t\t\t\t/*\n>> +\t\t\t\t * \\todo Converter::configure() API should be changed\n>> +\t\t\t\t * to use std::vector<std::reference_wrapper<const StreamConfiguration>> ?\n>> +\t\t\t\t */\n>> +\t\t\t\toutputCfgs.push_back(const_cast<StreamConfiguration &>(cfg));\n>> +\t\t\t\tdewarper_->configure(cfg, outputCfgs);\n>> +\t\t\t\tuseDewarper_ = true;\n>> +\t\t\t} else {\n>> +\t\t\t\tuseDewarper_ = false;\n>> +\t\t\t}\n>> +\n>>   \t\t} else if (hasSelfPath_) {\n>>   \t\t\tret = selfPath_.configure(cfg, format);\n>>   \t\t\tstreamConfig[1] = IPAStream(cfg.pixelFormat,\n>> @@ -839,6 +876,9 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S\n>>   \tRkISP1CameraData *data = cameraData(camera);\n>>   \tunsigned int count = stream->configuration().bufferCount;\n>>   \n>> +\tif (useDewarper_)\n>> +\t\treturn dewarper_->exportBuffers(&data->mainPathStream_, count, buffers);\n>> +\n>>   \tif (stream == &data->mainPathStream_)\n>>   \t\treturn mainPath_.exportBuffers(count, buffers);\n>>   \telse if (hasSelfPath_ && stream == &data->selfPathStream_)\n>> @@ -866,6 +906,16 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>>   \t\tret = stat_->allocateBuffers(maxCount, &statBuffers_);\n>>   \t\tif (ret < 0)\n>>   \t\t\tgoto error;\n>> +\n>> +\t\t/* If the dewarper is being used, allocate internal buffers for ISP */\n>> +\t\tif (useDewarper_) {\n>> +\t\t\tret = mainPath_.allocateBuffers(maxCount, &mainPathBuffers_);\n>> +\t\t\tif (ret < 0)\n>> +\t\t\t\tgoto error;\n>> +\n>> +\t\t\tfor (std::unique_ptr<FrameBuffer> &buffer : mainPathBuffers_)\n>> +\t\t\t\tavailableMainPathBuffers_.push(buffer.get());\n>> +\t\t}\n>>   \t}\n>>   \n>>   \tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {\n>> @@ -889,6 +939,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>>   error:\n>>   \tparamBuffers_.clear();\n>>   \tstatBuffers_.clear();\n>> +\tmainPathBuffers_.clear();\n>>   \n>>   \treturn ret;\n>>   }\n>> @@ -903,8 +954,12 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n>>   \twhile (!availableParamBuffers_.empty())\n>>   \t\tavailableParamBuffers_.pop();\n>>   \n>> +\twhile (!availableMainPathBuffers_.empty())\n>> +\t\tavailableMainPathBuffers_.pop();\n>> +\n>>   \tparamBuffers_.clear();\n>>   \tstatBuffers_.clear();\n>> +\tmainPathBuffers_.clear();\n>>   \n>>   \tstd::vector<unsigned int> ids;\n>>   \tfor (IPABuffer &ipabuf : data->ipaBuffers_)\n>> @@ -919,6 +974,9 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n>>   \tif (stat_->releaseBuffers())\n>>   \t\tLOG(RkISP1, Error) << \"Failed to release stat buffers\";\n>>   \n>> +\tif (useDewarper_)\n>> +\t\tmainPath_.releaseBuffers();\n>> +\n>>   \treturn 0;\n>>   }\n>>   \n>> @@ -961,6 +1019,14 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n>>   \t\t\t\t<< \"Failed to start statistics \" << camera->id();\n>>   \t\t\treturn ret;\n>>   \t\t}\n>> +\n>> +\t\tif (useDewarper_) {\n>> +\t\t\tret = dewarper_->start();\n>> +\t\t\tif (ret) {\n>> +\t\t\t\tLOG(RkISP1, Error) << \"Failed to start dewarper\";\n>> +\t\t\t\treturn ret;\n>> +\t\t\t}\n>> +\t\t}\n>>   \t}\n>>   \n>>   \tif (data->mainPath_->isEnabled()) {\n>> @@ -1015,6 +1081,9 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)\n>>   \t\tif (ret)\n>>   \t\t\tLOG(RkISP1, Warning)\n>>   \t\t\t\t<< \"Failed to stop parameters for \" << camera->id();\n>> +\n>> +\t\tif (useDewarper_)\n>> +\t\t\tdewarper_->stop();\n>>   \t}\n>>   \n>>   \tASSERT(data->queuedRequests_.empty());\n>> @@ -1045,6 +1114,14 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n>>   \t\t\t\t\t     info->paramBuffer->cookie());\n>>   \t}\n>>   \n>> +\tconst auto &crop = request->controls().get(controls::ScalerCrop);\n>> +\tif (crop && useDewarper_) {\n>> +\t\tRectangle rect = crop.value();\n>> +\t\tint ret = dewarper_->setScalerCrop(&data->mainPathStream_, &rect);\n>> +\t\tif (!ret)\n>> +\t\t\tinfo->scalerCrop = crop;\n>> +\t}\n>> +\n>>   \tdata->frame_++;\n>>   \n>>   \treturn 0;\n>> @@ -1110,6 +1187,12 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n>>   {\n>>   \tControlInfoMap::Map rkisp1Controls;\n>>   \n>> +\tif (dewarper_) {\n> Does this not need to check useDewarper_?\n\nuseDewarper_ is only true in the configured when a non-RAW stream is \nconfigured. updateControls() is called in configure() and createCamera().\n\nIf I use useDewarper_, scalerCrop control would not be registered for \nthe camera. The goal here is to register the scaler crop control for the \ncamera (when being created). To use the dewarper or not, depends on the \npipeline configuration and what has been configured.\n\n\n>\n> Otherwise looks good to me.\n>\n>\n> Paul\n>\n>> +\t\tauto [minCrop, maxCrop] = dewarper_->getCropBounds(&data->mainPathStream_);\n>> +\n>> +\t\trkisp1Controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);\n>> +\t}\n>> +\n>>   \t/* Add the IPA registered controls to list of camera controls. */\n>>   \tfor (const auto &ipaControl : data->ipaControls_)\n>>   \t\trkisp1Controls[ipaControl.first] = ipaControl.second;\n>> @@ -1173,6 +1256,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n>>   \n>>   bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n>>   {\n>> +\tstd::shared_ptr<MediaDevice> dwpMediaDevice;\n>>   \tconst MediaPad *pad;\n>>   \n>>   \tDeviceMatch dm(\"rkisp1\");\n>> @@ -1237,6 +1321,26 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n>>   \tstat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n>>   \tparam_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\n>>   \n>> +\t/* If dewarper is present, create its instance. */\n>> +\tDeviceMatch dwp(\"dw100\");\n>> +\tdwp.add(\"dw100-source\");\n>> +\tdwp.add(\"dw100-sink\");\n>> +\n>> +\tdwpMediaDevice = enumerator->search(dwp);\n>> +\tif (dwpMediaDevice) {\n>> +\t\tdewarper_ = std::make_unique<ConverterDW100>(std::move(dwpMediaDevice));\n>> +\t\tif (dewarper_->isValid()) {\n>> +\t\t\tdewarper_->outputBufferReady.connect(\n>> +\t\t\t\tthis, &PipelineHandlerRkISP1::dewarpBufferReady);\n>> +\n>> +\t\t\tLOG(RkISP1, Info) << \"Using DW100 dewarper \" << dewarper_->deviceNode();\n>> +\t\t} else {\n>> +\t\t\tLOG(RkISP1, Debug) << \"Found DW100 dewarper \" << dewarper_->deviceNode()\n>> +\t\t\t\t\t   << \" but invalid\";\n>> +\t\t\tdewarper_.reset();\n>> +\t\t}\n>> +\t}\n>> +\n>>   \t/*\n>>   \t * Enumerate all sensors connected to the ISP and create one\n>>   \t * camera instance for each of them.\n>> @@ -1283,7 +1387,7 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n>>   \t\treturn;\n>>   \n>>   \tconst FrameMetadata &metadata = buffer->metadata();\n>> -\tRequest *request = buffer->request();\n>> +\tRequest *request = info->request;\n>>   \n>>   \tif (metadata.status != FrameMetadata::FrameCancelled) {\n>>   \t\t/*\n>> @@ -1300,11 +1404,43 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n>>   \t\t\t\tdata->delayedCtrls_->get(metadata.sequence);\n>>   \t\t\tdata->ipa_->processStatsBuffer(info->frame, 0, ctrls);\n>>   \t\t}\n>> +\n>>   \t} else {\n>>   \t\tif (isRaw_)\n>>   \t\t\tinfo->metadataProcessed = true;\n>>   \t}\n>>   \n>> +\tif (useDewarper_) {\n>> +\t\t/*\n>> +\t\t * Queue input and output buffers to the dewarper. The output\n>> +\t\t * buffers for the dewarper are the buffers of the request, supplied\n>> +\t\t * by the application.\n>> +\t\t */\n>> +\t\tint ret = dewarper_->queueBuffers(buffer, request->buffers());\n>> +\t\tif (ret < 0)\n>> +\t\t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n>> +\t\t\t\t\t   << strerror(-ret);\n>> +\n>> +\t\treturn;\n>> +\t}\n>> +\n>> +\tcompleteBuffer(request, buffer);\n>> +\ttryCompleteRequest(info);\n>> +}\n>> +\n>> +void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer)\n>> +{\n>> +\tASSERT(activeCamera_);\n>> +\tRkISP1CameraData *data = cameraData(activeCamera_);\n>> +\tRequest *request = buffer->request();\n>> +\n>> +\tRkISP1FrameInfo *info = data->frameInfo_.find(buffer->request());\n>> +\tif (!info)\n>> +\t\treturn;\n>> +\n>> +\tif (info->scalerCrop)\n>> +\t\trequest->metadata().set(controls::ScalerCrop, info->scalerCrop.value());\n>> +\n>>   \tcompleteBuffer(request, buffer);\n>>   \ttryCompleteRequest(info);\n>>   }\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n>> index c49017d1..c96ec1d6 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n>> @@ -56,7 +56,7 @@ const std::map<PixelFormat, uint32_t> formatToMediaBus = {\n>>   \n>>   RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,\n>>   \t\t       const Size &minResolution, const Size &maxResolution)\n>> -\t: name_(name), running_(false), formats_(formats),\n>> +\t: name_(name), running_(false), internalBufs_(false), formats_(formats),\n>>   \t  minResolution_(minResolution), maxResolution_(maxResolution),\n>>   \t  link_(nullptr)\n>>   {\n>> @@ -402,10 +402,12 @@ int RkISP1Path::start()\n>>   \tif (running_)\n>>   \t\treturn -EBUSY;\n>>   \n>> -\t/* \\todo Make buffer count user configurable. */\n>> -\tret = video_->importBuffers(RKISP1_BUFFER_COUNT);\n>> -\tif (ret)\n>> -\t\treturn ret;\n>> +\tif (!internalBufs_) {\n>> +\t\t/* \\todo Make buffer count user configurable. */\n>> +\t\tret = video_->importBuffers(RKISP1_BUFFER_COUNT);\n>> +\t\tif (ret)\n>> +\t\t\treturn ret;\n>> +\t}\n>>   \n>>   \tret = video_->streamOn();\n>>   \tif (ret) {\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n>> index 08edefec..b7fa82d0 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n>> @@ -55,6 +55,19 @@ public:\n>>   \t\treturn video_->exportBuffers(bufferCount, buffers);\n>>   \t}\n>>   \n>> +\tint allocateBuffers(unsigned int bufferCount,\n>> +\t\t\t    std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n>> +\t{\n>> +\t\tinternalBufs_ = true;\n>> +\t\treturn video_->allocateBuffers(bufferCount, buffers);\n>> +\t}\n>> +\n>> +\tint releaseBuffers()\n>> +\t{\n>> +\t\tASSERT(internalBufs_);\n>> +\t\treturn video_->releaseBuffers();\n>> +\t}\n>> +\n>>   \tint start();\n>>   \tvoid stop();\n>>   \n>> @@ -68,6 +81,7 @@ private:\n>>   \n>>   \tconst char *name_;\n>>   \tbool running_;\n>> +\tbool internalBufs_;\n>>   \n>>   \tconst Span<const PixelFormat> formats_;\n>>   \tstd::set<PixelFormat> streamFormats_;\n>> -- \n>> 2.44.0\n>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 1419EBEFBE\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  3 Jul 2024 03:54:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C58EA62E22;\n\tWed,  3 Jul 2024 05:54:14 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BE253619C0\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Jul 2024 05:54:12 +0200 (CEST)","from [IPV6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f] (unknown\n\t[IPv6:2405:201:2015:f873:55d7:c02e:b2eb:ee3f])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 17F425A4;\n\tWed,  3 Jul 2024 05:53:43 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"ALPd9czJ\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719978824;\n\tbh=Z237fOkJx/oRPIIdD3O6hx1gXSEsVNjvdqOd+4rxIq0=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=ALPd9czJIWuVXtufXLoYIGl3zxbRNIWFZ62Q01hF8LvczkHT8XLrTrZZ6ih1hA+YY\n\tJ0uvWhUlvSbD6jE3FOVtY5uX4+qHMD7SvY40a8AAe0jiBmOU4/YooGhLn5Pr0DE1UT\n\tdc3+tr93zIvo/upHUtRrEvOxDXj3uRWzZPmMY9j8=","Message-ID":"<1c3c8828-17db-4253-b852-85eead2e9217@ideasonboard.com>","Date":"Wed, 3 Jul 2024 09:24:05 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 5/5] libcamera: rkisp1: Plumb the ConverterDW100\n\tconverter","Content-Language":"en-US","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20240627134656.582462-1-umang.jain@ideasonboard.com>\n\t<20240627134656.582462-6-umang.jain@ideasonboard.com>\n\t<ZoQEhp5_9ErfaoSA@pyrite.rasen.tech>","From":"Umang Jain <umang.jain@ideasonboard.com>","In-Reply-To":"<ZoQEhp5_9ErfaoSA@pyrite.rasen.tech>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30239,"web_url":"https://patchwork.libcamera.org/comment/30239/","msgid":"<ZoUBVfI87oLDYHTl@pyrite.rasen.tech>","date":"2024-07-03T07:44:21","subject":"Re: [PATCH v4 5/5] libcamera: rkisp1: Plumb the ConverterDW100\n\tconverter","submitter":{"id":17,"url":"https://patchwork.libcamera.org/api/people/17/","name":"Paul Elder","email":"paul.elder@ideasonboard.com"},"content":"On Wed, Jul 03, 2024 at 09:24:05AM +0530, Umang Jain wrote:\n> Hi Paul\n> \n> On 02/07/24 7:15 pm, Paul Elder wrote:\n> > On Thu, Jun 27, 2024 at 07:16:56PM +0530, Umang Jain wrote:\n> > > Plumb the ConverterDW100 converter in the rkisp1 pipeline handler.\n> > > If the dewarper is found, it is instantiated and buffers are exported\n> > > from it, instead of RkISP1Path. Internal buffers are allocated for the\n> > > RkISP1Path in case where dewarper is going to be used.\n> > > \n> > > The RKISP1 pipeline handler now supports scaler crop control through\n> > > ConverterDW100. Register the ScalerCrop control for the cameras created\n> > > in the RKISP1 pipeline handler.\n> > > \n> > > Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n> > > ---\n> > >   src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 144 +++++++++++++++++-\n> > >   src/libcamera/pipeline/rkisp1/rkisp1_path.cpp |  12 +-\n> > >   src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  14 ++\n> > >   3 files changed, 161 insertions(+), 9 deletions(-)\n> > > \n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index bfc44239..eb373be4 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -8,9 +8,11 @@\n> > >   #include <algorithm>\n> > >   #include <array>\n> > >   #include <iomanip>\n> > > +#include <map>\n> > >   #include <memory>\n> > >   #include <numeric>\n> > >   #include <queue>\n> > > +#include <vector>\n> > >   #include <linux/media-bus-format.h>\n> > >   #include <linux/rkisp1-config.h>\n> > > @@ -33,6 +35,7 @@\n> > >   #include \"libcamera/internal/camera.h\"\n> > >   #include \"libcamera/internal/camera_sensor.h\"\n> > > +#include \"libcamera/internal/converter/converter_dw100.h\"\n> > >   #include \"libcamera/internal/delayed_controls.h\"\n> > >   #include \"libcamera/internal/device_enumerator.h\"\n> > >   #include \"libcamera/internal/framebuffer.h\"\n> > > @@ -62,6 +65,8 @@ struct RkISP1FrameInfo {\n> > >   \tbool paramDequeued;\n> > >   \tbool metadataProcessed;\n> > > +\n> > > +\tstd::optional<Rectangle> scalerCrop;\n> > >   };\n> > >   class RkISP1Frames\n> > > @@ -181,6 +186,7 @@ private:\n> > >   \tvoid bufferReady(FrameBuffer *buffer);\n> > >   \tvoid paramReady(FrameBuffer *buffer);\n> > >   \tvoid statReady(FrameBuffer *buffer);\n> > > +\tvoid dewarpBufferReady(FrameBuffer *buffer);\n> > >   \tvoid frameStart(uint32_t sequence);\n> > >   \tint allocateBuffers(Camera *camera);\n> > > @@ -200,6 +206,13 @@ private:\n> > >   \tRkISP1MainPath mainPath_;\n> > >   \tRkISP1SelfPath selfPath_;\n> > > +\tstd::unique_ptr<ConverterDW100> dewarper_;\n> > > +\tbool useDewarper_;\n> > > +\n> > > +\t/* Internal buffers used when dewarper is being used */\n> > > +\tstd::vector<std::unique_ptr<FrameBuffer>> mainPathBuffers_;\n> > > +\tstd::queue<FrameBuffer *> availableMainPathBuffers_;\n> > > +\n> > >   \tstd::vector<std::unique_ptr<FrameBuffer>> paramBuffers_;\n> > >   \tstd::vector<std::unique_ptr<FrameBuffer>> statBuffers_;\n> > >   \tstd::queue<FrameBuffer *> availableParamBuffers_;\n> > > @@ -222,6 +235,8 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n> > >   \tFrameBuffer *paramBuffer = nullptr;\n> > >   \tFrameBuffer *statBuffer = nullptr;\n> > > +\tFrameBuffer *mainPathBuffer = nullptr;\n> > > +\tFrameBuffer *selfPathBuffer = nullptr;\n> > I don't think you need the selfPathBuffer?\n> \n> Just a comestic change. See selfPathBuffer hunk declaration below.\n> I have just moved it up here (not inlined)...\n> \n\nYeah I realized that...\n\nI also wondered if this would be for ISP versions that do have the self\npath and a dewarper, but since we don't have any yet it doesn't really\nmatter.\n\n> > \n> > >   \tif (!isRaw) {\n> > >   \t\tif (pipe_->availableParamBuffers_.empty()) {\n> > > @@ -239,10 +254,16 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n> > >   \t\tstatBuffer = pipe_->availableStatBuffers_.front();\n> > >   \t\tpipe_->availableStatBuffers_.pop();\n> > > +\n> > > +\t\tif (pipe_->useDewarper_) {\n> > > +\t\t\tmainPathBuffer = pipe_->availableMainPathBuffers_.front();\n> > > +\t\t\tpipe_->availableMainPathBuffers_.pop();\n> > > +\t\t}\n> > >   \t}\n> > > -\tFrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);\n> > > -\tFrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);\n> > > +\tif (!mainPathBuffer)\n> > > +\t\tmainPathBuffer = request->findBuffer(&data->mainPathStream_);\n> > > +\tselfPathBuffer = request->findBuffer(&data->selfPathStream_);\n> > >   \tRkISP1FrameInfo *info = new RkISP1FrameInfo;\n> > > @@ -268,6 +289,7 @@ int RkISP1Frames::destroy(unsigned int frame)\n> > >   \tpipe_->availableParamBuffers_.push(info->paramBuffer);\n> > >   \tpipe_->availableStatBuffers_.push(info->statBuffer);\n> > > +\tpipe_->availableMainPathBuffers_.push(info->mainPathBuffer);\n> > >   \tframeInfo_.erase(info->frame);\n> > > @@ -283,6 +305,7 @@ void RkISP1Frames::clear()\n> > >   \t\tpipe_->availableParamBuffers_.push(info->paramBuffer);\n> > >   \t\tpipe_->availableStatBuffers_.push(info->statBuffer);\n> > > +\t\tpipe_->availableMainPathBuffers_.push(info->mainPathBuffer);\n> > >   \t\tdelete info;\n> > >   \t}\n> > > @@ -607,7 +630,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n> > >    */\n> > >   PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n> > > -\t: PipelineHandler(manager), hasSelfPath_(true)\n> > > +\t: PipelineHandler(manager), hasSelfPath_(true), useDewarper_(false)\n> > >   {\n> > >   }\n> > > @@ -785,12 +808,26 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> > >   \t\t<< \" crop \" << rect;\n> > >   \tstd::map<unsigned int, IPAStream> streamConfig;\n> > > +\tstd::vector<std::reference_wrapper<StreamConfiguration>> outputCfgs;\n> > >   \tfor (const StreamConfiguration &cfg : *config) {\n> > >   \t\tif (cfg.stream() == &data->mainPathStream_) {\n> > >   \t\t\tret = mainPath_.configure(cfg, format);\n> > >   \t\t\tstreamConfig[0] = IPAStream(cfg.pixelFormat,\n> > >   \t\t\t\t\t\t    cfg.size);\n> > > +\t\t\t/* Configure dewarp */\n> > > +\t\t\tif (dewarper_ && !isRaw_) {\n> > > +\t\t\t\t/*\n> > > +\t\t\t\t * \\todo Converter::configure() API should be changed\n> > > +\t\t\t\t * to use std::vector<std::reference_wrapper<const StreamConfiguration>> ?\n> > > +\t\t\t\t */\n> > > +\t\t\t\toutputCfgs.push_back(const_cast<StreamConfiguration &>(cfg));\n> > > +\t\t\t\tdewarper_->configure(cfg, outputCfgs);\n> > > +\t\t\t\tuseDewarper_ = true;\n> > > +\t\t\t} else {\n> > > +\t\t\t\tuseDewarper_ = false;\n> > > +\t\t\t}\n> > > +\n> > >   \t\t} else if (hasSelfPath_) {\n> > >   \t\t\tret = selfPath_.configure(cfg, format);\n> > >   \t\t\tstreamConfig[1] = IPAStream(cfg.pixelFormat,\n> > > @@ -839,6 +876,9 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S\n> > >   \tRkISP1CameraData *data = cameraData(camera);\n> > >   \tunsigned int count = stream->configuration().bufferCount;\n> > > +\tif (useDewarper_)\n> > > +\t\treturn dewarper_->exportBuffers(&data->mainPathStream_, count, buffers);\n> > > +\n> > >   \tif (stream == &data->mainPathStream_)\n> > >   \t\treturn mainPath_.exportBuffers(count, buffers);\n> > >   \telse if (hasSelfPath_ && stream == &data->selfPathStream_)\n> > > @@ -866,6 +906,16 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> > >   \t\tret = stat_->allocateBuffers(maxCount, &statBuffers_);\n> > >   \t\tif (ret < 0)\n> > >   \t\t\tgoto error;\n> > > +\n> > > +\t\t/* If the dewarper is being used, allocate internal buffers for ISP */\n> > > +\t\tif (useDewarper_) {\n> > > +\t\t\tret = mainPath_.allocateBuffers(maxCount, &mainPathBuffers_);\n> > > +\t\t\tif (ret < 0)\n> > > +\t\t\t\tgoto error;\n> > > +\n> > > +\t\t\tfor (std::unique_ptr<FrameBuffer> &buffer : mainPathBuffers_)\n> > > +\t\t\t\tavailableMainPathBuffers_.push(buffer.get());\n> > > +\t\t}\n> > >   \t}\n> > >   \tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {\n> > > @@ -889,6 +939,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> > >   error:\n> > >   \tparamBuffers_.clear();\n> > >   \tstatBuffers_.clear();\n> > > +\tmainPathBuffers_.clear();\n> > >   \treturn ret;\n> > >   }\n> > > @@ -903,8 +954,12 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n> > >   \twhile (!availableParamBuffers_.empty())\n> > >   \t\tavailableParamBuffers_.pop();\n> > > +\twhile (!availableMainPathBuffers_.empty())\n> > > +\t\tavailableMainPathBuffers_.pop();\n> > > +\n> > >   \tparamBuffers_.clear();\n> > >   \tstatBuffers_.clear();\n> > > +\tmainPathBuffers_.clear();\n> > >   \tstd::vector<unsigned int> ids;\n> > >   \tfor (IPABuffer &ipabuf : data->ipaBuffers_)\n> > > @@ -919,6 +974,9 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n> > >   \tif (stat_->releaseBuffers())\n> > >   \t\tLOG(RkISP1, Error) << \"Failed to release stat buffers\";\n> > > +\tif (useDewarper_)\n> > > +\t\tmainPath_.releaseBuffers();\n> > > +\n> > >   \treturn 0;\n> > >   }\n> > > @@ -961,6 +1019,14 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n> > >   \t\t\t\t<< \"Failed to start statistics \" << camera->id();\n> > >   \t\t\treturn ret;\n> > >   \t\t}\n> > > +\n> > > +\t\tif (useDewarper_) {\n> > > +\t\t\tret = dewarper_->start();\n> > > +\t\t\tif (ret) {\n> > > +\t\t\t\tLOG(RkISP1, Error) << \"Failed to start dewarper\";\n> > > +\t\t\t\treturn ret;\n> > > +\t\t\t}\n> > > +\t\t}\n> > >   \t}\n> > >   \tif (data->mainPath_->isEnabled()) {\n> > > @@ -1015,6 +1081,9 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)\n> > >   \t\tif (ret)\n> > >   \t\t\tLOG(RkISP1, Warning)\n> > >   \t\t\t\t<< \"Failed to stop parameters for \" << camera->id();\n> > > +\n> > > +\t\tif (useDewarper_)\n> > > +\t\t\tdewarper_->stop();\n> > >   \t}\n> > >   \tASSERT(data->queuedRequests_.empty());\n> > > @@ -1045,6 +1114,14 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n> > >   \t\t\t\t\t     info->paramBuffer->cookie());\n> > >   \t}\n> > > +\tconst auto &crop = request->controls().get(controls::ScalerCrop);\n> > > +\tif (crop && useDewarper_) {\n> > > +\t\tRectangle rect = crop.value();\n> > > +\t\tint ret = dewarper_->setScalerCrop(&data->mainPathStream_, &rect);\n> > > +\t\tif (!ret)\n> > > +\t\t\tinfo->scalerCrop = crop;\n> > > +\t}\n> > > +\n> > >   \tdata->frame_++;\n> > >   \treturn 0;\n> > > @@ -1110,6 +1187,12 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n> > >   {\n> > >   \tControlInfoMap::Map rkisp1Controls;\n> > > +\tif (dewarper_) {\n> > Does this not need to check useDewarper_?\n> \n> useDewarper_ is only true in the configured when a non-RAW stream is\n> configured. updateControls() is called in configure() and createCamera().\n> \n> If I use useDewarper_, scalerCrop control would not be registered for the\n> camera. The goal here is to register the scaler crop control for the camera\n> (when being created). To use the dewarper or not, depends on the pipeline\n> configuration and what has been configured.\n\nHm that's true. Do we have a policy on whether or not to report controls\nthat depend on the configuration? Or maybe the control is reported to\nexist but the bounds should change depending on the configuration? iirc\nthings like FrameDurationLimits are like that?\n\n\nPaul\n\n> \n> \n> > \n> > Otherwise looks good to me.\n> > \n> > \n> > Paul\n> > \n> > > +\t\tauto [minCrop, maxCrop] = dewarper_->getCropBounds(&data->mainPathStream_);\n> > > +\n> > > +\t\trkisp1Controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);\n> > > +\t}\n> > > +\n> > >   \t/* Add the IPA registered controls to list of camera controls. */\n> > >   \tfor (const auto &ipaControl : data->ipaControls_)\n> > >   \t\trkisp1Controls[ipaControl.first] = ipaControl.second;\n> > > @@ -1173,6 +1256,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n> > >   bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n> > >   {\n> > > +\tstd::shared_ptr<MediaDevice> dwpMediaDevice;\n> > >   \tconst MediaPad *pad;\n> > >   \tDeviceMatch dm(\"rkisp1\");\n> > > @@ -1237,6 +1321,26 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n> > >   \tstat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n> > >   \tparam_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\n> > > +\t/* If dewarper is present, create its instance. */\n> > > +\tDeviceMatch dwp(\"dw100\");\n> > > +\tdwp.add(\"dw100-source\");\n> > > +\tdwp.add(\"dw100-sink\");\n> > > +\n> > > +\tdwpMediaDevice = enumerator->search(dwp);\n> > > +\tif (dwpMediaDevice) {\n> > > +\t\tdewarper_ = std::make_unique<ConverterDW100>(std::move(dwpMediaDevice));\n> > > +\t\tif (dewarper_->isValid()) {\n> > > +\t\t\tdewarper_->outputBufferReady.connect(\n> > > +\t\t\t\tthis, &PipelineHandlerRkISP1::dewarpBufferReady);\n> > > +\n> > > +\t\t\tLOG(RkISP1, Info) << \"Using DW100 dewarper \" << dewarper_->deviceNode();\n> > > +\t\t} else {\n> > > +\t\t\tLOG(RkISP1, Debug) << \"Found DW100 dewarper \" << dewarper_->deviceNode()\n> > > +\t\t\t\t\t   << \" but invalid\";\n> > > +\t\t\tdewarper_.reset();\n> > > +\t\t}\n> > > +\t}\n> > > +\n> > >   \t/*\n> > >   \t * Enumerate all sensors connected to the ISP and create one\n> > >   \t * camera instance for each of them.\n> > > @@ -1283,7 +1387,7 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> > >   \t\treturn;\n> > >   \tconst FrameMetadata &metadata = buffer->metadata();\n> > > -\tRequest *request = buffer->request();\n> > > +\tRequest *request = info->request;\n> > >   \tif (metadata.status != FrameMetadata::FrameCancelled) {\n> > >   \t\t/*\n> > > @@ -1300,11 +1404,43 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> > >   \t\t\t\tdata->delayedCtrls_->get(metadata.sequence);\n> > >   \t\t\tdata->ipa_->processStatsBuffer(info->frame, 0, ctrls);\n> > >   \t\t}\n> > > +\n> > >   \t} else {\n> > >   \t\tif (isRaw_)\n> > >   \t\t\tinfo->metadataProcessed = true;\n> > >   \t}\n> > > +\tif (useDewarper_) {\n> > > +\t\t/*\n> > > +\t\t * Queue input and output buffers to the dewarper. The output\n> > > +\t\t * buffers for the dewarper are the buffers of the request, supplied\n> > > +\t\t * by the application.\n> > > +\t\t */\n> > > +\t\tint ret = dewarper_->queueBuffers(buffer, request->buffers());\n> > > +\t\tif (ret < 0)\n> > > +\t\t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n> > > +\t\t\t\t\t   << strerror(-ret);\n> > > +\n> > > +\t\treturn;\n> > > +\t}\n> > > +\n> > > +\tcompleteBuffer(request, buffer);\n> > > +\ttryCompleteRequest(info);\n> > > +}\n> > > +\n> > > +void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer)\n> > > +{\n> > > +\tASSERT(activeCamera_);\n> > > +\tRkISP1CameraData *data = cameraData(activeCamera_);\n> > > +\tRequest *request = buffer->request();\n> > > +\n> > > +\tRkISP1FrameInfo *info = data->frameInfo_.find(buffer->request());\n> > > +\tif (!info)\n> > > +\t\treturn;\n> > > +\n> > > +\tif (info->scalerCrop)\n> > > +\t\trequest->metadata().set(controls::ScalerCrop, info->scalerCrop.value());\n> > > +\n> > >   \tcompleteBuffer(request, buffer);\n> > >   \ttryCompleteRequest(info);\n> > >   }\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > > index c49017d1..c96ec1d6 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n> > > @@ -56,7 +56,7 @@ const std::map<PixelFormat, uint32_t> formatToMediaBus = {\n> > >   RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,\n> > >   \t\t       const Size &minResolution, const Size &maxResolution)\n> > > -\t: name_(name), running_(false), formats_(formats),\n> > > +\t: name_(name), running_(false), internalBufs_(false), formats_(formats),\n> > >   \t  minResolution_(minResolution), maxResolution_(maxResolution),\n> > >   \t  link_(nullptr)\n> > >   {\n> > > @@ -402,10 +402,12 @@ int RkISP1Path::start()\n> > >   \tif (running_)\n> > >   \t\treturn -EBUSY;\n> > > -\t/* \\todo Make buffer count user configurable. */\n> > > -\tret = video_->importBuffers(RKISP1_BUFFER_COUNT);\n> > > -\tif (ret)\n> > > -\t\treturn ret;\n> > > +\tif (!internalBufs_) {\n> > > +\t\t/* \\todo Make buffer count user configurable. */\n> > > +\t\tret = video_->importBuffers(RKISP1_BUFFER_COUNT);\n> > > +\t\tif (ret)\n> > > +\t\t\treturn ret;\n> > > +\t}\n> > >   \tret = video_->streamOn();\n> > >   \tif (ret) {\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> > > index 08edefec..b7fa82d0 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n> > > @@ -55,6 +55,19 @@ public:\n> > >   \t\treturn video_->exportBuffers(bufferCount, buffers);\n> > >   \t}\n> > > +\tint allocateBuffers(unsigned int bufferCount,\n> > > +\t\t\t    std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n> > > +\t{\n> > > +\t\tinternalBufs_ = true;\n> > > +\t\treturn video_->allocateBuffers(bufferCount, buffers);\n> > > +\t}\n> > > +\n> > > +\tint releaseBuffers()\n> > > +\t{\n> > > +\t\tASSERT(internalBufs_);\n> > > +\t\treturn video_->releaseBuffers();\n> > > +\t}\n> > > +\n> > >   \tint start();\n> > >   \tvoid stop();\n> > > @@ -68,6 +81,7 @@ private:\n> > >   \tconst char *name_;\n> > >   \tbool running_;\n> > > +\tbool internalBufs_;\n> > >   \tconst Span<const PixelFormat> formats_;\n> > >   \tstd::set<PixelFormat> streamFormats_;\n> > > -- \n> > > 2.44.0\n> > > \n>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 05A2DBD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed,  3 Jul 2024 07:44:31 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1D25D62C96;\n\tWed,  3 Jul 2024 09:44:30 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 365B4619C7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed,  3 Jul 2024 09:44:28 +0200 (CEST)","from pyrite.rasen.tech (h175-177-049-156.catv02.itscom.jp\n\t[175.177.49.156])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 4156A3E6;\n\tWed,  3 Jul 2024 09:43:59 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"oWYayD3e\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1719992640;\n\tbh=yAwMxr5Qq6zHSzxsMGQcJKL3tD8MLUyIierev/ZwfZk=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=oWYayD3esR3r6e1qDjTzScsQ9DtZq114CgCsrpqs/N4B80aT9+qy5EshQbhlV4/ZU\n\tnsv9U3RqOxeGvS0Z8L4BvTvkQblXRXKYFHw3b/Oim1sc/P7ik31ukFyNL/Eb/xlQxk\n\tuVA6+No4Ln/fWGcVQTmQyfMAKAuHwLB8A5C5PFRk=","Date":"Wed, 3 Jul 2024 16:44:21 +0900","From":"Paul Elder <paul.elder@ideasonboard.com>","To":"Umang Jain <umang.jain@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v4 5/5] libcamera: rkisp1: Plumb the ConverterDW100\n\tconverter","Message-ID":"<ZoUBVfI87oLDYHTl@pyrite.rasen.tech>","References":"<20240627134656.582462-1-umang.jain@ideasonboard.com>\n\t<20240627134656.582462-6-umang.jain@ideasonboard.com>\n\t<ZoQEhp5_9ErfaoSA@pyrite.rasen.tech>\n\t<1c3c8828-17db-4253-b852-85eead2e9217@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=us-ascii","Content-Disposition":"inline","In-Reply-To":"<1c3c8828-17db-4253-b852-85eead2e9217@ideasonboard.com>","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}},{"id":30379,"web_url":"https://patchwork.libcamera.org/comment/30379/","msgid":"<63a0d465-86ce-4547-8165-27eb5dc87183@ideasonboard.com>","date":"2024-07-09T10:22:49","subject":"Re: [PATCH v4 5/5] libcamera: rkisp1: Plumb the ConverterDW100\n\tconverter","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Paul\n\nOn 03/07/24 1:14 pm, Paul Elder wrote:\n> On Wed, Jul 03, 2024 at 09:24:05AM +0530, Umang Jain wrote:\n>> Hi Paul\n>>\n>> On 02/07/24 7:15 pm, Paul Elder wrote:\n>>> On Thu, Jun 27, 2024 at 07:16:56PM +0530, Umang Jain wrote:\n>>>> Plumb the ConverterDW100 converter in the rkisp1 pipeline handler.\n>>>> If the dewarper is found, it is instantiated and buffers are exported\n>>>> from it, instead of RkISP1Path. Internal buffers are allocated for the\n>>>> RkISP1Path in case where dewarper is going to be used.\n>>>>\n>>>> The RKISP1 pipeline handler now supports scaler crop control through\n>>>> ConverterDW100. Register the ScalerCrop control for the cameras created\n>>>> in the RKISP1 pipeline handler.\n>>>>\n>>>> Signed-off-by: Umang Jain <umang.jain@ideasonboard.com>\n>>>> ---\n>>>>    src/libcamera/pipeline/rkisp1/rkisp1.cpp      | 144 +++++++++++++++++-\n>>>>    src/libcamera/pipeline/rkisp1/rkisp1_path.cpp |  12 +-\n>>>>    src/libcamera/pipeline/rkisp1/rkisp1_path.h   |  14 ++\n>>>>    3 files changed, 161 insertions(+), 9 deletions(-)\n>>>>\n>>>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> index bfc44239..eb373be4 100644\n>>>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> @@ -8,9 +8,11 @@\n>>>>    #include <algorithm>\n>>>>    #include <array>\n>>>>    #include <iomanip>\n>>>> +#include <map>\n>>>>    #include <memory>\n>>>>    #include <numeric>\n>>>>    #include <queue>\n>>>> +#include <vector>\n>>>>    #include <linux/media-bus-format.h>\n>>>>    #include <linux/rkisp1-config.h>\n>>>> @@ -33,6 +35,7 @@\n>>>>    #include \"libcamera/internal/camera.h\"\n>>>>    #include \"libcamera/internal/camera_sensor.h\"\n>>>> +#include \"libcamera/internal/converter/converter_dw100.h\"\n>>>>    #include \"libcamera/internal/delayed_controls.h\"\n>>>>    #include \"libcamera/internal/device_enumerator.h\"\n>>>>    #include \"libcamera/internal/framebuffer.h\"\n>>>> @@ -62,6 +65,8 @@ struct RkISP1FrameInfo {\n>>>>    \tbool paramDequeued;\n>>>>    \tbool metadataProcessed;\n>>>> +\n>>>> +\tstd::optional<Rectangle> scalerCrop;\n>>>>    };\n>>>>    class RkISP1Frames\n>>>> @@ -181,6 +186,7 @@ private:\n>>>>    \tvoid bufferReady(FrameBuffer *buffer);\n>>>>    \tvoid paramReady(FrameBuffer *buffer);\n>>>>    \tvoid statReady(FrameBuffer *buffer);\n>>>> +\tvoid dewarpBufferReady(FrameBuffer *buffer);\n>>>>    \tvoid frameStart(uint32_t sequence);\n>>>>    \tint allocateBuffers(Camera *camera);\n>>>> @@ -200,6 +206,13 @@ private:\n>>>>    \tRkISP1MainPath mainPath_;\n>>>>    \tRkISP1SelfPath selfPath_;\n>>>> +\tstd::unique_ptr<ConverterDW100> dewarper_;\n>>>> +\tbool useDewarper_;\n>>>> +\n>>>> +\t/* Internal buffers used when dewarper is being used */\n>>>> +\tstd::vector<std::unique_ptr<FrameBuffer>> mainPathBuffers_;\n>>>> +\tstd::queue<FrameBuffer *> availableMainPathBuffers_;\n>>>> +\n>>>>    \tstd::vector<std::unique_ptr<FrameBuffer>> paramBuffers_;\n>>>>    \tstd::vector<std::unique_ptr<FrameBuffer>> statBuffers_;\n>>>>    \tstd::queue<FrameBuffer *> availableParamBuffers_;\n>>>> @@ -222,6 +235,8 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n>>>>    \tFrameBuffer *paramBuffer = nullptr;\n>>>>    \tFrameBuffer *statBuffer = nullptr;\n>>>> +\tFrameBuffer *mainPathBuffer = nullptr;\n>>>> +\tFrameBuffer *selfPathBuffer = nullptr;\n>>> I don't think you need the selfPathBuffer?\n>> Just a comestic change. See selfPathBuffer hunk declaration below.\n>> I have just moved it up here (not inlined)...\n>>\n> Yeah I realized that...\n>\n> I also wondered if this would be for ISP versions that do have the self\n> path and a dewarper, but since we don't have any yet it doesn't really\n> matter.\n>\n>>>>    \tif (!isRaw) {\n>>>>    \t\tif (pipe_->availableParamBuffers_.empty()) {\n>>>> @@ -239,10 +254,16 @@ RkISP1FrameInfo *RkISP1Frames::create(const RkISP1CameraData *data, Request *req\n>>>>    \t\tstatBuffer = pipe_->availableStatBuffers_.front();\n>>>>    \t\tpipe_->availableStatBuffers_.pop();\n>>>> +\n>>>> +\t\tif (pipe_->useDewarper_) {\n>>>> +\t\t\tmainPathBuffer = pipe_->availableMainPathBuffers_.front();\n>>>> +\t\t\tpipe_->availableMainPathBuffers_.pop();\n>>>> +\t\t}\n>>>>    \t}\n>>>> -\tFrameBuffer *mainPathBuffer = request->findBuffer(&data->mainPathStream_);\n>>>> -\tFrameBuffer *selfPathBuffer = request->findBuffer(&data->selfPathStream_);\n>>>> +\tif (!mainPathBuffer)\n>>>> +\t\tmainPathBuffer = request->findBuffer(&data->mainPathStream_);\n>>>> +\tselfPathBuffer = request->findBuffer(&data->selfPathStream_);\n>>>>    \tRkISP1FrameInfo *info = new RkISP1FrameInfo;\n>>>> @@ -268,6 +289,7 @@ int RkISP1Frames::destroy(unsigned int frame)\n>>>>    \tpipe_->availableParamBuffers_.push(info->paramBuffer);\n>>>>    \tpipe_->availableStatBuffers_.push(info->statBuffer);\n>>>> +\tpipe_->availableMainPathBuffers_.push(info->mainPathBuffer);\n>>>>    \tframeInfo_.erase(info->frame);\n>>>> @@ -283,6 +305,7 @@ void RkISP1Frames::clear()\n>>>>    \t\tpipe_->availableParamBuffers_.push(info->paramBuffer);\n>>>>    \t\tpipe_->availableStatBuffers_.push(info->statBuffer);\n>>>> +\t\tpipe_->availableMainPathBuffers_.push(info->mainPathBuffer);\n>>>>    \t\tdelete info;\n>>>>    \t}\n>>>> @@ -607,7 +630,7 @@ CameraConfiguration::Status RkISP1CameraConfiguration::validate()\n>>>>     */\n>>>>    PipelineHandlerRkISP1::PipelineHandlerRkISP1(CameraManager *manager)\n>>>> -\t: PipelineHandler(manager), hasSelfPath_(true)\n>>>> +\t: PipelineHandler(manager), hasSelfPath_(true), useDewarper_(false)\n>>>>    {\n>>>>    }\n>>>> @@ -785,12 +808,26 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n>>>>    \t\t<< \" crop \" << rect;\n>>>>    \tstd::map<unsigned int, IPAStream> streamConfig;\n>>>> +\tstd::vector<std::reference_wrapper<StreamConfiguration>> outputCfgs;\n>>>>    \tfor (const StreamConfiguration &cfg : *config) {\n>>>>    \t\tif (cfg.stream() == &data->mainPathStream_) {\n>>>>    \t\t\tret = mainPath_.configure(cfg, format);\n>>>>    \t\t\tstreamConfig[0] = IPAStream(cfg.pixelFormat,\n>>>>    \t\t\t\t\t\t    cfg.size);\n>>>> +\t\t\t/* Configure dewarp */\n>>>> +\t\t\tif (dewarper_ && !isRaw_) {\n>>>> +\t\t\t\t/*\n>>>> +\t\t\t\t * \\todo Converter::configure() API should be changed\n>>>> +\t\t\t\t * to use std::vector<std::reference_wrapper<const StreamConfiguration>> ?\n>>>> +\t\t\t\t */\n>>>> +\t\t\t\toutputCfgs.push_back(const_cast<StreamConfiguration &>(cfg));\n>>>> +\t\t\t\tdewarper_->configure(cfg, outputCfgs);\n>>>> +\t\t\t\tuseDewarper_ = true;\n>>>> +\t\t\t} else {\n>>>> +\t\t\t\tuseDewarper_ = false;\n>>>> +\t\t\t}\n>>>> +\n>>>>    \t\t} else if (hasSelfPath_) {\n>>>>    \t\t\tret = selfPath_.configure(cfg, format);\n>>>>    \t\t\tstreamConfig[1] = IPAStream(cfg.pixelFormat,\n>>>> @@ -839,6 +876,9 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S\n>>>>    \tRkISP1CameraData *data = cameraData(camera);\n>>>>    \tunsigned int count = stream->configuration().bufferCount;\n>>>> +\tif (useDewarper_)\n>>>> +\t\treturn dewarper_->exportBuffers(&data->mainPathStream_, count, buffers);\n>>>> +\n>>>>    \tif (stream == &data->mainPathStream_)\n>>>>    \t\treturn mainPath_.exportBuffers(count, buffers);\n>>>>    \telse if (hasSelfPath_ && stream == &data->selfPathStream_)\n>>>> @@ -866,6 +906,16 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>>>>    \t\tret = stat_->allocateBuffers(maxCount, &statBuffers_);\n>>>>    \t\tif (ret < 0)\n>>>>    \t\t\tgoto error;\n>>>> +\n>>>> +\t\t/* If the dewarper is being used, allocate internal buffers for ISP */\n>>>> +\t\tif (useDewarper_) {\n>>>> +\t\t\tret = mainPath_.allocateBuffers(maxCount, &mainPathBuffers_);\n>>>> +\t\t\tif (ret < 0)\n>>>> +\t\t\t\tgoto error;\n>>>> +\n>>>> +\t\t\tfor (std::unique_ptr<FrameBuffer> &buffer : mainPathBuffers_)\n>>>> +\t\t\t\tavailableMainPathBuffers_.push(buffer.get());\n>>>> +\t\t}\n>>>>    \t}\n>>>>    \tfor (std::unique_ptr<FrameBuffer> &buffer : paramBuffers_) {\n>>>> @@ -889,6 +939,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>>>>    error:\n>>>>    \tparamBuffers_.clear();\n>>>>    \tstatBuffers_.clear();\n>>>> +\tmainPathBuffers_.clear();\n>>>>    \treturn ret;\n>>>>    }\n>>>> @@ -903,8 +954,12 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n>>>>    \twhile (!availableParamBuffers_.empty())\n>>>>    \t\tavailableParamBuffers_.pop();\n>>>> +\twhile (!availableMainPathBuffers_.empty())\n>>>> +\t\tavailableMainPathBuffers_.pop();\n>>>> +\n>>>>    \tparamBuffers_.clear();\n>>>>    \tstatBuffers_.clear();\n>>>> +\tmainPathBuffers_.clear();\n>>>>    \tstd::vector<unsigned int> ids;\n>>>>    \tfor (IPABuffer &ipabuf : data->ipaBuffers_)\n>>>> @@ -919,6 +974,9 @@ int PipelineHandlerRkISP1::freeBuffers(Camera *camera)\n>>>>    \tif (stat_->releaseBuffers())\n>>>>    \t\tLOG(RkISP1, Error) << \"Failed to release stat buffers\";\n>>>> +\tif (useDewarper_)\n>>>> +\t\tmainPath_.releaseBuffers();\n>>>> +\n>>>>    \treturn 0;\n>>>>    }\n>>>> @@ -961,6 +1019,14 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n>>>>    \t\t\t\t<< \"Failed to start statistics \" << camera->id();\n>>>>    \t\t\treturn ret;\n>>>>    \t\t}\n>>>> +\n>>>> +\t\tif (useDewarper_) {\n>>>> +\t\t\tret = dewarper_->start();\n>>>> +\t\t\tif (ret) {\n>>>> +\t\t\t\tLOG(RkISP1, Error) << \"Failed to start dewarper\";\n>>>> +\t\t\t\treturn ret;\n>>>> +\t\t\t}\n>>>> +\t\t}\n>>>>    \t}\n>>>>    \tif (data->mainPath_->isEnabled()) {\n>>>> @@ -1015,6 +1081,9 @@ void PipelineHandlerRkISP1::stopDevice(Camera *camera)\n>>>>    \t\tif (ret)\n>>>>    \t\t\tLOG(RkISP1, Warning)\n>>>>    \t\t\t\t<< \"Failed to stop parameters for \" << camera->id();\n>>>> +\n>>>> +\t\tif (useDewarper_)\n>>>> +\t\t\tdewarper_->stop();\n>>>>    \t}\n>>>>    \tASSERT(data->queuedRequests_.empty());\n>>>> @@ -1045,6 +1114,14 @@ int PipelineHandlerRkISP1::queueRequestDevice(Camera *camera, Request *request)\n>>>>    \t\t\t\t\t     info->paramBuffer->cookie());\n>>>>    \t}\n>>>> +\tconst auto &crop = request->controls().get(controls::ScalerCrop);\n>>>> +\tif (crop && useDewarper_) {\n>>>> +\t\tRectangle rect = crop.value();\n>>>> +\t\tint ret = dewarper_->setScalerCrop(&data->mainPathStream_, &rect);\n>>>> +\t\tif (!ret)\n>>>> +\t\t\tinfo->scalerCrop = crop;\n>>>> +\t}\n>>>> +\n>>>>    \tdata->frame_++;\n>>>>    \treturn 0;\n>>>> @@ -1110,6 +1187,12 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n>>>>    {\n>>>>    \tControlInfoMap::Map rkisp1Controls;\n>>>> +\tif (dewarper_) {\n>>> Does this not need to check useDewarper_?\n>> useDewarper_ is only true in the configured when a non-RAW stream is\n>> configured. updateControls() is called in configure() and createCamera().\n>>\n>> If I use useDewarper_, scalerCrop control would not be registered for the\n>> camera. The goal here is to register the scaler crop control for the camera\n>> (when being created). To use the dewarper or not, depends on the pipeline\n>> configuration and what has been configured.\n> Hm that's true. Do we have a policy on whether or not to report controls\n> that depend on the configuration? Or maybe the control is reported to\n> exist but the bounds should change depending on the configuration? iirc\n> things like FrameDurationLimits are like that?\n\nYes, the updateControls() also run as part of configure() call. So the \nbounds get updated whenever the configuration changes.\n\n>\n>\n> Paul\n>\n>>\n>>> Otherwise looks good to me.\n>>>\n>>>\n>>> Paul\n>>>\n>>>> +\t\tauto [minCrop, maxCrop] = dewarper_->getCropBounds(&data->mainPathStream_);\n>>>> +\n>>>> +\t\trkisp1Controls[&controls::ScalerCrop] = ControlInfo(minCrop, maxCrop, maxCrop);\n>>>> +\t}\n>>>> +\n>>>>    \t/* Add the IPA registered controls to list of camera controls. */\n>>>>    \tfor (const auto &ipaControl : data->ipaControls_)\n>>>>    \t\trkisp1Controls[ipaControl.first] = ipaControl.second;\n>>>> @@ -1173,6 +1256,7 @@ int PipelineHandlerRkISP1::createCamera(MediaEntity *sensor)\n>>>>    bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n>>>>    {\n>>>> +\tstd::shared_ptr<MediaDevice> dwpMediaDevice;\n>>>>    \tconst MediaPad *pad;\n>>>>    \tDeviceMatch dm(\"rkisp1\");\n>>>> @@ -1237,6 +1321,26 @@ bool PipelineHandlerRkISP1::match(DeviceEnumerator *enumerator)\n>>>>    \tstat_->bufferReady.connect(this, &PipelineHandlerRkISP1::statReady);\n>>>>    \tparam_->bufferReady.connect(this, &PipelineHandlerRkISP1::paramReady);\n>>>> +\t/* If dewarper is present, create its instance. */\n>>>> +\tDeviceMatch dwp(\"dw100\");\n>>>> +\tdwp.add(\"dw100-source\");\n>>>> +\tdwp.add(\"dw100-sink\");\n>>>> +\n>>>> +\tdwpMediaDevice = enumerator->search(dwp);\n>>>> +\tif (dwpMediaDevice) {\n>>>> +\t\tdewarper_ = std::make_unique<ConverterDW100>(std::move(dwpMediaDevice));\n>>>> +\t\tif (dewarper_->isValid()) {\n>>>> +\t\t\tdewarper_->outputBufferReady.connect(\n>>>> +\t\t\t\tthis, &PipelineHandlerRkISP1::dewarpBufferReady);\n>>>> +\n>>>> +\t\t\tLOG(RkISP1, Info) << \"Using DW100 dewarper \" << dewarper_->deviceNode();\n>>>> +\t\t} else {\n>>>> +\t\t\tLOG(RkISP1, Debug) << \"Found DW100 dewarper \" << dewarper_->deviceNode()\n>>>> +\t\t\t\t\t   << \" but invalid\";\n>>>> +\t\t\tdewarper_.reset();\n>>>> +\t\t}\n>>>> +\t}\n>>>> +\n>>>>    \t/*\n>>>>    \t * Enumerate all sensors connected to the ISP and create one\n>>>>    \t * camera instance for each of them.\n>>>> @@ -1283,7 +1387,7 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n>>>>    \t\treturn;\n>>>>    \tconst FrameMetadata &metadata = buffer->metadata();\n>>>> -\tRequest *request = buffer->request();\n>>>> +\tRequest *request = info->request;\n>>>>    \tif (metadata.status != FrameMetadata::FrameCancelled) {\n>>>>    \t\t/*\n>>>> @@ -1300,11 +1404,43 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n>>>>    \t\t\t\tdata->delayedCtrls_->get(metadata.sequence);\n>>>>    \t\t\tdata->ipa_->processStatsBuffer(info->frame, 0, ctrls);\n>>>>    \t\t}\n>>>> +\n>>>>    \t} else {\n>>>>    \t\tif (isRaw_)\n>>>>    \t\t\tinfo->metadataProcessed = true;\n>>>>    \t}\n>>>> +\tif (useDewarper_) {\n>>>> +\t\t/*\n>>>> +\t\t * Queue input and output buffers to the dewarper. The output\n>>>> +\t\t * buffers for the dewarper are the buffers of the request, supplied\n>>>> +\t\t * by the application.\n>>>> +\t\t */\n>>>> +\t\tint ret = dewarper_->queueBuffers(buffer, request->buffers());\n>>>> +\t\tif (ret < 0)\n>>>> +\t\t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n>>>> +\t\t\t\t\t   << strerror(-ret);\n>>>> +\n>>>> +\t\treturn;\n>>>> +\t}\n>>>> +\n>>>> +\tcompleteBuffer(request, buffer);\n>>>> +\ttryCompleteRequest(info);\n>>>> +}\n>>>> +\n>>>> +void PipelineHandlerRkISP1::dewarpBufferReady(FrameBuffer *buffer)\n>>>> +{\n>>>> +\tASSERT(activeCamera_);\n>>>> +\tRkISP1CameraData *data = cameraData(activeCamera_);\n>>>> +\tRequest *request = buffer->request();\n>>>> +\n>>>> +\tRkISP1FrameInfo *info = data->frameInfo_.find(buffer->request());\n>>>> +\tif (!info)\n>>>> +\t\treturn;\n>>>> +\n>>>> +\tif (info->scalerCrop)\n>>>> +\t\trequest->metadata().set(controls::ScalerCrop, info->scalerCrop.value());\n>>>> +\n>>>>    \tcompleteBuffer(request, buffer);\n>>>>    \ttryCompleteRequest(info);\n>>>>    }\n>>>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n>>>> index c49017d1..c96ec1d6 100644\n>>>> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n>>>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.cpp\n>>>> @@ -56,7 +56,7 @@ const std::map<PixelFormat, uint32_t> formatToMediaBus = {\n>>>>    RkISP1Path::RkISP1Path(const char *name, const Span<const PixelFormat> &formats,\n>>>>    \t\t       const Size &minResolution, const Size &maxResolution)\n>>>> -\t: name_(name), running_(false), formats_(formats),\n>>>> +\t: name_(name), running_(false), internalBufs_(false), formats_(formats),\n>>>>    \t  minResolution_(minResolution), maxResolution_(maxResolution),\n>>>>    \t  link_(nullptr)\n>>>>    {\n>>>> @@ -402,10 +402,12 @@ int RkISP1Path::start()\n>>>>    \tif (running_)\n>>>>    \t\treturn -EBUSY;\n>>>> -\t/* \\todo Make buffer count user configurable. */\n>>>> -\tret = video_->importBuffers(RKISP1_BUFFER_COUNT);\n>>>> -\tif (ret)\n>>>> -\t\treturn ret;\n>>>> +\tif (!internalBufs_) {\n>>>> +\t\t/* \\todo Make buffer count user configurable. */\n>>>> +\t\tret = video_->importBuffers(RKISP1_BUFFER_COUNT);\n>>>> +\t\tif (ret)\n>>>> +\t\t\treturn ret;\n>>>> +\t}\n>>>>    \tret = video_->streamOn();\n>>>>    \tif (ret) {\n>>>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1_path.h b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n>>>> index 08edefec..b7fa82d0 100644\n>>>> --- a/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n>>>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1_path.h\n>>>> @@ -55,6 +55,19 @@ public:\n>>>>    \t\treturn video_->exportBuffers(bufferCount, buffers);\n>>>>    \t}\n>>>> +\tint allocateBuffers(unsigned int bufferCount,\n>>>> +\t\t\t    std::vector<std::unique_ptr<FrameBuffer>> *buffers)\n>>>> +\t{\n>>>> +\t\tinternalBufs_ = true;\n>>>> +\t\treturn video_->allocateBuffers(bufferCount, buffers);\n>>>> +\t}\n>>>> +\n>>>> +\tint releaseBuffers()\n>>>> +\t{\n>>>> +\t\tASSERT(internalBufs_);\n>>>> +\t\treturn video_->releaseBuffers();\n>>>> +\t}\n>>>> +\n>>>>    \tint start();\n>>>>    \tvoid stop();\n>>>> @@ -68,6 +81,7 @@ private:\n>>>>    \tconst char *name_;\n>>>>    \tbool running_;\n>>>> +\tbool internalBufs_;\n>>>>    \tconst Span<const PixelFormat> formats_;\n>>>>    \tstd::set<PixelFormat> streamFormats_;\n>>>> -- \n>>>> 2.44.0\n>>>>","headers":{"Return-Path":"<libcamera-devel-bounces@lists.libcamera.org>","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 A67ACBD87C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tTue,  9 Jul 2024 10:22:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5E2A06336D;\n\tTue,  9 Jul 2024 12:22:57 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D0605619B9\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  9 Jul 2024 12:22:55 +0200 (CEST)","from [IPV6:2405:201:2015:f873:c173:4b:4a04:3a21] (unknown\n\t[IPv6:2405:201:2015:f873:c173:4b:4a04:3a21])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C92A61471;\n\tTue,  9 Jul 2024 12:22:22 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"rBTCJw2k\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1720520543;\n\tbh=utT2PFxGhnRxG5Tu+uQPTwLmxYgKaxfAvPNHVMi9KXE=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=rBTCJw2kHs4cUGtJf3vhymQnmWgPNPfN12zrpaojU9sqIXxq81Of66KWKyb6Mygms\n\tsIan+dQ/BAWewsiDKkOZALVWmw7w1KrJvcNxo02G00ARyKajUZFjiO3Y05W/kIXxQ/\n\tkr3QjG4D70TA3cauUWdnom+8NvOwKaZf3ICOOwf4=","Message-ID":"<63a0d465-86ce-4547-8165-27eb5dc87183@ideasonboard.com>","Date":"Tue, 9 Jul 2024 15:52:49 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v4 5/5] libcamera: rkisp1: Plumb the ConverterDW100\n\tconverter","To":"Paul Elder <paul.elder@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","References":"<20240627134656.582462-1-umang.jain@ideasonboard.com>\n\t<20240627134656.582462-6-umang.jain@ideasonboard.com>\n\t<ZoQEhp5_9ErfaoSA@pyrite.rasen.tech>\n\t<1c3c8828-17db-4253-b852-85eead2e9217@ideasonboard.com>\n\t<ZoUBVfI87oLDYHTl@pyrite.rasen.tech>","Content-Language":"en-US","From":"Umang Jain <umang.jain@ideasonboard.com>","In-Reply-To":"<ZoUBVfI87oLDYHTl@pyrite.rasen.tech>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>"}}]