diff --git a/CHANGELOG.md b/CHANGELOG.md index de1c26a..44beabc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ### Fixed - Fix conversion to `int` for `sample_count` in XDF footer ([#173](https://github.com/xdf-modules/pyxdf/pull/173) by [Clemens Brunner](https://github.com/cbrnr)) - Scope the log level set by `verbose` to the `load_xdf` call, so it no longer overrides the calling application's logging configuration for the rest of the process ([#175](https://github.com/xdf-modules/pyxdf/pull/175) by [Stefan Appelhoff](https://github.com/sappelhoff)) +- Stop truncating samples beyond the footer `sample_count` and the last clock offset (introduced in [#150](https://github.com/xdf-modules/xdf-Python/pull/150) and [#158](https://github.com/xdf-modules/xdf-Python/pull/158)), which dropped valid samples; garbage clock offsets are handled by clock reset segmentation since [#167](https://github.com/xdf-modules/xdf-Python/pull/167) ([#176](https://github.com/xdf-modules/pyxdf/issues/176) by [Mathieu Scheltienne](https://github.com/mscheltienne)) ## [1.17.5] - 2026-06-15 ### Changed diff --git a/src/pyxdf/pyxdf.py b/src/pyxdf/pyxdf.py index 143f244..ea9df2e 100644 --- a/src/pyxdf/pyxdf.py +++ b/src/pyxdf/pyxdf.py @@ -385,10 +385,6 @@ def load_xdf( else: stream.time_series = np.zeros((0, stream.nchns)) - # detect and truncate corrupted clock offsets (pylsl#67 bug) - if synchronize_clocks: - temp = _truncate_corrupted_offsets(temp, streams) - # perform (fault-tolerant) clock synchronization if requested if synchronize_clocks: logger.info(" performing clock synchronization...") @@ -612,155 +608,6 @@ def _segment_clock_diff(diff, thresh_stds, thresh_secs): return b_break -def _detect_corrupted_clock_offset( - clock_times, - clock_values, - time_thresh=10.0, - value_thresh=10.0, -): - """Detect if the last clock offset entry is statistically anomalous. - - Checks whether the last clock offset entry shows anomalous times or values compared - to the rest of the sequence. Uses robust statistics (MAD - Median Absolute - Deviation) to handle streams with naturally variable clock offsets. - - This function is used by ``_truncate_corrupted_offsets`` as part of the - detection logic for the pylsl#67 bug, but does not alone confirm the bug - since legitimate clock resets can also cause statistical anomalies. - - Args: - clock_times : list of clock offset measurement times. - clock_values : list of clock offset values. - time_thresh : flag if last time interval exceeds this multiple of the - median interval. (default: 10.0) - value_thresh : flag if last value's MAD-based z-score exceeds this. - (default: 10.0) - - Returns: - bool : True if the last clock offset is statistically anomalous. - - See Also: - https://github.com/labstreaminglayer/pylsl/issues/67 - """ - times = np.asarray(clock_times) - values = np.asarray(clock_values) - - # Need at least 3 offsets for meaningful statistics - if len(times) < 3: - return False - - # Strategy 1: Detect anomalous time interval - intervals = np.diff(times) - median_interval = np.median(intervals[:-1]) - last_interval = np.abs(intervals[-1]) # Use abs to catch backward jumps - - if median_interval > 0: - time_ratio = last_interval / median_interval - else: - time_ratio = np.inf if last_interval > 0 else 1.0 - - # Strategy 2: Detect anomalous value using MAD (robust to outliers) - # MAD = Median Absolute Deviation from the median - median_val = np.median(values[:-1]) - mad = np.median(np.abs(values[:-1] - median_val)) - - # Scale MAD to be comparable to standard deviation (for normal distributions) - # MAD * 1.4826 ≈ standard deviation - # See: https://en.wikipedia.org/wiki/Median_absolute_deviation#Relation_to_standard_deviation - if mad > np.finfo(float).eps: - value_zscore = np.abs(values[-1] - median_val) / (1.4826 * mad) - else: - # If MAD is essentially 0 (all values identical), we cannot reliably - # detect value anomalies. Rely on time ratio check alone. - value_zscore = 0.0 - - # Flag as corrupted if EITHER condition is met - # (the bug typically causes both, but even one is enough since we already validate - # that there is a problem with the sample count as well) - return time_ratio > time_thresh or value_zscore > value_thresh - - -def _truncate_corrupted_offsets(temp, streams): - """Truncate corrupted clock offsets and associated extra samples. - - The pylsl#67, liblsl#246 bug occurs when an LSL outlet is destroyed while an inlet - is still connected, causing an extra sample with potentially garbage clock - offset values to be recorded. - - This function requires BOTH conditions to confirm the pylsl#67, liblsl#246 bug: - - 1. Extra samples exist (sample count exceeds footer sample_count). - 2. The last clock offset is statistically anomalous (see - ``_detect_corrupted_clock_offset``). - - If both conditions are met, both the extra samples and the corrupted clock - offset are truncated. - - Note: Extra samples alone (without corrupted clock offset) are NOT truncated, - as the footer sample_count may be incorrect due to race conditions in - liblsl/LabRecorder where final samples arrive after the footer is written. - Similarly, anomalous clock offsets alone (without extra samples) are NOT - truncated, as they may be due to legitimate clock resets. - - Args: - temp : dict of StreamData objects indexed by stream ID. - streams : dict of stream info dicts indexed by stream ID. - - Returns: - dict : the modified temp dictionary. - - See Also: - https://github.com/labstreaminglayer/pylsl/issues/67 - https://github.com/sccn/liblsl/issues/246 - """ - for stream_id, stream in temp.items(): - # First check if there are extra samples - this is the primary evidence - # of a potential pylsl#67, liblsl#246 bug. - footer = streams.get(stream_id, {}).get("footer", {}).get("info") or {} - sample_count_str = footer.get("sample_count", [None])[0] - if sample_count_str is None: - continue - footer_count = int(float(sample_count_str)) # can be with .0 suffix - - if len(stream.time_stamps) <= footer_count: - continue # No extra samples → no evidence of pylsl#67, liblsl#246 - - # Extra samples exist - now check if clock offset is also corrupted. - # We require BOTH conditions because: - # - Extra samples alone could be valid "late" data (footer off-by-1) - # - Anomalous clock offsets alone could be legitimate clock resets - # - Both together strongly indicate the pylsl#67, liblsl#246 bug - clock_corrupted = False - if len(stream.clock_times) >= 3: - clock_corrupted = _detect_corrupted_clock_offset( - stream.clock_times, stream.clock_values - ) - - if not clock_corrupted: - continue # Extra samples but normal clock offset → keep everything - - # Both conditions met → truncate both - logger.warning( - "Stream %s: last clock offset is statistically anomalous, " - "truncating (see pylsl#67, liblsl#246).", - stream_id, - ) - stream.clock_times = stream.clock_times[:-1] - stream.clock_values = stream.clock_values[:-1] - - logger.warning( - "Stream %s: sample count (%d) exceeds footer sample_count (%d), " - "truncating extra samples.", - stream_id, - len(stream.time_stamps), - footer_count, - ) - stream.time_stamps = stream.time_stamps[:footer_count] - stream.time_series = stream.time_series[:footer_count] - - return temp - - def _detect_clock_resets( stream, time_thresh_stds, diff --git a/test/test_clock_sync.py b/test/test_clock_sync.py index 4d4acd0..d379afc 100644 --- a/test/test_clock_sync.py +++ b/test/test_clock_sync.py @@ -1,4 +1,7 @@ +from __future__ import annotations + import logging +from typing import TYPE_CHECKING import numpy as np import pytest @@ -6,7 +9,8 @@ from pyxdf.pyxdf import _clock_sync -# No clock resets +if TYPE_CHECKING: + from typing import Literal @pytest.mark.parametrize("n_clock_offsets", list(range(5))) @@ -609,3 +613,36 @@ def test_sync_clock_corrupt_final_offset(clock_times): (0, 9), ], ) + + +@pytest.mark.parametrize("n_garbage_offsets", [1, 2]) +def test_sync_garbage_final_offsets_and_extra_sample( + n_garbage_offsets: Literal[1, 2], +) -> None: + """Recording ending as reported in pylsl#67 must sync without dropping samples. + + Destroying an outlet with liblsl <= 1.17.1 while it is recorded appends one extra + sample stamped at the outlet destruction time, and the last clock offsets can then + be corrupted (e.g. -771221s). LabRecorder stores offsets as ``time = now - offset``, + so a corrupted value shifts both the offset time and value. The corrupted offsets + must end up in their own clock segment without any samples. + """ + offset = -0.001 + clock_times = [1000 + 5 * i for i in range(20)] + clock_values = [offset] * 20 + for i in range(n_garbage_offsets): + now = clock_times[19] + 5 * (i + 1) + clock_times.append(now + 771221.25) + clock_values.append(-771221.25) + # regular markers plus the extra sample pushed when the outlet was destroyed + time_stamps = np.append(np.arange(1000, 1095, 0.5), 1095.008) + streams = { + 1: MockStreamData( + time_stamps=time_stamps, + clock_times=clock_times, + clock_values=clock_values, + ) + } + _clock_sync(streams) + np.testing.assert_allclose(streams[1].time_stamps, time_stamps + offset, atol=1e-9) + np.testing.assert_equal(streams[1].clock_segments, [(0, len(time_stamps) - 1)]) diff --git a/test/test_truncate_corrupted_offsets.py b/test/test_truncate_corrupted_offsets.py deleted file mode 100644 index 0ac7a16..0000000 --- a/test/test_truncate_corrupted_offsets.py +++ /dev/null @@ -1,212 +0,0 @@ -"""Tests for _truncate_corrupted_offsets (pylsl#67 bug fix). - -These tests verify the automatic detection and truncation of extra samples -and corrupted clock offsets caused by the pylsl#67 bug, where destroying an -LSL outlet while an inlet is still connected causes an extra sample with -potentially garbage clock offset values to be recorded. - -See: https://github.com/labstreaminglayer/pylsl/issues/67 -""" - -import numpy as np -from mock_data_stream import MockStreamData - -from pyxdf.pyxdf import _clock_sync, _truncate_corrupted_offsets - - -def test_no_truncation_when_sample_count_matches(): - """Normal data with matching sample count should not be modified.""" - n_samples = 100 - n_offsets = 20 - clock_tdiff = 5 - - # Create normal stream data - time_stamps = np.linspace(0, 100, n_samples) - clock_times = [i * clock_tdiff for i in range(n_offsets)] - clock_values = [1e-6 * (i % 3 - 1) for i in range(n_offsets)] # Small jitter - - temp = { - 1: MockStreamData( - time_stamps=time_stamps, - clock_times=clock_times, - clock_values=clock_values, - ) - } - streams = {1: {"footer": {"info": {"sample_count": [str(n_samples)]}}}} - - # Store original values for comparison - orig_timestamps = temp[1].time_stamps.copy() - orig_clock_times = temp[1].clock_times.copy() - orig_clock_values = temp[1].clock_values.copy() - - # Apply truncation - temp = _truncate_corrupted_offsets(temp, streams) - - # Verify nothing was modified - np.testing.assert_array_equal(temp[1].time_stamps, orig_timestamps) - assert temp[1].clock_times == orig_clock_times - assert temp[1].clock_values == orig_clock_values - - -def test_no_truncation_when_clock_offsets_normal(): - """Extra samples should NOT be truncated when clock offsets are normal. - - This tests the case where footer sample_count is off-by-1 due to race - conditions (e.g., final sample arrives after footer is written), but the - data is valid. Without evidence of corruption (corrupted clock offset), - the extra samples should be preserved. - """ - n_samples = 100 - n_offsets = 20 - clock_tdiff = 5 - - # Create stream with extra sample (n_samples + 1) but normal clock offsets - time_stamps = np.linspace(0, 101, n_samples + 1) # One extra sample - clock_times = [i * clock_tdiff for i in range(n_offsets)] - clock_values = [1e-6 * (i % 3 - 1) for i in range(n_offsets)] # Normal values - - temp = { - 1: MockStreamData( - time_stamps=time_stamps, - clock_times=clock_times, - clock_values=clock_values, - ) - } - # Footer says n_samples, but we have n_samples + 1 - streams = {1: {"footer": {"info": {"sample_count": [str(n_samples)]}}}} - - # Store original values - orig_timestamps = temp[1].time_stamps.copy() - orig_clock_times = temp[1].clock_times.copy() - orig_clock_values = temp[1].clock_values.copy() - - # Apply truncation - temp = _truncate_corrupted_offsets(temp, streams) - - # Verify samples were NOT truncated (clock offsets are normal, no evidence - # of pylsl#67, liblsl#246 bug, so we keep the extra samples) - np.testing.assert_array_equal(temp[1].time_stamps, orig_timestamps) - assert len(temp[1].time_stamps) == n_samples + 1 - - # Verify clock offsets were NOT truncated - assert temp[1].clock_times == orig_clock_times - assert temp[1].clock_values == orig_clock_values - - -def test_no_truncation_when_no_extra_samples(): - """Anomalous clock offset should NOT be truncated without extra samples. - - This tests the case where clock offsets look anomalous (e.g., due to - legitimate clock resets) but there are no extra samples. Without extra - samples, we have no evidence of the pylsl#67 bug, so we should not - truncate the clock offset. - """ - n_samples = 100 - n_offsets = 20 - clock_tdiff = 5 - clock_offset_value = -0.001 # Normal offset: -1ms - - # Create stream with matching sample count but "anomalous" last clock offset - # This could be a legitimate clock reset, not pylsl#67 - liblsl#246 corruption - time_stamps = np.linspace(0, 100, n_samples) - clock_times = [i * clock_tdiff for i in range(n_offsets)] - clock_values = [clock_offset_value] * n_offsets - - # Add an "anomalous" clock offset (could be legitimate clock reset) - clock_times.append(clock_times[-1] + 800000) # Huge time jump - clock_values.append(-750000) # Huge value change - - temp = { - 1: MockStreamData( - time_stamps=time_stamps, - clock_times=clock_times, - clock_values=clock_values, - ) - } - # Footer matches sample count (no extra samples) - streams = {1: {"footer": {"info": {"sample_count": [str(n_samples)]}}}} - - # Store original values - orig_timestamps = temp[1].time_stamps.copy() - orig_clock_times = temp[1].clock_times.copy() - orig_clock_values = temp[1].clock_values.copy() - - # Apply truncation - temp = _truncate_corrupted_offsets(temp, streams) - - # Verify NOTHING was truncated (no extra samples = no evidence of pylsl#67) - np.testing.assert_array_equal(temp[1].time_stamps, orig_timestamps) - assert temp[1].clock_times == orig_clock_times - assert temp[1].clock_values == orig_clock_values - - -def test_truncation_of_samples_and_corrupted_clock_offset(): - """Extra sample and corrupted clock offset should both be truncated.""" - n_samples = 100 - n_offsets = 20 - clock_tdiff = 5 - clock_offset_value = -0.001 # Normal offset: -1ms - - # Create stream with extra sample AND corrupted last clock offset - time_stamps = np.linspace(0, 101, n_samples + 1) # One extra sample - clock_times = [i * clock_tdiff for i in range(n_offsets)] - clock_values = [clock_offset_value] * n_offsets - - # Corrupt the last clock offset (mimics the real bug pattern) - clock_times.append(clock_times[-1] + 800000) # Huge time jump - clock_values.append(-750000) # Huge corrupted value - - temp = { - 1: MockStreamData( - time_stamps=time_stamps, - clock_times=clock_times, - clock_values=clock_values, - ) - } - streams = {1: {"footer": {"info": {"sample_count": [str(n_samples)]}}}} - - # Apply truncation - temp = _truncate_corrupted_offsets(temp, streams) - - # Verify samples were truncated - assert len(temp[1].time_stamps) == n_samples - - # Verify corrupted clock offset was removed - assert len(temp[1].clock_times) == n_offsets - assert len(temp[1].clock_values) == n_offsets - - # Verify the remaining clock values are the original normal ones - assert all(v == clock_offset_value for v in temp[1].clock_values) - - # Verify clock sync now works correctly (regression doesn't get corrupted) - temp = _clock_sync(temp) - expected_timestamps = ( - np.linspace(0, 101, n_samples + 1)[:n_samples] + clock_offset_value - ) - np.testing.assert_allclose( - temp[1].time_stamps, - expected_timestamps, - atol=1e-6, - ) - - -def test_no_crash_when_footer_info_is_none(): - """Streams with empty footer info (None) should be skipped gracefully. - - _xml2dict returns {"info": None} for empty XML elements like - or . This must not crash _truncate_corrupted_offsets. - """ - temp = { - 1: MockStreamData(time_stamps=np.linspace(0, 100, 100)), - 2: MockStreamData(time_stamps=np.linspace(0, 50, 50)), - } - streams = { - 1: {"footer": {"info": {"sample_count": ["100"]}}}, - 2: {"footer": {"info": None}}, - } - - # Should not raise AttributeError - result = _truncate_corrupted_offsets(temp, streams) - - # Stream 2 should be unchanged - assert len(result[2].time_stamps) == 50