-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Possible bug with _clock_sync and clock reset detection #93
Comments
Can you provide a short example which shows the problem (ideally with a suitable XDF file)? What would be your proposed fix? |
I am closing this issue due to the original poster's lack of response to the follow-up question. @chkothe do you have any opinion on whether the current clock sync algorithm might be buggy? |
Hi, sorry to re-open this but I think @JakubOrlinski97 was correct in reporting the bug in this ( Line 621 in 0df1599
As far as I understand, I assume we need an additional step:
Does it make sense to you? |
See if it helps. It worked well on my data. |
Yes, this looks like a bug in the case where a PC clock resets or jumps during a recording. I think @andbiz and @JakubOrlinski97 share the price for the first users who actually encountered this. And yes the proposed fix sounds reasonable, but we'll want to make sure we don't have an off-by-one error at one of the segment edges, so having that test file would be useful. Is that something you got in an actual recording? (would be curious what that looks like in the data and how large the jump was) |
When looking through the code of _clock_sync (https://github.com/xdf-modules/pyxdf/blob/main/pyxdf/pyxdf.py#L538) which is used at import time to synchronise clocks between streams I think I found a bug.
In the synchronisation method there is a part that detects clock resets, and then processes each chunk of data separately if there is significant differences between the clock offsets.
The problem is, when the coefficients for the linear function are taken and applied to the timestamps, the code uses indexes from the range determined from the clock_values. These are not the same indexes as those that should be used to address the timestamps of the recording. This means only a tiny subset of the recording gets synchronised as the clock_values get saved only every 5 seconds or so.
The relevant code is in this for loop, specifically where
stream.time_stamps
is addressed with the slice:https://github.com/xdf-modules/pyxdf/blob/main/pyxdf/pyxdf.py#L637
The text was updated successfully, but these errors were encountered: