From 869c738cf34dea4ae831b184ebdf7df92219e05d Mon Sep 17 00:00:00 2001 From: pletch <17855860+pletch@users.noreply.github.com> Date: Tue, 8 Sep 2026 18:57:51 -0400 Subject: [PATCH] fix(h264): release the sync gate at the depth it waits on, not at zero waitForPending() holds the Guacamole sync response while pendingDecodes exceeds MAX_PIPELINE_DEPTH, but resolveIfIdle() only fired its resolvers at pendingDecodes <= 0. So once the backlog crossed the depth it had to drain completely before a sync could go out, and a session decoding continuously never reaches zero. Every sync therefore waited out its full timeout, logging "sync wait timeout" and reporting that much processing lag however fast the client was really going. guacd paces frames against the client's processing lag, so the session runs slow while the client is not in fact behind. The function's own documentation already said the callback fires "when the backlog is within the allowed pipeline depth"; only the implementation disagreed. It shows up first on AVC444, where a picture is two access units and the backlog therefore runs twice as deep for the same frame rate, making it far less likely to touch zero between frames. That presents as AVC444 being expensive rather than as a threshold mismatch, so an A/B that varies the server between AVC444 and AVC420 while holding the client constant cannot tell the two apart. --- static/guac/H264Decoder.js | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/static/guac/H264Decoder.js b/static/guac/H264Decoder.js index 7327b31..7534faf 100644 --- a/static/guac/H264Decoder.js +++ b/static/guac/H264Decoder.js @@ -134,12 +134,26 @@ Guacamole.H264Decoder = function H264Decoder(display) { var flushResolvers = []; /** - * If the backlog has drained, fire and clear all flush resolvers. + * If the backlog is back within the pipeline depth, fire and clear all + * flush resolvers. + * + * The threshold has to be the one waitForPending() gates on. Releasing + * only at zero meant that once the backlog exceeded the depth it had to + * drain completely before a sync could go out, and a session decoding + * continuously never reaches zero -- so every sync waited out its full + * timeout instead, reporting that much processing lag however fast the + * client was actually going. guacd paces frames against that figure, so + * the session runs slow while the client is not in fact behind. + * + * It shows up first on AVC444, where a picture is two access units and + * the backlog is therefore twice as deep for the same frame rate, far + * less likely to touch zero between frames -- which looks like AVC444 + * being expensive rather than like a threshold mismatch. * * @private */ function resolveIfIdle() { - if (pendingDecodes <= 0 && flushResolvers.length > 0) { + if (pendingDecodes <= MAX_PIPELINE_DEPTH && flushResolvers.length > 0) { var resolvers = flushResolvers; flushResolvers = []; for (var i = 0; i < resolvers.length; i++)