Xpra-org / xpra

Persistent remote applications for X11; screen sharing for X11, MacOS and MSWindows.
https://xpra.org/
GNU General Public License v2.0
1.99k stars 169 forks source link

better sub-window encoding: detect regions and use sub video encoder #410

Closed totaam closed 9 years ago

totaam commented 11 years ago

Issue migrated from trac ticket # 410

component: core | priority: critical | resolution: fixed

2013-08-09 07:19:24: totaam created the issue


If you have a large window but only a fraction of that window changes we waste a lot resources. We currently create a video encoder for the whole window and waste a fair bit of time capturing, csc-ing, encoding, (sending), decoding and displaying regions of the screen that have not changed at all (and we know they haven't) [[BR]] We have the "small region" code, which deals with small-ish regions by sending them using another encoding (usually rgb or png/jpeg) but it has its limits: it does not use a video encoder and is limited in size. [[BR]] We should keep track of the damage areas in more detail, including their location and size (easy). Then we can detect regions of the screen that update often (easy-ish) and create a sub video encoder just for those (harder). This could even work when the region is moving, as long as its size stays the same. This is negated somewhat by the fact that when we send the whole window (because of other updates), it will include this region - unless we purposedly blank it out before compression and ask the client to re-assemble the two before displaying the result (hard).

An even better solution to this would be #509 or #510, as this would give us the video area precisely everytime, even before it gets converted to RGB pixels.

totaam commented 10 years ago

2014-02-02 06:35:56: antoine changed status from new to assigned

totaam commented 10 years ago

2014-02-02 06:35:56: antoine edited the issue description

totaam commented 10 years ago

2014-02-10 14:10:19: antoine uploaded file video-subregion-client.patch (4.9 KiB)

allows clients to process video sub-regions

totaam commented 10 years ago

2014-02-10 14:10:39: antoine uploaded file video-subregion-server.patch (15.2 KiB)

server side patch: detect and use video sub-regions

totaam commented 10 years ago

2014-02-10 14:22:22: antoine commented


Lots of preparatory work in:

With the 2 patches attached, it seems to work. Including with the new test app tests.xpra.test_apps.test_videoregions (as long as the server is started with XPRA_FORCE_BATCH=1)

Still TODO:

  • add some tests for the region substraction code (looks right - but too hard to tell for sure by manual inspection)
  • ensure we don't use the window dimensions anywhere in the encoder/csc pipeline creation or scoring methods (looks clean!)
  • test with scaling
  • test with proxy encoding
  • benchmark and profile it

Maybes:

  • take fps into account: discard old damage events?
  • pass subregion down to encoders so we can blank it out if we've sent it as video already?
  • somehow pass an event to the client telling it that we've discarded the video encoder (rather than wait for the window to close, or a new encoding stream to take its place)
  • support multiple regions (hard)
totaam commented 10 years ago

2014-02-11 14:20:40: antoine commented


Merged the uncontroversial bits in:

  • r5429: client side
  • r5430: region handling code and tests
  • r5431: proxy encoder support

The tricky thing about the server side is that applications do stupid repaints... ie: firefox will repaint things on the right hand side of the flash player (even though they don't change..), so we have to be more clever than planned and introduce more heuristics, and region matching code. Work in progress.

totaam commented 10 years ago

2014-02-11 14:52:02: antoine uploaded file video-subregion-v4.patch (14.2 KiB)

remaining server side bits, still needs a proper way to ensure non-video regions don't use the video encoding (which does a re-init and wastes everything)

totaam commented 10 years ago

2014-02-12 04:31:40: antoine uploaded file video-subregion-v5.patch (18.3 KiB)

better patch if a little ugly: caller can provide method for deciding which encoding to use (so we can ensure we don't use video for anything but the current subregion)

totaam commented 10 years ago

2014-02-12 10:14:14: antoine uploaded file video-subregion-v6.patch (25.4 KiB)

ensure we don't end up using the video encoder whilst we still have a video region

totaam commented 10 years ago

2014-02-13 12:21:22: antoine commented


Big commit in r5437 with lots of details, small fix in r5441.

Bigger problem with nvenc in #517 blocks further work on this one.

totaam commented 10 years ago

2014-05-19 12:56:40: totaam changed status from assigned to new

totaam commented 10 years ago

2014-05-19 12:56:40: totaam changed owner from antoine to afarr

totaam commented 10 years ago

2014-05-19 12:56:40: totaam commented


Was actually enabled in 0.12.x and has been working OK so far.

afarr / smo: worth knowing about / testing as part of #419.

totaam commented 10 years ago

2014-06-20 02:00:31: totaam commented


Follow up work in #596, should have been closed by now: done 4 months ago..

totaam commented 10 years ago

2014-06-20 23:11:33: maxmylyn commented


Did some testing with some quality changes from #596#comment:2 : Tested with a r6853 Win7 64-bit client against a r6853 Fedora 20 client (with a -d refresh piped into a .txt):

  • Opened up a YouTube video
  • Started a video
    • The whole webpage renders in a low quality.

After changing the min-quality setting to a higher number, the whole webpage starts rendering much nicer. Lowering min-quality causes the whole webpage to start looking choppy. When the video is paused, the webpage starts to render clearly.

  • Ran another connection(no logs), this time just leaving it on Wikipedia.
    • Hovering over links(causing them to become underlined, with no other webpage changes) caused the whole webpage to become blurry.

It looks like partial refreshes aren't working on the 14.0 r6853 Windows client for sure. Our OSX build currently doesn't have h.264, so it switched over to PNG, which only allowed for lossless rendering. When Windows was forced onto PNG, it behaved identically; drawing only lossless. However, the video played at a lower framerate due to Xpra not being able to keep up.

That being said, I tested with our CentOS client, which had h.264, and I got the exact same behavior. Partial webpage changes are causing a full-screen refresh. Sometimes with quality extremely low, making the window become very blurry.

In addition, I'll leave a comment on #596 as well.

totaam commented 10 years ago

2014-06-20 23:12:05: maxmylyn uploaded file 410refresh.txt (861.6 KiB)

-d refresh from the server

totaam commented 10 years ago

2014-06-21 00:58:04: afarr commented


As mentioned in #596, the above behavior shows with chrome, but with firefox (& mostly on lazarus) the refreshes seem to be behaving as expected.

totaam commented 10 years ago

2014-06-21 04:07:39: totaam changed status from new to closed

totaam commented 10 years ago

2014-06-21 04:07:39: totaam changed resolution from * to fixed*

totaam commented 10 years ago

2014-06-21 04:07:39: totaam commented


This ticket is about xpra detecting that there is video on screen, and where it is. Judging by the log samples in #596, it does that. Another way of verifying this is with:

xpra info | grep "\.video_subregion="

[[BR]]

.. caused the whole webpage to become blurry It looks like partial refreshes aren't working on the 14.0 r6853 Windows client for sure [[BR]] Not necessarily. Just because you don't see anything refreshing visually, does not mean that the browser isn't repainting that part of the screen (often unnecessarily). This belongs in #596 anyway. Closing.

totaam commented 10 years ago

2014-06-21 04:20:18: totaam changed status from closed to reopened

totaam commented 10 years ago

2014-06-21 04:20:18: totaam changed resolution from fixed to **

totaam commented 10 years ago

2014-06-21 04:20:18: totaam commented


Actually re-opening this bug: the issue from #596#comment:4 belongs here instead: not detecting the video region with chrome.

Extracts:

auto refresh: h264 screen update (quality= 37), keeping existing timer \
    (region=rectangle[0, 0, 1450, 894], refresh regions=[R[0, 0, 1450, 894]])

The video region is not detected. All I see is many like this one:

auto refresh: jpeg screen update (quality= 34), scheduling refresh \
    (region=rectangle[0, 0, 213, 26], refresh regions=[R[0, 0, 213, 26]])

Where was the video showing in the window? (213x26 at 0x0 looks like the address bar) Please post the server's -d subregion log output of when the video is playing and not being detected. Also: which page can allow me to reproduce? Which screen settings, etc.

totaam commented 10 years ago

2014-06-21 04:20:30: totaam changed status from reopened to new

totaam commented 10 years ago

2014-06-21 04:20:30: totaam changed owner from afarr to maxmylyn

totaam commented 10 years ago

2014-06-21 05:00:25: maxmylyn commented


I'll attach a screenshot that I took earlier

totaam commented 10 years ago

2014-06-21 05:01:04: maxmylyn uploaded file 410videobeingblurry.png (352.9 KiB)

A screenshot depicting where the video was usually located while being watched in chrome. 410videobeingblurry.png

totaam commented 10 years ago

2014-06-21 05:04:31: totaam commented


Please post more information: see comment:10, comment:9, include -d refresh, -d compress, -d damage, -d encoding. And maybe all of them together -d compress,damage,encoding.

totaam commented 10 years ago

2014-06-21 05:05:35: maxmylyn commented


http://www.youtube.com/watch?v=f488uJAQgmw Here is the video. (a little crude humor, but I find it hilarious)

These were watched running full-screen on a 1080p monitor. The window wasn't entirely full-screen. I moved it to the top left and manually dragged to almost full-screen. Basically 1900-1916 x 1040-1075 pixels. All running in chrome - whatever the latest version is.

I'll get you some logs on Monday. (Unless I can get Xpra running at home - a task I'll get to at some point...)

totaam commented 10 years ago

2014-06-21 05:35:09: totaam changed status from new to assigned

totaam commented 10 years ago

2014-06-21 05:35:09: totaam changed owner from maxmylyn to totaam

totaam commented 10 years ago

2014-06-21 05:35:09: totaam commented


Well, that's interesting, I didn't see it before but now I do (even on a video which didn't have the problems when I had tried it earlier - wtf?).

That's because instead of updating the whole video region as one, chrome sends it in 4 smaller chunks every time:

damage(WindowModel(0x1200001 - ".. - YouTube - Chromium"), 5, 161, 640, 102, {})
damage(WindowModel(0x1200001 - ".. - YouTube - Chromium"), 5, 263, 640, 102, {})
damage(WindowModel(0x1200001 - ".. - YouTube - Chromium"), 5, 365, 640, 102, {})
damage(WindowModel(0x1200001 - ".. - YouTube - Chromium"), 5, 467, 640, 54, {})

With another video (same size), I got a different split:

damage(WindowModel(0x1200001 - ".. - YouTube - Chromium"), 133, 161, 480, 136, {})
damage(WindowModel(0x1200001 - ".. - YouTube - Chromium"), 133, 297, 480, 136, {})
damage(WindowModel(0x1200001 - ".. - YouTube - Chromium"), 133, 433, 480, 88, {})

I have no idea why it does that, but it probably throws the video region detection code off. The code does try to merge regions, but when doing so it increases the threshold (to prevent just merging everything - which isn't helpful) - and it looks like that's enough to fail the detection.

totaam commented 10 years ago

2014-06-21 17:09:34: totaam changed status from assigned to new

totaam commented 10 years ago

2014-06-21 17:09:34: totaam changed owner from totaam to maxmylyn

totaam commented 10 years ago

2014-06-21 17:09:34: totaam commented


Fixes in r6855 + r6856.

Can you still break it?

totaam commented 10 years ago

2014-06-23 20:36:42: maxmylyn commented


Yes still breakable with chrome. Running a Windows 7 64-bit client r6856 against a Fedora 20 r6856 server. The server detects the video region and properly starts sending a chunk of the screen as h.264, but it's slightly off from the actual video. Currently the top half of the video is coming in as lossless rgb, but the bottom half and some of the page below it is coming in as h.264.

I'll attach a screenshot and a -d refresh.

The interesting part starts at 12:33:01. From there you can see it sending h264 along with jpegs at the same time rather than a solid stream of h264 with the occasional jpeg full-refresh.(unless this behavior is expected?)

totaam commented 10 years ago

2014-06-23 20:38:16: maxmylyn uploaded file 410offsetrefresh.txt (375.0 KiB)

-d refresh of this behavior. Interesting bit is at 12:33.

totaam commented 10 years ago

2014-06-24 08:44:19: totaam commented


Damn, that's what the new heuristics in r6855 is supposed to detect (and worked here - will try again to break it on my laptop): we try to merge regions of the same width to make one large video region.

How easily can you trigger this? Does it remain like that for very long?

Can please you post the server debug log with -d subregion when this happens. We need to see why it prefers the smaller region to the larger merged one.

totaam commented 10 years ago

2014-06-24 18:25:16: maxmylyn uploaded file 410subregion.txt (1747.5 KiB)

Booted up chrome, and navigated to a YouTube video (Switzerland from Above - Top Sights). 10:22 - 10:23 was the choppiest before I closed it.

totaam commented 10 years ago

2014-06-24 18:32:17: maxmylyn commented


I attached some logs. Watched a bit of http://www.youtube.com/watch?v=Sfidg3430dc for a few minutes. Around the time-mark of 10:22 is when the video became really choppy and I could see it attempting to make a subregion, but it didn't quite stick. That video is the one I've found that really shows subregioning(is this a word?) really trying the hardest.

Of note: This is temporarily reproducible with Firefox. The only difference is that with Firefox Xpra catches on to the video after a short period and merges the subregions and video playback smooths out.

totaam commented 10 years ago

2014-06-24 22:11:36: totaam commented


Extract from the logs:

2014-06-24 10:22:21,827 identify video: most=4% damage count=\
    {R(282, 121, 1562, 41): AtomicInteger(14),
     R(282, 285, 1562, 41): AtomicInteger(15),
     R(282, 326, 1562, 41): AtomicInteger(15),
     R(282, 490, 1562, 41): AtomicInteger(15),
     R(282, 244, 1562, 41): AtomicInteger(14),
     R(282, 162, 1562, 41): AtomicInteger(14),
     R(282, 203, 1562, 41): AtomicInteger(14),
     R(282, 613, 1562, 41): AtomicInteger(15),
     R(282, 900, 1562, 41): AtomicInteger(16),
     R(282, 818, 1562, 41): AtomicInteger(15),
     R(282, 572, 1562, 41): AtomicInteger(15),
     R(282, 736, 1562, 41): AtomicInteger(15),
     R(282, 531, 1562, 41): AtomicInteger(15),
     R(282, 695, 1562, 41): AtomicInteger(15),
     R(282, 449, 1562, 41): AtomicInteger(15),
     R(282, 367, 1562, 41): AtomicInteger(15),
     R(282, 654, 1562, 41): AtomicInteger(15),
     R(282, 982, 1562, 14): AtomicInteger(16),
     R(282, 777, 1562, 41): AtomicInteger(15),
     R(282, 941, 1562, 41): AtomicInteger(16),
     R(282, 859, 1562, 41): AtomicInteger(16),
     R(282, 408, 1562, 41): AtomicInteger(15)}
(...)
2014-06-24 10:22:23,771 identify video: most=5% damage count={\
     R(282, 121, 1562, 41): AtomicInteger(14),
     R(282, 285, 1562, 41): AtomicInteger(13),
     R(282, 326, 1562, 41): AtomicInteger(13),
     R(282, 490, 1562, 41): AtomicInteger(13),
     R(282, 244, 1562, 41): AtomicInteger(13),
     R(282, 162, 1562, 41): AtomicInteger(14),
     R(282, 203, 1562, 41): AtomicInteger(14),
     R(282, 613, 1562, 41): AtomicInteger(13),
     R(282, 900, 1562, 41): AtomicInteger(17),
     R(282, 818, 1562, 41): AtomicInteger(15),
     R(282, 572, 1562, 41): AtomicInteger(13),
     R(282, 736, 1562, 41): AtomicInteger(13),
     R(282, 531, 1562, 41): AtomicInteger(13),
     R(282, 695, 1562, 41): AtomicInteger(13),
     R(282, 449, 1562, 41): AtomicInteger(13),
     R(282, 367, 1562, 41): AtomicInteger(13),
     R(282, 654, 1562, 41): AtomicInteger(13),
     R(282, 982, 1562, 14): AtomicInteger(18),
     R(282, 777, 1562, 41): AtomicInteger(13),
     R(282, 941, 1562, 41): AtomicInteger(17),
     R(282, 859, 1562, 41): AtomicInteger(17),
     R(282, 408, 1562, 41): AtomicInteger(13)}
2014-06-24 10:22:23,786 identified video region by size (1562x41), using recent match: rectangle[282, 941, 1562, 41]

The code should have been able to identify the region from this.

Am I right in thinking that the video region was quite big, possibly covering more than 70% of the window size? And that the problem did not occur if the video region is smaller than this?

If so, r6857 should fix that.

totaam commented 10 years ago

2014-06-25 00:24:17: maxmylyn commented


Found something of interest. (I'll retest with r6857 in a bit when I get a build in my hands)

Re-ran a test running r6856 Win7 64-bit against a r6856 Fedora 20 server:

  • Booted up chrome
  • Navigated to YouTube
  • Played, watched -d subregion
  • Paused it
    • While the video was playing the output was flooded with no video region, we may use the video encoder for something else
    • While the video was paused the output was flooded with what looked like video detection code.

I re-ran it and piped it into a .txt.

  • 16:05:00ish - 16:05:15ish I had the video playing
  • 16:05:15ish I hit pause
  • 16:05:30ish I hit play again

It looks like it's sending no video region, we may use the video encoder for something else

followed by


2014-06-24 16:05:30,259 identify video: most=18% damage count={R(0, 691, 1024, 44): AtomicInteger(13), R(56, 121, 640, 102): AtomicInteger(29), R(56, 427, 640, 54): AtomicInteger(25), R(56, 325, 640, 102): AtomicInteger(30), R(56, 223, 640, 102): AtomicInteger(30), R(0, 628, 1024, 63): AtomicInteger(13), R(0, 565, 1024, 63): AtomicInteger(13), R(0, 502, 1024, 63): AtomicInteger(13)}
2014-06-24 16:05:30,288 failed to identify a video region

every now and then while the video is playing, and various video detection prints when the video is paused. From what I'm seeing (this can be repro'd using a CentOS client, I'll check Fedora in a bit and update this comment) it's failing to detect the video while it's playing, but once it's paused it's seeing the video. This is on the http://www.youtube.com/watch?v=f488uJAQgmw kollektivet music video running with the 720p quality setting on YouTube with the "small" player.

totaam commented 10 years ago

2014-06-25 00:25:34: maxmylyn uploaded file 410subregiontest2.txt (219.3 KiB)

16:05:00ish video playing until 16:05:15ish at which point it's paused until 16:05:30ish where I hit play again.

totaam commented 10 years ago

2014-06-25 06:58:17: totaam commented


The no video region, we may use the video encoder for something else message is harmless, it means that it is sending a screen update following the no-video-region codepath, which may use the video encoder context for the whole window (if there are enough pixels to justify it) or simply using a non-video encoding.

The video region which is identified when the screen is paused looks wrong (too small), just like before. This should also be fixed in r6857.

totaam commented 10 years ago

2014-06-25 18:02:59: maxmylyn commented


Unfortunately it doesn't help. Re-ran with r6857 Win7 64-bit against r6857 Fedora 20 server; I'll attach a -d subregion log.

edit:

I'm seeing the same behavior as before. Only this time the entire window loses quality while it's hunting for the area of the screen where the video is playing. It seems to eventually settle on the lower half of the video and some more below it. Same behavior as the 410partialblurry.png screenshot.

totaam commented 10 years ago

2014-06-25 18:03:38: maxmylyn uploaded file 410subregionr6857.txt (1227.9 KiB)

Similar test. Again, launching chrome, navigating to YouTube and watching a video.

totaam commented 10 years ago

2014-06-25 19:19:31: totaam commented


What size is your screen? What size is a window? (Attaching xpra info should answer both of these questions and much more) I should be able to test with a 1080p screen later today if that helps me reproduce.

totaam commented 10 years ago

2014-06-25 19:25:03: maxmylyn commented


1080p monitor. Window size is about that big. It's reproducible with a fullscreen window with YouTube running in the small and large viewer.

totaam commented 10 years ago

2014-06-25 19:26:31: maxmylyn uploaded file 410xprainfo.txt (103.6 KiB)

Requested Xpra Info

totaam commented 10 years ago

2014-06-25 22:09:46: totaam commented


I see that the total client size is pretty big:

client.screen[0].size=(3520, 1196)

And the browser window is:

window[3].size=(1920, 1058)

So I should be able to reproduce this with a 1080p screen... but not having much luck so far. It would be good to figure out why, maybe it's batching / latency related.

[[BR]]

The log file is full of:

identified horizontal video region: rectangle[313, 417, 1593, 468]

I guess that the video was not this region then? Did the screen still look like the screenshot from 2 days ago? Can you estimate the location and size of the region which has the player on it?

[[BR]]

I've improved the detection code further in r6862 (+fix in r6863), and we now have a dedicated logger for the code that detects video region: -d regiondetect. (which will make the log file much easier to parse) Does this help?