Reduce the number of loaded, but never played media players. |
||||||||||||||
Issue descriptionMore and more sites are starting to have bad behavior around loading for clips. E.g., vimeo's mobile site creates MSE players non-stop as they roll onto the screen. Many emerging markets sites spawn ridiculous numbers of players in a single page. I posit that we want to go beyond just setting preload=metadata, as suggested by issue 310450 , and actually change what that means for Chrome's pipeline. Today preload=metadata just means we stop preloading data after spooling decoders for metadata and the first frame. Instead we should consider not spooling the decoders, but still start download and metadata extraction -- just no first frame, in situations like the following: - video is not playing and has a poster image (whether mse or src=) - video is preload auto|metadata and offscreen and not-playing. - video is mse and offscreen and not playing. This is most important on platforms like Android where there are low memory and codec creation constraints. Some of this will require us to change how HTMLME thinks of its playing state; today WMPI isn't told about playing if prior to have_future_data; which we want to fix for other reasons too. Step one is to land a measurement for utilization of players...
,
Feb 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c58ff8ebedea639ba58fb30a2e29faca9f8a0407 commit c58ff8ebedea639ba58fb30a2e29faca9f8a0407 Author: dalecurtis <dalecurtis@chromium.org> Date: Wed Feb 22 18:20:45 2017 Add metric for tracking media playback utilization. I.e., how many players completed preload but never saw playback. This will be handy in terms of guiding efforts to reduce the number of these types of "useless" players. I.e. maybe deferred offscreen loading, etc. BUG= 694855 TEST=manual Review-Url: https://codereview.chromium.org/2709903002 Cr-Commit-Position: refs/heads/master@{#452135} [modify] https://crrev.com/c58ff8ebedea639ba58fb30a2e29faca9f8a0407/content/browser/media/media_internals.cc [modify] https://crrev.com/c58ff8ebedea639ba58fb30a2e29faca9f8a0407/tools/metrics/histograms/histograms.xml
,
Feb 22 2017
MR-57 for metrics on stable faster.
,
Feb 23 2017
Your change meets the bar and is auto-approved for M57. Please go ahead and merge the CL to branch 2987 manually. Please contact milestone owner if you have questions. Owners: amineer@(clank), cmasso@(bling), ketakid@(cros), govind@(desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Feb 23 2017
If possible, could you please merge your change to M57 branch 2987 by 5:00 PM PT today, or latest by 4.00 PM PT, friday (02/24). Thank you.
,
Feb 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/24d1a80b879c17c908312ffdd84c8c6b76ad325a commit 24d1a80b879c17c908312ffdd84c8c6b76ad325a Author: Dale Curtis <dalecurtis@chromium.org> Date: Thu Feb 23 20:00:38 2017 Merge M57: "Add metric for tracking media playback utilization." I.e., how many players completed preload but never saw playback. This will be handy in terms of guiding efforts to reduce the number of these types of "useless" players. I.e. maybe deferred offscreen loading, etc. BUG= 694855 TEST=manual Review-Url: https://codereview.chromium.org/2709903002 Cr-Commit-Position: refs/heads/master@{#452135} (cherry picked from commit 35848da5131b081ee4751be9669230223baf7c43) Review-Url: https://codereview.chromium.org/2711803006 . Cr-Commit-Position: refs/branch-heads/2987@{#667} Cr-Branched-From: ad51088c0e8776e8dcd963dbe752c4035ba6dab6-refs/heads/master@{#444943} [modify] https://crrev.com/24d1a80b879c17c908312ffdd84c8c6b76ad325a/content/browser/media/media_internals.cc [modify] https://crrev.com/24d1a80b879c17c908312ffdd84c8c6b76ad325a/tools/metrics/histograms/histograms.xml
,
Feb 23 2017
,
Apr 10 2017
To update, the numbers are ~30% for desktop and ~50% for Android; tguilbert@ has some deferred loading projects he's going to look into this Q, so over to him.
,
Aug 31 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/cd8981cd5f87abb7c649668fb2010a515ec677a0 commit cd8981cd5f87abb7c649668fb2010a515ec677a0 Author: Dan Sanders <sandersd@chromium.org> Date: Thu Aug 31 22:37:02 2017 [media] UMA to measure poster availability for SRC= loads. Change-Id: I17985450f1cb0e8c2dea4ba29c786a8a9871ce83 Bug: 694855 Reviewed-on: https://chromium-review.googlesource.com/646384 Reviewed-by: Steven Holte <holte@chromium.org> Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Commit-Queue: Dan Sanders <sandersd@chromium.org> Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/heads/master@{#499054} [modify] https://crrev.com/cd8981cd5f87abb7c649668fb2010a515ec677a0/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/cd8981cd5f87abb7c649668fb2010a515ec677a0/media/blink/webmediaplayer_impl.h [modify] https://crrev.com/cd8981cd5f87abb7c649668fb2010a515ec677a0/tools/metrics/histograms/histograms.xml
,
Jan 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/82662e2171c9437e4141aeafd03c0841b96d2bc8 commit 82662e2171c9437e4141aeafd03c0841b96d2bc8 Author: Dale Curtis <dalecurtis@chromium.org> Date: Wed Jan 31 20:22:21 2018 Add layout tests exercising suspend/resume of src= media elements. Testing is accomplished by a couple new window.internals.* methods which allow modification and inspection of suspend state: - forceStaleStateForMediaElement(element) - isMediaElementSuspended(element) Unfortunately due to the async nature of suspend the method for testing suspend must be polled once a idle suspend is initiated via the forceStaleStateForMediaElement(). Test helper methods have been added to media/suspend-util.js to alleviate test cruft. forceStaleStateForMediaElement() does not actually force a suspend, it simply marks the element as stale. If an element is playing, seeking, or otherwise in a non-suspendable state, no suspend will occur. This is intentional to ensure tests cover our state machine as close to reality as possible. Unfortunately the only way to force given ready states for the media element is through throttled loading; this CL makes extensive use of throttled loading and adds a throttling limit to our layout test loader to avoid timeouts with slow initial rates. These tests were run with a --repeat-each=200 several times without flake, but I'll also watch the flakiness dashboard for these afterward. As a result of the above combined with the fact that we are not yet propagating play state to WMPI, we can reach a timeout state in the 'suspend after metadata' src= test. It's marked as pass and timeout in TestExpectations until the following lands: https://chromium-review.googlesource.com/c/chromium/src/+/876924 This is part 1 of 2, the 2nd part will add MSE based variants of these tests. No new internals methods are expected though, just a appendBuffer() based mechanism for triggering ready states. BUG= 694855 , 756897 TEST=it's all tests! Change-Id: I84285ea49161f24cdd885bfc8b9224aa51fa5643 Reviewed-on: https://chromium-review.googlesource.com/882283 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Philip Jägenstedt <foolip@chromium.org> Reviewed-by: Peter Beverloo <peter@chromium.org> Reviewed-by: Mounir Lamouri (slow) <mlamouri@chromium.org> Cr-Commit-Position: refs/heads/master@{#533380} [modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc [modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/content/shell/renderer/layout_test/layout_test_content_renderer_client.h [modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/media/blink/webmediaplayer_impl.h [modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/TestExpectations [add] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-after-have-future-data.html [add] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-after-have-metadata.html [add] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-before-have-metadata.html [modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/http/tests/media/video-throttled-load.cgi [add] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/media/media-src-suspend-after-have-enough-data.html [add] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/LayoutTests/media/suspend-util.js [modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/Source/core/testing/Internals.cpp [modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/Source/core/testing/Internals.h [modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/Source/core/testing/Internals.idl [modify] https://crrev.com/82662e2171c9437e4141aeafd03c0841b96d2bc8/third_party/WebKit/public/platform/WebMediaPlayer.h
,
Jan 31 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b commit 9f1fa9c053e29a586ccbd52c1f58d16e6de3318b Author: Tommy Steimel <steimel@chromium.org> Date: Wed Jan 31 21:33:04 2018 Revert "Add layout tests exercising suspend/resume of src= media elements." This reverts commit 82662e2171c9437e4141aeafd03c0841b96d2bc8. Reason for revert: http/tests/media/media-src-suspend-after-have-future-data.html is failing on WebKit Mac bots Original change's description: > Add layout tests exercising suspend/resume of src= media elements. > > Testing is accomplished by a couple new window.internals.* methods > which allow modification and inspection of suspend state: > - forceStaleStateForMediaElement(element) > - isMediaElementSuspended(element) > > Unfortunately due to the async nature of suspend the method for > testing suspend must be polled once a idle suspend is initiated via > the forceStaleStateForMediaElement(). Test helper methods have been > added to media/suspend-util.js to alleviate test cruft. > > forceStaleStateForMediaElement() does not actually force a suspend, > it simply marks the element as stale. If an element is playing, > seeking, or otherwise in a non-suspendable state, no suspend will > occur. This is intentional to ensure tests cover our state machine > as close to reality as possible. > > Unfortunately the only way to force given ready states for the > media element is through throttled loading; this CL makes extensive > use of throttled loading and adds a throttling limit to our layout > test loader to avoid timeouts with slow initial rates. These tests > were run with a --repeat-each=200 several times without flake, but > I'll also watch the flakiness dashboard for these afterward. > > As a result of the above combined with the fact that we are not > yet propagating play state to WMPI, we can reach a timeout state > in the 'suspend after metadata' src= test. It's marked as pass > and timeout in TestExpectations until the following lands: > > https://chromium-review.googlesource.com/c/chromium/src/+/876924 > > This is part 1 of 2, the 2nd part will add MSE based variants of > these tests. No new internals methods are expected though, just > a appendBuffer() based mechanism for triggering ready states. > > BUG= 694855 , 756897 > TEST=it's all tests! > > Change-Id: I84285ea49161f24cdd885bfc8b9224aa51fa5643 > Reviewed-on: https://chromium-review.googlesource.com/882283 > Commit-Queue: Dale Curtis <dalecurtis@chromium.org> > Reviewed-by: Philip Jägenstedt <foolip@chromium.org> > Reviewed-by: Peter Beverloo <peter@chromium.org> > Reviewed-by: Mounir Lamouri (slow) <mlamouri@chromium.org> > Cr-Commit-Position: refs/heads/master@{#533380} TBR=dalecurtis@chromium.org,peter@chromium.org,mlamouri@chromium.org,foolip@chromium.org Change-Id: I9225560f2d01b6f874ddb219641f72b3d5863741 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 694855 , 756897 Reviewed-on: https://chromium-review.googlesource.com/896111 Reviewed-by: Tommy Steimel <steimel@chromium.org> Commit-Queue: Tommy Steimel <steimel@chromium.org> Cr-Commit-Position: refs/heads/master@{#533418} [modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/content/shell/renderer/layout_test/layout_test_content_renderer_client.cc [modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/content/shell/renderer/layout_test/layout_test_content_renderer_client.h [modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/media/blink/webmediaplayer_impl.h [modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/third_party/WebKit/LayoutTests/TestExpectations [delete] https://crrev.com/28ec2ab2f5f3cf467e8987fbc7d6f8e883ba4c3b/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-after-have-future-data.html [delete] https://crrev.com/28ec2ab2f5f3cf467e8987fbc7d6f8e883ba4c3b/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-after-have-metadata.html [delete] https://crrev.com/28ec2ab2f5f3cf467e8987fbc7d6f8e883ba4c3b/third_party/WebKit/LayoutTests/http/tests/media/media-src-suspend-before-have-metadata.html [modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/third_party/WebKit/LayoutTests/http/tests/media/video-throttled-load.cgi [delete] https://crrev.com/28ec2ab2f5f3cf467e8987fbc7d6f8e883ba4c3b/third_party/WebKit/LayoutTests/media/media-src-suspend-after-have-enough-data.html [delete] https://crrev.com/28ec2ab2f5f3cf467e8987fbc7d6f8e883ba4c3b/third_party/WebKit/LayoutTests/media/suspend-util.js [modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/third_party/WebKit/Source/core/testing/Internals.cpp [modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/third_party/WebKit/Source/core/testing/Internals.h [modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/third_party/WebKit/Source/core/testing/Internals.idl [modify] https://crrev.com/9f1fa9c053e29a586ccbd52c1f58d16e6de3318b/third_party/WebKit/public/platform/WebMediaPlayer.h
,
Feb 2 2018
Design doc for reducing wastage: https://docs.google.com/document/d/1fSi8-sj6de7NtrSKf4A-fdXTLGgJEZ96jecSR1-zlBA/edit?ts=5a614edd tl;dr: we should stop at the have_metadata state when "preload=metadata && (content is audio only || has a poster image)"
,
Feb 2 2018
,
Feb 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f81942cadc658e33042acb8d932cef238373db77 commit f81942cadc658e33042acb8d932cef238373db77 Author: Dale Curtis <dalecurtis@chromium.org> Date: Wed Feb 07 10:01:45 2018 Remove kHaveFutureData restriction from play state notifications. HTMLMediaElement should tell WebMediaPlayer about play state changes as soon as the kHaveMetaData state is reached. This will eventually allow us to delete a bunch of guessing-game type code for idle suspension and instead rely on actually getting a play() call from Blink to wake up when needed. We can't stop WMPI from reaching the future data state quite yet though, because it has implications on the delivery of 'canplay' and 'canplaythrough' events that need to be part of a larger preload=metadata experiment. See old discussion on this here: https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Jul/0007.html Note: This change exposed some bugs in how the ready state maximum is used and the states at which a video frame is expected. These issues are fixed in the HTMLVideoElement. BUG= 694855 , 756897 TEST=existing suspend tests no longer flake. Change-Id: I45478c8238dbd48d526933caacc4cf106583d415 Reviewed-on: https://chromium-review.googlesource.com/876743 Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Cr-Commit-Position: refs/heads/master@{#534973} [modify] https://crrev.com/f81942cadc658e33042acb8d932cef238373db77/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/f81942cadc658e33042acb8d932cef238373db77/third_party/WebKit/Source/core/html/media/HTMLMediaElement.cpp [modify] https://crrev.com/f81942cadc658e33042acb8d932cef238373db77/third_party/WebKit/Source/core/html/media/HTMLVideoElement.cpp
,
Feb 7 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0affd0504bdad5b2a5c4777f46f200a6a43eb9ed commit 0affd0504bdad5b2a5c4777f46f200a6a43eb9ed Author: Guido Urdaneta <guidou@chromium.org> Date: Wed Feb 07 15:55:09 2018 Revert "Remove kHaveFutureData restriction from play state notifications." This reverts commit f81942cadc658e33042acb8d932cef238373db77. Reason for revert: Causes reliable failure on Mac bot for the following test: http/tests/media/media-src-suspend-after-have-future-data.html First failure: https://ci.chromium.org/buildbot/chromium.webkit/WebKit%20Mac10.12/10636 Original change's description: > Remove kHaveFutureData restriction from play state notifications. > > HTMLMediaElement should tell WebMediaPlayer about play state changes > as soon as the kHaveMetaData state is reached. > > This will eventually allow us to delete a bunch of guessing-game type > code for idle suspension and instead rely on actually getting a play() > call from Blink to wake up when needed. > > We can't stop WMPI from reaching the future data state quite yet though, > because it has implications on the delivery of 'canplay' and 'canplaythrough' > events that need to be part of a larger preload=metadata experiment. See old > discussion on this here: > > https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Jul/0007.html > > Note: This change exposed some bugs in how the ready state maximum is used > and the states at which a video frame is expected. These issues are fixed > in the HTMLVideoElement. > > BUG= 694855 , 756897 > TEST=existing suspend tests no longer flake. > > Change-Id: I45478c8238dbd48d526933caacc4cf106583d415 > Reviewed-on: https://chromium-review.googlesource.com/876743 > Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> > Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> > Cr-Commit-Position: refs/heads/master@{#534973} TBR=dalecurtis@chromium.org,mlamouri@chromium.org Change-Id: I2bfb38a8cbace8551d8a000b84c05f7b496b852d No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 694855 , 756897 Reviewed-on: https://chromium-review.googlesource.com/906770 Reviewed-by: Guido Urdaneta <guidou@chromium.org> Commit-Queue: Guido Urdaneta <guidou@chromium.org> Cr-Commit-Position: refs/heads/master@{#535013} [modify] https://crrev.com/0affd0504bdad5b2a5c4777f46f200a6a43eb9ed/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/0affd0504bdad5b2a5c4777f46f200a6a43eb9ed/third_party/WebKit/Source/core/html/media/HTMLMediaElement.cpp [modify] https://crrev.com/0affd0504bdad5b2a5c4777f46f200a6a43eb9ed/third_party/WebKit/Source/core/html/media/HTMLVideoElement.cpp
,
Feb 7 2018
,
Feb 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aaef5f10dbd2a6396dae99ebf76021a7da3d68b1 commit aaef5f10dbd2a6396dae99ebf76021a7da3d68b1 Author: Dale Curtis <dalecurtis@chromium.org> Date: Thu Feb 08 21:26:52 2018 Add support for suspending the media::Pipeline after metadata. Introduces a new StartType for media::Pipeline, which controls when to suspend after metadata. The valid values are kNormal (existing flow), kStartSuspendedForAudioOnly (suspend for audio-only), kStartSuspended (always suspend). This reuses the existing suspend/resume functionality and just trains the PipelineController to be aware that this is a possible state transition after startup. A new Pipeline::IsSuspended() method is introduced to facilitate this instead of modifying the SeekCB used in startup to avoid unnecessary churn and strengthen the existing suspend/resume state checks in the controller. We handle the suspend decision inside the Pipeline to avoid unnecessary startup delays in the autoplay path which hopping back to WMPI for such a decision would require. This does not actually enable suspend after metdata, that will be part of a larger base::Feature experiment. BUG= 694855 TEST=new unittests Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I8f3b04b1597aa0e808b04878ae9f1b3532e6d932 Reviewed-on: https://chromium-review.googlesource.com/903302 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Frank Liberato <liberato@chromium.org> Reviewed-by: Xiaohan Wang <xhwang@chromium.org> Cr-Commit-Position: refs/heads/master@{#535519} [modify] https://crrev.com/aaef5f10dbd2a6396dae99ebf76021a7da3d68b1/media/base/mock_filters.cc [modify] https://crrev.com/aaef5f10dbd2a6396dae99ebf76021a7da3d68b1/media/base/mock_filters.h [modify] https://crrev.com/aaef5f10dbd2a6396dae99ebf76021a7da3d68b1/media/base/pipeline.h [modify] https://crrev.com/aaef5f10dbd2a6396dae99ebf76021a7da3d68b1/media/base/pipeline_impl.cc [modify] https://crrev.com/aaef5f10dbd2a6396dae99ebf76021a7da3d68b1/media/base/pipeline_impl.h [modify] https://crrev.com/aaef5f10dbd2a6396dae99ebf76021a7da3d68b1/media/base/pipeline_impl_unittest.cc [modify] https://crrev.com/aaef5f10dbd2a6396dae99ebf76021a7da3d68b1/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/aaef5f10dbd2a6396dae99ebf76021a7da3d68b1/media/filters/pipeline_controller.cc [modify] https://crrev.com/aaef5f10dbd2a6396dae99ebf76021a7da3d68b1/media/filters/pipeline_controller.h [modify] https://crrev.com/aaef5f10dbd2a6396dae99ebf76021a7da3d68b1/media/filters/pipeline_controller_unittest.cc [modify] https://crrev.com/aaef5f10dbd2a6396dae99ebf76021a7da3d68b1/media/test/pipeline_integration_test_base.cc
,
Feb 8 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/074c378eb10148788e76b0705084289332c7202f commit 074c378eb10148788e76b0705084289332c7202f Author: Scott Graham <scottmg@chromium.org> Date: Thu Feb 08 23:16:49 2018 Revert "Add support for suspending the media::Pipeline after metadata." This reverts commit aaef5f10dbd2a6396dae99ebf76021a7da3d68b1. Reason for revert: Failing on CrOS, Linux Cast, and Fuchsia. Original change's description: > Add support for suspending the media::Pipeline after metadata. > > Introduces a new StartType for media::Pipeline, which controls when > to suspend after metadata. The valid values are kNormal (existing > flow), kStartSuspendedForAudioOnly (suspend for audio-only), > kStartSuspended (always suspend). > > This reuses the existing suspend/resume functionality and just trains > the PipelineController to be aware that this is a possible state > transition after startup. A new Pipeline::IsSuspended() method is > introduced to facilitate this instead of modifying the SeekCB used > in startup to avoid unnecessary churn and strengthen the existing > suspend/resume state checks in the controller. > > We handle the suspend decision inside the Pipeline to avoid unnecessary > startup delays in the autoplay path which hopping back to WMPI for such > a decision would require. > > This does not actually enable suspend after metdata, that will be part > of a larger base::Feature experiment. > > BUG= 694855 > TEST=new unittests > > Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel > Change-Id: I8f3b04b1597aa0e808b04878ae9f1b3532e6d932 > Reviewed-on: https://chromium-review.googlesource.com/903302 > Commit-Queue: Dale Curtis <dalecurtis@chromium.org> > Reviewed-by: Frank Liberato <liberato@chromium.org> > Reviewed-by: Xiaohan Wang <xhwang@chromium.org> > Cr-Commit-Position: refs/heads/master@{#535519} TBR=dalecurtis@chromium.org,xhwang@chromium.org,liberato@chromium.org Change-Id: I6c5f1467b8336fed86904f0b55c89fc081d400df No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: 694855 Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Reviewed-on: https://chromium-review.googlesource.com/910068 Reviewed-by: Scott Graham <scottmg@chromium.org> Commit-Queue: Scott Graham <scottmg@chromium.org> Cr-Commit-Position: refs/heads/master@{#535566} [modify] https://crrev.com/074c378eb10148788e76b0705084289332c7202f/media/base/mock_filters.cc [modify] https://crrev.com/074c378eb10148788e76b0705084289332c7202f/media/base/mock_filters.h [modify] https://crrev.com/074c378eb10148788e76b0705084289332c7202f/media/base/pipeline.h [modify] https://crrev.com/074c378eb10148788e76b0705084289332c7202f/media/base/pipeline_impl.cc [modify] https://crrev.com/074c378eb10148788e76b0705084289332c7202f/media/base/pipeline_impl.h [modify] https://crrev.com/074c378eb10148788e76b0705084289332c7202f/media/base/pipeline_impl_unittest.cc [modify] https://crrev.com/074c378eb10148788e76b0705084289332c7202f/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/074c378eb10148788e76b0705084289332c7202f/media/filters/pipeline_controller.cc [modify] https://crrev.com/074c378eb10148788e76b0705084289332c7202f/media/filters/pipeline_controller.h [modify] https://crrev.com/074c378eb10148788e76b0705084289332c7202f/media/filters/pipeline_controller_unittest.cc [modify] https://crrev.com/074c378eb10148788e76b0705084289332c7202f/media/test/pipeline_integration_test_base.cc
,
Feb 9 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c00d564817f2205be4d12e3ece58d0704d76e50d commit c00d564817f2205be4d12e3ece58d0704d76e50d Author: Dale Curtis <dalecurtis@chromium.org> Date: Fri Feb 09 20:55:06 2018 Add support for suspending the media::Pipeline after metadata. This is a reland of http://crrev.com/535519 which fixes the test expectations when run without DCHECKs enabled. The IsSuspended() call is in a DCHECK, so may not always be called during normal tests. -------------[ Original Description Follows ] -------------- Introduces a new StartType for media::Pipeline, which controls when to suspend after metadata. The valid values are kNormal (existing flow), kStartSuspendedForAudioOnly (suspend for audio-only), kStartSuspended (always suspend). This reuses the existing suspend/resume functionality and just trains the PipelineController to be aware that this is a possible state transition after startup. A new Pipeline::IsSuspended() method is introduced to facilitate this instead of modifying the SeekCB used in startup to avoid unnecessary churn and strengthen the existing suspend/resume state checks in the controller. We handle the suspend decision inside the Pipeline to avoid unnecessary startup delays in the autoplay path which hopping back to WMPI for such a decision would require. This does not actually enable suspend after metdata, that will be part of a larger base::Feature experiment. BUG= 694855 TEST=new unittests TBR=xhwang, liberato Cq-Include-Trybots: master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel Change-Id: I2b338a4158f261f2815eeffd0399aba04dee1c2d Reviewed-on: https://chromium-review.googlesource.com/911622 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/heads/master@{#535822} [modify] https://crrev.com/c00d564817f2205be4d12e3ece58d0704d76e50d/media/base/mock_filters.cc [modify] https://crrev.com/c00d564817f2205be4d12e3ece58d0704d76e50d/media/base/mock_filters.h [modify] https://crrev.com/c00d564817f2205be4d12e3ece58d0704d76e50d/media/base/pipeline.h [modify] https://crrev.com/c00d564817f2205be4d12e3ece58d0704d76e50d/media/base/pipeline_impl.cc [modify] https://crrev.com/c00d564817f2205be4d12e3ece58d0704d76e50d/media/base/pipeline_impl.h [modify] https://crrev.com/c00d564817f2205be4d12e3ece58d0704d76e50d/media/base/pipeline_impl_unittest.cc [modify] https://crrev.com/c00d564817f2205be4d12e3ece58d0704d76e50d/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/c00d564817f2205be4d12e3ece58d0704d76e50d/media/filters/pipeline_controller.cc [modify] https://crrev.com/c00d564817f2205be4d12e3ece58d0704d76e50d/media/filters/pipeline_controller.h [modify] https://crrev.com/c00d564817f2205be4d12e3ece58d0704d76e50d/media/filters/pipeline_controller_unittest.cc [modify] https://crrev.com/c00d564817f2205be4d12e3ece58d0704d76e50d/media/test/pipeline_integration_test_base.cc
,
Feb 13 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4 commit b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4 Author: Dale Curtis <dalecurtis@chromium.org> Date: Tue Feb 13 21:39:28 2018 Remove kHaveFutureData restriction from play state notifications. This reland marks media-src-suspend-after-have-future-data and media-src-suspend-after-have-metadata as SlowTest(s) now that they no longer permanently time out. Each takes ~2ish seconds in release build and so meets the criteria for being a SlowTest. This also changes WMPI to not try and get the current time value prior to the HaveCurrentData state (which Blink already respects). ========= [ Original Description Follows ] =============== HTMLMediaElement should tell WebMediaPlayer about play state changes as soon as the kHaveMetaData state is reached. This will eventually allow us to delete a bunch of guessing-game type code for idle suspension and instead rely on actually getting a play() call from Blink to wake up when needed. We can't stop WMPI from reaching the future data state quite yet though, because it has implications on the delivery of 'canplay' and 'canplaythrough' events that need to be part of a larger preload=metadata experiment. See old discussion on this here: https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Jul/0007.html Note: This change exposed some bugs in how the ready state maximum is used and the states at which a video frame is expected. These issues are fixed in the HTMLVideoElement. BUG= 694855 , 756897 , 809998 TEST=existing suspend tests no longer flake. Change-Id: Ie6297df474c1f5da56ca6c0e81efa636fbc349bf Reviewed-on: https://chromium-review.googlesource.com/915081 Reviewed-by: Frank Liberato <liberato@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/heads/master@{#536487} [modify] https://crrev.com/b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4/third_party/WebKit/LayoutTests/SlowTests [modify] https://crrev.com/b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4/third_party/WebKit/Source/core/html/media/HTMLMediaElement.cpp [modify] https://crrev.com/b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4/third_party/WebKit/Source/core/html/media/HTMLVideoElement.cpp
,
Feb 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dc5d47c8ab0d958bdd967a834c472106f9697375 commit dc5d47c8ab0d958bdd967a834c472106f9697375 Author: Dale Curtis <dalecurtis@chromium.org> Date: Thu Feb 15 11:15:19 2018 Revert "Remove kHaveFutureData restriction from play state notifications." This reverts commit b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4. Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=812423 Original change's description: > Remove kHaveFutureData restriction from play state notifications. > > This reland marks media-src-suspend-after-have-future-data and > media-src-suspend-after-have-metadata as SlowTest(s) now that they no > longer permanently time out. Each takes ~2ish seconds in release build > and so meets the criteria for being a SlowTest. > > This also changes WMPI to not try and get the current time value prior > to the HaveCurrentData state (which Blink already respects). > > ========= [ Original Description Follows ] =============== > > HTMLMediaElement should tell WebMediaPlayer about play state changes > as soon as the kHaveMetaData state is reached. > > This will eventually allow us to delete a bunch of guessing-game type > code for idle suspension and instead rely on actually getting a play() > call from Blink to wake up when needed. > > We can't stop WMPI from reaching the future data state quite yet though, > because it has implications on the delivery of 'canplay' and 'canplaythrough' > events that need to be part of a larger preload=metadata experiment. See old > discussion on this here: > > https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Jul/0007.html > > Note: This change exposed some bugs in how the ready state maximum is used > and the states at which a video frame is expected. These issues are fixed > in the HTMLVideoElement. > > BUG= 694855 , 756897 , 809998 > TEST=existing suspend tests no longer flake. > > Change-Id: Ie6297df474c1f5da56ca6c0e81efa636fbc349bf > Reviewed-on: https://chromium-review.googlesource.com/915081 > Reviewed-by: Frank Liberato <liberato@chromium.org> > Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> > Commit-Queue: Dale Curtis <dalecurtis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#536487} TBR=dalecurtis@chromium.org,mlamouri@chromium.org,liberato@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 694855 , 756897 , 809998 Change-Id: I176b5116682851853ff2773e6e4a77a9ac2664b2 Reviewed-on: https://chromium-review.googlesource.com/920481 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> Cr-Commit-Position: refs/heads/master@{#536988} [modify] https://crrev.com/dc5d47c8ab0d958bdd967a834c472106f9697375/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/dc5d47c8ab0d958bdd967a834c472106f9697375/third_party/WebKit/LayoutTests/SlowTests [modify] https://crrev.com/dc5d47c8ab0d958bdd967a834c472106f9697375/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/dc5d47c8ab0d958bdd967a834c472106f9697375/third_party/WebKit/Source/core/html/media/HTMLMediaElement.cpp [modify] https://crrev.com/dc5d47c8ab0d958bdd967a834c472106f9697375/third_party/WebKit/Source/core/html/media/HTMLVideoElement.cpp
,
Feb 15 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d1d5207d97b36a4f46a62563860b5792735db554 commit d1d5207d97b36a4f46a62563860b5792735db554 Author: Dale Curtis <dalecurtis@chromium.org> Date: Thu Feb 15 17:37:20 2018 Revert "Remove kHaveFutureData restriction from play state notifications." This reverts commit b0ec93f6f1c9dd4754c813f198336a7a4c2f22a4. Reason for revert: https://bugs.chromium.org/p/chromium/issues/detail?id=812423 Original change's description: > Remove kHaveFutureData restriction from play state notifications. > > This reland marks media-src-suspend-after-have-future-data and > media-src-suspend-after-have-metadata as SlowTest(s) now that they no > longer permanently time out. Each takes ~2ish seconds in release build > and so meets the criteria for being a SlowTest. > > This also changes WMPI to not try and get the current time value prior > to the HaveCurrentData state (which Blink already respects). > > ========= [ Original Description Follows ] =============== > > HTMLMediaElement should tell WebMediaPlayer about play state changes > as soon as the kHaveMetaData state is reached. > > This will eventually allow us to delete a bunch of guessing-game type > code for idle suspension and instead rely on actually getting a play() > call from Blink to wake up when needed. > > We can't stop WMPI from reaching the future data state quite yet though, > because it has implications on the delivery of 'canplay' and 'canplaythrough' > events that need to be part of a larger preload=metadata experiment. See old > discussion on this here: > > https://lists.w3.org/Archives/Public/public-whatwg-archive/2015Jul/0007.html > > Note: This change exposed some bugs in how the ready state maximum is used > and the states at which a video frame is expected. These issues are fixed > in the HTMLVideoElement. > > BUG= 694855 , 756897 , 809998 > TEST=existing suspend tests no longer flake. > > Change-Id: Ie6297df474c1f5da56ca6c0e81efa636fbc349bf > Reviewed-on: https://chromium-review.googlesource.com/915081 > Reviewed-by: Frank Liberato <liberato@chromium.org> > Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> > Commit-Queue: Dale Curtis <dalecurtis@chromium.org> > Cr-Commit-Position: refs/heads/master@{#536487} TBR=dalecurtis@chromium.org,mlamouri@chromium.org,liberato@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 694855 , 756897 , 809998 Change-Id: I176b5116682851853ff2773e6e4a77a9ac2664b2 Reviewed-on: https://chromium-review.googlesource.com/920481 Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Commit-Queue: Mounir Lamouri <mlamouri@chromium.org> Cr-Original-Commit-Position: refs/heads/master@{#536988}(cherry picked from commit dc5d47c8ab0d958bdd967a834c472106f9697375) Reviewed-on: https://chromium-review.googlesource.com/922301 Reviewed-by: Claude Masso <cmasso@chromium.org> Cr-Commit-Position: refs/branch-heads/3348@{#3} Cr-Branched-From: 62c55a88a1aa6b877054b25d06e3ab0ea505d253-refs/heads/master@{#536934} [modify] https://crrev.com/d1d5207d97b36a4f46a62563860b5792735db554/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/d1d5207d97b36a4f46a62563860b5792735db554/third_party/WebKit/LayoutTests/SlowTests [modify] https://crrev.com/d1d5207d97b36a4f46a62563860b5792735db554/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/d1d5207d97b36a4f46a62563860b5792735db554/third_party/WebKit/Source/core/html/media/HTMLMediaElement.cpp [modify] https://crrev.com/d1d5207d97b36a4f46a62563860b5792735db554/third_party/WebKit/Source/core/html/media/HTMLVideoElement.cpp
,
Feb 24 2018
,
Mar 1 2018
,
Mar 6 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a49d6f3feaa1fd2d4af3c61f718ce09a1ea7a300 commit a49d6f3feaa1fd2d4af3c61f718ce09a1ea7a300 Author: Dale Curtis <dalecurtis@chromium.org> Date: Tue Mar 06 01:01:57 2018 Move src= suspend tests into SlowTests. These should just be slow not flakily fail. BUG= 817705 , 694855 Change-Id: I371f57744442eebad53aea566b751df47d8b9a9e Reviewed-on: https://chromium-review.googlesource.com/950142 Reviewed-by: Thomas Guilbert <tguilbert@chromium.org> Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/heads/master@{#541011} [modify] https://crrev.com/a49d6f3feaa1fd2d4af3c61f718ce09a1ea7a300/third_party/WebKit/LayoutTests/SlowTests [modify] https://crrev.com/a49d6f3feaa1fd2d4af3c61f718ce09a1ea7a300/third_party/WebKit/LayoutTests/TestExpectations
,
Mar 9 2018
,
Mar 26 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2dc6089adc149d0ecb8ea44898e56c6a2b79c2e1 commit 2dc6089adc149d0ecb8ea44898e56c6a2b79c2e1 Author: Dale Curtis <dalecurtis@chromium.org> Date: Mon Mar 26 16:47:58 2018 Add the ability to suspend preload=metadata playbacks in some cases. This adds a base::Feature, kPreloadMetadataSuspend, for suspending preload=metadata players that are either audio-only or have video and have a poster image. Given the uncertainty and issues with canplay, canplaythrough, the current version of this feature signals HAVE_ENOUGH when after a suspended startup successfull completes. This also fixes a minor issue where streams with a positive start time were not suspending at the right time. BUG= 694855 TEST=enabling feature still passes all bots. More layout tests, which are disabled by default in this CL since the feature is disabled by default for now. Change-Id: Iccd79eddbd7a3b03933b3090eb99ad19c2fed768 Reviewed-on: https://chromium-review.googlesource.com/978867 Reviewed-by: Frank Liberato <liberato@chromium.org> Reviewed-by: Dan Sanders <sandersd@chromium.org> Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/heads/master@{#545817} [modify] https://crrev.com/2dc6089adc149d0ecb8ea44898e56c6a2b79c2e1/media/base/media_switches.cc [modify] https://crrev.com/2dc6089adc149d0ecb8ea44898e56c6a2b79c2e1/media/base/media_switches.h [modify] https://crrev.com/2dc6089adc149d0ecb8ea44898e56c6a2b79c2e1/media/base/pipeline_impl.cc [modify] https://crrev.com/2dc6089adc149d0ecb8ea44898e56c6a2b79c2e1/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/2dc6089adc149d0ecb8ea44898e56c6a2b79c2e1/media/blink/webmediaplayer_impl.h [modify] https://crrev.com/2dc6089adc149d0ecb8ea44898e56c6a2b79c2e1/third_party/WebKit/LayoutTests/TestExpectations [modify] https://crrev.com/2dc6089adc149d0ecb8ea44898e56c6a2b79c2e1/third_party/WebKit/LayoutTests/http/tests/security/resources/webaudio/media-element-audio-source-node-test.js [add] https://crrev.com/2dc6089adc149d0ecb8ea44898e56c6a2b79c2e1/third_party/WebKit/LayoutTests/media/audio-src-suspend-after-have-metadata.html [modify] https://crrev.com/2dc6089adc149d0ecb8ea44898e56c6a2b79c2e1/third_party/WebKit/LayoutTests/media/suspend-util.js [add] https://crrev.com/2dc6089adc149d0ecb8ea44898e56c6a2b79c2e1/third_party/WebKit/LayoutTests/media/video-src-skip-suspend-after-have-metadata.html [add] https://crrev.com/2dc6089adc149d0ecb8ea44898e56c6a2b79c2e1/third_party/WebKit/LayoutTests/media/video-src-suspend-after-have-metadata.html
,
Mar 29 2018
,
Mar 30 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ff57655e8674e9b167a062cec096b2c55a7da91d commit ff57655e8674e9b167a062cec096b2c55a7da91d Author: Dale Curtis <dalecurtis@chromium.org> Date: Fri Mar 30 02:32:44 2018 Ensure metrics for preload=metadata suspend are accurate. This adds three new metrics and fixes two existing ones to ensure they are recorded accurately when we suspend for preload=metadata, the new metrics are: Media.PreloadMetadataSuspendWasIdeal.Audio Media.PreloadMetadataSuspendWasIdeal.AudioVideo Media.PreloadMetadataSuspendWasIdeal.Video Which are boolean and set to true if a preload=metadata suspend is not *immediately* aborted. Immediate abort can only result from WMPI not knowing the true playback state until HAVE_FUTURE_DATA; this is a lie we'd like to fix, and these metrics will help us judge the importance of fixing it. The fixed metrics are: Media.HasEverPlayed Media.TimeToFirstFrame.SRC Media.TimeToPlayReady.SRC HasEverPlayed is fixed by not counting the BUFFERING_HAVE_ENOUGH event when it's issued for a suspended startup (determined by a new media log property, which is gross, but how the existing system must work for now). TimeToFirstFrame and TimeToPlayReady are metrics which are relative to the time of load start, but this does not work when we stop after metadata. To ensure these metrics are still accurate once playback does resume, we cache the |time_to_metadata_| value and use it account for the section of time prior to metadata suspend. BUG= 694855 , 827396 TEST=manually verified chrome://histograms has the right values. Change-Id: I5cdeea08a798ed0875c2ce603d6ee2b9a31d1e55 Reviewed-on: https://chromium-review.googlesource.com/980589 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Mark Pearson <mpearson@chromium.org> Reviewed-by: Frank Liberato <liberato@chromium.org> Cr-Commit-Position: refs/heads/master@{#547076} [modify] https://crrev.com/ff57655e8674e9b167a062cec096b2c55a7da91d/content/browser/media/media_internals.cc [modify] https://crrev.com/ff57655e8674e9b167a062cec096b2c55a7da91d/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/ff57655e8674e9b167a062cec096b2c55a7da91d/media/blink/webmediaplayer_impl.h [modify] https://crrev.com/ff57655e8674e9b167a062cec096b2c55a7da91d/tools/metrics/histograms/histograms.xml
,
Jul 24
,
Jul 24
|
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by dalecur...@chromium.org
, Feb 22 2017