[{"id":14471,"web_url":"https://patchwork.libcamera.org/comment/14471/","msgid":"<20210107163419.t2k6gjfkt3xbhydm@uno.localdomain>","date":"2021-01-07T16:34:19","subject":"Re: [libcamera-devel] [PATCH v2 10/11] libcamera: ipu3: Add helper\n\tfor parameter and statistic buffers","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Tue, Dec 29, 2020 at 05:03:17PM +0100, Niklas Söderlund wrote:\n> Add a helper class to aid in associating a parameter and statistic\n> buffer with each request queued to the pipeline. The helper helps with\n> tracking the state of the extra buffers and in completing the request\n> once all extra processing is done.\n>\n> This change only adds the helper more work is needed to integrate it\n> with the pipeline and an IPA.\n>\n> Signed-off-by: Niklas Söderlund \n\nLooks good!\n\nReviewed-by: Jacopo Mondi \n\nThanks\n j\n\n> ---\n> * Changes since v1\n> - Move mapping of buffers to IPA to the pipeline handler and a separate\n> patch.\n> - Move all pipeline interactions out of helper.\n> ---\n> src/libcamera/pipeline/ipu3/frames.cpp | 141 ++++++++++++++++++++++++\n> src/libcamera/pipeline/ipu3/frames.h | 63 +++++++++++\n> src/libcamera/pipeline/ipu3/meson.build | 1 +\n> 3 files changed, 205 insertions(+)\n> create mode 100644 src/libcamera/pipeline/ipu3/frames.cpp\n> create mode 100644 src/libcamera/pipeline/ipu3/frames.h\n>\n> diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp\n> new file mode 100644\n> index 0000000000000000..1ab078db056ce193\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/ipu3/frames.cpp\n> @@ -0,0 +1,141 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * frames.cpp - Intel IPU3 Frames helper\n> + */\n> +\n> +#include \"frames.h\"\n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/pipeline_handler.h\"\n> +#include \"libcamera/internal/v4l2_videodevice.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(IPU3)\n> +\n> +IPU3Frames::IPU3Frames()\n> +\t: nextId_(0)\n> +{\n> +}\n> +\n> +void IPU3Frames::init(const std::vector> ¶mBuffers,\n> +\t\t const std::vector> &statBuffers)\n> +{\n> +\tfor (const std::unique_ptr &buffer : paramBuffers)\n> +\t\tavailableParamBuffers_.push(buffer.get());\n> +\n> +\tfor (const std::unique_ptr &buffer : statBuffers)\n> +\t\tavailableStatBuffers_.push(buffer.get());\n> +\n> +\tnextId_ = 0;\n> +\tframeInfo_.clear();\n> +}\n> +\n> +void IPU3Frames::clear()\n> +{\n> +\tavailableParamBuffers_ = {};\n> +\tavailableStatBuffers_ = {};\n> +}\n> +\n> +IPU3Frames::Info *IPU3Frames::create(Request *request)\n> +{\n> +\tunsigned int id = nextId_++;\n> +\n> +\tif (availableParamBuffers_.empty()) {\n> +\t\tLOG(IPU3, Error) << \"Parameters buffer underrun\";\n> +\t\treturn nullptr;\n> +\t}\n> +\tFrameBuffer *paramBuffer = availableParamBuffers_.front();\n> +\n> +\tif (availableStatBuffers_.empty()) {\n> +\t\tLOG(IPU3, Error) << \"Statisitc buffer underrun\";\n> +\t\treturn nullptr;\n> +\t}\n> +\tFrameBuffer *statBuffer = availableStatBuffers_.front();\n> +\n> +\tavailableParamBuffers_.pop();\n> +\tavailableStatBuffers_.pop();\n> +\n> +\tstd::unique_ptr info = std::make_unique();\n> +\n> +\tinfo->id = id;\n> +\tinfo->request = request;\n> +\tinfo->rawBuffer = nullptr;\n> +\tinfo->paramBuffer = paramBuffer;\n> +\tinfo->statBuffer = statBuffer;\n> +\tinfo->paramFilled = false;\n> +\tinfo->paramDequeued = false;\n> +\tinfo->metadataProcessed = false;\n> +\n> +\tframeInfo_[id] = std::move(info);\n> +\n> +\treturn frameInfo_[id].get();\n> +}\n> +\n> +bool IPU3Frames::tryComplete(IPU3Frames::Info *info)\n> +{\n> +\tRequest *request = info->request;\n> +\n> +\tif (request->hasPendingBuffers())\n> +\t\treturn false;\n> +\n> +\tif (!info->metadataProcessed)\n> +\t\treturn false;\n> +\n> +\tif (!info->paramDequeued)\n> +\t\treturn false;\n> +\n> +\t/* Return params and stat buffer for reuse. */\n> +\tavailableParamBuffers_.push(info->paramBuffer);\n> +\tavailableStatBuffers_.push(info->statBuffer);\n> +\n> +\t/* Delete the extended frame information. */\n> +\tframeInfo_.erase(info->id);\n> +\n> +\treturn true;\n> +}\n> +\n> +IPU3Frames::Info *IPU3Frames::find(unsigned int id)\n> +{\n> +\tconst auto &itInfo = frameInfo_.find(id);\n> +\n> +\tif (itInfo != frameInfo_.end())\n> +\t\treturn itInfo->second.get();\n> +\n> +\treturn nullptr;\n> +}\n> +\n> +IPU3Frames::Info *IPU3Frames::find(FrameBuffer *buffer)\n> +{\n> +\tfor (auto const &itInfo : frameInfo_) {\n> +\t\tInfo *info = itInfo.second.get();\n> +\n> +\t\tfor (auto const itBuffers : info->request->buffers())\n> +\t\t\tif (itBuffers.second == buffer)\n> +\t\t\t\treturn info;\n> +\n> +\t\tif (info->rawBuffer == buffer || info->paramBuffer == buffer ||\n> +\t\t info->statBuffer == buffer)\n> +\t\t\treturn info;\n> +\t}\n> +\n> +\treturn nullptr;\n> +}\n> +\n> +IPU3Frames::Info *IPU3Frames::find(Request *request)\n> +{\n> +\tfor (auto const &itInfo : frameInfo_) {\n> +\t\tInfo *info = itInfo.second.get();\n> +\n> +\t\tif (info->request == request)\n> +\t\t\treturn info;\n> +\t}\n> +\n> +\treturn nullptr;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h\n> new file mode 100644\n> index 0000000000000000..06b2678be4fb04d7\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/ipu3/frames.h\n> @@ -0,0 +1,63 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * frames.h - Intel IPU3 Frames helper\n> + */\n> +#ifndef __LIBCAMERA_PIPELINE_IPU3_FRAMES_H__\n> +#define __LIBCAMERA_PIPELINE_IPU3_FRAMES_H__\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class FrameBuffer;\n> +class IPAProxy;\n> +class PipelineHandler;\n> +class Request;\n> +class V4L2VideoDevice;\n> +struct IPABuffer;\n> +\n> +class IPU3Frames\n> +{\n> +public:\n> +\tstruct Info {\n> +\t\tunsigned int id;\n> +\t\tRequest *request;\n> +\n> +\t\tFrameBuffer *rawBuffer;\n> +\t\tFrameBuffer *paramBuffer;\n> +\t\tFrameBuffer *statBuffer;\n> +\n> +\t\tbool paramFilled;\n> +\t\tbool paramDequeued;\n> +\t\tbool metadataProcessed;\n> +\t};\n> +\n> +\tIPU3Frames();\n> +\n> +\tvoid init(const std::vector> ¶mBuffers,\n> +\t\t const std::vector> &statBuffers);\n> +\tvoid clear();\n> +\n> +\tInfo *create(Request *request);\n> +\tbool tryComplete(IPU3Frames::Info *info);\n> +\n> +\tInfo *find(unsigned int id);\n> +\tInfo *find(FrameBuffer *buffer);\n> +\tInfo *find(Request *request);\n> +\n> +private:\n> +\tstd::queue availableParamBuffers_;\n> +\tstd::queue availableStatBuffers_;\n> +\n> +\tunsigned int nextId_;\n> +\tstd::map> frameInfo_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_PIPELINE_IPU3_FRAMES_H__ */\n> diff --git a/src/libcamera/pipeline/ipu3/meson.build b/src/libcamera/pipeline/ipu3/meson.build\n> index d60e07ae6ccac2bc..a1b0b31ac5bcf864 100644\n> --- a/src/libcamera/pipeline/ipu3/meson.build\n> +++ b/src/libcamera/pipeline/ipu3/meson.build\n> @@ -2,6 +2,7 @@\n>\n> libcamera_sources += files([\n> 'cio2.cpp',\n> + 'frames.cpp',\n> 'imgu.cpp',\n> 'ipu3.cpp',\n> ])\n> --\n> 2.29.2\n>\n> _______________________________________________\n> libcamera-devel mailing list\n> libcamera-devel@lists.libcamera.org\n> https://lists.libcamera.org/listinfo/libcamera-devel","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 7AF92C3256\n\tfor ;\n\tThu, 7 Jan 2021 16:34:07 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1C0326347A;\n\tThu, 7 Jan 2021 17:34:07 +0100 (CET)","from relay5-d.mail.gandi.net (relay5-d.mail.gandi.net\n\t[217.70.183.197])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 899B762013\n\tfor ;\n\tThu, 7 Jan 2021 17:34:05 +0100 (CET)","from uno.localdomain (unknown [93.56.78.48])\n\t(Authenticated sender: jacopo@jmondi.org)\n\tby relay5-d.mail.gandi.net (Postfix) with ESMTPSA id 8BA7E1C0003;\n\tThu, 7 Jan 2021 16:34:04 +0000 (UTC)"],"X-Originating-IP":"93.56.78.48","Date":"Thu, 7 Jan 2021 17:34:19 +0100","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20210107163419.t2k6gjfkt3xbhydm@uno.localdomain>","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-11-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201229160318.77536-11-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v2 10/11] libcamera: ipu3: Add helper\n\tfor parameter and statistic buffers","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14500,"web_url":"https://patchwork.libcamera.org/comment/14500/","msgid":"","date":"2021-01-10T21:01:29","subject":"Re: [libcamera-devel] [PATCH v2 10/11] libcamera: ipu3: Add helper\n\tfor parameter and statistic buffers","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 Tue, Dec 29, 2020 at 05:03:17PM +0100, Niklas Söderlund wrote:\n> Add a helper class to aid in associating a parameter and statistic\n> buffer with each request queued to the pipeline. The helper helps with\n> tracking the state of the extra buffers and in completing the request\n> once all extra processing is done.\n> \n> This change only adds the helper more work is needed to integrate it\n> with the pipeline and an IPA.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> * Changes since v1\n> - Move mapping of buffers to IPA to the pipeline handler and a separate\n> patch.\n> - Move all pipeline interactions out of helper.\n> ---\n> src/libcamera/pipeline/ipu3/frames.cpp | 141 ++++++++++++++++++++++++\n> src/libcamera/pipeline/ipu3/frames.h | 63 +++++++++++\n> src/libcamera/pipeline/ipu3/meson.build | 1 +\n> 3 files changed, 205 insertions(+)\n> create mode 100644 src/libcamera/pipeline/ipu3/frames.cpp\n> create mode 100644 src/libcamera/pipeline/ipu3/frames.h\n> \n> diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp\n> new file mode 100644\n> index 0000000000000000..1ab078db056ce193\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/ipu3/frames.cpp\n> @@ -0,0 +1,141 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * frames.cpp - Intel IPU3 Frames helper\n> + */\n> +\n> +#include \"frames.h\"\n> +\n> +#include \n> +#include \n> +\n> +#include \"libcamera/internal/pipeline_handler.h\"\n> +#include \"libcamera/internal/v4l2_videodevice.h\"\n> +\n> +namespace libcamera {\n> +\n> +LOG_DECLARE_CATEGORY(IPU3)\n> +\n> +IPU3Frames::IPU3Frames()\n> +\t: nextId_(0)\n> +{\n> +}\n> +\n> +void IPU3Frames::init(const std::vector> ¶mBuffers,\n> +\t\t const std::vector> &statBuffers)\n> +{\n> +\tfor (const std::unique_ptr &buffer : paramBuffers)\n> +\t\tavailableParamBuffers_.push(buffer.get());\n> +\n> +\tfor (const std::unique_ptr &buffer : statBuffers)\n> +\t\tavailableStatBuffers_.push(buffer.get());\n> +\n> +\tnextId_ = 0;\n> +\tframeInfo_.clear();\n> +}\n> +\n> +void IPU3Frames::clear()\n> +{\n> +\tavailableParamBuffers_ = {};\n> +\tavailableStatBuffers_ = {};\n> +}\n> +\n> +IPU3Frames::Info *IPU3Frames::create(Request *request)\n> +{\n> +\tunsigned int id = nextId_++;\n> +\n> +\tif (availableParamBuffers_.empty()) {\n> +\t\tLOG(IPU3, Error) << \"Parameters buffer underrun\";\n> +\t\treturn nullptr;\n> +\t}\n> +\tFrameBuffer *paramBuffer = availableParamBuffers_.front();\n> +\n> +\tif (availableStatBuffers_.empty()) {\n> +\t\tLOG(IPU3, Error) << \"Statisitc buffer underrun\";\n> +\t\treturn nullptr;\n> +\t}\n> +\tFrameBuffer *statBuffer = availableStatBuffers_.front();\n> +\n> +\tavailableParamBuffers_.pop();\n> +\tavailableStatBuffers_.pop();\n> +\n> +\tstd::unique_ptr info = std::make_unique();\n\nIt would be nice if we could preallocate the Info instances to lower the\nnumber of dynamic allocations at runtime.\n\n> +\n> +\tinfo->id = id;\n> +\tinfo->request = request;\n> +\tinfo->rawBuffer = nullptr;\n> +\tinfo->paramBuffer = paramBuffer;\n> +\tinfo->statBuffer = statBuffer;\n> +\tinfo->paramFilled = false;\n> +\tinfo->paramDequeued = false;\n> +\tinfo->metadataProcessed = false;\n> +\n> +\tframeInfo_[id] = std::move(info);\n> +\n> +\treturn frameInfo_[id].get();\n> +}\n> +\n> +bool IPU3Frames::tryComplete(IPU3Frames::Info *info)\n> +{\n> +\tRequest *request = info->request;\n> +\n> +\tif (request->hasPendingBuffers())\n> +\t\treturn false;\n> +\n> +\tif (!info->metadataProcessed)\n> +\t\treturn false;\n> +\n> +\tif (!info->paramDequeued)\n> +\t\treturn false;\n> +\n> +\t/* Return params and stat buffer for reuse. */\n> +\tavailableParamBuffers_.push(info->paramBuffer);\n> +\tavailableStatBuffers_.push(info->statBuffer);\n> +\n> +\t/* Delete the extended frame information. */\n> +\tframeInfo_.erase(info->id);\n> +\n> +\treturn true;\n> +}\n> +\n> +IPU3Frames::Info *IPU3Frames::find(unsigned int id)\n> +{\n> +\tconst auto &itInfo = frameInfo_.find(id);\n> +\n> +\tif (itInfo != frameInfo_.end())\n> +\t\treturn itInfo->second.get();\n> +\n> +\treturn nullptr;\n> +}\n> +\n> +IPU3Frames::Info *IPU3Frames::find(FrameBuffer *buffer)\n> +{\n> +\tfor (auto const &itInfo : frameInfo_) {\n> +\t\tInfo *info = itInfo.second.get();\n> +\n> +\t\tfor (auto const itBuffers : info->request->buffers())\n> +\t\t\tif (itBuffers.second == buffer)\n> +\t\t\t\treturn info;\n> +\n> +\t\tif (info->rawBuffer == buffer || info->paramBuffer == buffer ||\n> +\t\t info->statBuffer == buffer)\n> +\t\t\treturn info;\n> +\t}\n> +\n> +\treturn nullptr;\n> +}\n> +\n> +IPU3Frames::Info *IPU3Frames::find(Request *request)\n> +{\n> +\tfor (auto const &itInfo : frameInfo_) {\n> +\t\tInfo *info = itInfo.second.get();\n> +\n> +\t\tif (info->request == request)\n> +\t\t\treturn info;\n> +\t}\n> +\n> +\treturn nullptr;\n> +}\n\nI think this function isn't used.\n\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h\n> new file mode 100644\n> index 0000000000000000..06b2678be4fb04d7\n> --- /dev/null\n> +++ b/src/libcamera/pipeline/ipu3/frames.h\n> @@ -0,0 +1,63 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2020, Google Inc.\n> + *\n> + * frames.h - Intel IPU3 Frames helper\n> + */\n> +#ifndef __LIBCAMERA_PIPELINE_IPU3_FRAMES_H__\n> +#define __LIBCAMERA_PIPELINE_IPU3_FRAMES_H__\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class FrameBuffer;\n> +class IPAProxy;\n> +class PipelineHandler;\n> +class Request;\n> +class V4L2VideoDevice;\n> +struct IPABuffer;\n> +\n> +class IPU3Frames\n> +{\n> +public:\n> +\tstruct Info {\n> +\t\tunsigned int id;\n> +\t\tRequest *request;\n> +\n> +\t\tFrameBuffer *rawBuffer;\n> +\t\tFrameBuffer *paramBuffer;\n> +\t\tFrameBuffer *statBuffer;\n> +\n> +\t\tbool paramFilled;\n> +\t\tbool paramDequeued;\n> +\t\tbool metadataProcessed;\n> +\t};\n> +\n> +\tIPU3Frames();\n> +\n> +\tvoid init(const std::vector> ¶mBuffers,\n> +\t\t const std::vector> &statBuffers);\n> +\tvoid clear();\n> +\n> +\tInfo *create(Request *request);\n> +\tbool tryComplete(IPU3Frames::Info *info);\n> +\n> +\tInfo *find(unsigned int id);\n> +\tInfo *find(FrameBuffer *buffer);\n> +\tInfo *find(Request *request);\n> +\n> +private:\n> +\tstd::queue availableParamBuffers_;\n> +\tstd::queue availableStatBuffers_;\n> +\n> +\tunsigned int nextId_;\n> +\tstd::map> frameInfo_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_PIPELINE_IPU3_FRAMES_H__ */\n> diff --git a/src/libcamera/pipeline/ipu3/meson.build b/src/libcamera/pipeline/ipu3/meson.build\n> index d60e07ae6ccac2bc..a1b0b31ac5bcf864 100644\n> --- a/src/libcamera/pipeline/ipu3/meson.build\n> +++ b/src/libcamera/pipeline/ipu3/meson.build\n> @@ -2,6 +2,7 @@\n> \n> libcamera_sources += files([\n> 'cio2.cpp',\n> + 'frames.cpp',\n> 'imgu.cpp',\n> 'ipu3.cpp',\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 BC76CBD80C\n\tfor ;\n\tSun, 10 Jan 2021 21:01:45 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 8AE896807A;\n\tSun, 10 Jan 2021 22:01:45 +0100 (CET)","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 4021360523\n\tfor ;\n\tSun, 10 Jan 2021 22:01:44 +0100 (CET)","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 03091DA;\n\tSun, 10 Jan 2021 22:01:42 +0100 (CET)"],"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=\"gKC2XfIK\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1610312503;\n\tbh=O2KjwP9U7+fybwoJl0J4x0z1eJd2YUJpN323IbRKlFo=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=gKC2XfIK2VhY0EqUoS5kvmzAzgp2nK4s3NmFG77fMna1QUNA388HjLOqsDzIgsggh\n\tqqLN97OdCsNSPw7CWyrlztTfTtEIxj0nrdKIeQD/RcVMHZ2kLXvTdfe76ldIZ3pYb8\n\taSAiwRtK427AH6vfJaYd16UUaqHXahc+zErAUSCU=","Date":"Sun, 10 Jan 2021 23:01:29 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-11-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20201229160318.77536-11-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v2 10/11] libcamera: ipu3: Add helper\n\tfor parameter and statistic buffers","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","Content-Type":"text/plain; charset=\"utf-8\"","Content-Transfer-Encoding":"base64","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":14916,"web_url":"https://patchwork.libcamera.org/comment/14916/","msgid":"","date":"2021-02-03T10:24:32","subject":"Re: [libcamera-devel] [PATCH v2 10/11] libcamera: ipu3: Add helper\n\tfor parameter and statistic buffers","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your comments.\n\nOn 2021-01-10 23:01:29 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Tue, Dec 29, 2020 at 05:03:17PM +0100, Niklas Söderlund wrote:\n> > Add a helper class to aid in associating a parameter and statistic\n> > buffer with each request queued to the pipeline. The helper helps with\n> > tracking the state of the extra buffers and in completing the request\n> > once all extra processing is done.\n> > \n> > This change only adds the helper more work is needed to integrate it\n> > with the pipeline and an IPA.\n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > * Changes since v1\n> > - Move mapping of buffers to IPA to the pipeline handler and a separate\n> > patch.\n> > - Move all pipeline interactions out of helper.\n> > ---\n> > src/libcamera/pipeline/ipu3/frames.cpp | 141 ++++++++++++++++++++++++\n> > src/libcamera/pipeline/ipu3/frames.h | 63 +++++++++++\n> > src/libcamera/pipeline/ipu3/meson.build | 1 +\n> > 3 files changed, 205 insertions(+)\n> > create mode 100644 src/libcamera/pipeline/ipu3/frames.cpp\n> > create mode 100644 src/libcamera/pipeline/ipu3/frames.h\n> > \n> > diff --git a/src/libcamera/pipeline/ipu3/frames.cpp b/src/libcamera/pipeline/ipu3/frames.cpp\n> > new file mode 100644\n> > index 0000000000000000..1ab078db056ce193\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/ipu3/frames.cpp\n> > @@ -0,0 +1,141 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * frames.cpp - Intel IPU3 Frames helper\n> > + */\n> > +\n> > +#include \"frames.h\"\n> > +\n> > +#include \n> > +#include \n> > +\n> > +#include \"libcamera/internal/pipeline_handler.h\"\n> > +#include \"libcamera/internal/v4l2_videodevice.h\"\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DECLARE_CATEGORY(IPU3)\n> > +\n> > +IPU3Frames::IPU3Frames()\n> > +\t: nextId_(0)\n> > +{\n> > +}\n> > +\n> > +void IPU3Frames::init(const std::vector> ¶mBuffers,\n> > +\t\t const std::vector> &statBuffers)\n> > +{\n> > +\tfor (const std::unique_ptr &buffer : paramBuffers)\n> > +\t\tavailableParamBuffers_.push(buffer.get());\n> > +\n> > +\tfor (const std::unique_ptr &buffer : statBuffers)\n> > +\t\tavailableStatBuffers_.push(buffer.get());\n> > +\n> > +\tnextId_ = 0;\n> > +\tframeInfo_.clear();\n> > +}\n> > +\n> > +void IPU3Frames::clear()\n> > +{\n> > +\tavailableParamBuffers_ = {};\n> > +\tavailableStatBuffers_ = {};\n> > +}\n> > +\n> > +IPU3Frames::Info *IPU3Frames::create(Request *request)\n> > +{\n> > +\tunsigned int id = nextId_++;\n> > +\n> > +\tif (availableParamBuffers_.empty()) {\n> > +\t\tLOG(IPU3, Error) << \"Parameters buffer underrun\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\tFrameBuffer *paramBuffer = availableParamBuffers_.front();\n> > +\n> > +\tif (availableStatBuffers_.empty()) {\n> > +\t\tLOG(IPU3, Error) << \"Statisitc buffer underrun\";\n> > +\t\treturn nullptr;\n> > +\t}\n> > +\tFrameBuffer *statBuffer = availableStatBuffers_.front();\n> > +\n> > +\tavailableParamBuffers_.pop();\n> > +\tavailableStatBuffers_.pop();\n> > +\n> > +\tstd::unique_ptr info = std::make_unique();\n> \n> It would be nice if we could preallocate the Info instances to lower the\n> number of dynamic allocations at runtime.\n\nI agree, I have some crazy ideas for how we can make pipelines life \neasier by interacting with the pipeline depth at a core Camera level. \nThat would allow this allocation unnecessary. I will add a todo here so \nwe don't forget it.\n\n> \n> > +\n> > +\tinfo->id = id;\n> > +\tinfo->request = request;\n> > +\tinfo->rawBuffer = nullptr;\n> > +\tinfo->paramBuffer = paramBuffer;\n> > +\tinfo->statBuffer = statBuffer;\n> > +\tinfo->paramFilled = false;\n> > +\tinfo->paramDequeued = false;\n> > +\tinfo->metadataProcessed = false;\n> > +\n> > +\tframeInfo_[id] = std::move(info);\n> > +\n> > +\treturn frameInfo_[id].get();\n> > +}\n> > +\n> > +bool IPU3Frames::tryComplete(IPU3Frames::Info *info)\n> > +{\n> > +\tRequest *request = info->request;\n> > +\n> > +\tif (request->hasPendingBuffers())\n> > +\t\treturn false;\n> > +\n> > +\tif (!info->metadataProcessed)\n> > +\t\treturn false;\n> > +\n> > +\tif (!info->paramDequeued)\n> > +\t\treturn false;\n> > +\n> > +\t/* Return params and stat buffer for reuse. */\n> > +\tavailableParamBuffers_.push(info->paramBuffer);\n> > +\tavailableStatBuffers_.push(info->statBuffer);\n> > +\n> > +\t/* Delete the extended frame information. */\n> > +\tframeInfo_.erase(info->id);\n> > +\n> > +\treturn true;\n> > +}\n> > +\n> > +IPU3Frames::Info *IPU3Frames::find(unsigned int id)\n> > +{\n> > +\tconst auto &itInfo = frameInfo_.find(id);\n> > +\n> > +\tif (itInfo != frameInfo_.end())\n> > +\t\treturn itInfo->second.get();\n> > +\n> > +\treturn nullptr;\n> > +}\n> > +\n> > +IPU3Frames::Info *IPU3Frames::find(FrameBuffer *buffer)\n> > +{\n> > +\tfor (auto const &itInfo : frameInfo_) {\n> > +\t\tInfo *info = itInfo.second.get();\n> > +\n> > +\t\tfor (auto const itBuffers : info->request->buffers())\n> > +\t\t\tif (itBuffers.second == buffer)\n> > +\t\t\t\treturn info;\n> > +\n> > +\t\tif (info->rawBuffer == buffer || info->paramBuffer == buffer ||\n> > +\t\t info->statBuffer == buffer)\n> > +\t\t\treturn info;\n> > +\t}\n> > +\n> > +\treturn nullptr;\n> > +}\n> > +\n> > +IPU3Frames::Info *IPU3Frames::find(Request *request)\n> > +{\n> > +\tfor (auto const &itInfo : frameInfo_) {\n> > +\t\tInfo *info = itInfo.second.get();\n> > +\n> > +\t\tif (info->request == request)\n> > +\t\t\treturn info;\n> > +\t}\n> > +\n> > +\treturn nullptr;\n> > +}\n> \n> I think this function isn't used.\n\nYou are correct, it was added for completeness as I had intentions to \nprofligate this to the RkISP1 pipeline handler. But if that happens it \nmay be added back, will drop for next version.\n\n> \n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/pipeline/ipu3/frames.h b/src/libcamera/pipeline/ipu3/frames.h\n> > new file mode 100644\n> > index 0000000000000000..06b2678be4fb04d7\n> > --- /dev/null\n> > +++ b/src/libcamera/pipeline/ipu3/frames.h\n> > @@ -0,0 +1,63 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2020, Google Inc.\n> > + *\n> > + * frames.h - Intel IPU3 Frames helper\n> > + */\n> > +#ifndef __LIBCAMERA_PIPELINE_IPU3_FRAMES_H__\n> > +#define __LIBCAMERA_PIPELINE_IPU3_FRAMES_H__\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +class FrameBuffer;\n> > +class IPAProxy;\n> > +class PipelineHandler;\n> > +class Request;\n> > +class V4L2VideoDevice;\n> > +struct IPABuffer;\n> > +\n> > +class IPU3Frames\n> > +{\n> > +public:\n> > +\tstruct Info {\n> > +\t\tunsigned int id;\n> > +\t\tRequest *request;\n> > +\n> > +\t\tFrameBuffer *rawBuffer;\n> > +\t\tFrameBuffer *paramBuffer;\n> > +\t\tFrameBuffer *statBuffer;\n> > +\n> > +\t\tbool paramFilled;\n> > +\t\tbool paramDequeued;\n> > +\t\tbool metadataProcessed;\n> > +\t};\n> > +\n> > +\tIPU3Frames();\n> > +\n> > +\tvoid init(const std::vector> ¶mBuffers,\n> > +\t\t const std::vector> &statBuffers);\n> > +\tvoid clear();\n> > +\n> > +\tInfo *create(Request *request);\n> > +\tbool tryComplete(IPU3Frames::Info *info);\n> > +\n> > +\tInfo *find(unsigned int id);\n> > +\tInfo *find(FrameBuffer *buffer);\n> > +\tInfo *find(Request *request);\n> > +\n> > +private:\n> > +\tstd::queue availableParamBuffers_;\n> > +\tstd::queue availableStatBuffers_;\n> > +\n> > +\tunsigned int nextId_;\n> > +\tstd::map> frameInfo_;\n> > +};\n> > +\n> > +} /* namespace libcamera */\n> > +\n> > +#endif /* __LIBCAMERA_PIPELINE_IPU3_FRAMES_H__ */\n> > diff --git a/src/libcamera/pipeline/ipu3/meson.build b/src/libcamera/pipeline/ipu3/meson.build\n> > index d60e07ae6ccac2bc..a1b0b31ac5bcf864 100644\n> > --- a/src/libcamera/pipeline/ipu3/meson.build\n> > +++ b/src/libcamera/pipeline/ipu3/meson.build\n> > @@ -2,6 +2,7 @@\n> > \n> > libcamera_sources += files([\n> > 'cio2.cpp',\n> > + 'frames.cpp',\n> > 'imgu.cpp',\n> > 'ipu3.cpp',\n> > ])\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","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 1D7C6BD161\n\tfor ;\n\tWed, 3 Feb 2021 10:24:36 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 869A16843C;\n\tWed, 3 Feb 2021 11:24:35 +0100 (CET)","from mail-ej1-x62c.google.com (mail-ej1-x62c.google.com\n\t[IPv6:2a00:1450:4864:20::62c])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CCDF460305\n\tfor ;\n\tWed, 3 Feb 2021 11:24:34 +0100 (CET)","by mail-ej1-x62c.google.com with SMTP id f14so10528201ejc.8\n\tfor ;\n\tWed, 03 Feb 2021 02:24:34 -0800 (PST)","from localhost (p4fca2458.dip0.t-ipconnect.de. [79.202.36.88])\n\tby smtp.gmail.com with ESMTPSA id\n\tu17sm792036ejr.59.2021.02.03.02.24.32\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tWed, 03 Feb 2021 02:24:33 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com\n\theader.b=\"keBeOSNN\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to;\n\tbh=eQBt7xW7jqx5nS8of7y/OajomLTvyxccyzvS/BUr9Jo=;\n\tb=keBeOSNNESkmsVZhSqNFHWc4iXgLVmmGdv4TPhA2MgRWulBfkijdcWtqQ+J7x0IrVI\n\tFoGz/x/y3v2m20Xkc7voZaYrc+htrF6+76DPnA3fMLpbQR4/AefdLsX4eW/SzkCoZ7Ev\n\ty4PNUizcJ/HUeVtFXs834B5H59lOp0rBEkseG08gKzYEvpx25ulEbV7o5/BfXbmI5cq+\n\tgWoHsqTHVTTBzzsjPB2RrYl+GeBlvP2gkJ83bL8F2m99KR2TdimnujB+O0tqopcpZmNb\n\t3f2/Dipd2UAXknPfo3YDAL0OJ7qpVRRlo10Fd2Nx5UZMD+/yD0H/cKLE8gr0zTsmfUrd\n\tmOOA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to;\n\tbh=eQBt7xW7jqx5nS8of7y/OajomLTvyxccyzvS/BUr9Jo=;\n\tb=PNzPQ10MUbaodNfW4dvjgbnLgY0HweN1tFXGsvn9APwplL2rWVbVIDg2UAbizTpmBc\n\tr2O++tro0wEnk5FidPcneqytvdGEgpwYxNaamdJZTL/kDtcUGxWxrASFxUB7Z8Tq8oh2\n\t3UX84Hsaovs+pqNl2eM6MpohRpYHPVQf2EMEGDinaStK5Sy/PwwjyOozUrDj2SRHtl/9\n\tDWM6nWNJulqaiWrJrsL1ZCBalYSgPVsnxu8wKcZ6f36m02KfnFXSzXuRztdpNZPaW6qq\n\tdFVspwInm6V3GDytCFQfcJeHMx3nR4EiHi7PiYjFIH1h3wqDU8P/FZ7Plb/IwqOST8kH\n\tzHMQ==","X-Gm-Message-State":"AOAM533W+XzqWg4AwiDalWqVQALb5yzqHm8OsH8sp/MMINuEq2vg0jqk\n\tvG/Mot540zJjiPnjmujdYG/FS5OQzVOn2Q==","X-Google-Smtp-Source":"ABdhPJyWd72kQY9xez5TJjgCwRHkAPCLIfC3G67wyaQRvDtZlNLAcAWk31hUC5/OgLrW8cqESYt7xg==","X-Received":"by 2002:a17:906:78a:: with SMTP id\n\tl10mr708297ejc.264.1612347874425; \n\tWed, 03 Feb 2021 02:24:34 -0800 (PST)","Date":"Wed, 3 Feb 2021 11:24:32 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Message-ID":"","References":"<20201229160318.77536-1-niklas.soderlund@ragnatech.se>\n\t<20201229160318.77536-11-niklas.soderlund@ragnatech.se>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v2 10/11] libcamera: ipu3: Add helper\n\tfor parameter and statistic buffers","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","Content-Type":"text/plain; charset=\"iso-8859-1\"","Content-Transfer-Encoding":"quoted-printable","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]