Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rdpDeferredUpdateCallback with large screens and RFX #524

Closed
moobyfr opened this issue Dec 4, 2016 · 34 comments
Closed

rdpDeferredUpdateCallback with large screens and RFX #524

moobyfr opened this issue Dec 4, 2016 · 34 comments

Comments

@moobyfr
Copy link
Contributor

moobyfr commented Dec 4, 2016

with large screen (1900x1100) and using RFX, I get always a black screen , using current devel branch for xrdp and neutrinoRDP or xfreerdp for client.
The xrdp-sesman being filled by
rdpDeferredUpdateCallback: reschedule rect_id 1 rect_id_ack 0

in xrdp/xrdp_encoder.c:368

   if ((enc->num_crects > 512) || (enc->num_drects > 512))
    {
        return 0;
    }

According to xrdp.log:
xrdp:xrdp_encoder [3403921343]: process_enc_rfx: num_crects 540 num_drects 1

That's explain why RFX doesn't work with large resolution

@tfischer77
Copy link

I'm facing the exact same issue here. Compiling xrdp 0.9.1 without rfx "solves" it...
I'm using mstsc from Windows 7. When using a resolution of 1900x1200 and highest quality (LAN), the screen is "black" and the sesman log is filled with the above mentioned message..
Selecting a lower quality (WAN) is working, but if I try to show a picture with a lot of details in it, the RDP client (mstsc) disconnects immediaetely with a message "protocol error".

@macdanger
Copy link

Hi guys,
any idea how to fix this or work around this? Because working with large resolutions without remoteFX is no fun :-) And higher resolutions than full HD with remoteFX result in a black or empty sceen.
what I noticed is that the modlines in the xorg.conf only go up to 1920x1080. Maybe it helps when we add more modlines for higher resolutions?

@proski
Copy link
Contributor

proski commented Jan 29, 2017

Please try compiling xrdp and xorgxrdp with --without-simd

@moobyfr
Copy link
Contributor Author

moobyfr commented Jan 29, 2017

I've tried, the error is the same. The problem comes when the resolution of the screen needs more than 512 rect (each rect is 64x64) to be covered.

@sylvain121
Copy link

Hi,
I confirm problem. I have 2 large screen (1920x1080) and when I use RFX, I get black screen when connection crash.

@proski
Copy link
Contributor

proski commented Jan 30, 2017

I looked at process_enc_rfx, it's full of magic numbers and it doesn't even return an error or logs anything when enc->num_crects or enc->num_drects exceeds 512. It's not clear where 512 comes from. Is that a protocol limitation? I don't see any references to 512 in librfxcodec sources.

@jsorg71
Copy link
Contributor

jsorg71 commented Jan 30, 2017

That is just a sanity check, it can be increased or removed.

@moobyfr
Copy link
Contributor Author

moobyfr commented Jan 30, 2017

According to technet, max resolution is 32766x*32766 (8192x8192 per display) since RDP8, but there are some limitations on remoteFX: 2560x1600 (rdp8) or 4k (rdp10). This would be 1024 tiles as max for the same support

@jsorg71
Copy link
Contributor

jsorg71 commented Jan 30, 2017

xorgxrdp will eventually process a monitor at a time. Currently the whole desktop is in one message.

@tfischer77
Copy link

I just tried to remove the following lines of code in xrdp/xrdp_encoder.c:

if ((enc->num_crects > 512) || (enc->num_drects > 512))
     {
         return 0;
     }

It immediately solves all problems with large resolutions and rfx. Wouldn't it be good to implement that as a patch to xrdp?

@moobyfr
Copy link
Contributor Author

moobyfr commented Feb 2, 2017

I've updated the limits to 1024 on my test server as it should cover the resolutions limits for RDP10, and I still get some artefacts. But didn't have much time to debug this for the moment

@proski
Copy link
Contributor

proski commented Feb 2, 2017

Please run the patched xrdp under Valgrind to make sure there are no buffer overruns.

@tfischer77
Copy link

I can confirm the artefacts on my test system. Just increasing the rects to 1024 does not solve anything. And there is another problem: Running fast updates on the screen (as an example, run a screensaver under xfce4 like "XLyap") or showing bitmaps with a lot of details in it just crashes the client. Under "Remmina", the screen is frozen, while mstsc throws a "protocol error".
On the xrdp-sesman log, I see tons of messages like this:

rdpDeferredUpdateCallback: reschedule rect_id 74 rect_id_ack 73
rdpDeferredUpdateCallback: reschedule rect_id 74 rect_id_ack 73
rdpDeferredUpdateCallback: reschedule rect_id 74 rect_id_ack 73
rdpDeferredUpdateCallback: reschedule rect_id 74 rect_id_ack 73

"rect_id" number vary depending on the area on the screen where the artefacts are.

@moobyfr
Copy link
Contributor Author

moobyfr commented Feb 6, 2017

The crash itself is perhaps linked to another issue: neutrinolabs/xorgxrdp#5
BTW while increasing the max value to 1024, I have same problem on deferredUpdateCallback, I don't find the magic value or 512 elsewhere in the code btw.

@metalefty
Copy link
Member

@moobyfr I think @tfischer77 's issue is same as mine. I also get "protocol error" in mstsc.

@tfischer77 Does the protocol error stop if color depth is set less than 24 bit (>= 16 bit)?

@tfischer77
Copy link

@metalefty The protocol error stops if I manually select 16bit as resolution in mstsc.

@tfischer77
Copy link

Compiling xrdp without "rfxcodec" (and increasing num_rects to 1024) also works for large resolutions. No artefacts any longer. Is it possible that the error is somewhere in the rfxcodec?
Disabling rfx is not a real option, as it really slows up things.

@metalefty
Copy link
Member

Then, the protocol error issue looks as same as mine. Probably we should open new issue for the issue but let me ask you a few more things. I want you to check if protocol error occurs with each following mstsc settings.

  • ConnectionType=WAN (or lesser), 24 bit color
  • ConnectionType=WAN (or lesser), 16 bit color
  • ConnectionType=LAN, 24 bit color
  • ConnectionType=LAN, 16 bit color

@jsorg71
Copy link
Contributor

jsorg71 commented Feb 10, 2017

I found a few problem with RFX and multimon. I have a couple of PRs to fix.

@tfischer77
Copy link

@metalefty

  • WAN, 24 bit color: All ok, no artefacts, no protocol errors, no frozen screen
  • WAN, 16 bit color: All ok, no artefacts, no protocol errors, no frozen screen
  • LAN, >= 24 bit color: Crash / screen frozen when opening screen saver XLyap in xfce4
  • LAN, 16 bit color: All ok, no artefacts, no protocol errors, no frozen screen
    But I did some more changes in the code (don't know if that solves anything, but at least I do not get artefacts):
@@ -320,21 +320,21 @@ process_enc_rfx(struct xrdp_encoder *sel
     mutex = self->mutex;
     event_processed = self->xrdp_encoder_event_processed;
.
-    if ((enc->num_crects > 512) || (enc->num_drects > 512))
+    if ((enc->num_crects > 1024) || (enc->num_drects > 1024))
     {
         return 0;
     }
.
-    out_data_bytes = 16 * 1024 * 1024;
-    index = 256 + sizeof(struct rfx_tile) * 512 +
-                  sizeof(struct rfx_rect) * 512;
+    out_data_bytes = 32 * 1024 * 1024;
+    index = 1024 + sizeof(struct rfx_tile) * 1024 +
+                  sizeof(struct rfx_rect) * 1024;
     out_data = (char *) g_malloc(out_data_bytes + index, 0);
     if (out_data == 0)
     {
         return 0;
     }
-    tiles = (struct rfx_tile *) (out_data + out_data_bytes + 256);
-    rfxrects = (struct rfx_rect *) (tiles + 512);
+    tiles = (struct rfx_tile *) (out_data + out_data_bytes + 1024);
+    rfxrects = (struct rfx_rect *) (tiles + 1024);
.
     count = enc->num_crects;
     for (index = 0; index < count; index++)

@jsorg71 So, if I understood you correctly, we should wait for your PRs and then check again?

@moobyfr
Copy link
Contributor Author

moobyfr commented Feb 11, 2017

@tfischer77 for the crash with xlyap, this is probably related to another bug:neutrinolabs/xorgxrdp#5

@tfischer77
Copy link

@moobyfr I don' t think so, as the session itself is not crashing. It's just the screen that is frozen. I can even close mstsc and reconnect with 16bit colour and the session is still there. Xlyap rund then with 16bit. As soon as I reconnect with LAN and >=24bit, it stops...

@jsorg71
Copy link
Contributor

jsorg71 commented Feb 12, 2017

I did PR #664 and neutrinolabs/xorgxrdp#77 for this. It's similar to the above, plus now it does a tile set per monitor.

@tfischer77
Copy link

tfischer77 commented Feb 13, 2017

@jsorg71 I tested it, no artefacts any longer, but now Xorg is crashing (when running with my "test program" XLyap... I like it :-) ). The error is like this:


> rdpRRGetInfo:
> rdpClientConSendMsg: overrun error len, 34704 stream size 32868, client count 2
> (EE)
> (EE) Backtrace:
> (EE) 0: Xorg (xorg_backtrace+0x56) [0x7fd1ff555d46]
> (EE) 1: Xorg (0x7fd1ff39f000+0x1baf29) [0x7fd1ff559f29]
> (EE) 2: /lib/x86_64-linux-gnu/libc.so.6 (0x7fd1fd091000+0x350e0) [0x7fd1fd0c60e0]
> (EE) 3: Xorg (0x7fd1ff39f000+0x1b3d7d) [0x7fd1ff552d7d]
> (EE) 4: Xorg (WaitForSomething+0x59a) [0x7fd1ff5533ca]
> (EE) 5: Xorg (0x7fd1ff39f000+0x57211) [0x7fd1ff3f6211]
> (EE) 6: Xorg (0x7fd1ff39f000+0x5b596) [0x7fd1ff3fa596]
> (EE) 7: /lib/x86_64-linux-gnu/libc.so.6 (__libc_start_main+0xf5) [0x7fd1fd0b2b45]
> (EE) 8: Xorg (0x7fd1ff39f000+0x4590e) [0x7fd1ff3e490e]
> (EE)
> (EE) Segmentation fault at address 0x0
> (EE)
> Fatal server error:
> (EE) Caught signal 11 (Segmentation fault). Server aborting
> (EE)
> (EE)
> Please consult the The X.Org Foundation support
>          at http://wiki.x.org
>  for help.
> (EE) Please also check the log file at "/home/remoteusers/admin_tfische3/.local/share/xorg/Xorg.11.log" for additional information.
> (EE)
> 
> 

I understood that I hit bug neutrinolabs/xorgxrdp#5. The error lies here "rdpClientConSendMsg: overrun error len, 34704 stream size 32868, client count 2", so I changed the stream size:

--- xrdp-0.9.2dev1.orig/xorgxrdp/module/rdpClientCon.c
+++ xrdp-0.9.2dev1/xorgxrdp/module/rdpClientCon.c
@@ -162,7 +162,7 @@ rdpClientConGotConnection(ScreenPtr pScr
     make_stream(clientCon->in_s);
     init_stream(clientCon->in_s, 8192);
     make_stream(clientCon->out_s);
-    init_stream(clientCon->out_s, 8192 * 4 + 100);
+    init_stream(clientCon->out_s, 8192 * 16 + 100);
.
     new_sck = g_sck_accept(dev->listen_sck);
     if (new_sck == -1)

Now, Xorg does not crash any longer, but the screen freezes down as soon as I have an "overrun".

@tfischer77
Copy link

OK, found out the root cause: #664 does not fix it completely. I just found out that my very nice screensaver "XLyap" sometimes produces even more than 4096 drects. Just had 4280 drects, and then we run into the same problem as before. Increasing the number of drects and crects to 8192 solves the problem for me. However, I don't feel very comfortable about the code in #664. What happens if in future the number of crects / drects increases?

@jsorg71
Copy link
Contributor

jsorg71 commented Feb 13, 2017

Oh, wow, I was working on limiting crects, I see now that drects can go crazy too. This screen saver must be drawing pixels or small rects all over.
Can you try something like

jay@d7-nuc2:/media/work/home/jay/git/jsorg71/xorgxrdp$ git diff
diff --git a/module/rdpClientCon.c b/module/rdpClientCon.c
index d20fd59..c7dc6ae 100644
--- a/module/rdpClientCon.c
+++ b/module/rdpClientCon.c
@@ -2270,8 +2270,15 @@ rdpDeferredUpdateCallback(OsTimerPtr timer, CARD32 now, pointer arg)
             monitor_rect.y2 = cap_top + cap_height;
             monitor_dirty = rdpRegionCreate(&monitor_rect, 0);
             rdpRegionIntersect(monitor_dirty, monitor_dirty, clientCon->dirtyRegion);
-            if (rdpRegionNotEmpty(monitor_dirty))
+            num_rects = REGION_NUM_RECTS(monitor_dirty);
+            if (num_rects > 0)
             {
+                if (num_rects > 15)
+                {
+                    monitor_rect = rdpRegionExtents(monitor_dirty);
+                    rdpRegionDestroy(monitor_dirty);
+                    monitor_dirty = rdpRegionCreate(&monitor_rect, 0);
+                }
                 rects = 0;
                 num_rects = 0;
                 LLOGLN(10, ("rdpDeferredUpdateCallback: capture_code %d",

@tfischer77
Copy link

@jsorg71 The screensaver is really drawing pixels... will try that out tomorrow, please see also my comments on #664.

@jsorg71
Copy link
Contributor

jsorg71 commented Feb 14, 2017

@tfischer77 I pushed a change. I think this was causing the stream overrun. It was pushing this huge rectangle list on the stream.

@tfischer77
Copy link

@jsorg71 When applying #664 and neutrinolabs/xorgxrdp#77, it's simply working now. I just tested it, it even resolves the bug neutrinolabs/xorgxrdp#5. So no need to change the stream size. Thank you very much. I hope you will merge these PRs into xrdp and xorgxrdp very soon.

@macdanger
Copy link

Hi guys,
I just tried to check out the current devel + #615, #664 and neutrinolabs/xorgxrdp#77. But as soon as I use a resolution higher than 1920 x 1080 the xrdp log will get flooded with this message:
rdpDeferredUpdateCallback: reschedule rect_id 1 rect_id_ack 0

I can see the xrdp-sessman login screen perfectly but when I login I can't see anything of my xfce desktop and the above message appears. So this behaviour looks exactly the same at least for me.
Or do I have to use any additional patch e.g. this here (#524)?

Sorry if am writing stupid things, maybe I just didn't get it...

Cheers
Markus

@jsorg71
Copy link
Contributor

jsorg71 commented Feb 16, 2017

@macdanger Something must have gone wrong with you build, it does not print
rdpDeferredUpdateCallback: reschedule rect_id...
anymore. They are there but moved to level 10 so they are compiled out.

@macdanger
Copy link

@tfischer77 sorry, I guess I should take another session in using git :-)
I can confirm that it works now but in my case not flawlessly:
I have a monitor with a resolution of 2560 x 1444. When using this resolution with xrdp in fullscreen I get tearing with moving windows and videos. But the tearing appears ONLY at about the half of the screen so that it looks like the screen has been split horizontally into two pieces..

@jsorg71
Copy link
Contributor

jsorg71 commented Feb 17, 2017

@macdanger There is a limit now of 0x200000 pixels for non multi monitor case.
2560x1444 is bigger than that so you get 2 bands. I put this limit in because it looks like the ms server does this. I can increase it or remove it. The stream limit and xrdp 'tile set' limits make me not want to remove it. I can safely increase it to 0x800000. I think I should do that.

@moobyfr
Copy link
Contributor Author

moobyfr commented Feb 17, 2017

4K support is in RDS for Windows 2016, so updating the value makes it follow the latest server support :)

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

No branches or pull requests

7 participants