Copilot commented on code in PR #11119:
URL: https://github.com/apache/cloudstack/pull/11119#discussion_r2185236353
##########
systemvm/agent/noVNC/core/rfb.js:
##########
@@ -865,25 +869,48 @@ export default class RFB extends EventTargetMixin {
// Requests a change of remote desktop size. This message is an extension
// and may only be sent if we have received an ExtendedDesktopSize message
_requestRemoteResize() {
- clearTimeout(this._resizeTimeout);
- this._resizeTimeout = null;
+ if (!this._resizeSession) {
+ return;
+ }
+ if (this._viewOnly) {
+ return;
+ }
+ if (!this._supportsSetDesktopSize) {
+ return;
+ }
- if (!this._resizeSession || this._viewOnly ||
- !this._supportsSetDesktopSize) {
+ // Rate limit to one pending resize at a time
+ if (this._pendingRemoteResize) {
return;
}
+ // And no more than once every 100ms
+ if ((Date.now() - this._lastResize) < 100) {
+ clearTimeout(this._resizeTimeout);
+ this._resizeTimeout =
setTimeout(this._requestRemoteResize.bind(this),
+ 100 - (Date.now() - this._lastResize));
+ return;
+ }
+ this._resizeTimeout = null;
+
const size = this._screenSize();
+ // Do we actually change anything?
+ if (size.w === this._fbWidth && size.h === this._fbHeight) {
+ return;
+ }
+
+ this._pendingRemoteResize = true;
+ this._lastResize = Date.now();
RFB.messages.setDesktopSize(this._sock,
- Math.floor(size.w), Math.floor(size.h),
- this._screenID, this._screenFlags);
+ Math.floor(size.w), Math.floor(size.h),
+ this._screenID, this._screenFlags);
Log.Debug('Requested new desktop size: ' +
- size.w + 'x' + size.h);
+ size.w + 'x' + size.h);
}
- // Gets the size of the available screen
+ // Gets the the size of the available screen
Review Comment:
There's a duplicated word 'the' in the comment. It should read 'Gets the
size of the available screen'.
```suggestion
// Gets the size of the available screen
```
##########
systemvm/agent/noVNC/core/decoders/h264.js:
##########
@@ -0,0 +1,321 @@
+/*
+ * noVNC: HTML5 VNC client
+ * Copyright (C) 2024 The noVNC authors
+ * Licensed under MPL 2.0 (see LICENSE.txt)
+ *
+ * See README.md for usage and integration instructions.
+ *
+ */
+
+import * as Log from '../util/logging.js';
+
+export class H264Parser {
+ constructor(data) {
+ this._data = data;
+ this._index = 0;
+ this.profileIdc = null;
+ this.constraintSet = null;
+ this.levelIdc = null;
+ }
+
+ _getStartSequenceLen(index) {
+ let data = this._data;
+ if (data[index + 0] == 0 && data[index + 1] == 0 && data[index + 2] ==
0 && data[index + 3] == 1) {
+ return 4;
+ }
+ if (data[index + 0] == 0 && data[index + 1] == 0 && data[index + 2] ==
1) {
+ return 3;
+ }
+ return 0;
+ }
+
+ _indexOfNextNalUnit(index) {
+ let data = this._data;
+ for (let i = index; i < data.length; ++i) {
+ if (this._getStartSequenceLen(i) != 0) {
+ return i;
+ }
+ }
+ return -1;
+ }
+
+ _parseSps(index) {
+ this.profileIdc = this._data[index];
+ this.constraintSet = this._data[index + 1];
+ this.levelIdc = this._data[index + 2];
+ }
+
+ _parseNalUnit(index) {
+ const firstByte = this._data[index];
+ if (firstByte & 0x80) {
+ throw new Error('H264 parsing sanity check failed, forbidden zero
bit is set');
+ }
+ const unitType = firstByte & 0x1f;
+
+ switch (unitType) {
+ case 1: // coded slice, non-idr
+ return { slice: true };
+ case 5: // coded slice, idr
+ return { slice: true, key: true };
+ case 6: // sei
+ return {};
+ case 7: // sps
+ this._parseSps(index + 1);
+ return {};
+ case 8: // pps
+ return {};
+ default:
+ Log.Warn("Unhandled unit type: ", unitType);
+ break;
+ }
+ return {};
+ }
+
+ parse() {
+ const startIndex = this._index;
+ let isKey = false;
+
+ while (this._index < this._data.length) {
+ const startSequenceLen = this._getStartSequenceLen(this._index);
+ if (startSequenceLen == 0) {
+ throw new Error('Invalid start sequence in bit stream');
+ }
+
+ const { slice, key } = this._parseNalUnit(this._index +
startSequenceLen);
+
+ let nextIndex = this._indexOfNextNalUnit(this._index +
startSequenceLen);
+ if (nextIndex == -1) {
+ this._index = this._data.length;
+ } else {
+ this._index = nextIndex;
+ }
+
+ if (key) {
+ isKey = true;
+ }
+ if (slice) {
+ break;
+ }
+ }
+
+ if (startIndex === this._index) {
+ return null;
+ }
+
+ return {
+ frame: this._data.subarray(startIndex, this._index),
+ key: isKey,
+ };
+ }
+}
+
+export class H264Context {
+ constructor(width, height) {
+ this.lastUsed = 0;
+ this._width = width;
+ this._height = height;
+ this._profileIdc = null;
+ this._constraintSet = null;
+ this._levelIdc = null;
+ this._decoder = null;
+ this._pendingFrames = [];
+ }
+
+ _handleFrame(frame) {
+ let pending = this._pendingFrames.shift();
+ if (pending === undefined) {
+ throw new Error("Pending frame queue empty when receiving frame
from decoder");
+ }
+
+ if (pending.timestamp != frame.timestamp) {
+ throw new Error("Video frame timestamp mismatch. Expected " +
+ frame.timestamp + " but but got " + pending.timestamp);
+ }
+
+ pending.frame = frame;
+ pending.ready = true;
+ pending.resolve();
+
+ if (!pending.keep) {
+ frame.close();
+ }
+ }
+
+ _handleError(e) {
+ throw new Error("Failed to decode frame: " + e.message);
+ }
+
+ _configureDecoder(profileIdc, constraintSet, levelIdc) {
+ if (this._decoder === null || this._decoder.state === 'closed') {
+ this._decoder = new VideoDecoder({
+ output: frame => this._handleFrame(frame),
+ error: e => this._handleError(e),
+ });
+ }
+ const codec = 'avc1.' +
+ profileIdc.toString(16).padStart(2, '0') +
+ constraintSet.toString(16).padStart(2, '0') +
+ levelIdc.toString(16).padStart(2, '0');
+ this._decoder.configure({
+ codec: codec,
+ codedWidth: this._width,
+ codedHeight: this._height,
+ optimizeForLatency: true,
+ });
+ }
+
+ _preparePendingFrame(timestamp) {
+ let pending = {
+ timestamp: timestamp,
+ promise: null,
+ resolve: null,
+ frame: null,
+ ready: false,
+ keep: false,
+ };
+ pending.promise = new Promise((resolve) => {
+ pending.resolve = resolve;
+ });
+ this._pendingFrames.push(pending);
+
+ return pending;
+ }
+
+ decode(payload) {
+ let parser = new H264Parser(payload);
+ let result = null;
+
+ // Ideally, this timestamp should come from the server, but we'll just
+ // approximate it instead.
+ let timestamp = Math.round(window.performance.now() * 1e3);
+
+ while (true) {
+ let encodedFrame = parser.parse();
+ if (encodedFrame === null) {
+ break;
+ }
+
+ if (parser.profileIdc !== null) {
+ self._profileIdc = parser.profileIdc;
+ self._constraintSet = parser.constraintSet;
+ self._levelIdc = parser.levelIdc;
Review Comment:
Using `self` instead of `this` will cause a ReferenceError at runtime.
Replace `self._profileIdc`, `self._constraintSet`, and `self._levelIdc` with
`this._profileIdc`, `this._constraintSet`, and `this._levelIdc` respectively.
```suggestion
this._profileIdc = parser.profileIdc;
this._constraintSet = parser.constraintSet;
this._levelIdc = parser.levelIdc;
```
--
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: commits-unsubscr...@cloudstack.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
