Patch Detail
Show a patch.
GET /api/patches/9387/?format=api
{ "id": 9387, "url": "https://patchwork.libcamera.org/api/patches/9387/?format=api", "web_url": "https://patchwork.libcamera.org/patch/9387/", "project": { "id": 1, "url": "https://patchwork.libcamera.org/api/projects/1/?format=api", "name": "libcamera", "link_name": "libcamera", "list_id": "libcamera_core", "list_email": "libcamera-devel@lists.libcamera.org", "web_url": "", "scm_url": "", "webscm_url": "" }, "msgid": "<20200826110926.67192-4-paul.elder@ideasonboard.com>", "date": "2020-08-26T11:09:12", "name": "[libcamera-devel,RFC,03/17] libcamera: IPA: raspberrypi: Use generated IPARPiInterface", "commit_ref": null, "pull_url": null, "state": "superseded", "archived": false, "hash": "b926841d6b63667788110da5a054800f940f0f96", "submitter": { "id": 17, "url": "https://patchwork.libcamera.org/api/people/17/?format=api", "name": "Paul Elder", "email": "paul.elder@ideasonboard.com" }, "delegate": null, "mbox": "https://patchwork.libcamera.org/patch/9387/mbox/", "series": [ { "id": 1243, "url": "https://patchwork.libcamera.org/api/series/1243/?format=api", "web_url": "https://patchwork.libcamera.org/project/libcamera/list/?series=1243", "date": "2020-08-26T11:09:09", "name": "[libcamera-devel,RFC,01/17] IPA: IPC: raspberrypi: Add data definition and generated header", "version": 1, "mbox": "https://patchwork.libcamera.org/series/1243/mbox/" } ], "comments": "https://patchwork.libcamera.org/api/patches/9387/comments/", "check": "pending", "checks": "https://patchwork.libcamera.org/api/patches/9387/checks/", "tags": {}, "headers": { "Return-Path": "<libcamera-devel-bounces@lists.libcamera.org>", "X-Original-To": "parsemail@patchwork.libcamera.org", "Delivered-To": "parsemail@patchwork.libcamera.org", "Received": [ "from lancelot.ideasonboard.com (lancelot.ideasonboard.com\n\t[92.243.16.209])\n\tby patchwork.libcamera.org (Postfix) with ESMTPS id 09F64BD87E\n\tfor <parsemail@patchwork.libcamera.org>;\n\tWed, 26 Aug 2020 11:09:57 +0000 (UTC)", "from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id C979E628F9;\n\tWed, 26 Aug 2020 13:09:56 +0200 (CEST)", "from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B8C206037B\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tWed, 26 Aug 2020 13:09:55 +0200 (CEST)", "from pyrite.rasen.tech (unknown\n\t[IPv6:2400:4051:61:600:2c71:1b79:d06d:5032])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D649F9CE;\n\tWed, 26 Aug 2020 13:09:53 +0200 (CEST)" ], "Authentication-Results": "lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"CmK8ib4s\"; dkim-atps=neutral", "DKIM-Signature": "v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1598440195;\n\tbh=sjG9vxRkOt3lgltbA6fyd10Wfr0WcwxEZFAHE+5wVbs=;\n\th=From:To:Cc:Subject:Date:In-Reply-To:References:From;\n\tb=CmK8ib4swlSXI2CQ3Q4vpRuT+PHzl/1ioBLZ++0SlhZrOEtmIAkZYLdVT1z+6j2r8\n\tYs+psb36XHmDkrjivYDsO1Asnsj7nYb3S0v+3Qw1TanjAnhS4Uxs8crzsJn10zLx3j\n\tm2dECUfixLLXvzWDrOztlRZGgKD1cPcIy6CtdGXc=", "From": "Paul Elder <paul.elder@ideasonboard.com>", "To": "libcamera-devel@lists.libcamera.org", "Date": "Wed, 26 Aug 2020 20:09:12 +0900", "Message-Id": "<20200826110926.67192-4-paul.elder@ideasonboard.com>", "X-Mailer": "git-send-email 2.27.0", "In-Reply-To": "<20200826110926.67192-1-paul.elder@ideasonboard.com>", "References": "<20200826110926.67192-1-paul.elder@ideasonboard.com>", "MIME-Version": "1.0", "Subject": "[libcamera-devel] [RFC PATCH 03/17] libcamera: IPA: raspberrypi:\n\tUse generated IPARPiInterface", "X-BeenThere": "libcamera-devel@lists.libcamera.org", "X-Mailman-Version": "2.1.29", "Precedence": "list", "List-Id": "<libcamera-devel.lists.libcamera.org>", "List-Unsubscribe": "<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>", "List-Archive": "<https://lists.libcamera.org/pipermail/libcamera-devel/>", "List-Post": "<mailto:libcamera-devel@lists.libcamera.org>", "List-Help": "<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>", "List-Subscribe": "<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>", "Content-Type": "text/plain; charset=\"us-ascii\"", "Content-Transfer-Encoding": "7bit", "Errors-To": "libcamera-devel-bounces@lists.libcamera.org", "Sender": "\"libcamera-devel\" <libcamera-devel-bounces@lists.libcamera.org>" }, "content": "This patch shows how the IPA would use the generated IPA interface.\n\nLike the patch for the pipeline, this should be straightforward. It\nmainly switches to using the new data structures.\n\nA couple things that are noteworthy:\n\nipaCreate returns an IPAInterface, instead of a struct ipa_context. This\nmeans we don't need the C interface anymore!\n\nFile descriptors *cannot* be sent via ControlList and be expected to\nstill work on the other side. This is because ControlList is expected to\nbe sent during the stream. Every time an fd is sent across the process\nboundary, it is practically duped, which means we'll eventually run out\nof fds. Keeping a map of fds cannot be maintained practically either,\nsince the map on the other side of the process boundary will not be\nnotified of changes, among other nasty issues. Another big nasty issue\nis that we'll probably have a memory leak due to having so many fds open\npointing to the same resource.\n\nThe solution that I've come up with is to send one fd as a\nFileDescriptor from the pipeline handler that the IPA can use, and\nanother fd as an int32, that the IPA can send back to the pipeline\nhandler, and expect the pipeline handler to be able to use it. This is\nsomething to keep in mind during design of the IPA interface.\n\nSigned-off-by: Paul Elder <paul.elder@ideasonboard.com>\n---\n src/ipa/raspberrypi/raspberrypi.cpp | 107 ++++++++++++++++------------\n 1 file changed, 62 insertions(+), 45 deletions(-)", "diff": "diff --git a/src/ipa/raspberrypi/raspberrypi.cpp b/src/ipa/raspberrypi/raspberrypi.cpp\nindex 4557016c..2c088742 100644\n--- a/src/ipa/raspberrypi/raspberrypi.cpp\n+++ b/src/ipa/raspberrypi/raspberrypi.cpp\n@@ -19,6 +19,7 @@\n #include <libcamera/ipa/ipa_interface.h>\n #include <libcamera/ipa/ipa_module_info.h>\n #include <libcamera/ipa/raspberrypi.h>\n+#include <libcamera/ipa/raspberrypi_wrapper.h>\n #include <libcamera/request.h>\n #include <libcamera/span.h>\n \n@@ -60,7 +61,7 @@ namespace libcamera {\n \n LOG_DEFINE_CATEGORY(IPARPI)\n \n-class IPARPi : public IPAInterface\n+class IPARPi : public IPARPiInterface\n {\n public:\n \tIPARPi()\n@@ -82,12 +83,12 @@ public:\n \n \tvoid configure(const CameraSensorInfo &sensorInfo,\n \t\t const std::map<unsigned int, IPAStream> &streamConfig,\n-\t\t const std::map<unsigned int, const ControlInfoMap &> &entityControls,\n-\t\t const IPAOperationData &data,\n-\t\t IPAOperationData *response) override;\n+\t\t const std::map<unsigned int, const ControlInfoMap> &entityControls,\n+\t\t const RPiConfigureParams &data,\n+\t\t RPiConfigureParams *response) override;\n \tvoid mapBuffers(const std::vector<IPABuffer> &buffers) override;\n \tvoid unmapBuffers(const std::vector<unsigned int> &ids) override;\n-\tvoid processEvent(const IPAOperationData &event) override;\n+\tvoid processEvent(const RPiEventParams &event) override;\n \n private:\n \tvoid setMode(const CameraSensorInfo &sensorInfo);\n@@ -143,6 +144,11 @@ private:\n \tunsigned int mistrust_count_;\n \t/* LS table allocation passed in from the pipeline handler. */\n \tFileDescriptor lsTableHandle_;\n+\t/*\n+\t * LS table allocation passed in from the pipeline handler,\n+\t * in the context of the pipeline handler.\n+\t */\n+\tint32_t lsTableHandlePH_;\n \tvoid *lsTable_;\n };\n \n@@ -192,15 +198,13 @@ void IPARPi::setMode(const CameraSensorInfo &sensorInfo)\n \n void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n \t\t [[maybe_unused]] const std::map<unsigned int, IPAStream> &streamConfig,\n-\t\t const std::map<unsigned int, const ControlInfoMap &> &entityControls,\n-\t\t const IPAOperationData &ipaConfig,\n-\t\t IPAOperationData *result)\n+\t\t const std::map<unsigned int, const ControlInfoMap> &entityControls,\n+\t\t const RPiConfigureParams &ipaConfig,\n+\t\t RPiConfigureParams *result)\n {\n \tif (entityControls.empty())\n \t\treturn;\n \n-\tresult->operation = 0;\n-\n \tunicam_ctrls_ = entityControls.at(0);\n \tisp_ctrls_ = entityControls.at(1);\n \t/* Setup a metadata ControlList to output metadata. */\n@@ -222,11 +226,13 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n \t\thelper_->GetDelays(exposureDelay, gainDelay);\n \t\tsensorMetadata = helper_->SensorEmbeddedDataPresent();\n \n-\t\tresult->data.push_back(gainDelay);\n-\t\tresult->data.push_back(exposureDelay);\n-\t\tresult->data.push_back(sensorMetadata);\n+\t\tRPiConfigurePayload payload = {};\n+\t\tpayload.op_ = RPI_IPA_CONFIG_STAGGERED_WRITE;\n+\t\tpayload.staggeredWriteResult_.gainDelay_ = gainDelay;\n+\t\tpayload.staggeredWriteResult_.exposureDelay_ = exposureDelay;\n+\t\tpayload.staggeredWriteResult_.sensorMetadata_ = sensorMetadata;\n \n-\t\tresult->operation |= RPI_IPA_CONFIG_STAGGERED_WRITE;\n+\t\tresult->payload_.push_back(payload);\n \t}\n \n \t/* Re-assemble camera mode using the sensor info. */\n@@ -274,15 +280,23 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n \tif (agcStatus.shutter_time != 0.0 && agcStatus.analogue_gain != 0.0) {\n \t\tControlList ctrls(unicam_ctrls_);\n \t\tapplyAGC(&agcStatus, ctrls);\n-\t\tresult->controls.push_back(ctrls);\n \n-\t\tresult->operation |= RPI_IPA_CONFIG_SENSOR;\n+\t\tRPiConfigurePayload payload = {};\n+\t\tpayload.op_ = RPI_IPA_CONFIG_SENSOR;\n+\t\tpayload.controls_ = ctrls;\n+\n+\t\tresult->payload_.push_back(payload);\n \t}\n \n \tlastMode_ = mode_;\n \n \t/* Store the lens shading table pointer and handle if available. */\n-\tif (ipaConfig.operation & RPI_IPA_CONFIG_LS_TABLE) {\n+\tauto lens = std::find_if(ipaConfig.payload_.begin(),\n+\t\t\t\t ipaConfig.payload_.end(),\n+\t\t\t\t [] (const RPiConfigurePayload &p) {\n+\t\t\t\t\t return p.op_ == RPI_IPA_CONFIG_LS_TABLE;\n+\t\t\t\t });\n+\tif (lens != ipaConfig.payload_.end()) {\n \t\t/* Remove any previous table, if there was one. */\n \t\tif (lsTable_) {\n \t\t\tmunmap(lsTable_, MAX_LS_GRID_SIZE);\n@@ -290,7 +304,8 @@ void IPARPi::configure(const CameraSensorInfo &sensorInfo,\n \t\t}\n \n \t\t/* Map the LS table buffer into user space. */\n-\t\tlsTableHandle_ = FileDescriptor(ipaConfig.data[0]);\n+\t\tlsTableHandle_ = FileDescriptor(lens->lsTableHandle_);\n+\t\tlsTableHandlePH_ = lens->lsTableHandleStatic_;\n \t\tif (lsTableHandle_.isValid()) {\n \t\t\tlsTable_ = mmap(nullptr, MAX_LS_GRID_SIZE, PROT_READ | PROT_WRITE,\n \t\t\t\t\tMAP_SHARED, lsTableHandle_.fd(), 0);\n@@ -335,11 +350,11 @@ void IPARPi::unmapBuffers(const std::vector<unsigned int> &ids)\n \t}\n }\n \n-void IPARPi::processEvent(const IPAOperationData &event)\n+void IPARPi::processEvent(const RPiEventParams &event)\n {\n-\tswitch (event.operation) {\n+\tswitch (event.ev_) {\n \tcase RPI_IPA_EVENT_SIGNAL_STAT_READY: {\n-\t\tunsigned int bufferId = event.data[0];\n+\t\tunsigned int bufferId = event.bufferId_;\n \n \t\tif (++check_count_ != frame_count_) /* assert here? */\n \t\t\tLOG(IPARPI, Error) << \"WARNING: Prepare/Process mismatch!!!\";\n@@ -348,17 +363,17 @@ void IPARPi::processEvent(const IPAOperationData &event)\n \n \t\treportMetadata();\n \n-\t\tIPAOperationData op;\n-\t\top.operation = RPI_IPA_ACTION_STATS_METADATA_COMPLETE;\n-\t\top.data = { bufferId & RPiIpaMask::ID };\n-\t\top.controls = { libcameraMetadata_ };\n+\t\tRPiActionParams op;\n+\t\top.op_ = RPI_IPA_ACTION_STATS_METADATA_COMPLETE;\n+\t\top.statsComplete_.bufferId_ = { bufferId & RPiIpaMask::ID };\n+\t\top.statsComplete_.controls_ = { libcameraMetadata_ };\n \t\tqueueFrameAction.emit(0, op);\n \t\tbreak;\n \t}\n \n \tcase RPI_IPA_EVENT_SIGNAL_ISP_PREPARE: {\n-\t\tunsigned int embeddedbufferId = event.data[0];\n-\t\tunsigned int bayerbufferId = event.data[1];\n+\t\tunsigned int embeddedbufferId = event.ispPrepare_.embeddedbufferId_;\n+\t\tunsigned int bayerbufferId = event.ispPrepare_.bayerbufferId_;\n \n \t\t/*\n \t\t * At start-up, or after a mode-switch, we may want to\n@@ -368,23 +383,23 @@ void IPARPi::processEvent(const IPAOperationData &event)\n \t\tprepareISP(embeddedbufferId);\n \n \t\t/* Ready to push the input buffer into the ISP. */\n-\t\tIPAOperationData op;\n+\t\tRPiActionParams op;\n \t\tif (++frame_count_ > hide_count_)\n-\t\t\top.operation = RPI_IPA_ACTION_RUN_ISP;\n+\t\t\top.op_ = RPI_IPA_ACTION_RUN_ISP;\n \t\telse\n-\t\t\top.operation = RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME;\n-\t\top.data = { bayerbufferId & RPiIpaMask::ID };\n+\t\t\top.op_ = RPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME;\n+\t\top.bufferId_ = { bayerbufferId & RPiIpaMask::ID };\n \t\tqueueFrameAction.emit(0, op);\n \t\tbreak;\n \t}\n \n \tcase RPI_IPA_EVENT_QUEUE_REQUEST: {\n-\t\tqueueRequest(event.controls[0]);\n+\t\tqueueRequest(event.controls_);\n \t\tbreak;\n \t}\n \n \tdefault:\n-\t\tLOG(IPARPI, Error) << \"Unknown event \" << event.operation;\n+\t\tLOG(IPARPI, Error) << \"Unknown event \" << event.ev_;\n \t\tbreak;\n \t}\n }\n@@ -489,6 +504,8 @@ void IPARPi::queueRequest(const ControlList &controls)\n \t/* Clear the return metadata buffer. */\n \tlibcameraMetadata_.clear();\n \n+\tLOG(IPARPI, Info) << \"Request ctrl length: \" << controls.size();\n+\n \tfor (auto const &ctrl : controls) {\n \t\tLOG(IPARPI, Info) << \"Request ctrl: \"\n \t\t\t\t << controls::controls.at(ctrl.first)->name()\n@@ -698,9 +715,9 @@ void IPARPi::queueRequest(const ControlList &controls)\n \n void IPARPi::returnEmbeddedBuffer(unsigned int bufferId)\n {\n-\tIPAOperationData op;\n-\top.operation = RPI_IPA_ACTION_EMBEDDED_COMPLETE;\n-\top.data = { bufferId & RPiIpaMask::ID };\n+\tRPiActionParams op;\n+\top.op_ = RPI_IPA_ACTION_EMBEDDED_COMPLETE;\n+\top.bufferId_ = { bufferId & RPiIpaMask::ID };\n \tqueueFrameAction.emit(0, op);\n }\n \n@@ -763,9 +780,9 @@ void IPARPi::prepareISP(unsigned int bufferId)\n \t\t\tapplyDPC(dpcStatus, ctrls);\n \n \t\tif (!ctrls.empty()) {\n-\t\t\tIPAOperationData op;\n-\t\t\top.operation = RPI_IPA_ACTION_V4L2_SET_ISP;\n-\t\t\top.controls.push_back(ctrls);\n+\t\t\tRPiActionParams op;\n+\t\t\top.op_ = RPI_IPA_ACTION_V4L2_SET_ISP;\n+\t\t\top.controls_ = ctrls;\n \t\t\tqueueFrameAction.emit(0, op);\n \t\t}\n \t}\n@@ -823,9 +840,9 @@ void IPARPi::processStats(unsigned int bufferId)\n \t\tControlList ctrls(unicam_ctrls_);\n \t\tapplyAGC(&agcStatus, ctrls);\n \n-\t\tIPAOperationData op;\n-\t\top.operation = RPI_IPA_ACTION_V4L2_SET_STAGGERED;\n-\t\top.controls.push_back(ctrls);\n+\t\tRPiActionParams op;\n+\t\top.op_ = RPI_IPA_ACTION_V4L2_SET_STAGGERED;\n+\t\top.controls_ = ctrls;\n \t\tqueueFrameAction.emit(0, op);\n \t}\n }\n@@ -1056,7 +1073,7 @@ void IPARPi::applyLS(const struct AlscStatus *lsStatus, ControlList &ctrls)\n \t\t.grid_width = w,\n \t\t.grid_stride = w,\n \t\t.grid_height = h,\n-\t\t.dmabuf = lsTableHandle_.fd(),\n+\t\t.dmabuf = lsTableHandlePH_,\n \t\t.ref_transform = 0,\n \t\t.corner_sampled = 1,\n \t\t.gain_format = GAIN_FORMAT_U4P10\n@@ -1136,9 +1153,9 @@ const struct IPAModuleInfo ipaModuleInfo = {\n \t\"raspberrypi\",\n };\n \n-struct ipa_context *ipaCreate()\n+IPAInterface *ipaCreate()\n {\n-\treturn new IPAInterfaceWrapper(std::make_unique<IPARPi>());\n+\treturn new IPARPi();\n }\n \n }; /* extern \"C\" */\n", "prefixes": [ "libcamera-devel", "RFC", "03/17" ] }