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:
pletch
2026-08-26 19:07:48 -04:00
parent 691634701e
commit f16eed73ab
2 changed files with 47 additions and 22 deletions
+1 -1
View File
@@ -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
View File
@@ -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) {