mirror of
https://github.com/sol1/rustguac.git
synced 2026-10-24 20:52:24 +00:00
fix(h264): size aux snapshots from the render, keep the combine watchdog
Review notes from the upstream issue thread: - The auxiliary-view snapshot was sized to the auxiliary frame's own dimensions. The v1 chroma layout pads that view to a multiple of 16 rows, so the snapshot could be taller than the picture the renderer draws; with no rects the whole thing is blitted, painting a blank strip below the image. Size it from the rendered canvas instead. - The per-frame watchdog was cleared before combineFrame() ran, so the 4:4:4 path had no timeout despite the comment saying otherwise: a copyTo() that never settled would hold the ordered display queue with nothing able to release it. Clear it once the frame is done with instead -- in the finally for the synchronous paths, and after the copy settles for the combine. - Drop the unreachable view != 0 branch after the combine hand-off. - '0' no longer coerces to false in override(), so ?h264ChromaFilter=0 sets threshold zero rather than switching the filter off. Boolean overrides are unaffected: they coerce, and 0 is falsy. - Drop the stale stats() reference from client.html; the decoder has no such method.
This commit is contained in:
+1
-1
@@ -361,7 +361,7 @@
|
||||
|
||||
var tunnel = new Guacamole.WebSocketTunnel(wsUrl);
|
||||
var client = new Guacamole.Client(tunnel);
|
||||
// Expose for console debugging (e.g. client._h264Decoder.stats())
|
||||
// Expose for console debugging
|
||||
window.__guac_client = client;
|
||||
setupClient(client, tunnel);
|
||||
}
|
||||
|
||||
+46
-21
@@ -165,6 +165,20 @@ Guacamole.H264Decoder = function H264Decoder(display) {
|
||||
resolveIfIdle();
|
||||
}
|
||||
|
||||
/**
|
||||
* Cancels a frame's decode watchdog, if it is still armed.
|
||||
*
|
||||
* @private
|
||||
* @param {object} frameState
|
||||
* The frame's pending state, or null.
|
||||
*/
|
||||
function clearWatchdog(frameState) {
|
||||
if (frameState && frameState.watchdog) {
|
||||
clearTimeout(frameState.watchdog);
|
||||
frameState.watchdog = null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Canvases available for reuse as frame snapshots. A snapshot is held from
|
||||
* decode until its draw task runs, so several are live at once and one
|
||||
@@ -382,7 +396,11 @@ Guacamole.H264Decoder = function H264Decoder(display) {
|
||||
if (value === null || value === undefined)
|
||||
return undefined;
|
||||
|
||||
if (value === 'off' || value === 'false' || value === '0')
|
||||
/* '0' is deliberately not in that list: it is a valid threshold for
|
||||
* h264ChromaFilter, and an override that takes a number has to be
|
||||
* able to take zero. It still switches a boolean override off, since
|
||||
* callers coerce, and 0 is falsy. */
|
||||
if (value === 'off' || value === 'false')
|
||||
return false;
|
||||
if (value === 'on' || value === 'true')
|
||||
return true;
|
||||
@@ -560,7 +578,14 @@ Guacamole.H264Decoder = function H264Decoder(display) {
|
||||
if (frameState.settled)
|
||||
return;
|
||||
|
||||
var snapshot = acquireCanvas(pictureW, pictureH);
|
||||
/* Sized from the rendered canvas rather than from this frame:
|
||||
* the v1 chroma layout pads the auxiliary view to a multiple of
|
||||
* 16 rows, so an auxiliary frame's own dimensions can exceed the
|
||||
* picture's. The renderer draws at the main view's size, leaving
|
||||
* anything below that in a taller snapshot blank -- and with no
|
||||
* rects the whole snapshot is blitted, painting that blank strip
|
||||
* over the bottom of the display. */
|
||||
var snapshot = acquireCanvas(rendered.width, rendered.height);
|
||||
snapshot.getContext('2d').drawImage(rendered, 0, 0);
|
||||
frameState.canvas = snapshot;
|
||||
|
||||
@@ -585,6 +610,10 @@ Guacamole.H264Decoder = function H264Decoder(display) {
|
||||
|
||||
releaseBuffer(buffer);
|
||||
|
||||
/* The copy has settled, one way or the other; there is nothing
|
||||
* left for the watchdog to cover. */
|
||||
clearWatchdog(frameState);
|
||||
|
||||
if (frameState.onReady)
|
||||
frameState.onReady();
|
||||
|
||||
@@ -666,11 +695,6 @@ Guacamole.H264Decoder = function H264Decoder(display) {
|
||||
if (!frameState)
|
||||
return;
|
||||
|
||||
if (frameState.watchdog) {
|
||||
clearTimeout(frameState.watchdog);
|
||||
frameState.watchdog = null;
|
||||
}
|
||||
|
||||
/* An auxiliary view means this is an AVC444 stream, so
|
||||
* its chroma can be recovered. Switch over for the frames
|
||||
* that follow; this one cannot be combined, because the
|
||||
@@ -703,11 +727,6 @@ Guacamole.H264Decoder = function H264Decoder(display) {
|
||||
return;
|
||||
}
|
||||
|
||||
/* No auxiliary view in this stream, or combining is off:
|
||||
* the main view is a complete picture on its own. */
|
||||
if (frameState.view !== 0)
|
||||
return;
|
||||
|
||||
/* Snapshot to a canvas and release the VideoFrame before
|
||||
* returning, rather than holding it until the draw task
|
||||
* runs. Holding frames until their scheduled draw exhausts
|
||||
@@ -742,9 +761,21 @@ Guacamole.H264Decoder = function H264Decoder(display) {
|
||||
/* Null when combineFrame() took ownership: it closes the
|
||||
* frame once its plane copy has settled, and releases the
|
||||
* task itself. */
|
||||
if (frame)
|
||||
if (frame) {
|
||||
|
||||
/* Cleared here rather than on the way in, because the
|
||||
* combine path holds the frame across an asynchronous
|
||||
* copyTo() that nothing else times out: clearing the
|
||||
* watchdog before handing the frame over would leave a
|
||||
* copy that never settles holding the ordered display
|
||||
* queue with nothing able to release it.
|
||||
* combineFrame() clears it once the copy settles. */
|
||||
clearWatchdog(frameState);
|
||||
|
||||
frame.close();
|
||||
|
||||
}
|
||||
|
||||
/* Released here rather than after the try, because the
|
||||
* early returns above exit the function once this finally
|
||||
* has run -- they do not fall through to code following
|
||||
@@ -911,10 +942,7 @@ Guacamole.H264Decoder = function H264Decoder(display) {
|
||||
* frameState is undefined if the throw came from constructing the
|
||||
* chunk, before anything was registered. */
|
||||
if (frameState) {
|
||||
if (frameState.watchdog) {
|
||||
clearTimeout(frameState.watchdog);
|
||||
frameState.watchdog = null;
|
||||
}
|
||||
clearWatchdog(frameState);
|
||||
delete pendingFrames[token];
|
||||
|
||||
/* A snapshot already taken for this frame would otherwise be
|
||||
@@ -957,10 +985,7 @@ Guacamole.H264Decoder = function H264Decoder(display) {
|
||||
|
||||
delete pendingFrames[token];
|
||||
|
||||
if (frameState.watchdog) {
|
||||
clearTimeout(frameState.watchdog);
|
||||
frameState.watchdog = null;
|
||||
}
|
||||
clearWatchdog(frameState);
|
||||
|
||||
var snapshot = frameState.canvas;
|
||||
if (!snapshot) {
|
||||
|
||||
Reference in New Issue
Block a user