[{"id":31350,"web_url":"https://patchwork.libcamera.org/comment/31350/","msgid":"","date":"2024-09-25T07:50:10","subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch. \n\nOn Fri, Sep 06, 2024 at 12:04:44PM +0530, Umang Jain wrote:\n> Plumb the dw100 dewarper as a V4L2M2M converter in the rkisp1 pipeline\n> handler. If the dewarper is found, it is instantiated and buffers are\n> exported from it, instead of RkISP1Path. Internal buffers are allocated\n> for the RkISP1Path in case where dewarper is going to be used.\n> \n> The RKISP1 pipeline handler now supports scaler crop control through\n> the converter. Register the ScalerCrop control for the cameras created\n> in the RKISP1 pipeline handler.\n> \n> Signed-off-by: Umang Jain \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 183 ++++++++++++++++++++++-\n> 1 file changed, 176 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 651258e3..0a794d63 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -6,10 +6,12 @@\n> */\n> \n> #include \n> +#include \n> #include \n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -32,6 +34,7 @@\n> \n> #include \"libcamera/internal/camera.h\"\n> #include \"libcamera/internal/camera_sensor.h\"\n> +#include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n> #include \"libcamera/internal/delayed_controls.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> #include \"libcamera/internal/framebuffer.h\"\n> @@ -180,6 +183,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> @@ -199,6 +203,15 @@ private:\n> \tRkISP1MainPath mainPath_;\n> \tRkISP1SelfPath selfPath_;\n> \n> +\tstd::unique_ptr dewarper_;\n> +\tbool useDewarper_;\n> +\n> +\tstd::optional activeCrop_;\n> +\n> +\t/* Internal buffers used when dewarper is being used */\n> +\tstd::vector> mainPathBuffers_;\n> +\tstd::queue availableMainPathBuffers_;\n> +\n> \tstd::vector> paramBuffers_;\n> \tstd::vector> statBuffers_;\n> \tstd::queue availableParamBuffers_;\n> @@ -221,6 +234,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> \n> \tif (!isRaw) {\n> \t\tif (pipe_->availableParamBuffers_.empty()) {\n> @@ -238,10 +253,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> @@ -267,6 +288,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> @@ -282,6 +304,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> @@ -600,7 +623,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> @@ -778,12 +801,19 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \t\t<< \" crop \" << rect;\n> \n> \tstd::map streamConfig;\n> +\tstd::vector> 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\toutputCfgs.push_back(const_cast(cfg));\n> +\t\t\t\tret = dewarper_->configure(cfg, outputCfgs);\n> +\t\t\t\tuseDewarper_ = ret ? false : true;\n> +\t\t\t}\n> \t\t} else if (hasSelfPath_) {\n> \t\t\tret = selfPath_.configure(cfg, format);\n> \t\t\tstreamConfig[1] = IPAStream(cfg.pixelFormat,\n> @@ -833,10 +863,19 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S\n> \tRkISP1CameraData *data = cameraData(camera);\n> \tunsigned int count = stream->configuration().bufferCount;\n> \n> -\tif (stream == &data->mainPathStream_)\n> -\t\treturn mainPath_.exportBuffers(count, buffers);\n> -\telse if (hasSelfPath_ && stream == &data->selfPathStream_)\n> +\tif (stream == &data->mainPathStream_) {\n> +\t\t/*\n> +\t\t * Currently, i.MX8MP is the only platform with DW100 dewarper.\n> +\t\t * It has mainpath and no self path. Hence, export buffers from\n> +\t\t * dewarper just for the main path stream, for now.\n> +\t\t */\n> +\t\tif (useDewarper_)\n> +\t\t\treturn dewarper_->exportBuffers(&data->mainPathStream_, count, buffers);\n> +\t\telse\n> +\t\t\treturn mainPath_.exportBuffers(count, buffers);\n> +\t} else if (hasSelfPath_ && stream == &data->selfPathStream_) {\n> \t\treturn selfPath_.exportBuffers(count, buffers);\n> +\t}\n> \n> \treturn -EINVAL;\n> }\n> @@ -860,6 +899,16 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> \t\tret = stat_->allocateBuffers(maxCount, &statBuffers_);\n\nIs there a way for the user to prevent usage of the dewarper? It\nincreases latency and doubles the required buffers. Now that I write\nabout it. To configure actual lens dewarping I envision something like a\n\"dewarp\" algorithm in the tuning file. Maybe that is the best place for\nthat decision...\n\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_.exportBuffers(maxCount, &mainPathBuffers_);\n> +\t\t\tif (ret < 0)\n> +\t\t\t\tgoto error;\n> +\n> +\t\t\tfor (std::unique_ptr &buffer : mainPathBuffers_)\n> +\t\t\t\tavailableMainPathBuffers_.push(buffer.get());\n> +\t\t}\n> \t}\n> \n> \tfor (std::unique_ptr &buffer : paramBuffers_) {\n> @@ -883,6 +932,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> error:\n> \tparamBuffers_.clear();\n> \tstatBuffers_.clear();\n> +\tmainPathBuffers_.clear();\n> \n> \treturn ret;\n> }\n> @@ -897,8 +947,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 ids;\n> \tfor (IPABuffer &ipabuf : data->ipaBuffers_)\n> @@ -954,6 +1008,15 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n> \t\t\treturn ret;\n> \t\t}\n> \t\tactions += [&]() { stat_->streamOff(); };\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\tactions += [&]() { dewarper_->stop(); };\n> \t}\n> \n> \tif (data->mainPath_->isEnabled()) {\n> @@ -1000,6 +1063,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> @@ -1110,6 +1176,16 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n> {\n> \tControlInfoMap::Map controls;\n> \n> +\tif (dewarper_) {\n> +\t\tstd::pair cropLimits =\n> +\t\t\tdewarper_->inputCropBounds(&data->mainPathStream_);\n> +\n> +\t\tcontrols[&controls::ScalerCrop] = ControlInfo(cropLimits.first,\n> +\t\t\t\t\t\t\t cropLimits.second,\n> +\t\t\t\t\t\t\t cropLimits.second);\n> +\t\tactiveCrop_ = cropLimits.second;\n> +\t}\n> +\n\nI'm a bit uncertain here. The current dewarper kernel driver produces\nquite unexpected and difficult to explain results (modified crop\nrectangle, changed pixel aspect ratio). So to me it feels like we\nshouldn't expose that hardware control directly to the user, but do an\ninternal implementation (based on the same libcamera control). Could we\nsplit just the logic that adds the control into a separate patch?\n\nAnother approach would be to merge it in and improve later. But then we\nmight break the interface for users of the feature... I lean towards\nsplitting the control and merging the rest. In that case we should\ndisable the dewarper by default, as the user has no added benefit\nwithout the control. Still I think all the plumbing should definitely\ngo in.\n\n> \t/* Add the IPA registered controls to list of camera controls. */\n> \tfor (const auto &ipaControl : data->ipaControls_)\n> \t\tcontrols[ipaControl.first] = ipaControl.second;\n> @@ -1236,6 +1312,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> +\tstd::shared_ptr dwpMediaDevice = enumerator->search(dwp);\n> +\tif (dwpMediaDevice) {\n> +\t\tdewarper_ = std::make_unique(dwpMediaDevice.get());\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, Warning) << \"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> @@ -1282,7 +1378,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> @@ -1304,6 +1400,79 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> \t\t\tinfo->metadataProcessed = true;\n> \t}\n> \n> +\tif (!useDewarper_) {\n> +\t\tcompleteBuffer(request, buffer);\n> +\t\ttryCompleteRequest(info);\n> +\n> +\t\treturn;\n> +\t}\n> +\n> +\t/* Do not queue cancelled frames to dewarper. */\n> +\tif (metadata.status == FrameMetadata::FrameCancelled) {\n> +\t\t/*\n> +\t\t * i.MX8MP is the only known platform with dewarper. It has\n> +\t\t * no self path. Hence, only main path buffer completion is\n> +\t\t * required.\n> +\t\t *\n> +\t\t * Also, we cannot completeBuffer(request, buffer) as buffer\n> +\t\t * here, is an internal buffer (between ISP and dewarper) and\n> +\t\t * is not associated to the any specific request. The request\n> +\t\t * buffer associated with main path stream is the one that\n> +\t\t * is required to be completed (not the internal buffer).\n> +\t\t */\n> +\t\tfor (auto it : request->buffers()) {\n> +\t\t\tif (it.first == &data->mainPathStream_)\n> +\t\t\t\tcompleteBuffer(request, it.second);\n> +\t\t}\n> +\n> +\t\ttryCompleteRequest(info);\n> +\t\treturn;\n> +\t}\n> +\n> +\t/* Handle scaler crop control. */\n> +\tconst auto &crop = request->controls().get(controls::ScalerCrop);\n> +\tif (crop) {\n> +\t\tRectangle appliedRect = crop.value();\n> +\n> +\t\tint ret = dewarper_->setInputCrop(&data->mainPathStream_,\n> +\t\t\t\t\t\t &appliedRect);\n> +\t\tif (!ret && appliedRect != crop.value()) {\n> +\t\t\t/*\n> +\t\t\t * If the rectangle is changed by setInputCrop on the\n> +\t\t\t * dewarper, log a debug message and cache the actual\n> +\t\t\t * applied rectangle for metadata reporting.\n> +\t\t\t */\n> +\t\t\tLOG(RkISP1, Debug)\n> +\t\t\t\t<< \"Applied rectangle \" << appliedRect.toString()\n> +\t\t\t\t<< \" differs from requested \" << crop.value().toString();\n> +\t\t}\n> +\n> +\t\tactiveCrop_ = appliedRect;\n> +\t}\n> +\n> +\t/*\n> +\t * Queue input and output buffers to the dewarper. The output\n> +\t * buffers for the dewarper are the buffers of the request, supplied\n> +\t * by the application.\n> +\t */\n> +\tint ret = dewarper_->queueBuffers(buffer, request->buffers());\n> +\tif (ret < 0)\n> +\t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n> +\t\t\t\t << strerror(-ret);\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> +\trequest->metadata().set(controls::ScalerCrop, activeCrop_.value());\n\nI couldn't easily see if there are cases where more than one buffer are\nqueued to the dewarper. In that case the activeCrop_ could be the one\napplied to the next frame. If it is ensured that that is not the case it\nmight be worth a comment.\n\nBest regards,\nStefan\n\n> +\n> \tcompleteBuffer(request, buffer);\n> \ttryCompleteRequest(info);\n> }\n> -- \n> 2.45.0\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 ADF12C0F1B\n\tfor ;\n\tWed, 25 Sep 2024 07:50:16 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 859A16350F;\n\tWed, 25 Sep 2024 09:50:15 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B4CAB634F4\n\tfor ;\n\tWed, 25 Sep 2024 09:50:13 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:8d08:90cf:e8f9:5de7])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 3199F5B2;\n\tWed, 25 Sep 2024 09:48:46 +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=\"JPvClSio\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727250526;\n\tbh=OyUMi9JXjSWnhnapPCEH7DUPF/3tZTxsPDSQ++ringY=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=JPvClSio/RBl6qTsOTxvqK/VYMQtvcTkB8EjYmNEe5h/3Q2hSlcL/B3y9xmUFgL+8\n\tLxeuwu8ZD8JAbI4s/rXzsGSaN4OgoSXskmBZ0aqrTjHkv9M5M0XFw3jz5mBHdMXk+s\n\tfcGZjf878kQcWSWeU7Z9Gv2beqtUCdk67dT6Rszs=","Date":"Wed, 25 Sep 2024 09:50:10 +0200","From":"Stefan Klug ","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","Message-ID":"","References":"<20240906063444.32772-1-umang.jain@ideasonboard.com>\n\t<20240906063444.32772-5-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240906063444.32772-5-umang.jain@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31351,"web_url":"https://patchwork.libcamera.org/comment/31351/","msgid":"<42f61999-a99c-4b55-aa84-e582a9da939a@ideasonboard.com>","date":"2024-09-25T07:59:42","subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Stefan,\n\nOn 25/09/24 1:20 pm, Stefan Klug wrote:\n> Hi Umang,\n>\n> Thank you for the patch.\n>\n> On Fri, Sep 06, 2024 at 12:04:44PM +0530, Umang Jain wrote:\n>> Plumb the dw100 dewarper as a V4L2M2M converter in the rkisp1 pipeline\n>> handler. If the dewarper is found, it is instantiated and buffers are\n>> exported from it, instead of RkISP1Path. Internal buffers are allocated\n>> for the RkISP1Path in case where dewarper is going to be used.\n>>\n>> The RKISP1 pipeline handler now supports scaler crop control through\n>> the converter. Register the ScalerCrop control for the cameras created\n>> in the RKISP1 pipeline handler.\n>>\n>> Signed-off-by: Umang Jain \n>> ---\n>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 183 ++++++++++++++++++++++-\n>> 1 file changed, 176 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index 651258e3..0a794d63 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -6,10 +6,12 @@\n>> */\n>> \n>> #include \n>> +#include \n>> #include \n>> #include \n>> #include \n>> #include \n>> +#include \n>> \n>> #include \n>> #include \n>> @@ -32,6 +34,7 @@\n>> \n>> #include \"libcamera/internal/camera.h\"\n>> #include \"libcamera/internal/camera_sensor.h\"\n>> +#include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n>> #include \"libcamera/internal/delayed_controls.h\"\n>> #include \"libcamera/internal/device_enumerator.h\"\n>> #include \"libcamera/internal/framebuffer.h\"\n>> @@ -180,6 +183,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>> @@ -199,6 +203,15 @@ private:\n>> \tRkISP1MainPath mainPath_;\n>> \tRkISP1SelfPath selfPath_;\n>> \n>> +\tstd::unique_ptr dewarper_;\n>> +\tbool useDewarper_;\n>> +\n>> +\tstd::optional activeCrop_;\n>> +\n>> +\t/* Internal buffers used when dewarper is being used */\n>> +\tstd::vector> mainPathBuffers_;\n>> +\tstd::queue availableMainPathBuffers_;\n>> +\n>> \tstd::vector> paramBuffers_;\n>> \tstd::vector> statBuffers_;\n>> \tstd::queue availableParamBuffers_;\n>> @@ -221,6 +234,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>> \n>> \tif (!isRaw) {\n>> \t\tif (pipe_->availableParamBuffers_.empty()) {\n>> @@ -238,10 +253,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>> @@ -267,6 +288,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>> @@ -282,6 +304,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>> @@ -600,7 +623,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>> @@ -778,12 +801,19 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n>> \t\t<< \" crop \" << rect;\n>> \n>> \tstd::map streamConfig;\n>> +\tstd::vector> 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\toutputCfgs.push_back(const_cast(cfg));\n>> +\t\t\t\tret = dewarper_->configure(cfg, outputCfgs);\n>> +\t\t\t\tuseDewarper_ = ret ? false : true;\n>> +\t\t\t}\n>> \t\t} else if (hasSelfPath_) {\n>> \t\t\tret = selfPath_.configure(cfg, format);\n>> \t\t\tstreamConfig[1] = IPAStream(cfg.pixelFormat,\n>> @@ -833,10 +863,19 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S\n>> \tRkISP1CameraData *data = cameraData(camera);\n>> \tunsigned int count = stream->configuration().bufferCount;\n>> \n>> -\tif (stream == &data->mainPathStream_)\n>> -\t\treturn mainPath_.exportBuffers(count, buffers);\n>> -\telse if (hasSelfPath_ && stream == &data->selfPathStream_)\n>> +\tif (stream == &data->mainPathStream_) {\n>> +\t\t/*\n>> +\t\t * Currently, i.MX8MP is the only platform with DW100 dewarper.\n>> +\t\t * It has mainpath and no self path. Hence, export buffers from\n>> +\t\t * dewarper just for the main path stream, for now.\n>> +\t\t */\n>> +\t\tif (useDewarper_)\n>> +\t\t\treturn dewarper_->exportBuffers(&data->mainPathStream_, count, buffers);\n>> +\t\telse\n>> +\t\t\treturn mainPath_.exportBuffers(count, buffers);\n>> +\t} else if (hasSelfPath_ && stream == &data->selfPathStream_) {\n>> \t\treturn selfPath_.exportBuffers(count, buffers);\n>> +\t}\n>> \n>> \treturn -EINVAL;\n>> }\n>> @@ -860,6 +899,16 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>> \t\tret = stat_->allocateBuffers(maxCount, &statBuffers_);\n> Is there a way for the user to prevent usage of the dewarper? It\n> increases latency and doubles the required buffers. Now that I write\n> about it. To configure actual lens dewarping I envision something like a\n> \"dewarp\" algorithm in the tuning file. Maybe that is the best place for\n> that decision...\n\nI think somewhere in the previous interations, we decided that if the \ndewarper is present, it should be discovered and used. At that point, we \nfailed to justify any use cases, where a user might have to disable the \ndewarper 'intentionally'.\n\nIf there is use case, we can surely allow disabling the dewarper (and \nshouldn't be a tedious patch on top, I believe). But, The justification \nhas to be there.\n\n>\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_.exportBuffers(maxCount, &mainPathBuffers_);\n>> +\t\t\tif (ret < 0)\n>> +\t\t\t\tgoto error;\n>> +\n>> +\t\t\tfor (std::unique_ptr &buffer : mainPathBuffers_)\n>> +\t\t\t\tavailableMainPathBuffers_.push(buffer.get());\n>> +\t\t}\n>> \t}\n>> \n>> \tfor (std::unique_ptr &buffer : paramBuffers_) {\n>> @@ -883,6 +932,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>> error:\n>> \tparamBuffers_.clear();\n>> \tstatBuffers_.clear();\n>> +\tmainPathBuffers_.clear();\n>> \n>> \treturn ret;\n>> }\n>> @@ -897,8 +947,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 ids;\n>> \tfor (IPABuffer &ipabuf : data->ipaBuffers_)\n>> @@ -954,6 +1008,15 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n>> \t\t\treturn ret;\n>> \t\t}\n>> \t\tactions += [&]() { stat_->streamOff(); };\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\tactions += [&]() { dewarper_->stop(); };\n>> \t}\n>> \n>> \tif (data->mainPath_->isEnabled()) {\n>> @@ -1000,6 +1063,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>> @@ -1110,6 +1176,16 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n>> {\n>> \tControlInfoMap::Map controls;\n>> \n>> +\tif (dewarper_) {\n>> +\t\tstd::pair cropLimits =\n>> +\t\t\tdewarper_->inputCropBounds(&data->mainPathStream_);\n>> +\n>> +\t\tcontrols[&controls::ScalerCrop] = ControlInfo(cropLimits.first,\n>> +\t\t\t\t\t\t\t cropLimits.second,\n>> +\t\t\t\t\t\t\t cropLimits.second);\n>> +\t\tactiveCrop_ = cropLimits.second;\n>> +\t}\n>> +\n> I'm a bit uncertain here. The current dewarper kernel driver produces\n> quite unexpected and difficult to explain results (modified crop\n> rectangle, changed pixel aspect ratio). So to me it feels like we\n> shouldn't expose that hardware control directly to the user, but do an\n> internal implementation (based on the same libcamera control). Could we\n> split just the logic that adds the control into a separate patch?\n\nI don't understand internal implementation based on same libcamera \ncontrol? How would that look like?\n\nSo, you mean to say, we don't feed any scaler crop rectangles from \napplication, but do it internally ?\n>\n> Another approach would be to merge it in and improve later. But then we\n> might break the interface for users of the feature... I lean towards\n> splitting the control and merging the rest. In that case we should\n> disable the dewarper by default, as the user has no added benefit\n> without the control. Still I think all the plumbing should definitely\n> go in.\n\nI don't foresee breaking the interface for users. But there can be \nimprovements on top of this (for e.g. mapping the V4L2 Selection flags) \nand letting the application control that via an interface.\n\nhttps://docs.kernel.org/userspace-api/media/v4l/v4l2-selection-flags.html\n\n>\n>> \t/* Add the IPA registered controls to list of camera controls. */\n>> \tfor (const auto &ipaControl : data->ipaControls_)\n>> \t\tcontrols[ipaControl.first] = ipaControl.second;\n>> @@ -1236,6 +1312,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>> +\tstd::shared_ptr dwpMediaDevice = enumerator->search(dwp);\n>> +\tif (dwpMediaDevice) {\n>> +\t\tdewarper_ = std::make_unique(dwpMediaDevice.get());\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, Warning) << \"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>> @@ -1282,7 +1378,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>> @@ -1304,6 +1400,79 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n>> \t\t\tinfo->metadataProcessed = true;\n>> \t}\n>> \n>> +\tif (!useDewarper_) {\n>> +\t\tcompleteBuffer(request, buffer);\n>> +\t\ttryCompleteRequest(info);\n>> +\n>> +\t\treturn;\n>> +\t}\n>> +\n>> +\t/* Do not queue cancelled frames to dewarper. */\n>> +\tif (metadata.status == FrameMetadata::FrameCancelled) {\n>> +\t\t/*\n>> +\t\t * i.MX8MP is the only known platform with dewarper. It has\n>> +\t\t * no self path. Hence, only main path buffer completion is\n>> +\t\t * required.\n>> +\t\t *\n>> +\t\t * Also, we cannot completeBuffer(request, buffer) as buffer\n>> +\t\t * here, is an internal buffer (between ISP and dewarper) and\n>> +\t\t * is not associated to the any specific request. The request\n>> +\t\t * buffer associated with main path stream is the one that\n>> +\t\t * is required to be completed (not the internal buffer).\n>> +\t\t */\n>> +\t\tfor (auto it : request->buffers()) {\n>> +\t\t\tif (it.first == &data->mainPathStream_)\n>> +\t\t\t\tcompleteBuffer(request, it.second);\n>> +\t\t}\n>> +\n>> +\t\ttryCompleteRequest(info);\n>> +\t\treturn;\n>> +\t}\n>> +\n>> +\t/* Handle scaler crop control. */\n>> +\tconst auto &crop = request->controls().get(controls::ScalerCrop);\n>> +\tif (crop) {\n>> +\t\tRectangle appliedRect = crop.value();\n>> +\n>> +\t\tint ret = dewarper_->setInputCrop(&data->mainPathStream_,\n>> +\t\t\t\t\t\t &appliedRect);\n>> +\t\tif (!ret && appliedRect != crop.value()) {\n>> +\t\t\t/*\n>> +\t\t\t * If the rectangle is changed by setInputCrop on the\n>> +\t\t\t * dewarper, log a debug message and cache the actual\n>> +\t\t\t * applied rectangle for metadata reporting.\n>> +\t\t\t */\n>> +\t\t\tLOG(RkISP1, Debug)\n>> +\t\t\t\t<< \"Applied rectangle \" << appliedRect.toString()\n>> +\t\t\t\t<< \" differs from requested \" << crop.value().toString();\n>> +\t\t}\n>> +\n>> +\t\tactiveCrop_ = appliedRect;\n>> +\t}\n>> +\n>> +\t/*\n>> +\t * Queue input and output buffers to the dewarper. The output\n>> +\t * buffers for the dewarper are the buffers of the request, supplied\n>> +\t * by the application.\n>> +\t */\n>> +\tint ret = dewarper_->queueBuffers(buffer, request->buffers());\n>> +\tif (ret < 0)\n>> +\t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n>> +\t\t\t\t << strerror(-ret);\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>> +\trequest->metadata().set(controls::ScalerCrop, activeCrop_.value());\n> I couldn't easily see if there are cases where more than one buffer are\n> queued to the dewarper. In that case the activeCrop_ could be the one\n> applied to the next frame. If it is ensured that that is not the case it\n> might be worth a comment.\n>\n> Best regards,\n> Stefan\n>\n>> +\n>> \tcompleteBuffer(request, buffer);\n>> \ttryCompleteRequest(info);\n>> }\n>> -- \n>> 2.45.0\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 318DCC3257\n\tfor ;\n\tWed, 25 Sep 2024 07:59:50 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 7BE446350F;\n\tWed, 25 Sep 2024 09:59:49 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id D877D634F4\n\tfor ;\n\tWed, 25 Sep 2024 09:59:47 +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 DF0875B2;\n\tWed, 25 Sep 2024 09:58:19 +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=\"qfquMLOc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727251100;\n\tbh=DBF3xykCcLV/9g7u+Qa2ENZqJ/2U8wGttHC35+7niN4=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=qfquMLOcaRDHkDHIRo0mc8jik6KJNm/ScM5woI/R0QPCKFsIMjB/KpnDjDjb2W7GQ\n\t3tBTH+qBFU/h7tIcFlMemPbLJbU7XbgPrx3FgbLWJ79yJbKwDMdwzlWTaAhXfAdn1u\n\ttcy0CoQoKZOpK9ebZnkvCcxYiq+H6L6Rok7zaVTQ=","Message-ID":"<42f61999-a99c-4b55-aa84-e582a9da939a@ideasonboard.com>","Date":"Wed, 25 Sep 2024 13:29:42 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","To":"Stefan Klug ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20240906063444.32772-1-umang.jain@ideasonboard.com>\n\t<20240906063444.32772-5-umang.jain@ideasonboard.com>\n\t","Content-Language":"en-US","From":"Umang Jain ","In-Reply-To":"","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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31360,"web_url":"https://patchwork.libcamera.org/comment/31360/","msgid":"<20240925184245.GG27666@pendragon.ideasonboard.com>","date":"2024-09-25T18:42:45","subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Umang,\n\nThank you for the patch.\n\nOn Fri, Sep 06, 2024 at 12:04:44PM +0530, Umang Jain wrote:\n> Plumb the dw100 dewarper as a V4L2M2M converter in the rkisp1 pipeline\n> handler. If the dewarper is found, it is instantiated and buffers are\n> exported from it, instead of RkISP1Path. Internal buffers are allocated\n> for the RkISP1Path in case where dewarper is going to be used.\n> \n> The RKISP1 pipeline handler now supports scaler crop control through\n> the converter. Register the ScalerCrop control for the cameras created\n> in the RKISP1 pipeline handler.\n> \n> Signed-off-by: Umang Jain \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 183 ++++++++++++++++++++++-\n> 1 file changed, 176 insertions(+), 7 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 651258e3..0a794d63 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -6,10 +6,12 @@\n> */\n> \n> #include \n> +#include \n> #include \n> #include \n> #include \n> #include \n> +#include \n> \n> #include \n> #include \n> @@ -32,6 +34,7 @@\n> \n> #include \"libcamera/internal/camera.h\"\n> #include \"libcamera/internal/camera_sensor.h\"\n> +#include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n> #include \"libcamera/internal/delayed_controls.h\"\n> #include \"libcamera/internal/device_enumerator.h\"\n> #include \"libcamera/internal/framebuffer.h\"\n> @@ -180,6 +183,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> @@ -199,6 +203,15 @@ private:\n> \tRkISP1MainPath mainPath_;\n> \tRkISP1SelfPath selfPath_;\n> \n> +\tstd::unique_ptr dewarper_;\n> +\tbool useDewarper_;\n> +\n> +\tstd::optional activeCrop_;\n> +\n> +\t/* Internal buffers used when dewarper is being used */\n> +\tstd::vector> mainPathBuffers_;\n> +\tstd::queue availableMainPathBuffers_;\n> +\n> \tstd::vector> paramBuffers_;\n> \tstd::vector> statBuffers_;\n> \tstd::queue availableParamBuffers_;\n> @@ -221,6 +234,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> \n> \tif (!isRaw) {\n> \t\tif (pipe_->availableParamBuffers_.empty()) {\n> @@ -238,10 +253,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> @@ -267,6 +288,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> @@ -282,6 +304,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> @@ -600,7 +623,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> @@ -778,12 +801,19 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> \t\t<< \" crop \" << rect;\n> \n> \tstd::map streamConfig;\n> +\tstd::vector> 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\toutputCfgs.push_back(const_cast(cfg));\n> +\t\t\t\tret = dewarper_->configure(cfg, outputCfgs);\n> +\t\t\t\tuseDewarper_ = ret ? false : true;\n> +\t\t\t}\n> \t\t} else if (hasSelfPath_) {\n> \t\t\tret = selfPath_.configure(cfg, format);\n> \t\t\tstreamConfig[1] = IPAStream(cfg.pixelFormat,\n> @@ -833,10 +863,19 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S\n> \tRkISP1CameraData *data = cameraData(camera);\n> \tunsigned int count = stream->configuration().bufferCount;\n> \n> -\tif (stream == &data->mainPathStream_)\n> -\t\treturn mainPath_.exportBuffers(count, buffers);\n> -\telse if (hasSelfPath_ && stream == &data->selfPathStream_)\n> +\tif (stream == &data->mainPathStream_) {\n> +\t\t/*\n> +\t\t * Currently, i.MX8MP is the only platform with DW100 dewarper.\n> +\t\t * It has mainpath and no self path. Hence, export buffers from\n> +\t\t * dewarper just for the main path stream, for now.\n> +\t\t */\n> +\t\tif (useDewarper_)\n> +\t\t\treturn dewarper_->exportBuffers(&data->mainPathStream_, count, buffers);\n> +\t\telse\n> +\t\t\treturn mainPath_.exportBuffers(count, buffers);\n> +\t} else if (hasSelfPath_ && stream == &data->selfPathStream_) {\n> \t\treturn selfPath_.exportBuffers(count, buffers);\n> +\t}\n> \n> \treturn -EINVAL;\n> }\n> @@ -860,6 +899,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_.exportBuffers(maxCount, &mainPathBuffers_);\n> +\t\t\tif (ret < 0)\n> +\t\t\t\tgoto error;\n> +\n> +\t\t\tfor (std::unique_ptr &buffer : mainPathBuffers_)\n> +\t\t\t\tavailableMainPathBuffers_.push(buffer.get());\n> +\t\t}\n> \t}\n> \n> \tfor (std::unique_ptr &buffer : paramBuffers_) {\n> @@ -883,6 +932,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> error:\n> \tparamBuffers_.clear();\n> \tstatBuffers_.clear();\n> +\tmainPathBuffers_.clear();\n> \n> \treturn ret;\n> }\n> @@ -897,8 +947,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 ids;\n> \tfor (IPABuffer &ipabuf : data->ipaBuffers_)\n> @@ -954,6 +1008,15 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n> \t\t\treturn ret;\n> \t\t}\n> \t\tactions += [&]() { stat_->streamOff(); };\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\tactions += [&]() { dewarper_->stop(); };\n> \t}\n> \n> \tif (data->mainPath_->isEnabled()) {\n> @@ -1000,6 +1063,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> @@ -1110,6 +1176,16 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n> {\n> \tControlInfoMap::Map controls;\n> \n> +\tif (dewarper_) {\n> +\t\tstd::pair cropLimits =\n> +\t\t\tdewarper_->inputCropBounds(&data->mainPathStream_);\n> +\n> +\t\tcontrols[&controls::ScalerCrop] = ControlInfo(cropLimits.first,\n> +\t\t\t\t\t\t\t cropLimits.second,\n> +\t\t\t\t\t\t\t cropLimits.second);\n> +\t\tactiveCrop_ = cropLimits.second;\n> +\t}\n> +\n> \t/* Add the IPA registered controls to list of camera controls. */\n> \tfor (const auto &ipaControl : data->ipaControls_)\n> \t\tcontrols[ipaControl.first] = ipaControl.second;\n> @@ -1236,6 +1312,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> +\tstd::shared_ptr dwpMediaDevice = enumerator->search(dwp);\n> +\tif (dwpMediaDevice) {\n> +\t\tdewarper_ = std::make_unique(dwpMediaDevice.get());\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\n\t\t\tLOG(RkISP1, Info)\n\t\t\t\t<< \"Using DW100 dewarper \" << dewarper_->deviceNode();\n\t\t\t\n> +\t\t} else {\n> +\t\t\tLOG(RkISP1, Warning) << \"Found DW100 dewarper \" << dewarper_->deviceNode()\n> +\t\t\t\t\t << \" but invalid\";\n\n\t\t\tLOG(RkISP1, Warning)\n\t\t\t\t<< \"Found DW100 dewarper \" << dewarper_->deviceNode()\n\t\t\t\t<< \" but invalid\";\n\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> @@ -1282,7 +1378,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> @@ -1304,6 +1400,79 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> \t\t\tinfo->metadataProcessed = true;\n> \t}\n> \n> +\tif (!useDewarper_) {\n> +\t\tcompleteBuffer(request, buffer);\n> +\t\ttryCompleteRequest(info);\n> +\n> +\t\treturn;\n> +\t}\n> +\n> +\t/* Do not queue cancelled frames to dewarper. */\n> +\tif (metadata.status == FrameMetadata::FrameCancelled) {\n> +\t\t/*\n> +\t\t * i.MX8MP is the only known platform with dewarper. It has\n> +\t\t * no self path. Hence, only main path buffer completion is\n> +\t\t * required.\n> +\t\t *\n> +\t\t * Also, we cannot completeBuffer(request, buffer) as buffer\n> +\t\t * here, is an internal buffer (between ISP and dewarper) and\n> +\t\t * is not associated to the any specific request. The request\n> +\t\t * buffer associated with main path stream is the one that\n> +\t\t * is required to be completed (not the internal buffer).\n> +\t\t */\n> +\t\tfor (auto it : request->buffers()) {\n> +\t\t\tif (it.first == &data->mainPathStream_)\n> +\t\t\t\tcompleteBuffer(request, it.second);\n> +\t\t}\n> +\n> +\t\ttryCompleteRequest(info);\n> +\t\treturn;\n> +\t}\n> +\n> +\t/* Handle scaler crop control. */\n> +\tconst auto &crop = request->controls().get(controls::ScalerCrop);\n> +\tif (crop) {\n> +\t\tRectangle appliedRect = crop.value();\n> +\n> +\t\tint ret = dewarper_->setInputCrop(&data->mainPathStream_,\n> +\t\t\t\t\t\t &appliedRect);\n\nThe kernel driver doesn't handle this right :-( There's no\nsynchronization between setting the crop rectangle and running jobs.\nYou'll see glitches when zooming. That will need to be fixed in the\nkernel, and the implementation in libcamera will likely need to change.\nWe can keep this code here for now.\n\n> +\t\tif (!ret && appliedRect != crop.value()) {\n> +\t\t\t/*\n> +\t\t\t * If the rectangle is changed by setInputCrop on the\n> +\t\t\t * dewarper, log a debug message and cache the actual\n> +\t\t\t * applied rectangle for metadata reporting.\n> +\t\t\t */\n> +\t\t\tLOG(RkISP1, Debug)\n> +\t\t\t\t<< \"Applied rectangle \" << appliedRect.toString()\n> +\t\t\t\t<< \" differs from requested \" << crop.value().toString();\n> +\t\t}\n> +\n> +\t\tactiveCrop_ = appliedRect;\n> +\t}\n> +\n> +\t/*\n> +\t * Queue input and output buffers to the dewarper. The output\n> +\t * buffers for the dewarper are the buffers of the request, supplied\n> +\t * by the application.\n> +\t */\n> +\tint ret = dewarper_->queueBuffers(buffer, request->buffers());\n> +\tif (ret < 0)\n> +\t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n> +\t\t\t\t << strerror(-ret);\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> +\trequest->metadata().set(controls::ScalerCrop, activeCrop_.value());\n\nYou're racing with PipelineHandlerRkISP1::bufferReady(), there's no\nguarantee that activeCrop_ will be the right one. In v6 you stored the\nactive crop in the RkISP1FrameInfo, which produced the correct\nbehaviour. Another option would be to set the metadata in\nPipelineHandlerRkISP1::bufferReady(), which would be simpler.\n\n> +\n> \tcompleteBuffer(request, buffer);\n> \ttryCompleteRequest(info);\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 9DFA1C3257\n\tfor ;\n\tWed, 25 Sep 2024 18:42:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 447906350E;\n\tWed, 25 Sep 2024 20:42: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 085ED634F4\n\tfor ;\n\tWed, 25 Sep 2024 20:42:48 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1A46B842;\n\tWed, 25 Sep 2024 20:41: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=\"UgIt7dcD\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727289680;\n\tbh=1+2ZtNfd5Mmmp0hqDXvLZeEOpTyGDrYALzF98TxWQBU=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=UgIt7dcDdssxOXkMtwi55AqgxHxoKhi6u+/CqbOJNOxnn9ZacBEwkZtcRivUFuPff\n\tWjslY5uyZe8hZUxVegQRmqJMUUt+c5izbwRBicw/vxdaIIKcbGNJn2AxSdLm+0ObiZ\n\tXelXQsPxeCxMf6SY3eQi/Mp7r1ftyohHms/x6QG0=","Date":"Wed, 25 Sep 2024 21:42:45 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Cc":"libcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","Message-ID":"<20240925184245.GG27666@pendragon.ideasonboard.com>","References":"<20240906063444.32772-1-umang.jain@ideasonboard.com>\n\t<20240906063444.32772-5-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20240906063444.32772-5-umang.jain@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31361,"web_url":"https://patchwork.libcamera.org/comment/31361/","msgid":"<20240925184632.GA30399@pendragon.ideasonboard.com>","date":"2024-09-25T18:46:32","subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"On Wed, Sep 25, 2024 at 01:29:42PM +0530, Umang Jain wrote:\n> On 25/09/24 1:20 pm, Stefan Klug wrote:\n> > On Fri, Sep 06, 2024 at 12:04:44PM +0530, Umang Jain wrote:\n> >> Plumb the dw100 dewarper as a V4L2M2M converter in the rkisp1 pipeline\n> >> handler. If the dewarper is found, it is instantiated and buffers are\n> >> exported from it, instead of RkISP1Path. Internal buffers are allocated\n> >> for the RkISP1Path in case where dewarper is going to be used.\n> >>\n> >> The RKISP1 pipeline handler now supports scaler crop control through\n> >> the converter. Register the ScalerCrop control for the cameras created\n> >> in the RKISP1 pipeline handler.\n> >>\n> >> Signed-off-by: Umang Jain \n> >> ---\n> >> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 183 ++++++++++++++++++++++-\n> >> 1 file changed, 176 insertions(+), 7 deletions(-)\n> >>\n> >> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> >> index 651258e3..0a794d63 100644\n> >> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> >> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> >> @@ -6,10 +6,12 @@\n> >> */\n> >> \n> >> #include \n> >> +#include \n> >> #include \n> >> #include \n> >> #include \n> >> #include \n> >> +#include \n> >> \n> >> #include \n> >> #include \n> >> @@ -32,6 +34,7 @@\n> >> \n> >> #include \"libcamera/internal/camera.h\"\n> >> #include \"libcamera/internal/camera_sensor.h\"\n> >> +#include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n> >> #include \"libcamera/internal/delayed_controls.h\"\n> >> #include \"libcamera/internal/device_enumerator.h\"\n> >> #include \"libcamera/internal/framebuffer.h\"\n> >> @@ -180,6 +183,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> >> @@ -199,6 +203,15 @@ private:\n> >> \tRkISP1MainPath mainPath_;\n> >> \tRkISP1SelfPath selfPath_;\n> >> \n> >> +\tstd::unique_ptr dewarper_;\n> >> +\tbool useDewarper_;\n> >> +\n> >> +\tstd::optional activeCrop_;\n> >> +\n> >> +\t/* Internal buffers used when dewarper is being used */\n> >> +\tstd::vector> mainPathBuffers_;\n> >> +\tstd::queue availableMainPathBuffers_;\n> >> +\n> >> \tstd::vector> paramBuffers_;\n> >> \tstd::vector> statBuffers_;\n> >> \tstd::queue availableParamBuffers_;\n> >> @@ -221,6 +234,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> >> \n> >> \tif (!isRaw) {\n> >> \t\tif (pipe_->availableParamBuffers_.empty()) {\n> >> @@ -238,10 +253,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> >> @@ -267,6 +288,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> >> @@ -282,6 +304,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> >> @@ -600,7 +623,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> >> @@ -778,12 +801,19 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> >> \t\t<< \" crop \" << rect;\n> >> \n> >> \tstd::map streamConfig;\n> >> +\tstd::vector> 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\toutputCfgs.push_back(const_cast(cfg));\n> >> +\t\t\t\tret = dewarper_->configure(cfg, outputCfgs);\n> >> +\t\t\t\tuseDewarper_ = ret ? false : true;\n> >> +\t\t\t}\n> >> \t\t} else if (hasSelfPath_) {\n> >> \t\t\tret = selfPath_.configure(cfg, format);\n> >> \t\t\tstreamConfig[1] = IPAStream(cfg.pixelFormat,\n> >> @@ -833,10 +863,19 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S\n> >> \tRkISP1CameraData *data = cameraData(camera);\n> >> \tunsigned int count = stream->configuration().bufferCount;\n> >> \n> >> -\tif (stream == &data->mainPathStream_)\n> >> -\t\treturn mainPath_.exportBuffers(count, buffers);\n> >> -\telse if (hasSelfPath_ && stream == &data->selfPathStream_)\n> >> +\tif (stream == &data->mainPathStream_) {\n> >> +\t\t/*\n> >> +\t\t * Currently, i.MX8MP is the only platform with DW100 dewarper.\n> >> +\t\t * It has mainpath and no self path. Hence, export buffers from\n> >> +\t\t * dewarper just for the main path stream, for now.\n> >> +\t\t */\n> >> +\t\tif (useDewarper_)\n> >> +\t\t\treturn dewarper_->exportBuffers(&data->mainPathStream_, count, buffers);\n> >> +\t\telse\n> >> +\t\t\treturn mainPath_.exportBuffers(count, buffers);\n> >> +\t} else if (hasSelfPath_ && stream == &data->selfPathStream_) {\n> >> \t\treturn selfPath_.exportBuffers(count, buffers);\n> >> +\t}\n> >> \n> >> \treturn -EINVAL;\n> >> }\n> >> @@ -860,6 +899,16 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> >> \t\tret = stat_->allocateBuffers(maxCount, &statBuffers_);\n> >\n> > Is there a way for the user to prevent usage of the dewarper? It\n> > increases latency and doubles the required buffers. Now that I write\n> > about it. To configure actual lens dewarping I envision something like a\n> > \"dewarp\" algorithm in the tuning file. Maybe that is the best place for\n> > that decision...\n> \n> I think somewhere in the previous interations, we decided that if the \n> dewarper is present, it should be discovered and used. At that point, we \n> failed to justify any use cases, where a user might have to disable the \n> dewarper 'intentionally'.\n> \n> If there is use case, we can surely allow disabling the dewarper (and \n> shouldn't be a tedious patch on top, I believe). But, The justification \n> has to be there.\n\nIf the camera doesn't have a dewarping map, and if the user doesn't\nrequest digital zoom, then I think the dewarper should be bypassed\ninstead of introducing delays.\n\nI would still like to get to the bottom of using the resizer to\nimplement digital zoom.\n\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_.exportBuffers(maxCount, &mainPathBuffers_);\n> >> +\t\t\tif (ret < 0)\n> >> +\t\t\t\tgoto error;\n> >> +\n> >> +\t\t\tfor (std::unique_ptr &buffer : mainPathBuffers_)\n> >> +\t\t\t\tavailableMainPathBuffers_.push(buffer.get());\n> >> +\t\t}\n> >> \t}\n> >> \n> >> \tfor (std::unique_ptr &buffer : paramBuffers_) {\n> >> @@ -883,6 +932,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> >> error:\n> >> \tparamBuffers_.clear();\n> >> \tstatBuffers_.clear();\n> >> +\tmainPathBuffers_.clear();\n> >> \n> >> \treturn ret;\n> >> }\n> >> @@ -897,8 +947,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 ids;\n> >> \tfor (IPABuffer &ipabuf : data->ipaBuffers_)\n> >> @@ -954,6 +1008,15 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n> >> \t\t\treturn ret;\n> >> \t\t}\n> >> \t\tactions += [&]() { stat_->streamOff(); };\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\tactions += [&]() { dewarper_->stop(); };\n> >> \t}\n> >> \n> >> \tif (data->mainPath_->isEnabled()) {\n> >> @@ -1000,6 +1063,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> >> @@ -1110,6 +1176,16 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n> >> {\n> >> \tControlInfoMap::Map controls;\n> >> \n> >> +\tif (dewarper_) {\n> >> +\t\tstd::pair cropLimits =\n> >> +\t\t\tdewarper_->inputCropBounds(&data->mainPathStream_);\n> >> +\n> >> +\t\tcontrols[&controls::ScalerCrop] = ControlInfo(cropLimits.first,\n> >> +\t\t\t\t\t\t\t cropLimits.second,\n> >> +\t\t\t\t\t\t\t cropLimits.second);\n> >> +\t\tactiveCrop_ = cropLimits.second;\n> >> +\t}\n> >> +\n> >\n> > I'm a bit uncertain here. The current dewarper kernel driver produces\n> > quite unexpected and difficult to explain results (modified crop\n> > rectangle, changed pixel aspect ratio). So to me it feels like we\n> > shouldn't expose that hardware control directly to the user, but do an\n> > internal implementation (based on the same libcamera control). Could we\n> > split just the logic that adds the control into a separate patch?\n> \n> I don't understand internal implementation based on same libcamera \n> control? How would that look like?\n> \n> So, you mean to say, we don't feed any scaler crop rectangles from \n> application, but do it internally ?\n>\n> > Another approach would be to merge it in and improve later. But then we\n> > might break the interface for users of the feature... I lean towards\n> > splitting the control and merging the rest. In that case we should\n> > disable the dewarper by default, as the user has no added benefit\n> > without the control. Still I think all the plumbing should definitely\n> > go in.\n> \n> I don't foresee breaking the interface for users. But there can be \n> improvements on top of this (for e.g. mapping the V4L2 Selection flags) \n> and letting the application control that via an interface.\n> \n> https://docs.kernel.org/userspace-api/media/v4l/v4l2-selection-flags.html\n> \n> >> \t/* Add the IPA registered controls to list of camera controls. */\n> >> \tfor (const auto &ipaControl : data->ipaControls_)\n> >> \t\tcontrols[ipaControl.first] = ipaControl.second;\n> >> @@ -1236,6 +1312,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> >> +\tstd::shared_ptr dwpMediaDevice = enumerator->search(dwp);\n> >> +\tif (dwpMediaDevice) {\n> >> +\t\tdewarper_ = std::make_unique(dwpMediaDevice.get());\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, Warning) << \"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> >> @@ -1282,7 +1378,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> >> @@ -1304,6 +1400,79 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> >> \t\t\tinfo->metadataProcessed = true;\n> >> \t}\n> >> \n> >> +\tif (!useDewarper_) {\n> >> +\t\tcompleteBuffer(request, buffer);\n> >> +\t\ttryCompleteRequest(info);\n> >> +\n> >> +\t\treturn;\n> >> +\t}\n> >> +\n> >> +\t/* Do not queue cancelled frames to dewarper. */\n> >> +\tif (metadata.status == FrameMetadata::FrameCancelled) {\n> >> +\t\t/*\n> >> +\t\t * i.MX8MP is the only known platform with dewarper. It has\n> >> +\t\t * no self path. Hence, only main path buffer completion is\n> >> +\t\t * required.\n> >> +\t\t *\n> >> +\t\t * Also, we cannot completeBuffer(request, buffer) as buffer\n> >> +\t\t * here, is an internal buffer (between ISP and dewarper) and\n> >> +\t\t * is not associated to the any specific request. The request\n> >> +\t\t * buffer associated with main path stream is the one that\n> >> +\t\t * is required to be completed (not the internal buffer).\n> >> +\t\t */\n> >> +\t\tfor (auto it : request->buffers()) {\n> >> +\t\t\tif (it.first == &data->mainPathStream_)\n> >> +\t\t\t\tcompleteBuffer(request, it.second);\n> >> +\t\t}\n> >> +\n> >> +\t\ttryCompleteRequest(info);\n> >> +\t\treturn;\n> >> +\t}\n> >> +\n> >> +\t/* Handle scaler crop control. */\n> >> +\tconst auto &crop = request->controls().get(controls::ScalerCrop);\n> >> +\tif (crop) {\n> >> +\t\tRectangle appliedRect = crop.value();\n> >> +\n> >> +\t\tint ret = dewarper_->setInputCrop(&data->mainPathStream_,\n> >> +\t\t\t\t\t\t &appliedRect);\n> >> +\t\tif (!ret && appliedRect != crop.value()) {\n> >> +\t\t\t/*\n> >> +\t\t\t * If the rectangle is changed by setInputCrop on the\n> >> +\t\t\t * dewarper, log a debug message and cache the actual\n> >> +\t\t\t * applied rectangle for metadata reporting.\n> >> +\t\t\t */\n> >> +\t\t\tLOG(RkISP1, Debug)\n> >> +\t\t\t\t<< \"Applied rectangle \" << appliedRect.toString()\n> >> +\t\t\t\t<< \" differs from requested \" << crop.value().toString();\n> >> +\t\t}\n> >> +\n> >> +\t\tactiveCrop_ = appliedRect;\n> >> +\t}\n> >> +\n> >> +\t/*\n> >> +\t * Queue input and output buffers to the dewarper. The output\n> >> +\t * buffers for the dewarper are the buffers of the request, supplied\n> >> +\t * by the application.\n> >> +\t */\n> >> +\tint ret = dewarper_->queueBuffers(buffer, request->buffers());\n> >> +\tif (ret < 0)\n> >> +\t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n> >> +\t\t\t\t << strerror(-ret);\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> >> +\trequest->metadata().set(controls::ScalerCrop, activeCrop_.value());\n> >\n> > I couldn't easily see if there are cases where more than one buffer are\n> > queued to the dewarper. In that case the activeCrop_ could be the one\n> > applied to the next frame. If it is ensured that that is not the case it\n> > might be worth a comment.\n> >\n> >> +\n> >> \tcompleteBuffer(request, buffer);\n> >> \ttryCompleteRequest(info);\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 EFFF8C3257\n\tfor ;\n\tWed, 25 Sep 2024 18:46:37 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id F33026350E;\n\tWed, 25 Sep 2024 20:46:36 +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 306E5634F4\n\tfor ;\n\tWed, 25 Sep 2024 20:46:35 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-209-231.bb.dnainternet.fi\n\t[81.175.209.231])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 092DFFF1;\n\tWed, 25 Sep 2024 20:45:06 +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=\"Iuj8OrYy\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727289907;\n\tbh=oCi6XdeBHi1//jeE1V3S6v3AxbxkwLfiid+v0XezjWA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Iuj8OrYyghYSlg8nv+5FLblgsfzsaEZ/5M9kPu5BOuW+W1hQjvBJkzeX9QxNJkr3g\n\tDjUNKT0dKS1V7nQ5XivENozjQuIUZ5IzK0FRVdftLdiRBuNO51OclE887CfORCRLF0\n\t071xUIIaJfTEjqKP4gpHoo8UaOvI7xWQnK/Q6NMc=","Date":"Wed, 25 Sep 2024 21:46:32 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Cc":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","Message-ID":"<20240925184632.GA30399@pendragon.ideasonboard.com>","References":"<20240906063444.32772-1-umang.jain@ideasonboard.com>\n\t<20240906063444.32772-5-umang.jain@ideasonboard.com>\n\t\n\t<42f61999-a99c-4b55-aa84-e582a9da939a@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<42f61999-a99c-4b55-aa84-e582a9da939a@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31371,"web_url":"https://patchwork.libcamera.org/comment/31371/","msgid":"","date":"2024-09-26T07:35:18","subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 26/09/24 12:12 am, Laurent Pinchart wrote:\n> Hi Umang,\n>\n> Thank you for the patch.\n>\n> On Fri, Sep 06, 2024 at 12:04:44PM +0530, Umang Jain wrote:\n>> Plumb the dw100 dewarper as a V4L2M2M converter in the rkisp1 pipeline\n>> handler. If the dewarper is found, it is instantiated and buffers are\n>> exported from it, instead of RkISP1Path. Internal buffers are allocated\n>> for the RkISP1Path in case where dewarper is going to be used.\n>>\n>> The RKISP1 pipeline handler now supports scaler crop control through\n>> the converter. Register the ScalerCrop control for the cameras created\n>> in the RKISP1 pipeline handler.\n>>\n>> Signed-off-by: Umang Jain \n>> ---\n>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 183 ++++++++++++++++++++++-\n>> 1 file changed, 176 insertions(+), 7 deletions(-)\n>>\n>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> index 651258e3..0a794d63 100644\n>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>> @@ -6,10 +6,12 @@\n>> */\n>> \n>> #include \n>> +#include \n>> #include \n>> #include \n>> #include \n>> #include \n>> +#include \n>> \n>> #include \n>> #include \n>> @@ -32,6 +34,7 @@\n>> \n>> #include \"libcamera/internal/camera.h\"\n>> #include \"libcamera/internal/camera_sensor.h\"\n>> +#include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n>> #include \"libcamera/internal/delayed_controls.h\"\n>> #include \"libcamera/internal/device_enumerator.h\"\n>> #include \"libcamera/internal/framebuffer.h\"\n>> @@ -180,6 +183,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>> @@ -199,6 +203,15 @@ private:\n>> \tRkISP1MainPath mainPath_;\n>> \tRkISP1SelfPath selfPath_;\n>> \n>> +\tstd::unique_ptr dewarper_;\n>> +\tbool useDewarper_;\n>> +\n>> +\tstd::optional activeCrop_;\n>> +\n>> +\t/* Internal buffers used when dewarper is being used */\n>> +\tstd::vector> mainPathBuffers_;\n>> +\tstd::queue availableMainPathBuffers_;\n>> +\n>> \tstd::vector> paramBuffers_;\n>> \tstd::vector> statBuffers_;\n>> \tstd::queue availableParamBuffers_;\n>> @@ -221,6 +234,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>> \n>> \tif (!isRaw) {\n>> \t\tif (pipe_->availableParamBuffers_.empty()) {\n>> @@ -238,10 +253,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>> @@ -267,6 +288,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>> @@ -282,6 +304,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>> @@ -600,7 +623,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>> @@ -778,12 +801,19 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n>> \t\t<< \" crop \" << rect;\n>> \n>> \tstd::map streamConfig;\n>> +\tstd::vector> 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\toutputCfgs.push_back(const_cast(cfg));\n>> +\t\t\t\tret = dewarper_->configure(cfg, outputCfgs);\n>> +\t\t\t\tuseDewarper_ = ret ? false : true;\n>> +\t\t\t}\n>> \t\t} else if (hasSelfPath_) {\n>> \t\t\tret = selfPath_.configure(cfg, format);\n>> \t\t\tstreamConfig[1] = IPAStream(cfg.pixelFormat,\n>> @@ -833,10 +863,19 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S\n>> \tRkISP1CameraData *data = cameraData(camera);\n>> \tunsigned int count = stream->configuration().bufferCount;\n>> \n>> -\tif (stream == &data->mainPathStream_)\n>> -\t\treturn mainPath_.exportBuffers(count, buffers);\n>> -\telse if (hasSelfPath_ && stream == &data->selfPathStream_)\n>> +\tif (stream == &data->mainPathStream_) {\n>> +\t\t/*\n>> +\t\t * Currently, i.MX8MP is the only platform with DW100 dewarper.\n>> +\t\t * It has mainpath and no self path. Hence, export buffers from\n>> +\t\t * dewarper just for the main path stream, for now.\n>> +\t\t */\n>> +\t\tif (useDewarper_)\n>> +\t\t\treturn dewarper_->exportBuffers(&data->mainPathStream_, count, buffers);\n>> +\t\telse\n>> +\t\t\treturn mainPath_.exportBuffers(count, buffers);\n>> +\t} else if (hasSelfPath_ && stream == &data->selfPathStream_) {\n>> \t\treturn selfPath_.exportBuffers(count, buffers);\n>> +\t}\n>> \n>> \treturn -EINVAL;\n>> }\n>> @@ -860,6 +899,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_.exportBuffers(maxCount, &mainPathBuffers_);\n>> +\t\t\tif (ret < 0)\n>> +\t\t\t\tgoto error;\n>> +\n>> +\t\t\tfor (std::unique_ptr &buffer : mainPathBuffers_)\n>> +\t\t\t\tavailableMainPathBuffers_.push(buffer.get());\n>> +\t\t}\n>> \t}\n>> \n>> \tfor (std::unique_ptr &buffer : paramBuffers_) {\n>> @@ -883,6 +932,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>> error:\n>> \tparamBuffers_.clear();\n>> \tstatBuffers_.clear();\n>> +\tmainPathBuffers_.clear();\n>> \n>> \treturn ret;\n>> }\n>> @@ -897,8 +947,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 ids;\n>> \tfor (IPABuffer &ipabuf : data->ipaBuffers_)\n>> @@ -954,6 +1008,15 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n>> \t\t\treturn ret;\n>> \t\t}\n>> \t\tactions += [&]() { stat_->streamOff(); };\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\tactions += [&]() { dewarper_->stop(); };\n>> \t}\n>> \n>> \tif (data->mainPath_->isEnabled()) {\n>> @@ -1000,6 +1063,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>> @@ -1110,6 +1176,16 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n>> {\n>> \tControlInfoMap::Map controls;\n>> \n>> +\tif (dewarper_) {\n>> +\t\tstd::pair cropLimits =\n>> +\t\t\tdewarper_->inputCropBounds(&data->mainPathStream_);\n>> +\n>> +\t\tcontrols[&controls::ScalerCrop] = ControlInfo(cropLimits.first,\n>> +\t\t\t\t\t\t\t cropLimits.second,\n>> +\t\t\t\t\t\t\t cropLimits.second);\n>> +\t\tactiveCrop_ = cropLimits.second;\n>> +\t}\n>> +\n>> \t/* Add the IPA registered controls to list of camera controls. */\n>> \tfor (const auto &ipaControl : data->ipaControls_)\n>> \t\tcontrols[ipaControl.first] = ipaControl.second;\n>> @@ -1236,6 +1312,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>> +\tstd::shared_ptr dwpMediaDevice = enumerator->search(dwp);\n>> +\tif (dwpMediaDevice) {\n>> +\t\tdewarper_ = std::make_unique(dwpMediaDevice.get());\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\tLOG(RkISP1, Info)\n> \t\t\t\t<< \"Using DW100 dewarper \" << dewarper_->deviceNode();\n> \t\t\t\n>> +\t\t} else {\n>> +\t\t\tLOG(RkISP1, Warning) << \"Found DW100 dewarper \" << dewarper_->deviceNode()\n>> +\t\t\t\t\t << \" but invalid\";\n> \t\t\tLOG(RkISP1, Warning)\n> \t\t\t\t<< \"Found DW100 dewarper \" << dewarper_->deviceNode()\n> \t\t\t\t<< \" but invalid\";\n>\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>> @@ -1282,7 +1378,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>> @@ -1304,6 +1400,79 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n>> \t\t\tinfo->metadataProcessed = true;\n>> \t}\n>> \n>> +\tif (!useDewarper_) {\n>> +\t\tcompleteBuffer(request, buffer);\n>> +\t\ttryCompleteRequest(info);\n>> +\n>> +\t\treturn;\n>> +\t}\n>> +\n>> +\t/* Do not queue cancelled frames to dewarper. */\n>> +\tif (metadata.status == FrameMetadata::FrameCancelled) {\n>> +\t\t/*\n>> +\t\t * i.MX8MP is the only known platform with dewarper. It has\n>> +\t\t * no self path. Hence, only main path buffer completion is\n>> +\t\t * required.\n>> +\t\t *\n>> +\t\t * Also, we cannot completeBuffer(request, buffer) as buffer\n>> +\t\t * here, is an internal buffer (between ISP and dewarper) and\n>> +\t\t * is not associated to the any specific request. The request\n>> +\t\t * buffer associated with main path stream is the one that\n>> +\t\t * is required to be completed (not the internal buffer).\n>> +\t\t */\n>> +\t\tfor (auto it : request->buffers()) {\n>> +\t\t\tif (it.first == &data->mainPathStream_)\n>> +\t\t\t\tcompleteBuffer(request, it.second);\n>> +\t\t}\n>> +\n>> +\t\ttryCompleteRequest(info);\n>> +\t\treturn;\n>> +\t}\n>> +\n>> +\t/* Handle scaler crop control. */\n>> +\tconst auto &crop = request->controls().get(controls::ScalerCrop);\n>> +\tif (crop) {\n>> +\t\tRectangle appliedRect = crop.value();\n>> +\n>> +\t\tint ret = dewarper_->setInputCrop(&data->mainPathStream_,\n>> +\t\t\t\t\t\t &appliedRect);\n> The kernel driver doesn't handle this right :-( There's no\n> synchronization between setting the crop rectangle and running jobs.\n> You'll see glitches when zooming. That will need to be fixed in the\n> kernel, and the implementation in libcamera will likely need to change.\n> We can keep this code here for now.\n>\n>> +\t\tif (!ret && appliedRect != crop.value()) {\n>> +\t\t\t/*\n>> +\t\t\t * If the rectangle is changed by setInputCrop on the\n>> +\t\t\t * dewarper, log a debug message and cache the actual\n>> +\t\t\t * applied rectangle for metadata reporting.\n>> +\t\t\t */\n>> +\t\t\tLOG(RkISP1, Debug)\n>> +\t\t\t\t<< \"Applied rectangle \" << appliedRect.toString()\n>> +\t\t\t\t<< \" differs from requested \" << crop.value().toString();\n>> +\t\t}\n>> +\n>> +\t\tactiveCrop_ = appliedRect;\n>> +\t}\n>> +\n>> +\t/*\n>> +\t * Queue input and output buffers to the dewarper. The output\n>> +\t * buffers for the dewarper are the buffers of the request, supplied\n>> +\t * by the application.\n>> +\t */\n>> +\tint ret = dewarper_->queueBuffers(buffer, request->buffers());\n>> +\tif (ret < 0)\n>> +\t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n>> +\t\t\t\t << strerror(-ret);\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>> +\trequest->metadata().set(controls::ScalerCrop, activeCrop_.value());\n> You're racing with PipelineHandlerRkISP1::bufferReady(), there's no\n> guarantee that activeCrop_ will be the right one. In v6 you stored the\n> active crop in the RkISP1FrameInfo, which produced the correct\n> behaviour. Another option would be to set the metadata in\n> PipelineHandlerRkISP1::bufferReady(), which would be simpler.\n\nAh yes, this get's racy.\n\nI've opted for for the latter, i.e. setting the metadata in bufferReady()\n>\n>> +\n>> \tcompleteBuffer(request, buffer);\n>> \ttryCompleteRequest(info);\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 BF190C0F1B\n\tfor ;\n\tThu, 26 Sep 2024 07:35:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BF7856350E;\n\tThu, 26 Sep 2024 09:35:25 +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 26EFA634F2\n\tfor ;\n\tThu, 26 Sep 2024 09:35:23 +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 788108D4;\n\tThu, 26 Sep 2024 09:33:54 +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=\"SFNsw+sH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727336035;\n\tbh=2HPlAWaeZwLBI6pJEqRpWHwNFYf9QTlfrqFXeQSbC2s=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=SFNsw+sHdbPRb4VXSyrEUONxB6m9yAIsiNbYzjyzdV0faCzdpeQciiT1ZVrui7TZv\n\tC35Ppd9a60iXogGn/c+DshGIuLXWeuysPq2nxslGnB/j+1wJt3w60yV/opAO38J7cP\n\txbEXLIU+IY7N0zwPuyWH4vI1drtONGtEndhMldA8=","Message-ID":"","Date":"Thu, 26 Sep 2024 13:05:18 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","References":"<20240906063444.32772-1-umang.jain@ideasonboard.com>\n\t<20240906063444.32772-5-umang.jain@ideasonboard.com>\n\t<20240925184245.GG27666@pendragon.ideasonboard.com>","Content-Language":"en-US","From":"Umang Jain ","In-Reply-To":"<20240925184245.GG27666@pendragon.ideasonboard.com>","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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31372,"web_url":"https://patchwork.libcamera.org/comment/31372/","msgid":"<666e4f3e-420e-4639-b2f5-8d8f33a18257@ideasonboard.com>","date":"2024-09-26T07:43:24","subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","submitter":{"id":86,"url":"https://patchwork.libcamera.org/api/people/86/","name":"Umang Jain","email":"umang.jain@ideasonboard.com"},"content":"Hi Laurent,\n\nOn 26/09/24 12:16 am, Laurent Pinchart wrote:\n> On Wed, Sep 25, 2024 at 01:29:42PM +0530, Umang Jain wrote:\n>> On 25/09/24 1:20 pm, Stefan Klug wrote:\n>>> On Fri, Sep 06, 2024 at 12:04:44PM +0530, Umang Jain wrote:\n>>>> Plumb the dw100 dewarper as a V4L2M2M converter in the rkisp1 pipeline\n>>>> handler. If the dewarper is found, it is instantiated and buffers are\n>>>> exported from it, instead of RkISP1Path. Internal buffers are allocated\n>>>> for the RkISP1Path in case where dewarper is going to be used.\n>>>>\n>>>> The RKISP1 pipeline handler now supports scaler crop control through\n>>>> the converter. Register the ScalerCrop control for the cameras created\n>>>> in the RKISP1 pipeline handler.\n>>>>\n>>>> Signed-off-by: Umang Jain \n>>>> ---\n>>>> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 183 ++++++++++++++++++++++-\n>>>> 1 file changed, 176 insertions(+), 7 deletions(-)\n>>>>\n>>>> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> index 651258e3..0a794d63 100644\n>>>> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n>>>> @@ -6,10 +6,12 @@\n>>>> */\n>>>> \n>>>> #include \n>>>> +#include \n>>>> #include \n>>>> #include \n>>>> #include \n>>>> #include \n>>>> +#include \n>>>> \n>>>> #include \n>>>> #include \n>>>> @@ -32,6 +34,7 @@\n>>>> \n>>>> #include \"libcamera/internal/camera.h\"\n>>>> #include \"libcamera/internal/camera_sensor.h\"\n>>>> +#include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n>>>> #include \"libcamera/internal/delayed_controls.h\"\n>>>> #include \"libcamera/internal/device_enumerator.h\"\n>>>> #include \"libcamera/internal/framebuffer.h\"\n>>>> @@ -180,6 +183,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>>>> @@ -199,6 +203,15 @@ private:\n>>>> \tRkISP1MainPath mainPath_;\n>>>> \tRkISP1SelfPath selfPath_;\n>>>> \n>>>> +\tstd::unique_ptr dewarper_;\n>>>> +\tbool useDewarper_;\n>>>> +\n>>>> +\tstd::optional activeCrop_;\n>>>> +\n>>>> +\t/* Internal buffers used when dewarper is being used */\n>>>> +\tstd::vector> mainPathBuffers_;\n>>>> +\tstd::queue availableMainPathBuffers_;\n>>>> +\n>>>> \tstd::vector> paramBuffers_;\n>>>> \tstd::vector> statBuffers_;\n>>>> \tstd::queue availableParamBuffers_;\n>>>> @@ -221,6 +234,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>>>> \n>>>> \tif (!isRaw) {\n>>>> \t\tif (pipe_->availableParamBuffers_.empty()) {\n>>>> @@ -238,10 +253,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>>>> @@ -267,6 +288,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>>>> @@ -282,6 +304,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>>>> @@ -600,7 +623,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>>>> @@ -778,12 +801,19 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n>>>> \t\t<< \" crop \" << rect;\n>>>> \n>>>> \tstd::map streamConfig;\n>>>> +\tstd::vector> 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\toutputCfgs.push_back(const_cast(cfg));\n>>>> +\t\t\t\tret = dewarper_->configure(cfg, outputCfgs);\n>>>> +\t\t\t\tuseDewarper_ = ret ? false : true;\n>>>> +\t\t\t}\n>>>> \t\t} else if (hasSelfPath_) {\n>>>> \t\t\tret = selfPath_.configure(cfg, format);\n>>>> \t\t\tstreamConfig[1] = IPAStream(cfg.pixelFormat,\n>>>> @@ -833,10 +863,19 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S\n>>>> \tRkISP1CameraData *data = cameraData(camera);\n>>>> \tunsigned int count = stream->configuration().bufferCount;\n>>>> \n>>>> -\tif (stream == &data->mainPathStream_)\n>>>> -\t\treturn mainPath_.exportBuffers(count, buffers);\n>>>> -\telse if (hasSelfPath_ && stream == &data->selfPathStream_)\n>>>> +\tif (stream == &data->mainPathStream_) {\n>>>> +\t\t/*\n>>>> +\t\t * Currently, i.MX8MP is the only platform with DW100 dewarper.\n>>>> +\t\t * It has mainpath and no self path. Hence, export buffers from\n>>>> +\t\t * dewarper just for the main path stream, for now.\n>>>> +\t\t */\n>>>> +\t\tif (useDewarper_)\n>>>> +\t\t\treturn dewarper_->exportBuffers(&data->mainPathStream_, count, buffers);\n>>>> +\t\telse\n>>>> +\t\t\treturn mainPath_.exportBuffers(count, buffers);\n>>>> +\t} else if (hasSelfPath_ && stream == &data->selfPathStream_) {\n>>>> \t\treturn selfPath_.exportBuffers(count, buffers);\n>>>> +\t}\n>>>> \n>>>> \treturn -EINVAL;\n>>>> }\n>>>> @@ -860,6 +899,16 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>>>> \t\tret = stat_->allocateBuffers(maxCount, &statBuffers_);\n>>> Is there a way for the user to prevent usage of the dewarper? It\n>>> increases latency and doubles the required buffers. Now that I write\n>>> about it. To configure actual lens dewarping I envision something like a\n>>> \"dewarp\" algorithm in the tuning file. Maybe that is the best place for\n>>> that decision...\n>> I think somewhere in the previous interations, we decided that if the\n>> dewarper is present, it should be discovered and used. At that point, we\n>> failed to justify any use cases, where a user might have to disable the\n>> dewarper 'intentionally'.\n>>\n>> If there is use case, we can surely allow disabling the dewarper (and\n>> shouldn't be a tedious patch on top, I believe). But, The justification\n>> has to be there.\n> If the camera doesn't have a dewarping map, and if the user doesn't\n> request digital zoom, then I think the dewarper should be bypassed\n> instead of introducing delays.\n\nBehavior seems okay to me. Bypassing the dewarper should be handled when \nplumbing the vertex map, I think ? Stefan, what do you think?\n\n\n>\n> I would still like to get to the bottom of using the resizer to\n> implement digital zoom.\n\nAny specific reasons?  Dewarper gives us better digital zoom \ncapabilities limits, last I checked.\n\n>\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_.exportBuffers(maxCount, &mainPathBuffers_);\n>>>> +\t\t\tif (ret < 0)\n>>>> +\t\t\t\tgoto error;\n>>>> +\n>>>> +\t\t\tfor (std::unique_ptr &buffer : mainPathBuffers_)\n>>>> +\t\t\t\tavailableMainPathBuffers_.push(buffer.get());\n>>>> +\t\t}\n>>>> \t}\n>>>> \n>>>> \tfor (std::unique_ptr &buffer : paramBuffers_) {\n>>>> @@ -883,6 +932,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n>>>> error:\n>>>> \tparamBuffers_.clear();\n>>>> \tstatBuffers_.clear();\n>>>> +\tmainPathBuffers_.clear();\n>>>> \n>>>> \treturn ret;\n>>>> }\n>>>> @@ -897,8 +947,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 ids;\n>>>> \tfor (IPABuffer &ipabuf : data->ipaBuffers_)\n>>>> @@ -954,6 +1008,15 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n>>>> \t\t\treturn ret;\n>>>> \t\t}\n>>>> \t\tactions += [&]() { stat_->streamOff(); };\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\tactions += [&]() { dewarper_->stop(); };\n>>>> \t}\n>>>> \n>>>> \tif (data->mainPath_->isEnabled()) {\n>>>> @@ -1000,6 +1063,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>>>> @@ -1110,6 +1176,16 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n>>>> {\n>>>> \tControlInfoMap::Map controls;\n>>>> \n>>>> +\tif (dewarper_) {\n>>>> +\t\tstd::pair cropLimits =\n>>>> +\t\t\tdewarper_->inputCropBounds(&data->mainPathStream_);\n>>>> +\n>>>> +\t\tcontrols[&controls::ScalerCrop] = ControlInfo(cropLimits.first,\n>>>> +\t\t\t\t\t\t\t cropLimits.second,\n>>>> +\t\t\t\t\t\t\t cropLimits.second);\n>>>> +\t\tactiveCrop_ = cropLimits.second;\n>>>> +\t}\n>>>> +\n>>> I'm a bit uncertain here. The current dewarper kernel driver produces\n>>> quite unexpected and difficult to explain results (modified crop\n>>> rectangle, changed pixel aspect ratio). So to me it feels like we\n>>> shouldn't expose that hardware control directly to the user, but do an\n>>> internal implementation (based on the same libcamera control). Could we\n>>> split just the logic that adds the control into a separate patch?\n>> I don't understand internal implementation based on same libcamera\n>> control? How would that look like?\n>>\n>> So, you mean to say, we don't feed any scaler crop rectangles from\n>> application, but do it internally ?\n>>\n>>> Another approach would be to merge it in and improve later. But then we\n>>> might break the interface for users of the feature... I lean towards\n>>> splitting the control and merging the rest. In that case we should\n>>> disable the dewarper by default, as the user has no added benefit\n>>> without the control. Still I think all the plumbing should definitely\n>>> go in.\n>> I don't foresee breaking the interface for users. But there can be\n>> improvements on top of this (for e.g. mapping the V4L2 Selection flags)\n>> and letting the application control that via an interface.\n>>\n>> https://docs.kernel.org/userspace-api/media/v4l/v4l2-selection-flags.html\n>>\n>>>> \t/* Add the IPA registered controls to list of camera controls. */\n>>>> \tfor (const auto &ipaControl : data->ipaControls_)\n>>>> \t\tcontrols[ipaControl.first] = ipaControl.second;\n>>>> @@ -1236,6 +1312,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>>>> +\tstd::shared_ptr dwpMediaDevice = enumerator->search(dwp);\n>>>> +\tif (dwpMediaDevice) {\n>>>> +\t\tdewarper_ = std::make_unique(dwpMediaDevice.get());\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, Warning) << \"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>>>> @@ -1282,7 +1378,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>>>> @@ -1304,6 +1400,79 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n>>>> \t\t\tinfo->metadataProcessed = true;\n>>>> \t}\n>>>> \n>>>> +\tif (!useDewarper_) {\n>>>> +\t\tcompleteBuffer(request, buffer);\n>>>> +\t\ttryCompleteRequest(info);\n>>>> +\n>>>> +\t\treturn;\n>>>> +\t}\n>>>> +\n>>>> +\t/* Do not queue cancelled frames to dewarper. */\n>>>> +\tif (metadata.status == FrameMetadata::FrameCancelled) {\n>>>> +\t\t/*\n>>>> +\t\t * i.MX8MP is the only known platform with dewarper. It has\n>>>> +\t\t * no self path. Hence, only main path buffer completion is\n>>>> +\t\t * required.\n>>>> +\t\t *\n>>>> +\t\t * Also, we cannot completeBuffer(request, buffer) as buffer\n>>>> +\t\t * here, is an internal buffer (between ISP and dewarper) and\n>>>> +\t\t * is not associated to the any specific request. The request\n>>>> +\t\t * buffer associated with main path stream is the one that\n>>>> +\t\t * is required to be completed (not the internal buffer).\n>>>> +\t\t */\n>>>> +\t\tfor (auto it : request->buffers()) {\n>>>> +\t\t\tif (it.first == &data->mainPathStream_)\n>>>> +\t\t\t\tcompleteBuffer(request, it.second);\n>>>> +\t\t}\n>>>> +\n>>>> +\t\ttryCompleteRequest(info);\n>>>> +\t\treturn;\n>>>> +\t}\n>>>> +\n>>>> +\t/* Handle scaler crop control. */\n>>>> +\tconst auto &crop = request->controls().get(controls::ScalerCrop);\n>>>> +\tif (crop) {\n>>>> +\t\tRectangle appliedRect = crop.value();\n>>>> +\n>>>> +\t\tint ret = dewarper_->setInputCrop(&data->mainPathStream_,\n>>>> +\t\t\t\t\t\t &appliedRect);\n>>>> +\t\tif (!ret && appliedRect != crop.value()) {\n>>>> +\t\t\t/*\n>>>> +\t\t\t * If the rectangle is changed by setInputCrop on the\n>>>> +\t\t\t * dewarper, log a debug message and cache the actual\n>>>> +\t\t\t * applied rectangle for metadata reporting.\n>>>> +\t\t\t */\n>>>> +\t\t\tLOG(RkISP1, Debug)\n>>>> +\t\t\t\t<< \"Applied rectangle \" << appliedRect.toString()\n>>>> +\t\t\t\t<< \" differs from requested \" << crop.value().toString();\n>>>> +\t\t}\n>>>> +\n>>>> +\t\tactiveCrop_ = appliedRect;\n>>>> +\t}\n>>>> +\n>>>> +\t/*\n>>>> +\t * Queue input and output buffers to the dewarper. The output\n>>>> +\t * buffers for the dewarper are the buffers of the request, supplied\n>>>> +\t * by the application.\n>>>> +\t */\n>>>> +\tint ret = dewarper_->queueBuffers(buffer, request->buffers());\n>>>> +\tif (ret < 0)\n>>>> +\t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n>>>> +\t\t\t\t << strerror(-ret);\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>>>> +\trequest->metadata().set(controls::ScalerCrop, activeCrop_.value());\n>>> I couldn't easily see if there are cases where more than one buffer are\n>>> queued to the dewarper. In that case the activeCrop_ could be the one\n>>> applied to the next frame. If it is ensured that that is not the case it\n>>> might be worth a comment.\n>>>\n>>>> +\n>>>> \tcompleteBuffer(request, buffer);\n>>>> \ttryCompleteRequest(info);\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 F11E8C3257\n\tfor ;\n\tThu, 26 Sep 2024 07:43:32 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C64856350E;\n\tThu, 26 Sep 2024 09:43:31 +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 29047634F2\n\tfor ;\n\tThu, 26 Sep 2024 09:43:30 +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 6D441D80;\n\tThu, 26 Sep 2024 09:42:01 +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=\"uLEI3L+h\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727336522;\n\tbh=J7iziwvsDAhdc0TBnW0OR4keuG8AFwux6VK7ke01TLQ=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=uLEI3L+hckoCAJUi8Sezb45UvuY49zuj+jgHpDNko5NET82gxrMqylyPxhlm8FcMZ\n\tdl2K1C8WVc/ZC32BY7EQvyqd7jHVRPXs5FIxiBw/qAQsp5CiqA7WoA8cmDf7reRaFP\n\tRfbxFcQdzYzl6ahzf0dWdnsBbt/wya+8GoZxUmNs=","Message-ID":"<666e4f3e-420e-4639-b2f5-8d8f33a18257@ideasonboard.com>","Date":"Thu, 26 Sep 2024 13:13:24 +0530","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","To":"Laurent Pinchart ","Cc":"Stefan Klug ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20240906063444.32772-1-umang.jain@ideasonboard.com>\n\t<20240906063444.32772-5-umang.jain@ideasonboard.com>\n\t\n\t<42f61999-a99c-4b55-aa84-e582a9da939a@ideasonboard.com>\n\t<20240925184632.GA30399@pendragon.ideasonboard.com>","Content-Language":"en-US","From":"Umang Jain ","In-Reply-To":"<20240925184632.GA30399@pendragon.ideasonboard.com>","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":31387,"web_url":"https://patchwork.libcamera.org/comment/31387/","msgid":"<6d3rqarurrq75sxkmgtztmdhqcfdbm3mhatwyneujuqdmflpeq@kpqdgibmxsdc>","date":"2024-09-26T09:33:53","subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","submitter":{"id":184,"url":"https://patchwork.libcamera.org/api/people/184/","name":"Stefan Klug","email":"stefan.klug@ideasonboard.com"},"content":"Hi Umang,\n\nOn Thu, Sep 26, 2024 at 01:13:24PM +0530, Umang Jain wrote:\n> Hi Laurent,\n> \n> On 26/09/24 12:16 am, Laurent Pinchart wrote:\n> > On Wed, Sep 25, 2024 at 01:29:42PM +0530, Umang Jain wrote:\n> > > On 25/09/24 1:20 pm, Stefan Klug wrote:\n> > > > On Fri, Sep 06, 2024 at 12:04:44PM +0530, Umang Jain wrote:\n> > > > > Plumb the dw100 dewarper as a V4L2M2M converter in the rkisp1 pipeline\n> > > > > handler. If the dewarper is found, it is instantiated and buffers are\n> > > > > exported from it, instead of RkISP1Path. Internal buffers are allocated\n> > > > > for the RkISP1Path in case where dewarper is going to be used.\n> > > > > \n> > > > > The RKISP1 pipeline handler now supports scaler crop control through\n> > > > > the converter. Register the ScalerCrop control for the cameras created\n> > > > > in the RKISP1 pipeline handler.\n> > > > > \n> > > > > Signed-off-by: Umang Jain \n> > > > > ---\n> > > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 183 ++++++++++++++++++++++-\n> > > > > 1 file changed, 176 insertions(+), 7 deletions(-)\n> > > > > \n> > > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > > index 651258e3..0a794d63 100644\n> > > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > > @@ -6,10 +6,12 @@\n> > > > > */\n> > > > > #include \n> > > > > +#include \n> > > > > #include \n> > > > > #include \n> > > > > #include \n> > > > > #include \n> > > > > +#include \n> > > > > #include \n> > > > > #include \n> > > > > @@ -32,6 +34,7 @@\n> > > > > #include \"libcamera/internal/camera.h\"\n> > > > > #include \"libcamera/internal/camera_sensor.h\"\n> > > > > +#include \"libcamera/internal/converter/converter_v4l2_m2m.h\"\n> > > > > #include \"libcamera/internal/delayed_controls.h\"\n> > > > > #include \"libcamera/internal/device_enumerator.h\"\n> > > > > #include \"libcamera/internal/framebuffer.h\"\n> > > > > @@ -180,6 +183,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> > > > > @@ -199,6 +203,15 @@ private:\n> > > > > \tRkISP1MainPath mainPath_;\n> > > > > \tRkISP1SelfPath selfPath_;\n> > > > > +\tstd::unique_ptr dewarper_;\n> > > > > +\tbool useDewarper_;\n> > > > > +\n> > > > > +\tstd::optional activeCrop_;\n> > > > > +\n> > > > > +\t/* Internal buffers used when dewarper is being used */\n> > > > > +\tstd::vector> mainPathBuffers_;\n> > > > > +\tstd::queue availableMainPathBuffers_;\n> > > > > +\n> > > > > \tstd::vector> paramBuffers_;\n> > > > > \tstd::vector> statBuffers_;\n> > > > > \tstd::queue availableParamBuffers_;\n> > > > > @@ -221,6 +234,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> > > > > \tif (!isRaw) {\n> > > > > \t\tif (pipe_->availableParamBuffers_.empty()) {\n> > > > > @@ -238,10 +253,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> > > > > @@ -267,6 +288,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> > > > > @@ -282,6 +304,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> > > > > @@ -600,7 +623,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> > > > > @@ -778,12 +801,19 @@ int PipelineHandlerRkISP1::configure(Camera *camera, CameraConfiguration *c)\n> > > > > \t\t<< \" crop \" << rect;\n> > > > > \tstd::map streamConfig;\n> > > > > +\tstd::vector> 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\toutputCfgs.push_back(const_cast(cfg));\n> > > > > +\t\t\t\tret = dewarper_->configure(cfg, outputCfgs);\n> > > > > +\t\t\t\tuseDewarper_ = ret ? false : true;\n> > > > > +\t\t\t}\n> > > > > \t\t} else if (hasSelfPath_) {\n> > > > > \t\t\tret = selfPath_.configure(cfg, format);\n> > > > > \t\t\tstreamConfig[1] = IPAStream(cfg.pixelFormat,\n> > > > > @@ -833,10 +863,19 @@ int PipelineHandlerRkISP1::exportFrameBuffers([[maybe_unused]] Camera *camera, S\n> > > > > \tRkISP1CameraData *data = cameraData(camera);\n> > > > > \tunsigned int count = stream->configuration().bufferCount;\n> > > > > -\tif (stream == &data->mainPathStream_)\n> > > > > -\t\treturn mainPath_.exportBuffers(count, buffers);\n> > > > > -\telse if (hasSelfPath_ && stream == &data->selfPathStream_)\n> > > > > +\tif (stream == &data->mainPathStream_) {\n> > > > > +\t\t/*\n> > > > > +\t\t * Currently, i.MX8MP is the only platform with DW100 dewarper.\n> > > > > +\t\t * It has mainpath and no self path. Hence, export buffers from\n> > > > > +\t\t * dewarper just for the main path stream, for now.\n> > > > > +\t\t */\n> > > > > +\t\tif (useDewarper_)\n> > > > > +\t\t\treturn dewarper_->exportBuffers(&data->mainPathStream_, count, buffers);\n> > > > > +\t\telse\n> > > > > +\t\t\treturn mainPath_.exportBuffers(count, buffers);\n> > > > > +\t} else if (hasSelfPath_ && stream == &data->selfPathStream_) {\n> > > > > \t\treturn selfPath_.exportBuffers(count, buffers);\n> > > > > +\t}\n> > > > > \treturn -EINVAL;\n> > > > > }\n> > > > > @@ -860,6 +899,16 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> > > > > \t\tret = stat_->allocateBuffers(maxCount, &statBuffers_);\n> > > > Is there a way for the user to prevent usage of the dewarper? It\n> > > > increases latency and doubles the required buffers. Now that I write\n> > > > about it. To configure actual lens dewarping I envision something like a\n> > > > \"dewarp\" algorithm in the tuning file. Maybe that is the best place for\n> > > > that decision...\n> > > I think somewhere in the previous interations, we decided that if the\n> > > dewarper is present, it should be discovered and used. At that point, we\n> > > failed to justify any use cases, where a user might have to disable the\n> > > dewarper 'intentionally'.\n> > > \n> > > If there is use case, we can surely allow disabling the dewarper (and\n> > > shouldn't be a tedious patch on top, I believe). But, The justification\n> > > has to be there.\n> > If the camera doesn't have a dewarping map, and if the user doesn't\n> > request digital zoom, then I think the dewarper should be bypassed\n> > instead of introducing delays.\n> \n> Behavior seems okay to me. Bypassing the dewarper should be handled when\n> plumbing the vertex map, I think ? Stefan, what do you think?\n\nI'm fine with either way. It is most likely easier to merge it in as it\nis now. The chances are pretty good, that we can get the vertex map\nsupport in before the next release.\n\nRegards,\nStefan\n\n> \n> \n> > \n> > I would still like to get to the bottom of using the resizer to\n> > implement digital zoom.\n> \n> Any specific reasons?  Dewarper gives us better digital zoom capabilities\n> limits, last I checked.\n> \n> > \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_.exportBuffers(maxCount, &mainPathBuffers_);\n> > > > > +\t\t\tif (ret < 0)\n> > > > > +\t\t\t\tgoto error;\n> > > > > +\n> > > > > +\t\t\tfor (std::unique_ptr &buffer : mainPathBuffers_)\n> > > > > +\t\t\t\tavailableMainPathBuffers_.push(buffer.get());\n> > > > > +\t\t}\n> > > > > \t}\n> > > > > \tfor (std::unique_ptr &buffer : paramBuffers_) {\n> > > > > @@ -883,6 +932,7 @@ int PipelineHandlerRkISP1::allocateBuffers(Camera *camera)\n> > > > > error:\n> > > > > \tparamBuffers_.clear();\n> > > > > \tstatBuffers_.clear();\n> > > > > +\tmainPathBuffers_.clear();\n> > > > > \treturn ret;\n> > > > > }\n> > > > > @@ -897,8 +947,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 ids;\n> > > > > \tfor (IPABuffer &ipabuf : data->ipaBuffers_)\n> > > > > @@ -954,6 +1008,15 @@ int PipelineHandlerRkISP1::start(Camera *camera, [[maybe_unused]] const ControlL\n> > > > > \t\t\treturn ret;\n> > > > > \t\t}\n> > > > > \t\tactions += [&]() { stat_->streamOff(); };\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\tactions += [&]() { dewarper_->stop(); };\n> > > > > \t}\n> > > > > \tif (data->mainPath_->isEnabled()) {\n> > > > > @@ -1000,6 +1063,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> > > > > @@ -1110,6 +1176,16 @@ int PipelineHandlerRkISP1::updateControls(RkISP1CameraData *data)\n> > > > > {\n> > > > > \tControlInfoMap::Map controls;\n> > > > > +\tif (dewarper_) {\n> > > > > +\t\tstd::pair cropLimits =\n> > > > > +\t\t\tdewarper_->inputCropBounds(&data->mainPathStream_);\n> > > > > +\n> > > > > +\t\tcontrols[&controls::ScalerCrop] = ControlInfo(cropLimits.first,\n> > > > > +\t\t\t\t\t\t\t cropLimits.second,\n> > > > > +\t\t\t\t\t\t\t cropLimits.second);\n> > > > > +\t\tactiveCrop_ = cropLimits.second;\n> > > > > +\t}\n> > > > > +\n> > > > I'm a bit uncertain here. The current dewarper kernel driver produces\n> > > > quite unexpected and difficult to explain results (modified crop\n> > > > rectangle, changed pixel aspect ratio). So to me it feels like we\n> > > > shouldn't expose that hardware control directly to the user, but do an\n> > > > internal implementation (based on the same libcamera control). Could we\n> > > > split just the logic that adds the control into a separate patch?\n> > > I don't understand internal implementation based on same libcamera\n> > > control? How would that look like?\n> > > \n> > > So, you mean to say, we don't feed any scaler crop rectangles from\n> > > application, but do it internally ?\n> > > \n> > > > Another approach would be to merge it in and improve later. But then we\n> > > > might break the interface for users of the feature... I lean towards\n> > > > splitting the control and merging the rest. In that case we should\n> > > > disable the dewarper by default, as the user has no added benefit\n> > > > without the control. Still I think all the plumbing should definitely\n> > > > go in.\n> > > I don't foresee breaking the interface for users. But there can be\n> > > improvements on top of this (for e.g. mapping the V4L2 Selection flags)\n> > > and letting the application control that via an interface.\n> > > \n> > > https://docs.kernel.org/userspace-api/media/v4l/v4l2-selection-flags.html\n> > > \n> > > > > \t/* Add the IPA registered controls to list of camera controls. */\n> > > > > \tfor (const auto &ipaControl : data->ipaControls_)\n> > > > > \t\tcontrols[ipaControl.first] = ipaControl.second;\n> > > > > @@ -1236,6 +1312,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> > > > > +\tstd::shared_ptr dwpMediaDevice = enumerator->search(dwp);\n> > > > > +\tif (dwpMediaDevice) {\n> > > > > +\t\tdewarper_ = std::make_unique(dwpMediaDevice.get());\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, Warning) << \"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> > > > > @@ -1282,7 +1378,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> > > > > @@ -1304,6 +1400,79 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> > > > > \t\t\tinfo->metadataProcessed = true;\n> > > > > \t}\n> > > > > +\tif (!useDewarper_) {\n> > > > > +\t\tcompleteBuffer(request, buffer);\n> > > > > +\t\ttryCompleteRequest(info);\n> > > > > +\n> > > > > +\t\treturn;\n> > > > > +\t}\n> > > > > +\n> > > > > +\t/* Do not queue cancelled frames to dewarper. */\n> > > > > +\tif (metadata.status == FrameMetadata::FrameCancelled) {\n> > > > > +\t\t/*\n> > > > > +\t\t * i.MX8MP is the only known platform with dewarper. It has\n> > > > > +\t\t * no self path. Hence, only main path buffer completion is\n> > > > > +\t\t * required.\n> > > > > +\t\t *\n> > > > > +\t\t * Also, we cannot completeBuffer(request, buffer) as buffer\n> > > > > +\t\t * here, is an internal buffer (between ISP and dewarper) and\n> > > > > +\t\t * is not associated to the any specific request. The request\n> > > > > +\t\t * buffer associated with main path stream is the one that\n> > > > > +\t\t * is required to be completed (not the internal buffer).\n> > > > > +\t\t */\n> > > > > +\t\tfor (auto it : request->buffers()) {\n> > > > > +\t\t\tif (it.first == &data->mainPathStream_)\n> > > > > +\t\t\t\tcompleteBuffer(request, it.second);\n> > > > > +\t\t}\n> > > > > +\n> > > > > +\t\ttryCompleteRequest(info);\n> > > > > +\t\treturn;\n> > > > > +\t}\n> > > > > +\n> > > > > +\t/* Handle scaler crop control. */\n> > > > > +\tconst auto &crop = request->controls().get(controls::ScalerCrop);\n> > > > > +\tif (crop) {\n> > > > > +\t\tRectangle appliedRect = crop.value();\n> > > > > +\n> > > > > +\t\tint ret = dewarper_->setInputCrop(&data->mainPathStream_,\n> > > > > +\t\t\t\t\t\t &appliedRect);\n> > > > > +\t\tif (!ret && appliedRect != crop.value()) {\n> > > > > +\t\t\t/*\n> > > > > +\t\t\t * If the rectangle is changed by setInputCrop on the\n> > > > > +\t\t\t * dewarper, log a debug message and cache the actual\n> > > > > +\t\t\t * applied rectangle for metadata reporting.\n> > > > > +\t\t\t */\n> > > > > +\t\t\tLOG(RkISP1, Debug)\n> > > > > +\t\t\t\t<< \"Applied rectangle \" << appliedRect.toString()\n> > > > > +\t\t\t\t<< \" differs from requested \" << crop.value().toString();\n> > > > > +\t\t}\n> > > > > +\n> > > > > +\t\tactiveCrop_ = appliedRect;\n> > > > > +\t}\n> > > > > +\n> > > > > +\t/*\n> > > > > +\t * Queue input and output buffers to the dewarper. The output\n> > > > > +\t * buffers for the dewarper are the buffers of the request, supplied\n> > > > > +\t * by the application.\n> > > > > +\t */\n> > > > > +\tint ret = dewarper_->queueBuffers(buffer, request->buffers());\n> > > > > +\tif (ret < 0)\n> > > > > +\t\tLOG(RkISP1, Error) << \"Cannot queue buffers to dewarper: \"\n> > > > > +\t\t\t\t << strerror(-ret);\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> > > > > +\trequest->metadata().set(controls::ScalerCrop, activeCrop_.value());\n> > > > I couldn't easily see if there are cases where more than one buffer are\n> > > > queued to the dewarper. In that case the activeCrop_ could be the one\n> > > > applied to the next frame. If it is ensured that that is not the case it\n> > > > might be worth a comment.\n> > > > \n> > > > > +\n> > > > > \tcompleteBuffer(request, buffer);\n> > > > > \ttryCompleteRequest(info);\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 E39B7C3257\n\tfor ;\n\tThu, 26 Sep 2024 09:33:58 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 919F563511;\n\tThu, 26 Sep 2024 11:33:58 +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 B7904634F2\n\tfor ;\n\tThu, 26 Sep 2024 11:33:56 +0200 (CEST)","from ideasonboard.com (unknown\n\t[IPv6:2a00:6020:448c:6c00:9869:84c7:2d80:a3b])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id B37FB8D4;\n\tThu, 26 Sep 2024 11:32:28 +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=\"PKx8qQz5\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1727343148;\n\tbh=eaRcL/aff9rJCmxzyswa6oD8ppsQBQ1QxSx0HSHgOaE=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PKx8qQz5CqS0W5V5Xk2oKtfdLuINE7BjDB7LJKaZKaiQDb1se3pG4l1yVnGfj2eR7\n\tW+pctnuYadC+zQPaF2qFzjyKfDQqJvXD7AjgjtL6lCDN0d7K3XvvLPZwWRkcMKrPj6\n\tzw7jrbmI/TX2ED7XXzLzeAge41bVIdHOtTQxSB/0=","Date":"Thu, 26 Sep 2024 11:33:53 +0200","From":"Stefan Klug ","To":"Umang Jain ","Cc":"Laurent Pinchart , \n\tlibcamera-devel@lists.libcamera.org","Subject":"Re: [PATCH v7 4/4] libcamera: rkisp1: Plumb the dw100 dewarper as\n\tV4L2M2M converter","Message-ID":"<6d3rqarurrq75sxkmgtztmdhqcfdbm3mhatwyneujuqdmflpeq@kpqdgibmxsdc>","References":"<20240906063444.32772-1-umang.jain@ideasonboard.com>\n\t<20240906063444.32772-5-umang.jain@ideasonboard.com>\n\t\n\t<42f61999-a99c-4b55-aa84-e582a9da939a@ideasonboard.com>\n\t<20240925184632.GA30399@pendragon.ideasonboard.com>\n\t<666e4f3e-420e-4639-b2f5-8d8f33a18257@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<666e4f3e-420e-4639-b2f5-8d8f33a18257@ideasonboard.com>","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","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]