[libcamera-devel] [PATCH 02/13] libcamera: ipu3: Report sensor timestamp

[Laurent Pinchart]

Hello,

On Mon, Apr 19, 2021 at 03:53:42PM +0200, Jacopo Mondi wrote:
> On Mon, Apr 19, 2021 at 02:35:27PM +0100, Kieran Bingham wrote:
> > On 19/04/2021 14:14, Jacopo Mondi wrote:
> > > Report the sensor's timestamp in the Request metadata by using the
> > > CIO2 buffer timestamp as an initial approximation.
> > >
> > > The buffer's timestamp is recorded at DMA-transfer time, and it does not
> > > theoretically matches the 'start of exposure' definition, but when used
> > > to compare two consecutive frames it gives an acceptable estimation of
> > > the sensor frame period duration.
> > >
> > > Reviewed-by: Hirokazu Honda <hiroh at chromium.org>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> > > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > > ---
> > >  src/libcamera/pipeline/ipu3/ipu3.cpp | 9 +++++++++
> > >  1 file changed, 9 insertions(+)
> > >
> > > diff --git a/src/libcamera/pipeline/ipu3/ipu3.cpp b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > index 51446fcf5bc1..28e849a43a3e 100644
> > > --- a/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > +++ b/src/libcamera/pipeline/ipu3/ipu3.cpp
> > > @@ -1255,6 +1255,15 @@ void IPU3CameraData::cio2BufferReady(FrameBuffer *buffer)
> > >
> > >  	Request *request = info->request;
> > >
> > > +	/*
> > > +	 * Record the sensor's timestamp in the request metadata.
> > > +	 *
> > > +	 * \todo The sensor timestamp should be better estimated by connecting
> > > +	 * to the V4L2Device::frameStart signal.
> >
> > Shouldn't that be handled by the sensor driver itself then?
> 
> I'm not aware of a mechanism to transport that information from the
> subdevice up, nor from what I've see the typical sensor driver has a
> mean to connect to that event and timestamp it.
> 
> We could rely on the receiver's SOF interrupt, from which usually the
> V4L2_EVENT_FRAME_SYNC is signaled to userspace

It's currently messy in V4L2. Drivers report timestamps in different
ways, and most of them don't match the specification. The specification
itself has some useful features (a driver can for instance opt to report
the SOF timestamp if it has access to that information, but whether the
driver does so or not can't be queried). I think we'll have to solve
this in V4L2 first.

> > What defines the temporal location/point of the timestamp that is
> > returned currently?
> 
> It depends on the platform. When the buffer whose timestamp is used to
> populate the SensorTimestamp property is the application facing buffer
> (uvc, simple, rkisp1) it is usually the time whene the EOF interrupt
> is triggered. Platforms with an off-line design where the raw frame is
> accessible to libcamera use the raw frametimestamp, which is for the
> CIO2 in example, the EOF interrupt time of the raw frame.
> 
> > I would have thought getting the time at the frameStart() signal would
> > be susceptible to more latency due to the signal handling and calling up
> > to userspace?
> 
> Probably yes, I cannot really quantify that at the moment..
> 
> > This must surely be something better handled by the kernel drivers right?
> 
> Buffers are timestamped by the kernel. Thing is, even when we get the
> raw frame timestamp it usually refers to the end of the DMA transfer,
> not the start of the exposure.

When it comes to what drivers should do, I'd say they should provide
enough information for userspace to obtain the most accurate timestamp,
but not more. We will need to implement clock recovery logic based on
statistics for some platforms, and that doesn't belong to kernelspace.

> > > +	 */
> > > +	request->metadata().set(controls::SensorTimestamp,
> > > +				buffer->metadata().timestamp);
> > > +
> > >  	/* If the buffer is cancelled force a complete of the whole request. */
> > >  	if (buffer->metadata().status == FrameMetadata::FrameCancelled) {
> > >  		for (auto it : request->buffers())

-- 
Regards,

Laurent Pinchart