[{"id":12770,"web_url":"https://patchwork.libcamera.org/comment/12770/","msgid":"<20200925142018.5s5whsgupf6uzmni@uno.localdomain>","date":"2020-09-25T14:20:18","subject":"Re: [libcamera-devel] [PATCH v3 05/22] libcamera: pipeline: rkisp1:\n\tPrepare buffer ready handlers for multiple streams","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Fri, Sep 25, 2020 at 03:41:50AM +0200, Niklas Söderlund wrote:\n> The buffer ready handlers are designed for a single application facing\n> stream from the main path. To prepare for multiple application facing\n> streams from main and/or self path the handlers need to be prepared.\n>\n> The data keeping tasks of frame number and advancing the timeline can be\n\nKeepin 'track'\n\nthe frame number\nor\nframe numbers\n\n\n> moved from the application facing buffer ready handler to the statistics\n> handler. For each request processed there will always be a statistic\n\nbe one statistic\n\n> buffer and as the ISP is inline and is the source of both main, self and\n> statistic paths there is no lose in precision from this.\n\nnot sure but 'lose in precision' sounds weird\n\n>\n> The application facing handler no longer needs a special case for\n> cancelled frames and can be made simpler. With this change the handlers\n> are ready to deal with any combinations of application facing streams.\n>\n> Signed-off-by: Niklas Söderlund \n> Reviewed-by: Jacopo Mondi \n> Reviewed-by: Laurent Pinchart \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++------------\n> 1 file changed, 5 insertions(+), 12 deletions(-)\n>\n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index 8bb4582eeb688a4c..d6b5073b2084a9f4 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -1070,20 +1070,8 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)\n> void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> {\n> \tASSERT(activeCamera_);\n> -\tRkISP1CameraData *data = cameraData(activeCamera_);\n> \tRequest *request = buffer->request();\n>\n> -\tif (buffer->metadata().status == FrameMetadata::FrameCancelled) {\n> -\t\tcompleteBuffer(activeCamera_, request, buffer);\n> -\t\tcompleteRequest(activeCamera_, request);\n> -\t\treturn;\n> -\t}\n> -\n> -\tdata->timeline_.bufferReady(buffer);\n> -\n> -\tif (data->frame_ <= buffer->metadata().sequence)\n> -\t\tdata->frame_ = buffer->metadata().sequence + 1;\n> -\n> \tcompleteBuffer(activeCamera_, request, buffer);\n> \ttryCompleteRequest(request);\n> }\n> @@ -1114,6 +1102,11 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n> \tif (!info)\n> \t\treturn;\n>\n> +\tdata->timeline_.bufferReady(buffer);\n> +\n> +\tif (data->frame_ <= buffer->metadata().sequence)\n\nHow can frame_ be larger ?\n\nPlease keep my tag\n\nThanks\n j\n\n> +\t\tdata->frame_ = buffer->metadata().sequence + 1;\n> +\n> \tIPAOperationData op;\n> \top.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER;\n> \top.data = { info->frame, info->statBuffer->cookie() };\n> --\n> 2.28.0\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 089B6C3B5C\n\tfor ;\n\tFri, 25 Sep 2020 14:16:28 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id DAE3063021;\n\tFri, 25 Sep 2020 16:16:27 +0200 (CEST)","from relay3-d.mail.gandi.net (relay3-d.mail.gandi.net\n\t[217.70.183.195])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 852C162FD8\n\tfor ;\n\tFri, 25 Sep 2020 16:16:26 +0200 (CEST)","from uno.localdomain (host-87-18-63-10.retail.telecomitalia.it\n\t[87.18.63.10]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay3-d.mail.gandi.net (Postfix) with ESMTPSA id 6D96D6000E;\n\tFri, 25 Sep 2020 14:16:25 +0000 (UTC)"],"X-Originating-IP":"87.18.63.10","Date":"Fri, 25 Sep 2020 16:20:18 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200925142018.5s5whsgupf6uzmni@uno.localdomain>","References":"<20200925014207.1455796-1-niklas.soderlund@ragnatech.se>\n\t<20200925014207.1455796-6-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200925014207.1455796-6-niklas.soderlund@ragnatech.se>","Subject":"Re: [libcamera-devel] [PATCH v3 05/22] libcamera: pipeline: rkisp1:\n\tPrepare buffer ready handlers for multiple streams","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":12781,"web_url":"https://patchwork.libcamera.org/comment/12781/","msgid":"<20200925161813.GC1757254@oden.dyn.berto.se>","date":"2020-09-25T16:18:13","subject":"Re: [libcamera-devel] [PATCH v3 05/22] libcamera: pipeline: rkisp1:\n\tPrepare buffer ready handlers for multiple streams","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nThanks for your feedback.\n\nOn 2020-09-25 16:20:18 +0200, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Fri, Sep 25, 2020 at 03:41:50AM +0200, Niklas Söderlund wrote:\n> > The buffer ready handlers are designed for a single application facing\n> > stream from the main path. To prepare for multiple application facing\n> > streams from main and/or self path the handlers need to be prepared.\n> >\n> > The data keeping tasks of frame number and advancing the timeline can be\n> \n> Keepin 'track'\n> \n> the frame number\n> or\n> frame numbers\n> \n> \n> > moved from the application facing buffer ready handler to the statistics\n> > handler. For each request processed there will always be a statistic\n> \n> be one statistic\n> \n> > buffer and as the ISP is inline and is the source of both main, self and\n> > statistic paths there is no lose in precision from this.\n> \n> not sure but 'lose in precision' sounds weird\n> \n> >\n> > The application facing handler no longer needs a special case for\n> > cancelled frames and can be made simpler. With this change the handlers\n> > are ready to deal with any combinations of application facing streams.\n> >\n> > Signed-off-by: Niklas Söderlund \n> > Reviewed-by: Jacopo Mondi \n> > Reviewed-by: Laurent Pinchart \n> > ---\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++------------\n> > 1 file changed, 5 insertions(+), 12 deletions(-)\n> >\n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index 8bb4582eeb688a4c..d6b5073b2084a9f4 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -1070,20 +1070,8 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)\n> > void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> > {\n> > \tASSERT(activeCamera_);\n> > -\tRkISP1CameraData *data = cameraData(activeCamera_);\n> > \tRequest *request = buffer->request();\n> >\n> > -\tif (buffer->metadata().status == FrameMetadata::FrameCancelled) {\n> > -\t\tcompleteBuffer(activeCamera_, request, buffer);\n> > -\t\tcompleteRequest(activeCamera_, request);\n> > -\t\treturn;\n> > -\t}\n> > -\n> > -\tdata->timeline_.bufferReady(buffer);\n> > -\n> > -\tif (data->frame_ <= buffer->metadata().sequence)\n> > -\t\tdata->frame_ = buffer->metadata().sequence + 1;\n> > -\n> > \tcompleteBuffer(activeCamera_, request, buffer);\n> > \ttryCompleteRequest(request);\n> > }\n> > @@ -1114,6 +1102,11 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n> > \tif (!info)\n> > \t\treturn;\n> >\n> > +\tdata->timeline_.bufferReady(buffer);\n> > +\n> > +\tif (data->frame_ <= buffer->metadata().sequence)\n> \n> How can frame_ be larger ?\n\nIt can't :-) But if it's smaller the hardware have dropped frames and we \nneed to account for that.\n\n> \n> Please keep my tag\n> \n> Thanks\n> j\n> \n> > +\t\tdata->frame_ = buffer->metadata().sequence + 1;\n> > +\n> > \tIPAOperationData op;\n> > \top.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER;\n> > \top.data = { info->frame, info->statBuffer->cookie() };\n> > --\n> > 2.28.0\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 B949CC3B5C\n\tfor ;\n\tFri, 25 Sep 2020 16:19:15 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 81D916303C;\n\tFri, 25 Sep 2020 18:19:15 +0200 (CEST)","from mail-lf1-f65.google.com (mail-lf1-f65.google.com\n\t[209.85.167.65])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8EA3262FD8\n\tfor ;\n\tFri, 25 Sep 2020 18:19:14 +0200 (CEST)","by mail-lf1-f65.google.com with SMTP id u8so3464061lff.1\n\tfor ;\n\tFri, 25 Sep 2020 09:19:14 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tw17sm2647385lfr.31.2020.09.25.09.18.13\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 25 Sep 2020 09:18:13 -0700 (PDT)"],"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=\"Y73DabuU\"; 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=mKfr2hVnUkv6as3yIQx47G4i/V5Sp9tP7G2s41gG3XE=;\n\tb=Y73DabuUmEZ2fTwn1c5ZgYfeWXCIDxf8BAnWZ1IUEjZIeWTE+akFsEs+SvMcVXcwzY\n\t4ESGgdekf0OXZ7lB0iFwNhNY3cWLBhMFVPwhYfvLrK/2phNDD+8EBRr+3wfyEyscUR4w\n\teh0RSqaQO1cd2Vkd0rglouAvgD1TK9n24O3ogfHVKBsfdzqiRIpru3KyaDR+oVEDM5BO\n\tuB5/cndXP+rivIaZTj4INM3mFEzxGdPgELDvK2kQqhBsG0ttOdsjY6a7z5z3/HfCQ9F6\n\ttdNjW+69ksGsAGrqCs4QhRstY0hh2INcZ3stt5n0i28z1ZNdw0opwJMRSGIkj1Em1ZFg\n\tKFMA==","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=mKfr2hVnUkv6as3yIQx47G4i/V5Sp9tP7G2s41gG3XE=;\n\tb=FPrg85ZKy3r/+6SnRxLyf1FThOVezaz3tILCFz6pg1dKL8tBEQ/uOEP/BEq2dlAAnQ\n\tPEve9C/yCBHbdZIWhHzxVM88yf4gdgUR3lxqb/fb8nitqZgQf7o8sYJTkZBhZ6uAeY+t\n\tQLEgmd+7gu3aX3tP7TC2a7ukTmiWGoEBQnwwe6HszUbaICmFT4K5+f753fD1zSrnlh8r\n\t28ldc3U5RwKEelVrXGYBEWX8RrVTL59t31DExepR6Fls7eDCcyewIXy0Ae6XibKyE5tu\n\tdoptb6AeQFXem+8RLPAlEFVN6TCRf5d5Cx2iayruRRWORS+wFVGQLv09UGY6S23YG7qw\n\tYclQ==","X-Gm-Message-State":"AOAM531T3i2zpDB0jzrNu92fBGCX6drejZfysGJGV/5qlaJYKK51dIgD\n\tBp9i+OCzSEaGr5nFaMn7Pedv/SugWMo+/A==","X-Google-Smtp-Source":"ABdhPJwpux8uMGwaRymEToQl0FgpP2dURLLrBQ4Yl9I+71MZtKoBWJm+farvVwIugXEXOyqy5tQrqA==","X-Received":"by 2002:a05:6512:20c3:: with SMTP id\n\tu3mr1497960lfr.572.1601050694111; \n\tFri, 25 Sep 2020 09:18:14 -0700 (PDT)","Date":"Fri, 25 Sep 2020 18:18:13 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"<20200925161813.GC1757254@oden.dyn.berto.se>","References":"<20200925014207.1455796-1-niklas.soderlund@ragnatech.se>\n\t<20200925014207.1455796-6-niklas.soderlund@ragnatech.se>\n\t<20200925142018.5s5whsgupf6uzmni@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200925142018.5s5whsgupf6uzmni@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v3 05/22] libcamera: pipeline: rkisp1:\n\tPrepare buffer ready handlers for multiple streams","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\" "}},{"id":12793,"web_url":"https://patchwork.libcamera.org/comment/12793/","msgid":"<20200928081655.wrw72o475tjdv2nd@uno.localdomain>","date":"2020-09-28T08:16:55","subject":"Re: [libcamera-devel] [PATCH v3 05/22] libcamera: pipeline: rkisp1:\n\tPrepare buffer ready handlers for multiple streams","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Fri, Sep 25, 2020 at 06:18:13PM +0200, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your feedback.\n>\n> On 2020-09-25 16:20:18 +0200, Jacopo Mondi wrote:\n> > Hi Niklas,\n> >\n> > On Fri, Sep 25, 2020 at 03:41:50AM +0200, Niklas Söderlund wrote:\n> > > The buffer ready handlers are designed for a single application facing\n> > > stream from the main path. To prepare for multiple application facing\n> > > streams from main and/or self path the handlers need to be prepared.\n> > >\n> > > The data keeping tasks of frame number and advancing the timeline can be\n> >\n> > Keepin 'track'\n> >\n> > the frame number\n> > or\n> > frame numbers\n> >\n> >\n> > > moved from the application facing buffer ready handler to the statistics\n> > > handler. For each request processed there will always be a statistic\n> >\n> > be one statistic\n> >\n> > > buffer and as the ISP is inline and is the source of both main, self and\n> > > statistic paths there is no lose in precision from this.\n> >\n> > not sure but 'lose in precision' sounds weird\n> >\n> > >\n> > > The application facing handler no longer needs a special case for\n> > > cancelled frames and can be made simpler. With this change the handlers\n> > > are ready to deal with any combinations of application facing streams.\n> > >\n> > > Signed-off-by: Niklas Söderlund \n> > > Reviewed-by: Jacopo Mondi \n> > > Reviewed-by: Laurent Pinchart \n> > > ---\n> > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++------------\n> > > 1 file changed, 5 insertions(+), 12 deletions(-)\n> > >\n> > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > index 8bb4582eeb688a4c..d6b5073b2084a9f4 100644\n> > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > @@ -1070,20 +1070,8 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)\n> > > void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> > > {\n> > > \tASSERT(activeCamera_);\n> > > -\tRkISP1CameraData *data = cameraData(activeCamera_);\n> > > \tRequest *request = buffer->request();\n> > >\n> > > -\tif (buffer->metadata().status == FrameMetadata::FrameCancelled) {\n> > > -\t\tcompleteBuffer(activeCamera_, request, buffer);\n> > > -\t\tcompleteRequest(activeCamera_, request);\n> > > -\t\treturn;\n> > > -\t}\n> > > -\n> > > -\tdata->timeline_.bufferReady(buffer);\n> > > -\n> > > -\tif (data->frame_ <= buffer->metadata().sequence)\n> > > -\t\tdata->frame_ = buffer->metadata().sequence + 1;\n> > > -\n> > > \tcompleteBuffer(activeCamera_, request, buffer);\n> > > \ttryCompleteRequest(request);\n> > > }\n> > > @@ -1114,6 +1102,11 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n> > > \tif (!info)\n> > > \t\treturn;\n> > >\n> > > +\tdata->timeline_.bufferReady(buffer);\n> > > +\n> > > +\tif (data->frame_ <= buffer->metadata().sequence)\n> >\n> > How can frame_ be larger ?\n>\n> It can't :-) But if it's smaller the hardware have dropped frames and we\n> need to account for that.\n>\n\nIf it can't be larger why the check then ?\nAnd if it's smaller it seems to me it just gets advanced to\nmetadata().sequence + 1 unconditionally.\n\nWhat have I missed ?\n\n\n> >\n> > Please keep my tag\n> >\n> > Thanks\n> > j\n> >\n> > > +\t\tdata->frame_ = buffer->metadata().sequence + 1;\n> > > +\n> > > \tIPAOperationData op;\n> > > \top.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER;\n> > > \top.data = { info->frame, info->statBuffer->cookie() };\n> > > --\n> > > 2.28.0\n> > >\n>\n> --\n> Regards,\n> Niklas Söderlund","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 719CCC3B5B\n\tfor ;\n\tMon, 28 Sep 2020 08:13:02 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BF4C560BF7;\n\tMon, 28 Sep 2020 10:13:01 +0200 (CEST)","from relay6-d.mail.gandi.net (relay6-d.mail.gandi.net\n\t[217.70.183.198])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 8804C6035F\n\tfor ;\n\tMon, 28 Sep 2020 10:13:00 +0200 (CEST)","from uno.localdomain (93-34-118-233.ip49.fastwebnet.it\n\t[93.34.118.233]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay6-d.mail.gandi.net (Postfix) with ESMTPSA id C90B9C0016;\n\tMon, 28 Sep 2020 08:12:59 +0000 (UTC)"],"X-Originating-IP":"93.34.118.233","Date":"Mon, 28 Sep 2020 10:16:55 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20200928081655.wrw72o475tjdv2nd@uno.localdomain>","References":"<20200925014207.1455796-1-niklas.soderlund@ragnatech.se>\n\t<20200925014207.1455796-6-niklas.soderlund@ragnatech.se>\n\t<20200925142018.5s5whsgupf6uzmni@uno.localdomain>\n\t<20200925161813.GC1757254@oden.dyn.berto.se>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200925161813.GC1757254@oden.dyn.berto.se>","Subject":"Re: [libcamera-devel] [PATCH v3 05/22] libcamera: pipeline: rkisp1:\n\tPrepare buffer ready handlers for multiple streams","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":12827,"web_url":"https://patchwork.libcamera.org/comment/12827/","msgid":"<20200928212222.GA1052036@oden.dyn.berto.se>","date":"2020-09-28T21:22:22","subject":"Re: [libcamera-devel] [PATCH v3 05/22] libcamera: pipeline: rkisp1:\n\tPrepare buffer ready handlers for multiple streams","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Jacopo,\n\nOn 2020-09-28 10:16:55 +0200, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Fri, Sep 25, 2020 at 06:18:13PM +0200, Niklas Söderlund wrote:\n> > Hi Jacopo,\n> >\n> > Thanks for your feedback.\n> >\n> > On 2020-09-25 16:20:18 +0200, Jacopo Mondi wrote:\n> > > Hi Niklas,\n> > >\n> > > On Fri, Sep 25, 2020 at 03:41:50AM +0200, Niklas Söderlund wrote:\n> > > > The buffer ready handlers are designed for a single application facing\n> > > > stream from the main path. To prepare for multiple application facing\n> > > > streams from main and/or self path the handlers need to be prepared.\n> > > >\n> > > > The data keeping tasks of frame number and advancing the timeline can be\n> > >\n> > > Keepin 'track'\n> > >\n> > > the frame number\n> > > or\n> > > frame numbers\n> > >\n> > >\n> > > > moved from the application facing buffer ready handler to the statistics\n> > > > handler. For each request processed there will always be a statistic\n> > >\n> > > be one statistic\n> > >\n> > > > buffer and as the ISP is inline and is the source of both main, self and\n> > > > statistic paths there is no lose in precision from this.\n> > >\n> > > not sure but 'lose in precision' sounds weird\n> > >\n> > > >\n> > > > The application facing handler no longer needs a special case for\n> > > > cancelled frames and can be made simpler. With this change the handlers\n> > > > are ready to deal with any combinations of application facing streams.\n> > > >\n> > > > Signed-off-by: Niklas Söderlund \n> > > > Reviewed-by: Jacopo Mondi \n> > > > Reviewed-by: Laurent Pinchart \n> > > > ---\n> > > > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 17 +++++------------\n> > > > 1 file changed, 5 insertions(+), 12 deletions(-)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > index 8bb4582eeb688a4c..d6b5073b2084a9f4 100644\n> > > > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > > > @@ -1070,20 +1070,8 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)\n> > > > void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> > > > {\n> > > > \tASSERT(activeCamera_);\n> > > > -\tRkISP1CameraData *data = cameraData(activeCamera_);\n> > > > \tRequest *request = buffer->request();\n> > > >\n> > > > -\tif (buffer->metadata().status == FrameMetadata::FrameCancelled) {\n> > > > -\t\tcompleteBuffer(activeCamera_, request, buffer);\n> > > > -\t\tcompleteRequest(activeCamera_, request);\n> > > > -\t\treturn;\n> > > > -\t}\n> > > > -\n> > > > -\tdata->timeline_.bufferReady(buffer);\n> > > > -\n> > > > -\tif (data->frame_ <= buffer->metadata().sequence)\n> > > > -\t\tdata->frame_ = buffer->metadata().sequence + 1;\n> > > > -\n> > > > \tcompleteBuffer(activeCamera_, request, buffer);\n> > > > \ttryCompleteRequest(request);\n> > > > }\n> > > > @@ -1114,6 +1102,11 @@ void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n> > > > \tif (!info)\n> > > > \t\treturn;\n> > > >\n> > > > +\tdata->timeline_.bufferReady(buffer);\n> > > > +\n> > > > +\tif (data->frame_ <= buffer->metadata().sequence)\n> > >\n> > > How can frame_ be larger ?\n> >\n> > It can't :-) But if it's smaller the hardware have dropped frames and we\n> > need to account for that.\n> >\n> \n> If it can't be larger why the check then ?\n> And if it's smaller it seems to me it just gets advanced to\n> metadata().sequence + 1 unconditionally.\n> \n> What have I missed ?\n\nThis is not the optimum location to increment the internal frame counter \nas it's only incremented once a buffer is complete and have been \ndequeued. This will lead to multiple requests queued in a short interval \nwould have the same (pipeline internal) frame number.\n\nInstead we increment the frame whenever a request is queued to the \npipeline from the application. But we still need this check here to \ndetect if the hardware have dropped any frames due to the pipeline being \nto slow (this have been observed before OpenGL rendering was added to \nqcam).\n\nGoing forward I think we need to rethink the Timeline used in this \npipeline, possibly looking to how this is solved in the RPi pipeline. \nBut that is a task for another series.\n\n> \n> \n> > >\n> > > Please keep my tag\n> > >\n> > > Thanks\n> > > j\n> > >\n> > > > +\t\tdata->frame_ = buffer->metadata().sequence + 1;\n> > > > +\n> > > > \tIPAOperationData op;\n> > > > \top.operation = RKISP1_IPA_EVENT_SIGNAL_STAT_BUFFER;\n> > > > \top.data = { info->frame, info->statBuffer->cookie() };\n> > > > --\n> > > > 2.28.0\n> > > >\n> >\n> > --\n> > Regards,\n> > Niklas Söderlund","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 7F904C3B5C\n\tfor ;\n\tMon, 28 Sep 2020 21:22:26 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 09F3B603ED;\n\tMon, 28 Sep 2020 23:22:26 +0200 (CEST)","from mail-lf1-x143.google.com (mail-lf1-x143.google.com\n\t[IPv6:2a00:1450:4864:20::143])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id B9BE960366\n\tfor ;\n\tMon, 28 Sep 2020 23:22:24 +0200 (CEST)","by mail-lf1-x143.google.com with SMTP id q8so3003724lfb.6\n\tfor ;\n\tMon, 28 Sep 2020 14:22:24 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\ti5sm3024814lfe.8.2020.09.28.14.22.23\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tMon, 28 Sep 2020 14:22:23 -0700 (PDT)"],"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=\"VOLmDmUZ\"; 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=Qd7WkV/2Tsw5b0ZUcAijAp2RWoLfdVL7uPLrHmGp/dE=;\n\tb=VOLmDmUZcW1PMyUjqBwxphZa+4Ev81qLuMKKfnrXFJJIj/zquCNZdCs3djxi7G3bY7\n\t9iQh5j8fW1da1w+Utm+9IlCDDIBzhetq7to4GNNocxM7k8I4DWJKKCIN3MKqMByoORb3\n\t7utqEXQavs7XbmMVg4h4y4zaz90jGP2Lv36uzJfsnOipnqQayW/P20l5JC0aTXgGRHmM\n\ttaw4b1J1EwBIBQh0+EGYotnYN7eYBy8i4NZgo6g9XXrX1CLFLZ6nriix7INkIHU73zu9\n\tIYttxa4WHPbRttPN3mi4WWyjAMoH9qbS2E5j+GYKW2POybozv0ss6LAhMvUPz26YK1JO\n\t+4rQ==","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=Qd7WkV/2Tsw5b0ZUcAijAp2RWoLfdVL7uPLrHmGp/dE=;\n\tb=cygLcVssBMaaKt7hSinWDM4I/gg9Y38YWb/5S15LuQud5VlDait5fzwhVidZ/Hz8ML\n\tBYuHroJCrDxOXiHYRNPeCoQmiidk4vznzwmgAT/kZSwSsanxfIMwd56yEgXKzVK4BqtH\n\tuo74y9K1H2c2WUb0GYo+xUGjl/alhYRZyWjkNYg3Po7msE0vba4l3V+yJVqasSKmrshb\n\t6Jp0WAhM87Ck5F4Cfv3LIbJ4JY868XSz5R2GgHSyP6Dv7zZiIHJpALsmlxLBuMQU3J5h\n\tIKrW9goiw64Os9Nntvw/3RKwh5I/t/+ZWzmKPrekBqNAa/gsQQm2zr+C0iWZQ950hUJn\n\tSOaQ==","X-Gm-Message-State":"AOAM532RgISMy95jMML6s6cDg04a5pdx06TgCDf9HpDMJerP1Om/HqKR\n\tTyHwDSo1Ia7JBkphNRj0cZVvOw==","X-Google-Smtp-Source":"ABdhPJxH/iubTwOHCuXi4ygG6vzTjFLjH+o3w9enT3xepSkievDCKpiERvxr2Y/gE1Tr7TzqbwoZmg==","X-Received":"by 2002:a19:447:: with SMTP id 68mr59667lfe.26.1601328143950;\n\tMon, 28 Sep 2020 14:22:23 -0700 (PDT)","Date":"Mon, 28 Sep 2020 23:22:22 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"<20200928212222.GA1052036@oden.dyn.berto.se>","References":"<20200925014207.1455796-1-niklas.soderlund@ragnatech.se>\n\t<20200925014207.1455796-6-niklas.soderlund@ragnatech.se>\n\t<20200925142018.5s5whsgupf6uzmni@uno.localdomain>\n\t<20200925161813.GC1757254@oden.dyn.berto.se>\n\t<20200928081655.wrw72o475tjdv2nd@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20200928081655.wrw72o475tjdv2nd@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v3 05/22] libcamera: pipeline: rkisp1:\n\tPrepare buffer ready handlers for multiple streams","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\" "}}]