[{"id":4318,"web_url":"https://patchwork.libcamera.org/comment/4318/","msgid":"<20200326165259.GY20581@pendragon.ideasonboard.com>","date":"2020-03-26T16:52:59","subject":"Re: [libcamera-devel] [RFCv2 4/7] libcamera: pipeline: rkisp1: Do\n\tnot try to process cancelled frames","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Thu, Mar 26, 2020 at 05:08:16PM +0100, Niklas Söderlund wrote:\n> Their is no need to try and process cancelled frames.\n\ns/Their/There/\n\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +++++++++\n> 1 file changed, 9 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> index f64807984519779b..f49b3a7f0945ac92 100644\n> --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> @@ -1011,6 +1011,9 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)\n> \n> void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> {\n> +\tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> +\t\treturn;\n> +\n> \tASSERT(activeCamera_);\n> \tRkISP1CameraData *data = cameraData(activeCamera_);\n> \tRequest *request = buffer->request();\n\nFor the capture buffer, you actually need to process it in order to\ncomplete the request. You'll need a special case there, as you should\nnot wait for metadata in that case (we're stopping, metadata may never\narrive).\n\n> @@ -1026,6 +1029,9 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> \n> void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)\n> {\n> +\tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> +\t\treturn;\n> +\n> \tASSERT(activeCamera_);\n> \tRkISP1CameraData *data = cameraData(activeCamera_);\n\nFor the params buffer, I think it should not take part in request\ncompletion at all, so the logic in this function has to be reworked.\n\n> \n> @@ -1037,6 +1043,9 @@ void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)\n> \n> void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n> {\n> +\tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> +\t\treturn;\n> +\n> \tASSERT(activeCamera_);\n> \tRkISP1CameraData *data = cameraData(activeCamera_);\n>","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 48A9560412\n\tfor ;\n\tThu, 26 Mar 2020 17:53:04 +0100 (CET)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id BA18C2DC;\n\tThu, 26 Mar 2020 17:53:03 +0100 (CET)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key; \n\tunprotected) header.d=ideasonboard.com\n\theader.i=@ideasonboard.com\n\theader.b=\"Eqo08a+Y\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1585241584;\n\tbh=1JhVmQo5lt9SSKldMTxiPo5Ta6jt4yeDatjYHfdFsOw=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Eqo08a+YeJS5u5wox1MFu7TXeep46C4vwUEXPAMdNJAyYl9nHI4svw7p2Ji1kb1T4\n\tkjX5BNke7yWs3jOhw6pA4pl6wY8vsg1Ba0+LU9WdA9cPpZKrb+/AKq5TumEmZwDZ1u\n\tMqkNyodOVOV42qTXGfrgLNdeY9MSt/Pq6A97pSjc=","Date":"Thu, 26 Mar 2020 18:52:59 +0200","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200326165259.GY20581@pendragon.ideasonboard.com>","References":"<20200326160819.4088361-1-niklas.soderlund@ragnatech.se>\n\t<20200326160819.4088361-5-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200326160819.4088361-5-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFCv2 4/7] libcamera: pipeline: rkisp1: Do\n\tnot try to process cancelled frames","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","X-List-Received-Date":"Thu, 26 Mar 2020 16:53:04 -0000"}},{"id":4351,"web_url":"https://patchwork.libcamera.org/comment/4351/","msgid":"<20200328002807.GA1869774@oden.dyn.berto.se>","date":"2020-03-28T00:28:07","subject":"Re: [libcamera-devel] [RFCv2 4/7] libcamera: pipeline: rkisp1: Do\n\tnot try to process cancelled frames","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2020-03-26 18:52:59 +0200, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Thu, Mar 26, 2020 at 05:08:16PM +0100, Niklas Söderlund wrote:\n> > Their is no need to try and process cancelled frames.\n> \n> s/Their/There/\n> \n> > \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/libcamera/pipeline/rkisp1/rkisp1.cpp | 9 +++++++++\n> > 1 file changed, 9 insertions(+)\n> > \n> > diff --git a/src/libcamera/pipeline/rkisp1/rkisp1.cpp b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > index f64807984519779b..f49b3a7f0945ac92 100644\n> > --- a/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > +++ b/src/libcamera/pipeline/rkisp1/rkisp1.cpp\n> > @@ -1011,6 +1011,9 @@ void PipelineHandlerRkISP1::tryCompleteRequest(Request *request)\n> > \n> > void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> > {\n> > +\tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> > +\t\treturn;\n> > +\n> > \tASSERT(activeCamera_);\n> > \tRkISP1CameraData *data = cameraData(activeCamera_);\n> > \tRequest *request = buffer->request();\n> \n> For the capture buffer, you actually need to process it in order to\n> complete the request. You'll need a special case there, as you should\n> not wait for metadata in that case (we're stopping, metadata may never\n> arrive).\n\nTrue and then we have a problem :-(\n\nIn stop() we stops all video devices and set activeCamera_ to nullptr \nand we need the camera to complete the request. We would then need a way \nto wait for all requests to finish before that don't we ? I will dig \nsome and see what is what.\n\n> \n> > @@ -1026,6 +1029,9 @@ void PipelineHandlerRkISP1::bufferReady(FrameBuffer *buffer)\n> > \n> > void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)\n> > {\n> > +\tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> > +\t\treturn;\n> > +\n> > \tASSERT(activeCamera_);\n> > \tRkISP1CameraData *data = cameraData(activeCamera_);\n> \n> For the params buffer, I think it should not take part in request\n> completion at all, so the logic in this function has to be reworked.\n> \n> > \n> > @@ -1037,6 +1043,9 @@ void PipelineHandlerRkISP1::paramReady(FrameBuffer *buffer)\n> > \n> > void PipelineHandlerRkISP1::statReady(FrameBuffer *buffer)\n> > {\n> > +\tif (buffer->metadata().status == FrameMetadata::FrameCancelled)\n> > +\t\treturn;\n> > +\n> > \tASSERT(activeCamera_);\n> > \tRkISP1CameraData *data = cameraData(activeCamera_);\n> > \n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lf1-x12b.google.com (mail-lf1-x12b.google.com\n\t[IPv6:2a00:1450:4864:20::12b])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 0C9B560412\n\tfor ;\n\tSat, 28 Mar 2020 01:28:10 +0100 (CET)","by mail-lf1-x12b.google.com with SMTP id j11so9345898lfg.4\n\tfor ;\n\tFri, 27 Mar 2020 17:28:09 -0700 (PDT)","from localhost (h-200-138.A463.priv.bahnhof.se. [176.10.200.138])\n\tby smtp.gmail.com with ESMTPSA id\n\tn26sm3236325ljg.93.2020.03.27.17.28.08\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 27 Mar 2020 17:28:08 -0700 (PDT)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (2048-bit key; \n\tunprotected)\n\theader.d=ragnatech-se.20150623.gappssmtp.com\n\theader.i=@ragnatech-se.20150623.gappssmtp.com header.b=\"g39Rq//h\"; \n\tdkim-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=iq0rjvTcWdCg34Qo9pwrJzWntyhSZWoqfWRhagjgWZ8=;\n\tb=g39Rq//hIUmbbz1njIaWiUjlS3VJQhvAO7VHBKzoRBFQ1dZBXIVsv0hnMQ/AmvY1lx\n\tmHgHUPsAQq2cx6QpB2L7qXpjbAtpxLgCd05MNb/UYbdjYOPZlcy4tKqT3TmP4uLRWzMR\n\t1Q/y36g3S5MPYts7bG5c7A/d9sHwhbfUZnQtaE0/8SoNqZWJgXc9X6YlDRazGQYLDg07\n\t2wwX4o7a6zjLBUmFGoDJC/Xk4eoqiORpml1Kudu9/taXF8HXuTVQ0HBd5ooB/p1NpWER\n\tbVi8K3AF7qYTIO+sKQNZWKdJL7bwXrbEKhh3FkCe+XVg0ZPxAX+xBkQAjhpCKCJlxQ1C\n\tGcKA==","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=iq0rjvTcWdCg34Qo9pwrJzWntyhSZWoqfWRhagjgWZ8=;\n\tb=dGqZTd3yk+de32tvdj5IiW8R78JTWC0cSyrSGAgF8zCYJkqdptYJzU/AKAGuDK4M7i\n\titkwMtD5LW643ePf0VHUWyFzaw6kyypd9nf23p3tFTddhOjbe8LO+VNXIaEzEQLaiOC6\n\tUF8q5My98YVqm6ccVMQmFv/2r//aj/MdeagH7c8iOmug6yBTquq3THd/H0kN9L4ATEkC\n\tT1YMpPtuIFFPyJHlNZVAvRmkKVWB0frC0Op2AHRxQqxkr2tHXgtFCtGvyqAdO//uGDim\n\t6Pk8gqUhUf1IjXc5zc0GVGwP/pr2+aDjlzTOxZz0DDBqvqklJLT3TMdg+zXF3l1Lkjy4\n\tdK7w==","X-Gm-Message-State":"AGi0PubufC3XWelDYmrA0naUqRF2MpqqE5t4dYaKmApRzFQsYIbRLMx3\n\tvcsdKWBALqMvWb6s+BpUym0hbJvMoJU=","X-Google-Smtp-Source":"APiQypJD79t7NvBu7VdJGkW/ifmInf8jLXGoOwPavGtea2PdeLONv2OevpU9XE3aSk/2zRADsyQaVQ==","X-Received":"by 2002:a19:2391:: with SMTP id\n\tj139mr1078380lfj.147.1585355289135; \n\tFri, 27 Mar 2020 17:28:09 -0700 (PDT)","Date":"Sat, 28 Mar 2020 01:28:07 +0100","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200328002807.GA1869774@oden.dyn.berto.se>","References":"<20200326160819.4088361-1-niklas.soderlund@ragnatech.se>\n\t<20200326160819.4088361-5-niklas.soderlund@ragnatech.se>\n\t<20200326165259.GY20581@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20200326165259.GY20581@pendragon.ideasonboard.com>","Subject":"Re: [libcamera-devel] [RFCv2 4/7] libcamera: pipeline: rkisp1: Do\n\tnot try to process cancelled frames","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","X-List-Received-Date":"Sat, 28 Mar 2020 00:28:10 -0000"}}]