Validate playback rate and limiting #16
Conversation
| } else if (playbackRate > 1.0f) { | ||
| val delta = Duration.between(previousTimestamp ?: Instant.now(clock), shiftedTimestamp) | ||
| Duration.ofMillis((delta.toMillis() / playbackRate).toLong()).also { | ||
| previousTimestamp = shiftedTimestamp.plus(it) |
There was a problem hiding this comment.
why we make previousTimestamp=shiftedTimestamp.plus(delay) when playbackRate > 1.0? I feel it should be the same as previousTimestamp = shiftedTimestamp. 🤔 For example:
When playback rate is 2.0, and earliest record timestamp is 00:00, now time is 01:00, timeshiftNanos is 1 hr. Then for the records consumed from a cassette file in order, the calculated key properties will be:
| recordTimestamp | shiftedTimestamp | delta(line 26) | delay(return value) | previousTimestamp |
|---|---|---|---|---|
| 00:00 | 01:00 | N/A | null | 01:00 |
| 00:10 | 01:10 | 10s | 5s | 01:15 (should be 01:10) |
| 00:20 | 01:20 | 5s (should be 10s) | 2.5s (should be 5s) | 01:22.5 (should be 01:20) |
And for the corresponding test, it should pass correctly for two records, but if with three and more records, I suspect it may surface some problem.
How do you think? Or Maybe I misunderstand something here?
| previousTimestamp = shiftedTimestamp.plus(it) | ||
| } | ||
| } else { | ||
| Duration.between(previousTimestamp, shiftedTimestamp) |
There was a problem hiding this comment.
When playback rate is 1.0 here, should we update previousTimestamp as well? Otherwise previousTimestamp will always be the first record shiftedTimestamp, and for the third record and forward records, the calculated delay will be not correct.
Discussed in #14, playback rates less than 1.0 are running as quickly as possible.
This change introduces a limiter to propose the best delay based on the record time shift and the delta between current and next record timestamps.
Additionally, I had trouble getting Git to see new kt files. I've trimmed the .gitignore to just what is required.