[{"id":2861,"web_url":"https://patchwork.libcamera.org/comment/2861/","msgid":"<20191011122556.GB4882@pendragon.ideasonboard.com>","date":"2019-10-11T12:25:56","subject":"Re: [libcamera-devel] [PATCH v6 6/9] libcamera: ipa: Extend to\n\tsupport IPA interactions","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Fri, Oct 11, 2019 at 05:22:13AM +0200, Niklas Söderlund wrote:\n> The IPA interface needs to support interactions with the pipeline; add\n> interfaces to control the sensor and handling of request ISP parameters\n> and statistics.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> include/ipa/ipa_interface.h | 36 ++++\n> src/ipa/ipa_vimc.cpp | 7 +-\n> src/libcamera/ipa_interface.cpp | 219 ++++++++++++++++++++++++\n> src/libcamera/proxy/ipa_proxy_linux.cpp | 14 +-\n> 4 files changed, 267 insertions(+), 9 deletions(-)\n> \n> diff --git a/include/ipa/ipa_interface.h b/include/ipa/ipa_interface.h\n> index 2c5eb1fd524311cb..c393b64f6aa1a9d5 100644\n> --- a/include/ipa/ipa_interface.h\n> +++ b/include/ipa/ipa_interface.h\n> @@ -7,14 +7,50 @@\n> #ifndef __LIBCAMERA_IPA_INTERFACE_H__\n> #define __LIBCAMERA_IPA_INTERFACE_H__\n> \n> +#include \n> +#include \n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \"v4l2_controls.h\"\n> +\n> namespace libcamera {\n> \n> +struct IPAStream {\n> +\tunsigned int pixelFormat;\n> +\tSize size;\n> +};\n> +\n> +struct IPABuffer {\n> +\tunsigned int id;\n> +\tBufferMemory memory;\n> +};\n> +\n> +struct IPAOperationData {\n> +\tunsigned int operation;\n> +\tstd::vector data;\n> +\tstd::vector controls;\n> +\tstd::vector v4l2controls;\n> +};\n> +\n> class IPAInterface\n> {\n> public:\n> \tvirtual ~IPAInterface() {}\n> \n> \tvirtual int init() = 0;\n> +\n> +\tvirtual void configure(const std::map &streamConfig,\n> +\t\t\t const std::map &entityControls) = 0;\n> +\n> +\tvirtual void mapBuffers(const std::vector &buffers) = 0;\n> +\tvirtual void unmapBuffers(const std::vector &ids) = 0;\n> +\n> +\tvirtual void processEvent(const IPAOperationData &data) = 0;\n> +\tSignal queueFrameAction;\n> };\n> \n> } /* namespace libcamera */\n> diff --git a/src/ipa/ipa_vimc.cpp b/src/ipa/ipa_vimc.cpp\n> index 70d28fe24be2e67f..620dc25f456e9f44 100644\n> --- a/src/ipa/ipa_vimc.cpp\n> +++ b/src/ipa/ipa_vimc.cpp\n> @@ -29,7 +29,12 @@ public:\n> \tIPAVimc();\n> \t~IPAVimc();\n> \n> -\tint init();\n> +\tint init() override;\n> +\tvoid configure(const std::map &streamConfig,\n> +\t\t const std::map &entityControls) override {}\n> +\tvoid mapBuffers(const std::vector &buffers) override {}\n> +\tvoid unmapBuffers(const std::vector &ids) override {}\n> +\tvoid processEvent(const IPAOperationData &event) override {}\n> \n> private:\n> \tvoid initTrace();\n> diff --git a/src/libcamera/ipa_interface.cpp b/src/libcamera/ipa_interface.cpp\n> index d7d8ca8881efcf66..6eee8a43305c7921 100644\n> --- a/src/libcamera/ipa_interface.cpp\n> +++ b/src/libcamera/ipa_interface.cpp\n> @@ -10,13 +10,146 @@\n> /**\n> * \\file ipa_interface.h\n> * \\brief Image Processing Algorithm interface\n> + *\n> + * Pipeline handlers communicate with IPAs through a C++ interface defined by\n> + * the IPAInterface class. The class defines high-level methods and signals to\n> + * configure the IPA, notify it of events, and receive actions back from the\n> + * IPA.\n> + *\n> + * Pipeline handlers define the set of events and actions used to communicate\n> + * with their IPA. These are collectively referred to IPA operations and define\n\ns/referred to/referred to as/\n\n> + * the pipeline handler-specific IPA protocol. Each operation defines the data\n> + * that it carries, and how the data is encoded in the IPAOperationData\n> + * structure.\n> + *\n> + * IPAs can be isolated in a separate process. This implies that all arguments\n> + * to the IPA interface may need to be transferred over IPC. The IPA interface\n> + * thus uses serialisable data types only. The IPA interface defines custom data\n> + * structures that mirror core libcamera structures when the latter are not\n> + * suitable, such as IPAStream to carry StreamConfiguration data.\n> + *\n> + * Synchronous communication between pipeline handlers and IPAs can thus be\n> + * costly. For that reason, the interface operates asynchronously. This implies\n\n\"Due to IPC, Synchronous communication between pipeline handlers and\nIPAs can be costly.\"\n\nReviewed-by: Laurent Pinchart \n\n> + * that methods don't return a status, and that both methods and signals may\n> + * copy their arguments.\n> */\n> \n> namespace libcamera {\n> \n> +/**\n> + * \\struct IPAStream\n> + * \\brief Stream configuration for the IPA interface\n> + *\n> + * The IPAStream structure stores stream configuration parameters needed by the\n> + * IPAInterface::configure() method. It mirrors the StreamConfiguration class\n> + * that is not suitable for this purpose due to not being serialisable.\n> + */\n> +\n> +/**\n> + * \\var IPAStream::pixelFormat\n> + * \\brief The stream pixel format\n> + */\n> +\n> +/**\n> + * \\var IPAStream::size\n> + * \\brief The stream size in pixels\n> + */\n> +\n> +/**\n> + * \\struct IPABuffer\n> + * \\brief Buffer information for the IPA interface\n> + *\n> + * The IPABuffer structure associates buffer memory with a unique ID. It is\n> + * used to map buffers to the IPA with IPAInterface::mapBuffers(), after which\n> + * buffers will be identified by their ID in the IPA interface.\n> + */\n> +\n> +/**\n> + * \\var IPABuffer::id\n> + * \\brief The buffer unique ID\n> + *\n> + * Buffers mapped to the IPA are identified by numerical unique IDs. The IDs\n> + * are chosen by the pipeline handler to fulfil the following constraints:\n> + *\n> + * - IDs shall be positive integers different than zero\n> + * - IDs shall be unique among all mapped buffers\n> + *\n> + * When buffers are unmapped with IPAInterface::unmapBuffers() their IDs are\n> + * freed and may be reused for new buffer mappings.\n> + */\n> +\n> +/**\n> + * \\var IPABuffer::memory\n> + * \\brief The buffer memory description\n> + *\n> + * The memory field stores the dmabuf handle and size for each plane of the\n> + * buffer.\n> + */\n> +\n> +/**\n> + * \\struct IPAOperationData\n> + * \\brief Parameters for IPA operations\n> + *\n> + * The IPAOperationData structure carries parameters for the IPA operations\n> + * performed through the IPAInterface::processEvent() method and the\n> + * IPAInterface::queueFrameAction signal.\n> + */\n> +\n> +/**\n> + * \\var IPAOperationData::operation\n> + * \\brief IPA protocol operation\n> + *\n> + * The operation field describes which operation the receiver shall perform. It\n> + * defines, through the IPA protocol, how the other fields of the structure are\n> + * interpreted. The protocol freely assigns numerical values to operations.\n> + */\n> +\n> +/**\n> + * \\var IPAOperationData::data\n> + * \\brief Operation integer data\n> + *\n> + * The interpretation and position of different values in the array are defined\n> + * by the IPA protocol.\n> + */\n> +\n> +/**\n> + * \\var IPAOperationData::controls\n> + * \\brief Operation controls data\n> + *\n> + * The interpretation and position of different values in the array are defined\n> + * by the IPA protocol.\n> + */\n> +\n> +/**\n> + * \\var IPAOperationData::v4l2controls\n> + * \\brief Operation V4L2 controls data\n> + *\n> + * The interpretation and position of different values in the array are defined\n> + * by the IPA protocol.\n> + */\n> +\n> /**\n> * \\class IPAInterface\n> * \\brief Interface for IPA implementation\n> + *\n> + * Every pipeline handler in libcamera may attach all or some of its cameras to\n> + * an Image Processing Algorithm (IPA) module. An IPA module is developed for a\n> + * specific pipeline handler and each pipeline handler may have multiple\n> + * compatible IPA implementations, both open and closed source.\n> + *\n> + * To allow for multiple IPA modules for the same pipeline handler, a standard\n> + * interface for the pipeline handler and IPA communication is needed.\n> + * IPAInterface is this interface.\n> + *\n> + * The interface defines base data types and methods to exchange data. On top of\n> + * this, each pipeline handler is responsible for defining specific operations\n> + * that make up its IPA protocol, shared by all IPA modules compatible with the\n> + * pipeline handler.\n> + *\n> + * The pipeline handler shall use the IPAManager to locate a compatible\n> + * IPAInterface. The interface may then be used to interact with the IPA module.\n> + *\n> + * \\todo Add reference to how pipelines shall document their protocol.\n> */\n> \n> /**\n> @@ -24,4 +157,90 @@ namespace libcamera {\n> * \\brief Initialise the IPAInterface\n> */\n> \n> +/**\n> + * \\fn IPAInterface::configure()\n> + * \\brief Configure the IPA stream and sensor settings\n> + * \\param[in] streamConfig Configuration of all active streams\n> + * \\param[in] entityControls Controls provided by the pipeline entities\n> + *\n> + * This method shall be called when the camera is started to inform the IPA of\n> + * the camera's streams and the sensor settings. The meaning of the numerical\n> + * keys in the \\a streamConfig and \\a entityControls maps is defined by the IPA\n> + * protocol.\n> + */\n> +\n> +/**\n> + * \\fn IPAInterface::mapBuffers()\n> + * \\brief Map buffers shared between the pipeline handler and the IPA\n> + * \\param[in] buffers List of buffers to map\n> + *\n> + * This method informs the IPA module of memory buffers set up by the pipeline\n> + * handler that the IPA needs to access. It provides dmabuf file handles for\n> + * each buffer, and associates the buffers with unique numerical IDs.\n> + *\n> + * IPAs shall map the dmabuf file handles to their address space and keep a\n> + * cache of the mappings, indexed by the buffer numerical IDs. The IDs are used\n> + * in all other IPA interface methods to refer to buffers, including the\n> + * unmapBuffers() method.\n> + *\n> + * All buffers that the pipeline handler wishes to share with an IPA shall be\n> + * mapped with this method. Buffers may be mapped all at once with a single\n> + * call, or mapped and unmapped dynamically at runtime, depending on the IPA\n> + * protocol. Regardless of the protocol, all buffers mapped at a given time\n> + * shall have unique numerical IDs.\n> + *\n> + * The numerical IDs have no meaning defined by the IPA interface, and IPA\n> + * protocols shall not give them any specific meaning either. They should be\n> + * treated as opaque handles by IPAs, with the only exception that ID zero is\n> + * invalid.\n> + *\n> + * \\sa unmapBuffers()\n> + *\n> + * \\todo Provide a generic implementation of mapBuffers and unmapBuffers for\n> + * IPAs\n> + */\n> +\n> +/**\n> + * \\fn IPAInterface::unmapBuffers()\n> + * \\brief Unmap buffers shared by the pipeline to the IPA\n> + * \\param[in] ids List of buffer IDs to unmap\n> + *\n> + * This method removes mappings set up with mapBuffers(). Buffers may be\n> + * unmapped all at once with a single call, or selectively at runtime, depending\n> + * on the IPA protocol. Numerical IDs of unmapped buffers may be reused when\n> + * mapping new buffers.\n> + *\n> + * \\sa mapBuffers()\n> + */\n> +\n> +/**\n> + * \\fn IPAInterface::processEvent()\n> + * \\brief Process an event from the pipeline handler\n> + * \\param[in] data IPA operation data\n> + *\n> + * This operation is used by pipeline handlers to inform the IPA module of\n> + * events that occurred during the on-going capture operation.\n> + *\n> + * The event notified by the pipeline handler with this method is handled by the\n> + * IPA, which interprets the operation parameters according to the separately\n> + * documented IPA protocol.\n> + */\n> +\n> +/**\n> + * \\var IPAInterface::queueFrameAction\n> + * \\brief Queue an action associated with a frame to the pipeline handler\n> + * \\param[in] frame The frame number for the action\n> + * \\param[in] data IPA operation data\n> + *\n> + * This signal is emitted when the IPA wishes to queue a FrameAction on the\n> + * pipeline. The pipeline is still responsible for the scheduling of the action\n> + * on its timeline.\n> + *\n> + * This signal is emitted by the IPA to queue an action to be executed by the\n> + * pipeline handler on a frame. The type of action is identified by the\n> + * \\a data.operation field, as defined by the IPA protocol, and the rest of the\n> + * \\a data is interpreted accordingly. The pipeline handler shall queue the\n> + * action and execute it as appropriate.\n> + */\n> +\n> } /* namespace libcamera */\n> diff --git a/src/libcamera/proxy/ipa_proxy_linux.cpp b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> index 62fcb529e1c7e4ff..41bd965f02842e44 100644\n> --- a/src/libcamera/proxy/ipa_proxy_linux.cpp\n> +++ b/src/libcamera/proxy/ipa_proxy_linux.cpp\n> @@ -26,7 +26,12 @@ public:\n> \tIPAProxyLinux(IPAModule *ipam);\n> \t~IPAProxyLinux();\n> \n> -\tint init();\n> +\tint init() override { return 0; }\n> +\tvoid configure(const std::map &streamConfig,\n> +\t\t const std::map &entityControls) override {}\n> +\tvoid mapBuffers(const std::vector &buffers) override {}\n> +\tvoid unmapBuffers(const std::vector &ids) override {}\n> +\tvoid processEvent(const IPAOperationData &event) override {}\n> \n> private:\n> \tvoid readyRead(IPCUnixSocket *ipc);\n> @@ -36,13 +41,6 @@ private:\n> \tIPCUnixSocket *socket_;\n> };\n> \n> -int IPAProxyLinux::init()\n> -{\n> -\tLOG(IPAProxy, Debug) << \"initializing IPA via dummy proxy!\";\n> -\n> -\treturn 0;\n> -}\n> -\n> IPAProxyLinux::IPAProxyLinux(IPAModule *ipam)\n> \t: proc_(nullptr), socket_(nullptr)\n> {","headers":{"Return-Path":"","Received":["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 A666661564\n\tfor ;\n\tFri, 11 Oct 2019 14:25:58 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 1338933A;\n\tFri, 11 Oct 2019 14:25:57 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1570796758;\n\tbh=iBHvQm0aqA9SkwS2ESGVIiCq5aBUCY77xLOkhRnKbR0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kJ5GAMTSwy1vYD3xn54tqUsjjIG5eoM2KxKLYfO1IogS4D17i2Kwi6Zbg68pf3GzC\n\tulDHNtpz8ueoiYZtxtORcgy8pCb5P0x0G2kySWsxLLt0PnP7Z5Jz+fM2RwazaH3X8l\n\tmitk7Y1oq/fBGgACoREU92gxlIgNoXDjHsLWXgeg=","Date":"Fri, 11 Oct 2019 15:25:56 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20191011122556.GB4882@pendragon.ideasonboard.com>","References":"<20191011032216.2175173-1-niklas.soderlund@ragnatech.se>\n\t<20191011032216.2175173-7-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20191011032216.2175173-7-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v6 6/9] libcamera: ipa: Extend to\n\tsupport IPA interactions","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","X-List-Received-Date":"Fri, 11 Oct 2019 12:25:58 -0000"}}]