Message ID | 20250708085147.752248-3-naush@raspberrypi.com |
---|---|
State | Accepted |
Commit | 29a88d85b730baed52a2f2e5fde2a927474ce41c |
Headers | show |
Series |
|
Related | show |
Hi Naush Thanks for the patch. On Tue, 8 Jul 2025 at 09:52, Naushir Patuck <naush@raspberrypi.com> wrote: > > Use nanoseconds for the FrameWallClock control to match the units for > other timestamp controls, including SensorTimestamp. > > Update the RPi pipeline handlers to match the new nanoseconds units when > converting from SensorTimestamp to FrameWallClock. > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/libcamera/control_ids_core.yaml | 3 ++- > src/libcamera/pipeline/rpi/pisp/pisp.cpp | 2 +- > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 2 +- > 3 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml > index 566e1533702f..eec4b4f937ee 100644 > --- a/src/libcamera/control_ids_core.yaml > +++ b/src/libcamera/control_ids_core.yaml > @@ -1274,7 +1274,8 @@ controls: > description: | > This timestamp corresponds to the same moment in time as the > SensorTimestamp, but is represented as a wall clock time as measured by > - the CLOCK_REALTIME clock. > + the CLOCK_REALTIME clock. Like SensorTimestamp, the timestamp value is > + expressed in nanoseconds. > > Being a wall clock measurement, it can be used to synchronise timing > across different devices. > diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp > index 2df91bacf3be..92b9070c1bc2 100644 > --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp > +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp > @@ -1760,7 +1760,7 @@ void PiSPCameraData::cfeBufferDequeue(FrameBuffer *buffer) > */ > wallClockRecovery_.addSample(); > uint64_t sensorTimestamp = buffer->metadata().timestamp; > - uint64_t wallClockTimestamp = wallClockRecovery_.getOutput(sensorTimestamp / 1000); > + uint64_t wallClockTimestamp = wallClockRecovery_.getOutput(sensorTimestamp); > > ctrl.set(controls::SensorTimestamp, sensorTimestamp); > ctrl.set(controls::FrameWallClock, wallClockTimestamp); > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp > index e99a7edf809c..5cadef52712f 100644 > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp > @@ -778,7 +778,7 @@ void Vc4CameraData::unicamBufferDequeue(FrameBuffer *buffer) > */ > wallClockRecovery_.addSample(); > uint64_t sensorTimestamp = buffer->metadata().timestamp; > - uint64_t wallClockTimestamp = wallClockRecovery_.getOutput(sensorTimestamp / 1000); > + uint64_t wallClockTimestamp = wallClockRecovery_.getOutput(sensorTimestamp); Yes, looks good to me. Reviewed-by: David Plowman <david.plowman@raspberrypi.com> Thanks! David > > ctrl.set(controls::SensorTimestamp, sensorTimestamp); > ctrl.set(controls::FrameWallClock, wallClockTimestamp); > -- > 2.43.0 >
Quoting Naushir Patuck (2025-07-08 09:49:15) > Use nanoseconds for the FrameWallClock control to match the units for > other timestamp controls, including SensorTimestamp. > > Update the RPi pipeline handlers to match the new nanoseconds units when > converting from SensorTimestamp to FrameWallClock. Thanks, I think this removes the ambiguity/inconsistency. I noted that the ROS discussion was also worried that FrameWallClock is only applicable on Raspberry Pi at the moment - I know we went round in circles on that before too - I might try to see how we can make it easily work for all pipelines (they only noticed the lack of FrameWallClock in UVC ... but anyway - that's not related/blocking this patch) Reviewed-by: Kieran Bingham <kieran.bingham@ideasonboard.com> > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > --- > src/libcamera/control_ids_core.yaml | 3 ++- > src/libcamera/pipeline/rpi/pisp/pisp.cpp | 2 +- > src/libcamera/pipeline/rpi/vc4/vc4.cpp | 2 +- > 3 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml > index 566e1533702f..eec4b4f937ee 100644 > --- a/src/libcamera/control_ids_core.yaml > +++ b/src/libcamera/control_ids_core.yaml > @@ -1274,7 +1274,8 @@ controls: > description: | > This timestamp corresponds to the same moment in time as the > SensorTimestamp, but is represented as a wall clock time as measured by > - the CLOCK_REALTIME clock. > + the CLOCK_REALTIME clock. Like SensorTimestamp, the timestamp value is > + expressed in nanoseconds. > > Being a wall clock measurement, it can be used to synchronise timing > across different devices. > diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp > index 2df91bacf3be..92b9070c1bc2 100644 > --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp > +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp > @@ -1760,7 +1760,7 @@ void PiSPCameraData::cfeBufferDequeue(FrameBuffer *buffer) > */ > wallClockRecovery_.addSample(); > uint64_t sensorTimestamp = buffer->metadata().timestamp; > - uint64_t wallClockTimestamp = wallClockRecovery_.getOutput(sensorTimestamp / 1000); > + uint64_t wallClockTimestamp = wallClockRecovery_.getOutput(sensorTimestamp); > > ctrl.set(controls::SensorTimestamp, sensorTimestamp); > ctrl.set(controls::FrameWallClock, wallClockTimestamp); > diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp > index e99a7edf809c..5cadef52712f 100644 > --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp > +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp > @@ -778,7 +778,7 @@ void Vc4CameraData::unicamBufferDequeue(FrameBuffer *buffer) > */ > wallClockRecovery_.addSample(); > uint64_t sensorTimestamp = buffer->metadata().timestamp; > - uint64_t wallClockTimestamp = wallClockRecovery_.getOutput(sensorTimestamp / 1000); > + uint64_t wallClockTimestamp = wallClockRecovery_.getOutput(sensorTimestamp); > > ctrl.set(controls::SensorTimestamp, sensorTimestamp); > ctrl.set(controls::FrameWallClock, wallClockTimestamp); > -- > 2.43.0 >
diff --git a/src/libcamera/control_ids_core.yaml b/src/libcamera/control_ids_core.yaml index 566e1533702f..eec4b4f937ee 100644 --- a/src/libcamera/control_ids_core.yaml +++ b/src/libcamera/control_ids_core.yaml @@ -1274,7 +1274,8 @@ controls: description: | This timestamp corresponds to the same moment in time as the SensorTimestamp, but is represented as a wall clock time as measured by - the CLOCK_REALTIME clock. + the CLOCK_REALTIME clock. Like SensorTimestamp, the timestamp value is + expressed in nanoseconds. Being a wall clock measurement, it can be used to synchronise timing across different devices. diff --git a/src/libcamera/pipeline/rpi/pisp/pisp.cpp b/src/libcamera/pipeline/rpi/pisp/pisp.cpp index 2df91bacf3be..92b9070c1bc2 100644 --- a/src/libcamera/pipeline/rpi/pisp/pisp.cpp +++ b/src/libcamera/pipeline/rpi/pisp/pisp.cpp @@ -1760,7 +1760,7 @@ void PiSPCameraData::cfeBufferDequeue(FrameBuffer *buffer) */ wallClockRecovery_.addSample(); uint64_t sensorTimestamp = buffer->metadata().timestamp; - uint64_t wallClockTimestamp = wallClockRecovery_.getOutput(sensorTimestamp / 1000); + uint64_t wallClockTimestamp = wallClockRecovery_.getOutput(sensorTimestamp); ctrl.set(controls::SensorTimestamp, sensorTimestamp); ctrl.set(controls::FrameWallClock, wallClockTimestamp); diff --git a/src/libcamera/pipeline/rpi/vc4/vc4.cpp b/src/libcamera/pipeline/rpi/vc4/vc4.cpp index e99a7edf809c..5cadef52712f 100644 --- a/src/libcamera/pipeline/rpi/vc4/vc4.cpp +++ b/src/libcamera/pipeline/rpi/vc4/vc4.cpp @@ -778,7 +778,7 @@ void Vc4CameraData::unicamBufferDequeue(FrameBuffer *buffer) */ wallClockRecovery_.addSample(); uint64_t sensorTimestamp = buffer->metadata().timestamp; - uint64_t wallClockTimestamp = wallClockRecovery_.getOutput(sensorTimestamp / 1000); + uint64_t wallClockTimestamp = wallClockRecovery_.getOutput(sensorTimestamp); ctrl.set(controls::SensorTimestamp, sensorTimestamp); ctrl.set(controls::FrameWallClock, wallClockTimestamp);
Use nanoseconds for the FrameWallClock control to match the units for other timestamp controls, including SensorTimestamp. Update the RPi pipeline handlers to match the new nanoseconds units when converting from SensorTimestamp to FrameWallClock. Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/libcamera/control_ids_core.yaml | 3 ++- src/libcamera/pipeline/rpi/pisp/pisp.cpp | 2 +- src/libcamera/pipeline/rpi/vc4/vc4.cpp | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-)