[libcamera-devel] [PATCH 10/13] libcamera: raspberry: Report sensor timestamp

[Jacopo Mondi]

Hi Naush,
  thanks for review!

On Mon, Apr 19, 2021 at 02:45:29PM +0100, Naushir Patuck wrote:
> Hi Jacopo,
>
> Thank you for your work.
>
> On Mon, 19 Apr 2021 at 14:14, Jacopo Mondi <jacopo at jmondi.org> wrote:
>
> > Report the sensor's timestamp in the Request metadata by using the
> > Unicam::Image 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.
> >
> > Signed-off-by: Jacopo Mondi <jacopo at jmondi.org>
> > ---
> >  src/libcamera/pipeline/raspberrypi/raspberrypi.cpp | 12 ++++++++++++
> >  1 file changed, 12 insertions(+)
> >
> > diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > index d1902bfc3393..7dc92acf3c4f 100644
> > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> > @@ -1415,6 +1415,18 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer
> > *buffer)
> >                         << ", timestamp: " << buffer->metadata().timestamp;
> >
> >         if (stream == &unicam_[Unicam::Image]) {
> > +               /*
> > +                * Record the sensor timestamp in the Request.
> > +                *
> > +                * \todo The sensor timestamp should be better estimated by
> > +                * sampling the V4L2Device::frameStart signal.
> > +                */
> >
>
> Just a minor correction to this comment - the VB2 buffer timestamp that
> is provided by the device driver is actually sampled at the CSI2 frame start
> event, so is pretty much as close to the start of exposure as we can get
> without dead reckoning.  This sample is also before the DMA to memory
> and V4L2Device::frameStart signal.

Ah you see! That's great, I wrongly assumed the raw buffer timestamp
was sampled at DMA transfer time end, as it happens on IPU3. It's
great if I could drop that todo item!

>
> Other than that,
> Reviewed-by: Naushir Patuck <naush at raspberrypi.com>
>
>
>
> > +               Request *request = requestQueue_.front();
> > +               ASSERT(request);
> > +
> > +               request->metadata().set(controls::SensorTimestamp,
> > +                                       buffer->metadata().timestamp);
> > +
> >                 /*
> >                  * Lookup the sensor controls used for this frame sequence
> > from
> >                  * DelayedControl and queue them along with the frame
> > buffer.
> > --
> > 2.31.1
> >
> > _______________________________________________
> > libcamera-devel mailing list
> > libcamera-devel at lists.libcamera.org
> > https://lists.libcamera.org/listinfo/libcamera-devel
> >