| Message ID | 20250901111952.2155555-1-naush@raspberrypi.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series |
|
| Related | show |
Hi Naush, Quoting Naushir Patuck (2025-09-01 12:19:41) > When switching the FrameWallClock to use nanoseconds instead of > microseconds, the clock recovery class was left untouched. This would > cause an inaccurate clock model and return spurious results for > kernel -> wallclock timestamping. > > Annoyingly this was missed as there were no application level consumers > of FrameWallClock until now. > > Fixes: 29a88d85b730 ("libcamera: controls: Use nanoseconds units for FrameWallClock") > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> I think Paul beat you to it on this one: - https://patchwork.libcamera.org/patch/24231/ As the two patches are identical - can you add an RB tag to that one and I'll merge? > --- > src/libcamera/clock_recovery.cpp | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/src/libcamera/clock_recovery.cpp b/src/libcamera/clock_recovery.cpp > index abacf444fbf8..f9ccb6ae2144 100644 > --- a/src/libcamera/clock_recovery.cpp > +++ b/src/libcamera/clock_recovery.cpp > @@ -118,10 +118,10 @@ void ClockRecovery::addSample() > clock_gettime(CLOCK_BOOTTIME, &bootTime1); > clock_gettime(CLOCK_REALTIME, &wallTime); > clock_gettime(CLOCK_BOOTTIME, &bootTime2); > - uint64_t boot1 = bootTime1.tv_sec * 1000000ULL + bootTime1.tv_nsec / 1000; > - uint64_t boot2 = bootTime2.tv_sec * 1000000ULL + bootTime2.tv_nsec / 1000; > + uint64_t boot1 = bootTime1.tv_sec * 1000000000ULL + bootTime1.tv_nsec; > + uint64_t boot2 = bootTime2.tv_sec * 1000000000ULL + bootTime2.tv_nsec; > uint64_t boot = (boot1 + boot2) / 2; > - uint64_t wall = wallTime.tv_sec * 1000000ULL + wallTime.tv_nsec / 1000; > + uint64_t wall = wallTime.tv_sec * 1000000000ULL + wallTime.tv_nsec; > > addSample(boot, wall); > } > -- > 2.43.0 >
On Mon, 1 Sept 2025 at 12:22, Kieran Bingham <kieran.bingham@ideasonboard.com> wrote: > > Hi Naush, > > Quoting Naushir Patuck (2025-09-01 12:19:41) > > When switching the FrameWallClock to use nanoseconds instead of > > microseconds, the clock recovery class was left untouched. This would > > cause an inaccurate clock model and return spurious results for > > kernel -> wallclock timestamping. > > > > Annoyingly this was missed as there were no application level consumers > > of FrameWallClock until now. > > > > Fixes: 29a88d85b730 ("libcamera: controls: Use nanoseconds units for FrameWallClock") > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com> > > I think Paul beat you to it on this one: > > - https://patchwork.libcamera.org/patch/24231/ > > As the two patches are identical - can you add an RB tag to that one and > I'll merge? Done! Sorry I missed that Paul had already fixed this. Naush > > > --- > > src/libcamera/clock_recovery.cpp | 6 +++--- > > 1 file changed, 3 insertions(+), 3 deletions(-) > > > > diff --git a/src/libcamera/clock_recovery.cpp b/src/libcamera/clock_recovery.cpp > > index abacf444fbf8..f9ccb6ae2144 100644 > > --- a/src/libcamera/clock_recovery.cpp > > +++ b/src/libcamera/clock_recovery.cpp > > @@ -118,10 +118,10 @@ void ClockRecovery::addSample() > > clock_gettime(CLOCK_BOOTTIME, &bootTime1); > > clock_gettime(CLOCK_REALTIME, &wallTime); > > clock_gettime(CLOCK_BOOTTIME, &bootTime2); > > - uint64_t boot1 = bootTime1.tv_sec * 1000000ULL + bootTime1.tv_nsec / 1000; > > - uint64_t boot2 = bootTime2.tv_sec * 1000000ULL + bootTime2.tv_nsec / 1000; > > + uint64_t boot1 = bootTime1.tv_sec * 1000000000ULL + bootTime1.tv_nsec; > > + uint64_t boot2 = bootTime2.tv_sec * 1000000000ULL + bootTime2.tv_nsec; > > uint64_t boot = (boot1 + boot2) / 2; > > - uint64_t wall = wallTime.tv_sec * 1000000ULL + wallTime.tv_nsec / 1000; > > + uint64_t wall = wallTime.tv_sec * 1000000000ULL + wallTime.tv_nsec; > > > > addSample(boot, wall); > > } > > -- > > 2.43.0 > >
diff --git a/src/libcamera/clock_recovery.cpp b/src/libcamera/clock_recovery.cpp index abacf444fbf8..f9ccb6ae2144 100644 --- a/src/libcamera/clock_recovery.cpp +++ b/src/libcamera/clock_recovery.cpp @@ -118,10 +118,10 @@ void ClockRecovery::addSample() clock_gettime(CLOCK_BOOTTIME, &bootTime1); clock_gettime(CLOCK_REALTIME, &wallTime); clock_gettime(CLOCK_BOOTTIME, &bootTime2); - uint64_t boot1 = bootTime1.tv_sec * 1000000ULL + bootTime1.tv_nsec / 1000; - uint64_t boot2 = bootTime2.tv_sec * 1000000ULL + bootTime2.tv_nsec / 1000; + uint64_t boot1 = bootTime1.tv_sec * 1000000000ULL + bootTime1.tv_nsec; + uint64_t boot2 = bootTime2.tv_sec * 1000000000ULL + bootTime2.tv_nsec; uint64_t boot = (boot1 + boot2) / 2; - uint64_t wall = wallTime.tv_sec * 1000000ULL + wallTime.tv_nsec / 1000; + uint64_t wall = wallTime.tv_sec * 1000000000ULL + wallTime.tv_nsec; addSample(boot, wall); }
When switching the FrameWallClock to use nanoseconds instead of microseconds, the clock recovery class was left untouched. This would cause an inaccurate clock model and return spurious results for kernel -> wallclock timestamping. Annoyingly this was missed as there were no application level consumers of FrameWallClock until now. Fixes: 29a88d85b730 ("libcamera: controls: Use nanoseconds units for FrameWallClock") Signed-off-by: Naushir Patuck <naush@raspberrypi.com> --- src/libcamera/clock_recovery.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)