[{"id":3874,"web_url":"https://patchwork.libcamera.org/comment/3874/","msgid":"<20200229145929.GU18738@pendragon.ideasonboard.com>","date":"2020-02-29T14:59:29","subject":"Re: [libcamera-devel] [PATCH v2 23/27] gst: libcamerasrc: Implement\n\ttimestamp support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nThank you for the patch.\n\nOn Thu, Feb 27, 2020 at 03:04:03PM -0500, Nicolas Dufresne wrote:\n> This is an experimental patch adding timestamp support to the libcamerasrc\n> element. This patch currently assume that the driver timestamp are relative to\n> the system monotonic clock. Without a reference clock source, the timestamp are\n> otherwise unusable, and without timestamp only minor use case can be achieved.\n> \n> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> ---\n>  src/gstreamer/gstlibcamerapad.cpp | 23 +++++++++++++++++++++++\n>  src/gstreamer/gstlibcamerapad.h   |  2 ++\n>  src/gstreamer/gstlibcamerasrc.cpp | 20 ++++++++++++++++++++\n>  3 files changed, 45 insertions(+)\n> \n> diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp\n> index 2cf1630..31d3edd 100644\n> --- a/src/gstreamer/gstlibcamerapad.cpp\n> +++ b/src/gstreamer/gstlibcamerapad.cpp\n> @@ -62,9 +62,24 @@ gst_libcamera_pad_get_property(GObject *object, guint prop_id, GValue *value,\n>  \t}\n>  }\n>  \n> +static gboolean\n> +gst_libcamera_pad_query(GstPad *pad, GstObject *parent, GstQuery *query)\n> +{\n> +\tauto *self = GST_LIBCAMERA_PAD(pad);\n> +\n> +\tif (query->type != GST_QUERY_LATENCY)\n> +\t\treturn gst_pad_query_default(pad, parent, query);\n> +\n> +\t/* TRUE here means live, we assumes that max latency is the same as min\n> +\t * as we have no idea that duration of frames. */\n\n\t/*\n\t * ...\n\t */\n\n> +\tgst_query_set_latency(query, TRUE, self->latency, self->latency);\n> +\treturn TRUE;\n> +}\n> +\n>  static void\n>  gst_libcamera_pad_init(GstLibcameraPad *self)\n>  {\n> +\tGST_PAD_QUERYFUNC(self) = gst_libcamera_pad_query;\n>  }\n>  \n>  static GType\n> @@ -172,3 +187,11 @@ gst_libcamera_pad_has_pending(GstPad *pad)\n>  \tGLibLocker lock(GST_OBJECT(self));\n>  \treturn (self->pending_buffers.length > 0);\n>  }\n> +\n> +void\n> +gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency)\n> +{\n> +\tauto *self = GST_LIBCAMERA_PAD(pad);\n> +\tGLibLocker lock(GST_OBJECT(self));\n> +\tself->latency = latency;\n> +}\n> diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h\n> index eb24000..a3291e8 100644\n> --- a/src/gstreamer/gstlibcamerapad.h\n> +++ b/src/gstreamer/gstlibcamerapad.h\n> @@ -32,4 +32,6 @@ GstFlowReturn gst_libcamera_pad_push_pending(GstPad *pad);\n>  \n>  bool gst_libcamera_pad_has_pending(GstPad *pad);\n>  \n> +void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency);\n> +\n>  #endif /* __GST_LIBCAMERA_PAD_H__ */\n> diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> index 70f2048..73a1360 100644\n> --- a/src/gstreamer/gstlibcamerasrc.cpp\n> +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> @@ -151,6 +151,26 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n>  \tfor (GstPad *srcpad : this->srcpads) {\n>  \t\tStream *stream = gst_libcamera_pad_get_stream(srcpad);\n>  \t\tbuffer = wrap->detachBuffer(stream);\n> +\n> +\t\tFrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer);\n> +\n> +\t\tif (GST_ELEMENT_CLOCK(this->src)) {\n> +\t\t\tGstClockTime gst_base_time = GST_ELEMENT(this->src)->base_time;\n> +\t\t\tGstClockTime gst_now = gst_clock_get_time(GST_ELEMENT_CLOCK(this->src));\n> +\t\t\t/* \\todo Need to expose which reference clock the timestamp relates to. */\n\nSomething we need to do indeed. We're considering standardizing on\nCLOCK_BOOTTIME, would that be good for gstreamer ?\n\n> +\t\t\tGstClockTime sys_now = g_get_monotonic_time() * 1000;\n> +\n> +\t\t\t/* Deduced from: sys_now - sys_base_time == gst_now - gst_bae_time */\n\ns/gst_bae_time/gst_base_time/\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> +\t\t\tGstClockTime sys_base_time = sys_now - (gst_now - gst_base_time);\n> +\t\t\tGST_BUFFER_PTS(buffer) = fb->metadata().timestamp - sys_base_time;\n> +\t\t\tgst_libcamera_pad_set_latency(srcpad, sys_now - fb->metadata().timestamp);\n> +\t\t} else {\n> +\t\t\tGST_BUFFER_PTS(buffer) = 0;\n> +\t\t}\n> +\n> +\t\tGST_BUFFER_OFFSET(buffer) = fb->metadata().sequence;\n> +\t\tGST_BUFFER_OFFSET_END(buffer) = fb->metadata().sequence;\n> +\n>  \t\tgst_libcamera_pad_queue_buffer(srcpad, buffer);\n>  \t}\n>","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","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 9BC0562689\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 29 Feb 2020 15:59:53 +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 0289133E;\n\tSat, 29 Feb 2020 15:59:52 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1582988393;\n\tbh=6PPvzHg3XG54LrrAiF38Q0g/naARWbbSaeIjGkefDSg=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=Js+hiNnJhPKdTphK9ywoTadOAFRpEC6CH0hEoz7TkcASnY+sD6MOc/L4bPs9uYJ3x\n\tmSjoVepaeLQDSB02Y5FSsT56UXApLSYLq1+zxvJ3jH7a/D6u0Wyq/X2cUvF81w9d9V\n\tGpFE7vRoHtmRFkCPFWlwSqQ0l/blvvG5BGDGo/5g=","Date":"Sat, 29 Feb 2020 16:59:29 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200229145929.GU18738@pendragon.ideasonboard.com>","References":"<20200227200407.490616-1-nicolas.dufresne@collabora.com>\n\t<20200227200407.490616-24-nicolas.dufresne@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","In-Reply-To":"<20200227200407.490616-24-nicolas.dufresne@collabora.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 23/27] gst: libcamerasrc: Implement\n\ttimestamp support","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sat, 29 Feb 2020 14:59:53 -0000"}},{"id":3884,"web_url":"https://patchwork.libcamera.org/comment/3884/","msgid":"<4b3e94a4bb80f0b90461d454d207b3cf7e6ddc42.camel@collabora.com>","date":"2020-02-29T15:32:52","subject":"Re: [libcamera-devel] [PATCH v2 23/27] gst: libcamerasrc: Implement\n\ttimestamp support","submitter":{"id":31,"url":"https://patchwork.libcamera.org/api/people/31/","name":"Nicolas Dufresne","email":"nicolas.dufresne@collabora.com"},"content":"Le samedi 29 février 2020 à 16:59 +0200, Laurent Pinchart a écrit :\n> Hi Nicolas,\n> \n> Thank you for the patch.\n> \n> On Thu, Feb 27, 2020 at 03:04:03PM -0500, Nicolas Dufresne wrote:\n> > This is an experimental patch adding timestamp support to the libcamerasrc\n> > element. This patch currently assume that the driver timestamp are relative to\n> > the system monotonic clock. Without a reference clock source, the timestamp are\n> > otherwise unusable, and without timestamp only minor use case can be achieved.\n> > \n> > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > ---\n> >  src/gstreamer/gstlibcamerapad.cpp | 23 +++++++++++++++++++++++\n> >  src/gstreamer/gstlibcamerapad.h   |  2 ++\n> >  src/gstreamer/gstlibcamerasrc.cpp | 20 ++++++++++++++++++++\n> >  3 files changed, 45 insertions(+)\n> > \n> > diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp\n> > index 2cf1630..31d3edd 100644\n> > --- a/src/gstreamer/gstlibcamerapad.cpp\n> > +++ b/src/gstreamer/gstlibcamerapad.cpp\n> > @@ -62,9 +62,24 @@ gst_libcamera_pad_get_property(GObject *object, guint prop_id, GValue *value,\n> >  \t}\n> >  }\n> >  \n> > +static gboolean\n> > +gst_libcamera_pad_query(GstPad *pad, GstObject *parent, GstQuery *query)\n> > +{\n> > +\tauto *self = GST_LIBCAMERA_PAD(pad);\n> > +\n> > +\tif (query->type != GST_QUERY_LATENCY)\n> > +\t\treturn gst_pad_query_default(pad, parent, query);\n> > +\n> > +\t/* TRUE here means live, we assumes that max latency is the same as min\n> > +\t * as we have no idea that duration of frames. */\n> \n> \t/*\n> \t * ...\n> \t */\n> \n> > +\tgst_query_set_latency(query, TRUE, self->latency, self->latency);\n> > +\treturn TRUE;\n> > +}\n> > +\n> >  static void\n> >  gst_libcamera_pad_init(GstLibcameraPad *self)\n> >  {\n> > +\tGST_PAD_QUERYFUNC(self) = gst_libcamera_pad_query;\n> >  }\n> >  \n> >  static GType\n> > @@ -172,3 +187,11 @@ gst_libcamera_pad_has_pending(GstPad *pad)\n> >  \tGLibLocker lock(GST_OBJECT(self));\n> >  \treturn (self->pending_buffers.length > 0);\n> >  }\n> > +\n> > +void\n> > +gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency)\n> > +{\n> > +\tauto *self = GST_LIBCAMERA_PAD(pad);\n> > +\tGLibLocker lock(GST_OBJECT(self));\n> > +\tself->latency = latency;\n> > +}\n> > diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h\n> > index eb24000..a3291e8 100644\n> > --- a/src/gstreamer/gstlibcamerapad.h\n> > +++ b/src/gstreamer/gstlibcamerapad.h\n> > @@ -32,4 +32,6 @@ GstFlowReturn gst_libcamera_pad_push_pending(GstPad *pad);\n> >  \n> >  bool gst_libcamera_pad_has_pending(GstPad *pad);\n> >  \n> > +void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency);\n> > +\n> >  #endif /* __GST_LIBCAMERA_PAD_H__ */\n> > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > index 70f2048..73a1360 100644\n> > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > @@ -151,6 +151,26 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n> >  \tfor (GstPad *srcpad : this->srcpads) {\n> >  \t\tStream *stream = gst_libcamera_pad_get_stream(srcpad);\n> >  \t\tbuffer = wrap->detachBuffer(stream);\n> > +\n> > +\t\tFrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer);\n> > +\n> > +\t\tif (GST_ELEMENT_CLOCK(this->src)) {\n> > +\t\t\tGstClockTime gst_base_time = GST_ELEMENT(this->src)->base_time;\n> > +\t\t\tGstClockTime gst_now = gst_clock_get_time(GST_ELEMENT_CLOCK(this->src));\n> > +\t\t\t/* \\todo Need to expose which reference clock the timestamp relates to. */\n> \n> Something we need to do indeed. We're considering standardizing on\n> CLOCK_BOOTTIME, would that be good for gstreamer ?\n\nCan we do like DRM and just pass back the clock ID ? I've followed a\nlittle bit the discussion and haven't found enough rationale to show\nthat BOOTTIME is the right approach. It felt very usecase specific and\nto make things worst, drivers will have to support both for backward\ncompatibility reason.\n\nAs you can see, this code path is a userland translation from one clock\nto another. This the type of thing you want to avoid like the peste in\nfact, since it's really error prone due to scheduling latency. So\nforcing BOOTIME would imply that we'd have two of these translation\ngoing on per frame for V4L2 devices (at least in current state of v4l2\ndrivers).\n\nThat being said, I'll add a \\todo if someone want to do future\nenhancement, since we could try to use GstClock derivation mechanism.\nIt's a mechanism to which you pass samples of the match, and GstClock\nwill average and smooth a slope for derivation. Last time I tried, it\nwas a bit inconvenient to use and the result tend to be historic, hence\nthe reason I went for a KISS implementation. I would need to dig into\nthis 15 years old piece of code in GStreamer and find out what I'm\ndoing wrong, or what its doing wrong.\n\n> \n> > +\t\t\tGstClockTime sys_now = g_get_monotonic_time() * 1000;\n> > +\n> > +\t\t\t/* Deduced from: sys_now - sys_base_time == gst_now - gst_bae_time */\n> \n> s/gst_bae_time/gst_base_time/\n> \n> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> \n> > +\t\t\tGstClockTime sys_base_time = sys_now - (gst_now - gst_base_time);\n> > +\t\t\tGST_BUFFER_PTS(buffer) = fb->metadata().timestamp - sys_base_time;\n> > +\t\t\tgst_libcamera_pad_set_latency(srcpad, sys_now - fb->metadata().timestamp);\n> > +\t\t} else {\n> > +\t\t\tGST_BUFFER_PTS(buffer) = 0;\n> > +\t\t}\n> > +\n> > +\t\tGST_BUFFER_OFFSET(buffer) = fb->metadata().sequence;\n> > +\t\tGST_BUFFER_OFFSET_END(buffer) = fb->metadata().sequence;\n> > +\n> >  \t\tgst_libcamera_pad_queue_buffer(srcpad, buffer);\n> >  \t}\n> >","headers":{"Return-Path":"<nicolas.dufresne@collabora.com>","Received":["from bhuna.collabora.co.uk (bhuna.collabora.co.uk\n\t[IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id BEEFD62689\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tSat, 29 Feb 2020 16:32:55 +0100 (CET)","from [127.0.0.1] (localhost [127.0.0.1])\n\t(Authenticated sender: nicolas) with ESMTPSA id EBDB1263994"],"Message-ID":"<4b3e94a4bb80f0b90461d454d207b3cf7e6ddc42.camel@collabora.com>","From":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","Reply-To":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","To":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Date":"Sat, 29 Feb 2020 10:32:52 -0500","In-Reply-To":"<20200229145929.GU18738@pendragon.ideasonboard.com>","References":"<20200227200407.490616-1-nicolas.dufresne@collabora.com>\n\t<20200227200407.490616-24-nicolas.dufresne@collabora.com>\n\t<20200229145929.GU18738@pendragon.ideasonboard.com>","Organization":"Collabora","Content-Type":"multipart/signed; micalg=\"pgp-sha1\";\n\tprotocol=\"application/pgp-signature\"; \n\tboundary=\"=-lnkyXkcdLjORZj1Fl60g\"","User-Agent":"Evolution 3.34.4 (3.34.4-1.fc31) ","MIME-Version":"1.0","Subject":"Re: [libcamera-devel] [PATCH v2 23/27] gst: libcamerasrc: Implement\n\ttimestamp support","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Sat, 29 Feb 2020 15:32:56 -0000"}},{"id":3892,"web_url":"https://patchwork.libcamera.org/comment/3892/","msgid":"<20200302110152.GM11960@pendragon.ideasonboard.com>","date":"2020-03-02T11:01:52","subject":"Re: [libcamera-devel] [PATCH v2 23/27] gst: libcamerasrc: Implement\n\ttimestamp support","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Nicolas,\n\nOn Sat, Feb 29, 2020 at 10:32:52AM -0500, Nicolas Dufresne wrote:\n> Le samedi 29 février 2020 à 16:59 +0200, Laurent Pinchart a écrit :\n> > On Thu, Feb 27, 2020 at 03:04:03PM -0500, Nicolas Dufresne wrote:\n> > > This is an experimental patch adding timestamp support to the libcamerasrc\n> > > element. This patch currently assume that the driver timestamp are relative to\n> > > the system monotonic clock. Without a reference clock source, the timestamp are\n> > > otherwise unusable, and without timestamp only minor use case can be achieved.\n> > > \n> > > Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>\n> > > ---\n> > >  src/gstreamer/gstlibcamerapad.cpp | 23 +++++++++++++++++++++++\n> > >  src/gstreamer/gstlibcamerapad.h   |  2 ++\n> > >  src/gstreamer/gstlibcamerasrc.cpp | 20 ++++++++++++++++++++\n> > >  3 files changed, 45 insertions(+)\n> > > \n> > > diff --git a/src/gstreamer/gstlibcamerapad.cpp b/src/gstreamer/gstlibcamerapad.cpp\n> > > index 2cf1630..31d3edd 100644\n> > > --- a/src/gstreamer/gstlibcamerapad.cpp\n> > > +++ b/src/gstreamer/gstlibcamerapad.cpp\n> > > @@ -62,9 +62,24 @@ gst_libcamera_pad_get_property(GObject *object, guint prop_id, GValue *value,\n> > >  \t}\n> > >  }\n> > >  \n> > > +static gboolean\n> > > +gst_libcamera_pad_query(GstPad *pad, GstObject *parent, GstQuery *query)\n> > > +{\n> > > +\tauto *self = GST_LIBCAMERA_PAD(pad);\n> > > +\n> > > +\tif (query->type != GST_QUERY_LATENCY)\n> > > +\t\treturn gst_pad_query_default(pad, parent, query);\n> > > +\n> > > +\t/* TRUE here means live, we assumes that max latency is the same as min\n> > > +\t * as we have no idea that duration of frames. */\n> > \n> > \t/*\n> > \t * ...\n> > \t */\n> > \n> > > +\tgst_query_set_latency(query, TRUE, self->latency, self->latency);\n> > > +\treturn TRUE;\n> > > +}\n> > > +\n> > >  static void\n> > >  gst_libcamera_pad_init(GstLibcameraPad *self)\n> > >  {\n> > > +\tGST_PAD_QUERYFUNC(self) = gst_libcamera_pad_query;\n> > >  }\n> > >  \n> > >  static GType\n> > > @@ -172,3 +187,11 @@ gst_libcamera_pad_has_pending(GstPad *pad)\n> > >  \tGLibLocker lock(GST_OBJECT(self));\n> > >  \treturn (self->pending_buffers.length > 0);\n> > >  }\n> > > +\n> > > +void\n> > > +gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency)\n> > > +{\n> > > +\tauto *self = GST_LIBCAMERA_PAD(pad);\n> > > +\tGLibLocker lock(GST_OBJECT(self));\n> > > +\tself->latency = latency;\n> > > +}\n> > > diff --git a/src/gstreamer/gstlibcamerapad.h b/src/gstreamer/gstlibcamerapad.h\n> > > index eb24000..a3291e8 100644\n> > > --- a/src/gstreamer/gstlibcamerapad.h\n> > > +++ b/src/gstreamer/gstlibcamerapad.h\n> > > @@ -32,4 +32,6 @@ GstFlowReturn gst_libcamera_pad_push_pending(GstPad *pad);\n> > >  \n> > >  bool gst_libcamera_pad_has_pending(GstPad *pad);\n> > >  \n> > > +void gst_libcamera_pad_set_latency(GstPad *pad, GstClockTime latency);\n> > > +\n> > >  #endif /* __GST_LIBCAMERA_PAD_H__ */\n> > > diff --git a/src/gstreamer/gstlibcamerasrc.cpp b/src/gstreamer/gstlibcamerasrc.cpp\n> > > index 70f2048..73a1360 100644\n> > > --- a/src/gstreamer/gstlibcamerasrc.cpp\n> > > +++ b/src/gstreamer/gstlibcamerasrc.cpp\n> > > @@ -151,6 +151,26 @@ GstLibcameraSrcState::requestCompleted(Request *request)\n> > >  \tfor (GstPad *srcpad : this->srcpads) {\n> > >  \t\tStream *stream = gst_libcamera_pad_get_stream(srcpad);\n> > >  \t\tbuffer = wrap->detachBuffer(stream);\n> > > +\n> > > +\t\tFrameBuffer *fb = gst_libcamera_buffer_get_frame_buffer(buffer);\n> > > +\n> > > +\t\tif (GST_ELEMENT_CLOCK(this->src)) {\n> > > +\t\t\tGstClockTime gst_base_time = GST_ELEMENT(this->src)->base_time;\n> > > +\t\t\tGstClockTime gst_now = gst_clock_get_time(GST_ELEMENT_CLOCK(this->src));\n> > > +\t\t\t/* \\todo Need to expose which reference clock the timestamp relates to. */\n> > \n> > Something we need to do indeed. We're considering standardizing on\n> > CLOCK_BOOTTIME, would that be good for gstreamer ?\n> \n> Can we do like DRM and just pass back the clock ID ? I've followed a\n> little bit the discussion and haven't found enough rationale to show\n> that BOOTTIME is the right approach. It felt very usecase specific and\n> to make things worst, drivers will have to support both for backward\n> compatibility reason.\n> \n> As you can see, this code path is a userland translation from one clock\n> to another. This the type of thing you want to avoid like the peste in\n> fact, since it's really error prone due to scheduling latency. So\n> forcing BOOTIME would imply that we'd have two of these translation\n> going on per frame for V4L2 devices (at least in current state of v4l2\n> drivers).\n\nI was thinking of getting V4L2 to produce BOOTTIME timestamps :-)\n\nMy question was whether the BOOTTIME clock would be appropriate for\nGStreamer, or if there would be a clock that would be more suitable.\n\n> That being said, I'll add a \\todo if someone want to do future\n> enhancement, since we could try to use GstClock derivation mechanism.\n> It's a mechanism to which you pass samples of the match, and GstClock\n> will average and smooth a slope for derivation. Last time I tried, it\n> was a bit inconvenient to use and the result tend to be historic, hence\n> the reason I went for a KISS implementation. I would need to dig into\n> this 15 years old piece of code in GStreamer and find out what I'm\n> doing wrong, or what its doing wrong.\n\nThere are so many ways to get clock conversion wrong that I think it\nshould always be implemented in helpers, regardless of whether they're\nKISS or very complex.\n\n> > > +\t\t\tGstClockTime sys_now = g_get_monotonic_time() * 1000;\n> > > +\n> > > +\t\t\t/* Deduced from: sys_now - sys_base_time == gst_now - gst_bae_time */\n> > \n> > s/gst_bae_time/gst_base_time/\n> > \n> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n> > \n> > > +\t\t\tGstClockTime sys_base_time = sys_now - (gst_now - gst_base_time);\n> > > +\t\t\tGST_BUFFER_PTS(buffer) = fb->metadata().timestamp - sys_base_time;\n> > > +\t\t\tgst_libcamera_pad_set_latency(srcpad, sys_now - fb->metadata().timestamp);\n> > > +\t\t} else {\n> > > +\t\t\tGST_BUFFER_PTS(buffer) = 0;\n> > > +\t\t}\n> > > +\n> > > +\t\tGST_BUFFER_OFFSET(buffer) = fb->metadata().sequence;\n> > > +\t\tGST_BUFFER_OFFSET_END(buffer) = fb->metadata().sequence;\n> > > +\n> > >  \t\tgst_libcamera_pad_queue_buffer(srcpad, buffer);\n> > >  \t}\n> > >","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","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 40D9662710\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tMon,  2 Mar 2020 12:02:17 +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 A358D9D0;\n\tMon,  2 Mar 2020 12:02:16 +0100 (CET)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1583146936;\n\tbh=kEwjf7OIx/157CHKT9dV/unIP9RSKrMIWg4PKLFfCEA=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=mgylR1+ZCREW0rEWZ2b57KaUrNTIMFxK3Kn8+JTNicFbTBynGG1pQxf0jpTSUm5ka\n\tO38wxmPIItprjhWp9gMZRtP4Ljf3SX7iDjleAwoKXZFucfvc6rjPec4vuUjO+d9ail\n\tLIh8dNqjNPaXEKEcPeDdQA0R50dJRasfQsg6gHe8=","Date":"Mon, 2 Mar 2020 13:01:52 +0200","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Nicolas Dufresne <nicolas.dufresne@collabora.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20200302110152.GM11960@pendragon.ideasonboard.com>","References":"<20200227200407.490616-1-nicolas.dufresne@collabora.com>\n\t<20200227200407.490616-24-nicolas.dufresne@collabora.com>\n\t<20200229145929.GU18738@pendragon.ideasonboard.com>\n\t<4b3e94a4bb80f0b90461d454d207b3cf7e6ddc42.camel@collabora.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<4b3e94a4bb80f0b90461d454d207b3cf7e6ddc42.camel@collabora.com>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v2 23/27] gst: libcamerasrc: Implement\n\ttimestamp support","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.29","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 02 Mar 2020 11:02:17 -0000"}}]