[libcamera-devel,06/13] libcamera: simple: Report sensor timestamp

11988 diff mbox series

Report the sensor's timestamp in the Request metadata using the
completed buffer timestamp.

The buffer's timestamp is recorded at DMA-transfer time, and it does not
theoretically matches the 'start of exposure' definition. Record this with
a \todo entry.

Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
---
 src/libcamera/pipeline/simple/simple.cpp | 11 +++++++++++
 1 file changed, 11 insertions(+)

Hi Jacopo,

Thank you for the patch.

On Mon, Apr 19, 2021 at 03:14:26PM +0200, Jacopo Mondi wrote:
> Report the sensor's timestamp in the Request metadata using the
> completed buffer timestamp.
> 
> The buffer's timestamp is recorded at DMA-transfer time, and it does not
> theoretically matches the 'start of exposure' definition. Record this with
> a \todo entry.
> 
> Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  src/libcamera/pipeline/simple/simple.cpp | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> index f6095d38e97a..149bd91f1956 100644
> --- a/src/libcamera/pipeline/simple/simple.cpp
> +++ b/src/libcamera/pipeline/simple/simple.cpp
> @@ -22,6 +22,7 @@
>  #include <linux/media-bus-format.h>
>  
>  #include <libcamera/camera.h>
> +#include <libcamera/control_ids.h>
>  #include <libcamera/request.h>
>  #include <libcamera/stream.h>
>  
> @@ -1134,6 +1135,16 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)
>  
>  	/* Otherwise simply complete the request. */
>  	Request *request = buffer->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 if the platform provides it.
> +	 */
> +	request->metadata().set(controls::SensorTimestamp,
> +				buffer->metadata().timestamp);
> +
>  	completeBuffer(request, buffer);
>  	completeRequest(request);
>  }

Actually...

On Wed, Apr 21, 2021 at 12:46:43AM +0300, Laurent Pinchart wrote:
> Hi Jacopo,
> 
> Thank you for the patch.
> 
> On Mon, Apr 19, 2021 at 03:14:26PM +0200, Jacopo Mondi wrote:
> > Report the sensor's timestamp in the Request metadata using the
> > completed buffer timestamp.
> > 
> > The buffer's timestamp is recorded at DMA-transfer time, and it does not
> > theoretically matches the 'start of exposure' definition. Record this with
> > a \todo entry.
> > 
> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org>
> 
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 
> > ---
> >  src/libcamera/pipeline/simple/simple.cpp | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> > 
> > diff --git a/src/libcamera/pipeline/simple/simple.cpp b/src/libcamera/pipeline/simple/simple.cpp
> > index f6095d38e97a..149bd91f1956 100644
> > --- a/src/libcamera/pipeline/simple/simple.cpp
> > +++ b/src/libcamera/pipeline/simple/simple.cpp
> > @@ -22,6 +22,7 @@
> >  #include <linux/media-bus-format.h>
> >  
> >  #include <libcamera/camera.h>
> > +#include <libcamera/control_ids.h>
> >  #include <libcamera/request.h>
> >  #include <libcamera/stream.h>
> >  
> > @@ -1134,6 +1135,16 @@ void SimplePipelineHandler::bufferReady(FrameBuffer *buffer)
> >  
> >  	/* Otherwise simply complete the request. */
> >  	Request *request = buffer->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 if the platform provides it.
> > +	 */
> > +	request->metadata().set(controls::SensorTimestamp,
> > +				buffer->metadata().timestamp);
> > +

This will not run if a converter (similar to an offline ISP) is in use.
You should move this block higher up, either before the FrameSuccess
test, or after it. Do we want to fill the timestamp if an error occurs ?

> >  	completeBuffer(request, buffer);
> >  	completeRequest(request);
> >  }