[Video] Doesn't show when video in surfaces is enabled. |
||||||||||
Issue description1. Run chrome locally with --enable-features=UseSurfaceLayerForVideo. 2. Navigate to a site with a video. Play video. 3. Observe audio plays normally. [live] https://www.youtube.com/watch?v=KkfdRzLdbG0 - see screenshot [not live] https://www.youtube.com/watch?v=VwijdAREBEc - same as above [non yt] https://www.w3.org/2010/05/video/mediaevents.html - shows poster image as a still
,
Feb 12 2018
+fbeaufort@ who found the exact same bug today.
,
Feb 13 2018
After talking to sergeyu@, it seems like chrome remote desktop relies on the software path, which isnt perfect yet for viz.
,
Feb 13 2018
+danakj It looks like UseSurfaceLayerForVideo and software compositing is broken. You can reproduce by running locally with the following: $ ./chrome --enable-features=UseSurfaceLayerForVideo --disable-gpu www.youtube.com It looks like UseSurfaceLayerForVideo is doing something wrong with SharedBitmap allocation. This is unrelated to VizDisplayCompositor which is disabled by default. You would have to turn on --enable-features="UseSurfaceLayerForVideo,VizDisplayCompositor" to see them work together. It's possible that the work in crbug.com/730660 will be somewhat related to what you need, but it's not the cause of this issue.
,
Feb 13 2018
Yeah I think so also. I suggested looking at the mojo notifier Ipcs that happen from the ChildBitmapManager, and the display compositors interactions with the ServerSharedBitmapManager. And that site-isolation OOPIFs are an example of surfaces working with software compositing for comparison (slashdot.org has some for eg). 730660 may fix this by accident but is more likely to break it :P It would be orthogonal to understanding why this doesn't work tho, part of 730660 will be moving the current (not working) code from one API to another, but the underlying issues would likely persist.
,
Feb 13 2018
,
Feb 22 2018
,
Feb 22 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/db88a6705da3280e663a675bb415ed22d8a7de3f commit db88a6705da3280e663a675bb415ed22d8a7de3f Author: CJ DiMeglio <lethalantidote@chromium.org> Date: Thu Feb 22 18:48:14 2018 Shuts off SurfaceLayerForVideo in disable gpu case. This is a temp fix disabling Surface Layers for video in the case of disabling the gpu. It does not fix the issue, but make the feature more stable for testing until we can in fact debug the problem. Bug: 807840 Change-Id: Iada63e5b2b4c27988a75b56d51851a26f793a7be Reviewed-on: https://chromium-review.googlesource.com/927223 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org> Cr-Commit-Position: refs/heads/master@{#538501} [modify] https://crrev.com/db88a6705da3280e663a675bb415ed22d8a7de3f/content/renderer/media/media_factory.cc [modify] https://crrev.com/db88a6705da3280e663a675bb415ed22d8a7de3f/media/blink/video_frame_compositor.cc [modify] https://crrev.com/db88a6705da3280e663a675bb415ed22d8a7de3f/media/blink/video_frame_compositor.h [modify] https://crrev.com/db88a6705da3280e663a675bb415ed22d8a7de3f/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/db88a6705da3280e663a675bb415ed22d8a7de3f/media/blink/webmediaplayer_impl_unittest.cc [modify] https://crrev.com/db88a6705da3280e663a675bb415ed22d8a7de3f/media/blink/webmediaplayer_params.cc [modify] https://crrev.com/db88a6705da3280e663a675bb415ed22d8a7de3f/media/blink/webmediaplayer_params.h
,
Feb 27 2018
,
May 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e189916274cf571d7fa27d7a00dcb93da4f6568e commit e189916274cf571d7fa27d7a00dcb93da4f6568e Author: François Beaufort <beaufort.francois@gmail.com> Date: Mon May 07 14:54:00 2018 [Picture-in-Picture] Disable if no GPU compositing. This is a temporary fix that sets document.pictureInPictureEnabled to false in the case of disabling the GPU. Bug: 806249 , 807840 Change-Id: I84445a0231b46cfbd0bb2f25eaca53f96604ce55 Reviewed-on: https://chromium-review.googlesource.com/1042290 Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Cr-Commit-Position: refs/heads/master@{#556438} [modify] https://crrev.com/e189916274cf571d7fa27d7a00dcb93da4f6568e/content/renderer/render_view_impl.cc
,
May 8 2018
Tested this issue on Windows 10 and Mac OS 10.12.6 on the build without fix 66.0.3335.0 and unable to reproduce the issue following the steps given in the original comment. On launching chrome with the flag --enable-features=UseSurfaceLayerForVideo, and navigating to the above given links, can observe that the video is playing without any issues. Attached is the screen cast for reference. lethalantidote@ Request you to check and confirm if anything is missed from our end in testing the issue. Also help us in verifying the fix on the latest M-68 68.0.3424.0 build. Thanks...
,
May 9 2018
I am trying to repro this issue locally and cannot anymore. I do not see the problem in 68.0.3425.0 (Developer Build) (64-bit). I will be testing it using remote desktop once I get home.
,
May 9 2018
This still fails using remote desktop.
,
May 9 2018
lethalantidote: I'd imagine with CRD it's using software compositing. Check chrome://gpu to confirm that.
,
May 9 2018
Actually, it still repros. Make sure you are using --disable-gpu and NOT --disable_gpu
,
May 11 2018
,
May 11 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/123a218dc0dae928c4e32bc5acfe979943e1767d commit 123a218dc0dae928c4e32bc5acfe979943e1767d Author: kylechar <kylechar@chromium.org> Date: Fri May 11 20:49:05 2018 Add SharedBitmapReporter interface. VideoResourceUpdater currently uses LayerTreeFrameSink as the interface through which to notify the display compositor about shared bitmaps. This doesn't work UseSurfaceLayerForVideo where there is no LayerTreeFrameSink. Pull the two required functions out of LayerTreeFrameSink and into SharedBitmapReporter. LayerTreeFrameSink has SharedBitmapReporter as a subclass and doesn't implement the functions so nothing changes with regards to it. VideoResourceUpdater now takes a SharedBitmapReporter instead of a LayerTreeFrameSink. An implementation of SharedBitmapReporter that calls into mojom::CompositorFrameSinkPtr can be added trivially to support UseSurfaceLayerInVideo after this patch. Bug: 807840 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I9a5a07140f8aa1614d1b647f3af1dd5d7aab2cd2 Reviewed-on: https://chromium-review.googlesource.com/1053054 Commit-Queue: kylechar <kylechar@chromium.org> Reviewed-by: danakj <danakj@chromium.org> Cr-Commit-Position: refs/heads/master@{#558011} [modify] https://crrev.com/123a218dc0dae928c4e32bc5acfe979943e1767d/cc/layers/video_layer_impl.cc [modify] https://crrev.com/123a218dc0dae928c4e32bc5acfe979943e1767d/cc/resources/video_resource_updater.cc [modify] https://crrev.com/123a218dc0dae928c4e32bc5acfe979943e1767d/cc/resources/video_resource_updater.h [modify] https://crrev.com/123a218dc0dae928c4e32bc5acfe979943e1767d/cc/trees/layer_tree_frame_sink.h [modify] https://crrev.com/123a218dc0dae928c4e32bc5acfe979943e1767d/components/viz/common/BUILD.gn [add] https://crrev.com/123a218dc0dae928c4e32bc5acfe979943e1767d/components/viz/common/resources/shared_bitmap_reporter.cc [add] https://crrev.com/123a218dc0dae928c4e32bc5acfe979943e1767d/components/viz/common/resources/shared_bitmap_reporter.h
,
May 14 2018
Tested this issue on Windows 10, Ubuntu 14.04 and Mac OS 10.13.3 on the build without fix 66.0.3335.0. - Able to reproduce the issue by launching Chrome with flags --enable-features=UseSurfaceLayerForVideo --disable-gpu on Windows 10 on the build without fix and unable to reproduce the issue on linux and Mac OS. - Issue is fixed on Windows 10 on the latest M-68 68.0.3430.0 build - On Mac OS, chrome 66.0.3335.0 is crashing while navigating to the links given in the original comment, Attached is the screen casts for Windows and screen shot of linux behavior for reference. lethalantidote@ Request you to check and confirm if anything is missed from our end in testing the issue. Also help us in verifying the fix on the latest M-68 68.0.3430.0 build. Thanks...
,
May 14 2018
susan.boorgula: UseSurfaceLayerForVideo is disabled when --disable-gpu is provided. You would need to recompile without this line to test properly. https://cs.chromium.org/chromium/src/content/renderer/media/media_factory.cc?l=272&rcl=b5a26ee1d117a1d085ee2890dfa37efcd9d5a28b
,
May 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e09b316de4c2101695a8e2541dc907ff7e692d45 commit e09b316de4c2101695a8e2541dc907ff7e692d45 Author: CJ DiMeglio <lethalantidote@chromium.org> Date: Tue May 15 00:14:14 2018 Updates VideoFrameResourceProvider to allow for null context_provider. This CL allows for the option for the context_provider in VideoFrameResourceProvider to be null. This is necessary because when we are in software compositing mode, we will have a null context_provider. Bug: 807840 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: Idac8503b57b7a3490402f6db52092ffa4bd99d83 Reviewed-on: https://chromium-review.googlesource.com/1055110 Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org> Reviewed-by: Rick Byers <rbyers@chromium.org> Reviewed-by: Frank Liberato <liberato@chromium.org> Reviewed-by: Justin Novosad <junov@chromium.org> Cr-Commit-Position: refs/heads/master@{#558542} [modify] https://crrev.com/e09b316de4c2101695a8e2541dc907ff7e692d45/content/renderer/media/media_factory.cc [modify] https://crrev.com/e09b316de4c2101695a8e2541dc907ff7e692d45/third_party/blink/public/platform/web_video_frame_submitter.h [modify] https://crrev.com/e09b316de4c2101695a8e2541dc907ff7e692d45/third_party/blink/renderer/platform/graphics/surface_layer_bridge.cc [modify] https://crrev.com/e09b316de4c2101695a8e2541dc907ff7e692d45/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc [modify] https://crrev.com/e09b316de4c2101695a8e2541dc907ff7e692d45/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.h [modify] https://crrev.com/e09b316de4c2101695a8e2541dc907ff7e692d45/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc [modify] https://crrev.com/e09b316de4c2101695a8e2541dc907ff7e692d45/third_party/blink/renderer/platform/graphics/video_frame_submitter.h [modify] https://crrev.com/e09b316de4c2101695a8e2541dc907ff7e692d45/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc
,
May 16 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/298ef248dd85dc2f747982ad1ac3589124873c92 commit 298ef248dd85dc2f747982ad1ac3589124873c92 Author: CJ DiMeglio <lethalantidote@chromium.org> Date: Wed May 16 20:32:21 2018 Allows cc::Surfaces for video to work with software compositing. This CL makes software compositing for the cc::Surface for video case by making VideoFrameSubmitter a SharedBitmapReporter. Before we would see blue boxes instead of the video frames. Now we see video frames as they should be. Bug: 807840 Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2 Change-Id: I486456f4d4e96b05317089557084242eeda04455 Reviewed-on: https://chromium-review.googlesource.com/1057962 Reviewed-by: Justin Novosad <junov@chromium.org> Reviewed-by: Frank Liberato <liberato@chromium.org> Commit-Queue: CJ DiMeglio <lethalantidote@chromium.org> Cr-Commit-Position: refs/heads/master@{#559267} [modify] https://crrev.com/298ef248dd85dc2f747982ad1ac3589124873c92/content/renderer/media/media_factory.cc [modify] https://crrev.com/298ef248dd85dc2f747982ad1ac3589124873c92/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.cc [modify] https://crrev.com/298ef248dd85dc2f747982ad1ac3589124873c92/third_party/blink/renderer/platform/graphics/video_frame_resource_provider.h [modify] https://crrev.com/298ef248dd85dc2f747982ad1ac3589124873c92/third_party/blink/renderer/platform/graphics/video_frame_submitter.cc [modify] https://crrev.com/298ef248dd85dc2f747982ad1ac3589124873c92/third_party/blink/renderer/platform/graphics/video_frame_submitter.h [modify] https://crrev.com/298ef248dd85dc2f747982ad1ac3589124873c92/third_party/blink/renderer/platform/graphics/video_frame_submitter_test.cc
,
May 16 2018
,
May 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f4a5eacc8259fa41850582fc2374e4d88c7dc407 commit f4a5eacc8259fa41850582fc2374e4d88c7dc407 Author: François Beaufort <beaufort.francois@gmail.com> Date: Thu May 17 09:31:36 2018 [Picture-in-Picture] Enable if no GPU compositing. This reverts https://chromium-review.googlesource.com/1042290 since cc::Surfaces for video are now allowed to work with software compositing. Bug: 806249 , 807840 Change-Id: Ie02c69d527b8e13235d744d3c2e98ea5b073f61d Reviewed-on: https://chromium-review.googlesource.com/1062745 Reviewed-by: Jochen Eisinger <jochen@chromium.org> Commit-Queue: François Beaufort <beaufort.francois@gmail.com> Cr-Commit-Position: refs/heads/master@{#559470} [modify] https://crrev.com/f4a5eacc8259fa41850582fc2374e4d88c7dc407/content/renderer/render_view_impl.cc |
||||||||||
►
Sign in to add a comment |
||||||||||
Comment 1 by hohlic...@gmail.com
, Feb 1 2018