src= video track changes may advance video frames ahead of audio indefinitely with enable/disable chains. |
||||||||||||||
Issue descriptionTo reproduce launch chrome with, --enable-feature=BackgroundVideoTrackOptimization then choose some video like: https://storage.googleapis.com/dalecurtis/buck720.mp4 Then open a new tab once it starts playing. Swap between these tabs rapidly and observe that each time you get a new video frame that is further into the future. If you stop swapping you'll notice that audio is very far behind; ~behind by the keyframe distance times the number of swaps. This is because for src= playback we do not have a packet queue, we end up tossing packets as soon as they are sent to the decoder. When the track disable event happens we also empty our frame queue. So each track switch jumps to the next key frame. It's easily possible to get minutes ahead of audio using this approach. There are a few consequences of this behavior: 1. If we reach EOS for audio while this occurs and then there is a loop or subsequent playback attempt, we will record the time from shown until that playback attempt as Media.Video.TimeFromForegroundToFirstFrame.DisableTrack; leading to inflation in that UMA metric. 2. The above metric is meaningless for src= since the actual "time to first frame" is equal to the current audio time relative to the time of the next keyframe. I.e. it may be up to 5s, 10s, or whatever we've chosen as our cutoff point for this optimization. Note: #2 is true on Android as well. Note: MSE is not affected because we can seek the video track independently. To resolve the issue with src= I think we'll either need to maintain a packet queue or figure out a way to seek ffmpeg while preserving the current audio position. The latter shouldn't be too hard, we will have the data in the multibuffer cache so the seek itself will be fast. We would then need to drop packets that have already been buffered or vended for other streams while accumulating the video packets necessary.
,
Apr 11 2017
General idea https://codereview.chromium.org/2808833005 -- definitely needs more work to not break a bunch of existing content though.
,
Apr 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1af3c1a4a85d37eaa0398bb5c115d2349f01c6ef commit 1af3c1a4a85d37eaa0398bb5c115d2349f01c6ef Author: dalecurtis <dalecurtis@chromium.org> Date: Tue Apr 11 00:53:49 2017 Don't histogram paint time after ending or seeking. There will be no further paint if we seek past the end of the video stream or reach end of stream; without this, the next actual play which vends a video frame will trigger the histogram path. Which may be minutes or hours later. BUG=709302 TEST=seek to near end of video, trigger bg opt, observe no bad time. Review-Url: https://codereview.chromium.org/2805683004 Cr-Commit-Position: refs/heads/master@{#463476} [modify] https://crrev.com/1af3c1a4a85d37eaa0398bb5c115d2349f01c6ef/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/1af3c1a4a85d37eaa0398bb5c115d2349f01c6ef/media/blink/webmediaplayer_impl.h
,
Apr 11 2017
Dale, could you merge your change to 58 so we get fixed data faster (e.g. it may remove the cause of the 95% delay reported).
,
Apr 11 2017
Sure, +MR-58 for UMA fix.
,
Apr 11 2017
This bug requires manual review: We are only 13 days from stable. Please contact the milestone owner if you have questions. Owners: amineer@(Android), cmasso@(iOS), bhthompson@(ChromeOS), govind@(Desktop) For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
,
Apr 11 2017
Please add all applicable OS. Seems like CL in #3 is not yet baked in canary, let us know whether its a safe merge. Note : Final Beta RC cut is today (04/11) 4.00 PM PST and we will be rolling out M58 to Pre-Stable next week.
,
Apr 11 2017
,
Apr 13 2017
Based on conversation, this is well tested, safe merge and only affects metrics. Approving for M58 merge.
,
Apr 13 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ced09bec389e0a04ff27f49527b25557dae7420f commit ced09bec389e0a04ff27f49527b25557dae7420f Author: Dale Curtis <dalecurtis@chromium.org> Date: Thu Apr 13 22:27:31 2017 Merge M58: "Don't histogram paint time after ending or seeking." There will be no further paint if we seek past the end of the video stream or reach end of stream; without this, the next actual play which vends a video frame will trigger the histogram path. Which may be minutes or hours later. BUG=709302 TEST=seek to near end of video, trigger bg opt, observe no bad time. Review-Url: https://codereview.chromium.org/2805683004 Cr-Commit-Position: refs/heads/master@{#463476} (cherry picked from commit 1af3c1a4a85d37eaa0398bb5c115d2349f01c6ef) Review-Url: https://codereview.chromium.org/2821573002 . Cr-Commit-Position: refs/branch-heads/3029@{#701} Cr-Branched-From: 939b32ee5ba05c396eef3fd992822fcca9a2e262-refs/heads/master@{#454471} [modify] https://crrev.com/ced09bec389e0a04ff27f49527b25557dae7420f/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/ced09bec389e0a04ff27f49527b25557dae7420f/media/blink/webmediaplayer_impl.h
,
Apr 19 2017
Tested the issue on Mac os 10.12.3 using chrome M58 #58.0.3029.81 and followed the below steps : 1. Launched chrome with "--enable-feature=BackgroundVideoTrackOptimization" from command line. 2. Opened video "https://storage.googleapis.com/dalecurtis/buck720.mp4" and played the video. 3. Once the video starts playing ..opened new tab page in text tab and started swapping between the two tabs fastly and observed as attached in screencast . @Could someone look into the attached screencast and help us in confirming the fix or if we had missed any steps in verifying the issue. Thanks!
,
Apr 19 2017
This issue is not fixed, we just modified a histogram.
,
Apr 27 2017
Hey Sergey! Do you know if/when you have a chance to look into this?
,
Apr 27 2017
Sorry for the delay on this, I'm planning to take a closer look today or tomorrow.
,
Apr 29 2017
I think Dale's CL https://codereview.chromium.org/2808833005 is getting us closer to fixing this, but a few things there are not clear to me, let's discuss on https://codereview.chromium.org/2808833005 how we can address those.
,
Jun 13 2017
Issue 732044 has been merged into this issue.
,
Jun 19 2017
,
Jul 10 2017
,
Aug 8 2017
Think we can drop the priority on this, it'd still be nice to fix, but I realized there's a trivial workaround: use a timeout, which provides better ux to boot. https://chromium-review.googlesource.com/c/607154
,
Aug 8 2017
Actually testing with src= some more, there are cases when you get a keyframe at max distance and end up waiting 5 seconds for the next frame. We can't enable src= without seeking support and we can't enable seeking without fixing how track changes work :(
,
Aug 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9e3773cd49b2469b25404dd09022824b953393d5 commit 9e3773cd49b2469b25404dd09022824b953393d5 Author: Dale Curtis <dalecurtis@chromium.org> Date: Fri Aug 11 22:47:47 2017 Require at least two frames for resume after Flush() or underflow. Since we don't have duration information on our video frames, without two frames we can't know if a given frame is effective for rendering. In low delay mode, since we don't know we count that frame as effective and start rendering immediately even if the frame is actually far behind the media clock. It'd be nice to have duration information on the frames as a hint, but reorder windows make that difficult across our decoders. Since we don't want to mess with time-to-first-frame upon startup, the best we can do is tweak post-Flush() or underflow behavior to wait for two frames before resuming. This means that after a seek we may take longer to paint the first updated frame at the new time, but we will be sure that it's the right one now. BUG=709302 TEST=unittests still pass. Change-Id: I490ec60f86d48c73c18d71372dcf045dea2d2ade Reviewed-on: https://chromium-review.googlesource.com/607214 Commit-Queue: Matthew Wolenetz <wolenetz@chromium.org> Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org> Cr-Commit-Position: refs/heads/master@{#493874} [modify] https://crrev.com/9e3773cd49b2469b25404dd09022824b953393d5/media/renderers/video_renderer_impl.cc [modify] https://crrev.com/9e3773cd49b2469b25404dd09022824b953393d5/media/renderers/video_renderer_impl.h [modify] https://crrev.com/9e3773cd49b2469b25404dd09022824b953393d5/media/renderers/video_renderer_impl_unittest.cc
,
Sep 15 2017
Sergey are you still planning to get back to this? If not we should find a new owner. I see the following outstanding CLs: https://codereview.chromium.org/2845333003/ https://codereview.chromium.org/2808833005/
,
Oct 20 2017
Hi: We have a project running on chrome, using the MSE (Media Source extension), it is unexpected when run on the new chrome 62 version since it is normal on the chrome 61 version. We read the chrome related source code, found that it is due to the new chrome 62 version will wait until it have two frames in rendering(VideoRendererImpl::FrameReady()). We expect that we call appendBuffer() to append one frame and it can be immediately rendered and displayed, cause we are using the sequence mode and low delay mode. Is there any way to guarantee immediately rendering after call appendBuffer() to append one frame (in low latency, sequence mode)?
,
Nov 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/21ece16c7d9e0d06786579e7d86a7993a706f656 commit 21ece16c7d9e0d06786579e7d86a7993a706f656 Author: Dale Curtis <dalecurtis@chromium.org> Date: Wed Nov 29 02:10:55 2017 Use video frame duration metadata instead of waiting for 2 frames. http://crrev.com/493874 changed the low delay case to wait for two frames since we need to be sure we don't resume until we actually have valid frames since the first frame was valid forever without duration information. It turns out we actually do have duration infromation hanging off the DecoderStream, so use this information and set it as the FRAME_DURATION metadata key for each VideoFrame. We can then have the algorithm use this information for the estimated end time of frames when only a single frame is present in the queue. Note: The video-canvas layout test was showing the wrong frame; I manually extracted the frames and verified that before we were showing the "7" frame when pts=2.0s == "6" frame. I've updated the test expectations appropriately. BUG= 786576 , 767878 ,709302 TEST=new unittest, old unittests pass w/o modification, manual test of 4k60 vp9 low latency content doesn't exhibit multiple stalls after returning to the foreground when the video track has been disabled, manual test with https://jsfiddle.net/u3enjLzz/ 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: I963903d6a173038a2d534db4e040c8f4774825b5 Reviewed-on: https://chromium-review.googlesource.com/780267 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Matthew Wolenetz <wolenetz@chromium.org> Cr-Commit-Position: refs/heads/master@{#519961} [modify] https://crrev.com/21ece16c7d9e0d06786579e7d86a7993a706f656/media/filters/video_renderer_algorithm.cc [modify] https://crrev.com/21ece16c7d9e0d06786579e7d86a7993a706f656/media/filters/video_renderer_algorithm.h [modify] https://crrev.com/21ece16c7d9e0d06786579e7d86a7993a706f656/media/filters/video_renderer_algorithm_unittest.cc [modify] https://crrev.com/21ece16c7d9e0d06786579e7d86a7993a706f656/media/renderers/video_renderer_impl.cc [modify] https://crrev.com/21ece16c7d9e0d06786579e7d86a7993a706f656/media/renderers/video_renderer_impl.h [modify] https://crrev.com/21ece16c7d9e0d06786579e7d86a7993a706f656/third_party/WebKit/LayoutTests/media/video-canvas.html [delete] https://crrev.com/afedd691d118f767302c309ffa7b337ae07ca13e/third_party/WebKit/LayoutTests/platform/linux/media/video-canvas-expected.txt
,
Dec 14 2017
Last comment with my thoughts: Argh, I had a long draft comment here that got deleted by BeyondCorp :( Here's take 2: I think the root of all our issues stem from the fact that we allowed MediaResource::SetStreamStatusChangedCB to be called from the Renderer; we should not have. By doing so we've introduced a back channel which has renderer state implications that the pipeline is unaware of. Instead PipelineImpl and PipelineController should have a new kTrackChanging state; they should pass a PipelineStatusCB into OnEnabledAudioTracksChanged() and OnSelectedVideoTrackChanged(). PipelineController should have queuing code for handling pending seek / pending track change operations. PipelineImpl should trigger the demuxer track change and provide a PipelineStatusCB. When that completes it should trigger a new Renderer::OnAudioTracksChanged(PipelineStatusCB), Renderer::OnVideoTracksChanged(PipelineStatusCB) methods. These methods should be split apart from the existing RendererImpl::OnStreamStatusChanged() method, but largely do as they do today. When the renderers have been flushed and restarted, they should fire the pipeline status cb. This will percolate back up and the Pipeline will resume allowing other state change events. We don't need the DemuxerStream pointer passed back to OnStreamStatusChanged() today since we only allow one enabled track, we can just reuse MediaResource::GetFirstStream(); later we could add a MediaResource::GetStreamForTrackID() to handle the multiple audio/video case. This allows us to delete the lock hack added to RendererImpl since PipelineImpl will handle media time caching during track changes (like it does for seek). It allows us not to worry about the interplay of seeks and track changes since we'll have one top level arbiter. This keeps our code consistent with what we're doing for seeks; i.e. all async operations must be complete before other state change events are allowed. Another potential improvement after the above is done s to allow renderer re-initialization similar to how config changes work today. Pipeline would call something like Renderer::Flush(<tracks>), Demuxer::OnXXXTracksChanged(), then Renderer::Initialize(<tracks>). This allows us to avoid having any special logic in RendererImpl for track changes.
,
Jan 12 2018
Ted is picking this up for the coming quarter. The gist is we need to convert track changes from being synchronous to asynchronous. The previous comment details my thoughts on it.
,
Apr 19 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da82b60044d96bf76bf67f6d85bd8c993db82f8f commit da82b60044d96bf76bf67f6d85bd8c993db82f8f Author: Ted Meyer <tmathmeyer@chromium.org> Date: Thu Apr 19 00:36:56 2018 Replace StreamStatusChanged callback with async track switching Allow the pipeline to control the entire track switch process without the media source communicating directly with the renderer. WMPL | pipeline | demuxer | demuxer_stream | renderer | video/audio_renderer ───────────────────────────Old Control Flow───────────────────────────────── | | | | | switch_track | | | | ---------> | | | | | switch track | | | | ---------> | | | | | disable/enable stream | | | | -----------> | | ───────────────────────Sometime in the future─────────────────────────────── | | | read from stream | | | <----------------------------- | | | |End of stream | | | -----------------------------> | | | | OnStreamStatus | | | | <---------------- | | | | Flush/Restart/Reset | | | | ----------------> | | | | OnBufferingStateChange | | | | <---------------- | | OnBufferingStateChange | | | <-------------------------------------- | OnBufferingStateChange | | | <---------- | | | | ───────────────────────────New Control Flow───────────────────────────────── WMPL | pipeline | demuxer | demuxer_stream | renderer | video/audio_renderer | | | | | switch_track | | | | ---------> | | | | | switch track | | | | ---------> | | | | | disable/enable stream | | | | -----------> | | | active streams | | | | <--------- | | | | | switch track | | | --------------------------------------> | | | | | Flush/Restart/Reset | | | | ---------------> | | Notify pipeline of completed track change | <----------------------------------------------------- ───────────────────────Sometime in the future─────────────────────────────── | | | | OnBufferingStateChange | | | | <---------------- | | OnBufferingStateChange | | | <-------------------------------------- | OnBufferingStateChange | | | <---------- | | | | In the new control flow the pipeline is able to control both the demuxer and the renderer, instead of controlling the demuxer and waiting for the renderer to act on it's own. Bug: 709302 Cq-Include-Trybots: luci.chromium.try:android_optional_gpu_tests_rel;luci.chromium.try:linux_optional_gpu_tests_rel;luci.chromium.try:mac_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel;luci.chromium.try:win_optional_gpu_tests_rel Change-Id: Iafc862e79f5afc1d3a6ea3400c03decd0ce91d27 Reviewed-on: https://chromium-review.googlesource.com/899843 Commit-Queue: Ted Meyer <tmathmeyer@chromium.org> Reviewed-by: Sergey Volk <servolk@chromium.org> Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Dan Sanders <sandersd@chromium.org> Reviewed-by: Xiangjun Zhang <xjz@chromium.org> Reviewed-by: Yuri Wiitala <miu@chromium.org> Cr-Commit-Position: refs/heads/master@{#551901} [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/base/demuxer.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/base/fake_demuxer_stream.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/base/fake_demuxer_stream.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/base/media_resource.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/base/media_url_demuxer.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/base/media_url_demuxer.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/base/mock_filters.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/base/pipeline.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/base/pipeline_impl.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/base/pipeline_impl.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/base/renderer.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/base/renderer.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/filters/chunk_demuxer.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/filters/chunk_demuxer.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/filters/chunk_demuxer_unittest.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/filters/ffmpeg_demuxer.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/filters/ffmpeg_demuxer.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/filters/ffmpeg_demuxer_unittest.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/filters/pipeline_controller.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/filters/pipeline_controller.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/filters/pipeline_controller_unittest.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/mojo/services/media_resource_shim.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/mojo/services/media_resource_shim.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/remoting/courier_renderer.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/remoting/end2end_test_renderer.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/remoting/end2end_test_renderer.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/remoting/fake_media_resource.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/remoting/fake_media_resource.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/remoting/stream_provider.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/renderers/renderer_impl.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/renderers/renderer_impl.h [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/renderers/renderer_impl_unittest.cc [modify] https://crrev.com/da82b60044d96bf76bf67f6d85bd8c993db82f8f/media/test/pipeline_integration_test.cc
,
Jul 19
This bug has a stale milestone. Please close appropriately, update the milestone and set P1/P2, or drop the milestone and set as P3. I'll automatically punt these issues to M70 next week otherwise.
,
Jul 27
These issues have seen no update and have stale milestones, dropping priority and removing milestone.
,
Aug 13
Hi there, I noticed an issue with HLS video playback sometimes stalling out before playback can begin in Chrome on Android after da82b60044. As far as I can tell, this issue is triggered by PipelineImpl::OnBufferingStateChange(BUFFERING_HAVE_ENOUGH) [1] getting called prior to PipelineController::OnTrackChangeComplete() [2]. This causes the call to WMPI::OnBufferingStateChangeInternal() to return early because the PipelineController is not stable [3], and the pipeline is not stable because OnTrackChangeComplete() has not been called yet. After OnTrackChangeComplete() is finally called, there's nothing to trigger playback to actually begin (this is normally accomplished by OnBufferingStateChange() getting called). This has been easiest to reproduce on video.foxnews.com with "Request Desktop Sites" enabled (required to get served HLS video), but I've seen the same issue on espn.com. Repro steps: 1. Launch Chrome on Android 2. Enable "Request Desktop Sites" 3. Navigate to video.foxnews.com 4. The first video that loads will attempt to autoplay and will be not be allowed to do so (expected) 5. Scroll down to the list of "Editor's Picks" and select a video - the video should load and attempt to begin playing 6. If the video begins playing without issue, repeat step 5 by selecting a different video On my Pixel 2 (running Android P), the video will fail to play once in every 8 attempts or so. When it does fail, without further input, the spinner over the video will spin indefinitely. It's possible to get the video to play by tapping on the video timeline to seek the video. I built the chrome_public_apk target in an Android workspace on the tip of master with a lot of logging added to the PipelineImpl, PipelineController, and WMPI, and found that the difference between a video playing and failing to play is whether or not OnBufferingStateChange() was called prior to OnTrackChangeComplete(). Reverting da82b60044 locally appears to cause this issue to go away. I'm not a media/ expert and I wasn't able to come up with an obvious fix, so I wanted to bring this to your attention. Let me know if you have any issues reproducing this, or if you'd prefer me to file this as a separate bug instead of tracking it here. [1] https://cs.chromium.org/chromium/src/media/base/pipeline_impl.cc?l=702&rcl=8a9a99f1c29a68370b3d479efc439d8511c9bdb8 [2] https://cs.chromium.org/chromium/src/media/filters/pipeline_controller.cc?l=384&rcl=33cd2bccd7dbd33dfddbe3fb4f4c8a6b1a7e4617 [3] https://cs.chromium.org/chromium/src/media/blink/webmediaplayer_impl.cc?l=1779&rcl=33cd2bccd7dbd33dfddbe3fb4f4c8a6b1a7e4617
,
Aug 13
Interesting, track changes are not enabled for HLS playback. Can you file a new bug with this information?
,
Aug 13
Hi Dale, I've filed https://bugs.chromium.org/p/chromium/issues/detail?id=873837 Thanks!
,
Sep 8
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a918e59ba9c602d08b26e4aac82e7ed20e083c72 commit a918e59ba9c602d08b26e4aac82e7ed20e083c72 Author: Ted Meyer <tmathmeyer@chromium.org> Date: Sat Sep 08 00:16:20 2018 Prepare for src= video background track switch finch experimentation. Previously, the BackgroundVideoTrackOptimization flag was used to control both src= and MSE videos. MSE videos has been fully decoupled from the flag now. This CL also uses max_keyframe_distance as a fixed value, determined by the previous finch experiment on MSE videos. The chrome://flags entry is also being removed in preparation for the experiment and (most likely) subsequent enabling of the feature. Bug: 709302 Change-Id: I3875fc700ceb569da7a7c4fa39b28d938235e6a8 Reviewed-on: https://chromium-review.googlesource.com/1194794 Commit-Queue: Ted Meyer <tmathmeyer@chromium.org> Reviewed-by: Ken Buchanan <kenrb@chromium.org> Reviewed-by: Nasko Oskov <nasko@chromium.org> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org> Reviewed-by: Jochen Eisinger <jochen@chromium.org> Reviewed-by: Dale Curtis <dalecurtis@chromium.org> Cr-Commit-Position: refs/heads/master@{#589728} [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/chrome/browser/about_flags.cc [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/chrome/browser/flag_descriptions.cc [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/chrome/browser/flag_descriptions.h [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/content/browser/renderer_host/render_view_host_impl.cc [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/content/public/common/common_param_traits_macros.h [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/content/public/common/web_preferences.cc [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/content/public/common/web_preferences.h [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/content/renderer/media/media_factory.cc [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/content/renderer/render_view_impl.cc [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/media/base/media_switches.cc [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/media/base/media_switches.h [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/media/blink/webmediaplayer_impl.cc [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/media/blink/webmediaplayer_impl_unittest.cc [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/media/blink/webmediaplayer_params.cc [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/media/blink/webmediaplayer_params.h [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/testing/variations/fieldtrial_testing_config.json [modify] https://crrev.com/a918e59ba9c602d08b26e4aac82e7ed20e083c72/third_party/blink/renderer/core/html/media/html_media_element.cc
,
Sep 18
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/18126ed5fbb639672d928dfe4be3cb67dbaa58f0 commit 18126ed5fbb639672d928dfe4be3cb67dbaa58f0 Author: Dale Curtis <dalecurtis@chromium.org> Date: Tue Sep 18 21:40:23 2018 Ensure video memory statistics are accurate upon ended. When disabling a video track we send an end-of-stream buffer but no true decoded frame, this prevents stats from being updated because the decoded frame count hasn't changed. So histogram metrics around memory still report a full video frame queue instead of 0-1 frames. BUG=709302 TEST=disable track reports lower memory usage now; updated test. R=tmathmeyer Change-Id: I15fcd286f9420d4453a03c2c28503858d96f7841 Reviewed-on: https://chromium-review.googlesource.com/1231697 Commit-Queue: Dale Curtis <dalecurtis@chromium.org> Reviewed-by: Ted Meyer <tmathmeyer@chromium.org> Cr-Commit-Position: refs/heads/master@{#592207} [modify] https://crrev.com/18126ed5fbb639672d928dfe4be3cb67dbaa58f0/media/renderers/video_renderer_impl.cc [modify] https://crrev.com/18126ed5fbb639672d928dfe4be3cb67dbaa58f0/media/renderers/video_renderer_impl.h [modify] https://crrev.com/18126ed5fbb639672d928dfe4be3cb67dbaa58f0/media/renderers/video_renderer_impl_unittest.cc |
||||||||||||||
►
Sign in to add a comment |
||||||||||||||
Comment 1 by dalecur...@chromium.org
, Apr 7 2017Status: Assigned (was: Unconfirmed)