[{"id":16709,"web_url":"https://patchwork.libcamera.org/comment/16709/","msgid":"","date":"2021-05-01T06:26:49","subject":"Re: [libcamera-devel] [PATCH v4 05/16] libcamera: ipu3: Report\n\tsensor timestamp","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 work.\n\nOn 2021-04-30 18:00:15 +0200, Jacopo Mondi wrote:\n> Report the sensor's timestamp in the Request metadata by using the\n> CIO2 buffer timestamp as an initial approximation.\n> \n> The buffer's timestamp is recorded at DMA-transfer time, and it does not\n> theoretically matches the 'start of exposure' definition, but when used\n> to compare two consecutive frames it gives an acceptable estimation of\n> the sensor frame period duration.\n> \n> Reviewed-by: Kieran Bingham \n> Reviewed-by: Hirokazu Honda \n> Reviewed-by: Laurent Pinchart \n> Signed-off-by: Jacopo Mondi \n> ---\n> src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++++++++\n> 1 file changed, 9 insertions(+)\n> \n> diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> index 73306cea6b37..88b7bd1e52c3 100644\n> --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> @@ -1255,6 +1255,15 @@ 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\nI still believe getting the timestamp from the buffer as done in this \npatch is much better then creating a timestamp in user-space at the \nframeStart signal.\n\nReviewed-by: Niklas Söderlund \n\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> -- \n> 2.31.1\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 795F0BDE6A\n\tfor ;\n\tSat, 1 May 2021 06:26:54 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id E9E4D60511;\n\tSat, 1 May 2021 08:26:53 +0200 (CEST)","from mail-lj1-x22e.google.com (mail-lj1-x22e.google.com\n\t[IPv6:2a00:1450:4864:20::22e])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CC829602BF\n\tfor ;\n\tSat, 1 May 2021 08:26:51 +0200 (CEST)","by mail-lj1-x22e.google.com with SMTP id s25so801751lji.0\n\tfor ;\n\tFri, 30 Apr 2021 23:26:51 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tt14sm490562lfp.6.2021.04.30.23.26.50\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tFri, 30 Apr 2021 23:26:50 -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=\"EcJGFE5l\"; 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=/e1xJTjNN7AHIOBhDoK8M3VdpegcPUiu/VHtbNSO0SI=;\n\tb=EcJGFE5lshfI1Fztd9KKQnX8Dx/BUpwM7KTQc+lSAFzAGxAJ0yz+IJ5Yoqz3GyOaCi\n\tH6HAVpmWszAS2zg6WI/ZOIq9QM1D/YkGtRVDxof1GGTQ11odxjtTXfnez80tvoC2bRO1\n\tEiBkjbX9Vjx8tUbgyGSSK4T5Xy1LSm+29hGpIS6PBFWtR1NAwQaK1Q4otuIr3GA511OE\n\tebdSjZnxpZLRNW0/Ckvz9+9UFxyamzef9F3PKakgtvmgeIhJZQ7zMoUi4HHTI+DJmmZ5\n\t0yKrfqWAWQNw04+L3fgRJe5hsha4JhyFr5PxuEdyejbb5UZFkflYprHZMvM9GBQKJkSn\n\tHTHQ==","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=/e1xJTjNN7AHIOBhDoK8M3VdpegcPUiu/VHtbNSO0SI=;\n\tb=iF0AxewVc2rYx/DOBxRYqgA6/pE2OYjb7HczTEqe74jwTk2GwHy4HVmO64qXSnWorG\n\tuwNC4ObE0Nwa6+gqKrPU84B596CMHaWKmy1QAGaZ3YZEU34YbywX4p3sBAXeASgIZSeh\n\tYCr16JriEah67soNqNi7AZEcrV8jnTaWbHZ8TnaJ4HaW464gM97VDxnYnPbGv0sART6T\n\tdR2bBWcNqlokEAq0e5lNignfCgrIbggu9GybPQjHihbx6VdHYxDAofEYJFB9wzWTOYiF\n\tK+eW6Oaqjevk8RVCkjjVpkiPlTf8fRDOoYzmb9R8EDNA+4qUzJqdaoZJDqO6Kw+QQJe+\n\tryjw==","X-Gm-Message-State":"AOAM533XBH7vl7keZJjxS8Vhyf0X+/Mck81q3M6L3+6KOps7PzfD3vC+\n\tB0l1Ce6QBDPUcGy6U8awg3RBOKTPBxntyw==","X-Google-Smtp-Source":"ABdhPJwTR5U/8ksp3NJdkpIuCDO7wAkOYmIa5rx+K/cxPIcAlgvwQzbA/1/d+JNayFMjtuAHgN6RzQ==","X-Received":"by 2002:a2e:b6c7:: with SMTP id m7mr5931308ljo.241.1619850411236;\n\tFri, 30 Apr 2021 23:26:51 -0700 (PDT)","Date":"Sat, 1 May 2021 08:26:49 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210430160026.190724-1-jacopo@jmondi.org>\n\t<20210430160026.190724-6-jacopo@jmondi.org>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210430160026.190724-6-jacopo@jmondi.org>","Subject":"Re: [libcamera-devel] [PATCH v4 05/16] libcamera: ipu3: Report\n\tsensor timestamp","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":16717,"web_url":"https://patchwork.libcamera.org/comment/16717/","msgid":"<20210501075103.gsdltulk6rqycvic@uno.localdomain>","date":"2021-05-01T07:51:03","subject":"Re: [libcamera-devel] [PATCH v4 05/16] libcamera: ipu3: Report\n\tsensor timestamp","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Sat, May 01, 2021 at 08:26:49AM +0200, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> Thanks for your work.\n>\n> On 2021-04-30 18:00:15 +0200, Jacopo Mondi wrote:\n> > Report the sensor's timestamp in the Request metadata by using the\n> > CIO2 buffer timestamp as an initial approximation.\n> >\n> > The buffer's timestamp is recorded at DMA-transfer time, and it does not\n> > theoretically matches the 'start of exposure' definition, but when used\n> > to compare two consecutive frames it gives an acceptable estimation of\n> > the sensor frame period duration.\n> >\n> > Reviewed-by: Kieran Bingham \n> > Reviewed-by: Hirokazu Honda \n> > Reviewed-by: Laurent Pinchart \n> > Signed-off-by: Jacopo Mondi \n> > ---\n> > src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++++++++\n> > 1 file changed, 9 insertions(+)\n> >\n> > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > index 73306cea6b37..88b7bd1e52c3 100644\n> > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > @@ -1255,6 +1255,15 @@ 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>\n> I still believe getting the timestamp from the buffer as done in this\n> patch is much better then creating a timestamp in user-space at the\n> frameStart signal.\n\nThing is the buffer is timestamped in kernel space at the end of the\nDMA transfer in the CIO2 driver not at exposure begin time.\n\nFor sake of comparing frame durations I agree this is acceptable, but\ncontradicts the Control definition and this should be noted down imho.\n\n>\n> Reviewed-by: Niklas Söderlund \n>\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> > --\n> > 2.31.1\n> >\n> > _______________________________________________\n> > libcamera-devel mailing list\n> > libcamera-devel@lists.libcamera.org\n> > https://lists.libcamera.org/listinfo/libcamera-devel\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 59C0CBDE6C\n\tfor ;\n\tSat, 1 May 2021 07:50:24 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id BA122688CA;\n\tSat, 1 May 2021 09:50:23 +0200 (CEST)","from relay9-d.mail.gandi.net (relay9-d.mail.gandi.net\n\t[217.70.183.199])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 1D65C688AE\n\tfor ;\n\tSat, 1 May 2021 09:50:22 +0200 (CEST)","from uno.localdomain (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay9-d.mail.gandi.net (Postfix) with ESMTPSA id 78496FF803;\n\tSat, 1 May 2021 07:50:20 +0000 (UTC)"],"X-Originating-IP":"93.61.96.190","Date":"Sat, 1 May 2021 09:51:03 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20210501075103.gsdltulk6rqycvic@uno.localdomain>","References":"<20210430160026.190724-1-jacopo@jmondi.org>\n\t<20210430160026.190724-6-jacopo@jmondi.org>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 05/16] libcamera: ipu3: Report\n\tsensor timestamp","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":16721,"web_url":"https://patchwork.libcamera.org/comment/16721/","msgid":"","date":"2021-05-01T08:05:56","subject":"Re: [libcamera-devel] [PATCH v4 05/16] libcamera: ipu3: Report\n\tsensor timestamp","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 2021-05-01 09:51:03 +0200, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Sat, May 01, 2021 at 08:26:49AM +0200, Niklas Söderlund wrote:\n> > Hi Jacopo,\n> >\n> > Thanks for your work.\n> >\n> > On 2021-04-30 18:00:15 +0200, Jacopo Mondi wrote:\n> > > Report the sensor's timestamp in the Request metadata by using the\n> > > CIO2 buffer timestamp as an initial approximation.\n> > >\n> > > The buffer's timestamp is recorded at DMA-transfer time, and it does not\n> > > theoretically matches the 'start of exposure' definition, but when used\n> > > to compare two consecutive frames it gives an acceptable estimation of\n> > > the sensor frame period duration.\n> > >\n> > > Reviewed-by: Kieran Bingham \n> > > Reviewed-by: Hirokazu Honda \n> > > Reviewed-by: Laurent Pinchart \n> > > Signed-off-by: Jacopo Mondi \n> > > ---\n> > > src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++++++++\n> > > 1 file changed, 9 insertions(+)\n> > >\n> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > index 73306cea6b37..88b7bd1e52c3 100644\n> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > @@ -1255,6 +1255,15 @@ 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> >\n> > I still believe getting the timestamp from the buffer as done in this\n> > patch is much better then creating a timestamp in user-space at the\n> > frameStart signal.\n> \n> Thing is the buffer is timestamped in kernel space at the end of the\n> DMA transfer in the CIO2 driver not at exposure begin time.\n\nMy worry is that the jitter in user-space will render the timestamp \nunreliable and I don't see it as unreasonable that a timestamp created \nin user-space as a result of a SOE V4L2 event will be later then a \ntimestamp from the kernels end of DMA.\n\nIf the V4L2 SOE event where to be augmented with a timestamp I agree \nthat would be better then a timestamp from end of DMA.\n\n> \n> For sake of comparing frame durations I agree this is acceptable, but\n> contradicts the Control definition and this should be noted down imho.\n\nCreating a timestamp in user-space for the SOE event also contradicts \nthe Control definition. IMHO end of DMA and user-space ts are equally \nwrong with regard to the Control, but end of DMA is at least more \ndeterministic. In a pinch 'end of DMA' - 'exposure time' would be a \nbetter estimate.\n\n> \n> >\n> > Reviewed-by: Niklas Söderlund \n> >\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> > > --\n> > > 2.31.1\n> > >\n> > > _______________________________________________\n> > > libcamera-devel mailing list\n> > > libcamera-devel@lists.libcamera.org\n> > > https://lists.libcamera.org/listinfo/libcamera-devel\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 13B30BDE6C\n\tfor ;\n\tSat, 1 May 2021 08:06:01 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 91667688CA;\n\tSat, 1 May 2021 10:06:00 +0200 (CEST)","from mail-lf1-x136.google.com (mail-lf1-x136.google.com\n\t[IPv6:2a00:1450:4864:20::136])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 7B46F688AE\n\tfor ;\n\tSat, 1 May 2021 10:05:58 +0200 (CEST)","by mail-lf1-x136.google.com with SMTP id 12so569753lfq.13\n\tfor ;\n\tSat, 01 May 2021 01:05:58 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\th9sm506674lfu.305.2021.05.01.01.05.57\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 01 May 2021 01:05:57 -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=\"18YXgb0I\"; 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=OcXyTM3r3Oiam1VAx7j24HRWwxtTBBDVszssVa9eIXw=;\n\tb=18YXgb0INY2X5Q4LwH4UIJlF5HwliYVv+OVPLwkXKcRmelc5V5nmm7KWuhVuL6DfOp\n\tcC3XjUZOvs44gM8tUpiGrqo6rLoxdwejm0MQno9oQ0pWQAnimoA9gRScXXMwYQ62TLei\n\tNFKhF70wCZh/sUxirFJOKDt3+I4lLc3JOxn8RUhTZMk/LrrD2wZ5ra17xUMjyhSWknI9\n\tIdrBCnuHM2J2hTXoV90tEMXD4StcI+5XUDLVpKBn9RL/GOwPB+0l4EPbo8JNPqs5R/Ic\n\tcX8Ktf29ib1efA4MieUw/29rnPZ0i/0swPjgDJXxOPNDp3Tun4opp/eRHrm+W4ptauOq\n\t6JnQ==","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=OcXyTM3r3Oiam1VAx7j24HRWwxtTBBDVszssVa9eIXw=;\n\tb=W5E0hgVaYRYf289fd2UdXQ+2nfdLWH6UU9n4uN0lsvgj4wejVm4R3Ygld/xjiZQV4r\n\t01Egod5Xt6YXJ4d4HYVAlvzpjkkWggKBxmpVNfLXwbpvxUW2f8WOi4/IoX5bOV1+uQo2\n\ttgpdRxuFB1Ykm3IS+jV4UDsAgdYJMz3Dui8CUOFk7symqL38GYwKXpovDCAdBZPm+K1J\n\twA+KJuA10SKr0kQRWqf05koZgL6EdxnBkNof/GjsmKW4yeXg78qEYXNIpcDGVn3+T+j2\n\t44haQRhprpiWdrnwKvWP78ngcB4JYqY5VY7spSOU0fIpLNEItht0amgVY90nWNqZbV8x\n\tG/ng==","X-Gm-Message-State":"AOAM530TloGkM1hHk5cwQ43MYGe9LokG4PqMOWMYoGz17u56o3qnLT4L\n\tToFcBKMXWqzVgMBFb5vP/2ec6w==","X-Google-Smtp-Source":"ABdhPJzkcdGYfJ1gxfWlUktzj0NPBD/joo45Qb90ngGpD9KWXXbUM1YMEQWQ9VvqIJH0ZMjDnc2oJw==","X-Received":"by 2002:a19:7503:: with SMTP id y3mr6047466lfe.476.1619856357941;\n\tSat, 01 May 2021 01:05:57 -0700 (PDT)","Date":"Sat, 1 May 2021 10:05:56 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210430160026.190724-1-jacopo@jmondi.org>\n\t<20210430160026.190724-6-jacopo@jmondi.org>\n\t\n\t<20210501075103.gsdltulk6rqycvic@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210501075103.gsdltulk6rqycvic@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v4 05/16] libcamera: ipu3: Report\n\tsensor timestamp","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":16723,"web_url":"https://patchwork.libcamera.org/comment/16723/","msgid":"<20210501082913.oz6q7bjezdblow4y@uno.localdomain>","date":"2021-05-01T08:29:13","subject":"Re: [libcamera-devel] [PATCH v4 05/16] libcamera: ipu3: Report\n\tsensor timestamp","submitter":{"id":3,"url":"https://patchwork.libcamera.org/api/people/3/","name":"Jacopo Mondi","email":"jacopo@jmondi.org"},"content":"Hi Niklas,\n\nOn Sat, May 01, 2021 at 10:05:56AM +0200, Niklas Söderlund wrote:\n> Hi Jacopo,\n>\n> On 2021-05-01 09:51:03 +0200, Jacopo Mondi wrote:\n> > Hi Niklas,\n> >\n> > On Sat, May 01, 2021 at 08:26:49AM +0200, Niklas Söderlund wrote:\n> > > Hi Jacopo,\n> > >\n> > > Thanks for your work.\n> > >\n> > > On 2021-04-30 18:00:15 +0200, Jacopo Mondi wrote:\n> > > > Report the sensor's timestamp in the Request metadata by using the\n> > > > CIO2 buffer timestamp as an initial approximation.\n> > > >\n> > > > The buffer's timestamp is recorded at DMA-transfer time, and it does not\n> > > > theoretically matches the 'start of exposure' definition, but when used\n> > > > to compare two consecutive frames it gives an acceptable estimation of\n> > > > the sensor frame period duration.\n> > > >\n> > > > Reviewed-by: Kieran Bingham \n> > > > Reviewed-by: Hirokazu Honda \n> > > > Reviewed-by: Laurent Pinchart \n> > > > Signed-off-by: Jacopo Mondi \n> > > > ---\n> > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++++++++\n> > > > 1 file changed, 9 insertions(+)\n> > > >\n> > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > index 73306cea6b37..88b7bd1e52c3 100644\n> > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > @@ -1255,6 +1255,15 @@ 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> > >\n> > > I still believe getting the timestamp from the buffer as done in this\n> > > patch is much better then creating a timestamp in user-space at the\n> > > frameStart signal.\n> >\n> > Thing is the buffer is timestamped in kernel space at the end of the\n> > DMA transfer in the CIO2 driver not at exposure begin time.\n>\n> My worry is that the jitter in user-space will render the timestamp\n> unreliable and I don't see it as unreasonable that a timestamp created\n> in user-space as a result of a SOE V4L2 event will be later then a\n> timestamp from the kernels end of DMA.\n\nA full frame later ? 16ms @ 60FPS of jitter ? Are we planning to run\nlibcamera on an 8086 :) ?\n\n>\n> If the V4L2 SOE event where to be augmented with a timestamp I agree\n> that would be better then a timestamp from end of DMA.\n>\n> >\n> > For sake of comparing frame durations I agree this is acceptable, but\n> > contradicts the Control definition and this should be noted down imho.\n>\n> Creating a timestamp in user-space for the SOE event also contradicts\n> the Control definition. IMHO end of DMA and user-space ts are equally\n> wrong with regard to the Control, but end of DMA is at least more\n> deterministic. In a pinch 'end of DMA' - 'exposure time' would be a\n> better estimate.\n\nThe exposure might well be smaller than the frame duration.\n\nI agree that having a timestamp in the event would be best and that\ntimestamping in userspace is subject to variables out of our controls\n(system load being the easiest one to mention).\n\nTbh I don't mind the \\todo, it remember we should do better than this,\nbut possibly doing that in the library is not the best way forward I\nagree.\n\n>\n> >\n> > >\n> > > Reviewed-by: Niklas Söderlund \n> > >\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> > > > --\n> > > > 2.31.1\n> > > >\n> > > > _______________________________________________\n> > > > libcamera-devel mailing list\n> > > > libcamera-devel@lists.libcamera.org\n> > > > https://lists.libcamera.org/listinfo/libcamera-devel\n> > >\n> > > --\n> > > Regards,\n> > > Niklas Söderlund\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 ACBA7BDE6A\n\tfor ;\n\tSat, 1 May 2021 08:28:33 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 3C4B9688CA;\n\tSat, 1 May 2021 10:28:33 +0200 (CEST)","from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net\n\t[217.70.183.194])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 6357A688AE\n\tfor ;\n\tSat, 1 May 2021 10:28:31 +0200 (CEST)","from uno.localdomain (93-61-96-190.ip145.fastwebnet.it\n\t[93.61.96.190]) (Authenticated sender: jacopo@jmondi.org)\n\tby relay2-d.mail.gandi.net (Postfix) with ESMTPSA id D148840002;\n\tSat, 1 May 2021 08:28:30 +0000 (UTC)"],"X-Originating-IP":"93.61.96.190","Date":"Sat, 1 May 2021 10:29:13 +0200","From":"Jacopo Mondi ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Message-ID":"<20210501082913.oz6q7bjezdblow4y@uno.localdomain>","References":"<20210430160026.190724-1-jacopo@jmondi.org>\n\t<20210430160026.190724-6-jacopo@jmondi.org>\n\t\n\t<20210501075103.gsdltulk6rqycvic@uno.localdomain>\n\t","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"","Subject":"Re: [libcamera-devel] [PATCH v4 05/16] libcamera: ipu3: Report\n\tsensor timestamp","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":16724,"web_url":"https://patchwork.libcamera.org/comment/16724/","msgid":"","date":"2021-05-01T09:02:56","subject":"Re: [libcamera-devel] [PATCH v4 05/16] libcamera: ipu3: Report\n\tsensor timestamp","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 2021-05-01 10:29:13 +0200, Jacopo Mondi wrote:\n> Hi Niklas,\n> \n> On Sat, May 01, 2021 at 10:05:56AM +0200, Niklas Söderlund wrote:\n> > Hi Jacopo,\n> >\n> > On 2021-05-01 09:51:03 +0200, Jacopo Mondi wrote:\n> > > Hi Niklas,\n> > >\n> > > On Sat, May 01, 2021 at 08:26:49AM +0200, Niklas Söderlund wrote:\n> > > > Hi Jacopo,\n> > > >\n> > > > Thanks for your work.\n> > > >\n> > > > On 2021-04-30 18:00:15 +0200, Jacopo Mondi wrote:\n> > > > > Report the sensor's timestamp in the Request metadata by using the\n> > > > > CIO2 buffer timestamp as an initial approximation.\n> > > > >\n> > > > > The buffer's timestamp is recorded at DMA-transfer time, and it does not\n> > > > > theoretically matches the 'start of exposure' definition, but when used\n> > > > > to compare two consecutive frames it gives an acceptable estimation of\n> > > > > the sensor frame period duration.\n> > > > >\n> > > > > Reviewed-by: Kieran Bingham \n> > > > > Reviewed-by: Hirokazu Honda \n> > > > > Reviewed-by: Laurent Pinchart \n> > > > > Signed-off-by: Jacopo Mondi \n> > > > > ---\n> > > > > src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++++++++\n> > > > > 1 file changed, 9 insertions(+)\n> > > > >\n> > > > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > > index 73306cea6b37..88b7bd1e52c3 100644\n> > > > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp\n> > > > > @@ -1255,6 +1255,15 @@ 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> > > >\n> > > > I still believe getting the timestamp from the buffer as done in this\n> > > > patch is much better then creating a timestamp in user-space at the\n> > > > frameStart signal.\n> > >\n> > > Thing is the buffer is timestamped in kernel space at the end of the\n> > > DMA transfer in the CIO2 driver not at exposure begin time.\n> >\n> > My worry is that the jitter in user-space will render the timestamp\n> > unreliable and I don't see it as unreasonable that a timestamp created\n> > in user-space as a result of a SOE V4L2 event will be later then a\n> > timestamp from the kernels end of DMA.\n> \n> A full frame later ? 16ms @ 60FPS of jitter ?\n\nNot a full frame later. My point was that the ts from user-space from a \nSOE might be larger then the ts in the CIO2 buffer due to jitter. All \nthat is needed for that to happen is that the user-space jitter is \nlarger then the difference from SOE to end of DMA right?\n\n> Are we planning to run libcamera on an 8086 :) ?\n\nI have a 8086 prototype board from way back when, it however needs the \nprogram to be inputted using a hexadecimal keypad and I'm not sure I \nwill manage to fit musl in there...\n\n> \n> >\n> > If the V4L2 SOE event where to be augmented with a timestamp I agree\n> > that would be better then a timestamp from end of DMA.\n> >\n> > >\n> > > For sake of comparing frame durations I agree this is acceptable, but\n> > > contradicts the Control definition and this should be noted down imho.\n> >\n> > Creating a timestamp in user-space for the SOE event also contradicts\n> > the Control definition. IMHO end of DMA and user-space ts are equally\n> > wrong with regard to the Control, but end of DMA is at least more\n> > deterministic. In a pinch 'end of DMA' - 'exposure time' would be a\n> > better estimate.\n> \n> The exposure might well be smaller than the frame duration.\n\nMy bad, yes it's probably better to subtract the frame duration then the \nexpose time,\n\n> \n> I agree that having a timestamp in the event would be best and that\n> timestamping in userspace is subject to variables out of our controls\n> (system load being the easiest one to mention).\n> \n> Tbh I don't mind the \\todo, it remember we should do better than this,\n> but possibly doing that in the library is not the best way forward I\n> agree.\n\nI agree that todo is highlighting that we could do better and I don't \nmind that. I think that hinting that the V4L2Device::frameStart is ready \nto be the next step is wrong as without a kernel change to add a ts I \nthink it would make things worse. But we are arguing about future \nchanges so if you feel good about the todo you already have my tag and \nwe can instead argue when we try to address the todo item :-)\n\n> \n> >\n> > >\n> > > >\n> > > > Reviewed-by: Niklas Söderlund \n> > > >\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> > > > > --\n> > > > > 2.31.1\n> > > > >\n> > > > > _______________________________________________\n> > > > > libcamera-devel mailing list\n> > > > > libcamera-devel@lists.libcamera.org\n> > > > > https://lists.libcamera.org/listinfo/libcamera-devel\n> > > >\n> > > > --\n> > > > Regards,\n> > > > Niklas Söderlund\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 AF617BDE6A\n\tfor ;\n\tSat, 1 May 2021 09:02:59 +0000 (UTC)","from lancelot.ideasonboard.com (localhost [IPv6:::1])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTP id 08F2F68901;\n\tSat, 1 May 2021 11:02:59 +0200 (CEST)","from mail-lf1-x134.google.com (mail-lf1-x134.google.com\n\t[IPv6:2a00:1450:4864:20::134])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 02510688AE\n\tfor ;\n\tSat, 1 May 2021 11:02:57 +0200 (CEST)","by mail-lf1-x134.google.com with SMTP id 4so694249lfp.11\n\tfor ;\n\tSat, 01 May 2021 02:02:57 -0700 (PDT)","from localhost (h-209-203.A463.priv.bahnhof.se. [155.4.209.203])\n\tby smtp.gmail.com with ESMTPSA id\n\tk8sm28993lfo.123.2021.05.01.02.02.56\n\t(version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256);\n\tSat, 01 May 2021 02:02:56 -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=\"Mc5VGpI1\"; 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=P9uoSJk6RfXFhcJWCfkCJ+2Swcgb/KgOJkEv0WSsKuU=;\n\tb=Mc5VGpI1NlXJvhyLXQY4urVSt1IOPDSZIpmuuUdbtIXKXPjejupaA6z+zZTWfCIvGT\n\tEq2AesGZU09dJnPSGiye1idXVxJHvqBKq3XKb1w0ZK2PedoXTXFgchw8+5Nz4Uw+KRGZ\n\tIXb1dKtW+LlNlnrtgt6a2FhBG5d0hCfp732CisZArxmY4H8pexF8VlbwFSEV7Hjujjua\n\tRKW0x8DvNG5cYaXvq64LVsO9D6OWRf8TaOFxCWfu2KRn/1JhL0BHTpPnMV85eRRsLiHq\n\tvOxdzWMWg5+/2qYOOo0wCGpCd09UyqrdT1H95gXkTN0HLO0riErHbcYQ5SZJnrXBzM69\n\toeAg==","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=P9uoSJk6RfXFhcJWCfkCJ+2Swcgb/KgOJkEv0WSsKuU=;\n\tb=rX+FPZ+wBos3ZiStgSCiPF/WDcjBmuquBLX/p7WGMwDEkkVpFqWnkV0gl6lK2fCkp+\n\tXYsTCeKx4NBS0OPmU0HQuW+H995Is/D6pGF1DU6QyKwENu/GjcNkO+30CGldckXJOO0g\n\tl6W7odrKilT2y18SguDfLkE//FutiS9mRiX59HuJrQzDDqABANFGFKfLMNOke/2eYLHq\n\tEgEvyLtI2Gtar23IqCiNteBId/sSypit/3fzmtAz0sl0Rzzl/07v4ILxjwgN0MalsG6L\n\tPQcYVC+ZpEJlRWBeVx54kpqbREKxEol+HM9DJ2uc6y4GBYlBzEtjxZ3aaOo7DKZokqOn\n\trqbg==","X-Gm-Message-State":"AOAM530EyqKgMWjsbWRpDjCCpWokn4acQODDcJOXuTHaA7okEx2/EkHZ\n\tTF0Pe8yHO1n9jzgO+1F7mgkrXw==","X-Google-Smtp-Source":"ABdhPJxRgGzBwmzh3PeKwqVoeePRVQLQJn/veLHOoVXSU2FoN3jrAHlP1eIqGj+XVu3u0GWYmetUqQ==","X-Received":"by 2002:a05:6512:b0f:: with SMTP id\n\tw15mr6468384lfu.333.1619859777257; \n\tSat, 01 May 2021 02:02:57 -0700 (PDT)","Date":"Sat, 1 May 2021 11:02:56 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Jacopo Mondi ","Message-ID":"","References":"<20210430160026.190724-1-jacopo@jmondi.org>\n\t<20210430160026.190724-6-jacopo@jmondi.org>\n\t\n\t<20210501075103.gsdltulk6rqycvic@uno.localdomain>\n\t\n\t<20210501082913.oz6q7bjezdblow4y@uno.localdomain>","MIME-Version":"1.0","Content-Disposition":"inline","In-Reply-To":"<20210501082913.oz6q7bjezdblow4y@uno.localdomain>","Subject":"Re: [libcamera-devel] [PATCH v4 05/16] libcamera: ipu3: Report\n\tsensor timestamp","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\" "}}]