[libcamera-devel] [PATCH 1/2] pipeline: raspberrypi: Store timestamp in the correct Request metadata
David Plowman
david.plowman at raspberrypi.com
Wed May 12 17:51:38 CEST 2021
Hi Naush
Thanks for this patch!
On Wed, 12 May 2021 at 09:47, Naushir Patuck <naush at raspberrypi.com> wrote:
>
> Write the controls::SensorTimestamp value in the Request metadata when
> the request is popped from the queue ready to run the pipeline. This
> ensures that the timestamp is written to the correct Request item,
> which may not be at the top of the queue when the Unicam buffer dequeue
> occurs.
>
> Fixes: fcfb1dc02a6b ("libcamera: raspberry: Report sensor timestamp")
> Signed-off-by: Naushir Patuck <naush at raspberrypi.com>
> ---
> .../pipeline/raspberrypi/raspberrypi.cpp | 29 +++++++++++--------
> 1 file changed, 17 insertions(+), 12 deletions(-)
>
> diff --git a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> index 6fbdba0487bf..eb6d31670567 100644
> --- a/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> +++ b/src/libcamera/pipeline/raspberrypi/raspberrypi.cpp
> @@ -221,6 +221,8 @@ public:
>
> private:
> void checkRequestCompleted();
> + void fillRequestMetadata(const ControlList &bufferControls,
> + Request *request);
> void tryRunPipeline();
> bool findMatchingBuffers(BayerFrame &bayerFrame, FrameBuffer *&embeddedBuffer);
>
> @@ -1416,18 +1418,6 @@ 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);
> -
Indeed, I've actually been seeing segmentation faults at this line,
maybe ~10% of the time when running with the ov5647. I've applied
these patches and set the system running in a loop - all seems to be
good now!
> /*
> * Lookup the sensor controls used for this frame sequence from
> * DelayedControl and queue them along with the frame buffer.
> @@ -1689,6 +1679,13 @@ void RPiCameraData::applyScalerCrop(const ControlList &controls)
> }
> }
>
> +void RPiCameraData::fillRequestMetadata(const ControlList &bufferControls,
> + Request *request)
> +{
> + request->metadata().set(controls::SensorTimestamp,
> + bufferControls.get(controls::SensorTimestamp));
> +}
I did wonder why this is worth a whole new function, but I see the
next commit adds something more to it!
> +
> void RPiCameraData::tryRunPipeline()
> {
> FrameBuffer *embeddedBuffer;
> @@ -1708,6 +1705,14 @@ void RPiCameraData::tryRunPipeline()
> /* See if a new ScalerCrop value needs to be applied. */
> applyScalerCrop(request->controls());
>
> + /*
> + * Clear the request metadata and fill it with some initial non-IPA
> + * related controls. We clear it first because the request metadata
> + * may have been populated if we have dropped the previous frame.
> + */
> + request->metadata().clear();
Ah yes, thank you. That's been most annoying since the
ControlList::merge function has started spitting out warnings!
Reviewed-by: David Plowman <david.plowman at raspberrypi.com>
Tested-by: David Plowman <david.plowman at raspberrypi.com>
Thanks
David
> + fillRequestMetadata(bayerFrame.controls, request);
> +
> /*
> * Process all the user controls by the IPA. Once this is complete, we
> * queue the ISP output buffer listed in the request to start the HW
> --
> 2.25.1
>
> _______________________________________________
> libcamera-devel mailing list
> libcamera-devel at lists.libcamera.org
> https://lists.libcamera.org/listinfo/libcamera-devel
More information about the libcamera-devel
mailing list