[{"id":15466,"web_url":"https://patchwork.libcamera.org/comment/15466/","msgid":"<YEBDjW65FWwWZAZw@pendragon.ideasonboard.com>","date":"2021-03-04T02:18:53","subject":"Re: [libcamera-devel] [PATCH v4 4/4] pipeline: raspberrypi: Allow\n\teither strict or non-strict buffer matching","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Naush,\n\nThank you for the patch.\n\nOn Tue, Mar 02, 2021 at 03:11:11PM +0000, Naushir Patuck wrote:\n> A new flag, StrictBufferMatching, is used to control the behavior of\n> the embedded and bayer buffer matching routine.\n> \n> If set to true (default), we reject and drop all bayer frames that do\n> not have a matching embedded data buffer and vice-versa. This guarantees\n> the IPA will always have the correct frame exposure and gain values to\n> use.\n> \n> If set to false, we use bayer frames that do not have a matching\n> embedded data buffer. In this case, IPA will use use the ControlList\n> passed to it for gain and exposure values.\n> \n> Additionally, allow external stream buffers to behave as if\n> StrictBufferMatching = false since we are not allowed to drop them.\n> \n> Signed-off-by: Naushir Patuck <naush@raspberrypi.com>\n> Tested-by: David Plowman <david.plowman@raspberrypi.com>\n\nI've skipped this one for now, as there's ongoing discussion in v3. I'll\npush the first three patches once my compile tests complete.\n\n> ---\n>  .../pipeline/raspberrypi/raspberrypi.cpp      | 31 ++++++++++++++++---\n>  1 file changed, 26 insertions(+), 5 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> index 5ae2551957f8..549d6147cd60 100644\n> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> @@ -46,6 +46,22 @@ namespace libcamera {\n>  \n>  LOG_DEFINE_CATEGORY(RPI)\n>  \n> +/*\n> + * Set this to true to reject and drop all bayer frames that do not have a\n> + * matching embedded data buffer and vice-versa. This guarantees the IPA will\n> + * always have the correct frame exposure and gain values to use.\n> + *\n> + * Set this to false to use bayer frames that do not have a matching embedded\n> + * data buffer. In this case, IPA will use use our local history for gain and\n> + * exposure values, occasional frame drops may cause these number to be out of\n> + * sync for a short period.\n> + *\n> + * \\todo Ideally, this property should be set by the application, but we don't\n> + * have any mechanism to pass generic properties into a pipeline handler at\n> + * present.\n> + */\n> +static const bool StrictBufferMatching = true;\n> +\n>  namespace {\n>  \n>  bool isRaw(PixelFormat &pixFmt)\n> @@ -1725,13 +1741,13 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em\n>  \t\tembeddedBuffer = nullptr;\n>  \t\twhile (!embeddedQueue_.empty()) {\n>  \t\t\tFrameBuffer *b = embeddedQueue_.front();\n> -\t\t\tif (!unicam_[Unicam::Embedded].isExternal() && b->metadata().timestamp < ts) {\n> +\t\t\tif (b->metadata().timestamp < ts) {\n>  \t\t\t\tembeddedQueue_.pop();\n>  \t\t\t\tunicam_[Unicam::Embedded].queueBuffer(b);\n>  \t\t\t\tembeddedRequeueCount++;\n>  \t\t\t\tLOG(RPI, Warning) << \"Dropping unmatched input frame in stream \"\n>  \t\t\t\t\t\t  << unicam_[Unicam::Embedded].name();\n> -\t\t\t} else if (unicam_[Unicam::Embedded].isExternal() || b->metadata().timestamp == ts) {\n> +\t\t\t} else if (b->metadata().timestamp == ts) {\n>  \t\t\t\t/* We pop the item from the queue lower down. */\n>  \t\t\t\tembeddedBuffer = b;\n>  \t\t\t\tbreak;\n> @@ -1745,10 +1761,15 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em\n>  \n>  \t\t\tLOG(RPI, Debug) << \"Could not find matching embedded buffer\";\n>  \n> -\t\t\tif (!sensorMetadata_) {\n> +\t\t\tif (unicam_[Unicam::Embedded].isExternal() ||\n> +\t\t\t    !StrictBufferMatching || !sensorMetadata_) {\n>  \t\t\t\t/*\n> -\t\t\t\t * If there is no sensor metadata, simply return the\n> -\t\t\t\t * first bayer frame in the queue.\n> +\t\t\t\t * If any of the following is true:\n> +\t\t\t\t * - This is an external stream buffer (so cannot be dropped).\n> +\t\t\t\t * - We do not care about strict buffer matching.\n> +\t\t\t\t * - There is no sensor metadata present.\n> +\t\t\t\t * we can simply return the first bayer frame in the queue\n> +\t\t\t\t * without a matching embedded buffer.\n>  \t\t\t\t */\n>  \t\t\t\tLOG(RPI, Debug) << \"Returning bayer frame without a match\";\n>  \t\t\t\tbayerFrame = std::move(bayerQueue_.front());","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 1B490BD80C\n\tfor <parsemail@patchwork.libcamera.org>;\n\tThu,  4 Mar 2021 02:19:25 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 9A5AF602E7;\n\tThu,  4 Mar 2021 03:19:24 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7CD2F60106\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tThu,  4 Mar 2021 03:19:23 +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 04B4F27A;\n\tThu,  4 Mar 2021 03:19:22 +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=\"ts9xWidS\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614824363;\n\tbh=4V61Aphz4Jy5Un3UMYCHK94EO1k3jBGn83uPdEOYFhc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=ts9xWidSEtKyRPJN513H3sZkaEpa8ZAe0Nf1BZb8NbnonBQokLa2ltFs7/5Qo9DcM\n\tT0lD0ZlZr93ukR8WY9guXEk0DqFWU+kSBnewzKsxgK9ZboUz8lJAnvWtCBQ7xf/UN4\n\tSeEzU/vo4nDgSUYDlX95Tuw8IDHrZ8AHl79lmXnQ=","Date":"Thu, 4 Mar 2021 04:18:53 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Naushir Patuck <naush@raspberrypi.com>","Message-ID":"<YEBDjW65FWwWZAZw@pendragon.ideasonboard.com>","References":"<20210302151111.212591-1-naush@raspberrypi.com>\n\t<20210302151111.212591-5-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210302151111.212591-5-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v4 4/4] pipeline: raspberrypi: Allow\n\teither strict or non-strict buffer matching","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>","Cc":"libcamera-devel@lists.libcamera.org","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>"}}]