[{"id":15269,"web_url":"https://patchwork.libcamera.org/comment/15269/","msgid":"","date":"2021-02-21T23:09:32","subject":"Re: [libcamera-devel] [PATCH v3 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 Thu, Feb 18, 2021 at 05:01:26PM +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\nCould you explain the use case for both options ? Assuming an API for\napplications to set this, how would an application decide ?\n\n> Signed-off-by: Naushir Patuck \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 d969c77993eb..7f66d6995176 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> @@ -1724,13 +1740,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> @@ -1744,10 +1760,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 follwing 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\tbayerQueue_.pop();","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 E2322BD1F1\n\tfor ;\n\tSun, 21 Feb 2021 23:10:00 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id AECBB689F2;\n\tMon, 22 Feb 2021 00:10:00 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id EC8EB689B3\n\tfor ;\n\tMon, 22 Feb 2021 00:09:58 +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 83F15A4E;\n\tMon, 22 Feb 2021 00:09:58 +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=\"PgksX5L+\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1613948998;\n\tbh=UWXtUdsspxyA571o/vCALWIXWolwax3UVUS2EYvUvjI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PgksX5L+5bDPa621horw520FV8ZZG219wdXdqXLEk9OJp71C9n0ei2RienK4C8N9T\n\tfvQgrpeY8eTUiRFeRawJS2UsxLyyvivII0VHnDhaJE+SexFbYN8TVPMIHz9dwyh6jd\n\t8CZwWAS0Cjuhpyfh//YFlpPt8F/vs9UueKiKjhKg=","Date":"Mon, 22 Feb 2021 01:09:32 +0200","From":"Laurent Pinchart ","To":"Naushir Patuck ","Message-ID":"","References":"<20210218170126.2060783-1-naush@raspberrypi.com>\n\t<20210218170126.2060783-5-naush@raspberrypi.com>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210218170126.2060783-5-naush@raspberrypi.com>","Subject":"Re: [libcamera-devel] [PATCH v3 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":"","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=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15291,"web_url":"https://patchwork.libcamera.org/comment/15291/","msgid":"","date":"2021-02-22T13:51:42","subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Allow\n\teither strict or non-strict buffer matching","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent\n\n\nOn Sun, 21 Feb 2021 at 23:09, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hi Naush,\n>\n> Thank you for the patch.\n>\n> On Thu, Feb 18, 2021 at 05:01:26PM +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> Could you explain the use case for both options ? Assuming an API for\n> applications to set this, how would an application decide ?\n>\n\nAs an example, consider a sensor that generates some kind of computational\nmetadata (e.g. face/object detection) in the sensor silicon and sends it out\nvia embedded data. In such cases, the image data alone is not very helpful\nfor the application. An application would need both embedded data and image\ndata buffers to do its magic, and crucially they must be in sync. This is\nwhere\nthe StrictBufferMatching = true would be used. We are actually talking to a\nvendor about this exact use case. Of course, today we do not have the\nability to\nget to that embedded data buffer in the application, but we will in the\nfuture, so\nright now this hint is more for the IPA having a guarantee that both\nbuffers are in\nsync.\n\nFor more simpler use cases, e.g. high framerate video, we do not\nnecessarily care\nabout strict matching, as the embedded buffer will only be used for AWB/AE\nloops,\nand avoiding frame drops is more desirable. These cases would\nuse StrictBufferMatching = false.\nPerhaps a better strategy would be to set this to the default...?\n\nHope that makes sense.\n\nNaush\n\n\n\n>\n> > Signed-off-by: Naushir Patuck \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\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > index d969c77993eb..7f66d6995176 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\n> have a\n> > + * matching embedded data buffer and vice-versa. This guarantees the\n> 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\n> embedded\n> > + * data buffer. In this case, IPA will use use our local history for\n> gain and\n> > + * exposure values, occasional frame drops may cause these number to be\n> out of\n> > + * sync for a short period.\n> > + *\n> > + * \\todo Ideally, this property should be set by the application, but\n> we don't\n> > + * have any mechanism to pass generic properties into a pipeline\n> handler at\n> > + * present.\n> > + */\n> > +static const bool StrictBufferMatching = true;\n> > +\n> > namespace {\n> >\n> > bool isRaw(PixelFormat &pixFmt)\n> > @@ -1724,13 +1740,13 @@ bool\n> RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em\n> > embeddedBuffer = nullptr;\n> > while (!embeddedQueue_.empty()) {\n> > FrameBuffer *b = embeddedQueue_.front();\n> > - if (!unicam_[Unicam::Embedded].isExternal() &&\n> b->metadata().timestamp < ts) {\n> > + if (b->metadata().timestamp < ts) {\n> > embeddedQueue_.pop();\n> > unicam_[Unicam::Embedded].queueBuffer(b);\n> > embeddedRequeueCount++;\n> > LOG(RPI, Warning) << \"Dropping unmatched\n> input frame in stream \"\n> > <<\n> unicam_[Unicam::Embedded].name();\n> > - } else if (unicam_[Unicam::Embedded].isExternal()\n> || b->metadata().timestamp == ts) {\n> > + } else if (b->metadata().timestamp == ts) {\n> > /* We pop the item from the queue lower\n> down. */\n> > embeddedBuffer = b;\n> > break;\n> > @@ -1744,10 +1760,15 @@ bool\n> RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em\n> >\n> > LOG(RPI, Debug) << \"Could not find matching\n> embedded buffer\";\n> >\n> > - if (!sensorMetadata_) {\n> > + if (unicam_[Unicam::Embedded].isExternal() ||\n> > + !StrictBufferMatching || !sensorMetadata_) {\n> > /*\n> > - * If there is no sensor metadata, simply\n> return the\n> > - * first bayer frame in the queue.\n> > + * If any of the follwing is true:\n> > + * - This is an external stream buffer (so\n> cannot be dropped).\n> > + * - We do not care about strict buffer\n> matching.\n> > + * - There is no sensor metadata present.\n> > + * we can simply return the first bayer\n> frame in the queue\n> > + * without a matching embedded buffer.\n> > */\n> > LOG(RPI, Debug) << \"Returning bayer frame\n> without a match\";\n> > bayerQueue_.pop();\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 C5D89BD1F1\n\tfor ;\n\tMon, 22 Feb 2021 13:52:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 545F368A07;\n\tMon, 22 Feb 2021 14:52:01 +0100 (CET)","from mail-lj1-x233.google.com (mail-lj1-x233.google.com\n\t[IPv6:2a00:1450:4864:20::233])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6B2F268A07\n\tfor ;\n\tMon, 22 Feb 2021 14:51:59 +0100 (CET)","by mail-lj1-x233.google.com with SMTP id c17so58183706ljn.0\n\tfor ;\n\tMon, 22 Feb 2021 05:51:59 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"C0I1E24X\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=c3eht7u39xRTPWZRftTjKtq3wqAqNwrzzkBTJ1n6NBI=;\n\tb=C0I1E24Xu6IsU5p4w4UZAqfRLLRCqJ4tBE2hTLIvCZZ9roF99GjMoHnhwBnTKL3RKg\n\t0NuRIi1KhKRwI/7IJ15tFQmnrPBfXuoioOfhRWpI5YyGqv/UCj6YPkjoSrmirpRZksgW\n\t+msTRlHbCoLTFmM9m2Y6RnZRygNGDSMYdJjpEwU7Kbe/GWPaKNKi96SIJEWdz06HFEIp\n\tatscvO2qivtiwlJ17kfkarDR6TPpxMyyxSKW41QAMjfdQm9WQCMLsCL5sb928MC5HG7Y\n\trklE8N030AJg3v/OSMlFf8smCHSe5nUm/kE1tuvxNlZEGeOSYWUWr8R0CEl26+9kdsnn\n\tDPBQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=c3eht7u39xRTPWZRftTjKtq3wqAqNwrzzkBTJ1n6NBI=;\n\tb=D0oo0qKRTjE65hqo+wvuncifI6BVcEM+M91KIlFQDjuld2iAHa6ItJTJgGDmUtI+1d\n\tgNVy+G6RXLRyLGhoaBPbn4LF1hw9QJc4/s1b/4ZXYPkTV0Q9A9xAFRsmpCGy5GpCt9SH\n\tUn19Cm4NkurydXq1j4soUMqiVRUgDPuzBsTG/cF2oLD++2RpVIDdH1Vw5PM+xEov74aB\n\tG01Y8FF05vlSW+qnp9HMNiHb/b7SCm6BdNaSV/qLtZ7nbMPhSKtxVVO1tG83QB/p7BIU\n\tcVBZ2znegwQHrvp9iA8Wpsg/f/yRTbZMSnGHGvk3WqZ4gXlGCmQYsY3fwMJeMxu5oNNM\n\tro+g==","X-Gm-Message-State":"AOAM533FXTX7YUhtMc+G+buZwwe+YhOoMfJlLS0F5bYHO4Z/yAOq6Fz1\n\tPEO444+WsczuwxvJMgr5l/2GhcbXEOUyAxh58F/PlA==","X-Google-Smtp-Source":"ABdhPJyLdwnyxSQrJbZpiuv2uwXqEn7hWZdN+MCt9Ru8smEWsWsguGiFnVfiPhB4d2X71/3QBkO9iCb7RZs7pUq9YDo=","X-Received":"by 2002:a19:48c2:: with SMTP id\n\tv185mr13140199lfa.375.1614001918852; \n\tMon, 22 Feb 2021 05:51:58 -0800 (PST)","MIME-Version":"1.0","References":"<20210218170126.2060783-1-naush@raspberrypi.com>\n\t<20210218170126.2060783-5-naush@raspberrypi.com>\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Mon, 22 Feb 2021 13:51:42 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v3 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"multipart/mixed;\n\tboundary=\"===============2550919644067761272==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15314,"web_url":"https://patchwork.libcamera.org/comment/15314/","msgid":"","date":"2021-02-25T11:17:37","subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Allow\n\teither strict or non-strict buffer matching","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Another interesting case that we're looking at is a sensor with\non-board HDR. Because the sensor tonemaps the image before outputting\nit, it means that the statistics we get from the ISP are no good for\nthe AGC/AEC algorithm - they're not linear with exposure/gain. Because\nof this the sensor actually outputs some statistics of its own that we\ncan collect into the \"embedded data\" buffers in our usual way. From\nthere, the plan would be to pass them to our AGC/AEC.\n\nBut anyway, this is another case where \"strict\" matching is important.\n(Also, there will likely be changes in our pipeline handler and IPAs\nin the short-to-medium term to support this kind of use-case more\nstraightforwardly!).\n\nDavid\n\nOn Mon, 22 Feb 2021 at 13:52, Naushir Patuck wrote:\n>\n> Hi Laurent\n>\n>\n> On Sun, 21 Feb 2021 at 23:09, Laurent Pinchart wrote:\n>>\n>> Hi Naush,\n>>\n>> Thank you for the patch.\n>>\n>> On Thu, Feb 18, 2021 at 05:01:26PM +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>> Could you explain the use case for both options ? Assuming an API for\n>> applications to set this, how would an application decide ?\n>\n>\n> As an example, consider a sensor that generates some kind of computational\n> metadata (e.g. face/object detection) in the sensor silicon and sends it out\n> via embedded data. In such cases, the image data alone is not very helpful\n> for the application. An application would need both embedded data and image\n> data buffers to do its magic, and crucially they must be in sync. This is where\n> the StrictBufferMatching = true would be used. We are actually talking to a\n> vendor about this exact use case. Of course, today we do not have the ability to\n> get to that embedded data buffer in the application, but we will in the future, so\n> right now this hint is more for the IPA having a guarantee that both buffers are in\n> sync.\n>\n> For more simpler use cases, e.g. high framerate video, we do not necessarily care\n> about strict matching, as the embedded buffer will only be used for AWB/AE loops,\n> and avoiding frame drops is more desirable. These cases would use StrictBufferMatching = false.\n> Perhaps a better strategy would be to set this to the default...?\n>\n> Hope that makes sense.\n>\n> Naush\n>\n>\n>>\n>>\n>> > Signed-off-by: Naushir Patuck \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 d969c77993eb..7f66d6995176 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>> > @@ -1724,13 +1740,13 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em\n>> > embeddedBuffer = nullptr;\n>> > while (!embeddedQueue_.empty()) {\n>> > FrameBuffer *b = embeddedQueue_.front();\n>> > - if (!unicam_[Unicam::Embedded].isExternal() && b->metadata().timestamp < ts) {\n>> > + if (b->metadata().timestamp < ts) {\n>> > embeddedQueue_.pop();\n>> > unicam_[Unicam::Embedded].queueBuffer(b);\n>> > embeddedRequeueCount++;\n>> > LOG(RPI, Warning) << \"Dropping unmatched input frame in stream \"\n>> > << unicam_[Unicam::Embedded].name();\n>> > - } else if (unicam_[Unicam::Embedded].isExternal() || b->metadata().timestamp == ts) {\n>> > + } else if (b->metadata().timestamp == ts) {\n>> > /* We pop the item from the queue lower down. */\n>> > embeddedBuffer = b;\n>> > break;\n>> > @@ -1744,10 +1760,15 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em\n>> >\n>> > LOG(RPI, Debug) << \"Could not find matching embedded buffer\";\n>> >\n>> > - if (!sensorMetadata_) {\n>> > + if (unicam_[Unicam::Embedded].isExternal() ||\n>> > + !StrictBufferMatching || !sensorMetadata_) {\n>> > /*\n>> > - * If there is no sensor metadata, simply return the\n>> > - * first bayer frame in the queue.\n>> > + * If any of the follwing is true:\n>> > + * - This is an external stream buffer (so cannot be dropped).\n>> > + * - We do not care about strict buffer matching.\n>> > + * - There is no sensor metadata present.\n>> > + * we can simply return the first bayer frame in the queue\n>> > + * without a matching embedded buffer.\n>> > */\n>> > LOG(RPI, Debug) << \"Returning bayer frame without a match\";\n>> > bayerQueue_.pop();\n>>\n>> --\n>> Regards,\n>>\n>> Laurent Pinchart\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 6D869BD808\n\tfor ;\n\tThu, 25 Feb 2021 11:17:51 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B784C68A3F;\n\tThu, 25 Feb 2021 12:17:50 +0100 (CET)","from mail-oi1-x22b.google.com (mail-oi1-x22b.google.com\n\t[IPv6:2607:f8b0:4864:20::22b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 96D28602EE\n\tfor ;\n\tThu, 25 Feb 2021 12:17:49 +0100 (CET)","by mail-oi1-x22b.google.com with SMTP id d20so5685161oiw.10\n\tfor ;\n\tThu, 25 Feb 2021 03:17:49 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"HtFffZFH\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=K6EwY0dzS4i+bDwmmAngEbtCITBQebFYLs2Kf+jwFtI=;\n\tb=HtFffZFHCS7kB+IlkV4WXe6mOVzaRexnw/5avO5XX/ujYO1lMkiwRVD6hLOozY57fK\n\t/8HJRHWbOYbyWpCjYJXggQ54sKdSjyHg7h380MdW9jHogDud5wf0q78er9b8ZHB4HE94\n\tih5xhFElDzLtrYm2h4/d8S0Iv1pi7ngD3xLqjthsbMzmLD2fkBC8gDN6vNVVb4hw8nY2\n\tYHSqzVP/vhbMiLR5zWBc9tbhjpeusuZvR6TJM6tPcndCiFQEpuk4L+cV2V40j6QKxIQE\n\tmWuix428cNV58cLqAu8nUEoAU2vxJ0HB9+rAkJznLgzuoKzTdlpHupw98Jn3PeFAqE68\n\tB28g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=K6EwY0dzS4i+bDwmmAngEbtCITBQebFYLs2Kf+jwFtI=;\n\tb=GPmLqeePQ37BPP+Vw45X4JGe1/BGX5SvPA5TknoQB9KBufpOTz7jeTlm5UY0Yp/8j2\n\tmjgE6QZomDpdjAVbpsu3X30V2ETnwv4lbBcafhizSVnwMD56nmhNiTKyFK1/mXtTWrZT\n\tnNx7c3b1VwIFM5ep0PLY/dSclBUQFHKvznKF+K6eFBmcY4zsYt23LmnCd8R8VcbDEUCu\n\thdzLv4hvtCSe6sJs2+unN52R8oY4JjzprfzvWt6+Y7xDAMl5WQ8m1Uw8gwc2xJR/VsYB\n\tOguoDqfECHRhzGK/Xz6Df+RolqYJP9HbgdzuQXJ2QCpGKwRg2Z1Wv4AQgjcIanaAbYsj\n\tv9cg==","X-Gm-Message-State":"AOAM533Sr+QFiAxQ9XkLL0dU5K9DmM1ERIuaYUhBBV0YyAwx8SIJ6MIf\n\tiGgeQTD88T7ZYM8KB0ORt/iPEaoqseQonfDlWAxdlg==","X-Google-Smtp-Source":"ABdhPJxirvwgN82xqewwx/WLeYYn0FK68l7UeY09c17C6491jyFlkTIeXMI5J2DOAnBAWW96MrnrOFfWJ/Td9SQlKrs=","X-Received":"by 2002:aca:c315:: with SMTP id\n\tt21mr1465804oif.107.1614251868191; \n\tThu, 25 Feb 2021 03:17:48 -0800 (PST)","MIME-Version":"1.0","References":"<20210218170126.2060783-1-naush@raspberrypi.com>\n\t<20210218170126.2060783-5-naush@raspberrypi.com>\n\t\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Thu, 25 Feb 2021 11:17:37 +0000","Message-ID":"","To":"Naushir Patuck ","Subject":"Re: [libcamera-devel] [PATCH v3 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15465,"web_url":"https://patchwork.libcamera.org/comment/15465/","msgid":"","date":"2021-03-04T02:12:17","subject":"Re: [libcamera-devel] [PATCH v3 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":"Hello,\n\nOn Thu, Feb 25, 2021 at 11:17:37AM +0000, David Plowman wrote:\n> Another interesting case that we're looking at is a sensor with\n> on-board HDR. Because the sensor tonemaps the image before outputting\n> it, it means that the statistics we get from the ISP are no good for\n> the AGC/AEC algorithm - they're not linear with exposure/gain. Because\n> of this the sensor actually outputs some statistics of its own that we\n> can collect into the \"embedded data\" buffers in our usual way. From\n> there, the plan would be to pass them to our AGC/AEC.\n> \n> But anyway, this is another case where \"strict\" matching is important.\n> (Also, there will likely be changes in our pipeline handler and IPAs\n> in the short-to-medium term to support this kind of use-case more\n> straightforwardly!).\n> \n> On Mon, 22 Feb 2021 at 13:52, Naushir Patuck wrote:\n> > On Sun, 21 Feb 2021 at 23:09, Laurent Pinchart wrote:\n> >> On Thu, Feb 18, 2021 at 05:01:26PM +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> >> Could you explain the use case for both options ? Assuming an API for\n> >> applications to set this, how would an application decide ?\n> >\n> > As an example, consider a sensor that generates some kind of computational\n> > metadata (e.g. face/object detection) in the sensor silicon and sends it out\n> > via embedded data. In such cases, the image data alone is not very helpful\n> > for the application. An application would need both embedded data and image\n> > data buffers to do its magic, and crucially they must be in sync. This is where\n> > the StrictBufferMatching = true would be used. We are actually talking to a\n> > vendor about this exact use case. Of course, today we do not have the ability to\n> > get to that embedded data buffer in the application, but we will in the future, so\n> > right now this hint is more for the IPA having a guarantee that both buffers are in\n> > sync.\n> >\n> > For more simpler use cases, e.g. high framerate video, we do not necessarily care\n> > about strict matching, as the embedded buffer will only be used for AWB/AE loops,\n> > and avoiding frame drops is more desirable. These cases would use StrictBufferMatching = false.\n> > Perhaps a better strategy would be to set this to the default...?\n> >\n> > Hope that makes sense.\n\nSo I understand why strict matching is important, and that's the default\nin this patch. Non-strict matching relaxes the requirements, but I'm not\nsure what impact it will have on algorithms.\n\nFurthermore, to make this really useful, we would need a way to\nconfigure the policy at runtime, and I think it may be a hard choice for\napplications. I wonder if there could be a way to handle the buffer\nmatching policy automagically ? The code hear deals with the case were\nno matching buffer is available, which I assume would be caused by a\nframe drop (either on the image stream or the embedded data stream).\nWould it be possible to detect such situations and handle them in an\nautomatic way ? I'm not sure what to propose exactly, but going in the\ndirection of exposing to applications controls that tune heuristics in\nill-defined ways worries me. We could easily end up with controls that\napplications would have no meaningful way to set. If you ask me if I\nneed to avoid frame drops or avoid bad images, I'll tell you I need both\n:-) I could possibly make a decision if I knew the exact risk of frame\ndrop, and exactly how \"bad\" images could be, but I don't think that's\nsomething we'll be able to express in an API. I'd really like to avoid\ngoing in this direction.\n\n> >> > Signed-off-by: Naushir Patuck \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 d969c77993eb..7f66d6995176 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> >> > @@ -1724,13 +1740,13 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em\n> >> > embeddedBuffer = nullptr;\n> >> > while (!embeddedQueue_.empty()) {\n> >> > FrameBuffer *b = embeddedQueue_.front();\n> >> > - if (!unicam_[Unicam::Embedded].isExternal() && b->metadata().timestamp < ts) {\n> >> > + if (b->metadata().timestamp < ts) {\n> >> > embeddedQueue_.pop();\n> >> > unicam_[Unicam::Embedded].queueBuffer(b);\n> >> > embeddedRequeueCount++;\n> >> > LOG(RPI, Warning) << \"Dropping unmatched input frame in stream \"\n> >> > << unicam_[Unicam::Embedded].name();\n> >> > - } else if (unicam_[Unicam::Embedded].isExternal() || b->metadata().timestamp == ts) {\n> >> > + } else if (b->metadata().timestamp == ts) {\n> >> > /* We pop the item from the queue lower down. */\n> >> > embeddedBuffer = b;\n> >> > break;\n> >> > @@ -1744,10 +1760,15 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em\n> >> >\n> >> > LOG(RPI, Debug) << \"Could not find matching embedded buffer\";\n> >> >\n> >> > - if (!sensorMetadata_) {\n> >> > + if (unicam_[Unicam::Embedded].isExternal() ||\n> >> > + !StrictBufferMatching || !sensorMetadata_) {\n> >> > /*\n> >> > - * If there is no sensor metadata, simply return the\n> >> > - * first bayer frame in the queue.\n> >> > + * If any of the follwing is true:\n> >> > + * - This is an external stream buffer (so cannot be dropped).\n> >> > + * - We do not care about strict buffer matching.\n> >> > + * - There is no sensor metadata present.\n> >> > + * we can simply return the first bayer frame in the queue\n> >> > + * without a matching embedded buffer.\n> >> > */\n> >> > LOG(RPI, Debug) << \"Returning bayer frame without a match\";\n> >> > bayerQueue_.pop();","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 4AB8DBD1F1\n\tfor ;\n\tThu, 4 Mar 2021 02:12:48 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id B9D6C68A92;\n\tThu, 4 Mar 2021 03:12:47 +0100 (CET)","from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E7FA960106\n\tfor ;\n\tThu, 4 Mar 2021 03:12:46 +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 63CD127A;\n\tThu, 4 Mar 2021 03:12:46 +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=\"t/JZjjlg\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1614823966;\n\tbh=9ZYsBzKs94iGsZqMqybcwyVAVN63sZuyxz0dKWXSmME=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=t/JZjjlggb5qnBK25JHFlLyIsJP1brRUzDnuyAv1rLkY/jladCgeCaXL+i2fHu/oP\n\tuVyVzLlgahy7l9X1J0AAPuG9Y3AcY0zwp3yh3MsSlvcPiG3bpT5gGFcOLS4c6lsUX9\n\tk6ZHS3H/QKcUwpuiOBkxxx6xQfgpmsPx79QNSK7Q=","Date":"Thu, 4 Mar 2021 04:12:17 +0200","From":"Laurent Pinchart ","To":"David Plowman ","Message-ID":"","References":"<20210218170126.2060783-1-naush@raspberrypi.com>\n\t<20210218170126.2060783-5-naush@raspberrypi.com>\n\t\n\t\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v3 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15471,"web_url":"https://patchwork.libcamera.org/comment/15471/","msgid":"","date":"2021-03-04T08:48:50","subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Allow\n\teither strict or non-strict buffer matching","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi Laurent,\n\nOn Thu, 4 Mar 2021 at 02:12, Laurent Pinchart <\nlaurent.pinchart@ideasonboard.com> wrote:\n\n> Hello,\n>\n> On Thu, Feb 25, 2021 at 11:17:37AM +0000, David Plowman wrote:\n> > Another interesting case that we're looking at is a sensor with\n> > on-board HDR. Because the sensor tonemaps the image before outputting\n> > it, it means that the statistics we get from the ISP are no good for\n> > the AGC/AEC algorithm - they're not linear with exposure/gain. Because\n> > of this the sensor actually outputs some statistics of its own that we\n> > can collect into the \"embedded data\" buffers in our usual way. From\n> > there, the plan would be to pass them to our AGC/AEC.\n> >\n> > But anyway, this is another case where \"strict\" matching is important.\n> > (Also, there will likely be changes in our pipeline handler and IPAs\n> > in the short-to-medium term to support this kind of use-case more\n> > straightforwardly!).\n> >\n> > On Mon, 22 Feb 2021 at 13:52, Naushir Patuck \n> wrote:\n> > > On Sun, 21 Feb 2021 at 23:09, Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com> wrote:\n> > >> On Thu, Feb 18, 2021 at 05:01:26PM +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\n> do\n> > >> > not have a matching embedded data buffer and vice-versa. This\n> guarantees\n> > >> > the IPA will always have the correct frame exposure and gain values\n> 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> > >> Could you explain the use case for both options ? Assuming an API for\n> > >> applications to set this, how would an application decide ?\n> > >\n> > > As an example, consider a sensor that generates some kind of\n> computational\n> > > metadata (e.g. face/object detection) in the sensor silicon and sends\n> it out\n> > > via embedded data. In such cases, the image data alone is not very\n> helpful\n> > > for the application. An application would need both embedded data and\n> image\n> > > data buffers to do its magic, and crucially they must be in sync.\n> This is where\n> > > the StrictBufferMatching = true would be used. We are actually\n> talking to a\n> > > vendor about this exact use case. Of course, today we do not have the\n> ability to\n> > > get to that embedded data buffer in the application, but we will in\n> the future, so\n> > > right now this hint is more for the IPA having a guarantee that both\n> buffers are in\n> > > sync.\n> > >\n> > > For more simpler use cases, e.g. high framerate video, we do not\n> necessarily care\n> > > about strict matching, as the embedded buffer will only be used for\n> AWB/AE loops,\n> > > and avoiding frame drops is more desirable. These cases would use\n> StrictBufferMatching = false.\n> > > Perhaps a better strategy would be to set this to the default...?\n> > >\n> > > Hope that makes sense.\n>\n> So I understand why strict matching is important, and that's the default\n> in this patch. Non-strict matching relaxes the requirements, but I'm not\n> sure what impact it will have on algorithms.\n>\n\nHopefully non-strict buffer matching will not have any impact on the\nalgorithms :-)\nI suppose it could possibly cause AE to go wrong in extreme cases where\nfor whatever reason, the locally stored values were incorrect.\n\n\n> Furthermore, to make this really useful, we would need a way to\n> configure the policy at runtime, and I think it may be a hard choice for\n> applications. I wonder if there could be a way to handle the buffer\n> matching policy automagically ? The code hear deals with the case were\n> no matching buffer is available, which I assume would be caused by a\n> frame drop (either on the image stream or the embedded data stream).\n> Would it be possible to detect such situations and handle them in an\n> automatic way ?\n\n\nThe pipeline handler is able to detect a buffer that cannot be matched,\nbut after that, I don't see a way for it to know how to handle this in an\nautomatic way. It does not know the context of the buffers or application,\ne.g, if there is some computer vision embedded data that goes with the\nimage frame so if we don't have a match we should drop the frame.\n\n\n> I'm not sure what to propose exactly, but going in the\n> direction of exposing to applications controls that tune heuristics in\n> ill-defined ways worries me. We could easily end up with controls that\n> applications would have no meaningful way to set. If you ask me if I\n> need to avoid frame drops or avoid bad images, I'll tell you I need both\n> :-) I could possibly make a decision if I knew the exact risk of frame\n> drop, and exactly how \"bad\" images could be, but I don't think that's\n> something we'll be able to express in an API. I'd really like to avoid\n> going in this direction.\n>\n\nYes, I do see your concerns here. We do not want to put more onus on\nthe application side to manage pipelines, and in possibly a vendor specific\nway. However, I do see that more advanced users, or unique apps that\nhave a very specific purpose may want this sort of control available.\n\nSome time ago, we talked about allowing applications to provide \"hints\"\nto the pipeline handlers on possible operating behaviors. I was thinking\nthat StrictBufferMatching could be one of those hints. Other possible\nhints may be HDR, ZSL, high framerate, low power operating modes\nas a few examples from the top of my head.\n\nWe still need to be careful, as hints should not be enforceable, but act\nonly as a suggestion (hence \"hints\" :-)) that the pipeline handler may or\nmay not handle.\n\nI'm not sure that the right approach here is, or if there even is one...?\n\nRegards,\nNaush\n\n> >> > Signed-off-by: Naushir Patuck \n> > >> > ---\n> > >> > .../pipeline/raspberrypi/raspberrypi.cpp | 31\n> ++++++++++++++++---\n> > >> > 1 file changed, 26 insertions(+), 5 deletions(-)\n> > >> >\n> > >> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp\n> > >> > index d969c77993eb..7f66d6995176 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\n> not have a\n> > >> > + * matching embedded data buffer and vice-versa. This guarantees\n> 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\n> matching embedded\n> > >> > + * data buffer. In this case, IPA will use use our local history\n> for gain and\n> > >> > + * exposure values, occasional frame drops may cause these number\n> to be out of\n> > >> > + * sync for a short period.\n> > >> > + *\n> > >> > + * \\todo Ideally, this property should be set by the application,\n> but we don't\n> > >> > + * have any mechanism to pass generic properties into a pipeline\n> handler at\n> > >> > + * present.\n> > >> > + */\n> > >> > +static const bool StrictBufferMatching = true;\n> > >> > +\n> > >> > namespace {\n> > >> >\n> > >> > bool isRaw(PixelFormat &pixFmt)\n> > >> > @@ -1724,13 +1740,13 @@ bool\n> RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em\n> > >> > embeddedBuffer = nullptr;\n> > >> > while (!embeddedQueue_.empty()) {\n> > >> > FrameBuffer *b = embeddedQueue_.front();\n> > >> > - if (!unicam_[Unicam::Embedded].isExternal()\n> && b->metadata().timestamp < ts) {\n> > >> > + if (b->metadata().timestamp < ts) {\n> > >> > embeddedQueue_.pop();\n> > >> >\n> unicam_[Unicam::Embedded].queueBuffer(b);\n> > >> > embeddedRequeueCount++;\n> > >> > LOG(RPI, Warning) << \"Dropping\n> unmatched input frame in stream \"\n> > >> > <<\n> unicam_[Unicam::Embedded].name();\n> > >> > - } else if\n> (unicam_[Unicam::Embedded].isExternal() || b->metadata().timestamp == ts) {\n> > >> > + } else if (b->metadata().timestamp == ts) {\n> > >> > /* We pop the item from the queue\n> lower down. */\n> > >> > embeddedBuffer = b;\n> > >> > break;\n> > >> > @@ -1744,10 +1760,15 @@ bool\n> RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em\n> > >> >\n> > >> > LOG(RPI, Debug) << \"Could not find matching\n> embedded buffer\";\n> > >> >\n> > >> > - if (!sensorMetadata_) {\n> > >> > + if (unicam_[Unicam::Embedded].isExternal() ||\n> > >> > + !StrictBufferMatching ||\n> !sensorMetadata_) {\n> > >> > /*\n> > >> > - * If there is no sensor metadata,\n> simply return the\n> > >> > - * first bayer frame in the queue.\n> > >> > + * If any of the follwing is true:\n> > >> > + * - This is an external stream\n> buffer (so cannot be dropped).\n> > >> > + * - We do not care about strict\n> buffer matching.\n> > >> > + * - There is no sensor metadata\n> present.\n> > >> > + * we can simply return the first\n> bayer frame in the queue\n> > >> > + * without a matching embedded buffer.\n> > >> > */\n> > >> > LOG(RPI, Debug) << \"Returning bayer\n> frame without a match\";\n> > >> > bayerQueue_.pop();\n>\n> --\n> Regards,\n>\n> Laurent Pinchart\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 9D424BD80C\n\tfor ;\n\tThu, 4 Mar 2021 08:49:08 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 5B73A68A98;\n\tThu, 4 Mar 2021 09:49:08 +0100 (CET)","from mail-lf1-x12a.google.com (mail-lf1-x12a.google.com\n\t[IPv6:2a00:1450:4864:20::12a])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1CE04602EC\n\tfor ;\n\tThu, 4 Mar 2021 09:49:07 +0100 (CET)","by mail-lf1-x12a.google.com with SMTP id u4so41890223lfs.0\n\tfor ;\n\tThu, 04 Mar 2021 00:49:07 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"YdY1a+hr\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=nkV/uRy8etWeM4wMzLyWXmerm/HwZI7tImJLyR/JxXU=;\n\tb=YdY1a+hrjxJvOWeurrnY+LZT99UwRGJES8zLU049BEAacDnHRRKIsFlhzW7qJ/qkuj\n\t48oqwZ4YZjzuk6B+Hk2/YecbpQdNcZa52boPOtRuPEZJfgrzG/IIcNvkJub3qxqrIo8a\n\tAA/OzNPT6pr+wcXn/HLC+s1k8ZRPag2ya8ODO3tIBOqFVzo3mzLZK6brA+xk8PgmEGIW\n\twG+P4nxywHjZMsZoP0Oy678DxA3//7WTPyCpo+ezX6fT5T9E8RcYgXKYTKa7d4D4prD4\n\tYFIlL7VinP9GfMIUTMQmtl8EvPDFqH4yM3jEyCy4PphuLJyzOZTUMJqNnGqhjtYE6wj+\n\t/r0g==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=nkV/uRy8etWeM4wMzLyWXmerm/HwZI7tImJLyR/JxXU=;\n\tb=q11YITkHOHtdTzd+kTwAOmcKdu+pHNiw+kYAcilhsRku4TFWmBUNWDiDlMYcTpIO5A\n\t9Hjj1773vw/L6fn8LYdkAeUAbDVIovIoWdsoPSBJABBtHIzeiD2schJaO4+xoo5QJn7t\n\trI8THxv2N/G7ZN+8NH0u87utrYH8e6LyOolHb4kpLBUyMFJajkDtAuRyLvxclRUzThOF\n\tK1yXd7uhowKMwon29XjOfKJ6EN2TJSbuSuI2JeITWTHW/0VlHSQ4yLdi0c1ZNXLONuAt\n\tBIlJK9IudJsrm3BeeW5wTxQNFKN32Kjtwmy7LFKLEYjGqxNF3GRFpJmYAkhV+qGsohS6\n\t83Kg==","X-Gm-Message-State":"AOAM533M5S0tpOGc5Ygrpruz3l7BldxJXliJ9DaeVDpyoUCTBSq+LvZ0\n\tqbxH7zXOn4jhYnBF+GUCCVbIU0yE431N94WzVQZOOA==","X-Google-Smtp-Source":"ABdhPJy/NptKGHZMgEm0LCFNWvrjtPiQ4XxS38FlPCE6IUT2exvHcyB5i5ePba5gB+GCg2TFhQjIqxWIZ3txTo2wwwA=","X-Received":"by 2002:a05:6512:210b:: with SMTP id\n\tq11mr1687926lfr.133.1614847746484; \n\tThu, 04 Mar 2021 00:49:06 -0800 (PST)","MIME-Version":"1.0","References":"<20210218170126.2060783-1-naush@raspberrypi.com>\n\t<20210218170126.2060783-5-naush@raspberrypi.com>\n\t\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Thu, 4 Mar 2021 08:48:50 +0000","Message-ID":"","To":"Laurent Pinchart ","Subject":"Re: [libcamera-devel] [PATCH v3 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"multipart/mixed;\n\tboundary=\"===============7868171849924928824==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15483,"web_url":"https://patchwork.libcamera.org/comment/15483/","msgid":"","date":"2021-03-04T17:19:14","subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Allow\n\teither strict or non-strict buffer matching","submitter":{"id":42,"url":"https://patchwork.libcamera.org/api/people/42/","name":"David Plowman","email":"david.plowman@raspberrypi.com"},"content":"Hi Naush, Laurent\n\nOn Thu, 4 Mar 2021 at 08:49, Naushir Patuck wrote:\n>\n> Hi Laurent,\n>\n> On Thu, 4 Mar 2021 at 02:12, Laurent Pinchart wrote:\n>>\n>> Hello,\n>>\n>> On Thu, Feb 25, 2021 at 11:17:37AM +0000, David Plowman wrote:\n>> > Another interesting case that we're looking at is a sensor with\n>> > on-board HDR. Because the sensor tonemaps the image before outputting\n>> > it, it means that the statistics we get from the ISP are no good for\n>> > the AGC/AEC algorithm - they're not linear with exposure/gain. Because\n>> > of this the sensor actually outputs some statistics of its own that we\n>> > can collect into the \"embedded data\" buffers in our usual way. From\n>> > there, the plan would be to pass them to our AGC/AEC.\n>> >\n>> > But anyway, this is another case where \"strict\" matching is important.\n>> > (Also, there will likely be changes in our pipeline handler and IPAs\n>> > in the short-to-medium term to support this kind of use-case more\n>> > straightforwardly!).\n>> >\n>> > On Mon, 22 Feb 2021 at 13:52, Naushir Patuck wrote:\n>> > > On Sun, 21 Feb 2021 at 23:09, Laurent Pinchart wrote:\n>> > >> On Thu, Feb 18, 2021 at 05:01:26PM +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>> > >> Could you explain the use case for both options ? Assuming an API for\n>> > >> applications to set this, how would an application decide ?\n>> > >\n>> > > As an example, consider a sensor that generates some kind of computational\n>> > > metadata (e.g. face/object detection) in the sensor silicon and sends it out\n>> > > via embedded data. In such cases, the image data alone is not very helpful\n>> > > for the application. An application would need both embedded data and image\n>> > > data buffers to do its magic, and crucially they must be in sync. This is where\n>> > > the StrictBufferMatching = true would be used. We are actually talking to a\n>> > > vendor about this exact use case. Of course, today we do not have the ability to\n>> > > get to that embedded data buffer in the application, but we will in the future, so\n>> > > right now this hint is more for the IPA having a guarantee that both buffers are in\n>> > > sync.\n>> > >\n>> > > For more simpler use cases, e.g. high framerate video, we do not necessarily care\n>> > > about strict matching, as the embedded buffer will only be used for AWB/AE loops,\n>> > > and avoiding frame drops is more desirable. These cases would use StrictBufferMatching = false.\n>> > > Perhaps a better strategy would be to set this to the default...?\n>> > >\n>> > > Hope that makes sense.\n>>\n>> So I understand why strict matching is important, and that's the default\n>> in this patch. Non-strict matching relaxes the requirements, but I'm not\n>> sure what impact it will have on algorithms.\n>\n>\n> Hopefully non-strict buffer matching will not have any impact on the algorithms :-)\n> I suppose it could possibly cause AE to go wrong in extreme cases where\n> for whatever reason, the locally stored values were incorrect.\n>\n>>\n>> Furthermore, to make this really useful, we would need a way to\n>> configure the policy at runtime, and I think it may be a hard choice for\n>> applications. I wonder if there could be a way to handle the buffer\n>> matching policy automagically ? The code hear deals with the case were\n>> no matching buffer is available, which I assume would be caused by a\n>> frame drop (either on the image stream or the embedded data stream).\n>> Would it be possible to detect such situations and handle them in an\n>> automatic way ?\n>\n>\n> The pipeline handler is able to detect a buffer that cannot be matched,\n> but after that, I don't see a way for it to know how to handle this in an\n> automatic way. It does not know the context of the buffers or application,\n> e.g, if there is some computer vision embedded data that goes with the\n> image frame so if we don't have a match we should drop the frame.\n>\n>>\n>> I'm not sure what to propose exactly, but going in the\n>> direction of exposing to applications controls that tune heuristics in\n>> ill-defined ways worries me. We could easily end up with controls that\n>> applications would have no meaningful way to set. If you ask me if I\n>> need to avoid frame drops or avoid bad images, I'll tell you I need both\n>> :-) I could possibly make a decision if I knew the exact risk of frame\n>> drop, and exactly how \"bad\" images could be, but I don't think that's\n>> something we'll be able to express in an API. I'd really like to avoid\n>> going in this direction.\n>\n>\n> Yes, I do see your concerns here. We do not want to put more onus on\n> the application side to manage pipelines, and in possibly a vendor specific\n> way. However, I do see that more advanced users, or unique apps that\n> have a very specific purpose may want this sort of control available.\n>\n> Some time ago, we talked about allowing applications to provide \"hints\"\n> to the pipeline handlers on possible operating behaviors. I was thinking\n> that StrictBufferMatching could be one of those hints. Other possible\n> hints may be HDR, ZSL, high framerate, low power operating modes\n> as a few examples from the top of my head.\n>\n> We still need to be careful, as hints should not be enforceable, but act\n> only as a suggestion (hence \"hints\" :-)) that the pipeline handler may or\n> may not handle.\n>\n> I'm not sure that the right approach here is, or if there even is one...?\n\nI wonder if CamHelpers would help here? Particularly in view of our\nplans going forward to generalise the parsing for more types of\nmetadata, the CamHelpers will have to know what the metadata is and\ncould take a view on the risks of dropping frames or not.\n\nDavid\n\n>\n> Regards,\n> Naush\n>\n>> > >> > Signed-off-by: Naushir Patuck \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 d969c77993eb..7f66d6995176 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>> > >> > @@ -1724,13 +1740,13 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em\n>> > >> > embeddedBuffer = nullptr;\n>> > >> > while (!embeddedQueue_.empty()) {\n>> > >> > FrameBuffer *b = embeddedQueue_.front();\n>> > >> > - if (!unicam_[Unicam::Embedded].isExternal() && b->metadata().timestamp < ts) {\n>> > >> > + if (b->metadata().timestamp < ts) {\n>> > >> > embeddedQueue_.pop();\n>> > >> > unicam_[Unicam::Embedded].queueBuffer(b);\n>> > >> > embeddedRequeueCount++;\n>> > >> > LOG(RPI, Warning) << \"Dropping unmatched input frame in stream \"\n>> > >> > << unicam_[Unicam::Embedded].name();\n>> > >> > - } else if (unicam_[Unicam::Embedded].isExternal() || b->metadata().timestamp == ts) {\n>> > >> > + } else if (b->metadata().timestamp == ts) {\n>> > >> > /* We pop the item from the queue lower down. */\n>> > >> > embeddedBuffer = b;\n>> > >> > break;\n>> > >> > @@ -1744,10 +1760,15 @@ bool RPiCameraData::findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&em\n>> > >> >\n>> > >> > LOG(RPI, Debug) << \"Could not find matching embedded buffer\";\n>> > >> >\n>> > >> > - if (!sensorMetadata_) {\n>> > >> > + if (unicam_[Unicam::Embedded].isExternal() ||\n>> > >> > + !StrictBufferMatching || !sensorMetadata_) {\n>> > >> > /*\n>> > >> > - * If there is no sensor metadata, simply return the\n>> > >> > - * first bayer frame in the queue.\n>> > >> > + * If any of the follwing is true:\n>> > >> > + * - This is an external stream buffer (so cannot be dropped).\n>> > >> > + * - We do not care about strict buffer matching.\n>> > >> > + * - There is no sensor metadata present.\n>> > >> > + * we can simply return the first bayer frame in the queue\n>> > >> > + * without a matching embedded buffer.\n>> > >> > */\n>> > >> > LOG(RPI, Debug) << \"Returning bayer frame without a match\";\n>> > >> > bayerQueue_.pop();\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 ADDDCBD80C\n\tfor ;\n\tThu, 4 Mar 2021 17:19:29 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 0D09368A99;\n\tThu, 4 Mar 2021 18:19:29 +0100 (CET)","from mail-ot1-x330.google.com (mail-ot1-x330.google.com\n\t[IPv6:2607:f8b0:4864:20::330])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id DB09B602ED\n\tfor ;\n\tThu, 4 Mar 2021 18:19:27 +0100 (CET)","by mail-ot1-x330.google.com with SMTP id j8so2539799otc.0\n\tfor ;\n\tThu, 04 Mar 2021 09:19:27 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"q5ABsX8p\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=pdOreQJ7t4x6vmgk6ftVj+MRgDDkWKV33SnjJN08Qh0=;\n\tb=q5ABsX8ptee6D12cjeTdfXxiOmWoVFbAX1wSJjkAt91VbpnQaUnX41VYmILgZgtMlI\n\t2qKjeu3SeRYUQfArzpDFl5vYpypqt/iQlqiXkPsl8aSmsaPGAY9HtMuoEVFGER702meP\n\tfDT8xP8A3jERoKLvBNp7XrTQLLj2wN4Pvh2uhxhMvu2lzueEG+3HPD/UihjYfv8RuWjv\n\tCiOJECc0O6QNiUYJktjsGupmfOavR05te4PUcf8228o1Bp2Lql9G/tG4A27W9Lg2Mtw2\n\t0bgIojp8zw9sE40uavP4mBaqKij4bhmEsEP2/lKxUKRlyn6oGiD/1KrwB24vzLlp2kou\n\t5raA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=pdOreQJ7t4x6vmgk6ftVj+MRgDDkWKV33SnjJN08Qh0=;\n\tb=Rucn1htT7oWSvDbBnIJWvpWDwe16+9ql+aQkrO5LP05lkTzzROmkXh+gxoOmqtehhi\n\tL2F89/jr5tKWTZ2KWlfA5U/yS6yYVmpkmHdSXZ5dDVopZbI9zfvxQEKc4fyBY8NgAgvZ\n\t+q8wV5sFbs655qgN1MYLOookmbCa9kxllWo6bkJWsdzgmjWH2KDjKkh7WDq37Hf7AYeo\n\t8A7mTluMy1liz0lk/mgdSmrVwpPwU3icSPE9vKntNylltIqysB5aw9+2gIWA8drdESBo\n\tBwfIebjgzemX4cWeCY0p0kleiXjyT1w2gVBF5fI0gYGgGRxfbeLDhm4LECzpNgeS2Abh\n\tjhRw==","X-Gm-Message-State":"AOAM531UW0zs21PRyaCfomU6NXacpU5Jww8+ITglbdWAUjHsYPROuZBY\n\tcBzn4zQGzRU6GNmiOYi3vJmK5TKQ1EY3J08MOz35RA==","X-Google-Smtp-Source":"ABdhPJw74TzHkYKQSm1boaYFYKbROth6J5LRuHbUamtG2aUEg4m4GOAN/KdmmIYk8ad85aRh56qR/irH5AQ5uuS+A0U=","X-Received":"by 2002:a05:6830:1502:: with SMTP id\n\tk2mr4350457otp.166.1614878366470; \n\tThu, 04 Mar 2021 09:19:26 -0800 (PST)","MIME-Version":"1.0","References":"<20210218170126.2060783-1-naush@raspberrypi.com>\n\t<20210218170126.2060783-5-naush@raspberrypi.com>\n\t\n\t\n\t\n\t\n\t","In-Reply-To":"","From":"David Plowman ","Date":"Thu, 4 Mar 2021 17:19:14 +0000","Message-ID":"","To":"Naushir Patuck ","Subject":"Re: [libcamera-devel] [PATCH v3 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"text/plain; charset=\"us-ascii\"","Content-Transfer-Encoding":"7bit","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":15486,"web_url":"https://patchwork.libcamera.org/comment/15486/","msgid":"","date":"2021-03-05T09:02:55","subject":"Re: [libcamera-devel] [PATCH v3 4/4] pipeline: raspberrypi: Allow\n\teither strict or non-strict buffer matching","submitter":{"id":34,"url":"https://patchwork.libcamera.org/api/people/34/","name":"Naushir Patuck","email":"naush@raspberrypi.com"},"content":"Hi David,\n\nOn Thu, 4 Mar 2021 at 17:19, David Plowman \nwrote:\n\n> Hi Naush, Laurent\n>\n> On Thu, 4 Mar 2021 at 08:49, Naushir Patuck wrote:\n> >\n> > Hi Laurent,\n> >\n> > On Thu, 4 Mar 2021 at 02:12, Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com> wrote:\n> >>\n> >> Hello,\n> >>\n> >> On Thu, Feb 25, 2021 at 11:17:37AM +0000, David Plowman wrote:\n> >> > Another interesting case that we're looking at is a sensor with\n> >> > on-board HDR. Because the sensor tonemaps the image before outputting\n> >> > it, it means that the statistics we get from the ISP are no good for\n> >> > the AGC/AEC algorithm - they're not linear with exposure/gain. Because\n> >> > of this the sensor actually outputs some statistics of its own that we\n> >> > can collect into the \"embedded data\" buffers in our usual way. From\n> >> > there, the plan would be to pass them to our AGC/AEC.\n> >> >\n> >> > But anyway, this is another case where \"strict\" matching is important.\n> >> > (Also, there will likely be changes in our pipeline handler and IPAs\n> >> > in the short-to-medium term to support this kind of use-case more\n> >> > straightforwardly!).\n> >> >\n> >> > On Mon, 22 Feb 2021 at 13:52, Naushir Patuck \n> wrote:\n> >> > > On Sun, 21 Feb 2021 at 23:09, Laurent Pinchart <\n> laurent.pinchart@ideasonboard.com> wrote:\n> >> > >> On Thu, Feb 18, 2021 at 05:01:26PM +0000, Naushir Patuck wrote:\n> >> > >> > A new flag, StrictBufferMatching, is used to control the\n> behavior of\n> >> > >> > the embedded and bayer buffer matching routine.\n> >> > >> >\n> >> > >> > If set to true (default), we reject and drop all bayer frames\n> that do\n> >> > >> > not have a matching embedded data buffer and vice-versa. This\n> guarantees\n> >> > >> > the IPA will always have the correct frame exposure and gain\n> 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\n> 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\n> them.\n> >> > >>\n> >> > >> Could you explain the use case for both options ? Assuming an API\n> for\n> >> > >> applications to set this, how would an application decide ?\n> >> > >\n> >> > > As an example, consider a sensor that generates some kind of\n> computational\n> >> > > metadata (e.g. face/object detection) in the sensor silicon and\n> sends it out\n> >> > > via embedded data. In such cases, the image data alone is not very\n> helpful\n> >> > > for the application. An application would need both embedded data\n> and image\n> >> > > data buffers to do its magic, and crucially they must be in sync.\n> This is where\n> >> > > the StrictBufferMatching = true would be used. We are actually\n> talking to a\n> >> > > vendor about this exact use case. Of course, today we do not have\n> the ability to\n> >> > > get to that embedded data buffer in the application, but we will in\n> the future, so\n> >> > > right now this hint is more for the IPA having a guarantee that\n> both buffers are in\n> >> > > sync.\n> >> > >\n> >> > > For more simpler use cases, e.g. high framerate video, we do not\n> necessarily care\n> >> > > about strict matching, as the embedded buffer will only be used for\n> AWB/AE loops,\n> >> > > and avoiding frame drops is more desirable. These cases would use\n> StrictBufferMatching = false.\n> >> > > Perhaps a better strategy would be to set this to the default...?\n> >> > >\n> >> > > Hope that makes sense.\n> >>\n> >> So I understand why strict matching is important, and that's the default\n> >> in this patch. Non-strict matching relaxes the requirements, but I'm not\n> >> sure what impact it will have on algorithms.\n> >\n> >\n> > Hopefully non-strict buffer matching will not have any impact on the\n> algorithms :-)\n> > I suppose it could possibly cause AE to go wrong in extreme cases where\n> > for whatever reason, the locally stored values were incorrect.\n> >\n> >>\n> >> Furthermore, to make this really useful, we would need a way to\n> >> configure the policy at runtime, and I think it may be a hard choice for\n> >> applications. I wonder if there could be a way to handle the buffer\n> >> matching policy automagically ? The code hear deals with the case were\n> >> no matching buffer is available, which I assume would be caused by a\n> >> frame drop (either on the image stream or the embedded data stream).\n> >> Would it be possible to detect such situations and handle them in an\n> >> automatic way ?\n> >\n> >\n> > The pipeline handler is able to detect a buffer that cannot be matched,\n> > but after that, I don't see a way for it to know how to handle this in an\n> > automatic way. It does not know the context of the buffers or\n> application,\n> > e.g, if there is some computer vision embedded data that goes with the\n> > image frame so if we don't have a match we should drop the frame.\n> >\n> >>\n> >> I'm not sure what to propose exactly, but going in the\n> >> direction of exposing to applications controls that tune heuristics in\n> >> ill-defined ways worries me. We could easily end up with controls that\n> >> applications would have no meaningful way to set. If you ask me if I\n> >> need to avoid frame drops or avoid bad images, I'll tell you I need both\n> >> :-) I could possibly make a decision if I knew the exact risk of frame\n> >> drop, and exactly how \"bad\" images could be, but I don't think that's\n> >> something we'll be able to express in an API. I'd really like to avoid\n> >> going in this direction.\n> >\n> >\n> > Yes, I do see your concerns here. We do not want to put more onus on\n> > the application side to manage pipelines, and in possibly a vendor\n> specific\n> > way. However, I do see that more advanced users, or unique apps that\n> > have a very specific purpose may want this sort of control available.\n> >\n> > Some time ago, we talked about allowing applications to provide \"hints\"\n> > to the pipeline handlers on possible operating behaviors. I was\n> thinking\n> > that StrictBufferMatching could be one of those hints. Other possible\n> > hints may be HDR, ZSL, high framerate, low power operating modes\n> > as a few examples from the top of my head.\n> >\n> > We still need to be careful, as hints should not be enforceable, but act\n> > only as a suggestion (hence \"hints\" :-)) that the pipeline handler may or\n> > may not handle.\n> >\n> > I'm not sure that the right approach here is, or if there even is one...?\n>\n> I wonder if CamHelpers would help here? Particularly in view of our\n> plans going forward to generalise the parsing for more types of\n> metadata, the CamHelpers will have to know what the metadata is and\n> could take a view on the risks of dropping frames or not.\n>\n\nWhile I think this could be part of the solution, I don't think it would be\ncomplete. I still feel there is a level of control that an application may\n(optionally) want to exert on the pipeline handlers. If, for example,\nthe application wants the pipeline handler to return the metadata buffer\n(I know we cannot do this right now but...), then the IPA cannot make\nthe decision in isolation. If you expand on the \"hints\" idea, you could\nalso see the application sending hints about mode choices, e.g. fast\nfps preferred, or larger FoV preferred, etc. The IPA would not know or\ncare about any of these choices.\n\nRegards,\nNaush","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 9891EBD1F1\n\tfor ;\n\tFri, 5 Mar 2021 09:03:14 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E8BC368A92;\n\tFri, 5 Mar 2021 10:03:13 +0100 (CET)","from mail-lf1-x131.google.com (mail-lf1-x131.google.com\n\t[IPv6:2a00:1450:4864:20::131])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BC6E8602E8\n\tfor ;\n\tFri, 5 Mar 2021 10:03:11 +0100 (CET)","by mail-lf1-x131.google.com with SMTP id 18so2371910lff.6\n\tfor ;\n\tFri, 05 Mar 2021 01:03:11 -0800 (PST)"],"Authentication-Results":"lancelot.ideasonboard.com;\n\tdkim=fail reason=\"signature verification failed\" (2048-bit key;\n\tunprotected) header.d=raspberrypi.com header.i=@raspberrypi.com\n\theader.b=\"R9BWAUA4\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=raspberrypi.com; s=google;\n\th=mime-version:references:in-reply-to:from:date:message-id:subject:to\n\t:cc; bh=Zdh79gXHIx0BZ5cWYOO0sq2dGnrjXiDZmEGDmw3GHSU=;\n\tb=R9BWAUA4wR3LRoxCi8mrEgOXhs9IoE6/rJ9pgmKLfG6eBqT5IADGcA8YdT+CKyAdCs\n\tiOqGO7mpuC0xrLw+TYuSDEgxcMWY783XvNlelZ/NvW9dU54DqW4XDF7bWUJW+fZVuYJ5\n\tdpYVAi+0Byod4L5Na8eKbBEG/HkK/LG3Or/0+KylaoYTnEP46Oe7i8I3XTSOfNUFKZZ4\n\tzu3/IOX7d8bzI8IJ5ioT2yIZk0E9EKwxJSdffBUVXp3vyohu/AsrIOvl67LnFY03iFJK\n\twvT1h96zzA/BwEg3dBgO5NhGEc2evuiLaqafLl8hpQnip/wzawHW7Qi13CkKA6w7v1WX\n\t8YZA==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:mime-version:references:in-reply-to:from:date\n\t:message-id:subject:to:cc;\n\tbh=Zdh79gXHIx0BZ5cWYOO0sq2dGnrjXiDZmEGDmw3GHSU=;\n\tb=tEjdzoavgdAolFEVSA9Y4YR8vz2OkzE0il8a9p2/dW5scl9X1LmcDQtfrcBrf21jQG\n\t9LbKrkdYQnVDOM5uLFeRdqltuT0g6IpE9ZLMPrjkeqrdwKcSpRQhgwSCMW4SbYCWJZkw\n\tl8Mse7V7Bywoxffn4vKyx5PIxWP5hv9fufhghl0D07nBs+qDtP1RfzyRBVYukPDwOWoB\n\tPYB44E9PbICVnSTjS/Q4h9TzssVSAFIFkrIHqOQGBSOHgb1aSw4M6lXsvxAalfiGO+s/\n\tQ0LAMBjYmfz0O/WCWGRrj7iWREza5r3tBJ9LrRwbSpBUq/gn4Bpj3Cv6ETHGMVip9HqT\n\tUeYg==","X-Gm-Message-State":"AOAM533sEwsRi/iY0YB5Pda6BQfBRzIP1xpIAGN7JIXARidzeU0DnraU\n\tG+siEYfWB1Yjl9JX+gb541uy4myPTDciygpwpaiDFw==","X-Google-Smtp-Source":"ABdhPJzqH/8FLTgsdDJtR4ggF+c1M2stbq/PV6Z550mRs8pmTPK6nF648EnGkkROUDrw2u2W/oBADWyaAyPLamieVj8=","X-Received":"by 2002:a05:6512:ac7:: with SMTP id\n\tn7mr4827710lfu.567.1614934990920; \n\tFri, 05 Mar 2021 01:03:10 -0800 (PST)","MIME-Version":"1.0","References":"<20210218170126.2060783-1-naush@raspberrypi.com>\n\t<20210218170126.2060783-5-naush@raspberrypi.com>\n\t\n\t\n\t\n\t\n\t\n\t","In-Reply-To":"","From":"Naushir Patuck ","Date":"Fri, 5 Mar 2021 09:02:55 +0000","Message-ID":"","To":"David Plowman ","Subject":"Re: [libcamera-devel] [PATCH v3 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":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera devel ","Content-Type":"multipart/mixed;\n\tboundary=\"===============2455340572259878480==\"","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]