Large WebGL video playback performance regression
Reported by
dustin.k...@gmail.com,
Jul 5 2016
|
||||||||||||||
Issue descriptionUserAgent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2788.0 Safari/537.36 Example URL: https://jsfiddle.net/qz9ka6xm/ Steps to reproduce the problem: 1. Go to https://jsfiddle.net/qz9ka6xm/ 2. Hit Run 3. Compare results from Chrome 51.x, 52.x, 53.x, and 54.x 4. Note that Chrome 53.x and 54.x are significantly slower (over 4x slower for 2160p) What is the expected behavior? I've noticed this for the past few weeks in Canary but thought it was being tracked by other tickets. Since it hasn't changed in behavior I am logging it now. This can also be reproduced from the original benchmark code - http://codeflow.org/issues/slow_video_to_texture/ - but that page only tests up to 1080p. My jsfiddle uses both 1080p and 2160p files and a different (and maybe more accurate) way of calculating the rendered frame rate. It appears that both texImage2D and texSubImage2D have regressions. Playback of video without webGL doesn't seem to be affected, ie. directly playing http://distribution.bbb3d.renderfarming.net/video/mp4/bbb_sunflower_2160p_60fps_normal.mp4 What went wrong? Not sure. Did this work before? Yes Chrome v51.x and v52.x Is it a problem with Flash or HTML5? HTML5 Does this work in other browsers? Yes Chrome version: 54.0.2788.0 Channel: canary OS Version: 10.0 Flash Version: Shockwave Flash 22.0 r0
,
Jul 5 2016
Sorry, I was mistaken. This issue is only reproducible on Chrome 54.x currently.
,
Jul 5 2016
I'm not aware of any change between 53 and 54 on Windows in the upload code, adding DS that is familiar with the upload code too. Dustin, could you try running the experiment passing the flag --enable-gpu-memory-buffer-video-frames when you launch Chromium?
,
Jul 5 2016
Thanks for looking into it. I just tried --enable-gpu-memory-buffer-video-frames on 54 and I wasn't able to detect any difference. Let me know if I can do anything else to test.
,
Jul 5 2016
Ooh, my bad, the decoder already produces hw videoframes since it's using an hw decoder, so that flag is not changing anythign. We could try to bisect to find the CL with the regression: https://www.chromium.org/developers/bisect-builds-py
,
Jul 5 2016
Doing a bisect is probably a bit out of my domain expertise :) I'll leave that to the experts. I just did a little bit more testing when Hardware-accelerated video decode == disabled. In that case, Chrome 54 is on par with Chrome 53/52/51. Interestingly on Chrome 54 the software decoding is performing better than hardware decoding (2160p averages around 10fps on 54 with GPU enabled and 20fps with GPU disabled).
,
Jul 5 2016
Adding Dale that might know if anything changed in the way we decode videos between 53 and 54.
,
Jul 6 2016
was only Windows regressed? If so, --enable-zero-copy-dxgi-video might be related. https://bugs.chromium.org/p/chromium/issues/detail?id=574292 Could someone clarify which OS and which configuration are regressed? configuration consists of gpu-decode, cpu-decode, and cpu-decode-on-GMBs
,
Jul 6 2016
I've only tested on my Surface Pro 2 i5 running Windows 10. I haven't had the chance to test other machines just yet.
,
Jul 6 2016
And yes! June 16th is the day I noticed this issue. https://bugs.chromium.org/p/chromium/issues/detail?id=574292#c30 Luckily I had the slowdown referenced in my Hangout chat history.
,
Jul 6 2016
Actually it was on June 17th when I noticed it, and this explains why I've seen the performance occasionally back to normal (seems the flag was disabled on 6/25 then re-enabled 6/30).
,
Jul 6 2016
Yeah, looks like we do an extra copy now with the NV12 DXGI video path, which should be unnecessary.
,
Jul 6 2016
,
Jul 7 2016
Also, looks like after we do the YUV->RGB conversion, both SkCanvasVideoRenderer::Paint and WebGLRenderingContextBase::texImageHelperHTMLVideoElement (through ImageBuffer::copyToPlatformTexture) do RGBA->RGBA copies. Neither of those copies should be necessary in the fast path.
,
Jul 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/48ec0365657930e8428945f8090432b05f42f14e commit 48ec0365657930e8428945f8090432b05f42f14e Author: jbauman <jbauman@chromium.org> Date: Thu Jul 07 02:44:03 2016 Avoid copying NV12 GL_TEXTURE_EXTERNAL_OES textures for use with skia. Skia can now handle reading from GL_TEXTURE_EXTERNAL_OES textures. It'll immediately do a conversion from YUV to RGB and release the input texture, so we don't need to worry about the texture lifetime. BUG= 625900 ,505026 Review-Url: https://codereview.chromium.org/2126053002 Cr-Commit-Position: refs/heads/master@{#404042} [modify] https://crrev.com/48ec0365657930e8428945f8090432b05f42f14e/media/renderers/skcanvas_video_renderer.cc
,
Jul 8 2016
I imagine there's still work going on, but I did notice a performance increase in today's Canary build so I figured I'd post the 4k results. It's better, but still only about 2/3rd of Chrome 51 performance. Chrome 51 48fps 0.42ms Chrome 54 32fps .81ms Test results from https://jsfiddle.net/qz9ka6xm/ I did also notice an occasional green cast on some decoded frames. This is something I've not seen before on Chrome 54 (or any other current Chrome version).
,
Jul 8 2016
You can see the occasional green frame by using this test - http://codeflow.org/issues/slow_video_to_texture/
,
Jul 9 2016
Ok, I've got https://codereview.chromium.org/2127053004/ out to fix more performance, and https://chromium-review.googlesource.com/#/c/359430/ to fix the green flicker.
,
Jul 9 2016
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/9585a784bff9752bed369d8247a918495bcdf3ab commit 9585a784bff9752bed369d8247a918495bcdf3ab Author: John Bauman <jbauman@chromium.org> Date: Fri Jul 08 22:12:34 2016 Clear SRV cache on keyed mutex texture unbind from TextureStorage11_External This fixes the same issue as https://chromium-review.googlesource.com/328400, except with NV12 external storage textures. The SRV cache thinks the texture is still bound, but releasing the keyed mutex has unbound it. BUG= 625900 , 626524 Change-Id: I991cb3eeaaea0a1c4570b88de2ba873ae868ec2a Reviewed-on: https://chromium-review.googlesource.com/359430 Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: John Bauman <jbauman@chromium.org> [modify] https://crrev.com/9585a784bff9752bed369d8247a918495bcdf3ab/src/libANGLE/renderer/d3d/d3d11/TextureStorage11.cpp [modify] https://crrev.com/9585a784bff9752bed369d8247a918495bcdf3ab/src/libANGLE/renderer/d3d/d3d11/TextureStorage11.h
,
Jul 11 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bb9647c492682316add50636b2ef1e0a4c1f3aa8 commit bb9647c492682316add50636b2ef1e0a4c1f3aa8 Author: jmadill <jmadill@chromium.org> Date: Mon Jul 11 21:27:20 2016 Roll ANGLE e8d5c5c..9585a78 https://chromium.googlesource.com/angle/angle.git/+log/e8d5c5c..9585a78 BUG= 625900 , 626524 TEST=bots TBR=geofflang@chromium.org CQ_INCLUDE_TRYBOTS=tryserver.chromium.win:win_optional_gpu_tests_rel;tryserver.chromium.mac:mac_optional_gpu_tests_rel;tryserver.chromium.linux:linux_optional_gpu_tests_rel Review-Url: https://codereview.chromium.org/2141433004 Cr-Commit-Position: refs/heads/master@{#404735} [modify] https://crrev.com/bb9647c492682316add50636b2ef1e0a4c1f3aa8/DEPS
,
Jul 12 2016
Restart this all single link HD video player to jwplayer all progress Support
,
Jul 17 2016
Hey! I know the performance optimizations are still being worked on (https://codereview.chromium.org/2127053004/) but I was just curious if the original changeset that caused the slowdown (https://bugs.chromium.org/p/chromium/issues/detail?id=574292) was checked into 53.x as I now seem to be getting the same slowdown on 53.0.2785.8 dev-m (64-bit) Does that seem to be the case, or could it be something else? Thanks, Dustin
,
Jul 21 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/581d041c9971bb533e2cbd5c57014ee026fb50e6 commit 581d041c9971bb533e2cbd5c57014ee026fb50e6 Author: jbauman <jbauman@chromium.org> Date: Thu Jul 21 01:01:03 2016 Optimize webgl texImage2D from YUV video textures. The current path does a conversion from YUV to an RGBA SkImage, then paints that to another Skia canvas and does a CopyTexImageCHROMIUM to the WebGL texture. We can optimize this by removing the paint to the canvas and doing a copy directly from the SkImage to the WebGL texture. BUG= 625900 Review-Url: https://codereview.chromium.org/2127053004 Cr-Commit-Position: refs/heads/master@{#406735} [modify] https://crrev.com/581d041c9971bb533e2cbd5c57014ee026fb50e6/content/renderer/media/webmediaplayer_ms.cc [modify] https://crrev.com/581d041c9971bb533e2cbd5c57014ee026fb50e6/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/581d041c9971bb533e2cbd5c57014ee026fb50e6/media/renderers/skcanvas_video_renderer.cc [modify] https://crrev.com/581d041c9971bb533e2cbd5c57014ee026fb50e6/media/renderers/skcanvas_video_renderer.h
,
Jul 21 2016
The slowdown is also in M53, so we'll want to merge the changes (once they've had a bit more time in canary).
,
Jul 22 2016
Moving this nonessential bug to the next milestone. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Jul 23 2016
I tested just now in 54.0.2805.0 canary (64-bit) and the performance is looking close to 52.x 52.0.2743.82 m (64-bit) texImage2D 1080p 57.8000fps 0.95ms texImage2D 2160p 48.6000fps 0.38ms 54.0.2805.0 canary (64-bit) texImage2D 1080p 57.8000fps 1.05ms texImage2D 2160p 43.4000fps 0.89ms Interestingly the 4K 2160p resolution is still a few frames lower and is also showing ~2x slower upload times, yet the 1080p is statistically the same given the variance I see running multiple times. Overall it seems to be much better, though I am curious if this is the expected behavior. Compared to 52.x I was thinking the performance should be better with the reduced texture copying/conversion. Is that right? Thanks, Dustin
,
Jul 26 2016
Yeah, in the case where we're compositing the video to a screen we can avoid an unnecessary conversion in M54, but for the WebGL case it has to do that conversion anyway. Part of the reason the upload seems slower is that we now do the YUV->RGB conversion during the upload, rather than during the decode step. There's also a copy from YUV->YUV that we need to do in M54. When --enable-zero-copy-dxgi-video is enabled by default we should be able to get rid of that.
,
Jul 26 2016
Ok, so even with this performance optimization, due to that unrelated YUV->YUV conversion in v54 we see less than expected performance. Once that is removed, we should see better performance in v54 compared to v52, correct? Is there an existing ticket for tracking that? Also, is that extra YUV->YUV conversion in v53 as well? Thanks, Dustin
,
Jul 27 2016
,
Jul 27 2016
[Automated comment] DEPS changes referenced in bugdroid comments, needs manual review.
,
Jul 29 2016
Before we approve merge to M53, Could you please confirm whether this change is baked/verified in Canary and safe to merge?
,
Jul 29 2016
Yes, this was baked in canary (see comment 26) and should be safe to merge.
,
Jul 29 2016
Approving merge to M53 branch 2785 based on comment #32. Please merge as at your earliest convenience. Thank you.
,
Aug 1 2016
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible! If all merges have been completed, please remove any remaining Merge-Approved labels from this issue. Thanks for your time! To disable nags, add the Disable-Nags label. For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Aug 1 2016
Please try to merge your change to M53 branch 2785 asap so we cant take it for this week beta release on Wednesday. Thank you very much.
,
Aug 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/angle/angle/+/c9e8931198428ce58a7c2c772539186ffa3e1aca commit c9e8931198428ce58a7c2c772539186ffa3e1aca Author: John Bauman <jbauman@chromium.org> Date: Fri Jul 08 22:12:34 2016 Clear SRV cache on keyed mutex texture unbind from TextureStorage11_External This fixes the same issue as https://chromium-review.googlesource.com/328400, except with NV12 external storage textures. The SRV cache thinks the texture is still bound, but releasing the keyed mutex has unbound it. BUG= 625900 , 626524 Change-Id: I991cb3eeaaea0a1c4570b88de2ba873ae868ec2a Reviewed-on: https://chromium-review.googlesource.com/359430 Reviewed-by: Jamie Madill <jmadill@chromium.org> Commit-Queue: John Bauman <jbauman@chromium.org> Reviewed-on: https://chromium-review.googlesource.com/365160 Reviewed-by: John Bauman <jbauman@chromium.org> [modify] https://crrev.com/c9e8931198428ce58a7c2c772539186ffa3e1aca/src/libANGLE/renderer/d3d/d3d11/TextureStorage11.cpp [modify] https://crrev.com/c9e8931198428ce58a7c2c772539186ffa3e1aca/src/libANGLE/renderer/d3d/d3d11/TextureStorage11.h
,
Aug 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8d2e0414e4d63b63a056b39847d95b06334d08cb commit 8d2e0414e4d63b63a056b39847d95b06334d08cb Author: John Bauman <jbauman@chromium.org> Date: Mon Aug 01 22:12:58 2016 Avoid copying NV12 GL_TEXTURE_EXTERNAL_OES textures for use with skia. Skia can now handle reading from GL_TEXTURE_EXTERNAL_OES textures. It'll immediately do a conversion from YUV to RGB and release the input texture, so we don't need to worry about the texture lifetime. BUG= 625900 ,505026 Review-Url: https://codereview.chromium.org/2126053002 Cr-Commit-Position: refs/heads/master@{#404042} (cherry picked from commit 48ec0365657930e8428945f8090432b05f42f14e) Review URL: https://codereview.chromium.org/2198373002 . Cr-Commit-Position: refs/branch-heads/2785@{#447} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/8d2e0414e4d63b63a056b39847d95b06334d08cb/media/renderers/skcanvas_video_renderer.cc
,
Aug 1 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9453c3f083a2f9d777412cf67cbd5feacfc3b87d commit 9453c3f083a2f9d777412cf67cbd5feacfc3b87d Author: John Bauman <jbauman@chromium.org> Date: Mon Aug 01 22:17:39 2016 Optimize webgl texImage2D from YUV video textures. The current path does a conversion from YUV to an RGBA SkImage, then paints that to another Skia canvas and does a CopyTexImageCHROMIUM to the WebGL texture. We can optimize this by removing the paint to the canvas and doing a copy directly from the SkImage to the WebGL texture. BUG= 625900 Review-Url: https://codereview.chromium.org/2127053004 Cr-Commit-Position: refs/heads/master@{#406735} (cherry picked from commit 581d041c9971bb533e2cbd5c57014ee026fb50e6) Review URL: https://codereview.chromium.org/2199093002 . Cr-Commit-Position: refs/branch-heads/2785@{#449} Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382} [modify] https://crrev.com/9453c3f083a2f9d777412cf67cbd5feacfc3b87d/content/renderer/media/webmediaplayer_ms.cc [modify] https://crrev.com/9453c3f083a2f9d777412cf67cbd5feacfc3b87d/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/9453c3f083a2f9d777412cf67cbd5feacfc3b87d/media/renderers/skcanvas_video_renderer.cc [modify] https://crrev.com/9453c3f083a2f9d777412cf67cbd5feacfc3b87d/media/renderers/skcanvas_video_renderer.h
,
Aug 3 2016
Retested the above issue on Windows below are the observation on Canary & latest Beta. Canary - 54.0.2817.0 --------------------- texImage2D 1080p 23.2000 9.66 texImage2D 2160p 13.4000 22.08 Beta - 53.0.2785.45 ------------------- texImage2D 1080p 22.8000 8.99 texImage2D 2160p 13.4000 22.73 @jbauman: Can you please check and let us know your thoughts on the above results. Really appreciate the help. Thank you.
,
Aug 3 2016
Are those results using GPU decode or software? They seem too low for GPU decoding. Have these changes been merged into the 53 branch yet? My Windows 10 Surface Pro 2 results appear to suggest they have not yet: 52.0.2743.82 m (64-bit) ----------------------- texImage2D 1080p 57.6000 0.57 texImage2D 2160p 42.8000 0.73 53.0.2785.34 beta-m (64-bit) ---------------------------- texImage2D 1080p 30.6000 0.76 texImage2D 2160p 10.0000 1.75 54.0.2817.0 canary (64-bit) --------------------------- texImage2D 1080p 56.2000 0.90 texImage2D 2160p 43.2000 0.88 54.0.2816.0 dev-m (64-bit) -------------------------- texImage2D 1080p 57.0000 0.89 texImage2D 2160p 42.0000 0.94
,
Aug 3 2016
John, could you please review the results from #39 and ensure whether this is correct.
,
Aug 3 2016
ashejole@: What's the about:gpu from that system? dustin: The changes were merged into M53, but we haven't yet released a new version that includes them.
,
Aug 4 2016
@John: Please find attach about://gpu of the system below. Do let me know if you need any more info. Below are results on 53.0.2785.46 --------------------------------- texImage2D 1080p 24.4000 9.29 texImage2D 2160p 15.8000 22.00 Thank you!
,
Aug 4 2016
ashejole@ from the errors at the bottom it doesn't appear that your system is utilizing GPU decoding - "HW video decode not available for profile 3". I would think that an Intel HD 4600 should be GPU decoding as my Intel HD 4400 has no issues. It does look like your system reports two GPU's: GPU0 VENDOR = 0x8086, DEVICE= 0x0416 *ACTIVE* GPU1 VENDOR = 0x10de, DEVICE= 0x0ff6 Is that correct? -Dustin
,
Aug 4 2016
This issue is not tagged as a Beta blocker., so not holding beta release scheduled today. Requesting John to verify the results and provide the feedback.
,
Aug 4 2016
Yeah, I think this bug is essentially fixed (particularly now that canary includes the patch to avoid NV12->NV12 copying). It is weird that ashejole's computer doesn't seem to support hardware video decoding. We should probably follow up to see why that is - an about:gpu from chrome canary (after trying that test) would have more logging and might be interesting.
,
Aug 5 2016
John, I can confirm that 53.0.2785.46 beta-m (64-bit) appears to be working as expected. Though I am intrigued as to why ashejole's computer doesn't seem to support GPU decoding. Let me know if you're able to diagnose that. Thanks, Dustin |
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by dustin.k...@gmail.com
, Jul 5 2016