{"id":8484,"url":"https://patchwork.libcamera.org/api/covers/8484/?format=json","web_url":"https://patchwork.libcamera.org/cover/8484/","project":{"id":1,"url":"https://patchwork.libcamera.org/api/projects/1/?format=json","name":"libcamera","link_name":"libcamera","list_id":"libcamera_core","list_email":"libcamera-devel@lists.libcamera.org","web_url":"","scm_url":"","webscm_url":""},"msgid":"<20200628231934.29025-1-laurent.pinchart@ideasonboard.com>","date":"2020-06-28T23:19:25","name":"[libcamera-devel,v1,0/9] Support passing custom data to IPA configure()","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/?format=json","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"mbox":"https://patchwork.libcamera.org/cover/8484/mbox/","series":[{"id":1054,"url":"https://patchwork.libcamera.org/api/series/1054/?format=json","web_url":"https://patchwork.libcamera.org/project/libcamera/list/?series=1054","date":"2020-06-28T23:19:25","name":"Support passing custom data to IPA configure()","version":1,"mbox":"https://patchwork.libcamera.org/series/1054/mbox/"}],"comments":"https://patchwork.libcamera.org/api/covers/8484/comments/","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 E0AE1C2E66\n\tfor ;\n\tSun, 28 Jun 2020 23:19:43 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1FBD160A16;\n\tMon, 29 Jun 2020 01:19:43 +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 531EB603B6\n\tfor ;\n\tMon, 29 Jun 2020 01:19:41 +0200 (CEST)","from pendragon.bb.dnainternet.fi (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id AFEF54FB;\n\tMon, 29 Jun 2020 01:19:40 +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=\"H8VD6hvY\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1593386380;\n\tbh=XoQGfpi3wtVYo3WXvtn35D4UxK/1Hv3rWKRqaTqDgVk=;\n\th=From:To:Cc:Subject:Date:From;\n\tb=H8VD6hvYOvx9iM0SXyku4UI3GZrm/RDEcINQ9EROgdX0m8zOfeY4HQwG8MP5oxAmy\n\tExxI0cZCRLCrzHWX9Uufbrtozhrxs6d51Cjx3v79S8t7PPLrBG9n/JwHEzr8uw+H5J\n\tZImZy1+mhWGseM0/7bMvOsirDztX4zzRazamPRP0=","From":"Laurent Pinchart ","To":"libcamera-devel@lists.libcamera.org","Date":"Mon, 29 Jun 2020 02:19:25 +0300","Message-Id":"<20200628231934.29025-1-laurent.pinchart@ideasonboard.com>","X-Mailer":"git-send-email 2.27.0","MIME-Version":"1.0","Subject":"[libcamera-devel] [PATCH v1 0/9] Support passing custom data to IPA\n\tconfigure()","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","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "},"content":"The IPAInterface::configure() function passes miscellaneous\nconfiguration data to the IPA, but offers no way for pipeline handlers\nto pass custom data, or to receive custom data back. This has led to the\nRaspberry Pi IPA protocol using custom frame actions and events to pass\ninitialization data to the IPA and to receive initial sensor\nconfiguration back. To support this, the Raspberry Pi IPA needs to start\nthe IPA thread before configure() time, while it was meant to be started\nwhen starting the camera.\n\nThis patch series addresses this issue by adding a way to pass custom\nconfiguration data to the IPA and receive data back. It starts with two\nsmall drive-by cleanups in patches 1/9 and 2/9. Patch 3/9 performs the\nbulk of the work by modifying IPAInterface::configure(). The C API isn't\naddressed yet, as larger refactoring is considered to handle the C <->\nC++ translation in a better way.\n\nPatch 4/9 is another small drive-by fix. Patch 5/9 follows by moving\ncode around in the Raspberry Pi pipeline handler to prepare for the\nupcoming changes, and patches 6/9 and 7/9 replace custom events and\nframe actions with the new IPA-specific data passed to configure().\nPatch 8/9 moves the IPA start and stop to the camera start and stop,\naligning the implementation with how the IPAInterface API is meant to be\nused. Patch 9/9 is an additional simplification made possible by the\nrest of the series.\n\nI'm not entirely happy with patch 7/9 as it handles the data received\nfrom the IPA manually. The Raspberry Pi IPA can send both staggered\nwrite configuration data and sensor gain and exposure controls, with\nboth set of parameters being optional. Better code is needed to\nconstruct a protocol on top of the IPAOperationData passed to\nIPAInterface::configure(). This also applies to processEvent and\nqueueFrameAction, and I think we should investigate usage of something\nsimilar to protobuf to handle both the IPA C <-> C++ translation and the\nIPA protocol built on top of IPAOperationData. Clever ideas are welcome,\nbut this will hopefully not be a blocker for this patch series.\n\nNaush, I'm not entirely sure about 9/9, could you confirm that\nRPI_IPA_ACTION_V4L2_SET_ISP never occurs in the stop state ?\n\nAn additional improvement that I believe could be done on top of this\nwould be to merge the RPI_IPA_ACTION_RUN_ISP,\nRPI_IPA_ACTION_RUN_ISP_AND_DROP_FRAME and RPI_IPA_ACTION_V4L2_SET_ISP\nactions. The first two are essentially the same with an option, which\ncould be passed to IPAOperationData::data, and the last one carries a\nset of controls that could be passed in one go.\nRPI_IPA_ACTION_EMBEDDED_COMPLETE could also be merged, as it's always\nsent with RPI_IPA_ACTION_RUN_ISP(_AND_DROP_FRAME), but there may be a\nfuture use case to keep it separate that I can't think about now.\n\nSimilarly, RPI_IPA_ACTION_V4L2_SET_STAGGERED and\nRPI_IPA_ACTION_STATS_METADATA_COMPLETE could also be merged into a\nsingle action.\n\nThis would lower the number of cross-thread calls, improving\nperformances.\n\nLaurent Pinchart (9):\n\n libcamera: ipa: Document the parameters of the IPA C configure\n function\n libcamera: ipa_context_wrapper: Fix bad copy&paste in comment\n libcamera: ipa_interface: Add support for custom IPA data to\n configure()\n libcamera: pipeline: raspberrypi: Constify parameter to\n StaggeredCtrl::set()\n libcamera: pipeline: raspberrypi: Move configureIPA() to RPiCameraData\n ipa: raspberrypi: Pass lens shading table through configure() function\n ipa: raspberrypi: Pass sensor config back from configure()\n libcamera: pipeline: raspberrypi: Start IPA when starting camera\n libcamera: pipeline: raspberrypi: Don't handle any actions in stop\n state\n\n .../libcamera/internal/ipa_context_wrapper.h | 4 +-\n include/libcamera/ipa/ipa_interface.h | 4 +-\n include/libcamera/ipa/raspberrypi.h | 8 +-\n src/ipa/libipa/ipa_interface_wrapper.cpp | 5 +-\n src/ipa/raspberrypi/raspberrypi.cpp | 64 +++---\n src/ipa/rkisp1/rkisp1.cpp | 8 +-\n src/ipa/vimc/vimc.cpp | 4 +-\n src/libcamera/ipa_context_wrapper.cpp | 10 +-\n src/libcamera/ipa_interface.cpp | 12 +\n .../pipeline/raspberrypi/raspberrypi.cpp | 205 +++++++++---------\n .../pipeline/raspberrypi/staggered_ctrl.cpp | 2 +-\n .../pipeline/raspberrypi/staggered_ctrl.h | 2 +-\n src/libcamera/pipeline/rkisp1/rkisp1.cpp | 4 +-\n src/libcamera/proxy/ipa_proxy_linux.cpp | 4 +-\n src/libcamera/proxy/ipa_proxy_thread.cpp | 11 +-\n test/ipa/ipa_wrappers_test.cpp | 8 +-\n 16 files changed, 207 insertions(+), 148 deletions(-)"}