auto-refresh kicks in too early

[totaam]

Issue migrated from trac ticket # 694

component: server | priority: blocker | resolution: fixed

2014-09-25 04:14:27: totaam created the issue

---

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.

[totaam]

2014-09-26 06:59:08: totaam uploaded file refresh-fix.patch (9.4 KiB)

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

[totaam]

2014-09-26 17:25:30: totaam changed owner from antoine to afarr

[totaam]

2014-09-26 17:25:30: totaam commented

---

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 (better sub-window encoding: detect regions and use sub video encoder #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?

[totaam]

2014-09-27 02:23:48: afarr commented

---

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 7467 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.

[totaam]

2014-09-28 12:10:09: totaam changed status from new to closed

[totaam]

2014-09-28 12:10:09: totaam changed resolution from ** to fixed

[totaam]

2014-09-28 12:10:09: totaam commented

---

Backport in 7819. This will do for now.

[totaam]

2015-01-06 12:37:26: antoine commented

---

Related: r8407, r8410.