[{"id":18648,"web_url":"https://patchwork.libcamera.org/comment/18648/","msgid":"","date":"2021-08-10T01:40:22","subject":"Re: [libcamera-devel] [PATCH 4/4] ipa: vimc: Send and retrieve\n\tFrameBuffers from IPA","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, Aug 06, 2021 at 03:44:09PM +0530, Umang Jain wrote:\n> Plumb through actions and events in the VIMC mojo interface.\n> Events are send from pipeline handler to IPA and actions can be\n> emitted from IPA and handled in the pipeline handler. The plumbing\n> ensures that, we can send/retrieve FrameBuffers from the vimc IPA.\n> \n> Signed-off-by: Umang Jain \n> ---\n> include/libcamera/ipa/vimc.mojom | 23 ++++++++++++++-\n> src/ipa/vimc/vimc.cpp | 42 +++++++++++++++++++++++++++-\n> src/libcamera/pipeline/vimc/vimc.cpp | 27 ++++++++++++++++++\n> 3 files changed, 90 insertions(+), 2 deletions(-)\n> \n> diff --git a/include/libcamera/ipa/vimc.mojom b/include/libcamera/ipa/vimc.mojom\n> index 8452461d..85a87227 100644\n> --- a/include/libcamera/ipa/vimc.mojom\n> +++ b/include/libcamera/ipa/vimc.mojom\n> @@ -17,6 +17,25 @@ enum IPATraceCode {\n> \tIPATraceStop,\n> };\n> \n> +enum VimcOperations {\n> + ActionParamFilled = 1,\n> + EventFillParams = 2,\n> + EventProcessControls = 3,\n> +};\n\nIPA operations are legacy, they come from a time when we didn't have\nmojo to generate the IPA interface. Today it's best to create dedicated\nfunctions in the IPA interface.\n\n> +\n> +struct VimcEvent {\n> + VimcOperations op;\n> + uint32 frame;\n> + int64 frameTimestamp;\n> + uint32 bufferId;\n> + libcamera.ControlList controls;\n> +};\n> +\n> +struct VimcAction {\n> + VimcOperations op;\n> + libcamera.ControlList controls;\n> +};\n\nSame for these data types. You can still create structures to group\nrelated data together, but there's no need anymore to create structures\nthat group all data that any operation may ever need.\n\n> +\n> interface IPAVimcInterface {\n> \tinit(libcamera.IPASettings settings) => (int32 ret);\n> \n> @@ -29,8 +48,10 @@ interface IPAVimcInterface {\n> \n> \tmapBuffers(array buffers);\n> \tunmapBuffers(array ids);\n> +\n> +\t[async] processEvent(VimcEvent ev);\n> };\n> \n> interface IPAVimcEventInterface {\n> -\tdummyEvent(uint32 val);\n> +\tqueueFrameAction(uint32 frame, VimcAction action);\n> };\n> diff --git a/src/ipa/vimc/vimc.cpp b/src/ipa/vimc/vimc.cpp\n> index 2b00687f..784d7993 100644\n> --- a/src/ipa/vimc/vimc.cpp\n> +++ b/src/ipa/vimc/vimc.cpp\n> @@ -25,6 +25,8 @@ namespace libcamera {\n> \n> LOG_DEFINE_CATEGORY(IPAVimc)\n> \n> +namespace ipa::vimc {\n> +\n> class IPAVimc : public ipa::vimc::IPAVimcInterface\n> {\n> public:\n> @@ -43,7 +45,11 @@ public:\n> \tvoid mapBuffers(const std::vector &buffers) override;\n> \tvoid unmapBuffers(const std::vector &ids) override;\n> \n> +\tvoid processEvent(const VimcEvent &event) override;\n> +\n> private:\n> +\tvoid processControls(unsigned int frame, const ControlList &controls);\n> +\n> \tvoid initTrace();\n> \tvoid trace(enum ipa::vimc::IPATraceCode operation);\n> \n> @@ -125,6 +131,38 @@ void IPAVimc::unmapBuffers(const std::vector &ids)\n> \t}\n> }\n> \n> +void IPAVimc::processControls([[maybe_unused]] unsigned int frame,\n> +\t\t\t [[maybe_unused]] const ControlList &controls)\n> +{\n> + /* \\todo Start processing for 'frame' based on 'controls'. */\n> +}\n> +\n> +void IPAVimc::processEvent(const VimcEvent &event)\n> +{\n> +\tswitch (event.op) {\n> +\tcase EventFillParams: {\n> +\t\tauto it = buffers_.find(event.bufferId);\n> +\t\tif (it == buffers_.end()) {\n> +\t\t\tLOG(IPAVimc, Error) << \"Could not find param buffer!\";\n> +\t\t\treturn;\n> +\t\t}\n> +\n> +\t\t/* \\todo Fill parameters with actual parameter buffer */\n> +\t\tVimcAction op;\n> +\t\top.op = ActionParamFilled;\n> +\t\tqueueFrameAction.emit(event.frame, op);\n> +\t\tbreak;\n> +\t}\n> +\tcase EventProcessControls: {\n> +\t\tprocessControls(event.frame, event.controls);\n> +\t\tbreak;\n> +\t}\n> +\tdefault:\n> +\t\tLOG(IPAVimc, Error) << \"Unknown event \" << event.op;\n> +\t\tbreak;\n> +\t}\n> +}\n> +\n> void IPAVimc::initTrace()\n> {\n> \tstruct stat fifoStat;\n> @@ -156,6 +194,8 @@ void IPAVimc::trace(enum ipa::vimc::IPATraceCode operation)\n> \t}\n> }\n> \n> +} /* namespace ipa::vimc */\n> +\n> /*\n> * External IPA module interface\n> */\n> @@ -170,7 +210,7 @@ const struct IPAModuleInfo ipaModuleInfo = {\n> \n> IPAInterface *ipaCreate()\n> {\n> -\treturn new IPAVimc();\n> +\treturn new ipa::vimc::IPAVimc();\n> }\n> }\n> \n> diff --git a/src/libcamera/pipeline/vimc/vimc.cpp b/src/libcamera/pipeline/vimc/vimc.cpp\n> index fa21128d..4b7a542d 100644\n> --- a/src/libcamera/pipeline/vimc/vimc.cpp\n> +++ b/src/libcamera/pipeline/vimc/vimc.cpp\n> @@ -51,6 +51,8 @@ public:\n> \n> \tint init();\n> \tvoid bufferReady(FrameBuffer *buffer);\n> +\tvoid queueFrameAction(unsigned int id,\n> +\t\t\t const ipa::vimc::VimcAction &action);\n> \n> \tMediaDevice *media_;\n> \tstd::unique_ptr sensor_;\n> @@ -438,6 +440,12 @@ int PipelineHandlerVimc::queueRequestDevice(Camera *camera, Request *request)\n> \tif (ret < 0)\n> \t\treturn ret;\n> \n> +\tipa::vimc::VimcEvent ev;\n> +\tev.op = ipa::vimc::EventProcessControls;\n> +\tev.frame = request->sequence();\n> +\tev.controls = request->controls();\n> +\tdata->ipa_->processEvent(ev);\n> +\n> \treturn 0;\n> }\n> \n> @@ -471,6 +479,8 @@ bool PipelineHandlerVimc::match(DeviceEnumerator *enumerator)\n> \t\treturn false;\n> \t}\n> \n> +\tdata->ipa_->queueFrameAction.connect(data.get(), &VimcCameraData::queueFrameAction);\n> +\n> \tstd::string conf = data->ipa_->configurationFile(\"vimc.conf\");\n> \tdata->ipa_->init(IPASettings{ conf, data->sensor_->model() });\n> \n> @@ -569,6 +579,23 @@ void VimcCameraData::bufferReady(FrameBuffer *buffer)\n> \n> \tpipe_->completeBuffer(request, buffer);\n> \tpipe_->completeRequest(request);\n> +\n> +\tipa::vimc::VimcEvent ev;\n> +\tev.op = ipa::vimc::EventFillParams;\n> +\tev.frame = request->sequence();\n> +\tev.bufferId = buffer->cookie();\n> +\tipa_->processEvent(ev);\n> +}\n> +\n> +void VimcCameraData::queueFrameAction([[maybe_unused]] unsigned int id,\n> +\t\t\t\t const ipa::vimc::VimcAction &action)\n> +{\n> +\tswitch (action.op) {\n> +\tcase ipa::vimc::ActionParamFilled:\n> +\t\tbreak;\n> +\tdefault:\n> +\t\tLOG(VIMC, Error) << \"unknown action: \" << action.op;\n> +\t}\n> }\n> \n> REGISTER_PIPELINE_HANDLER(PipelineHandlerVimc)","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 44671BD87D\n\tfor ;\n\tTue, 10 Aug 2021 01:40:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id A09C868826;\n\tTue, 10 Aug 2021 03:40:25 +0200 (CEST)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 5493A687EB\n\tfor ;\n\tTue, 10 Aug 2021 03:40:24 +0200 (CEST)","from pendragon.ideasonboard.com (62-78-145-57.bb.dnainternet.fi\n\t[62.78.145.57])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id C93B43F0;\n\tTue, 10 Aug 2021 03:40:23 +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=\"GnVaWxYc\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1628559624;\n\tbh=i+VMlPzeD5RSqVJAd8G6MoTc6I484lfNDhmanakQG2E=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=GnVaWxYcgbQaYjK5VGeRumD3Ge8grDn1pVC7RVqzN8tfyNWyIdlQ/lCE6AqcVwimV\n\tkZcUYv4//QVxJ0snvLGFVSiiaqaVaKa1yW0VOpHi4gM+o2sNYaHWv4G4lsdXwcHBHB\n\tLEV7vZ4cA7ave8VthghXZOONZv/LXwmaSvwvq0+U=","Date":"Tue, 10 Aug 2021 04:40:22 +0300","From":"Laurent Pinchart ","To":"Umang Jain ","Message-ID":"","References":"<20210806101409.324645-1-umang.jain@ideasonboard.com>\n\t<20210806101409.324645-5-umang.jain@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20210806101409.324645-5-umang.jain@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH 4/4] ipa: vimc: Send and retrieve\n\tFrameBuffers from IPA","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","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]