mirror of
https://github.com/sol1/rustguac.git
synced 2026-10-24 20:52:24 +00:00
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.
This commit is contained in:
@@ -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++)
|
||||
|
||||
Reference in New Issue
Block a user