[{"id":31678,"web_url":"https://patchwork.libcamera.org/comment/31678/","msgid":"<172851397750.532453.14297864457977065014@ping.linuxembedded.co.uk>","date":"2024-10-09T22:46:17","subject":"Re: [PATCH 0/5] Add InfoFrame","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Harvey,\n\nQuoting Harvey Yang (2024-10-09 08:41:18)\n> Hi folks,\n> \n> This series of patches adds MailBox, Pool and InfoFrame as helper\n> classes, which are useful to save duplicated code in the upcoming\n> mtkisp7 implementation. We also think that they're helpful for other\n> pipeline handlers.\n> \n> It's based on the `Add VirtualPipelineHandler` series, as one\n> of the patch updates struct FrameBuffer.\n> \n> We can also check if we should append DmaBufAllocator to support\n> strideAlign and scanAlign, so that InfoFramePool can save some\n> duplicated code.\n> \n> It passed the gitlab pipeline:\n> https://gitlab.freedesktop.org/chenghaoyang/libcamera/-/pipelines/1286167\n> \n> Please take a look. Thanks!\n\nthese certainly look interesting, but without users it's hard to fully\ngrasp what they're doing.\n\nI think at least the MailBox and Pool would likely benefit from unit\ntests to be added. Adding unit tests would also show how they can be\nused (or not used) and would help a lot with reviewing.\n\nOr possibly as well, converting other pipeline handlers to use the\nhelpers if it they are to be common helpers would really be useful to\nsee the power they are delivering.\n\n--\nKieran\n\n\n> \n> BR,\n> Harvey\n> \n> Han-Lin Chen (2):\n> libcamera: Add mailbox template helper\n> libcamera: format: Extend plane size calculation to accept scanline\n> alignment\n> \n> Harvey Yang (3):\n> libcamera: Add template for pool implementation\n> libcamera: Add stride in FrameBuffer::Plane\n> libcamera: Add InfoFrame implememtation\n> \n> include/libcamera/framebuffer.h | 1 +\n> include/libcamera/internal/formats.h | 5 +-\n> include/libcamera/internal/info_frame.h | 105 ++++++++\n> include/libcamera/internal/mailbox.h | 75 ++++++\n> include/libcamera/internal/meson.build | 3 +\n> include/libcamera/internal/pool.h | 119 ++++++++++\n> src/libcamera/dma_buf_allocator.cpp | 3 +-\n> src/libcamera/formats.cpp | 17 +-\n> src/libcamera/framebuffer.cpp | 5 +\n> src/libcamera/info_frame.cpp | 302 ++++++++++++++++++++++++\n> src/libcamera/ipa_data_serializer.cpp | 3 +\n> src/libcamera/mailbox.cpp | 60 +++++\n> src/libcamera/meson.build | 3 +\n> src/libcamera/pool.cpp | 70 ++++++\n> 14 files changed, 764 insertions(+), 7 deletions(-)\n> create mode 100644 include/libcamera/internal/info_frame.h\n> create mode 100644 include/libcamera/internal/mailbox.h\n> create mode 100644 include/libcamera/internal/pool.h\n> create mode 100644 src/libcamera/info_frame.cpp\n> create mode 100644 src/libcamera/mailbox.cpp\n> create mode 100644 src/libcamera/pool.cpp\n> \n> -- \n> 2.47.0.rc0.187.ge670bccf7e-goog\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 DC9E2C32E0\n\tfor ;\n\tWed, 9 Oct 2024 22:46:22 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0D5DD618C5;\n\tThu, 10 Oct 2024 00:46:22 +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 87A2D618C5\n\tfor ;\n\tThu, 10 Oct 2024 00:46:20 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(cpc89244-aztw30-2-0-cust6594.18-1.cable.virginm.net [86.31.185.195])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id D4CBE226;\n\tThu, 10 Oct 2024 00:44:42 +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=\"dXht74oV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1728513882;\n\tbh=720pnMyBSZZSFMQj4Vs2AriZEtmR3M+okDW8iQ9ykto=;\n\th=In-Reply-To:References:Subject:From:Cc:To:Date:From;\n\tb=dXht74oVwoFfVtEuONbOr96F8Ta+VN72u/7dpCYLZtQDWw2v9tYAl90DmnnT+UhNa\n\trZkw8lemJPq25Pg7moWiEwRwuyGzFeL8LfMQZHrJuABeKzDKBlVusWtXwAlgzxGD8H\n\tntZufuI+A89aKQNjwEiK2UcG1fT368D2SRgdl37E=","Content-Type":"text/plain; charset=\"utf-8\"","MIME-Version":"1.0","Content-Transfer-Encoding":"quoted-printable","In-Reply-To":"<20241009074642.2965791-1-chenghaoyang@chromium.org>","References":"<20241009074642.2965791-1-chenghaoyang@chromium.org>","Subject":"Re: [PATCH 0/5] Add InfoFrame","From":"Kieran Bingham ","Cc":"Harvey Yang ","To":"Harvey Yang ,\n\tlibcamera-devel@lists.libcamera.org","Date":"Wed, 09 Oct 2024 23:46:17 +0100","Message-ID":"<172851397750.532453.14297864457977065014@ping.linuxembedded.co.uk>","User-Agent":"alot/0.10","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\" "}}]