sccn / liblsl

C++ lsl library for multi-modal time-synched data transmission over the local network
Other
108 stars 63 forks source link

Need ability to reset timestamp postprocessor state #117

Closed cboulay closed 3 years ago

cboulay commented 3 years ago

I ran into an issue when using flush for a period to ignore data while not allowing the data to accumulate in the queue, then when I resumed using pull_chunk the timestamps were incorrect because the postprocessing was trying to smooth them with the pre-flush timestamps.

I think it would be good if using flush reset the internal postprocessing state.

More generally, there should be a mechanism to reset the post-processing state. This should be done automatically when flush is called, also when set_postprocessing is called, and we can expose it via the inlet interface.

Inside the post-processor, the state is reset whenever smoothing_initialized_ is false (a private variable, false on init): https://github.com/sccn/liblsl/blob/c8cb4dfaba980ffab1b0d18fbc033898a459b361/src/time_postprocessor.cpp#L51

So if we can somehow set smoothing_initialized_ to false then this should reset the state and fix my problem. We could write a function or a setter to expose this. But first I want to see if there are any existing ways to do this. smoothing_initialize_ will be set to false if the query_reset_ callback returns true: https://github.com/sccn/liblsl/blob/c8cb4dfaba980ffab1b0d18fbc033898a459b361/src/time_postprocessor.cpp#L33-L39

Here is where the inlet initializes the post-processor and passes the callbacks, the 3rd is the query_reset_ callback. https://github.com/sccn/liblsl/blob/c8cb4dfaba980ffab1b0d18fbc033898a459b361/src/stream_inlet_impl.h#L45-L47

time_receiver_.was_reset() is not too helpful other than telling it to look for was_reset_, another private variable: https://github.com/sccn/liblsl/blob/c8cb4dfaba980ffab1b0d18fbc033898a459b361/src/time_receiver.cpp#L68-L73

One way the was_reset_ variable gets changed is when reset_timeoffset_on_recovery https://github.com/sccn/liblsl/blob/c8cb4dfaba980ffab1b0d18fbc033898a459b361/src/time_receiver.cpp#L201-L208 This is a bit of a dead end; I don't want to "trick" the inlet into thinking we're recovering from a disconnect. But, we can expose another way to change was_reset_ and that could be helpful.

Proposed changes:

cboulay commented 3 years ago

In the first option above (adding smoothing_initialize_ = false;), we would also need to add samples_seen_ = 0.;.

I don't know if setting was_reset_ = true; is good enough. It can take up to 20 samples or 500 msec for that change to have an effect. I'd wager that if someone wants to reset the internal state then they want it done immediately. Maybe then the first proposed change is the only way, which, after the proposed change can be triggered by (re-) setting the inlet's postproc options.

Tristan's working on a way to make the regression robust to the missing timestamps. This indeed would be better for the flush case because then we wouldn't lose the history that went into calculation the clock corrections and smoothing. Even with those changes, I think we still need an immediate manual reset.

tstenner commented 3 years ago

There's two things to this issue:

1) resetting the postprocessing flags should reset the postprocessing state 2) flushing an inlet's buffer is meant as a quick way to "receive" samples without copying them to a buffer and get fresh samples (received after the call to flush) when pulling samples from the inlet. I wouldn't expect all individual sample timestamps to be analyzed when flushing, but pretending the samples didn't ever exist gives the wrong results for the dejittered timestamps.

1) is simple: unsetting a flag should reset the postprocessing parameters iff it was enabled before 2) is partially implemented for dejittering in #118, but I haven't checked if any other regression parameters need to be tweaked for flushed samples. Clock synchronization might also need some adjustment, but for 99% of workloads it shouldn't matter.