mike-jumper commented on code in PR #883:
URL: https://github.com/apache/guacamole-client/pull/883#discussion_r1228588817
##########
guacamole-common-js/src/main/webapp/modules/SessionRecording.js:
##########
@@ -684,14 +766,13 @@ Guacamole.SessionRecording = function
SessionRecording(source) {
// Pull the upcoming frame
var next = frames[currentFrame + 1];
- // Calculate the real timestamp corresponding to when the next
- // frame begins
- var nextRealTimestamp = next.timestamp - startVideoTimestamp +
startRealTimestamp;
+ // Add the offset to the next frame timestamp to calculate the
delay
+ var delay = toRelativeTimestamp(next.timestamp) - currentPosition;
Review Comment:
> So to be clear - you're suggesting that we modify `seekToFrame` to either
advance to the next frame _or_ increment `currentPosition` depending on whether
`currentPosition + refreshInterval` >= the timestamp of the next frame?
More or less. My thinking was:
* The core issue is that the previous code scheduled the next seek based on
the frame duration, which can be significantly longer than one second.
* Part of these changes is ensuring we can reliably seek to any position,
even between frames.
* We might be able to achieve the desired effect by ensuring the delay until
next seek is bounded at the refresh interval, even if that seek would be
mid-frame.
I'd think that would mean we'd have to:
1. Calculate the timestamp of the nearest whole
multiple-of-`refreshInterval`.
2. If the next frame is before that, easy - seek to _that_.
3. If not, seek to the calculated timestamp.
That way we are guaranteed to be updated every `refreshInterval` relative to
playback position, not relative to a local timer.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
