| Message ID | 20210421160319.42251-14-jacopo@jmondi.org |
|---|---|
| State | Accepted |
| Delegated to: | Jacopo Mondi |
| Headers | show |
| Series |
|
| Related | show |
Hi Jacopo, thank you for the patch. On Thu, Apr 22, 2021 at 1:03 AM Jacopo Mondi <jacopo@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. > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > Signed-off-by: Jacopo Mondi <jacopo@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 6c6d31f78c88..ac135f95de12 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -1414,6 +1414,18 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) > << ", timestamp: " << buffer->metadata().timestamp; > > if (stream == &unicam_[Unicam::Image]) { > + /* > + * Record the sensor timestamp in the Request. > + * > + * \todo Do not assume the request in the front of the queue > + * is the correct one > + */ Can you have any idea how to get rid of this assumption? I am ok with this patch as-is while the todo should be resolved in the near future. Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > + 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@lists.libcamera.org > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Hiro, On Thu, Apr 22, 2021 at 02:23:34PM +0900, Hirokazu Honda wrote: > Hi Jacopo, thank you for the patch. > > On Thu, Apr 22, 2021 at 1:03 AM Jacopo Mondi <jacopo@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. > > > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > > Signed-off-by: Jacopo Mondi <jacopo@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 6c6d31f78c88..ac135f95de12 100644 > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > @@ -1414,6 +1414,18 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) > > << ", timestamp: " << buffer->metadata().timestamp; > > > > if (stream == &unicam_[Unicam::Image]) { > > + /* > > + * Record the sensor timestamp in the Request. > > + * > > + * \todo Do not assume the request in the front of the queue > > + * is the correct one > > + */ > > Can you have any idea how to get rid of this assumption? > I am ok with this patch as-is while the todo should be resolved in the > near future. Yes, there's a plan https://patchwork.libcamera.org/patch/11992/ > > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > > + 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@lists.libcamera.org > > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, On Thu, Apr 22, 2021 at 4:11 PM Jacopo Mondi <jacopo@jmondi.org> wrote: > > Hi Hiro, > > On Thu, Apr 22, 2021 at 02:23:34PM +0900, Hirokazu Honda wrote: > > Hi Jacopo, thank you for the patch. > > > > On Thu, Apr 22, 2021 at 1:03 AM Jacopo Mondi <jacopo@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. > > > > > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > > > Signed-off-by: Jacopo Mondi <jacopo@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 6c6d31f78c88..ac135f95de12 100644 > > > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > > > @@ -1414,6 +1414,18 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) > > > << ", timestamp: " << buffer->metadata().timestamp; > > > > > > if (stream == &unicam_[Unicam::Image]) { > > > + /* > > > + * Record the sensor timestamp in the Request. > > > + * > > > + * \todo Do not assume the request in the front of the queue > > > + * is the correct one > > > + */ > > > > Can you have any idea how to get rid of this assumption? > > I am ok with this patch as-is while the todo should be resolved in the > > near future. > > Yes, there's a plan > https://patchwork.libcamera.org/patch/11992/ > Good to know. I look forward to it. -Hiro > > > > Reviewed-by: Hirokazu Honda <hiroh@chromium.org> > > > > > + 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@lists.libcamera.org > > > https://lists.libcamera.org/listinfo/libcamera-devel
Hi Jacopo, Thank you for the patch. On Wed, Apr 21, 2021 at 06:03:16PM +0200, Jacopo Mondi 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. > > Reviewed-by: Naushir Patuck <naush@raspberrypi.com> > Signed-off-by: Jacopo Mondi <jacopo@jmondi.org> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > 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 6c6d31f78c88..ac135f95de12 100644 > --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp > @@ -1414,6 +1414,18 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) > << ", timestamp: " << buffer->metadata().timestamp; > > if (stream == &unicam_[Unicam::Image]) { > + /* > + * Record the sensor timestamp in the Request. > + * > + * \todo Do not assume the request in the front of the queue > + * is the correct one > + */ > + 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.
diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp index 6c6d31f78c88..ac135f95de12 100644 --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp @@ -1414,6 +1414,18 @@ void RPiCameraData::unicamBufferDequeue(FrameBuffer *buffer) << ", timestamp: " << buffer->metadata().timestamp; if (stream == &unicam_[Unicam::Image]) { + /* + * Record the sensor timestamp in the Request. + * + * \todo Do not assume the request in the front of the queue + * is the correct one + */ + 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.