Skip to content

Pick the frame rate from the link, never reduce resolution - #116

Open
londospark wants to merge 2 commits into
omacom:mainfrom
londospark:adaptive-fps
Open

londospark wants to merge 2 commits into
omacom:mainfrom
londospark:adaptive-fps

Conversation

@londospark

@londospark londospark commented Nov 1, 2026 •

Copy link
Copy Markdown

Fixes #113

What happens

The ladder accepts a rung once each stream gets FLOOR_BPP = 0.03 bits per pixel per frame. On a 2560x1600 Dual420 stream at 15 Mbit/s, that keeps 60 fps with blocky text, where sharp text wants about 0.1. The rungs are also coarse (60, 30, 15, 15, 10, 10, 5), and the bottom two halve the resolution.

The change

The ladder now chooses a Rung { aux, fps } directly from the measured rate:

fps = 0.85 × rate / (streams × pixels × quality_bpp)
  • Quality bar: quality_bpp defaults to 0.08 (QUALITY_BPP). The new server flag --quality-bpp changes it.
  • Order of trade-offs: the 4:4:4 chroma stream stays while that frame rate is at least 30 fps. Below 30, the stream drops to Single420 and the frame rate is recomputed, down to a floor of 5 fps. The rate never goes above the session's ceiling.
    • A Dual420-only client keeps both streams and trades frame rate alone.
  • Resolution: never reduced. apply_stream_params and fall_back_to_cpu always use base_stream, and the scale field is gone.
  • Hysteresis: unchanged. Stepping up still needs 1.25× headroom and the hold, and a flap still doubles the hold. Stepping down still needs 0.9× and 2 s, or congestion. In addition, no move is made for a frame-rate change under 15%, so the cadence doesn't wander with each rate sample. A frame-rate-only step still re-paces without rebuilding the encoder.
  • Bitrate ceiling: the controller's hard cap goes from 80 to 250 Mbit/s. The per-stream bound pixels × streams × fps × 0.1 is unchanged, so at 60 fps this only matters for streams above 4K.

At the current 60 fps ceiling, this mostly changes what happens on slower links. #117 raises the ceiling to the client's refresh rate, and the continuous frame rate is what keeps that from jumping to 240 fps with too few bits.

Testing

  • scripts/check.sh passes. The ladder tests are rewritten for rungs, with new ones covering:
    • selection by rate
    • a higher quality bar giving a lower frame rate
    • a Dual420-only client
    • small rate changes not moving the frame rate
    • a lower ceiling applying without a step
    • the startup and flap behaviour
  • On an RTX 5080 server with a laptop client on 5 GHz Wi-Fi, at 2536x1496: text stays sharp, and the frame rate settles where the link allows instead of oscillating between 60 and 15.
  • scripts/e2e.sh not run.

🤖 Generated with Claude Code

The ladder judged a rung affordable at 0.03 bits per pixel per stream, so
a 2560x1600 Dual420 stream at 15 Mbit/s ran at 60 fps with visibly blocky
text; sharp desktop text wants about 0.1. The rungs were also coarse
(60, 30, 15, 10, 5), and the bottom two halved the resolution.

The ladder now chooses a rung { aux, fps } directly: the highest frame
rate the measured rate pays for at --quality-bpp (0.08 by default),
keeping the 4:4:4 chroma stream while that is at least 30 fps, then
Single420, down to 5 fps. The stream is always the window's size.
Hysteresis is unchanged, plus no move smaller than 15% of the frame rate.
The bitrate ceiling is raised from 80 to 250 Mbit/s (still bounded by
pixels x streams x fps x 0.1) so a large wired stream can pay for full
quality.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@greptile-apps

greptile-apps Bot commented Nov 1, 2026 •

Copy link
Copy Markdown

RetriggerView in GreptileConfidence Score: 4/5

[Medium impact] The changes affect the server's video quality adaptation for remote desktop sessions.

Run the required hardware checks before merging; no new code defect was established.

Findings

  1. P2 Required hardware checks are missing ▶

Summary

The PR chooses frame rate from the measured link rate and keeps the full fitted resolution.

  • The server adjusts frame rate and chroma to match the link.
  • The stream keeps its fitted resolution as link quality changes.
  • The bitrate controller keeps headroom for restoring two streams.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[Measured link rate] --> B[Choose affordable frame rate]
    B --> C{Dual420 affordable at 30 fps?}
    C -- Yes --> D[Keep both streams]
    C -- No, client supports Single420 --> E[Use one stream and recompute frame rate]
    D --> F[Apply holds and minimum change size]
    E --> F
    F --> G[Keep full fitted resolution]
Loading

Reviews (2) · Last reviewed commit: "Keep the bitrate ceiling at the peak str..." · Reviewed by Greptile

Comment thread crates/gliff-server/src/rate.rs Resolved
…return

After the ladder dropped to Single420, RateController bounded the target
by one stream (6 x pixels at 60 fps), but restoring Dual420 at 30 fps
with 1.25x headroom needs 7.06 x pixels, so a recovered link never got
full chroma back. Bound it by the most streams the session has used;
the per-stream bitrate still divides by the active count.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Comment on lines +1011 to +1013
// The stream is always the full fit size: the ladder trades frame
// rate and chroma, never resolution.
let want = self.base_stream;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Required hardware checks are missing

The PR says scripts/e2e.sh was not run. These changes affect frame pacing, chroma switching, and encoded size. CLAUDE.md requires that check before merging video pipeline changes, so run it and record the result before merging.

Context Used: CLAUDE.md (source)

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The quality ladder keeps the frame rate at 0.03 bits per pixel, so text goes blocky, and its last rungs halve the resolution

1 participant