[libcamera-devel] [PATCH v4 05/16] libcamera: ipu3: Report sensor timestamp

[Jacopo Mondi]

Hi Niklas,

On Sat, May 01, 2021 at 08:26:49AM +0200, Niklas Söderlund wrote:
> Hi Jacopo,
>
> Thanks for your work.
>
> On 2021-04-30 18:00:15 +0200, 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: Kieran Bingham <kieran.bingham at ideasonboard.com>
> > 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 73306cea6b37..88b7bd1e52c3 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.
>
> I still believe getting the timestamp from the buffer as done in this
> patch is much better then creating a timestamp in user-space at the
> frameStart signal.

Thing is the buffer is timestamped in kernel space at the end of the
DMA transfer in the CIO2 driver not at exposure begin time.

For sake of comparing frame durations I agree this is acceptable, but
contradicts the Control definition and this should be noted down imho.

>
> Reviewed-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
>
> > +	 */
> > +	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()) {
> > --
> > 2.31.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
>
> --
> Regards,
> Niklas Söderlund