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

downscaling server side - not just for video #2052

Closed
totaam opened this issue Nov 21, 2018 · 11 comments
Closed

downscaling server side - not just for video #2052

totaam opened this issue Nov 21, 2018 · 11 comments
Labels

Comments

@totaam
Copy link
Collaborator

totaam commented Nov 21, 2018

When the client is using a desktop-scaling value lower than 1.0, we end up sending pixels that aren't all used client-side since we end up downscaling the picture to display it. (the render target is smaller than the picture the client receives)

It would be better if the client could tell the server about the scaling factor it is using then the server can:

  • choose this same factor (or more) for video scaling - easy
  • downscale picture data before compression: new capability flag, new client option, image down scaling pre-compression
@totaam
Copy link
Collaborator Author

totaam commented Oct 20, 2019

See also #2458.

@totaam
Copy link
Collaborator Author

totaam commented Jan 30, 2020

2020-01-30 14:08:29: leo60228 commented


I'd argue that a major reason for this feature is shadowing. Currently, there isn't any way to use a shadowed desktop of a higher resolution than the client (at least in HTML5, the only client I use).

@totaam
Copy link
Collaborator Author

totaam commented Jun 9, 2020

Mostly done in:

  • r26652: client tells the server about its "render-size", the resolution it will actually render at - the video scaling code will
  • r26656: downscale pixel data before compressing, and upscale it client-side

Still TODO:

  • maybe support some kind of fast scaling so we can compress webp and jpeg via the native encoders rather than using pillow? (meh: it is likely that the savings from the downscaling via pillow are enough to not have to bother)
  • client side should be able to decode using any decoder and just upscale during the backend pixel paint (cairo / opengl) rather than being forced to use the pillow decoder
  • drop delta compression? (not very useful and makes the code a lot more complicated)

... at least in HTML5, the only client I use ...

@leo60228: there is no support for any window scaling in the HTML5 client, feel free to create a new ticket for that.

@totaam
Copy link
Collaborator Author

totaam commented Jun 10, 2020

  • r26663: remove delta support (+r26665 + r26666)
  • r26664: honour gravity adjustments when resizing with opengl backend (could be backported with extra care: v4 does gravity adjust in paint_rgb, trunk now does it in do_paint_rgb and gl_paint_planar)
  • r26668: handle scaling during the backing paint rather than the decompression stage, so all the decoders can use it and not just pillow

Notes:

  • webp can decompress to a scaled image - not sure there are any benefits to doing that
  • in non-opengl video mode, we could avoid scaling during the csc stage, and let the cairo paint handle it - meh

@stdedos: I am re-assigning this ticket to you because you will see huge improvements in bandwidth usage and performance when using desktop-scaling values lower than 1.0.
Feel free to just close.

(this requires 4.1-r26668 or later on both client and server)

@totaam
Copy link
Collaborator Author

totaam commented Jun 10, 2020

2020-06-10 11:37:35: stdedos commented


I can keep it open, and I cannot test it (my OS update plan for the server is late June to August)

@totaam
Copy link
Collaborator Author

totaam commented Jul 31, 2020

This has caused a regression for normal windows when we resize them up: the render-size does not get updated server side, and we end up downscaling when we don't need to..

@totaam
Copy link
Collaborator Author

totaam commented Jul 31, 2020

... the render-size does not get updated server side ..

Fixed in r27044.

Side effect: we now call update_encoding_selection from set_client_properties, even when the render-size is identical to the window size, which is a little bit of a waste.

stdedos: I'm going to close this ticket for 4.1 - try it when you get a chance.

@totaam totaam closed this as completed Jul 31, 2020
@totaam
Copy link
Collaborator Author

totaam commented Sep 23, 2020

This causes problems with windows that don't fit the sizing constraints exactly: we end up downscaling them to the client render size (which is often only a few pixels off) and that makes it blurry. Also makes us use pillow (png) instead of the faster options (webp, jpeg).

r27525 fixes that using a threshold.

@stdedos
Copy link
Collaborator

stdedos commented May 4, 2021

As-seen in #2903 (comment), I can of course connect to the shadow server. Apart from the intermittent latency, jitter spikes and "pls wait" spinners, the shadowing works.

The original issue was problematic, and therefore the idea of this feature was to pre-scale the video, to reduce its size - because the vp[89] was unusable on Xenial (lack of codecs? IIRC). This change, however, was only valid in v4 (i.e. >Xenial).

So, I cannot expressively say that "this feature works and only this one solves the issue", since I don't know how to measure xpra's bandwidth pre/post of this feature, and the vp[89] may of course help (I have seen it in a lot of seamless rendering, and vp[89] codecs are used for said rendering), the overall quality appears to be increased, and the overall lags/"pls wait" has dropped.

I can testify that "this feature did not break shadow" though 😛

@totaam
Copy link
Collaborator Author

totaam commented May 4, 2021

Related fix for video scaling: a437a9e (use the same ratio or a multiple thereof)

@totaam
Copy link
Collaborator Author

totaam commented Nov 20, 2021

We now also downscale jpeg and webp using libyuv: b764753 - this could be improved: #3357

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants