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

better/faster encoding selection #419

Closed
totaam opened this issue Aug 21, 2013 · 27 comments
Closed

better/faster encoding selection #419

totaam opened this issue Aug 21, 2013 · 27 comments

Comments

@totaam
Copy link
Collaborator

totaam commented Aug 21, 2013

Issue migrated from trac ticket # 419

component: server | priority: blocker | resolution: fixed

2013-08-21 11:02:35: totaam created the issue


The problem: we need lossless encoding (small regions, auto-refresh), but:

  • PNG is slow
  • RGB + zlib is inefficient
  • webp has the potential, but the python API does not allow us to choose the trade off between speed and output size, though the underlying API does offer this option: see Advanced Encoding API:
int lossless;           // Lossless encoding (0=lossy(default), 1=lossless).
float quality;          // between 0 (smallest file) and 100 (biggest)
int method;             // quality/speed trade-off (0=fast, 6=slower-better)
@totaam
Copy link
Collaborator Author

totaam commented Aug 21, 2013

2013-08-21 11:03:48: totaam changed status from new to assigned

@totaam
Copy link
Collaborator Author

totaam commented Aug 21, 2013

2013-08-21 11:03:48: totaam changed owner from antoine to totaam

@totaam
Copy link
Collaborator Author

totaam commented Aug 21, 2013

2013-08-21 11:03:48: totaam edited the issue description

@totaam
Copy link
Collaborator Author

totaam commented Nov 15, 2013

2013-11-15 14:14:00: totaam commented


too late for 0.11 and fixing webp (#491) will have to be done first

@totaam
Copy link
Collaborator Author

totaam commented Feb 9, 2014

2014-02-09 08:25:21: antoine commented


re-scheduling all webp stuff

@totaam
Copy link
Collaborator Author

totaam commented Mar 30, 2014

2014-03-30 12:28:56: totaam uploaded file webp-advanced-encoding.patch (18.5 KiB)

stub for a cython version of the webp encoder

@totaam
Copy link
Collaborator Author

totaam commented Mar 31, 2014

2014-03-31 12:22:42: totaam commented


The webp encoder in Cython was added in r5958, used ahead of the old python-webm encoder (which is still used as fallback for pixel modes it cannot handle).

r5959 adds some tests that make it easier to compare webp / webm, png and jpeg encoders. Some preliminary results:

  • webp (on its medium speed setting) is faster than python-webm
  • webp is very slow at low speeds..
  • jpeg is still an order of magnitude faster than the rest

Conclusions:

  • maybe we need to normalize the speed setting, so that webp won't go too slow unless we set the speed right down
  • we should use webp instead of png in some cases
  • rgb + lz4 does remarkably well - and when it doesn't compress well (small alpha test), it doesn't matter: the picture is so small that 6KB vs 3KB is not that big a deal
  • maybe we need to change our API usage as lossless compression is way too slow
  • lossless mode may need tweaking (as we may assume a specific encoding is lossless, as was the case with png - but webp can do both)

Some performance data using some sample images, showing both speed (in mega pixels per second - higher is better, and compression ratio - lower is better):
|||||||||| Mode ||||=Desktop 2560x1600=||||=Browser 1920x1080=||||=Diagram 640x800||||=Small Alpha 64x48||
||Encoding||Compressor||Preset||Quality||Speed||MPixels/s||comp||MPixels/s||comp||MPixels/s||comp||MPixels/s||comp||
||png||PIL (old)||optimized|| || 0|| 1.6|| 8.1||
||png||PIL (old)|| || || 100|| 6.5|| 8.5||
||png||PIL|| || || 100|| 11.6|| 10.0|| 16.1|| 3.1|| 14.6|| 5.0|| 6.0|| 30.1||
||png/P||PIL|| || || 100|| 21.8|| 4.7|| 17.2|| 9.0|| 23.9|| 3.9|| 2.2|| 14.0||
||png/L||PIL|| || || 100|| 24.3|| 3.8|| 31.1|| 1.4|| 29.7|| 2.6|| 7.6|| 4.7||
||webp||PIL|| || 0|| || 5.5|| 0.5|| 6.3|| 0.5|| 5.9|| 0.7|| 0.4|| 13.4||
||webp||PIL|| || 50|| || 5.1|| 2.1|| 6.0|| 1.1|| 5.6|| 1.6|| 0.4|| 16.7||
||webp||PIL|| || 100|| || 4.3|| 6.2|| 5.9|| 3.1|| 5.3|| 4.3|| 0.4|| 28.8||
||jpeg||PIL||optimized|| 0|| || 71.0|| 0.5|| 68.5|| 0.4|| 80.6|| 0.5|| 16.6|| 3.2||
||jpeg||PIL|| || 50|| || 86.0|| 3.1|| 68.1|| 1.9|| 68.2|| 2.5|| 12.7|| 8.8||
||jpeg||PIL||optimized|| 100|| || 48.0|| 10.8|| 56.8|| 5.7|| 56.0|| 7.7|| 9.5|| 26.1||
||jpeg||PIL|| || 100|| || 72.0|| 13.0|| 58.4|| 5.7|| 56.5|| 7.7|| 8.4|| 26.1||
||rgb||lz4|| || || 100|| 224.0|| 15.0|| 497.0|| 4.3|| 526.0|| 6.9|| 76.7|| 56.0||
||rgb||zlib|| || || 50|| 21.0|| 9.1|| 30.2|| 2.5|| 27.8|| 3.1|| 11.3|| 39.4||
||webp||python-webm|| || 0|| || 6.0|| 0.8|| 6.7|| 0.5|| 6.2|| 0.7|| 0.5|| 13.4||
||webp||python-webm|| || 50|| || 5.3|| 2.1|| 6.5|| 1.1|| 5.7|| 1.6|| 0.6|| 16.7||
||webp||python-webm|| || 100|| || 0.3|| 4.8|| 1.0|| 0.7|| 0.7|| 1.6|| 0.1|| 25.0||
||webp||Cython||TEXT|| 0|| 100|| 18.6|| 0.6|| 19.4|| 0.4|| 18.9|| 0.6|| 1.5|| 2.9||
||webp||Cython||TEXT|| 50|| 100|| 15.5|| 2.7|| 17.7|| 1.5|| 16.6|| 2.2|| 1.2|| 11.9||
||webp||Cython||TEXT|| 99|| 100|| 12.6|| 6.7|| 15.2|| 3.3|| 13.9|| 4.6|| 0.8|| 29.7||
||webp||Cython||TEXT + lossless|| 100|| 0|| 0.1|| 4.9|| 0.6|| 0.7|| 0.3|| 1.6|| 0.0|| 24.7||
||webp||Cython||TEXT + lossless|| 100|| 100|| 2.3|| 5.9|| 27.8|| 1.3|| 27.2|| 1.9|| 1.5|| 26.9||

Notes and caveats:

  • Low quality, low speed doesn't really make much sense, so some values have been omitted
  • changing the webp preset does not affect speed or output size much, it probably does affect the perceived picture quality (seems best to stick with TEXT to ensure that text remains readable)
  • The new webp encoder seems to perform about the same with speeds >50%, only low speed is really slow, and lossless is unbearably slow (it can take more than 30 seconds to encode a single frame using lossless + low speed!)
  • quality is not the same for each encoder, some have lossless modes others not, etc..

@totaam
Copy link
Collaborator Author

totaam commented Apr 1, 2014

2014-04-01 04:45:36: totaam commented


After figuring out that libwebp wants us to use the quality attribute for storing the speed when in lossless mode... I kid you not! applied in r5962. The results are a lot better on "high speed" (s=100%), but still much slower than png... (table above updated)

@totaam
Copy link
Collaborator Author

totaam commented Apr 1, 2014

2014-04-01 08:47:12: totaam commented


Digging further shows that it is possible to make png faster instead.
r5966 tunes png and jpeg better.

I have also tested to see if changing the png compression_type attribute can give us better compression or performance (no: default is best):

|||| ||||=Desktop 2560x1600=||||=Browser 1920x1080=||||=Diagram 640x800=||||=Small Alpha 64x48=||
||=Compression Type=||=Level=||=MPixels/s=||=comp=||=MPixels/s=||=comp=||=MPixels/s=||=comp=||=MPixels/s=||=comp=||
||HUFFMAN_ONLY|| n/a|| 9.8|| 19.0|| 11.4|| 15.8|| 10.8|| 17.3|| 6.6|| 29.9||
||FIXED|| 1|| 12.1|| 11.7|| 16.7|| 3.6|| 14.2|| 5.8|| 5.7|| 35.1||
||FIXED|| 3|| 10.7|| 10.7|| 16.6|| 3.5|| 13.8|| 5.5|| 5.0|| 33.0||
||RLE|| 1|| 12.1|| 11.5|| 16.6|| 4.6|| 15.2|| 6.5|| 7.1|| 27.3||
||RLE|| 3|| 10.8|| 10.7|| 16.6|| 4.6|| 14.7|| 6.5|| 6.8|| 27.3||
||FILTERED|| 1|| 12.1|| 10.0|| 16.7|| 3.1|| 14.1|| 5.0|| 5.8|| 30.1||
||FILTERED|| 3|| 10.7|| 9.4|| 16.4|| 3.0|| 12.0|| 4.8|| 5.0|| 29.1||
||DEFAULT (old code)|| unset|| 6.8|| 8.4|| 12.5|| 2.7|| 10.3|| 4.9|| 3.4|| 26.2||

Notes:

  • Level 1 now corresponds to >75% speed and level 3 to 26% to 50% speed
  • Lower speeds aren't very interesting, and we may want to prevent the level from going that low
  • HUFFMAN_ONLY ignores the compression level
  • DEFAULT is clearly the same as FILTERED (identical results - not shown)

@totaam
Copy link
Collaborator Author

totaam commented Apr 1, 2014

2014-04-01 15:00:15: totaam changed title from better/faster lossless encoding (probably using webp) to better/faster lossless encoding selection

@totaam
Copy link
Collaborator Author

totaam commented Apr 1, 2014

2014-04-01 15:00:15: totaam commented


(updating bug title to reflect the greater scope of this ticket)

@totaam
Copy link
Collaborator Author

totaam commented Apr 2, 2014

2014-04-02 15:33:08: totaam uploaded file new-best-encoding-heuristics.patch (20.6 KiB)

updates the encoding heuristics based on the new data

@totaam
Copy link
Collaborator Author

totaam commented Apr 3, 2014

2014-04-03 06:08:23: totaam changed title from better/faster lossless encoding selection to better/faster encoding selection

@totaam
Copy link
Collaborator Author

totaam commented Apr 3, 2014

2014-04-03 06:08:23: totaam commented


Many improvements:

  • r5987 (logging with -d compress)
  • r5988 normalize speed for webp
  • r5989 discard delta
  • r5990 (BIG!) lots of changes, see commit log message
  • r5994 avoid very small and very large areas with webp
  • r6003 + r6001 + r5999: better video + csc pipeline selection
  • r6008: saves getting a new xshm image when processing multiple regions

@totaam
Copy link
Collaborator Author

totaam commented May 17, 2014

2014-05-17 06:30:38: totaam changed status from assigned to new

@totaam
Copy link
Collaborator Author

totaam commented May 17, 2014

2014-05-17 06:30:38: totaam changed owner from totaam to smo

@totaam
Copy link
Collaborator Author

totaam commented May 17, 2014

2014-05-17 06:30:38: totaam commented


  • added info to Encodings
  • try to stick with the chosen encoding for non-video encodings (r6478)
  • add ability to force the use of the selected encoding with XPRA_ENCODING_STRICT_MODE=1 (r6519). Note: with video encodings, we still need to send the edges of odd sized windows with rgb, when the picture is lossy we may still use another encoding for the lossless refresh..
  • use BGRX directly with rgb encodings and mmap (already backported to 0.12.6): r6492, re-stride to save space: r6474, and associated fixes (GL: r6462)
  • encoding selection bug fix for video encoding selection (ouch! r6477)
  • ensure we don't use webp when it is too slow, even when selected as primary encoding (see r6500)
  • webp encoder leak fixed in r6505, see also updates in webp artifacts with transparency #487

smo: please run this latest code on your test system so we can compare with previous runs and ensure the performance is at least as good as before for most tests

@totaam
Copy link
Collaborator Author

totaam commented May 26, 2014

2014-05-26 16:32:25: totaam changed priority from major to blocker

@totaam
Copy link
Collaborator Author

totaam commented May 26, 2014

2014-05-26 16:32:25: totaam commented


Important regression discovered and fixed in r6568 (see "Trouble with xpra 0.13" mailing list posts for the bug report), please test and verify that PNG, RGB, png/L and png/P never end up using lossy encodings, no matter what command line options or previous encodings were used. And no matter how bad the refresh rate gets when under pressure.

Raising to blocker as this should definitely go in 0.13.1. Backported in 6571.
Quite a few fixes have gone in, none of them quite as critical as the one above (transparency, opengl toggle, etc): r6577, r6578, r6583

@totaam
Copy link
Collaborator Author

totaam commented Jul 21, 2014

2014-07-21 23:09:00: afarr commented


Testing with win client 0.14.0 r6910 against fedora 20 0.14.0 r6910 server.

With a client-side --encoding=png the encoding seems to stick to png, no matter how bad it gets.

2014-07-21 12:39:45,496 update_speed() info={'damage_latency.abs_factor': 0, 'damage_latency.avg': 6, 'decoding_latency.target': 8000000, 'damage_latency.target': 35, 'min_speed': 0, 'frame_delay': 8, 'decoding_latency.factor': 109$
2014-07-21 12:39:45,496 reconfigure(False) csc_encoder=None, video_encoder=None
2014-07-21 12:39:47,190 damage(WindowModel(0xa00022 - "jimador@zapopan:~"), 155, 301, 6, 13, {})
2014-07-21 12:39:47,190 damage(155, 301, 6, 13, {}) wid=1, sending now with sequence 59
2014-07-21 12:39:47,191 process_damage_regions: wid=1, adding png pixel data to queue, elapsed time: 1.5 ms, request rgb time: 0.6 ms
2014-07-21 12:39:47,192 make_data_packet: image=XShmImageWrapper(BGRX: 155, 301, 6, 13), damage data: (1, 155, 301, 6, 13, 'png')
2014-07-21 12:39:47,195 update_speed() info={'damage_latency.abs_factor': 0, 'damage_latency.avg': 7, 'decoding_latency.target': 8000000, 'damage_latency.target': 35, 'min_speed': 0, 'frame_delay': 4, 'decoding_latency.factor': $
2014-07-21 12:39:47,195 sending 6x13 BGRX as png, mode=RGB, options={'compress_level': 1}
2014-07-21 12:39:47,195 reconfigure(False) csc_encoder=None, video_encoder=None

Likewise with RGB & png/L.

With --encoding=png/P however, the encoder seems to use png/P for "existing delayed regions" (2014-07-21 14:32:10,285 damage(86, 427, 640, 54, {}) wid=3, using existing delayed png/P regions created 0.0ms ago), but seems to bounce around from rgb to webp to png/P overall.

Here's a bit of a clip of server-side logs (I'll include a full long log for this):

2014-07-21 14:31:32,441 update_speed() info={'damage_latency.abs_factor': 0, 'damage_latency.avg': 8, 'decoding_latency.target': 8000000, 'damage_latency.target': 35, 'min_speed': 0, 'frame_delay': 24, 'decoding_latency.factor':$
2014-07-21 14:31:32,442 damage(143, 2, 6, 13, {}) wid=1, sending now with sequence 2
2014-07-21 14:31:32,442 reconfigure(False) csc_encoder=None, video_encoder=None
2014-07-21 14:31:32,444 damage(WindowModel(0xc00022 - "jimador@zapopan:~"), 143, 2, 6, 13, {})
2014-07-21 14:31:32,444 damage(143, 2, 6, 13, {}) wid=2, scheduling batching expiry for sequence 3 in 24.0 ms
2014-07-21 14:31:32,446 process_damage_regions: wid=1, adding rgb24 pixel data to queue, elapsed time: 5.5 ms, request rgb time: 1.1 ms
2014-07-21 14:31:32,446 make_data_packet: image=XShmImageWrapper(BGRX: 143, 2, 6, 13), damage data: (1, 143, 2, 6, 13, 'rgb24')

...

2014-07-21 14:32:22,577 damage(86, 325, 640, 102, {}) wid=3, using existing delayed png/P regions created 0.0ms ago
2014-07-21 14:32:22,577 damage(WindowModel(0xe00001 - "John Oliver Is Psyched for the World Cup - Late Night with Seth Meyers - YouTube - Google Chrome"), 86, 427, 640, 54, {})
2014-07-21 14:32:22,577 damage(86, 427, 640, 54, {}) wid=3, using existing delayed png/P regions created 0.0ms ago
2014-07-21 14:32:22,618 send_delayed for wid 3, delaying again because of backlog: 3 packets, batch delay is 42.3135884506, elapsed time is 43.0 ms
2014-07-21 14:32:22,664 damage(WindowModel(0xe00001 - "John Oliver Is Psyched for the World Cup - Late Night with Seth Meyers - YouTube - Google Chrome"), 86, 481, 512, 6, {})
2014-07-21 14:32:22,664 damage(86, 481, 512, 6, {}) wid=3, using existing delayed png/P regions created 0.1ms ago
2014-07-21 14:32:22,704 send_delayed_regions: bytes_cost=238592, bytes_threshold=698084, pixel_count=233472
2014-07-21 14:32:22,707 process_damage_regions: wid=3, adding webp pixel data to queue, elapsed time: 132.2 ms, request rgb time: 2.5 ms
2014-07-21 14:32:22,707 make_data_packet: image=XShmImageWrapper(BGRX: 86, 121, 640, 366), damage data: (3, 86, 121, 640, 366, 'webp')

If I'm not mistaken, though, none of those are "lossy encodings" ... so that may be expected behavior.

Smo will try to arrange those tests for performance as soon as he can.

@totaam
Copy link
Collaborator Author

totaam commented Jul 22, 2014

2014-07-22 02:45:02: afarr uploaded file ticket419-png-P_server-encoding_test1.zip (490.6 KiB)

ticket 419 (overlarge) server-side png/P -d encode log

@totaam
Copy link
Collaborator Author

totaam commented Aug 20, 2014

2014-08-20 03:20:48: totaam changed status from new to assigned

@totaam
Copy link
Collaborator Author

totaam commented Aug 20, 2014

2014-08-20 03:20:48: totaam changed owner from smo to totaam

@totaam
Copy link
Collaborator Author

totaam commented Aug 20, 2014

2014-08-20 03:20:48: totaam commented


I guess this should have been re-assigned to me - taking it back.

I thought that this was all working as expected until #645

I will try to clarify the logging:

  • the encoding seen in the damage call is the encoding currently set as default (generally the one selected by the user - except when auto-refresh kicks in, or when specifically overriden..)
  • process_damage_regions shows rgb.. which is not an encoding at all: in this case, that's just the raw pixel format we get from the x11 server
  • the encoding actually used for compression is shown in make_data_packet

And so the encoding actually may well be different from the one that is set, especially when:

  • with video modes, you can get pretty much any encoding out depending on many heuristics
  • we prefer rgb for small regions (or when we have bandwidth to spare) - but not when using png/L (which is black and white, as this would look funny)
  • auto-refresh will use a lossless encoding
    etc.

@totaam
Copy link
Collaborator Author

totaam commented Aug 24, 2014

2014-08-24 07:08:04: totaam changed status from assigned to closed

@totaam
Copy link
Collaborator Author

totaam commented Aug 24, 2014

2014-08-24 07:08:04: totaam changed resolution from ** to fixed

@totaam
Copy link
Collaborator Author

totaam commented Aug 24, 2014

2014-08-24 07:08:04: totaam commented


Despite some fairly big bugs that went unnoticed until after the release (#645 mentioned above and also #651), I think this is now working as expected and also much faster than before (see #620).

r7424 removes 'rgb' from the logging in process_damage_regions.

I have added information to the wiki on debugging encodings here: Encodings. Closing this ticket, some of these issues will be followed up in #410.

@totaam totaam closed this as completed Aug 24, 2014
This was referenced Jan 22, 2021
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

1 participant