From b8de8ebb902d7601f0a991414d884ee0025e090f Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Fri, 2 Jun 2023 23:12:05 +0000 Subject: [PATCH 1/4] GUACAMOLE-1803: Update session recording playback at least once per second and allow arbitrary seeking. --- .../main/webapp/modules/SessionRecording.js | 185 ++++++++++++++---- .../src/app/player/directives/player.js | 11 +- 2 files changed, 156 insertions(+), 40 deletions(-) diff --git a/guacamole-common-js/src/main/webapp/modules/SessionRecording.js b/guacamole-common-js/src/main/webapp/modules/SessionRecording.js index ce8968f2b..37270a5fb 100644 --- a/guacamole-common-js/src/main/webapp/modules/SessionRecording.js +++ b/guacamole-common-js/src/main/webapp/modules/SessionRecording.js @@ -32,8 +32,13 @@ var Guacamole = Guacamole || {}; * @param {!Blob|Guacamole.Tunnel} source * The Blob from which the instructions of the recording should * be read. + * @param {number} [refreshInterval=1000] + * The minimum number of milliseconds between updates to the recording + * position through the provided onseek() callback. If non-positive, this + * parameter will be ignored, and the recording position will only be + * updated when seek requests are made, or when new frames are rendered. */ -Guacamole.SessionRecording = function SessionRecording(source) { +Guacamole.SessionRecording = function SessionRecording(source, refreshInterval) { /** * Reference to this Guacamole.SessionRecording. @@ -139,22 +144,20 @@ Guacamole.SessionRecording = function SessionRecording(source) { var currentFrame = -1; /** - * The timestamp of the frame when playback began, in milliseconds. If - * playback is not in progress, this will be null. + * True if the player is currently playing, or false otherwise. * * @private - * @type {number} + * @type {boolean} */ - var startVideoTimestamp = null; + var currentlyPlaying = null; /** - * The real-world timestamp when playback began, in milliseconds. If - * playback is not in progress, this will be null. + * The current position within the recording, in milliseconds. * * @private - * @type {number} + * @type {!number} */ - var startRealTimestamp = null; + var currentPosition = 0; /** * An object containing a single "aborted" property which is set to @@ -211,6 +214,25 @@ Guacamole.SessionRecording = function SessionRecording(source) { */ var seekCallback = null; + /** + * Any current timeout associated with scheduling frame replay, or updating + * the current position, or null if no frame position increment is currently + * scheduled. + * + * @private + * @type {number} + */ + var updateTimeout = null; + + /** + * The browser timestamp of the last time that currentPosition was updated + * while playing, or null if the recording is not currently playing. + * + * @private + * @type {number} + */ + var lastUpdateTimestamp = null; + /** * Parses all Guacamole instructions within the given blob, invoking * the provided instruction callback for each such instruction. Once @@ -482,8 +504,9 @@ Guacamole.SessionRecording = function SessionRecording(source) { }; /** - * Searches through the given region of frames for the frame having a - * relative timestamp closest to the timestamp given. + * Searches through the given region of frames for the closest frame + * having a relative timestamp less than or equal to the to the given + * relative timestamp. * * @private * @param {!number} minIndex @@ -504,9 +527,22 @@ Guacamole.SessionRecording = function SessionRecording(source) { */ var findFrame = function findFrame(minIndex, maxIndex, timestamp) { - // Do not search if the region contains only one element - if (minIndex === maxIndex) - return minIndex; + // The region has only one frame - determine if it is before or after + // the requested timestamp + if (minIndex === maxIndex) { + + // Skip checking if this is the very first frame - no frame could + // possibly be earlier + if (minIndex === 0) + return minIndex; + + // If the closest frame occured after the requested timestamp, + // return the previous frame, which will be the closest with a + // timestamp before the requested timestamp + if (toRelativeTimestamp(frames[minIndex].timestamp) > timestamp) + return minIndex - 1; + + } // Split search region into two halves var midIndex = Math.floor((minIndex + maxIndex) / 2); @@ -599,10 +635,11 @@ Guacamole.SessionRecording = function SessionRecording(source) { // Replay any applicable incremental frames var continueReplay = function continueReplay() { - // Notify of changes in position + // Set the current position and notify changes if (recording.onseek && currentFrame > startIndex) { - recording.onseek(toRelativeTimestamp(frames[currentFrame].timestamp), - currentFrame - startIndex, index - startIndex); + currentPosition = toRelativeTimestamp(frames[currentFrame].timestamp); + recording.onseek(currentPosition, currentFrame - startIndex, + index - startIndex); } // Cancel seek if aborted @@ -623,8 +660,18 @@ Guacamole.SessionRecording = function SessionRecording(source) { // immediately if no delay was requested var continueAfterRequiredDelay = function continueAfterRequiredDelay() { var delay = nextRealTimestamp ? Math.max(nextRealTimestamp - new Date().getTime(), 0) : 0; - if (delay) - window.setTimeout(continueReplay, delay); + if (delay) { + + // Clear any already-scheduled update before scheduling again + // to avoid multiple updates in flight at the same time + updateTimeout && clearTimeout(updateTimeout); + + // Schedule with the appropriate delay + updateTimeout = window.setTimeout(function timeoutComplete() { + updateTimeout = null; + continueReplay(); + }, delay); + } else continueReplay(); }; @@ -653,7 +700,7 @@ Guacamole.SessionRecording = function SessionRecording(source) { } continueAfterRequiredDelay(); - + }; /** @@ -678,20 +725,62 @@ Guacamole.SessionRecording = function SessionRecording(source) { */ var continuePlayback = function continuePlayback() { + // Do not continue playback if the recording is paused + if (!recording.isPlaying()) + return; + // If frames remain after advancing, schedule next frame if (currentFrame + 1 < frames.length) { // 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; + // The position at which the next frame should be rendered + var nextFramePosition = toRelativeTimestamp(next.timestamp); - // Advance to next frame after enough time has elapsed - seekToFrame(currentFrame + 1, function frameDelayElapsed() { - continuePlayback(); - }, nextRealTimestamp); + // The position at which the refresh interval would induce an + // update to the current recording position, rounded to the nearest + // whole multiple of refreshInterval to ensure consistent timing + // for refresh intervals even with inconsistent frame timing + var nextRefreshPosition = refreshInterval > 0 ? refreshInterval * ( + Math.floor((currentPosition + refreshInterval) / refreshInterval) + ) : nextFramePosition; + + // If the next frame will occur before the next refresh interval, + // advance to the frame after the appropriate delay + if (nextFramePosition <= nextRefreshPosition) + + seekToFrame(currentFrame + 1, function frameDelayElapsed() { + + // Record when the timestamp was updated and continue on + lastUpdateTimestamp = Date.now(); + continuePlayback(); + + }, Date.now() + (nextFramePosition - currentPosition)); + + // The position needs to be incremented before the next frame + else { + + // Clear any existing update timeout + updateTimeout && window.clearTimeout(updateTimeout); + + updateTimeout = window.setTimeout(function incrementPosition() { + + updateTimeout = null; + + // Update the position + currentPosition = nextRefreshPosition; + + // Notifiy the new position using the onseek handler + if (recording.onseek) + recording.onseek(currentPosition); + + // Record when the timestamp was updated and continue on + lastUpdateTimestamp = Date.now(); + continuePlayback(); + + }, nextRefreshPosition - currentPosition); + } } @@ -839,7 +928,7 @@ Guacamole.SessionRecording = function SessionRecording(source) { * true if playback is currently in progress, false otherwise. */ this.isPlaying = function isPlaying() { - return !!startVideoTimestamp; + return !!currentlyPlaying; }; /** @@ -899,13 +988,9 @@ Guacamole.SessionRecording = function SessionRecording(source) { if (recording.onplay) recording.onplay(); - // Store timestamp of playback start for relative scheduling of - // future frames - var next = frames[currentFrame + 1]; - startVideoTimestamp = next.timestamp; - startRealTimestamp = new Date().getTime(); - // Begin playback of video + currentlyPlaying = true; + lastUpdateTimestamp = Date.now(); continuePlayback(); } @@ -957,8 +1042,23 @@ Guacamole.SessionRecording = function SessionRecording(source) { }; - // Perform seek - seekToFrame(findFrame(0, frames.length - 1, position), seekCallback); + // Find the index of the closest frame at or before the requested position + var closestFrame = findFrame(0, frames.length - 1, position); + + // Seek to the closest frame before or at the requested position + seekToFrame(closestFrame, function seekComplete() { + + // Update the current position to the requested position + // and invoke the the onseek callback. Note that this is the + // position provided to this function, NOT the position of the + // frame that was just seeked + currentPosition = position; + if (recording.onseek) + recording.onseek(position); + + seekCallback(); + + }); }; @@ -988,6 +1088,14 @@ Guacamole.SessionRecording = function SessionRecording(source) { // Abort any in-progress seek / playback abortSeek(); + // Cancel any currently-scheduled updates + updateTimeout && clearTimeout(updateTimeout); + + // Increment the current position by the amount of time passed since the + // the last time it was updated + currentPosition += Date.now() - lastUpdateTimestamp; + lastUpdateTimestamp = null; + // Stop playback only if playback is in progress if (recording.isPlaying()) { @@ -996,8 +1104,7 @@ Guacamole.SessionRecording = function SessionRecording(source) { recording.onpause(); // Playback is stopped - startVideoTimestamp = null; - startRealTimestamp = null; + currentlyPlaying = false; } @@ -1124,4 +1231,4 @@ Guacamole.SessionRecording._PlaybackTunnel = function _PlaybackTunnel() { tunnel.oninstruction(opcode, args); }; -}; \ No newline at end of file +}; diff --git a/guacamole/src/main/frontend/src/app/player/directives/player.js b/guacamole/src/main/frontend/src/app/player/directives/player.js index 7b3e08792..f2a008fee 100644 --- a/guacamole/src/main/frontend/src/app/player/directives/player.js +++ b/guacamole/src/main/frontend/src/app/player/directives/player.js @@ -96,6 +96,14 @@ angular.module('player').directive('guacPlayer', ['$injector', function guacPlay config.controller = ['$scope', '$element', '$injector', function guacPlayerController($scope) { + /** + * The minimum number of milliseconds that should occur between updates + * to the progress indicator. + * + * @type {number} + */ + const PROGRESS_REFRESH_INTERVAL = 1000; + /** * Guacamole.SessionRecording instance to be used to playback the * session recording given via $scope.src. If the recording has not @@ -303,7 +311,8 @@ angular.module('player').directive('guacPlayer', ['$injector', function guacPlay // Otherwise, begin loading the provided recording else { - $scope.recording = new Guacamole.SessionRecording(src); + $scope.recording = new Guacamole.SessionRecording( + src, PROGRESS_REFRESH_INTERVAL); // Begin downloading the recording $scope.recording.connect(); From 55eabaee558fde0e5acd739f3faeb67fd0df717c Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Tue, 13 Jun 2023 18:00:05 +0000 Subject: [PATCH 2/4] GUACAMOLE-1803: Fix potential sulf-suppression error. --- .../connection/HistoryConnectionRecord.java | 22 ++++++++++++++++++- 1 file changed, 21 insertions(+), 1 deletion(-) diff --git a/extensions/guacamole-history-recording-storage/src/main/java/org/apache/guacamole/history/connection/HistoryConnectionRecord.java b/extensions/guacamole-history-recording-storage/src/main/java/org/apache/guacamole/history/connection/HistoryConnectionRecord.java index fbc7bdf08..f1c53ff87 100644 --- a/extensions/guacamole-history-recording-storage/src/main/java/org/apache/guacamole/history/connection/HistoryConnectionRecord.java +++ b/extensions/guacamole-history-recording-storage/src/main/java/org/apache/guacamole/history/connection/HistoryConnectionRecord.java @@ -131,7 +131,9 @@ public class HistoryConnectionRecord extends DelegatingConnectionRecord { */ private boolean isSessionRecording(File file) { - try (Reader reader = new InputStreamReader(new FileInputStream(file), StandardCharsets.UTF_8)) { + Reader reader = null; + try { + reader = new InputStreamReader(new FileInputStream(file), StandardCharsets.UTF_8); GuacamoleReader guacReader = new ReaderGuacamoleReader(reader); if (guacReader.readInstruction() != null) @@ -148,6 +150,24 @@ public class HistoryConnectionRecord extends DelegatingConnectionRecord { + "identified as it cannot be read: {}", file, e.getMessage()); logger.debug("Possible session recording \"{}\" could not be read.", file, e); } + finally { + + // If the reader was successfully constructed, close it + if (reader != null) { + + try { + reader.close(); + } + + catch (IOException e) { + logger.warn("Unexpected error closing recording file \"{}\": {}", + file, e.getMessage()); + logger.debug("Session recording file \"{}\" could not be closed.", + file, e); + } + + } + } return false; From e4d6fe10124ca6e4a9cebb86992b78bcbccdbf31 Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Tue, 13 Jun 2023 19:58:10 +0000 Subject: [PATCH 3/4] GUACAMOLE-1803: Default to 1000 millisecond refresh interval. --- .../src/main/webapp/modules/SessionRecording.js | 5 +++++ .../main/frontend/src/app/player/directives/player.js | 11 +---------- 2 files changed, 6 insertions(+), 10 deletions(-) diff --git a/guacamole-common-js/src/main/webapp/modules/SessionRecording.js b/guacamole-common-js/src/main/webapp/modules/SessionRecording.js index 37270a5fb..02a083bba 100644 --- a/guacamole-common-js/src/main/webapp/modules/SessionRecording.js +++ b/guacamole-common-js/src/main/webapp/modules/SessionRecording.js @@ -37,9 +37,14 @@ var Guacamole = Guacamole || {}; * position through the provided onseek() callback. If non-positive, this * parameter will be ignored, and the recording position will only be * updated when seek requests are made, or when new frames are rendered. + * If not specified, refreshInterval will default to 1000 milliseconds. */ Guacamole.SessionRecording = function SessionRecording(source, refreshInterval) { + // Default the refresh interval to 1 second if not specified otherwise + if (refreshInterval === undefined) + refreshInterval = 1000; + /** * Reference to this Guacamole.SessionRecording. * diff --git a/guacamole/src/main/frontend/src/app/player/directives/player.js b/guacamole/src/main/frontend/src/app/player/directives/player.js index f2a008fee..7b3e08792 100644 --- a/guacamole/src/main/frontend/src/app/player/directives/player.js +++ b/guacamole/src/main/frontend/src/app/player/directives/player.js @@ -96,14 +96,6 @@ angular.module('player').directive('guacPlayer', ['$injector', function guacPlay config.controller = ['$scope', '$element', '$injector', function guacPlayerController($scope) { - /** - * The minimum number of milliseconds that should occur between updates - * to the progress indicator. - * - * @type {number} - */ - const PROGRESS_REFRESH_INTERVAL = 1000; - /** * Guacamole.SessionRecording instance to be used to playback the * session recording given via $scope.src. If the recording has not @@ -311,8 +303,7 @@ angular.module('player').directive('guacPlayer', ['$injector', function guacPlay // Otherwise, begin loading the provided recording else { - $scope.recording = new Guacamole.SessionRecording( - src, PROGRESS_REFRESH_INTERVAL); + $scope.recording = new Guacamole.SessionRecording(src); // Begin downloading the recording $scope.recording.connect(); From 7176ccce682202fcc0f804d52e9fba5efeee5a83 Mon Sep 17 00:00:00 2001 From: James Muehlner Date: Thu, 15 Jun 2023 17:51:54 +0000 Subject: [PATCH 4/4] GUACAMOLE-1803: Restore logic to account for frame rendering delays. --- .../main/webapp/modules/SessionRecording.js | 35 ++++++++++++++----- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/guacamole-common-js/src/main/webapp/modules/SessionRecording.js b/guacamole-common-js/src/main/webapp/modules/SessionRecording.js index 02a083bba..17b4ea7cb 100644 --- a/guacamole-common-js/src/main/webapp/modules/SessionRecording.js +++ b/guacamole-common-js/src/main/webapp/modules/SessionRecording.js @@ -149,12 +149,22 @@ Guacamole.SessionRecording = function SessionRecording(source, refreshInterval) var currentFrame = -1; /** - * True if the player is currently playing, or false otherwise. + * The timestamp of the frame when playback began, in milliseconds. If + * playback is not in progress, this will be null. * * @private - * @type {boolean} + * @type {number} */ - var currentlyPlaying = null; + var startVideoTimestamp = null; + + /** + * The real-world timestamp when playback began, in milliseconds. If + * playback is not in progress, this will be null. + * + * @private + * @type {number} + */ + var startRealTimestamp = null; /** * The current position within the recording, in milliseconds. @@ -740,8 +750,9 @@ Guacamole.SessionRecording = function SessionRecording(source, refreshInterval) // Pull the upcoming frame var next = frames[currentFrame + 1]; - // The position at which the next frame should be rendered - var nextFramePosition = toRelativeTimestamp(next.timestamp); + // The position at which the next frame should be rendered, taking + // into account any accumulated delays from rendering frames so far + var nextFramePosition = next.timestamp - startVideoTimestamp + startRealTimestamp; // The position at which the refresh interval would induce an // update to the current recording position, rounded to the nearest @@ -933,7 +944,7 @@ Guacamole.SessionRecording = function SessionRecording(source, refreshInterval) * true if playback is currently in progress, false otherwise. */ this.isPlaying = function isPlaying() { - return !!currentlyPlaying; + return !!startVideoTimestamp; }; /** @@ -993,8 +1004,13 @@ Guacamole.SessionRecording = function SessionRecording(source, refreshInterval) if (recording.onplay) recording.onplay(); + // Store timestamp of playback start for relative scheduling of + // future frames + var next = frames[currentFrame + 1]; + startVideoTimestamp = next.timestamp; + startRealTimestamp = Date.now(); + // Begin playback of video - currentlyPlaying = true; lastUpdateTimestamp = Date.now(); continuePlayback(); @@ -1099,7 +1115,6 @@ Guacamole.SessionRecording = function SessionRecording(source, refreshInterval) // Increment the current position by the amount of time passed since the // the last time it was updated currentPosition += Date.now() - lastUpdateTimestamp; - lastUpdateTimestamp = null; // Stop playback only if playback is in progress if (recording.isPlaying()) { @@ -1109,7 +1124,9 @@ Guacamole.SessionRecording = function SessionRecording(source, refreshInterval) recording.onpause(); // Playback is stopped - currentlyPlaying = false; + lastUpdateTimestamp = null; + startVideoTimestamp = null; + startRealTimestamp = null; }