[{"id":18850,"web_url":"https://patchwork.libcamera.org/comment/18850/","msgid":"<81553fbf-e97d-e7da-dba1-86ca1ab602aa@ideasonboard.com>","date":"2021-08-16T16:08:22","subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: ipu3: Skip recording\n\ttimestamp for cancelled buffers","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"On 16/08/2021 15:35, Umang Jain wrote:\n> There is no point in recording sensor's timestamp when V4L2VideoDevice\n> has marked the frame buffers with FrameMetadata::FrameCancelled\n> (happens when the streams are stopped). The cancelled buffers\n> handling block will proceed to cleanup and cause an early\n> return in cio2BufferReady() anyway and we are sure that at this\n> point, there is no useful purposes of setting the timestamp.\n\nThe metadata of a cancelled buffer is invalid, so setting them has no\nmeaning.\n\nReviewed-by: Kieran Bingham \n\n\n> Signed-off-by: Umang Jain \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 18 +++++++++---------\n> 1 file changed, 9 insertions(+), 9 deletions(-)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 6e26a7b7..1c4776be 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1318,15 +1318,6 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> \n> \tRequest *request = info->request;\n> \n> -\t/*\n> -\t * Record the sensor's timestamp in the request metadata.\n> -\t *\n> -\t * \\todo The sensor timestamp should be better estimated by connecting\n> -\t * to the V4L2Device::frameStart signal.\n> -\t */\n> -\trequest->metadata().set(controls::SensorTimestamp,\n> -\t\t\t\tbuffer->metadata().timestamp);\n> -\n> \t/* If the buffer is cancelled force a complete of the whole request. */\n> \tif (buffer->metadata().status == FrameMetadata::FrameCancelled) {\n> \t\tfor (auto it : request->buffers()) {\n> @@ -1340,6 +1331,15 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> \t\treturn;\n> \t}\n> \n> +\t/*\n> +\t * Record the sensor's timestamp in the request metadata.\n> +\t *\n> +\t * \\todo The sensor timestamp should be better estimated by connecting\n> +\t * to the V4L2Device::frameStart signal.\n> +\t */\n> +\trequest->metadata().set(controls::SensorTimestamp,\n> +\t\t\t\tbuffer->metadata().timestamp);\n> +\n> \tif (request->findBuffer(&rawStream_))\n> \t\tpipe_->completeBuffer(request, buffer);\n> \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 BCD0DBD87D\n\tfor ;\n\tMon, 16 Aug 2021 16:08:27 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 1E36B68893;\n\tMon, 16 Aug 2021 18:08:27 +0200 (CEST)","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 843416025D\n\tfor ;\n\tMon, 16 Aug 2021 18:08:25 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89244-aztw30-2-0-cust3082.18-1.cable.virginm.net [86.31.172.11])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id F08103E5;\n\tMon, 16 Aug 2021 18:08:24 +0200 (CEST)"],"Authentication-Results":"lancelot.ideasonboard.com; dkim=pass (1024-bit key;\n\tunprotected) header.d=ideasonboard.com header.i=@ideasonboard.com\n\theader.b=\"eZ8tzBnW\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629130105;\n\tbh=a9vicOPRtTZo9LvloT9riAfXRxVlBpbxuwp7SoOWJPU=;\n\th=To:References:From:Subject:Date:In-Reply-To:From;\n\tb=eZ8tzBnWFe+XoKiI2q3j2DfBbermgmxpFAHNVaG9/enY/Zq6WakIG+EJqOZtEmPZa\n\tCfaNypCottYvDsKAK7wGlRNb0jcTA3OAV3JVH5z5ZWx0Lcir4yMHNHPKuDfZDXFd3i\n\tIs4Hwsf6vdSsC8iuh4B6Ltm87Pf8CxgwrN1IEgLA=","To":"Umang Jain ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20210816143536.125939-1-umang.jain@ideasonboard.com>\n\t<20210816143536.125939-2-umang.jain@ideasonboard.com>","From":"Kieran Bingham ","Message-ID":"<81553fbf-e97d-e7da-dba1-86ca1ab602aa@ideasonboard.com>","Date":"Mon, 16 Aug 2021 17:08:22 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101\n\tThunderbird/78.11.0","MIME-Version":"1.0","In-Reply-To":"<20210816143536.125939-2-umang.jain@ideasonboard.com>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: ipu3: Skip recording\n\ttimestamp for cancelled buffers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}},{"id":18851,"web_url":"https://patchwork.libcamera.org/comment/18851/","msgid":"","date":"2021-08-16T16:22:25","subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: ipu3: Skip recording\n\ttimestamp for cancelled buffers","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hello,\n\nOn Mon, Aug 16, 2021 at 05:08:22PM +0100, Kieran Bingham wrote:\n> On 16/08/2021 15:35, Umang Jain wrote:\n> > There is no point in recording sensor's timestamp when V4L2VideoDevice\n> > has marked the frame buffers with FrameMetadata::FrameCancelled\n> > (happens when the streams are stopped). The cancelled buffers\n> > handling block will proceed to cleanup and cause an early\n> > return in cio2BufferReady() anyway and we are sure that at this\n> > point, there is no useful purposes of setting the timestamp.\n> \n> The metadata of a cancelled buffer is invalid, so setting them has no\n> meaning.\n\nThere's some metadata that could be valid (we could use metadata to\nreport failure causes for instance), but in this case, setting the\nsensor timestamp is clearly invalid. The V4L2 cancelled buffer will have\nno meaningful timestamp to start with.\n\n> Reviewed-by: Kieran Bingham \n\nReviewed-by: Laurent Pinchart \n\n> > Signed-off-by: Umang Jain \n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 18 +++++++++---------\n> > 1 file changed, 9 insertions(+), 9 deletions(-)\n> > \n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 6e26a7b7..1c4776be 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -1318,15 +1318,6 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> > \n> > \tRequest *request = info->request;\n> > \n> > -\t/*\n> > -\t * Record the sensor's timestamp in the request metadata.\n> > -\t *\n> > -\t * \\todo The sensor timestamp should be better estimated by connecting\n> > -\t * to the V4L2Device::frameStart signal.\n> > -\t */\n> > -\trequest->metadata().set(controls::SensorTimestamp,\n> > -\t\t\t\tbuffer->metadata().timestamp);\n> > -\n> > \t/* If the buffer is cancelled force a complete of the whole request. */\n> > \tif (buffer->metadata().status == FrameMetadata::FrameCancelled) {\n> > \t\tfor (auto it : request->buffers()) {\n> > @@ -1340,6 +1331,15 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)\n> > \t\treturn;\n> > \t}\n> > \n> > +\t/*\n> > +\t * Record the sensor's timestamp in the request metadata.\n> > +\t *\n> > +\t * \\todo The sensor timestamp should be better estimated by connecting\n> > +\t * to the V4L2Device::frameStart signal.\n> > +\t */\n> > +\trequest->metadata().set(controls::SensorTimestamp,\n> > +\t\t\t\tbuffer->metadata().timestamp);\n> > +\n> > \tif (request->findBuffer(&rawStream_))\n> > \t\tpipe_->completeBuffer(request, buffer);\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 707D3BD87C\n\tfor ;\n\tMon, 16 Aug 2021 16:22:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id CD68868893;\n\tMon, 16 Aug 2021 18:22:32 +0200 (CEST)","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 AB3636025D\n\tfor ;\n\tMon, 16 Aug 2021 18:22:31 +0200 (CEST)","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 1C17A3E5;\n\tMon, 16 Aug 2021 18:22:31 +0200 (CEST)"],"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=\"f8tDPvkV\"; dkim-atps=neutral","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1629130951;\n\tbh=7qj65L80eQGMwa+zRXMLiLHUrBBtRJ0IfERkA8k9chI=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=f8tDPvkV5yLRh8N9mudAtFWnzKngIW0y9AuP72kdoJONvfcc74QROU696hZvzrUa3\n\tdjmDZlon8xwpmm3+Cg/eS5D4WHLf8GgUXEdxWjaRB1edYn0t3kHzwFBB09g+zLZf1s\n\tyTlofaymc0+0HTyrIGuRvY1rhozbZt4d4g3ZrNqg=","Date":"Mon, 16 Aug 2021 19:22:25 +0300","From":"Laurent Pinchart ","To":"Kieran Bingham ","Message-ID":"","References":"<20210816143536.125939-1-umang.jain@ideasonboard.com>\n\t<20210816143536.125939-2-umang.jain@ideasonboard.com>\n\t<81553fbf-e97d-e7da-dba1-86ca1ab602aa@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<81553fbf-e97d-e7da-dba1-86ca1ab602aa@ideasonboard.com>","Subject":"Re: [libcamera-devel] [PATCH v2 2/2] pipeline: ipu3: Skip recording\n\ttimestamp for cancelled buffers","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","Cc":"libcamera-devel@lists.libcamera.org","Errors-To":"libcamera-devel-bounces@lists.libcamera.org","Sender":"\"libcamera-devel\" "}}]