30fps videos do not run smoothly with surfaces for video turned on |
|||||
Issue descriptionFrom mlamouri: "30fps videos smoothness seems to have significantly regressed on surface pro and macbook pro" It appears that this when Blink aborts main frames with nothing to do, then cc is taking longer to respond. The DisplayScheduler is waiting for cc to respond, and is inconsistent about smoothly putting up frames from the video provider in the meantime.
,
Aug 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/430c8fbe06f29f492df7f212a9cc83d272d7634b commit 430c8fbe06f29f492df7f212a9cc83d272d7634b Author: Adrienne Walker <enne@chromium.org> Date: Fri Aug 17 00:49:46 2018 cc: Finish frame early if main thread is idle regardless of redraw state This condition was added originally in this patch: https://codereview.chromium.org/27200003 That patch was trying to trigger early deadlines when the compositor thread had work to do but the main thread had aborted. In a world where we have begin frame acks even when frames aren't produced, it's worth triggering early deadlines even when the compositor thread doesn't have any work to do either so that the display compositor can go ahead and do work earlier. This is intended as a short-term fix to solve smoothness issues with surface for video with 30fps videos. Ideally the DisplayCompositor will have some followups as well so that one client that is slow to respond will not cause smoothness issues with another client. Bug: 874676 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: I8b6663570aba7d135c1f211a21c37b1e17d19577 Reviewed-on: https://chromium-review.googlesource.com/1176559 Commit-Queue: enne <enne@chromium.org> Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org> Cr-Commit-Position: refs/heads/master@{#583908} [modify] https://crrev.com/430c8fbe06f29f492df7f212a9cc83d272d7634b/cc/scheduler/scheduler_state_machine.cc
,
Aug 17
Based on issue 875255 , I am not sure that there are any short-term fixes that are reasonably possible in the renderer scheduler. samans: I'm assigning this back to you to fix the DisplayScheduler to not be sensitive to late frames from other frame producers. If that's the fix, then maybe issue 874680 should be duped back into this one.
,
Aug 27
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2e5401607fcfe8d434f2f91ae37a76222d9df322 commit 2e5401607fcfe8d434f2f91ae37a76222d9df322 Author: enne <enne@chromium.org> Date: Mon Aug 27 20:41:09 2018 Revert "cc: Finish frame early if main thread is idle regardless of redraw state" This reverts commit 430c8fbe06f29f492df7f212a9cc83d272d7634b. Reason for revert: various perf regressions. This should instead be solved via issue 874680. Original change's description: > cc: Finish frame early if main thread is idle regardless of redraw state > > This condition was added originally in this patch: > https://codereview.chromium.org/27200003 > > That patch was trying to trigger early deadlines when the compositor > thread had work to do but the main thread had aborted. > > In a world where we have begin frame acks even when frames aren't > produced, it's worth triggering early deadlines even when the compositor > thread doesn't have any work to do either so that the display compositor > can go ahead and do work earlier. > > This is intended as a short-term fix to solve smoothness issues with > surface for video with 30fps videos. Ideally the DisplayCompositor will > have some followups as well so that one client that is slow to respond > will not cause smoothness issues with another client. > > Bug: 874676 > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel > Change-Id: I8b6663570aba7d135c1f211a21c37b1e17d19577 > Reviewed-on: https://chromium-review.googlesource.com/1176559 > Commit-Queue: enne <enne@chromium.org> > Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org> > Cr-Commit-Position: refs/heads/master@{#583908} TBR=enne@chromium.org,sunnyps@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 874676 , 875255 , 875316, 875323 , 875328 , 875424 , 876099 Change-Id: I4fd531fe07396ec2275bfd9109c8d86c6b4778f5 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/1191326 Commit-Queue: enne <enne@chromium.org> Reviewed-by: enne <enne@chromium.org> Cr-Commit-Position: refs/heads/master@{#586402} [modify] https://crrev.com/2e5401607fcfe8d434f2f91ae37a76222d9df322/cc/scheduler/scheduler_state_machine.cc
,
Aug 27
To summarise the results from the bots, we've seen that this fix had a very good impact on the regressions and fixed many of them.
,
Aug 28
,
Sep 5
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c13dcf77ea89896271187aa929923ed84bc57dd0 commit c13dcf77ea89896271187aa929923ed84bc57dd0 Author: Sunny Sachanandani <sunnyps@chromium.org> Date: Tue Sep 04 23:48:35 2018 cc: Finish frame early if main thread is idle regardless of redraw state This condition was added originally in this patch: https://codereview.chromium.org/27200003 That patch was trying to trigger early deadlines when the compositor thread had work to do but the main thread had aborted. In a world where we have begin frame acks even when frames aren't produced, it's worth triggering early deadlines even when the compositor thread doesn't have any work to do either so that the display compositor can go ahead and do work earlier. This is intended as a short-term fix to solve smoothness issues with surface for video with 30fps videos. Ideally the DisplayCompositor will have some followups as well so that one client that is slow to respond will not cause smoothness issues with another client. Bug: 874676 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ib442797dda715c7a34ef8c1cba740cb44c94eb60 Reviewed-on: https://chromium-review.googlesource.com/1205101 Reviewed-by: enne <enne@chromium.org> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Cr-Commit-Position: refs/heads/master@{#588694} [modify] https://crrev.com/c13dcf77ea89896271187aa929923ed84bc57dd0/cc/scheduler/scheduler_state_machine.cc [modify] https://crrev.com/c13dcf77ea89896271187aa929923ed84bc57dd0/cc/scheduler/scheduler_state_machine_unittest.cc
,
Sep 14
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2bbe289cc8e1ba48e6aac024521bafeb489f3ea0 commit 2bbe289cc8e1ba48e6aac024521bafeb489f3ea0 Author: Sunny Sachanandani <sunnyps@chromium.org> Date: Fri Sep 14 00:25:21 2018 Revert "cc: Finish frame early if main thread is idle regardless of redraw state" This reverts commit c13dcf77ea89896271187aa929923ed84bc57dd0. Reason for revert: Cause of many regressions (see linked bugs) Original change's description: > cc: Finish frame early if main thread is idle regardless of redraw state > > This condition was added originally in this patch: > https://codereview.chromium.org/27200003 > > That patch was trying to trigger early deadlines when the compositor > thread had work to do but the main thread had aborted. > > In a world where we have begin frame acks even when frames aren't > produced, it's worth triggering early deadlines even when the compositor > thread doesn't have any work to do either so that the display compositor > can go ahead and do work earlier. > > This is intended as a short-term fix to solve smoothness issues with > surface for video with 30fps videos. Ideally the DisplayCompositor will > have some followups as well so that one client that is slow to respond > will not cause smoothness issues with another client. > > Bug: 874676 > Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel > Change-Id: Ib442797dda715c7a34ef8c1cba740cb44c94eb60 > Reviewed-on: https://chromium-review.googlesource.com/1205101 > Reviewed-by: enne <enne@chromium.org> > Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> > Cr-Commit-Position: refs/heads/master@{#588694} TBR=enne@chromium.org,sunnyps@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 874676 , 880957, 880856 Change-Id: I7813aab2c71df33b0d8c184c174f2e73cbf64256 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Reviewed-on: https://chromium-review.googlesource.com/1211664 Reviewed-by: Sunny Sachanandani <sunnyps@chromium.org> Reviewed-by: enne <enne@chromium.org> Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Cr-Commit-Position: refs/heads/master@{#591224} [modify] https://crrev.com/2bbe289cc8e1ba48e6aac024521bafeb489f3ea0/cc/scheduler/scheduler_state_machine.cc [modify] https://crrev.com/2bbe289cc8e1ba48e6aac024521bafeb489f3ea0/cc/scheduler/scheduler_state_machine_unittest.cc
,
Sep 14
crouleau fyi
,
Sep 20
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/38a7b535f8e60563c53ba4ae080f47874350ba6b commit 38a7b535f8e60563c53ba4ae080f47874350ba6b Author: Sunny Sachanandani <sunnyps@chromium.org> Date: Thu Sep 20 20:46:02 2018 cc: Send early DidNotProduceFrame if draw isn't expected Send an early DidNotProduceFrame if the scheduler uses a late deadline, and no new frame is expected. The DisplayScheduler uses this as a signal to draw if other producers have submitted frames. This is intended as a short-term fix to solve smoothness issues with surface for video with 30fps videos. Ideally the DisplayCompositor will have some followups as well so that one client that is slow to respond will not cause smoothness issues with another client. Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;master.tryserver.blink:linux_trusty_blink_rel Change-Id: Icf2f067dd08836de24ea5d0cdc896dc433062d5c Bug: 874676 Reviewed-on: https://chromium-review.googlesource.com/1211917 Commit-Queue: Sunny Sachanandani <sunnyps@chromium.org> Reviewed-by: enne <enne@chromium.org> Cr-Commit-Position: refs/heads/master@{#592936} [modify] https://crrev.com/38a7b535f8e60563c53ba4ae080f47874350ba6b/cc/scheduler/scheduler.cc [modify] https://crrev.com/38a7b535f8e60563c53ba4ae080f47874350ba6b/cc/scheduler/scheduler.h [modify] https://crrev.com/38a7b535f8e60563c53ba4ae080f47874350ba6b/cc/scheduler/scheduler_unittest.cc
,
Dec 14
samans@, do you know if the fix in #10 addressed the immediate issue? Should this still be a P1.
,
Dec 14
I think so because video surfaces launched. I'm going to close this bug. mlamouri@ please feel free to reopen if I'm wrong. |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by dalecur...@chromium.org
, Aug 16