xpra icon
Bug tracker and wiki

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#694 closed defect (fixed)

auto-refresh kicks in too early

Reported by: Antoine Martin Owned by: alas
Priority: blocker Milestone: 0.15
Component: server Version: trunk
Keywords: Cc:

Description

Tested with glxspheres, which generates a constant stream of screen updates. The auto-refresh should never get a chance to kick in.

This costs us a lot of bandwidth, and makes the picture stutter.

Attachments (1)

refresh-fix.patch (9.4 KB) - added by Antoine Martin 6 years ago.
this patch fixes the refresh by delaying it for as long as window updates keep coming in

Download all attachments as: .zip

Change History (5)

Changed 6 years ago by Antoine Martin

Attachment: refresh-fix.patch added

this patch fixes the refresh by delaying it for as long as window updates keep coming in

comment:1 Changed 6 years ago by Antoine Martin

Owner: changed from Antoine Martin to alas

Well, r7813 is a bit big, but I couldn't find a clean way to do this without making quite a few changes and adding a lot of comments to explain what is going on. It's not simple because the first half happens in the compression thread, but the actual refresh happens in the UI thread, and we don't want to use locking which would slow things down.

The real problem here is that we have different subsystems all competing for encoding decisions, they all do slightly different things and they all have an impact on each other:

  • window update batching (aggregate window updates together)
  • the dynamic encoding selection logic (choosing the best encoding for any given window update size and speed/quality settings..)
  • the region merging code (so we don't send many small regions)
  • the video region code (#410) - and video encoder restrictions
  • the refresh timer code (here)

What we had before was refreshing too often, but it is possible that with this change we may not refresh enough now.. (at least not as much as before)
If window updates keep coming through, we may keep delaying the window refresh indefinitely.
It tries to avoid that by delaying less for small updates than larger ones, and smaller updates are also less likely to be using lossy encodings in the first place.

Ideally, we could just keep track of every single region we process, when it got updated, at what quality, etc. Then we could easily determine when something needs a refresh. But this could quite quickly end up using up more CPU time and memory than just actually doing the work... (as we already know that the rectangle calculation code isn't very fast: #620)

With the old code, I could see the auto-refresh kicking in despite new full-window updates happening around the same time. Causing a sort of stutter in middle of fast moving frames.
With this new code, I see instead:

refresh_timer_function: rescheduling auto refresh timer with extra delay 292ms
auto refresh:   vp8 screen update (quality=  0), re-scheduling refresh \
    (due in 299ms, 39ms added - sched_delay=300, pct=100, batch=35.6382594128) ...
auto refresh:   vp8 screen update (quality=  0), re-scheduling refresh \
    (due in 299ms, 33ms added - sched_delay=300, pct=100, batch=35.6382594128) ...

etc.

@afarr: can you break it? can you verify things are "better" than they were?

comment:2 Changed 6 years ago by alas

Can't seem to break it, despite some effort.

It is better, with windows client at least (and a relatively weak windows 7 client host at that).

  • Testing with 0.15.0 r7514 client, both against a 0.14.4 r7467 fedora 20 server and against a 0.15.0 r7794 fedora 20 server I am able to get "fuzzy" text while scrolling (especially on a page that also has video) as well as choppy "cuts" in the video rendering while scrolling.
  • Testing with 0.15.0 r7804 client against same servers the behavior was noticeably better, but there was a fair amount of "fuzzy" text rendering and it was still possible, though less easy, to get the choppy "cuts" in the scrolling video.
  • Testing with 0.15.0 r7813 against the same servers the behavior was noticeably better yet... the text "fuzziness" resolved itself faster and I couldn't get any of the choppy "cuts" on the video.

I'll test again with an osx client when I get the chance.

Last edited 6 years ago by Antoine Martin (previous) (diff)

comment:3 Changed 6 years ago by Antoine Martin

Resolution: fixed
Status: newclosed

Backport in r7819. This will do for now.

comment:4 Changed 6 years ago by Antoine Martin

Related: r8407, r8410.

Last edited 6 years ago by Antoine Martin (previous) (diff)
Note: See TracTickets for help on using tickets.