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

fix broadway software h264 decoding #68

Closed
totaam opened this issue May 13, 2018 · 13 comments
Closed

fix broadway software h264 decoding #68

totaam opened this issue May 13, 2018 · 13 comments

Comments

@totaam
Copy link
Collaborator

totaam commented May 13, 2018

Issue migrated from trac ticket # 1839

component: html5 | priority: major | resolution: fixed

2018-05-13 06:38:13: antoine created the issue


Split from #25, the error that comes up is RangeError: Source is too large.

@totaam
Copy link
Collaborator Author

totaam commented May 13, 2018

The error RangeError: Source is too large turned out to be trivial to fix: r19294, just needed a "new" pair of eyes, found it when looking into mpeg1 decoding (#1816).

So r19295 enables broadway software decoding by default, again.

What we may still want to do here:

  • tune the encoder to maximize decoding speed for the html5 client since software decoding is going to be CPU intensive: fastdecode – disables CABAC and the in-loop deblocking filter to allow for faster decoding on devices with lower computational power
  • do a test run during startup and verify that we can decode some test frames, and fast enough (tricky since the broadway javascript decoder may not be optimized the first time we run it) - or blacklist some browsers / OS / CPU

@totaam
Copy link
Collaborator Author

totaam commented May 13, 2018

2018-05-13 08:01:40: antoine commented


The html5 client already sets the correct h264 encoding properties (see also Xpra-org/xpra#1840):

"encoding.h264.YUV420P.profile"		: "baseline",
"encoding.h264.YUV420P.level"		: "2.1",
"encoding.h264.cabac"			: false,
"encoding.h264.deblocking-filter"	: false,

And these are correctly applied by the x264 codec (as seen with "-d x264"):

x264 context=0x7f08788859c0, YUV420P  640x356  quality=40, speed=40, source=unknown
 preset=medium, profile=baseline, tune=zerolatency
 me=DIA, me_range=16, mv_range=-1, weighted-pred=0
 b-frames=0, max delayed frames=0
 vfr-input=False, lookahead=-1, sync-lookahead=0, mb-tree=False, bframe-adaptive=FAST
 open-gop=True, bluray-compat=False, cabac=False, deblocking-filter=False
 intra-refresh=False, interlaced=False, constrained_intra=False
 threads=auto, sliced-threads=True
x264 encode YUV420P frame     0 as  IDR slice with 8 nals, tune=zerolatency, total   10776 bytes, keyframe=True , delayed=0

Note: nvenc doesn't have such controls, so it may or may not produce a stream which is more CPU intensive to decode.

So the only remaining issue is the blacklisting, which we can look at after some testing.
This:

Decoder.js:450 Invalid asm.js: Unexpected token

could slow things down quite a bit - not sure how to fix that (building it from source is a pain).

@totaam
Copy link
Collaborator Author

totaam commented May 15, 2018

Updates:

  • r19318: fix asm.js, don't minify source in svn, we do it during the build instead
  • r19321: better x264 tuning for the broadway decoder: "fast-decode" tune, higher min-speed
  • r19322: minor fix
  • r19323: better "fast-decode" tuning
  • r19324: better x264 encoder content type tuning
  • r19325: expose more x264 settings via xpra info

Which gives us:

window.5.encoder=x264
window.5.encoder.b-frames=0
window.5.encoder.delayed=11
window.5.encoder.fast-decode=True
window.5.encoder.formats=('BGRX', 'YUV422P', 'YUV420P', 'BGRA', 'YUV444P')
window.5.encoder.fps=16
window.5.encoder.frame-types.IDR=1
window.5.encoder.frame-types.P=7
window.5.encoder.frames=8
window.5.encoder.generation=9
window.5.encoder.height=432
window.5.encoder.lossless=False
window.5.encoder.max-size=(8192, 4096)
window.5.encoder.ms_per_frame=1
window.5.encoder.params.bframe-adaptive=NONE
window.5.encoder.params.bluray-compat=False
window.5.encoder.params.cabac=False
window.5.encoder.params.constrained_intra=False
window.5.encoder.params.deblocking-filter=False
window.5.encoder.params.interlaced=False
window.5.encoder.params.intra-refresh=False
window.5.encoder.params.lookahead=0
window.5.encoder.params.mb-tree=False
window.5.encoder.params.me.me-range=16
window.5.encoder.params.me.mv-range=256
window.5.encoder.params.me.type=HEX
window.5.encoder.params.me.weighted-pred=0
window.5.encoder.params.open-gop=False
window.5.encoder.params.sliced-threads=False
window.5.encoder.params.threads=12
window.5.encoder.params.vfr-input=False
window.5.encoder.pixels_per_second=159132896
window.5.encoder.preset=veryfast
window.5.encoder.profile=baseline
window.5.encoder.quality=58
window.5.encoder.source=video
window.5.encoder.speed=82
window.5.encoder.src_format=YUV420P
window.5.encoder.total_time_ms=13
window.5.encoder.tune=film
window.5.encoder.version=148
window.5.encoder.width=640

Still TODO:

  • why am I seeing many delayed frames?
  • some corrupted frames too?

@totaam
Copy link
Collaborator Author

totaam commented Jun 7, 2018

why am I seeing many delayed frames?

I was and now I'm not. No idea why.

some corrupted frames too?

I believe those were jpeg frames, there were more of those because of this bug: r19579

Just like Xpra-org/xpra#1816, this is ready for testing.
The quality is better than mpeg1, the bandwidth is slightly lower but the CPU usage is higher.

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2018

2018-06-12 17:51:51: maxmylyn commented


Quick question:

How do I force it to use h264 over mpeg-1? In Xpra-org/xpra#1816 you mentioned that adding --video-encoders=ffmpeg will force mpeg-1. At least that's how I understood it. Running without that flag definitely has a quality increase. I also tried --video-encoders=x264 and nothing yelled at me so I assume (probably incorrectly) that it worked.

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2018

How do I force it to use h264 over mpeg-1?

h264 will be used ahead of mpeg1 by default.
If you really want to ensure that mpeg1 will not be used, add --video-encoders=x264,vpx

You can always check what is actually being used using the paint boxes and -d compress.

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2018

2018-06-12 21:06:49: maxmylyn commented


Okay that did it.

The h264 decoding looks really good - it's seriously impressive.

Is there anything in particular I should look for? As far as I can tell it's working great.

I'll use it some more today and see if I can't break something. But until then - it's pretty fantastic. I can actually play FTL ([https://www.gog.com/game/faster_than_light]) at a decent framerate (almost the same as the Python2 client) and is actually playable. That's totally awesome.

@totaam
Copy link
Collaborator Author

totaam commented Jun 12, 2018

2018-06-12 21:44:13: maxmylyn commented


Playing around with the HTML5 client some more I am fairly confident I've found a memory leak. If you open up the Chrome task manager (or whatever it's called) with shift+escape in Google Chrome, you can sort tabs by memory. Simply watching a video file with the HTML5 client causes the memory to increase steadily up to over 3GB after a few minutes. Disconnecting causes the memory to go away. Reconnecting with video decoders disabled does not cause the memory to climb.

EDIT: I should make it clear that I also started my server with --video-encoders=x264,vpx, so it's definitely running with h.264 and not mpeg-1.

I'm not sure what logs to get for you, but it's very obvious there's a memory leak with video enabled.

@totaam
Copy link
Collaborator Author

totaam commented Jun 13, 2018

I can only reproduce this by having the javascript console activated. In this case, the memory is used by the "GPU Process" which climbs at the same rate as the xpra html5 client.
When I do see the memory leak, it is reproducible without h264 enabled, so it would not belong in this ticket.
Note: there was a bug which prevented the server from suspending the video stream when the browser tab is not active, that's fixed in r19631. This does mean that the tab may have to be shown / active to reproduce the bug, as otherwise the screen updates stop.

There are lots of details missing, just to name a few:

  • chrome exact version and OS
  • does it occur with other browsers or OSes?
  • "watching a video" - how? A specific player, all of them? what resolution?
  • xpra info
  • data from the memory tab of the developer tools
  • javascript console output with "draw" debugging enabled

Assuming that there is a leak without the debug console enabled, here are some links that may be relevant:

@totaam
Copy link
Collaborator Author

totaam commented Jun 13, 2018

2018-06-13 18:19:52: maxmylyn commented


Okay, in that case, I will follow up in a new ticket.

@totaam
Copy link
Collaborator Author

totaam commented Jun 20, 2018

2018-06-20 18:09:31: maxmylyn commented


Since the memory leak isn't related to the h264 decoding, is there anything left on this ticket or should we close it?

@totaam
Copy link
Collaborator Author

totaam commented Jun 20, 2018

Okay, in that case, I will follow up in a new ticket.

The html5 client memleak ticket is Xpra-org/xpra#1879.

Since the memory leak isn't related to the h264 decoding, is there anything left on this ticket or should we close it?

I would have expected you guys to investigate bandwidth savings, CPU usage and such (this could be done using the automated tests). But this works well enough for me.

@totaam totaam closed this as completed Jun 20, 2018
@totaam
Copy link
Collaborator Author

totaam commented Nov 24, 2018

Missed a glaring bug: Xpra-org/xpra#2060.

@totaam totaam transferred this issue from Xpra-org/xpra Jul 1, 2021
@totaam totaam mentioned this issue Jul 1, 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